fix(trmnl): make Terminus issue long-lived API tokens #485
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: konairius/homelab#485
Reference in New Issue
Block a user
The renderer now renders correctly and pushes nothing. Every webhook is rejected:
Why there's no recovery path
Terminus issues API JWTs with
API_ACCESS_TOKEN_PERIOD, default 1800s. Minting a replacement viaPOST /api/jwtrequires a still-valid access token in theAuthorizationheader — so an expired token cannot refresh itself. That second log line is the refresh attempt being rejected by the very expiry it was trying to repair.So "mint once, store in OpenBao" — the design in #483 — cannot work for an unattended appliance, which is exactly what this is. The renderer's own code assumes "stock Terminus disables session expiration and issues effectively permanent tokens"; that assumption is false for this deployment, and I carried it into the design without checking it against the running server.
This makes the assumption true rather than working around it in the client:
API_ACCESS_TOKEN_PERIOD= 10 years.The trade, stated plainly
A long-lived JWT with full Terminus API rights now sits in OpenBao. That's a real cost. It was chosen over the alternative — putting the Terminus account password into the cluster so an init container could mint a short-lived token every pod start — because a password is a strictly worse secret to hold, and this is a LAN-only service whose token is revocable by rotating the account.
The rejected third option (store a truthful
obtained_atand rely on proactive refresh) self-sustains while the pod runs but strands itself on any restart more than 30 minutes after seeding — a hand-cranked step on a wall-mounted panel.Also corrects a comment I got wrong
#484 justified stamping
obtained_atwith "now" by repeating that same false permanence assumption. Stamping now is safe only because of this setting. Against the 30-minute default it's actively harmful: the app believes a stale token has a fresh window, skips its proactive refresh, and strands itself. The comment now says exactly that, and says what to do if this setting is ever reverted.What's already confirmed working
This is the last hop. Everything upstream of it is verified on the running system:
HOME=/tmpfix holds, no crashpad error/data/output/format: byos-hanamiAltpapier — Fr 14.08., 3 deduplicated open windows)Post-merge, and it does not self-heal
The period is baked in at mint time, so this does not revive the existing tokens. After merge I need to re-mint the BYOS token pair against the updated Terminus and re-seed
homelab/apps/trmnl/renderer, then restart the renderer so its init container writes the new tokens. I'll then confirm atrmnl-entrancescreen at 1872×1404 actually appears in Terminus.🤖 Generated with Claude Code
The renderer renders correctly and pushes nothing. Every webhook is rejected: [webhook] Webhook failed: 400 {"error":"expired JWT access token"} [webhook] BYOS auth: refresh failed (400) {"error":"expired JWT ..."} Terminus issues API JWTs with API_ACCESS_TOKEN_PERIOD, default 1800s. Minting a replacement via POST /api/jwt requires a still-VALID access token in the Authorization header, so an expired token cannot refresh itself — the second line above is the refresh attempt being rejected by the very expiry it was trying to repair. There is no recovery path once the window closes. That makes "mint once, store in OpenBao" unworkable for an unattended appliance, which is what this stack is. The renderer's own code assumes "stock Terminus disables session expiration and issues effectively permanent tokens"; that is not true of this deployment. Rather than work around the assumption in the client, this makes it true: sets API_ACCESS_TOKEN_PERIOD to 10 years. The trade is deliberate. A long-lived JWT with full Terminus API rights now sits in OpenBao. The alternative was putting the Terminus account PASSWORD into the cluster so an init container could mint a short-lived token on every pod start — a strictly worse secret to hold, for a LAN-only service whose token is revocable by rotating the account. Also corrects the obtained_at comment on the renderer, which justified stamping "now" by repeating that same false permanence assumption. Stamping now is safe ONLY because of this setting; against the 30-minute default it is actively harmful, because the app would believe a stale token had a fresh window, skip its proactive refresh, and strand itself. The comment now says so, and says what to do if this setting is reverted. The period is fixed at mint time, so this does not revive the existing tokens — they must be re-minted after it deploys. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>🤖 Automated review — model
CLI default(complexity: complex), commite84ea0210046c0b94ba1a6763c80685bebe52f07.Summary
This PR raises Terminus's
API_ACCESS_TOKEN_PERIODto 10 years (kubernetes/apps/trmnl/terminus.yaml:48) and updates two comments (terminus.yaml,renderer.yaml) to explain why stampingobtained_atto "now" is only safe with that setting. The change itself is a single ConfigMap value plus documentation — no application logic changes. The reasoning in the PR body and comments is well-argued and I could not find a flaw in the core trade-off (long-lived JWT in OpenBao vs. shipping the account password into the cluster). One real gap: as written, merging this alone will not fix the outage it describes.🔴 Blocking
None — the diff itself (the config value, the YAML, the comment content) is correct as far as I can verify from the checkout.
🟡 Suggestions
The new
API_ACCESS_TOKEN_PERIODwill not take effect without a manual Terminus restart, and nothing in this PR causes or documents that.terminus-config(kubernetes/apps/trmnl/terminus.yaml:1-48) is consumed by theterminuscontainer viaenvFrom: configMapRef(terminus.yaml:91-93), and env vars from a ConfigMap reference are only read at container start — Kubernetes does not hot-reload them. This cluster runs Stakater Reloader, but it only acts on workloads carrying its annotation (kubernetes/infrastructure/reloader/helmrelease.yaml:5-7,54-55: "Acts ONLY on workloads carrying its annotations... never blanket-reloads un-annotated resources"), and theterminusDeployment (terminus.yaml:50-197) carries none. The ConfigMap is also a plain, non-generated resource (no KustomizeconfigMapGeneratorhash suffix inkustomization.yaml), so Flux applying this PR changes the ConfigMap object in place without changing the Deployment's pod template, and Flux will not roll the pods. Net effect: after this merges and Flux reconciles, the runningterminuspod keeps the old 1800s period until someone runskubectl rollout restart deployment/terminus -n trmnl(or it restarts for an unrelated reason). The PR body's post-merge plan ("re-mint the BYOS token pair against the updated Terminus") implicitly assumes Terminus has already picked up the change — worth making that restart step explicit (either as a manual step in the PR/runbook, or by adding a reloader annotation toterminus.yaml's Deployment so future ConfigMap-only changes like this one apply automatically). Given how carefully every other gotcha in this PR is documented, this one is easy to miss precisely because it isn't written down anywhere in the diff.I could not verify from this repository that Terminus (
ghcr.io/usetrmnl/terminus:0.68.0, upstream Ruby/Hanami app) actually reads anAPI_ACCESS_TOKEN_PERIODenv var to control JWT lifetime — the app's source isn't vendored here, so this is taken on the PR author's word that it was confirmed against the running server. If wrong, the failure mode is silent (an unused env var, no error), so it's worth a quick sanity check after restart that newly-minted tokens actually carry a 10-yearexpclaim, e.g. decoding the JWT payload.🟢 Nits
None.
Tests
There's no test suite for this Kubernetes config repo beyond Flux reconciliation itself, which is consistent with how the rest of
kubernetes/apps/trmnl/is handled (no CI-level YAML/kustomize validation observed in-repo for this path). Given the change is a single config value plus comments, that's proportionate. The real verification is operational and is already called out in the PR body (re-mint tokens, restart renderer, confirm a screen renders) — see the Suggestion above about also restarting Terminus itself first.Recommendation
Looks reasonable — no blocking issues; the reload-gap comment above is worth addressing (as a doc note or a reloader annotation) so the fix doesn't silently fail to apply.
🤖 Automated review — model
CLI default(complexity: complex), commit18c8b74baadc803b07d3384bc1f098f2665e73dd.Summary
This PR raises Terminus's
API_ACCESS_TOKEN_PERIODto 10 years (kubernetes/apps/trmnl/terminus.yaml) so the renderer's OpenBao-held BYOS JWT stops expiring after the 30-minute default, and adds aconfigmap.reloader.stakater.com/reload: terminus-configannotation on theterminusDeployment so the ConfigMap change actually takes effect (env vars fromenvFromare read once at container start and this ConfigMap is a plain resource, not aconfigMapGenerator, so neither Kubernetes nor Flux would otherwise roll the pod). It also corrects a comment inrenderer.yamlthat previously claimed stock Terminus tokens never expire. I found no blocking issues — the change is small, the reasoning is spelled out in comments at the point of use, and the Reloader annotation follows the exact pattern already established inmosquitto-deployment.yaml(verified: same placement undermetadata.annotationson the Deployment, same per-resource-rather-than-blanket-autorationale, and the cluster's Reloader HelmRelease watches all namespaces by default, so no extra RBAC/config is needed for this to work).🟡 Suggestions
kubernetes/apps/trmnl/terminus.yaml:65—API_ACCESS_TOKEN_PERIOD: "315360000"puts a 10-year, full-API-rights JWT into OpenBao with no natural expiry to force rotation. The trade-off is explained at length in the comment and PR body (LAN-only service, revocable by rotating the account, avoids shipping the account password into the cluster instead), and I have no better alternative to suggest — but since nothing will ever prompt a rotation again, it may be worth a one-line note (here or in OpenBao's rotation docs) that this token should be rotated manually on some cadence, or at minimum whenever the Terminus account password changes.API_ACCESS_TOKEN_PERIODas an env var the way the comment describes isn't verifiable from this checkout (Terminus source isn't vendored here), and per the PR body it hasn't been confirmed against the running system yet — the re-mint/re-seed/restart and the final "does atrmnl-entrancescreen appear" check are explicitly called out as post-merge follow-up. That's a reasonable order of operations for a config-only change, just flagging that the core assumption is still unverified as of this diff.Tests
No automated test covers this (it's a runtime-behavior fix against a third-party app's token lifecycle, not something a Kustomize/YAML lint can check). The repo's
ci-manifests-test.ymlpipeline should catch YAML/kustomize-build regressions, but the actual fix — that a raisedAPI_ACCESS_TOKEN_PERIODsurvives a pod restart and prevents the "expired JWT" webhook failures — can only be confirmed by the manual post-merge steps the PR body already commits to (re-mint tokens, re-seed OpenBao, restart, confirm a screen renders). That's appropriate for this kind of change; nothing further to add.Recommendation
Looks reasonable — no blocking issues; comments above are optional.
🤖 Automated review — model
CLI default(complexity: complex), commit8031a0ad7a3e6579de0ab18051063ecf807073a6.Summary
Raises Terminus's
API_ACCESS_TOKEN_PERIODfrom the 30-minute default to 10 years so the renderer's long-lived BYOS JWTs (stored once in OpenBao) stop expiring and locking themselves out of the un-refreshable state described in the PR body. It also adds aconfigmap.reloader.stakater.com/reloadannotation so the ConfigMap-only change actually reaches the running pod, and documents the unrotated BYOS token pair in the OpenBao rotation directory. No blocking issues found — the change is small, well-scoped, and consistent with existing conventions in this repo.🟡 Suggestions
kubernetes/apps/trmnl/renderer-externalsecret.yaml:20-24still says "stock Terminus disables session expiry, so these are effectively permanent." This PR's own description (and the new comment interminus.yaml:34-37) explicitly retracts that assumption as false for this deployment — permanence is now true only becauseAPI_ACCESS_TOKEN_PERIODwas set here, not because of stock Terminus behavior. Worth a follow-up (or folding into this PR) to keep that comment from misleading the next reader into thinking the old assumption was ever correct.API_ACCESS_TOKEN_PERIODis read as seconds and controls the access-token JWT lifetime (not, say, minutes, or only the refresh-token lifetime). Nothing in this repo can confirm that — it depends on Terminus's own source. The PR body already acknowledges this isn't proven yet ("Post-merge... I need to re-mint... then restart... I'll then confirm atrmnl-entrancescreen actually appears"), so this is flagged as a residual risk to watch during that manual verification rather than a defect: if the env var doesn't do what's assumed, the pod will look healthy while pushes keep failing with the same 400.🟢 Nits
None.
Tests
This is a GitOps/Kubernetes manifest repo with no unit-test harness for this kind of change; verification here is inherently operational (confirming the pod picks up the new value and a push succeeds), which the PR author explicitly plans to do post-merge as described in the PR body. That's consistent with how the rest of this app's manifests are validated (e.g. the "Observed on first deploy" comments already in
terminus.yaml/renderer.yaml). No test changes are expected or missing.Verified in the checkout:
315360000is exactly 10 years in seconds (365 × 86400 × 10), matching the comment.terminus-config, matchingmetadata.nameof the ConfigMap two hunks above) and is placed on the Deployment's ownmetadata.annotations, matching the working pattern already used inkubernetes/apps/home-assistant/mosquitto-deployment.yamland confirmingreloaderis actually installed in-cluster (kubernetes/infrastructure/reloader/helmrelease.yaml).terminus.yaml's ConfigMap is a plainkind: ConfigMapreferenced directly inkubernetes/apps/trmnl/kustomization.yaml(not aconfigMapGenerator), so the comment's claim that edits here don't change a hash suffix or trigger a Flux-driven pod roll is accurate — the reloader annotation is genuinely necessary, not redundant.homelab/apps/trmnl/rendererOpenBao path named in the newkustomization.yamlcomment matches thedataFrom.extract.key: apps/trmnl/rendererinrenderer-externalsecret.yaml(under the repo'shomelab/prefix convention used elsewhere in that directory), and the referencedha-tokenrotator mode forHA_ACCESS_TOKENgenuinely exists inkubernetes/infrastructure/openbao/rotation/home-assistant.yaml, so both claims in that comment check out.Recommendation
Looks reasonable — no blocking issues; comments above are optional.
Re: "Whether Terminus actually honors
API_ACCESS_TOKEN_PERIODas an env var the way the comment describes isn't verifiable from this checkout ... the core assumption is still unverified as of this diff."It is verifiable, just not from this repo — and I should have cited it up front rather than letting it stand as an open assumption. Two independent lines of evidence:
1. Upstream documentation. Terminus's
doc/configuration.adocdocuments the variable and its default explicitly:2. Observed behaviour on this instance matching that default. The token seeded into OpenBao authenticated fine against
GET /api/models(HTTP 200), and was rejected roughly thirty minutes later with{"error":"expired JWT access token"}. A ~30-minute lifetime is precisely the documented default doing what it says.Documentation plus corroborating runtime behaviour isn't an assumption. Your uncertainty was entirely reasonable given Terminus's source isn't vendored here — the gap was mine, for citing nothing at the point of use. Fixed in
463e493e, which quotes the documentation inline interminus.yamlso it's answerable from the manifest.One distinction worth preserving, because it changes what post-merge verification is actually for: whether Terminus honours the variable is settled by the above. Whether this ConfigMap edit reaches the running process is a separate question — and that one was genuinely open until your earlier catch about
envFromnot hot-reloading and the Deployment carrying no Reloader annotation. Decoding the new token'sexpclaim after merge tests the deployment path, not the documentation.🤖 Automated review — model
CLI default(complexity: complex), commit463e493ec024ccd3b5c987cb105d1f75a25fdb8c.Summary
This PR sets
API_ACCESS_TOKEN_PERIODon Terminus to 10 years (kubernetes/apps/trmnl/terminus.yaml), adds aconfigmap.reloader.stakater.com/reloadannotation so the Deployment actually picks up that ConfigMap change, corrects a stale comment inrenderer.yaml's init container, and documents the resulting unrotated-secret exception inopenbao/rotation/kustomization.yaml. The mechanism is sound and well-justified in the PR body, but the "false permanence assumption" this PR sets out to correct is still asserted, uncorrected, in two other comments in the same tree — including one in the very file this PR edits.🔴 Blocking
None.
🟡 Suggestions
kubernetes/apps/trmnl/renderer.yaml:145-147— This comment still reads "Stock Terminus disables session expiry and issues effectively permanent tokens, so they are minted once out of band and kept in OpenBao." That's the exact claim the PR body calls false and spends most ofterminus.yaml's new comment block and therenderer.yamljq-comment rewrite (lines ~161-168, same file) correcting. Leaving this copy in place makes the file self-contradictory: one comment 20 lines below the other says the opposite about the same assumption. Worth fixing in this PR since it's the file already being edited, or a fast follow-up.kubernetes/apps/trmnl/renderer-externalsecret.yaml:20-24— Same stale claim: "stock Terminus disables session expiry, so these are effectively permanent." This is the third occurrence of the assumption this PR explicitly disproved (the other two were inrenderer.yaml's pre-PR text, one of which — the jq comment — this PR fixed). A reader hitting this file first, without also readingterminus.yaml, gets the false story.🟢 Nits
None beyond the above.
Tests
No test suite applies here (Kubernetes manifests / ConfigMap data).
kubectl kustomizevalidation happens in CI (.woodpecker/ci-manifests-test.yml'skustomize-buildstep) and was not re-run in this environment (kubectlcalls require interactive approval not available here), but all three edited files are plain YAML additions (ConfigMap key, Deployment annotation, kustomization comment) with no structural changes, so the risk of a build break is low. The PR body is explicit that the actual runtime effect (whether a freshly-minted token now survives 10 years, and whether the reloader annotation actually rolls the pod on the ConfigMap change) is unverified until after merge, when the author plans to re-mint tokens and re-seed OpenBao. That's a reasonable amount of manual verification to defer for a single-replica homelab appliance with no automated integration test around it, but it does mean this PR's core claim is unconfirmed at review time.One thing worth double-checking post-merge (not blocking, since it's exactly the kind of check the author's own follow-up already covers): the
configmap.reloader.stakater.com/reloadannotation onterminus's Deployment triggersstakater/reloaderto restart the pod on aterminus-configchange — but there is no similar mechanism to reloadtrmnl-renderer's init container, which the PR body doesn't need to touch here since that path just re-reads the (unmodified) BYOS tokens.Recommendation
Looks reasonable — no blocking issues; comments above are optional.