Merge branch 'ps-upload-pack-env' into 'master'

Reduce dependency of the upload-pack on config.Config

See merge request gitlab-org/gitaly!2966

11 files changed, 19 insertions, 20 deletions

diff --git a/internal/gitaly/service/conflicts/resolve_conflicts.go b/internal/gitaly/service/conflicts/resolve_conflicts.go
index 0d7d495a7..024e2df1a 100644
--- a/internal/gitaly/service/conflicts/resolve_conflicts.go
+++ b/internal/gitaly/service/conflicts/resolve_conflicts.go
@@ -279,7 +279,7 @@ func (s *server) repoWithBranchCommit(ctx context.Context, srcRepo, targetRepo *
 		return nil
 	}
 
-	env, err := gitalyssh.UploadPackEnv(ctx, &gitalypb.SSHUploadPackRequest{Repository: targetRepo})
+	env, err := gitalyssh.UploadPackEnv(ctx, s.cfg, &gitalypb.SSHUploadPackRequest{Repository: targetRepo})
 	if err != nil {
 		return err
 	}
diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go
index ee9922272..df2f90024 100644
--- a/internal/gitaly/service/operations/commit_files.go
+++ b/internal/gitaly/service/operations/commit_files.go
@@ -429,7 +429,7 @@ func (s *Server) fetchMissingCommit(ctx context.Context, local, remote *gitalypb
 }
 
 func (s *Server) fetchRemoteObject(ctx context.Context, local, remote *gitalypb.Repository, sha string) error {
-	env, err := gitalyssh.UploadPackEnv(ctx, &gitalypb.SSHUploadPackRequest{
+	env, err := gitalyssh.UploadPackEnv(ctx, s.cfg, &gitalypb.SSHUploadPackRequest{
 		Repository:       remote,
 		GitConfigOptions: []string{"uploadpack.allowAnySHA1InWant=true"},
 	})
diff --git a/internal/gitaly/service/register.go b/internal/gitaly/service/register.go
index f7365d3fe..83f24ab49 100644
--- a/internal/gitaly/service/register.go
+++ b/internal/gitaly/service/register.go
@@ -87,7 +87,7 @@ func RegisterAll(grpcServer *grpc.Server, cfg config.Cfg, rubyServer *rubyserver
 	))
 	gitalypb.RegisterWikiServiceServer(grpcServer, wiki.NewServer(rubyServer, locator))
 	gitalypb.RegisterConflictsServiceServer(grpcServer, conflicts.NewServer(rubyServer, cfg, locator))
-	gitalypb.RegisterRemoteServiceServer(grpcServer, remote.NewServer(rubyServer, locator))
+	gitalypb.RegisterRemoteServiceServer(grpcServer, remote.NewServer(cfg, rubyServer, locator))
 	gitalypb.RegisterServerServiceServer(grpcServer, server.NewServer(cfg.Storages))
 	gitalypb.RegisterObjectPoolServiceServer(grpcServer, objectpool.NewServer(cfg, locator))
 	gitalypb.RegisterHookServiceServer(grpcServer, hook.NewServer(cfg, hookManager))
diff --git a/internal/gitaly/service/remote/fetch_internal_remote.go b/internal/gitaly/service/remote/fetch_internal_remote.go
index 72fe4765a..321be53b0 100644
--- a/internal/gitaly/service/remote/fetch_internal_remote.go
+++ b/internal/gitaly/service/remote/fetch_internal_remote.go
@@ -27,7 +27,7 @@ func (s *server) FetchInternalRemote(ctx context.Context, req *gitalypb.FetchInt
 		return nil, status.Errorf(codes.InvalidArgument, "FetchInternalRemote: %v", err)
 	}
 
-	env, err := gitalyssh.UploadPackEnv(ctx, &gitalypb.SSHUploadPackRequest{Repository: req.RemoteRepository})
+	env, err := gitalyssh.UploadPackEnv(ctx, s.cfg, &gitalypb.SSHUploadPackRequest{Repository: req.RemoteRepository})
 	if err != nil {
 		return nil, fmt.Errorf("upload pack environment: %w", err)
 	}
diff --git a/internal/gitaly/service/remote/server.go b/internal/gitaly/service/remote/server.go
index 7da72b02d..ef48bbf9a 100644
--- a/internal/gitaly/service/remote/server.go
+++ b/internal/gitaly/service/remote/server.go
@@ -2,12 +2,14 @@ package remote
 
 import (
 	"gitlab.com/gitlab-org/gitaly/client"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"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 {
+	cfg     config.Cfg
 	ruby    *rubyserver.Server
 	locator storage.Locator
 
@@ -15,8 +17,9 @@ type server struct {
 }
 
 // NewServer creates a new instance of a grpc RemoteServiceServer
-func NewServer(rs *rubyserver.Server, locator storage.Locator) gitalypb.RemoteServiceServer {
+func NewServer(cfg config.Cfg, rs *rubyserver.Server, locator storage.Locator) gitalypb.RemoteServiceServer {
 	return &server{
+		cfg:     cfg,
 		ruby:    rs,
 		locator: locator,
 		conns: client.NewPoolWithOptions(
diff --git a/internal/gitaly/service/remote/testhelper_test.go b/internal/gitaly/service/remote/testhelper_test.go
index 44cd2bdaa..c90669e73 100644
--- a/internal/gitaly/service/remote/testhelper_test.go
+++ b/internal/gitaly/service/remote/testhelper_test.go
@@ -38,7 +38,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, config.NewLocator(config.Config)))
+	gitalypb.RegisterRemoteServiceServer(srv.GrpcServer(), NewServer(config.Config, RubyServer, config.NewLocator(config.Config)))
 	reflection.Register(srv.GrpcServer())
 
 	srv.Start(t)
diff --git a/internal/gitaly/service/repository/fetch.go b/internal/gitaly/service/repository/fetch.go
index 417e1f2f8..3d7d357a7 100644
--- a/internal/gitaly/service/repository/fetch.go
+++ b/internal/gitaly/service/repository/fetch.go
@@ -76,7 +76,7 @@ func (s *server) FetchSourceBranch(ctx context.Context, req *gitalypb.FetchSourc
 	// There's no need to perform the fetch if we already have the object
 	// available.
 	if !containsObject {
-		env, err := gitalyssh.UploadPackEnv(ctx, &gitalypb.SSHUploadPackRequest{Repository: req.SourceRepository})
+		env, err := gitalyssh.UploadPackEnv(ctx, s.cfg, &gitalypb.SSHUploadPackRequest{Repository: req.SourceRepository})
 		if err != nil {
 			return nil, err
 		}
diff --git a/internal/gitaly/service/repository/fork.go b/internal/gitaly/service/repository/fork.go
index 721b07c2f..46c96b7fb 100644
--- a/internal/gitaly/service/repository/fork.go
+++ b/internal/gitaly/service/repository/fork.go
@@ -48,7 +48,7 @@ func (s *server) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest
 		return nil, status.Errorf(codes.Internal, "CreateFork: create dest dir: %v", err)
 	}
 
-	env, err := gitalyssh.UploadPackEnv(ctx, &gitalypb.SSHUploadPackRequest{Repository: sourceRepository})
+	env, err := gitalyssh.UploadPackEnv(ctx, s.cfg, &gitalypb.SSHUploadPackRequest{Repository: sourceRepository})
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/gitalyssh/gitalyssh.go b/internal/gitalyssh/gitalyssh.go
index 4b57f6168..26ccaad9f 100644
--- a/internal/gitalyssh/gitalyssh.go
+++ b/internal/gitalyssh/gitalyssh.go
@@ -30,15 +30,16 @@ var (
 	envInjector = tracing.NewEnvInjector()
 )
 
-func UploadPackEnv(ctx context.Context, req *gitalypb.SSHUploadPackRequest) ([]string, error) {
-	env, err := commandEnv(ctx, req.Repository.StorageName, "upload-pack", req)
+// UploadPackEnv returns a list of the key=val pairs required to set proper configuration options for upload-pack command.
+func UploadPackEnv(ctx context.Context, cfg config.Cfg, req *gitalypb.SSHUploadPackRequest) ([]string, error) {
+	env, err := commandEnv(ctx, cfg, req.Repository.StorageName, "upload-pack", req)
 	if err != nil {
 		return nil, err
 	}
 	return envInjector(ctx, env), nil
 }
 
-func commandEnv(ctx context.Context, storageName, command string, message proto.Message) ([]string, error) {
+func commandEnv(ctx context.Context, cfg config.Cfg, storageName, command string, message proto.Message) ([]string, error) {
 	var pbMarshaler jsonpb.Marshaler
 	payload, err := pbMarshaler.MarshalToString(message)
 	if err != nil {
@@ -63,7 +64,7 @@ func commandEnv(ctx context.Context, storageName, command string, message proto.
 
 	return []string{
 		fmt.Sprintf("GITALY_PAYLOAD=%s", payload),
-		fmt.Sprintf("GIT_SSH_COMMAND=%s %s", gitalySSHPath(), command),
+		fmt.Sprintf("GIT_SSH_COMMAND=%s %s", filepath.Join(cfg.BinDir, "gitaly-ssh"), command),
 		fmt.Sprintf("GITALY_ADDRESS=%s", storageInfo.Address),
 		fmt.Sprintf("GITALY_TOKEN=%s", storageInfo.Token),
 		fmt.Sprintf("GITALY_FEATUREFLAGS=%s", strings.Join(featureFlagPairs, ",")),
@@ -76,7 +77,3 @@ func commandEnv(ctx context.Context, storageName, command string, message proto.
 		fmt.Sprintf("%s=%s", gitaly_x509.SSLCertFile, os.Getenv(gitaly_x509.SSLCertFile)),
 	}, nil
 }
-
-func gitalySSHPath() string {
-	return filepath.Join(config.Config.BinDir, "gitaly-ssh")
-}
diff --git a/internal/gitalyssh/gitalyssh_test.go b/internal/gitalyssh/gitalyssh_test.go
index 7c7fe9dc9..17173bdaa 100644
--- a/internal/gitalyssh/gitalyssh_test.go
+++ b/internal/gitalyssh/gitalyssh_test.go
@@ -3,7 +3,6 @@ package gitalyssh
 import (
 	"encoding/base64"
 	"fmt"
-	"path/filepath"
 	"testing"
 
 	"github.com/golang/protobuf/jsonpb"
@@ -34,11 +33,11 @@ func TestUploadPackEnv(t *testing.T) {
 	expectedPayload, err := pbMarshaler.MarshalToString(&req)
 	require.NoError(t, err)
 
-	env, err := UploadPackEnv(ctx, &req)
+	env, err := UploadPackEnv(ctx, config.Cfg{BinDir: "/path/bin"}, &req)
 
 	require.NoError(t, err)
 	require.Subset(t, env, []string{
-		fmt.Sprintf("GIT_SSH_COMMAND=%s upload-pack", filepath.Join(config.Config.BinDir, "gitaly-ssh")),
+		"GIT_SSH_COMMAND=/path/bin/gitaly-ssh upload-pack",
 		fmt.Sprintf("GITALY_PAYLOAD=%s", expectedPayload),
 		"CORRELATION_ID=correlation-id-1",
 		"GIT_SSH_VARIANT=simple",
diff --git a/internal/praefect/replicator_test.go b/internal/praefect/replicator_test.go
index 121a2142e..6bab2c2a4 100644
--- a/internal/praefect/replicator_test.go
+++ b/internal/praefect/replicator_test.go
@@ -1034,7 +1034,7 @@ func newReplicationService(tb testing.TB) (*grpc.Server, string) {
 	locator := gitaly_config.NewLocator(gitaly_config.Config)
 	gitalypb.RegisterRepositoryServiceServer(svr, repository.NewServer(gitaly_config.Config, RubyServer, locator))
 	gitalypb.RegisterObjectPoolServiceServer(svr, objectpoolservice.NewServer(gitaly_config.Config, locator))
-	gitalypb.RegisterRemoteServiceServer(svr, remote.NewServer(RubyServer, locator))
+	gitalypb.RegisterRemoteServiceServer(svr, remote.NewServer(gitaly_config.Config, RubyServer, locator))
 	gitalypb.RegisterSSHServiceServer(svr, ssh.NewServer(locator))
 	gitalypb.RegisterRefServiceServer(svr, ref.NewServer(locator))
 	healthpb.RegisterHealthServer(svr, health.NewServer())