feat(paperless-ngx)!: quote every operator scalar, pin the consume mover, drop oidcSecretRef #131

Merged
jklein merged 2 commits from feat/paperless-ngx-scalars-pinning-oidc into main 2026-10-13 21:29:32 +02:00
2 Commits
Author SHA1 Message Date
jklein 39844fc92c ci: hold every scalar that must be a string, and the required podAffinity
Validate Helm Charts / Detect changed charts (pull_request) Successful in 4s
Validate Helm Charts / HTTPRoute match (pull_request) Successful in 15s
Validate Helm Charts / Build paperless-ngx (pull_request) Successful in 9s
Validate Helm Charts / Resource quantities (pull_request) Successful in 15s
Validate Helm Charts / Selector match (pull_request) Successful in 14s
Validate Helm Charts / Bump level (pull_request) Successful in 7s
Validate Helm Charts / YAML scalars (pull_request) Successful in 15s
Validate Helm Charts / Trivy config paperless-ngx (pull_request) Successful in 29s
Install-test / Install-test paperless-ngx (pull_request) Successful in 5m11s
Validate Helm Charts / Install-test (pull_request) Successful in 5m11s
Publish Helm Chart with patched Application Version / publish-helm-chart (push) Successful in 16s
Publish Paperless-NGX Helm Chart / Publish Paperless-NGX Helm Chart (push) Successful in 16s
SonarQube Analysis / SonarQube Analysis (push) Successful in 47s
The test Gitea #11 asked for ("whoever fixes this ships a test case that would
have caught it"), plus the guard for the fix #101 introduces.

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

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

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

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

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

Part of #11
Part of #101
2026-10-13 19:10:26 +00:00
jklein 4b2bde505d feat(paperless-ngx)!: quote every operator scalar, pin the consume mover, drop oidcSecretRef
Three issues on one chart, bundled because they share values.yaml,
values.schema.json and the version line, and because the first half of #11
lives in the same template as #101.

Gitea #11, first half: the consume CronJob's schedule was rendered bare, so a
schedule in the commonest cron notation there is - a star, a slash, an interval
- aborted the render with a YAML parse error pointing at the template while the
operator's value appeared nowhere in the message. Quoting it in the values file
does not help; Helm passes the content through. Reproduced before the fix, at
the line the issue names, and fixed with | quote.

Gitea #11, second half: secrets.oidcSecretRef is removed, values and schema
both. No template ever read it, so no OnePasswordItem was created and nothing
reached the pod - the quiet kind of defect that looks like successful
configuration. It is not reimplemented, and the reason is in paperless: the
whole OIDC configuration travels in one json environment variable with no _FILE
variant, so a secretKeyRef cannot substitute a fragment into it. The
operational question is answered by #83 - Keycloak marks a client public per
client, so nothing forces a confidential one here.

In the same breath, oidc.provider.tokenAuthMethod is restricted to "none". Its
comment named client_secret_basic as the example, which asks the provider for
an authentication this chart cannot perform: it configures no client secret and
cannot. The two values read together looked like a working secret setup, and the
symptom was a login dying at the callback with invalid_client. No live release
sets it (measured against all three instances).

Gitea #112 asked for an extraEnv and the chart already had the standard
additive env block - the issue was measured against a deployed 12.2.0 and read
only the top-level keys. What was actually broken is the block itself: it
rendered its operand bare, so a "*"-leading value aborted the render and "true"
or "0755" arrived as a bool or an integer, which the API server rejects after a
green render. It is brought to the repository's documented quoted form
(template-chart carries the rule), which is what makes an arbitrary PAPERLESS_*
variable settable in practice. PAPERLESS_LOGGING_DIR is documented as the
worked example, with the cross-reference it needs: the root cause is the
NFS-exported RWX data volume in jk-cluster-config #360 and this does not fix it
- it only moves the log off that volume. The two Ingress upload-size
annotations had the same bare render and are quoted too.

Gitea #101: the consume mover now runs on the node of the StatefulSet pod, via
a required podAffinity on its label. It 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 left the StatefulSet node's kubelet
retrying every two minutes - 291 events in 10 hours in both releases with SMB
on, none in the release without the CronJob. A static nodeSelector would be
wrong because the StatefulSet pod is not pinned either, and preferred would
reproduce the bug more rarely and harder to find.

The cost of a required term is handled rather than left open:
concurrencyPolicy: Forbid, a startingDeadlineSeconds that also keeps the
controller away from its 100-missed-schedules stop, and an activeDeadlineSeconds
that fails a job which cannot be placed instead of leaving one Pending pod per
schedule. scaleDown already suspends the CronJob; what remains is
paperless.replicas: 0 with SMB on, which now reports a bounded, visible failure.
successfulJobsHistoryLimit is deliberately untouched: Longhorn's check asks for
the existence of a Pending pod, not the absence of Succeeded ones, so those
three pods are the error message's evidence and not its cause.

Side defect fixed on the way: every read of smbConnection.cron dereferenced the
block directly, so an override that had erased it aborted with a nil-pointer
error naming an internal template path instead of falling back to the defaults
this chart promises everywhere else. Measured on origin/main; all reads now go
through a merged-defaults helper that preserves an explicit false.

Major: a value is removed and fails the strict schema afterwards, and runtime
placement changes.

Fixes #11
Fixes #101
Fixes #112
2026-10-13 19:09:45 +00:00