Skip to content

Conversation

@AndreuCodina
Copy link
Contributor

No description provided.

@hramezani
Copy link
Member

Thanks @AndreuCodina for the PR.

Could you please add a test for enabled=False?

@AndreuCodina
Copy link
Contributor Author

Thanks @AndreuCodina for the PR.

Could you please add a test for enabled=False?

If I don't mock .get_secret, I never get a value in settings, and if I mock it, I always get the mocked value. I don't know how it can be tested.

@hramezani
Copy link
Member

Thanks @AndreuCodina for the PR.
Could you please add a test for enabled=False?

If I don't mock .get_secret, I never get a value in settings, and if I mock it, I always get the mocked value. I don't know how it can be tested.

You are mocking list_properties_of_secrets return values like:

        expected_secrets = [
            type('', (), {'name': 'SqlServerUser', 'enabled': True}),
            type('', (), {'name': 'SqlServer--Password', 'enabled': True}),
        ]

You can add a new test and change the enabled value to False, then we expect a validation error because the value is not enabled. right?

@AndreuCodina
Copy link
Contributor Author

Thanks @AndreuCodina for the PR.
Could you please add a test for enabled=False?

If I don't mock .get_secret, I never get a value in settings, and if I mock it, I always get the mocked value. I don't know how it can be tested.

You are mocking list_properties_of_secrets return values like:

        expected_secrets = [
            type('', (), {'name': 'SqlServerUser', 'enabled': True}),
            type('', (), {'name': 'SqlServer--Password', 'enabled': True}),
        ]

You can add a new test and change the enabled value to False, then we expect a validation error because the value is not enabled. right?

I tried that, but I get this:

{'SqlServerPassword': 'SecretValue', 'DisabledSqlServerPassword': 'SecretValue'}

This is the test:

    def test_do_not_load_disabled_secrets(self, mocker: MockerFixture) -> None:
        class AzureKeyVaultSettings(BaseSettings):
            """AzureKeyVault settings."""

            SqlServerPassword: str
            DisabledSqlServerPassword: str

        expected_secrets = [
            type('', (), {'name': 'SqlServerPassword', 'enabled': True}),
            type('', (), {'name': 'DisabledSqlServerPassword', 'enabled': False}),
        ]
        mocker.patch(
            f'{AzureKeyVaultSettingsSource.__module__}.{SecretClient.list_properties_of_secrets.__qualname__}',
            return_value=expected_secrets,
        )
        mocker.patch(
            f'{AzureKeyVaultSettingsSource.__module__}.{SecretClient.get_secret.__qualname__}',
            return_value=KeyVaultSecret(SecretProperties(), 'SecretValue'),
        )
        obj = AzureKeyVaultSettingsSource(
            AzureKeyVaultSettings, 'https://my-resource.vault.azure.net/', DefaultAzureCredential()
        )

        settings = obj()

        assert 'SqlServerPassword' in settings
        assert 'DisabledSqlServerPassword' not in settings

@hramezani
Copy link
Member

You need to change

if key not in self._loaded_secrets:

to if key not in self._loaded_secrets and key in self._secret_names:

@hramezani
Copy link
Member

Thanks @AndreuCodina

@hramezani hramezani merged commit d54d146 into pydantic:main Apr 7, 2025
18 checks passed
@AndreuCodina AndreuCodina deleted the bugfix/read-disabled-secret branch April 7, 2025 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants