From 03ca4e892fd6bdee876a832177c76584de205f85 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Thu, 30 Jun 2016 19:07:56 -0700 Subject: [PATCH] feat(lint): support linting multiple charts --- cmd/helm/lint.go | 33 ++++++++--- pkg/lint/lint_test.go | 14 ++--- pkg/lint/rules/chartfile.go | 97 ++++++++++++++------------------ pkg/lint/rules/chartfile_test.go | 44 ++++++--------- pkg/lint/rules/template.go | 76 +++++++++++-------------- pkg/lint/rules/template_test.go | 16 +++--- pkg/lint/rules/values.go | 24 ++++---- pkg/lint/support/message.go | 36 ++++++------ pkg/lint/support/message_test.go | 30 +++++----- 9 files changed, 177 insertions(+), 193 deletions(-) diff --git a/cmd/helm/lint.go b/cmd/helm/lint.go index a14480bbc..660973183 100644 --- a/cmd/helm/lint.go +++ b/cmd/helm/lint.go @@ -51,19 +51,33 @@ func init() { RootCommand.AddCommand(lintCommand) } -var errLintNoChart = errors.New("no chart found for linting (missing Chart.yaml)") -var errLintFailed = errors.New("lint failed") +var errLintNoChart = errors.New("No chart found for linting (missing Chart.yaml)") +var errLintFailed = errors.New("Lint failed") func lintCmd(cmd *cobra.Command, args []string) error { - path := "." + paths := []string{"."} if len(args) > 0 { - path = args[0] + paths = args } - if err := lintChart(path); err != nil { - return err + var failures int + for _, path := range paths { + if err := lintChart(path); err != nil { + fmt.Println(err) + if err != errLintNoChart { + failures = failures + 1 + } + } + fmt.Println("") + } + + msg := fmt.Sprintf("%d chart(s) linted", len(paths)) + if failures > 0 { + return fmt.Errorf("%s. %d chart(s) failed.", msg, failures) } + fmt.Printf("%s. No failures.\n", msg) + return nil } @@ -91,9 +105,12 @@ func lintChart(path string) error { // Guard: Error out of this is not a chart. if _, err := os.Stat(filepath.Join(path, "Chart.yaml")); err != nil { + fmt.Println("==> Skipping", path) return errLintNoChart } + fmt.Println("==> Linting", path) + linter := lint.All(path) if len(linter.Messages) == 0 { @@ -101,8 +118,8 @@ func lintChart(path string) error { return nil } - for _, i := range linter.Messages { - fmt.Printf("%s\n", i) + for _, msg := range linter.Messages { + fmt.Println(msg) } if linter.HighestSeverity == support.ErrorSev { diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go index 8f683cc44..66541c179 100644 --- a/pkg/lint/lint_test.go +++ b/pkg/lint/lint_test.go @@ -39,18 +39,18 @@ func TestBadChart(t *testing.T) { var w, e, e2, e3 bool for _, msg := range m { if msg.Severity == support.WarningSev { - if strings.Contains(msg.Text, "Directory 'templates/' not found") { + if strings.Contains(msg.Err.Error(), "directory not found") { w = true } } if msg.Severity == support.ErrorSev { - if strings.Contains(msg.Text, "'version' 0.0.0 is less than or equal to 0") { + if strings.Contains(msg.Err.Error(), "version 0.0.0 is less than or equal to 0") { e = true } - if strings.Contains(msg.Text, "'name' is required") { + if strings.Contains(msg.Err.Error(), "name is required") { e2 = true } - if strings.Contains(msg.Text, "'name' and directory do not match") { + if strings.Contains(msg.Err.Error(), "directory name (badchartfile) and chart name () must be the same") { e3 = true } } @@ -65,7 +65,7 @@ func TestInvalidYaml(t *testing.T) { if len(m) != 1 { t.Errorf("All didn't fail with expected errors, got %#v", m) } - if !strings.Contains(m[0].Text, "deliberateSyntaxError") { + if !strings.Contains(m[0].Err.Error(), "deliberateSyntaxError") { t.Errorf("All didn't have the error for deliberateSyntaxError") } } @@ -75,8 +75,8 @@ func TestBadValues(t *testing.T) { if len(m) != 1 { t.Errorf("All didn't fail with expected errors, got %#v", m) } - if !strings.Contains(m[0].Text, "cannot unmarshal") { - t.Errorf("All didn't have the error for invalid key format: %s", m[0].Text) + if !strings.Contains(m[0].Err.Error(), "cannot unmarshal") { + t.Errorf("All didn't have the error for invalid key format: %s", m[0].Err) } } diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index 1cfc88512..b8b0e2d10 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -17,12 +17,14 @@ limitations under the License. package rules import ( + "errors" "fmt" "os" "path/filepath" "strings" "github.com/Masterminds/semver" + "github.com/asaskevich/govalidator" "k8s.io/helm/pkg/chartutil" "k8s.io/helm/pkg/lint/support" @@ -31,95 +33,83 @@ import ( // Chartfile runs a set of linter rules related to Chart.yaml file func Chartfile(linter *support.Linter) { - chartPath := filepath.Join(linter.ChartDir, "Chart.yaml") + chartFileName := "Chart.yaml" + chartPath := filepath.Join(linter.ChartDir, chartFileName) - linter.RunLinterRule(support.ErrorSev, validateChartYamlFileExistence(chartPath)) - linter.RunLinterRule(support.ErrorSev, validateChartYamlNotDirectory(chartPath)) + linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartYamlNotDirectory(chartPath)) chartFile, err := chartutil.LoadChartfile(chartPath) - validChartFile := linter.RunLinterRule(support.ErrorSev, validateChartYamlFormat(err)) + validChartFile := linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartYamlFormat(err)) // Guard clause. Following linter rules require a parseable ChartFile if !validChartFile { return } - linter.RunLinterRule(support.ErrorSev, validateChartName(chartFile)) - linter.RunLinterRule(support.ErrorSev, validateChartNameDirMatch(linter.ChartDir, chartFile)) + linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartName(chartFile)) + linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartNameDirMatch(linter.ChartDir, chartFile)) // Chart metadata - linter.RunLinterRule(support.ErrorSev, validateChartVersion(chartFile)) - linter.RunLinterRule(support.ErrorSev, validateChartEngine(chartFile)) - linter.RunLinterRule(support.ErrorSev, validateChartMaintainer(chartFile)) - linter.RunLinterRule(support.ErrorSev, validateChartSources(chartFile)) - linter.RunLinterRule(support.ErrorSev, validateChartHome(chartFile)) -} - -// Auxiliar validation methods -func validateChartYamlFileExistence(chartPath string) (lintError support.LintError) { - _, err := os.Stat(chartPath) - if err != nil { - lintError = fmt.Errorf("Chart.yaml file does not exist") - } - return + linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartVersion(chartFile)) + linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartEngine(chartFile)) + linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartMaintainer(chartFile)) + linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartSources(chartFile)) } -func validateChartYamlNotDirectory(chartPath string) (lintError support.LintError) { +func validateChartYamlNotDirectory(chartPath string) error { fi, err := os.Stat(chartPath) if err == nil && fi.IsDir() { - lintError = fmt.Errorf("Chart.yaml is a directory") + return errors.New("should be a file, not a directory") } - return + return nil } -func validateChartYamlFormat(chartFileError error) (lintError support.LintError) { +func validateChartYamlFormat(chartFileError error) error { if chartFileError != nil { - lintError = fmt.Errorf("Chart.yaml is malformed: %s", chartFileError.Error()) + return fmt.Errorf("unable to parse YAML\n\t%s", chartFileError.Error()) } - return + return nil } -func validateChartName(cf *chart.Metadata) (lintError support.LintError) { +func validateChartName(cf *chart.Metadata) error { if cf.Name == "" { - lintError = fmt.Errorf("Chart.yaml: 'name' is required") + return errors.New("name is required") } - return + return nil } -func validateChartNameDirMatch(chartDir string, cf *chart.Metadata) (lintError support.LintError) { +func validateChartNameDirMatch(chartDir string, cf *chart.Metadata) error { if cf.Name != filepath.Base(chartDir) { - lintError = fmt.Errorf("Chart.yaml: 'name' and directory do not match") + return fmt.Errorf("directory name (%s) and chart name (%s) must be the same", filepath.Base(chartDir), cf.Name) } - return + return nil } -func validateChartVersion(cf *chart.Metadata) (lintError support.LintError) { +func validateChartVersion(cf *chart.Metadata) error { if cf.Version == "" { - lintError = fmt.Errorf("Chart.yaml: 'version' value is required") - return + return errors.New("version is required") } version, err := semver.NewVersion(cf.Version) if err != nil { - lintError = fmt.Errorf("Chart.yaml: version '%s' is not a valid SemVer", cf.Version) - return + return fmt.Errorf("version '%s' is not a valid SemVer", cf.Version) } c, err := semver.NewConstraint("> 0") valid, msg := c.Validate(version) if !valid && len(msg) > 0 { - lintError = fmt.Errorf("Chart.yaml: 'version' %v", msg[0]) + return fmt.Errorf("version %v", msg[0]) } - return + return nil } -func validateChartEngine(cf *chart.Metadata) (lintError support.LintError) { +func validateChartEngine(cf *chart.Metadata) error { if cf.Engine == "" { - return + return nil } keys := make([]string, 0, len(chart.Metadata_Engine_value)) @@ -131,39 +121,38 @@ func validateChartEngine(cf *chart.Metadata) (lintError support.LintError) { } if str == cf.Engine { - return + return nil } keys = append(keys, str) } - lintError = fmt.Errorf("Chart.yaml: engine '%v' not valid. Valid options are %v", cf.Engine, keys) - return + return fmt.Errorf("engine '%v' not valid. Valid options are %v", cf.Engine, keys) } -func validateChartMaintainer(cf *chart.Metadata) (lintError support.LintError) { +func validateChartMaintainer(cf *chart.Metadata) error { for _, maintainer := range cf.Maintainers { if maintainer.Name == "" { - lintError = fmt.Errorf("Chart.yaml: maintainer requires a name") + return errors.New("each maintainer requires a name") } else if maintainer.Email != "" && !govalidator.IsEmail(maintainer.Email) { - lintError = fmt.Errorf("Chart.yaml: maintainer invalid email") + return fmt.Errorf("invalid email '%s' for maintainer '%s'", maintainer.Email, maintainer.Name) } } - return + return nil } -func validateChartSources(cf *chart.Metadata) (lintError support.LintError) { +func validateChartSources(cf *chart.Metadata) error { for _, source := range cf.Sources { if source == "" || !govalidator.IsRequestURL(source) { - lintError = fmt.Errorf("Chart.yaml: 'source' invalid URL %s", source) + return fmt.Errorf("invalid source URL '%s'", source) } } - return + return nil } -func validateChartHome(cf *chart.Metadata) (lintError support.LintError) { +func validateChartHome(cf *chart.Metadata) error { if cf.Home != "" && !govalidator.IsRequestURL(cf.Home) { - lintError = fmt.Errorf("Chart.yaml: 'home' invalid URL %s", cf.Home) + return fmt.Errorf("invalid home URL '%s'", cf.Home) } - return + return nil } diff --git a/pkg/lint/rules/chartfile_test.go b/pkg/lint/rules/chartfile_test.go index 02b5f2c12..e1295a7cf 100644 --- a/pkg/lint/rules/chartfile_test.go +++ b/pkg/lint/rules/chartfile_test.go @@ -43,18 +43,6 @@ var badChart, chatLoadRrr = chartutil.LoadChartfile(badChartFilePath) var goodChart, _ = chartutil.LoadChartfile(goodChartFilePath) // Validation functions Test -func TestValidateChartYamlFileExistence(t *testing.T) { - err := validateChartYamlFileExistence(nonExistingChartFilePath) - if err == nil { - t.Errorf("validateChartYamlFileExistence to return a linter error, got no error") - } - - err = validateChartYamlFileExistence(badChartFilePath) - if err != nil { - t.Errorf("validateChartYamlFileExistence to return no error, got a linter error") - } -} - func TestValidateChartYamlNotDirectory(t *testing.T) { _ = os.Mkdir(nonExistingChartFilePath, os.ModePerm) defer os.Remove(nonExistingChartFilePath) @@ -107,10 +95,10 @@ func TestValidateChartVersion(t *testing.T) { Version string ErrorMsg string }{ - {"", "'version' value is required"}, + {"", "version is required"}, {"0", "0 is less than or equal to 0"}, - {"waps", "is not a valid SemVer"}, - {"-3", "is not a valid SemVer"}, + {"waps", "'waps' is not a valid SemVer"}, + {"-3", "'-3' is not a valid SemVer"}, } var successTest = []string{"0.0.1", "0.0.1+build", "0.0.1-beta"} @@ -156,9 +144,9 @@ func TestValidateChartMaintainer(t *testing.T) { Email string ErrorMsg string }{ - {"", "", "maintainer requires a name"}, - {"", "test@test.com", "maintainer requires a name"}, - {"John Snow", "wrongFormatEmail.com", "maintainer invalid email"}, + {"", "", "each maintainer requires a name"}, + {"", "test@test.com", "each maintainer requires a name"}, + {"John Snow", "wrongFormatEmail.com", "invalid email"}, } var successTest = []struct { @@ -192,8 +180,8 @@ func TestValidateChartSources(t *testing.T) { for _, test := range failTest { badChart.Sources = []string{test} err := validateChartSources(badChart) - if err == nil || !strings.Contains(err.Error(), "invalid URL") { - t.Errorf("validateChartSources(%s) to return \"invalid URL\", got no error", test) + if err == nil || !strings.Contains(err.Error(), "invalid source URL") { + t.Errorf("validateChartSources(%s) to return \"invalid source URL\", got no error", test) } } @@ -213,8 +201,8 @@ func TestValidateChartHome(t *testing.T) { for _, test := range failTest { badChart.Home = test err := validateChartHome(badChart) - if err == nil || !strings.Contains(err.Error(), "invalid URL") { - t.Errorf("validateChartHome(%s) to return \"invalid URL\", got no error", test) + if err == nil || !strings.Contains(err.Error(), "invalid home URL") { + t.Errorf("validateChartHome(%s) to return \"invalid home URL\", got no error", test) } } @@ -236,15 +224,15 @@ func TestChartfile(t *testing.T) { t.Errorf("Expected 3 errors, got %d", len(msgs)) } - if !strings.Contains(msgs[0].Text, "'name' is required") { - t.Errorf("Unexpected message 0: %s", msgs[0].Text) + if !strings.Contains(msgs[0].Err.Error(), "name is required") { + t.Errorf("Unexpected message 0: %s", msgs[0].Err) } - if !strings.Contains(msgs[1].Text, "'name' and directory do not match") { - t.Errorf("Unexpected message 1: %s", msgs[1].Text) + if !strings.Contains(msgs[1].Err.Error(), "directory name (badchartfile) and chart name () must be the same") { + t.Errorf("Unexpected message 1: %s", msgs[1].Err) } - if !strings.Contains(msgs[2].Text, "'version' 0.0.0 is less than or equal to 0") { - t.Errorf("Unexpected message 2: %s", msgs[2].Text) + if !strings.Contains(msgs[2].Err.Error(), "version 0.0.0 is less than or equal to 0") { + t.Errorf("Unexpected message 2: %s", msgs[2].Err) } } diff --git a/pkg/lint/rules/template.go b/pkg/lint/rules/template.go index 448a7567e..ace4acf8c 100644 --- a/pkg/lint/rules/template.go +++ b/pkg/lint/rules/template.go @@ -18,6 +18,7 @@ package rules import ( "bytes" + "errors" "fmt" "os" "path/filepath" @@ -35,9 +36,10 @@ import ( // Templates lints the templates in the Linter. func Templates(linter *support.Linter) { - templatesPath := filepath.Join(linter.ChartDir, "templates") + path := "templates/" + templatesPath := filepath.Join(linter.ChartDir, path) - templatesDirExist := linter.RunLinterRule(support.WarningSev, validateTemplatesDir(templatesPath)) + templatesDirExist := linter.RunLinterRule(support.WarningSev, path, validateTemplatesDir(templatesPath)) // Templates directory is optional for now if !templatesDirExist { @@ -47,7 +49,7 @@ func Templates(linter *support.Linter) { // Load chart and parse templates, based on tiller/release_server chart, err := chartutil.Load(linter.ChartDir) - chartLoaded := linter.RunLinterRule(support.ErrorSev, validateNoError(err)) + chartLoaded := linter.RunLinterRule(support.ErrorSev, path, err) if !chartLoaded { return @@ -63,7 +65,7 @@ func Templates(linter *support.Linter) { } renderedContentMap, err := engine.New().Render(chart, valuesToRender) - renderOk := linter.RunLinterRule(support.ErrorSev, validateNoError(err)) + renderOk := linter.RunLinterRule(support.ErrorSev, path, err) if !renderOk { return @@ -78,8 +80,9 @@ func Templates(linter *support.Linter) { */ for _, template := range chart.Templates { fileName, preExecutedTemplate := template.Name, template.Data + path = fileName - linter.RunLinterRule(support.ErrorSev, validateAllowedExtension(fileName)) + linter.RunLinterRule(support.ErrorSev, path, validateAllowedExtension(fileName)) // We only apply the following lint rules to yaml files if filepath.Ext(fileName) != ".yaml" { @@ -87,9 +90,9 @@ func Templates(linter *support.Linter) { } // Check that all the templates have a matching value - linter.RunLinterRule(support.WarningSev, validateNonMissingValues(fileName, templatesPath, valuesToRender, preExecutedTemplate)) + linter.RunLinterRule(support.WarningSev, path, validateNoMissingValues(templatesPath, valuesToRender, preExecutedTemplate)) - linter.RunLinterRule(support.WarningSev, validateQuotes(fileName, string(preExecutedTemplate))) + linter.RunLinterRule(support.WarningSev, path, validateQuotes(string(preExecutedTemplate))) renderedContent := renderedContentMap[fileName] var yamlStruct K8sYamlStruct @@ -97,30 +100,30 @@ func Templates(linter *support.Linter) { // key will be raised as well err := yaml.Unmarshal([]byte(renderedContent), &yamlStruct) - validYaml := linter.RunLinterRule(support.ErrorSev, validateYamlContent(fileName, err)) + validYaml := linter.RunLinterRule(support.ErrorSev, path, validateYamlContent(err)) if !validYaml { continue } - linter.RunLinterRule(support.ErrorSev, validateNoNamespace(fileName, yamlStruct)) + linter.RunLinterRule(support.ErrorSev, path, validateNoNamespace(yamlStruct)) } } // Validation functions -func validateTemplatesDir(templatesPath string) (lintError support.LintError) { +func validateTemplatesDir(templatesPath string) error { if fi, err := os.Stat(templatesPath); err != nil { - lintError = fmt.Errorf("Directory 'templates/' not found") + return errors.New("directory not found") } else if err == nil && !fi.IsDir() { - lintError = fmt.Errorf("'templates' is not a directory") + return errors.New("not a directory") } - return + return nil } // Validates that go template tags include the quote pipelined function // i.e {{ .Foo.bar }} -> {{ .Foo.bar | quote }} // {{ .Foo.bar }}-{{ .Foo.baz }} -> "{{ .Foo.bar }}-{{ .Foo.baz }}" -func validateQuotes(templateName string, templateContent string) (lintError support.LintError) { +func validateQuotes(templateContent string) error { // {{ .Foo.bar }} r, _ := regexp.Compile(`(?m)(:|-)\s+{{[\w|\.|\s|\']+}}\s*$`) functions := r.FindAllString(templateContent, -1) @@ -128,8 +131,7 @@ func validateQuotes(templateName string, templateContent string) (lintError supp for _, str := range functions { if match, _ := regexp.MatchString("quote", str); !match { result := strings.Replace(str, "}}", " | quote }}", -1) - lintError = fmt.Errorf("templates: \"%s\". Wrap your substitution functions in quotes or use the sprig \"quote\" function: %s -> %s", templateName, str, result) - return + return fmt.Errorf("wrap substitution functions in quotes or use the sprig \"quote\" function: %s -> %s", str, result) } } @@ -139,29 +141,27 @@ func validateQuotes(templateName string, templateContent string) (lintError supp for _, str := range functions { result := strings.Replace(str, str, fmt.Sprintf("\"%s\"", str), -1) - lintError = fmt.Errorf("templates: \"%s\". Wrap your substitution functions in quotes: %s -> %s", templateName, str, result) - return + return fmt.Errorf("wrap substitution functions in quotes: %s -> %s", str, result) } - return + return nil } -func validateAllowedExtension(fileName string) (lintError support.LintError) { +func validateAllowedExtension(fileName string) error { ext := filepath.Ext(fileName) validExtensions := []string{".yaml", ".tpl"} for _, b := range validExtensions { if b == ext { - return + return nil } } - lintError = fmt.Errorf("templates: \"%s\" needs to use .yaml or .tpl extensions", fileName) - return + return fmt.Errorf("file extension '%s' not valid. Valid extensions are .yaml or .tpl", ext) } -// validateNonMissingValues checks that all the {{}} functions returns a non empty value ( or "") +// validateNoMissingValues checks that all the {{}} functions returns a non empty value ( or "") // and return an error otherwise. -func validateNonMissingValues(fileName string, templatesPath string, chartValues chartutil.Values, templateContent []byte) (lintError support.LintError) { +func validateNoMissingValues(templatesPath string, chartValues chartutil.Values, templateContent []byte) error { // 1 - Load Main and associated templates // Main template that we will parse dynamically tmpl := template.New("tpl").Funcs(sprig.TxtFuncMap()) @@ -188,8 +188,7 @@ func validateNonMissingValues(fileName string, templatesPath string, chartValues for _, str := range functions { newtmpl, err := tmpl.Parse(str) if err != nil { - lintError = fmt.Errorf("templates: %s", err.Error()) - return + return err } err = newtmpl.ExecuteTemplate(&buf, "tpl", chartValues) @@ -207,30 +206,23 @@ func validateNonMissingValues(fileName string, templatesPath string, chartValues } if len(emptyValues) > 0 { - lintError = fmt.Errorf("templates: %s: The following functions are not returning any value %v", fileName, emptyValues) - } - return -} - -func validateNoError(readError error) (lintError support.LintError) { - if readError != nil { - lintError = fmt.Errorf("templates: %s", readError.Error()) + return fmt.Errorf("these substitution functions are returning no value: %v", emptyValues) } - return + return nil } -func validateYamlContent(filePath string, err error) (lintError support.LintError) { +func validateYamlContent(err error) error { if err != nil { - lintError = fmt.Errorf("templates: \"%s\". Wrong YAML content", filePath) + return fmt.Errorf("unable to parse YAML\n\t%s", err) } - return + return nil } -func validateNoNamespace(filePath string, yamlStruct K8sYamlStruct) (lintError support.LintError) { +func validateNoNamespace(yamlStruct K8sYamlStruct) error { if yamlStruct.Metadata.Namespace != "" { - lintError = fmt.Errorf("templates: \"%s\". namespace option is currently NOT supported", filePath) + return errors.New("namespace option is currently NOT supported") } - return + return nil } // K8sYamlStruct stubs a Kubernetes YAML file. diff --git a/pkg/lint/rules/template_test.go b/pkg/lint/rules/template_test.go index ecce95238..dbd792774 100644 --- a/pkg/lint/rules/template_test.go +++ b/pkg/lint/rules/template_test.go @@ -31,8 +31,8 @@ func TestValidateAllowedExtension(t *testing.T) { var failTest = []string{"/foo", "/test.yml", "/test.toml", "test.yml"} for _, test := range failTest { err := validateAllowedExtension(test) - if err == nil || !strings.Contains(err.Error(), "needs to use .yaml or .tpl extension") { - t.Errorf("validateAllowedExtension('%s') to return \"needs to use .yaml or .tpl extension\", got no error", test) + if err == nil || !strings.Contains(err.Error(), "Valid extensions are .yaml or .tpl") { + t.Errorf("validateAllowedExtension('%s') to return \"Valid extensions are .yaml or .tpl\", got no error", test) } } var successTest = []string{"/foo.yaml", "foo.yaml", "foo.tpl", "/foo/bar/baz.yaml"} @@ -49,7 +49,7 @@ func TestValidateQuotes(t *testing.T) { var failTest = []string{"foo: {{.Release.Service }}", "foo: {{.Release.Service }}", "- {{.Release.Service }}", "foo: {{default 'Never' .restart_policy}}", "- {{.Release.Service }} "} for _, test := range failTest { - err := validateQuotes("testTemplate.yaml", test) + err := validateQuotes(test) if err == nil || !strings.Contains(err.Error(), "use the sprig \"quote\" function") { t.Errorf("validateQuotes('%s') to return \"use the sprig \"quote\" function:\", got no error.", test) } @@ -58,7 +58,7 @@ func TestValidateQuotes(t *testing.T) { var successTest = []string{"foo: {{.Release.Service | quote }}", "foo: {{.Release.Service | quote }}", "- {{.Release.Service | quote }}", "foo: {{default 'Never' .restart_policy | quote }}", "foo: \"{{ .Release.Service }}\"", "foo: \"{{ .Release.Service }} {{ .Foo.Bar }}\"", "foo: \"{{ default 'Never' .Release.Service }} {{ .Foo.Bar }}\"", "foo: {{.Release.Service | squote }}"} for _, test := range successTest { - err := validateQuotes("testTemplate.yaml", test) + err := validateQuotes(test) if err != nil { t.Errorf("validateQuotes('%s') to return not error and got \"%s\"", test, err.Error()) } @@ -68,9 +68,9 @@ func TestValidateQuotes(t *testing.T) { failTest = []string{"foo: {{.Release.Service }}-{{ .Release.Bar }}", "foo: {{.Release.Service }} {{ .Release.Bar }}", "- {{.Release.Service }}-{{ .Release.Bar }}", "- {{.Release.Service }}-{{ .Release.Bar }} {{ .Release.Baz }}", "foo: {{.Release.Service | default }}-{{ .Release.Bar }}"} for _, test := range failTest { - err := validateQuotes("testTemplate.yaml", test) - if err == nil || !strings.Contains(err.Error(), "Wrap your substitution functions in quotes") { - t.Errorf("validateQuotes('%s') to return \"Wrap your substitution functions in quotes\", got no error", test) + err := validateQuotes(test) + if err == nil || !strings.Contains(err.Error(), "wrap substitution functions in quotes") { + t.Errorf("validateQuotes('%s') to return \"wrap substitution functions in quotes\", got no error", test) } } @@ -85,7 +85,7 @@ func TestTemplateParsing(t *testing.T) { t.Fatalf("Expected one error, got %d, %v", len(res), res) } - if !strings.Contains(res[0].Text, "deliberateSyntaxError") { + if !strings.Contains(res[0].Err.Error(), "deliberateSyntaxError") { t.Errorf("Unexpected error: %s", res[0]) } } diff --git a/pkg/lint/rules/values.go b/pkg/lint/rules/values.go index 6d1d7af7f..9b97598f0 100644 --- a/pkg/lint/rules/values.go +++ b/pkg/lint/rules/values.go @@ -18,36 +18,38 @@ package rules import ( "fmt" - "k8s.io/helm/pkg/chartutil" - "k8s.io/helm/pkg/lint/support" "os" "path/filepath" + + "k8s.io/helm/pkg/chartutil" + "k8s.io/helm/pkg/lint/support" ) // Values lints a chart's values.yaml file. func Values(linter *support.Linter) { - vf := filepath.Join(linter.ChartDir, "values.yaml") - fileExists := linter.RunLinterRule(support.InfoSev, validateValuesFileExistence(linter, vf)) + file := "values.yaml" + vf := filepath.Join(linter.ChartDir, file) + fileExists := linter.RunLinterRule(support.InfoSev, file, validateValuesFileExistence(linter, vf)) if !fileExists { return } - linter.RunLinterRule(support.ErrorSev, validateValuesFile(linter, vf)) + linter.RunLinterRule(support.ErrorSev, file, validateValuesFile(linter, vf)) } -func validateValuesFileExistence(linter *support.Linter, valuesPath string) (lintError support.LintError) { +func validateValuesFileExistence(linter *support.Linter, valuesPath string) error { _, err := os.Stat(valuesPath) if err != nil { - lintError = fmt.Errorf("values.yaml file does not exists") + return fmt.Errorf("file does not exist") } - return + return nil } -func validateValuesFile(linter *support.Linter, valuesPath string) (lintError support.LintError) { +func validateValuesFile(linter *support.Linter, valuesPath string) error { _, err := chartutil.ReadValuesFile(valuesPath) if err != nil { - lintError = fmt.Errorf("values.yaml is malformed: %s", err.Error()) + return fmt.Errorf("unable to parse YAML\n\t%s", err) } - return + return nil } diff --git a/pkg/lint/support/message.go b/pkg/lint/support/message.go index 8615bc15e..05de920e6 100644 --- a/pkg/lint/support/message.go +++ b/pkg/lint/support/message.go @@ -33,14 +33,6 @@ const ( // sev matches the *Sev states. var sev = []string{"UNKNOWN", "INFO", "WARNING", "ERROR"} -// Message is a linting output message -type Message struct { - // Severity is one of the *Sev constants - Severity int - // Text contains the message text - Text string -} - // Linter encapsulates a linting run of a particular chart. type Linter struct { Messages []Message @@ -49,31 +41,35 @@ type Linter struct { ChartDir string } -// LintError describes an error encountered while linting. -type LintError interface { - error +// Message describes an error encountered while linting. +type Message struct { + // Severity is one of the *Sev constants + Severity int + Path string + Err error +} + +func (m Message) Error() string { + return fmt.Sprintf("[%s] %s: %s", sev[m.Severity], m.Path, m.Err.Error()) } -// String prints a string representation of this Message. -// -// Implements fmt.Stringer. -func (m Message) String() string { - return fmt.Sprintf("[%s] %s", sev[m.Severity], m.Text) +func NewMessage(severity int, path string, err error) Message { + return Message{Severity: severity, Path: path, Err: err} } // RunLinterRule returns true if the validation passed -func (l *Linter) RunLinterRule(severity int, lintError LintError) bool { +func (l *Linter) RunLinterRule(severity int, path string, err error) bool { // severity is out of bound if severity < 0 || severity >= len(sev) { return false } - if lintError != nil { - l.Messages = append(l.Messages, Message{Text: lintError.Error(), Severity: severity}) + if err != nil { + l.Messages = append(l.Messages, NewMessage(severity, path, err)) if severity > l.HighestSeverity { l.HighestSeverity = severity } } - return lintError == nil + return err == nil } diff --git a/pkg/lint/support/message_test.go b/pkg/lint/support/message_test.go index fcaa63c4f..b2a4d4d98 100644 --- a/pkg/lint/support/message_test.go +++ b/pkg/lint/support/message_test.go @@ -17,12 +17,12 @@ limitations under the License. package support import ( - "fmt" + "errors" "testing" ) var linter = Linter{} -var lintError LintError = fmt.Errorf("Foobar") +var lintError = errors.New("lint failed") func TestRunLinterRule(t *testing.T) { var tests = []struct { @@ -46,34 +46,34 @@ func TestRunLinterRule(t *testing.T) { } for _, test := range tests { - isValid := linter.RunLinterRule(test.Severity, test.LintError) + isValid := linter.RunLinterRule(test.Severity, "chart", test.LintError) if len(linter.Messages) != test.ExpectedMessages { - t.Errorf("RunLinterRule(%d, %v), linter.Messages should now have %d message, we got %d", test.Severity, test.LintError, test.ExpectedMessages, len(linter.Messages)) + t.Errorf("RunLinterRule(%d, \"chart\", %v), linter.Messages should now have %d message, we got %d", test.Severity, test.LintError, test.ExpectedMessages, len(linter.Messages)) } if linter.HighestSeverity != test.ExpectedHighestSeverity { - t.Errorf("RunLinterRule(%d, %v), linter.HighestSeverity should be %d, we got %d", test.Severity, test.LintError, test.ExpectedHighestSeverity, linter.HighestSeverity) + t.Errorf("RunLinterRule(%d, \"chart\", %v), linter.HighestSeverity should be %d, we got %d", test.Severity, test.LintError, test.ExpectedHighestSeverity, linter.HighestSeverity) } if isValid != test.ExpectedReturn { - t.Errorf("RunLinterRule(%d, %v), should have returned %t but returned %t", test.Severity, test.LintError, test.ExpectedReturn, isValid) + t.Errorf("RunLinterRule(%d, \"chart\", %v), should have returned %t but returned %t", test.Severity, test.LintError, test.ExpectedReturn, isValid) } } } func TestMessage(t *testing.T) { - m := Message{ErrorSev, "Foo"} - if m.String() != "[ERROR] Foo" { - t.Errorf("Unexpected output: %s", m.String()) + m := Message{ErrorSev, "Chart.yaml", errors.New("Foo")} + if m.Error() != "[ERROR] Chart.yaml: Foo" { + t.Errorf("Unexpected output: %s", m.Error()) } - m = Message{WarningSev, "Bar"} - if m.String() != "[WARNING] Bar" { - t.Errorf("Unexpected output: %s", m.String()) + m = Message{WarningSev, "templates/", errors.New("Bar")} + if m.Error() != "[WARNING] templates/: Bar" { + t.Errorf("Unexpected output: %s", m.Error()) } - m = Message{InfoSev, "FooBar"} - if m.String() != "[INFO] FooBar" { - t.Errorf("Unexpected output: %s", m.String()) + m = Message{InfoSev, "templates/rc.yaml", errors.New("FooBar")} + if m.Error() != "[INFO] templates/rc.yaml: FooBar" { + t.Errorf("Unexpected output: %s", m.Error()) } }