Skip to content

fix(cli): reject invalid arguments on plugin, plugin install, and convert - #349

Open
kayemkim wants to merge 1 commit into
apache:mainfrom
kayemkim:fix/cli-reject-invalid-args
Open

fix(cli): reject invalid arguments on plugin, plugin install, and convert#349
kayemkim wants to merge 1 commit into
apache:mainfrom
kayemkim:fix/cli-reject-invalid-args

Conversation

@kayemkim

Copy link
Copy Markdown
Contributor

Summary

All five reproductions from #345 now exit non-zero with usage shown:

$ ossie plugin bogus
Error: unknown command "bogus" for "ossie plugin"
$ ossie plugin help list
Error: unknown command "help" for "ossie plugin"
$ ossie plugin install a b c
Error: accepts at most 1 arg(s), received 3
$ ossie plugin install
Error: requires a plugin name or --all
$ ossie convert --from x --input y extra
Error: unknown command "extra" for "ossie convert"

As the issue's cause analysis lays out, this took two different fixes:

  • plugin install and convert simply lacked an Args validator. install now takes at most one positional argument and requires exactly one of the plugin name or --all (so install --all foo is also rejected); convert is flag-only and takes none.
  • The plugin parent had to become runnable first: without a Run/RunE, cobra bails out with flag.ErrHelp before reaching ValidateArgs, so Args alone is dead code there. It now has a RunE that returns an error for a bare ossie plugin, and once the command is runnable, NoArgs produces the "unknown command" error for a typo'd subcommand.

Tests follow the issue's suggestion on both points. They drive the real entry point through rootCmd.SetArgs() / Execute(), since a direct ValidateArgs call passes on the non-runnable parent while the CLI still exits 0; covered are the five reproductions, the valid invocations, and the help paths (--help, -h, and help plugin all still exit 0). A second test walks rootCmd.Commands() recursively and fails if any command lacks an Args validator or any parent is not runnable, so the next command added without validation fails the suite.

The "stubs print not yet implemented and exit 0" part at the end of the issue is left out here, since it's marked there as likely a separate issue.

Related Issues

Fixes #345.

Checklist

Specification

  • Spec changes are included in core-spec/ and follow the existing structure
  • Spec changes have been discussed on the mailing list or in a linked issue
  • Breaking changes to the spec are clearly called out in the summary

Ontology

  • Ontology changes in ontology/ are consistent with spec changes
  • New or modified terms are defined and documented

Converters

  • Converter logic in converters/ is updated to reflect spec or ontology changes
  • New converters include tests under the converter's test directory

Validation

  • Validation rules in validation/ are updated if the spec changed
  • New validation cases are covered by tests

Documentation

  • docs/ is updated to reflect any user-facing changes
  • New features or behaviors are documented with examples where appropriate
  • CONTRIBUTING.md is updated if the contribution process changed

Examples

  • examples/ are added or updated for any new spec constructs or converter support

Tests

  • All existing tests pass (pytest / CI green)
  • New functionality is covered by tests

Compliance

  • ASF license headers are present on all new source files
  • No third-party dependencies are added without PMC/IPMC approval

…vert

Fixes apache#345. Three commands accepted invalid invocations and exited 0:

- plugin install and convert declared no Args validator, so cobra
  accepted arbitrary positional arguments (same root cause as apache#334).
  install now takes at most one argument and requires exactly one of
  the plugin name or --all; convert is flag-only and takes none.

- The plugin parent command needed a different fix: without a RunE it
  is not runnable, cobra returns flag.ErrHelp before ever reaching
  ValidateArgs, and both a bare "ossie plugin" and an unknown
  subcommand printed help and exited 0. It now has Args: NoArgs plus a
  RunE that reports an error, which puts validation back in the
  execution path.

Tests exercise the real entry point via rootCmd.Execute() rather than
cmd.ValidateArgs(), which passes on a non-runnable parent while the
CLI still exits 0. A recursive walk over rootCmd.Commands() asserts
every command declares an Args validator and every parent is runnable,
so the next command added without validation fails the suite.

Signed-off-by: km <kayemkim@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cli: argument validation gaps remain after #334 (plugin, plugin install, convert)

1 participant