From 697821edb2ef8be1dc51f325e7083d6bcdd98b6d Mon Sep 17 00:00:00 2001
From: Dan Walters <dan@walters.io>
Date: Sat, 19 Apr 2025 20:07:43 -0500
Subject: [PATCH] feat: send hooks through the post-renderer

This is done by moving the post render logic to happen before manifests
are split into hooks & other.

Serialize all manifests, including hooks, to a buffer before running
them through the post renderer, then parse the output back into the
filename -> content map when complete.

This is a change in behavior from helm v3, where hooks are not post
rendered by default.  A separate, backwards compatible PR to address
this in helm v3 can be found in #30778.

fixes #7891

Signed-off-by: Dan Walters <dan@walters.io>
---
 pkg/action/action.go      | 76 ++++++++++++++++++++++++++++++++++-----
 pkg/action/action_test.go | 27 ++++++++++++++
 2 files changed, 94 insertions(+), 9 deletions(-)

diff --git a/pkg/action/action.go b/pkg/action/action.go
index 40194dfd7..b2723d231 100644
--- a/pkg/action/action.go
+++ b/pkg/action/action.go
@@ -25,6 +25,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"sync"
 	"text/template"
@@ -91,6 +92,63 @@ type Configuration struct {
 	mutex sync.Mutex
 }
 
+var sep = regexp.MustCompile("(?:^|\\s*\n)---\\s*")
+var sourceFilename = regexp.MustCompile("^# Source: (\\S+)\n")
+
+// Runs the PostRenderer on the given files, returning an updated map when complete.
+func runPostRenderer(pr postrender.PostRenderer, files map[string]string) (map[string]string, error) {
+	postRenderedFiles := make(map[string]string)
+
+	// Serialize to a giant buffer, with a comment to indicate the filename
+	b := bytes.NewBuffer(nil)
+	for filename, content := range files {
+		// Skip partials and empty manifests
+		// Note that in helm v3, hooks are not run through the post renderer by default.
+		if content == "" || strings.HasPrefix(path.Base(filename), "_") {
+			continue
+		}
+
+		// Buffer for the post renderer
+		_, err := fmt.Fprintf(b, "---\n# Source: %s\n%s", filename, content)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	// Run through the post renderer.
+	b, err := pr.Run(b)
+	if err != nil {
+		return nil, err
+	}
+
+	// Rebuild the files map from the post render output.
+	docs := sep.Split(b.String(), -1)
+	for i, d := range docs {
+		if d == "" {
+			continue
+		}
+
+		// If the "# Source: ..." comments were preserved, we will keep the same filename here.
+		var filename string
+		m := sourceFilename.FindStringSubmatch(d)
+		if m != nil {
+			filename = m[1]
+			d = d[len(m[0]):]
+		} else {
+			filename = fmt.Sprintf("manifest-%d", i)
+		}
+
+		// Append the doc to the named pseudo-file
+		if data, ok := postRenderedFiles[filename]; ok {
+			postRenderedFiles[filename] = data + "\n---\n" + d
+		} else {
+			postRenderedFiles[filename] = d
+		}
+	}
+
+	return postRenderedFiles, nil
+}
+
 // renderResources renders the templates in a chart
 //
 // TODO: This function is badly in need of a refactor.
@@ -160,6 +218,14 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu
 	}
 	notes := notesBuffer.String()
 
+	// Invoke the post renderer, if using.
+	if pr != nil {
+		files, err = runPostRenderer(pr, files)
+		if err != nil {
+			return hs, b, "", fmt.Errorf("error running post renderer: %w", err)
+		}
+	}
+
 	// Sort hooks, manifests, and partials. Only hooks and manifests are returned,
 	// as partials are not used after renderer.Render. Empty manifests are also
 	// removed here.
@@ -208,8 +274,7 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu
 			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
+			// NOTE: 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])
@@ -220,13 +285,6 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu
 		}
 	}
 
-	if pr != nil {
-		b, err = pr.Run(b)
-		if err != nil {
-			return hs, b, notes, fmt.Errorf("error while running post render on files: %w", err)
-		}
-	}
-
 	return hs, b, notes, nil
 }
 
diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go
index 9436abef5..9f8cd3483 100644
--- a/pkg/action/action_test.go
+++ b/pkg/action/action_test.go
@@ -16,10 +16,12 @@ limitations under the License.
 package action
 
 import (
+	"bytes"
 	"flag"
 	"fmt"
 	"io"
 	"log/slog"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -368,3 +370,28 @@ func TestGetVersionSet(t *testing.T) {
 		t.Error("Non-existent version is reported found.")
 	}
 }
+
+type mutatingPostRenderer struct {
+	Old, New []byte
+}
+
+func (pr *mutatingPostRenderer) Run(renderedManifests *bytes.Buffer) (*bytes.Buffer, error) {
+	modifiedManifests := bytes.ReplaceAll(renderedManifests.Bytes(), pr.Old, pr.New)
+	return bytes.NewBuffer(modifiedManifests), nil
+}
+
+func TestRunPostRenderer(t *testing.T) {
+	files := map[string]string{
+		"test-cm.yaml": manifestWithHook,
+	}
+
+	postRenderer := &mutatingPostRenderer{[]byte("name: value"), []byte("name: VALUE")}
+	postRenderedFiles, err := runPostRenderer(postRenderer, files)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if postRenderedFiles["test-cm.yaml"] != strings.ReplaceAll(manifestWithHook, "name: value", "name: VALUE") {
+		t.Error("Expected test-cm to be present and mutated by the post processor")
+	}
+}