From 3d6f40bd5ef7121bb2e4f6cbcbb30afc683a6d08 Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Tue, 9 May 2023 13:16:22 -0500 Subject: [PATCH] Revert "Fix using same helm chart with different versions (#4999)" This reverts commit 0f244a4a07eca4d4aa7ff7d6a3f2d0f68137c378. --- .../builtins/HelmChartInflationGenerator.go | 18 +-- .../HelmChartInflationGenerator.go | 18 +-- .../HelmChartInflationGenerator_test.go | 137 ------------------ 3 files changed, 6 insertions(+), 167 deletions(-) diff --git a/api/internal/builtins/HelmChartInflationGenerator.go b/api/internal/builtins/HelmChartInflationGenerator.go index 2b0abc6bad..113f56ea7c 100644 --- a/api/internal/builtins/HelmChartInflationGenerator.go +++ b/api/internal/builtins/HelmChartInflationGenerator.go @@ -91,12 +91,7 @@ func (p *HelmChartInflationGeneratorPlugin) validateArgs() (err error) { // be under the loader root (unless root restrictions are // disabled). if p.ValuesFile == "" { - // 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") - } + p.ValuesFile = filepath.Join(p.ChartHome, p.Name, "values.yaml") } for i, file := range p.AdditionalValuesFiles { // use Load() to enforce root restrictions @@ -137,17 +132,10 @@ func (p *HelmChartInflationGeneratorPlugin) errIfIllegalValuesMerge() error { } func (p *HelmChartInflationGeneratorPlugin) absChartHome() string { - var chartHome string if filepath.IsAbs(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 p.ChartHome } - return chartHome + return filepath.Join(p.h.Loader().Root(), p.ChartHome) } func (p *HelmChartInflationGeneratorPlugin) runHelmCommand( diff --git a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go index dbc6c3df72..959528abe7 100644 --- a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go +++ b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go @@ -97,12 +97,7 @@ func (p *plugin) validateArgs() (err error) { // be under the loader root (unless root restrictions are // disabled). if p.ValuesFile == "" { - // 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") - } + p.ValuesFile = filepath.Join(p.ChartHome, p.Name, "values.yaml") } for i, file := range p.AdditionalValuesFiles { // use Load() to enforce root restrictions @@ -143,17 +138,10 @@ func (p *plugin) errIfIllegalValuesMerge() error { } func (p *plugin) absChartHome() string { - var chartHome string if filepath.IsAbs(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 p.ChartHome } - return chartHome + return filepath.Join(p.h.Loader().Root(), p.ChartHome) } func (p *plugin) runHelmCommand( diff --git a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator_test.go b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator_test.go index e59eb97151..0c2b4932c5 100644 --- a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator_test.go +++ b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator_test.go @@ -9,7 +9,6 @@ import ( "path/filepath" "testing" - "github.com/stretchr/testify/assert" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) @@ -580,139 +579,3 @@ 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") -}