Merge branch 'jv-dry-alternate-refs' into 'master'

Remove duplication of receive-pack config

See merge request gitlab-org/gitaly!1332

5 files changed, 60 insertions, 43 deletions

diff --git a/changelogs/unreleased/jv-dry-alternate-refs.yml b/changelogs/unreleased/jv-dry-alternate-refs.yml
new file mode 100644
index 000000000..83023f881
--- /dev/null
+++ b/changelogs/unreleased/jv-dry-alternate-refs.yml
@@ -0,0 +1,5 @@
+---
+title: Remove duplication of receive-pack config
+merge_request: 1332
+author:
+type: other
diff --git a/internal/git/receivepack.go b/internal/git/receivepack.go
new file mode 100644
index 000000000..8e1f2e54d
--- /dev/null
+++ b/internal/git/receivepack.go
@@ -0,0 +1,42 @@
+package git
+
+import (
+	"fmt"
+
+	"gitlab.com/gitlab-org/gitaly/internal/config"
+	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
+)
+
+// ReceivePackRequest abstracts away the different requests that end up
+// spawning git-receive-pack.
+type ReceivePackRequest interface {
+	GetGlId() string
+	GetGlUsername() string
+	GetGlRepository() string
+}
+
+// HookEnv is information we pass down to the Git hooks during
+// git-receive-pack.
+func HookEnv(req ReceivePackRequest) []string {
+	return []string{
+		fmt.Sprintf("GL_ID=%s", req.GetGlId()),
+		fmt.Sprintf("GL_USERNAME=%s", req.GetGlUsername()),
+		fmt.Sprintf("GITLAB_SHELL_DIR=%s", config.Config.GitlabShell.Dir),
+		fmt.Sprintf("GL_REPOSITORY=%s", req.GetGlRepository()),
+	}
+}
+
+// ReceivePackConfig contains config options we want to enforce when
+// receiving a push with git-receive-pack.
+func ReceivePackConfig() []string {
+	return []string{
+		fmt.Sprintf("core.hooksPath=%s", hooks.Path()),
+
+		// In case the repository belongs to an object pool, we want to prevent
+		// Git from including the pool's refs in the ref advertisement. We do
+		// this by rigging core.alternateRefsCommand to produce no output.
+		// Because Git itself will append the pool repository directory, the
+		// command ends with a "#". The end result is that Git runs `/bin/sh -c 'exit 0 # /path/to/pool.git`.
+		"core.alternateRefsCommand=exit 0 #",
+	}
+}
diff --git a/internal/service/smarthttp/inforefs.go b/internal/service/smarthttp/inforefs.go
index 7b0018679..dcfb8b94d 100644
--- a/internal/service/smarthttp/inforefs.go
+++ b/internal/service/smarthttp/inforefs.go
@@ -42,14 +42,13 @@ func handleInfoRefs(ctx context.Context, service string, req *gitalypb.InfoRefsR
 		return err
 	}
 
-	// In case the repository belongs to an object pool, we want to prevent
-	// Git from including the pool's refs in the ref advertisement. We do
-	// this by rigging core.alternateRefsCommand to produce no output.
-	// Because Git itself will append the pool repository directory, the
-	// command ends with a "#". The end result is that Git runs
-	// `/bin/sh -c 'exit 0 # /path/to/pool.git`.
-	args := []string{"-c", "core.alternateRefsCommand=exit 0 #"}
-	for _, params := range req.GitConfigOptions {
+	opts := req.GitConfigOptions
+	if service == "receive-pack" {
+		opts = append(git.ReceivePackConfig(), opts...)
+	}
+
+	var args []string
+	for _, params := range opts {
 		args = append(args, "-c", params)
 	}
 
diff --git a/internal/service/smarthttp/receive_pack.go b/internal/service/smarthttp/receive_pack.go
index fd214f98c..75734a9c6 100644
--- a/internal/service/smarthttp/receive_pack.go
+++ b/internal/service/smarthttp/receive_pack.go
@@ -1,15 +1,11 @@
 package smarthttp
 
 import (
-	"fmt"
-
 	grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 	log "github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/internal/command"
-	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
-	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/streamio"
 	"google.golang.org/grpc/codes"
@@ -41,17 +37,8 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac
 	stdout := streamio.NewWriter(func(p []byte) error {
 		return stream.Send(&gitalypb.PostReceivePackResponse{Data: p})
 	})
-	env := []string{
-		fmt.Sprintf("GL_ID=%s", req.GlId),
-		"GL_PROTOCOL=http",
-		fmt.Sprintf("GITLAB_SHELL_DIR=%s", config.Config.GitlabShell.Dir),
-	}
-	if req.GlRepository != "" {
-		env = append(env, fmt.Sprintf("GL_REPOSITORY=%s", req.GlRepository))
-	}
-	if req.GlUsername != "" {
-		env = append(env, fmt.Sprintf("GL_USERNAME=%s", req.GlUsername))
-	}
+
+	env := append(git.HookEnv(req), "GL_PROTOCOL=http")
 
 	repoPath, err := helper.GetRepoPath(req.Repository)
 	if err != nil {
@@ -61,7 +48,7 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac
 	env = git.AddGitProtocolEnv(ctx, req, env)
 	env = append(env, command.GitEnv...)
 
-	opts := append([]string{fmt.Sprintf("core.hooksPath=%s", hooks.Path())}, req.GitConfigOptions...)
+	opts := append(git.ReceivePackConfig(), req.GitConfigOptions...)
 
 	gitOptions := git.BuildGitOptions(opts, "receive-pack", "--stateless-rpc", repoPath)
 	cmd, err := git.BareCommand(ctx, stdin, stdout, nil, env, gitOptions...)
diff --git a/internal/service/ssh/receive_pack.go b/internal/service/ssh/receive_pack.go
index e8224808c..2deaa62b7 100644
--- a/internal/service/ssh/receive_pack.go
+++ b/internal/service/ssh/receive_pack.go
@@ -7,9 +7,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/internal/command"
-	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
-	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/streamio"
 )
@@ -51,15 +49,8 @@ func sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer, req *gitaly
 	stderr := streamio.NewWriter(func(p []byte) error {
 		return stream.Send(&gitalypb.SSHReceivePackResponse{Stderr: p})
 	})
-	env := []string{
-		fmt.Sprintf("GL_ID=%s", req.GlId),
-		fmt.Sprintf("GL_USERNAME=%s", req.GlUsername),
-		"GL_PROTOCOL=ssh",
-		fmt.Sprintf("GITLAB_SHELL_DIR=%s", config.Config.GitlabShell.Dir),
-	}
-	if req.GlRepository != "" {
-		env = append(env, fmt.Sprintf("GL_REPOSITORY=%s", req.GlRepository))
-	}
+
+	env := append(git.HookEnv(req), "GL_PROTOCOL=ssh")
 
 	repoPath, err := helper.GetRepoPath(req.Repository)
 	if err != nil {
@@ -69,15 +60,8 @@ func sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer, req *gitaly
 	env = git.AddGitProtocolEnv(ctx, req, env)
 	env = append(env, command.GitEnv...)
 
-	opts := append([]string{fmt.Sprintf("core.hooksPath=%s", hooks.Path())}, req.GitConfigOptions...)
+	opts := append(git.ReceivePackConfig(), req.GitConfigOptions...)
 
-	// In case the repository belongs to an object pool, we want to prevent
-	// Git from including the pool's refs in the ref advertisement. We do
-	// this by rigging core.alternateRefsCommand to produce no output.
-	// Because Git itself will append the pool repository directory, the
-	// command ends with a "#". The end result is that Git runs
-	// `/bin/sh -c 'exit 0 # /path/to/pool.git`.
-	opts = append(opts, "core.alternateRefsCommand=exit 0 #")
 	gitOptions := git.BuildGitOptions(opts, "receive-pack", repoPath)
 	cmd, err := git.BareCommand(ctx, stdin, stdout, stderr, env, gitOptions...)