-
Notifications
You must be signed in to change notification settings - Fork 7.3k
feat(helm): add --password-stdin to helm repo add
#9980
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
Conversation
**What* Without this commit, you can't pipe the password into `helm repo add`: ``` $ echo password | helm repo add repo-name https://repo-url --username username Password: Error: inappropriate ioctl for device ``` This commit introduces `--password-stdin`: ``` $ echo password | helm repo add repo-name https://repo-url --username username --password-stdin "repo-name" has been added to your repositories ``` **Why** There are two reasons I see for adding this: * I personally would expect that it's possible to pipe the password into `helm repo add` but that's currently not the case. If I understand it correctly, you currently either need to pass the password via a cli parameter (`--password`) or use a expect/send mechanism. * Subcommands like `helm registry login` already support `--password-stdin`. The cli interfaces should be consistent regarding which options they support. **Notes** I basically just copy-pasted code from `cmd/helm/registry_login.go`. Signed-off-by: André Schröder <andre.schroedr@gmail.com>
PRs must be linked to an existing issue. If none exists, may you please open one explaining the issue and expected outcome? |
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.
Could you add some unit tests?
Signed-off-by: André Schröder <andre.schroedr@gmail.com>
Signed-off-by: André Schröder <andre.schroedr@gmail.com>
b029df4
to
2131f4c
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.
Code LGTM
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
What
Without this commit, you can't pipe the password into
helm repo add
:This commit introduces
--password-stdin
:Fixes #10032
Why
There are two reasons I see for adding this:
I personally would expect that it's possible to pipe the password into
helm repo add
but that's currently not the case. If I understand itcorrectly, you currently either need to pass the password via a cli
parameter (
--password
) or use a expect/send mechanism.Subcommands like
helm registry login
already support--password-stdin
. The cli interfaces should be consistent regardingwhich options they support.
Notes
I basically just copy-pasted code from
cmd/helm/registry_login.go
.I don't know if this is the best approach to enable piping into the command. Maybe it would be sufficient to replace
term.ReadPassword(fd)
with something else that also supports piping/ doesn't yieldinappropriate ioctl for device
.Special notes for your reviewer:
If applicable: