From 9f96e595ac23b0196da5941048366df278ae5fef Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 17 Aug 2018 16:45:27 -0400 Subject: [PATCH 1/7] Change build component labeling to use new registry interface --- buildman/component/buildcomponent.py | 29 +++++++------- data/model/label.py | 3 -- data/registry_model/datatypes.py | 13 +++++++ data/registry_model/interface.py | 18 +++++++++ data/registry_model/registry_pre_oci_model.py | 39 ++++++++++++++++++- .../registry_model/test/test_pre_oci_model.py | 36 +++++++++++++++++ 6 files changed, 121 insertions(+), 17 deletions(-) diff --git a/buildman/component/buildcomponent.py b/buildman/component/buildcomponent.py index 4b25c4636..d0e9edfae 100644 --- a/buildman/component/buildcomponent.py +++ b/buildman/component/buildcomponent.py @@ -17,9 +17,10 @@ from buildman.jobutil.buildstatus import StatusHandler from buildman.jobutil.workererror import WorkerError from app import app -from data import model from data.database import BUILD_PHASE, UseThenDisconnect from data.model import InvalidRepositoryBuildException +from data.registry_model import registry_model +from data.registry_model.datatypes import RepositoryReference from util import slash_join HEARTBEAT_DELTA = datetime.timedelta(seconds=60) @@ -29,6 +30,9 @@ INITIAL_TIMEOUT = 25 SUPPORTED_WORKER_VERSIONS = ['0.3'] +# Label which marks a manifest with its source build ID. +INTERNAL_LABEL_BUILD_UUID = 'quay.build.uuid' + logger = logging.getLogger(__name__) class ComponentStatus(object): @@ -357,18 +361,17 @@ class BuildComponent(BaseComponent): # Label the pushed manifests with the build metadata. manifest_digests = kwargs.get('digests') or [] - for digest in manifest_digests: - with UseThenDisconnect(app.config): - try: - manifest = model.tag.load_manifest_by_digest(self._current_job.namespace, - self._current_job.repo_name, digest) - model.label.create_manifest_label(manifest, model.label.INTERNAL_LABEL_BUILD_UUID, - build_id, 'internal', 'text/plain') - except model.InvalidManifestException: - logger.debug('Could not find built manifest with digest %s under repo %s/%s for build %s', - digest, self._current_job.namespace, self._current_job.repo_name, - build_id) - continue + repository = registry_model.lookup_repository(self._current_job.namespace, + self._current_job.repo_name) + if repository is not None: + for digest in manifest_digests: + with UseThenDisconnect(app.config): + manifest = registry_model.lookup_manifest_by_digest(repository, digest) + if manifest is None: + continue + + registry_model.create_manifest_label(manifest, INTERNAL_LABEL_BUILD_UUID, + build_id, 'internal', 'text/plain') # Send the notification that the build has completed successfully. self._current_job.send_notification('build_success', diff --git a/data/model/label.py b/data/model/label.py index 1592efaa6..3e461e2d7 100644 --- a/data/model/label.py +++ b/data/model/label.py @@ -11,9 +11,6 @@ from util.validation import is_json logger = logging.getLogger(__name__) -# Label which marks a manifest with its source build ID. -INTERNAL_LABEL_BUILD_UUID = 'quay.build.uuid' - @lru_cache(maxsize=1) def get_label_source_types(): diff --git a/data/registry_model/datatypes.py b/data/registry_model/datatypes.py index 3f2cae187..4cf2a0015 100644 --- a/data/registry_model/datatypes.py +++ b/data/registry_model/datatypes.py @@ -7,6 +7,9 @@ class RepositoryReference(object): @classmethod def for_repo_obj(cls, repo_obj): + if repo_obj is None: + return None + return RepositoryReference(repo_obj.id) @@ -18,3 +21,13 @@ class Tag(namedtuple('Tag', ['id', 'name'])): return None return Tag(id=repository_tag.id, name=repository_tag.name) + + +class Manifest(namedtuple('Manifest', ['id', 'digest'])): + """ Manifest represents a manifest in a repository. """ + @classmethod + def for_tag_manifest(cls, tag_manifest): + if tag_manifest is None: + return None + + return Manifest(id=tag_manifest.id, digest=tag_manifest.digest) diff --git a/data/registry_model/interface.py b/data/registry_model/interface.py index e67366733..d92ccdea9 100644 --- a/data/registry_model/interface.py +++ b/data/registry_model/interface.py @@ -19,3 +19,21 @@ class RegistryDataInterface(object): """ Returns the most recently pushed alive tag in the repository, if any. If none, returns None. """ + + @abstractmethod + def lookup_repository(self, namespace_name, repo_name, kind_filter=None): + """ Looks up and returns a reference to the repository with the given namespace and name, + or None if none. """ + + @abstractmethod + def get_manifest_for_tag(self, tag): + """ Returns the manifest associated with the given tag. """ + + @abstractmethod + def lookup_manifest_by_digest(self, repository_ref, manifest_digest, allow_dead=False): + """ 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. """ diff --git a/data/registry_model/registry_pre_oci_model.py b/data/registry_model/registry_pre_oci_model.py index 5cd6ed631..69443ec71 100644 --- a/data/registry_model/registry_pre_oci_model.py +++ b/data/registry_model/registry_pre_oci_model.py @@ -1,6 +1,7 @@ +from data import database from data import model from data.registry_model.interface import RegistryDataInterface -from data.registry_model.datatypes import Tag +from data.registry_model.datatypes import Tag, RepositoryReference, Manifest class PreOCIModel(RegistryDataInterface): @@ -23,5 +24,41 @@ class PreOCIModel(RegistryDataInterface): found_tag = model.tag.get_most_recent_tag(repository_ref.repo_id) return Tag.for_repository_tag(found_tag) + def lookup_repository(self, namespace_name, repo_name, kind_filter=None): + """ Looks up and returns a reference to the repository with the given namespace and name, + or None if none. """ + repo = model.repository.get_repository(namespace_name, repo_name, kind_filter=kind_filter) + return RepositoryReference.for_repo_obj(repo) + + def get_manifest_for_tag(self, tag): + """ Returns the manifest associated with the given tag. """ + try: + tag_manifest = database.TagManifest.get(tag_id=tag.id) + except database.TagManifest.DoesNotExist: + return + + return Manifest.for_tag_manifest(tag_manifest) + + def lookup_manifest_by_digest(self, repository_ref, manifest_digest, allow_dead=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.repo_id) + if repo is None: + return None + + tag_manifest = model.tag.load_manifest_by_digest(repo.namespace_user.username, + repo.name, + manifest_digest, allow_dead=allow_dead) + return Manifest.for_tag_manifest(tag_manifest) + + 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.id) + except database.TagManifest.DoesNotExist: + return + + model.label.create_manifest_label(tag_manifest, key, value, source_type_name, media_type_name) + pre_oci_model = PreOCIModel() diff --git a/data/registry_model/test/test_pre_oci_model.py b/data/registry_model/test/test_pre_oci_model.py index 6e81c3fff..7e46b8cc6 100644 --- a/data/registry_model/test/test_pre_oci_model.py +++ b/data/registry_model/test/test_pre_oci_model.py @@ -39,3 +39,39 @@ def test_get_most_recent_tag(repo_namespace, repo_name, expected, pre_oci_model) assert found is None else: assert found.name in expected + + +@pytest.mark.parametrize('repo_namespace, repo_name, expected', [ + ('devtable', 'simple', True), + ('buynlarge', 'orgrepo', True), + ('buynlarge', 'unknownrepo', False), +]) +def test_lookup_repository(repo_namespace, repo_name, expected, pre_oci_model): + repo_ref = pre_oci_model.lookup_repository(repo_namespace, repo_name) + if expected: + assert repo_ref + else: + assert repo_ref is None + + +@pytest.mark.parametrize('repo_namespace, repo_name', [ + ('devtable', 'simple'), + ('buynlarge', 'orgrepo'), +]) +def test_lookup_manifests(repo_namespace, repo_name, pre_oci_model): + repo = model.repository.get_repository(repo_namespace, repo_name) + 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) + assert found.id == found_manifest.id + assert found.digest == found_manifest.digest + + +def test_create_manifest_label(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) + + pre_oci_model.create_manifest_label(found_manifest, 'foo', 'bar', 'internal') From 8aafbf8b8c84b3267cec6df2aa9545870e249211 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 17 Aug 2018 17:57:27 -0400 Subject: [PATCH 2/7] Switch the registry data model types to our own class constructor This allows us to hide the DB ID from external-to-the-package users of the types and will allow us to add model-level caching as well --- data/registry_model/datatypes.py | 38 ++++++++++++++----- data/registry_model/registry_pre_oci_model.py | 12 +++--- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/data/registry_model/datatypes.py b/data/registry_model/datatypes.py index 4cf2a0015..8a65f4ec8 100644 --- a/data/registry_model/datatypes.py +++ b/data/registry_model/datatypes.py @@ -1,33 +1,51 @@ -from collections import namedtuple +def datatype(name, static_fields): + """ Defines a base class for a datatype that will represent a row from the database, + in an abstracted form. + """ + class DataType(object): + __name__ = name -class RepositoryReference(object): + def __init__(self, *args, **kwargs): + self._db_id = kwargs.pop('db_id', None) + self._fields = kwargs + + for name in static_fields: + assert name in self._fields, 'Missing field %s' % name + + def __getattr__(self, name): + if name in static_fields: + return self._fields[name] + + return None + + return DataType + + +class RepositoryReference(datatype('Repository', [])): """ RepositoryReference is a reference to a repository, passed to registry interface methods. """ - def __init__(self, repo_id): - self.repo_id = repo_id - @classmethod def for_repo_obj(cls, repo_obj): if repo_obj is None: return None - return RepositoryReference(repo_obj.id) + return RepositoryReference(db_id=repo_obj.id) -class Tag(namedtuple('Tag', ['id', 'name'])): +class Tag(datatype('Tag', ['name'])): """ Tag represents a tag in a repository, which points to a manifest or image. """ @classmethod def for_repository_tag(cls, repository_tag): if repository_tag is None: return None - return Tag(id=repository_tag.id, name=repository_tag.name) + return Tag(db_id=repository_tag.id, name=repository_tag.name) -class Manifest(namedtuple('Manifest', ['id', 'digest'])): +class Manifest(datatype('Manifest', ['digest'])): """ Manifest represents a manifest in a repository. """ @classmethod def for_tag_manifest(cls, tag_manifest): if tag_manifest is None: return None - return Manifest(id=tag_manifest.id, digest=tag_manifest.digest) + return Manifest(db_id=tag_manifest.id, digest=tag_manifest.digest) diff --git a/data/registry_model/registry_pre_oci_model.py b/data/registry_model/registry_pre_oci_model.py index 69443ec71..0249955cb 100644 --- a/data/registry_model/registry_pre_oci_model.py +++ b/data/registry_model/registry_pre_oci_model.py @@ -1,3 +1,5 @@ +# pylint: disable=protected-access + from data import database from data import model from data.registry_model.interface import RegistryDataInterface @@ -14,14 +16,14 @@ class PreOCIModel(RegistryDataInterface): """ Finds an alive tag in the repository matching one of the given tag names and returns it or None if none. """ - found_tag = model.tag.find_matching_tag(repository_ref.repo_id, tag_names) + found_tag = model.tag.find_matching_tag(repository_ref._db_id, tag_names) return Tag.for_repository_tag(found_tag) def get_most_recent_tag(self, repository_ref): """ Returns the most recently pushed alive tag in the repository, if any. If none, returns None. """ - found_tag = model.tag.get_most_recent_tag(repository_ref.repo_id) + found_tag = model.tag.get_most_recent_tag(repository_ref._db_id) return Tag.for_repository_tag(found_tag) def lookup_repository(self, namespace_name, repo_name, kind_filter=None): @@ -33,7 +35,7 @@ class PreOCIModel(RegistryDataInterface): def get_manifest_for_tag(self, tag): """ Returns the manifest associated with the given tag. """ try: - tag_manifest = database.TagManifest.get(tag_id=tag.id) + tag_manifest = database.TagManifest.get(tag_id=tag._db_id) except database.TagManifest.DoesNotExist: return @@ -42,7 +44,7 @@ class PreOCIModel(RegistryDataInterface): def lookup_manifest_by_digest(self, repository_ref, manifest_digest, allow_dead=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.repo_id) + repo = model.repository.lookup_repository(repository_ref._db_id) if repo is None: return None @@ -54,7 +56,7 @@ class PreOCIModel(RegistryDataInterface): 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.id) + tag_manifest = database.TagManifest.get(id=manifest._db_id) except database.TagManifest.DoesNotExist: return From 254f06e63446d23e77e1be77ff2efbf336d4a015 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 17 Aug 2018 18:34:10 -0400 Subject: [PATCH 3/7] Implement legacy image portion of the data model This also makes use of the newly created input system --- data/registry_model/datatype.py | 49 ++++++++++++++ data/registry_model/datatypes.py | 65 ++++++++++++------- data/registry_model/interface.py | 13 ++++ data/registry_model/registry_pre_oci_model.py | 43 +++++++++++- .../registry_model/test/test_pre_oci_model.py | 41 ++++++++++++ 5 files changed, 188 insertions(+), 23 deletions(-) create mode 100644 data/registry_model/datatype.py diff --git a/data/registry_model/datatype.py b/data/registry_model/datatype.py new file mode 100644 index 000000000..6571e61b4 --- /dev/null +++ b/data/registry_model/datatype.py @@ -0,0 +1,49 @@ +# pylint: disable=protected-access + +from functools import wraps, total_ordering + +def datatype(name, static_fields): + """ Defines a base class for a datatype that will represent a row from the database, + in an abstracted form. + """ + @total_ordering + class DataType(object): + __name__ = name + + def __init__(self, **kwargs): + self._db_id = kwargs.pop('db_id', None) + self._inputs = kwargs.pop('inputs', None) + self._fields = kwargs + + for name in static_fields: + assert name in self._fields, 'Missing field %s' % name + + def __eq__(self, other): + return self._db_id == other._db_id + + def __lt__(self, other): + return self._db_id < other._db_id + + def __getattr__(self, name): + if name in static_fields: + return self._fields[name] + + return None + + return DataType + + +def requiresinput(input_name): + """ Marks a property on the data type as requiring an input to be invoked. """ + def inner(func): + @wraps(func) + def wrapper(self, *args, **kwargs): + if self._inputs.get(input_name) is None: + raise Exception('Cannot invoke function with missing input `%s`', input_name) + + kwargs[input_name] = self._inputs[input_name] + result = func(self, *args, **kwargs) + return result + + return wrapper + return inner diff --git a/data/registry_model/datatypes.py b/data/registry_model/datatypes.py index 8a65f4ec8..7487d1564 100644 --- a/data/registry_model/datatypes.py +++ b/data/registry_model/datatypes.py @@ -1,25 +1,4 @@ -def datatype(name, static_fields): - """ Defines a base class for a datatype that will represent a row from the database, - in an abstracted form. - """ - class DataType(object): - __name__ = name - - def __init__(self, *args, **kwargs): - self._db_id = kwargs.pop('db_id', None) - self._fields = kwargs - - for name in static_fields: - assert name in self._fields, 'Missing field %s' % name - - def __getattr__(self, name): - if name in static_fields: - return self._fields[name] - - return None - - return DataType - +from data.registry_model.datatype import datatype, requiresinput class RepositoryReference(datatype('Repository', [])): """ RepositoryReference is a reference to a repository, passed to registry interface methods. """ @@ -49,3 +28,45 @@ class Manifest(datatype('Manifest', ['digest'])): return None return Manifest(db_id=tag_manifest.id, digest=tag_manifest.digest) + + +class LegacyImage(datatype('LegacyImage', ['docker_image_id', 'created', 'comment', 'command', + 'image_size', 'uploading'])): + """ LegacyImage represents a Docker V1-style image found in a repository. """ + @classmethod + def for_image(cls, image, images_map=None, tags_map=None): + if image is None: + return None + + return LegacyImage(db_id=image.id, + inputs=dict(images_map=images_map, tags_map=tags_map, + ancestor_id_list=image.ancestor_id_list()), + docker_image_id=image.docker_image_id, + created=image.created, + comment=image.comment, + command=image.command, + image_size=image.storage.image_size, + uploading=image.storage.uploading) + + @property + @requiresinput('images_map') + @requiresinput('ancestor_id_list') + def parents(self, images_map, ancestor_id_list): + """ Returns the parent images for this image. Raises an exception if the parents have + not been loaded before this property is invoked. + """ + return [LegacyImage.for_image(images_map[ancestor_id], images_map=images_map) + for ancestor_id in ancestor_id_list + if images_map.get(ancestor_id)] + + @property + @requiresinput('tags_map') + def tags(self, tags_map): + """ Returns the tags pointing to this image. Raises an exception if the tags have + not been loaded before this property is invoked. + """ + tags = tags_map.get(self._db_id) + if not tags: + return [] + + return [Tag.for_repository_tag(tag) for tag in tags] diff --git a/data/registry_model/interface.py b/data/registry_model/interface.py index d92ccdea9..9961f7dda 100644 --- a/data/registry_model/interface.py +++ b/data/registry_model/interface.py @@ -37,3 +37,16 @@ class RegistryDataInterface(object): @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): + """ + Returns an iterator of all the LegacyImage's defined in the matching repository. + """ + + @abstractmethod + def get_legacy_image(self, repository_ref, docker_image_id, include_parents=False): + """ + Returns the matching LegacyImages under the matching repository, if any. If none, + returns None. + """ diff --git a/data/registry_model/registry_pre_oci_model.py b/data/registry_model/registry_pre_oci_model.py index 0249955cb..6e37814de 100644 --- a/data/registry_model/registry_pre_oci_model.py +++ b/data/registry_model/registry_pre_oci_model.py @@ -1,9 +1,11 @@ # pylint: disable=protected-access +from collections import defaultdict + from data import database from data import model from data.registry_model.interface import RegistryDataInterface -from data.registry_model.datatypes import Tag, RepositoryReference, Manifest +from data.registry_model.datatypes import Tag, RepositoryReference, Manifest, LegacyImage class PreOCIModel(RegistryDataInterface): @@ -62,5 +64,44 @@ class PreOCIModel(RegistryDataInterface): model.label.create_manifest_label(tag_manifest, key, value, source_type_name, media_type_name) + def get_legacy_images(self, repository_ref): + """ + Returns an iterator of all the LegacyImage's defined in the matching repository. + """ + repo = model.repository.lookup_repository(repository_ref._db_id) + if repo is None: + return None + + all_images = model.image.get_repository_images_without_placements(repo) + all_images_map = {image.id: image for image in all_images} + + all_tags = model.tag.list_repository_tags(repo.namespace_user.username, repo.name) + tags_by_image_id = defaultdict(list) + for tag in all_tags: + tags_by_image_id[tag.image_id].append(tag) + + return [LegacyImage.for_image(image, images_map=all_images_map, tags_map=tags_by_image_id) + for image in all_images] + + def get_legacy_image(self, repository_ref, docker_image_id, include_parents=False): + """ + Returns the matching LegacyImages under the matching repository, if any. If none, + returns None. + """ + repo = model.repository.lookup_repository(repository_ref._db_id) + if repo is None: + return None + + image = model.image.get_image(repository_ref._db_id, docker_image_id) + if image is None: + return None + + parent_images_map = None + if include_parents: + parent_images = model.image.get_parent_images(repo.namespace_user.username, repo.name, image) + parent_images_map = {image.id: image for image in parent_images} + + return LegacyImage.for_image(image, images_map=parent_images_map) + pre_oci_model = PreOCIModel() diff --git a/data/registry_model/test/test_pre_oci_model.py b/data/registry_model/test/test_pre_oci_model.py index 7e46b8cc6..78b5ca545 100644 --- a/data/registry_model/test/test_pre_oci_model.py +++ b/data/registry_model/test/test_pre_oci_model.py @@ -75,3 +75,44 @@ def test_create_manifest_label(pre_oci_model): found_manifest = pre_oci_model.get_manifest_for_tag(found_tag) pre_oci_model.create_manifest_label(found_manifest, 'foo', 'bar', 'internal') + + +@pytest.mark.parametrize('repo_namespace, repo_name', [ + ('devtable', 'simple'), + ('devtable', 'complex'), + ('devtable', 'history'), + ('buynlarge', 'orgrepo'), +]) +def test_legacy_images(repo_namespace, repo_name, pre_oci_model): + repository_ref = pre_oci_model.lookup_repository(repo_namespace, repo_name) + legacy_images = pre_oci_model.get_legacy_images(repository_ref) + assert len(legacy_images) + + found_tags = set() + for image in legacy_images: + found_image = pre_oci_model.get_legacy_image(repository_ref, image.docker_image_id, + include_parents=True) + + assert found_image.docker_image_id == image.docker_image_id + assert found_image.parents == image.parents + + # Check that the tags list can be retrieved. + assert image.tags is not None + found_tags.update({tag.name for tag in image.tags}) + + # Check against the actual DB row. + model_image = model.image.get_image(repository_ref._db_id, found_image.docker_image_id) + assert model_image.id == found_image._db_id + assert ([pid for pid in model_image.ancestor_id_list()] == + [p._db_id for p in found_image.parents]) + + # Try without parents and ensure it raises an exception. + found_image = pre_oci_model.get_legacy_image(repository_ref, image.docker_image_id, + include_parents=False) + with pytest.raises(Exception): + assert not found_image.parents + + assert found_tags + + unknown = pre_oci_model.get_legacy_image(repository_ref, 'unknown', include_parents=True) + assert unknown is None From 7b95082a99b6d38643162b53909adc16132731cf Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 20 Aug 2018 14:37:03 -0400 Subject: [PATCH 4/7] Change image API endpoint to use new registry model --- endpoints/api/image.py | 44 +++++++++++--- endpoints/api/image_models_interface.py | 77 ------------------------- endpoints/api/image_models_pre_oci.py | 56 ------------------ 3 files changed, 37 insertions(+), 140 deletions(-) delete mode 100644 endpoints/api/image_models_interface.py delete mode 100644 endpoints/api/image_models_pre_oci.py diff --git a/endpoints/api/image.py b/endpoints/api/image.py index 522fdf951..f126ed89e 100644 --- a/endpoints/api/image.py +++ b/endpoints/api/image.py @@ -1,11 +1,36 @@ """ List and lookup repository images. """ +import json +from data.registry_model import registry_model from endpoints.api import (resource, nickname, require_repo_read, RepositoryParamResource, - path_param, disallow_for_app_repositories) -from endpoints.api.image_models_pre_oci import pre_oci_model as model + path_param, disallow_for_app_repositories, format_date) from endpoints.exception import NotFound +def _image_dict(image, with_history=False, with_tags=False): + image_data = { + 'id': image.docker_image_id, + 'created': format_date(image.created), + 'comment': image.comment, + 'command': json.loads(image.command) if image.command else None, + 'size': image.image_size, + 'uploading': image.uploading, + 'sort_index': len(image.parents), + } + + if with_tags: + image_data['tags'] = [tag.name for tag in image.tags] + + if with_history: + image_data['history'] = [_image_dict(parent, with_history, with_tags) + for parent in image.parents] + + # Calculate the ancestors string, with the DBID's replaced with the docker IDs. + parent_docker_ids = [parent_image.docker_image_id for parent_image in image.parents] + image_data['ancestors'] = '/{0}/'.format('/'.join(parent_docker_ids)) + return image_data + + @resource('/v1/repository//image/') @path_param('repository', 'The full path of the repository. e.g. namespace/name') class RepositoryImageList(RepositoryParamResource): @@ -16,11 +41,12 @@ class RepositoryImageList(RepositoryParamResource): @disallow_for_app_repositories def get(self, namespace, repository): """ List the images for the specified repository. """ - images = model.get_repository_images(namespace, repository) - if images is None: + repo_ref = registry_model.lookup_repository(namespace, repository) + if repo_ref is None: raise NotFound() - return {'images': [image.to_dict() for image in images]} + images = registry_model.get_legacy_images(repo_ref) + return {'images': [_image_dict(image, with_tags=True) for image in images]} @resource('/v1/repository//image/') @@ -34,8 +60,12 @@ class RepositoryImage(RepositoryParamResource): @disallow_for_app_repositories def get(self, namespace, repository, image_id): """ Get the information available for the specified image. """ - image = model.get_repository_image(namespace, repository, image_id) + repo_ref = registry_model.lookup_repository(namespace, repository) + if repo_ref is None: + raise NotFound() + + image = registry_model.get_legacy_image(repo_ref, image_id, include_parents=True) if image is None: raise NotFound() - return image.to_dict() + return _image_dict(image, with_history=True) diff --git a/endpoints/api/image_models_interface.py b/endpoints/api/image_models_interface.py deleted file mode 100644 index 0cd5c0a8f..000000000 --- a/endpoints/api/image_models_interface.py +++ /dev/null @@ -1,77 +0,0 @@ -import json - -from endpoints.api import format_date - -from abc import ABCMeta, abstractmethod -from collections import namedtuple -from six import add_metaclass - -class Image(namedtuple('Image', ['docker_image_id', 'created', 'comment', 'command', 'image_size', - 'uploading', 'parents'])): - """ - Image represents an image. - :type name: string - """ - - def to_dict(self): - image_data = { - 'id': self.docker_image_id, - 'created': format_date(self.created), - 'comment': self.comment, - 'command': json.loads(self.command) if self.command else None, - 'size': self.image_size, - 'uploading': self.uploading, - 'sort_index': len(self.parents), - } - - # Calculate the ancestors string, with the DBID's replaced with the docker IDs. - parent_docker_ids = [parent_image.docker_image_id for parent_image in self.parents] - image_data['ancestors'] = '/{0}/'.format('/'.join(parent_docker_ids)) - - return image_data - -class ImageWithTags(namedtuple('ImageWithTags', ['image', 'tag_names'])): - """ - ImageWithTags represents an image, along with the tags that point to it. - :type image: Image - :type tag_names: list of string - """ - - def to_dict(self): - image_dict = self.image.to_dict() - image_dict['tags'] = self.tag_names - return image_dict - - -class ImageWithHistory(namedtuple('ImageWithHistory', ['image'])): - """ - ImageWithHistory represents an image, along with its full parent image dictionaries. - :type image: Image - :type history: list of Image parents (name is old and must be kept for compat) - """ - - def to_dict(self): - image_dict = self.image.to_dict() - image_dict['history'] = [parent_image.to_dict() for parent_image in self.image.parents] - return image_dict - - -@add_metaclass(ABCMeta) -class ImageInterface(object): - """ - Interface that represents all data store interactions required by the image API endpoint. - """ - - @abstractmethod - def get_repository_images(self, namespace_name, repo_name): - """ - Returns an iterator of all the ImageWithTag's defined in the matching repository. If the - repository doesn't exist, returns None. - """ - - @abstractmethod - def get_repository_image(self, namespace_name, repo_name, docker_image_id): - """ - Returns the matching ImageWithHistory under the matching repository, if any. If none, - returns None. - """ diff --git a/endpoints/api/image_models_pre_oci.py b/endpoints/api/image_models_pre_oci.py deleted file mode 100644 index e29c294b0..000000000 --- a/endpoints/api/image_models_pre_oci.py +++ /dev/null @@ -1,56 +0,0 @@ -from collections import defaultdict -from data import model -from endpoints.api.image_models_interface import (ImageInterface, ImageWithHistory, ImageWithTags, - Image) - -def _image(namespace_name, repo_name, image, all_images, include_parents=True): - parent_image_tuples = [] - if include_parents: - parent_images = [all_images[ancestor_id] for ancestor_id in image.ancestor_id_list() - if all_images.get(ancestor_id)] - parent_image_tuples = [_image(namespace_name, repo_name, parent_image, all_images, False) - for parent_image in parent_images] - - return Image(image.docker_image_id, image.created, image.comment, image.command, - image.storage.image_size, image.storage.uploading, parent_image_tuples) - -def _tag_names(image, tags_by_docker_id): - return [tag.name for tag in tags_by_docker_id.get(image.docker_image_id, [])] - - -class PreOCIModel(ImageInterface): - """ - PreOCIModel implements the data model for the Image API using a database schema - before it was changed to support the OCI specification. - """ - def get_repository_images(self, namespace_name, repo_name): - repo = model.repository.get_repository(namespace_name, repo_name) - if not repo: - return None - - all_images = model.image.get_repository_images_without_placements(repo) - all_images_map = {image.id: image for image in all_images} - - all_tags = list(model.tag.list_repository_tags(namespace_name, repo_name)) - - tags_by_docker_id = defaultdict(list) - for tag in all_tags: - tags_by_docker_id[tag.image.docker_image_id].append(tag) - - def _build_image(image): - image_itself = _image(namespace_name, repo_name, image, all_images_map) - return ImageWithTags(image_itself, tag_names=_tag_names(image, tags_by_docker_id)) - - return [_build_image(image) for image in all_images] - - def get_repository_image(self, namespace_name, repo_name, docker_image_id): - image = model.image.get_repo_image_and_storage(namespace_name, repo_name, docker_image_id) - if not image: - return None - - parent_images = model.image.get_parent_images(namespace_name, repo_name, image) - all_images_map = {image.id: image for image in parent_images} - return ImageWithHistory(_image(namespace_name, repo_name, image, all_images_map)) - - -pre_oci_model = PreOCIModel() From 584a18c0e11501a5c7200c3189f29d98283cdc04 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 20 Aug 2018 15:49:01 -0400 Subject: [PATCH 5/7] Change datatype base to raise AttributeError on missing keys --- data/registry_model/datatype.py | 2 +- data/registry_model/test/test_pre_oci_model.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/data/registry_model/datatype.py b/data/registry_model/datatype.py index 6571e61b4..0b72b5fa8 100644 --- a/data/registry_model/datatype.py +++ b/data/registry_model/datatype.py @@ -28,7 +28,7 @@ def datatype(name, static_fields): if name in static_fields: return self._fields[name] - return None + raise AttributeError('Unknown field `%s`' % name) return DataType diff --git a/data/registry_model/test/test_pre_oci_model.py b/data/registry_model/test/test_pre_oci_model.py index 78b5ca545..d3047cff1 100644 --- a/data/registry_model/test/test_pre_oci_model.py +++ b/data/registry_model/test/test_pre_oci_model.py @@ -64,7 +64,7 @@ def test_lookup_manifests(repo_namespace, repo_name, pre_oci_model): 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) - assert found.id == found_manifest.id + assert found._db_id == found_manifest._db_id assert found.digest == found_manifest.digest From 23ff49f0c1c9753a6a888a11f381a74edcff0cbf Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 20 Aug 2018 16:47:38 -0400 Subject: [PATCH 6/7] Adjust usage of image model in manifest model --- endpoints/api/image.py | 8 ++++---- endpoints/api/manifest_models_interface.py | 3 ++- endpoints/api/manifest_models_pre_oci.py | 12 +++++++++--- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/endpoints/api/image.py b/endpoints/api/image.py index f126ed89e..53c4bf68b 100644 --- a/endpoints/api/image.py +++ b/endpoints/api/image.py @@ -7,7 +7,7 @@ from endpoints.api import (resource, nickname, require_repo_read, RepositoryPara from endpoints.exception import NotFound -def _image_dict(image, with_history=False, with_tags=False): +def image_dict(image, with_history=False, with_tags=False): image_data = { 'id': image.docker_image_id, 'created': format_date(image.created), @@ -22,7 +22,7 @@ def _image_dict(image, with_history=False, with_tags=False): image_data['tags'] = [tag.name for tag in image.tags] if with_history: - image_data['history'] = [_image_dict(parent, with_history, with_tags) + image_data['history'] = [image_dict(parent, with_history, with_tags) for parent in image.parents] # Calculate the ancestors string, with the DBID's replaced with the docker IDs. @@ -46,7 +46,7 @@ class RepositoryImageList(RepositoryParamResource): raise NotFound() images = registry_model.get_legacy_images(repo_ref) - return {'images': [_image_dict(image, with_tags=True) for image in images]} + return {'images': [image_dict(image, with_tags=True) for image in images]} @resource('/v1/repository//image/') @@ -68,4 +68,4 @@ class RepositoryImage(RepositoryParamResource): if image is None: raise NotFound() - return _image_dict(image, with_history=True) + return image_dict(image, with_history=True) diff --git a/endpoints/api/manifest_models_interface.py b/endpoints/api/manifest_models_interface.py index 03615b208..539399b2b 100644 --- a/endpoints/api/manifest_models_interface.py +++ b/endpoints/api/manifest_models_interface.py @@ -1,6 +1,7 @@ from abc import ABCMeta, abstractmethod from collections import namedtuple +from endpoints.api.image import image_dict from six import add_metaclass @@ -40,7 +41,7 @@ class ManifestAndImage( return { 'digest': self.digest, 'manifest_data': self.manifest_data, - 'image': self.image.to_dict(), + 'image': image_dict(self.image), } diff --git a/endpoints/api/manifest_models_pre_oci.py b/endpoints/api/manifest_models_pre_oci.py index a5d3d78af..6a40eb76a 100644 --- a/endpoints/api/manifest_models_pre_oci.py +++ b/endpoints/api/manifest_models_pre_oci.py @@ -2,7 +2,7 @@ import json from manifest_models_interface import ManifestLabel, ManifestLabelInterface, ManifestAndImage from data import model -from image_models_pre_oci import pre_oci_model as image_models +from data.registry_model import registry_model class ManifestLabelPreOCI(ManifestLabelInterface): @@ -47,8 +47,14 @@ class ManifestLabelPreOCI(ManifestLabelInterface): return None # TODO: remove this dependency on image once we've moved to the new data model. - image = image_models.get_repository_image(namespace_name, repository_name, - tag_manifest.tag.image.docker_image_id) + 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) From 5a997eb4e3b4ffb086845eb84e8030ce14c931da Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 21 Aug 2018 14:27:10 -0400 Subject: [PATCH 7/7] Change label creation function to return the label created --- data/registry_model/datatypes.py | 10 ++++++++++ data/registry_model/registry_pre_oci_model.py | 8 +++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/data/registry_model/datatypes.py b/data/registry_model/datatypes.py index 7487d1564..66c39c2f4 100644 --- a/data/registry_model/datatypes.py +++ b/data/registry_model/datatypes.py @@ -10,6 +10,16 @@ class RepositoryReference(datatype('Repository', [])): return RepositoryReference(db_id=repo_obj.id) +class Label(datatype('Label', ['key', 'value'])): + """ 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) + + class Tag(datatype('Tag', ['name'])): """ Tag represents a tag in a repository, which points to a manifest or image. """ @classmethod diff --git a/data/registry_model/registry_pre_oci_model.py b/data/registry_model/registry_pre_oci_model.py index 6e37814de..b879f96f0 100644 --- a/data/registry_model/registry_pre_oci_model.py +++ b/data/registry_model/registry_pre_oci_model.py @@ -5,7 +5,7 @@ from collections import defaultdict from data import database from data import model from data.registry_model.interface import RegistryDataInterface -from data.registry_model.datatypes import Tag, RepositoryReference, Manifest, LegacyImage +from data.registry_model.datatypes import Tag, RepositoryReference, Manifest, LegacyImage, Label class PreOCIModel(RegistryDataInterface): @@ -60,9 +60,11 @@ class PreOCIModel(RegistryDataInterface): try: tag_manifest = database.TagManifest.get(id=manifest._db_id) except database.TagManifest.DoesNotExist: - return + return None - model.label.create_manifest_label(tag_manifest, key, value, source_type_name, media_type_name) + label = model.label.create_manifest_label(tag_manifest, key, value, source_type_name, + media_type_name) + return Label.for_label(label) def get_legacy_images(self, repository_ref): """