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 <aiopsclub@163.com>
This commit is contained in:
yxxhero
yxxhero
parent
b50c9d6256
commit
c6e74b4394
|
|
@ -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"])
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue