upstream/helm
1
0
Fork 0
mirror of https://github.com/helm/helm.git synced 2026-05-28 04:35:48 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 17

Merge pull request #12162 from mattfarina/fix-merge-values-ugh

Browse source 
Fix multiple bugs in values handling
This commit is contained in:
Matt Farina 2023-07-26 09:30:09 -04:00 committed by GitHub
commit 343389856b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
38 changed files with 1098 additions and 55 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
cmd/helm/template_test.go
View file

@ -25,6 +25,8 @@ import (
var chartPath = "testdata/testcharts/subchart"
func TestTemplateCmd(t *testing.T) {
deletevalchart := "testdata/testcharts/issue-9027"
tests := []cmdTestCase{
{
name: "check name",
@ -131,6 +133,34 @@ func TestTemplateCmd(t *testing.T) {
cmd: fmt.Sprintf(`template '%s' --skip-tests`, chartPath),
golden: "output/template-skip-tests.txt",
},
{
// This test case is to ensure the case where specified dependencies
// in the Chart.yaml and those where the Chart.yaml don't have them
// specified are the same.
name: "ensure nil/null values pass to subcharts delete values",
cmd: fmt.Sprintf("template '%s'", deletevalchart),
golden: "output/issue-9027.txt",
},
{
// Ensure that imported values take precedence over parent chart values
name: "template with imported subchart values ensuring import",
cmd: fmt.Sprintf("template '%s' --set configmap.enabled=true --set subchartb.enabled=true", chartPath),
golden: "output/template-subchart-cm.txt",
},
{
// Ensure that user input values take precedence over imported
// values from sub-charts.
name: "template with imported subchart values set with --set",
cmd: fmt.Sprintf("template '%s' --set configmap.enabled=true --set subchartb.enabled=true --set configmap.value=baz", chartPath),
golden: "output/template-subchart-cm-set.txt",
},
{
// Ensure that user input values take precedence over imported
// values from sub-charts when passed by file
name: "template with imported subchart values set with --set",
cmd: fmt.Sprintf("template '%s' -f %s/extra_values.yaml", chartPath, chartPath),
golden: "output/template-subchart-cm-set-file.txt",
},
}
runTestCmd(t, tests)
}

32
cmd/helm/testdata/output/issue-9027.txt vendored Normal file
View file

@ -0,0 +1,32 @@
---
# Source: issue-9027/charts/subchart/templates/values.yaml
global:
hash:
key3: 13
key4: 4
key5: 5
key6: 6
hash:
key3: 13
key4: 4
key5: 5
key6: 6
---
# Source: issue-9027/templates/values.yaml
global:
hash:
key1: null
key2: null
key3: 13
subchart:
global:
hash:
key3: 13
key4: 4
key5: 5
key6: 6
hash:
key3: 13
key4: 4
key5: 5
key6: 6

122
cmd/helm/testdata/output/template-subchart-cm-set-file.txt vendored Normal file
View file

@ -0,0 +1,122 @@
---
# Source: subchart/templates/subdir/serviceaccount.yaml
apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/configmap.yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: subchart-cm
data:
value: qux
---
# Source: subchart/templates/subdir/role.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: subchart-role
rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: subchart-binding
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: subchart-role
subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
kind: Service
metadata:
name: subcharta
labels:
helm.sh/chart: "subcharta-0.1.0"
spec:
type: ClusterIP
ports:
- port: 80
targetPort: 80
protocol: TCP
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
kind: Service
metadata:
name: subchartb
labels:
helm.sh/chart: "subchartb-0.1.0"
spec:
type: ClusterIP
ports:
- port: 80
targetPort: 80
protocol: TCP
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1
kind: Service
metadata:
name: subchart
labels:
helm.sh/chart: "subchart-0.1.0"
app.kubernetes.io/instance: "release-name"
kube-version/major: "1"
kube-version/minor: "20"
kube-version/version: "v1.20.0"
spec:
type: ClusterIP
ports:
- port: 80
targetPort: 80
protocol: TCP
name: nginx
selector:
app.kubernetes.io/name: subchart
---
# Source: subchart/templates/tests/test-config.yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: "release-name-testconfig"
annotations:
"helm.sh/hook": test
data:
message: Hello World
---
# Source: subchart/templates/tests/test-nothing.yaml
apiVersion: v1
kind: Pod
metadata:
name: "release-name-test"
annotations:
"helm.sh/hook": test
spec:
containers:
- name: test
image: "alpine:latest"
envFrom:
- configMapRef:
name: "release-name-testconfig"
command:
- echo
- "$message"
restartPolicy: Never

122
cmd/helm/testdata/output/template-subchart-cm-set.txt vendored Normal file
View file

@ -0,0 +1,122 @@
---
# Source: subchart/templates/subdir/serviceaccount.yaml
apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/configmap.yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: subchart-cm
data:
value: baz
---
# Source: subchart/templates/subdir/role.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: subchart-role
rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: subchart-binding
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: subchart-role
subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
kind: Service
metadata:
name: subcharta
labels:
helm.sh/chart: "subcharta-0.1.0"
spec:
type: ClusterIP
ports:
- port: 80
targetPort: 80
protocol: TCP
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
kind: Service
metadata:
name: subchartb
labels:
helm.sh/chart: "subchartb-0.1.0"
spec:
type: ClusterIP
ports:
- port: 80
targetPort: 80
protocol: TCP
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1
kind: Service
metadata:
name: subchart
labels:
helm.sh/chart: "subchart-0.1.0"
app.kubernetes.io/instance: "release-name"
kube-version/major: "1"
kube-version/minor: "20"
kube-version/version: "v1.20.0"
spec:
type: ClusterIP
ports:
- port: 80
targetPort: 80
protocol: TCP
name: nginx
selector:
app.kubernetes.io/name: subchart
---
# Source: subchart/templates/tests/test-config.yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: "release-name-testconfig"
annotations:
"helm.sh/hook": test
data:
message: Hello World
---
# Source: subchart/templates/tests/test-nothing.yaml
apiVersion: v1
kind: Pod
metadata:
name: "release-name-test"
annotations:
"helm.sh/hook": test
spec:
containers:
- name: test
image: "alpine:latest"
envFrom:
- configMapRef:
name: "release-name-testconfig"
command:
- echo
- "$message"
restartPolicy: Never

122
cmd/helm/testdata/output/template-subchart-cm.txt vendored Normal file
View file

@ -0,0 +1,122 @@
---
# Source: subchart/templates/subdir/serviceaccount.yaml
apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/configmap.yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: subchart-cm
data:
value: bar
---
# Source: subchart/templates/subdir/role.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: subchart-role
rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: subchart-binding
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: subchart-role
subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
kind: Service
metadata:
name: subcharta
labels:
helm.sh/chart: "subcharta-0.1.0"
spec:
type: ClusterIP
ports:
- port: 80
targetPort: 80
protocol: TCP
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
kind: Service
metadata:
name: subchartb
labels:
helm.sh/chart: "subchartb-0.1.0"
spec:
type: ClusterIP
ports:
- port: 80
targetPort: 80
protocol: TCP
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1
kind: Service
metadata:
name: subchart
labels:
helm.sh/chart: "subchart-0.1.0"
app.kubernetes.io/instance: "release-name"
kube-version/major: "1"
kube-version/minor: "20"
kube-version/version: "v1.20.0"
spec:
type: ClusterIP
ports:
- port: 80
targetPort: 80
protocol: TCP
name: nginx
selector:
app.kubernetes.io/name: subchart
---
# Source: subchart/templates/tests/test-config.yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: "release-name-testconfig"
annotations:
"helm.sh/hook": test
data:
message: Hello World
---
# Source: subchart/templates/tests/test-nothing.yaml
apiVersion: v1
kind: Pod
metadata:
name: "release-name-test"
annotations:
"helm.sh/hook": test
spec:
containers:
- name: test
image: "alpine:latest"
envFrom:
- configMapRef:
name: "release-name-testconfig"
command:
- echo
- "$message"
restartPolicy: Never

6
cmd/helm/testdata/testcharts/issue-9027/Chart.yaml vendored Normal file
View file

@ -0,0 +1,6 @@
apiVersion: v2
name: issue-9027
version: 0.1.0
dependencies:
- name: subchart
version: 0.1.0

3
cmd/helm/testdata/testcharts/issue-9027/charts/subchart/Chart.yaml vendored Normal file
View file

@ -0,0 +1,3 @@
apiVersion: v2
name: subchart
version: 0.1.0

1
cmd/helm/testdata/testcharts/issue-9027/charts/subchart/templates/values.yaml vendored Normal file
View file

@ -0,0 +1 @@
{{ .Values | toYaml }}

17
cmd/helm/testdata/testcharts/issue-9027/charts/subchart/values.yaml vendored Normal file
View file

@ -0,0 +1,17 @@
global:
hash:
key1: 1
key2: 2
key3: 3
key4: 4
key5: 5
key6: 6
hash:
key1: 1
key2: 2
key3: 3
key4: 4
key5: 5
key6: 6

1
cmd/helm/testdata/testcharts/issue-9027/templates/values.yaml vendored Normal file
View file

@ -0,0 +1 @@
{{ .Values | toYaml }}

11
cmd/helm/testdata/testcharts/issue-9027/values.yaml vendored Normal file
View file

@ -0,0 +1,11 @@
global:
hash:
key1: null
key2: null
key3: 13
subchart:
hash:
key1: null
key2: null
key3: 13

3
cmd/helm/testdata/testcharts/subchart/Chart.yaml vendored
View file

@ -29,6 +29,9 @@ dependencies:
parent: imported-chartA-B
- child: exports.SCBexported2
parent: exports.SCBexported2
# - child: exports.configmap
# parent: configmap
- configmap
- SCBexported1
tags:

4
cmd/helm/testdata/testcharts/subchart/charts/subchartB/values.yaml vendored
View file

@ -20,6 +20,10 @@ exports:
SCBexported2:
SCBexported2A: "blaster"
configmap:
configmap:
value: "bar"
global:
kolla:

5
cmd/helm/testdata/testcharts/subchart/extra_values.yaml vendored Normal file
View file

@ -0,0 +1,5 @@
# This file is used to test values passed by file at the command line
configmap:
enabled: true
value: "qux"

8
cmd/helm/testdata/testcharts/subchart/templates/subdir/configmap.yaml vendored Normal file
View file

@ -0,0 +1,8 @@
{{ if .Values.configmap.enabled -}}
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ .Chart.Name }}-cm
data:
value: {{ .Values.configmap.value }}
{{- end }}

4
cmd/helm/testdata/testcharts/subchart/values.yaml vendored
View file

@ -53,3 +53,7 @@ exports:
SC1exported2:
all:
SC1exported3: "SC1expstr"
configmap:
enabled: false
value: "foo"

2
pkg/action/install.go
View file

@ -229,7 +229,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma
return nil, err
}
if err := chartutil.ProcessDependencies(chrt, vals); err != nil {
if err := chartutil.ProcessDependenciesWithMerge(chrt, vals); err != nil {
return nil, err
}

2
pkg/action/upgrade.go
View file

@ -226,7 +226,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin
return nil, nil, err
}
if err := chartutil.ProcessDependencies(chart, vals); err != nil {
if err := chartutil.ProcessDependenciesWithMerge(chart, vals); err != nil {
return nil, nil, err
}

108
pkg/chartutil/coalesce.go
View file

@ -43,6 +43,36 @@ func concatPrefix(a, b string) string {
// - A chart has access to all of the variables for it, as well as all of
// the values destined for its dependencies.
func CoalesceValues(chrt *chart.Chart, vals map[string]interface{}) (Values, error) {
valsCopy, err := copyValues(vals)
if err != nil {
return vals, err
}
return coalesce(log.Printf, chrt, valsCopy, "", false)
}
// MergeValues is used to merge the values in a chart and its subcharts. This
// is different from Coalescing as nil/null values are preserved.
//
// Values are coalesced together using the following rules:
//
// - Values in a higher level chart always override values in a lower-level
// dependency chart
// - Scalar values and arrays are replaced, maps are merged
// - A chart has access to all of the variables for it, as well as all of
// the values destined for its dependencies.
//
// Retaining Nils is useful when processes early in a Helm action or business
// logic need to retain them for when Coalescing will happen again later in the
// business logic.
func MergeValues(chrt *chart.Chart, vals map[string]interface{}) (Values, error) {
valsCopy, err := copyValues(vals)
if err != nil {
return vals, err
}
return coalesce(log.Printf, chrt, valsCopy, "", true)
}
func copyValues(vals map[string]interface{}) (Values, error) {
v, err := copystructure.Copy(vals)
if err != nil {
return vals, err
@ -53,21 +83,26 @@ func CoalesceValues(chrt *chart.Chart, vals map[string]interface{}) (Values, err
if valsCopy == nil {
valsCopy = make(map[string]interface{})
}
return coalesce(log.Printf, chrt, valsCopy, "")
return valsCopy, nil
}
type printFn func(format string, v ...interface{})
// coalesce coalesces the dest values and the chart values, giving priority to the dest values.
//
// This is a helper function for CoalesceValues.
func coalesce(printf printFn, ch *chart.Chart, dest map[string]interface{}, prefix string) (map[string]interface{}, error) {
coalesceValues(printf, ch, dest, prefix)
return coalesceDeps(printf, ch, dest, prefix)
// This is a helper function for CoalesceValues and MergeValues.
//
// Note, the merge argument specifies whether this is being used by MergeValues
// or CoalesceValues. Coalescing removes null values and their keys in some
// situations while merging keeps the null values.
func coalesce(printf printFn, ch *chart.Chart, dest map[string]interface{}, prefix string, merge bool) (map[string]interface{}, error) {
coalesceValues(printf, ch, dest, prefix, merge)
return coalesceDeps(printf, ch, dest, prefix, merge)
}
// coalesceDeps coalesces the dependencies of the given chart.
func coalesceDeps(printf printFn, chrt *chart.Chart, dest map[string]interface{}, prefix string) (map[string]interface{}, error) {
func coalesceDeps(printf printFn, chrt *chart.Chart, dest map[string]interface{}, prefix string, merge bool) (map[string]interface{}, error) {
for _, subchart := range chrt.Dependencies() {
if c, ok := dest[subchart.Name()]; !ok {
// If dest doesn't already have the key, create it.
@ -78,13 +113,11 @@ func coalesceDeps(printf printFn, chrt *chart.Chart, dest map[string]interface{}
if dv, ok := dest[subchart.Name()]; ok {
dvmap := dv.(map[string]interface{})
subPrefix := concatPrefix(prefix, chrt.Metadata.Name)
// Get globals out of dest and merge them into dvmap.
coalesceGlobals(printf, dvmap, dest, subPrefix)
coalesceGlobals(printf, dvmap, dest, subPrefix, merge)
// Now coalesce the rest of the values.
var err error
dest[subchart.Name()], err = coalesce(printf, subchart, dvmap, subPrefix)
dest[subchart.Name()], err = coalesce(printf, subchart, dvmap, subPrefix, merge)
if err != nil {
return dest, err
}
@ -96,7 +129,7 @@ func coalesceDeps(printf printFn, chrt *chart.Chart, dest map[string]interface{}
// coalesceGlobals copies the globals out of src and merges them into dest.
//
// For convenience, returns dest.
func coalesceGlobals(printf printFn, dest, src map[string]interface{}, prefix string) {
func coalesceGlobals(printf printFn, dest, src map[string]interface{}, prefix string, merge bool) {
var dg, sg map[string]interface{}
if destglob, ok := dest[GlobalKey]; !ok {
@ -130,7 +163,10 @@ func coalesceGlobals(printf printFn, dest, src map[string]interface{}, prefix st
// Basically, we reverse order of coalesce here to merge
// top-down.
subPrefix := concatPrefix(prefix, key)
coalesceTablesFullKey(printf, vv, destvmap, subPrefix)
// In this location coalesceTablesFullKey should always have
// merge set to true. The output of coalesceGlobals is run
// through coalesce where any nils will be removed.
coalesceTablesFullKey(printf, vv, destvmap, subPrefix, true)
dg[key] = vv
}
}
@ -156,12 +192,38 @@ func copyMap(src map[string]interface{}) map[string]interface{} {
// coalesceValues builds up a values map for a particular chart.
//
// Values in v will override the values in the chart.
func coalesceValues(printf printFn, c *chart.Chart, v map[string]interface{}, prefix string) {
func coalesceValues(printf printFn, c *chart.Chart, v map[string]interface{}, prefix string, merge bool) {
subPrefix := concatPrefix(prefix, c.Metadata.Name)
for key, val := range c.Values {
// Using c.Values directly when coalescing a table can cause problems where
// the original c.Values is altered. Creating a deep copy stops the problem.
// This section is fault-tolerant as there is no ability to return an error.
valuesCopy, err := copystructure.Copy(c.Values)
var vc map[string]interface{}
var ok bool
if err != nil {
// If there is an error something is wrong with copying c.Values it
// means there is a problem in the deep copying package or something
// wrong with c.Values. In this case we will use c.Values and report
// an error.
printf("warning: unable to copy values, err: %s", err)
vc = c.Values
} else {
vc, ok = valuesCopy.(map[string]interface{})
if !ok {
// c.Values has a map[string]interface{} structure. If the copy of
// it cannot be treated as map[string]interface{} there is something
// strangely wrong. Log it and use c.Values
printf("warning: unable to convert values copy to values type")
vc = c.Values
}
}
for key, val := range vc {
if value, ok := v[key]; ok {
if value == nil {
// When the YAML value is null, we remove the value's key.
if value == nil && !merge {
// When the YAML value is null and we are coalescing instead of
// merging, we remove the value's key.
// This allows Helm's various sources of values (value files or --set) to
// remove incompatible keys from any previous chart, file, or set values.
delete(v, key)
@ -177,7 +239,7 @@ func coalesceValues(printf printFn, c *chart.Chart, v map[string]interface{}, pr
} else {
// Because v has higher precedence than nv, dest values override src
// values.
coalesceTablesFullKey(printf, dest, src, concatPrefix(subPrefix, key))
coalesceTablesFullKey(printf, dest, src, concatPrefix(subPrefix, key), merge)
}
}
} else {
@ -191,13 +253,17 @@ func coalesceValues(printf printFn, c *chart.Chart, v map[string]interface{}, pr
//
// dest is considered authoritative.
func CoalesceTables(dst, src map[string]interface{}) map[string]interface{} {
return coalesceTablesFullKey(log.Printf, dst, src, "")
return coalesceTablesFullKey(log.Printf, dst, src, "", false)
}
func MergeTables(dst, src map[string]interface{}) map[string]interface{} {
return coalesceTablesFullKey(log.Printf, dst, src, "", true)
}
// coalesceTablesFullKey merges a source map into a destination map.
//
// dest is considered authoritative.
func coalesceTablesFullKey(printf printFn, dst, src map[string]interface{}, prefix string) map[string]interface{} {
func coalesceTablesFullKey(printf printFn, dst, src map[string]interface{}, prefix string, merge bool) map[string]interface{} {
// When --reuse-values is set but there are no modifications yet, return new values
if src == nil {
return dst
@ -209,13 +275,13 @@ func coalesceTablesFullKey(printf printFn, dst, src map[string]interface{}, pref
// values.
for key, val := range src {
fullkey := concatPrefix(prefix, key)
if dv, ok := dst[key]; ok && dv == nil {
if dv, ok := dst[key]; ok && !merge && dv == nil {
delete(dst, key)
} else if !ok {
dst[key] = val
} else if istable(val) {
if istable(dv) {
coalesceTablesFullKey(printf, dv.(map[string]interface{}), val.(map[string]interface{}), fullkey)
coalesceTablesFullKey(printf, dv.(map[string]interface{}), val.(map[string]interface{}), fullkey, merge)
} else {
printf("warning: cannot overwrite table with non table for %s (%v)", fullkey, val)
}

293
pkg/chartutil/coalesce_test.go
View file

@ -213,6 +213,160 @@ func TestCoalesceValues(t *testing.T) {
is.Equal(valsCopy, vals)
}
func TestMergeValues(t *testing.T) {
is := assert.New(t)
c := withDeps(&chart.Chart{
Metadata: &chart.Metadata{Name: "moby"},
Values: map[string]interface{}{
"back": "exists",
"bottom": "exists",
"front": "exists",
"left": "exists",
"name": "moby",
"nested": map[string]interface{}{"boat": true},
"override": "bad",
"right": "exists",
"scope": "moby",
"top": "nope",
"global": map[string]interface{}{
"nested2": map[string]interface{}{"l0": "moby"},
},
},
},
withDeps(&chart.Chart{
Metadata: &chart.Metadata{Name: "pequod"},
Values: map[string]interface{}{
"name": "pequod",
"scope": "pequod",
"global": map[string]interface{}{
"nested2": map[string]interface{}{"l1": "pequod"},
},
},
},
&chart.Chart{
Metadata: &chart.Metadata{Name: "ahab"},
Values: map[string]interface{}{
"global": map[string]interface{}{
"nested": map[string]interface{}{"foo": "bar"},
"nested2": map[string]interface{}{"l2": "ahab"},
},
"scope": "ahab",
"name": "ahab",
"boat": true,
"nested": map[string]interface{}{"foo": false, "bar": true},
},
},
),
&chart.Chart{
Metadata: &chart.Metadata{Name: "spouter"},
Values: map[string]interface{}{
"scope": "spouter",
"global": map[string]interface{}{
"nested2": map[string]interface{}{"l1": "spouter"},
},
},
},
)
vals, err := ReadValues(testCoalesceValuesYaml)
if err != nil {
t.Fatal(err)
}
// taking a copy of the values before passing it
// to MergeValues as argument, so that we can
// use it for asserting later
valsCopy := make(Values, len(vals))
for key, value := range vals {
valsCopy[key] = value
}
v, err := MergeValues(c, vals)
if err != nil {
t.Fatal(err)
}
j, _ := json.MarshalIndent(v, "", " ")
t.Logf("Coalesced Values: %s", string(j))
tests := []struct {
tpl string
expect string
}{
{"{{.top}}", "yup"},
{"{{.back}}", ""},
{"{{.name}}", "moby"},
{"{{.global.name}}", "Ishmael"},
{"{{.global.subject}}", "Queequeg"},
{"{{.global.harpooner}}", "<no value>"},
{"{{.pequod.name}}", "pequod"},
{"{{.pequod.ahab.name}}", "ahab"},
{"{{.pequod.ahab.scope}}", "whale"},
{"{{.pequod.ahab.nested.foo}}", "true"},
{"{{.pequod.ahab.global.name}}", "Ishmael"},
{"{{.pequod.ahab.global.nested.foo}}", "bar"},
{"{{.pequod.ahab.global.subject}}", "Queequeg"},
{"{{.pequod.ahab.global.harpooner}}", "Tashtego"},
{"{{.pequod.global.name}}", "Ishmael"},
{"{{.pequod.global.nested.foo}}", "<no value>"},
{"{{.pequod.global.subject}}", "Queequeg"},
{"{{.spouter.global.name}}", "Ishmael"},
{"{{.spouter.global.harpooner}}", "<no value>"},
{"{{.global.nested.boat}}", "true"},
{"{{.pequod.global.nested.boat}}", "true"},
{"{{.spouter.global.nested.boat}}", "true"},
{"{{.pequod.global.nested.sail}}", "true"},
{"{{.spouter.global.nested.sail}}", "<no value>"},
{"{{.global.nested2.l0}}", "moby"},
{"{{.global.nested2.l1}}", "<no value>"},
{"{{.global.nested2.l2}}", "<no value>"},
{"{{.pequod.global.nested2.l0}}", "moby"},
{"{{.pequod.global.nested2.l1}}", "pequod"},
{"{{.pequod.global.nested2.l2}}", "<no value>"},
{"{{.pequod.ahab.global.nested2.l0}}", "moby"},
{"{{.pequod.ahab.global.nested2.l1}}", "pequod"},
{"{{.pequod.ahab.global.nested2.l2}}", "ahab"},
{"{{.spouter.global.nested2.l0}}", "moby"},
{"{{.spouter.global.nested2.l1}}", "spouter"},
{"{{.spouter.global.nested2.l2}}", "<no value>"},
}
for _, tt := range tests {
if o, err := ttpl(tt.tpl, v); err != nil || o != tt.expect {
t.Errorf("Expected %q to expand to %q, got %q", tt.tpl, tt.expect, o)
}
}
// nullKeys is different from coalescing. Here the null/nil values are not
// removed.
nullKeys := []string{"bottom", "right", "left", "front"}
for _, nullKey := range nullKeys {
if vv, ok := v[nullKey]; !ok {
t.Errorf("Expected key %q to be present but it was removed", nullKey)
} else if vv != nil {
t.Errorf("Expected key %q to be null but it has a value of %v", nullKey, vv)
}
}
if _, ok := v["nested"].(map[string]interface{})["boat"]; !ok {
t.Error("Expected nested boat key to be present but it was removed")
}
subchart := v["pequod"].(map[string]interface{})["ahab"].(map[string]interface{})
if _, ok := subchart["boat"]; !ok {
t.Error("Expected subchart boat key to be present but it was removed")
}
if _, ok := subchart["nested"].(map[string]interface{})["bar"]; !ok {
t.Error("Expected subchart nested bar key to be present but it was removed")
}
// CoalesceValues should not mutate the passed arguments
is.Equal(valsCopy, vals)
}
func TestCoalesceTables(t *testing.T) {
dst := map[string]interface{}{
"name": "Ishmael",
@ -341,6 +495,143 @@ func TestCoalesceTables(t *testing.T) {
}
}
func TestMergeTables(t *testing.T) {
dst := map[string]interface{}{
"name": "Ishmael",
"address": map[string]interface{}{
"street": "123 Spouter Inn Ct.",
"city": "Nantucket",
"country": nil,
},
"details": map[string]interface{}{
"friends": []string{"Tashtego"},
},
"boat": "pequod",
"hole": nil,
}
src := map[string]interface{}{
"occupation": "whaler",
"address": map[string]interface{}{
"state": "MA",
"street": "234 Spouter Inn Ct.",
"country": "US",
},
"details": "empty",
"boat": map[string]interface{}{
"mast": true,
},
"hole": "black",
}
// What we expect is that anything in dst overrides anything in src, but that
// otherwise the values are coalesced.
MergeTables(dst, src)
if dst["name"] != "Ishmael" {
t.Errorf("Unexpected name: %s", dst["name"])
}
if dst["occupation"] != "whaler" {
t.Errorf("Unexpected occupation: %s", dst["occupation"])
}
addr, ok := dst["address"].(map[string]interface{})
if !ok {
t.Fatal("Address went away.")
}
if addr["street"].(string) != "123 Spouter Inn Ct." {
t.Errorf("Unexpected address: %v", addr["street"])
}
if addr["city"].(string) != "Nantucket" {
t.Errorf("Unexpected city: %v", addr["city"])
}
if addr["state"].(string) != "MA" {
t.Errorf("Unexpected state: %v", addr["state"])
}
// This is one test that is different from CoalesceTables. Because country
// is a nil value and it's not removed it's still present.
if _, ok = addr["country"]; !ok {
t.Error("The country is left out.")
}
if det, ok := dst["details"].(map[string]interface{}); !ok {
t.Fatalf("Details is the wrong type: %v", dst["details"])
} else if _, ok := det["friends"]; !ok {
t.Error("Could not find your friends. Maybe you don't have any. :-(")
}
if dst["boat"].(string) != "pequod" {
t.Errorf("Expected boat string, got %v", dst["boat"])
}
// This is one test that is different from CoalesceTables. Because hole
// is a nil value and it's not removed it's still present.
if _, ok = dst["hole"]; !ok {
t.Error("The hole no longer exists.")
}
dst2 := map[string]interface{}{
"name": "Ishmael",
"address": map[string]interface{}{
"street": "123 Spouter Inn Ct.",
"city": "Nantucket",
"country": "US",
},
"details": map[string]interface{}{
"friends": []string{"Tashtego"},
},
"boat": "pequod",
"hole": "black",
"nilval": nil,
}
// What we expect is that anything in dst should have all values set,
// this happens when the --reuse-values flag is set but the chart has no modifications yet
MergeTables(dst2, nil)
if dst2["name"] != "Ishmael" {
t.Errorf("Unexpected name: %s", dst2["name"])
}
addr2, ok := dst2["address"].(map[string]interface{})
if !ok {
t.Fatal("Address went away.")
}
if addr2["street"].(string) != "123 Spouter Inn Ct." {
t.Errorf("Unexpected address: %v", addr2["street"])
}
if addr2["city"].(string) != "Nantucket" {
t.Errorf("Unexpected city: %v", addr2["city"])
}
if addr2["country"].(string) != "US" {
t.Errorf("Unexpected Country: %v", addr2["country"])
}
if det2, ok := dst2["details"].(map[string]interface{}); !ok {
t.Fatalf("Details is the wrong type: %v", dst2["details"])
} else if _, ok := det2["friends"]; !ok {
t.Error("Could not find your friends. Maybe you don't have any. :-(")
}
if dst2["boat"].(string) != "pequod" {
t.Errorf("Expected boat string, got %v", dst2["boat"])
}
if dst2["hole"].(string) != "black" {
t.Errorf("Expected hole string, got %v", dst2["boat"])
}
if dst2["nilval"] != nil {
t.Error("Expected nilvalue to have nil value but it does not")
}
}
func TestCoalesceValuesWarnings(t *testing.T) {
c := withDeps(&chart.Chart{
@ -391,7 +682,7 @@ func TestCoalesceValuesWarnings(t *testing.T) {
warnings = append(warnings, fmt.Sprintf(format, v...))
}
_, err := coalesce(printf, c, vals, "")
_, err := coalesce(printf, c, vals, "", false)
if err != nil {
t.Fatal(err)
}

91
pkg/chartutil/dependencies.go
View file

@ -19,15 +19,29 @@ import (
"log"
"strings"
"github.com/mitchellh/copystructure"
"helm.sh/helm/v3/pkg/chart"
)
// ProcessDependencies checks through this chart's dependencies, processing accordingly.
//
// TODO: For Helm v4 this can be combined with or turned into ProcessDependenciesWithMerge
func ProcessDependencies(c *chart.Chart, v Values) error {
if err := processDependencyEnabled(c, v, ""); err != nil {
return err
}
return processDependencyImportValues(c)
return processDependencyImportValues(c, false)
}
// ProcessDependenciesWithMerge checks through this chart's dependencies, processing accordingly.
// It is similar to ProcessDependencies but it does not remove nil values during
// the import/export handling process.
func ProcessDependenciesWithMerge(c *chart.Chart, v Values) error {
if err := processDependencyEnabled(c, v, ""); err != nil {
return err
}
return processDependencyImportValues(c, true)
}
// processDependencyConditions disables charts based on condition path value in values
@ -217,12 +231,18 @@ func set(path []string, data map[string]interface{}) map[string]interface{} {
}
// processImportValues merges values from child to parent based on the chart's dependencies' ImportValues field.
func processImportValues(c *chart.Chart) error {
func processImportValues(c *chart.Chart, merge bool) error {
if c.Metadata.Dependencies == nil {
return nil
}
// combine chart values and empty config to get Values
cvals, err := CoalesceValues(c, nil)
var cvals Values
var err error
if merge {
cvals, err = MergeValues(c, nil)
} else {
cvals, err = CoalesceValues(c, nil)
}
if err != nil {
return err
}
@ -248,7 +268,11 @@ func processImportValues(c *chart.Chart) error {
continue
}
// create value map from child to be merged into parent
b = CoalesceTables(cvals, pathToMap(parent, vv.AsMap()))
if merge {
b = MergeTables(b, pathToMap(parent, vv.AsMap()))
} else {
b = CoalesceTables(b, pathToMap(parent, vv.AsMap()))
}
case string:
child := "exports." + iv
outiv = append(outiv, map[string]string{
@ -260,26 +284,71 @@ func processImportValues(c *chart.Chart) error {
log.Printf("Warning: ImportValues missing table: %v", err)
continue
}
b = CoalesceTables(b, vm.AsMap())
if merge {
b = MergeTables(b, vm.AsMap())
} else {
b = CoalesceTables(b, vm.AsMap())
}
}
}
// set our formatted import values
r.ImportValues = outiv
}
// set the new values
c.Values = CoalesceTables(cvals, b)
// Imported values from a child to a parent chart have a higher priority than
// values specified in the parent chart.
if merge {
// deep copying the cvals as there are cases where pointers can end
// up in the cvals when they are copied onto b in ways that break things.
cvals = deepCopyMap(cvals)
c.Values = MergeTables(b, cvals)
} else {
// Trimming the nil values from cvals is needed for backwards compatibility.
// Previously, the b value had been populated with cvals along with some
// overrides. This caused the coalescing functionality to remove the
// nil/null values. This trimming is for backwards compat.
cvals = trimNilValues(cvals)
c.Values = CoalesceTables(b, cvals)
}
return nil
}
func deepCopyMap(vals map[string]interface{}) map[string]interface{} {
valsCopy, err := copystructure.Copy(vals)
if err != nil {
return vals
}
return valsCopy.(map[string]interface{})
}
func trimNilValues(vals map[string]interface{}) map[string]interface{} {
valsCopy, err := copystructure.Copy(vals)
if err != nil {
return vals
}
valsCopyMap := valsCopy.(map[string]interface{})
for key, val := range valsCopyMap {
if val == nil {
log.Printf("trim deleting %q", key)
// Iterate over the values and remove nil keys
delete(valsCopyMap, key)
} else if istable(val) {
log.Printf("trim copying %q", key)
// Recursively call into ourselves to remove keys from inner tables
valsCopyMap[key] = trimNilValues(val.(map[string]interface{}))
}
}
return valsCopyMap
}
// processDependencyImportValues imports specified chart values from child to parent.
func processDependencyImportValues(c *chart.Chart) error {
func processDependencyImportValues(c *chart.Chart, merge bool) error {
for _, d := range c.Dependencies() {
// recurse
if err := processDependencyImportValues(d); err != nil {
if err := processDependencyImportValues(d, merge); err != nil {
return err
}
}
return processImportValues(c)
return processImportValues(c, merge)
}

85
pkg/chartutil/dependencies_test.go
View file

@ -181,10 +181,13 @@ func TestProcessDependencyImportValues(t *testing.T) {
e["imported-chartA-B.SPextra5"] = "k8s"
e["imported-chartA-B.SC1extra5"] = "tiller"
e["overridden-chart1.SC1bool"] = "false"
e["overridden-chart1.SC1float"] = "3.141592"
e["overridden-chart1.SC1int"] = "99"
e["overridden-chart1.SC1string"] = "pollywog"
// These values are imported from the child chart to the parent. Imported
// values take precedence over those in the parent so these should be the
// values from the child chart.
e["overridden-chart1.SC1bool"] = "true"
e["overridden-chart1.SC1float"] = "3.14"
e["overridden-chart1.SC1int"] = "100"
e["overridden-chart1.SC1string"] = "dollywood"
e["overridden-chart1.SPextra2"] = "42"
e["overridden-chartA.SCAbool"] = "true"
@ -193,14 +196,17 @@ func TestProcessDependencyImportValues(t *testing.T) {
e["overridden-chartA.SCAstring"] = "jabberwocky"
e["overridden-chartA.SPextra4"] = "true"
// These values are imported from the child chart to the parent. Imported
// values take precedence over those in the parent so these should be the
// values from the child chart.
e["overridden-chartA-B.SCAbool"] = "true"
e["overridden-chartA-B.SCAfloat"] = "41.3"
e["overridden-chartA-B.SCAint"] = "808"
e["overridden-chartA-B.SCAstring"] = "jabberwocky"
e["overridden-chartA-B.SCBbool"] = "false"
e["overridden-chartA-B.SCBfloat"] = "1.99"
e["overridden-chartA-B.SCBint"] = "77"
e["overridden-chartA-B.SCBstring"] = "jango"
e["overridden-chartA-B.SCAfloat"] = "3.33"
e["overridden-chartA-B.SCAint"] = "555"
e["overridden-chartA-B.SCAstring"] = "wormwood"
e["overridden-chartA-B.SCBbool"] = "true"
e["overridden-chartA-B.SCBfloat"] = "0.25"
e["overridden-chartA-B.SCBint"] = "98"
e["overridden-chartA-B.SCBstring"] = "murkwood"
e["overridden-chartA-B.SPextra6"] = "111"
e["overridden-chartA-B.SCAextra1"] = "23"
e["overridden-chartA-B.SCBextra1"] = "13"
@ -212,7 +218,7 @@ func TestProcessDependencyImportValues(t *testing.T) {
e["SCBexported2A"] = "blaster"
e["global.SC1exported2.all.SC1exported3"] = "SC1expstr"
if err := processDependencyImportValues(c); err != nil {
if err := processDependencyImportValues(c, false); err != nil {
t.Fatalf("processing import values dependencies %v", err)
}
cc := Values(c.Values)
@ -225,18 +231,44 @@ func TestProcessDependencyImportValues(t *testing.T) {
switch pv := pv.(type) {
case float64:
if s := strconv.FormatFloat(pv, 'f', -1, 64); s != vv {
t.Errorf("failed to match imported float value %v with expected %v", s, vv)
t.Errorf("failed to match imported float value %v with expected %v for key %q", s, vv, kk)
}
case bool:
if b := strconv.FormatBool(pv); b != vv {
t.Errorf("failed to match imported bool value %v with expected %v", b, vv)
t.Errorf("failed to match imported bool value %v with expected %v for key %q", b, vv, kk)
}
default:
if pv != vv {
t.Errorf("failed to match imported string value %q with expected %q", pv, vv)
t.Errorf("failed to match imported string value %q with expected %q for key %q", pv, vv, kk)
}
}
}
// Since this was processed with coalescing there should be no null values.
// Here we verify that.
_, err := cc.PathValue("ensurenull")
if err == nil {
t.Error("expect nil value not found but found it")
}
switch xerr := err.(type) {
case ErrNoValue:
// We found what we expected
default:
t.Errorf("expected an ErrNoValue but got %q instead", xerr)
}
c = loadChart(t, "testdata/subpop")
if err := processDependencyImportValues(c, true); err != nil {
t.Fatalf("processing import values dependencies %v", err)
}
cc = Values(c.Values)
val, err := cc.PathValue("ensurenull")
if err != nil {
t.Error("expect value but ensurenull was not found")
}
if val != nil {
t.Errorf("expect nil value but got %q instead", val)
}
}
func TestProcessDependencyImportValuesMultiLevelPrecedence(t *testing.T) {
@ -244,10 +276,25 @@ func TestProcessDependencyImportValuesMultiLevelPrecedence(t *testing.T) {
e := make(map[string]string)
// The order of precedence should be:
// 1. User specified values (e.g CLI)
// 2. Imported values
// 3. Parent chart values
// 4. Sub-chart values
// The 4 app charts here deal with things differently:
// - app1 has a port value set in the umbrella chart. It does not import any
// values so the value from the umbrella chart should be used.
// - app2 has a value in the app chart and imports from the library. The
// library chart value should take precedence.
// - app3 has no value in the app chart and imports the value from the library
// chart. The library chart value should be used.
// - app4 has a value in the app chart and does not import the value from the
// library chart. The app charts value should be used.
e["app1.service.port"] = "3456"
e["app2.service.port"] = "8080"
if err := processDependencyImportValues(c); err != nil {
e["app2.service.port"] = "9090"
e["app3.service.port"] = "9090"
e["app4.service.port"] = "1234"
if err := processDependencyImportValues(c, true); err != nil {
t.Fatalf("processing import values dependencies %v", err)
}
cc := Values(c.Values)
@ -274,7 +321,7 @@ func TestProcessDependencyImportValuesForEnabledCharts(t *testing.T) {
c := loadChart(t, "testdata/import-values-from-enabled-subchart/parent-chart")
nameOverride := "parent-chart-prod"
if err := processDependencyImportValues(c); err != nil {
if err := processDependencyImportValues(c, true); err != nil {
t.Fatalf("processing import values dependencies %v", err)
}

2
pkg/chartutil/testdata/subpop/values.yaml vendored
View file

@ -41,3 +41,5 @@ tags:
subchart2alias:
enabled: false
ensurenull: null

6
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/Chart.yaml vendored
View file

@ -11,3 +11,9 @@ dependencies:
- name: app2
version: 0.1.0
condition: app2.enabled
- name: app3
version: 0.1.0
condition: app3.enabled
- name: app4
version: 0.1.0
condition: app4.enabled

11
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/charts/app3/Chart.yaml vendored Normal file
View file

@ -0,0 +1,11 @@
apiVersion: v2
name: app3
description: A Helm chart for Kubernetes
type: application
version: 0.1.0
dependencies:
- name: library
version: 0.1.0
import-values:
- defaults

5
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/charts/app3/charts/library/Chart.yaml vendored Normal file
View file

@ -0,0 +1,5 @@
apiVersion: v2
name: library
description: A Helm chart for Kubernetes
type: library
version: 0.1.0

9
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/charts/app3/charts/library/templates/service.yaml vendored Normal file
View file

@ -0,0 +1,9 @@
apiVersion: v1
kind: Service
spec:
type: {{ .Values.service.type }}
ports:
- port: {{ .Values.service.port }}
targetPort: http
protocol: TCP
name: http

5
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/charts/app3/charts/library/values.yaml vendored Normal file
View file

@ -0,0 +1,5 @@
exports:
defaults:
service:
type: ClusterIP
port: 9090

1
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/charts/app3/templates/service.yaml vendored Normal file
View file

@ -0,0 +1 @@
{{- include "library.service" . }}

2
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/charts/app3/values.yaml vendored Normal file
View file

@ -0,0 +1,2 @@
service:
type: ClusterIP

9
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/charts/app4/Chart.yaml vendored Normal file
View file

@ -0,0 +1,9 @@
apiVersion: v2
name: app4
description: A Helm chart for Kubernetes
type: application
version: 0.1.0
dependencies:
- name: library
version: 0.1.0

5
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/charts/app4/charts/library/Chart.yaml vendored Normal file
View file

@ -0,0 +1,5 @@
apiVersion: v2
name: library
description: A Helm chart for Kubernetes
type: library
version: 0.1.0

9
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/charts/app4/charts/library/templates/service.yaml vendored Normal file
View file

@ -0,0 +1,9 @@
apiVersion: v1
kind: Service
spec:
type: {{ .Values.service.type }}
ports:
- port: {{ .Values.service.port }}
targetPort: http
protocol: TCP
name: http

5
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/charts/app4/charts/library/values.yaml vendored Normal file
View file

@ -0,0 +1,5 @@
exports:
defaults:
service:
type: ClusterIP
port: 9090

1
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/charts/app4/templates/service.yaml vendored Normal file
View file

@ -0,0 +1 @@
{{- include "library.service" . }}

3
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/charts/app4/values.yaml vendored Normal file
View file

@ -0,0 +1,3 @@
service:
type: ClusterIP
port: 1234

6
pkg/chartutil/testdata/three-level-dependent-chart/umbrella/values.yaml vendored
View file

@ -6,3 +6,9 @@ app1:
app2:
enabled: true
app3:
enabled: true
app4:
enabled: true

2
pkg/lint/rules/template.go
View file

@ -72,7 +72,7 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace
// lint ignores import-values
// See https://github.com/helm/helm/issues/9658
if err := chartutil.ProcessDependencies(chart, values); err != nil {
if err := chartutil.ProcessDependenciesWithMerge(chart, values); err != nil {
return
}