Draft: Auth: classify token-fetch failures and retry the exchange once
<
Fable 5.1, acting for @armin.pasalic.
What does this MR do and why?
Second of three MRs for #1190 (closed), cut from the final design after the review of the earlier stack. It targets !1334 (closed)'s branch and retargets to master when that merges.
A failure of the OAuth token exchange during the /auth callback used to collapse into one error value and a bare 503. This MR:
- returns a typed token-fetch error that keeps the status, the OAuth
error/error_descriptionbody and the transport cause, with URLs redacted; - retries the exchange once in place, after 250 ms, for transport errors other than timeouts, upstream 5xx and
invalid_grant. GitLab answers a grant that a lagging replica has not seen yet with the sameinvalid_grantas a spent code, and a dropped connection leaves the code unspent, so a second attempt with the same code succeeds in both cases; a spent code fails the retry within milliseconds; - counts every attempt on
gitlab_pages_auth_token_fetch_failures_total{reason,outcome}and a success after a failure ongitlab_pages_auth_token_fetch_recoveries_total{via}, logging only terminal failures at error level; - declares
Content-Typeon the token request and carries the correlation ID, so a failure in Pages' logs can be joined to the GitLab request.
A terminal failure still serves the existing 503 in this MR. The next MR replaces that with a bounded restart and a failure page at the requested page.
How this relates to the earlier stack
The final tree of !1309 (closed), !1335 (closed), !1336 (closed), !1337 (closed) and !1338 (closed) is split into this MR and the next, without the intermediate designs those MRs carried (the session restart marker and the unconditional restart). The upstream fix for the replica-lag case, gitlab!257390 (merged), is rolling out behind feature flags; the in-place retry stays as a safety net for dropped connections and upstream 5xx.
How to validate
- Unit:
go test ./internal/auth/ - Acceptance:
make acceptance ARGS='-run "TestSignIn|TestAccessControlWithPKCE"'(the sign-in scenarios run against the stub's contract mode from !1334 (closed)) - Manually, with access control enabled and a token endpoint that fails once (for example a proxy that drops the first
POST /oauth/token): sign in to a private page and expect it served after one retry, withgitlab_pages_auth_token_fetch_recoveries_total{via="retry"}incremented.
TODO
- This change does not require a feature flag: it only changes the error path of the token exchange, and the retry is bounded to one attempt.
- I added the
Changelogtrailer to the commits that need to be included in the changelog. - I added unit tests.
- I added acceptance tests.
- Documentation: a troubleshooting note on the new log fields and counters follows once the last MR merges.
- I followed the code review and Go style guidelines.
</