Merge pull request #3326 from quay/remove-default-parse

Skip parsing the manifest where applicable

endpoints/v2/manifest.py

@@ -15,7 +15,7 @@ from endpoints.v2 import v2_bp, require_repo_read, require_repo_write
 from endpoints.v2.errors import (ManifestInvalid, ManifestUnknown, NameInvalid, TagExpired,
                                  NameUnknown)
 from image.docker import ManifestException
-from image.docker.schema1 import DOCKER_SCHEMA1_MANIFEST_CONTENT_TYPE
+from image.docker.schema1 import DOCKER_SCHEMA1_MANIFEST_CONTENT_TYPE, DOCKER_SCHEMA1_CONTENT_TYPES
 from image.docker.schema2 import DOCKER_SCHEMA2_CONTENT_TYPES, OCI_CONTENT_TYPES
 from image.docker.schemas import parse_manifest_from_bytes
 from notifications import spawn_notification
@@ -57,15 +57,9 @@ def fetch_manifest_by_tagname(namespace_name, repo_name, manifest_ref):
     # Something went wrong.
     raise ManifestInvalid()
 
-  try:
-    parsed = manifest.get_parsed_manifest()
-  except ManifestException:
-    logger.exception('Got exception when trying to parse manifest `%s`', manifest_ref)
-    raise ManifestInvalid()
-
-  supported = _rewrite_schema_if_necessary(namespace_name, repo_name, manifest_ref, manifest,
-                                           parsed)
-  if supported is None:
+  manifest_bytes, manifest_digest, manifest_media_type = _rewrite_schema_if_necessary(
+    namespace_name, repo_name, manifest_ref, manifest)
+  if manifest_bytes is None:
     raise ManifestUnknown()
 
   track_and_log('pull_repo', repository_ref, analytics_name='pull_repo_100x', analytics_sample=0.01,
@@ -73,11 +67,11 @@ def fetch_manifest_by_tagname(namespace_name, repo_name, manifest_ref):
   metric_queue.repository_pull.Inc(labelvalues=[namespace_name, repo_name, 'v2', True])
 
   return Response(
-    supported.bytes.as_unicode(),
+    manifest_bytes.as_unicode(),
     status=200,
     headers={
-      'Content-Type': '%s; charset=utf-8' % supported.media_type,
-      'Docker-Content-Digest': supported.digest,
+      'Content-Type': '%s; charset=utf-8' % manifest_media_type,
+      'Docker-Content-Digest': manifest_digest,
     },
   )
 
@@ -96,43 +90,49 @@ def fetch_manifest_by_digest(namespace_name, repo_name, manifest_ref):
   if manifest is None:
     raise ManifestUnknown()
 
-  try:
-    parsed = manifest.get_parsed_manifest()
-  except ManifestException:
-    logger.exception('Got exception when trying to parse manifest `%s`', manifest_ref)
-    raise ManifestInvalid()
-
-  supported = _rewrite_schema_if_necessary(namespace_name, repo_name, '$digest', manifest,
-                                           parsed)
-  if supported is None:
+  manifest_bytes, manifest_digest, manifest_media_type = _rewrite_schema_if_necessary(
+    namespace_name, repo_name, '$digest', manifest)
+  if manifest_digest is None:
     raise ManifestUnknown()
 
   track_and_log('pull_repo', repository_ref, manifest_digest=manifest_ref)
   metric_queue.repository_pull.Inc(labelvalues=[namespace_name, repo_name, 'v2', True])
 
-  return Response(supported.bytes.as_unicode(), status=200, headers={
-    'Content-Type': '%s; charset=utf-8' % supported.media_type,
-    'Docker-Content-Digest': supported.digest,
+  return Response(manifest_bytes.as_unicode(), status=200, headers={
+    'Content-Type': '%s; charset=utf-8' % manifest_media_type,
+    'Docker-Content-Digest': manifest_digest,
   })
 
 
-def _rewrite_schema_if_necessary(namespace_name, repo_name, tag_name, manifest, parsed):
+def _rewrite_schema_if_necessary(namespace_name, repo_name, tag_name, manifest):
   # As per the Docker protocol, if the manifest is not schema version 1 and the manifest's
   # media type is not in the Accept header, we return a schema 1 version of the manifest for
   # the amd64+linux platform, if any, or None if none.
   # See: https://docs.docker.com/registry/spec/manifest-v2-2
   mimetypes = [mimetype for mimetype, _ in request.accept_mimetypes]
-  if parsed.media_type in mimetypes:
-    return parsed
+  if manifest.media_type in mimetypes:
+    return manifest.internal_manifest_bytes, manifest.digest, manifest.media_type
 
+  # Short-circuit check: If the mimetypes is empty or just `application/json`, verify we have
+  # a schema 1 manifest and return it.
+  if not mimetypes or mimetypes == ['application/json']:
+    if manifest.media_type in DOCKER_SCHEMA1_CONTENT_TYPES:
+      return manifest.internal_manifest_bytes, manifest.digest, manifest.media_type
+
+  logger.debug('Manifest `%s` not compatible against %s; checking for conversion', manifest.digest,
+               request.accept_mimetypes)
   converted = registry_model.convert_manifest(manifest, namespace_name, repo_name, tag_name,
                                               mimetypes, storage)
   if converted is not None:
-    return converted
+    return converted.bytes, converted.digest, converted.media_type
 
   # For back-compat, we always default to schema 1 if the manifest could not be converted.
-  return registry_model.get_schema1_parsed_manifest(manifest, namespace_name, repo_name, tag_name,
-                                                    storage)
+  schema1 = registry_model.get_schema1_parsed_manifest(manifest, namespace_name, repo_name,
+                                                       tag_name, storage)
+  if schema1 is None:
+    return None, None, None
+
+  return schema1.bytes, schema1.digest, schema1.media_type
 
 
 def _reject_manifest2_schema2(func):

test/registry/protocol_v2.py

@@ -208,7 +208,7 @@ class V2Protocol(RegistryProtocol):
 
     headers = {
       'Authorization': 'Bearer ' + token,
-      'Accept': ','.join(options.accept_mimetypes) if options.accept_mimetypes else '*/*',
+      'Accept': ','.join(options.accept_mimetypes) if options.accept_mimetypes is not None else '*/*',
     }
 
     # Push all blobs.
@@ -340,7 +340,7 @@ class V2Protocol(RegistryProtocol):
 
     headers = {
       'Authorization': 'Bearer ' + token,
-      'Accept': ','.join(options.accept_mimetypes) if options.accept_mimetypes else '*/*',
+      'Accept': ','.join(options.accept_mimetypes) if options.accept_mimetypes is not None else '*/*',
     }
 
     # Build fake manifests.
@@ -530,7 +530,9 @@ class V2Protocol(RegistryProtocol):
     }
 
     if self.schema2:
-      headers['Accept'] = ','.join(options.accept_mimetypes or DOCKER_SCHEMA2_CONTENT_TYPES)
+      headers['Accept'] = ','.join(options.accept_mimetypes
+                                   if options.accept_mimetypes is not None
+                                   else DOCKER_SCHEMA2_CONTENT_TYPES)
 
     manifests = {}
     image_ids = {}

test/registry/registry_tests.py

@@ -1854,3 +1854,27 @@ def test_push_pull_emoji_unicode_direct(pusher, puller, unicode_emoji_images, li
   # Pull the repository to verify.
   puller.pull(liveserver_session, 'devtable', 'newrepo', 'latest', unicode_emoji_images,
               credentials=credentials, options=options)
+
+
+@pytest.mark.parametrize('accepted_mimetypes', [
+  [],
+  ['application/json'],
+])
+def test_push_pull_older_mimetype(pusher, puller, basic_images, liveserver_session, app_reloader,
+                                  accepted_mimetypes):
+  """ Test: Push and pull an image, but override the accepted mimetypes to that sent by older
+            Docker clients.
+  """
+  credentials = ('devtable', 'password')
+
+  # Push a new repository.
+  pusher.push(liveserver_session, 'devtable', 'newrepo', 'latest', basic_images,
+              credentials=credentials)
+
+  # Turn off automatic unicode encoding when building the manifests.
+  options = ProtocolOptions()
+  options.accept_mimetypes = accepted_mimetypes
+
+  # Pull the repository to verify.
+  puller.pull(liveserver_session, 'devtable', 'newrepo', 'latest', basic_images,
+              credentials=credentials, options=options)