Skip to content

feat: add 'cat' command for displaying package resources#4505

Open
aravindtga wants to merge 4 commits intokptdev:mainfrom
Nordix:add-kpt-pkg-cat
Open

feat: add 'cat' command for displaying package resources#4505
aravindtga wants to merge 4 commits intokptdev:mainfrom
Nordix:add-kpt-pkg-cat

Conversation

@aravindtga
Copy link
Copy Markdown
Contributor

Restores the cat subcommand under pkg that was removed in #2393 (#2393), with small correctness and testability fixes on top.

Changes

  • Restore thirdparty/cmdconfig/commands/cmdcat/cmdcat.go and wire it into pkg.
  • Non-YAML/JSON file args (e.g. README.md, Kptfile) now return a clear error and non-zero exit instead of warning + silently succeeding. Directory form is unchanged; Kptfile stays excluded from the resource stream.

Test

  • go build, go vet, and go test pass for thirdparty/cmdconfig/commands/cmdcat/...
  • Manually verified the commands in the CLI

AI usage disclosure

  • AI assistance (Kiro CLI) was used to draft portions of this change, including exploratory edits, test scaffolding, and this PR description.

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 27, 2026

Deploy Preview for kptdocs ready!

Name Link
🔨 Latest commit 145d498
🔍 Latest deploy log https://app.netlify.com/projects/kptdocs/deploys/69f304d5f21c070008c3d1b9
😎 Deploy Preview https://deploy-preview-4505--kptdocs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@aravindtga aravindtga marked this pull request as ready for review April 27, 2026 11:30
Copilot AI review requested due to automatic review settings April 27, 2026 11:30
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request go Pull requests that update Go code labels Apr 27, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Restores the kpt pkg cat subcommand (previously removed in #2393) to print package resources, with additional correctness behavior around rejecting non-resource file arguments and improved test coverage for edge cases.

Changes:

  • Reintroduce thirdparty/cmdconfig/commands/cmdcat implementation and hook it into kpt pkg.
  • Add comprehensive unit tests covering directory traversal, nested packages, annotation behavior, formatting/comment stripping, and error cases.
  • Change behavior for non-YAML/JSON file arguments to return an error and non-zero exit.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
thirdparty/cmdconfig/commands/cmdcat/cmdcat.go Restores the cat command implementation and adds stricter file-argument validation and contextual error diagnostics.
thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go Adds tests covering core output behavior and multiple historical issues/edge cases.
commands/pkg/pkgcmd.go Wires the restored cat command into the pkg command group.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread thirdparty/cmdconfig/commands/cmdcat/cmdcat.go
Comment thread thirdparty/cmdconfig/commands/cmdcat/cmdcat.go
Comment thread thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go Outdated
Comment thread thirdparty/cmdconfig/commands/cmdcat/cmdcat.go Outdated
@dosubot dosubot Bot added the lgtm label Apr 27, 2026
Comment thread commands/pkg/pkgcmd.go Outdated
Comment thread thirdparty/cmdconfig/commands/cmdcat/cmdcat.go Outdated
Comment thread thirdparty/cmdconfig/commands/cmdcat/cmdcat.go Outdated
Comment thread thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go Outdated
Comment thread thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go
Copilot AI review requested due to automatic review settings April 29, 2026 18:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Restores the kpt pkg cat subcommand (previously removed in #2393) to print KRM resources from a file or package directory, along with a focused test suite to validate output formatting, annotation behavior, recursion behavior, and error cases.

Changes:

  • Reintroduce cmdcat Cobra command implementation and package traversal wiring.
  • Add comprehensive unit tests covering directory/file inputs, annotation behavior, recursion, and failure modes.
  • Wire pkg cat into the kpt pkg command group.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
thirdparty/cmdconfig/commands/cmdcat/cmdcat.go Implements the restored pkg cat command, including flag handling, package traversal, and output formatting.
thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go Adds test coverage for core behaviors and key regressions/edge cases.
commands/pkg/pkgcmd.go Registers pkg cat under the existing kpt pkg command group.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread thirdparty/cmdconfig/commands/cmdcat/cmdcat.go Outdated
Comment thread thirdparty/cmdconfig/commands/cmdcat/cmdcat.go Outdated
Signed-off-by: Aravindhan Ayyanathan <aravindhan.a@est.tech>
Signed-off-by: Aravindhan Ayyanathan <aravindhan.a@est.tech>
Signed-off-by: Aravindhan Ayyanathan <aravindhan.a@est.tech>
Signed-off-by: Aravindhan Ayyanathan <aravindhan.a@est.tech>
@@ -0,0 +1,162 @@
// Copyright 2019,2026 The Kubernetes Authors.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copyright header?

case ".yaml", ".yml", ".json":
default:
if filepath.Base(args[0]) == kptfilev1.KptFileName {
return fmt.Errorf("%q is a %s package metadata file, not a resource file; no resources will be read", args[0], kptfilev1.KptFileName)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: "No resouces will be printed?" instead of "read"

if filepath.Base(args[0]) == kptfilev1.KptFileName {
return fmt.Errorf("%q is a %s package metadata file, not a resource file; no resources will be read", args[0], kptfilev1.KptFileName)
}
return fmt.Errorf("%q is not a YAML/JSON file; no resources will be read", args[0])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Primted? Similar to the previous comment

outStr := out.String()
fmt.Fprint(w, outStr)
if outStr != "" {
fmt.Fprint(w, "---")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this added? It seems at #100 it's trimmed unconditionally


const f1Yaml = "f1.yaml"

// writeFile is a small helper to keep the test bodies terse.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Learned a new word :)


// TestCmd_NestedPackages exercises a true multi-pkg tree (Kptfile in root and
// in subdir) and asserts each pkg's resource is emitted exactly once.
// This guards Issue 3 (path-clean) and Issue 9 (no double-traverse).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably AI instructions geeting into test descriptions

// Resources in the sub-dir are emitted as part of the root package.
func TestCmd_Subpkgs(t *testing.T) {
d := t.TempDir()
if err := os.MkdirAll(filepath.Join(d, "subpkg"), 0700); err != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Writefile uses 0600, while mkdir uses 0700.

assert.Contains(t, got, "name: sub-cm")
}

// TestCmd_PathCleaning (Issue 3): ./path and path/ must behave like path.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI instruction in test description

Args: cobra.MaximumNArgs(1),
}
c.Flags().BoolVar(&r.Format, "format", true,
"format resource config yaml before printing.")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "format" mean? Tje last testcase makes it a bit confusing :(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a testcase to check what happens on cpntext cancels, like sigterm/sigkill

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request go Pull requests that update Go code lgtm size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants