diff --git a/.cursor/BUGBOT.md b/.cursor/BUGBOT.md index 3783dac0..36e22d6e 100644 --- a/.cursor/BUGBOT.md +++ b/.cursor/BUGBOT.md @@ -102,8 +102,8 @@ for *what the operator sees and can act on*, not code elegance. files should be mode 0600. - **A changed bootstrap-fetched script without a regenerated `scripts/manifest.sha256`** - (`scripts/gen-manifest.sh`). The "Installer manifest is current (supply-chain, R8)" step - in `installer-tests.yaml` fails, and a stale manifest breaks `install.sh`'s verified fetch. + (`scripts/gen-manifest.sh`). `make drift` fails in the required `Source-of-truth drift` + check, and a stale manifest breaks `install.sh`'s verified fetch. - **A `Chart.yaml` `version` bump without the matching `appVersion`** — the `app.kubernetes.io/version` label depends on it. @@ -117,7 +117,8 @@ for *what the operator sees and can act on*, not code elegance. so they can inspect a failing check's exit code instead of aborting. - **SC2034 "unused variable" in `scripts/lib/*.sh` is a known false positive** — those vars are consumed cross-file once the libs are sourced together. CI blocks at - `--severity=error` and runs `--severity=warning` advisory-only (`installer-tests.yaml:63-67`). + `--severity=error` via `make lint` in the required `Lint` check, and prints the + `--severity=warning` sweep advisory-only via `make lint-warnings`. - `scripts/manifest.sha256` and `scripts/testdata/golden/*.golden` are **generated**. The golden copy catalog is regenerated with `TB_UPDATE_GOLDEN=1 bats scripts/tests/copy-catalog.bats`, never hand-edited — review the diff --git a/.github/workflows/drift-checks.yaml b/.github/workflows/drift-checks.yaml index 0770ae73..9a4489a7 100644 --- a/.github/workflows/drift-checks.yaml +++ b/.github/workflows/drift-checks.yaml @@ -13,6 +13,12 @@ name: Drift checks # printing red on a workflow nobody has to wait for. A guard that cannot block # is advice (backend#1729). # +# It is therefore also the home for the R8 installer-manifest check and the +# cross-OS facts check, which moved here from installer-tests.yaml's `static` +# job (`Static analysis`, required on NO branch) on 2026-08-19. If you add a +# guard, add it to the Makefile's `drift` target -- this job runs that target +# and holds no list of its own. +# # NO `paths:` ON pull_request, deliberately. `Source-of-truth drift` is a # REQUIRED status check on develop and on main, and a required check that is # path-filtered can never report on a PR outside those paths -- GitHub leaves it @@ -31,6 +37,13 @@ on: - 'scripts/**' - 'client/**' - '.github/workflows/drift-checks.yaml' + # #616: check-facts pins the GPU node image's k3s tag against + # scripts/spec/facts.env, so a K3S_TAG edit under docker/k3s-cuda must run + # this job too. These moved here with the guard itself, out of + # installer-tests.yaml's `paths`. + - 'docker/k3s-cuda/**' + - '.github/workflows/build-k3s-cuda.yaml' + - 'Makefile' pull_request: branches: [main, develop, openshift] workflow_dispatch: @@ -42,31 +55,40 @@ jobs: drift: name: Source-of-truth drift runs-on: ubuntu-latest + # 20 minutes against guards that measure ~2 s. The previous revision of this + # file argued for NO timeout, on the grounds that a killed job reports its + # guards as `skipped`. That reasoning does not survive this PR's own change + # (Arturo, #755 review): `skipped` guards were harmless only while `Static + # analysis` was required nowhere. Now that `Source-of-truth drift` IS a + # required check, a timeout-kill sets it `cancelled`, which branch protection + # treats as not-success -- red and blocked, which is exactly what we want. + # So a bound is now free, and it is worth having: the hang risk here is not + # `make drift`, it is `azure/setup-helm` below fetching a binary before any + # guard runs. + timeout-minutes: 20 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Set up Helm uses: azure/setup-helm@1a275c3b69536ee54be43f2070a358922e12c8d4 # v4.3.1 with: version: v3.15.4 - - name: check-drift - run: bash scripts/tests/check-drift.sh - # One CLIENT_ENV vocabulary, four declarations (Go template, JSON Schema, - # bash, PowerShell), backend#1729 sweep 5. Lives here rather than in - # helm-ci because helm-ci's `Helm lint` is NOT a required check, so the - # guard could only advise: a PR that made the four disagree was mergeable - # with it red. It needs bash + python3 only (~2s), and drift-checks runs - # unfiltered on every PR, so it reports on the installer-only PRs that - # helm-ci's `paths:` filter would have skipped entirely. - - name: CLIENT_ENV vocabulary agreement (backend#1729) - run: bash scripts/tests/env-vocabulary-agreement.sh - # The installer's telemetry vocabularies (backend#1907): four closed sets - # whose values are produced elsewhere -- the phases by install-k8s.sh's - # step_header calls, the client states by summary.sh, the script names by - # gen-manifest.sh's FILES array, the error classes by telemetry.sh's own - # classifier. A closed set that has drifted from its producer does not fail - # loudly; it reports `unknown` forever, on exactly the runs somebody added - # the new value for. It lives HERE for the same reason the CLIENT_ENV guard - # does: `Source-of-truth drift` is required, so this can block rather than - # advise. bash only, ~1s. - - name: Telemetry vocabulary agreement (backend#1907) - run: bash scripts/tests/telemetry-vocabulary-agreement.sh + # ONE step, and it names no guard. `make drift` is the single declaration of + # "the duplicated-declaration guards", and the pre-push hook (`make check` + # -> `drift`) runs the identical set. Add a guard in the Makefile and it + # gates here automatically. + # + # There is deliberately NO enumeration of the guards in this file. An + # earlier version of this comment listed five of them "for orientation", + # and within two hours it was wrong -- `telemetry-vocabulary-agreement.sh` + # arrived on develop and no prose copy followed it, while the header eight + # lines above claimed this job "holds no list of its own" (Asad + Arturo, + # #755 review). A second copy of a list is the defect this job exists to + # catch; it does not get an exemption for being a comment. `make drift` + # prints the live set and its count on every run. + # + # These guards moved here from installer-tests.yaml's `static` job on + # 2026-08-19, because `Static analysis` is required on no branch: a PR could + # change an installer script, leave manifest.sha256 stale, and merge on its + # required checks. + - name: Duplicated-declaration guards (make drift) + run: make drift diff --git a/.github/workflows/installer-tests.yaml b/.github/workflows/installer-tests.yaml index c167b0bc..2ebf6b70 100644 --- a/.github/workflows/installer-tests.yaml +++ b/.github/workflows/installer-tests.yaml @@ -2,9 +2,6 @@ name: Installer tests # Validates the curl/PowerShell installer (scripts/) across the breadth of # environments a customer might actually have: -# • static — bash -n + PSScriptAnalyzer + the repo's drift guards -# (shellcheck is NOT here: it is the `quality / shellcheck` -# required check — see the `static` job for why) # • unit-bash — bats (mocked) for the bash installer # • unit-pester— Pester for the PowerShell installer, on Linux AND real Windows # • distro-prereqs — runs the REAL Linux prerequisite-install path (package @@ -18,11 +15,6 @@ on: paths: - 'scripts/**' - '.github/workflows/installer-tests.yaml' - # #616: the check-facts drift gate enforces the GPU node image's k3s pin against - # facts.env — run it when those files change too, else a K3S_TAG edit could drift - # and merge green (Bugbot). - - 'docker/k3s-cuda/**' - - '.github/workflows/build-k3s-cuda.yaml' pull_request: # `labeled` is required so adding the `e2e` label to an open PR starts the # e2e-journey job immediately — with the default types it would only fire on @@ -32,11 +24,6 @@ on: paths: - 'scripts/**' - '.github/workflows/installer-tests.yaml' - # #616: the check-facts drift gate enforces the GPU node image's k3s pin against - # facts.env — run it when those files change too, else a K3S_TAG edit could drift - # and merge green (Bugbot). - - 'docker/k3s-cuda/**' - - '.github/workflows/build-k3s-cuda.yaml' schedule: - cron: '0 3 * * 1' # Mondays 03:00 UTC — catch drift as distro base images move workflow_dispatch: @@ -52,91 +39,30 @@ concurrency: cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: - static: - timeout-minutes: 10 - name: Static analysis - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - - name: bash -n (syntax) on every shell script - run: | - # .bats files are bats DSL (@test "name" { … }), not valid bash — they are - # syntax-checked by actually running them in the unit-bash job. Parse the - # real shell scripts here. - find scripts -type f -name '*.sh' -print0 \ - | while IFS= read -r -d '' f; do bash -n "$f" || exit 1; done - echo "all shell scripts parse" - - # There is no shellcheck step here any more, and adding one back is a - # regression. shellcheck ran in THREE places in this repo (#753): here, - # `Standard checks / Lint`, and `quality / shellcheck` from - # tracebloc/.github's code-quality.yml. This copy is the one with no - # reason to exist: - # - # • It is not a REQUIRED check on develop (this whole job isn't), so it - # could never block anything — it was advice, not a gate. Making the - # R8 step below a gate is backend#2182 / #2183, a separate concern. - # • It ENUMERATED its file set. Measured on develop 2026-08-19: the list - # that used to sit here expanded to 34 files where deriving from the - # tree yields 44 — it had drifted past eight real scripts, including - # scripts/chart-version-guard.sh, scripts/check-digest-drift.sh and - # scripts/index-invariants.sh, and #747 then had to hand-edit the list - # in three places to add one new script. A parse error planted in - # index-invariants.sh was invisible to this step. - # • shellcheck is preinstalled on ubuntu-latest, so it needs no - # `apt-get install` — the very step that hung this job to its - # 10-minute timeout twice on 2026-08-19 and reported as `cancelled`. - # - # The two survivors are complementary rather than duplicated, and both - # derive their file set: `Standard checks / Lint` runs `make lint` over - # the WHOLE TREE, and `quality / shellcheck` scans the PR DIFF. Same - # severity (error) and same SC1091 exclusion as the step deleted here, so - # no verdict changes. - - - name: Installer manifest is current (supply-chain, R8) - # The bootstrap verifies each sub-script against scripts/manifest.sha256 - # before running the privileged steps. If a sub-script changed but the - # manifest didn't, the release would publish a manifest that rejects the - # very scripts it ships — so fail the PR until the manifest is regenerated. - # Also cross-checks the bootstrap's FILES list against gen-manifest.sh. - run: | - chmod +x scripts/gen-manifest.sh - scripts/gen-manifest.sh --check - - - name: Installer facts are single-sourced (#435, RFC D3/D4) - # Cross-OS facts (tool version pins) live once in scripts/spec/facts.env and are - # stamped into every consumer (bash common.sh, PowerShell install-k8s.ps1). Fail - # the PR if a consumer drifted from the spec — so a pin bumped in one OS path but - # not the other (the #410 incident) can no longer ship. Fix: check-facts.sh --write. - run: | - chmod +x scripts/check-facts.sh - scripts/check-facts.sh --check - - - name: Style + terminology guard - # Enforces the terminal style system (STYLE.md): no hardcoded brand colour - # outside the tone engine (common.sh), no status/traffic-light emoji, and - # "secure environment" not "workspace" in user-facing text. Mechanical - # checks only — role/wording judgement stays with CODEOWNERS review. - run: bash scripts/check-style.sh - - - name: PSScriptAnalyzer (PowerShell installer) - shell: pwsh - run: | - Set-PSRepository PSGallery -InstallationPolicy Trusted - Install-Module PSScriptAnalyzer -Force -SkipPublisherCheck -Scope CurrentUser - # Both PowerShell entrypoints (the bootstrap install.ps1, R8 trust root, and the - # main installer install-k8s.ps1) plus the Windows e2e driver (#436) — the driver - # only RUNS on the self-hosted nested-virt runner, but lint it here on every push - # so a syntax/verb regression fails fast instead of at the nightly run. - $issues = @() - foreach ($p in 'scripts/install.ps1', 'scripts/install-k8s.ps1', 'scripts/tests/e2e-windows.ps1') { - $issues += Invoke-ScriptAnalyzer -Path $p -Severity Error,Warning - } - if ($issues) { $issues | Format-Table -AutoSize } - $errs = @($issues | Where-Object { $_.Severity -eq 'Error' }) - if ($errs.Count -gt 0) { Write-Error "PSScriptAnalyzer: $($errs.Count) error(s)"; exit 1 } - Write-Host "no PSScriptAnalyzer errors" + # The `static` job lived here until 2026-08-19. It is gone, and every part of it + # now sits behind a check that can actually block. + # + # #753/#754 had already deleted its shellcheck step (three copies of shellcheck, + # only this one non-gating and enumerated) and said the R8-step-as-a-gate + # question was backend#2182/#2183's to answer. This is that answer: `Static + # analysis` is a required status check on NO branch — verified against develop, + # staging, main and both rulesets — so R8, the cross-OS facts check, the style + # guard and PSScriptAnalyzer could annotate a PR and never stop one. Worse, they + # shared this job's `timeout-minutes: 10` with an apt-get that ate the whole + # budget five times on client#752; all four reported `skipped` while every + # required check went green. + # + # Where each part went: + # bash -n -> standard-checks.yml `Lint` (required), via `make parse` + # PSScriptAnalyzer -> standard-checks.yml `Lint` (required) + # gen-manifest --check -> drift-checks.yaml `Source-of-truth drift` (required) + # check-facts --check -> drift-checks.yaml `Source-of-truth drift` (required) + # check-style.sh -> drift-checks.yaml `Source-of-truth drift` (required) + # + # The three guard moves go through the Makefile's `drift` target, so the gate and + # the pre-push hook read one list instead of two that had already diverged. This + # job keeps only what nothing else can do: the real bats suite, Pester on real + # Windows, the 9-distro prereq matrix, PATH persistence and the e2e runs. unit-bash: # 10 -> 15. At 10 the bounded install (390 s worst case) plus ~155 s of bats diff --git a/.github/workflows/standard-checks.yml b/.github/workflows/standard-checks.yml index b3a16d63..9a9d3c3f 100644 --- a/.github/workflows/standard-checks.yml +++ b/.github/workflows/standard-checks.yml @@ -10,10 +10,22 @@ name: Standard checks # forever at "Expected — waiting for status to be reported", blocking the merge. # # These jobs reuse the same proven steps as `installer-tests.yaml`'s deeper -# suite (which keeps the broader coverage: PSScriptAnalyzer, Pester on Windows, -# distro-prereqs, e2e). If the two ever feel redundant, fold the deep suite's -# names into these and drop the duplicates — but keep SOMETHING reporting -# `Lint` + `Unit tests` unconditionally. +# suite (which keeps the broader coverage: Pester on Windows, distro-prereqs, +# e2e). Keep SOMETHING reporting `Lint` + `Unit tests` unconditionally. +# +# That "if the two ever feel redundant, fold the deep suite's names into these +# and drop the duplicates" invitation was taken on 2026-08-19: installer-tests' +# `static` job restated this job's parse loop and its exact 19-file shellcheck +# sweep, and carried four guards that gated nothing because `Static analysis` is +# a required check on no branch. The job is gone. Its shellcheck and bash -n are +# `make lint` here, its PSScriptAnalyzer is a step below, and its three +# duplicated-declaration guards are in drift-checks.yaml's required job. +# +# STILL NOT GATED, and deliberately out of scope of that change: Pester. The +# PowerShell installer's unit suite reports as `Pester (windows-latest)` / +# `Pester (ubuntu-latest)`, neither of which is a required context, so `Unit +# tests` below gates bats only. Adding it means a required check on a +# windows-latest runner -- a cost and flake question, not a cleanup. on: push: @@ -66,6 +78,65 @@ jobs: # (not just a newer local build) before arming it. run: make lint + # Advisory only, and it carries no file list of its own -- `make + # lint-warnings` reuses SHELLCHECK_FILES. Inherited from the deleted + # `static` job so the SC2034 visibility survives the move. `|| true` lives + # in the target; this step cannot fail the check. + - name: shellcheck warnings (advisory, non-blocking) + # `!cancelled()` not `always()`: run even when `make lint` above failed + # (both halves are mechanical and independent, so reporting one and hiding + # the other costs a round trip), but do NOT run after a cancel or a + # timeout-kill — `always()` would burn what is left of this job's + # `timeout-minutes: 10` on a job that is already lost. + if: ${{ !cancelled() }} + run: make lint-warnings + + # PSScriptAnalyzer moved here from installer-tests.yaml's `static` job on + # 2026-08-19, for the same reason R8 moved to the drift job: `Static + # analysis` is a required check on no branch, so the PowerShell lint could + # only advise. `Lint` IS required on develop and on main, and a lint of + # scripts/install.ps1 -- the R8 trust root on Windows -- belongs behind a + # gate rather than beside one. + # + # NOT in `make lint`: it needs pwsh and a PSGallery fetch, which most dev + # machines have neither of, and a `make check` that fails on a missing pwsh + # is a `make check` people stop running. Skipping it locally is fine + # precisely because this job cannot skip it -- `shell: pwsh` fails the + # REQUIRED check outright if pwsh is ever absent. + # + # `if: !cancelled()` so a red `make lint` above still reports the PowerShell + # verdict in the same run. Both are mechanical and independent; making the + # author fix one to discover the other is two round trips for no reason. + - name: PSScriptAnalyzer (PowerShell installer) + if: ${{ !cancelled() }} + shell: pwsh + run: | + # Use the module the image already ships; only fetch if a future image + # drops it. PSScriptAnalyzer 1.25.0 is preinstalled on ubuntu-latest and + # was already loaded ("currently in use") while this step spent 38 s of + # the job's 59 s re-fetching it from PSGallery (Arturo, #755 review) -- + # a network dependency inside a REQUIRED check, buying nothing. Same + # reasoning that removed `apt-get install shellcheck` in #753. + if (-not (Get-Module -ListAvailable -Name PSScriptAnalyzer)) { + Write-Host "PSScriptAnalyzer not on the image - installing from PSGallery" + Set-PSRepository PSGallery -InstallationPolicy Trusted + Install-Module PSScriptAnalyzer -Force -SkipPublisherCheck -Scope CurrentUser + } + Import-Module PSScriptAnalyzer + Write-Host "PSScriptAnalyzer $((Get-Module PSScriptAnalyzer).Version)" + # Both PowerShell entrypoints (the bootstrap install.ps1, R8 trust root, and the + # main installer install-k8s.ps1) plus the Windows e2e driver (#436) — the driver + # only RUNS on the self-hosted nested-virt runner, but lint it here on every push + # so a syntax/verb regression fails fast instead of at the nightly run. + $issues = @() + foreach ($p in 'scripts/install.ps1', 'scripts/install-k8s.ps1', 'scripts/tests/e2e-windows.ps1') { + $issues += Invoke-ScriptAnalyzer -Path $p -Severity Error,Warning + } + if ($issues) { $issues | Format-Table -AutoSize } + $errs = @($issues | Where-Object { $_.Severity -eq 'Error' }) + if ($errs.Count -gt 0) { Write-Error "PSScriptAnalyzer: $($errs.Count) error(s)"; exit 1 } + Write-Host "no PSScriptAnalyzer errors" + unit-tests: name: Unit tests runs-on: ubuntu-latest diff --git a/Makefile b/Makefile index c9bf962d..548949f7 100644 --- a/Makefile +++ b/Makefile @@ -10,13 +10,16 @@ # that runs `make check` (skip once with --no-verify). # # This file is a THIN WRAPPER. Almost every command below is copied from the -# workflow that already runs it — standard-checks.yml, installer-tests.yaml -# and helm-ci.yaml. It introduces no new tool, no new config, and no new -# rule. When a workflow changes, change the matching line here. +# workflow that already runs it — standard-checks.yml, installer-tests.yaml, +# drift-checks.yaml and helm-ci.yaml. It introduces no new tool, no new config, +# and no new rule. When a workflow changes, change the matching line here. # -# `shellcheck` is the one target not copied from a workflow LINE, but it is -# still exactly what a workflow runs: since #753, `Standard checks / Lint` runs -# `make lint`, so this file IS the definition rather than a copy of one. The org +# `shellcheck` and `drift` are the two targets not copied from a workflow LINE, +# but both are still exactly what a workflow runs: since #753, `Standard checks +# / Lint` runs `make lint`, and since #755 the REQUIRED `Source-of-truth drift` +# runs `make drift`. For those two this file IS the definition rather than a copy +# of one — which is the point: the pre-push tier and the merge gate cannot +# disagree about what linting, or drift, means. The org # reusable job `quality / shellcheck` (tracebloc/.github) applies the same rule # to the PR diff; that one cannot be copied from, so this target reproduces its # classification and flags, and says so in its own comment. @@ -262,46 +265,129 @@ parse: # materialised first and shellcheck's own status is what propagates. # The `; :` inside the classifier is load-bearing: `grep -q ... && printf` # exits 1 on every non-shell file, which would otherwise make xargs return 123. +# Both shellcheck sweeps read ONE definition of "which files are shell": +# $(SH_FILES). Overridable ONLY so the fail-closed path can be exercised without +# editing the real classifier -- `make shellcheck SH_FILES=/path/to/stub`. See that file's header for why it is a script and not a +# list -- in short, #753 replaced a 19-entry SHELLCHECK_FILES enumeration with a +# derivation after the list drifted past eight real scripts, and this PR's +# advisory sweep then kept referencing the deleted variable and went permanently +# green with no operands behind `|| true` (Arturo, #755 review). The script exits +# non-zero on a zero-file classification, so a broken derivation is loud in both +# sweeps instead of looking like a clean run in either. +SH_FILES ?= scripts/sh-files.sh + .PHONY: shellcheck shellcheck: @files=$$(mktemp); \ - git ls-files -z \ - | xargs -0 -r -n1 sh -c 'case "$$1" in \ - *.sh|*.bash|*.ksh) printf "%s\n" "$$1" ;; \ - *.bats|*.ps1|*.psm1|*.zsh) ;; \ - *) head -n 1 "$$1" 2>/dev/null \ - | grep -Eq "^#![[:space:]]*[^[:space:]]*(/|[[:space:]])(ba|da|k)?sh([[:space:]]|$$)" \ - && printf "%s\n" "$$1" ;; \ - esac; :' sh > "$$files"; \ + if ! $(SH_FILES) > "$$files"; then rm -f "$$files"; exit 1; fi; \ n=$$(wc -l < "$$files" | tr -d " "); \ - if [ "$$n" -eq 0 ]; then \ - echo "shellcheck: classified ZERO shell files -- the derivation above is broken."; \ - echo " Refusing to report green on an empty file set."; \ - rm -f "$$files"; exit 1; \ - fi; \ echo "shellcheck: $$n file(s), severity=error"; \ tr "\n" "\0" < "$$files" | xargs -0 -r shellcheck --severity=error --exclude=SC1091; \ rc=$$?; rm -f "$$files"; exit $$rc -# drift: the repo's duplicated-declaration guards. The first three come from -# installer-tests.yaml's `static` job; all three are pure local file -# comparisons (~0.2 s) and all three have a --write / regenerate mode named in -# their own output. -# -# The fourth is the CLIENT_ENV vocabulary-agreement guard (backend#1729 -# sweep 5). It lives here, not in `helm-vocab`, because #715 moved it out of -# helm-ci's `Helm lint` into drift-checks.yaml's `Source-of-truth drift` job -- -# the one that is a REQUIRED check, so the guard can block rather than advise. -# helm-ci's lint job calls `make helm-lint helm-vocab`, so keeping the guard in -# `helm-vocab` would silently put it back where #715 took it from. ~2 s, bash -# and python3 only. +# lint-warnings: the advisory `--severity=warning` sweep, over the SAME derived +# set as `shellcheck` -- same script, so the two cannot diverge again. +# +# NOT part of `lint` and NOT a gate: the libs are sourced together as one +# program, so single-file shellcheck reports SC2034 "unused" for shared vars +# (CURL_SECURE, ARCH_DL, the colours) that common.sh defines and other libs +# consume. Printed for visibility only. +# +# `|| true` sits on the shellcheck INVOCATION only, never around the derivation +# -- that is the distinction the old one-liner lost. A broken classifier exits 1 +# through sh-files.sh; only genuine warnings are tolerated. +# +# It exists as a target because installer-tests.yaml's `static` job used to run +# this sweep inline, and that job is gone; standard-checks.yml's `Lint` calls +# this so the visibility survives the move rather than being quietly dropped. +.PHONY: lint-warnings +lint-warnings: + @files=$$(mktemp); \ + if ! $(SH_FILES) > "$$files"; then rm -f "$$files"; exit 1; fi; \ + n=$$(wc -l < "$$files" | tr -d " "); \ + echo "lint-warnings: $$n file(s), severity=warning (advisory)"; \ + tr "\n" "\0" < "$$files" | xargs -0 -r shellcheck --severity=warning --exclude=SC1091 || true; \ + rm -f "$$files" + +# drift: the repo's duplicated-declaration guards, and the ONLY declaration of +# that set. drift-checks.yaml's `Source-of-truth drift` job -- the one that is a +# REQUIRED check on develop and on main -- runs `make drift` and lists nothing +# itself, so a guard added here gates automatically and the pre-push hook and +# the merge gate cannot disagree about what "the drift guards" are. +# +# They could, and did. Until 2026-08-19 this target held gen-manifest / +# check-facts / check-style while the required job held check-drift, so each +# side gated exactly what the other did not: a stale manifest.sha256 was caught +# by the pre-push hook and NOT at the merge gate, because the job that ran R8 +# (`Static analysis`) is required on no branch. Hence one list, here. +# +# No count is written down. It was "five" for about two hours and was already +# six -- `telemetry-vocabulary-agreement.sh` arrived with a develop merge and +# neither prose copy followed it (Asad + Arturo, #755 review), which is the exact +# divergence this target exists to stop. The recipe prints the live number. +# +# Most are pure local file comparisons with a --write / regenerate mode named in +# their own output. `check-drift.sh` is the exception and wants `helm template`; +# an earlier version of this comment claimed all of them regenerate, one sentence +# before admitting that one shells out to helm. +# +# env-vocabulary-agreement lives here, not in `helm-vocab`, because #715 moved +# it out of helm-ci's `Helm lint` into the required drift job; helm-ci's lint job +# calls `make helm-lint helm-vocab`, so keeping it there would silently put it +# back where #715 took it from. +# +# `|`-separated because each guard is a multi-word command. One entry per guard, +# and this is the only place they are written down. +DRIFT_GUARDS := scripts/gen-manifest.sh --check|scripts/check-facts.sh --check|bash scripts/check-style.sh|bash scripts/tests/check-drift.sh|bash scripts/tests/env-vocabulary-agreement.sh|bash scripts/tests/telemetry-vocabulary-agreement.sh + +# EXPORTED, not interpolated. The recipe reads $$DRIFT_GUARDS from the +# environment; it used to do `guards='$(DRIFT_GUARDS)'`, which Make expands +# INSIDE single quotes, so the first guard containing a `'` -- `bash -c '...'`, +# `python3 -c '...'` -- would terminate the assignment, collapse the list, and +# run zero guards while printing "all guards green" and exiting 0. In the check +# this PR makes REQUIRED. Found by Asad, reproduced independently by Arturo and +# again here (#755 review): +# +# $ make drift DRIFT_GUARDS= -> green, exit 0 +# $ make drift "DRIFT_GUARDS=bash -c 'exit 1'|scripts/..." -> green, exit 0 +# +# The irony was the point: this PR's whole argument is that a guard which cannot +# fail is not a gate, and its own comment here claimed "this is NOT the fail-open +# shape" -- a claim that should have been a machine check (backend#1729 rule 7). +# Now it is one, three ways: +# * an empty list is a FAILURE, not a clean sweep; +# * the environment carries the value, so no quote in a guard can collapse it; +# * the loop COUNTS its iterations and refuses to report green unless it ran +# exactly the number of `|`-separated entries the list declares. +export DRIFT_GUARDS + +# Every guard RUNS even when an earlier one fails, and the target fails at the +# end if any did -- so a stale manifest no longer hides a terminology violation +# until you fix the manifest, push, and wait for CI again. The guards are +# independent in their REPORTING, never in whether they block. .PHONY: drift drift: - scripts/gen-manifest.sh --check - scripts/check-facts.sh --check - bash scripts/check-style.sh - bash scripts/tests/env-vocabulary-agreement.sh - bash scripts/tests/telemetry-vocabulary-agreement.sh + @if [ -z "$${DRIFT_GUARDS:-}" ]; then \ + echo "drift: the guard list is EMPTY -- refusing to report green on zero guards."; \ + exit 1; \ + fi; \ + exp=$$(printf '%s' "$$DRIFT_GUARDS" | awk -F'|' '{print NF}'); \ + fail=0; ran=0; oifs=$$IFS; IFS='|'; \ + for g in $$DRIFT_GUARDS; do \ + IFS=$$oifs; ran=$$((ran+1)); \ + printf '\n==> %s\n' "$$g"; \ + sh -c "$$g" || { fail=1; printf '!! FAILED: %s\n' "$$g"; }; \ + IFS='|'; \ + done; IFS=$$oifs; \ + if [ "$$ran" -ne "$$exp" ]; then \ + printf '\ndrift: ran %s guard(s) but the list declares %s -- refusing to report green.\n' "$$ran" "$$exp"; \ + exit 1; \ + fi; \ + if [ "$$fail" -ne 0 ]; then \ + printf '\ndrift: one or more guards FAILED (all %s were run -- see !! lines above)\n' "$$ran"; \ + exit 1; \ + fi; \ + printf '\ndrift: all %s guards green\n' "$$ran" # digest-drift: the watcher on every mutable label that points at a pinned # digest (backend#1853). NOT in `check`: it needs the network and a docker diff --git a/docs/SUPPLY_CHAIN.md b/docs/SUPPLY_CHAIN.md index 92aecbda..25eb002a 100644 --- a/docs/SUPPLY_CHAIN.md +++ b/docs/SUPPLY_CHAIN.md @@ -96,9 +96,35 @@ job (runs on `release: published`, same trigger as the chart publish) that: 6. Attaches `install.sh`, `manifest.sha256`, `manifest.sha256.sig`, `manifest.sha256.cert` to the release. -A CI gate in `installer-tests.yaml` (`gen-manifest.sh --check`) fails any PR that -changes a sub-script without regenerating the committed manifest, so the -in-repo manifest never drifts from the scripts it covers. +`gen-manifest.sh --check` runs in the **`Source-of-truth drift`** job +(`drift-checks.yaml`, via `make drift`) and fails any PR that changes a +sub-script without regenerating the committed manifest, so the in-repo manifest +never drifts from the scripts it covers. That job is a **required status check** +on `develop` and on `main` — which is the whole reason the check lives there. + +> Until 2026-08-19 this paragraph named `installer-tests.yaml`, and the claim it +> made was false. That workflow's `Static analysis` job is a required check on no +> branch, so the R8 step could only annotate a PR, never block it — and on five +> runs of client#752 it did not even annotate: it shared a `timeout-minutes: 10` +> budget with an `apt-get`, which consumed the lot, and R8 reported `skipped` +> while every required check went green. +> +> If you move this check again, move it to a job whose check-run name is in the +> required-contexts list, and re-word this paragraph to name that job. Read the +> list with: +> +> ```bash +> gh api repos/tracebloc/client/branches/develop --jq '.protection.required_status_checks.contexts' +> ``` +> +> **Not** `branches/develop/protection` — an earlier version of this note said to +> use that, and it is admin-only. Without admin it returns +> `{"message":"Not Found","status":"404"}`, which is **indistinguishable from +> "this branch is not protected"**, so a non-admin following the old instruction +> concluded the opposite of the truth and the note defeated its own purpose +> (Arturo, #755 review). The same 404 hides ruleset-only protection from that +> endpoint even for admins. `branches/develop` → `.protection` is readable at +> plain-member level and returns the same list. ## 5. Human follow-ups required to make this fully real diff --git a/scripts/check-style.sh b/scripts/check-style.sh index 241dc765..c7c297fc 100755 --- a/scripts/check-style.sh +++ b/scripts/check-style.sh @@ -3,7 +3,10 @@ # check-style.sh — enforce the tracebloc terminal style system + terminology # on the installer scripts. See STYLE.md. # -# Runs in CI (the "Static analysis" job) and locally: bash scripts/check-style.sh +# Runs in CI as part of the REQUIRED "Source-of-truth drift" check (via +# `make drift`), and locally: bash scripts/check-style.sh +# It used to run in the "Static analysis" job, which is required on no branch — +# so until 2026-08-19 a violation here printed red and merged anyway. # Exit 0 = clean, 1 = violations found, 2 = the guard itself errored (fail-closed). # # Three mechanical checks (semantic calls — role misuse, judgement-y wording — diff --git a/scripts/sh-files.sh b/scripts/sh-files.sh new file mode 100755 index 00000000..184d46ca --- /dev/null +++ b/scripts/sh-files.sh @@ -0,0 +1,57 @@ +#!/usr/bin/env bash +# ============================================================================= +# sh-files.sh — print every shell file in the repo, one per line. +# +# THE single definition of "which files are shell", read by the Makefile's +# `shellcheck` target (severity=error, gating, run by the required +# `Standard checks / Lint`) and by `lint-warnings` (severity=warning, +# advisory). It is a file rather than a Make `define` because a multi-line +# define cannot end in a line continuation without swallowing its own `endef`, +# and a single-line Make variable cannot contain the `#!` of a shebang regex +# without starting a Make comment. A script has neither problem, and is itself +# both testable and lintable by the very sweep it feeds. +# +# (That last line is deliberately not phrased with the linter's name first: a +# comment STARTING with it is inline-directive syntax, and an unparseable +# directive is an SC1073 *error* — which this file would then fail on, in the +# gating sweep it defines. The org's code-quality.yml carries the same warning.) +# +# Why one definition at all: #753 replaced a 19-entry SHELLCHECK_FILES list +# with a derivation because the list had drifted past eight real scripts (the +# SECOND such drift). The gating half got a zero-count guard; the advisory half +# kept referencing the deleted variable, expanded to no operands, and went +# permanently green behind `|| true`. Two sweeps over "the same file set" +# diverge unless they literally read the same code. +# +# The rule is the one `quality / shellcheck` applies (tracebloc/.github's +# code-quality.yml): classify by extension, else by shebang; skip +# .bats/.ps1/.psm1/.zsh. +# +# Exit 0 = one or more files printed. Exit 1 = the derivation found NOTHING, +# which is a broken derivation, not an empty repo — callers must treat it as a +# failure rather than a clean sweep (backend#1729 rule 3, fail closed). +# ============================================================================= +set -uo pipefail + +cd "$(dirname "${BASH_SOURCE[0]}")/.." || exit 1 + +count=0 +while IFS= read -r -d '' f; do + [ -f "$f" ] || continue + case "$f" in + *.sh|*.bash|*.ksh) printf '%s\n' "$f"; count=$((count+1)) ;; + *.bats|*.ps1|*.psm1|*.zsh) ;; + *) + if head -n 1 "$f" 2>/dev/null \ + | grep -Eq '^#![[:space:]]*[^[:space:]]*(/|[[:space:]])(ba|da|k)?sh([[:space:]]|$)'; then + printf '%s\n' "$f"; count=$((count+1)) + fi + ;; + esac +done < <(git ls-files -z) + +if [ "$count" -eq 0 ]; then + echo "sh-files: classified ZERO shell files — the derivation is broken." >&2 + echo " Refusing to report an empty set as success." >&2 + exit 1 +fi