Merge branch 'pks-reftx-hook-protorepo' into 'master'

git: Allow setup of reference-transaction hook with repo interface

See merge request gitlab-org/gitaly!3117

6 files changed, 19 insertions, 25 deletions

diff --git a/internal/git/hooks_options.go b/internal/git/hooks_options.go
index 67a23a986..bfd7780d1 100644
--- a/internal/git/hooks_options.go
+++ b/internal/git/hooks_options.go
@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 
 	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
+	"gitlab.com/gitlab-org/gitaly/internal/git/repository"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/log"
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/metadata"
@@ -25,13 +26,22 @@ func WithDisabledHooks() CmdOpt {
 // WithRefTxHook returns an option that populates the safe command with the
 // environment variables necessary to properly execute a reference hook for
 // repository changes that may possibly update references
-func WithRefTxHook(ctx context.Context, repo *gitalypb.Repository, cfg config.Cfg) CmdOpt {
+func WithRefTxHook(ctx context.Context, repo repository.GitRepo, cfg config.Cfg) CmdOpt {
 	return func(cc *cmdCfg) error {
 		if repo == nil {
 			return fmt.Errorf("missing repo: %w", ErrInvalidArg)
 		}
 
-		if err := cc.configureHooks(ctx, repo, cfg, nil); err != nil {
+		// The reference-transaction hook does not need any project-specific information
+		// about the repository. So in order to make the hook usable by sites which do not
+		// have a project repository available (e.g. object pools), this function accepts a
+		// `repository.GitRepo` and just creates an ad-hoc proto repo.
+		if err := cc.configureHooks(ctx, &gitalypb.Repository{
+			StorageName:                   repo.GetStorageName(),
+			GitAlternateObjectDirectories: repo.GetGitAlternateObjectDirectories(),
+			GitObjectDirectory:            repo.GetGitObjectDirectory(),
+			RelativePath:                  repo.GetRelativePath(),
+		}, cfg, nil); err != nil {
 			return fmt.Errorf("ref hook env var: %w", err)
 		}
 
diff --git a/internal/git/localrepo/refs.go b/internal/git/localrepo/refs.go
index 851d0da83..a35d56caf 100644
--- a/internal/git/localrepo/refs.go
+++ b/internal/git/localrepo/refs.go
@@ -11,7 +11,6 @@ import (
 
 	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
 // HasRevision checks if a revision in the repository exists. This will not
@@ -151,7 +150,7 @@ func (repo *Repo) UpdateRef(ctx context.Context, reference git.ReferenceName, ne
 			Flags: []git.Option{git.Flag{Name: "-z"}, git.Flag{Name: "--stdin"}},
 		},
 		git.WithStdin(strings.NewReader(fmt.Sprintf("update %s\x00%s\x00%s\x00", reference, newValue.String(), oldValue.String()))),
-		git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo), repo.cfg),
+		git.WithRefTxHook(ctx, repo, repo.cfg),
 	); err != nil {
 		return fmt.Errorf("UpdateRef: failed updating reference %q from %q to %q: %w", reference, newValue, oldValue, err)
 	}
diff --git a/internal/git/localrepo/remote.go b/internal/git/localrepo/remote.go
index 98ecea861..ae4501e25 100644
--- a/internal/git/localrepo/remote.go
+++ b/internal/git/localrepo/remote.go
@@ -8,7 +8,6 @@ import (
 	"strings"
 
 	"gitlab.com/gitlab-org/gitaly/internal/git"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
 // Remote provides functionality of the 'remote' git sub-command.
@@ -35,7 +34,7 @@ func (remote Remote) Add(ctx context.Context, name, url string, opts git.RemoteA
 			Args:   []string{name, url},
 		},
 		git.WithStderr(&stderr),
-		git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(remote.repo), remote.repo.cfg),
+		git.WithRefTxHook(ctx, remote.repo, remote.repo.cfg),
 	); err != nil {
 		switch {
 		case isExitWithCode(err, 3):
@@ -91,7 +90,7 @@ func (remote Remote) Remove(ctx context.Context, name string) error {
 			Args:   []string{name},
 		},
 		git.WithStderr(&stderr),
-		git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(remote.repo), remote.repo.cfg),
+		git.WithRefTxHook(ctx, remote.repo, remote.repo.cfg),
 	); err != nil {
 		switch {
 		case isExitWithCode(err, 2):
@@ -127,7 +126,7 @@ func (remote Remote) SetURL(ctx context.Context, name, url string, opts git.SetU
 			Args:   []string{name, url},
 		},
 		git.WithStderr(&stderr),
-		git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(remote.repo), remote.repo.cfg),
+		git.WithRefTxHook(ctx, remote.repo, remote.repo.cfg),
 	); err != nil {
 		switch {
 		case isExitWithCode(err, 2):
diff --git a/internal/git/remote/remote.go b/internal/git/remote/remote.go
index 787af75e4..940bca4b3 100644
--- a/internal/git/remote/remote.go
+++ b/internal/git/remote/remote.go
@@ -7,7 +7,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/repository"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
 //Remove removes the remote from repository
@@ -16,7 +15,7 @@ func Remove(ctx context.Context, cfg config.Cfg, repo repository.GitRepo, name s
 		Name:   "remote",
 		Action: "remove",
 		Args:   []string{name},
-	}, git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo), cfg))
+	}, git.WithRefTxHook(ctx, repo, cfg))
 	if err != nil {
 		return err
 	}
@@ -29,7 +28,7 @@ func Remove(ctx context.Context, cfg config.Cfg, repo repository.GitRepo, name s
 func Exists(ctx context.Context, cfg config.Cfg, repo repository.GitRepo, name string) (bool, error) {
 	cmd, err := git.NewCommand(ctx, repo, nil,
 		git.SubCmd{Name: "remote"},
-		git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo), cfg),
+		git.WithRefTxHook(ctx, repo, cfg),
 	)
 	if err != nil {
 		return false, err
diff --git a/internal/git/updateref/updateref.go b/internal/git/updateref/updateref.go
index e556d27ea..5defef251 100644
--- a/internal/git/updateref/updateref.go
+++ b/internal/git/updateref/updateref.go
@@ -8,7 +8,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/repository"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
 // Updater wraps a `git update-ref --stdin` process, presenting an interface
@@ -46,7 +45,7 @@ func New(ctx context.Context, conf config.Cfg, gitCmdFactory git.CommandFactory,
 		opt(&cfg)
 	}
 
-	txOption := git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo), conf)
+	txOption := git.WithRefTxHook(ctx, repo, conf)
 	if cfg.disableTransactions {
 		txOption = git.WithDisabledHooks()
 	}
diff --git a/internal/helper/repo.go b/internal/helper/repo.go
index b70e9c7a3..192d1c3b6 100644
--- a/internal/helper/repo.go
+++ b/internal/helper/repo.go
@@ -2,7 +2,6 @@ package helper
 
 import (
 	"gitlab.com/gitlab-org/gitaly/internal/git/repository"
-	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
 // RepoPathEqual compares if two repositories are in the same location
@@ -10,14 +9,3 @@ func RepoPathEqual(a, b repository.GitRepo) bool {
 	return a.GetStorageName() == b.GetStorageName() &&
 		a.GetRelativePath() == b.GetRelativePath()
 }
-
-// ProtoRepoFromRepo allows object pools and repository abstractions to be used
-// in places that require a concrete type
-func ProtoRepoFromRepo(repo repository.GitRepo) *gitalypb.Repository {
-	return &gitalypb.Repository{
-		StorageName:                   repo.GetStorageName(),
-		GitAlternateObjectDirectories: repo.GetGitAlternateObjectDirectories(),
-		GitObjectDirectory:            repo.GetGitObjectDirectory(),
-		RelativePath:                  repo.GetRelativePath(),
-	}
-}