diff options
author | Rémy Coutable <remy@rymai.me> | 2016-12-06 16:43:29 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-12-06 16:43:29 +0000 |
commit | e1b0aca197cb61a4ee33b8de37b79c57bb3369ad (patch) | |
tree | cf0170fc1b87fd4f4d28ec2e9860febcd9d89d78 | |
parent | d4f2f5bae9808dd08b67f04cf928dab3a5f04f13 (diff) | |
parent | b77f6530b96c520ef8637ceba4ef2889a008d383 (diff) | |
download | gitlab-shell-e1b0aca197cb61a4ee33b8de37b79c57bb3369ad.tar.gz |
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
-rw-r--r-- | .gitignore | 1 | ||||
-rw-r--r-- | CHANGELOG | 4 | ||||
-rw-r--r-- | Gemfile.lock | 3 | ||||
-rwxr-xr-x | hooks/post-receive | 2 | ||||
-rwxr-xr-x | hooks/pre-receive | 2 | ||||
-rwxr-xr-x | hooks/update | 2 | ||||
-rw-r--r-- | lib/gitlab_custom_hook.rb | 75 | ||||
-rw-r--r-- | spec/gitlab_custom_hook_spec.rb | 254 | ||||
-rwxr-xr-x | spec/support/hook_fail | 3 | ||||
-rwxr-xr-x | spec/support/hook_ok | 3 |
10 files changed, 308 insertions, 41 deletions
@@ -8,3 +8,4 @@ coverage/ .bundle tags .bundle/ +custom_hooks @@ -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 |