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