Merge branch '36717-container-repositories-can-not-be-replicated' into 'master'

Fix "Container repositories can not be replicated" when s3 is used

Closes #36717

See merge request gitlab-org/gitlab!21068

changelogs/unreleased/36717-container-repositories-can-not-be-replicated.yml

+---
+title: Fix Container repositories can not be replicated when s3 is used
+merge_request: 21068
+author:
+type: fixed

ee/lib/ee/container_registry/client.rb

@@ -33,21 +33,29 @@ module EE
         faraday.head("/v2/#{name}/blobs/#{digest}").success?
       end
 
+      # Pulls a blob from the Registry.
       # We currently use Faraday 0.12 which does not support streaming download yet
       # Given that we aim to migrate to HTTP.rb client and that updating Faraday is potentialy
-      # dangerous, we use HTTP.rb here
+      # dangerous, we use HTTP.rb here.
+      #
+      # @return {Object} Returns a Tempfile object or nil when no success
       def pull_blob(name, digest)
         file = Tempfile.new("blob-#{digest}")
 
         response = HTTP
           .headers({ "Authorization" => "Bearer #{@options[:token]}" }) # rubocop:disable Gitlab/ModuleWithInstanceVariables
-          .follow
           .get("#{@base_uri}/v2/#{name}/blobs/#{digest}") # rubocop:disable Gitlab/ModuleWithInstanceVariables
+
+        raise Error.new("Pull Blob error: #{response.body}") unless response.status.redirect?
+
+        response = HTTP.get(response['Location'])
         response.body.each do |chunk|
           file.binmode
           file.write(chunk)
         end
 
+        raise Error.new("Could not download the blob: #{digest}") unless response.status.success?
+
         file
       ensure
         file.close

ee/spec/lib/container_registry/client_spec.rb

@@ -136,4 +136,42 @@ describe ContainerRegistry::Client do
       expect(client.repository_raw_manifest('group/test', 'my-tag')).to eq(manifest)
     end
   end
+
+  describe '#pull_blob' do
+    let(:auth_headers) { { 'Authorization' => 'Bearer 12345' } }
+
+    before do
+      stub_request(:get, "http://registry/v2/group/test/blobs/e2312abc")
+          .with(headers: auth_headers)
+          .to_return(status: 302, headers: { "Location" => 'http://download-link.com' })
+    end
+
+    it 'GET "/v2/:name/blobs/:reference' do
+      stub_request(:get, "http://download-link.com/")
+        .to_return(status: 200)
+
+      # With this stub we assert that there is no Authorization header in the request.
+      # This also mimics the real case because Amazon s3 returns error too.
+      stub_request(:get, "http://download-link.com/")
+        .with(headers: auth_headers)
+        .to_return(status: 500)
+
+      expect(client.pull_blob('group/test', 'e2312abc')).to be_a_kind_of(Tempfile)
+    end
+
+    it 'raises error when it can not download blob' do
+      stub_request(:get, "http://download-link.com/")
+        .to_return(status: 500)
+
+      expect { client.pull_blob('group/test', 'e2312abc') }.to raise_error(EE::ContainerRegistry::Client::Error)
+    end
+
+    it 'raises error when request is not authenticated' do
+      stub_request(:get, "http://registry/v2/group/test/blobs/e2312abc")
+          .with(headers: auth_headers)
+          .to_return(status: 401)
+
+      expect { client.pull_blob('group/test', 'e2312abc') }.to raise_error(EE::ContainerRegistry::Client::Error)
+    end
+  end
 end