Merge 750b8446e8 into fb90ce55af

pkg/action/action.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
+	"sigs.k8s.io/yaml"
 
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chartutil"
@@ -63,6 +64,8 @@ var (
 	errPending = errors.New("another operation (install/upgrade/rollback) is in progress")
 )
 
+const HookPostRenderAnnotation = "helm.sh/hook-post-render"
+
 // ValidName is a regular expression for resource names.
 //
 // DEPRECATED: This will be removed in Helm 4, and is no longer used here. See
@@ -101,6 +104,94 @@ type Configuration struct {
 	HookOutputFunc func(namespace, pod, container string) io.Writer
 }
 
+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
+		if content == "" || strings.HasPrefix(path.Base(filename), "_") {
+			continue
+		}
+
+		// We want to maintain compatibility with the original behavior of hooks not being
+		// passed to the post renderer.  To do so, we pass through everything that is *not*
+		// a hook, and only the hooks with a `helm.sh/hook-post-render=true` annotation.
+		//
+		// This would be a good candidate for simplification via a change in behavior in helm 4.
+		rawManifests := releaseutil.SplitManifests(content)
+		for _, manifest := range rawManifests {
+			var entry releaseutil.SimpleHead
+			if err := yaml.Unmarshal([]byte(manifest), &entry); err != nil {
+				return nil, errors.Wrapf(err, "YAML parse error on %s", filename)
+			}
+
+			// If it's a hook, we need to see an explicit hook-post-render=true annotation
+			shouldPostRender := true
+			if entry.Metadata != nil {
+				if _, ok := entry.Metadata.Annotations[release.HookAnnotation]; ok {
+					if entry.Metadata.Annotations[HookPostRenderAnnotation] != "true" {
+						shouldPostRender = false
+					}
+				}
+			}
+
+			if shouldPostRender {
+				// Buffer for the post renderer
+				_, err := fmt.Fprintf(b, "\n---\n# Source: %s\n%s", filename, manifest)
+				if err != nil {
+					return nil, err
+				}
+			} else {
+				// Append the doc to the named pseudo-file
+				if data, ok := postRenderedFiles[filename]; ok {
+					postRenderedFiles[filename] = data + "\n---\n" + manifest
+				} else {
+					postRenderedFiles[filename] = manifest
+				}
+			}
+		}
+	}
+
+	// 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.
@@ -166,6 +257,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, "", errors.Wrap(err, "error while running post renderer on files")
+		}
+	}
+
 	// 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.
@@ -214,8 +313,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
+			// NOTE: Output dir is only used by `helm template`. In the next major
-			// 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])
@@ -226,13 +324,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, errors.Wrap(err, "error while running post render on files")
-		}
-	}
-
 	return hs, b, notes, nil
 }
 

pkg/action/action_test.go

@@ -16,9 +16,11 @@ limitations under the License.
 package action
 
 import (
+	"bytes"
 	"flag"
 	"fmt"
 	"io"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -71,6 +73,15 @@ metadata:
 data:
   name: value`
 
+var manifestWithPostRenderHook = `kind: ConfigMap
+metadata:
+  name: pr-test-cm
+  annotations:
+    "helm.sh/hook": post-install,pre-delete,post-upgrade
+    "helm.sh/hook-post-render": "true"
+data:
+  name: value`
+
 var manifestWithTestHook = `kind: Pod
   metadata:
 	name: finding-nemo,
@@ -366,3 +377,32 @@ 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{
+		"pr-test-cm.yaml": manifestWithPostRenderHook,
+		"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["pr-test-cm.yaml"] != strings.ReplaceAll(manifestWithPostRenderHook, "name: value", "name: VALUE") {
+		t.Error("Expected pr-test-cm to be present and mutated by the post processor")
+	}
+	if postRenderedFiles["test-cm.yaml"] != manifestWithHook {
+		t.Error("Expected test-cm to be present and unmodified")
+	}
+}