refactor template and renderResources

Signed-off-by: wujunwei <wjw3323@live.com>

cmd/helm/template.go

@@ -47,9 +47,8 @@ faked locally. Additionally, none of the server-side testing of chart validity
 `
 
 func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
-	var validate bool
+	var validate, includeCrds, skipTests, useReleaseName bool
-	var includeCrds bool
+	var outputDir string
-	var skipTests bool
 	client := action.NewInstall(cfg)
 	valueOpts := &values.Options{}
 	var kubeVersion string
@@ -77,97 +76,75 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 			client.ReleaseName = "release-name"
 			client.Replace = true // Skip the name check
 			client.ClientOnly = !validate
-			client.APIVersions = chartutil.VersionSet(extraAPIs)
+			client.APIVersions = extraAPIs
-			client.IncludeCRDs = includeCrds
 			rel, err := runInstall(args, client, valueOpts, out)
-
+			if rel == nil || (err != nil && !settings.Debug) {
-			if err != nil && !settings.Debug {
 				if rel != nil {
 					return fmt.Errorf("%w\n\nUse --debug flag to render out invalid YAML", err)
 				}
 				return err
 			}
-
 			// We ignore a potential error here because, when the --debug flag was specified,
 			// we always want to print the YAML, even if it is not valid. The error is still returned afterwards.
-			if rel != nil {
-				var manifests bytes.Buffer
-				fmt.Fprintln(&manifests, strings.TrimSpace(rel.Manifest))
-				if !client.DisableHooks {
-					fileWritten := make(map[string]bool)
-					for _, m := range rel.Hooks {
-						if skipTests && isTestHook(m) {
-							continue
-						}
-						if client.OutputDir == "" {
-							fmt.Fprintf(&manifests, "---\n# Source: %s\n%s\n", m.Path, m.Manifest)
-						} else {
-							newDir := client.OutputDir
-							if client.UseReleaseName {
-								newDir = filepath.Join(client.OutputDir, client.ReleaseName)
-							}
-							err = writeToFile(newDir, m.Path, m.Manifest, fileWritten[m.Path])
-							if err != nil {
-								return err
-							}
-							fileWritten[m.Path] = true
-						}
 
-					}
+			if outputDir != "" && useReleaseName {
-				}
+				outputDir = filepath.Join(outputDir, client.ReleaseName)
+			}
+			fileWritten := make(map[string]bool)
 
-				// if we have a list of files to render, then check that each of the
+			// deal hooks
-				// provided files exists in the chart.
+			if !client.DisableHooks {
-				if len(showFiles) > 0 {
+				for _, m := range rel.Hooks {
-					// This is necessary to ensure consistent manifest ordering when using --show-only
+					if matchFilePatterns(m.Path, showFiles) || (skipTests && isTestHook(m)) {
-					// with globs or directory names.
+						continue
-					splitManifests := releaseutil.SplitManifests(manifests.String())
-					manifestsKeys := make([]string, 0, len(splitManifests))
-					for k := range splitManifests {
-						manifestsKeys = append(manifestsKeys, k)
 					}
-					sort.Sort(releaseutil.BySplitManifestsOrder(manifestsKeys))
+					err = writeManifest(outputDir, m.Path, m.Manifest, fileWritten, out)
-
+					if err != nil {
-					manifestNameRegex := regexp.MustCompile("# Source: [^/]+/(.+)")
+						return err
-					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)
+			}
+			// deal crds
+			if includeCrds && !client.SkipCRDs && rel.Chart != nil {
+				for _, crd := range rel.Chart.CRDObjects() {
+					if !matchFilePatterns(crd.Name, showFiles) {
+						continue
+					}
+					err = writeManifest(outputDir, crd.Name, string(crd.File.Data), fileWritten, out)
+					if err != nil {
+						return err
 					}
-				} else {
-					fmt.Fprintf(out, "%s", manifests.String())
 				}
 			}
 
+			// deal manifests
+			var manifests bytes.Buffer
+			_, _ = fmt.Fprintln(&manifests, strings.TrimSpace(rel.Manifest))
+			// 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))
+			manifestNameRegex := regexp.MustCompile("# Source: [^/]+/(.+)")
+			for k := range splitManifests {
+				manifestsKeys = append(manifestsKeys, k)
+			}
+			sort.Sort(releaseutil.BySplitManifestsOrder(manifestsKeys))
+			for _, manifestKey := range manifestsKeys {
+				manifest := splitManifests[manifestKey]
+				submatch := manifestNameRegex.FindStringSubmatch(manifest)
+				var manifestName string
+				if len(submatch) > 1 {
+					manifestName = submatch[1]
+				}
+				if matchFilePatterns(manifestName, showFiles) {
+					// remove text like # Source: XXX/XXX.yaml
+					manifest = manifestNameRegex.ReplaceAllString(manifest, "")
+					//err = writeManifest(outputDir, , manifest, fileWritten, out) todo
+					if err != nil {
+						return err
+					}
+				}
+			}
 			return err
 		},
 	}
@@ -175,14 +152,14 @@ 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.StringVar(&client.OutputDir, "output-dir", "", "writes the executed templates to files in output-dir instead of stdout")
+	f.StringVar(&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(&skipTests, "skip-tests", false, "skip tests from templated output")
 	f.BoolVar(&client.IsUpgrade, "is-upgrade", false, "set .Release.IsUpgrade instead of .Release.IsInstall")
 	f.StringVar(&kubeVersion, "kube-version", "", "Kubernetes version used for Capabilities.KubeVersion")
 	f.StringSliceVarP(&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.")
+	f.BoolVar(&useReleaseName, "release-name", false, "use release name in the output-dir path.")
 	bindPostRenderFlag(cmd, &client.PostRenderer)
 
 	return cmd
@@ -197,11 +174,7 @@ func isTestHook(h *release.Hook) bool {
 	return false
 }
 
-// The following functions (writeToFile, createOrOpenFile, and ensureDirectoryForFile)
+// writeToFile write manifests into output dir.
-// are copied from the actions package. This is part of a change to correct a
-// bug introduced by #8156. As part of the todo to refactor renderResources
-// this duplicate code should be removed. It is added here so that the API
-// surface area is as minimally impacted as possible in fixing the issue.
 func writeToFile(outputDir string, name string, data string, append bool) error {
 	outfileName := strings.Join([]string{outputDir, name}, string(filepath.Separator))
 
@@ -243,3 +216,30 @@ func ensureDirectoryForFile(file string) error {
 
 	return os.MkdirAll(baseDir, 0755)
 }
+
+func matchFilePatterns(target string, sf []string) bool {
+	if len(sf) == 0 {
+		return true
+	}
+	for _, pattern := range sf {
+		pattern = filepath.ToSlash(pattern)
+		matched, _ := filepath.Match(pattern, target)
+		if matched {
+			return true
+		}
+	}
+	return false
+}
+
+func writeManifest(outputDir, path, manifest string, fileWritten map[string]bool, outStream io.Writer) error {
+	if outputDir == "" {
+		fmt.Fprintf(outStream, "---\n# Source: %s\n%s\n", path, manifest)
+	} else {
+		err := writeToFile(outputDir, path, manifest, fileWritten[path])
+		if err != nil {
+			return err
+		}
+		fileWritten[path] = true
+	}
+	return nil
+}

pkg/action/action.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"os"
 	"path"
-	"path/filepath"
 	"regexp"
 	"strings"
 
@@ -98,12 +97,8 @@ type Configuration struct {
 }
 
 // renderResources renders the templates in a chart
-//
+func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, subNotes bool, pr postrender.PostRenderer, dryRun bool) ([]*release.Hook, *bytes.Buffer, string, error) {
-// TODO: This function is badly in need of a refactor.
+	var hs []*release.Hook
-// 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 (cfg *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) {
-	hs := []*release.Hook{}
 	b := bytes.NewBuffer(nil)
 
 	caps, err := cfg.getCapabilities()
@@ -178,41 +173,8 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu
 		return hs, b, "", err
 	}
 
-	// Aggregate all valid manifests into one big doc.
-	fileWritten := make(map[string]bool)
-
-	if includeCrds {
-		for _, crd := range ch.CRDObjects() {
-			if outputDir == "" {
-				fmt.Fprintf(b, "---\n# Source: %s\n%s\n", crd.Name, string(crd.File.Data[:]))
-			} else {
-				err = writeToFile(outputDir, crd.Filename, string(crd.File.Data[:]), fileWritten[crd.Name])
-				if err != nil {
-					return hs, b, "", err
-				}
-				fileWritten[crd.Name] = true
-			}
-		}
-	}
-
 	for _, m := range manifests {
-		if outputDir == "" {
+		fmt.Fprintf(b, "---\n# Source: %s\n%s\n", m.Name, m.Content)
-			fmt.Fprintf(b, "---\n# Source: %s\n%s\n", m.Name, m.Content)
-		} else {
-			newDir := outputDir
-			if useReleaseName {
-				newDir = filepath.Join(outputDir, releaseName)
-			}
-			// NOTE: We do not have to worry about the post-renderer because
-			// output dir is only used by `helm template`. In the next major
-			// release, we should move this logic to template only as it is not
-			// used by install or upgrade
-			err = writeToFile(newDir, m.Name, m.Content, fileWritten[m.Name])
-			if err != nil {
-				return hs, b, "", err
-			}
-			fileWritten[m.Name] = true
-		}
 	}
 
 	if pr != nil {

pkg/action/install.go

@@ -23,7 +23,6 @@ import (
 	"io/ioutil"
 	"net/url"
 	"os"
-	"path"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -68,27 +67,29 @@ type Install struct {
 
 	ChartPathOptions
 
-	ClientOnly               bool
+	ClientOnly       bool
-	Force                    bool
+	Force            bool
-	CreateNamespace          bool
+	CreateNamespace  bool
-	DryRun                   bool
+	DryRun           bool
-	DisableHooks             bool
+	DisableHooks     bool
-	Replace                  bool
+	Replace          bool
-	Wait                     bool
+	Wait             bool
-	WaitForJobs              bool
+	WaitForJobs      bool
-	Devel                    bool
+	Devel            bool
-	DependencyUpdate         bool
+	DependencyUpdate bool
-	Timeout                  time.Duration
+	Timeout          time.Duration
-	Namespace                string
+	Namespace        string
-	ReleaseName              string
+	ReleaseName      string
-	GenerateName             bool
+	GenerateName     bool
-	NameTemplate             string
+	NameTemplate     string
-	Description              string
+	Description      string
+	// Deprecated
 	OutputDir                string
 	Atomic                   bool
 	SkipCRDs                 bool
 	SubNotes                 bool
 	DisableOpenAPIValidation bool
+	// Deprecated
 	IncludeCRDs              bool
 	// KubeVersion allows specifying a custom kubernetes version to use and
 	// APIVersions allows a manual set of supported API Versions to be passed
@@ -99,7 +100,7 @@ type Install struct {
 	IsUpgrade bool
 	// Used by helm template to add the release as part of OutputDir path
 	// OutputDir/<ReleaseName>
-	UseReleaseName bool
+	UseReleaseName bool // Deprecated
 	PostRenderer   postrender.PostRenderer
 	// Lock to control raceconditions when the process receives a SIGTERM
 	Lock sync.Mutex
@@ -186,7 +187,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release.
 	return i.RunWithContext(ctx, chrt, vals)
 }
 
-// Run executes the installation with Context
+// RunWithContext executes the installation with Context
 func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals map[string]interface{}) (*release.Release, error) {
 	// Check reachability of cluster unless in client-only mode (e.g. `helm template` without `--validate`)
 	if !i.ClientOnly {
@@ -257,7 +258,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma
 	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.SubNotes, i.PostRenderer, i.DryRun)
 	// Even for errors, attach this if available
 	if manifestDoc != nil {
 		rel.Manifest = manifestDoc.String()
@@ -534,50 +535,6 @@ func (i *Install) replaceRelease(rel *release.Release) error {
 	return i.recordRelease(last)
 }
 
-// write the <data> to <output-dir>/<name>. <append> controls if the file is created or content will be appended
-func writeToFile(outputDir string, name string, data string, append bool) error {
-	outfileName := strings.Join([]string{outputDir, name}, string(filepath.Separator))
-
-	err := ensureDirectoryForFile(outfileName)
-	if err != nil {
-		return err
-	}
-
-	f, err := createOrOpenFile(outfileName, append)
-	if err != nil {
-		return err
-	}
-
-	defer f.Close()
-
-	_, err = f.WriteString(fmt.Sprintf("---\n# Source: %s\n%s\n", name, data))
-
-	if err != nil {
-		return err
-	}
-
-	fmt.Printf("wrote %s\n", outfileName)
-	return nil
-}
-
-func createOrOpenFile(filename string, append bool) (*os.File, error) {
-	if append {
-		return os.OpenFile(filename, os.O_APPEND|os.O_WRONLY, 0600)
-	}
-	return os.Create(filename)
-}
-
-// check if the directory exists to create file. creates if don't exists
-func ensureDirectoryForFile(file string) error {
-	baseDir := path.Dir(file)
-	_, err := os.Stat(baseDir)
-	if err != nil && !os.IsNotExist(err) {
-		return err
-	}
-
-	return os.MkdirAll(baseDir, defaultDirectoryPermission)
-}
-
 // NameAndChart returns the name and chart that should be used.
 //
 // This will read the flags and handle name generation if necessary.

pkg/action/upgrade.go

@@ -231,7 +231,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, u.SubNotes, u.PostRenderer, u.DryRun)
 	if err != nil {
 		return nil, nil, err
 	}