diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md
index a6af7b73..27aacd38 100644
--- a/docs/UPGRADING.md
+++ b/docs/UPGRADING.md
@@ -7,6 +7,50 @@ guidance that the changelog's breaking-change entries link to.
We are pre-1.0, so breaking changes bump the **minor** version (release-please is configured with
`bump-minor-pre-major`) rather than the major. Read the relevant entry before upgrading across it.
+## Unreleased — kustomize decides what it renders, and what it touched (next minor; bug fixes + behavior change)
+
+The write path no longer contains a re-implementation of kustomize's image and replica
+transformers. It asks kustomize what a folder renders to, and — by rendering a second time
+with a unique nonce written into every override entry — which entry supplied each value.
+`renderImage`, `imageSuppliers`, `simulateImageRender` and `isReplicaKind` are deleted.
+
+**Three shipped bugs go with them.** Each was a case where we believed a folder rendered
+one thing while kustomize rendered another, and the projection then wrote the difference
+into your source manifest as though you had typed it there.
+
+| | If your repo has… | What was happening |
+|---|---|---|
+| **B1** | an `images:` entry whose `name:` is not a literal — `- name: "ap."`, `- name: ".*"`, `- name: app:v1` | A kustomization `name:` is a **regular expression**, and kustomize matches on it as one. Our matcher was string equality, so we thought the entry matched nothing while kustomize rewrote the image. We read the difference as a user edit and wrote the *rendered* value into the source manifest — which then no longer matched the entry, silently killing the override. |
+| **B2** | a `replicas:` entry naming a **ReplicationController** | kustomize's replica fieldspec is `[Deployment, StatefulSet, ReplicaSet, ReplicationController]` — it says so in its own error message. Ours listed three of the four. A scale on an RC was written into the source document, where the transformer overrode it right back: non-converging drift, on every reconcile, forever. |
+| **B3** | an OCI **`volumes[].image.reference`**, or an **`ephemeralContainers`** entry | kustomize rewrites volume image references (measured) and does **not** rewrite ephemeral containers (measured). We had it backwards on both: we never looked at volume images, so the rendered value was mirrored back into your source file; and we treated ephemeral containers as override-governed when the source file owns them. |
+
+**Nothing here needs migration** — these are fixes, and they make the operator stop
+rewriting files it should have left alone. Check `git log` on your manifests if you want to
+see whether a past reconcile touched an image or a replica count you did not change.
+
+**Deleting a resource now also removes its `resources:` entry.** Previously the manifest was
+deleted and the entry was left behind, pointing at a file that no longer exists — which
+kustomize refuses to build over (*"accumulating resources … doesn't exist"*), so the folder
+became undeployable and the `GitTarget` was refused on the next reconcile. Registering the
+entry when a resource is created was only half the job.
+
+The entry is removed **only when the file itself is actually gone**. A file holding several
+documents survives the deletion of one of them, and its entry stays — pulling it would
+un-deploy every other resource in that file.
+
+**One more behavior change.** A write routed through a kustomization is now
+re-rendered with kustomize before it is committed, and must reproduce the live object
+exactly while leaving every other rendered object untouched. A write that fails that check
+**refuses the flush** — `GitPathAccepted=False` / `WriteBoundaryRefused`, naming the file
+and the object — rather than being committed.
+
+This is deliberate, and it is the safe direction: a write that does not survive the
+re-render is one the override entry overrides straight back on the next render, so
+committing it would leave the resource permanently un-mirrored while looking like it
+worked. If you see this refusal, the live state cannot be expressed in the repository as it
+stands — most often because something we do not model (a `patches:` block, a
+`replacements:` entry) owns the field. The refusal names it.
+
## Unreleased — a folder kustomize cannot build is now refused (next minor; behavior change)
The analyzer now **builds** every render root with kustomize, instead of only parsing the
diff --git a/docs/design/support-boundary/generated-repo-map.md b/docs/design/support-boundary/generated-repo-map.md
deleted file mode 100644
index ca548828..00000000
--- a/docs/design/support-boundary/generated-repo-map.md
+++ /dev/null
@@ -1,321 +0,0 @@
-# The generated repo map: draw the inverse, and show where it does not exist
-
-> **design** — direction-setting; ships no code. Nothing it describes is supported today.
-> Captured: 2026-07-14
-> Related:
-> [README.md](README.md),
-> [support-contract.md](support-contract.md),
-> [kustomize-support-boundary.md](kustomize-support-boundary.md),
-> [render-attribution.md](render-attribution.md),
-> [render-root-scoping.md](render-root-scoping.md),
-> [acceptance-precision.md](acceptance-precision.md),
-> [repo-discovery-and-onboarding-scan.md](repo-discovery-and-onboarding-scan.md)
-
-Can we generate, from a user's GitOps repo, a diagram that shows how their repo is
-actually built — and that explains, visually, why parts of it cannot be reversed?
-
-Yes. And it is cheaper than it looks, because **we already compute the graph and throw it
-away**. But the diagram is only worth shipping if it is honest, and honesty here has a
-precise meaning: *never draw an edge you cannot justify*. Kustomize will happily hand you
-an edge that means "ran over this object" and let you mislabel it "changed this object".
-The whole design problem is the fidelity of the arrows, not the drawing of them.
-
-## 1. The reframe: the build DAG is the boring half
-
-Every kustomize visualiser ever written draws the same picture — sources fan into an
-overlay, an overlay fans into rendered YAML. Left to right, top to bottom, and it tells
-the user nothing they did not already know from their directory listing.
-
-We are not a build tool. We are a *reverser*. Our user's question is never "what does this
-render to"; they can run `kustomize build`. Their question is:
-
-> I changed the replica count in the cluster. **Which file are you going to write, and
-> why can't you write it for this other thing?**
-
-That is the inverse arrow. So the diagram must be drawn in the direction of the question:
-from the live object back to the source. And once you draw it that way, the interesting
-content is not the arrows that exist — it is **the arrows that have no inverse**. Those
-are exactly the support boundary, and they are exactly what the user is confused about.
-
-The build DAG is the substrate. The missing inverse edges are the product.
-
-## 2. Four graphs, three of which we can already draw
-
-| Tier | Edge | Source of truth | Fidelity | Cost to us |
-|---|---|---|---|---|
-| **1. Inclusion** | kustomization → resources / bases / components / patches / generators | `kustomization.yaml`, decoded with `kustypes.Kustomization` | **Exact.** Declared by the user. | Free — [`parseKustomizations`](../../../internal/manifestanalyzer/kustomization_parse.go) already runs on every scan. |
-| **2. Origin** | source file → rendered object | kustomize's `config.kubernetes.io/origin` annotation | **Exact.** Kustomize stamps the file that produced the object. | Free — [`renderedObject.OriginPath`](../../../internal/manifestanalyzer/kustomize_render.go) is already populated, then discarded. |
-| **3. Transformed-by** | kustomization → rendered object | kustomize's `alpha.config.kubernetes.io/transformations` annotation | **Over-approximate.** A superset. See below. | Free — [`renderedObject.TransformedBy`](../../../internal/manifestanalyzer/kustomize_render.go) is already populated, then discarded. |
-| **4. Field attribution** | override *entry* → field of an object | — | **Does not exist.** | Requires the dye ([render-attribution.md](render-attribution.md) §3), or a fork. |
-
-Tier 3 is the trap, and it is worth being exact about why, because the temptation to
-mislabel it is enormous. Kustomize runs a transformer and then annotates **every resource
-currently in the ResMap**, with no check that the transformer touched any of them
-(`api/resmap/reswrangler.go`, `AddTransformerAnnotation` — it iterates `m.rList`
-unconditionally; upstream's own `transformerannotation_test.go` shows a `Namespace` object
-carrying two `PrefixTransformer` entries despite the prefix transformer excluding
-namespaces by fieldspec). So:
-
-> "Transformer X is in this object's annotation" means **X ran while this object was in
-> the map**. It does not mean X changed it.
-
-A diagram that draws that as `overlays/prod --changed--> Deployment/web` is lying, and it
-is lying in the direction that makes the user trust a write we cannot actually perform.
-Tier 3 edges must be drawn, but they must be drawn as *may have* — and they must look
-different from tier 2 edges at a glance.
-
-Tier 4 — "the `images:` entry at index 1 supplied `spec.template.spec.containers[0].image`" —
-is the edge users most want, and kustomize does not have it at any level of its API. It is
-not hidden behind an internal package; it is not computed at all. That is the subject of
-[render-attribution.md](render-attribution.md); this doc simply must not pretend otherwise.
-
-## 3. The legend is the thesis
-
-Three line styles, and they are not cosmetic — each one is a claim about how much we know.
-
-```mermaid
-flowchart LR
- subgraph source["source — what the user edits"]
- BK["base/kustomization.yaml"]
- BD["base/deployment.yaml
image: web:v1"]
- OK["overlays/prod/kustomization.yaml
images: web → newTag v2
replicas: web → 3"]
- end
- subgraph rendered["rendered — what the cluster gets"]
- RD["Deployment/web
image: web:v2
replicas: 3"]
- RS["Service/web"]
- end
-
- OK -->|declares| BK
- BK -->|declares| BD
- BD ==>|origin| RD
- BK -.->|may have shaped| RS
- OK -.->|may have shaped| RD
- OK -.->|may have shaped| RS
-
- classDef editable fill:#dfd,stroke:#3a3,color:#111
- classDef obj fill:#eef,stroke:#66a,color:#111
- class BK,BD,OK editable
- class RD,RS obj
-```
-
-| Style | Meaning | Guarantee |
-|---|---|---|
-| `-->` **declares** | inclusion, read straight out of `kustomization.yaml` | exact |
-| `==>` **origin** | this file produced this object | exact, from kustomize |
-| `-.->` **may have shaped** | this kustomization's transformers ran over this object | **superset** — it may have changed nothing |
-
-Note what the picture already tells the user, honestly: `Service/web` is dashed from both
-kustomizations. It has no `images:` or `replicas:` entry that could possibly apply to it —
-and yet kustomize's annotation names them both. The dashed edge is *correct* and the user
-can see for themselves that it is weak. That is the diagram doing its job.
-
-## 4. Now reverse it, and the boundary draws itself
-
-Same repo, arrows reversed, and the question changed from "what renders" to "where does an
-edit land".
-
-```mermaid
-flowchart LR
- LIVE["live edit
Deployment/web
image → web:v3"]
-
- LIVE ==>|"origin⁻¹ — exact"| BD["base/deployment.yaml"]
- LIVE -.->|"which entry?
no inverse"| OK["overlays/prod/kustomization.yaml
images: web → newTag v2"]
- BD -->|"but the write jail (L1)
forbids writing the base
from a prod edit"| JAIL["refused"]
-
- classDef editable fill:#dfd,stroke:#3a3,color:#111
- classDef refused fill:#fdd,stroke:#c33,color:#111
- class OK editable
- class BD,JAIL refused
-```
-
-Two different failures, in one picture, and the user can tell them apart:
-
-- **The `newTag` arrow has no inverse.** Kustomize will not tell us that the `v2` in the
- rendered image came from the overlay's `newTag` rather than from the file's own tag. We
- can *guess* (and today we guess by re-implementing the transformer in
- [`overrides_projection.go`](../../../internal/manifestanalyzer/overrides_projection.go) —
- ~400 lines of re-implementation that [render-attribution.md](render-attribution.md) is
- trying to delete). The diagram should say *we cannot see this*, not invent an arrow.
-- **The origin arrow exists and is exact, and we still refuse it.** Writing
- `base/deployment.yaml` because production changed would silently change staging too.
- That is [render-root-scoping.md](render-root-scoping.md) §4 / the L1 write jail in
- [`plan_flush.go`](../../../internal/git/plan_flush.go): *an edit to an object rendered by
- an overlay lands in that overlay, never in the base.*
-
-These are the two sentences we currently make users discover by having a write refused. A
-picture states both before they ever try.
-
-## 5. The strongest visual we have: an arrow with no tail
-
-The single clearest "this cannot be reversed" is not a colour or a label. It is a
-**missing tail**.
-
-```mermaid
-flowchart LR
- subgraph source["source"]
- K["apps/web/kustomization.yaml
configMapGenerator: app-config
helmCharts: redis"]
- D["apps/web/deployment.yaml"]
- end
- subgraph rendered["rendered"]
- RD["Deployment/web"]
- CM["ConfigMap/app-config-g9df72cd5b"]
- RH["StatefulSet/redis"]
- end
-
- D ==>|origin| RD
- K -->|declares| D
- K ==>|"configuredIn — no file"| CM
- K ==>|"configuredIn — no file"| RH
-
- NOTE["no source document exists
to write an edit back into"]
- CM --- NOTE
- RH --- NOTE
-
- classDef editable fill:#dfd,stroke:#3a3,color:#111
- classDef refused fill:#fdd,stroke:#c33,color:#111
- classDef obj fill:#eef,stroke:#66a,color:#111
- class K,D editable
- class RD obj
- class CM,RH,NOTE refused
-```
-
-This is not a metaphor we are imposing — it is literally what kustomize reports. For a
-generated resource, the origin annotation carries **no `path`**; it carries `configuredIn:
-kustomization.yaml` and `configuredBy: {apiVersion: builtin, kind: ConfigMapGenerator}`.
-Our renderer already records this as an empty `OriginPath`
-([`kustomize_render.go`](../../../internal/manifestanalyzer/kustomize_render.go): *"Empty
-for a generated resource"*).
-
-An object with an empty origin path is an object with no file. There is nowhere to write.
-The diagram shows the arrow starting at a kustomization stanza instead of a document, and
-the user understands the refusal in about two seconds — which is roughly two seconds
-faster than any refusal message we could write.
-
-The same trick covers the whole permanent boundary: `helmCharts`, `configMapGenerator`,
-`secretGenerator`, and anything else that manufactures an object rather than including one.
-
-## 6. The refusal overlay: colouring a graph we already have
-
-The repo-level map needs no new analysis whatsoever. [`scan_repo.go`](../../../internal/manifestanalyzer/scan_repo.go)
-already produces, per candidate subtree, exactly the node attributes a diagram needs — and
-already emits them as JSON:
-
-| Field in `RepoCandidate` | What it becomes in the diagram |
-|---|---|
-| `Path` | the node |
-| `Layout` (`plain`, `kustomize-single`, `kustomize-overlay`, `refused-structural`) | the node's shape/colour |
-| `AcceptedByOperator` | green vs red |
-| `RefusalReasons[].Code` | the label on the red node — and the two codes must stay distinguishable: `overlay-fan-out-unsupported` is a **"not yet"**, `refused-structural` is the **permanent** boundary. A diagram that paints them the same red destroys the one distinction the discovery scan exists to preserve. |
-| `ReadScope` | the dashed edges leaving the subtree — i.e. *this overlay reads a base you did not give us* |
-| `Resources.Rendered` vs `.Editable` | the gap, printed on the node. `12 rendered / 0 editable` is the entire overlay problem in five characters. |
-| `OverlapsWith` | a conflict edge — two candidates that can never both be adopted |
-
-```mermaid
-flowchart TB
- BASE["base/
(not a candidate on its own)"]
- PROD["overlays/prod
kustomize-overlay
12 rendered / 0 editable"]
- STAG["overlays/staging
kustomize-overlay
12 rendered / 0 editable"]
- PLAIN["clusters/dev
plain — accepted
9 rendered / 9 editable"]
- HELM["platform/cert-manager
refused-structural
helmCharts"]
-
- PROD -.->|reads outside its subtree| BASE
- STAG -.->|reads outside its subtree| BASE
-
- classDef editable fill:#dfd,stroke:#3a3,color:#111
- classDef notyet fill:#ffe9c7,stroke:#d89614,color:#111
- classDef refused fill:#fdd,stroke:#c33,color:#111
- class PLAIN editable
- class PROD,STAG notyet
- class HELM refused
- class BASE obj
-```
-
-Amber for "not yet", red for "never", green for "adopt it now". That is the onboarding
-conversation, generated.
-
-And the same colouring extends to the per-document diagnostics the store already carries —
-`kustomize-build-failed` ([`override_chain.go`](../../../internal/manifestanalyzer/override_chain.go)),
-`ambiguous-kustomize-overrides` ([`overrides.go`](../../../internal/manifestanalyzer/overrides.go),
-the fan-in > 1 case), `duplicate-identity`, `non-editable`. Each is already a
-machine-readable reason attached to a path. Each is a node colour and a label. **We are not
-building an analyser. We are building a renderer for the analyser we have.**
-
-## 7. Traps — edges the drawing will beg you to add
-
-| Tempting edge | Why it is wrong |
-|---|---|
-| `transformer --changed--> object` | Kustomize's annotation means *ran over*, not *changed* (§2). Every solid arrow here is a promise we cannot keep. |
-| `images[1] --> containers[0].image` | Tier 4. Does not exist. The dye ([render-attribution.md](render-attribution.md) §3) is how you would earn it; until then, do not draw it. |
-| `patch.yaml --> spec.replicas` | Same class of lie. A strategic-merge patch's *effect* is not reported per field. We know the patch ran; we do not know what it hit. |
-| `object --edit-lands-here--> base/deployment.yaml` | Origin is exact, but the write jail forbids it (§4). **Origin is where it came from, not where the edit goes.** Conflating those two is the single most dangerous arrow on the page. |
-| `origin ⇒ always a file` | Not always. When a transformer produces a resource that had no origin, kustomize writes the *transformer's* origin into the origin annotation. An origin can name a kustomization, not a document. |
-| a diagram of `base/` drawn standalone | `buildMetadata` is honoured **only on the root kustomization**, and it is force-propagated down into bases (a base's own setting is overwritten). So a base rendered on its own and the same base seen through an overlay are two different graphs. Always diagram *a render root*, and say which one. |
-
-## 8. Where it ships
-
-Not the operator. This is an onboarding/explanation artifact, so it belongs with the
-discovery work in the `manifest-analyzer` CLI — beside
-[`RenderText` / `RenderJSON` / `RenderScanText`](../../../internal/manifestanalyzer/render.go),
-as one more output format over data structures that already exist:
-
-```
-manifest-analyzer scan --repo . --format mermaid # tier 1 repo map (§6)
-manifest-analyzer explain --root overlays/prod --format mermaid # tiers 1-3 (§3)
-manifest-analyzer explain --object Deployment/web --format mermaid # the inverse (§4)
-```
-
-Three zoom levels, because **mermaid does not scale** and pretending otherwise wastes the
-feature. A 200-app monorepo has thousands of objects; a single graph of it is an
-unreadable hairball that no browser will lay out. The repo map is tens of nodes (one per
-candidate). A render-root map is tens to low hundreds. An object's ancestry is a handful.
-There is no fourth level, and "the whole repo's objects" is not a diagram, it is a denial
-of service.
-
-Two implementation notes that are easy to get wrong:
-
-- **Node labels are user data.** Paths, resource names and kustomization stanzas come from
- the scanned repo and land in a client-rendered diagram. Sanitise IDs (index or hash them;
- never interpolate a path into a mermaid node id) and escape label text. A path containing
- a quote or a bracket must not be able to break — or extend — the diagram.
-- **The render-root map needs one retention change.** Today
- [`renderChains`](../../../internal/manifestanalyzer/override_chain.go) keeps only the
- override assignments and drops the `renderedObject`s. The graph needs those objects kept.
- That is the same change [render-attribution.md](render-attribution.md) §7 step 1 already
- proposes for the oracle — so the diagram rides on it for free rather than justifying it
- alone.
-
-## 9. Order of work
-
-1. **Repo map from `RepoCandidate`** — pure rendering over an existing, already-JSON
- struct. No new analysis, no dependency on the render-attribution work. This is the piece
- that is worth doing *now*, and it is the one users see first.
-2. **Render-root map** — after the `renderedObject` retention change lands for the oracle.
- Tiers 1–3, with the three-line-style legend.
-3. **Object ancestry / the inverse view** — the §4 picture. This one is worth waiting for,
- because it should be generated from the *real* attribution (the dye) rather than from
- our re-implementation's guess. A diagram sourced from
- [`simulateImageRender`](../../../internal/manifestanalyzer/overrides_projection.go) would
- render our own bugs as if they were kustomize's behaviour.
-4. **Refusal colouring everywhere** — fold `DiagReason` and `RefusalReason` into every
- level. Cheap once the nodes exist.
-
-## Still open
-
-- **Do the docs get generated diagrams too?** The corpus fixtures are real repos. A CI step
- that regenerates a mermaid per fixture would make the support boundary *visibly*
- regress-tested — a diff in the picture is a diff in the boundary. Tempting; it also makes
- every fixture change a diagram review.
-- **Where does the user see it?** A CLI that prints mermaid is useful to us and to a
- motivated user pasting into a viewer. It is not yet a product surface. If the answer is
- eventually "in the UI", the graph should be emitted as data (nodes + typed edges + the
- fidelity of each edge) and rendered client-side — mermaid then becomes one renderer, not
- the format.
-- **Should tier 3 be shown at all at the object level?** The dashed "may have shaped" edge
- is honest, but on a real overlay it connects nearly every kustomization to nearly every
- object, and a graph where everything is dashed-connected to everything teaches nothing.
- Possibly it should only appear when the user asks about a specific object (§4), where the
- fan-out is bounded and the ambiguity is the point.
-- **Upstream.** A `BuildObserver`-style hook around the generator/transformer loop would
- give exact per-field provenance and collapse tiers 3 and 4 into one exact tier. That is
- worth *proposing* upstream ([render-attribution.md](render-attribution.md) §4 argues the
- fork is not), but nothing here should be blocked on it.
diff --git a/docs/design/support-boundary/next-prompt-simple-patches.md b/docs/design/support-boundary/next-prompt-simple-patches.md
new file mode 100644
index 00000000..8b84f07c
--- /dev/null
+++ b/docs/design/support-boundary/next-prompt-simple-patches.md
@@ -0,0 +1,168 @@
+# Prompt: tolerate `patches:`, and route a simple one
+
+Copy everything below the line into a fresh session.
+
+---
+
+Continue the kustomize support-boundary workstream. The next thing is `patches:`, and it
+comes in two halves that must not be confused: **tolerating a patch** (accept the folder,
+mirror what it renders) and **authoring a patch** (write one from scratch). The first is the
+job. The second stays deferred and this change must not drift into it.
+
+## Read first
+
+- `AGENTS.md`.
+- The memory note `kustomize-renderer-workstream`.
+- **`docs/design/support-boundary/render-root-scoping.md` §6** — this is the design. It is
+ titled "Why a patch still blocks the folder, and why it should not", and it already
+ separates the two gates you need: *renderable* (per folder) and *routable* (per object,
+ per field).
+- `docs/design/support-boundary/render-attribution.md` §3 (the dye and its sink proof), §5
+ (*attribution may be heuristic, verification may not*), §7 (the guardrails).
+
+## Where things stand
+
+PR #233 shipped the projection swap. Attribution is a **dyed render**: a nonce is written
+into every declared override entry, the root is rendered a second time, and the entry a value
+came from is read off the output. Verification is a **real re-render** (`VerifyBatchRenders`):
+before any kustomize-governed flush is committed, the whole tree is rebuilt with the write
+applied, every written document must reproduce its live object, and **every object the flush
+did not write must come out byte-identical**. A proposal that fails refuses the flush and
+names the file and the object.
+
+The re-implemented transformers (`renderImage`, `imageSuppliers`, `simulateImageRender`,
+`isReplicaKind`) are gone. Nothing left in the write path models kustomize; it asks kustomize.
+
+## Why patches, and why now
+
+Counted across the layout corpus, `patches` is **the single biggest refusal cause**: 8
+occurrences, against `namePrefix` 6 and `configMapGenerator` 6. And it refuses the **whole
+GitTarget**, not the edit. `flux-monorepo/apps/production`, whose patch touches replicas and
+an env var, therefore also loses `images:`/`replicas:` edit-through, which the patch has
+nothing to do with.
+
+`render-attribution.md` §3 names the blocker in as many words: *"Per-field refusal requires
+per-field attribution. The dye is the mechanism that milestone is waiting for."* That
+mechanism now exists. This is the milestone it unblocks.
+
+## Four things measured, not assumed. Do not re-derive them; do extend them.
+
+1. **A folder containing `patches:` builds fine.** kustomize renders it without complaint.
+ The refusal is *our fence*, not kustomize's. (Probe: add a `patches:` block to an
+ `imageFixture`-style tree and call `renderRoot`. No build failure.)
+
+2. **Patches run BEFORE `images:`/`replicas:`, and the transformers WIN.** Measured with a
+ patch and an entry both aimed at the same fields:
+
+ ```
+ patch says image app:patched, images: says newTag v2 -> renders app:v2
+ patch says replicas 7, replicas: says count 3 -> renders 3
+ ```
+
+ **A patch that sets a field an entry also governs is DEAD TEXT.** This is the fact that
+ makes "just edit the patch" wrong, and it is the first thing to get right.
+
+3. **The dye already answers both cases correctly, with no model of patches.** Where the entry
+ still matches, the dye lands and the value is attributed to the *entry* (the patch's value
+ is dead text). Where the patch changes the image *name*, the entry stops matching, no dye
+ lands, nothing is attributed, and the oracle refuses the write. Neither outcome required
+ knowing what a patch is.
+
+4. **A patch-owned field is currently unroutable, and fails safe.** No dye, so no attribution,
+ so nothing routes to an entry; the write falls back to the source document, the re-render
+ shows the patch overriding it, and the flush is refused. Wrong *outcome* for the user,
+ right *direction*: it refuses rather than corrupting.
+
+## The one thing not to get wrong
+
+**Do not read the patch and reason about who wins.** That is re-implementation wearing a
+better hat, and (2) is exactly the ordering rule you would get wrong. The patch is a sparse
+KRM document, so its fields are trivially readable, and that is precisely the trap: reading
+them tells you what the patch *asks for*, never what the build *does*.
+
+**Ask kustomize.** Extend the dye to scalars inside the patch: write a nonce into the patch's
+value, render, and see whether it survives to the output. If it does, the patch supplies that
+field and an edit belongs in the patch. If it does not, something downstream overrode it, and
+the dye that *did* land tells you who. Ordering, precedence and dead text all fall out for
+free, and none of it has to be modelled.
+
+**The sink proof is not optional.** §3: a dye is sound only where the dyed value is a **pure
+sink**, never an input to a matcher. Inside a patch this bites harder than it does for images:
+
+- **a list merge key is a selector, not a sink.** `containers[].name` is how strategic merge
+ decides *which* container to merge into. Dye it and the patch merges into the wrong element,
+ or creates a new one. Never dye a merge key. Enumerate the ones you rely on and say why each
+ is safe.
+- `metadata.name` / `metadata.namespace` in the patch body are the object selector. Same rule.
+- `$patch: delete` / `$patch: replace` directives change what the merge *means*. A dye near one
+ is not a sink.
+
+**Baseline first, then dye** (§7): if the dyed build errors where the real one did not, the dye
+hit something that is not a sink. Fall back to **NO ATTRIBUTION**, never to another heuristic.
+
+## Order of work. Each stage is independently shippable; the risky one is last.
+
+1. **Tolerate.** Stop refusing the folder for `patches:`. It comes out of the unsupported set
+ for *acceptance*; it stays unsupported for *authoring*. Nothing routes to a patch yet, so a
+ patch-owned edit is refused by the oracle exactly as it is today, with a message that says
+ so. **Regenerate the corpus baseline and read it**: this is the stage whose whole point is
+ the rows that move, and it is where you find out what tolerating patches actually exposes
+ (`flux-monorepo` should go from Partial to more-accepted; look at what else it drags in).
+ *Ship this alone if the rest slips.* It converts 8 whole-folder refusals into per-field
+ ones.
+
+2. **Attribute.** Extend the dye to patch scalars, with the merge-key guard above. A field the
+ patch supplies is now attributable to `(patch file, field path)`.
+
+3. **Route.** A live change to a patch-supplied scalar edits the **patch document**, at the
+ same path, preserving its bytes and comments (this is `manifestedit` territory, the same
+ way `PatchKustomization` preserves hand-authoring). The oracle proves it or refuses it.
+
+4. **Refuse per field, not per folder.** Name the patch that owns the field and say that
+ authoring is not supported. This is the tier-2 accounting in
+ `unreflectable-edits-and-write-gating.md`.
+
+## Scope: what "simple" has to mean, and it must be enforced, not assumed
+
+Start with **one** shape and refuse the rest by name:
+
+- a **single** strategic-merge patch per object (two patches touching one field is a precedence
+ question you have not earned yet);
+- **scalar** fields (`spec.replicas`, an env value, a resource limit). A change *inside a list*
+ is merge-key semantics and is a different problem;
+- `patches:` with a **`path:`** to a sparse KRM document. Inline `patch: |` and **JSON6902**
+ (`op`/`from`/`value`) are not sparse KRM at all and should be refused explicitly rather than
+ fall through.
+
+Check what `FixKustomization` does with the deprecated `patchesStrategicMerge` and
+`patchesJson6902` spellings before you write the parser: the analyzer already runs it
+(`kustomization_parse.go`), and folded fields will arrive in `Patches` looking like something
+they are not. **Measure it; do not assume.**
+
+## The test net
+
+The 12 `TestSplitDesired_*` tests now build a real tree, render it with kustomize, read the
+attribution off a dyed render, and drive the projection with the result. Extend that harness;
+do not build a second one. The corpus invariant to hold onto is
+`TestProjection_InSyncCorpusFolderIsANoOp`: **an in-sync folder must project to a complete
+no-op.** A patch that we mis-attribute will show up there as a phantom edit, across every
+fixture in both corpora.
+
+Add the ordering fact (2) as a fixture. It is the one a future refactor will break.
+
+## Validation and delivery
+
+Full sequence per `AGENTS.md`: `task fmt` → `generate` → `manifests` → `vet` → `lint` →
+`test` → `test-e2e` (needs Docker; check `docker info`; run sequentially). Regenerate
+`task gitops-layouts-baseline` and **explain every row that moves** — in stage 1 the moving
+rows are the deliverable. Branch off `main`, push, open a PR, and report the honest line
+delta, including when it is unflattering.
+
+## How this workstream finds bugs
+
+Every stage of it has found a real, shipped bug, and not one came from reading kustomize's
+source. They came from making kustomize the arbiter and probing it with a throwaway test: the
+regex image matcher, the missing `ReplicationController`, the digest that silently clears the
+tag, the `int` that panics `DeepCopyJSON`, and the patch-versus-transformer ordering above.
+
+**When you want to know what kustomize does, do not reason about it. Ask it.**
diff --git a/docs/design/support-boundary/patching-kustomize.md b/docs/design/support-boundary/patching-kustomize.md
deleted file mode 100644
index 8f96ef30..00000000
--- a/docs/design/support-boundary/patching-kustomize.md
+++ /dev/null
@@ -1,262 +0,0 @@
-# Patching kustomize: there is no seam, and there are thirty lines that would be one
-
-> **design** — direction-setting; ships no code. Nothing it describes is supported today.
-> Captured: 2026-07-14
-> Related:
-> [render-attribution.md](render-attribution.md),
-> [generated-repo-map.md](generated-repo-map.md),
-> [render-root-scoping.md](render-root-scoping.md),
-> [kustomize-support-boundary.md](kustomize-support-boundary.md),
-> [support-contract.md](support-contract.md)
-
-> **This doc revises [render-attribution.md](render-attribution.md) §4.** That section rules
-> out "get the DAG out of kustomize" on the grounds that it means a fork, and that a fork
-> means re-implementation. The first half is right. **The second half is wrong**, and the
-> measurement below is why: the change is ~30 lines in one file, it cannot alter rendered
-> output, and it yields the field-level attribution the dye was invented to approximate.
-
-Three routes to "make kustomize tell us more": **(a)** upstream it, **(b)** carry a patch,
-**(c)** stay outside and be clever. This doc measures all three against the checkout in
-`external-sources/kustomize`.
-
-## 1. Route (c) is dead. There is no seam.
-
-Not "awkward" — absent. Three independent walls, any one of which is fatal:
-
-| Hoped-for seam | What the code says |
-|---|---|
-| A hook on `krusty.Options` | The struct has **exactly four fields**: `Reorder`, `AddManagedbyLabel`, `LoadRestrictions`, `PluginConfig` (`api/krusty/options.go:22-47`). No callback, observer, or listener. `MakeKustomizer` hardcodes its `DepProvider`. |
-| Register a Go transformer that wraps the builtins | The builtin list is a **hardcoded slice literal** (`api/internal/target/kusttarget_configplugin.go:69-91`), resolved against factory maps in `api/internal/plugins/builtinhelpers`. `api/internal/...` is unreachable from our module by Go's internal rule. There is no `RegisterTransformer` of any kind; `PluginConfig` has four fields and none is a registry. |
-| A custom plugin that *observes* the build | Custom transformers are appended **after** every builtin, as peers in one flat slice (`kusttarget.go:330-345`). A plugin never sees a pre-builtin state and cannot intercept one. |
-
-So the choice is genuinely binary: **patch kustomize, or accept resource-level "ran over"
-semantics forever.** Everything clever we could do from outside — the dye
-([render-attribution.md](render-attribution.md) §3), leave-one-out probing, N+1 builds — is
-a workaround for this absence, not an alternative to it.
-
-## 2. The one seam that would work is already half-built
-
-Every builtin transformer runs through one loop, `multiTransformer.Transform`
-(`api/internal/target/multitransformer.go:27-41`):
-
-```go
-for _, t := range o.transformers {
- if err := t.Transform(m); err != nil { return err }
- if t.Origin != nil {
- if err := m.AddTransformerAnnotation(t.Origin); err != nil { return err }
- }
- m.DropEmpties()
-}
-```
-
-`AddTransformerAnnotation` then walks **every resource in the map, unconditionally**
-(`api/resmap/reswrangler.go:526-550`). That single unconditional loop is precisely where
-"ran over" gets baked in — and it has **exactly one production call site**, the line above.
-
-Everything needed to do better already exists and is already used this way elsewhere:
-
-- `ResMap.DeepCopy()` (`api/resmap/reswrangler.go:377`) — and `kusttarget.go:385-390`
- **already deep-copies the ResMap before and after running a validator and compares it.**
- The pattern we want is upstream's own pattern, applied one loop over.
-- `resource.AsYAML()` (`api/resource/resource.go:382`) for content equality; `ResMap.GetById()`
- (`reswrangler.go:214`) matches across renames via `PrevIds()`.
-- The whole cost is gated on `t.Origin != nil`, and origins are only constructed when
- `len(BuildMetadata) != 0` (`kusttarget.go:131-135`). **A default `kustomize build` pays
- literally zero.**
-
-Snapshot before, diff after, annotate only what changed: ~30 added lines, one file, no
-interface change. And `multitransformer.go` has been touched **seven times in its life,
-most recently in January 2022** — a four-year-stable file.
-
-## 3. The decisive fact: one transformer instance per entry
-
-This is the finding that changes the shape of the argument, and it is easy to miss.
-
-Kustomize does **not** build one `ImageTagTransformer` holding all your images. It builds
-**one per `images:` entry**, in file order (`kusttarget_configplugin.go:412-429`):
-
-```go
-for _, args := range kt.kustomization.Images {
- c.ImageTag = args
- p := f()
- ...
- result = append(result, p) // one transformer instance per entry
-}
-```
-
-And it is the rule, not an images-only quirk:
-
-| Stanza | Instances | Cite |
-|---|---|---|
-| `images:` | **one per entry** | `kusttarget_configplugin.go:419` |
-| `replicas:` | **one per entry** | `:455` |
-| `patches:` | **one per entry** | `:270-278` |
-| `labels:` | **one per entry** | `:294` |
-| `replacements:` | *one instance for all* — the exception | `:439-441` |
-
-Therefore the before/after diff in §2 is **not merely "did this transformer change
-anything."** Because the loop iterates *entries*, a structural field-path diff inside it
-yields, exactly:
-
-> `overlays/prod/kustomization.yaml` → `images[1]` → set
-> `Deployment/web` `spec.template.spec.containers[0].image` → `web:v2`
-
-That is **tier 4** — the field-level attribution that
-[generated-repo-map.md](generated-repo-map.md) §2 records as *"does not exist"* and that
-[render-attribution.md](render-attribution.md) is entirely about approximating. It does not
-exist in kustomize's **output**. It is one structural diff away from existing in kustomize's
-**execution**. Upstream never exposed it because it never needed it: `transformerOrigin` is
-built once per transformer *type* and shared across every instance
-(`kusttarget_configplugin.go:88-101`), so the annotation is structurally incapable of
-naming an entry even in principle. The information is there at runtime; only the reporting
-throws it away.
-
-## 4. What this does to the dye
-
-The dye is a good idea born of a false constraint. Compare honestly:
-
-| | The dye (render-attribution §3) | The patched loop |
-|---|---|---|
-| Attribution | inferred from a nonce surviving into the output | **observed directly** |
-| `newTag`, `digest`, `replicas` | works | works |
-| `newName` | **cannot work** — it is the join key, not a sink | works |
-| `patches:` | no | **works** (one instance per patch) |
-| Charset constraints | **a correctness requirement** — the nonce must survive a regex over the whole image string | none |
-| Builds per render | 2 | 1 |
-| Failure mode | silent mis-attribution if a nonce collides or is rewritten | none of that class |
-
-The dye's central caveat — *sound only for pure sinks* — is a consequence of standing
-outside the process and inferring. From inside the loop there are no pure-sink
-restrictions, because nothing is being inferred.
-
-**But be precise about what this buys, because it is easy to overclaim: attribution is
-necessary for reversal, not sufficient.** Knowing that `patches[0]` set `spec.replicas: 3`
-tells us where the value came from. It does not tell us how to *edit the patch* so it
-produces `5` — for a scalar set that is mechanical, for a strategic merge with list
-semantics it is not. So this does not make `patches:` supported. It removes the reason we
-cannot even *see* what a patch did, which is the first of several locks on that door. The
-oracle in [render-root-scoping.md](render-root-scoping.md) §3 — re-render and require the
-proposal to reproduce the live object exactly — remains the gate, and remains necessary.
-
-## 5. Fork risk: this is an observability fork, not a semantics fork
-
-The standing objection to forking kustomize is exactly right in general and does not apply
-here, and the distinction is worth naming precisely, because it is the whole argument.
-
-**Our correctness contract is: render what the user's controller renders.** A fork that
-drifts from that is not merely a maintenance cost, it is a correctness hazard — we would
-propose writes against a render nobody actually deploys.
-
-The measured skew today is **zero**:
-
-| | kustomize `api` | how |
-|---|---|---|
-| Us | **v0.21.1** | `go.mod:33-34` |
-| Flux | **v0.21.1** | `fluxcd/pkg/kustomize@v1.32.0` pins *and `replace`s* it — the controller links `krusty` with `LoadRestrictionsNone` + `DisabledPluginConfig()`, **byte-identical options to ours** |
-| Argo (default) | **v0.21.1** | execs the `kustomize` **binary**, shipped at 5.8.1, which pins api v0.21.1 |
-
-Now the key property: **the patch is structurally incapable of changing rendered content.**
-
-- It only writes **annotations** — and only `config.kubernetes.io/origin` /
- `alpha.config.kubernetes.io/transformations`, which our renderer already **strips** before
- anything reaches Git ([`kustomize_render.go`](../../../internal/manifestanalyzer/kustomize_render.go),
- `collectRendered`).
-- It only runs when `BuildMetadata` is non-empty — a path **upstream users never take** and
- **we always take** (we inject it into our in-memory copy of the root). Flux does not set
- it; Argo does not set it.
-- It touches no transformer, no fieldspec, no merge logic. The object graph is untouched.
-
-A fork is dangerous when it changes the thing you must match. This one cannot reach it.
-Those are different risk classes and should not be priced the same.
-
-*(Aside, and not caused by this: Argo is the real fidelity problem, and it is unrelated to
-forking. It execs a user-swappable binary, at a user-chosen version, and defaults to
-`LoadRestrictionsRootOnly` where we and Flux use `LoadRestrictionsNone` — so there exist
-repos we render and Argo refuses. That belongs in its own doc.)*
-
-## 6. The cost that is actually real: `replace` does not compose
-
-The maintenance cost of the patch is small (one four-year-stable file; rebase when Flux
-bumps). The cost that will bite is subtler:
-
-**A `replace` directive is honoured only in the main module.** It is ignored when our code
-is consumed as a library. We *have* a public library — `pkg/manifestanalyzer` — so a third
-party importing it would silently link **upstream** kustomize, the patched loop would not
-exist, the trace would come back empty, and attribution would **degrade silently to
-nothing** rather than fail.
-
-That failure mode is unacceptable and it is cheap to close: the analyzer must **probe for
-the patched build at startup and fail loudly** if the trace hook is absent, rather than
-quietly falling back to guesswork. Any design that carries this patch must carry that probe
-with it. (This also argues for keeping the dye's *verification* half —
-re-render-and-compare — regardless: it is the check that catches exactly this.)
-
-## 7. Upstream: not hostile, but slow, and the goldens are against us
-
-The resource-level half of this is genuinely framable as a **bug fix**, not a feature —
-upstream's own documentation already describes the semantics we want while the code
-implements the other one:
-
-- `site/content/en/docs/Tasks/build_metadata.md:259` — *"the transformer that **updated** the
- resource"* (what we want).
-- Same file, `:214` and the proposal — *"transformers that have **acted on** them"* / *"**touched**
- each resource"* (what the code does).
-- And `:209-212` states the annotation is **alpha**: *"We are not guaranteeing that the
- annotation content will be stable during alpha, and reserve the right to make changes."*
-
-That is a strong opening. The countervailing facts are equally concrete:
-
-- **The existing goldens assert the current semantics.** `api/krusty/transformerannotation_test.go`
- has a `Namespace` object carrying two `PrefixTransformer` entries despite the prefix
- transformer excluding namespaces by fieldspec. Our patch deletes those annotations — i.e.
- it rewrites tests written by the feature's own author.
-- **kustomize is vendored into `kubectl`.** Changing `buildMetadata` output changes
- `kubectl kustomize` output, which per `proposals/README.md` pushes toward a full KEP.
-- **`CONTRIBUTING.md:216-218`**: a feature PR is not reviewable without a triaged/accepted
- issue first.
-- **Staffing.** `ROADMAP.md` is titled *"Kustomize roadmap 2023-2024"*, is largely about
- understaffing, and says of this very feature area that *"due to limited staffing, we have
- been unable to drive this feature out of alpha."* The annotation has been alpha for four
- years and nobody has promoted it.
-- **The precedent's price tag.** `buildMetadata` itself shipped as a **469-line in-repo
- proposal** followed by 3–5 PRs of 500–800 lines each, authored by the project owner.
-
-Field-level attribution (§3) is a larger ask than the resource-level fix, with **zero
-in-repo precedent** — the original proposal scoped itself to resource granularity
-deliberately, and nothing about field-level provenance exists in the tree.
-
-So: propose it, but do not plan around it landing.
-
-## 8. Recommendation
-
-1. **Carry the patch** (`replace` directive, ~30–50 lines in `multitransformer.go`),
- emitting a per-(entry, resource) changed-field-path trace. It cannot alter rendered
- content (§5), and it is the only route to tier 4 (§3).
-2. **Ship the loud probe** (§6) in the same change. Silent degradation to no-attribution is
- worse than not having the feature.
-3. **Keep the oracle regardless** — re-render and require byte-identical reproduction
- ([render-root-scoping.md](render-root-scoping.md) §3). Attribution may be observed;
- verification must still be independent. A patched loop that we ourselves wrote is not
- permitted to be its own witness.
-4. **File the upstream issue in parallel**, framed as a bug fix against
- `build_metadata.md:259`, with a regression test as a separate first commit per
- `CONTRIBUTING.md:226-235`. Treat acceptance as upside. If it lands, our `replace`
- evaporates — which is the quiet virtue of an observability-only patch: it is
- forward-compatible with its own obsolescence.
-5. **Demote the dye** from *the* attribution mechanism to the fallback for an unpatched
- build — and keep its verification half permanently.
-
-## Still open
-
-- **Does the trace escape the process, or stay a side-channel?** Annotating per-field
- provenance onto the resources themselves would bloat output and change what
- `RemoveBuildAnnotations` has to strip. A side-channel (`Kustomizer` returning a trace
- alongside the `ResMap`) is cleaner for us but is a public API change, which makes the
- upstream story harder. These two goals pull in opposite directions and the doc does not
- resolve it.
-- **`replacements:` stays coarse** (one instance for all — §3 table). We refuse them today,
- so it costs nothing now; it would need an index inside the transformer to fix later.
-- **Which version do we fork from, and what happens when Flux bumps?** Today the answer is
- trivially v0.21.1 because all three of us are there. The first divergence between Flux's
- pin and Argo's shipped binary is the moment this question gets a real answer, and we
- should decide *then* whether we track Flux or track the user.
diff --git a/docs/design/support-boundary/render-attribution.md b/docs/design/support-boundary/render-attribution.md
index e1098679..f275edc6 100644
--- a/docs/design/support-boundary/render-attribution.md
+++ b/docs/design/support-boundary/render-attribution.md
@@ -1,25 +1,33 @@
# Render attribution: which override entry supplied this value?
-> **design** — direction-setting; ships no code. Captured: 2026-07-14
+> **design** — direction-setting. Captured: 2026-07-14
> Related:
> [kustomize-support-boundary.md](kustomize-support-boundary.md) §7 — the decision to embed the renderer ·
> [render-root-scoping.md](render-root-scoping.md) — the oracle, and §6's tolerate-don't-author plan ·
+> *patching-kustomize.md* (ConfigButler/kustomize-tracer, `plans/`) — **revises §4**: the fork is ~30 lines, and it is built ·
> [finished/images-and-replicas-edit-through.md](finished/images-and-replicas-edit-through.md) — what shipped ·
> [unreflectable-edits-and-write-gating.md](unreflectable-edits-and-write-gating.md)
-
-The workstream that replaced our hand-written kustomize with the real one has one step
-left, and it is the load-bearing one: **deleting the re-implemented transformers the
-write path still uses to decide which file an edit belongs in** —
-[`renderImage`, `imageSuppliers`, `simulateImageRender`, `isReplicaKind`](../../../internal/manifestanalyzer/overrides_projection.go),
-~400 lines.
-
-That needs an answer to a question the renderer does not answer. This document states
-the approach we had settled on, a second approach that is better, and a third that keeps
-getting proposed and cannot work — each **measured against kustomize v0.21.1**, not
-argued from its source.
-
-Everything marked *measured* was run. §6 is the bug ledger the measuring produced; it is
-longer than expected, and one of its entries blocks the PR that is currently open.
+>
+> **Status: shipped (§7 stages 1-3).** `renderRootWith` is the counterfactual primitive;
+> verification is a real re-render (`VerifyBatchRenders`); attribution is the dye (`dye.go`,
+> `overrides_attribution.go`). `renderImage`, `imageSuppliers`, `simulateImageRender` and
+> `isReplicaKind` are deleted, and **B1, B2 and B3** with them. Stage 4 (chain to set) is not
+> done.
+>
+> Building it corrected two things, both marked below: **B4 is worse than recorded** (a
+> rendered object is not a valid `unstructured` at all; `DeepCopyJSON` *panics* on the Go
+> `int` kustomize returns), and **the rename-chain guard must use kustomize's compiled regex,
+> not the string equality §3 proposed** (an entry `name:` is a regex, so `mirror/ap.` matches
+> `mirror/app` without equalling it).
+>
+> §5's verdict, *attribution may be heuristic, verification may not*, is what made the rest
+> safe to delete. But see *patching-kustomize.md* (ConfigButler/kustomize-tracer, `plans/`):
+> the dye is now the **fallback** for an unpatched build, not the destination.
+
+This document is the record of how attribution was decided: the approach we had settled on,
+a second that is better, and a third that keeps getting proposed and cannot work. Each is
+**measured against kustomize v0.21.1**, not argued from its source. §6 is the bug ledger the
+measuring produced.
---
@@ -109,11 +117,11 @@ collide.** Attribution by "what changed" is blind to a writer whose write is inv
because something else wrote the same bytes — and "the overlay pins the tag the base
already has" is not a corner case, it is the steady state of a GitOps repo.
-Today's [`renderImage`](../../../internal/manifestanalyzer/overrides_projection.go) gets
-both right, because it attributes by *position in the chain* (last matching entry wins) and
-never compares values. So Approach A is not merely imperfect — **shipping it would regress
-behaviour that works today**, in exchange for deleting the code that makes it work. Worth
-saying plainly, because the design was settled before the measurement contradicted it.
+The `renderImage` this replaced got both right, because it attributed by *position in the
+chain* (last matching entry wins) and never compared values. So Approach A was not merely
+imperfect: **shipping it would have regressed behaviour that already worked**, in exchange
+for deleting the code that made it work. Worth saying plainly, because the design was settled
+before the measurement contradicted it.
---
@@ -226,9 +234,9 @@ this field"**. A `patches:` entry (or a `replacements:` block) that clobbers the
field *erases the dye*. No dye, and the value differs from source → an unmodelled owner
exists → refuse, don't route.
-Today's code cannot make that distinction. `simulateImageRender` "verifies" the inversion
-against a simulation that **shares its own blind spot**, so a value owned by a patch is
-confidently written into a file that does not own it. That is invisible only because
+The code this replaced could not make that distinction. `simulateImageRender` "verified" the
+inversion against a simulation that **shared its own blind spot**, so a value owned by a patch
+was confidently written into a file that does not own it. That is invisible only because
`patches:` currently refuses the whole folder — and
[render-root-scoping.md §6](render-root-scoping.md) is a plan to **stop doing that**:
tolerate patches as read-only context, refuse per *field* instead of per folder. Per-field
@@ -334,6 +342,13 @@ is checking for.
## 6. The bug ledger: what the probes found
+> **All of it is closed.** P1 and C1 shipped in #232. **B1, B2 and B3 are not fixed — they are
+> DELETED**, along with the `renderImage` / `isReplicaKind` / fieldspec code that caused them
+> (see [UPGRADING.md](../../UPGRADING.md)). B4 was found to be worse than recorded and is
+> handled by a type switch (`renderedReplicaCount`) plus a JSON normalisation in the tests.
+> The ledger is kept as written, because *how* each was found is the argument for the method:
+> not one came from reading kustomize's source. Every one came from asking it.
+
Every stage of this workstream found a shipped bug by making kustomize the arbiter. Writing
this document found more — none by reading code, all from ~60 lines of throwaway probe
against the existing `imageFixture` harness. **P1 and C1 block #232.**
diff --git a/docs/design/support-boundary/render-root-scoping.md b/docs/design/support-boundary/render-root-scoping.md
index f3de515c..fcb292e0 100644
--- a/docs/design/support-boundary/render-root-scoping.md
+++ b/docs/design/support-boundary/render-root-scoping.md
@@ -39,17 +39,21 @@ code:
> live object exactly — and leave every other object in the build byte-identical — refuse
> the write.**
+> **This shipped.** It is [`VerifyBatchRenders`](../../../internal/manifestanalyzer/render_verify.go),
+> run as a write-plan precondition once per flush. See
+> [render-attribution.md](render-attribution.md) §5.
+
This is not a new idea imported from outside. F1 shipped the *shape* of it in
-[`simulateImageRender`](../../../internal/manifestanalyzer/overrides_projection.go): propose
-the entry edits, replay, and discard the whole inversion unless every planned source image
-comes back as its live value.
+`simulateImageRender`: propose the entry edits, replay, and discard the whole inversion
+unless every planned source image comes back as its live value.
But be precise about how far short of the statement above that falls, because the gap is the
-work: `simulateImageRender` replays **our re-implemented image chain, not kustomize**, and it
-checks **only the images it planned** — never that the rest of the build is untouched. So it
-is image-specific verification, and it shares a blind spot with the thing it is verifying: a
+work: `simulateImageRender` replayed **our re-implemented image chain, not kustomize**, and it
+checked **only the images it planned** — never that the rest of the build is untouched. So it
+was image-specific verification, and it shared a blind spot with the thing it verified: a
value owned by a patch, or by a matching rule we got wrong, reproduces perfectly in the
-simulation and wrongly in reality. F1 shipped the pattern. **What F1 did not have is a
+simulation and wrongly in reality. (It is deleted; the re-render replaced it.) F1 shipped the
+pattern. **What F1 did not have is a
renderer** — and without one, neither half of the guarantee above is actually in force. See
[render-attribution.md](render-attribution.md) §5.
@@ -72,8 +76,8 @@ renderer** — and without one, neither half of the guarantee above is actually
| Piece | What it is | Status |
|---|---|---|
| [`renderRoots`](../../../internal/manifestanalyzer/override_chain.go) | every kustomization directory no other kustomization references | **kept** — something must decide which directories a build is invoked on. Everything the walk did *beyond* that now comes from the renderer. |
-| [`renderImage`](../../../internal/manifestanalyzer/overrides_projection.go) | a ~20-line reimplementation of kustomize's image transformer | **still there.** Measured to diverge from kustomize: its matcher is string equality where kustomize's is a *regex over the whole image string*. |
-| [`isReplicaKind`](../../../internal/manifestanalyzer/overrides_projection.go) | the replica transformer's fieldspec, hardcoded to three kinds | **still there.** kustomize's fieldspec has four — it misses `ReplicationController`. |
+| `renderImage` | a ~20-line reimplementation of kustomize's image transformer | **deleted.** It diverged from kustomize (its matcher was string equality where kustomize's is a *regex over the whole image string*). Attribution now reads a dyed render: [render-attribution.md](render-attribution.md) §3. |
+| `isReplicaKind` | the replica transformer's fieldspec, hardcoded to three kinds | **deleted.** kustomize's fieldspec has four; it missed `ReplicationController`. There is no list of kinds any more. |
| [`unsupportedKustomizeFeatureKeys`](../../../internal/manifestanalyzer/store.go) | 17 keys that refuse the folder outright | **replaced** (#229): the unsupported set is now derived by *reflecting over kustomize's own struct*, so a field we have never heard of refuses rather than being silently tolerated. |
The deny-list is not a statement about what is editable. **It is a fence around the
@@ -321,10 +325,10 @@ silently lost**. That is the gate on all of this, and it is the right one.
1. **The minimal-overlay fixture.** Until it exists, the overlay code path is unobserved.
2. **Tier-2 accounting** — `FullyReflected` per edit; refused edits reported and reverted.
-3. **The oracle**: krusty, sandboxed, in the acceptance gate and in the write-plan
- precondition. Ship it first against `images`/`replicas`, where it must agree with
- `simulateImageRender` on every corpus fixture — a free differential test of the thing
- we are about to trust.
+3. ~~**The oracle**~~ — **done.** krusty, sandboxed, in the acceptance gate (#232) and in the
+ write-plan precondition (`VerifyBatchRenders`). The differential test against
+ `simulateImageRender` was overtaken: the simulation is deleted, and the corpus test that
+ replaced it makes the stronger claim that an in-sync folder projects to a no-op.
4. **Render-root scoping proper**: read `../../base`; bases become declared read-only
context; generalise `fanInPrecondition` to any file reachable from more than one render
root.
diff --git a/internal/git/kustomize_delete_test.go b/internal/git/kustomize_delete_test.go
new file mode 100644
index 00000000..741d7f91
--- /dev/null
+++ b/internal/git/kustomize_delete_test.go
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package git
+
+import (
+ "context"
+ "os"
+ "path/filepath"
+ "testing"
+
+ gogit "github.com/go-git/go-git/v5"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+ "k8s.io/apimachinery/pkg/runtime/schema"
+
+ "github.com/ConfigButler/gitops-reverser/internal/types"
+ "github.com/ConfigButler/gitops-reverser/internal/typeset"
+)
+
+// Deleting a managed document is only half a delete when the file is named in a
+// kustomization's resources:. A resources: entry pointing at a file that no longer exists is
+// one kustomize refuses to build over —
+//
+// accumulating resources ... '/scan/api.yaml' doesn't exist
+//
+// — so the repository is left in a state no GitOps controller can deploy.
+//
+// The entry must come out. But ONLY when the file itself is actually gone: a file holding
+// several documents survives the deletion of one of them, and pulling its resources: entry
+// would then un-deploy every OTHER resource in it. That is the sharp edge here, and it is the
+// first test below.
+
+const deleteKustomizationYAML = `apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+namespace: default
+resources:
+ - web.yaml
+ - bundle.yaml
+`
+
+// bundle.yaml holds TWO documents. Deleting one of them must leave the file — and therefore
+// its resources: entry — in place, or the surviving document silently stops being deployed.
+const deleteBundleYAML = `apiVersion: v1
+kind: ConfigMap
+metadata:
+ name: keep-me
+data:
+ a: "1"
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+ name: delete-me
+data:
+ b: "2"
+`
+
+// seedDeleteWorktree stages the files in git, not merely on disk: removing a file the index
+// has never seen fails with "entry not found", so a delete test that only wrote to the
+// filesystem would be testing the fixture rather than the writer.
+func seedDeleteWorktree(t *testing.T, worktree *gogit.Worktree, kustomization string) {
+ t.Helper()
+ seedPlacedManifest(t, worktree, "web.yaml", sharedImageDeploymentYAML("web"))
+ seedPlacedManifest(t, worktree, "bundle.yaml", deleteBundleYAML)
+ seedPlacedManifest(t, worktree, "kustomization.yaml", kustomization)
+}
+
+func deleteConfigMapEvent(name string) Event {
+ return Event{
+ Identifier: types.ResourceIdentifier{
+ Version: "v1", Resource: "configmaps", Namespace: "default", Name: name,
+ },
+ Operation: "DELETE",
+ }
+}
+
+// mergedMapper resolves both kinds the delete-plus-governed-write flush needs.
+func mergedMapper() typeset.Lookup {
+ return typeset.NewSnapshotRegistry(typeset.Snapshot{
+ Entries: []typeset.Entry{
+ {
+ GVK: schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
+ GVR: schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"},
+ Namespaced: true,
+ Allowed: true,
+ },
+ {
+ GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
+ GVR: schema.GroupVersionResource{Version: "v1", Resource: "configmaps"},
+ Namespaced: true,
+ Allowed: true,
+ },
+ },
+ })
+}
+
+// THE ONE THAT MATTERS. bundle.yaml holds two ConfigMaps; one is deleted. The file still
+// holds the other, so the file stays — and its resources: entry MUST stay with it. Removing
+// the entry here would un-deploy keep-me, a resource nobody asked to touch.
+func TestPlanFlush_DeletingOneDocumentOfAFileKeepsTheFileAndItsResourcesEntry(t *testing.T) {
+ writer := newContentWriter(types.SensitiveResourcePolicy{})
+ worktree := newWorktreeForTest(t)
+ root := worktree.Filesystem.Root()
+ seedDeleteWorktree(t, worktree, deleteKustomizationYAML)
+
+ changed, err := flushEventsForTest(t, writer, worktree, configMapMapper(),
+ deleteConfigMapEvent("delete-me"))
+
+ require.NoError(t, err)
+ require.True(t, changed, "the document must have been removed")
+
+ bundle, readErr := os.ReadFile(filepath.Join(root, "bundle.yaml"))
+ require.NoError(t, readErr, "the file must survive: it still holds another document")
+ assert.Contains(t, string(bundle), "name: keep-me", "the surviving document must be untouched")
+ assert.NotContains(t, string(bundle), "name: delete-me")
+
+ assertFileBytes(t, filepath.Join(root, "kustomization.yaml"), deleteKustomizationYAML,
+ "the file still exists, so its resources: entry must NOT be removed — "+
+ "pulling it would un-deploy keep-me")
+}
+
+// The file's LAST document is deleted, so the file goes. Its resources: entry now names a
+// file that does not exist, which kustomize refuses to build over — so the entry goes too,
+// and the repository stays deployable.
+func TestPlanFlush_DeletingTheLastDocumentAlsoRemovesTheResourcesEntry(t *testing.T) {
+ writer := newContentWriter(types.SensitiveResourcePolicy{})
+ worktree := newWorktreeForTest(t)
+ root := worktree.Filesystem.Root()
+ seedDeleteWorktree(t, worktree, deleteKustomizationYAML)
+
+ changed, err := flushEventsForTest(t, writer, worktree, configMapMapper(),
+ deleteConfigMapEvent("keep-me"),
+ deleteConfigMapEvent("delete-me"),
+ )
+
+ require.NoError(t, err)
+ require.True(t, changed)
+
+ _, statErr := os.Stat(filepath.Join(root, "bundle.yaml"))
+ assert.True(t, os.IsNotExist(statErr), "the file held nothing else, so it is gone")
+
+ kust, readErr := os.ReadFile(filepath.Join(root, "kustomization.yaml"))
+ require.NoError(t, readErr)
+ assert.NotContains(t, string(kust), "bundle.yaml",
+ "a resources: entry naming a file that no longer exists makes the folder unbuildable")
+ assert.Contains(t, string(kust), "web.yaml", "and every other entry is left exactly as it was")
+}
+
+// A delete inside a render root goes to the oracle, whether or not the file itself goes with
+// it. So if the resources: entry is ever left dangling — by a kustomization we cannot edit, or
+// by a future refactor that forgets to remove it — the re-render fails to build and the flush
+// is refused, rather than committing a repository kustomize cannot deploy.
+//
+// Raised in review on #233. It is verified here rather than argued: the delete flush below
+// runs the render precondition, which is what makes the dangling-entry case unreachable
+// instead of merely unlikely.
+func TestPlanFlush_DeleteInsideARenderRootIsVerified(t *testing.T) {
+ writer := newContentWriter(types.SensitiveResourcePolicy{})
+ worktree := newWorktreeForTest(t)
+ root := worktree.Filesystem.Root()
+ seedDeleteWorktree(t, worktree, deleteKustomizationYAML)
+
+ scan, err := scanWorktreeSubtree(root)
+ require.NoError(t, err)
+ batch := newWriteBatch(context.Background(), writer, configMapMapper(), scan, nil)
+ batch.applyDelete(context.Background(), deleteConfigMapEvent("delete-me"))
+
+ assert.True(t, batch.putToKustomize,
+ "the deleted document's file is named in resources:, so the flush must be put to kustomize")
+}
+
+// The delete lands in the same flush as a governed write. Both must survive: without the
+// entry removal the re-render cannot build the tree at all, and the oracle would refuse the
+// whole flush — losing the delete AND the unrelated image bump.
+func TestPlanFlush_DeleteAndGovernedWriteInOneFlush(t *testing.T) {
+ writer := newContentWriter(types.SensitiveResourcePolicy{})
+ worktree := newWorktreeForTest(t)
+ root := worktree.Filesystem.Root()
+ seedDeleteWorktree(t, worktree, deleteKustomizationYAML+`images:
+ - name: ghcr.io/example/shared
+ newTag: "1.0.0"
+`)
+
+ changed, err := flushEventsForTest(t, writer, worktree, mergedMapper(),
+ sharedImageEvent("web", "ghcr.io/example/shared:2.0.0"),
+ deleteConfigMapEvent("keep-me"),
+ deleteConfigMapEvent("delete-me"),
+ )
+
+ require.NoError(t, err, "the tree must still build with the delete applied, or the oracle refuses everything")
+ require.True(t, changed)
+
+ kust, readErr := os.ReadFile(filepath.Join(root, "kustomization.yaml"))
+ require.NoError(t, readErr)
+ assert.Contains(t, string(kust), `newTag: "2.0.0"`, "the governed write must land")
+ assert.NotContains(t, string(kust), "bundle.yaml", "and the dangling entry must be gone")
+}
diff --git a/internal/git/kustomize_oracle_test.go b/internal/git/kustomize_oracle_test.go
new file mode 100644
index 00000000..00e15183
--- /dev/null
+++ b/internal/git/kustomize_oracle_test.go
@@ -0,0 +1,304 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package git
+
+import (
+ "context"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ gogit "github.com/go-git/go-git/v5"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+ "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
+ "github.com/ConfigButler/gitops-reverser/internal/manifestanalyzer"
+ "github.com/ConfigButler/gitops-reverser/internal/types"
+ "github.com/ConfigButler/gitops-reverser/internal/typeset"
+)
+
+// flushEventsForTest is applyEventsViaPlanFlushWithMapper's sibling for the cases that are
+// ABOUT the error: a refused flush returns one, and swallowing it with require.NoError
+// would assert the opposite of the thing under test.
+func flushEventsForTest(
+ t *testing.T,
+ writer *contentWriter,
+ worktree *gogit.Worktree,
+ mapper typeset.Lookup,
+ events ...Event,
+) (bool, error) {
+ t.Helper()
+ w := &BranchWorker{contentWriter: writer, mapper: mapper}
+ return w.flushEventsToWorktree(context.Background(), worktree, "", events, nil)
+}
+
+// Before a kustomize-governed write is committed, the repository is re-rendered WITH it
+// and the result is required to be exactly the live object, with every other rendered
+// object untouched. These are the tests for the refusal — the happy path is already
+// covered by inplace_overrides_test.go, and a check that only ever says yes is not a
+// check.
+//
+// See docs/design/support-boundary/render-attribution.md §5.
+
+// A base pinned at one tag, an overlay that shadows it, and a SECOND deployment the same
+// images: entry also matches. Converging web's tag by editing the entry would drag api's
+// image along with it — the entry is shared context. The oracle re-renders, sees api move,
+// and refuses the whole write rather than committing a change to a resource nobody asked
+// to change.
+const sharedEntryKustomizationYAML = `apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+namespace: default
+resources:
+ - apps/web.yaml
+ - apps/api.yaml
+images:
+ - name: ghcr.io/example/shared
+ newTag: "1.0.0"
+`
+
+func sharedImageDeploymentYAML(name string) string {
+ return `apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: ` + name + `
+spec:
+ selector:
+ matchLabels:
+ app: ` + name + `
+ template:
+ metadata:
+ labels:
+ app: ` + name + `
+ spec:
+ containers:
+ - name: main
+ image: ghcr.io/example/shared:0.9.0
+`
+}
+
+func seedSharedEntryWorktree(t *testing.T, root string) (string, string, string) {
+ t.Helper()
+ webPath := filepath.Join(root, "apps", "web.yaml")
+ apiPath := filepath.Join(root, "apps", "api.yaml")
+ kustPath := filepath.Join(root, "kustomization.yaml")
+ require.NoError(t, os.MkdirAll(filepath.Join(root, "apps"), 0o750))
+ require.NoError(t, os.WriteFile(webPath, []byte(sharedImageDeploymentYAML("web")), 0o600))
+ require.NoError(t, os.WriteFile(apiPath, []byte(sharedImageDeploymentYAML("api")), 0o600))
+ require.NoError(t, os.WriteFile(kustPath, []byte(sharedEntryKustomizationYAML), 0o600))
+ return webPath, apiPath, kustPath
+}
+
+// sharedImageEvent is the live Deployment as the cluster holds it: the rendered form,
+// which is what the source file plus the overlay's entry must reproduce.
+func sharedImageEvent(name, image string) Event { //nolint:unparam // image varies by intent, not by call today
+ return Event{
+ Object: &unstructured.Unstructured{Object: map[string]interface{}{
+ "apiVersion": "apps/v1",
+ "kind": "Deployment",
+ "metadata": map[string]interface{}{"name": name, "namespace": "default"},
+ "spec": map[string]interface{}{
+ "selector": map[string]interface{}{"matchLabels": map[string]interface{}{"app": name}},
+ "template": map[string]interface{}{
+ "metadata": map[string]interface{}{"labels": map[string]interface{}{"app": name}},
+ "spec": map[string]interface{}{
+ "containers": []interface{}{
+ map[string]interface{}{"name": "main", "image": image},
+ },
+ },
+ },
+ },
+ }},
+ Identifier: types.ResourceIdentifier{
+ Group: "apps", Version: "v1", Resource: "deployments", Namespace: "default", Name: name,
+ },
+ Operation: "UPDATE",
+ }
+}
+
+// Routing web's new tag onto the entry would re-render api at that tag too, and api's live
+// state says otherwise. The proposal is not isolated, so the flush is refused — loudly, as
+// an AcceptanceRefusedError that reaches the GitTarget — and not one byte is written.
+//
+// A refusal, not a silent skip: a write that does not survive the re-render is one that
+// would never converge, and absorbing it would leave the resource un-mirrored forever with
+// nothing to show for it (render-attribution.md §7).
+func TestPlanFlush_RefusesAWriteThatDragsASiblingAlong(t *testing.T) {
+ writer := newContentWriter(types.SensitiveResourcePolicy{})
+ worktree := newWorktreeForTest(t)
+ webPath, apiPath, kustPath := seedSharedEntryWorktree(t, worktree.Filesystem.Root())
+
+ _, err := flushEventsForTest(t, writer, worktree, deploymentMapper(),
+ sharedImageEvent("web", "ghcr.io/example/shared:2.0.0"))
+
+ var refused *manifestanalyzer.AcceptanceRefusedError
+ require.ErrorAs(t, err, &refused, "the flush must be refused, and refused legibly")
+ assert.Contains(t, refused.Error(), "Deployment/api",
+ "the refusal must name the object the write would have dragged along")
+ assert.True(t, refused.AllIssuesOfKinds(manifestanalyzer.IssueRenderRefused),
+ "it is a write-boundary refusal, so it surfaces as WriteBoundaryRefused")
+
+ assertFileBytes(t, kustPath, sharedEntryKustomizationYAML,
+ "the entry is shared context; editing it would move api too")
+ assertFileBytes(t, webPath, sharedImageDeploymentYAML("web"), "a refused flush writes nothing")
+ assertFileBytes(t, apiPath, sharedImageDeploymentYAML("api"), "and certainly leaves the sibling alone")
+}
+
+// A NEW resource landing in a kustomize directory that supplies its namespace must not be
+// read as collateral damage by the oracle.
+//
+// The bytes written for it deliberately OMIT metadata.namespace — the kustomization's
+// namespace: transformer puts it back, and every sibling in that directory follows the same
+// convention. But the render therefore HAS a namespace, so an intent built from the
+// namespace-stripped object demands a render that does not, and the whole flush is refused:
+// the new resource is lost, and so is the perfectly good governed write batched with it.
+// Recovery never comes, because resync replays the same batch.
+//
+// The write and the intent are different objects. This is the test that says so.
+func TestPlanFlush_NewResourceInANamespaceInheritingDirIsNotCollateralDamage(t *testing.T) {
+ writer := newContentWriter(types.SensitiveResourcePolicy{})
+ worktree := newWorktreeForTest(t)
+ root := worktree.Filesystem.Root()
+
+ // FLAT on purpose: the kustomization sits beside the manifests it lists, so a new
+ // document lands in the same directory AND gets a resources: entry — which is what puts
+ // it inside the render, where the oracle can see it. (With the manifests in a subfolder
+ // the new file is never added to resources:, so it is not rendered at all and this case
+ // cannot arise.)
+ kustPath := filepath.Join(root, "kustomization.yaml")
+ webPath := filepath.Join(root, "web.yaml")
+ require.NoError(t, os.WriteFile(webPath, []byte(sharedImageDeploymentYAML("web")), 0o600))
+ require.NoError(t, os.WriteFile(kustPath, []byte(`apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+namespace: default
+resources:
+ - web.yaml
+images:
+ - name: ghcr.io/example/shared
+ newTag: "1.0.0"
+`), 0o600))
+
+ changed, err := flushEventsForTest(t, writer, worktree, deploymentMapper(),
+ // A governed write, which is what turns the oracle on...
+ sharedImageEvent("web", "ghcr.io/example/shared:2.0.0"),
+ // ...and a brand-new resource in the same namespace-inheriting directory.
+ newCacheDeploymentEvent("redis:7"),
+ )
+
+ require.NoError(t, err, "the flush must not be refused: both writes are exactly what we intended")
+ require.True(t, changed)
+
+ kust, readErr := os.ReadFile(kustPath)
+ require.NoError(t, readErr)
+ assert.Contains(t, string(kust), `newTag: "2.0.0"`, "the governed write must still land")
+ assert.Contains(t, string(kust), "cache.yaml", "and the new document must be added to the render")
+
+ created := findFileContaining(t, root, "name: cache")
+ require.NotEmpty(t, created, "the new resource must have been written")
+ assert.NotContains(t, created, "namespace: default",
+ "the directory supplies the namespace; the file must not repeat it")
+}
+
+// findFileContaining returns the content of the first YAML file under root holding needle.
+// The new document's placement is the placement rules' business, not this test's.
+func findFileContaining(t *testing.T, root, needle string) string {
+ t.Helper()
+ var found string
+ require.NoError(t, filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
+ if err != nil || d.IsDir() || found != "" || !strings.HasSuffix(path, ".yaml") {
+ return nil //nolint:nilerr // a walk error simply means nothing was found here
+ }
+ b, readErr := os.ReadFile(path)
+ if readErr == nil && strings.Contains(string(b), needle) {
+ found = string(b)
+ }
+ return nil
+ }))
+ return found
+}
+
+// A NEW resource whose image an existing images: entry overrides cannot be mirrored by
+// writing the live value into a new file: the entry rewrites it on the very next render, so
+// the committed file asserts a state the folder does not produce, and it never converges.
+//
+// We do not route a new document's values onto an entry — it has no override chain yet, it
+// did not exist when the store was built — so the honest answer is that this live state cannot
+// be expressed in this repository as it stands. The oracle says so, names the file and the
+// object, and writes nothing. Before, this was committed silently and wrongly.
+func TestPlanFlush_RefusesANewResourceAnEntryWouldOverride(t *testing.T) {
+ writer := newContentWriter(types.SensitiveResourcePolicy{})
+ worktree := newWorktreeForTest(t)
+ root := worktree.Filesystem.Root()
+
+ require.NoError(t, os.WriteFile(filepath.Join(root, "web.yaml"),
+ []byte(sharedImageDeploymentYAML("web")), 0o600))
+ require.NoError(t, os.WriteFile(filepath.Join(root, "kustomization.yaml"), []byte(
+ `apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+namespace: default
+resources:
+ - web.yaml
+images:
+ - name: ghcr.io/example/shared
+ newTag: "1.0.0"
+`), 0o600))
+
+ // The new resource runs the image the entry matches, at a tag the entry does not produce.
+ _, err := flushEventsForTest(t, writer, worktree, deploymentMapper(),
+ newCacheDeploymentEvent("ghcr.io/example/shared:2.0.0"))
+
+ var refused *manifestanalyzer.AcceptanceRefusedError
+ require.ErrorAs(t, err, &refused, "the entry would override the new file; that must not be committed silently")
+ assert.Contains(t, refused.Error(), "Deployment/cache", "the refusal must name the object it cannot express")
+
+ _, statErr := os.Stat(filepath.Join(root, "cache.yaml"))
+ assert.True(t, os.IsNotExist(statErr), "a refused flush writes nothing")
+}
+
+// newCacheDeploymentEvent is a live Deployment with no document in Git yet.
+func newCacheDeploymentEvent(image string) Event {
+ return Event{
+ Object: &unstructured.Unstructured{Object: map[string]interface{}{
+ "apiVersion": "apps/v1",
+ "kind": "Deployment",
+ "metadata": map[string]interface{}{"name": "cache", "namespace": "default"},
+ "spec": map[string]interface{}{
+ "selector": map[string]interface{}{"matchLabels": map[string]interface{}{"app": "cache"}},
+ "template": map[string]interface{}{
+ "metadata": map[string]interface{}{"labels": map[string]interface{}{"app": "cache"}},
+ "spec": map[string]interface{}{
+ "containers": []interface{}{
+ map[string]interface{}{"name": "cache", "image": image},
+ },
+ },
+ },
+ },
+ }},
+ Identifier: types.ResourceIdentifier{
+ Group: "apps", Version: "v1", Resource: "deployments", Namespace: "default", Name: "cache",
+ },
+ Operation: "CREATE",
+ }
+}
+
+// The same tag bump, but both deployments move together: now the entry edit reproduces
+// BOTH live objects, nothing else drifts, and the write lands. This is the control — it is
+// what proves the refusal above is discriminating rather than a blanket "no".
+func TestPlanFlush_AllowsTheSharedEntryWriteWhenEverySiblingAgrees(t *testing.T) {
+ writer := newContentWriter(types.SensitiveResourcePolicy{})
+ worktree := newWorktreeForTest(t)
+ webPath, apiPath, kustPath := seedSharedEntryWorktree(t, worktree.Filesystem.Root())
+
+ changed := applyEventsViaPlanFlushWithMapper(t, writer, worktree, deploymentMapper(),
+ sharedImageEvent("web", "ghcr.io/example/shared:2.0.0"),
+ sharedImageEvent("api", "ghcr.io/example/shared:2.0.0"))
+
+ require.True(t, changed, "both siblings agree on the new tag; the entry edit is now isolated")
+ kust, err := os.ReadFile(kustPath)
+ require.NoError(t, err)
+ assert.Contains(t, string(kust), `newTag: "2.0.0"`, "the entry absorbs the tag both deployments now run")
+ assertFileBytes(t, webPath, sharedImageDeploymentYAML("web"), "the source manifests keep their bytes")
+ assertFileBytes(t, apiPath, sharedImageDeploymentYAML("api"), "the source manifests keep their bytes")
+}
diff --git a/internal/git/manifestedit/kustomization.go b/internal/git/manifestedit/kustomization.go
index efd50da1..9d94eab1 100644
--- a/internal/git/manifestedit/kustomization.go
+++ b/internal/git/manifestedit/kustomization.go
@@ -178,6 +178,64 @@ func AppendKustomizationResource(path string, content []byte, entry string) (Edi
return EditResult{Content: []byte(joinDocuments(docs)), Mode: EditPatched}, nil
}
+// RemoveKustomizationResource drops one entry from an existing kustomization.yaml's
+// resources: sequence. It is AppendKustomizationResource's counterpart, and it exists for
+// exactly the reason that one does, read backwards: a file named in resources: that no
+// longer exists is a file kustomize refuses to build over —
+//
+// accumulating resources ... '/scan/apps/api.yaml' doesn't exist
+//
+// so deleting a managed document without removing its entry leaves a repository no GitOps
+// controller can deploy. Deleting the manifest is only half the delete.
+//
+// It is idempotent (an entry that is not there is EditNoChange) and all-or-nothing in the
+// same way as its sibling: a multi-document file, unparseable YAML, or a document with no
+// resources: sequence skips the whole call with a diagnostic rather than inventing structure.
+// An emptied sequence is left as an empty sequence — removing the key is not this function's
+// call to make.
+func RemoveKustomizationResource(path string, content []byte, entry string) (EditResult, []Diagnostic) {
+ skip := func(format string, args ...interface{}) (EditResult, []Diagnostic) {
+ return EditResult{Content: content, Mode: EditSkipped},
+ []Diagnostic{diag(DiagWarning, Location{Path: path}, format, args...)}
+ }
+
+ docs, idx, root, reason, ok := locateKustomizationDocument(path, content)
+ if !ok {
+ return skip("%s", reason)
+ }
+ target := docs[idx].body
+
+ section := nodeMapGet(root, "resources")
+ if section == nil || section.Kind != yaml.SequenceNode {
+ return skip("kustomization %s has no resources sequence", path)
+ }
+
+ kept := make([]*yaml.Node, 0, len(section.Content))
+ removed := false
+ for _, item := range section.Content {
+ if item.Kind == yaml.ScalarNode && strings.TrimSpace(item.Value) == strings.TrimSpace(entry) {
+ removed = true
+ continue
+ }
+ kept = append(kept, item)
+ }
+ if !removed {
+ return EditResult{Content: content, Mode: EditNoChange}, nil
+ }
+ section.Content = kept
+
+ encoded, err := encodeNode(root)
+ if err != nil {
+ return skip("kustomization %s: re-encode failed: %v", path, err)
+ }
+ body := reskinDocument(target, string(encoded))
+ if body == target {
+ return EditResult{Content: content, Mode: EditNoChange}, nil
+ }
+ docs[idx].body = body
+ return EditResult{Content: []byte(joinDocuments(docs)), Mode: EditPatched}, nil
+}
+
// setOverrideScalar writes the new value, keeping the value string-typed for the
// image fields (the encoder quotes "1.29"-style values when the tag is !!str) and
// integer-typed for count. An existing quoting style is kept; other styles reset
diff --git a/internal/git/manifestedit/kustomization_test.go b/internal/git/manifestedit/kustomization_test.go
index 520aeb3d..af35e422 100644
--- a/internal/git/manifestedit/kustomization_test.go
+++ b/internal/git/manifestedit/kustomization_test.go
@@ -183,6 +183,65 @@ func TestAppendKustomizationResource_IdempotentWhenAlreadyListed(t *testing.T) {
}
}
+func TestRemoveKustomizationResource_DropsEntryPreservingHandAuthoring(t *testing.T) {
+ withExtra, _ := AppendKustomizationResource(
+ "kustomization.yaml",
+ []byte(kustomizationFixture),
+ "debug-toolbox.yaml",
+ )
+
+ res, diags := RemoveKustomizationResource("kustomization.yaml", withExtra.Content, "debug-toolbox.yaml")
+ if res.Mode != EditPatched {
+ t.Fatalf("Mode = %q, want patched (diags %+v)", res.Mode, diags)
+ }
+ got := string(res.Content)
+ if strings.Contains(got, "debug-toolbox.yaml") {
+ t.Errorf("the entry must be gone:\n%s", got)
+ }
+ // The hand-authoring, the surviving entry, and every unrelated section stay put.
+ for _, want := range []string{"# pin the app image here", "- deployment.yaml", "namespace: app", "count: 3"} {
+ if !strings.Contains(got, want) {
+ t.Errorf("want %q preserved in:\n%s", want, got)
+ }
+ }
+}
+
+func TestRemoveKustomizationResource_IdempotentWhenNotListed(t *testing.T) {
+ res, _ := RemoveKustomizationResource("kustomization.yaml", []byte(kustomizationFixture), "never-there.yaml")
+ if res.Mode != EditNoChange {
+ t.Fatalf("Mode = %q, want no-change for an entry that is not listed", res.Mode)
+ }
+ if string(res.Content) != kustomizationFixture {
+ t.Errorf("a no-op must leave the bytes byte-identical")
+ }
+}
+
+func TestRemoveKustomizationResource_RefusalsLeaveContentUntouched(t *testing.T) {
+ cases := []struct {
+ name string
+ content string
+ }{
+ {"no resources sequence", "namespace: app\n"},
+ {"resources is not a sequence", "resources: not-a-list\n"},
+ {"multi-document file", kustomizationFixture + "---\nnamespace: other\n"},
+ {"unparseable", "resources: [::\n"},
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ res, diags := RemoveKustomizationResource("kustomization.yaml", []byte(tc.content), "deployment.yaml")
+ if res.Mode != EditSkipped {
+ t.Fatalf("Mode = %q, want skipped", res.Mode)
+ }
+ if string(res.Content) != tc.content {
+ t.Errorf("a refused removal must leave the bytes untouched")
+ }
+ if len(diags) == 0 {
+ t.Errorf("a refusal must carry a diagnostic")
+ }
+ })
+ }
+}
+
func TestAppendKustomizationResource_RefusalsLeaveContentUntouched(t *testing.T) {
cases := []struct {
name string
diff --git a/internal/git/plan_flush.go b/internal/git/plan_flush.go
index cc942296..5f550fba 100644
--- a/internal/git/plan_flush.go
+++ b/internal/git/plan_flush.go
@@ -5,6 +5,7 @@ package git
import (
"bytes"
"context"
+ "errors"
"fmt"
"math"
"os"
@@ -14,6 +15,7 @@ import (
"strings"
gogit "github.com/go-git/go-git/v5"
+ "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/controller-runtime/pkg/log"
sigsyaml "sigs.k8s.io/yaml"
@@ -74,6 +76,17 @@ type writeBatch struct {
docLoc map[*manifestanalyzer.DocumentModel]manifestanalyzer.RecordRef
contentByPath map[string][]byte
buffers map[string]*fileBuffer
+ // intents records what each document this flush writes must render to, so the
+ // render precondition can tell a change the flush MEANT from one it merely caused.
+ // Anything not named here has to come out of the re-render untouched.
+ intents []manifestanalyzer.WriteIntent
+ // putToKustomize records that this flush touched a kustomize render root — it edited a
+ // governed document, or placed a new one into a kustomization's resources:. It is what
+ // turns the oracle on, and it is deliberately NOT the same question as WriteIntent.Governed:
+ // that one additionally ASSERTS the document is rendered, which a new document is not
+ // entitled to claim (its resources: entry can legitimately fail to be added — see
+ // appendKustomizationResource — leaving the file written but outside every render).
+ putToKustomize bool
// policy is the GitTarget's declared new-file placement policy, consulted
// only for a resource with no existing document. nil means no declared policy —
// placement falls through to sibling inference and then the canonical path.
@@ -214,7 +227,7 @@ func (wb *writeBatch) applyEvent(ctx context.Context, event Event) error {
case event.IsFieldPatch():
return wb.applyFieldPatch(ctx, event)
case event.Operation == "DELETE":
- wb.applyDelete(event)
+ wb.applyDelete(ctx, event)
return nil
default:
_, err := wb.applyUpsert(ctx, event)
@@ -231,16 +244,39 @@ func (wb *writeBatch) applyEvent(ctx context.Context, event Event) error {
// existing document is placed by createNew. It returns what it did to the bytes
// (created / updated / no change).
func (wb *writeBatch) applyUpsert(ctx context.Context, event Event) (upsertOutcome, error) {
- if id, ok := manifestIdentity(event.Object); ok {
- if dm := wb.store.ByManifestIdentity[id]; dm != nil {
- filePath := wb.docLoc[dm].FilePath
- if wb.writer.isSensitiveIdentifier(event.Identifier) {
- return wb.writeWholeFile(ctx, event, filePath)
- }
- return wb.patchExisting(ctx, event, filePath, id, dm)
- }
+ id, ok := manifestIdentity(event.Object)
+ if !ok {
+ return wb.createNew(ctx, event)
+ }
+ dm := wb.store.ByManifestIdentity[id]
+ if dm == nil {
+ return wb.createNew(ctx, event)
}
- return wb.createNew(ctx, event)
+ filePath := wb.docLoc[dm].FilePath
+ if !wb.writer.isSensitiveIdentifier(event.Identifier) {
+ return wb.patchExisting(ctx, event, filePath, id, dm)
+ }
+ return wb.rewriteSensitive(ctx, event, filePath)
+}
+
+// rewriteSensitive re-encrypts a sensitive document wholesale at its existing path.
+//
+// Its intent is UNCHECKED: the file is SOPS ciphertext, so kustomize renders the encrypted
+// blob and no plaintext live object can ever equal it. The oracle is told to expect this
+// object to move without being able to say what to — while still holding the write to
+// disturbing nothing else, which is the half that protects other environments.
+func (wb *writeBatch) rewriteSensitive(ctx context.Context, event Event, filePath string) (upsertOutcome, error) {
+ outcome, err := wb.writeWholeFile(ctx, event, filePath)
+ if err == nil && wroteBytes(outcome) {
+ wb.intend(markUnchecked(intentFor(event.Object, filePath, false), true))
+ }
+ return outcome, err
+}
+
+// wroteBytes reports whether an upsert actually changed the worktree, which is the only
+// case that owes the oracle an intent.
+func wroteBytes(o upsertOutcome) bool {
+ return o == upsertCreated || o == upsertUpdated
}
// createNew resolves the placement of a resource with no existing document —
@@ -269,6 +305,11 @@ func (wb *writeBatch) createNew(ctx context.Context, event Event) (upsertOutcome
return upsertSkippedUnsafe, nil
}
+ // The LIVE object, kept before the namespace strip below rewrites it. The bytes we write
+ // and the object the render must produce are not the same thing, and only this scope
+ // still holds both — see intentFor.
+ live := event.Object
+
if placement.Kustomization != nil {
wb.appendKustomizationResource(ctx, event, placement)
}
@@ -283,6 +324,34 @@ func (wb *writeBatch) createNew(ctx context.Context, event Event) (upsertOutcome
event.Object.SetNamespace("")
}
+ outcome, err := wb.placeNewDocument(ctx, event, placement, sensitive)
+ if err != nil || !wroteBytes(outcome) {
+ return outcome, err
+ }
+
+ // A new document that joins a kustomization's resources: list is INSIDE a render root, so
+ // the folder's images:/replicas: entries govern it from the moment it lands — and we do not
+ // route a new document's values onto an entry (it has no override chain yet; it did not
+ // exist when the store was built). So the live value goes into the file, and if an entry
+ // overrides it, the folder renders something else and the resource never converges.
+ //
+ // Declaring it governed puts it in front of the oracle, which turns that from a silent
+ // non-converging commit into a reported refusal naming the file and the object. It does not
+ // make the write work — that needs attribution for a document that does not exist yet — but
+ // "we cannot express this here" is an answer, and quietly writing a lie is not.
+ wb.putToKustomize = wb.putToKustomize || placement.Kustomization != nil
+ wb.intend(markUnchecked(intentFor(live, placement.Path, false), sensitive))
+ return outcome, nil
+}
+
+// placeNewDocument writes the new document at its resolved placement: appended to an existing
+// accepted bundle, folded into a same-batch cold bundle, or as a file of its own.
+func (wb *writeBatch) placeNewDocument(
+ ctx context.Context,
+ event Event,
+ placement manifestanalyzer.PlacementResult,
+ sensitive bool,
+) (upsertOutcome, error) {
if placement.Append {
return wb.appendNewDocument(ctx, event, placement.Path)
}
@@ -469,8 +538,17 @@ func (wb *writeBatch) applyFieldPatch(ctx context.Context, event Event) error {
}
assignments := event.FieldPatch.Assignments
- if dm := wb.store.ByManifestIdentity[id]; dm != nil && dm.Overrides != nil {
+ dm := wb.store.ByManifestIdentity[id]
+ governed := dm != nil && dm.Overrides != nil
+ if governed {
assignments = wb.routeGovernedFieldAssignments(ctx, event, dm, assignments)
+ // A routed scale changes only the kustomization entry, which still moves what this
+ // document renders to — so it must be declared, or the oracle would read its own
+ // intended write as collateral damage. It is UNCHECKED because a field patch carries
+ // a few audited assignments, never a whole object to compare the render against: the
+ // oracle can still prove the write disturbs nothing else, but not that it landed.
+ wb.putToKustomize = true
+ wb.intend(fieldPatchIntent(filePath, id, governed))
if len(assignments) == 0 {
return nil
}
@@ -489,6 +567,9 @@ func (wb *writeBatch) applyFieldPatch(ctx context.Context, event Event) error {
switch res.Mode {
case manifestedit.EditPatched:
buf.current = res.Content
+ if !governed {
+ wb.intend(fieldPatchIntent(filePath, id, false))
+ }
case manifestedit.EditNoChange, manifestedit.EditDeleted:
// No-op: the audited value already matched (or, impossible here, a delete).
case manifestedit.EditSkipped, manifestedit.EditWholeReplace:
@@ -595,9 +676,9 @@ func (wb *writeBatch) patchExisting(
}
projected := manifestreport.Project(desired)
var overrideEdits []manifestanalyzer.OverrideEdit
- if dm.Overrides != nil {
+ if dm.Rendered != nil {
if gitRaw, parsed := gitDocRawObject(buf.current, idx); parsed {
- projected, overrideEdits = manifestanalyzer.SplitDesiredForOverrides(gitRaw, projected, dm.Overrides)
+ projected, overrideEdits = manifestanalyzer.SplitDesiredForOverrides(gitRaw, projected, dm.Rendered)
}
}
c := manifestedit.Comparison{
@@ -622,9 +703,158 @@ func (wb *writeBatch) patchExisting(
if wb.applyOverrideEdits(ctx, event, overrideEdits) {
outcome = upsertUpdated
}
+
+ // Declare what this document must render to. Attribution above decided WHERE the edit
+ // goes and is allowed to be wrong; the render precondition adjudicates it once the whole
+ // plan is known (see renderPrecondition).
+ //
+ // A GOVERNED document declares its intent even when its own bytes did not change, and
+ // that is not belt-and-braces — it is the difference between the oracle working and the
+ // oracle refusing perfectly good writes. An images: entry is shared: when two Deployments
+ // run the same image and are bumped together, the FIRST event's entry edit already moves
+ // what the second one renders to, so by the time the second is processed there is nothing
+ // left to write. Its render still moves, and it moves onto its own live state — that is
+ // the resource converging, not collateral damage, and only its declared intent says so.
+ if dm.Overrides != nil {
+ wb.putToKustomize = true
+ }
+ if outcome == upsertUpdated || dm.Overrides != nil {
+ wb.intend(intentFor(event.Object, filePath, dm.Overrides != nil))
+ }
return outcome, nil
}
+// renderPrecondition is the oracle, and it is a write-plan precondition like the three
+// above it: it runs at the one moment the whole plan is known and before a single byte is
+// touched, so a refusal aborts the flush and commits nothing.
+//
+// It only runs when the flush actually routed something through a kustomization. A repo
+// with no override chain pays nothing, and a flush that changed no governed document has
+// nothing for kustomize to adjudicate.
+//
+// A refusal is an AcceptanceRefusedError, which is the seam that carries it to the user as
+// GitPathAccepted=False / Stalled=True with the file and object named. That is deliberate:
+// render-attribution.md §7 is explicit that a proposal the renderer cannot vouch for
+// "becomes a refused flush — that is the correct outcome and it must be reported, not
+// absorbed." A resource we silently stop mirroring is the failure this path exists to
+// prevent, so it must not be the failure this path introduces.
+func (wb *writeBatch) renderPrecondition() error {
+ if !wb.putToKustomize {
+ return nil
+ }
+
+ before := make([]manifestedit.FileContent, 0, len(wb.contentByPath))
+ for _, path := range sortedContentKeys(wb.contentByPath) {
+ before = append(before, manifestedit.FileContent{Path: path, Content: wb.contentByPath[path]})
+ }
+
+ var refused *manifestanalyzer.RenderRefusedError
+ if err := manifestanalyzer.VerifyBatchRenders(before, wb.files(), wb.intents); err != nil {
+ if errors.As(err, &refused) {
+ issues := make([]manifestanalyzer.AcceptanceIssue, 0, len(refused.Reasons))
+ for _, reason := range refused.Reasons {
+ issues = append(issues, manifestanalyzer.AcceptanceIssue{
+ Kind: manifestanalyzer.IssueRenderRefused,
+ Message: reason,
+ })
+ }
+ return &manifestanalyzer.AcceptanceRefusedError{Issues: issues}
+ }
+ return err
+ }
+ return nil
+}
+
+// intend records what one document of this flush must render to, so the oracle can tell a
+// change the flush MEANT from a change it merely caused. Everything not intended has to
+// come out of the render untouched.
+func (wb *writeBatch) intend(in manifestanalyzer.WriteIntent) {
+ if in.Kind == "" || in.Name == "" {
+ return // nothing addressable to check; the render comparison keys on kind+name
+ }
+ wb.intents = append(wb.intents, in)
+}
+
+// intentFor builds the intent for an ordinary object-bearing write: the document must
+// render to exactly the live object.
+//
+// It takes the LIVE object, not the event, and that distinction is load-bearing. 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. But the render therefore HAS the namespace, so an intent built from the
+// stripped object would demand that the render not have one, and the oracle would refuse a
+// flush it had just planned perfectly. The bytes and the intent are different objects, and
+// the caller is the only one that still holds both.
+func intentFor(live *unstructured.Unstructured, filePath string, governed bool) manifestanalyzer.WriteIntent {
+ desired := manifestreport.Project(live)
+ return manifestanalyzer.WriteIntent{
+ SourcePath: filePath,
+ Kind: desired.GetKind(),
+ Name: desired.GetName(),
+ Desired: desired,
+ Governed: governed,
+ }
+}
+
+// unchecked marks a write whose rendered form cannot be predicted, so the oracle lets the
+// object move without comparing it (but still holds the write to disturbing nothing else).
+func markUnchecked(in manifestanalyzer.WriteIntent, unchecked bool) manifestanalyzer.WriteIntent {
+ if unchecked {
+ in.Unchecked = true
+ in.Desired = nil
+ }
+ return in
+}
+
+// fieldPatchIntent declares a bounded field patch. It is always unchecked: the event carries
+// a handful of audited assignments, not a whole object, so there is nothing to require the
+// render to equal.
+func fieldPatchIntent(filePath string, id manifestedit.Identity, governed bool) manifestanalyzer.WriteIntent {
+ return manifestanalyzer.WriteIntent{
+ SourcePath: filePath,
+ Kind: id.Kind,
+ Name: id.Name,
+ Unchecked: true,
+ Governed: governed,
+ }
+}
+
+// files is the batch's complete tree as the flush would leave it: the worktree bytes with
+// every buffer folded over them, and deleted files removed. Sorted, so the render is
+// reproducible.
+func (wb *writeBatch) files() []manifestedit.FileContent {
+ byPath := make(map[string][]byte, len(wb.contentByPath)+len(wb.buffers))
+ for path, content := range wb.contentByPath {
+ byPath[path] = content
+ }
+ for path, b := range wb.buffers {
+ if b.current == nil {
+ delete(byPath, path) // the flush deletes this file
+ continue
+ }
+ byPath[path] = b.current
+ }
+ paths := make([]string, 0, len(byPath))
+ for path := range byPath {
+ paths = append(paths, path)
+ }
+ sort.Strings(paths)
+ out := make([]manifestedit.FileContent, 0, len(paths))
+ for _, path := range paths {
+ out = append(out, manifestedit.FileContent{Path: path, Content: byPath[path]})
+ }
+ return out
+}
+
+func sortedContentKeys(m map[string][]byte) []string {
+ out := make([]string, 0, len(m))
+ for k := range m {
+ out = append(out, k)
+ }
+ sort.Strings(out)
+ return out
+}
+
// applyOverrideEdits folds routed override edits into their kustomization file
// buffers, so they flush (and hit the .gittargetignore shadow precondition)
// exactly like any other planned write. It reports whether any buffer changed.
@@ -734,7 +964,7 @@ func (wb *writeBatch) writeWholeFile(ctx context.Context, event Event, rel strin
// the same batch that shifted a multi-document file does not misdirect this one.
// Removing the last document in a file marks it for deletion; otherwise the surviving
// documents are kept byte-for-byte.
-func (wb *writeBatch) applyDelete(event Event) {
+func (wb *writeBatch) applyDelete(ctx context.Context, event Event) {
target, found := wb.resolveDelete(event)
if !found {
return
@@ -747,12 +977,93 @@ func (wb *writeBatch) applyDelete(event Event) {
if !ok {
return
}
+ wb.intend(manifestanalyzer.WriteIntent{
+ SourcePath: target.filePath,
+ Kind: target.id.Kind,
+ Name: target.id.Name,
+ Removed: true,
+ })
+ // Any delete inside a render root changes what that root renders, so it goes to the
+ // oracle — whether or not the file itself goes with it. Gating this on "the file was
+ // emptied" would be a rule about our own bookkeeping rather than about the render, and
+ // the whole point of the oracle is that it does not take our word for anything.
+ if len(wb.kustomizationsListing(target.filePath)) > 0 {
+ wb.putToKustomize = true
+ }
res, _ := manifestedit.DeleteDocument(buf.current, idx)
- if res.FileEmpty {
- buf.current = nil
+ if !res.FileEmpty {
+ buf.current = res.Content
return
}
- buf.current = res.Content
+ // The file is gone. Anything still naming it in a resources: list now names a file that
+ // does not exist, and kustomize refuses to build over that — so deleting the manifest is
+ // only half the delete.
+ buf.current = nil
+ wb.dropKustomizationResource(ctx, event, target.filePath)
+}
+
+// dropKustomizationResource removes the resources: entry naming a file this flush deleted,
+// from every supported kustomization that lists it.
+//
+// It is the counterpart of appendKustomizationResource, and it fails the same way: a
+// kustomization it cannot edit only loses its entry (logged), it does not abort the delete —
+// the render precondition is what decides whether the resulting tree is committable.
+func (wb *writeBatch) dropKustomizationResource(ctx context.Context, event Event, filePath string) {
+ for _, listing := range wb.kustomizationsListing(filePath) {
+ buf := wb.buffer(listing.kustomization)
+ if buf.current == nil {
+ continue // the kustomization is itself being deleted in this batch
+ }
+ res, diags := manifestedit.RemoveKustomizationResource(listing.kustomization, buf.current, listing.entry)
+ switch res.Mode {
+ case manifestedit.EditPatched:
+ buf.current = res.Content
+ log.FromContext(ctx).Info("Removed resources: entry for deleted file",
+ "kustomization", listing.kustomization, "entry", listing.entry,
+ "resource", event.Identifier.String())
+ case manifestedit.EditNoChange:
+ case manifestedit.EditSkipped, manifestedit.EditDeleted, manifestedit.EditWholeReplace:
+ // The entry stays, and it now names a file that does not exist. We do not have to
+ // decide how bad that is: the render precondition rebuilds the tree and refuses
+ // the flush, because kustomize will not build over a missing resource.
+ log.FromContext(ctx).Info("Could not remove resources: entry for deleted file",
+ "kustomization", listing.kustomization, "entry", listing.entry,
+ "resource", event.Identifier.String())
+ logManifestDiagnostics(ctx, diags)
+ }
+ }
+}
+
+// resourceListing is one kustomization's resources: entry naming a given file.
+type resourceListing struct {
+ kustomization string // the kustomization.yaml's own path
+ entry string // the entry text, relative to the kustomization's directory
+}
+
+// kustomizationsListing returns every supported kustomization whose resources: names filePath,
+// in deterministic order. It is the one definition of "this file is inside a render root",
+// shared by the oracle's trigger and by the entry removal, so the two cannot drift apart.
+func (wb *writeBatch) kustomizationsListing(filePath string) []resourceListing {
+ dirs := make([]string, 0, len(wb.store.Kustomizations))
+ for dir := range wb.store.Kustomizations {
+ dirs = append(dirs, dir)
+ }
+ sort.Strings(dirs)
+
+ var out []resourceListing
+ for _, dir := range dirs {
+ info := wb.store.Kustomizations[dir]
+ if info.Unsupported {
+ continue // never edit a kustomization we do not model
+ }
+ base := path.Dir(info.Path)
+ for _, entry := range info.Resources {
+ if path.Clean(path.Join(base, entry)) == filePath {
+ out = append(out, resourceListing{kustomization: info.Path, entry: entry})
+ }
+ }
+ }
+ return out
}
// deleteTarget names the file and manifest identity a delete targets. The document
@@ -841,6 +1152,9 @@ func (wb *writeBatch) flush(ctx context.Context, worktree *gogit.Worktree, root,
if err := wb.fanInPrecondition(); err != nil {
return false, err
}
+ if err := wb.renderPrecondition(); err != nil {
+ return false, err
+ }
logger := log.FromContext(ctx)
changed := false
for _, rel := range sortedBufferKeys(wb.buffers) {
diff --git a/internal/manifestanalyzer/acceptance.go b/internal/manifestanalyzer/acceptance.go
index 918442c4..9602aaf0 100644
--- a/internal/manifestanalyzer/acceptance.go
+++ b/internal/manifestanalyzer/acceptance.go
@@ -122,8 +122,20 @@ const (
// made explicit; the broader "any file shared by multiple render roots" generalization is
// Per-render-root scoping would generalize this.
IssueWriteFanIn IssueKind = "write-fan-in"
+ // IssueRenderRefused marks a planned write that kustomize itself will not vouch for: the
+ // flush was re-rendered with the write applied, and either the edited document did not
+ // come out as the live object, or the write moved an object it never set out to touch.
+ //
+ // It is the write-plan half of "attribution may be heuristic, verification may not"
+ // (docs/design/support-boundary/render-attribution.md §5). The projection is ALLOWED to
+ // guess which file an edit belongs in, precisely because this refuses the guess when the
+ // renderer disagrees. And it must refuse LOUDLY: a write that does not survive the
+ // re-render is one that would not converge — the entry overrides it straight back on the
+ // next render — so absorbing it would leave a resource silently un-mirrored forever,
+ // which is the exact failure this whole path exists to prevent.
+ IssueRenderRefused IssueKind = "kustomize-render-refused"
- // A refusal made up purely of the two write-boundary kinds above surfaces as the GitTarget
+ // A refusal made up purely of the write-boundary kinds above surfaces as the GitTarget
// reason WriteBoundaryRefused rather than the umbrella UnsupportedContent: the folder holds
// nothing the operator cannot manage, the edit simply had nowhere safe to land. See the
// watch package's gitPathRefusalReason.
diff --git a/internal/manifestanalyzer/analyzer_test.go b/internal/manifestanalyzer/analyzer_test.go
index 08edde13..baee943b 100644
--- a/internal/manifestanalyzer/analyzer_test.go
+++ b/internal/manifestanalyzer/analyzer_test.go
@@ -113,15 +113,18 @@ func TestAnalyze_Issues(t *testing.T) {
IssueUnresolvedKRM: 0,
IssueOutOfScope: 0,
IssueUnsupportedKustomize: 0,
- // Foreign-content, ignore-shadow, and the L1/L2 write-boundary refusals are
+ // Foreign-content, ignore-shadow, and the write-boundary refusals are
// acceptance-gate / write-plan facts, not part of the structure-only Analyze report,
- // so they never surface here.
+ // so they never surface here. IssueRenderRefused is the strongest case of that: it is
+ // a fact about a PROPOSED WRITE re-rendered with kustomize, which structure-only
+ // analysis has no live object to propose.
IssueForeignFile: 0,
IssueForeignSymlink: 0,
IssueForeignSubmodule: 0,
IssueIgnoreShadowsManaged: 0,
IssueWriteEscapesScope: 0,
IssueWriteFanIn: 0,
+ IssueRenderRefused: 0,
}
for kind, n := range want {
if got := countIssues(rep, kind); got != n {
diff --git a/internal/manifestanalyzer/dye.go b/internal/manifestanalyzer/dye.go
new file mode 100644
index 00000000..c07cace7
--- /dev/null
+++ b/internal/manifestanalyzer/dye.go
@@ -0,0 +1,256 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package manifestanalyzer
+
+import (
+ "fmt"
+ "regexp"
+ "sort"
+
+ kustypes "sigs.k8s.io/kustomize/api/types"
+ "sigs.k8s.io/yaml"
+
+ "github.com/ConfigButler/gitops-reverser/internal/git/manifestedit"
+)
+
+// The dye: which override entry supplied this value?
+//
+// A render does not say. `kustomize build` returns `web:2.0`; it never says where the `2.0`
+// came from, and there is no field-level provenance anywhere in kustomize, at any visibility
+// level — the image filter rewrites the field and records nothing. So attribution cannot be
+// READ out of kustomize. It can only be inferred by QUESTIONING it: perturb an input, render,
+// see what moves.
+//
+// The obvious perturbation — remove an entry and see what changes — is measurably wrong, and
+// wrong on the most ordinary configuration there is. A base at `app:v1` under an overlay
+// declaring `newTag: v1` is the state every repo is in the moment a release lands in both
+// places; remove the entry and NOTHING MOVES, so removal concludes the source file supplies
+// the tag. Write the user's next tag into the base, and the overlay overrides it straight back
+// on every reconcile, forever. The cause is structural: removal probes the VALUE, and values
+// collide.
+//
+// The dye is that same idea with the flaw removed. Write a unique nonce into every override
+// entry, render ONCE, and read the nonces off the output: wherever a dye lands, THAT entry
+// supplied THAT field. Absence is indistinguishable from "someone else wrote the same value";
+// a nonce nothing else can produce is not. Ties resolve, the idempotent pin resolves, and the
+// cost is one extra build per root rather than one per entry.
+//
+// See docs/design/support-boundary/render-attribution.md §3.
+
+// The nonce alphabet is not a style choice, it is a CORRECTNESS REQUIREMENT, and it is the
+// one thing here that will silently ruin a render if it is got wrong.
+//
+// kustomize does not validate newTag or digest at all — a 200-character tag renders straight
+// through. But it MATCHES on the image with a regex over the whole string
+// (api/internal/image/image.go):
+//
+// "^" + name + "(:[a-zA-Z0-9_.{}-]*)?(@sha256:[a-zA-Z0-9_.{}-]*)?$"
+//
+// so a dye outside that charset leaves the image un-matchable and EVERY LATER ENTRY SILENTLY
+// STOPS FIRING. No error; a different render. Measured: `newTag: zz/probe` renders, and kills
+// the next entry. A digest without the mandatory `sha256:` prefix does the same.
+const (
+ // dyeTagPrefix keeps tag nonces inside [a-zA-Z0-9_.{}-].
+ dyeTagPrefix = "grdye-t-"
+ // dyeNamePrefix is for newName. A name is not matched by the tag charset — it is the
+ // regex's literal prefix — but it must still be a plain image name: no ':' (a tag
+ // separator), no '@' (a digest separator), no '/' (a registry separator).
+ dyeNamePrefix = "grdye-n-"
+ // dyeDigestPrefix carries the sha256: the regex REQUIRES, and stays alphanumeric after it.
+ dyeDigestPrefix = "sha256:grdye"
+ // dyeReplicaBase is a reserved count. Measured: it renders through untouched, and no
+ // real deployment has two billion replicas.
+ dyeReplicaBase = int64(2_000_000_000)
+)
+
+// dyeMark says what a nonce was minted for, so a dye read out of the tag position can be
+// required to BE a tag dye rather than merely a nonce that happens to have landed there.
+type dyeMark struct {
+ image *ImageOverride
+ replica *ReplicaOverride
+ field string
+}
+
+// dyePlan is the counterfactual: the dyed kustomizations to render, and the table that reads
+// the nonces back. One plan covers the whole scan, so it is built once and reused for every
+// render root.
+type dyePlan struct {
+ // replace is the dyed kustomization content, by slash path — the overlay handed to
+ // renderRootWith.
+ replace map[string][]byte
+ // byNonce maps a nonce back to the entry that carries it.
+ byNonce map[string]dyeMark
+ // byReplicaCount does the same for the reserved integer counts.
+ byReplicaCount map[int64]dyeMark
+ // namesDyed records whether newName was dyed. When a rename chain exists, it is not —
+ // see dyeingNamesIsSafe — and a name change is then simply unattributable, which means
+ // no entry edit, which means the oracle adjudicates whatever the source file can carry.
+ namesDyed bool
+}
+
+// planDye mints a nonce for every override entry field that is DECLARED, and re-serialises
+// each kustomization with the nonces in place.
+//
+// "Declared" matters: injecting a newTag into a newName-only entry would fabricate a supplier
+// that does not exist, and the projection would then route a tag edit to an entry that never
+// set a tag.
+//
+// The dye is applied to kustomize's own typed Kustomization and re-marshalled, exactly as
+// withBuildMetadata already does, so YAML quoting is the encoder's problem rather than ours.
+// It has to be: `newTag: y` is a YAML BOOLEAN, and kustomize's own Unmarshal rejects it.
+func planDye(files []manifestedit.FileContent) *dyePlan {
+ plan := &dyePlan{
+ replace: map[string][]byte{},
+ byNonce: map[string]dyeMark{},
+ byReplicaCount: map[int64]dyeMark{},
+ namesDyed: dyeingNamesIsSafe(allImageOverrides(files)),
+ }
+
+ next := 0
+ mint := func() int { next++; return next }
+
+ for _, f := range sortedKustomizationFiles(files) {
+ var k kustypes.Kustomization
+ if err := k.Unmarshal(f.Content); err != nil {
+ continue // an unparseable kustomization refuses the folder elsewhere; nothing to dye
+ }
+ k.FixKustomization()
+ path := filepathToSlash(f.Path)
+
+ dyed := plan.dyeImages(&k, path, mint)
+ dyed = plan.dyeReplicas(&k, path, mint) || dyed
+ if !dyed {
+ continue
+ }
+ content, err := yaml.Marshal(&k)
+ if err != nil {
+ continue // cannot dye this file; its entries simply go unattributed
+ }
+ plan.replace[path] = content
+ }
+ return plan
+}
+
+// dyeImages replaces every DECLARED images: field with a nonce, in place, and records what each
+// nonce was minted for. It reports whether anything was dyed.
+func (p *dyePlan) dyeImages(k *kustypes.Kustomization, path string, mint func() int) bool {
+ entries, ok := imageOverrides(k.Images, path)
+ if !ok || len(entries) != len(k.Images) {
+ return false
+ }
+ dyed := false
+ for i := range k.Images {
+ entry := &entries[i]
+ if entry.HasNewTag {
+ nonce := fmt.Sprintf("%s%04d", dyeTagPrefix, mint())
+ p.byNonce[nonce] = dyeMark{image: entry, field: fieldNewTag}
+ k.Images[i].NewTag = nonce
+ dyed = true
+ }
+ if entry.HasDigest {
+ nonce := fmt.Sprintf("%s%04d", dyeDigestPrefix, mint())
+ p.byNonce[nonce] = dyeMark{image: entry, field: fieldDigest}
+ k.Images[i].Digest = nonce
+ dyed = true
+ }
+ if entry.HasNewName && p.namesDyed {
+ nonce := fmt.Sprintf("%s%04d", dyeNamePrefix, mint())
+ p.byNonce[nonce] = dyeMark{image: entry, field: fieldNewName}
+ k.Images[i].NewName = nonce
+ dyed = true
+ }
+ }
+ return dyed
+}
+
+// dyeReplicas replaces every replicas: count with a reserved integer, in place.
+func (p *dyePlan) dyeReplicas(k *kustypes.Kustomization, path string, mint func() int) bool {
+ entries, ok := replicaOverrides(k.Replicas, path)
+ if !ok || len(entries) != len(k.Replicas) {
+ return false
+ }
+ for i := range k.Replicas {
+ count := dyeReplicaBase + int64(mint())
+ p.byReplicaCount[count] = dyeMark{replica: &entries[i], field: fieldCount}
+ k.Replicas[i].Count = count
+ }
+ return len(k.Replicas) > 0
+}
+
+// The entry fields the projection can write, and the dye can therefore attribute.
+const (
+ fieldNewName = "newName"
+ fieldNewTag = "newTag"
+ fieldDigest = "digest"
+ fieldCount = "count"
+)
+
+// dyeingNamesIsSafe reports whether dyeing newName can be trusted in this tree.
+//
+// A dye is sound exactly when the dyed field is a PURE SINK — never an input to a matcher.
+// newTag, digest and replicas[].count are sinks: nothing selects on them. newName IS NOT. It
+// is the join key for every later entry:
+//
+// images: [{name: app, newName: renamed}, {name: renamed, newTag: "4.0"}]
+// undyed -> renamed:4.0
+// newName dyed -> grdye-n-0001:v1 # entry 2 stopped matching. the render changed shape.
+//
+// The condition is exact and needs no model of kustomize's matching: dyeing a newName can only
+// change a matching decision if some OTHER entry's name: matches it. And "matches" is asked of
+// kustomize's own compiled pattern rather than of string equality, because an entry's name is a
+// REGULAR EXPRESSION — `name: "rename."` matches `renamed` without being equal to it.
+//
+// Where no rename chain exists — essentially every real repository — names are dyed. Where one
+// does, they are not, and a name change is simply not attributed. That is the correct fallback:
+// no attribution, never a guess.
+func dyeingNamesIsSafe(entries []ImageOverride) bool {
+ for i := range entries {
+ if !entries[i].HasNewName {
+ continue
+ }
+ for j := range entries {
+ if i == j {
+ continue
+ }
+ pattern, err := regexp.Compile(imageNamePattern(entries[j].Name))
+ if err != nil {
+ return false // an uncompilable name refuses the folder anyway; do not dye into it
+ }
+ if pattern.MatchString(entries[i].NewName) {
+ return false
+ }
+ }
+ }
+ return true
+}
+
+// allImageOverrides is every images: entry in the scan, which is the scope the rename-chain
+// guard is asked over. Scoping it per render root would dye more, and the entries a root
+// cannot reach cost nothing to be conservative about.
+func allImageOverrides(files []manifestedit.FileContent) []ImageOverride {
+ var out []ImageOverride
+ for _, f := range sortedKustomizationFiles(files) {
+ var k kustypes.Kustomization
+ if err := k.Unmarshal(f.Content); err != nil {
+ continue
+ }
+ k.FixKustomization()
+ if entries, ok := imageOverrides(k.Images, filepathToSlash(f.Path)); ok {
+ out = append(out, entries...)
+ }
+ }
+ return out
+}
+
+// sortedKustomizationFiles keeps nonce minting deterministic: the same tree always produces
+// the same dyes, so a render is reproducible and a diff of two runs is empty.
+func sortedKustomizationFiles(files []manifestedit.FileContent) []manifestedit.FileContent {
+ var out []manifestedit.FileContent
+ for _, f := range files {
+ if isKustomizationFile(f.Path) {
+ out = append(out, f)
+ }
+ }
+ sort.Slice(out, func(i, j int) bool { return out[i].Path < out[j].Path })
+ return out
+}
diff --git a/internal/manifestanalyzer/dye_test.go b/internal/manifestanalyzer/dye_test.go
new file mode 100644
index 00000000..f6719acd
--- /dev/null
+++ b/internal/manifestanalyzer/dye_test.go
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package manifestanalyzer
+
+import (
+ "regexp"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+
+ "github.com/ConfigButler/gitops-reverser/internal/git/manifestedit"
+)
+
+// The dye's guardrails. Every one of these is a CORRECTNESS requirement rather than a
+// preference, and every one was measured against kustomize rather than reasoned about.
+// See docs/design/support-boundary/render-attribution.md §3 and §7.
+
+// The charset is the one that will silently ruin a render if it is got wrong.
+//
+// kustomize does not validate a tag or a digest — anything renders through. But it MATCHES on
+// the whole image string with a regex, so a nonce outside that charset leaves the image
+// un-matchable and EVERY LATER ENTRY SILENTLY STOPS FIRING. No error, just a different render,
+// and an attribution that credits the wrong entry.
+func TestPlanDye_NoncesStayInsideKustomizesMatchCharset(t *testing.T) {
+ // Exactly kustomize's own pattern (api/internal/image/image.go), for the image name "app".
+ matcher := regexp.MustCompile(imageNamePattern("app"))
+
+ plan := planDye([]manifestedit.FileContent{
+ {Path: "kustomization.yaml", Content: []byte(
+ "resources:\n - d.yaml\nimages:\n" +
+ " - name: app\n newTag: v1\n" +
+ " - name: app\n digest: sha256:abc\n")},
+ })
+ require.NotEmpty(t, plan.byNonce)
+
+ for nonce, mark := range plan.byNonce {
+ var image string
+ switch mark.field {
+ case fieldNewTag:
+ image = "app:" + nonce
+ case fieldDigest:
+ image = "app@" + nonce
+ default:
+ continue
+ }
+ require.True(t, matcher.MatchString(image),
+ "the dyed image %q must still match kustomize's own pattern, or every later entry "+
+ "silently stops firing", image)
+ }
+}
+
+// A digest dye MUST carry the sha256: prefix. The pattern is `(@sha256:[...])?` — a bare nonce
+// after the @ does not match, and the next entry goes quietly dead. Measured.
+func TestPlanDye_DigestNonceCarriesTheMandatorySha256Prefix(t *testing.T) {
+ plan := planDye([]manifestedit.FileContent{
+ {Path: "kustomization.yaml", Content: []byte(
+ "resources:\n - d.yaml\nimages:\n - name: app\n digest: sha256:abc\n")},
+ })
+
+ found := false
+ for nonce, mark := range plan.byNonce {
+ if mark.field != fieldDigest {
+ continue
+ }
+ found = true
+ require.Regexp(t, `^sha256:[a-zA-Z0-9]+$`, nonce,
+ "a digest dye without the sha256: prefix disables every later entry")
+ }
+ require.True(t, found, "the digest entry must have been dyed")
+}
+
+// Only a field the entry ALREADY DECLARES may be dyed. Injecting a newTag into a
+// newName-only entry would fabricate a supplier that does not exist, and the projection would
+// then route a tag edit onto an entry that never set a tag.
+func TestPlanDye_OnlyDyesFieldsTheEntryDeclares(t *testing.T) {
+ plan := planDye([]manifestedit.FileContent{
+ {Path: "kustomization.yaml", Content: []byte(
+ "resources:\n - d.yaml\nimages:\n - name: app\n newName: mirror/app\n")},
+ })
+
+ for _, mark := range plan.byNonce {
+ require.Equal(t, fieldNewName, mark.field,
+ "the entry declares only newName, so only newName may carry a dye")
+ }
+}
+
+// newName is NOT a pure sink — it is the join key for every later entry — so dyeing it can
+// change which entries match and alter the render's shape. The guard is exact: a newName may be
+// dyed unless some other entry's name: matches it.
+func TestDyeingNamesIsSafe(t *testing.T) {
+ tests := []struct {
+ name string
+ entries []ImageOverride
+ safe bool
+ }{
+ {
+ name: "no rename at all",
+ entries: []ImageOverride{
+ {Name: "app", NewTag: "v2", HasNewTag: true},
+ },
+ safe: true,
+ },
+ {
+ name: "a rename nothing else refers to",
+ entries: []ImageOverride{
+ {Name: "app", NewName: "mirror/app", HasNewName: true},
+ {Name: "other", NewTag: "v2", HasNewTag: true},
+ },
+ safe: true,
+ },
+ {
+ name: "a rename CHAIN: a later entry keys off the new name",
+ entries: []ImageOverride{
+ {Name: "app", NewName: "mirror/app", HasNewName: true},
+ {Name: "mirror/app", NewTag: "v2", HasNewTag: true},
+ },
+ safe: false,
+ },
+ {
+ // The guard must be asked of kustomize's compiled pattern, not of string
+ // equality: an entry's name is a REGULAR EXPRESSION, so `mirror/ap.` matches
+ // `mirror/app` without being equal to it. A string-equality guard would dye the
+ // name here, kill the second entry, and mis-attribute the tag.
+ name: "a rename chain joined by a REGEX, not by string equality",
+ entries: []ImageOverride{
+ {Name: "app", NewName: "mirror/app", HasNewName: true},
+ {Name: "mirror/ap.", NewTag: "v2", HasNewTag: true},
+ },
+ safe: false,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ require.Equal(t, tc.safe, dyeingNamesIsSafe(tc.entries))
+ })
+ }
+}
+
+// The dye must be reproducible: the same tree mints the same nonces every time, or two renders
+// of an unchanged repository would disagree and the store would flap.
+func TestPlanDye_IsDeterministic(t *testing.T) {
+ files := []manifestedit.FileContent{
+ {
+ Path: "b/kustomization.yaml",
+ Content: []byte("resources:\n - d.yaml\nimages:\n - name: b\n newTag: v1\n"),
+ },
+ {
+ Path: "a/kustomization.yaml",
+ Content: []byte("resources:\n - d.yaml\nimages:\n - name: a\n newTag: v1\n"),
+ },
+ }
+
+ first := planDye(files)
+ for range 5 {
+ require.Equal(t, first.replace, planDye(files).replace)
+ }
+}
diff --git a/internal/manifestanalyzer/kustomize_render.go b/internal/manifestanalyzer/kustomize_render.go
index d3a913af..195031f3 100644
--- a/internal/manifestanalyzer/kustomize_render.go
+++ b/internal/manifestanalyzer/kustomize_render.go
@@ -124,10 +124,34 @@ const renderMountPoint = "/scan"
// build never touches the real disk, never executes a plugin, and never reaches the
// network.
func renderRoot(files []manifestedit.FileContent, rootDir string) ([]renderedObject, error) {
- if err := refuseBeforeBuild(parseKustomizations(files), rootDir); err != nil {
+ return renderRootWith(files, rootDir, nil)
+}
+
+// renderRootWith is renderRoot over a COUNTERFACTUAL tree: replace layers in-memory
+// content for any scanned path, and the build sees that instead of what the scan holds.
+//
+// It is the whole new API this workstream needs, and every question we ask kustomize is
+// this call with a different overlay:
+//
+// - dye an entry and see where the nonce lands -> attribution;
+// - apply a proposed write and see what it renders -> verification.
+//
+// The point is that the counterfactual goes through the SAME sandbox, the same refusals
+// and the same kustomize invocation as the real render. A question answered by a
+// different renderer than the one that produces the answer we ship is not an answer.
+//
+// files is never mutated: callers hold the scan, and a probe must not be able to corrupt it.
+// See docs/design/support-boundary/render-attribution.md §7.
+func renderRootWith(
+ files []manifestedit.FileContent,
+ rootDir string,
+ replace map[string][]byte,
+) ([]renderedObject, error) {
+ input := replacedFiles(files, replace)
+ if err := refuseBeforeBuild(parseKustomizations(input), rootDir); err != nil {
return nil, err
}
- fSys, err := renderFilesystem(files, rootDir)
+ fSys, err := renderFilesystem(input, rootDir)
if err != nil {
return nil, err
}
@@ -138,6 +162,25 @@ func renderRoot(files []manifestedit.FileContent, rootDir string) ([]renderedObj
return collectRendered(resMap, rootDir)
}
+// replacedFiles returns a copy of files with replace layered over it, keyed by slash path.
+//
+// A replacement for a path the scan does not hold is ignored rather than appended: a
+// counterfactual may only perturb files that are actually in the tree kustomize is about
+// to build, and inventing one would be a way to render something the repository does not
+// contain.
+func replacedFiles(files []manifestedit.FileContent, replace map[string][]byte) []manifestedit.FileContent {
+ if len(replace) == 0 {
+ return files
+ }
+ out := append([]manifestedit.FileContent(nil), files...)
+ for i := range out {
+ if content, ok := replace[filepathToSlash(out[i].Path)]; ok {
+ out[i].Content = content
+ }
+ }
+ return out
+}
+
// build runs krusty, converting a panic into an error (errBuildPanicked).
//
// LoadRestrictionsNone is what Flux itself builds with, and it is safe here for the same
@@ -223,7 +266,6 @@ func unbuildable(k *kustomizationDoc) error {
// for provenance.
func renderFilesystem(files []manifestedit.FileContent, rootDir string) (filesys.FileSystem, error) {
fSys := filesys.MakeFsInMemory()
- rootKust := path.Join(rootDir, "kustomization.yaml")
for _, f := range files {
rel := filepathToSlash(f.Path)
@@ -231,7 +273,15 @@ func renderFilesystem(files []manifestedit.FileContent, rootDir string) (filesys
// Only the root needs to ask for provenance: the annotations describe the
// whole build, bases included.
- if rel == rootKust {
+ //
+ // The root is matched the way the rest of the analyzer matches one —
+ // isKustomizationFile, which accepts kustomization.yml as well. Comparing
+ // against the literal "kustomization.yaml" instead left a .yml root building
+ // happily with NO buildMetadata: no origin, no transformations, so every
+ // object came back with an empty OriginPath and an empty override chain, and
+ // a governed image was written into the source manifest for the overlay to
+ // shadow straight back.
+ if slashDir(rel) == rootDir && isKustomizationFile(rel) {
var k kustypes.Kustomization
if err := k.Unmarshal(content); err != nil {
return nil, fmt.Errorf("%s: %w", rel, err)
diff --git a/internal/manifestanalyzer/kustomize_render_hostile_test.go b/internal/manifestanalyzer/kustomize_render_hostile_test.go
index d2a8068b..a3a5ca92 100644
--- a/internal/manifestanalyzer/kustomize_render_hostile_test.go
+++ b/internal/manifestanalyzer/kustomize_render_hostile_test.go
@@ -38,13 +38,60 @@ func TestRenderRoot_ValidRegexImageNameStillBuilds(t *testing.T) {
require.NoError(t, err)
require.Len(t, rendered, 1)
- slots := collectContainerSlots(rendered[0].Object.Object)
+ slots := collectImageSlots(rendered[0].Object.Object)
require.Len(t, slots, 1)
- // Measured against kustomize: `ngin.` matches `nginx`. Our own renderImage compares
- // names for equality and does NOT — see docs/design/support-boundary/render-attribution.md.
+ // Measured against kustomize: `ngin.` matches `nginx`. Our own matcher used to compare
+ // names for equality and did NOT (B1) — which is why attribution now reads kustomize's
+ // dyes instead of re-deciding what matched.
require.Equal(t, "nginx:2.0", slots[0].image)
}
+// The counterfactual render must go through the same renderer as the baseline, and must not
+// be able to corrupt the scan it is probing. Both halves matter: attribution and verification
+// are only worth anything if the question is asked of the renderer that gives the answer.
+func TestRenderRootWith_RendersTheReplacementAndLeavesTheScanAlone(t *testing.T) {
+ files := imageFixture("nginx:v1", " - name: nginx\n newTag: v2\n")
+ replacement := []byte("resources:\n - deployment.yaml\nimages:\n - name: nginx\n newTag: v3\n")
+
+ rendered, err := renderRootWith(files, ".", map[string][]byte{"kustomization.yaml": replacement})
+
+ require.NoError(t, err)
+ require.Len(t, rendered, 1)
+ slots := collectImageSlots(rendered[0].Object.Object)
+ require.Len(t, slots, 1)
+ require.Equal(t, "nginx:v3", slots[0].image, "the build must see the counterfactual")
+ require.Contains(t, string(files[1].Content), "newTag: v2", "the scan must survive the probe unchanged")
+}
+
+// A replacement for a path the scan does not hold is ignored, never appended: a probe may
+// perturb the tree kustomize builds, not invent a file the repository does not contain.
+func TestRenderRootWith_IgnoresAReplacementForAnAbsentPath(t *testing.T) {
+ files := imageFixture("nginx:v1", " - name: nginx\n newTag: v2\n")
+
+ rendered, err := renderRootWith(files, ".", map[string][]byte{
+ "not/in/the/scan.yaml": []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: ghost\n"),
+ })
+
+ require.NoError(t, err)
+ require.Len(t, rendered, 1, "the phantom file must not have been rendered")
+}
+
+// kustomization.yml is a build directive everywhere else in the analyzer, so it has to be
+// one here too. Matching the root against the literal "kustomization.yaml" left a .yml root
+// building with no buildMetadata at all: the objects came back with no origin and no
+// transformations, which reads downstream as "this file is governed by nothing".
+func TestRenderRoot_KustomizationYMLCarriesProvenance(t *testing.T) {
+ files := imageFixture("nginx:v1", " - name: nginx\n newTag: v2\n")
+ files[1].Path = "kustomization.yml"
+
+ rendered, err := renderRoot(files, ".")
+
+ require.NoError(t, err)
+ require.Len(t, rendered, 1)
+ require.Equal(t, "deployment.yaml", rendered[0].OriginPath, "the source file must be attributable")
+ require.NotEmpty(t, rendered[0].TransformedBy, "the override chain must be readable")
+}
+
// The net under krusty: whatever panics in there, the caller gets an error and the process
// keeps its footing. Driven straight at build(), because the refusal above means the panic
// we know about can no longer reach it.
diff --git a/internal/manifestanalyzer/kustomize_render_semantics_test.go b/internal/manifestanalyzer/kustomize_render_semantics_test.go
index 788bebd0..8b652afc 100644
--- a/internal/manifestanalyzer/kustomize_render_semantics_test.go
+++ b/internal/manifestanalyzer/kustomize_render_semantics_test.go
@@ -4,117 +4,19 @@ package manifestanalyzer
import (
"fmt"
- "testing"
-
- "github.com/stretchr/testify/require"
"github.com/ConfigButler/gitops-reverser/internal/git/manifestedit"
)
-// The layout corpus barely exercises images:, so on its own it cannot license
-// deleting our re-implemented image transformer. This table does: it drives the
-// cases the re-implementation actually has to get right — chained renames, digest
-// precedence, a registry port that looks like a tag — through BOTH kustomize and
-// renderImage, and requires them to agree byte for byte.
+// TestRenderImage_MatchesKustomizeOnTheHardCases lived here: eleven hand-picked cases driven
+// through BOTH kustomize and our own renderImage, required to agree. It is gone because
+// renderImage is gone — there is no second opinion left to pin to the first.
//
-// Every row is a claim about kustomize's semantics that our code depends on. If
-// kustomize ever changes one, this test fails instead of the operator silently
-// writing a file that renders to something else.
-
-func TestRenderImage_MatchesKustomizeOnTheHardCases(t *testing.T) {
- cases := []struct {
- name string
- source string // the container image in the source Deployment
- images string // the images: block of the overlay kustomization
- }{
- {
- name: "newTag only",
- source: "ghcr.io/org/web:1.0",
- images: " - name: ghcr.io/org/web\n newTag: \"2.0\"\n",
- },
- {
- name: "newName only",
- source: "ghcr.io/org/web:1.0",
- images: " - name: ghcr.io/org/web\n newName: ghcr.io/org/web-hardened\n",
- },
- {
- name: "newName and newTag in one entry",
- source: "ghcr.io/org/web:1.0",
- images: " - name: ghcr.io/org/web\n newName: ghcr.io/org/hardened\n newTag: \"3.0\"\n",
- },
- {
- name: "digest replaces the tag",
- source: "ghcr.io/org/web:1.0",
- images: " - name: ghcr.io/org/web\n digest: sha256:abc123\n",
- },
- {
- // kustomize's own doc on types.Image: "If digest is present NewTag value
- // is ignored." Our renderImage must not disagree.
- name: "digest wins over newTag in the same entry",
- source: "ghcr.io/org/web:1.0",
- images: " - name: ghcr.io/org/web\n newTag: \"9.9\"\n digest: sha256:abc123\n",
- },
- {
- // The chain interaction that makes a naive inversion wrong: the first
- // entry renames, and only then does the second entry's name match.
- name: "a rename makes a later entry match",
- source: "ghcr.io/org/web:1.0",
- images: " - name: ghcr.io/org/web\n newName: ghcr.io/org/renamed\n" +
- " - name: ghcr.io/org/renamed\n newTag: \"4.0\"\n",
- },
- {
- name: "the last matching entry wins",
- source: "ghcr.io/org/web:1.0",
- images: " - name: ghcr.io/org/web\n newTag: \"2.0\"\n" +
- " - name: ghcr.io/org/web\n newTag: \"5.0\"\n",
- },
- {
- name: "an entry that matches nothing changes nothing",
- source: "ghcr.io/org/web:1.0",
- images: " - name: ghcr.io/org/other\n newTag: \"7.0\"\n",
- },
- {
- // The parseImageRef edge case: the colon in a registry port is not a tag
- // separator.
- name: "a registry port is not a tag",
- source: "localhost:5000/org/web:1.0",
- images: " - name: localhost:5000/org/web\n newTag: \"2.0\"\n",
- },
- {
- name: "an untagged source image",
- source: "ghcr.io/org/web",
- images: " - name: ghcr.io/org/web\n newTag: \"2.0\"\n",
- },
- {
- name: "a source image that already carries a digest",
- source: "ghcr.io/org/web@sha256:oldoldold",
- images: " - name: ghcr.io/org/web\n newTag: \"2.0\"\n",
- },
- }
-
- for _, tc := range cases {
- t.Run(tc.name, func(t *testing.T) {
- files := imageFixture(tc.source, tc.images)
-
- // What kustomize actually renders.
- rendered, err := renderRoot(files, ".")
- require.NoError(t, err)
- require.Len(t, rendered, 1)
- slots := collectContainerSlots(rendered[0].Object.Object)
- require.Len(t, slots, 1)
- kustomizeSays := slots[0].image
-
- // What our re-implemented chain renders.
- kusts := parseKustomizations(files)
- doc := kusts["."]
- require.NotNil(t, doc)
- ours, _ := renderImage(parseImageRef(tc.source), doc.images)
-
- require.Equal(t, kustomizeSays, ours.String(),
- "kustomize renders %q; renderImage renders %q", kustomizeSays, ours.String())
- })
- }
-}
+// It is worth recording why the table was never enough, because it is the argument for the
+// whole workstream. Its rows were the cases we THOUGHT of. B1 — that an images: entry name is
+// a regex, so `- name: "ap."` matches `app` — was not among them, and shipped. A table of
+// cases we thought of is not a substitute for making kustomize the arbiter at runtime, which
+// is what the dye now does. See docs/design/support-boundary/render-attribution.md §6.
// imageFixture is a one-file render root: a Deployment with one container, plus a
// kustomization carrying the images: block under test.
diff --git a/internal/manifestanalyzer/kustomize_render_test.go b/internal/manifestanalyzer/kustomize_render_test.go
index 29a6d2b4..f136e8df 100644
--- a/internal/manifestanalyzer/kustomize_render_test.go
+++ b/internal/manifestanalyzer/kustomize_render_test.go
@@ -4,6 +4,7 @@ package manifestanalyzer
import (
"bytes"
+ "encoding/json"
"os"
"path/filepath"
"strings"
@@ -16,32 +17,42 @@ import (
"github.com/ConfigButler/gitops-reverser/internal/git/manifestedit"
)
-// This is the differential test that licenses deleting the re-implemented
-// transformers: for every kustomize render root in both corpora, the image our
-// renderImage chain produces must be byte-for-byte the image kustomize produces.
+// The corpus-wide invariant that licenses deleting the re-implemented transformers.
//
-// It used to also compare the override CHAIN against kustomize's transformations
-// annotation. That assertion has done its job and is gone: the chain is now READ
-// from that annotation (override_chain.go), so comparing the two would be comparing
-// kustomize to itself.
+// This test used to render every corpus image through our own renderImage chain and require it
+// to equal kustomize's. That comparison is gone with the code it compared — there is no second
+// opinion left to check against the first. What replaces it is stronger, and it is the property
+// the deleted code kept violating:
+//
+// AN IN-SYNC FOLDER MUST PROJECT TO A COMPLETE NO-OP.
+//
+// Take what kustomize renders and hand it back as the live object — the folder is by definition
+// already converged — then run the projection. It must route NO entry edits and it must hand
+// back the source document unchanged. Any disagreement between what we think a folder renders
+// to and what it actually renders shows up here as a phantom edit or a rewritten source file,
+// on every render root of every fixture in both corpora.
+//
+// That is exactly the shape of #231: a digest entry clears the tag, we thought it did not, and
+// on a perfectly in-sync folder the projection "helpfully" rewrote the tag out of the source
+// manifest. This test fails on that. The old one could not — it compared our belief to our
+// belief.
-func TestRenderRoot_ImagesAgreeWithKustomize(t *testing.T) {
+func TestProjection_InSyncCorpusFolderIsANoOp(t *testing.T) {
roots := allCorpusRenderRoots(t)
require.NotEmpty(t, roots, "no render roots found — the test would prove nothing")
- compared, skipped := 0, 0
+ checked, skipped := 0, 0
for _, root := range roots {
t.Run(root.name, func(t *testing.T) {
rendered, err := renderRoot(root.files, root.dir)
if err != nil {
- // A folder we refuse (remote base, generators, patches, plugins)
- // need not render: the gate refuses it and the writer never sees it.
+ // A folder we refuse (remote base, generators, patches, plugins) need not
+ // render: the gate refuses it and the writer never sees it.
skipped++
t.Skipf("not renderable, and refused by the acceptance gate: %v", err)
}
+ chains, _ := renderChains(root.files, parseKustomizations(root.files))
- kusts := parseKustomizations(root.files)
- chains, _ := renderChains(root.files, kusts) // once per fixture, not once per object
for _, ro := range rendered {
if ro.OriginPath == "" {
continue // a generated resource; generators are refused
@@ -50,60 +61,84 @@ func TestRenderRoot_ImagesAgreeWithKustomize(t *testing.T) {
if src == nil {
continue // renamed by a transformer we refuse; not a supported shape
}
- chain, ambiguous := ourChainFor(chains, ro)
- if ambiguous {
- continue // we route nothing through it; there is no claim to check
+ attribution, ambiguous := ourAttributionFor(chains, ro)
+ if ambiguous || attribution == nil {
+ continue // nothing is routed through it, so there is no claim to check
}
- compared += assertImagesMatchKustomize(t, ro, src, chain)
+ assertInSyncIsANoOp(t, ro, src, attribution)
+ checked++
}
})
}
- t.Logf("compared %d rendered images against the hand-rolled chain (%d roots skipped as refused)",
- compared, skipped)
+ t.Logf("checked %d rendered documents for no-op projection (%d roots skipped as refused)",
+ checked, skipped)
}
-// assertImagesMatchKustomize renders each source container image through our own
-// chain and requires it to equal what kustomize actually produced. Returns the
-// number of images compared.
-func assertImagesMatchKustomize(
+// assertInSyncIsANoOp hands kustomize's own render back as the live object — the folder is by
+// definition converged — and requires the projection to route nothing and to leave the source
+// document's images exactly as they are.
+func assertInSyncIsANoOp(
t *testing.T,
ro renderedObject,
src *unstructured.Unstructured,
- chain *KustomizeOverrides,
-) int {
+ attribution *RenderedOverrides,
+) {
t.Helper()
- var entries []ImageOverride
- if chain != nil {
- entries = chain.Images
- }
- ours := map[string]string{}
- for _, slot := range collectContainerSlots(src.Object) {
- got, _ := renderImage(parseImageRef(slot.image), entries)
- ours[slot.key] = got.String()
+ where := ro.OriginPath + " " + ro.Object.GetKind() + "/" + ro.Object.GetName()
+
+ out, edits := SplitDesiredForOverrides(src.Object, asLiveObject(t, ro.Object), attribution)
+
+ require.Empty(t, edits,
+ "%s: the folder is already in sync, so nothing may be routed to an entry", where)
+
+ for _, slot := range collectImageSlots(out.Object) {
+ want := sourceImageAt(src.Object, slot.key)
+ if want == "" {
+ continue // the live object has a slot the source does not; not our claim
+ }
+ require.Equal(t, want, slot.image,
+ "%s: an in-sync folder must hand back the SOURCE image untouched at %q", where, slot.key)
}
+}
- compared := 0
- for _, slot := range collectContainerSlots(ro.Object.Object) {
- got, known := ours[slot.key]
- if !known {
- continue
+// asLiveObject turns a RENDERED object into the shape a live one actually has.
+//
+// This is not a formality. A rendered object is NOT a valid unstructured: kustomize hands
+// numbers back as Go `int`, and DeepCopyJSON accepts only the JSON types, so
+// unstructured.DeepCopy PANICS on one outright ("cannot deep copy int"). The API server hands
+// out JSON, so a real live object has int64 — round-tripping through JSON is what makes this
+// fixture faithful rather than merely non-crashing.
+//
+// The same landmine sits under any code that reads a number off a rendered object with the
+// standard helpers: unstructured.NestedInt64 reports found=FALSE on a rendered spec.replicas,
+// and silently gives you zero. See renderedReplicaCount, which is why the projection does not
+// use it.
+func asLiveObject(t *testing.T, rendered *unstructured.Unstructured) *unstructured.Unstructured {
+ t.Helper()
+ encoded, err := json.Marshal(rendered.Object)
+ require.NoError(t, err)
+ var obj map[string]interface{}
+ require.NoError(t, json.Unmarshal(encoded, &obj))
+ return &unstructured.Unstructured{Object: obj}
+}
+
+// sourceImageAt is the image the SOURCE document holds at a slot, or "" when it has none.
+func sourceImageAt(src map[string]interface{}, key string) string {
+ for _, slot := range collectImageSlots(src) {
+ if slot.key == key {
+ return slot.image
}
- want := slot.image // kustomize's render is the expected truth
- compared++
- require.Equal(t, want, got,
- "%s container %s: kustomize renders %q, our chain renders %q",
- ro.OriginPath, slot.key, want, got)
}
- return compared
+ return ""
}
-// ourChainFor is the override chain the store attributes to a document, and whether
-// it was found ambiguous (reached by more than one render root with differing
-// chains, which we refuse to route through).
-func ourChainFor(
+// ourAttributionFor is what the store attributes to a document, and whether it was found
+// ambiguous (reached by more than one render root with differing answers, which we refuse to
+// route through).
+func ourAttributionFor(
chains map[chainKey]*overrideAssignment,
ro renderedObject,
-) (*KustomizeOverrides, bool) {
+) (*RenderedOverrides, bool) {
a := chains[chainKey{
originPath: ro.OriginPath,
kind: ro.Object.GetKind(),
@@ -115,7 +150,7 @@ func ourChainFor(
if a.ambiguous() {
return nil, true
}
- return a.overrides, false
+ return a.rendered, false
}
// sourceDocFor finds the document in the origin file that produced a rendered
diff --git a/internal/manifestanalyzer/override_chain.go b/internal/manifestanalyzer/override_chain.go
index 934f3515..03d4aef7 100644
--- a/internal/manifestanalyzer/override_chain.go
+++ b/internal/manifestanalyzer/override_chain.go
@@ -146,6 +146,10 @@ func renderChains(
out := map[chainKey]*overrideAssignment{}
failed := map[string]string{}
+ // One dye plan for the whole scan, so the nonces are stable across roots and a base
+ // reached by two overlays is dyed identically in both. Building it costs no render.
+ plan := planDye(files)
+
for _, rootDir := range renderTargets(kusts) {
rendered, err := renderRoot(files, rootDir)
if err != nil {
@@ -154,6 +158,13 @@ func renderChains(
}
continue
}
+ // BASELINE FIRST, THEN DYE. The real render is what we ship; the dyed one is only
+ // ever a question we ask about it. A dyed build that fails where the real one
+ // succeeded means the dye perturbed something that is not a pure sink, and the
+ // answer is then no attribution — never a fallback to a second guess.
+ dyed, dyeErr := renderRootWith(files, rootDir, plan.replace)
+ attribution := attributeRoot(rendered, dyed, dyeErr, plan)
+
for _, ro := range rendered {
if ro.OriginPath == "" {
continue // a generated resource: it has no source document to edit
@@ -163,7 +174,7 @@ func renderChains(
kind: ro.Object.GetKind(),
name: ro.Object.GetName(),
}
- record(out, key, chainOf(ro, kusts))
+ record(out, key, chainOf(ro, kusts), attribution[key])
}
}
return out, failed
@@ -219,23 +230,47 @@ func chainOf(ro renderedObject, kusts map[string]*kustomizationDoc) *KustomizeOv
// anyOverrides preserves the existing narrowness of the fan-in refusal: a base
// document reached by two roots that declare no images:/replicas: at all is shared
// context, but nothing is at stake in it, so it is not refused.
-func record(out map[chainKey]*overrideAssignment, key chainKey, ov *KustomizeOverrides) {
- fp := fingerprint(ov)
+func record(out map[chainKey]*overrideAssignment, key chainKey, ov *KustomizeOverrides, rd *RenderedOverrides) {
+ // The fingerprint now covers the ATTRIBUTION as well as the chain, which is the sharper
+ // question the fan-in check was always trying to ask: two roots agree only if they
+ // attribute the same field to the same entry AND render it to the same value.
+ fp := fingerprint(ov) + "\x03" + fingerprintRendered(rd)
prev, seen := out[key]
if !seen {
out[key] = &overrideAssignment{
chainKeys: map[string]struct{}{fp: {}},
overrides: ov,
- anyOverrides: ov != nil,
+ rendered: rd,
+ anyOverrides: hasSomethingAtStake(ov, rd),
}
return
}
if _, same := prev.chainKeys[fp]; same {
- return // the same chain, reached twice; not an ambiguity
+ return // the same chain and the same attribution, reached twice; not an ambiguity
}
prev.chainKeys[fp] = struct{}{}
- prev.anyOverrides = prev.anyOverrides || ov != nil
- prev.overrides = nil // more than one distinct chain: route through none of them
+ prev.anyOverrides = prev.anyOverrides || hasSomethingAtStake(ov, rd)
+ // More than one distinct answer: route through none of them.
+ prev.overrides = nil
+ prev.rendered = nil
+}
+
+// hasSomethingAtStake reports whether a root's view of a document carries anything an edit
+// could be routed through, which is what makes a disagreement between two roots worth
+// refusing rather than merely noting.
+//
+// It asks about the ATTRIBUTION as well as the chain. The chain alone is not the same
+// question: an object can render with no images:/replicas: entry governing it at all (ov nil)
+// and still carry a rendered image the projection reads (rd non-nil). Two roots that
+// disagreed 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.
+//
+// That 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 here because the invariant it protects is not
+// obvious enough to leave resting on that argument.
+func hasSomethingAtStake(ov *KustomizeOverrides, rd *RenderedOverrides) bool {
+ return ov != nil || rd != nil
}
// fingerprint reduces a chain to a comparable string, so two roots reaching one
diff --git a/internal/manifestanalyzer/overrides.go b/internal/manifestanalyzer/overrides.go
index 7445a5b1..36ca8c8a 100644
--- a/internal/manifestanalyzer/overrides.go
+++ b/internal/manifestanalyzer/overrides.go
@@ -72,6 +72,7 @@ type KustomizeOverrides struct {
type overrideAssignment struct {
chainKeys map[string]struct{}
overrides *KustomizeOverrides
+ rendered *RenderedOverrides
anyOverrides bool
}
@@ -87,17 +88,17 @@ func resolveOverrides(
loc manifestedit.Location,
id manifestedit.Identity,
assignments map[chainKey]*overrideAssignment,
-) (*KustomizeOverrides, *manifestedit.Diagnostic) {
+) (*KustomizeOverrides, *RenderedOverrides, *manifestedit.Diagnostic) {
a := assignments[chainKey{
originPath: filepathToSlash(loc.Path),
kind: id.Kind,
name: id.Name,
}]
if a == nil {
- return nil, nil
+ return nil, nil, nil
}
if a.ambiguous() {
- return nil, &manifestedit.Diagnostic{
+ return nil, nil, &manifestedit.Diagnostic{
Level: manifestedit.DiagWarning,
Reason: reasonAmbiguousOverrides,
Message: "multiple render roots reach this file with different images/replicas override chains; " +
@@ -106,7 +107,7 @@ func resolveOverrides(
DocumentIndex: loc.DocumentIndex,
}
}
- return a.overrides, nil
+ return a.overrides, a.rendered, nil
}
// OverridesAmbiguousAt reports whether the store refused to route a kustomize override chain
diff --git a/internal/manifestanalyzer/overrides_attribution.go b/internal/manifestanalyzer/overrides_attribution.go
new file mode 100644
index 00000000..cfbdc318
--- /dev/null
+++ b/internal/manifestanalyzer/overrides_attribution.go
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package manifestanalyzer
+
+import (
+ "math"
+ "sort"
+ "strconv"
+ "strings"
+)
+
+// This file turns a pair of renders — the real one and the dyed one — into the answer the
+// projection needs: for this document, what does kustomize render each image and replica
+// count to, and WHICH ENTRY supplied it.
+//
+// Both halves come from the renderer. The VALUES are read off the real render, so what we
+// believe a folder renders to is what kustomize says it renders to. The SUPPLIERS are read
+// off the dyed render, so which entry owns a field is observed rather than re-derived.
+// Nothing here re-implements a transformer, which is the whole point: the transformers we
+// used to re-implement are where every shipped bug in this area came from.
+//
+// See docs/design/support-boundary/render-attribution.md §3.
+
+// RenderedOverrides is what kustomize renders one document to, plus the override entry
+// behind each override-produced value. A nil supplier means THE SOURCE DOCUMENT supplies
+// that value — so an edit to it belongs in the file, not in an entry.
+type RenderedOverrides struct {
+ // Images is keyed by image slot (the container list path plus the container name), so
+ // the live object, the Git document and the render all address the same field.
+ Images map[string]RenderedImage
+ // Replicas is set only when the document actually renders a spec.replicas — which is
+ // kustomize's decision, not ours.
+ Replicas *RenderedReplicas
+}
+
+// RenderedImage is one image slot: what it renders to, and who supplied each component.
+type RenderedImage struct {
+ // Rendered is the image kustomize produces for this slot.
+ Rendered string
+ // Name, Tag and Digest are the entries supplying each component, or nil when the
+ // source document does. They are the dye's answer, and the reason renderImage is gone.
+ Name, Tag, Digest *ImageOverride
+}
+
+// RenderedReplicas is the rendered spec.replicas and the entry that pinned it (nil when the
+// source document supplies the count).
+type RenderedReplicas struct {
+ Rendered int64
+ Entry *ReplicaOverride
+}
+
+// attributeRoot pairs one root's real render with its dyed render and reads the dyes off it.
+//
+// BASELINE FIRST, THEN DYE. If the dyed build fails where the real one succeeded, the dye has
+// perturbed something that is not a pure sink — a replacements: block consuming the image as
+// a SOURCE will do exactly this — and the honest answer is that we cannot attribute this root.
+// The fallback is NO ATTRIBUTION, and it is never another heuristic: not renderImage, not
+// leave-one-out, not "probably the last matching entry". Those are the silent-corruption paths
+// this design exists to delete, and the moment the renderer says "I cannot tell you" is the
+// worst possible moment to start guessing. With no attribution nothing routes to an entry, the
+// proposal falls back to what the source document alone can carry, and the verification
+// re-render adjudicates it — which, for a field an entry governs, means a refused flush. That
+// is the correct outcome, and it is reported rather than absorbed.
+//
+// Objects are aligned BY POSITION, never by name: a generated name can carry a content hash
+// that drifts between two builds. The keys are then required to agree at every position, so a
+// misalignment refuses attribution instead of silently attributing one object's dyes to
+// another.
+func attributeRoot(plain, dyed []renderedObject, dyeErr error, plan *dyePlan) map[chainKey]*RenderedOverrides {
+ if dyeErr != nil || len(plain) != len(dyed) {
+ return nil
+ }
+ out := make(map[chainKey]*RenderedOverrides, len(plain))
+ for i := range plain {
+ key := renderedKey(plain[i])
+ if key != renderedKey(dyed[i]) {
+ return nil // the two builds disagree on what they rendered; attribute nothing
+ }
+ if plain[i].OriginPath == "" {
+ continue // generated: no source document to route an edit into
+ }
+ if attribution := readDyes(plain[i], dyed[i], plan); attribution != nil {
+ out[key] = attribution
+ }
+ }
+ return out
+}
+
+func renderedKey(o renderedObject) chainKey {
+ return chainKey{originPath: o.OriginPath, kind: o.Object.GetKind(), name: o.Object.GetName()}
+}
+
+// readDyes reads the nonces out of ONE document's image slots and replica count.
+//
+// Only the fields being attributed are read. The whole output is never grepped for a nonce:
+// vars and replacements can carry a dyed value into args, env, or ConfigMap data, and a dye
+// found there says nothing about who supplies the image.
+func readDyes(plain, dyed renderedObject, plan *dyePlan) *RenderedOverrides {
+ out := &RenderedOverrides{Images: map[string]RenderedImage{}}
+
+ dyedSlots := map[string]imageSlot{}
+ for _, s := range collectImageSlots(dyed.Object.Object) {
+ dyedSlots[s.key] = s
+ }
+ for _, slot := range collectImageSlots(plain.Object.Object) {
+ image := RenderedImage{Rendered: slot.image}
+ if probe, found := dyedSlots[slot.key]; found {
+ ref := parseImageRef(probe.image)
+ image.Name = markedImage(plan, ref.name, fieldNewName)
+ image.Tag = markedImage(plan, ref.tag, fieldNewTag)
+ image.Digest = markedImage(plan, ref.digest, fieldDigest)
+ }
+ out.Images[slot.key] = image
+ }
+
+ // spec.replicas is read off the object rather than gated on a list of kinds we believe
+ // the transformer touches. kustomize's fieldspec is the authority: if a dyed count came
+ // out here, an entry governs this field, whatever the kind is. That is how
+ // ReplicationController — which our own isReplicaKind forgot — attributes for free.
+ if count, ok := renderedReplicaCount(plain.Object.Object); ok {
+ replicas := &RenderedReplicas{Rendered: count}
+ if dyedCount, found := renderedReplicaCount(dyed.Object.Object); found {
+ if mark, isDye := plan.byReplicaCount[dyedCount]; isDye && mark.field == fieldCount {
+ replicas.Entry = mark.replica
+ }
+ }
+ out.Replicas = replicas
+ }
+
+ if len(out.Images) == 0 && out.Replicas == nil {
+ return nil
+ }
+ return out
+}
+
+// markedImage looks a candidate nonce up, and requires it to have been minted FOR THE FIELD IT
+// WAS FOUND IN. A tag dye surfacing in the name position is not attribution, it is a signal
+// that something we do not model moved the value, so it is not treated as a supplier.
+func markedImage(plan *dyePlan, value, field string) *ImageOverride {
+ if value == "" {
+ return nil
+ }
+ mark, found := plan.byNonce[value]
+ if !found || mark.field != field {
+ return nil
+ }
+ return mark.image
+}
+
+// renderedReplicaCount reads spec.replicas off a RENDERED object.
+//
+// unstructured.NestedInt64 is deliberately not used, and this is a landmine rather than a
+// preference: kustomize hands numbers back as Go `int`, so NestedInt64 returns found=FALSE on
+// a rendered spec.replicas and the caller silently reads zero. Measured.
+func renderedReplicaCount(obj map[string]interface{}) (int64, bool) {
+ spec, ok := obj["spec"].(map[string]interface{})
+ if !ok {
+ return 0, false
+ }
+ switch n := spec["replicas"].(type) {
+ case int:
+ return int64(n), true
+ case int64:
+ return n, true
+ case float64:
+ if n != math.Trunc(n) {
+ return 0, false
+ }
+ return int64(n), true
+ default:
+ return 0, false
+ }
+}
+
+// fingerprintRendered reduces an attribution to a comparable string, so two render roots
+// reaching one document can be compared for agreement exactly as their chains are. This is the
+// sharper question the fan-in check was always trying to ask: do two roots attribute this
+// field to DIFFERENT entries?
+func fingerprintRendered(rd *RenderedOverrides) string {
+ if rd == nil {
+ return ""
+ }
+ keys := make([]string, 0, len(rd.Images))
+ for k := range rd.Images {
+ keys = append(keys, k)
+ }
+ sort.Strings(keys)
+
+ var b strings.Builder
+ for _, k := range keys {
+ img := rd.Images[k]
+ b.WriteString(k)
+ b.WriteByte(0)
+ b.WriteString(img.Rendered)
+ b.WriteByte(0)
+ b.WriteString(entryRef(img.Name) + entryRef(img.Tag) + entryRef(img.Digest))
+ b.WriteByte(1)
+ }
+ if rd.Replicas != nil {
+ b.WriteString(strconv.FormatInt(rd.Replicas.Rendered, 10))
+ b.WriteByte(0)
+ if e := rd.Replicas.Entry; e != nil {
+ b.WriteString(e.Source + ":" + strconv.Itoa(e.Index))
+ }
+ }
+ return b.String()
+}
+
+func entryRef(e *ImageOverride) string {
+ if e == nil {
+ return "-;"
+ }
+ return e.Source + ":" + strconv.Itoa(e.Index) + ";"
+}
diff --git a/internal/manifestanalyzer/overrides_projection.go b/internal/manifestanalyzer/overrides_projection.go
index 005566d5..14c2a2f7 100644
--- a/internal/manifestanalyzer/overrides_projection.go
+++ b/internal/manifestanalyzer/overrides_projection.go
@@ -32,29 +32,38 @@ type OverrideEdit struct {
Edit manifestedit.KustomizationEdit
}
-// SplitDesiredForOverrides maps the live desired object back through the
-// override chain. It returns the object the source document should be compared
-// against (a copy of desired with override-produced values restored to their
-// source form) plus the entry edits for values whose supplier is an override
-// entry. Anything it cannot route safely — a component removal an entry
-// supplies, conflicting values for one entry field, or a simulated render that
-// would not reproduce live — falls back to the unmodified live value
-// (today's write-through), never to a guess.
+// SplitDesiredForOverrides maps the live desired object back through what kustomize actually
+// renders. It returns the object the source document should be compared against — a copy of
+// desired with every override-produced value restored to its SOURCE form, so the file keeps
+// its bytes — plus the entry edits for the values an override entry supplies.
//
-// gitRaw is the source document parsed as JSON-typed maps (sigs.k8s.io/yaml);
-// desired is the sanitized projection the writer would otherwise compare. The
-// returned object is always a copy; desired is never mutated.
+// It is driven by RenderedOverrides, which carries both halves of the answer straight from
+// the renderer: what each field renders to, and which entry supplied it (read off a dyed
+// counterfactual build). Nothing in here re-implements a kustomize transformer, which is the
+// entire point of this workstream — every shipped bug in this area came from the
+// re-implementation, and all of them are deleted with it.
+//
+// Anything it cannot route safely — a component removal an entry supplies, a component a
+// sibling entry clears, or two containers demanding different values for one entry field —
+// routes NOTHING and leaves the live value in place. That is not a guess and not a fallback to
+// another heuristic: the proposal then has to survive the verification re-render, which for a
+// field an entry governs it will not, so it becomes a reported refusal rather than a commit
+// that quietly never converges.
+//
+// gitRaw is the source document parsed as JSON-typed maps (sigs.k8s.io/yaml); desired is the
+// sanitized projection the writer would otherwise compare. The returned object is always a
+// copy; desired is never mutated.
func SplitDesiredForOverrides(
gitRaw map[string]interface{},
desired *unstructured.Unstructured,
- ov *KustomizeOverrides,
+ rendered *RenderedOverrides,
) (*unstructured.Unstructured, []OverrideEdit) {
- if ov == nil || desired == nil || gitRaw == nil {
+ if rendered == nil || desired == nil || gitRaw == nil {
return desired, nil
}
out := desired.DeepCopy()
- edits := projectImages(gitRaw, out, ov.Images)
- edits = append(edits, projectReplicas(gitRaw, out, ov.Replicas)...)
+ edits := projectImages(gitRaw, out, rendered.Images)
+ edits = append(edits, projectReplicas(gitRaw, out, rendered.Replicas)...)
return out, edits
}
@@ -93,65 +102,13 @@ func (r imageRef) String() string {
return out
}
-// imageSuppliers records which override entry last supplied each component of a
-// rendered image; nil means the source file supplies it.
-type imageSuppliers struct {
- name *ImageOverride
- tag *ImageOverride
- digest *ImageOverride
-}
-
-// renderImage runs the override chain over a source image, kustomize-style:
-// each entry whose name matches the image's CURRENT name rewrites the
-// components it declares, in chain order.
-//
-// Tag and digest are MUTUALLY EXCLUSIVE, and this is the part that is easy to get
-// wrong — we did. Quoting kustomize's own image transformer
-// (filters/imagetag/updater.go, SetImageValue):
-//
-// // overriding tag or digest will replace both original tag and digest values
-// case NewTag != "" && Digest != "": tag = NewTag; digest = Digest
-// case NewTag != "": tag = NewTag; digest = ""
-// case Digest != "": tag = ""; digest = Digest
-//
-// Setting the two components independently makes us believe a folder renders to
-// `web:1.0@sha256:abc` where kustomize renders `web@sha256:abc`. The projection
-// then reads the difference as a user removing the tag and rewrites the tag out of
-// the source file — silent corruption, on every reconcile. Pinned against a real
-// `kustomize build` by TestRenderImage_MatchesKustomizeOnTheHardCases.
-func renderImage(src imageRef, entries []ImageOverride) (imageRef, imageSuppliers) {
- cur := src
- var sup imageSuppliers
- for i := range entries {
- e := &entries[i]
- if e.Name != cur.name {
- continue
- }
- if e.HasNewName {
- cur.name = e.NewName
- sup.name = e
- }
- switch {
- case e.HasNewTag && e.HasDigest:
- cur.tag, cur.digest = e.NewTag, e.Digest
- sup.tag, sup.digest = e, e
- case e.HasNewTag:
- cur.tag, cur.digest = e.NewTag, ""
- sup.tag, sup.digest = e, e
- case e.HasDigest:
- cur.tag, cur.digest = "", e.Digest
- sup.tag, sup.digest = e, e
- }
- }
- return cur, sup
-}
-
-// containerSlot is one container-shaped list item holding an image, addressed
-// by its list path plus container name so the live and Git objects align.
-type containerSlot struct {
+// imageSlot is one image-bearing field of an object, addressed by its list path plus the
+// item's name so the live object, the Git document and the render all address the same one.
+// set writes a new image back into whichever shape the field has.
+type imageSlot struct {
key string
- item map[string]interface{}
image string
+ set func(string)
}
func isContainerListKey(k string) bool {
@@ -163,24 +120,38 @@ func isContainerListKey(k string) bool {
}
}
-// collectContainerSlots walks the object for container lists at any depth,
-// mirroring the builtin image transformer's generic traversal. Slots are sorted
-// by key for deterministic edit output.
-func collectContainerSlots(obj map[string]interface{}) []containerSlot {
- var out []containerSlot
+// collectImageSlots walks the object for every field that can hold an image.
+//
+// Which fields those are was MEASURED against kustomize, not derived from its fieldspecs,
+// and the two surprises are both in here:
+//
+// - volumes[].image.reference — an OCI volume source. kustomize REWRITES it (measured), and
+// the old collector did not look at it, so the rendered value was written back into the
+// source document as if the user had typed it.
+// - ephemeralContainers — kustomize does NOT rewrite them (measured), so no dye ever lands
+// here and no entry is ever credited with the value. They are still collected, because the
+// SOURCE document owns them and an edit to one belongs in the file. That is the dye doing
+// the fieldspec's job: we no longer have to know which fields kustomize touches, only to
+// look at where its dyes came out.
+//
+// Slots are sorted by key so edit output is deterministic.
+func collectImageSlots(obj map[string]interface{}) []imageSlot {
+ var out []imageSlot
var walk func(prefix string, v interface{})
walk = func(prefix string, v interface{}) {
switch t := v.(type) {
case map[string]interface{}:
for k, val := range t {
p := prefix + "/" + k
- if isContainerListKey(k) {
- if list, ok := val.([]interface{}); ok {
- out = append(out, containerSlotsOf(p, list)...)
- continue
- }
+ list, isList := val.([]interface{})
+ switch {
+ case isList && isContainerListKey(k):
+ out = append(out, containerImageSlots(p, list)...)
+ case isList && k == "volumes":
+ out = append(out, volumeImageSlots(p, list)...)
+ default:
+ walk(p, val)
}
- walk(p, val)
}
case []interface{}:
for i, item := range t {
@@ -193,8 +164,9 @@ func collectContainerSlots(obj map[string]interface{}) []containerSlot {
return out
}
-func containerSlotsOf(listPath string, list []interface{}) []containerSlot {
- var out []containerSlot
+// containerImageSlots reads containers[].image — a plain string field.
+func containerImageSlots(listPath string, list []interface{}) []imageSlot {
+ var out []imageSlot
for _, item := range list {
m, isMap := item.(map[string]interface{})
if !isMap {
@@ -205,73 +177,112 @@ func containerSlotsOf(listPath string, list []interface{}) []containerSlot {
if name == "" || !hasImage {
continue
}
- out = append(out, containerSlot{key: listPath + "\x00" + name, item: m, image: image})
+ out = append(out, imageSlot{
+ key: listPath + "\x00" + name,
+ image: image,
+ set: func(v string) { m["image"] = v },
+ })
}
return out
}
-// slotPlan is the per-container outcome of the inversion: the image the source
-// file should hold, the live image it must render to, and the entry edits that
-// make the render true.
+// volumeImageSlots reads volumes[].image.reference — a nested field, and the one the old
+// collector missed. A volume with no image (a configMap or emptyDir) simply has no slot.
+func volumeImageSlots(listPath string, list []interface{}) []imageSlot {
+ var out []imageSlot
+ for _, item := range list {
+ m, isMap := item.(map[string]interface{})
+ if !isMap {
+ continue
+ }
+ name, _ := m["name"].(string)
+ img, hasImage := m["image"].(map[string]interface{})
+ if name == "" || !hasImage {
+ continue
+ }
+ reference, hasReference := img["reference"].(string)
+ if !hasReference {
+ continue
+ }
+ out = append(out, imageSlot{
+ key: listPath + "\x00" + name,
+ image: reference,
+ set: func(v string) { img["reference"] = v },
+ })
+ }
+ return out
+}
+
+// slotPlan is the per-slot outcome of the inversion: the image the source file should
+// hold, and the entry edits that make the render come out as live.
type slotPlan struct {
- slot containerSlot
+ slot imageSlot
fileImage string
- live imageRef
edits []OverrideEdit
}
-// projectImages inverts the image chain for every container the live object and
-// the Git document share. It mutates out's container images to their
-// source-file form and returns the entry edits — or routes nothing (leaving the
-// live values in out, today's write-through) when the inversion is unsafe:
-// conflicting edits to one entry field, a removal an entry supplies, or a
-// simulation that fails to reproduce live.
+// projectImages inverts the image transformers for every slot the live object and the Git
+// document share. It rewrites out's images to their SOURCE-FILE form and returns the entry
+// edits — or routes nothing when the inversion is unsafe.
+//
+// Nothing here re-derives what kustomize does any more. The rendered value comes from the
+// renderer and the supplier comes from the dye, so the two questions that used to be answered
+// by a hand-written transformer — "what does this folder render to" and "who supplied it" —
+// are now both answered by kustomize.
+//
+// And nothing here is trusted. The proposal is put to kustomize before it can become a commit
+// (VerifyBatchRenders), so this only has to be a candidate that is usually right. Routing
+// nothing is always a legal answer: the proposal then falls back to whatever the source
+// document alone can carry, and the re-render adjudicates it.
func projectImages(
gitRaw map[string]interface{},
out *unstructured.Unstructured,
- entries []ImageOverride,
+ rendered map[string]RenderedImage,
) []OverrideEdit {
- if len(entries) == 0 {
+ if len(rendered) == 0 {
return nil
}
gitImages := map[string]string{}
- for _, s := range collectContainerSlots(gitRaw) {
+ for _, s := range collectImageSlots(gitRaw) {
gitImages[s.key] = s.image
}
var plans []slotPlan
- for _, slot := range collectContainerSlots(out.Object) {
- src, exists := gitImages[slot.key]
- if !exists {
+ for _, slot := range collectImageSlots(out.Object) {
+ src, inGit := gitImages[slot.key]
+ render, isRendered := rendered[slot.key]
+ if !inGit || !isRendered {
continue // a new container writes through; the supplier rule converges it later
}
- plan, routable := invertImage(slot, src, entries)
+ plan, routable := invertImage(slot, src, render)
if !routable {
- return nil // one unroutable container abandons routing for the object
+ return nil // one unroutable slot abandons routing for the whole object
}
plans = append(plans, plan)
}
edits, ok := collectConsistentEdits(plans)
- if !ok || !simulateImageRender(plans, entries, edits) {
+ if !ok {
return nil
}
for _, p := range plans {
- p.slot.item["image"] = p.fileImage
+ p.slot.set(p.fileImage)
}
return edits
}
-// invertImage computes one container's source-file image and entry edits.
-// routable is false when a live change cannot be expressed on the existing
-// entries (a component removal whose supplier is an entry).
-func invertImage(slot containerSlot, src string, entries []ImageOverride) (slotPlan, bool) {
+// invertImage computes one slot's source-file image and entry edits, given what kustomize
+// renders it to and which entry supplied each component.
+//
+// routable is false when a live change cannot be expressed on the entries that exist, which
+// abandons routing for the whole object.
+func invertImage(slot imageSlot, src string, render RenderedImage) (slotPlan, bool) {
srcRef := parseImageRef(src)
- rendered, sup := renderImage(srcRef, entries)
+ rendered := parseImageRef(render.Rendered)
live := parseImageRef(slot.image)
- plan := slotPlan{slot: slot, live: live}
+
+ plan := slotPlan{slot: slot, fileImage: src}
if rendered == live {
- plan.fileImage = src
- return plan, true
+ return plan, true // the folder already renders to live; the file keeps its bytes
}
newSrc := srcRef
@@ -287,24 +298,26 @@ func invertImage(slot containerSlot, src string, entries []ImageOverride) (slotP
},
})
}
+
if live.name != rendered.name {
- if sup.name != nil {
- route(sup.name, "newName", live.name)
- } else {
+ switch {
+ case render.Name != nil:
+ route(render.Name, fieldNewName, live.name)
+ default:
newSrc.name = live.name
}
}
if live.tag != rendered.tag {
- if !routeComponent(sup.tag, declaresNewTag, live.tag,
+ if !routeComponent(render.Tag, render.Digest, live.tag,
func(v string) { newSrc.tag = v },
- func(e *ImageOverride, v string) { route(e, "newTag", v) }) {
+ func(e *ImageOverride, v string) { route(e, fieldNewTag, v) }) {
return plan, false
}
}
if live.digest != rendered.digest {
- if !routeComponent(sup.digest, declaresDigest, live.digest,
+ if !routeComponent(render.Digest, render.Tag, live.digest,
func(v string) { newSrc.digest = v },
- func(e *ImageOverride, v string) { route(e, "digest", v) }) {
+ func(e *ImageOverride, v string) { route(e, fieldDigest, v) }) {
return plan, false
}
}
@@ -312,40 +325,44 @@ func invertImage(slot containerSlot, src string, entries []ImageOverride) (slotP
return plan, true
}
-// declaresNewTag / declaresDigest report whether an entry actually carries the key
-// the writer would have to set. An entry can GOVERN a component without declaring
-// it: a digest entry clears the tag, and a newTag entry clears the digest.
-func declaresNewTag(e *ImageOverride) bool { return e.HasNewTag }
-func declaresDigest(e *ImageOverride) bool { return e.HasDigest }
-
-// routeComponent decides where one changed image component (tag or digest) goes:
-// into the source file when no entry supplies it, onto the supplying entry when
-// that entry declares the key, or nowhere at all. It reports false when the change
-// is unroutable, which abandons routing for the whole object (write-through).
+// routeComponent decides where one changed image component (tag or digest) goes: onto the
+// entry that supplies it, into the source file when no entry does, or nowhere at all.
+//
+// TAG AND DIGEST ARE MUTUALLY EXCLUSIVE IN KUSTOMIZE, and that is what `sibling` is for. From
+// its own image transformer (filters/imagetag/updater.go, SetImageValue):
+//
+// case NewTag != "" && Digest != "": tag = NewTag; digest = Digest
+// case NewTag != "": tag = NewTag; digest = "" // a tag entry CLEARS the digest
+// case Digest != "": tag = ""; digest = Digest // a digest entry CLEARS the tag
+//
+// So an entry can GOVERN a component it does not declare. When a digest entry has cleared the
+// tag, no dye lands in the tag — nothing supplies it — but writing a tag into the source file
+// would be wiped by the very next render. The dye cannot see that on its own; the sibling
+// component's supplier is what reveals it, and it is the bug (#231) that corrupted real source
+// files by rewriting a tag out of them.
//
-// The two unroutable cases are worth naming:
+// The two unroutable cases:
//
-// - a REMOVAL of a component an entry supplies — there is no way to say "no tag"
-// on an entry that sets one; and
-// - a change to a component an entry governs but does not declare — a digest entry
-// clears the tag, so setting a tag has no key to land in, and writing it into the
-// source file would be undone by the very next render.
+// - a REMOVAL of a component an entry supplies — there is no way to say "no tag" on an
+// entry that sets one;
+// - a change to a component the SIBLING entry clears — nowhere to land, and the file would
+// be overridden straight back.
func routeComponent(
- sup *ImageOverride,
- declares func(*ImageOverride) bool,
+ supplier *ImageOverride,
+ sibling *ImageOverride,
live string,
setSource func(string),
route func(*ImageOverride, string),
) bool {
switch {
- case sup == nil:
- setSource(live) // the source file supplies it; the change flows into the file
- case live == "":
- return false
- case !declares(sup):
- return false
+ case supplier != nil && live == "":
+ return false // cannot express "no tag" on an entry that sets one
+ case supplier != nil:
+ route(supplier, live)
+ case sibling != nil:
+ return false // the sibling component's entry clears this one; the file cannot own it
default:
- route(sup, live)
+ setSource(live) // the source document supplies it; the change flows into the file
}
return true
}
@@ -390,62 +407,25 @@ func collectConsistentEdits(plans []slotPlan) ([]OverrideEdit, bool) {
return out, true
}
-// simulateImageRender verifies the whole inversion: with the edits applied to a
-// copy of the entries, every planned source image must render exactly to its
-// live image. Chained entries can interact (a newName edit can change which
-// later entry matches), so this closes the loop the per-component logic cannot.
-func simulateImageRender(plans []slotPlan, entries []ImageOverride, edits []OverrideEdit) bool {
- sim := make([]ImageOverride, len(entries))
- copy(sim, entries)
- for _, oe := range edits {
- for i := range sim {
- if sim[i].Source != oe.KustomizationPath || sim[i].Index != oe.Edit.EntryIndex {
- continue
- }
- switch oe.Edit.Field {
- case "newName":
- sim[i].NewName = oe.Edit.Value
- case "newTag":
- sim[i].NewTag = oe.Edit.Value
- case "digest":
- sim[i].Digest = oe.Edit.Value
- }
- }
- }
- for _, p := range plans {
- rendered, _ := renderImage(parseImageRef(p.fileImage), sim)
- if rendered != p.live {
- return false
- }
- }
- return true
-}
-
-// replicaKinds are the kinds the builtin replica transformer touches.
-func isReplicaKind(kind string) bool {
- switch kind {
- case "Deployment", "ReplicaSet", "StatefulSet":
- return true
- default:
- return false
- }
-}
-
-// projectReplicas inverts the replicas transformer: when an entry pins this
-// document's replica count, the source form of spec.replicas is restored on out
-// (including its absence — the transformer creates the field) and a count edit
-// is emitted only when live diverges from the pinned count.
+// projectReplicas inverts the replica transformer: when an entry pins this document's replica
+// count, the source form of spec.replicas is restored on out (including its ABSENCE — the
+// transformer creates the field) and a count edit is emitted only when live diverges from the
+// pinned count.
+//
+// There is no list of kinds here any more, and that is a bug fix rather than a tidy-up. We
+// used to gate this on isReplicaKind — Deployment, ReplicaSet, StatefulSet — while kustomize's
+// fieldspec is Deployment, ReplicaSet, StatefulSet AND ReplicationController. A scale on an RC
+// governed by a replicas: entry was written into the source document, where the transformer
+// overrode it right back: non-converging drift, silently, forever. The dye ends the argument:
+// if a dyed count came out of this object, an entry governs the field, whatever the kind is.
+// kustomize's fieldspec is the authority, and we no longer keep a second opinion about it.
func projectReplicas(
gitRaw map[string]interface{},
out *unstructured.Unstructured,
- entries []ReplicaOverride,
+ rendered *RenderedReplicas,
) []OverrideEdit {
- if len(entries) == 0 || !isReplicaKind(out.GetKind()) {
- return nil
- }
- sup := replicaSupplier(entries, out.GetName())
- if sup == nil {
- return nil
+ if rendered == nil || rendered.Entry == nil {
+ return nil // no entry supplies the count; a scale flows into the source document
}
liveCount, liveHas, err := unstructured.NestedInt64(out.Object, "spec", "replicas")
if err != nil || !liveHas {
@@ -453,55 +433,34 @@ func projectReplicas(
}
restoreSourceReplicas(gitRaw, out)
- if liveCount == sup.Count {
- return nil
- }
- return []OverrideEdit{{
- KustomizationPath: sup.Source,
- Edit: manifestedit.KustomizationEdit{
- Section: manifestedit.KustomizationSectionReplicas,
- EntryIndex: sup.Index,
- EntryName: sup.Name,
- Field: "count",
- Value: strconv.FormatInt(liveCount, 10),
- },
- }}
-}
-
-// replicaSupplier is the last entry in the chain matching the document's name —
-// the one whose count the render ends up with.
-func replicaSupplier(entries []ReplicaOverride, name string) *ReplicaOverride {
- var sup *ReplicaOverride
- for i := range entries {
- if entries[i].Name == name {
- sup = &entries[i]
- }
+ if liveCount == rendered.Rendered {
+ return nil // the folder already renders to live
}
- return sup
+ return []OverrideEdit{replicaCountEdit(rendered.Entry, liveCount)}
}
-// ReplicaCountEdit returns the entry edit that absorbs a live replica count for
-// the document, when its override chain governs spec.replicas. The writer's
-// field-patch path (the /scale subresource) uses it to route a scale to the
-// kustomization entry instead of writing the count into the source manifest.
+// ReplicaCountEdit returns the entry edit that absorbs a live replica count for the document,
+// when a replicas: entry supplies spec.replicas. The writer's field-patch path (the /scale
+// subresource) uses it to route a scale onto the entry instead of writing the count into the
+// source manifest, where the transformer would override it back.
func ReplicaCountEdit(dm *DocumentModel, count int64) (OverrideEdit, bool) {
- if dm == nil || dm.Overrides == nil || !isReplicaKind(dm.ManifestIdentity.Kind) {
- return OverrideEdit{}, false
- }
- sup := replicaSupplier(dm.Overrides.Replicas, dm.ManifestIdentity.Name)
- if sup == nil {
+ if dm == nil || dm.Rendered == nil || dm.Rendered.Replicas == nil || dm.Rendered.Replicas.Entry == nil {
return OverrideEdit{}, false
}
+ return replicaCountEdit(dm.Rendered.Replicas.Entry, count), true
+}
+
+func replicaCountEdit(entry *ReplicaOverride, count int64) OverrideEdit {
return OverrideEdit{
- KustomizationPath: sup.Source,
+ KustomizationPath: entry.Source,
Edit: manifestedit.KustomizationEdit{
Section: manifestedit.KustomizationSectionReplicas,
- EntryIndex: sup.Index,
- EntryName: sup.Name,
- Field: "count",
+ EntryIndex: entry.Index,
+ EntryName: entry.Name,
+ Field: fieldCount,
Value: strconv.FormatInt(count, 10),
},
- }, true
+ }
}
// sourceReplicaCount reads spec.replicas off a source KRM document. This is a
diff --git a/internal/manifestanalyzer/overrides_projection_test.go b/internal/manifestanalyzer/overrides_projection_test.go
index 68983bd3..efcbddc5 100644
--- a/internal/manifestanalyzer/overrides_projection_test.go
+++ b/internal/manifestanalyzer/overrides_projection_test.go
@@ -3,14 +3,91 @@
package manifestanalyzer
import (
+ "fmt"
"testing"
+ "github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+ "sigs.k8s.io/yaml"
"github.com/ConfigButler/gitops-reverser/internal/git/manifestedit"
)
-func deploymentObj(image string, replicas *int64) map[string]interface{} {
+// These tests drive the projection with GROUND TRUTH.
+//
+// They used to construct (gitRaw, desired, overrides) by hand, which meant they asserted what
+// we BELIEVED a folder renders to. That is exactly the belief that was wrong — twice, in
+// shipped code — so a test net woven out of it could not have caught either bug, and did not.
+//
+// Now every case builds a small real repository, renders it with kustomize, reads the
+// attribution off a dyed counterfactual render, and drives the projection with the result. A
+// row that disagrees with kustomize now fails here instead of silently corrupting a source
+// file, and if kustomize ever changes its semantics these fail rather than the operator.
+
+// splitFixture is the whole harness: build the tree, ask kustomize what it renders and which
+// entry supplied what, then invert the live object against that answer.
+func splitFixture(
+ t *testing.T,
+ files []manifestedit.FileContent,
+ sourcePath string,
+ live map[string]interface{},
+) (*unstructured.Unstructured, []OverrideEdit) {
+ t.Helper()
+ desired := &unstructured.Unstructured{Object: live}
+
+ assignments, failed := renderChains(files, parseKustomizations(files))
+ require.Empty(t, failed, "the fixture must be a folder kustomize can build")
+
+ assignment := assignments[chainKey{
+ originPath: sourcePath,
+ kind: desired.GetKind(),
+ name: desired.GetName(),
+ }]
+ require.NotNil(t, assignment, "kustomize must render %s/%s from %s",
+ desired.GetKind(), desired.GetName(), sourcePath)
+
+ var gitRaw map[string]interface{}
+ require.NoError(t, yaml.Unmarshal(contentOf(t, files, sourcePath), &gitRaw))
+
+ return SplitDesiredForOverrides(gitRaw, desired, assignment.rendered)
+}
+
+func contentOf(t *testing.T, files []manifestedit.FileContent, path string) []byte {
+ t.Helper()
+ for _, f := range files {
+ if f.Path == path {
+ return f.Content
+ }
+ }
+ t.Fatalf("fixture has no file %q", path)
+ return nil
+}
+
+func file(path, content string) manifestedit.FileContent {
+ return manifestedit.FileContent{Path: path, Content: []byte(content)}
+}
+
+// deploymentSource is the Deployment as GIT holds it.
+func deploymentSource(image string, replicas string) string {
+ if replicas != "" {
+ replicas = " replicas: " + replicas + "\n"
+ }
+ return fmt.Sprintf(`apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: web
+spec:
+%s template:
+ spec:
+ containers:
+ - name: app
+ image: %s
+`, replicas, image)
+}
+
+// liveDeployment is the Deployment as the CLUSTER holds it — the rendered form, which is what
+// the source file plus the entries have to reproduce.
+func liveDeployment(image string, replicas *int64) map[string]interface{} {
spec := map[string]interface{}{
"template": map[string]interface{}{
"spec": map[string]interface{}{
@@ -31,314 +108,476 @@ func deploymentObj(image string, replicas *int64) map[string]interface{} {
}
}
-func desiredOf(obj map[string]interface{}) *unstructured.Unstructured {
- u := &unstructured.Unstructured{Object: obj}
- // Live objects carry int64 replicas; normalize the fixture.
- if v, ok, _ := unstructured.NestedFieldNoCopy(obj, "spec", "replicas"); ok {
- if n, isInt := v.(int); isInt {
- _ = unstructured.SetNestedField(obj, int64(n), "spec", "replicas")
- }
- }
- return u
+func kustomizationWith(body string) string {
+ return "resources:\n - deployment.yaml\n" + body
}
-func desiredImage(t *testing.T, u *unstructured.Unstructured) string {
+func imageOf(t *testing.T, u *unstructured.Unstructured) string {
t.Helper()
- slots := collectContainerSlots(u.Object)
- if len(slots) != 1 {
- t.Fatalf("want exactly one container slot, got %d", len(slots))
- }
+ slots := collectImageSlots(u.Object)
+ require.Len(t, slots, 1, "want exactly one image slot")
return slots[0].image
}
-func imgEntry(name string, set map[string]string) ImageOverride {
- e := ImageOverride{Source: "kustomization.yaml", Name: name}
- if v, ok := set["newName"]; ok {
- e.NewName, e.HasNewName = v, true
- }
- if v, ok := set["newTag"]; ok {
- e.NewTag, e.HasNewTag = v, true
- }
- if v, ok := set["digest"]; ok {
- e.Digest, e.HasDigest = v, true
+func imagesEntry(body string) []manifestedit.FileContent {
+ return []manifestedit.FileContent{
+ file("deployment.yaml", deploymentSource("ghcr.io/example/app:1.0.0", "")),
+ file("kustomization.yaml", kustomizationWith(body)),
}
- return e
}
-// TestSplitDesired_TagRoutedToEntry pins the core edit-through behavior: a live tag change
-// produced by a newTag entry lands on the entry and the file keeps its bytes.
+// The core of the edit-through: a live tag change produced by a newTag entry lands on the
+// entry, and the source file keeps its bytes.
func TestSplitDesired_TagRoutedToEntry(t *testing.T) {
- git := deploymentObj("ghcr.io/example/app:1.0.0", nil)
- desired := desiredOf(deploymentObj("ghcr.io/example/app:2.0.0", nil))
- ov := &KustomizeOverrides{Images: []ImageOverride{
- imgEntry("ghcr.io/example/app", map[string]string{"newTag": "1.5.0"}),
- }}
-
- out, edits := SplitDesiredForOverrides(git, desired, ov)
- if got := desiredImage(t, out); got != "ghcr.io/example/app:1.0.0" {
- t.Errorf("file image = %q, want the source form restored", got)
- }
- if len(edits) != 1 {
- t.Fatalf("want one entry edit, got %+v", edits)
- }
- e := edits[0]
- if e.KustomizationPath != "kustomization.yaml" ||
- e.Edit != (manifestedit.KustomizationEdit{
+ files := imagesEntry("images:\n - name: ghcr.io/example/app\n newTag: \"1.5.0\"\n")
+
+ out, edits := splitFixture(t, files, "deployment.yaml",
+ liveDeployment("ghcr.io/example/app:2.0.0", nil))
+
+ require.Equal(t, "ghcr.io/example/app:1.0.0", imageOf(t, out), "the source form must be restored")
+ require.Len(t, edits, 1)
+ require.Equal(t, OverrideEdit{
+ KustomizationPath: "kustomization.yaml",
+ Edit: manifestedit.KustomizationEdit{
Section: manifestedit.KustomizationSectionImages, EntryIndex: 0,
EntryName: "ghcr.io/example/app", Field: "newTag", Value: "2.0.0",
- }) {
- t.Errorf("unexpected edit %+v", e)
- }
+ },
+ }, edits[0])
}
-// TestSplitDesired_LiveMatchesRenderIsNoOp pins the write-through fix: live equal
-// to the rendered value must restore the source form and route nothing, so the
-// source file's "stale" tag is never overwritten.
-func TestSplitDesired_LiveMatchesRenderIsNoOp(t *testing.T) {
- git := deploymentObj("ghcr.io/example/app:1.0.0", nil)
- desired := desiredOf(deploymentObj("ghcr.io/example/app:1.5.0", nil))
- ov := &KustomizeOverrides{Images: []ImageOverride{
- imgEntry("ghcr.io/example/app", map[string]string{"newTag": "1.5.0"}),
- }}
-
- out, edits := SplitDesiredForOverrides(git, desired, ov)
- if got := desiredImage(t, out); got != "ghcr.io/example/app:1.0.0" {
- t.Errorf("file image = %q, want source form", got)
+// THE IDEMPOTENT PIN — the case that killed leave-one-out probing, and the reason the dye
+// exists.
+//
+// The source is already at v1 and the overlay pins v1: the state every repository is in the
+// moment a release lands in both places. Removing the entry moves NOTHING, so a probe that
+// asks "what changed?" concludes the source file supplies the tag — and writes the user's next
+// tag into the base, where the overlay overrides it straight back, on every reconcile, forever.
+//
+// The dye is not fooled: a nonce nothing else can produce comes out of that field, so the
+// entry is the supplier even though its value is identical to the source's.
+func TestSplitDesired_IdempotentPinIsStillAttributedToTheEntry(t *testing.T) {
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", deploymentSource("app:v1", "")),
+ file("kustomization.yaml", kustomizationWith("images:\n - name: app\n newTag: v1\n")),
}
- if len(edits) != 0 {
- t.Errorf("want no edits, got %+v", edits)
+
+ out, edits := splitFixture(t, files, "deployment.yaml", liveDeployment("app:v2", nil))
+
+ require.Equal(t, "app:v1", imageOf(t, out), "the base must NOT absorb the tag; the entry owns it")
+ require.Len(t, edits, 1, "the tag must land on the entry, or it never converges")
+ require.Equal(t, "newTag", edits[0].Edit.Field)
+ require.Equal(t, "v2", edits[0].Edit.Value)
+}
+
+// A TIE: two entries pinning the same tag. Removal cannot attribute either of them — drop
+// one and the other still produces the same bytes. The dye reads the LAST writer straight off
+// the output, because the two nonces are distinguishable even though the two values were not.
+func TestSplitDesired_TieIsAttributedToTheLastWriter(t *testing.T) {
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", deploymentSource("app:v1", "")),
+ file("kustomization.yaml", kustomizationWith(
+ "images:\n - name: app\n newTag: v9\n - name: app\n newTag: v9\n")),
}
+
+ out, edits := splitFixture(t, files, "deployment.yaml", liveDeployment("app:v10", nil))
+
+ require.Equal(t, "app:v1", imageOf(t, out))
+ require.Len(t, edits, 1)
+ require.Equal(t, 1, edits[0].Edit.EntryIndex, "the LAST matching entry is the one that renders")
+ require.Equal(t, "v10", edits[0].Edit.Value)
}
-// TestSplitDesired_UngovernedComponentPatchesFile: the matching entry declares
-// only newName, so a tag change is file-supplied and flows into the source image
-// while the name stays in its source form.
+// Live equal to the rendered value is a full no-op: the source file's "stale" tag is dead text
+// the entry shadows, and must not be overwritten.
+func TestSplitDesired_LiveMatchesRenderIsNoOp(t *testing.T) {
+ files := imagesEntry("images:\n - name: ghcr.io/example/app\n newTag: \"1.5.0\"\n")
+
+ out, edits := splitFixture(t, files, "deployment.yaml",
+ liveDeployment("ghcr.io/example/app:1.5.0", nil))
+
+ require.Equal(t, "ghcr.io/example/app:1.0.0", imageOf(t, out), "the source keeps its bytes")
+ require.Empty(t, edits)
+}
+
+// The matching entry declares only newName, so the TAG is file-supplied: it flows into the
+// source image while the name stays in its source form.
func TestSplitDesired_UngovernedComponentPatchesFile(t *testing.T) {
- git := deploymentObj("old/app:1.0.0", nil)
- desired := desiredOf(deploymentObj("new/app:2.0.0", nil))
- ov := &KustomizeOverrides{Images: []ImageOverride{
- imgEntry("old/app", map[string]string{"newName": "new/app"}),
- }}
-
- out, edits := SplitDesiredForOverrides(git, desired, ov)
- if got := desiredImage(t, out); got != "old/app:2.0.0" {
- t.Errorf("file image = %q, want source name with the live tag", got)
- }
- if len(edits) != 0 {
- t.Errorf("tag is file-supplied here; want no edits, got %+v", edits)
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", deploymentSource("old/app:1.0.0", "")),
+ file("kustomization.yaml", kustomizationWith(
+ "images:\n - name: old/app\n newName: new/app\n")),
}
+
+ out, edits := splitFixture(t, files, "deployment.yaml", liveDeployment("new/app:2.0.0", nil))
+
+ require.Equal(t, "old/app:2.0.0", imageOf(t, out), "the source name stays; the live tag lands in the file")
+ require.Empty(t, edits, "no entry supplies the tag")
}
-// TestSplitDesired_NameChangeRoutedToNewName: a live name change whose supplier
-// is a newName entry updates the entry.
+// A live name change whose supplier is a newName entry updates the entry.
func TestSplitDesired_NameChangeRoutedToNewName(t *testing.T) {
- git := deploymentObj("old/app:1.0.0", nil)
- desired := desiredOf(deploymentObj("mirror/app:1.0.0", nil))
- ov := &KustomizeOverrides{Images: []ImageOverride{
- imgEntry("old/app", map[string]string{"newName": "new/app"}),
- }}
-
- out, edits := SplitDesiredForOverrides(git, desired, ov)
- if got := desiredImage(t, out); got != "old/app:1.0.0" {
- t.Errorf("file image = %q, want untouched source", got)
- }
- if len(edits) != 1 || edits[0].Edit.Field != "newName" || edits[0].Edit.Value != "mirror/app" {
- t.Fatalf("want one newName edit to mirror/app, got %+v", edits)
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", deploymentSource("old/app:1.0.0", "")),
+ file("kustomization.yaml", kustomizationWith(
+ "images:\n - name: old/app\n newName: new/app\n")),
}
+
+ out, edits := splitFixture(t, files, "deployment.yaml", liveDeployment("mirror/app:1.0.0", nil))
+
+ require.Equal(t, "old/app:1.0.0", imageOf(t, out), "the source is untouched")
+ require.Len(t, edits, 1)
+ require.Equal(t, "newName", edits[0].Edit.Field)
+ require.Equal(t, "mirror/app", edits[0].Edit.Value)
}
-// TestSplitDesired_DigestRoutedToEntry: a live digest change whose supplier is a
-// digest entry updates the entry; tag and name keep their source form.
-//
-// Note the live image carries NO tag. A digest entry replaces the tag as well as
-// the digest (kustomize: "overriding tag or digest will replace both original tag
-// and digest values"), so `app@sha256:...` is the only thing this folder can
-// render. This test used to assert `app:1.0.0@sha256:ccc` — a live state kustomize
-// cannot produce — because renderImage set the two components independently.
+// A digest entry replaces the TAG as well as the digest — kustomize: "overriding tag or digest
+// will replace both original tag and digest values" — so `app@sha256:...` is the only thing
+// this folder can render, and the live object carries no tag.
func TestSplitDesired_DigestRoutedToEntry(t *testing.T) {
- git := deploymentObj("app:1.0.0", nil)
- desired := desiredOf(deploymentObj("app@sha256:ccc", nil))
- ov := &KustomizeOverrides{Images: []ImageOverride{
- imgEntry("app", map[string]string{"digest": "sha256:bbb"}),
- }}
-
- out, edits := SplitDesiredForOverrides(git, desired, ov)
- if got := desiredImage(t, out); got != "app:1.0.0" {
- t.Errorf("file image = %q, want untouched source (no digest)", got)
- }
- if len(edits) != 1 ||
- edits[0].Edit != (manifestedit.KustomizationEdit{
- Section: manifestedit.KustomizationSectionImages, EntryIndex: 0,
- EntryName: "app", Field: "digest", Value: "sha256:ccc",
- }) {
- t.Fatalf("want one digest edit to sha256:ccc, got %+v", edits)
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", deploymentSource("app:1.0.0", "")),
+ file("kustomization.yaml", kustomizationWith(
+ "images:\n - name: app\n digest: sha256:bbb\n")),
}
+
+ out, edits := splitFixture(t, files, "deployment.yaml", liveDeployment("app@sha256:ccc", nil))
+
+ require.Equal(t, "app:1.0.0", imageOf(t, out), "the source keeps its tag")
+ require.Len(t, edits, 1)
+ require.Equal(t, manifestedit.KustomizationEdit{
+ Section: manifestedit.KustomizationSectionImages, EntryIndex: 0,
+ EntryName: "app", Field: "digest", Value: "sha256:ccc",
+ }, edits[0].Edit)
}
-// TestSplitDesired_DigestEntryDoesNotStripTheSourceTag pins the corruption that a
-// hand-written image transformer caused, so it cannot come back.
+// The #231 corruption, pinned so it cannot come back.
//
-// Source `app:1.0.0`, an entry supplying only a digest. kustomize renders
-// `app@sha256:bbb` — the digest REPLACES the tag. We used to believe the render was
-// `app:1.0.0@sha256:bbb`, so on seeing the real live object (no tag) the projection
-// concluded the user had removed the tag and rewrote `app:1.0.0` to `app` in the
-// source file. On every reconcile, silently.
-//
-// The source document must come back untouched.
+// Source `app:1.0.0`, an entry supplying only a digest. kustomize renders `app@sha256:bbb` —
+// the digest REPLACES the tag. The hand-written transformer believed the render was
+// `app:1.0.0@sha256:bbb`, so on seeing the real live object (no tag) it concluded the user had
+// removed the tag, and rewrote `app:1.0.0` to `app` in the source file. On every reconcile,
+// silently. Nothing about the source document may change here.
func TestSplitDesired_DigestEntryDoesNotStripTheSourceTag(t *testing.T) {
- git := deploymentObj("app:1.0.0", nil)
- // What the folder actually renders to, mirrored back unchanged by the user.
- desired := desiredOf(deploymentObj("app@sha256:bbb", nil))
- ov := &KustomizeOverrides{Images: []ImageOverride{
- imgEntry("app", map[string]string{"digest": "sha256:bbb"}),
- }}
-
- out, edits := SplitDesiredForOverrides(git, desired, ov)
- if got := desiredImage(t, out); got != "app:1.0.0" {
- t.Errorf("source image = %q, want it untouched at app:1.0.0 — the tag must not be stripped", got)
- }
- if len(edits) != 0 {
- t.Errorf("live matches the render, so nothing should be routed; got %+v", edits)
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", deploymentSource("app:1.0.0", "")),
+ file("kustomization.yaml", kustomizationWith(
+ "images:\n - name: app\n digest: sha256:bbb\n")),
}
+
+ out, edits := splitFixture(t, files, "deployment.yaml", liveDeployment("app@sha256:bbb", nil))
+
+ require.Equal(t, "app:1.0.0", imageOf(t, out), "the tag must NOT be stripped out of the source")
+ require.Empty(t, edits, "live already matches the render")
}
-// TestSplitDesired_RemovalUnroutable: live drops the digest an entry supplies;
-// nothing can express that on the entry, so the whole object writes through.
-func TestSplitDesired_RemovalUnroutable(t *testing.T) {
- git := deploymentObj("app:1.0.0", nil)
- desired := desiredOf(deploymentObj("app:1.0.0", nil))
- ov := &KustomizeOverrides{Images: []ImageOverride{
- imgEntry("app", map[string]string{"digest": "sha256:abc"}),
- }}
-
- out, edits := SplitDesiredForOverrides(git, desired, ov)
- if got := desiredImage(t, out); got != "app:1.0.0" {
- t.Errorf("file image = %q, want the live value written through", got)
- }
- if len(edits) != 0 {
- t.Errorf("want no edits on write-through, got %+v", edits)
+// Live drops the digest an entry supplies. There is no way to say "no digest" on an entry that
+// sets one, so nothing routes and the object writes through — where the oracle then adjudicates
+// it, because a write-through of a governed field does not converge.
+func TestSplitDesired_RemovalIsUnroutable(t *testing.T) {
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", deploymentSource("app:1.0.0", "")),
+ file("kustomization.yaml", kustomizationWith(
+ "images:\n - name: app\n digest: sha256:abc\n")),
}
+
+ out, edits := splitFixture(t, files, "deployment.yaml", liveDeployment("app:1.0.0", nil))
+
+ require.Equal(t, "app:1.0.0", imageOf(t, out), "the live value is written through")
+ require.Empty(t, edits)
}
-// TestSplitDesired_ConflictingContainersAbandonRouting: two containers governed
-// by one entry cannot pin two different tags on it.
+// Two containers governed by one entry cannot pin two different tags on it.
func TestSplitDesired_ConflictingContainersAbandonRouting(t *testing.T) {
- containers := func(tagA, tagB string) map[string]interface{} {
- return map[string]interface{}{
- "apiVersion": "apps/v1",
- "kind": "Deployment",
- "metadata": map[string]interface{}{"name": "web"},
- "spec": map[string]interface{}{
- "template": map[string]interface{}{
- "spec": map[string]interface{}{
- "containers": []interface{}{
- map[string]interface{}{"name": "a", "image": "app:" + tagA},
- map[string]interface{}{"name": "b", "image": "app:" + tagB},
- },
+ source := `apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: web
+spec:
+ template:
+ spec:
+ containers:
+ - name: a
+ image: app:1.0.0
+ - name: b
+ image: app:1.0.0
+`
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", source),
+ file("kustomization.yaml", kustomizationWith("images:\n - name: app\n newTag: \"1.5.0\"\n")),
+ }
+ live := map[string]interface{}{
+ "apiVersion": "apps/v1",
+ "kind": "Deployment",
+ "metadata": map[string]interface{}{"name": "web"},
+ "spec": map[string]interface{}{
+ "template": map[string]interface{}{
+ "spec": map[string]interface{}{
+ "containers": []interface{}{
+ map[string]interface{}{"name": "a", "image": "app:2.0.0"},
+ map[string]interface{}{"name": "b", "image": "app:3.0.0"},
},
},
},
- }
- }
- git := containers("1.0.0", "1.0.0")
- desired := desiredOf(containers("2.0.0", "3.0.0"))
- ov := &KustomizeOverrides{Images: []ImageOverride{
- imgEntry("app", map[string]string{"newTag": "1.5.0"}),
- }}
-
- out, edits := SplitDesiredForOverrides(git, desired, ov)
- if len(edits) != 0 {
- t.Fatalf("conflicting demands must abandon routing, got %+v", edits)
- }
- slots := collectContainerSlots(out.Object)
- if slots[0].image != "app:2.0.0" || slots[1].image != "app:3.0.0" {
- t.Errorf("write-through must keep the live values, got %q / %q", slots[0].image, slots[1].image)
+ },
}
+
+ out, edits := splitFixture(t, files, "deployment.yaml", live)
+
+ require.Empty(t, edits, "one entry cannot carry two different tags")
+ slots := collectImageSlots(out.Object)
+ require.Len(t, slots, 2)
+ require.Equal(t, "app:2.0.0", slots[0].image, "write-through keeps the live values")
+ require.Equal(t, "app:3.0.0", slots[1].image)
}
-// TestSplitDesired_ChainedEntriesCompose: a base renames the image, the parent
-// pins the tag of the renamed image; a live tag change lands on the parent entry.
+// A base renames the image and the overlay pins the tag OF THE RENAMED IMAGE. A live tag change
+// lands on the overlay's entry.
+//
+// This is also the rename-chain guard doing its job: the overlay's name: matches the base's
+// newName:, so dyeing newName would stop the overlay's entry matching and change the render's
+// shape. Names therefore go undyed here — and the TAG is still attributed exactly, because a
+// tag dye is a pure sink even inside a rename chain.
func TestSplitDesired_ChainedEntriesCompose(t *testing.T) {
- git := deploymentObj("app:1.0.0", nil)
- desired := desiredOf(deploymentObj("mirror/app:9.9.9", nil))
- base := imgEntry("app", map[string]string{"newName": "mirror/app"})
- base.Source = "base/kustomization.yaml"
- parent := imgEntry("mirror/app", map[string]string{"newTag": "2.0.0"})
- ov := &KustomizeOverrides{Images: []ImageOverride{base, parent}}
-
- out, edits := SplitDesiredForOverrides(git, desired, ov)
- if got := desiredImage(t, out); got != "app:1.0.0" {
- t.Errorf("file image = %q, want untouched source", got)
- }
- if len(edits) != 1 || edits[0].KustomizationPath != "kustomization.yaml" ||
- edits[0].Edit.Field != "newTag" || edits[0].Edit.Value != "9.9.9" {
- t.Fatalf("want one newTag edit on the parent entry, got %+v", edits)
+ files := []manifestedit.FileContent{
+ file("base/deployment.yaml", deploymentSource("app:1.0.0", "")),
+ file("base/kustomization.yaml",
+ "resources:\n - deployment.yaml\nimages:\n - name: app\n newName: mirror/app\n"),
+ file("kustomization.yaml",
+ "resources:\n - base\nimages:\n - name: mirror/app\n newTag: \"2.0.0\"\n"),
+ }
+
+ out, edits := splitFixture(t, files, "base/deployment.yaml",
+ liveDeployment("mirror/app:9.9.9", nil))
+
+ require.Equal(t, "app:1.0.0", imageOf(t, out), "the source is untouched")
+ require.Len(t, edits, 1)
+ require.Equal(t, "kustomization.yaml", edits[0].KustomizationPath, "the OVERLAY's entry owns the tag")
+ require.Equal(t, "newTag", edits[0].Edit.Field)
+ require.Equal(t, "9.9.9", edits[0].Edit.Value)
+}
+
+// B1: an images: entry's name: is a REGULAR EXPRESSION, and kustomize matches on it as one.
+// Our matcher was string equality, so we believed `- name: "ap."` matched nothing while
+// kustomize rewrote the image — and the projection then read the difference as a user edit and
+// wrote the rendered value into the source manifest, killing the entry. The dye cannot make
+// that mistake: it does not match anything, it reads where kustomize's own nonce came out.
+func TestSplitDesired_RegexEntryNameIsAttributed(t *testing.T) {
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", deploymentSource("app:1.0.0", "")),
+ file("kustomization.yaml", kustomizationWith("images:\n - name: \"ap.\"\n newTag: \"1.5.0\"\n")),
}
+
+ out, edits := splitFixture(t, files, "deployment.yaml", liveDeployment("app:2.0.0", nil))
+
+ require.Equal(t, "app:1.0.0", imageOf(t, out), "the source keeps its bytes; the regex entry owns the tag")
+ require.Len(t, edits, 1, "kustomize matched this entry, so we must attribute to it")
+ require.Equal(t, "newTag", edits[0].Edit.Field)
+ require.Equal(t, "2.0.0", edits[0].Edit.Value)
}
-// TestSplitDesired_ReplicasRoutedToEntry: a pinned count absorbs the live scale;
-// the file's replicas field is restored to its source form (absent here).
+// A pinned count absorbs the live scale; the file's replicas field is restored to its source
+// form — here, ABSENT, because the transformer creates the field.
func TestSplitDesired_ReplicasRoutedToEntry(t *testing.T) {
five := int64(5)
- git := deploymentObj("app:1.0.0", nil) // source has no spec.replicas
- desired := desiredOf(deploymentObj("app:1.0.0", &five))
- ov := &KustomizeOverrides{Replicas: []ReplicaOverride{
- {Source: "kustomization.yaml", Index: 0, Name: "web", Count: 3},
- }}
-
- out, edits := SplitDesiredForOverrides(git, desired, ov)
- if _, has, _ := unstructured.NestedInt64(out.Object, "spec", "replicas"); has {
- t.Errorf("source has no replicas field, so the desired-for-file must not either")
- }
- if len(edits) != 1 ||
- edits[0].Edit != (manifestedit.KustomizationEdit{
- Section: manifestedit.KustomizationSectionReplicas, EntryIndex: 0,
- EntryName: "web", Field: "count", Value: "5",
- }) {
- t.Fatalf("want one count edit to 5, got %+v", edits)
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", deploymentSource("app:1.0.0", "")), // no spec.replicas
+ file("kustomization.yaml", kustomizationWith("replicas:\n - name: web\n count: 3\n")),
}
+
+ out, edits := splitFixture(t, files, "deployment.yaml", liveDeployment("app:1.0.0", &five))
+
+ _, has, _ := unstructured.NestedInt64(out.Object, "spec", "replicas")
+ require.False(t, has, "the source has no replicas field, so the desired-for-file must not either")
+ require.Len(t, edits, 1)
+ require.Equal(t, manifestedit.KustomizationEdit{
+ Section: manifestedit.KustomizationSectionReplicas, EntryIndex: 0,
+ EntryName: "web", Field: "count", Value: "5",
+ }, edits[0].Edit)
}
-// TestSplitDesired_ReplicasMatchRestoresSource: live equals the pinned count; the
-// source's own (stale) value is restored and nothing is routed.
+// Live equals the pinned count: the source's own (stale) value is restored and nothing routes.
func TestSplitDesired_ReplicasMatchRestoresSource(t *testing.T) {
- one, three := int64(1), int64(3)
- git := deploymentObj("app:1.0.0", &one)
- desired := desiredOf(deploymentObj("app:1.0.0", &three))
- ov := &KustomizeOverrides{Replicas: []ReplicaOverride{
- {Source: "kustomization.yaml", Index: 0, Name: "web", Count: 3},
- }}
-
- out, edits := SplitDesiredForOverrides(git, desired, ov)
- if got, _, _ := unstructured.NestedInt64(out.Object, "spec", "replicas"); got != 1 {
- t.Errorf("desired-for-file replicas = %d, want the source's 1", got)
+ three := int64(3)
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", deploymentSource("app:1.0.0", "1")),
+ file("kustomization.yaml", kustomizationWith("replicas:\n - name: web\n count: 3\n")),
}
- if len(edits) != 0 {
- t.Errorf("want no edits, got %+v", edits)
+
+ out, edits := splitFixture(t, files, "deployment.yaml", liveDeployment("app:1.0.0", &three))
+
+ count, _, _ := unstructured.NestedInt64(out.Object, "spec", "replicas")
+ require.Equal(t, int64(1), count, "the source's own value is restored")
+ require.Empty(t, edits)
+}
+
+// B2: kustomize's replica fieldspec is Deployment, ReplicaSet, StatefulSet AND
+// ReplicationController. isReplicaKind listed three of the four, so a scale on an RC governed
+// by a replicas: entry was written into the source document, where the transformer overrode it
+// straight back — non-converging drift, silently, forever.
+//
+// There is no list of kinds any more. kustomize's fieldspec is the authority, and the dye
+// reports what it did.
+func TestSplitDesired_ReplicationControllerIsGoverned(t *testing.T) {
+ source := `apiVersion: v1
+kind: ReplicationController
+metadata:
+ name: web
+spec:
+ template:
+ spec:
+ containers:
+ - name: app
+ image: app:1.0.0
+`
+ files := []manifestedit.FileContent{
+ file("rc.yaml", source),
+ file("kustomization.yaml", "resources:\n - rc.yaml\nreplicas:\n - name: web\n count: 3\n"),
+ }
+ live := map[string]interface{}{
+ "apiVersion": "v1",
+ "kind": "ReplicationController",
+ "metadata": map[string]interface{}{"name": "web"},
+ "spec": map[string]interface{}{
+ "replicas": int64(7),
+ "template": map[string]interface{}{
+ "spec": map[string]interface{}{
+ "containers": []interface{}{
+ map[string]interface{}{"name": "app", "image": "app:1.0.0"},
+ },
+ },
+ },
+ },
}
+
+ out, edits := splitFixture(t, files, "rc.yaml", live)
+
+ _, has, _ := unstructured.NestedInt64(out.Object, "spec", "replicas")
+ require.False(t, has, "the source has no replicas field; the entry supplies it")
+ require.Len(t, edits, 1, "kustomize DOES scale a ReplicationController, so the count must route")
+ require.Equal(t, "count", edits[0].Edit.Field)
+ require.Equal(t, "7", edits[0].Edit.Value)
}
-// TestSplitDesired_ReplicasIgnoresOtherKinds: the replica transformer only
-// touches Deployment/ReplicaSet/StatefulSet.
-func TestSplitDesired_ReplicasIgnoresOtherKinds(t *testing.T) {
- git := map[string]interface{}{
- "apiVersion": "v1", "kind": "ConfigMap",
- "metadata": map[string]interface{}{"name": "web"},
+// A document no replicas: entry names gets no dyed count, so nothing governs its spec.replicas
+// and a live scale simply writes through into the file.
+//
+// The second workload is not padding: kustomize REFUSES to build a folder whose replicas: entry
+// matches nothing ("resource with name other does not match a config with the following GVK
+// [Deployment StatefulSet ReplicaSet ReplicationController]"). Which is also kustomize stating
+// its own replica fieldspec out loud — all four kinds, the fourth being the one isReplicaKind
+// forgot.
+func TestSplitDesired_UnmatchedNameLeavesReplicasToTheFile(t *testing.T) {
+ other := `apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: other
+spec:
+ template:
+ spec:
+ containers:
+ - name: app
+ image: app:1.0.0
+`
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", deploymentSource("app:1.0.0", "1")),
+ file("other.yaml", other),
+ file("kustomization.yaml",
+ "resources:\n - deployment.yaml\n - other.yaml\nreplicas:\n - name: other\n count: 3\n"),
}
- desired := desiredOf(map[string]interface{}{
- "apiVersion": "v1", "kind": "ConfigMap",
- "metadata": map[string]interface{}{"name": "web"},
- "spec": map[string]interface{}{"replicas": int64(5)},
- })
- ov := &KustomizeOverrides{Replicas: []ReplicaOverride{
- {Source: "kustomization.yaml", Index: 0, Name: "web", Count: 3},
- }}
-
- out, edits := SplitDesiredForOverrides(git, desired, ov)
- if got, _, _ := unstructured.NestedInt64(out.Object, "spec", "replicas"); got != 5 {
- t.Errorf("non-workload kinds are untouched, got replicas %d", got)
+ five := int64(5)
+
+ out, edits := splitFixture(t, files, "deployment.yaml", liveDeployment("app:1.0.0", &five))
+
+ count, _, _ := unstructured.NestedInt64(out.Object, "spec", "replicas")
+ require.Equal(t, int64(5), count, "no entry names web, so the source file owns its count")
+ require.Empty(t, edits)
+}
+
+// B3, both halves, measured against kustomize:
+//
+// - volumes[].image.reference IS rewritten by the image transformer. The old collector never
+// looked at it, so the rendered value was mirrored back into the source document as though
+// the user had typed it. It must now route to the entry like any other image.
+// - ephemeralContainers are NOT rewritten. No dye lands there, so no entry is credited, and
+// the change belongs in the source file — which is exactly where it goes.
+func TestSplitDesired_VolumeImageRoutesAndEphemeralContainerDoesNot(t *testing.T) {
+ source := `apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: web
+spec:
+ template:
+ spec:
+ containers:
+ - name: app
+ image: app:1.0.0
+ ephemeralContainers:
+ - name: debug
+ image: app:1.0.0
+ volumes:
+ - name: vol
+ image:
+ reference: app:1.0.0
+`
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", source),
+ file("kustomization.yaml", kustomizationWith("images:\n - name: app\n newTag: \"1.5.0\"\n")),
+ }
+ // Live: the container and the VOLUME render at 1.5.0 (kustomize rewrote both); the
+ // ephemeral container still renders at the source value. The user bumps the entry's tag to
+ // 2.0.0 and independently edits the ephemeral container.
+ live := map[string]interface{}{
+ "apiVersion": "apps/v1",
+ "kind": "Deployment",
+ "metadata": map[string]interface{}{"name": "web"},
+ "spec": map[string]interface{}{
+ "template": map[string]interface{}{
+ "spec": map[string]interface{}{
+ "containers": []interface{}{
+ map[string]interface{}{"name": "app", "image": "app:2.0.0"},
+ },
+ "ephemeralContainers": []interface{}{
+ map[string]interface{}{"name": "debug", "image": "app:9.9.9"},
+ },
+ "volumes": []interface{}{
+ map[string]interface{}{
+ "name": "vol",
+ "image": map[string]interface{}{"reference": "app:2.0.0"},
+ },
+ },
+ },
+ },
+ },
+ }
+
+ out, edits := splitFixture(t, files, "deployment.yaml", live)
+
+ require.Len(t, edits, 1, "the container and the volume agree on 2.0.0, so one entry edit carries both")
+ require.Equal(t, "newTag", edits[0].Edit.Field)
+ require.Equal(t, "2.0.0", edits[0].Edit.Value)
+
+ slots := map[string]string{}
+ for _, s := range collectImageSlots(out.Object) {
+ slots[s.key] = s.image
}
- if len(edits) != 0 {
- t.Errorf("want no edits, got %+v", edits)
+ require.Contains(t, slots, "/spec/template/spec/volumes\x00vol")
+ require.Equal(t, "app:1.0.0", slots["/spec/template/spec/volumes\x00vol"],
+ "the volume image is entry-governed, so the source keeps its bytes")
+ require.Equal(t, "app:1.0.0", slots["/spec/template/spec/containers\x00app"],
+ "the container image is entry-governed, so the source keeps its bytes")
+ require.Equal(t, "app:9.9.9", slots["/spec/template/spec/ephemeralContainers\x00debug"],
+ "kustomize does not rewrite ephemeralContainers, so the SOURCE FILE owns this one")
+}
+
+// A document no entry governs at all attributes nothing, and the whole object writes through.
+func TestSplitDesired_NoOverridesWritesThrough(t *testing.T) {
+ files := []manifestedit.FileContent{
+ file("deployment.yaml", deploymentSource("app:1.0.0", "")),
+ file("kustomization.yaml", "resources:\n - deployment.yaml\n"),
}
+
+ out, edits := splitFixture(t, files, "deployment.yaml", liveDeployment("app:2.0.0", nil))
+
+ require.Empty(t, edits)
+ require.Equal(t, "app:2.0.0", imageOf(t, out), "nothing governs it, so the live value lands in the file")
}
diff --git a/internal/manifestanalyzer/overrides_test.go b/internal/manifestanalyzer/overrides_test.go
index 545866c4..d3a02011 100644
--- a/internal/manifestanalyzer/overrides_test.go
+++ b/internal/manifestanalyzer/overrides_test.go
@@ -8,6 +8,8 @@ import (
"path/filepath"
"testing"
+ "github.com/stretchr/testify/require"
+
"github.com/ConfigButler/gitops-reverser/internal/git/manifestedit"
"github.com/ConfigButler/gitops-reverser/internal/typeset"
)
@@ -247,3 +249,43 @@ func hasOverrideAmbiguityDiag(store *ManifestStore) bool {
}
return false
}
+
+// A document can render an image while NO images:/replicas: entry governs it: the chain is
+// nil, but the attribution is not, because the projection still needs to know what the folder
+// renders that slot to. The two are different questions, and anyOverrides has to ask both --
+// otherwise two roots disagreeing only on the attribution would diverge in the fingerprint
+// while ambiguous() stayed false, leaving the document silently un-routed with no diagnostic
+// and no fan-in refusal.
+//
+// Requested in review on #233.
+func TestRecord_AttributionWithoutAChainIsStillSomethingAtStake(t *testing.T) {
+ files := []manifestedit.FileContent{
+ {Path: "deployment.yaml", Content: []byte(`apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: web
+spec:
+ template:
+ spec:
+ containers:
+ - name: app
+ image: app:v1
+`)},
+ // No images:, no replicas: -- nothing governs this document.
+ {Path: "kustomization.yaml", Content: []byte("resources:\n - deployment.yaml\n")},
+ }
+
+ assignments, failed := renderChains(files, parseKustomizations(files))
+ require.Empty(t, failed)
+
+ a := assignments[chainKey{originPath: "deployment.yaml", kind: "Deployment", name: "web"}]
+ require.NotNil(t, a)
+ require.Nil(t, a.overrides, "no entry governs it, so there is no chain")
+ require.NotNil(t, a.rendered, "but the folder still renders its image, and the projection reads that")
+ require.True(t, a.anyOverrides,
+ "an attribution with no chain is still something an edit could be routed through")
+
+ // And with nothing supplying the image, an edit to it flows into the source file.
+ require.Empty(t, a.rendered.Images["/spec/template/spec/containers\x00app"].Tag,
+ "no entry supplies the tag")
+}
diff --git a/internal/manifestanalyzer/render_verify.go b/internal/manifestanalyzer/render_verify.go
new file mode 100644
index 00000000..7541ec45
--- /dev/null
+++ b/internal/manifestanalyzer/render_verify.go
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package manifestanalyzer
+
+import (
+ "bytes"
+ "encoding/json"
+ "fmt"
+ "sort"
+ "strings"
+
+ "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
+ "github.com/ConfigButler/gitops-reverser/internal/git/manifestedit"
+)
+
+// This file is the oracle: before a commit that routes anything through a kustomization,
+// we render the repository as it would stand AFTER the commit and refuse it unless the
+// answer is exactly the one we intended.
+//
+// ATTRIBUTION MAY BE HEURISTIC. VERIFICATION MAY NOT. Deciding which file an edit belongs
+// in is an inference, and this workstream is deliberately replacing one inference with a
+// better one. But the check that says "and the result is right" must not be an inference at
+// all, because a check that shares the blind spot of the thing it checks turns a wrong guess
+// into a CONFIDENT wrong write.
+//
+// That is precisely what the deleted simulateImageRender did: it replayed OUR chain over the
+// images OUR projection had planned, so a case our model got wrong, it got wrong twice — and
+// agreed with itself. This cannot do that. It runs the library Flux renders with, over the
+// exact bytes we are about to commit.
+//
+// It runs ONCE PER FLUSH, not once per resource, and that is a correctness requirement
+// rather than an optimisation. An images: entry is shared context: converging one Deployment
+// through it necessarily moves every other object the entry matches. Asked resource by
+// resource, "did anything else move?" is yes for the first of two Deployments that share an
+// image and are being bumped together — so a per-resource oracle would refuse a write that
+// converges perfectly well. Only the whole batch knows that the sibling is moving because it,
+// too, is being written to exactly where it now lives.
+//
+// See docs/design/support-boundary/render-attribution.md §5 and
+// docs/design/support-boundary/render-root-scoping.md §3.
+
+// WriteIntent is one document a flush writes, and what the render must show for it
+// afterwards. Everything the batch does NOT declare an intent for must come out of the
+// render byte-for-byte unchanged — that is the blast-radius half of the oracle, and it is
+// what makes it safe for the projection to guess.
+type WriteIntent struct {
+ // SourcePath is the file holding the document, slash-relative to the scan root.
+ SourcePath string
+ // Kind and Name identify the document within that file.
+ Kind, Name string
+ // Desired is the live object the document must render to. It is the whole point of
+ // the check, and it is nil only when Removed or Unchecked says there is nothing to
+ // check against.
+ Desired *unstructured.Unstructured
+ // Removed marks a document the flush deletes: it must disappear from the render.
+ Removed bool
+ // Unchecked marks a write whose rendered form we cannot predict, so the object is
+ // permitted to move without being compared. There are exactly two: a SENSITIVE
+ // document (the file is SOPS-encrypted, so kustomize renders the ciphertext, which
+ // no plaintext live object can equal), and a bounded FIELD PATCH (the event carries
+ // a few assignments, never a whole object to compare against).
+ //
+ // Unchecked weakens the oracle for those documents, and it is stated rather than
+ // hidden: we can still prove that such a write disturbs nothing ELSE, which is the
+ // half that protects other people's environments.
+ Unchecked bool
+ // Governed marks a document whose write was routed through a kustomization override
+ // chain. These are the writes the oracle exists for, so one that turns out not to be
+ // rendered by any root at all is a contradiction, and refused rather than skipped.
+ Governed bool
+}
+
+func (w WriteIntent) key() chainKey {
+ return chainKey{originPath: filepathToSlash(w.SourcePath), kind: w.Kind, name: w.Name}
+}
+
+// RenderRefusedError is the oracle's verdict: the bytes the flush was about to commit do
+// not render to the live cluster state, or they move something the flush never intended to
+// touch. It aborts the flush — nothing is written — and it names the file and the object,
+// because the correct outcome here is a REPORTED refusal, never a resource that is quietly
+// not mirrored (render-attribution.md §7).
+type RenderRefusedError struct {
+ // Reasons are the individual findings, sorted, so the message is stable across runs.
+ Reasons []string
+}
+
+func (e *RenderRefusedError) Error() string {
+ return "kustomize render refused the write: " + strings.Join(e.Reasons, "; ")
+}
+
+// VerifyBatchRenders re-renders every root of the subtree twice — as the flush found it,
+// and as the flush 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 is byte-for-byte unchanged.
+//
+// (2) is not a nicety. A kustomization is shared context: an images: entry edited to
+// converge one Deployment governs every other object it matches, and a base is rendered by
+// every overlay above it. 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 — the one edit write-fan-in
+// exists to forbid.
+//
+// before and after are complete file trees. The cost is two builds per render root, once per
+// flush, and it is only paid when the flush routed something through a kustomization.
+func VerifyBatchRenders(before, after []manifestedit.FileContent, intents []WriteIntent) error {
+ byKey := make(map[chainKey]WriteIntent, len(intents))
+ for _, in := range intents {
+ byKey[in.key()] = in
+ }
+ seen := map[chainKey]struct{}{}
+
+ var reasons []string
+ for _, root := range renderTargets(parseKustomizations(after)) {
+ was, err := renderRoot(before, root)
+ if err != nil {
+ // The tree did not build BEFORE we touched it. The acceptance gate refuses
+ // such a folder, so we should never be writing into one — but an unverifiable
+ // root is not a verified root, so say so rather than skip it.
+ reasons = append(reasons, fmt.Sprintf("render root %s did not build before the write: %v", root, err))
+ continue
+ }
+ now, err := renderRoot(after, root)
+ if err != nil {
+ reasons = append(
+ reasons,
+ fmt.Sprintf("render root %s no longer builds with the write applied: %v", root, err),
+ )
+ continue
+ }
+ reasons = append(reasons, compareRoot(root, byKey, seen, renderedByKey(was), renderedByKey(now))...)
+ }
+
+ for _, in := range intents {
+ if _, hit := seen[in.key()]; in.Governed && !hit {
+ reasons = append(reasons, fmt.Sprintf(
+ "%s/%s in %s was routed through a kustomization override, but no render root renders it",
+ in.Kind, in.Name, in.SourcePath))
+ }
+ }
+
+ if len(reasons) == 0 {
+ return nil
+ }
+ sort.Strings(reasons)
+ return &RenderRefusedError{Reasons: reasons}
+}
+
+// compareRoot checks one render root's before/after pair against the flush's intents.
+func compareRoot(
+ root string,
+ intents map[chainKey]WriteIntent,
+ seen map[chainKey]struct{},
+ was, now map[chainKey]renderedObject,
+) []string {
+ var reasons []string
+ for key := range unionKeys(was, now) {
+ before, existed := was[key]
+ after, exists := now[key]
+ intent, intended := intents[key]
+ if intended {
+ seen[key] = struct{}{}
+ }
+
+ switch {
+ case !intended:
+ // The blast radius. This object is nobody's target, so the flush has no
+ // business changing it — in this root or any other.
+ if !existed || !exists {
+ reasons = append(reasons, fmt.Sprintf(
+ "the write adds or removes %s/%s in render root %s, which it never set out to write",
+ key.kind, key.name, root))
+ continue
+ }
+ if !sameObject(before.Object, after.Object) {
+ reasons = append(reasons, fmt.Sprintf(
+ "the write also changes what render root %s renders for %s/%s (from %s), "+
+ "which it never set out to write",
+ root, key.kind, key.name, key.originPath))
+ }
+ case intent.Removed:
+ if exists {
+ reasons = append(reasons, fmt.Sprintf(
+ "%s/%s was deleted, but render root %s still renders it", key.kind, key.name, root))
+ }
+ case intent.Unchecked:
+ // A sensitive document or a bounded field patch: it is allowed to move, and
+ // there is nothing to compare it against. The other cases above still hold it
+ // to not disturbing anything else.
+ case !exists:
+ reasons = append(reasons, fmt.Sprintf(
+ "%s/%s was written, but render root %s no longer renders it", key.kind, key.name, root))
+ case !sameObject(after.Object, intent.Desired):
+ reasons = append(reasons, fmt.Sprintf(
+ "in render root %s, %s/%s (from %s) does not render to the live object after the write",
+ root, key.kind, key.name, key.originPath))
+ }
+ }
+ return reasons
+}
+
+func unionKeys(a, b map[chainKey]renderedObject) map[chainKey]struct{} {
+ out := make(map[chainKey]struct{}, len(a)+len(b))
+ for k := range a {
+ out[k] = struct{}{}
+ }
+ for k := range b {
+ out[k] = struct{}{}
+ }
+ return out
+}
+
+// sameObject compares two rendered/live objects by canonical JSON.
+//
+// Canonical JSON rather than reflect.DeepEqual on purpose: kustomize hands numbers back as
+// Go int where the API machinery uses int64 — measured, and it is why
+// unstructured.NestedInt64 reports found=false on a rendered spec.replicas — and a
+// comparison that called those two different would refuse every replica write there is.
+// json.Marshal also sorts map keys, so this is key-order independent.
+func sameObject(a, b *unstructured.Unstructured) bool {
+ if a == nil || b == nil {
+ return a == b
+ }
+ left, err := json.Marshal(a.Object)
+ if err != nil {
+ return false
+ }
+ right, err := json.Marshal(b.Object)
+ if err != nil {
+ return false
+ }
+ return bytes.Equal(left, right)
+}
+
+// renderedByKey indexes a render by the document each object came from. The key carries the
+// ORIGIN FILE, not just kind/name, so two same-named objects rendered from different source
+// files stay distinct.
+func renderedByKey(objects []renderedObject) map[chainKey]renderedObject {
+ out := make(map[chainKey]renderedObject, len(objects))
+ for _, o := range objects {
+ out[chainKey{originPath: o.OriginPath, kind: o.Object.GetKind(), name: o.Object.GetName()}] = o
+ }
+ return out
+}
diff --git a/internal/manifestanalyzer/store.go b/internal/manifestanalyzer/store.go
index 495785b2..12acdf46 100644
--- a/internal/manifestanalyzer/store.go
+++ b/internal/manifestanalyzer/store.go
@@ -184,6 +184,19 @@ type DocumentModel struct {
// docs/design/support-boundary/finished/images-and-replicas-edit-through.md.
Overrides *KustomizeOverrides
+ // Rendered is what kustomize ACTUALLY renders this document to, plus which override
+ // entry supplied each override-produced value — the values read off the real render, the
+ // suppliers read off a dyed counterfactual one. It is what the write-side projection
+ // inverts against, and it replaced ~400 lines that re-implemented kustomize's
+ // transformers in order to guess the same thing.
+ //
+ // Nil when no render root supplies a chain, when distinct roots disagree, or when the
+ // dyed build could not be trusted (see attributeRoot). Nil means NO ATTRIBUTION: the
+ // writer routes nothing to an entry, and the verification re-render adjudicates whatever
+ // the source document alone can carry. See
+ // docs/design/support-boundary/render-attribution.md.
+ Rendered *RenderedOverrides
+
// ResourceIdentity is the API-side identity (GVR + namespace + name). It is set
// only when the injected GVK->GVR mapper resolves the document's GVK to a single
// served, allowed resource; structure-only analysis (and any unresolved lookup)
@@ -496,7 +509,7 @@ func (s *ManifestStore) materialize(
if diag != nil {
s.Diagnostics = append(s.Diagnostics, *diag)
}
- overrides, ovDiag := resolveOverrides(r.Location, r.Identity, ovAssignments)
+ overrides, rendered, ovDiag := resolveOverrides(r.Location, r.Identity, ovAssignments)
if ovDiag != nil {
s.Diagnostics = append(s.Diagnostics, *ovDiag)
}
@@ -510,6 +523,7 @@ func (s *ManifestStore) materialize(
ManifestIdentity: identity,
NamespaceSource: nsSource,
Overrides: overrides,
+ Rendered: rendered,
Editable: r.Editable && !r.Encrypted,
Cause: causeFor(r),
}
diff --git a/internal/watch/event_router.go b/internal/watch/event_router.go
index 93ffa311..088c9764 100644
--- a/internal/watch/event_router.go
+++ b/internal/watch/event_router.go
@@ -269,8 +269,9 @@ func (r *EventRouter) handleScopedResyncError(
//
// - purely the .gittargetignore-shadows-a-write case (§4.3) — the unrecoverable footgun —
// gets IgnoreShadowsManagedPath;
-// - purely write-boundary violations (a planned write escaping spec.path, or an in-place
-// edit of a file more than one render root reaches) gets WriteBoundaryRefused: the folder
+// - purely write-boundary violations (a planned write escaping spec.path, an in-place edit
+// of a file more than one render root reaches, or a write kustomize will not vouch for
+// when the folder is re-rendered with it applied) gets WriteBoundaryRefused: the folder
// content is fine, the *edit* had nowhere safe to land.
//
// Any other refusal, and any mix of shapes, keeps the umbrella UnsupportedContent. The strings
@@ -281,7 +282,11 @@ func gitPathRefusalReason(refused *manifestanalyzer.AcceptanceRefusedError) stri
switch {
case refused.AllIssuesOfKinds(manifestanalyzer.IssueIgnoreShadowsManaged):
return "IgnoreShadowsManagedPath"
- case refused.AllIssuesOfKinds(manifestanalyzer.IssueWriteEscapesScope, manifestanalyzer.IssueWriteFanIn):
+ case refused.AllIssuesOfKinds(
+ manifestanalyzer.IssueWriteEscapesScope,
+ manifestanalyzer.IssueWriteFanIn,
+ manifestanalyzer.IssueRenderRefused,
+ ):
return "WriteBoundaryRefused"
default:
return "UnsupportedContent"
diff --git a/test/e2e/new_file_placement_e2e_test.go b/test/e2e/new_file_placement_e2e_test.go
index b0d2cfc7..3c745fbe 100644
--- a/test/e2e/new_file_placement_e2e_test.go
+++ b/test/e2e/new_file_placement_e2e_test.go
@@ -114,4 +114,42 @@ var _ = Describe("Manager New-File Placement", Label("manager", "new-file-placem
By("✅ new resource placed inside the kustomize overlay and registered in resources:")
})
+
+ // The round trip, and the half that used to be missing. Deleting the resource removes its
+ // file — and a resources: entry naming a file that no longer exists is one kustomize
+ // REFUSES to build over ("accumulating resources ... doesn't exist"), which would leave the
+ // repository in a state no GitOps controller can deploy. Registering the entry on create is
+ // only half the job if nothing ever takes it back out.
+ //
+ // Ordered: this deletes exactly the ConfigMap the spec above placed.
+ It("removes the resources: entry when the resource is deleted, leaving a folder that still builds", func() {
+ kustFullPath := filepath.Join(repo.CheckoutDir, gitPath, kustRepoPath)
+ newFileFullPath := filepath.Join(repo.CheckoutDir, gitPath, newFileRepoPath)
+
+ By("deleting the ConfigMap from the cluster")
+ _, err := kubectlRunInNamespace(testNs, "delete", "configmap", newConfigMap)
+ Expect(err).NotTo(HaveOccurred(), "failed to delete the ConfigMap")
+
+ By("verifying the file is gone AND its resources: entry went with it")
+ Eventually(func(g Gomega) {
+ pullLatestRepoState(g, repo.CheckoutDir)
+
+ _, statErr := os.Stat(newFileFullPath)
+ g.Expect(os.IsNotExist(statErr)).To(BeTrue(), "the resource's file must be removed")
+
+ kustBody := readRepoFile(g, kustFullPath)
+ g.Expect(kustBody).NotTo(ContainSubstring("- "+newFileRepoPath),
+ "a resources: entry pointing at a deleted file makes the folder unbuildable")
+ g.Expect(kustBody).To(ContainSubstring("- deployment.yaml"),
+ "and every other entry must be left exactly as it was")
+ }, 120*time.Second, 3*time.Second).Should(Succeed())
+
+ By("verifying kustomize can still build the folder — which is the whole point")
+ _, err = kubectlRunInNamespace(testNs, "apply", "-k",
+ filepath.Join(repo.CheckoutDir, gitPath), "--dry-run=server")
+ Expect(err).NotTo(HaveOccurred(),
+ "the overlay must still build after the delete; a dangling resources: entry would fail here")
+
+ By("✅ deleted resource's file and its resources: entry both removed; the folder still builds")
+ })
})