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

Helm does not error on typo or missing sub-command #8268

Closed
Michael-Sinz opened this issue Jun 8, 2020 · 10 comments
Closed

Helm does not error on typo or missing sub-command #8268

Michael-Sinz opened this issue Jun 8, 2020 · 10 comments
Labels
bug Categorizes issue or PR as related to a bug. Stale v3.x Issues and Pull Requests related to the major version v3

Comments

@Michael-Sinz
Copy link

Output of helm version: version.BuildInfo{Version:"v3.2.3", GitCommit:"8f832046e258e2cb800894579b1b3b50c2d83492", GitTreeState:"clean", GoVersion:"go1.13.12"}

Output of kubectl version: n/a

Cloud Provider/Platform (AKS, GKE, Minikube etc.): n/a

When issuing a helm command, helm returns success on typos.

For example "helm repo sadd foo https://foo/bar" will show the help for "helm repo" but exit code will be 0 (success) rather than non-zero (failure)

This prevents tools from correctly identifying an error happened.

The fix for cobra command line parsing to correctly fix the problem they had is in spf13/cobra#922 and has been in master for over half a year now.

Could we please get helm to fix this.

(This would also then match most commands that fail when typos or missing sub-commands are given.)

Note that git acts correctly - it will provide help if you mistype a command but it will be an error unless you specifically asked for help.

For example, just "git" shows the help but has an exit code of non-zero (1). But "git --help" shows the help with an exit code of 0 (success) in that it did what you asked it to.

@mattfarina mattfarina added the bug Categorizes issue or PR as related to a bug. label Jun 9, 2020
@bacongobbler
Copy link
Member

Helm 3.2.3 uses cobra 1.0.0, which should contain the fix. cobra 1.0 was released in April. That PR you linked to appears to have been merged in August 2019, so it should be in the 1.0 release.

If you can pinpoint where in the codebase that may be causing the issue, we'd appreciate the help.

@mattfarina mattfarina added the v3.x Issues and Pull Requests related to the major version v3 label Jun 9, 2020
@mattfarina
Copy link
Collaborator

I've replicated the issue on the tip of master which runs cobra 1.0.0. Any pointers to what's causing the issue or pull requests with a fix are welcome.

@wawa0210
Copy link
Contributor

wawa0210 commented Jun 9, 2020

If allowed, I can understand the problem and try to fix it

@bacongobbler
Copy link
Member

feel free

@wawa0210
Copy link
Contributor

wawa0210 commented Jun 9, 2020

/assign

@marckhouzam
Copy link
Member

The fix for cobra command line parsing to correctly fix the problem they had is in spf13/cobra#922 and has been in master for over half a year now.

The Cobra fix in question was reverted in spf13/cobra#1068.
I haven't dug into it further, but it explains why helm is behaving the way it is.

@mattfarina
Copy link
Collaborator

This is now fixed on the tip of master so I'm going to close the issue.

@marckhouzam
Copy link
Member

This is now fixed on the tip of master so I'm going to close the issue.

@mattfarina Is this really fixed on master or was there a mixup in which issue you mean to close?

@marckhouzam
Copy link
Member

After speaking at the helm dev call, we agreed to re-open if the problem could be reproduced, so:

$ bin/helm version
version.BuildInfo{Version:"v3.3+unreleased", GitCommit:"198f40368874abbe4dda2ff6ba3eca516932da76", GitTreeState:"clean", GoVersion:"go1.15"}
$ bin/helm repo sadd foo https://foo/bar  &>/dev/null && echo "No error" || echo "With error"
No error

As we can see an invalid command below helm repo still does not generate an error.
There is a PR in Cobra for this but I had some backwards-compatibility concerns which are waiting for opinions from the Cobra maintainers: spf13/cobra#1157

@marckhouzam marckhouzam reopened this Aug 20, 2020
@github-actions
Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Categorizes issue or PR as related to a bug. Stale v3.x Issues and Pull Requests related to the major version v3
Projects
None yet
Development

No branches or pull requests

5 participants