ci: hold every scalar that must be a string, and the required podAffinity
Validate Helm Charts / Detect changed charts (pull_request) Successful in 4s
Validate Helm Charts / HTTPRoute match (pull_request) Successful in 15s
Validate Helm Charts / Build paperless-ngx (pull_request) Successful in 9s
Validate Helm Charts / Resource quantities (pull_request) Successful in 15s
Validate Helm Charts / Selector match (pull_request) Successful in 14s
Validate Helm Charts / Bump level (pull_request) Successful in 7s
Validate Helm Charts / YAML scalars (pull_request) Successful in 15s
Validate Helm Charts / Trivy config paperless-ngx (pull_request) Successful in 29s
Install-test / Install-test paperless-ngx (pull_request) Successful in 5m11s
Validate Helm Charts / Install-test (pull_request) Successful in 5m11s
Publish Helm Chart with patched Application Version / publish-helm-chart (push) Successful in 16s
Publish Paperless-NGX Helm Chart / Publish Paperless-NGX Helm Chart (push) Successful in 16s
SonarQube Analysis / SonarQube Analysis (push) Successful in 47s

The test Gitea #11 asked for ("whoever fixes this ships a test case that would
have caught it"), plus the guard for the fix #101 introduces.

ci/run-yaml-scalar-render-test.sh renders every chart with the fixtures the
install-test already uses and asserts three properties Kubernetes itself
requires, so no per-chart expectations are needed: a CronJob schedule is a
non-empty string, every env[].value of every container and initContainer is a
string, and every label and annotation value is a string. The first catches the
loud failure - the render aborting with a YAML error that names the template
and not the value - and the other two catch the quiet one, where the render
succeeds and the API server rejects the object at apply time.

WHY A NEW JOB AND NOT THE INSTALL-TEST. The object that carried the defect is
rendered by no fixture at all: paperless-ngx' consume CronJob sits behind
smbConnection.enabled, false in the chart defaults and false in its install-test
fixture, since the CI cluster has no SMB CSI driver. Nothing in this repository
ever produced it. So the overlays under ci/yaml-scalar/ are the test, not the
scaffolding: they switch such objects on AND feed them hostile values, and the
script fails when an overlay stops reaching its object or when the run asserted
nothing at all.

The fail path was exercised in all three shapes against a throwaway copy of the
chart as it stands on main, not reasoned about: the bare schedule aborted with
"yaml: line 6: did not find expected alphabetic or numeric character"; with only
the schedule quoted, the star-valued env entry aborted on the StatefulSet
instead; and with only the bool-looking value left - a render that succeeds -
the type assertion named the variable, the container and the API server's
message. Green repo-wide otherwise: 30 charts, 408 scalars, 0 failed.

ci/run-selector-match-test.sh additionally holds every
requiredDuringSchedulingIgnoredDuringExecution podAffinity term against all pod
templates of the same render (Gitea #101). That case is not the gap the script's
header describes: a preference that cannot be met is dropped and an
anti-affinity over zero pods is satisfied, but a REQUIRED term matching no pod
can never be satisfied and the pod stays Pending for good. paperless-ngx' mover
selects a label written in another template, which is exactly the copy this test
holds - renaming it would leave both templates rendering perfectly while the
mover silently stopped moving. Exercised by doing that rename in a throwaway
copy: one FAIL naming the selector, the pod templates it missed and the labels
actually present.

Six charts still render their env block bare (matrix-synapse, minecraft-router,
minecraft-server, rallly, satisfactory, teamspeak-server). They are green only
because no fixture of theirs sets a hostile value; README now says so instead of
repeating template-chart's claim that every chart quotes.

Part of #11
Part of #101
This commit was merged in pull request #131.
This commit is contained in:
jklein committed 2026-10-13 19:10:26 +00:00
1 parent 4b2bde505d
commit 39844fc92c
6 files changed
+622 -9

No files matched your search

+27 -5
View File
@@ -68,8 +68,8 @@ Every chart has a wiki page of its own — `https://git.jk-dev.de/jlab-cluster/h
Note: the registry **accepts a re-upload of an already published version** (HTTP 201) and replaces the stored artifact, so a republish of an existing version is still silent — see #160. That is also what makes a manual `workflow_dispatch` of any `publish-<chart>.yml` a harmless way to prove the shared block works.
- `change-app-version.yml` — manually dispatched app-version bump; the chart must be listed in its `chart-name` choice list.
- `ci/helm-dependencies.sh <chart-dir>` — not a workflow but the step they all share: fetches a chart's declared dependencies into `charts/`, and returns silently for the charts that declare none (all but three: `ytdlp-web-player`, `stalwart`, `paperless-ngx`). It runs in **seven** places, because every helm command this repo builds on exits 1 while a declared dependency is missing: before lint and package in `validate-charts.yml`, before package in the shared publish block (which is why that block names this script as a prerequisite of its callers — see above), and inside `ci/run-install-test.sh`, `ci/run-quantity-render-test.sh`, `ci/run-selector-match-test.sh`, `ci/run-httproute-match-test.sh` and `ci/run-trivy-config.sh`. See "Chart dependencies" in the style guidelines for what that costs.
- `validate-charts.yml` — PR pipeline: detects which charts a PR touches and, for exactly those, runs `helm lint --strict`, `helm package`, and a chart-version bump check (`template-chart` is exempt from the version check). PRs without chart changes skip the build matrix. After the build stage, an **install-test** job (the same changed-charts matrix, minus the charts listed in `ci/excluded-charts.txt`) installs each changed chart into an ephemeral [k3d](https://k3d.io) cluster on the runner and requires every workload to become Ready — see below. Both matrices follow the changed chart directories, so a PR that only changes the harness under `ci/` exercises nothing by itself; the Trivy scan has a fixed-sample fallback for that case, the install-test has none — bump the affected chart's patch version to pull it into the matrix, or run the all-charts install-test after the merge. The **resource-quantity render test**, the **selector-match render test** and the **HTTPRoute render test** are the three exceptions to that rule: all of them run over every chart on every PR — see below. A fourth job, the **bump-level test**, ignores the matrix for a different reason: it tests no chart at all, but the `postUpgradeTasks` hook that computes their versions.
- `ci/helm-dependencies.sh <chart-dir>` — not a workflow but the step they all share: fetches a chart's declared dependencies into `charts/`, and returns silently for the charts that declare none (all but three: `ytdlp-web-player`, `stalwart`, `paperless-ngx`). It runs in **eight** places, because every helm command this repo builds on exits 1 while a declared dependency is missing: before lint and package in `validate-charts.yml`, before package in the shared publish block (which is why that block names this script as a prerequisite of its callers — see above), and inside `ci/run-install-test.sh`, `ci/run-quantity-render-test.sh`, `ci/run-selector-match-test.sh`, `ci/run-httproute-match-test.sh`, `ci/run-yaml-scalar-render-test.sh` and `ci/run-trivy-config.sh`. See "Chart dependencies" in the style guidelines for what that costs.
- `validate-charts.yml` — PR pipeline: detects which charts a PR touches and, for exactly those, runs `helm lint --strict`, `helm package`, and a chart-version bump check (`template-chart` is exempt from the version check). PRs without chart changes skip the build matrix. After the build stage, an **install-test** job (the same changed-charts matrix, minus the charts listed in `ci/excluded-charts.txt`) installs each changed chart into an ephemeral [k3d](https://k3d.io) cluster on the runner and requires every workload to become Ready — see below. Both matrices follow the changed chart directories, so a PR that only changes the harness under `ci/` exercises nothing by itself; the Trivy scan has a fixed-sample fallback for that case, the install-test has none — bump the affected chart's patch version to pull it into the matrix, or run the all-charts install-test after the merge. The **resource-quantity render test**, the **selector-match render test**, the **HTTPRoute render test** and the **YAML-scalar render test** are the four exceptions to that rule: all of them run over every chart on every PR — see below. A fourth job, the **bump-level test**, ignores the matrix for a different reason: it tests no chart at all, but the `postUpgradeTasks` hook that computes their versions.
- `install-test.yml` — reusable workflow (`workflow_call`) holding the install-test **job** itself: the runner-label expression, the pinned `kubectl` with its skew check, `envsubst`, the containerised-job detection, the per-leg k3d cluster and the `freesailarr` VPN credentials. Its one input is a JSON array of chart names; its callers decide *which* charts are tested and whether the job runs at all, never *how*. Two callers: `validate-charts.yml` and `install-test-all-charts.yml`. **It is not copied for the second caller** — a second copy is a second chance to test differently, unnoticed, the same argument that gives `ci/excluded-charts.txt` exactly one parser.
**It stays in this repository, and that is a decision, not an oversight** (Gitea #66, decision 4). It is reusable in form but not in substance: it is welded to the `ci/` fixtures, to the `ci/excluded-charts.txt` parser and to this repository's chart matrix. A building block that only one repository can use is not shared, it is merely moved somewhere else — and moving it would buy nothing while putting a tag between us and a file we change often. `publish-helm-chart.yml` was the opposite case: it takes a chart name and a registry owner and knows nothing about this repository, which is exactly why it could go. This note exists so the question is not asked again in half a year.
@@ -156,10 +156,11 @@ Neither is a render error, neither fails the install-test, and neither shows up
Three things to know before touching it:
- **Helm is the YAML parser, through the one-template chart `ci/yaml2json`.** The runners have neither `pyyaml` nor `yq` (measured; the same constraint `ci/run-trivy-config.sh` and `ci/run-quantity-render-test.sh` work under), and a YAML reader written in `awk` was the obvious alternative and the wrong one: its failure mode is a parser that quietly reads nothing, finds no selector, reports no mismatch and leaves the test green for a chart it never looked at. So the rendered manifest stream is handed to that chart as a **values file** and comes back as JSON, which `python3` reads from the standard library. The one textual step is the document split on a line that is exactly `---` in column one, and getting *that* wrong does not produce a false pass — the fragment stops being valid YAML and `helm` exits 1.
- **What is deliberately not checked, named so the gap is not mistaken for coverage.** `podAffinity`/`podAntiAffinity` are out of scope: their semantics are relative to pods that exist, so a term matching nothing is a legitimate configuration rather than a mistake. And a selector expressed **only** through `matchExpressions` is **reported as "not checked", never counted as passing** — evaluating set-based requirements means reimplementing label-selector semantics, which is a second parser by another name.
- **Two anti-vacuum guards, because this test can be green over nothing.** Both objects are rendered only above a replica threshold, and most charts' CI fixtures sit below it. `ci/selector-match/<chart>.yaml` is a render-only overlay applied **last**, whose only job is to push a chart over that threshold — and a chart that *has* one and still renders no selector **fails**, since that overlay existing is the whole claim that something was checked. On top of that the run fails if **no selector at all** was checked anywhere. Today `stalwart` is exercised through its overlay (two replicas in both node groups plus an `externalServices.valkey.host`, without which the chart's own "cluster with no in-memory store" abort fires) and `keycloak` through its ordinary install-test fixture, which already runs two replicas.
- **What is deliberately not checked, named so the gap is not mistaken for coverage.** *Preferred* `podAffinity` and `podAntiAffinity` of either strength are out of scope: their semantics are relative to pods that exist, so a term matching nothing is a legitimate configuration rather than a mistake — a preference that cannot be met is dropped, and an anti-affinity over zero pods is satisfied. And a selector expressed **only** through `matchExpressions` is **reported as "not checked", never counted as passing** — evaluating set-based requirements means reimplementing label-selector semantics, which is a second parser by another name.
- **`requiredDuringSchedulingIgnoredDuringExecution` `podAffinity` *is* checked** (Gitea #101), and it is the one affinity case that is not a preference: a required term whose selector matches no pod in the release can never be satisfied, so the pod stays `Pending` for good. It is held against **all** pod templates of the render rather than its own, because selecting a *different* workload's pod is the entire purpose of such a term. First and so far only user: `paperless-ngx`' consume CronJob, pinned onto the node of its StatefulSet pod so Longhorn will mount the shared RWX consume volume — where the silent failure would be a mover that stops moving, one `Pending` pod per schedule, with both templates still rendering perfectly.
- **Two anti-vacuum guards, because this test can be green over nothing.** Both objects are rendered only above a replica threshold, and most charts' CI fixtures sit below it. `ci/selector-match/<chart>.yaml` is a render-only overlay applied **last**, whose only job is to push a chart over that threshold — and a chart that *has* one and still renders no selector **fails**, since that overlay existing is the whole claim that something was checked. On top of that the run fails if **no selector at all** was checked anywhere. Today `stalwart` is exercised through its overlay (two replicas in both node groups plus an `externalServices.valkey.host`, without which the chart's own "cluster with no in-memory store" abort fires), `keycloak` through its ordinary install-test fixture, which already runs two replicas, and `paperless-ngx` through `ci/selector-match/paperless-ngx.yaml`, whose only job is to switch `smbConnection` on so the consume CronJob — and with it the required `podAffinity` — renders at all.
Measured: 30 charts, **5 selectors verified in ~14 s**, no cluster. The fail path was exercised rather than assumed — renaming `node-group` to `nodegroup` in a hand-written copy of the PDB selector produced two `FAIL` lines naming both the selector and the labels actually present, and a non-zero exit; the same was done for the spread constraint's selector.
Measured: 30 charts, **6 selectors verified**, no cluster. The fail path was exercised rather than assumed — renaming `node-group` to `nodegroup` in a hand-written copy of the PDB selector produced two `FAIL` lines naming both the selector and the labels actually present, and a non-zero exit; the same was done for the spread constraint's selector. For the `podAffinity` check the same was done by renaming the StatefulSet's `app` label in a throwaway copy of `paperless-ngx` while leaving the CronJob's term alone: one `FAIL` naming the selector, the three pod templates it missed and the labels actually present, and a non-zero exit.
### HTTPRoute render test (all charts, every PR)
@@ -199,6 +200,26 @@ What the cases cover: each update type, the `freesailarr` shape (an app minor wh
**What it cannot assert, and does not pretend to:** that Renovate passes the update type at all. The hook runs only when the command is allow-listed on the shared instance, which is outside this repository — which is why a *missing* second argument is itself a case here, and why the change that introduced it carries a dry run instead of a claim (see "The version-bump hook").
### YAML-scalar render test (all charts, every PR)
`ci/run-yaml-scalar-render-test.sh [chart …]` (no argument = all charts) renders every chart with the same fixtures the install-test uses and checks that **every scalar Kubernetes types as a string really arrived as one**. Introduced with Gitea #11, where a value rendered without `| quote` had been shipping in a chart for months; #112 turned out to be the same defect in the same chart's `env` block. Three assertions, and all three are properties the API itself requires — there is no chart for which the opposite is a legitimate configuration, which is why this test needs no per-chart expectations:
| Assertion | What a bare render does | Who notices today |
|---|---|---|
| `CronJob.spec.schedule` is a non-empty string | a schedule beginning with `*` (the commonest cron notation there is) or with `@` **aborts the render** — YAML reads the leading character as an alias or a reserved indicator | the operator, eventually: the error names the **template** and their own value appears nowhere in it, and **quoting it in the values file does not help**, because Helm passes the string's content through |
| every `env[].value` of every container and initContainer is a string | `"true"` and `"0755"` are legal plain scalars that are **not strings**: the chart renders perfectly and the API server rejects the object with `cannot unmarshal bool into Go struct field EnvVar.value of type string` | nobody until apply time — which in this repo means the install-test or a cluster, never the render |
| every `metadata.labels` and `metadata.annotations` value is a string | both are typed `map[string]string`, so a unit-less `100` in an upload-size annotation is rejected whole | nobody in the render |
**Why this job exists next to the render tests that already run:** the object that carried the original defect is rendered by **no fixture at all**. `paperless-ngx`' consume CronJob sits behind `smbConnection.enabled`, which is `false` in the chart defaults and `false` in its install-test fixture — deliberately, since the CI cluster has no SMB CSI driver. So nothing in this repository ever produced that object: not lint, not the other render tests, not the install-test. A test that renders only what the fixtures already render would have been just as blind, which is the whole argument for the overlays below.
Three things to know before touching it:
- **Helm is the YAML parser**, through `ci/yaml2json`, for the same reason as the two tests above — the runners have neither `pyyaml` nor `yq`, and a parser written in `awk` fails by reading nothing and reporting nothing.
- **Two anti-vacuum guards.** `ci/yaml-scalar/<chart>.yaml` is a render-only overlay applied **last**, whose job is to switch such objects on *and to feed them the hostile values* — a schedule beginning with a star, an `env` value that is a bare `true`. A chart that *has* one and still renders none of the three kinds **fails**, and the run fails if **nothing at all** was asserted anywhere.
- **What is deliberately not checked**, named so the gaps are not mistaken for coverage. A value rendered into a scalar that is **hand-quoted in the template** (`"{{ .Values.x }}"`) is a string in the output by construction, so this test is blind to it — what it can still do is break the YAML when the value itself contains a quote or a backslash, and only `| quote` fixes that. Everything that is **not** a label, an annotation, an `env` value or a schedule: a bare `storage:` or `className:` is just as unquoted, but its value space is DNS labels and quantities, where no legal value is YAML-ambiguous. And **whether a cron schedule is a *valid* schedule** — that is the controller's judgement, and narrowing the schema to a regex was considered and rejected, because the defect was never a wrong schedule but a correct one rendered bare.
Measured locally: **30 charts, 408 scalars verified, 0 failed**, no cluster. The fail path was exercised rather than assumed, in all three of its shapes, against a throwaway copy of the chart as it stands on `main`: the unquoted schedule aborted the render with `yaml: line 6: did not find expected alphabetic or numeric character` and the test reported it; with only the schedule quoted, the star-valued `env` entry aborted the render on the StatefulSet instead; and with only the bool-looking value left — a render that **succeeds** — the type assertion produced a `FAIL` naming the variable, the container and the exact API-server message. **Seven charts still render their `env` block bare** (`matrix-synapse`, `minecraft-router`, `minecraft-server`, `rallly`, `satisfactory`, `teamspeak-server`, and `paperless-ngx` until #11); the test is green for them only because their fixtures set no hostile `env` value, and the overlay is what makes the difference between covered and merely green.
### Install-test (PR pipeline stage 2)
For every chart a PR changes, `ci/run-install-test.sh <chart>` runs against a throwaway k3d cluster: it applies the stub `OnePasswordItem` CRD plus the chart's fixtures, `helm install`s the chart with its CI values, and waits for all Deployments/StatefulSets to roll out (per-chart timeout). Since probes and security hardening are active on all workloads, "Ready" means the app actually answers. On failure the job dumps pod descriptions, events and container logs.
@@ -518,6 +539,7 @@ These guidelines are **binding for every current and future chart** in this repo
- Every workload container offers an additive `env` value block, appended to the env entries the chart always sets.
- Entries are rendered through `tpl` (so values may contain Helm templating) and support the `value`, `secretKeyRef` and `configMapKeyRef` forms (pattern: `template-chart/templates/xxx.deployment.yaml`).
- **`| quote` on the rendered `value`, and on the `name`/`key` of the ref forms, is part of the pattern and not cosmetic.** An env value is typed `string` by Kubernetes, so a value of `"true"` or `"0755"` rendered bare becomes a YAML bool or integer and the API server rejects the whole manifest with `cannot unmarshal bool into Go struct field EnvVar.value of type string` — after a perfectly green render. A value beginning with `*` is worse in the other direction: it aborts the render with a YAML error naming the template, while the operator's own value appears nowhere in the message, and quoting it in the values file does not help. **The rule is held by `ci/run-yaml-scalar-render-test.sh` on every PR** (Gitea #11/#112), which is also how the count below is known rather than assumed: `template-chart` carries the rule in a comment claiming "every chart in this repo quotes here", and **six still do not** — `matrix-synapse`, `minecraft-router`, `minecraft-server`, `rallly`, `satisfactory`, `teamspeak-server`. They are green only because no fixture of theirs sets a hostile value; bringing a chart to the quoted form is a **major** bump for it, since it changes rendered output for values that currently work.
- **Additive means additive: an entry repeating a name the chart already sets does not replace it, and what the pair does depends on when it arrives.** At install both entries reach the pod and the later one — the user's — wins. On an *existing* release the same entry is lost: Kubernetes merges `env` by name, the pair collapses to the chart's own value, and `helm upgrade` still reports success. Measured on `urlaubsverwaltung` (helm 3.19.0, Kubernetes 1.35): `helm get manifest` shows the variable twice while the live Deployment shows it once, carrying the chart's value. The failure mode is silence, so a chart whose `NOTES.txt` tells operators to override one of its own variables has to say that the override only takes at install time — and that a green upgrade proves nothing, the value belongs read back off the Deployment.
### 11. Scaling