diff --git a/.github/workflows/validate-charts.yml b/.github/workflows/validate-charts.yml index 32d8f8b..4979c78 100644 --- a/.github/workflows/validate-charts.yml +++ b/.github/workflows/validate-charts.yml @@ -263,6 +263,46 @@ jobs: - name: "Check every bump level of the version-bump hook" run: ci/run-bump-level-test.sh + yaml-scalar: + name: "YAML scalars" + runs-on: ubuntu-latest + # The fifth job that ignores the changed-chart matrix, and the fourth that + # runs over *every* chart (Gitea #11), for the same reason as the three + # render tests above: the unquoted + # form arrives by copy into a chart whose own diff nobody reads for it, and + # it is in seven charts right now. + # + # What it asserts: every CronJob schedule, every env[].value and every label + # and annotation value a chart renders is a STRING, which is what Kubernetes + # types all three as. A value rendered without | quote fails in one of two + # ways and the quiet one is the dangerous one: the render aborts with a YAML + # error naming the TEMPLATE while the operator's value appears nowhere in + # it, or - when the content is a legal plain scalar that simply is not a + # string - the render succeeds and the API server rejects the object at + # apply time with "cannot unmarshal bool into Go struct field + # EnvVar.value of type string". + # + # WHY THIS JOB EXISTS AT ALL, rather than the install-test covering it: the + # object that carried the defect is rendered by no fixture. paperless-ngx' + # consume CronJob sits behind smbConnection.enabled, false in the defaults + # and in every committed fixture, so nothing in this repository ever + # produced it - which is how an unquoted schedule shipped for months. The + # overlays under ci/yaml-scalar/ switch such objects on and feed them + # hostile values, and the script fails when an overlay stops reaching its + # object. + # + # No cluster and no Trivy; the whole repository in well under a minute, + # depends on no other job. The YAML is parsed by Helm through ci/yaml2json, + # because the runners have neither pyyaml nor yq and a hand-written parser + # could read nothing and report nothing - see the script's header. + steps: + - name: "Checkout Repository" + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + - name: "Install Helm" + uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 # v5 + - name: "Check every rendered scalar that must be a string" + run: ci/run-yaml-scalar-render-test.sh + trivy-config: name: "Trivy config ${{ matrix.chart }}" needs: detect-changed-charts diff --git a/README.md b/README.md index 79cfaf2..b1f1102 100644 --- a/README.md +++ b/README.md @@ -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-.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 ` — 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 ` — 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/.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/.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/.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 ` 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 diff --git a/ci/run-selector-match-test.sh b/ci/run-selector-match-test.sh index 692bb9b..b688a0e 100755 --- a/ci/run-selector-match-test.sh +++ b/ci/run-selector-match-test.sh @@ -52,10 +52,22 @@ # # WHAT IS DELIBERATELY NOT CHECKED # -# - podAffinity / podAntiAffinity. They carry labelSelectors too, but their -# semantics are "relative to pods that exist", and an anti-affinity term -# matching nothing is a legitimate configuration rather than a mistake. -# Out of scope, and named so that the gap is not mistaken for coverage. +# - preferred podAffinity and podAntiAffinity of either strength. They carry +# labelSelectors too, but their semantics are "relative to pods that exist", +# and 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. Out of scope, and named so that the gap is +# not mistaken for coverage. +# +# requiredDuringSchedulingIgnoredDuringExecution podAffinity is NOT in that +# gap and is checked (Gitea #101). Its failure is not a preference quietly +# dropped: a required term whose selector matches no pod in the release can +# never be satisfied, so the pod stays Pending forever. For the workload +# that introduced one here - paperless-ngx' consume CronJob, pinned onto the +# node of its StatefulSet pod so that Longhorn will mount the shared RWX +# volume - that would mean a mover that silently stops moving, with one +# Pending pod per schedule and a green render behind it. The term names a +# label written in ANOTHER template, which is the copy this test holds. # - selectors expressed ONLY through matchExpressions. Evaluating a set-based # requirement means reimplementing label-selector semantics, which is a # second parser by another name. Those are REPORTED as "not checked" and @@ -343,6 +355,55 @@ for doc in pdbs: ", ".join("%s=%s" % kv for kv in sorted(match_labels.items())), len(all_templates), "; ".join(known) or "none")) +# 3. Every REQUIRED podAffinity term, against ALL pod templates of the render +# (Gitea #101). Not against its own template: the whole point of such a term +# is to select a DIFFERENT workload's pod, so the correct question is whether +# anything in this release carries the labels it asks for. Preferred terms and +# anti-affinity of either strength are left alone - see the header. +for where, template in all_templates: + affinity = template.get("spec", {}).get("affinity") + if not isinstance(affinity, dict): + continue + pod_affinity = affinity.get("podAffinity") + if not isinstance(pod_affinity, dict): + continue + terms = pod_affinity.get("requiredDuringSchedulingIgnoredDuringExecution") + if not isinstance(terms, list): + continue + for i, term in enumerate(terms): + if not isinstance(term, dict): + lines.append("FAIL|%s: required podAffinity[%d] at %s is not a mapping" + % (chart, i, where)) + continue + tag = "required podAffinity[%d] on %s at %s" % ( + i, term.get("topologyKey", ""), where) + match_labels, reason = classify(term.get("labelSelector"), tag) + if match_labels is None: + if term.get("labelSelector") is None: + lines.append("FAIL|%s: %s" % (chart, reason)) + else: + lines.append("SKIP|%s: %s" % (chart, reason)) + continue + hits = [w for w, t in all_templates if matches(match_labels, labels_of(t))] + if hits: + lines.append("OK|%s: %s matches %d pod template(s): %s" + % (chart, tag, len(hits), ", ".join(hits))) + else: + known = sorted({"%s=%s" % kv for _, t in all_templates for kv in labels_of(t).items()}) + lines.append("FAIL|%s: %s selects %s, which matches NONE of the %d " + "pod template(s) in the same render. A REQUIRED " + "podAffinity term that matches no pod can never be " + "satisfied, so the pod it is attached to stays Pending " + "forever - for a CronJob that means one Pending pod per " + "schedule and a workload that silently stopped running, " + "with nothing in the render to show for it. The usual " + "cause is a label renamed in the template that carries " + "the selected pod. Labels actually present on the pod " + "templates: %s" + % (chart, tag, + ", ".join("%s=%s" % kv for kv in sorted(match_labels.items())), + len(all_templates), "; ".join(known) or "none")) + for line in lines: print(line) PY diff --git a/ci/run-yaml-scalar-render-test.sh b/ci/run-yaml-scalar-render-test.sh new file mode 100755 index 0000000..b130af2 --- /dev/null +++ b/ci/run-yaml-scalar-render-test.sh @@ -0,0 +1,412 @@ +#!/usr/bin/env bash +# YAML-scalar render test: an operator value that must arrive in the manifest as +# a STRING is held against the manifest it actually produced. +# +# Usage: ci/run-yaml-scalar-render-test.sh [chart ...] (no argument = all charts) +# +# WHAT IT GUARDS AGAINST (Gitea #11, and #112 turned out to be the same genus) +# +# A value is rendered into a YAML scalar without `| quote`. The operator quotes +# it correctly in their values file, Helm passes the string's CONTENT through, +# and the quotes are gone again in the manifest. Two outcomes, and the quiet one +# is the worse one: +# +# 1. LOUD, but misdirected - the content is not a legal plain scalar and the +# render aborts. Measured on paperless-ngx' consume CronJob, whose schedule +# was unquoted: a schedule in the commonest cron notation of all (a star, a +# slash, an interval) made `helm template` exit 1 with +# +# error converting YAML to JSON: yaml: line 6: did not find expected +# alphabetic or numeric character +# +# YAML read the leading "*" as an alias indicator. The message names the +# TEMPLATE and the operator's own value is nowhere in it, so the search +# starts in the wrong place. "@daily" fails the same way. +# 2. SILENT until the API server - the content IS a legal plain scalar, just +# not a string. "true" becomes a bool, "0755" becomes an integer, and the +# chart renders perfectly. Kubernetes then rejects the object with +# "cannot unmarshal number into Go struct field EnvVar.value of type +# string" - at apply time, which for this repository means in the +# install-test or in a cluster, never in the render. +# +# Neither is caught by "does helm template exit 0" alone: the first one is, but +# only if something actually renders the object, and the second one is not +# caught at all. +# +# WHAT IS ASSERTED, and all three are properties Kubernetes itself requires - +# there is no chart for which the opposite is a legitimate configuration, which +# is why this needs no per-chart expectations: +# +# 1. CronJob .spec.schedule is a non-empty string. +# 2. Every env[].value of every container and initContainer is a string. +# 3. Every metadata.labels and metadata.annotations value is a string +# (the API types both as map[string]string). +# +# WHY OVER ALL CHARTS, not the changed ones: the same reason as +# ci/run-quantity-render-test.sh and ci/run-selector-match-test.sh. The env +# block is copied into every chart rather than shared, so the unquoted form +# travels by copy into whatever chart is started from an older one - and that +# chart's own pull request is not where anyone would look for it. Running over +# everything also means there is no list to maintain: adding a chart enrols it. +# +# THE OVERLAYS, AND WHY THIS TEST IS USELESS WITHOUT THEM +# +# The defect above survived in a shipped chart for months for one reason: the +# object that carried it is rendered by NO fixture. paperless-ngx' CronJob sits +# behind smbConnection.enabled, which is false in the chart defaults and in +# every committed fixture, so neither lint, nor the render tests, nor the +# install-test ever produced it. A test that renders only what the fixtures +# already render would have been just as blind. +# +# So ci/yaml-scalar/.yaml is an 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". Two rules follow, +# the same ones ci/run-selector-match-test.sh works under: +# +# 1. A chart that HAS an overlay and still renders none of the three things +# above FAILS. The overlay exists for exactly that, so an overlay that +# stopped reaching its object is a finding and not a shrug. +# 2. If nothing at all was asserted across the whole run, the run fails. A +# pass over zero assertions is not a pass. +# +# WHAT IS DELIBERATELY NOT CHECKED, so the gap is not mistaken for coverage: +# +# - Values rendered into a scalar that is hand-quoted in the template +# ("{{ .Values.x }}"). Those are strings in the output, so this test is +# blind to them by construction; what they can still do is break the YAML +# when the value itself contains a quote or a backslash, and only `| quote` +# fixes that. Reported here rather than silently implied. +# - 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. The +# three above are where operator free text actually lands. +# - Whether a cron schedule is a VALID schedule. That is the cron +# controller's judgement, and narrowing the schema to a regex was considered +# and rejected: the defect was never a wrong schedule, it was a correct one +# rendered bare. +# +# How the YAML is parsed: with helm, via ci/yaml2json, exactly as in +# ci/run-selector-match-test.sh - the runners have neither pyyaml nor yq, and a +# YAML reader written in awk here would fail by quietly reading nothing and +# staying green. That script's header carries the full argument. +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +CI_DIR="$ROOT/ci" +YAML2JSON="$CI_DIR/yaml2json" +OVERLAY_DIR="$CI_DIR/yaml-scalar" +RELEASE="scalar" + +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +# --- Which charts ----------------------------------------------------------- +CHARTS=() +if [ "$#" -gt 0 ]; then + CHARTS=("$@") +else + for DIR in "$ROOT"/*/; do + [ -f "$DIR/Chart.yaml" ] || continue + CHARTS+=("$(basename "$DIR")") + done +fi +if [ "${#CHARTS[@]}" -eq 0 ]; then + echo "ERROR: no chart directories found under $ROOT" >&2 + exit 1 +fi + +FAILED="" +CHECKED="" +NOT_EXERCISED="" +TOTAL_ASSERTIONS=0 +SUMMARY_ROWS="" + +for CHART in "${CHARTS[@]}"; do + if [ ! -f "$ROOT/$CHART/Chart.yaml" ]; then + echo "ERROR: '$CHART' is not a chart directory" >&2 + exit 1 + fi + + # A chart with a declared dependency cannot be rendered at all while charts/ + # is empty, and this job runs over EVERY chart on EVERY pull request. + "$ROOT/ci/helm-dependencies.sh" "$ROOT/$CHART" + + VALUES_ARGS=() + VALUES_LABEL="chart defaults" + if [ -f "$CI_DIR/$CHART/test-values.yaml" ]; then + VALUES_ARGS=(-f "$CI_DIR/$CHART/test-values.yaml") + VALUES_LABEL="ci/$CHART/test-values.yaml" + elif [ -f "$CI_DIR/trivy/values/$CHART.yaml" ]; then + VALUES_ARGS=(-f "$CI_DIR/trivy/values/$CHART.yaml") + VALUES_LABEL="ci/trivy/values/$CHART.yaml" + fi + HAS_OVERLAY="" + if [ -f "$OVERLAY_DIR/$CHART.yaml" ]; then + # Last, so it wins over the install-test values. + VALUES_ARGS+=(-f "$OVERLAY_DIR/$CHART.yaml") + VALUES_LABEL="$VALUES_LABEL + ci/yaml-scalar/$CHART.yaml" + HAS_OVERLAY=1 + fi + + echo "::group::$CHART ($VALUES_LABEL)" + + RENDER="$WORK/$CHART.manifest.yaml" + if ! helm template "$RELEASE" "$ROOT/$CHART" \ + ${VALUES_ARGS[@]+"${VALUES_ARGS[@]}"} >"$RENDER" 2>"$WORK/err"; then + sed 's/^/ /' "$WORK/err" + echo "::endgroup::" + echo "::error title=yaml-scalar $CHART::$CHART does not render with these values. If the error is a YAML parse error naming a template line, that IS the finding this test exists for: a value reached a scalar unquoted. Reproduce: helm template $RELEASE $CHART $(printf '%s ' "${VALUES_ARGS[@]+${VALUES_ARGS[@]}}")" + FAILED="$FAILED $CHART" + continue + fi + + # The document split, the only textual step. A wrong split cannot produce a + # false pass: the result stops being parsable and helm exits 1. + DOCS="$WORK/$CHART.docs.yaml" + awk ' + BEGIN { print "docs:"; started = 0 } + /^---[ \t]*$/ { print " -"; started = 1; next } + { + if (!started) { print " -"; started = 1 } + if (length($0) == 0) print ""; else print " " $0 + } + ' "$RENDER" >"$DOCS" + + JSON="$WORK/$CHART.json" + if ! helm template y2j "$YAML2JSON" -f "$DOCS" 2>"$WORK/err" \ + | awk '/^\{/ { print; exit }' >"$JSON"; then + sed 's/^/ /' "$WORK/err" + echo "::endgroup::" + echo "::error title=yaml-scalar $CHART::the rendered manifests of $CHART could not be re-read as YAML by ci/yaml2json. That is a parse failure, not a scalar finding." + FAILED="$FAILED $CHART" + continue + fi + if [ ! -s "$JSON" ]; then + echo "::endgroup::" + echo "::error title=yaml-scalar $CHART::ci/yaml2json produced no JSON for $CHART - the manifests were not re-read, so nothing was checked." + FAILED="$FAILED $CHART" + continue + fi + + # --- The comparison -------------------------------------------------------- + # Standard library only, and it never decides the exit code itself: it writes + # "VERDICT|detail" lines and the loop below decides, so a verdict can never + # be lost to a swallowed status. + set +e + python3 - "$CHART" "$JSON" >"$WORK/$CHART.out" 2>"$WORK/$CHART.pyerr" <<'PY' +import json, sys + +chart, path = sys.argv[1], sys.argv[2] +docs = json.load(open(path)).get("docs") or [] +lines = [] + + +def name_of(doc): + meta = doc.get("metadata") + if isinstance(meta, dict) and meta.get("name"): + return str(meta["name"]) + return "" % doc.get("kind", "object") + + +def typename(value): + return {bool: "bool", int: "int", float: "float", type(None): "null", + dict: "mapping", list: "list"}.get(type(value), type(value).__name__) + + +def pod_templates(node, path="$"): + """Any value of a key named "template" carrying a spec with containers. + Found by walking, so a CronJob's spec.jobTemplate.spec.template needs no + special case and neither does whatever workload kind a chart adds next.""" + out = [] + if isinstance(node, dict): + for key, value in node.items(): + here = "%s.%s" % (path, key) + if (key == "template" and isinstance(value, dict) + and isinstance(value.get("spec"), dict) + and "containers" in value["spec"]): + out.append((here, value)) + out.extend(pod_templates(value, here)) + elif isinstance(node, list): + for i, value in enumerate(node): + out.extend(pod_templates(value, "%s[%d]" % (path, i))) + return out + + +def metadatas(node, path="$"): + """Every metadata mapping anywhere in the render - top level and inside pod + templates both, because labels and annotations exist at both levels.""" + out = [] + if isinstance(node, dict): + for key, value in node.items(): + here = "%s.%s" % (path, key) + if key == "metadata" and isinstance(value, dict): + out.append((here, value)) + out.extend(metadatas(value, here)) + elif isinstance(node, list): + for i, value in enumerate(node): + out.extend(metadatas(value, "%s[%d]" % (path, i))) + return out + + +# --- 1. CronJob schedules --------------------------------------------------- +for doc in docs: + if not isinstance(doc, dict) or doc.get("kind") != "CronJob": + continue + where = "CronJob %s .spec.schedule" % name_of(doc) + spec = doc.get("spec") if isinstance(doc.get("spec"), dict) else {} + schedule = spec.get("schedule", "") + if isinstance(schedule, str) and schedule.strip(): + lines.append("OK|%s: %s is the string %r" % (chart, where, schedule)) + elif schedule == "": + lines.append("FAIL|%s: %s is missing entirely - a CronJob without a " + "schedule is rejected by the API server" % (chart, where)) + else: + lines.append("FAIL|%s: %s rendered as %s (%r), not a string. A schedule " + "is operator free text and has to leave the template " + "through | quote; without it YAML re-reads the content and " + "a schedule beginning with a digit survives by luck while " + "one beginning with * or @ does not" + % (chart, where, typename(schedule), schedule)) + +# --- 2. env values ---------------------------------------------------------- +for where, template in pod_templates({"docs": docs}): + spec = template.get("spec") if isinstance(template.get("spec"), dict) else {} + for kind in ("initContainers", "containers"): + for container in spec.get(kind) or []: + if not isinstance(container, dict): + continue + cname = container.get("name", "") + for entry in container.get("env") or []: + if not isinstance(entry, dict) or "value" not in entry: + continue # valueFrom entries carry no literal to check + value = entry["value"] + tag = "env %s of container %s at %s" % ( + entry.get("name", ""), cname, where) + if isinstance(value, str): + lines.append("OK|%s: %s is a string" % (chart, tag)) + else: + lines.append("FAIL|%s: %s rendered as %s (%r), not a string. " + "Kubernetes types env[].value as a string and " + "the API server rejects the whole manifest with " + "\"cannot unmarshal %s into Go struct field " + "EnvVar.value of type string\" - at apply time, " + "not in the render. The fix is | quote at the " + "render site; template-chart carries the form" + % (chart, tag, typename(value), value, + "number" if not isinstance(value, bool) else "bool")) + +# --- 3. labels and annotations ---------------------------------------------- +for where, meta in metadatas({"docs": docs}): + for field in ("labels", "annotations"): + block = meta.get(field) + if not isinstance(block, dict): + continue + for key, value in block.items(): + tag = "%s[%r] at %s" % (field, key, where) + if isinstance(value, str): + lines.append("OK|%s: %s is a string" % (chart, tag)) + else: + lines.append("FAIL|%s: %s rendered as %s (%r), not a string. " + "Kubernetes types %s as map[string]string, so the " + "API server rejects the object - the render stays " + "green. The fix is | quote at the render site" + % (chart, tag, typename(value), value, field)) + +for line in lines: + print(line) +PY + RC=$? + set -e + if [ "$RC" -gt 1 ]; then + sed 's/^/ /' "$WORK/$CHART.pyerr" + echo "::endgroup::" + echo "::error title=yaml-scalar $CHART::the scalar comparison itself failed for $CHART (python exit $RC). Treated as a failure, never as a pass." + FAILED="$FAILED $CHART" + continue + fi + + CHART_FAIL="" + CHART_OK=0 + while IFS='|' read -r VERDICT DETAIL; do + case "$VERDICT" in + OK) + CHART_OK=$((CHART_OK + 1)) + echo " ok $DETAIL" + ;; + FAIL) + CHART_FAIL=1 + echo " FAIL $DETAIL" + echo "::error title=yaml-scalar $CHART::$DETAIL" + ;; + *) + [ -n "$VERDICT" ] && echo " $VERDICT $DETAIL" + ;; + esac + done <"$WORK/$CHART.out" + echo "::endgroup::" + + TOTAL_ASSERTIONS=$((TOTAL_ASSERTIONS + CHART_OK)) + + if [ -n "$CHART_FAIL" ]; then + FAILED="$FAILED $CHART" + SUMMARY_ROWS="${SUMMARY_ROWS}| \`$CHART\` | **failed** | $CHART_OK |"$'\n' + echo "$CHART: FAILED" + elif [ "$CHART_OK" -gt 0 ]; then + CHECKED="$CHECKED $CHART" + SUMMARY_ROWS="${SUMMARY_ROWS}| \`$CHART\` | ok | $CHART_OK |"$'\n' + echo "$CHART: ok ($CHART_OK scalar(s) checked)" + elif [ -n "$HAS_OVERLAY" ]; then + FAILED="$FAILED $CHART" + SUMMARY_ROWS="${SUMMARY_ROWS}| \`$CHART\` | **failed** | 0 |"$'\n' + echo "::error title=yaml-scalar $CHART::$CHART has an overlay at ci/yaml-scalar/$CHART.yaml but its render carried no schedule, no env value and no label or annotation at all, so nothing was checked. That overlay exists only to make such a scalar render; a green tick here would be a tick for a check that ran over nothing." + echo "$CHART: FAILED (overlay present, nothing asserted)" + else + NOT_EXERCISED="$NOT_EXERCISED $CHART" + echo "$CHART: nothing of the three kinds rendered - nothing to check." + fi +done + +# --- Report ------------------------------------------------------------------ +count() { set -- $1; echo "$#"; } +listing() { set -- $1; [ "$#" -eq 0 ] && { echo "–"; return; }; printf '`%s`' "$1"; shift; for C in "$@"; do printf ', `%s`' "$C"; done; echo; } +SUMMARY="${GITHUB_STEP_SUMMARY:-/dev/null}" +{ + echo "### YAML-scalar render test (Gitea #11)" + echo + echo "Every \`CronJob.spec.schedule\`, every \`env[].value\` and every label and" + echo "annotation value in a chart's render, held against the type Kubernetes requires" + echo "for it. A value rendered without \`| quote\` either aborts the render with an" + echo "error pointing at the template instead of at the value, or arrives as a bool or" + echo "an integer and is rejected by the API server at apply time - never in the render." + echo + echo "| Result | Count | Charts |" + echo "|---|---|---|" + echo "| Scalars checked | $(count "$CHECKED") | $(listing "$CHECKED") |" + echo "| Nothing of the kind rendered | $(count "$NOT_EXERCISED") | $(listing "$NOT_EXERCISED") |" + echo "| Failed | $(count "$FAILED") | $(listing "$FAILED") |" + echo + echo "$TOTAL_ASSERTIONS scalar(s) verified in total." + if [ -n "$SUMMARY_ROWS" ]; then + echo + echo "| Chart | Result | Checked |" + echo "| --- | --- | --- |" + printf '%s' "$SUMMARY_ROWS" + fi +} >>"$SUMMARY" + +echo "===================== yaml scalar render test ======================" +echo "checked: $(count "$CHECKED")${CHECKED:+ -$CHECKED}" +echo "not exercised: $(count "$NOT_EXERCISED")${NOT_EXERCISED:+ -$NOT_EXERCISED}" +echo "failed: $(count "$FAILED")${FAILED:+ -$FAILED}" +echo "scalars verified: $TOTAL_ASSERTIONS" +echo "===================================================================" + +if [ -n "$FAILED" ]; then + echo "ERROR: a value that has to arrive as a string did not, in:$FAILED" >&2 + exit 1 +fi + +if [ "$TOTAL_ASSERTIONS" -eq 0 ]; then + echo "ERROR: not a single scalar was checked in this run. Either no chart renders a schedule, an env value, a label or an annotation any more, or the overlays under ci/yaml-scalar/ no longer reach their objects. A pass over zero assertions is not a pass." >&2 + exit 1 +fi diff --git a/ci/selector-match/paperless-ngx.yaml b/ci/selector-match/paperless-ngx.yaml new file mode 100644 index 0000000..2d5a34e --- /dev/null +++ b/ci/selector-match/paperless-ngx.yaml @@ -0,0 +1,25 @@ +# Selector-match overlay for paperless-ngx (Gitea #101). +# +# Its one job: make the consume CronJob render, so that the required podAffinity +# it carries is actually held against the StatefulSet's pod labels. The template +# sits behind smbConnection.enabled, false in the chart defaults and in +# ci/paperless-ngx/test-values.yaml, so without this overlay the affinity term +# exists in the repository and in no render - and the test would pass over it. +# +# What it guards: the term selects the StatefulSet pod by a label written in +# ANOTHER template (paperless-server.sfs.yaml). Renaming that label leaves both +# files rendering perfectly while the mover can never be scheduled again, one +# Pending pod per schedule, nothing in the render to show for it. +# +# Kept separate from ci/yaml-scalar/paperless-ngx.yaml although both switch SMB +# on: that one feeds the CronJob a hostile SCHEDULE and hostile env values for a +# different test, and folding the two would make each test's failure depend on +# the other's fixture. +smbConnection: + enabled: true + server: "//smb.ci.local/scans" + +secrets: + # Satisfies the required() behind the SMB PersistentVolume's + # nodeStageSecretRef. Nothing is mounted in a render. + smbConnectionSecretRef: op://ci/paperless-smb diff --git a/ci/yaml-scalar/paperless-ngx.yaml b/ci/yaml-scalar/paperless-ngx.yaml new file mode 100644 index 0000000..6ee0a0d --- /dev/null +++ b/ci/yaml-scalar/paperless-ngx.yaml @@ -0,0 +1,53 @@ +# YAML-scalar overlay for paperless-ngx (Gitea #11). +# +# THIS FILE IS THE TEST CASE the issue asked for, and its first job is to make +# an object exist at all. templates/paperless-consume.cronjob.yaml sits behind +# smbConnection.enabled, which is false in the chart defaults and false in +# ci/paperless-ngx/test-values.yaml - deliberately, because the install-test has +# no SMB CSI driver. So before this overlay NOTHING in the repository rendered +# that CronJob: not lint, not the render tests, not the install-test. That is +# the whole reason an unquoted schedule survived in a shipped chart, and it is +# why the fix comes with a render test rather than with a fixture change. +# +# The values below are hostile on purpose. Against the chart before 13.0.0 this +# overlay does not produce a finding - it produces an aborted render, which this +# test reports as the finding it is: +# +# 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 +smbConnection: + enabled: true + server: "//smb.ci.local/scans" + # A schedule beginning with "*" - the commonest cron notation there is, and + # the one that YAML reads as an alias indicator. Written here as a quoted + # string, which is exactly how an operator would write it and exactly what + # does NOT help: Helm passes the content through, and without | quote in the + # template the star stands bare in the manifest again. + cronJob: "*/15 * * * *" + +secrets: + # The CronJob's SMB volume has a required() secret ref behind it; the + # PersistentVolume names it as nodeStageSecretRef. Nothing is mounted in a + # render, so no fixture Secret is needed - only a value that satisfies the + # required(). + smbConnectionSecretRef: op://ci/paperless-smb + +paperless: + # The silent half of the genus, on the chart's additive env block: both of + # these are STRINGS in this file and both are schema-valid, and before 13.0.0 + # the first one aborted the render while the second arrived in the manifest as + # a YAML bool - which the API server rejects at apply time with "cannot + # unmarshal bool into Go struct field EnvVar.value of type string", so the + # render test is the only place it can be caught cheaply. + env: + - name: PAPERLESS_STAR_VALUE + value: "*never-interpreted*" + - name: PAPERLESS_BOOLEAN_LOOKING_VALUE + value: "true" + # The variable the whole env block was asked for (Gitea #112): the log off + # the NFS-backed RWX data volume and onto the /tmp emptyDir this pod already + # mounts. Harmless in a render, and it keeps the worked example honest by + # being the one an operator will actually copy. + - name: PAPERLESS_LOGGING_DIR + value: "/tmp/paperless-log" diff --git a/paperless-ngx/Chart.yaml b/paperless-ngx/Chart.yaml index 8c4b3cd..957c2c2 100644 --- a/paperless-ngx/Chart.yaml +++ b/paperless-ngx/Chart.yaml @@ -5,7 +5,7 @@ icon: https://raw.githubusercontent.com/paperless-ngx/paperless-ngx/dev/src-ui/s type: application -version: 12.3.0+up3.2.1 +version: 13.0.0+up3.2.1 appVersion: 3.2.1 diff --git a/paperless-ngx/templates/NOTES.txt b/paperless-ngx/templates/NOTES.txt index 268c484..68defad 100644 --- a/paperless-ngx/templates/NOTES.txt +++ b/paperless-ngx/templates/NOTES.txt @@ -8,6 +8,9 @@ {{- $valkeySvc := printf "%s-valkey-service" .Release.Name -}} {{- $pre := default (list) $scripts.preConsumptionScripts -}} {{- $post := default (list) $scripts.postConsumptionScripts -}} +{{- $cron := fromYaml (include "paperless-ngx.cron.scheduling" .Values.smbConnection.cron) -}} +{{- $cronPinned := $cron.pinToServerNode -}} +{{- $cronDeadline := include "paperless-ngx.formatNumber" $cron.activeDeadlineSeconds -}} {{- $httpRoute := .Values.httpRoute | default dict -}} {{- $hostnames := $httpRoute.hostnames | default list -}} {{- if not $hostnames -}} @@ -184,15 +187,14 @@ WHAT THE 1PASSWORD ITEMS MUST CONTAIN here, so a wrong field name surfaces as a mount failure on the CronJob's pod — a describe on that pod, never a missing variable. {{- end }} -{{- if .Values.secrets.oidcSecretRef }} - - secrets.oidcSecretRef IS SET AND IS READ BY NOTHING - "{{ .Values.secrets.oidcSecretRef }}" — no template in this chart references - that value. No OnePasswordItem is created for it and no Secret appears. The - OIDC client id travels as plain env from oidc.provider.clientId, and no - client secret is configured at all (see the OIDC section below). Whatever - you put in that item has no effect on this release. -{{- end }} +{{- /* +The warning that used to stand here - "secrets.oidcSecretRef IS SET AND IS READ +BY NOTHING" - is gone with the value itself (13.0.0, Gitea #11). It cannot be +printed any more and would not be reached: the key is out of values.schema.json +too, so a release that still sets it fails BEFORE any template renders, with +"additional properties 'oidcSecretRef' not allowed". The fix is to delete the +key; values.yaml says why it is not coming back. +*/}} CHANGING A SECRET'S CONTENT RESTARTS NOTHING Helm never sees the content of a OnePasswordItem, so it cannot hash it, and @@ -340,12 +342,44 @@ HOW DOCUMENTS GET IN move, not a copy: once the job has run, the share no longer holds the file and the only copy is on the consume volume. - One thing to know before you edit that schedule: it is rendered into the - manifest unquoted, so a schedule that STARTS with "*" ("*/15 * * * *") - aborts the render with a YAML parse error on this template — YAML reads the - leading * as an alias. Quoting it in your values file does not help, 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. + The schedule is quoted on the way into the manifest since 13.0.0, so any + cron notation works — one starting with "*" or an "@daily" macro included. + Before that such a schedule aborted the render with a YAML parse error + pointing at the template, and the advice here was to avoid it; if you are + carrying a workaround schedule from that era ("5,20,35,50 * * * *" instead + of every 15 minutes), you can write what you meant now. +{{- if $cronPinned }} + + PLACEMENT: the mover runs on the NODE OF THE STATEFULSET POD, via a + required podAffinity on its label (Gitea #101). Without that it lands + anywhere, and because it mounts the consume volume — RWX, from Longhorn — + the kubelet of the StatefulSet's node then retries the mount every two + minutes with "no Pending workload pods for volume ... to be mounted". It + was measured at 291 such events in 10 hours. + + THE PRICE, and it is the one thing to know before scaling down: with no + StatefulSet pod the mover CANNOT BE SCHEDULED. scaleDown: true suspends + this CronJob outright, so that case is covered — but paperless.replicas: 0 + with SMB still on is not, and there the job fails once per schedule after + activeDeadlineSeconds ({{ $cronDeadline }}s) with reason DeadlineExceeded. That is + deliberate: a bounded, visible failure rather than a Pending pod per + schedule that nobody sees. Switch smbConnection.cron.pinToServerNode to + false to go back to unpinned placement — and to the mount errors. + + 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. +{{- else }} + + PLACEMENT IS OFF (smbConnection.cron.pinToServerNode: false). The mover + will be scheduled wherever there is room, and when that is not the node of + the StatefulSet pod, Longhorn refuses to mount the shared RWX consume + volume there: the kubelet of the StatefulSet's node retries every two + minutes with "no Pending workload pods for volume ... to be mounted" + (Gitea #101, measured at 291 events in 10 hours). The errors stop when the + StatefulSet pod is deleted and comes back Pending, and return with the next + firing of this job. +{{- end }} The job has NO probes (the documented exception to the chart conventions), and it cannot fail usefully either: with nothing to move it exits 0, which @@ -432,8 +466,13 @@ OIDC IS ON, AND IT IS PKCE-ONLY {{- if not .Values.oidc.provider.tokenAuthMethod }} oidc.provider.tokenAuthMethod is unset. Since the allauth update in 3.0 some - providers need it spelled out (client_secret_basic); the symptom is a login - that dies at the callback with invalid_client. + providers need it spelled out, and the symptom when they do is a login that + dies at the callback with invalid_client. The only value this chart accepts is + "none" — it states the public client explicitly. A method that authenticates + WITH a client secret cannot work here and is rejected by the schema (13.0.0, + Gitea #11): this release configures no client secret and cannot, because + paperless reads the whole OIDC configuration out of one json environment + variable with no _FILE variant. {{- end }} {{- if or .Values.oidc.autoLoginWithProvider .Values.oidc.disableRegularLogin }} @@ -453,6 +492,20 @@ paperless.scripts IS SET AND DOES NOTHING PAPERLESS_PRE_CONSUME_SCRIPT nor PAPERLESS_POST_CONSUME_SCRIPT is set on the container. These values render no object and change no behaviour. {{- end }} +{{- if .Values.paperless.env }} + +paperless.env ADDS {{ len .Values.paperless.env }} VARIABLE(S) — AND AN OVERRIDE ONLY BITES AT INSTALL + {{ range $i, $e := .Values.paperless.env }}{{ if $i }}, {{ end }}{{ $e.name }}{{ end }} + + These are appended AFTER the variables the chart sets itself, so at INSTALL an + entry repeating one of those names wins. On an upgrade of 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. Read the value + back off the StatefulSet, never off the upgrade's exit code: + + kubectl -n {{ .Release.Namespace }} get statefulset {{ .Release.Name }}-{{ .Chart.Name }}-sfs \ + -o jsonpath='{.spec.template.spec.containers[0].env}' +{{- end }} {{- if gt (int $replicas) 1 }} paperless.replicas IS {{ $replicas }} — EXPECT ONE POD TO WORK AND THE REST TO WAIT diff --git a/paperless-ngx/templates/_helpers.tpl b/paperless-ngx/templates/_helpers.tpl index 5083293..2fba559 100644 --- a/paperless-ngx/templates/_helpers.tpl +++ b/paperless-ngx/templates/_helpers.tpl @@ -340,3 +340,41 @@ The same three fallbacks for the SMB consume-mover CronJob. It has no probes {{- $merged := fromYaml (include "paperless-ngx.defaultedDict" (dict "defaults" $defaults "given" . "path" "smbConnection.cron.resources")) -}} {{- include "paperless-ngx.resources" $merged -}} {{- end -}} + +{{/* +The whole smbConnection.cron block, merged onto its defaults (YAML; consume +with fromYaml). + +Input: .Values.smbConnection.cron (may be nil, empty or partial). + +WHY EVERY READ OF THAT BLOCK GOES THROUGH HERE. The CronJob template used to +dereference .Values.smbConnection.cron. directly, so an override that had +erased the block - the leftover childless + + cron: + +that issue #61 is about - did not fall back to the defaults the way this chart +promises everywhere else. It aborted the render with + + nil pointer evaluating interface {}.securityContext + +naming an internal template path and neither the value nor the cause. Measured +on origin/main before 13.0.0, so this is a fix and not a precaution. + +The scalar defaults below MUST stay in sync with the smbConnection.cron block +in values.yaml, which stays the documented, user-facing place for them; this +copy is only the fallback. The sub-blocks (securityContext, resources, env) +keep their own defaults in the helpers above and are only passed through here. + +An explicit "pinToServerNode: false" survives this, which is the whole reason +it is not a "default true" pipe: sprig's default would read false as unset and +silently turn the pinning back on - the one value whose explicit false has to +be obeyed, since it exists to switch off a hard placement rule. +*/}} +{{- define "paperless-ngx.cron.scheduling" -}} +{{- $defaults := dict + "pinToServerNode" true + "startingDeadlineSeconds" 600 + "activeDeadlineSeconds" 3600 -}} +{{- include "paperless-ngx.defaultedDict" (dict "defaults" $defaults "given" . "path" "smbConnection.cron") -}} +{{- end -}} diff --git a/paperless-ngx/templates/paperless-consume.cronjob.yaml b/paperless-ngx/templates/paperless-consume.cronjob.yaml index ba8ba82..db2c7bb 100644 --- a/paperless-ngx/templates/paperless-consume.cronjob.yaml +++ b/paperless-ngx/templates/paperless-consume.cronjob.yaml @@ -1,26 +1,165 @@ {{- if .Values.smbConnection.enabled }} +{{- /* +The cron block, merged onto its defaults, so that an override which erased it +falls back instead of aborting with a nil-pointer error naming an internal +template path - which is what every direct .Values.smbConnection.cron. +read did before 13.0.0. The helper carries the measurement. +*/}} +{{- $cron := fromYaml (include "paperless-ngx.cron.scheduling" .Values.smbConnection.cron) -}} apiVersion: batch/v1 kind: CronJob metadata: name: {{ .Release.Name }}-{{ .Chart.Name }}-consume-cronjob spec: - schedule: {{ .Values.smbConnection.cronJob }} + {{- /* + QUOTED, AND THAT IS THE WHOLE OF GITEA #11's FIRST HALF. The default + "0 0,12 * * *" renders safely only because it begins with a digit. The most + common cron notation of all - a star, a slash and an interval, every 15 + minutes - begins with "*", which YAML reads as an alias indicator, and the + render ABORTS: + + (and the notation cannot be written out in this comment, because the two + characters that start it would end a Go template comment right here) + + 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 trap is that quoting it in the VALUES does not help: Helm passes the + string's content through, and the star stands bare in the manifest again. So + the operator writes a correctly quoted value and gets an error that points at + this template instead of at their value. "@daily" fails the same way ("@" is + a reserved YAML indicator). + + ci/run-yaml-scalar-render-test.sh is the test that would have caught it, and + ci/yaml-scalar/paperless-ngx.yaml is the overlay that feeds it a star-leading + schedule - needed because this whole template sits behind + smbConnection.enabled, which is false in every other fixture, so nothing else + in the repository renders this object at all. + */}} + schedule: {{ .Values.smbConnection.cronJob | quote }} + {{- /* + Forbid, hard-wired rather than a value: two movers running at once race on + the same files ("mv /mnt/smb/*.pdf" twice over one directory), and the loser + reports a file that is already gone. There is no configuration in which + overlapping runs of this job are wanted, so there is nothing to configure. + */}} + concurrencyPolicy: Forbid + {{- /* + Bounds how late a missed run may still start, and it is here for a second + reason that has nothing to do with lateness: while the CronJob is suspended + (scaleDown) or its runs are being suppressed, the controller counts missed + schedules, and above 100 it STOPS SCHEDULING ALTOGETHER with only + "Cannot determine if job needs to be started: too many missed start times" in + the controller log. With this field set, the controller only counts misses + inside this window, so that state cannot be reached. The cost is stated in + values.yaml: a run that could not start inside the window is skipped, not + queued. + */}} + startingDeadlineSeconds: {{ include "paperless-ngx.formatNumber" $cron.startingDeadlineSeconds }} {{- if .Values.scaleDown }} suspend: true {{- end }} jobTemplate: spec: + {{- /* + THE SAFETY NET UNDER THE AFFINITY BELOW (Gitea #101). activeDeadlineSeconds + counts from the job's start, not from the pod's, so it also bounds a pod + that NEVER gets scheduled: when the StatefulSet pod is absent and the + required affinity cannot be satisfied, the job fails after this many + seconds with reason DeadlineExceeded instead of sitting Pending forever + and gaining a sibling every schedule. That turns the one new failure mode + this issue's fix could introduce into a visible, self-clearing one - + failedJobsHistoryLimit (Kubernetes' default, 1) keeps the evidence. + */}} + activeDeadlineSeconds: {{ include "paperless-ngx.formatNumber" $cron.activeDeadlineSeconds }} template: spec: + {{- if $cron.pinToServerNode }} + {{- /* + GITEA #101: this pod mounts the consume volume, which is RWX and comes + from Longhorn. Without a placement rule the scheduler puts it on any + node, and Longhorn's CSI driver then refuses the mount on the node + where the StatefulSet pod runs: + + MountVolume.SetUp failed ... code = Aborted desc = no Pending + workload pods for volume pvc-... to be mounted: + map[Running:[...-sfs-0] Succeeded:[...-consume-cronjob-...]] + + Measured at every two minutes, 291 events in 10 hours, in both + releases that have SMB switched on; the release with + smbConnection.enabled: false and therefore no CronJob produced none of + them on identically built volumes. + + WHY podAffinity AND NOT nodeSelector: the StatefulSet pod is not + pinned either - it sits wherever the scheduler put it and moves on + every restart. A static nodeSelector would be correct exactly until + then and silently wrong afterwards. The affinity follows the pod. + + WHY required AND NOT preferred: preferred falls back to another node + in the very situation this exists for, which reproduces the bug more + rarely and therefore harder to find. The fallback is what went + unnoticed for ten hours. + + The label is the one the StatefulSet puts on its pod template + (paperless-server.sfs.yaml, metadata.labels.app) and is read off the + running pod, not assumed. + + WHAT THIS DOES NOT FIX: the volume stays RWX and keeps its + share-manager; co-locating the two consumers removes the CROSS-NODE + attach, not the second consumer. The design that removes it is moving + the mv into the StatefulSet pod, which cannot be done for the running + installations because RWX cannot be taken back off an existing PVC. + Gitea #101 names it as the alternative and not as the proposal. + */}} + affinity: + podAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + - labelSelector: + matchLabels: + app: {{ .Release.Name }}-{{ .Chart.Name }}-sfs + topologyKey: kubernetes.io/hostname + {{- end }} automountServiceAccountToken: false securityContext: - {{- include "paperless-ngx.cron.podSecurityContext" .Values.smbConnection.cron.securityContext | nindent 12 }} + {{- include "paperless-ngx.cron.podSecurityContext" $cron.securityContext | nindent 12 }} containers: - name: paperless-consume-cronjob image: "{{ .Values.paperless.busybox.image.repository }}:{{ .Values.paperless.busybox.image.tag }}" imagePullPolicy: IfNotPresent securityContext: - {{- include "paperless-ngx.cron.containerSecurityContext" .Values.smbConnection.cron.securityContext | nindent 16 }} + {{- include "paperless-ngx.cron.containerSecurityContext" $cron.securityContext | nindent 16 }} + {{- if $cron.env }} + env: + {{- range $value := $cron.env }} + {{- if $value.value }} + {{- /* + quote is required, not cosmetic: a value like "8080" or "true" + renders unquoted as a YAML number/bool, and the API server then + rejects the whole manifest with "cannot unmarshal number into + Go struct field EnvVar.value of type string". A template + comment and not a "#" one, because a "#" line here is part of + the rendered manifest. + */}} + - name: {{ $value.name | quote }} + value: {{ tpl $value.value $ | quote }} + {{- else if $value.valueFrom }} + {{- if $value.valueFrom.secretKeyRef }} + - name: {{ $value.name }} + valueFrom: + secretKeyRef: + name: {{ tpl $value.valueFrom.secretKeyRef.name $ | quote }} + key: {{ tpl $value.valueFrom.secretKeyRef.key $ | quote }} + {{- else if $value.valueFrom.configMapKeyRef }} + - name: {{ $value.name }} + valueFrom: + configMapKeyRef: + name: {{ tpl $value.valueFrom.configMapKeyRef.name $ | quote }} + key: {{ tpl $value.valueFrom.configMapKeyRef.key $ | quote }} + {{- end }} + {{- end }} + {{- end }} + {{- end }} command: - /bin/sh - -c @@ -33,7 +172,7 @@ spec: mountPath: "/mnt/consume" readOnly: false resources: - {{- include "paperless-ngx.cron.effectiveResources" .Values.smbConnection.cron.resources | nindent 16 }} + {{- include "paperless-ngx.cron.effectiveResources" $cron.resources | nindent 16 }} restartPolicy: Never volumes: - name: smb-share @@ -42,4 +181,4 @@ spec: - name: consume-folder persistentVolumeClaim: claimName: {{ .Release.Name }}-{{ .Chart.Name }}-consume-volume-claim -{{- end }} \ No newline at end of file +{{- end }} diff --git a/paperless-ngx/templates/paperless-server.sfs.yaml b/paperless-ngx/templates/paperless-server.sfs.yaml index 1484a87..6a10397 100644 --- a/paperless-ngx/templates/paperless-server.sfs.yaml +++ b/paperless-ngx/templates/paperless-server.sfs.yaml @@ -204,23 +204,41 @@ spec: - name: PAPERLESS_DISABLE_REGULAR_LOGIN value: "{{ .Values.oidc.disableRegularLogin }}" {{- end }} + {{- /* + QUOTED SINCE 13.0.0, and it is a fix rather than a tidy-up (Gitea + #112, same genus as #11). This block is the chart's additive env + interface, and until 13.0.0 it rendered its operand bare. Two + measured consequences, both of them the operator's value arriving + as something else: + + - a value beginning with "*" aborted the render, with the error + pointing at THIS template rather than at the value; + - "true" and "0755" rendered as a YAML bool and an integer, and + the API server rejects those with "cannot unmarshal number + into Go struct field EnvVar.value of type string" - the chart + renders, the install fails. + + The quoted form is the repository's documented one + (template-chart/templates/xxx.deployment.yaml carries the rule); + this chart was one of seven that had not been brought to it. + */}} {{- range $value := .Values.paperless.env }} {{- if $value.value }} - - name: {{ $value.name }} - value: {{ tpl $value.value $ }} + - name: {{ $value.name | quote }} + value: {{ tpl $value.value $ | quote }} {{- else if $value.valueFrom }} {{- if $value.valueFrom.secretKeyRef }} - name: {{ $value.name }} valueFrom: secretKeyRef: - name: {{ tpl $value.valueFrom.secretKeyRef.name $ }} - key: {{ tpl $value.valueFrom.secretKeyRef.key $ }} + name: {{ tpl $value.valueFrom.secretKeyRef.name $ | quote }} + key: {{ tpl $value.valueFrom.secretKeyRef.key $ | quote }} {{- else if $value.valueFrom.configMapKeyRef }} - name: {{ $value.name }} valueFrom: configMapKeyRef: - name: {{ tpl $value.valueFrom.configMapKeyRef.name $ }} - key: {{ tpl $value.valueFrom.configMapKeyRef.key $ }} + name: {{ tpl $value.valueFrom.configMapKeyRef.name $ | quote }} + key: {{ tpl $value.valueFrom.configMapKeyRef.key $ | quote }} {{- end }} {{- end }} {{- end }} diff --git a/paperless-ngx/templates/paperless.ingress.yaml b/paperless-ngx/templates/paperless.ingress.yaml index 33f6287..1270324 100644 --- a/paperless-ngx/templates/paperless.ingress.yaml +++ b/paperless-ngx/templates/paperless.ingress.yaml @@ -23,8 +23,16 @@ metadata: name: {{ .Release.Name }}-{{ .Chart.Name }}-ingress annotations: cert-manager.io/cluster-issuer: {{ required "A Cluster-Issuer must be provided" .Values.ingress.clusterIssuer }} - nginx.ingress.kubernetes.io/proxy-body-size: {{ .Values.paperless.files.maxUploadFileSize }} - nginx.org/client-max-body-size: {{ .Values.paperless.files.maxUploadFileSize }} + {{- /* + Quoted for the same reason as the env block in the StatefulSet and the + schedule in the CronJob (Gitea #11's genus): an annotation value must be a + string, maxUploadFileSize is free operator text, and a unit-less "100" - + schema-valid, since the schema types it as a string - renders bare as an + integer and the API server rejects the whole Ingress. The default "10m" + happens to be safe, which is exactly how this survived. + */}} + nginx.ingress.kubernetes.io/proxy-body-size: {{ .Values.paperless.files.maxUploadFileSize | quote }} + nginx.org/client-max-body-size: {{ .Values.paperless.files.maxUploadFileSize | quote }} {{- with .Values.ingress.annotations }} {{- toYaml . | nindent 4 }} {{- end }} diff --git a/paperless-ngx/values.schema.json b/paperless-ngx/values.schema.json index d5d2758..dffd273 100644 --- a/paperless-ngx/values.schema.json +++ b/paperless-ngx/values.schema.json @@ -69,9 +69,6 @@ }, "smbConnectionSecretRef": { "type": ["string", "null"] - }, - "oidcSecretRef": { - "type": ["string", "null"] } } }, @@ -279,13 +276,28 @@ "type": ["string", "null"] }, "cronJob": { - "description": "Cron schedule of the consume-mover job.", + "description": "Cron schedule of the consume-mover job. Deliberately without a pattern: cron syntax is richer than a regex here would admit, and the failure this value used to have was not a wrong schedule but a correct one rendered bare into the manifest - fixed at the render site with | quote (Gitea #11), not by narrowing what may be written.", "type": ["string", "null"] }, "cron": { "type": "object", "additionalProperties": false, "properties": { + "pinToServerNode": { + "description": "Place the consume-mover pod on the node that carries the StatefulSet pod, via a required podAffinity on its label (Gitea #101). Default true: the mover mounts the RWX consume volume, and Longhorn's CSI driver refuses that mount unless it finds a workload pod in Pending, so a mover on another node leaves the StatefulSet node's kubelet retrying every two minutes. false restores the unpinned behaviour and with it those mount errors.", + "type": ["boolean", "null"] + }, + "startingDeadlineSeconds": { + "description": "How late a missed run may still start. Also bounds the CronJob controller's missed-schedule count, which above 100 stops this job being scheduled at all, reported only in the controller's own log.", + "type": ["integer", "null"] + }, + "activeDeadlineSeconds": { + "description": "Hard stop for one mover run, counted from the job's start, so it also bounds a pod that never gets scheduled - with pinToServerNode on and no StatefulSet pod, the job fails with DeadlineExceeded instead of leaving a Pending pod per schedule.", + "type": ["integer", "null"] + }, + "env": { + "$ref": "#/definitions/env" + }, "securityContext": { "$ref": "#/definitions/securityContext" }, @@ -326,8 +338,9 @@ "type": ["string", "null"] }, "tokenAuthMethod": { - "description": "Explicit token endpoint auth method (e.g. client_secret_basic). Some providers need this since the allauth update in paperless-ngx 3.0.", - "type": ["string", "null"] + "description": "Explicit token endpoint auth method. Restricted to 'none' in 13.0.0 (Gitea #11): this chart registers a PUBLIC client with PKCE and configures no client secret, so a method that authenticates with one cannot work - it produced a login that died at the callback with invalid_client while reading like a working secret setup. null omits the key; 'none' states the public client explicitly, which some providers need spelled out since the allauth update in paperless-ngx 3.0.", + "type": ["string", "null"], + "enum": ["none", null] } } } diff --git a/paperless-ngx/values.yaml b/paperless-ngx/values.yaml index 431d6df..9282567 100644 --- a/paperless-ngx/values.yaml +++ b/paperless-ngx/values.yaml @@ -13,7 +13,26 @@ secrets: databaseSecretRef: null paperlessSecretRef: null smbConnectionSecretRef: null - oidcSecretRef: null + # REMOVED IN 13.0.0: secrets.oidcSecretRef (Gitea #11). It is gone from the + # schema too, so a release that still sets it now fails to render with + # additional properties 'oidcSecretRef' not allowed + # instead of being ignored - DELETE THE KEY from your values, nothing else is + # needed. It never did anything: no template read it, no OnePasswordItem was + # created for it, and whatever the item held never reached the pod. + # + # It is not coming back as a feature, and the reason is in paperless itself: + # the whole OIDC configuration travels in ONE json environment variable + # (SOCIALACCOUNT_PROVIDERS = json.loads(os.getenv( + # "PAPERLESS_SOCIALACCOUNT_PROVIDERS", "{}"))), and there is no _FILE variant + # of it. A secretKeyRef cannot substitute a fragment INTO a json variable, so + # an honest implementation would have to either inline the secret in the pod + # spec or push the complete json into the vault by hand - and the second one + # makes every required() in the oidc block below dead wood. + # + # The operational question behind it is answered rather than dodged: this + # chart registers a PUBLIC client with PKCE, Keycloak (Gitea #83) can mark a + # client public per client, and so no provider constraint forces a + # confidential client here. See the oidc block below. ingress: # Set to false to omit the Ingress entirely. NEW in the chart version that @@ -179,7 +198,38 @@ paperless: timeoutSeconds: 5 # Extra environment variables, appended to the ones the chart always sets. # Values are rendered through tpl; value, secretKeyRef and configMapKeyRef - # forms are supported. + # forms are supported. paperless-ngx reads around 80 PAPERLESS_* variables and + # this chart maps a handful of them explicitly - everything else goes here. + # + # env: + # - name: PAPERLESS_LOGGING_DIR + # value: /tmp/paperless-log + # + # ADDITIVE MEANS ADDITIVE, and the timing matters: an entry repeating a name + # the chart already sets reaches the pod twice, and at INSTALL the later one - + # yours - wins. On an EXISTING release the same entry is lost, because + # Kubernetes merges env by name and the pair collapses to the chart's own + # value, while helm upgrade still reports success. Read an override back off + # the StatefulSet, never off the upgrade's exit code. + # + # VALUES MUST BE STRINGS (the schema enforces it) and they are quoted on the + # way out since 13.0.0. Before that a value beginning with "*" aborted the + # render and "true" or "0755" arrived as a bool or an integer, which the API + # server rejects - see the comment at the render site in + # templates/paperless-server.sfs.yaml. + # + # THE WORKED EXAMPLE, PAPERLESS_LOGGING_DIR, AND WHAT IT DOES NOT FIX + # (jk-cluster-config #360): with the data directory on an RWX volume that + # Longhorn exports over NFS, paperless' concurrent_log_handler cannot hold its + # lock file there and fails with "RuntimeError: Cannot acquire lock after 20 + # attempts" - measured at 45 % of the application log across all three + # installations. Pointing PAPERLESS_LOGGING_DIR at a path under the /tmp + # emptyDir this pod already mounts takes the log off the NFS share and the + # noise stops. It does NOT fix the RWX data volume, which is the actual root + # cause and a cluster-config decision (the PVC asks for ReadWriteOnce and was + # provisioned ReadWriteMany; taking RWX back means recreating the volume). + # A log under /tmp does not survive a pod restart - no loss here, paperless + # also writes to stdout, which is what kubectl logs reads. env: [] resources: # Limits are optional: memory and ephemeral-storage limits default to @@ -214,8 +264,58 @@ paperless: smbConnection: enabled: false server: null + # Cron schedule of the consume mover. Quote it or not as you like - the chart + # quotes it on the way into the manifest (since 13.0.0, Gitea #11), which is + # what a schedule beginning with "*" or "@" needs. Before that such a + # schedule aborted the render with a YAML error pointing at the template. cronJob: "0 0,12 * * *" cron: + # Run the mover on the node that carries the StatefulSet pod (Gitea #101). + # + # WHY IT DEFAULTS TO TRUE: this pod 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 - a mover pod that ran on + # a different node leaves the kubelet of the StatefulSet's node retrying + # every two minutes with "no Pending workload pods for volume ... to be + # mounted" (291 events in 10 hours, in both releases with SMB on; the + # release without the CronJob produced none). + # + # It is a switch and not a free-form affinity block on purpose: the default + # has to name the StatefulSet pod's label, that label contains the RELEASE + # NAME, and a values.yaml is never templated - so the useful default cannot + # be expressed here at all (the same property that forces + # valkey.secrets.authSecretRef to be typed out twice, further down). + # + # WHAT IT COSTS, AND IT IS A REAL COST: the affinity is required, so with + # the StatefulSet pod absent the mover cannot be scheduled. The two ways + # that happens are handled rather than left open - scaleDown: true suspends + # this CronJob outright, and activeDeadlineSeconds below fails a job that + # cannot be placed instead of letting Pending pods pile up. What stays: with + # paperless.replicas: 0 and SMB on, the mover does not run and reports one + # failed job per schedule. Set this to false to get the old, unpinned + # behaviour back - including the mount errors. + pinToServerNode: true + # How late a missed run may still start. Also keeps the CronJob controller + # from reaching "too many missed start times (> 100)", after which it stops + # scheduling this job entirely and says so only in its own log - reachable + # by leaving scaleDown: true in place for months. The cost of a window: a + # run that could not start inside it is skipped, not queued. Must stay well + # above 10s, below which the controller may never schedule at all. + startingDeadlineSeconds: 600 + # Hard stop for one mover run, counted from the job's start - so it also + # bounds a pod that never gets scheduled (see pinToServerNode). Exceeding it + # shows up as a job with reason DeadlineExceeded, which is the point: a + # loud, self-clearing failure instead of a Pending pod that nobody sees. + # Raise it if a run legitimately moves more than an hour's worth of files; + # the mover copies across filesystems, so large scans are not instant. + activeDeadlineSeconds: 3600 + # Extra environment variables for the mover container, same form as + # paperless.env above. Deliberately empty: the container is a busybox + # running one mv, so this exists for the odd proxy or locale variable and + # for the convention that every workload container offers the block - not + # because the mover needs configuration. Note that the cron SCHEDULE's time + # zone is the controller's and cannot be changed with a TZ entry here. + env: [] # The consume mover only shuffles files between the SMB share (mounted # with 0777 modes) and the consume PVC, so it runs as the same non-root # uid/gid as paperless itself with a fully hardened context. @@ -251,9 +351,25 @@ oidc: name: null clientId: null discoveryUrl: null - # Optional: explicit token endpoint auth method (e.g. client_secret_basic). - # Some providers need this since the allauth update in paperless-ngx 3.0 - # (login fails at the callback with invalid_client otherwise). + # Optional: explicit token endpoint auth method. "none" is the only accepted + # value, and the schema enforces that (Gitea #11). + # + # WHY IT IS RESTRICTED, since the previous comment here named + # client_secret_basic as the example and that was actively misleading: this + # chart registers a PUBLIC client and configures NO client secret - it + # renders OAUTH_PKCE_ENABLED and nothing else, and secrets.oidcSecretRef was + # removed in 13.0.0 because no secret can reach paperless through a json + # environment variable anyway. A method that authenticates WITH a client + # secret therefore cannot work here; asking for one produced a login that + # died at the callback with invalid_client, while the two values read + # together looked like a working secret setup. + # + # So: null omits the key and lets allauth decide, "none" states the public + # client explicitly - which is what some providers need spelled out since + # the allauth update in paperless-ngx 3.0, and the symptom when they do is + # that same invalid_client at the callback. If a future paperless release + # supports an auth method that needs no secret, widening the enum is a minor + # bump; allowing one that needs a secret is not on the table. tokenAuthMethod: null # ---------------------------------------------------------------------------