EN-11919: add chart lint, values schema, and CI tooling (1 of 2) - #155
Open
thefirstofthe300 wants to merge 7 commits into
Open
thefirstofthe300 wants to merge 7 commits into
thefirstofthe300 wants to merge 7 commits into
Conversation
thefirstofthe300
force-pushed
the
EN-11919/chart-tooling-workflow
branch
from
September 25, 2026 02:16
d8c2993 to
2009114
Compare
Makefile targets, pre-commit hooks, and a matrixed GitHub Actions workflow wiring helm lint, values-schema generation, and helm-docs. Neither chart is touched yet - no .schema.yaml or README.md.gotmpl exists, so the schema and docs targets are deliberate no-ops until the following steps land them. make setup installs only what is missing and never replaces a tool that is already present, warning instead when an installed version differs from the pin. The hook scripts warn on the same mismatch rather than failing, since a hard failure there would block commits for exactly the contributor that tolerance exists to accommodate. helm plugin install is not portable across Helm majors - Helm 4 refuses an unverifiable source without --verify=false and Helm 3 has no such flag - so install-schema-plugin.sh probes for the flag rather than parsing a version. The lint and schema jobs are matrixed over both majors with fail-fast disabled, and assert-helm-version.sh fails loudly when a binary is missing rather than silently testing one major twice. The lint override values deliberately use non-PEM placeholders: the chart base64-encodes that material without parsing it, so a placeholder exercises the same render path without putting a private key in a public repository. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L3TrXY8bWRGNv3aRFZZwGZ
Adds a .schema.yaml per chart and the generated values.schema.json, giving chart consumers real values validation at helm lint/template/install time instead of a malformed value failing deep inside template rendering or producing an invalid object that only fails at apply. Thirteen inline @Schema annotations correct types the generator cannot infer from a default: blank sentinels that would otherwise be typed null-only and reject any real value, and both maxUnavailable fields, which Kubernetes treats as IntOrString but which infer from whichever literal each workload happens to default to. gremlin.podSecurity.seLinuxOptions cannot be inferred at all - it has no live default keys - so it refs a hand-written fragment. Both the fragment's nullable type and the annotation's sibling type are load-bearing, each for a different Helm major: Helm 3 follows draft-07 and discards keywords beside a $ref, Helm 4 applies them. Dropping either half breaks the chart's own documented BottleRocket configuration on one major while staying green on the other. The two terminationGracePeriodSeconds cases asserting that a string renders literally are superseded - the schema now rejects that input before rendering - and are rewritten to assert the rejection, alongside the existing cases proving a valid integer is accepted, so the field is covered on both sides of the schema boundary. They match on the value path only, since Helm 3 and 4 word the rejection differently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L3TrXY8bWRGNv3aRFZZwGZ
This repository is public-facing, and a reference to an internal design document is meaningless to a reader outside the team and rots as soon as that document moves. Restates each rationale directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L3TrXY8bWRGNv3aRFZZwGZ
Agent workflows default to committing their contract, design and validation records as a shared audit trail. That default suits a private repository and not this one: those documents are dense with Jira keys, internal tooling paths and rationale written for people inside the team - precisely what this repo's existing convention against internal references exists to keep out. Gitignores the directory and records the rule, including the reasoning, so the default is not silently reapplied later. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L3TrXY8bWRGNv3aRFZZwGZ
Five value shapes rendered before this branch and were rejected after it.
Each hard-fails an external operator's next upgrade rather than warning, and
each is now accepted again, verified against both Helm majors:
image.tag / chaoimage.tag an unquoted tag like 1.2 or 3 is a YAML number,
and both charts typed it string-only
maxSurge Kubernetes treats it as IntOrString, and the
adjacent maxUnavailable was already annotated
for exactly that reason
replicaCount a quoted "2"
terminationGracePeriodSeconds a quoted "45", on both workloads
The grace-period keys take a digits-only pattern alongside the widened type,
so tolerating a quoted number does not reopen the multiline pod-spec
injection the rewritten unit tests exist to close - that payload and a
non-numeric string are both still rejected on both majors.
The common thread is that the annotations were derived from which defaults
are blank, and none asked whether a user might supply a different YAML scalar
type than the default. That is the question the schema now answers.
Also fixes defects found in the same review:
make test returned only the last chart's exit code, so a failing suite
passed if a later one succeeded; the drift check used git diff, which
cannot see an untracked generated file, so a new chart's schema would be
generated, never committed, and reported clean; the CI jobs hardcoded two
chart paths where the Makefile derives them; assert-helm-version.sh died
silently under set -e before printing the diagnostic that is its entire
purpose, and never checked its second binary; and setup.sh could not
complete on an ordinary Linux box - no writable-directory check, no
aarch64 mapping, a leaked temp dir, and a pip invocation that aborts under
PEP 668.
The helm-docs download is now checksummed against upstream's published
digests before extraction, since a version tag pins a name rather than
content and the binary it installs runs on every commit. Third-party actions
are pinned to commit SHAs for the same reason, and make setup now says out
loud that it installs a plugin with signature verification disabled, which
was previously visible only in a source comment.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L3TrXY8bWRGNv3aRFZZwGZ
The plugin install disables Helm 4's signature verification because upstream publishes nothing signed to verify against. That is defensible, but it was visible only in a source comment - someone running make setup saw a bland "Installing..." line and no indication that third-party code was arriving from a mutable reference with verification off. It now says so where the decision is made, and the README explains why. pre-commit was the one member of the toolchain the README described as pinned that was installed unpinned, so the claim is now true rather than aspirational. Also stops checkout persisting its credentials in .git/config; no job here pushes anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L3TrXY8bWRGNv3aRFZZwGZ
The probes an agent workflow generates to grade its own work belong with the contract they serve, not in this repository. They grade one piece of work against one agreement: nothing here invokes them, they need two Helm majors on PATH to run at all, and their headers are written in the contract's vocabulary rather than in terms a reader of this repository would recognise. Gitignores them alongside the contract and records the reasoning, so the default of committing them is not silently reapplied later. What this repository does run is unaffected: the helm unittest suites under each chart's tests/ directory, which CI executes on every pull request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L3TrXY8bWRGNv3aRFZZwGZ
thefirstofthe300
force-pushed
the
EN-11919/chart-tooling-workflow
branch
from
September 25, 2026 02:33
f0c2754 to
e6cff87
Compare
sirged
approved these changes
Sep 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds automated linting and values-schema validation to both charts, plus the scaffolding the docs generation in PR 2 builds on. Follows the pattern already used in
helm-platform— adapted rather than copied, since that repo's charts sit two directory levels deep, it has nomake setup, and it enforces sync through pre-commit alone with no CI job.PR 1 of 2. This lands tooling and schemas; #2 lands the generated README values tables. The split falls here on purpose: the ~40 hand-written schema lines below carry the entire consumer-facing regression risk, and reviewing them alongside ~1,550 lines of mechanical description comments would mean reviewing both at the same attention level.
What changes for chart consumers
Both charts now ship a
values.schema.json, so Helm validates values atlint/template/installand rejects a malformed value with its path named, instead of failing deep inside template rendering or producing an invalid object that only fails at apply.That is a real behavior change for people outside this repo, so the schemas are deliberately permissive:
additionalProperties: falseandrequiredare not emitted at either schema root. Thirteen inline@schemaannotations correct types the generator cannot infer — blank sentinels that would otherwise be typed null-only and reject any real value, and bothmaxUnavailablefields, which Kubernetes treats as IntOrString but which infer from whichever literal each workload happens to default to.Without those annotations a naively generated schema rejects seven configurations this chart's own README documents as valid, including
--set-file ssl.certFileand the BottleRocketseLinuxOptionsexample.Why CI runs two Helm versions
lintandschemaare matrixed over Helm 3.17.0 and 4.3.0 withfail-fast: false, because the two validate schemas with different JSON Schema dialects. Helm 3 follows draft-07, where keywords sitting beside a$refare ignored; Helm 4 applies them.gremlin.podSecurity.seLinuxOptionshas no live default keys to infer a shape from, so it references a hand-written fragment. Measured across both majors:type: []Each wrong variant is fully green on the major it does not break. That is why the matrix is required rather than nice to have, and why
hooks/assert-helm-version.shfails loudly when a Helm binary is missing rather than silently testing one major twice — a half-running matrix is worse than none, because it manufactures the appearance of coverage.helm plugin installis also not portable: Helm 4 refuses an unverifiable source without--verify=false, and Helm 3.17 has no such flag.hooks/install-schema-plugin.shprobes--helpfor the flag rather than parsing a version, which degrades sanely on a future major in either direction.Local workflow
make setupinstalls any missing tooling at its pinned version, and never replaces or upgrades a tool you already have, at any version. It warns instead — naming the tool, the version found and the version expected — so a later drift failure is debuggable rather than mysterious. The pre-commit hooks warn on the same mismatch rather than failing, since a hard failure there would block commits for exactly the contributor that tolerance exists for.make lint/schema/docs/check/testall run the same scripts CI runs, so local and CI cannot drift apart by construction.Two Helm behaviors worth knowing
helm lint --strictdoes not propagate a chart'sfail()call — it logs it and reports success. It is therefore not a backstop for the mutual-exclusion checks in_helpers.tpl; onlyhelm template,installorunittestsurface those.helm-docswith noREADME.md.gotmplpresent does not no-op — it falls back to its own default template and overwrites hand-written READMEs wholesale.hooks/helm-docs.shgenerates only for charts that have a template, which is what lets this PR ship the tooling without touching either README.Verification
helm unittest: 253/253 across both charts. The twoterminationGracePeriodSecondscases asserting that a string renders literally are superseded — the schema rejects that input before rendering — and now assert the rejection, alongside the existing cases proving a valid integer is accepted, so the field is covered on both sides of the schema boundary. They match on the value path only, since Helm 3 and 4 word the rejection differently.helm installexamples, and all thirteen annotated paths — renders under both Helm majors, positively and negatively.helm lint --strictpasses on both charts, with and without theci/linting/*.yamloverrides, on both majors.tests/contract/holds executable checks for the above, runnable directly. They need a Helm 3 and a Helm 4 binary and will refuse to run with only one rather than silently testing a single major.For whoever merges
The six matrix-expanded checks need adding to the
masterbranch protection rule, and marking only thev3.17.0legs required would silently void the point — the 4.3 leg would run, go red, and merge anyway. Check names embed the matrix values, so bumping a pin renames a required check and the rule must be updated in the same change. This is a configuration step invisible in the diff, which makes it the easiest thing here to get wrong.The
cert/keyvalues inci/linting/*.yamlare deliberately not real PEM — the chart base64-encodes that material without parsing it, so a placeholder exercises the same render path without putting a private key in a public repository.🤖 Generated with Claude Code
https://claude.ai/code/session_01L3TrXY8bWRGNv3aRFZZwGZ