Merge branch 'make-custom-hook-dir-configurable' into 'master'

Make custom hook dir configurable

Add a new configuration option, custom_hook_dir. When this is set, we
will look for global custom hooks in: `<custom_hook_dir>/{pre-receive,update,post-receive}.d/*`

When this is not set, default to `<ROOT_PATH>/hooks`.

Relates to https://gitlab.com/gitlab-org/omnibus-gitlab/issues/1754 and
gitlab-org/gitlab-ce!8040.

See merge request !113

.gitignore

@@ -9,3 +9,4 @@ coverage/
 tags
 .bundle/
 custom_hooks
+hooks/*.d

CHANGELOG

 v4.1.0
-  - Add support for global custom hooks and chained hook directories (Elan Ruusamäe, Dirk Hörner), !93, !89, #32
+  - Add support for global custom hooks and chained hook directories (Elan Ruusamäe, Dirk Hörner), !113, !111, !93, !89, #32
   - Clear up text with merge request after new branch push (Lisanne Fellinger)
 
 v4.0.3

VERSION

-4.0.3
+4.1.0

config.yml.example

@@ -28,9 +28,13 @@ http_settings:
 auth_file: "/home/git/.ssh/authorized_keys"
 
 # File that contains the secret key for verifying access to GitLab.
-# Default is .gitlab_shell_secret in the root directory.
+# Default is .gitlab_shell_secret in the gitlab-shell directory.
 # secret_file: "/home/git/gitlab-shell/.gitlab_shell_secret"
 
+# Parent directory for global custom hook directories (pre-receive.d, update.d, post-receive.d)
+# Default is hooks in the gitlab-shell directory.
+# custom_hooks_dir: "/home/git/gitlab-shell/hooks"
+
 # Redis settings used for pushing commit notices to gitlab
 redis:
   bin: /usr/bin/redis-cli

lib/gitlab_config.rb

@@ -19,6 +19,13 @@ class GitlabConfig
     @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)
+    @config['custom_hooks_dir'] || default
+  end
+
   def gitlab_url
     (@config['gitlab_url'] ||= "http://localhost:8080").sub(%r{/*$}, '')
   end

lib/gitlab_custom_hook.rb

@@ -3,11 +3,12 @@ require_relative 'gitlab_init'
 require_relative 'gitlab_metrics'
 
 class GitlabCustomHook
-  attr_reader :vars
+  attr_reader :vars, :config
 
   def initialize(repo_path, key_id)
     @repo_path = repo_path
     @vars = { 'GL_ID' => key_id }
+    @config = GitlabConfig.new
   end
 
   def pre_receive(changes)
@@ -67,16 +68,17 @@ class GitlabCustomHook
     hook_files = []
 
     # <repository>.git/custom_hooks/<hook_name>
-    hook_file = File.join(@repo_path, 'custom_hooks', hook_name)
-    hook_files.push(hook_file) if File.executable?(hook_file)
+    project_custom_hook_file = File.join(@repo_path, 'custom_hooks', hook_name)
+    hook_files.push(project_custom_hook_file) if File.executable?(project_custom_hook_file)
 
     # <repository>.git/custom_hooks/<hook_name>.d/*
-    hook_path = File.join(@repo_path, 'custom_hooks', "#{hook_name}.d")
-    hook_files += match_hook_files(hook_path)
+    project_custom_hooks_dir = File.join(@repo_path, 'custom_hooks', "#{hook_name}.d")
+    hook_files += match_hook_files(project_custom_hooks_dir)
 
-    # <repository>.git/hooks/<hook_name>.d/*
-    hook_path = File.join(@repo_path, 'hooks', "#{hook_name}.d")
-    hook_files += match_hook_files(hook_path)
+    # <repository>.git/hooks/<hook_name>.d/* OR <custom_hook_dir>/<hook_name>.d/*
+    global_custom_hooks_parent = config.custom_hooks_dir(default: File.join(@repo_path, 'hooks'))
+    global_custom_hooks_dir = File.join(global_custom_hooks_parent, "#{hook_name}.d")
+    hook_files += match_hook_files(global_custom_hooks_dir)
 
     hook_files
   end

spec/gitlab_custom_hook_spec.rb

@@ -3,13 +3,15 @@ require 'spec_helper'
 require 'gitlab_custom_hook'
 
 describe GitlabCustomHook do
-  let(:tmp_repo_path) { File.join(ROOT_PATH, 'tmp', 'repo.git') }
-  let(:tmp_root_path) { File.join(ROOT_PATH, 'tmp') }
-  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(:vars) { {"GL_ID" => "key_1"} }
+  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(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" }
   let(:new_value) { "new-value" }
   let(:ref_name) { "name/of/ref" }
@@ -21,6 +23,10 @@ describe GitlabCustomHook do
     File.join(tmp_repo_path, path.split('/'))
   end
 
+  def global_hook_path(path)
+    File.join(tmp_root_path, path.split('/'))
+  end
+
   def create_hook(path, which)
     FileUtils.ln_sf(which, hook_path(path))
   end
@@ -48,7 +54,9 @@ describe GitlabCustomHook do
 
   def cleanup_hook_setup
     FileUtils.rm_rf(File.join(tmp_repo_path))
+    FileUtils.rm_rf(File.join(global_custom_hooks_path))
     FileUtils.rm_rf(File.join(tmp_root_path, 'hooks'))
+    FileUtils.rm_f(File.join(tmp_root_path, 'config.yml'))
   end
 
   def expect_call_receive_hook(path)
@@ -79,16 +87,17 @@ describe GitlabCustomHook do
     cleanup_hook_setup
 
     FileUtils.mkdir_p(File.join(tmp_repo_path, 'custom_hooks'))
-    FileUtils.mkdir_p(File.join(tmp_repo_path, 'custom_hooks', 'update.d'))
-    FileUtils.mkdir_p(File.join(tmp_repo_path, 'custom_hooks', 'pre-receive.d'))
-    FileUtils.mkdir_p(File.join(tmp_repo_path, 'custom_hooks', 'post-receive.d'))
-
     FileUtils.mkdir_p(File.join(tmp_root_path, 'hooks'))
-    FileUtils.mkdir_p(File.join(tmp_root_path, 'hooks', 'update.d'))
-    FileUtils.mkdir_p(File.join(tmp_root_path, 'hooks', 'pre-receive.d'))
-    FileUtils.mkdir_p(File.join(tmp_root_path, 'hooks', 'post-receive.d'))
+
+    ['pre-receive', 'update', 'post-receive'].each do |hook|
+      FileUtils.mkdir_p(File.join(tmp_repo_path, 'custom_hooks', "#{hook}.d"))
+      FileUtils.mkdir_p(File.join(tmp_root_path, 'hooks', "#{hook}.d"))
+    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', tmp_root_path)
   end
 
   after do
@@ -254,4 +263,33 @@ describe GitlabCustomHook do
       gitlab_custom_hook.post_receive(changes)
     end
   end
+
+  context "when the custom_hooks_dir config option is set" do
+    before do
+      allow(gitlab_custom_hook.config).to receive(:custom_hooks_dir).and_return(global_custom_hooks_path)
+
+      FileUtils.mkdir_p(File.join(global_custom_hooks_path, "pre-receive.d"))
+      FileUtils.ln_sf(hook_ok, File.join(global_custom_hooks_path, "pre-receive.d", "hook"))
+
+      create_global_hooks_d(hook_fail)
+    end
+
+    it "finds hooks in that directory" do
+      expect(gitlab_custom_hook)
+        .to receive(:call_receive_hook)
+        .with(global_hook_path("custom_global_hooks/pre-receive.d/hook"), changes)
+        .and_call_original
+
+      expect(gitlab_custom_hook.pre_receive(changes)).to eq(true)
+    end
+
+    it "does not execute hooks in the default location" do
+      expect(gitlab_custom_hook)
+        .not_to receive(:call_receive_hook)
+        .with("hooks/pre-receive.d/hook", changes)
+        .and_call_original
+
+      gitlab_custom_hook.pre_receive(changes)
+    end
+  end
 end