Merge branch '357939-force-import-cancellation-method' into 'master'

Add a force cancel param to the gitlab registry client

See merge request gitlab-org/gitlab!84525

app/models/container_repository.rb

@@ -34,7 +34,17 @@ class ContainerRepository < ApplicationRecord
 
   enum status: { delete_scheduled: 0, delete_failed: 1 }
   enum expiration_policy_cleanup_status: { cleanup_unscheduled: 0, cleanup_scheduled: 1, cleanup_unfinished: 2, cleanup_ongoing: 3 }
-  enum migration_skipped_reason: { not_in_plan: 0, too_many_retries: 1, too_many_tags: 2, root_namespace_in_deny_list: 3, migration_canceled: 4, not_found: 5, native_import: 6 }
+
+  enum migration_skipped_reason: {
+    not_in_plan: 0,
+    too_many_retries: 1,
+    too_many_tags: 2,
+    root_namespace_in_deny_list: 3,
+    migration_canceled: 4,
+    not_found: 5,
+    native_import: 6,
+    migration_forced_canceled: 7
+  }
 
   delegate :client, :gitlab_api_client, to: :registry
 
@@ -504,6 +514,19 @@ class ContainerRepository < ApplicationRecord
     gitlab_api_client.cancel_repository_import(self.path)
   end
 
+  # This method is not meant for consumption by the code
+  # It is meant for manual use in the case that a migration needs to be
+  # cancelled by an admin or SRE
+  def force_migration_cancel
+    return :error unless gitlab_api_client.supports_gitlab_api?
+
+    response = gitlab_api_client.cancel_repository_import(self.path, force: true)
+
+    skip_import(reason: :migration_forced_canceled) if response[:status] == :ok
+
+    response
+  end
+
   def self.build_from_path(path)
     self.new(project: path.repository_project,
              name: path.repository_name)

lib/container_registry/gitlab_api_client.rb

@@ -58,10 +58,12 @@ module ContainerRegistry
       IMPORT_RESPONSES.fetch(response.status, :error)
     end
 
-    # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#import-repository
-    def cancel_repository_import(path)
+    # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#cancel-repository-import
+    def cancel_repository_import(path, force: false)
       response = with_import_token_faraday do |faraday_client|
-        faraday_client.delete(import_url_for(path))
+        faraday_client.delete(import_url_for(path)) do |req|
+          req.params['force'] = true if force
+        end
       end
 
       status = IMPORT_RESPONSES.fetch(response.status, :error)

spec/lib/container_registry/gitlab_api_client_spec.rb

@@ -107,7 +107,9 @@ RSpec.describe ContainerRegistry::GitlabApiClient do
   end
 
   describe '#cancel_repository_import' do
-    subject { client.cancel_repository_import(path) }
+    let(:force) { false }
+
+    subject { client.cancel_repository_import(path, force: force) }
 
     where(:status_code, :expected_result) do
       200 | :already_imported
@@ -124,7 +126,7 @@ RSpec.describe ContainerRegistry::GitlabApiClient do
 
     with_them do
       before do
-        stub_import_cancel(path, status_code)
+        stub_import_cancel(path, status_code, force: force)
       end
 
       it { is_expected.to eq({ status: expected_result, migration_state: nil }) }
@@ -134,11 +136,21 @@ RSpec.describe ContainerRegistry::GitlabApiClient do
       let(:status) { 'this_is_a_test' }
 
       before do
-        stub_import_cancel(path, 400, status: status)
+        stub_import_cancel(path, 400, status: status, force: force)
       end
 
       it { is_expected.to eq({ status: :bad_request, migration_state: status }) }
     end
+
+    context 'force cancel' do
+      let(:force) { true }
+
+      before do
+        stub_import_cancel(path, 202, force: force)
+      end
+
+      it { is_expected.to eq({ status: :ok, migration_state: nil }) }
+    end
   end
 
   describe '#import_status' do
@@ -330,15 +342,24 @@ RSpec.describe ContainerRegistry::GitlabApiClient do
       )
   end
 
-  def stub_import_cancel(path, http_status, status: nil)
+  def stub_import_cancel(path, http_status, status: nil, force: false)
     body = {}
 
     if http_status == 400
       body = { status: status }
     end
 
-    stub_request(:delete, "#{registry_api_url}/gitlab/v1/import/#{path}/")
-      .with(headers: { 'Accept' => described_class::JSON_TYPE, 'Authorization' => "bearer #{import_token}" })
+    headers = {
+      'Accept' => described_class::JSON_TYPE,
+      'Authorization' => "bearer #{import_token}",
+      'User-Agent' => "GitLab/#{Gitlab::VERSION}",
+      'Accept-Encoding' => 'gzip;q=1.0,deflate;q=0.6,identity;q=0.3'
+    }
+
+    params = force ? '?force=true' : ''
+
+    stub_request(:delete, "#{registry_api_url}/gitlab/v1/import/#{path}/#{params}")
+      .with(headers: headers)
       .to_return(
         status: http_status,
         body: body.to_json,

spec/models/container_repository_spec.rb

@@ -762,6 +762,61 @@ RSpec.describe ContainerRepository, :aggregate_failures do
 
       it_behaves_like 'gitlab migration client request', :cancel_repository_import
     end
+
+    describe '#force_migration_cancel' do
+      subject { repository.force_migration_cancel }
+
+      shared_examples 'returning the same response as the client' do
+        it 'returns the same response' do
+          expect(repository.gitlab_api_client)
+            .to receive(:cancel_repository_import).with(repository.path, force: true).and_return(client_response)
+
+          expect(subject).to eq(client_response)
+        end
+      end
+
+      context 'successful cancellation' do
+        let(:client_response) { { status: :ok } }
+
+        it_behaves_like 'returning the same response as the client'
+
+        it 'skips the migration' do
+          expect(repository.gitlab_api_client)
+            .to receive(:cancel_repository_import).with(repository.path, force: true).and_return(client_response)
+
+          expect { subject }.to change { repository.reload.migration_state }.to('import_skipped')
+            .and change { repository.migration_skipped_reason }.to('migration_forced_canceled')
+            .and change { repository.migration_skipped_at }
+        end
+      end
+
+      context 'failed cancellation' do
+        let(:client_response) { { status: :error } }
+
+        it_behaves_like 'returning the same response as the client'
+
+        it 'does not skip the migration' do
+          expect(repository.gitlab_api_client)
+            .to receive(:cancel_repository_import).with(repository.path, force: true).and_return(client_response)
+
+          expect { subject }.to not_change { repository.reload.migration_state }
+            .and not_change { repository.migration_skipped_reason }
+            .and not_change { repository.migration_skipped_at }
+        end
+      end
+
+      context 'when the gitlab_api feature is not supported' do
+        before do
+          allow(repository.gitlab_api_client).to receive(:supports_gitlab_api?).and_return(false)
+        end
+
+        it 'returns :error' do
+          expect(repository.gitlab_api_client).not_to receive(:cancel_repository_import)
+
+          expect(subject).to eq(:error)
+        end
+      end
+    end
   end
 
   describe '.build_from_path' do