diff --git a/image/docker/schema1.py b/image/docker/schema1.py index 72b1aa8d2..d93cdc62b 100644 --- a/image/docker/schema1.py +++ b/image/docker/schema1.py @@ -259,13 +259,9 @@ class DockerSchema1Manifest(object): updated_id_map[extracted_v1_metadata.image_id] = working_image_id # Lookup the parent image for the layer, if any. - parent_image_id = None - if extracted_v1_metadata.parent_image_id is not None: - parent_image = images_map.get(extracted_v1_metadata.parent_image_id, None) - if parent_image is None: - raise MalformedSchema1Manifest('parent not found with image ID: %s' % - extracted_v1_metadata.parent_image_id) - parent_image_id = updated_id_map.get(parent_image.image_id, parent_image.image_id) + parent_image_id = extracted_v1_metadata.parent_image_id + if parent_image_id is not None: + parent_image_id = updated_id_map.get(parent_image_id, parent_image_id) # Synthesize and store the v1 metadata in the db. v1_metadata_json = layer.raw_v1_metadata @@ -285,7 +281,6 @@ class DockerSchema1Manifest(object): content_checksum=digest_str, ) - images_map[updated_image.image_id] = updated_image yield updated_image diff --git a/test/registry_tests.py b/test/registry_tests.py index 680d5133a..0b58fff2d 100644 --- a/test/registry_tests.py +++ b/test/registry_tests.py @@ -38,7 +38,7 @@ from endpoints.csrf import generate_csrf_token from endpoints.v1 import v1_bp from endpoints.v2 import v2_bp from endpoints.verbs import verbs -from image.docker.schema1 import DockerSchema1ManifestBuilder +from image.docker.schema1 import DockerSchema1ManifestBuilder, DockerSchema1Manifest from initdb import wipe_database, initialize_database, populate_database from jsonschema import validate as validate_schema from util.security.registry_jwt import decode_bearer_header @@ -72,6 +72,14 @@ def set_fakestorage_directdownload(enabled): return 'OK' +@testbp.route('/deleteimage/', methods=['POST']) +def delete_image(image_id): + image = Image.get(docker_image_id=image_id) + image.docker_image_id = 'DELETED' + image.save() + return 'OK' + + @testbp.route('/storagerepentry/', methods=['GET']) def get_storage_replication_entry(image_id): image = Image.get(docker_image_id=image_id) @@ -342,7 +350,7 @@ class V1RegistryPushMixin(V1RegistryMixin): push_version = 'v1' def do_push(self, namespace, repository, username, password, images=None, expect_failure=None, - munge_shas=False, tag_names=None): + munge_shas=[], tag_names=None, head_check=True): images = images or self._get_default_images() auth = (username, password) repo_name = _get_repo_name(namespace, repository) @@ -399,7 +407,7 @@ class V1RegistryPullMixin(V1RegistryMixin): pull_version = 'v1' def do_pull(self, namespace, repository, username=None, password='password', expect_failure=None, - images=None, munge_shas=False): + images=None, munge_shas=[]): images = images or self._get_default_images() repo_name = _get_repo_name(namespace, repository) @@ -422,10 +430,11 @@ class V1RegistryPullMixin(V1RegistryMixin): tags_result = json.loads(self.conduct('GET', prefix + 'tags', auth='sig').text) self.assertEquals(1, len(tags_result.values())) - # Ensure we do (or do not) have a matching image ID. tag_image_id = tags_result['latest'] - known_ids = [item['id'] for item in images] - self.assertEquals(not munge_shas, tag_image_id in known_ids) + if not munge_shas: + # Ensure we have a matching image ID. + known_ids = [item['id'] for item in images] + self.assertTrue(tag_image_id in known_ids) # Retrieve the ancestry of the tag image. image_prefix = '/v1/images/%s/' % tag_image_id @@ -527,7 +536,8 @@ class V2RegistryPushMixin(V2RegistryMixin): push_version = 'v2' def do_push(self, namespace, repository, username, password, images=None, tag_names=None, - cancel=False, invalid=False, expect_failure=None, scopes=None, munge_shas=False): + cancel=False, invalid=False, expect_failure=None, scopes=None, munge_shas=[], + head_check=True): images = images or self._get_default_images() repo_name = _get_repo_name(namespace, repository) @@ -551,8 +561,9 @@ class V2RegistryPushMixin(V2RegistryMixin): manifests = {} full_contents = {} for image_data in reversed(images): - full_contents[image_data['id']] = _get_full_contents(image_data, - additional_fields=munge_shas) + image_id = image_data['id'] + full_contents[image_id] = _get_full_contents(image_data, + additional_fields=image_id in munge_shas) # Build a fake manifest. for tag_name in tag_names: @@ -577,8 +588,10 @@ class V2RegistryPushMixin(V2RegistryMixin): # Layer data should not yet exist. checksum = 'sha256:' + hashlib.sha256(layer_bytes).hexdigest() - self.conduct('HEAD', '/v2/%s/blobs/%s' % (repo_name, checksum), - expected_code=404, auth='jwt') + + if head_check: + self.conduct('HEAD', '/v2/%s/blobs/%s' % (repo_name, checksum), + expected_code=404, auth='jwt') # If we expected a non-404 status code, then the HEAD operation has failed and we cannot # continue performing the push. @@ -657,7 +670,7 @@ class V2RegistryPullMixin(V2RegistryMixin): pull_version = 'v2' def do_pull(self, namespace, repository, username=None, password='password', expect_failure=None, - manifest_id=None, images=None, munge_shas=False): + manifest_id=None, images=None, munge_shas=[]): images = images or self._get_default_images() repo_name = _get_repo_name(namespace, repository) @@ -709,7 +722,7 @@ class V2RegistryPullMixin(V2RegistryMixin): for image in images: self.assertIn(image['id'], found_v1_layers) - return blobs + return blobs, manifest_data class V1RegistryLoginMixin(object): @@ -749,6 +762,120 @@ class V2RegistryLoginMixin(object): class RegistryTestsMixin(object): + def test_middle_layer_different_sha(self): + if self.push_version == 'v1': + # No SHAs to munge in V1. + return + + images = [ + { + 'id': 'rootid', + 'contents': 'The root image', + }, + { + 'id': 'baseid', + 'contents': 'The base image', + }, + ] + + # Push a new repository with two layers. + self.do_push('public', 'newrepo', 'public', 'password', images=images) + + # Pull the repository to verify. + self.do_pull('public', 'newrepo', 'public', 'password', images=images) + + # Push again, munging the middle layer to ensure it gets assigned a different ID. + images = [ + { + 'id': 'rootid', + 'contents': 'The root image', + }, + { + 'id': 'baseid', + 'contents': 'The base image', + }, + { + 'id': 'latestid', + 'contents': 'the latest image', + 'parent': 'baseid', + }, + ] + + munged_shas = ['baseid'] + + # Push the repository. + self.do_push('public', 'newrepo', 'public', 'password', images=images, munge_shas=munged_shas, + head_check=False) + + # Pull the repository to verify. + self.do_pull('public', 'newrepo', 'public', 'password', images=images, munge_shas=munged_shas) + + # Ensures we don't hit weird tag overwrite issues. + time.sleep(1) + + # Delete the baseid image. + self.conduct('POST', '/__test/deleteimage/baseid') + + images = [ + { + 'id': 'rootid', + 'contents': 'The root image', + }, + { + 'id': 'baseid', + 'contents': 'The base image', + }, + { + 'id': 'latestid', + 'contents': 'the latest image', + 'parent': 'baseid', + }, + ] + + # Push the repository again, this time munging the root layer. Since the baseid does not exist + # anymore (since we deleted it above), this will have to look in the layer metadata itself + # to work (which didn't before). + munged_shas = ['rootid'] + self.do_push('public', 'newrepo', 'public', 'password', images=images, munge_shas=munged_shas, + head_check=False) + + # Pull the repository to verify. + self.do_pull('public', 'newrepo', 'public', 'password', images=images, munge_shas=munged_shas) + + + def test_push_same_ids_different_base_sha(self): + if self.push_version == 'v1': + # No SHAs to munge in V1. + return + + images = [ + { + 'id': 'baseid', + 'contents': 'The base image', + }, + { + 'id': 'latestid', + 'contents': 'the latest image', + 'parent': 'baseid', + }, + ] + + munged_shas = ['baseid'] + + # Push a new repository. + self.do_push('public', 'newrepo', 'public', 'password', images=images) + + # Pull the repository. + self.do_pull('public', 'newrepo', 'public', 'password', images=images) + + # Push a the repository again, but with different SHAs. + self.do_push('public', 'newrepo', 'public', 'password', images=images, munge_shas=munged_shas, + head_check=False) + + # Pull the repository. + self.do_pull('public', 'newrepo', 'public', 'password', images=images, munge_shas=munged_shas) + + def test_push_same_ids_different_sha(self): if self.push_version == 'v1': # No SHAs to munge in V1. @@ -766,6 +893,8 @@ class RegistryTestsMixin(object): }, ] + munged_shas = ['latestid'] + # Push a new repository. self.do_push('public', 'newrepo', 'public', 'password', images=images) @@ -773,10 +902,44 @@ class RegistryTestsMixin(object): self.do_pull('public', 'newrepo', 'public', 'password', images=images) # Push a the repository again, but with different SHAs. - self.do_push('public', 'newrepo', 'public', 'password', images=images, munge_shas=True) + self.do_push('public', 'newrepo', 'public', 'password', images=images, munge_shas=munged_shas, + head_check=False) # Pull the repository. - self.do_pull('public', 'newrepo', 'public', 'password', images=images, munge_shas=True) + self.do_pull('public', 'newrepo', 'public', 'password', images=images, munge_shas=munged_shas) + + + def test_push_same_ids_different_sha_both_layers(self): + if self.push_version == 'v1': + # No SHAs to munge in V1. + return + + images = [ + { + 'id': 'baseid', + 'contents': 'The base image', + }, + { + 'id': 'latestid', + 'contents': 'the latest image', + 'parent': 'baseid', + }, + ] + + munged_shas = ['baseid', 'latestid'] + + # Push a new repository. + self.do_push('public', 'newrepo', 'public', 'password', images=images) + + # Pull the repository. + self.do_pull('public', 'newrepo', 'public', 'password', images=images) + + # Push a the repository again, but with different SHAs. + self.do_push('public', 'newrepo', 'public', 'password', images=images, munge_shas=munged_shas, + head_check=False) + + # Pull the repository. + self.do_pull('public', 'newrepo', 'public', 'password', images=images, munge_shas=munged_shas) def test_push_same_ids_different_sha_with_unicode(self): @@ -797,6 +960,8 @@ class RegistryTestsMixin(object): }, ] + munged_shas = ['latestid', 'baseid'] + # Push a new repository. self.do_push('public', 'newrepo', 'public', 'password', images=images) @@ -804,10 +969,11 @@ class RegistryTestsMixin(object): self.do_pull('public', 'newrepo', 'public', 'password', images=images) # Push a the repository again, but with different SHAs. - self.do_push('public', 'newrepo', 'public', 'password', images=images, munge_shas=True) + self.do_push('public', 'newrepo', 'public', 'password', images=images, munge_shas=munged_shas, + head_check=False) # Pull the repository. - self.do_pull('public', 'newrepo', 'public', 'password', images=images, munge_shas=True) + self.do_pull('public', 'newrepo', 'public', 'password', images=images, munge_shas=munged_shas) def test_push_pull_logging(self): @@ -1363,7 +1529,7 @@ class V2RegistryTests(V2RegistryPullMixin, V2RegistryPushMixin, RegistryTestsMix self.do_push('devtable', 'newrepo', 'devtable', 'password', images=images) # Pull the image back and verify the contents. - blobs = self.do_pull('devtable', 'newrepo', 'devtable', 'password', images=images) + blobs, _ = self.do_pull('devtable', 'newrepo', 'devtable', 'password', images=images) self.assertEquals(len(blobs.items()), 1) self.assertEquals(blobs.items()[0][1], contents) @@ -1384,7 +1550,7 @@ class V2RegistryTests(V2RegistryPullMixin, V2RegistryPushMixin, RegistryTestsMix self.do_push('devtable', 'newrepo', 'devtable', 'password', images=images) # Pull the image back and verify the contents. - blobs = self.do_pull('devtable', 'newrepo', 'devtable', 'password', images=images) + blobs, _ = self.do_pull('devtable', 'newrepo', 'devtable', 'password', images=images) self.assertEquals(len(blobs.items()), 1) self.assertEquals(blobs.items()[0][1], contents) @@ -1406,7 +1572,7 @@ class V2RegistryTests(V2RegistryPullMixin, V2RegistryPushMixin, RegistryTestsMix self.do_push('devtable', 'newrepo', 'devtable', 'password', images=images) # Pull the image back and verify the contents. - blobs = self.do_pull('devtable', 'newrepo', 'devtable', 'password', images=images) + blobs, _ = self.do_pull('devtable', 'newrepo', 'devtable', 'password', images=images) self.assertEquals(len(blobs.items()), 1) self.assertEquals(blobs.items()[0][1], contents) @@ -1583,8 +1749,8 @@ class TorrentTestMixin(V2RegistryPullMixin): self.do_push('devtable', 'newrepo', 'devtable', 'password', images=initial_images) # Retrieve the manifest for the tag. - blobs = self.do_pull('devtable', 'newrepo', 'devtable', 'password', manifest_id='latest', - images=initial_images) + blobs, _ = self.do_pull('devtable', 'newrepo', 'devtable', 'password', manifest_id='latest', + images=initial_images) self.assertEquals(1, len(list(blobs.keys()))) blobsum = list(blobs.keys())[0]