Repository: helm/helm
Pull request #8277 (2020-06-09 18:46:26)

fix(cmd): display warnings on stderr
The warnings introduced when a chart has been deprecated is displayed on standard out. This is a regression for users piping the output of `helm template` from a deprecated chart to `kubectl`. This changes the error message to display on standard error instead.

identified in #8262.

Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>

bacongobbler: thanks for catching that. Fixed.

Patch:
diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go
index 544fb7608..040773d7b 100644
--- a/cmd/helm/flags.go
+++ b/cmd/helm/flags.go
@@ -52,7 +52,6 @@ func addChartPathOptionsFlags(f *pflag.FlagSet, c *action.ChartPathOptions) {
 	f.StringVar(&c.Password, "password", "", "chart repository password where to locate the requested chart")
 	f.StringVar(&c.CertFile, "cert-file", "", "identify HTTPS client using this SSL certificate file")
 	f.StringVar(&c.KeyFile, "key-file", "", "identify HTTPS client using this SSL key file")
-	f.BoolVar(&c.InsecureSkipTLSverify, "insecure-skip-tls-verify", false, "skip tls certificate checks for the chart download")
 	f.StringVar(&c.CaFile, "ca-file", "", "verify certificates of HTTPS-enabled servers using this CA bundle")
 }
 
diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go
index 474a3d2d4..d4e82bb01 100644
--- a/cmd/helm/helm.go
+++ b/cmd/helm/helm.go
@@ -56,6 +56,11 @@ func debug(format string, v ...interface{}) {
 	}
 }
 
+func warning(format string, v ...interface{}) {
+	format = fmt.Sprintf("WARNING: %s\n", format)
+	fmt.Fprintf(os.Stderr, format, v...)
+}
+
 func initKubeLogs() {
 	pflag.CommandLine.SetNormalizeFunc(wordSepNormalizeFunc)
 	gofs := flag.NewFlagSet("klog", flag.ExitOnError)
@@ -68,11 +73,7 @@ func main() {
 	initKubeLogs()
 
 	actionConfig := new(action.Configuration)
-	cmd, err := newRootCmd(actionConfig, os.Stdout, os.Args[1:])
-	if err != nil {
-		debug("%+v", err)
-		os.Exit(1)
-	}
+	cmd := newRootCmd(actionConfig, os.Stdout, os.Args[1:])
 
 	// run when each command's execute method is called
 	cobra.OnInitialize(func() {
diff --git a/cmd/helm/install.go b/cmd/helm/install.go
index 9024add2f..cdb38a10a 100644
--- a/cmd/helm/install.go
+++ b/cmd/helm/install.go
@@ -17,7 +17,6 @@ limitations under the License.
 package main
 
 import (
-	"fmt"
 	"io"
 	"log"
 	"time"
@@ -207,7 +206,7 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options
 	}
 
 	if chartRequested.Metadata.Deprecated {
-		fmt.Fprintln(out, "WARNING: This chart is deprecated")
+		warning("This chart is deprecated")
 	}
 
 	if req := chartRequested.Metadata.Dependencies; req != nil {
diff --git a/cmd/helm/root.go b/cmd/helm/root.go
index 20dd1d93b..69f5855d9 100644
--- a/cmd/helm/root.go
+++ b/cmd/helm/root.go
@@ -68,7 +68,7 @@ By default, the default directories depend on the Operating System. The defaults
 | Windows          | %TEMP%\helm               | %APPDATA%\helm                 | %APPDATA%\helm          |
 `
 
-func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string) (*cobra.Command, error) {
+func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:          "helm",
 		Short:        "The Helm package manager for Kubernetes.",
@@ -175,10 +175,10 @@ func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string
 	registryClient, err := registry.NewClient(
 		registry.ClientOptDebug(settings.Debug),
 		registry.ClientOptWriter(out),
-		registry.ClientOptCredentialsFile(settings.RegistryConfig),
 	)
 	if err != nil {
-		return nil, err
+		// TODO: don't panic here, refactor newRootCmd to return error
+		panic(err)
 	}
 	actionConfig.RegistryClient = registryClient
 	cmd.AddCommand(
@@ -189,5 +189,5 @@ func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string
 	// Find and add plugins
 	loadPlugins(cmd, out)
 
-	return cmd, nil
+	return cmd
 }
diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go
index dc3bc9a58..c992e2f6b 100644
--- a/cmd/helm/upgrade.go
+++ b/cmd/helm/upgrade.go
@@ -148,7 +148,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 			}
 
 			if ch.Metadata.Deprecated {
-				fmt.Fprintln(out, "WARNING: This chart is deprecated")
+				warning("This chart is deprecated")
 			}
 
 			rel, err := client.Run(args[0], ch, vals)
diff --git a/internal/experimental/registry/client.go b/internal/experimental/registry/client.go
index 5756030c0..f664c9f38 100644
--- a/internal/experimental/registry/client.go
+++ b/internal/experimental/registry/client.go
@@ -42,13 +42,11 @@ const (
 type (
 	// Client works with OCI-compliant registries and local Helm chart cache
 	Client struct {
-		debug bool
-		// path to repository config file e.g. ~/.docker/config.json
-		credentialsFile string
-		out             io.Writer
-		authorizer      *Authorizer
-		resolver        *Resolver
-		cache           *Cache
+		debug      bool
+		out        io.Writer
+		authorizer *Authorizer
+		resolver   *Resolver
+		cache      *Cache
 	}
 )
 
@@ -61,11 +59,9 @@ func NewClient(opts ...ClientOption) (*Client, error) {
 		opt(client)
 	}
 	// set defaults if fields are missing
-	if client.credentialsFile == "" {
-		client.credentialsFile = helmpath.CachePath("registry", CredentialsFileBasename)
-	}
 	if client.authorizer == nil {
-		authClient, err := auth.NewClient(client.credentialsFile)
+		credentialsFile := helmpath.CachePath("registry", CredentialsFileBasename)
+		authClient, err := auth.NewClient(credentialsFile)
 		if err != nil {
 			return nil, err
 		}
@@ -240,7 +236,7 @@ func (c *Client) PrintChartTable() error {
 // printCacheRefSummary prints out chart ref summary
 func (c *Client) printCacheRefSummary(r *CacheRefSummary) {
 	fmt.Fprintf(c.out, "ref:     %s\n", r.Name)
-	fmt.Fprintf(c.out, "digest:  %s\n", r.Manifest.Digest.Hex())
+	fmt.Fprintf(c.out, "digest:  %s\n", r.Digest.Hex())
 	fmt.Fprintf(c.out, "size:    %s\n", byteCountBinary(r.Size))
 	fmt.Fprintf(c.out, "name:    %s\n", r.Chart.Metadata.Name)
 	fmt.Fprintf(c.out, "version: %s\n", r.Chart.Metadata.Version)
@@ -257,7 +253,7 @@ func (c *Client) getChartTableRows() ([][]interface{}, error) {
 		refsMap[r.Name] = map[string]string{
 			"name":    r.Chart.Metadata.Name,
 			"version": r.Chart.Metadata.Version,
-			"digest":  shortDigest(r.Manifest.Digest.Hex()),
+			"digest":  shortDigest(r.Digest.Hex()),
 			"size":    byteCountBinary(r.Size),
 			"created": timeAgo(r.CreatedAt),
 		}
diff --git a/internal/experimental/registry/client_opts.go b/internal/experimental/registry/client_opts.go
index e2f742aec..cd295813a 100644
--- a/internal/experimental/registry/client_opts.go
+++ b/internal/experimental/registry/client_opts.go
@@ -60,10 +60,3 @@ func ClientOptCache(cache *Cache) ClientOption {
 		client.cache = cache
 	}
 }
-
-// ClientOptCredentialsFile returns a function that sets the cache setting on a client options set
-func ClientOptCredentialsFile(credentialsFile string) ClientOption {
-	return func(client *Client) {
-		client.credentialsFile = credentialsFile
-	}
-}
diff --git a/pkg/action/install.go b/pkg/action/install.go
index 00fb208b0..351e0928c 100644
--- a/pkg/action/install.go
+++ b/pkg/action/install.go
@@ -104,16 +104,15 @@ type Install struct {
 
 // ChartPathOptions captures common options used for controlling chart paths
 type ChartPathOptions struct {
-	CaFile                string // --ca-file
-	CertFile              string // --cert-file
-	KeyFile               string // --key-file
-	InsecureSkipTLSverify bool   // --insecure-skip-verify
-	Keyring               string // --keyring
-	Password              string // --password
-	RepoURL               string // --repo
-	Username              string // --username
-	Verify                bool   // --verify
-	Version               string // --version
+	CaFile   string // --ca-file
+	CertFile string // --cert-file
+	KeyFile  string // --key-file
+	Keyring  string // --keyring
+	Password string // --password
+	RepoURL  string // --repo
+	Username string // --username
+	Verify   bool   // --verify
+	Version  string // --version
 }
 
 // NewInstall creates a new Install object with the given configuration.
@@ -645,7 +644,6 @@ func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) (
 		Options: []getter.Option{
 			getter.WithBasicAuth(c.Username, c.Password),
 			getter.WithTLSClientConfig(c.CertFile, c.KeyFile, c.CaFile),
-			getter.WithInsecureSkipVerifyTLS(c.InsecureSkipTLSverify),
 		},
 		RepositoryConfig: settings.RepositoryConfig,
 		RepositoryCache:  settings.RepositoryCache,
diff --git a/pkg/action/pull.go b/pkg/action/pull.go
index a46e98bae..ee20bbe83 100644
--- a/pkg/action/pull.go
+++ b/pkg/action/pull.go
@@ -64,7 +64,6 @@ func (p *Pull) Run(chartRef string) (string, error) {
 		Options: []getter.Option{
 			getter.WithBasicAuth(p.Username, p.Password),
 			getter.WithTLSClientConfig(p.CertFile, p.KeyFile, p.CaFile),
-			getter.WithInsecureSkipVerifyTLS(p.InsecureSkipTLSverify),
 		},
 		RepositoryConfig: p.Settings.RepositoryConfig,
 		RepositoryCache:  p.Settings.RepositoryCache,


Test patch:
diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go
index 5e59c41ed..9ba9d78fb 100644
--- a/cmd/helm/helm_test.go
+++ b/cmd/helm/helm_test.go
@@ -115,11 +115,7 @@ func executeActionCommandStdinC(store *storage.Storage, in *os.File, cmd string)
 		Log:          func(format string, v ...interface{}) {},
 	}
 
-	root, err := newRootCmd(actionConfig, buf, args)
-	if err != nil {
-		return nil, "", err
-	}
-
+	root := newRootCmd(actionConfig, buf, args)
 	root.SetOut(buf)
 	root.SetErr(buf)
 	root.SetArgs(args)
diff --git a/cmd/helm/testdata/output/deprecated-chart.txt b/cmd/helm/testdata/output/deprecated-chart.txt
index e5be2c3f1..039d6aef6 100644
--- a/cmd/helm/testdata/output/deprecated-chart.txt
+++ b/cmd/helm/testdata/output/deprecated-chart.txt
@@ -1,4 +1,3 @@
-WARNING: This chart is deprecated
 NAME: aeneas
 LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default