From 63e1c9bc7f6a88efe40f5badcc0766e6748283c9 Mon Sep 17 00:00:00 2001 From: zwwhdls <33822635+zwwhdls@users.noreply.github.com> Date: Thu, 23 Jan 2020 22:31:05 +0800 Subject: [PATCH] [v2]stop with an error immediately if a file or directory with that name already exists (#7188) * fix #7182 Signed-off-by: zwwhdls * fix typo Signed-off-by: zwwhdls * add testcase Signed-off-by: zwwhdls * fix conflict subdirectory when untardir is the clashing directory Signed-off-by: zwwhdls --- cmd/helm/fetch.go | 17 +++++++++++----- cmd/helm/fetch_test.go | 44 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/cmd/helm/fetch.go b/cmd/helm/fetch.go index bc1c07cb7..9407b8f1f 100644 --- a/cmd/helm/fetch.go +++ b/cmd/helm/fetch.go @@ -166,13 +166,20 @@ func (f *fetchCmd) run() error { if !filepath.IsAbs(ud) { ud = filepath.Join(f.destdir, ud) } - if fi, err := os.Stat(ud); err != nil { - if err := os.MkdirAll(ud, 0755); err != nil { + // Let udCheck to check conflict file/dir without replacing ud when untarDir is the current directory(.). + udCheck := ud + if udCheck == "." { + _, udCheck = filepath.Split(f.chartRef) + } else { + _, chartName := filepath.Split(f.chartRef) + udCheck = filepath.Join(udCheck, chartName) + } + if _, err := os.Stat(udCheck); err != nil { + if err := os.MkdirAll(udCheck, 0755); err != nil { return fmt.Errorf("Failed to untar (mkdir): %s", err) } - - } else if !fi.IsDir() { - return fmt.Errorf("Failed to untar: %s is not a directory", ud) + } else { + return fmt.Errorf("Failed to untar: a file or directory with the name %s already exists", udCheck) } return chartutil.ExpandFile(ud, saved) diff --git a/cmd/helm/fetch_test.go b/cmd/helm/fetch_test.go index 3fba37dd6..de796bab2 100644 --- a/cmd/helm/fetch_test.go +++ b/cmd/helm/fetch_test.go @@ -22,6 +22,7 @@ import ( "os" "path/filepath" "regexp" + "strings" "testing" "k8s.io/helm/pkg/repo/repotest" @@ -48,6 +49,9 @@ func TestFetchCmd(t *testing.T) { chart string flags []string fail bool + wantErrorMsg string + existFile string + existDir string failExpect string expectFile string expectDir bool @@ -98,13 +102,29 @@ func TestFetchCmd(t *testing.T) { expectFile: "./signtest", expectDir: true, }, + { + name: "Fetch untar when file with same name existed", + flags: []string{"test/test1 --untar --untardir test1"}, + existFile: "test1", + fail: true, + wantErrorMsg: "failed to untar", + }, + { + name: "Fetch untar when dir with same name existed", + flags: []string{"test/test2 --untar --untardir test2"}, + existDir: "test2", + fail: true, + wantErrorMsg: "failed to untar", + }, { name: "Fetch, verify, untar", chart: "test/signtest", - flags: []string{"--verify", "--keyring", "testdata/helm-test-key.pub", "--untar", "--untardir", "signtest"}, - expectFile: "./signtest", + flags: []string{"--verify", "--keyring", "testdata/helm-test-key.pub", "--untar", "--untardir", "signtest2"}, + expectFile: "./signtest2", expectDir: true, expectVerify: true, + failExpect: "Failed to untar signtest", + fail: true, }, { name: "Chart fetch using repo URL", @@ -146,13 +166,31 @@ func TestFetchCmd(t *testing.T) { os.RemoveAll(outdir) os.Mkdir(outdir, 0755) + // Create file or Dir before helm pull --untar, see: https://github.com/helm/helm/issues/7182 + if tt.existFile != "" { + file := filepath.Join(outdir, tt.existFile) + _, err := os.Create(file) + if err != nil { + t.Fatal("err") + } + } + if tt.existDir != "" { + file := filepath.Join(outdir, tt.existDir) + err := os.Mkdir(file, 0755) + if err != nil { + t.Fatal(err) + } + } buf := bytes.NewBuffer(nil) cmd := newFetchCmd(buf) tt.flags = append(tt.flags, "-d", outdir) cmd.ParseFlags(tt.flags) if err := cmd.RunE(cmd, []string{tt.chart}); err != nil { if tt.fail { - continue + if tt.wantErrorMsg != "" && strings.Contains(tt.wantErrorMsg, err.Error()) { + t.Fatalf("Actual error %s, not equal to expected error %s", err, tt.wantErrorMsg) + } + return } t.Errorf("%q reported error: %s", tt.name, err) continue