From 8e643ce5d9179b7fb671e7c1837b3d004e3f4097 Mon Sep 17 00:00:00 2001 From: Kenny Lee Sin Cheong <2530351+kleesc@users.noreply.github.com> Date: Fri, 14 Sep 2018 15:30:54 -0400 Subject: [PATCH] Repository endpoint tags pagination (#3238) * endpoint/api/repository: limit the number of tags returned - Limit the number of tags returned by /api/v1/repository/ to 500. - Uses the tag history endpoint instead, with an active tag filte. - Update UI to use tag history endpoint instead. --- data/model/tag.py | 5 ++- data/registry_model/interface.py | 2 +- data/registry_model/registry_pre_oci_model.py | 5 +-- endpoints/api/repository.py | 6 +++- endpoints/api/repository_models_interface.py | 9 ++--- endpoints/api/repository_models_pre_oci.py | 20 ++++++----- endpoints/api/tag.py | 18 ++++++++-- .../directives/repo-view/repo-panel-tags.js | 6 ++-- .../js/directives/ui/tag-operations-dialog.js | 4 +-- static/js/pages/app-view.js | 3 +- static/js/pages/build-view.js | 3 +- .../pages/create-repository-notification.js | 5 +-- static/js/pages/manifest-view.js | 3 +- static/js/pages/repo-view.js | 35 ++++++++++++++++++- static/js/pages/trigger-setup.js | 5 +-- test/test_api_usage.py | 4 +-- 16 files changed, 99 insertions(+), 34 deletions(-) diff --git a/data/model/tag.py b/data/model/tag.py index fb366c01a..52fa1ba39 100644 --- a/data/model/tag.py +++ b/data/model/tag.py @@ -491,7 +491,7 @@ def get_tag_image(namespace_name, repository_name, tag_name, include_storage=Fal return _get_repo_tag_image(tag_name, include_storage, modifier) -def list_repository_tag_history(repo_obj, page=1, size=100, specific_tag=None): +def list_repository_tag_history(repo_obj, page=1, size=100, specific_tag=None, active_tags_only=False): query = (RepositoryTag .select(RepositoryTag, Image, ImageStorage) .join(Image) @@ -503,6 +503,9 @@ def list_repository_tag_history(repo_obj, page=1, size=100, specific_tag=None): .limit(size + 1) .offset(size * (page - 1))) + if active_tags_only: + query = _tag_alive(query) + if specific_tag: query = query.where(RepositoryTag.name == specific_tag) diff --git a/data/registry_model/interface.py b/data/registry_model/interface.py index c1014b4d4..a565504d6 100644 --- a/data/registry_model/interface.py +++ b/data/registry_model/interface.py @@ -81,7 +81,7 @@ class RegistryDataInterface(object): """ @abstractmethod - def list_repository_tag_history(self, repository_ref, page=1, size=100, specific_tag_name=None): + def list_repository_tag_history(self, repository_ref, page=1, size=100, specific_tag_name=None, active_tags_only=False): """ Returns the history of all tags in the repository (unless filtered). This includes tags that have been made in-active due to newer versions of those tags coming into service. diff --git a/data/registry_model/registry_pre_oci_model.py b/data/registry_model/registry_pre_oci_model.py index ec197a5dd..cdb038533 100644 --- a/data/registry_model/registry_pre_oci_model.py +++ b/data/registry_model/registry_pre_oci_model.py @@ -164,14 +164,15 @@ class PreOCIModel(RegistryDataInterface): else None)) for tag in tags] - def list_repository_tag_history(self, repository_ref, page=1, size=100, specific_tag_name=None): + def list_repository_tag_history(self, repository_ref, page=1, size=100, specific_tag_name=None, active_tags_only=False): """ Returns the history of all tags in the repository (unless filtered). This includes tags that have been made in-active due to newer versions of those tags coming into service. """ tags, manifest_map, has_more = model.tag.list_repository_tag_history(repository_ref._db_id, page, size, - specific_tag_name) + specific_tag_name, + active_tags_only) return [Tag.for_repository_tag(tag, manifest_map.get(tag.id), legacy_image=LegacyImage.for_image(tag.image)) for tag in tags], has_more diff --git a/endpoints/api/repository.py b/endpoints/api/repository.py index c76dffd99..8baedb60a 100644 --- a/endpoints/api/repository.py +++ b/endpoints/api/repository.py @@ -192,12 +192,16 @@ class Repository(RepositoryParamResource): @parse_args() @query_param('includeStats', 'Whether to include action statistics', type=truthy_bool, default=False) + @query_param('includeTags', 'Whether to include repository tags', type=truthy_bool, + default=True) @require_repo_read @nickname('getRepo') def get(self, namespace, repository, parsed_args): """Fetch the specified repository.""" logger.debug('Get repo: %s/%s' % (namespace, repository)) - repo = model.get_repo(namespace, repository, get_authenticated_user()) + include_tags = parsed_args['includeTags'] + max_tags = 500; + repo = model.get_repo(namespace, repository, get_authenticated_user(), include_tags, max_tags) if repo is None: raise NotFound() diff --git a/endpoints/api/repository_models_interface.py b/endpoints/api/repository_models_interface.py index f24103c91..107c8415a 100644 --- a/endpoints/api/repository_models_interface.py +++ b/endpoints/api/repository_models_interface.py @@ -87,7 +87,7 @@ class ImageRepositoryRepository( """ def to_dict(self): - return { + img_repo = { 'namespace': self.repository_base_elements.namespace_name, 'name': self.repository_base_elements.repository_name, 'kind': self.repository_base_elements.kind_name, @@ -95,12 +95,13 @@ class ImageRepositoryRepository( 'is_public': self.repository_base_elements.is_public, 'is_organization': self.repository_base_elements.namespace_user_organization, 'is_starred': self.repository_base_elements.is_starred, - 'tags': {tag.name: tag.to_dict() - for tag in self.tags}, 'status_token': self.badge_token if not self.repository_base_elements.is_public else '', 'trust_enabled': bool(features.SIGNING) and self.trust_enabled, 'tag_expiration_s': self.repository_base_elements.namespace_user_removed_tag_expiration_s, } + if self.tags is not None: + img_repo['tags'] = {tag.name: tag.to_dict() for tag in self.tags} + return img_repo class Repository(namedtuple('Repository', [ @@ -203,7 +204,7 @@ class RepositoryDataInterface(object): """ @abstractmethod - def get_repo(self, namespace_name, repository_name, user): + def get_repo(self, namespace_name, repository_name, user, include_tags=True, max_tags=500): """ Returns a repository """ diff --git a/endpoints/api/repository_models_pre_oci.py b/endpoints/api/repository_models_pre_oci.py index 0ef62773c..4e6409c20 100644 --- a/endpoints/api/repository_models_pre_oci.py +++ b/endpoints/api/repository_models_pre_oci.py @@ -134,7 +134,7 @@ class PreOCIModel(RepositoryDataInterface): repo_kind=repo_kind, description=description) return Repository(namespace_name, repository_name) - def get_repo(self, namespace_name, repository_name, user): + def get_repo(self, namespace_name, repository_name, user, include_tags=True, max_tags=500): repo = model.repository.get_repository(namespace_name, repository_name) if repo is None: return None @@ -156,18 +156,22 @@ class PreOCIModel(RepositoryDataInterface): for release in releases ]) + tags = None repo_ref = RepositoryReference.for_repo_obj(repo) - tags = registry_model.list_repository_tags(repo_ref, include_legacy_images=True) + if include_tags: + tags, _ = registry_model.list_repository_tag_history(repo_ref, page=1, size=max_tags, active_tags_only=True) + tags = [ + Tag(tag.name, tag.legacy_image.docker_image_id, tag.legacy_image.aggregate_size, + tag.lifetime_start_ts, + tag.manifest_digest, + tag.lifetime_end_ts) for tag in tags + ] start_date = datetime.now() - timedelta(days=MAX_DAYS_IN_3_MONTHS) counts = model.log.get_repository_action_counts(repo, start_date) - return ImageRepositoryRepository(base, [ - Tag(tag.name, tag.legacy_image.docker_image_id, tag.legacy_image.aggregate_size, - tag.lifetime_start_ts, - tag.manifest_digest, - tag.lifetime_end_ts) for tag in tags - ], [Count(count.date, count.count) for count in counts], repo.badge_token, repo.trust_enabled) + return ImageRepositoryRepository(base, tags, + [Count(count.date, count.count) for count in counts], repo.badge_token, repo.trust_enabled) pre_oci_model = PreOCIModel() diff --git a/endpoints/api/tag.py b/endpoints/api/tag.py index 36eeea1a3..f7fe496d4 100644 --- a/endpoints/api/tag.py +++ b/endpoints/api/tag.py @@ -7,7 +7,8 @@ from auth.auth_context import get_authenticated_user from data.registry_model import registry_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) + parse_args, query_param, truthy_bool, disallow_for_app_repositories, + format_date) from endpoints.api.image import image_dict from endpoints.exception import NotFound, InvalidRequest from util.names import TAG_ERROR, TAG_REGEX @@ -30,6 +31,16 @@ def _tag_dict(tag): if tag.legacy_image: tag_info['docker_image_id'] = tag.legacy_image.docker_image_id + tag_info['image_id'] = tag.legacy_image.docker_image_id + tag_info['size'] = tag.legacy_image.aggregate_size + + if tag.lifetime_start_ts > 0: + last_modified = format_date(datetime.fromtimestamp(tag.lifetime_start_ts)) + tag_info['last_modified'] = last_modified + + if tag.lifetime_end_ts is not None: + expiration = format_date(datetime.fromtimestamp(tag.lifetime_end_ts)) + tag_info['expiration'] = expiration return tag_info @@ -46,11 +57,13 @@ class ListRepositoryTags(RepositoryParamResource): @query_param('limit', 'Limit to the number of results to return per page. Max 100.', type=int, default=50) @query_param('page', 'Page index for the results. Default 1.', type=int, default=1) + @query_param('onlyActiveTags', 'Filter to only active tags.', type=truthy_bool, default=False) @nickname('listRepoTags') def get(self, namespace, repository, parsed_args): 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))) + active_tags_only = parsed_args.get('onlyActiveTags') repo_ref = registry_model.lookup_repository(namespace, repository) if repo_ref is None: @@ -58,7 +71,8 @@ class ListRepositoryTags(RepositoryParamResource): history, has_more = registry_model.list_repository_tag_history(repo_ref, page=page, size=limit, - specific_tag_name=specific_tag) + specific_tag_name=specific_tag, + active_tags_only=active_tags_only) return { 'tags': [_tag_dict(tag) for tag in history], 'page': page, diff --git a/static/js/directives/repo-view/repo-panel-tags.js b/static/js/directives/repo-view/repo-panel-tags.js index f765df037..4e602cd2c 100644 --- a/static/js/directives/repo-view/repo-panel-tags.js +++ b/static/js/directives/repo-view/repo-panel-tags.js @@ -237,13 +237,13 @@ angular.module('quay').directive('repoPanelTags', function () { })); }, true); - $scope.$watch('repository', function(repository) { - if (!repository) { return; } + $scope.$watch('repository', function(updatedRepoObject, previousRepoObject) { + if (updatedRepoObject.tags === previousRepoObject.tags) { return; } // Process each of the tags. setTagState(); loadRepoSignatures(); - }); + }, true); $scope.loadImageVulnerabilities = function(image_id, imageData) { VulnerabilityService.loadImageVulnerabilities($scope.repository, image_id, function(resp) { diff --git a/static/js/directives/ui/tag-operations-dialog.js b/static/js/directives/ui/tag-operations-dialog.js index 65ebb988d..c3db786e4 100644 --- a/static/js/directives/ui/tag-operations-dialog.js +++ b/static/js/directives/ui/tag-operations-dialog.js @@ -46,7 +46,7 @@ angular.module('quay').directive('tagOperationsDialog', function () { }; $scope.isAnotherImageTag = function(image, tag) { - if (!$scope.repository) { return; } + if (!$scope.repository.tags) { return; } var found = $scope.repository.tags[tag]; if (found == null) { return false; } @@ -54,7 +54,7 @@ angular.module('quay').directive('tagOperationsDialog', function () { }; $scope.isOwnedTag = function(image, tag) { - if (!$scope.repository) { return; } + if (!$scope.repository.tags) { return; } var found = $scope.repository.tags[tag]; if (found == null) { return false; } diff --git a/static/js/pages/app-view.js b/static/js/pages/app-view.js index e4c54a797..a218e94fa 100644 --- a/static/js/pages/app-view.js +++ b/static/js/pages/app-view.js @@ -25,7 +25,8 @@ var params = { 'repository': $scope.namespace + '/' + $scope.name, 'repo_kind': 'application', - 'includeStats': true + 'includeStats': true, + 'includeTags': false }; $scope.repositoryResource = ApiService.getRepoAsResource(params).get(function(repo) { diff --git a/static/js/pages/build-view.js b/static/js/pages/build-view.js index 77f0f503e..29bdc8a50 100644 --- a/static/js/pages/build-view.js +++ b/static/js/pages/build-view.js @@ -35,7 +35,8 @@ var loadRepository = function() { var params = { - 'repository': $scope.namespace + '/' + $scope.name + 'repository': $scope.namespace + '/' + $scope.name, + 'includeTags': false }; $scope.repoResource = ApiService.getRepoAsResource(params).get(function(repo) { diff --git a/static/js/pages/create-repository-notification.js b/static/js/pages/create-repository-notification.js index 7a02ca4d6..8c32b62f8 100644 --- a/static/js/pages/create-repository-notification.js +++ b/static/js/pages/create-repository-notification.js @@ -16,7 +16,8 @@ var loadRepository = function() { var params = { - 'repository': $scope.namespace + '/' + $scope.name + 'repository': $scope.namespace + '/' + $scope.name, + 'includeTags': false }; $scope.repositoryResource = ApiService.getRepoAsResource(params).get(function(repo) { @@ -30,4 +31,4 @@ $location.url('repository/' + $scope.namespace + '/' + $scope.name + '?tab=settings'); }; } -})(); \ No newline at end of file +})(); diff --git a/static/js/pages/manifest-view.js b/static/js/pages/manifest-view.js index b95fba6b5..bbb4506cc 100644 --- a/static/js/pages/manifest-view.js +++ b/static/js/pages/manifest-view.js @@ -36,7 +36,8 @@ var loadRepository = function() { var params = { - 'repository': namespace + '/' + name + 'repository': namespace + '/' + name, + 'includeTags': false }; $scope.repositoryResource = ApiService.getRepoAsResource(params).get(function(repo) { diff --git a/static/js/pages/repo-view.js b/static/js/pages/repo-view.js index 1081cff71..a7dcba245 100644 --- a/static/js/pages/repo-view.js +++ b/static/js/pages/repo-view.js @@ -33,6 +33,8 @@ 'historyFilter': '' }; + $scope.repositoryTags = {}; + var buildPollChannel = null; // Make sure we track the current user. @@ -50,17 +52,48 @@ }); }; + var loadRepositoryTags = function() { + loadPaginatedRepositoryTags(1); + }; + + var loadPaginatedRepositoryTags = function(page) { + var params = { + 'repository': $scope.namespace + '/' + $scope.name, + 'limit': 100, + 'page': page, + 'onlyActiveTags': true + }; + + ApiService.listRepoTags(null, params).then(function(resp) { + var newTags = resp.tags.reduce(function(result, item, index, array) { + var tag_name = item['name']; + result[tag_name] = item; + return result; + }, {}); + + $.extend($scope.repositoryTags, newTags); + + if (resp.has_additional) { + loadPaginatedRepositoryTags(page + 1); + } + }); + }; + var loadRepository = function() { // Mark the images to be reloaded. $scope.viewScope.images = null; var params = { 'repository': $scope.namespace + '/' + $scope.name, - 'includeStats': true + 'includeStats': true, + 'includeTags': false }; $scope.repositoryResource = ApiService.getRepoAsResource(params).get(function(repo) { if (repo != undefined) { + loadRepositoryTags(); + repo.tags = $scope.repositoryTags; + $scope.repository = repo; $scope.viewScope.repository = repo; diff --git a/static/js/pages/trigger-setup.js b/static/js/pages/trigger-setup.js index c88373850..1eb81cdfb 100644 --- a/static/js/pages/trigger-setup.js +++ b/static/js/pages/trigger-setup.js @@ -17,7 +17,8 @@ var loadRepository = function() { var params = { - 'repository': namespace + '/' + name + 'repository': namespace + '/' + name, + 'includeTags': false }; $scope.repositoryResource = ApiService.getRepoAsResource(params).get(function(repo) { @@ -86,4 +87,4 @@ return trigger_uuid.split('-')[0]; }; } -}()); \ No newline at end of file +}()); diff --git a/test/test_api_usage.py b/test/test_api_usage.py index 7337748f9..bc19d5074 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -2219,11 +2219,11 @@ class TestGetRepository(ApiTestCase): self.login(ADMIN_ACCESS_USER) # base + repo + is_starred + tags - with assert_query_count(BASE_LOGGEDIN_QUERY_COUNT + 5): + with assert_query_count(BASE_LOGGEDIN_QUERY_COUNT + 6): self.getJsonResponse(Repository, params=dict(repository=ADMIN_ACCESS_USER + '/simple')) # base + repo + is_starred + tags - with assert_query_count(BASE_LOGGEDIN_QUERY_COUNT + 5): + with assert_query_count(BASE_LOGGEDIN_QUERY_COUNT + 6): json = self.getJsonResponse(Repository, params=dict(repository=ADMIN_ACCESS_USER + '/gargantuan'))