The `TestValidateChartIconPresence` test fails when run after
`TestValidateChartIconURL` as they both are using a global
variable `badChart.Icon`
```
: go test -v -test.shuffle 1 -run '^(TestValidateChartIconPresence|TestValidateChartIconURL)$' ./pkg/lint/rules/
-test.shuffle 1
=== RUN TestValidateChartIconURL
--- PASS: TestValidateChartIconURL (0.00s)
=== RUN TestValidateChartIconPresence
chartfile_test.go:171: validateChartIconPresence to return a linter error, got no error
--- FAIL: TestValidateChartIconPresence (0.00s)
FAIL
FAIL helm.sh/helm/v4/pkg/lint/rules 0.051s
FAIL
: go test -v -test.shuffle 2 -run '^(TestValidateChartIconPresence|TestValidateChartIconURL)$' ./pkg/lint/rules/
-test.shuffle 2
=== RUN TestValidateChartIconPresence
--- PASS: TestValidateChartIconPresence (0.00s)
=== RUN TestValidateChartIconURL
--- PASS: TestValidateChartIconURL (0.00s)
PASS
ok helm.sh/helm/v4/pkg/lint/rules 0.050s
```
This commit:
1. Remove dependency on global variable
2. Explicitly set the state of the test object.
Signed-off-by: Rostyslav Polishchuk <rostyslavp@google.com>
When "helm lint" is run, it now errors on invalid chartfiles,
e.g. those with duplicate keys
Closes#12381
Signed-off-by: Edward Miller <edmiller287@gmail.com>
A new library was introduced that provides JSON Schema checking for
newer versions of the schema. In Helm v4, there is no need to have
two packages doing the JSON schema validation. The message output
can have breaking changes.
This change moves everything to the newer library. It also uses a
wrapper error to enable a clean Helm only interface for the
public Go API validation functions. This would enable the replacement
of the Schema validation library, if needed, without breaking the
Go API contract.
Signed-off-by: Matt Farina <matt.farina@suse.com>
This change moves the code, updates the import locations, and
adds a doc.go file to document what the v2 package is for.
This is part of HIP 20 for v3 charts
Signed-off-by: Matt Farina <matt.farina@suse.com>
chartutil was originally created to operate on protobufs which are
no longer part of Helm. The util package makes more sense to be
part of the chart package.
This change is part of the HIP 20 to create v3 charts and
explicitly call out v2 charts. The changes for this are in smaller
bite size changes.
Signed-off-by: Matt Farina <matt.farina@suse.com>
Since Helm is going through breaking changes with Helm v4, the version path to
Helm needs to be updated.
Signed-off-by: Matt Farina <matt.farina@suse.com>
When --skip-schema-validation is set, any schema contain in the helm chart is ignored. Defaults to 'false'.
Closes#10398
Signed-off-by: anessi <16045045+anessi@users.noreply.github.com>
Noteis:
1. This moves golangci scanning to a GitHub action. This will
enable inline pointers to issues in the PR where linting fails.
2. Go 1.21 is specified in the go.mod because Kubernetes libs
require it.
3. The lint issues were removed. Some were fixed while others
were handled by skipping linting or using _ as an argument.
Many of these can be refactored later for better cleanup.
Signed-off-by: Matt Farina <matt.farina@suse.com>
This commit replaces `ensure.TempDir` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests complete.
Prior to this commit, temporary directory created using `ensure.TempDir`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
defer func() {
if err := os.RemoveAll(dir); err != nil {
t.Fatal(err)
}
}
is also tedious, but `t.TempDir` handles this for us nicely.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Some charts don't need a templates directory. The warning appears to originate back when helm expected all charts to contain templates. In CI situations where linting is performed with --strict the lint warning causes unneccessary failures for charts that quite legitimately do not contain a templates directory.
As discussed in #8033 there are no longer any scenarios understood where this warning is helpful so this commit removes it.
Signed-off-by: James Blair <mail@jamesblair.net>
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>
Report charts with the following bad dependency specifications as bad charts:
dependencies:
- name: foo
alias: baz # ← baz used twice
version: 1.0.0
- name: bar
alias: baz # ← baz used twice
version: 1.0.0
dependencies:
- name: foo
alias: bar # ← shadows chart below
version: 1.0.0
- name: bar
version: 1.0.0
dependencies:
- name: foo
version: 1.0.0
- name: foo # ← chart with same name as above (although version or repo will be different, this will not work currently)
version: 1.2.3
Closes#9169
Signed-off-by: Daniel Strobusch <1847260+dastrobu@users.noreply.github.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>
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>
When running helm lint, import-values for dependencies are ignored,
also added test for linting chart with import-values
Closes#9658
Signed-off-by: Stuart Drennan <stuart.drennan@gmail.com>
The 'helm.sh/resource-policy' annotation is only supported on top level
objects. The annotation is ignored if given on a nested object within a
list.
Ref #9829
Signed-off-by: Adam Reese <adam@reese.io>
findnature
George Jenkins
Matt Farina
Rostyslav Polishchuk
Justen Stall
Edward Miller
Robert Sirchia
Benoit Tigeot
Terry Howe
Zach Burgess
anessi
Antoine Deschênes
Joe Julian
Matt Farina
Eng Zer Jun
James Blair
Daniel Strobusch
wujunwei
Sverre Boschman
Stuart Drennan
Adam Reese
Martin Hickey
Mark Sagi-Kazar
Adam Reese