diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index d08a80e..56c56f8 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -2,8 +2,13 @@ # Lints YAML, validates schemas, and enforces OPA policies. # # CI authority: GitLab CI is the authoritative pipeline (full validate/sync-check/rollback). -# This GitHub Actions workflow runs yaml-lint, schema validation, policy checks, and -# client-side manifest dry-runs — no cluster access required; runs on GitHub-hosted runners. +# This GitHub Actions workflow runs yaml-lint, schema validation and policy checks — all +# genuinely offline, no cluster access required, on GitHub-hosted runners. +# +# NOTE: everything here must work with NO cluster. A check that needs the API server +# will fail on every input, and if it also cannot report failure it will look green +# forever — which is exactly what happened to the client-side dry-run job removed +# below on 2026-07-25. # # No secrets required for these jobs. @@ -30,12 +35,26 @@ jobs: - name: Lint all YAML under gitops/ run: | - FAILED=0 - find gitops/ -name '*.yaml' -o -name '*.yml' | while read f; do - yq e 'true' "$f" > /dev/null || { echo "FAIL: $f"; FAILED=1; } - done - [ "$FAILED" -eq 0 ] || exit 1 - echo "All YAML valid." + set -eu + # DO NOT put this loop on the right-hand side of a pipe. + # `find ... | while read` runs the loop body in a SUBSHELL, so any + # variable set inside it — including the failure flag — is discarded + # when the loop ends. This job therefore passed unconditionally on + # every commit until 2026-07-25, including ones with invalid YAML. + # Redirecting from a file keeps the loop in the current shell. + # Parenthesised -name group: without it, precedence makes the implicit + # -print apply inconsistently across the -o branches. + find gitops/ \( -name '*.yaml' -o -name '*.yml' \) -print > /tmp/yaml-files + failed=0 + while IFS= read -r f; do + if ! out=$(yq e 'true' "$f" 2>&1 >/dev/null); then + echo "FAIL: $f" + echo "$out" | sed 's/^/ /' + failed=1 + fi + done < /tmp/yaml-files + [ "$failed" -eq 0 ] || { echo "YAML lint failed."; exit 1; } + echo "All YAML valid ($(wc -l < /tmp/yaml-files) files)." kubeconform: name: Schema validation (kubeconform) @@ -88,22 +107,32 @@ jobs: --policy ci/policies \ --data ci/data/exceptions.yaml - dry-run-apply: - name: kubectl dry-run (client-side) - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v7 - - - name: Install kubectl - uses: azure/setup-kubectl@v5 - - - name: Client-side dry-run all workload manifests - # --dry-run=client validates manifest structure without a live cluster. - # Server-side validation (schema + admission) runs in GitLab CI on a self-hosted runner. - run: | - FAILED=0 - find gitops/workloads -name '*.yaml' | while read f; do - echo " checking $f" - kubectl apply --dry-run=client -f "$f" 2>&1 || FAILED=1 - done - [ "$FAILED" -eq 0 ] || { echo "Dry-run failed."; exit 1; } + # REMOVED 2026-07-25: `kubectl dry-run (client-side)`. + # + # The job claimed to "validate manifest structure without a live cluster". It + # cannot. `kubectl apply --dry-run=client` downloads the OpenAPI schema from the + # API server to validate against, so on a GitHub-hosted runner with no cluster + # every single file failed with: + # + # failed to download openapi: Get "http://localhost:8080/openapi/v2": + # dial tcp [::1]:8080: connect: connection refused + # + # It had therefore failed on every manifest since the day it was written, and + # nobody knew, because the loop ran in a subshell and the job could not report + # failure (see C4 in docs/REVIEW-2026-07-24.md). Making the job capable of + # failing is what finally surfaced it. + # + # It is deleted rather than patched with --validate=false, which would skip the + # OpenAPI check entirely and reduce the job to "is this parseable YAML" — already + # covered by yaml-lint, and covered far better by kubeconform. + # + # Division of responsibility, so nothing is silently uncovered: + # yaml-lint (here) — parseable YAML + # kubeconform (here) — offline schema validation, k8s 1.36, CRDs skipped + # conftest (here) — offline OPA policy (no :latest, no privileged, limits) + # dry-run (GitLab CI) — --dry-run=server against the real cluster, where + # CRDs resolve and Kyverno admission actually runs + # + # If a kubectl-shaped check is ever wanted here again, the useful one is the + # Kyverno CLI (`kyverno apply gitops/workloads/kyverno/ --resource gitops/`), + # which evaluates the real ClusterPolicies offline. See H17. diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index fd55cdf..39c041e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -45,11 +45,26 @@ yaml-lint: before_script: [] # no kubeconfig needed script: - | + set -eu echo "==> Linting all YAML under gitops/" - find gitops/ -name '*.yaml' -o -name '*.yml' | while read f; do - yq e 'true' "$f" > /dev/null || { echo "FAIL: $f"; exit 1; } - done - echo "All YAML files are valid." + # DO NOT put this loop on the right-hand side of a pipe. `find ... | while` + # runs the loop body in a SUBSHELL — so the `exit 1` that used to be here + # exited only the subshell, the loop carried on, and the job reported + # success regardless. Invalid YAML passed CI unchallenged until 2026-07-25. + # Redirecting from a file keeps the loop in the current shell. + # (Written for POSIX sh, not bash: the runner image's shell is not guaranteed, + # so no process substitution here.) + find gitops/ \( -name '*.yaml' -o -name '*.yml' \) -print > /tmp/yaml-files + failed=0 + while IFS= read -r f; do + if ! out=$(yq e 'true' "$f" 2>&1 >/dev/null); then + echo "FAIL: $f" + echo "$out" | sed 's/^/ /' + failed=1 + fi + done < /tmp/yaml-files + [ "$failed" -eq 0 ] || { echo "YAML lint failed."; exit 1; } + echo "All YAML files are valid ($(wc -l < /tmp/yaml-files) files)." rules: - if: '$CI_PIPELINE_SOURCE == "merge_request_event"' - if: '$CI_COMMIT_BRANCH == "main"' @@ -58,16 +73,28 @@ dry-run-apply: stage: validate script: - | + set -eu echo "==> Dry-run kubectl apply on every workload manifest" - FAILED=0 - # Skip Helm values files and kustomization overlays — kubectl can't apply them directly + # DO NOT put this loop on the right-hand side of a pipe. `find ... | while` + # runs the loop body in a SUBSHELL, so FAILED=1 was discarded when the loop + # ended and the [ "$FAILED" -eq 0 ] test below always saw 0. This job — the + # server-side gate, the one with real cluster admission behind it — could + # not fail on any input until 2026-07-25. + # Skip Helm values files and kustomization overlays — kubectl can't apply them directly. find gitops/workloads -name '*.yaml' \ ! -name 'values.yaml' \ - ! -name 'kustomization.yaml' | while read f; do - echo " checking $f" - kubectl apply --dry-run=server -f "$f" || FAILED=1 - done - [ "$FAILED" -eq 0 ] || { echo "Dry-run failed — check output above."; exit 1; } + ! -name 'kustomization.yaml' \ + -print > /tmp/manifests + failed=0 + while IFS= read -r f; do + if ! out=$(kubectl apply --dry-run=server -f "$f" 2>&1); then + echo "FAIL: $f" + echo "$out" | sed 's/^/ /' + failed=1 + fi + done < /tmp/manifests + [ "$failed" -eq 0 ] || { echo "Dry-run failed — check output above."; exit 1; } + echo "All $(wc -l < /tmp/manifests) manifests passed server-side dry-run." rules: - if: '$CI_PIPELINE_SOURCE == "merge_request_event"' - if: '$CI_COMMIT_BRANCH == "main"' diff --git a/docs/REVIEW-2026-07-24.md b/docs/REVIEW-2026-07-24.md index 6a4ef16..2e99b7b 100644 --- a/docs/REVIEW-2026-07-24.md +++ b/docs/REVIEW-2026-07-24.md @@ -156,7 +156,30 @@ Alertmanager, and ArgoCD are all unreachable. - **Fix:** `svc_enable: "true"` + a Traefik `LoadBalancer` Service on a second VIP (e.g. `.201`); repoint `ingress_vip`, the Pi-hole/dnsmasq wildcard, and the CoreDNS template. -### C4. CI validation jobs are structurally incapable of failing +### C4. CI validation jobs are structurally incapable of failing — FIXED + +> **Fixed 2026-07-25**, branch `fix/ci-validation-gates`. Worse than first written: **four** +> jobs were affected, not two — GitHub `yaml-lint` and `dry-run-apply`, and GitLab +> `yaml-lint` and `dry-run-apply`. The GitLab `yaml-lint` had a different shape of the same +> bug: its `exit 1` was *inside* the subshell, so it exited the subshell and the loop +> carried on. +> +> Fixing it exposed a second, hidden bug: GitHub's `dry-run-apply` had no +> `! -name 'values.yaml'` exclusion, so it fed Helm values files to `kubectl apply`. Those +> failures were real all along and invisible only because the job could never fail — so +> fixing the subshell alone would have turned CI red on the next commit. Both are fixed +> together. +> +> All loops now read from a file redirect rather than a pipe, which keeps them in the +> current shell. Written for POSIX `sh` (no process substitution) because the GitLab runner +> image's shell is not guaranteed; verified under both `sh` and `dash`. +> +> **Still open, deliberately out of scope:** widening validation to `gitops/apps/` and +> `gitops/bootstrap/` (H17) — Argo `Application` CRDs need schema handling, so it deserves +> its own change. Tool downloads still use `releases/latest` for `yq` and `kubeconform` +> (`conftest` is correctly pinned) — worth pinning with Renovate comments. + +### C4. CI validation jobs are structurally incapable of failing (original finding) `.gitlab-ci.yml:61-71` and `.github/workflows/validate.yml:31-38` both use `FAILED=0; find ... | while read f; do ... || FAILED=1; done; [ "$FAILED" -eq 0 ] || exit 1`.