OnePasswordItem references: required() for the four that are not optional #38
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: jlab-cluster/helm-charts#38
Reference in new issue
Block a user
Fixes #18
Four of the five unguarded
OnePasswordItemreferences were not optional, and now abort the render instead of producing anitemPathof null. The fifth, searxng's valkey reference, was already correct and is touched for documentation only.secrets.cwaSecretRefsecrets.minecraftServerRefsecrets.databaseSecretRef,secrets.oidcSecretRefMajors, because an installation that renders today - broken, but silently - stops rendering. Every
appVersionis unchanged and the+upsuffix recomputed from it.Rebased onto current
mainon 2026-09-26. Renovate had lifted urlaubsverwaltung twice while this branch was open (app 6.12.1 -> 6.13.0 -> 6.13.1, chart 9.0.3 -> 9.0.5), which collided inurlaubsverwaltung/Chart.yamland nowhere else. Resolved as the combination of both sides rather than either one:main'sappVersion: 6.13.1kept, so no app update is rolled back; this branch's major kept; the+upsuffix recomputed from the newappVersion, giving10.0.0+up6.13.1. The major stays at 10 becausemainnever left the 9 line. Every check below was re-run against the new base afterwards - the earlier byte-for-byte comparison was void the moment the rendered image tag moved.Read this before merging
A release that currently runs without one of these four references will fail its next
helm upgradeat render time. That is the point of the change, not a side effect, but it is worth one look at the live releases first. Nothing here was checked againstcluster-configbundles orhelm get values.The part that did not survive contact with Helm
The plan called for the key to be added to a
requiredarray in each chart'svalues.schema.json, on the assumption that Helm's coalescing deletes a null-valued key before schema validation and thereby lets the array fire. Measured on Helm 3.22.0 with a probe chart, that is not what happens for a key whose chart default isnull- which is all four of these:requiredfiresnullnullnull--set x=null/x: nullnull"set"--set x=nullvalues.yamlJSON Schema's
requiredtests presence, not null-ness, so for these keys the array could never fire under any input. Both ways of making it fire break the rule thathelm lint --strictpasses on chart defaults, since lint validates those defaults:nulldefault ->helm lint --strictfails,at '/secrets': missing property 'cwaSecretRef'(and for cwa and minecraft-server thesecretsobject then has no members at all and renders asnull);["string", "null"]to"string"->helm lint --strictfails,at '/secrets/cwaSecretRef': got null, want string.So the schema describes each key as required and names
required()as the enforcement, which is the shape searxng already uses for the opposite case. Arequiredarray next to it would be a safeguard that cannot fire, and this repo treats that as a defect rather than as documentation. This is also why no chart here has ever carried one.That finding is written into README guideline 12 in this PR, in two places: the bullet on how
nullreaches the schema was wrong in exactly the case it was being applied to (anullchart default is not coalesced away), and the general rule it implies - a value with no valid default cannot be enforced byvalues.schema.jsonwhile lint must pass on defaults - was missing. The wiki'sChart-Style-Guide§12 andLessons-Learnedcarry the same correction.What else is in here
A 1Password item reference must be provided (secrets.cwaSecretRef), the same forsecrets.minecraftServerRefplus the reason it is needed with RCON and CurseForge both off,A database Secret Reference must be provided (secrets.databaseSecretRef),An OIDC Secret Reference must be provided (secrets.oidcSecretRef).values.yaml: each of the four keys now opens withREQUIRED -and states the consequence, reusing the wording the charts' ownNOTES.txtalready carry rather than inventing new claims.NOTES.txthad a hand-written{{- if not .Values.secrets.cwaSecretRef }}warning for precisely the case the guard now prevents - unreachable after it, since the render aborts first. Gone, together with the(secrets.cwaSecretRef is unset - see below)fallback that pointed at it. minecraft-server's note stays true and now points at the guard instead; its| default "NOT SET"is likewise dropped.values.yamland in the two_helpers.tploccurrences. The same sentence survives in other charts' helpers and needs its own round.Verification, re-run against the rebased base
helm lint --strict: 0 failed for all four charts.ci/run-quantity-render-test.sh: 23 exercised, 0 failed.--set secrets.<key>=nullfails, the value set renders. Each urlaubsverwaltung guard verified in isolation, so neither hides behind the other. minecraft-server's guard fires fromminecraft-server.sfs.yaml:35rather than the secret template, because the StatefulSet'schecksum/secret-refincludes it first - same message.ci/<chart>/test-values.yamland diffed against the current base: all four byte-identical, withdocker.io/urlaubsverwaltung/urlaubsverwaltung:6.13.1on both sides. The four fixtures already set these references, so the install-test is unaffected.NOTES.txtfiles render without template errors.Not verified: there is no Docker daemon in this workspace, so no k3d cluster could be started and nothing was installed into a real cluster. The install-test stage of this PR is the runtime proof.
calibre-web-automated 7.0.0, minecraft-server 6.0.0 and urlaubsverwaltung 10.0.0 each majors: an installation that renders today with an empty itemPath stops rendering. searxng 1.0.6 is documentation only - its valkey reference is correctly optional and stays gated by {{- if }}. Every message names the values path. values.yaml now says per key why it is required, reusing the wording the charts' NOTES.txt already carry. The hand-rolled "cwaSecretRef IS UNSET" block in calibre-web-automated's NOTES.txt is gone: after the guard the render aborts first and that block is unreachable. minecraft-server's note stays true and now points at the guard. No "required" array in the schemas, and README guideline 12 now says why. The plan called for one; measuring it showed it could never fire: the key ships as null, so it is present at validation time, and JSON Schema's required tests presence, not null-ness. Making it fire means either dropping the null default or narrowing the type to "string", and both turn helm lint --strict red on the chart's own defaults. The schema therefore describes the key as required and required() enforces it. The README bullet on how null reaches the schema was also wrong in the case it was being applied to - a null chart default is not coalesced away - and now distinguishes the three cases. searxng: the claim "charts in this repo have no dependencies (README, guideline 13)" in the valkey template, in values.yaml and twice in _helpers.tpl has been false since #214 and cited the wrong guideline; dependencies are admissible and are guideline 14. The same sentence survives in other charts' helpers and needs its own round.6371f60eebto8074ed14a5