From 727c3a956424f0da74d7ec0757cdf6ef7e3ea297 Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Fri, 17 Apr 2026 14:26:46 +0100 Subject: [PATCH] fix(templating): allow disabling hooks from postrenderers entirely Signed-off-by: Matheus Pimenta --- pkg/action/action.go | 48 +++++++++++++++++++------- pkg/action/action_test.go | 71 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 12 deletions(-) diff --git a/pkg/action/action.go b/pkg/action/action.go index b47048f5d..8c1888144 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -105,6 +105,14 @@ const ( // in Helm 4; Helm 3 never did so, which is why the issue only surfaces // with the Helm 4 combined default. PostRenderStrategySeparate PostRenderStrategy = "separate" + + // PostRenderStrategyNoHooks sends only regular templates to the + // post-renderer and leaves hooks untouched. This matches the Helm 3 + // behavior and is useful for post-renderers that declare transforms + // targeting template-only resources (for example Kustomize patches + // against a Deployment that exists in templates but not in hooks), + // which would otherwise fail against the hook stream. + PostRenderStrategyNoHooks PostRenderStrategy = "nohooks" ) // Configuration injects the dependencies that all actions share. @@ -332,12 +340,14 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, if pr != nil { switch postRenderStrategy { - case PostRenderStrategySeparate: - // Split hooks from manifests before post-rendering so that hooks and - // templates are sent to the post-renderer as separate streams. This - // prevents duplicate-resource errors when the same resource appears in - // both hooks and templates (e.g. a ServiceAccount used by a pre-install - // hook that is also declared in the chart's regular templates). + case PostRenderStrategySeparate, PostRenderStrategyNoHooks: + // Split hooks from manifests before post-rendering. For "separate", + // hooks and templates are sent to the post-renderer as independent + // streams to avoid duplicate-resource errors when the same resource + // appears in both (e.g. a ServiceAccount used by a pre-install hook + // that is also declared in the chart's regular templates). For + // "nohooks", hooks skip the post-renderer entirely, matching the + // Helm 3 behavior. sortedHooks, sortedManifests, err := releaseutil.SortManifests(files, nil, releaseutil.InstallOrder) if err != nil { for name, content := range files { @@ -367,20 +377,34 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, } } - // Post-render hooks and manifests separately, then merge. - files = make(map[string]string) + // Decide which groups to post-render. "nohooks" passes hooks + // through untouched and only post-renders manifests. groups := []struct { - name string - files map[string]string + name string + files map[string]string + postRender bool }{ - {"hooks", hookFiles}, - {"manifests", manifestFiles}, + {"hooks", hookFiles, postRenderStrategy == PostRenderStrategySeparate}, + {"manifests", manifestFiles, true}, } + + files = make(map[string]string) for _, group := range groups { if len(group.files) == 0 { continue } + if !group.postRender { + for k, v := range group.files { + if existing, ok := files[k]; ok { + files[k] = existing + "\n---\n" + v + } else { + files[k] = v + } + } + continue + } + merged, err := annotateAndMerge(group.files) if err != nil { return hs, b, notes, fmt.Errorf("error merging %s: %w", group.name, err) diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 1c24c5b8e..54b07273b 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -2195,6 +2195,77 @@ metadata: assert.Equal(t, 1, calls, "separate strategy should skip the empty hook group and invoke the post-renderer only once") } +func TestRenderResources_PostRenderer_NoHooksSkipsHooks(t *testing.T) { + cfg := actionConfigFixture(t) + + modTime := time.Now() + ch := buildChartWithTemplates([]*common.File{ + {Name: "templates/hook.yaml", ModTime: modTime, Data: []byte(`apiVersion: v1 +kind: ConfigMap +metadata: + name: hook-cm + annotations: + "helm.sh/hook": pre-install`)}, + {Name: "templates/cm.yaml", ModTime: modTime, Data: []byte(`apiVersion: v1 +kind: ConfigMap +metadata: + name: template-cm`)}, + }) + + var inputs []string + mockPR := &mockPostRenderer{ + transform: func(content string) string { + inputs = append(inputs, content) + return content + }, + } + + hooks, manifestDoc, _, err := cfg.renderResources( + ch, nil, "test-release", "", false, false, false, + mockPR, false, false, false, PostRenderStrategyNoHooks, + ) + + assert.NoError(t, err) + assert.Len(t, inputs, 1, "nohooks strategy should invoke the post-renderer exactly once (for templates only)") + assert.NotContains(t, inputs[0], "hook-cm", "hooks must not be sent to the post-renderer") + assert.Contains(t, inputs[0], "template-cm", "templates must be sent to the post-renderer") + + // Hooks still round-trip through the release so they can execute. + require.Len(t, hooks, 1) + assert.Contains(t, hooks[0].Manifest, "hook-cm") + assert.Contains(t, manifestDoc.String(), "template-cm") +} + +func TestRenderResources_PostRenderer_NoHooksWithOnlyHooks(t *testing.T) { + cfg := actionConfigFixture(t) + + modTime := time.Now() + ch := buildChartWithTemplates([]*common.File{ + {Name: "templates/hook.yaml", ModTime: modTime, Data: []byte(`apiVersion: v1 +kind: ConfigMap +metadata: + name: hook-cm + annotations: + "helm.sh/hook": pre-install`)}, + }) + + var calls int + mockPR := &mockPostRenderer{ + transform: func(content string) string { + calls++ + return content + }, + } + + _, _, _, err := cfg.renderResources( + ch, nil, "test-release", "", false, false, false, + mockPR, false, false, false, PostRenderStrategyNoHooks, + ) + + assert.NoError(t, err) + assert.Equal(t, 0, calls, "nohooks strategy should not invoke the post-renderer when the chart only has hooks") +} + func TestRenderResources_PostRenderer_UnknownStrategyErrors(t *testing.T) { cfg := actionConfigFixture(t)