Merge branch 'jc-change-to-hooks' into 'master'

Move gitlab shell hooks values under [hooks]

See merge request gitlab-org/gitaly!2187

8 files changed, 53 insertions, 43 deletions

diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index bdca5d353..5df748688 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -123,10 +123,10 @@ func TestHooksPrePostReceive(t *testing.T) {
 				defer cleanup()
 
 				if featureSet.IsEnabled("use_gitaly_gitlabshell_config") {
-					config.Config.GitlabShell.GitlabURL = ts.URL
-					config.Config.GitlabShell.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret")
-					config.Config.GitlabShell.HTTPSettings.User = gitlabUser
-					config.Config.GitlabShell.HTTPSettings.Password = gitlabPassword
+					config.Config.Gitlab.URL = ts.URL
+					config.Config.Gitlab.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret")
+					config.Config.Gitlab.HTTPSettings.User = gitlabUser
+					config.Config.Gitlab.HTTPSettings.Password = gitlabPassword
 				}
 
 				var stderr, stdout bytes.Buffer
@@ -218,7 +218,7 @@ func TestHooksUpdate(t *testing.T) {
 	for _, featureSet := range featureSets {
 		t.Run(fmt.Sprintf("enabled features: %v", featureSet), func(t *testing.T) {
 			if featureSet.IsEnabled("use_gitaly_gitlabshell_config") {
-				config.Config.GitlabShell.CustomHooksDir = customHooksDir
+				config.Config.Hooks.CustomHooksDir = customHooksDir
 			}
 
 			testHooksUpdate(t, tempGitlabShellDir, socket, token, testhelper.GlHookValues{
@@ -332,8 +332,8 @@ func TestHooksPostReceiveFailed(t *testing.T) {
 	testhelper.WriteShellSecretFile(t, tempGitlabShellDir, secretToken)
 
 	config.Config.GitlabShell.Dir = tempGitlabShellDir
-	config.Config.GitlabShell.GitlabURL = ts.URL
-	config.Config.GitlabShell.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret")
+	config.Config.Gitlab.URL = ts.URL
+	config.Config.Gitlab.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret")
 
 	featureSets, err := testhelper.NewFeatureSets([]string{featureflag.HooksRPC})
 	require.NoError(t, err)
@@ -416,9 +416,7 @@ func TestHooksNotAllowed(t *testing.T) {
 	testhelper.WriteShellSecretFile(t, tempGitlabShellDir, "the wrong token")
 
 	config.Config.GitlabShell.Dir = tempGitlabShellDir
-
-	config.Config.GitlabShell.Dir = tempGitlabShellDir
-	config.Config.GitlabShell.GitlabURL = ts.URL
+	config.Config.Gitlab.URL = ts.URL
 
 	customHookOutputPath, cleanup := testhelper.WriteEnvToCustomHook(t, testRepoPath, "post-receive")
 	defer cleanup()
diff --git a/config.toml.example b/config.toml.example
index aaab83c21..d89f2da8a 100644
--- a/config.toml.example
+++ b/config.toml.example
@@ -86,11 +86,14 @@ dir = "/home/git/gitaly/ruby"
 [gitlab-shell]
 # The directory where gitlab-shell is installed
 dir = "/home/git/gitlab-shell"
-secret_file = "/home/git/gitlab-shell/.gitlab_shell_secret"
+
+[hooks]
 custom_hooks_dir = "/home/git/custom_hooks"
-gitlab_url = "http://localhost:8080"
 
-[gitlab-shell.http-settings]
+[gitlab]
+secret_file = "/home/git/gitlab-shell/.gitlab_shell_secret"
+gitlab_url = "http://localhost:8080"
+[gitlab.http-settings]
 # read_timeout = 300
 # user = someone
 # password = somepass
diff --git a/internal/config/config.go b/internal/config/config.go
index 0b31aaf00..94005beda 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -43,7 +43,9 @@ type Cfg struct {
 	Auth                       auth.Config       `toml:"auth"`
 	TLS                        TLS               `toml:"tls"`
 	Ruby                       Ruby              `toml:"gitaly-ruby"`
+	Gitlab                     Gitlab            `toml:"gitlab"`
 	GitlabShell                GitlabShell       `toml:"gitlab-shell"`
+	Hooks                      Hooks             `toml:"hooks"`
 	Concurrency                []Concurrency     `toml:"concurrency"`
 	GracefulRestartTimeout     time.Duration
 	GracefulRestartTimeoutToml duration `toml:"graceful_restart_timeout"`
@@ -58,11 +60,19 @@ type TLS struct {
 
 // GitlabShell contains the settings required for executing `gitlab-shell`
 type GitlabShell struct {
-	CustomHooksDir string       `toml:"custom_hooks_dir" json:"custom_hooks_dir"`
-	Dir            string       `toml:"dir" json:"dir"`
-	GitlabURL      string       `toml:"gitlab_url" json:"gitlab_url"`
-	HTTPSettings   HTTPSettings `toml:"http-settings" json:"http_settings"`
-	SecretFile     string       `toml:"secret_file" json:"secret_file"`
+	Dir string `toml:"dir" json:"dir"`
+}
+
+// Gitlab contains settings required to connect to the Gitlab api
+type Gitlab struct {
+	URL          string       `toml:"url" json:"url"`
+	HTTPSettings HTTPSettings `toml:"http-settings" json:"http_settings"`
+	SecretFile   string       `toml:"secret_file" json:"secret_file"`
+}
+
+// Hooks contains the settings required for hooks
+type Hooks struct {
+	CustomHooksDir string `toml:"custom_hooks_dir" json:"custom_hooks_dir"`
 }
 
 type HTTPSettings struct {
@@ -164,8 +174,8 @@ func (c *Cfg) setDefaults() {
 		c.GracefulRestartTimeout = 1 * time.Minute
 	}
 
-	if c.GitlabShell.SecretFile == "" {
-		c.GitlabShell.SecretFile = filepath.Join(c.GitlabShell.Dir, ".gitlab_shell_secret")
+	if c.Gitlab.SecretFile == "" {
+		c.Gitlab.SecretFile = filepath.Join(c.GitlabShell.Dir, ".gitlab_shell_secret")
 	}
 
 	if c.GitlabShell.CustomHooksDir == "" {
diff --git a/internal/gitlabshell/env.go b/internal/gitlabshell/env.go
index 24a1f6296..9165932ab 100644
--- a/internal/gitlabshell/env.go
+++ b/internal/gitlabshell/env.go
@@ -27,14 +27,14 @@ type Config struct {
 // 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.GitlabShell.CustomHooksDir,
-		GitlabURL:      cfg.GitlabShell.GitlabURL,
-		HTTPSettings:   cfg.GitlabShell.HTTPSettings,
+		CustomHooksDir: cfg.Hooks.CustomHooksDir,
+		GitlabURL:      cfg.Gitlab.URL,
+		HTTPSettings:   cfg.Gitlab.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,
+		SecretFile:     cfg.Gitlab.SecretFile,
 	}
 
 	gitlabShellConfigString, err := json.Marshal(&gitlabShellConfig)
diff --git a/internal/gitlabshell/env_test.go b/internal/gitlabshell/env_test.go
index a5b9249c6..9eac8f076 100644
--- a/internal/gitlabshell/env_test.go
+++ b/internal/gitlabshell/env_test.go
@@ -29,11 +29,10 @@ func TestGitHooksConfig(t *testing.T) {
 	config.Config.Logging.Dir = loggingDir
 	config.Config.Logging.Level = "fatal"
 	config.Config.Logging.Format = "my-custom-format"
-
-	config.Config.GitlabShell = config.GitlabShell{
-		CustomHooksDir: "/path/to/custom_hooks",
-		Dir:            "../../ruby/gitlab-shell",
-		GitlabURL:      "http://gitlaburl.com",
+	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",
@@ -63,19 +62,19 @@ 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.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"])
 
 	// 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"])
+	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"])
 
 	dir := filepath.Dir(rubyConfigMap["log_file"].(string))
 	require.Equal(t, config.Config.Logging.Dir, dir)
diff --git a/internal/service/hooks/update.go b/internal/service/hooks/update.go
index 9d0e2091b..5741da00c 100644
--- a/internal/service/hooks/update.go
+++ b/internal/service/hooks/update.go
@@ -83,7 +83,7 @@ func (s *server) UpdateHook(in *gitalypb.UpdateHookRequest, stream gitalypb.Hook
 	if err != nil {
 		return err
 	}
-	executor, err := newCustomHooksExecutor(repoPath, config.Config.GitlabShell.CustomHooksDir, "update")
+	executor, err := newCustomHooksExecutor(repoPath, config.Config.Hooks.CustomHooksDir, "update")
 	if err != nil {
 		return helper.ErrInternal(err)
 	}
diff --git a/internal/service/smarthttp/receive_pack_test.go b/internal/service/smarthttp/receive_pack_test.go
index d60672ade..4abe9b93c 100644
--- a/internal/service/smarthttp/receive_pack_test.go
+++ b/internal/service/smarthttp/receive_pack_test.go
@@ -403,8 +403,8 @@ func testPostReceivePackToHooks(t *testing.T, callRPC bool) {
 
 	testhelper.WriteCustomHook(testRepoPath, "pre-receive", []byte(testhelper.CheckNewObjectExists))
 
-	config.Config.GitlabShell.GitlabURL = ts.URL
-	config.Config.GitlabShell.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret")
+	config.Config.Gitlab.URL = ts.URL
+	config.Config.Gitlab.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret")
 
 	defer func(override string) {
 		hooks.Override = override
diff --git a/internal/service/ssh/receive_pack_test.go b/internal/service/ssh/receive_pack_test.go
index 07a0016c7..cbe2fd2be 100644
--- a/internal/service/ssh/receive_pack_test.go
+++ b/internal/service/ssh/receive_pack_test.go
@@ -243,8 +243,8 @@ func TestSSHReceivePackToHooks(t *testing.T) {
 	testhelper.WriteTemporaryGitlabShellConfigFile(t, tempGitlabShellDir, testhelper.GitlabShellConfig{GitlabURL: ts.URL})
 	testhelper.WriteShellSecretFile(t, tempGitlabShellDir, secretToken)
 
-	config.Config.GitlabShell.GitlabURL = ts.URL
-	config.Config.GitlabShell.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret")
+	config.Config.Gitlab.URL = ts.URL
+	config.Config.Gitlab.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret")
 
 	testhelper.WriteCustomHook(cloneDetails.RemoteRepoPath, "pre-receive", []byte(testhelper.CheckNewObjectExists))