Skip to content

Handle non-fatal httprc errors in JWKS registration - #6221

Merged
danbarr merged 1 commit into
mainfrom
fix-jwks-errnotready
Aug 6, 2026
Merged

Handle non-fatal httprc errors in JWKS registration#6221
danbarr merged 1 commit into
mainfrom
fix-jwks-errnotready

Conversation

@danbarr

@danbarr danbarr commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

ensureJWKSRegistered treats every Register error as fatal and leaves the registration flag unset for retry. But httprc returns ErrNotReady when the resource was successfully registered and only the first fetch hasn't completed; the resource is already in httprc's map, so every retry then fails with ErrResourceAlreadyExists. One slow or failed first JWKS fetch (a slow IdP, a network blip on the first authenticated request, or the client bypass fixed in #6220) permanently disabled token validation for the life of the process.

  • Treat ErrNotReady as registered-but-pending: the validator proceeds and Lookup surfaces a not-ready key set until a background fetch succeeds, so transient first-fetch failures now self-heal.
  • Treat ErrResourceAlreadyExists as already-registered. Besides being the escape hatch for the retry trap, this also covers OIDC re-discovery resolving to the same JWKS URL after the registration flag is reset.
  • New tests cover both sentinels; each fails against the previous code with the exact error strings from the issue.

Fixes #6218

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Test plan

  • Unit tests pass (pkg/auth/... with race detection; new tests verified to fail pre-fix)
  • task lint passes

Does this introduce a user-facing change?

Transient failures fetching an external OIDC issuer's JWKS on first use no longer permanently disable token validation until restart; validation recovers once a background fetch succeeds.

Special notes for reviewers

Stacked on #6220, which fixed the trigger that surfaced this; this PR hardens the retry path against any first-fetch failure. Review only this layer's diff via the stacked PR view.

Generated with Claude Code

@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Aug 6, 2026
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 72.50%. Comparing base (e7a995b) to head (e5fe29f).

Files with missing lines Patch % Lines
pkg/auth/token.go 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6221      +/-   ##
==========================================
+ Coverage   72.47%   72.50%   +0.02%     
==========================================
  Files         739      739              
  Lines       76719    76728       +9     
==========================================
+ Hits        55600    55628      +28     
+ Misses      17152    17121      -31     
- Partials     3967     3979      +12     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

stack merge was automatically disabled August 6, 2026 02:01

Pull Request is not mergeable

Base automatically changed from fix-jwks-resource-client to main August 6, 2026 02:04
@danbarr
danbarr force-pushed the fix-jwks-errnotready branch from 2701a9b to b734433 Compare August 6, 2026 02:04
@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Aug 6, 2026
ensureJWKSRegistered treated every Register error as fatal and left
jwksRegistered unset so the next call retries. But httprc returns
ErrNotReady when the resource was successfully registered and only
the first fetch has not completed; the resource is already in
httprc's map, so every retry then fails with
ErrResourceAlreadyExists. One slow or failed first fetch permanently
disabled the validator for the life of the process.

Treat ErrNotReady as registered-but-pending so Lookup surfaces a
not-ready key set until a background fetch succeeds, and treat
ErrResourceAlreadyExists as already-registered. The latter also
covers OIDC re-discovery resolving to the same JWKS URL after the
registration flag is reset.

Fixes #6218

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@danbarr
danbarr force-pushed the fix-jwks-errnotready branch from b734433 to e5fe29f Compare August 6, 2026 02:05
@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Aug 6, 2026
@danbarr
danbarr merged commit 8863eb0 into main Aug 6, 2026
47 checks passed
@danbarr
danbarr deleted the fix-jwks-errnotready branch August 6, 2026 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TokenValidator JWKS registration deadlocks permanently after the first ErrNotReady

2 participants