Avoid --skip-refresh on local charts (#541)

All the dependencies get correctly installed when dealing with remote
charts.

If there's a local chart that depends on remote dependencies then those
don't get automatically installed. See #526. They end up with this
error:

```
Error: no cached repository for helm-manager-b6cf96b91af4f01317d185adfbe32610179e5246214be9646a52cb0b86032272 found. (try 'helm repo update'): open /root/.cache/helm/repository/helm-manager-b6cf96b91af4f01317d185adfbe32610179e5246214be9646a52cb0b86032272-index.yaml: no such file or directory
```

One workaround for that would be to add the repositories from the local
charts. Something like this:

```
cd local-chart/ && helm dependency list $dir 2> /dev/null | tail +2 | head -n -1 | awk '{ print "helm repo add " $1 " " $3 }' | while read cmd; do $cmd; done
```

This however is not trivial to parse and implement.

An easier fix which I did here is just to not allow doing
`--skip-refresh` for local repositories.

Fixes #526

Signed-off-by: Indrek Juhkam <indrek@urgas.eu>

Signed-off-by: Indrek Juhkam <indrek@urgas.eu>
Signed-off-by: yxxhero <aiopsclub@163.com>

pkg/app/app_test.go

@@ -2554,7 +2554,7 @@ func (helm *mockHelmExec) UpdateDeps(chart string) error {
 	return nil
 }
 
-func (helm *mockHelmExec) BuildDeps(name, chart string) error {
+func (helm *mockHelmExec) BuildDeps(name, chart string, flags ...string) error {
 	return nil
 }
 

pkg/app/mocks_test.go

@@ -39,7 +39,7 @@ func (helm *noCallHelmExec) UpdateDeps(chart string) error {
 	return nil
 }
 
-func (helm *noCallHelmExec) BuildDeps(name, chart string) error {
+func (helm *noCallHelmExec) BuildDeps(name, chart string, flags ...string) error {
 	helm.doPanic()
 	return nil
 }

pkg/exectest/helm.go

@@ -78,7 +78,7 @@ func (helm *Helm) UpdateDeps(chart string) error {
 	return nil
 }
 
-func (helm *Helm) BuildDeps(name, chart string) error {
+func (helm *Helm) BuildDeps(name, chart string, flags ...string) error {
 	if strings.Contains(chart, "error") {
 		return errors.New("error")
 	}

pkg/helmexec/exec.go

@@ -225,7 +225,7 @@ func (helm *execer) RegistryLogin(repository string, username string, password s
 	return err
 }
 
-func (helm *execer) BuildDeps(name, chart string) error {
+func (helm *execer) BuildDeps(name, chart string, flags ...string) error {
 	helm.logger.Infof("Building dependency release=%v, chart=%v", name, chart)
 	args := []string{
 		"dependency",
@@ -233,9 +233,7 @@ func (helm *execer) BuildDeps(name, chart string) error {
 		chart,
 	}
 
-	if helm.IsHelm3() {
+	args = append(args, flags...)
-		args = append(args, "--skip-refresh")
-	}
 
 	out, err := helm.exec(args, map[string]string{}, nil)
 	helm.info(out)

pkg/helmexec/exec_test.go

@@ -363,7 +363,7 @@ func Test_BuildDeps(t *testing.T) {
 	logger := NewLogger(&buffer, "debug")
 	helm3Runner := mockRunner{output: []byte("v3.2.4+ge29ce2a")}
 	helm := New("helm", false, logger, "dev", &helm3Runner)
-	err := helm.BuildDeps("foo", "./chart/foo")
+	err := helm.BuildDeps("foo", "./chart/foo", []string{"--skip-refresh"}...)
 	expected := `Building dependency release=foo, chart=./chart/foo
 exec: helm --kube-context dev dependency build ./chart/foo --skip-refresh
 v3.2.4+ge29ce2a
@@ -376,9 +376,22 @@ v3.2.4+ge29ce2a
 	}
 
 	buffer.Reset()
-	helm.SetExtraArgs("--verify")
 	err = helm.BuildDeps("foo", "./chart/foo")
 	expected = `Building dependency release=foo, chart=./chart/foo
+exec: helm --kube-context dev dependency build ./chart/foo
+v3.2.4+ge29ce2a
+`
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if buffer.String() != expected {
+		t.Errorf("helmexec.BuildDeps()\nactual = %v\nexpect = %v", buffer.String(), expected)
+	}
+
+	buffer.Reset()
+	helm.SetExtraArgs("--verify")
+	err = helm.BuildDeps("foo", "./chart/foo", []string{"--skip-refresh"}...)
+	expected = `Building dependency release=foo, chart=./chart/foo
 exec: helm --kube-context dev dependency build ./chart/foo --skip-refresh --verify
 v3.2.4+ge29ce2a
 `

pkg/helmexec/helmexec.go

@@ -20,7 +20,7 @@ type Interface interface {
 	AddRepo(name, repository, cafile, certfile, keyfile, username, password string, managed string, passCredentials string, skipTLSVerify string) error
 	UpdateRepo() error
 	RegistryLogin(name string, username string, password string) error
-	BuildDeps(name, chart string) error
+	BuildDeps(name, chart string, flags ...string) error
 	UpdateDeps(chart string) error
 	SyncRelease(context HelmContext, name, chart string, flags ...string) error
 	DiffRelease(context HelmContext, name, chart string, suppressDiff bool, flags ...string) error

pkg/state/state.go

@@ -1015,6 +1015,7 @@ type chartPrepareResult struct {
 	chartPath              string
 	err                    error
 	buildDeps              bool
+	skipRefresh            bool
 	chartFetchedByGoGetter bool
 }
 
@@ -1263,6 +1264,7 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre
 					releaseContext:         release.KubeContext,
 					chartPath:              chartPath,
 					buildDeps:              buildDeps,
+					skipRefresh:            !isLocal,
 					chartFetchedByGoGetter: chartFetchedByGoGetter,
 				}
 			}
@@ -1314,7 +1316,8 @@ func (st *HelmState) runHelmDepBuilds(helm helmexec.Interface, concurrency int,
 	//
 	//    See https://github.com/roboll/helmfile/issues/1521
 	for _, r := range builds {
-		if err := helm.BuildDeps(r.releaseName, r.chartPath); err != nil {
+		buildDepsFlags := getBuildDepsFlags(helm, r)
+		if err := helm.BuildDeps(r.releaseName, r.chartPath, buildDepsFlags...); err != nil {
 			if r.chartFetchedByGoGetter {
 				diagnostic := fmt.Sprintf(
 					"WARN: `helm dep build` failed. While processing release %q, Helmfile observed that remote chart %q fetched by go-getter is seemingly broken. "+

pkg/state/util.go

@@ -5,6 +5,8 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+
+	"github.com/helmfile/helmfile/pkg/helmexec"
 )
 
 var (
@@ -62,3 +64,12 @@ func normalizeChart(basePath, chart string) string {
 	}
 	return filepath.Join(basePath, chart)
 }
+
+func getBuildDepsFlags(helm helmexec.Interface, cpr *chartPrepareResult) []string {
+	flags := []string{}
+	if helm.IsHelm3() && cpr.skipRefresh {
+		flags = append(flags, "--skip-refresh")
+	}
+
+	return flags
+}

test/e2e/template/helmfile/snapshot_test.go

@@ -33,19 +33,23 @@ type ociChart struct {
 	digest  string
 }
 
-func TestHelmfileTemplateWithBuildCommand(t *testing.T) {
+type Config struct {
-	type Config struct {
 	LocalDockerRegistry struct {
 		Enabled  bool   `yaml:"enabled"`
 		Port     int    `yaml:"port"`
+		ChartDir string `yaml:"chartDir"`
 	} `yaml:"localDockerRegistry"`
 	LocalChartRepoServer struct {
 		Enabled  bool   `yaml:"enabled"`
 		Port     int    `yaml:"port"`
+		ChartDir string `yaml:"chartDir"`
 	} `yaml:"localChartRepoServer"`
 	ChartifyTempDir string   `yaml:"chartifyTempDir"`
 	HelmfileArgs    []string `yaml:"helmfileArgs"`
-	}
+}
+
+func TestHelmfileTemplateWithBuildCommand(t *testing.T) {
+	localChartPortSets := make(map[int]struct{})
 
 	_, filename, _, _ := runtime.Caller(0)
 	projectRoot := filepath.Join(filepath.Dir(filename), "..", "..", "..", "..")
@@ -54,7 +58,7 @@ func TestHelmfileTemplateWithBuildCommand(t *testing.T) {
 		helmfileBin = helmfileBin + ".exe"
 	}
 	testdataDir := "testdata/snapshot"
-	chartsDir := "testdata/charts"
+	defaultChartsDir := "testdata/charts"
 
 	entries, err := os.ReadDir(testdataDir)
 	require.NoError(t, err)
@@ -81,6 +85,21 @@ func TestHelmfileTemplateWithBuildCommand(t *testing.T) {
 			}
 		}
 
+		if config.LocalChartRepoServer.Enabled {
+			if _, ok := localChartPortSets[config.LocalChartRepoServer.Port]; ok {
+				t.Fatalf("Port %d is already in use", config.LocalChartRepoServer.Port)
+			} else {
+				localChartPortSets[config.LocalChartRepoServer.Port] = struct{}{}
+			}
+			if config.LocalChartRepoServer.ChartDir == "" {
+				config.LocalChartRepoServer.ChartDir = defaultChartsDir
+			}
+			helmtesting.StartChartRepoServer(t, helmtesting.ChartRepoServerConfig{
+				Port:      config.LocalChartRepoServer.Port,
+				ChartsDir: config.LocalChartRepoServer.ChartDir,
+			})
+		}
+
 		// We run `helmfile build` by default.
 		// If you want to test `helmfile template`, set the following in the config.yaml:
 		//
@@ -139,11 +158,14 @@ func TestHelmfileTemplateWithBuildCommand(t *testing.T) {
 
 				// We helm-package and helm-push every test chart saved in the ./testdata/charts directory
 				// to the local registry, so that they can be accessed by helmfile and helm invoked while testing.
-				charts, err := os.ReadDir(chartsDir)
+				if config.LocalDockerRegistry.ChartDir == "" {
+					config.LocalDockerRegistry.ChartDir = defaultChartsDir
+				}
+				charts, err := os.ReadDir(config.LocalDockerRegistry.ChartDir)
 				require.NoError(t, err)
 
 				for _, c := range charts {
-					chartPath := filepath.Join(chartsDir, c.Name())
+					chartPath := filepath.Join(config.LocalDockerRegistry.ChartDir, c.Name())
 					if !c.IsDir() {
 						t.Fatalf("%s is not a directory", c)
 					}
@@ -160,13 +182,6 @@ func TestHelmfileTemplateWithBuildCommand(t *testing.T) {
 				}
 			}
 
-			if config.LocalChartRepoServer.Enabled {
-				helmtesting.StartChartRepoServer(t, helmtesting.ChartRepoServerConfig{
-					Port:      config.LocalChartRepoServer.Port,
-					ChartsDir: chartsDir,
-				})
-			}
-
 			inputFile := filepath.Join(testdataDir, name, "input.yaml")
 			outputFile := filepath.Join(testdataDir, name, "output.yaml")
 

test/e2e/template/helmfile/testdata/snapshot/chart_need/output.yaml

@@ -2,6 +2,9 @@ Adding repo myrepo http://localhost:18080/
 "myrepo" has been added to your repositories
 
 Building dependency release=foo, chart=$WD/temp1/foo
+Hang tight while we grab the latest from your chart repositories...
+...Successfully got an update from the "myrepo" chart repository
+Update Complete. ⎈Happy Helming!⎈
 Saving 1 charts
 Downloading raw from repo http://localhost:18080/
 Deleting outdated charts

test/e2e/template/helmfile/testdata/snapshot/chart_need_enable_live_output/config.yaml

@@ -1,6 +1,6 @@
 localChartRepoServer:
   enabled: true
-  port: 18080
+  port: 18081
 chartifyTempDir: temp1
 helmfileArgs:
 - --enable-live-output

test/e2e/template/helmfile/testdata/snapshot/chart_need_enable_live_output/input.yaml

@@ -1,6 +1,6 @@
 repositories:
 - name: myrepo
-  url: http://localhost:18080/
+  url: http://localhost:18081/
 
 releases:
 - name: foo

test/e2e/template/helmfile/testdata/snapshot/chart_need_enable_live_output/output.yaml

@@ -1,10 +1,13 @@
 Live output is enabled
-Adding repo myrepo http://localhost:18080/
+Adding repo myrepo http://localhost:18081/
 "myrepo" has been added to your repositories
 
 Building dependency release=foo, chart=$WD/temp1/foo
+Hang tight while we grab the latest from your chart repositories...
+...Successfully got an update from the "myrepo" chart repository
+Update Complete. ⎈Happy Helming!⎈
 Saving 1 charts
-Downloading raw from repo http://localhost:18080/
+Downloading raw from repo http://localhost:18081/
 Deleting outdated charts
 
 Templating release=foo, chart=$WD/temp1/foo

test/e2e/template/helmfile/testdata/snapshot/oci_need/output.yaml

@@ -1,4 +1,8 @@
 Building dependency release=foo, chart=$WD/temp1/foo
+Hang tight while we grab the latest from your chart repositories...
+...Successfully got an update from the "myrepo" chart repository
+...Successfully got an update from the "istio" chart repository
+Update Complete. ⎈Happy Helming!⎈
 Saving 1 charts
 Downloading raw from repo oci://localhost:5000/myrepo
 Deleting outdated charts

test/e2e/template/helmfile/testdata/snapshot/pr_560/output.yaml

@@ -51,7 +51,7 @@ merged environment: &{default map[] map[]}
 helm> $HelmVersion
 helm> 
 Building dependency release=foo, chart=../../charts/raw-0.1.0
-exec: helm dependency build ../../charts/raw-0.1.0 --skip-refresh
+exec: helm dependency build ../../charts/raw-0.1.0
 1 release(s) found in input.yaml
 
 processing 1 groups of releases in this order:

test/e2e/template/helmfile/testdata/snapshot/templated_lockfile/config.yaml

@@ -1,6 +1,6 @@
 localChartRepoServer:
   enabled: true
-  port: 18080
+  port: 18082
 chartifyTempDir: temp1
 helmfileArgs:
 - --environment

test/e2e/template/helmfile/testdata/snapshot/templated_lockfile/input.yaml

@@ -1,6 +1,6 @@
 repositories:
 - name: myrepo
-  url: http://localhost:18080/
+  url: http://localhost:18082/
 
 environments:
   prod:

test/e2e/template/helmfile/testdata/snapshot/templated_lockfile/output.yaml

@@ -1,4 +1,4 @@
-Adding repo myrepo http://localhost:18080/
+Adding repo myrepo http://localhost:18082/
 "myrepo" has been added to your repositories
 
 Templating release=raw, chart=myrepo/raw