From df7befd208e71e152884cc455b545bab5e011d3c Mon Sep 17 00:00:00 2001
From: Daniel Strobusch <1847260+dastrobu@users.noreply.github.com>
Date: Wed, 23 Dec 2020 16:37:31 +0100
Subject: [PATCH 1/2] copy dependencies on aliasing to avoid sharing chart
 references on multiply 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 #9150

Signed-off-by: Daniel Strobusch <1847260+dastrobu@users.noreply.github.com>
---
 pkg/chart/v2/chart.go                         |  2 ++
 pkg/chart/v2/util/dependencies.go             |  9 ++++++
 pkg/chart/v2/util/dependencies_test.go        | 32 +++++++++++++++++++
 .../Chart.yaml                                | 14 ++++++++
 .../charts/child/Chart.yaml                   |  6 ++++
 .../charts/child/charts/grandchild/Chart.yaml |  6 ++++
 .../charts/grandchild/templates/dummy.yaml    |  7 ++++
 .../charts/child/templates/dummy.yaml         |  7 ++++
 .../values.yaml                               |  7 ++++
 9 files changed, 90 insertions(+)
 create mode 100644 pkg/chartutil/testdata/chart-with-dependency-aliased-twice/Chart.yaml
 create mode 100644 pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/Chart.yaml
 create mode 100644 pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/Chart.yaml
 create mode 100644 pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/templates/dummy.yaml
 create mode 100644 pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/templates/dummy.yaml
 create mode 100644 pkg/chartutil/testdata/chart-with-dependency-aliased-twice/values.yaml

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/dependencies.go b/pkg/chart/v2/util/dependencies.go
index b7f78010b..a9b53baec 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 {
@@ -107,6 +108,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
 		}
diff --git a/pkg/chart/v2/util/dependencies_test.go b/pkg/chart/v2/util/dependencies_test.go
index 5bd332990..ca59a3eae 100644
--- a/pkg/chart/v2/util/dependencies_test.go
+++ b/pkg/chart/v2/util/dependencies_test.go
@@ -430,6 +430,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 +524,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/chartutil/testdata/chart-with-dependency-aliased-twice/Chart.yaml b/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/Chart.yaml
new file mode 100644
index 000000000..d778f8fe9
--- /dev/null
+++ b/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
+
diff --git a/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/Chart.yaml b/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/Chart.yaml
new file mode 100644
index 000000000..220fda663
--- /dev/null
+++ b/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
+
diff --git a/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/Chart.yaml b/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/Chart.yaml
new file mode 100644
index 000000000..50e620a8d
--- /dev/null
+++ b/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
+
diff --git a/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/templates/dummy.yaml b/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/templates/dummy.yaml
new file mode 100644
index 000000000..1830492ef
--- /dev/null
+++ b/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 }}
+
diff --git a/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/templates/dummy.yaml b/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/templates/dummy.yaml
new file mode 100644
index 000000000..b5d55af7c
--- /dev/null
+++ b/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 }}
+
diff --git a/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/values.yaml b/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/values.yaml
new file mode 100644
index 000000000..695521a4a
--- /dev/null
+++ b/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/values.yaml
@@ -0,0 +1,7 @@
+foo:
+  grandchild:
+    from: foo
+bar:
+  grandchild:
+    from: bar
+

From b183eccfc40922cb8053fef5459cab63ea47a309 Mon Sep 17 00:00:00 2001
From: Daniel Strobusch <1847260+dastrobu@users.noreply.github.com>
Date: Thu, 8 Dec 2022 18:35:55 +0100
Subject: [PATCH 2/2] copy dependency metadata on aliasing to avoid sharing
 imported values

imported values are stored in dependency objects, which breaks if a chart dependency is shared among multiple aliases.
By copying the dependency objects in the metadata values can be imported correctly.

Supersedes #10174

Signed-off-by: Daniel Strobusch <1847260+dastrobu@users.noreply.github.com>
---
 pkg/chart/v2/util/dependencies.go             | 19 +++++++++--
 pkg/chart/v2/util/dependencies_test.go        | 32 +++++++++++++++++++
 .../Chart.yaml                                |  0
 .../charts/child/Chart.yaml                   |  0
 .../charts/child/charts/grandchild/Chart.yaml |  0
 .../charts/grandchild/templates/dummy.yaml    |  0
 .../charts/child/templates/dummy.yaml         |  0
 .../values.yaml                               |  0
 .../Chart.yaml                                | 20 ++++++++++++
 .../charts/child/Chart.yaml                   | 12 +++++++
 .../charts/child/charts/grandchild/Chart.yaml |  6 ++++
 .../child/charts/grandchild/values.yaml       |  2 ++
 .../charts/child/templates/dummy.yaml         |  7 ++++
 .../templates/dummy.yaml                      |  7 ++++
 14 files changed, 102 insertions(+), 3 deletions(-)
 rename pkg/{chartutil => chart/v2/util}/testdata/chart-with-dependency-aliased-twice/Chart.yaml (100%)
 rename pkg/{chartutil => chart/v2/util}/testdata/chart-with-dependency-aliased-twice/charts/child/Chart.yaml (100%)
 rename pkg/{chartutil => chart/v2/util}/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/Chart.yaml (100%)
 rename pkg/{chartutil => chart/v2/util}/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/templates/dummy.yaml (100%)
 rename pkg/{chartutil => chart/v2/util}/testdata/chart-with-dependency-aliased-twice/charts/child/templates/dummy.yaml (100%)
 rename pkg/{chartutil => chart/v2/util}/testdata/chart-with-dependency-aliased-twice/values.yaml (100%)
 create mode 100644 pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/Chart.yaml
 create mode 100644 pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/Chart.yaml
 create mode 100644 pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/charts/grandchild/Chart.yaml
 create mode 100644 pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/charts/grandchild/values.yaml
 create mode 100644 pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/charts/child/templates/dummy.yaml
 create mode 100644 pkg/chart/v2/util/testdata/chart-with-import-from-aliased-dependencies/templates/dummy.yaml

diff --git a/pkg/chart/v2/util/dependencies.go b/pkg/chart/v2/util/dependencies.go
index a9b53baec..e2cce6f2f 100644
--- a/pkg/chart/v2/util/dependencies.go
+++ b/pkg/chart/v2/util/dependencies.go
@@ -105,8 +105,7 @@ 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
@@ -117,13 +116,27 @@ func getAliasDependency(charts []*chart.Chart, dep *chart.Dependency) *chart.Cha
 		}
 
 		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 ca59a3eae..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")
 
diff --git a/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/Chart.yaml b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/Chart.yaml
similarity index 100%
rename from pkg/chartutil/testdata/chart-with-dependency-aliased-twice/Chart.yaml
rename to pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/Chart.yaml
diff --git a/pkg/chartutil/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
similarity index 100%
rename from pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/Chart.yaml
rename to pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/Chart.yaml
diff --git a/pkg/chartutil/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
similarity index 100%
rename from pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/Chart.yaml
rename to pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/Chart.yaml
diff --git a/pkg/chartutil/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
similarity index 100%
rename from pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/templates/dummy.yaml
rename to pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/charts/grandchild/templates/dummy.yaml
diff --git a/pkg/chartutil/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
similarity index 100%
rename from pkg/chartutil/testdata/chart-with-dependency-aliased-twice/charts/child/templates/dummy.yaml
rename to pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/charts/child/templates/dummy.yaml
diff --git a/pkg/chartutil/testdata/chart-with-dependency-aliased-twice/values.yaml b/pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/values.yaml
similarity index 100%
rename from pkg/chartutil/testdata/chart-with-dependency-aliased-twice/values.yaml
rename to pkg/chart/v2/util/testdata/chart-with-dependency-aliased-twice/values.yaml
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 }}
+