Merge branch 'po-fork-repo-recover' into 'master'

CreateFork recovers when encountering existing empty directory target

Closes #3296

See merge request gitlab-org/gitaly!2886

3 files changed, 37 insertions, 5 deletions

diff --git a/changelogs/unreleased/po-fork-repo-recover.yml b/changelogs/unreleased/po-fork-repo-recover.yml
new file mode 100644
index 000000000..b0d86e764
--- /dev/null
+++ b/changelogs/unreleased/po-fork-repo-recover.yml
@@ -0,0 +1,5 @@
+---
+title: CreateFork recovers when encountering existing empty directory target
+merge_request: 2886
+author:
+type: fixed
diff --git a/internal/gitaly/service/repository/fork.go b/internal/gitaly/service/repository/fork.go
index 802d5a5c5..8ebd735e3 100644
--- a/internal/gitaly/service/repository/fork.go
+++ b/internal/gitaly/service/repository/fork.go
@@ -28,8 +28,20 @@ func (s *server) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest
 		return nil, err
 	}
 
-	if _, err := os.Stat(targetRepositoryFullPath); !os.IsNotExist(err) {
-		return nil, status.Errorf(codes.InvalidArgument, "CreateFork: dest dir exists")
+	if info, err := os.Stat(targetRepositoryFullPath); err != nil {
+		if !os.IsNotExist(err) {
+			return nil, status.Errorf(codes.Internal, "CreateFork: check destination path: %v", err)
+		}
+
+		// directory does not exist, proceed
+	} else {
+		if !info.IsDir() {
+			return nil, status.Errorf(codes.InvalidArgument, "CreateFork: destination path exists")
+		}
+
+		if err := os.Remove(targetRepositoryFullPath); err != nil {
+			return nil, status.Errorf(codes.InvalidArgument, "CreateFork: destination directory is not empty")
+		}
 	}
 
 	if err := os.MkdirAll(targetRepositoryFullPath, 0770); err != nil {
diff --git a/internal/gitaly/service/repository/fork_test.go b/internal/gitaly/service/repository/fork_test.go
index d18c5693a..7dbbd6737 100644
--- a/internal/gitaly/service/repository/fork_test.go
+++ b/internal/gitaly/service/repository/fork_test.go
@@ -21,12 +21,18 @@ import (
 func TestSuccessfulCreateForkRequest(t *testing.T) {
 	locator := config.NewLocator(config.Config)
 
+	createEmptyTarget := func(repoPath string) {
+		require.NoError(t, os.MkdirAll(repoPath, 0755))
+	}
+
 	for _, tt := range []struct {
-		name   string
-		secure bool
+		name          string
+		secure        bool
+		beforeRequest func(repoPath string)
 	}{
 		{name: "secure", secure: true},
 		{name: "insecure"},
+		{name: "existing empty directory target", beforeRequest: createEmptyTarget},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			var (
@@ -72,6 +78,10 @@ func TestSuccessfulCreateForkRequest(t *testing.T) {
 			require.NoError(t, err)
 			require.NoError(t, os.RemoveAll(forkedRepoPath))
 
+			if tt.beforeRequest != nil {
+				tt.beforeRequest(forkedRepoPath)
+			}
+
 			req := &gitalypb.CreateForkRequest{
 				Repository:       forkedRepo,
 				SourceRepository: testRepo,
@@ -117,7 +127,7 @@ func TestFailedCreateForkRequestDueToExistingTarget(t *testing.T) {
 		isDir    bool
 	}{
 		{
-			desc:     "target is a directory",
+			desc:     "target is a non-empty directory",
 			repoPath: "forks/test-repo-fork-dir.git",
 			isDir:    true,
 		},
@@ -140,6 +150,11 @@ func TestFailedCreateForkRequestDueToExistingTarget(t *testing.T) {
 
 			if testCase.isDir {
 				require.NoError(t, os.MkdirAll(forkedRepoPath, 0770))
+				require.NoError(t, ioutil.WriteFile(
+					filepath.Join(forkedRepoPath, "config"),
+					nil,
+					0644,
+				))
 			} else {
 				require.NoError(t, ioutil.WriteFile(forkedRepoPath, nil, 0644))
 			}