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.