feat(kustomize)!: attribute overrides with a dyed render, verify by re-render, delete the re-implementation#233
Merged
Merged
Conversation
…atch changed The patch argued for in patching-kustomize.md has now been built and measured on feat/build-trace-observer in external-sources/kustomize (290d04199, on top of upstream 79bb1aa2b): 638 lines added across 11 files, upstream's api suite still green. Building it corrected the doc on three points, now recorded in a new section 9: - the patch should be an *additive* krusty.Options.Observer, not a change to the transformer annotation's semantics -- no golden churn, no kubectl output change, and it carries the entry index and field paths the annotation structurally cannot; - the zero-cost gate is structural rather than merely safe, and is now asserted by TestBuildTraceDoesNotChangeTheBuild; - two facts only the running code produced: matching a resource across a rename needs an EffectiveNamespace-normalised key, and ConfiguredIn is relative to the build root rather than the repo root. The measured trace also settles section 3's headline claim by test rather than by argument: an idempotent pin produces no event at all, which is the case the dye was invented for and the one leave-one-out probing cannot answer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
renderFilesystem asked for buildMetadata only on a root spelled exactly "kustomization.yaml". Every other part of the analyzer accepts kustomization.yml too (isKustomizationFile), so a repository whose root is spelled .yml built happily with no buildMetadata at all: no config.kubernetes.io/origin, no alpha.config.kubernetes.io/transformations. Downstream that is not a missing annotation, it is a wrong answer. Every rendered object came back with an empty OriginPath and an empty transformation list, which reads as "this document is governed by no kustomization" -- so the images:/replicas: override chain was empty, and a live tag bump governed by an entry was written into the SOURCE MANIFEST, where the overlay shadows it straight back on the next render. Non-converging drift, silently, on every reconcile. Measured against main by the new test, which fails there with OriginPath="" and an empty chain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
renderRoot becomes renderRootWith over an empty overlay. The new call builds a render root with in-memory replacements layered over the scanned files, so a question about what kustomize would do with a DIFFERENT tree goes through the same sandbox, the same pre-build refusals and the same krusty invocation as the render we ship. It is the primitive the rest of this workstream is made of. Both remaining questions are this call with a different overlay: dye an entry and see where the nonce lands (attribution), or apply a proposed write and see what it renders (verification). No behaviour change on its own: every existing caller renders with no overlay. See docs/design/support-boundary/render-attribution.md section 7. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…simulating it simulateImageRender "verified" the projection's inversion by replaying OUR chain over the images OUR projection had planned. A check that shares the blind spot of the thing it checks cannot fail where that thing is wrong -- it agrees with itself -- so it did not catch a wrong attribution, it made one CONFIDENT. Both shipped image bugs (#231's digest/tag corruption, and the regex-vs-equality matcher) sailed straight through it. Replace it with the real thing. A flush that routes anything through a kustomization now re-renders every render root twice -- as it found the tree, and as it would leave it -- and proves both halves of the oracle: 1. every document the flush writes renders to exactly the live object, and 2. every object it does not write comes out byte-for-byte unchanged. (2) is the half that makes it safe for the projection to guess. A kustomization is shared context: an images: entry edited to converge one Deployment governs every other object it matches. A proposal that fixes its own target and moves a second object has written a live value into a file another render root also reads. It runs ONCE PER FLUSH, not once per resource, and that is correctness rather than economy. Asked resource by resource, "did anything else move?" is YES for the first of two Deployments that share an image and are bumped together -- so a per-resource oracle refuses a write that converges perfectly well. Only the whole batch can see that the sibling is moving because it, too, is being written to exactly where it now lives. That is why every document a flush touches declares a WriteIntent, and why a governed one declares it even when its own bytes did not change. A refused proposal REFUSES THE FLUSH -- an AcceptanceRefusedError surfacing as GitPathAccepted=False / WriteBoundaryRefused, naming the file and the object. It is not absorbed into a silent skip: a write that does not survive the re-render is one the entry overrides straight back on the next render, so absorbing it would leave the resource un-mirrored forever with nothing to show for it (render-attribution.md section 7). Two writes are declared UNCHECKED rather than compared, and it is stated rather than hidden: a SENSITIVE document (SOPS ciphertext -- no plaintext live object can equal it) and a bounded FIELD PATCH (a few audited assignments, never a whole object). Both are still held to disturbing nothing else. Attribution is untouched here and still comes from renderImage, so no routing changes. That is the point: the check now cannot share the blind spot of what it checks, which is what makes the attribution swap that follows safe to make at all. See docs/design/support-boundary/render-attribution.md section 5 and docs/design/support-boundary/render-root-scoping.md section 3. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… re-implementation
Deletes the last of the hand-rolled kustomize: renderImage, imageSuppliers and
isReplicaKind, the ~400 lines the write path used to decide which file an edit belongs
in. The projection now inverts against what kustomize ACTUALLY renders, read off the
renderer, instead of against our re-derivation of it.
HOW IT WORKS. A render says web:2.0; it never says who supplied the 2.0, and kustomize
keeps no field-level provenance anywhere, at any visibility level. So attribution cannot
be READ out of it -- only inferred by questioning it. Write a unique nonce into every
declared override entry, render the root a second time, and read the nonces off the
output: wherever a dye lands, THAT entry supplied THAT field. Two builds per root,
constant in the number of entries.
WHY NOT LEAVE-ONE-OUT. Removing an entry and seeing what moves is blind exactly where it
matters. A base at app:v1 under an overlay pinning newTag: v1 -- the state every repo is
in the moment a release lands in both places -- moves NOTHING when the entry is removed,
so the probe concludes the source file owns the tag, writes the user's next tag into the
base, and the overlay overrides it straight back. Forever. Removal probes the VALUE, and
values collide. A nonce nothing else can produce does not. Both cases are now tests
(idempotent pin, and a tie between two entries).
GUARDRAILS, all measured, none optional:
- the nonce charset is a CORRECTNESS requirement. kustomize does not validate a tag, but
it MATCHES on the image with a regex, so an out-of-charset dye leaves the image
un-matchable and every later entry SILENTLY STOPS FIRING. Tag dyes stay in
[a-zA-Z0-9_.{}-]; digest dyes carry the mandatory sha256: prefix.
- newName is not a pure sink -- it is the join key for every later entry -- so it is dyed
only when no other entry's name: matches it. And "matches" is asked of kustomize's own
compiled pattern, not of string equality, because an entry name is a REGEX.
- only a field an entry already DECLARES is dyed; injecting one would fabricate a supplier.
- baseline first, then dye. A dyed build that fails where the real one succeeded means the
dye hit something that is not a sink, and the answer is then NO ATTRIBUTION -- never a
fallback to another heuristic. Nothing routes, and the oracle from the previous commit
adjudicates whatever the source document alone can carry.
THREE SHIPPED BUGS DIE WITH THE CODE THAT CAUSED THEM (see docs/UPGRADING.md):
- B1: our image matcher was string equality; kustomize's is a regex over the whole image
string. `- name: "ap."` matches `app` in kustomize and did not in us, so we believed the
folder rendered one thing while it rendered another -- and wrote the difference into the
source manifest, killing the entry.
- B2: isReplicaKind listed Deployment, ReplicaSet, StatefulSet. kustomize's fieldspec also
includes ReplicationController -- it says so in its own error message. A scale on an RC
was written into the source document, where the transformer overrode it back.
- B3: we collected ephemeralContainers (kustomize does NOT rewrite them) and missed
volumes[].image.reference (kustomize DOES -- measured). Both directions mis-attributed.
None of these are fixed. They are DELETED, along with the fieldspec we had no business
keeping a second opinion about. The dye does not decide what kustomize touches; it reads
where kustomize's own nonces came out.
THE TEST NET IS REBUILT, which the signature change required and the change deserved. The
12 TestSplitDesired_* tests constructed (gitRaw, desired, overrides) BY HAND -- they
asserted what we BELIEVED a folder renders to, which is the belief that was wrong twice in
shipped code, so they could not have caught either bug and did not. Every case now builds a
real tree, renders it with kustomize, reads the attribution off a dyed render, and drives
the projection with the result. TestRenderImage_MatchesKustomizeOnTheHardCases is gone with
its subject; the corpus differential is replaced by a stronger invariant across both
corpora: AN IN-SYNC FOLDER MUST PROJECT TO A COMPLETE NO-OP -- which is exactly the
property #231's digest/tag corruption violated.
One more landmine confirmed while building it: a rendered object is not even a valid
unstructured. kustomize hands numbers back as Go `int`, so DeepCopyJSON PANICS on one
("cannot deep copy int") and NestedInt64 reports found=false. The projection reads replica
counts with a type switch, and the corpus test normalises through JSON.
The corpus support baseline is byte-for-byte unchanged: this changes how attribution is
derived, not which folders are supported.
See docs/design/support-boundary/render-attribution.md sections 3, 6 and 7.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…edger UPGRADING.md gets the entry B1, B2 and B3 are owed. All three shipped, and all three had the same shape: we believed a folder rendered one thing while kustomize rendered another, and the projection wrote the difference into the user's source manifest as though they had typed it. They arrive as deletions rather than fixes, which is the point. It also records the behaviour change that comes with them: a write routed through a kustomization is now re-rendered before it is committed, and one that does not reproduce the live object refuses the flush instead of landing. render-attribution.md marks section 7 done through stage 3, and records the two things building it corrected: - B4 is worse than the ledger said. A rendered object is not merely awkward to read a number off, it is not a valid unstructured at all: DeepCopyJSON PANICS on the Go int kustomize hands back. - the rename-chain guard has to use kustomize's own compiled regex, not the string equality section 3 proposed -- an entry name is a regex, so `mirror/ap.` matches `mirror/app` without equalling it, and an equality guard would dye the name, kill the next entry, and mis-attribute the tag. The ledger itself is kept as written rather than deleted, because how each bug was found is the whole argument for the method: not one came from reading kustomize's source. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The three docs about the kustomize build-trace fork (patching-kustomize, generated-repo-map, render-plan-artifact) now live in the fork itself, under plans/ in ConfigButler/kustomize-tracer, next to the code they describe and the tests that verify them. They remain in this repository's history; this removes them going forward. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
render-attribution.md linked patching-kustomize.md, which now lives in the fork (ConfigButler/kustomize-tracer, plans/). Left as a plain reference so the link checker does not chase it into a repository it cannot see. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e oracle Both were in the new code, and both are proved by a test that fails without the fix. 1. THE WRITE AND THE INTENT ARE DIFFERENT OBJECTS. createNew strips metadata.namespace out of the bytes it writes when the destination inherits its namespace from a kustomization's namespace: transformer -- correct, because the transformer puts it back, and every sibling in that directory follows the convention. But the three writers then built the WriteIntent from that same stripped object. The render HAS the namespace, so the intent demanded a render that did not, and the oracle refused a flush it had just planned perfectly: the new resource was lost, and so was any governed write batched with it. Resync replays the same batch, so it never recovered. patchExisting had it right all along -- it strips a LOCAL copy and hands the untouched event to the intent -- which is exactly the asymmetry that gave the bug away. Intent recording moves out of the three writers, which only ever see the mutated object, and up to the callers, which still hold both. This fires on the most ordinary kustomize layout there is: a per-env folder with a namespace: and an images: entry, plus any new resource. 2. A NEW RESOURCE COULD NOT REACH THE ORACLE AT ALL. Its intent was never Governed (Governed came from dm.Overrides -- a PRE-batch store document, which a new document by definition does not have), so a flush containing only new resources never turned the oracle on. And createNew does not route a new document's values onto an entry: it has no override chain yet. So a new Deployment whose image an existing images: entry matches was committed at its live tag, the entry rewrote it on the next render, and the mirror asserted a state the folder does not produce -- forever. Silently. That is precisely the failure this whole path exists to prevent, in the one write path the oracle could not see. It still does not route (that needs attribution for a document that does not exist yet), but it is now put in front of the oracle, which turns a silent non-converging commit into a reported refusal naming the file and the object. "We cannot express this here" is an answer. Quietly writing a lie is not. Turning the oracle on is now a separate question from WriteIntent.Governed, because the two are not the same claim: Governed additionally ASSERTS the document is rendered, and a new document is not entitled to that -- its resources: entry can legitimately fail to be added (a kustomization with no resources: sequence), leaving the file written but outside every render. Conflating them refused that case too, which the placement suite caught. Found by an adversarial review of the diff. It also cleared the dye itself: nonce collision, by-position alignment, and the tag/digest routing all held. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe write path now derives image and replica attribution from real kustomize renders, projects live changes through attributed overrides, and verifies planned batches with before/after renders before writing. Render mismatches become write-boundary refusals. ChangesKustomize render attribution
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Event
participant PlanFlush
participant VerifyBatchRenders
participant Kustomize
participant Worktree
Event->>PlanFlush: build write intents
PlanFlush->>VerifyBatchRenders: verify planned batch
VerifyBatchRenders->>Kustomize: render before and after trees
Kustomize-->>VerifyBatchRenders: rendered objects
VerifyBatchRenders-->>PlanFlush: success or render refusal
PlanFlush->>Worktree: write files after successful verification
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Registering a new resource in a kustomization's resources: list was only half the job.
Nothing ever took the entry back out, so deleting the manifest left an entry naming a file
that no longer exists -- and kustomize refuses to build over that:
accumulating resources ... '/scan/bundle.yaml' doesn't exist
The folder became undeployable, and the GitTarget was refused on the next reconcile.
RemoveKustomizationResource is AppendKustomizationResource read backwards, with the same
all-or-nothing semantics: idempotent, and a kustomization it cannot edit (multi-document,
no resources: sequence, unparseable) is skipped with a diagnostic rather than having
structure invented for it.
THE ENTRY COMES OUT ONLY WHEN THE FILE ACTUALLY GOES. A file holding several documents
survives the deletion of one of them, and its resources: entry must survive with it --
pulling it would un-deploy every OTHER resource in that file, which nobody asked to touch.
That invariant is the first test, and it is one that bites: dropping the entry on any
delete rather than only on the last one fails it immediately.
Three unit tests at the writer (the multi-document guard, the last-document removal, and a
delete batched with a governed write -- which without this could not build the counterfactual
tree at all, so the oracle refused the whole flush), three at the editor, and an e2e that
closes the round trip against the real controller: the placement spec creates a ConfigMap in
a kustomize overlay and asserts the entry appears; the new spec deletes it, asserts the entry
is gone, and then asks the cluster to `apply -k` the folder -- which is the whole point, and
which a dangling entry fails.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
internal/manifestanalyzer/render_verify.go (1)
220-223: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd a fast path to avoid serialization overhead for unchanged objects.
sameObjectis called frequently (for every object in every render root). In the!intended(blast-radius) case, bothbefore.Objectandafter.Objectcome directly from Kustomize renders and share the same internal numeric types (e.g.,int). This meansreflect.DeepEqualwill succeed for unmodified objects.Adding a
reflect.DeepEqualfast path avoids the allocation and serialization overhead ofjson.Marshalfor the vast majority of objects across a repository.⚡ Proposed fast path optimization
Make sure to add
"reflect"to the import block.func sameObject(a, b *unstructured.Unstructured) bool { if a == nil || b == nil { return a == b } + if reflect.DeepEqual(a.Object, b.Object) { + return true + } left, err := json.Marshal(a.Object)🤖 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/render_verify.go` around lines 220 - 223, Update sameObject to return immediately when reflect.DeepEqual reports the two non-nil objects equal, before JSON serialization; add the reflect import and preserve the existing serialization comparison for objects that are not deeply equal.internal/git/plan_flush.go (1)
706-857: 🚀 Performance & Scalability | 🔵 TrivialRender precondition re-renders every kustomize root in the repo on any governed write.
renderPreconditioncallsVerifyBatchRenderswith the whole-treebefore/afterfile sets wheneverputToKustomizeis true, andVerifyBatchRendersiteratesrenderTargets(parseKustomizations(after))— i.e. every render root found anywhere, not just the ones touched by this batch. For a repository with many independent kustomize roots, a single governed edit in one folder pays the render cost of every other root too. This is presumably an accepted correctness/perf tradeoff given the design's "verification may not be heuristic" stance, but worth keeping in mind as the fleet of managed roots grows.🤖 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/git/plan_flush.go` around lines 706 - 857, Limit renderPrecondition verification to kustomize roots affected by the current write batch instead of passing the entire repository to VerifyBatchRenders for every governed change. Update the render-target selection around writeBatch.renderPrecondition and the VerifyBatchRenders call so independent untouched roots are excluded, while preserving complete before/after validation for all roots touched by the batch.
🤖 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.
Inline comments:
In `@docs/design/support-boundary/render-attribution.md`:
- Around line 9-11: Update the blockquote in the section containing the
renderRootWith status note by replacing the bare blank separator with a
blockquote-prefixed blank line, preserving the quoted formatting used by the
surrounding separators.
In `@internal/git/plan_flush.go`:
- Around line 980-985: Update applyDelete around the wb.intend call to set
wb.putToKustomize for deletes that require kustomize rendering, ensuring
renderPrecondition invokes VerifyBatchRenders even when deletion is the only
kustomize-relevant batch step and dangling resources entries are rejected.
In `@internal/manifestanalyzer/override_chain.go`:
- Around line 233-255: Update record to set anyOverrides based on ov != nil or
rd != nil, including the initial assignment and subsequent accumulation so
rendered-only overrides are recognized. Add a test in overrides_test.go covering
ov == nil with non-nil RenderedOverrides and verifying the resulting assignment
is treated as ambiguous when roots differ.
---
Nitpick comments:
In `@internal/git/plan_flush.go`:
- Around line 706-857: Limit renderPrecondition verification to kustomize roots
affected by the current write batch instead of passing the entire repository to
VerifyBatchRenders for every governed change. Update the render-target selection
around writeBatch.renderPrecondition and the VerifyBatchRenders call so
independent untouched roots are excluded, while preserving complete before/after
validation for all roots touched by the batch.
In `@internal/manifestanalyzer/render_verify.go`:
- Around line 220-223: Update sameObject to return immediately when
reflect.DeepEqual reports the two non-nil objects equal, before JSON
serialization; add the reflect import and preserve the existing serialization
comparison for objects that are not deeply equal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d974081-344f-49ca-a0bf-70e3a3d5ecf1
📒 Files selected for processing (22)
docs/UPGRADING.mddocs/design/support-boundary/generated-repo-map.mddocs/design/support-boundary/patching-kustomize.mddocs/design/support-boundary/render-attribution.mdinternal/git/kustomize_oracle_test.gointernal/git/plan_flush.gointernal/manifestanalyzer/acceptance.gointernal/manifestanalyzer/analyzer_test.gointernal/manifestanalyzer/dye.gointernal/manifestanalyzer/dye_test.gointernal/manifestanalyzer/kustomize_render.gointernal/manifestanalyzer/kustomize_render_hostile_test.gointernal/manifestanalyzer/kustomize_render_semantics_test.gointernal/manifestanalyzer/kustomize_render_test.gointernal/manifestanalyzer/override_chain.gointernal/manifestanalyzer/overrides.gointernal/manifestanalyzer/overrides_attribution.gointernal/manifestanalyzer/overrides_projection.gointernal/manifestanalyzer/overrides_projection_test.gointernal/manifestanalyzer/render_verify.gointernal/manifestanalyzer/store.gointernal/watch/event_router.go
💤 Files with no reviewable changes (2)
- docs/design/support-boundary/generated-repo-map.md
- docs/design/support-boundary/patching-kustomize.md
render-root-scoping.md still listed renderImage and isReplicaKind as "still there" with their divergences described in the present tense, and still planned the oracle as future work whose differential test was to be a comparison against simulateImageRender. All three are deleted, so those rows and that step now describe code that does not exist. render-attribution.md loses its "one step left" framing and the long status block: the workstream is done, so the doc is a record of how attribution was decided, not a plan. No content is removed that still says something true. The bug ledger stays exactly as written, because how each bug was found is the argument for the method. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…k both questions in anyOverrides Three findings from the review. One was right, one was wrong but pointed at a real gap next to it, and one was a lint break. 1. anyOverrides only asked whether a CHAIN existed, never whether an ATTRIBUTION did. A document can render an image with no images:/replicas: entry governing it: the chain is nil, the attribution is not. Two roots disagreeing only there would diverge in the fingerprint while ambiguous() stayed false, so the document would be silently un-routed with no diagnostic and no fan-in refusal. Routing was already safe (record() nils the attribution on any divergence), but the guard was not. The divergence is not constructible today -- a nil chain means no image transformer ran, which means the rendered value IS the source value, which is the same in every root -- so this changes no current behaviour. It is fixed because the invariant is not obvious enough to leave resting on that argument. Test added, as asked. 2. The claim was that applyDelete never sets putToKustomize, so a delete could commit a dangling resources: entry. That is not so: dropKustomizationResource sets the flag BEFORE attempting the removal, so a removal that fails still puts the flush to the oracle. Measured: forcing RemoveKustomizationResource to fail refuses the flush with kustomize's own "'/scan/bundle.yaml' doesn't exist". But the instinct was right about the case beside it. A delete that does NOT empty the file never set the flag at all, so the rule was "verified only when our own bookkeeping says the file went" rather than "verified because it is inside a render root". The whole point of the oracle is that it does not take our word for anything, so the rule is now uniform: any delete of a document whose file is named in a resources: list goes to the oracle. The lookup that decides that is now one function shared with the entry removal, so the two cannot drift. 3. MD028: a bare blank line ended the blockquote instead of continuing it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rdering fact measured patches is the biggest refusal cause in the layout corpus (8, against namePrefix 6 and configMapGenerator 6), it refuses the whole GitTarget rather than the edit, and the dye shipped in #233 is the mechanism its milestone was waiting for. Carries the four things worth measuring rather than re-deriving, the load-bearing one being that patches run BEFORE the images:/replicas: transformers and the transformers WIN -- so a patch that sets a field an entry also governs is dead text. That is the fact that makes 'just edit the patch' wrong, and the reason the dye should be extended into the patch rather than the patch being read and reasoned about. 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.
Finishes the kustomize-renderer workstream. This is the code that decides which file a user's edit gets committed to, and it was the last of the hand-rolled kustomize re-implementation.
renderImage,imageSuppliers,simulateImageRenderandisReplicaKindare gone. The projection now inverts against what kustomize actually renders, and against which entry kustomize's own dye came out of.The honest number first
Production code is net +1011 lines. The brief said "delete ~400 lines"; it deleted them and then added roughly a thousand (the dye, the attribution reader, the oracle, and the writer plumbing to feed it).
The trade is not lines. It is that none of the new code contains a model of kustomize's semantics, so none of it can drift from kustomize. The 400 lines it replaces were a second opinion about what a folder renders to, and that second opinion shipped three bugs and caused two more (#229, #231).
Attribution by dye
A render says
web:2.0but never says who supplied the2.0, and kustomize keeps no field-level provenance anywhere. So attribution can only be inferred by questioning it: write a unique nonce into every declared override entry, render a second time, and read the nonces off the output. Wherever a dye lands, that entry supplied that field.The obvious alternative (remove an entry, see what moves) is blind where it matters most. A base at
app:v1under an overlay pinningnewTag: v1, which is the state every repo is in the moment a release lands in both places, moves nothing when the entry is removed. So removal concludes the source file owns the tag, writes the user's next tag into the base, and the overlay overrides it straight back. Forever. Removal probes the value, and values collide. A nonce nothing else can produce does not. That case and a tie between two entries are both tests now.Verification by re-render
simulateImageRenderreplayed our chain over the images our projection had planned. A check that shares the blind spot of the thing it checks cannot fail where that thing is wrong, so it did not catch a bad attribution, it made one confident.It is replaced by a real re-render of every root, once per flush, requiring every written document to reproduce its live object while every object the flush did not write stays byte-identical. That second half is what makes it safe for attribution to be a guess. A proposal that fails refuses the flush loudly (
GitPathAccepted=False/WriteBoundaryRefused, naming the file and the object) rather than being absorbed into a silent skip.Bugs deleted (all shipped, all measured; see
docs/UPGRADING.md)- name: "ap."matchesappin kustomize and did not in us, so we wrote the rendered value into the source manifest, which then no longer matched the entry and silently killed the override.isReplicaKindforgotReplicationController. kustomize's fieldspec has four kinds and says so in its own error message. A scale on an RC was written into the source document, where the transformer overrode it back.volumes[].image.reference(measured) and does not rewriteephemeralContainers(measured). We had it backwards on both.kustomization.yml(not.yaml) render root got no provenance at all, so its override chain read as empty and governed images were written into source manifests.resources:entry, which kustomize refuses to build over, making the folder undeployable. The entry now comes out with the file, but only when the file is actually gone: a file holding several documents keeps its entry, or the other resources in it would be un-deployed.These are not fixed, they are deleted, along with the fieldspec we had no business keeping a second opinion about.
The test net was rebuilt, because it had to be
The 12
TestSplitDesired_*tests constructed(gitRaw, desired, overrides)by hand, so they asserted what we believed a folder renders to. That is the belief that was wrong twice in shipped code, and they could not have caught B1. Every case now builds a real tree, renders it with kustomize, reads attribution off a dyed render, and drives the projection with the result.The corpus differential is replaced by a stronger claim across both corpora: an in-sync folder must project to a complete no-op, which is exactly the property #231's corruption violated. The support baseline is byte-for-byte unchanged: this changes how attribution is derived, not what is supported.
Two bugs an adversarial review found in this diff (
8a71c37)Both were in the new code, both now have a test that fails without the fix.
createNewstripsmetadata.namespacefrom the bytes it writes when the directory supplies the namespace (correct), but the writers built the verification intent from that same stripped object. The render puts the namespace back, so the oracle refused a flush it had just planned perfectly, losing the new resource and any governed write batched with it. It fired on the most ordinary kustomize layout there is.The review cleared the dye itself: nonce collision, by-position alignment, and tag/digest routing all held.
Validation
fmt→generate→manifests→vet→lint→test→test-e2e, all green (e2e: 55 passed, 0 failed, including a new spec that deletes a resource from a kustomize overlay and then asks the cluster toapply -kthe folder). Corpus baseline regenerated, unchanged.Design
docs/design/support-boundary/render-attribution.md§3 (the dye), §5 (attribution may be heuristic, verification may not), §6 (the bug ledger), §7 (stages 1-3 shipped; stage 4 is not).🤖 Generated with Claude Code