From fe80e2bb3689d4d744638ae090948cfda4412e6f Mon Sep 17 00:00:00 2001 From: yxxhero Date: Wed, 1 Apr 2026 08:56:14 +0800 Subject: [PATCH] fix: helmDefaults.postRendererArgs not passed to helm commands (#2508) The commit b5eb8793 (#1839) added template support for postRendererArgs by copying HelmDefaults.PostRendererArgs to each release and then niling out HelmDefaults.PostRendererArgs. However, the nil-out prevented the fallback in appendPostRenderArgsFlags from ever being reached, causing helmDefaults.postRendererArgs to be silently ignored when no release-level postRendererArgs was set. Remove the nil-out line so that HelmDefaults.PostRendererArgs remains available as a fallback while still supporting template expressions via the copy-to-release mechanism. Signed-off-by: yxxhero --- pkg/app/app_test.go | 150 +++++++++++++++++++++++++++ pkg/app/desired_state_file_loader.go | 1 - pkg/state/state_test.go | 136 ++++++++++++++++++++++++ 3 files changed, 286 insertions(+), 1 deletion(-) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 21ec2d4e..f37194f8 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2927,6 +2927,156 @@ releases: } } +func TestTemplate_HelmDefaultsPostRendererArgsFromDefaults(t *testing.T) { + postRenderer := "foo.sh" + files := map[string]string{ + "/path/to/helmfile.yaml": fmt.Sprintf(` +helmDefaults: + postRenderer: %s + postRendererArgs: + - --arg1 + - --arg2 + +releases: +- name: myrelease + chart: stable/mychart +`, postRenderer), + } + + var helm = &mockHelmExec{} + + var buffer bytes.Buffer + syncWriter := testhelper.NewSyncWriter(&buffer) + logger := helmexec.NewLogger(syncWriter, "debug") + + valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + if err != nil { + t.Fatalf("unexpected error creating vals runtime: %v", err) + } + + app := appWithFs(&App{ + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + helms: map[helmKey]helmexec.Interface{ + createHelmKey("helm", "default"): helm, + }, + valsRuntime: valsRuntime, + }, files) + + if err := app.Template(configImpl{}); err != nil { + t.Fatalf("%v", err) + } + + if len(helm.templated) != 1 { + t.Fatalf("expected 1 release, got %d", len(helm.templated)) + } + + flags := helm.templated[0].flags + hasPostRenderer := false + hasPostRendererArg1 := false + hasPostRendererArg2 := false + for i, f := range flags { + if f == "--post-renderer" && i+1 < len(flags) && flags[i+1] == postRenderer { + hasPostRenderer = true + } + if f == "--post-renderer-args" && i+1 < len(flags) && flags[i+1] == "--arg1" { + hasPostRendererArg1 = true + } + if f == "--post-renderer-args" && i+1 < len(flags) && flags[i+1] == "--arg2" { + hasPostRendererArg2 = true + } + } + + if !hasPostRenderer { + t.Errorf("expected --post-renderer %s in flags, got %v", postRenderer, flags) + } + if !hasPostRendererArg1 { + t.Errorf("expected --post-renderer-args --arg1 in flags, got %v", flags) + } + if !hasPostRendererArg2 { + t.Errorf("expected --post-renderer-args --arg2 in flags, got %v", flags) + } +} + +func TestTemplate_HelmDefaultsPostRendererArgsMultiDoc(t *testing.T) { + postRenderer := "foo.sh" + files := map[string]string{ + "/path/to/helmfile.yaml": fmt.Sprintf(` +helmDefaults: + postRenderer: %s + postRendererArgs: + - --arg1 + - --arg2 +--- +releases: +- name: myrelease + chart: stable/mychart +`, postRenderer), + } + + var helm = &mockHelmExec{} + + var buffer bytes.Buffer + syncWriter := testhelper.NewSyncWriter(&buffer) + logger := helmexec.NewLogger(syncWriter, "debug") + + valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + if err != nil { + t.Fatalf("unexpected error creating vals runtime: %v", err) + } + + app := appWithFs(&App{ + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + helms: map[helmKey]helmexec.Interface{ + createHelmKey("helm", "default"): helm, + }, + valsRuntime: valsRuntime, + }, files) + + if err := app.Template(configImpl{}); err != nil { + t.Fatalf("%v", err) + } + + if len(helm.templated) != 1 { + t.Fatalf("expected 1 release, got %d", len(helm.templated)) + } + + flags := helm.templated[0].flags + hasPostRenderer := false + hasPostRendererArg1 := false + hasPostRendererArg2 := false + for i, f := range flags { + if f == "--post-renderer" && i+1 < len(flags) && flags[i+1] == postRenderer { + hasPostRenderer = true + } + if f == "--post-renderer-args" && i+1 < len(flags) && flags[i+1] == "--arg1" { + hasPostRendererArg1 = true + } + if f == "--post-renderer-args" && i+1 < len(flags) && flags[i+1] == "--arg2" { + hasPostRendererArg2 = true + } + } + + if !hasPostRenderer { + t.Errorf("expected --post-renderer %s in flags, got %v", postRenderer, flags) + } + if !hasPostRendererArg1 { + t.Errorf("expected --post-renderer-args --arg1 in flags, got %v", flags) + } + if !hasPostRendererArg2 { + t.Errorf("expected --post-renderer-args --arg2 in flags, got %v", flags) + } +} + func TestApply(t *testing.T) { type fields struct { skipNeeds bool diff --git a/pkg/app/desired_state_file_loader.go b/pkg/app/desired_state_file_loader.go index 03064c47..07bedd09 100644 --- a/pkg/app/desired_state_file_loader.go +++ b/pkg/app/desired_state_file_loader.go @@ -276,7 +276,6 @@ func (ld *desiredStateLoader) load(env, overrodeEnv *environment.Environment, ba finalState.Releases[i].PostRendererArgs = finalState.HelmDefaults.PostRendererArgs } } - finalState.HelmDefaults.PostRendererArgs = nil } env = &finalState.Env diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 98c1f258..64a57c29 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -908,6 +908,74 @@ func TestHelmState_flagsForUpgrade(t *testing.T) { "--namespace", "test-namespace", }, }, + { + name: "post-renderer-args-flags-use-helmdefault", + defaults: HelmSpec{ + Verify: false, + CreateNamespace: &enable, + PostRendererArgs: []string{"--arg1", "--arg2"}, + }, + version: semver.MustParse("3.10.0"), + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Verify: &disable, + Name: "test-charts", + Namespace: "test-namespace", + CreateNamespace: &disable, + }, + want: []string{ + "--version", "0.1", + "--post-renderer-args", "--arg1", + "--post-renderer-args", "--arg2", + "--namespace", "test-namespace", + }, + }, + { + name: "post-renderer-args-flags-use-release", + defaults: HelmSpec{ + Verify: false, + CreateNamespace: &enable, + }, + version: semver.MustParse("3.10.0"), + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Verify: &disable, + Name: "test-charts", + Namespace: "test-namespace", + CreateNamespace: &disable, + PostRendererArgs: []string{"--release-arg"}, + }, + want: []string{ + "--version", "0.1", + "--post-renderer-args", "--release-arg", + "--namespace", "test-namespace", + }, + }, + { + name: "post-renderer-args-flags-use-release-prior-helmdefault", + defaults: HelmSpec{ + Verify: false, + CreateNamespace: &enable, + PostRendererArgs: []string{"--default-arg"}, + }, + version: semver.MustParse("3.10.0"), + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Verify: &disable, + Name: "test-charts", + Namespace: "test-namespace", + CreateNamespace: &disable, + PostRendererArgs: []string{"--release-arg"}, + }, + want: []string{ + "--version", "0.1", + "--post-renderer-args", "--release-arg", + "--namespace", "test-namespace", + }, + }, { name: "description-from-release", defaults: HelmSpec{ @@ -1204,6 +1272,74 @@ func TestHelmState_flagsForTemplate(t *testing.T) { "--namespace", "test-namespace", }, }, + { + name: "post-renderer-args-flags-use-helmdefault", + defaults: HelmSpec{ + Verify: false, + CreateNamespace: &enable, + PostRendererArgs: []string{"--arg1", "--arg2"}, + }, + version: semver.MustParse("3.10.0"), + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Verify: &disable, + Name: "test-charts", + Namespace: "test-namespace", + CreateNamespace: &disable, + }, + want: []string{ + "--version", "0.1", + "--post-renderer-args", "--arg1", + "--post-renderer-args", "--arg2", + "--namespace", "test-namespace", + }, + }, + { + name: "post-renderer-args-flags-use-release", + defaults: HelmSpec{ + Verify: false, + CreateNamespace: &enable, + }, + version: semver.MustParse("3.10.0"), + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Verify: &disable, + Name: "test-charts", + Namespace: "test-namespace", + CreateNamespace: &disable, + PostRendererArgs: []string{"--release-arg"}, + }, + want: []string{ + "--version", "0.1", + "--post-renderer-args", "--release-arg", + "--namespace", "test-namespace", + }, + }, + { + name: "post-renderer-args-flags-use-release-prior-helmdefault", + defaults: HelmSpec{ + Verify: false, + CreateNamespace: &enable, + PostRendererArgs: []string{"--default-arg"}, + }, + version: semver.MustParse("3.10.0"), + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Verify: &disable, + Name: "test-charts", + Namespace: "test-namespace", + CreateNamespace: &disable, + PostRendererArgs: []string{"--release-arg"}, + }, + want: []string{ + "--version", "0.1", + "--post-renderer-args", "--release-arg", + "--namespace", "test-namespace", + }, + }, { name: "kube-version-flag-should-be-used", defaults: HelmSpec{