From b14fe2ceadf7778e1e2775c50a93c0c643f8b08c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Schr=C3=B6der?= Date: Wed, 28 Jul 2021 10:19:03 +0200 Subject: [PATCH 1/3] feat(helm): add --password-stdin to `helm repo add` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **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 --- cmd/helm/repo_add.go | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index beafb31cf..707e13fe5 100644 --- a/cmd/helm/repo_add.go +++ b/cmd/helm/repo_add.go @@ -48,6 +48,7 @@ type repoAddOptions struct { url string username string password string + passwordFromStdinOpt bool passCredentialsAll bool forceUpdate bool allowDeprecatedRepos bool @@ -85,6 +86,7 @@ func newRepoAddCmd(out io.Writer) *cobra.Command { f := cmd.Flags() f.StringVar(&o.username, "username", "", "chart repository username") f.StringVar(&o.password, "password", "", "chart repository password") + f.BoolVarP(&o.passwordFromStdinOpt, "password-stdin", "", false, "read chart repository password from stdin") f.BoolVar(&o.forceUpdate, "force-update", false, "replace (overwrite) the repo if it already exists") f.BoolVar(&o.deprecatedNoUpdate, "no-update", false, "Ignored. Formerly, it would disabled forced updates. It is deprecated by force-update.") f.StringVar(&o.certFile, "cert-file", "", "identify HTTPS client using this SSL certificate file") @@ -143,14 +145,24 @@ func (o *repoAddOptions) run(out io.Writer) error { } if o.username != "" && o.password == "" { - fd := int(os.Stdin.Fd()) - fmt.Fprint(out, "Password: ") - password, err := term.ReadPassword(fd) - fmt.Fprintln(out) - if err != nil { - return err + if o.passwordFromStdinOpt { + passwordFromStdin, err := ioutil.ReadAll(os.Stdin) + if err != nil { + return err + } + password := strings.TrimSuffix(string(passwordFromStdin), "\n") + password = strings.TrimSuffix(password, "\r") + o.password = password + } else { + fd := int(os.Stdin.Fd()) + fmt.Fprint(out, "Password: ") + password, err := term.ReadPassword(fd) + fmt.Fprintln(out) + if err != nil { + return err + } + o.password = string(password) } - o.password = string(password) } c := repo.Entry{ From 6515ea84e2c20e844fd015ca8a973953c87285bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Schr=C3=B6der?= Date: Wed, 18 Aug 2021 14:54:07 +0200 Subject: [PATCH 2/3] [fix concern] use io.ReadAll instead of ioutil.ReadAll MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: André Schröder --- cmd/helm/repo_add.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index 707e13fe5..8844174be 100644 --- a/cmd/helm/repo_add.go +++ b/cmd/helm/repo_add.go @@ -146,7 +146,7 @@ func (o *repoAddOptions) run(out io.Writer) error { if o.username != "" && o.password == "" { if o.passwordFromStdinOpt { - passwordFromStdin, err := ioutil.ReadAll(os.Stdin) + passwordFromStdin, err := io.ReadAll(os.Stdin) if err != nil { return err } From 2131f4cba830dc2910990b629ba12f40de8edd3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Schr=C3=B6der?= Date: Wed, 18 Aug 2021 14:54:43 +0200 Subject: [PATCH 3/3] [fix concern] implement test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: André Schröder --- cmd/helm/repo_add_test.go | 31 +++++++++++++++++++++++++ cmd/helm/testdata/password | 1 + pkg/downloader/chart_downloader_test.go | 15 +----------- pkg/repo/repotest/server.go | 17 ++++++++++++++ 4 files changed, 50 insertions(+), 14 deletions(-) create mode 100644 cmd/helm/testdata/password diff --git a/cmd/helm/repo_add_test.go b/cmd/helm/repo_add_test.go index 739173ee7..c88479ea1 100644 --- a/cmd/helm/repo_add_test.go +++ b/cmd/helm/repo_add_test.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "sync" "testing" @@ -204,3 +205,33 @@ func TestRepoAddFileCompletion(t *testing.T) { checkFileCompletion(t, "repo add reponame", false) checkFileCompletion(t, "repo add reponame https://example.com", false) } + +func TestRepoAddWithPasswordFromStdin(t *testing.T) { + srv := repotest.NewTempServerWithCleanupAndBasicAuth(t, "testdata/testserver/*.*") + defer srv.Stop() + + defer resetEnv()() + + in, err := os.Open("testdata/password") + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + + tmpdir := ensure.TempDir(t) + repoFile := filepath.Join(tmpdir, "repositories.yaml") + + store := storageFixture() + + const testName = "test-name" + const username = "username" + cmd := fmt.Sprintf("repo add %s %s --repository-config %s --repository-cache %s --username %s --password-stdin", testName, srv.URL(), repoFile, tmpdir, username) + var result string + _, result, err = executeActionCommandStdinC(store, in, cmd) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + + if !strings.Contains(result, fmt.Sprintf("\"%s\" has been added to your repositories", testName)) { + t.Errorf("Repo was not successfully added. Output: %s", result) + } +} diff --git a/cmd/helm/testdata/password b/cmd/helm/testdata/password new file mode 100644 index 000000000..f3097ab13 --- /dev/null +++ b/cmd/helm/testdata/password @@ -0,0 +1 @@ +password diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index 38a06671c..f70a56422 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -16,7 +16,6 @@ limitations under the License. package downloader import ( - "net/http" "os" "path/filepath" "testing" @@ -171,19 +170,7 @@ func TestIsTar(t *testing.T) { } func TestDownloadTo(t *testing.T) { - // Set up a fake repo with basic auth enabled - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*") - srv.Stop() - if err != nil { - t.Fatal(err) - } - srv.WithMiddleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - username, password, ok := r.BasicAuth() - if !ok || username != "username" || password != "password" { - t.Errorf("Expected request to use basic auth and for username == 'username' and password == 'password', got '%v', '%s', '%s'", ok, username, password) - } - })) - srv.Start() + srv := repotest.NewTempServerWithCleanupAndBasicAuth(t, "testdata/*.tgz*") defer srv.Stop() if err := srv.CreateIndex(); err != nil { t.Fatal(err) diff --git a/pkg/repo/repotest/server.go b/pkg/repo/repotest/server.go index 64d22257b..5c13fdf69 100644 --- a/pkg/repo/repotest/server.go +++ b/pkg/repo/repotest/server.go @@ -56,6 +56,23 @@ func NewTempServerWithCleanup(t *testing.T, glob string) (*Server, error) { return srv, err } +// Set up a fake repo with basic auth enabled +func NewTempServerWithCleanupAndBasicAuth(t *testing.T, glob string) *Server { + srv, err := NewTempServerWithCleanup(t, glob) + srv.Stop() + if err != nil { + t.Fatal(err) + } + srv.WithMiddleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + username, password, ok := r.BasicAuth() + if !ok || username != "username" || password != "password" { + t.Errorf("Expected request to use basic auth and for username == 'username' and password == 'password', got '%v', '%s', '%s'", ok, username, password) + } + })) + srv.Start() + return srv +} + type OCIServer struct { *registry.Registry RegistryURL string