Run custom Git hooks when creating or deleting branches through the UI. #1156
This commit is contained in:
parent
b5103a83a8
commit
5145706c82
|
@ -6,6 +6,7 @@ v 8.3.0 (unreleased)
|
||||||
- Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera)
|
- Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera)
|
||||||
- Add ignore whitespace change option to commit view
|
- Add ignore whitespace change option to commit view
|
||||||
- Fire update hook from GitLab
|
- Fire update hook from GitLab
|
||||||
|
- Run custom Git hooks when branch is created or deleted. #1156
|
||||||
|
|
||||||
v 8.2.2
|
v 8.2.2
|
||||||
- Fix 404 in redirection after removing a project (Stan Hu)
|
- Fix 404 in redirection after removing a project (Stan Hu)
|
||||||
|
|
|
@ -1,7 +1,6 @@
|
||||||
require 'securerandom'
|
require 'securerandom'
|
||||||
|
|
||||||
class Repository
|
class Repository
|
||||||
class PreReceiveError < StandardError; end
|
|
||||||
class CommitError < StandardError; end
|
class CommitError < StandardError; end
|
||||||
|
|
||||||
include Gitlab::ShellAdapter
|
include Gitlab::ShellAdapter
|
||||||
|
@ -108,10 +107,19 @@ class Repository
|
||||||
tags.find { |tag| tag.name == name }
|
tags.find { |tag| tag.name == name }
|
||||||
end
|
end
|
||||||
|
|
||||||
def add_branch(branch_name, ref)
|
def add_branch(user, branch_name, target)
|
||||||
expire_branches_cache
|
oldrev = Gitlab::Git::BLANK_SHA
|
||||||
|
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
|
||||||
|
target = commit(target).try(:id)
|
||||||
|
|
||||||
gitlab_shell.add_branch(path_with_namespace, branch_name, ref)
|
return false unless target
|
||||||
|
|
||||||
|
GitHooksService.new.execute(user, path_to_repo, oldrev, target, ref) do
|
||||||
|
rugged.branches.create(branch_name, target)
|
||||||
|
end
|
||||||
|
|
||||||
|
expire_branches_cache
|
||||||
|
find_branch(branch_name)
|
||||||
end
|
end
|
||||||
|
|
||||||
def add_tag(tag_name, ref, message = nil)
|
def add_tag(tag_name, ref, message = nil)
|
||||||
|
@ -120,10 +128,20 @@ class Repository
|
||||||
gitlab_shell.add_tag(path_with_namespace, tag_name, ref, message)
|
gitlab_shell.add_tag(path_with_namespace, tag_name, ref, message)
|
||||||
end
|
end
|
||||||
|
|
||||||
def rm_branch(branch_name)
|
def rm_branch(user, branch_name)
|
||||||
expire_branches_cache
|
expire_branches_cache
|
||||||
|
|
||||||
gitlab_shell.rm_branch(path_with_namespace, branch_name)
|
branch = find_branch(branch_name)
|
||||||
|
oldrev = branch.try(:target)
|
||||||
|
newrev = Gitlab::Git::BLANK_SHA
|
||||||
|
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
|
||||||
|
|
||||||
|
GitHooksService.new.execute(user, path_to_repo, oldrev, newrev, ref) do
|
||||||
|
rugged.branches.delete(branch_name)
|
||||||
|
end
|
||||||
|
|
||||||
|
expire_branches_cache
|
||||||
|
true
|
||||||
end
|
end
|
||||||
|
|
||||||
def rm_tag(tag_name)
|
def rm_tag(tag_name)
|
||||||
|
@ -550,7 +568,6 @@ class Repository
|
||||||
def commit_with_hooks(current_user, branch)
|
def commit_with_hooks(current_user, branch)
|
||||||
oldrev = Gitlab::Git::BLANK_SHA
|
oldrev = Gitlab::Git::BLANK_SHA
|
||||||
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
|
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
|
||||||
gl_id = Gitlab::ShellEnv.gl_id(current_user)
|
|
||||||
was_empty = empty?
|
was_empty = empty?
|
||||||
|
|
||||||
# Create temporary ref
|
# Create temporary ref
|
||||||
|
@ -569,15 +586,7 @@ class Repository
|
||||||
raise CommitError.new('Failed to create commit')
|
raise CommitError.new('Failed to create commit')
|
||||||
end
|
end
|
||||||
|
|
||||||
# Run GitLab pre-receive hook
|
GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do
|
||||||
pre_receive_hook = Gitlab::Git::Hook.new('pre-receive', path_to_repo)
|
|
||||||
pre_receive_hook_status = pre_receive_hook.trigger(gl_id, oldrev, newrev, ref)
|
|
||||||
|
|
||||||
# Run GitLab update hook
|
|
||||||
update_hook = Gitlab::Git::Hook.new('update', path_to_repo)
|
|
||||||
update_hook_status = update_hook.trigger(gl_id, oldrev, newrev, ref)
|
|
||||||
|
|
||||||
if pre_receive_hook_status && update_hook_status
|
|
||||||
if was_empty
|
if was_empty
|
||||||
# Create branch
|
# Create branch
|
||||||
rugged.references.create(ref, newrev)
|
rugged.references.create(ref, newrev)
|
||||||
|
@ -592,16 +601,11 @@ class Repository
|
||||||
raise CommitError.new('Commit was rejected because branch received new push')
|
raise CommitError.new('Commit was rejected because branch received new push')
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# Run GitLab post receive hook
|
|
||||||
post_receive_hook = Gitlab::Git::Hook.new('post-receive', path_to_repo)
|
|
||||||
post_receive_hook.trigger(gl_id, oldrev, newrev, ref)
|
|
||||||
else
|
|
||||||
# Remove tmp ref and return error to user
|
|
||||||
rugged.references.delete(tmp_ref)
|
|
||||||
|
|
||||||
raise PreReceiveError.new('Commit was rejected by git hook')
|
|
||||||
end
|
end
|
||||||
|
rescue GitHooksService::PreReceiveError
|
||||||
|
# Remove tmp ref and return error to user
|
||||||
|
rugged.references.delete(tmp_ref)
|
||||||
|
raise
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
|
@ -13,8 +13,7 @@ class CreateBranchService < BaseService
|
||||||
return error('Branch already exists')
|
return error('Branch already exists')
|
||||||
end
|
end
|
||||||
|
|
||||||
repository.add_branch(branch_name, ref)
|
new_branch = repository.add_branch(current_user, branch_name, ref)
|
||||||
new_branch = repository.find_branch(branch_name)
|
|
||||||
|
|
||||||
if new_branch
|
if new_branch
|
||||||
push_data = build_push_data(project, current_user, new_branch)
|
push_data = build_push_data(project, current_user, new_branch)
|
||||||
|
@ -27,6 +26,8 @@ class CreateBranchService < BaseService
|
||||||
else
|
else
|
||||||
error('Invalid reference name')
|
error('Invalid reference name')
|
||||||
end
|
end
|
||||||
|
rescue GitHooksService::PreReceiveError
|
||||||
|
error('Branch creation was rejected by Git hook')
|
||||||
end
|
end
|
||||||
|
|
||||||
def success(branch)
|
def success(branch)
|
||||||
|
|
|
@ -24,7 +24,7 @@ class DeleteBranchService < BaseService
|
||||||
return error('You dont have push access to repo', 405)
|
return error('You dont have push access to repo', 405)
|
||||||
end
|
end
|
||||||
|
|
||||||
if repository.rm_branch(branch_name)
|
if repository.rm_branch(current_user, branch_name)
|
||||||
push_data = build_push_data(branch)
|
push_data = build_push_data(branch)
|
||||||
|
|
||||||
EventCreateService.new.push(project, current_user, push_data)
|
EventCreateService.new.push(project, current_user, push_data)
|
||||||
|
@ -35,6 +35,8 @@ class DeleteBranchService < BaseService
|
||||||
else
|
else
|
||||||
error('Failed to remove branch')
|
error('Failed to remove branch')
|
||||||
end
|
end
|
||||||
|
rescue GitHooksService::PreReceiveError
|
||||||
|
error('Branch deletion was rejected by Git hook')
|
||||||
end
|
end
|
||||||
|
|
||||||
def error(message, return_code = 400)
|
def error(message, return_code = 400)
|
||||||
|
|
|
@ -26,7 +26,7 @@ module Files
|
||||||
else
|
else
|
||||||
error("Something went wrong. Your changes were not committed")
|
error("Something went wrong. Your changes were not committed")
|
||||||
end
|
end
|
||||||
rescue Repository::CommitError, Repository::PreReceiveError, ValidationError => ex
|
rescue Repository::CommitError, GitHooksService::PreReceiveError, ValidationError => ex
|
||||||
error(ex.message)
|
error(ex.message)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,32 @@
|
||||||
|
class GitHooksService
|
||||||
|
PreReceiveError = Class.new(StandardError)
|
||||||
|
|
||||||
|
def execute(user, repo_path, oldrev, newrev, ref)
|
||||||
|
@repo_path = repo_path
|
||||||
|
@user = Gitlab::ShellEnv.gl_id(user)
|
||||||
|
@oldrev = oldrev
|
||||||
|
@newrev = newrev
|
||||||
|
@ref = ref
|
||||||
|
|
||||||
|
pre_status = run_hook('pre-receive')
|
||||||
|
|
||||||
|
if pre_status
|
||||||
|
yield
|
||||||
|
|
||||||
|
run_hook('post-receive')
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def run_hook(name)
|
||||||
|
hook = Gitlab::Git::Hook.new(name, @repo_path)
|
||||||
|
status = hook.trigger(@user, @oldrev, @newrev, @ref)
|
||||||
|
|
||||||
|
if !status && (name != 'post-receive')
|
||||||
|
raise PreReceiveError.new("Git operation was rejected by #{name} hook")
|
||||||
|
end
|
||||||
|
|
||||||
|
status
|
||||||
|
end
|
||||||
|
end
|
|
@ -4,6 +4,7 @@ describe Repository do
|
||||||
include RepoHelpers
|
include RepoHelpers
|
||||||
|
|
||||||
let(:repository) { create(:project).repository }
|
let(:repository) { create(:project).repository }
|
||||||
|
let(:user) { create(:user) }
|
||||||
|
|
||||||
describe :branch_names_contains do
|
describe :branch_names_contains do
|
||||||
subject { repository.branch_names_contains(sample_commit.id) }
|
subject { repository.branch_names_contains(sample_commit.id) }
|
||||||
|
@ -99,5 +100,104 @@ describe Repository do
|
||||||
it { expect(subject.startline).to eq(186) }
|
it { expect(subject.startline).to eq(186) }
|
||||||
it { expect(subject.data.lines[2]).to eq(" - Feature: Replace teams with group membership\n") }
|
it { expect(subject.data.lines[2]).to eq(" - Feature: Replace teams with group membership\n") }
|
||||||
end
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe :add_branch do
|
||||||
|
context 'when pre hooks were successful' do
|
||||||
|
it 'should run without errors' do
|
||||||
|
hook = double(trigger: true)
|
||||||
|
expect(Gitlab::Git::Hook).to receive(:new).twice.and_return(hook)
|
||||||
|
|
||||||
|
expect { repository.add_branch(user, 'new_feature', 'master') }.not_to raise_error
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'should create the branch' do
|
||||||
|
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true)
|
||||||
|
|
||||||
|
branch = repository.add_branch(user, 'new_feature', 'master')
|
||||||
|
|
||||||
|
expect(branch.name).to eq('new_feature')
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when pre hooks failed' do
|
||||||
|
it 'should get an error' do
|
||||||
|
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
|
||||||
|
|
||||||
|
expect do
|
||||||
|
repository.add_branch(user, 'new_feature', 'master')
|
||||||
|
end.to raise_error(GitHooksService::PreReceiveError)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'should not create the branch' do
|
||||||
|
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
|
||||||
|
|
||||||
|
expect do
|
||||||
|
repository.add_branch(user, 'new_feature', 'master')
|
||||||
|
end.to raise_error(GitHooksService::PreReceiveError)
|
||||||
|
expect(repository.find_branch('new_feature')).to be_nil
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe :rm_branch do
|
||||||
|
context 'when pre hooks were successful' do
|
||||||
|
it 'should run without errors' do
|
||||||
|
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true)
|
||||||
|
|
||||||
|
expect { repository.rm_branch(user, 'feature') }.not_to raise_error
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'should delete the branch' do
|
||||||
|
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true)
|
||||||
|
|
||||||
|
expect { repository.rm_branch(user, 'feature') }.not_to raise_error
|
||||||
|
|
||||||
|
expect(repository.find_branch('feature')).to be_nil
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when pre hooks failed' do
|
||||||
|
it 'should get an error' do
|
||||||
|
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
|
||||||
|
|
||||||
|
expect do
|
||||||
|
repository.rm_branch(user, 'new_feature')
|
||||||
|
end.to raise_error(GitHooksService::PreReceiveError)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'should not delete the branch' do
|
||||||
|
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
|
||||||
|
|
||||||
|
expect do
|
||||||
|
repository.rm_branch(user, 'feature')
|
||||||
|
end.to raise_error(GitHooksService::PreReceiveError)
|
||||||
|
expect(repository.find_branch('feature')).not_to be_nil
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe :commit_with_hooks do
|
||||||
|
context 'when pre hooks were successful' do
|
||||||
|
it 'should run without errors' do
|
||||||
|
expect_any_instance_of(GitHooksService).to receive(:execute).and_return(true)
|
||||||
|
|
||||||
|
expect do
|
||||||
|
repository.commit_with_hooks(user, 'feature') { sample_commit.id }
|
||||||
|
end.not_to raise_error
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when pre hooks failed' do
|
||||||
|
it 'should get an error' do
|
||||||
|
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
|
||||||
|
|
||||||
|
expect do
|
||||||
|
repository.commit_with_hooks(user, 'feature') { sample_commit.id }
|
||||||
|
end.to raise_error(GitHooksService::PreReceiveError)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|
|
@ -0,0 +1,38 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
|
||||||
|
describe GitHooksService do
|
||||||
|
include RepoHelpers
|
||||||
|
|
||||||
|
let(:user) { create :user }
|
||||||
|
let(:project) { create :project }
|
||||||
|
let(:service) { GitHooksService.new }
|
||||||
|
|
||||||
|
before do
|
||||||
|
@blankrev = Gitlab::Git::BLANK_SHA
|
||||||
|
@oldrev = sample_commit.parent_id
|
||||||
|
@newrev = sample_commit.id
|
||||||
|
@ref = 'refs/heads/feature'
|
||||||
|
@repo_path = project.repository.path_to_repo
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '#execute' do
|
||||||
|
|
||||||
|
context 'when pre hooks were successful' do
|
||||||
|
it 'should call post hooks' do
|
||||||
|
expect(service).to receive(:run_hook).with('pre-receive').and_return(true)
|
||||||
|
expect(service).to receive(:run_hook).with('post-receive').and_return(true)
|
||||||
|
expect(service.execute(user, @repo_path, @blankrev, @newrev, @ref) { }).to eq(true)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when pre hooks failed' do
|
||||||
|
it 'should not call post hooks' do
|
||||||
|
expect(service).to receive(:run_hook).with('pre-receive').and_return(false)
|
||||||
|
expect(service).not_to receive(:run_hook).with('post-receive')
|
||||||
|
|
||||||
|
service.execute(user, @repo_path, @blankrev, @newrev, @ref)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
end
|
||||||
|
end
|
Loading…
Reference in New Issue