Skip to content

Expose GCP Secret Manager case sensitive option#626

Merged
hramezani merged 5 commits intopydantic:mainfrom
bellmatthewf:expose-gcp-case-sensitivity
Jun 9, 2025
Merged

Expose GCP Secret Manager case sensitive option#626
hramezani merged 5 commits intopydantic:mainfrom
bellmatthewf:expose-gcp-case-sensitivity

Conversation

@bellmatthewf
Copy link
Copy Markdown
Contributor

@bellmatthewf bellmatthewf commented Jun 2, 2025 •
edited
Loading

Issue 625

Issue
The "pydantic-settings[gcp-secret-manager]" integration does not expose the case_sensitive parameter in it's API. It defaults to True and cannot be changed.

class GoogleSecretManagerSettingsSource(EnvSettingsSource):
    def __init__(
        self,
        settings_cls: type[BaseSettings],
        credentials: Credentials | None = None,
        project_id: str | None = None,
        env_prefix: str | None = None,
        env_parse_none_str: str | None = None,
        env_parse_enums: bool | None = None,
        secret_client: SecretManagerServiceClient | None = None,
    ) -> None:
        
        ...

        super().__init__(
            settings_cls,
            case_sensitive=True,
            env_prefix=env_prefix,
            env_ignore_empty=False,
            env_parse_none_str=env_parse_none_str,
            env_parse_enums=env_parse_enums,
        )

Note on changes
I considered using the parse_env_vars util function to keep this source in line with others, but that function isn't ideal here because it would iteratively call the GCP API during instantiation to get each variable value.

Better to keep those calls lazy by implementing case sensitivity changes at the mapping class level

@bellmatthewf bellmatthewf marked this pull request as ready for review June 2, 2025 01:48
hramezani
hramezani reviewed Jun 3, 2025
View reviewed changes
Comment thread pydantic_settings/sources/providers/gcp.py
_secret_client: SecretManagerServiceClient

def __init__(self, secret_client: SecretManagerServiceClient, project_id: str) -> None:
def __init__(self, secret_client: SecretManagerServiceClient, project_id: str, case_sensitive: bool) -> None:
Copy link
Copy Markdown
Member

@hramezani hramezani Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need to define a default value for case_sensitive for backward compatibility?

Copy link
Copy Markdown
Contributor Author

@bellmatthewf bellmatthewf Jun 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users should never be instantiating this class directly. The GoogleSecretManagerMapping class is only instantiated internally within the GoogleSecretManagerSettingsSource class.

I added a default value to the new case_sensitive parameter of that class which will carry through to here.

Copy link
Copy Markdown
Contributor

@ezwiefel ezwiefel Jun 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bellmatthewf - That makes sense to me.

@hramezani
Copy link
Copy Markdown
Member

hramezani commented Jun 3, 2025

Thanks @bellmatthewf for this PR.

@ezwiefel Can you take a look at this PR?

ezwiefel
ezwiefel approved these changes Jun 9, 2025
View reviewed changes
Comment thread pydantic_settings/sources/providers/gcp.py
_secret_client: SecretManagerServiceClient

def __init__(self, secret_client: SecretManagerServiceClient, project_id: str) -> None:
def __init__(self, secret_client: SecretManagerServiceClient, project_id: str, case_sensitive: bool) -> None:
Copy link
Copy Markdown
Contributor

@ezwiefel ezwiefel Jun 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bellmatthewf - That makes sense to me.

@ezwiefel
Copy link
Copy Markdown
Contributor

ezwiefel commented Jun 9, 2025

@hramezani LGTM

@hramezani
Copy link
Copy Markdown
Member

hramezani commented Jun 9, 2025

Thanks @bellmatthewf for the PR and @ezwiefel for review.

@hramezani hramezani merged commit 159ef14 into pydantic:main Jun 9, 2025
19 checks passed
This was referenced Jun 22, 2025
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hramezani hramezani hramezani approved these changes

+1 more reviewer

@ezwiefel ezwiefel ezwiefel approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bellmatthewf @hramezani @ezwiefel