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

Argument parsing in scripts is too permissive #482

Open
DazWorrall opened this issue May 15, 2019 · 1 comment

Comments

Projects
None yet
2 participants
@DazWorrall
Copy link
Member

commented May 15, 2019

Bug report

  • When required arguments are missing (e.g. context and namespace) the fail state is inconsistent:
$ kubernetes-restart 
[INFO][2019-05-15 10:05:00 +0100]
[INFO][2019-05-15 10:05:00 +0100]       -----------------------------------Phase 1: Initializing restart------------------------------------
[INFO][2019-05-15 10:05:00 +0100]
[INFO][2019-05-15 10:05:00 +0100]       ------------------------------------------Result: FAILURE-------------------------------------------
[FATAL][2019-05-15 10:05:00 +0100]      `` context must be configured in your kubeconfig file(s) (/users/daz/.kube/config).
[FATAL][2019-05-15 10:05:00 +0100]

vs

$ kubernetes-deploy
[ERROR][2019-05-15 10:07:16 +0100]      Template directory is unknown. Either specify --template-dir argument or set $ENVIRONMENT to use config/deploy/$ENVIRONMENT as a default path.
  • Script execution continues even when invalid options are passed
$ kubernetes-deploy --foo
kubernetes-deploy: invalid option: --foo
[ERROR][2019-05-15 10:07:59 +0100]      Template directory is unknown. Either specify --template-dir argument or set $ENVIRONMENT to use config/deploy/$ENVIRONMENT as a default path.

(This one is due to our use of #options without checking the result of the block)

Expected behavior:

  • Error messaging for common fail states between the CLI tools (e.g. missing required arguments) is clear and consistent - this bug report originates from a user who was unable to get kubernetes-run to work as they expected
  • Passing invalid options aborts execution

Version(s) affected: Examples from v0.26.4

@dturn

This comment has been minimized.

Copy link
Contributor

commented May 15, 2019

I think this will be good to keep in mind as we transition to the new krane cli, but seems low priority for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.