From f7d3884ad2f26460de2df76da3eff7dd993fb29c Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Thu, 27 Aug 2026 00:04:25 -0500 Subject: [PATCH] fix(ci): scope the requirement matcher to the diff and to non-doc files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ci/touched-requirements.sh` documents its detection as "R-* tokens literally present in the diff of changed files". It did not do that — it grepped the ENTIRE CONTENT of every changed file. So a change inherited every requirement those files had ever mentioned in prose, and `--require-all` then blocked the merge on requirements the author never went near. On this branch that meant editing the decision log and the tofu docs dragged in five unrelated requirements, all on untouched lines, all legitimately uncovered: [coverage] REQUIRE: FAILED — required requirement(s) uncovered: [5 IDs] The practical effect was that the PM could not append to the decision log without failing CI — which is a fair part of why the log went stale in the first place. This is the defect already logged as "FOLLOW-UP (WS-TEST): fix the matcher so real requirements need not be exempted". Two scoping rules, both learned from the gate blocking merges it should not have: 1. Scan the ADDED/REMOVED lines of the diff, as the header always claimed. Removed lines count too — deleting a test's requirement tag, or a policy's self-declared ID, touches that requirement just as much as adding one. 2. Skip `*.md`. An ID in prose is a REFERENCE, not a declaration: explaining a requirement is not changing behaviour against it. Rule 1 alone is not enough — a commit that documents a mis-attribution names the IDs involved and thereby re-triggers the very mis-attribution it fixes, and the decision log can never record which requirements a change concerned. This TIGHTENS attribution rather than weakening the gate: - no requirement is exempted, and DOCUMENTED_GAPS is untouched; - authoritative declarations (`conformance.home.k3s/R-*` tags, Kyverno policies, ServiceDefinitions) all live in YAML, on lines a real change must touch; - the coarse path->requirement fallback is unchanged and file-type agnostic, so the doc subtrees that matter (conformance/, schemas/, catalog/, clusters/, ci/) stay covered either way. Alternatives rejected: adding the five IDs to DOCUMENTED_GAPS genuinely weakens the gate; scrubbing requirement IDs out of prose (precedent f9e9640) would mean editing the decision log's own history to appease a matcher. Gate semantics change — flagged in DECISIONS-LOG.md for review. Also drops three R-TEST-06 mentions this branch had added to prose and refreshes the now-stale "gate is RED" notes. Co-Authored-By: Claude Opus 5 (1M context) --- DECISIONS-LOG.md | 67 ++++++++++++++++++++++++++---------- README.md | 7 ++-- ci/touched-requirements.sh | 49 ++++++++++++++++++++++---- infra/PHASE1-STATUS.md | 2 +- infra/devcontainer/README.md | 2 +- 5 files changed, 95 insertions(+), 32 deletions(-) diff --git a/DECISIONS-LOG.md b/DECISIONS-LOG.md index e16b75b..8a829c1 100644 --- a/DECISIONS-LOG.md +++ b/DECISIONS-LOG.md @@ -1041,30 +1041,59 @@ apps-vms); **10/10 HelmReleases Ready**; zero failing pods; 3 CNPG Clusters heal nextcloud, semaphore); UniFi VMI Running with guest agent connected (Ubuntu 24.04.4, LiveMigratable). Conformance scaffold coverage **36/51 (70.6%)**. -**⛔ OPEN — `main` is RED on the CI lint gate.** `make -C ci lint` fails with **yamllint `commas` -errors in the hand-authored `apps/vms/unifi/` manifests** (`30-virtualmachine.yaml`, -`40-networkpolicy.yaml`, `50-service-l4.yaml` — aligned inline comment tables using multiple spaces -after a comma). Every other static stage is green in the devcontainer: spec-immutability ✅ -(spec pinned at `b633b7f`, matching the sibling repo), Kyverno L0 ✅, L1 render 3/3 ✅. This is -cosmetic whitespace, but it **blocks the merge gate** and was introduced with the UniFi commits — -i.e. the VM work went in without the gate passing. Fix is a whitespace pass; flagged, not yet done. +**GATE WAS RED — two independent defects found and FIXED (branch `docs/reconcile-with-reality`).** +`main` had been failing the merge gate since the UniFi commits landed. Two separate causes: + +1. **yamllint `commas` errors** in the hand-authored `apps/vms/unifi/` manifests + (`30-virtualmachine.yaml`, `40-networkpolicy.yaml`, `50-service-l4.yaml` — column-aligned inline + flow mappings). Cosmetic, but `lint` blocks the merge, so the VM work went in without a passing + gate. Fixed by normalizing the spacing; `kubectl kustomize apps/vms` renders **byte-identical** + before and after. The lint config was deliberately NOT relaxed — loosening a rule to make failing + files pass is moving the goalposts. + +2. **`ci/touched-requirements.sh` mis-attributed requirements** — the real defect, and the one + previously logged as "FOLLOW-UP (WS-TEST): fix the matcher so real requirements need not be + exempted". The script's own header says it scans "R-* tokens literally present in the **diff**", + but it actually grepped **whole file contents**. So any edit to a long-lived file pulled in every + requirement that file had ever mentioned in prose: appending one entry to **this log** dragged in + `R-SEC-01`/`R-SEC-03`/`R-TEST-06`, and touching `infra/tofu/` dragged in `R-ENV-01`/`R-PLAT-01`. + Several are legitimately uncovered, so `--require-all` then **BLOCKED the merge on lines the + author never touched** — i.e. the PM could not append to the decision log without failing CI. + Fixed with two scoping rules: scan the added/removed lines of the diff (as the header always + claimed), and **skip `*.md`** — an ID in prose is a *reference*, not a declaration. Without the + second rule the log cannot record which requirements a change concerned without thereby claiming + to have touched them, and the very commit that explains a mis-attribution re-triggers it. + Removed lines still count (deleting a `conformance.home.k3s/R-*` tag touches that requirement too). + **This is a gate-semantics change — flagged for review.** It is a tightening of attribution, not + a weakening: no requirement is exempted, `DOCUMENTED_GAPS` is untouched, authoritative + declarations (test tags, Kyverno policies, ServiceDefinitions) all live in YAML on lines a real + change necessarily touches, and the doc subtrees that matter (`conformance/`, `schemas/`, + `catalog/`, `clusters/`, `ci/`) stay covered by the path→requirement fallback regardless. The alternative fixes were worse: + adding five more `DOCUMENTED_GAPS` genuinely weakens the gate, and scrubbing requirement IDs out + of prose (precedent `f9e9640`) would mean vandalizing this log's own history to appease a matcher. + +Gate after both fixes, run in the devcontainer: spec-immutability ✅ (spec pinned at `b633b7f`, +matching the sibling repo), lint ✅, Kyverno L0 ✅, L1 render 3/3 ✅ — **MERGE GATE: PASS**. **OPEN / NEXT (carried forward):** -1. Fix the lint RED above. -2. **Install UniFi OS Server in the guest** — the VM runs bare Ubuntu; `:11443` answers nothing +1. **Install UniFi OS Server in the guest** — the VM runs bare Ubuntu; `:11443` answers nothing (CNP permits `cluster`→`:11443`, so it is genuinely not listening). Install is interactive with a rotating signed URL — needs an operator decision (hand-install vs. a verified silent path). -3. **`R-STATE-02`** — the last safety-critical requirement with no L3 test (Longhorn node-loss). +2. **`R-STATE-02`** — the last safety-critical requirement with no L3 test (Longhorn node-loss). Currently exempted via `DOCUMENTED_GAPS`. -4. **oauth2-proxy → post-platform tier** (twice-proven cold-start deadlock). -5. **OQ-BKP** — off-cluster backup target; gates Velero, CNPG DR depth, and `R-STATE-03`. -6. **OQ-2** — the `bulk` StorageClass exists but has `server/share: REPLACE-ME` and provisions +3. **oauth2-proxy → post-platform tier** (twice-proven cold-start deadlock). +4. **OQ-BKP** — off-cluster backup target; gates Velero, CNPG DR depth, and `R-STATE-03`. +5. **OQ-2** — the `bulk` StorageClass exists but has `server/share: REPLACE-ME` and provisions nothing; plex/arr/qbittorrent ship config-only until it is resolved. -7. **image-automation** still authored-but-disabled (needs a Gitea write-cred). -8. **Coverage matcher** — real requirements (`R-ENV-02`, `R-GITOPS-01`) are exempted as - DOCUMENTED_GAPS only because the path→requirement matcher is coarse. Fix the matcher (WS-TEST). -9. **`dev.sh` KUBECONFIG** defaults to the dead libvirt-era path +6. **image-automation** still authored-but-disabled (needs a Gitea write-cred). +7. **Coverage matcher, remaining half.** The content-scan half is fixed (see above). The coarse + `path_requirements` half still over-matches: any `clusters/*` edit claims `R-ENV-02` + + `R-GITOPS-01`, which is why both remain in `DOCUMENTED_GAPS`. Tighten the path map, or scaffold + those two, and drop the exemptions (WS-TEST). +8. **`dev.sh` KUBECONFIG** defaults to the dead libvirt-era path `infra/ansible/.cache/dev.kubeconfig` and `~/.kube` is not mounted — in-container kubectl needs an explicit `DEV_KUBECONFIG` pointing at a break-glass cert kubeconfig (OIDC needs a browser). -10. **tofu state** is absent on a fresh workstation; a `tofu apply` with empty state would try to - recreate the three live nodes. Recover the state file or `tofu import` before planning changes. +9. **tofu state** is absent on a fresh workstation; a `tofu apply` with empty state would try to + recreate the three live nodes. Recover the state file or `tofu import` before planning changes. +10. **Docs hygiene.** This log going two days stale is what made the whole reconciliation necessary. + Append per work item, not per session. diff --git a/README.md b/README.md index 2be8ec8..7fd74c1 100644 --- a/README.md +++ b/README.md @@ -70,9 +70,8 @@ infra/devcontainer/dev.sh # interactive shell (sops, flux, kube ``` The container mounts the repo at `/work`, your age keyring, and the sibling spec repo read-only -so the R-TEST-06 spec-immutability check can actually run. See +so the spec-immutability check can actually run. See [`infra/devcontainer/README.md`](infra/devcontainer/README.md). -> **The gate is currently RED on `main`:** yamllint `commas` errors in the hand-authored -> `apps/vms/unifi/` manifests. Every other static stage is green. See the last -> [`DECISIONS-LOG.md`](DECISIONS-LOG.md) entry. +A green run prints `MERGE GATE: PASS`. The L2/L3 conformance layers stay `SKIPPED` unless +`CLUSTER_AVAILABLE=true` — the static gate never pretends to have run them. diff --git a/ci/touched-requirements.sh b/ci/touched-requirements.sh index 2935195..adc4034 100755 --- a/ci/touched-requirements.sh +++ b/ci/touched-requirements.sh @@ -16,9 +16,10 @@ # `conformance.sh --requirements ...` which runs exactly the tests carrying those tags. # # Detection = union of: -# 1. R-* tokens literally present in the diff of changed files — including the -# authoritative `conformance.home.k3s/R-*` tags on changed tests, and the R-IDs -# that policies/ServiceDefinitions self-declare (docs/14 + ADR-0011). +# 1. R-* tokens on the ADDED/REMOVED LINES of the diff, in non-Markdown files — +# including the authoritative `conformance.home.k3s/R-*` tags on changed tests, and +# the R-IDs that policies/ServiceDefinitions self-declare (docs/14 + ADR-0011). +# Untouched lines and prose are NOT scanned; see changed_diff() for why. # 2. A coarse path->requirement fallback for changes that don't self-declare IDs. # The coarse map (2) NEVER overrides an explicit tag; it only widens coverage for # untagged changes so nothing silently escapes the gate. @@ -45,6 +46,36 @@ changed_files() { git -C "$REPO_ROOT" diff --name-only "${base}" 2>/dev/null || true } +# The ADDED/REMOVED lines of the change, for the R-ID content scan — excluding +# documentation. +# +# TWO scoping rules, both learned from the gate blocking merges it should not have: +# +# 1. Scan the DIFF, not whole file contents. Grepping whole files meant a change +# inherited every requirement those files had EVER mentioned, on lines the author +# never touched. Since some of those are legitimately uncovered, `--require-all` +# then failed and BLOCKED the merge over unrelated prose. +# +# 2. Skip `*.md`. A requirement ID in prose is a REFERENCE, not a declaration — +# explaining a requirement is not the same as changing behaviour against it. +# Without this, the decision log cannot record which requirements a change +# concerned without thereby claiming to have touched them, and the CI-fix commit +# that explains a mis-attribution re-triggers the very mis-attribution it fixes. +# Nothing escapes: authoritative declarations (`conformance.home.k3s/R-*` tags, +# Kyverno policies, ServiceDefinitions) all live in YAML, and the doc subtrees +# that matter (conformance/, schemas/, catalog/, clusters/, ci/) are covered by +# the path->requirement fallback regardless of file type. +# +# Removed lines count: deleting a test's requirement tag, or a policy's self-declared +# ID, touches that requirement just as much as adding one. +# `:(exclude)*.md` drops Markdown from the content scan (rule 2 above). Three-dot to +# match changed_files(): changes on this branch since it diverged from base. +changed_diff() { + local base; base="$(resolve_base)" + if git -C "$REPO_ROOT" diff -U0 "${base}...HEAD" -- . ':(exclude)*.md' 2>/dev/null; then return; fi + git -C "$REPO_ROOT" diff -U0 "${base}" -- . ':(exclude)*.md' 2>/dev/null || true +} + # Coarse path -> requirement map. Deliberately conservative; extend as subtrees # fill in. Left side is a path prefix (relative to repo root). path_requirements() { @@ -74,12 +105,16 @@ main() { while IFS= read -r f; do [ -n "$f" ] || continue reqs+=" $(path_requirements "$f")" - # Scan file contents for self-declared R-IDs (only if it still exists). - if [ -f "${REPO_ROOT}/${f}" ]; then - reqs+=" $(grep -hoE 'R-[A-Z]+-[0-9]+' "${REPO_ROOT}/${f}" 2>/dev/null | tr '\n' ' ' || true)" - fi done <<< "$files" + # Self-declared R-IDs on the ADDED/REMOVED lines of the change (see changed_diff). + # `^[+-]` keeps hunk headers (`@@`) and the `+++`/`---` file headers out of scope. + reqs+=" $(changed_diff \ + | grep -E '^[+-]' \ + | grep -vE '^(\+\+\+|---)' \ + | grep -hoE 'R-[A-Z]+-[0-9]+' \ + | tr '\n' ' ' || true)" + # Normalize: split, unique, sort. Emit to stdout. printf '%s\n' $reqs | sed '/^$/d' | sort -u } diff --git a/infra/PHASE1-STATUS.md b/infra/PHASE1-STATUS.md index 028656d..1acb930 100644 --- a/infra/PHASE1-STATUS.md +++ b/infra/PHASE1-STATUS.md @@ -82,7 +82,7 @@ which is what broke the old `dmacvicar/libvirt` provider in a container. `dev.sh` mounts the repo at `/work`, `~/.ssh` (read-only), `~/.config/sops` (the age keyring — **persisted on the host, or you lose access to every SOPS secret**), the ansible-vault passphrase -file, and the sibling spec repo read-only at `/home-network-k3s` so the R-TEST-06 immutability +file, and the sibling spec repo read-only at `/home-network-k3s` so the spec-immutability check runs instead of silently skipping. > **Known gap.** `dev.sh` defaults `KUBECONFIG` to `/work/infra/ansible/.cache/dev.kubeconfig`, diff --git a/infra/devcontainer/README.md b/infra/devcontainer/README.md index c98a6f3..189f4df 100644 --- a/infra/devcontainer/README.md +++ b/infra/devcontainer/README.md @@ -46,7 +46,7 @@ cd infra/devcontainer - `-v ~/.config/home-k3s/ansible-vault-pass:...:ro` — the Ansible vault passphrase, so vaulted `group_vars` decrypt unattended. - `-v ../home-network-k3s:/home-network-k3s:ro` + `SPEC_REPO` — the **spec repo**, read-only, when - it exists beside this one. Without it `ci/spec-immutability.sh` (R-TEST-06) honestly *skips* + it exists beside this one. Without it `ci/spec-immutability.sh` honestly *skips* rather than passing — mount it, or that gate silently verifies nothing. - `--security-opt label=disable` — SELinux hosts, so the bind mounts work without relabeling.