From dbe4fb35bdcb2ffeb64ccc4bda4d5189adda7b18 Mon Sep 17 00:00:00 2001 From: Scott Morgan Date: Sat, 23 Nov 2019 08:21:25 -0600 Subject: [PATCH] feat(template) allow linting to be run at template time refactor lint run to a function refactor template run to a function Signed-off-by: Scott Morgan --- cmd/helm/lint.go | 123 +++++++------- cmd/helm/template.go | 159 ++++++++++-------- cmd/helm/template_test.go | 17 ++ .../testdata/output/template-linting-fail.txt | 6 + .../output/template-linting-strict-fail.txt | 6 + .../output/template-linting-success.txt | 7 + 6 files changed, 194 insertions(+), 124 deletions(-) create mode 100644 cmd/helm/testdata/output/template-linting-fail.txt create mode 100644 cmd/helm/testdata/output/template-linting-strict-fail.txt create mode 100644 cmd/helm/testdata/output/template-linting-success.txt diff --git a/cmd/helm/lint.go b/cmd/helm/lint.go index bc0d1852b..f25715894 100644 --- a/cmd/helm/lint.go +++ b/cmd/helm/lint.go @@ -48,79 +48,88 @@ func newLintCmd(out io.Writer) *cobra.Command { Use: "lint PATH", Short: "examines a chart for possible issues", Long: longLintHelp, - RunE: func(cmd *cobra.Command, args []string) error { - paths := []string{"."} - if len(args) > 0 { - paths = args - } - if client.WithSubcharts { - for _, p := range paths { - filepath.Walk(filepath.Join(p, "charts"), func(path string, info os.FileInfo, err error) error { - if info != nil { - if info.Name() == "Chart.yaml" { - paths = append(paths, filepath.Dir(path)) - } else if strings.HasSuffix(path, ".tgz") || strings.HasSuffix(path, ".tar.gz") { - paths = append(paths, path) - } - } - return nil - }) - } - } - - client.Namespace = settings.Namespace() - vals, err := valueOpts.MergeValues(getter.All(settings)) + RunE: func(_ *cobra.Command, args []string) error { + err := runLint(args, client, valueOpts, out) if err != nil { return err } - var message strings.Builder - failed := 0 + return nil + }, + } - for _, path := range paths { - fmt.Fprintf(&message, "==> Linting %s\n", path) + f := cmd.Flags() + f.BoolVar(&client.Strict, "strict", false, "fail on lint warnings") + f.BoolVar(&client.WithSubcharts, "with-subcharts", false, "lint dependent charts") + addValueOptionsFlags(f, valueOpts) - result := client.Run([]string{path}, vals) + return cmd +} - // All the Errors that are generated by a chart - // that failed a lint will be included in the - // results.Messages so we only need to print - // the Errors if there are no Messages. - if len(result.Messages) == 0 { - for _, err := range result.Errors { - fmt.Fprintf(&message, "Error %s\n", err) +func runLint(args []string, client *action.Lint, valueOpts *values.Options, out io.Writer) error { + paths := []string{"."} + if len(args) > 0 { + paths = args + } + if client.WithSubcharts { + for _, p := range paths { + filepath.Walk(filepath.Join(p, "charts"), func(path string, info os.FileInfo, err error) error { + if info != nil { + if info.Name() == "Chart.yaml" { + paths = append(paths, filepath.Dir(path)) + } else if strings.HasSuffix(path, ".tgz") || strings.HasSuffix(path, ".tar.gz") { + paths = append(paths, path) } } + return nil + }) + } + } - for _, msg := range result.Messages { - fmt.Fprintf(&message, "%s\n", msg) - } + client.Namespace = settings.Namespace() + vals, err := valueOpts.MergeValues(getter.All(settings)) + if err != nil { + return err + } - if len(result.Errors) != 0 { - failed++ - } + var message strings.Builder + failed := 0 - // Adding extra new line here to break up the - // results, stops this from being a big wall of - // text and makes it easier to follow. - fmt.Fprint(&message, "\n") - } + for _, path := range paths { + fmt.Fprintf(&message, "==> Linting %s\n", path) - fmt.Fprintf(out, message.String()) + result := client.Run([]string{path}, vals) - summary := fmt.Sprintf("%d chart(s) linted, %d chart(s) failed", len(paths), failed) - if failed > 0 { - return errors.New(summary) + // All the Errors that are generated by a chart + // that failed a lint will be included in the + // results.Messages so we only need to print + // the Errors if there are no Messages. + if len(result.Messages) == 0 { + for _, err := range result.Errors { + fmt.Fprintf(&message, "Error %s\n", err) } - fmt.Fprintln(out, summary) - return nil - }, + } + + for _, msg := range result.Messages { + fmt.Fprintf(&message, "%s\n", msg) + } + + if len(result.Errors) != 0 { + failed++ + } + + // Adding extra new line here to break up the + // results, stops this from being a big wall of + // text and makes it easier to follow. + fmt.Fprint(&message, "\n") } - f := cmd.Flags() - f.BoolVar(&client.Strict, "strict", false, "fail on lint warnings") - f.BoolVar(&client.WithSubcharts, "with-subcharts", false, "lint dependent charts") - addValueOptionsFlags(f, valueOpts) + fmt.Fprintf(out, message.String()) - return cmd + summary := fmt.Sprintf("%d chart(s) linted, %d chart(s) failed", len(paths), failed) + if failed > 0 { + return errors.New(summary) + } + fmt.Fprintln(out, summary) + return nil } diff --git a/cmd/helm/template.go b/cmd/helm/template.go index 320718344..10f546a74 100644 --- a/cmd/helm/template.go +++ b/cmd/helm/template.go @@ -43,13 +43,19 @@ faked locally. Additionally, none of the server-side testing of chart validity (e.g. whether an API is supported) is done. ` +type templateOptions struct { + extraAPIs []string + showFiles []string + lint bool + includeCrds bool + validate bool +} + func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { - var validate bool - var includeCrds bool - client := action.NewInstall(cfg) + o := &templateOptions{} + installClient := action.NewInstall(cfg) + lintClient := action.NewLint() valueOpts := &values.Options{} - var extraAPIs []string - var showFiles []string cmd := &cobra.Command{ Use: "template [NAME] [CHART]", @@ -57,61 +63,17 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { Long: templateDesc, Args: require.MinimumNArgs(1), RunE: func(_ *cobra.Command, args []string) error { - client.DryRun = true - client.ReleaseName = "RELEASE-NAME" - client.Replace = true // Skip the name check - client.ClientOnly = !validate - client.APIVersions = chartutil.VersionSet(extraAPIs) - client.IncludeCRDs = includeCrds - rel, err := runInstall(args, client, valueOpts, out) - if err != nil { - return err - } - - var manifests bytes.Buffer - fmt.Fprintln(&manifests, strings.TrimSpace(rel.Manifest)) - - if !client.DisableHooks { - for _, m := range rel.Hooks { - fmt.Fprintf(&manifests, "---\n# Source: %s\n%s\n", m.Path, m.Manifest) + var err error + if o.lint { + err := runLint(args, lintClient, valueOpts, out) + if err != nil { + return err } } - // if we have a list of files to render, then check that each of the - // provided files exists in the chart. - if len(showFiles) > 0 { - splitManifests := releaseutil.SplitManifests(manifests.String()) - manifestNameRegex := regexp.MustCompile("# Source: [^/]+/(.+)") - var manifestsToRender []string - for _, f := range showFiles { - missing := true - for _, manifest := range splitManifests { - submatch := manifestNameRegex.FindStringSubmatch(manifest) - if len(submatch) == 0 { - continue - } - manifestName := submatch[1] - // manifest.Name is rendered using linux-style filepath separators on Windows as - // well as macOS/linux. - manifestPathSplit := strings.Split(manifestName, "/") - manifestPath := filepath.Join(manifestPathSplit...) - - // if the filepath provided matches a manifest path in the - // chart, render that manifest - if f == manifestPath { - manifestsToRender = append(manifestsToRender, manifest) - missing = false - } - } - if missing { - return fmt.Errorf("could not find template %s in chart", f) - } - } - for _, m := range manifestsToRender { - fmt.Fprintf(out, "---\n%s\n", m) - } - } else { - fmt.Fprintf(out, "%s", manifests.String()) + err = runTemplate(args, installClient, valueOpts, o, out) + if err != nil { + return err } return nil @@ -120,19 +82,82 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { // Function providing dynamic auto-completion completion.RegisterValidArgsFunc(cmd, func(cmd *cobra.Command, args []string, toComplete string) ([]string, completion.BashCompDirective) { - return compInstall(args, toComplete, client) + return compInstall(args, toComplete, installClient) }) f := cmd.Flags() - addInstallFlags(f, client, valueOpts) - f.StringArrayVarP(&showFiles, "show-only", "s", []string{}, "only show manifests rendered from the given templates") - f.StringVar(&client.OutputDir, "output-dir", "", "writes the executed templates to files in output-dir instead of stdout") - f.BoolVar(&validate, "validate", false, "validate your manifests against the Kubernetes cluster you are currently pointing at. This is the same validation performed on an install") - f.BoolVar(&includeCrds, "include-crds", false, "include CRDs in the templated output") - f.BoolVar(&client.IsUpgrade, "is-upgrade", false, "set .Release.IsUpgrade instead of .Release.IsInstall") - f.StringArrayVarP(&extraAPIs, "api-versions", "a", []string{}, "Kubernetes api versions used for Capabilities.APIVersions") - f.BoolVar(&client.UseReleaseName, "release-name", false, "use release name in the output-dir path.") - bindPostRenderFlag(cmd, &client.PostRenderer) + addInstallFlags(f, installClient, valueOpts) + f.StringArrayVarP(&o.showFiles, "show-only", "s", []string{}, "only show manifests rendered from the given templates") + f.StringVar(&installClient.OutputDir, "output-dir", "", "writes the executed templates to files in output-dir instead of stdout") + f.BoolVar(&installClient.IsUpgrade, "is-upgrade", false, "set .Release.IsUpgrade instead of .Release.IsInstall") + f.BoolVar(&o.validate, "validate", false, "validate your manifests against the Kubernetes cluster you are currently pointing at. This is the same validation performed on an install") + f.BoolVar(&o.includeCrds, "include-crds", false, "include CRDs in the templated output") + f.BoolVar(&o.lint, "lint", false, "examines a chart for possible issues") + f.BoolVar(&lintClient.Strict, "strict", false, "fail on lint warnings") + f.StringArrayVarP(&o.extraAPIs, "api-versions", "a", []string{}, "Kubernetes api versions used for Capabilities.APIVersions") + f.BoolVar(&installClient.UseReleaseName, "release-name", false, "use release name in the output-dir path.") + bindPostRenderFlag(cmd, &installClient.PostRenderer) return cmd } + +func runTemplate(args []string, client *action.Install, valueOpts *values.Options, o *templateOptions, out io.Writer) error { + client.DryRun = true + client.ReleaseName = "RELEASE-NAME" + client.Replace = true // Skip the name check + client.ClientOnly = !o.validate + client.APIVersions = chartutil.VersionSet(o.extraAPIs) + client.IncludeCRDs = o.includeCrds + rel, err := runInstall(args, client, valueOpts, out) + if err != nil { + return err + } + + var manifests bytes.Buffer + fmt.Fprintln(&manifests, strings.TrimSpace(rel.Manifest)) + + if !client.DisableHooks { + for _, m := range rel.Hooks { + fmt.Fprintf(&manifests, "---\n# Source: %s\n%s\n", m.Path, m.Manifest) + } + } + + // if we have a list of files to render, then check that each of the + // provided files exists in the chart. + if len(o.showFiles) > 0 { + splitManifests := releaseutil.SplitManifests(manifests.String()) + manifestNameRegex := regexp.MustCompile("# Source: [^/]+/(.+)") + var manifestsToRender []string + for _, f := range o.showFiles { + missing := true + for _, manifest := range splitManifests { + submatch := manifestNameRegex.FindStringSubmatch(manifest) + if len(submatch) == 0 { + continue + } + manifestName := submatch[1] + // manifest.Name is rendered using linux-style filepath separators on Windows as + // well as macOS/linux. + manifestPathSplit := strings.Split(manifestName, "/") + manifestPath := filepath.Join(manifestPathSplit...) + + // if the filepath provided matches a manifest path in the + // chart, render that manifest + if f == manifestPath { + manifestsToRender = append(manifestsToRender, manifest) + missing = false + } + } + if missing { + return fmt.Errorf("could not find template %s in chart", f) + } + } + for _, m := range manifestsToRender { + fmt.Fprintf(out, "---\n%s\n", m) + } + } else { + fmt.Fprintf(out, "%s", manifests.String()) + } + + return nil +} diff --git a/cmd/helm/template_test.go b/cmd/helm/template_test.go index dc7987d01..d6697d98a 100644 --- a/cmd/helm/template_test.go +++ b/cmd/helm/template_test.go @@ -74,6 +74,23 @@ func TestTemplateCmd(t *testing.T) { cmd: fmt.Sprintf("template '%s'", "testdata/testcharts/chart-with-template-lib-archive-dep"), golden: "output/template-chart-with-template-lib-archive-dep.txt", }, + { + name: "check linting success", + cmd: fmt.Sprintf("template '%s' --lint", "testdata/testcharts/empty"), + golden: "output/template-linting-success.txt", + }, + { + name: "check linting fail", + cmd: fmt.Sprintf("template '%s' --lint", "testdata/testcharts/novals"), + wantError: true, + golden: "output/template-linting-fail.txt", + }, + { + name: "check linting strict fail", + cmd: fmt.Sprintf("template '%s' --lint --strict", "testdata/testcharts/novals"), + wantError: true, + golden: "output/template-linting-strict-fail.txt", + }, { name: "check kube api versions", cmd: fmt.Sprintf("template --api-versions helm.k8s.io/test '%s'", chartPath), diff --git a/cmd/helm/testdata/output/template-linting-fail.txt b/cmd/helm/testdata/output/template-linting-fail.txt new file mode 100644 index 000000000..aa7123f78 --- /dev/null +++ b/cmd/helm/testdata/output/template-linting-fail.txt @@ -0,0 +1,6 @@ +==> Linting testdata/testcharts/novals +[INFO] Chart.yaml: icon is recommended +[INFO] values.yaml: file does not exist +[ERROR] templates/: template: novals/templates/alpine-pod.yaml:18:33: executing "novals/templates/alpine-pod.yaml" at <.Release.Time.Seconds>: nil pointer evaluating interface {}.Seconds + +Error: 1 chart(s) linted, 1 chart(s) failed diff --git a/cmd/helm/testdata/output/template-linting-strict-fail.txt b/cmd/helm/testdata/output/template-linting-strict-fail.txt new file mode 100644 index 000000000..ffe77211a --- /dev/null +++ b/cmd/helm/testdata/output/template-linting-strict-fail.txt @@ -0,0 +1,6 @@ +==> Linting testdata/testcharts/novals +[INFO] Chart.yaml: icon is recommended +[INFO] values.yaml: file does not exist +[ERROR] templates/: template: novals/templates/alpine-pod.yaml:4:36: executing "novals/templates/alpine-pod.yaml" at <.Values.Name>: map has no entry for key "Name" + +Error: 1 chart(s) linted, 1 chart(s) failed diff --git a/cmd/helm/testdata/output/template-linting-success.txt b/cmd/helm/testdata/output/template-linting-success.txt new file mode 100644 index 000000000..44e050793 --- /dev/null +++ b/cmd/helm/testdata/output/template-linting-success.txt @@ -0,0 +1,7 @@ +==> Linting testdata/testcharts/empty +[INFO] Chart.yaml: icon is recommended + +1 chart(s) linted, 0 chart(s) failed +--- +# Source: empty/templates/empty.yaml +# This file is intentionally blank