Skip to content

Preserve registry identity tokens for OCI chart pulls#334

Closed
mateipinzaru-parloa wants to merge 4 commits into
crossplane-contrib:mainfrom
mateipinzaru-parloa:fix/acr-identity-token-oci-auth
Closed

Preserve registry identity tokens for OCI chart pulls#334
mateipinzaru-parloa wants to merge 4 commits into
crossplane-contrib:mainfrom
mateipinzaru-parloa:fix/acr-identity-token-oci-auth

Conversation

@mateipinzaru-parloa

@mateipinzaru-parloa mateipinzaru-parloa commented May 15, 2026

Copy link
Copy Markdown

Description of your changes

Fixes #335

This change preserves Docker credential-helper identity tokens returned by the default keychain and uses them for OCI chart pulls.

go-containerregistry maps Docker helper responses with username <token> to authn.AuthConfig.IdentityToken. Before this change, provider-helm copied only Username and Password into RepoCreds, dropping identity-token credentials such as ACR refresh tokens returned by docker-credential-acr-env. Private OCI chart pulls that relied on provider runtime identity could therefore fall back to anonymous/no usable credentials and fail with 401 unauthorized.

The implementation:

  • Preserves authConfig.IdentityToken when resolving credentials from the default keychain.
  • Extends RepoCreds with IdentityToken and uses ORAS auth.Credential.RefreshToken for OCI identity-token pulls.
  • Keeps existing username/password behavior unchanged for pull secrets and registries where credentials are already materialized as username plus password/access token.
  • Makes identity-token credentials take precedence over Helm's basic-auth login path for OCI pulls when both are present.
  • Resets reused pull-client chart fields before each pull and restores the default Helm registry client after each pull.
  • Preserves Helm's retry transport plus explicit plainHTTP and insecureSkipTLSVerify behavior for the identity-token path.

provider-helm already reuses Helm action clients across reconciles. This PR does not change that ownership model; it resets pull-scoped fields and restores the default registry client so identity-token auth state does not leak into later pulls or install/upgrade actions.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Unit coverage includes keychain identity-token mapping, identity-token precedence, pull-state reset, registry-client reset, malformed OCI host handling, direct HTTP chart URL reset behavior, TLS transport wiring, and a fake OCI bearer challenge that asserts ORAS exchanges the identity token as grant_type=refresh_token.

Latest local validation passed:

go test ./pkg/clients/helm ./pkg/clients/registryauth
go test ./...
go vet ./pkg/clients/helm ./pkg/clients/registryauth
go mod tidy -diff
git diff --check
go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.2 run ./pkg/clients/helm ./pkg/clients/registryauth
GOTOOLCHAIN=go1.24.13 make reviewable

Runtime validation passed against a private ACR OCI Helm chart:

  • Deployed this PR build with Azure Workload Identity enabled for the provider runtime.
  • Created a namespaced Release for a private OCI chart with chart.pullSecretRef omitted.
  • Verified the Release reached Synced=True, Ready=True, and Helm state deployed.
  • As a control, the same chart pull failed with ACR 401 unauthorized before the provider service account was allowed to federate into the runtime identity.

@mateipinzaru-parloa mateipinzaru-parloa force-pushed the fix/acr-identity-token-oci-auth branch from 16cebd2 to f22da52 Compare May 15, 2026 17:10
Signed-off-by: Matei Pinzaru <201221140+mateipinzaru-parloa@users.noreply.github.com>
@mateipinzaru-parloa mateipinzaru-parloa force-pushed the fix/acr-identity-token-oci-auth branch from f22da52 to dd78ac1 Compare May 15, 2026 17:28
Signed-off-by: Matei Pinzaru <201221140+mateipinzaru-parloa@users.noreply.github.com>
@mateipinzaru-parloa mateipinzaru-parloa force-pushed the fix/acr-identity-token-oci-auth branch from 470e0c7 to 81c8279 Compare May 15, 2026 20:10
Signed-off-by: Matei Pinzaru <201221140+mateipinzaru-parloa@users.noreply.github.com>
@mateipinzaru-parloa mateipinzaru-parloa force-pushed the fix/acr-identity-token-oci-auth branch from 73c8fe2 to bbb6b46 Compare May 15, 2026 22:54
Signed-off-by: Matei Pinzaru <201221140+mateipinzaru-parloa@users.noreply.github.com>
@mateipinzaru-parloa

Copy link
Copy Markdown
Author

Github automation unintentionally closed the PR, reopened it now 🙈.

Please let me know if I'm missing anything to get this reviewed and merged.

Thank you!

@mateipinzaru-parloa mateipinzaru-parloa closed this by deleting the head repository Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OCI chart pulls fail when default keychain credentials return Docker identity tokens

1 participant