Merge remote-tracking branch 'origin/main' into feat/oauth2-proxy-auth-failure-visible
# Conflicts: # runbooks/kustomization.yaml
This commit is contained in:
jakman85 committed
commit
ba79a1b59b
30 files changed
+3515
-17
No files matched your search
@@ -0,0 +1,273 @@
|
||||
---
|
||||
date: 2026-09-30
|
||||
branch: fix/jackett-oauth2-proxy-session
|
||||
title: Jackett — radarr's oauth2-proxy deletes jackett's session cookie; the fix cannot be made per-app
|
||||
---
|
||||
|
||||
## What changed
|
||||
|
||||
Reported as *"GET requests work, but most PUT/POST requests to add indexers fail. Reloading the page
|
||||
never fixes it, and I am never re-prompted for login if it is a login issue."*
|
||||
|
||||
**This branch lands the diagnosis and a runbook. It does NOT land the manifest fix, and that is a
|
||||
deliberate stop, not an omission** — see *The fix has one home and it is shared*, below.
|
||||
|
||||
Three mechanisms, all measured. **Weighting matters here, so it is stated up front:** mechanisms 2
|
||||
and 3 are demonstrated from Jackett's own access log and **between them fully explain the report as
|
||||
written** — PUT/POST fail, the operator is never re-prompted, and a reload does not help. Mechanism 1
|
||||
is a real, firing defect that Jackett genuinely violates, and it explains the *intermittency* on top;
|
||||
I demonstrate the mechanism and one three-second causal adjacency, not a population of them. Fix all
|
||||
three; do not rest the case on the first.
|
||||
|
||||
**1. Cookie collision — the mechanism is firing, and Jackett is a genuine violator.** Six oauth2-proxy
|
||||
instances (seven pods) emit oauth2-proxy's **default** cookie name `_oauth2_proxy` scoped to
|
||||
`.home.keithjackson.dev` while holding seven **different** cookie-secrets. The ping-pong is in two
|
||||
pods' own logs, interleaved to the second:
|
||||
|
||||
15:23:17 radarr [AuthSuccess] -> radarr writes _oauth2_proxy
|
||||
15:23:44 jackett "cookie signature not valid, removing session" -> re-login #1
|
||||
15:23:58 radarr "cookie signature not valid, removing session" -> radarr DELETES jackett's
|
||||
15:27:17 jackett [AuthSuccess] -> re-login #2
|
||||
15:27:59 radarr "cookie signature not valid, removing session"
|
||||
15:31:06 jackett [AuthSuccess] -> re-login #3
|
||||
15:31:29 radarr "cookie signature not valid, removing session" <- deletes it again
|
||||
15:31:32 jackett "No valid authentication in request" <- 3 SECONDS later, no cookie
|
||||
|
||||
`removing session` is oauth2-proxy calling ClearSession, which expires the cookie on **its own**
|
||||
`--cookie-domain` — the whole zone. So jackett's session is not merely overwritten, it is **deleted
|
||||
by a process in another namespace**, 26 s after a clean login whose own `expires` was five minutes
|
||||
out. That is the answer to the open question in the brief ("26 s is well inside the 5-minute window,
|
||||
so plain expiry does not explain it"): it was never expiry, and it was never Jackett.
|
||||
|
||||
It also explains the half of the report that looked like user error. **Reloading genuinely does not
|
||||
help**, because the operator's Radarr tab re-clears the cookie seconds after each new login — and
|
||||
Radarr is not a passive tab. Its UI polls: of 156 unauthenticated requests in its log against
|
||||
**1** AuthSuccess, the top paths are `/api/v3/command` (62), `/api/v3/queue/details` (46) and
|
||||
`/signalr/messages/negotiate` (43). Every one of those polls, arriving with a cookie Radarr cannot
|
||||
decrypt, calls ClearSession again. **A left-open Radarr tab is a cookie-deletion engine firing every
|
||||
few seconds**, which is why Jackett's sessions die at irregular, short, apparently random intervals.
|
||||
|
||||
**A counter-hypothesis was put to me and it does not survive bounding.** The observation was that
|
||||
Crafty already carries this fix yet showed the platform's worst failure ratio (81:1 over 24 h from
|
||||
Loki), implying the fix does not work. But Crafty's pod started **2026-09-30T03:57:19Z with 0
|
||||
restarts**, so a 24 h window is ~10 h of the *pre-fix* pod. Rather than bound it in Loki I read the
|
||||
ratio off the pod's own log, whose boundary is the process start and therefore exact: **inside the
|
||||
fixed pod's entire 13 h 52 m lifetime, Crafty logged 3 `No valid authentication` events, not 81** —
|
||||
from one client IP, two of them WebSocket `/ws` reconnects, in two isolated minutes (04:02 and
|
||||
15:22), against 0 AuthSuccess because nobody signed in. Post-fix Crafty is idle, not broken. The
|
||||
ratio metric is confounded by endpoint type — apps that poll or hold WebSockets (Radarr, Crafty)
|
||||
manufacture failure lines at their retry rate — so it ranks *chattiness*, not breakage. Correctly
|
||||
bounded, Crafty is evidence **for** the fix.
|
||||
|
||||
**2. Session lifetime pinned to a 5-minute access token.** Measured from the sidecar's own
|
||||
`[AuthSuccess]`: `created:15:31:06.391860254` / `expires:` + **300.000 s** exactly, with
|
||||
`refresh_token:true` on the same line. `--cookie-refresh` is unset and its default is `0`
|
||||
(disabled), so the proxy holds a refresh token it has no mandate to spend.
|
||||
|
||||
**3. XHRs answered with 302 instead of 401 — why none of it was visible.** oauth2-proxy's built-in
|
||||
AJAX guard fires only on an **exact** `Accept: application/json`, which Jackett's jQuery does not
|
||||
consistently send. Measured on `/api/` paths in this pod's log: **33 × 302 against 2 × 401**, and the
|
||||
split runs straight through Jackett's own POSTs (`/api/v2.0/server/adminpassword` → 401,
|
||||
`/api/v2.0/server/update` → 302). A 302 on an unauthenticated XHR is followed invisibly by the
|
||||
browser, satisfied silently by the live Keycloak SSO session, and resolves with HTML where the
|
||||
JavaScript expected JSON — no prompt, no error. On a write it is worse: a browser following a 302
|
||||
re-issues a POST as a **GET and drops the body**, so the indexer configuration is discarded with
|
||||
nothing logged as a failure anywhere.
|
||||
|
||||
### The fix has one home, and it is shared — this is the blocker
|
||||
|
||||
The brief assigned me `apps/wave-0/jackett.yaml` as exclusively mine, and assigned the other four
|
||||
apps to a sibling branch. **That split is not implementable, and I found out by trying it.**
|
||||
|
||||
`apps/wave-0/jackett.yaml` is a **generated golden**. The sidecar's argument list is built by one
|
||||
shared function, `_oauth2_sidecar_container()` in `catalog/render.py`. `catalog/jackett.yaml`
|
||||
declares only `auth.mode` and `auth.groups`, and the `auth` schema is `additionalProperties: false`
|
||||
— **there is no per-app override hook**. I wrote the fix into the rendered file first; the merge
|
||||
gate rejected it exactly as designed:
|
||||
|
||||
[fail] GOLDEN DRIFT: apps/wave-0/jackett.yaml does not match the canonical render
|
||||
[fail] stage render-golden: FAIL -> MERGE GATE: BLOCKED (rc 1)
|
||||
|
||||
So I made the change at its real source instead and **measured the blast radius** before committing
|
||||
to it. A change confined to that one function rewrites the sidecar block in **all five** rendered
|
||||
apps, and `render-golden` requires all five be regenerated in the same commit:
|
||||
|
||||
CHANGED: apps/wave-0/jackett.yaml
|
||||
CHANGED: apps/wave-0/pihole.yaml
|
||||
CHANGED: apps/wave-0/qbittorrent.yaml
|
||||
CHANGED: apps/wave-1/radarr.yaml
|
||||
CHANGED: apps/wave-1/sonarr.yaml
|
||||
|
||||
Four of those five are the files I was explicitly told not to touch, and all four are being edited
|
||||
right now on `fix/oauth2-proxy-cookie-collision-fleet`. **The gate and the ownership boundary are in
|
||||
direct contradiction**: I cannot fix Jackett's sidecar without regenerating four files another
|
||||
branch owns, and two branches rewriting the same generated block across five files is precisely the
|
||||
cross-branch conflict this repo's §7 exists to prevent.
|
||||
|
||||
I stopped rather than force it. **The cookie fix is one change, not five**, and it should be re-cut
|
||||
as a single renderer commit on one branch. The intended content, validated below:
|
||||
|
||||
--cookie-name=_oauth2_proxy_{name} # per-app, derived
|
||||
--cookie-domain={host} # host-scoped, derived
|
||||
--whitelist-domain=.{ZONE} # unchanged: redirect targets, not cookie scope
|
||||
--cookie-refresh=2m30s # half the measured 300 s token lifetime
|
||||
(drop --pass-access-token, --set-xauthrequest)
|
||||
|
||||
`--api-route=^/api/` is the one flag I would **not** apply renderer-wide on this evidence: I
|
||||
measured the 302/401 split on Jackett only, and PiHole's UI does not serve `/api/` at all. It needs
|
||||
either a per-app hook or four more measurements.
|
||||
|
||||
**Metrics — asked for, and declined here for the same structural reason, not on merit.** It is true
|
||||
and I re-measured it: **no per-app sidecar exposes metrics**. Only the shared proxy carries
|
||||
`--metrics-address=0.0.0.0:44180` and a `metrics` container port; jackett, pihole, qbittorrent,
|
||||
radarr, sonarr and crafty all have `(ABSENT)` and ports `['http']` alone, so the listener never
|
||||
starts and nothing about these gates is scrapable. Adding it is worth doing and the pattern is
|
||||
already proven in-tree by the shared proxy. But it is **not** "inside my file": the container's
|
||||
`args` *and* its `ports` both come from `_oauth2_sidecar_container()`, so adding
|
||||
`--metrics-address` plus a `metrics` port hits the identical five-file blast radius as the cookie
|
||||
change. It belongs in the same single renderer commit — which is an argument for cutting that commit
|
||||
soon, not for me to hand-edit a golden a second time after the gate already refused the first.
|
||||
(Crafty is separately hand-written and could take the metrics flag on its own.)
|
||||
|
||||
**Crafty is unaffected by all of this** — its sidecar is hand-written in
|
||||
`platform/crafty/deployment.yaml`, not rendered, which is why it could be and was fixed alone on
|
||||
2026-09-29.
|
||||
|
||||
### What this branch does land
|
||||
|
||||
`runbooks/oauth2-proxy-session.md` — the four sidecar log lines that tell Keycloak, the sidecar and
|
||||
the app's own login apart (including the upstream `-` field that means the proxy answered a request
|
||||
the app never saw), the cookie-collision cross-log diagnosis, the 5-minute-session check, the
|
||||
302-vs-401 trap, and the generated-golden warning with its measured blast radius. Registered in the
|
||||
runbooks `configMapGenerator` **and** listed in `runbooks/README.md`, so the URL resolves rather
|
||||
than the file merely existing.
|
||||
|
||||
## What I verified
|
||||
|
||||
- **The collision is the live failure.** `[stored_session.go:94] Error loading cookied session:
|
||||
cookie signature not valid, removing session` — **1 occurrence in jackett's log (15:23:44), 8 in
|
||||
radarr's** (15:22:49, 15:23:58, 15:27:59, **15:31:29**, 15:32:29 ×4), and **zero** in pihole,
|
||||
qbittorrent, sonarr, crafty and both shared-proxy replicas. The 15:31:29 → 15:31:32 pair is the
|
||||
operator's whole report.
|
||||
- **Six instances / seven pods are in the collision set** — the brief said five; the shared
|
||||
forward-auth proxy in namespace `oauth2-proxy` is the sixth and runs 2 replicas. Read from live
|
||||
pod specs: jackett, pihole, qbittorrent, radarr, sonarr and `oauth2-proxy` all have **no**
|
||||
`--cookie-name` argument and `--cookie-domain=.home.keithjackson.dev`. Only crafty is fixed.
|
||||
- **All seven cookie-secrets differ.** Compared in-cluster by sha256, first 12 hex only, values
|
||||
never printed: `d9c5…`/`0b3b…`/`b025…`/`bb6a…`/`25f4…`/`e679…`/`911a…`. So no two of these proxies
|
||||
can read each other's cookie — the precondition for the eviction.
|
||||
- **The 5-minute lifetime**, from four `[AuthSuccess]` lines: `expires − created = 300.000 s`, with
|
||||
`refresh_token:true` and `groups:[platform-admins]`. Live args confirm `--cookie-refresh` absent.
|
||||
- **The 302/401 split** on `/api/` paths in this pod's log: 33 × 302, 18 × 200, 2 × 401, 2 × 204.
|
||||
- **4 of the 6 POSTs never reached Jackett** — the access log's upstream field is `-`, not `/`:
|
||||
`adminpassword` → 401, `adminpassword` → 401, `server/update` → 302, `server/update` → 302. The
|
||||
two that did reach it (`adminpassword`, `indexers/1337x/config`) carry the authenticated email.
|
||||
- **The proposed argument list starts, against the pinned image and the real issuer.** Ran
|
||||
`quay.io/oauth2-proxy/oauth2-proxy:v7.8.1` locally with the full proposed args; OIDC discovery
|
||||
against `keycloak.home.keithjackson.dev` succeeded and it printed back its own reading:
|
||||
`Cookie settings: name:_oauth2_proxy_jackett secure(https):true httponly:true expiry:168h0m0s
|
||||
domains:jackett.home.keithjackson.dev path:/ samesite: refresh:after 2m30s` and
|
||||
`API route - Path: ^/api/`.
|
||||
- **`--api-route` is singular.** The brief specified `--api-routes`, which does not exist in v7.8.1;
|
||||
the proxy would have refused to start. Read off `--help`.
|
||||
- **`--session-cookie-minimal` is mutually exclusive with `--cookie-refresh`** — provoked, not
|
||||
assumed: `invalid configuration: cookie_refresh > 0 requires oauth tokens in sessions.
|
||||
session_cookie_minimal cannot be set`. This kills the brief's premise that dropping
|
||||
`--pass-access-token` shrinks the cookie: only `--session-cookie-minimal` strips tokens from the
|
||||
session, and it is unavailable while refresh is on. Dropping those two flags is justified on
|
||||
**least privilege and inertness**, not size.
|
||||
- **`--set-xauthrequest` is inert here**: HTTPRoute `jackett-9117` backendRef is
|
||||
`Service/jackett port 4180`, there is no ext-authz attachment on the `shared` Gateway, so
|
||||
`/oauth2/auth` is never called.
|
||||
- **`apps/**` is generated and the fix cannot be scoped to one app** — both halves measured, not
|
||||
inferred: the gate's `render-golden` stage rejected the hand-edit, and re-rendering after a
|
||||
renderer-only change listed all five apps as changed (transcript above).
|
||||
- **Crafty's post-fix failure rate, bounded exactly.** Pod `crafty-647c7d8cc5-5spv9`,
|
||||
`startTime=2026-09-30T03:57:19Z`, `restarts=0`, live args `--cookie-name=_oauth2_proxy_crafty` and
|
||||
`--cookie-domain=crafty.home.keithjackson.dev`. Over that pod's whole lifetime: **3**
|
||||
`No valid authentication`, 0 AuthSuccess, one client IP, 2 of the 3 on `/ws`. The 81:1 figure is a
|
||||
24 h window over a 13.9 h pod and is dominated by the pre-fix process.
|
||||
- **The ratio metric ranks chattiness, not breakage.** Radarr 156:1 with the top paths being
|
||||
`/api/v3/command`, `/api/v3/queue/details` and `/signalr/messages/negotiate` — UI polling, not 156
|
||||
distinct user-visible failures.
|
||||
- **No per-app sidecar exposes metrics.** `--metrics-address` is `(ABSENT)` and `ports` is
|
||||
`['http']` on all six per-app sidecars; only the shared proxy has
|
||||
`--metrics-address=0.0.0.0:44180` and a `metrics` port.
|
||||
- **Gate:** `./ci/gate.sh` in the pinned toolbox, **rc 3 — UNDETERMINED, not green**. Green:
|
||||
`spec-immutability`, `approval-policy`, `decisions-log`, `lint`, `l0`, `render-golden`, `l1`.
|
||||
Skipped: `gitea-settings` (no `SETTINGS_API_TOKEN`) and L2/L3 conformance (`CLUSTER_AVAILABLE`
|
||||
unset), leaving touched requirement `R-OBS-04` uncertified. Note `spec-immutability` is **green**,
|
||||
not skipped — mounting the sibling spec repo at `/home-network-k3s` (the path
|
||||
`ci/spec-immutability.sh` actually defaults to) in the ENGINE shim fixes that skip; the commonly
|
||||
circulated shim mounts it at its host path, where `SPEC_REPO` never finds it.
|
||||
- **Gitea CI on this PR, read from the commit's `/statuses` rather than from a tick.** Two contexts
|
||||
reported, and **only one of them ran**:
|
||||
|
||||
gate / gate (pull_request) state=success desc='Successful in 1m41s'
|
||||
gate / conformance (pull_request) state=success desc='Has been skipped'
|
||||
|
||||
That second line is §2b's trap in the wild: a **skipped** Gitea job reports `state=success`. The
|
||||
conformance job did not execute, so the green tick beside it certifies nothing. Read together with
|
||||
the local rc 3 above, the two agree exactly — the offline L0/L1 subset passed and L2/L3 never ran.
|
||||
Anyone reading the PR's check mark alone would conclude otherwise.
|
||||
- **`main` is protected**, re-measured this session rather than assumed:
|
||||
`GET /repos/.../branches/main` → `protected=true`, `user_can_push=false`.
|
||||
|
||||
## What I did NOT verify
|
||||
|
||||
- **The operator's problem is NOT FIXED by this branch.** Only the diagnosis and the runbook land.
|
||||
Jackett's sidecar is unchanged and will keep losing its session to Radarr until the renderer
|
||||
change ships. **This is the single most important line in this entry.**
|
||||
- **No browser round-trip.** Every mechanism is measured from logs and specs and the argument list
|
||||
is proven to start, but nobody has added an indexer through the proposed configuration. The human
|
||||
test is below, for whoever lands the renderer change.
|
||||
- **Nothing was applied.** No `kubectl apply/edit/patch`, no `flux reconcile`. The running pod is
|
||||
still `jackett-c5bfcbfb8-c75f9` (2/2 Running, 17 d, 0 restarts — unchanged by this session's
|
||||
read-only probes). Note that when the fix does land, merging is what deploys, and it will restart
|
||||
the pod (`strategy: Recreate` on an RWO Longhorn volume), briefly interrupting Radarr's and
|
||||
Sonarr's direct `:9117` access.
|
||||
- **Cookie size and chunking: UNMEASURED, and I will not claim either way.** I could not read a
|
||||
`Set-Cookie` header without a browser. I found no `_oauth2_proxy_0`/`_1` chunk evidence and no
|
||||
4 kB warning in 36,287 log lines — but I did **not** establish that v7.8.1 emits such a warning at
|
||||
this log level, so that absence is not evidence. If sessions still misbehave after the fix lands,
|
||||
this is the next thing to measure, with `--session-store-type=redis` as the escape hatch, since
|
||||
`--session-cookie-minimal` is ruled out (above).
|
||||
- **`--cookie-refresh=2m30s` has never executed a refresh here.** It depends on Keycloak accepting
|
||||
the refresh token and on the realm's SSO Session Idle/Max, none of which I read (that needs admin
|
||||
credentials). If Keycloak's *refresh* token is also short-lived, the five-minute logout persists
|
||||
and the realm is where to look next.
|
||||
- **`--api-route` is measured for Jackett only.** PiHole and qBittorrent were not measured, and
|
||||
PiHole's UI does not serve `/api/`. Do not apply it renderer-wide on my evidence.
|
||||
- **The four sibling apps still collide with each other** regardless of what happens to Jackett.
|
||||
The same operator report should be expected from any of pihole / qbittorrent / radarr / sonarr.
|
||||
- **L2/L3 conformance did not run** (`CLUSTER_AVAILABLE` unset), so nothing about runtime behaviour
|
||||
is certified by the gate. `gitea-settings` also skipped, so branch protection and runner state are
|
||||
**unverified by me this session** — I did not re-measure them against the Gitea API.
|
||||
- **I did not query Loki.** The Crafty bounding above comes from the pod log, which I argue is the
|
||||
better instrument (an exact process boundary rather than a wall-clock window against a 72 h
|
||||
retention). But it means I cannot speak to the 24 h or 12 h *platform-wide* ratios, and a
|
||||
per-app ratio over a window longer than the current pod's life is not something I measured at all.
|
||||
- **Collision causation is one adjacency, not a population.** The 15:31:29 → 15:31:32 pair is three
|
||||
seconds and the mechanism is documented and present, but I did not enumerate every jackett failure
|
||||
and match it to a radarr ClearSession. Treat mechanism 1 as **necessary and firing, not proven
|
||||
sufficient** — the coordinator's framing, and I agree with it on this evidence.
|
||||
- **The metrics change is NOT in this branch** (reason above). Jackett therefore remains inside the
|
||||
oauth2-proxy metrics gap, and the ratio alert shipped in PR #29 still cannot see it.
|
||||
- **The runbook's commands were not all executed verbatim.** The log-reading ones I ran; the
|
||||
per-namespace `for` loops are edited from the scripted forms I actually used.
|
||||
|
||||
### The human test — for whoever lands the renderer fix
|
||||
|
||||
After it merges and the pod rolls (watch for a **new** pod name at 2/2):
|
||||
|
||||
1. **Sign in** at `https://jackett.home.keithjackson.dev/UI/Dashboard`.
|
||||
2. **Open Radarr in a second tab** and sign in there too. **This is the step that breaks Jackett
|
||||
today** — it is the 15:31:29 line. Return to the Jackett tab.
|
||||
3. **Add an indexer** — *+ Add indexer*, pick one, *Configure*, *Okay*. It must save.
|
||||
4. **Wait six minutes on the open dashboard, then save another.** That is the `--cookie-refresh`
|
||||
test; the session currently dies at five.
|
||||
5. **If something does still fail, it must now fail LOUDLY** — DevTools → Network, a broken `/api/`
|
||||
call shows **401**, not a 302 followed by HTML. A silent success-looking no-op means the change
|
||||
did not take; check the running pod's args before assuming anything else.
|
||||
6. **A plain page reload must still sign you in silently** (`/UI/Dashboard` is outside `^/api/`).
|
||||
A 401 page on reload means the regex is too broad — that is a regression.
|
||||
@@ -0,0 +1,283 @@
|
||||
---
|
||||
date: 2026-09-30
|
||||
branch: feat/node-health-render-device
|
||||
title: The Intel render device is now self-re-measuring, not a fact someone checked once
|
||||
---
|
||||
|
||||
## What changed
|
||||
|
||||
### Why this probe has value at all
|
||||
|
||||
**Nothing else in the platform would notice the device going missing.** Plex exposes no
|
||||
ServiceMonitor, so no alert fires on a silent fall back to software transcoding. And the
|
||||
manifest side mounts the device with `hostPath type: DirectoryOrCreate`, which means a node
|
||||
*without* the device mounts an **empty directory** — Plex starts, runs, and degrades quietly
|
||||
rather than failing loudly. There is no crash loop, no failed mount, no alert: just a hot CPU.
|
||||
This check is the only thing in the system that would surface it, which is the actual
|
||||
justification for the role carrying it.
|
||||
|
||||
### The change
|
||||
|
||||
`roles/node_health` grew a graphics / render-device probe. Nothing was installed, no module
|
||||
was loaded, no group was changed, nothing was rebooted: the prod nodes already have a working
|
||||
Intel render device (measured by hand on all three on 2026-09-30) and the point of this change
|
||||
is that the measurement now **re-runs forever** instead of decaying silently in a handoff note.
|
||||
|
||||
The failure this exists to catch is silent by construction. Plex hardware-transcodes through
|
||||
`/dev/dri/renderD128`; if that node stops existing — node rebuild, a BIOS change that disables
|
||||
the onboard IGD, a kernel that binds a different DRM driver, a driver regression — Plex falls
|
||||
back to software transcoding on its own and the only symptom is a hot CPU.
|
||||
|
||||
Four files:
|
||||
|
||||
- `files/node_health_collect.py` — new `collect_graphics()`, wired into the **unprivileged**
|
||||
pass (`main()`). Everything it reads is world-readable: `lstat()` on `/dev/dri/*`,
|
||||
`grp.getgrgid()` for the owning group name, the `/sys/class/drm/card*/device/driver` symlink
|
||||
target for the bound driver, `/sys/bus/pci/devices/*/class` for display-class (PCI base class
|
||||
`0x03`) devices whether or not a driver is bound, `/sys/class/drm/card*/engine/*` for the
|
||||
`vcs*`/`vecs*` media engines, `/proc/modules`, `/proc/cmdline`, and
|
||||
`/sys/module/{i915,xe}/parameters/enable_guc`. `lspci` and `vainfo` were added to the
|
||||
existing tool probe; both are optional and neither is depended on.
|
||||
- `tasks/assess.yml` — one shared derive task plus three checks: `graphics.render_device`
|
||||
(PASS/FAIL/SKIP), `graphics.drm_driver` (PASS/WARN/FAIL/SKIP), `graphics.media_stack`
|
||||
(INFO/SKIP).
|
||||
- `defaults/main.yml` — `node_health_graphics_expected_driver: i915`, so the one comparison the
|
||||
check makes is data rather than task logic.
|
||||
- `templates/summary.txt.j2` — a `GRAPHICS / RENDER DEVICE` section, and
|
||||
`roles/node_health/README.md` gained a section explaining the design and two new "known
|
||||
limits".
|
||||
|
||||
Three design rulings worth stating, because each is the kind of thing a later edit would get
|
||||
wrong:
|
||||
|
||||
1. **The render node's GID is reported, never asserted.** It read `root:render`, gid **992**,
|
||||
mode `0660` on all three prod nodes. The check does *not* compare against `992`. `render` is
|
||||
a **systemd dynamic** allocation (`systemd-sysusers`), not a Debian static gid from
|
||||
`base-passwd`, so a rebuild can legitimately move it — and Plex's container entrypoint stats
|
||||
the device and joins whichever group owns it, so drift genuinely is not a fault. The
|
||||
observed group name and number appear in the detail line, which makes drift *visible*
|
||||
without making it an error. A hard `!= 992` would be a false alarm waiting for a reinstall.
|
||||
2. **The PCI scan is deliberately separate from the DRM scan.** `/sys/class/drm/card*` exists
|
||||
only once a driver has *bound*, so a GPU with nothing attached has no `card0` and would
|
||||
otherwise be indistinguishable from a machine with no GPU. Scanning the PCI bus separately
|
||||
is what lets "no display device at all" be an honest **SKIP** (a VM guest — the same
|
||||
convention `thermal.max` already uses for "no sensor on this hardware") while "a GPU is on
|
||||
the bus and there is still no render node" is a **FAIL**.
|
||||
3. **GuC/HuC and host VAAPI userspace are INFO and must stay INFO.** GuC/HuC off
|
||||
(`i915.enable_guc=0`) is an accepted state here and costs low-power H.264 encode *quality*
|
||||
only. No host `vainfo` is not a defect at all — Plex ships its own libva and media driver
|
||||
inside its container. Grading either would put a permanent WARN on a chosen configuration,
|
||||
or invite someone to `apt install vainfo` on a node to clear a health check, and this role
|
||||
installs nothing.
|
||||
|
||||
`xe` bound instead of `i915` is a **WARN**, not a FAIL: the device still works, but the two
|
||||
drivers do not share a userspace media stack. On these nodes `xe.ko.xz` exists on disk and is
|
||||
bound to nothing.
|
||||
|
||||
## What I verified
|
||||
|
||||
### The real prod run (read-only, all three nodes)
|
||||
|
||||
`--list-hosts` first, every time. It named exactly three hosts and nothing else:
|
||||
|
||||
play #1 (k3s_prod): Inspect node hardware health (read-only)
|
||||
pattern: ['k3s_prod']
|
||||
hosts (3): k3s-prod-3 k3s-prod-2 k3s-prod-1
|
||||
|
||||
Then `--syntax-check` (rc 0), then the run:
|
||||
|
||||
ansible-playbook playbooks/node-health.yml -e node_health_hosts=k3s_prod --limit k3s_prod
|
||||
|
||||
k3s-prod-1 : ok=51 changed=3 unreachable=0 failed=0 skipped=1
|
||||
k3s-prod-2 : ok=51 changed=2 unreachable=0 failed=0 skipped=1
|
||||
k3s-prod-3 : ok=51 changed=2 unreachable=0 failed=0 skipped=1
|
||||
PLAY rc=0
|
||||
|
||||
(The `changed=` counts are the *controller-side* artifact writes under
|
||||
`infra/ansible/_artifacts/node-health/`, delegated to localhost. Nothing on any node changed —
|
||||
the collector task is `changed_when: false` and the role touches nothing.)
|
||||
|
||||
All three nodes graded **PASS / PASS / INFO** on the new checks, 0 FAIL overall:
|
||||
|
||||
k3s-prod-1 VERDICT 0 FAIL · 1 WARN · 18 PASS · 2 SKIP · 6 INFO
|
||||
k3s-prod-2 VERDICT 0 FAIL · 3 WARN · 15 PASS · 3 SKIP · 6 INFO
|
||||
k3s-prod-3 VERDICT 0 FAIL · 2 WARN · 17 PASS · 2 SKIP · 6 INFO
|
||||
|
||||
[PASS] graphics.render_device — DRM render node present for hardware transcoding (/dev/dri/renderD*)
|
||||
[PASS] graphics.drm_driver — DRM driver bound to the display device (expected i915)
|
||||
[INFO] graphics.media_stack — Media engines, GuC/HuC firmware and host VAAPI userspace (informational)
|
||||
|
||||
(The pre-existing WARNs and SKIPs are other checks — SMART, memattr, resolvers — and are not
|
||||
mine; none of them moved.)
|
||||
|
||||
The new report section, **byte-identical on all three nodes**:
|
||||
|
||||
--------------------------------------------------------------------------------
|
||||
GRAPHICS / RENDER DEVICE (the Plex hardware-transcoding path)
|
||||
--------------------------------------------------------------------------------
|
||||
pci 0000:00:02.0 00:02.0 VGA compatible controller [0300]: Intel Corporation CoffeeLake-S GT2 [UHD Graphics 630] [8086:3e98] (rev 02)
|
||||
driver i915
|
||||
drm card0 driver i915 at 0000:00:02.0 engines bcs0 rcs0 vcs0 vecs0
|
||||
dev card0 character special mode 0660 root:video gid 44
|
||||
dev renderD128 character special mode 0660 root:render gid 992
|
||||
GuC/HuC NOT READ — /sys/module/i915/parameters/enable_guc is not
|
||||
readable without root (i915 module parameters are mode 0400)
|
||||
no i915/xe parameter on the kernel cmdline
|
||||
modules amdgpu=false, i915=true, nouveau=false, nvidia=false, radeon=false, xe=false (/proc/modules — corroboration only; the bound-driver symlink above decides)
|
||||
vainfo NOT installed on this host — not a defect: Plex ships its own libva/media driver inside its container
|
||||
|
||||
That reproduces every item of the 2026-09-30 hand measurement independently: `renderD128`
|
||||
character special, `root:render`, **gid 992**, mode `0660`; `card0` `root:video` gid 44 mode
|
||||
`0660`; `i915` bound; `xe` **not** loaded and bound to nothing; `[8086:3e98] (rev 02)`
|
||||
CoffeeLake-S GT2 UHD Graphics 630 at `00:02.0`; `vcs0` and `vecs0` live; no host `vainfo`.
|
||||
|
||||
**One measured correction to my own phase-1 assumption:** `lspci` **is** present on all three
|
||||
prod nodes (`/usr/bin/lspci` — Debian ships `pciutils` by default). I had written that it was
|
||||
absent. It is still *treated* as optional, because `pciutils` is genuinely not declared in
|
||||
`node_baseline_packages` and depending on an undeclared package would be wrong — but the
|
||||
comments and README that asserted it was missing were factually wrong and are now corrected.
|
||||
Its presence is why the `pci` line above carries a readable device name rather than bare IDs.
|
||||
|
||||
### The gate
|
||||
|
||||
DEV_KUBECONFIG=/dev/null ENGINE=<shim> ./infra/devcontainer/dev.sh run bash -c './ci/gate.sh'
|
||||
GATE rc=3
|
||||
|
||||
**rc 3 — UNDETERMINED. NOT green.** Nothing failed, and nothing is certified either. Exactly
|
||||
three stages did not run, all three the known local ceiling from a worktree, and **no fourth
|
||||
skip**:
|
||||
|
||||
[skip] spec-immutability spec repo not present at /work/../home-network-k3s (set SPEC_REPO)
|
||||
[skip] gitea-settings SETTINGS_API_TOKEN is not set — cannot read the live Gitea rule
|
||||
[skip] conformance no cluster configured (CLUSTER_AVAILABLE!=true) — L2/L3 NOT RUN
|
||||
|
||||
[ ok ] decisions-log: pass [ ok ] lint: pass [ ok ] l0: pass
|
||||
[ ok ] render-golden: pass [ ok ] l1: pass [ ok ] approval-policy: pass
|
||||
|
||||
Both CLAUDE.md §3 worktree workarounds were needed and both worked: `DEV_KUBECONFIG=/dev/null`
|
||||
(the credential in the main checkout was **not** copied) and an `ENGINE` shim that bind-mounts
|
||||
the main checkout's `.git` read-only. No `tracked_files()` traceback appeared, which is how I
|
||||
know the shim took effect rather than the git stages silently degrading.
|
||||
|
||||
`ansible-lint` was **not** run, because **`ci/` does not invoke it anywhere** — I grepped
|
||||
`ci/` and `.gitea/` and the lint stage is `yamllint -c ci/yamllint.yaml` only. I am not
|
||||
claiming to have matched a lint convention that does not exist.
|
||||
|
||||
### The CI gate on the PR — a green tick that is exit 3, read from the log
|
||||
|
||||
The required context `gate / gate (pull_request)` reported **`success`, "Successful in 1m40s"**
|
||||
on commit `a78f010`. That description matters: a *skipped* Gitea job reports `success` with
|
||||
"Has been skipped", so this one genuinely ran, and 1m40s matches CLAUDE.md §2's ~1m35s.
|
||||
|
||||
**But the green tick does NOT mean `gate.sh` returned 0.** `.gitea/workflows/gate.yml:138` is
|
||||
`run: ./ci/gate.sh || [ "$?" -eq 3 ]` — the job tolerates rc 3 by design. So I fetched the job
|
||||
log (`/actions/runs/173/jobs/0/logs`; the `/api/v1/.../tasks/{id}/logs` endpoints 404 on this
|
||||
Gitea, the web route serves it) and read the gate's own headline:
|
||||
|
||||
[fail] MERGE GATE: UNDETERMINED — greenness was NOT ESTABLISHED.
|
||||
[fail] Stages that did not run: spec-immutability
|
||||
|
||||
**So CI is also rc 3, with exactly ONE stage skipped — `spec-immutability`.** The green Gitea
|
||||
check is the tolerance swallowing that 3, not a rc 0.
|
||||
|
||||
**CI verified two things my local run could not, and both came back green:**
|
||||
|
||||
[ ok ] stage gitea-settings: pass
|
||||
[ ok ] enable_push = false enable_status_check = true
|
||||
[ ok ] required_approvals = 1 block_admin_merge_override = true
|
||||
[ ok ] block_on_rejected_reviews = true dismiss_stale_approvals = true
|
||||
[ ok ] runners online: 1 (>= 1)
|
||||
[ ok ] Gitea settings match the declaration.
|
||||
[ ok ] stage approval-policy: pass
|
||||
[ ok ] no architecturally/security-relevant paths touched — the GATE is the approver.
|
||||
|
||||
My local run **skipped** `gitea-settings` (no `SETTINGS_API_TOKEN`) and could not give
|
||||
`approval-policy` a PR number. CI has the secret and `REQUIRE_GITEA_SETTINGS: "true"`, so a skip
|
||||
there would have failed the job. That is measured confirmation that the live Gitea matches
|
||||
`infra/gitea/expected-branch-protection.json`, and that `infra/ansible/**` needs no human
|
||||
reviewer. `lint` passed over 405 files. `conformance` SKIPPED in CI too
|
||||
(`CLUSTER_AVAILABLE != true`), so L2/L3 remains unverified there as well.
|
||||
|
||||
Corroborated independently: `GET /repos/.../branches/main` →
|
||||
`protected=true user_can_push=false enable_status_check=true required_approvals=1`,
|
||||
contexts `["gate / gate (pull_request)"]`; `GET /admin/actions/runners` → `incluster-dind
|
||||
status=online`. And note the commit's **combined** state is `pending`, not success, because
|
||||
`gate / conformance (pull_request)` sits at "Blocked by required conditions" — that context is
|
||||
not in the required list, so it does not block, but the PR is not "all green" either.
|
||||
|
||||
The gate's `lint` stage passed with one `line too long` **warning** on
|
||||
`roles/node_health/tasks/assess.yml:189` — that is pre-existing, in the SMART block, well
|
||||
above my insertion point, and warnings do not fail the stage.
|
||||
|
||||
### Offline work (before the lock was released)
|
||||
|
||||
- `python3 -m py_compile` on the collector: clean.
|
||||
- `collect_graphics()` executed for real against this workstation's hardware (an unrelated
|
||||
machine, Raptor Lake-S UHD Graphics + an NVIDIA discrete GPU) and returned the whole
|
||||
structure with no exception, correctly reporting two cards, two render nodes, the bound
|
||||
driver per card, the `vcs0/vcs1/vecs0` media engines, and `enable_guc` as *not readable
|
||||
without root* rather than as a fabricated `0`.
|
||||
- That last point is a real finding and changed the design: `/sys/module/i915/parameters/*` is
|
||||
**mode 0400**, so an unprivileged pass cannot read `enable_guc`. It now reports
|
||||
`NOT READ — <reason>` and `/proc/cmdline` is read as the world-readable second source. An
|
||||
earlier draft would have silently reported a null.
|
||||
- Two other bugs found and fixed by running it rather than reasoning about it:
|
||||
`/dev/dri/by-path` is a **directory** and was being `lstat()`ed as a device; and
|
||||
`/sys/class/drm` carries one entry per *connector* (`card0-HDMI-A-1`), which had to be
|
||||
excluded from the card scan.
|
||||
- All three checks and the new template section were rendered through a standalone Jinja
|
||||
harness (Ansible is not on the host — CLAUDE.md §5) against **seven** fact sets: the real
|
||||
workstation; GPU present with the render node removed; no display device at all; `xe` bound
|
||||
in place of `i915`; a display device with **no** driver bound; `renderD128` present but not a
|
||||
character device; and a shape with neither `lspci` nor `vainfo`. Zero template errors, and
|
||||
the statuses came out as designed — `FAIL/PASS/INFO`, `SKIP/SKIP/SKIP`, `PASS/WARN/INFO`,
|
||||
`FAIL/FAIL/INFO`, `FAIL/PASS/INFO` respectively. The harness supplies stand-ins for the
|
||||
Ansible-only filters (`bool`, `dict2items`, `flatten`, `zip`, `regex_replace`) and matches
|
||||
Ansible's `trim_blocks=True`.
|
||||
**Read that for exactly what it is: evidence that the LOGIC is right, and no evidence
|
||||
whatsoever that the HARDWARE DETECTION is.** Six of those seven fact sets were hand-mutated
|
||||
dictionaries, not machines. They are not hardware tests and must not be written up as if
|
||||
they were.
|
||||
|
||||
## What I did NOT verify
|
||||
|
||||
- **The FAIL path has never fired on a real node.** Every prod node has a working render
|
||||
device, and this role is read-only, so a device cannot be taken away to prove the check
|
||||
catches its absence. The failure branches were exercised against **synthetic fact sets
|
||||
only** (see above). I am not claiming the failure path is tested against hardware.
|
||||
- **`xe` detection is likewise untested against reality.** No node here has `xe` bound — the
|
||||
module exists on disk as `xe.ko.xz` and is bound to nothing — so the WARN branch was only
|
||||
ever driven by a mutated fact set. If `xe` ever does bind, the *shape* of
|
||||
`/sys/class/drm/card*` under it is something I have assumed matches i915's (same
|
||||
`device/driver` symlink, same `engine/*` directories). That assumption is unverified.
|
||||
- **The media-engine sysfs path is confirmed on these nodes but not proven portable.**
|
||||
`vcs0`/`vecs0` were read from `/sys/class/drm/card0/engine/*` on all three prod nodes, so it
|
||||
is measured here. I did **not** confirm against kernel source that no other kernel version or
|
||||
driver publishes engines elsewhere. If one does, the check reports "none exposed" — an INFO
|
||||
line, not a false verdict, which is why this failure mode is tolerable.
|
||||
- **`enable_guc` is still unread on prod.** The report says `NOT READ — mode 0400`, which is
|
||||
honest but means I have **not** confirmed by measurement that GuC/HuC is off on these nodes;
|
||||
I only know the kernel cmdline carries no `i915.enable_guc=` override. The brief's ground
|
||||
truth says `i915.enable_guc=0`; my check does not corroborate that value, it declines to
|
||||
guess it. Reading it would need root, and it is INFO either way, so I left it alone.
|
||||
- **Not a transcode test.** These checks prove the render *device* exists, is a character
|
||||
device, and which driver owns it. They do **not** prove a transcode succeeds — that needs a
|
||||
VAAPI round trip inside Plex's own container with Plex's own libva, and is out of scope for a
|
||||
node-level check that installs nothing.
|
||||
- **Greenness was never established, locally or in CI.** Both runs returned **rc 3**. My local
|
||||
run skipped three stages (`spec-immutability`, `gitea-settings`, `conformance`); CI skipped
|
||||
one (`spec-immutability`) and its own headline says `MERGE GATE: UNDETERMINED`. **I am not
|
||||
describing this PR as green**, and the green Gitea tick must not be read as one: the workflow
|
||||
line is `./ci/gate.sh || [ "$?" -eq 3 ]`, so it is a tolerated 3.
|
||||
- **L2/L3 conformance did not run anywhere** — `CLUSTER_AVAILABLE != true` in both. Whatever
|
||||
runtime requirements this touches remain UNVERIFIED; only the L0/L1 offline subset passed.
|
||||
- **`spec-immutability` ran in neither.** The sibling spec repo is checked out in neither my
|
||||
worktree nor the CI workspace, so nothing confirms this change does not drift from the spec.
|
||||
It is the expected, tolerated skip — but it is still a skip, not a pass.
|
||||
- **No claim about the live platform beyond what was printed.** Per CLAUDE.md §2b this repo is
|
||||
not evidence about prod; the only prod facts I assert are the ones quoted verbatim above.
|
||||
Branch protection and runner state I did query directly, and the CI `gitea-settings` stage
|
||||
passed — so those two I assert on measurement, not on this file's prose.
|
||||
- **The stale dev inventory was left alone deliberately.** `hosts.ini` still defines the
|
||||
`k3s-dev-*` groups as the default target of most playbooks and the dev cluster no longer
|
||||
exists. That is real, it is out of this change's scope, and it is unclaimed work.
|
||||
@@ -0,0 +1,378 @@
|
||||
---
|
||||
date: 2026-09-30
|
||||
branch: fix/oauth2-proxy-cookie-collision-fleet
|
||||
title: The cookie collision was never Crafty's — it was the renderer's, and it shipped to five apps
|
||||
---
|
||||
|
||||
## What changed
|
||||
|
||||
Follow-up #3 of `decisions/2026-09-29-crafty-oauth2-proxy-signin.md`: *"Any further per-app
|
||||
reverse-proxy sidecar needs a unique `--cookie-name` too. The collision is a property of the
|
||||
pattern, not of Crafty."*
|
||||
|
||||
It is a property of the pattern, and this branch found where the pattern actually lives.
|
||||
|
||||
**Read the scope claim carefully before the detail.** This branch fixes THREE defects in the
|
||||
forward-auth sidecar contract, all in one function, all landing on all five apps at once:
|
||||
|
||||
| Defect | Fix | Measured evidence |
|
||||
|---|---|---|
|
||||
| Six independent instances, seven different cookie-secrets, **one** cookie name on **one** zone-wide domain — so they overwrite and `ClearSession`-delete each other's sessions | `--cookie-name=_oauth2_proxy_<app>` + host-scoped `--cookie-domain` | 7/7 distinct secrets; 6 instances on the default name; radarr logs the eviction fingerprint 8× in 12h |
|
||||
| The session dies **5 minutes** after sign-in — `--cookie-refresh` defaults to 0 = *disabled*, so the refresh token the proxy already holds is never spent | `--cookie-refresh=2m30s` (half the lifetime) | access-token lifetime = **300.000s exactly**, `refresh_token:true`, flag absent |
|
||||
| **No** per-app sidecar serves `/metrics` at all — without `--metrics-address` the listener never starts | `--metrics-address=0.0.0.0:44180` + a `metrics` container port | `ports` was `['http']` on all six |
|
||||
|
||||
Rejection *volume* is dominated by the second defect, not the first: ~95% of radarr's rejections
|
||||
carry no cookie at all. **Do not merge this expecting one number to drop** — three mechanisms were
|
||||
tangled together, and separating them is most of the value here.
|
||||
|
||||
**Boundary note.** My brief originally reserved `apps/wave-0/jackett.yaml` for a concurrent branch.
|
||||
That split is not achievable and has been revoked: the file is generated, the fix is one renderer
|
||||
edit, and `render-golden` requires all five goldens regenerated in the *same commit*. I own all
|
||||
five. The proof I ran before touching jackett is under "What I verified".
|
||||
|
||||
**The defect was not in the four manifests it was reported in.** `apps/wave-0/pihole.yaml`,
|
||||
`apps/wave-0/qbittorrent.yaml`, `apps/wave-1/radarr.yaml`, `apps/wave-1/sonarr.yaml` — and
|
||||
`apps/wave-0/jackett.yaml` — are **generated goldens**. One function emits all five sidecars:
|
||||
`catalog/render.py::_oauth2_sidecar_container()`, which hardcoded `--cookie-domain=.{ZONE}` and
|
||||
emitted no `--cookie-name` at all. Hand-editing a golden is not a fix twice over: `ci/render-golden.sh`
|
||||
FAILS on the drift (provoked, not assumed — see below), and the next re-render silently reverts it.
|
||||
|
||||
So the fix is in the generator, and both flags are now **derived** from the ServiceDefinition:
|
||||
|
||||
--cookie-name=_oauth2_proxy_{s['name']} unique per service, by construction
|
||||
--cookie-domain={host} the app's own route host, not the zone
|
||||
|
||||
`--whitelist-domain` deliberately stays zone-wide — it governs permitted post-login *redirect*
|
||||
targets, not cookie scope. `catalog/render.py --selftest` gained four assertions, including one
|
||||
that renders a **second** synthetic forward-auth service and requires the two cookie names to
|
||||
differ, so a future refactor to a constant fails in the renderer's own test.
|
||||
|
||||
Re-rendering with the canonical command regenerated **five** goldens, not four. Jackett is the
|
||||
fifth, and this is the one thing in this branch the PM brief did not anticipate — see
|
||||
"the jackett collision" below. The three `conformance/tests/l1-rendering/*/golden/` fixtures are
|
||||
expected-output copies of the same renderer, so they are regenerated too; that is not collateral
|
||||
damage, it is the L1 stage doing its job (it FAILED on the first gate run and named the drift).
|
||||
|
||||
**The new gate check** is `conformance/tests/l0-static/oauth2-proxy-cookie-names-distinct/`,
|
||||
declaring `conformance.home.k3s/scope: whole-repo` so `ci/l0.sh`'s `--include-scope` union runs it
|
||||
on **every** gate invocation. No bespoke `ci/l0.sh` stage was added — `whole-repo-invariants-wired`
|
||||
fails the gate if anyone adds one, and the scope annotation is the supported mechanism.
|
||||
|
||||
It inventories every oauth2-proxy instance the repo authors (containers at any depth, plus the
|
||||
shared proxy's HelmRelease values) and resolves each one's **effective** cookie name — its declared
|
||||
name, **or oauth2-proxy's built-in `_oauth2_proxy` when it declares none**. That single modelling
|
||||
decision is the whole check: an omitted flag is not an absence, it is a *claim on a name*, and
|
||||
treating it as an absence is precisely why five reviewers read these manifests and saw nothing.
|
||||
Then: (1) no two effective names collide; (2) no name is another's name + `_csrf`, because that is
|
||||
how oauth2-proxy derives the CSRF cookie and two such session names look distinct while their CSRF
|
||||
cookies collide; (3) the default name has exactly one owner, and it is the one registered in
|
||||
`SHARED_DEFAULT_COOKIE_OWNER`; (4) every non-registered instance is host-scoped; (5)
|
||||
`catalog/render.py` derives rather than lists; (6) the **shipped** renderer is proven end-to-end on
|
||||
two synthetic ServiceDefinitions run as a subprocess.
|
||||
|
||||
**On the shared proxy — "distinct" or "explicitly declared"?** The brief asked me to decide, and
|
||||
the answer is *neither, quite*. The shared forward-auth proxy at `platform/oauth2-proxy/` **must
|
||||
keep** `_oauth2_proxy`: it is the single session every forward-auth-gated surface without its own
|
||||
sidecar rides on, so renaming it signs all of them out at once. Requiring *every* instance to
|
||||
declare a name would therefore need an exemption, and an exemption is a hole: delete the shared
|
||||
proxy and whichever app next omitted the flag would silently inherit the default with the rule
|
||||
still satisfied. So the assertion is plain **distinctness over effective names**, with the default
|
||||
treated as a name like any other — held by **exactly one owner, named in a shrink-only register
|
||||
with its reason**. The shared proxy needs no exemption from the rule; it is simply the registered
|
||||
holder of that one name. A stale register entry FAILS (the exemption would outlive its object) and
|
||||
an unregistered holder FAILS (no silent heirs). Both were provoked and are quoted below.
|
||||
|
||||
**On host-scoped `--cookie-domain` — overreach, or not?** Asserted, and here is the argument both
|
||||
ways. *Against:* distinct names alone close the eviction bug completely — two differently-named
|
||||
cookies do not overwrite each other however wide their domains are — so this assertion fixes
|
||||
nothing (1) has not already fixed, and a check that fails a legitimate configuration is a check
|
||||
someone eventually disables. *For, and this wins:* a zone-wide cookie is **transmitted by the
|
||||
browser to every host in the zone**. qBittorrent's session token was being sent to pihole, radarr,
|
||||
sonarr and every other name under the zone, including hosts behind no proxy at all — a
|
||||
session-credential disclosure across trust boundaries that nothing needs. And it is what makes a
|
||||
*future* name collision catastrophic rather than contained: with host-scoped cookies, two
|
||||
instances that accidentally share a name cannot see each other's cookie at all. Defence in depth
|
||||
on the exact axis that already failed once. The escape hatch is the same register, by name and
|
||||
with a reason, rather than a loosened rule.
|
||||
|
||||
### The jackett boundary, and why it could not hold
|
||||
|
||||
`apps/wave-0/jackett.yaml` carries the identical defect and was initially assigned to a concurrent
|
||||
branch, with an instruction to me not to touch it. **That separation is not achievable** — the
|
||||
ownership split was drawn across a generated file — and I did not discover this by reasoning about
|
||||
it, I provoked it. With the renderer fixed and jackett's golden left at its `main` content,
|
||||
`./ci/render-golden.sh` exits **1**:
|
||||
|
||||
[fail] GOLDEN DRIFT: apps/wave-0/jackett.yaml does not match the canonical render:
|
||||
- - --cookie-domain=.${BASE_DOMAIN}
|
||||
+ - --cookie-name=_oauth2_proxy_jackett
|
||||
+ - --cookie-domain=jackett.${BASE_DOMAIN}
|
||||
[fail] render==golden check FAILED
|
||||
|
||||
Jackett's golden is an *output* of the file that contains the defect. There are exactly three
|
||||
options and two of them are worse: hand-edit four goldens and leave the generator broken (fails
|
||||
`render-golden`, and reverts on the next render); add a per-service carve-out to the renderer so
|
||||
jackett alone keeps defaulting (ships the exact "list, don't derive" anti-pattern this check
|
||||
exists to forbid, and leaves jackett broken *by design*); or fix the generator and regenerate all
|
||||
five. I took the third. Jackett's golden here is **machine output of a one-line generator change**,
|
||||
not an independent edit of Mira's file, and it is byte-identical to what her branch must produce.
|
||||
|
||||
**Consequence:** the jackett branch has no cookie work left in it — this commit fixes jackett
|
||||
identically and by the same mechanism, and both branches independently arrived at the *same two
|
||||
values* (`_oauth2_proxy_jackett`, `jackett.${BASE_DOMAIN}`), which is a good sign for the
|
||||
diagnosis. Its hand-edit of the rendered file will fail `render-golden` on its own terms, so it
|
||||
should be closed rather than rebased. **What must not be lost with it** is its evidence: the
|
||||
interleaved radarr/jackett log showing `removing session` deleting a clean session 26s after login,
|
||||
and the `expires − created = 300.000s` measurement that produced `--cookie-refresh` here. Both are
|
||||
carried into this entry and into the renderer's comments; the branch itself is now redundant, the
|
||||
work behind it is not.
|
||||
|
||||
**The general lesson, which is worth more than this bug:** an ownership split across
|
||||
`apps/wave-*/*.yaml` cannot hold, because those files are *outputs*. Two agents assigned two
|
||||
rendered apps are assigned the same function and will collide in `render-golden` no matter how
|
||||
carefully they stay in their own files. Ownership of a rendered app is ownership of
|
||||
`catalog/render.py`, and briefs should say so.
|
||||
|
||||
### What this branch deliberately does NOT do
|
||||
|
||||
- **`--api-route=^/api/` is NOT added**, and not because of scope discipline alone: it would be
|
||||
**wrong** renderer-wide. It was measured on jackett, and **PiHole does not serve `/api/`** — so
|
||||
a renderer constant applies one app's path shape to five. Doing it properly means declaring API
|
||||
path prefixes per service, and the `auth` block is `additionalProperties: false`, so that is a
|
||||
*schema change*. Separate PR. (Note the flag is **`--api-route`, singular** — verified in the
|
||||
pinned binary's own `--help`. The plural `--api-routes` does not exist in v7.8.1 and the proxy
|
||||
refuses to start with it. That error was propagating through briefs; it stops here.)
|
||||
- **`--session-cookie-minimal` is NOT added** and must not be, as a cookie-size fix: it is
|
||||
**mutually exclusive** with `--cookie-refresh` (*"cookie_refresh > 0 requires oauth tokens in
|
||||
sessions"*). Dropping `--pass-access-token` is a least-privilege argument, never a size one.
|
||||
- **Metrics are NOT scraped, only served.** The flag and container port are the prerequisite;
|
||||
a container port is not a Service port. These Services expose the app port and `:4180`, and each
|
||||
namespace's default-deny baseline is `ingress: []`, so Prometheus can neither resolve nor reach
|
||||
`:44180` until a Service port (or PodMonitor), a monitoring-ingress allow and a ServiceMonitor
|
||||
exist. That wiring is someone else's follow-up and I have deliberately not faked its start.
|
||||
**Nobody should read this commit as "the sidecars have metrics now."**
|
||||
- **The new check was NOT widened to cover cookie-refresh or metrics.** Its subject is cookie
|
||||
*identity*; those two share neither its inputs nor its failure mode. Bundling them would make
|
||||
the check's name a lie and couple unrelated ratchets, so a future metrics decision could only
|
||||
be expressed by weakening a *cookie* check. The cost of that discipline is named honestly in
|
||||
the follow-ups: those two flags are currently **ungated**.
|
||||
|
||||
## What I verified
|
||||
|
||||
- **The premise, in-cluster, rather than inherited from the handoff** (CLAUDE.md §2b). Read-only,
|
||||
`kubectl --context k3s-prod`, through the pinned toolbox.
|
||||
- **Seven** `*-oauth2-proxy` Secrets carry a `cookie-secret`: crafty, jackett, pihole,
|
||||
qbittorrent, radarr, sonarr, and the shared `oauth2-proxy-credentials`. **All seven values are
|
||||
DISTINCT.** Compared without printing anything: each value was hashed **in-cluster-side script,
|
||||
never echoed**, and only the first 8 hex characters of each `sha256` were emitted — enough to
|
||||
say same-or-different, recovering nothing. Secret *key names* were listed; no `.data` or
|
||||
`.stringData` was printed (CLAUDE.md §6). **So no instance can decrypt another's cookie — it can
|
||||
only overwrite it. The premise holds.**
|
||||
- **The default cookie names really were in use.** Of 8 live oauth2-proxy containers, **6 declare
|
||||
no `--cookie-name` at all** — jackett, pihole, qbittorrent, radarr, sonarr, and both replicas of
|
||||
the shared proxy — so all six emit `_oauth2_proxy` / `_oauth2_proxy_csrf`. All six also carry
|
||||
`--cookie-domain=.home.keithjackson.dev`. **One cookie pair, six independent instances.**
|
||||
- Crafty's live pod (`crafty-647c7d8cc5-5spv9`) already carries
|
||||
`--cookie-name=_oauth2_proxy_crafty` and `--cookie-domain=crafty.home.keithjackson.dev`, so
|
||||
PR #22 is merged and reconciled. It is the only compliant per-app instance on the cluster.
|
||||
- **THE COLLISION IS REAL. IT IS NOT ESTABLISHED AS THE CAUSE OF THE REPORTED FAILURES.** These
|
||||
are two different claims and only the first is supported. I was handed counter-evidence —
|
||||
crafty, the app already fixed, showed the *worst* rejection ratio of the set over a 24h Loki
|
||||
window (`crafty 81:1`, `radarr 67:1`, `jackett 39:4`) — and it is right to refuse the causal
|
||||
claim. What I could do was bound the window. Read from **each sidecar's own log** rather than
|
||||
Loki, because `kubectl logs` on a pod that has not restarted cannot reach a pod older than
|
||||
itself, so a window inside the pod's age is inside the fixed spec's lifetime *by construction*:
|
||||
|
||||
namespace age(h) rst reject success ratio cleared badsig
|
||||
crafty 13.9 0 3 0 n/a 0 0 <- FIXED spec, whole life
|
||||
radarr 422.5 0 155 1 155.0:1 8 8
|
||||
jackett 422.6 0 39 4 9.8:1 1 1
|
||||
pihole 395.7 0 0 0 0:0 0 0
|
||||
qbittorrent 404.1 0 0 0 0:0 0 0
|
||||
sonarr 422.5 0 0 0 0:0 0 0
|
||||
oauth2-proxy 422.6 0 0 0 0:0 0 0 (both replicas)
|
||||
|
||||
`reject` = `No valid authentication in request`; `success` = `AuthSuccess`;
|
||||
`cleared` = `removing session` (oauth2-proxy's `ClearSession`); `badsig` = `cookie signature
|
||||
not valid`. Window `--since=24h`, which exceeds every pod's age only for crafty — whose pod is
|
||||
13.9h old with **0 restarts**, so these are its complete post-fix figures.
|
||||
|
||||
Then I read crafty's four auth-relevant lines rather than counting them, because at n=3 the
|
||||
content is the evidence:
|
||||
|
||||
03:57:20 Cookie settings: name:_oauth2_proxy_crafty secure(https):true httponly:true
|
||||
expiry:168h0m0s domains:crafty.home.keithjackson.dev path:/
|
||||
04:02:55 No valid authentication in request. Initiating login.
|
||||
15:22:39 No valid authentication in request. Initiating login.
|
||||
15:22:44 No valid authentication in request. Initiating login.
|
||||
|
||||
6743 log lines; those are all four. **Zero** `removing session`, **zero** `cookie signature
|
||||
not valid`, zero anything else.
|
||||
|
||||
Four conclusions, kept separate:
|
||||
1. **`crafty 81:1` is a window artefact. It is not evidence against the fix and must not be
|
||||
cited as such.** The fixed pod emitted **3** rejections in 13h 52m; the 81 came from a 24h
|
||||
window straddling it, i.e. from the pre-fix process.
|
||||
2. **The eviction FINGERPRINT is gone where the fix is deployed.** `cookie signature not
|
||||
valid` + `removing session` is what a clobbered instance logs — a cookie *is* present,
|
||||
written with a key this instance does not hold, so it cannot decrypt it and then
|
||||
`ClearSession` expires it on its own `--cookie-domain`. Fixed crafty: **0 and 0** across
|
||||
its whole life. Unfixed radarr: **8 and 8** in 12h. Unfixed jackett: 1 and 1. That is a
|
||||
real, directional signal and it is the strongest evidence available that the fix works.
|
||||
3. **It is still not a demonstrated successful sign-in.** Crafty logged **0 `AuthSuccess`** —
|
||||
so what is shown is *absence of the failure mode*, not a session observed working
|
||||
end-to-end. Those three rejections are consistent with benign first-hits (one client, two
|
||||
of them 5s apart, i.e. a page plus a `/ws` reconnect). I am comfortable saying the fix
|
||||
works; I am not willing to say it has been watched working, and the distinction is the
|
||||
whole reason this section exists.
|
||||
4. **The collision is a MINORITY of the rejection volume.** radarr: 8 of 155 (5%) carry the
|
||||
fingerprint; ~95% carry **no cookie at all**. Those are the 5-minute session expiry and
|
||||
`302`-on-XHR — the first of which this branch now also fixes, the second of which it
|
||||
deliberately does not.
|
||||
|
||||
Two cautions about the instrument. `reject` ranks **chattiness, not breakage**: radarr's are
|
||||
`/api/v3/command`, `/api/v3/queue/details` and `/signalr/negotiate` polls, so a busy *arr
|
||||
inflates the count with nobody inconvenienced. And that same polling is the *amplifier* that
|
||||
made this intermittent — every poll carrying an undecryptable cookie calls `ClearSession`
|
||||
again on the zone-wide domain, so **a left-open Radarr tab deleted Jackett's cookie every few
|
||||
seconds, from another namespace, with no user action at all.** That is why signing in again
|
||||
never helped for longer than the gap between two polls. It is now a comment in the renderer,
|
||||
because it is the part of this bug that no one would reconstruct from the flags alone.
|
||||
- **The four target files are generated, not authored.** Rendering `catalog/` with the canonical
|
||||
command into a temp dir and diffing against `apps/` reported all 10 onboarded goldens
|
||||
byte-identical before any change. Hand-editing one is drift by definition.
|
||||
- **`render-golden` FAILS if jackett is excluded** — provoked (output quoted above), rc **1**.
|
||||
Restoring the regenerated golden returns it to `all 10 onboarded service golden(s) reproduce`,
|
||||
rc **0**.
|
||||
- **Every flag verified against the PINNED v7.8.1 binary**, not against docs, memory, or the
|
||||
shared proxy (which runs **v7.15.3** and therefore could not vouch for v7.8.1):
|
||||
`podman run --rm quay.io/oauth2-proxy/oauth2-proxy:v7.8.1 --help`.
|
||||
- `--cookie-name string ... (default "_oauth2_proxy")` — **this confirms the constant my whole
|
||||
check is built on, from the binary itself.** The check's central claim, that an omitted flag
|
||||
is a *claim on a name* rather than an absence, now rests on the implementation's own help
|
||||
text rather than on my reading of its source.
|
||||
- `--cookie-refresh duration refresh the cookie after this duration; 0 to disable` — confirms
|
||||
the default really is disabled.
|
||||
- `--metrics-address string` — present in v7.8.1, so the new flag cannot crashloop the pods.
|
||||
- `--api-route strings` — **singular**, confirming the brief's plural was wrong.
|
||||
- `--session-cookie-minimal` — exists, and is the flag that conflicts with `--cookie-refresh`.
|
||||
- **44180 is the platform's existing convention, measured, not chosen.** The shared proxy already
|
||||
runs `--metrics-address=0.0.0.0:44180` with a `metrics` container port, a `metrics:44180`
|
||||
Service port, a ServiceMonitor whose endpoint is `metrics`, and **2 ready endpoint addresses** —
|
||||
i.e. it demonstrably works on this platform.
|
||||
- **`catalog/render.py --selftest`: PASS**, now including the two-service distinctness assertion,
|
||||
a `--cookie-refresh` assertion that PARSES the duration and requires `0 < refresh < 300s` (so
|
||||
`2m30s` -> `150s` -> `2.5m` all stay green but a silent widening past the token lifetime does
|
||||
not), a `--session-cookie-minimal` mutual-exclusion guard, and both metrics assertions.
|
||||
- **The check is selected by the whole-repo union.** `conformance/run.sh --layer l0 --requirements
|
||||
r-nosuchreq-99 --include-scope whole-repo --print-selection` lists
|
||||
`tests/l0-static/oauth2-proxy-cookie-names-distinct` among 20 tests — i.e. it runs even when the
|
||||
touched-requirement filter matches nothing, which is the blind spot it was scoped for.
|
||||
- **The check was tested in BOTH directions**, against throwaway trees outside the repo, never by
|
||||
mutating the branch:
|
||||
- **Compliant tree (this branch): rc 0, 39 assertions, 7 instances audited.**
|
||||
- **Pre-fix tree** (`git archive HEAD` + this test dropped in): **rc 1, 26 failures**, naming all
|
||||
five violators, the 6-way `_oauth2_proxy` collision, the zone-wide domains, the renderer
|
||||
emitting no `--cookie-name`, and both synthetic fixtures rendering `None`.
|
||||
- **Targeted mutations on a compliant copy**, one assertion class each, all fired and nothing
|
||||
else did: duplicate name (sonarr←radarr) → 1 failure; CSRF derivation
|
||||
(`_oauth2_proxy_sonarr_csrf` as a session name) → fired; zone-wide `--cookie-domain` on pihole
|
||||
→ 2 failures; crafty's `--cookie-name` deleted → unregistered-default-heir + 2-way collision;
|
||||
shared proxy renamed → **stale register entry** failure.
|
||||
- A **false positive was found and fixed** by this testing: `platform/oauth2-proxy/forward-auth-cec.yaml`
|
||||
contains `serviceName: oauth2-proxy/oauth2-proxy:80` — an Envoy `namespace/service:port`
|
||||
reference, not an image — which the first inventory scan counted as an unaudited instance. The
|
||||
scan is now anchored on `image:`, and the near-miss is documented in the check.
|
||||
- **Gate:** toolbox `./ci/gate.sh` — exit code and stage list in the next section, because it is
|
||||
not a pass and that belongs there.
|
||||
|
||||
## What I did NOT verify
|
||||
|
||||
- **Local gate: rc 3 (UNDETERMINED), not 0.** Green: `spec-immutability`, `decisions-log`, `lint`,
|
||||
`l0`, `render-golden`, `l1`, `approval-policy`. **Skipped:** `gitea-settings` (no
|
||||
`SETTINGS_API_TOKEN` locally) and **L2/L3 conformance** (`CLUSTER_AVAILABLE` unset). Nothing
|
||||
FAILED; nothing runtime was certified either. **rc 3 is UNDETERMINED, never green.** Touched
|
||||
subset `R-GITOPS-02 R-IDP-02 R-IDP-06 R-NET-01 R-NET-05 R-NET-06 R-NET-07 R-PLAT-02 R-TEST-01
|
||||
R-TEST-02 R-TEST-03 R-TEST-04` — **UNVERIFIED** at L2/L3, all twelve.
|
||||
- **`spec-immutability` was skipping because of a BUG IN MY TOOLING, not the environment.** The
|
||||
worktree ENGINE shim in circulation mounts only the main checkout's `.git`, and `dev.sh`
|
||||
computes the spec path from its own `REPO_ROOT` — which in a worktree is
|
||||
`.claude/worktrees/agent-*/`, so `../home-network-k3s` does not resolve and it adds no spec
|
||||
mount at all. The gate then reported the skip, and the skip read as "the one tolerated
|
||||
environmental skip" while actually being a path bug being waved through. Adding
|
||||
`-v /var/home/keith/source/home-network-k3s:/home-network-k3s:ro -e SPEC_REPO=/home-network-k3s`
|
||||
makes **R-TEST-06 actually run and pass**. Worth putting in every worktree agent brief; the
|
||||
two-mount shim in CLAUDE.md §3 should say so.
|
||||
- **CI: `gate / gate (pull_request)` reports FAILURE on `75a4029`, for exactly ONE reason, and it
|
||||
is not this change.** Read from the job log rather than the tick:
|
||||
|
||||
[fail] approval-policy: a sensitive change lacks a non-author approval -> blocks merge
|
||||
[fail] MERGE GATE: BLOCKED
|
||||
|
||||
Every substantive stage is **green in CI** — `gitea-settings: pass` (CI has the token my local
|
||||
run does not), `decisions-log`, `lint`, `l0`, `render-golden`, `l1`. Skipped: `spec-immutability`
|
||||
(the workflow does not check the spec repo out — the documented tolerated skip) and
|
||||
`conformance` (declared). So CI is a **stricter, more complete** run than my local one on every
|
||||
axis except the spec repo, and it agrees.
|
||||
`infra/gitea/approval-policy.json` lists `conformance/**` as sensitive — *"the tests and the
|
||||
harness; changing them changes what green MEANS"* — so this PR is **fail-closed pending a
|
||||
non-author approving review**, which is CLAUDE.md §1 working, not a defect to route around.
|
||||
**The red tick is the policy, and it clears when a human reviews it.** Note also that
|
||||
`gate / conformance (pull_request)` reports `success` with description **"Has been skipped"** —
|
||||
the precise trap CLAUDE.md §2b names; it certifies nothing.
|
||||
- **A SUCCESSFUL SIGN-IN, ANYWHERE.** The strongest post-fix evidence is the *absence* of the
|
||||
eviction fingerprint on crafty (0 `removing session`, 0 `cookie signature not valid`, across
|
||||
13h 52m) against radarr's 8-and-8. That is directional and I believe it. But crafty logged
|
||||
**0 `AuthSuccess`** in the same window, so no session has been observed working end to end on a
|
||||
fixed instance. Whoever closes this out should re-measure crafty **after real sign-in traffic**.
|
||||
- **No browser round-trip by me at all**, for any of the three defects. Reproducing the collision
|
||||
needs two gated apps visited in one session. The concurrent jackett branch does carry an observed
|
||||
log-level reproduction (interleaved radarr/jackett `removing session` deleting a clean session
|
||||
26s after login) — real evidence for the MECHANISM, and it is theirs, not mine.
|
||||
- **`--cookie-refresh=2m30s` has never been exercised.** The value is derived from a measured
|
||||
300.000s access-token lifetime and is asserted to sit inside `(0, 300s)`, but no session has been
|
||||
held past the five-minute cliff to watch a refresh succeed. If Keycloak's refresh-token or SSO
|
||||
idle timeouts are shorter than expected, the symptom moves rather than disappears. **This is the
|
||||
fix most likely to need a second pass**, and the first thing to watch after merge.
|
||||
- **The `metrics` port has never been fetched.** The listener is asserted to start by flag
|
||||
presence only; nobody has curled `:44180` on a rendered pod. And it is unreachable by design
|
||||
until the Service/policy/ServiceMonitor wiring lands — see above.
|
||||
- **Nothing was applied.** No `kubectl apply/edit/patch`, no `flux reconcile`. Cluster access was
|
||||
read-only throughout; the one container I ran was the oauth2-proxy image locally under podman to
|
||||
read its `--help`. The running pods are still the pre-fix spec. **Merging is what deploys**, and
|
||||
it will roll five Deployments; every user's existing session on those five apps is invalidated by
|
||||
the cookie rename and they will re-authenticate once. Radarr/sonarr/qbittorrent use
|
||||
`strategy: Recreate` on RWO volumes, so there is a brief gap per app. Pick the moment — and
|
||||
note this now changes **three** flags plus a port on all five, so a bad `--cookie-refresh` value
|
||||
would surface as five apps misbehaving, not one.
|
||||
- **The live flags are not re-measured by the check.** git is not the cluster. I measured the live
|
||||
pods by hand, once; nothing in the gate re-reads them, so hand-applied drift would be invisible
|
||||
to it. A live-state assertion is L2 and is not written.
|
||||
- **`--cookie-refresh` and `--metrics-address` are UNGATED, and I found out why.** Nothing in
|
||||
`ci/` or `.gitea/` runs `catalog/render.py --selftest` — I grepped for it. So the twenty-odd
|
||||
assertions in that selftest, including the six I just added, are dev-time only: a future edit
|
||||
that drops either flag *and* re-renders the goldens would pass the entire merge gate.
|
||||
`render-golden` catches accidental drift (golden vs renderer) but not a deliberate removal of
|
||||
both together, and my L0 check deliberately does not cover these two flags. **The repo already
|
||||
has the precedent for the fix and the one-line shape of it:**
|
||||
`conformance/tests/l0-static/auditability-proofs-producible/check.sh:291` drives
|
||||
`conformance/lib/<tool> --selftest` as a subprocess for exactly this reason — its own header
|
||||
says it does so "because nothing [else] drives them". Adding `catalog/render.py` to that list,
|
||||
or giving the renderer its own scope-declaring L0 test, closes it. I did not do it here because
|
||||
it widens an already review-blocked PR into a third subject; it is the highest-value follow-up
|
||||
in this entry.
|
||||
- **Log-derived counts are a weak instrument and I did not strengthen them.** `kubectl logs
|
||||
--since=` returns what the kubelet still retains, so an old, busy container's window may be
|
||||
truncated; and `No valid authentication in request` counts unauthenticated first hits and XHR
|
||||
polls indiscriminately. The `cookie signature not valid` / `removing session` pair is the only
|
||||
collision-specific signal in there, and it is the one I leaned on. None of this is gated —
|
||||
there is no alert on it and no L2 check reads it.
|
||||
- **The cookie-secrets' distinctness is not gated either.** It was measured out of band and is
|
||||
asserted nowhere. If they were ever made identical the collision would become harmless — but
|
||||
sharing one session-encryption key across trust zones is a worse idea than the bug it would mask,
|
||||
so the check does not offer it as an alternative and does not look for it.
|
||||
- **The shared proxy's own configuration is unchanged and untested by me.** I did not touch
|
||||
`platform/oauth2-proxy/**` (owned by another agent this cycle); the check only reads it.
|
||||
- **Nothing about group authorization, CSRF behaviour, or session contents.** Names and domains only.
|
||||
- **`vars.CLUSTER_AVAILABLE` is still unset in CI**, so the PR's green tick — if it goes green —
|
||||
proves the L0/L1 offline subset and nothing about runtime.
|
||||
@@ -0,0 +1,174 @@
|
||||
---
|
||||
date: 2026-09-30
|
||||
branch: feat/plex-quicksync
|
||||
title: Plex opts into `hardware.gpu: intel-quicksync` — the device mount is GitOps, the Plex-side toggle is a documented manual step, and hardware transcoding has NOT been observed working
|
||||
---
|
||||
|
||||
## What changed
|
||||
|
||||
Three things, and the third is the point of the branch.
|
||||
|
||||
1. **`catalog/plex.yaml`** — added the four lines the whole feature reduces to:
|
||||
|
||||
```yaml
|
||||
hardware:
|
||||
gpu: intel-quicksync
|
||||
```
|
||||
|
||||
placed between `env` and `exposure` to match the schema's own property order, with ~48 lines of
|
||||
comment in the file's established style. The comment records: the field is **intent, not
|
||||
mechanism** (a later move to the Intel device plugin is a `render.py` change and this block does
|
||||
not move); that `/dev/dri/renderD128` is present on **all three** prod nodes with `i915` bound;
|
||||
**why a `supplementalGroups` entry would be inert** (the image's cont-init hook does the group
|
||||
work inside the container and `s6-setuidgid` replaces the process group list from `/etc/group`);
|
||||
why `DirectoryOrCreate` is the load-bearing safe failure mode (`apps-wave-0` is `wait: true` with
|
||||
`apps-wave-1` depending on it, so a failed mount would stall the whole app tier for a feature
|
||||
that is not load-bearing); and that the Plex-side toggle is a **separate manual operator step**,
|
||||
pointing at the new runbook.
|
||||
|
||||
2. **`apps/wave-0/plex.yaml`** — re-rendered. Six added lines, nothing else, in the whole `apps/`
|
||||
tree:
|
||||
|
||||
```
|
||||
- name: dri
|
||||
mountPath: /dev/dri
|
||||
- name: dri
|
||||
hostPath:
|
||||
path: /dev/dri
|
||||
type: DirectoryOrCreate
|
||||
```
|
||||
|
||||
3. **`runbooks/plex-quicksync.md`** (new), registered in `runbooks/kustomization.yaml`'s
|
||||
`configMapGenerator` as `- plex-quicksync=plex-quicksync.md` and indexed in `runbooks/README.md`
|
||||
under "Operator procedures". **The file alone would have been a 404** — the pod serves a
|
||||
ConfigMap, not the git tree, and `conformance/tests/l0-static/alert-runbooks-resolvable` exists
|
||||
precisely because "the file is in git" and "the URL resolves" are different claims. The page
|
||||
covers: the GitOps/manual boundary as a table; the toggle (Settings → Transcoder → "Use hardware
|
||||
acceleration when available", preference key `HardwareAcceleratedCodecs`); the real neighbouring
|
||||
keys and the widely-cited one that **does not exist**; that the only proof is `Transcode (hw)` on
|
||||
the Plex dashboard **and why no alert is possible**; the device-cgroup failure to expect first
|
||||
with a concrete open() diagnostic and the Intel-device-plugin fallback; and the floating-`:latest`
|
||||
caveat with a digest check.
|
||||
|
||||
**Stacked on PR #27 (`feat/sd-hardware-capability`), which is open and unmerged.** This branch was
|
||||
cut from it rather than from `main`, so its one commit (`d9baa2a`, the schema + renderer + contract)
|
||||
is in this PR's range. That is deliberate: merging #27 is not an agent's action (CLAUDE.md §1), and
|
||||
duplicating its changes here would have been worse than showing them. **This must merge after #27.**
|
||||
|
||||
Not done, deliberately: no `--emit-waves` (it writes into `clusters/base/`, a sensitive path that
|
||||
would force a human reviewer, and it is a no-op unless the *set* of files under `apps/` changes — it
|
||||
does not here). No `podSecurityContext`. No `nodeSelector`. No alert rule. No `kubectl apply`.
|
||||
|
||||
### Two follow-ups left unclaimed, with the reasoning
|
||||
|
||||
- **Transcode scratch is on the wrong medium, and it is invisible.** The image sets
|
||||
`TranscoderTempDirectory=/transcode`, which is **not a volume** — it is the container's writable
|
||||
overlay, i.e. the node's `/data`. That is NVMe on prod-2 but **SATA on prod-1 and prod-3**, so
|
||||
transcode scratch performance changes silently on reschedule, with nothing reporting it. Both
|
||||
candidate fixes need a **schema** change and so neither belongs in this PR: pin Plex to prod-2
|
||||
(needs a `nodeSelector` field the schema does not have), or give `/transcode` a `medium: Memory`
|
||||
emptyDir and transcode in RAM (~31 GiB per node; needs an emptyDir field the schema does not
|
||||
have). **The RAM option is the better one** — it removes the node asymmetry outright instead of
|
||||
working around it — which is exactly why it deserves its own PR and its own review rather than
|
||||
being smuggled in behind a four-line catalog change. Documented in the runbook's "Known gaps".
|
||||
- **GuC/HuC firmware is not loaded** (`i915.enable_guc=0`). The cost is low-power H.264 **encode**
|
||||
quality only; decode and this feature are unaffected. Changing it needs a kernel parameter plus a
|
||||
**serialized rolling reboot** — an operator action under CLAUDE.md §5, on a cluster with a
|
||||
documented history of that going badly (a prod node hung on a warm reboot on 2026-09-03). Kept
|
||||
well clear of this PR.
|
||||
|
||||
### Rebase note (2026-10-01, Ruth Okonkwo) — resolution only, no redesign
|
||||
|
||||
Rebased onto `main` @ `08deb20` (was cut from `0fbf544`), restacked on the rebased
|
||||
`feat/sd-hardware-capability` (PR #27) so the stack stays coherent — this branch still carries
|
||||
exactly PR #27's commit plus its own, with no duplicate of anything already in `main`. The branch
|
||||
ref was force-updated with `--force-with-lease`, so PR #32 is the same PR with a new head.
|
||||
|
||||
**The `catalog/render.py` conflict disappeared** once PR #27 was rebased; it was never this
|
||||
branch's own conflict. **One conflict remained, in `runbooks/README.md`, purely textual.** PR #30
|
||||
(`fix/jackett-oauth2-proxy-session`, merged as `08deb20`) appended an `oauth2-proxy-session` index
|
||||
entry under *Operator procedures* at the same point this branch appended its `plex-quicksync` entry.
|
||||
**Both entries were kept, in full.** Ordering follows this file's existing convention, which is
|
||||
grouped/narrative rather than alphabetical (the *Nodes* section runs down/io/memory/disk/cpu):
|
||||
`plex-quicksync` sits next to `plex-claim` as the other Plex page, leaving `oauth2-proxy-session`
|
||||
adjacent to `crafty-admin-credential`, which cross-references the same sidecar gate. The change to
|
||||
`runbooks/README.md` is an 8-line pure insertion, so PR #30's entry is untouched.
|
||||
`runbooks/kustomization.yaml` auto-merged and **both** `configMapGenerator` entries are present.
|
||||
|
||||
Re-verified after the resolution, on the rebased stack: `--selftest` PASS (rc 0); a full re-render
|
||||
reproduces every committed golden byte-identically; and the **only** `apps/` diff against the new
|
||||
`main` is `apps/wave-0/plex.yaml`, confined to the expected six lines (the `dri` mount and its
|
||||
`DirectoryOrCreate` hostPath volume). No other service's golden moved — PR #28 added ~176 lines of
|
||||
oauth2-proxy sidecar rendering, and this confirms by measurement rather than assumption that Plex
|
||||
(`auth: native`, no sidecar) is unaffected by it.
|
||||
|
||||
## What I verified
|
||||
|
||||
- **The render is exactly the intended change, and nothing else moved.** After
|
||||
`python3 catalog/render.py --all catalog -o apps --wave-layout --no-image-automation` (exit 0,
|
||||
silent): `git diff --stat apps/` = `apps/wave-0/plex.yaml | 6 ++++++`, and
|
||||
`git status --porcelain apps/` = a single ` M apps/wave-0/plex.yaml`. Read line by line, the diff
|
||||
is the `dri` volumeMount and the `dri` hostPath volume, and nothing more. **Specifically checked:**
|
||||
the same invocation also rewrites `apps/wave-1/keycloak.yaml` and `apps/wave-0/vm-demo.yaml`, which
|
||||
are git-tracked but excluded from the golden comparison by `RENDER_GOLDEN_NOT_ONBOARDED` in
|
||||
`ci/render-golden.sh` — a spurious diff there would have landed **ungated**. Neither file shows a
|
||||
diff; whole-repo `git status --porcelain` lists only `apps/wave-0/plex.yaml` and `catalog/plex.yaml`
|
||||
plus my new files.
|
||||
- **The rendered output matches what the contract promises** — `schemas/renderer-contract.md`
|
||||
`### hardware` says exactly two things (a `/dev/dri` hostPath with `type: DirectoryOrCreate` and a
|
||||
read-write mount on the app container) and exactly two things appeared. No `securityContext`, no
|
||||
`nodeSelector`, no resources in the diff.
|
||||
- **The runbook is projected into the server, not merely committed.** `runbooks/kustomization.yaml`
|
||||
carries `- plex-quicksync=plex-quicksync.md`, whose shape matches the generator-entry regex the
|
||||
L0 check parses; the extensionless key is what makes `/runbooks/plex-quicksync` a plain static
|
||||
lookup. Also indexed in `runbooks/README.md`.
|
||||
- **The runbook clears the L0 quality floor** that `alert-runbooks-resolvable` applies to
|
||||
alert-linked pages, even though nothing links to this one (it is advisory for an unreferenced
|
||||
page): all four required sections (`## What this means here`, `## First commands`,
|
||||
`## Likely causes, most likely first`, `## When to escalate or stop`), well over the 1500-byte
|
||||
floor, `dev.sh` used throughout, and `--context k3s-prod` on every command.
|
||||
- **The gate.** See the run recorded in the PR description for the per-stage table and the real rc.
|
||||
- **No secret was printed.** Every `Preferences.xml` read in the runbook greps one named key, and the
|
||||
page states explicitly why: the file holds `PlexOnlineToken`. Nothing was `sops -d`'d.
|
||||
|
||||
## What I did NOT verify
|
||||
|
||||
**Hardware transcoding has NOT been observed working.** Nothing in this branch is evidence that it
|
||||
does. A green gate here proves the manifest renders and the golden matches — it proves nothing about
|
||||
runtime behaviour, and `vars.CLUSTER_AVAILABLE` is unset so L2/L3 conformance did not run at all.
|
||||
|
||||
- **The device-cgroup question is OPEN, and it is the largest remaining technical risk.** Whether
|
||||
containerd on this k3s build permits a **non-privileged** pod to `open()` a bind-mounted character
|
||||
device is **UNVERIFIED**. It can only be settled by deploying and trying. The container inits as
|
||||
uid 0, which settles *file* permissions, but uid 0 does **not** bypass the cgroup device gate — and
|
||||
the two failures look identical from the outside. If it is refused, the fallback is the Intel
|
||||
Device Plugin (`gpu.intel.com/i915`), which is a `catalog/render.py` change plus a re-render with
|
||||
**no ServiceDefinition edit**; `privileged: true` is not available (Kyverno
|
||||
`restrict-host-namespaces` is Enforce). The runbook has the diagnostic sequence.
|
||||
- **The Plex-side toggle was still unset at the time of writing.** `HardwareAcceleratedCodecs` was
|
||||
**absent** from `Preferences.xml` — the toggle has never been enabled on this server. Until an
|
||||
operator ticks it in the UI, this branch changes the pod spec and **nothing about transcoding**.
|
||||
That is the whole reason the runbook exists.
|
||||
- **No metric, so no alert, so no automated detection — permanently.** `catalog/plex.yaml` declares
|
||||
no `metricsEndpoint`, so Plex renders no ServiceMonitor; there is no transcode-mode signal for a
|
||||
rule to evaluate. A silent degradation to software transcoding (a node losing its iGPU, the
|
||||
`DirectoryOrCreate` path) will **not** be reported by anything. I deliberately did not add an
|
||||
alert: with no metric behind it, it could never fire, and this repo has been burned at scale by
|
||||
exactly that. Closing the gap honestly means an exporter first.
|
||||
- **I did not re-derive this session's node or in-container measurements.** The three-node
|
||||
`renderD128`/`i915`/GID-992 facts, the cont-init hook behaviour, the `s6-setuidgid` group
|
||||
replacement, the `PLEX_PREFERENCE*` absence, the six `setPref` keys and the
|
||||
`HardwareAcceleratedCodecs` string were established earlier in this session and are cited, not
|
||||
re-measured, by me. Per CLAUDE.md §2b, treat this file as a record of belief and re-measure against
|
||||
the cluster before relying on any of it.
|
||||
- **The image is a floating tag.** Everything known about the hook came from digest
|
||||
`sha256:e0ab27395614…` (`plexmediaserver 1.43.4.10903`), while the Deployment runs
|
||||
`plexinc/pms-docker:latest` with `imagePullPolicy: Always`. A silent pull can invalidate the
|
||||
"no `supplementalGroups` needed" conclusion **with no commit in this repo**. Not guarded against —
|
||||
the runbook tells the reader to re-check the hook after an image bump, which is documentation, not
|
||||
enforcement.
|
||||
- **I did not verify the live Gitea gate state** (branch protection, required contexts, runner
|
||||
online) for this PR. Per §2b that must be queried, not read out of `CLAUDE.md`'s table; I did not
|
||||
query it.
|
||||
- **`/runbooks/plex-quicksync` was not fetched.** The L0 check asserts every static precondition for
|
||||
an HTTP 200; it does not make the request, and I did not either.
|
||||
@@ -0,0 +1,204 @@
|
||||
---
|
||||
date: 2026-09-30
|
||||
branch: feat/sd-hardware-capability
|
||||
title: ServiceDefinition gains a `hardware` capability field — `gpu: intel-quicksync` mounts /dev/dri and nothing else; the enum is what keeps the mechanism swappable
|
||||
---
|
||||
|
||||
## What changed
|
||||
|
||||
A new optional top-level ServiceDefinition property, `hardware`, with one optional member,
|
||||
`gpu`, whose enum today has exactly one value: `intel-quicksync`. **No catalog entry uses it.**
|
||||
This PR delivers the capability; consuming it (Plex) is separate work on top.
|
||||
|
||||
- `schemas/servicedefinition.schema.json` — adds `hardware` (object, `additionalProperties: false`,
|
||||
`minProperties: 1`) with `gpu` (enum `["intel-quicksync"]`). Schema version bumped **1.7.0 →
|
||||
1.8.0**, changelog entry prepended to the schema `description` per that file's own convention.
|
||||
- `catalog/render.py` — two module-level constants (`DRI_VOLUME_NAME`, `DRI_DEVICE_DIR`) and the
|
||||
emission in `r_deployment`.
|
||||
- `schemas/renderer-contract.md` — new `### hardware` field-mapping section, plus cross-field
|
||||
invariant **8** in that file's numbered list.
|
||||
- `schemas/README.md` — its `**Schema version:**` line said `1.0.0`; it had been stale since 1.1.0.
|
||||
Synced to 1.8.0 and annotated that the schema `description` is the changelog of record. Added one
|
||||
bullet to "Cross-field rules" for the new enum. That was the whole of the staleness; nothing
|
||||
ballooned.
|
||||
|
||||
**What the renderer emits** for `hardware.gpu: intel-quicksync`, on the app container only (never a
|
||||
sidecar), and it is exactly two things:
|
||||
|
||||
1. a hostPath volume `dri` → `/dev/dri`, `type: DirectoryOrCreate`;
|
||||
2. a read-write `volumeMount` of `dri` at `/dev/dri` on the app container (deliberately not
|
||||
`readOnly` — VA-API opens the render node read-write).
|
||||
|
||||
**Mounting the device is sufficient.** It emits **no** `privileged: true` (Kyverno
|
||||
`restrict-host-namespaces` is Enforce on app namespaces and it is not needed), **no** `nodeSelector`
|
||||
(placement is a separate concern with its own justification), **no** resource requests (there is no
|
||||
app-container resource render path), and — the one that is worth spelling out — **no
|
||||
`podSecurityContext.supplementalGroups`**.
|
||||
|
||||
**Why no `supplementalGroups`, when `/dev/dri/renderD128` is `root:render 0660`.** An earlier
|
||||
revision of this branch merged the host `render` GID into `supplementalGroups`. **It was measured to
|
||||
be a no-op and has been removed.** In the running container on prod-2, 2026-09-30:
|
||||
|
||||
PID 1 s6-svscan Uid: 0 Gid: 0 Groups: 0
|
||||
pid 246 Plex Media Server Uid: 1000 Gid: 1000 Groups: 100 1000
|
||||
|
||||
`/etc/services.d/plex/run` ends in `exec s6-setuidgid plex …`, and the child's group list is exactly
|
||||
the `/etc/group` account-database list — root's gid `0`, which PID 1 holds, has **vanished**. That is
|
||||
a `setgroups()` that *replaces* rather than appends, so anything Kubernetes puts in
|
||||
`supplementalGroups` reaches `s6-svscan` and is discarded before the app ever starts. What actually
|
||||
works is the image's own `/etc/cont-init.d/45-plex-hw-transcode-and-connected-tuner`, which `find`s
|
||||
every char device under `/dev/dri`, `stat -c '%g'`s its GID, `groupadd`s a matching group inside the
|
||||
container when none exists, and `usermod -a -G`s the app user into it — every start, nothing
|
||||
hardcoded; `cont-init.d` completes before `services.d`, so the database is correct at exec time.
|
||||
|
||||
Shipping an inert `supplementalGroups` would have been worse than shipping nothing: it would read as
|
||||
the mechanism to whoever debugs this next. The reasoning lives in a comment **at the emission site**
|
||||
and in `renderer-contract.md`, and `render.py --selftest` now asserts the *absence* of a
|
||||
`securityContext`, so it cannot be "helpfully" added back without a test failing. A pleasant
|
||||
corollary: because the hook re-`stat`s the device every start, host GID drift is **self-healing**,
|
||||
and no GID is pinned anywhere in this repo.
|
||||
|
||||
**The design decision worth defending: the field is an enum of intent, not a device path.** The SD
|
||||
says *what class of host device the workload needs*; the hostPath and the mount exist only in
|
||||
`catalog/render.py`. Migrating to the Intel GPU device plugin (`gpu.intel.com/i915` resource
|
||||
requests, no hostPath at all) is then a renderer change with zero catalog edits. Both the schema
|
||||
`description` and the contract section say this explicitly, because the obvious "simplification" —
|
||||
letting an SD name `/dev/dri` — would push a platform implementation detail into the operator-facing
|
||||
contract and make that migration a catalog-wide edit.
|
||||
|
||||
**`DirectoryOrCreate` is load-bearing.** A node that comes back without the render device (rebuild,
|
||||
BIOS change, driver regression, hardware swap) must degrade the workload to software transcoding,
|
||||
not fail the mount: `apps-wave-0` is `wait: true` and `apps-wave-1` `dependsOn` it, so a failed
|
||||
hostPath mount stalls the entire app tier for the sake of a feature that is not load-bearing.
|
||||
`/dev` is tmpfs, so the empty directory the kubelet creates on such a node leaves nothing durable
|
||||
behind. Do not "tighten" it to `Directory`. The reasoning is recorded at the emission site.
|
||||
|
||||
### Rebase note (2026-10-01, Ruth Okonkwo) — resolution only, no redesign
|
||||
|
||||
Rebased onto `main` @ `08deb20` (was cut from `0fbf544`). Re-landed as a single cherry-pick onto
|
||||
current `main`; the branch ref was force-updated with `--force-with-lease`, so PR #27 is the same
|
||||
PR with a new head.
|
||||
|
||||
**One conflict, in `catalog/render.py`, purely textual.** PR #28
|
||||
(`fix/oauth2-proxy-cookie-collision-fleet`, merged as `6caa5f2`) inserted its own constants blocks
|
||||
(`OAUTH2_ACCESS_TOKEN_LIFETIME_S` / `OAUTH2_COOKIE_REFRESH`, and `OAUTH2_PROXY_METRICS_PORT`) at the
|
||||
same insertion point immediately after `KEYCLOAK_ISSUER` that this branch used for
|
||||
`DRI_VOLUME_NAME` / `DRI_DEVICE_DIR`. The two blocks are unrelated and do not interact, so **both
|
||||
sides were kept in full**, oauth2-proxy's first and the hardware block after it. Nothing was
|
||||
dropped and nothing was rewritten: the rebased commit adds the **same 126 `render.py` lines** as the
|
||||
pre-rebase commit (verified by comparing the added-line sets) and deletes none. In particular the
|
||||
`supplementalGroups`-is-deliberately-not-emitted and `DirectoryOrCreate`-is-load-bearing comments
|
||||
survive verbatim.
|
||||
|
||||
Re-verified after the resolution: `--selftest` PASS (rc 0), and a full re-render still produces a
|
||||
**zero** diff in `apps/` — this PR remains inert, as it must be while no ServiceDefinition opts in.
|
||||
|
||||
## What I verified
|
||||
|
||||
Measured, in this worktree, on this branch:
|
||||
|
||||
- **`python3 catalog/render.py --selftest` → `render.py selftest: PASS`, rc 0.** New cases in the
|
||||
existing `check(...)` style: the positive (volume + mount, and *no* `securityContext`, *no*
|
||||
privileged, *no* nodeSelector), the negative that matters (an SD without `hardware` gains no
|
||||
volume, no volumeMount and no `securityContext` — mirroring the existing `podSecurityContext`
|
||||
negative), and a pass-through case (an SD declaring both `hardware` and its own
|
||||
`podSecurityContext: {fsGroup: 1001, supplementalGroups: [44]}` keeps it **exactly** as written).
|
||||
- **The self-tests actually bite.** I mutated the guard (`if hw_gpu == "intel-quicksync":` → `if
|
||||
True:`) in a scratch copy and re-ran: **rc 1**, failing on "an SD without `hardware` must gain no
|
||||
dri volume, no mount and no securityContext". A test that cannot fail is not a test; this one can.
|
||||
- **Re-render is a zero diff.**
|
||||
`python3 catalog/render.py --all catalog -o apps --wave-layout --no-image-automation` then
|
||||
`git diff --stat apps/` → **empty**, and `git status --porcelain apps/` → **empty** (so the two
|
||||
not-onboarded by-products, `apps/wave-1/keycloak.yaml` and `apps/wave-0/vm-demo.yaml`, were
|
||||
rewritten byte-identically and needed no revert). The change is inert until a service opts in.
|
||||
- **Schema validity and catalog conformance.** `Draft202012Validator.check_schema` passes; all
|
||||
**12** `catalog/*.yaml` validate with **0** errors. On the new field specifically: `gpu:
|
||||
intel-quicksync` accepted; `gpu: /dev/dri` rejected (enum); `hardware: {gpu: …, path: …}` rejected
|
||||
(`additionalProperties: false`); `hardware: {}` rejected (`minProperties: 1`).
|
||||
- **Hand-checked the emitted YAML** by rendering a throwaway SD carrying `hardware`, a Longhorn
|
||||
volume and its own `podSecurityContext: {runAsUser: 1000, supplementalGroups: [44]}`. Result:
|
||||
`volumeMounts` = config then `{name: dri, mountPath: /dev/dri}`; `volumes` = the PVC then
|
||||
`{name: dri, hostPath: {path: /dev/dri, type: DirectoryOrCreate}}`; `securityContext` =
|
||||
`{runAsUser: 1000, supplementalGroups: [44]}` — i.e. **unchanged**, nothing appended. That toy SD
|
||||
was never written into `catalog/`.
|
||||
- **GATE: `rc 3` — UNDETERMINED, NOT green.** Run in the pinned toolbox from this worktree with
|
||||
both CLAUDE.md §3 workarounds (`DEV_KUBECONFIG=/dev/null`, and an `ENGINE` shim bind-mounting the
|
||||
main checkout's `.git` at its own absolute path). Diff base `origin/main @ 0fbf544d5e2f`; touched
|
||||
requirements `R-DNS-03 R-GITOPS-02 R-IDP-02`. Run **four** times on this branch with an identical
|
||||
result each time, most recently against tree `c132893045b1` — the final commit differs from that
|
||||
run only by this paragraph. Per stage:
|
||||
|
||||
| stage | result |
|
||||
|---|---|
|
||||
| spec-immutability | **SKIPPED** — sibling spec repo not at `/work/../home-network-k3s`. Expected from a worktree; tolerated by §3. |
|
||||
| decisions-log | pass |
|
||||
| lint | pass |
|
||||
| l0 | pass — 19 offline cases, `0 failure(s), 0 skip(s), 0 unwired` |
|
||||
| render-golden | pass — *"all 10 onboarded service golden(s) reproduce from the canonical render"* |
|
||||
| l1 | pass — 3 offline checks, `0 failure(s), 0 skip(s), 0 unwired`; coverage 51/51 requirements scaffolded |
|
||||
| approval-policy | pass (`event='none'`, not a pull_request event — N/A locally; it will evaluate for real on the PR) |
|
||||
| gitea-settings | **SKIPPED** — `SETTINGS_API_TOKEN` unset. Expected; branch protection and runner registration are therefore UNVERIFIED by this run. |
|
||||
| conformance (L2/L3) | **SKIPPED** — `CLUSTER_AVAILABLE!=true`. Phase-0 standing skip. |
|
||||
|
||||
The gate's own verdict: *"Stages that did not run: spec-immutability gitea-settings"*. **No stage
|
||||
failed, and no stage skipped for a reason caused by this change.** rc 3 is reported as rc 3; I am
|
||||
not calling this green.
|
||||
- **Blast radius of the schema change:** `hardware` is absent from every catalog entry, so
|
||||
`apply_defaults` never materializes it and `s.get("hardware")` is `None` on all 12 — which is what
|
||||
the zero diff above demonstrates rather than asserts.
|
||||
- **Touched no sensitive path.** Changes are confined to `schemas/`, `catalog/`, `decisions/`; per
|
||||
`infra/gitea/approval-policy.json` none of those are sensitive. `conformance/**` and `clusters/**`
|
||||
are untouched. I did not run `--emit-waves` (it writes `clusters/base/`, and the set of files
|
||||
under `apps/` did not change).
|
||||
|
||||
## What I did NOT verify
|
||||
|
||||
- **Whether containerd's device cgroup permits a non-privileged pod to `open()` a bind-mounted char
|
||||
device. Still unverified, and it is the single largest technical risk in this workstream.**
|
||||
containerd installs a per-container **device cgroup** allow-list, and whether
|
||||
`/dev/dri/renderD128` arriving as a bind-mounted device node is permitted by it on this
|
||||
k3s/containerd version is **unmeasured**. Settling it requires applying a pod, which is an
|
||||
operator action, not an agent one. The container's PID 1 running as uid 0 helps on the DAC side
|
||||
but does **not** bypass the cgroup gate. If it blocks, the mount will succeed, the image's group
|
||||
hook will do its job, and the `open()` will still fail — and the fallback is the Intel GPU device
|
||||
plugin, which *sets* the cgroup entry. Note that that fallback is a **renderer-only change**: no
|
||||
ServiceDefinition moves, because the authoring surface is an intent enum. That is the design
|
||||
justifying itself, and it is the reason to keep the enum even if the hostPath mechanism loses.
|
||||
- **That a missing device degrades silently — and nothing will tell you.** This is the accepted cost
|
||||
of `DirectoryOrCreate`. The workload will software-transcode and **no signal reports it**: media
|
||||
apps in this catalog declare no `metricsEndpoint`, so no ServiceMonitor exists, and there is no
|
||||
metric to alert on. I deliberately did **not** propose an alert, because an alert that cannot fire
|
||||
is worse than none — see the 2026-09-13 correction in `renderer-contract.md` §`observability`,
|
||||
where twelve monitors discovered zero targets. The only proof of hardware transcode is an operator
|
||||
seeing it in the application's own dashboard.
|
||||
- **That "mounting the device is sufficient" generalizes beyond this one image.** It does not, and
|
||||
I have not verified that it does. The claim was measured for an image that resolves the device
|
||||
group itself in `cont-init.d`. A future consumer of `hardware.gpu` that runs non-root with no
|
||||
equivalent hook would still need the group solved — and the fix for that case is a **renderer**
|
||||
change (per-image handling, or the device plugin), never a new SD field. I scoped the claim in
|
||||
`renderer-contract.md` rather than letting it read as general; the first such consumer must
|
||||
re-measure.
|
||||
- **The feature now depends on a hook inside an image that floats.** The `cont-init.d` behaviour
|
||||
above was read from digest `sha256:e0ab27395614…` (`plexmediaserver 1.43.4.10903`), but the
|
||||
deployment runs `plexinc/pms-docker:latest` with `imagePullPolicy: Always`. **A silent pull can
|
||||
change the behaviour this feature depends on, with no commit anywhere in this repo to show for
|
||||
it** — and the failure would be the silent software-transcode described above. I have not fixed
|
||||
that here (it is the Plex wiring's tag/digest decision, not the capability's), but it should not
|
||||
be forgotten.
|
||||
- **GID drift is NOT on this list any more, deliberately.** An earlier revision pinned
|
||||
`INTEL_RENDER_GID = 992` and listed "a node rebuild could move it" as an open risk. The hook
|
||||
re-`stat`s the device on every start, so drift is self-healing and no GID is pinned anywhere. The
|
||||
risk is retired, not overlooked.
|
||||
- **Anything on the live cluster.** I ran no `kubectl` and I opened no shell in any container. The
|
||||
node facts (device present, `root:render` 0660, `i915` bound, no device plugin, no
|
||||
`gpu.intel.com/*` advertised), the in-container measurements quoted above, and the Kyverno
|
||||
posture (`restrict-host-namespaces` Enforce; no hostPath or `supplementalGroups`
|
||||
restriction; no PSA labels on app namespaces) are **prior recon handed to me, not re-measured by
|
||||
me**. Per CLAUDE.md §2b, treat them as second-hand until re-queried.
|
||||
- **That CI exercises the self-tests. It does not.** `git grep -- --selftest ci/` returns nothing —
|
||||
no gate stage invokes the renderer's self-test. They are the executable spec and house style, and
|
||||
I ran them by hand, but a future regression in this code path **will not be caught by the gate**.
|
||||
Wiring `--selftest` into a gate stage is a real gap and is not part of this PR (`ci/` is a
|
||||
sensitive path and would drag in a human reviewer for a change unrelated to this one).
|
||||
- **Runtime behaviour of any kind.** The gate proves the L0/L1 offline subset only;
|
||||
`vars.CLUSTER_AVAILABLE` is unset, so L2/L3 conformance does not run in CI.
|
||||
Reference in new issue
Block a user