Merge branch 'smh-update-ref-file-directory-errors' into 'master'

Parse file-directory errors from update-ref

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5109

Merged-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Co-authored-by: Sami Hiltunen <shiltunen@gitlab.com>

2 files changed, 117 insertions, 2 deletions

diff --git a/internal/git/updateref/updateref.go b/internal/git/updateref/updateref.go
index a79c218b3..26d801078 100644
--- a/internal/git/updateref/updateref.go
+++ b/internal/git/updateref/updateref.go
@@ -31,6 +31,33 @@ func (e ErrInvalidReferenceFormat) Error() string {
 	return fmt.Sprintf("invalid reference format: %q", e.ReferenceName)
 }
 
+// ErrFileDirectoryConflict is returned when an operation would causes a file-directory conflict
+// in the reference store.
+type ErrFileDirectoryConflict struct {
+	// ConflictingReferenceName is the name of the reference that would have conflicted.
+	ConflictingReferenceName string
+	// ExistingReferenceName is the name of the already existing reference.
+	ExistingReferenceName string
+}
+
+func (e ErrFileDirectoryConflict) Error() string {
+	return fmt.Sprintf("%q conflicts with %q", e.ConflictingReferenceName, e.ExistingReferenceName)
+}
+
+// ErrInTransactionConflict is returned when attempting to modify two references in the same transaction
+// in a manner that is not allowed. For example, modifying 'refs/heads/parent' and creating
+// 'refs/heads/parent/child' is not allowed.
+type ErrInTransactionConflict struct {
+	// FirstReferenceName is the name of the first reference that was modified.
+	FirstReferenceName string
+	// SecondReferenceName is the name of the second reference that was modified.
+	SecondReferenceName string
+}
+
+func (e ErrInTransactionConflict) Error() string {
+	return fmt.Sprintf("%q and %q conflict in the same transaction", e.FirstReferenceName, e.SecondReferenceName)
+}
+
 // state represents a possible state the updater can be in.
 type state string
 
@@ -275,8 +302,10 @@ func (u *Updater) write(format string, args ...interface{}) error {
 }
 
 var (
-	refLockedRegex        = regexp.MustCompile(`^fatal: prepare: cannot lock ref '(.+?)': Unable to create '.*': File exists.`)
-	refInvalidFormatRegex = regexp.MustCompile(`^fatal: invalid ref format: (.*)\n$`)
+	refLockedRegex               = regexp.MustCompile(`^fatal: prepare: cannot lock ref '(.+?)': Unable to create '.*': File exists.`)
+	refInvalidFormatRegex        = regexp.MustCompile(`^fatal: invalid ref format: (.*)\n$`)
+	referenceExistsConflictRegex = regexp.MustCompile(`^fatal: .*: cannot lock ref '(.*)': '(.*)' exists; cannot create '.*'\n$`)
+	inTransactionConflictRegex   = regexp.MustCompile(`^fatal: .*: cannot lock ref '.*': cannot process '(.*)' and '(.*)' at the same time\n$`)
 )
 
 func (u *Updater) setState(state string) error {
@@ -316,6 +345,22 @@ func (u *Updater) setState(state string) error {
 			return ErrInvalidReferenceFormat{ReferenceName: string(matches[1])}
 		}
 
+		matches = referenceExistsConflictRegex.FindSubmatch(u.stderr.Bytes())
+		if len(matches) > 1 {
+			return ErrFileDirectoryConflict{
+				ExistingReferenceName:    string(matches[2]),
+				ConflictingReferenceName: string(matches[1]),
+			}
+		}
+
+		matches = inTransactionConflictRegex.FindSubmatch(u.stderr.Bytes())
+		if len(matches) > 1 {
+			return ErrInTransactionConflict{
+				FirstReferenceName:  string(matches[1]),
+				SecondReferenceName: string(matches[2]),
+			}
+		}
+
 		return fmt.Errorf("state update to %q failed: %w, stderr: %q", state, err, u.stderr)
 	}
 
diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go
index ae305547b..17517c09d 100644
--- a/internal/git/updateref/updateref_test.go
+++ b/internal/git/updateref/updateref_test.go
@@ -63,6 +63,76 @@ func TestUpdater_create(t *testing.T) {
 	require.Equal(t, gittest.ResolveRevision(t, cfg, repoPath, "refs/heads/_create"), commitID)
 }
 
+func TestUpdater_fileDirectoryConflict(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+
+	for _, tc := range []struct {
+		desc            string
+		firstReference  git.ReferenceName
+		secondReference git.ReferenceName
+	}{
+		{
+			desc:            "directory-file",
+			firstReference:  "refs/heads/directory",
+			secondReference: "refs/heads/directory/file",
+		},
+		{
+			desc:            "file-directory",
+			firstReference:  "refs/heads/directory/file",
+			secondReference: "refs/heads/directory",
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			for _, method := range []struct {
+				desc   string
+				finish func(*Updater) error
+			}{
+				{desc: "prepare", finish: func(updater *Updater) error { return updater.Prepare() }},
+				{desc: "commit", finish: func(updater *Updater) error { return updater.Commit() }},
+			} {
+				t.Run(method.desc, func(t *testing.T) {
+					t.Run("different transaction", func(t *testing.T) {
+						cfg, _, repoPath, updater := setupUpdater(t, ctx)
+						defer func() { require.ErrorContains(t, updater.Close(), "closing updater: exit status 128") }()
+
+						commitID := gittest.WriteCommit(t, cfg, repoPath)
+
+						require.NoError(t, updater.Start())
+						require.NoError(t, updater.Create(tc.firstReference, commitID))
+						require.NoError(t, updater.Commit())
+
+						require.NoError(t, updater.Start())
+						require.NoError(t, updater.Create(tc.secondReference, commitID))
+
+						require.Equal(t, ErrFileDirectoryConflict{
+							ExistingReferenceName:    tc.firstReference.String(),
+							ConflictingReferenceName: tc.secondReference.String(),
+						}, method.finish(updater))
+					})
+
+					t.Run("same transaction", func(t *testing.T) {
+						cfg, _, repoPath, updater := setupUpdater(t, ctx)
+						defer func() { require.ErrorContains(t, updater.Close(), "closing updater: exit status 128") }()
+
+						commitID := gittest.WriteCommit(t, cfg, repoPath)
+
+						require.NoError(t, updater.Start())
+						require.NoError(t, updater.Create(tc.firstReference, commitID))
+						require.NoError(t, updater.Create(tc.secondReference, commitID))
+
+						require.Equal(t, ErrInTransactionConflict{
+							FirstReferenceName:  tc.firstReference.String(),
+							SecondReferenceName: tc.secondReference.String(),
+						}, method.finish(updater))
+					})
+				})
+			}
+		})
+	}
+}
+
 func TestUpdater_invalidStateTransitions(t *testing.T) {
 	t.Parallel()