From c584c0e07fc1242f10493dfd9d64a618808ebc4b Mon Sep 17 00:00:00 2001 From: yxxhero <11087727+yxxhero@users.noreply.github.com> Date: Thu, 16 Apr 2026 11:08:15 +0800 Subject: [PATCH] fix: helmDefaults.postRendererArgs not passed to helm commands (#2510) * 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 * fix: use helm4-compatible postRenderer value in app tests Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/17a1a3c2-e104-49c5-a607-1e81a7b9de06 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> * fix: remove postRendererArgs copy loop so CLI flags can override helmDefaults Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/c85f0436-2346-402e-8ad6-e08a4fff7413 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> * test: add missing CLI>helmDefault and release>CLI postRendererArgs priority tests Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/37cf3613-a4e1-4eac-b6bc-002761256d31 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> * fix: clarify comment wording in app_test.go Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/37cf3613-a4e1-4eac-b6bc-002761256d31 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> * refactor: extract newPostRendererTestApp and hasFlagWithValue helpers in app_test.go Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/faf50bca-33b2-4eb4-8ef1-49f470dfa5b7 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/app/app_test.go | 218 ++++++++++++++++++++++++- pkg/app/desired_state_file_loader.go | 8 - pkg/state/state_test.go | 236 +++++++++++++++++++++++++++ 3 files changed, 452 insertions(+), 10 deletions(-) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 73697915..40de4b3b 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2283,6 +2283,8 @@ type configImpl struct { enforceNeedsAreInstalled bool skipCharts bool kubeVersion string + postRenderer string + postRendererArgs []string } func (c configImpl) Selectors() []string { @@ -2370,11 +2372,11 @@ func (c configImpl) SkipCharts() bool { } func (c configImpl) PostRenderer() string { - return "" + return c.postRenderer } func (c configImpl) PostRendererArgs() []string { - return nil + return c.postRendererArgs } func (c configImpl) KubeVersion() string { @@ -2988,6 +2990,218 @@ releases: } } +// newPostRendererTestApp creates an App and mockHelmExec wired together from a helmfile YAML content map, +// suitable for testing post-renderer flag propagation. +func newPostRendererTestApp(t *testing.T, files map[string]string) (*App, *mockHelmExec) { + t.Helper() + 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) + return app, helm +} + +// hasFlagWithValue reports whether flags contains "--flagName value" as adjacent entries. +func hasFlagWithValue(flags []string, flagName, value string) bool { + for i, f := range flags { + if f == flagName && i+1 < len(flags) && flags[i+1] == value { + return true + } + } + return false +} + +func TestTemplate_HelmDefaultsPostRendererArgs(t *testing.T) { + tests := []struct { + name string + content string + }{ + { + name: "single-doc", + content: ` +helmDefaults: + postRenderer: foo + postRendererArgs: + - --arg1 + - --arg2 + +releases: +- name: myrelease + chart: stable/mychart +`, + }, + { + name: "multi-doc", + content: ` +helmDefaults: + postRenderer: foo + postRendererArgs: + - --arg1 + - --arg2 +--- +releases: +- name: myrelease + chart: stable/mychart +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + app, helm := newPostRendererTestApp(t, map[string]string{"/path/to/helmfile.yaml": tt.content}) + + 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 + if !hasFlagWithValue(flags, "--post-renderer", "foo") { + t.Errorf("expected --post-renderer foo in flags, got %v", flags) + } + if !hasFlagWithValue(flags, "--post-renderer-args", "--arg1") { + t.Errorf("expected --post-renderer-args --arg1 in flags, got %v", flags) + } + if !hasFlagWithValue(flags, "--post-renderer-args", "--arg2") { + t.Errorf("expected --post-renderer-args --arg2 in flags, got %v", flags) + } + }) + } +} + +func TestTemplate_CLIPostRendererArgsOverridesHelmDefaults(t *testing.T) { + tests := []struct { + name string + content string + }{ + { + name: "single-doc", + content: ` +helmDefaults: + postRenderer: foo + postRendererArgs: + - --default-arg + +releases: +- name: myrelease + chart: stable/mychart +`, + }, + { + name: "multi-doc", + content: ` +helmDefaults: + postRenderer: foo + postRendererArgs: + - --default-arg +--- +releases: +- name: myrelease + chart: stable/mychart +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + app, helm := newPostRendererTestApp(t, map[string]string{"/path/to/helmfile.yaml": tt.content}) + + // CLI provides --post-renderer-args which should override helmDefaults.postRendererArgs + if err := app.Template(configImpl{postRendererArgs: []string{"--cli-arg"}}); 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 + if !hasFlagWithValue(flags, "--post-renderer-args", "--cli-arg") { + t.Errorf("expected --post-renderer-args --cli-arg in flags (CLI should override helmDefaults), got %v", flags) + } + if hasFlagWithValue(flags, "--post-renderer-args", "--default-arg") { + t.Errorf("unexpected --post-renderer-args --default-arg in flags (CLI should override helmDefaults), got %v", flags) + } + }) + } +} + +func TestTemplate_ReleasePostRendererArgsOverridesCLI(t *testing.T) { + tests := []struct { + name string + content string + }{ + { + name: "single-doc", + content: ` +helmDefaults: + postRenderer: foo + +releases: +- name: myrelease + chart: stable/mychart + postRendererArgs: + - --release-arg +`, + }, + { + name: "multi-doc", + content: ` +helmDefaults: + postRenderer: foo +--- +releases: +- name: myrelease + chart: stable/mychart + postRendererArgs: + - --release-arg +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + app, helm := newPostRendererTestApp(t, map[string]string{"/path/to/helmfile.yaml": tt.content}) + + // Release explicitly sets postRendererArgs, CLI also provides --post-renderer-args flag; release should win + if err := app.Template(configImpl{postRendererArgs: []string{"--cli-arg"}}); 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 + if !hasFlagWithValue(flags, "--post-renderer-args", "--release-arg") { + t.Errorf("expected --post-renderer-args --release-arg in flags (release should override CLI), got %v", flags) + } + if hasFlagWithValue(flags, "--post-renderer-args", "--cli-arg") { + t.Errorf("unexpected --post-renderer-args --cli-arg in flags (release should override CLI), 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 2f2d4d03..8ffe3942 100644 --- a/pkg/app/desired_state_file_loader.go +++ b/pkg/app/desired_state_file_loader.go @@ -279,14 +279,6 @@ func (ld *desiredStateLoader) load(env, overrodeEnv *environment.Environment, ba finalState.RenderedValues = currentState.RenderedValues } - if len(finalState.HelmDefaults.PostRendererArgs) > 0 { - for i := range finalState.Releases { - if len(finalState.Releases[i].PostRendererArgs) == 0 { - finalState.Releases[i].PostRendererArgs = finalState.HelmDefaults.PostRendererArgs - } - } - finalState.HelmDefaults.PostRendererArgs = nil - } env = &finalState.Env ld.logger.Debugf("merged environment: %v", env) diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 98c1f258..0d10624a 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -908,6 +908,124 @@ 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: "post-renderer-args-cli-overrides-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, + }, + syncOpts: &SyncOpts{ + PostRendererArgs: []string{"--cli-arg"}, + }, + want: []string{ + "--version", "0.1", + "--post-renderer-args", "--cli-arg", + "--namespace", "test-namespace", + }, + }, + { + name: "post-renderer-args-release-overrides-cli", + 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"}, + }, + syncOpts: &SyncOpts{ + PostRendererArgs: []string{"--cli-arg"}, + }, + want: []string{ + "--version", "0.1", + "--post-renderer-args", "--release-arg", + "--namespace", "test-namespace", + }, + }, { name: "description-from-release", defaults: HelmSpec{ @@ -1204,6 +1322,124 @@ 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: "post-renderer-args-cli-overrides-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, + }, + templateOpts: TemplateOpts{ + PostRendererArgs: []string{"--cli-arg"}, + }, + want: []string{ + "--version", "0.1", + "--post-renderer-args", "--cli-arg", + "--namespace", "test-namespace", + }, + }, + { + name: "post-renderer-args-release-overrides-cli", + 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"}, + }, + templateOpts: TemplateOpts{ + PostRendererArgs: []string{"--cli-arg"}, + }, + want: []string{ + "--version", "0.1", + "--post-renderer-args", "--release-arg", + "--namespace", "test-namespace", + }, + }, { name: "kube-version-flag-should-be-used", defaults: HelmSpec{