diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-01-29 17:13:55 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-02-03 09:52:57 +0300 |
commit | 65da4cadf6b009f55840297c4d738580595aa0a0 (patch) | |
tree | 30a850ced9558099f187ec4c8df7c023d41a66d1 | |
parent | 495e948cc300644d9892944fa5ae6ecd67dbe580 (diff) |
git: Move setup of uploadpack.packObjectsHook config entry
The `uploadpack.packObjectsHook` config entry is currently being set up
manually both in the smarthttp and the ssh service. It's thus easy to
get out of sync if it ever needs to change and easy to forget in case we
grow additional callsites where the pack-objects hook needs to be
injected. We've thus started to inject such paths via the hook options
directly.
Let's do the same for `WithPackObjectsHookEnv()` and move setup of the
config entry in there.
-rw-r--r-- | internal/git/hooks_options.go | 6 | ||||
-rw-r--r-- | internal/gitaly/service/smarthttp/upload_pack.go | 17 | ||||
-rw-r--r-- | internal/gitaly/service/ssh/upload_pack.go | 19 |
3 files changed, 23 insertions, 19 deletions
diff --git a/internal/git/hooks_options.go b/internal/git/hooks_options.go index c7cd5f23e..ca8e2e408 100644 --- a/internal/git/hooks_options.go +++ b/internal/git/hooks_options.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "path/filepath" "gitlab.com/gitlab-org/gitaly/internal/git/hooks" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" @@ -57,6 +58,11 @@ func WithPackObjectsHookEnv(ctx context.Context, repo *gitalypb.Repository, cfg fmt.Sprintf("%s=%s", log.GitalyLogDirEnvKey, cfg.Logging.Dir), ) + cc.globals = append(cc.globals, ConfigPair{ + Key: "uploadpack.packObjectsHook", + Value: filepath.Join(cfg.BinDir, "gitaly-hooks"), + }) + return nil } } diff --git a/internal/gitaly/service/smarthttp/upload_pack.go b/internal/gitaly/service/smarthttp/upload_pack.go index 9e4097b67..31c6d964f 100644 --- a/internal/gitaly/service/smarthttp/upload_pack.go +++ b/internal/gitaly/service/smarthttp/upload_pack.go @@ -4,7 +4,6 @@ import ( "crypto/sha1" "fmt" "io" - "path/filepath" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/internal/command" @@ -79,21 +78,21 @@ func (s *server) PostUploadPack(stream gitalypb.SmartHTTPService_PostUploadPackS globalOpts[i] = git.ValueFlag{"-c", o} } + commandOpts := []git.CmdOpt{ + git.WithStdin(stdin), + git.WithStdout(stdout), + git.WithGitProtocol(ctx, req), + } + if featureflag.IsEnabled(ctx, featureflag.UploadPackGitalyHooks) { - hookBin := filepath.Join(s.cfg.BinDir, "gitaly-hooks") - globalOpts = append(globalOpts, git.ValueFlag{"-c", "uploadpack.packObjectsHook=" + hookBin}) + commandOpts = append(commandOpts, git.WithPackObjectsHookEnv(ctx, req.Repository, s.cfg)) } cmd, err := git.NewCommandWithoutRepo(ctx, globalOpts, git.SubCmd{ Name: "upload-pack", Flags: []git.Option{git.Flag{Name: "--stateless-rpc"}}, Args: []string{repoPath}, - }, - git.WithStdin(stdin), - git.WithStdout(stdout), - git.WithGitProtocol(ctx, req), - git.WithPackObjectsHookEnv(ctx, req.Repository, s.cfg), - ) + }, commandOpts...) if err != nil { return status.Errorf(codes.Unavailable, "PostUploadPack: cmd: %v", err) diff --git a/internal/gitaly/service/ssh/upload_pack.go b/internal/gitaly/service/ssh/upload_pack.go index 63b9ba910..a6bd5b9e9 100644 --- a/internal/gitaly/service/ssh/upload_pack.go +++ b/internal/gitaly/service/ssh/upload_pack.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "io" - "path/filepath" "sync" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" @@ -85,11 +84,6 @@ func (s *server) sshUploadPack(stream gitalypb.SSHService_SSHUploadPackServer, r globalOpts[i] = git.ValueFlag{"-c", o} } - if featureflag.IsEnabled(ctx, featureflag.UploadPackGitalyHooks) { - hookBin := filepath.Join(s.cfg.BinDir, "gitaly-hooks") - globalOpts = append(globalOpts, git.ValueFlag{"-c", "uploadpack.packObjectsHook=" + hookBin}) - } - pr, pw := io.Pipe() defer pw.Close() stdin = io.TeeReader(stdin, pw) @@ -110,13 +104,18 @@ func (s *server) sshUploadPack(stream gitalypb.SSHService_SSHUploadPackServer, r stats.UpdateMetrics(s.packfileNegotiationMetrics) }() + commandOpts := []git.CmdOpt{ + git.WithGitProtocol(ctx, req), + } + + if featureflag.IsEnabled(ctx, featureflag.UploadPackGitalyHooks) { + commandOpts = append(commandOpts, git.WithPackObjectsHookEnv(ctx, req.Repository, s.cfg)) + } + cmd, monitor, err := monitorStdinCommand(ctx, s.gitCmdFactory, stdin, stdout, stderr, globalOpts, git.SubCmd{ Name: "upload-pack", Args: []string{repoPath}, - }, - git.WithGitProtocol(ctx, req), - git.WithPackObjectsHookEnv(ctx, req.Repository, s.cfg), - ) + }, commandOpts...) if err != nil { return err |