Merge branch 'po-remote-repo-resolve-refish' into 'master'

Remote repository abstraction for resolving refish

See merge request gitlab-org/gitaly!2629

9 files changed, 317 insertions, 123 deletions

diff --git a/changelogs/unreleased/po-remote-repo-resolve-refish.yml b/changelogs/unreleased/po-remote-repo-resolve-refish.yml
new file mode 100644
index 000000000..003ec9ebd
--- /dev/null
+++ b/changelogs/unreleased/po-remote-repo-resolve-refish.yml
@@ -0,0 +1,5 @@
+---
+title: Remote repository abstraction for resolving refish
+merge_request: 2629
+author:
+type: other
diff --git a/internal/git/remote.go b/internal/git/remote.go
new file mode 100644
index 000000000..c97760c25
--- /dev/null
+++ b/internal/git/remote.go
@@ -0,0 +1,57 @@
+package git
+
+import (
+	"context"
+	"fmt"
+
+	"gitlab.com/gitlab-org/gitaly/client"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/internal/storage"
+	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
+)
+
+// remoteRepository represents a Git repository on a different Gitaly storage
+type remoteRepository struct {
+	UnimplementedRepo
+	repo   *gitalypb.Repository
+	server storage.ServerInfo
+	pool   *client.Pool
+}
+
+// NewRepository creates a new remote Repository from its protobuf representation.
+func NewRemoteRepository(ctx context.Context, repo *gitalypb.Repository, pool *client.Pool) (Repository, error) {
+	server, err := helper.ExtractGitalyServer(ctx, repo.GetStorageName())
+	if err != nil {
+		return nil, fmt.Errorf("remote repository: %w", err)
+	}
+
+	return remoteRepository{
+		repo:   repo,
+		server: server,
+		pool:   pool,
+	}, nil
+}
+
+// ResolveRefish will dial to the remote repository and attempt to resolve the
+// refish string via the gRPC interface
+func (rr remoteRepository) ResolveRefish(ctx context.Context, ref string, verify bool) (string, error) {
+	if verify {
+		return "", ErrUnimplemented
+	}
+
+	cc, err := rr.pool.Dial(ctx, rr.server.Address, rr.server.Token)
+	if err != nil {
+		return "", err
+	}
+
+	cli := gitalypb.NewCommitServiceClient(cc)
+	resp, err := cli.FindCommit(ctx, &gitalypb.FindCommitRequest{
+		Repository: rr.repo,
+		Revision:   []byte(ref),
+	})
+	if err != nil {
+		return "", err
+	}
+
+	return resp.GetCommit().GetId(), nil
+}
diff --git a/internal/git/repository.go b/internal/git/repository.go
index ab0dce686..51bdf20a7 100644
--- a/internal/git/repository.go
+++ b/internal/git/repository.go
@@ -35,7 +35,9 @@ type Repository interface {
 	// gitrevisions(1) for accepted syntax. This will not verify whether the
 	// object ID exists. To do so, you can peel the reference to a given
 	// object type, e.g. by passing `refs/heads/master^{commit}`.
-	ResolveRefish(ctx context.Context, ref string) (string, error)
+	// Verify indicates if you want to return an error when no valid SHA-1
+	// can be returned for a given refish.
+	ResolveRefish(ctx context.Context, ref string, verify bool) (string, error)
 
 	// ContainsRef checks if a ref in the repository exists. This will not
 	// verify whether the target object exists. To do so, you can peel the
@@ -74,6 +76,52 @@ type Repository interface {
 	CatFile(ctx context.Context, oid string) ([]byte, error)
 }
 
+// ErrUnimplemented indicates the repository abstraction does not yet implement
+// a specific behavior
+var ErrUnimplemented = errors.New("behavior not implemented yet")
+
+// UnimplementedRepo satisfies the Repository interface to reduce friction in
+// writing new Repository implementations
+type UnimplementedRepo struct{}
+
+func (UnimplementedRepo) ResolveRefish(ctx context.Context, ref string, verify bool) (string, error) {
+	return "", ErrUnimplemented
+}
+
+func (UnimplementedRepo) ContainsRef(ctx context.Context, ref string) (bool, error) {
+	return false, ErrUnimplemented
+}
+
+func (UnimplementedRepo) GetReference(ctx context.Context, ref string) (Reference, error) {
+	return Reference{}, ErrUnimplemented
+}
+
+func (UnimplementedRepo) GetReferences(ctx context.Context, pattern string) ([]Reference, error) {
+	return nil, ErrUnimplemented
+}
+
+func (UnimplementedRepo) GetBranch(ctx context.Context, branch string) (Reference, error) {
+	return Reference{}, ErrUnimplemented
+}
+
+func (UnimplementedRepo) GetBranches(ctx context.Context) ([]Reference, error) {
+	return nil, ErrUnimplemented
+}
+
+func (UnimplementedRepo) UpdateRef(ctx context.Context, reference, newrev, oldrev string) error {
+	return ErrUnimplemented
+}
+
+func (UnimplementedRepo) WriteBlob(context.Context, string, io.Reader) (string, error) {
+	return "", ErrUnimplemented
+}
+
+func (UnimplementedRepo) CatFile(context.Context, string) ([]byte, error) {
+	return nil, ErrUnimplemented
+}
+
+var _ Repository = UnimplementedRepo{} // compile time assertion
+
 // localRepository represents a local Git repository.
 type localRepository struct {
 	repo repository.GitRepo
@@ -150,14 +198,19 @@ func (repo *localRepository) CatFile(ctx context.Context, oid string) ([]byte, e
 	return stdout.Bytes(), nil
 }
 
-func (repo *localRepository) ResolveRefish(ctx context.Context, refish string) (string, error) {
+func (repo *localRepository) ResolveRefish(ctx context.Context, refish string, verify bool) (string, error) {
 	if refish == "" {
 		return "", errors.New("repository cannot contain empty reference name")
 	}
 
+	var flags []Option
+	if verify {
+		flags = append(flags, Flag{Name: "--verify"})
+	}
+
 	cmd, err := repo.command(ctx, nil, SubCmd{
 		Name:  "rev-parse",
-		Flags: []Option{Flag{Name: "--verify"}},
+		Flags: flags,
 		Args:  []string{refish},
 	})
 	if err != nil {
@@ -183,7 +236,7 @@ func (repo *localRepository) ResolveRefish(ctx context.Context, refish string) (
 }
 
 func (repo *localRepository) ContainsRef(ctx context.Context, ref string) (bool, error) {
-	if _, err := repo.ResolveRefish(ctx, ref); err != nil {
+	if _, err := repo.ResolveRefish(ctx, ref, true); err != nil {
 		if errors.Is(err, ErrReferenceNotFound) {
 			return false, nil
 		}
diff --git a/internal/git/repository_test.go b/internal/git/repository_test.go
index 3b6a2e3e0..b30cc7f08 100644
--- a/internal/git/repository_test.go
+++ b/internal/git/repository_test.go
@@ -1,7 +1,8 @@
-package git
+package git_test
 
 import (
 	"errors"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -11,7 +12,12 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/client"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper/testserver"
 )
 
 const (
@@ -19,12 +25,10 @@ const (
 	NonexistentID = "ba4f184e126b751d1bffad5897f263108befc780"
 )
 
-func TestLocalRepository_ResolveRefish(t *testing.T) {
+func TestRepository_ResolveRefish(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	repo := NewRepository(testhelper.TestRepository())
-
 	testcases := []struct {
 		desc     string
 		refish   string
@@ -60,18 +64,42 @@ func TestLocalRepository_ResolveRefish(t *testing.T) {
 		},
 	}
 
-	for _, tc := range testcases {
-		t.Run(tc.desc, func(t *testing.T) {
-			oid, err := repo.ResolveRefish(ctx, tc.refish)
+	_, serverSocketPath, cleanup := testserver.RunInternalGitalyServer(t, config.Config.Storages, config.Config.Auth.Token)
+	defer cleanup()
 
-			if tc.expected == "" {
-				require.Error(t, err)
-				require.Equal(t, err, ErrReferenceNotFound)
-				return
-			}
+	for _, repo := range []git.Repository{
+		git.NewRepository(testhelper.TestRepository()),
+		func() git.Repository {
+			ctx, err := helper.InjectGitalyServers(ctx, "default", serverSocketPath, config.Config.Auth.Token)
+			require.NoError(t, err)
 
+			r, err := git.NewRemoteRepository(
+				helper.OutgoingToIncoming(ctx),
+				testhelper.TestRepository(),
+				client.NewPool(),
+			)
 			require.NoError(t, err)
-			require.Equal(t, tc.expected, oid)
+			return r
+		}(),
+	} {
+		t.Run(fmt.Sprintf("%T", repo), func(t *testing.T) {
+			for _, tc := range testcases {
+				t.Run(tc.desc, func(t *testing.T) {
+					oid, err := repo.ResolveRefish(ctx, tc.refish, true)
+					if errors.Is(err, git.ErrUnimplemented) {
+						t.Skip()
+					}
+
+					if tc.expected == "" {
+						require.Error(t, err)
+						require.Equal(t, err, git.ErrReferenceNotFound)
+						return
+					}
+
+					require.NoError(t, err)
+					require.Equal(t, tc.expected, oid)
+				})
+			}
 		})
 	}
 }
@@ -80,7 +108,7 @@ func TestLocalRepository_ContainsRef(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	repo := NewRepository(testhelper.TestRepository())
+	repo := git.NewRepository(testhelper.TestRepository())
 
 	testcases := []struct {
 		desc      string
@@ -117,32 +145,32 @@ func TestLocalRepository_GetReference(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	repo := NewRepository(testhelper.TestRepository())
+	repo := git.NewRepository(testhelper.TestRepository())
 
 	testcases := []struct {
 		desc     string
 		ref      string
-		expected Reference
+		expected git.Reference
 	}{
 		{
 			desc:     "fully qualified master branch",
 			ref:      "refs/heads/master",
-			expected: NewReference("refs/heads/master", MasterID),
+			expected: git.NewReference("refs/heads/master", MasterID),
 		},
 		{
 			desc:     "unqualified master branch fails",
 			ref:      "master",
-			expected: Reference{},
+			expected: git.Reference{},
 		},
 		{
 			desc:     "nonexistent branch",
 			ref:      "refs/heads/nonexistent",
-			expected: Reference{},
+			expected: git.Reference{},
 		},
 		{
 			desc:     "nonexistent branch",
 			ref:      "nonexistent",
-			expected: Reference{},
+			expected: git.Reference{},
 		},
 	}
 
@@ -150,7 +178,7 @@ func TestLocalRepository_GetReference(t *testing.T) {
 		t.Run(tc.desc, func(t *testing.T) {
 			ref, err := repo.GetReference(ctx, tc.ref)
 			if tc.expected.Name == "" {
-				require.True(t, errors.Is(err, ErrReferenceNotFound))
+				require.True(t, errors.Is(err, git.ErrReferenceNotFound))
 			} else {
 				require.NoError(t, err)
 				require.Equal(t, tc.expected, ref)
@@ -163,32 +191,32 @@ func TestLocalRepository_GetBranch(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	repo := NewRepository(testhelper.TestRepository())
+	repo := git.NewRepository(testhelper.TestRepository())
 
 	testcases := []struct {
 		desc     string
 		ref      string
-		expected Reference
+		expected git.Reference
 	}{
 		{
 			desc:     "fully qualified master branch",
 			ref:      "refs/heads/master",
-			expected: NewReference("refs/heads/master", MasterID),
+			expected: git.NewReference("refs/heads/master", MasterID),
 		},
 		{
 			desc:     "half-qualified master branch",
 			ref:      "heads/master",
-			expected: NewReference("refs/heads/master", MasterID),
+			expected: git.NewReference("refs/heads/master", MasterID),
 		},
 		{
 			desc:     "fully qualified master branch",
 			ref:      "master",
-			expected: NewReference("refs/heads/master", MasterID),
+			expected: git.NewReference("refs/heads/master", MasterID),
 		},
 		{
 			desc:     "nonexistent branch",
 			ref:      "nonexistent",
-			expected: Reference{},
+			expected: git.Reference{},
 		},
 	}
 
@@ -196,7 +224,7 @@ func TestLocalRepository_GetBranch(t *testing.T) {
 		t.Run(tc.desc, func(t *testing.T) {
 			ref, err := repo.GetBranch(ctx, tc.ref)
 			if tc.expected.Name == "" {
-				require.True(t, errors.Is(err, ErrReferenceNotFound))
+				require.True(t, errors.Is(err, git.ErrReferenceNotFound))
 			} else {
 				require.NoError(t, err)
 				require.Equal(t, tc.expected, ref)
@@ -209,40 +237,40 @@ func TestLocalRepository_GetReferences(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	repo := NewRepository(testhelper.TestRepository())
+	repo := git.NewRepository(testhelper.TestRepository())
 
 	testcases := []struct {
 		desc    string
 		pattern string
-		match   func(t *testing.T, refs []Reference)
+		match   func(t *testing.T, refs []git.Reference)
 	}{
 		{
 			desc:    "master branch",
 			pattern: "refs/heads/master",
-			match: func(t *testing.T, refs []Reference) {
-				require.Equal(t, []Reference{
-					NewReference("refs/heads/master", MasterID),
+			match: func(t *testing.T, refs []git.Reference) {
+				require.Equal(t, []git.Reference{
+					git.NewReference("refs/heads/master", MasterID),
 				}, refs)
 			},
 		},
 		{
 			desc:    "all references",
 			pattern: "",
-			match: func(t *testing.T, refs []Reference) {
+			match: func(t *testing.T, refs []git.Reference) {
 				require.Len(t, refs, 94)
 			},
 		},
 		{
 			desc:    "branches",
 			pattern: "refs/heads/",
-			match: func(t *testing.T, refs []Reference) {
+			match: func(t *testing.T, refs []git.Reference) {
 				require.Len(t, refs, 91)
 			},
 		},
 		{
 			desc:    "branches",
 			pattern: "refs/heads/nonexistent",
-			match: func(t *testing.T, refs []Reference) {
+			match: func(t *testing.T, refs []git.Reference) {
 				require.Empty(t, refs)
 			},
 		},
@@ -275,7 +303,7 @@ crlf binary
 lf   text
 	`), os.ModePerm))
 
-	repo := NewRepository(pbRepo)
+	repo := git.NewRepository(pbRepo)
 
 	for _, tc := range []struct {
 		desc    string
@@ -288,7 +316,7 @@ lf   text
 		{
 			desc:  "error reading",
 			input: ReaderFunc(func([]byte) (int, error) { return 0, assert.AnError }),
-			error: errorWithStderr(assert.AnError, nil),
+			error: fmt.Errorf("%w, stderr: %q", assert.AnError, []byte{}),
 		},
 		{
 			desc:    "successful empty blob",
@@ -336,7 +364,7 @@ func TestLocalRepository_CatFile(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	repo := NewRepository(testhelper.TestRepository())
+	repo := git.NewRepository(testhelper.TestRepository())
 
 	for _, tc := range []struct {
 		desc    string
@@ -346,8 +374,8 @@ func TestLocalRepository_CatFile(t *testing.T) {
 	}{
 		{
 			desc:  "invalid object",
-			oid:   NullSHA,
-			error: InvalidObjectError(NullSHA),
+			oid:   git.NullSHA,
+			error: git.InvalidObjectError(git.NullSHA),
 		},
 		{
 			desc: "valid object",
@@ -368,7 +396,7 @@ func TestLocalRepository_GetBranches(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	repo := NewRepository(testhelper.TestRepository())
+	repo := git.NewRepository(testhelper.TestRepository())
 
 	refs, err := repo.GetBranches(ctx)
 	require.NoError(t, err)
@@ -382,7 +410,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) {
 	testRepo, _, cleanup := testhelper.NewTestRepo(t)
 	defer cleanup()
 
-	otherRef, err := NewRepository(testRepo).GetReference(ctx, "refs/heads/gitaly-test-ref")
+	otherRef, err := git.NewRepository(testRepo).GetReference(ctx, "refs/heads/gitaly-test-ref")
 	require.NoError(t, err)
 
 	testcases := []struct {
@@ -390,14 +418,14 @@ func TestLocalRepository_UpdateRef(t *testing.T) {
 		ref    string
 		newrev string
 		oldrev string
-		verify func(t *testing.T, repo Repository, err error)
+		verify func(t *testing.T, repo git.Repository, err error)
 	}{
 		{
 			desc:   "successfully update master",
 			ref:    "refs/heads/master",
 			newrev: otherRef.Target,
 			oldrev: MasterID,
-			verify: func(t *testing.T, repo Repository, err error) {
+			verify: func(t *testing.T, repo git.Repository, err error) {
 				require.NoError(t, err)
 				ref, err := repo.GetReference(ctx, "refs/heads/master")
 				require.NoError(t, err)
@@ -409,7 +437,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) {
 			ref:    "refs/heads/master",
 			newrev: otherRef.Target,
 			oldrev: NonexistentID,
-			verify: func(t *testing.T, repo Repository, err error) {
+			verify: func(t *testing.T, repo git.Repository, err error) {
 				require.Error(t, err)
 				ref, err := repo.GetReference(ctx, "refs/heads/master")
 				require.NoError(t, err)
@@ -421,7 +449,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) {
 			ref:    "refs/heads/master",
 			newrev: NonexistentID,
 			oldrev: MasterID,
-			verify: func(t *testing.T, repo Repository, err error) {
+			verify: func(t *testing.T, repo git.Repository, err error) {
 				require.Error(t, err)
 				ref, err := repo.GetReference(ctx, "refs/heads/master")
 				require.NoError(t, err)
@@ -433,7 +461,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) {
 			ref:    "refs/heads/master",
 			newrev: otherRef.Target,
 			oldrev: "",
-			verify: func(t *testing.T, repo Repository, err error) {
+			verify: func(t *testing.T, repo git.Repository, err error) {
 				require.NoError(t, err)
 				ref, err := repo.GetReference(ctx, "refs/heads/master")
 				require.NoError(t, err)
@@ -445,7 +473,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) {
 			ref:    "master",
 			newrev: otherRef.Target,
 			oldrev: MasterID,
-			verify: func(t *testing.T, repo Repository, err error) {
+			verify: func(t *testing.T, repo git.Repository, err error) {
 				require.Error(t, err)
 				ref, err := repo.GetReference(ctx, "refs/heads/master")
 				require.NoError(t, err)
@@ -457,7 +485,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) {
 			ref:    "refs/heads/master",
 			newrev: strings.Repeat("0", 40),
 			oldrev: MasterID,
-			verify: func(t *testing.T, repo Repository, err error) {
+			verify: func(t *testing.T, repo git.Repository, err error) {
 				require.NoError(t, err)
 				_, err = repo.GetReference(ctx, "refs/heads/master")
 				require.Error(t, err)
@@ -468,7 +496,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) {
 			ref:    "refs/heads/new",
 			newrev: MasterID,
 			oldrev: strings.Repeat("0", 40),
-			verify: func(t *testing.T, repo Repository, err error) {
+			verify: func(t *testing.T, repo git.Repository, err error) {
 				require.NoError(t, err)
 				ref, err := repo.GetReference(ctx, "refs/heads/new")
 				require.NoError(t, err)
@@ -483,7 +511,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) {
 			testRepo, _, cleanup := testhelper.NewTestRepo(t)
 			defer cleanup()
 
-			repo := NewRepository(testRepo)
+			repo := git.NewRepository(testRepo)
 			err := repo.UpdateRef(ctx, tc.ref, tc.newrev, tc.oldrev)
 
 			tc.verify(t, repo, err)
diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go
index 466cbf5e0..e666f94bb 100644
--- a/internal/gitaly/service/operations/merge.go
+++ b/internal/gitaly/service/operations/merge.go
@@ -393,13 +393,13 @@ func (s *server) userMergeToRef(ctx context.Context, request *gitalypb.UserMerge
 		refName = string(request.FirstParentRef)
 	}
 
-	ref, err := repo.ResolveRefish(ctx, refName)
+	ref, err := repo.ResolveRefish(ctx, refName, true)
 	if err != nil {
 		//nolint:stylecheck
 		return nil, helper.ErrInvalidArgument(errors.New("Invalid merge source"))
 	}
 
-	sourceRef, err := repo.ResolveRefish(ctx, request.SourceSha)
+	sourceRef, err := repo.ResolveRefish(ctx, request.SourceSha, true)
 	if err != nil {
 		//nolint:stylecheck
 		return nil, helper.ErrInvalidArgument(errors.New("Invalid merge source"))
diff --git a/internal/helper/storage.go b/internal/helper/storage.go
index 254e51fa6..28d4d19c7 100644
--- a/internal/helper/storage.go
+++ b/internal/helper/storage.go
@@ -60,6 +60,16 @@ func IncomingToOutgoing(ctx context.Context) context.Context {
 	return metadata.NewOutgoingContext(ctx, md)
 }
 
+// OutgoingToIncoming creates an incoming context out of an outgoing context with the same storage metadata
+func OutgoingToIncoming(ctx context.Context) context.Context {
+	md, ok := metadata.FromOutgoingContext(ctx)
+	if !ok {
+		return ctx
+	}
+
+	return metadata.NewIncomingContext(ctx, md)
+}
+
 // InjectGitalyServers injects gitaly-servers metadata into an outgoing context
 func InjectGitalyServers(ctx context.Context, name, address, token string) (context.Context, error) {
 	gitalyServers := storage.GitalyServers{
diff --git a/internal/helper/storage_test.go b/internal/helper/storage_test.go
index 44e49b218..6c4a70ba2 100644
--- a/internal/helper/storage_test.go
+++ b/internal/helper/storage_test.go
@@ -97,3 +97,18 @@ func TestInjectGitalyServers(t *testing.T) {
 		require.Equal(t, []string{"bar"}, md["foo"])
 	})
 }
+
+func TestOutgoingToIncoming(t *testing.T) {
+	ctx := context.Background()
+	ctx, err := InjectGitalyServers(ctx, "a", "b", "c")
+	require.NoError(t, err)
+
+	_, err = ExtractGitalyServer(ctx, "a")
+	require.Error(t, err, "server should not be found in the incoming context")
+
+	ctx = OutgoingToIncoming(ctx)
+
+	info, err := ExtractGitalyServer(ctx, "a")
+	require.NoError(t, err)
+	require.Equal(t, storage.ServerInfo{Address: "b", Token: "c"}, info)
+}
diff --git a/internal/praefect/helper_test.go b/internal/praefect/helper_test.go
index 499df18d8..960185cf4 100644
--- a/internal/praefect/helper_test.go
+++ b/internal/praefect/helper_test.go
@@ -14,9 +14,6 @@ import (
 	gconfig "gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	internalauth "gitlab.com/gitlab-org/gitaly/internal/gitaly/config/auth"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/server/auth"
-	"gitlab.com/gitlab-org/gitaly/internal/gitaly/service/internalgitaly"
-	"gitlab.com/gitlab-org/gitaly/internal/gitaly/service/repository"
-	gitalyserver "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/server"
 	"gitlab.com/gitlab-org/gitaly/internal/log"
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/config"
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/datastore"
@@ -27,7 +24,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/transactions"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper/promtest"
-	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper/testserver"
 	correlation "gitlab.com/gitlab-org/labkit/correlation/grpc"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/health"
@@ -144,7 +141,7 @@ func flattenVirtualStoragesToStoragePath(virtualStorages []*config.VirtualStorag
 func withRealGitalyShared(t testing.TB) func([]*config.VirtualStorage) []testhelper.Cleanup {
 	return func(virtualStorages []*config.VirtualStorage) []testhelper.Cleanup {
 		gStorages := flattenVirtualStoragesToStoragePath(virtualStorages, testhelper.GitlabTestStoragePath())
-		_, backendAddr, cleanupGitaly := runInternalGitalyServer(t, gStorages, virtualStorages[0].Nodes[0].Token)
+		_, backendAddr, cleanupGitaly := testserver.RunInternalGitalyServer(t, gStorages, virtualStorages[0].Nodes[0].Token)
 
 		for _, vs := range virtualStorages {
 			for i, node := range vs.Nodes {
@@ -263,66 +260,6 @@ func runPraefectServer(t testing.TB, conf config.Config, opt buildOptions) (*grp
 	return cc, prf, cleanup
 }
 
-// partialGitaly is a subset of Gitaly's behavior needed to properly test
-// Praefect
-type partialGitaly interface {
-	gitalypb.ServerServiceServer
-	gitalypb.RepositoryServiceServer
-	gitalypb.InternalGitalyServer
-	healthpb.HealthServer
-}
-
-func registerGitalyServices(server *grpc.Server, pg partialGitaly) {
-	gitalypb.RegisterServerServiceServer(server, pg)
-	gitalypb.RegisterRepositoryServiceServer(server, pg)
-	gitalypb.RegisterInternalGitalyServer(server, pg)
-	healthpb.RegisterHealthServer(server, pg)
-}
-
-func realGitaly(storages []gconfig.Storage, authToken, internalSocketPath string) partialGitaly {
-	return struct {
-		gitalypb.ServerServiceServer
-		gitalypb.RepositoryServiceServer
-		gitalypb.InternalGitalyServer
-		healthpb.HealthServer
-	}{
-		gitalyserver.NewServer(storages),
-		repository.NewServer(RubyServer, gconfig.NewLocator(gconfig.Config), internalSocketPath),
-		internalgitaly.NewServer(gconfig.Config.Storages),
-		health.NewServer(),
-	}
-}
-
-func runInternalGitalyServer(t testing.TB, storages []gconfig.Storage, token string) (*grpc.Server, string, func()) {
-	streamInt := []grpc.StreamServerInterceptor{auth.StreamServerInterceptor(internalauth.Config{Token: token})}
-	unaryInt := []grpc.UnaryServerInterceptor{auth.UnaryServerInterceptor(internalauth.Config{Token: token})}
-
-	server := testhelper.NewTestGrpcServer(t, streamInt, unaryInt)
-	serverSocketPath := testhelper.GetTemporaryGitalySocketFileName()
-
-	listener, err := net.Listen("unix", serverSocketPath)
-	require.NoError(t, err)
-
-	internalSocketPath := gconfig.GitalyInternalSocketPath()
-	internalListener, err := net.Listen("unix", internalSocketPath)
-	require.NoError(t, err)
-
-	registerGitalyServices(server, realGitaly(storages, token, internalSocketPath))
-
-	errQ := make(chan error)
-
-	go func() { errQ <- server.Serve(listener) }()
-	go func() { errQ <- server.Serve(internalListener) }()
-
-	cleanup := func() {
-		server.Stop()
-		require.NoError(t, <-errQ)
-		require.NoError(t, <-errQ)
-	}
-
-	return server, "unix://" + serverSocketPath, cleanup
-}
-
 func mustLoadProtoReg(t testing.TB) *descriptor.FileDescriptorProto {
 	gz, _ := (*mock.SimpleRequest)(nil).Descriptor()
 	fd, err := protoregistry.ExtractFileDescriptor(gz)
diff --git a/internal/testhelper/testserver/gitaly.go b/internal/testhelper/testserver/gitaly.go
new file mode 100644
index 000000000..be38eba67
--- /dev/null
+++ b/internal/testhelper/testserver/gitaly.go
@@ -0,0 +1,89 @@
+package testserver
+
+import (
+	"net"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
+	internalauth "gitlab.com/gitlab-org/gitaly/internal/gitaly/config/auth"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/server/auth"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/service/commit"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/service/internalgitaly"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/service/repository"
+	gitalyserver "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/server"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/health"
+	healthpb "google.golang.org/grpc/health/grpc_health_v1"
+)
+
+var RubyServer = &rubyserver.Server{}
+
+// PartialGitaly is a subset of Gitaly's behavior
+type PartialGitaly interface {
+	gitalypb.ServerServiceServer
+	gitalypb.RepositoryServiceServer
+	gitalypb.InternalGitalyServer
+	gitalypb.CommitServiceServer
+	healthpb.HealthServer
+}
+
+func registerGitalyServices(server *grpc.Server, pg PartialGitaly) {
+	gitalypb.RegisterServerServiceServer(server, pg)
+	gitalypb.RegisterRepositoryServiceServer(server, pg)
+	gitalypb.RegisterInternalGitalyServer(server, pg)
+	gitalypb.RegisterCommitServiceServer(server, pg)
+	healthpb.RegisterHealthServer(server, pg)
+}
+
+// RealGitaly instantiates an instance of PartialGitaly that uses the real world
+// services. This is intended to be used in integration tests to validate
+// Gitaly services.
+func RealGitaly(storages []config.Storage, authToken, internalSocketPath string) PartialGitaly {
+	return struct {
+		gitalypb.ServerServiceServer
+		gitalypb.RepositoryServiceServer
+		gitalypb.InternalGitalyServer
+		gitalypb.CommitServiceServer
+		healthpb.HealthServer
+	}{
+		gitalyserver.NewServer(storages),
+		repository.NewServer(RubyServer, config.NewLocator(config.Config), internalSocketPath),
+		internalgitaly.NewServer(config.Config.Storages),
+		commit.NewServer(config.NewLocator(config.Config)),
+		health.NewServer(),
+	}
+}
+
+func RunInternalGitalyServer(t testing.TB, storages []config.Storage, token string) (*grpc.Server, string, func()) {
+	streamInt := []grpc.StreamServerInterceptor{auth.StreamServerInterceptor(internalauth.Config{Token: token})}
+	unaryInt := []grpc.UnaryServerInterceptor{auth.UnaryServerInterceptor(internalauth.Config{Token: token})}
+
+	server := testhelper.NewTestGrpcServer(t, streamInt, unaryInt)
+	serverSocketPath := testhelper.GetTemporaryGitalySocketFileName()
+
+	listener, err := net.Listen("unix", serverSocketPath)
+	require.NoError(t, err)
+
+	internalSocketPath := config.GitalyInternalSocketPath()
+	internalListener, err := net.Listen("unix", internalSocketPath)
+	require.NoError(t, err)
+
+	registerGitalyServices(server, RealGitaly(storages, token, internalSocketPath))
+
+	errQ := make(chan error)
+
+	go func() { errQ <- server.Serve(listener) }()
+	go func() { errQ <- server.Serve(internalListener) }()
+
+	cleanup := func() {
+		server.Stop()
+		require.NoError(t, <-errQ)
+		require.NoError(t, <-errQ)
+	}
+
+	return server, "unix://" + serverSocketPath, cleanup
+}