Add support for optionally post-rendering hooks

Fixes #7891; fixes #10961.

This PR adds support for optionally post-rendering hooks.

I opted to implement this by adding two new CLI flags:
`--post-renderer-hooks` and `--post-renderer-hooks-args`. Why?

1. It retains full backwards compatibility: hooks continue to be skipped
   by the main post-renderer.

2. It gives users the ability to customize their main manifests and
   hooks in separate ways. One example of where this might be useful
   is if somebody is using kubectl-kustomize as their post-renderer to
   add a new resource to their chart. If we reused that post-renderer
   as-is, we would duplicate the new resource in the hooks manifest
   which does not seem like a good idea.

Two remaining possible issues with this implementation are:

1. It repeatedly re-exec-ing the hooks post-renderer against each hook
   manifest, which may incur a perf penalty.
2. Some post-renderers may want to apply different transforms for
   different hooks and may find parsing the comment added to the hook
   manifests to be challenging.

But I think these issues are fine to ignore for now: users can use the
Go library or write a custom plugin if they want finer-grained control.

Signed-off-by: Michael Lee <michael.lee.0x2a@gmail.com>

cmd/helm/flags.go

@@ -37,9 +37,11 @@ import (
 )
 
 const (
-	outputFlag         = "output"
-	postRenderFlag     = "post-renderer"
-	postRenderArgsFlag = "post-renderer-args"
+	outputFlag              = "output"
+	postRenderMainFlag      = "post-renderer"
+	postRenderMainArgsFlag  = "post-renderer-args"
+	postRenderHooksFlag     = "post-renderer-hooks"
+	postRenderHooksArgsFlag = "post-renderer-hooks-args"
 )
 
 func addValueOptionsFlags(f *pflag.FlagSet, v *values.Options) {
@@ -115,10 +117,14 @@ func (o *outputValue) Set(s string) error {
 	return nil
 }
 
-func bindPostRenderFlag(cmd *cobra.Command, varRef *postrender.PostRenderer) {
-	p := &postRendererOptions{varRef, "", []string{}}
-	cmd.Flags().Var(&postRendererString{p}, postRenderFlag, "the path to an executable to be used for post rendering. If it exists in $PATH, the binary will be used, otherwise it will try to look for the executable at the given path")
-	cmd.Flags().Var(&postRendererArgsSlice{p}, postRenderArgsFlag, "an argument to the post-renderer (can specify multiple)")
+func bindPostRenderFlag(cmd *cobra.Command, mainRef *postrender.PostRenderer, hooksRef *postrender.PostRenderer) {
+	mainOptions := &postRendererOptions{mainRef, "", []string{}}
+	cmd.Flags().Var(&postRendererString{mainOptions}, postRenderMainFlag, "the path to an executable to be used for post rendering CRDs and manifests. If it exists in $PATH, the binary will be used, otherwise it will try to look for the executable at the given path")
+	cmd.Flags().Var(&postRendererArgsSlice{mainOptions}, postRenderMainArgsFlag, "an argument to the main post-renderer (can specify multiple)")
+
+	hooksOptions := &postRendererOptions{hooksRef, "", []string{}}
+	cmd.Flags().Var(&postRendererString{hooksOptions}, postRenderHooksFlag, "the path to an executable to be used for post rendering hooks. If it exists in $PATH, the binary will be used, otherwise it will try to look for the executable at the given path")
+	cmd.Flags().Var(&postRendererArgsSlice{hooksOptions}, postRenderHooksArgsFlag, "an argument to the hooks post-renderer (can specify multiple)")
 }
 
 type postRendererOptions struct {

cmd/helm/install.go

@@ -160,7 +160,7 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 
 	addInstallFlags(cmd, cmd.Flags(), client, valueOpts)
 	bindOutputFlag(cmd, &outfmt)
-	bindPostRenderFlag(cmd, &client.PostRenderer)
+	bindPostRenderFlag(cmd, &client.PostRenderer, &client.PostRendererHooks)
 
 	return cmd
 }

cmd/helm/template.go

@@ -200,7 +200,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	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.")
-	bindPostRenderFlag(cmd, &client.PostRenderer)
+	bindPostRenderFlag(cmd, &client.PostRenderer, &client.PostRendererHooks)
 
 	return cmd
 }

cmd/helm/upgrade.go

@@ -136,6 +136,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 					instClient.Namespace = client.Namespace
 					instClient.Atomic = client.Atomic
 					instClient.PostRenderer = client.PostRenderer
+					instClient.PostRendererHooks = client.PostRendererHooks
 					instClient.DisableOpenAPIValidation = client.DisableOpenAPIValidation
 					instClient.SubNotes = client.SubNotes
 					instClient.Description = client.Description
@@ -266,7 +267,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	addChartPathOptionsFlags(f, &client.ChartPathOptions)
 	addValueOptionsFlags(f, valueOpts)
 	bindOutputFlag(cmd, &outfmt)
-	bindPostRenderFlag(cmd, &client.PostRenderer)
+	bindPostRenderFlag(cmd, &client.PostRenderer, &client.PostRendererHooks)
 
 	err := cmd.RegisterFlagCompletionFunc("version", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 		if len(args) != 2 {

pkg/action/action.go

@@ -103,7 +103,19 @@ type Configuration struct {
 // 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, interactWithRemote, enableDNS bool) ([]*release.Hook, *bytes.Buffer, string, error) {
+func (cfg *Configuration) renderResources(
+	ch *chart.Chart,
+	values chartutil.Values,
+	releaseName,
+	outputDir string,
+	subNotes,
+	useReleaseName,
+	includeCrds bool,
+	mainPostRenderer postrender.PostRenderer,
+	hooksPostRenderer postrender.PostRenderer,
+	interactWithRemote,
+	enableDNS bool,
+) ([]*release.Hook, *bytes.Buffer, string, error) {
 	hs := []*release.Hook{}
 	b := bytes.NewBuffer(nil)
 
@@ -218,13 +230,27 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu
 		}
 	}
 
-	if pr != nil {
-		b, err = pr.Run(b)
+	if mainPostRenderer != nil {
+		b, err = mainPostRenderer.Run(b)
 		if err != nil {
 			return hs, b, notes, errors.Wrap(err, "error while running post render on files")
 		}
 	}
 
+	// Post-rendering hooks is disabled by default for backwards compat.
+	// See note above about outputDir.
+	if hooksPostRenderer != nil && outputDir == "" {
+		for _, hook := range hs {
+			hookBuffer := bytes.NewBuffer(nil)
+			fmt.Fprintf(hookBuffer, "# Source: %s\n%s\n", hook.Name, hook.Manifest)
+			newManifest, err := hooksPostRenderer.Run(hookBuffer)
+			if err != nil {
+				return hs, b, notes, errors.Wrapf(err, "error while running post render on hook %v", hook.Name)
+			}
+			hook.Manifest = newManifest.String()
+		}
+	}
+
 	return hs, b, notes, nil
 }
 

pkg/action/install.go

@@ -104,8 +104,9 @@ type Install struct {
 	EnableDNS bool
 	// Used by helm template to add the release as part of OutputDir path
 	// OutputDir/<ReleaseName>
-	UseReleaseName bool
-	PostRenderer   postrender.PostRenderer
+	UseReleaseName    bool
+	PostRenderer      postrender.PostRenderer
+	PostRendererHooks postrender.PostRenderer
 	// Lock to control raceconditions when the process receives a SIGTERM
 	Lock sync.Mutex
 }
@@ -301,7 +302,19 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma
 	rel := i.createRelease(chrt, vals, i.Labels)
 
 	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, interactWithRemote, i.EnableDNS)
+	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.PostRendererHooks,
+		interactWithRemote,
+		i.EnableDNS,
+	)
 	// Even for errors, attach this if available
 	if manifestDoc != nil {
 		rel.Manifest = manifestDoc.String()

pkg/action/install_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package action
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"io"
@@ -491,6 +492,51 @@ func TestInstallRelease_Atomic_Interrupted(t *testing.T) {
 	is.Equal(err, driver.ErrReleaseNotFound)
 
 }
+
+type testPostRenderer struct {
+	injectedStr string
+}
+
+func (p *testPostRenderer) Run(renderedManifests *bytes.Buffer) (*bytes.Buffer, error) {
+	out := bytes.NewBuffer(nil)
+	out.WriteString(fmt.Sprintf("# %v\n", p.injectedStr))
+	out.Write(renderedManifests.Bytes())
+	return out, nil
+}
+
+func TestInstallRelease_WithPostRenderer_EnabledForMain(t *testing.T) {
+	injectedStr := "Added by post-renderer"
+	is := assert.New(t)
+	instAction := installAction(t)
+	instAction.PostRenderer = &testPostRenderer{injectedStr}
+	res, err := instAction.Run(buildChart(), map[string]interface{}{})
+	if err != nil {
+		t.Fatalf("Failed install: %s", err)
+	}
+	is.Contains(res.Manifest, injectedStr)
+	for _, hook := range res.Hooks {
+		is.NotContains(hook.Manifest, injectedStr)
+	}
+}
+
+func TestInstallRelease_WithPostRenderer_EnabledAll(t *testing.T) {
+	mainInjectedStr := "Added by main post-renderer"
+	hooksInjectedStr := "Added by hooks post-renderer"
+
+	is := assert.New(t)
+	instAction := installAction(t)
+	instAction.PostRenderer = &testPostRenderer{mainInjectedStr}
+	instAction.PostRendererHooks = &testPostRenderer{hooksInjectedStr}
+	res, err := instAction.Run(buildChart(), map[string]interface{}{})
+	if err != nil {
+		t.Fatalf("Failed install: %s", err)
+	}
+	is.Contains(res.Manifest, mainInjectedStr)
+	for _, hook := range res.Hooks {
+		is.Contains(hook.Manifest, hooksInjectedStr)
+	}
+}
+
 func TestNameTemplate(t *testing.T) {
 	testCases := []nameTemplateTestCase{
 		// Just a straight up nop please

pkg/action/upgrade.go

@@ -97,11 +97,13 @@ type Upgrade struct {
 	// Description is the description of this operation
 	Description string
 	Labels      map[string]string
-	// PostRender is an optional post-renderer
+	// PostRender is an optional post-renderer that runs on manifests and CRDs.
 	//
 	// If this is non-nil, then after templates are rendered, they will be sent to the
 	// post renderer before sending to the Kubernetes API server.
 	PostRenderer postrender.PostRenderer
+	// PostRendererHooks is similar to PostRenderer, except it runs only on hooks.
+	PostRendererHooks postrender.PostRenderer
 	// DisableOpenAPIValidation controls whether OpenAPI validation is enforced.
 	DisableOpenAPIValidation bool
 	// Get missing dependencies
@@ -259,7 +261,19 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin
 		interactWithRemote = true
 	}
 
-	hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, interactWithRemote, u.EnableDNS)
+	hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(
+		chart,
+		valuesToRender,
+		"",
+		"",
+		u.SubNotes,
+		false,
+		false,
+		u.PostRenderer,
+		u.PostRendererHooks,
+		interactWithRemote,
+		u.EnableDNS,
+	)
 	if err != nil {
 		return nil, nil, err
 	}