Merge branch 'jv-fix-custom-global-hooks' into 'master'

Fix default lookup of global custom hooks

Closes #1758

See merge request gitlab-org/gitaly!1336

4 files changed, 23 insertions, 16 deletions

diff --git a/changelogs/unreleased/jv-fix-custom-global-hooks.yml b/changelogs/unreleased/jv-fix-custom-global-hooks.yml
new file mode 100644
index 000000000..652dd2682
--- /dev/null
+++ b/changelogs/unreleased/jv-fix-custom-global-hooks.yml
@@ -0,0 +1,5 @@
+---
+title: Fix default lookup of global custom hooks
+merge_request: 1336
+author:
+type: fixed
diff --git a/ruby/gitlab-shell/lib/gitlab_config.rb b/ruby/gitlab-shell/lib/gitlab_config.rb
index a33443783..8779ec18a 100644
--- a/ruby/gitlab-shell/lib/gitlab_config.rb
+++ b/ruby/gitlab-shell/lib/gitlab_config.rb
@@ -14,8 +14,8 @@ class GitlabConfig
   # 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
+  def custom_hooks_dir
+    @config['custom_hooks_dir'] || File.join(ROOT_PATH, 'hooks')
   end
 
   def gitlab_url
diff --git a/ruby/gitlab-shell/lib/gitlab_custom_hook.rb b/ruby/gitlab-shell/lib/gitlab_custom_hook.rb
index 67096dfe7..53069df87 100644
--- a/ruby/gitlab-shell/lib/gitlab_custom_hook.rb
+++ b/ruby/gitlab-shell/lib/gitlab_custom_hook.rb
@@ -76,7 +76,7 @@ class GitlabCustomHook
     hook_files += match_hook_files(project_custom_hooks_dir)
 
     # <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_parent = config.custom_hooks_dir
     global_custom_hooks_dir = File.join(global_custom_hooks_parent, "#{hook_name}.d")
     hook_files += match_hook_files(global_custom_hooks_dir)
 
diff --git a/ruby/gitlab-shell/spec/gitlab_custom_hook_spec.rb b/ruby/gitlab-shell/spec/gitlab_custom_hook_spec.rb
index 540cd2b0a..50865331b 100644
--- a/ruby/gitlab-shell/spec/gitlab_custom_hook_spec.rb
+++ b/ruby/gitlab-shell/spec/gitlab_custom_hook_spec.rb
@@ -59,17 +59,19 @@ describe GitlabCustomHook do
     FileUtils.rm_f(File.join(tmp_root_path, 'config.yml'))
   end
 
-  def expect_call_receive_hook(path)
+  def expect_call_receive_hook(path, global: false)
+    expected_hook_path = global ? global_hook_path(path) : hook_path(path)
     expect(gitlab_custom_hook)
       .to receive(:call_receive_hook)
-      .with(hook_path(path), changes)
+      .with(expected_hook_path, changes)
       .and_call_original
   end
 
-  def expect_call_update_hook(path)
+  def expect_call_update_hook(path, global: false)
+    expected_hook_path = global ? global_hook_path(path) : hook_path(path)
     expect(gitlab_custom_hook)
       .to receive(:system)
-      .with(vars, hook_path(path), ref_name, old_value, new_value)
+      .with(vars, expected_hook_path, ref_name, old_value, new_value)
       .and_call_original
   end
 
@@ -221,11 +223,11 @@ describe GitlabCustomHook do
     end
 
     it "executes the relevant hooks" do
-      expect_call_receive_hook("hooks/pre-receive.d/hook")
+      expect_call_receive_hook("hooks/pre-receive.d/hook", global: true)
       expect_call_receive_hook("custom_hooks/pre-receive.d/hook")
-      expect_call_update_hook("hooks/update.d/hook")
+      expect_call_update_hook("hooks/update.d/hook", global: true)
       expect_call_update_hook("custom_hooks/update.d/hook")
-      expect_call_receive_hook("hooks/post-receive.d/hook")
+      expect_call_receive_hook("hooks/post-receive.d/hook", global: true)
       expect_call_receive_hook("custom_hooks/post-receive.d/hook")
 
       gitlab_custom_hook.pre_receive(changes)
@@ -245,18 +247,18 @@ describe GitlabCustomHook do
     it "executes hooks in order" do
       expect_call_receive_hook("custom_hooks/pre-receive.d/01-test").ordered
       expect_call_receive_hook("custom_hooks/pre-receive.d/02-test").ordered
-      expect_call_receive_hook("hooks/pre-receive.d/03-test").ordered
-      expect_call_receive_hook("hooks/pre-receive.d/04-test").ordered
+      expect_call_receive_hook("hooks/pre-receive.d/03-test", global: true).ordered
+      expect_call_receive_hook("hooks/pre-receive.d/04-test", global: true).ordered
 
       expect_call_update_hook("custom_hooks/update.d/01-test").ordered
       expect_call_update_hook("custom_hooks/update.d/02-test").ordered
-      expect_call_update_hook("hooks/update.d/03-test").ordered
-      expect_call_update_hook("hooks/update.d/04-test").ordered
+      expect_call_update_hook("hooks/update.d/03-test", global: true).ordered
+      expect_call_update_hook("hooks/update.d/04-test", global: true).ordered
 
       expect_call_receive_hook("custom_hooks/post-receive.d/01-test").ordered
       expect_call_receive_hook("custom_hooks/post-receive.d/02-test").ordered
-      expect_call_receive_hook("hooks/post-receive.d/03-test").ordered
-      expect_call_receive_hook("hooks/post-receive.d/04-test").ordered
+      expect_call_receive_hook("hooks/post-receive.d/03-test", global: true).ordered
+      expect_call_receive_hook("hooks/post-receive.d/04-test", global: true).ordered
 
       gitlab_custom_hook.pre_receive(changes)
       gitlab_custom_hook.update(ref_name, old_value, new_value)