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>
At this time both Go 1.19 and 1.20 are supported. The version
specified in the go.mod file is the minimum version we expect Helm
to be compiled against. This is the oldest supported version to
support environments where others compile Helm. The Helm project
is using Go 1.20 to build Helm itself.
Updating to Go 1.19 also includes dealing with io/ioutil
deprecation and some additional linting issues around staticcheck.
All the staticcheck issues were in test files so linting was
skipped for those.
Signed-off-by: Matt Farina <matt.farina@suse.com>
The directory created by `T.TempDir` is automatically removed when the
test and all its subtests complete.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
golint which is used as one of the sublinters in golangci-lint is deprecated.
It is replaced with revive which is a drop-in replacement.
Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
Co-authored-by: Martin Mulholland <mmulholl@redhat.com>
This bug came about because of three issues that this change
corrects:
- The CI scripts run on a pull request did not test building
Helm. This means that a failure to set a variable using LDFLAGS
had no opportunity to be caught.
- #8608 provided a means to match the k8s version used in linting
and chartutil with the version of the package we pull in. With
one problem. It attempts to set a const as if it were a string.
This is ignored and everyone missed it.
- #10325 moved those constants to vars so it could be set. This
looked good and passed tests but missed that you can't set an
int as if it were a string. See first bullet.
This change fixes this by moved the internal representation to
be a string. These are internal variables not exposed in the public
API which makes this change non-breaking to the API.
Closes#10367
Signed-off-by: Matt Farina <matt.farina@suse.com>
Problem: the warnings don't give enough details about which
values are problematic, only the name of the leaf key. This is
all the more annoying when you have a chart depending on other charts.
```
mainchart
|
+- subchart1
+- subchart2
+- subchart3
```
Here are some warnings I get before the change:
```
coalesce.go:199: warning: destination for credentials is a table. Ignoring non-table value
coalesce.go:160: warning: skipped value for resources: Not a table.
coalesce.go:160: warning: skipped value for googleSheetsServiceAccount: Not a table.
coalesce.go:199: warning: destination for googleSheetsServiceAccount is a table. Ignoring non-table value
coalesce.go:199: warning: destination for resources is a table. Ignoring non-table value []
coalesce.go:199: warning: destination for credentials is a table. Ignoring non-table value
coalesce.go:199: warning: destination for credentials is a table. Ignoring non-table value
coalesce.go:160: warning: skipped value for resources: Not a table.
coalesce.go:160: warning: skipped value for googleSheetsServiceAccount: Not a table.
```
with fix:
```
coalesce.go:162: warning: skipped value for subchart1.resources: Not a table.
coalesce.go:162: warning: skipped value for subchart2.googleSheetsServiceAccount: Not a table.
coalesce.go:211: warning: destination for subchart3.aws.credentials is a table. Ignoring non-table value ()
coalesce.go:211: warning: destination for mainchart.subchart3.aws.credentials is a table. Ignoring non-table value ()
coalesce.go:211: warning: destination for mainchart.subchart2.googleSheetsServiceAccount is a table. Ignoring non-table value ()
coalesce.go:211: warning: destination for mainchart.subchart1.resources is a table. Ignoring non-table value ([])
coalesce.go:162: warning: skipped value for subchart1.resources: Not a table.
coalesce.go:162: warning: skipped value for subchart2.googleSheetsServiceAccount: Not a table.
coalesce.go:211: warning: destination for subchart3.aws.credentials is a table. Ignoring non-table value ()
```
Signed-off-by: Damien Nozay <damiennozay+github@gmail.com>
add tests
Signed-off-by: Damien Nozay <damiennozay+github@gmail.com>
Problem: the warnings don't give enough details about which
values are problematic, only the name of the leaf key. This is
all the more annoying when you have a chart depending on other charts.
```
mainchart
|
+- subchart1
+- subchart2
+- subchart3
```
Here are some warnings I get before the change:
```
coalesce.go:199: warning: destination for credentials is a table. Ignoring non-table value
coalesce.go:160: warning: skipped value for resources: Not a table.
coalesce.go:160: warning: skipped value for googleSheetsServiceAccount: Not a table.
coalesce.go:199: warning: destination for googleSheetsServiceAccount is a table. Ignoring non-table value
coalesce.go:199: warning: destination for resources is a table. Ignoring non-table value []
coalesce.go:199: warning: destination for credentials is a table. Ignoring non-table value
coalesce.go:199: warning: destination for credentials is a table. Ignoring non-table value
coalesce.go:160: warning: skipped value for resources: Not a table.
coalesce.go:160: warning: skipped value for googleSheetsServiceAccount: Not a table.
```
with fix:
```
coalesce.go:162: warning: skipped value for subchart1.resources: Not a table.
coalesce.go:162: warning: skipped value for subchart2.googleSheetsServiceAccount: Not a table.
coalesce.go:211: warning: destination for subchart3.aws.credentials is a table. Ignoring non-table value ()
coalesce.go:211: warning: destination for mainchart.subchart3.aws.credentials is a table. Ignoring non-table value ()
coalesce.go:211: warning: destination for mainchart.subchart2.googleSheetsServiceAccount is a table. Ignoring non-table value ()
coalesce.go:211: warning: destination for mainchart.subchart1.resources is a table. Ignoring non-table value ([])
coalesce.go:162: warning: skipped value for subchart1.resources: Not a table.
coalesce.go:162: warning: skipped value for subchart2.googleSheetsServiceAccount: Not a table.
coalesce.go:211: warning: destination for subchart3.aws.credentials is a table. Ignoring non-table value ()
```
Signed-off-by: Damien Nozay <damiennozay+github@gmail.com>
add tests
Signed-off-by: Damien Nozay <damiennozay+github@gmail.com>
Tim Chaplin
Joe Julian
guoguangwu
Matt Farina
Matt Farina
Dmitry Kamenskikh
wujunwei
AdamKorcz
genofire
Eugene Zuev
Pat Riehecky
Martin Hickey
Peter Leong
yxxhero
Eng Zer Jun
mengjiao.liu
Sverre Boschman
Aram Zegerius
Damien Nozay
Bernd Gutjahr
Giacomo Margaria
Mark Sagi-Kazar
Thomas Dy