operations: Implement SHA256 support for UserCommitFiles

Implement support for the SHA256 object format in UserCommitFiles and
enable testing with this object format.

Changelog: added

4 files changed, 47 insertions, 28 deletions

diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go
index bc83c8d37..dffae1da5 100644
--- a/internal/gitaly/service/operations/commit_files.go
+++ b/internal/gitaly/service/operations/commit_files.go
@@ -25,6 +25,8 @@ import (
 // UserCommitFiles allows for committing from a set of actions. See the protobuf documentation
 // for details.
 func (s *Server) UserCommitFiles(stream gitalypb.OperationService_UserCommitFilesServer) error {
+	ctx := stream.Context()
+
 	firstRequest, err := stream.Recv()
 	if err != nil {
 		return err
@@ -35,13 +37,20 @@ func (s *Server) UserCommitFiles(stream gitalypb.OperationService_UserCommitFile
 		return structerr.NewInvalidArgument("empty UserCommitFilesRequestHeader")
 	}
 
-	if err = validateUserCommitFilesHeader(s.locator, header); err != nil {
+	if err := s.locator.ValidateRepository(header.GetRepository()); err != nil {
 		return structerr.NewInvalidArgument("%w", err)
 	}
 
-	ctx := stream.Context()
+	objectHash, err := git.DetectObjectHash(ctx, s.gitCmdFactory, header.GetRepository())
+	if err != nil {
+		return fmt.Errorf("detecting object hash: %w", err)
+	}
 
-	if err := s.userCommitFiles(ctx, header, stream); err != nil {
+	if err := validateUserCommitFilesHeader(header, objectHash); err != nil {
+		return structerr.NewInvalidArgument("%w", err)
+	}
+
+	if err := s.userCommitFiles(ctx, header, stream, objectHash); err != nil {
 		ctxlogrus.AddFields(ctx, logrus.Fields{
 			"repository_storage":       header.Repository.StorageName,
 			"repository_relative_path": header.Repository.RelativePath,
@@ -410,7 +419,12 @@ func (s *Server) userCommitFilesGit(
 	)
 }
 
-func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommitFilesRequestHeader, stream gitalypb.OperationService_UserCommitFilesServer) error {
+func (s *Server) userCommitFiles(
+	ctx context.Context,
+	header *gitalypb.UserCommitFilesRequestHeader,
+	stream gitalypb.OperationService_UserCommitFilesServer,
+	objectHash git.ObjectHash,
+) error {
 	quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, header.GetRepository())
 	if err != nil {
 		return err
@@ -454,7 +468,7 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi
 			return fmt.Errorf("resolve parent commit: %w", err)
 		}
 	} else {
-		parentCommitOID, err = git.ObjectHashSHA1.FromHex(header.StartSha)
+		parentCommitOID, err = objectHash.FromHex(header.StartSha)
 		if err != nil {
 			return structerr.NewInvalidArgument("cannot resolve parent commit: %w", err)
 		}
@@ -595,7 +609,7 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi
 
 	var oldRevision git.ObjectID
 	if expectedOldOID := header.GetExpectedOldOid(); expectedOldOID != "" {
-		oldRevision, err = git.ObjectHashSHA1.FromHex(expectedOldOID)
+		oldRevision, err = objectHash.FromHex(expectedOldOID)
 		if err != nil {
 			return structerr.NewInvalidArgument("invalid expected old object ID: %w", err).WithMetadata("old_object_id", expectedOldOID)
 		}
@@ -610,7 +624,7 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi
 	} else {
 		oldRevision = parentCommitOID
 		if targetBranchCommit == "" {
-			oldRevision = git.ObjectHashSHA1.ZeroOID
+			oldRevision = objectHash.ZeroOID
 		} else if header.Force {
 			oldRevision = targetBranchCommit
 		}
@@ -627,7 +641,7 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi
 	return stream.SendAndClose(&gitalypb.UserCommitFilesResponse{BranchUpdate: &gitalypb.OperationBranchUpdate{
 		CommitId:      commitID.String(),
 		RepoCreated:   !hasBranches,
-		BranchCreated: git.ObjectHashSHA1.IsZeroOID(oldRevision),
+		BranchCreated: objectHash.IsZeroOID(oldRevision),
 	}})
 }
 
@@ -719,10 +733,7 @@ func (s *Server) fetchMissingCommit(
 	return nil
 }
 
-func validateUserCommitFilesHeader(locator storage.Locator, header *gitalypb.UserCommitFilesRequestHeader) error {
-	if err := locator.ValidateRepository(header.GetRepository()); err != nil {
-		return err
-	}
+func validateUserCommitFilesHeader(header *gitalypb.UserCommitFilesRequestHeader, objectHash git.ObjectHash) error {
 	if header.GetUser() == nil {
 		return errors.New("empty User")
 	}
@@ -735,7 +746,7 @@ func validateUserCommitFilesHeader(locator storage.Locator, header *gitalypb.Use
 
 	startSha := header.GetStartSha()
 	if len(startSha) > 0 {
-		err := git.ObjectHashSHA1.ValidateHex(startSha)
+		err := objectHash.ValidateHex(startSha)
 		if err != nil {
 			return err
 		}
diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go
index dd8365c59..d850c9c63 100644
--- a/internal/gitaly/service/operations/commit_files_test.go
+++ b/internal/gitaly/service/operations/commit_files_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package operations
 
 import (
@@ -999,7 +997,10 @@ func testUserCommitFilesStableCommitID(t *testing.T, ctx context.Context) {
 	resp, err := stream.CloseAndRecv()
 	require.NoError(t, err)
 
-	require.Equal(t, resp.BranchUpdate.CommitId, "23ec4ccd7fcc6ecf39431805bbff1cbcb6c23b9d")
+	require.Equal(t, resp.BranchUpdate.CommitId, gittest.ObjectHashDependent(t, map[string]string{
+		"sha1":   "23ec4ccd7fcc6ecf39431805bbff1cbcb6c23b9d",
+		"sha256": "0ab6f5df19cb4387f5b1bdac29ea497e12ad4ebd6c50c0a6ade01c75d2f5c5ad",
+	}))
 	require.True(t, resp.BranchUpdate.BranchCreated)
 	require.True(t, resp.BranchUpdate.RepoCreated)
 	gittest.RequireTree(t, cfg, repoPath, "refs/heads/master", []gittest.TreeEntry{
@@ -1009,8 +1010,14 @@ func testUserCommitFilesStableCommitID(t *testing.T, ctx context.Context) {
 	commit, err := repo.ReadCommit(ctx, "refs/heads/master")
 	require.NoError(t, err)
 	require.Equal(t, &gitalypb.GitCommit{
-		Id:       "23ec4ccd7fcc6ecf39431805bbff1cbcb6c23b9d",
-		TreeId:   "541550ddcf8a29bcd80b0800a142a7d47890cfd6",
+		Id: gittest.ObjectHashDependent(t, map[string]string{
+			"sha1":   "23ec4ccd7fcc6ecf39431805bbff1cbcb6c23b9d",
+			"sha256": "0ab6f5df19cb4387f5b1bdac29ea497e12ad4ebd6c50c0a6ade01c75d2f5c5ad",
+		}),
+		TreeId: gittest.ObjectHashDependent(t, map[string]string{
+			"sha1":   "541550ddcf8a29bcd80b0800a142a7d47890cfd6",
+			"sha256": "77313ea10ef747ceeb25c7177971d55b5cc67bf41cdb56e3497e905ebcad6303",
+		}),
 		Subject:  []byte("commit message"),
 		Body:     []byte("commit message"),
 		BodySize: 14,
@@ -1081,7 +1088,7 @@ func testUserCommitFilesQuarantine(t *testing.T, ctx context.Context) {
 	), err)
 
 	hookOutput := testhelper.MustReadFile(t, outputPath)
-	oid, err := git.ObjectHashSHA1.FromHex(text.ChompBytes(hookOutput))
+	oid, err := gittest.DefaultObjectHash.FromHex(text.ChompBytes(hookOutput))
 	require.NoError(t, err)
 	exists, err := repo.HasRevision(ctx, oid.Revision()+"^{commit}")
 	require.NoError(t, err)
diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go
index 87e3ca2d6..1bbb03ea1 100644
--- a/internal/gitaly/service/operations/merge_test.go
+++ b/internal/gitaly/service/operations/merge_test.go
@@ -35,7 +35,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb/testproto"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
-	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
@@ -1141,14 +1140,6 @@ func testUserMergeBranchAllowed(t *testing.T, ctx context.Context) {
 	}
 }
 
-func errWithDetails(tb testing.TB, err error, details ...proto.Message) error {
-	detailedErr := structerr.New("%w", err)
-	for _, detail := range details {
-		detailedErr = detailedErr.WithDetail(detail)
-	}
-	return detailedErr
-}
-
 func TestUserFFBranch(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/gitaly/service/operations/testhelper_test.go b/internal/gitaly/service/operations/testhelper_test.go
index b5bfbe38c..84ebaebf4 100644
--- a/internal/gitaly/service/operations/testhelper_test.go
+++ b/internal/gitaly/service/operations/testhelper_test.go
@@ -16,12 +16,14 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service/ssh"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitlab"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/client"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testserver"
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
+	"google.golang.org/protobuf/proto"
 )
 
 var (
@@ -187,3 +189,11 @@ func (s *testTransactionServer) VoteTransaction(ctx context.Context, in *gitalyp
 		State: gitalypb.VoteTransactionResponse_COMMIT,
 	}, nil
 }
+
+func errWithDetails(tb testing.TB, err error, details ...proto.Message) error {
+	detailedErr := structerr.New("%w", err)
+	for _, detail := range details {
+		detailedErr = detailedErr.WithDetail(detail)
+	}
+	return detailedErr
+}