fix(placement): register a new file with its nearest ancestor kustomization - #319
Merged
Merged
Conversation
…zation
A declared path into a subdirectory of a kustomize folder produced a document
that no kustomization lists, so `kubectl apply -k` never rendered it: one
byType line was enough to put a file in Git that nothing applies.
governingKustomization looked only in the file's own directory, plus a special
case for the write scope's root, so registration depended on whether
render-root scoping happened to be in force — a difference the user cannot see.
It now walks up from the file's own directory to the nearest kustomization,
bounded by the write jail, because a kustomization above the write scope is a
read-only base whose resources: list is not ours to edit. That also reaches
namespaceIsInheritedFromContext, which was unreachable for these paths: the
document used to carry a metadata.namespace line the folder's own documents
omit.
Drop the {version} requirement from IdentityCompletePlacementTemplate. The
built-in canonical path is deliberately versionless, because two served
versions of one group/resource are the same object, so a version segment
separates no identities. Requiring it judged the canonical shape as not
identity-complete, which is what would make any spec-level default fail our own
validation gate.
Fixes #295
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change fixes declared placement registration in kustomize subdirectories and updates identity validation for the versionless canonical template. Documentation and regression tests cover ancestor registration, metrics, rendered content, and validation. ChangesPlacement corrections
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to When creating a new manifest, a failure to update its governing kustomization can still commit the file without rendering it, and later updates may not repair that registration; the change is otherwise confined to the writable subtree, so it is mergeable with explicit owner awareness or follow-up. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Docstring CoverageExplanation Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 5 files. (1 skipped: 1 unsupported.) Full details: Description checkExplanation The description explains the defects, implementation changes, linked issue, acceptance criteria, regression tests, and local test results. It does not reproduce every template section, but it provides the required review context.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
sunib
added a commit
that referenced
this pull request
Aug 28, 2026
…t express Reverses this document's own thesis. It argued that a path template is the wrong primitive and proposed replacing spec.placement with a spec.layout discriminated union. That argument rested on five points, and three of them were retired by #319: a new file is now registered with the nearest kustomization that governs it, whatever chose its path. Once the template is no longer asked to express "beside this folder's one kustomization", the discriminator has nothing left to discriminate — registration was always the best idea in the model, and it is the part that already shipped. So the template stays and two additive fields join it, each with a default equal to today's behavior. serializeNamespace (Auto, Always, Never) is the one thing a path genuinely cannot express: kustomize takes metadata.namespace from either the document or a governing root, and where the file sits decides neither. It is not called writeNamespace because "write" is this API's most loaded word — mode: Write, the write boundary, the write jail, WriteBoundaryRefused — so writeNamespace: Never invites the reading "never write to this namespace", a permission, which is exactly what the neighbouring sourceNamespace fields are. kustomizeRoot (Adopt, Create, Require) answers "do we want kustomize" on one axis: what to do when no kustomization governs the path. When one does, every value registers, because that is the invariant. Adopt is today's behavior, Create is the empty-repository bootstrap that was the last surviving argument from the old thesis, and Require is the safety value the Never guard needs — if the root disappears, stop writing rather than commit files nothing renders. Four kustomize facts are measured rather than recalled, and three contradict assumptions the earlier model was built on: a root does not require a flat folder, nested roots work one per subfolder and supply their own namespace, there is no ambient pickup (globs and bare directories both fail), and an unlisted file in a listed subdirectory renders nothing. The first is why the path may be anything; the third is why registration must be an invariant. Two values are considered and not taken, with reasons. Ignore is rejected: spec.path already expresses it, since the ancestor walk is bounded by the write jail, and it is the only candidate that changes what happens when a root IS present — the half that should be invariant. CreatePerDirectory is deferred with its trigger recorded, because fact 2 proves it would work and it is what would make Never safe in a multi-namespace tree. Also takes the maintainer review's status findings, since they are decisions this document owes: renderRootReason becomes a condition reason rather than a bespoke field, the accumulating counters go to metrics where placements_total already carries them, and conditions plus observedGeneration are shown. The headline is what this deletes: spec.layout, kind, scope, kustomize.create, the LayoutProfile question, the immutability machinery, and the migration. The layout model was the largest breaking change in the queue; on this shape it is not a breaking change at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sunib
added a commit
that referenced
this pull request
Aug 28, 2026
The plan still described the #295 correctness fixes as PR 1's first half and called PR 1 "this PR". Both fixes are on main and released in 0.42.1 via #319: the ancestor walk and the versionless identity gate. PR 1 is the corpus alone now, and the shipped fixes become the first thing it asserts, which makes the corpus regression cover from its first commit rather than scaffolding for later PRs. Worth stating plainly that the ancestor walk is also why this plan reversed. The visibility page kept two claims the reversal invalidated. It said the CRD default for placement.default was superseded because layout.kind is the defaultable thing; there is no layout.kind, the template stays, and the question is live again — with a weaker objection than before, because a defaulted path into a subdirectory is now registered with the kustomization that governs it. And status.layout is status.placement since spec.layout stopped existing, so the section says so once at the top rather than being renamed throughout, since this page is still where the field is argued. Also fixes a Go comment pointing at docs/design/manifest/version2/, a path that has not existed for some time and that the folder move only half-corrected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 28, 2026
sunib
added a commit
that referenced
this pull request
Aug 28, 2026
#293 is closed with the evidence that reversed it, and its successor is #322: serializeNamespace and kustomizeRoot as additive fields, which is why the row leaves the wave. The Tier 1 placement-correctness row shipped in 0.42.1 via #319, and B2 carries the status.placement rename. Co-Authored-By: Claude Opus 5 <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.
Fixes #295. Split out of #318 so the fix can ship on its own — #318 stays open for the layout docs and the maintainer-review feedback.
What was wrong
A declared path into a subdirectory of a kustomize folder produced a document that no
kustomization.yamllists, sokubectl apply -knever rendered it. OnebyTypeline was enough to put a file in Git that nothing applies:governingKustomizationlooked only in the file's own directory, plus a special case for the write scope's root, so registration depended on whether render-root scoping happened to be in force — a difference the user cannot see.What changed
resources:list is not ours to edit. That also reachesnamespaceIsInheritedFromContext, which was unreachable for these paths: the document used to carry ametadata.namespaceline the folder's own documents omit.IdentityCompletePlacementTemplateno longer requires{version}. The built-in canonical path is deliberately versionless, because two served versions of one group/resource are the same object, so a version segment separates no identities. Requiring it judged the canonical shape as not identity-complete, which is what would make any spec-level default fail our own validation gate.Acceptance
byTypepath into a subdirectory of a kustomize folder is registered with the root that governs it, with a test that fails before the change.placement_kustomization_entries_totalno longer counts these asfailed.IdentityCompletePlacementTemplateaccepts the versionless canonical shape; the Secret-safety gate still rejects what it should.task lint,task testandtask test-e2eall pass locally (e2e: 80 passed, 0 failed, 23 skipped opt-in corners).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes