gitlabshell: Remove `Env()` in favor of `EnvFromConfig()`

It's not at all obvious what the `gitlabshell.Env()` derives from as it
doesn't receive any input parameters. Turns out, it's using values from
Gitaly's configuration and puts those into the environment in order for
any called executable to be able to figure out what's what. Most
importantly, this will include the Gitaly binary directory, but also its
socket path.

Let's make this a bit more explicit by converting all users to
`gitlabshell.EnvFromConfig()`. This also gets rid of one more location
which implicitly used the global configuration.

9 files changed, 15 insertions, 22 deletions

diff --git a/internal/git/hooks.go b/internal/git/hooks.go
index 8716ffc7b..4a51281a8 100644
--- a/internal/git/hooks.go
+++ b/internal/git/hooks.go
@@ -63,9 +63,9 @@ type ReceivePackRequest interface {
 // WithReceivePackHooks returns an option that populates the safe command with the environment
 // variables necessary to properly execute the pre-receive, update and post-receive hooks for
 // git-receive-pack(1).
-func WithReceivePackHooks(ctx context.Context, req ReceivePackRequest, protocol string) CmdOpt {
+func WithReceivePackHooks(ctx context.Context, cfg config.Cfg, req ReceivePackRequest, protocol string) CmdOpt {
 	return func(cc *cmdCfg) error {
-		env, err := receivePackHookEnv(ctx, req, protocol)
+		env, err := receivePackHookEnv(ctx, cfg, req, protocol)
 		if err != nil {
 			return fmt.Errorf("receive-pack hook envvars: %w", err)
 		}
@@ -75,8 +75,8 @@ func WithReceivePackHooks(ctx context.Context, req ReceivePackRequest, protocol
 	}
 }
 
-func receivePackHookEnv(ctx context.Context, req ReceivePackRequest, protocol string) ([]string, error) {
-	gitlabshellEnv, err := gitlabshell.Env()
+func receivePackHookEnv(ctx context.Context, cfg config.Cfg, req ReceivePackRequest, protocol string) ([]string, error) {
+	gitlabshellEnv, err := gitlabshell.EnvFromConfig(cfg)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/gitaly/rubyserver/rubyserver.go b/internal/gitaly/rubyserver/rubyserver.go
index 1d7542859..40c3a6b30 100644
--- a/internal/gitaly/rubyserver/rubyserver.go
+++ b/internal/gitaly/rubyserver/rubyserver.go
@@ -78,7 +78,7 @@ func (s *Server) start() error {
 
 	cfg := config.Config
 
-	gitlabshellEnv, err := gitlabshell.Env()
+	gitlabshellEnv, err := gitlabshell.EnvFromConfig(cfg)
 	if err != nil {
 		return err
 	}
diff --git a/internal/gitaly/service/hook/post_receive.go b/internal/gitaly/service/hook/post_receive.go
index fb2d0ee92..546e987e5 100644
--- a/internal/gitaly/service/hook/post_receive.go
+++ b/internal/gitaly/service/hook/post_receive.go
@@ -45,7 +45,7 @@ func (s *server) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServ
 		return stream.Send(&gitalypb.PostReceiveHookResponse{Stderr: p})
 	})
 
-	env, err := hookRequestEnv(firstRequest)
+	env, err := s.hookRequestEnv(firstRequest)
 	if err != nil {
 		return helper.ErrInternal(err)
 	}
diff --git a/internal/gitaly/service/hook/pre_receive.go b/internal/gitaly/service/hook/pre_receive.go
index d5708e3d8..6e1ebaaf6 100644
--- a/internal/gitaly/service/hook/pre_receive.go
+++ b/internal/gitaly/service/hook/pre_receive.go
@@ -23,16 +23,16 @@ type prePostRequest interface {
 	GetGitPushOptions() []string
 }
 
-func hookRequestEnv(req hookRequest) ([]string, error) {
-	gitlabshellEnv, err := gitlabshell.Env()
+func (s *server) hookRequestEnv(req hookRequest) ([]string, error) {
+	gitlabshellEnv, err := gitlabshell.EnvFromConfig(s.cfg)
 	if err != nil {
 		return nil, err
 	}
 	return append(gitlabshellEnv, req.GetEnvironmentVariables()...), nil
 }
 
-func preReceiveEnv(req prePostRequest) ([]string, error) {
-	env, err := hookRequestEnv(req)
+func (s *server) preReceiveEnv(req prePostRequest) ([]string, error) {
+	env, err := s.hookRequestEnv(req)
 	if err != nil {
 		return nil, err
 	}
@@ -67,7 +67,7 @@ func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer
 		return stream.Send(&gitalypb.PreReceiveHookResponse{Stderr: p})
 	})
 
-	env, err := preReceiveEnv(firstRequest)
+	env, err := s.preReceiveEnv(firstRequest)
 	if err != nil {
 		return fmt.Errorf("getting env vars from request: %v", err)
 	}
diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go
index 73ce55c95..466f7fdaa 100644
--- a/internal/gitaly/service/operations/merge.go
+++ b/internal/gitaly/service/operations/merge.go
@@ -110,7 +110,7 @@ func validateMergeBranchRequest(request *gitalypb.UserMergeBranchRequest) error
 }
 
 func (s *server) updateReferenceWithHooks(ctx context.Context, repo *gitalypb.Repository, user *gitalypb.User, reference, newrev, oldrev string) error {
-	gitlabshellEnv, err := gitlabshell.Env()
+	gitlabshellEnv, err := gitlabshell.EnvFromConfig(s.cfg)
 	if err != nil {
 		return err
 	}
diff --git a/internal/gitaly/service/smarthttp/receive_pack.go b/internal/gitaly/service/smarthttp/receive_pack.go
index f1187a2b3..49db0d394 100644
--- a/internal/gitaly/service/smarthttp/receive_pack.go
+++ b/internal/gitaly/service/smarthttp/receive_pack.go
@@ -54,7 +54,7 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac
 			Flags: []git.Option{git.Flag{Name: "--stateless-rpc"}},
 			Args:  []string{repoPath},
 		},
-		git.WithReceivePackHooks(ctx, req, "http"),
+		git.WithReceivePackHooks(ctx, config.Config, req, "http"),
 		git.WithGitProtocol(ctx, req),
 		git.WithRefTxHook(ctx, req.Repository, config.Config),
 	)
diff --git a/internal/gitaly/service/ssh/receive_pack.go b/internal/gitaly/service/ssh/receive_pack.go
index d949eeb87..75247a4fc 100644
--- a/internal/gitaly/service/ssh/receive_pack.go
+++ b/internal/gitaly/service/ssh/receive_pack.go
@@ -71,7 +71,7 @@ func (s *server) sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer,
 			Name: "receive-pack",
 			Args: []string{repoPath},
 		},
-		git.WithReceivePackHooks(ctx, req, "ssh"),
+		git.WithReceivePackHooks(ctx, config.Config, req, "ssh"),
 		git.WithGitProtocol(ctx, req),
 		git.WithRefTxHook(ctx, req.Repository, config.Config),
 	)
diff --git a/internal/gitlabshell/env.go b/internal/gitlabshell/env.go
index 6a859c967..99091af0b 100644
--- a/internal/gitlabshell/env.go
+++ b/internal/gitlabshell/env.go
@@ -8,13 +8,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/log"
 )
 
-// Env is a helper that returns a slice with environment variables used by gitlab shell
-func Env() ([]string, error) {
-	cfg := config.Config
-
-	return EnvFromConfig(cfg)
-}
-
 type Config struct {
 	CustomHooksDir string              `json:"custom_hooks_dir"`
 	GitlabURL      string              `json:"gitlab_url"`
diff --git a/internal/gitlabshell/env_test.go b/internal/gitlabshell/env_test.go
index ff5627a4a..45ec94c3c 100644
--- a/internal/gitlabshell/env_test.go
+++ b/internal/gitlabshell/env_test.go
@@ -42,7 +42,7 @@ func TestGitHooksConfig(t *testing.T) {
 		SecretFile: "secret_file_path",
 	}
 
-	env, err := gitlabshell.Env()
+	env, err := gitlabshell.EnvFromConfig(config.Config)
 	require.NoError(t, err)
 
 	require.Contains(t, env, "GITALY_GITLAB_SHELL_DIR="+config.Config.GitlabShell.Dir)