Skip to content

dockerconfig: fix credential refresh for expired auth cache entries - #4174

Closed
ansh0014 wants to merge 1 commit into
docker:masterfrom
ansh0014:fix/auth-cache-expire
Closed

ansh0014 wants to merge 1 commit into
docker:masterfrom
ansh0014:fix/auth-cache-expire

Conversation

@ansh0014

Copy link
Copy Markdown

What this PR does

Fixes #3977.

During long-running builds with --push or --cache-to against registries with short-lived tokens (such as Google Artifact Registry via the gcloud credential helper), token fetching failed with:
failed to authorize: failed to fetch oauth token: unauthorized: authentication failed

This happens because the in-memory credential cache retained stale credentials past their expiration window.

This change:

  1. Ensures loadHost defaults to the 4m50s expiration threshold even if called with a nil check function.
  2. Evicts expired entries from authConfigCache so credential helpers are re-executed to obtain fresh tokens on renewal.
  3. Adds unit tests covering credential cache hits, expiration, and nil check handling.
  • Signed-off-by according to Developer Certificate of Origin (DCO)

Signed-off-by: Anshul Jagota <anshul.jagota@yahoo.com>
Copilot AI balanced review requested due to automatic review settings October 28, 2026 09:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@crazy-max

Copy link
Copy Markdown
Member

The existing code already reloads credentials when a cache entry expires: it falls through to GetAuthConfig, invokes the configured helper, and replaces the cached entry. Deleting the entry first doesn't enable that refresh. Both production calls to loadHost also already receive a non-nil expiration callback from load.

I ran the added tests against the original code. Only the direct loadHost(nil) test fails, and that call pattern doesn't occur in production. I've added coverage using a credential-helper subprocess and a local token endpoint in #4175, which confirms refresh and recovery work without production changes.

I'm closing this because it doesn't demonstrate a fix for #3977. That issue still needs investigation. A reproduction showing where the existing refresh path fails would help us identify the right change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

3 participants