stop with an error immediately if a file or directory with that name already exists (#7187)

* fix #7182

Signed-off-by: zwwhdls <zwwhdls@hotmail.com>

* fix testcase

Signed-off-by: zwwhdls <zwwhdls@hotmail.com>

* update error message

Signed-off-by: zwwhdls <zwwhdls@hotmail.com>

* complete testCase when file/dir existed.

Signed-off-by: zwwhdls <zwwhdls@hotmail.com>

* fix the case with current directory

Signed-off-by: zwwhdls <zwwhdls@hotmail.com>

* fix conflict subdirectory when untardir is the clashing directory

Signed-off-by: zwwhdls <zwwhdls@hotmail.com>

* update comment

Signed-off-by: zwwhdls <zwwhdls@hotmail.com>

* add case when destination exists.

Signed-off-by: zwwhdls <zwwhdls@hotmail.com>

cmd/helm/pull_test.go

@@ -41,7 +41,10 @@ func TestPullCmd(t *testing.T) {
 	tests := []struct {
 		name         string
 		args         string
+		existFile    string
+		existDir     string
 		wantError    bool
+		wantErrorMsg string
 		failExpect   string
 		expectFile   string
 		expectDir    bool
@@ -87,10 +90,24 @@ func TestPullCmd(t *testing.T) {
 			expectFile: "./signtest",
 			expectDir:  true,
 		},
+		{
+			name:         "Fetch untar when file with same name existed",
+			args:         "test/test1 --untar --untardir test1",
+			existFile:    "test1",
+			wantError:    true,
+			wantErrorMsg: fmt.Sprintf("failed to untar: a file or directory with the name %s already exists", filepath.Join(srv.Root(), "test1")),
+		},
+		{
+			name:         "Fetch untar when dir with same name existed",
+			args:         "test/test2 --untar --untardir test2",
+			existDir:     "test2",
+			wantError:    true,
+			wantErrorMsg: fmt.Sprintf("failed to untar: a file or directory with the name %s already exists", filepath.Join(srv.Root(), "test2")),
+		},
 		{
 			name:         "Fetch, verify, untar",
-			args:         "test/signtest --verify --keyring=testdata/helm-test-key.pub --untar --untardir signtest",
+			args:         "test/signtest --verify --keyring=testdata/helm-test-key.pub --untar --untardir signtest2",
-			expectFile:   "./signtest",
+			expectFile:   "./signtest2",
 			expectDir:    true,
 			expectVerify: true,
 		},
@@ -127,9 +144,27 @@ func TestPullCmd(t *testing.T) {
 				filepath.Join(outdir, "repositories.yaml"),
 				outdir,
 			)
+			// 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)
+				}
+			}
 			_, out, err := executeActionCommand(cmd)
 			if err != nil {
 				if tt.wantError {
+					if tt.wantErrorMsg != "" && tt.wantErrorMsg == err.Error() {
+						t.Fatalf("Actual error %s, not equal to expected error %s", err, tt.wantErrorMsg)
+					}
 					return
 				}
 				t.Fatalf("%q reported error: %s", tt.name, err)

pkg/action/pull.go

@@ -110,13 +110,21 @@ func (p *Pull) Run(chartRef string) (string, error) {
 		if !filepath.IsAbs(ud) {
 			ud = filepath.Join(p.DestDir, ud)
 		}
-		if fi, err := os.Stat(ud); err != nil {
+		// Let udCheck to check conflict file/dir without replacing ud when untarDir is the current directory(.).
-			if err := os.MkdirAll(ud, 0755); err != nil {
+		udCheck := ud
+		if udCheck == "." {
+			_, udCheck = filepath.Split(chartRef)
+		} else {
+			_, chartName := filepath.Split(chartRef)
+			udCheck = filepath.Join(udCheck, chartName)
+		}
+		if _, err := os.Stat(udCheck); err != nil {
+			if err := os.MkdirAll(udCheck, 0755); err != nil {
 				return out.String(), errors.Wrap(err, "failed to untar (mkdir)")
 			}
 
-		} else if !fi.IsDir() {
+		} else {
-			return out.String(), errors.Errorf("failed to untar: %s is not a directory", ud)
+			return out.String(), errors.Errorf("failed to untar: a file or directory with the name %s already exists", udCheck)
 		}
 
 		return out.String(), chartutil.ExpandFile(ud, saved)