Merge branch 'jv-prepare-hooks' into 'master'

Prepare for vendoring gitlab-shell hooks

See merge request gitlab-org/gitaly!1020

22 files changed, 264 insertions, 93 deletions

diff --git a/_support/makegen.go b/_support/makegen.go
index 202357cc7..2a6a83380 100644
--- a/_support/makegen.go
+++ b/_support/makegen.go
@@ -69,13 +69,18 @@ func (gm *gitalyMake) BuildDir() string {
 	return gm.cwd
 }
 
-func (gm *gitalyMake) Pkg() string         { return "gitlab.com/gitlab-org/gitaly" }
-func (gm *gitalyMake) GoImports() string   { return "bin/goimports" }
-func (gm *gitalyMake) GoCovMerge() string  { return "bin/gocovmerge" }
-func (gm *gitalyMake) GoLint() string      { return "bin/golint" }
-func (gm *gitalyMake) GoVendor() string    { return "bin/govendor" }
-func (gm *gitalyMake) MegaCheck() string   { return "bin/megacheck" }
-func (gm *gitalyMake) CoverageDir() string { return filepath.Join(gm.BuildDir(), "cover") }
+func (gm *gitalyMake) Pkg() string               { return "gitlab.com/gitlab-org/gitaly" }
+func (gm *gitalyMake) GoImports() string         { return "bin/goimports" }
+func (gm *gitalyMake) GoCovMerge() string        { return "bin/gocovmerge" }
+func (gm *gitalyMake) GoLint() string            { return "bin/golint" }
+func (gm *gitalyMake) GoVendor() string          { return "bin/govendor" }
+func (gm *gitalyMake) MegaCheck() string         { return "bin/megacheck" }
+func (gm *gitalyMake) CoverageDir() string       { return filepath.Join(gm.BuildDir(), "cover") }
+func (gm *gitalyMake) GitalyRubyDir() string     { return filepath.Join(gm.SourceDir(), "ruby") }
+func (gm *gitalyMake) GitlabShellRelDir() string { return "ruby/vendor/gitlab-shell" }
+func (gm *gitalyMake) GitlabShellDir() string {
+	return filepath.Join(gm.SourceDir(), gm.GitlabShellRelDir())
+}
 
 // SourceDir is the location of gitaly's files, inside the _build GOPATH.
 func (gm *gitalyMake) SourceDir() string { return filepath.Join(gm.BuildDir(), "src", gm.Pkg()) }
@@ -243,10 +248,10 @@ build: ../.ruby-bundle
 # This file is used by Omnibus and CNG to skip the "bundle install"
 # step. Both Omnibus and CNG assume it is in the Gitaly root, not in
 # _build. Hence the '../' in front.
-../.ruby-bundle: {{ .SourceDir }}/ruby/Gemfile.lock {{ .SourceDir }}/ruby/Gemfile
-	cd {{ .SourceDir }}/ruby && bundle config # for debugging
-	cd {{ .SourceDir }}/ruby && bundle install $(BUNDLE_FLAGS)
-	cd {{ .SourceDir }}/ruby && bundle show gitaly-proto # sanity check
+../.ruby-bundle:  {{ .GitalyRubyDir }}/Gemfile.lock  {{ .GitalyRubyDir }}/Gemfile
+	cd  {{ .GitalyRubyDir }} && bundle config # for debugging
+	cd  {{ .GitalyRubyDir }} && bundle install $(BUNDLE_FLAGS)
+	cd  {{ .GitalyRubyDir }} && bundle show gitaly-proto # sanity check
 	touch $@
 
 .PHONY: install
@@ -277,9 +282,9 @@ assemble-go: build
 assemble-ruby:
 	rm -rf $(ASSEMBLY_ROOT)/ruby
 	mkdir -p $(ASSEMBLY_ROOT)
-	rm -rf {{ .SourceDir }}/ruby/tmp
-	cp -r {{ .SourceDir }}/ruby $(ASSEMBLY_ROOT)/ruby
-	rm -rf $(ASSEMBLY_ROOT)/ruby/spec
+	rm -rf {{ .GitalyRubyDir }}/tmp {{ .GitlabShellDir }}/tmp 
+	cp -r  {{ .GitalyRubyDir }} $(ASSEMBLY_ROOT)/ruby
+	rm -rf $(ASSEMBLY_ROOT)/ruby/spec $(ASSEMBLY_ROOT)/{{ .GitlabShellRelDir }}/spec $(ASSEMBLY_ROOT)/{{ .GitlabShellRelDir }}/gitlab-shell.log
 
 binaries: assemble
 	@if [ $$(uname -m) != 'x86_64' ]; then echo Incorrect architecture for build: $(uname -m); exit 1; fi
@@ -317,7 +322,7 @@ race-go: prepare-tests
 
 .PHONY: rspec
 rspec: assemble-go prepare-tests
-	cd {{ .SourceDir }}/ruby && bundle exec rspec
+	cd  {{ .GitalyRubyDir }} && bundle exec rspec
 
 .PHONY: verify
 verify: lint check-formatting megacheck govendor-status notice-up-to-date govendor-tagged rubocop
@@ -381,7 +386,7 @@ govendor-tagged: {{ .GoVendor }}
 
 .PHONY: rubocop
 rubocop: ../.ruby-bundle
-	cd {{ .SourceDir }}/ruby && bundle exec rubocop --parallel
+	cd  {{ .GitalyRubyDir }} && bundle exec rubocop --parallel
 
 .PHONY: cover
 cover: prepare-tests {{ .GoCovMerge }}
@@ -403,8 +408,8 @@ cover: prepare-tests {{ .GoCovMerge }}
 docker:
 	rm -rf docker/
 	mkdir -p docker/bin/
-	rm -rf {{ .SourceDir }}/ruby/tmp
-	cp -r {{ .SourceDir }}/ruby docker/ruby
+	rm -rf  {{ .GitalyRubyDir }}/tmp
+	cp -r  {{ .GitalyRubyDir }} docker/ruby
 	rm -rf docker/ruby/vendor/bundle
 {{ $pkg := .Pkg }}
 {{ $goLdFlags := .GoLdFlags }}
diff --git a/_support/vendor-gitlab-shell b/_support/vendor-gitlab-shell
new file mode 100755
index 000000000..c5fdbc99f
--- /dev/null
+++ b/_support/vendor-gitlab-shell
@@ -0,0 +1,41 @@
+#!/usr/bin/env ruby
+
+require 'tempfile'
+
+require_relative 'run'
+
+REMOTE = 'https://gitlab.com/gitlab-org/gitlab-shell.git'
+REMOVE_PATHS = %w[go support spec/action .git spec/gitlab_shell_gitlab_shell_spec.rb].freeze
+
+def main
+  Dir.mktmpdir do |dir|
+    run!(%W[git clone --depth=1 --quiet #{REMOTE} gitlab-shell], dir)
+    tmp_shell_dir = File.join(dir, 'gitlab-shell')
+
+    run!(%w[mv README.md README.orig.md], tmp_shell_dir)
+
+    revision = capture!(%w[git rev-parse HEAD], tmp_shell_dir).chomp
+    remote_project = REMOTE.sub(/\.git$/, '')
+
+    readme = <<-EOS
+# gitlab-shell
+
+Vendored from #{remote_project} at [#{revision}](#{remote_project}/commit/#{revision}).
+
+Original README: [README.orig.md](README.orig.md).
+EOS
+    File.write(File.join(tmp_shell_dir, 'README.md'), readme)
+
+    run!(%w[rm -rf --] + REMOVE_PATHS, tmp_shell_dir)
+
+    gitlab_init = File.join(tmp_shell_dir, 'lib/gitlab_init.rb')
+    gitlab_init_contents = File.read(gitlab_init)
+    File.write(gitlab_init, gitlab_init_contents.sub(/^GITALY_EMBEDDED =.*/, 'GITALY_EMBEDDED = true'))
+
+    shell_vendor_dir = 'ruby/vendor/gitlab-shell'
+    run!(%W[mkdir -p #{shell_vendor_dir}])
+    run!(%W[rsync -av --delete #{tmp_shell_dir}/ #{shell_vendor_dir}/])
+  end
+end
+
+main
diff --git a/changelogs/unreleased/jv-prepare-hooks.yml b/changelogs/unreleased/jv-prepare-hooks.yml
new file mode 100644
index 000000000..a2f4b9051
--- /dev/null
+++ b/changelogs/unreleased/jv-prepare-hooks.yml
@@ -0,0 +1,5 @@
+---
+title: Prepare for vendoring gitlab-shell hooks
+merge_request: 1020
+author:
+type: other
diff --git a/internal/git/hooks/hooks.go b/internal/git/hooks/hooks.go
index 63e105330..9357f0fe5 100644
--- a/internal/git/hooks/hooks.go
+++ b/internal/git/hooks/hooks.go
@@ -1,12 +1,27 @@
 package hooks
 
 import (
+	"os"
 	"path"
 
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 )
 
-// Path returns the path where the global git hooks are located
+// Override allows tests to control where the hooks directory is.
+var Override string
+
+// Path returns the path where the global git hooks are located. As a
+// transitional mechanism, GITALY_USE_EMBEDDED_HOOKS=1 will cause
+// Gitaly's embedded hooks to be used instead of the legacy gitlab-shell
+// hooks.
 func Path() string {
+	if len(Override) > 0 {
+		return Override
+	}
+
+	if os.Getenv("GITALY_USE_EMBEDDED_HOOKS") == "1" {
+		return path.Join(config.Config.Ruby.Dir, "git-hooks")
+	}
+
 	return path.Join(config.Config.GitlabShell.Dir, "hooks")
 }
diff --git a/internal/git/hooks/hooks_test.go b/internal/git/hooks/hooks_test.go
index 9a8088de9..655366b7d 100644
--- a/internal/git/hooks/hooks_test.go
+++ b/internal/git/hooks/hooks_test.go
@@ -1,12 +1,40 @@
 package hooks
 
 import (
-	"strings"
+	"fmt"
+	"os"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/config"
 )
 
 func TestPath(t *testing.T) {
-	assert.True(t, strings.HasSuffix(Path(), "hooks"))
+	defer func(rubyDir, shellDir string) {
+		config.Config.Ruby.Dir = rubyDir
+		config.Config.GitlabShell.Dir = shellDir
+	}(config.Config.Ruby.Dir, config.Config.GitlabShell.Dir)
+	config.Config.Ruby.Dir = "/bazqux/gitaly-ruby"
+	config.Config.GitlabShell.Dir = "/foobar/gitlab-shell"
+
+	hooksVar := "GITALY_USE_EMBEDDED_HOOKS"
+	t.Run(fmt.Sprintf("with %s=1", hooksVar), func(t *testing.T) {
+		os.Setenv(hooksVar, "1")
+		defer os.Unsetenv(hooksVar)
+
+		require.Equal(t, "/bazqux/gitaly-ruby/git-hooks", Path())
+	})
+
+	t.Run(fmt.Sprintf("without %s=1", hooksVar), func(t *testing.T) {
+		os.Unsetenv(hooksVar)
+
+		require.Equal(t, "/foobar/gitlab-shell/hooks", Path())
+	})
+
+	t.Run("with an override", func(t *testing.T) {
+		Override = "/override/hooks"
+		defer func() { Override = "" }()
+
+		require.Equal(t, "/override/hooks", Path())
+	})
 }
diff --git a/internal/service/smarthttp/receive_pack.go b/internal/service/smarthttp/receive_pack.go
index 5c11fa144..5ff0d584e 100644
--- a/internal/service/smarthttp/receive_pack.go
+++ b/internal/service/smarthttp/receive_pack.go
@@ -7,6 +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"
@@ -43,6 +44,7 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac
 	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))
diff --git a/internal/service/smarthttp/receive_pack_test.go b/internal/service/smarthttp/receive_pack_test.go
index 3b906ec7f..8e51b6aa0 100644
--- a/internal/service/smarthttp/receive_pack_test.go
+++ b/internal/service/smarthttp/receive_pack_test.go
@@ -23,33 +23,12 @@ import (
 )
 
 func TestSuccessfulReceivePackRequest(t *testing.T) {
-	server, serverSocketPath := runSmartHTTPServer(t)
-	defer server.Stop()
+	defer func(dir string) { config.Config.GitlabShell.Dir = dir }(config.Config.GitlabShell.Dir)
+	config.Config.GitlabShell.Dir = "/foo/bar/gitlab-shell"
 
-	repo, repoPath, cleanup := testhelper.NewTestRepo(t)
+	hookOutputFile, cleanup := testhelper.CaptureHookEnv(t)
 	defer cleanup()
 
-	client, conn := newSmartHTTPClient(t, serverSocketPath)
-	defer conn.Close()
-
-	ctx, cancel := testhelper.Context()
-	defer cancel()
-
-	stream, err := client.PostReceivePack(ctx)
-	require.NoError(t, err)
-
-	push := newTestPush(t, nil)
-	firstRequest := &gitalypb.PostReceivePackRequest{Repository: repo, GlId: "user-123", GlRepository: "project-123"}
-	response := doPush(t, stream, firstRequest, push.body)
-
-	expectedResponse := "0030\x01000eunpack ok\n0019ok refs/heads/master\n00000000"
-	require.Equal(t, expectedResponse, string(response), "Expected response to be %q, got %q", expectedResponse, response)
-
-	// The fact that this command succeeds means that we got the commit correctly, no further checks should be needed.
-	testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "show", push.newHead)
-}
-
-func TestSuccessfulReceivePackRequestWithGitOpts(t *testing.T) {
 	server, serverSocketPath := runSmartHTTPServer(t)
 	defer server.Stop()
 
@@ -66,7 +45,7 @@ func TestSuccessfulReceivePackRequestWithGitOpts(t *testing.T) {
 	require.NoError(t, err)
 
 	push := newTestPush(t, nil)
-	firstRequest := &gitalypb.PostReceivePackRequest{Repository: repo, GlId: "user-123", GlRepository: "project-123", GitConfigOptions: []string{"receive.MaxInputSize=10000"}}
+	firstRequest := &gitalypb.PostReceivePackRequest{Repository: repo, GlId: "user-123", GlRepository: "project-456"}
 	response := doPush(t, stream, firstRequest, push.body)
 
 	expectedResponse := "0030\x01000eunpack ok\n0019ok refs/heads/master\n00000000"
@@ -74,6 +53,18 @@ func TestSuccessfulReceivePackRequestWithGitOpts(t *testing.T) {
 
 	// The fact that this command succeeds means that we got the commit correctly, no further checks should be needed.
 	testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "show", push.newHead)
+
+	envData, err := ioutil.ReadFile(hookOutputFile)
+	require.NoError(t, err, "get git env data")
+
+	for _, env := range []string{
+		"GL_ID=user-123",
+		"GL_REPOSITORY=project-456",
+		"GL_PROTOCOL=http",
+		"GITLAB_SHELL_DIR=" + "/foo/bar/gitlab-shell",
+	} {
+		require.Contains(t, strings.Split(string(envData), "\n"), env)
+	}
 }
 
 func TestSuccessfulReceivePackRequestWithGitProtocol(t *testing.T) {
diff --git a/internal/service/ssh/.gitignore b/internal/service/ssh/.gitignore
new file mode 100644
index 000000000..ace1063ab
--- /dev/null
+++ b/internal/service/ssh/.gitignore
@@ -0,0 +1 @@
+/testdata
diff --git a/internal/service/ssh/receive_pack.go b/internal/service/ssh/receive_pack.go
index f2a69a289..51df1774a 100644
--- a/internal/service/ssh/receive_pack.go
+++ b/internal/service/ssh/receive_pack.go
@@ -7,6 +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"
@@ -47,6 +48,7 @@ func (s *server) SSHReceivePack(stream gitalypb.SSHService_SSHReceivePackServer)
 		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))
diff --git a/internal/service/ssh/receive_pack_test.go b/internal/service/ssh/receive_pack_test.go
index 350f84bc2..a40a1ec1b 100644
--- a/internal/service/ssh/receive_pack_test.go
+++ b/internal/service/ssh/receive_pack_test.go
@@ -7,6 +7,7 @@ import (
 	"os"
 	"os/exec"
 	"path"
+	"strings"
 	"testing"
 	"time"
 
@@ -76,21 +77,32 @@ func TestFailedReceivePackRequestDueToValidationError(t *testing.T) {
 }
 
 func TestReceivePackPushSuccess(t *testing.T) {
+	defer func(dir string) { config.Config.GitlabShell.Dir = dir }(config.Config.GitlabShell.Dir)
+	config.Config.GitlabShell.Dir = "/foo/bar/gitlab-shell"
+
+	hookOutputFile, cleanup := testhelper.CaptureHookEnv(t)
+	defer cleanup()
+
 	server, serverSocketPath := runSSHServer(t)
 	defer server.Stop()
 
-	lHead, rHead, err := testCloneAndPush(t, serverSocketPath, pushParams{storageName: testRepo.GetStorageName(), glID: "1"})
+	lHead, rHead, err := testCloneAndPush(t, serverSocketPath, pushParams{storageName: testRepo.GetStorageName(), glID: "user-123"})
 	if err != nil {
 		t.Fatal(err)
 	}
 	require.Equal(t, lHead, rHead, "local and remote head not equal. push failed")
 
-	lHead, rHead, err = testCloneAndPush(t, serverSocketPath, pushParams{storageName: testRepo.GetStorageName(), glID: "1", gitConfigOptions: []string{"receive.MaxInputSize=10000"}})
-	if err != nil {
-		t.Fatal(err)
-	}
+	envData, err := ioutil.ReadFile(hookOutputFile)
+	require.NoError(t, err, "get git env data")
 
-	require.Equal(t, lHead, rHead, "local and remote head not equal. push failed")
+	for _, env := range []string{
+		"GL_ID=user-123",
+		"GL_REPOSITORY=project-456",
+		"GL_PROTOCOL=ssh",
+		"GITLAB_SHELL_DIR=" + "/foo/bar/gitlab-shell",
+	} {
+		require.Contains(t, strings.Split(string(envData), "\n"), env)
+	}
 }
 
 func TestReceivePackPushSuccessWithGitProtocol(t *testing.T) {
@@ -179,7 +191,7 @@ func testCloneAndPush(t *testing.T, serverSocketPath string, params pushParams)
 	pbMarshaler := &jsonpb.Marshaler{}
 	payload, err := pbMarshaler.MarshalToString(&gitalypb.SSHReceivePackRequest{
 		Repository:       pbTempRepo,
-		GlRepository:     pbTempRepo.GetRelativePath(),
+		GlRepository:     "project-456",
 		GlId:             params.glID,
 		GitConfigOptions: params.gitConfigOptions,
 		GitProtocol:      params.gitProtocol,
diff --git a/internal/testhelper/hook_env.go b/internal/testhelper/hook_env.go
new file mode 100644
index 000000000..b2f11449a
--- /dev/null
+++ b/internal/testhelper/hook_env.go
@@ -0,0 +1,33 @@
+package testhelper
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
+)
+
+// CaptureHookEnv creates a bogus 'update' Git hook to sniff out what
+// environment variables get set for hooks.
+func CaptureHookEnv(t *testing.T) (hookPath string, cleanup func()) {
+	var err error
+	hooks.Override, err = filepath.Abs("testdata/hooks")
+	require.NoError(t, err)
+
+	hookOutputFile, err := filepath.Abs("testdata/hook.env")
+	require.NoError(t, err)
+
+	require.NoError(t, os.RemoveAll(hookOutputFile))
+
+	require.NoError(t, os.MkdirAll(hooks.Override, 0755))
+	require.NoError(t, ioutil.WriteFile(filepath.Join(hooks.Override, "update"), []byte(`
+#!/bin/sh
+env | grep -e ^GIT -e ^GL_ > `+hookOutputFile+"\n"), 0755))
+
+	return hookOutputFile, func() {
+		hooks.Override = ""
+	}
+}
diff --git a/ruby/git-hooks/gitlab-shell-hook b/ruby/git-hooks/gitlab-shell-hook
new file mode 100755
index 000000000..487b0a870
--- /dev/null
+++ b/ruby/git-hooks/gitlab-shell-hook
@@ -0,0 +1,6 @@
+#!/bin/sh
+
+# This is the single source of truth for where Gitaly's embedded Git hooks are.
+hooks_dir="$(dirname $0)/../vendor/gitlab-shell/hooks"
+
+exec "$hooks_dir/$(basename $0)" "$@"
diff --git a/ruby/git-hooks/post-receive b/ruby/git-hooks/post-receive
new file mode 120000
index 000000000..3c6ac8d8c
--- /dev/null
+++ b/ruby/git-hooks/post-receive
@@ -0,0 +1 @@
+gitlab-shell-hook
\ No newline at end of file
diff --git a/ruby/git-hooks/pre-receive b/ruby/git-hooks/pre-receive
new file mode 120000
index 000000000..3c6ac8d8c
--- /dev/null
+++ b/ruby/git-hooks/pre-receive
@@ -0,0 +1 @@
+gitlab-shell-hook
\ No newline at end of file
diff --git a/ruby/git-hooks/update b/ruby/git-hooks/update
new file mode 120000
index 000000000..3c6ac8d8c
--- /dev/null
+++ b/ruby/git-hooks/update
@@ -0,0 +1 @@
+gitlab-shell-hook
\ No newline at end of file
diff --git a/ruby/lib/gitlab/config.rb b/ruby/lib/gitlab/config.rb
index 379d28f4c..6aa445aa8 100644
--- a/ruby/lib/gitlab/config.rb
+++ b/ruby/lib/gitlab/config.rb
@@ -1,13 +1,31 @@
 module Gitlab
-  # Config lets Gitlab::Git do mock config lookups.
+  #
+  # In production, gitaly-ruby configuration is derived from environment
+  # variables set by the Go gitaly parent process. During the rspec test
+  # suite this parent process is not there so we need to get configuration
+  # values from somewhere else. We used to work around this by setting
+  # variables in ENV during the rspec boot but that turned out to be
+  # fragile because Bundler.with_clean_env resets changes to ENV made
+  # after Bundler was loaded. Instead of changing ENV, the TestSetup
+  # module gives us a hacky way to set instance variables on the config
+  # objects, bypassing the ENV lookups.
+  #
+  module TestSetup
+    def test_global_ivar_override(name, value)
+      instance_variable_set("@#{name}".to_sym, value)
+    end
+  end
+
   class Config
     class Git
+      include TestSetup
+
       def bin_path
-        ENV['GITALY_RUBY_GIT_BIN_PATH']
+        @bin_path ||= ENV['GITALY_RUBY_GIT_BIN_PATH']
       end
 
       def hooks_directory
-        ENV['GITALY_GIT_HOOKS_DIR']
+        @hooks_directory ||= ENV['GITALY_GIT_HOOKS_DIR']
       end
 
       def write_buffer_size
@@ -20,35 +38,39 @@ module Gitlab
     end
 
     class GitlabShell
+      include TestSetup
+
       def path
-        ENV['GITALY_RUBY_GITLAB_SHELL_PATH']
+        @path ||= ENV['GITALY_RUBY_GITLAB_SHELL_PATH']
       end
 
       def git_timeout
-        10800 # TODO make this configurable or eliminate otherwise https://gitlab.com/gitlab-org/gitaly/issues/885
+        @git_timeout ||= 10800 # TODO make this configurable or eliminate otherwise https://gitlab.com/gitlab-org/gitaly/issues/885
       end
     end
 
     class Gitaly
+      include TestSetup
+
       def client_path
-        ENV['GITALY_RUBY_GITALY_BIN_DIR']
+        @client_path ||= ENV['GITALY_RUBY_GITALY_BIN_DIR']
       end
     end
 
     def git
-      Git.new
+      @git ||= Git.new
     end
 
     def gitlab_shell
-      GitlabShell.new
+      @gitlab_shell ||= GitlabShell.new
     end
 
     def gitaly
-      Gitaly.new
+      @gitaly ||= Gitaly.new
     end
   end
 
   def self.config
-    Config.new
+    @config ||= Config.new
   end
 end
diff --git a/ruby/lib/gitlab/git/hook.rb b/ruby/lib/gitlab/git/hook.rb
index 56024bcc5..f89f88ff1 100644
--- a/ruby/lib/gitlab/git/hook.rb
+++ b/ruby/lib/gitlab/git/hook.rb
@@ -101,6 +101,7 @@ module Gitlab
 
       def env_base_vars(gl_id, gl_username)
         {
+          'GITLAB_SHELL_DIR' => Gitlab.config.gitlab_shell.path,
           'GL_ID' => gl_id,
           'GL_USERNAME' => gl_username,
           'PWD' => repo_path,
diff --git a/ruby/spec/lib/gitlab/config_spec.rb b/ruby/spec/lib/gitlab/config_spec.rb
index 2a503d005..e419408a2 100644
--- a/ruby/spec/lib/gitlab/config_spec.rb
+++ b/ruby/spec/lib/gitlab/config_spec.rb
@@ -7,11 +7,7 @@ describe Gitlab::Config do
     let(:gitlab_shell_path) { '/foo/bar/gitlab-shell' }
 
     before do
-      ENV['GITALY_RUBY_GITLAB_SHELL_PATH'] = gitlab_shell_path
-    end
-
-    after do
-      ENV.delete('GITALY_RUBY_GITLAB_SHELL_PATH')
+      allow(ENV).to receive(:[]).with('GITALY_RUBY_GITLAB_SHELL_PATH').and_return(gitlab_shell_path)
     end
 
     it { expect(subject.path).to eq(gitlab_shell_path) }
diff --git a/ruby/spec/lib/gitlab/git/hook_spec.rb b/ruby/spec/lib/gitlab/git/hook_spec.rb
index ac2062975..402980704 100644
--- a/ruby/spec/lib/gitlab/git/hook_spec.rb
+++ b/ruby/spec/lib/gitlab/git/hook_spec.rb
@@ -10,7 +10,6 @@ describe Gitlab::Git::Hook do
   end
 
   describe '#trigger' do
-    let!(:old_env) { ENV['GITALY_GIT_HOOKS_DIR'] }
     let(:tmp_dir) { Dir.mktmpdir }
     let(:hook_names) { %w[pre-receive post-receive update] }
     let(:repo) { gitlab_git_from_gitaly(test_repo_read_only) }
@@ -22,28 +21,46 @@ describe Gitlab::Git::Hook do
         FileUtils.chmod("u+x", path)
       end
 
-      ENV['GITALY_GIT_HOOKS_DIR'] = tmp_dir
+      allow(Gitlab.config.git).to receive(:hooks_directory).and_return(tmp_dir)
+      allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('/foobar/gitlab-shell')
     end
 
     after do
       FileUtils.remove_entry(tmp_dir)
-      ENV['GITALY_GIT_HOOKS_DIR'] = old_env
     end
 
     context 'when the hooks require environment variables' do
-      let(:vars) { %w[GL_ID GL_USERNAME PWD GIT_DIR] }
+      let(:vars) do
+        {
+          'GL_ID' => 'user-123',
+          'GL_USERNAME' => 'janedoe',
+          'PWD' => repo.path,
+          'GIT_DIR' => repo.path,
+          'GITLAB_SHELL_DIR' => '/foobar/gitlab-shell'
+        }
+      end
+
       let(:script) do
-        "#!/bin/sh\n" +
-          vars.map { |var| "if [ -z \"$#{var}\"]; then exit 1; fi\n" }.join("\n") +
-          "exit 0\n"
+        [
+          "#!/bin/sh",
+          vars.map do |key, value|
+            <<-SCRIPT
+              if [ x$#{key} != x#{value} ]; then
+                echo "unexpected value: #{key}=$#{key}"
+                exit 1
+               fi
+            SCRIPT
+          end.join,
+          "exit 0"
+        ].join("\n")
       end
 
       it 'returns true' do
         hook_names.each do |hook|
           trigger_result = described_class.new(hook, repo)
-                                          .trigger('1', 'admin', '0' * 40, 'a' * 40, 'master')
+                                          .trigger(vars['GL_ID'], vars['GL_USERNAME'], '0' * 40, 'a' * 40, 'master')
 
-          expect(trigger_result.first).to be(true)
+          expect(trigger_result.first).to be(true), "#{hook} failed:  #{trigger_result.last}"
         end
       end
     end
@@ -54,7 +71,7 @@ describe Gitlab::Git::Hook do
       it 'returns true' do
         hook_names.each do |hook|
           trigger_result = described_class.new(hook, repo)
-                                          .trigger('1', 'admin', '0' * 40, 'a' * 40, 'master')
+                                          .trigger('user-456', 'admin', '0' * 40, 'a' * 40, 'master')
 
           expect(trigger_result.first).to be(true)
         end
@@ -67,7 +84,7 @@ describe Gitlab::Git::Hook do
       it 'returns false' do
         hook_names.each do |hook|
           trigger_result = described_class.new(hook, repo)
-                                          .trigger('1', 'admin', '0' * 40, 'a' * 40, 'master')
+                                          .trigger('user-1', 'admin', '0' * 40, 'a' * 40, 'master')
 
           expect(trigger_result.first).to be(false)
         end
diff --git a/ruby/spec/lib/gitlab/git/repository_spec.rb b/ruby/spec/lib/gitlab/git/repository_spec.rb
index e5cfb7d7b..82e69f935 100644
--- a/ruby/spec/lib/gitlab/git/repository_spec.rb
+++ b/ruby/spec/lib/gitlab/git/repository_spec.rb
@@ -23,15 +23,6 @@ describe Gitlab::Git::Repository do # rubocop:disable Metrics/BlockLength
       }
     end
     let(:call) { double(metadata: call_metadata) }
-    let(:gitlab_shell_path) { '/foo/bar/gitlab-shell' }
-
-    before do
-      ENV['GITALY_RUBY_GITLAB_SHELL_PATH'] = gitlab_shell_path
-    end
-
-    after do
-      ENV.delete('GITALY_RUBY_GITLAB_SHELL_PATH')
-    end
 
     it 'cleans up the repository' do
       described_class.from_gitaly_with_block(test_repo_read_only, call) do |repository|
diff --git a/ruby/spec/spec_helper.rb b/ruby/spec/spec_helper.rb
index 183cac7ba..8b861254d 100644
--- a/ruby/spec/spec_helper.rb
+++ b/ruby/spec/spec_helper.rb
@@ -11,9 +11,9 @@ require File.join(__dir__, 'support/helpers/gitlab_shell_helper.rb')
 
 Dir[File.join(__dir__, 'support/helpers/*.rb')].each { |f| require f }
 
-ENV['GITALY_RUBY_GIT_BIN_PATH'] ||= 'git'
-ENV['GITALY_GIT_HOOKS_DIR'] ||= File.join(Gitlab.config.gitlab_shell.path.to_s, "hooks")
-ENV['GITALY_RUBY_GITALY_BIN_DIR'] = __dir__
+Gitlab.config.git.test_global_ivar_override(:bin_path, 'git')
+Gitlab.config.git.test_global_ivar_override(:hooks_directory, File.join(Gitlab.config.gitlab_shell.path.to_s, "hooks"))
+Gitlab.config.gitaly.test_global_ivar_override(:client_path, __dir__)
 
 RSpec.configure do |config|
   config.include FactoryBot::Syntax::Methods
diff --git a/ruby/spec/support/helpers/gitlab_shell_helper.rb b/ruby/spec/support/helpers/gitlab_shell_helper.rb
index ad127bf2e..b38b6ae62 100644
--- a/ruby/spec/support/helpers/gitlab_shell_helper.rb
+++ b/ruby/spec/support/helpers/gitlab_shell_helper.rb
@@ -7,7 +7,7 @@ GITLAB_SHELL_DIR = File.join(TMP_DIR, 'gitlab-shell').freeze
 
 module GitlabShellHelper
   def self.setup_gitlab_shell
-    ENV['GITALY_RUBY_GITLAB_SHELL_PATH'] = GITLAB_SHELL_DIR
+    Gitlab.config.gitlab_shell.test_global_ivar_override(:path, GITLAB_SHELL_DIR)
 
     FileUtils.mkdir_p([TMP_DIR, File.join(GITLAB_SHELL_DIR, 'hooks')])
   end