Merge branch '30186-mirror-pull-api-no-unpause' into 'master'

Do not start mirroring via API when paused

See merge request gitlab-org/gitlab!17930

changelogs/unreleased/30186-mirror-pull-api-no-unpause.yml

+---
+title: Do not start mirroring via API when paused
+merge_request: 17930
+author:
+type: changed

ee/app/services/start_pull_mirroring_service.rb

+# frozen_string_literal: true
+
+class StartPullMirroringService < BaseService
+  def execute
+    return error('Mirroring for the project is on pause', 403) if project.import_state.hard_failed?
+
+    project.import_state.force_import_job!
+    success
+  end
+end

ee/lib/api/project_mirror.rb

@@ -41,6 +41,22 @@ module API
       def project
         @project ||= github_webhook_signature ? find_project(params[:id]) : user_project
       end
+
+      def process_pull_request
+        external_pull_request = ProcessGithubPullRequestEventService.new(project, current_user).execute(params)
+
+        if external_pull_request
+          render_validation_error!(external_pull_request)
+        else
+          render_api_error!('The pull request event is not processable', 422)
+        end
+      end
+
+      def start_pull_mirroring
+        result = StartPullMirroringService.new(project, current_user).execute
+
+        render_api_error!(result[:message], result[:http_status]) if result[:status] == :error
+      end
     end
 
     params do
@@ -64,13 +80,9 @@ module API
         break render_api_error!('The project is not mirrored', 400) unless project.mirror?
 
         if params[:pull_request]
-          if external_pull_request = ProcessGithubPullRequestEventService.new(project, current_user).execute(params)
-            render_validation_error!(external_pull_request)
-          else
-            render_api_error!('The pull request event is not processable', 422)
-          end
+          process_pull_request
         else
-          project.import_state.force_import_job!
+          start_pull_mirroring
         end
 
         status 200

ee/spec/requests/api/project_mirror_spec.rb

@@ -24,64 +24,70 @@ describe API::ProjectMirror do
       end
 
       context 'when it receives a "push" event' do
-        context 'when import state is' do
-          def project_in_state(state)
-            project = create(:project, :repository, namespace: user.namespace)
-            import_state = create(:import_state, :mirror, state, project: project)
-            import_state.update(next_execution_timestamp: 10.minutes.from_now)
-
-            project
-          end
-
-          it 'none it triggers the pull mirroring operation' do
-            project = project_in_state(:none)
-
+        shared_examples_for 'an API endpoint that triggers pull mirroring operation' do
+          it 'executes UpdateAllMirrorsWorker' do
             expect(UpdateAllMirrorsWorker).to receive(:perform_async).once
 
             post api("/projects/#{project.id}/mirror/pull", user)
 
             expect(response).to have_gitlab_http_status(200)
           end
+        end
 
-          it 'failed it triggers the pull mirroring operation' do
-            project = project_in_state(:failed)
-
-            expect(UpdateAllMirrorsWorker).to receive(:perform_async).once
+        shared_examples_for 'an API endpoint that does not trigger pull mirroring operation' do |status_code|
+          it "does not execute UpdateAllMirrorsWorker and returns #{status_code}" do
+            expect(UpdateAllMirrorsWorker).not_to receive(:perform_async)
 
             post api("/projects/#{project.id}/mirror/pull", user)
 
-            expect(response).to have_gitlab_http_status(200)
+            expect(response).to have_gitlab_http_status(status_code)
           end
+        end
 
-          it 'finished it triggers the pull mirroring operation' do
-            project = project_in_state(:finished)
-
-            expect(UpdateAllMirrorsWorker).to receive(:perform_async).once
+        let(:project) do
+          create(:project, :repository, namespace: user.namespace) do |project|
+            create(:import_state, :mirror, state, project: project) do |import_state|
+              import_state.update(next_execution_timestamp: 10.minutes.from_now)
+            end
+          end
+        end
 
-            post api("/projects/#{project.id}/mirror/pull", user)
+        context 'when import state is none' do
+          let(:state) { :none }
 
-            expect(response).to have_gitlab_http_status(200)
-          end
+          it_behaves_like 'an API endpoint that triggers pull mirroring operation'
+        end
 
-          it 'scheduled does not trigger the pull mirroring operation and returns 200' do
-            project = project_in_state(:scheduled)
+        context 'when import state is failed' do
+          let(:state) { :failed }
 
-            expect(UpdateAllMirrorsWorker).not_to receive(:perform_async)
+          it_behaves_like 'an API endpoint that triggers pull mirroring operation'
 
-            post api("/projects/#{project.id}/mirror/pull", user)
+          context "and retried more than #{Gitlab::Mirror::MAX_RETRY} times" do
+            before do
+              project.import_state.update(retry_count: Gitlab::Mirror::MAX_RETRY + 1)
+            end
 
-            expect(response).to have_gitlab_http_status(200)
+            it_behaves_like 'an API endpoint that does not trigger pull mirroring operation', 403
           end
+        end
 
-          it 'started does not trigger the pull mirroring operation and returns 200' do
-            project = project_in_state(:started)
+        context 'when import state is finished' do
+          let(:state) { :finished }
 
-            expect(UpdateAllMirrorsWorker).not_to receive(:perform_async)
+          it_behaves_like 'an API endpoint that triggers pull mirroring operation'
+        end
 
-            post api("/projects/#{project.id}/mirror/pull", user)
+        context 'when import state is scheduled' do
+          let(:state) { :scheduled }
 
-            expect(response).to have_gitlab_http_status(200)
-          end
+          it_behaves_like 'an API endpoint that does not trigger pull mirroring operation', 200
+        end
+
+        context 'when import state is started' do
+          let(:state) { :started }
+
+          it_behaves_like 'an API endpoint that does not trigger pull mirroring operation', 200
         end
       end
 

ee/spec/services/start_pull_mirroring_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe StartPullMirroringService do
+  let(:project) { create(:project) }
+  let(:import_state) { create(:import_state, project: project) }
+  let(:user) { create(:user) }
+
+  subject { described_class.new(project, user) }
+
+  context "when retried more than #{Gitlab::Mirror::MAX_RETRY} times" do
+    before do
+      import_state.update(retry_count: Gitlab::Mirror::MAX_RETRY + 1)
+    end
+
+    it 'does not start pull mirroring' do
+      expect(UpdateAllMirrorsWorker).not_to receive(:perform_async)
+      expect(subject.execute[:status]).to eq(:error)
+    end
+  end
+
+  context 'when does not reach the max retry limit yet' do
+    before do
+      import_state.update(retry_count: Gitlab::Mirror::MAX_RETRY - 1)
+    end
+
+    it 'starts pull mirroring' do
+      expect(UpdateAllMirrorsWorker).to receive(:perform_async).once
+      expect(import_state.reload.retry_count).not_to eq(0)
+      expect(subject.execute[:status]).to eq(:success)
+    end
+  end
+end