From bb9426c4e2bc15a3b8db3338e6f82bc19fb47e2f Mon Sep 17 00:00:00 2001 From: Karuppiah Natarajan Date: Mon, 30 Sep 2019 23:41:11 +0530 Subject: [PATCH] fix(pkg/lint): fix lint silently ignoring non existing packaged charts Closes #6428 Signed-off-by: Karuppiah Natarajan --- .../chart-with-no-templates-dir/Chart.yaml | 5 ++ .../chart-with-no-templates-dir/values.yaml | 1 + .../testcharts/corrupted-compressed-chart.tgz | 0 pkg/action/lint.go | 33 +++++------ pkg/action/lint_test.go | 56 +++++++++++++++++++ pkg/lint/support/message.go | 2 +- 6 files changed, 76 insertions(+), 21 deletions(-) create mode 100644 cmd/helm/testdata/testcharts/chart-with-no-templates-dir/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-no-templates-dir/values.yaml create mode 100644 cmd/helm/testdata/testcharts/corrupted-compressed-chart.tgz diff --git a/cmd/helm/testdata/testcharts/chart-with-no-templates-dir/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-no-templates-dir/Chart.yaml new file mode 100644 index 000000000..d3458f6a2 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-no-templates-dir/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +name: chart-with-no-templates-dir +description: an example chart +version: 199.44.12345-Alpha.1+cafe009 +icon: http://riverrun.io diff --git a/cmd/helm/testdata/testcharts/chart-with-no-templates-dir/values.yaml b/cmd/helm/testdata/testcharts/chart-with-no-templates-dir/values.yaml new file mode 100644 index 000000000..ec82367be --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-with-no-templates-dir/values.yaml @@ -0,0 +1 @@ +justAValue: "an example chart here" diff --git a/cmd/helm/testdata/testcharts/corrupted-compressed-chart.tgz b/cmd/helm/testdata/testcharts/corrupted-compressed-chart.tgz new file mode 100644 index 000000000..e69de29bb diff --git a/pkg/action/lint.go b/pkg/action/lint.go index c5a77eb4e..9d57cfb00 100644 --- a/pkg/action/lint.go +++ b/pkg/action/lint.go @@ -29,8 +29,6 @@ import ( "helm.sh/helm/pkg/lint/support" ) -var errLintNoChart = errors.New("no chart found for linting (missing Chart.yaml)") - // Lint is the action for checking that the semantics of a chart are well-formed. // // It provides the implementation of 'helm lint'. @@ -56,24 +54,19 @@ func (l *Lint) Run(paths []string, vals map[string]interface{}) *LintResult { if l.Strict { lowestTolerance = support.WarningSev } - result := &LintResult{} for _, path := range paths { linter, err := lintChart(path, vals, l.Namespace, l.Strict) if err != nil { - if err == errLintNoChart { - result.Errors = append(result.Errors, err) - } - if linter.HighestSeverity >= lowestTolerance { - result.Errors = append(result.Errors, err) - } - } else { - result.Messages = append(result.Messages, linter.Messages...) - result.TotalChartsLinted++ - for _, msg := range linter.Messages { - if msg.Severity == support.ErrorSev { - result.Errors = append(result.Errors, msg.Err) - } + result.Errors = append(result.Errors, err) + continue + } + + result.Messages = append(result.Messages, linter.Messages...) + result.TotalChartsLinted++ + for _, msg := range linter.Messages { + if msg.Severity >= lowestTolerance { + result.Errors = append(result.Errors, msg.Err) } } } @@ -87,18 +80,18 @@ func lintChart(path string, vals map[string]interface{}, namespace string, stric if strings.HasSuffix(path, ".tgz") { tempDir, err := ioutil.TempDir("", "helm-lint") if err != nil { - return linter, err + return linter, errors.Wrap(err, "unable to create temp dir to extract tarball") } defer os.RemoveAll(tempDir) file, err := os.Open(path) if err != nil { - return linter, err + return linter, errors.Wrap(err, "unable to open tarball") } defer file.Close() if err = chartutil.Expand(tempDir, file); err != nil { - return linter, err + return linter, errors.Wrap(err, "unable to extract tarball") } lastHyphenIndex := strings.LastIndex(filepath.Base(path), "-") @@ -113,7 +106,7 @@ func lintChart(path string, vals map[string]interface{}, namespace string, stric // Guard: Error out if this is not a chart. if _, err := os.Stat(filepath.Join(chartPath, "Chart.yaml")); err != nil { - return linter, errLintNoChart + return linter, errors.Wrap(err, "unable to check Chart.yaml file in chart") } return lint.All(chartPath, vals, namespace, strict), nil diff --git a/pkg/action/lint_test.go b/pkg/action/lint_test.go index 7f7765af5..d5aaae9dd 100644 --- a/pkg/action/lint_test.go +++ b/pkg/action/lint_test.go @@ -33,6 +33,8 @@ var ( chartSchemaNegative = "../../cmd/helm/testdata/testcharts/chart-with-schema-negative" chart1MultipleChartLint = "../../cmd/helm/testdata/testcharts/multiplecharts-lint-chart-1" chart2MultipleChartLint = "../../cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2" + corruptedTgzChart = "../../cmd/helm/testdata/testcharts/corrupted-compressed-chart.tgz" + chartWithNoTemplatesDir = "../../cmd/helm/testdata/testcharts/chart-with-no-templates-dir" ) func TestLintChart(t *testing.T) { @@ -59,6 +61,40 @@ func TestLintChart(t *testing.T) { } } +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"} + expectedError := "unable to open tarball: open non-existent-chart.tgz: no such file or directory" + testLint := NewLint() + + result := testLint.Run(testCharts, values) + if len(result.Errors) != 1 { + t.Error("expected one error, but got", len(result.Errors)) + } + + actual := result.Errors[0].Error() + if actual != expectedError { + t.Errorf("expected '%s', but got '%s'", expectedError, actual) + } + }) + + t.Run("should error out for corrupted tgz chart", func(t *testing.T) { + testCharts := []string{corruptedTgzChart} + expectedEOFError := "unable to extract tarball: EOF" + testLint := NewLint() + + result := testLint.Run(testCharts, values) + if len(result.Errors) != 1 { + t.Error("expected one error, but got", len(result.Errors)) + } + + actual := result.Errors[0].Error() + if actual != expectedEOFError { + t.Errorf("expected '%s', but got '%s'", expectedEOFError, actual) + } + }) +} + func TestLint_MultipleCharts(t *testing.T) { testCharts := []string{chart2MultipleChartLint, chart1MultipleChartLint} testLint := NewLint() @@ -74,3 +110,23 @@ func TestLint_EmptyResultErrors(t *testing.T) { t.Error("Expected no error, got more") } } + +func TestLint_ChartWithWarnings(t *testing.T) { + t.Run("should pass when not strict", func(t *testing.T) { + testCharts := []string{chartWithNoTemplatesDir} + testLint := NewLint() + testLint.Strict = false + if result := testLint.Run(testCharts, values); len(result.Errors) > 0 { + t.Error("Expected no error, got more") + } + }) + + t.Run("should fail with errors when strict", func(t *testing.T) { + testCharts := []string{chartWithNoTemplatesDir} + testLint := NewLint() + testLint.Strict = true + if result := testLint.Run(testCharts, values); len(result.Errors) != 1 { + t.Error("expected one error, but got", len(result.Errors)) + } + }) +} diff --git a/pkg/lint/support/message.go b/pkg/lint/support/message.go index 4dd485c98..5efbc7a61 100644 --- a/pkg/lint/support/message.go +++ b/pkg/lint/support/message.go @@ -18,7 +18,7 @@ package support import "fmt" -// Severity indicatest the severity of a Message. +// Severity indicates the severity of a Message. const ( // UnknownSev indicates that the severity of the error is unknown, and should not stop processing. UnknownSev = iota