Skip to content

Fail fast on 401, and reload the service account token - #9

Closed
maboelnour wants to merge 1 commit into
masterfrom
fix/surface-401-read-token-file
Closed

Fail fast on 401, and reload the service account token#9
maboelnour wants to merge 1 commit into
masterfrom
fix/surface-401-read-token-file

Conversation

@maboelnour

@maboelnour maboelnour commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

What's wrong

After a cluster upgrade our scaltainer pod quietly stopped scaling anything, logging this on every tick:

ERROR -- : Could not find resource with name customizer_by_filter in namespace rayyan-staging: Unauthorized

The deployment exists. The API server returned 401 — it couldn't authenticate us at all. But get_service wraps every exception into "Could not find resource…", so a rejected credential reads like a missing object.

And it never stops. NetworkError is a RuntimeError, and iterate_services rescues RuntimeError so one bad service doesn't stop the rest. Sensible for a missing deployment, wrong for a dead credential: every service fails on every tick, forever, and the process still exits 0. The pod looks healthy from the outside.

Why the credential dies

We read the token once at startup and hand Kubeclient the string:

auth_options = {bearer_token: read_secret(serviceaccount, 'token')}

Since Kubernetes 1.21, in-cluster tokens are short-lived and kubelet rotates the file in place. We keep presenting our boot-time copy while a valid token sits on disk next to us. Clusters have been papering over this with --service-account-extend-token-expiration; when that stopped applying for us, the latent bug became an outage.

The fix

Hand over the path instead of the contents, so Kubeclient can re-read it:

auth_options = {bearer_token_file: token_path(serviceaccount)}

This needs both a code change and a version bump, because neither works alone:

bearer_token: bearer_token_file:
kubeclient 4.9.3 broken (today) broken — read once in the constructor
kubeclient 4.10+ broken — nothing to re-read works — re-read on every request

4.10.0 added a get_headers that re-reads the file per request (#566, closing #561); it doesn't exist in 4.9.3. So the gemspec goes to ~> 4.10 — not just the lockfile. ~> 4.9 would still permit 4.9.3, where bearer_token_file is accepted silently and read only once, quietly reintroducing this exact bug with no error to hint at it. Lockfile pins 4.13.0.

Also: stop swallowing 401

Even with the above, a rejected credential shouldn't look like a missing resource and shouldn't loop forever. Adds Scaltainer::AuthenticationError, converts a 401 into it, and exits 77 so the orchestrator restarts us.

The non-obvious bit: it's a StandardError, not an ApplicationError like everything else here. That's deliberate — if it inherited from ApplicationError it would be a RuntimeError, iterate_services would swallow it, and nothing would change. runner.rb also needs two explicit re-raises, since the bare rescue => e in get_service and scale_out would otherwise relabel it right back.

Only 401. 403 and 404 keep skipping and continuing as before — a restart can't fix those, so crash-looping on them would hide the cause and block the other services. A 401 is always fixed by a restart. Swarm is untouched.

Exit code 77 is just to keep this distinguishable in kubectl describe. Happy to make it plain 1.

Tests

New specs cover the token path being passed rather than its contents, the exception hierarchy (that it isn't a RuntimeError), 401 converting while 403/404 pass through, and all three call sites propagating it while still swallowing ordinary errors.

Notes

  • kubeclient 4.11+ requires Ruby >= 2.7.0. The Dockerfile is FROM ruby:2.7, so this is fine, but worth knowing the floor moved.
  • Gemfile.lock was edited by hand; http stays at 4.4.1, which still satisfies the relaxed >= 3.0, < 6.0. Worth a bundle install to confirm the resolution.

@maboelnour maboelnour changed the title Surface HTTP 401 as fatal instead of swallowing it Fail fast on 401, and reload the service account token Jul 27, 2026
@coveralls

coveralls commented Jul 27, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 79.422% (+6.2%) from 73.181% — fix/surface-401-read-token-file into master

@maboelnour
maboelnour marked this pull request as draft July 27, 2026 19:10
@hammady

hammady commented Jul 27, 2026

Copy link
Copy Markdown
Owner

There are 2 architecturally different approaches in the PR and they can't happen at the same time.

  1. You used bearer_token_path instead of bearer_token which uses a potentially new token on every request so it is inefficient.
  2. You fail fast on 401 so that you get a new token on the next restart

Either pick 1 or 2 but not both!

@maboelnour maboelnour closed this Jul 27, 2026
@maboelnour
maboelnour deleted the fix/surface-401-read-token-file branch July 27, 2026 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants