fix(ci): make validation jobs capable of failing

.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.

.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"'

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`.