Merge branch '34827-delete-container-registry-tag-undefined-method-success-for-nil' into 'master'

Fix crash when docker fails to delete tags

Closes #34827

See merge request gitlab-org/gitlab!19208

app/services/projects/container_repository/delete_tags_service.rb

@@ -22,10 +22,18 @@ module Projects
       def smart_delete(container_repository, tag_names)
         # generates the blobs for the dummy image
         dummy_manifest = container_repository.client.generate_empty_manifest(container_repository.path)
+        return error('could not generate manifest') if dummy_manifest.nil?
 
         # update the manifests of the tags with the new dummy image
-        tag_digests = tag_names.map do |name|
-          container_repository.client.put_tag(container_repository.path, name, dummy_manifest)
+        deleted_tags = []
+        tag_digests = []
+
+        tag_names.each do |name|
+          digest = container_repository.client.put_tag(container_repository.path, name, dummy_manifest)
+          next unless digest
+
+          deleted_tags << name
+          tag_digests << digest
         end
 
         # make sure the digests are the same (it should always be)
@@ -37,8 +45,8 @@ module Projects
         # Deletes the dummy image
         # All created tag digests are the same since they all have the same dummy image.
         # a single delete is sufficient to remove all tags with it
-        if container_repository.delete_tag_by_digest(tag_digests.first)
-          success(deleted: tag_names)
+        if tag_digests.any? && container_repository.delete_tag_by_digest(tag_digests.first)
+          success(deleted: deleted_tags)
         else
           error('could not delete tags')
         end

changelogs/unreleased/34827-delete-container-registry-tag-undefined-method-success-for-nil.yml

+---
+title: Fix crash when docker fails deleting tags
+merge_request: 19208
+author:
+type: fixed

lib/container_registry/client.rb

@@ -51,7 +51,7 @@ module ContainerRegistry
 
     def upload_blob(name, content, digest)
       upload = faraday.post("/v2/#{name}/blobs/uploads/")
-      return unless upload.success?
+      return upload unless upload.success?
 
       location = URI(upload.headers['location'])
 

spec/lib/container_registry/client_spec.rb

@@ -99,8 +99,8 @@ describe ContainerRegistry::Client do
         stub_upload('path', 'content', 'sha256:123', 400)
       end
 
-      it 'returns nil' do
-        expect(subject).to be nil
+      it 'returns a failure' do
+        expect(subject).not_to be_success
       end
     end
   end
@@ -125,6 +125,14 @@ describe ContainerRegistry::Client do
 
       expect(subject).to eq(result_manifest)
     end
+
+    context 'when upload fails' do
+      before do
+        stub_upload('path', "{\n  \"config\": {\n  }\n}", 'sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3', 500)
+      end
+
+      it { is_expected.to be nil }
+    end
   end
 
   describe '#put_tag' do

spec/services/projects/container_repository/delete_tags_service_spec.rb

@@ -88,6 +88,33 @@ describe Projects::ContainerRepository::DeleteTagsService do
 
           is_expected.to include(status: :success)
         end
+
+        context 'with failures' do
+          context 'when the dummy manifest generation fails' do
+            before do
+              stub_upload("{\n  \"config\": {\n  }\n}", 'sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3', success: false)
+            end
+
+            it { is_expected.to include(status: :error) }
+          end
+
+          context 'when updating the tags fails' do
+            before do
+              stub_upload("{\n  \"config\": {\n  }\n}", 'sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3')
+
+              stub_request(:put, "http://registry.gitlab/v2/#{repository.path}/manifests/A")
+                .to_return(status: 500, body: "", headers: { 'docker-content-digest' => 'sha256:dummy' })
+
+              stub_request(:put, "http://registry.gitlab/v2/#{repository.path}/manifests/Ba")
+                .to_return(status: 500, body: "", headers: { 'docker-content-digest' => 'sha256:dummy' })
+
+              stub_request(:delete, "http://registry.gitlab/v2/#{repository.path}/manifests/sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3")
+                .to_return(status: 200, body: "", headers: {})
+            end
+
+            it { is_expected.to include(status: :error) }
+          end
+        end
       end
     end
   end
@@ -107,10 +134,10 @@ describe Projects::ContainerRepository::DeleteTagsService do
     end
   end
 
-  def stub_upload(content, digest)
+  def stub_upload(content, digest, success: true)
     expect_any_instance_of(ContainerRegistry::Client)
       .to receive(:upload_blob)
-      .with(repository.path, content, digest) { double(success?: true ) }
+      .with(repository.path, content, digest) { double(success?: success ) }
   end
 
   def expect_delete_tag(digest)