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>
2022-12-12 09:43:10 +02:00 · 2022-12-12 09:43:10 +02:00 · bdc6982172
parent 2a18134986
commit bdc6982172
18 changed files with 91 additions and 41 deletions
--- a/pkg/app/app_test.go
+++ b/pkg/app/app_test.go
@ -2545,7 +2545,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
 }

--- a/pkg/app/mocks_test.go
+++ b/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
 }
--- a/pkg/exectest/helm.go
+++ b/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")
 	}
--- a/pkg/helmexec/exec.go
+++ b/pkg/helmexec/exec.go
@ -216,7 +216,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",
@ -224,9 +224,7 @@ func (helm *execer) BuildDeps(name, chart string) error {
 		chart,
 	}

-	if helm.IsHelm3() {
-		args = append(args, "--skip-refresh")
-	}
+	args = append(args, flags...)

 	out, err := helm.exec(args, map[string]string{}, nil)
 	helm.info(out)
--- a/pkg/helmexec/exec_test.go
+++ b/pkg/helmexec/exec_test.go
@ -339,7 +339,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
@ -352,9 +352,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
 `
--- a/pkg/helmexec/helmexec.go
+++ b/pkg/helmexec/helmexec.go
@ -18,7 +18,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
--- a/pkg/state/state.go
+++ b/pkg/state/state.go
@ -1010,6 +1010,7 @@ type chartPrepareResult struct {
 	chartPath              string
 	err                    error
 	buildDeps              bool
+	skipRefresh            bool
 	chartFetchedByGoGetter bool
 }

@ -1258,6 +1259,7 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre
 					releaseContext:         release.KubeContext,
 					chartPath:              chartPath,
 					buildDeps:              buildDeps,
+					skipRefresh:            !isLocal,
 					chartFetchedByGoGetter: chartFetchedByGoGetter,
 				}
 			}
@ -1309,7 +1311,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. "+
--- a/pkg/state/util.go
+++ b/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
+}
--- a/test/e2e/template/helmfile/snapshot_test.go
+++ b/test/e2e/template/helmfile/snapshot_test.go
@ -33,19 +33,23 @@ type ociChart struct {
 	digest  string
 }

+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) {
-	type Config struct {
-		LocalDockerRegistry struct {
-			Enabled bool `yaml:"enabled"`
-			Port    int  `yaml:"port"`
-		} `yaml:"localDockerRegistry"`
-		LocalChartRepoServer struct {
-			Enabled bool `yaml:"enabled"`
-			Port    int  `yaml:"port"`
-		} `yaml:"localChartRepoServer"`
-		ChartifyTempDir string   `yaml:"chartifyTempDir"`
-		HelmfileArgs    []string `yaml:"helmfileArgs"`
-	}
+	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")

--- a/test/e2e/template/helmfile/testdata/snapshot/chart_need/output.yaml
+++ b/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
--- a/test/e2e/template/helmfile/testdata/snapshot/chart_need_enable_live_output/config.yaml
+++ b/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
--- a/test/e2e/template/helmfile/testdata/snapshot/chart_need_enable_live_output/input.yaml
+++ b/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
--- a/test/e2e/template/helmfile/testdata/snapshot/chart_need_enable_live_output/output.yaml
+++ b/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
--- a/test/e2e/template/helmfile/testdata/snapshot/oci_need/output.yaml
+++ b/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
--- a/test/e2e/template/helmfile/testdata/snapshot/pr_560/output.yaml
+++ b/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:
--- a/test/e2e/template/helmfile/testdata/snapshot/templated_lockfile/config.yaml
+++ b/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
--- a/test/e2e/template/helmfile/testdata/snapshot/templated_lockfile/input.yaml
+++ b/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:
--- a/test/e2e/template/helmfile/testdata/snapshot/templated_lockfile/output.yaml
+++ b/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