feat(overlay): prove new-object placement; author images/replicas overrides and $patch:delete#244
Merged
Merged
Conversation
…ay; fix stale docs render-root-scoping.md §4 labelled "new object -> overlay-local file plus resources: entry" as Planned, but #239's render-root scoping already wired it: WriteScope threads the overlay's write jail into placement, the base kustomization is skipped as out-of-jail (writableCohort / resolveKustomizeRoot), appendKustomizationResource adds the resources: entry in the same commit, and the render oracle verifies the result. It simply had no test proving it and the docs still read "Planned". Add TestPlacement_ExternalBaseOverlay_NewObject: a GitTarget rooted at an overlay that reads ../../base gains a new ConfigMap. It asserts the file lands inside the overlay, the overlay's OWN kustomization gains the resources: entry (never the base's), the read-only base is untouched byte-for-byte, and the render oracle accepts the flush. Correct the stale docs (render-root-scoping §1/§4/§5/top, README, support-contract, kustomize-support-boundary) to mark new-object creation shipped. What remains for an overlay: adding a MISSING images:/replicas: declaration (no source value to attribute yet) and base-owned-field patch authoring. No production code changed — this proves and documents existing behaviour. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughExternal-base overlays now write overlay-local resources, ChangesExternal-base overlay authoring
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LiveEvent
participant BranchWorker
participant OverrideProjection
participant KustomizationEditor
participant Overlay
LiveEvent->>BranchWorker: Submit object update or delete
BranchWorker->>OverrideProjection: Project change for overlay authoring
OverrideProjection->>KustomizationEditor: Create override edit
BranchWorker->>KustomizationEditor: Append override or delete patch
KustomizationEditor->>Overlay: Write overlay-local kustomization artifacts
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…alue is changed per-environment
An external-base overlay could edit an EXISTING images:/replicas: entry, but when the
base itself supplied an image or replica count, changing it in one environment tried to
write the read-only base and was refused ("planned write path base/deployment.yaml
escapes the GitTarget write scope"). So "edit the image in dev" did not work unless the
overlay already declared an entry.
Author the entry instead. When the projection finds a diverging image component or replica
count whose supplier is the SOURCE document and that source is out of the write jail (a base
an overlay reads), it emits a Create OverrideEdit into the overlay's own kustomization rather
than writing the base. The writer applies it with a new
manifestedit.AppendKustomizationOverride primitive (creating the images:/replicas: section if
the overlay has none), and the base source form stays unchanged so nothing is written to the
base.
Every authored entry is put to the existing re-render oracle before it can commit, so a
proposal that over-reaches (an images: name shared by another object) is refused there, not
written. Idempotent on resync: once the entry exists the store sees it and the change routes
to it, no duplicate.
Tests: manifestedit primitive unit tests; overrides projection call-site updates; and writer
integration TestOverlayAuthors_{ImageEntry,ReplicaEntry,Idempotent}. Docs updated
(render-root-scoping, support-contract, README, kustomize-support-boundary).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deleting an object an external-base overlay inherits from its base tried to delete the base document (out of the write jail, shared by other environments) and was refused. Author a `$patch: delete` in the overlay instead: when the delete target is out of the write jail and the overlay has a supported render root, applyDelete writes a small `$patch: delete` document inside spec.path, names it in the overlay's own patches: (new manifestedit.AppendKustomizationPatch primitive, creating the patches: section if absent), and keeps the existing Removed intent so the re-render oracle proves the object leaves the render. The base is never touched. The patch pins the object by apiVersion/kind/namespace/name; a patch that does not match makes kustomize fail to build and the oracle refuses the flush (proven: the namespace-less first cut was correctly refused, the namespaced one commits). It needs no field attribution, so it does not wait on strategic-merge field-patch authoring — which stays refused. This is a deliberate, narrow widening of the patch-authoring boundary for the delete slice only; noted in patch-authoring.md and the render-root-scoping/support-contract records. Tests: AppendKustomizationPatch unit tests; integration TestOverlayAuthors_DeletePatch_ForInheritedObject. Coverage baseline bumped 76.4% -> 76.5%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sunib
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/support-boundary/README.md`:
- Around line 42-50: The patch-authoring documentation must distinguish
read-only field patches from the shipped deletion exception. Update the table
row in docs/design/support-boundary/README.md (lines 42-50) and the
taxonomy/accepted-layout description in
docs/design/support-boundary/kustomize-support-boundary.md (lines 36-41) to
state that field patches remain read-only while $patch: delete may be authored.
In `@internal/git/plan_flush.go`:
- Around line 1170-1175: Update the inherited-delete patch flow around
inheritedDeletePatchName and wb.buffer(patchPath) so an existing patch-path
collision is detected before assignment. Reuse the existing content when it
matches the generated patch, otherwise choose a conflict-free patch path or
return a clear error; never overwrite existing content, and propagate Git or
manifest errors gracefully.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b614dc3e-6910-4bd0-8832-3d6b3a98c776
📒 Files selected for processing (14)
.coverage-baselinedocs/design/support-boundary/README.mddocs/design/support-boundary/kustomize-support-boundary.mddocs/design/support-boundary/patch-authoring.mddocs/design/support-boundary/render-root-scoping.mddocs/design/support-boundary/support-contract.mdinternal/git/manifestedit/kustomization.gointernal/git/manifestedit/kustomization_test.gointernal/git/placement_test.gointernal/git/plan_flush.gointernal/manifestanalyzer/kustomize_render_test.gointernal/manifestanalyzer/overrides_projection.gointernal/manifestanalyzer/overrides_projection_test.gointernal/manifestanalyzer/source_form_test.go
…ress CodeRabbit - Resolve README / render-root-scoping conflicts between #243 (scan-repo adopts external-base overlays) and this branch (overlay write-path): keep both facts — scan-repo reports kustomize-overlay accepted, and the overlay authors new objects, images/replicas entries, and $patch: delete. - CodeRabbit #1: distinguish read-only field patches from the authored $patch: delete in README and kustomize-support-boundary. - CodeRabbit #2: authorInheritedDelete never overwrites an unrelated file at the deterministic patch path — it skips the delete on a content collision (idempotent resync still proceeds). Covered by TestOverlayAuthors_DeletePatch_SkipsOnPathCollision. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overlay write-path work on the render-root-scoping boundary. Three commits, each verified by the existing re-render oracle (a wrong proposal is refused, never committed) and by writer-integration tests.
1. Prove new-object placement (no production code)
§4 listed "new object → overlay file +
resources:entry" as Planned, but #239 already wired it.TestPlacement_ExternalBaseOverlay_NewObjectproves it end-to-end (file lands in the overlay, overlay's own kustomization gains the entry, base untouched, oracle accepts). Stale docs corrected.2. Author a missing
images:/replicas:entry — "edit a specific environment, get the override"Changing a base-supplied image or replica count in one overlay used to try to write the read-only base and was refused. Now the projection, on a diverging component whose supplier is the source document and whose source is out of the write jail, emits a Create
OverrideEditinto the overlay's own kustomization. New primitivemanifestedit.AppendKustomizationOverridecreates the entry (and theimages:/replicas:section if absent); the base source form stays byte-identical so nothing is written to the base. Idempotent on resync.TestOverlayAuthors_ImageEntry_ForBaseSuppliedImage,..._ReplicaEntry_ForBaseSuppliedCount,..._Idempotent_OnResync3.
$patch: deletefor an inherited objectDeleting an object the overlay inherits from its base used to try to delete the base doc (refused). Now
applyDeleteauthors a$patch: deletedocument under the overlay + apatches:entry (new primitiveAppendKustomizationPatch), keeps theRemovedintent so the oracle proves the object leaves the render, and never touches the base. The patch pins apiVersion/kind/namespace/name — a non-matching patch is refused by the oracle (proven: the namespace-less first cut was refused, the namespaced one commits).TestOverlayAuthors_DeletePatch_ForInheritedObjectNote:
$patch: deleteis a deliberate, narrow widening of the patch-authoring boundary for the delete slice only (identified by name, no field attribution). Strategic-merge patch authoring for a base-owned field remains planned. Recorded in patch-authoring.md, render-root-scoping.md, support-contract.md.Validation
task lint— doccheck OK, golangci-lint 0 issuestask test— all unit + writer-integration tests pass; coverage 76.3% → 76.5% (baseline bumped)task test-e2e— running (both features touch the write path)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests