Merge branch 'jc-revert-8e18cae9' into 'master'

Revert "Merge branch 'jc-revert-revert-414c9b7c' into 'master'"

See merge request gitlab-org/gitaly!2246

14 files changed, 276 insertions, 95 deletions

diff --git a/changelogs/unreleased/jc-deprecate-gitlab-shell-yml.yml b/changelogs/unreleased/jc-deprecate-gitlab-shell-yml.yml
deleted file mode 100644
index 175e4c93f..000000000
--- a/changelogs/unreleased/jc-deprecate-gitlab-shell-yml.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-title: Remove usage of gitlab shell yml
-merge_request: 2243
-author:
-type: deprecated
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index 2d7fd82f3..bf13a4707 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -93,6 +93,16 @@ func TestHooksPrePostReceive(t *testing.T) {
 
 	config.Config.GitlabShell.Dir = tempGitlabShellDir
 
+	testhelper.WriteTemporaryGitlabShellConfigFile(t,
+		tempGitlabShellDir,
+		testhelper.GitlabShellConfig{
+			GitlabURL: ts.URL,
+			HTTPSettings: testhelper.HTTPSettings{
+				User:     gitlabUser,
+				Password: gitlabPassword,
+			},
+		})
+
 	testhelper.WriteShellSecretFile(t, tempGitlabShellDir, secretToken)
 
 	gitObjectDirRegex := regexp.MustCompile(`(?m)^GIT_OBJECT_DIRECTORY=(.*)$`)
@@ -192,8 +202,9 @@ func TestHooksUpdate(t *testing.T) {
 	customHooksDir, cleanup := testhelper.TempDir(t)
 	defer cleanup()
 
-	config.Config.Gitlab.URL = "http://www.example.com"
-	config.Config.Hooks.CustomHooksDir = customHooksDir
+	testhelper.WriteTemporaryGitlabShellConfigFile(t, tempGitlabShellDir, testhelper.GitlabShellConfig{GitlabURL: "http://www.example.com", CustomHooksDir: customHooksDir})
+
+	os.Symlink(filepath.Join(config.Config.GitlabShell.Dir, "config.yml"), filepath.Join(tempGitlabShellDir, "config.yml"))
 
 	testhelper.WriteShellSecretFile(t, tempGitlabShellDir, "the wrong token")
 
@@ -208,7 +219,9 @@ func TestHooksUpdate(t *testing.T) {
 
 	for _, featureSet := range featureSets {
 		t.Run(fmt.Sprintf("enabled features: %v", featureSet), func(t *testing.T) {
-			config.Config.Hooks.CustomHooksDir = customHooksDir
+			if featureSet.IsEnabled("use_gitaly_gitlabshell_config") {
+				config.Config.Hooks.CustomHooksDir = customHooksDir
+			}
 
 			testHooksUpdate(t, tempGitlabShellDir, socket, token, testhelper.GlHookValues{
 				GLID:       glID,
@@ -405,11 +418,12 @@ func TestHooksNotAllowed(t *testing.T) {
 	ts := testhelper.NewGitlabTestServer(c)
 	defer ts.Close()
 
+	testhelper.WriteTemporaryGitlabShellConfigFile(t, tempGitlabShellDir, testhelper.GitlabShellConfig{GitlabURL: ts.URL})
+	testhelper.WriteShellSecretFile(t, tempGitlabShellDir, "the wrong token")
+
+	config.Config.GitlabShell.Dir = tempGitlabShellDir
 	config.Config.Gitlab.URL = ts.URL
 	config.Config.Gitlab.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret")
-	config.Config.GitlabShell.Dir = tempGitlabShellDir
-
-	testhelper.WriteShellSecretFile(t, tempGitlabShellDir, "the wrong token")
 
 	customHookOutputPath, cleanup := testhelper.WriteEnvToCustomHook(t, testRepoPath, "post-receive")
 	defer cleanup()
@@ -476,6 +490,9 @@ func TestCheckOK(t *testing.T) {
 	require.NoError(t, os.Symlink(filepath.Join(cwd, "../../ruby/gitlab-shell/bin/check"), filepath.Join(binDir, "check")))
 
 	testhelper.WriteShellSecretFile(t, gitlabShellDir, "the secret")
+	testhelper.WriteTemporaryGitlabShellConfigFile(t,
+		gitlabShellDir,
+		testhelper.GitlabShellConfig{GitlabURL: ts.URL, HTTPSettings: testhelper.HTTPSettings{User: user, Password: password}})
 
 	configPath, cleanup := testhelper.WriteTemporaryGitalyConfigFile(t, tempDir, ts.URL, user, password)
 	defer cleanup()
@@ -521,6 +538,7 @@ func TestCheckBadCreds(t *testing.T) {
 	require.NoError(t, err)
 	require.NoError(t, os.Symlink(filepath.Join(cwd, "../../ruby/gitlab-shell/bin/check"), filepath.Join(binDir, "check")))
 
+	testhelper.WriteTemporaryGitlabShellConfigFile(t, gitlabShellDir, testhelper.GitlabShellConfig{GitlabURL: ts.URL, HTTPSettings: testhelper.HTTPSettings{User: user + "wrong", Password: password}})
 	testhelper.WriteShellSecretFile(t, gitlabShellDir, "the secret")
 
 	configPath, cleanup := testhelper.WriteTemporaryGitalyConfigFile(t, tempDir, ts.URL, "wrong", password)
diff --git a/go.mod b/go.mod
index d209f9350..c9581053b 100644
--- a/go.mod
+++ b/go.mod
@@ -23,7 +23,7 @@ require (
 	golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e
 	golang.org/x/sys v0.0.0-20200113162924-86b910548bc1
 	google.golang.org/grpc v1.24.0
-	gopkg.in/yaml.v2 v2.2.8 // indirect
+	gopkg.in/yaml.v2 v2.2.8
 )
 
 go 1.13
diff --git a/internal/gitlabshell/env.go b/internal/gitlabshell/env.go
index 8e601270e..9165932ab 100644
--- a/internal/gitlabshell/env.go
+++ b/internal/gitlabshell/env.go
@@ -14,13 +14,13 @@ func Env() ([]string, error) {
 }
 
 type Config struct {
-	GitlabDir      string              `json:"dir"`
 	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"`
+	RootPath       string              `json:"root_path"`
 	SecretFile     string              `json:"secret_file"`
 }
 
@@ -30,10 +30,10 @@ func EnvFromConfig(cfg config.Cfg) ([]string, error) {
 		CustomHooksDir: cfg.Hooks.CustomHooksDir,
 		GitlabURL:      cfg.Gitlab.URL,
 		HTTPSettings:   cfg.Gitlab.HTTPSettings,
-		GitlabDir:      cfg.GitlabShell.Dir,
 		LogFormat:      cfg.Logging.Format,
 		LogLevel:       cfg.Logging.Level,
 		LogPath:        cfg.Logging.Dir,
+		RootPath:       cfg.GitlabShell.Dir, //GITLAB_SHELL_DIR has been deprecated
 		SecretFile:     cfg.Gitlab.SecretFile,
 	}
 
@@ -43,6 +43,11 @@ func EnvFromConfig(cfg config.Cfg) ([]string, error) {
 	}
 
 	return []string{
+		//TODO: remove GITALY_GITLAB_SHELL_DIR: https://gitlab.com/gitlab-org/gitaly/-/issues/2679
+		"GITALY_GITLAB_SHELL_DIR=" + cfg.GitlabShell.Dir,
+		"GITALY_LOG_DIR=" + cfg.Logging.Dir,
+		"GITALY_LOG_FORMAT=" + cfg.Logging.Format,
+		"GITALY_LOG_LEVEL=" + cfg.Logging.Level,
 		"GITALY_BIN_DIR=" + cfg.BinDir,
 		"GITALY_RUBY_DIR=" + cfg.Ruby.Dir,
 		"GITALY_GITLAB_SHELL_CONFIG=" + string(gitlabShellConfigString),
diff --git a/internal/gitlabshell/env_test.go b/internal/gitlabshell/env_test.go
index 8dc8309f4..9eac8f076 100644
--- a/internal/gitlabshell/env_test.go
+++ b/internal/gitlabshell/env_test.go
@@ -65,7 +65,6 @@ func TestGitHooksConfig(t *testing.T) {
 	require.Equal(t, config.Config.Gitlab.SecretFile, rubyConfigMap["secret_file"])
 	require.Equal(t, config.Config.Hooks.CustomHooksDir, rubyConfigMap["custom_hooks_dir"])
 	require.Equal(t, config.Config.Gitlab.URL, rubyConfigMap["gitlab_url"])
-	require.Equal(t, config.Config.GitlabShell.Dir, rubyConfigMap["gitlab_shell_dir"])
 
 	// HTTP Settings
 	httpSettings, ok := rubyConfigMap["http_settings"].(map[string]interface{})
diff --git a/internal/service/operations/squash_test.go b/internal/service/operations/squash_test.go
index a8dd78131..6787bfe48 100644
--- a/internal/service/operations/squash_test.go
+++ b/internal/service/operations/squash_test.go
@@ -21,7 +21,6 @@ var (
 	author = &gitalypb.User{
 		Name:  []byte("John Doe"),
 		Email: []byte("johndoe@gitlab.com"),
-		GlId:  testhelper.GlID,
 	}
 	branchName    = "not-merged-branch"
 	startSha      = "b83d6e391c22777fca1ed3012fce84f633d7fed0"
@@ -282,7 +281,7 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) {
 				Repository:    nil,
 				User:          testhelper.TestUser,
 				SquashId:      "1",
-				Author:        author,
+				Author:        testhelper.TestUser,
 				CommitMessage: commitMessage,
 				StartSha:      startSha,
 				EndSha:        endSha,
@@ -295,7 +294,7 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) {
 				Repository:    testRepo,
 				User:          nil,
 				SquashId:      "1",
-				Author:        author,
+				Author:        testhelper.TestUser,
 				CommitMessage: commitMessage,
 				StartSha:      startSha,
 				EndSha:        endSha,
@@ -308,7 +307,7 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) {
 				Repository:    testRepo,
 				User:          testhelper.TestUser,
 				SquashId:      "",
-				Author:        author,
+				Author:        testhelper.TestUser,
 				CommitMessage: commitMessage,
 				StartSha:      startSha,
 				EndSha:        endSha,
@@ -321,7 +320,7 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) {
 				Repository:    testRepo,
 				User:          testhelper.TestUser,
 				SquashId:      "1",
-				Author:        author,
+				Author:        testhelper.TestUser,
 				CommitMessage: commitMessage,
 				StartSha:      "",
 				EndSha:        endSha,
@@ -334,7 +333,7 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) {
 				Repository:    testRepo,
 				User:          testhelper.TestUser,
 				SquashId:      "1",
-				Author:        author,
+				Author:        testhelper.TestUser,
 				CommitMessage: commitMessage,
 				StartSha:      startSha,
 				EndSha:        "",
@@ -360,7 +359,7 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) {
 				Repository:    testRepo,
 				User:          testhelper.TestUser,
 				SquashId:      "1",
-				Author:        author,
+				Author:        testhelper.TestUser,
 				CommitMessage: nil,
 				StartSha:      startSha,
 				EndSha:        endSha,
diff --git a/internal/service/smarthttp/receive_pack_test.go b/internal/service/smarthttp/receive_pack_test.go
index fd3f8bf5c..35a83d23f 100644
--- a/internal/service/smarthttp/receive_pack_test.go
+++ b/internal/service/smarthttp/receive_pack_test.go
@@ -68,6 +68,7 @@ func TestSuccessfulReceivePackRequest(t *testing.T) {
 		"GL_ID=user-123",
 		"GL_REPOSITORY=project-456",
 		"GL_PROTOCOL=http",
+		"GITALY_GITLAB_SHELL_DIR=" + "/foo/bar/gitlab-shell",
 	} {
 		require.Contains(t, strings.Split(string(envData), "\n"), env)
 	}
@@ -394,6 +395,7 @@ func testPostReceivePackToHooks(t *testing.T, callRPC bool) {
 	})
 	defer ts.Close()
 
+	testhelper.WriteTemporaryGitlabShellConfigFile(t, tempGitlabShellDir, testhelper.GitlabShellConfig{GitlabURL: ts.URL})
 	testhelper.WriteShellSecretFile(t, tempGitlabShellDir, secretToken)
 
 	testhelper.WriteCustomHook(testRepoPath, "pre-receive", []byte(testhelper.CheckNewObjectExists))
@@ -495,10 +497,15 @@ func TestPostReceiveWithTransactions(t *testing.T) {
 			gitlabShellDir, cleanup := testhelper.CreateTemporaryGitlabShellDir(t)
 			defer cleanup()
 			config.Config.GitlabShell.Dir = gitlabShellDir
-			config.Config.Gitlab.URL = gitlabServer.URL
-			config.Config.Gitlab.HTTPSettings.User = gitlabUser
-			config.Config.Gitlab.HTTPSettings.Password = gitlabPassword
-
+			testhelper.WriteTemporaryGitlabShellConfigFile(t,
+				gitlabShellDir,
+				testhelper.GitlabShellConfig{
+					GitlabURL: gitlabServer.URL,
+					HTTPSettings: testhelper.HTTPSettings{
+						User:     gitlabUser,
+						Password: gitlabPassword,
+					},
+				})
 			testhelper.WriteShellSecretFile(t, gitlabShellDir, secretToken)
 
 			gitalyServer := testhelper.NewServerWithAuth(t, nil, nil, config.Config.Auth.Token)
diff --git a/internal/service/ssh/receive_pack_test.go b/internal/service/ssh/receive_pack_test.go
index a9fd11bdb..a72cdbd5e 100644
--- a/internal/service/ssh/receive_pack_test.go
+++ b/internal/service/ssh/receive_pack_test.go
@@ -102,6 +102,7 @@ func TestReceivePackPushSuccess(t *testing.T) {
 		"GL_ID=user-123",
 		fmt.Sprintf("GL_REPOSITORY=%s", glRepository),
 		"GL_PROTOCOL=ssh",
+		"GITALY_GITLAB_SHELL_DIR=" + "/foo/bar/gitlab-shell",
 	} {
 		require.Contains(t, strings.Split(string(envData), "\n"), env)
 	}
@@ -239,6 +240,7 @@ func TestSSHReceivePackToHooks(t *testing.T) {
 	})
 	defer ts.Close()
 
+	testhelper.WriteTemporaryGitlabShellConfigFile(t, tempGitlabShellDir, testhelper.GitlabShellConfig{GitlabURL: ts.URL})
 	testhelper.WriteShellSecretFile(t, tempGitlabShellDir, secretToken)
 
 	config.Config.Gitlab.URL = ts.URL
diff --git a/internal/testhelper/testserver.go b/internal/testhelper/testserver.go
index a73f4d64b..26f990aa7 100644
--- a/internal/testhelper/testserver.go
+++ b/internal/testhelper/testserver.go
@@ -38,6 +38,7 @@ import (
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/health"
 	healthpb "google.golang.org/grpc/health/grpc_health_v1"
+	"gopkg.in/yaml.v2"
 )
 
 // PraefectEnabled returns whether or not tests should use a praefect proxy
@@ -683,6 +684,24 @@ func CreateTemporaryGitlabShellDir(t testing.TB) (string, func()) {
 	}
 }
 
+// WriteTemporaryGitlabShellConfigFile writes a gitlab shell config.yml in a temporary directory. It returns the path
+// and a cleanup function
+func WriteTemporaryGitlabShellConfigFile(t FatalLogger, dir string, config GitlabShellConfig) (string, func()) {
+	out, err := yaml.Marshal(&config)
+	if err != nil {
+		t.Fatalf("error marshalling config", err)
+	}
+
+	path := filepath.Join(dir, "config.yml")
+	if err = ioutil.WriteFile(path, out, 0644); err != nil {
+		t.Fatalf("error writing gitlab shell config", err)
+	}
+
+	return path, func() {
+		os.RemoveAll(path)
+	}
+}
+
 // WriteTemporaryGitalyConfigFile writes a gitaly toml file into a temporary directory. It returns the path to
 // the file as well as a cleanup function
 func WriteTemporaryGitalyConfigFile(t testing.TB, tempDir, gitlabURL, user, password string) (string, func()) {
@@ -690,11 +709,10 @@ func WriteTemporaryGitalyConfigFile(t testing.TB, tempDir, gitlabURL, user, pass
 	contents := fmt.Sprintf(`
 [gitlab-shell]
   dir = "%s/gitlab-shell"
-[gitlab]
-url = %q
-[gitlab.http-settings]
-  user = %q
-  password = %q
+  gitlab_url = %q
+  [gitlab-shell.http-settings]
+    user = %q
+    password = %q
 `, tempDir, gitlabURL, user, password)
 
 	require.NoError(t, ioutil.WriteFile(path, []byte(contents), 0644))
@@ -791,6 +809,7 @@ func NewHealthServerWithListener(t testing.TB, listener net.Listener) (*grpc.Ser
 func SetupAndStartGitlabServer(t FatalLogger, c *GitlabTestServerOptions) (string, func()) {
 	ts := NewGitlabTestServer(*c)
 
+	WriteTemporaryGitlabShellConfigFile(t, config.Config.GitlabShell.Dir, GitlabShellConfig{GitlabURL: ts.URL})
 	WriteShellSecretFile(t, config.Config.GitlabShell.Dir, c.SecretToken)
 
 	return ts.URL, func() {
diff --git a/ruby/gitlab-shell/lib/gitlab_config.rb b/ruby/gitlab-shell/lib/gitlab_config.rb
index 0e196aad1..d6d964cce 100644
--- a/ruby/gitlab-shell/lib/gitlab_config.rb
+++ b/ruby/gitlab-shell/lib/gitlab_config.rb
@@ -1,29 +1,30 @@
+require 'yaml'
+
 class GitlabConfig
   def secret_file
-    fetch_from_config('secret_file',  File.join(gitlab_shell_dir, '.gitlab_shell_secret'))
+    fetch_from_config('secret_file', fetch_from_legacy_config('secret_file', File.join(ROOT_PATH, '.gitlab_shell_secret')))
   end
 
   # Pass a default value because this is called from a repo's context; in which
   # case, the repo's hooks directory should be the default.
   #
-  def gitlab_shell_dir
-    fetch_from_config('dir', File.dirname(__dir__))
-  end
-
   def custom_hooks_dir(default: nil)
-    fetch_from_config('custom_hooks_dir', File.join(gitlab_shell_dir, 'hooks'))
+    fetch_from_config('custom_hooks_dir', fetch_from_legacy_config('custom_hooks_dir', File.join(ROOT_PATH, 'hooks')))
   end
 
   def gitlab_url
-    fetch_from_config('gitlab_url', "http://localhost:8080").sub(%r{/*$}, '')
+    fetch_from_config('gitlab_url', fetch_from_legacy_config('gitlab_url',"http://localhost:8080").sub(%r{/*$}, ''))
   end
 
   class HTTPSettings
     DEFAULT_TIMEOUT = 300
 
     attr_reader :settings
-    def initialize(settings)
+    attr_reader :legacy_settings
+
+    def initialize(settings, legacy_settings = {})
       @settings = settings || {}
+      @legacy_settings = legacy_settings || {}
     end
 
     def user
@@ -57,24 +58,42 @@ class GitlabConfig
     private
 
     def fetch_from_settings(key)
-      settings[key]
+      value = settings[key]
+
+      return legacy_settings[key] if value.nil? || (value.is_a?(String) && value.empty?)
+
+      value
     end
   end
 
   def http_settings
-    @http_settings ||= GitlabConfig::HTTPSettings.new(fetch_from_config('http_settings', {}))
+    @http_settings ||= GitlabConfig::HTTPSettings.new(
+                        fetch_from_config('http_settings', {}),
+                        fetch_from_legacy_config('http_settings', {}))
   end
 
   def log_file
-    File.join(fetch_from_config('log_path', gitlab_shell_dir), 'gitlab-shell.log')
+    log_path = Pathname.new(fetch_from_config('log_path', LOG_PATH))
+
+    log_path = ROOT_PATH if log_path === ''
+
+    return log_path.join('gitlab-shell.log')
   end
 
   def log_level
-    fetch_from_config('log_level', 'INFO')
+    log_level = fetch_from_config('log_level', LOG_LEVEL)
+
+    return log_level unless log_level.empty?
+
+    'INFO'
   end
 
   def log_format
-    fetch_from_config('log_format', 'text')
+    log_format = fetch_from_config('log_format', LOG_FORMAT)
+
+    return log_format unless log_format.empty?
+
+    'text'
   end
 
   def to_json
@@ -86,10 +105,13 @@ class GitlabConfig
       log_file: log_file,
       log_level: log_level,
       log_format: log_format,
-      gitlab_shell_dir: gitlab_shell_dir,
     }.to_json
   end
 
+  def fetch_from_legacy_config(key, default)
+    legacy_config[key] || default
+  end
+
   private
 
   def fetch_from_config(key, default)
@@ -103,4 +125,12 @@ class GitlabConfig
   def config
     @config ||= JSON.parse(ENV.fetch('GITALY_GITLAB_SHELL_CONFIG', '{}'))
   end
+
+  def legacy_config
+    # TODO: deprecate @legacy_config that is parsing the gitlab-shell config.yml
+    legacy_file = ROOT_PATH.join('config.yml')
+    return {} unless legacy_file.exist?
+
+    @legacy_config ||= YAML.load_file(legacy_file)
+  end
 end
diff --git a/ruby/gitlab-shell/lib/gitlab_init.rb b/ruby/gitlab-shell/lib/gitlab_init.rb
index 188c5e6a5..949135493 100644
--- a/ruby/gitlab-shell/lib/gitlab_init.rb
+++ b/ruby/gitlab-shell/lib/gitlab_init.rb
@@ -1,3 +1,11 @@
+# GITLAB_SHELL_DIR has been deprecated
+require 'pathname'
+
+ROOT_PATH = Pathname.new(ENV['GITALY_GITLAB_SHELL_DIR'] || ENV['GITLAB_SHELL_DIR'] || File.expand_path('..', __dir__)).freeze
+LOG_PATH = Pathname.new(ENV.fetch('GITALY_LOG_DIR', ROOT_PATH))
+LOG_LEVEL = ENV.fetch('GITALY_LOG_LEVEL', 'INFO')
+LOG_FORMAT = ENV.fetch('GITALY_LOG_FORMAT', 'text')
+
 # We are transitioning parts of gitlab-shell into the gitaly project. In
 # gitaly, GITALY_EMBEDDED will be true.
 GITALY_EMBEDDED = true
diff --git a/ruby/gitlab-shell/spec/gitlab_config_spec.rb b/ruby/gitlab-shell/spec/gitlab_config_spec.rb
index 339b7e84b..6d5623e81 100644
--- a/ruby/gitlab-shell/spec/gitlab_config_spec.rb
+++ b/ruby/gitlab-shell/spec/gitlab_config_spec.rb
@@ -4,57 +4,158 @@ require 'json'
 
 describe GitlabConfig do
   let(:config) { GitlabConfig.new }
-  let(:config_data) { {'secret_file' => 'path/to/secret/file',
-          'custom_hooks_dir' => '/path/to/custom_hooks',
-          'gitlab_url' => 'http://localhost:123454',
-          'http_settings' => {'user' => 'user_123', 'password' =>'password123', 'ca_file' => '/path/to/ca_file', 'ca_path' => 'path/to/ca_path', 'read_timeout' => 200, 'self_signed' => true},
-          'log_path' => '/path/to/log',
-          'log_level' => 'myloglevel',
-          'log_format' => 'log_format'} }
-
-  before do
-   allow(ENV).to receive(:fetch).with('GITALY_GITLAB_SHELL_CONFIG', '{}').and_return(config_data.to_json)
-  end
 
-  describe '#secret_file' do
-    it 'returns the correct secret_file' do
-      expect(config.secret_file).to eq(config_data['secret_file'])
-    end
-  end
+  let(:config_data) { {} }
 
-  describe '#custom_hooks_dir' do
-    it 'returns the correct custom_hooks_dir' do
-      expect(config.custom_hooks_dir).to eq(config_data['custom_hooks_dir'])
-    end
-  end
+  before { allow(YAML).to receive(:load_file).and_return(config_data) }
+
+  describe '#gitlab_url' do
+    let(:url) { 'http://test.com' }
+
+    subject { config.gitlab_url }
 
-  describe '#http_settings' do
-    it 'returns the correct http_settings' do
-      expect(config.http_settings.settings).to eq(config_data['http_settings'])
+    before { config_data['gitlab_url'] = url }
+
+    it { is_expected.not_to be_empty }
+    it { is_expected.to eq(url) }
+
+    context 'remove trailing slashes' do
+      before { config_data['gitlab_url'] = url + '//' }
+
+      it { is_expected.to eq(url) }
     end
   end
 
-  describe '#gitlab_url' do
-    it 'returns the correct gitlab_url' do
-      expect(config.gitlab_url).to eq(config_data['gitlab_url'])
+  describe '#secret_file' do
+    subject { config.secret_file }
+
+    it 'returns ".gitlab_shell_secret" by default' do
+      is_expected.to eq(File.join(File.expand_path('..', __dir__),'.gitlab_shell_secret'))
     end
   end
 
-  describe '#log_path' do
-    it 'returns the correct log_path' do
-      expect(config.log_file).to eq(File.join(config_data['log_path'], 'gitlab-shell.log'))
+  describe '#fetch_from_legacy_config' do
+    let(:key) { 'yaml_key' }
+
+    where(:yaml_value, :default, :expected_value) do
+      [
+        ['a', 'b', 'a'],
+        [nil, 'b', 'b'],
+        ['a', nil, 'a'],
+        [nil, {}, {}]
+      ]
     end
-  end
 
-  describe '#log_level' do
-    it 'returns the correct log_level' do
-      expect(config.log_level).to eq(config_data['log_level'])
+    with_them do
+      it 'returns the correct value' do
+        config_data[key] = yaml_value
+
+        expect(config.fetch_from_legacy_config(key, default)).to eq(expected_value)
+      end
     end
-  end
 
-  describe '#log_format' do
-    it 'returns the correct log_format' do
-      expect(config.log_format).to eq(config_data['log_format'])
+    context "when ENV['GITALY_GITLAB_SHELL_CONFIG'] is passed in" do
+      let(:config_data) { {'secret_file' => 'path/to/secret/file',
+        'custom_hooks_dir' => '/path/to/custom_hooks',
+        'gitlab_url' => 'http://localhost:123454',
+        'http_settings' => {'user' => 'user_123', 'password' =>'password123', 'ca_file' => '/path/to/ca_file', 'ca_path' => 'path/to/ca_path', 'read_timeout' => 200, 'self_signed' => true},
+        'log_path' => '/path/to/log',
+        'log_level' => 'myloglevel',
+        'log_format' => 'log_format'} }
+       let(:gitaly_config_data) { config_data }
+      before do
+       allow(ENV).to receive(:fetch).with('GITALY_GITLAB_SHELL_CONFIG', '{}').and_return(gitaly_config_data.to_json)
+      end
+
+      describe '#secret_file' do
+        it 'returns the correct secret_file' do
+          expect(config.secret_file).to eq(gitaly_config_data['secret_file'])
+        end
+      end
+
+      describe '#custom_hooks_dir' do
+        it 'returns the correct custom_hooks_dir' do
+          expect(config.custom_hooks_dir).to eq(gitaly_config_data['custom_hooks_dir'])
+        end
+      end
+
+      describe '#http_settings' do
+        it 'returns the correct http_settings' do
+          expect(config.http_settings.settings).to eq(gitaly_config_data['http_settings'])
+        end
+
+        context 'when string values are nil' do
+          let(:gitaly_config_data) { {'http_settings': {'user': nil, 'password': nil, 'ca_file': nil, 'ca_path': nil, 'read_timeout': 200, 'self_signed': true}} }
+
+          it 'falls back to legacy config for user' do
+            expect(config.http_settings.user).to eq(config_data['http_settings']['user'])
+            expect(config.http_settings.user).not_to be_nil
+          end
+
+          it 'falls back to legacy config for password' do
+            expect(config.http_settings.password).to eq(config_data['http_settings']['password'])
+            expect(config.http_settings.password).not_to be_nil
+          end
+
+          it 'falls back to legacy config for ca_file' do
+            expect(config.http_settings.ca_file).to eq(config_data['http_settings']['ca_file'])
+            expect(config.http_settings.ca_file).not_to be_nil
+          end
+
+          it 'falls back to legacy config for ca_path' do
+            expect(config.http_settings.ca_path).to eq(config_data['http_settings']['ca_path'])
+            expect(config.http_settings.ca_path).not_to be_nil
+          end
+        end
+
+        context 'when string values are empty' do
+          let(:gitaly_config_data) { {'http_settings': {'user': '', 'password': '', 'ca_file': '', 'ca_path': '', 'read_timeout': 200, 'self_signed': true}} }
+
+          it 'falls back to legacy config for user' do
+            expect(config.http_settings.user).to eq(config_data['http_settings']['user'])
+            expect(config.http_settings.user).not_to be_empty
+          end
+
+          it 'falls back to legacy config for password' do
+            expect(config.http_settings.password).to eq(config_data['http_settings']['password'])
+            expect(config.http_settings.password).not_to be_empty
+          end
+
+          it 'falls back to legacy config for ca_file' do
+            expect(config.http_settings.ca_file).to eq(config_data['http_settings']['ca_file'])
+            expect(config.http_settings.ca_file).not_to be_empty
+          end
+
+          it 'falls back to legacy config for ca_path' do
+            expect(config.http_settings.ca_path).to eq(config_data['http_settings']['ca_path'])
+            expect(config.http_settings.ca_path).not_to be_empty
+          end
+        end
+      end
+
+      describe '#gitlab_url' do
+        it 'returns the correct gitlab_url' do
+          expect(config.gitlab_url).to eq(gitaly_config_data['gitlab_url'])
+        end
+      end
+
+      describe '#log_path' do
+        it 'returns the correct log_path' do
+          expect(config.log_file).to eq(Pathname.new(File.join(gitaly_config_data['log_path'], 'gitlab-shell.log')))
+        end
+      end
+
+      describe '#log_level' do
+        it 'returns the correct log_level' do
+          expect(config.log_level).to eq(gitaly_config_data['log_level'])
+        end
+      end
+
+      describe '#log_format' do
+        it 'returns the correct log_format' do
+          expect(config.log_format).to eq(gitaly_config_data['log_format'])
+        end
+      end
     end
   end
 end
diff --git a/ruby/gitlab-shell/spec/gitlab_custom_hook_spec.rb b/ruby/gitlab-shell/spec/gitlab_custom_hook_spec.rb
index 03a193c6e..c19b90f87 100644
--- a/ruby/gitlab-shell/spec/gitlab_custom_hook_spec.rb
+++ b/ruby/gitlab-shell/spec/gitlab_custom_hook_spec.rb
@@ -3,14 +3,13 @@ require 'spec_helper'
 require 'gitlab_custom_hook'
 
 describe GitlabCustomHook do
-  let(:root_path) { File.expand_path('..', __dir__) }
-  let(:tmp_repo_path) { File.join(root_path, 'tmp', 'repo.git') }
-  let(:tmp_root_path) { File.join(root_path, 'tmp') }
-  let(:gitaly_config_data) { {'dir' => root_path, 'custom_hooks_dir' => File.join(tmp_root_path, 'hooks') } }
+  let(:original_root_path) { ROOT_PATH }
+  let(:tmp_repo_path) { File.join(original_root_path, 'tmp', 'repo.git') }
+  let(:tmp_root_path) { File.join(original_root_path, 'tmp') }
   let(:global_custom_hooks_path) { global_hook_path('custom_global_hooks') }
-  let(:hook_ok) { File.join(root_path, 'spec', 'support', 'hook_ok') }
-  let(:hook_fail) { File.join(root_path, 'spec', 'support', 'hook_fail') }
-  let(:hook_gl_id) { File.join(root_path, 'spec', 'support', 'gl_id_test_hook') }
+  let(:hook_ok) { File.join(original_root_path, 'spec', 'support', 'hook_ok') }
+  let(:hook_fail) { File.join(original_root_path, 'spec', 'support', 'hook_fail') }
+  let(:hook_gl_id) { File.join(original_root_path, 'spec', 'support', 'gl_id_test_hook') }
 
   let(:vars) { { "GL_ID" => "key_1" } }
   let(:old_value) { "old-value" }
@@ -20,10 +19,6 @@ describe GitlabCustomHook do
 
   let(:gitlab_custom_hook) { GitlabCustomHook.new(tmp_repo_path, 'key_1') }
 
-  before do
-    allow(ENV).to receive(:fetch).with('GITALY_GITLAB_SHELL_CONFIG', '{}').and_return(gitaly_config_data.to_json)
-  end
-
   def hook_path(path)
     File.join(tmp_repo_path, path.split('/'))
   end
@@ -102,6 +97,9 @@ describe GitlabCustomHook do
     end
 
     FileUtils.symlink(File.join(tmp_root_path, 'hooks'), File.join(tmp_repo_path, 'hooks'))
+    FileUtils.symlink(File.join(ROOT_PATH, 'config.yml.example'), File.join(tmp_root_path, 'config.yml'))
+
+    stub_const('ROOT_PATH', Pathname.new(tmp_root_path))
   end
 
   after do
diff --git a/ruby/gitlab-shell/spec/support/gitlab_shell_setup.rb b/ruby/gitlab-shell/spec/support/gitlab_shell_setup.rb
index d74dea717..eddd2d141 100644
--- a/ruby/gitlab-shell/spec/support/gitlab_shell_setup.rb
+++ b/ruby/gitlab-shell/spec/support/gitlab_shell_setup.rb
@@ -1,6 +1,6 @@
 RSpec.shared_context 'gitlab shell', shared_context: :metadata do
   def original_root_path
-    File.dirname(__dir__)
+    ROOT_PATH
   end
 
   def config_path