From c6e74b4394dce893bf2ccb2692d0369a5451452f Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 26 Apr 2026 11:56:43 +0800 Subject: [PATCH] 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)