From 9fa0529304e8e0838bf63412f2c1ab4ebdc91b6a Mon Sep 17 00:00:00 2001 From: yxxhero <11087727+yxxhero@users.noreply.github.com> Date: Tue, 28 Apr 2026 09:01:48 +0800 Subject: [PATCH] fix: apply post-renderer to output-dir-template output (#2531) * fix: apply post-renderer to output-dir-template output When --output-dir and --post-renderer are both passed to helm template, Helm writes pre-post-renderer content to files and sends post-renderer output to stdout. This workaround strips --output-dir from helm flags, captures the post-renderer-processed stdout, and writes it to the output directory. Fixes #2515 Signed-off-by: yxxhero * test: add integration test for issue-2515 (post-renderer with output-dir-template) Verifies that --post-renderer output is written to files when --output-dir-template is set, instead of pre-renderer content. Signed-off-by: yxxhero * fix: address review comments - correct HasPrefix args, fix output dir structure, fix test mock init Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/33d92423-fc47-4080-8307-5af9b16dd9c6 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> * fix: wrap file operation errors with context in post-renderer workaround Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/33d92423-fc47-4080-8307-5af9b16dd9c6 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> * fix: correct chart path and use absolute case dir path in integration test Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/43b7a794-1e7b-4577-8829-deb544a1a105 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> * fix: restrict --output-dir + --post-renderer workaround to Helm 3 only Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/229b14e2-b1ad-4f19-bd00-b8f7821383cd Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> * fix: clean up stale templates dir on re-runs in Helm 3 post-renderer workaround Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/f6c66284-8eca-4db3-8711-c9b6d3a9c179 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> * fix: detect --post-renderer= form and use targeted file cleanup Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/8c9e4af4-84ae-4cbd-bc0a-8fcd9adddaed Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> * feat: add Helm 4 post-renderer plugin and enable Helm 4 issue-2515 integration test Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/3da2949c-a9d6-4e16-9b4a-a7e241080089 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> * fix: search recursively for YAML files in Helm 4 output-dir integration test Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/c5d33143-f611-40db-b73a-e5189d944ffd Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> * fix: limit find depth and truncate log in Helm 4 integration test fallback message Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/c5d33143-f611-40db-b73a-e5189d944ffd Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --------- Signed-off-by: yxxhero Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> --- pkg/helmexec/exec.go | 68 ++++++++++++++++-- pkg/helmexec/exec_test.go | 72 +++++++++++++++++-- test/integration/run.sh | 1 + test/integration/test-cases/issue-2515.sh | 62 ++++++++++++++++ .../test-cases/issue-2515/input/filter.bash | 2 + .../input/helm-plugin-filter/filter.sh | 6 ++ .../input/helm-plugin-filter/plugin.yaml | 8 +++ .../test-cases/issue-2515/input/helmfile.yaml | 13 ++++ 8 files changed, 224 insertions(+), 8 deletions(-) create mode 100644 test/integration/test-cases/issue-2515.sh create mode 100755 test/integration/test-cases/issue-2515/input/filter.bash create mode 100755 test/integration/test-cases/issue-2515/input/helm-plugin-filter/filter.sh create mode 100644 test/integration/test-cases/issue-2515/input/helm-plugin-filter/plugin.yaml create mode 100644 test/integration/test-cases/issue-2515/input/helmfile.yaml diff --git a/pkg/helmexec/exec.go b/pkg/helmexec/exec.go index cdfa90a8..a2153576 100644 --- a/pkg/helmexec/exec.go +++ b/pkg/helmexec/exec.go @@ -646,17 +646,77 @@ func (helm *execer) TemplateRelease(name string, chart string, flags ...string) helm.logger.Infof("Templating release=%v, chart=%v", name, redactedURL(chart)) args := []string{"template", name, chart} - out, err := helm.exec(append(args, flags...), map[string]string{}, nil) - var outputToFile bool + var hasPostRenderer bool for _, f := range flags { - if strings.HasPrefix("--output-dir", f) { + if f == "--output-dir" || strings.HasPrefix(f, "--output-dir=") { outputToFile = true - break + } + if f == "--post-renderer" || strings.HasPrefix(f, "--post-renderer=") { + hasPostRenderer = true } } + if outputToFile && hasPostRenderer && helm.IsHelm3() { + // Helm 3 does not apply --post-renderer to files written by --output-dir. + // It writes pre-post-renderer content to files and sends post-renderer output to stdout. + // Helm 4 handles this correctly, so the workaround is only needed for Helm 3. + // Workaround: run without --output-dir, capture stdout (with post-renderer applied), + // and write the output to the output directory ourselves. + var outputDir string + filteredFlags := make([]string, 0, len(flags)) + for i := 0; i < len(flags); i++ { + if flags[i] == "--output-dir" && i+1 < len(flags) { + outputDir = flags[i+1] + i++ + continue + } + if strings.HasPrefix(flags[i], "--output-dir=") { + outputDir = strings.TrimPrefix(flags[i], "--output-dir=") + continue + } + filteredFlags = append(filteredFlags, flags[i]) + } + + if outputDir == "" { + return fmt.Errorf("output dir not found for template command") + } + + out, err := helm.exec(append(args, filteredFlags...), map[string]string{}, nil) + if err != nil { + return err + } + + templatesDir := filepath.Join(outputDir, "templates") + legacyOutputPath := filepath.Join(outputDir, name+".yaml") + outputPath := filepath.Join(templatesDir, name+".yaml") + + if removeErr := os.Remove(legacyOutputPath); removeErr != nil && !os.IsNotExist(removeErr) { + return fmt.Errorf("failed to remove legacy output file %s: %w", legacyOutputPath, removeErr) + } + + // Remove only the specific file written by the previous run to avoid clobbering + // unrelated files in a shared output directory. + if removeErr := os.Remove(outputPath); removeErr != nil && !os.IsNotExist(removeErr) { + return fmt.Errorf("failed to remove stale output file %s: %w", outputPath, removeErr) + } + + if len(out) > 0 { + if mkdirErr := os.MkdirAll(templatesDir, 0755); mkdirErr != nil { + return fmt.Errorf("failed to create templates directory %s: %w", templatesDir, mkdirErr) + } + + if writeErr := os.WriteFile(outputPath, append(out, '\n'), 0644); writeErr != nil { + return fmt.Errorf("failed to write output file %s: %w", outputPath, writeErr) + } + helm.logger.Debugf("Wrote post-renderer output to %s", outputPath) + } + return nil + } + + out, err := helm.exec(append(args, flags...), map[string]string{}, nil) + if outputToFile { // With --output-dir is passed to helm-template, // we can safely direct all the logs from it to our logger. diff --git a/pkg/helmexec/exec_test.go b/pkg/helmexec/exec_test.go index 9644c21a..2d4dc577 100644 --- a/pkg/helmexec/exec_test.go +++ b/pkg/helmexec/exec_test.go @@ -21,8 +21,9 @@ import ( // Mocking the command-line runner type mockRunner struct { - output []byte - err error + output []byte + versionOutput []byte // if set, returned for "helm version --short" probe; overrides default Helm 4 fallback + err error } func (mock *mockRunner) ExecuteStdIn(cmd string, args []string, env map[string]string, stdin io.Reader) ([]byte, error) { @@ -30,8 +31,13 @@ func (mock *mockRunner) ExecuteStdIn(cmd string, args []string, env map[string]s } func (mock *mockRunner) Execute(cmd string, args []string, env map[string]string, enableLiveOutput bool) ([]byte, error) { - if len(mock.output) == 0 && strings.Join(args, " ") == "version --short" { - return []byte("v4.0.1+g12500dd"), nil + if strings.Join(args, " ") == "version --short" { + if mock.versionOutput != nil { + return mock.versionOutput, nil + } + if len(mock.output) == 0 { + return []byte("v4.0.1+g12500dd"), nil + } } return mock.output, mock.err } @@ -1255,6 +1261,64 @@ exec: helm --kubeconfig config --kube-context dev template release https://examp } } +func Test_Template_PostRendererWithOutputDir(t *testing.T) { + tests := []struct { + name string + postRendererFlag string + }{ + {"separate flags", "--post-renderer"}, + {"combined flag", "--post-renderer=/bin/echo"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + var buffer bytes.Buffer + logger := NewLogger(&buffer, "debug") + + // Use Helm 3 version for the version probe so the Helm 3 workaround is applied. + // The workaround is not needed for Helm 4, which natively applies --post-renderer to --output-dir output. + runner := &mockRunner{versionOutput: []byte("v3.20.0")} + helm, err := New("helm", HelmExecOptions{}, logger, "config", "dev", runner) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + runner.output = []byte("apiVersion: v1\nkind: Namespace\n") + + var flags []string + if tt.postRendererFlag == "--post-renderer" { + flags = []string{"--post-renderer", "/bin/echo", "--output-dir", tmpDir, "--values", "file.yml"} + } else { + flags = []string{tt.postRendererFlag, "--output-dir", tmpDir, "--values", "file.yml"} + } + + err = helm.TemplateRelease("myrelease", "path/to/chart", flags...) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + outputPath := filepath.Join(tmpDir, "templates", "myrelease.yaml") + data, err := os.ReadFile(outputPath) + if err != nil { + t.Fatalf("expected output file %s to exist: %v", outputPath, err) + } + + expected := "apiVersion: v1\nkind: Namespace\n\n" + if string(data) != expected { + t.Errorf("output file content:\nactual=%q\nexpect=%q", string(data), expected) + } + + outputLog := buffer.String() + if strings.Contains(outputLog, "--output-dir") { + t.Errorf("helm should NOT have been called with --output-dir, got: %s", outputLog) + } + if !strings.Contains(outputLog, "--post-renderer") { + t.Errorf("helm should have been called with --post-renderer, got: %s", outputLog) + } + }) + } +} + func Test_IsHelm3(t *testing.T) { helm3Runner := mockRunner{output: []byte("v3.0.0+ge29ce2a\n")} helm, err := New("helm", HelmExecOptions{}, NewLogger(os.Stdout, "info"), "", "dev", &helm3Runner) diff --git a/test/integration/run.sh b/test/integration/run.sh index 9fb6ecac..14d4b435 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -115,6 +115,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/yaml-overwrite.sh . ${dir}/test-cases/chart-needs.sh . ${dir}/test-cases/postrender.sh +. ${dir}/test-cases/issue-2515.sh . ${dir}/test-cases/chartify.sh . ${dir}/test-cases/deps-mr-1011.sh . ${dir}/test-cases/deps-kustomization-i-1402.sh diff --git a/test/integration/test-cases/issue-2515.sh b/test/integration/test-cases/issue-2515.sh new file mode 100644 index 00000000..c1919c37 --- /dev/null +++ b/test/integration/test-cases/issue-2515.sh @@ -0,0 +1,62 @@ +issue_2515_case_dir="$(cd "${cases_dir}/issue-2515" && pwd)" +issue_2515_tmp=$(mktemp -d) + +# Determine the post-renderer argument. +# Helm 3 accepts an executable script; Helm 4 requires a plugin name. +if [ "${HELMFILE_HELM4}" = "1" ]; then + test_start "issue-2515 post-renderer with output-dir-template (Helm 4)" + info "Installing filter post-renderer plugin for Helm 4" + ${helm} plugin uninstall filter &>/dev/null || true + ${helm} plugin install ${issue_2515_case_dir}/input/helm-plugin-filter ${PLUGIN_INSTALL_FLAGS} || fail "Failed to install filter plugin" + issue_2515_postrenderer_arg="filter" +else + test_start "issue-2515 post-renderer with output-dir-template" + issue_2515_postrenderer_arg="${issue_2515_case_dir}/input/filter.bash" +fi + +info "Testing that --post-renderer output is written to files when --output-dir-template is set" + +issue_2515_output_dir="${issue_2515_tmp}/output" + +${helmfile} -f ${issue_2515_case_dir}/input/helmfile.yaml \ + template \ + --post-renderer ${issue_2515_postrenderer_arg} \ + --output-dir-template "${issue_2515_output_dir}/{{.Release.Name}}" \ + &> ${issue_2515_tmp}/log || fail "helmfile template should not fail" + +if [ "${HELMFILE_HELM4}" = "1" ]; then + # Helm 4 natively applies --post-renderer to --output-dir output. + # The directory structure may differ from Helm 3 (no guaranteed templates/ subdir), + # so search recursively for any YAML file. Fall back to stdout (log) if no files written. + issue_2515_output_file=$(find "${issue_2515_output_dir}" -maxdepth 5 -type f \( -name '*.yaml' -o -name '*.yml' \) 2>/dev/null | head -n 1) + if [ -z "${issue_2515_output_file}" ]; then + # Helm 4 may write post-rendered output to stdout rather than files + issue_2515_output_file="${issue_2515_tmp}/log" + if ! grep -q "postrendered" "${issue_2515_output_file}"; then + fail "Expected post-rendered YAML (namespace postrendered) in output files under ${issue_2515_output_dir} or stdout. Dir: $(find ${issue_2515_output_dir} 2>/dev/null || echo 'not found'). Log (last 50 lines): $(tail -50 ${issue_2515_output_file})" + fi + fi +else + issue_2515_templates_dir="${issue_2515_output_dir}/issue-2515/templates" + if [ ! -d "${issue_2515_templates_dir}" ]; then + fail "Expected templates directory ${issue_2515_templates_dir} to exist" + fi + issue_2515_output_file=$(find "${issue_2515_templates_dir}" -type f \( -name '*.yaml' -o -name '*.yml' \) | head -n 1) + if [ -z "${issue_2515_output_file}" ]; then + fail "Expected rendered YAML file under ${issue_2515_templates_dir}" + fi +fi + +if grep -q "original-cm" "${issue_2515_output_file}"; then + fail "Output should contain post-renderer output (Namespace), not original templates (original-cm). File contents: $(cat ${issue_2515_output_file})" +fi + +if ! grep -q "postrendered" "${issue_2515_output_file}"; then + fail "Output should contain post-renderer content (namespace postrendered). File contents: $(cat ${issue_2515_output_file})" +fi + +if [ "${HELMFILE_HELM4}" = "1" ]; then + test_pass "issue-2515 post-renderer with output-dir-template (Helm 4)" +else + test_pass "issue-2515 post-renderer with output-dir-template" +fi diff --git a/test/integration/test-cases/issue-2515/input/filter.bash b/test/integration/test-cases/issue-2515/input/filter.bash new file mode 100755 index 00000000..b069f429 --- /dev/null +++ b/test/integration/test-cases/issue-2515/input/filter.bash @@ -0,0 +1,2 @@ +#!/usr/bin/env bash +printf -- "---\napiVersion: v1\nkind: Namespace\nmetadata:\n name: postrendered\n" diff --git a/test/integration/test-cases/issue-2515/input/helm-plugin-filter/filter.sh b/test/integration/test-cases/issue-2515/input/helm-plugin-filter/filter.sh new file mode 100755 index 00000000..172b0694 --- /dev/null +++ b/test/integration/test-cases/issue-2515/input/helm-plugin-filter/filter.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash +# Discard stdin (pre-rendered content) and output a fixed Namespace resource. +# This verifies that the post-renderer output — not the original templates — is +# written to the output directory. +cat > /dev/null +printf -- "---\napiVersion: v1\nkind: Namespace\nmetadata:\n name: postrendered\n" diff --git a/test/integration/test-cases/issue-2515/input/helm-plugin-filter/plugin.yaml b/test/integration/test-cases/issue-2515/input/helm-plugin-filter/plugin.yaml new file mode 100644 index 00000000..59517564 --- /dev/null +++ b/test/integration/test-cases/issue-2515/input/helm-plugin-filter/plugin.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +type: postrenderer/v1 +name: filter +version: 0.1.0 +runtime: subprocess +runtimeConfig: + platformCommand: + - command: ${HELM_PLUGIN_DIR}/filter.sh diff --git a/test/integration/test-cases/issue-2515/input/helmfile.yaml b/test/integration/test-cases/issue-2515/input/helmfile.yaml new file mode 100644 index 00000000..1940ca05 --- /dev/null +++ b/test/integration/test-cases/issue-2515/input/helmfile.yaml @@ -0,0 +1,13 @@ +releases: +- name: issue-2515 + chart: ../../../charts/raw + values: + - templates: + - | + apiVersion: v1 + kind: ConfigMap + metadata: + name: original-cm + namespace: {{ .Release.Namespace }} + data: + key: value