From 22552e9dd0fec89bc141ccc00c29ed10bb2c1c96 Mon Sep 17 00:00:00 2001 From: jorgevillaverde Date: Fri, 7 May 2021 20:40:36 -0300 Subject: [PATCH] fix: dep up when exists tmpcharts Check if tmpcharts exists, and delete it before RenameWithFallback. Refs: #5567 Signed-off-by: jorgevillaverde --- pkg/downloader/manager.go | 9 ++++ pkg/downloader/manager_test.go | 86 ++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index e89ac7c02..604330285 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -248,6 +248,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { destPath := filepath.Join(m.ChartPath, "charts") tmpPath := filepath.Join(m.ChartPath, "tmpcharts") + defer os.RemoveAll(tmpPath) // Create 'charts' directory if it doesn't already exist. if fi, err := os.Stat(destPath); err != nil { @@ -258,6 +259,14 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { return errors.Errorf("%q is not a directory", destPath) } + // Check if tmpcharts exists, is so, delete it. see https://github.com/helm/helm/issues/5567 + if _, err := os.Stat(tmpPath); !os.IsNotExist(err) { + err := os.RemoveAll(tmpPath) + if err != nil { + return errors.Wrap(err, "unable to delete tmp dir") + } + } + if err := fs.RenameWithFallback(destPath, tmpPath); err != nil { return errors.Wrap(err, "unable to move current charts to tmp dir") } diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index fc8d9abb2..ddc7ae185 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -17,10 +17,13 @@ package downloader import ( "bytes" + "fmt" + "os" "path/filepath" "reflect" "testing" + "github.com/pkg/errors" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/getter" @@ -249,6 +252,89 @@ func TestUpdateBeforeBuild(t *testing.T) { } } +func TestUpdateWithExistingTempDir(t *testing.T) { + // Set up a fake repo + srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*") + if err != nil { + t.Fatal(err) + } + defer srv.Stop() + if err := srv.LinkIndices(); err != nil { + t.Fatal(err) + } + + dir := func(p ...string) string { + return filepath.Join(append([]string{srv.Root()}, p...)...) + } + + // Save dep + d := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "dep-chart", + Version: "0.1.0", + APIVersion: "v1", + }, + } + if err := chartutil.SaveDir(d, dir()); err != nil { + t.Fatal(err) + } + // Save a chart + c := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "with-dependency-existing-tmpcharts", + Version: "0.1.0", + APIVersion: "v2", + Dependencies: []*chart.Dependency{{ + Name: d.Metadata.Name, + Version: ">=0.1.0", + Repository: "file://../dep-chart", + }}, + }, + } + + if err := chartutil.SaveDir(c, dir()); err != nil { + t.Fatal(err) + } + + // add a tmpcharts to the chart dir + outdir := filepath.Join(dir(), c.Name()) + tmpPath := filepath.Join(outdir, "tmpcharts") + fmt.Println(tmpPath) + // Create 'tmpcharts' directory if it doesn't already exist. + if fi, err := os.Stat(tmpPath); err != nil { + if err := os.MkdirAll(tmpPath, 0755); err != nil { + t.Fatal(err) + } + } else if !fi.IsDir() { + t.Fatal(errors.Errorf("%q is not a directory", tmpPath)) + } + + // Set-up a manager + b := bytes.NewBuffer(nil) + g := getter.Providers{getter.Provider{ + Schemes: []string{"http", "https"}, + New: getter.NewHTTPGetter, + }} + m := &Manager{ + ChartPath: dir(c.Metadata.Name), + Out: b, + Getters: g, + RepositoryConfig: dir("repositories.yaml"), + RepositoryCache: dir(), + } + + // Update before Build. see issue: https://github.com/helm/helm/issues/7101 + err = m.Update() + if err != nil { + t.Fatal(err) + } + + err = m.Build() + if err != nil { + t.Fatal(err) + } +} + // TestUpdateWithNoRepo is for the case of a dependency that has no repo listed. // This happens when the dependency is in the charts directory and does not need // to be fetched.