From c3bf83559e401b5d930aed86a2ff498869ade303 Mon Sep 17 00:00:00 2001 From: "Daniel J. Pritchett" Date: Fri, 16 Aug 2024 14:56:39 -0500 Subject: [PATCH] Integrates HIP-0019 lint ignorer with - adds ignorer usage to cmd/helm/lint.go - adds ignorer usage to pkg/action/lint.go and its lint_test.go See HIP-0019 proposal at helm/community: https://github.com/helm/community/pull/353 Co-authored-by: Danilo Patrucco Signed-off-by: Daniel J. Pritchett --- cmd/helm/lint.go | 4 +- pkg/action/lint.go | 21 ++++-- pkg/action/lint_test.go | 68 +++++++++++++++---- .../.helmlintignore | 4 ++ .../messy-chart-with-lintignore/Chart.yaml | 2 + .../requirements.lock | 6 ++ .../requirements.yaml | 7 ++ .../withsubchartlintignore/.helmlintignore | 3 + .../charts/withsubchartlintignore/Chart.yaml | 16 +++++ .../charts/subchart/Chart.yaml | 6 ++ .../charts/subchart/templates/subchart.yaml | 3 + .../charts/subchart/values.yaml | 2 + .../templates/mainchart.yaml | 2 + .../charts/withsubchartlintignore/values.yaml | 0 pkg/lint/support/message.go | 13 +++- 15 files changed, 138 insertions(+), 19 deletions(-) create mode 100644 pkg/action/testdata/charts/messy-chart-with-lintignore/.helmlintignore create mode 100755 pkg/action/testdata/charts/messy-chart-with-lintignore/Chart.yaml create mode 100755 pkg/action/testdata/charts/messy-chart-with-lintignore/requirements.lock create mode 100755 pkg/action/testdata/charts/messy-chart-with-lintignore/requirements.yaml create mode 100644 pkg/action/testdata/charts/withsubchartlintignore/.helmlintignore create mode 100644 pkg/action/testdata/charts/withsubchartlintignore/Chart.yaml create mode 100644 pkg/action/testdata/charts/withsubchartlintignore/charts/subchart/Chart.yaml create mode 100644 pkg/action/testdata/charts/withsubchartlintignore/charts/subchart/templates/subchart.yaml create mode 100644 pkg/action/testdata/charts/withsubchartlintignore/charts/subchart/values.yaml create mode 100644 pkg/action/testdata/charts/withsubchartlintignore/templates/mainchart.yaml create mode 100644 pkg/action/testdata/charts/withsubchartlintignore/values.yaml diff --git a/cmd/helm/lint.go b/cmd/helm/lint.go index 4c5e24149..fd3790563 100644 --- a/cmd/helm/lint.go +++ b/cmd/helm/lint.go @@ -46,6 +46,7 @@ func newLintCmd(out io.Writer) *cobra.Command { client := action.NewLint() valueOpts := &values.Options{} var kubeVersion string + var lintIgnoreFile string cmd := &cobra.Command{ Use: "lint PATH", @@ -91,7 +92,7 @@ func newLintCmd(out io.Writer) *cobra.Command { errorsOrWarnings := 0 for _, path := range paths { - result := client.Run([]string{path}, vals) + result := client.Run([]string{path}, vals, lintIgnoreFile, debug) // If there is no errors/warnings and quiet flag is set // go to the next chart @@ -150,6 +151,7 @@ func newLintCmd(out io.Writer) *cobra.Command { f.BoolVar(&client.Quiet, "quiet", false, "print only warnings and errors") f.BoolVar(&client.SkipSchemaValidation, "skip-schema-validation", false, "if set, disables JSON schema validation") f.StringVar(&kubeVersion, "kube-version", "", "Kubernetes version used for capabilities and deprecation checks") + f.StringVar(&lintIgnoreFile, "lint-ignore-file", "", "path to .helmlintignore file to specify ignore patterns") addValueOptionsFlags(f, valueOpts) return cmd diff --git a/pkg/action/lint.go b/pkg/action/lint.go index 63a1bf354..9d7129a8b 100644 --- a/pkg/action/lint.go +++ b/pkg/action/lint.go @@ -17,6 +17,7 @@ limitations under the License. package action import ( + "helm.sh/helm/v3/pkg/lint/ignore" "os" "path/filepath" "strings" @@ -37,6 +38,7 @@ type Lint struct { WithSubcharts bool Quiet bool SkipSchemaValidation bool + IgnoreFilePath string KubeVersion *chartutil.KubeVersion } @@ -53,23 +55,32 @@ func NewLint() *Lint { } // Run executes 'helm Lint' against the given chart. -func (l *Lint) Run(paths []string, vals map[string]interface{}) *LintResult { +func (l *Lint) Run(paths []string, vals map[string]interface{}, lintIgnoreFilePath string, debugLogFn func(string, ...interface{})) *LintResult { lowestTolerance := support.ErrorSev if l.Strict { lowestTolerance = support.WarningSev } result := &LintResult{} - for _, path := range paths { + for chartIndex, path := range paths { + // attempt to build an action-level lint result ignorer + ignorer, err := ignore.NewIgnorer(path, lintIgnoreFilePath, debugLogFn) linter, err := lintChart(path, vals, l.Namespace, l.KubeVersion, l.SkipSchemaValidation) if err != nil { - result.Errors = append(result.Errors, err) + // ❗ Discard ignorable errors as early as possible + if ignorer.ShouldKeepError(err) { + result.Errors = append(result.Errors, err) + } continue } - result.Messages = append(result.Messages, linter.Messages...) + // ❗ Discard ignorable messages as early as possible, BEFORE they get duplicated as errors + // in the loop below + keeperMessages := ignorer.FilterMessages(linter.Messages) + result.Messages = append(result.Messages, keeperMessages...) result.TotalChartsLinted++ - for _, msg := range linter.Messages { + for _, msg := range result.Messages { if msg.Severity >= lowestTolerance { + debugLogFn("action/lint/Run is promoting a message to Error", "chartIndex", chartIndex, "path", path, "lowestTolerance", lowestTolerance, msg.LogAttrs()) result.Errors = append(result.Errors, msg.Err) } } diff --git a/pkg/action/lint_test.go b/pkg/action/lint_test.go index a01580b0a..78649d35d 100644 --- a/pkg/action/lint_test.go +++ b/pkg/action/lint_test.go @@ -17,18 +17,33 @@ limitations under the License. package action import ( + "fmt" + "helm.sh/helm/v3/pkg/cli" + "log" "testing" ) var ( - values = make(map[string]interface{}) - namespace = "testNamespace" - chart1MultipleChartLint = "testdata/charts/multiplecharts-lint-chart-1" - chart2MultipleChartLint = "testdata/charts/multiplecharts-lint-chart-2" - corruptedTgzChart = "testdata/charts/corrupted-compressed-chart.tgz" - chartWithNoTemplatesDir = "testdata/charts/chart-with-no-templates-dir" + values = make(map[string]interface{}) + namespace = "testNamespace" + chart1MultipleChartLint = "testdata/charts/multiplecharts-lint-chart-1" + chart2MultipleChartLint = "testdata/charts/multiplecharts-lint-chart-2" + corruptedTgzChart = "testdata/charts/corrupted-compressed-chart.tgz" + chartWithNoTemplatesDir = "testdata/charts/chart-with-no-templates-dir" + messyChartWithLintIgnore = "testdata/charts/messy-chart-with-lintignore" ) +const emptyLintIgnoreFilePath = "" + +var settings = cli.New() + +func debugLogFn(format string, v ...interface{}) { + if settings.Debug { + format = fmt.Sprintf("[debug] %s\n", format) + log.Output(2, fmt.Sprintf(format, v...)) + } +} + func TestLintChart(t *testing.T) { tests := []struct { name string @@ -100,7 +115,7 @@ func TestNonExistentChart(t *testing.T) { expectedError := "unable to open tarball: open non-existent-chart.tgz: no such file or directory" testLint := NewLint() - result := testLint.Run(testCharts, values) + result := testLint.Run(testCharts, values, emptyLintIgnoreFilePath, debugLogFn) if len(result.Errors) != 1 { t.Error("expected one error, but got", len(result.Errors)) } @@ -116,7 +131,7 @@ func TestNonExistentChart(t *testing.T) { expectedEOFError := "unable to extract tarball: EOF" testLint := NewLint() - result := testLint.Run(testCharts, values) + result := testLint.Run(testCharts, values, emptyLintIgnoreFilePath, debugLogFn) if len(result.Errors) != 1 { t.Error("expected one error, but got", len(result.Errors)) } @@ -131,7 +146,7 @@ func TestNonExistentChart(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, emptyLintIgnoreFilePath, debugLogFn); len(result.Errors) > 0 { t.Error(result.Errors) } } @@ -139,7 +154,7 @@ func TestLint_MultipleCharts(t *testing.T) { func TestLint_EmptyResultErrors(t *testing.T) { testCharts := []string{chart2MultipleChartLint} testLint := NewLint() - if result := testLint.Run(testCharts, values); len(result.Errors) > 0 { + if result := testLint.Run(testCharts, values, emptyLintIgnoreFilePath, debugLogFn); len(result.Errors) > 0 { t.Error("Expected no error, got more") } } @@ -149,7 +164,7 @@ func TestLint_ChartWithWarnings(t *testing.T) { testCharts := []string{chartWithNoTemplatesDir} testLint := NewLint() testLint.Strict = false - if result := testLint.Run(testCharts, values); len(result.Errors) > 0 { + if result := testLint.Run(testCharts, values, emptyLintIgnoreFilePath, debugLogFn); len(result.Errors) > 0 { t.Error("Expected no error, got more") } }) @@ -158,8 +173,37 @@ func TestLint_ChartWithWarnings(t *testing.T) { testCharts := []string{chartWithNoTemplatesDir} testLint := NewLint() testLint.Strict = true - if result := testLint.Run(testCharts, values); len(result.Errors) != 0 { + if result := testLint.Run(testCharts, values, emptyLintIgnoreFilePath, debugLogFn); len(result.Errors) != 0 { t.Error("expected no errors, but got", len(result.Errors)) } }) } + +func TestLint_MessyChartWithLintIgnoreFile(t *testing.T) { + t.Run("should find no errors or messages using its own .helmlintignore file", func(t *testing.T) { + testCharts := []string{messyChartWithLintIgnore} + testLint := NewLint() + testLint.Strict = false + result := testLint.Run(testCharts, values, emptyLintIgnoreFilePath, debugLogFn) + if len(result.Errors) != 0 { + t.Error("expected no errors, but got", len(result.Errors)) + } + if len(result.Messages) != 0 { + t.Error("expected no messages, but got", len(result.Messages)) + } + }) + + t.Run("should find all four errors when we feed it a fake lint ignore file path", func(t *testing.T) { + fakeLintIgnoreFilePath := "some/file/might/exist/here" + testCharts := []string{messyChartWithLintIgnore} + testLint := NewLint() + testLint.Strict = true + result := testLint.Run(testCharts, values, fakeLintIgnoreFilePath, debugLogFn) + if len(result.Errors) != 2 { + t.Error("expected two errors, but got", len(result.Errors)) + } + if len(result.Messages) != 4 { + t.Error("expected four messages, but got", len(result.Messages)) + } + }) +} diff --git a/pkg/action/testdata/charts/messy-chart-with-lintignore/.helmlintignore b/pkg/action/testdata/charts/messy-chart-with-lintignore/.helmlintignore new file mode 100644 index 000000000..74dbf7e43 --- /dev/null +++ b/pkg/action/testdata/charts/messy-chart-with-lintignore/.helmlintignore @@ -0,0 +1,4 @@ +error_lint_ignore=icon is recommended +error_lint_ignore=apiVersion is required. The value must be either \"v1\" or \"v2\" +error_lint_ignore=file does not exist +error_lint_ignore=chart directory is missing these dependencies: mariadb diff --git a/pkg/action/testdata/charts/messy-chart-with-lintignore/Chart.yaml b/pkg/action/testdata/charts/messy-chart-with-lintignore/Chart.yaml new file mode 100755 index 000000000..ba10ee803 --- /dev/null +++ b/pkg/action/testdata/charts/messy-chart-with-lintignore/Chart.yaml @@ -0,0 +1,2 @@ +name: chart-with-missing-deps +version: 2.1.8 diff --git a/pkg/action/testdata/charts/messy-chart-with-lintignore/requirements.lock b/pkg/action/testdata/charts/messy-chart-with-lintignore/requirements.lock new file mode 100755 index 000000000..dcda2b142 --- /dev/null +++ b/pkg/action/testdata/charts/messy-chart-with-lintignore/requirements.lock @@ -0,0 +1,6 @@ +dependencies: +- name: mariadb + repository: https://charts.helm.sh/stable/ + version: 4.3.1 +digest: sha256:82a0e5374376169d2ecf7d452c18a2ed93507f5d17c3393a1457f9ffad7e9b26 +generated: 2018-08-02T22:07:51.905271776Z diff --git a/pkg/action/testdata/charts/messy-chart-with-lintignore/requirements.yaml b/pkg/action/testdata/charts/messy-chart-with-lintignore/requirements.yaml new file mode 100755 index 000000000..fef7d0b7f --- /dev/null +++ b/pkg/action/testdata/charts/messy-chart-with-lintignore/requirements.yaml @@ -0,0 +1,7 @@ +dependencies: +- name: mariadb + version: 4.x.x + repository: https://charts.helm.sh/stable/ + condition: mariadb.enabled + tags: + - wordpress-database diff --git a/pkg/action/testdata/charts/withsubchartlintignore/.helmlintignore b/pkg/action/testdata/charts/withsubchartlintignore/.helmlintignore new file mode 100644 index 000000000..5218810db --- /dev/null +++ b/pkg/action/testdata/charts/withsubchartlintignore/.helmlintignore @@ -0,0 +1,3 @@ +error_lint_ignore=chart metadata is missing these dependencies** +rules/testdata/withsubchartlintignore/charts/subchart/templates/subchart.yaml +rules/testdata/withsubchartlintignore/charts/subchart/templates/subchart.yaml unexpected EOF \ No newline at end of file diff --git a/pkg/action/testdata/charts/withsubchartlintignore/Chart.yaml b/pkg/action/testdata/charts/withsubchartlintignore/Chart.yaml new file mode 100644 index 000000000..6648daf56 --- /dev/null +++ b/pkg/action/testdata/charts/withsubchartlintignore/Chart.yaml @@ -0,0 +1,16 @@ +apiVersion: v2 +name: withsubchart +description: A Helm chart for Kubernetes +type: application +version: 0.1.0 +appVersion: "1.16.0" +icon: http://riverrun.io + +dependencies: + - name: subchart + version: 0.1.16 + repository: "file://../subchart" + import-values: + - child: subchart + parent: subchart + diff --git a/pkg/action/testdata/charts/withsubchartlintignore/charts/subchart/Chart.yaml b/pkg/action/testdata/charts/withsubchartlintignore/charts/subchart/Chart.yaml new file mode 100644 index 000000000..8610a4f25 --- /dev/null +++ b/pkg/action/testdata/charts/withsubchartlintignore/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: "1.16.0" diff --git a/pkg/action/testdata/charts/withsubchartlintignore/charts/subchart/templates/subchart.yaml b/pkg/action/testdata/charts/withsubchartlintignore/charts/subchart/templates/subchart.yaml new file mode 100644 index 000000000..f1f0cbf45 --- /dev/null +++ b/pkg/action/testdata/charts/withsubchartlintignore/charts/subchart/templates/subchart.yaml @@ -0,0 +1,3 @@ +metadata: + name: {{ .Values.subchart.name | lower }} +{{- if not (include "this.is.test.data" .) }} \ No newline at end of file diff --git a/pkg/action/testdata/charts/withsubchartlintignore/charts/subchart/values.yaml b/pkg/action/testdata/charts/withsubchartlintignore/charts/subchart/values.yaml new file mode 100644 index 000000000..422a359d5 --- /dev/null +++ b/pkg/action/testdata/charts/withsubchartlintignore/charts/subchart/values.yaml @@ -0,0 +1,2 @@ +subchart: + name: subchart \ No newline at end of file diff --git a/pkg/action/testdata/charts/withsubchartlintignore/templates/mainchart.yaml b/pkg/action/testdata/charts/withsubchartlintignore/templates/mainchart.yaml new file mode 100644 index 000000000..6cb6cc2af --- /dev/null +++ b/pkg/action/testdata/charts/withsubchartlintignore/templates/mainchart.yaml @@ -0,0 +1,2 @@ +metadata: + name: {{ .Values.subchart.name | lower }} diff --git a/pkg/action/testdata/charts/withsubchartlintignore/values.yaml b/pkg/action/testdata/charts/withsubchartlintignore/values.yaml new file mode 100644 index 000000000..e69de29bb diff --git a/pkg/lint/support/message.go b/pkg/lint/support/message.go index 5efbc7a61..4ff30a205 100644 --- a/pkg/lint/support/message.go +++ b/pkg/lint/support/message.go @@ -16,7 +16,10 @@ limitations under the License. package support -import "fmt" +import ( + "fmt" + "log/slog" +) // Severity indicates the severity of a Message. const ( @@ -53,6 +56,14 @@ func (m Message) Error() string { return fmt.Sprintf("[%s] %s: %s", sev[m.Severity], m.Path, m.Err.Error()) } +func (m Message) LogAttrs() slog.Attr { + return slog.Group("Message", + slog.Int("Severity", m.Severity), + slog.String("Path", m.Path), + slog.String("Err", m.Err.Error()), + ) +} + // NewMessage creates a new Message struct func NewMessage(severity int, path string, err error) Message { return Message{Severity: severity, Path: path, Err: err}