From 2750e4d78181b614776d82031f6ddfece8d020e2 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Thu, 9 Jul 2020 14:31:51 -0600 Subject: [PATCH] Lint dependencies (#7970) * feat: add dependency tests Signed-off-by: Matt Butcher * replaced on-disk fixtures with in-memory fixtures Signed-off-by: Matt Butcher --- cmd/helm/lint_test.go | 2 +- ...hart-with-bad-subcharts-with-subcharts.txt | 6 +- .../output/lint-chart-with-bad-subcharts.txt | 4 +- .../charts/bad-subchart/Chart.yaml | 2 +- pkg/chartutil/save_test.go | 6 +- pkg/lint/lint.go | 1 + pkg/lint/lint_test.go | 12 ++- pkg/lint/rules/dependencies.go | 82 +++++++++++++++ pkg/lint/rules/dependencies_test.go | 99 +++++++++++++++++++ 9 files changed, 202 insertions(+), 12 deletions(-) create mode 100644 pkg/lint/rules/dependencies.go create mode 100644 pkg/lint/rules/dependencies_test.go diff --git a/cmd/helm/lint_test.go b/cmd/helm/lint_test.go index 7638acb84..9079935d4 100644 --- a/cmd/helm/lint_test.go +++ b/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), diff --git a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt b/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt index cda011b57..e77aa387f 100644 --- a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt +++ b/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 diff --git a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts.txt b/cmd/helm/testdata/output/lint-chart-with-bad-subcharts.txt index 51f3f3718..265e555f7 100644 --- a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts.txt +++ b/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 diff --git a/cmd/helm/testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart/Chart.yaml index 0daa5c188..a6754b24f 100644 --- a/cmd/helm/testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart/Chart.yaml +++ b/cmd/helm/testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart/Chart.yaml @@ -1 +1 @@ -description: Bad subchart \ No newline at end of file +description: Bad subchart diff --git a/pkg/chartutil/save_test.go b/pkg/chartutil/save_test.go index 306c13cee..3a45b2992 100644 --- a/pkg/chartutil/save_test.go +++ b/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")} { diff --git a/pkg/lint/lint.go b/pkg/lint/lint.go index 223ead75a..67e76bd3d 100644 --- a/pkg/lint/lint.go +++ b/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 } diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go index e7ff4cd7a..29ed67026 100644 --- a/pkg/lint/lint_test.go +++ b/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) } } diff --git a/pkg/lint/rules/dependencies.go b/pkg/lint/rules/dependencies.go new file mode 100644 index 000000000..abecd1feb --- /dev/null +++ b/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 +} diff --git a/pkg/lint/rules/dependencies_test.go b/pkg/lint/rules/dependencies_test.go new file mode 100644 index 000000000..075190eac --- /dev/null +++ b/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) + } + } +}