From 56bea0a1066059f6bd224d88b7fe5dbc63fb6eb1 Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Fri, 28 May 2021 14:08:27 +0200 Subject: [PATCH] fix(template) Do not error when both template show-only and output-dir Previously, when one set --show-only and --output-dir together, helm template would throw an error and all files were rendered. Refactor some of the code so that manifests and hooks are filtered in a central location and only render manifests and hooks that match --show-only. Additionally, use StringSliceVarP for the --show-only flag. Fixes #9722 Signed-off-by: Andreas Karis --- cmd/helm/template.go | 58 +--------------- cmd/helm/template_test.go | 9 +++ .../template-show-only-one-and-output-dir.txt | 1 + pkg/action/action.go | 9 ++- pkg/action/install.go | 3 +- pkg/action/upgrade.go | 2 +- pkg/releaseutil/manifest.go | 67 +++++++++++++++++++ 7 files changed, 89 insertions(+), 60 deletions(-) create mode 100644 cmd/helm/testdata/output/template-show-only-one-and-output-dir.txt diff --git a/cmd/helm/template.go b/cmd/helm/template.go index e3c1d421f..f701dcdea 100644 --- a/cmd/helm/template.go +++ b/cmd/helm/template.go @@ -23,8 +23,6 @@ import ( "os" "path" "path/filepath" - "regexp" - "sort" "strings" "helm.sh/helm/v3/pkg/release" @@ -35,7 +33,6 @@ import ( "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/cli/values" - "helm.sh/helm/v3/pkg/releaseutil" ) const templateDesc = ` @@ -54,7 +51,6 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { valueOpts := &values.Options{} var kubeVersion string var extraAPIs []string - var showFiles []string cmd := &cobra.Command{ Use: "template [NAME] [CHART]", @@ -115,57 +111,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } } - - // 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 { - // This is necessary to ensure consistent manifest ordering when using --show-only - // with globs or directory names. - splitManifests := releaseutil.SplitManifests(manifests.String()) - manifestsKeys := make([]string, 0, len(splitManifests)) - for k := range splitManifests { - manifestsKeys = append(manifestsKeys, k) - } - sort.Sort(releaseutil.BySplitManifestsOrder(manifestsKeys)) - - manifestNameRegex := regexp.MustCompile("# Source: [^/]+/(.+)") - var manifestsToRender []string - for _, f := range showFiles { - missing := true - // Use linux-style filepath separators to unify user's input path - f = filepath.ToSlash(f) - for _, manifestKey := range manifestsKeys { - manifest := splitManifests[manifestKey] - 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, "/") - // manifest.Path is connected using linux-style filepath separators on Windows as - // well as macOS/linux - manifestPath := strings.Join(manifestPathSplit, "/") - - // if the filepath provided matches a manifest path in the - // chart, render that manifest - if matched, _ := filepath.Match(f, manifestPath); !matched { - continue - } - 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()) - } + fmt.Fprintf(out, "%s", manifests.String()) } return err @@ -174,7 +120,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f := cmd.Flags() addInstallFlags(cmd, f, client, valueOpts) - f.StringArrayVarP(&showFiles, "show-only", "s", []string{}, "only show manifests rendered from the given templates") + f.StringSliceVarP(&client.FileFilter, "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") diff --git a/cmd/helm/template_test.go b/cmd/helm/template_test.go index 30ee2722b..9d2dd7ec8 100644 --- a/cmd/helm/template_test.go +++ b/cmd/helm/template_test.go @@ -18,6 +18,7 @@ package main import ( "fmt" + "os" "path/filepath" "testing" ) @@ -25,6 +26,9 @@ import ( var chartPath = "testdata/testcharts/subchart" func TestTemplateCmd(t *testing.T) { + outputDir := "/tmp/template-with-show-only-one-and-output-dir" + defer os.RemoveAll(outputDir) + tests := []cmdTestCase{ { name: "check name", @@ -94,6 +98,11 @@ func TestTemplateCmd(t *testing.T) { cmd: fmt.Sprintf("template '%s' --show-only templates/service.yaml", chartPath), golden: "output/template-show-only-one.txt", }, + { + name: "template with show-only one and output-dir", + cmd: fmt.Sprintf("template '%s' --show-only templates/service.yaml --output-dir '%s'", chartPath, outputDir), + golden: "output/template-show-only-one-and-output-dir.txt", + }, { name: "template with show-only multiple", cmd: fmt.Sprintf("template '%s' --show-only templates/service.yaml --show-only charts/subcharta/templates/service.yaml", chartPath), diff --git a/cmd/helm/testdata/output/template-show-only-one-and-output-dir.txt b/cmd/helm/testdata/output/template-show-only-one-and-output-dir.txt new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/cmd/helm/testdata/output/template-show-only-one-and-output-dir.txt @@ -0,0 +1 @@ + diff --git a/pkg/action/action.go b/pkg/action/action.go index 38ba638e4..d11752c4e 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -102,7 +102,7 @@ type Configuration struct { // TODO: This function is badly in need of a refactor. // TODO: As part of the refactor the duplicate code in cmd/helm/template.go should be removed // This code has to do with writing files to disk. -func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer, dryRun bool) ([]*release.Hook, *bytes.Buffer, string, error) { +func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, fileFilter []string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer, dryRun bool) ([]*release.Hook, *bytes.Buffer, string, error) { hs := []*release.Hook{} b := bytes.NewBuffer(nil) @@ -177,7 +177,6 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values } return hs, b, "", err } - // Aggregate all valid manifests into one big doc. fileWritten := make(map[string]bool) @@ -195,6 +194,12 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values } } + // Filter manifests according to --show-only + manifests, hs, err = releaseutil.FilterManifestsAndHooks(manifests, hs, fileFilter) + if err != nil { + return hs, b, "", err + } + for _, m := range manifests { if outputDir == "" { fmt.Fprintf(b, "---\n# Source: %s\n%s\n", m.Name, m.Content) diff --git a/pkg/action/install.go b/pkg/action/install.go index af99717d1..c226f574d 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -87,6 +87,7 @@ type Install struct { NameTemplate string Description string OutputDir string + FileFilter []string Atomic bool SkipCRDs bool SubNotes bool @@ -242,7 +243,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. rel := i.createRelease(chrt, vals) var manifestDoc *bytes.Buffer - rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, i.DryRun) + rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.FileFilter, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, i.DryRun) // Even for errors, attach this if available if manifestDoc != nil { rel.Manifest = manifestDoc.String() diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 3b3dd3f1c..af14fe654 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -212,7 +212,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin return nil, nil, err } - hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, u.DryRun) + hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", []string{}, u.SubNotes, false, false, u.PostRenderer, u.DryRun) if err != nil { return nil, nil, err } diff --git a/pkg/releaseutil/manifest.go b/pkg/releaseutil/manifest.go index 0b04a4599..ea553fca8 100644 --- a/pkg/releaseutil/manifest.go +++ b/pkg/releaseutil/manifest.go @@ -18,9 +18,12 @@ package releaseutil import ( "fmt" + "path/filepath" "regexp" "strconv" "strings" + + "helm.sh/helm/v3/pkg/release" ) // SimpleHead defines what the structure of the head of a manifest file @@ -70,3 +73,67 @@ func (a BySplitManifestsOrder) Less(i, j int) bool { return anum < bnum } func (a BySplitManifestsOrder) Swap(i, j int) { a[i], a[j] = a[j], a[i] } + +// FilterManifestsAndHooks takes a map of manifests and a map of *hooks and returns only those which match +// the fileFilter map +func FilterManifestsAndHooks(manifests []Manifest, hooks []*release.Hook, fileFilter []string) ([]Manifest, []*release.Hook, error) { + if len(fileFilter) == 0 { + return manifests, hooks, nil + } + + // Ignore everything until the first slash + // This will look like: dir/templates/template.yaml + // And the regex will return templates/template.yaml + pathRegex := regexp.MustCompile("[^/]+/(.+)") + + filteredManifests := make([]Manifest, 0) + filteredHooks := make([]*release.Hook, 0) + var missing bool + for _, fileName := range fileFilter { + missing = true + // Use linux-style filepath separators to unify user's input path + fileName = filepath.ToSlash(fileName) + for _, manifest := range manifests { + if !IsPathMatch(fileName, manifest.Name, pathRegex) { + continue + } + missing = false + filteredManifests = append(filteredManifests, manifest) + } + // If the path was found in the manifest, we do not have to search for it in the hooks + if !missing { + continue + } + for _, hook := range hooks { + if !IsPathMatch(fileName, hook.Path, pathRegex) { + continue + } + missing = false + filteredHooks = append(filteredHooks, hook) + } + if missing { + return nil, nil, fmt.Errorf("Could not find template %s in chart", fileName) + } + } + + return filteredManifests, filteredHooks, nil +} + +func IsPathMatch(fileName string, path string, pathRegex *regexp.Regexp) bool { + submatch := pathRegex.FindStringSubmatch(path) + if len(submatch) == 0 { + return false + } + + submatchPath := submatch[1] + // hook.Path is rendered using linux-style filepath separators on Windows as + // well as macOS/linux. + pathSplit := strings.Split(submatchPath, "/") + // hook.Path is connected using linux-style filepath separators on Windows as + // well as macOS/linux + joinedPath := strings.Join(pathSplit, "/") + // if the filepath provided matches a manifest path in the + // chart, render that manifest + matched, _ := filepath.Match(fileName, joinedPath) + return matched +}