From 03d27779d39cb982d85d6f83f72e50ed27ea345d Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Mon, 20 Jun 2016 19:17:36 -0700 Subject: [PATCH] Templates Lint rules Template rules Adding chart errors Added function that checks the existence of all the values in the templates Adding chartfile unit tests Testing runLinterRule Fixing out of range Fixing out of range Improving quote detector --- .../examples/alpine/templates/alpine-pod.yaml | 10 +- pkg/chartutil/load.go | 1 + pkg/lint/rules/chartfile.go | 42 ++-- pkg/lint/rules/chartfile_test.go | 205 +++++++++++++++++- pkg/lint/rules/template.go | 196 ++++++++++++++--- pkg/lint/rules/template_test.go | 32 +++ pkg/lint/support/message.go | 7 +- pkg/lint/support/message_test.go | 33 ++- 8 files changed, 461 insertions(+), 65 deletions(-) diff --git a/docs/examples/alpine/templates/alpine-pod.yaml b/docs/examples/alpine/templates/alpine-pod.yaml index d0eca151c..2294cfd03 100644 --- a/docs/examples/alpine/templates/alpine-pod.yaml +++ b/docs/examples/alpine/templates/alpine-pod.yaml @@ -1,19 +1,19 @@ apiVersion: v1 kind: Pod metadata: - name: {{.Release.Name}}-{{.Chart.Name}} + name: "{{.Release.Name}}-{{.Chart.Name}}" labels: # The "heritage" label is used to track which tool deployed a given chart. # It is useful for admins who want to see what releases a particular tool # is responsible for. - heritage: {{.Release.Service}} + heritage: {{.Release.Service | quote }} # The "release" convention makes it easy to tie a release to all of the # Kubernetes resources that were created as part of that release. - release: {{.Release.Name}} + release: {{.Release.Name | quote }} # This makes it easy to audit chart usage. - chart: {{.Chart.Name}}-{{.Chart.Version}} + chart: "{{.Chart.Name}}-{{.Chart.Version}}" annotations: - "helm.sh/created": "{{.Release.Time.Seconds}}" + "helm.sh/created": {{.Release.Time.Seconds | quote }} spec: # This shows how to use a simple value. This will look for a passed-in value # called restartPolicy. If it is not found, it will use the default value. diff --git a/pkg/chartutil/load.go b/pkg/chartutil/load.go index 1ae07ce0a..296b11231 100644 --- a/pkg/chartutil/load.go +++ b/pkg/chartutil/load.go @@ -211,6 +211,7 @@ func LoadDir(dir string) (*chart.Chart, error) { files := []*afile{} topdir += string(filepath.Separator) + err = filepath.Walk(topdir, func(name string, fi os.FileInfo, err error) error { n := strings.TrimPrefix(name, topdir) if err != nil { diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index 907eed77d..b0333fc27 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -33,30 +33,30 @@ import ( func Chartfile(linter *support.Linter) { chartPath := filepath.Join(linter.ChartDir, "Chart.yaml") - linter.RunLinterRule(support.ErrorSev, validateChartYamlFileExistence(linter, chartPath)) - linter.RunLinterRule(support.ErrorSev, validateChartYamlNotDirectory(linter, chartPath)) + linter.RunLinterRule(support.ErrorSev, validateChartYamlFileExistence(chartPath)) + linter.RunLinterRule(support.ErrorSev, validateChartYamlNotDirectory(chartPath)) chartFile, err := chartutil.LoadChartfile(chartPath) - validChartFile := linter.RunLinterRule(support.ErrorSev, validateChartYamlFormat(linter, err)) + validChartFile := linter.RunLinterRule(support.ErrorSev, validateChartYamlFormat(err)) // Guard clause. Following linter rules require a parseable ChartFile if !validChartFile { return } - linter.RunLinterRule(support.ErrorSev, validateChartName(linter, chartFile)) - linter.RunLinterRule(support.ErrorSev, validateChartNameDirMatch(linter, chartFile)) + linter.RunLinterRule(support.ErrorSev, validateChartName(chartFile)) + linter.RunLinterRule(support.ErrorSev, validateChartNameDirMatch(linter.ChartDir, chartFile)) // Chart metadata - linter.RunLinterRule(support.ErrorSev, validateChartVersion(linter, chartFile)) - linter.RunLinterRule(support.ErrorSev, validateChartEngine(linter, chartFile)) - linter.RunLinterRule(support.ErrorSev, validateChartMaintainer(linter, chartFile)) - linter.RunLinterRule(support.ErrorSev, validateChartSources(linter, chartFile)) - linter.RunLinterRule(support.ErrorSev, validateChartHome(linter, chartFile)) + 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(linter *support.Linter, chartPath string) (lintError support.LintError) { +func validateChartYamlFileExistence(chartPath string) (lintError support.LintError) { _, err := os.Stat(chartPath) if err != nil { lintError = fmt.Errorf("Chart.yaml file does not exists") @@ -64,7 +64,7 @@ func validateChartYamlFileExistence(linter *support.Linter, chartPath string) (l return } -func validateChartYamlNotDirectory(linter *support.Linter, chartPath string) (lintError support.LintError) { +func validateChartYamlNotDirectory(chartPath string) (lintError support.LintError) { fi, err := os.Stat(chartPath) if err == nil && fi.IsDir() { @@ -73,28 +73,28 @@ func validateChartYamlNotDirectory(linter *support.Linter, chartPath string) (li return } -func validateChartYamlFormat(linter *support.Linter, chartFileError error) (lintError support.LintError) { +func validateChartYamlFormat(chartFileError error) (lintError support.LintError) { if chartFileError != nil { lintError = fmt.Errorf("Chart.yaml is malformed: %s", chartFileError.Error()) } return } -func validateChartName(linter *support.Linter, cf *chart.Metadata) (lintError support.LintError) { +func validateChartName(cf *chart.Metadata) (lintError support.LintError) { if cf.Name == "" { lintError = fmt.Errorf("Chart.yaml: 'name' is required") } return } -func validateChartNameDirMatch(linter *support.Linter, cf *chart.Metadata) (lintError support.LintError) { - if cf.Name != filepath.Base(linter.ChartDir) { +func validateChartNameDirMatch(chartDir string, cf *chart.Metadata) (lintError support.LintError) { + if cf.Name != filepath.Base(chartDir) { lintError = fmt.Errorf("Chart.yaml: 'name' and directory do not match") } return } -func validateChartVersion(linter *support.Linter, cf *chart.Metadata) (lintError support.LintError) { +func validateChartVersion(cf *chart.Metadata) (lintError support.LintError) { if cf.Version == "" { lintError = fmt.Errorf("Chart.yaml: 'version' value is required") return @@ -117,7 +117,7 @@ func validateChartVersion(linter *support.Linter, cf *chart.Metadata) (lintError return } -func validateChartEngine(linter *support.Linter, cf *chart.Metadata) (lintError support.LintError) { +func validateChartEngine(cf *chart.Metadata) (lintError support.LintError) { if cf.Engine == "" { return } @@ -141,7 +141,7 @@ func validateChartEngine(linter *support.Linter, cf *chart.Metadata) (lintError return } -func validateChartMaintainer(linter *support.Linter, cf *chart.Metadata) (lintError support.LintError) { +func validateChartMaintainer(cf *chart.Metadata) (lintError support.LintError) { for _, maintainer := range cf.Maintainers { if maintainer.Name == "" { lintError = fmt.Errorf("Chart.yaml: maintainer requires a name") @@ -152,7 +152,7 @@ func validateChartMaintainer(linter *support.Linter, cf *chart.Metadata) (lintEr return } -func validateChartSources(linter *support.Linter, cf *chart.Metadata) (lintError support.LintError) { +func validateChartSources(cf *chart.Metadata) (lintError support.LintError) { for _, source := range cf.Sources { if source == "" || !govalidator.IsRequestURL(source) { lintError = fmt.Errorf("Chart.yaml: 'source' invalid URL %s", source) @@ -161,7 +161,7 @@ func validateChartSources(linter *support.Linter, cf *chart.Metadata) (lintError return } -func validateChartHome(linter *support.Linter, cf *chart.Metadata) (lintError support.LintError) { +func validateChartHome(cf *chart.Metadata) (lintError support.LintError) { if cf.Home != "" && !govalidator.IsRequestURL(cf.Home) { lintError = fmt.Errorf("Chart.yaml: 'home' invalid URL %s", cf.Home) } diff --git a/pkg/lint/rules/chartfile_test.go b/pkg/lint/rules/chartfile_test.go index 9079aa7c7..6d49a55f4 100644 --- a/pkg/lint/rules/chartfile_test.go +++ b/pkg/lint/rules/chartfile_test.go @@ -17,15 +17,214 @@ limitations under the License. package rules import ( - "k8s.io/helm/pkg/lint/support" + "errors" + "os" + "path/filepath" "strings" "testing" + + "k8s.io/helm/pkg/chartutil" + "k8s.io/helm/pkg/lint/support" + "k8s.io/helm/pkg/proto/hapi/chart" ) -const badchartfile = "testdata/badchartfile" +const badChartDir = "testdata/badchartfile" +const goodChartDir = "testdata/goodone" + +var badChartFilePath string = filepath.Join(badChartDir, "Chart.yaml") +var goodChartFilePath string = filepath.Join(goodChartDir, "Chart.yaml") +var nonExistingChartFilePath string = filepath.Join(os.TempDir(), "Chart.yaml") + +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) + + err := validateChartYamlNotDirectory(nonExistingChartFilePath) + if err == nil { + t.Errorf("validateChartYamlNotDirectory to return a linter error, got no error") + } +} + +func TestValidateChartYamlFormat(t *testing.T) { + err := validateChartYamlFormat(errors.New("Read error")) + if err == nil { + t.Errorf("validateChartYamlFormat to return a linter error, got no error") + } + + err = validateChartYamlFormat(nil) + if err != nil { + t.Errorf("validateChartYamlFormat to return no error, got a linter error") + } +} + +func TestValidateChartName(t *testing.T) { + err := validateChartName(badChart) + if err == nil { + t.Errorf("validateChartName to return a linter error, got no error") + } +} + +func TestValidateChartNameDirMatch(t *testing.T) { + err := validateChartNameDirMatch(goodChartDir, goodChart) + if err != nil { + t.Errorf("validateChartNameDirMatch to return no error, gor a linter error") + } + // It has not name + err = validateChartNameDirMatch(badChartDir, badChart) + if err == nil { + t.Errorf("validatechartnamedirmatch to return a linter error, got no error") + } + + // Wrong path + err = validateChartNameDirMatch(badChartDir, goodChart) + if err == nil { + t.Errorf("validatechartnamedirmatch to return a linter error, got no error") + } +} + +func TestValidateChartVersion(t *testing.T) { + var failTest = []struct { + Version string + ErrorMsg string + }{ + {"", "'version' value is required"}, + {"0", "0 is less than or equal to 0"}, + {"waps", "is not a valid SemVer"}, + {"-3", "is not a valid SemVer"}, + } + + var successTest = []string{"0.0.1", "0.0.1+build", "0.0.1-beta"} + + for _, test := range failTest { + badChart.Version = test.Version + err := validateChartVersion(badChart) + if err == nil || !strings.Contains(err.Error(), test.ErrorMsg) { + t.Errorf("validateChartVersion(%s) to return \"%s\", got no error", test.Version, test.ErrorMsg) + } + } + + for _, version := range successTest { + badChart.Version = version + err := validateChartVersion(badChart) + if err != nil { + t.Errorf("validateChartVersion(%s) to return no error, got a linter error", version) + } + } +} + +func TestValidateChartEngine(t *testing.T) { + var successTest = []string{"", "gotpl"} + + for _, engine := range successTest { + badChart.Engine = engine + err := validateChartEngine(badChart) + if err != nil { + t.Errorf("validateChartEngine(%s) to return no error, got a linter error %s", engine, err.Error()) + } + } + + badChart.Engine = "foobar" + err := validateChartEngine(badChart) + if err == nil || !strings.Contains(err.Error(), "not valid. Valid options are [gotpl") { + t.Errorf("validateChartEngine(%s) to return an error, got no error", badChart.Engine) + } +} + +func TestValidateChartMaintainer(t *testing.T) { + var failTest = []struct { + Name string + Email string + ErrorMsg string + }{ + {"", "", "maintainer requires a name"}, + {"", "test@test.com", "maintainer requires a name"}, + {"John Snow", "wrongFormatEmail.com", "maintainer invalid email"}, + } + + var successTest = []struct { + Name string + Email string + }{ + {"John Snow", ""}, + {"John Snow", "john@winterfell.com"}, + } + + for _, test := range failTest { + badChart.Maintainers = []*chart.Maintainer{{Name: test.Name, Email: test.Email}} + err := validateChartMaintainer(badChart) + if err == nil || !strings.Contains(err.Error(), test.ErrorMsg) { + t.Errorf("validateChartMaintainer(%s, %s) to return \"%s\", got no error", test.Name, test.Email, test.ErrorMsg) + } + } + + for _, test := range successTest { + badChart.Maintainers = []*chart.Maintainer{{Name: test.Name, Email: test.Email}} + err := validateChartMaintainer(badChart) + if err != nil { + t.Errorf("validateChartMaintainer(%s, %s) to return no error, got %s", test.Name, test.Email, err.Error()) + } + } +} + +func TestValidateChartSources(t *testing.T) { + var failTest = []string{"", "RiverRun", "john@winterfell", "riverrun.io"} + var successTest = []string{"http://riverrun.io", "https://riverrun.io", "https://riverrun.io/blackfish"} + 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) + } + } + + for _, test := range successTest { + badChart.Sources = []string{test} + err := validateChartSources(badChart) + if err != nil { + t.Errorf("validateChartSources(%s) to return no error, got %s", test, err.Error()) + } + } +} + +func TestValidateChartHome(t *testing.T) { + var failTest = []string{"RiverRun", "john@winterfell", "riverrun.io"} + var successTest = []string{"", "http://riverrun.io", "https://riverrun.io", "https://riverrun.io/blackfish"} + + 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) + } + } + + for _, test := range successTest { + badChart.Home = test + err := validateChartHome(badChart) + if err != nil { + t.Errorf("validateChartHome(%s) to return no error, got %s", test, err.Error()) + } + } +} func TestChartfile(t *testing.T) { - linter := support.Linter{ChartDir: badchartfile} + linter := support.Linter{ChartDir: badChartDir} Chartfile(&linter) msgs := linter.Messages diff --git a/pkg/lint/rules/template.go b/pkg/lint/rules/template.go index 27c47b92a..15796d5e7 100644 --- a/pkg/lint/rules/template.go +++ b/pkg/lint/rules/template.go @@ -17,70 +17,200 @@ limitations under the License. package rules import ( + "bytes" "fmt" "github.com/Masterminds/sprig" - "io/ioutil" + "gopkg.in/yaml.v2" + "k8s.io/helm/pkg/chartutil" + "k8s.io/helm/pkg/engine" "k8s.io/helm/pkg/lint/support" + "k8s.io/helm/pkg/timeconv" "os" "path/filepath" + "regexp" + "strings" "text/template" ) -// Templates lints a chart's templates. func Templates(linter *support.Linter) { - templatespath := filepath.Join(linter.ChartDir, "templates") + templatesPath := filepath.Join(linter.ChartDir, "templates") - templatesExist := linter.RunLinterRule(support.WarningSev, validateTemplatesExistence(linter, templatespath)) + templatesDirExist := linter.RunLinterRule(support.WarningSev, validateTemplatesDir(linter, templatesPath)) // Templates directory is optional for now - if !templatesExist { + if !templatesDirExist { return } - linter.RunLinterRule(support.ErrorSev, validateTemplatesDir(linter, templatespath)) - linter.RunLinterRule(support.ErrorSev, validateTemplatesParseable(linter, templatespath)) -} + // Load chart and parse templates, based on tiller/release_server + chart, err := chartutil.Load(linter.ChartDir) -func validateTemplatesExistence(linter *support.Linter, templatesPath string) (lintError support.LintError) { - if _, err := os.Stat(templatesPath); err != nil { - lintError = fmt.Errorf("Templates directory not found") + chartLoaded := linter.RunLinterRule(support.ErrorSev, validateNoError(err)) + + if !chartLoaded { + return + } + + // Based on cmd/tiller/release_server.go + overrides := map[string]interface{}{ + "Release": map[string]interface{}{ + "Name": "testRelease", + "Service": "Tiller", + "Time": timeconv.Now(), + }, + "Chart": chart.Metadata, + } + + chartValues, _ := chartutil.CoalesceValues(chart, chart.Values, overrides) + renderedContentMap, err := engine.New().Render(chart, chartValues) + + renderOk := linter.RunLinterRule(support.ErrorSev, validateNoError(err)) + + if !renderOk { + return + } + + /* Iterate over all the templates to check: + - It is a .yaml file + - All the values in the template file is defined + - {{}} include | quote + - Generated content is a valid Yaml file + - Metadata.Namespace is not set + */ + for _, template := range chart.Templates { + fileName, preExecutedTemplate := template.Name, template.Data + + yamlFile := linter.RunLinterRule(support.ErrorSev, validateYamlExtension(linter, fileName)) + + if !yamlFile { + return + } + + // Check that all the templates have a matching value + linter.RunLinterRule(support.WarningSev, validateNonMissingValues(fileName, chartValues, preExecutedTemplate)) + + linter.RunLinterRule(support.WarningSev, validateQuotes(fileName, string(preExecutedTemplate))) + + renderedContent := renderedContentMap[fileName] + var yamlStruct K8sYamlStruct + // Even though K8sYamlStruct only defines Metadata namespace, an error in any other + // key will be raised as well + err := yaml.Unmarshal([]byte(renderedContent), &yamlStruct) + + validYaml := linter.RunLinterRule(support.ErrorSev, validateYamlContent(fileName, err)) + + if !validYaml { + return + } + + linter.RunLinterRule(support.ErrorSev, validateNoNamespace(fileName, yamlStruct)) } - return } +// Validation functions func validateTemplatesDir(linter *support.Linter, templatesPath string) (lintError support.LintError) { - fi, err := os.Stat(templatesPath) - if err == nil && !fi.IsDir() { + if fi, err := os.Stat(templatesPath); err != nil { + lintError = fmt.Errorf("Templates directory not found") + } else if err == nil && !fi.IsDir() { lintError = fmt.Errorf("'templates' is not a directory") } return } -func validateTemplatesParseable(linter *support.Linter, templatesPath string) (lintError support.LintError) { - tpl := template.New("tpl").Funcs(sprig.TxtFuncMap()) - - lintError = filepath.Walk(templatesPath, func(name string, fi os.FileInfo, e error) error { - if e != nil { - return e - } - if fi.IsDir() { - 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) { + // {{ .Foo.bar }} + r, _ := regexp.Compile(`(?m)(:|-)\s+{{[\w|\.|\s|\']+}}\s*$`) + functions := r.FindAllString(templateContent, -1) + + for _, str := range functions { + if match, _ := regexp.MatchString("quote", str); !match { + result := strings.Replace(str, "}}", " | quote }}", -1) + lintError = fmt.Errorf("templates: \"%s\". add \"| quote\" to your substitution functions: %s -> %s", templateName, str, result) + return } + } + + // {{ .Foo.bar }}-{{ .Foo.baz }} -> "{{ .Foo.bar }}-{{ .Foo.baz }}" + r, _ = regexp.Compile(`(?m)({{(\w|\.|\s|\')+}}(\s|-)*)+\s*$`) + functions = r.FindAllString(templateContent, -1) - data, err := ioutil.ReadFile(name) + for _, str := range functions { + result := strings.Replace(str, str, fmt.Sprintf("\"%s\"", str), -1) + lintError = fmt.Errorf("templates: \"%s\". wrap your substitution functions in double quotes: %s -> %s", templateName, str, result) + return + } + return +} + +func validateYamlExtension(linter *support.Linter, fileName string) (lintError support.LintError) { + if filepath.Ext(fileName) != ".yaml" { + lintError = fmt.Errorf("templates: \"%s\" needs to use the .yaml extension", fileName) + } + return +} + +// validateNonMissingValues checks that all the {{}} functions returns a non empty value ( or "") +// and return an error otherwise. +func validateNonMissingValues(fileName string, chartValues chartutil.Values, templateContent []byte) (lintError support.LintError) { + tmpl := template.New("tpl").Funcs(sprig.TxtFuncMap()) + var buf bytes.Buffer + var emptyValues []string + + // Supported {{ .Chart.Name }}, {{ .Chart.Name | quote }} + r, _ := regexp.Compile(`{{([\w]|\.*|\s|\|)+}}`) + functions := r.FindAllString(string(templateContent), -1) + + // Iterate over the {{ FOO }} templates, executing them against the chartValues + // We do individual templates parsing so we keep the reference for the key (str) that we want it to be interpolated. + for _, str := range functions { + newtmpl, err := tmpl.Parse(str) if err != nil { - lintError = fmt.Errorf("cannot read %s: %s", name, err) - return lintError + lintError = fmt.Errorf("templates: %s", err.Error()) + return } - newtpl, err := tpl.Parse(string(data)) - if err != nil { - lintError = fmt.Errorf("error processing %s: %s", name, err) - return lintError + err = newtmpl.Execute(&buf, chartValues) + renderedValue := buf.String() + + if renderedValue == "" || renderedValue == "" { + emptyValues = append(emptyValues, str) } - tpl = newtpl - return nil - }) + buf.Reset() + } + + if len(emptyValues) > 0 { + lintError = fmt.Errorf("templates: %s: The following functions are not returning eny value %v", fileName, emptyValues) + } + return +} + +func validateNoError(readError error) (lintError support.LintError) { + if readError != nil { + lintError = fmt.Errorf("templates: %s", readError.Error()) + } + return +} + +func validateYamlContent(filePath string, err error) (lintError support.LintError) { + if err != nil { + lintError = fmt.Errorf("templates: \"%s\". Wrong YAML content.", filePath) + } + return +} +func validateNoNamespace(filePath string, yamlStruct K8sYamlStruct) (lintError support.LintError) { + if yamlStruct.Metadata.Namespace != "" { + lintError = fmt.Errorf("templates: \"%s\". namespace option is currently NOT supported.", filePath) + } return } + +// Need to access for now to Namespace only +type K8sYamlStruct struct { + Metadata struct { + Namespace string + } +} diff --git a/pkg/lint/rules/template_test.go b/pkg/lint/rules/template_test.go index b274359b1..04fc7f42c 100644 --- a/pkg/lint/rules/template_test.go +++ b/pkg/lint/rules/template_test.go @@ -24,6 +24,38 @@ import ( const templateTestBasedir = "./testdata/albatross" +func TestValidateQuotes(t *testing.T) { + // add `| quote` lint error + 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) + if err == nil || !strings.Contains(err.Error(), "add \"| quote\" to your substitution functions") { + t.Errorf("validateQuotes('%s') to return \"add | quote error\", got no error", test) + } + } + + 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 }}\""} + + for _, test := range successTest { + err := validateQuotes("testTemplate.yaml", test) + if err != nil { + t.Errorf("validateQuotes('%s') to return not error and got \"%s\"", test, err.Error()) + } + } + + // Surrounding quotes + 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 double quotes") { + t.Errorf("validateQuotes('%s') to return \"wrap your substitution functions in double quotes\", got no error %s", test, err.Error()) + } + } + +} + func TestTemplate(t *testing.T) { linter := support.Linter{ChartDir: templateTestBasedir} Templates(&linter) diff --git a/pkg/lint/support/message.go b/pkg/lint/support/message.go index 29607f2db..169090f54 100644 --- a/pkg/lint/support/message.go +++ b/pkg/lint/support/message.go @@ -52,8 +52,6 @@ type LintError interface { error } -type ValidationFunc func(*Linter) LintError - // String prints a string representation of this Message. // // Implements fmt.Stringer. @@ -63,6 +61,11 @@ func (m Message) String() string { // Returns true if the validation passed func (l *Linter) RunLinterRule(severity Severity, lintError LintError) bool { + // severity is out of bound + if severity < 0 || int(severity) >= len(sev) { + return false + } + if lintError != nil { l.Messages = append(l.Messages, Message{Text: lintError.Error(), Severity: severity}) } diff --git a/pkg/lint/support/message_test.go b/pkg/lint/support/message_test.go index 6a1801e63..6ab0bead7 100644 --- a/pkg/lint/support/message_test.go +++ b/pkg/lint/support/message_test.go @@ -21,7 +21,38 @@ import ( "testing" ) -var _ fmt.Stringer = Message{} +var linter Linter = Linter{} +var lintError LintError = fmt.Errorf("Foobar") + +func TestRunLinterRule(t *testing.T) { + var tests = []struct { + Severity Severity + LintError error + ExpectedMessages int + ExpectedReturn bool + }{ + {ErrorSev, lintError, 1, false}, + {WarningSev, lintError, 2, false}, + {InfoSev, lintError, 3, false}, + // No error so it returns true + {ErrorSev, nil, 3, true}, + // Invalid severity values + {4, lintError, 3, false}, + {22, lintError, 3, false}, + {-1, lintError, 3, false}, + } + + for _, test := range tests { + isValid := linter.RunLinterRule(test.Severity, test.LintError) + if len(linter.Messages) != test.ExpectedMessages { + t.Errorf("RunLinterRule(%d, %v), linter.Messages should have now %d message, we got %d", test.Severity, test.LintError, test.ExpectedMessages, len(linter.Messages)) + } + + if isValid != test.ExpectedReturn { + t.Errorf("RunLinterRule(%d, %v), should have returned %t but returned %t", test.Severity, test.LintError, test.ExpectedReturn, isValid) + } + } +} func TestMessage(t *testing.T) { m := Message{ErrorSev, "Foo"}