Merge branch 'ps-remote-uses-locator' into 'master'

Introduce Locator abstraction to remote service

See merge request gitlab-org/gitaly!2608

11 files changed, 31 insertions, 22 deletions

diff --git a/internal/gitaly/service/operations/testhelper_test.go b/internal/gitaly/service/operations/testhelper_test.go
index 98f124d8b..036205964 100644
--- a/internal/gitaly/service/operations/testhelper_test.go
+++ b/internal/gitaly/service/operations/testhelper_test.go
@@ -95,7 +95,7 @@ func runOperationServiceServerWithRubyServer(t *testing.T, ruby *rubyserver.Serv
 	gitalypb.RegisterOperationServiceServer(srv.GrpcServer(), server)
 	gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hook.NewServer(hookManager))
 	gitalypb.RegisterRepositoryServiceServer(srv.GrpcServer(), repository.NewServer(ruby, locator, internalSocket))
-	gitalypb.RegisterRefServiceServer(srv.GrpcServer(), ref.NewServer())
+	gitalypb.RegisterRefServiceServer(srv.GrpcServer(), ref.NewServer(locator))
 	gitalypb.RegisterCommitServiceServer(srv.GrpcServer(), commit.NewServer(locator))
 	gitalypb.RegisterSSHServiceServer(srv.GrpcServer(), ssh.NewServer())
 	reflection.Register(srv.GrpcServer())
diff --git a/internal/gitaly/service/ref/refs.go b/internal/gitaly/service/ref/refs.go
index 6db83b80e..97ddd5796 100644
--- a/internal/gitaly/service/ref/refs.go
+++ b/internal/gitaly/service/ref/refs.go
@@ -131,7 +131,7 @@ func parseAndReturnTags(ctx context.Context, repo *gitalypb.Repository, stream g
 func (s *server) FindAllTags(in *gitalypb.FindAllTagsRequest, stream gitalypb.RefService_FindAllTagsServer) error {
 	ctx := stream.Context()
 
-	if err := validateFindAllTagsRequest(in); err != nil {
+	if err := s.validateFindAllTagsRequest(in); err != nil {
 		return helper.ErrInvalidArgument(err)
 	}
 
@@ -141,12 +141,12 @@ func (s *server) FindAllTags(in *gitalypb.FindAllTagsRequest, stream gitalypb.Re
 	return nil
 }
 
-func validateFindAllTagsRequest(request *gitalypb.FindAllTagsRequest) error {
+func (s *server) validateFindAllTagsRequest(request *gitalypb.FindAllTagsRequest) error {
 	if request.GetRepository() == nil {
 		return errors.New("empty Repository")
 	}
 
-	if _, err := helper.GetRepoPath(request.GetRepository()); err != nil {
+	if _, err := s.locator.GetRepoPath(request.GetRepository()); err != nil {
 		return fmt.Errorf("invalid git directory: %v", err)
 	}
 
@@ -364,7 +364,7 @@ func findAllBranches(in *gitalypb.FindAllBranchesRequest, stream gitalypb.RefSer
 
 func (s *server) FindTag(ctx context.Context, in *gitalypb.FindTagRequest) (*gitalypb.FindTagResponse, error) {
 	var err error
-	if err = validateFindTagRequest(in); err != nil {
+	if err = s.validateFindTagRequest(in); err != nil {
 		return nil, helper.ErrInvalidArgument(err)
 	}
 
@@ -447,12 +447,12 @@ func findTag(ctx context.Context, repository *gitalypb.Repository, tagName []byt
 	return tag, nil
 }
 
-func validateFindTagRequest(in *gitalypb.FindTagRequest) error {
+func (s *server) validateFindTagRequest(in *gitalypb.FindTagRequest) error {
 	if in.GetRepository() == nil {
 		return errors.New("repository is empty")
 	}
 
-	if _, err := helper.GetRepoPath(in.GetRepository()); err != nil {
+	if _, err := s.locator.GetRepoPath(in.GetRepository()); err != nil {
 		return fmt.Errorf("invalid git directory: %v", err)
 	}
 
diff --git a/internal/gitaly/service/ref/server.go b/internal/gitaly/service/ref/server.go
index e68f418a5..69370613b 100644
--- a/internal/gitaly/service/ref/server.go
+++ b/internal/gitaly/service/ref/server.go
@@ -1,13 +1,15 @@
 package ref
 
 import (
+	"gitlab.com/gitlab-org/gitaly/internal/storage"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
 type server struct {
+	locator storage.Locator
 }
 
 // NewServer creates a new instance of a grpc RefServer
-func NewServer() gitalypb.RefServiceServer {
-	return &server{}
+func NewServer(locator storage.Locator) gitalypb.RefServiceServer {
+	return &server{locator: locator}
 }
diff --git a/internal/gitaly/service/ref/testhelper_test.go b/internal/gitaly/service/ref/testhelper_test.go
index f40730ede..26a82661a 100644
--- a/internal/gitaly/service/ref/testhelper_test.go
+++ b/internal/gitaly/service/ref/testhelper_test.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/golang/protobuf/ptypes/timestamp"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/lines"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
@@ -95,7 +96,7 @@ func testMain(m *testing.M) int {
 func runRefServiceServer(t *testing.T) (func(), string) {
 	srv := testhelper.NewServer(t, nil, nil)
 
-	gitalypb.RegisterRefServiceServer(srv.GrpcServer(), &server{})
+	gitalypb.RegisterRefServiceServer(srv.GrpcServer(), NewServer(config.NewLocator(config.Config)))
 	reflection.Register(srv.GrpcServer())
 
 	require.NoError(t, srv.Start())
diff --git a/internal/gitaly/service/register.go b/internal/gitaly/service/register.go
index c579c0f22..736067c86 100644
--- a/internal/gitaly/service/register.go
+++ b/internal/gitaly/service/register.go
@@ -74,7 +74,7 @@ func RegisterAll(grpcServer *grpc.Server, cfg config.Cfg, rubyServer *rubyserver
 	gitalypb.RegisterDiffServiceServer(grpcServer, diff.NewServer(locator))
 	gitalypb.RegisterNamespaceServiceServer(grpcServer, namespace.NewServer())
 	gitalypb.RegisterOperationServiceServer(grpcServer, operations.NewServer(cfg, rubyServer, hookManager, locator))
-	gitalypb.RegisterRefServiceServer(grpcServer, ref.NewServer())
+	gitalypb.RegisterRefServiceServer(grpcServer, ref.NewServer(locator))
 	gitalypb.RegisterRepositoryServiceServer(grpcServer, repository.NewServer(rubyServer, locator, config.GitalyInternalSocketPath()))
 	gitalypb.RegisterSSHServiceServer(grpcServer, ssh.NewServer(
 		ssh.WithPackfileNegotiationMetrics(sshPackfileNegotiationMetrics),
@@ -84,7 +84,7 @@ func RegisterAll(grpcServer *grpc.Server, cfg config.Cfg, rubyServer *rubyserver
 	))
 	gitalypb.RegisterWikiServiceServer(grpcServer, wiki.NewServer(rubyServer))
 	gitalypb.RegisterConflictsServiceServer(grpcServer, conflicts.NewServer(rubyServer))
-	gitalypb.RegisterRemoteServiceServer(grpcServer, remote.NewServer(rubyServer))
+	gitalypb.RegisterRemoteServiceServer(grpcServer, remote.NewServer(rubyServer, locator))
 	gitalypb.RegisterServerServiceServer(grpcServer, server.NewServer(cfg.Storages))
 	gitalypb.RegisterObjectPoolServiceServer(grpcServer, objectpool.NewServer(locator))
 	gitalypb.RegisterHookServiceServer(grpcServer, hook.NewServer(hookManager))
diff --git a/internal/gitaly/service/remote/fetch_internal_remote.go b/internal/gitaly/service/remote/fetch_internal_remote.go
index 35851cc94..311fb1da5 100644
--- a/internal/gitaly/service/remote/fetch_internal_remote.go
+++ b/internal/gitaly/service/remote/fetch_internal_remote.go
@@ -32,7 +32,7 @@ func (s *server) FetchInternalRemote(ctx context.Context, req *gitalypb.FetchInt
 		return nil, err
 	}
 
-	repoPath, err := helper.GetRepoPath(req.Repository)
+	repoPath, err := s.locator.GetRepoPath(req.Repository)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/gitaly/service/remote/fetch_internal_remote_test.go b/internal/gitaly/service/remote/fetch_internal_remote_test.go
index b4fe6bff8..9121fd549 100644
--- a/internal/gitaly/service/remote/fetch_internal_remote_test.go
+++ b/internal/gitaly/service/remote/fetch_internal_remote_test.go
@@ -41,7 +41,7 @@ func TestSuccessfulFetchInternalRemote(t *testing.T) {
 
 	gitaly0Server := testhelper.NewServer(t, nil, nil, testhelper.WithStorages([]string{"gitaly-0"}))
 	gitalypb.RegisterSSHServiceServer(gitaly0Server.GrpcServer(), ssh.NewServer())
-	gitalypb.RegisterRefServiceServer(gitaly0Server.GrpcServer(), ref.NewServer())
+	gitalypb.RegisterRefServiceServer(gitaly0Server.GrpcServer(), ref.NewServer(config.NewLocator(config.Config)))
 	reflection.Register(gitaly0Server.GrpcServer())
 	require.NoError(t, gitaly0Server.Start())
 	defer gitaly0Server.Stop()
diff --git a/internal/gitaly/service/remote/server.go b/internal/gitaly/service/remote/server.go
index 08057a8b1..ec090412d 100644
--- a/internal/gitaly/service/remote/server.go
+++ b/internal/gitaly/service/remote/server.go
@@ -3,18 +3,22 @@ package remote
 import (
 	"gitlab.com/gitlab-org/gitaly/client"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
+	"gitlab.com/gitlab-org/gitaly/internal/storage"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
 type server struct {
-	ruby  *rubyserver.Server
+	ruby    *rubyserver.Server
+	locator storage.Locator
+
 	conns *client.Pool
 }
 
 // NewServer creates a new instance of a grpc RemoteServiceServer
-func NewServer(rs *rubyserver.Server) gitalypb.RemoteServiceServer {
+func NewServer(rs *rubyserver.Server, locator storage.Locator) gitalypb.RemoteServiceServer {
 	return &server{
-		ruby:  rs,
-		conns: client.NewPool(),
+		ruby:    rs,
+		locator: locator,
+		conns:   client.NewPool(),
 	}
 }
diff --git a/internal/gitaly/service/remote/testhelper_test.go b/internal/gitaly/service/remote/testhelper_test.go
index c6a46e748..aec62d020 100644
--- a/internal/gitaly/service/remote/testhelper_test.go
+++ b/internal/gitaly/service/remote/testhelper_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
@@ -36,7 +37,7 @@ func testMain(m *testing.M) int {
 func RunRemoteServiceServer(t *testing.T, opts ...testhelper.TestServerOpt) (string, func()) {
 	srv := testhelper.NewServer(t, nil, nil, opts...)
 
-	gitalypb.RegisterRemoteServiceServer(srv.GrpcServer(), NewServer(RubyServer))
+	gitalypb.RegisterRemoteServiceServer(srv.GrpcServer(), NewServer(RubyServer, config.NewLocator(config.Config)))
 	reflection.Register(srv.GrpcServer())
 
 	require.NoError(t, srv.Start())
diff --git a/internal/middleware/commandstatshandler/commandstatshandler_test.go b/internal/middleware/commandstatshandler/commandstatshandler_test.go
index 262f06216..c02103323 100644
--- a/internal/middleware/commandstatshandler/commandstatshandler_test.go
+++ b/internal/middleware/commandstatshandler/commandstatshandler_test.go
@@ -11,6 +11,7 @@ import (
 	grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/service/ref"
 	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
@@ -36,7 +37,7 @@ func createNewServer(t *testing.T) *grpc.Server {
 
 	server := grpc.NewServer(opts...)
 
-	gitalypb.RegisterRefServiceServer(server, ref.NewServer())
+	gitalypb.RegisterRefServiceServer(server, ref.NewServer(config.NewLocator(config.Config)))
 
 	return server
 }
diff --git a/internal/praefect/replicator_test.go b/internal/praefect/replicator_test.go
index 1602489a1..994c51f69 100644
--- a/internal/praefect/replicator_test.go
+++ b/internal/praefect/replicator_test.go
@@ -978,9 +978,9 @@ func newReplicationService(tb testing.TB) (*grpc.Server, string) {
 	locator := gitaly_config.NewLocator(gitaly_config.Config)
 	gitalypb.RegisterRepositoryServiceServer(svr, repository.NewServer(RubyServer, locator, internalSocketName))
 	gitalypb.RegisterObjectPoolServiceServer(svr, objectpoolservice.NewServer(locator))
-	gitalypb.RegisterRemoteServiceServer(svr, remote.NewServer(RubyServer))
+	gitalypb.RegisterRemoteServiceServer(svr, remote.NewServer(RubyServer, locator))
 	gitalypb.RegisterSSHServiceServer(svr, ssh.NewServer())
-	gitalypb.RegisterRefServiceServer(svr, ref.NewServer())
+	gitalypb.RegisterRefServiceServer(svr, ref.NewServer(locator))
 	healthpb.RegisterHealthServer(svr, health.NewServer())
 	reflection.Register(svr)