Fix remote mirrors not updating after tag pushsh-fix-tag-push-remote-mirror

Remote mirrors were only being updated after pushes to branches, not
tags. This change consolidates the functionality into
Git::BaseHooksService so that both tags and branches will now update
remote mirrors.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/51240

7 files changed, 116 insertions, 67 deletions

diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb
index 9d371e234ee..d30df34e54b 100644
--- a/app/services/git/base_hooks_service.rb
+++ b/app/services/git/base_hooks_service.rb
@@ -17,6 +17,8 @@ module Git
       # Not a hook, but it needs access to the list of changed commits
       enqueue_invalidate_cache
 
+      update_remote_mirrors
+
       push_data
     end
 
@@ -92,5 +94,12 @@ module Git
     def pipeline_options
       {}
     end
+
+    def update_remote_mirrors
+      return unless project.has_remote_mirror?
+
+      project.mark_stuck_remote_mirrors_as_failed!
+      project.update_remote_mirrors
+    end
   end
 end
diff --git a/app/services/git/branch_push_service.rb b/app/services/git/branch_push_service.rb
index abf11f253f6..c4910180787 100644
--- a/app/services/git/branch_push_service.rb
+++ b/app/services/git/branch_push_service.rb
@@ -27,7 +27,6 @@ module Git
       execute_related_hooks
       perform_housekeeping
 
-      update_remote_mirrors
       stop_environments
 
       true
diff --git a/changelogs/unreleased/sh-fix-tag-push-remote-mirror.yml b/changelogs/unreleased/sh-fix-tag-push-remote-mirror.yml
new file mode 100644
index 00000000000..7f33ab28e3d
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-tag-push-remote-mirror.yml
@@ -0,0 +1,5 @@
+---
+title: Fix remote mirrors not updating after tag push
+merge_request:
+author:
+type: fixed
diff --git a/spec/services/git/base_hooks_service_spec.rb b/spec/services/git/base_hooks_service_spec.rb
new file mode 100644
index 00000000000..4a2ec769116
--- /dev/null
+++ b/spec/services/git/base_hooks_service_spec.rb
@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Git::BaseHooksService do
+  include RepoHelpers
+  include GitHelpers
+
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :repository) }
+  let(:service) { described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) }
+
+  let(:oldrev) { Gitlab::Git::BLANK_SHA }
+  let(:newrev) { "8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b" } # gitlab-test: git rev-parse refs/tags/v1.1.0
+  let(:ref) { 'refs/tags/v1.1.0' }
+
+  describe 'with remote mirrors' do
+    class TestService < described_class
+      def commits
+        []
+      end
+    end
+
+    let(:project) { create(:project, :repository, :remote_mirror) }
+
+    subject { TestService.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) }
+
+    before do
+      expect(subject).to receive(:execute_project_hooks)
+    end
+
+    context 'when remote mirror feature is enabled' do
+      it 'fails stuck remote mirrors' do
+        allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors)
+        expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!)
+
+        subject.execute
+      end
+
+      it 'updates remote mirrors' do
+        expect(project).to receive(:update_remote_mirrors)
+
+        subject.execute
+      end
+    end
+
+    context 'when remote mirror feature is disabled' do
+      before do
+        stub_application_setting(mirror_available: false)
+      end
+
+      context 'with remote mirrors global setting overridden' do
+        before do
+          project.remote_mirror_available_overridden = true
+        end
+
+        it 'fails stuck remote mirrors' do
+          allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors)
+          expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!)
+
+          subject.execute
+        end
+
+        it 'updates remote mirrors' do
+          expect(project).to receive(:update_remote_mirrors)
+
+          subject.execute
+        end
+      end
+
+      context 'without remote mirrors global setting overridden' do
+        before do
+          project.remote_mirror_available_overridden = false
+        end
+
+        it 'does not fails stuck remote mirrors' do
+          expect(project).not_to receive(:mark_stuck_remote_mirrors_as_failed!)
+
+          subject.execute
+        end
+
+        it 'does not updates remote mirrors' do
+          expect(project).not_to receive(:update_remote_mirrors)
+
+          subject.execute
+        end
+      end
+    end
+  end
+end
diff --git a/spec/services/git/branch_hooks_service_spec.rb b/spec/services/git/branch_hooks_service_spec.rb
index 4895e762602..22faa996015 100644
--- a/spec/services/git/branch_hooks_service_spec.rb
+++ b/spec/services/git/branch_hooks_service_spec.rb
@@ -18,6 +18,12 @@ describe Git::BranchHooksService do
     described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
   end
 
+  it 'update remote mirrors' do
+    expect(service).to receive(:update_remote_mirrors).and_call_original
+
+    service.execute
+  end
+
   describe "Git Push Data" do
     subject(:push_data) { service.execute }
 
diff --git a/spec/services/git/branch_push_service_spec.rb b/spec/services/git/branch_push_service_spec.rb
index ad21f710833..6e39fa6b3c0 100644
--- a/spec/services/git/branch_push_service_spec.rb
+++ b/spec/services/git/branch_push_service_spec.rb
@@ -17,72 +17,6 @@ describe Git::BranchPushService, services: true do
     project.add_maintainer(user)
   end
 
-  describe 'with remote mirrors' do
-    let(:project) { create(:project, :repository, :remote_mirror) }
-
-    subject do
-      described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
-    end
-
-    context 'when remote mirror feature is enabled' do
-      it 'fails stuck remote mirrors' do
-        allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors)
-        expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!)
-
-        subject.execute
-      end
-
-      it 'updates remote mirrors' do
-        expect(project).to receive(:update_remote_mirrors)
-
-        subject.execute
-      end
-    end
-
-    context 'when remote mirror feature is disabled' do
-      before do
-        stub_application_setting(mirror_available: false)
-      end
-
-      context 'with remote mirrors global setting overridden' do
-        before do
-          project.remote_mirror_available_overridden = true
-        end
-
-        it 'fails stuck remote mirrors' do
-          allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors)
-          expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!)
-
-          subject.execute
-        end
-
-        it 'updates remote mirrors' do
-          expect(project).to receive(:update_remote_mirrors)
-
-          subject.execute
-        end
-      end
-
-      context 'without remote mirrors global setting overridden' do
-        before do
-          project.remote_mirror_available_overridden = false
-        end
-
-        it 'does not fails stuck remote mirrors' do
-          expect(project).not_to receive(:mark_stuck_remote_mirrors_as_failed!)
-
-          subject.execute
-        end
-
-        it 'does not updates remote mirrors' do
-          expect(project).not_to receive(:update_remote_mirrors)
-
-          subject.execute
-        end
-      end
-    end
-  end
-
   describe 'Push branches' do
     subject do
       execute_service(project, user, oldrev, newrev, ref)
diff --git a/spec/services/git/tag_hooks_service_spec.rb b/spec/services/git/tag_hooks_service_spec.rb
index f4c02932f98..f5938a5c708 100644
--- a/spec/services/git/tag_hooks_service_spec.rb
+++ b/spec/services/git/tag_hooks_service_spec.rb
@@ -18,6 +18,12 @@ describe Git::TagHooksService, :service do
     described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
   end
 
+  it 'update remote mirrors' do
+    expect(service).to receive(:update_remote_mirrors).and_call_original
+
+    service.execute
+  end
+
   describe 'System hooks' do
     it 'Executes system hooks' do
       push_data = service.execute