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 <ak.karis@gmail.com>

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,58 +111,8 @@ 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())
 			}
-			}
 
 			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")

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),

cmd/helm/testdata/output/template-show-only-one-and-output-dir.txt

@@ -0,0 +1 @@
+

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)

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()

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
 	}

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
+}