Uh oh!
There was an error while loading. Please reload this page.
fix(chart): close the CLIENT_ENV + channelTags vocabularies, and correct a false schema doc - #695
Conversation
…ect a false schema doc Three findings from a mutable-tags sweep, all in the same place: the chart documented a vocabulary it never enforced, and in one case documented a fallback it does not have. Follow-up to backend#1723, which fixed the resolution and left the validation open. 1. env.CLIENT_ENV had no `enum`. tracebloc.clientEnv normalized three aliases and passed anything else through RAW. `CLIENT_ENV: prd` rendered jobs-manager:prd, pods-monitor:prd and resource-monitor:prd — tags no producer publishes — missed images.ingestor.channelTags, missed serviceDbAccountsByEnv, AND silently dropped the prod digest pin, which applies only where the env resolves to exactly "prod". Load-bearing in four places, validated in none. The only validator was client-runtime jobs_manager.py's sys.exit(1) on "Unknown CLIENT_ENV" — inside the container that cannot start. Now closed by an `enum` (the primary gate) plus a `fail` in tracebloc.clientEnv (the backstop, for --skip-schema-validation and any repackaging without the schema). 2. channelTags accepted arbitrary keys. `channelTags.staging: 0.7` on a staging edge validated fine and was then ignored — CLIENT_ENV=staging normalizes to stg and the lookup reads channelTags.stg — while `channelTags.stg: 0.7` took effect. The same word, normalized in one place and meaningless one key over. `additionalProperties: false`. 3. The mysql-client tag description said "Empty falls back to env.CLIENT_ENV". The template is `| default "prod"`, and dev/stg/prod all render :prod. tracebloc/mysql-client publishes 8.0, 8.4, four 8.4-<sha> builds, latest and prod — no dev, no stg — so a reader who believed it and set an env-derived tag would pull a nonexistent image AND disarm the mysql-format-guard, which reads "unknown" for an unrecognized tag. Note values.yaml already stated this correctly; the schema — the copy Helm shows customers — held the false half. Description corrected; the template is right and is left alone. BREAKING for any edge deploying an out-of-vocabulary CLIENT_ENV. Such an edge is already broken — it is pulling tags that do not exist — but it now fails at `helm upgrade` rather than at pod start. See the PR body. Tests: the gates cannot be asserted from helm-unittest, which treats a schema violation as a plugin-level error rather than a template failure and offers no way to skip validation and reach the `fail`. So they are exercised from scripts/tests/chart-env-vocabulary.sh (28 checks, every rejection paired with an accept control on the same command line), wired into `make check` and the helm-ci lint job. Five helm-unittest cases pin the mysql-client behaviour the corrected description now describes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
… hiding it in a word list Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 12, 2026
Two notes on this PR's CI, both worth knowing before you read the red marks. 1. The red checks are a GitHub release-CDN incident, not this change. Every failure is a download step, and I confirmed it per-job rather than assuming:
Every check that actually exercises this diff is green: Helm lint (which now runs the new vocabulary step), Helm unit tests, Template render aks/bm/eks, Lint, Source-of-truth drift, quality/shellcheck, quality/house-rules, and the chart version bump gate. I re-ran the failed jobs once; they re-failed on the same download steps, so it needs a re-run when the CDN recovers rather than a code change. 2. Adding a file under I kept the script there deliberately: chart tooling in this repo already lives in |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka
commented
Aug 12, 2026
Independently reproduced, and it compounds with backend#1853Rendered the real chart on So a one-letter typo — The part that raises the severityI filed backend#1853 earlier today: the Put the two together:
Until today that same typo landed on 0.8.4, which still had the fallback, so it degraded quietly instead of failing. The window where this is merely latent closed with the 0.8.8 release. Worth reflecting in the PR body — this is no longer only a "renders an unpublished tag" problem. On my own verificationMy first two render attempts reported "no Recording that because it is the same shape as the bug class this whole epic is about: a check that passes (or in this case, "finds nothing") because it was never connected to anything. The fix was to stop suppressing stderr and to fail loudly on a non-render, which is what the new Verified separately
|
…d#1723) - chart-env-vocabulary.sh: capture helm --help then match, so grep -q cannot SIGPIPE helm under pipefail and misread the flag as absent (the capture-then-match rule already used for the usermod help probe). - helm-ci.yaml: add scripts/tests/chart-env-vocabulary.sh to the push + pull_request paths filters, so a PR touching only that script still runs the gate it owns. - installer-tests.yaml: add the script to the static shellcheck set (error + warning), re-syncing CI with the Makefile SHELLCHECK_FILES.
LukasWodka
commented
Aug 12, 2026
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8c33546. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
shujaatTracebloc
left a comment
There was a problem hiding this comment.
Approving. Verified all four Bugbot findings rather than trusting the resolve — three are genuinely fixed, and I'm accepting the fourth's deferral.
Bugbot fixes, checked in the tree
- Help probe under pipefail → now capture-then-match:
_helm_help="$(helm template --help 2>&1 || true)"thengrep -q <<<"$_helm_help". No pipe, so no SIGPIPE misbranch. Fixed ✓ - Vocabulary test missing from CI paths → added to both the
pushandpull_requestfilters, not just one ✓ - Shellcheck list drift →
chart-env-vocabulary.shadded to the Makefile'sSHELLCHECK_FILESand to both invocations ininstaller-tests.yaml(error and advisory). Both sides in sync ✓ - Fallback test cannot prove path → resolved as intended, not fixed. I checked the claim it rests on and it holds:
client/tests/ingestor_channel_tag_test.yaml:93really does have aimages.ingestor.channelTags: nullcase, so the fallback-fired path has independent coverage. Combined with helm-unittest's deep-merge making "one entry absent, others present" inexpressible, strengthening theprodcase would couple it to #694's merge order for no new coverage. Reasonable trade — not reopening.
Two things to handle at merge time
The chart version assumes #694 lands first.develop is at 1.9.34; #694 → 1.9.35, this → 1.9.36. That only works in that order. If this merges first, #694 hits a conflict on the same version/appVersion lines and has to re-bump to .37 — and the two PRs share three files (Chart.yaml, _helpers.tpl, ingestor_channel_tag_test.yaml), so the second one through needs a rebase regardless. Worth flagging because #694 is on a different reviewer, so nobody is watching the ordering from both sides.
additionalProperties: false on channelTags is a breaking chart change. The chart's own values.yaml only uses dev/stg/prod, so nothing here breaks itself ✓ — but the PR's own rationale is that channelTags.staging: "0.8.2""validated fine and was then ignored", which means it may exist in a live edge's values. Those edges now fail at helm upgrade rather than silently ignoring the key. That's the correct outcome and the point of the change, but it deserves a line in Deployment notes so a rollout doesn't discover it by surprise.
On the design
Putting the enum in values.schema.jsonand a fail in tracebloc.clientEnv is right, and the reason given is the part that matters: the enum only binds where the packaged schema is read, and --skip-schema-validation or a repackage without the schema walks past it. Since the helper is already the single chokepoint every consumer goes through, the backstop costs nothing and closes the hole the enum can't.
The justification for fail over pass-through is the strongest argument in the PR — an unrecognised value wasn't graceful degradation, it silently reconfigured the edge in four places at once (three control-plane tags at unpublished names, a missed channelTags lookup, a missed serviceDbAccountsByEnv lookup, and a dropped prod digest pin, since the pin only applies where the env resolves to exactly prod). And noting that the only existing validator was jobs_manager.py's sys.exit(1)inside the container that can't start is exactly why failing at helm upgrade is the right layer.
chart-env-vocabulary.sh earns being a shell test rather than a helm-unittest suite, and the reason is stated concretely (plugin 0.5.2 reports a schema violation as a plugin-level error, so failedTemplate can't assert it, and there's no flag to skip validation and reach the fail). Two details I'd have asked for and didn't have to:
- every reject paired with an accept control on the same command line, matched against specific error text rather than just non-zero exit — a rejection test that failed to render for an unrelated reason would otherwise pass for free
- the
--skip-schema-validationgate self-skips on the pinned helm 3.15.4 and turns on for anyone on 3.16+, with the skip announced rather than silent, so bumping the pin activates it with no edit here
BAD_ENVS choosing prd, Prod and develop is well-judged — an abbreviation, a capitalisation, and an alias one letter off development are what people actually type. Keeping the whitespace-only case out of the array so the tab survives word-splitting is a nice touch.
The mysql_test.yaml additions are the sleeper value here: the schema said "Empty falls back to env.CLIENT_ENV" when the template is | default "prod", and the five new cases pin why that doc had teeth beyond a failed pull — an env-derived tag is unrecognised by tracebloc.mysqlEngineMajor, so the mysql-format-guard stands down at unknown instead of staying armed at 5.7. Asserting the believing-the-old-doc case and the real-fallback control is what makes that provable rather than asserted.
Summary
Three findings from a mutable-tags sweep, all the same shape: the chart
documented a vocabulary it never enforced, and in one case documented a
fallback it does not have. Follow-up to backend#1723, which fixed the
resolution and left the validation open.
Every claim below was verified by rendering the real chart, each with a control
proving the opposite input behaves differently. Helm
v4.1.1, valuesclient/ci/bm-values.yaml, baseorigin/develop@ead58c6.1.
env.CLIENT_ENVhad noenum— the important onetracebloc.clientEnvnormalized three aliases and passed anything elsethrough raw. Measured on
develop:Published tags, checked against Docker Hub 2026-08-12 — no
prd, noqa, noProd:The clearest demonstration is a one-letter typo.
developmentis an alias;developis not:Four things break at once, none of them loudly: a control-plane tag that does
not exist,
images.ingestor.channelTagsmissed,serviceDbAccountsByEnvmissed so no service DB account is provisioned, and the prod digest pin
dropped (it applies only where the env resolves to exactly
prod). That lastpair is backend#1752 reconstructed from a typo — and it matters more now that
the
0.8float has moved past theedgeuserceiling (backend#1853).The only validator that existed was
client-runtimejobs_manager.py'ssys.exit(1)on "Unknown CLIENT_ENV" — which lives inside the container thatcannot start, so it cannot help.
Fix: an
enuminvalues.schema.json(primary gate) plus afailintracebloc.clientEnv(backstop — the enum is only checked where the packagedschema is read, and
--skip-schema-validationskips it). Failing closed matchesthe chart's own conventions: it already
fails on placeholderclientId, emptytraining CIDRs, non-alphanumeric service passwords,
perDatasetPvcswithoutclusterScope, and a missing metrics API.Accepted vocabulary — the complete list:
devdevstgstgprodproddevelopmentdevstagingstgproductionprod""/ unsetprodEverything else is refused.
2.
channelTagsaccepted arbitrary keysProven with the control, on
develop:The lookup is on the resolved env, so only
dev/stg/prodcan evermatch. The same word "staging" is normalized in one place and meaningless one
key over — a customer writing the alias gets silence.
Fix:
additionalProperties: false.3. A schema description that was false
values.schema.jsonsaid the mysql-client tag "Empty falls back toenv.CLIENT_ENV". The template is
.Values.images.mysqlClient.tag | default "prod":Published tags (Docker Hub, 2026-08-12) — no
dev, nostg:So the doc was not merely imprecise, it was an instruction to break the edge.
And it costs more than a failed pull — an env-derived tag is unrecognized by
tracebloc.mysqlEngineMajor, so the mysql-format-guard silently standsdown:
Worth noting:
values.yamlalready said this correctly ("mysql-client isonly published under the
prodtag — it has no dev/staging variants, so wedecouple it from
env.CLIENT_ENV"). The two files contradicted each other andthe schema — the copy Helm shows customers — held the false half. Description
corrected; the template is right and is left alone.
Blast radius — the part that needs your call
Closing an open vocabulary is breaking for any edge currently deploying an
out-of-vocabulary value. Such an edge is already broken — it is pulling
control-plane tags that do not exist, running with no service DB account, and
has no prod digest pin — but the failure moves:
ImagePullBackOff, orsys.exit(1)inside the container)helm upgrade/helm install, with a message naming the accepted valuesThat second row is the real risk and the reason I am not merging this myself:
the auto-upgrade CronJob runs
helm upgrade --reset-then-reuse-values, so amisconfigured edge's next auto-upgrade would start failing. It would fail
visibly, which is the point — but it is a behaviour change on live edges.
I could not find such an edge from inside this repo: every
CLIENT_ENVvalue anywhere in the tree is one of the six accepted spellings (the only
exceptions were two test fixtures deliberately exercising the pass-through, both
handled here).
arm-canaryis a published image tag but is referenced nowherein the repo as a
CLIENT_ENV. Confirming no live edge is out of vocabulary isoutside what I can check — that needs a look at the actual installed values,
and it is the thing I would want checked before this merges.
If you'd rather not take that on now: findings 2 and 3 are non-breaking and
can ship on their own.
Tests
The gates cannot be asserted from helm-unittest — verified, not assumed: it
validates values against the packaged schema and reports a violation as a
plugin-level error rather than a template failure, so
failedTemplatecannot catch it, and it exposes no flag to skip validation and reach the
helper's
fail. So they are exercised fromscripts/tests/chart-env-vocabulary.sh, wired intomake checkand thehelm-cilint job. Every rejection is paired with an accept control on thesame command line, and matched against the specific error text rather than
merely a non-zero exit.
Mutation-checked — each guard is independently load-bearing:
MUT C is the one that matters: it shows the backstop case is not just
re-testing the enum.
make check(shell parse + shellcheck + 3 drift guards + helm lint, 5 value files):make helm-unittest— 5 new cases pin the mysql-client behaviour the correcteddescription now describes:
Mutation-checked too — someone "fixing" the template to match the old false doc
gets caught, and the prod control correctly stays green because prod is
indistinguishable either way:
make helm-template— all four platforms render;shellcheck -S warning -xonthe new script is clean.
Manifest:
scripts/gen-manifest.sh --checkpasses and needs noregeneration — the manifest is the installer bootstrap's integrity surface
(
scripts/install.sh's fetch list), and a CI-only test script is correctly notin it. Confirmed by reading the
FILESarray rather than assuming.Type
Notes for the reviewer
values.schema.jsonis chart contentper
scripts/chart-version-guard.sh; I read the guard to confirm).fallback per-environment. The two are independent; whichever lands second
will want a trivial
Chart.yamlrebase.CLIENT_ENV: produktion, asserting the pass-through)is replaced rather than re-pointed, because the input is now illegal. Its
replacement deliberately asserts on prod, whose fallback is the same on
this branch and on fix(chart): the ingestor tag fallback is per-environment, not always prod's #694, so it does not depend on merge order.
| default "0.8"tail in fix(chart): the ingestor tag fallback is per-environment, not always prod's #694's fallback becomesunreachable-by-construction. Left in as a belt-and-braces guard rather than
coupling the two PRs; say the word if you'd rather it go.
Note
High Risk
Closing
CLIENT_ENVis a breaking change: fleet auto-upgrade on edges with out-of-vocabulary values will fail athelm upgradeinstead of proceeding with a broken cluster.Overview
Closes undocumented open vocabularies on the client Helm chart so bad
env.CLIENT_ENVandimages.ingestor.channelTagskeys fail at install/upgrade instead of silently becoming wrong image tags, missed lookups, and dropped prod digest pins.values.schema.jsonadds aCLIENT_ENVenum (six spellings + empty) andchannelTagswithadditionalProperties: false.tracebloc.clientEnvadds afailbackstop when schema validation is skipped. The mysql-clienttagdescription is corrected to match the template: empty tag always falls back toprod, notCLIENT_ENV.scripts/tests/chart-env-vocabulary.shexercises both gates viahelm template(paired accept/reject controls); wired intomake check/helm-ciand shellcheck. Helm unittest cases are updated (illegalproduktionremoved; mysql empty-tag →:prodpinned). Chart version 1.9.34 → 1.9.36.Reviewed by Cursor Bugbot for commit 8c33546. Bugbot is set up for automated code reviews on this repo. Configure here.