8000 feat(helm): add --password-stdin to `helm repo add` by schra · Pull Request #9980 · helm/helm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 3 commits into from
Aug 20, 2021

Conversation

schra
Copy link
Contributor
@schra schra commented Jul 28, 2021

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

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 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.

  • 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 yield inappropriate ioctl for device.

Special notes for your reviewer:

If applicable:

  • this PR contains documentation
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

**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>
@helm-bot helm-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 28, 2021
@cndoit18
Copy link
Contributor

PRs must be linked to an existing issue. If none exists, may you please open one explaining the issue and expected outcome?
Please also write tests to test and verify this PR fixes your issue. Thanks.

Copy link
Contributor
@cndoit18 cndoit18 left a 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?

@helm-bot helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 18, 2021
schra added 2 commits August 18, 2021 15:03
Signed-off-by: André Schröder <andre.schroedr@gmail.com>
Signed-off-by: André Schröder <andre.schroedr@gmail.com>
@schra schra force-pushed the feat/password-stdin branch from b029df4 to 2131f4c Compare August 18, 2021 13:03
@hickeyma hickeyma added this to the 3.7.0 milestone Aug 18, 2021
Copy link
Member
@technosophos technosophos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM

Copy link
Member
@yxxhero yxxhero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bacongobbler bacongobbler merged commit 8d8a27e into helm:main Aug 20, 2021
@hickeyma hickeyma added needs-pick Indicates that a PR needs to be cherry-picked into the next release candidate. and removed awaiting review needs-pick Indicates that a PR needs to be cherry-picked into the next release candidate. labels Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

helm repo add is missing --password-stdin
7 participants
0