From bdc69821723ec5949f63bbefcc91a466f0d3b552 Mon Sep 17 00:00:00 2001 From: Indrek Juhkam Date: Mon, 12 Dec 2022 09:43:10 +0200 Subject: [PATCH] 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 Signed-off-by: Indrek Juhkam --- pkg/app/app_test.go | 2 +- pkg/app/mocks_test.go | 2 +- pkg/exectest/helm.go | 2 +- pkg/helmexec/exec.go | 6 +- pkg/helmexec/exec_test.go | 17 +++++- pkg/helmexec/helmexec.go | 2 +- pkg/state/state.go | 5 +- pkg/state/util.go | 11 ++++ test/e2e/template/helmfile/snapshot_test.go | 59 ++++++++++++------- .../testdata/snapshot/chart_need/output.yaml | 3 + .../chart_need_enable_live_output/config.yaml | 2 +- .../chart_need_enable_live_output/input.yaml | 2 +- .../chart_need_enable_live_output/output.yaml | 7 ++- .../testdata/snapshot/oci_need/output.yaml | 4 ++ .../testdata/snapshot/pr_560/output.yaml | 2 +- .../snapshot/templated_lockfile/config.yaml | 2 +- .../snapshot/templated_lockfile/input.yaml | 2 +- .../snapshot/templated_lockfile/output.yaml | 2 +- 18 files changed, 91 insertions(+), 41 deletions(-) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index f561ad15..c5bb34d9 100644 --- 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 } diff --git a/pkg/app/mocks_test.go b/pkg/app/mocks_test.go index a6c28a57..28cb5832 100644 --- 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 } diff --git a/pkg/exectest/helm.go b/pkg/exectest/helm.go index 31741c12..b60607af 100644 --- 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") } diff --git a/pkg/helmexec/exec.go b/pkg/helmexec/exec.go index 3ad41525..1b610b01 100644 --- 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) diff --git a/pkg/helmexec/exec_test.go b/pkg/helmexec/exec_test.go index 31a21f44..25b3494e 100644 --- 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 ` diff --git a/pkg/helmexec/helmexec.go b/pkg/helmexec/helmexec.go index 462adf29..84938317 100644 --- 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 diff --git a/pkg/state/state.go b/pkg/state/state.go index 578e639d..62b75f5e 100644 --- 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. "+ diff --git a/pkg/state/util.go b/pkg/state/util.go index 095f75a3..002cae84 100644 --- 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 +} diff --git a/test/e2e/template/helmfile/snapshot_test.go b/test/e2e/template/helmfile/snapshot_test.go index fa133799..9ee219ea 100644 --- 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") diff --git a/test/e2e/template/helmfile/testdata/snapshot/chart_need/output.yaml b/test/e2e/template/helmfile/testdata/snapshot/chart_need/output.yaml index 0374ae81..7954f3c1 100644 --- 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 diff --git 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 index 41314ff1..e16b3010 100644 --- 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 diff --git 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 index b37a1299..39b9091d 100644 --- 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 diff --git 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 index 2a590d7d..67616a32 100644 --- 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 diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_need/output.yaml b/test/e2e/template/helmfile/testdata/snapshot/oci_need/output.yaml index 3ff51e69..863188cd 100644 --- 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 diff --git a/test/e2e/template/helmfile/testdata/snapshot/pr_560/output.yaml b/test/e2e/template/helmfile/testdata/snapshot/pr_560/output.yaml index ee75ffae..bf9bc119 100644 --- 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: diff --git a/test/e2e/template/helmfile/testdata/snapshot/templated_lockfile/config.yaml b/test/e2e/template/helmfile/testdata/snapshot/templated_lockfile/config.yaml index 3c0659b7..db6f18fc 100644 --- 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 diff --git a/test/e2e/template/helmfile/testdata/snapshot/templated_lockfile/input.yaml b/test/e2e/template/helmfile/testdata/snapshot/templated_lockfile/input.yaml index 1ff49e2e..f4f92ab4 100644 --- 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: diff --git a/test/e2e/template/helmfile/testdata/snapshot/templated_lockfile/output.yaml b/test/e2e/template/helmfile/testdata/snapshot/templated_lockfile/output.yaml index 3fb38130..18cd584e 100644 --- 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