Repository navigation
Conversation
✅ Deploy Preview for kptdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
🟡 Changes recommended
The newly successful path can corrupt empty sequence patches during attribution.
1 open finding
What changed in this PR
Adds local namespace detection so kpt live init accepts packages containing bare YAML sequences.
Changes:
- Reads packages with bare-sequence wrapping enabled.
- Skips bare sequences during namespace inference.
- Adds namespace and command-level regression tests.
| File | Description |
|---|---|
commands/live/init/cmdliveinit.go |
Implements bare-sequence-aware namespace detection. |
commands/live/init/cmdliveinit_test.go |
Tests namespace inference with non-KRM YAML. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| return "", err | ||
| } | ||
| if enforceNamespace { | ||
| klog.V(6).Infof("enforcing namespace: %s", namespace) |
There was a problem hiding this comment.
Let's try to avoid adding new klog logs, I think we want to eliminate klog from this repo and use slog instead.
There was a problem hiding this comment.
Removed in c7e1def. They were debug traces copied from cli-utils' FindNamespace, so I dropped them instead of moving them to slog.
|
Thanks @liamfallon! The |
|
this PR to me seems identical in nature to #4812.
all the .krmignore based logic in all kpt commands are being restructured by myself at the moment as discussed in #4790 to have identical functionality in all places and operate as intended. |
|
Thanks @Catalin-Stratulat-Ericsson! I think they're a bit different. #4812 makes the error clearer, but |
|
ok i might have misread it at a glance @Nikshay1 apologies for that on my end for the overreaction. the init in this scenario should have a check before operating if resources are non krm like how fn render does. Especially since it already checks if there is anything in the ".krmignore" that it should exclude from its resource list. im not in disagreement that this fix needs to go in but yes if we could please wait a little before merging this in it would save me a few headaches :D ive seen quite a few PR's in the last week touching the .krmignore stuff. i could write an issue to clarify what i plan on changing for clarity. but thank you very much for your flexibility. i should have the PR up in a day or 2 max. id love your review on it actually to make sure its properly solving the underlying problems as you are working around similar issues with this PR :) |
|
Hehe, no worries at all! I'll move this to draft until your PR is in, and I'd be glad to review it when it's ready :P. |
|
See also #2445 |
oh nice, hadn’t seen that one. makes sense, I guess that’s partly why the patch files end up in packages, since kpt doesn’t have its own patch function yet. btw, if y’all ever decide to build one i’m in! |
kpt live init failed with 'wrong node kind: expected MappingNode but got SequenceNode' when the package contained a YAML file that is a bare sequence, such as a JSON 6902 patch used by kustomize. Find the namespace with a kpt-local version of cli-utils' config.FindNamespace that reads the package with WrapBareSeqNode and skips bare sequences. Fixes kptdev#4789 Assisted-by: Claude:claude-opus-5-5 Assisted-by: Codex:gpt-6.1-sol Signed-off-by: Nikshay <nikshayyadav90@gmail.com>
Signed-off-by: Nikshay <nikshayyadav90@gmail.com>
c7e1def to
a703d46
Compare
|
rebased onto main, sorry it took a while! one heads-up about #4812. its |
|




Description
kpt live initno longer fails when the package contains a YAML file that is a bare sequence instead of a KRM resource, such as a JSON 6902 patch used by kustomize.The namespace for the inventory object was found with
config.FindNamespacefrom cli-utils. It reads the package with akio.LocalPackageReaderthat does not setWrapBareSeqNode, so a bare sequence makes the read fail:This PR replaces that call with a kpt-local
findNamespaceincommands/live/init/cmdliveinit.go. It is the cli-utils logic with two differences:WrapBareSeqNode: true, as the other readers in kpt do (for examplepkg/live/rgpath.go);With the change, the command above prints:
Everything that worked before behaves the same: the
--namespaceflag still takes precedence, cluster-scoped and local-config resources are still ignored, and the namespace of the kubeconfig context is still the fallback. Theklog.V(6)messages of the cli-utils implementation are kept.Motivation
Kustomize overlays often contain JSON 6902 patch files, and the kustomize example documents running
kpt live initon an overlay directory. The only workaround was to pass--namespace.The cause is in cli-utils, whose
allInSameNamespaceis the same on itsmasterbranch today, so I fixed it in kpt instead of waiting for a cli-utils release.Fixes #4789
Notes for reviewers
values.yaml) never failed, and it still makes the detection fall back to the namespace of the context, as before. There is a test that pins this. I can change it to skip those files too if you prefer that.[]is rewritten asbareSeqNodeWrappingKey: []by the attribution step at the end oflive init. This already happens onmainwhen--namespaceis passed, and it does not happen withKRM_DISABLE_ATTRIBUTION=1. It comes from the attribution step rewriting every file in the package, which is kpt live init shouldn't reformat all yaml files #4791. I plan to send a separate PR for kpt live init shouldn't reformat all yaml files #4791, so I did not change it here.cmdliveinit.goand its test, in different places. I put the newTestCmd_Runcase at the end of the table to avoid a conflict.Testing
TestFindNamespacetable for the namespace logic that kpt now owns, including bare sequence files, a bare sequence document next to a resource in the same file, and a package with only a bare sequence file.TestCmd_Runcase that runs the command on a package with a ConfigMap and a JSON 6902 patch.go test ./commands/live/init/,go vet,gofmt,go mod tidy(no changes) and golangci-lint v2.11.4 on the package all pass. I could not run the tests that need Docker on my machine.AI usage
I have used AI in the creation of this PR.
I have used the following AI tools:
findNamespace,allInSameNamespaceand the tests