Merge branch 'jc/commit-files-with-git' into 'master'

Implement UserCommitFiles with git plumbing commands

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

Merged-by: John Cai <jcai@gitlab.com>
Approved-by: Christian Couder <chriscool@tuxfamily.org>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>

5 files changed, 445 insertions, 49 deletions

diff --git a/internal/featureflag/ff_commit_files_in_git.go b/internal/featureflag/ff_commit_files_in_git.go
new file mode 100644
index 000000000..32ad53869
--- /dev/null
+++ b/internal/featureflag/ff_commit_files_in_git.go
@@ -0,0 +1,10 @@
+package featureflag
+
+// CommitFilesInGit enables implementation of UserCommitFiles using
+// git plumbing commands instead of git2go
+var CommitFilesInGit = NewFeatureFlag(
+	"commit_files_in_git",
+	"16.1.0",
+	"https://gitlab.com/gitlab-org/gitaly/-/issues/5019",
+	false,
+)
diff --git a/internal/git/localrepo/tree.go b/internal/git/localrepo/tree.go
index da82d2b04..b8d3efe6b 100644
--- a/internal/git/localrepo/tree.go
+++ b/internal/git/localrepo/tree.go
@@ -121,6 +121,8 @@ var (
 	ErrAbsolutePath = errors.New("path is absolute")
 	// ErrEmptyPath indicates the path is an absolute path
 	ErrEmptyPath = errors.New("path is empty")
+	// ErrInvalidPath indicates the path is invalid
+	ErrInvalidPath = errors.New("invalid path")
 )
 
 func validateFileCreationPath(path string) (string, error) {
@@ -138,6 +140,10 @@ func validateFileCreationPath(path string) (string, error) {
 		return "", ErrEmptyPath
 	}
 
+	if before, _, _ := strings.Cut(path, "/"); before == ".git" {
+		return "", ErrInvalidPath
+	}
+
 	return path, nil
 }
 
diff --git a/internal/git/localrepo/tree_test.go b/internal/git/localrepo/tree_test.go
index 828538267..ec14cdf21 100644
--- a/internal/git/localrepo/tree_test.go
+++ b/internal/git/localrepo/tree_test.go
@@ -1983,6 +1983,17 @@ func TestValidatePath(t *testing.T) {
 			path:        ".",
 			expectedErr: ErrEmptyPath,
 		},
+		{
+			desc:        "contains .git",
+			path:        ".git/objects/info",
+			expectedErr: ErrInvalidPath,
+		},
+		{
+			desc:         "contains .git but not the git directory",
+			path:         ".gitsomefile",
+			expectedPath: ".gitsomefile",
+			expectedErr:  nil,
+		},
 	}
 
 	for _, tc := range testCases {
diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go
index 24f376801..2f69130c4 100644
--- a/internal/gitaly/service/operations/commit_files.go
+++ b/internal/gitaly/service/operations/commit_files.go
@@ -7,10 +7,12 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"path/filepath"
 	"strings"
 
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"github.com/sirupsen/logrus"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/remoterepo"
@@ -125,6 +127,324 @@ func validatePath(rootPath, relPath string) (string, error) {
 	return path, nil
 }
 
+// applyAction applies an action to an TreeEntry.
+func applyAction(
+	ctx context.Context,
+	action git2go.Action,
+	root *localrepo.TreeEntry,
+	repo *localrepo.Repo,
+) error {
+	switch action := action.(type) {
+	case git2go.ChangeFileMode:
+		if err := root.Modify(
+			action.Path,
+			func(entry *localrepo.TreeEntry) error {
+				if action.ExecutableMode {
+					if entry.Mode != "100755" {
+						entry.Mode = "100755"
+					}
+				} else {
+					if entry.Mode == "100755" {
+						entry.Mode = "100644"
+					}
+				}
+
+				return nil
+			}); err != nil {
+			return translateToGit2GoError(err, action.Path)
+		}
+	case git2go.UpdateFile:
+		if err := root.Modify(
+			action.Path,
+			func(entry *localrepo.TreeEntry) error {
+				entry.OID = git.ObjectID(action.OID)
+				return nil
+			}); err != nil {
+			return translateToGit2GoError(err, action.Path)
+		}
+	case git2go.MoveFile:
+		entry, err := root.Get(action.Path)
+		if err != nil {
+			return translateToGit2GoError(err, action.Path)
+		}
+
+		if entry.Type != localrepo.Blob {
+			return git2go.IndexError{
+				Path: action.Path,
+				Type: git2go.ErrFileNotFound,
+			}
+		}
+
+		mode := entry.Mode
+
+		if action.OID == "" {
+			action.OID = string(entry.OID)
+		}
+
+		if err := root.Delete(action.Path); err != nil {
+			return translateToGit2GoError(err, action.Path)
+		}
+
+		if err := root.Add(
+			action.NewPath,
+			localrepo.TreeEntry{
+				OID:  git.ObjectID(action.OID),
+				Mode: mode,
+				Path: filepath.Base(action.NewPath),
+			},
+			localrepo.WithOverwriteDirectory(),
+		); err != nil {
+			return translateToGit2GoError(err, action.NewPath)
+		}
+	case git2go.CreateDirectory:
+		if entry, err := root.Get(action.Path); err != nil && !errors.Is(err, localrepo.ErrEntryNotFound) {
+			return translateToGit2GoError(err, action.Path)
+		} else if entry != nil {
+			switch entry.Type {
+			case localrepo.Tree, localrepo.Submodule:
+				return git2go.IndexError{
+					Path: action.Path,
+					Type: git2go.ErrDirectoryExists,
+				}
+			default:
+				return git2go.IndexError{
+					Path: action.Path,
+					Type: git2go.ErrFileExists,
+				}
+			}
+		}
+
+		blobID, err := repo.WriteBlob(ctx, filepath.Join(action.Path, ".gitkeep"), strings.NewReader(""))
+		if err != nil {
+			return err
+		}
+
+		if err := root.Add(
+			filepath.Join(action.Path, ".gitkeep"),
+			localrepo.TreeEntry{
+				Mode: "100644",
+				Path: ".gitkeep",
+				Type: localrepo.Blob,
+				OID:  blobID,
+			},
+		); err != nil {
+			if errors.Is(err, localrepo.ErrEntryExists) {
+				return git2go.IndexError{
+					Path: action.Path,
+					Type: git2go.ErrDirectoryExists,
+				}
+			}
+
+			return translateToGit2GoError(err, action.Path)
+		}
+	case git2go.CreateFile:
+		mode := "100644"
+		if action.ExecutableMode {
+			mode = "100755"
+		}
+
+		if err := root.Add(
+			action.Path,
+			localrepo.TreeEntry{
+				OID:  git.ObjectID(action.OID),
+				Path: filepath.Base(action.Path),
+				Type: localrepo.Blob,
+				Mode: mode,
+			},
+			localrepo.WithOverwriteDirectory(),
+		); err != nil {
+			return translateToGit2GoError(err, action.Path)
+		}
+	case git2go.DeleteFile:
+		if err := root.Delete(
+			action.Path,
+		); err != nil {
+			return translateToGit2GoError(err, action.Path)
+		}
+	default:
+		return errors.New("unsupported action")
+	}
+
+	return nil
+}
+
+// translateToGit2GoError maintains backwards compatibility with existing git2go
+// errors.
+func translateToGit2GoError(err error, path string) error {
+	switch err {
+	case localrepo.ErrEntryNotFound, localrepo.ErrObjectNotFound:
+		return git2go.IndexError{
+			Path: path,
+			Type: git2go.ErrFileNotFound,
+		}
+	case localrepo.ErrEmptyPath,
+		localrepo.ErrPathTraversal,
+		localrepo.ErrAbsolutePath,
+		localrepo.ErrInvalidPath:
+		//The error coming back from git2go has the path in single
+		//quotes. This is to match the git2go error for now.
+		//nolint:gitaly-linters
+		return git2go.UnknownIndexError(
+			fmt.Sprintf("invalid path: '%s'", path),
+		)
+	case localrepo.ErrPathTraversal:
+		return git2go.IndexError{
+			Path: path,
+			Type: git2go.ErrDirectoryTraversal,
+		}
+	case localrepo.ErrEntryExists:
+		return git2go.IndexError{
+			Path: path,
+			Type: git2go.ErrFileExists,
+		}
+	}
+	return err
+}
+
+// ErrSignatureMissingNameOrEmail matches the git2go error
+var ErrSignatureMissingNameOrEmail = errors.New(
+	"commit: failed to parse signature - Signature cannot have an empty name or email",
+)
+
+func (s *Server) userCommitFilesGit(
+	ctx context.Context,
+	header *gitalypb.UserCommitFilesRequestHeader,
+	parentCommitOID git.ObjectID,
+	quarantineRepo *localrepo.Repo,
+	repoPath string,
+	actions []git2go.Action,
+) (git.ObjectID, error) {
+	now, err := dateFromProto(header)
+	if err != nil {
+		return "", structerr.NewInvalidArgument("%w", err)
+	}
+
+	var treeish git.ObjectID
+
+	if parentCommitOID != "" {
+		treeish, err = quarantineRepo.ResolveRevision(
+			ctx,
+			git.Revision(fmt.Sprintf("%s^{tree}", parentCommitOID)),
+		)
+		if err != nil {
+			return "", err
+		}
+	}
+
+	treeEntry := &localrepo.TreeEntry{
+		Mode: "040000",
+		Type: localrepo.Tree,
+	}
+
+	if treeish != "" {
+		treeEntry, err = quarantineRepo.ReadTree(
+			ctx,
+			git.Revision(treeish),
+			localrepo.WithRecursive(),
+		)
+		if err != nil {
+			return "", err
+		}
+	}
+
+	for _, action := range actions {
+		if err = applyAction(
+			ctx,
+			action,
+			treeEntry,
+			quarantineRepo,
+		); err != nil {
+			return "", err
+		}
+	}
+
+	if len(actions) > 0 {
+		if err := treeEntry.Write(
+			ctx,
+			quarantineRepo,
+		); err != nil {
+			return "", err
+		}
+
+		treeish = treeEntry.OID
+	}
+
+	if treeish == "" {
+		objectHash, err := quarantineRepo.ObjectHash(ctx)
+		if err != nil {
+			return "", fmt.Errorf("getting object hash: %w", err)
+		}
+
+		treeish = objectHash.EmptyTreeOID
+	}
+
+	cfg := localrepo.WriteCommitConfig{
+		AuthorDate:     now,
+		AuthorName:     strings.TrimSpace(string(header.CommitAuthorName)),
+		AuthorEmail:    strings.TrimSpace(string(header.CommitAuthorEmail)),
+		CommitterDate:  now,
+		CommitterName:  strings.TrimSpace(string(header.User.Name)),
+		CommitterEmail: strings.TrimSpace(string(header.User.Email)),
+		Message:        string(header.CommitMessage),
+		TreeID:         treeish,
+	}
+
+	if cfg.AuthorName == "" {
+		cfg.AuthorName = cfg.CommitterName
+	}
+
+	if cfg.AuthorEmail == "" {
+		cfg.AuthorEmail = cfg.CommitterEmail
+	}
+
+	if cfg.AuthorName == "" || cfg.AuthorEmail == "" {
+		return "", structerr.NewInvalidArgument("%w", ErrSignatureMissingNameOrEmail)
+	}
+
+	if parentCommitOID != "" {
+		cfg.Parents = []git.ObjectID{parentCommitOID}
+	}
+
+	return quarantineRepo.WriteCommit(
+		ctx,
+		cfg,
+	)
+}
+
+func (s *Server) userCommitFilesGit2Go(
+	ctx context.Context,
+	header *gitalypb.UserCommitFilesRequestHeader,
+	parentCommitOID git.ObjectID,
+	quarantineRepo *localrepo.Repo,
+	repoPath string,
+	actions []git2go.Action,
+) (git.ObjectID, error) {
+	now, err := dateFromProto(header)
+	if err != nil {
+		return "", structerr.NewInvalidArgument("%w", err)
+	}
+
+	committer := git2go.NewSignature(string(header.User.Name), string(header.User.Email), now)
+	author := committer
+	if len(header.CommitAuthorName) > 0 && len(header.CommitAuthorEmail) > 0 {
+		author = git2go.NewSignature(string(header.CommitAuthorName), string(header.CommitAuthorEmail), now)
+	}
+
+	commitID, err := s.git2goExecutor.Commit(ctx, quarantineRepo, git2go.CommitCommand{
+		Repository: repoPath,
+		Author:     author,
+		Committer:  committer,
+		Message:    string(header.CommitMessage),
+		Parent:     parentCommitOID.String(),
+		Actions:    actions,
+	})
+	if err != nil {
+		return "", fmt.Errorf("%w", err)
+	}
+
+	return commitID, nil
+}
+
 func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommitFilesRequestHeader, stream gitalypb.OperationService_UserCommitFilesServer) error {
 	quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, header.GetRepository())
 	if err != nil {
@@ -261,7 +581,6 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi
 					return err
 				}
 			}
-
 			actions = append(actions, git2go.MoveFile{
 				Path:    prevPath,
 				NewPath: path,
@@ -288,27 +607,30 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi
 		}
 	}
 
-	now, err := dateFromProto(header)
-	if err != nil {
-		return structerr.NewInvalidArgument("%w", err)
-	}
+	var commitID git.ObjectID
 
-	committer := git2go.NewSignature(string(header.User.Name), string(header.User.Email), now)
-	author := committer
-	if len(header.CommitAuthorName) > 0 && len(header.CommitAuthorEmail) > 0 {
-		author = git2go.NewSignature(string(header.CommitAuthorName), string(header.CommitAuthorEmail), now)
+	if featureflag.CommitFilesInGit.IsEnabled(ctx) {
+		commitID, err = s.userCommitFilesGit(
+			ctx,
+			header,
+			parentCommitOID,
+			quarantineRepo,
+			repoPath,
+			actions,
+		)
+	} else {
+		commitID, err = s.userCommitFilesGit2Go(
+			ctx,
+			header,
+			parentCommitOID,
+			quarantineRepo,
+			repoPath,
+			actions,
+		)
 	}
 
-	commitID, err := s.git2goExecutor.Commit(ctx, quarantineRepo, git2go.CommitCommand{
-		Repository: repoPath,
-		Author:     author,
-		Committer:  committer,
-		Message:    string(header.CommitMessage),
-		Parent:     parentCommitOID.String(),
-		Actions:    actions,
-	})
 	if err != nil {
-		return fmt.Errorf("commit: %w", err)
+		return err
 	}
 
 	hasBranches, err := quarantineRepo.HasBranches(ctx)
diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go
index 6dd6586e4..3aa3868de 100644
--- a/internal/gitaly/service/operations/commit_files_test.go
+++ b/internal/gitaly/service/operations/commit_files_test.go
@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo"
@@ -31,8 +32,12 @@ var commitFilesMessage = []byte("Change files")
 
 func TestUserCommitFiles(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
 
+	testhelper.NewFeatureSets(featureflag.CommitFilesInGit).
+		Run(t, testUserCommitFiles)
+}
+
+func testUserCommitFiles(t *testing.T, ctx context.Context) {
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
 	const (
@@ -965,8 +970,12 @@ func TestUserCommitFiles(t *testing.T) {
 
 func TestUserCommitFilesStableCommitID(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
 
+	testhelper.NewFeatureSets(featureflag.CommitFilesInGit).
+		Run(t, testUserCommitFilesStableCommitID)
+}
+
+func testUserCommitFilesStableCommitID(t *testing.T, ctx context.Context) {
 	ctx, cfg, _, _, client := setupOperationsService(t, ctx)
 
 	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
@@ -1023,8 +1032,12 @@ func TestUserCommitFilesStableCommitID(t *testing.T) {
 
 func TestUserCommitFilesQuarantine(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
 
+	testhelper.NewFeatureSets(featureflag.CommitFilesInGit).
+		Run(t, testUserCommitFilesQuarantine)
+}
+
+func testUserCommitFilesQuarantine(t *testing.T, ctx context.Context) {
 	ctx, cfg, _, _, client := setupOperationsService(t, ctx)
 
 	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
@@ -1073,10 +1086,14 @@ func TestUserCommitFilesQuarantine(t *testing.T) {
 	require.False(t, exists, "quarantined commit should have been discarded")
 }
 
-func TestSuccessfulUserCommitFilesFilesRequest(t *testing.T) {
+func TestSuccessfulUserCommitFilesRequest(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
 
+	testhelper.NewFeatureSets(featureflag.CommitFilesInGit).
+		Run(t, testSuccessfulUserCommitFilesRequest)
+}
+
+func testSuccessfulUserCommitFilesRequest(t *testing.T, ctx context.Context) {
 	ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
 
 	newRepo, newRepoPath := gittest.CreateRepository(t, ctx, cfg)
@@ -1224,10 +1241,14 @@ func TestSuccessfulUserCommitFilesFilesRequest(t *testing.T) {
 	}
 }
 
-func TestSuccessUserCommitFilesRequestMove(t *testing.T) {
+func TestSuccessfulUserCommitFilesRequestMove(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
 
+	testhelper.NewFeatureSets(featureflag.CommitFilesInGit).
+		Run(t, testSuccessfulUserCommitFilesRequestMove)
+}
+
+func testSuccessfulUserCommitFilesRequestMove(t *testing.T, ctx context.Context) {
 	ctx, cfg, _, _, client := setupOperationsService(t, ctx)
 
 	branchName := "master"
@@ -1282,10 +1303,14 @@ func TestSuccessUserCommitFilesRequestMove(t *testing.T) {
 	}
 }
 
-func TestSuccessUserCommitFilesRequestForceCommit(t *testing.T) {
+func TestSuccessfulUserCommitFilesRequestForceCommit(t *testing.T) {
 	t.Parallel()
 
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.CommitFilesInGit).
+		Run(t, testSuccessUserCommitFilesRequestForceCommit)
+}
+
+func testSuccessUserCommitFilesRequestForceCommit(t *testing.T, ctx context.Context) {
 	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
 
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -1327,10 +1352,14 @@ func TestSuccessUserCommitFilesRequestForceCommit(t *testing.T) {
 	require.Equal(t, newTargetBranchCommit.ParentIds, []string{startBranchCommit.Id})
 }
 
-func TestSuccessUserCommitFilesRequestStartSha(t *testing.T) {
+func TestSuccessUserCommitFilesRequestForceCommit(t *testing.T) {
 	t.Parallel()
 
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.CommitFilesInGit).
+		Run(t, testSuccessUserCommitFilesRequestStartSha)
+}
+
+func testSuccessUserCommitFilesRequestStartSha(t *testing.T, ctx context.Context) {
 	ctx, cfg, repoProto, _, client := setupOperationsService(t, ctx)
 
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -1363,17 +1392,19 @@ func TestSuccessUserCommitFilesRequestStartSha(t *testing.T) {
 func TestSuccessUserCommitFilesRequestStartShaRemoteRepository(t *testing.T) {
 	t.Parallel()
 
-	testSuccessfulUserCommitFilesRemoteRepositoryRequest(func(header *gitalypb.UserCommitFilesRequest) {
-		setStartSha(header, "1e292f8fedd741b75372e19097c76d327140c312")
-	})
+	testhelper.NewFeatureSets(featureflag.CommitFilesInGit).
+		Run(t, testSuccessfulUserCommitFilesRemoteRepositoryRequest(func(header *gitalypb.UserCommitFilesRequest) {
+			setStartSha(header, "1e292f8fedd741b75372e19097c76d327140c312")
+		}))
 }
 
 func TestSuccessUserCommitFilesRequestStartBranchRemoteRepository(t *testing.T) {
 	t.Parallel()
 
-	testSuccessfulUserCommitFilesRemoteRepositoryRequest(func(header *gitalypb.UserCommitFilesRequest) {
-		setStartBranchName(header, []byte("master"))
-	})
+	testhelper.NewFeatureSets(featureflag.CommitFilesInGit).
+		Run(t, testSuccessfulUserCommitFilesRemoteRepositoryRequest(func(header *gitalypb.UserCommitFilesRequest) {
+			setStartBranchName(header, []byte("master"))
+		}))
 }
 
 func testSuccessfulUserCommitFilesRemoteRepositoryRequest(setHeader func(header *gitalypb.UserCommitFilesRequest)) func(*testing.T, context.Context) {
@@ -1417,7 +1448,11 @@ func testSuccessfulUserCommitFilesRemoteRepositoryRequest(setHeader func(header
 func TestSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *testing.T) {
 	t.Parallel()
 
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.CommitFilesInGit).
+		Run(t, testSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature)
+}
+
+func testSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *testing.T, ctx context.Context) {
 	ctx, cfg, _, _, client := setupOperationsService(t, ctx)
 
 	repoProto, _ := gittest.CreateRepository(t, ctx, cfg)
@@ -1468,7 +1503,11 @@ func TestSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *tes
 func TestFailedUserCommitFilesRequestDueToHooks(t *testing.T) {
 	t.Parallel()
 
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.CommitFilesInGit).
+		Run(t, testFailedUserCommitFilesRequestDueToHooks)
+}
+
+func testFailedUserCommitFilesRequestDueToHooks(t *testing.T, ctx context.Context) {
 	ctx, _, repoProto, repoPath, client := setupOperationsService(t, ctx)
 
 	branchName := "feature"
@@ -1517,7 +1556,11 @@ func TestFailedUserCommitFilesRequestDueToHooks(t *testing.T) {
 func TestFailedUserCommitFilesRequestDueToIndexError(t *testing.T) {
 	t.Parallel()
 
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.CommitFilesInGit).
+		Run(t, testFailedUserCommitFilesRequestDueToIndexError)
+}
+
+func testFailedUserCommitFilesRequestDueToIndexError(t *testing.T, ctx context.Context) {
 	ctx, _, repo, _, client := setupOperationsService(t, ctx)
 
 	testCases := []struct {
@@ -1590,7 +1633,11 @@ func TestFailedUserCommitFilesRequestDueToIndexError(t *testing.T) {
 func TestFailedUserCommitFilesRequest(t *testing.T) {
 	t.Parallel()
 
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.CommitFilesInGit).
+		Run(t, testFailedUserCommitFilesRequest)
+}
+
+func testFailedUserCommitFilesRequest(t *testing.T, ctx context.Context) {
 	ctx, _, repo, _, client := setupOperationsService(t, ctx)
 
 	branchName := "feature"
@@ -1629,29 +1676,29 @@ func TestFailedUserCommitFilesRequest(t *testing.T) {
 			expectedErr: status.Error(codes.InvalidArgument, fmt.Sprintf(`invalid object ID: "foobar", expected length %v, got 6`, gittest.DefaultObjectHash.EncodedLen())),
 		},
 		{
-			desc:        "failed to parse signature - Signature cannot have an empty name or email",
+			desc:        "failed to parse signature - Signature cannot have an empty name or email 1",
 			req:         headerRequest(repo, &gitalypb.User{}, branchName, commitFilesMessage, "", ""),
-			expectedErr: status.Error(codes.InvalidArgument, "commit: commit: failed to parse signature - Signature cannot have an empty name or email"),
+			expectedErr: status.Error(codes.InvalidArgument, "commit: failed to parse signature - Signature cannot have an empty name or email"),
 		},
 		{
-			desc:        "failed to parse signature - Signature cannot have an empty name or email",
+			desc:        "failed to parse signature - Signature cannot have an empty name or email 2",
 			req:         headerRequest(repo, &gitalypb.User{Name: []byte(""), Email: []byte("")}, branchName, commitFilesMessage, "", ""),
-			expectedErr: status.Error(codes.InvalidArgument, "commit: commit: failed to parse signature - Signature cannot have an empty name or email"),
+			expectedErr: status.Error(codes.InvalidArgument, "commit: failed to parse signature - Signature cannot have an empty name or email"),
 		},
 		{
-			desc:        "failed to parse signature - Signature cannot have an empty name or email",
+			desc:        "failed to parse signature - Signature cannot have an empty name or email 3",
 			req:         headerRequest(repo, &gitalypb.User{Name: []byte(" "), Email: []byte(" ")}, branchName, commitFilesMessage, "", ""),
-			expectedErr: status.Error(codes.InvalidArgument, "commit: commit: failed to parse signature - Signature cannot have an empty name or email"),
+			expectedErr: status.Error(codes.InvalidArgument, "commit: failed to parse signature - Signature cannot have an empty name or email"),
 		},
 		{
-			desc:        "failed to parse signature - Signature cannot have an empty name or email",
+			desc:        "failed to parse signature - Signature cannot have an empty name or email 4",
 			req:         headerRequest(repo, &gitalypb.User{Name: []byte("Jane Doe"), Email: []byte("")}, branchName, commitFilesMessage, "", ""),
-			expectedErr: status.Error(codes.InvalidArgument, "commit: commit: failed to parse signature - Signature cannot have an empty name or email"),
+			expectedErr: status.Error(codes.InvalidArgument, "commit: failed to parse signature - Signature cannot have an empty name or email"),
 		},
 		{
-			desc:        "failed to parse signature - Signature cannot have an empty name or email",
+			desc:        "failed to parse signature - Signature cannot have an empty name or email 5",
 			req:         headerRequest(repo, &gitalypb.User{Name: []byte(""), Email: []byte("janedoe@gitlab.com")}, branchName, commitFilesMessage, "", ""),
-			expectedErr: status.Error(codes.InvalidArgument, "commit: commit: failed to parse signature - Signature cannot have an empty name or email"),
+			expectedErr: status.Error(codes.InvalidArgument, "commit: failed to parse signature - Signature cannot have an empty name or email"),
 		},
 	}