diff --git a/CI-CD.md b/CI-CD.md index 329983a..fd28193 100644 --- a/CI-CD.md +++ b/CI-CD.md @@ -74,7 +74,7 @@ Three things were listed here as only that run could prove, and they were the wh ## The PR pipeline — `validate-charts.yml` -Seven jobs. Everything except the three render tests and the bump-level test follows the **changed** chart directories. +Eight jobs. Everything except the four render tests and the bump-level test follows the **changed** chart directories. ### 1. `detect-changed-charts` @@ -125,7 +125,8 @@ Three things to know before touching it: - **Helm is the YAML parser, through the one-template chart `ci/yaml2json`** (`{{ .Values | toJson }}`). The runners have neither pyyaml nor `yq` — 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. The rendered manifest stream is therefore handed to that chart as a **values file** and comes back as JSON for `python3`. 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. `ci/yaml2json` lives under `ci/`, so it has no publish workflow and no version-bump obligation — the chart-discovery loops look one level below the repo root and `ci` has no `Chart.yaml`. - **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. A selector expressed **only** through `matchExpressions` is **reported as "not checked" and never counted as passing** — evaluating set-based requirements means reimplementing label-selector semantics, i.e. a second parser by another name. -- **Two anti-vacuum guards, because this test can be green over nothing.** Both objects render only above a replica threshold and most charts' CI fixtures sit below it. `ci/selector-match/.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**, because that overlay existing is the whole claim that something was checked. On top of that, a run in which **no selector at all** was checked fails. 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. +- **Required `podAffinity` is covered too, since Gitea #101.** *Preferred* affinity and anti-affinity of either strength stay out of scope — their semantics are relative to pods that exist, so a term matching nothing is a legitimate configuration — but a `requiredDuringSchedulingIgnoredDuringExecution` `podAffinity` whose selector matches no pod in the release can never be satisfied, and 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 whole point of such a term. First and so far only user: [paperless-ngx](paperless-ngx)' consume CronJob, pinned onto its StatefulSet pod's node; `ci/selector-match/paperless-ngx.yaml` is the overlay that makes it render at all. +- **Two anti-vacuum guards, because this test can be green over nothing.** Both objects render only above a replica threshold and most charts' CI fixtures sit below it. `ci/selector-match/.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**, because that overlay existing is the whole claim that something was checked. On top of that, a run in which **no selector at all** was checked fails. 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; `paperless-ngx` joins them through an overlay whose only job is to switch `smbConnection` on. 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, plus a non-zero exit; the same was done for the spread constraint's selector, and the `matchExpressions`-only and overlay-without-selector paths were probed too. @@ -191,7 +192,26 @@ Its failure mode is the silent kind: **a wrong level is a valid version string.* - **What is covered:** each update type; the `freesailarr` shape (an app minor while `appVersion` stands still); all seven non-semver app versions plus the majors that must *not* fall back; the maximum rule in both directions, including a stale branch version that must **not** be kept; a missing, an empty and an unknown second argument; and the inputs the script must leave alone (a chart new on the branch, `template-chart`, a `.github/` or `ci/` argument, no argument at all). The full `version:` line is asserted every time, so the `+up` suffix and its dash-to-dot normalization are covered throughout. - **What it cannot assert, named so the gap is not mistaken for coverage:** that Renovate passes the update type at all. The hook runs only where the command is allow-listed, which is on the shared instance and outside this repository — which is exactly why a *missing* second argument is one of the cases. See [Renovate → the version-bump hook](Renovate#the-version-bump-hook). -### 7. `trivy-config` (reporting only) +### 7. `yaml-scalar` (every chart, every PR) + +The fifth job that ignores the changed-chart matrix and the fourth that renders every chart (Gitea #11). `ci/run-yaml-scalar-render-test.sh` renders **every** chart with the fixtures the install-test uses and checks that **every scalar Kubernetes types as a string really arrived as one**. Three assertions, each a property the API itself requires — so there is no chart for which the opposite is a legitimate configuration, and none of them needs a per-chart expectation: + +| Assertion | A bare render does this | Who notices today | +|---|---|---| +| `CronJob.spec.schedule` is a non-empty string | a schedule beginning with `*` or `@` **aborts the render** — YAML reads the leading character as an alias or a reserved indicator | the operator, the hard way: the error names the **template**, 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 here 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 | + +The defect has a **loud and a silent half**, like `quantity-render`'s, and the same consequence: checking the exit code alone covers one of them. The second and third assertions read the rendered output back and type-check it. + +- **Why this job and not a fixture.** The object that carried the original defect was rendered by **nothing**. `paperless-ngx`' consume CronJob sits behind `smbConnection.enabled`, `false` in the chart defaults *and* in its install-test fixture, deliberately — the CI cluster has no SMB CSI driver. So neither lint, nor the other render tests, nor the install-test ever produced that template's output, which is how an unquoted schedule shipped for months. A test over what the fixtures already render would have been just as blind. +- **Two anti-vacuum guards, and here the overlay *is* the test.** `ci/yaml-scalar/.yaml` is a render-only overlay applied **last**, whose job is to switch such objects on **and to feed them hostile values** — a star-leading schedule, an `env` value that is a bare `true`. A chart that *has* one and still renders none of the three kinds **fails**, and a run that asserted nothing at all fails. Today only `paperless-ngx` has one. +- **Helm is the YAML parser**, through `ci/yaml2json`, for exactly the same reason as in `selector-match` — no pyyaml, no `yq`, and a hand-written parser fails by reading nothing and reporting nothing. +- **What is deliberately not checked.** 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 equally 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 — the controller's judgement; narrowing the schema to a regex was considered and rejected, since the defect was never a wrong schedule but a correct one rendered bare. + +Measured: **30 charts, 408 scalars, 0 failed**, no cluster. The fail path was exercised in all three shapes against a throwaway copy of the chart as it stood on `main` — the bare schedule aborting the render, the star-valued `env` entry aborting it on the StatefulSet once the schedule was quoted, and finally the bool-looking value on a render that **succeeds**, where the type assertion named the variable, the container and the API server's own message. **Six charts still render their `env` block bare** (`matrix-synapse`, `minecraft-router`, `minecraft-server`, `rallly`, `satisfactory`, `teamspeak-server`): they pass only because no fixture of theirs sets a hostile value, and bringing one over is a major bump for that chart. + +### 8. `trivy-config` (reporting only) Renders each chart in the scan matrix and lists what `trivy config` finds. **It never fails the job** (`TRIVY_CONFIG_MODE: report`). Deliberately not a gate yet: the repo has several documented deviations, and a check that goes red on those every time is one people learn to scroll past. Switching the mode to `gate` is the intended step once the seccomp gap is closed. @@ -199,7 +219,7 @@ Trivy is installed from its release tarball, not via `aquasecurity/setup-trivy` Exceptions live in `ci/trivy/trivyignore.yaml`, each with a mandatory `statement` — the script refuses to run without one and prints every suppressed finding together with its reason. **The trusted-registry list for AVD-KSV-0125 is a separate file on purpose**, `ci/trivy/data/ksv0125.yaml`: it is *configured* rather than suppressed, so the check keeps being able to say something about an image from a genuinely foreign source. A new registry therefore means an entry there, not an ignore — `quay.io` joined it with [keycloak](keycloak), the first chart in the repo using it. Note what the check does *not* forgive: an unqualified Docker Hub image puts the organisation in the registry position and is still reported, which is why every chart writes its image fully qualified. -### 8. `install-test` (per changed, non-excluded chart) +### 9. `install-test` (per changed, non-excluded chart) `ci/run-install-test.sh ` against a throwaway [k3d](https://k3d.io) cluster: apply the stub `OnePasswordItem` CRD plus the chart's fixtures, `helm install` with the CI values, and wait for every Deployment and StatefulSet to roll out. Since probes and hardening are active, "Ready" means the app actually answers. On failure the job dumps pod descriptions, events and container logs. @@ -218,6 +238,8 @@ Fixtures live at the **repo root** under `ci/`, deliberately *not* inside the ch | `ci/common/` | applied for every chart (client-side): the schema-less stub CRD for `OnePasswordItem` | | `ci/gateway-api-crds.sh` | the **real** Gateway-API CRDs, applied `--server-side` and only for a chart whose render actually contains an `HTTPRoute` — see below | | `ci/httproute/.yaml` | render-only overlay for the `httproute-match` job, never used at install time | +| `ci/yaml-scalar/.yaml` | render-only overlay for the `yaml-scalar` job: switches objects on that no fixture renders **and** feeds them YAML-hostile values, never used at install time | +| `ci/selector-match/.yaml` | render-only overlay for the `selector-match` job: pushes a chart over the threshold at which it renders a selector, never used at install time | | `ci//test-values.yaml` | values for the CI install — **required for every installable chart** | | `ci//fixtures.yaml` | dummy secrets, throwaway infrastructure (postgres/redis/dex), static hostPath PVs and storage classes, seed jobs | | `ci//fixtures.env.yaml` | fixtures needing a value that must not be committed: `${VARIABLE}` placeholders only, rendered with `envsubst` and piped straight into `kubectl`, never printed | @@ -389,13 +411,13 @@ All three are recorded on [`Gemessenes-Verhalten`](https://git.jk-dev.de/workflo Not a workflow but the prerequisite of every helm command in this repo: it fetches a chart's declared dependencies into `charts/`, and returns silently for the charts that declare none (all but one). -It runs in **seven** places, because every helm command exits 1 while a declared dependency is missing: before lint and package in `validate-charts.yml`, before package in the shared publish block, 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`. The second of those is now across a repository boundary: the block calls this script from the calling repository's checkout, which makes the script part of the contract — renaming or deleting it breaks publishing from a place that no longer reads our diffs. Three of those call sites are the render tests that cover **every** chart on **every** PR — so a dependency-carrying chart can turn a third-party Helm repository's bad day into a red pipeline on an unrelated PR, now from either of them. Measured while writing the selector-match test: the registry answered `503` for several minutes and both tests went red on `stalwart`, `ytdlp-web-player` and `paperless-ngx` with nothing in the diff to blame. +It runs in **eight** places, because every helm command exits 1 while a declared dependency is missing: before lint and package in `validate-charts.yml`, before package in the shared publish block, 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`. The second of those is now across a repository boundary: the block calls this script from the calling repository's checkout, which makes the script part of the contract — renaming or deleting it breaks publishing from a place that no longer reads our diffs. Four of those call sites are the render tests that cover **every** chart on **every** PR — so a dependency-carrying chart can turn a third-party Helm repository's bad day into a red pipeline on an unrelated PR, now from either of them. Measured while writing the selector-match test: the registry answered `503` for several minutes and both tests went red on `stalwart`, `ytdlp-web-player` and `paperless-ngx` with nothing in the diff to blame. It runs `helm dependency update`, not `build`: `Chart.lock` is not committed, because Renovate rewrites `Chart.yaml` and cannot regenerate the lock, so a committed one would arrive stale. ## A PR that touches only `ci/` -Both matrices follow the changed **chart directories**, so a PR that only changes the harness 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](#the-all-charts-install-test-on-demand) once the change is on `main`. The quantity render test, the selector-match test and the HTTPRoute render test are the three exceptions to the rule: all of them always run over everything. The [`bump-level` job](#6-bump-level-the-version-bump-hook-every-pr) is a fourth exception of a different kind — a PR that changes only `ci/renovate-bump-chart.sh` *is* exercised by it, since that job tests the harness rather than the charts. +Both matrices follow the changed **chart directories**, so a PR that only changes the harness 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](#the-all-charts-install-test-on-demand) once the change is on `main`. The quantity render test, the selector-match test, the HTTPRoute render test and the YAML-scalar render test are the four exceptions to the rule: all of them always run over everything. The [`bump-level` job](#6-bump-level-the-version-bump-hook-every-pr) is a fourth exception of a different kind — a PR that changes only `ci/renovate-bump-chart.sh` *is* exercised by it, since that job tests the harness rather than the charts. ## What a Gitea runner does differently diff --git a/Chart-Style-Guide.md b/Chart-Style-Guide.md index cfeef59..69296af 100644 --- a/Chart-Style-Guide.md +++ b/Chart-Style-Guide.md @@ -196,6 +196,10 @@ If a Deployment-based chart ever does set it, mind the precedence: the annotatio Every workload container offers an additive `env` value block, appended to the entries the chart always sets. Entries are rendered through `tpl` (so values may contain Helm templating) and support the `value`, `secretKeyRef` and `configMapKeyRef` forms. +**`| quote` on the rendered `value` — and on the `name` and `key` of the ref forms — is part of the pattern, not cosmetics.** Kubernetes types `env[].value` as a string, 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 `*` fails in the other direction and aborts the render, with an error naming the template while the operator's value appears nowhere in it; quoting it in the values file does not help, because Helm passes the string's content through. `template-chart/templates/xxx.deployment.yaml` carries the rule, and **`ci/run-yaml-scalar-render-test.sh` holds it on every PR** — see [the `yaml-scalar` job](CI-CD#7-yaml-scalar-every-chart-every-pr) and the [lesson](Lessons-Learned) it was bought with. + +**And the count, because a convention with unnamed exceptions decays into a claim.** 24 of 30 charts follow it; **six do not** — `matrix-synapse`, `minecraft-router`, `minecraft-server`, `rallly`, `satisfactory`, `teamspeak-server` — and they are green only because no fixture of theirs sets a hostile value. Bringing one over changes rendered output for values that currently work, so it is a **major** bump for that chart. `template-chart`'s own comment claimed "every chart in this repo quotes here" while seven did not; that is what the test and this count replace. + **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. diff --git a/Lessons-Learned.md b/Lessons-Learned.md index 218f48e..2cfaa34 100644 --- a/Lessons-Learned.md +++ b/Lessons-Learned.md @@ -786,3 +786,52 @@ Two Ingress objects whose `tls` entries name the same secret therefore drive the Three transferable pieces. **A justification carried in a rule is load-bearing** — it is what someone reaches for when the rule meets a case its author did not have, so a reason that is merely true of the first instance will mislead at the second. **Parallel rounds are a conformance test nobody wrote**: this surfaced only because two agents implemented the same shape from the same sentence at the same time and disagreed; sequentially, the second would simply have followed the first and the broken reason would have survived. And **where a rule is corrected rather than the code, say which way round it went**, because the next reader will otherwise re-derive the original objection — which is exactly what happened here, correctly, and cost a round. **And a second occurrence of the entry above it, which is what turns one chart's story into a rule.** `stalwart`'s per-entry `tlsSecretName` check had exactly the defect the realm-list round describes: with **both** routing modes on and a `tlsSecretName` missing, the render reported *"in Ingress mode every entry needs one"* — true, and the wrong diagnosis, because that configuration is not coherent to begin with. Found by rendering the conflicting combination rather than by reading the template, and fixed the same way: the Ingress-mode block is gated on `ingress.enabled` **and not** `httpRoute.enabled`, so in that state it validates nothing and the only `fail` left is the structural one. Two charts met it in the same round, from two agents, neither having seen the other's code — so expect it in the next chart that gains a per-entry check beside a mode switch, and put the conflicting combination in the test matrix for the new value. + +## The value the operator quoted correctly, and the manifest where it stood bare + +A chart rendered a cron schedule straight into its CronJob: + +```yaml +schedule: {{ .Values.smbConnection.cronJob }} +``` + +The default, `"0 0,12 * * *"`, rendered cleanly — because it begins with a digit. The commonest notation of all, `*/15 * * * *`, did not: + +``` +Error: YAML parse error on paperless-ngx/templates/paperless-consume.cronjob.yaml: +error converting YAML to JSON: yaml: line 6: did not find expected alphabetic or numeric character +``` + +**The part that makes this a genus and not a typo: quoting the value does not help.** Helm hands the template the string's *content*, so the quotes an operator writes in their values file are gone by the time the scalar is emitted, and the star stands bare in the manifest again. The operator does everything right and gets an error that names **the template** — a file they did not write, on a line their value does not appear on. `@daily` fails identically (`@` is a reserved YAML indicator), and so would any value beginning with `&`, `{`, `[`, `%` or `!`. + +The fix is one word, `| quote`. Four things generalise, and the first two are why this is worth a page. + +### The quiet half is the dangerous one + +The same missing `| quote` has a second failure mode that does **not** abort anything. A value whose content is a legal plain scalar but not a *string* renders perfectly and lands in the manifest as the wrong type: + +| Value in `values.yaml` | In the manifest | What happens | +|---|---|---| +| `"*/15 * * * *"` | `*/15 * * * *` | render aborts — **loud**, and misdirected | +| `"true"` | `true` | a YAML **bool**; the API server rejects the object at apply time | +| `"0755"` | `0755` | an **integer**; same | + +Both of the latter are schema-valid strings on the way in. `helm template` exits 0, `helm lint` is happy, the diff looks right, and the failure arrives at `kubectl apply` with `cannot unmarshal bool into Go struct field EnvVar.value of type string`. **So "does the render succeed" is not the test** — the output has to be read back and type-checked. Kubernetes types `env[].value`, every label value and every annotation value as a string, so there is no chart for which the opposite is legitimate: that is what makes it checkable without per-chart expectations. + +### A defect lives where no fixture renders + +The schedule survived in a shipped chart for months for one reason: **the object carrying it was rendered by nothing.** The CronJob sits behind `smbConnection.enabled`, which is `false` in the chart defaults *and* `false` in the install-test fixture — deliberately, since the CI cluster has no SMB CSI driver. Neither lint, nor the render tests, nor the install-test ever produced that template's output. + +A test over what the fixtures already render would therefore have been just as blind, and the test that was added is the **overlay** as much as the script: `ci/yaml-scalar/.yaml` switches such objects on *and* feeds them hostile values, and the run fails when an overlay stops reaching its object or when nothing was asserted at all. The transferable question for any new check: **is there a values combination under which this template produces no output?** If yes, that combination is where the next defect will sit, and an anti-vacuum guard is the only thing keeping the green tick honest. + +### A convention that 23 charts follow and 7 do not is not yet a convention + +`template-chart` carried the rule all along, in a comment ending: *"Every chart in this repo quotes here."* It was false when it was written — **seven** charts rendered their `env` block bare. The comment was not a lie, it was a **generalisation from the file it sits in**, and nothing ever compared it with the other 29 charts. + +Two consequences. **Write the exceptions, or write a test** — a count in prose decays silently, and this one had decayed into the opposite of the truth. And **when a sweep conforms one chart, say which ones remain**: the README now names the six charts that still render it bare, and that bringing each one over is a major bump for that chart, instead of repeating the claim. + +### The issue can be wrong about the defect and right about the pain + +The companion issue reported that the chart had *no* way to set an arbitrary `PAPERLESS_*` variable. It had one — the standard additive `env` block, in the schema and rendering after the fixed entries. The report had read `helm show values` of a **deployed, older** release and listed only the *top-level* keys; the block sits one level in. + +The reported pain was real anyway, because the block could not be trusted with an arbitrary value. **So the right move was neither "works as designed" nor building what was asked for** (an `extraEnv`, which would have been a second name beside `env`) — it was fixing what was actually broken about the existing one and naming the deviation from the issue's title in the pull request. Check a "the chart cannot do X" report against the chart on `main` before designing X; and where the finding differs, say so instead of quietly delivering something else. diff --git a/paperless-ngx.md b/paperless-ngx.md index ef7859c..d8e8647 100644 --- a/paperless-ngx.md +++ b/paperless-ngx.md @@ -104,7 +104,8 @@ kubectl -n exec -it -paperless-ngx-sfs-0 -- \ | `secrets.redisSecretRef` | string | `null` | **Required.** `op://` path of the Redis item. | | `secrets.paperlessSecretRef` | string | `null` | **Required.** `op://` path of the app item. | | `secrets.smbConnectionSecretRef` | string | `null` | **Required while `smbConnection.enabled` is true**, ignored otherwise. | -| `secrets.oidcSecretRef` | string | `null` | Accepted by the schema and **read by no template** — see [`secrets.oidcSecretRef` renders nothing](#secretsoidcsecretref-renders-nothing). | + +`secrets.oidcSecretRef` **was removed in 13.0.0** and is no longer accepted — see [`secrets.oidcSecretRef` is gone](#secretsoidcsecretref-is-gone). ### `ingress` @@ -158,7 +159,7 @@ What the chart renders in this mode is **only** the `HTTPRoute`: one rule, `Path | `paperless.livenessProbe` | object | `GET /:8000`, period 30s, timeout 5s, 3 failures | | | `paperless.readinessProbe` | object | `GET /:8000`, period 10s, timeout 5s, 3 failures | | | `paperless.startupProbe` | object | `GET /:8000`, period 10s, 60 failures (= 600s) | Gates liveness and readiness. The budget covers migrations and, on the first 3.x start, the Whoosh → Tantivy index rebuild. | -| `paperless.env` | list | `[]` | Extra environment variables appended to the chart's own. Rendered through `tpl`; `value`, `secretKeyRef` and `configMapKeyRef` forms. **Additive, not overriding** — see [Environment variables](Chart-Style-Guide#10-environment-variables). | +| `paperless.env` | list | `[]` | Extra environment variables appended to the chart's own — paperless reads ~80 `PAPERLESS_*` variables and the chart maps a handful explicitly, so everything else goes here. Rendered through `tpl`; `value`, `secretKeyRef` and `configMapKeyRef` forms, **quoted since 13.0.0** (before that a `*`-leading value aborted the render and `"true"` arrived as a bool). **Additive, not overriding** — see [Environment variables](Chart-Style-Guide#10-environment-variables) and [Moving the log off the data volume](#moving-the-log-off-the-data-volume). | | `paperless.resources.limitFactor` | number | `3` | Memory and ephemeral-storage limits are computed as factor × request. | | `paperless.resources.requests` | object | `cpu: 0.2`, `memory: 1024Mi`, `ephemeralStorage: 200Mi` | | | `paperless.resources.limits` | object | unset | An explicit limit always wins. A CPU limit is only rendered when set here. | @@ -171,7 +172,11 @@ What the chart renders in this mode is **only** the `HTTPRoute`: one rule, `Path |---|---|---|---| | `smbConnection.enabled` | bool | `false` | Renders the SMB PV, PVC, `OnePasswordItem` and the consume CronJob. | | `smbConnection.server` | string | `null` | Share address, used as `//` and as the PV's `volumeHandle`. See [The SMB PersistentVolume outlives the release](#the-smb-persistentvolume-outlives-the-release). | -| `smbConnection.cronJob` | string | `0 0,12 * * *` | Cron schedule. **A schedule starting with `*` breaks the render** — see [A schedule may not start with `*`](#a-schedule-may-not-start-with-). | +| `smbConnection.cronJob` | string | `0 0,12 * * *` | Cron schedule. Any notation works **since 13.0.0** — quote it or not, the chart quotes it on the way out. Before that a schedule starting with `*` or `@` aborted the render; see [Any cron notation works now](#any-cron-notation-works-now). | +| `smbConnection.cron.pinToServerNode` | bool | `true` | Runs the mover on the **node of the StatefulSet pod** via a required `podAffinity`. Without it Longhorn refuses to mount the shared RWX consume volume on the StatefulSet's node. `false` restores the old placement **and its mount errors** — see [The mover runs where the server runs](#the-mover-runs-where-the-server-runs). | +| `smbConnection.cron.startingDeadlineSeconds` | int | `600` | How late a missed run may still start. Also keeps the CronJob controller away from its "too many missed start times (> 100)" stop. | +| `smbConnection.cron.activeDeadlineSeconds` | int | `3600` | Hard stop for one run, counted from the **job's** start — so it also bounds a pod that never gets scheduled. Raise it if a run legitimately moves more than an hour's worth of files. | +| `smbConnection.cron.env` | list | `[]` | Extra environment variables for the mover container, same form as `paperless.env`. Present for the convention rather than for a need: the container is a busybox running one `mv`. The schedule's **time zone is the controller's** and no `TZ` here changes it. | | `smbConnection.cron.securityContext.pod` | object | non-root 1000/1000, `fsGroup: 1000`, `fsGroupChangePolicy: Always` | The mover only shuffles files between two volumes, so it runs fully hardened as the same uid as paperless. | | `smbConnection.cron.securityContext.container` | object | `readOnlyRootFilesystem: true`, all caps dropped, no privilege escalation, non-root | | | `smbConnection.cron.resources.limitFactor` | number | `3` | | @@ -189,7 +194,7 @@ What the chart renders in this mode is **only** the `HTTPRoute`: one rule, `Path | `oidc.provider.name` | string | `null` | **Required while `oidc.enabled` is true.** Label of the login button. | | `oidc.provider.clientId` | string | `null` | **Required while `oidc.enabled` is true.** Travels as plain env, not through a Secret. | | `oidc.provider.discoveryUrl` | string | `null` | **Required while `oidc.enabled` is true.** The provider's `.well-known` endpoint. | -| `oidc.provider.tokenAuthMethod` | string | `null` | Explicit token-endpoint auth method (e.g. `client_secret_basic`). Some providers need it since the allauth update in 3.0; the symptom without it is a login dying at the callback with `invalid_client`. | +| `oidc.provider.tokenAuthMethod` | string | `null` | Explicit token-endpoint auth method. **Restricted to `none` since 13.0.0** (the schema rejects everything else). Some providers need the public client spelled out since the allauth update in 3.0; the symptom without it is a login dying at the callback with `invalid_client`. The old comment named `client_secret_basic`, which **cannot work here** — see [No client secret, and no value that pretends otherwise](#no-client-secret-and-no-value-that-pretends-otherwise). | ### `valkeySubchart` and `valkey` (subchart) @@ -357,11 +362,67 @@ kubectl -n get jobs --sort-by=.metadata.creationTimestamp kubectl -n logs job/ ``` -`scaleDown: true` suspends the CronJob. `paperless.replicas: 0` does **not** — it keeps moving PDFs off the share onto a consume volume that nothing is reading, so the files are gone from the share and not yet in paperless. +`scaleDown: true` suspends the CronJob. `paperless.replicas: 0` does **not** suspend it — but **since 13.0.0 it stops the mover all the same**, and for a different reason: the required `podAffinity` cannot be satisfied without a StatefulSet pod, so the job fails per schedule with `DeadlineExceeded` instead of running. The old hazard that stood here — PDFs moved off the share onto a consume volume nothing is reading, gone from the share and not yet in paperless — is therefore no longer reachable in that configuration, and what replaces it is a visible failing job. See [The mover runs where the server runs](#the-mover-runs-where-the-server-runs); with `pinToServerNode: false` the old behaviour, and the old hazard, are back. -### A schedule may not start with `*` +### Any cron notation works now -`smbConnection.cronJob` is rendered into the manifest **unquoted**, so a schedule that starts with `*` (`*/15 * * * *`) aborts the render with a YAML parse error — YAML reads the leading `*` as an alias, and quoting it in your values file does not help because the quotes are gone by then. Use an equivalent that starts with a digit (`5,20,35,50 * * * *`); a `*` anywhere but in first position is fine. +**Since 13.0.0.** Until then `smbConnection.cronJob` was rendered into the manifest **unquoted**, so a schedule starting with `*` — `*/15 * * * *`, the commonest notation there is — aborted the render: + +``` +Error: YAML parse error on paperless-ngx/templates/paperless-consume.cronjob.yaml: +error converting YAML to JSON: yaml: line 6: did not find expected alphabetic or numeric character +``` + +YAML read the leading `*` as an alias indicator, and **quoting it in the values file did not help**: Helm passes the string's content through, so the star stood bare in the manifest again. `@daily` failed the same way. The error named the *template* and the operator's own value appeared nowhere in it, which is the part that cost the time. + +If you are carrying a workaround schedule from that era — `5,20,35,50 * * * *` written because every-15-minutes would not render — you can now write what you meant. Fixed in Gitea #11 with `| quote`, and held by `ci/run-yaml-scalar-render-test.sh` on every PR; the overlay that feeds it a star-leading schedule is `ci/yaml-scalar/paperless-ngx.yaml`, needed because **no fixture renders this CronJob at all**. That is the whole reason the defect survived in a shipped chart. The transferable half is in [Lessons learned](Lessons-Learned). + +### The mover runs where the server runs + +**Since 13.0.0** the mover pod carries a required `podAffinity` on the StatefulSet pod's label with `topologyKey: kubernetes.io/hostname` (`smbConnection.cron.pinToServerNode`, default `true`). + +Why: the mover mounts the **consume volume**, which is RWX and comes from Longhorn, and Longhorn's CSI driver refuses to mount an RWX volume unless it finds a workload pod in **Pending**. With the mover on a different node, the kubelet of the *StatefulSet's* node retried every two minutes: + +``` +MountVolume.SetUp failed for volume "pvc-…" : rpc error: code = Aborted +desc = no Pending workload pods for volume pvc-… to be mounted: +map[Running:[…-sfs-0] Succeeded:[…-consume-cronjob-…]] +``` + +291 events in 10 hours, in both releases with SMB on. The release **without** the CronJob produced none on identically built volumes, which is what identified the mover rather than the volume. + +A static `nodeSelector` would have been the wrong tool: the StatefulSet pod is not pinned either and moves on every restart, so the selector would have been right until then and silently wrong after. `preferred` would have fallen back to another node in exactly the situation the rule exists for — the quiet version of the same bug. + +**What it costs, and the one case that is not covered.** A required term cannot be satisfied while the StatefulSet pod is absent: + +| Case | What happens | +|---|---| +| `scaleDown: true` | the CronJob is suspended outright, so no job is created | +| rolling restart | the mover waits and is scheduled once the StatefulSet pod is back; a *running* mover is never evicted (`IgnoredDuringExecution`) | +| `paperless.replicas: 0` with SMB still on | the mover **cannot be placed** and the job fails after `activeDeadlineSeconds` with `DeadlineExceeded` — one visible failure per schedule instead of a `Pending` pod nobody sees | + +`concurrencyPolicy: Forbid` and `startingDeadlineSeconds` come with it: two movers racing on the same `mv` is never wanted, and the deadline keeps the controller away from the state where more than 100 missed schedules stop it scheduling this job at all (reachable by leaving `scaleDown: true` in place for months, and reported only in the controller's own log). + +**What the pinning does not fix:** the consume volume stays RWX with a share-manager behind it. Co-locating the two consumers removes the **cross-node** mount, not the second consumer. The design that removes it is running the `mv` inside the StatefulSet pod — which cannot be done for the existing installations, because RWX cannot be taken back off an existing PVC. Gitea #101 names it as the alternative rather than the proposal. + +**`successfulJobsHistoryLimit` is deliberately untouched.** The three `Succeeded` pods in the message look like the cause and are not: Longhorn's predicate is the *existence* of a `Pending` pod, not the *absence* of finished ones. At `0` there would still be one `Running` pod and no `Pending` one, and the mount would still abort. + +**Not provable in CI:** the install-test runs in k3d without Longhorn, so the mount failure cannot be reproduced there. What CI does hold is that the affinity renders and that its selector really matches the StatefulSet's pod labels (`ci/run-selector-match-test.sh`, extended for required `podAffinity` in Gitea #101) — the fix is demonstrable at the render and placement level, not at the symptom. + +### Moving the log off the data volume + +`PAPERLESS_LOGGING_DIR` through `paperless.env` is the worked example of that block, and the reason it exists is **not** in this chart: + +```yaml +paperless: + env: + - name: PAPERLESS_LOGGING_DIR + value: /tmp/paperless-log +``` + +The data directory sits on an RWX volume that Longhorn exports over **NFS** via a share-manager, and paperless writes its log there with `concurrent_log_handler`, which takes a lock file before every write and rotation. File locks over NFS are unreliable, so the handler fails with `RuntimeError: Cannot acquire lock after 20 attempts` — measured at **45 % of the application log** across all three installations. Pointing the log at a path under the `/tmp` emptyDir the pod already mounts takes it off the share and the noise stops. No extra volume is needed, and that the log does not survive a pod restart is no loss: paperless also writes to stdout, which is what `kubectl logs` reads. + +**This does not fix the RWX data volume**, which is the actual root cause: the PVC asks for `ReadWriteOnce` and was provisioned `ReadWriteMany`, and taking RWX back means recreating the volume. That is a `jk-cluster-config` decision and is tracked there (#360); this chart's part is only being able to set the variable at all. ### The SMB PersistentVolume outlives the release @@ -397,9 +458,27 @@ The chart prefixes `http://` itself, so `tikaEndpoint` and `gotenbergEndpoint` a The ConfigMap that would carry the pre/post-consumption scripts is disabled at the top of its own template (`{{- if false }}`), no volume mounts it, and neither `PAPERLESS_PRE_CONSUME_SCRIPT` nor `PAPERLESS_POST_CONSUME_SCRIPT` is set on the container. Both values render no object and change no behaviour; `NOTES.txt` says so out loud when either list is non-empty. -### `secrets.oidcSecretRef` renders nothing +### `secrets.oidcSecretRef` is gone -No template references it: no `OnePasswordItem` is created and no Secret appears. The OIDC client id travels as plain env from `oidc.provider.clientId`, and no client secret is configured at all. Whatever the item holds has no effect on the release. +**Removed in 13.0.0** (Gitea #11), from the values *and* from the schema. A release that still sets it no longer renders: + +``` +values don't meet the specifications of the schema(s) in the following chart(s): +paperless-ngx: +- at '/secrets': additional properties 'oidcSecretRef' not allowed +``` + +**Upgrading: delete the key.** Nothing else is needed, and nothing is lost — no template ever read it, no `OnePasswordItem` was created for it, and whatever the item held never reached the pod. It was the quiet kind of defect: it looked like successful configuration. + +It is not coming back as a feature, and the reason is in paperless rather than in the chart. The entire OIDC configuration travels in **one** JSON environment variable (`SOCIALACCOUNT_PROVIDERS = json.loads(os.getenv("PAPERLESS_SOCIALACCOUNT_PROVIDERS", "{}"))`) with no `_FILE` variant, and a `secretKeyRef` cannot substitute a fragment *into* a JSON variable. The honest alternatives would be the secret inline in the pod spec, or the whole JSON typed into the vault by hand — which would make every `required()` in the `oidc` block dead wood. The operational question behind it is answered by Gitea #83: Keycloak marks a client public **per client**, so no provider constraint forces a confidential client here. + +### No client secret, and no value that pretends otherwise + +The chart registers a **public client with PKCE** and configures no client secret — it renders `OAUTH_PKCE_ENABLED` and no `secret` at all. + +`oidc.provider.tokenAuthMethod` is therefore restricted to `none` since 13.0.0. Its old comment offered `client_secret_basic` as the example, and setting that asked the provider for an authentication this release cannot perform: the login died at the callback with `invalid_client`, while the two values read together — a secret ref beside a secret-based auth method — looked like a working setup. Both halves are fixed in the same round, because they were the same mistake seen twice. + +`null` omits the key and lets allauth decide; `none` states the public client explicitly, which is what some providers need spelled out. Widening the enum for a future method that needs **no** secret would be a minor bump; allowing one that needs a secret is not on the table. Measured when the restriction landed: all three live instances run PKCE and none of them sets the value, so nothing in the cluster was affected. ### Locking yourself out with OIDC @@ -431,6 +510,8 @@ The same mechanism is the reason `httpRoute.hostnames` must not disagree with `i **The subchart is ON in CI, and the throwaway Redis Deployment is gone** — this is the **first real installation of the valkey-subchart path** anywhere in this repository. [stalwart](stalwart) added the same dependency first but covered it by a local render only, deliberately: its in-memory store is a database object, so a Valkey in that namespace would have been reachable and unused and the test would have looked as though it covered clustering. Here it is the opposite — paperless reads `PAPERLESS_REDIS` from the vault item, so pointing that url at `ci-valkey-service` makes the bundled Valkey the broker the release actually uses. What the run proves beyond the previous one: the subchart's StatefulSet becomes Ready inside a parent release (so its init container found the `requirepass ` line and its three authenticated `valkey-cli` probes answered `PONG`), its PVC path works under a parent release name, and the names really are `-valkey-`. A wrong Service name or a password mismatch between the item's two fields is a red run rather than a quietly idle Valkey. +**Two render-only overlays switch `smbConnection` on, and they are the only thing in the repository that renders the consume CronJob.** The install-test fixture keeps SMB off (no SMB CSI driver on the runner), so until Gitea #11 that object existed in the chart and in no render — which is exactly how an unquoted schedule shipped for months. `ci/yaml-scalar/paperless-ngx.yaml` renders it with a **star-leading schedule** and with hostile `env` values for `ci/run-yaml-scalar-render-test.sh`; `ci/selector-match/paperless-ngx.yaml` renders it plainly so `ci/run-selector-match-test.sh` can hold the mover's required `podAffinity` against the StatefulSet's pod labels. They are kept apart on purpose: folding them would make each test's failure depend on the other's fixture. Neither reaches the symptom of Gitea #101 — k3d has no Longhorn — so the placement fix is covered at the render level and the mount failure is not reproducible in CI. + **The install-test stays in Ingress mode, and here that was the easy call.** The install runs exactly one configuration per chart, and this chart's fixture is the only installation of the valkey-subchart path anywhere — switching it into Gateway mode would have traded that coverage, plus the Ingress, for a CRD schema [cyberchef](cyberchef) already exercises against the **real** Gateway-API CRDs. Instead `ci/httproute/paperless-ngx.yaml` is a **render-only overlay** that switches this chart into Gateway mode for `ci/run-httproute-match-test.sh`, which asserts that every `backendRefs` entry names a Service **and a port** the same render creates and that `parentRefs` is not empty. A chart that has an overlay and renders no route **fails** that test. Neither half proves that traffic flows: no Gateway controller runs in CI. Both fixture Secrets carry **all three fields**, because in a real cluster both `OnePasswordItem`s name the same item and the operator materializes every field of it. Trimming each to what its consumer reads would have been a fixture tidier than reality, hiding the one thing worth knowing. The Valkey PVC uses `local-path` (k3d's provisioner): the chart's `longhorn-encrypted-ns` does not exist on the runner, and the pre-bound hostPath PVs of the `longhorn` stand-in would be worse, since the kubelet applies no `fsGroup` to a hostPath volume and the ownership path this test exists for would silently stop being exercised.