fix: validate --output flag value in version command by venkatasai-kadamati · Pull Request #6118 · kubernetes-sigs/kustomize

venkatasai-kadamati · 2026-04-08T15:30:14Z

When a user passes an unrecognized value to the --output flag of kustomize version (for example, --output=yml), the command produces no output and exits silently with no error, leaving the user no indication of what went wrong.

This PR adds validation in Validate() that rejects unrecognized output values before Run() is called, consistent with how other kustomize commands structure pre-flight checks and how kubectl version handles the same flag. Validate() is the correct location because it gives Cobra a chance to surface the error with usage text before any I/O occurs. Run() also gains a defensive default: branch that returns an explicit error for callers that invoke it directly without going through Validate(). The error string is extracted to a const (errInvalidOutput) shared by both sites.

Test coverage: 7 table-driven cases in TestOptions_Validate covering valid values (yaml, json, "") and invalid ones, 3 cases in TestOptions_Run_ValidOutputs confirming correct output format, TestOptions_Run_InvalidOutputReturnsError exercising the Run() defensive branch directly, and TestNewCmdVersion_InvalidOutputFlag exercising the full Cobra command path. All 12 tests pass.

Note: #6033 addresses the same issue but adds only the Run() default branch without a Validate() step or any tests.

k8s-ci-robot · 2026-04-08T15:30:18Z

This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot · 2026-04-08T15:30:23Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: venkatasai-kadamati
Once this PR has been reviewed and has the lgtm label, please assign koba1t for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2026-04-08T15:30:25Z

Welcome @venkatasai-kadamati!

It looks like this is your first PR to kubernetes-sigs/kustomize 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kustomize has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2026-04-08T15:30:26Z

Hi @venkatasai-kadamati. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copilot

Pull request overview

Adds explicit validation for kustomize version --output to prevent silent no-op behavior on unrecognized output formats, aligning behavior with common Cobra pre-flight validation patterns and kubectl version.

Changes:

Add Validate() checks rejecting unsupported --output values early.
Add a defensive default: branch in Run() returning an explicit error when Output is invalid.
Introduce new unit tests covering valid/invalid --output values and the Cobra command execution path.

Reviewed changes

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

File	Description
`kustomize/commands/version/version.go`	Validates `--output` values and returns a consistent error from both `Validate()` and `Run()`.
`kustomize/commands/version/version_test.go`	Adds table-driven tests for validation and run behavior, plus a Cobra execution-path test for invalid flags.

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

Copilot · 2026-04-08T15:34:07Z

+// errInvalidOutput is returned when --output is set to an unrecognized value.
+const errInvalidOutput = "--output must be 'yaml' or 'json'"
+


errInvalidOutput is a string constant but is named like an error, which makes the call sites use fmt.Errorf("%s", errInvalidOutput) just to convert it back into an error. Consider defining it as an error value (e.g., via errors.New) or renaming it to indicate it’s a message string, and then returning it without the redundant formatting.

Copilot · 2026-04-08T15:34:08Z

+			wantErr: "--output must be 'yaml' or 'json'",
+		},
+		{
+			name:    "invalid output xml",
+			opts:    newOptionsWithOutput("xml"),
+			wantErr: "--output must be 'yaml' or 'json'",


These test cases hardcode the invalid-output error string. Since the production code already defines errInvalidOutput, consider referencing that constant here (tests are in the same package) to avoid the tests silently diverging if the message changes.

Suggested change

wantErr: "--output must be 'yaml' or 'json'",

},

{

name: "invalid output xml",

opts: newOptionsWithOutput("xml"),

wantErr: "--output must be 'yaml' or 'json'",

wantErr: errInvalidOutput.Error(),

},

{

name: "invalid output xml",

opts: newOptionsWithOutput("xml"),

wantErr: errInvalidOutput.Error(),

Copilot · 2026-04-08T15:34:08Z

+			wantErr: "--output must be 'yaml' or 'json'",
+		},
+		{
+			name:    "invalid output xml",
+			opts:    newOptionsWithOutput("xml"),
+			wantErr: "--output must be 'yaml' or 'json'",


These test cases hardcode the invalid-output error string. Since the production code already defines errInvalidOutput, consider referencing that constant here (tests are in the same package) to avoid duplication and drift.

Suggested change

wantErr: "--output must be 'yaml' or 'json'",

},

{

name: "invalid output xml",

opts: newOptionsWithOutput("xml"),

wantErr: "--output must be 'yaml' or 'json'",

wantErr: errInvalidOutput.Error(),

},

{

name: "invalid output xml",

opts: newOptionsWithOutput("xml"),

wantErr: errInvalidOutput.Error(),

Copilot · 2026-04-08T15:34:09Z

+func TestOptions_Run_InvalidOutputReturnsError(t *testing.T) {
+	o := newOptionsWithOutput("yml")
+	err := o.Run()
+	require.EqualError(t, err, "--output must be 'yaml' or 'json'")
+}


This assertion hardcodes the invalid-output error string. Consider asserting against errInvalidOutput (defined in version.go) so test expectations stay aligned with the command’s error message.

Copilot · 2026-04-08T15:34:09Z

+func TestNewCmdVersion_InvalidOutputFlag(t *testing.T) {
+	buf := &bytes.Buffer{}
+	cmd := NewCmdVersion(buf)
+	cmd.SilenceErrors = true // prevent cobra from printing to os.Stderr
+	cmd.SilenceUsage = true  // prevent cobra from printing usage to os.Stderr
+	cmd.SetArgs([]string{"--output=yml"})
+	err := cmd.Execute()
+	require.EqualError(t, err, "--output must be 'yaml' or 'json'")
+}


This command-path test hardcodes the invalid-output error string. Consider using the shared errInvalidOutput constant to keep the test coupled to the intended message and reduce duplication.

kustomize version --output=yml exits 0 with no output and no error. Add validation in Validate() — consistent with the existing --short/--output mutual-exclusion check — and a defensive default: case in Run() for direct callers that bypass Validate(). Define errInvalidOutput as a package-level error var so call sites return it directly and tests reference errInvalidOutput.Error() instead of hardcoding the message string.

venkatasai-kadamati · 2026-04-08T15:37:11Z

Good catch on all five points — they're all the same root concern. Addressed in the latest push (3d9df10):

const errInvalidOutput → var errInvalidOutput = fmt.Errorf("--output must be 'yaml' or 'json'") (used existing fmt import to avoid a naming conflict with the kyaml/errors alias already in the file)
Both call sites in version.go now return errInvalidOutput directly
All four wantErr hardcoded strings in version_test.go now reference errInvalidOutput.Error()

test: add failing test for invalid --output flag in version command

28e90a7

Copilot AI review requested due to automatic review settings April 8, 2026 15:30

k8s-ci-robot requested review from sarab97 and varshaprasad96 April 8, 2026 15:30

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 8, 2026

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 8, 2026

Copilot started reviewing on behalf of venkatasai-kadamati April 8, 2026 15:31 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

venkatasai-kadamati force-pushed the fix/6032-version-output-validation branch from ceca8f0 to 3d9df10 Compare April 8, 2026 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: validate --output flag value in version command#6118

fix: validate --output flag value in version command#6118
venkatasai-kadamati wants to merge 2 commits intokubernetes-sigs:masterfrom
venkatasai-kadamati:fix/6032-version-output-validation

venkatasai-kadamati commented Apr 8, 2026

Uh oh!

k8s-ci-robot commented Apr 8, 2026

Uh oh!

k8s-ci-robot commented Apr 8, 2026

Uh oh!

k8s-ci-robot commented Apr 8, 2026

Uh oh!

k8s-ci-robot commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

venkatasai-kadamati commented Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// errInvalidOutput is returned when --output is set to an unrecognized value.
		const errInvalidOutput = "--output must be 'yaml' or 'json'"

Conversation

venkatasai-kadamati commented Apr 8, 2026

Uh oh!

k8s-ci-robot commented Apr 8, 2026

Uh oh!

k8s-ci-robot commented Apr 8, 2026

Uh oh!

k8s-ci-robot commented Apr 8, 2026

Uh oh!

k8s-ci-robot commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

venkatasai-kadamati commented Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants