From 104930f4dfa49bdf48f442524111d61290b58d42 Mon Sep 17 00:00:00 2001 From: Raffael Sahli Date: Fri, 31 Jan 2020 12:37:37 +0100 Subject: [PATCH] fix(lint): add --recursive flag to 'helm lint' When 'helm lint PATH --recursive' is run, internal sub charts are linted as well. This is needed where an umbrella chart holds sub charts. Note that external dependencies are not linted. Closes #4910 Signed-off-by: Raffael Sahli --- cmd/helm/lint.go | 5 ++- .../chart-with-invalidsubchart/Chart.yaml | 6 +++ .../charts/subchart/Chart.yaml | 6 +++ .../charts/subchart/templates/empty.yaml | 1 + .../charts/subchart/values.yaml | 0 .../templates/empty.yaml | 1 + .../chart-with-invalidsubchart/values.yaml | 1 + .../testcharts/chart-with-subchart/Chart.yaml | 6 +++ .../charts/subchart/Chart.yaml | 6 +++ .../charts/subchart/templates/empty.yaml | 1 + .../charts/subchart/values.yaml | 0 .../chart-with-subchart/templates/empty.yaml | 1 + .../chart-with-subchart/values.yaml | 1 + pkg/action/lint.go | 40 +++++++++++++++++ pkg/action/lint_test.go | 43 +++++++++++++++++++ 15 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 cmd/helm/testdata/testcharts/chart-with-invalidsubchart/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-invalidsubchart/charts/subchart/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-invalidsubchart/charts/subchart/templates/empty.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-invalidsubchart/charts/subchart/values.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-invalidsubchart/templates/empty.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-invalidsubchart/values.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart/charts/subchart/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart/charts/subchart/templates/empty.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart/charts/subchart/values.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart/templates/empty.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-subchart/values.yaml diff --git a/cmd/helm/lint.go b/cmd/helm/lint.go index 9a2e8d31c..01d65a6e6 100644 --- a/cmd/helm/lint.go +++ b/cmd/helm/lint.go @@ -59,11 +59,13 @@ func newLintCmd(out io.Writer) *cobra.Command { var message strings.Builder failed := 0 + total := 0 for _, path := range paths { fmt.Fprintf(&message, "==> Linting %s\n", path) result := client.Run([]string{path}, vals) + total += result.TotalChartsLinted // All the Errors that are generated by a chart // that failed a lint will be included in the @@ -91,7 +93,7 @@ func newLintCmd(out io.Writer) *cobra.Command { fmt.Fprintf(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", total, failed) if failed > 0 { return errors.New(summary) } @@ -102,6 +104,7 @@ func newLintCmd(out io.Writer) *cobra.Command { f := cmd.Flags() f.BoolVar(&client.Strict, "strict", false, "fail on lint warnings") + f.BoolVar(&client.Recursive, "recursive", false, "lint sub charts") addValueOptionsFlags(f, valueOpts) return cmd diff --git a/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/Chart.yaml new file mode 100644 index 000000000..dd7697345 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +name: chart-with-invalidsubchart +description: A Helm chart for Kubernetes +type: application +version: 0.1.0 +appVersion: 0.1.0 diff --git a/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/charts/subchart/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/charts/subchart/Chart.yaml new file mode 100644 index 000000000..b0e637c81 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/charts/subchart/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +name: subchart +description: A Helm chart for Kubernetes +type: application +version: 0.1.0 +appVersion: 0.1.0 diff --git a/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/charts/subchart/templates/empty.yaml b/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/charts/subchart/templates/empty.yaml new file mode 100644 index 000000000..c80812f6e --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/charts/subchart/templates/empty.yaml @@ -0,0 +1 @@ +# This file is intentionally blank diff --git a/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/charts/subchart/values.yaml b/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/charts/subchart/values.yaml new file mode 100644 index 000000000..e69de29bb diff --git a/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/templates/empty.yaml b/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/templates/empty.yaml new file mode 100644 index 000000000..c80812f6e --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/templates/empty.yaml @@ -0,0 +1 @@ +# This file is intentionally blank diff --git a/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/values.yaml b/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/values.yaml new file mode 100644 index 000000000..c9deafc00 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-invalidsubchart/values.yaml @@ -0,0 +1 @@ +firstname: "John" diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart/Chart.yaml new file mode 100644 index 000000000..93d2882a2 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +name: chart-with-subchart +description: A Helm chart for Kubernetes +type: application +version: 0.1.0 +appVersion: 0.1.0 diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart/charts/subchart/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart/charts/subchart/Chart.yaml new file mode 100644 index 000000000..b0e637c81 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart/charts/subchart/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +name: subchart +description: A Helm chart for Kubernetes +type: application +version: 0.1.0 +appVersion: 0.1.0 diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart/charts/subchart/templates/empty.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart/charts/subchart/templates/empty.yaml new file mode 100644 index 000000000..c80812f6e --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart/charts/subchart/templates/empty.yaml @@ -0,0 +1 @@ +# This file is intentionally blank diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart/charts/subchart/values.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart/charts/subchart/values.yaml new file mode 100644 index 000000000..e69de29bb diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart/templates/empty.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart/templates/empty.yaml new file mode 100644 index 000000000..c80812f6e --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart/templates/empty.yaml @@ -0,0 +1 @@ +# This file is intentionally blank diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart/values.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart/values.yaml new file mode 100644 index 000000000..c9deafc00 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-subchart/values.yaml @@ -0,0 +1 @@ +firstname: "John" diff --git a/pkg/action/lint.go b/pkg/action/lint.go index c216c5832..b919b3def 100644 --- a/pkg/action/lint.go +++ b/pkg/action/lint.go @@ -34,6 +34,7 @@ import ( // It provides the implementation of 'helm lint'. type Lint struct { Strict bool + Recursive bool Namespace string } @@ -62,6 +63,16 @@ func (l *Lint) Run(paths []string, vals map[string]interface{}) *LintResult { continue } + //Lint sub charts within an umbrella chart + if l.Recursive { + subResult := l.recursiveLint(result, path, vals) + if subResult != nil { + result.Messages = append(result.Messages, subResult.Messages...) + result.Errors = append(result.Errors, subResult.Errors...) + result.TotalChartsLinted += subResult.TotalChartsLinted + } + } + result.Messages = append(result.Messages, linter.Messages...) result.TotalChartsLinted++ for _, msg := range linter.Messages { @@ -73,6 +84,35 @@ func (l *Lint) Run(paths []string, vals map[string]interface{}) *LintResult { return result } +func (l *Lint) recursiveLint(result *LintResult, path string, vals map[string]interface{}) *LintResult { + subPath := filepath.Join(path, "charts") + info, err := os.Stat(subPath) + if os.IsNotExist(err) { + return nil + } + + if !info.IsDir() { + return nil + } + + files, err := ioutil.ReadDir(subPath) + + if err != nil { + result.Errors = append(result.Errors, errors.Wrapf(err, "unable to read sub charts directory %s", subPath)) + return nil + } + + var subPaths []string + for _, path := range files { + if path.IsDir() { + chartPath := filepath.Join(subPath, path.Name()) + subPaths = append(subPaths, chartPath) + } + } + + return l.Run(subPaths, vals) +} + func lintChart(path string, vals map[string]interface{}, namespace string, strict bool) (support.Linter, error) { var chartPath string linter := support.Linter{} diff --git a/pkg/action/lint_test.go b/pkg/action/lint_test.go index 68d3c4bb4..f3e8725ee 100644 --- a/pkg/action/lint_test.go +++ b/pkg/action/lint_test.go @@ -17,6 +17,7 @@ limitations under the License. package action import ( + "fmt" "testing" ) @@ -89,6 +90,48 @@ func TestLintChart(t *testing.T) { } } +func TestRecursiveUmbrellaChart(t *testing.T) { + t.Run("should lint both, the umbrella chart and its sub chart", func(t *testing.T) { + testCharts := []string{"../../cmd/helm/testdata/testcharts/chart-with-subchart"} + testLint := NewLint() + testLint.Recursive = true + + result := testLint.Run(testCharts, values) + if len(result.Errors) != 0 { + t.Error("expected no error, but got", len(result.Errors)) + } + + if result.TotalChartsLinted != 2 { + t.Error("expected a total of two linted charts", result.TotalChartsLinted) + } + }) + + t.Run("should error out if sub chart got an error", func(t *testing.T) { + testCharts := []string{"../../cmd/helm/testdata/testcharts/chart-with-invalidsubchart"} + testLint := NewLint() + testLint.Recursive = true + expectedError := "chart type is not valid in apiVersion 'v1'. It is valid in apiVersion 'v2'" + + result := testLint.Run(testCharts, values) + if len(result.Errors) != 1 { + t.Error("expected one error, but got", len(result.Errors)) + } + + for _, err := range result.Errors { + fmt.Printf("%#v\n", err) + } + + if result.TotalChartsLinted != 2 { + t.Error("expected a total of two linted charts", result.TotalChartsLinted) + } + + actual := result.Errors[0].Error() + if actual != expectedError { + t.Errorf("expected '%s', but got '%s'", expectedError, actual) + } + }) +} + func TestNonExistentChart(t *testing.T) { t.Run("should error out for non existent tgz chart", func(t *testing.T) { testCharts := []string{"non-existent-chart.tgz"}