Merge branch 'glensc/gitlab-shell-pr-245' into 'master'

Chained global hooks

Closes #32. Docs MR: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6721

See merge request !111

10 files changed, 308 insertions, 41 deletions

diff --git a/.gitignore b/.gitignore
index 62e2cd1..995e19d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -8,3 +8,4 @@ coverage/
 .bundle
 tags
 .bundle/
+custom_hooks
diff --git a/CHANGELOG b/CHANGELOG
index e520a99..0caca15 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,9 @@
+v4.1.0
+  - Add support for global custom hooks and chained hook directories (Elan Ruusamäe, Dirk Hörner), !93, !89, #32
+
 v4.0.3
   - Fetch repositories with `--prune` option by default
+
 v4.0.2
   - Fix gitlab_custom_hook dependencies
 
diff --git a/Gemfile.lock b/Gemfile.lock
index 0fbade8..9bf7d9f 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -81,6 +81,3 @@ DEPENDENCIES
   rubocop (= 0.28.0)
   vcr
   webmock
-
-BUNDLED WITH
-   1.11.2
diff --git a/hooks/post-receive b/hooks/post-receive
index b84d0d1..7877306 100755
--- a/hooks/post-receive
+++ b/hooks/post-receive
@@ -11,7 +11,7 @@ require_relative '../lib/gitlab_custom_hook'
 require_relative '../lib/gitlab_post_receive'
 
 if GitlabPostReceive.new(repo_path, key_id, refs).exec &&
-    GitlabCustomHook.new(key_id).post_receive(refs, repo_path)
+    GitlabCustomHook.new(repo_path, key_id).post_receive(refs)
   exit 0
 else
   exit 1
diff --git a/hooks/pre-receive b/hooks/pre-receive
index 4d9a4e9..1b16fd0 100755
--- a/hooks/pre-receive
+++ b/hooks/pre-receive
@@ -17,7 +17,7 @@ require_relative '../lib/gitlab_access'
 # other hand, we run GitlabPostReceive first because the push is already done
 # and we don't want to skip it if the custom hook fails.
 if GitlabAccess.new(repo_path, key_id, refs, protocol).exec &&
-    GitlabCustomHook.new(key_id).pre_receive(refs, repo_path) &&
+    GitlabCustomHook.new(repo_path, key_id).pre_receive(refs) &&
     GitlabReferenceCounter.new(repo_path).increase
   exit 0
 else
diff --git a/hooks/update b/hooks/update
index 223575d..4c2fc08 100755
--- a/hooks/update
+++ b/hooks/update
@@ -11,7 +11,7 @@ key_id    = ENV.delete('GL_ID')
 
 require_relative '../lib/gitlab_custom_hook'
 
-if GitlabCustomHook.new(key_id).update(ref_name, old_value, new_value, repo_path)
+if GitlabCustomHook.new(repo_path, key_id).update(ref_name, old_value, new_value)
   exit 0
 else
   exit 1
diff --git a/lib/gitlab_custom_hook.rb b/lib/gitlab_custom_hook.rb
index 25385cf..a48ad12 100644
--- a/lib/gitlab_custom_hook.rb
+++ b/lib/gitlab_custom_hook.rb
@@ -5,29 +5,33 @@ require_relative 'gitlab_metrics'
 class GitlabCustomHook
   attr_reader :vars
 
-  def initialize(key_id)
+  def initialize(repo_path, key_id)
+    @repo_path = repo_path
     @vars = { 'GL_ID' => key_id }
   end
 
-  def pre_receive(changes, repo_path)
-    hook = hook_file('pre-receive', repo_path)
-    return true if hook.nil?
-
-    GitlabMetrics.measure("pre-receive-hook") { call_receive_hook(hook, changes) }
+  def pre_receive(changes)
+    GitlabMetrics.measure("pre-receive-hook") do
+      find_hooks('pre-receive').all? do |hook|
+        call_receive_hook(hook, changes)
+      end
+    end
   end
 
-  def post_receive(changes, repo_path)
-    hook = hook_file('post-receive', repo_path)
-    return true if hook.nil?
-
-    GitlabMetrics.measure("post-receive-hook") { call_receive_hook(hook, changes) }
+  def post_receive(changes)
+    GitlabMetrics.measure("post-receive-hook") do
+      find_hooks('post-receive').all? do |hook|
+        call_receive_hook(hook, changes)
+      end
+    end
   end
 
-  def update(ref_name, old_value, new_value, repo_path)
-    hook = hook_file('update', repo_path)
-    return true if hook.nil?
-
-    GitlabMetrics.measure("update-hook") { system(vars, hook, ref_name, old_value, new_value) }
+  def update(ref_name, old_value, new_value)
+    GitlabMetrics.measure("update-hook") do
+      find_hooks('update').all? do |hook|
+        system(vars, hook, ref_name, old_value, new_value)
+      end
+    end
   end
 
   private
@@ -53,9 +57,40 @@ class GitlabCustomHook
     $?.success?
   end
 
-  def hook_file(hook_type, repo_path)
-    hook_path = File.join(repo_path.strip, 'custom_hooks')
-    hook_file = "#{hook_path}/#{hook_type}"
-    hook_file if File.exist?(hook_file)
+  # lookup hook files in this order:
+  #
+  # 1. <repository>.git/custom_hooks/<hook_name> - per project hook
+  # 2. <repository>.git/custom_hooks/<hook_name>.d/* - per project hooks
+  # 3. <repository>.git/hooks/<hook_name>.d/* - global hooks
+  #
+  def find_hooks(hook_name)
+    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)
+
+    # <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)
+
+    # <repository>.git/hooks/<hook_name>.d/*
+    hook_path = File.join(@repo_path, 'hooks', "#{hook_name}.d")
+    hook_files += match_hook_files(hook_path)
+
+    hook_files
+  end
+
+  # match files from path:
+  # 1. file must be executable
+  # 2. file must not match backup file
+  #
+  # the resulting list is sorted
+  def match_hook_files(path)
+    return [] unless Dir.exist?(path)
+
+    Dir["#{path}/*"].select do |f|
+      !f.end_with?('~') && File.executable?(f)
+    end.sort
   end
 end
diff --git a/spec/gitlab_custom_hook_spec.rb b/spec/gitlab_custom_hook_spec.rb
index f93c8b4..b5be8ec 100644
--- a/spec/gitlab_custom_hook_spec.rb
+++ b/spec/gitlab_custom_hook_spec.rb
@@ -1,33 +1,257 @@
 # coding: utf-8
 require 'spec_helper'
-require 'pry'
 require 'gitlab_custom_hook'
 
 describe GitlabCustomHook do
-  let(:gitlab_custom_hook) { GitlabCustomHook.new('key_1') }
-  let(:hook_path) { File.join(ROOT_PATH, 'spec/support/gl_id_test_hook') }
+  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') }
 
-  context 'pre_receive hook' do
-    it 'passes GL_ID variable to hook' do
-      allow(gitlab_custom_hook).to receive(:hook_file).and_return(hook_path)
+  let(:vars) { {"GL_ID" => "key_1"} }
+  let(:old_value) { "old-value" }
+  let(:new_value) { "new-value" }
+  let(:ref_name) { "name/of/ref" }
+  let(:changes) { "#{old_value} #{new_value} #{ref_name}\n" }
 
-      expect(gitlab_custom_hook.pre_receive('changes', 'repo_path')).to be_true
+  let(:gitlab_custom_hook) { GitlabCustomHook.new(tmp_repo_path, 'key_1') }
+
+  def hook_path(path)
+    File.join(tmp_repo_path, path.split('/'))
+  end
+
+  def create_hook(path, which)
+    FileUtils.ln_sf(which, hook_path(path))
+  end
+
+  # global hooks multiplexed
+  def create_global_hooks_d(which, hook_name = 'hook')
+    create_hook('hooks/pre-receive.d/' + hook_name, which)
+    create_hook('hooks/update.d/' + hook_name, which)
+    create_hook('hooks/post-receive.d/' + hook_name, which)
+  end
+
+  # repo hooks
+  def create_repo_hooks(which)
+    create_hook('custom_hooks/pre-receive', which)
+    create_hook('custom_hooks/update', which)
+    create_hook('custom_hooks/post-receive', which)
+  end
+
+  # repo hooks multiplexed
+  def create_repo_hooks_d(which, hook_name = 'hook')
+    create_hook('custom_hooks/pre-receive.d/' + hook_name, which)
+    create_hook('custom_hooks/update.d/' + hook_name, which)
+    create_hook('custom_hooks/post-receive.d/' + hook_name, which)
+  end
+
+  def cleanup_hook_setup
+    FileUtils.rm_rf(File.join(tmp_repo_path))
+    FileUtils.rm_rf(File.join(tmp_root_path, 'hooks'))
+  end
+
+  def expect_call_receive_hook(path)
+    expect(gitlab_custom_hook)
+      .to receive(:call_receive_hook)
+      .with(hook_path(path), changes)
+      .and_call_original
+  end
+
+  def expect_call_update_hook(path)
+    expect(gitlab_custom_hook)
+      .to receive(:system)
+      .with(vars, hook_path(path), ref_name, old_value, new_value)
+      .and_call_original
+  end
+
+  # setup paths
+  # <repository>.git/hooks/ - symlink to gitlab-shell/hooks global dir
+  # <repository>.git/hooks/<hook_name> - executed by git itself, this is gitlab-shell/hooks/<hook_name>
+  # <repository>.git/hooks/<hook_name>.d/* - global hooks: all executable files (minus editor backup files)
+  # <repository>.git/custom_hooks/<hook_name> - per project hook (this is already existing behavior)
+  # <repository>.git/custom_hooks/<hook_name>.d/* - per project hooks
+  #
+  # custom hooks are invoked in such way that first failure prevents other scripts being ran
+  # as global scripts are ran first, failing global skips repo hooks
+
+  before 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'))
+
+    FileUtils.symlink(File.join(tmp_root_path, 'hooks'), File.join(tmp_repo_path, 'hooks'))
+  end
+
+  after do
+    cleanup_hook_setup
+  end
+
+  context 'with gl_id_test_hook as repo hook' do
+    before do
+      create_repo_hooks(hook_gl_id)
+    end
+
+    context 'pre_receive hook' do
+      it 'passes GL_ID variable to hook' do
+        expect(gitlab_custom_hook.pre_receive(changes)).to eq(true)
+      end
+    end
+
+    context 'post_receive hook' do
+      it 'passes GL_ID variable to hook' do
+        expect(gitlab_custom_hook.post_receive(changes)).to eq(true)
+      end
+    end
+
+    context 'update hook' do
+      it 'passes GL_ID variable to hook' do
+        expect(gitlab_custom_hook.update(ref_name, old_value, new_value)).to eq(true)
+      end
     end
   end
 
-  context 'post_receive hook' do
-    it 'passes GL_ID variable to hook' do
-      allow(gitlab_custom_hook).to receive(:hook_file).and_return(hook_path)
+  context 'with gl_id_test_hook as global hook' do
+    before do
+      create_global_hooks_d(hook_gl_id)
+    end
+
+    context 'pre_receive hook' do
+      it 'passes GL_ID variable to hook' do
+        expect(gitlab_custom_hook.pre_receive(changes)).to eq(true)
+      end
+    end
 
-      expect(gitlab_custom_hook.post_receive('changes', 'repo_path')).to be_true
+    context 'post_receive hook' do
+      it 'passes GL_ID variable to hook' do
+        expect(gitlab_custom_hook.post_receive(changes)).to eq(true)
+      end
+    end
+
+    context 'update hook' do
+      it 'passes GL_ID variable to hook' do
+        expect(gitlab_custom_hook.update(ref_name, old_value, new_value)).to eq(true)
+      end
+    end
+  end
+
+  context "having no hooks" do
+    it "returns true" do
+      expect(gitlab_custom_hook.pre_receive(changes)).to eq(true)
+      expect(gitlab_custom_hook.update(ref_name, old_value, new_value)).to eq(true)
+      expect(gitlab_custom_hook.post_receive(changes)).to eq(true)
+    end
+  end
+
+  context "having only successful repo hooks" do
+    before do
+      create_repo_hooks(hook_ok)
+    end
+
+    it "returns true" do
+      expect(gitlab_custom_hook.pre_receive(changes)).to eq(true)
+      expect(gitlab_custom_hook.update(ref_name, old_value, new_value)).to eq(true)
+      expect(gitlab_custom_hook.post_receive(changes)).to eq(true)
+    end
+  end
+
+  context "having both successful repo and global hooks" do
+    before do
+      create_repo_hooks(hook_ok)
+      create_global_hooks_d(hook_ok)
+    end
+
+    it "returns true" do
+      expect(gitlab_custom_hook.pre_receive(changes)).to eq(true)
+      expect(gitlab_custom_hook.update(ref_name, old_value, new_value)).to eq(true)
+      expect(gitlab_custom_hook.post_receive(changes)).to eq(true)
+    end
+  end
+
+  context "having failing repo and successful global hooks" do
+    before do
+      create_repo_hooks_d(hook_fail)
+      create_global_hooks_d(hook_ok)
+    end
+
+    it "returns false" do
+      expect(gitlab_custom_hook.pre_receive(changes)).to eq(false)
+      expect(gitlab_custom_hook.update(ref_name, old_value, new_value)).to eq(false)
+      expect(gitlab_custom_hook.post_receive(changes)).to eq(false)
+    end
+
+    it "only executes the global hook" do
+      expect_call_receive_hook("custom_hooks/pre-receive.d/hook")
+      expect_call_update_hook("custom_hooks/update.d/hook")
+      expect_call_receive_hook("custom_hooks/post-receive.d/hook")
+
+      gitlab_custom_hook.pre_receive(changes)
+      gitlab_custom_hook.update(ref_name, old_value, new_value)
+      gitlab_custom_hook.post_receive(changes)
     end
   end
 
-  context 'update hook' do
-    it 'passes GL_ID variable to hook' do
-      allow(gitlab_custom_hook).to receive(:hook_file).and_return(hook_path)
+  context "having successful repo but failing global hooks" do
+    before do
+      create_repo_hooks_d(hook_ok)
+      create_global_hooks_d(hook_fail)
+    end
+
+    it "returns false" do
+      expect(gitlab_custom_hook.pre_receive(changes)).to eq(false)
+      expect(gitlab_custom_hook.update(ref_name, old_value, new_value)).to eq(false)
+      expect(gitlab_custom_hook.post_receive(changes)).to eq(false)
+    end
+
+    it "executes the relevant hooks" do
+      expect_call_receive_hook("hooks/pre-receive.d/hook")
+      expect_call_receive_hook("custom_hooks/pre-receive.d/hook")
+      expect_call_update_hook("hooks/update.d/hook")
+      expect_call_update_hook("custom_hooks/update.d/hook")
+      expect_call_receive_hook("hooks/post-receive.d/hook")
+      expect_call_receive_hook("custom_hooks/post-receive.d/hook")
+
+      gitlab_custom_hook.pre_receive(changes)
+      gitlab_custom_hook.update(ref_name, old_value, new_value)
+      gitlab_custom_hook.post_receive(changes)
+    end
+  end
+
+  context "executing hooks in expected order" do
+    before do
+      create_repo_hooks_d(hook_ok, '01-test')
+      create_repo_hooks_d(hook_ok, '02-test')
+      create_global_hooks_d(hook_ok, '03-test')
+      create_global_hooks_d(hook_ok, '04-test')
+    end
+
+    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_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_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(gitlab_custom_hook.update('master', '', '', 'repo_path')).to be_true
+      gitlab_custom_hook.pre_receive(changes)
+      gitlab_custom_hook.update(ref_name, old_value, new_value)
+      gitlab_custom_hook.post_receive(changes)
     end
   end
 end
diff --git a/spec/support/hook_fail b/spec/support/hook_fail
new file mode 100755
index 0000000..4420796
--- /dev/null
+++ b/spec/support/hook_fail
@@ -0,0 +1,3 @@
+#!/bin/bash
+#echo "fail: $0"
+exit 1
diff --git a/spec/support/hook_ok b/spec/support/hook_ok
new file mode 100755
index 0000000..eb1e3bc
--- /dev/null
+++ b/spec/support/hook_ok
@@ -0,0 +1,3 @@
+#!/bin/bash
+#echo "ok:   $0"
+exit 0