From c6e74b4394dce893bf2ccb2692d0369a5451452f Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 26 Apr 2026 11:56:43 +0800 Subject: [PATCH 1/8] fix: include release values in .Values for jsonPatches/strategicMergePatches/transformers gotmpl rendering Before this fix, .Values in patch template files only contained environment values, not the release's own values. This meant references like {{ .Values.ingress.enabled }} would fail when ingress.enabled was set in the release's values: file rather than environment values. Now patch gotmpl files see .Values as merged(environment values, release values), matching user expectations that values defined in the release should be accessible in conditional patches. Fixes #1904 Signed-off-by: yxxhero --- pkg/state/create_test.go | 164 +++++++++++++++++++++++++++++++++++++ pkg/state/helmx.go | 15 +++- pkg/state/state.go | 171 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 347 insertions(+), 3 deletions(-) diff --git a/pkg/state/create_test.go b/pkg/state/create_test.go index aa6b704e..2c91ef65 100644 --- a/pkg/state/create_test.go +++ b/pkg/state/create_test.go @@ -7,6 +7,7 @@ import ( "testing" "dario.cat/mergo" + "github.com/helmfile/vals" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap" @@ -932,3 +933,166 @@ releases: }) } } + +func TestMergedReleaseTemplateData_IncludesReleaseValues(t *testing.T) { + logger := zaptest.NewLogger(t).Sugar() + testValsRuntime, _ := vals.New(vals.Options{CacheSize: 32}) + + yamlFile := "/example/path/to/helmfile.yaml" + yamlContent := []byte(`environments: + default: + values: + - env.yaml + +releases: +- name: myrelease + chart: mychart + values: + - values.yaml +`) + + envYamlFile := "/example/path/to/env.yaml" + envYamlContent := []byte(`envKey: envValue`) + + valuesFile := "/example/path/to/values.yaml" + valuesContent := []byte(`ingress: + enabled: true + host: example.com`) + + testFs := testhelper.NewTestFs(map[string]string{ + envYamlFile: string(envYamlContent), + valuesFile: string(valuesContent), + }) + testFs.Cwd = "/example/path/to" + + r := remote.NewRemote(logger, testFs.Cwd, testFs.ToFileSystem()) + env := environment.Environment{ + Name: "default", + } + state, err := NewCreator(logger, testFs.ToFileSystem(), testValsRuntime, nil, "", "", r, false, ""). + ParseAndLoad(yamlContent, filepath.Dir(yamlFile), yamlFile, "default", true, true, true, &env, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + release := state.Releases[0] + + templateData, err := state.mergedReleaseTemplateData(&release) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + ingress, ok := templateData.Values["ingress"] + if !ok { + t.Fatalf("expected .Values to contain 'ingress' key from release values") + } + ingressMap, ok := ingress.(map[string]any) + if !ok { + t.Fatalf("expected ingress to be a map, got %T", ingress) + } + if ingressMap["enabled"] != true { + t.Errorf("expected ingress.enabled to be true, got %v", ingressMap["enabled"]) + } + if ingressMap["host"] != "example.com" { + t.Errorf("expected ingress.host to be 'example.com', got %v", ingressMap["host"]) + } + + if templateData.Values["envKey"] != "envValue" { + t.Errorf("expected envKey to be 'envValue', got %v", templateData.Values["envKey"]) + } +} + +func TestMergedReleaseTemplateData_ReleaseValuesOverrideEnvValues(t *testing.T) { + logger := zaptest.NewLogger(t).Sugar() + testValsRuntime, _ := vals.New(vals.Options{CacheSize: 32}) + + yamlFile := "/example/path/to/helmfile.yaml" + yamlContent := []byte(`environments: + default: + values: + - env.yaml + +releases: +- name: myrelease + chart: mychart + values: + - values.yaml +`) + + envYamlFile := "/example/path/to/env.yaml" + envYamlContent := []byte(`replicaCount: 1`) + + valuesFile := "/example/path/to/values.yaml" + valuesContent := []byte(`replicaCount: 3`) + + testFs := testhelper.NewTestFs(map[string]string{ + envYamlFile: string(envYamlContent), + valuesFile: string(valuesContent), + }) + testFs.Cwd = "/example/path/to" + + r := remote.NewRemote(logger, testFs.Cwd, testFs.ToFileSystem()) + env := environment.Environment{ + Name: "default", + } + state, err := NewCreator(logger, testFs.ToFileSystem(), testValsRuntime, nil, "", "", r, false, ""). + ParseAndLoad(yamlContent, filepath.Dir(yamlFile), yamlFile, "default", true, true, true, &env, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + release := state.Releases[0] + + templateData, err := state.mergedReleaseTemplateData(&release) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if templateData.Values["replicaCount"] != 3 { + t.Errorf("expected replicaCount to be 3 (release value overriding env), got %v", templateData.Values["replicaCount"]) + } +} + +func TestMergedReleaseTemplateData_InlineValues(t *testing.T) { + logger := zaptest.NewLogger(t).Sugar() + testValsRuntime, _ := vals.New(vals.Options{CacheSize: 32}) + + yamlFile := "/example/path/to/helmfile.yaml" + yamlContent := []byte(`releases: +- name: myrelease + chart: mychart + values: + - ingress: + enabled: true + host: example.com +`) + + testFs := testhelper.NewTestFs(map[string]string{}) + testFs.Cwd = "/example/path/to" + + r := remote.NewRemote(logger, testFs.Cwd, testFs.ToFileSystem()) + state, err := NewCreator(logger, testFs.ToFileSystem(), testValsRuntime, nil, "", "", r, false, ""). + ParseAndLoad(yamlContent, filepath.Dir(yamlFile), yamlFile, "default", true, true, true, nil, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + release := state.Releases[0] + + templateData, err := state.mergedReleaseTemplateData(&release) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + ingress, ok := templateData.Values["ingress"] + if !ok { + t.Fatalf("expected .Values to contain 'ingress' key from inline release values") + } + ingressMap, ok := ingress.(map[string]any) + if !ok { + t.Fatalf("expected ingress to be a map, got %T", ingress) + } + if ingressMap["enabled"] != true { + t.Errorf("expected ingress.enabled to be true, got %v", ingressMap["enabled"]) + } +} diff --git a/pkg/state/helmx.go b/pkg/state/helmx.go index b337cd30..f26a2f41 100644 --- a/pkg/state/helmx.go +++ b/pkg/state/helmx.go @@ -404,9 +404,18 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp shouldRun = true } + var patchTemplateData *releaseTemplateData + if len(release.JSONPatches) > 0 || len(release.StrategicMergePatches) > 0 || len(release.Transformers) > 0 { + td, err := st.mergedReleaseTemplateData(release) + if err != nil { + return nil, clean, fmt.Errorf("failed to compute merged release values for patch rendering: %v", err) + } + patchTemplateData = &td + } + jsonPatches := release.JSONPatches if len(jsonPatches) > 0 { - generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, jsonPatches) + generatedFiles, err := st.generateTemporaryReleaseValuesFilesWithData(release, jsonPatches, *patchTemplateData) if err != nil { return nil, clean, err } @@ -420,7 +429,7 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp strategicMergePatches := release.StrategicMergePatches if len(strategicMergePatches) > 0 { - generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, strategicMergePatches) + generatedFiles, err := st.generateTemporaryReleaseValuesFilesWithData(release, strategicMergePatches, *patchTemplateData) if err != nil { return nil, clean, err } @@ -434,7 +443,7 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp transformers := release.Transformers if len(transformers) > 0 { - generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, transformers) + generatedFiles, err := st.generateTemporaryReleaseValuesFilesWithData(release, transformers, *patchTemplateData) if err != nil { return nil, clean, err } diff --git a/pkg/state/state.go b/pkg/state/state.go index f51a0dd1..b00e79a6 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -36,6 +36,7 @@ import ( "github.com/helmfile/helmfile/pkg/event" "github.com/helmfile/helmfile/pkg/filesystem" "github.com/helmfile/helmfile/pkg/helmexec" + "github.com/helmfile/helmfile/pkg/maputil" "github.com/helmfile/helmfile/pkg/remote" "github.com/helmfile/helmfile/pkg/tmpl" "github.com/helmfile/helmfile/pkg/yaml" @@ -3916,6 +3917,176 @@ func (st *HelmState) newReleaseTemplateData(release *ReleaseSpec) releaseTemplat return templateData } +func (st *HelmState) mergedReleaseTemplateData(release *ReleaseSpec) (releaseTemplateData, error) { + releaseValues, err := st.resolveReleaseValues(release) + if err != nil { + return releaseTemplateData{}, err + } + mergedVals := maputil.MergeMaps(st.Values(), releaseValues) + return st.createReleaseTemplateData(release, mergedVals), nil +} + +func (st *HelmState) resolveReleaseValues(release *ReleaseSpec) (map[string]any, error) { + merged := map[string]any{} + + values := []any{} + for _, v := range release.Values { + switch typedValue := v.(type) { + case string: + path := st.storage().normalizePath(release.ValuesPathPrefix + typedValue) + values = append(values, path) + default: + values = append(values, v) + } + } + + valuesMapSecretsRendered, err := st.valsRuntime.Eval(map[string]any{"values": values}) + if err != nil { + return nil, err + } + + valuesSecretsRendered, ok := valuesMapSecretsRendered["values"].([]any) + if !ok { + return nil, fmt.Errorf("failed to render values in %s for release %s: type %T isn't supported", st.FilePath, release.Name, valuesMapSecretsRendered["values"]) + } + + for _, v := range valuesSecretsRendered { + switch typedValue := v.(type) { + case string: + paths, skip, err := st.storage().resolveFile(st.getReleaseMissingFileHandler(release), "values", typedValue, st.getReleaseMissingFileHandlerConfig(release).resolveFileOptions()...) + if err != nil { + return nil, err + } + if skip { + continue + } + if len(paths) > 1 { + return nil, fmt.Errorf("glob patterns in release values is not supported for template data resolution") + } + path := paths[0] + + yamlBytes, err := st.RenderReleaseValuesFileToBytes(release, path) + if err != nil { + return nil, fmt.Errorf("failed to render values file \"%s\": %v", typedValue, err) + } + + var vals map[string]any + if err := yaml.Unmarshal(yamlBytes, &vals); err != nil { + return nil, fmt.Errorf("failed to parse values file \"%s\": %v", typedValue, err) + } + + merged = maputil.MergeMaps(merged, vals) + case map[any]any: + strMap, err := maputil.CastKeysToStrings(typedValue) + if err != nil { + return nil, err + } + merged = maputil.MergeMaps(merged, strMap) + case map[string]any: + merged = maputil.MergeMaps(merged, typedValue) + default: + return nil, fmt.Errorf("unexpected type of value in release values: value=%v, type=%T", typedValue, typedValue) + } + } + + return merged, nil +} + +func (st *HelmState) renderValuesFileToBytesWithData(path string, templateData releaseTemplateData) ([]byte, error) { + r := tmpl.NewFileRenderer(st.fs, filepath.Dir(path), templateData) + rawBytes, err := r.RenderToBytes(path) + if err != nil { + return nil, err + } + + match, err := regexp.Match("ref\\+.*", rawBytes) + if err != nil { + return nil, err + } + + if match { + var rawYaml map[string]any + + if err := yaml.Unmarshal(rawBytes, &rawYaml); err != nil { + return nil, err + } + + parsedYaml, err := st.valsRuntime.Eval(rawYaml) + if err != nil { + return nil, err + } + + return yaml.Marshal(parsedYaml) + } + + return rawBytes, nil +} + +func (st *HelmState) generateTemporaryReleaseValuesFilesWithData(release *ReleaseSpec, values []any, templateData releaseTemplateData) ([]string, error) { + generatedFiles := []string{} + + for _, value := range values { + switch typedValue := value.(type) { + case string: + paths, skip, err := st.storage().resolveFile(st.getReleaseMissingFileHandler(release), "values", typedValue, st.getReleaseMissingFileHandlerConfig(release).resolveFileOptions()...) + if err != nil { + return generatedFiles, err + } + if skip { + continue + } + + if len(paths) > 1 { + return generatedFiles, fmt.Errorf("glob patterns in release values and secrets is not supported yet. please submit a feature request if necessary") + } + path := paths[0] + + yamlBytes, err := st.renderValuesFileToBytesWithData(path, templateData) + if err != nil { + return generatedFiles, fmt.Errorf("failed to render values files \"%s\": %v", typedValue, err) + } + + valfile, err := createTempValuesFile(release, yamlBytes) + if err != nil { + return generatedFiles, err + } + defer func() { + _ = valfile.Close() + }() + + if _, err := valfile.Write(yamlBytes); err != nil { + return generatedFiles, fmt.Errorf("failed to write %s: %v", valfile.Name(), err) + } + + st.logger.Debugf("Successfully generated the value file at %s. produced:\n%s", path, string(yamlBytes)) + + generatedFiles = append(generatedFiles, valfile.Name()) + case map[any]any, map[string]any: + valfile, err := createTempValuesFile(release, typedValue) + if err != nil { + return generatedFiles, err + } + defer func() { + _ = valfile.Close() + }() + + encoder := yaml.NewEncoder(valfile) + defer func() { + _ = encoder.Close() + }() + + if err := encoder.Encode(typedValue); err != nil { + return generatedFiles, err + } + + generatedFiles = append(generatedFiles, valfile.Name()) + default: + return generatedFiles, fmt.Errorf("unexpected type of value: value=%v, type=%T", typedValue, typedValue) + } + } + return generatedFiles, nil +} + func (st *HelmState) newReleaseTemplateFuncMap(dir string) template.FuncMap { r := tmpl.NewFileRenderer(st.fs, dir, nil) From e8fee8b84cc342326c26f95b446a2aaf623f183a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Apr 2026 04:17:06 +0000 Subject: [PATCH 2/8] test: add more tests for resolveReleaseValues, renderValuesFileToBytesWithData, and generateTemporaryReleaseValuesFilesWithData Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/5da5c9d8-7464-4146-84b5-1433ed6193f3 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/state/create_test.go | 287 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 287 insertions(+) diff --git a/pkg/state/create_test.go b/pkg/state/create_test.go index 2c91ef65..326bddcf 100644 --- a/pkg/state/create_test.go +++ b/pkg/state/create_test.go @@ -1096,3 +1096,290 @@ func TestMergedReleaseTemplateData_InlineValues(t *testing.T) { t.Errorf("expected ingress.enabled to be true, got %v", ingressMap["enabled"]) } } + +func newTestHelmStateWithFiles(t *testing.T, files map[string]string, basePath string) (*HelmState, func()) { + t.Helper() + logger := zaptest.NewLogger(t).Sugar() + valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + require.NoError(t, err) + + testFs := testhelper.NewTestFs(files) + testFs.Cwd = basePath + + st := &HelmState{ + logger: logger, + fs: testFs.ToFileSystem(), + valsRuntime: valsRuntime, + basePath: basePath, + FilePath: basePath + "/helmfile.yaml", + RenderedValues: map[string]any{}, + } + return st, func() {} +} + +func TestResolveReleaseValues_Empty(t *testing.T) { + st, cleanup := newTestHelmStateWithFiles(t, map[string]string{}, "/project") + defer cleanup() + + release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} + result, err := st.resolveReleaseValues(release) + require.NoError(t, err) + assert.Empty(t, result) +} + +func TestResolveReleaseValues_FromFile(t *testing.T) { + valuesFile := "/project/values.yaml" + valuesContent := `replicaCount: 2 +image: + repository: nginx + tag: "1.21" +` + st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + valuesFile: valuesContent, + }, "/project") + defer cleanup() + + release := &ReleaseSpec{ + Name: "myrelease", + Chart: "mychart", + Values: []any{ + "values.yaml", + }, + } + result, err := st.resolveReleaseValues(release) + require.NoError(t, err) + assert.Equal(t, 2, result["replicaCount"]) + imageMap, ok := result["image"].(map[string]any) + require.True(t, ok, "expected image to be a map") + assert.Equal(t, "nginx", imageMap["repository"]) +} + +func TestResolveReleaseValues_InlineMap(t *testing.T) { + st, cleanup := newTestHelmStateWithFiles(t, map[string]string{}, "/project") + defer cleanup() + + release := &ReleaseSpec{ + Name: "myrelease", + Chart: "mychart", + Values: []any{ + map[string]any{ + "replicaCount": 5, + "service": map[string]any{ + "type": "ClusterIP", + "port": 80, + }, + }, + }, + } + result, err := st.resolveReleaseValues(release) + require.NoError(t, err) + assert.Equal(t, 5, result["replicaCount"]) + serviceMap, ok := result["service"].(map[string]any) + require.True(t, ok, "expected service to be a map") + assert.Equal(t, "ClusterIP", serviceMap["type"]) +} + +func TestResolveReleaseValues_MultipleSourcesMerged(t *testing.T) { + baseValuesFile := "/project/base.yaml" + baseValuesContent := `replicaCount: 1 +service: + type: ClusterIP +` + overrideValuesFile := "/project/override.yaml" + overrideValuesContent := `replicaCount: 3 +ingress: + enabled: true +` + st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + baseValuesFile: baseValuesContent, + overrideValuesFile: overrideValuesContent, + }, "/project") + defer cleanup() + + release := &ReleaseSpec{ + Name: "myrelease", + Chart: "mychart", + Values: []any{ + "base.yaml", + "override.yaml", + }, + } + result, err := st.resolveReleaseValues(release) + require.NoError(t, err) + // override.yaml value wins + assert.Equal(t, 3, result["replicaCount"]) + // from base.yaml + serviceMap, ok := result["service"].(map[string]any) + require.True(t, ok, "expected service to be a map") + assert.Equal(t, "ClusterIP", serviceMap["type"]) + // from override.yaml + ingressMap, ok := result["ingress"].(map[string]any) + require.True(t, ok, "expected ingress to be a map") + assert.Equal(t, true, ingressMap["enabled"]) +} + +func TestResolveReleaseValues_FileAndInlineMerged(t *testing.T) { + valuesFile := "/project/values.yaml" + valuesContent := `replicaCount: 1 +` + st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + valuesFile: valuesContent, + }, "/project") + defer cleanup() + + release := &ReleaseSpec{ + Name: "myrelease", + Chart: "mychart", + Values: []any{ + "values.yaml", + map[string]any{ + "extraEnv": "production", + }, + }, + } + result, err := st.resolveReleaseValues(release) + require.NoError(t, err) + assert.Equal(t, 1, result["replicaCount"]) + assert.Equal(t, "production", result["extraEnv"]) +} + +func TestRenderValuesFileToBytesWithData_PlainYAML(t *testing.T) { + valuesFile := "/project/values.yaml" + valuesContent := `replicaCount: 2 +image: + repository: nginx +` + st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + valuesFile: valuesContent, + }, "/project") + defer cleanup() + + release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} + tmplData := st.createReleaseTemplateData(release, map[string]any{}) + + result, err := st.renderValuesFileToBytesWithData(valuesFile, tmplData) + require.NoError(t, err) + assert.Contains(t, string(result), "replicaCount: 2") + assert.Contains(t, string(result), "repository: nginx") +} + +func TestRenderValuesFileToBytesWithData_WithValuesTemplate(t *testing.T) { + valuesFile := "/project/values.yaml.gotmpl" + valuesContent := `replicaCount: {{ .Values.replicaCount }} +enabled: {{ .Values.ingress.enabled }} +` + st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + valuesFile: valuesContent, + }, "/project") + defer cleanup() + + release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} + tmplData := st.createReleaseTemplateData(release, map[string]any{ + "replicaCount": 3, + "ingress": map[string]any{ + "enabled": true, + }, + }) + + result, err := st.renderValuesFileToBytesWithData(valuesFile, tmplData) + require.NoError(t, err) + assert.Contains(t, string(result), "replicaCount: 3") + assert.Contains(t, string(result), "enabled: true") +} + +func TestRenderValuesFileToBytesWithData_WithReleaseTemplate(t *testing.T) { + valuesFile := "/project/values.yaml.gotmpl" + valuesContent := `releaseName: {{ .Release.Name }} +releaseNamespace: {{ .Release.Namespace }} +` + st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + valuesFile: valuesContent, + }, "/project") + defer cleanup() + + release := &ReleaseSpec{Name: "myapp", Chart: "mychart", Namespace: "production"} + tmplData := st.createReleaseTemplateData(release, map[string]any{}) + + result, err := st.renderValuesFileToBytesWithData(valuesFile, tmplData) + require.NoError(t, err) + assert.Contains(t, string(result), "releaseName: myapp") + assert.Contains(t, string(result), "releaseNamespace: production") +} + +func TestGenerateTemporaryReleaseValuesFilesWithData_StringPath(t *testing.T) { + patchFile := "/project/patch.yaml.gotmpl" + patchContent := `enabled: {{ .Values.ingress.enabled }} +host: {{ .Values.ingress.host }} +` + st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + patchFile: patchContent, + }, "/project") + defer cleanup() + + release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} + tmplData := st.createReleaseTemplateData(release, map[string]any{ + "ingress": map[string]any{ + "enabled": true, + "host": "example.com", + }, + }) + + generatedFiles, err := st.generateTemporaryReleaseValuesFilesWithData( + release, + []any{"patch.yaml.gotmpl"}, + tmplData, + ) + require.NoError(t, err) + require.Len(t, generatedFiles, 1) + + // Read and verify the generated file content + content, err := filesystem.DefaultFileSystem().ReadFile(generatedFiles[0]) + require.NoError(t, err) + assert.Contains(t, string(content), "enabled: true") + assert.Contains(t, string(content), "host: example.com") +} + +func TestGenerateTemporaryReleaseValuesFilesWithData_InlineMap(t *testing.T) { + st, cleanup := newTestHelmStateWithFiles(t, map[string]string{}, "/project") + defer cleanup() + + release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} + tmplData := st.createReleaseTemplateData(release, map[string]any{}) + + inlineValues := map[string]any{ + "replicaCount": 5, + "service": map[string]any{ + "type": "NodePort", + }, + } + + generatedFiles, err := st.generateTemporaryReleaseValuesFilesWithData( + release, + []any{inlineValues}, + tmplData, + ) + require.NoError(t, err) + require.Len(t, generatedFiles, 1) + + content, err := filesystem.DefaultFileSystem().ReadFile(generatedFiles[0]) + require.NoError(t, err) + assert.Contains(t, string(content), "replicaCount: 5") + assert.Contains(t, string(content), "NodePort") +} + +func TestGenerateTemporaryReleaseValuesFilesWithData_UnknownTypeError(t *testing.T) { + st, cleanup := newTestHelmStateWithFiles(t, map[string]string{}, "/project") + defer cleanup() + + release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} + tmplData := st.createReleaseTemplateData(release, map[string]any{}) + + // Passing an unsupported type (int) should return an error + _, err := st.generateTemporaryReleaseValuesFilesWithData( + release, + []any{42}, + tmplData, + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "unexpected type of value") +} From c66017c26614fa7792aa7fd674a41e33211fe6a5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Apr 2026 04:22:01 +0000 Subject: [PATCH 3/8] test: simplify newTestHelmStateWithFiles by removing empty cleanup func Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/5da5c9d8-7464-4146-84b5-1433ed6193f3 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/state/create_test.go | 41 +++++++++++++++------------------------- 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/pkg/state/create_test.go b/pkg/state/create_test.go index 326bddcf..dba3a0bd 100644 --- a/pkg/state/create_test.go +++ b/pkg/state/create_test.go @@ -1097,7 +1097,7 @@ func TestMergedReleaseTemplateData_InlineValues(t *testing.T) { } } -func newTestHelmStateWithFiles(t *testing.T, files map[string]string, basePath string) (*HelmState, func()) { +func newTestHelmStateWithFiles(t *testing.T, files map[string]string, basePath string) *HelmState { t.Helper() logger := zaptest.NewLogger(t).Sugar() valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) @@ -1106,7 +1106,7 @@ func newTestHelmStateWithFiles(t *testing.T, files map[string]string, basePath s testFs := testhelper.NewTestFs(files) testFs.Cwd = basePath - st := &HelmState{ + return &HelmState{ logger: logger, fs: testFs.ToFileSystem(), valsRuntime: valsRuntime, @@ -1114,12 +1114,10 @@ func newTestHelmStateWithFiles(t *testing.T, files map[string]string, basePath s FilePath: basePath + "/helmfile.yaml", RenderedValues: map[string]any{}, } - return st, func() {} } func TestResolveReleaseValues_Empty(t *testing.T) { - st, cleanup := newTestHelmStateWithFiles(t, map[string]string{}, "/project") - defer cleanup() + st := newTestHelmStateWithFiles(t, map[string]string{}, "/project") release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} result, err := st.resolveReleaseValues(release) @@ -1134,10 +1132,9 @@ image: repository: nginx tag: "1.21" ` - st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + st := newTestHelmStateWithFiles(t, map[string]string{ valuesFile: valuesContent, }, "/project") - defer cleanup() release := &ReleaseSpec{ Name: "myrelease", @@ -1155,8 +1152,7 @@ image: } func TestResolveReleaseValues_InlineMap(t *testing.T) { - st, cleanup := newTestHelmStateWithFiles(t, map[string]string{}, "/project") - defer cleanup() + st := newTestHelmStateWithFiles(t, map[string]string{}, "/project") release := &ReleaseSpec{ Name: "myrelease", @@ -1190,11 +1186,10 @@ service: ingress: enabled: true ` - st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + st := newTestHelmStateWithFiles(t, map[string]string{ baseValuesFile: baseValuesContent, overrideValuesFile: overrideValuesContent, }, "/project") - defer cleanup() release := &ReleaseSpec{ Name: "myrelease", @@ -1222,10 +1217,9 @@ func TestResolveReleaseValues_FileAndInlineMerged(t *testing.T) { valuesFile := "/project/values.yaml" valuesContent := `replicaCount: 1 ` - st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + st := newTestHelmStateWithFiles(t, map[string]string{ valuesFile: valuesContent, }, "/project") - defer cleanup() release := &ReleaseSpec{ Name: "myrelease", @@ -1249,10 +1243,9 @@ func TestRenderValuesFileToBytesWithData_PlainYAML(t *testing.T) { image: repository: nginx ` - st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + st := newTestHelmStateWithFiles(t, map[string]string{ valuesFile: valuesContent, }, "/project") - defer cleanup() release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} tmplData := st.createReleaseTemplateData(release, map[string]any{}) @@ -1268,10 +1261,9 @@ func TestRenderValuesFileToBytesWithData_WithValuesTemplate(t *testing.T) { valuesContent := `replicaCount: {{ .Values.replicaCount }} enabled: {{ .Values.ingress.enabled }} ` - st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + st := newTestHelmStateWithFiles(t, map[string]string{ valuesFile: valuesContent, }, "/project") - defer cleanup() release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} tmplData := st.createReleaseTemplateData(release, map[string]any{ @@ -1292,10 +1284,9 @@ func TestRenderValuesFileToBytesWithData_WithReleaseTemplate(t *testing.T) { valuesContent := `releaseName: {{ .Release.Name }} releaseNamespace: {{ .Release.Namespace }} ` - st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + st := newTestHelmStateWithFiles(t, map[string]string{ valuesFile: valuesContent, }, "/project") - defer cleanup() release := &ReleaseSpec{Name: "myapp", Chart: "mychart", Namespace: "production"} tmplData := st.createReleaseTemplateData(release, map[string]any{}) @@ -1311,10 +1302,9 @@ func TestGenerateTemporaryReleaseValuesFilesWithData_StringPath(t *testing.T) { patchContent := `enabled: {{ .Values.ingress.enabled }} host: {{ .Values.ingress.host }} ` - st, cleanup := newTestHelmStateWithFiles(t, map[string]string{ + st := newTestHelmStateWithFiles(t, map[string]string{ patchFile: patchContent, }, "/project") - defer cleanup() release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} tmplData := st.createReleaseTemplateData(release, map[string]any{ @@ -1332,7 +1322,7 @@ host: {{ .Values.ingress.host }} require.NoError(t, err) require.Len(t, generatedFiles, 1) - // Read and verify the generated file content + // The temp files are created on the real OS filesystem via os.Create, so we read them with os.ReadFile content, err := filesystem.DefaultFileSystem().ReadFile(generatedFiles[0]) require.NoError(t, err) assert.Contains(t, string(content), "enabled: true") @@ -1340,8 +1330,7 @@ host: {{ .Values.ingress.host }} } func TestGenerateTemporaryReleaseValuesFilesWithData_InlineMap(t *testing.T) { - st, cleanup := newTestHelmStateWithFiles(t, map[string]string{}, "/project") - defer cleanup() + st := newTestHelmStateWithFiles(t, map[string]string{}, "/project") release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} tmplData := st.createReleaseTemplateData(release, map[string]any{}) @@ -1361,6 +1350,7 @@ func TestGenerateTemporaryReleaseValuesFilesWithData_InlineMap(t *testing.T) { require.NoError(t, err) require.Len(t, generatedFiles, 1) + // The temp files are created on the real OS filesystem via os.Create, so we read them with os.ReadFile content, err := filesystem.DefaultFileSystem().ReadFile(generatedFiles[0]) require.NoError(t, err) assert.Contains(t, string(content), "replicaCount: 5") @@ -1368,8 +1358,7 @@ func TestGenerateTemporaryReleaseValuesFilesWithData_InlineMap(t *testing.T) { } func TestGenerateTemporaryReleaseValuesFilesWithData_UnknownTypeError(t *testing.T) { - st, cleanup := newTestHelmStateWithFiles(t, map[string]string{}, "/project") - defer cleanup() + st := newTestHelmStateWithFiles(t, map[string]string{}, "/project") release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} tmplData := st.createReleaseTemplateData(release, map[string]any{}) From 01838037b92fd23066edfbdee8991e8d3ee9bc15 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 00:46:55 +0000 Subject: [PATCH 4/8] fix: remove always-constant basePath param from newTestHelmStateWithFiles to fix unparam lint error Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/b4a669cb-692c-4ca6-a68b-1b04a062b989 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/state/create_test.go | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/pkg/state/create_test.go b/pkg/state/create_test.go index dba3a0bd..3e174652 100644 --- a/pkg/state/create_test.go +++ b/pkg/state/create_test.go @@ -1097,8 +1097,9 @@ func TestMergedReleaseTemplateData_InlineValues(t *testing.T) { } } -func newTestHelmStateWithFiles(t *testing.T, files map[string]string, basePath string) *HelmState { +func newTestHelmStateWithFiles(t *testing.T, files map[string]string) *HelmState { t.Helper() + const basePath = "/project" logger := zaptest.NewLogger(t).Sugar() valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) require.NoError(t, err) @@ -1117,7 +1118,7 @@ func newTestHelmStateWithFiles(t *testing.T, files map[string]string, basePath s } func TestResolveReleaseValues_Empty(t *testing.T) { - st := newTestHelmStateWithFiles(t, map[string]string{}, "/project") + st := newTestHelmStateWithFiles(t, map[string]string{}) release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} result, err := st.resolveReleaseValues(release) @@ -1134,7 +1135,7 @@ image: ` st := newTestHelmStateWithFiles(t, map[string]string{ valuesFile: valuesContent, - }, "/project") + }) release := &ReleaseSpec{ Name: "myrelease", @@ -1152,7 +1153,7 @@ image: } func TestResolveReleaseValues_InlineMap(t *testing.T) { - st := newTestHelmStateWithFiles(t, map[string]string{}, "/project") + st := newTestHelmStateWithFiles(t, map[string]string{}) release := &ReleaseSpec{ Name: "myrelease", @@ -1189,7 +1190,7 @@ ingress: st := newTestHelmStateWithFiles(t, map[string]string{ baseValuesFile: baseValuesContent, overrideValuesFile: overrideValuesContent, - }, "/project") + }) release := &ReleaseSpec{ Name: "myrelease", @@ -1219,7 +1220,7 @@ func TestResolveReleaseValues_FileAndInlineMerged(t *testing.T) { ` st := newTestHelmStateWithFiles(t, map[string]string{ valuesFile: valuesContent, - }, "/project") + }) release := &ReleaseSpec{ Name: "myrelease", @@ -1245,7 +1246,7 @@ image: ` st := newTestHelmStateWithFiles(t, map[string]string{ valuesFile: valuesContent, - }, "/project") + }) release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} tmplData := st.createReleaseTemplateData(release, map[string]any{}) @@ -1263,7 +1264,7 @@ enabled: {{ .Values.ingress.enabled }} ` st := newTestHelmStateWithFiles(t, map[string]string{ valuesFile: valuesContent, - }, "/project") + }) release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} tmplData := st.createReleaseTemplateData(release, map[string]any{ @@ -1286,7 +1287,7 @@ releaseNamespace: {{ .Release.Namespace }} ` st := newTestHelmStateWithFiles(t, map[string]string{ valuesFile: valuesContent, - }, "/project") + }) release := &ReleaseSpec{Name: "myapp", Chart: "mychart", Namespace: "production"} tmplData := st.createReleaseTemplateData(release, map[string]any{}) @@ -1304,7 +1305,7 @@ host: {{ .Values.ingress.host }} ` st := newTestHelmStateWithFiles(t, map[string]string{ patchFile: patchContent, - }, "/project") + }) release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} tmplData := st.createReleaseTemplateData(release, map[string]any{ @@ -1330,7 +1331,7 @@ host: {{ .Values.ingress.host }} } func TestGenerateTemporaryReleaseValuesFilesWithData_InlineMap(t *testing.T) { - st := newTestHelmStateWithFiles(t, map[string]string{}, "/project") + st := newTestHelmStateWithFiles(t, map[string]string{}) release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} tmplData := st.createReleaseTemplateData(release, map[string]any{}) @@ -1358,7 +1359,7 @@ func TestGenerateTemporaryReleaseValuesFilesWithData_InlineMap(t *testing.T) { } func TestGenerateTemporaryReleaseValuesFilesWithData_UnknownTypeError(t *testing.T) { - st := newTestHelmStateWithFiles(t, map[string]string{}, "/project") + st := newTestHelmStateWithFiles(t, map[string]string{}) release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} tmplData := st.createReleaseTemplateData(release, map[string]any{}) From 71bae6beb0fcb51e4753fad1d2907691a5a64177 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 01:05:32 +0000 Subject: [PATCH 5/8] fix: address c66017c review comments - error messages, defer-in-loop, map normalization, test cleanup Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/44988dd8-1c67-465b-995c-80525a24eb93 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/state/create_test.go | 20 ++++++- pkg/state/state.go | 120 ++++++++++++++++++++++----------------- 2 files changed, 84 insertions(+), 56 deletions(-) diff --git a/pkg/state/create_test.go b/pkg/state/create_test.go index 3e174652..21fcafca 100644 --- a/pkg/state/create_test.go +++ b/pkg/state/create_test.go @@ -2,6 +2,7 @@ package state import ( "fmt" + "os" "path/filepath" "reflect" "testing" @@ -936,7 +937,8 @@ releases: func TestMergedReleaseTemplateData_IncludesReleaseValues(t *testing.T) { logger := zaptest.NewLogger(t).Sugar() - testValsRuntime, _ := vals.New(vals.Options{CacheSize: 32}) + testValsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + require.NoError(t, err) yamlFile := "/example/path/to/helmfile.yaml" yamlContent := []byte(`environments: @@ -1004,7 +1006,8 @@ releases: func TestMergedReleaseTemplateData_ReleaseValuesOverrideEnvValues(t *testing.T) { logger := zaptest.NewLogger(t).Sugar() - testValsRuntime, _ := vals.New(vals.Options{CacheSize: 32}) + testValsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + require.NoError(t, err) yamlFile := "/example/path/to/helmfile.yaml" yamlContent := []byte(`environments: @@ -1055,7 +1058,8 @@ releases: func TestMergedReleaseTemplateData_InlineValues(t *testing.T) { logger := zaptest.NewLogger(t).Sugar() - testValsRuntime, _ := vals.New(vals.Options{CacheSize: 32}) + testValsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + require.NoError(t, err) yamlFile := "/example/path/to/helmfile.yaml" yamlContent := []byte(`releases: @@ -1322,6 +1326,11 @@ host: {{ .Values.ingress.host }} ) require.NoError(t, err) require.Len(t, generatedFiles, 1) + t.Cleanup(func() { + for _, f := range generatedFiles { + _ = os.Remove(f) + } + }) // The temp files are created on the real OS filesystem via os.Create, so we read them with os.ReadFile content, err := filesystem.DefaultFileSystem().ReadFile(generatedFiles[0]) @@ -1350,6 +1359,11 @@ func TestGenerateTemporaryReleaseValuesFilesWithData_InlineMap(t *testing.T) { ) require.NoError(t, err) require.Len(t, generatedFiles, 1) + t.Cleanup(func() { + for _, f := range generatedFiles { + _ = os.Remove(f) + } + }) // The temp files are created on the real OS filesystem via os.Create, so we read them with os.ReadFile content, err := filesystem.DefaultFileSystem().ReadFile(generatedFiles[0]) diff --git a/pkg/state/state.go b/pkg/state/state.go index b00e79a6..b6c83744 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -3961,7 +3961,7 @@ func (st *HelmState) resolveReleaseValues(release *ReleaseSpec) (map[string]any, continue } if len(paths) > 1 { - return nil, fmt.Errorf("glob patterns in release values is not supported for template data resolution") + return nil, fmt.Errorf("glob patterns in release values are not supported for template data resolution: value=%q, resolvedPaths=%v", typedValue, paths) } path := paths[0] @@ -3999,6 +3999,7 @@ func (st *HelmState) renderValuesFileToBytesWithData(path string, templateData r return nil, err } + // If 'ref+.*' exists in file, run vals against the file match, err := regexp.Match("ref\\+.*", rawBytes) if err != nil { return nil, err @@ -4037,7 +4038,7 @@ func (st *HelmState) generateTemporaryReleaseValuesFilesWithData(release *Releas } if len(paths) > 1 { - return generatedFiles, fmt.Errorf("glob patterns in release values and secrets is not supported yet. please submit a feature request if necessary") + return generatedFiles, fmt.Errorf("glob patterns are not supported in patch/transformer values yet. please submit a feature request if necessary") } path := paths[0] @@ -4046,40 +4047,81 @@ func (st *HelmState) generateTemporaryReleaseValuesFilesWithData(release *Releas return generatedFiles, fmt.Errorf("failed to render values files \"%s\": %v", typedValue, err) } - valfile, err := createTempValuesFile(release, yamlBytes) + if err := func() error { + valfile, err := createTempValuesFile(release, yamlBytes) + if err != nil { + return err + } + defer func() { + _ = valfile.Close() + }() + + if _, err := valfile.Write(yamlBytes); err != nil { + return fmt.Errorf("failed to write %s: %v", valfile.Name(), err) + } + + st.logger.Debugf("Successfully generated the value file at %s. produced:\n%s", path, string(yamlBytes)) + + generatedFiles = append(generatedFiles, valfile.Name()) + + return nil + }(); err != nil { + return generatedFiles, err + } + case map[any]any: + strMap, err := maputil.CastKeysToStrings(typedValue) if err != nil { return generatedFiles, err } - defer func() { - _ = valfile.Close() - }() + if err := func() error { + valfile, err := createTempValuesFile(release, strMap) + if err != nil { + return err + } + defer func() { + _ = valfile.Close() + }() - if _, err := valfile.Write(yamlBytes); err != nil { - return generatedFiles, fmt.Errorf("failed to write %s: %v", valfile.Name(), err) - } + encoder := yaml.NewEncoder(valfile) + defer func() { + _ = encoder.Close() + }() - st.logger.Debugf("Successfully generated the value file at %s. produced:\n%s", path, string(yamlBytes)) + if err := encoder.Encode(strMap); err != nil { + return err + } - generatedFiles = append(generatedFiles, valfile.Name()) - case map[any]any, map[string]any: - valfile, err := createTempValuesFile(release, typedValue) - if err != nil { + generatedFiles = append(generatedFiles, valfile.Name()) + + return nil + }(); err != nil { return generatedFiles, err } - defer func() { - _ = valfile.Close() - }() + case map[string]any: + if err := func() error { + valfile, err := createTempValuesFile(release, typedValue) + if err != nil { + return err + } + defer func() { + _ = valfile.Close() + }() - encoder := yaml.NewEncoder(valfile) - defer func() { - _ = encoder.Close() - }() + encoder := yaml.NewEncoder(valfile) + defer func() { + _ = encoder.Close() + }() - if err := encoder.Encode(typedValue); err != nil { + if err := encoder.Encode(typedValue); err != nil { + return err + } + + generatedFiles = append(generatedFiles, valfile.Name()) + + return nil + }(); err != nil { return generatedFiles, err } - - generatedFiles = append(generatedFiles, valfile.Name()) default: return generatedFiles, fmt.Errorf("unexpected type of value: value=%v, type=%T", typedValue, typedValue) } @@ -4095,35 +4137,7 @@ func (st *HelmState) newReleaseTemplateFuncMap(dir string) template.FuncMap { func (st *HelmState) RenderReleaseValuesFileToBytes(release *ReleaseSpec, path string) ([]byte, error) { templateData := st.newReleaseTemplateData(release) - - r := tmpl.NewFileRenderer(st.fs, filepath.Dir(path), templateData) - rawBytes, err := r.RenderToBytes(path) - if err != nil { - return nil, err - } - - // If 'ref+.*' exists in file, run vals against the file - match, err := regexp.Match("ref\\+.*", rawBytes) - if err != nil { - return nil, err - } - - if match { - var rawYaml map[string]any - - if err := yaml.Unmarshal(rawBytes, &rawYaml); err != nil { - return nil, err - } - - parsedYaml, err := st.valsRuntime.Eval(rawYaml) - if err != nil { - return nil, err - } - - return yaml.Marshal(parsedYaml) - } - - return rawBytes, nil + return st.renderValuesFileToBytesWithData(path, templateData) } func (st *HelmState) storage() *Storage { From 09bf3c22a904b0166f715df5c22743e72ad99cc3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 06:41:08 +0000 Subject: [PATCH 6/8] refactor: extract generateTemporaryReleaseValuesFilesCore to eliminate duplication; fix temp dir leaks in tests Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/b254ddda-aa95-4e2f-8dd9-1ce4c40eedb6 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/state/create_test.go | 16 +--- pkg/state/state.go | 201 +++++++++++++++------------------------ 2 files changed, 80 insertions(+), 137 deletions(-) diff --git a/pkg/state/create_test.go b/pkg/state/create_test.go index 21fcafca..164d811b 100644 --- a/pkg/state/create_test.go +++ b/pkg/state/create_test.go @@ -2,7 +2,6 @@ package state import ( "fmt" - "os" "path/filepath" "reflect" "testing" @@ -15,6 +14,7 @@ import ( "go.uber.org/zap/zaptest" "github.com/helmfile/helmfile/pkg/environment" + "github.com/helmfile/helmfile/pkg/envvar" "github.com/helmfile/helmfile/pkg/filesystem" "github.com/helmfile/helmfile/pkg/remote" "github.com/helmfile/helmfile/pkg/testhelper" @@ -1303,6 +1303,8 @@ releaseNamespace: {{ .Release.Namespace }} } func TestGenerateTemporaryReleaseValuesFilesWithData_StringPath(t *testing.T) { + t.Setenv(envvar.TempDir, t.TempDir()) + patchFile := "/project/patch.yaml.gotmpl" patchContent := `enabled: {{ .Values.ingress.enabled }} host: {{ .Values.ingress.host }} @@ -1326,11 +1328,6 @@ host: {{ .Values.ingress.host }} ) require.NoError(t, err) require.Len(t, generatedFiles, 1) - t.Cleanup(func() { - for _, f := range generatedFiles { - _ = os.Remove(f) - } - }) // The temp files are created on the real OS filesystem via os.Create, so we read them with os.ReadFile content, err := filesystem.DefaultFileSystem().ReadFile(generatedFiles[0]) @@ -1340,6 +1337,8 @@ host: {{ .Values.ingress.host }} } func TestGenerateTemporaryReleaseValuesFilesWithData_InlineMap(t *testing.T) { + t.Setenv(envvar.TempDir, t.TempDir()) + st := newTestHelmStateWithFiles(t, map[string]string{}) release := &ReleaseSpec{Name: "myrelease", Chart: "mychart"} @@ -1359,11 +1358,6 @@ func TestGenerateTemporaryReleaseValuesFilesWithData_InlineMap(t *testing.T) { ) require.NoError(t, err) require.Len(t, generatedFiles, 1) - t.Cleanup(func() { - for _, f := range generatedFiles { - _ = os.Remove(f) - } - }) // The temp files are created on the real OS filesystem via os.Create, so we read them with os.ReadFile content, err := filesystem.DefaultFileSystem().ReadFile(generatedFiles[0]) diff --git a/pkg/state/state.go b/pkg/state/state.go index b6c83744..dc372ab8 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -4024,109 +4024,9 @@ func (st *HelmState) renderValuesFileToBytesWithData(path string, templateData r } func (st *HelmState) generateTemporaryReleaseValuesFilesWithData(release *ReleaseSpec, values []any, templateData releaseTemplateData) ([]string, error) { - generatedFiles := []string{} - - for _, value := range values { - switch typedValue := value.(type) { - case string: - paths, skip, err := st.storage().resolveFile(st.getReleaseMissingFileHandler(release), "values", typedValue, st.getReleaseMissingFileHandlerConfig(release).resolveFileOptions()...) - if err != nil { - return generatedFiles, err - } - if skip { - continue - } - - if len(paths) > 1 { - return generatedFiles, fmt.Errorf("glob patterns are not supported in patch/transformer values yet. please submit a feature request if necessary") - } - path := paths[0] - - yamlBytes, err := st.renderValuesFileToBytesWithData(path, templateData) - if err != nil { - return generatedFiles, fmt.Errorf("failed to render values files \"%s\": %v", typedValue, err) - } - - if err := func() error { - valfile, err := createTempValuesFile(release, yamlBytes) - if err != nil { - return err - } - defer func() { - _ = valfile.Close() - }() - - if _, err := valfile.Write(yamlBytes); err != nil { - return fmt.Errorf("failed to write %s: %v", valfile.Name(), err) - } - - st.logger.Debugf("Successfully generated the value file at %s. produced:\n%s", path, string(yamlBytes)) - - generatedFiles = append(generatedFiles, valfile.Name()) - - return nil - }(); err != nil { - return generatedFiles, err - } - case map[any]any: - strMap, err := maputil.CastKeysToStrings(typedValue) - if err != nil { - return generatedFiles, err - } - if err := func() error { - valfile, err := createTempValuesFile(release, strMap) - if err != nil { - return err - } - defer func() { - _ = valfile.Close() - }() - - encoder := yaml.NewEncoder(valfile) - defer func() { - _ = encoder.Close() - }() - - if err := encoder.Encode(strMap); err != nil { - return err - } - - generatedFiles = append(generatedFiles, valfile.Name()) - - return nil - }(); err != nil { - return generatedFiles, err - } - case map[string]any: - if err := func() error { - valfile, err := createTempValuesFile(release, typedValue) - if err != nil { - return err - } - defer func() { - _ = valfile.Close() - }() - - encoder := yaml.NewEncoder(valfile) - defer func() { - _ = encoder.Close() - }() - - if err := encoder.Encode(typedValue); err != nil { - return err - } - - generatedFiles = append(generatedFiles, valfile.Name()) - - return nil - }(); err != nil { - return generatedFiles, err - } - default: - return generatedFiles, fmt.Errorf("unexpected type of value: value=%v, type=%T", typedValue, typedValue) - } - } - return generatedFiles, nil + return st.generateTemporaryReleaseValuesFilesCore(release, values, func(path string) ([]byte, error) { + return st.renderValuesFileToBytesWithData(path, templateData) + }) } func (st *HelmState) newReleaseTemplateFuncMap(dir string) template.FuncMap { @@ -4265,6 +4165,14 @@ func (st *HelmState) getMissingFileHandler() *string { } func (st *HelmState) generateTemporaryReleaseValuesFiles(release *ReleaseSpec, values []any) ([]string, error) { + return st.generateTemporaryReleaseValuesFilesCore(release, values, func(path string) ([]byte, error) { + return st.RenderReleaseValuesFileToBytes(release, path) + }) +} + +// generateTemporaryReleaseValuesFilesCore is the shared implementation for generating temporary values files. +// renderStringValue is called for each string value entry after the file path has been resolved. +func (st *HelmState) generateTemporaryReleaseValuesFilesCore(release *ReleaseSpec, values []any, renderStringValue func(path string) ([]byte, error)) ([]string, error) { generatedFiles := []string{} for _, value := range values { @@ -4283,45 +4191,86 @@ func (st *HelmState) generateTemporaryReleaseValuesFiles(release *ReleaseSpec, v } path := paths[0] - yamlBytes, err := st.RenderReleaseValuesFileToBytes(release, path) + yamlBytes, err := renderStringValue(path) if err != nil { return generatedFiles, fmt.Errorf("failed to render values files \"%s\": %v", typedValue, err) } - valfile, err := createTempValuesFile(release, yamlBytes) + if err := func() error { + valfile, err := createTempValuesFile(release, yamlBytes) + if err != nil { + return err + } + defer func() { + _ = valfile.Close() + }() + + if _, err := valfile.Write(yamlBytes); err != nil { + return fmt.Errorf("failed to write %s: %v", valfile.Name(), err) + } + + st.logger.Debugf("Successfully generated the value file at %s. produced:\n%s", path, string(yamlBytes)) + + generatedFiles = append(generatedFiles, valfile.Name()) + + return nil + }(); err != nil { + return generatedFiles, err + } + case map[any]any: + strMap, err := maputil.CastKeysToStrings(typedValue) if err != nil { return generatedFiles, err } - defer func() { - _ = valfile.Close() - }() + if err := func() error { + valfile, err := createTempValuesFile(release, strMap) + if err != nil { + return err + } + defer func() { + _ = valfile.Close() + }() - if _, err := valfile.Write(yamlBytes); err != nil { - return generatedFiles, fmt.Errorf("failed to write %s: %v", valfile.Name(), err) - } + encoder := yaml.NewEncoder(valfile) + defer func() { + _ = encoder.Close() + }() - st.logger.Debugf("Successfully generated the value file at %s. produced:\n%s", path, string(yamlBytes)) + if err := encoder.Encode(strMap); err != nil { + return err + } - generatedFiles = append(generatedFiles, valfile.Name()) - case map[any]any, map[string]any: - valfile, err := createTempValuesFile(release, typedValue) - if err != nil { + generatedFiles = append(generatedFiles, valfile.Name()) + + return nil + }(); err != nil { return generatedFiles, err } - defer func() { - _ = valfile.Close() - }() + case map[string]any: + if err := func() error { + valfile, err := createTempValuesFile(release, typedValue) + if err != nil { + return err + } + defer func() { + _ = valfile.Close() + }() - encoder := yaml.NewEncoder(valfile) - defer func() { - _ = encoder.Close() - }() + encoder := yaml.NewEncoder(valfile) + defer func() { + _ = encoder.Close() + }() - if err := encoder.Encode(typedValue); err != nil { + if err := encoder.Encode(typedValue); err != nil { + return err + } + + generatedFiles = append(generatedFiles, valfile.Name()) + + return nil + }(); err != nil { return generatedFiles, err } - - generatedFiles = append(generatedFiles, valfile.Name()) default: return generatedFiles, fmt.Errorf("unexpected type of value: value=%v, type=%T", typedValue, typedValue) } From 5792ea4df05e02431971dff3e320c3a3292f2bf1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 10:55:47 +0000 Subject: [PATCH 7/8] fix: remove rendered content from debug log; extract prepareReleaseValuesEntries shared helper Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/321c6ba5-f835-4afd-be5e-ee790bc6b4a5 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/state/state.go | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/pkg/state/state.go b/pkg/state/state.go index dc372ab8..3ce1e732 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -3926,9 +3926,9 @@ func (st *HelmState) mergedReleaseTemplateData(release *ReleaseSpec) (releaseTem return st.createReleaseTemplateData(release, mergedVals), nil } -func (st *HelmState) resolveReleaseValues(release *ReleaseSpec) (map[string]any, error) { - merged := map[string]any{} - +// prepareReleaseValuesEntries normalizes release.Values path entries (applying ValuesPathPrefix) +// and evaluates any vals ref+ secrets, returning the fully-rendered values slice ready for processing. +func (st *HelmState) prepareReleaseValuesEntries(release *ReleaseSpec) ([]any, error) { values := []any{} for _, v := range release.Values { switch typedValue := v.(type) { @@ -3936,7 +3936,7 @@ func (st *HelmState) resolveReleaseValues(release *ReleaseSpec) (map[string]any, path := st.storage().normalizePath(release.ValuesPathPrefix + typedValue) values = append(values, path) default: - values = append(values, v) + values = append(values, typedValue) } } @@ -3950,6 +3950,17 @@ func (st *HelmState) resolveReleaseValues(release *ReleaseSpec) (map[string]any, return nil, fmt.Errorf("failed to render values in %s for release %s: type %T isn't supported", st.FilePath, release.Name, valuesMapSecretsRendered["values"]) } + return valuesSecretsRendered, nil +} + +func (st *HelmState) resolveReleaseValues(release *ReleaseSpec) (map[string]any, error) { + merged := map[string]any{} + + valuesSecretsRendered, err := st.prepareReleaseValuesEntries(release) + if err != nil { + return nil, err + } + for _, v := range valuesSecretsRendered { switch typedValue := v.(type) { case string: @@ -4209,7 +4220,7 @@ func (st *HelmState) generateTemporaryReleaseValuesFilesCore(release *ReleaseSpe return fmt.Errorf("failed to write %s: %v", valfile.Name(), err) } - st.logger.Debugf("Successfully generated the value file at %s. produced:\n%s", path, string(yamlBytes)) + st.logger.Debugf("Successfully generated the value file from %s to %s", path, valfile.Name()) generatedFiles = append(generatedFiles, valfile.Name()) @@ -4279,27 +4290,11 @@ func (st *HelmState) generateTemporaryReleaseValuesFilesCore(release *ReleaseSpe } func (st *HelmState) generateVanillaValuesFiles(release *ReleaseSpec) ([]string, error) { - values := []any{} - for _, v := range release.Values { - switch typedValue := v.(type) { - case string: - path := st.storage().normalizePath(release.ValuesPathPrefix + typedValue) - values = append(values, path) - default: - values = append(values, v) - } - } - - valuesMapSecretsRendered, err := st.valsRuntime.Eval(map[string]any{"values": values}) + valuesSecretsRendered, err := st.prepareReleaseValuesEntries(release) if err != nil { return nil, err } - valuesSecretsRendered, ok := valuesMapSecretsRendered["values"].([]any) - if !ok { - return nil, fmt.Errorf("Failed to render values in %s for release %s: type %T isn't supported", st.FilePath, release.Name, valuesMapSecretsRendered["values"]) - } - generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, valuesSecretsRendered) if err != nil { return nil, err From badd5b8e00e8823dbd90fdbba03d36d94df1fd08 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Apr 2026 06:08:56 +0000 Subject: [PATCH 8/8] fix: compute mergedReleaseTemplateData lazily in PrepareChartify Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/100b7974-3268-4dc8-be21-12bd82aa2dbb Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/state/create_test.go | 6 +++--- pkg/state/helmx.go | 28 +++++++++++++++++++--------- pkg/state/state.go | 6 +++++- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/pkg/state/create_test.go b/pkg/state/create_test.go index 164d811b..cd305eee 100644 --- a/pkg/state/create_test.go +++ b/pkg/state/create_test.go @@ -1324,7 +1324,7 @@ host: {{ .Values.ingress.host }} generatedFiles, err := st.generateTemporaryReleaseValuesFilesWithData( release, []any{"patch.yaml.gotmpl"}, - tmplData, + func() (releaseTemplateData, error) { return tmplData, nil }, ) require.NoError(t, err) require.Len(t, generatedFiles, 1) @@ -1354,7 +1354,7 @@ func TestGenerateTemporaryReleaseValuesFilesWithData_InlineMap(t *testing.T) { generatedFiles, err := st.generateTemporaryReleaseValuesFilesWithData( release, []any{inlineValues}, - tmplData, + func() (releaseTemplateData, error) { return tmplData, nil }, ) require.NoError(t, err) require.Len(t, generatedFiles, 1) @@ -1376,7 +1376,7 @@ func TestGenerateTemporaryReleaseValuesFilesWithData_UnknownTypeError(t *testing _, err := st.generateTemporaryReleaseValuesFilesWithData( release, []any{42}, - tmplData, + func() (releaseTemplateData, error) { return tmplData, nil }, ) require.Error(t, err) assert.Contains(t, err.Error(), "unexpected type of value") diff --git a/pkg/state/helmx.go b/pkg/state/helmx.go index f26a2f41..f259d585 100644 --- a/pkg/state/helmx.go +++ b/pkg/state/helmx.go @@ -404,18 +404,28 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp shouldRun = true } - var patchTemplateData *releaseTemplateData - if len(release.JSONPatches) > 0 || len(release.StrategicMergePatches) > 0 || len(release.Transformers) > 0 { - td, err := st.mergedReleaseTemplateData(release) - if err != nil { - return nil, clean, fmt.Errorf("failed to compute merged release values for patch rendering: %v", err) + // patchTemplateData is computed lazily on first use: only when an actual string patch/transformer + // file path is rendered. Inline-map entries don't require template data at all, so we avoid + // unnecessary I/O for releases whose patches are all inline maps or that have no string entries. + var ( + cachedPatchTemplateData releaseTemplateData + cachedPatchTemplateDataErr error + cachedPatchTemplateDataSet bool + ) + getPatchTemplateData := func() (releaseTemplateData, error) { + if !cachedPatchTemplateDataSet { + cachedPatchTemplateDataSet = true + cachedPatchTemplateData, cachedPatchTemplateDataErr = st.mergedReleaseTemplateData(release) + if cachedPatchTemplateDataErr != nil { + cachedPatchTemplateDataErr = fmt.Errorf("failed to compute merged release values for patch rendering: %v", cachedPatchTemplateDataErr) + } } - patchTemplateData = &td + return cachedPatchTemplateData, cachedPatchTemplateDataErr } jsonPatches := release.JSONPatches if len(jsonPatches) > 0 { - generatedFiles, err := st.generateTemporaryReleaseValuesFilesWithData(release, jsonPatches, *patchTemplateData) + generatedFiles, err := st.generateTemporaryReleaseValuesFilesWithData(release, jsonPatches, getPatchTemplateData) if err != nil { return nil, clean, err } @@ -429,7 +439,7 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp strategicMergePatches := release.StrategicMergePatches if len(strategicMergePatches) > 0 { - generatedFiles, err := st.generateTemporaryReleaseValuesFilesWithData(release, strategicMergePatches, *patchTemplateData) + generatedFiles, err := st.generateTemporaryReleaseValuesFilesWithData(release, strategicMergePatches, getPatchTemplateData) if err != nil { return nil, clean, err } @@ -443,7 +453,7 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp transformers := release.Transformers if len(transformers) > 0 { - generatedFiles, err := st.generateTemporaryReleaseValuesFilesWithData(release, transformers, *patchTemplateData) + generatedFiles, err := st.generateTemporaryReleaseValuesFilesWithData(release, transformers, getPatchTemplateData) if err != nil { return nil, clean, err } diff --git a/pkg/state/state.go b/pkg/state/state.go index 3ce1e732..a2d45960 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -4034,8 +4034,12 @@ func (st *HelmState) renderValuesFileToBytesWithData(path string, templateData r return rawBytes, nil } -func (st *HelmState) generateTemporaryReleaseValuesFilesWithData(release *ReleaseSpec, values []any, templateData releaseTemplateData) ([]string, error) { +func (st *HelmState) generateTemporaryReleaseValuesFilesWithData(release *ReleaseSpec, values []any, getTemplateData func() (releaseTemplateData, error)) ([]string, error) { return st.generateTemporaryReleaseValuesFilesCore(release, values, func(path string) ([]byte, error) { + templateData, err := getTemplateData() + if err != nil { + return nil, err + } return st.renderValuesFileToBytesWithData(path, templateData) }) }