Skip to content

Commit 8f56f24

Browse files
matheuscscpmatheuscscp
authored
fix(templating): hooks conflicting with templates in post-renderers (#32049)
* fix(templating): hooks conflicting with templates in post-renderers Signed-off-by: Matheus Pimenta <matheuscscp@gmail.com> * fix(templating): allow disabling hooks from postrenderers entirely Signed-off-by: Matheus Pimenta <matheuscscp@gmail.com> --------- Signed-off-by: Matheus Pimenta <matheuscscp@gmail.com>
1 parent 29d309e commit 8f56f24

4 files changed

Lines changed: 481 additions & 41 deletions

File tree

‎pkg/action/action.go‎

Lines changed: 155 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,33 @@ const (
8888
DryRunServer DryRunStrategy = "server"
8989
)
9090

91+
// PostRenderStrategy determines how hooks and regular templates are passed
92+
// to the configured post-renderer.
93+
type PostRenderStrategy string
94+
95+
const (
96+
// PostRenderStrategyCombined sends hooks and regular templates together
97+
// as a single stream to the post-renderer. This is the default in Helm 4.
98+
PostRenderStrategyCombined PostRenderStrategy = "combined"
99+
100+
// PostRenderStrategySeparate sends hooks and regular templates to the
101+
// post-renderer in independent invocations. This avoids duplicate-resource
102+
// errors from post-renderers that de-duplicate by resource identity
103+
// (for example Kustomize) when the same resource appears in both a hook
104+
// and a regular template. Passing hooks to post-renderers was introduced
105+
// in Helm 4; Helm 3 never did so, which is why the issue only surfaces
106+
// with the Helm 4 combined default.
107+
PostRenderStrategySeparate PostRenderStrategy = "separate"
108+
109+
// PostRenderStrategyNoHooks sends only regular templates to the
110+
// post-renderer and leaves hooks untouched. This matches the Helm 3
111+
// behavior and is useful for post-renderers that declare transforms
112+
// targeting template-only resources (for example Kustomize patches
113+
// against a Deployment that exists in templates but not in hooks),
114+
// which would otherwise fail against the hook stream.
115+
PostRenderStrategyNoHooks PostRenderStrategy = "nohooks"
116+
)
117+
91118
// Configuration injects the dependencies that all actions share.
92119
type Configuration struct {
93120
// RESTClientGetter is an interface that loads Kubernetes clients.
@@ -198,7 +225,14 @@ func annotateAndMerge(files map[string]string) (string, error) {
198225

199226
// splitAndDeannotate reconstructs individual files from a merged YAML stream,
200227
// removing filename annotations and grouping documents by their original filenames.
201-
func splitAndDeannotate(postrendered string) (map[string]string, error) {
228+
// Documents without a filename annotation are assigned a synthesized name of the
229+
// form "generated-by-postrender-<fallbackPrefix>-<i>.yaml" (or
230+
// "generated-by-postrender-<i>.yaml" when fallbackPrefix is empty). The prefix
231+
// disambiguates fallback filenames across multiple post-render invocations (for
232+
// example when PostRenderStrategySeparate runs the post-renderer once per
233+
// group), so that merging results from different invocations does not collide
234+
// on the same synthetic key.
235+
func splitAndDeannotate(postrendered, fallbackPrefix string) (map[string]string, error) {
202236
manifests, err := kio.ParseAll(postrendered)
203237
if err != nil {
204238
return nil, fmt.Errorf("error parsing YAML: %w", err)
@@ -212,7 +246,11 @@ func splitAndDeannotate(postrendered string) (map[string]string, error) {
212246
}
213247
fname := meta.Annotations[filenameAnnotation]
214248
if fname == "" {
215-
fname = fmt.Sprintf("generated-by-postrender-%d.yaml", i)
249+
if fallbackPrefix == "" {
250+
fname = fmt.Sprintf("generated-by-postrender-%d.yaml", i)
251+
} else {
252+
fname = fmt.Sprintf("generated-by-postrender-%s-%d.yaml", fallbackPrefix, i)
253+
}
216254
}
217255
if err := manifest.PipeE(kyaml.ClearAnnotation(filenameAnnotation)); err != nil {
218256
return nil, fmt.Errorf("clearing filename annotation: %w", err)
@@ -237,7 +275,7 @@ func splitAndDeannotate(postrendered string) (map[string]string, error) {
237275
// TODO: As part of the refactor the duplicate code in cmd/helm/template.go should be removed
238276
//
239277
// This code has to do with writing files to disk.
240-
func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrenderer.PostRenderer, interactWithRemote, enableDNS, hideSecret bool) ([]*release.Hook, *bytes.Buffer, string, error) {
278+
func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrenderer.PostRenderer, interactWithRemote, enableDNS, hideSecret bool, postRenderStrategy PostRenderStrategy) ([]*release.Hook, *bytes.Buffer, string, error) {
241279
var hs []*release.Hook
242280
b := bytes.NewBuffer(nil)
243281

@@ -301,29 +339,122 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values,
301339
notes := notesBuffer.String()
302340

303341
if pr != nil {
304-
// We need to send files to the post-renderer before sorting and splitting
305-
// hooks from manifests. The post-renderer interface expects a stream of
306-
// manifests (similar to what tools like Kustomize and kubectl expect), whereas
307-
// the sorter uses filenames.
308-
// Here, we merge the documents into a stream, post-render them, and then split
309-
// them back into a map of filename -> content.
310-
311-
// Merge files as stream of documents for sending to post renderer
312-
merged, err := annotateAndMerge(files)
313-
if err != nil {
314-
return hs, b, notes, fmt.Errorf("error merging manifests: %w", err)
315-
}
342+
switch postRenderStrategy {
343+
case PostRenderStrategySeparate, PostRenderStrategyNoHooks:
344+
// Split hooks from manifests before post-rendering. For "separate",
345+
// hooks and templates are sent to the post-renderer as independent
346+
// streams to avoid duplicate-resource errors when the same resource
347+
// appears in both (e.g. a ServiceAccount used by a pre-install hook
348+
// that is also declared in the chart's regular templates). For
349+
// "nohooks", hooks skip the post-renderer entirely, matching the
350+
// Helm 3 behavior.
351+
sortedHooks, sortedManifests, err := releaseutil.SortManifests(files, nil, releaseutil.InstallOrder)
352+
if err != nil {
353+
for name, content := range files {
354+
if strings.TrimSpace(content) == "" {
355+
continue
356+
}
357+
fmt.Fprintf(b, "---\n# Source: %s\n%s\n", name, content)
358+
}
359+
return hs, b, "", err
360+
}
316361

317-
// Run the post renderer
318-
postRendered, err := pr.Run(bytes.NewBufferString(merged))
319-
if err != nil {
320-
return hs, b, notes, fmt.Errorf("error while running post render on files: %w", err)
321-
}
362+
// Build separate files maps for hooks and manifests.
363+
hookFiles := make(map[string]string)
364+
for _, h := range sortedHooks {
365+
if existing, ok := hookFiles[h.Path]; ok {
366+
hookFiles[h.Path] = existing + "\n---\n" + h.Manifest
367+
} else {
368+
hookFiles[h.Path] = h.Manifest
369+
}
370+
}
371+
manifestFiles := make(map[string]string)
372+
for _, m := range sortedManifests {
373+
if existing, ok := manifestFiles[m.Name]; ok {
374+
manifestFiles[m.Name] = existing + "\n---\n" + m.Content
375+
} else {
376+
manifestFiles[m.Name] = m.Content
377+
}
378+
}
322379

323-
// Use the file list and contents received from the post renderer
324-
files, err = splitAndDeannotate(postRendered.String())
325-
if err != nil {
326-
return hs, b, notes, fmt.Errorf("error while parsing post rendered output: %w", err)
380+
// Decide which groups to post-render. "nohooks" passes hooks
381+
// through untouched and only post-renders manifests.
382+
groups := []struct {
383+
name string
384+
files map[string]string
385+
postRender bool
386+
}{
387+
{"hooks", hookFiles, postRenderStrategy == PostRenderStrategySeparate},
388+
{"manifests", manifestFiles, true},
389+
}
390+
391+
files = make(map[string]string)
392+
for _, group := range groups {
393+
if len(group.files) == 0 {
394+
continue
395+
}
396+
397+
if !group.postRender {
398+
for k, v := range group.files {
399+
if existing, ok := files[k]; ok {
400+
files[k] = existing + "\n---\n" + v
401+
} else {
402+
files[k] = v
403+
}
404+
}
405+
continue
406+
}
407+
408+
merged, err := annotateAndMerge(group.files)
409+
if err != nil {
410+
return hs, b, notes, fmt.Errorf("error merging %s: %w", group.name, err)
411+
}
412+
413+
postRendered, err := pr.Run(bytes.NewBufferString(merged))
414+
if err != nil {
415+
return hs, b, notes, fmt.Errorf("error while running post render on %s: %w", group.name, err)
416+
}
417+
418+
rendered, err := splitAndDeannotate(postRendered.String(), group.name)
419+
if err != nil {
420+
return hs, b, notes, fmt.Errorf("error while parsing post rendered output for %s: %w", group.name, err)
421+
}
422+
423+
for k, v := range rendered {
424+
if existing, ok := files[k]; ok {
425+
files[k] = existing + "\n---\n" + v
426+
} else {
427+
files[k] = v
428+
}
429+
}
430+
}
431+
case PostRenderStrategyCombined, "":
432+
// We need to send files to the post-renderer before sorting and splitting
433+
// hooks from manifests. The post-renderer interface expects a stream of
434+
// manifests (similar to what tools like Kustomize and kubectl expect), whereas
435+
// the sorter uses filenames.
436+
// Here, we merge the documents into a stream, post-render them, and then split
437+
// them back into a map of filename -> content.
438+
439+
// Merge files as stream of documents for sending to post renderer
440+
merged, err := annotateAndMerge(files)
441+
if err != nil {
442+
return hs, b, notes, fmt.Errorf("error merging manifests: %w", err)
443+
}
444+
445+
// Run the post renderer
446+
postRendered, err := pr.Run(bytes.NewBufferString(merged))
447+
if err != nil {
448+
return hs, b, notes, fmt.Errorf("error while running post render on files: %w", err)
449+
}
450+
451+
// Use the file list and contents received from the post renderer
452+
files, err = splitAndDeannotate(postRendered.String(), "")
453+
if err != nil {
454+
return hs, b, notes, fmt.Errorf("error while parsing post rendered output: %w", err)
455+
}
456+
default:
457+
return hs, b, notes, fmt.Errorf("unknown post-render strategy: '%s'", postRenderStrategy)
327458
}
328459
}
329460

0 commit comments

Comments
 (0)