From 73e1bca5bf450541e63abcfa28ace02aca845684 Mon Sep 17 00:00:00 2001 From: Aravindhan Ayyanathan Date: Mon, 27 Apr 2026 11:10:11 +0100 Subject: [PATCH 1/4] feat: add 'cat' command for displaying package resources Signed-off-by: Aravindhan Ayyanathan --- commands/pkg/pkgcmd.go | 5 +- .../cmdconfig/commands/cmdcat/cmdcat.go | 161 ++++++ .../cmdconfig/commands/cmdcat/cmdcat_test.go | 509 ++++++++++++++++++ 3 files changed, 673 insertions(+), 2 deletions(-) create mode 100644 thirdparty/cmdconfig/commands/cmdcat/cmdcat.go create mode 100644 thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go diff --git a/commands/pkg/pkgcmd.go b/commands/pkg/pkgcmd.go index 0430c33d65..052b6bcd44 100644 --- a/commands/pkg/pkgcmd.go +++ b/commands/pkg/pkgcmd.go @@ -1,4 +1,4 @@ -// Copyright 2019 The kpt Authors +// Copyright 2019-2026 The kpt Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -22,6 +22,7 @@ import ( initialization "github.com/kptdev/kpt/commands/pkg/init" "github.com/kptdev/kpt/commands/pkg/update" "github.com/kptdev/kpt/internal/docs/generated/pkgdocs" + "github.com/kptdev/kpt/thirdparty/cmdconfig/commands/cmdcat" "github.com/kptdev/kpt/thirdparty/cmdconfig/commands/cmdtree" "github.com/spf13/cobra" ) @@ -47,7 +48,7 @@ func GetCommand(ctx context.Context, name string) *cobra.Command { pkg.AddCommand( get.NewCommand(ctx, name), initialization.NewCommand(ctx, name), update.NewCommand(ctx, name), diff.NewCommand(ctx, name), - cmdtree.NewCommand(ctx, name), + cmdtree.NewCommand(ctx, name), cmdcat.NewCommand(ctx, name), ) return pkg } diff --git a/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go b/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go new file mode 100644 index 0000000000..87b0312b83 --- /dev/null +++ b/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go @@ -0,0 +1,161 @@ +// Copyright 2019-2026 The Kpt Authors. +// SPDX-License-Identifier: Apache-2.0 + +package cmdcat + +import ( + "bytes" + "context" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/kptdev/kpt/internal/docs/generated/pkgdocs" + kptfilev1 "github.com/kptdev/kpt/pkg/api/kptfile/v1" + "github.com/kptdev/kpt/thirdparty/cmdconfig/commands/runner" + "github.com/spf13/cobra" + "sigs.k8s.io/kustomize/kyaml/kio" + "sigs.k8s.io/kustomize/kyaml/kio/filters" + "sigs.k8s.io/kustomize/kyaml/yaml" +) + +// GetCatRunner returns a command CatRunner. +func GetCatRunner(ctx context.Context, _ string) *CatRunner { + r := &CatRunner{ + Ctx: ctx, + errWriter: os.Stderr, + } + c := &cobra.Command{ + Use: "cat [FILE | DIR]", + Short: pkgdocs.CatShort, + Long: pkgdocs.CatLong, + Example: pkgdocs.CatExamples, + RunE: r.runE, + Args: cobra.MaximumNArgs(1), + } + c.Flags().BoolVar(&r.Format, "format", true, + "format resource config yaml before printing.") + c.Flags().BoolVar(&r.KeepAnnotations, "annotate", false, + "annotate resources with their file origins.") + c.Flags().StringSliceVar(&r.Styles, "style", []string{}, + "yaml styles to apply. may be 'TaggedStyle', 'DoubleQuotedStyle', 'LiteralStyle', "+ + "'FoldedStyle', 'FlowStyle'.") + c.Flags().BoolVar(&r.StripComments, "strip-comments", false, + "remove comments from yaml.") + c.Flags().BoolVarP(&r.RecurseSubPackages, "recurse-subpackages", "R", true, + "print resources recursively in all the nested subpackages") + r.Command = c + return r +} + +func NewCommand(ctx context.Context, name string) *cobra.Command { + return GetCatRunner(ctx, name).Command +} + +// CatRunner contains the run function +type CatRunner struct { + Command *cobra.Command + Ctx context.Context + Format bool + KeepAnnotations bool + Styles []string + StripComments bool + RecurseSubPackages bool + errWriter io.Writer +} + +func (r *CatRunner) runE(c *cobra.Command, args []string) error { + var writer = c.OutOrStdout() + if len(args) == 0 { + args = append(args, ".") + } + + if info, err := os.Stat(args[0]); err == nil && !info.IsDir() { + switch strings.ToLower(filepath.Ext(args[0])) { + case ".yaml", ".yml", ".json": + default: + return fmt.Errorf("%q is not a YAML/JSON file; no resources will be read", args[0]) + } + } + + out := &bytes.Buffer{} + e := runner.ExecuteCmdOnPkgs{ + Writer: out, + NeedOpenAPI: false, + RecurseSubPackages: r.RecurseSubPackages, + CmdRunner: r, + RootPkgPath: filepath.Clean(args[0]), + SkipPkgPathPrint: true, + } + + err := e.Execute() + if err != nil { + return err + } + + res := strings.TrimSuffix(out.String(), "---\n") + res = strings.TrimSuffix(res, "---") + fmt.Fprintf(writer, "%s", res) + + return nil +} + +func (r *CatRunner) ExecuteCmd(w io.Writer, pkgPath string) error { + input := kio.LocalPackageReader{PackagePath: pkgPath, PackageFileName: kptfilev1.KptFileName} + out := &bytes.Buffer{} + err := kio.Pipeline{ + Inputs: []kio.Reader{input}, + Filters: r.catFilters(), + Outputs: r.out(out), + }.Execute() + + if err != nil { + // Emit a contextual diagnostic on stderr so the user knows which + // package failed; the runner currently aborts on the first error. + fmt.Fprintf(r.errWriter, "kpt pkg cat: %s in package %q\n", err.Error(), pkgPath) + return err + } + fmt.Fprint(w, out.String()) + if out.String() != "" { + fmt.Fprint(w, "---") + } + return nil +} + +func (r *CatRunner) catFilters() []kio.Filter { + var fltrs []kio.Filter + if r.Format { + fltrs = append(fltrs, filters.FormatFilter{}) + } + if r.StripComments { + fltrs = append(fltrs, filters.StripCommentsFilter{}) + } + return fltrs +} + +func (r *CatRunner) out(w io.Writer) []kio.Writer { + var outputs []kio.Writer + var functionConfig *yaml.RNode + + // remove these annotations explicitly; the ByteWriter won't clear them by + // default because they were set by the LocalPackageReader, not by it. + clear := []string{ + "config.kubernetes.io/path", + "internal.config.kubernetes.io/path", + } + if r.KeepAnnotations { + clear = nil + } + + outputs = append(outputs, kio.ByteWriter{ + Writer: w, + KeepReaderAnnotations: r.KeepAnnotations, + FunctionConfig: functionConfig, + Style: yaml.GetStyle(r.Styles...), + ClearAnnotations: clear, + }) + + return outputs +} diff --git a/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go b/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go new file mode 100644 index 0000000000..05d1f1bee5 --- /dev/null +++ b/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go @@ -0,0 +1,509 @@ +// Copyright 2019-2026 The kpt Authors. +// SPDX-License-Identifier: Apache-2.0 + +package cmdcat + +import ( + "bytes" + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +const f1Yaml = "f1.yaml" + +// writeFile is a small helper to keep the test bodies terse. +func writeFile(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0600); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} + +// runCat invokes `kpt pkg cat` with the given args and captures stdout. +func runCat(t *testing.T, args ...string) (string, error) { + t.Helper() + b := &bytes.Buffer{} + r := GetCatRunner(context.Background(), "") + r.Command.SetArgs(args) + r.Command.SetOut(b) + r.Command.SetErr(&bytes.Buffer{}) + err := r.Command.Execute() + return b.String(), err +} + +// TestCmd_DIR covers the basic directory case with two multi-doc YAML files. +func TestCmd_DIR(t *testing.T) { + d := t.TempDir() + writeFile(t, filepath.Join(d, f1Yaml), ` +kind: Deployment +metadata: + labels: + app: nginx2 + name: foo + annotations: + app: nginx2 +spec: + replicas: 1 +--- +kind: Service +metadata: + name: foo + annotations: + app: nginx +spec: + selector: + app: nginx +`) + writeFile(t, filepath.Join(d, "f2.yaml"), ` +apiVersion: v1 +kind: Abstraction +metadata: + name: foo + configFn: + container: + image: gcr.io/example/reconciler:v1 + annotations: + config.kubernetes.io/local-config: "true" +spec: + replicas: 3 +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: nginx + name: bar + annotations: + app: nginx +spec: + replicas: 3 +`) + + got, err := runCat(t, d) + assert.NoError(t, err) + assert.Equal(t, `kind: Deployment +metadata: + labels: + app: nginx2 + name: foo + annotations: + app: nginx2 +spec: + replicas: 1 +--- +kind: Service +metadata: + name: foo + annotations: + app: nginx +spec: + selector: + app: nginx +--- +apiVersion: v1 +kind: Abstraction +metadata: + name: foo + annotations: + config.kubernetes.io/local-config: "true" + configFn: + container: + image: gcr.io/example/reconciler:v1 +spec: + replicas: 3 +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: bar + labels: + app: nginx + annotations: + app: nginx +spec: + replicas: 3 +`, got) +} + +// TestCmd_File covers the single-file-arg code path. +func TestCmd_File(t *testing.T) { + d := t.TempDir() + writeFile(t, filepath.Join(d, f1Yaml), ` +kind: Deployment +metadata: + labels: + app: nginx2 + name: foo + annotations: + app: nginx2 +spec: + replicas: 1 +`) + + got, err := runCat(t, filepath.Join(d, f1Yaml)) + assert.NoError(t, err) + assert.Equal(t, `kind: Deployment +metadata: + labels: + app: nginx2 + name: foo + annotations: + app: nginx2 +spec: + replicas: 1 +`, got) +} + +// TestCmd_Annotate verifies --annotate emits all four annotations +// (config.*/path, config.*/index, internal.*/path, internal.*/index). +func TestCmd_Annotate(t *testing.T) { + d := t.TempDir() + writeFile(t, filepath.Join(d, f1Yaml), ` +kind: Deployment +metadata: + labels: + app: nginx2 + name: foo + annotations: + app: nginx2 +spec: + replicas: 1 +`) + + got, err := runCat(t, filepath.Join(d, f1Yaml), "--annotate") + assert.NoError(t, err) + assert.Equal(t, `kind: Deployment +metadata: + labels: + app: nginx2 + name: foo + annotations: + app: nginx2 + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'f1.yaml' + internal.config.kubernetes.io/index: '0' + internal.config.kubernetes.io/path: 'f1.yaml' +spec: + replicas: 1 +`, got) +} + +// TestCmd_AnnotateDefaultOff verifies Issue 8: in default mode, neither +// config.kubernetes.io/path nor internal.config.kubernetes.io/path leaks. +func TestCmd_AnnotateDefaultOff(t *testing.T) { + d := t.TempDir() + writeFile(t, filepath.Join(d, f1Yaml), ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: a +`) + + got, err := runCat(t, filepath.Join(d, f1Yaml)) + assert.NoError(t, err) + assert.NotContains(t, got, "config.kubernetes.io/path", + "config.kubernetes.io/path should be cleared by default") + assert.NotContains(t, got, "internal.config.kubernetes.io/path", + "internal.config.kubernetes.io/path should be cleared by default (Issue 8)") +} + +// TestCmd_Subpkgs covers a directory with a regular sub-directory (no Kptfile). +// 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 { + t.Fatal(err) + } + writeFile(t, filepath.Join(d, f1Yaml), ` +kind: Deployment +metadata: + labels: + app: nginx1 + name: foo + annotations: + app: nginx1 +spec: + replicas: 1 +`) + writeFile(t, filepath.Join(d, "subpkg", "f2.yaml"), ` +kind: Deployment +metadata: + labels: + app: nginx2 + name: foo + annotations: + app: nginx2 +spec: + replicas: 1 +`) + + got, err := runCat(t, d) + assert.NoError(t, err) + assert.Equal(t, `kind: Deployment +metadata: + labels: + app: nginx1 + name: foo + annotations: + app: nginx1 +spec: + replicas: 1 +--- +kind: Deployment +metadata: + labels: + app: nginx2 + name: foo + annotations: + app: nginx2 +spec: + replicas: 1 +`, got) +} + +// 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). +func TestCmd_NestedPackages(t *testing.T) { + d := t.TempDir() + writeFile(t, filepath.Join(d, "Kptfile"), `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: root +`) + writeFile(t, filepath.Join(d, "root.yaml"), `apiVersion: v1 +kind: ConfigMap +metadata: + name: root-cm +`) + if err := os.MkdirAll(filepath.Join(d, "sub"), 0700); err != nil { + t.Fatal(err) + } + writeFile(t, filepath.Join(d, "sub", "Kptfile"), `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: sub +`) + writeFile(t, filepath.Join(d, "sub", "sub.yaml"), `apiVersion: v1 +kind: ConfigMap +metadata: + name: sub-cm +`) + + got, err := runCat(t, d) + assert.NoError(t, err) + // exactly one of each resource, separated by a single ---. + assert.Equal(t, 2, strings.Count(got, "\nkind: ConfigMap\n"), + "each pkg should be emitted exactly once") + assert.Equal(t, 1, strings.Count(got, "\n---\n"), + "a single separator between the two packages") + assert.Contains(t, got, "name: root-cm") + assert.Contains(t, got, "name: sub-cm") +} + +// TestCmd_PathCleaning (Issue 3): ./path and path/ must behave like path. +func TestCmd_PathCleaning(t *testing.T) { + d := t.TempDir() + writeFile(t, filepath.Join(d, "Kptfile"), `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: root +`) + writeFile(t, filepath.Join(d, "cm.yaml"), `apiVersion: v1 +kind: ConfigMap +metadata: + name: cm +`) + + for _, arg := range []string{d, d + "/", "./" + filepath.Base(d)} { + t.Run(arg, func(t *testing.T) { + // Run from d's parent so "./" resolves. + oldWd, _ := os.Getwd() + if err := os.Chdir(filepath.Dir(d)); err != nil { + t.Fatal(err) + } + defer os.Chdir(oldWd) + + got, err := runCat(t, arg) + assert.NoError(t, err) + assert.Equal(t, 1, strings.Count(got, "\nkind: ConfigMap\n"), + "arg %q should yield exactly one resource", arg) + }) + } +} + +// TestCmd_TrailingSeparator (Issue 4): a pkg followed by an empty sub-pkg +// must not leave a stray `---\n` at the end of output. +func TestCmd_TrailingSeparator(t *testing.T) { + d := t.TempDir() + writeFile(t, filepath.Join(d, "Kptfile"), `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: root +`) + writeFile(t, filepath.Join(d, "dep.yaml"), `apiVersion: apps/v1 +kind: Deployment +metadata: + name: d +`) + if err := os.MkdirAll(filepath.Join(d, "sub-empty"), 0700); err != nil { + t.Fatal(err) + } + writeFile(t, filepath.Join(d, "sub-empty", "Kptfile"), `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: sub-empty +`) + + got, err := runCat(t, d) + assert.NoError(t, err) + assert.False(t, strings.HasSuffix(got, "---\n") || strings.HasSuffix(got, "---"), + "output must not end with a stray separator; got %q", got) + assert.Equal(t, 0, strings.Count(got, "\n---\n"), + "no inter-pkg separator expected when only one pkg has resources") +} + +// TestCmd_BrokenYAMLReturnsError (Issue 5, narrowed): a broken YAML file +// must cause a non-nil error; the command must not silently succeed. +func TestCmd_BrokenYAMLReturnsError(t *testing.T) { + d := t.TempDir() + writeFile(t, filepath.Join(d, "Kptfile"), `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: root +`) + writeFile(t, filepath.Join(d, "broken.yaml"), `apiVersion: v1 +kind: ConfigMap +metadata: + name: x +data: + a: [unterminated +`) + + _, err := runCat(t, d) + assert.Error(t, err) + assert.Contains(t, err.Error(), "broken.yaml") + + // Stderr must carry the contextual diagnostic. + var errBuf bytes.Buffer + r := GetCatRunner(context.Background(), "") + r.errWriter = &errBuf + r.Command.SetArgs([]string{d}) + r.Command.SetOut(&bytes.Buffer{}) + r.Command.SetErr(&bytes.Buffer{}) + _ = r.Command.Execute() + assert.Contains(t, errBuf.String(), "kpt pkg cat:") + assert.Contains(t, errBuf.String(), "broken.yaml") +} + +// TestCmd_NonExistent verifies a clean error on a missing path. +func TestCmd_NonExistent(t *testing.T) { + d := t.TempDir() + _, err := runCat(t, filepath.Join(d, "nope.yaml")) + assert.Error(t, err) + assert.Contains(t, err.Error(), "no such file or directory") +} + +// TestCmd_KptfileArgRejected: passing the Kptfile directly must error. +// The Kptfile is package metadata, not a resource; the directory form of +// `kpt pkg cat` excludes it from output, so the file form must too. +func TestCmd_KptfileArgRejected(t *testing.T) { + d := t.TempDir() + kpt := filepath.Join(d, "Kptfile") + writeFile(t, kpt, `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: root +`) + _, err := runCat(t, kpt) + assert.Error(t, err) + assert.Contains(t, err.Error(), "not a YAML/JSON file") +} + +// TestCmd_NonYAMLFileErrors: a regular non-YAML/JSON/Kptfile file must +// fail loudly rather than silently succeed with no output. +func TestCmd_NonYAMLFileErrors(t *testing.T) { + d := t.TempDir() + md := filepath.Join(d, "README.md") + writeFile(t, md, "# hi\n") + _, err := runCat(t, md) + assert.Error(t, err) + assert.Contains(t, err.Error(), "not a YAML/JSON file") +} + +// TestCmd_RecurseFalse ensures -R=false limits traversal to the root pkg. +func TestCmd_RecurseFalse(t *testing.T) { + d := t.TempDir() + writeFile(t, filepath.Join(d, "Kptfile"), `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: root +`) + writeFile(t, filepath.Join(d, "root.yaml"), `apiVersion: v1 +kind: ConfigMap +metadata: + name: root-cm +`) + if err := os.MkdirAll(filepath.Join(d, "sub"), 0700); err != nil { + t.Fatal(err) + } + writeFile(t, filepath.Join(d, "sub", "Kptfile"), `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: sub +`) + writeFile(t, filepath.Join(d, "sub", "sub.yaml"), `apiVersion: v1 +kind: ConfigMap +metadata: + name: sub-cm +`) + + got, err := runCat(t, d, "-R=false") + assert.NoError(t, err) + assert.Contains(t, got, "name: root-cm") + assert.NotContains(t, got, "name: sub-cm", + "sub-pkg should not be traversed when -R=false") +} + +// TestCmd_StripComments verifies the --strip-comments flag removes comments. +func TestCmd_StripComments(t *testing.T) { + d := t.TempDir() + writeFile(t, filepath.Join(d, "f.yaml"), `# top comment +apiVersion: v1 +kind: ConfigMap +metadata: + name: c # inline comment +`) + + got, err := runCat(t, filepath.Join(d, "f.yaml"), "--strip-comments") + assert.NoError(t, err) + assert.NotContains(t, got, "top comment") + assert.NotContains(t, got, "inline comment") +} + +// TestCmd_FormatFalse verifies --format=false preserves original map key +// order (Issue 6 — documented as won't-fix, but the flag must work). +func TestCmd_FormatFalse(t *testing.T) { + d := t.TempDir() + writeFile(t, filepath.Join(d, "f.yaml"), `apiVersion: v1 +kind: ConfigMap +metadata: + name: c +data: + z-last: "1" + a-first: "2" +`) + + got, err := runCat(t, filepath.Join(d, "f.yaml"), "--format=false") + assert.NoError(t, err) + zIdx := strings.Index(got, "z-last") + aIdx := strings.Index(got, "a-first") + assert.True(t, zIdx >= 0 && aIdx >= 0, "both keys should be present") + assert.Less(t, zIdx, aIdx, + "with --format=false, original order (z-last before a-first) must be preserved") +} From fcc043407ba4c01bc7420c3ce3654d890dc57b92 Mon Sep 17 00:00:00 2001 From: Aravindhan Ayyanathan Date: Mon, 27 Apr 2026 12:55:47 +0100 Subject: [PATCH 2/4] Address copilot review comments Signed-off-by: Aravindhan Ayyanathan --- .../cmdconfig/commands/cmdcat/cmdcat.go | 13 +++++---- .../cmdconfig/commands/cmdcat/cmdcat_test.go | 27 ++++++++++++++----- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go b/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go index 87b0312b83..4d743c95c5 100644 --- a/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go +++ b/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go @@ -1,4 +1,4 @@ -// Copyright 2019-2026 The Kpt Authors. +// Copyright 2019-2026 The kpt Authors. // SPDX-License-Identifier: Apache-2.0 package cmdcat @@ -14,6 +14,7 @@ import ( "github.com/kptdev/kpt/internal/docs/generated/pkgdocs" kptfilev1 "github.com/kptdev/kpt/pkg/api/kptfile/v1" + "github.com/kptdev/kpt/pkg/printer" "github.com/kptdev/kpt/thirdparty/cmdconfig/commands/runner" "github.com/spf13/cobra" "sigs.k8s.io/kustomize/kyaml/kio" @@ -24,8 +25,7 @@ import ( // GetCatRunner returns a command CatRunner. func GetCatRunner(ctx context.Context, _ string) *CatRunner { r := &CatRunner{ - Ctx: ctx, - errWriter: os.Stderr, + Ctx: ctx, } c := &cobra.Command{ Use: "cat [FILE | DIR]", @@ -63,7 +63,6 @@ type CatRunner struct { Styles []string StripComments bool RecurseSubPackages bool - errWriter io.Writer } func (r *CatRunner) runE(c *cobra.Command, args []string) error { @@ -76,6 +75,9 @@ func (r *CatRunner) runE(c *cobra.Command, args []string) error { switch strings.ToLower(filepath.Ext(args[0])) { 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) + } return fmt.Errorf("%q is not a YAML/JSON file; no resources will be read", args[0]) } } @@ -114,7 +116,8 @@ func (r *CatRunner) ExecuteCmd(w io.Writer, pkgPath string) error { if err != nil { // Emit a contextual diagnostic on stderr so the user knows which // package failed; the runner currently aborts on the first error. - fmt.Fprintf(r.errWriter, "kpt pkg cat: %s in package %q\n", err.Error(), pkgPath) + fmt.Fprintf(printer.FromContextOrDie(r.Ctx).ErrStream(), + "kpt pkg cat: %s in package %q\n", err.Error(), pkgPath) return err } fmt.Fprint(w, out.String()) diff --git a/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go b/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go index 05d1f1bee5..5bab81c632 100644 --- a/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go +++ b/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go @@ -6,11 +6,13 @@ package cmdcat import ( "bytes" "context" + "io" "os" "path/filepath" "strings" "testing" + "github.com/kptdev/kpt/pkg/printer" "github.com/stretchr/testify/assert" ) @@ -28,7 +30,8 @@ func writeFile(t *testing.T, path, content string) { func runCat(t *testing.T, args ...string) (string, error) { t.Helper() b := &bytes.Buffer{} - r := GetCatRunner(context.Background(), "") + ctx := printer.WithContext(context.Background(), printer.New(nil, io.Discard)) + r := GetCatRunner(ctx, "") r.Command.SetArgs(args) r.Command.SetOut(b) r.Command.SetErr(&bytes.Buffer{}) @@ -323,11 +326,18 @@ metadata: for _, arg := range []string{d, d + "/", "./" + filepath.Base(d)} { t.Run(arg, func(t *testing.T) { // Run from d's parent so "./" resolves. - oldWd, _ := os.Getwd() + oldWd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } if err := os.Chdir(filepath.Dir(d)); err != nil { t.Fatal(err) } - defer os.Chdir(oldWd) + defer func() { + if err := os.Chdir(oldWd); err != nil { + t.Fatal(err) + } + }() got, err := runCat(t, arg) assert.NoError(t, err) @@ -389,10 +399,12 @@ data: assert.Error(t, err) assert.Contains(t, err.Error(), "broken.yaml") - // Stderr must carry the contextual diagnostic. + // Stderr must carry the contextual diagnostic. Inject a printer via + // context (mirroring how run.go wires the root cobra err stream) and + // assert on its err stream. var errBuf bytes.Buffer - r := GetCatRunner(context.Background(), "") - r.errWriter = &errBuf + ctx := printer.WithContext(context.Background(), printer.New(nil, &errBuf)) + r := GetCatRunner(ctx, "") r.Command.SetArgs([]string{d}) r.Command.SetOut(&bytes.Buffer{}) r.Command.SetErr(&bytes.Buffer{}) @@ -422,7 +434,8 @@ metadata: `) _, err := runCat(t, kpt) assert.Error(t, err) - assert.Contains(t, err.Error(), "not a YAML/JSON file") + assert.Contains(t, err.Error(), "Kptfile") + assert.Contains(t, err.Error(), "metadata") } // TestCmd_NonYAMLFileErrors: a regular non-YAML/JSON/Kptfile file must From bdffd404d876b457f6097c6bbc8b63ee5a45364c Mon Sep 17 00:00:00 2001 From: Aravindhan Ayyanathan Date: Wed, 29 Apr 2026 18:57:17 +0100 Subject: [PATCH 3/4] update copyright year and improve error handling in cmdcat Signed-off-by: Aravindhan Ayyanathan --- commands/pkg/pkgcmd.go | 11 +++++--- .../cmdconfig/commands/cmdcat/cmdcat.go | 6 ++--- .../cmdconfig/commands/cmdcat/cmdcat_test.go | 25 ++++++++++--------- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/commands/pkg/pkgcmd.go b/commands/pkg/pkgcmd.go index 052b6bcd44..6c3951e0f4 100644 --- a/commands/pkg/pkgcmd.go +++ b/commands/pkg/pkgcmd.go @@ -1,4 +1,4 @@ -// Copyright 2019-2026 The kpt Authors +// Copyright 2019,2026 The kpt Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -46,9 +46,12 @@ func GetCommand(ctx context.Context, name string) *cobra.Command { } pkg.AddCommand( - get.NewCommand(ctx, name), initialization.NewCommand(ctx, name), - update.NewCommand(ctx, name), diff.NewCommand(ctx, name), - cmdtree.NewCommand(ctx, name), cmdcat.NewCommand(ctx, name), + get.NewCommand(ctx, name), + initialization.NewCommand(ctx, name), + update.NewCommand(ctx, name), + diff.NewCommand(ctx, name), + cmdtree.NewCommand(ctx, name), + cmdcat.NewCommand(ctx, name), ) return pkg } diff --git a/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go b/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go index 4d743c95c5..0487030703 100644 --- a/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go +++ b/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go @@ -1,4 +1,4 @@ -// Copyright 2019-2026 The kpt Authors. +// Copyright 2019,2026 The Kubernetes Authors. // SPDX-License-Identifier: Apache-2.0 package cmdcat @@ -110,7 +110,7 @@ func (r *CatRunner) ExecuteCmd(w io.Writer, pkgPath string) error { err := kio.Pipeline{ Inputs: []kio.Reader{input}, Filters: r.catFilters(), - Outputs: r.out(out), + Outputs: r.outputWriter(out), }.Execute() if err != nil { @@ -138,7 +138,7 @@ func (r *CatRunner) catFilters() []kio.Filter { return fltrs } -func (r *CatRunner) out(w io.Writer) []kio.Writer { +func (r *CatRunner) outputWriter(w io.Writer) []kio.Writer { var outputs []kio.Writer var functionConfig *yaml.RNode diff --git a/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go b/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go index 5bab81c632..dcf76b0a89 100644 --- a/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go +++ b/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go @@ -1,4 +1,4 @@ -// Copyright 2019-2026 The kpt Authors. +// Copyright 2019,2026 The Kubernetes Authors. // SPDX-License-Identifier: Apache-2.0 package cmdcat @@ -14,6 +14,7 @@ import ( "github.com/kptdev/kpt/pkg/printer" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const f1Yaml = "f1.yaml" @@ -88,7 +89,7 @@ spec: `) got, err := runCat(t, d) - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, `kind: Deployment metadata: labels: @@ -149,7 +150,7 @@ spec: `) got, err := runCat(t, filepath.Join(d, f1Yaml)) - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, `kind: Deployment metadata: labels: @@ -179,7 +180,7 @@ spec: `) got, err := runCat(t, filepath.Join(d, f1Yaml), "--annotate") - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, `kind: Deployment metadata: labels: @@ -208,7 +209,7 @@ metadata: `) got, err := runCat(t, filepath.Join(d, f1Yaml)) - assert.NoError(t, err) + require.NoError(t, err) assert.NotContains(t, got, "config.kubernetes.io/path", "config.kubernetes.io/path should be cleared by default") assert.NotContains(t, got, "internal.config.kubernetes.io/path", @@ -246,7 +247,7 @@ spec: `) got, err := runCat(t, d) - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, `kind: Deployment metadata: labels: @@ -299,7 +300,7 @@ metadata: `) got, err := runCat(t, d) - assert.NoError(t, err) + require.NoError(t, err) // exactly one of each resource, separated by a single ---. assert.Equal(t, 2, strings.Count(got, "\nkind: ConfigMap\n"), "each pkg should be emitted exactly once") @@ -340,7 +341,7 @@ metadata: }() got, err := runCat(t, arg) - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, 1, strings.Count(got, "\nkind: ConfigMap\n"), "arg %q should yield exactly one resource", arg) }) @@ -371,7 +372,7 @@ metadata: `) got, err := runCat(t, d) - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, strings.HasSuffix(got, "---\n") || strings.HasSuffix(got, "---"), "output must not end with a stray separator; got %q", got) assert.Equal(t, 0, strings.Count(got, "\n---\n"), @@ -477,7 +478,7 @@ metadata: `) got, err := runCat(t, d, "-R=false") - assert.NoError(t, err) + require.NoError(t, err) assert.Contains(t, got, "name: root-cm") assert.NotContains(t, got, "name: sub-cm", "sub-pkg should not be traversed when -R=false") @@ -494,7 +495,7 @@ metadata: `) got, err := runCat(t, filepath.Join(d, "f.yaml"), "--strip-comments") - assert.NoError(t, err) + require.NoError(t, err) assert.NotContains(t, got, "top comment") assert.NotContains(t, got, "inline comment") } @@ -513,7 +514,7 @@ data: `) got, err := runCat(t, filepath.Join(d, "f.yaml"), "--format=false") - assert.NoError(t, err) + require.NoError(t, err) zIdx := strings.Index(got, "z-last") aIdx := strings.Index(got, "a-first") assert.True(t, zIdx >= 0 && aIdx >= 0, "both keys should be present") From 145d4987389d84111d230cd58d5a441c1a4abb85 Mon Sep 17 00:00:00 2001 From: Aravindhan Ayyanathan Date: Thu, 30 Apr 2026 08:29:00 +0100 Subject: [PATCH 4/4] improve error handling in pkg cat command to include package context Signed-off-by: Aravindhan Ayyanathan --- thirdparty/cmdconfig/commands/cmdcat/cmdcat.go | 14 ++++++-------- .../cmdconfig/commands/cmdcat/cmdcat_test.go | 15 ++++++++++----- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go b/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go index 0487030703..3a6f27efac 100644 --- a/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go +++ b/thirdparty/cmdconfig/commands/cmdcat/cmdcat.go @@ -14,7 +14,6 @@ import ( "github.com/kptdev/kpt/internal/docs/generated/pkgdocs" kptfilev1 "github.com/kptdev/kpt/pkg/api/kptfile/v1" - "github.com/kptdev/kpt/pkg/printer" "github.com/kptdev/kpt/thirdparty/cmdconfig/commands/runner" "github.com/spf13/cobra" "sigs.k8s.io/kustomize/kyaml/kio" @@ -114,14 +113,13 @@ func (r *CatRunner) ExecuteCmd(w io.Writer, pkgPath string) error { }.Execute() if err != nil { - // Emit a contextual diagnostic on stderr so the user knows which - // package failed; the runner currently aborts on the first error. - fmt.Fprintf(printer.FromContextOrDie(r.Ctx).ErrStream(), - "kpt pkg cat: %s in package %q\n", err.Error(), pkgPath) - return err + // Wrap with package context so the user knows which package failed; + // the root command's error handler is responsible for printing. + return fmt.Errorf("kpt pkg cat: %q: %w", pkgPath, err) } - fmt.Fprint(w, out.String()) - if out.String() != "" { + outStr := out.String() + fmt.Fprint(w, outStr) + if outStr != "" { fmt.Fprint(w, "---") } return nil diff --git a/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go b/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go index dcf76b0a89..ad5d04ca19 100644 --- a/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go +++ b/thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go @@ -398,11 +398,16 @@ data: _, err := runCat(t, d) assert.Error(t, err) + // The returned error must carry the pkg-path context ("kpt pkg cat: "":") + // as well as the underlying file that failed. The root cobra handler + // (main.handleErr) is responsible for printing; ExecuteCmd must not + // write to stderr itself, or the user would see the message twice. + assert.Contains(t, err.Error(), "kpt pkg cat:") + assert.Contains(t, err.Error(), d) assert.Contains(t, err.Error(), "broken.yaml") - // Stderr must carry the contextual diagnostic. Inject a printer via - // context (mirroring how run.go wires the root cobra err stream) and - // assert on its err stream. + // Sanity-check that ExecuteCmd itself does not write to the printer's + // ErrStream — otherwise the root handler would duplicate the output. var errBuf bytes.Buffer ctx := printer.WithContext(context.Background(), printer.New(nil, &errBuf)) r := GetCatRunner(ctx, "") @@ -410,8 +415,8 @@ data: r.Command.SetOut(&bytes.Buffer{}) r.Command.SetErr(&bytes.Buffer{}) _ = r.Command.Execute() - assert.Contains(t, errBuf.String(), "kpt pkg cat:") - assert.Contains(t, errBuf.String(), "broken.yaml") + assert.Empty(t, errBuf.String(), + "ExecuteCmd must not print to ErrStream; the root handler prints the returned error") } // TestCmd_NonExistent verifies a clean error on a missing path.