Add and remove ref hooks as needed

Once the cache middleware was removed, it created new failures in CI
that alert us to where ref hook options are needed. This updates those
places and removes places where it is no longer needed due to updates in
internal/git/subcommand.go

6 files changed, 22 insertions, 17 deletions

diff --git a/internal/git/remote.go b/internal/git/remote.go
index 2e9cf0b11..372bd577d 100644
--- a/internal/git/remote.go
+++ b/internal/git/remote.go
@@ -8,6 +8,7 @@ import (
 
 	"gitlab.com/gitlab-org/gitaly/client"
 	"gitlab.com/gitlab-org/gitaly/internal/git/repository"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/storage"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
@@ -191,6 +192,7 @@ func (repo RepositoryRemote) Add(ctx context.Context, name, url string, opts Rem
 			Args:  []string{name, url},
 		},
 		WithStderr(&stderr),
+		WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo.repo), config.Config),
 	)
 	if err != nil {
 		return err
@@ -220,6 +222,7 @@ func (repo RepositoryRemote) Remove(ctx context.Context, name string) error {
 			Args:  []string{name},
 		},
 		WithStderr(&stderr),
+		WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo.repo), config.Config),
 	)
 	if err != nil {
 		return err
@@ -264,6 +267,7 @@ func (repo RepositoryRemote) SetURL(ctx context.Context, name, url string, opts
 			Args:  []string{name, url},
 		},
 		WithStderr(&stderr),
+		WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo.repo), config.Config),
 	)
 	if err != nil {
 		return err
diff --git a/internal/git/repository.go b/internal/git/repository.go
index b3b65adc0..81a433192 100644
--- a/internal/git/repository.go
+++ b/internal/git/repository.go
@@ -421,6 +421,7 @@ func (repo *localRepository) FetchRemote(ctx context.Context, remoteName string,
 			Args:  []string{remoteName},
 		},
 		WithStderr(opts.Stderr),
+		WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo.repo), config.Config),
 	)
 	if err != nil {
 		return err
diff --git a/internal/git/safecmd_test.go b/internal/git/safecmd_test.go
index 9c60ab1cf..735b34d61 100644
--- a/internal/git/safecmd_test.go
+++ b/internal/git/safecmd_test.go
@@ -112,6 +112,7 @@ func TestSafeCmdInvalidArg(t *testing.T) {
 			&gitalypb.Repository{},
 			tt.globals,
 			tt.subCmd,
+			git.WithRefTxHook(context.Background(), &gitalypb.Repository{}, config.Config),
 		)
 		require.EqualError(t, err, tt.errMsg)
 		require.True(t, git.IsInvalidArgErr(err))
@@ -198,21 +199,22 @@ func TestSafeCmdValid(t *testing.T) {
 			expectArgs: []string{"--contributing", "--author", "a-gopher", "accept", "--is-important", "--why", "looking-for-first-contribution", "mr", endOfOptions},
 		},
 	} {
-		cmd, err := git.SafeCmd(ctx, testRepo, tt.globals, tt.subCmd)
+		opts := []git.CmdOpt{git.WithRefTxHook(ctx, &gitalypb.Repository{}, config.Config)}
+		cmd, err := git.SafeCmd(ctx, testRepo, tt.globals, tt.subCmd, opts...)
 		require.NoError(t, err)
 		// ignore first 3 indeterministic args (executable path and repo args)
 		require.Equal(t, tt.expectArgs, cmd.Args()[3:])
 
-		cmd, err = git.SafeCmdWithEnv(ctx, nil, testRepo, tt.globals, tt.subCmd)
+		cmd, err = git.SafeCmdWithEnv(ctx, nil, testRepo, tt.globals, tt.subCmd, opts...)
 		require.NoError(t, err)
 		// ignore first 3 indeterministic args (executable path and repo args)
 		require.Equal(t, tt.expectArgs, cmd.Args()[3:])
 
-		cmd, err = git.SafeStdinCmd(ctx, testRepo, tt.globals, tt.subCmd)
+		cmd, err = git.SafeStdinCmd(ctx, testRepo, tt.globals, tt.subCmd, opts...)
 		require.NoError(t, err)
 		require.Equal(t, tt.expectArgs, cmd.Args()[3:])
 
-		cmd, err = git.SafeBareCmd(ctx, git.CmdStream{}, nil, tt.globals, tt.subCmd)
+		cmd, err = git.SafeBareCmd(ctx, git.CmdStream{}, nil, tt.globals, tt.subCmd, opts...)
 		require.NoError(t, err)
 		// ignore first indeterministic arg (executable path)
 		require.Equal(t, tt.expectArgs, cmd.Args()[1:])
@@ -243,7 +245,8 @@ func TestSafeCmdWithEnv(t *testing.T) {
 
 	env := []string{"TEST_VAR1=1", "TEST_VAR2=2"}
 
-	cmd, err := git.SafeCmdWithEnv(ctx, env, testRepo, globals, subCmd)
+	opts := []git.CmdOpt{git.WithRefTxHook(ctx, &gitalypb.Repository{}, config.Config)}
+	cmd, err := git.SafeCmdWithEnv(ctx, env, testRepo, globals, subCmd, opts...)
 	require.NoError(t, err)
 	// ignore first 3 indeterministic args (executable path and repo args)
 	require.Equal(t, expectArgs, cmd.Args()[3:])
diff --git a/internal/gitaly/service/ref/refs.go b/internal/gitaly/service/ref/refs.go
index d439a06e0..c318188de 100644
--- a/internal/gitaly/service/ref/refs.go
+++ b/internal/gitaly/service/ref/refs.go
@@ -413,13 +413,16 @@ func parseTagLine(c *catfile.Batch, tagLine string) (*gitalypb.Tag, error) {
 }
 
 func findTag(ctx context.Context, repository *gitalypb.Repository, tagName []byte) (*gitalypb.Tag, error) {
-	tagCmd, err := git.SafeCmd(ctx, repository, nil, git.SubCmd{
-		Name: "tag",
-		Flags: []git.Option{
-			git.Flag{Name: "-l"}, git.ValueFlag{"--format", tagFormat},
+	tagCmd, err := git.SafeCmd(ctx, repository, nil,
+		git.SubCmd{
+			Name: "tag",
+			Flags: []git.Option{
+				git.Flag{Name: "-l"}, git.ValueFlag{"--format", tagFormat},
+			},
+			Args: []string{string(tagName)},
 		},
-		Args: []string{string(tagName)},
-	})
+		git.WithRefTxHook(ctx, repository, config.Config),
+	)
 	if err != nil {
 		return nil, fmt.Errorf("for-each-ref error: %v", err)
 	}
diff --git a/internal/gitaly/service/repository/fsck.go b/internal/gitaly/service/repository/fsck.go
index ea5862106..4c2d5c3cc 100644
--- a/internal/gitaly/service/repository/fsck.go
+++ b/internal/gitaly/service/repository/fsck.go
@@ -20,7 +20,6 @@ func (s *server) Fsck(ctx context.Context, req *gitalypb.FsckRequest) (*gitalypb
 	cmd, err := git.SafeBareCmd(ctx, git.CmdStream{Out: &stdout, Err: &stderr}, env,
 		[]git.Option{git.ValueFlag{"--git-dir", repoPath}},
 		git.SubCmd{Name: "fsck"},
-		git.WithRefTxHook(ctx, req.GetRepository(), s.cfg),
 	)
 	if err != nil {
 		return nil, err
diff --git a/internal/gitaly/service/repository/testdata/gitlab-shell/config.yml b/internal/gitaly/service/repository/testdata/gitlab-shell/config.yml
deleted file mode 100644
index bcfe53ab6..000000000
--- a/internal/gitaly/service/repository/testdata/gitlab-shell/config.yml
+++ /dev/null
@@ -1,5 +0,0 @@
-gitlab_url: http://127.0.0.1:51981
-http_settings:
-  user: ""
-  password: ""
-custom_hooks_dir: ""