Repository: helm/helm
Pull request #7466 (2020-01-25 18:44:27)

fix(comp): Allow zsh completion to handle -n flag
I ran into this while adding more tests to the acceptance-testing repo.

When doing zsh completion, the `-n` flag would not be handled properly.
Doing
```
helm -n<TAB>
```
would not add the space after the `-n`.
This was caused by the fact that the `-n` flag was being swallowed by the `echo` command.

To fix this, we use `printf` instead.

This has been regression-tested with the acceptance-testing repo


Patch:
diff --git a/SECURITY.md b/SECURITY.md
new file mode 100644
index 000000000..c84a6f866
--- /dev/null
+++ b/SECURITY.md
@@ -0,0 +1,3 @@
+# Helm Security Reporting and Policy
+
+The Helm project has [a common process and policy that can be found here](https://github.com/helm/community/blob/master/SECURITY.md).
\ No newline at end of file
diff --git a/cmd/helm/completion.go b/cmd/helm/completion.go
index a44140d9f..1601cb448 100644
--- a/cmd/helm/completion.go
+++ b/cmd/helm/completion.go
@@ -139,7 +139,10 @@ __helm_compgen() {
 	fi
 	for w in "${completions[@]}"; do
 		if [[ "${w}" = "$1"* ]]; then
-			echo "${w}"
+			# Use printf instead of echo beause it is possible that
+			# the value to print is -n, which would be interpreted
+			# as a flag to echo
+			printf "%s\n" "${w}"
 		fi
 	done
 }
diff --git a/cmd/helm/package.go b/cmd/helm/package.go
index 20cde6e4c..00fe0ef11 100644
--- a/cmd/helm/package.go
+++ b/cmd/helm/package.go
@@ -118,7 +118,6 @@ func newPackageCmd(out io.Writer) *cobra.Command {
 	f.StringVar(&client.AppVersion, "app-version", "", "set the appVersion on the chart to this version")
 	f.StringVarP(&client.Destination, "destination", "d", ".", "location to write the chart.")
 	f.BoolVarP(&client.DependencyUpdate, "dependency-update", "u", false, `update dependencies from "Chart.yaml" to dir "charts/" before packaging`)
-	addValueOptionsFlags(f, valueOpts)
 
 	return cmd
 }
diff --git a/pkg/action/package.go b/pkg/action/package.go
index 5c85ebe0d..b48fc65f0 100644
--- a/pkg/action/package.go
+++ b/pkg/action/package.go
@@ -60,12 +60,6 @@ func (p *Package) Run(path string, vals map[string]interface{}) (string, error)
 		return "", err
 	}
 
-	combinedVals, err := chartutil.CoalesceValues(ch, vals)
-	if err != nil {
-		return "", err
-	}
-	ch.Values = combinedVals
-
 	// If version is set, modify the version.
 	if p.Version != "" {
 		if err := setVersion(ch, p.Version); err != nil {
diff --git a/pkg/chartutil/create.go b/pkg/chartutil/create.go
index 390f12f4c..496f20166 100644
--- a/pkg/chartutil/create.go
+++ b/pkg/chartutil/create.go
@@ -304,7 +304,7 @@ kind: ServiceAccount
 metadata:
   name: {{ include "<CHARTNAME>.serviceAccountName" . }}
   labels:
-{{ include "<CHARTNAME>.labels" . | nindent 4 }}
+    {{- include "<CHARTNAME>.labels" . | nindent 4 }}
   {{- with .Values.serviceAccount.annotations }}
   annotations:
     {{- toYaml . | nindent 4 }}
@@ -405,7 +405,7 @@ kind: Pod
 metadata:
   name: "{{ include "<CHARTNAME>.fullname" . }}-test-connection"
   labels:
-{{ include "<CHARTNAME>.labels" . | nindent 4 }}
+    {{- include "<CHARTNAME>.labels" . | nindent 4 }}
   annotations:
     "helm.sh/hook": test-success
 spec:
@@ -413,7 +413,7 @@ spec:
     - name: wget
       image: busybox
       command: ['wget']
-      args:  ['{{ include "<CHARTNAME>.fullname" . }}:{{ .Values.service.port }}']
+      args: ['{{ include "<CHARTNAME>.fullname" . }}:{{ .Values.service.port }}']
   restartPolicy: Never
 `
 
diff --git a/pkg/chartutil/save.go b/pkg/chartutil/save.go
index fb985bb59..be0dfdc24 100644
--- a/pkg/chartutil/save.go
+++ b/pkg/chartutil/save.go
@@ -50,11 +50,12 @@ func SaveDir(c *chart.Chart, dest string) error {
 	}
 
 	// Save values.yaml
-	if c.Values != nil {
-		vf := filepath.Join(outdir, ValuesfileName)
-		b, _ := yaml.Marshal(c.Values)
-		if err := writeFile(vf, b); err != nil {
-			return err
+	for _, f := range c.Raw {
+		if f.Name == ValuesfileName {
+			vf := filepath.Join(outdir, ValuesfileName)
+			if err := writeFile(vf, f.Data); err != nil {
+				return err
+			}
 		}
 	}
 
@@ -161,12 +162,12 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
 	}
 
 	// Save values.yaml
-	ydata, err := yaml.Marshal(c.Values)
-	if err != nil {
-		return err
-	}
-	if err := writeToTar(out, filepath.Join(base, ValuesfileName), ydata); err != nil {
-		return err
+	for _, f := range c.Raw {
+		if f.Name == ValuesfileName {
+			if err := writeToTar(out, filepath.Join(base, ValuesfileName), f.Data); err != nil {
+				return err
+			}
+		}
 	}
 
 	// Save values.schema.json if it exists
diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go
index 1d98cd763..490561037 100644
--- a/pkg/engine/engine.go
+++ b/pkg/engine/engine.go
@@ -95,6 +95,7 @@ type renderable struct {
 
 const warnStartDelim = "HELM_ERR_START"
 const warnEndDelim = "HELM_ERR_END"
+const recursionMaxNums = 1000
 
 var warnRegex = regexp.MustCompile(warnStartDelim + `(.*)` + warnEndDelim)
 
@@ -105,19 +106,20 @@ func warnWrap(warn string) string {
 // initFunMap creates the Engine's FuncMap and adds context-specific functions.
 func (e Engine) initFunMap(t *template.Template, referenceTpls map[string]renderable) {
 	funcMap := funcMap()
-	includedNames := make([]string, 0)
+	includedNames := make(map[string]int)
 
 	// Add the 'include' function here so we can close over t.
 	funcMap["include"] = func(name string, data interface{}) (string, error) {
 		var buf strings.Builder
-		for _, n := range includedNames {
-			if n == name {
+		if v, ok := includedNames[name]; ok {
+			if v > recursionMaxNums {
 				return "", errors.Wrapf(fmt.Errorf("unable to execute template"), "rendering template has a nested reference name: %s", name)
 			}
+			includedNames[name]++
+		} else {
+			includedNames[name] = 1
 		}
-		includedNames = append(includedNames, name)
 		err := t.ExecuteTemplate(&buf, name, data)
-		includedNames = includedNames[:len(includedNames)-1]
 		return buf.String(), err
 	}
 


Test patch:
diff --git a/cmd/helm/package_test.go b/cmd/helm/package_test.go
index 38938b4af..e0a5fabd6 100644
--- a/cmd/helm/package_test.go
+++ b/cmd/helm/package_test.go
@@ -17,13 +17,9 @@ package main
 
 import (
 	"bytes"
-	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"regexp"
-	"runtime"
-	"strings"
 	"testing"
 
 	"github.com/spf13/cobra"
@@ -31,15 +27,9 @@ import (
 	"helm.sh/helm/v3/internal/test/ensure"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chart/loader"
-	"helm.sh/helm/v3/pkg/chartutil"
 )
 
 func TestPackage(t *testing.T) {
-	statFileMsg := "no such file or directory"
-	if runtime.GOOS == "windows" {
-		statFileMsg = "The system cannot find the file specified."
-	}
-
 	tests := []struct {
 		name    string
 		flags   map[string]string
@@ -108,13 +98,6 @@ func TestPackage(t *testing.T) {
 			hasfile: "chart-missing-deps-0.1.0.tgz",
 			err:     true,
 		},
-		{
-			name:   "package --values does-not-exist",
-			args:   []string{"testdata/testcharts/alpine"},
-			flags:  map[string]string{"values": "does-not-exist"},
-			expect: fmt.Sprintf("does-not-exist: %s", statFileMsg),
-			err:    true,
-		},
 		{
 			name: "package testdata/testcharts/chart-bad-type",
 			args: []string{"testdata/testcharts/chart-bad-type"},
@@ -213,159 +196,6 @@ func TestSetAppVersion(t *testing.T) {
 	}
 }
 
-func TestPackageValues(t *testing.T) {
-	defer resetEnv()()
-
-	repoFile := "testdata/helmhome/helm/repositories.yaml"
-
-	testCases := []struct {
-		desc               string
-		args               []string
-		valuefilesContents []string
-		flags              map[string]string
-		expected           []string
-	}{
-		{
-			desc:               "helm package, single values file",
-			args:               []string{"testdata/testcharts/alpine"},
-			flags:              map[string]string{"repository-config": repoFile},
-			valuefilesContents: []string{"Name: chart-name-foo"},
-			expected:           []string{"Name: chart-name-foo"},
-		},
-		{
-			desc:               "helm package, multiple values files",
-			args:               []string{"testdata/testcharts/alpine"},
-			flags:              map[string]string{"repository-config": repoFile},
-			valuefilesContents: []string{"Name: chart-name-foo", "foo: bar"},
-			expected:           []string{"Name: chart-name-foo", "foo: bar"},
-		},
-		{
-			desc:     "helm package, with set option",
-			args:     []string{"testdata/testcharts/alpine"},
-			flags:    map[string]string{"set": "Name=chart-name-foo", "repository-config": repoFile},
-			expected: []string{"Name: chart-name-foo"},
-		},
-		{
-			desc:               "helm package, set takes precedence over value file",
-			args:               []string{"testdata/testcharts/alpine"},
-			valuefilesContents: []string{"Name: chart-name-foo"},
-			flags:              map[string]string{"set": "Name=chart-name-bar", "repository-config": repoFile},
-			expected:           []string{"Name: chart-name-bar"},
-		},
-	}
-
-	for _, tc := range testCases {
-		var files []string
-		for _, contents := range tc.valuefilesContents {
-			f := createValuesFile(t, contents)
-			files = append(files, f)
-		}
-		valueFiles := strings.Join(files, ",")
-
-		expected, err := chartutil.ReadValues([]byte(strings.Join(tc.expected, "\n")))
-		if err != nil {
-			t.Errorf("unexpected error parsing values: %q", err)
-		}
-
-		outputDir := ensure.TempDir(t)
-
-		if len(tc.flags) == 0 {
-			tc.flags = make(map[string]string)
-		}
-		tc.flags["destination"] = outputDir
-
-		if len(valueFiles) > 0 {
-			tc.flags["values"] = valueFiles
-		}
-
-		cmd := newPackageCmd(&bytes.Buffer{})
-		setFlags(cmd, tc.flags)
-		if err := cmd.RunE(cmd, tc.args); err != nil {
-			t.Fatalf("unexpected error: %q", err)
-		}
-
-		outputFile := filepath.Join(outputDir, "alpine-0.1.0.tgz")
-		verifyOutputChartExists(t, outputFile)
-
-		actual, err := getChartValues(outputFile)
-		if err != nil {
-			t.Fatalf("unexpected error extracting chart values: %q", err)
-		}
-
-		verifyValues(t, actual, expected)
-	}
-}
-
-func TestNonExistentDirAndBadPermission(t *testing.T) {
-	nonExistentDir := "testdata/testcharts/non-existent-directory"
-
-	tests := []struct {
-		name    string
-		flags   map[string]string
-		args    []string
-		expect  string
-		hasfile string
-		err     bool
-	}{
-		{
-			name:   "package testdata/testcharts/non-existent-directory",
-			args:   []string{"testdata/testcharts/non-existent-directory"},
-			expect: fmt.Sprintf("stat %s: no such file or directory", nonExistentDir),
-			err:    true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			var buf bytes.Buffer
-			c := newPackageCmd(&buf)
-			re := regexp.MustCompile(tt.expect)
-			err := c.RunE(c, tt.args)
-			if err != nil {
-				if tt.err && re.MatchString(err.Error()) {
-					return
-				}
-				t.Fatalf("%q: expected error %q, got %q", tt.name, tt.expect, err)
-			}
-		})
-	}
-}
-
-func createValuesFile(t *testing.T, data string) string {
-	outputDir := ensure.TempDir(t)
-
-	outputFile := filepath.Join(outputDir, "values.yaml")
-	if err := ioutil.WriteFile(outputFile, []byte(data), 0644); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	return outputFile
-}
-
-func getChartValues(chartPath string) (chartutil.Values, error) {
-	chart, err := loader.Load(chartPath)
-	if err != nil {
-		return nil, err
-	}
-	return chart.Values, nil
-}
-
-func verifyValues(t *testing.T, actual, expected chartutil.Values) {
-	t.Helper()
-	for key, value := range expected.AsMap() {
-		if got := actual[key]; got != value {
-			t.Errorf("Expected %q, got %q (%v)", value, got, actual)
-		}
-	}
-}
-
-func verifyOutputChartExists(t *testing.T, chartPath string) {
-	if chartFile, err := os.Stat(chartPath); err != nil {
-		t.Errorf("expected file %q, got err %q", chartPath, err)
-	} else if chartFile.Size() == 0 {
-		t.Errorf("file %q has zero bytes.", chartPath)
-	}
-}
-
 func setFlags(cmd *cobra.Command, flags map[string]string) {
 	dest := cmd.Flags()
 	for f, v := range flags {