Solve #12798 | checking sub-charts with properly scoped values

Signed-off-by: tomasz.ziolkowski <tomasz.ziolkowski@allegro.pl>

cmd/helm/lint.go

@@ -23,6 +23,9 @@ import (
 	"path/filepath"
 	"strings"
 
+	"helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/chart/loader"
+
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 
@@ -33,6 +36,8 @@ import (
 	"helm.sh/helm/v3/pkg/lint/support"
 )
 
+const globalKey = "global"
+
 var longLintHelp = `
 This command takes a path to a chart and runs a series of tests to verify that
 the chart is well-formed.
@@ -42,6 +47,16 @@ it will emit [ERROR] messages. If it encounters issues that break with conventio
 or recommendation, it will emit [WARNING] messages.
 `
 
+type scopedChart struct {
+	path string
+	root bool
+}
+
+type parentScope struct {
+	metadata *chart.Metadata
+	values   map[string]interface{}
+}
+
 func newLintCmd(out io.Writer) *cobra.Command {
 	client := action.NewLint()
 	valueOpts := &values.Options{}
@@ -52,9 +67,13 @@ func newLintCmd(out io.Writer) *cobra.Command {
 		Short: "examine a chart for possible issues",
 		Long:  longLintHelp,
 		RunE: func(_ *cobra.Command, args []string) error {
-			paths := []string{"."}
+			if len(args) > 1 {
+				return fmt.Errorf("invalid call, expected path to a single chart, got: %s", args)
+			}
+			charts := []scopedChart{{".", true}}
+			root := &parentScope{}
 			if len(args) > 0 {
-				paths = args
+				charts[0].path = args[0]
 			}
 
 			if kubeVersion != "" {
@@ -64,34 +83,39 @@ func newLintCmd(out io.Writer) *cobra.Command {
 				}
 				client.KubeVersion = parsedKubeVersion
 			}
+			client.Namespace = settings.Namespace()
+			vals, err := valueOpts.MergeValues(getter.All(settings))
+			if err != nil {
+				return err
+			}
 
 			if client.WithSubcharts {
-				for _, p := range paths {
+				// load root chart metadata & chart values to prepare values for sub-charts
-					filepath.Walk(filepath.Join(p, "charts"), func(path string, info os.FileInfo, _ error) error {
+				root.metadata, _ = chartutil.LoadChartfile(filepath.Join(charts[0].path, "Chart.yaml"))
+				root.values, _ = chartutil.ReadValuesFile(filepath.Join(charts[0].path, "values.yaml"))
+				root.values = chartutil.CoalesceTables(root.values, vals)
+
+				filepath.Walk(filepath.Join(charts[0].path, "charts"), func(path string, info os.FileInfo, _ error) error {
 					if info != nil {
 						if info.Name() == "Chart.yaml" {
-								paths = append(paths, filepath.Dir(path))
+							charts = append(charts, scopedChart{filepath.Dir(path), false})
 						} else if strings.HasSuffix(path, ".tgz") || strings.HasSuffix(path, ".tar.gz") {
-								paths = append(paths, path)
+							charts = append(charts, scopedChart{path, false})
 						}
 					}
 					return nil
 				})
 			}
-			}
-
-			client.Namespace = settings.Namespace()
-			vals, err := valueOpts.MergeValues(getter.All(settings))
-			if err != nil {
-				return err
-			}
 
 			var message strings.Builder
 			failed := 0
 			errorsOrWarnings := 0
+			linted := 0
 
-			for _, path := range paths {
+			for _, single := range charts {
-				result := client.Run([]string{path}, vals)
+				for scope, scopedVals := range getScopedValues(root, single, vals) {
+					linted++
+					result := client.Run([]string{single.path}, scopedVals)
 
 					// If there is no errors/warnings and quiet flag is set
 					// go to the next chart
@@ -103,7 +127,7 @@ func newLintCmd(out io.Writer) *cobra.Command {
 						continue
 					}
 
-				fmt.Fprintf(&message, "==> Linting %s\n", path)
+					fmt.Fprintf(&message, "==> Linting %s (scope: %s)\n", single.path, scope)
 
 					// All the Errors that are generated by a chart
 					// that failed a lint will be included in the
@@ -130,10 +154,11 @@ func newLintCmd(out io.Writer) *cobra.Command {
 					// text and makes it easier to follow.
 					fmt.Fprint(&message, "\n")
 				}
+			}
 
 			fmt.Fprint(out, message.String())
 
-			summary := fmt.Sprintf("%d chart(s) linted, %d chart(s) failed", len(paths), failed)
+			summary := fmt.Sprintf("%d chart(s) linted, %d chart(s) failed", linted, failed)
 			if failed > 0 {
 				return errors.New(summary)
 			}
@@ -153,3 +178,39 @@ func newLintCmd(out io.Writer) *cobra.Command {
 
 	return cmd
 }
+
+func getScopedValues(parent *parentScope, single scopedChart, vals chartutil.Values) map[string]chartutil.Values {
+	if single.root {
+		return map[string]chartutil.Values{"root": vals}
+	}
+	result := map[string]chartutil.Values{}
+	if parent.metadata != nil {
+		// try to load dependent chart and match it with parent definition of dependencies
+		if subChart, err := loader.Load(single.path); err == nil {
+			for _, dependency := range parent.metadata.Dependencies {
+				// it is worth remembering that there could be the same sub-chart used in many dependencies
+				// to handle this we should validate such sub-chart as many times (with the appropriate set of values)
+				// aa it is defined in the parent chart
+				if subChart.Metadata.Version == dependency.Version && subChart.Name() == dependency.Name {
+					alias := dependency.Alias
+					if alias == "" {
+						alias = dependency.Name
+					}
+					tmp := chartutil.Values{}
+					if _, ok := parent.values[alias]; ok {
+						tmp = parent.values[alias].(map[string]interface{})
+					}
+					if _, ok := parent.values[globalKey]; ok {
+						tmp[globalKey] = parent.values[globalKey].(map[string]interface{})
+					}
+					result[alias] = tmp
+				}
+			}
+		}
+	}
+	// if we can't find any suitable values, just provide an empty set for sub-chart to at least validate other aspects
+	if len(result) == 0 {
+		result["empty"] = chartutil.Values{}
+	}
+	return result
+}

cmd/helm/lint_test.go

@@ -21,6 +21,21 @@ import (
 	"testing"
 )
 
+func TestLintCmdWithMultipleChartsIsProhibited(t *testing.T) {
+	tests := []cmdTestCase{{
+		name:      "lint two charts simultaneously",
+		cmd:       "lint first second",
+		golden:    "output/lint-chart-with-two-charts.txt",
+		wantError: true,
+	}, {
+		name:      "lint three charts simultaneously",
+		cmd:       "lint first second third",
+		golden:    "output/lint-chart-with-three-charts.txt",
+		wantError: true,
+	}}
+	runTestCmd(t, tests)
+}
+
 func TestLintCmdWithSubchartsFlag(t *testing.T) {
 	testChart := "testdata/testcharts/chart-with-bad-subcharts"
 	tests := []cmdTestCase{{
@@ -37,6 +52,44 @@ func TestLintCmdWithSubchartsFlag(t *testing.T) {
 	runTestCmd(t, tests)
 }
 
+func TestLintCmdWithSubchartsFlagShouldPassScopedValuesToSubcharts(t *testing.T) {
+	scopeName := "testdata/testcharts/issue-12798"
+	scopeAlias := "testdata/testcharts/issue-12798-alias"
+	scopeGlobal := "testdata/testcharts/issue-12798-global"
+	tests := []cmdTestCase{{
+		name:      "lint umbrella chart without required parameter by sub-chart",
+		cmd:       fmt.Sprintf("lint --with-subcharts %s", scopeName),
+		golden:    "output/lint-chart-with-dependency-values-scoped-by-name.txt",
+		wantError: true,
+	}, {
+		name:      "lint umbrella chart with required parameter by sub-chart",
+		cmd:       fmt.Sprintf("lint --with-subcharts --set child.sample=1 %s", scopeName),
+		golden:    "",
+		wantError: false,
+	}, {
+		name:      "lint umbrella chart with required parameter by aliased sub-chart",
+		cmd:       fmt.Sprintf("lint --with-subcharts --set lastname=test %s", scopeAlias),
+		golden:    "output/lint-chart-with-dependency-values-scoped-by-alias.txt",
+		wantError: true,
+	}, {
+		name:      "lint umbrella chart with required parameter by aliased sub-chart",
+		cmd:       fmt.Sprintf("lint --with-subcharts --set short.sample=1 %s", scopeAlias),
+		golden:    "",
+		wantError: false,
+	}, {
+		name:      "lint umbrella chart with required global parameter",
+		cmd:       fmt.Sprintf("lint --with-subcharts %s", scopeGlobal),
+		golden:    "output/lint-chart-with-dependency-values-scoped-by-global.txt",
+		wantError: true,
+	}, {
+		name:      "lint umbrella chart with required global parameter",
+		cmd:       fmt.Sprintf("lint --with-subcharts --set global.sample=1 %s", scopeGlobal),
+		golden:    "",
+		wantError: false,
+	}}
+	runTestCmd(t, tests)
+}
+
 func TestLintCmdWithQuietFlag(t *testing.T) {
 	testChart1 := "testdata/testcharts/alpine"
 	testChart2 := "testdata/testcharts/chart-bad-requirements"
@@ -45,8 +98,8 @@ func TestLintCmdWithQuietFlag(t *testing.T) {
 		cmd:    fmt.Sprintf("lint --quiet %s", testChart1),
 		golden: "output/lint-quiet.txt",
 	}, {
-		name:      "lint two charts, one with error using --quiet flag",
+		name:      "lint malformed chart using --quiet flag",
-		cmd:       fmt.Sprintf("lint --quiet %s %s", testChart1, testChart2),
+		cmd:       fmt.Sprintf("lint --quiet %s", testChart2),
 		golden:    "output/lint-quiet-with-error.txt",
 		wantError: true,
 	}, {
@@ -60,7 +113,6 @@ func TestLintCmdWithQuietFlag(t *testing.T) {
 		wantError: true,
 	}}
 	runTestCmd(t, tests)
-
 }
 
 func TestLintCmdWithKubeVersionFlag(t *testing.T) {

cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt

@@ -1,10 +1,10 @@
-==> Linting testdata/testcharts/chart-with-bad-subcharts
+==> Linting testdata/testcharts/chart-with-bad-subcharts (scope: root)
 [INFO] Chart.yaml: icon is recommended
 [ERROR] templates/: error unpacking bad-subchart in chart-with-bad-subcharts: validation: chart.metadata.name is required
 [ERROR] : unable to load chart
 	error unpacking bad-subchart in chart-with-bad-subcharts: validation: chart.metadata.name is required
 
-==> Linting testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart
+==> Linting testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart (scope: empty)
 [ERROR] Chart.yaml: name is required
 [ERROR] Chart.yaml: apiVersion is required. The value must be either "v1" or "v2"
 [ERROR] Chart.yaml: version is required
@@ -13,7 +13,7 @@
 [ERROR] : unable to load chart
 	validation: chart.metadata.name is required
 
-==> Linting testdata/testcharts/chart-with-bad-subcharts/charts/good-subchart
+==> Linting testdata/testcharts/chart-with-bad-subcharts/charts/good-subchart (scope: empty)
 [INFO] Chart.yaml: icon is recommended
 
 Error: 3 chart(s) linted, 2 chart(s) failed

cmd/helm/testdata/output/lint-chart-with-bad-subcharts.txt

@@ -1,4 +1,4 @@
-==> Linting testdata/testcharts/chart-with-bad-subcharts
+==> Linting testdata/testcharts/chart-with-bad-subcharts (scope: root)
 [INFO] Chart.yaml: icon is recommended
 [ERROR] templates/: error unpacking bad-subchart in chart-with-bad-subcharts: validation: chart.metadata.name is required
 [ERROR] : unable to load chart

cmd/helm/testdata/output/lint-chart-with-dependency-values-scoped-by-alias.txt

@@ -0,0 +1,17 @@
+==> Linting testdata/testcharts/issue-12798-alias (scope: root)
+[INFO] Chart.yaml: icon is recommended
+[ERROR] templates/: values don't meet the specifications of the schema(s) in the following chart(s):
+short:
+- (root): sample is required
+
+
+==> Linting testdata/testcharts/issue-12798-alias/charts/child (scope: short)
+[INFO] Chart.yaml: icon is recommended
+[ERROR] values.yaml: - (root): sample is required
+
+[ERROR] templates/: values don't meet the specifications of the schema(s) in the following chart(s):
+child:
+- (root): sample is required
+
+
+Error: 2 chart(s) linted, 2 chart(s) failed

cmd/helm/testdata/output/lint-chart-with-dependency-values-scoped-by-global.txt

@@ -0,0 +1,17 @@
+==> Linting testdata/testcharts/issue-12798-global (scope: root)
+[INFO] Chart.yaml: icon is recommended
+[ERROR] templates/: values don't meet the specifications of the schema(s) in the following chart(s):
+child:
+- global: sample is required
+
+
+==> Linting testdata/testcharts/issue-12798-global/charts/child (scope: child)
+[INFO] Chart.yaml: icon is recommended
+[ERROR] values.yaml: - (root): global is required
+
+[ERROR] templates/: values don't meet the specifications of the schema(s) in the following chart(s):
+child:
+- (root): global is required
+
+
+Error: 2 chart(s) linted, 2 chart(s) failed

cmd/helm/testdata/output/lint-chart-with-dependency-values-scoped-by-name.txt

@@ -0,0 +1,17 @@
+==> Linting testdata/testcharts/issue-12798 (scope: root)
+[INFO] Chart.yaml: icon is recommended
+[ERROR] templates/: values don't meet the specifications of the schema(s) in the following chart(s):
+child:
+- (root): sample is required
+
+
+==> Linting testdata/testcharts/issue-12798/charts/child (scope: child)
+[INFO] Chart.yaml: icon is recommended
+[ERROR] values.yaml: - (root): sample is required
+
+[ERROR] templates/: values don't meet the specifications of the schema(s) in the following chart(s):
+child:
+- (root): sample is required
+
+
+Error: 2 chart(s) linted, 2 chart(s) failed

cmd/helm/testdata/output/lint-chart-with-deprecated-api-old-k8s.txt

@@ -1,4 +1,4 @@
-==> Linting testdata/testcharts/chart-with-deprecated-api
+==> Linting testdata/testcharts/chart-with-deprecated-api (scope: root)
 [INFO] Chart.yaml: icon is recommended
 
 1 chart(s) linted, 0 chart(s) failed

cmd/helm/testdata/output/lint-chart-with-deprecated-api-strict.txt

@@ -1,4 +1,4 @@
-==> Linting testdata/testcharts/chart-with-deprecated-api
+==> Linting testdata/testcharts/chart-with-deprecated-api (scope: root)
 [INFO] Chart.yaml: icon is recommended
 [WARNING] templates/horizontalpodautoscaler.yaml: autoscaling/v2beta1 HorizontalPodAutoscaler is deprecated in v1.22+, unavailable in v1.25+; use autoscaling/v2 HorizontalPodAutoscaler
 

cmd/helm/testdata/output/lint-chart-with-deprecated-api.txt

@@ -1,4 +1,4 @@
-==> Linting testdata/testcharts/chart-with-deprecated-api
+==> Linting testdata/testcharts/chart-with-deprecated-api (scope: root)
 [INFO] Chart.yaml: icon is recommended
 [WARNING] templates/horizontalpodautoscaler.yaml: autoscaling/v2beta1 HorizontalPodAutoscaler is deprecated in v1.22+, unavailable in v1.25+; use autoscaling/v2 HorizontalPodAutoscaler
 

cmd/helm/testdata/output/lint-chart-with-schema-and-subchart.txt

@@ -0,0 +1,7 @@
+==> Linting testdata/testcharts/chart-with-bad-subcharts (scope: root)
+[INFO] Chart.yaml: icon is recommended
+[ERROR] templates/: error unpacking bad-subchart in chart-with-bad-subcharts: validation: chart.metadata.name is required
+[ERROR] : unable to load chart
+	error unpacking bad-subchart in chart-with-bad-subcharts: validation: chart.metadata.name is required
+
+Error: 1 chart(s) linted, 1 chart(s) failed

cmd/helm/testdata/output/lint-chart-with-three-charts.txt

@@ -0,0 +1 @@
+Error: invalid call, expected path to a single chart, got: [first second third]

cmd/helm/testdata/output/lint-chart-with-two-charts.txt

@@ -0,0 +1 @@
+Error: invalid call, expected path to a single chart, got: [first second]

cmd/helm/testdata/output/lint-quiet-with-error.txt

@@ -1,8 +1,8 @@
-==> Linting testdata/testcharts/chart-bad-requirements
+==> Linting testdata/testcharts/chart-bad-requirements (scope: root)
 [ERROR] Chart.yaml: unable to parse YAML
 	error converting YAML to JSON: yaml: line 6: did not find expected '-' indicator
 [ERROR] templates/: cannot load Chart.yaml: error converting YAML to JSON: yaml: line 6: did not find expected '-' indicator
 [ERROR] : unable to load chart
 	cannot load Chart.yaml: error converting YAML to JSON: yaml: line 6: did not find expected '-' indicator
 
-Error: 2 chart(s) linted, 1 chart(s) failed
+Error: 1 chart(s) linted, 1 chart(s) failed

cmd/helm/testdata/testcharts/chart-with-schema-and-subchart/requirements.yaml

@@ -0,0 +1,5 @@
+dependencies:
+  - name: good-subchart
+    version: 0.0.1
+  - name: bad-subchart
+    version: 0.0.1

cmd/helm/testdata/testcharts/issue-12798-alias/Chart.yaml

@@ -0,0 +1,10 @@
+apiVersion: v2
+name: umbrella
+description: An umbrella Helm chart for Kubernetes
+type: application
+version: 0.1.0
+appVersion: 0.1.0
+dependencies:
+  - name: child
+    alias: short
+    version: 0.1.0

cmd/helm/testdata/testcharts/issue-12798-alias/charts/child/Chart.yaml

@@ -0,0 +1,6 @@
+apiVersion: v2
+name: child
+description: A Helm chart for Kubernetes
+type: application
+version: 0.1.0
+appVersion: 0.1.0

cmd/helm/testdata/testcharts/issue-12798-alias/charts/child/templates/empty.yaml

@@ -0,0 +1 @@
+# This file is intentionally blank

cmd/helm/testdata/testcharts/issue-12798-alias/charts/child/values.schema.json

@@ -0,0 +1,14 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "title": "Values",
+  "type": "object",
+  "properties": {
+    "sample": {
+      "minimum": 0,
+      "type": "integer"
+    }
+  },
+  "required": [
+    "sample"
+  ]
+}

cmd/helm/testdata/testcharts/issue-12798-alias/templates/empty.yaml

@@ -0,0 +1 @@
+# This file is intentionally blank

cmd/helm/testdata/testcharts/issue-12798-alias/values.schema.json

@@ -0,0 +1,18 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "title": "Values",
+  "type": "object",
+  "properties": {
+    "firstname": {
+      "description": "First name",
+      "type": "string"
+    },
+    "lastname": {
+      "type": "string"
+    }
+  },
+  "required": [
+    "firstname",
+    "lastname"
+  ]
+}

cmd/helm/testdata/testcharts/issue-12798-alias/values.yaml

@@ -0,0 +1,2 @@
+firstname: "John"
+lastname: "Doe"

cmd/helm/testdata/testcharts/issue-12798-global/Chart.yaml

@@ -0,0 +1,9 @@
+apiVersion: v2
+name: umbrella
+description: An umbrella Helm chart for Kubernetes
+type: application
+version: 0.1.0
+appVersion: 0.1.0
+dependencies:
+  - name: child
+    version: 0.1.0

cmd/helm/testdata/testcharts/issue-12798-global/charts/child/Chart.yaml

@@ -0,0 +1,6 @@
+apiVersion: v2
+name: child
+description: A Helm chart for Kubernetes
+type: application
+version: 0.1.0
+appVersion: 0.1.0

cmd/helm/testdata/testcharts/issue-12798-global/charts/child/templates/empty.yaml

@@ -0,0 +1 @@
+# This file is intentionally blank

cmd/helm/testdata/testcharts/issue-12798-global/charts/child/values.schema.json

@@ -0,0 +1,22 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "title": "Values",
+  "type": "object",
+  "properties": {
+    "global": {
+      "type": "object",
+      "properties": {
+        "sample": {
+          "minimum": 0,
+          "type": "integer"
+        }
+      },
+      "required": [
+        "sample"
+      ]
+    }
+  },
+  "required": [
+    "global"
+  ]
+}

cmd/helm/testdata/testcharts/issue-12798-global/templates/empty.yaml

@@ -0,0 +1 @@
+# This file is intentionally blank

cmd/helm/testdata/testcharts/issue-12798-global/values.schema.json

@@ -0,0 +1,18 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "title": "Values",
+  "type": "object",
+  "properties": {
+    "firstname": {
+      "description": "First name",
+      "type": "string"
+    },
+    "lastname": {
+      "type": "string"
+    }
+  },
+  "required": [
+    "firstname",
+    "lastname"
+  ]
+}

cmd/helm/testdata/testcharts/issue-12798-global/values.yaml

@@ -0,0 +1,2 @@
+firstname: "John"
+lastname: "Doe"

cmd/helm/testdata/testcharts/issue-12798/Chart.yaml

@@ -0,0 +1,9 @@
+apiVersion: v2
+name: umbrella
+description: An umbrella Helm chart for Kubernetes
+type: application
+version: 0.1.0
+appVersion: 0.1.0
+dependencies:
+  - name: child
+    version: 0.1.0

cmd/helm/testdata/testcharts/issue-12798/charts/child/Chart.yaml

@@ -0,0 +1,6 @@
+apiVersion: v2
+name: child
+description: A Helm chart for Kubernetes
+type: application
+version: 0.1.0
+appVersion: 0.1.0

cmd/helm/testdata/testcharts/issue-12798/charts/child/templates/empty.yaml

@@ -0,0 +1 @@
+# This file is intentionally blank

cmd/helm/testdata/testcharts/issue-12798/charts/child/values.schema.json

@@ -0,0 +1,14 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "title": "Values",
+  "type": "object",
+  "properties": {
+    "sample": {
+      "minimum": 0,
+      "type": "integer"
+    }
+  },
+  "required": [
+    "sample"
+  ]
+}

cmd/helm/testdata/testcharts/issue-12798/templates/empty.yaml

@@ -0,0 +1 @@
+# This file is intentionally blank

cmd/helm/testdata/testcharts/issue-12798/values.schema.json

@@ -0,0 +1,18 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "title": "Values",
+  "type": "object",
+  "properties": {
+    "firstname": {
+      "description": "First name",
+      "type": "string"
+    },
+    "lastname": {
+      "type": "string"
+    }
+  },
+  "required": [
+    "firstname",
+    "lastname"
+  ]
+}

cmd/helm/testdata/testcharts/issue-12798/values.yaml

@@ -0,0 +1,2 @@
+firstname: "John"
+lastname: "Doe"