Merge branch 'fixes_for_regostry_replication' into 'master'

Fixes for Docker Registry Replication

See merge request gitlab-org/gitlab!16196

ee/app/services/geo/container_repository_sync.rb

@@ -35,9 +35,10 @@ def execute
 
     def sync_tag(tag)
       file = nil
-      manifest = client.repository_manifest(name, tag)
+      manifest = client.repository_raw_manifest(name, tag)
+      manifest_parsed = JSON.parse(manifest)
 
-      list_blobs(manifest).each do |digest|
+      list_blobs(manifest_parsed).each do |digest|
         next if container_repository.blob_exists?(digest)
 
         file = client.pull_blob(name, digest)
@@ -45,7 +46,7 @@ def sync_tag(tag)
         file.unlink
       end
 
-      container_repository.push_manifest(tag, manifest, manifest['mediaType'])
+      container_repository.push_manifest(tag, manifest, manifest_parsed['mediaType'])
     ensure
       file.try(:unlink)
     end

ee/lib/ee/container_registry/client.rb

@@ -3,6 +3,8 @@
 module EE
   module ContainerRegistry
     module Client
+      include ::Gitlab::Utils::StrongMemoize
+
       Error = Class.new(StandardError)
 
       # In the future we may want to read a small chunks into memory and use chunked upload
@@ -12,7 +14,7 @@ def push_blob(name, digest, file_path)
         url = get_upload_url(name, digest)
         headers = { 'Content-Type' => 'application/octet-stream', 'Content-Length' => payload.size.to_s }
 
-        response = faraday_upload.put(url, payload, headers)
+        response = faraday.put(url, payload, headers)
 
         raise Error.new("Push Blob error: #{response.body}") unless response.success?
 
@@ -51,6 +53,10 @@ def pull_blob(name, digest)
         file.close
       end
 
+      def repository_raw_manifest(name, reference)
+        response_body faraday_raw.get("/v2/#{name}/manifests/#{reference}")
+      end
+
       private
 
       def get_upload_url(name, digest)
@@ -58,21 +64,24 @@ def get_upload_url(name, digest)
 
         raise Error.new("Get upload URL error: #{response.body}") unless response.success?
 
-        response.headers['location']
-
         upload_url = URI(response.headers['location'])
         upload_url.query = "#{upload_url.query}&#{URI.encode_www_form(digest: digest)}"
         upload_url
       end
 
-      def faraday_upload
-        @faraday_upload ||= Faraday.new(@base_uri) do |conn| # rubocop:disable Gitlab/ModuleWithInstanceVariables
-          initialize_connection(conn, @options) # rubocop:disable Gitlab/ModuleWithInstanceVariables
-          conn.request :multipart
-          conn.request :url_encoded
-          conn.adapter :net_http
+      # rubocop:disable Gitlab/ModuleWithInstanceVariables
+      def faraday_raw
+        strong_memoize(:faraday_raw) do
+          Faraday.new(@base_uri) do |conn|
+            initialize_connection(conn, @options, &method(:accept_raw_manifest))
+          end
         end
       end
+      # rubocop:enable Gitlab/ModuleWithInstanceVariables
+
+      def accept_raw_manifest(conn)
+        conn.headers['Accept'] = ::ContainerRegistry::Client::ACCEPTED_TYPES
+      end
     end
   end
 end

ee/spec/lib/container_registry/client_spec.rb

@@ -121,4 +121,20 @@
       expect(client.blob_exists?('group/test', digest)).to eq(false)
     end
   end
+
+  describe '#repository_raw_manifest' do
+    let(:manifest) { '{schemaVersion: 2, layers:[]}' }
+
+    it 'GET "/v2/:name/manifests/:reference' do
+      stub_request(:get, 'http://registry/v2/group/test/manifests/my-tag')
+        .with(
+          headers: {
+            'Accept' => 'application/vnd.docker.distribution.manifest.v2+json, application/vnd.oci.image.manifest.v1+json',
+            'Authorization' => 'bearer 12345'
+          })
+        .to_return(status: 200, body: manifest, headers: {})
+
+      expect(client.repository_raw_manifest('group/test', 'my-tag')).to eq(manifest)
+    end
+  end
 end

ee/spec/services/geo/container_repository_sync_spec.rb

@@ -10,6 +10,11 @@
     create(:container_repository, name: 'my_image', project: project)
   end
 
+  # Break symbol will be removed if JSON encode/decode operation happens
+  # so we use this to prove that it does not happen and we preserve original
+  # human readable JSON
+  let(:manifest) { "{\"schemaVersion\":2,\n\"layers\":[]}" }
+
   before do
     stub_container_registry_config(enabled: true,
                                    api_url: 'http://registry.gitlab',
@@ -52,12 +57,30 @@
           'Authorization' => 'bearer token'
         })
       .to_return(status: 200, body: "", headers: { 'docker-content-digest' => 'sha256:aaaaa' })
+
+    stub_request(:get, "http://primary.registry.gitlab/v2/group/test/my_image/manifests/tag-to-sync")
+      .with(
+        headers: {
+          'Accept' => 'application/vnd.docker.distribution.manifest.v2+json, application/vnd.oci.image.manifest.v1+json',
+          'Authorization' => 'bearer pull-token'
+        })
+      .to_return(status: 200, body: manifest, headers: {})
+
+    stub_request(:put, "http://registry.gitlab/v2/group/test/my_image/manifests/tag-to-sync")
+      .with(
+        body: manifest,
+        headers: {
+          'Accept' => 'application/vnd.docker.distribution.manifest.v2+json, application/vnd.oci.image.manifest.v1+json',
+          'Authorization' => 'bearer token',
+          'Content-Type' => 'application/json'
+        })
+      .to_return(status: 200, body: "", headers: {})
   end
 
   describe 'execute' do
     it 'determines list of tags to sync and to remove correctly' do
       expect(container_repository).to receive(:delete_tag_by_digest).with('sha256:aaaaa')
-      expect_any_instance_of(described_class).to receive(:sync_tag)
+      expect_any_instance_of(described_class).to receive(:sync_tag).with('tag-to-sync').and_call_original
 
       described_class.new(container_repository).execute
     end