copy dependencies on aliasing to avoid sharing chart references on mu…

…ltiply aliased dependencies

Dependencies keep a reference on their parent chart, which breaks if a chart reference is shared among multiple aliases.
By copying the dependencies, parent information can be set correctly to render the templates as expected later on.

Note that this change will make ChartFullPath return a different path for sub-subcharts. It will contain the alias names instead of the path to the chart files which makes it consistent with paths to templates on the subchart level.

Closes helm#9150

Signed-off-by: Daniel Strobusch <1847260+dastrobu@users.noreply.github.com>

pkg/chart/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/chartutil/dependencies.go

@@ -90,6 +90,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 {
@@ -106,6 +107,14 @@ func getAliasDependency(charts []*chart.Chart, dep *chart.Dependency) *chart.Cha
 		md := *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
 		}

pkg/chartutil/dependencies_test.go

@@ -364,6 +364,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())
@@ -455,3 +458,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/chartutil/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/chartutil/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/chartutil/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/chartutil/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/chartutil/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/chartutil/testdata/chart-with-dependency-aliased-twice/values.yaml

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