-
Notifications
You must be signed in to change notification settings - Fork 0
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
refactor!: parser, credentials and secret handling #30
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ntial through env variables
… identity credential
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request contains major changes on the layout of the entire module.
This to remove a lot of dependencies that might not be desired (
azcore
,azidentity
andazsecrets
) and make the module more self-contained. To those who wants to reuse credentials (minimizing calls for tokens, have the same credentials across the whole application) retreived with the help ofazidentity
can still do so with the submoduleauthopts
which provides the option functionWithTokenCredential()
.The built-in credential handling has some limitiations in regards to client credentials (service principal) and managed identities. As for now it only supports client secret and not certificate or assertion. The supported managed identitiy platforms is those based on IMDS(Azure VMs, Container Instances etc) and app service (including container apps). For now a user of the module will have to use
authopts
andTokenCredential
retreived fromazidentity
to those scenarios.Since the removal of the dependency
azidentity
andDefaultAzureCredential
an update to the configuration environment variables have been done. This to yet further the module as self-contained, and to not overlap configuration of this is not desired.The new environment variables are:
Service Principal
AZCFG_KEYVAULT_NAME
- Name of the Azure Key Vault.AZCFG_TENANT_ID
- Tenant ID of the service principal/application registration.AZCFG_CLIENT_ID
- Client ID (also called Application ID) of the service principal/application registration.AZCFG_CLIENT_SECRET
- Client Secret of the service principal/application registration.Managed identity
AZCFG_KEYVAULT_NAME
- Name of the Azure Key Vault.AZCFG_CLIENT_ID
- (Optional) Client ID (also called Application ID) of the Managed Identity. Set if using a user assigned managed identity.Note that the various environment variable names that can be used for the key vault name has been narrowed down to one.
Furthermore the entire model of how options have been updated to the function options pattern. More details can be gleaned from the
README.md
file.Additionally a lot of cleanup of the internal workings have been done and a lot of less good design decisions have been addressed. No more package level variables to configure, instead these are always passed as options to the
Parse
function, or when createing a newparser
.Resolves #28 and resolves #29.