feat(kustomize)!: read kustomization.yaml with kustomize, not a hand-written parser#229
Merged
Conversation
…written parser
The analyzer decoded kustomization.yaml with a hand-written walk over a generic
YAML map, and checked it against a hand-maintained list of 17 unsupported keys.
It now decodes with kustomize's own type and runs the same Unmarshal +
FixKustomization sequence kustomize's builder runs (internal/target: kusttarget.load),
and derives the unsupported set by reflecting over that type: anything not
explicitly modelled refuses the folder.
The dependency was never the cost we thought it was. sigs.k8s.io/kustomize/api
and kyaml were already in this module's requirement graph; taking them directly
at the version Flux ships adds zero new modules to the build.
The hand-written deny-list could only refuse what someone had remembered to add
to it, and it had holes. Five verdicts change, each verified against a real
`kustomize build`, and in every case the new behaviour is the one that agrees
with the renderer:
- vars was accepted, and $(VAR) in a source document was silently overwritten
with its substituted value on mirror. Now refused.
- validators (plugin code) was accepted, unmodelled. Now refused.
- A kustomization kustomize cannot decode — resources: as a string, or a file
that is really a Flux Kustomization CR — was accepted and written into.
Now refused, quoting kustomize's own decode error so the refusal is
actionable.
- imageTags was silently ignored (an override we never saw still changed the
render); bases was read by hand. Both are now normalised into
images/resources exactly as the builder does.
- A case-variant key (newtag:) and a blank optional component (newName: "")
refused the folder. kustomize honours both, so folders that render fine are
no longer refused.
Remote-base detection does NOT move to the library, and this is the load-bearing
exception: kustomize resolves a remote base by shelling out to `git fetch`, and
does so under LoadRestrictionsRootOnly and under an in-memory filesystem alike
(both measured). No build option turns it off. hasRemoteResource is therefore
promoted to a security precondition that runs before any build is invoked, which
is what keeps "the operator never fetches a remote base" true.
Net production code: -45 lines. The line count is not the point; the next change,
which replaces the re-implemented transformers with the real render, is where the
bulk goes.
Refs docs/design/support-boundary/kustomize-support-boundary.md §7
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe analyzer now decodes and normalizes ChangesKustomization support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Analyzer
participant KustomizeAPI
participant SupportGate
participant Worktree
Analyzer->>KustomizeAPI: Decode and normalize kustomization.yaml
KustomizeAPI-->>Analyzer: Kustomization or decode error
Analyzer->>SupportGate: Evaluate unsupported fields and remote resources
SupportGate-->>Analyzer: Accept or structural refusal
Analyzer->>Worktree: Flush changes only when accepted
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/manifestanalyzer/scan_repo.go (1)
449-456: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the string local to
errMsg
erris conventionally reserved forerrorvalues; this one holds a string, soerrMsgordecodeErris clearer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/manifestanalyzer/scan_repo.go` around lines 449 - 456, Rename the string local in the kustomization decode check to errMsg (or decodeErr) and update its use when appending the detail text; preserve the existing kustomizationDecodeError handling and output.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/manifestanalyzer/scan_repo.go`:
- Around line 449-456: Rename the string local in the kustomization decode check
to errMsg (or decodeErr) and update its use when appending the detail text;
preserve the existing kustomizationDecodeError handling and output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3194b807-a485-43cb-9457-42219021dbc8
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (14)
docs/UPGRADING.mddocs/design/support-boundary/finished/images-and-replicas-edit-through.mddocs/design/support-boundary/kustomize-support-boundary.mddocs/design/support-boundary/orchestrator-knowledge-boundary.mddocs/design/support-boundary/support-contract.mdgo.modinternal/git/placement_test.gointernal/manifestanalyzer/kustomization_parse.gointernal/manifestanalyzer/overrides.gointernal/manifestanalyzer/overrides_projection.gointernal/manifestanalyzer/overrides_test.gointernal/manifestanalyzer/scan_repo.gointernal/manifestanalyzer/store.gotest/fixtures/gitops-layouts/support-today.md
💤 Files with no reviewable changes (1)
- internal/manifestanalyzer/overrides.go
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
sunib
added a commit
that referenced
this pull request
Jul 14, 2026
… shipped Review feedback on #232, all of it fair, plus one thing it asked for. **Accuracy.** The docs were written before the renderer landed and several passages had gone stale in ways that mattered: - render-root-scoping said `sigs.k8s.io/kustomize` "is not a dependency of this module. Zero hits in go.mod". It is one, since #229 — that is the argument the doc *won*. Marked historical, and the inventory table now says which rows are gone (the 17-key deny-list, replaced by reflection over kustomize's struct) and which are still standing (renderImage, isReplicaKind — with the divergences they cause). - The sandbox table proposed `LoadRestrictionsRootOnly`. The shipped renderer uses `LoadRestrictionsNone`, deliberately, because RootOnly rejects `resources: [../shared.yaml]` which Flux renders happily — and failing to build a root silently disarms the write-fan-in guard. The contract as built is recorded rather than quietly corrected. - The doc claimed simulateImageRender "already does" propose-re-render-compare. It does not: it replays OUR re-implemented chain, not kustomize, over only the images it planned, and never checks that the rest of the build is untouched. It shares a blind spot with the thing it verifies, which is the whole reason it has to die. Said plainly. - values-file-projection conflated three different surfaces. Verified upstream: Argo's `helm.valueFiles` and Flux's `spec.chart.spec.valuesFiles` name a FILE; `valuesObject` is inline RawExtension in the Application; Flux's `valuesFrom` is a KRM object reference (`Secret/my-secret-values`). Only the first is a values file. Collapsing them would claim a file-projection capability over content living inside another document, or inside a Secret. **render-attribution**, tightened where the review was right: - "put a nonce in ANY knob" was an overclaim. Each new field costs a sink proof, and the fields worth reaching for next are the ones most likely to fail it: a replacements source is read by a selector, a generator literal feeds a name hash, a patch's containers[].name is a merge key. - "refuse and fall back" was ambiguous, and the review's instinct was sharper than it knew. Falling back to write-through is NOT harmless: writing a live value into a source document whose field an entry governs does not converge — the entry overrides it on the next render, which is the same non-convergence that convicts leave-one-out in §2. What makes the fallback safe is that the verification re-render CATCHES it. Fallback means no attribution, never another heuristic. **acceptance-precision** now contains only what is not implemented, as asked. `vars` is gone from it: #229 closed that hole, and closed it structurally — the unsupported set is derived by reflecting over kustomize's own struct, so a field we have never heard of refuses instead of being tolerated. Every remaining claim re-verified against the code (labels/commonLabels are still tolerated and still leak; Generated{path} is still emitted by nothing; the values file still cannot be rescued by .gittargetignore because *.yaml is a catastrophic pattern). Fenced blocks given language identifiers (MD040). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sunib
added a commit
that referenced
this pull request
Jul 14, 2026
…override chain from the renderer (#232) * fix(kustomize): refuse a folder kustomize cannot build, and read the override chain from the renderer Two changes, and the second is why the first matters. 1. The override chain — which kustomization's images:/replicas: entries govern a document — was reconstructed by a hand-written DFS over the resources graph, re-deriving kustomize's build order, its cycle rules and its diamond behaviour. It is now read off kustomize's own provenance: every rendered object carries alpha.config.kubernetes.io/transformations, the ordered list of which kustomization configured which builtin transformer. That IS the chain, handed to us by the thing that applies it. It is now attributed per OBJECT rather than per file, which closes a latent hazard: a Deployment and a Service sharing one file were given the same chain. 2. A folder whose render root kustomize cannot BUILD was being silently accepted, because the old parse never tried to build anything. Both of these passed the acceptance gate: - a resources: entry that does not resolve (a manifest moved or renamed); - a diamond — one render root reaching a shared base through two overlays — which kustomize rejects outright: "may not add resource with an already registered id". Neither folder can be deployed by Flux or Argo. And accepting them did not just look untidy, it DISARMED THE GUARD PROTECTING THEM: the override chain, and therefore the write-fan-in precondition, is derived from the render. A root that does not build yields no chain, so no ambiguity is recorded, so the fan-in guard never fires — and the operator would write straight through into a base shared by two render paths, which is the one edit fan-in = 1 exists to forbid. A render root that fails to build now refuses the GitTarget, quoting kustomize's own error. Three fixtures in our own test suite turned out to be unbuildable, which is how this was found. Also: LoadRestrictionsNone, which is what Flux itself builds with. The in-memory filesystem contains only the scanned tree, so the filesystem is the jail — precisely why Flux can use it safely. RootOnly was the wrong kind of strict: it refuses to load a FILE from outside the render root (resources: [../shared.yaml]) which Flux renders happily, so we would fail to build a root that deploys in production, see no chain for it, and quietly stop enforcing write-fan-in on the file it shares. Refusing to look is not a safety property. The write-fan-in tests seeded a diamond, which is now refused at the gate and no longer reaches the write-boundary preconditions they exist to test. They are re-pointed at two separate render roots sharing one base — a shape that does build and does disagree, which is exactly what the fan-in guard is for. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(kustomize): refuse a kustomization kustomize panics on, and build the roots that have none Two ways a folder could get past the refusal path that #232 introduced, both found by probing kustomize rather than by reading it. An images: entry's name: is a REGULAR EXPRESSION, and kustomize compiles it while discarding the compile error (api/internal/image/image.go) before dereferencing the nil *Regexp. So `- name: "ngin["` does not fail the build — it panics inside it, on bytes that came straight out of a user's repository. Nothing reaches krusty on main; #232 is what puts it on the store-build path, so this lands with it. controller-runtime recovers reconciler panics by default, which does not make it safe: it turns a crash into panic → requeue → panic, for as long as the repository stays as it is. The manifest-analyzer CLI has no such net at all. Refuse the entry before the build, and put a recover() under krusty for the panics we have not found yet: a build runs library code we do not own over bytes we do not control, so a panic there has to become a refused folder. A cycle (a → b → a) has NO render root — every directory in it is referenced by another — so renderRoots returned nothing for it, renderChains built nothing, and no failure was recorded. That is exactly the hole the refusal exists to close: no build means no chain, no chain means no ambiguity, and no ambiguity means the write-fan-in guard never fires on a folder kustomize cannot build at all. Cover every kustomization with some build attempt and let kustomize give the verdict; it says "cycle detected", and Flux would say the same. Also, from the same probing: kustomize builds one ImageTagTransformer PER images: ENTRY and stamps them all with the same origin, so a kustomization with three entries leaves three byte-identical transformation records on every object in the build — and chainOf added that file's whole entry list once per record. Dedupe the records. The annotation's comments said kustomize records "the transformers that touched each resource"; it does not, and it cannot: it records every transformer that RAN, against every object in the build, modified or not (api/resmap/reswrangler.go loops the whole ResMap with no diff check). Say what it does. Review feedback from #232, all addressed: sort render-failure diagnostics (map iteration was nondeterministic), tell users about build failures in the Unsupported godoc and in the refusal message, and compute renderChains once per fixture in the differential test instead of once per rendered object. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(support-boundary): the render-attribution design, and the three docs it argues from The Go comment in kustomize_render_hostile_test.go points at render-attribution.md, so doccheck refuses the reference until the doc is tracked — and the doc links to the three design docs it builds on, so they land together or not at all. render-attribution.md is the design for the last step of the kustomize-renderer workstream: deleting the re-implemented transformers the write path still uses to decide which file an edit belongs in. Its argument, in short: - kustomize has NO field-level provenance, at any visibility level, and no DAG object even internally — accumulation is control flow, never a retained structure. So attribution ("which images: entry supplied this tag?") cannot be READ out of the renderer. It can only be inferred by questioning it, and every viable design is a query strategy. - Leave-one-out probing (re-render with one entry removed; whatever moves is what it supplied) is measured WRONG, and would regress today's code: removal probes the VALUE, and values collide. It is blind whenever an overlay pins the tag the base already has — the steady state of a GitOps repo — and blind on ties. - Dyeing every entry with a distinguishable nonce and rendering ONCE is the same idea with the flaw removed: it resolves ties, costs 2 builds instead of 1+N, and is the only field-level provenance available for kustomize at any price. - Attribution may be heuristic. Verification may not — which is why the real re-render replaces simulateImageRender first, alone. §6 is the bug ledger the measuring produced, and it is the justification for the deletion: our matcher is string equality where kustomize's is a regex over the whole image string, isReplicaKind misses ReplicationController, and collectContainerSlots collects ephemeralContainers (kustomize won't touch them) while missing OCI volume images (it will). All three exist only because the re-implementation exists. It also records P1 and C1, the panic and the cycle fixed in 5a85afa. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(support-boundary): correct the design docs against what actually shipped Review feedback on #232, all of it fair, plus one thing it asked for. **Accuracy.** The docs were written before the renderer landed and several passages had gone stale in ways that mattered: - render-root-scoping said `sigs.k8s.io/kustomize` "is not a dependency of this module. Zero hits in go.mod". It is one, since #229 — that is the argument the doc *won*. Marked historical, and the inventory table now says which rows are gone (the 17-key deny-list, replaced by reflection over kustomize's struct) and which are still standing (renderImage, isReplicaKind — with the divergences they cause). - The sandbox table proposed `LoadRestrictionsRootOnly`. The shipped renderer uses `LoadRestrictionsNone`, deliberately, because RootOnly rejects `resources: [../shared.yaml]` which Flux renders happily — and failing to build a root silently disarms the write-fan-in guard. The contract as built is recorded rather than quietly corrected. - The doc claimed simulateImageRender "already does" propose-re-render-compare. It does not: it replays OUR re-implemented chain, not kustomize, over only the images it planned, and never checks that the rest of the build is untouched. It shares a blind spot with the thing it verifies, which is the whole reason it has to die. Said plainly. - values-file-projection conflated three different surfaces. Verified upstream: Argo's `helm.valueFiles` and Flux's `spec.chart.spec.valuesFiles` name a FILE; `valuesObject` is inline RawExtension in the Application; Flux's `valuesFrom` is a KRM object reference (`Secret/my-secret-values`). Only the first is a values file. Collapsing them would claim a file-projection capability over content living inside another document, or inside a Secret. **render-attribution**, tightened where the review was right: - "put a nonce in ANY knob" was an overclaim. Each new field costs a sink proof, and the fields worth reaching for next are the ones most likely to fail it: a replacements source is read by a selector, a generator literal feeds a name hash, a patch's containers[].name is a merge key. - "refuse and fall back" was ambiguous, and the review's instinct was sharper than it knew. Falling back to write-through is NOT harmless: writing a live value into a source document whose field an entry governs does not converge — the entry overrides it on the next render, which is the same non-convergence that convicts leave-one-out in §2. What makes the fallback safe is that the verification re-render CATCHES it. Fallback means no attribution, never another heuristic. **acceptance-precision** now contains only what is not implemented, as asked. `vars` is gone from it: #229 closed that hole, and closed it structurally — the unsupported set is derived by reflecting over kustomize's own struct, so a field we have never heard of refuses instead of being tolerated. Every remaining claim re-verified against the code (labels/commonLabels are still tolerated and still leak; Generated{path} is still emitted by nothing; the values file still cannot be rescued by .gittargetignore because *.yaml is a catastrophic pattern). Fenced blocks given language identifiers (MD040). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sunib
added a commit
that referenced
this pull request
Jul 15, 2026
…e, fix the vars contradiction The SourceFormRefusedError message (surfaced to the user via WriteBoundaryRefused) and its IssueUnplaceableEdit doc comment both cut off mid-clause; complete them. And §2's vars bullet claimed the folder is accepted today while a parenthetical said it now refuses -- rewrite the passage in past tense (both leaks it cites are now closed, by #229 and by sourceForm).
sunib
added a commit
that referenced
this pull request
Jul 15, 2026
…e, fix the vars contradiction The SourceFormRefusedError message (surfaced to the user via WriteBoundaryRefused) and its IssueUnplaceableEdit doc comment both cut off mid-clause; complete them. And §2's vars bullet claimed the folder is accepted today while a parenthetical said it now refuses -- rewrite the passage in past tense (both leaks it cites are now closed, by #229 and by sourceForm).
sunib
added a commit
that referenced
this pull request
Jul 15, 2026
…input (#234) * fix(kustomize): stop writing the build's own output into the build's input The writer mirrors a live object into the file that produced it. Under kustomize that file is not what the cluster runs, and mirroring the live object straight back writes the BUILD'S OUTPUT into the build's INPUT. Measured, on a folder we accept today, with nothing changed in the cluster and nothing changed in the render: a kustomization declaring `labels:` + `commonAnnotations:` and nothing else commits the overlay's `env: prod` and `owner: platform` into the base manifest, on the first reconcile of an unchanged folder. Every reconcile of an in-sync folder produced a commit, and the file was left wrong. In a base shared by two overlays, the value baked in is one environment's. The fix is one rule, and it models no transformer: WHERE THE LIVE OBJECT AND THE RENDER AGREE, THE SOURCE KEEPS ITS BYTES. WHERE THEY DISAGREE, THE USER CHANGED SOMETHING, AND THAT IS WHAT WE WRITE. Agreement means the build already produces exactly what the cluster runs, so the source is — by construction — what produced it, and there is nothing to write. Disagreement is the user's edit: it is routed to an images:/replicas: entry when the dye says one supplies the field, and written through to the source otherwise — where, if the build owns the field, the re-render refuses the flush. Because it needs no model of labels, of namespace, or of a patch, it closes all of them at once. It is also the gate on tolerating `patches:` at all: a patched base would otherwise absorb one environment's values, and no re-render can catch that (the patch re-imposes its value, so the render comes out identical). Two behavior changes go with it: - the re-render now runs for ANY document a render root produces, not only one an override chain governs. A change to a build-supplied field in a folder with no images:/replicas: entries used to be committed and silently never converge; it is now a reported refusal. - a live change the projection cannot place is refused (`unplaceable-edit`): the build and the user both rewrote one list whose elements carry no unique `name:` to pair them by. Pairing by position is not a conservative guess, it is measurably wrong — kustomize PREPENDS a container a patch adds. The corpus no-op invariant now compares the WHOLE document, not just its images, which is how a projection that quietly rewrote every field we had not modelled passed it for as long as it did: 8 documents checked before, 53 now. * docs(kustomize): address review on #234 -- finish the refusal sentence, fix the vars contradiction The SourceFormRefusedError message (surfaced to the user via WriteBoundaryRefused) and its IssueUnplaceableEdit doc comment both cut off mid-clause; complete them. And §2's vars bullet claimed the folder is accepted today while a parenthetical said it now refuses -- rewrite the passage in past tense (both leaks it cites are now closed, by #229 and by sourceForm). * docs(support-boundary): design work alongside the render-fidelity implementation Bring the support-boundary design docs onto this PR — they are the design that sits next to this real implementation, so they belong together rather than in a separate PR: render-fidelity.md (our render is not the orchestrator's; the render-vs-live fence, the blocking RenderFaithful condition, and where to implement it), admission-consent.md, orchestrator-reconcile-trigger.md, and the kpt-and-krm-functions orientation note, plus their README index rows. Consolidated from PR #237, now closed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(render-fidelity): rename the condition RenderFaithful -> RenderMatchesLive RenderMatchesLive names what it measures directly — does our render match the live object — better than the 'faithful' framing. The negative reason follows: RenderNotFaithful -> RenderDoesNotMatchLive, and the gate AcceptRenderFaithful -> AcceptRenderMatchesLive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(render-fidelity): conform to the decisions, and add the implementation prompt Align the prose to the RenderMatchesLive framing (the "faithful/unfaithful" language follows the rename to "matches / diverges"), and mark §8's 5a-first as decided rather than open. Add next-prompt-render-matches-live-gate.md: a handoff for a fresh session to implement the token gate — the design, the code entry points (patchExisting + the resync path), the CRD lesson (measure render-vs-live, not the disk; the reverted structural check broke CRD mirroring), the test net, and the e2e-tail gotcha. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(render-fidelity): the predicate is render-vs-live, not git-vs-live Incorporate the review findings: - The predicate must compare the RENDER to live, not the Git source to live. kustomize does not RESOLVE ${...}, but it does not PRESERVE every token-bearing source field either: a supported labels/commonLabels transform overwrites metadata.labels[...] via SetEntry, so `env: ${ENV}` under `labels: {env: prod}` renders to `prod` == live. Git-vs-live would falsely refuse that faithful folder. The render is dm.Rendered.Object for a kustomize doc, the Git doc for a plain one, and reading the render also catches a token a patches: block injects. Fixed the prompt (which had the git-vs-live shortcut + "does not need dm.Rendered") and §5a. - Narrow the causal claim in the fact doc and the design: a rendered token that differs from live proves only that our render did not produce the value — could be Flux substitution, a live edit, admission, another controller. Refusal is safe regardless; RenderDoesNotMatchLive is a fact, "must have been substituted" a guess. - State the two integration requirements for the blocking gate: aggregate fidelity across ALL scoped resyncs (a last-successful-GVR status masks a diverging type), and stop opening write windows while failed (status-only refusal isn't a gate). - Record the bias: blocking a shade too soon is fine; failing to block is not. The simple token gate is the right first cut. Also fixes the fact doc's external-sources markdown links (doccheck treats upstream checkouts as untracked) to code spans, matching the convention in the sibling docs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: define render fidelity gate * feat: gate writes on render fidelity * docs: clarify reconcile trigger barrier * fix: getting the bidirectional e2e test to work again * test: last extra checks to see if all behaves as expected * docs: last edits * docs: feedback of codex --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What
The analyzer decoded
kustomization.yamlwith a hand-written walk over a generic YAML map, checked against a hand-maintained list of 17 unsupported keys. It now decodes with kustomize's own type (sigs.k8s.io/kustomize/api/types.Kustomization), runs the sameUnmarshal+FixKustomizationsequence kustomize's builder runs (internal/target/kusttarget.go: load), and derives the unsupported set by reflecting over that type — anything not explicitly modelled refuses the folder.The dependency was never the cost we thought
sigs.k8s.io/kustomize/apiandkyamlwere already in this module's requirement graph (v0.15.0, unused). Taking them directly at the version Flux ships (v0.21.1) adds zero new modules to the build. We were re-writing, by instalments, a library we already carried.Five verdicts change, and the old code was wrong in all five
Each was verified against a real
kustomize build, not against my reading of the docs. In every case the new behaviour is the one that agrees with the renderer:kustomize buildvars:$(VAR)validators:(plugin code)resources: not-a-list, or akustomization.yamlthat is really a FluxKustomizationCRimageTags:imagesnewtag:(case variant),newName: ""We were refusing two folders Flux renders happily, and writing into two folders whose render we had misunderstood.
varsis the sharp one: a source file containing$(VAR)had the variable overwritten with whatever it happened to resolve to.The allowlist is now derived from kustomize's struct, so a field we have never heard of refuses the folder instead of being silently tolerated. That is what makes this class of bug unrepeatable — and it is worth more than the line count.
The one thing that does NOT move to the library
Remote-base detection stays ours, and gets promoted to a security precondition.
kustomize resolves a remote base by shelling out to
/usr/bin/git fetch— and it does so underLoadRestrictionsRootOnlyand under an in-memory filesystem. Both were measured. No build option turns it off. SohasRemoteResourcemust run before any build is invoked; it is what keeps "the operator never fetches a remote base" true, and it is the one piece of the re-implementation that must survive.Lines
Net production code: −45 (228 deleted, 21 added, 162 in the new file). The line count is not the point of this PR. The bulk — the 466-line
overrides_projection.go, the resource-DAG walk, the namespace walk — goes in the next change, which replaces the re-implemented transformers with the real render.Behaviour changes
Three of the five refuse folders that previously worked, all of them unsafe. Documented in
docs/UPGRADING.md.Corpus
One row of
support-today.mdmoves, in the hostile fixture: itskustomization.yamlis a Flux CR, so it is nowrefused-structural: unparseable (invalid Kustomization: json: unknown field "spec")— refused before and after, but the reason is now actionable.Validation
task fmt·generate·manifests·vet·lint·lint-docs·testall pass.task test-e2e: 54 of 65 specs passed, 0 failed (11 skipped = the opt-in bi-directional corner).🤖 Generated with Claude Code
Summary by CodeRabbit
Breaking Changes
vars,validators, and certain image options are refused.imageTagsandbasesforms are normalized and accepted where supported.Bug Fixes