From 7da1f35386dc0116ab51bcb2d6374356cf87e16a Mon Sep 17 00:00:00 2001 From: Vibhav Bobade Date: Sat, 17 Aug 2019 02:02:16 +0530 Subject: [PATCH 1/3] Clone the vals map for every path to avoid mutation Signed-off-by: Vibhav Bobade --- pkg/action/lint.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/action/lint.go b/pkg/action/lint.go index d7ca8cd03..6a0ec1488 100644 --- a/pkg/action/lint.go +++ b/pkg/action/lint.go @@ -77,6 +77,10 @@ func (l *Lint) Run(paths []string, vals map[string]interface{}) *LintResult { func lintChart(path string, vals map[string]interface{}, namespace string, strict bool) (support.Linter, error) { var chartPath string linter := support.Linter{} + currentVals := make(map[string]interface{}, len(vals)) + for key, value := range vals { + currentVals[key] = value + } if strings.HasSuffix(path, ".tgz") { tempDir, err := ioutil.TempDir("", "helm-lint") @@ -110,5 +114,5 @@ func lintChart(path string, vals map[string]interface{}, namespace string, stric return linter, errLintNoChart } - return lint.All(chartPath, vals, namespace, strict), nil + return lint.All(chartPath, currentVals, namespace, strict), nil } From 9ab927f0bb352a4b5f8079430e435966a6821169 Mon Sep 17 00:00:00 2001 From: Vibhav Bobade Date: Sat, 17 Aug 2019 14:23:22 +0530 Subject: [PATCH 2/3] Test for Linting multiple charts with the same vals instance Signed-off-by: Vibhav Bobade --- .../testcharts/multiplecharts-lint-chart-1/Chart.yaml | 4 ++++ .../templates/configmap.yaml | 6 ++++++ .../multiplecharts-lint-chart-1/values.yaml | 1 + .../testcharts/multiplecharts-lint-chart-2/Chart.yaml | 4 ++++ .../templates/configmap.yaml | 5 +++++ .../multiplecharts-lint-chart-2/values.yaml | 2 ++ pkg/action/lint_test.go | 11 +++++++++++ 7 files changed, 33 insertions(+) create mode 100644 cmd/helm/testdata/testcharts/multiplecharts-lint-chart-1/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/multiplecharts-lint-chart-1/templates/configmap.yaml create mode 100644 cmd/helm/testdata/testcharts/multiplecharts-lint-chart-1/values.yaml create mode 100644 cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2/templates/configmap.yaml create mode 100644 cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2/values.yaml diff --git a/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-1/Chart.yaml b/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-1/Chart.yaml new file mode 100644 index 000000000..3b342ae4f --- /dev/null +++ b/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-1/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +name: multiplecharts-lint-chart-1 +version: 1 +icon: "" \ No newline at end of file diff --git a/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-1/templates/configmap.yaml b/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-1/templates/configmap.yaml new file mode 100644 index 000000000..88ebf2468 --- /dev/null +++ b/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-1/templates/configmap.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +metadata: + name: multicharttest-chart1-configmap +data: + dat: | + {{ .Values.config | indent 4 }} diff --git a/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-1/values.yaml b/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-1/values.yaml new file mode 100644 index 000000000..aafb09e4b --- /dev/null +++ b/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-1/values.yaml @@ -0,0 +1 @@ +config: "Test" \ No newline at end of file diff --git a/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2/Chart.yaml b/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2/Chart.yaml new file mode 100644 index 000000000..bd101d808 --- /dev/null +++ b/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +name: multiplecharts-lint-chart-2 +version: 1 +icon: "" \ No newline at end of file diff --git a/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2/templates/configmap.yaml b/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2/templates/configmap.yaml new file mode 100644 index 000000000..8484bfe6a --- /dev/null +++ b/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2/templates/configmap.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +metadata: + name: multicharttest-chart2-configmap +data: + {{ toYaml .Values.config | indent 4 }} diff --git a/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2/values.yaml b/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2/values.yaml new file mode 100644 index 000000000..9139f486e --- /dev/null +++ b/cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2/values.yaml @@ -0,0 +1,2 @@ +config: + test: "Test" \ No newline at end of file diff --git a/pkg/action/lint_test.go b/pkg/action/lint_test.go index eec9f9533..45af0df95 100644 --- a/pkg/action/lint_test.go +++ b/pkg/action/lint_test.go @@ -31,6 +31,8 @@ var ( chartMissingManifest = "../../cmd/helm/testdata/testcharts/chart-missing-manifest" chartSchema = "../../cmd/helm/testdata/testcharts/chart-with-schema" 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" ) func TestLintChart(t *testing.T) { @@ -56,3 +58,12 @@ func TestLintChart(t *testing.T) { t.Error(err) } } + +func TestLint_MultipleCharts(t *testing.T) { + testCharts := []string{chart2MultipleChartLint, chart1MultipleChartLint} + testLint := NewLint() + if result := testLint.Run(testCharts, values); len(result.Errors) == 0 { + t.Error(result.Errors) + } + +} From 68a8f36a9288b918767bd21d107cac3ccfda22c0 Mon Sep 17 00:00:00 2001 From: Vibhav Bobade Date: Mon, 19 Aug 2019 03:58:48 +0530 Subject: [PATCH 3/3] Fix Adding Errors from Linter.Messages to result.Errors This also fixes beig added to result.Errors Signed-off-by: Vibhav Bobade --- pkg/action/lint.go | 13 ++++++++++--- pkg/action/lint_test.go | 9 ++++++++- pkg/lint/rules/template.go | 4 +--- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/pkg/action/lint.go b/pkg/action/lint.go index 6a0ec1488..f9c0c613f 100644 --- a/pkg/action/lint.go +++ b/pkg/action/lint.go @@ -59,15 +59,22 @@ func (l *Lint) Run(paths []string, vals map[string]interface{}) *LintResult { result := &LintResult{} for _, path := range paths { - if linter, err := lintChart(path, vals, l.Namespace, l.Strict); err != nil { + 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++ - if linter.HighestSeverity >= lowestTolerance { - result.Errors = append(result.Errors, err) + for _, msg := range linter.Messages { + if msg.Severity == support.ErrorSev { + result.Errors = append(result.Errors, msg.Err) + result.Messages = append(result.Messages, msg) + } } } } diff --git a/pkg/action/lint_test.go b/pkg/action/lint_test.go index 45af0df95..7f7765af5 100644 --- a/pkg/action/lint_test.go +++ b/pkg/action/lint_test.go @@ -62,8 +62,15 @@ func TestLintChart(t *testing.T) { func TestLint_MultipleCharts(t *testing.T) { testCharts := []string{chart2MultipleChartLint, chart1MultipleChartLint} testLint := NewLint() - if result := testLint.Run(testCharts, values); len(result.Errors) == 0 { + if result := testLint.Run(testCharts, values); len(result.Errors) > 0 { t.Error(result.Errors) } +} +func TestLint_EmptyResultErrors(t *testing.T) { + testCharts := []string{chart2MultipleChartLint} + testLint := NewLint() + if result := testLint.Run(testCharts, values); len(result.Errors) > 0 { + t.Error("Expected no error, got more") + } } diff --git a/pkg/lint/rules/template.go b/pkg/lint/rules/template.go index db86417dc..36230c4ee 100644 --- a/pkg/lint/rules/template.go +++ b/pkg/lint/rules/template.go @@ -61,9 +61,7 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace } valuesToRender, err := chartutil.ToRenderValues(chart, cvals, options, nil) if err != nil { - // FIXME: This seems to generate a duplicate, but I can't find where the first - // error is coming from. - //linter.RunLinterRule(support.ErrorSev, err) + linter.RunLinterRule(support.ErrorSev, path, err) return } var e engine.Engine