New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Azure Key Vault settings source #272
base: main
Are you sure you want to change the base?
Add Azure Key Vault settings source #272
Conversation
Thanks @AndreuCodina for this PR.
It has to be added to optional dependencies section Also, I can't see any test added here. You need to add some tests |
How do I make the imports in code? I've created the function |
You need to update the requirements by running |
$ make refresh-lockfiles
`make: *** No rule to make target 'refresh-lockfiles'. Stop.` |
Please refresh your fork. this command was added recently |
field_value: Any | None = None | ||
|
||
# It's not possible to use underscores in Azure Key Vault | ||
secret_name = field_name.replace('_', '-') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to use _extract_field_info to respect the field alias if defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is complex and I don't see the point in aliasing secret names. Can we iterate in the next version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the basic functionality that we already have in all of the source classes.
You need to move the _extract_field_info
to PydanticBaseSettingsSource
class and then call it here. it returns a list of tuples that contain field_key
, env_name
and value_is_complex
. you can use the env_name
to fetch the value from key vault . there are some use-cases of _extract_field_info
in the code, you can check them as well.
tests/test_sources.py
Outdated
AzureKeyVaultSettings, 'https://my-resource.vault.azure.net/', DefaultAzureCredential() | ||
) | ||
|
||
obj.get_field_value(field=FieldInfo(), field_name='sqlserverpassword') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need any assertion here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method is just a mocked HTTP call to an API. I think I can't test anything more. The reliable test is the Jupyter notebook I'm executing against my Azure infrastructure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can test the returned value from the mocked method.
I think it would be good to have a complete test like other tests. I mean having a settings model that uses the AzureKeyVaultSettingsSource
.
def test_azure_key_vault_settings_source():
class AzureKeyVaultSettings(BaseSettings):
my_password: str
sql_server__password: str
@classmethod
def settings_customise_sources(
cls,
settings_cls: type[BaseSettings],
init_settings: PydanticBaseSettingsSource,
env_settings: PydanticBaseSettingsSource,
dotenv_settings: PydanticBaseSettingsSource,
file_secret_settings: PydanticBaseSettingsSource,
) -> tuple[PydanticBaseSettingsSource, ...]:
return (
AzureKeyVaultSettingsSource(
settings_cls, os.environ['KEY_VAULT__URL'], DefaultAzureCredential()
),
)
with mocker.patch(f'{MODULE}.SecretClient.get_secret', return_value=<return value>):
settings = AzureKeyVaultSettings()
assert settings.my_password == ...
assert settings.sql_server__password == ...```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
tests/test_sources.py
Outdated
AzureKeyVaultSettings, 'https://my-resource.vault.azure.net/', DefaultAzureCredential() | ||
) | ||
|
||
obj() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here as well, you can test the mocked function return value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Add Azure Key Vault settings source.
Doubt: Where should I add the required Python packages (
azure-keyvault-secrets==4.8.0
andazure-identity==1.16.0
) to useAzureKeyVaultSettingsSource
?This is the infrastructure:
Closes #143