Skip to content

auth/utils: ensure registry credentials are always fresh#1244

Merged
matheuscscp merged 1 commit into
mainfrom
improve-registry-auth
Jun 13, 2026
Merged

auth/utils: ensure registry credentials are always fresh#1244
matheuscscp merged 1 commit into
mainfrom
improve-registry-auth

Conversation

@matheuscscp

Copy link
Copy Markdown
Member

This PR fixes a bug (not a vuln!), but I have never seen it happen, there are no user reports that I'm aware of.

Long-running operations using artifact registry credentials for multiple OCI/HTTP calls can hit 401, since credentials are statically resolved by auth.GetArtifactRegistryCredentials(). That function returns a user+pass pair immediately (due to how auth/{aws|azure|gcp} are implemented for ECR/ACR/GAR). In other words, the Authorize() method of the returned authn.Authenticator will return a cached user+pass pair that never changes. This fix delays the call to auth.GetArtifactRegistryCredentials() to happen on demand inside authn.Authenticator.Authorize(), which ensures that multiple calls will always return a fresh credential. This is because auth.GetArtifactRegistryCredentials() has a smart cache inside that keeps track of the credential lifetime. This makes it never return an expired credential, as a new one is created when the cached one is expired (the cache knows when).

This fix is super good for flux-mirror sync, which calls auth/utills.GetArtifactRegistryCredentials() (not the low-level one from auth) for ECR/ACR/GAR only once during the process startup and hands the authn.Authenticator off to go-containerregistry. Before this fix, go-containerregistry would call Authorize() again internally upon a 401 and get the same credentials acquired during process startup. This PR causes Authorize() to return valid creds on every call.

I don't think SC or IRC would ever hit this bug, since their operations are not super long. The reconcilers just perform a handful of calls, and no OCI artifact is large enough that it takes 2-digit minutes to reconcile. But this PR fixes the whole thing anyway.

@matheuscscp matheuscscp requested a review from stefanprodan June 12, 2026 22:43
@matheuscscp matheuscscp requested a review from a team as a code owner June 12, 2026 22:43
@matheuscscp matheuscscp added bug Something isn't working area/oci OCI related issues and pull requests area/security Security related issues and pull requests labels Jun 12, 2026
@matheuscscp

matheuscscp commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

Note: We already do this for other operations that are known to be long-running, like reconciling YAML on remote clusters:

// Build wrapped *rest.Config that will call
// auth.GetRESTConfig for every HTTP request.
restConfig := &rest.Config{
Host: conf.Host,
TLSClientConfig: rest.TLSClientConfig{CAData: conf.CAData},
}
restConfig.Wrap(func(base http.RoundTripper) http.RoundTripper {
return &restConfigRoundTripper{
base: base,
provider: provider,
opts: opts,
}
})
return restConfig, nil
}
// restConfigRoundTripper is an http.RoundTripper that wraps the base
// RoundTripper and retrieves a bearer token for the remote cluster
// using auth.GetRESTConfig before each HTTP request.
type restConfigRoundTripper struct {
base http.RoundTripper
provider auth.RESTConfigProvider
opts []auth.Option
}
// RoundTrip implements http.RoundTripper.
func (r *restConfigRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
details, err := auth.GetRESTConfig(req.Context(), r.provider, r.opts...)
if err != nil {
return nil, err
}
req.Header.Set("Authorization", "Bearer "+details.BearerToken)
return r.base.RoundTrip(req)
}

@matheuscscp

matheuscscp commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

Signed-off-by: Matheus Pimenta <matheuscscp@gmail.com>
@matheuscscp matheuscscp force-pushed the improve-registry-auth branch from cd04af0 to bf1b9c7 Compare June 12, 2026 22:58
@matheuscscp

matheuscscp commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

I have deployed this change through a source-controller release candidate in my EKS clusters (where all my sources are OCIRepository) and they are all healthy. Will keep it there until I can switch to the 2.9 release.

@matheuscscp matheuscscp merged commit 84ae38c into main Jun 13, 2026
14 checks passed
@matheuscscp matheuscscp deleted the improve-registry-auth branch June 13, 2026 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/oci OCI related issues and pull requests area/security Security related issues and pull requests bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants