Merge pull request 'feat(obs): make oauth2-proxy auth failures visible — the gates emit nothing' (#29) from feat/oauth2-proxy-auth-failure-visible into main

Reviewed-on: #29
Reviewed-by: Keith Jackson <[email protected]>

decisions/2026-09-30-oauth2-proxy-auth-failure-visible.md

@@ -0,0 +1,289 @@
+---
+date: 2026-09-30
+branch: feat/oauth2-proxy-auth-failure-visible
+title: The Jackett auth failure was invisible because the gates emit nothing — alert on the gap, not on a metric that does not exist
+---
+
+## What changed
+
+Jackett's UI broke in the way that costs the most: **HTTP 200 and a plausible page**. The
+operator's report named the property that matters — *"reloading the page never fixes the issue and
+the user is never re-prompted for login"*. Two sibling branches fix the auth defects. This one is
+about the fact that **no signal existed anywhere**, and it turns out that is not an oversight in
+the alert rules. It is an absence of data.
+
+**The measurement that decided the whole design** (prod, live Prometheus, 2026-09-30):
+
+    count by (__name__) ({__name__=~"oauth2_proxy.*"})
+      oauth2_proxy_requests_in_flight => 2
+
+Two series. Both from the **shared** forward-auth Deployment in the `oauth2-proxy` namespace.
+That is every oauth2-proxy metric this cluster holds. The six per-app gate sidecars — crafty,
+jackett, pihole, qbittorrent, radarr, sonarr — publish **nothing at all**: they run with
+`--http-address=0.0.0.0:4180` and no `--metrics-address`, so oauth2-proxy's metrics listener never
+starts; they declare only a `http:4180` container port; and no ServiceMonitor or PodMonitor in the
+cluster selects them.
+
+So the alert the brief asked for — *"a gated app is serving a sustained rate of auth redirects to
+a user who is authenticated"* — **is not writable**, and writing it anyway would have produced
+exactly the artefact this repo keeps getting burned by: a rule that is present, evaluating,
+permanently silent, and indistinguishable from health. It was not written.
+
+What was written instead alerts on the condition that made the incident invisible, from families
+that **are** scraped:
+
+- **`platform/oauth2-proxy/auth-visibility-alerts.yaml`** — `OAuth2ProxyGateUnobserved`:
+
+      count by (namespace) (kube_pod_container_info{container="oauth2-proxy"})
+        unless
+      count by (namespace) (up{container="oauth2-proxy"})
+
+  `for: 1h`, `severity: warning`. `kube_pod_container_info` is kube-state-metrics (a declared
+  owner in `alert-metrics-have-scrape-targets`) and sees the container whether or not it exports
+  anything; `up` is UNIVERSAL and carries a `container` label for any scrape target, so the rule
+  needs no advance knowledge of what job name a future per-app scrape will use. Nothing in it
+  touches `oauth2_proxy_*`, so it cannot be inert for the reason it is reporting. **It returns six
+  namespaces against live prod right now** and excludes `oauth2-proxy`, which is scraped. It
+  clears per-namespace as each gate gains a real scrape.
+- **`platform/oauth2-proxy/auth-visibility-alerts.promtool-test.yaml`** — four states pinned:
+  fires for an unscraped namespace, never for the scraped one, honours `for: 1h` (pending at 59m),
+  and **stops** once a scrape appears under a *different* job name. Series shapes copied off live
+  prod, not invented.
+- **`runbooks/oauth2-proxy-auth-failures.md`** — how to tell "the gate refused me" from "the group
+  check refused me" from "the app refused me" from "the XHR silently got HTML", with the
+  2026-09-30 jackett trace as the worked example. Registered in `runbooks/kustomization.yaml`'s
+  `configMapGenerator` and listed in `runbooks/README.md`.
+- **`runbooks/README.md`** — a new row in *"Alerts you should not trust the silence of"* for the
+  per-app sidecars, plus the measured reason.
+- **`platform/oauth2-proxy/README.md`** — what this component can and cannot see, and the exact
+  three-part change that would close the gap.
+
+**Why the obvious alternative was rejected.** An alert on the *shared* proxy's
+`oauth2_proxy_requests_total{code="302"}` ratio is the one a reader expects to see here. It would
+need a new `SCRAPE_OWNERS` entry in `conformance/**` (a sensitive path needing human approval),
+and — decisively — **it would not have fired on this incident**: the shared instance is not in the
+data path for any sidecar-gated app, and its current pods export no `oauth2_proxy_requests_total`
+at all, having proxied nothing since they started (the counter exists in the TSDB only from an
+older ReplicaSet, `oauth2-proxy-78449b5657-*`, with `code` labels `302` and `401` and no `202`).
+Shipping it would have looked like coverage of the reported failure and been none.
+
+## What I verified
+
+Everything below was measured against live prod through the pinned container, read-only. No
+`apply`, no `edit`, no `patch`, no `flux reconcile`.
+
+- **The full oauth2-proxy metric inventory.** `count by (__name__) ({__name__=~"oauth2_proxy.*"})`
+  → `oauth2_proxy_requests_in_flight => 2`. `count by (__name__) ({job="oauth2-proxy"})` returns
+  go_*/process_*/promhttp_* plus that one family and nothing else. **Only the shared instance is
+  scraped**: `up{job="oauth2-proxy"}` → 2 targets, `10.0.1.189:44180` and `10.0.2.47:44180`, both
+  `1`, `namespace="oauth2-proxy"`.
+- **The per-app sidecars are unscraped, and unscrapable as configured.** Every pod carrying an
+  `oauth2-proxy` container, with its declared ports: the two shared replicas have
+  `[('http',4180),('metrics',44180)]`; crafty, jackett, pihole, qbittorrent, radarr and sonarr all
+  have `[('http',4180)]` only. The live jackett sidecar's args contain no `--metrics-address`.
+  `kubectl get servicemonitors,podmonitors -A` lists 16 ServiceMonitors and 5 PodMonitors; the
+  only oauth2-proxy entry is `oauth2-proxy/oauth2-proxy`.
+- **No existing alert is inert *for this reason*, because no alert references these metrics at
+  all.** `grep -rn oauth2_proxy --include=*.yaml` over the tree hits only cookie-name strings in
+  `platform/crafty/`. The 39 live PrometheusRules include `platform-health-identity`, whose four
+  rules are about Keycloak readiness and the client reconciler and are all keyed on `kube_*`
+  (scraped, fine). **The inertness here is the absence of a rule, not a broken one** — which is
+  why the runbook README's inert-alerts table gains a row phrased like the etcd one.
+- **Keycloak cannot express it either.** `count by (__name__) ({__name__=~"keycloak_.*"})` →
+  empty, and empty over `[30d]`. The `keycloak-service` scrape yields JVM/Agroal/JGroups metrics
+  only; there are no user-event or login counters, so "a burst of re-logins for one client" is not
+  observable from the IdP side.
+- **`oauth2_proxy_requests_total` has existed historically but not on the current pods.**
+  `count_over_time(oauth2_proxy_requests_total[30d])` returns three series, all from
+  `oauth2-proxy-78449b5657-*` (a prior ReplicaSet): `code="302"` on both replicas and `code="401"`
+  on one, ~634 samples each (~5h of a 30d window). **No `code="202"` ever.**
+- **The alert expression fires against live data.** Run verbatim on prod:
+  `count by (namespace) (kube_pod_container_info{container="oauth2-proxy"}) unless count by
+  (namespace) (up{container="oauth2-proxy"})` → `pihole, jackett, crafty, qbittorrent, sonarr,
+  radarr`, value 1 each; `oauth2-proxy` correctly absent.
+- **The log signal exists and is collected.** Loki (`loki-0`, SingleBinary) and a 3-node Alloy
+  DaemonSet are running, and Alloy labels pod logs with `namespace`/`container`. LogQL over 24h,
+  `{container="oauth2-proxy"}`: crafty 12036 lines, jackett 11669, oauth2-proxy 34559, pihole
+  11520, qbittorrent 11520, radarr 11763, sonarr 11520. Filtered to `|= "No valid authentication
+  in request"`: **crafty 81, jackett 39, radarr 67**; to `|= "AuthSuccess"`: **crafty 1, jackett
+  4, radarr 1**. Those ratios (81:1, 10:1, 67:1) are the incident's signature and are in the
+  runbook as the thing to compare against.
+- **Why `kubectl logs` hid it.** The jackett sidecar's stdout is ~11.5k lines/day and almost all
+  of it is `GET "/ping" ... 200 2` from the kubelet probe, twice every 15s. A `--tail=40` shows
+  forty `/ping` lines and looks perfectly healthy. The runbook's commands all filter.
+- **Runbook registration, both halves.** `- oauth2-proxy-auth-failures=oauth2-proxy-auth-failures.md`
+  is in `runbooks/kustomization.yaml`'s `configMapGenerator.files`, and the page is listed under
+  *Identity* in `runbooks/README.md`. The page clears the L0 quality floor (the four operator
+  sections, >1500 bytes, mentions `dev.sh` and `--context k3s-prod`), and the alert's
+  `runbook_url` points at exactly that name.
+- **Gate:** run in the pinned toolbox from this worktree with the `ENGINE` shim. Exit code and
+  per-stage outcomes in the next section — read them there, not here.
+
+## What I did NOT verify
+
+- **The alert has never fired in Alertmanager.** Nothing is applied; the PrometheusRule exists
+  only in git on this branch. What is proven is that its expression returns the expected vector
+  against live prod data and that promtool agrees about its behaviour over synthetic series.
+  **Merging is what deploys it.**
+- **`OAuth2ProxyGateUnobserved` would NOT have fired *during* the 2026-09-30 jackett incident in
+  any useful sense** — it describes a standing condition that has been true since the sidecars
+  were deployed, so it would have been firing before, during and after. That is deliberate and it
+  is the honest claim: it is a **gap tracker**, not an incident detector. It would have meant that
+  on 2026-09-30 an operator asking "why did nothing alert?" had the answer already on the board
+  instead of discovering it during the investigation. **No alert on this platform could have
+  detected the incident itself, and none can today.**
+- **The alert that the brief actually asked for does not exist, and cannot be written yet.** The
+  sustained-redirect-ratio rule needs `oauth2_proxy_requests_total` from the *per-app* sidecars.
+  **Enabling that requires editing files owned by the two sibling branches** — the sidecar args in
+  `apps/wave-0/jackett.yaml` (Mira Vance) and `apps/wave-0/{qbittorrent,pihole}.yaml`,
+  `apps/wave-1/{radarr,sonarr}.yaml` (Theo Bright) — so it was **not attempted here**. The change
+  is three things per app, together: `--metrics-address=0.0.0.0:44180`, a `metrics` containerPort
+  plus Service port, and a ServiceMonitor. Once any one app has them, the rule belongs beside
+  `OAuth2ProxyGateUnobserved`, and that app's namespace drops out of it automatically.
+- **The Loki alerting path is inert and I did not fix it.** Loki *has* a ruler stanza and the
+  chart's `loki-sc-rules` sidecar is running with `LABEL=loki_rule`, `FOLDER=/rules` — so a
+  ConfigMap-shipped LogQL rule would be delivered. But `GET /loki/api/v1/rules` answers **"unable
+  to read rule dir /var/loki/rules/fake: no such file or directory"** (the ruler reads a different
+  directory from the one the sidecar writes), and the rendered config contains **no
+  `alertmanager_url`** — `grep -i 'alertmanager\|rule_path'` over the `loki` ConfigMap returns
+  nothing. So a LogQL alert on `|= "No valid authentication in request"` — which *would* have
+  caught this incident, and is the single highest-value follow-up here — **would evaluate and
+  notify nobody**. Wiring it means `rulerConfig` (alertmanager_url + a matching rules directory)
+  in `platform/monitoring/loki.yaml`, which is not a file this brief gave me. **I did not ship an
+  inert rule ConfigMap to look productive.**
+- **Nothing was reproduced in a browser.** The 302-to-200-with-HTML mechanism is read off the
+  sidecar access log and the response sizes (`/UI/Dashboard 200 44379`), not observed in devtools.
+- **Loki retention is 72h** (`platform/monitoring/loki.yaml`), so the 24h ratios above are a
+  snapshot; anything older than three days is already gone. This is stated in the runbook because
+  it bounds what an operator can still investigate.
+- **The gate's L2/L3 conformance did not run** (`CLUSTER_AVAILABLE` unset), so no runtime
+  requirement touched by this branch is certified. See the exit code below — **rc 3 is
+  UNDETERMINED, not green.**
+
+### Follow-ups, in order of value
+
+1. **Wire the Loki ruler to Alertmanager** (`platform/monitoring/loki.yaml`). This is the only
+   route to an alert that would have caught the actual 2026-09-30 failure, and the data is
+   already being collected. The rule is one expression:
+   `sum by (namespace) (rate({container="oauth2-proxy"} |= "No valid authentication in request" [15m]))
+   > 5 * sum by (namespace) (rate({container="oauth2-proxy"} |= "AuthSuccess" [15m]))`.
+2. **Give one gate sidecar a metrics endpoint** and watch its namespace drop out of
+   `OAuth2ProxyGateUnobserved`. That is the cheapest proof the alert is a tracker and not a nag,
+   and it unlocks the ratio rule in Prometheus.
+3. **Consider whether `--set-xauthrequest` apps should get `--api-routes`** so API paths answer
+   401 instead of 302. That is the fix that would make this failure *loud* rather than merely
+   observable, and it belongs with whoever owns the sidecar args.
+
+---
+
+# Round 2 — 2026-10-01: the scrape got wired, and found a bug in this branch's own alert
+
+Added to the same PR (#29) rather than a fourth concurrent branch. PR #28 merged in the meantime
+and `main` moved three PRs ahead; `origin/main` is merged in here, and the `runbooks/kustomization.yaml`
+conflict was resolved by **keeping both** runbook entries (mine and `oauth2-proxy-session`).
+
+## What changed
+
+**`platform/oauth2-proxy/podmonitor.yaml` (new).** PR #28 gave the five fleet sidecars
+`--metrics-address=0.0.0.0:44180` and a named `metrics` port. No Service exposes 44180 on any of
+them, so a ServiceMonitor would have nothing to select — a **PodMonitor** targets pod ports directly
+and needs no renderer change. Selects on the pod label `homelab.keithjackson.dev/auth-mode:
+forward-auth`, which is on exactly the five fleet gate pods and absent from crafty's: the property,
+not an app list, so a sixth forward-auth app needs no edit.
+
+**`auth-visibility-alerts.yaml`: `up{container="oauth2-proxy"}` → `up{container="oauth2-proxy"} == 1`.
+This is a real bug fix, and it is the most important thing in this round.** A target that is
+discovered but failing still publishes `up`, with value **0**. The series exists, so
+`count by (namespace)` returned 1, `unless` removed the namespace, and `OAuth2ProxyGateUnobserved`
+would have **gone quiet while nothing was being scraped at all** — this branch's own disease,
+reproduced inside the alert written to cure it. Not hypothetical: the PodMonitor lands five targets
+whose scrape is dropped, so the bare form would have silently cleared on this very commit.
+
+**Two new unit-test scenarios** pinning that: a discovered-but-failing target must still fire, and a
+target that works and then breaks must re-fire rather than latch cleared.
+
+**Docs corrected rather than appended to.** The 2026-09-30 claim "the sidecars export no metrics at
+all" is now only half true and was rewritten in `runbooks/README.md`, `platform/oauth2-proxy/README.md`
+and the alert header, each showing the two-step history, because each step looked like the end of it.
+The runbook gains the policy-vs-no-target triage (step 1b) and the **radarr `/signalr` worked case**.
+
+**Deliberately NOT shipped: the ratio alert.** `oauth2_proxy_requests_total` now exists on the
+endpoint, but not in the TSDB, because the scrape is blocked. Writing the ratio rule today would make
+it inert for precisely the reason it was refused on 2026-09-30. It becomes writable the day a scrape
+succeeds, not the day a listener exists.
+
+## What I verified
+
+- **The endpoint serves — by payload, not by flag.** Port-forwarded to the jackett pod's 44180:
+  `http=200 bytes=10208`, exporting `oauth2_proxy_requests_in_flight`, `oauth2_proxy_requests_total`
+  (`code="200" 3`, `code="302" 2`) and `oauth2_proxy_response_duration_seconds`. `--metrics-address`
+  being in `args` and `/metrics` returning a body are different claims; both now hold.
+- **The scrape path is DROPPED, measured from the right place.** From grafana in ns `monitoring`
+  (beside Prometheus, same policy position) to the jackett pod IP: `:44180` → `wget: download timed
+  out`; `:4180` → **also** timed out; the shared proxy's `:44180` → 200 with `oauth2_proxy_*` present.
+  Cause, read from the objects: ns jackett's `default-deny` is `podSelector: {}`,
+  `policyTypes: [Ingress, Egress]`, with an `egress` rule and **no `ingress` key** (deny-all ingress);
+  the only allowance is CNP `jackett-allow-gateway-ingress`, `fromEntities: [ingress]` to port 4180.
+  The shared proxy is scrapable only because ns `oauth2-proxy` has **no NetworkPolicy at all**.
+- **No Service exposes 44180** on any of the five (enumerated).
+- **The pod label selector is exact:** `auth-mode=forward-auth` on all five fleet gate pods, absent
+  from crafty's. Crafty's sidecar still has no `--metrics-address` (args read in full).
+- **Flux ordering forbids shipping the fix from here:** `oauth2-proxy dependsOn=['platform-config']`
+  and `apps-wave-0 dependsOn=['platform-config']` — siblings, no ordering. A NetworkPolicy naming ns
+  `jackett` from this tier would apply today and fail on a cold rebuild.
+- **The `== 1` guard is non-vacuous — proven by mutation.** Reverting it to the bare `up` makes the
+  two new scenarios FAIL (`got: nil` at 1h5m and 2h5m) and leaves every other assertion passing;
+  restoring it returns the suite to SUCCESS under pinned promtool v3.14.0.
+- **Gate:** see the next section for the exit code and skips.
+
+## What I did NOT verify
+
+- **The PodMonitor has never been applied.** That it discovers the five pods is reasoned from the
+  measured pod labels and port name, not observed — no target list has been read with it in place,
+  because applying it is an operator action. **Expect `up == 0` on all five when it lands**, which is
+  the documented state, not a regression.
+- **I did not fix the NetworkPolicy, and this PR therefore does not make the gates observable.** It
+  makes them *discovered*, and makes the remaining gap loud instead of silent. The one narrow rule
+  per app (ns monitoring → `auth-mode=forward-auth` → TCP 44180, per
+  `platform/longhorn/networkpolicy-metrics.yaml`) has to be rendered with each app's own policy, in
+  the catalog/renderer that is in flight on another branch. **It needs an owner.**
+- **Whether Prometheus will actually select a PodMonitor in ns `oauth2-proxy` targeting other
+  namespaces.** `namespaceSelector.any: true` plus `podMonitorSelectorNilUsesHelmValues: false` and
+  the fact that cross-namespace ServiceMonitors/PrometheusRules already work here say yes; it is not
+  measured with this object.
+- **No session-lifecycle claim can be made from the current window.** `AuthSuccess` is **0** on both
+  jackett and radarr since the PR #28 restart, so nobody has signed in and `--cookie-refresh=2m30s`
+  has never executed a refresh. The cookie fix is unproven in the one dimension that matters most.
+- **The radarr `/signalr` finding is the coordinator's measurement, not mine** — I reproduced neither
+  the 206/209 sample nor the jackett 0-in-13,612. They are recorded as reported.
+- **L2/L3 conformance still did not run** (`CLUSTER_AVAILABLE` unset). Nothing is certified.
+
+### Round 2 addendum — CI is RED, by design, and the local gate cannot see why
+
+After pushing round 2, `gate / gate (pull_request)` reported **failure after 1m42s** while the local
+toolbox run was rc 3 with nothing failed. Measured cause, singular:
+
+    sensitive paths touched:
+      conformance/tests/l0-static/servicemonitor-ports-resolve/check.sh   (matched conformance/**)
+    approving reviews: (no reviews at all)        author=gitea-admin
+
+`ci/approval-policy.sh` is fail-closed and exits 1 when a PR touches a `sensitive_paths` glob with no
+`APPROVED` review from a non-author — "a change to the gate cannot be gated by the gate". The single
+`KNOWN_EXTERNAL_MONITORS` entry is what trips it. **Nothing in L0/L1 failed.**
+
+**The local gate structurally cannot catch this, and said `approval-policy: green (N/A)`.** The stage
+short-circuits to N/A when `GITHUB_EVENT_NAME` is unset, which it always is off-CI. So for any branch
+touching a sensitive path, a clean local gate is a FALSE REASSURANCE — the PR run is the only honest
+signal. That is a general trap, not specific to this change, and it is worth knowing before someone
+concludes a red PR means broken code.
+
+To clear: a non-author approving review, then **re-run the gate** — approval does not re-trigger the
+status check, so the required context stays red until the job runs again. `required_approvals=1`
+already applied to every PR here, so the human was needed regardless; the conformance entry only
+turns the tick red while that is pending.
+
+The alternative is to drop the register entry, which means dropping `podmonitor.yaml` with it (the
+check fails without the declaration). I did not take it: the entry is the "declare the limit"
+mechanism the check explicitly asks for, it is shrink-only so it cannot rot quietly, and
+`flux-podmonitor.yaml` is the same case already in the register. But it is a real trade and the
+reviewer should make it knowingly.