Fix issue with install and upgrade running all hooks

When #8156 was merged it had the side effect that all hooks were
run all the time. All the hooks were put in the flow of the
content rendered and sent to Kubernetes on every command.

For example, if you ran the following 2 commands the test hooks
would run:

    helm create foo
    helm install foo ./foo

This should not run any hooks. But, the generated test hook is run.

The change in this commit moves the writing of the hooks to output
or disk back into the template command rather than in a private
function within the actions. This is where it was for v3.2.

One side effect is that post renderers will not work on hooks. This
was the case in v3.2. Since this bug is blocking the release of v3.3.0
it is being rolled back. A refactor effort is underway for this section
of code. post renderer for hooks should be added back as part of that
work. Since post renderer hooks did not make it into a release it
is ok to roll it back for now.

There is code in the cmd/helm package that has been duplicated from
pkg/action. This is a temporary measure to fix the immediate bug
with plans to correct the situation as part of a refactor
of renderResources.

Signed-off-by: Matt Farina <matt@mattfarina.com>

cmd/helm/template.go

@@ -20,6 +20,8 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"os"
+	"path"
 	"path/filepath"
 	"regexp"
 	"sort"
@@ -79,6 +81,25 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 			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 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 we have a list of files to render, then check that each of the
 				// provided files exists in the chart.
@@ -149,3 +170,50 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 
 	return cmd
 }
+
+// The following functions (writeToFile, createOrOpenFile, and ensureDirectoryForFile)
+// are coppied 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))
+
+	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)
+}
+
+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, 0755)
+}

pkg/action/action.go

@@ -99,7 +99,9 @@ type Configuration struct {
 // renderResources renders the templates in a chart
 //
 // TODO: This function is badly in need of a refactor.
-func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, disableHooks bool, pr postrender.PostRenderer, dryRun bool) ([]*release.Hook, *bytes.Buffer, string, error) {
+// 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 dick.
+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) {
 	hs := []*release.Hook{}
 	b := bytes.NewBuffer(nil)
 
@@ -212,24 +214,6 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values
 		}
 	}
 
-	if !disableHooks && len(hs) > 0 {
-		for _, h := range hs {
-			if outputDir == "" {
-				fmt.Fprintf(b, "---\n# Source: %s\n%s\n", h.Path, h.Manifest)
-			} else {
-				newDir := outputDir
-				if useReleaseName {
-					newDir = filepath.Join(outputDir, releaseName)
-				}
-				err = writeToFile(newDir, h.Path, h.Manifest, fileWritten[h.Path])
-				if err != nil {
-					return hs, b, "", err
-				}
-				fileWritten[h.Path] = true
-			}
-		}
-	}
-
 	if pr != nil {
 		b, err = pr.Run(b)
 		if err != nil {

pkg/action/install.go

@@ -236,7 +236,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.DisableHooks, i.PostRenderer, i.DryRun)
+	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)
 	// Even for errors, attach this if available
 	if manifestDoc != nil {
 		rel.Manifest = manifestDoc.String()

pkg/action/install_test.go

@@ -499,9 +499,6 @@ func TestInstallReleaseOutputDir(t *testing.T) {
 	_, err = os.Stat(filepath.Join(dir, "hello/templates/with-partials"))
 	is.NoError(err)
 
-	_, err = os.Stat(filepath.Join(dir, "hello/templates/hooks"))
-	is.NoError(err)
-
 	_, err = os.Stat(filepath.Join(dir, "hello/templates/rbac"))
 	is.NoError(err)
 
@@ -542,9 +539,6 @@ func TestInstallOutputDirWithReleaseName(t *testing.T) {
 	_, err = os.Stat(filepath.Join(newDir, "hello/templates/with-partials"))
 	is.NoError(err)
 
-	_, err = os.Stat(filepath.Join(newDir, "hello/templates/hooks"))
-	is.NoError(err)
-
 	_, err = os.Stat(filepath.Join(newDir, "hello/templates/rbac"))
 	is.NoError(err)
 

pkg/action/upgrade.go

@@ -222,7 +222,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, false, u.PostRenderer, u.DryRun)
+	hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, u.DryRun)
 	if err != nil {
 		return nil, nil, err
 	}