-
Notifications
You must be signed in to change notification settings - Fork 6.8k
docs(proposal): workload identity #26174
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
base: master
Are you sure you want to change the base?
docs(proposal): workload identity #26174
Conversation
🔴 Preview Environment stopped on BunnyshellSee: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
d454057 to
f0cddfb
Compare
A proposal for acquiring temporary repository credentials using the tokenrequest api. Signed-off-by: Blake Pettersson <[email protected]>
f0cddfb to
1e8955c
Compare
Signed-off-by: Blake Pettersson <[email protected]>
Signed-off-by: Blake Pettersson <[email protected]>
| name: argocd-project-production | ||
| annotations: | ||
| azure.workload.identity/client-id: "client-id-uuid" | ||
| azure.workload.identity/tenant-id: "tenant-id-uuid" |
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 haven't read all of it yet, but this is exactly what I'm trying to fix in #26030
It's not enough to set the tenant id here, because Workload Identity can only get one Identity from Entra Id. To make this more broadly applicable, we need the feature to be able to overwrite the tenant id per repository - both for Helm/OCI (Azure Container Registry) and for Git (Azure DevOps).
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.
Hi @lieberlois,
In this proposal you should be able to set a custom tenant id by setting a custom token url in the repo cred.
something like
apiVersion: v1
kind: Secret
metadata:
name: my-ecr-repo
labels:
argocd.argoproj.io/secret-type: repository
stringData:
type: oci
url: oci://your-registry.azurecr.io/myrepository
project: production
workloadIdentityProvider: "azure"
workloadIdentityTokenURL: "https://login.microsoftonline.com/your-custom-tenant-id/oauth2/v2.0/token"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.
But it seems AKS not expose the Token directly, the token exchanging is managed by AKS and AAD. User use label `azure.workload.identity/use=true' to let AKS to write the cred back to the pod.
ref: https://azure.github.io/azure-workload-identity/docs/topics/service-account-labels-and-annotations.html
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 was wondering what @ninjadq is asking here too. I think what would happen is some argo process would:
- Get a token for the service account
/api/v1/namespaces/argocd/serviceaccounts/argocd-project-production/token - Exchange K8s JWT with Azure AD
https://login.microsoftonline.com/{tenant}/oauth2/token - Get the ACR token (simile to 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.
Agreed, it should be added to service account and be read from there for overide.
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's how I've done this in my WIP:
-
The initial acquisition of the Azure token: https://github.com/blakepettersson/argo-cd/blob/feature/workload-identity-v2-wip/util/workloadidentity/v2/identity/azure.go
-
The token being exchanged for an ACR token: https://github.com/blakepettersson/argo-cd/blob/feature/workload-identity-v2-wip/util/workloadidentity/v2/repository/acr.go
-
The intermediary of this whole enchilada: https://github.com/blakepettersson/argo-cd/blob/feature/workload-identity-v2-wip/util/workloadidentity/v2/resolver.go
ninjadq
left a comment
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 proposal looks great! It’s more comprehensive and scalable than current implementation
| **Upgrade:** | ||
| - The `workloadIdentityProvider` field defaults to empty, so existing repositories continue to work unchanged | ||
| - Users opt-in to workload identity by setting `workloadIdentityProvider` to their desired provider | ||
| - No migration required for existing deployments |
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.
Azure workload identity already used for some customer. It seems need some migration to the new proposal
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.
Good point, I'll amend
| name: argocd-project-production | ||
| annotations: | ||
| azure.workload.identity/client-id: "client-id-uuid" | ||
| azure.workload.identity/tenant-id: "tenant-id-uuid" |
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.
But it seems AKS not expose the Token directly, the token exchanging is managed by AKS and AAD. User use label `azure.workload.identity/use=true' to let AKS to write the cred back to the pod.
ref: https://azure.github.io/azure-workload-identity/docs/topics/service-account-labels-and-annotations.html
Signed-off-by: Blake Pettersson <[email protected]>
| **Azure:** | ||
| ```yaml | ||
| apiVersion: v1 | ||
| kind: ServiceAccount |
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.
@blakepettersson As this service account is not used as kubenetes object we are only using it to read the configurations for workload identity, these configurations can be added in the secrets for repo conffigurations?
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.
@jagpreetstamber my concern is having to need to add a bunch of cloud-specific fields to the repository spec itself. IMO having e.g tenantId and clientId are Azure-specific options and should be kept somewhere where we have the options to have more flexibility (i.e annotations). TBF we could have this as annotations on the repository itself, but that would also mean needing to duplicate those settings for all repositories needing to make use of it.
| - ArgoCD stores registry credentials (username/password, tokens) in Kubernetes secrets | ||
| - These credentials are long-lived and must be manually rotated | ||
| - Credential leakage poses significant security risks | ||
| - The existing implementation of Workload Identity is in practice scoped on the whole repo-server, meaning that there |
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.
Should we document a migration path and how we would support any backwards compatibility?
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.
Same comment @ninjadq left below but I didn't see theirs before I left mine. Feel free to resolve.
| │ | ||
| ▼ | ||
| ┌─────────────────────────────────────────────────────────────────────────────┐ | ||
| │ Workload Identity Resolver │ |
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.
What component would be responsible for the token exchange?
| #### Example 1: AWS ECR with Multi-Project Setup | ||
|
|
||
| ```yaml | ||
| # Service Account for production project |
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.
Pull request generator also needs to be added in the scope of workload identity
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.
Yes good point, will check to see if this would require any changes (I don't think so, or it shouldn't be too extensive)
|
|
||
| ```yaml | ||
| # Service Account for production project | ||
| apiVersion: v1 |
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.
Single Sign On Also needs to be added in the scope
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.
Could you expand on why this would be needed?
|
Here's the current WIP |
A proposal for acquiring temporary repository credentials using the tokenrequest api (or via spiffe if applicable).
Checklist: