Skip to content

Commit

Permalink
Integrates HIP-0019 lint ignorer with
Browse files Browse the repository at this point in the history
- 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: helm/community#353

Co-authored-by: Danilo Patrucco <[email protected]>

Signed-off-by: Daniel J. Pritchett <[email protected]>
  • Loading branch information
dpritchett committed Aug 16, 2024
1 parent f93c255 commit c3bf835
Show file tree
Hide file tree
Showing 15 changed files with 138 additions and 19 deletions.
4 changes: 3 additions & 1 deletion cmd/helm/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
21 changes: 16 additions & 5 deletions pkg/action/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package action

import (
"helm.sh/helm/v3/pkg/lint/ignore"
"os"
"path/filepath"
"strings"
Expand All @@ -37,6 +38,7 @@ type Lint struct {
WithSubcharts bool
Quiet bool
SkipSchemaValidation bool
IgnoreFilePath string
KubeVersion *chartutil.KubeVersion
}

Expand All @@ -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)
}
}
Expand Down
68 changes: 56 additions & 12 deletions pkg/action/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
}
Expand All @@ -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))
}
Expand All @@ -131,15 +146,15 @@ 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)
}
}

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")
}
}
Expand All @@ -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")
}
})
Expand All @@ -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))
}
})
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
name: chart-with-missing-deps
version: 2.1.8
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
dependencies:
- name: mariadb
version: 4.x.x
repository: https://charts.helm.sh/stable/
condition: mariadb.enabled
tags:
- wordpress-database
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
error_lint_ignore=chart metadata is missing these dependencies**
rules/testdata/withsubchartlintignore/charts/subchart/templates/subchart.yaml <include "this.is.test.data" .>
rules/testdata/withsubchartlintignore/charts/subchart/templates/subchart.yaml unexpected EOF
16 changes: 16 additions & 0 deletions pkg/action/testdata/charts/withsubchartlintignore/Chart.yaml
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
apiVersion: v2
name: subchart
description: A Helm chart for Kubernetes
type: application
version: 0.1.0
appVersion: "1.16.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
metadata:
name: {{ .Values.subchart.name | lower }}
{{- if not (include "this.is.test.data" .) }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
subchart:
name: subchart
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
metadata:
name: {{ .Values.subchart.name | lower }}
Empty file.
13 changes: 12 additions & 1 deletion pkg/lint/support/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ limitations under the License.

package support

import "fmt"
import (
"fmt"
"log/slog"
)

// Severity indicates the severity of a Message.
const (
Expand Down Expand Up @@ -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}
Expand Down

0 comments on commit c3bf835

Please sign in to comment.