From 96072a44c055c875743944b20cbcae58b6fa02fe Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 7 Mar 2019 12:53:02 -0500 Subject: [PATCH] Fix ordering of tags in the OCI model Before this change, we were neglecting to sort the tags by ID, which meant that pagination was broken --- data/model/oci/tag.py | 6 +++- data/registry_model/interface.py | 3 +- data/registry_model/registry_oci_model.py | 9 +++-- data/registry_model/registry_pre_oci_model.py | 3 +- data/registry_model/test/test_interface.py | 34 +++++++++++++++++++ endpoints/v2/tag.py | 2 +- 6 files changed, 51 insertions(+), 6 deletions(-) diff --git a/data/model/oci/tag.py b/data/model/oci/tag.py index 178747081..de2ced020 100644 --- a/data/model/oci/tag.py +++ b/data/model/oci/tag.py @@ -44,7 +44,7 @@ def get_tag(repository_id, tag_name): return None -def list_alive_tags(repository_id, start_pagination_id=None, limit=None): +def list_alive_tags(repository_id, start_pagination_id=None, limit=None, sort_tags=False): """ Returns a list of all the tags alive in the specified repository, with optional limits. Tag's returned are joined with their manifest. """ @@ -54,11 +54,15 @@ def list_alive_tags(repository_id, start_pagination_id=None, limit=None): .where(Tag.repository == repository_id)) if start_pagination_id is not None: + assert sort_tags query = query.where(Tag.id >= start_pagination_id) if limit is not None: query = query.limit(limit) + if sort_tags: + query = query.order_by(Tag.id) + return filter_to_visible_tags(filter_to_alive_tags(query)) diff --git a/data/registry_model/interface.py b/data/registry_model/interface.py index 9d990c92b..d346201a8 100644 --- a/data/registry_model/interface.py +++ b/data/registry_model/interface.py @@ -114,7 +114,8 @@ class RegistryDataInterface(object): @abstractmethod def list_repository_tags(self, repository_ref, include_legacy_images=False, start_pagination_id=None, - limit=None): + limit=None, + sort_tags=False): """ Returns a list of all the active tags in the repository. Note that this can be a *heavy* operation on repositories with a lot of tags, and should be avoided for more targetted diff --git a/data/registry_model/registry_oci_model.py b/data/registry_model/registry_oci_model.py index 0d27a3dbb..fac95426b 100644 --- a/data/registry_model/registry_oci_model.py +++ b/data/registry_model/registry_oci_model.py @@ -201,13 +201,18 @@ class OCIModel(SharedModel, RegistryDataInterface): def list_repository_tags(self, repository_ref, include_legacy_images=False, start_pagination_id=None, - limit=None): + limit=None, + sort_tags=False): """ Returns a list of all the active tags in the repository. Note that this can be a *heavy* operation on repositories with a lot of tags, and should be avoided for more targetted operations wherever possible. """ - tags = list(oci.tag.list_alive_tags(repository_ref._db_id, start_pagination_id, limit)) + if start_pagination_id: + assert sort_tags + + tags = list(oci.tag.list_alive_tags(repository_ref._db_id, start_pagination_id, limit, + sort_tags=sort_tags)) legacy_images_map = {} if include_legacy_images: legacy_images_map = oci.tag.get_legacy_images_for_tags(tags) diff --git a/data/registry_model/registry_pre_oci_model.py b/data/registry_model/registry_pre_oci_model.py index db4b53ecb..cb1718790 100644 --- a/data/registry_model/registry_pre_oci_model.py +++ b/data/registry_model/registry_pre_oci_model.py @@ -265,7 +265,8 @@ class PreOCIModel(SharedModel, RegistryDataInterface): def list_repository_tags(self, repository_ref, include_legacy_images=False, start_pagination_id=None, - limit=None): + limit=None, + sort_tags=False): """ Returns a list of all the active tags in the repository. Note that this can be a *heavy* operation on repositories with a lot of tags, and should be avoided for more targetted diff --git a/data/registry_model/test/test_interface.py b/data/registry_model/test/test_interface.py index 747a21eb1..1c7e62e0b 100644 --- a/data/registry_model/test/test_interface.py +++ b/data/registry_model/test/test_interface.py @@ -936,3 +936,37 @@ def test_unicode_emoji(registry_model): assert found.digest == manifest.digest assert found.internal_manifest_bytes.as_encoded_str() == manifest.bytes.as_encoded_str() assert found.get_parsed_manifest().digest == manifest.digest + + +def test_list_repository_tags(oci_model): + repository_ref = oci_model.lookup_repository('devtable', 'simple') + latest_tag = oci_model.get_repo_tag(repository_ref, 'latest') + manifest = oci_model.get_manifest_for_tag(latest_tag) + + tag_count = 500 + + # Create a bunch of tags. + tags_expected = set() + for index in range(0, tag_count): + tags_expected.add('somenewtag%s' % index) + oci_model.retarget_tag(repository_ref, 'somenewtag%s' % index, manifest, storage) + + # List the tags. + tags_found = set() + tag_id = None + while True: + tags = oci_model.list_repository_tags(repository_ref, start_pagination_id=tag_id, + limit=11, sort_tags=True) + assert len(tags) <= 11 + for tag in tags[0:10]: + assert tag.name not in tags_found + if tag.name in tags_expected: + tags_expected.remove(tag.name) + + if len(tags) < 11: + break + + tag_id = tags[10].id + + # Make sure we've found all the tags. + assert not tags_expected diff --git a/endpoints/v2/tag.py b/endpoints/v2/tag.py index fec91f11e..2d75d7350 100644 --- a/endpoints/v2/tag.py +++ b/endpoints/v2/tag.py @@ -21,7 +21,7 @@ def list_all_tags(namespace_name, repo_name, start_id, limit, pagination_callbac # NOTE: We add 1 to the limit because that's how pagination_callback knows if there are # additional tags. tags = registry_model.list_repository_tags(repository_ref, start_pagination_id=start_id, - limit=limit + 1) + limit=limit + 1, sort_tags=True) response = jsonify({ 'name': '{0}/{1}'.format(namespace_name, repo_name), 'tags': [tag.name for tag in tags][0:limit],