Repository: helm/helm
Pull request #8996 (2020-11-07 14:19:00)

chore(comp): Remove unnecessary completion code
<!--  Thanks for sending a pull request!  Here are some tips for you:
1. Make sure to read the Contributing Guide before submitting your PR: https://github.com/helm/helm/blob/master/CONTRIBUTING.md
2. If this PR closes another issue, add 'closes #<issue number>' somewhere in the PR summary. GitHub will automatically close that issue when this PR gets merged. Alternatively, adding 'refs #<issue number>' will not close the issue, but help provide the reviewer more context.-->

**What this PR does / why we need it**:

Cleanup of unnecessary code after moving to Cobra 1.1.1

With the move to Cobra 1.1.1, all commands that have sub-commands have file completion automatically disabled, so helm need not do it itself.




Patch:
diff --git a/cmd/helm/completion.go b/cmd/helm/completion.go
index 68c6a9b59..5e7343069 100644
--- a/cmd/helm/completion.go
+++ b/cmd/helm/completion.go
@@ -73,11 +73,10 @@ var disableCompDescriptions bool
 
 func newCompletionCmd(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:               "completion",
-		Short:             "generate autocompletions script for the specified shell",
-		Long:              completionDesc,
-		Args:              require.NoArgs,
-		ValidArgsFunction: noCompletions, // Disable file completion
+		Use:   "completion",
+		Short: "generate autocompletions script for the specified shell",
+		Long:  completionDesc,
+		Args:  require.NoArgs,
 	}
 
 	bash := &cobra.Command{
diff --git a/cmd/helm/dependency.go b/cmd/helm/dependency.go
index 39dfd98ce..2cc4c5045 100644
--- a/cmd/helm/dependency.go
+++ b/cmd/helm/dependency.go
@@ -84,12 +84,11 @@ This will produce an error if the chart cannot be loaded.
 
 func newDependencyCmd(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:               "dependency update|build|list",
-		Aliases:           []string{"dep", "dependencies"},
-		Short:             "manage a chart's dependencies",
-		Long:              dependencyDesc,
-		Args:              require.NoArgs,
-		ValidArgsFunction: noCompletions, // Disable file completion
+		Use:     "dependency update|build|list",
+		Aliases: []string{"dep", "dependencies"},
+		Short:   "manage a chart's dependencies",
+		Long:    dependencyDesc,
+		Args:    require.NoArgs,
 	}
 
 	cmd.AddCommand(newDependencyListCmd(out))
diff --git a/cmd/helm/dependency_build.go b/cmd/helm/dependency_build.go
index 4e87684ce..a0b63f038 100644
--- a/cmd/helm/dependency_build.go
+++ b/cmd/helm/dependency_build.go
@@ -58,6 +58,7 @@ func newDependencyBuildCmd(out io.Writer) *cobra.Command {
 				Out:              out,
 				ChartPath:        chartpath,
 				Keyring:          client.Keyring,
+				SkipUpdate:       client.SkipRefresh,
 				Getters:          getter.All(settings),
 				RepositoryConfig: settings.RepositoryConfig,
 				RepositoryCache:  settings.RepositoryCache,
@@ -77,6 +78,7 @@ func newDependencyBuildCmd(out io.Writer) *cobra.Command {
 	f := cmd.Flags()
 	f.BoolVar(&client.Verify, "verify", false, "verify the packages against signatures")
 	f.StringVar(&client.Keyring, "keyring", defaultKeyring(), "keyring containing public keys")
+	f.BoolVar(&client.SkipRefresh, "skip-refresh", false, "do not refresh the local repository cache")
 
 	return cmd
 }
diff --git a/cmd/helm/get.go b/cmd/helm/get.go
index e94871c7b..7c4854b59 100644
--- a/cmd/helm/get.go
+++ b/cmd/helm/get.go
@@ -37,11 +37,10 @@ get extended information about the release, including:
 
 func newGetCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:               "get",
-		Short:             "download extended information of a named release",
-		Long:              getHelp,
-		Args:              require.NoArgs,
-		ValidArgsFunction: noCompletions, // Disable file completion
+		Use:   "get",
+		Short: "download extended information of a named release",
+		Long:  getHelp,
+		Args:  require.NoArgs,
 	}
 
 	cmd.AddCommand(newGetAllCmd(cfg, out))
diff --git a/cmd/helm/plugin.go b/cmd/helm/plugin.go
index a118a03eb..8e1044f54 100644
--- a/cmd/helm/plugin.go
+++ b/cmd/helm/plugin.go
@@ -32,10 +32,9 @@ Manage client-side Helm plugins.
 
 func newPluginCmd(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:               "plugin",
-		Short:             "install, list, or uninstall Helm plugins",
-		Long:              pluginHelp,
-		ValidArgsFunction: noCompletions, // Disable file completion
+		Use:   "plugin",
+		Short: "install, list, or uninstall Helm plugins",
+		Long:  pluginHelp,
 	}
 	cmd.AddCommand(
 		newPluginInstallCmd(out),
diff --git a/cmd/helm/repo.go b/cmd/helm/repo.go
index 5aac38819..ad6ceaa8f 100644
--- a/cmd/helm/repo.go
+++ b/cmd/helm/repo.go
@@ -34,11 +34,10 @@ It can be used to add, remove, list, and index chart repositories.
 
 func newRepoCmd(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:               "repo add|remove|list|index|update [ARGS]",
-		Short:             "add, list, remove, update, and index chart repositories",
-		Long:              repoHelm,
-		Args:              require.NoArgs,
-		ValidArgsFunction: noCompletions, // Disable file completion
+		Use:   "repo add|remove|list|index|update [ARGS]",
+		Short: "add, list, remove, update, and index chart repositories",
+		Long:  repoHelm,
+		Args:  require.NoArgs,
 	}
 
 	cmd.AddCommand(newRepoAddCmd(out))
diff --git a/cmd/helm/root.go b/cmd/helm/root.go
index fced43743..75742ca4a 100644
--- a/cmd/helm/root.go
+++ b/cmd/helm/root.go
@@ -88,9 +88,6 @@ func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string
 		Short:        "The Helm package manager for Kubernetes.",
 		Long:         globalUsage,
 		SilenceUsage: true,
-		// This breaks completion for 'helm help <TAB>'
-		// The Cobra release following 1.0 will fix this
-		//ValidArgsFunction: noCompletions, // Disable file completion
 	}
 	flags := cmd.PersistentFlags()
 
diff --git a/cmd/helm/search.go b/cmd/helm/search.go
index 44c8d64e3..240d5e7c7 100644
--- a/cmd/helm/search.go
+++ b/cmd/helm/search.go
@@ -31,10 +31,9 @@ search subcommands to search different locations for charts.
 func newSearchCmd(out io.Writer) *cobra.Command {
 
 	cmd := &cobra.Command{
-		Use:               "search [keyword]",
-		Short:             "search for a keyword in charts",
-		Long:              searchDesc,
-		ValidArgsFunction: noCompletions, // Disable file completion
+		Use:   "search [keyword]",
+		Short: "search for a keyword in charts",
+		Long:  searchDesc,
 	}
 
 	cmd.AddCommand(newSearchHubCmd(out))
diff --git a/go.mod b/go.mod
index 9afbca7b9..a7f92b37d 100644
--- a/go.mod
+++ b/go.mod
@@ -35,13 +35,13 @@ require (
 	github.com/stretchr/testify v1.6.1
 	github.com/xeipuuv/gojsonschema v1.2.0
 	golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9
-	k8s.io/api v0.19.2
-	k8s.io/apiextensions-apiserver v0.19.2
-	k8s.io/apimachinery v0.19.2
-	k8s.io/cli-runtime v0.19.2
-	k8s.io/client-go v0.19.2
+	k8s.io/api v0.19.3
+	k8s.io/apiextensions-apiserver v0.19.3 
+	k8s.io/apimachinery v0.19.3
+	k8s.io/cli-runtime v0.19.3 
+	k8s.io/client-go v0.19.3 
 	k8s.io/klog v1.0.0
-	k8s.io/kubectl v0.19.2
+	k8s.io/kubectl v0.19.3
 	sigs.k8s.io/yaml v1.2.0
 )
 
diff --git a/go.sum b/go.sum
index 1c99059e3..cfa9efc3a 100644
--- a/go.sum
+++ b/go.sum
@@ -1056,18 +1056,32 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
 k8s.io/api v0.19.2 h1:q+/krnHWKsL7OBZg/rxnycsl9569Pud76UJ77MvKXms=
 k8s.io/api v0.19.2/go.mod h1:IQpK0zFQ1xc5iNIQPqzgoOwuFugaYHK4iCknlAQP9nI=
+k8s.io/api v0.19.3 h1:GN6ntFnv44Vptj/b+OnMW7FmzkpDoIDLZRvKX3XH9aU=
+k8s.io/api v0.19.3/go.mod h1:VF+5FT1B74Pw3KxMdKyinLo+zynBaMBiAfGMuldcNDs=
 k8s.io/apiextensions-apiserver v0.19.2 h1:oG84UwiDsVDu7dlsGQs5GySmQHCzMhknfhFExJMz9tA=
 k8s.io/apiextensions-apiserver v0.19.2/go.mod h1:EYNjpqIAvNZe+svXVx9j4uBaVhTB4C94HkY3w058qcg=
+k8s.io/apiextensions-apiserver v0.19.3 h1:WZxBypSHW4SdXHbdPTS/Jy7L2la6Niggs8BuU5o+avo=
+k8s.io/apiextensions-apiserver v0.19.3/go.mod h1:igVEkrE9TzInc1tYE7qSqxaLg/rEAp6B5+k9Q7+IC8Q=
 k8s.io/apimachinery v0.19.2 h1:5Gy9vQpAGTKHPVOh5c4plE274X8D/6cuEiTO2zve7tc=
 k8s.io/apimachinery v0.19.2/go.mod h1:DnPGDnARWFvYa3pMHgSxtbZb7gpzzAZ1pTfaUNDVlmA=
+k8s.io/apimachinery v0.19.3 h1:bpIQXlKjB4cB/oNpnNnV+BybGPR7iP5oYpsOTEJ4hgc=
+k8s.io/apimachinery v0.19.3/go.mod h1:DnPGDnARWFvYa3pMHgSxtbZb7gpzzAZ1pTfaUNDVlmA=
 k8s.io/apiserver v0.19.2/go.mod h1:FreAq0bJ2vtZFj9Ago/X0oNGC51GfubKK/ViOKfVAOA=
+k8s.io/apiserver v0.19.3/go.mod h1:bx6dMm+H6ifgKFpCQT/SAhPwhzoeIMlHIaibomUDec0=
 k8s.io/cli-runtime v0.19.2 h1:d4uOtKhy3ImdaKqZJ8yQgLrdtUwsJLfP4Dw7L/kVPOo=
 k8s.io/cli-runtime v0.19.2/go.mod h1:CMynmJM4Yf02TlkbhKxoSzi4Zf518PukJ5xep/NaNeY=
+k8s.io/cli-runtime v0.19.3 h1:vZUTphJIvlh7+867cXiLmyzoCAuQdukbPLIad6eEajQ=
+k8s.io/cli-runtime v0.19.3/go.mod h1:q+l845i5/uWzcUpCrl+L4f3XLaJi8ZeLVQ/decwty0A=
 k8s.io/client-go v0.19.2 h1:gMJuU3xJZs86L1oQ99R4EViAADUPMHHtS9jFshasHSc=
 k8s.io/client-go v0.19.2/go.mod h1:S5wPhCqyDNAlzM9CnEdgTGV4OqhsW3jGO1UM1epwfJA=
+k8s.io/client-go v0.19.3 h1:ctqR1nQ52NUs6LpI0w+a5U+xjYwflFwA13OJKcicMxg=
+k8s.io/client-go v0.19.3/go.mod h1:+eEMktZM+MG0KO+PTkci8xnbCZHvj9TqR6Q1XDUIJOM=
 k8s.io/code-generator v0.19.2/go.mod h1:moqLn7w0t9cMs4+5CQyxnfA/HV8MF6aAVENF+WZZhgk=
+k8s.io/code-generator v0.19.3/go.mod h1:moqLn7w0t9cMs4+5CQyxnfA/HV8MF6aAVENF+WZZhgk=
 k8s.io/component-base v0.19.2 h1:jW5Y9RcZTb79liEhW3XDVTW7MuvEGP0tQZnfSX6/+gs=
 k8s.io/component-base v0.19.2/go.mod h1:g5LrsiTiabMLZ40AR6Hl45f088DevyGY+cCE2agEIVo=
+k8s.io/component-base v0.19.3 h1:c+DzDNAQFlaoyX+yv8YuWi8xmlQvvY5DnJGbaz5U74o=
+k8s.io/component-base v0.19.3/go.mod h1:WhLWSIefQn8W8jxSLl5WNiR6z8oyMe/8Zywg7alOkRc=
 k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0=
 k8s.io/gengo v0.0.0-20200428234225-8167cfdcfc14/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0=
 k8s.io/klog v1.0.0 h1:Pt+yjF5aB1xDSVbau4VsWe+dQNzA0qv1LlXdC2dF6Q8=
@@ -1079,8 +1093,11 @@ k8s.io/kube-openapi v0.0.0-20200805222855-6aeccd4b50c6 h1:+WnxoVtG8TMiudHBSEtrVL
 k8s.io/kube-openapi v0.0.0-20200805222855-6aeccd4b50c6/go.mod h1:UuqjUnNftUyPE5H64/qeyjQoUZhGpeFDVdxjTeEVN2o=
 k8s.io/kubectl v0.19.2 h1:/Dxz9u7S0GnchLA6Avqi5k1qhZH4Fusgecj8dHsSnbk=
 k8s.io/kubectl v0.19.2/go.mod h1:4ib3oj5ma6gF95QukTvC7ZBMxp60+UEAhDPjLuBIrV4=
+k8s.io/kubectl v0.19.3 h1:T8IHHpg+uRIfn34wqJ8wHG5bbH+VV5FNPtJ+jKcho1U=
+k8s.io/kubectl v0.19.3/go.mod h1:t5cscfrAuHUvEGNyNJjPKt+rGlaJzk8jrKYHXxEsANE=
 k8s.io/kubernetes v1.13.0/go.mod h1:ocZa8+6APFNC2tX1DZASIbocyYT5jHzqFVsY5aoB7Jk=
 k8s.io/metrics v0.19.2/go.mod h1:IlLaAGXN0q7yrtB+SV0q3JIraf6VtlDr+iuTcX21fCU=
+k8s.io/metrics v0.19.3/go.mod h1:Eap/Lk1FiAIjkaArFuv41v+ph6dbDpVGwAg7jMI+4vg=
 k8s.io/utils v0.0.0-20200729134348-d5654de09c73 h1:uJmqzgNWG7XyClnU/mLPBWwfKKF1K8Hf8whTseBgJcg=
 k8s.io/utils v0.0.0-20200729134348-d5654de09c73/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
 rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
diff --git a/pkg/chart/loader/load.go b/pkg/chart/loader/load.go
index 6ed3b4187..7cc8878a8 100644
--- a/pkg/chart/loader/load.go
+++ b/pkg/chart/loader/load.go
@@ -73,10 +73,11 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) {
 	c := new(chart.Chart)
 	subcharts := make(map[string][]*BufferedFile)
 
+	// do not rely on assumed ordering of files in the chart and crash
+	// if Chart.yaml was not coming early enough to initialize metadata
 	for _, f := range files {
 		c.Raw = append(c.Raw, &chart.File{Name: f.Name, Data: f.Data})
-		switch {
-		case f.Name == "Chart.yaml":
+		if f.Name == "Chart.yaml" {
 			if c.Metadata == nil {
 				c.Metadata = new(chart.Metadata)
 			}
@@ -89,6 +90,13 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) {
 			if c.Metadata.APIVersion == "" {
 				c.Metadata.APIVersion = chart.APIVersionV1
 			}
+		}
+	}
+	for _, f := range files {
+		switch {
+		case f.Name == "Chart.yaml":
+			// already processed
+			continue
 		case f.Name == "Chart.lock":
 			c.Lock = new(chart.Lock)
 			if err := yaml.Unmarshal(f.Data, &c.Lock); err != nil {


Test patch:
diff --git a/cmd/helm/dependency_build_test.go b/cmd/helm/dependency_build_test.go
index d6dfdabcb..8e5f24af7 100644
--- a/cmd/helm/dependency_build_test.go
+++ b/cmd/helm/dependency_build_test.go
@@ -99,6 +99,19 @@ func TestDependencyBuildCmd(t *testing.T) {
 	if v := reqver.Version; v != "0.1.0" {
 		t.Errorf("mismatched versions. Expected %q, got %q", "0.1.0", v)
 	}
+
+	skipRefreshCmd := fmt.Sprintf("dependency build '%s' --skip-refresh --repository-config %s --repository-cache %s", filepath.Join(rootDir, chartname), repoFile, rootDir)
+	_, out, err = executeActionCommand(skipRefreshCmd)
+
+	// In this pass, we check --skip-refresh option becomes effective.
+	if err != nil {
+		t.Logf("Output: %s", out)
+		t.Fatal(err)
+	}
+
+	if strings.Contains(out, `update from the "test" chart repository`) {
+		t.Errorf("Repo did get updated\n%s", out)
+	}
 }
 
 func TestDependencyBuildCmdWithHelmV2Hash(t *testing.T) {
diff --git a/pkg/chart/loader/load_test.go b/pkg/chart/loader/load_test.go
index 3819161d2..9605b3152 100644
--- a/pkg/chart/loader/load_test.go
+++ b/pkg/chart/loader/load_test.go
@@ -22,6 +22,7 @@ import (
 	"compress/gzip"
 	"io"
 	"io/ioutil"
+	"log"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -306,6 +307,76 @@ icon: https://example.com/64x64.png
 	}
 }
 
+// Test the order of file loading. The Chart.yaml file needs to come first for
+// later comparison checks. See https://github.com/helm/helm/pull/8948
+func TestLoadFilesOrder(t *testing.T) {
+	goodFiles := []*BufferedFile{
+		{
+			Name: "requirements.yaml",
+			Data: []byte("dependencies:"),
+		},
+		{
+			Name: "values.yaml",
+			Data: []byte("var: some values"),
+		},
+
+		{
+			Name: "templates/deployment.yaml",
+			Data: []byte("some deployment"),
+		},
+		{
+			Name: "templates/service.yaml",
+			Data: []byte("some service"),
+		},
+		{
+			Name: "Chart.yaml",
+			Data: []byte(`apiVersion: v1
+name: frobnitz
+description: This is a frobnitz.
+version: "1.2.3"
+keywords:
+  - frobnitz
+  - sprocket
+  - dodad
+maintainers:
+  - name: The Helm Team
+    email: helm@example.com
+  - name: Someone Else
+    email: nobody@example.com
+sources:
+  - https://example.com/foo/bar
+home: http://example.com
+icon: https://example.com/64x64.png
+`),
+		},
+	}
+
+	// Capture stderr to make sure message about Chart.yaml handle dependencies
+	// is not present
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("Unable to create pipe: %s", err)
+	}
+	stderr := log.Writer()
+	log.SetOutput(w)
+	defer func() {
+		log.SetOutput(stderr)
+	}()
+
+	_, err = LoadFiles(goodFiles)
+	if err != nil {
+		t.Errorf("Expected good files to be loaded, got %v", err)
+	}
+	w.Close()
+
+	var text bytes.Buffer
+	io.Copy(&text, r)
+	if text.String() != "" {
+		t.Errorf("Expected no message to Stderr, got %s", text.String())
+	}
+
+}
+
 // Packaging the chart on a Windows machine will produce an
 // archive that has \\ as delimiters. Test that we support these archives
 func TestLoadFileBackslash(t *testing.T) {