10 files changed, 20 insertions, 181 deletions
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index 47c73425a..c0efd9cfa 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -21,7 +21,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	gitalyhook "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/service/hook"
-	"gitlab.com/gitlab-org/gitaly/internal/gitlabshell"
 	gitalylog "gitlab.com/gitlab-org/gitaly/internal/log"
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/metadata"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
@@ -40,15 +39,12 @@ type proxyValues struct {
 
 // envForHooks generates a set of environment variables for gitaly hooks
 func envForHooks(t testing.TB, gitlabShellDir string, repo *gitalypb.Repository, glHookValues glHookValues, proxyValues proxyValues, gitPushOptions ...string) []string {
-	env, err := gitlabshell.EnvFromConfig(config.Config)
-	require.NoError(t, err)
-
 	payload, err := git.NewHooksPayload(config.Config, repo, nil, nil).Env()
 	require.NoError(t, err)
 
-	env = append(env, os.Environ()...)
-	env = append(env, []string{
+	env := append(os.Environ(), []string{
 		payload,
+		"GITALY_BIN_DIR=" + config.Config.BinDir,
 		fmt.Sprintf("GL_ID=%s", glHookValues.GLID),
 		fmt.Sprintf("GL_REPOSITORY=%s", glHookValues.GLRepo),
 		fmt.Sprintf("GL_PROTOCOL=%s", glHookValues.GLProtocol),
diff --git a/internal/git/hooks_options.go b/internal/git/hooks_options.go
index a091351d1..10d5df9a7 100644
--- a/internal/git/hooks_options.go
+++ b/internal/git/hooks_options.go
@@ -7,7 +7,7 @@ import (
 
 	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/internal/gitlabshell"
+	"gitlab.com/gitlab-org/gitaly/internal/log"
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/metadata"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
@@ -47,7 +47,12 @@ func (cc *cmdCfg) configureHooks(ctx context.Context, repo *gitalypb.Repository,
 		return err
 	}
 
-	cc.env = append(cc.env, payload, "GITALY_BIN_DIR="+cfg.BinDir)
+	cc.env = append(
+		cc.env,
+		payload,
+		"GITALY_BIN_DIR="+cfg.BinDir,
+		fmt.Sprintf("%s=%s", log.GitalyLogDirEnvKey, cfg.Logging.Dir),
+	)
 
 	cc.globals = append(cc.globals, ValueFlag{"-c", fmt.Sprintf("core.hooksPath=%s", hooks.Path(cfg))})
 	cc.hooksConfigured = true
@@ -73,7 +78,7 @@ func WithReceivePackHooks(ctx context.Context, cfg config.Cfg, req ReceivePackRe
 			return err
 		}
 
-		env, err := receivePackHookEnv(ctx, cc, cfg, req, protocol)
+		env, err := receivePackHookEnv(req, protocol)
 		if err != nil {
 			return fmt.Errorf("receive-pack hook envvars: %w", err)
 		}
@@ -83,19 +88,12 @@ func WithReceivePackHooks(ctx context.Context, cfg config.Cfg, req ReceivePackRe
 	}
 }
 
-func receivePackHookEnv(ctx context.Context, cc *cmdCfg, cfg config.Cfg, req ReceivePackRequest, protocol string) ([]string, error) {
-	gitlabshellEnv, err := gitlabshell.EnvFromConfig(cfg)
-	if err != nil {
-		return nil, err
-	}
-
-	env := append(gitlabshellEnv,
+func receivePackHookEnv(req ReceivePackRequest, protocol string) ([]string, error) {
+	return []string{
 		fmt.Sprintf("GL_ID=%s", req.GetGlId()),
 		fmt.Sprintf("GL_USERNAME=%s", req.GetGlUsername()),
 		fmt.Sprintf("GL_REPOSITORY=%s", req.GetGlRepository()),
 		fmt.Sprintf("GL_PROJECT_PATH=%s", req.GetRepository().GetGlProjectPath()),
 		fmt.Sprintf("GL_PROTOCOL=%s", protocol),
-	)
-
-	return env, nil
+	}, nil
 }
diff --git a/internal/git/hooks_options_test.go b/internal/git/hooks_options_test.go
index 054fb6078..4117d4c1e 100644
--- a/internal/git/hooks_options_test.go
+++ b/internal/git/hooks_options_test.go
@@ -69,6 +69,7 @@ func TestWithRefHook(t *testing.T) {
 			require.EqualValues(t, []string{
 				"GITALY_HOOKS_PAYLOAD",
 				"GITALY_BIN_DIR",
+				"GITALY_LOG_DIR",
 			}, actualEnvVars)
 		})
 	}
diff --git a/internal/gitaly/rubyserver/rubyserver.go b/internal/gitaly/rubyserver/rubyserver.go
index f4ef92345..b64b381bc 100644
--- a/internal/gitaly/rubyserver/rubyserver.go
+++ b/internal/gitaly/rubyserver/rubyserver.go
@@ -16,7 +16,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver/balancer"
-	"gitlab.com/gitlab-org/gitaly/internal/gitlabshell"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/supervisor"
 	"gitlab.com/gitlab-org/gitaly/internal/version"
@@ -78,11 +77,6 @@ func (s *Server) start() error {
 
 	cfg := config.Config
 
-	gitlabshellEnv, err := gitlabshell.EnvFromConfig(cfg)
-	if err != nil {
-		return err
-	}
-
 	env := append(
 		os.Environ(),
 		"GITALY_RUBY_GIT_BIN_PATH="+cfg.Git.BinPath,
@@ -93,9 +87,8 @@ func (s *Server) start() error {
 		"GITALY_GIT_HOOKS_DIR="+hooks.Path(cfg),
 		"GITALY_SOCKET="+cfg.GitalyInternalSocketPath(),
 		"GITALY_TOKEN="+cfg.Auth.Token,
-		"GITALY_RUGGED_GIT_CONFIG_SEARCH_PATH="+cfg.Ruby.RuggedGitConfigSearchPath)
-	env = append(env, gitlabshellEnv...)
-
+		"GITALY_RUGGED_GIT_CONFIG_SEARCH_PATH="+cfg.Ruby.RuggedGitConfigSearchPath,
+	)
 	env = append(env, command.GitEnv...)
 
 	if dsn := cfg.Logging.RubySentryDSN; dsn != "" {
diff --git a/internal/gitaly/service/hook/post_receive.go b/internal/gitaly/service/hook/post_receive.go
index 546e987e5..957949280 100644
--- a/internal/gitaly/service/hook/post_receive.go
+++ b/internal/gitaly/service/hook/post_receive.go
@@ -45,16 +45,11 @@ func (s *server) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServ
 		return stream.Send(&gitalypb.PostReceiveHookResponse{Stderr: p})
 	})
 
-	env, err := s.hookRequestEnv(firstRequest)
-	if err != nil {
-		return helper.ErrInternal(err)
-	}
-
 	if err := s.manager.PostReceiveHook(
 		stream.Context(),
 		firstRequest.Repository,
 		firstRequest.GetGitPushOptions(),
-		env,
+		firstRequest.GetEnvironmentVariables(),
 		stdin,
 		stdout,
 		stderr,
diff --git a/internal/gitaly/service/hook/pre_receive.go b/internal/gitaly/service/hook/pre_receive.go
index 8efe327a7..56d68e109 100644
--- a/internal/gitaly/service/hook/pre_receive.go
+++ b/internal/gitaly/service/hook/pre_receive.go
@@ -7,24 +7,11 @@ import (
 	"sync"
 
 	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
-	"gitlab.com/gitlab-org/gitaly/internal/gitlabshell"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/streamio"
 )
 
-type hookRequest interface {
-	GetEnvironmentVariables() []string
-}
-
-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 (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer) error {
 	firstRequest, err := stream.Recv()
 	if err != nil {
@@ -49,11 +36,7 @@ func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer
 		return stream.Send(&gitalypb.PreReceiveHookResponse{Stderr: p})
 	})
 
-	env, err := s.hookRequestEnv(firstRequest)
-	if err != nil {
-		return fmt.Errorf("getting env vars from request: %v", err)
-	}
-	env = append(env, hooks.GitPushOptions(firstRequest.GetGitPushOptions())...)
+	env := append(firstRequest.GetEnvironmentVariables(), hooks.GitPushOptions(firstRequest.GetGitPushOptions())...)
 
 	if err := s.manager.PreReceiveHook(
 		stream.Context(),
diff --git a/internal/gitaly/service/operations/update_with_hooks.go b/internal/gitaly/service/operations/update_with_hooks.go
index 7a7537a6a..134466236 100644
--- a/internal/gitaly/service/operations/update_with_hooks.go
+++ b/internal/gitaly/service/operations/update_with_hooks.go
@@ -9,7 +9,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/hook"
-	"gitlab.com/gitlab-org/gitaly/internal/gitlabshell"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/metadata"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
@@ -32,11 +31,6 @@ func (e updateRefError) Error() string {
 }
 
 func (s *Server) updateReferenceWithHooks(ctx context.Context, repo *gitalypb.Repository, user *gitalypb.User, reference, newrev, oldrev string) error {
-	gitlabshellEnv, err := gitlabshell.EnvFromConfig(s.cfg)
-	if err != nil {
-		return err
-	}
-
 	transaction, praefect, err := metadata.TransactionMetadataFromContext(ctx)
 	if err != nil {
 		return err
@@ -57,14 +51,14 @@ func (s *Server) updateReferenceWithHooks(ctx context.Context, repo *gitalypb.Re
 		return helper.ErrInternalf("updateReferenceWithHooks: got invalid new value: %w", err)
 	}
 
-	env := append([]string{
+	env := []string{
 		payload,
 		"GL_PROTOCOL=web",
 		fmt.Sprintf("GL_ID=%s", user.GetGlId()),
 		fmt.Sprintf("GL_USERNAME=%s", user.GetGlUsername()),
 		fmt.Sprintf("GL_REPOSITORY=%s", repo.GetGlRepository()),
 		fmt.Sprintf("GL_PROJECT_PATH=%s", repo.GetGlProjectPath()),
-	}, gitlabshellEnv...)
+	}
 
 	changes := fmt.Sprintf("%s %s %s\n", oldrev, newrev, reference)
 	var stdout, stderr bytes.Buffer
diff --git a/internal/gitaly/service/operations/update_with_hooks_test.go b/internal/gitaly/service/operations/update_with_hooks_test.go
index 3b34707d5..ecd480e90 100644
--- a/internal/gitaly/service/operations/update_with_hooks_test.go
+++ b/internal/gitaly/service/operations/update_with_hooks_test.go
@@ -136,7 +136,6 @@ func TestUpdateReferenceWithHooks(t *testing.T) {
 
 	expectedEnv := []string{
 		payload,
-		"GITALY_BIN_DIR=" + config.Config.BinDir,
 		"GL_ID=1234",
 		"GL_PROJECT_PATH=gitlab-org/gitlab-test",
 		"GL_PROTOCOL=web",
diff --git a/internal/gitlabshell/env.go b/internal/gitlabshell/env.go
deleted file mode 100644
index f337af58b..000000000
--- a/internal/gitlabshell/env.go
+++ /dev/null
@@ -1,43 +0,0 @@
-package gitlabshell
-
-import (
-	"encoding/json"
-	"fmt"
-
-	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/internal/log"
-)
-
-type Config struct {
-	CustomHooksDir string              `json:"custom_hooks_dir"`
-	GitlabURL      string              `json:"gitlab_url"`
-	HTTPSettings   config.HTTPSettings `json:"http_settings"`
-	LogFormat      string              `json:"log_format"`
-	LogLevel       string              `json:"log_level"`
-	LogPath        string              `json:"log_path"`
-	SecretFile     string              `json:"secret_file"`
-}
-
-// EnvFromConfig returns a set of environment variables from a config struct relevant to gitlab shell
-func EnvFromConfig(cfg config.Cfg) ([]string, error) {
-	gitlabShellConfig := Config{
-		CustomHooksDir: cfg.Hooks.CustomHooksDir,
-		GitlabURL:      cfg.Gitlab.URL,
-		HTTPSettings:   cfg.Gitlab.HTTPSettings,
-		LogFormat:      cfg.Logging.Format,
-		LogLevel:       cfg.Logging.Level,
-		LogPath:        cfg.Logging.Dir,
-		SecretFile:     cfg.Gitlab.SecretFile,
-	}
-
-	gitlabShellConfigString, err := json.Marshal(&gitlabShellConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	return []string{
-		fmt.Sprintf("%s=%s", log.GitalyLogDirEnvKey, cfg.Logging.Dir),
-		"GITALY_BIN_DIR=" + cfg.BinDir,
-		"GITALY_GITLAB_SHELL_CONFIG=" + string(gitlabShellConfigString),
-	}, nil
-}
diff --git a/internal/gitlabshell/env_test.go b/internal/gitlabshell/env_test.go
deleted file mode 100644
index b4357c9f2..000000000
--- a/internal/gitlabshell/env_test.go
+++ /dev/null
@@ -1,77 +0,0 @@
-package gitlabshell_test
-
-import (
-	"encoding/json"
-	"io/ioutil"
-	"os"
-	"strings"
-	"testing"
-
-	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/internal/gitlabshell"
-	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
-)
-
-func TestGitHooksConfig(t *testing.T) {
-	defer func(cfg config.Cfg) {
-		config.Config = cfg
-	}(config.Config)
-
-	require.NoError(t, testhelper.ConfigureRuby(&config.Config))
-
-	loggingDir, err := ioutil.TempDir("", t.Name())
-	require.NoError(t, err)
-	defer func() { os.RemoveAll(loggingDir) }()
-
-	config.Config.Logging.Dir = loggingDir
-	config.Config.Logging.Level = "fatal"
-	config.Config.Logging.Format = "my-custom-format"
-	config.Config.GitlabShell.Dir = "../../ruby/gitlab-shell"
-	config.Config.Hooks.CustomHooksDir = "/path/to/custom_hooks"
-	config.Config.Gitlab = config.Gitlab{
-		URL: "http://gitlaburl.com",
-		HTTPSettings: config.HTTPSettings{
-			ReadTimeout: 100,
-			User:        "user_name",
-			Password:    "pwpw",
-			CAFile:      "/ca_file_path",
-			CAPath:      "/ca_path",
-			SelfSigned:  true,
-		},
-		SecretFile: "secret_file_path",
-	}
-
-	env, err := gitlabshell.EnvFromConfig(config.Config)
-	require.NoError(t, err)
-
-	require.Contains(t, env, "GITALY_LOG_DIR="+config.Config.Logging.Dir)
-	require.Contains(t, env, "GITALY_BIN_DIR="+config.Config.BinDir)
-
-	jsonShellConfig := ""
-	for _, envVar := range env {
-		if strings.HasPrefix(envVar, "GITALY_GITLAB_SHELL_CONFIG=") {
-			jsonShellConfig = strings.SplitN(envVar, "=", 2)[1]
-			break
-		}
-	}
-
-	var configMap map[string]interface{}
-
-	require.NoError(t, json.Unmarshal([]byte(jsonShellConfig), &configMap))
-	require.Equal(t, config.Config.Logging.Level, configMap["log_level"])
-	require.Equal(t, config.Config.Logging.Format, configMap["log_format"])
-	require.Equal(t, config.Config.Gitlab.SecretFile, configMap["secret_file"])
-	require.Equal(t, config.Config.Hooks.CustomHooksDir, configMap["custom_hooks_dir"])
-	require.Equal(t, config.Config.Gitlab.URL, configMap["gitlab_url"])
-
-	// HTTP Settings
-	httpSettings, ok := configMap["http_settings"].(map[string]interface{})
-	require.True(t, ok)
-	require.Equal(t, float64(config.Config.Gitlab.HTTPSettings.ReadTimeout), httpSettings["read_timeout"])
-	require.Equal(t, config.Config.Gitlab.HTTPSettings.User, httpSettings["user"])
-	require.Equal(t, config.Config.Gitlab.HTTPSettings.Password, httpSettings["password"])
-	require.Equal(t, config.Config.Gitlab.HTTPSettings.CAFile, httpSettings["ca_file"])
-	require.Equal(t, config.Config.Gitlab.HTTPSettings.CAPath, httpSettings["ca_path"])
-	require.Equal(t, config.Config.Gitlab.HTTPSettings.SelfSigned, httpSettings["self_signed_cert"])
-}