diff --git a/.coverage-baseline b/.coverage-baseline index a5216746..9ddeb3cd 100644 --- a/.coverage-baseline +++ b/.coverage-baseline @@ -1 +1 @@ -76.4 +76.5 diff --git a/docs/design/support-boundary/README.md b/docs/design/support-boundary/README.md index c6b7da7f..3a8d948c 100644 --- a/docs/design/support-boundary/README.md +++ b/docs/design/support-boundary/README.md @@ -38,15 +38,19 @@ flowchart LR |---|---| | Plain KRM and higher-level KRM documents | Editable in place, preserving document form where possible. A `HelmRelease`, `Application`, KRO resource, or Crossplane claim is ordinary intent-layer KRM. | | Self-contained Kustomize | Local `resources`, `namespace`, `images`, and `replicas` are supported. Image and replica changes write to their declaring entry. | -| External-base overlay | The overlay may read `../../base` as context, but writes remain inside the target overlay. Existing overlay-local documents and declared image/replica entries are editable; base-owned fields are refused. | -| Path-based strategic-merge patch | The render is accepted; the patch is read-only context. We tolerate it but do not author or edit patches yet. | +| External-base overlay | The overlay may read `../../base` as context, but writes remain inside the target overlay. Existing overlay-local documents and declared image/replica entries are editable; a brand-new object is created as an overlay-local file registered in the overlay's own `resources:`; and changing a base-supplied image/replica in one environment **authors a new `images:`/`replicas:` entry** in the overlay. Other base-owned fields are refused. | +| Path-based strategic-merge patch | The render is accepted and a patch that **edits a field** is read-only context — never authored or edited. The one authored patch is **`$patch: delete`** for an object an overlay inherits from its base (a patch file plus a `patches:` entry), verified by re-render. | | Render verification | A proposed batch is built with kustomize before any bytes are written. Mismatch or blast-radius change refuses the flush. | | Write boundary | Writes never leave `spec.path`, and a file read by more than one render root is never edited in place. | | Foreign-content boundary | A GitTarget subtree is operator-exclusive: loose scripts, binaries, and symlinks refuse the folder. Inert repo-hygiene passengers — documentation (`*.md`), a license, and `.gitignore`/`.gitattributes`/`.gitkeep` — are accepted so adopting an existing repo does not stall on them. Anything else is named in a root `.gittargetignore`. | -The remaining overlay gap is deliberately narrow: creating a **new object** and adding its -`resources:` entry needs a placement/write-path correction. It is planned, not shipped. -`scan-repo` now adopts external-base overlays too, matching the runtime: it reports a +Three overlay capabilities are now shipped and verified by re-render: creating a **new object** +(an overlay-local file plus its `resources:` entry); **authoring a missing `images:`/`replicas:` +entry** when a base-supplied image or replica count is changed in one environment; and +**`$patch: delete`** for an object the overlay inherits from its base — so editing a specific +environment adds the override, or the deletion, for you. The remaining overlay gap is a +strategic-merge patch for a base-owned *field* that is not an image, replica, or whole-object +delete. `scan-repo` now adopts external-base overlays too, matching the runtime: it reports a `kustomize-overlay` candidate as accepted and lets its `editable` count show how much the overlay owns (a pure passthrough overlay is adopted yet `editable: 0`). diff --git a/docs/design/support-boundary/kustomize-support-boundary.md b/docs/design/support-boundary/kustomize-support-boundary.md index f436a77c..11dd5b72 100644 --- a/docs/design/support-boundary/kustomize-support-boundary.md +++ b/docs/design/support-boundary/kustomize-support-boundary.md @@ -33,9 +33,14 @@ these fields a general reverse-edit feature. An external-base overlay can edit an existing overlay-local document and a declared image/replica entry. Its shared base is read-only. Creating a new overlay-local resource -and adding its `resources:` entry is **planned** pending a placement/write-path correction; -it must not be advertised as shipped yet. `scan-repo` still reports external-base overlays -as unsupported while its classification catches up with the runtime. +and adding its `resources:` entry is **shipped**, and so is **authoring a missing +`images:`/`replicas:` entry** when a base-supplied image or replica count is changed in one +environment — the writer creates the entry (and the section, if absent) in the overlay's own +kustomization, verified by re-render, rather than writing the base. A **`$patch: delete`** is +also authored for an object the overlay inherits from its base. A strategic-merge patch that +**edits a field** of a base-owned object is *not* authored — it stays read-only build context — +and that field-patch authoring is the one remaining overlay gap. `scan-repo` now adopts +external-base overlays too (a `kustomize-overlay` candidate is reported accepted). ```mermaid flowchart LR diff --git a/docs/design/support-boundary/patch-authoring.md b/docs/design/support-boundary/patch-authoring.md index bf6d0cde..37a9396d 100644 --- a/docs/design/support-boundary/patch-authoring.md +++ b/docs/design/support-boundary/patch-authoring.md @@ -36,6 +36,14 @@ flowchart LR than guessed or written through. - Inline patches, JSON6902 patches, deprecated patch spellings, and out-of-tree paths remain refused; they do not provide the narrow sparse-KRM unit this design relies on. +- **`$patch: delete` is now authored (shipped), as a slice distinct from field patching.** + Deleting an object the overlay inherits from its base writes a small `$patch: delete` document + under the overlay and names it in `patches:`, verified by the re-render oracle (the object must + leave the render, everything else unchanged; a non-matching patch is refused). It needs no + field attribution — the target is identified by apiVersion/kind/namespace/name — so it does not + wait on the scalar-attribution work below. See + [render-root scoping §1/§4](render-root-scoping.md). What is still refused is authoring a patch + that *edits a field* of a base-owned object. ## Narrow first slice diff --git a/docs/design/support-boundary/render-root-scoping.md b/docs/design/support-boundary/render-root-scoping.md index 949980d8..db53cc79 100644 --- a/docs/design/support-boundary/render-root-scoping.md +++ b/docs/design/support-boundary/render-root-scoping.md @@ -1,8 +1,12 @@ # Render-root scoping > **Implementation record.** External-base overlay support is shipped for existing -> overlay-local documents and declared `images`/`replicas` entries. New-resource entry -> creation and patch authoring remain separate work. +> overlay-local documents, declared `images`/`replicas` entries, **creating a new +> overlay-local object** (a new file plus its `resources:` entry), **authoring a missing +> `images`/`replicas` entry** when a base-supplied image or replica count is changed in one +> environment, and **`$patch: delete`** for an object the overlay inherits from its base — all +> verified by re-render. Strategic-merge patch authoring for a base-owned *field* (not an +> image/replica) remains separate work. > > Related: [support contract](support-contract.md), > [Kustomize boundary](kustomize-support-boundary.md), @@ -33,11 +37,27 @@ flowchart LR while the **write scope** remains `spec.path`. - An existing overlay-local document is edited in place. An existing `images:` or `replicas:` declaration receives the corresponding edit-through change. +- When a **base** supplies an image or replica count and it is changed in one environment (the + classic "bump the image in dev"), the writer **authors a new `images:`/`replicas:` entry** in + the overlay's own kustomization — creating the section if the overlay has none yet — rather + than writing the read-only base. The re-render verifies the entry produces the live value and + moves nothing else; an entry that would over-reach (an image name shared by another object) is + refused there, not committed. +- A **brand-new object** with no existing document is created as an overlay-local file and + registered in the overlay's **own** `resources:` entry — never the base's — with the + re-render verifying it before the commit. A new object whose field a folder-level + `images:`/`replicas:` entry would override is refused loudly by that oracle instead of + committed as a non-converging write. +- Deleting an object the overlay **inherits from its base** authors a `$patch: delete` in the + overlay (a patch file plus a `patches:` entry) rather than deleting the read-only base — the + base, and every other environment that reads it, is untouched. The re-render verifies the + object leaves the overlay's render; a patch that does not match is refused there, not committed. - A source file reached by more than one render root is read-only. The entire flush is refused before a commit when a plan would write it. - A path-based strategic-merge patch is read-only build context. It no longer rejects the whole overlay merely by existing. -- A base-owned field is refused rather than written through to the shared base. +- A base-owned *field* (not an image, replica, or whole-object delete) is refused rather than + written through to the shared base — strategic-merge field patch authoring is still planned. The discovery report has caught up: `scan-repo` runs the same adoption gate over an external-base overlay's render scope, so a `kustomize-overlay` candidate is now reported @@ -78,26 +98,36 @@ refuses the edit. See [render attribution](render-attribution.md) for the method | Observed change in selected overlay | Destination | Status | |---|---|---| -| Image tag or repository | declared overlay `images:` entry | Editable | -| Replica count | declared overlay `replicas:` entry | Editable | +| Image tag or repository | overlay `images:` entry — edited if declared, **authored if the base supplies it** | Editable | +| Replica count | overlay `replicas:` entry — edited if declared, **authored if the base supplies it** | Editable | | Existing overlay-local document field | that document | Editable | -| New object | overlay-local file plus `resources:` entry | **Planned** — placement/write-path correction required | -| Base-owned field | operator-authored strategic-merge patch | Refused pending [patch authoring](patch-authoring.md) | -| Delete inherited object in one overlay | `$patch: delete` | Refused | - -Entry creation is distinct from editing an entry. The current writer does not safely add a -missing `images:`/`replicas:` declaration or create a resource entry for a new object, so the -contract labels those paths planned rather than relying on the existing generic placement -feature. +| New object | overlay-local file plus `resources:` entry | **Editable** — placed in the overlay, registered in its own kustomization, verified by re-render | +| Delete inherited object in one overlay | overlay `$patch: delete` (patch file plus `patches:` entry) | **Editable** — authored in the overlay, verified by re-render; the base is untouched | +| Base-owned field (not image/replica/delete) | operator-authored strategic-merge patch | Refused pending [patch authoring](patch-authoring.md) | + +Entry creation is distinct from editing an entry, and both are now shipped for images/replicas. +Creating a **new object** — an overlay-local file plus a `resources:` entry in the overlay's +**own** kustomization (the base's is out of the write jail) — is shipped. **Authoring a missing +`images:`/`replicas:` entry** is too: when a base supplies an image component or replica count +and the environment changes it, the writer creates the entry (and the section, if absent) in the +overlay rather than writing the read-only base, and the re-render adjudicates the proposal. +**Deleting an inherited object** authors a `$patch: delete` in the overlay under the same +verification. What remains is a strategic-merge patch for a base-owned field that is *not* an +image, replica, or whole-object delete. ## 5. Remaining work -1. Correct overlay-local new-object placement, including a deterministic `resources:` entry. -2. Add missing `images:` and `replicas:` declarations only under the same verification proof. -3. Discovery classification is flipped (an external-base overlay reports accepted); a dedicated - cluster end-to-end overlay case remains. -4. Author a narrow scalar strategic-merge patch for base-owned fields; lists, deletes, and - structural merges require separate decisions. +1. Add a dedicated cluster end-to-end overlay case (discovery classification is already flipped — + an external-base overlay reports accepted). +2. Author a narrow scalar strategic-merge patch for a base-owned *field*; list merges, element + deletes, and structural merges require separate decisions. + +Overlay-local new-object placement (a new file plus its `resources:` entry), authoring a missing +`images:`/`replicas:` entry, and `$patch: delete` for an inherited object are all done, covered by +`TestPlacement_ExternalBaseOverlay_NewObject`, `TestOverlayAuthors_ImageEntry_ForBaseSuppliedImage` +/ `..._ReplicaEntry_ForBaseSuppliedCount`, and `TestOverlayAuthors_DeletePatch_ForInheritedObject`; +they left the list as the write path proved them end-to-end. `scan-repo`'s discovery flip landed +separately in #243. The earlier exploratory prompt material was consolidated into this implementation record and [patch authoring](patch-authoring.md). Git history retains the detailed experiments without diff --git a/docs/design/support-boundary/support-contract.md b/docs/design/support-boundary/support-contract.md index 5e9d8db7..9b05e44d 100644 --- a/docs/design/support-boundary/support-contract.md +++ b/docs/design/support-boundary/support-contract.md @@ -78,8 +78,10 @@ templates, and multi-input ResourceSet templates alike. It is specified once, in | Construct | Verdict | Why | |---|---|---| | kustomize `resources`, `namespace`, `images`, `replicas` | **Editable** | invertible; `images`/`replicas` edit-through is shipped | -| kustomize base + un-fancy overlays | **Editable, narrow** | render-root scoping reads `../../base` as read-only context and writes existing overlay-local documents plus declared image/replica entries in the overlay; the base is never written. `scan-repo` still reports these as `overlay-fan-out-unsupported` — the discovery-side flip is a follow-up | -| New object in an external-base overlay | **Planned** | needs an overlay-local file **and** a correct `resources:` entry; the current placement/write path does not yet prove that route | +| kustomize base + un-fancy overlays | **Editable, narrow** | render-root scoping reads `../../base` as read-only context and writes only inside the overlay; the base is never written. It edits existing overlay-local documents and declared image/replica entries, and **authors a new `images:`/`replicas:` entry** when a base-supplied image/replica is changed in one environment. `scan-repo` still reports these as `overlay-fan-out-unsupported` — the discovery-side flip is a follow-up | +| New object in an external-base overlay | **Editable** | placed as an overlay-local file and registered in the overlay's own `resources:` entry (never the base's), verified by re-render before the commit; proven by `TestPlacement_ExternalBaseOverlay_NewObject`. A new object a folder `images:`/`replicas:` entry would override is refused loudly by the oracle, not committed | +| Change a base-supplied image/replica in one overlay | **Editable** | the writer authors a new `images:`/`replicas:` entry in the overlay's own kustomization (the base stays read-only), verified by re-render; proven by `TestOverlayAuthors_ImageEntry_ForBaseSuppliedImage` / `..._ReplicaEntry_ForBaseSuppliedCount` | +| Delete a base-inherited object in one overlay | **Editable** | the writer authors a `$patch: delete` (patch file plus `patches:` entry) in the overlay, verified by re-render that the object leaves the render; the base is untouched. Proven by `TestOverlayAuthors_DeletePatch_ForInheritedObject`. A patch that fails to match is refused by the oracle, not committed | | kustomize base shared by >1 overlay, edited in place | **Refused** | fan-in > 1 | | kustomize `patches:` — a strategic-merge document named by `path:` | **Tolerated, not authored** | the folder is accepted and the render is mirrored; the patch is read-only build context. An edit to a field the patch OWNS is refused per object, not per folder | | kustomize `patches:` — inline, JSON6902, or a path outside the tree | **Refused** | not a sparse KRM document we can read; refused by name | diff --git a/internal/git/manifestedit/kustomization.go b/internal/git/manifestedit/kustomization.go index 9d94eab1..c60826d3 100644 --- a/internal/git/manifestedit/kustomization.go +++ b/internal/git/manifestedit/kustomization.go @@ -178,6 +178,158 @@ func AppendKustomizationResource(path string, content []byte, entry string) (Edi return EditResult{Content: []byte(joinDocuments(docs)), Mode: EditPatched}, nil } +// AppendKustomizationOverride AUTHORS a new images:/replicas: override entry in a +// kustomization.yaml — the mechanism half of "edit a specific environment and the override is +// added for you" (docs/design/support-boundary/render-root-scoping.md §4). Unlike +// PatchKustomization, which only updates a scalar on an entry that already exists, this creates +// the entry (and the section sequence, if the kustomization has none yet) so an overlay can +// override a value its base supplies WITHOUT touching the read-only base. +// +// It writes exactly one field beside name:, matching how the writer inverts one changed image +// component or replica count at a time: +// +// images: { name: , newName|newTag|digest: } +// replicas: { name: , count: } +// +// It is idempotent and never duplicates: an entry named name that already sets field to value is +// a no-op (EditNoChange), so a resync re-observing the same live state does not append a second +// entry. All-or-nothing like its siblings: a multi-document file, unparseable YAML, or a +// non-mapping document skips the whole call with a diagnostic. Every proposal it produces is put +// to kustomize by the re-render oracle before it can become a commit, so an entry that would +// over-reach (an images: name shared by another object the overlay did not mean to move) is +// refused there, not written. +func AppendKustomizationOverride( + path string, content []byte, section, name, field, value 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...)} + } + if section != KustomizationSectionImages && section != KustomizationSectionReplicas { + return skip("kustomization %s: unknown override section %q", path, section) + } + + docs, idx, root, reason, ok := locateKustomizationDocument(path, content) + if !ok { + return skip("%s", reason) + } + target := docs[idx].body + + seq := nodeMapGet(root, section) + if seq != nil && seq.Kind != yaml.SequenceNode { + return skip("kustomization %s: %s is not a sequence", path, section) + } + if seq == nil { + seq = &yaml.Node{Kind: yaml.SequenceNode, Tag: "!!seq"} + root.Content = append(root.Content, + &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: section}, + seq) + } + if overrideEntryPresent(seq, name, field, value) { + return EditResult{Content: content, Mode: EditNoChange}, nil // already authored + } + seq.Content = append(seq.Content, overrideEntryNode(section, name, field, value)) + + 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 +} + +// AppendKustomizationPatch adds a path-based `patches:` entry (`{ path: }`) to a +// kustomization.yaml, creating the patches: sequence if it has none — the mechanism half of +// authoring a `$patch: delete` for an object an overlay inherits from its base +// (docs/design/support-boundary/render-root-scoping.md §4). The patch document itself is written +// separately; this only names it so kustomize applies it. +// +// It is idempotent (an entry that already names patchPath is EditNoChange) and all-or-nothing +// like its siblings. The re-render oracle verifies the object actually disappears from the render +// before the flush can commit, so a patch that does not match is refused there, not committed. +func AppendKustomizationPatch(path string, content []byte, patchPath 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 + + seq := nodeMapGet(root, "patches") + if seq != nil && seq.Kind != yaml.SequenceNode { + return skip("kustomization %s: patches is not a sequence", path) + } + if seq == nil { + seq = &yaml.Node{Kind: yaml.SequenceNode, Tag: "!!seq"} + root.Content = append(root.Content, + &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "patches"}, seq) + } + for _, item := range seq.Content { + if item.Kind != yaml.MappingNode { + continue + } + if p := nodeMapGet(item, "path"); p != nil && strings.TrimSpace(p.Value) == strings.TrimSpace(patchPath) { + return EditResult{Content: content, Mode: EditNoChange}, nil // already listed + } + } + seq.Content = append(seq.Content, &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map", Content: []*yaml.Node{ + {Kind: yaml.ScalarNode, Tag: "!!str", Value: "path"}, + {Kind: yaml.ScalarNode, Tag: "!!str", Value: patchPath}, + }}) + + 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 +} + +// overrideEntryPresent reports whether the sequence already holds an entry named name that sets +// field to value — the idempotency check that keeps a resync from appending a duplicate. +func overrideEntryPresent(seq *yaml.Node, name, field, value string) bool { + for _, item := range seq.Content { + if item.Kind != yaml.MappingNode { + continue + } + if n := nodeMapGet(item, "name"); n == nil || n.Value != name { + continue + } + if f := nodeMapGet(item, field); f != nil && f.Value == value { + return true + } + } + return false +} + +// overrideEntryNode builds the `{ name: , : }` mapping for a new override +// entry. count is integer-typed (kustomize's replicas count is an int); image components are +// string-typed and left plain so the encoder can quote a "1.29"-style tag safely. +func overrideEntryNode(section, name, field, value string) *yaml.Node { + valueNode := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: value} + if section == KustomizationSectionReplicas && field == "count" { + valueNode.Tag = "!!int" + } + return &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map", Content: []*yaml.Node{ + {Kind: yaml.ScalarNode, Tag: "!!str", Value: "name"}, + {Kind: yaml.ScalarNode, Tag: "!!str", Value: name}, + {Kind: yaml.ScalarNode, Tag: "!!str", Value: field}, + valueNode, + }} +} + // 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 diff --git a/internal/git/manifestedit/kustomization_test.go b/internal/git/manifestedit/kustomization_test.go index af35e422..8c31dab5 100644 --- a/internal/git/manifestedit/kustomization_test.go +++ b/internal/git/manifestedit/kustomization_test.go @@ -183,6 +183,101 @@ func TestAppendKustomizationResource_IdempotentWhenAlreadyListed(t *testing.T) { } } +func TestAppendKustomizationOverride_AddsImageEntryToExistingSection(t *testing.T) { + res, diags := AppendKustomizationOverride( + "kustomization.yaml", []byte(kustomizationFixture), KustomizationSectionImages, "nginx", "newTag", "1.29") + if res.Mode != EditPatched { + t.Fatalf("Mode = %q, want patched (diags %+v)", res.Mode, diags) + } + got := string(res.Content) + // The existing entry survives with its comments; the new entry is appended, tag quoted. + for _, want := range []string{ + "# pin the app image here", + "newTag: \"6.4.0\" # deployed version", + "- name: nginx", + "newTag: \"1.29\"", + } { + if !strings.Contains(got, want) { + t.Errorf("want %q in:\n%s", want, got) + } + } +} + +func TestAppendKustomizationOverride_CreatesReplicasSectionWhenAbsent(t *testing.T) { + // A kustomization with no replicas: section yet gains one with a single entry. + src := "apiVersion: kustomize.config.k8s.io/v1beta1\nkind: Kustomization\nnamespace: app\n" + + "resources:\n - ../../base\n" + res, diags := AppendKustomizationOverride( + "kustomization.yaml", []byte(src), KustomizationSectionReplicas, "web", "count", "5") + if res.Mode != EditPatched { + t.Fatalf("Mode = %q, want patched (diags %+v)", res.Mode, diags) + } + got := string(res.Content) + if !strings.Contains(got, "replicas:") || !strings.Contains(got, "- name: web") || + !strings.Contains(got, "count: 5") { + t.Errorf("expected a new replicas: section with the entry:\n%s", got) + } + // count must be an integer, never a quoted string. + if strings.Contains(got, "count: \"5\"") || strings.Contains(got, "count: '5'") { + t.Errorf("count must be integer-typed:\n%s", got) + } + // The base reference must survive untouched. + if !strings.Contains(got, "- ../../base") { + t.Errorf("resources: must be untouched:\n%s", got) + } +} + +func TestAppendKustomizationOverride_IdempotentWhenEntryAlreadyAuthored(t *testing.T) { + res, _ := AppendKustomizationOverride( + "kustomization.yaml", []byte(kustomizationFixture), + KustomizationSectionImages, "ghcr.io/example/podinfo", "newTag", "6.4.0") + if res.Mode != EditNoChange { + t.Fatalf("Mode = %q, want no-change for an entry that already sets that value", res.Mode) + } + if string(res.Content) != kustomizationFixture { + t.Errorf("a no-op must leave the bytes byte-identical") + } +} + +func TestAppendKustomizationOverride_RefusesUnknownSection(t *testing.T) { + res, diags := AppendKustomizationOverride( + "kustomization.yaml", []byte(kustomizationFixture), "patches", "web", "path", "p.yaml") + if res.Mode != EditSkipped || len(diags) == 0 { + t.Fatalf("an unknown section must skip with a diagnostic; got %q diags=%+v", res.Mode, diags) + } + if string(res.Content) != kustomizationFixture { + t.Errorf("a skip must leave the bytes untouched") + } +} + +func TestAppendKustomizationPatch_CreatesPatchesSectionWhenAbsent(t *testing.T) { + src := "apiVersion: kustomize.config.k8s.io/v1beta1\nkind: Kustomization\nnamespace: app\n" + + "resources:\n - ../../base\n" + res, diags := AppendKustomizationPatch("kustomization.yaml", []byte(src), "shared-delete.yaml") + if res.Mode != EditPatched { + t.Fatalf("Mode = %q, want patched (diags %+v)", res.Mode, diags) + } + got := string(res.Content) + if !strings.Contains(got, "patches:") || !strings.Contains(got, "path: shared-delete.yaml") { + t.Errorf("expected a new patches: section naming the file:\n%s", got) + } + if !strings.Contains(got, "- ../../base") { + t.Errorf("resources: must be untouched:\n%s", got) + } +} + +func TestAppendKustomizationPatch_IdempotentWhenAlreadyListed(t *testing.T) { + src := "apiVersion: kustomize.config.k8s.io/v1beta1\nkind: Kustomization\n" + + "patches:\n - path: shared-delete.yaml\n" + res, _ := AppendKustomizationPatch("kustomization.yaml", []byte(src), "shared-delete.yaml") + if res.Mode != EditNoChange { + t.Fatalf("Mode = %q, want no-change for a patch already listed", res.Mode) + } + if string(res.Content) != src { + t.Errorf("a no-op must leave the bytes byte-identical") + } +} + func TestRemoveKustomizationResource_DropsEntryPreservingHandAuthoring(t *testing.T) { withExtra, _ := AppendKustomizationResource( "kustomization.yaml", diff --git a/internal/git/placement_test.go b/internal/git/placement_test.go index 79626b51..c2cb642c 100644 --- a/internal/git/placement_test.go +++ b/internal/git/placement_test.go @@ -241,6 +241,225 @@ func TestPlacement_UndecodableKustomization_RefusesTheFlush(t *testing.T) { require.Error(t, err, "a kustomization kustomize cannot build must refuse the folder, not be written into") } +// TestPlacement_ExternalBaseOverlay_NewObject proves render-root scoping's WRITE half for a +// brand-new object: a GitTarget rooted at an overlay that reads ../../base gains a new +// ConfigMap. The file must land inside the overlay (never the read-only base), the overlay's +// kustomization must gain the resources: entry so kustomize renders it, the base must be left +// byte-for-byte untouched, and the render oracle must accept the flush — kustomize builds the +// new object through the re-rooted scope. This is the "New object -> overlay-local file plus +// resources: entry" row of render-root-scoping.md §4. +func TestPlacement_ExternalBaseOverlay_NewObject(t *testing.T) { + worktree := newWorktreeForTest(t) + root := worktree.Filesystem.Root() + + // The read-only base, outside the overlay's own subtree. + seedPlacedManifest(t, worktree, "base/kustomization.yaml", "resources:\n - deployment.yaml\n") + seedPlacedManifest(t, worktree, "base/deployment.yaml", + "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: web\n") + // The overlay GitTarget reaches ../../base and sets the namespace transformer. + seedPlacedManifest(t, worktree, "overlays/test/kustomization.yaml", + "namespace: podinfo-test\nresources:\n - ../../base\n") + baseKustBefore, err := os.ReadFile(filepath.Join(root, "base/kustomization.yaml")) + require.NoError(t, err) + + w := &BranchWorker{contentWriter: newContentWriter(types.SensitiveResourcePolicy{}), mapper: configMapMapper()} + changed, err := w.flushEventsToWorktree( + context.Background(), worktree, "overlays/test", + []Event{newConfigMapEvent("cache", "podinfo-test")}, nil, + ) + require.NoError(t, err, "the overlay new-object flush must pass the render oracle") + require.True(t, changed) + + // The new object lands inside the overlay, not the base. + newFile, err := os.ReadFile(filepath.Join(root, "overlays/test/cache.yaml")) + require.NoError(t, err, "the new resource must land inside the overlay directory") + assert.Contains(t, string(newFile), "name: cache") + + // The overlay kustomization gains the resources: entry so kustomize renders it. + kust, err := os.ReadFile(filepath.Join(root, "overlays/test/kustomization.yaml")) + require.NoError(t, err) + assert.Contains(t, string(kust), "- ../../base", "the base reference must survive") + assert.Contains(t, string(kust), "cache.yaml", "the new file must be registered in the overlay's resources:") + + // The read-only base is never written into. + _, statErr := os.Stat(filepath.Join(root, "base/cache.yaml")) + assert.True(t, os.IsNotExist(statErr), "nothing may be written into the read-only base") + baseKustAfter, err := os.ReadFile(filepath.Join(root, "base/kustomization.yaml")) + require.NoError(t, err) + assert.Equal(t, string(baseKustBefore), string(baseKustAfter), + "the base kustomization must be untouched") +} + +// overlayBaseDeploymentWorktree seeds an external-base overlay whose base declares one +// Deployment, and returns the worktree. The overlay reaches ../../base and sets the namespace. +func overlayBaseDeploymentWorktree(t *testing.T, baseImage string, baseReplicas string) *gogit.Worktree { + t.Helper() + worktree := newWorktreeForTest(t) + seedPlacedManifest(t, worktree, "base/kustomization.yaml", "resources:\n - deployment.yaml\n") + dep := "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: web\n namespace: podinfo-test\nspec:\n" + if baseReplicas != "" { + dep += " replicas: " + baseReplicas + "\n" + } + dep += " template:\n spec:\n containers:\n - name: app\n image: " + baseImage + "\n" + seedPlacedManifest(t, worktree, "base/deployment.yaml", dep) + seedPlacedManifest(t, worktree, "overlays/test/kustomization.yaml", + "namespace: podinfo-test\nresources:\n - ../../base\n") + return worktree +} + +func liveDeployment(image string, replicas int64) Event { + spec := map[string]interface{}{ + "template": map[string]interface{}{"spec": map[string]interface{}{ + "containers": []interface{}{map[string]interface{}{"name": "app", "image": image}}}}, + } + if replicas >= 0 { + spec["replicas"] = replicas + } + return Event{ + Object: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apps/v1", "kind": "Deployment", + "metadata": map[string]interface{}{"name": "web", "namespace": "podinfo-test"}, + "spec": spec, + }}, + Identifier: types.NewResourceIdentifier("apps", "v1", "deployments", "podinfo-test", "web"), + Operation: "UPDATE", + } +} + +func flushOverlayDeployment(t *testing.T, worktree *gogit.Worktree, event Event) error { + t.Helper() + w := &BranchWorker{contentWriter: newContentWriter(types.SensitiveResourcePolicy{}), mapper: deploymentMapper()} + _, err := w.flushEventsToWorktree(context.Background(), worktree, "overlays/test", []Event{event}, nil) + return err +} + +// TestOverlayAuthors_ImageEntry_ForBaseSuppliedImage is the flagship of "edit a specific +// environment and the override is authored for you": bumping a base-rendered Deployment's image +// in an overlay authors a new images: entry in the overlay's OWN kustomization (never the base), +// verified by the re-render oracle. Before this the flush was refused for escaping the write jail. +func TestOverlayAuthors_ImageEntry_ForBaseSuppliedImage(t *testing.T) { + worktree := overlayBaseDeploymentWorktree(t, "nginx:1.0", "") + root := worktree.Filesystem.Root() + + require.NoError(t, flushOverlayDeployment(t, worktree, liveDeployment("nginx:2.0", -1)), + "an image bump must be authored as an overlay images: entry, not refused") + + kust, err := os.ReadFile(filepath.Join(root, "overlays/test/kustomization.yaml")) + require.NoError(t, err) + assert.Contains(t, string(kust), "images:", "the overlay must gain an images: section") + assert.Contains(t, string(kust), "name: nginx", "the entry names the base image") + assert.Contains(t, string(kust), "newTag: \"2.0\"", "the entry carries the new tag") + + base, err := os.ReadFile(filepath.Join(root, "base/deployment.yaml")) + require.NoError(t, err) + assert.Contains(t, string(base), "image: nginx:1.0", "the read-only base image must be untouched") +} + +// TestOverlayAuthors_ReplicaEntry_ForBaseSuppliedCount scales a base-rendered Deployment in an +// overlay: the overlay authors a replicas: entry over the base's count, base untouched. +func TestOverlayAuthors_ReplicaEntry_ForBaseSuppliedCount(t *testing.T) { + worktree := overlayBaseDeploymentWorktree(t, "nginx:1.0", "2") + root := worktree.Filesystem.Root() + + require.NoError(t, flushOverlayDeployment(t, worktree, liveDeployment("nginx:1.0", 5)), + "a scale must be authored as an overlay replicas: entry, not refused") + + kust, err := os.ReadFile(filepath.Join(root, "overlays/test/kustomization.yaml")) + require.NoError(t, err) + assert.Contains(t, string(kust), "replicas:", "the overlay must gain a replicas: section") + assert.Contains(t, string(kust), "name: web", "the entry names the resource") + assert.Contains(t, string(kust), "count: 5", "the entry carries the new count") + + base, err := os.ReadFile(filepath.Join(root, "base/deployment.yaml")) + require.NoError(t, err) + assert.Contains(t, string(base), "replicas: 2", "the read-only base replica count must be untouched") +} + +// TestOverlayAuthors_Idempotent_OnResync re-observes the same live image after the entry was +// authored: the store now sees the entry, so the change routes to it (no duplicate entry). +func TestOverlayAuthors_Idempotent_OnResync(t *testing.T) { + worktree := overlayBaseDeploymentWorktree(t, "nginx:1.0", "") + root := worktree.Filesystem.Root() + + require.NoError(t, flushOverlayDeployment(t, worktree, liveDeployment("nginx:2.0", -1))) + // A second flush of the same live state must not append a second entry. + require.NoError(t, flushOverlayDeployment(t, worktree, liveDeployment("nginx:2.0", -1))) + + kust, err := os.ReadFile(filepath.Join(root, "overlays/test/kustomization.yaml")) + require.NoError(t, err) + assert.Equal(t, 1, strings.Count(string(kust), "name: nginx"), + "the authored images: entry must appear exactly once") +} + +// TestOverlayAuthors_DeletePatch_ForInheritedObject deletes an object the overlay inherits from +// its base: the writer authors a $patch: delete in the overlay (file + patches: entry), the +// re-render oracle proves the object leaves the render, and the read-only base is untouched. +func TestOverlayAuthors_DeletePatch_ForInheritedObject(t *testing.T) { + worktree := newWorktreeForTest(t) + root := worktree.Filesystem.Root() + seedPlacedManifest(t, worktree, "base/kustomization.yaml", "resources:\n - cm.yaml\n") + seedPlacedManifest(t, worktree, "base/cm.yaml", + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: shared\n namespace: podinfo-test\ndata:\n k: v\n") + seedPlacedManifest(t, worktree, "overlays/test/kustomization.yaml", + "namespace: podinfo-test\nresources:\n - ../../base\n") + + del := Event{ + Object: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "v1", "kind": "ConfigMap", + "metadata": map[string]interface{}{"name": "shared", "namespace": "podinfo-test"}}}, + Identifier: types.NewResourceIdentifier("", "v1", "configmaps", "podinfo-test", "shared"), + Operation: "DELETE", + } + w := &BranchWorker{contentWriter: newContentWriter(types.SensitiveResourcePolicy{}), mapper: configMapMapper()} + _, err := w.flushEventsToWorktree(context.Background(), worktree, "overlays/test", []Event{del}, nil) + require.NoError(t, err, "deleting an inherited object must author a $patch: delete, not refuse") + + patch, err := os.ReadFile(filepath.Join(root, "overlays/test/configmap-shared-delete.yaml")) + require.NoError(t, err, "the overlay must gain a $patch: delete file") + assert.Contains(t, string(patch), "$patch: delete") + assert.Contains(t, string(patch), "name: shared") + + kust, err := os.ReadFile(filepath.Join(root, "overlays/test/kustomization.yaml")) + require.NoError(t, err) + assert.Contains(t, string(kust), "patches:") + assert.Contains(t, string(kust), "configmap-shared-delete.yaml") + + base, err := os.ReadFile(filepath.Join(root, "base/cm.yaml")) + require.NoError(t, err) + assert.Contains(t, string(base), "name: shared", "the read-only base object must be untouched") +} + +// TestOverlayAuthors_DeletePatch_SkipsOnPathCollision proves the delete-patch author never +// clobbers an unrelated file that happens to occupy the deterministic patch path: the flush skips +// the delete and leaves the existing file byte-for-byte, rather than overwriting it. +func TestOverlayAuthors_DeletePatch_SkipsOnPathCollision(t *testing.T) { + worktree := newWorktreeForTest(t) + root := worktree.Filesystem.Root() + seedPlacedManifest(t, worktree, "base/kustomization.yaml", "resources:\n - cm.yaml\n") + seedPlacedManifest(t, worktree, "base/cm.yaml", + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: shared\n namespace: podinfo-test\ndata:\n k: v\n") + // An unrelated overlay-local file already occupies the delete patch's deterministic path. + collision := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: keepme\n namespace: podinfo-test\ndata:\n x: y\n" + seedPlacedManifest(t, worktree, "overlays/test/configmap-shared-delete.yaml", collision) + seedPlacedManifest(t, worktree, "overlays/test/kustomization.yaml", + "namespace: podinfo-test\nresources:\n - ../../base\n - configmap-shared-delete.yaml\n") + + del := Event{ + Object: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "v1", "kind": "ConfigMap", + "metadata": map[string]interface{}{"name": "shared", "namespace": "podinfo-test"}}}, + Identifier: types.NewResourceIdentifier("", "v1", "configmaps", "podinfo-test", "shared"), + Operation: "DELETE", + } + w := &BranchWorker{contentWriter: newContentWriter(types.SensitiveResourcePolicy{}), mapper: configMapMapper()} + _, err := w.flushEventsToWorktree(context.Background(), worktree, "overlays/test", []Event{del}, nil) + require.NoError(t, err, "a patch-path collision must be skipped, not error") + + got, err := os.ReadFile(filepath.Join(root, "overlays/test/configmap-shared-delete.yaml")) + require.NoError(t, err) + assert.Equal(t, collision, string(got), "the colliding file must be left byte-for-byte, never overwritten") +} + func newTestWriteBatch(t *testing.T) *writeBatch { t.Helper() writer := newContentWriter(types.SensitiveResourcePolicy{}) diff --git a/internal/git/plan_flush.go b/internal/git/plan_flush.go index 981a2326..27adb2ab 100644 --- a/internal/git/plan_flush.go +++ b/internal/git/plan_flush.go @@ -692,7 +692,7 @@ func (wb *writeBatch) patchExisting( desired.SetNamespace("") } projected, overrideEdits, err := projectThroughKustomize( - manifestreport.Project(desired), buf.current, idx, dm) + manifestreport.Project(desired), buf.current, idx, dm, wb.overlayAuthorKustomization(filePath)) if err != nil { var fidelity *renderFidelityRefusedError if errors.As(err, &fidelity) { @@ -766,6 +766,7 @@ func projectThroughKustomize( content []byte, idx int, dm *manifestanalyzer.DocumentModel, + authorInto string, ) (*unstructured.Unstructured, []manifestanalyzer.OverrideEdit, error) { gitRaw, parsed := gitDocRawObject(content, idx) if !parsed { @@ -781,7 +782,23 @@ func projectThroughKustomize( if dm.Rendered == nil { return projected, nil, nil } - return manifestanalyzer.SplitDesiredForOverrides(gitRaw, projected, dm.Rendered) + return manifestanalyzer.SplitDesiredForOverrides(gitRaw, projected, dm.Rendered, authorInto) +} + +// overlayAuthorKustomization is the kustomization the writer may author a NEW images:/replicas: +// entry into for an edit to filePath. It is set only when render-root scoping put filePath OUT of +// the write jail — a base document an overlay reads read-only — and the overlay has a supported +// render root of its own: then a value the base supplies can be overridden by authoring an entry +// in the overlay instead of refusing the base write. It is "" for a self-contained subtree and +// for an in-jail document, where the source file itself is writable. +func (wb *writeBatch) overlayAuthorKustomization(filePath string) string { + if wb.writeSubdir == "" || pathWithin(filePath, wb.writeSubdir) { + return "" + } + if k := wb.store.Kustomizations[wb.writeSubdir]; k != nil && !k.Unsupported { + return k.Path + } + return "" } // renderFidelityRefusedError travels from the projection seam to patchExisting, where the file @@ -973,13 +990,13 @@ func (wb *writeBatch) applyOverrideEdits( if len(edits) == 0 { return false } - byPath := map[string][]manifestedit.KustomizationEdit{} + byPath := map[string][]manifestanalyzer.OverrideEdit{} paths := make([]string, 0, len(edits)) for _, e := range edits { if _, seen := byPath[e.KustomizationPath]; !seen { paths = append(paths, e.KustomizationPath) } - byPath[e.KustomizationPath] = append(byPath[e.KustomizationPath], e.Edit) + byPath[e.KustomizationPath] = append(byPath[e.KustomizationPath], e) } sort.Strings(paths) @@ -989,17 +1006,42 @@ func (wb *writeBatch) applyOverrideEdits( if buf.current == nil { continue // the kustomization vanished within this batch; nothing to edit } - res, diags := manifestedit.PatchKustomization(p, buf.current, byPath[p]) - switch res.Mode { - case manifestedit.EditPatched: - buf.current = res.Content - changed = true - log.FromContext(ctx).Info("Routed live change to kustomization override", - "kustomization", p, "resource", event.Identifier.String(), "edits", len(byPath[p])) - case manifestedit.EditNoChange: - // Another event in this batch already landed the same value. - case manifestedit.EditSkipped, manifestedit.EditWholeReplace, manifestedit.EditDeleted: - logManifestDiagnostics(ctx, diags) + // A skipped or no-change edit leaves buf.current as it is; a patch updates it. Folded + // here so PatchKustomization (edit an existing entry) and AppendKustomizationOverride + // (author a new one) share one place that advances the buffer and logs. + fold := func(res manifestedit.EditResult, diags []manifestedit.Diagnostic, n int, msg string) { + switch res.Mode { + case manifestedit.EditPatched: + buf.current = res.Content + changed = true + log.FromContext(ctx).Info(msg, + "kustomization", p, "resource", event.Identifier.String(), "edits", n) + case manifestedit.EditNoChange: + // Another event in this batch already landed the same value. + case manifestedit.EditSkipped, manifestedit.EditWholeReplace, manifestedit.EditDeleted: + logManifestDiagnostics(ctx, diags) + } + } + + var patches []manifestedit.KustomizationEdit + var creates []manifestanalyzer.OverrideEdit + for _, e := range byPath[p] { + if e.Create { + creates = append(creates, e) + } else { + patches = append(patches, e.Edit) + } + } + if len(patches) > 0 { + res, diags := manifestedit.PatchKustomization(p, buf.current, patches) + fold(res, diags, len(patches), "Routed live change to kustomization override") + } + // Each authored entry is applied on its own (the writer creates one image component or + // replica count at a time), reading buf.current so several land in the same file. + for _, c := range creates { + res, diags := manifestedit.AppendKustomizationOverride( + p, buf.current, c.Edit.Section, c.Edit.EntryName, c.Edit.Field, c.Edit.Value) + fold(res, diags, 1, "Authored kustomization override for a base-supplied value") } } return changed @@ -1081,6 +1123,13 @@ func (wb *writeBatch) applyDelete(ctx context.Context, event Event) { if !ok { return } + // An object the overlay INHERITS from its read-only base cannot be deleted from the base + // (that is out of the write jail and shared by other environments). Author a $patch: delete + // in the overlay instead; the re-render oracle proves the object leaves the render. + if authorInto := wb.overlayAuthorKustomization(target.filePath); authorInto != "" { + wb.authorInheritedDelete(ctx, event, target, authorInto) + return + } wb.intend(manifestanalyzer.WriteIntent{ SourcePath: target.filePath, Kind: target.id.Kind, @@ -1106,6 +1155,78 @@ func (wb *writeBatch) applyDelete(ctx context.Context, event Event) { wb.dropKustomizationResource(ctx, event, target.filePath) } +// authorInheritedDelete removes an object the overlay INHERITS from its read-only base by +// authoring a `$patch: delete` in the overlay, rather than deleting the base document (which is +// out of the write jail and shared by other environments). It writes the patch file inside +// spec.path, names it in the overlay's own patches:, and declares the Removed intent so the +// re-render oracle proves the object leaves the render — a patch that fails to match is refused +// there, and the base is never touched. +func (wb *writeBatch) authorInheritedDelete( + ctx context.Context, + event Event, + target deleteTarget, + authorInto string, +) { + patchName := inheritedDeletePatchName(target.id) + patchPath := cleanSlash(path.Join(path.Dir(authorInto), patchName)) + want := inheritedDeletePatchDocument(target.id) + + // Never clobber unrelated content: if the deterministic patch path is already occupied by + // something that is not this exact delete patch, skip authoring rather than overwrite it. The + // inherited object then stays until the collision is resolved (a human renames the passenger + // file) and the next resync retries. A path already holding our own patch — an idempotent + // resync — matches want and proceeds harmlessly. + patchBuf := wb.buffer(patchPath) + if patchBuf.current != nil && !bytes.Equal(patchBuf.current, want) { + log.FromContext(ctx).Info( + "Skipping inherited-object delete: patch path already holds different content", + "patch", patchPath, "resource", event.Identifier.String()) + return + } + patchBuf.current = want + + buf := wb.buffer(authorInto) + res, diags := manifestedit.AppendKustomizationPatch(authorInto, buf.current, patchName) + switch res.Mode { + case manifestedit.EditPatched: + buf.current = res.Content + log.FromContext(ctx).Info("Authored $patch: delete for an inherited object", + "kustomization", authorInto, "patch", patchName, "resource", event.Identifier.String()) + case manifestedit.EditNoChange: + case manifestedit.EditSkipped, manifestedit.EditWholeReplace, manifestedit.EditDeleted: + logManifestDiagnostics(ctx, diags) + } + wb.intend(manifestanalyzer.WriteIntent{ + SourcePath: target.filePath, + Kind: target.id.Kind, + Name: target.id.Name, + Removed: true, + }) + wb.putToKustomize = true +} + +// inheritedDeletePatchName is the deterministic file name for an inherited-object delete patch: +// kind and name are DNS-safe, and the overlay resolves one namespace, so kind+name is unique in +// its render. +func inheritedDeletePatchName(id manifestedit.Identity) string { + return strings.ToLower(id.Kind) + "-" + id.Name + "-delete.yaml" +} + +// inheritedDeletePatchDocument is the strategic-merge `$patch: delete` document that removes the +// object from the overlay's render. kustomize matches a strategic-merge patch by full identity — +// apiVersion/kind/namespace/name — so the patch pins the object's namespace when it has one +// (a base that declares the namespace, or the live object's namespace). If the proposed patch +// does not match the render, the oracle refuses the flush; nothing is committed on a bad match. +func inheritedDeletePatchDocument(id manifestedit.Identity) []byte { + meta := " name: " + id.Name + "\n" + if id.Namespace != "" { + meta = " namespace: " + id.Namespace + "\n" + meta + } + return []byte(fmt.Sprintf( + "apiVersion: %s\nkind: %s\nmetadata:\n%s$patch: delete\n", + id.APIVersion, id.Kind, meta)) +} + // dropKustomizationResource removes the resources: entry naming a file this flush deleted, // from every supported kustomization that lists it. // diff --git a/internal/manifestanalyzer/kustomize_render_test.go b/internal/manifestanalyzer/kustomize_render_test.go index 321ac91f..599a48ca 100644 --- a/internal/manifestanalyzer/kustomize_render_test.go +++ b/internal/manifestanalyzer/kustomize_render_test.go @@ -86,7 +86,7 @@ func assertInSyncIsANoOp( t.Helper() where := ro.OriginPath + " " + ro.Object.GetKind() + "/" + ro.Object.GetName() - out, edits, err := SplitDesiredForOverrides(src.Object, asLiveObject(t, ro.Object), attribution) + out, edits, err := SplitDesiredForOverrides(src.Object, asLiveObject(t, ro.Object), attribution, "") require.NoError(t, err, "%s: an in-sync folder needs no edit placed at all", where) require.Empty(t, edits, diff --git a/internal/manifestanalyzer/overrides_projection.go b/internal/manifestanalyzer/overrides_projection.go index 71db7e27..272ad80d 100644 --- a/internal/manifestanalyzer/overrides_projection.go +++ b/internal/manifestanalyzer/overrides_projection.go @@ -22,12 +22,18 @@ import ( // where the value lives". See // docs/design/support-boundary/finished/images-and-replicas-edit-through.md. -// OverrideEdit routes one live-value change to a field of an existing -// kustomization override entry. +// OverrideEdit routes one live-value change to a field of a kustomization override entry. type OverrideEdit struct { // KustomizationPath is the kustomization file (slash, relative to the - // GitTarget subtree) declaring the entry. + // GitTarget subtree) that carries the entry. KustomizationPath string + // Create AUTHORS a new entry (via AppendKustomizationOverride) rather than editing a + // scalar on an existing one: an overlay overriding a value its base supplies, where no + // entry exists yet. Edit.EntryName is the new entry's name: (the image name for an + // images: entry, the resource name for a replicas: entry); Edit.EntryIndex is unused. + // Every authored entry is put to the re-render oracle before it can commit, so a proposal + // that over-reaches is refused there, not written. + Create bool // Edit is the bounded scalar update the manifestedit editor applies. Edit manifestedit.KustomizationEdit } @@ -63,10 +69,18 @@ type OverrideEdit struct { // 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. +// authorInto is the kustomization the writer may AUTHOR a new images:/replicas: entry into when +// a value the SOURCE document supplies diverges in live and the source is out of the write jail +// (a base an overlay reads read-only). It is "" for a self-contained subtree and for an in-jail +// document, where a source-supplied change is written into the file directly. When set, a +// diverging source-supplied image component or replica count becomes a proposed new entry rather +// than a refused base write — the "edit a specific environment, get the override authored" +// capability of docs/design/support-boundary/render-root-scoping.md §4. func SplitDesiredForOverrides( gitRaw map[string]interface{}, desired *unstructured.Unstructured, rendered *RenderedOverrides, + authorInto string, ) (*unstructured.Unstructured, []OverrideEdit, error) { if rendered == nil || desired == nil || gitRaw == nil { return desired, nil, nil @@ -76,8 +90,8 @@ func SplitDesiredForOverrides( return nil, nil, err } out := &unstructured.Unstructured{Object: source} - edits := projectImages(gitRaw, desired, out, rendered.Images) - edits = append(edits, projectReplicas(gitRaw, desired, out, rendered.Replicas)...) + edits := projectImages(gitRaw, desired, out, rendered.Images, authorInto) + edits = append(edits, projectReplicas(gitRaw, desired, out, rendered.Replicas, authorInto)...) return out, edits, nil } @@ -252,6 +266,7 @@ func projectImages( gitRaw map[string]interface{}, live, out *unstructured.Unstructured, rendered map[string]RenderedImage, + authorInto string, ) []OverrideEdit { if len(rendered) == 0 { return nil @@ -276,7 +291,7 @@ func projectImages( if !inGit || !isRendered || !inSource { continue // a new container writes through; the supplier rule converges it later } - plan, routable := invertImage(slot, src, render) + plan, routable := invertImage(slot, src, render, authorInto) if !routable { return nil // one unroutable slot abandons routing for the whole object } @@ -298,7 +313,7 @@ func projectImages( // // 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) { +func invertImage(slot imageSlot, src string, render RenderedImage, authorInto string) (slotPlan, bool) { srcRef := parseImageRef(src) rendered := parseImageRef(render.Rendered) live := parseImageRef(slot.image) @@ -321,11 +336,30 @@ func invertImage(slot imageSlot, src string, render RenderedImage) (slotPlan, bo }, }) } + // author writes a NEW images: entry into the overlay kustomization, matching the source + // image by name. It fires only where the source document supplies the component AND an + // overlay is available to author into — otherwise the change goes into the source file. + author := func(field, value string) { + plan.edits = append(plan.edits, OverrideEdit{ + KustomizationPath: authorInto, + Create: true, + Edit: manifestedit.KustomizationEdit{ + Section: manifestedit.KustomizationSectionImages, + EntryName: srcRef.name, + Field: field, + Value: value, + }, + }) + } + authorTag := authorFor(authorInto != "", author, fieldNewTag) + authorDigest := authorFor(authorInto != "", author, fieldDigest) if live.name != rendered.name { switch { case render.Name != nil: route(render.Name, fieldNewName, live.name) + case authorInto != "": + author(fieldNewName, live.name) default: newSrc.name = live.name } @@ -333,14 +367,14 @@ func invertImage(slot imageSlot, src string, render RenderedImage) (slotPlan, bo if live.tag != rendered.tag { if !routeComponent(render.Tag, render.Digest, live.tag, func(v string) { newSrc.tag = v }, - func(e *ImageOverride, v string) { route(e, fieldNewTag, v) }) { + func(e *ImageOverride, v string) { route(e, fieldNewTag, v) }, authorTag) { return plan, false } } if live.digest != rendered.digest { if !routeComponent(render.Digest, render.Tag, live.digest, func(v string) { newSrc.digest = v }, - func(e *ImageOverride, v string) { route(e, fieldDigest, v) }) { + func(e *ImageOverride, v string) { route(e, fieldDigest, v) }, authorDigest) { return plan, false } } @@ -348,6 +382,15 @@ func invertImage(slot imageSlot, src string, render RenderedImage) (slotPlan, bo return plan, true } +// authorFor returns a component author (the routeComponent "no entry, but an overlay is +// available" hook) for one image field, or nil when there is nowhere to author into. +func authorFor(enabled bool, author func(field, value string), field string) func(string) { + if !enabled { + return nil + } + return func(value string) { author(field, value) } +} + // 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. // @@ -370,12 +413,17 @@ func invertImage(slot imageSlot, src string, render RenderedImage) (slotPlan, bo // entry that sets one; // - a change to a component the SIBLING entry clears — nowhere to land, and the file would // be overridden straight back. +// +// author is the overlay hook: when the source document supplies the component and an overlay is +// available to author into, the change becomes a new images: entry instead of a source write. +// It is nil for a self-contained subtree and for an in-jail source, where the file is writable. func routeComponent( supplier *ImageOverride, sibling *ImageOverride, live string, setSource func(string), route func(*ImageOverride, string), + author func(string), ) bool { switch { case supplier != nil && live == "": @@ -384,6 +432,8 @@ func routeComponent( route(supplier, live) case sibling != nil: return false // the sibling component's entry clears this one; the file cannot own it + case author != nil && live != "": + author(live) // no entry supplies it, but an overlay can author one over the base default: setSource(live) // the source document supplies it; the change flows into the file } @@ -446,15 +496,36 @@ func projectReplicas( gitRaw map[string]interface{}, live, out *unstructured.Unstructured, rendered *RenderedReplicas, + authorInto string, ) []OverrideEdit { - if rendered == nil || rendered.Entry == nil { - return nil // no entry supplies the count; a scale flows into the source document + if rendered == nil { + return nil // the object renders no spec.replicas; nothing an entry governs } liveCount, liveHas, err := unstructured.NestedInt64(live.Object, "spec", "replicas") if err != nil || !liveHas { return nil } + if rendered.Entry == nil { + // No entry supplies the count. If the source is out of the write jail (a base) and an + // overlay is available, author a new replicas: entry over it; otherwise the scale flows + // into the source document. + if authorInto == "" || liveCount == rendered.Rendered { + return nil + } + restoreSourceReplicas(gitRaw, out) + return []OverrideEdit{{ + KustomizationPath: authorInto, + Create: true, + Edit: manifestedit.KustomizationEdit{ + Section: manifestedit.KustomizationSectionReplicas, + EntryName: live.GetName(), + Field: fieldCount, + Value: strconv.FormatInt(liveCount, 10), + }, + }} + } + restoreSourceReplicas(gitRaw, out) if liveCount == rendered.Rendered { return nil // the folder already renders to live diff --git a/internal/manifestanalyzer/overrides_projection_test.go b/internal/manifestanalyzer/overrides_projection_test.go index e8cba0d1..a1971545 100644 --- a/internal/manifestanalyzer/overrides_projection_test.go +++ b/internal/manifestanalyzer/overrides_projection_test.go @@ -49,7 +49,7 @@ func splitFixture( var gitRaw map[string]interface{} require.NoError(t, yaml.Unmarshal(contentOf(t, files, sourcePath), &gitRaw)) - out, edits, err := SplitDesiredForOverrides(gitRaw, desired, assignment.rendered) + out, edits, err := SplitDesiredForOverrides(gitRaw, desired, assignment.rendered, "") require.NoError(t, err, "the projection must be able to place the edit") return out, edits } diff --git a/internal/manifestanalyzer/source_form_test.go b/internal/manifestanalyzer/source_form_test.go index 24ec5d48..78f11009 100644 --- a/internal/manifestanalyzer/source_form_test.go +++ b/internal/manifestanalyzer/source_form_test.go @@ -202,7 +202,7 @@ spec: chains, _ := renderChains(files, parseKustomizations(files)) rendered := chains[chainKey{originPath: sourceDoc, kind: "Deployment", name: "web"}].rendered - _, _, err := SplitDesiredForOverrides(gitRaw, &unstructured.Unstructured{Object: live}, rendered) + _, _, err := SplitDesiredForOverrides(gitRaw, &unstructured.Unstructured{Object: live}, rendered, "") var refused *SourceFormRefusedError require.ErrorAs(t, err, &refused)