Skip to content

Give the autoscale guard an org-scoped metrics token - #562

Open
Joe2k wants to merge 1 commit into
mainfrom
fix-autoscale-guard-token
Open

Give the autoscale guard an org-scoped metrics token#562
Joe2k wants to merge 1 commit into
mainfrom
fix-autoscale-guard-token

Conversation

@Joe2k

@Joe2k Joe2k commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Every scheduled Autoscale Guard run since it landed has failed:

urllib.error.HTTPError: HTTP Error 403: Forbidden

Cause

FLY_API_TOKEN (created 2023-07-06) is the app-scoped deploy token. It authenticates correctly — which is why the failure is a 403 and not a 401 — but the org-level Prometheus endpoint api.fly.io/prometheus/upai/ is outside its scope.

Verified: a deliberately bogus token returns 401 with something went wrong resolving organization, while a personal org token returns 200 against the identical query. So this is an authorisation gap, not a bad credential.

Change

The guard now reads FLY_METRICS_TOKEN. The deploy token stays app-scoped rather than being widened, so the blast radius of the more widely used secret is unchanged.

HTTP errors now report status and body instead of a urllib traceback — the traceback didn't distinguish "wrong token" from "wrong scope", which is the whole question here.

error: metrics API returned 403. A deploy token cannot read org metrics --
set FLY_METRICS_TOKEN to a read-only org token
(fly tokens create readonly -o upai). <body>

Required before this works

This PR alone does not fix the failures — the secret has to exist:

fly tokens create readonly -o upai
gh secret set FLY_METRICS_TOKEN   # paste the token

Until then the guard fails with neither FLY_METRICS_TOKEN nor FLY_API_TOKEN is set, which is at least the actionable version of the message.

Note the org already has a Read-only org token (expires 2046). If its value was saved somewhere, it can be reused instead of minting a new one.

Testing

./scripts/lint passes. Ran locally against the live API: a valid token reports healthy: 2h peak 200000, 15m min 199948; an invalid one exits 1 with the status and body and no traceback.

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings August 29, 2026 20:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes Autoscale Guard’s inability to query org-level Fly Prometheus metrics by switching authentication from the app-scoped deploy token (FLY_API_TOKEN) to an org-scoped metrics token (FLY_METRICS_TOKEN), and improves HTTP error reporting to make authorization failures actionable.

Changes:

  • Prefer FLY_METRICS_TOKEN (with fallback to FLY_API_TOKEN) when querying the Fly Prometheus API.
  • Add HTTPError handling that surfaces status code + response body (and provides a specific hint for 403 deploy-token scope issues).
  • Update the Autoscale Guard GitHub Actions workflow to pass FLY_METRICS_TOKEN instead of FLY_API_TOKEN.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
scripts/check-scale.py Switch token selection to metrics-first and improve HTTP error handling for Prometheus queries.
.github/workflows/autoscale-guard.yml Pass the new org-scoped metrics secret into the Autoscale Guard run.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/check-scale.py Outdated
Comment on lines +46 to +55
detail = exc.read().decode(errors="replace").strip()[:200]
hint = ""
if exc.code == HTTP_FORBIDDEN:
# A deploy token authenticates but is scoped to one app, so it cannot
# read org-level metrics. A bad token gives 401, not 403.
hint = (
f" A deploy token cannot read org metrics -- set FLY_METRICS_TOKEN to"
f" a read-only org token (fly tokens create readonly -o {ORG})."
)
sys.exit(f"error: metrics API returned {exc.code}.{hint} {detail}")
Comment thread scripts/check-scale.py
Comment on lines +81 to +84
# Prefer a metrics-scoped token; FLY_API_TOKEN is the app-scoped deploy token.
token = os.environ.get("FLY_METRICS_TOKEN") or os.environ.get("FLY_API_TOKEN")
if not token:
sys.exit("error: FLY_API_TOKEN is not set")
sys.exit("error: neither FLY_METRICS_TOKEN nor FLY_API_TOKEN is set")
@Joe2k
Joe2k force-pushed the fix-autoscale-guard-token branch from 50d44b2 to acacc75 Compare September 3, 2026 01:52
@Joe2k

Joe2k commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed both review comments and added tests.

exc.read() unbounded / trailing space — the read is now capped at MAX_ERROR_BODY (2048 bytes), and the body is only appended when non-empty:

message = f"error: metrics API returned {code}.{hint}"
detail = body.decode(errors="replace").strip()
return f"{message} {detail}" if detail else message

Covered by test_empty_body_leaves_no_trailing_space.

Stale docstring — now names FLY_METRICS_TOKEN and says why the fallback isn't sufficient:

Needs FLY_METRICS_TOKEN, which must be org-scoped; the app-scoped FLY_API_TOKEN is accepted but gets a 403.

Tests

Pulled the decision and the error message out into decide() and http_error_message() so they're testable without the network. 14 tests in server/tests/test_check_scale.py, collected by pytest the way CI runs it.

The two that carry the design:

def test_escalates_on_the_2026_08_28_drain(self):
    # 2h window peaked at 56,010, five hours before the first 504
    escalate, reason = check_scale.decide("shared", 56010.0, 41134.0)
    self.assertTrue(escalate)

def test_does_not_escalate_on_a_deploy_dip(self):
    # bottomed at 9,983 but recovered inside 90 min, so the window
    # still holds a healthy reading. Depth alone would have fired.
    escalate, _ = check_scale.decide("shared", 100000.0, 9983.0)
    self.assertFalse(escalate)

Plus: no escalation on a performance profile or with missing samples, the floor backstop, $GITHUB_OUTPUT formatting (escalate=true lowercase — GitHub compares strings), the 403-vs-other-code error paths, main() exiting cleanly with no token, and a guard on the fly.toml field path that decides whether the guard runs at all.

Env access is isolated with mock.patch.dict so nothing leaks between tests under pytest.

Full suite: 497 passed, 1 pre-existing failure (TOPSCORE_CLIENT_ID, unset locally, set in CI), 2 Integration deselected. ./scripts/lint clean.

Still needs the secret before the guard actually runs:

fly tokens create readonly -o upai
gh secret set FLY_METRICS_TOKEN

Every scheduled run since the guard landed has failed with a 403 from the Fly
metrics API. FLY_API_TOKEN is the app-scoped deploy token created in 2023: it
authenticates fine, which is why the failure is 403 and not the 401 a bad token
gets, but org-level Prometheus is out of its scope.

The guard now reads FLY_METRICS_TOKEN, which needs a read-only org token:

    fly tokens create readonly -o upai

Leaving the deploy token app-scoped rather than widening it keeps the blast
radius of the more widely used secret unchanged.

HTTP errors report the status and body instead of surfacing a urllib
traceback, since the traceback said nothing about which of the two plausible
causes it was. The body is read with a cap and only appended when non-empty.

fly.toml is read with a regex rather than tomllib. The workflow runs this on a
bare runner with no install, and pyproject supports Python 3.10, which has no
tomllib -- so importing it broke collection for the whole test suite on CI.

The decision and the error message are separate functions so they can be
tested without the network, covered by server/tests/test_check_scale.py. The
cases that matter are the two the design turns on: the 2026-08-28 drain, whose
2h peak of 56,010 escalates; and a deploy dip, which bottoms at 9,983 and does
not, because the window still holds a healthy reading.
@Joe2k
Joe2k force-pushed the fix-autoscale-guard-token branch from acacc75 to 56b7274 Compare September 3, 2026 07:15
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.

2 participants