Merge remote-tracking branch 'helm/master'

.github/pull_request_template.md

@@ -0,0 +1,12 @@
+<!--  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**:
+
+**Special notes for your reviewer**:
+
+**If applicable**:
+- [ ] this PR contains documentation
+- [ ] this PR contains unit tests
+- [ ] this PR has been tested for backwards compatibility

CONTRIBUTING.md

@@ -184,9 +184,9 @@ contributing to Helm. All issue types follow the same general lifecycle. Differe
 
 ## How to Contribute a Patch
 
-1. If you haven't already done so, sign a Contributor License Agreement (see details above).
-2. Fork the desired repo, develop and test your code changes.
-3. Submit a pull request.
+1. Identify or create the related issue.
+2. Fork the desired repo; develop and test your code changes.
+3. Submit a pull request, making sure to sign your work and link the related issue.
 
 Coding conventions and standards are explained in the [official developer docs](https://helm.sh/docs/developers/).
 

cmd/helm/repo_list.go

@@ -38,7 +38,7 @@ func newRepoListCmd(out io.Writer) *cobra.Command {
 		Args:    require.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			f, err := repo.LoadFile(settings.RepositoryConfig)
-			if isNotExist(err) || len(f.Repositories) == 0 {
+			if isNotExist(err) || (len(f.Repositories) == 0 && !(outfmt == output.JSON || outfmt == output.YAML)) {
 				return errors.New("no repositories to show")
 			}
 

internal/ignore/rules.go

@@ -18,6 +18,7 @@ package ignore
 
 import (
 	"bufio"
+	"bytes"
 	"io"
 	"log"
 	"os"
@@ -65,8 +66,18 @@ func Parse(file io.Reader) (*Rules, error) {
 	r := &Rules{patterns: []*pattern{}}
 
 	s := bufio.NewScanner(file)
+	currentLine := 0
+	utf8bom := []byte{0xEF, 0xBB, 0xBF}
 	for s.Scan() {
-		if err := r.parseRule(s.Text()); err != nil {
+		scannedBytes := s.Bytes()
+		// We trim UTF8 BOM
+		if currentLine == 0 {
+			scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom)
+		}
+		line := string(scannedBytes)
+		currentLine++
+
+		if err := r.parseRule(line); err != nil {
 			return r, err
 		}
 	}

pkg/chart/chart_test.go

@@ -159,3 +159,53 @@ func TestChartFullPath(t *testing.T) {
 	is.Equal("foo/charts/", chrt1.ChartFullPath())
 	is.Equal("foo", chrt2.ChartFullPath())
 }
+
+func TestCRDObjects(t *testing.T) {
+	chrt := Chart{
+		Files: []*File{
+			{
+				Name: "crds/foo.yaml",
+				Data: []byte("hello"),
+			},
+			{
+				Name: "bar.yaml",
+				Data: []byte("hello"),
+			},
+			{
+				Name: "crds/foo/bar/baz.yaml",
+				Data: []byte("hello"),
+			},
+			{
+				Name: "crdsfoo/bar/baz.yaml",
+				Data: []byte("hello"),
+			},
+			{
+				Name: "crds/README.md",
+				Data: []byte("# hello"),
+			},
+		},
+	}
+
+	expected := []CRD{
+		{
+			Name:     "crds/foo.yaml",
+			Filename: "crds/foo.yaml",
+			File: &File{
+				Name: "crds/foo.yaml",
+				Data: []byte("hello"),
+			},
+		},
+		{
+			Name:     "crds/foo/bar/baz.yaml",
+			Filename: "crds/foo/bar/baz.yaml",
+			File: &File{
+				Name: "crds/foo/bar/baz.yaml",
+				Data: []byte("hello"),
+			},
+		},
+	}
+
+	is := assert.New(t)
+	crds := chrt.CRDObjects()
+	is.Equal(expected, crds)
+}

pkg/chart/loader/archive.go

@@ -173,7 +173,9 @@ func LoadArchiveFiles(in io.Reader) ([]*BufferedFile, error) {
 			return nil, err
 		}
 
-		files = append(files, &BufferedFile{Name: n, Data: b.Bytes()})
+		data := bytes.TrimPrefix(b.Bytes(), utf8bom)
+
+		files = append(files, &BufferedFile{Name: n, Data: data})
 		b.Reset()
 	}
 

pkg/chart/loader/directory.go

@@ -17,6 +17,7 @@ limitations under the License.
 package loader
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -30,6 +31,8 @@ import (
 	"helm.sh/helm/v3/pkg/chart"
 )
 
+var utf8bom = []byte{0xEF, 0xBB, 0xBF}
+
 // DirLoader loads a chart from a directory
 type DirLoader string
 
@@ -104,6 +107,8 @@ func LoadDir(dir string) (*chart.Chart, error) {
 			return errors.Wrapf(err, "error reading %s", n)
 		}
 
+		data = bytes.TrimPrefix(data, utf8bom)
+
 		files = append(files, &BufferedFile{Name: n, Data: data})
 		return nil
 	}

pkg/chart/loader/load_test.go

@@ -20,6 +20,7 @@ import (
 	"archive/tar"
 	"bytes"
 	"compress/gzip"
+	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -85,6 +86,86 @@ func TestLoadDirWithSymlink(t *testing.T) {
 	verifyDependenciesLock(t, c)
 }
 
+func TestBomTestData(t *testing.T) {
+	testFiles := []string{"frobnitz_with_bom/.helmignore", "frobnitz_with_bom/templates/template.tpl", "frobnitz_with_bom/Chart.yaml"}
+	for _, file := range testFiles {
+		data, err := ioutil.ReadFile("testdata/" + file)
+		if err != nil || !bytes.HasPrefix(data, utf8bom) {
+			t.Errorf("Test file has no BOM or is invalid: testdata/%s", file)
+		}
+	}
+
+	archive, err := ioutil.ReadFile("testdata/frobnitz_with_bom.tgz")
+	if err != nil {
+		t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err)
+	}
+	unzipped, err := gzip.NewReader(bytes.NewReader(archive))
+	if err != nil {
+		t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err)
+	}
+	defer unzipped.Close()
+	for _, testFile := range testFiles {
+		data := make([]byte, 3)
+		err := unzipped.Reset(bytes.NewReader(archive))
+		if err != nil {
+			t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err)
+		}
+		tr := tar.NewReader(unzipped)
+		for {
+			file, err := tr.Next()
+			if err == io.EOF {
+				break
+			}
+			if err != nil {
+				t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err)
+			}
+			if file != nil && strings.EqualFold(file.Name, testFile) {
+				_, err := tr.Read(data)
+				if err != nil {
+					t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err)
+				} else {
+					break
+				}
+			}
+		}
+		if !bytes.Equal(data, utf8bom) {
+			t.Fatalf("Test file has no BOM or is invalid: frobnitz_with_bom.tgz/%s", testFile)
+		}
+	}
+}
+
+func TestLoadDirWithUTFBOM(t *testing.T) {
+	l, err := Loader("testdata/frobnitz_with_bom")
+	if err != nil {
+		t.Fatalf("Failed to load testdata: %s", err)
+	}
+	c, err := l.Load()
+	if err != nil {
+		t.Fatalf("Failed to load testdata: %s", err)
+	}
+	verifyFrobnitz(t, c)
+	verifyChart(t, c)
+	verifyDependencies(t, c)
+	verifyDependenciesLock(t, c)
+	verifyBomStripped(t, c.Files)
+}
+
+func TestLoadArchiveWithUTFBOM(t *testing.T) {
+	l, err := Loader("testdata/frobnitz_with_bom.tgz")
+	if err != nil {
+		t.Fatalf("Failed to load testdata: %s", err)
+	}
+	c, err := l.Load()
+	if err != nil {
+		t.Fatalf("Failed to load testdata: %s", err)
+	}
+	verifyFrobnitz(t, c)
+	verifyChart(t, c)
+	verifyDependencies(t, c)
+	verifyDependenciesLock(t, c)
+	verifyBomStripped(t, c.Files)
+}
+
 func TestLoadV1(t *testing.T) {
 	l, err := Loader("testdata/frobnitz.v1")
 	if err != nil {
@@ -465,3 +546,11 @@ func verifyChartFileAndTemplate(t *testing.T, c *chart.Chart, name string) {
 		}
 	}
 }
+
+func verifyBomStripped(t *testing.T, files []*chart.File) {
+	for _, file := range files {
+		if bytes.HasPrefix(file.Data, utf8bom) {
+			t.Errorf("Byte Order Mark still present in processed file %s", file.Name)
+		}
+	}
+}

pkg/chart/loader/testdata/frobnitz_with_bom/.helmignore

@@ -0,0 +1 @@
+ignore/

pkg/chart/loader/testdata/frobnitz_with_bom/Chart.lock

@@ -0,0 +1,8 @@
+dependencies:
+  - name: alpine
+    version: "0.1.0"
+    repository: https://example.com/charts
+  - name: mariner
+    version: "4.3.2"
+    repository: https://example.com/charts
+digest: invalid

pkg/chart/loader/testdata/frobnitz_with_bom/Chart.yaml

@@ -0,0 +1,27 @@
+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
+annotations:
+  extrakey: extravalue
+  anotherkey: anothervalue
+dependencies:
+  - name: alpine
+    version: "0.1.0"
+    repository: https://example.com/charts
+  - name: mariner
+    version: "4.3.2"
+    repository: https://example.com/charts

pkg/chart/loader/testdata/frobnitz_with_bom/INSTALL.txt

@@ -0,0 +1 @@
+This is an install document. The client may display this.

pkg/chart/loader/testdata/frobnitz_with_bom/LICENSE

@@ -0,0 +1 @@
+LICENSE placeholder.

pkg/chart/loader/testdata/frobnitz_with_bom/README.md

@@ -0,0 +1,11 @@
+# Frobnitz
+
+This is an example chart.
+
+## Usage
+
+This is an example. It has no usage.
+
+## Development
+
+For developer info, see the top-level repository.

pkg/chart/loader/testdata/frobnitz_with_bom/charts/_ignore_me

@@ -0,0 +1 @@
+This should be ignored by the loader, but may be included in a chart.

pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/Chart.yaml

@@ -0,0 +1,5 @@
+apiVersion: v1
+name: alpine
+description: Deploy a basic Alpine Linux pod
+version: 0.1.0
+home: https://helm.sh/helm

pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/README.md

@@ -0,0 +1,9 @@
+This example was generated using the command `helm create alpine`.
+
+The `templates/` directory contains a very simple pod resource with a
+couple of parameters.
+
+The `values.toml` file contains the default values for the
+`alpine-pod.yaml` template.
+
+You can install this example using `helm install ./alpine`.

pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/Chart.yaml

@@ -0,0 +1,5 @@
+apiVersion: v1
+name: mast1
+description: A Helm chart for Kubernetes
+version: 0.1.0
+home: ""

pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/values.yaml

@@ -0,0 +1,4 @@
+# Default values for mast1.
+# This is a YAML-formatted file.
+# Declare name/value pairs to be passed into your templates.
+# name = "value"

pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/templates/alpine-pod.yaml

@@ -0,0 +1,14 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: {{.Release.Name}}-{{.Chart.Name}}
+  labels:
+    app.kubernetes.io/managed-by: {{.Release.Service}}
+    app.kubernetes.io/name: {{.Chart.Name}}
+    helm.sh/chart: "{{.Chart.Name}}-{{.Chart.Version}}"
+spec:
+  restartPolicy: {{default "Never" .restart_policy}}
+  containers:
+  - name: waiter
+    image: "alpine:3.9"
+    command: ["/bin/sleep","9000"]

pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/values.yaml

@@ -0,0 +1,2 @@
+# The pod name
+name: "my-alpine"

pkg/chart/loader/testdata/frobnitz_with_bom/docs/README.md

@@ -0,0 +1 @@
+This is a placeholder for documentation.

pkg/chart/loader/testdata/frobnitz_with_bom/icon.svg

@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<svg xmlns:svg="http://www.w3.org/2000/svg" xmlns="http://www.w3.org/2000/svg"
+    xmlns:xlink="http://www.w3.org/1999/xlink"
+    version="1.0" width="256" height="256" id="test">
+  <desc>Example icon</desc>
+  <rect id="first" x="2" y="2" width="40" height="60" fill="navy"/>
+  <rect id="second" x="15" y="4" width="40" height="60" fill="red"/>
+</svg>

pkg/chart/loader/testdata/frobnitz_with_bom/templates/template.tpl

@@ -0,0 +1 @@
+Hello {{.Name | default "world"}}

pkg/chart/loader/testdata/frobnitz_with_bom/values.yaml

@@ -0,0 +1,6 @@
+# A values file contains configuration.
+
+name: "Some Name"
+
+section:
+  name: "Name in a section"

pkg/lint/lint_test.go

@@ -104,7 +104,10 @@ func TestBadValues(t *testing.T) {
 func TestGoodChart(t *testing.T) {
 	m := All(goodChartDir, values, namespace, strict).Messages
 	if len(m) != 0 {
-		t.Errorf("All failed but shouldn't have: %#v", m)
+		t.Error("All returned linter messages when it shouldn't have")
+		for i, msg := range m {
+			t.Logf("Message %d: %s", i, msg)
+		}
 	}
 }
 
@@ -130,6 +133,9 @@ func TestHelmCreateChart(t *testing.T) {
 	m := All(createdChart, values, namespace, true).Messages
 	if ll := len(m); ll != 1 {
 		t.Errorf("All should have had exactly 1 error. Got %d", ll)
+		for i, msg := range m {
+			t.Logf("Message %d: %s", i, msg.Error())
+		}
 	} else if msg := m[0].Err.Error(); !strings.Contains(msg, "icon is recommended") {
 		t.Errorf("Unexpected lint error: %s", msg)
 	}

pkg/lint/rules/template.go

@@ -17,9 +17,11 @@ limitations under the License.
 package rules
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"regexp"
+	"strings"
 
 	"github.com/pkg/errors"
 	"sigs.k8s.io/yaml"
@@ -35,6 +37,14 @@ var (
 	releaseTimeSearch = regexp.MustCompile(`\.Release\.Time`)
 )
 
+// validName is a regular expression for names.
+//
+// This is different than action.ValidName. It conforms to the regular expression
+// `kubectl` says it uses, plus it disallows empty names.
+//
+// For details, see https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+var validName = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`)
+
 // Templates lints the templates in the Linter.
 func Templates(linter *support.Linter, values map[string]interface{}, namespace string, strict bool) {
 	path := "templates/"
@@ -57,7 +67,7 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace
 	}
 
 	options := chartutil.ReleaseOptions{
-		Name:      "testRelease",
+		Name:      "test-release",
 		Namespace: namespace,
 	}
 
@@ -111,14 +121,17 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace
 		// linter.RunLinterRule(support.WarningSev, path, validateQuotes(string(preExecutedTemplate)))
 
 		renderedContent := renderedContentMap[filepath.Join(chart.Name(), fileName)]
-		var yamlStruct K8sYamlStruct
-		// Even though K8sYamlStruct only defines Metadata namespace, an error in any other
-		// key will be raised as well
-		err := yaml.Unmarshal([]byte(renderedContent), &yamlStruct)
-
-		// If YAML linting fails, we sill progress. So we don't capture the returned state
-		// on this linter run.
-		linter.RunLinterRule(support.ErrorSev, path, validateYamlContent(err))
+		if strings.TrimSpace(renderedContent) != "" {
+			var yamlStruct K8sYamlStruct
+			// Even though K8sYamlStruct only defines a few fields, an error in any other
+			// key will be raised as well
+			err := yaml.Unmarshal([]byte(renderedContent), &yamlStruct)
+
+			// If YAML linting fails, we sill progress. So we don't capture the returned state
+			// on this linter run.
+			linter.RunLinterRule(support.ErrorSev, path, validateYamlContent(err))
+			linter.RunLinterRule(support.ErrorSev, path, validateMetadataName(&yamlStruct))
+		}
 	}
 }
 
@@ -149,6 +162,15 @@ func validateYamlContent(err error) error {
 	return errors.Wrap(err, "unable to parse YAML")
 }
 
+func validateMetadataName(obj *K8sYamlStruct) error {
+	// This will return an error if the characters do not abide by the standard OR if the
+	// name is left empty.
+	if validName.MatchString(obj.Metadata.Name) {
+		return nil
+	}
+	return fmt.Errorf("object name does not conform to Kubernetes naming requirements: %q", obj.Metadata.Name)
+}
+
 func validateNoCRDHooks(manifest []byte) error {
 	if crdHookSearch.Match(manifest) {
 		return errors.New("manifest is a crd-install hook. This hook is no longer supported in v3 and all CRDs should also exist the crds/ directory at the top level of the chart")
@@ -164,9 +186,14 @@ func validateNoReleaseTime(manifest []byte) error {
 }
 
 // K8sYamlStruct stubs a Kubernetes YAML file.
-// Need to access for now to Namespace only
+//
+// DEPRECATED: In Helm 4, this will be made a private type, as it is for use only within
+// the rules package.
 type K8sYamlStruct struct {
-	Metadata struct {
-		Namespace string
-	}
+	Metadata k8sYamlMetadata
+}
+
+type k8sYamlMetadata struct {
+	Namespace string
+	Name      string
 }

pkg/lint/rules/template_test.go

@@ -101,3 +101,32 @@ func TestV3Fail(t *testing.T) {
 		t.Errorf("Unexpected error: %s", res[2].Err)
 	}
 }
+
+func TestValidateMetadataName(t *testing.T) {
+	names := map[string]bool{
+		"":                          false,
+		"foo":                       true,
+		"foo.bar1234baz.seventyone": true,
+		"FOO":                       false,
+		"123baz":                    true,
+		"foo.BAR.baz":               false,
+		"one-two":                   true,
+		"-two":                      false,
+		"one_two":                   false,
+		"a..b":                      false,
+		"%^&#$%*@^*@&#^":            false,
+	}
+	for input, expectPass := range names {
+		obj := K8sYamlStruct{Metadata: k8sYamlMetadata{Name: input}}
+		if err := validateMetadataName(&obj); (err == nil) != expectPass {
+			st := "fail"
+			if expectPass {
+				st = "succeed"
+			}
+			t.Errorf("Expected %q to %s", input, st)
+			if err != nil {
+				t.Log(err)
+			}
+		}
+	}
+}

pkg/lint/rules/testdata/goodone/templates/goodone.yaml

@@ -1,2 +1,2 @@
 metadata:
-  name: {{.Values.name | default "foo" | title}}
+  name: {{ .Values.name | default "foo" | lower }}

pkg/lint/rules/testdata/goodone/values.yaml

@@ -1 +1 @@
-name: "goodone here"
+name: "goodone-here"