feat(azure): support identity binding auth provider - #2839
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds Kubernetes identity binding as a first-class ACR authentication method, integrating it into the existing Azure credential chain and provider configuration.
Changes:
- Introduces an
azcore.TokenCredentialimplementation that exchanges projected SA tokens via the API server identity binding endpoint. - Extends the Azure credential chain to optionally try identity binding before workload identity and managed identity.
- Wires identity binding configuration through the Azure credential provider registration and adds dedicated unit tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| internal/store/credentialprovider/azure/register.go | Adds identityBinding config block and passes it into the Azure credential chain. |
| internal/cloudprovider/azure/tokencredential.go | Adds CreateCredentialChainWithIdentityBinding and makes identity binding first in the chain when configured. |
| internal/cloudprovider/azure/identitybinding.go | New identity binding credential + transport logic (fixed-IP dial + SNI + cluster CA trust). |
| internal/cloudprovider/azure/identitybinding_test.go | Adds unit coverage for validation, token exchange paths, and transport/CA behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2839 +/- ##
==========================================
+ Coverage 76.58% 76.80% +0.22%
==========================================
Files 89 90 +1
Lines 4095 4259 +164
==========================================
+ Hits 3136 3271 +135
- Misses 812 830 +18
- Partials 147 158 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Also the CI seems to failing |
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create identity binding credential: %w", err) | ||
| } | ||
| sources = append(sources, ibCred) |
There was a problem hiding this comment.
It seems Identity binding is tried first, but WI and MI are still always appended, saying a runtime identity binding failure will silently falls back to the node's kubelet identity, which typically holds AcrPull via attach-acr, this is going to restore the node-wide access that is supposed to be restricted. I think we should consider not appending the MI source when identityBinding is explicitly set.
There was a problem hiding this comment.
Good catch — this is an important security point. Fixed in 32274e5: when identity binding is configured it is now used exclusively; workload identity and managed identity are no longer appended as fallbacks. A misconfigured/failing identity binding surfaces as an error instead of silently downgrading to the node kubelet identity (which holds AcrPull via attach-acr). See CreateCredentialChainWithIdentityBinding and the new TestCreateCredentialChainWithIdentityBinding exclusive-use test.
Add Kubernetes identity binding as an ACR authentication method, mirroring the azure-acr-credential-provider implementation in cloud-provider-azure. Instead of federating the projected service account token with Entra (Workload Identity), the token is exchanged directly against the cluster API server's identity binding endpoint (SNI host + fixed API server IP + cluster CA) using a client_credentials + jwt-bearer grant. The resulting AAD access token is then exchanged for an ACR refresh token via the existing flow. - Add identityBindingCredential implementing azcore.TokenCredential - Add CreateCredentialChainWithIdentityBinding; identity binding is tried first in the chain, before workload identity and managed identity - Expose identityBinding options on the Azure credential provider config Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: xinhl <xinhl@microsoft.com>
Address review of the ACR identity binding design so it mirrors how AKS actually provisions the feature and how Ratify sources workload identity config: - Cluster-scoped endpoint values (SNI host, API server host) are injected by the platform via environment variables, not typed into the CRD. On AKS these come from ServiceAccountImagePullProfile.LocalAuthoritySNI and the API server FQDN (the same values AgentBaker passes to the ACR credential provider). - APIServerHost accepts an IP or FQDN (AKS passes the API server FQDN). - The service account token defaults to the standard projected token; the api://AKSIdentityBinding audience is a pod-spec projection concern. - clientID/tenantID remain CRD-optional with AZURE_CLIENT_ID/AZURE_TENANT_ID fallback, matching the workload identity path. - The CRD now exposes only identityBinding.enabled; endpoint config is resolved from env via LoadIdentityBindingConfigFromEnv. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: xinhl <xinhl@microsoft.com>
Add an end-to-end AKS test that exercises the new ACR identity binding credential provider on a real identity-binding (preview) cluster: the projected service account token (audience api://AKSIdentityBinding) is exchanged against the cluster identity binding local authority for an AAD token, then for an ACR refresh token used to pull image manifests and notation signatures. - internal/cloudprovider/azure/identitybinding.go: LoadIdentityBindingConfigFromEnv now falls back to the standard AKS workload-identity webhook env vars (AZURE_KUBERNETES_SNI_NAME, AZURE_KUBERNETES_TOKEN_PROXY, AZURE_KUBERNETES_CA_FILE, AZURE_FEDERATED_TOKEN_FILE) when the ratify-specific AZURE_ACR_IDENTITY_BINDING_* vars are unset, so identity binding works out of the box on AKS. Ratify-specific vars still take precedence. Added tests for the fallback and precedence behavior. - deployments/ratify-gatekeeper-provider: render the azure store credential's identityBinding config in the Executor, annotate the pod with azure.workload.identity/use-identity-binding when enabled, and document the new value. New ratify.identityBindingEnabled helper. - scripts/create-azure-resources-identity-binding.sh: provision cluster + UAMI + identity binding + ACR + use-managed-identity RBAC (no per-cluster FIC). - scripts/azure-ci-test-identity-binding.sh + test/bats/azure-identity-binding-test.bats: deploy ratify with identity binding + inline notation trust store and verify signed images are admitted and unsigned images rejected. - Makefile: add e2e-aks-identity-binding target. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: xinhl <xinhl@microsoft.com>
Verified end-to-end on a real AKS identity-binding (preview) cluster: the ratify image built from this PR, deployed with the azure store credential in identity binding mode, admits the notation-signed image (proving it authenticated to the authenticated ACR via identity binding to pull the manifest and signature) and rejects the unsigned image. - Move the "notation identity binding test" case into the shared test/bats/azure-test.bats instead of a separate file; the identity binding orchestrator runs just that case via `bats -f`. - scripts/create-azure-resources-identity-binding.sh: add --attach-acr so the cluster kubelet identity can pull the ratify provider image and admitted workload images (independent of ratify's own identity binding auth). - scripts/azure-ci-test-identity-binding.sh: drop the unsupported per-store scopes set (Executor CRD has no spec.stores[].scopes) and run the filtered bats case from azure-test.bats. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: xinhl <xinhl@microsoft.com>
Tenant governance policy force-enables the Azure Policy add-on on AKS clusters, which installs its own Gatekeeper and a byovalidation admission webhook that rejects the Gatekeeper constraint/mutation resources this test applies (and re-enables itself shortly after `az aks disable-addons`). Tag the resource group with SkipAKSCluster=true so the policy skips these clusters, avoiding the add-on entirely instead of racing to tear its webhook down. ensure_resource_group merges the tag onto a pre-existing shared RG or creates the RG with the tag; on subscriptions without the policy it is a no-op. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: xinhl <xinhl@microsoft.com>
…ents - Emit an info-level log when acquiring an AAD token through the Azure identity binding auth provider, so operators can confirm from default logs which registry auth path was taken (previously debug-only, invisible at the default log level). - Remove the explanatory comments from the const block in identitybinding.go. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: xinhl <xinhl@microsoft.com>
1f18e74 to
3cfae7b
Compare
Resolve the review feedback on the identity binding credential: - Use identity binding exclusively when configured: no workload-identity or managed-identity fallback. A silent fallback to the node kubelet identity (which holds AcrPull via attach-acr) would restore the node-wide registry access identity binding is meant to restrict, so a misconfigured/failing identity binding now surfaces as an error instead of downgrading. (addresses @charleswool) - Return a clear error when no Azure credential source can be constructed instead of deferring to a less obvious chained-credential error. - Trim the service account token before the empty check so a whitespace-only token file is rejected up front. - Bound the token endpoint response with io.LimitReader and truncate the body included in error messages to guard against oversized/misbehaving responses. - Improve the "identity binding enabled but not configured" error to mention the AKS-injected fallback variable. - Drop the now-unused tenantID from the identity binding credential. Adds unit tests for the exclusive-use chain, whitespace token rejection, response truncation, hostFromURL, and the identity binding provider wiring, and fixes the test build broken by the tenantID removal (the lint/typecheck CI failure). Signed-off-by: Xinhe Li <xinhl@microsoft.com>
The 'validate image signed by leaf cert' test reads ~/.config/notation/truststore/x509/ca/leaf-test/root.crt which is created by the e2e-notation-leaf-cert-setup make target. Add it to the make invocation in azure-ci-test.sh.
e5fa04b to
151dd5e
Compare
Description
Adds Kubernetes identity binding as an ACR authentication method, mirroring the
azure-acr-credential-providerimplementation in cloud-provider-azure.Instead of federating the projected service account token with Entra (Workload Identity), the token is exchanged directly against the cluster API server's identity binding endpoint — a TLS connection whose SNI host is dialed to a fixed API server IP and validated against the cluster CA — using a
client_credentials+jwt-bearergrant. The resulting AAD access token is then exchanged for an ACR refresh token via the existing flow.Changes
internal/cloudprovider/azure/identitybinding.go(new):IdentityBindingConfig+identityBindingCredentialimplementingazcore.TokenCredential. Reads the SA token fresh on each call (rotation-safe), lazily builds/caches a transport that dialsAPIServerIPwhile presentingSNINameand trusting the cluster CA (default/etc/kubernetes/certs/ca.crt). Falls back toAZURE_CLIENT_ID,AZURE_TENANT_ID,AZURE_FEDERATED_TOKEN_FILE.internal/cloudprovider/azure/tokencredential.go: addsCreateCredentialChainWithIdentityBinding(...). When identity binding is configured it is tried first in the chain, before workload identity and managed identity.CreateCredentialChainis retained as a thin wrapper (no breaking change).internal/store/credentialprovider/azure/register.go: exposes anidentityBindingconfig block (sniName,apiServerIP,tokenFilePath,caCertPath) and wires it through.identitybinding_test.go): validation,GetTokensuccess/error paths, transport/CA handling, and chain integration.Testing
go build ./...,go vet, andgofmtall clean.GetWithTTLnetwork-dependent tests hang locally without network egress; this behavior is unchanged frommainand unrelated to this PR.