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

Introduce Locator abstraction to smarthttp service

See merge request gitlab-org/gitaly!2637

9 files changed, 21 insertions, 27 deletions

diff --git a/internal/gitaly/service/register.go b/internal/gitaly/service/register.go
index 70821d380..65a9a9906 100644
--- a/internal/gitaly/service/register.go
+++ b/internal/gitaly/service/register.go
@@ -81,6 +81,7 @@ func RegisterAll(grpcServer *grpc.Server, cfg config.Cfg, rubyServer *rubyserver
 		ssh.WithPackfileNegotiationMetrics(sshPackfileNegotiationMetrics),
 	))
 	gitalypb.RegisterSmartHTTPServiceServer(grpcServer, smarthttp.NewServer(
+		locator,
 		smarthttp.WithPackfileNegotiationMetrics(smarthttpPackfileNegotiationMetrics),
 	))
 	gitalypb.RegisterWikiServiceServer(grpcServer, wiki.NewServer(rubyServer, locator))
diff --git a/internal/gitaly/service/smarthttp/inforefs.go b/internal/gitaly/service/smarthttp/inforefs.go
index 6e12f4aaa..3dc6112bb 100644
--- a/internal/gitaly/service/smarthttp/inforefs.go
+++ b/internal/gitaly/service/smarthttp/inforefs.go
@@ -9,7 +9,6 @@ import (
 	log "github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/pktline"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/streamio"
 	"google.golang.org/grpc/codes"
@@ -27,7 +26,7 @@ func (s *server) InfoRefsUploadPack(in *gitalypb.InfoRefsRequest, stream gitalyp
 	})
 
 	return tryCache(stream.Context(), in, w, func(w io.Writer) error {
-		return handleInfoRefs(stream.Context(), uploadPackSvc, in, w)
+		return s.handleInfoRefs(stream.Context(), uploadPackSvc, in, w)
 	})
 }
 
@@ -35,17 +34,17 @@ func (s *server) InfoRefsReceivePack(in *gitalypb.InfoRefsRequest, stream gitaly
 	w := streamio.NewWriter(func(p []byte) error {
 		return stream.Send(&gitalypb.InfoRefsResponse{Data: p})
 	})
-	return handleInfoRefs(stream.Context(), receivePackSvc, in, w)
+	return s.handleInfoRefs(stream.Context(), receivePackSvc, in, w)
 }
 
-func handleInfoRefs(ctx context.Context, service string, req *gitalypb.InfoRefsRequest, w io.Writer) error {
+func (s *server) handleInfoRefs(ctx context.Context, service string, req *gitalypb.InfoRefsRequest, w io.Writer) error {
 	ctxlogrus.Extract(ctx).WithFields(log.Fields{
 		"service": service,
 	}).Debug("handleInfoRefs")
 
 	env := git.AddGitProtocolEnv(ctx, req, []string{})
 
-	repoPath, err := helper.GetRepoPath(req.Repository)
+	repoPath, err := s.locator.GetRepoPath(req.Repository)
 	if err != nil {
 		return err
 	}
diff --git a/internal/gitaly/service/smarthttp/inforefs_test.go b/internal/gitaly/service/smarthttp/inforefs_test.go
index a7c390dff..692f7f132 100644
--- a/internal/gitaly/service/smarthttp/inforefs_test.go
+++ b/internal/gitaly/service/smarthttp/inforefs_test.go
@@ -20,7 +20,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git/objectpool"
 	"gitlab.com/gitlab-org/gitaly/internal/git/stats"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/tempdir"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
@@ -358,7 +357,7 @@ func TestCacheInfoRefsUploadPack(t *testing.T) {
 			StorageName:  testRepo.StorageName,
 		},
 	} // invalid request because repo is empty
-	invalidRepoCleanup := createInvalidRepo(t, invalidReq.Repository)
+	invalidRepoCleanup := createInvalidRepo(t, filepath.Join(testhelper.GitlabTestStoragePath(), invalidReq.Repository.RelativePath))
 	defer invalidRepoCleanup()
 
 	_, err = makeInfoRefsUploadPackRequest(ctx, t, serverSocketPath, invalidReq)
@@ -381,9 +380,7 @@ func TestCacheInfoRefsUploadPack(t *testing.T) {
 	require.True(t, happened)
 }
 
-func createInvalidRepo(t testing.TB, repo *gitalypb.Repository) func() {
-	repoDir, err := helper.GetPath(repo)
-	require.NoError(t, err)
+func createInvalidRepo(t testing.TB, repoDir string) func() {
 	for _, subDir := range []string{"objects", "refs", "HEAD"} {
 		require.NoError(t, os.MkdirAll(filepath.Join(repoDir, subDir), 0755))
 	}
diff --git a/internal/gitaly/service/smarthttp/receive_pack.go b/internal/gitaly/service/smarthttp/receive_pack.go
index 2a5f38918..756e40bf3 100644
--- a/internal/gitaly/service/smarthttp/receive_pack.go
+++ b/internal/gitaly/service/smarthttp/receive_pack.go
@@ -44,7 +44,7 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac
 	}
 	env := append(hookEnv, "GL_PROTOCOL=http")
 
-	repoPath, err := helper.GetRepoPath(req.Repository)
+	repoPath, err := s.locator.GetRepoPath(req.Repository)
 	if err != nil {
 		return err
 	}
diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go
index 0f5793844..858d3bf38 100644
--- a/internal/gitaly/service/smarthttp/receive_pack_test.go
+++ b/internal/gitaly/service/smarthttp/receive_pack_test.go
@@ -446,7 +446,7 @@ func runSmartHTTPHookServiceServer(t *testing.T) (*grpc.Server, string) {
 		t.Fatal(err)
 	}
 
-	gitalypb.RegisterSmartHTTPServiceServer(server, NewServer())
+	gitalypb.RegisterSmartHTTPServiceServer(server, NewServer(config.NewLocator(config.Config)))
 	gitalypb.RegisterHookServiceServer(server, hook.NewServer(gitalyhook.NewManager(gitalyhook.GitlabAPIStub, config.Config)))
 	reflection.Register(server)
 
@@ -513,7 +513,7 @@ func TestPostReceiveWithTransactionsViaPraefect(t *testing.T) {
 			testhelper.WriteShellSecretFile(t, gitlabShellDir, secretToken)
 
 			gitalyServer := testhelper.NewServerWithAuth(t, nil, nil, config.Config.Auth.Token)
-			gitalypb.RegisterSmartHTTPServiceServer(gitalyServer.GrpcServer(), NewServer())
+			gitalypb.RegisterSmartHTTPServiceServer(gitalyServer.GrpcServer(), NewServer(config.NewLocator(config.Config)))
 			gitalypb.RegisterHookServiceServer(gitalyServer.GrpcServer(), hook.NewServer(gitalyhook.NewManager(gitalyhook.GitlabAPIStub, config.Config)))
 			reflection.Register(gitalyServer.GrpcServer())
 			require.NoError(t, gitalyServer.Start())
@@ -563,7 +563,7 @@ func TestPostReceiveWithReferenceTransactionHook(t *testing.T) {
 	refTransactionServer := &testTransactionServer{}
 
 	gitalyServer := testhelper.NewTestGrpcServer(t, nil, nil)
-	gitalypb.RegisterSmartHTTPServiceServer(gitalyServer, NewServer())
+	gitalypb.RegisterSmartHTTPServiceServer(gitalyServer, NewServer(config.NewLocator(config.Config)))
 	gitalypb.RegisterHookServiceServer(gitalyServer, hook.NewServer(gitalyhook.NewManager(gitalyhook.GitlabAPIStub, config.Config)))
 	gitalypb.RegisterRefTransactionServer(gitalyServer, refTransactionServer)
 	reflection.Register(gitalyServer)
diff --git a/internal/gitaly/service/smarthttp/server.go b/internal/gitaly/service/smarthttp/server.go
index 15a6bb969..68f96a238 100644
--- a/internal/gitaly/service/smarthttp/server.go
+++ b/internal/gitaly/service/smarthttp/server.go
@@ -2,16 +2,19 @@ package smarthttp
 
 import (
 	"github.com/prometheus/client_golang/prometheus"
+	"gitlab.com/gitlab-org/gitaly/internal/storage"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
 type server struct {
+	locator                    storage.Locator
 	packfileNegotiationMetrics *prometheus.CounterVec
 }
 
 // NewServer creates a new instance of a grpc SmartHTTPServer
-func NewServer(serverOpts ...ServerOpt) gitalypb.SmartHTTPServiceServer {
+func NewServer(locator storage.Locator, serverOpts ...ServerOpt) gitalypb.SmartHTTPServiceServer {
 	s := &server{
+		locator: locator,
 		packfileNegotiationMetrics: prometheus.NewCounterVec(
 			prometheus.CounterOpts{},
 			[]string{"git_negotiation_feature"},
diff --git a/internal/gitaly/service/smarthttp/testhelper_test.go b/internal/gitaly/service/smarthttp/testhelper_test.go
index 919926a4f..29551e301 100644
--- a/internal/gitaly/service/smarthttp/testhelper_test.go
+++ b/internal/gitaly/service/smarthttp/testhelper_test.go
@@ -55,7 +55,7 @@ func runSmartHTTPServer(t *testing.T, serverOpts ...ServerOpt) (string, func())
 		},
 	)
 
-	gitalypb.RegisterSmartHTTPServiceServer(srv.GrpcServer(), NewServer(serverOpts...))
+	gitalypb.RegisterSmartHTTPServiceServer(srv.GrpcServer(), NewServer(config.NewLocator(config.Config), serverOpts...))
 	reflection.Register(srv.GrpcServer())
 
 	require.NoError(t, srv.Start())
diff --git a/internal/gitaly/service/smarthttp/upload_pack.go b/internal/gitaly/service/smarthttp/upload_pack.go
index 6ccec7f16..d99544cf6 100644
--- a/internal/gitaly/service/smarthttp/upload_pack.go
+++ b/internal/gitaly/service/smarthttp/upload_pack.go
@@ -10,7 +10,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/stats"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/service/inspect"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/streamio"
 	"google.golang.org/grpc/codes"
@@ -68,7 +67,7 @@ func (s *server) PostUploadPack(stream gitalypb.SmartHTTPService_PostUploadPackS
 
 	env := git.AddGitProtocolEnv(ctx, req, command.GitEnv)
 
-	repoPath, err := helper.GetRepoPath(req.Repository)
+	repoPath, err := s.locator.GetRepoPath(req.Repository)
 	if err != nil {
 		return err
 	}
diff --git a/internal/gitaly/service/smarthttp/upload_pack_test.go b/internal/gitaly/service/smarthttp/upload_pack_test.go
index 235b25882..cdd2757fd 100644
--- a/internal/gitaly/service/smarthttp/upload_pack_test.go
+++ b/internal/gitaly/service/smarthttp/upload_pack_test.go
@@ -17,7 +17,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/pktline"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/streamio"
@@ -40,8 +39,7 @@ func TestSuccessfulUploadPackRequest(t *testing.T) {
 	defer cancel()
 
 	testRepo := testhelper.TestRepository()
-	testRepoPath, err := helper.GetRepoPath(testRepo)
-	require.NoError(t, err)
+	testRepoPath := filepath.Join(testhelper.GitlabTestStoragePath(), testRepo.RelativePath)
 
 	storagePath := testhelper.GitlabTestStoragePath()
 	remoteRepoRelativePath := "gitlab-test-remote"
@@ -110,8 +108,7 @@ func TestUploadPackRequestWithGitConfigOptions(t *testing.T) {
 	defer cancel()
 
 	testRepo := testhelper.TestRepository()
-	testRepoPath, err := helper.GetRepoPath(testRepo)
-	require.NoError(t, err)
+	testRepoPath := filepath.Join(testhelper.GitlabTestStoragePath(), testRepo.RelativePath)
 
 	storagePath := testhelper.GitlabTestStoragePath()
 	ourRepoRelativePath := "gitlab-test-remote"
@@ -177,8 +174,7 @@ func TestUploadPackRequestWithGitProtocol(t *testing.T) {
 	defer cancel()
 
 	testRepo := testhelper.TestRepository()
-	testRepoPath, err := helper.GetRepoPath(testRepo)
-	require.NoError(t, err)
+	testRepoPath := filepath.Join(testhelper.GitlabTestStoragePath(), testRepo.RelativePath)
 
 	storagePath := testhelper.GitlabTestStoragePath()
 	relativePath, err := filepath.Rel(storagePath, testRepoPath)
@@ -335,8 +331,7 @@ func TestUploadPackRequestForPartialCloneSuccess(t *testing.T) {
 	defer stop()
 
 	testRepo := testhelper.TestRepository()
-	testRepoPath, err := helper.GetRepoPath(testRepo)
-	require.NoError(t, err)
+	testRepoPath := filepath.Join(testhelper.GitlabTestStoragePath(), testRepo.RelativePath)
 
 	storagePath := testhelper.GitlabTestStoragePath()
 	remoteRepoRelativePath := "gitlab-test-remote"