Merge pull request #6117 from hickeyma/upd-linter-v2

feat(chart): Update linting and checking for apiVersion v2

pkg/chart/loader/load.go

@@ -18,6 +18,7 @@ package loader
 
 import (
 	"bytes"
+	"log"
 	"os"
 	"path/filepath"
 	"strings"
@@ -103,6 +104,9 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) {
 		// Deprecated: requirements.yaml is deprecated use Chart.yaml.
 		// We will handle it for you because we are nice people
 		case f.Name == "requirements.yaml":
+			if c.Metadata.APIVersion != chart.APIVersionV1 {
+				log.Printf("Warning: Dependencies are handled in Chart.yaml since apiVersion \"v2\". We recommend migrating dependencies to Chart.yaml.")
+			}
 			if c.Metadata == nil {
 				c.Metadata = new(chart.Metadata)
 			}

pkg/chart/loader/load_test.go

@@ -147,6 +147,19 @@ func TestLoadFileBackslash(t *testing.T) {
 	verifyDependencies(t, c)
 }
 
+func TestLoadV2WithReqs(t *testing.T) {
+	l, err := Loader("testdata/frobnitz.v2.reqs")
+	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)
+	}
+	verifyDependencies(t, c)
+	verifyDependenciesLock(t, c)
+}
+
 func verifyChart(t *testing.T, c *chart.Chart) {
 	t.Helper()
 	if c.Name() == "" {

pkg/chart/loader/testdata/frobnitz.v2.reqs/.helmignore

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

pkg/chart/loader/testdata/frobnitz.v2.reqs/Chart.yaml

@@ -0,0 +1,20 @@
+apiVersion: v2
+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

pkg/chart/loader/testdata/frobnitz.v2.reqs/INSTALL.txt

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

pkg/chart/loader/testdata/frobnitz.v2.reqs/LICENSE

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

pkg/chart/loader/testdata/frobnitz.v2.reqs/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.v2.reqs/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.v2.reqs/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.v2.reqs/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.v2.reqs/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.v2.reqs/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.v2.reqs/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.v2.reqs/charts/alpine/values.yaml

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

pkg/chart/loader/testdata/frobnitz.v2.reqs/docs/README.md

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

pkg/chart/loader/testdata/frobnitz.v2.reqs/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.v2.reqs/requirements.yaml

@@ -0,0 +1,7 @@
+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.v2.reqs/templates/template.tpl

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

pkg/chart/loader/testdata/frobnitz.v2.reqs/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

@@ -35,12 +35,12 @@ const goodChartDir = "rules/testdata/goodone"
 
 func TestBadChart(t *testing.T) {
 	m := All(badChartDir, values, namespace, strict).Messages
-	if len(m) != 6 {
+	if len(m) != 8 {
 		t.Errorf("Number of errors %v", len(m))
 		t.Errorf("All didn't fail with expected errors, got %#v", m)
 	}
 	// There should be one INFO, 2 WARNINGs and one ERROR messages, check for them
-	var i, w, e, e2, e3, e4 bool
+	var i, w, e, e2, e3, e4, e5, e6 bool
 	for _, msg := range m {
 		if msg.Severity == support.InfoSev {
 			if strings.Contains(msg.Err.Error(), "icon is recommended") {
@@ -66,9 +66,17 @@ func TestBadChart(t *testing.T) {
 			if strings.Contains(msg.Err.Error(), "apiVersion is required. The value must be either \"v1\" or \"v2\"") {
 				e4 = true
 			}
+
+			if strings.Contains(msg.Err.Error(), "chart type is not valid in apiVersion") {
+				e5 = true
+			}
+
+			if strings.Contains(msg.Err.Error(), "dependencies are not valid in the Chart file with apiVersion") {
+				e6 = true
+			}
 		}
 	}
-	if !e || !e2 || !e3 || !e4 || !w || !i {
+	if !e || !e2 || !e3 || !e4 || !e5 || !e6 || !w || !i {
 		t.Errorf("Didn't find all the expected errors, got %#v", m)
 	}
 }

pkg/lint/rules/chartfile.go

@@ -55,6 +55,8 @@ func Chartfile(linter *support.Linter) {
 	linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartSources(chartFile))
 	linter.RunLinterRule(support.InfoSev, chartFileName, validateChartIconPresence(chartFile))
 	linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartIconURL(chartFile))
+	linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartType(chartFile))
+	linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartDependencies(chartFile))
 }
 
 func validateChartYamlNotDirectory(chartPath string) error {
@@ -92,7 +94,7 @@ func validateChartAPIVersion(cf *chart.Metadata) error {
 		return errors.New("apiVersion is required. The value must be either \"v1\" or \"v2\"")
 	}
 
-	if cf.APIVersion != "v1" && cf.APIVersion != "v2" {
+	if cf.APIVersion != chart.APIVersionV1 && cf.APIVersion != chart.APIVersionV2 {
 		return fmt.Errorf("apiVersion '%s' is not valid. The value must be either \"v1\" or \"v2\"", cf.APIVersion)
 	}
 
@@ -158,3 +160,17 @@ func validateChartIconURL(cf *chart.Metadata) error {
 	}
 	return nil
 }
+
+func validateChartDependencies(cf *chart.Metadata) error {
+	if len(cf.Dependencies) > 0 && cf.APIVersion != chart.APIVersionV2 {
+		return fmt.Errorf("dependencies are not valid in the Chart file with apiVersion '%s'. They are valid in apiVersion '%s'", cf.APIVersion, chart.APIVersionV2)
+	}
+	return nil
+}
+
+func validateChartType(cf *chart.Metadata) error {
+	if len(cf.Type) > 0 && cf.APIVersion != chart.APIVersionV2 {
+		return fmt.Errorf("chart type is not valid in apiVersion '%s'. It is valid in apiVersion '%s'", cf.APIVersion, chart.APIVersionV2)
+	}
+	return nil
+}

pkg/lint/rules/chartfile_test.go

@@ -209,8 +209,8 @@ func TestChartfile(t *testing.T) {
 	Chartfile(&linter)
 	msgs := linter.Messages
 
-	if len(msgs) != 5 {
-		t.Errorf("Expected 4 errors, got %d", len(msgs))
+	if len(msgs) != 7 {
+		t.Errorf("Expected 7 errors, got %d", len(msgs))
 	}
 
 	if !strings.Contains(msgs[0].Err.Error(), "name is required") {
@@ -226,11 +226,19 @@ func TestChartfile(t *testing.T) {
 	}
 
 	if !strings.Contains(msgs[3].Err.Error(), "version 0.0.0 is less than or equal to 0") {
-		t.Errorf("Unexpected message 3: %s", msgs[2].Err)
+		t.Errorf("Unexpected message 3: %s", msgs[3].Err)
 	}
 
 	if !strings.Contains(msgs[4].Err.Error(), "icon is recommended") {
-		t.Errorf("Unexpected message 4: %s", msgs[3].Err)
+		t.Errorf("Unexpected message 4: %s", msgs[4].Err)
+	}
+
+	if !strings.Contains(msgs[5].Err.Error(), "chart type is not valid in apiVersion") {
+		t.Errorf("Unexpected message 5: %s", msgs[5].Err)
+	}
+
+	if !strings.Contains(msgs[6].Err.Error(), "dependencies are not valid in the Chart file with apiVersion") {
+		t.Errorf("Unexpected message 6: %s", msgs[6].Err)
 	}
 
 }

pkg/lint/rules/testdata/badchartfile/Chart.yaml

@@ -1,3 +1,11 @@
 description: A Helm chart for Kubernetes
 version: 0.0.0
 home: ""
+type: application
+dependencies:
+- name: mariadb
+  version: 5.x.x
+  repository: https://kubernetes-charts.storage.googleapis.com/
+  condition: mariadb.enabled
+  tags:
+    - database