fix(templating): allow disabling hooks from postrenderers entirely

Signed-off-by: Matheus Pimenta <matheuscscp@gmail.com>
pull/32049/head
Matheus Pimenta 6 months ago
parent d52a872b97
commit 727c3a9564
No known key found for this signature in database
GPG Key ID: 4639F038AE28FBFF

@ -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)

@ -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)

Loading…
Cancel
Save