From 750b8446e8e4f9334e2d675642e2059c14c8bcba Mon Sep 17 00:00:00 2001 From: Dan Walters Date: Sat, 19 Apr 2025 20:07:43 -0500 Subject: [PATCH] fix: send hooks through the post-renderer This is a little awkward, but maintains full backwards compatibility by requiring a helm.sh/hook-post-render=true annotation to be specified on any hooks that should be passed through the post renderer. fixes #7891 Signed-off-by: Dan Walters --- pkg/action/action.go | 109 ++++++++++++++++++++++++++++++++++---- pkg/action/action_test.go | 40 ++++++++++++++ 2 files changed, 140 insertions(+), 9 deletions(-) diff --git a/pkg/action/action.go b/pkg/action/action.go index 9aaf64ca4..11cf1d597 100644 --- a/pkg/action/action.go +++ b/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 - // 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]) @@ -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 } diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 94163749e..c2b5c4938 100644 --- a/pkg/action/action_test.go +++ b/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") + } +}