Add gitlabshell config values to gitaly tomljc-add-gitlabshell-config-to-gitaly-config

14 files changed, 162 insertions, 45 deletions

diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go
index 552b9a663..3877148ac 100644
--- a/cmd/gitaly-hooks/hooks.go
+++ b/cmd/gitaly-hooks/hooks.go
@@ -256,7 +256,12 @@ func check(configPath string) (int, error) {
 	cmd := exec.Command(filepath.Join(c.GitlabShell.Dir, "bin", "check"))
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
-	cmd.Env = append(os.Environ(), gitlabshell.EnvFromConfig(c)...)
+
+	gitlabshellEnv, err := gitlabshell.EnvFromConfig(c)
+	if err != nil {
+		return 1, err
+	}
+	cmd.Env = append(os.Environ(), gitlabshellEnv...)
 
 	if err = cmd.Run(); err != nil {
 		if status, ok := command.ExitStatus(err); ok {
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index 152da93fd..96f60dad0 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -426,7 +426,7 @@ func TestCheckOK(t *testing.T) {
 	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)
+	configPath, cleanup := testhelper.WriteTemporaryGitalyConfigFile(t, tempDir, ts.URL, user, password)
 	defer cleanup()
 
 	cmd := exec.Command(fmt.Sprintf("%s/gitaly-hooks", config.Config.BinDir), "check", configPath)
@@ -436,6 +436,7 @@ func TestCheckOK(t *testing.T) {
 	cmd.Stdout = &stdout
 
 	require.NoError(t, cmd.Run())
+
 	require.Empty(t, stderr.String())
 	expectedCheckOutput := "Check GitLab API access: OK\nRedis available via internal API: OK\n"
 	require.Equal(t, expectedCheckOutput, stdout.String())
@@ -474,7 +475,7 @@ func TestCheckBadCreds(t *testing.T) {
 	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)
+	configPath, cleanup := testhelper.WriteTemporaryGitalyConfigFile(t, tempDir, ts.URL, user+"wrong", password)
 	defer cleanup()
 
 	cmd := exec.Command(fmt.Sprintf("%s/gitaly-hooks", config.Config.BinDir), "check", configPath)
diff --git a/internal/config/config.go b/internal/config/config.go
index 0f041f2a9..a528fee70 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -58,14 +58,26 @@ type TLS struct {
 
 // GitlabShell contains the settings required for executing `gitlab-shell`
 type GitlabShell struct {
-	Dir string `toml:"dir"`
+	CustomHooksDir string       `toml:"custom_hooks_dir"`
+	Dir            string       `toml:"dir"`
+	GitlabURL      string       `toml:"gitlab_url"`
+	HTTPSettings   HTTPSettings `toml:"http-settings"`
+	SecretFile     string       `toml:"secret_file"`
+}
+
+type HTTPSettings struct {
+	ReadTimeout int    `toml:"read_timeout" json:"read_timeout"`
+	User        string `toml:"user" json:"user"`
+	Password    string `toml:"password" json:"password"`
+	CAFile      string `toml:"ca_file" json:"ca_file"`
+	CAPath      string `toml:"ca_path" json:"ca_path"`
+	SelfSigned  bool   `toml:"self_signed_cert" json:"self_signed_cert"`
 }
 
 // Git contains the settings for the Git executable
 type Git struct {
-	BinPath string `toml:"bin_path"`
-
-	CatfileCacheSize int `toml:"catfile_cache_size"`
+	BinPath          string `toml:"bin_path"`
+	CatfileCacheSize int    `toml:"catfile_cache_size"`
 }
 
 // Storage contains a single storage-shard
diff --git a/internal/git/receivepack.go b/internal/git/receivepack.go
index b321a83b1..b8bf83582 100644
--- a/internal/git/receivepack.go
+++ b/internal/git/receivepack.go
@@ -29,13 +29,17 @@ func HookEnv(req ReceivePackRequest) ([]string, error) {
 		return nil, err
 	}
 
+	gitlabshellEnv, err := gitlabshell.Env()
+	if err != nil {
+		return nil, err
+	}
 	return append([]string{
 		fmt.Sprintf("GL_ID=%s", req.GetGlId()),
 		fmt.Sprintf("GL_USERNAME=%s", req.GetGlUsername()),
 		fmt.Sprintf("GL_REPOSITORY=%s", req.GetGlRepository()),
 		fmt.Sprintf("GITALY_SOCKET=" + config.GitalyInternalSocketPath()),
 		fmt.Sprintf("GITALY_REPO=%s", repo),
-	}, gitlabshell.Env()...), nil
+	}, gitlabshellEnv...), nil
 }
 
 // ReceivePackConfig contains config options we want to enforce when
diff --git a/internal/gitlabshell/env.go b/internal/gitlabshell/env.go
index 4db3c4fd0..f3919e343 100644
--- a/internal/gitlabshell/env.go
+++ b/internal/gitlabshell/env.go
@@ -1,25 +1,52 @@
 package gitlabshell
 
 import (
+	"encoding/json"
+	"fmt"
+
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 )
 
 // Env is a helper that returns a slice with environment variables used by gitlab shell
-func Env() []string {
+func Env() ([]string, error) {
 	cfg := config.Config
 
 	return EnvFromConfig(cfg)
 }
 
 // EnvFromConfig returns a set of environment variables from a config struct relevant to gitlab shell
-func EnvFromConfig(cfg config.Cfg) []string {
+func EnvFromConfig(cfg config.Cfg) ([]string, error) {
+	gitlabShellConfig := Config{
+		CustomHooksDir: cfg.GitlabShell.CustomHooksDir,
+		GitlabURL:      cfg.GitlabShell.GitlabURL,
+		HTTPSettings:   cfg.GitlabShell.HTTPSettings,
+		LogFormat:      cfg.Logging.Format,
+		LogLevel:       cfg.Logging.Level,
+		LogPath:        cfg.Logging.Dir,
+		RootPath:       cfg.GitlabShell.Dir, //GITLAB_SHELL_DIR has been deprecated
+		SecretFile:     cfg.GitlabShell.SecretFile,
+	}
+
+	gitlabShellConfigString, err := json.Marshal(&gitlabShellConfig)
+	if err != nil {
+		return nil, err
+	}
+
 	return []string{
 		"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,
-		"GITLAB_SHELL_DIR=" + cfg.GitlabShell.Dir, //GITLAB_SHELL_DIR has been deprecated
 		"GITALY_BIN_DIR=" + cfg.BinDir,
 		"GITALY_RUBY_DIR=" + cfg.Ruby.Dir,
-	}
+		fmt.Sprintf("GITALY_GITLAB_SHELL_CONFIG=%s", gitlabShellConfigString),
+	}, nil
+}
+
+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"`
+	RootPath       string              `json:"root_path"`
+	SecretFile     string              `json:"secret_file"`
 }
diff --git a/internal/gitlabshell/env_test.go b/internal/gitlabshell/env_test.go
index 809a436c2..aad9699ce 100644
--- a/internal/gitlabshell/env_test.go
+++ b/internal/gitlabshell/env_test.go
@@ -30,13 +30,18 @@ func TestGitHooksConfig(t *testing.T) {
 	config.Config.Logging.Level = "fatal"
 	config.Config.Logging.Format = "my-custom-format"
 	config.Config.GitlabShell.Dir = "../../ruby/gitlab-shell"
+	config.Config.GitlabShell.SecretFile = "secret_file.very_secret"
+	config.Config.GitlabShell.CustomHooksDir = "custom_hooks_directory/is/here"
+	config.Config.GitlabShell.GitlabURL = "http://localhost:1234"
 
 	dumpConfigPath := filepath.Join(config.Config.Ruby.Dir, "gitlab-shell", "bin", "dump-config")
 
 	var stdout bytes.Buffer
 
 	cmd := exec.Command(dumpConfigPath)
-	cmd.Env = append(os.Environ(), gitlabshell.Env()...)
+	gitlabshellEnv, err := gitlabshell.Env()
+	require.NoError(t, err)
+	cmd.Env = append(os.Environ(), gitlabshellEnv...)
 	cmd.Stdout = &stdout
 
 	require.NoError(t, cmd.Run())
@@ -46,6 +51,20 @@ func TestGitHooksConfig(t *testing.T) {
 	require.NoError(t, json.NewDecoder(&stdout).Decode(&rubyConfigMap))
 	require.Equal(t, config.Config.Logging.Level, rubyConfigMap["log_level"])
 	require.Equal(t, config.Config.Logging.Format, rubyConfigMap["log_format"])
+	require.Equal(t, config.Config.GitlabShell.SecretFile, rubyConfigMap["secret_file"])
+	require.Equal(t, config.Config.GitlabShell.CustomHooksDir, rubyConfigMap["custom_hooks_dir"])
+	require.Equal(t, config.Config.GitlabShell.GitlabURL, rubyConfigMap["gitlab_url"])
+	require.Equal(t, config.Config.GitlabShell.SecretFile, rubyConfigMap["secret_file"])
+
+	// HTTP Settings
+	httpSettings, ok := rubyConfigMap["http_settings"].(map[string]interface{})
+	require.True(t, ok)
+	require.Equal(t, float64(config.Config.GitlabShell.HTTPSettings.ReadTimeout), httpSettings["read_timeout"])
+	require.Equal(t, config.Config.GitlabShell.HTTPSettings.User, httpSettings["user"])
+	require.Equal(t, config.Config.GitlabShell.HTTPSettings.Password, httpSettings["password"])
+	require.Equal(t, config.Config.GitlabShell.HTTPSettings.CAFile, httpSettings["ca_file"])
+	require.Equal(t, config.Config.GitlabShell.HTTPSettings.CAPath, httpSettings["ca_path"])
+	require.Equal(t, config.Config.GitlabShell.HTTPSettings.SelfSigned, httpSettings["self_signed_cert"])
 
 	dir := filepath.Dir(rubyConfigMap["log_file"].(string))
 	require.Equal(t, config.Config.Logging.Dir, dir)
diff --git a/internal/rubyserver/rubyserver.go b/internal/rubyserver/rubyserver.go
index d3e8863f8..967d8a19f 100644
--- a/internal/rubyserver/rubyserver.go
+++ b/internal/rubyserver/rubyserver.go
@@ -87,6 +87,12 @@ func (s *Server) start() error {
 	}
 
 	cfg := config.Config
+
+	gitlabshellEnv, err := gitlabshell.Env()
+	if err != nil {
+		return err
+	}
+
 	env := append(
 		os.Environ(),
 		"GITALY_RUBY_GIT_BIN_PATH="+command.GitPath(),
@@ -97,7 +103,7 @@ func (s *Server) start() error {
 		"GITALY_VERSION="+version.GetVersion(),
 		"GITALY_GIT_HOOKS_DIR="+hooks.Path(),
 		"GITALY_RUGGED_GIT_CONFIG_SEARCH_PATH="+cfg.Ruby.RuggedGitConfigSearchPath)
-	env = append(env, gitlabshell.Env()...)
+	env = append(env, gitlabshellEnv...)
 
 	env = append(env, command.GitEnv...)
 
diff --git a/internal/service/hooks/post_receive.go b/internal/service/hooks/post_receive.go
index f4e312d08..9385df70d 100644
--- a/internal/service/hooks/post_receive.go
+++ b/internal/service/hooks/post_receive.go
@@ -20,7 +20,10 @@ func (s *server) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServ
 		return helper.ErrInvalidArgument(err)
 	}
 
-	hookEnv := append(hookRequestEnv(firstRequest), hooks.GitPushOptions(firstRequest.GetGitPushOptions())...)
+	hookEnv, err := hookRequestEnv(firstRequest)
+	if err != nil {
+		return helper.ErrInternal(err)
+	}
 
 	stdin := streamio.NewReader(func() ([]byte, error) {
 		req, err := stream.Recv()
@@ -42,7 +45,7 @@ func (s *server) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServ
 		stdin,
 		stdout, stderr,
 		c,
-		hookEnv,
+		append(hookEnv, hooks.GitPushOptions(firstRequest.GetGitPushOptions())...),
 	)
 
 	if err != nil {
diff --git a/internal/service/hooks/pre_receive.go b/internal/service/hooks/pre_receive.go
index 0564bb71c..a8261115f 100644
--- a/internal/service/hooks/pre_receive.go
+++ b/internal/service/hooks/pre_receive.go
@@ -18,8 +18,12 @@ type hookRequest interface {
 	GetRepository() *gitalypb.Repository
 }
 
-func hookRequestEnv(req hookRequest) []string {
-	return append(gitlabshell.Env(), req.GetEnvironmentVariables()...)
+func hookRequestEnv(req hookRequest) ([]string, error) {
+	gitlabshellEnv, err := gitlabshell.Env()
+	if err != nil {
+		return nil, err
+	}
+	return append(gitlabshellEnv, req.GetEnvironmentVariables()...), nil
 }
 
 func preReceiveEnv(req hookRequest) ([]string, error) {
@@ -27,7 +31,13 @@ func preReceiveEnv(req hookRequest) ([]string, error) {
 	if err != nil {
 		return nil, err
 	}
-	return append(hookRequestEnv(req), env...), nil
+
+	hookEnv, err := hookRequestEnv(req)
+	if err != nil {
+		return nil, err
+	}
+
+	return append(hookEnv, env...), nil
 }
 
 func gitlabShellHook(hookName string) string {
diff --git a/internal/service/hooks/update.go b/internal/service/hooks/update.go
index c980dd92f..626f02ec9 100644
--- a/internal/service/hooks/update.go
+++ b/internal/service/hooks/update.go
@@ -25,12 +25,17 @@ func (s *server) UpdateHook(in *gitalypb.UpdateHookRequest, stream gitalypb.Hook
 	c := exec.Command(gitlabShellHook("update"), string(in.GetRef()), in.GetOldValue(), in.GetNewValue())
 	c.Dir = repoPath
 
+	updateEnv, err := hookRequestEnv(in)
+	if err != nil {
+		return helper.ErrInternal(err)
+	}
+
 	status, err := streamCommandResponse(
 		stream.Context(),
 		nil,
 		stdout, stderr,
 		c,
-		hookRequestEnv(in),
+		updateEnv,
 	)
 
 	if err != nil {
diff --git a/internal/testhelper/testserver.go b/internal/testhelper/testserver.go
index 11ca87255..128a82bb0 100644
--- a/internal/testhelper/testserver.go
+++ b/internal/testhelper/testserver.go
@@ -446,12 +446,16 @@ func WriteTemporaryGitlabShellConfigFile(t TB, dir string, config GitlabShellCon
 
 // 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 TB, tempDir string) (string, func()) {
+func WriteTemporaryGitalyConfigFile(t TB, tempDir, gitlabURL, user, password string) (string, func()) {
 	path := filepath.Join(tempDir, "config.toml")
 	contents := fmt.Sprintf(`
 [gitlab-shell]
   dir = "%s/gitlab-shell"
-`, tempDir)
+  gitlab_url = "%s"
+  [gitlab-shell.http-settings]
+    user = "%s"
+    password = "%s"
+`, tempDir, gitlabURL, user, password)
 	require.NoError(t, ioutil.WriteFile(path, []byte(contents), 0644))
 
 	return path, func() {
diff --git a/ruby/gitlab-shell/lib/gitlab_config.rb b/ruby/gitlab-shell/lib/gitlab_config.rb
index 19f03c346..0d4963c7c 100644
--- a/ruby/gitlab-shell/lib/gitlab_config.rb
+++ b/ruby/gitlab-shell/lib/gitlab_config.rb
@@ -1,45 +1,50 @@
 require 'yaml'
+require 'json'
 
 class GitlabConfig
   def secret_file
-    fetch_from_legacy_config('secret_file',File.join(ROOT_PATH, '.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 custom_hooks_dir(default: nil)
-    fetch_from_legacy_config('custom_hooks_dir', File.join(ROOT_PATH, '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_legacy_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
 
   def http_settings
-    fetch_from_legacy_config('http_settings', {})
+    fetch_from_config('http_settings', fetch_from_legacy_config('http_settings', {}))
   end
 
   def log_file
-    return File.join(LOG_PATH, 'gitlab-shell.log') unless LOG_PATH.empty?
+    log_path = fetch_from_config('log_path', LOG_PATH)
 
-    fetch_from_legacy_config('log_file', File.join(ROOT_PATH, 'gitlab-shell.log'))
+    return File.join(log_path, 'gitlab-shell.log') unless log_path.empty?
+
+    File.join(ROOT_PATH, 'gitlab-shell.log')
   end
 
   def log_level
-    return LOG_LEVEL unless LOG_LEVEL.empty?
+    log_level = fetch_from_config('log_level', LOG_LEVEL)
+
+    log_level = LOG_LEVEL if log_level.empty?
+
+    return log_level unless log_level.empty?
 
-    fetch_from_legacy_config('log_level', 'INFO')
+    'INFO'
   end
 
   def log_format
-    return LOG_FORMAT unless LOG_FORMAT.empty?
+    log_format = fetch_from_config('log_format', LOG_FORMAT)
 
-    fetch_from_legacy_config('log_format', 'text')
-  end
+    return log_format unless log_format.empty?
 
-  def metrics_log_file
-    fetch_from_legacy_config('metrics_log_file', File.join(ROOT_PATH, 'gitlab-shell-metrics.log'))
+    'text'
   end
 
   def to_json
@@ -51,7 +56,6 @@ class GitlabConfig
       log_file: log_file,
       log_level: log_level,
       log_format: log_format,
-      metrics_log_file: metrics_log_file
     }.to_json
   end
 
@@ -61,8 +65,22 @@ class GitlabConfig
 
   private
 
+  def fetch_from_config(key, default='')
+    value = config[key]
+    return value unless value.nil? || value.empty?
+
+    default
+  end
+
+  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_config ||= YAML.load_file(File.join(ROOT_PATH, 'config.yml'))
+    legacy_file = File.join(ROOT_PATH, 'config.yml')
+    return {} unless File.exist?(legacy_file)
+
+    @legacy_config ||= YAML.load_file(legacy_file)
   end
 end
diff --git a/ruby/gitlab-shell/lib/http_helper.rb b/ruby/gitlab-shell/lib/http_helper.rb
index c6a4bb845..5cadb6196 100644
--- a/ruby/gitlab-shell/lib/http_helper.rb
+++ b/ruby/gitlab-shell/lib/http_helper.rb
@@ -120,6 +120,9 @@ module HTTPHelper
   end
 
   def read_timeout
-    config.http_settings['read_timeout'] || READ_TIMEOUT
+    read_timeout = config.http_settings['read_timeout']
+    return read_timeout unless read_timeout == 0
+
+    READ_TIMEOUT
   end
 end
diff --git a/ruby/gitlab-shell/spec/gitlab_config_spec.rb b/ruby/gitlab-shell/spec/gitlab_config_spec.rb
index 6de0cd08e..6114bda31 100644
--- a/ruby/gitlab-shell/spec/gitlab_config_spec.rb
+++ b/ruby/gitlab-shell/spec/gitlab_config_spec.rb
@@ -24,11 +24,11 @@ describe GitlabConfig do
     end
   end
 
-  describe '#log_format' do
-    subject { config.log_format }
+  describe '#secret_file' do
+    subject { config.secret_file }
 
-    it 'returns "text" by default' do
-      is_expected.to eq('text')
+    it 'returns ".gitlab_shell_secret" by default' do
+      is_expected.to eq(File.join(File.expand_path('..', __dir__),'.gitlab_shell_secret'))
     end
   end