From 941cb4b4eeb40fe5c10ce1e87e325b8681798cb7 Mon Sep 17 00:00:00 2001 From: Charlton Austin Date: Tue, 27 Jun 2017 10:01:20 -0400 Subject: [PATCH 1/3] refactor(endpoints/api/tag*): adding in new support for tags api this creates an interface for hidding details of the data model for pre oci and post oci code Issue: https://coreosdev.atlassian.net/browse/QUAY-632 - [ ] It works! - [ ] Comments provide sufficient explanations for the next contributor - [ ] Tests cover changes and corner cases - [ ] Follows Quay syntax patterns and format --- endpoints/api/tag.py | 61 ++++++------ endpoints/api/tag_interface/__init__.py | 0 .../api/tag_interface/models_interface.py | 39 ++++++++ endpoints/api/tag_interface/models_pre_oci.py | 27 ++++++ endpoints/api/tag_interface/test/__init__.py | 0 .../tag_interface/test/test_models_pre_oci.py | 95 +++++++++++++++++++ 6 files changed, 192 insertions(+), 30 deletions(-) create mode 100644 endpoints/api/tag_interface/__init__.py create mode 100644 endpoints/api/tag_interface/models_interface.py create mode 100644 endpoints/api/tag_interface/models_pre_oci.py create mode 100644 endpoints/api/tag_interface/test/__init__.py create mode 100644 endpoints/api/tag_interface/test/test_models_pre_oci.py diff --git a/endpoints/api/tag.py b/endpoints/api/tag.py index ed170a7ff..cc6ae3f39 100644 --- a/endpoints/api/tag.py +++ b/endpoints/api/tag.py @@ -2,18 +2,38 @@ from flask import request, abort +from auth.auth_context import get_authenticated_user +from data import model from endpoints.api import ( resource, nickname, require_repo_read, require_repo_write, RepositoryParamResource, log_action, validate_json_request, path_param, parse_args, query_param, truthy_bool, disallow_for_app_repositories) -from endpoints.exception import NotFound from endpoints.api.image import image_view +from endpoints.api.tag_interface.models_pre_oci import pre_oci_model +from endpoints.exception import NotFound from endpoints.v2.manifest import _generate_and_store_manifest -from data import model -from auth.auth_context import get_authenticated_user from util.names import TAG_ERROR, TAG_REGEX +def tag_view(tag): + tag_info = { + 'name': tag.name, + 'docker_image_id': tag.docker_image_id, + 'reversion': tag.reversion, + } + + if tag.lifetime_start_ts > 0: + tag_info['start_ts'] = tag.lifetime_start_ts + + if tag.lifetime_end_ts > 0: + tag_info['end_ts'] = tag.lifetime_end_ts + + if tag.manifest_list: + tag_info['manifest_digest'] = tag.manifest_list + + return tag_info + + @resource('/v1/repository//tag/') @path_param('repository', 'The full path of the repository. e.g. namespace/name') class ListRepositoryTags(RepositoryParamResource): @@ -28,39 +48,20 @@ class ListRepositoryTags(RepositoryParamResource): @query_param('page', 'Page index for the results. Default 1.', type=int, default=1) @nickname('listRepoTags') def get(self, namespace, repository, parsed_args): - repo = model.repository.get_repository(namespace, repository) - if not repo: - raise NotFound() - - def tag_view(tag): - tag_info = { - 'name': tag.name, - 'docker_image_id': tag.image.docker_image_id, - 'reversion': tag.reversion, - } - - if tag.lifetime_start_ts > 0: - tag_info['start_ts'] = tag.lifetime_start_ts - - if tag.lifetime_end_ts > 0: - tag_info['end_ts'] = tag.lifetime_end_ts - - if tag.id in manifest_map: - tag_info['manifest_digest'] = manifest_map[tag.id] - - return tag_info - specific_tag = parsed_args.get('specificTag') or None - page = max(1, parsed_args.get('page', 1)) limit = min(100, max(1, parsed_args.get('limit', 50))) - tags, manifest_map, more = model.tag.list_repository_tag_history(repo, page=page, size=limit, - specific_tag=specific_tag) + + tag_history = pre_oci_model.list_repository_tag_history(namespace_name=namespace, repository_name=repository, + page=page, size=limit, specific_tag=specific_tag) + + if not tag_history: + raise NotFound() return { - 'tags': [tag_view(tag) for tag in tags], + 'tags': [tag_view(tag) for tag in tag_history.tags], 'page': page, - 'has_additional': more, + 'has_additional': tag_history.more, } diff --git a/endpoints/api/tag_interface/__init__.py b/endpoints/api/tag_interface/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/endpoints/api/tag_interface/models_interface.py b/endpoints/api/tag_interface/models_interface.py new file mode 100644 index 000000000..ec2ec9093 --- /dev/null +++ b/endpoints/api/tag_interface/models_interface.py @@ -0,0 +1,39 @@ +from abc import ABCMeta, abstractmethod +from collections import namedtuple + +from six import add_metaclass + + +class Tag(namedtuple('Tag', ['name', 'image', 'reversion', 'lifetime_start_ts', 'lifetime_end_ts', + 'manifest_list', 'docker_image_id'])): + """ + Tag represents a name to an image. + :type name: string + :type image: Image + :type reversion: boolean + :type lifetime_start_ts: int + :type lifetime_end_ts: int + :type manifest_list: [manifest_digest] + :type docker_image_id: string + """ + + +class RepositoryTagHistory(namedtuple('RepositoryTagHistory', ['tags', 'more'])): + """ + Tag represents a name to an image. + :type tags: [Tag] + :type more: boolean + """ + + +@add_metaclass(ABCMeta) +class TagDataInterface(object): + """ + Interface that represents all data store interactions required by a Tag. + """ + + @abstractmethod + def list_repository_tag_history(self, namespace_name, repository_name, page=1, size=100, specific_tag=None): + """ + Returns a RepositoryTagHistory with a list of historic tags and whether there are more tags then returned. + """ diff --git a/endpoints/api/tag_interface/models_pre_oci.py b/endpoints/api/tag_interface/models_pre_oci.py new file mode 100644 index 000000000..fe1025195 --- /dev/null +++ b/endpoints/api/tag_interface/models_pre_oci.py @@ -0,0 +1,27 @@ +from data import model +from endpoints.api.tag_interface.models_interface import TagDataInterface, Tag, RepositoryTagHistory + + +class PreOCIModel(TagDataInterface): + """ + PreOCIModel implements the data model for the Tags using a database schema + before it was changed to support the OCI specification. + """ + + def list_repository_tag_history(self, namespace_name, repository_name, page=1, size=100, specific_tag=None): + repository = model.repository.get_repository(namespace_name, repository_name) + if repository is None: + return None + tags, manifest_map, more = model.tag.list_repository_tag_history(repository, page, size, specific_tag) + repository_tag_history = [] + for tag in tags: + manifest_list = None + if tag.id in manifest_map: + manifest_list = manifest_map[tag.id] + repository_tag_history.append( + Tag(name=tag.name, image=tag.image, reversion=tag.reversion, lifetime_start_ts=tag.lifetime_start_ts, + lifetime_end_ts=tag.lifetime_end_ts, manifest_list=manifest_list, + docker_image_id=tag.image.docker_image_id)) + return RepositoryTagHistory(tags=repository_tag_history, more=more) + +pre_oci_model = PreOCIModel() diff --git a/endpoints/api/tag_interface/test/__init__.py b/endpoints/api/tag_interface/test/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/endpoints/api/tag_interface/test/test_models_pre_oci.py b/endpoints/api/tag_interface/test/test_models_pre_oci.py new file mode 100644 index 000000000..ba2e462af --- /dev/null +++ b/endpoints/api/tag_interface/test/test_models_pre_oci.py @@ -0,0 +1,95 @@ +import pytest +from data.model.tag_interface.models_interface import RepositoryTagHistory, Tag +from mock import Mock + +from data import model +from endpoints.api.tag_interface.models_pre_oci import pre_oci_model + +EMPTY_REPOSITORY = 'empty_repository' + +EMPTY_NAMESPACE = 'empty_namespace' + +BAD_REPOSITORY_NAME = 'bad_repository_name' + +BAD_NAMESPACE_NAME = 'bad_namespace_name' + + +@pytest.fixture +def get_monkeypatch(monkeypatch): + return monkeypatch + + +def mock_out_get_repository(monkeypatch, namespace_name, repository_name): + def return_none(namespace_name, repository_name): + return None + + def return_repository(namespace_name, repository_name): + return 'repository' + + if namespace_name == BAD_NAMESPACE_NAME or repository_name == BAD_REPOSITORY_NAME: + return_function = return_none + else: + return_function = return_repository + + monkeypatch.setattr(model.repository, 'get_repository', return_function) + + +def create_mock_tag(name, reversion, lifetime_start_ts, lifetime_end_ts, mock_id, docker_image_id, manifest_list): + tag_mock = Mock() + tag_mock.name = name + image_mock = Mock() + image_mock.docker_image_id = docker_image_id + tag_mock.image = image_mock + tag_mock.reversion = reversion + tag_mock.lifetime_start_ts = lifetime_start_ts + tag_mock.lifetime_end_ts = lifetime_end_ts + tag_mock.id = mock_id + tag_mock.manifest_list = manifest_list + tag = Tag(name=name, reversion=reversion, image=image_mock, docker_image_id=docker_image_id, + lifetime_start_ts=lifetime_start_ts, lifetime_end_ts=lifetime_end_ts, manifest_list=manifest_list) + return tag_mock, tag + + +first_mock, first_tag = create_mock_tag('tag1', 'rev1', 'start1', 'end1', 'id1', 'docker_image_id1', []) +second_mock, second_tag = create_mock_tag('tag2', 'rev2', 'start2', 'end2', 'id2', 'docker_image_id2', ['manifest']) + + +def mock_out_list_repository_tag_history(monkeypatch, namespace_name, repository_name, page, size, specific_tag): + def list_empty_tag_history(repository, page, size, specific_tag): + return [], {}, False + + def list_filled_tag_history(repository, page, size, specific_tag): + tags = [first_mock, second_mock] + return tags, {first_mock.id: first_mock.manifest_list, + second_mock.id: second_mock.manifest_list}, len(tags) > size + + def list_only_second_tag(repository, page, size, specific_tag): + tags = [second_mock] + return tags, {second_mock.id: second_mock.manifest_list}, len(tags) > size + + if namespace_name == EMPTY_NAMESPACE or repository_name == EMPTY_REPOSITORY: + return_function = list_empty_tag_history + else: + if specific_tag == 'tag2': + return_function = list_only_second_tag + else: + return_function = list_filled_tag_history + + monkeypatch.setattr(model.tag, 'list_repository_tag_history', return_function) + + +@pytest.mark.parametrize( + 'expected, namespace_name, repository_name, page, size, specific_tag', + [(None, BAD_NAMESPACE_NAME, 'repository_name', 1, 100, None), + (None, 'namespace_name', BAD_REPOSITORY_NAME, 1, 100, None), + (RepositoryTagHistory(tags=[], more=False), EMPTY_NAMESPACE, EMPTY_REPOSITORY, 1, 100, None), + (RepositoryTagHistory(tags=[first_tag, second_tag], more=False), 'namespace', 'repository', 1, 100, None), + (RepositoryTagHistory(tags=[first_tag, second_tag], more=True), 'namespace', 'repository', 1, 1, None), + (RepositoryTagHistory(tags=[second_tag], more=False), 'namespace', 'repository', 1, 100, 'tag2'), + ]) +def test_list_repository_tag_history(expected, namespace_name, repository_name, + page, size, specific_tag, get_monkeypatch): + mock_out_get_repository(get_monkeypatch, namespace_name, repository_name) + mock_out_list_repository_tag_history(get_monkeypatch, namespace_name, repository_name, page, size, specific_tag) + assert pre_oci_model.list_repository_tag_history(namespace_name, repository_name, page, size, + specific_tag) == expected From 337b68abdc71a4366efe507bbdb0f34451e6c2f3 Mon Sep 17 00:00:00 2001 From: Charlton Austin Date: Tue, 27 Jun 2017 14:24:23 -0400 Subject: [PATCH 2/3] style(endpoints/api/tag*): ran yapf ### Description of Changes Issue: https://coreosdev.atlassian.net/browse/QUAY-632 ## Reviewer Checklist - [ ] It works! - [ ] Comments provide sufficient explanations for the next contributor - [ ] Tests cover changes and corner cases - [ ] Follows Quay syntax patterns and format --- endpoints/api/tag.py | 24 ++++----- .../api/tag_interface/models_interface.py | 10 ++-- endpoints/api/tag_interface/models_pre_oci.py | 13 +++-- .../tag_interface/test/test_models_pre_oci.py | 51 +++++++++++-------- 4 files changed, 55 insertions(+), 43 deletions(-) diff --git a/endpoints/api/tag.py b/endpoints/api/tag.py index cc6ae3f39..7563e6c15 100644 --- a/endpoints/api/tag.py +++ b/endpoints/api/tag.py @@ -4,10 +4,9 @@ from flask import request, abort from auth.auth_context import get_authenticated_user from data import model -from endpoints.api import ( - resource, nickname, require_repo_read, require_repo_write, RepositoryParamResource, log_action, - validate_json_request, path_param, parse_args, query_param, truthy_bool, - disallow_for_app_repositories) +from endpoints.api import (resource, nickname, require_repo_read, require_repo_write, + RepositoryParamResource, log_action, validate_json_request, path_param, + parse_args, query_param, truthy_bool, disallow_for_app_repositories) from endpoints.api.image import image_view from endpoints.api.tag_interface.models_pre_oci import pre_oci_model from endpoints.exception import NotFound @@ -52,8 +51,9 @@ class ListRepositoryTags(RepositoryParamResource): page = max(1, parsed_args.get('page', 1)) limit = min(100, max(1, parsed_args.get('limit', 50))) - tag_history = pre_oci_model.list_repository_tag_history(namespace_name=namespace, repository_name=repository, - page=page, size=limit, specific_tag=specific_tag) + tag_history = pre_oci_model.list_repository_tag_history(namespace_name=namespace, + repository_name=repository, page=page, + size=limit, specific_tag=specific_tag) if not tag_history: raise NotFound() @@ -74,9 +74,7 @@ class RepositoryTag(RepositoryParamResource): 'MoveTag': { 'type': 'object', 'description': 'Description of to which image a new or existing tag should point', - 'required': [ - 'image', - ], + 'required': ['image',], 'properties': { 'image': { 'type': 'string', @@ -206,9 +204,7 @@ class RestoreTag(RepositoryParamResource): 'RestoreTag': { 'type': 'object', 'description': 'Restores a tag to a specific image', - 'required': [ - 'image', - ], + 'required': ['image',], 'properties': { 'image': { 'type': 'string', @@ -252,8 +248,8 @@ class RestoreTag(RepositoryParamResource): if existing_image is not None: log_data['original_image'] = existing_image.docker_image_id - log_action('revert_tag', namespace, log_data, repo=model.repository.get_repository( - namespace, repository)) + log_action('revert_tag', namespace, log_data, repo=model.repository.get_repository(namespace, + repository)) return { 'image_id': image_id, diff --git a/endpoints/api/tag_interface/models_interface.py b/endpoints/api/tag_interface/models_interface.py index ec2ec9093..c98737f9a 100644 --- a/endpoints/api/tag_interface/models_interface.py +++ b/endpoints/api/tag_interface/models_interface.py @@ -4,8 +4,11 @@ from collections import namedtuple from six import add_metaclass -class Tag(namedtuple('Tag', ['name', 'image', 'reversion', 'lifetime_start_ts', 'lifetime_end_ts', - 'manifest_list', 'docker_image_id'])): +class Tag( + namedtuple('Tag', [ + 'name', 'image', 'reversion', 'lifetime_start_ts', 'lifetime_end_ts', 'manifest_list', + 'docker_image_id' + ])): """ Tag represents a name to an image. :type name: string @@ -33,7 +36,8 @@ class TagDataInterface(object): """ @abstractmethod - def list_repository_tag_history(self, namespace_name, repository_name, page=1, size=100, specific_tag=None): + def list_repository_tag_history(self, namespace_name, repository_name, page=1, size=100, + specific_tag=None): """ Returns a RepositoryTagHistory with a list of historic tags and whether there are more tags then returned. """ diff --git a/endpoints/api/tag_interface/models_pre_oci.py b/endpoints/api/tag_interface/models_pre_oci.py index fe1025195..995b69de0 100644 --- a/endpoints/api/tag_interface/models_pre_oci.py +++ b/endpoints/api/tag_interface/models_pre_oci.py @@ -8,20 +8,23 @@ class PreOCIModel(TagDataInterface): before it was changed to support the OCI specification. """ - def list_repository_tag_history(self, namespace_name, repository_name, page=1, size=100, specific_tag=None): + def list_repository_tag_history(self, namespace_name, repository_name, page=1, size=100, + specific_tag=None): repository = model.repository.get_repository(namespace_name, repository_name) if repository is None: return None - tags, manifest_map, more = model.tag.list_repository_tag_history(repository, page, size, specific_tag) + tags, manifest_map, more = model.tag.list_repository_tag_history(repository, page, size, + specific_tag) repository_tag_history = [] for tag in tags: manifest_list = None if tag.id in manifest_map: manifest_list = manifest_map[tag.id] repository_tag_history.append( - Tag(name=tag.name, image=tag.image, reversion=tag.reversion, lifetime_start_ts=tag.lifetime_start_ts, - lifetime_end_ts=tag.lifetime_end_ts, manifest_list=manifest_list, - docker_image_id=tag.image.docker_image_id)) + Tag(name=tag.name, image=tag.image, reversion=tag.reversion, + lifetime_start_ts=tag.lifetime_start_ts, lifetime_end_ts=tag.lifetime_end_ts, + manifest_list=manifest_list, docker_image_id=tag.image.docker_image_id)) return RepositoryTagHistory(tags=repository_tag_history, more=more) + pre_oci_model = PreOCIModel() diff --git a/endpoints/api/tag_interface/test/test_models_pre_oci.py b/endpoints/api/tag_interface/test/test_models_pre_oci.py index ba2e462af..1df367dd1 100644 --- a/endpoints/api/tag_interface/test/test_models_pre_oci.py +++ b/endpoints/api/tag_interface/test/test_models_pre_oci.py @@ -5,12 +5,10 @@ from mock import Mock from data import model from endpoints.api.tag_interface.models_pre_oci import pre_oci_model + EMPTY_REPOSITORY = 'empty_repository' - EMPTY_NAMESPACE = 'empty_namespace' - BAD_REPOSITORY_NAME = 'bad_repository_name' - BAD_NAMESPACE_NAME = 'bad_namespace_name' @@ -34,7 +32,8 @@ def mock_out_get_repository(monkeypatch, namespace_name, repository_name): monkeypatch.setattr(model.repository, 'get_repository', return_function) -def create_mock_tag(name, reversion, lifetime_start_ts, lifetime_end_ts, mock_id, docker_image_id, manifest_list): +def create_mock_tag(name, reversion, lifetime_start_ts, lifetime_end_ts, mock_id, docker_image_id, + manifest_list): tag_mock = Mock() tag_mock.name = name image_mock = Mock() @@ -46,22 +45,28 @@ def create_mock_tag(name, reversion, lifetime_start_ts, lifetime_end_ts, mock_id tag_mock.id = mock_id tag_mock.manifest_list = manifest_list tag = Tag(name=name, reversion=reversion, image=image_mock, docker_image_id=docker_image_id, - lifetime_start_ts=lifetime_start_ts, lifetime_end_ts=lifetime_end_ts, manifest_list=manifest_list) + lifetime_start_ts=lifetime_start_ts, lifetime_end_ts=lifetime_end_ts, + manifest_list=manifest_list) return tag_mock, tag -first_mock, first_tag = create_mock_tag('tag1', 'rev1', 'start1', 'end1', 'id1', 'docker_image_id1', []) -second_mock, second_tag = create_mock_tag('tag2', 'rev2', 'start2', 'end2', 'id2', 'docker_image_id2', ['manifest']) +first_mock, first_tag = create_mock_tag('tag1', 'rev1', 'start1', 'end1', 'id1', + 'docker_image_id1', []) +second_mock, second_tag = create_mock_tag('tag2', 'rev2', 'start2', 'end2', 'id2', + 'docker_image_id2', ['manifest']) -def mock_out_list_repository_tag_history(monkeypatch, namespace_name, repository_name, page, size, specific_tag): +def mock_out_list_repository_tag_history(monkeypatch, namespace_name, repository_name, page, size, + specific_tag): def list_empty_tag_history(repository, page, size, specific_tag): return [], {}, False def list_filled_tag_history(repository, page, size, specific_tag): tags = [first_mock, second_mock] - return tags, {first_mock.id: first_mock.manifest_list, - second_mock.id: second_mock.manifest_list}, len(tags) > size + return tags, { + first_mock.id: first_mock.manifest_list, + second_mock.id: second_mock.manifest_list + }, len(tags) > size def list_only_second_tag(repository, page, size, specific_tag): tags = [second_mock] @@ -79,17 +84,21 @@ def mock_out_list_repository_tag_history(monkeypatch, namespace_name, repository @pytest.mark.parametrize( - 'expected, namespace_name, repository_name, page, size, specific_tag', - [(None, BAD_NAMESPACE_NAME, 'repository_name', 1, 100, None), - (None, 'namespace_name', BAD_REPOSITORY_NAME, 1, 100, None), - (RepositoryTagHistory(tags=[], more=False), EMPTY_NAMESPACE, EMPTY_REPOSITORY, 1, 100, None), - (RepositoryTagHistory(tags=[first_tag, second_tag], more=False), 'namespace', 'repository', 1, 100, None), - (RepositoryTagHistory(tags=[first_tag, second_tag], more=True), 'namespace', 'repository', 1, 1, None), - (RepositoryTagHistory(tags=[second_tag], more=False), 'namespace', 'repository', 1, 100, 'tag2'), - ]) -def test_list_repository_tag_history(expected, namespace_name, repository_name, - page, size, specific_tag, get_monkeypatch): + 'expected, namespace_name, repository_name, page, size, specific_tag', [ + (None, BAD_NAMESPACE_NAME, 'repository_name', 1, 100, None), + (None, 'namespace_name', BAD_REPOSITORY_NAME, 1, 100, None), + (RepositoryTagHistory(tags=[], more=False), EMPTY_NAMESPACE, EMPTY_REPOSITORY, 1, 100, None), + (RepositoryTagHistory(tags=[first_tag, second_tag], more=False), 'namespace', 'repository', 1, + 100, None), + (RepositoryTagHistory(tags=[first_tag, second_tag], more=True), 'namespace', 'repository', 1, + 1, None), + (RepositoryTagHistory(tags=[second_tag], more=False), 'namespace', 'repository', 1, 100, + 'tag2'), + ]) +def test_list_repository_tag_history(expected, namespace_name, repository_name, page, size, + specific_tag, get_monkeypatch): mock_out_get_repository(get_monkeypatch, namespace_name, repository_name) - mock_out_list_repository_tag_history(get_monkeypatch, namespace_name, repository_name, page, size, specific_tag) + mock_out_list_repository_tag_history(get_monkeypatch, namespace_name, repository_name, page, + size, specific_tag) assert pre_oci_model.list_repository_tag_history(namespace_name, repository_name, page, size, specific_tag) == expected From 400b346953879e97d2eb1d35837aa874e6763171 Mon Sep 17 00:00:00 2001 From: Charlton Austin Date: Tue, 27 Jun 2017 17:07:20 -0400 Subject: [PATCH 3/3] test(endpoints/api): add in test for tag this adds tests for the pro oci model Issue: https://coreosdev.atlassian.net/browse/QUAY-632 - [ ] It works! - [ ] Comments provide sufficient explanations for the next contributor - [ ] Tests cover changes and corner cases - [ ] Follows Quay syntax patterns and format --- endpoints/api/tag.py | 2 +- endpoints/api/tag_interface/__init__.py | 0 endpoints/api/tag_interface/test/__init__.py | 0 ...s_interface.py => tag_models_interface.py} | 0 ...odels_pre_oci.py => tag_models_pre_oci.py} | 2 +- .../test/test_models_pre_oci.py | 5 +- endpoints/api/test/test_tag.py | 91 ++++++++++++++++++- 7 files changed, 90 insertions(+), 10 deletions(-) delete mode 100644 endpoints/api/tag_interface/__init__.py delete mode 100644 endpoints/api/tag_interface/test/__init__.py rename endpoints/api/{tag_interface/models_interface.py => tag_models_interface.py} (100%) rename endpoints/api/{tag_interface/models_pre_oci.py => tag_models_pre_oci.py} (92%) rename endpoints/api/{tag_interface => }/test/test_models_pre_oci.py (96%) diff --git a/endpoints/api/tag.py b/endpoints/api/tag.py index 7563e6c15..1422b2971 100644 --- a/endpoints/api/tag.py +++ b/endpoints/api/tag.py @@ -8,7 +8,7 @@ from endpoints.api import (resource, nickname, require_repo_read, require_repo_w RepositoryParamResource, log_action, validate_json_request, path_param, parse_args, query_param, truthy_bool, disallow_for_app_repositories) from endpoints.api.image import image_view -from endpoints.api.tag_interface.models_pre_oci import pre_oci_model +from endpoints.api.tag_models_pre_oci import pre_oci_model from endpoints.exception import NotFound from endpoints.v2.manifest import _generate_and_store_manifest from util.names import TAG_ERROR, TAG_REGEX diff --git a/endpoints/api/tag_interface/__init__.py b/endpoints/api/tag_interface/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/endpoints/api/tag_interface/test/__init__.py b/endpoints/api/tag_interface/test/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/endpoints/api/tag_interface/models_interface.py b/endpoints/api/tag_models_interface.py similarity index 100% rename from endpoints/api/tag_interface/models_interface.py rename to endpoints/api/tag_models_interface.py diff --git a/endpoints/api/tag_interface/models_pre_oci.py b/endpoints/api/tag_models_pre_oci.py similarity index 92% rename from endpoints/api/tag_interface/models_pre_oci.py rename to endpoints/api/tag_models_pre_oci.py index 995b69de0..a1842b036 100644 --- a/endpoints/api/tag_interface/models_pre_oci.py +++ b/endpoints/api/tag_models_pre_oci.py @@ -1,5 +1,5 @@ from data import model -from endpoints.api.tag_interface.models_interface import TagDataInterface, Tag, RepositoryTagHistory +from endpoints.api.tag_models_interface import TagDataInterface, Tag, RepositoryTagHistory class PreOCIModel(TagDataInterface): diff --git a/endpoints/api/tag_interface/test/test_models_pre_oci.py b/endpoints/api/test/test_models_pre_oci.py similarity index 96% rename from endpoints/api/tag_interface/test/test_models_pre_oci.py rename to endpoints/api/test/test_models_pre_oci.py index 1df367dd1..7481511a0 100644 --- a/endpoints/api/tag_interface/test/test_models_pre_oci.py +++ b/endpoints/api/test/test_models_pre_oci.py @@ -1,10 +1,9 @@ import pytest -from data.model.tag_interface.models_interface import RepositoryTagHistory, Tag +from endpoints.api.tag_models_interface import RepositoryTagHistory, Tag from mock import Mock from data import model -from endpoints.api.tag_interface.models_pre_oci import pre_oci_model - +from endpoints.api.tag_models_pre_oci import pre_oci_model EMPTY_REPOSITORY = 'empty_repository' EMPTY_NAMESPACE = 'empty_namespace' diff --git a/endpoints/api/test/test_tag.py b/endpoints/api/test/test_tag.py index 0c80ef4ee..b2425c3ad 100644 --- a/endpoints/api/test/test_tag.py +++ b/endpoints/api/test/test_tag.py @@ -1,9 +1,12 @@ +import json + import pytest -from mock import patch, Mock +from mock import patch, Mock, MagicMock, call +from endpoints.api.tag_models_interface import RepositoryTagHistory, Tag from endpoints.api.test.shared import client_with_identity, conduct_api_call -from endpoints.api.tag import RepositoryTag, RestoreTag +from endpoints.api.tag import RepositoryTag, RestoreTag, ListRepositoryTags from features import FeatureNameValue from test.fixtures import * @@ -80,6 +83,28 @@ def authd_client(client): yield cl +@pytest.fixture() +def list_repository_tag_history(): + def list_repository_tag_history(namespace_name, repository_name, page, size, specific_tag): + return RepositoryTagHistory(tags=[ + Tag(name='First Tag', image='image', reversion=False, lifetime_start_ts=0, lifetime_end_ts=0, manifest_list=[], + docker_image_id='first docker image id'), + Tag(name='Second Tag', image='second image', reversion=True, lifetime_start_ts=10, lifetime_end_ts=100, + manifest_list=[], docker_image_id='second docker image id')], more=False) + + with patch('endpoints.api.tag.pre_oci_model.list_repository_tag_history', side_effect=list_repository_tag_history): + yield + + +@pytest.fixture() +def find_no_repo_tag_history(): + def list_repository_tag_history(namespace_name, repository_name, page, size, specific_tag): + return None + + with patch('endpoints.api.tag.pre_oci_model.list_repository_tag_history', side_effect=list_repository_tag_history): + yield + + @pytest.mark.parametrize('test_image,test_tag,expected_status', [ ('image1', '-INVALID-TAG-NAME', 400), ('image1', '.INVALID-TAG-NAME', 400), @@ -93,7 +118,7 @@ def authd_client(client): ]) def test_move_tag(test_image, test_tag, expected_status, get_repo_image, get_repo_tag_image, create_or_update_tag, generate_manifest, authd_client): - params = {'repository': 'devtable/repo', 'tag': test_tag} + params = {'repository': 'devtable/simple', 'tag': test_tag} request_body = {'image': test_image} if expected_status is None: with pytest.raises(Exception): @@ -102,6 +127,62 @@ def test_move_tag(test_image, test_tag, expected_status, get_repo_image, get_rep conduct_api_call(authd_client, RepositoryTag, 'put', params, request_body, expected_status) +@pytest.mark.parametrize('namespace, repository, specific_tag, page, limit, expected_response_code, expected', [ + ('devtable', 'simple', None, 1, 10, 200, {'has_additional': False}), + ('devtable', 'simple', None, 1, 10, 200, {'page': 1}), + ('devtable', 'simple', None, 1, 10, 200, {'tags': [{'docker_image_id': 'first docker image id', + 'name': 'First Tag', + 'reversion': False}, + {'docker_image_id': 'second docker image id', + 'end_ts': 100, + 'name': 'Second Tag', + 'reversion': True, + 'start_ts': 10}]}), +]) +def test_list_repository_tags_view_is_correct(namespace, repository, specific_tag, page, limit, + list_repository_tag_history, expected_response_code, expected, + authd_client): + params = {'repository': namespace + '/' + repository, 'specificTag': specific_tag, 'page': page, 'limit': limit} + response = conduct_api_call(authd_client, ListRepositoryTags, 'get', params, expected_code=expected_response_code) + compare_list_history_tags_response(expected, response.json) + + +def compare_list_history_tags_response(expected, actual): + if 'has_additional' in expected: + assert expected['has_additional'] == actual['has_additional'] + + if 'page' in expected: + assert expected['page'] == actual['page'] + + if 'tags' in expected: + assert expected['tags'] == actual['tags'] + + +def test_no_repo_tag_history(find_no_repo_tag_history, authd_client): + params = {'repository': 'devtable/simple', 'specificTag': None, 'page': 1, 'limit': 10} + conduct_api_call(authd_client, ListRepositoryTags, 'get', params, expected_code=404) + + +@pytest.mark.parametrize( + 'specific_tag, page, limit, expected_specific_tag, expected_page, expected_limit', [ + (None, None, None, None, 1, 50), + ('specific_tag', 12, 13, 'specific_tag', 12, 13), + ('specific_tag', -1, 101, 'specific_tag', 1, 100), + ('specific_tag', 0, 0, 'specific_tag', 1, 1), + ]) +def test_repo_tag_history_param_parse(specific_tag, page, limit, expected_specific_tag, expected_page, expected_limit, + authd_client): + mock = MagicMock() + mock.return_value = RepositoryTagHistory(tags=[], more=False) + + with patch('endpoints.api.tag.pre_oci_model.list_repository_tag_history', side_effect=mock): + params = {'repository': 'devtable/simple', 'specificTag': specific_tag, 'page': page, 'limit': limit} + conduct_api_call(authd_client, ListRepositoryTags, 'get', params) + + assert mock.call_args == call(namespace_name='devtable', repository_name='simple', + page=expected_page, size=expected_limit, specific_tag=expected_specific_tag) + + @pytest.mark.parametrize('test_manifest,test_tag,manifest_generated,expected_status', [ (None, 'newtag', True, 200), (None, 'generatemanifestfail', True, None), @@ -110,7 +191,7 @@ def test_move_tag(test_image, test_tag, expected_status, get_repo_image, get_rep def test_restore_tag(test_manifest, test_tag, manifest_generated, expected_status, get_repository, restore_tag_to_manifest, restore_tag_to_image, generate_manifest, authd_client): - params = {'repository': 'devtable/repo', 'tag': test_tag} + params = {'repository': 'devtable/simple', 'tag': test_tag} request_body = {'image': 'image1'} if test_manifest is not None: request_body['manifest_digest'] = test_manifest @@ -121,4 +202,4 @@ def test_restore_tag(test_manifest, test_tag, manifest_generated, expected_statu conduct_api_call(authd_client, RestoreTag, 'post', params, request_body, expected_status) if manifest_generated: - generate_manifest.assert_called_with('devtable', 'repo', test_tag) + generate_manifest.assert_called_with('devtable', 'simple', test_tag)