Merge branch...

Merge branch '336013-geo-multi-arch-containers-not-properly-replicating-non-primary-architectures-to-secondary' into 'master'

Geo: Replicate multi-arch containers

See merge request gitlab-org/gitlab!67624

ee/app/services/geo/container_repository_sync.rb

@@ -25,21 +25,36 @@ module Geo
     private
 
     def sync_tag(tag)
-      file = nil
       manifest = client.repository_raw_manifest(repository_path, tag[:name])
       manifest_parsed = Gitlab::Json.parse(manifest)
 
-      list_blobs(manifest_parsed).each do |digest|
+      case manifest_parsed['mediaType']
+      when ContainerRegistry::Client::DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE
+        push_manifest_blobs(manifest_parsed)
+      when ContainerRegistry::Client::DOCKER_DISTRIBUTION_MANIFEST_LIST_V2_TYPE
+        manifest_parsed['manifests'].each do |submanifest|
+          image_info_raw = client.repository_raw_manifest(repository_path, submanifest['digest'])
+          image_info = Gitlab::Json.parse(image_info_raw)
+          push_manifest_blobs(image_info)
+          container_repository.push_manifest(submanifest['digest'], image_info_raw, image_info['mediaType'])
+        end
+      else
+        raise "Unexpected mediaType: #{manifest_parsed['mediaType']}"
+      end
+      container_repository.push_manifest(tag[:name], manifest, manifest_parsed['mediaType'])
+    end
+
+    def push_manifest_blobs(manifest)
+      list_blobs(manifest).each do |digest|
         next if container_repository.blob_exists?(digest)
 
         file = client.pull_blob(repository_path, digest)
-        container_repository.push_blob(digest, file.path)
-        file.unlink
+        begin
+          container_repository.push_blob(digest, file.path)
+        ensure
+          file.unlink
+        end
       end
-
-      container_repository.push_manifest(tag[:name], manifest, manifest_parsed['mediaType'])
-    ensure
-      file.try(:unlink)
     end
 
     def remove_tag(tag)

ee/lib/ee/container_registry/client.rb

@@ -94,7 +94,7 @@ module EE
       # rubocop:enable Gitlab/ModuleWithInstanceVariables
 
       def accept_raw_manifest(conn)
-        conn.headers['Accept'] = ::ContainerRegistry::Client::ACCEPTED_TYPES
+        conn.headers['Accept'] = ::ContainerRegistry::Client::ACCEPTED_TYPES_RAW
       end
     end
   end

ee/spec/lib/container_registry/client_spec.rb

@@ -24,6 +24,14 @@ RSpec.describe ContainerRegistry::Client do
     }
   end
 
+  let(:headers_with_accept_types_with_list) do
+    {
+      'Accept' => 'application/vnd.docker.distribution.manifest.v2+json, application/vnd.oci.image.manifest.v1+json, application/vnd.docker.distribution.manifest.list.v2+json',
+      'Authorization' => 'bearer 12345',
+      'User-Agent' => "GitLab/#{Gitlab::VERSION}"
+    }
+  end
+
   describe '#push_blob' do
     let(:file) do
       file = Tempfile.new('test1')
@@ -119,7 +127,7 @@ RSpec.describe ContainerRegistry::Client do
 
     it 'GET "/v2/:name/manifests/:reference' do
       stub_request(:get, 'http://registry/v2/group/test/manifests/my-tag')
-        .with(headers: headers_with_accept_types)
+        .with(headers: headers_with_accept_types_with_list)
         .to_return(status: 200, body: manifest, headers: {})
 
       expect(client.repository_raw_manifest('group/test', 'my-tag')).to eq(manifest)

ee/spec/services/geo/container_repository_sync_spec.rb

@@ -17,6 +17,7 @@ RSpec.describe Geo::ContainerRepositorySync, :geo do
   let(:manifest) do
     "{" \
       "\n\"schemaVersion\":2," \
+      "\n\"mediaType\":\"application/vnd.docker.distribution.manifest.v2+json\"," \
       "\n\"layers\":[" \
         "{\n\"mediaType\":\"application/vnd.docker.distribution.manifest.v2+json\",\n\"size\":3333,\n\"digest\":\"sha256:3333\"}," \
         "{\n\"mediaType\":\"application/vnd.docker.distribution.manifest.v2+json\",\n\"size\":4444,\n\"digest\":\"sha256:4444\"}," \
@@ -25,6 +26,26 @@ RSpec.describe Geo::ContainerRepositorySync, :geo do
     "}"
   end
 
+  let(:manifest_list) do
+    %Q(
+      {
+        "schemaVersion":2,
+        "mediaType":"application/vnd.docker.distribution.manifest.list.v2+json",
+        "manifests":[
+          {
+            "mediaType":"application/vnd.docker.distribution.manifest.v2+json",
+            "size":6666,
+            "digest":"sha256:6666",
+            "platform":
+              {
+                "architecture":"arm64","os":"linux"
+              }
+          }
+        ]
+       }
+       )
+  end
+
   before do
     stub_container_registry_config(enabled: true, api_url: secondary_api_url)
     stub_registry_replication_config(enabled: true, primary_api_url: primary_api_url)
@@ -61,6 +82,16 @@ RSpec.describe Geo::ContainerRepositorySync, :geo do
       .to_return(status: 200, body: manifest, headers: {})
   end
 
+  def stub_secondary_raw_manifest_request(repository_url, tag, manifest)
+    stub_request(:get, "#{repository_url}/manifests/#{tag}")
+      .to_return(status: 200, body: manifest, headers: {})
+  end
+
+  def stub_primary_raw_manifest_list_request(repository_url, tag, manifest)
+    stub_request(:get, "#{repository_url}/manifests/#{tag}")
+      .to_return(status: 200, body: manifest_list, headers: {})
+  end
+
   def stub_secondary_push_manifest_request(repository_url, tag, manifest)
     stub_request(:put, "#{repository_url}/manifests/#{tag}")
       .with(body: manifest)
@@ -82,26 +113,46 @@ RSpec.describe Geo::ContainerRepositorySync, :geo do
   describe '#execute' do
     subject { described_class.new(container_repository) }
 
-    it 'determines list of tags to sync and to remove correctly' do
-      stub_primary_repository_tags_requests(primary_repository_url, { 'tag-to-sync' => 'sha256:1111' })
-      stub_secondary_repository_tags_requests(secondary_repository_url, { 'tag-to-remove' => 'sha256:2222' })
-      stub_primary_raw_manifest_request(primary_repository_url, 'tag-to-sync', manifest)
-      stub_missing_blobs_requests(primary_repository_url, secondary_repository_url, { 'sha256:3333' => true, 'sha256:4444' => false })
-      stub_secondary_push_manifest_request(secondary_repository_url, 'tag-to-sync', manifest)
+    context 'single manifest' do
+      it 'determines list of tags to sync and to remove correctly' do
+        stub_primary_repository_tags_requests(primary_repository_url, { 'tag-to-sync' => 'sha256:1111' })
+        stub_secondary_repository_tags_requests(secondary_repository_url, { 'tag-to-remove' => 'sha256:2222' })
+        stub_primary_raw_manifest_request(primary_repository_url, 'tag-to-sync', manifest)
+        stub_missing_blobs_requests(primary_repository_url, secondary_repository_url, { 'sha256:3333' => true, 'sha256:4444' => false })
+        stub_secondary_push_manifest_request(secondary_repository_url, 'tag-to-sync', manifest)
 
-      expect(container_repository).to receive(:push_blob).with('sha256:3333', anything)
-      expect(container_repository).not_to receive(:push_blob).with('sha256:4444', anything)
-      expect(container_repository).not_to receive(:push_blob).with('sha256:5555', anything)
-      expect(container_repository).to receive(:delete_tag_by_digest).with('sha256:2222')
+        expect(container_repository).to receive(:push_blob).with('sha256:3333', anything)
+        expect(container_repository).not_to receive(:push_blob).with('sha256:4444', anything)
+        expect(container_repository).not_to receive(:push_blob).with('sha256:5555', anything)
+        expect(container_repository).to receive(:delete_tag_by_digest).with('sha256:2222')
 
-      subject.execute
-    end
+        subject.execute
+      end
 
-    context 'when primary repository has no tags' do
-      it 'removes secondary tags and does not fail' do
-        stub_primary_repository_tags_requests(primary_repository_url, {})
-        stub_secondary_repository_tags_requests(secondary_repository_url, { 'tag-to-remove' => 'sha256:2222' })
+      context 'when primary repository has no tags' do
+        it 'removes secondary tags and does not fail' do
+          stub_primary_repository_tags_requests(primary_repository_url, {})
+          stub_secondary_repository_tags_requests(secondary_repository_url, { 'tag-to-remove' => 'sha256:2222' })
+
+          expect(container_repository).to receive(:delete_tag_by_digest).with('sha256:2222')
+
+          subject.execute
+        end
+      end
+    end
 
+    context 'manifest list' do
+      it 'pushes the correct blobs and manifests' do
+        stub_primary_repository_tags_requests(primary_repository_url, { 'tag-to-sync' => 'sha256:1111' })
+        stub_secondary_repository_tags_requests(secondary_repository_url, {})
+        stub_primary_raw_manifest_list_request(primary_repository_url, 'tag-to-sync', manifest_list)
+        stub_primary_raw_manifest_request(primary_repository_url, 'sha256:6666', manifest)
+        stub_secondary_raw_manifest_request(secondary_repository_url, 'sha256:6666', manifest)
+        stub_missing_blobs_requests(primary_repository_url, secondary_repository_url, { 'sha256:3333' => true, 'sha256:4444' => false })
+
+        expect(container_repository).to receive(:push_blob).with('sha256:3333', anything)
+        expect(container_repository).to receive(:push_manifest).with('sha256:6666', anything, anything)
+        expect(container_repository).to receive(:push_manifest).with('tag-to-sync', anything, anything)
         expect(container_repository).to receive(:delete_tag_by_digest).with('sha256:2222')
 
         subject.execute

lib/container_registry/client.rb

@@ -11,6 +11,7 @@ module ContainerRegistry
     attr_accessor :uri
 
     DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE = 'application/vnd.docker.distribution.manifest.v2+json'
+    DOCKER_DISTRIBUTION_MANIFEST_LIST_V2_TYPE = 'application/vnd.docker.distribution.manifest.list.v2+json'
     OCI_MANIFEST_V1_TYPE = 'application/vnd.oci.image.manifest.v1+json'
     CONTAINER_IMAGE_V1_TYPE = 'application/vnd.docker.container.image.v1+json'
     REGISTRY_VERSION_HEADER = 'gitlab-container-registry-version'
@@ -19,6 +20,8 @@ module ContainerRegistry
 
     ACCEPTED_TYPES = [DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE, OCI_MANIFEST_V1_TYPE].freeze
 
+    ACCEPTED_TYPES_RAW = [DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE, OCI_MANIFEST_V1_TYPE, DOCKER_DISTRIBUTION_MANIFEST_LIST_V2_TYPE].freeze
+
     # Taken from: FaradayMiddleware::FollowRedirects
     REDIRECT_CODES = Set.new [301, 302, 303, 307]