fix(chart): harden Gateway API routing and fix fatal secrets-loop scoping - #126
Conversation
…ping
The gateway value schema already matched s9genericchart-v2 exactly (the
HTTPRoute template is byte-identical), so this changes no schema. It fixes
the four ways that schema could fail silently or fatally, which matters
because the mtm chart backs six live releases (funride, kwickbox and
tamweelna x staging/production) that are all about to move onto the shared
Cilium Gateway.
- httproute/ingress/NOTES: guard `.Values.gateway` and `.Values.ingress`
before dereferencing `.enabled`. Nulling either block raised a
nil-pointer error instead of simply rendering no route.
- httproute: normalise `backendRef` to a dict before use. A route carrying
an explicitly empty `backendRef:` — the exact shape produced by
commenting out the two children shipped in values.yaml — crashed with
"interface conversion: interface {} is nil".
- httproute: `path` is now `required`. It previously rendered `value:`
(null), producing an HTTPRoute that Helm accepted and the API server
rejected.
- httproute: emit `backendRef.weight` via `hasKey` rather than `with`, so
an explicit `weight: 0` is honoured instead of silently discarded.
- deployment: pass `$root` to project.fullname inside the `.Values.secrets`
range. `.` is rebound to the map value there, so ANY release that set
`secrets.*` failed to render with "can't evaluate field Values in type
string". `$root` was already captured for this and never used.
Verified: `helm lint --strict` clean; all four failures reproduced before
and confirmed fixed after; and rendering the chart against the live
user-supplied values of all six production/staging releases produces
byte-identical output, so no existing deployment changes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VHkpqv6dB6wjALyFe9ocMU
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: simplify9/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)Review Helm chart changes for insecure defaults, exposed services,⚙️ CodeRabbit configuration file Files:
🪛 LanguageToolcharts/default/templates/NOTES.txt[style] ~10-~10: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE) 🪛 YAMLlint (1.37.1)charts/default/templates/httproute.yaml[error] 1-1: syntax error: expected the node content, but found '-' (syntax) charts/default/templates/ingress.yaml[error] 1-1: syntax error: expected the node content, but found '-' (syntax) 🔇 Additional comments (4)
📝 WalkthroughWhat changed
Risk
The changes affect Helm rendering and Gateway API routing. Invalid or incomplete route values now fail validation instead of causing template errors or producing ambiguous resources. Security-sensitive areasNo direct security control changed. Gateway and Ingress rendering can affect external exposure. The Test coverage impact
Operational concerns
WalkthroughThe chart templates now guard optional routing values, validate required HTTPRoute fields, preserve explicit zero weights, and use the root context when resolving deployment secret names. ChangesHelm template hardening
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The chart now safely handles null or empty Gateway settings, validates required route names, preserves zero weights, and fixes secrets rendering; no actionable merge-blocking risk remains after normal checks and review. Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (4 skipped: 4 unsupported.) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
MusaMisto
added
bug
1 participant
Why
The
mtmchart is deployed six times across the estate —funride,kwickboxandtamweelna× staging/production — and all six are about to move onto the shared Cilium Gateway. Gateway API support was merged tomainin June, but the schema had four latent failure modes, two of them fatal and two silent.Worth stating up front: the gateway value schema already matches
s9genericchart-v2exactly —charts/default/templates/httproute.yamlwas byte-identical to that chart's, and thegateway:values block is semantically identical apart fromenabled: false(correct here, since this chart keeps its ingress path). No schema changes in this PR. It only hardens what was already there.What changed
gateway: null/ingress: nullbackendRef:interface conversion: interface {} is nilpathvalue:(null) — Helm accepts, API server rejectsrequiredbackendRef.weight: 0withtreats 0 as falsy)hasKeysecrets.*can't evaluate field Values in type string— total render failureThe last one is the most serious and is unrelated to Gateway API: inside
range $key, $values := .Values.secrets,.is rebound to the map value (a string), sotemplate "project.fullname" .blew up.$rootwas already captured on the line above for exactly this purpose and never used. It is masked today only because every caller passes the connection string through the top-leveldbvalue rather thansecrets.Verification
helm lint --strict— clean (only the pre-existingicon is recommendedINFO).mainfirst, then confirmed fixed.helm get valuesoutput of all six releases, plus chart defaults and gateway-enabled defaults — 8/8 byte-identical before vs after. No existing deployment changes behaviour.kubectl apply --dry-run=serveragainst the live SBN gateway cluster (k8s v1.36.3, HTTPRoutev1served and storage).Follow-up (not in this PR)
The published chart is the real blocker:
charts.sf9.iohasmtm-6.0.8(Aug 2025) and GHCR OCI has6.0.7— neither containshttproute.yaml, because the newest git tag (6.0.9, Feb 2026) predates the June gateway merge. Any deploy passinggateway.*against a published chart today silently renders an Ingress instead. Merging this PR triggers CI and publishes the first gateway-capable chart version.🤖 Generated with Claude Code
https://claude.ai/code/session_01VHkpqv6dB6wjALyFe9ocMU