Repository: helm/helm
Pull request #7753 (2020-03-09 22:14:11)

Solves the issue #7749
Resolves the [issue](https://github.com/helm/helm/issues/7749) where proper formatting was not being done if short(-q) option was used with other formatting options like json, yaml.
@mattfarina , Can you please review this.

Please do let me know if any change is required.
Thank You very much.


Patch:
diff --git a/cmd/helm/list.go b/cmd/helm/list.go
index 4b652088d..08d6beb79 100644
--- a/cmd/helm/list.go
+++ b/cmd/helm/list.go
@@ -83,10 +83,29 @@ func newListCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 			}
 
 			if client.Short {
+
+				names := make([]string, 0)
 				for _, res := range results {
-					fmt.Fprintln(out, res.Name)
+					names = append(names, res.Name)
+				}
+
+				outputFlag := cmd.Flag("output")
+
+				switch outputFlag.Value.String() {
+				case "json":
+					output.EncodeJSON(out, names)
+					return nil
+				case "yaml":
+					output.EncodeYAML(out, names)
+					return nil
+				case "table":
+					for _, res := range results {
+						fmt.Fprintln(out, res.Name)
+					}
+					return nil
+				default:
+					return outfmt.Write(out, newReleaseListWriter(results))
 				}
-				return nil
 			}
 
 			return outfmt.Write(out, newReleaseListWriter(results))
diff --git a/pkg/action/install.go b/pkg/action/install.go
index fa13bbade..e7b481d47 100644
--- a/pkg/action/install.go
+++ b/pkg/action/install.go
@@ -744,7 +744,6 @@ func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) (
 		Getters: getter.All(settings),
 		Options: []getter.Option{
 			getter.WithBasicAuth(c.Username, c.Password),
-			getter.WithTLSClientConfig(c.CertFile, c.KeyFile, c.CaFile),
 		},
 		RepositoryConfig: settings.RepositoryConfig,
 		RepositoryCache:  settings.RepositoryCache,
diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go
index 340a65472..f3d4321c5 100644
--- a/pkg/downloader/chart_downloader.go
+++ b/pkg/downloader/chart_downloader.go
@@ -181,10 +181,8 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er
 		c.Options = append(
 			c.Options,
 			getter.WithURL(rc.URL),
+			getter.WithTLSClientConfig(rc.CertFile, rc.KeyFile, rc.CAFile),
 		)
-		if rc.CertFile != "" || rc.KeyFile != "" || rc.CAFile != "" {
-			c.Options = append(c.Options, getter.WithTLSClientConfig(rc.CertFile, rc.KeyFile, rc.CAFile))
-		}
 		if rc.Username != "" && rc.Password != "" {
 			c.Options = append(
 				c.Options,
@@ -212,14 +210,12 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er
 	if err != nil {
 		return u, err
 	}
+	if r != nil && r.Config != nil && r.Config.Username != "" && r.Config.Password != "" {
+		c.Options = append(c.Options, getter.WithBasicAuth(r.Config.Username, r.Config.Password))
+	}
 
-	if r != nil && r.Config != nil {
-		if r.Config.CertFile != "" || r.Config.KeyFile != "" || r.Config.CAFile != "" {
-			c.Options = append(c.Options, getter.WithTLSClientConfig(r.Config.CertFile, r.Config.KeyFile, r.Config.CAFile))
-		}
-		if r.Config.Username != "" && r.Config.Password != "" {
-			c.Options = append(c.Options, getter.WithBasicAuth(r.Config.Username, r.Config.Password))
-		}
+	if r.Config.CertFile != "" || r.Config.KeyFile != "" || r.Config.CAFile != "" {
+		c.Options = append(c.Options, getter.WithTLSClientConfig(r.Config.CertFile, r.Config.KeyFile, r.Config.CAFile))
 	}
 
 	// Next, we need to load the index, and actually look up the chart.
@@ -259,6 +255,9 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er
 		if _, err := getter.NewHTTPGetter(getter.WithURL(rc.URL)); err != nil {
 			return repoURL, err
 		}
+		if r != nil && r.Config != nil && r.Config.Username != "" && r.Config.Password != "" {
+			c.Options = append(c.Options, getter.WithBasicAuth(r.Config.Username, r.Config.Password))
+		}
 		return u, err
 	}
 


Test patch:
diff --git a/cmd/helm/list_test.go b/cmd/helm/list_test.go
index fe773a803..dadb57b94 100644
--- a/cmd/helm/list_test.go
+++ b/cmd/helm/list_test.go
@@ -198,6 +198,16 @@ func TestListCmd(t *testing.T) {
 		cmd:    "list --short",
 		golden: "output/list-short.txt",
 		rels:   releaseFixture,
+	}, {
+		name:   "list releases in short output format",
+		cmd:    "list --short --output yaml",
+		golden: "output/list-short-yaml.txt",
+		rels:   releaseFixture,
+	}, {
+		name:   "list releases in short output format",
+		cmd:    "list --short --output json",
+		golden: "output/list-short-json.txt",
+		rels:   releaseFixture,
 	}, {
 		name:   "list superseded releases",
 		cmd:    "list --superseded",
diff --git a/cmd/helm/testdata/output/list-short-json.txt b/cmd/helm/testdata/output/list-short-json.txt
new file mode 100644
index 000000000..acbf1e44d
--- /dev/null
+++ b/cmd/helm/testdata/output/list-short-json.txt
@@ -0,0 +1,1 @@
+["hummingbird","iguana","rocket","starlord"]
diff --git a/cmd/helm/testdata/output/list-short-yaml.txt b/cmd/helm/testdata/output/list-short-yaml.txt
new file mode 100644
index 000000000..86fb3d670
--- /dev/null
+++ b/cmd/helm/testdata/output/list-short-yaml.txt
@@ -0,0 +1,4 @@
+- hummingbird
+- iguana
+- rocket
+- starlord
diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go
index abfb007ff..e0692c8c8 100644
--- a/pkg/downloader/chart_downloader_test.go
+++ b/pkg/downloader/chart_downloader_test.go
@@ -227,58 +227,6 @@ func TestDownloadTo(t *testing.T) {
 	}
 }
 
-func TestDownloadTo_TLS(t *testing.T) {
-	// Set up mock server w/ tls enabled
-	srv, err := repotest.NewTempServer("testdata/*.tgz*")
-	srv.Stop()
-	if err != nil {
-		t.Fatal(err)
-	}
-	srv.StartTLS()
-	defer srv.Stop()
-	if err := srv.CreateIndex(); err != nil {
-		t.Fatal(err)
-	}
-	if err := srv.LinkIndices(); err != nil {
-		t.Fatal(err)
-	}
-
-	repoConfig := filepath.Join(srv.Root(), "repositories.yaml")
-	repoCache := srv.Root()
-
-	c := ChartDownloader{
-		Out:              os.Stderr,
-		Verify:           VerifyAlways,
-		Keyring:          "testdata/helm-test-key.pub",
-		RepositoryConfig: repoConfig,
-		RepositoryCache:  repoCache,
-		Getters: getter.All(&cli.EnvSettings{
-			RepositoryConfig: repoConfig,
-			RepositoryCache:  repoCache,
-		}),
-		Options: []getter.Option{},
-	}
-	cname := "test/signtest"
-	dest := srv.Root()
-	where, v, err := c.DownloadTo(cname, "", dest)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	target := filepath.Join(dest, "signtest-0.1.0.tgz")
-	if expect := target; where != expect {
-		t.Errorf("Expected download to %s, got %s", expect, where)
-	}
-
-	if v.FileHash == "" {
-		t.Error("File hash was empty, but verification is required.")
-	}
-
-	if _, err := os.Stat(target); err != nil {
-		t.Error(err)
-	}
-}
-
 func TestDownloadTo_VerifyLater(t *testing.T) {
 	defer ensure.HelmHome(t)()
 
diff --git a/pkg/repo/repotest/server.go b/pkg/repo/repotest/server.go
index b18bce49c..96a8bbfcc 100644
--- a/pkg/repo/repotest/server.go
+++ b/pkg/repo/repotest/server.go
@@ -22,8 +22,6 @@ import (
 	"os"
 	"path/filepath"
 
-	"helm.sh/helm/v3/internal/tlsutil"
-
 	"sigs.k8s.io/yaml"
 
 	"helm.sh/helm/v3/pkg/repo"
@@ -145,40 +143,6 @@ func (s *Server) Start() {
 	}))
 }
 
-func (s *Server) StartTLS() {
-	cd := "../../testdata"
-	ca, pub, priv := filepath.Join(cd, "rootca.crt"), filepath.Join(cd, "crt.pem"), filepath.Join(cd, "key.pem")
-
-	s.srv = httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if s.middleware != nil {
-			s.middleware.ServeHTTP(w, r)
-		}
-		http.FileServer(http.Dir(s.Root())).ServeHTTP(w, r)
-	}))
-	tlsConf, err := tlsutil.NewClientTLS(pub, priv, ca)
-	if err != nil {
-		panic(err)
-	}
-	tlsConf.BuildNameToCertificate()
-	tlsConf.ServerName = "helm.sh"
-	s.srv.TLS = tlsConf
-	s.srv.StartTLS()
-
-	// Set up repositories config with ca file
-	repoConfig := filepath.Join(s.Root(), "repositories.yaml")
-
-	r := repo.NewFile()
-	r.Add(&repo.Entry{
-		Name:   "test",
-		URL:    s.URL(),
-		CAFile: filepath.Join("../../testdata", "rootca.crt"),
-	})
-
-	if err := r.WriteFile(repoConfig, 0644); err != nil {
-		panic(err)
-	}
-}
-
 // Stop stops the server and closes all connections.
 //
 // It should be called explicitly.