Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpk: add error for unknown command execution #18650

Merged
merged 2 commits into from
May 27, 2024

Conversation

r-vasquez
Copy link
Contributor

Cobra won't recognize unknown commands unless
cobra.NoArg is set as an Arg validation parameter
in the command definition.

This means that running 'rpk cluster unknowncommand'
won't return an 'unknown command' error, therefore,
exit as a successful command execution.

See: spf13/cobra#706

Now, cobra will return an 'unknown error' and exit
with code 1.

Fixes #18627

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.1.x
  • v23.3.x
  • v23.2.x

Release Notes

Improvements

  • rpk now will exit (1) when running rpk with unknown commands

r-vasquez added 2 commits May 23, 2024 14:49
Cobra won't recognize unknown commands unless
cobra.NoArg is set as an Arg validation parameter
in the command definition.

This means that running 'rpk cluster unknowncommand'
won't return a 'unknown command' error, therefore,
exit as a successful command execution.

Now, cobra will return an 'unknown error' and exit
with code 1.
@r-vasquez r-vasquez changed the title Err for unknown rpk: add error for unknown command execution May 23, 2024
@twmb
Copy link
Contributor

twmb commented May 24, 2024

Another option rather than walking is to add a test that fails if Args is empty. <-This may be better for the future, to enforce at code-writing-time that all new commands have Args carefully chosen to be NoArgs vs. something else.

@twmb
Copy link
Contributor

twmb commented May 24, 2024

But this is good too

@r-vasquez
Copy link
Contributor Author

Going ahead with this one since adding a test like this would force commands with optional arguments to have: cobra.MinimumArgs(0) which can be confusing.

We can revisit this back in the future since adding it would be fairly easy.

@r-vasquez r-vasquez merged commit d8f01f3 into redpanda-data:dev May 27, 2024
25 checks passed
@vbotbuildovich
Copy link
Collaborator

/backport v24.1.x

r-vasquez added a commit to r-vasquez/redpanda that referenced this pull request May 30, 2024
In redpanda-data#18650 we added Arg validations for every
comman and an automatic "walk" through the command
space to add cobra.NoArgs.

Plugins were not taken into account, this commit
explicitly set the cobra.MinimumArg(0) to avoid
adding "NoArgs" on execution.
vbotbuildovich pushed a commit to vbotbuildovich/redpanda that referenced this pull request May 30, 2024
In redpanda-data#18650 we added Arg validations for every
comman and an automatic "walk" through the command
space to add cobra.NoArgs.

Plugins were not taken into account, this commit
explicitly set the cobra.MinimumArg(0) to avoid
adding "NoArgs" on execution.

(cherry picked from commit 0c9a20b)
WillemKauf pushed a commit to WillemKauf/redpanda that referenced this pull request May 31, 2024
In redpanda-data#18650 we added Arg validations for every
comman and an automatic "walk" through the command
space to add cobra.NoArgs.

Plugins were not taken into account, this commit
explicitly set the cobra.MinimumArg(0) to avoid
adding "NoArgs" on execution.
Lazin pushed a commit to Lazin/redpanda that referenced this pull request Jun 1, 2024
In redpanda-data#18650 we added Arg validations for every
comman and an automatic "walk" through the command
space to add cobra.NoArgs.

Plugins were not taken into account, this commit
explicitly set the cobra.MinimumArg(0) to avoid
adding "NoArgs" on execution.
jackietung-redpanda pushed a commit that referenced this pull request Jun 4, 2024
In #18650 we added Arg validations for every
comman and an automatic "walk" through the command
space to add cobra.NoArgs.

Plugins were not taken into account, this commit
explicitly set the cobra.MinimumArg(0) to avoid
adding "NoArgs" on execution.

(cherry picked from commit 0c9a20b)
Lazin pushed a commit to Lazin/redpanda that referenced this pull request Jun 12, 2024
In redpanda-data#18650 we added Arg validations for every
comman and an automatic "walk" through the command
space to add cobra.NoArgs.

Plugins were not taken into account, this commit
explicitly set the cobra.MinimumArg(0) to avoid
adding "NoArgs" on execution.
@r-vasquez r-vasquez deleted the err-for-unknown branch July 3, 2024 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rpk: subcommands does not return "unknown command" error
3 participants