From fb21d634d4be81729b9b6cd034733b756f76282e Mon Sep 17 00:00:00 2001 From: Dustin Lish Date: Mon, 24 Apr 2023 16:09:02 -0600 Subject: [PATCH] Fix using same helm chart with different versions (#4999) * Fix using same helm chart with different versions * Fix p.ValuesFile when version is set * Updated: Fix using same helm chart with different versions * Add test for issue #4813 * Use if/else for readability, add version check to absChartHome --- .../builtins/HelmChartInflationGenerator.go | 18 ++- .../HelmChartInflationGenerator.go | 18 ++- .../HelmChartInflationGenerator_test.go | 137 ++++++++++++++++++ 3 files changed, 167 insertions(+), 6 deletions(-) diff --git a/api/internal/builtins/HelmChartInflationGenerator.go b/api/internal/builtins/HelmChartInflationGenerator.go index 113f56ea7cf..2b0abc6bad3 100644 --- a/api/internal/builtins/HelmChartInflationGenerator.go +++ b/api/internal/builtins/HelmChartInflationGenerator.go @@ -91,7 +91,12 @@ func (p *HelmChartInflationGeneratorPlugin) validateArgs() (err error) { // be under the loader root (unless root restrictions are // disabled). if p.ValuesFile == "" { - p.ValuesFile = filepath.Join(p.ChartHome, p.Name, "values.yaml") + // If the version is specified, use the versioned values file. + if p.Version != "" { + p.ValuesFile = filepath.Join(p.ChartHome, fmt.Sprintf("%s-%s", p.Name, p.Version), p.Name, "values.yaml") + } else { + p.ValuesFile = filepath.Join(p.ChartHome, p.Name, "values.yaml") + } } for i, file := range p.AdditionalValuesFiles { // use Load() to enforce root restrictions @@ -132,10 +137,17 @@ func (p *HelmChartInflationGeneratorPlugin) errIfIllegalValuesMerge() error { } func (p *HelmChartInflationGeneratorPlugin) absChartHome() string { + var chartHome string if filepath.IsAbs(p.ChartHome) { - return p.ChartHome + chartHome = p.ChartHome + } else { + chartHome = filepath.Join(p.h.Loader().Root(), p.ChartHome) + } + + if p.Version != "" { + return filepath.Join(chartHome, fmt.Sprintf("%s-%s", p.Name, p.Version)) } - return filepath.Join(p.h.Loader().Root(), p.ChartHome) + return chartHome } func (p *HelmChartInflationGeneratorPlugin) runHelmCommand( diff --git a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go index 959528abe78..dbc6c3df726 100644 --- a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go +++ b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go @@ -97,7 +97,12 @@ func (p *plugin) validateArgs() (err error) { // be under the loader root (unless root restrictions are // disabled). if p.ValuesFile == "" { - p.ValuesFile = filepath.Join(p.ChartHome, p.Name, "values.yaml") + // If the version is specified, use the versioned values file. + if p.Version != "" { + p.ValuesFile = filepath.Join(p.ChartHome, fmt.Sprintf("%s-%s", p.Name, p.Version), p.Name, "values.yaml") + } else { + p.ValuesFile = filepath.Join(p.ChartHome, p.Name, "values.yaml") + } } for i, file := range p.AdditionalValuesFiles { // use Load() to enforce root restrictions @@ -138,10 +143,17 @@ func (p *plugin) errIfIllegalValuesMerge() error { } func (p *plugin) absChartHome() string { + var chartHome string if filepath.IsAbs(p.ChartHome) { - return p.ChartHome + chartHome = p.ChartHome + } else { + chartHome = filepath.Join(p.h.Loader().Root(), p.ChartHome) + } + + if p.Version != "" { + return filepath.Join(chartHome, fmt.Sprintf("%s-%s", p.Name, p.Version)) } - return filepath.Join(p.h.Loader().Root(), p.ChartHome) + return chartHome } func (p *plugin) runHelmCommand( diff --git a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator_test.go b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator_test.go index 0c2b4932c53..e59eb97151d 100644 --- a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator_test.go +++ b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator_test.go @@ -9,6 +9,7 @@ import ( "path/filepath" "testing" + "github.com/stretchr/testify/assert" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) @@ -579,3 +580,139 @@ valuesInline: `) th.AssertActualEqualsExpected(rm, "") } + +func TestHelmChartInflationGeneratorWithSameChartMultipleVersions(t *testing.T) { + th := kusttest_test.MakeEnhancedHarnessWithTmpRoot(t). + PrepBuiltin("HelmChartInflationGenerator") + defer th.Reset() + if err := th.ErrIfNoHelm(); err != nil { + t.Skip("skipping: " + err.Error()) + } + + tests := []struct { + name string + chartName string + repo string + version string + releaseName string + }{ + { + name: "terraform chart with no version grabs latest", + chartName: "terraform", + repo: "https://helm.releases.hashicorp.com", + version: "", + releaseName: "terraform-latest", + }, + { + name: "terraform chart with version 1.1.1", + chartName: "terraform", + repo: "https://helm.releases.hashicorp.com", + version: "1.1.1", + releaseName: "terraform-1.1.1", + }, + { + name: "terraform chart with version 1.1.1 again", + chartName: "terraform", + repo: "https://helm.releases.hashicorp.com", + version: "1.1.1", + releaseName: "terraform-1.1.1-1", + }, + { + name: "terraform chart with version 1.1.2", + chartName: "terraform", + repo: "https://helm.releases.hashicorp.com", + version: "1.1.2", + releaseName: "terraform-1.1.2", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config := fmt.Sprintf(` +apiVersion: builtin +kind: HelmChartInflationGenerator +metadata: + name: %s +name: %s +version: %s +repo: %s +releaseName: %s +`, tt.chartName, tt.chartName, tt.version, tt.repo, tt.releaseName) + + rm := th.LoadAndRunGenerator(config) + assert.True(t, len(rm.Resources()) > 0) + + var chartDir string + if tt.version != "" { + chartDir = fmt.Sprintf("charts/%s-%s/%s", tt.chartName, tt.version, tt.chartName) + } else { + chartDir = fmt.Sprintf("charts/%s", tt.chartName) + } + + d, err := th.GetFSys().ReadFile(filepath.Join(th.GetRoot(), chartDir, "Chart.yaml")) + if err != nil { + t.Fatal(err) + } + + assert.Contains(t, string(d), fmt.Sprintf("name: %s", tt.chartName)) + if tt.version != "" { + assert.Contains(t, string(d), fmt.Sprintf("version: %s", tt.version)) + } + }) + } +} + +// Test that verifies +1 instances of same chart with different versions +// https://github.com/kubernetes-sigs/kustomize/issues/4813 +func TestHelmChartInflationGeneratorWithMultipleInstancesSameChartDifferentVersions(t *testing.T) { + th := kusttest_test.MakeEnhancedHarnessWithTmpRoot(t). + PrepBuiltin("HelmChartInflationGenerator") + defer th.Reset() + if err := th.ErrIfNoHelm(); err != nil { + t.Skip("skipping: " + err.Error()) + } + + podinfo1 := th.LoadAndRunGenerator(` +apiVersion: builtin +kind: HelmChartInflationGenerator +metadata: + name: podinfo +name: podinfo +version: 6.2.1 +repo: https://stefanprodan.github.io/podinfo +releaseName: podinfo1 +`) + + podinfo2 := th.LoadAndRunGenerator(` +apiVersion: builtin +kind: HelmChartInflationGenerator +metadata: + name: podinfo +name: podinfo +version: 6.1.8 +repo: https://stefanprodan.github.io/podinfo +releaseName: podinfo2 +`) + + podinfo1Img, err := podinfo1.Resources()[1].GetFieldValue("spec.template.spec.containers.0.image") + assert.NoError(t, err) + assert.Equal(t, "ghcr.io/stefanprodan/podinfo:6.2.1", podinfo1Img) + + podinfo2Img, err := podinfo2.Resources()[1].GetFieldValue("spec.template.spec.containers.0.image") + assert.NoError(t, err) + assert.Equal(t, "ghcr.io/stefanprodan/podinfo:6.1.8", podinfo2Img) + + podinfo1ChartsDir := filepath.Join(th.GetRoot(), "charts/podinfo-6.2.1/podinfo") + assert.True(t, th.GetFSys().Exists(podinfo1ChartsDir)) + + podinfo2ChartsDir := filepath.Join(th.GetRoot(), "charts/podinfo-6.1.8/podinfo") + assert.True(t, th.GetFSys().Exists(podinfo2ChartsDir)) + + podinfo1ChartContents, err := th.GetFSys().ReadFile(filepath.Join(podinfo1ChartsDir, "Chart.yaml")) + assert.NoError(t, err) + assert.Contains(t, string(podinfo1ChartContents), "version: 6.2.1") + + podinfo2ChartContents, err := th.GetFSys().ReadFile(filepath.Join(podinfo2ChartsDir, "Chart.yaml")) + assert.NoError(t, err) + assert.Contains(t, string(podinfo2ChartContents), "version: 6.1.8") +}