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 <aiopsclub@163.com>
* 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 <aiopsclub@163.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
This commit is contained in:
parent
6faa477290
commit
c584c0e07f
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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{
|
||||
|
|
|
|||
Loading…
Reference in New Issue