Merge branch 'pass-correct-tag-target-to-post-receive' into 'master'

Pass correct tag target to post-receive hook when creating tag via UI

Fixes #24437

See merge request !7556

3 files changed, 36 insertions, 3 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 146424d2b1c..31be06be50c 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -176,11 +176,18 @@ class Repository
 
     options = { message: message, tagger: user_to_committer(user) } if message
 
-    GitHooksService.new.execute(user, path_to_repo, oldrev, target, ref) do
-      rugged.tags.create(tag_name, target, options)
+    rugged.tags.create(tag_name, target, options)
+    tag = find_tag(tag_name)
+
+    GitHooksService.new.execute(user, path_to_repo, oldrev, tag.target, ref) do
+      # we already created a tag, because we need tag SHA to pass correct
+      # values to hooks
     end
 
-    find_tag(tag_name)
+    tag
+  rescue GitHooksService::PreReceiveError
+    rugged.tags.delete(tag_name)
+    raise
   end
 
   def rm_branch(user, branch_name)
diff --git a/changelogs/unreleased/pass-correct-tag-target-to-post-receive.yml b/changelogs/unreleased/pass-correct-tag-target-to-post-receive.yml
new file mode 100644
index 00000000000..5e868027ed6
--- /dev/null
+++ b/changelogs/unreleased/pass-correct-tag-target-to-post-receive.yml
@@ -0,0 +1,4 @@
+---
+title: Pass correct tag target to post-receive hook when creating tag via UI
+merge_request: 7556
+author:
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 2470d504c68..72ac41f3472 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1354,6 +1354,28 @@ describe Repository, models: true do
         repository.add_tag(user, '8.5', 'master', 'foo')
       end
 
+      it 'does not create a tag when a pre-hook fails' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
+
+        expect do
+          repository.add_tag(user, '8.5', 'master', 'foo')
+        end.to raise_error(GitHooksService::PreReceiveError)
+
+        repository.expire_tags_cache
+        expect(repository.find_tag('8.5')).to be_nil
+      end
+
+      it 'passes tag SHA to hooks' do
+        spy = GitHooksService.new
+        allow(GitHooksService).to receive(:new).and_return(spy)
+        allow(spy).to receive(:execute).and_call_original
+
+        tag = repository.add_tag(user, '8.5', 'master', 'foo')
+
+        expect(spy).to have_received(:execute).
+          with(anything, anything, anything, tag.target, anything)
+      end
+
       it 'returns a Gitlab::Git::Tag object' do
         tag = repository.add_tag(user, '8.5', 'master', 'foo')