From 9d7e8e5b65dad9ca0c248e1410c019302938c475 Mon Sep 17 00:00:00 2001 From: Niek Otten Date: Fri, 18 Sep 2026 13:25:24 +0000 Subject: [PATCH] =?UTF-8?q?ci(k8s):=20gate=20the=20Helm=20chart=20in=20CI?= =?UTF-8?q?=20+=20a=20compose=E2=86=94chart=20drift=20check=20(closes=20#1?= =?UTF-8?q?68)=20(#171)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What & why The Helm chart landed in #167 with two gaps written into ADR-0033: `make k8s-lint` existed but no CI job ran it, and *"a second deployment description to keep in step with compose — nothing enforces that today; a drift check belongs in CI (follow-up)"*. Both are closed here. **`make k8s-drift`** (`infra/helm/check-drift.py`, stdlib only) compares what each stack actually deploys rather than diffing two files that differ by design: workload names and resolved container images, taken from `docker compose config --format json` and a rendered chart. The six differences that exist today are declared in `DEVIATIONS` with the reason each was forced — the four `*-init` Django services folded into their web pods, and the two bootstrap Jobs compose runs from the host — so only a *new* difference fails. **A `k8s` CI job** runs `k8s-lint` then `k8s-drift` on every push and PR. No cluster, no marketplace action: helm is fetched as the pinned static binary the Talos runbook already gives developers. Closes #168 ## Definition of Done - [x] Linked Gitea issue (above). - [x] Failing test committed before the implementation — the red commit reports all six real differences; the green commit declares them. - [x] Implementation makes the test pass. - [x] Conventional Commits referencing the issue (`refs #168`). - [x] CI green — awaiting the run on this PR (`make k8s-lint` and `make k8s-drift` pass locally). - [x] `docker compose up` unaffected — no service, image or compose file is touched. - [x] Docs updated — `docs/runbooks/ci.md` (job table + the one place local and CI now differ), `docs/runbooks/kubernetes-talos.md` §7/§"not ported", and ADR-0033's cost note. - [x] No ADR needed: no new dependency (python stdlib, and helm/docker were already prerequisites of the `k8s-*` targets), no boundary moved, no §8 rule bent. - [x] Not user-visible, so no demo note. ## Notes for reviewers Verified by hand that both drift classes fail the check, not just that it passes today: - bumping `OPENZAAK_TAG` in compose alone → reports `openzaak` and `oz-celery` with both image strings; - adding a workload to `values.yaml` alone → reports it by name. Deliberate limits (there is a `ponytail:` note in the script): - **Names and images only**, as sets — no per-workload env, ports or volumes. Those differ by design in four documented places, so comparing them would mean re-encoding every deviation field by field for very little more signal. - **The three observability workloads are rendered with `enabled=true`** by the check, even though both stacks default them off, so their images can't drift unwatched. - **`k8s-lint`/`k8s-drift` are not in `make ci`**, to avoid making `helm` a hard prerequisite for everyone. That is now the only local/CI difference; it's called out in `docs/runbooks/ci.md`. Follow-ups filed while reviewing the chart, not addressed here: #169 (the published docs omit every ADR after 0010 and all runbooks but `ci.md`) and #170 (the production-posture ADR #25 asked for — secrets are still plain text in `values.yaml`).Reviewed-on: https://git.labs.respellion.tech/eho/register-referentie/pulls/171 --- .gitea/workflows/ci.yaml | 21 ++++ Makefile | 9 +- .../adr-0033-kubernetes-via-one-helm-chart.md | 5 +- docs/runbooks/ci.md | 8 +- docs/runbooks/kubernetes-talos.md | 5 +- infra/helm/check-drift.py | 116 ++++++++++++++++++ 6 files changed, 158 insertions(+), 6 deletions(-) create mode 100755 infra/helm/check-drift.py diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml index 1441ed4..0679a2e 100644 --- a/.gitea/workflows/ci.yaml +++ b/.gitea/workflows/ci.yaml @@ -41,6 +41,27 @@ jobs: nuget-${{ runner.os }}- - run: make lint + # The Helm chart's only automated gate: it renders and schema-checks the whole + # stack, and checks it still describes the same stack as the compose file + # (ADR-0033). No cluster involved — see docs/runbooks/kubernetes-talos.md. + k8s: + runs-on: ubuntu-latest + steps: + - uses: https://github.com/actions/checkout@v4 + # helm as its pinned static binary rather than a marketplace action: one URL, + # the same one the Talos runbook §0 gives a developer, and no third-party + # action to vet (CLAUDE.md §13). The drift check also needs `docker compose`, + # which the runner already has (see docs/runbooks/ci.md). + - name: Install helm + run: | + mkdir -p "$HOME/.local/bin" + curl -sSL https://get.helm.sh/helm-v3.16.4-linux-amd64.tar.gz \ + | tar xz -O linux-amd64/helm > "$HOME/.local/bin/helm" + chmod +x "$HOME/.local/bin/helm" + echo "$HOME/.local/bin" >> "$GITHUB_PATH" + - run: make k8s-lint + - run: make k8s-drift + build: runs-on: ubuntu-latest steps: diff --git a/Makefile b/Makefile index 83cd9d9..9c76f3b 100644 --- a/Makefile +++ b/Makefile @@ -43,7 +43,7 @@ export DOCKER_HOST := unix://$(PODMAN_SOCK) endif endif -.PHONY: ci lint build unit mutation frontend integration verify verify-up verify-acl verify-nrc verify-projection verify-bff verify-domain verify-observability verify-tracing verify-metrics verify-objecttypen verify-objecten verify-registerrecord verify-objecten-notifications verify-notifications smoke up down local verify-local local-down changelog openzaak-up openzaak-smoke openzaak-seed openzaak-down stack-up stack-smoke stack-down keycloak-up keycloak-smoke keycloak-down flowable-up flowable-smoke flowable-down k8s-lint k8s-registry k8s-images k8s-seed k8s-up k8s-reseed k8s-portals k8s-down k8s-purge help +.PHONY: ci lint build unit mutation frontend integration verify verify-up verify-acl verify-nrc verify-projection verify-bff verify-domain verify-observability verify-tracing verify-metrics verify-objecttypen verify-objecten verify-registerrecord verify-objecten-notifications verify-notifications smoke up down local verify-local local-down changelog openzaak-up openzaak-smoke openzaak-seed openzaak-down stack-up stack-smoke stack-down keycloak-up keycloak-smoke keycloak-down flowable-up flowable-smoke flowable-down k8s-lint k8s-drift k8s-registry k8s-images k8s-seed k8s-up k8s-reseed k8s-portals k8s-down k8s-purge help ## ci: run the full pipeline — lint, build, unit, mutation, frontend, verify (mirrors Gitea Actions) ## `verify` is the live-stack stage (full stack up once → ACL + notification checks). @@ -353,6 +353,13 @@ k8s-lint: helm lint $(K8S_CHART) helm template big $(K8S_CHART) -n $(K8S_NS) --set images.registry=registry.invalid:5000 >/dev/null +## k8s-drift: fail if compose and the Helm chart describe different stacks +# Compose is CI-canonical (ADR-0033) and the chart is a transcription of it; this +# compares what each one deploys — workload names and resolved images. Needs +# `docker compose` and `helm`, no cluster. +k8s-drift: + python3 infra/helm/check-drift.py + ## k8s-registry: deploy the in-cluster image registry (NodePort 30500) k8s-registry: kubectl apply -f infra/helm/registry.yaml diff --git a/docs/architecture/adr-0033-kubernetes-via-one-helm-chart.md b/docs/architecture/adr-0033-kubernetes-via-one-helm-chart.md index d551ebd..3bf9bd0 100644 --- a/docs/architecture/adr-0033-kubernetes-via-one-helm-chart.md +++ b/docs/architecture/adr-0033-kubernetes-via-one-helm-chart.md @@ -126,8 +126,9 @@ Consequences of that shape, each chosen deliberately: **Negative / costs** -- A second deployment description to keep in step with compose. Nothing enforces that - today; a drift check belongs in CI (follow-up). +- A second deployment description to keep in step with compose. `make k8s-drift` (#168) + now enforces the part that bites — the workload set and the resolved images, with the + four deviations below declared — but not per-workload env, ports or volumes. - `helm install` alone is not enough — the ConfigMaps must be seeded first, and a missing one surfaces as `ContainerCreating`, not as a clear error. - Generic templates mean a values typo can render valid-but-wrong YAML; `k8s-lint` catches diff --git a/docs/runbooks/ci.md b/docs/runbooks/ci.md index 5c6d3a7..0a22a95 100644 --- a/docs/runbooks/ci.md +++ b/docs/runbooks/ci.md @@ -2,8 +2,10 @@ > **Status: active.** The workflow `.gitea/workflows/ci.yaml` runs on Gitea's > hosted `ubuntu-latest` runner — no self-hosted runner required. -> **`make ci` is still the local gate** — it runs the exact same checks -> (the workflow calls the same `make` targets). +> **`make ci` is still the local gate** — it runs the same checks via the same +> `make` targets, with one exception: the `k8s` job's targets are not in `make ci`, +> because `helm` is optional for everyone not deploying to Kubernetes. Run +> `make k8s-lint k8s-drift` by hand after touching the chart or the compose file. ## The pipeline @@ -16,6 +18,8 @@ and CI cannot drift: | `lint` | `make lint` → `dotnet format … --verify-no-changes` | .NET 10 SDK | | `build` | `make build` → `dotnet build … -c Release` | .NET 10 SDK | | `unit` | `make unit` → `dotnet test … -c Release --filter "Category!=Integration"` | .NET 10 SDK | +| `frontend` | `make frontend` → Nx lint/test/build for the four portals | pnpm + Node | +| `k8s` | `make k8s-lint` (render + schema-check the Helm chart) → `make k8s-drift` (chart still describes the same stack as `infra/docker-compose.yml`) | pinned `helm` binary + `docker compose` | | `mutation` | `make mutation` → `dotnet tool restore` → `dotnet stryker` (ACL); uploads the HTML report as an artifact | .NET 10 SDK | | `verify-stack` | the single live-stack stage — steps: `make verify-up` (full stack up + health, the DoD smoke) → `make verify-acl` (ACL ↔ OpenZaak) → `make verify-nrc` (OpenZaak → NRC delivery) → `make down` | container engine + egress (base images, nuget, `selectielijst.openzaak.nl`) | diff --git a/docs/runbooks/kubernetes-talos.md b/docs/runbooks/kubernetes-talos.md index c4c1f54..52271eb 100644 --- a/docs/runbooks/kubernetes-talos.md +++ b/docs/runbooks/kubernetes-talos.md @@ -322,6 +322,7 @@ The PVCs carry `helm.sh/resource-policy: keep`, so `make k8s-down` leaves the da ```bash make k8s-lint # render + schema-check the chart, no cluster needed +make k8s-drift # fail if compose and the chart describe different stacks make k8s-portals # forward the portals + Keycloak to localhost (browser access) make k8s-images K8S_REGISTRY=... # after changing a service or a portal make k8s-up TALOS_HOST=... K8S_REGISTRY=... @@ -366,5 +367,7 @@ immutable, so `helm upgrade` is rejected with `cannot patch "…" with kind Job` `K8S_SET='--set workloads.tempo.enabled=true --set workloads.prometheus.enabled=true --set workloads.grafana.enabled=true'`. The .NET services still export OTLP; the exporter fails harmlessly when Tempo is absent. - **The verify/e2e lanes.** `make verify*` and the Playwright e2e drive compose, not the - chart. The Kubernetes path is verified with §5's smoke test. + chart. The Kubernetes path is verified with §5's smoke test. CI's `k8s` job runs the two + clusterless checks (`k8s-lint`, `k8s-drift`) on every PR — a values typo or a compose + image bump that skipped the chart fails there, but nothing deploys the chart in CI. - **Ingress, TLS, and resource requests.** See the ponytail ceiling in ADR-0033. diff --git a/infra/helm/check-drift.py b/infra/helm/check-drift.py new file mode 100755 index 0000000..e4df743 --- /dev/null +++ b/infra/helm/check-drift.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python3 +"""Fail when the compose stack and the Helm chart stop describing the same stack. + +`infra/docker-compose.yml` is CI-canonical; `infra/helm/big-reference` is a +transcription of it (ADR-0033), and until now nothing kept the two in step — an +upstream image bump or a new service applied to only one of them landed +unnoticed. This compares what each side actually *deploys*, not the two files: +the rendered chart against `docker compose config`. Both tools are already +prerequisites of the `k8s-*` make targets. + +Run it with `make k8s-drift`. No cluster needed. + +ponytail: names and images only, as sets — no per-workload env/ports/volumes. +Those differ by design in four documented places (ADR-0033), so comparing them +would mean re-encoding every deviation field by field; a tag bump and a missing +service are the drift that actually bites. +""" + +import json +import re +import subprocess +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +COMPOSE = ROOT / "infra/docker-compose.yml" +CHART = ROOT / "infra/helm/big-reference" + +# The busybox init container that every `waitFor` workload gets exists only in +# the chart (compose has `depends_on`). Rendering it under a sentinel makes it +# filterable without teaching the check what busybox is. +BUSYBOX = "drift-check-ignored-init-image" + +# Differences that Kubernetes forces, not drift (ADR-0033). A name listed here is +# expected to be on exactly one side; anything else fails. +DEVIATIONS = { + # The four Django services apply their own setup_configuration in the web pod + # (`args: [sh, -c, "/setup_configuration.sh && exec /start.sh"]`) rather than in a + # separate init Job. Both that script and /start.sh run `manage.py migrate`, and + # Kubernetes has no `depends_on: service_completed_successfully` to serialise them, + # so the Job and its web pod migrated the same database concurrently. + "oz-init": "folded into the openzaak pod", + "nrc-init": "folded into the nrc-web pod", + "objecttypen-init": "folded into the objecttypen pod", + "objecten-init": "folded into the objecten pod", + # Compose seeds these from the host — the verify scripts `docker cp` the two + # scripts into a running container, and docker-compose.local.yml carries + # `local-seed` + `nrc-subscribe` for `make local`. A cluster has no host to seed + # from, so both became Jobs in the chart. + "seed-zaaktype": "compose seeds the catalogus from the host (infra/openzaak/seed_catalogus.py)", + "nrc-subscribe": "compose registers the abonnement from the host (infra/local/register-abonnement.py)", +} + +# Workloads the observability backplane adds. Off by default in both stacks' +# defaults, so they are rendered on purpose here — otherwise their images drift +# unwatched. +OBSERVABILITY = ["tempo", "prometheus", "grafana"] + + +def compose_services() -> dict[str, str]: + """Service name -> image, with ${TAG:-default} interpolation already applied.""" + out = run(["docker", "compose", "-f", str(COMPOSE), "config", "--format", "json"]) + return {name: svc.get("image", "") for name, svc in json.loads(out)["services"].items()} + + +def chart_workloads() -> dict[str, str]: + """Workload name -> image, read back out of the rendered manifests.""" + out = run( + ["helm", "template", "big", str(CHART), "-n", "big", "--set", f"images.busybox={BUSYBOX}"] + + [f"--set=workloads.{w}.enabled=true" for w in OBSERVABILITY] + ) + workloads = {} + for doc in out.split("\n---"): + if not re.search(r"^kind: (Deployment|Job)$", doc, re.M): + continue + name = re.search(r"^ name: (\S+)$", doc, re.M)[1] + images = [i for i in re.findall(r"^\s+image: (\S+)$", doc, re.M) if i != BUSYBOX] + workloads[name] = images[0] + return workloads + + +def run(argv: list[str]) -> str: + proc = subprocess.run(argv, capture_output=True, text=True) + if proc.returncode != 0: + sys.exit(f"{argv[0]} failed:\n{proc.stderr}") + return proc.stdout + + +def main() -> int: + compose, chart = compose_services(), chart_workloads() + problems = [] + + for name in sorted(set(compose) - set(chart) - set(DEVIATIONS)): + problems.append(f" {name}: in docker-compose.yml, not in the chart") + for name in sorted(set(chart) - set(compose) - set(DEVIATIONS)): + problems.append(f" {name}: in the chart, not in docker-compose.yml") + for name in sorted(set(compose) & set(chart)): + if compose[name] != chart[name]: + problems.append(f" {name}: compose runs {compose[name]}, the chart runs {chart[name]}") + + if problems: + print("compose and the Helm chart describe different stacks:\n" + "\n".join(problems)) + print( + "\nPort the change to the other stack, or — if the difference is forced by\n" + "Kubernetes — declare it in DEVIATIONS in this file, with the reason." + ) + return 1 + + print(f"no drift: {len(chart)} workloads, images identical on both stacks") + for name, why in sorted(DEVIATIONS.items()): + print(f" deviation (declared): {name} — {why}") + return 0 + + +if __name__ == "__main__": + sys.exit(main())