Add latest changes from gitlab-org/gitlab@master

2 files changed, 28 insertions, 10 deletions

diff --git a/app/services/projects/container_repository/destroy_service.rb b/app/services/projects/container_repository/destroy_service.rb
index 6db6b449671..6cb0d55aea4 100644
--- a/app/services/projects/container_repository/destroy_service.rb
+++ b/app/services/projects/container_repository/destroy_service.rb
@@ -10,12 +10,15 @@ module Projects
       }.freeze
 
       def execute(container_repository, disable_timeout: true)
-        return false unless can?(current_user, :update_container_image, project)
+        return error('Unauthorized access') unless can_destroy?
 
         # Delete tags outside of the transaction to avoid hitting an idle-in-transaction timeout
-        unless delete_tags(container_repository, disable_timeout) &&
+        if delete_tags(container_repository, disable_timeout) &&
             destroy_container_repository(container_repository)
+          success
+        else
           container_repository.delete_failed!
+          error('Deletion failed for container repository')
         end
       end
 
@@ -40,9 +43,19 @@ module Projects
         false
       end
 
+      def can_destroy?
+        return true if skip_permission_check?
+
+        can?(current_user, :destroy_container_image, project)
+      end
+
       def error_message(container_repository, message)
-        "Container repository with ID: #{container_repository.id} and path: #{container_repository.path}" \
-        " failed with message: #{message}"
+        "Container repository with ID: #{container_repository.id} and path: #{container_repository.path} " \
+          "failed with message: #{message}"
+      end
+
+      def skip_permission_check?
+        !!params[:skip_permission_check]
       end
     end
   end
diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index 2e346ff6f12..2279ab301dc 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -257,12 +257,12 @@ module Projects
       return true unless Gitlab.config.registry.enabled
       return false unless remove_legacy_registry_tags
 
+      results = []
       project.container_repositories.find_each do |container_repository|
-        service = Projects::ContainerRepository::DestroyService.new(project, current_user)
-        service.execute(container_repository)
+        results << destroy_repository(project, container_repository)
       end
 
-      true
+      results.all?
     end
 
     ##
@@ -272,9 +272,14 @@ module Projects
     def remove_legacy_registry_tags
       return true unless Gitlab.config.registry.enabled
 
-      ::ContainerRepository.build_root_repository(project).tap do |repository|
-        break repository.has_tags? ? repository.delete_tags! : true
-      end
+      root_repository = ::ContainerRepository.build_root_repository(project)
+      root_repository.has_tags? ? destroy_repository(project, root_repository) : true
+    end
+
+    def destroy_repository(project, repository)
+      service = ContainerRepository::DestroyService.new(project, current_user, { skip_permission_check: true })
+      response = service.execute(repository)
+      response[:status] == :success
     end
 
     def raise_error(message)