fix(security): repair the flux-operator post-renderer so its security fields apply - #3580
Conversation
…lds apply #3577 placed both fields in one JSON-6902 patch. The pod-level op targets /spec/template/spec/securityContext/fsGroupChangePolicy, but flux-operator 0.50.0 renders no pod-level securityContext at all, so the op fails `doc is missing path` — and a JSON patch is all-or-nothing, so it took the container ops down with it. Neither field landed, and the HelmRelease has been unable to run an upgrade since (helm-controller 2026-09-04T10:41:27.775Z), while latching observedPostRenderersDigest and reporting Ready=True. The `{}` the previous comment cited was read from the live Deployment, where the API server serialises the absent object; the rendered chart is the surface a post-renderer patches, and there it is absent. Splits the pod-level field into a strategic-merge patch, which creates the missing parent and merges into a chart-set one — so a future chart that starts rendering it keeps its fields, which a whole-object `add` would have discarded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…post-renderer repair The repair changes the flux-operator HelmRelease's rendered text, and that HelmRelease is a selected authorization-capable document, so the aggregate surface digest moves — the tripwire working as designed, not a privilege change. Conservation re-proven with one renderer on both sides: all five overlays from main d7613b0 and from this branch, compared over the full apiVersion|kind|namespace|name identity in both directions. 552 documents each side, zero added, zero removed; a negative control that drops one document reports removed=1 and names it. Four of five roots are byte-identical and the whole delta is 3 lines removed / 13 added inside one postRenderers block. Digest taken from CI run 33866934697, which pins kubectl v1.36.2; that run reported this fingerprint as its only error class. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai review |
Readiness record. Programmatically tested. RED: the pre-fix patch reproduces helm-controller's exact failure through Control on the design choice, not just the outcome. Against a fixture representing a future chart Evidence for the diagnosis itself. Blast radius measured, not assumed. The four FluxInstance controllers from #3544 do carry both Validation. Authorization digest. Conservation proven with one renderer on both sides across all five overlays: Tried and evaluated as a user — with a caveat I want to be explicit about. The failure this fixes |
|
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (2)
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)Never run a cluster Put a change in the layer that matches its scope📄 CodeRabbit inference engine (AGENTS.md) Files:
🧠 Learnings (1)📚 Learning: 2026-08-11T12:41:28.242ZApplied to files:
🔇 Additional comments (3)
📝 WalkthroughWalkthroughThe Flux operator HelmRelease post-renderer now applies the pod-level Merge Risk: ⚪ Minimal · up to This corrects Flux Operator rendering so the pod security setting is applied without preventing container security settings from rendering. The validated change preserves existing security fields and does not alter the authorization resource surface. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Post-merge verification on the live cluster — the check whose absence let #3577 look successful. Merged 12:02:12Z; Read from PROD at 12:03:03Z:
Both fields are present, a genuinely new Helm revision exists, and the pod rebuilt and became ready — Worth stating plainly, because it is the reason this PR exists: the previous attempt merged green on |
1 participant
Why
The change that shipped in #3577 does not work, and it left the Flux Operator's release unable to
reconcile. The helm-controller has been reporting the operator as healthy the whole time, so nothing
surfaced it.
The intent was right: give the Flux Operator two security fields it was missing. But one of the two
was written in a form that only works if the chart already provides a place to put it, and this chart
does not. Because that style of edit is all-or-nothing, the one field that could not be placed took
the other one down with it — so neither landed, and every attempt to update the operator since has
failed the same way.
Left alone this gets worse rather than staying still: the next routine version bump of the operator
would fail to apply for the same reason.
What
Splits the two fields into the two forms the chart actually needs — the one that has a place to go
keeps the precise form, and the one that does not now uses a form that creates its own place. That
also makes it safe if a future chart version starts setting these itself, which the previous approach
would have silently overwritten.
Confirmed by reproducing the exact failure, applying the fix, and checking the result against the
real chart rather than against the running cluster — reading the cluster is what produced the wrong
conclusion the first time.
Part of #3239