-
Notifications
You must be signed in to change notification settings - Fork 187
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
Support Workload Identity for Azure Vault #813
Conversation
internal/sops/azkv/keysource.go
Outdated
creds := key.token | ||
if key.token == nil { | ||
// if there is no token credential set, use the DefaultAzureCredential | ||
newCreds, err := azidentity.NewDefaultAzureCredential(nil) |
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.
AFAIK, this attempts to shell-out: https://github.com/Azure/azure-sdk-for-go/blob/sdk/azidentity/v1.3.0-beta.4/sdk/azidentity/azure_cli_credential.go#L95
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.
We still get the attempt to shell out to the CLI when key.token
isn't set and we use the server from sops
https://github.com/mozilla/sops/blob/master/azkv/keysource.go#L142
By using the NewDefaultAzureCredential, we attempt all the other methods(including Workload Identity) before trying to shell out
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.
Given that there are no CLIs in the controller container, this only confuses people. Locally if you run the tests on a dev machine, it will find az
, but it will always fail in-cluster.
Am not sure what is going on in the last commit, why does the |
So we can use credentials from the environment variable if there was nothing configured in the secret. This is basically the approach we use by returning the default server (it checks the environment variables and the CLI last), but in this case, we are using the function provided by Azure which also checks for workload identity environment variables. With this change, users don't have to set a client id in the secret to use Workload Identity(this is also the same way GCP Workload Identity works) |
@somtochiama can you please move the docs to the v1 API spec? Thanks |
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.
LGTM
Thanks @somtochiama 🏅
5a1b325
to
20d5c3d
Compare
Signed-off-by: Somtochi Onyekwere <somtochionyekwere@gmail.com> Co-authored-by: Hidde Beydals <hidde@hhh.computer>
This pull request adds support for using Azure Workload Identity when decrypting secrets in
Kustomization
It updates azidentity dependencies to v1.3.0-beta.4 and uses defaultCredentials from environment variables when no secret is set when decrypting with Azure KMS
Closes #795
Part of: fluxcd/flux2#3041