-
Notifications
You must be signed in to change notification settings - Fork 366
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
Reject unsupported postional arguments for antctl #2011
Conversation
pkg/antctl/raw/proxy/command.go
Outdated
@@ -132,6 +132,12 @@ func init() { | |||
Long: "Run a reverse proxy to access Antrea API (Controller or Agent). Command only supports remote mode. HTTPS connections between the proxy and the Antrea API will not be secure (no certificate verification).", | |||
Example: proxyCommandExample, | |||
RunE: runE, | |||
Args: func(cmd *cobra.Command, args []string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we use the builtin args: NoArgs
command validator here? See https://pkg.go.dev/github.com/spf13/cobra#readme-positional-and-custom-arguments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Didn't notice that before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's a pretty basic change but do you think you could add a simple unit test to pkg/antctl/antctl_test.go?
I'm not sure how to write unit test but I will try it. |
1bcf499
to
d3b1511
Compare
Codecov Report
@@ Coverage Diff @@
## main #2011 +/- ##
==========================================
- Coverage 65.39% 57.15% -8.25%
==========================================
Files 197 262 +65
Lines 17217 19469 +2252
==========================================
- Hits 11259 11127 -132
- Misses 4785 7166 +2381
- Partials 1173 1176 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
dad13af
to
dc7dab0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the test, this looks good to me, we can merge once you address Jianjun's comment
For traceflow and proxy, antctl will fail and log error when the user enters extra postional arguments. Fixes antrea-io#1915
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/test-all |
For traceflow and proxy, antctl will fail and log error when the user
enters extra postional arguments.
Before:
After:
Fixes #1915