Registry tag API to use CleanupTag job21405-fix-registry-tag-delete

Modifify the TagsControllers and the
container registry api to delete tags
by using the CleanupTagService insteaf of
doing it directly.

This centralizes the logic for deleting
tags into a single place.

5 files changed, 49 insertions, 27 deletions

diff --git a/app/controllers/projects/registry/tags_controller.rb b/app/controllers/projects/registry/tags_controller.rb
index 54e2faa2dd7..dc0579595a5 100644
--- a/app/controllers/projects/registry/tags_controller.rb
+++ b/app/controllers/projects/registry/tags_controller.rb
@@ -19,14 +19,14 @@ module Projects
       end
 
       def destroy
-        if tag.delete
-          respond_to do |format|
-            format.json { head :no_content }
-          end
-        else
-          respond_to do |format|
-            format.json { head :bad_request }
-          end
+        CleanupContainerRepositoryWorker.perform_async(
+          current_user.id,
+          image.id,
+          names: [params[:id]]
+        )
+
+        respond_to do |format|
+          format.json { head :accepted }
         end
       end
 
@@ -70,10 +70,6 @@ module Projects
         @image ||= project.container_repositories
           .find(params[:repository_id])
       end
-
-      def tag
-        @tag ||= image.tag(params[:id])
-      end
     end
   end
 end
diff --git a/lib/api/project_container_repositories.rb b/lib/api/project_container_repositories.rb
index 6d53abcc500..717d5e07426 100644
--- a/lib/api/project_container_repositories.rb
+++ b/lib/api/project_container_repositories.rb
@@ -106,9 +106,11 @@ module API
         authorize_destroy_container_image!
         validate_tag!
 
-        tag.delete
+        ::Projects::ContainerRepository::CleanupTagsService
+          .new(repository&.project, current_user, 'names' => [declared_params[:tag_name]])
+          .execute(repository)
 
-        status :ok
+        status :accepted
       end
     end
 
diff --git a/spec/controllers/projects/registry/tags_controller_spec.rb b/spec/controllers/projects/registry/tags_controller_spec.rb
index c6e063d8229..92b2f53346f 100644
--- a/spec/controllers/projects/registry/tags_controller_spec.rb
+++ b/spec/controllers/projects/registry/tags_controller_spec.rb
@@ -84,17 +84,19 @@ describe Projects::Registry::TagsController do
 
       context 'when there is matching tag present' do
         before do
-          stub_container_registry_tags(repository: repository.path, tags: %w[rc1 test.])
+          stub_container_registry_tags(repository: repository.path, tags: %w[rc1 test.], with_manifest: true)
         end
 
         it 'makes it possible to delete regular tag' do
-          expect_any_instance_of(ContainerRegistry::Tag).to receive(:delete)
+          expect(CleanupContainerRepositoryWorker).to receive(:perform_async)
+            .with(user.id, repository.id, names: ['rc1'])
 
           destroy_tag('rc1')
         end
 
         it 'makes it possible to delete a tag that ends with a dot' do
-          expect_any_instance_of(ContainerRegistry::Tag).to receive(:delete)
+          expect(CleanupContainerRepositoryWorker).to receive(:perform_async)
+            .with(user.id, repository.id, names: ['test.'])
 
           destroy_tag('test.')
         end
diff --git a/spec/features/container_registry_spec.rb b/spec/features/container_registry_spec.rb
index aefdc4d6d4f..cf6a2c29f8d 100644
--- a/spec/features/container_registry_spec.rb
+++ b/spec/features/container_registry_spec.rb
@@ -53,7 +53,8 @@ describe 'Container Registry', :js do
       find('.js-toggle-repo').click
       wait_for_requests
 
-      expect_any_instance_of(ContainerRegistry::Tag).to receive(:delete).and_return(true)
+      expect(CleanupContainerRepositoryWorker).to receive(:perform_async)
+        .with(user.id, container_repository.id, names: ['latest'])
 
       click_on(class: 'js-delete-registry-row', visible: false)
       expect(find('.modal .modal-title')).to have_content 'Remove image'
diff --git a/spec/requests/api/project_container_repositories_spec.rb b/spec/requests/api/project_container_repositories_spec.rb
index f1dc4e6f0b2..63eaeea9f1b 100644
--- a/spec/requests/api/project_container_repositories_spec.rb
+++ b/spec/requests/api/project_container_repositories_spec.rb
@@ -150,7 +150,7 @@ describe API::ProjectContainerRepositories do
           expect(response).to have_gitlab_http_status(:accepted)
         end
 
-        context 'called multiple times in one hour' do
+        context 'called multiple times in one hour', :clean_gitlab_redis_shared_state do
           it 'returns 400 with an error message' do
             stub_exclusive_lease_taken(lease_key, timeout: 1.hour)
             subject
@@ -210,18 +210,39 @@ describe API::ProjectContainerRepositories do
     context 'for developer' do
       let(:api_user) { developer }
 
-      before do
-        stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA), with_manifest: true)
+      context 'when there are multiple tags' do
+        let(:expected_tag_sha) { 'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15' }
+
+        before do
+          stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA rootB), with_manifest: true)
+        end
+
+        it 'properly removes tag' do
+          expect_any_instance_of(ContainerRegistry::Client)
+            .to receive(:put_dummy_tag).with(root_repository.path, 'rootA')
+          expect_any_instance_of(ContainerRegistry::Client)
+            .to receive(:delete_repository_tag).with(root_repository.path, expected_tag_sha)
+
+          subject
+
+          expect(response).to have_gitlab_http_status(:accepted)
+        end
       end
 
-      it 'properly removes tag' do
-        expect_any_instance_of(ContainerRegistry::Client)
-          .to receive(:delete_repository_tag).with(root_repository.path,
-            'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15')
+      context 'when there\'s only one tag' do
+        before do
+          stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA), with_manifest: true)
+        end
 
-        subject
+        it 'properly removes tag' do
+          expect_any_instance_of(ContainerRegistry::Client)
+            .to receive(:delete_repository_tag).with(root_repository.path,
+              'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15')
 
-        expect(response).to have_gitlab_http_status(:ok)
+          subject
+
+          expect(response).to have_gitlab_http_status(:accepted)
+        end
       end
     end
   end