operations: Simplify checking if a branch to be created exists

In other RPCs that use updateReferenceWithHooks we need to fetch the ref
to figure out what the commit ID is for locking. In the case of a new
branch we always specify this commit ID as zero. Since we have no commit
ID to pass through, this extra reference check does not give us any
protection. The branch could still be created between the initial
reference check and the actual update.

Removing this extra check means that the error message is already marked
as failed precondition correctly. Although it appears that this error
message is less specific this is still and improvement:

* Having the error marked as a failed precondition means that the error
  does not trickle up to sentry.
* Although the error looks user facing, gitlab-rails ignores the actual
  message.

In future we could capture the actual git errors as part of
updateref.Error. For now the message is hardcoded as part of various
operations endpoints.

2 files changed, 2 insertions, 8 deletions

diff --git a/internal/gitaly/service/operations/branches.go b/internal/gitaly/service/operations/branches.go
index 40cebfbc2..410835193 100644
--- a/internal/gitaly/service/operations/branches.go
+++ b/internal/gitaly/service/operations/branches.go
@@ -50,12 +50,6 @@ func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB
 	}
 
 	referenceName := git.NewReferenceNameFromBranchName(string(req.BranchName))
-	_, err = quarantineRepo.GetReference(ctx, referenceName)
-	if err == nil {
-		return nil, status.Errorf(codes.FailedPrecondition, "Could not update %s. Please refresh and try again.", req.BranchName)
-	} else if !errors.Is(err, git.ErrReferenceNotFound) {
-		return nil, status.Error(codes.Internal, err.Error())
-	}
 
 	if err := s.updateReferenceWithHooks(ctx, req.GetRepository(), req.User, quarantineDir, referenceName, startPointOID, git.ObjectHashSHA1.ZeroOID); err != nil {
 		var customHookErr updateref.CustomHookError
diff --git a/internal/gitaly/service/operations/branches_test.go b/internal/gitaly/service/operations/branches_test.go
index 447cfd1fa..e0bf721a0 100644
--- a/internal/gitaly/service/operations/branches_test.go
+++ b/internal/gitaly/service/operations/branches_test.go
@@ -373,14 +373,14 @@ func TestUserCreateBranch_Failure(t *testing.T) {
 			branchName: "master",
 			startPoint: "master",
 			user:       gittest.TestUser,
-			err:        status.Errorf(codes.FailedPrecondition, "Could not update %s. Please refresh and try again.", "master"),
+			err:        status.Errorf(codes.FailedPrecondition, "Could not update %s. Please refresh and try again.", "refs/heads/master"),
 		},
 		{
 			desc:       "conflicting with refs/heads/improve/awesome",
 			branchName: "improve",
 			startPoint: "master",
 			user:       gittest.TestUser,
-			err:        status.Errorf(codes.Internal, "reference is ambiguous: conflicts with %q", "refs/heads/improve/awesome"),
+			err:        status.Errorf(codes.FailedPrecondition, "Could not update %s. Please refresh and try again.", "refs/heads/improve"),
 		},
 	}