Merge branch 'ps-di-config-updateref' into 'master'

Broke dependency of updateref on config.Config

See merge request gitlab-org/gitaly!2958

20 files changed, 41 insertions, 34 deletions

diff --git a/cmd/gitaly-ssh/upload_pack_test.go b/cmd/gitaly-ssh/upload_pack_test.go
index 88d60ae9b..56c7bfeea 100644
--- a/cmd/gitaly-ssh/upload_pack_test.go
+++ b/cmd/gitaly-ssh/upload_pack_test.go
@@ -37,7 +37,7 @@ func TestVisibilityOfHiddenRefs(t *testing.T) {
 	existingSha := "1e292f8fedd741b75372e19097c76d327140c312"
 	keepAroundRef := fmt.Sprintf("%s/%s", keepAroundNamespace, existingSha)
 
-	updater, err := updateref.New(ctx, testRepo)
+	updater, err := updateref.New(ctx, config.Config, testRepo)
 
 	require.NoError(t, err)
 	require.NoError(t, updater.Create(git.ReferenceName(keepAroundRef), existingSha))
diff --git a/internal/git/objectpool/fetch.go b/internal/git/objectpool/fetch.go
index 6fc0f8c4e..8f43dcb26 100644
--- a/internal/git/objectpool/fetch.go
+++ b/internal/git/objectpool/fetch.go
@@ -18,6 +18,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git/housekeeping"
 	"gitlab.com/gitlab-org/gitaly/internal/git/repository"
 	"gitlab.com/gitlab-org/gitaly/internal/git/updateref"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
@@ -111,7 +112,7 @@ func (o *ObjectPool) FetchFromOrigin(ctx context.Context, origin *gitalypb.Repos
 		return err
 	}
 
-	if err := rescueDanglingObjects(ctx, o); err != nil {
+	if err := rescueDanglingObjects(ctx, o.cfg, o); err != nil {
 		return err
 	}
 
@@ -143,7 +144,7 @@ const danglingObjectNamespace = "refs/dangling/"
 // relies on. There is currently no way for us to reliably determine if
 // an object is still used anywhere, so the only safe thing to do is to
 // assume that every object _is_ used.
-func rescueDanglingObjects(ctx context.Context, repo repository.GitRepo) error {
+func rescueDanglingObjects(ctx context.Context, cfg config.Cfg, repo repository.GitRepo) error {
 	fsck, err := git.NewCommand(ctx, repo, nil, git.SubCmd{
 		Name:  "fsck",
 		Flags: []git.Option{git.Flag{Name: "--connectivity-only"}, git.Flag{Name: "--dangling"}},
@@ -152,7 +153,7 @@ func rescueDanglingObjects(ctx context.Context, repo repository.GitRepo) error {
 		return err
 	}
 
-	updater, err := updateref.New(ctx, repo, updateref.WithDisabledTransactions())
+	updater, err := updateref.New(ctx, cfg, repo, updateref.WithDisabledTransactions())
 	if err != nil {
 		return err
 	}
diff --git a/internal/git/updateref/updateref.go b/internal/git/updateref/updateref.go
index 348c00ed5..f1a3db68c 100644
--- a/internal/git/updateref/updateref.go
+++ b/internal/git/updateref/updateref.go
@@ -40,13 +40,13 @@ func WithDisabledTransactions() UpdaterOpt {
 //
 // It is important that ctx gets canceled somewhere. If it doesn't, the process
 // spawned by New() may never terminate.
-func New(ctx context.Context, repo repository.GitRepo, opts ...UpdaterOpt) (*Updater, error) {
+func New(ctx context.Context, conf config.Cfg, repo repository.GitRepo, opts ...UpdaterOpt) (*Updater, error) {
 	var cfg updaterConfig
 	for _, opt := range opts {
 		opt(&cfg)
 	}
 
-	txOption := git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo), config.Config)
+	txOption := git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo), conf)
 	if cfg.disableTransactions {
 		txOption = git.WithDisabledHooks()
 	}
diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go
index 6fc28965f..8ba70d686 100644
--- a/internal/git/updateref/updateref_test.go
+++ b/internal/git/updateref/updateref_test.go
@@ -46,7 +46,7 @@ func TestCreate(t *testing.T) {
 	headCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD")
 	require.NoError(t, err)
 
-	updater, err := New(ctx, testRepo)
+	updater, err := New(ctx, config.Config, testRepo)
 	require.NoError(t, err)
 
 	ref := git.ReferenceName("refs/heads/_create")
@@ -69,7 +69,7 @@ func TestUpdate(t *testing.T) {
 	headCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD")
 	require.NoError(t, err)
 
-	updater, err := New(ctx, testRepo)
+	updater, err := New(ctx, config.Config, testRepo)
 	require.NoError(t, err)
 
 	ref := git.ReferenceName("refs/heads/feature")
@@ -103,7 +103,7 @@ func TestDelete(t *testing.T) {
 	ctx, testRepo, _, teardown := setup(t)
 	defer teardown()
 
-	updater, err := New(ctx, testRepo)
+	updater, err := New(ctx, config.Config, testRepo)
 	require.NoError(t, err)
 
 	ref := git.ReferenceName("refs/heads/feature")
@@ -127,7 +127,7 @@ func TestBulkOperation(t *testing.T) {
 	headCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD")
 	require.NoError(t, err)
 
-	updater, err := New(ctx, testRepo)
+	updater, err := New(ctx, config.Config, testRepo)
 	require.NoError(t, err)
 
 	for i := 0; i < 1000; i++ {
@@ -152,7 +152,7 @@ func TestContextCancelAbortsRefChanges(t *testing.T) {
 	require.NoError(t, err)
 
 	childCtx, childCancel := context.WithCancel(ctx)
-	updater, err := New(childCtx, testRepo)
+	updater, err := New(childCtx, config.Config, testRepo)
 	require.NoError(t, err)
 
 	ref := git.ReferenceName("refs/heads/_shouldnotexist")
@@ -182,7 +182,7 @@ func TestUpdater_closingStdinAbortsChanges(t *testing.T) {
 
 	ref := git.ReferenceName("refs/heads/shouldnotexist")
 
-	updater, err := New(ctx, testRepo)
+	updater, err := New(ctx, config.Config, testRepo)
 	require.NoError(t, err)
 	require.NoError(t, updater.Create(ref, headCommit.Id))
 
diff --git a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go
index 67b80995a..1065a99e8 100644
--- a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go
+++ b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go
@@ -47,7 +47,7 @@ func (s *server) ApplyBfgObjectMapStream(server gitalypb.CleanupService_ApplyBfg
 
 	// It doesn't matter if new internal references are added after this RPC
 	// starts running - they shouldn't point to the objects removed by the BFG
-	cleaner, err := internalrefs.NewCleaner(ctx, repo, notifier.Notify)
+	cleaner, err := internalrefs.NewCleaner(ctx, s.cfg, repo, notifier.Notify)
 	if err != nil {
 		return helper.ErrInternal(err)
 	}
diff --git a/internal/gitaly/service/cleanup/internalrefs/cleaner.go b/internal/gitaly/service/cleanup/internalrefs/cleaner.go
index f2add93cf..498402125 100644
--- a/internal/gitaly/service/cleanup/internalrefs/cleaner.go
+++ b/internal/gitaly/service/cleanup/internalrefs/cleaner.go
@@ -11,6 +11,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/updateref"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
@@ -46,13 +47,13 @@ type ErrInvalidObjectMap error
 
 // NewCleaner builds a new instance of Cleaner, which is used to apply a
 // filter-repo or BFG object map to a repository.
-func NewCleaner(ctx context.Context, repo *gitalypb.Repository, forEach ForEachFunc) (*Cleaner, error) {
+func NewCleaner(ctx context.Context, cfg config.Cfg, repo *gitalypb.Repository, forEach ForEachFunc) (*Cleaner, error) {
 	table, err := buildLookupTable(ctx, repo)
 	if err != nil {
 		return nil, err
 	}
 
-	updater, err := updateref.New(ctx, repo)
+	updater, err := updateref.New(ctx, cfg, repo)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/gitaly/service/cleanup/server.go b/internal/gitaly/service/cleanup/server.go
index 4e8137dbb..7dba3ee2b 100644
--- a/internal/gitaly/service/cleanup/server.go
+++ b/internal/gitaly/service/cleanup/server.go
@@ -1,15 +1,17 @@
 package cleanup
 
 import (
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/storage"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
 type server struct {
+	cfg     config.Cfg
 	locator storage.Locator
 }
 
 // NewServer creates a new instance of a grpc CleanupServer
-func NewServer(locator storage.Locator) gitalypb.CleanupServiceServer {
-	return &server{locator: locator}
+func NewServer(cfg config.Cfg, locator storage.Locator) gitalypb.CleanupServiceServer {
+	return &server{cfg: cfg, locator: locator}
 }
diff --git a/internal/gitaly/service/cleanup/testhelper_test.go b/internal/gitaly/service/cleanup/testhelper_test.go
index ed9b2318d..2cd8f663b 100644
--- a/internal/gitaly/service/cleanup/testhelper_test.go
+++ b/internal/gitaly/service/cleanup/testhelper_test.go
@@ -29,7 +29,7 @@ func runCleanupServiceServer(t *testing.T, cfg config.Cfg) (string, func()) {
 	srv := testhelper.NewServer(t, nil, nil, testhelper.WithInternalSocket(cfg))
 
 	locator := config.NewLocator(cfg)
-	gitalypb.RegisterCleanupServiceServer(srv.GrpcServer(), NewServer(locator))
+	gitalypb.RegisterCleanupServiceServer(srv.GrpcServer(), NewServer(cfg, locator))
 	gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hookservice.NewServer(cfg, hook.NewManager(locator, hook.GitlabAPIStub, cfg)))
 	reflection.Register(srv.GrpcServer())
 
diff --git a/internal/gitaly/service/operations/testhelper_test.go b/internal/gitaly/service/operations/testhelper_test.go
index 2f432a823..4db77f2cd 100644
--- a/internal/gitaly/service/operations/testhelper_test.go
+++ b/internal/gitaly/service/operations/testhelper_test.go
@@ -87,7 +87,7 @@ func runOperationServiceServerWithRubyServer(t *testing.T, ruby *rubyserver.Serv
 	gitalypb.RegisterOperationServiceServer(srv.GrpcServer(), server)
 	gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hook.NewServer(config.Config, hookManager))
 	gitalypb.RegisterRepositoryServiceServer(srv.GrpcServer(), repository.NewServer(config.Config, ruby, locator))
-	gitalypb.RegisterRefServiceServer(srv.GrpcServer(), ref.NewServer(locator))
+	gitalypb.RegisterRefServiceServer(srv.GrpcServer(), ref.NewServer(config.Config, locator))
 	gitalypb.RegisterCommitServiceServer(srv.GrpcServer(), commit.NewServer(config.Config, locator))
 	gitalypb.RegisterSSHServiceServer(srv.GrpcServer(), ssh.NewServer(locator))
 	reflection.Register(srv.GrpcServer())
diff --git a/internal/gitaly/service/operations/update_with_hooks.go b/internal/gitaly/service/operations/update_with_hooks.go
index 431c14ff3..0b395f86f 100644
--- a/internal/gitaly/service/operations/update_with_hooks.go
+++ b/internal/gitaly/service/operations/update_with_hooks.go
@@ -88,7 +88,7 @@ func (s *Server) updateReferenceWithHooks(ctx context.Context, repo *gitalypb.Re
 		return preReceiveError{message: err.Error()}
 	}
 
-	updater, err := updateref.New(ctx, repo)
+	updater, err := updateref.New(ctx, s.cfg, repo)
 	if err != nil {
 		return err
 	}
diff --git a/internal/gitaly/service/ref/delete_refs.go b/internal/gitaly/service/ref/delete_refs.go
index c205e7dca..c772684da 100644
--- a/internal/gitaly/service/ref/delete_refs.go
+++ b/internal/gitaly/service/ref/delete_refs.go
@@ -19,7 +19,7 @@ func (s *server) DeleteRefs(ctx context.Context, in *gitalypb.DeleteRefsRequest)
 		return nil, status.Errorf(codes.InvalidArgument, "DeleteRefs: %v", err)
 	}
 
-	updater, err := updateref.New(ctx, in.GetRepository())
+	updater, err := updateref.New(ctx, s.cfg, in.GetRepository())
 	if err != nil {
 		if errors.Is(err, git.ErrInvalidArg) {
 			return nil, helper.ErrInvalidArgument(err)
diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go
index 63f32e668..b0a5e9b84 100644
--- a/internal/gitaly/service/ref/refs_test.go
+++ b/internal/gitaly/service/ref/refs_test.go
@@ -82,7 +82,7 @@ func TestFindAllBranchNamesVeryLargeResponse(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	updater, err := updateref.New(ctx, testRepo)
+	updater, err := updateref.New(ctx, config.Config, testRepo)
 	require.NoError(t, err)
 
 	// We want to create enough refs to overflow the default bufio.Scanner
diff --git a/internal/gitaly/service/ref/server.go b/internal/gitaly/service/ref/server.go
index 69370613b..af15f66eb 100644
--- a/internal/gitaly/service/ref/server.go
+++ b/internal/gitaly/service/ref/server.go
@@ -1,15 +1,17 @@
 package ref
 
 import (
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/storage"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
 type server struct {
+	cfg     config.Cfg
 	locator storage.Locator
 }
 
 // NewServer creates a new instance of a grpc RefServer
-func NewServer(locator storage.Locator) gitalypb.RefServiceServer {
-	return &server{locator: locator}
+func NewServer(cfg config.Cfg, locator storage.Locator) gitalypb.RefServiceServer {
+	return &server{cfg: cfg, locator: locator}
 }
diff --git a/internal/gitaly/service/ref/testhelper_test.go b/internal/gitaly/service/ref/testhelper_test.go
index f12cb80ef..c116c8058 100644
--- a/internal/gitaly/service/ref/testhelper_test.go
+++ b/internal/gitaly/service/ref/testhelper_test.go
@@ -45,7 +45,7 @@ func runRefServiceServer(t *testing.T) (func(), string) {
 	srv := testhelper.NewServer(t, nil, nil, testhelper.WithInternalSocket(config.Config))
 
 	locator := config.NewLocator(config.Config)
-	gitalypb.RegisterRefServiceServer(srv.GrpcServer(), NewServer(locator))
+	gitalypb.RegisterRefServiceServer(srv.GrpcServer(), NewServer(config.Config, locator))
 	gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hookservice.NewServer(config.Config, hook.NewManager(locator, hook.GitlabAPIStub, config.Config)))
 
 	srv.Start(t)
diff --git a/internal/gitaly/service/register.go b/internal/gitaly/service/register.go
index 83f24ab49..ebd491c96 100644
--- a/internal/gitaly/service/register.go
+++ b/internal/gitaly/service/register.go
@@ -70,12 +70,12 @@ func RegisterAll(grpcServer *grpc.Server, cfg config.Cfg, rubyServer *rubyserver
 	})
 
 	gitalypb.RegisterBlobServiceServer(grpcServer, blob.NewServer(rubyServer, locator))
-	gitalypb.RegisterCleanupServiceServer(grpcServer, cleanup.NewServer(locator))
+	gitalypb.RegisterCleanupServiceServer(grpcServer, cleanup.NewServer(cfg, locator))
 	gitalypb.RegisterCommitServiceServer(grpcServer, commit.NewServer(cfg, locator))
 	gitalypb.RegisterDiffServiceServer(grpcServer, diff.NewServer(locator))
 	gitalypb.RegisterNamespaceServiceServer(grpcServer, namespace.NewServer(locator))
 	gitalypb.RegisterOperationServiceServer(grpcServer, operations.NewServer(cfg, rubyServer, hookManager, locator, conns))
-	gitalypb.RegisterRefServiceServer(grpcServer, ref.NewServer(locator))
+	gitalypb.RegisterRefServiceServer(grpcServer, ref.NewServer(cfg, locator))
 	gitalypb.RegisterRepositoryServiceServer(grpcServer, repository.NewServer(cfg, rubyServer, locator))
 	gitalypb.RegisterSSHServiceServer(grpcServer, ssh.NewServer(
 		locator,
diff --git a/internal/gitaly/service/remote/fetch_internal_remote_test.go b/internal/gitaly/service/remote/fetch_internal_remote_test.go
index db7acc65a..8ce2c8db3 100644
--- a/internal/gitaly/service/remote/fetch_internal_remote_test.go
+++ b/internal/gitaly/service/remote/fetch_internal_remote_test.go
@@ -49,7 +49,7 @@ func TestSuccessfulFetchInternalRemote(t *testing.T) {
 	locator := config.NewLocator(config.Config)
 	gitaly0Server := testhelper.NewServer(t, nil, nil, testhelper.WithStorages([]string{"gitaly-0"}))
 	gitalypb.RegisterSSHServiceServer(gitaly0Server.GrpcServer(), ssh.NewServer(locator))
-	gitalypb.RegisterRefServiceServer(gitaly0Server.GrpcServer(), ref.NewServer(config.NewLocator(config.Config)))
+	gitalypb.RegisterRefServiceServer(gitaly0Server.GrpcServer(), ref.NewServer(config.Config, locator))
 	reflection.Register(gitaly0Server.GrpcServer())
 	gitaly0Server.Start(t)
 	defer gitaly0Server.Stop()
diff --git a/internal/gitaly/service/repository/optimize_test.go b/internal/gitaly/service/repository/optimize_test.go
index be58f282c..5e2ebed70 100644
--- a/internal/gitaly/service/repository/optimize_test.go
+++ b/internal/gitaly/service/repository/optimize_test.go
@@ -101,7 +101,7 @@ func TestOptimizeRepository(t *testing.T) {
 	blobs := 10
 	blobIDs := testhelper.WriteBlobs(t, testRepoPath, blobs)
 
-	updater, err := updateref.New(ctx, testRepo)
+	updater, err := updateref.New(ctx, config.Config, testRepo)
 	require.NoError(t, err)
 
 	for _, blobID := range blobIDs {
diff --git a/internal/gitaly/service/repository/write_ref.go b/internal/gitaly/service/repository/write_ref.go
index d7d33e8dc..17366f551 100644
--- a/internal/gitaly/service/repository/write_ref.go
+++ b/internal/gitaly/service/repository/write_ref.go
@@ -7,6 +7,7 @@ import (
 
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/updateref"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
@@ -26,7 +27,7 @@ func (s *server) writeRef(ctx context.Context, req *gitalypb.WriteRefRequest) er
 	if string(req.Ref) == "HEAD" {
 		return s.updateSymbolicRef(ctx, req)
 	}
-	return updateRef(ctx, req)
+	return updateRef(ctx, s.cfg, req)
 }
 
 func (s *server) updateSymbolicRef(ctx context.Context, req *gitalypb.WriteRefRequest) error {
@@ -46,8 +47,8 @@ func (s *server) updateSymbolicRef(ctx context.Context, req *gitalypb.WriteRefRe
 	return nil
 }
 
-func updateRef(ctx context.Context, req *gitalypb.WriteRefRequest) error {
-	u, err := updateref.New(ctx, req.GetRepository())
+func updateRef(ctx context.Context, cfg config.Cfg, req *gitalypb.WriteRefRequest) error {
+	u, err := updateref.New(ctx, cfg, req.GetRepository())
 	if err != nil {
 		return fmt.Errorf("error when running creating new updater: %v", err)
 	}
diff --git a/internal/middleware/commandstatshandler/commandstatshandler_test.go b/internal/middleware/commandstatshandler/commandstatshandler_test.go
index bccb9d11b..fdf93e26a 100644
--- a/internal/middleware/commandstatshandler/commandstatshandler_test.go
+++ b/internal/middleware/commandstatshandler/commandstatshandler_test.go
@@ -37,7 +37,7 @@ func createNewServer(t *testing.T) *grpc.Server {
 
 	server := grpc.NewServer(opts...)
 
-	gitalypb.RegisterRefServiceServer(server, ref.NewServer(config.NewLocator(config.Config)))
+	gitalypb.RegisterRefServiceServer(server, ref.NewServer(config.Config, config.NewLocator(config.Config)))
 
 	return server
 }
diff --git a/internal/praefect/replicator_test.go b/internal/praefect/replicator_test.go
index 6bab2c2a4..4962090a0 100644
--- a/internal/praefect/replicator_test.go
+++ b/internal/praefect/replicator_test.go
@@ -1036,7 +1036,7 @@ func newReplicationService(tb testing.TB) (*grpc.Server, string) {
 	gitalypb.RegisterObjectPoolServiceServer(svr, objectpoolservice.NewServer(gitaly_config.Config, locator))
 	gitalypb.RegisterRemoteServiceServer(svr, remote.NewServer(gitaly_config.Config, RubyServer, locator))
 	gitalypb.RegisterSSHServiceServer(svr, ssh.NewServer(locator))
-	gitalypb.RegisterRefServiceServer(svr, ref.NewServer(locator))
+	gitalypb.RegisterRefServiceServer(svr, ref.NewServer(gitaly_config.Config, locator))
 	healthpb.RegisterHealthServer(svr, health.NewServer())
 	reflection.Register(svr)