Conversation
Test Impact PlanDeterministic summary of how this PR changes tests, CI runners, and coverage-risk signals. Summary
Signals
Coverage risk: neutral or increased No coverage-reduction warnings detected. |
bill-ph
left a comment
There was a problem hiding this comment.
Reviewed 49fe6d8. No actionable findings or P0 blocker identified.
The shared request path now uses Basic authentication when an API username is configured while retaining the separate capability header. An unset username preserves the existing Bearer behavior. Username validation rejects ambiguous Basic-auth separators and control characters; the default transport still requires HTTPS, verifies TLS, and refuses redirects. The new tests exercise both authentication layers, backend operations, invalid usernames, and redirect refusal.
Overengineering/scope-creep check: the optional username reuses existing configuration and credentials without adding an authorization exception or a second client implementation. Tests and documentation are proportional to the change.
Validation: static review of the full diff and surrounding client construction/request handling; no local tests or live Gateway acceptance run. Coordinate deployment with charts #15954 and verify active-pool acceptance after the new image and username configuration are deployed.
— Robo Bill
Summary
A form-authenticated Gateway requires an API-role identity before checking the pool administration capability. The pool client previously sent an opaque token as Bearer authentication, which the outer form-session authenticator rejected with HTTP 403.
APIUsernameand wire it from existingDUCKGRES_TRINO_MANAGED_GATEWAY_USERNAME.X-Gateway-Transaction-Admin-Tokenfor the separate capability check.Validation
just lintpass.Rollout
Merge this source PR first and let its image publish. Then merge charts #15954, which supplies the existing API username to the pool operator. This PR does not claim real-cluster acceptance has passed.