From 53538f900184bede99393e75c3fca7e56fe85890 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 2 Jun 2016 16:36:38 -0400 Subject: [PATCH] Optimize get_tag_image query No caller uses the image placements or locations, so no need to load them. --- data/model/tag.py | 34 +++++++++++++++++++++++++++------- endpoints/api/tag.py | 2 +- endpoints/v2/manifest.py | 2 +- test/test_secscan.py | 32 ++++++++++++++++---------------- 4 files changed, 45 insertions(+), 25 deletions(-) diff --git a/data/model/tag.py b/data/model/tag.py index a392df2fd..a1e1610e6 100644 --- a/data/model/tag.py +++ b/data/model/tag.py @@ -162,20 +162,40 @@ def garbage_collect_tags(repo): logger.debug('Removed %s tags with %s manifests', num_deleted_tags, num_deleted_manifests) -def get_tag_image(namespace_name, repository_name, tag_name): - def limit_to_tag(query): - return _tag_alive(query - .switch(Image) - .join(RepositoryTag) - .where(RepositoryTag.name == tag_name)) - images = image.get_repository_images_base(namespace_name, repository_name, limit_to_tag) +def _get_repo_tag_image(tag_name, include_storage, modifier): + query = Image.select().join(RepositoryTag) + + if include_storage: + query = (Image.select(Image, ImageStorage) + .join(ImageStorage) + .switch(Image) + .join(RepositoryTag)) + + images = _tag_alive(modifier(query.where(RepositoryTag.name == tag_name))) if not images: raise DataModelException('Unable to find image for tag.') else: return images[0] +def get_repo_tag_image(repo, tag_name, include_storage=False): + def modifier(query): + return query.where(RepositoryTag.repository == repo) + + return _get_repo_tag_image(tag_name, include_storage, modifier) + + +def get_tag_image(namespace_name, repository_name, tag_name, include_storage=False): + def modifier(query): + return (query.switch(RepositoryTag) + .join(Repository) + .join(Namespace) + .where(Namespace.username == namespace_name, Repository.name == repository_name)) + + return _get_repo_tag_image(tag_name, include_storage, modifier) + + def list_repository_tag_history(repo_obj, page=1, size=100, specific_tag=None): query = (RepositoryTag .select(RepositoryTag, Image) diff --git a/endpoints/api/tag.py b/endpoints/api/tag.py index 4be7ab112..9aa462611 100644 --- a/endpoints/api/tag.py +++ b/endpoints/api/tag.py @@ -98,7 +98,7 @@ class RepositoryTag(RepositoryParamResource): original_image_id = None try: - original_tag_image = model.tag.get_tag_image(namespace, repository, tag) + original_tag_image = model.tag.get_repo_tag_image(image.repository, tag) if original_tag_image: original_image_id = original_tag_image.docker_image_id except model.DataModelException: diff --git a/endpoints/v2/manifest.py b/endpoints/v2/manifest.py index 43547a1a9..3c87c199f 100644 --- a/endpoints/v2/manifest.py +++ b/endpoints/v2/manifest.py @@ -514,7 +514,7 @@ def delete_manifest_by_digest(namespace_name, repo_name, manifest_ref): def _generate_and_store_manifest(namespace_name, repo_name, tag_name): # First look up the tag object and its ancestors - image = model.tag.get_tag_image(namespace_name, repo_name, tag_name) + image = model.tag.get_tag_image(namespace_name, repo_name, tag_name, include_storage=True) parents = model.image.get_parent_images(namespace_name, repo_name, image) # If the manifest is being generated under the library namespace, then we make its namespace diff --git a/test/test_secscan.py b/test/test_secscan.py index 4984355a4..e63b70c5f 100644 --- a/test/test_secscan.py +++ b/test/test_secscan.py @@ -147,7 +147,7 @@ class TestSecurityScanner(unittest.TestCase): def test_get_layer_success(self): - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) with HTTMock(get_layer_success_mock, response_content): result = self.api.get_layer_data(layer, include_vulnerabilities=True) self.assertIsNotNone(result) @@ -155,7 +155,7 @@ class TestSecurityScanner(unittest.TestCase): def test_get_layer_failure(self): - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) with HTTMock(get_layer_failure_mock, response_content): result = self.api.get_layer_data(layer, include_vulnerabilities=True) self.assertIsNone(result) @@ -171,7 +171,7 @@ class TestSecurityScanner(unittest.TestCase): # Already registered. pass - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) self.assertFalse(layer.security_indexed) self.assertEquals(-1, layer.security_indexed_engine) @@ -199,7 +199,7 @@ class TestSecurityScanner(unittest.TestCase): def test_analyze_layer_success(self): - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) self.assertFalse(layer.security_indexed) self.assertEquals(-1, layer.security_indexed_engine) @@ -212,7 +212,7 @@ class TestSecurityScanner(unittest.TestCase): def test_analyze_layer_failure(self): - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) self.assertFalse(layer.security_indexed) self.assertEquals(-1, layer.security_indexed_engine) @@ -225,7 +225,7 @@ class TestSecurityScanner(unittest.TestCase): def test_analyze_layer_internal_error(self): - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) self.assertFalse(layer.security_indexed) self.assertEquals(-1, layer.security_indexed_engine) @@ -238,7 +238,7 @@ class TestSecurityScanner(unittest.TestCase): def test_analyze_layer_bad_request(self): - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) self.assertFalse(layer.security_indexed) self.assertEquals(-1, layer.security_indexed_engine) @@ -253,7 +253,7 @@ class TestSecurityScanner(unittest.TestCase): def test_analyze_layer_missing_storage(self): - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) self.assertFalse(layer.security_indexed) self.assertEquals(-1, layer.security_indexed_engine) @@ -273,7 +273,7 @@ class TestSecurityScanner(unittest.TestCase): def test_analyze_layer_success_events(self): - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) self.assertFalse(layer.security_indexed) self.assertEquals(-1, layer.security_indexed_engine) @@ -356,7 +356,7 @@ class TestSecurityScanner(unittest.TestCase): } def test_notification_new_layers_not_vulnerable(self): - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid) # Add a repo event for the layer. @@ -394,7 +394,7 @@ class TestSecurityScanner(unittest.TestCase): def test_notification_delete(self): - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid) # Add a repo event for the layer. @@ -413,7 +413,7 @@ class TestSecurityScanner(unittest.TestCase): def test_notification_new_layers(self): - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid) # Add a repo event for the layer. @@ -464,7 +464,7 @@ class TestSecurityScanner(unittest.TestCase): def test_notification_no_new_layers(self): - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid) # Add a repo event for the layer. @@ -484,7 +484,7 @@ class TestSecurityScanner(unittest.TestCase): def test_notification_no_new_layers_increased_severity(self): - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid) # Add a repo event for the layer. @@ -558,7 +558,7 @@ class TestSecurityScanner(unittest.TestCase): def get_notification(url, request): if url.query.find('page=nextpage') >= 0: pages_called.append('GET-2') - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, COMPLEX_REPO, 'prod') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, COMPLEX_REPO, 'prod', include_storage=True) layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid) data = { @@ -568,7 +568,7 @@ class TestSecurityScanner(unittest.TestCase): return json.dumps(data) else: pages_called.append('GET-1') - layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest') + layer = model.tag.get_tag_image(ADMIN_ACCESS_USER, SIMPLE_REPO, 'latest', include_storage=True) layer_id = '%s.%s' % (layer.docker_image_id, layer.storage.uuid) notification_data = self._get_notification_data([layer_id], [layer_id])