fix(oci): clean dead code (#290)

fix(oci): remove dead code

Signed-off-by: Jean-Yves CAMIER <jycamier@gmail.com>
Signed-off-by: Yusuke Kuoka <ykuoka@gmail.com>
Co-authored-by: Yusuke Kuoka <ykuoka@gmail.com>

pkg/app/app_test.go

@@ -2518,7 +2518,7 @@ func (helm *mockHelmExec) TemplateRelease(name, chart string, flags ...string) e
 	return nil
 }
 
-func (helm *mockHelmExec) ChartPull(chart string, flags ...string) error {
+func (helm *mockHelmExec) ChartPull(chart string, path string, flags ...string) error {
 	return nil
 }
 

pkg/app/mocks_test.go

@@ -22,7 +22,7 @@ func (helm *noCallHelmExec) TemplateRelease(name, chart string, flags ...string)
 	helm.doPanic()
 	return nil
 }
-func (helm *noCallHelmExec) ChartPull(chart string, flags ...string) error {
+func (helm *noCallHelmExec) ChartPull(chart string, path string, flags ...string) error {
 	helm.doPanic()
 	return nil
 }

pkg/exectest/helm.go

@@ -192,7 +192,7 @@ func (helm *Helm) TemplateRelease(name, chart string, flags ...string) error {
 	helm.Templated = append(helm.Templated, Release{Name: name, Flags: flags})
 	return nil
 }
-func (helm *Helm) ChartPull(chart string, flags ...string) error {
+func (helm *Helm) ChartPull(chart string, path string, flags ...string) error {
 	return nil
 }
 func (helm *Helm) ChartExport(chart string, path string, flags ...string) error {

pkg/helmexec/exec.go

@@ -458,21 +458,15 @@ func (helm *execer) Fetch(chart string, flags ...string) error {
 	return err
 }
 
-func (helm *execer) ChartPull(chart string, flags ...string) error {
+func (helm *execer) ChartPull(chart string, path string, flags ...string) error {
-	helm.logger.Infof("Pulling %v", chart)
-	helm.logger.Infof("Exporting %v", chart)
-	helmVersionConstraint, _ := semver.NewConstraint(">= 3.7.0")
 	var helmArgs []string
+	helm.logger.Infof("Pulling %v", chart)
+	helmVersionConstraint, _ := semver.NewConstraint(">= 3.7.0")
 	if helmVersionConstraint.Check(&helm.version) {
+		// in the 3.7.0 version, the chart pull has been replaced with helm pull
+		// https://github.com/helm/helm/releases/tag/v3.7.0
 		ociChartURL, ociChartTag := resolveOciChart(chart)
-		tempDir, err := os.MkdirTemp("", "chart*")
+		helmArgs = []string{"pull", ociChartURL, "--version", ociChartTag, "--destination", path}
-		if err != nil {
-			return err
-		}
-		defer func() {
-			_ = os.RemoveAll(tempDir)
-		}()
-		helmArgs = []string{"fetch", ociChartURL, "--version", ociChartTag, "--destination", tempDir}
 	} else {
 		helmArgs = []string{"chart", "pull", chart}
 	}
@@ -482,16 +476,16 @@ func (helm *execer) ChartPull(chart string, flags ...string) error {
 }
 
 func (helm *execer) ChartExport(chart string, path string, flags ...string) error {
-	helm.logger.Infof("Exporting %v", chart)
 	helmVersionConstraint, _ := semver.NewConstraint(">= 3.7.0")
-	var helmArgs []string
 	if helmVersionConstraint.Check(&helm.version) {
-		ociChartURL, ociChartTag := resolveOciChart(chart)
+		// in the 3.7.0 version, the chart export has been removed
-		helmArgs = []string{"pull", ociChartURL, "--version", ociChartTag, "--untar"}
+		// https://github.com/helm/helm/releases/tag/v3.7.0
-	} else {
+		return nil
-		helmArgs = []string{"chart", "export", chart}
 	}
-	out, err := helm.exec(append(append(helmArgs, "--destination", path), flags...), map[string]string{"HELM_EXPERIMENTAL_OCI": "1"}, nil)
+	var helmArgs []string
+	helm.logger.Infof("Exporting %v", chart)
+	helmArgs = []string{"chart", "export", chart, "--destination", path}
+	out, err := helm.exec(append(helmArgs, flags...), map[string]string{"HELM_EXPERIMENTAL_OCI": "1"}, nil)
 	helm.info(out)
 	return err
 }

pkg/helmexec/exec_test.go

@@ -710,6 +710,7 @@ func Test_ChartPull(t *testing.T) {
 		helmBin     string
 		helmVersion string
 		chartName   string
+		chartPath   string
 		chartFlags  []string
 		listResult  string
 	}{
@@ -718,9 +719,9 @@ func Test_ChartPull(t *testing.T) {
 			helmBin:     "helm",
 			helmVersion: "v3.6.0",
 			chartName:   "chart",
+			chartPath:   "path1",
 			chartFlags:  []string{"--untar", "--untardir", "/tmp/dir"},
 			listResult: `Pulling chart
-Exporting chart
 exec: helm --kube-context dev chart pull chart --untar --untardir /tmp/dir
 `,
 		},
@@ -729,10 +730,10 @@ exec: helm --kube-context dev chart pull chart --untar --untardir /tmp/dir
 			helmBin:     "helm",
 			helmVersion: "v3.9.0",
 			chartName:   "repo/helm-charts:0.14.0",
+			chartPath:   "path1",
 			chartFlags:  []string{"--untardir", "/tmp/dir"},
 			listResult: `Pulling repo/helm-charts:0.14.0
-Exporting repo/helm-charts:0.14.0
+exec: helm --kube-context dev pull oci://repo/helm-charts --version 0.14.0 --destination path1 --untardir /tmp/dir
-exec: helm --kube-context dev fetch oci://repo/helm-charts --version 0.14.0 --destination [\w/]+ --untardir /tmp/dir
 `,
 		},
 	}
@@ -747,7 +748,7 @@ exec: helm --kube-context dev fetch oci://repo/helm-charts --version 0.14.0 --de
 				kubeContext: "dev",
 				runner:      &mockRunner{},
 			}
-			err := helm.ChartPull(tt.chartName, tt.chartFlags...)
+			err := helm.ChartPull(tt.chartName, tt.chartPath, tt.chartFlags...)
 			if err != nil {
 				t.Errorf("unexpected error: %v", err)
 			}
@@ -781,18 +782,6 @@ func Test_ChartExport(t *testing.T) {
 			chartFlags:  []string{"--untar", "--untardir", "/tmp/dir"},
 			listResult: `Exporting chart
 exec: helm --kube-context dev chart export chart --destination path1 --untar --untardir /tmp/dir
-`,
-			expectedError: "",
-		},
-		{
-			name:        "",
-			helmBin:     "helm",
-			helmVersion: "v3.9.0",
-			chartName:   "repo/helm-charts:0.14.0",
-			chartPath:   "path1",
-			chartFlags:  []string{"--untardir", "/tmp/dir"},
-			listResult: `Exporting repo/helm-charts:0.14.0
-exec: helm --kube-context dev pull oci://repo/helm-charts --version 0.14.0 --untar --destination path1 --untardir /tmp/dir
 `,
 			expectedError: "",
 		},

pkg/helmexec/helmexec.go

@@ -22,7 +22,7 @@ type Interface interface {
 	DiffRelease(context HelmContext, name, chart string, suppressDiff bool, flags ...string) error
 	TemplateRelease(name, chart string, flags ...string) error
 	Fetch(chart string, flags ...string) error
-	ChartPull(chart string, flags ...string) error
+	ChartPull(chart string, path string, flags ...string) error
 	ChartExport(chart string, path string, flags ...string) error
 	Lint(name, chart string, flags ...string) error
 	ReleaseStatus(context HelmContext, name string, flags ...string) error

pkg/state/state.go

@@ -80,12 +80,6 @@ type ReleaseSetSpec struct {
 	MissingFileHandler string `yaml:"missingFileHandler,omitempty"`
 }
 
-// PullCommand is a command to pull a chart
-type PullCommand struct {
-	ChartRef     string
-	responseChan chan error
-}
-
 // HelmState structure for the helmfile
 type HelmState struct {
 	basePath string
@@ -1064,11 +1058,6 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre
 	}
 
 	var builds []*chartPrepareResult
-	pullChan := make(chan PullCommand)
-	defer func() {
-		close(pullChan)
-	}()
-	go st.pullChartWorker(pullChan, helm)
 
 	st.scatterGather(
 		concurrency,
@@ -1104,7 +1093,7 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre
 				chartFetchedByGoGetter := chartPath != chartName
 
 				if !chartFetchedByGoGetter {
-					ociChartPath, err := st.getOCIChart(pullChan, release, dir, helm)
+					ociChartPath, err := st.getOCIChart(release, dir, helm)
 					if err != nil {
 						results <- &chartPrepareResult{err: fmt.Errorf("release %q: %w", release.Name, err)}
 
@@ -3286,7 +3275,7 @@ func (st *HelmState) Reverse() {
 	}
 }
 
-func (st *HelmState) getOCIChart(pullChan chan PullCommand, release *ReleaseSpec, tempDir string, helm helmexec.Interface) (*string, error) {
+func (st *HelmState) getOCIChart(release *ReleaseSpec, tempDir string, helm helmexec.Interface) (*string, error) {
 	repo, name := st.GetRepositoryAndNameFromChartName(release.Chart)
 	if repo == nil {
 		return nil, nil
@@ -3303,11 +3292,6 @@ func (st *HelmState) getOCIChart(pullChan chan PullCommand, release *ReleaseSpec
 
 	qualifiedChartName := fmt.Sprintf("%s/%s:%s", repo.URL, name, chartVersion)
 
-	err := st.pullChart(pullChan, qualifiedChartName)
-	if err != nil {
-		return nil, err
-	}
-
 	pathElems := []string{
 		tempDir,
 	}
@@ -3323,6 +3307,11 @@ func (st *HelmState) getOCIChart(pullChan chan PullCommand, release *ReleaseSpec
 	pathElems = append(pathElems, release.Name, name, chartVersion)
 
 	chartPath := path.Join(pathElems...)
+
+	err := helm.ChartPull(qualifiedChartName, chartPath)
+	if err != nil {
+		return nil, err
+	}
 	err = helm.ChartExport(qualifiedChartName, chartPath)
 	if err != nil {
 		return nil, err
@@ -3337,22 +3326,3 @@ func (st *HelmState) getOCIChart(pullChan chan PullCommand, release *ReleaseSpec
 
 	return &chartPath, nil
 }
-
-// Pull charts one by one to prevent concurrent pull problems with Helm
-func (st *HelmState) pullChartWorker(pullChan chan PullCommand, helm helmexec.Interface) {
-	for pullCmd := range pullChan {
-		err := helm.ChartPull(pullCmd.ChartRef)
-		pullCmd.responseChan <- err
-	}
-}
-
-// Send a pull command to the pull worker
-func (st *HelmState) pullChart(pullChan chan PullCommand, chartRef string) error {
-	response := make(chan error, 1)
-	cmd := PullCommand{
-		responseChan: response,
-		ChartRef:     chartRef,
-	}
-	pullChan <- cmd
-	return <-response
-}