Merge branch 'skip-commit-git-hook-for-tag' into 'master'

Skip git hooks commit validation when pushing new tag.

Addresses gitlab/gitlabhq#2190.

See merge request !386

CHANGELOG-EE

-v 7.11.1
+v 7.11.0
+  - Skip git hooks commit validation when pushing new tag.
+
+v 7.10.1
   - Check if comment exists in Jira before sending a reference
 
 v 7.10.0

lib/gitlab/git_access.rb

@@ -150,14 +150,14 @@ module Gitlab
 
       # Return build_status_object(true) if all git hook checks passed successfully
       # or build_status_object(false) if any hook fails
-      pass_git_hooks?(user, project, ref, oldrev, newrev)
+      git_hook_check(user, project, ref, oldrev, newrev)
     end
 
     def forced_push?(oldrev, newrev)
       Gitlab::ForcePushCheck.force_push?(project, oldrev, newrev)
     end
 
-    def pass_git_hooks?(user, project, ref, oldrev, newrev)
+    def git_hook_check(user, project, ref, oldrev, newrev)
       return build_status_object(true) unless project.git_hook
 
       return build_status_object(true) unless newrev && oldrev
@@ -165,15 +165,14 @@ module Gitlab
       git_hook = project.git_hook
 
       # Prevent tag removal
-      if git_hook.deny_delete_tag
-        if project.repository.tag_names.include?(ref) && newrev =~ /0000000/
+      if Gitlab::Git.tag_ref?(ref)
+        if git_hook.deny_delete_tag && protected_tag?(tag_name(ref)) && Gitlab::Git.blank_ref?(newrev)
           return build_status_object(false, "You can not delete tag")
         end
-      end
-
+      else
         # Check commit messages unless its branch removal
-      if git_hook.commit_validation? && newrev !~ /00000000/
-        if oldrev == Gitlab::Git::BLANK_SHA
+        if git_hook.commit_validation? && !Gitlab::Git.blank_ref?(newrev)
+          if Gitlab::Git.blank_ref?(oldrev)
             oldrev = project.default_branch
           end
 
@@ -189,6 +188,7 @@ module Gitlab
               unless commit.committer_email =~ Regexp.new(git_hook.author_email_regex)
                 return build_status_object(false, "Commiter's email does not follow the pattern")
               end
+
               unless commit.author_email =~ Regexp.new(git_hook.author_email_regex)
                 return build_status_object(false, "Author's email does not follow the pattern")
               end
@@ -199,6 +199,7 @@ module Gitlab
               unless User.existing_member?(commit.author_email)
                 return build_status_object(false, "Author is not a member of team")
               end
+
               if commit.author_email != commit.committer_email
                 unless User.existing_member?(commit.committer_email)
                   return build_status_object(false, "Commiter is not a member of team")
@@ -215,6 +216,7 @@ module Gitlab
             end
           end
         end
+      end
 
       build_status_object(true)
     end

spec/lib/gitlab/git_access_spec.rb

@@ -233,16 +233,22 @@ describe Gitlab::GitAccess do
     end
   end
 
-  describe "pass_git_hooks?" do
+  describe "git_hook_check" do
     describe "author email check" do
       it 'returns true' do
-        access.pass_git_hooks?(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').should be_truthy
+        access.git_hook_check(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').should be_truthy
       end
 
       it 'returns false' do
         project.create_git_hook
         project.git_hook.update(commit_message_regex: "@only.com")
-        access.pass_git_hooks?(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').allowed?.should be_falsey
+        access.git_hook_check(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').allowed?.should be_falsey
+      end
+
+      it 'returns true for tags' do
+        project.create_git_hook
+        project.git_hook.update(commit_message_regex: "@only.com")
+        access.git_hook_check(user, project, 'refs/tags/v1', '6f6d7e7ed', '570e7b2ab').allowed?.should be_truthy
       end
     end
 
@@ -253,12 +259,12 @@ describe Gitlab::GitAccess do
       end
 
       it 'returns false for non-member user' do
-        access.pass_git_hooks?(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').allowed?.should be_falsey
+        access.git_hook_check(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').allowed?.should be_falsey
       end
 
       it 'returns true if committer is a gitlab member' do
         create(:user, email: 'dmitriy.zaporozhets@gmail.com')
-        access.pass_git_hooks?(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').allowed?.should be_truthy
+        access.git_hook_check(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').allowed?.should be_truthy
       end
     end
 
@@ -266,13 +272,13 @@ describe Gitlab::GitAccess do
       it 'returns false when filename is prohibited' do
         project.create_git_hook
         project.git_hook.update(file_name_regex: "jpg$")
-        access.pass_git_hooks?(user, project, 'refs/heads/master', '913c66a37', '33f3729a4').allowed?.should be_falsey
+        access.git_hook_check(user, project, 'refs/heads/master', '913c66a37', '33f3729a4').allowed?.should be_falsey
       end
 
       it 'returns true if file name is allowed' do
         project.create_git_hook
         project.git_hook.update(file_name_regex: "exe$")
-        access.pass_git_hooks?(user, project, 'refs/heads/master', '913c66a37', '33f3729a4').allowed?.should be_truthy
+        access.git_hook_check(user, project, 'refs/heads/master', '913c66a37', '33f3729a4').allowed?.should be_truthy
       end
     end
   end