Lint dependencies (#7970)

* feat: add dependency tests

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>

* replaced on-disk fixtures with in-memory fixtures

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>

cmd/helm/lint_test.go

@@ -27,7 +27,7 @@ func TestLintCmdWithSubchartsFlag(t *testing.T) {
 		name:      "lint good chart with bad subcharts",
 		cmd:       fmt.Sprintf("lint %s", testChart),
 		golden:    "output/lint-chart-with-bad-subcharts.txt",
-		wantError: false,
+		wantError: true,
 	}, {
 		name:      "lint good chart with bad subcharts using --with-subcharts flag",
 		cmd:       fmt.Sprintf("lint --with-subcharts %s", testChart),

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

@@ -1,6 +1,8 @@
 ==> Linting testdata/testcharts/chart-with-bad-subcharts
 [INFO] Chart.yaml: icon is recommended
 [WARNING] templates/: directory not found
+[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
 [ERROR] Chart.yaml: name is required
@@ -8,9 +10,11 @@
 [ERROR] Chart.yaml: version is required
 [INFO] Chart.yaml: icon is recommended
 [WARNING] templates/: directory not found
+[ERROR] : unable to load chart
+	validation: chart.metadata.name is required
 
 ==> Linting testdata/testcharts/chart-with-bad-subcharts/charts/good-subchart
 [INFO] Chart.yaml: icon is recommended
 [WARNING] templates/: directory not found
 
-Error: 3 chart(s) linted, 1 chart(s) failed
+Error: 3 chart(s) linted, 2 chart(s) failed

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

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

cmd/helm/testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart/Chart.yaml

@@ -1 +1 @@
-description: Bad subchart
+description: Bad subchart

pkg/chartutil/save_test.go

@@ -30,15 +30,13 @@ import (
 	"testing"
 	"time"
 
+	"helm.sh/helm/v3/internal/test/ensure"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chart/loader"
 )
 
 func TestSave(t *testing.T) {
-	tmp, err := ioutil.TempDir("", "helm-")
-	if err != nil {
-		t.Fatal(err)
-	}
+	tmp := ensure.TempDir(t)
 	defer os.RemoveAll(tmp)
 
 	for _, dest := range []string{tmp, path.Join(tmp, "newdir")} {

pkg/lint/lint.go

@@ -32,5 +32,6 @@ func All(basedir string, values map[string]interface{}, namespace string, strict
 	rules.Chartfile(&linter)
 	rules.ValuesWithOverrides(&linter, values)
 	rules.Templates(&linter, values, namespace, strict)
+	rules.Dependencies(&linter)
 	return linter
 }

pkg/lint/lint_test.go

@@ -38,12 +38,12 @@ const goodChartDir = "rules/testdata/goodone"
 
 func TestBadChart(t *testing.T) {
 	m := All(badChartDir, values, namespace, strict).Messages
-	if len(m) != 7 {
+	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, e5 bool
+	// There should be one INFO, 2 WARNINGs and 2 ERROR messages, check for them
+	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") {
@@ -74,9 +74,13 @@ func TestBadChart(t *testing.T) {
 			if strings.Contains(msg.Err.Error(), "dependencies are not valid in the Chart file with apiVersion") {
 				e5 = true
 			}
+			// This comes from the dependency check, which loads dependency info from the Chart.yaml
+			if strings.Contains(msg.Err.Error(), "unable to load chart") {
+				e6 = true
+			}
 		}
 	}
-	if !e || !e2 || !e3 || !e4 || !e5 || !w || !i {
+	if !e || !e2 || !e3 || !e4 || !e5 || !w || !i || !e6 {
 		t.Errorf("Didn't find all the expected errors, got %#v", m)
 	}
 }

pkg/lint/rules/dependencies.go

@@ -0,0 +1,82 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package rules // import "helm.sh/helm/v3/pkg/lint/rules"
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/pkg/errors"
+
+	"helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/chart/loader"
+	"helm.sh/helm/v3/pkg/lint/support"
+)
+
+// Dependencies runs lints against a chart's dependencies
+//
+// See https://github.com/helm/helm/issues/7910
+func Dependencies(linter *support.Linter) {
+	c, err := loader.LoadDir(linter.ChartDir)
+	if !linter.RunLinterRule(support.ErrorSev, "", validateChartFormat(err)) {
+		return
+	}
+
+	linter.RunLinterRule(support.ErrorSev, linter.ChartDir, validateDependencyInMetadata(c))
+	linter.RunLinterRule(support.WarningSev, linter.ChartDir, validateDependencyInChartsDir(c))
+}
+
+func validateChartFormat(chartError error) error {
+	if chartError != nil {
+		return errors.Errorf("unable to load chart\n\t%s", chartError)
+	}
+	return nil
+}
+
+func validateDependencyInChartsDir(c *chart.Chart) (err error) {
+	dependencies := map[string]struct{}{}
+	missing := []string{}
+	for _, dep := range c.Dependencies() {
+		dependencies[dep.Metadata.Name] = struct{}{}
+	}
+	for _, dep := range c.Metadata.Dependencies {
+		if _, ok := dependencies[dep.Name]; !ok {
+			missing = append(missing, dep.Name)
+		}
+	}
+	if len(missing) > 0 {
+		err = fmt.Errorf("chart directory is missing these dependencies: %s", strings.Join(missing, ","))
+	}
+	return err
+}
+
+func validateDependencyInMetadata(c *chart.Chart) (err error) {
+	dependencies := map[string]struct{}{}
+	missing := []string{}
+	for _, dep := range c.Metadata.Dependencies {
+		dependencies[dep.Name] = struct{}{}
+	}
+	for _, dep := range c.Dependencies() {
+		if _, ok := dependencies[dep.Metadata.Name]; !ok {
+			missing = append(missing, dep.Metadata.Name)
+		}
+	}
+	if len(missing) > 0 {
+		err = fmt.Errorf("chart metadata is missing these dependencies: %s", strings.Join(missing, ","))
+	}
+	return err
+}

pkg/lint/rules/dependencies_test.go

@@ -0,0 +1,99 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package rules
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"helm.sh/helm/v3/internal/test/ensure"
+	"helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/chartutil"
+	"helm.sh/helm/v3/pkg/lint/support"
+)
+
+func chartWithBadDependencies() chart.Chart {
+	badChartDeps := chart.Chart{
+		Metadata: &chart.Metadata{
+			Name:       "badchart",
+			Version:    "0.1.0",
+			APIVersion: "v2",
+			Dependencies: []*chart.Dependency{
+				{
+					Name: "sub2",
+				},
+				{
+					Name: "sub3",
+				},
+			},
+		},
+	}
+
+	badChartDeps.SetDependencies(
+		&chart.Chart{
+			Metadata: &chart.Metadata{
+				Name:       "sub1",
+				Version:    "0.1.0",
+				APIVersion: "v2",
+			},
+		},
+		&chart.Chart{
+			Metadata: &chart.Metadata{
+				Name:       "sub2",
+				Version:    "0.1.0",
+				APIVersion: "v2",
+			},
+		},
+	)
+	return badChartDeps
+}
+
+func TestValidateDependencyInChartsDir(t *testing.T) {
+	c := chartWithBadDependencies()
+
+	if err := validateDependencyInChartsDir(&c); err == nil {
+		t.Error("chart should have been flagged for missing deps in chart directory")
+	}
+}
+
+func TestValidateDependencyInMetadata(t *testing.T) {
+	c := chartWithBadDependencies()
+
+	if err := validateDependencyInMetadata(&c); err == nil {
+		t.Errorf("chart should have been flagged for missing deps in chart metadata")
+	}
+}
+
+func TestDependencies(t *testing.T) {
+	tmp := ensure.TempDir(t)
+	defer os.RemoveAll(tmp)
+
+	c := chartWithBadDependencies()
+	err := chartutil.SaveDir(&c, tmp)
+	if err != nil {
+		t.Fatal(err)
+	}
+	linter := support.Linter{ChartDir: filepath.Join(tmp, c.Metadata.Name)}
+
+	Dependencies(&linter)
+	if l := len(linter.Messages); l != 2 {
+		t.Errorf("expected 2 linter errors for bad chart dependencies. Got %d.", l)
+		for i, msg := range linter.Messages {
+			t.Logf("Message: %d, Error: %#v", i, msg)
+		}
+	}
+}