From d44490736cf01be046dd01e6f7f1c27197735925 Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Thu, 1 Oct 2026 21:19:33 -0500 Subject: [PATCH] fix(ci): catalog/render.py's 106 selftest assertions never ran in the gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `catalog/render.py --selftest` executes 106 assertions on the renderer's output contract. Nothing in ci/ or .gitea/ had ever invoked it — grepped and confirmed — so all 106 were dev-time only, and the merge gate could not tell a sound renderer from a gutted one. Recorded as the top follow-up of decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md. `ci/render-golden.sh` now runs it and exits 1 when it fails. One file, six lines of invocation; the rest is the rationale comment. WHY HERE. The lead was auditability-proofs-producible/check.sh:291, which drives conformance/lib/{record_run,status}.py --selftest "because nothing else drives them". That is the wrong hook: the check serves R-SEC-03 and its subject is the evidence chain — can a forged or stale run become an audit proof? The renderer is not in that chain, and hanging it there would make the check's name, its intent annotation and its requirement mapping untrue. This stage's subject already IS catalog/render.py, so the renderer's own contract test belongs in it. The two assertions are independent, and that is the point: render-golden catches golden-vs-renderer drift but is satisfied by ANY renderer whose output was re-rendered — including one that just lost a contract invariant and had all ten goldens regenerated in the same commit. That is precisely the hole the follow-up named. The selftest catches it; render-golden cannot. It runs before the empty-catalog exit 3 because the selftest renders synthetic ServiceDefinitions in memory and is catalog-independent — an empty catalog must not be able to skip it. Failure is a hard rc 1, never a skip: the renderer is source in this repo, so once python3 and render.py are both present (asserted above it) there is no legitimate environmental cause for "could not run". PROVEN TO FAIL, not merely to pass. Against mutated COPIES of the renderer via the stage's existing RENDER_PY override, so catalog/render.py — another branch's file this session — was never touched: * deleting the --cookie-name emission (reverting PR #28's fleet-wide cookie fix) -> stage rc 1; * OAUTH2_COOKIE_REFRESH 2m30s -> 10m -> stage rc 1 with the named assertion "parses to 600.0s, which is not inside (0, 300s)" — one of the two flags the cookie entry recorded as UNGATED, and one render-golden's diff would miss. AND PROVEN TO PROPAGATE, in the pinned toolbox: selftest rc 1 -> stage render-golden FAIL -> `MERGE GATE: BLOCKED`, gate.sh rc 1. Worth provoking: this repo flattens exit codes, and .gitea/workflows/gate.yml:138 runs `./ci/gate.sh || [ "$?" -eq 3 ]`, so a failure degraded to a skip would be swallowed by CI. render.py itself is unchanged. No conformance/ path is touched, no gate stage is added, nothing is deployed by this. Co-Authored-By: Claude Opus 5 (1M context) --- ci/render-golden.sh | 55 ++++- ...2026-10-01-render-selftest-runs-in-gate.md | 205 ++++++++++++++++++ 2 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 decisions/2026-10-01-render-selftest-runs-in-gate.md diff --git a/ci/render-golden.sh b/ci/render-golden.sh index 4031f45..ba7b6cc 100755 --- a/ci/render-golden.sh +++ b/ci/render-golden.sh @@ -1,5 +1,10 @@ #!/usr/bin/env bash -# ci/render-golden.sh — Stage: render==golden reproducibility gate. +# ci/render-golden.sh — Stage: the renderer is trustworthy. +# (1) catalog/render.py passes its OWN output-contract self-test, and +# (2) every committed apps/ golden reproduces byte-for-byte from the canonical render. +# Two independent assertions about one subject, catalog/render.py. (1) was added +# 2026-10-01 because nothing in ci/ or .gitea/ had ever invoked `--selftest`; see the +# block above the invocation for why it lives here and not in a conformance check. # # WHY: the committed apps/ tree is a set of GOLDENS produced by ONE canonical # invocation of catalog/render.py. If a golden is hand-edited, or a catalog @@ -54,6 +59,54 @@ if [ ! -f "$RENDER_PY" ]; then summary "- render-golden: **SKIPPED** (renderer missing)" exit 3 fi +# --- The renderer's OWN self-test, before anything is compared --------------- +# `catalog/render.py --selftest` is ~106 executed assertions on the renderer's output +# contract (default-deny covers both directions, the Kyverno namespace hook label, a valid +# auth-mode, the oauth2-proxy sidecar's cookie-name/-domain/-refresh flags, the +# placeholder-substitution guard, the observability refusals...). Until now NOTHING in ci/ +# or .gitea/ invoked it — measured by grep on 2026-09-30 and recorded as the top follow-up +# of decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md — so every one of those +# assertions was dev-time only and a merge gate could not tell a sound renderer from a +# gutted one. +# +# WHY HERE, AND NOT IN A CONFORMANCE CHECK. The lead was +# conformance/tests/l0-static/auditability-proofs-producible/check.sh, which drives +# `conformance/lib/{record_run,status}.py --selftest` "because nothing else drives them". +# That loop is NOT the right hook: that check serves R-SEC-03 and its subject is the +# EVIDENCE CHAIN (can a forged or stale run become a proof?). The renderer is not part of +# that chain, and hanging it there would make the check's name and its requirement mapping +# a lie. This stage's subject already IS catalog/render.py — it exists to assert that the +# renderer's output is trustworthy — so the renderer's own contract test belongs in it. +# +# It runs BEFORE the catalog/empty-catalog exit-3 below on purpose: the self-test is +# deterministic and catalog-independent (it renders synthetic ServiceDefinitions in +# memory), so an empty catalog must not be able to skip it. +# +# render-golden and the self-test are INDEPENDENT assertions, which is why both are needed: +# render-golden catches golden-vs-renderer drift but is satisfied by ANY renderer whose +# output was re-rendered, including one that just lost a contract invariant and had all ten +# goldens regenerated in the same commit. The self-test catches exactly that. +# +# FAILURE IS A HARD FAIL, NOT A SKIP: the renderer is source in this repo, so "could not +# run its self-test" has no legitimate environmental cause once python3 and render.py are +# both present (both asserted above). ci/gate.sh maps this stage's rc 1 to BLOCKED. +log "Stage: catalog/render.py --selftest (the renderer's own output-contract assertions)" +selftest_out="$(mktemp)" +if "$PYTHON" "$RENDER_PY" --selftest >"$selftest_out" 2>&1; then + ok "render.py --selftest passed" + summary "- render-selftest: **passed** (\`catalog/render.py --selftest\`)" +else + err "catalog/render.py --selftest FAILED — the renderer violates its own output contract." + err " Every golden under apps/ is produced by this renderer, so the goldens below cannot" + err " be trusted even if they reproduce byte-for-byte. Reproduce with:" + err " ${PYTHON} catalog/render.py --selftest" + sed 's/^/ /' "$selftest_out" >&2 || true + summary "- render-selftest: **FAILED** (\`catalog/render.py --selftest\` — renderer contract violated)" + rm -f "$selftest_out" + exit 1 +fi +rm -f "$selftest_out" + if [ ! -d "$CATALOG_DIR" ] || ! compgen -G "${CATALOG_DIR}/*.yaml" >/dev/null 2>&1; then skip "no catalog ServiceDefinitions under '${CATALOG_DIR}' — nothing to render." summary "- render-golden: **SKIPPED** (empty catalog)" diff --git a/decisions/2026-10-01-render-selftest-runs-in-gate.md b/decisions/2026-10-01-render-selftest-runs-in-gate.md new file mode 100644 index 0000000..dc09f41 --- /dev/null +++ b/decisions/2026-10-01-render-selftest-runs-in-gate.md @@ -0,0 +1,205 @@ +--- +date: 2026-10-01 +branch: fix/render-selftest-runs-in-gate +title: catalog/render.py's 106 assertions were dev-time only; the gate now runs them and blocks on them +--- + +## What changed + +Single subject, one file: **`ci/render-golden.sh` now invokes `catalog/render.py --selftest`** +and exits 1 when it fails. Nothing else. No renderer change, no new conformance test, no new +gate stage. + +This closes the top follow-up of `decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md`: + +> 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. + +Re-measured here and confirmed: before this commit, `grep -rl -- --selftest` over `ci/` and +`.gitea/` returned **nothing** for the renderer. The selftest executes **106** assertions +(`check()` invocations counted at runtime via `sys.settrace`, without editing `render.py`; 109 +static call sites, three of them on failure-only branches). All 106 were unexercised by the +merge gate. + +### The lead was WRONG, and here is why + +The brief named `conformance/tests/l0-static/auditability-proofs-producible/check.sh:291` as the +one-line fix. That line is real and it does drive selftests: + + for tool in ("record_run.py", "status.py"): + r = run([sys.executable, str(LIB / tool), "--selftest"]) + +But it is **the wrong hook**. That loop is section (d) of a check that serves **R-SEC-03**, and +its subject is the *evidence chain* — can a self-skipped run, or a stale ledger row, become an +audit proof? `record_run.py` and `status.py` are **in** that chain; `catalog/render.py` is not. +Hanging the renderer there would have made the check's name, its `intent` annotation and its +requirement mapping (`R-SEC-03`) all untrue, and a future decision about the renderer could then +only be expressed by weakening a *security-auditability* check. That is the precise coupling the +cookie-collision entry refused for its own check, and it applies here. + +The second option that entry offered — a new scope-declaring L0 test — is the supported mechanism +for a **whole-repo invariant** (`ci/l0.sh` Stage 3b's `--include-scope whole-repo` union, ratcheted +by `tests/l0-static/whole-repo-invariants-wired`). It is the right shape for an invariant +quantified over the tracked tree. It is the wrong shape for *"a script in this repo must pass its +own unit test"*: it would need to declare requirements it does not serve, and it would be two new +files of several hundred lines by this tree's conventions for what is a six-line invocation. + +**The honest home is `ci/render-golden.sh`** — the gate stage whose subject *already is* +`catalog/render.py`. It already resolves `$PYTHON` and `$RENDER_PY`, already reports rc 3 when +either is missing, already exits 1 on renderer error, and `ci/gate.sh` already maps it through +`for s in lint l0 render-golden l1` where a FAIL is BLOCKED and a SKIP is UNDETERMINED. The +stage's two assertions are independent and both are needed: + +- `render-golden` catches golden-vs-renderer drift, but is satisfied by **any** renderer whose + output was re-rendered — including one that just lost a contract invariant and had all ten + goldens regenerated in the same commit. That is exactly the hole named in the quote above. +- `--selftest` catches that, and is blind to golden drift. + +The file header and the stage name were updated so neither is a lie: the stage is now "the +renderer is trustworthy", with two numbered assertions, and the step summary emits its own +`- render-selftest:` line so the gate output still NAMES what ran. + +The selftest runs **before** the empty-catalog `exit 3` on purpose: it renders synthetic +ServiceDefinitions in memory and is catalog-independent, so an empty catalog must not be able to +skip it. + +`catalog/render.py` itself was **not touched** (owned by a concurrent branch this session), and +neither was any file under `conformance/`. + +## What I verified + +- **The selftest runs and passes: rc 0, `render.py selftest: PASS`, 106 assertions executed.** + Counted at runtime, not from the source: a `sys.settrace` hook counting `check()` calls, in a + throwaway script outside the repo, with no edit to `render.py`. +- **The stage passes with it wired in**, host run of the single stage: + + [gate] Stage: render==golden reproducibility (canonical render vs committed apps/) + [gate] Stage: catalog/render.py --selftest (the renderer's own output-contract assertions) + [ ok ] render.py --selftest passed + [ ok ] render==golden: all 10 onboarded service golden(s) reproduce from the canonical render + + rc **0**. The golden comparison still reports all 10 goldens, so the new invocation did not + displace what the stage already asserted. +- **IT CAN FAIL — provoked twice, against a mutated COPY of the renderer, never against the + tree.** `ci/render-golden.sh` already honours `RENDER_PY`, so each mutant lived in + `/tmp/claude-1000/desmond-selftest/` and the repo's `catalog/render.py` was never modified — + which matters, because that file belongs to a concurrent branch this session. + - **Mutation 1 — the regression this selftest was written for.** Deleted the + `f"--cookie-name=_oauth2_proxy_{s['name']}"` emission from `_oauth2_sidecar_container()`, + i.e. reverted the fleet-wide cookie-collision fix of PR #28. Stage rc **1**. Note the shape + of the failure, because it is a real (minor) weakness and is listed as a follow-up: the + selftest died with an uncaught `StopIteration` out of `next(a for a in container["args"] if + a.startswith("--cookie-name="))` at `render.py:2641`, rather than printing its collected + `SELFTEST FAIL:` lines. It is correctly non-zero and the stage correctly blocks, but the + operator gets a traceback instead of the assertion message. + - **Mutation 2 — a clean, named assertion failure.** `OAUTH2_COOKIE_REFRESH = "2m30s"` -> + `"10m"`, widening the refresh past the measured 300s access-token lifetime. Stage rc **1**: + + [fail] catalog/render.py --selftest FAILED — the renderer violates its own output contract. + [fail] Every golden under apps/ is produced by this renderer, so the goldens below cannot + [fail] be trusted even if they reproduce byte-for-byte. Reproduce with: + [fail] python3 catalog/render.py --selftest + SELFTEST FAIL: --cookie-refresh=10m parses to 600.0s, which is not inside (0, 300s). A + refresh at or past the access-token lifetime never renews in time, which is the same + logout it was added to stop + + This is the mutation that proves the wiring's value most directly: mutation 2 would **not** + be caught by `render-golden`'s diff if the goldens were regenerated in the same commit, and + it is one of the two flags the cookie-collision entry explicitly recorded as **ungated**. +- **THE NON-ZERO PROPAGATES ALL THE WAY TO `gate.sh`'s rc — provoked, in the pinned toolbox, not + reasoned about.** `RENDER_PY=/desmond-selftest/render-mutant2.py ./ci/gate.sh`: + + [ ok ] stage spec-immutability: pass + [ ok ] stage decisions-log: pass + [ ok ] stage lint: pass + [ ok ] stage l0: pass + [gate] Stage: catalog/render.py --selftest (the renderer's own output-contract assertions) + [fail] catalog/render.py --selftest FAILED — the renderer violates its own output contract. + SELFTEST FAIL: --cookie-refresh=10m parses to 600.0s, which is not inside (0, 300s). ... + [fail] stage render-golden: FAIL + [ ok ] stage l1: pass + ... + [fail] render-golden: FAILED -> blocks merge + [fail] MERGE GATE: BLOCKED + + `gate.sh` rc **1**. Three links in the chain, each measured: selftest rc 1 -> stage rc 1 -> + gate rc 1 / `MERGE GATE: BLOCKED`. This mattered enough to provoke rather than assume: this + repo has a documented habit of exit codes getting flattened, and + `.gitea/workflows/gate.yml:138` runs `./ci/gate.sh || [ "$?" -eq 3 ]`, which tolerates rc 3 — + so a stage that degraded a failure to a skip would be swallowed by CI. rc 1 is not tolerated + there, and rc 1 is what this produces. +- **`spec-immutability` genuinely RAN and passed, it did not skip.** The two-mount worktree shim + plus `-e SPEC_REPO=/home-network-k3s` (the fix recorded in the 2026-09-30 cookie entry) works: + `stage spec-immutability: pass`, not SKIPPED. R-TEST-06 is therefore actually covered by these + runs. +- **No collision with PR #29.** The brief warned that `feat/oauth2-proxy-auth-failure-visible` + had added checks under `conformance/tests/l0-static/`. Measured — + `git diff origin/main...origin/feat/oauth2-proxy-auth-failure-visible --name-only` — it touches + **no `conformance/` path at all**: `decisions/`, `platform/oauth2-proxy/` (a promtool test, an + alert rule, its kustomization), `runbooks/`. Disjoint from this branch either way, since this + branch touches only `ci/render-golden.sh` and this file. +- **Priya's files were not touched.** `catalog/render.py`, `schemas/servicedefinition.schema.json`, + `apps/wave-*` and `conformance/tests/l1-rendering/` are all untouched by this branch — confirmed + by `git diff --name-only`, which lists exactly two paths. +- **`catalog/render.py --selftest` was the LAST unwired selftest in the tree.** Measured: + `grep -rl -- --selftest` across the repo finds selftest-capable artifacts in exactly four + places, and after this commit every one is driven by the gate — `conformance/lib/record_run.py` + and `conformance/lib/status.py` by `auditability-proofs-producible/check.sh:291`, the Keycloak + client-secret reconciler (embedded in `platform/keycloak/client-secret-reconciler.yaml`) by + `keycloak-client-secret-reconciler/check.sh:633`, and `catalog/render.py` by this change. + +## What I did NOT verify + +- **Local gate: rc 3 (UNDETERMINED), NOT 0. This is not green and must not be reported as + green.** Measured in the pinned toolbox on `eeb6211`: + + [ ok ] spec-immutability: green + [ ok ] approval-policy: green + [ ok ] decisions-log: green + [ ok ] lint: green + [ ok ] l0: green + [ ok ] render-golden: green + [ ok ] l1: green + [fail] MERGE GATE: UNDETERMINED — greenness was NOT ESTABLISHED. + [fail] Stages that did not run: gitea-settings + + Nothing FAILED. The single cause is `gitea-settings` **SKIPPED** — `SETTINGS_API_TOKEN` is not + set in my environment, so branch protection and runner registration were not read; CI has that + token and the stage is expected to run there. **L2/L3 `conformance`** also skipped + (`CLUSTER_AVAILABLE` unset) but is a *declared* gap that `gate.sh` treats as not-applicable + rather than undetermined, so it is not what produced the 3. Touched requirements + **R-GITOPS-01 R-TEST-02 R-TEST-03 R-TEST-06** are therefore **UNVERIFIED at L2/L3**, all four. + The stage this branch changes, `render-golden`, is green **and** was independently proven able + to turn the whole gate red (above). +- **`approval-policy` passed LOCALLY only because there is no pull_request event** + (`not a pull_request event (event='none') — approval policy N/A`). That is not evidence that it + passes. This diff touches `ci/**`, a sensitive path, so in CI the stage is expected to **FAIL** + until a non-author approving review exists. That is §1 working, fail-closed, not a defect. +- **The selftest's own quality is unchanged and ungated.** I wired its *invocation*; I did not + review its 106 assertions, and nothing ratchets them. Deleting the body of `_selftest()` would + still pass this stage, because an empty selftest returns 0. The repo already has the right + answer for that — `conformance/tests/l0-static/keycloak-client-secret-reconciler/mutations.py` + injects regressions one at a time and requires `--selftest` to catch every one — and applying + that pattern to `catalog/render.py` is the follow-up, deliberately not done here: it is a + second subject, it touches `conformance/`, and it would need to live beside the renderer whose + file another branch owns this session. **Until it exists, "the selftest runs" is proven and + "the selftest is falsifiable" rests on my two hand-run mutations, which are not in the gate.** +- **Mutation 1's traceback is not fixed.** `render.py:2641`'s `next(...)` has no default, so the + absence of `--cookie-name` raises `StopIteration` and the selftest's collected `SELFTEST FAIL:` + lines are never printed. Correct verdict, poor diagnostics, and it suppresses the other + assertions that mutation also broke. The one-line fix (`next(..., None)` plus a `check()`) is in + `catalog/render.py`, which is another branch's file this session, so it is a follow-up and not + a thing I touched. +- **Nothing about the live cluster.** No `kubectl` of any kind was run on this branch — not even + read-only; this change is offline CI wiring and needed none. No `apply`/`edit`/`patch`, no + `flux reconcile`. Merging this deploys nothing: it changes only what the gate refuses. +- **CI's own run of this branch.** Quoted in the PR body from the commit's `/statuses`, with + descriptions read rather than colours. A *skipped* Gitea job reports `success` with + "Has been skipped" and certifies nothing. +- **`vars.CLUSTER_AVAILABLE` is still unset in CI**, so even a fully green PR proves the L0/L1 + offline subset and nothing about runtime behaviour. +- **I did not re-measure branch protection, required approvals, or the runner.** Those are + CLAUDE.md §2 claims and §2b says re-measure before relying on them; this branch relies on none + of them beyond the PR flow itself, and I read the gate's own output rather than the API.