Merge pull request #3216 from quay/joseph.schorr/QUAY-1030/interfacing-part3

Change manifest API endpoints to use new registry data interface

data/registry_model/datatypes.py

@@ -10,14 +10,16 @@ class RepositoryReference(datatype('Repository', [])):
     return RepositoryReference(db_id=repo_obj.id)
 
 
-class Label(datatype('Label', ['key', 'value'])):
+class Label(datatype('Label', ['key', 'value', 'uuid', 'source_type_name', 'media_type_name'])):
   """ Label represents a label on a manifest. """
   @classmethod
   def for_label(cls, label):
     if label is None:
       return None
 
-    return Label(db_id=label.id, key=label.key, value=label.value)
+    return Label(db_id=label.id, key=label.key, value=label.value,
+                 uuid=label.uuid, media_type_name=label.media_type.name,
+                 source_type_name=label.source_type.name)
 
 
 class Tag(datatype('Tag', ['name'])):
@@ -30,14 +32,24 @@ class Tag(datatype('Tag', ['name'])):
     return Tag(db_id=repository_tag.id, name=repository_tag.name)
 
 
-class Manifest(datatype('Manifest', ['digest'])):
+class Manifest(datatype('Manifest', ['digest', 'manifest_bytes'])):
   """ Manifest represents a manifest in a repository. """
   @classmethod
-  def for_tag_manifest(cls, tag_manifest):
+  def for_tag_manifest(cls, tag_manifest, legacy_image=None):
     if tag_manifest is None:
       return None
 
-    return Manifest(db_id=tag_manifest.id, digest=tag_manifest.digest)
+    return Manifest(db_id=tag_manifest.id, digest=tag_manifest.digest,
+                    manifest_bytes=tag_manifest.json_data,
+                    inputs=dict(legacy_image=legacy_image))
+
+  @property
+  @requiresinput('legacy_image')
+  def legacy_image(self, legacy_image):
+    """ Returns the legacy Docker V1-style image for this manifest. Note that this
+        will be None for manifests that point to other manifests instead of images.
+    """
+    return legacy_image
 
 
 class LegacyImage(datatype('LegacyImage', ['docker_image_id', 'created', 'comment', 'command',

data/registry_model/interface.py

@@ -34,10 +34,6 @@ class RegistryDataInterface(object):
     """ Looks up the manifest with the given digest under the given repository and returns it
         or None if none. """
 
-  @abstractmethod
-  def create_manifest_label(self, manifest, key, value, source_type_name, media_type_name=None):
-    """ Creates a label on the manifest with the given key and value. """
-
   @abstractmethod
   def get_legacy_images(self, repository_ref):
     """
@@ -50,3 +46,27 @@ class RegistryDataInterface(object):
     Returns the matching LegacyImages under the matching repository, if any. If none,
     returns None.
     """
+
+  @abstractmethod
+  def create_manifest_label(self, manifest, key, value, source_type_name, media_type_name=None):
+    """ Creates a label on the manifest with the given key and value.
+
+        Can raise InvalidLabelKeyException or InvalidMediaTypeException depending
+        on the validation errors.
+    """
+
+  @abstractmethod
+  def list_manifest_labels(self, manifest, key_prefix=None):
+    """ Returns all labels found on the manifest. If specified, the key_prefix will filter the
+        labels returned to those keys that start with the given prefix.
+    """
+
+  @abstractmethod
+  def get_manifest_label(self, manifest, label_uuid):
+    """ Returns the label with the specified UUID on the manifest or None if none. """
+
+  @abstractmethod
+  def delete_manifest_label(self, manifest, label_uuid):
+    """ Delete the label with the specified UUID on the manifest. Returns the label deleted
+        or None if none.
+    """

data/registry_model/registry_pre_oci_model.py

@@ -43,28 +43,28 @@ class PreOCIModel(RegistryDataInterface):
 
     return Manifest.for_tag_manifest(tag_manifest)
 
-  def lookup_manifest_by_digest(self, repository_ref, manifest_digest, allow_dead=False):
+  def lookup_manifest_by_digest(self, repository_ref, manifest_digest, allow_dead=False,
+                                include_legacy_image=False):
     """ Looks up the manifest with the given digest under the given repository and returns it
         or None if none. """
     repo = model.repository.lookup_repository(repository_ref._db_id)
     if repo is None:
       return None
 
+    try:
       tag_manifest = model.tag.load_manifest_by_digest(repo.namespace_user.username,
                                                        repo.name,
-                                                     manifest_digest, allow_dead=allow_dead)
+                                                       manifest_digest,
-    return Manifest.for_tag_manifest(tag_manifest)
+                                                       allow_dead=allow_dead)
-
+    except model.tag.InvalidManifestException:
-  def create_manifest_label(self, manifest, key, value, source_type_name, media_type_name=None):
-    """ Creates a label on the manifest with the given key and value. """
-    try:
-      tag_manifest = database.TagManifest.get(id=manifest._db_id)
-    except database.TagManifest.DoesNotExist:
       return None
 
-    label = model.label.create_manifest_label(tag_manifest, key, value, source_type_name,
+    legacy_image = None
-                                              media_type_name)
+    if include_legacy_image:
-    return Label.for_label(label)
+      legacy_image = self.get_legacy_image(repository_ref, tag_manifest.tag.image.docker_image_id,
+                                           include_parents=True)
+
+    return Manifest.for_tag_manifest(tag_manifest, legacy_image)
 
   def get_legacy_images(self, repository_ref):
     """
@@ -105,5 +105,32 @@ class PreOCIModel(RegistryDataInterface):
 
     return LegacyImage.for_image(image, images_map=parent_images_map)
 
+  def create_manifest_label(self, manifest, key, value, source_type_name, media_type_name=None):
+    """ Creates a label on the manifest with the given key and value. """
+    try:
+      tag_manifest = database.TagManifest.get(id=manifest._db_id)
+    except database.TagManifest.DoesNotExist:
+      return None
+
+    label = model.label.create_manifest_label(tag_manifest, key, value, source_type_name,
+                                              media_type_name)
+    return Label.for_label(label)
+
+  def list_manifest_labels(self, manifest, key_prefix=None):
+    """ Returns all labels found on the manifest. If specified, the key_prefix will filter the
+        labels returned to those keys that start with the given prefix.
+    """
+    labels = model.label.list_manifest_labels(manifest._db_id, prefix_filter=key_prefix)
+    return [Label.for_label(l) for l in labels]
+
+  def get_manifest_label(self, manifest, label_uuid):
+    """ Returns the label with the specified UUID on the manifest or None if none. """
+    return Label.for_label(model.label.get_manifest_label(label_uuid, manifest._db_id))
+
+  def delete_manifest_label(self, manifest, label_uuid):
+    """ Delete the label with the specified UUID on the manifest. Returns the label deleted
+        or None if none.
+    """
+    return Label.for_label(model.label.delete_manifest_label(label_uuid, manifest._db_id))
 
 pre_oci_model = PreOCIModel()

data/registry_model/test/test_pre_oci_model.py

@@ -28,7 +28,7 @@ def test_find_matching_tag(names, expected, pre_oci_model):
 
 
 @pytest.mark.parametrize('repo_namespace, repo_name, expected', [
-  ('devtable', 'simple', {'latest'}),
+  ('devtable', 'simple', {'latest', 'prod'}),
   ('buynlarge', 'orgrepo', {'latest', 'prod'}),
 ])
 def test_get_most_recent_tag(repo_namespace, repo_name, expected, pre_oci_model):
@@ -63,18 +63,18 @@ def test_lookup_manifests(repo_namespace, repo_name, pre_oci_model):
   repository_ref = RepositoryReference.for_repo_obj(repo)
   found_tag = pre_oci_model.find_matching_tag(repository_ref, ['latest'])
   found_manifest = pre_oci_model.get_manifest_for_tag(found_tag)
-  found = pre_oci_model.lookup_manifest_by_digest(repository_ref, found_manifest.digest)
+  found = pre_oci_model.lookup_manifest_by_digest(repository_ref, found_manifest.digest,
+                                                  include_legacy_image=True)
   assert found._db_id == found_manifest._db_id
   assert found.digest == found_manifest.digest
+  assert found.legacy_image
 
 
-def test_create_manifest_label(pre_oci_model):
+def test_lookup_unknown_manifest(pre_oci_model):
   repo = model.repository.get_repository('devtable', 'simple')
   repository_ref = RepositoryReference.for_repo_obj(repo)
-  found_tag = pre_oci_model.find_matching_tag(repository_ref, ['latest'])
+  found = pre_oci_model.lookup_manifest_by_digest(repository_ref, 'sha256:deadbeef')
-  found_manifest = pre_oci_model.get_manifest_for_tag(found_tag)
+  assert found is None
-
-  pre_oci_model.create_manifest_label(found_manifest, 'foo', 'bar', 'internal')
 
 
 @pytest.mark.parametrize('repo_namespace, repo_name', [
@@ -116,3 +116,32 @@ def test_legacy_images(repo_namespace, repo_name, pre_oci_model):
 
   unknown = pre_oci_model.get_legacy_image(repository_ref, 'unknown', include_parents=True)
   assert unknown is None
+
+
+def test_manifest_labels(pre_oci_model):
+  repo = model.repository.get_repository('devtable', 'simple')
+  repository_ref = RepositoryReference.for_repo_obj(repo)
+  found_tag = pre_oci_model.find_matching_tag(repository_ref, ['latest'])
+  found_manifest = pre_oci_model.get_manifest_for_tag(found_tag)
+
+  # Create a new label.
+  created = pre_oci_model.create_manifest_label(found_manifest, 'foo', 'bar', 'api')
+  assert created.key == 'foo'
+  assert created.value == 'bar'
+  assert created.source_type_name == 'api'
+  assert created.media_type_name == 'text/plain'
+
+  # Ensure we can look it up.
+  assert pre_oci_model.get_manifest_label(found_manifest, created.uuid) == created
+
+  # Ensure it is in our list of labels.
+  assert created in pre_oci_model.list_manifest_labels(found_manifest)
+  assert created in pre_oci_model.list_manifest_labels(found_manifest, key_prefix='fo')
+
+  # Ensure it is *not* in our filtered list.
+  assert created not in pre_oci_model.list_manifest_labels(found_manifest, key_prefix='ba')
+
+  # Delete the label and ensure it is gone.
+  assert pre_oci_model.delete_manifest_label(found_manifest, created.uuid)
+  assert pre_oci_model.get_manifest_label(found_manifest, created.uuid) is None
+  assert created not in pre_oci_model.list_manifest_labels(found_manifest)

endpoints/api/manifest.py

@@ -1,23 +1,43 @@
 """ Manage the manifests of a repository. """
-import json
+from flask import request
 
 from app import label_validator
-from flask import request
+from data.model import InvalidLabelKeyException, InvalidMediaTypeException
+from data.registry_model import registry_model
+from digest import digest_tools
 from endpoints.api import (resource, nickname, require_repo_read, require_repo_write,
                            RepositoryParamResource, log_action, validate_json_request,
                            path_param, parse_args, query_param, abort, api,
                            disallow_for_app_repositories)
+from endpoints.api.image import image_dict
 from endpoints.exception import NotFound
-from manifest_models_pre_oci import pre_oci_model as model
-from data.model import InvalidLabelKeyException, InvalidMediaTypeException
-
-from digest import digest_tools
 from util.validation import VALID_LABEL_KEY_REGEX
 
+
 BASE_MANIFEST_ROUTE = '/v1/repository/<apirepopath:repository>/manifest/<regex("{0}"):manifestref>'
 MANIFEST_DIGEST_ROUTE = BASE_MANIFEST_ROUTE.format(digest_tools.DIGEST_PATTERN)
 ALLOWED_LABEL_MEDIA_TYPES = ['text/plain', 'application/json']
 
+def _label_dict(label):
+  return {
+    'id': label.uuid,
+    'key': label.key,
+    'value': label.value,
+    'source_type': label.source_type_name,
+    'media_type': label.media_type_name,
+  }
+
+def _manifest_dict(manifest):
+  image = None
+  if manifest.legacy_image is not None:
+    image = image_dict(manifest.legacy_image, with_history=True)
+
+  return {
+    'digest': manifest.digest,
+    'manifest_data': manifest.manifest_bytes,
+    'image': image,
+  }
+
 
 @resource(MANIFEST_DIGEST_ROUTE)
 @path_param('repository', 'The full path of the repository. e.g. namespace/name')
@@ -28,11 +48,16 @@ class RepositoryManifest(RepositoryParamResource):
   @nickname('getRepoManifest')
   @disallow_for_app_repositories
   def get(self, namespace_name, repository_name, manifestref):
-    manifest = model.get_repository_manifest(namespace_name, repository_name, manifestref)
+    repo_ref = registry_model.lookup_repository(namespace_name, repository_name)
+    if repo_ref is None:
+      raise NotFound()
+
+    manifest = registry_model.lookup_manifest_by_digest(repo_ref, manifestref,
+                                                        include_legacy_image=True)
     if manifest is None:
       raise NotFound()
 
-    return manifest.to_dict()
+    return _manifest_dict(manifest)
 
 
 @resource(MANIFEST_DIGEST_ROUTE + '/labels')
@@ -74,11 +99,20 @@ class RepositoryManifestLabels(RepositoryParamResource):
   @query_param('filter', 'If specified, only labels matching the given prefix will be returned',
                type=str, default=None)
   def get(self, namespace_name, repository_name, manifestref, parsed_args):
-    labels = model.get_manifest_labels(namespace_name, repository_name, manifestref, filter=parsed_args['filter'])
+    repo_ref = registry_model.lookup_repository(namespace_name, repository_name)
+    if repo_ref is None:
+      raise NotFound()
+
+    manifest = registry_model.lookup_manifest_by_digest(repo_ref, manifestref)
+    if manifest is None:
+      raise NotFound()
+
+    labels = registry_model.list_manifest_labels(manifest, parsed_args['filter'])
     if labels is None:
       raise NotFound()
+
     return {
-      'labels': [label.to_dict() for label in labels]
+      'labels': [_label_dict(label) for label in labels]
     }
 
   @require_repo_write
@@ -93,20 +127,28 @@ class RepositoryManifestLabels(RepositoryParamResource):
     if label_validator.has_reserved_prefix(label_data['key']):
       abort(400, message='Label has a reserved prefix')
 
+    repo_ref = registry_model.lookup_repository(namespace_name, repository_name)
+    if repo_ref is None:
+      raise NotFound()
+
+    manifest = registry_model.lookup_manifest_by_digest(repo_ref, manifestref)
+    if manifest is None:
+      raise NotFound()
+
     label = None
     try:
-      label = model.create_manifest_label(namespace_name, 
+      label = registry_model.create_manifest_label(manifest,
-                                          repository_name, 
-                                          manifestref, 
                                                    label_data['key'],
                                                    label_data['value'],
                                                    'api',
                                                    label_data['media_type'])
     except InvalidLabelKeyException:
-      abort(400, message='Label is of an invalid format or missing please use %s format for labels'.format(
+      message = ('Label is of an invalid format or missing please ' +
-            VALID_LABEL_KEY_REGEX))
+                 'use %s format for labels' % VALID_LABEL_KEY_REGEX)
+      abort(400, message=message)
     except InvalidMediaTypeException:
-      abort(400, message='Media type is invalid please use a valid media type of text/plain or application/json')
+      message = 'Media type is invalid please use a valid media type: text/plain, application/json'
+      abort(400, message=message)
 
     if label is None:
       raise NotFound()
@@ -123,7 +165,7 @@ class RepositoryManifestLabels(RepositoryParamResource):
 
     log_action('manifest_label_add', namespace_name, metadata, repo_name=repository_name)
 
-    resp = {'label': label.to_dict()}
+    resp = {'label': _label_dict(label)}
     repo_string = '%s/%s' % (namespace_name, repository_name)
     headers = {
       'Location': api.url_for(ManageRepositoryManifestLabel, repository=repo_string,
@@ -143,11 +185,19 @@ class ManageRepositoryManifestLabel(RepositoryParamResource):
   @disallow_for_app_repositories
   def get(self, namespace_name, repository_name, manifestref, labelid):
     """ Retrieves the label with the specific ID under the manifest. """
-    label = model.get_manifest_label(namespace_name, repository_name, manifestref, labelid)
+    repo_ref = registry_model.lookup_repository(namespace_name, repository_name)
+    if repo_ref is None:
+      raise NotFound()
+
+    manifest = registry_model.lookup_manifest_by_digest(repo_ref, manifestref)
+    if manifest is None:
+      raise NotFound()
+
+    label = registry_model.get_manifest_label(manifest, labelid)
     if label is None:
       raise NotFound()
 
-    return label.to_dict()
+    return _label_dict(label)
 
 
   @require_repo_write
@@ -155,7 +205,15 @@ class ManageRepositoryManifestLabel(RepositoryParamResource):
   @disallow_for_app_repositories
   def delete(self, namespace_name, repository_name, manifestref, labelid):
     """ Deletes an existing label from a manifest. """
-    deleted = model.delete_manifest_label(namespace_name, repository_name, manifestref, labelid)
+    repo_ref = registry_model.lookup_repository(namespace_name, repository_name)
+    if repo_ref is None:
+      raise NotFound()
+
+    manifest = registry_model.lookup_manifest_by_digest(repo_ref, manifestref)
+    if manifest is None:
+      raise NotFound()
+
+    deleted = registry_model.delete_manifest_label(manifest, labelid)
     if deleted is None:
       raise NotFound()
 
@@ -170,4 +228,3 @@ class ManageRepositoryManifestLabel(RepositoryParamResource):
 
     log_action('manifest_label_delete', namespace_name, metadata, repo_name=repository_name)
     return '', 204
-

endpoints/api/manifest_models_interface.py

@@ -1,118 +0,0 @@
-from abc import ABCMeta, abstractmethod
-from collections import namedtuple
-
-from endpoints.api.image import image_dict
-from six import add_metaclass
-
-
-class ManifestLabel(
-  namedtuple('ManifestLabel', [
-    'uuid',
-    'key',
-    'value',
-    'source_type_name',
-    'media_type_name',
-    ])):
-  """
-  ManifestLabel represents a label on a manifest
-  :type uuid: string
-  :type key: string 
-  :type value: string 
-  :type source_type_name: string  
-  :type media_type_name: string 
-  """ 
-  def to_dict(self):
-    return {
-      'id': self.uuid,
-      'key': self.key,
-      'value': self.value,
-      'source_type': self.source_type_name,
-      'media_type': self.media_type_name,
-    }
-
-  
-class ManifestAndImage(
-  namedtuple('ManifestAndImage', [
-    'digest',
-    'manifest_data',
-    'image',
-    ])):
-  def to_dict(self):
-    return {
-      'digest': self.digest,
-      'manifest_data': self.manifest_data,
-      'image': image_dict(self.image),
-    }
-
-  
-@add_metaclass(ABCMeta)
-class ManifestLabelInterface(object):
-  """
-  Data interface that the manifest labels API uses
-  """
-  
-  @abstractmethod
-  def get_manifest_labels(self, namespace_name, repository_name, manifestref, filter=None):
-    """
-    
-    Args:
-      namespace_name: string
-      repository_name: string
-      manifestref: string
-      filter: string
-
-    Returns:
-      list(ManifestLabel) or None
-
-    """   
-
-  @abstractmethod
-  def create_manifest_label(self, namespace_name, repository_name, manifestref, key, value, source_type_name, media_type_name):
-    """
-    
-    Args:
-      namespace_name: string
-      repository_name: string
-      manifestref: string
-      key: string
-      value: string
-      source_type_name: string 
-      media_type_name: string
-
-    Returns:
-      ManifestLabel or None 
-    """
-  
-  @abstractmethod
-  def get_manifest_label(self, namespace_name, repository_name, manifestref, label_uuid):
-    """
-    
-    Args:
-      namespace_name: string
-      repository_name: string
-      manifestref: string
-      label_uuid: string
-
-    Returns:
-      ManifestLabel or None
-    """
-  
-  @abstractmethod
-  def delete_manifest_label(self, namespace_name, repository_name, manifestref, label_uuid):
-    """
-    
-    Args:
-      namespace_name: string
-      repository_name: string
-      manifestref: string
-      label_uuid: string
-
-    Returns:
-      ManifestLabel or None
-    """
-
-  @abstractmethod
-  def get_repository_manifest(self, namespace_name, repository_name, digest):
-    """
-    Returns the manifest and image for the manifest with the specified digest, if any.
-    """

endpoints/api/manifest_models_pre_oci.py

@@ -1,74 +0,0 @@
-import json
-
-from manifest_models_interface import ManifestLabel, ManifestLabelInterface, ManifestAndImage
-from data import model
-from data.registry_model import registry_model
-
-
-class ManifestLabelPreOCI(ManifestLabelInterface):
-  def get_manifest_labels(self, namespace_name, repository_name, manifestref, filter=None):
-    try:
-      tag_manifest = model.tag.load_manifest_by_digest(namespace_name, repository_name, manifestref)
-    except model.DataModelException:
-      return None
-
-    labels = model.label.list_manifest_labels(tag_manifest, prefix_filter=filter)
-    return [self._label(l) for l in labels]
-
-  def create_manifest_label(self, namespace_name, repository_name, manifestref, key, value, source_type_name, media_type_name):
-    try:
-      tag_manifest = model.tag.load_manifest_by_digest(namespace_name, repository_name, manifestref)
-    except model.DataModelException:
-      return None
-
-    return self._label(model.label.create_manifest_label(tag_manifest, key, value, source_type_name, media_type_name))
-  
-  def get_manifest_label(self, namespace_name, repository_name, manifestref, label_uuid):
-    try:
-      tag_manifest = model.tag.load_manifest_by_digest(namespace_name, repository_name, manifestref)
-    except model.DataModelException:
-      return None
-
-    return self._label(model.label.get_manifest_label(label_uuid, tag_manifest))
-
-  def delete_manifest_label(self, namespace_name, repository_name, manifestref, label_uuid):
-    try:
-      tag_manifest = model.tag.load_manifest_by_digest(namespace_name, repository_name, manifestref)
-    except model.DataModelException:
-      return None
-
-    return self._label(model.label.delete_manifest_label(label_uuid, tag_manifest))
-  
-  def get_repository_manifest(self, namespace_name, repository_name, digest):
-    try:
-      tag_manifest = model.tag.load_manifest_by_digest(namespace_name, repository_name, digest,
-                                                       allow_dead=True)
-    except model.DataModelException:
-      return None
-
-    # TODO: remove this dependency on image once we've moved to the new data model.
-    repo_ref = registry_model.lookup_repository(namespace_name, repository_name)
-    if repo_ref is None:
-      return None
-
-    image = registry_model.get_legacy_image(repo_ref, tag_manifest.tag.image.docker_image_id,
-                                            include_parents=True)
-    if image is None:
-      return None
-
-    manifest_data = json.loads(tag_manifest.json_data)
-    return ManifestAndImage(digest=digest, manifest_data=manifest_data, image=image)
-
-
-  def _label(self, label_obj):
-    if not label_obj:
-      return None
-    return ManifestLabel(
-      uuid=label_obj.uuid,
-      key=label_obj.key,
-      value=label_obj.value,
-      source_type_name=label_obj.source_type.name,
-      media_type_name=label_obj.media_type.name,
-    )
-
-pre_oci_model = ManifestLabelPreOCI()