ci: authenticate runtime tests with OIDC instead of a stored API key - #6798
Conversation
The runtime integration tests drop FELDERA_API_KEY_CI_GKE_AMD64 and FELDERA_API_KEY_CI_GKE_ARM64. The job presents its GitHub OIDC token as the bearer credential, which each CI instance matches against a registered trust relationship, and the auth step confirms the instance accepts it before the suites start. testutils hands the SDK a callable rather than a token string when it runs under Actions. An ID token expires well inside a 50-minute test run, and the SDK re-resolves a callable per request and retries once on 401, so a token that lapses mid-run is replaced instead of failing every request after it. The suites also drop FELDERA_TLS_INSECURE, since the CI instances present publicly trusted certificates. test-integration-platform keeps it, talking to a docker service container over a self-signed certificate, and keeps creating an API key: that path is what covers key creation, against a local instance, so no cluster secret is involved. Signed-off-by: Gerd Zellweger <mail@gerdzellweger.com>
The SDK resolves a callable credential before every request, so the provider as written asked GitHub for a token on every API call. A suite that polls in loops across parallel workers sends enough of those to be throttled, and it arrives as a connection timeout inside the client rather than as anything naming the token endpoint. The token is now held until two minutes before its own expiry, which takes a 500-request run from 500 mints to one. An unreadable payload falls back to minting each time, so a token is never served past its expiry. Signed-off-by: Gerd Zellweger <mail@gerdzellweger.com>
mihaibudiu
left a comment
There was a problem hiding this comment.
I should approve, but these comments are terrible
Review feedback: name the units the cache and the refresh margin are measured in, say plainly what an unreadable payload does rather than referring to previous behaviour, and drop the asides about what a fixed credential cannot do and which credential wins. Signed-off-by: Gerd Zellweger <mail@gerdzellweger.com>
mythical-fred
left a comment
There was a problem hiding this comment.
Approach is sound: request an OIDC token per audience, cache it, re-mint with a 2-minute margin before exp, and pass a callable to the SDK so it re-resolves per request and retries once on 401. Cheaper than minting on every request, safer than trusting a long-lived token across a multi-hour suite.
A couple of small observations, all non-blocking:
_token_expiryreturningtime.time()on parse failure means the cache entry storesexp = now - 120s, so the next call re-mints immediately. That's the safe direction, and worth a one-line comment saying so — otherwise a reader wonders why we cache at all in the failure path.- The cache is module-global with no lock. Under pytest-xdist each worker is a separate process, so that's fine today. If anything ever runs the tests in-process across threads, add a
threading.Lockaround the mint. urllib.request.urlopen(..., timeout=30)is good; the request-ID endpoint is normally sub-second, so a 30s ceiling is generous without being pathological if GitHub's metadata service is degraded.
The workflow changes read cleanly: id-token: write granted at the caller and re-declared on the reusable workflow (a called workflow gets no more than the caller grants — the comment captures exactly the footgun I'd want documented), matrix stops carrying per-runner API-key secret names, and FELDERA_TLS_INSECURE is gone from every step. The action pin (feldera/oidc-auth-action@d1fce411 v1.0.0) uses a full SHA.
CI green on main, mihaibudiu already approved. Ship it.
Replaces the stored API keys the runtime integration tests used with a GitHub OIDC token that each CI instance matches against a registered trust relationship.
Verification
The same action version and the same two CI hosts are already exercised by a scheduled job that authenticates this way and reports the version it reached on each. The runtime suites themselves only run in the merge queue, so this PR is the first exercise of that path.