Merge branch 'main' into stdlib-errors-2

Signed-off-by: Justen Stall <39888103+justenstall@users.noreply.github.com>

.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

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()

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 {

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 = copyMetadata(c.Metadata)
-		out.Metadata = &md
+
+		// 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 {

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

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
+

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
+

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
+

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

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

pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/values.yaml

@@ -0,0 +1,7 @@
+foo:
+  grandchild:
+    from: foo
+bar:
+  grandchild:
+    from: bar
+

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
+

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

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
+

pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/charts/grandchild/values.yaml

@@ -0,0 +1,2 @@
+defaults:
+  defaultValue: "42"

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

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

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 {

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")

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