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

feat(helm): prompt for password with helm repo add #8431

Merged
merged 2 commits into from
Jul 10, 2020

Conversation

jrweldon
Copy link
Contributor

@jrweldon jrweldon commented Jul 10, 2020

What this PR does / why we need it:
Closes #7174

Previously in Helm 2, helm repo add would prompt for your password if you only provided the --username flag. This helps prevent someone's credentials from being logged in their shell's history, from showing up in process lists, and from being seen by others nearby.

Special notes for your reviewer:
This is my first time contributing to a large open-source project, please let me know if anything else is needed or desired and I'll do my best to address the concerns as soon as possible.

If applicable:

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

Previously in Helm 2, 'helm repo add' would prompt for your password if
you only provided the --username flag. This helps prevent someone's
credentials from being logged in their shell's history, from showing
up in process lists, and from being seen by others nearby.

Closes helm#7174

Signed-off-by: Jack Weldon <jack.weldon.scm@gmail.com>
@helm-bot helm-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 10, 2020
Copy link
Member

@bacongobbler bacongobbler left a comment

Choose a reason for hiding this comment

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

thanks for looking into this. I left one note for feedback.

Have you tested this yet to make sure it works?

cmd/helm/repo_add.go Outdated Show resolved Hide resolved
Signed-off-by: Jack Weldon <jack.weldon.scm@gmail.com>
@jrweldon
Copy link
Contributor Author

thanks for looking into this. I left one note for feedback.

Have you tested this yet to make sure it works?

Yes, I have tested locally using a private repository. My ${XDG_CONFIG_HOME}/helm/repositories.yaml was updated to include the new entry, similar to how Helm 2 behaved.

@bacongobbler bacongobbler merged commit 0f43a81 into helm:master Jul 10, 2020
@bacongobbler bacongobbler added this to the 3.4.0 milestone Jul 10, 2020
jrweldon added a commit to jrweldon/helm that referenced this pull request Jul 11, 2020
Fix broken windows/amd64 build caused by helm#8431

https://golang.org/pkg/syscall/#pkg-variables indicated that `syscall.Stdin`
should have been an `int`, though it's actually a `syscall.Handle` on
Windows, which is a `uintptr`. In a similar issue posted to golang, a member
of the Golang team stated that the fix would be to cast it to an int.

Signed-off-by: Jack Weldon <jack.weldon.scm@gmail.com>
jrweldon added a commit to jrweldon/helm that referenced this pull request Jul 11, 2020
No longer using the 'syscall' package, as further reading into this
issue has shown that 'syscall' is deprecated/locked down. Additional
issues posted on Golang's github indicates that the newer preferred
mechanism to get the file descriptor for stdin is: int(os.Stdin.Fd())

Signed-off-by: Jack Weldon <jack.weldon.scm@gmail.com>
bacongobbler pushed a commit that referenced this pull request Jul 11, 2020
@jrweldon jrweldon deleted the feat/prompt-for-password branch July 13, 2020 12:18
ashokponkumar pushed a commit to ashokponkumar/helm that referenced this pull request Jul 14, 2020
No longer using the 'syscall' package, as further reading into this
issue has shown that 'syscall' is deprecated/locked down. Additional
issues posted on Golang's github indicates that the newer preferred
mechanism to get the file descriptor for stdin is: int(os.Stdin.Fd())

Signed-off-by: Jack Weldon <jack.weldon.scm@gmail.com>
Signed-off-by: Ashok Pon Kumar <ashokponkumar@in.ibm.com>
ashokponkumar pushed a commit to ashokponkumar/helm that referenced this pull request Jul 14, 2020
No longer using the 'syscall' package, as further reading into this
issue has shown that 'syscall' is deprecated/locked down. Additional
issues posted on Golang's github indicates that the newer preferred
mechanism to get the file descriptor for stdin is: int(os.Stdin.Fd())

Signed-off-by: Jack Weldon <jack.weldon.scm@gmail.com>
Signed-off-by: Ashok Pon Kumar <ashokponkumar@in.ibm.com>
@philomory
Copy link

Did this feature not make it into Helm 3.3.0?

@jrweldon
Copy link
Contributor Author

No, it was a few days after the 3.3 cutoff, it'll be in 3.4

vladfr pushed a commit to vladfr/helm that referenced this pull request Sep 30, 2020
No longer using the 'syscall' package, as further reading into this
issue has shown that 'syscall' is deprecated/locked down. Additional
issues posted on Golang's github indicates that the newer preferred
mechanism to get the file descriptor for stdin is: int(os.Stdin.Fd())

Signed-off-by: Jack Weldon <jack.weldon.scm@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

helm3: repo add no longer interactively asks for password
4 participants