diff --git a/.github/workflows/govulncheck.yml b/.github/workflows/govulncheck.yml index 8d183e244..6befb7954 100644 --- a/.github/workflows/govulncheck.yml +++ b/.github/workflows/govulncheck.yml @@ -13,6 +13,8 @@ jobs: name: govulncheck runs-on: ubuntu-latest steps: + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # pin@v4.2.2 - name: Add variables to environment file run: cat ".github/env" >> "$GITHUB_ENV" - name: Setup Go diff --git a/pkg/chart/v2/chart.go b/pkg/chart/v2/chart.go index dcc2a43eb..66ddf98a5 100644 --- a/pkg/chart/v2/chart.go +++ b/pkg/chart/v2/chart.go @@ -113,6 +113,8 @@ func (ch *Chart) ChartPath() string { } // ChartFullPath returns the full path to this chart. +// Note that the path may not correspond to the path where the file can be found on the file system if the path +// points to an aliased subchart. func (ch *Chart) ChartFullPath() string { if !ch.IsRoot() { return ch.Parent().ChartFullPath() + "/charts/" + ch.Name() diff --git a/pkg/chart/v2/util/coalesce.go b/pkg/chart/v2/util/coalesce.go index 11b829a3d..76dfdfa1a 100644 --- a/pkg/chart/v2/util/coalesce.go +++ b/pkg/chart/v2/util/coalesce.go @@ -282,6 +282,11 @@ func coalesceTablesFullKey(printf printFn, dst, src map[string]interface{}, pref if dst == nil { return src } + for key, val := range dst { + if val == nil { + src[key] = nil + } + } // Because dest has higher precedence than src, dest values override src // values. for key, val := range src { diff --git a/pkg/chart/v2/util/dependencies.go b/pkg/chart/v2/util/dependencies.go index b7f78010b..e2cce6f2f 100644 --- a/pkg/chart/v2/util/dependencies.go +++ b/pkg/chart/v2/util/dependencies.go @@ -91,6 +91,7 @@ func processDependencyTags(reqs []*chart.Dependency, cvals Values) { } } +// getAliasDependency finds the chart for an alias dependency and copies parts that will be modified func getAliasDependency(charts []*chart.Chart, dep *chart.Dependency) *chart.Chart { for _, c := range charts { if c == nil { @@ -104,17 +105,38 @@ func getAliasDependency(charts []*chart.Chart, dep *chart.Dependency) *chart.Cha } out := *c - md := *c.Metadata - out.Metadata = &md + out.Metadata = copyMetadata(c.Metadata) + + // empty dependencies and shallow copy all dependencies, otherwise parent info may be corrupted if + // there is more than one dependency aliasing this chart + out.SetDependencies() + for _, dependency := range c.Dependencies() { + cpy := *dependency + out.AddDependency(&cpy) + } if dep.Alias != "" { - md.Name = dep.Alias + out.Metadata.Name = dep.Alias } return &out } return nil } +func copyMetadata(metadata *chart.Metadata) *chart.Metadata { + md := *metadata + + if md.Dependencies != nil { + dependencies := make([]*chart.Dependency, len(md.Dependencies)) + for i := range md.Dependencies { + dependency := *md.Dependencies[i] + dependencies[i] = &dependency + } + md.Dependencies = dependencies + } + return &md +} + // processDependencyEnabled removes disabled charts from dependencies func processDependencyEnabled(c *chart.Chart, v map[string]interface{}, path string) error { if c.Metadata.Dependencies == nil { diff --git a/pkg/chart/v2/util/dependencies_test.go b/pkg/chart/v2/util/dependencies_test.go index 5bd332990..9b7fe3bef 100644 --- a/pkg/chart/v2/util/dependencies_test.go +++ b/pkg/chart/v2/util/dependencies_test.go @@ -286,6 +286,38 @@ func TestProcessDependencyImportValues(t *testing.T) { } } +func TestProcessDependencyImportValuesFromSharedDependencyToAliases(t *testing.T) { + c := loadChart(t, "testdata/chart-with-import-from-aliased-dependencies") + + if err := processDependencyEnabled(c, c.Values, ""); err != nil { + t.Fatalf("expected no errors but got %q", err) + } + if err := processDependencyImportValues(c, true); err != nil { + t.Fatalf("processing import values dependencies %v", err) + } + e := make(map[string]string) + + e["foo-defaults.defaultValue"] = "42" + e["bar-defaults.defaultValue"] = "42" + + e["foo.defaults.defaultValue"] = "42" + e["bar.defaults.defaultValue"] = "42" + + e["foo.grandchild.defaults.defaultValue"] = "42" + e["bar.grandchild.defaults.defaultValue"] = "42" + + cValues := Values(c.Values) + for kk, vv := range e { + pv, err := cValues.PathValue(kk) + if err != nil { + t.Fatalf("retrieving import values table %v %v", kk, err) + } + if pv != vv { + t.Errorf("failed to match imported value %v with expected %v", pv, vv) + } + } +} + func TestProcessDependencyImportValuesMultiLevelPrecedence(t *testing.T) { c := loadChart(t, "testdata/three-level-dependent-chart/umbrella") @@ -430,6 +462,9 @@ func TestDependentChartAliases(t *testing.T) { if aliasChart == nil { t.Fatalf("failed to get dependency chart for alias %s", req[2].Name) } + if aliasChart.Parent() != c { + t.Fatalf("dependency chart has wrong parent, expected %s but got %s", c.Name(), aliasChart.Parent().Name()) + } if req[2].Alias != "" { if aliasChart.Name() != req[2].Alias { t.Fatalf("dependency chart name should be %s but got %s", req[2].Alias, aliasChart.Name()) @@ -521,3 +556,32 @@ func TestDependentChartsWithSomeSubchartsSpecifiedInDependency(t *testing.T) { t.Fatalf("expected 1 dependency specified in Chart.yaml, got %d", len(c.Metadata.Dependencies)) } } + +func validateDependencyTree(t *testing.T, c *chart.Chart) { + for _, dependency := range c.Dependencies() { + if dependency.Parent() != c { + if dependency.Parent() != c { + t.Fatalf("dependency chart %s has wrong parent, expected %s but got %s", dependency.Name(), c.Name(), dependency.Parent().Name()) + } + } + // recurse entire tree + validateDependencyTree(t, dependency) + } +} + +func TestChartWithDependencyAliasedTwiceAndDoublyReferencedSubDependency(t *testing.T) { + c := loadChart(t, "testdata/chart-with-dependency-aliased-twice") + + if len(c.Dependencies()) != 1 { + t.Fatalf("expected one dependency for this chart, but got %d", len(c.Dependencies())) + } + + if err := processDependencyEnabled(c, c.Values, ""); err != nil { + t.Fatalf("expected no errors but got %q", err) + } + + if len(c.Dependencies()) != 2 { + t.Fatal("expected two dependencies after processing aliases") + } + validateDependencyTree(t, c) +} diff --git a/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/Chart.yaml b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/Chart.yaml new file mode 100644 index 000000000..d778f8fe9 --- /dev/null +++ b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/Chart.yaml @@ -0,0 +1,14 @@ +apiVersion: v2 +appVersion: 1.0.0 +name: chart-with-dependency-aliased-twice +type: application +version: 1.0.0 + +dependencies: + - name: child + alias: foo + version: 1.0.0 + - name: child + alias: bar + version: 1.0.0 + diff --git a/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/Chart.yaml b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/Chart.yaml new file mode 100644 index 000000000..220fda663 --- /dev/null +++ b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +appVersion: 1.0.0 +name: child +type: application +version: 1.0.0 + diff --git a/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/Chart.yaml b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/Chart.yaml new file mode 100644 index 000000000..50e620a8d --- /dev/null +++ b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +appVersion: 1.0.0 +name: grandchild +type: application +version: 1.0.0 + diff --git a/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/templates/dummy.yaml b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/templates/dummy.yaml new file mode 100644 index 000000000..1830492ef --- /dev/null +++ b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/templates/dummy.yaml @@ -0,0 +1,7 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: {{ .Chart.Name }}-{{ .Values.from }} +data: + {{- toYaml .Values | nindent 2 }} + diff --git a/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/templates/dummy.yaml b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/templates/dummy.yaml new file mode 100644 index 000000000..b5d55af7c --- /dev/null +++ b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/templates/dummy.yaml @@ -0,0 +1,7 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: {{ .Chart.Name }} +data: + {{- toYaml .Values | nindent 2 }} + diff --git a/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/values.yaml b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/values.yaml new file mode 100644 index 000000000..695521a4a --- /dev/null +++ b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/values.yaml @@ -0,0 +1,7 @@ +foo: + grandchild: + from: foo +bar: + grandchild: + from: bar + diff --git a/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/Chart.yaml b/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/Chart.yaml new file mode 100644 index 000000000..c408f0ca8 --- /dev/null +++ b/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/Chart.yaml @@ -0,0 +1,20 @@ +apiVersion: v2 +appVersion: 1.0.0 +name: chart-with-dependency-aliased-twice +type: application +version: 1.0.0 + +dependencies: + - name: child + alias: foo + version: 1.0.0 + import-values: + - parent: foo-defaults + child: defaults + - name: child + alias: bar + version: 1.0.0 + import-values: + - parent: bar-defaults + child: defaults + diff --git a/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/Chart.yaml b/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/Chart.yaml new file mode 100644 index 000000000..ecdaf04dc --- /dev/null +++ b/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/Chart.yaml @@ -0,0 +1,12 @@ +apiVersion: v2 +appVersion: 1.0.0 +name: child +type: application +version: 1.0.0 + +dependencies: + - name: grandchild + version: 1.0.0 + import-values: + - parent: defaults + child: defaults diff --git a/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/charts/grandchild/Chart.yaml b/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/charts/grandchild/Chart.yaml new file mode 100644 index 000000000..50e620a8d --- /dev/null +++ b/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/charts/grandchild/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +appVersion: 1.0.0 +name: grandchild +type: application +version: 1.0.0 + diff --git a/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/charts/grandchild/values.yaml b/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/charts/grandchild/values.yaml new file mode 100644 index 000000000..f51c594f4 --- /dev/null +++ b/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/charts/grandchild/values.yaml @@ -0,0 +1,2 @@ +defaults: + defaultValue: "42" \ No newline at end of file diff --git a/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/templates/dummy.yaml b/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/templates/dummy.yaml new file mode 100644 index 000000000..3140f53dd --- /dev/null +++ b/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/templates/dummy.yaml @@ -0,0 +1,7 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: {{ .Chart.Name }} +data: + {{ .Values.defaults | toYaml }} + diff --git a/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/templates/dummy.yaml b/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/templates/dummy.yaml new file mode 100644 index 000000000..a2b62c95a --- /dev/null +++ b/pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/templates/dummy.yaml @@ -0,0 +1,7 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: {{ .Chart.Name }} +data: + {{ toYaml .Values.defaults | indent 2 }} + diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index 7631e2570..e41226fa4 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -217,7 +217,10 @@ func FindChartInRepoURL(repoURL string, chartName string, getters getter.Provide } cv, err := repoIndex.Get(chartName, opts.ChartVersion) if err != nil { - return "", fmt.Errorf("%s not found in %s repository", errMsg, repoURL) + return "", ChartNotFoundError{ + Chart: errMsg, + RepoURL: repoURL, + } } if len(cv.URLs) == 0 { diff --git a/pkg/repo/chartrepo_test.go b/pkg/repo/chartrepo_test.go index 41bac9827..c29c95a7e 100644 --- a/pkg/repo/chartrepo_test.go +++ b/pkg/repo/chartrepo_test.go @@ -18,6 +18,7 @@ package repo import ( "bytes" + "errors" "net/http" "net/http/httptest" "os" @@ -202,6 +203,9 @@ func TestErrorFindChartInRepoURL(t *testing.T) { } else if err.Error() != `chart "nginx1" not found in `+srv.URL+` repository` { t.Errorf("Expected error for chart not found, but got a different error (%v)", err) } + if !errors.Is(err, ChartNotFoundError{}) { + t.Errorf("error is not of correct error type structure") + } if _, err = FindChartInRepoURL(srv.URL, "nginx1", g, WithChartVersion("0.1.0")); err == nil { t.Errorf("Expected error for chart not found, but did not get any errors") diff --git a/pkg/repo/error.go b/pkg/repo/error.go new file mode 100644 index 000000000..16264ed26 --- /dev/null +++ b/pkg/repo/error.go @@ -0,0 +1,35 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package repo + +import ( + "fmt" +) + +type ChartNotFoundError struct { + RepoURL string + Chart string +} + +func (e ChartNotFoundError) Error() string { + return fmt.Sprintf("%s not found in %s repository", e.Chart, e.RepoURL) +} + +func (e ChartNotFoundError) Is(err error) bool { + _, ok := err.(ChartNotFoundError) + return ok +}