diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2023-08-22 13:10:03 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2023-08-22 13:10:03 +0300 |
commit | 53eb3fe00d1ae3fbb3ad8608e2ef2d8f26c1fabc (patch) | |
tree | 1008b368f9e4127cefdf32c8a28a544c6c55f714 | |
parent | 87dc7898d15ee104c4a3285ae84f177f25918871 (diff) | |
parent | abdbb3b696862376e8c478c113f91e145f4372b1 (diff) |
Merge branch 'pks-user-rebase-confirmable-sha256' into 'master'
operations: Implement SHA256 support for UserRebaseConfirmable
Closes #5494
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6244
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
17 files changed, 305 insertions, 252 deletions
diff --git a/internal/gitaly/service/operations/apply_patch_test.go b/internal/gitaly/service/operations/apply_patch_test.go index 33579d10e..281f27600 100644 --- a/internal/gitaly/service/operations/apply_patch_test.go +++ b/internal/gitaly/service/operations/apply_patch_test.go @@ -30,7 +30,7 @@ func TestUserApplyPatch(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) errPatchingFailed := status.Error( codes.FailedPrecondition, @@ -605,7 +605,7 @@ func TestUserApplyPatch_stableID(t *testing.T) { ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) parentCommitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(git.DefaultBranch), gittest.WithTreeEntries( @@ -692,7 +692,7 @@ func TestUserApplyPatch_transactional(t *testing.T) { ctx := testhelper.Context(t) txManager := transaction.NewTrackingManager() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx, testserver.WithTransactionManager(txManager)) + ctx, cfg, client := setupOperationsService(t, ctx, testserver.WithTransactionManager(txManager)) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) parentCommitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(git.DefaultBranch), gittest.WithTreeEntries( @@ -756,7 +756,7 @@ func TestUserApplyPatch_validation(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, _ := gittest.CreateRepository(t, ctx, cfg) diff --git a/internal/gitaly/service/operations/cherry_pick_test.go b/internal/gitaly/service/operations/cherry_pick_test.go index 1b5ef721e..5d445eb7b 100644 --- a/internal/gitaly/service/operations/cherry_pick_test.go +++ b/internal/gitaly/service/operations/cherry_pick_test.go @@ -40,7 +40,7 @@ func testUserCherryPick(t *testing.T, ctx context.Context) { skipSHA256WithGit2goCherryPick(t, ctx) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) destinationBranch := "dst-branch" @@ -373,7 +373,7 @@ func testServerUserCherryPickSuccessfulGitHooks(t *testing.T, ctx context.Contex skipSHA256WithGit2goCherryPick(t, ctx) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -435,7 +435,7 @@ func testServerUserCherryPickMergeCommit(t *testing.T, ctx context.Context) { opts = append(opts, testserver.WithSigningKey("testdata/signing_ssh_key_ecdsa")) } - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx, opts...) + ctx, cfg, client := setupOperationsService(t, ctx, opts...) if featureflag.GPGSigning.IsEnabled(ctx) { testcfg.BuildGitalyGPG(t, cfg) @@ -545,7 +545,7 @@ func testServerUserCherryPickStableID(t *testing.T, ctx context.Context) { skipSHA256WithGit2goCherryPick(t, ctx) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -636,7 +636,7 @@ func testServerUserCherryPickFailedValidations(t *testing.T, ctx context.Context skipSHA256WithGit2goCherryPick(t, ctx) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -749,7 +749,7 @@ func testServerUserCherryPickFailedWithPreReceiveError(t *testing.T, ctx context skipSHA256WithGit2goCherryPick(t, ctx) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -814,7 +814,7 @@ func testServerUserCherryPickFailedWithCreateTreeError(t *testing.T, ctx context skipSHA256WithGit2goCherryPick(t, ctx) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -868,7 +868,7 @@ func testServerUserCherryPickFailedWithCommitError(t *testing.T, ctx context.Con skipSHA256WithGit2goCherryPick(t, ctx) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -931,7 +931,7 @@ func testServerUserCherryPickFailedWithConflict(t *testing.T, ctx context.Contex skipSHA256WithGit2goCherryPick(t, ctx) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -998,7 +998,7 @@ func testServerUserCherryPickSuccessfulWithGivenCommits(t *testing.T, ctx contex skipSHA256WithGit2goCherryPick(t, ctx) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -1076,7 +1076,7 @@ func testServerUserCherryPickQuarantine(t *testing.T, ctx context.Context) { skipSHA256WithGit2goCherryPick(t, ctx) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -1145,7 +1145,7 @@ func testServerUserCherryPickReverse(t *testing.T, ctx context.Context) { skipSHA256WithGit2goCherryPick(t, ctx) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go index 16a85d488..fbe770451 100644 --- a/internal/gitaly/service/operations/commit_files_test.go +++ b/internal/gitaly/service/operations/commit_files_test.go @@ -47,7 +47,7 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) { opts = append(opts, testserver.WithSigningKey("testdata/signing_gpg_key")) } - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx, opts...) + ctx, cfg, client := setupOperationsService(t, ctx, opts...) if featureflag.GPGSigning.IsEnabled(ctx) { testcfg.BuildGitalyGPG(t, cfg) @@ -973,7 +973,7 @@ func TestUserCommitFilesStableCommitID(t *testing.T) { func testUserCommitFilesStableCommitID(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -1047,7 +1047,7 @@ func TestUserCommitFilesQuarantine(t *testing.T) { func testUserCommitFilesQuarantine(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -1107,7 +1107,7 @@ func TestSuccessfulUserCommitFilesRequest(t *testing.T) { func testSuccessfulUserCommitFilesRequest(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) filePath := "héllo/wörld" authorName := []byte("Jane Doe") @@ -1342,7 +1342,7 @@ func TestUserCommitFiles_move(t *testing.T) { func testUserCommitFilesMove(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) branchName := "master" previousFilePath := "README" @@ -1430,7 +1430,7 @@ func TestSuccessUserCommitFilesRequestForceCommit(t *testing.T) { func testSuccessUserCommitFilesRequestForceCommit(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) authorName := []byte("Jane Doe") authorEmail := []byte("janedoe@gitlab.com") @@ -1477,7 +1477,7 @@ func TestSuccessUserCommitFilesRequestStartSha(t *testing.T) { func testSuccessUserCommitFilesRequestStartSha(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) @@ -1548,7 +1548,7 @@ func testUserCommitFilesRemoteRepository(t *testing.T, ctx context.Context) { t.Run(tc.desc, func(t *testing.T) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) newRepoProto, _ := gittest.CreateRepository(t, ctx, cfg) newRepo := localrepo.NewTestRepo(t, cfg, newRepoProto) @@ -1589,7 +1589,7 @@ func TestSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *tes func testSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, _ := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -1647,7 +1647,7 @@ func TestFailedUserCommitFilesRequestDueToHooks(t *testing.T) { func testFailedUserCommitFilesRequestDueToHooks(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("feature")) @@ -1704,7 +1704,7 @@ func TestFailedUserCommitFilesRequestDueToIndexError(t *testing.T) { } func testFailedUserCommitFilesRequestDueToIndexError(t *testing.T, ctx context.Context) { - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) type setupData struct { requests []*gitalypb.UserCommitFilesRequest @@ -1809,7 +1809,7 @@ func TestFailedUserCommitFilesRequest(t *testing.T) { func testFailedUserCommitFilesRequest(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) branchName := "feature" @@ -1893,7 +1893,7 @@ func TestUserCommitFilesFailsIfRepositoryMissing(t *testing.T) { } func testUserCommitFilesFailsIfRepositoryMissing(t *testing.T, ctx context.Context) { - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo := &gitalypb.Repository{ StorageName: cfg.Storages[0].Name, RelativePath: t.Name(), diff --git a/internal/gitaly/service/operations/ff_branch_test.go b/internal/gitaly/service/operations/ff_branch_test.go index f4caea6f8..b6d01b757 100644 --- a/internal/gitaly/service/operations/ff_branch_test.go +++ b/internal/gitaly/service/operations/ff_branch_test.go @@ -18,7 +18,7 @@ func TestUserFFBranch(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) type setupData struct { repoPath string @@ -324,7 +324,7 @@ func TestUserFFBranch_failingHooks(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) parentID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("branch")) @@ -354,7 +354,7 @@ func TestUserFFBranch_ambiguousReference(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) // We're creating both a branch and a tag with the same name. // If `git rev-parse` is called on the branch name directly diff --git a/internal/gitaly/service/operations/merge_branch_test.go b/internal/gitaly/service/operations/merge_branch_test.go index a4f74e722..a74b2fafd 100644 --- a/internal/gitaly/service/operations/merge_branch_test.go +++ b/internal/gitaly/service/operations/merge_branch_test.go @@ -51,7 +51,7 @@ func testUserMergeBranch(t *testing.T, ctx context.Context) { opts = append(opts, testserver.WithSigningKey("testdata/signing_gpg_key")) } - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx, opts...) + ctx, cfg, client := setupOperationsService(t, ctx, opts...) if featureflag.GPGSigning.IsEnabled(ctx) { testcfg.BuildGitalyGPG(t, cfg) @@ -364,7 +364,7 @@ func TestUserMergeBranch_failure(t *testing.T) { func testUserMergeBranchFailure(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) master := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("master"), gittest.WithTreeEntries( @@ -524,7 +524,7 @@ func TestUserMergeBranch_quarantine(t *testing.T) { func testUserMergeBranchQuarantine(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath, commits := setupRepoWithMergeableCommits(t, ctx, cfg, "branch") repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -587,7 +587,7 @@ func TestUserMergeBranch_stableMergeIDs(t *testing.T) { func testUserMergeBranchStableMergeIDs(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, _, commits := setupRepoWithMergeableCommits(t, ctx, cfg, "branch") repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -666,7 +666,7 @@ func TestUserMergeBranch_abort(t *testing.T) { func testUserMergeBranchAbort(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, _, commits := setupRepoWithMergeableCommits(t, ctx, cfg, "branch") repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -732,7 +732,7 @@ func TestUserMergeBranch_concurrentUpdate(t *testing.T) { func testUserMergeBranchConcurrentUpdate(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath, commits := setupRepoWithMergeableCommits(t, ctx, cfg, "branch") repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -806,7 +806,7 @@ func TestUserMergeBranch_ambiguousReference(t *testing.T) { func testUserMergeBranchAmbiguousReference(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath, commits := setupRepoWithMergeableCommits(t, ctx, cfg, "branch") repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -872,7 +872,7 @@ func TestUserMergeBranch_failingHooks(t *testing.T) { func testUserMergeBranchFailingHooks(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath, commits := setupRepoWithMergeableCommits(t, ctx, cfg, "branch") hookContent := []byte("#!/bin/sh\necho 'stdout' && echo 'stderr' >&2\nexit 1") @@ -972,7 +972,7 @@ func TestUserMergeBranch_conflict(t *testing.T) { func testUserMergeBranchConflict(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) const mergeIntoBranch = "mergeIntoBranch" const mergeFromBranch = "mergeFromBranch" diff --git a/internal/gitaly/service/operations/merge_to_ref_test.go b/internal/gitaly/service/operations/merge_to_ref_test.go index 54ca16345..25db51ec2 100644 --- a/internal/gitaly/service/operations/merge_to_ref_test.go +++ b/internal/gitaly/service/operations/merge_to_ref_test.go @@ -32,7 +32,7 @@ func TestUserMergeToRef_successful(t *testing.T) { func testUserMergeToRefSuccessful(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -167,7 +167,7 @@ func TestUserMergeToRef_conflicts(t *testing.T) { func testUserMergeToRefConflicts(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) common := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTreeEntries( @@ -230,7 +230,7 @@ func TestUserMergeToRef_stableMergeID(t *testing.T) { func testUserMergeToRefStableMergeID(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -304,7 +304,7 @@ func TestUserMergeToRef_failure(t *testing.T) { func testUserMergeToRefFailure(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) @@ -433,7 +433,7 @@ func TestUserMergeToRef_ignoreHooksRequest(t *testing.T) { func testUserMergeToRefIgnoreHooksRequest(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) common := gittest.WriteCommit(t, cfg, repoPath) diff --git a/internal/gitaly/service/operations/rebase_confirmable.go b/internal/gitaly/service/operations/rebase_confirmable.go index 340e161b5..ddaf6e944 100644 --- a/internal/gitaly/service/operations/rebase_confirmable.go +++ b/internal/gitaly/service/operations/rebase_confirmable.go @@ -39,8 +39,13 @@ func (s *Server) UserRebaseConfirmable(stream gitalypb.OperationService_UserReba return structerr.NewInternal("creating repo quarantine: %w", err) } + objectHash, err := quarantineRepo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("detecting object hash: %w", err) + } + branch := git.NewReferenceNameFromBranchName(string(header.Branch)) - oldrev, err := git.ObjectHashSHA1.FromHex(header.BranchSha) + oldrev, err := objectHash.FromHex(header.BranchSha) if err != nil { return structerr.NewNotFound("%w", err) } diff --git a/internal/gitaly/service/operations/rebase_confirmable_test.go b/internal/gitaly/service/operations/rebase_confirmable_test.go index 5e1ff1b21..7a7831d4c 100644 --- a/internal/gitaly/service/operations/rebase_confirmable_test.go +++ b/internal/gitaly/service/operations/rebase_confirmable_test.go @@ -1,5 +1,3 @@ -//go:build !gitaly_test_sha256 - package operations import ( @@ -13,6 +11,7 @@ import ( "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" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction" "gitlab.com/gitlab-org/gitaly/v16/internal/grpc/metadata" @@ -26,8 +25,6 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" ) -var rebaseBranchName = "many_files" - func TestUserRebaseConfirmable_successful(t *testing.T) { t.Parallel() @@ -38,35 +35,32 @@ func TestUserRebaseConfirmable_successful(t *testing.T) { } func testUserRebaseConfirmableSuccessful(t *testing.T, ctx context.Context) { + skipSHA256WithGit2goRebase(t, ctx) + t.Parallel() - ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) pushOptions := []string{"ci.skip", "test=value"} cfg.Gitlab.URL = setupAndStartGitlabServer(t, gittest.GlID, "project-1", cfg, pushOptions...) - repo := localrepo.NewTestRepo(t, cfg, repoProto) - - repoCopyProto, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ - Seed: gittest.SeedGitLabTest, - }) - - branchOID := gittest.ResolveRevision(t, cfg, repoPath, rebaseBranchName) + setup := setupRebasableRepositories(t, ctx, cfg, false) + localRepo := localrepo.NewTestRepo(t, cfg, setup.localRepo) rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) - preReceiveHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, "pre-receive") - postReceiveHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, "post-receive") + preReceiveHookOutputPath := gittest.WriteEnvToCustomHook(t, setup.localRepoPath, "pre-receive") + postReceiveHookOutputPath := gittest.WriteEnvToCustomHook(t, setup.localRepoPath, "post-receive") - headerRequest := buildUserRebaseConfirmableHeaderRequest(repoProto, gittest.TestUser, "1", rebaseBranchName, branchOID, repoCopyProto, "master") + headerRequest := buildUserRebaseConfirmableHeaderRequest(setup.localRepo, gittest.TestUser, "1", setup.localBranch, setup.localCommit, setup.remoteRepo, setup.remoteBranch) headerRequest.GetHeader().GitPushOptions = pushOptions require.NoError(t, rebaseStream.Send(headerRequest), "send header") firstResponse, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - _, err = repo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha())) + _, err = localRepo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha())) require.Equal(t, localrepo.ErrObjectNotFound, err, "commit should not exist in the normal repo given that it is quarantined") applyRequest := buildUserRebaseConfirmableApplyRequest(true) @@ -78,12 +72,11 @@ func testUserRebaseConfirmableSuccessful(t *testing.T, ctx context.Context) { _, err = rebaseStream.Recv() require.Equal(t, io.EOF, err) - _, err = repo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha())) + _, err = localRepo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha())) require.NoError(t, err) - newBranchCommit := gittest.ResolveRevision(t, cfg, repoPath, rebaseBranchName) - - require.NotEqual(t, newBranchCommit, branchOID) + newBranchCommit := gittest.ResolveRevision(t, cfg, setup.localRepoPath, setup.localBranch) + require.NotEqual(t, newBranchCommit, setup.localCommit) require.Equal(t, newBranchCommit.String(), firstResponse.GetRebaseSha()) require.True(t, secondResponse.GetRebaseApplied(), "the second rebase is applied") @@ -106,9 +99,13 @@ func TestUserRebaseConfirmable_skipEmptyCommits(t *testing.T) { } func testUserRebaseConfirmableSkipEmptyCommits(t *testing.T, ctx context.Context) { + skipSHA256WithGit2goRebase(t, ctx) + t.Parallel() - ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) + + repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) // This is the base commit from which both "theirs" and "ours" branch from". baseCommit := gittest.WriteCommit(t, cfg, repoPath, @@ -186,13 +183,19 @@ func testUserRebaseConfirmableSkipEmptyCommits(t *testing.T, ctx context.Context rebaseCommit, err := localrepo.NewTestRepo(t, cfg, repoProto).ReadCommit(ctx, rebaseOID.Revision()) require.NoError(t, err) testhelper.ProtoEqual(t, &gitalypb.GitCommit{ - Subject: []byte("ours with additional changes"), - Body: []byte("ours with additional changes"), - BodySize: 28, - Id: "ef7f98be1f753f1a9fa895d999a855611d691629", + Subject: []byte("ours with additional changes"), + Body: []byte("ours with additional changes"), + BodySize: 28, + Id: gittest.ObjectHashDependent(t, map[string]string{ + "sha1": "ef7f98be1f753f1a9fa895d999a855611d691629", + "sha256": "29c9b79bd0e742d7bb51ac0be5283f65fb806a94c19cb591b3621e58703164fa", + }), ParentIds: []string{theirs.String()}, - TreeId: "b68aeb18813d7f2e180f2cc0bccc128511438b29", - Author: gittest.DefaultCommitAuthor, + TreeId: gittest.ObjectHashDependent(t, map[string]string{ + "sha1": "b68aeb18813d7f2e180f2cc0bccc128511438b29", + "sha256": "17546e000464ad5829197d0a4fa52ca5fb42ad16150261f148002cd80013669a", + }), + Author: gittest.DefaultCommitAuthor, Committer: &gitalypb.CommitAuthor{ Name: gittest.TestUser.Name, Email: gittest.TestUser.Email, @@ -212,11 +215,13 @@ func TestUserRebaseConfirmable_transaction(t *testing.T) { } func testUserRebaseConfirmableTransaction(t *testing.T, ctx context.Context) { + skipSHA256WithGit2goRebase(t, ctx) + t.Parallel() txManager := transaction.NewTrackingManager() - ctx, cfg, repoProto, repoPath, client := setupOperationsService( + ctx, cfg, client := setupOperationsService( t, ctx, // Praefect would intercept our call and inject its own transaction. testserver.WithDisablePraefect(), @@ -224,8 +229,6 @@ func testUserRebaseConfirmableTransaction(t *testing.T, ctx context.Context) { ) cfg.Gitlab.URL = setupAndStartGitlabServer(t, gittest.GlID, "project-1", cfg) - repo := localrepo.NewTestRepo(t, cfg, repoProto) - for _, tc := range []struct { desc string withTransaction bool @@ -254,7 +257,8 @@ func testUserRebaseConfirmableTransaction(t *testing.T, ctx context.Context) { }, } { t.Run(tc.desc, func(t *testing.T) { - preReceiveHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, "pre-receive") + setup := setupRebasableRepositories(t, ctx, cfg, false) + preReceiveHookOutputPath := gittest.WriteEnvToCustomHook(t, setup.localRepoPath, "pre-receive") txManager.Reset() @@ -268,13 +272,10 @@ func testUserRebaseConfirmableTransaction(t *testing.T, ctx context.Context) { ctx = metadata.IncomingToOutgoing(ctx) } - branchCommitID, err := repo.ResolveRevision(ctx, git.Revision(rebaseBranchName)) - require.NoError(t, err) - rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) - headerRequest := buildUserRebaseConfirmableHeaderRequest(repoProto, gittest.TestUser, "1", rebaseBranchName, branchCommitID, repoProto, "master") + headerRequest := buildUserRebaseConfirmableHeaderRequest(setup.localRepo, gittest.TestUser, "1", setup.localBranch, setup.localCommit, setup.remoteRepo, setup.remoteBranch) require.NoError(t, rebaseStream.Send(headerRequest)) _, err = rebaseStream.Recv() require.NoError(t, err) @@ -308,37 +309,44 @@ func TestUserRebaseConfirmable_stableCommitIDs(t *testing.T) { } func testUserRebaseConfirmableStableCommitIDs(t *testing.T, ctx context.Context) { + skipSHA256WithGit2goRebase(t, ctx) + t.Parallel() - ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) cfg.Gitlab.URL = setupAndStartGitlabServer(t, gittest.GlID, "project-1", cfg) - repo := localrepo.NewTestRepo(t, cfg, repoProto) + setup := setupRebasableRepositories(t, ctx, cfg, false) + localRepo := localrepo.NewTestRepo(t, cfg, setup.localRepo) rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) committerDate := ×tamppb.Timestamp{Seconds: 100000000} - parentSha := gittest.ResolveRevision(t, cfg, repoPath, "master") require.NoError(t, rebaseStream.Send(&gitalypb.UserRebaseConfirmableRequest{ UserRebaseConfirmableRequestPayload: &gitalypb.UserRebaseConfirmableRequest_Header_{ Header: &gitalypb.UserRebaseConfirmableRequest_Header{ - Repository: repoProto, + Repository: setup.localRepo, User: gittest.TestUser, RebaseId: "1", - Branch: []byte(rebaseBranchName), - BranchSha: gittest.ResolveRevision(t, cfg, repoPath, rebaseBranchName).String(), - RemoteRepository: repoProto, - RemoteBranch: []byte("master"), + Branch: []byte(setup.localBranch), + BranchSha: setup.localCommit.String(), + RemoteRepository: setup.remoteRepo, + RemoteBranch: []byte(setup.remoteBranch), Timestamp: committerDate, }, }, }), "send header") + expectedCommitID := gittest.ObjectHashDependent(t, map[string]string{ + "sha1": "85b0186925c57efa608939afea01b627a2f4d4cf", + "sha256": "a14d9fb56edf718b4aaeaabd2de8cd2403820396ee905f9c87337c5bea8598cf", + }) + response, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - require.Equal(t, "c52b98024db0d3af0ccb20ed2a3a93a21cfbba87", response.GetRebaseSha()) + require.Equal(t, expectedCommitID, response.GetRebaseSha()) applyRequest := buildUserRebaseConfirmableApplyRequest(true) require.NoError(t, rebaseStream.Send(applyRequest), "apply rebase") @@ -350,22 +358,19 @@ func testUserRebaseConfirmableStableCommitIDs(t *testing.T, ctx context.Context) _, err = rebaseStream.Recv() require.Equal(t, io.EOF, err) - commit, err := repo.ReadCommit(ctx, git.Revision(rebaseBranchName)) + commit, err := localRepo.ReadCommit(ctx, git.Revision(setup.localBranch)) require.NoError(t, err, "look up git commit") testhelper.ProtoEqual(t, &gitalypb.GitCommit{ - Subject: []byte("Add a directory with many files to allow testing of default 1,000 entry limit"), - Body: []byte("Add a directory with many files to allow testing of default 1,000 entry limit\n\nFor performance reasons, GitLab will add a file viewer limit and only show\nthe first 1,000 entries in a directory. Having this directory with many\nempty files in the test project will make the test easy.\n"), - BodySize: 283, - Id: "c52b98024db0d3af0ccb20ed2a3a93a21cfbba87", - ParentIds: []string{parentSha.String()}, - TreeId: "d0305132f880aa0ab4102e56a09cf1343ba34893", - Author: &gitalypb.CommitAuthor{ - Name: []byte("Drew Blessing"), - Email: []byte("drew@gitlab.com"), - // Nanoseconds get ignored because commit timestamps aren't that granular. - Date: ×tamppb.Timestamp{Seconds: 1510610637}, - Timezone: []byte("-0600"), - }, + Subject: []byte("message"), + Body: []byte("message"), + BodySize: 7, + Id: expectedCommitID, + ParentIds: []string{setup.remoteCommit.String()}, + TreeId: gittest.ObjectHashDependent(t, map[string]string{ + "sha1": "a3eb530e96ad4d04d646c3fb5f30ad4807d300b4", + "sha256": "633ab76f30bf7f3766ba215255972f6ee89f0c54bff5af122743c78ddde07d9e", + }), + Author: gittest.DefaultCommitAuthor, Committer: &gitalypb.CommitAuthor{ Name: gittest.TestUser.Name, Email: gittest.TestUser.Email, @@ -386,15 +391,14 @@ func TestUserRebaseConfirmable_inputValidation(t *testing.T) { } func testUserRebaseConfirmableInputValidation(t *testing.T, ctx context.Context) { - t.Parallel() + skipSHA256WithGit2goRebase(t, ctx) - ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx) + t.Parallel() - repoCopy, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ - Seed: gittest.SeedGitLabTest, - }) + ctx, cfg, client := setupOperationsService(t, ctx) - branchCommitID := gittest.ResolveRevision(t, cfg, repoPath, rebaseBranchName) + repo, repoPath := gittest.CreateRepository(t, ctx, cfg) + commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("branch")) testCases := []struct { desc string @@ -402,31 +406,31 @@ func testUserRebaseConfirmableInputValidation(t *testing.T, ctx context.Context) }{ { desc: "repository not set", - req: buildUserRebaseConfirmableHeaderRequest(nil, gittest.TestUser, "1", rebaseBranchName, branchCommitID, repoCopy, "master"), + req: buildUserRebaseConfirmableHeaderRequest(nil, gittest.TestUser, "1", "branch", commitID, repo, "branch"), }, { desc: "empty User", - req: buildUserRebaseConfirmableHeaderRequest(repo, nil, "1", rebaseBranchName, branchCommitID, repoCopy, "master"), + req: buildUserRebaseConfirmableHeaderRequest(repo, nil, "1", "branch", commitID, repo, "branch"), }, { desc: "empty Branch", - req: buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", "", branchCommitID, repoCopy, "master"), + req: buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", "", commitID, repo, "branch"), }, { desc: "empty BranchSha", - req: buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", rebaseBranchName, "", repoCopy, "master"), + req: buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", "branch", "", repo, "branch"), }, { desc: "empty RemoteRepository", - req: buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", rebaseBranchName, branchCommitID, nil, "master"), + req: buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", "branch", commitID, nil, "branch"), }, { desc: "empty RemoteBranch", - req: buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", rebaseBranchName, branchCommitID, repoCopy, ""), + req: buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", "branch", commitID, repo, ""), }, { desc: "invalid branch name", - req: buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", rebaseBranchName, branchCommitID, repoCopy, "+dev:master"), + req: buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", "branch", commitID, repo, "+dev:branch"), }, } @@ -455,9 +459,11 @@ func TestUserRebaseConfirmable_abortViaClose(t *testing.T) { } func testUserRebaseConfirmableAbortViaClose(t *testing.T, ctx context.Context) { + skipSHA256WithGit2goRebase(t, ctx) + t.Parallel() - ctx, cfg, _, _, client := setupOperationsService(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) testCases := []struct { desc string @@ -497,13 +503,9 @@ func testUserRebaseConfirmableAbortViaClose(t *testing.T, ctx context.Context) { for i, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - createRepoOpts := gittest.CreateRepositoryConfig{Seed: gittest.SeedGitLabTest} - testRepo, testRepoPath := gittest.CreateRepository(t, ctx, cfg, createRepoOpts) - testRepoCopy, _ := gittest.CreateRepository(t, ctx, cfg, createRepoOpts) + setup := setupRebasableRepositories(t, ctx, cfg, true) - branchCommitID := gittest.ResolveRevision(t, cfg, testRepoPath, rebaseBranchName) - - headerRequest := buildUserRebaseConfirmableHeaderRequest(testRepo, gittest.TestUser, fmt.Sprintf("%v", i), rebaseBranchName, branchCommitID, testRepoCopy, "master") + headerRequest := buildUserRebaseConfirmableHeaderRequest(setup.localRepo, gittest.TestUser, fmt.Sprintf("%v", i), setup.localBranch, setup.localCommit, setup.remoteRepo, setup.remoteBranch) rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) @@ -526,8 +528,8 @@ func testUserRebaseConfirmableAbortViaClose(t *testing.T, ctx context.Context) { testhelper.RequireGrpcError(t, tc.expectedErr, err) require.Nil(t, secondResponse) - newBranchCommitID := gittest.ResolveRevision(t, cfg, testRepoPath, rebaseBranchName) - require.Equal(t, newBranchCommitID, branchCommitID, "branch should not change when the rebase is aborted") + newBranchCommitID := gittest.ResolveRevision(t, cfg, setup.localRepoPath, setup.localBranch) + require.Equal(t, newBranchCommitID, setup.localCommit, "branch should not change when the rebase is aborted") }) } } @@ -542,28 +544,24 @@ func TestUserRebaseConfirmable_abortViaApply(t *testing.T) { } func testUserRebaseConfirmableAbortViaApply(t *testing.T, ctx context.Context) { - t.Parallel() - - ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx) + skipSHA256WithGit2goRebase(t, ctx) - repo := localrepo.NewTestRepo(t, cfg, repoProto) - - testRepoCopy, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ - Seed: gittest.SeedGitLabTest, - }) + t.Parallel() - branchCommitID := gittest.ResolveRevision(t, cfg, repoPath, rebaseBranchName) + ctx, cfg, client := setupOperationsService(t, ctx) + setup := setupRebasableRepositories(t, ctx, cfg, true) + localRepo := localrepo.NewTestRepo(t, cfg, setup.localRepo) rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) - headerRequest := buildUserRebaseConfirmableHeaderRequest(repoProto, gittest.TestUser, "1", rebaseBranchName, branchCommitID, testRepoCopy, "master") + headerRequest := buildUserRebaseConfirmableHeaderRequest(setup.localRepo, gittest.TestUser, "1", setup.localBranch, setup.localCommit, setup.remoteRepo, setup.remoteBranch) require.NoError(t, rebaseStream.Send(headerRequest), "send header") firstResponse, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - _, err = repo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha())) + _, err = localRepo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha())) require.Equal(t, localrepo.ErrObjectNotFound, err, "commit should not exist in the normal repo given that it is quarantined") applyRequest := buildUserRebaseConfirmableApplyRequest(false) @@ -574,11 +572,11 @@ func testUserRebaseConfirmableAbortViaApply(t *testing.T, ctx context.Context) { testhelper.RequireGrpcCode(t, err, codes.FailedPrecondition) require.False(t, secondResponse.GetRebaseApplied(), "the second rebase is not applied") - _, err = repo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha())) + _, err = localRepo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha())) require.Equal(t, localrepo.ErrObjectNotFound, err, "commit should have been discarded") - newBranchCommitID := gittest.ResolveRevision(t, cfg, repoPath, rebaseBranchName) - require.Equal(t, branchCommitID, newBranchCommitID, "branch should not change when the rebase is not applied") + newBranchCommitID := gittest.ResolveRevision(t, cfg, setup.localRepoPath, setup.localBranch) + require.Equal(t, setup.localCommit, newBranchCommitID, "branch should not change when the rebase is not applied") require.NotEqual(t, newBranchCommitID, firstResponse.GetRebaseSha(), "branch should not be the sha returned when the rebase is not applied") } @@ -592,33 +590,31 @@ func TestUserRebaseConfirmable_preReceiveError(t *testing.T) { } func testUserRebaseConfirmablePreReceiveError(t *testing.T, ctx context.Context) { - t.Parallel() + skipSHA256WithGit2goRebase(t, ctx) - ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx) - repo := localrepo.NewTestRepo(t, cfg, repoProto) + t.Parallel() - repoCopyProto, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ - Seed: gittest.SeedGitLabTest, - }) + ctx, cfg, client := setupOperationsService(t, ctx) - branchCommitID := gittest.ResolveRevision(t, cfg, repoPath, rebaseBranchName) + setup := setupRebasableRepositories(t, ctx, cfg, true) + localRepo := localrepo.NewTestRepo(t, cfg, setup.localRepo) hookContent := []byte("#!/bin/sh\necho 'failure'\nexit 1") for i, hookName := range GitlabPreHooks { t.Run(hookName, func(t *testing.T) { - gittest.WriteCustomHook(t, repoPath, hookName, hookContent) + gittest.WriteCustomHook(t, setup.localRepoPath, hookName, hookContent) rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) - headerRequest := buildUserRebaseConfirmableHeaderRequest(repoProto, gittest.TestUser, fmt.Sprintf("%v", i), rebaseBranchName, branchCommitID, repoCopyProto, "master") + headerRequest := buildUserRebaseConfirmableHeaderRequest(setup.localRepo, gittest.TestUser, fmt.Sprintf("%v", i), setup.localBranch, setup.localCommit, setup.remoteRepo, setup.remoteBranch) require.NoError(t, rebaseStream.Send(headerRequest), "send header") firstResponse, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - _, err = repo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha())) + _, err = localRepo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha())) require.Equal(t, localrepo.ErrObjectNotFound, err, "commit should not exist in the normal repo given that it is quarantined") applyRequest := buildUserRebaseConfirmableApplyRequest(true) @@ -637,69 +633,69 @@ func testUserRebaseConfirmablePreReceiveError(t *testing.T, ctx context.Context) }, ), err) - _, err = repo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha())) + _, err = localRepo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha())) if hookName == "pre-receive" { require.Equal(t, localrepo.ErrObjectNotFound, err, "commit should have been discarded") } else { require.NoError(t, err) } - newBranchCommitID := gittest.ResolveRevision(t, cfg, repoPath, rebaseBranchName) - require.Equal(t, branchCommitID, newBranchCommitID, "branch should not change when the rebase fails due to PreReceiveError") + newBranchCommitID := gittest.ResolveRevision(t, cfg, setup.localRepoPath, setup.localBranch) + require.Equal(t, setup.localCommit, newBranchCommitID, "branch should not change when the rebase fails due to PreReceiveError") require.NotEqual(t, newBranchCommitID, firstResponse.GetRebaseSha(), "branch should not be the sha returned when the rebase fails due to PreReceiveError") }) } } -func TestUserRebaseConfirmable_gitError(t *testing.T) { +func TestUserRebaseConfirmable_mergeConflict(t *testing.T) { t.Parallel() testhelper.NewFeatureSets( featureflag.GPGSigning, featureflag.UserRebaseConfirmablePureGit, - ).Run(t, testUserRebaseConfirmableGitError) + ).Run(t, testUserRebaseConfirmableMergeConflict) } -func testUserRebaseConfirmableGitError(t *testing.T, ctx context.Context) { - t.Parallel() +func testUserRebaseConfirmableMergeConflict(t *testing.T, ctx context.Context) { + skipSHA256WithGit2goRebase(t, ctx) - ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx) + t.Parallel() - repoCopyProto, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ - Seed: gittest.SeedGitLabTest, - }) + ctx, cfg, client := setupOperationsService(t, ctx) - targetBranch := "rebase-encoding-failure-trigger" - targetBranchCommitID := gittest.ResolveRevision(t, cfg, repoPath, targetBranch) - sourceBranchCommitID := gittest.ResolveRevision(t, cfg, repoPath, "master") + setup := setupRebasableRepositories(t, ctx, cfg, true) + localConflictingCommit := gittest.WriteCommit(t, cfg, setup.localRepoPath, gittest.WithBranch(setup.localBranch), gittest.WithTreeEntries( + gittest.TreeEntry{Path: "local", Mode: "100644", Content: "local\n"}, + gittest.TreeEntry{Path: "remote", Mode: "100644", Content: "remote-conflict\n"}, + ), gittest.WithParents(setup.localCommit)) rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) - headerRequest := buildUserRebaseConfirmableHeaderRequest(repoProto, gittest.TestUser, "1", targetBranch, targetBranchCommitID, repoCopyProto, "master") + headerRequest := buildUserRebaseConfirmableHeaderRequest(setup.localRepo, gittest.TestUser, "1", setup.localBranch, localConflictingCommit, setup.remoteRepo, setup.remoteBranch) require.NoError(t, rebaseStream.Send(headerRequest), "send header") response, err := rebaseStream.Recv() require.Nil(t, response) - testhelper.RequireGrpcError(t, structerr.NewFailedPrecondition(`rebasing commits: rebase: commit "eb8f5fb9523b868cef583e09d4bf70b99d2dd404": there are conflicting files`).WithDetail( + testhelper.RequireGrpcError(t, structerr.NewFailedPrecondition(`rebasing commits: rebase: commit %q: there are conflicting files`, localConflictingCommit).WithDetail( &gitalypb.UserRebaseConfirmableError{ Error: &gitalypb.UserRebaseConfirmableError_RebaseConflict{ RebaseConflict: &gitalypb.MergeConflictError{ ConflictingFiles: [][]byte{ - []byte("README.md"), + []byte("remote"), }, ConflictingCommitIds: []string{ - sourceBranchCommitID.String(), - targetBranchCommitID.String(), + setup.remoteCommit.String(), + localConflictingCommit.String(), }, }, }, }, ), err) - newBranchCommitID := gittest.ResolveRevision(t, cfg, repoPath, targetBranch) - require.Equal(t, targetBranchCommitID, newBranchCommitID, "branch should not change when the rebase fails due to GitError") + newBranchCommitID := gittest.ResolveRevision(t, cfg, setup.localRepoPath, setup.localBranch) + require.Equal(t, localConflictingCommit, newBranchCommitID, "branch should not change when the rebase fails due to GitError") } func TestUserRebaseConfirmable_deletedFileInLocalRepo(t *testing.T) { @@ -712,9 +708,11 @@ func TestUserRebaseConfirmable_deletedFileInLocalRepo(t *testing.T) { } func testUserRebaseConfirmableDeletedFileInLocalRepo(t *testing.T, ctx context.Context) { + skipSHA256WithGit2goRebase(t, ctx) + t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) localRepoProto, localRepoPath := gittest.CreateRepository(t, ctx, cfg) localRepo := localrepo.NewTestRepo(t, cfg, localRepoProto) @@ -790,9 +788,11 @@ func TestUserRebaseConfirmable_deletedFileInRemoteRepo(t *testing.T) { } func testUserRebaseConfirmableDeletedFileInRemoteRepo(t *testing.T, ctx context.Context) { + skipSHA256WithGit2goRebase(t, ctx) + t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) localRepoProto, localRepoPath := gittest.CreateRepository(t, ctx, cfg) localRepo := localrepo.NewTestRepo(t, cfg, localRepoProto) @@ -862,11 +862,13 @@ func TestUserRebaseConfirmable_failedWithCode(t *testing.T) { } func testUserRebaseConfirmableFailedWithCode(t *testing.T, ctx context.Context) { - t.Parallel() + skipSHA256WithGit2goRebase(t, ctx) - ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx) + t.Parallel() - branchCommitID := gittest.ResolveRevision(t, cfg, repoPath, rebaseBranchName) + ctx, cfg, client := setupOperationsService(t, ctx) + repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) + commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("master")) testCases := []struct { desc string @@ -876,7 +878,7 @@ func testUserRebaseConfirmableFailedWithCode(t *testing.T, ctx context.Context) { desc: "no repository provided", buildHeaderRequest: func() *gitalypb.UserRebaseConfirmableRequest { - return buildUserRebaseConfirmableHeaderRequest(nil, gittest.TestUser, "1", rebaseBranchName, branchCommitID, nil, "master") + return buildUserRebaseConfirmableHeaderRequest(nil, gittest.TestUser, "1", "master", commitID, nil, "master") }, expectedErr: structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet), }, @@ -886,7 +888,7 @@ func testUserRebaseConfirmableFailedWithCode(t *testing.T, ctx context.Context) repo := proto.Clone(repoProto).(*gitalypb.Repository) repo.StorageName = "@this-storage-does-not-exist" - return buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", rebaseBranchName, branchCommitID, repo, "master") + return buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", "master", commitID, repo, "master") }, expectedErr: testhelper.ToInterceptedMetadata(structerr.NewInvalidArgument( "%w", storage.NewStorageNotFoundError("@this-storage-does-not-exist"), @@ -898,7 +900,7 @@ func testUserRebaseConfirmableFailedWithCode(t *testing.T, ctx context.Context) repo := proto.Clone(repoProto).(*gitalypb.Repository) repo.RelativePath = "" - return buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", rebaseBranchName, branchCommitID, repo, "master") + return buildUserRebaseConfirmableHeaderRequest(repo, gittest.TestUser, "1", "master", commitID, repo, "master") }, expectedErr: structerr.NewInvalidArgument("%w", storage.ErrRepositoryPathNotSet), }, @@ -941,3 +943,62 @@ func buildUserRebaseConfirmableApplyRequest(apply bool) *gitalypb.UserRebaseConf }, } } + +type rebasableRepositoriesSetup struct { + localRepo *gitalypb.Repository + localRepoPath string + localCommit git.ObjectID + localBranch string + + remoteRepo *gitalypb.Repository + remoteRepoPath string + remoteCommit git.ObjectID + remoteBranch string + + commonCommit git.ObjectID +} + +func setupRebasableRepositories(tb testing.TB, ctx context.Context, cfg config.Cfg, separateRepos bool) rebasableRepositoriesSetup { + tb.Helper() + + localRepo, localRepoPath := gittest.CreateRepository(tb, ctx, cfg) + localCommonCommit := gittest.WriteCommit(tb, cfg, localRepoPath, gittest.WithTreeEntries( + gittest.TreeEntry{Path: "local", Mode: "100644", Content: "local\n"}, + gittest.TreeEntry{Path: "remote", Mode: "100644", Content: "remote\n"}, + )) + localDivergingCommit := gittest.WriteCommit(tb, cfg, localRepoPath, gittest.WithParents(localCommonCommit), gittest.WithTreeEntries( + gittest.TreeEntry{Path: "local", Mode: "100644", Content: "local-changed\n"}, + gittest.TreeEntry{Path: "remote", Mode: "100644", Content: "remote\n"}, + ), gittest.WithBranch("branch-local")) + + var remoteRepo *gitalypb.Repository + var remoteRepoPath string + if separateRepos { + remoteRepo, remoteRepoPath = gittest.CreateRepository(tb, ctx, cfg) + } else { + remoteRepo, remoteRepoPath = localRepo, localRepoPath + } + + repoBBaseCommitID := gittest.WriteCommit(tb, cfg, remoteRepoPath, gittest.WithTreeEntries( + gittest.TreeEntry{Path: "local", Mode: "100644", Content: "local\n"}, + gittest.TreeEntry{Path: "remote", Mode: "100644", Content: "remote\n"}, + )) + remoteDivergingCommit := gittest.WriteCommit(tb, cfg, remoteRepoPath, gittest.WithParents(repoBBaseCommitID), gittest.WithTreeEntries( + gittest.TreeEntry{Path: "local", Mode: "100644", Content: "local\n"}, + gittest.TreeEntry{Path: "remote", Mode: "100644", Content: "remote-changed\n"}, + ), gittest.WithBranch("branch-remote")) + + require.Equal(tb, localCommonCommit, repoBBaseCommitID) + + return rebasableRepositoriesSetup{ + localRepo: localRepo, localRepoPath: localRepoPath, localCommit: localDivergingCommit, localBranch: "branch-local", + remoteRepo: remoteRepo, remoteRepoPath: remoteRepoPath, remoteCommit: remoteDivergingCommit, remoteBranch: "branch-remote", + commonCommit: localCommonCommit, + } +} + +func skipSHA256WithGit2goRebase(t *testing.T, ctx context.Context) { + if gittest.DefaultObjectHash.Format == git.ObjectHashSHA256.Format && featureflag.UserRebaseConfirmablePureGit.IsDisabled(ctx) { + t.Skip("SHA256 repositories are only supported when using the pure Git implementation") + } +} diff --git a/internal/gitaly/service/operations/rebase_to_ref_test.go b/internal/gitaly/service/operations/rebase_to_ref_test.go index 5f703f338..ed6caa0d0 100644 --- a/internal/gitaly/service/operations/rebase_to_ref_test.go +++ b/internal/gitaly/service/operations/rebase_to_ref_test.go @@ -31,7 +31,7 @@ func testUserRebaseToRefSuccessful(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -97,7 +97,7 @@ func testUserRebaseToRefFailure(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) mergeBaseOID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("first commit")) @@ -225,7 +225,7 @@ func testUserRebaseToRefConflict(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) mergeBaseOID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("first commit")) diff --git a/internal/gitaly/service/operations/revert_test.go b/internal/gitaly/service/operations/revert_test.go index bef89503c..225669662 100644 --- a/internal/gitaly/service/operations/revert_test.go +++ b/internal/gitaly/service/operations/revert_test.go @@ -33,7 +33,7 @@ func TestUserRevert(t *testing.T) { func testUserRevert(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) branchName := "revert-branch" @@ -433,7 +433,7 @@ func TestServer_UserRevert_quarantine(t *testing.T) { func testServerUserRevertQuarantine(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -499,7 +499,7 @@ func testServerUserRevertMergeCommit(t *testing.T, ctx context.Context) { opts = append(opts, testserver.WithSigningKey("testdata/signing_ssh_key_rsa")) } - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx, opts...) + ctx, cfg, client := setupOperationsService(t, ctx, opts...) if featureflag.GPGSigning.IsEnabled(ctx) { testcfg.BuildGitalyGPG(t, cfg) @@ -605,7 +605,7 @@ func TestServer_UserRevert_stableID(t *testing.T) { func testServerUserRevertStableID(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -701,7 +701,7 @@ func TestServer_UserRevert_successfulIntoEmptyRepo(t *testing.T) { func testServerUserRevertSuccessfulIntoEmptyRepo(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) startRepoProto, startRepoPath := gittest.CreateRepository(t, ctx, cfg) startRepo := localrepo.NewTestRepo(t, cfg, startRepoProto) @@ -773,7 +773,7 @@ func TestServer_UserRevert_successfulGitHooks(t *testing.T) { func testServerUserRevertSuccessfulGitHooks(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -828,7 +828,7 @@ func TestServer_UserRevert_failedDueToPreReceiveError(t *testing.T) { func testServerUserRevertFailedDueToPreReceiveError(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -877,7 +877,7 @@ func TestServer_UserRevert_failedDueToCreateTreeErrorConflict(t *testing.T) { func testServerUserRevertFailedDueToCreateTreeErrorConflict(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -934,7 +934,7 @@ func TestServer_UserRevert_failedDueToCreateTreeErrorEmpty(t *testing.T) { func testServerUserRevertFailedDueToCreateTreeErrorEmpty(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -1009,7 +1009,7 @@ func TestServer_UserRevert_failedDueToCommitError(t *testing.T) { func testServerUserRevertFailedDueToCommitError(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) diff --git a/internal/gitaly/service/operations/squash_test.go b/internal/gitaly/service/operations/squash_test.go index 6084ab68c..3f1267102 100644 --- a/internal/gitaly/service/operations/squash_test.go +++ b/internal/gitaly/service/operations/squash_test.go @@ -53,7 +53,7 @@ func testUserSquashSuccessful(t *testing.T, ctx context.Context) { opts = append(opts, testserver.WithSigningKey("testdata/signing_ssh_key_ed25519")) } - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx, opts...) + ctx, cfg, client := setupOperationsService(t, ctx, opts...) testcfg.BuildGitalyGPG(t, cfg) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) @@ -121,7 +121,7 @@ func testUserSquashTransactional(t *testing.T, ctx context.Context) { txManager := transaction.MockManager{} - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx, + ctx, cfg, client := setupOperationsService(t, ctx, testserver.WithTransactionManager(&txManager), ) @@ -248,7 +248,7 @@ func TestUserSquash_stableID(t *testing.T) { func testUserSquashStableID(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) @@ -312,7 +312,7 @@ func TestUserSquash_threeWayMerge(t *testing.T) { func testUserSquashThreeWayMerge(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -367,7 +367,7 @@ func TestUserSquash_renames(t *testing.T) { func testUserSquashRenames(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) @@ -445,7 +445,7 @@ func TestUserSquash_missingFileOnTargetBranch(t *testing.T) { func testUserSquashMissingFileOnTargetBranch(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) @@ -484,7 +484,7 @@ func TestUserSquash_emptyCommit(t *testing.T) { func testUserSquashEmptyCommit(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -600,7 +600,7 @@ func TestUserSquash_validation(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) commitID := gittest.WriteCommit(t, cfg, repoPath) @@ -703,7 +703,7 @@ func TestUserSquash_conflicts(t *testing.T) { func testUserSquashConflicts(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) @@ -760,7 +760,7 @@ func TestUserSquash_ancestry(t *testing.T) { func testUserSquashAncestry(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) @@ -806,7 +806,7 @@ func TestUserSquash_gitError(t *testing.T) { func testUserSquashGitError(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) commitID := gittest.WriteCommit(t, cfg, repoPath) @@ -917,7 +917,7 @@ func TestUserSquash_squashingMerge(t *testing.T) { func testUserSquashSquashingMerge(t *testing.T, ctx context.Context) { t.Parallel() - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) diff --git a/internal/gitaly/service/operations/submodules_test.go b/internal/gitaly/service/operations/submodules_test.go index b2d59fd4c..ba17c9d2c 100644 --- a/internal/gitaly/service/operations/submodules_test.go +++ b/internal/gitaly/service/operations/submodules_test.go @@ -28,7 +28,7 @@ func TestUserUpdateSubmodule(t *testing.T) { } func testUserUpdateSubmodule(t *testing.T, ctx context.Context) { - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) type setupData struct { request *gitalypb.UserUpdateSubmoduleRequest diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go index bbcf7644d..ce1b1747e 100644 --- a/internal/gitaly/service/operations/tags_test.go +++ b/internal/gitaly/service/operations/tags_test.go @@ -29,7 +29,7 @@ func TestUserDeleteTag(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) type setupData struct { repoPath string @@ -297,7 +297,7 @@ func TestUserDeleteTag_hooks(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) for _, hookName := range GitlabHooks { hookName := hookName @@ -376,7 +376,7 @@ func TestUserCreateTag_successful(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -572,7 +572,7 @@ func TestUserCreateTag_quarantine(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -636,7 +636,7 @@ func TestUserCreateTag_message(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) for _, tc := range []struct { desc string @@ -732,7 +732,7 @@ func TestUserCreateTag_targetRevision(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) for _, tc := range []struct { desc string @@ -832,7 +832,7 @@ func TestUserCreateTag_nonCommitTarget(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) blobID := gittest.WriteBlob(t, cfg, repoPath, []byte("content")) @@ -925,7 +925,7 @@ func TestUserCreateTag_nestedTags(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -1045,7 +1045,7 @@ func TestUserCreateTag_stableTagIDs(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -1080,7 +1080,7 @@ func TestUserCreateTag_prefixedTag(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) @@ -1116,7 +1116,7 @@ func TestUserCreateTag_gitHooks(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) for _, hookName := range GitlabHooks { t.Run(hookName, func(t *testing.T) { @@ -1155,7 +1155,7 @@ func TestUserDeleteTag_hookFailure(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) commitID := gittest.WriteCommit(t, cfg, repoPath) @@ -1185,7 +1185,7 @@ func TestUserCreateTag_hookFailure(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) for _, tc := range []struct { hook string @@ -1235,7 +1235,7 @@ func TestUserCreateTag_preexisting(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents()) @@ -1290,7 +1290,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents()) @@ -1418,7 +1418,7 @@ func TestTagHookOutput(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) for _, tc := range []struct { desc string diff --git a/internal/gitaly/service/operations/testhelper_test.go b/internal/gitaly/service/operations/testhelper_test.go index 46fd62b51..a21c8b995 100644 --- a/internal/gitaly/service/operations/testhelper_test.go +++ b/internal/gitaly/service/operations/testhelper_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/require" gitalyauth "gitlab.com/gitlab-org/gitaly/v16/auth" - "gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service/commit" @@ -38,18 +37,6 @@ func TestMain(m *testing.M) { testhelper.Run(m) } -func setupOperationsService(tb testing.TB, ctx context.Context, options ...testserver.GitalyServerOpt) (context.Context, config.Cfg, *gitalypb.Repository, string, gitalypb.OperationServiceClient) { - cfg := testcfg.Build(tb) - - ctx, cfg, client := setupOperationsServiceWithCfg(tb, ctx, cfg, options...) - - repo, repoPath := gittest.CreateRepository(tb, ctx, cfg, gittest.CreateRepositoryConfig{ - Seed: gittest.SeedGitLabTest, - }) - - return ctx, cfg, repo, repoPath, client -} - func setupOperationsServiceWithCfg( tb testing.TB, ctx context.Context, cfg config.Cfg, options ...testserver.GitalyServerOpt, ) (context.Context, config.Cfg, gitalypb.OperationServiceClient) { @@ -69,7 +56,7 @@ func setupOperationsServiceWithCfg( return ctx, cfg, client } -func setupOperationsServiceWithoutRepo( +func setupOperationsService( tb testing.TB, ctx context.Context, options ...testserver.GitalyServerOpt, ) (context.Context, config.Cfg, gitalypb.OperationServiceClient) { cfg := testcfg.Build(tb) diff --git a/internal/gitaly/service/operations/user_create_branch_test.go b/internal/gitaly/service/operations/user_create_branch_test.go index 890a70119..6e58e4a48 100644 --- a/internal/gitaly/service/operations/user_create_branch_test.go +++ b/internal/gitaly/service/operations/user_create_branch_test.go @@ -27,7 +27,7 @@ func TestUserCreateBranch_successful(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) @@ -199,7 +199,7 @@ func TestUserCreateBranch_hook(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) startPoint := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTreeEntries( @@ -233,7 +233,7 @@ func TestUserCreateBranch_startPoint(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTreeEntries( @@ -303,7 +303,7 @@ func TestUserCreateBranch_hookFailure(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTreeEntries( @@ -344,7 +344,7 @@ func TestUserCreateBranch_failure(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTreeEntries( diff --git a/internal/gitaly/service/operations/user_delete_branch_test.go b/internal/gitaly/service/operations/user_delete_branch_test.go index 918cf855d..2269e4273 100644 --- a/internal/gitaly/service/operations/user_delete_branch_test.go +++ b/internal/gitaly/service/operations/user_delete_branch_test.go @@ -33,7 +33,7 @@ func TestUserDeleteBranch(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) type setupResponse struct { request *gitalypb.UserDeleteBranchRequest @@ -324,7 +324,7 @@ func TestUserDeleteBranch_allowed(t *testing.T) { }, } { t.Run(tc.desc, func(t *testing.T) { - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx, testserver.WithGitLabClient( + ctx, cfg, client := setupOperationsService(t, ctx, testserver.WithGitLabClient( gitlab.NewMockClient(t, tc.allowed, gitlab.MockPreReceive, gitlab.MockPostReceive), )) @@ -352,7 +352,7 @@ func TestUserDeleteBranch_concurrentUpdate(t *testing.T) { ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("concurrent-update")) @@ -395,7 +395,7 @@ func TestUserDeleteBranch_hooks(t *testing.T) { ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(git.DefaultBranch)) @@ -489,7 +489,7 @@ func TestUserDeleteBranch_invalidArgument(t *testing.T) { ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, _ := gittest.CreateRepository(t, ctx, cfg) testCases := []struct { @@ -542,7 +542,7 @@ func TestUserDeleteBranch_hookFailure(t *testing.T) { ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(git.DefaultBranch)) @@ -598,7 +598,7 @@ func TestBranchHookOutput(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(git.DefaultBranch)) diff --git a/internal/gitaly/service/operations/user_update_branch_test.go b/internal/gitaly/service/operations/user_update_branch_test.go index b873c85dc..056ea0204 100644 --- a/internal/gitaly/service/operations/user_update_branch_test.go +++ b/internal/gitaly/service/operations/user_update_branch_test.go @@ -16,7 +16,7 @@ func TestUserUpdateBranch(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) type setupData struct { request *gitalypb.UserUpdateBranchRequest @@ -222,7 +222,7 @@ func TestUserUpdateBranch_successfulGitHooks(t *testing.T) { ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) for _, hookName := range GitlabHooks { t.Run(hookName, func(t *testing.T) { @@ -255,7 +255,7 @@ func TestUserUpdateBranch_failingGitHooks(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) // Write a hook that will fail with the environment as the error message // so we can check that string for our env variables. @@ -293,7 +293,7 @@ func TestUserUpdateBranch_failures(t *testing.T) { t.Parallel() ctx := testhelper.Context(t) - ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + ctx, cfg, client := setupOperationsService(t, ctx) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) |