diff --git a/cmd/helm/lint.go b/cmd/helm/lint.go index 2f3129b65..0c259d987 100644 --- a/cmd/helm/lint.go +++ b/cmd/helm/lint.go @@ -31,7 +31,6 @@ import ( "helm.sh/helm/v3/pkg/cli/values" "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/lint/rules" - "helm.sh/helm/v3/pkg/lint/support" ) var longLintHelp = ` @@ -45,6 +44,7 @@ or recommendation, it will emit [WARNING] messages. func newLintCmd(out io.Writer) *cobra.Command { client := action.NewLint() + client.Debug = settings.Debug valueOpts := &values.Options{} var kubeVersion string var lintIgnoreFile string @@ -92,15 +92,16 @@ func newLintCmd(out io.Writer) *cobra.Command { var message strings.Builder failed := 0 for _, path := range paths { - result := client.Run([]string{path}, vals) - filteredResult := FilterIgnoredMessages(result, ignorePatterns, settings.Debug) fmt.Fprintf(&message, "==> Linting %s\n", path) - for _, msg := range filteredResult.Messages { + result := client.Run([]string{path}, vals) + result.Messages = rules.FilterIgnoredMessages(result.Messages, ignorePatterns) + result.Errors = rules.FilterIgnoredErrors(result.Errors, ignorePatterns) + for _, msg := range result.Messages { fmt.Fprintf(&message, "%s\n", msg) } - if len(filteredResult.Messages) != 0 { + if len(result.Messages) != 0 { failed++ - for _, err := range filteredResult.Errors { + for _, err := range result.Errors { fmt.Fprintf(&message, "Error: %s\n", err) } } @@ -124,58 +125,4 @@ func newLintCmd(out io.Writer) *cobra.Command { f.StringVar(&lintIgnoreFile, "lint-ignore-file", "", "path to .helmlintignore file to specify ignore patterns") return cmd -} - -func FilterIgnoredMessages(result *action.LintResult, patterns map[string][]string, debug bool) *action.LintResult { - filteredMessages := make([]support.Message, 0) - for _, msg := range result.Messages { - fullPath := extractFullPathFromError(msg.Err.Error()) - if len(fullPath) == 0 { - break - } - if debug { - fmt.Printf("Extracted full path: %s\n", fullPath) - } - ignore := false - for path, pathPatterns := range patterns { - if strings.Contains(fullPath, filepath.Clean(path)) { - for _, pattern := range pathPatterns { - if strings.Contains(msg.Err.Error(), pattern) { - if debug { - fmt.Printf("Ignoring message: [%s] %s\n\n", fullPath, msg.Err.Error()) - } - ignore = true - break - } - } - } - if ignore { - break - } - } - if !ignore { - filteredMessages = append(filteredMessages, msg) - } - } - return &action.LintResult{Messages: filteredMessages} -} - -func extractFullPathFromError(errorString string) string { - parts := strings.Split(errorString, ":") - if len(parts) > 2 { - return strings.TrimSpace(parts[1]) - } - return "" -} - -/* TODO HIP-0019 - - find ignore file path for a subchart - - add a chart or two for the end to end tests via testdata like in pkg/lint/lint_test.go - - review debug / output patterns across the helm project - - Later/never - - XDG support - - helm config file support - - ignore file validation - - -*/ \ No newline at end of file +} \ No newline at end of file diff --git a/cmd/helm/lint_test.go b/cmd/helm/lint_test.go index 166b69ba0..08d449f23 100644 --- a/cmd/helm/lint_test.go +++ b/cmd/helm/lint_test.go @@ -94,4 +94,4 @@ func TestLintCmdWithKubeVersionFlag(t *testing.T) { func TestLintFileCompletion(t *testing.T) { checkFileCompletion(t, "lint", true) checkFileCompletion(t, "lint mypath", true) // Multiple paths can be given -} +} \ No newline at end of file diff --git a/pkg/action/lint.go b/pkg/action/lint.go index 150768398..78cfbaa70 100644 --- a/pkg/action/lint.go +++ b/pkg/action/lint.go @@ -9,7 +9,6 @@ import ( "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/lint" - // "helm.sh/helm/v3/pkg/lint/rules" "helm.sh/helm/v3/pkg/lint/support" ) @@ -21,6 +20,7 @@ type Lint struct { Quiet bool KubeVersion *chartutil.KubeVersion IgnoreFilePath string + Debug bool } type LintResult struct { TotalChartsLinted int @@ -38,7 +38,7 @@ func (l *Lint) Run(paths []string, vals map[string]interface{}) *LintResult { result := &LintResult{} for _, path := range paths { - linter, err := lintChart(path, vals, l.Namespace, l.KubeVersion, l.IgnoreFilePath) + linter, err := lintChart(path, vals, l.Namespace, l.KubeVersion) if err != nil { result.Errors = append(result.Errors, err) continue @@ -64,7 +64,7 @@ func HasWarningsOrErrors(result *LintResult) bool { return len(result.Errors) > 0 } -func lintChart(path string, vals map[string]interface{}, namespace string, kubeVersion *chartutil.KubeVersion, ignoreFilePath string) (support.Linter, error) { +func lintChart(path string, vals map[string]interface{}, namespace string, kubeVersion *chartutil.KubeVersion) (support.Linter, error) { var chartPath string linter := support.Linter{} @@ -101,5 +101,5 @@ func lintChart(path string, vals map[string]interface{}, namespace string, kubeV if _, err := os.Stat(filepath.Join(chartPath, "Chart.yaml")); err != nil { return linter, errors.Wrap(err, "Chart.yaml file not found in chart") } - return lint.AllWithKubeVersion(chartPath, vals, namespace, kubeVersion, ignoreFilePath), nil + return lint.AllWithKubeVersion(chartPath, vals, namespace, kubeVersion), nil } diff --git a/pkg/lint/lint.go b/pkg/lint/lint.go index 178079839..fcf424e55 100644 --- a/pkg/lint/lint.go +++ b/pkg/lint/lint.go @@ -17,34 +17,17 @@ limitations under the License. package lint import ( - "fmt" - "log" - "io/ioutil" - "path/filepath" "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/lint/rules" "helm.sh/helm/v3/pkg/lint/support" + "path/filepath" ) func All(basedir string, values map[string]interface{}, namespace string, _ bool) support.Linter { - return AllWithKubeVersion(basedir, values, namespace, nil, "") + return AllWithKubeVersion(basedir, values, namespace, nil) } -func AllWithKubeVersion(basedir string, values map[string]interface{}, namespace string, kubeVersion *chartutil.KubeVersion, lintIgnoreFile string) support.Linter { +func AllWithKubeVersion(basedir string, values map[string]interface{}, namespace string, kubeVersion *chartutil.KubeVersion) support.Linter { chartDir, _ := filepath.Abs(basedir) - var ignorePatterns map[string][]string - var err error - if lintIgnoreFile != "" { - ignorePatterns, err = rules.ParseIgnoreFile(lintIgnoreFile) - for key, value := range ignorePatterns { - fmt.Printf("Pattern: %s, Error: %s\n", key, value) - } - // Review this to properly handle logging - log.SetOutput(ioutil.Discard) - log.Println(err) - } linter := support.Linter{ChartDir: chartDir} - if rules.IsIgnored(chartDir, ignorePatterns) { - return linter - } rules.Chartfile(&linter) rules.ValuesWithOverrides(&linter, values) rules.TemplatesWithKubeVersion(&linter, values, namespace, kubeVersion) diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index 910602b7d..9a86f45b2 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -210,4 +210,4 @@ func loadChartFileForTypeCheck(filename string) (map[string]interface{}, error) y := make(map[string]interface{}) err = yaml.Unmarshal(b, &y) return y, err -} +} \ No newline at end of file diff --git a/pkg/lint/rules/ignore.go b/pkg/lint/rules/ignore.go index 70c1126e3..a17a2bffc 100644 --- a/pkg/lint/rules/ignore.go +++ b/pkg/lint/rules/ignore.go @@ -3,18 +3,27 @@ package rules import ( "bufio" "fmt" + "helm.sh/helm/v3/pkg/lint/support" + "log" "os" "path/filepath" "strings" ) +var Debug bool + func ParseIgnoreFile(filePath string) (map[string][]string, error) { patterns := make(map[string][]string) file, err := os.Open(filePath) if err != nil { return nil, err } - defer file.Close() + defer func() { + err := file.Close() + if err != nil { + log.Printf("Failed to close ignore file at [%s]: %v", filePath, err) + } + }() scanner := bufio.NewScanner(file) for scanner.Scan() { @@ -34,18 +43,105 @@ func ParseIgnoreFile(filePath string) (map[string][]string, error) { return patterns, scanner.Err() } -func IsIgnored(errorMessage string, patterns map[string][]string) bool { - for path, pathPatterns := range patterns { - cleanedPath := filepath.Clean(path) - if strings.Contains(errorMessage, cleanedPath) { - for _, pattern := range pathPatterns { - if strings.Contains(errorMessage, pattern) { - fmt.Printf("Ignoring error related to path: %s with pattern: %s\n", path, pattern) - return true +func FilterIgnoredErrors(errors []error, patterns map[string][]string) []error { + filteredErrors := make([]error, 0) + for _, err := range errors { + errText := err.Error() + errorFullPath := extractFullPathFromError(errText) + if len(errorFullPath) == 0 { + debug("Unable to find a path for error, guess we'll keep it: %s", errText) + filteredErrors = append(filteredErrors, err) + continue + } + ignore := false + debug("Extracted full path: %s\n", errorFullPath) + for ignorablePath, pathPatterns := range patterns { + cleanIgnorablePath := filepath.Clean(ignorablePath) + if strings.Contains(errorFullPath, cleanIgnorablePath) { + for _, pattern := range pathPatterns { + if strings.Contains(err.Error(), pattern) { + debug("Ignoring error: [%s] %s\n\n", errorFullPath, errText) + ignore = true + break + } } } + if ignore { + break + } + } + if !ignore { + debug("keeping unignored error: [%s]", errText) + filteredErrors = append(filteredErrors, err) } } - return false + + return filteredErrors } +func FilterIgnoredMessages(messages []support.Message, patterns map[string][]string) []support.Message { + filteredMessages := make([]support.Message, 0) + for _, msg := range messages { + errText := msg.Err.Error() + errorFullPath := extractFullPathFromError(errText) + if len(errorFullPath) == 0 { + debug("Unable to find a path for message, guess we'll keep it: %s", errText) + filteredMessages = append(filteredMessages, msg) + continue + } + ignore := false + debug("Extracted full path: %s\n", errorFullPath) + for ignorablePath, pathPatterns := range patterns { + cleanIgnorablePath := filepath.Clean(ignorablePath) + if strings.Contains(errorFullPath, cleanIgnorablePath) { + for _, pattern := range pathPatterns { + if strings.Contains(msg.Err.Error(), pattern) { + debug("Ignoring message: [%s] %s\n\n", errorFullPath, errText) + ignore = true + break + } + } + } + if ignore { + break + } + } + if !ignore { + debug("keeping unignored message: [%s]", errText) + filteredMessages = append(filteredMessages, msg) + } + } + + return filteredMessages +} + +// TODO: figure out & fix or remove +func extractFullPathFromError(errorString string) string { + parts := strings.Split(errorString, ":") + if len(parts) > 2 { + return strings.TrimSpace(parts[1]) + } + return "" +} + +// TODO: DELETE +var logger = log.New(os.Stderr, "[debug] ", log.Lshortfile) +func debug(format string, v ...interface{}) { + if Debug { + format = fmt.Sprintf("[debug] %s\n", format) + logger.Output(2, fmt.Sprintf(format, v...)) + } +} +// END TODO: DELETE + + +/* TODO HIP-0019 +- find ignore file path for a subchart +- add a chart or two for the end to end tests via testdata like in pkg/lint/lint_test.go +- review debug / output patterns across the helm project +Later/never +- XDG support +- helm config file support +- ignore file validation +- +*/ diff --git a/pkg/lint/rules/ignore_test.go b/pkg/lint/rules/ignore_test.go new file mode 100644 index 000000000..5967344ab --- /dev/null +++ b/pkg/lint/rules/ignore_test.go @@ -0,0 +1,71 @@ +package rules + +import ( + "fmt" + "github.com/stretchr/testify/assert" + "helm.sh/helm/v3/pkg/lint/support" + "testing" + "text/template" +) + +func TestFilterIgnoredMessages(t *testing.T) { + type args struct { + messages []support.Message + ignorePatterns map[string][]string + } + tests := []struct { + name string + args args + want []support.Message + }{ + { + name: "should filter ignored messages only", + args: args{ + messages: []support.Message{ + { + Severity: 3, + Path: "templates/", + Err: template.ExecError{ + Name: "certmanager-issuer/templates/rbac-config.yaml", + Err: fmt.Errorf(`template: certmanager-issuer/templates/rbac-config.yaml:1:67: executing "certmanager-issuer/templates/rbac-config.yaml" at <.Values.global.ingress>: nil pointer evaluating interface {}.ingress`), + }, + }, + { + Severity: 1, + Path: "values.yaml", + Err: fmt.Errorf("file does not exist"), + }, + { + Severity: 1, + Path: "Chart.yaml", + Err: fmt.Errorf("icon is recommended"), + }, + }, + ignorePatterns: map[string][]string{ + "certmanager-issuer/templates/rbac-config.yaml": { + "<.Values.global.ingress>", + }, + }, + }, + want: []support.Message{ + { + Severity: 1, + Path: "values.yaml", + Err: fmt.Errorf("file does not exist"), + }, + { + Severity: 1, + Path: "Chart.yaml", + Err: fmt.Errorf("icon is recommended"), + }, + + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := FilterIgnoredMessages(tt.args.messages, tt.args.ignorePatterns) + assert.Equalf(t, tt.want, got, "FilterIgnoredMessages(%v, %v)", tt.args.messages, tt.args.ignorePatterns) + }) + } +}