Fix multiple bugs in values handling

First, some notes about priority and how some code flow works.

For Helm handling values, the expected order of precidence is:
1. User specified values (e.g CLI)
2. Imported values
3. Parent chart values
4. Subchart values

Helm handles dependency values slightly differently. If there are dependencies
in the charts folder that are not marked as dependencies all of the values,
including nil values, are pulled in. If those charts are listed as a
dependency in the Chart.yaml file than they are processed for import handling.
Prior to the changes here, it caused nil values at the top level to NOT remove
values specified.

The changes:

1. The order of priority was chagned from the list above. Parnet chart values
would override specifically imported values. This is due to a change from
just over a year ago that introduced a bug. That was undone by changing the
precidence when maps were merged.

2. To handle merging while retaining the nil values, which was causing
inconsistent behavior, a new set of Merge functions were introduced. These
functions are just like coalesce except that they DO NOT remove nil/null values.
The new functions are used in a backward compatible manner meaning some new
functions were introduced that called them.

Specific issues fixed (that are known):

Closes #9027

Can now delete subkeys from charts when specified in the parent. This behavior
was previously inconsistent. Sometimes they could be deleted and other times
it did not work. Now it is consistent.

Closes #10899

Imported values (from library or other subcharts) are now used following the
order above.

The previous behavior was inconsistent. import-values using just a string
would import them. When named with a child/parent it did not work if the
parent already had a value. If string and named were mixed the imports
worked if the string happened first but just for the string not the named.
If the named parent/child went first then none of them worked for cases
where the parent already had a value. It was inconsistent and the tests
sometimes mirrored the functionality rather than expected behavior.

Tests for this fall into the sub-packages and are in the template tests
to verify it's happening in the output. Including having values passed
at the CLI as the ultimate highest priority to be used.

This relates to a fix that went in for #9940. The expected values there don't
fit the precedence above where the parent value would override the imported
value. That fix/change introduced more bugs.

Closes #10052

This is the case where imported values using the parent/child designation
just didn't work right. That has been fixed and there are tests. The underlying
issue had to do with the precedence order handling.

Note, a lot of tests were added. Hope we got it more right this time.

Signed-off-by: Matt Farina <matt.farina@suse.com>
pull/12162/head
Matt Farina 1 year ago
parent 4e447d87cd
commit 0a5148faff
No known key found for this signature in database
GPG Key ID: 92C44A3D421FF7F9

@ -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)
}

@ -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

@ -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

@ -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

@ -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

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

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

@ -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

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

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

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

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

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

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

@ -223,7 +223,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
}

@ -215,7 +215,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
}

@ -53,7 +53,35 @@ 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 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) {
v, err := copystructure.Copy(vals)
if err != nil {
return vals, err
}
valsCopy := v.(map[string]interface{})
// if we have an empty map, make sure it is initialized
if valsCopy == nil {
valsCopy = make(map[string]interface{})
}
return coalesce(log.Printf, chrt, valsCopy, "", true)
}
type printFn func(format string, v ...interface{})
@ -61,13 +89,13 @@ 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)
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 +106,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 +122,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 +156,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 +185,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 +232,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 +246,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 +268,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)
}

@ -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)
}

@ -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)
}

@ -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, false); err != nil {
t.Fatalf("processing import values dependencies %v", err)
}

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

@ -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

@ -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

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

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

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

@ -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

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

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

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

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

@ -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
}

Loading…
Cancel
Save