diff --git a/app/views/projects/protected_branches/index.html.haml b/app/views/projects/protected_branches/index.html.haml index 8eaef1f2904abf8c1722ca98fc73c097c2f8e6af..75c27d85e9f6c3fc88117d524acd118ce30eb31a 100644 --- a/app/views/projects/protected_branches/index.html.haml +++ b/app/views/projects/protected_branches/index.html.haml @@ -4,17 +4,17 @@ .col-lg-3 %h4.prepend-top-0 = page_title - %p Keep stable branches secure and force developers to use Merge Requests - .col-lg-9 - %h5.prepend-top-0 - Protect a branch - .account-well.append-bottom-default - %p.light-header.append-bottom-0 Protected branches are designed to + %p Keep stable branches secure and force developers to use merge requests. + %p.prepend-top-20 + Protected branches are designed to: %ul %li prevent pushes from everybody except #{link_to "masters", help_page_path("permissions", "permissions"), class: "vlink"} %li prevent anyone from force pushing to the branch %li prevent anyone from deleting the branch %p.append-bottom-0 Read more about #{link_to "project permissions", help_page_path("permissions", "permissions"), class: "underlined-link"} + .col-lg-9 + %h5.prepend-top-0 + Protect a branch - if can? current_user, :admin_project, @project = form_for [@project.namespace.becomes(Namespace), @project, @protected_branch] do |f| = form_errors(@protected_branch) @@ -35,7 +35,7 @@ = f.label :developers_can_push, "Developers can push", class: "label-light append-bottom-0" %p.light.append-bottom-0 Allow developers to push to this branch - = f.submit "Protect", class: "btn-create btn" + = f.submit "Protect", class: "btn-create btn protect-branch-btn", disabled: true %hr = render "branches_list" diff --git a/lib/gitlab/git/hook.rb b/lib/gitlab/git/hook.rb index 420c6883c45a0913da78ebdb87738cdb1812319f..db87d44735872ac9e2652b2eeb9100b950092151 100644 --- a/lib/gitlab/git/hook.rb +++ b/lib/gitlab/git/hook.rb @@ -14,7 +14,7 @@ module Gitlab end def trigger(gl_id, oldrev, newrev, ref) - return true unless exists? + return [true, nil] unless exists? case name when "pre-receive", "post-receive" @@ -68,13 +68,10 @@ module Gitlab end def call_update_hook(gl_id, oldrev, newrev, ref) - status = nil - Dir.chdir(repo_path) do - status = system({ 'GL_ID' => gl_id }, path, ref, oldrev, newrev) + stdout, stderr, status = Open3.capture3({ 'GL_ID' => gl_id }, path, ref, oldrev, newrev) + [status.success?, stderr.presence || stdout] end - - [status, nil] end def retrieve_error_message(stderr, stdout) diff --git a/spec/lib/gitlab/git/hook_spec.rb b/spec/lib/gitlab/git/hook_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..a15aa173fbd314baf0435024a1ef9e6cfabcc12e --- /dev/null +++ b/spec/lib/gitlab/git/hook_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' +require 'fileutils' + +describe Gitlab::Git::Hook, lib: true do + describe "#trigger" do + let(:project) { create(:project) } + let(:user) { create(:user) } + + def create_hook(name) + FileUtils.mkdir_p(File.join(project.repository.path, 'hooks')) + File.open(File.join(project.repository.path, 'hooks', name), 'w', 0755) do |f| + f.write('exit 0') + end + end + + def create_failing_hook(name) + FileUtils.mkdir_p(File.join(project.repository.path, 'hooks')) + File.open(File.join(project.repository.path, 'hooks', name), 'w', 0755) do |f| + f.write(<<-HOOK) + echo 'regular message from the hook' + echo 'error message from the hook' 1>&2 + exit 1 + HOOK + end + end + + ['pre-receive', 'post-receive', 'update'].each do |hook_name| + + context "when triggering a #{hook_name} hook" do + context "when the hook is successful" do + it "returns success with no errors" do + create_hook(hook_name) + hook = Gitlab::Git::Hook.new(hook_name, project.repository.path) + blank = Gitlab::Git::BLANK_SHA + ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' + + status, errors = hook.trigger(Gitlab::GlId.gl_id(user), blank, blank, ref) + expect(status).to be true + expect(errors).to be_blank + end + end + + context "when the hook is unsuccessful" do + it "returns failure with errors" do + create_failing_hook(hook_name) + hook = Gitlab::Git::Hook.new(hook_name, project.repository.path) + blank = Gitlab::Git::BLANK_SHA + ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' + + status, errors = hook.trigger(Gitlab::GlId.gl_id(user), blank, blank, ref) + expect(status).to be false + expect(errors).to eq("error message from the hook\n") + end + end + end + end + + context "when the hook doesn't exist" do + it "returns success with no errors" do + hook = Gitlab::Git::Hook.new('unknown_hook', project.repository.path) + blank = Gitlab::Git::BLANK_SHA + ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' + + status, errors = hook.trigger(Gitlab::GlId.gl_id(user), blank, blank, ref) + expect(status).to be true + expect(errors).to be_nil + end + end + end +end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 9f9ef20f99b55015692d58f5bd0e92b5d53a2ee3..6b99b0f24cb0cb669c696a10875956007bd53791 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -63,7 +63,7 @@ module TestEnv end def disable_pre_receive - allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true) + allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) end # Clean /tmp/tests