From 1eaf5b18dd4f416288370a8fe00717e88ef816be Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 15 Nov 2018 13:51:48 +0200 Subject: [PATCH] Adjustments based on code review feedback --- data/model/oci/test/test_oci_manifest.py | 10 ++-------- image/docker/interfaces.py | 6 +++++- image/docker/schema1.py | 4 ++++ image/docker/schema2/list.py | 8 ++++++-- image/docker/schema2/manifest.py | 4 ++++ image/docker/schema2/test/test_list.py | 4 ---- .../manifest-security-view.component.ts | 2 +- test/registry/protocol_v2.py | 3 ++- workers/manifestbackfillworker.py | 8 ++++++-- 9 files changed, 30 insertions(+), 19 deletions(-) diff --git a/data/model/oci/test/test_oci_manifest.py b/data/model/oci/test/test_oci_manifest.py index 45df20597..def62400d 100644 --- a/data/model/oci/test/test_oci_manifest.py +++ b/data/model/oci/test/test_oci_manifest.py @@ -206,14 +206,8 @@ def test_get_or_create_manifest_list(initialized_db): v2_manifest = v2_builder.build() # Write the manifests as blobs. - location = ImageStorageLocation.get(name='local_us') - blob = store_blob_record_and_temp_link('devtable', 'newrepo', v1_manifest.digest, location, - len(v1_manifest.bytes), 120) - storage.put_content(['local_us'], get_layer_path(blob), v1_manifest.bytes) - - blob = store_blob_record_and_temp_link('devtable', 'newrepo', v2_manifest.digest, location, - len(v2_manifest.bytes), 120) - storage.put_content(['local_us'], get_layer_path(blob), v2_manifest.bytes) + _populate_blob(v1_manifest.bytes) + _populate_blob(v2_manifest.bytes) # Build the manifest list. list_builder = DockerSchema2ManifestListBuilder() diff --git a/image/docker/interfaces.py b/image/docker/interfaces.py index 2af17f1d4..d34e1e140 100644 --- a/image/docker/interfaces.py +++ b/image/docker/interfaces.py @@ -4,9 +4,13 @@ from six import add_metaclass @add_metaclass(ABCMeta) class ManifestInterface(object): """ Defines the interface for the various manifests types supported. """ + @abstractproperty + def is_manifest_list(self): + """ Returns whether this manifest is a list. """ + @abstractproperty def schema_version(self): - """ The version of the schema, or None for lists. """ + """ The version of the schema. """ @abstractproperty def digest(self): diff --git a/image/docker/schema1.py b/image/docker/schema1.py index 2a5d26e25..e5698151d 100644 --- a/image/docker/schema1.py +++ b/image/docker/schema1.py @@ -206,6 +206,10 @@ class DockerSchema1Manifest(ManifestInterface): if not verified: raise InvalidSchema1Signature() + @property + def is_manifest_list(self): + return False + @property def schema_version(self): return 1 diff --git a/image/docker/schema2/list.py b/image/docker/schema2/list.py index 27939523d..0b068f0c6 100644 --- a/image/docker/schema2/list.py +++ b/image/docker/schema2/list.py @@ -183,10 +183,14 @@ class DockerSchema2ManifestList(ManifestInterface): except ValidationError as ve: raise MalformedSchema2ManifestList('manifest data does not match schema: %s' % ve) + @property + def is_manifest_list(self): + """ Returns whether this manifest is a list. """ + return True + @property def schema_version(self): - """ The version of the schema, or None for lists. """ - return None + return 2 @property def digest(self): diff --git a/image/docker/schema2/manifest.py b/image/docker/schema2/manifest.py index 13fe52626..b38f53ac4 100644 --- a/image/docker/schema2/manifest.py +++ b/image/docker/schema2/manifest.py @@ -140,6 +140,10 @@ class DockerSchema2Manifest(ManifestInterface): if layer.is_remote and not layer.urls: raise MalformedSchema2Manifest('missing `urls` for remote layer') + @property + def is_manifest_list(self): + return False + @property def schema_version(self): return 2 diff --git a/image/docker/schema2/test/test_list.py b/image/docker/schema2/test/test_list.py index 2dc3e2ad5..dc5ca5252 100644 --- a/image/docker/schema2/test/test_list.py +++ b/image/docker/schema2/test/test_list.py @@ -75,8 +75,6 @@ def test_valid_manifestlist(): manifestlist = DockerSchema2ManifestList(MANIFESTLIST_BYTES) assert len(manifestlist.manifests(_get_manifest)) == 2 - assert (manifestlist.digest == - 'sha256:340d7dadea77035533a2d43e8ff711ecca1965978a6e7699b87e32b35f76038d') assert manifestlist.media_type == 'application/vnd.docker.distribution.manifest.list.v2+json' assert manifestlist.bytes == MANIFESTLIST_BYTES @@ -108,8 +106,6 @@ def test_get_v1_compatible_manifest_no_matching_list(): manifestlist = DockerSchema2ManifestList(NO_AMD_MANIFESTLIST_BYTES) assert len(manifestlist.manifests(_get_manifest)) == 1 - assert (manifestlist.digest == - 'sha256:40ed1cfe692333bfa519a9bfed9676975a990fff5afd35efa628320c39c793ca') assert manifestlist.media_type == 'application/vnd.docker.distribution.manifest.list.v2+json' assert manifestlist.bytes == NO_AMD_MANIFESTLIST_BYTES diff --git a/static/js/directives/ui/manifest-security-view/manifest-security-view.component.ts b/static/js/directives/ui/manifest-security-view/manifest-security-view.component.ts index be0ec9c15..e02c4f439 100644 --- a/static/js/directives/ui/manifest-security-view/manifest-security-view.component.ts +++ b/static/js/directives/ui/manifest-security-view/manifest-security-view.component.ts @@ -72,5 +72,5 @@ export class ManifestSecurityView { securityStatus.loading = false; securityStatus.hasError = true; }); - }; + } } diff --git a/test/registry/protocol_v2.py b/test/registry/protocol_v2.py index 2c37aa4f3..0ce02440f 100644 --- a/test/registry/protocol_v2.py +++ b/test/registry/protocol_v2.py @@ -159,7 +159,8 @@ class V2Protocol(RegistryProtocol): # Parse the returned manifest list and ensure it matches. assert response.headers['Content-Type'] == DOCKER_SCHEMA2_MANIFESTLIST_CONTENT_TYPE manifest = parse_manifest_from_bytes(response.text, response.headers['Content-Type']) - assert manifest.schema_version is None + assert manifest.schema_version == 2 + assert manifest.is_manifest_list assert manifest.digest == manifestlist.digest diff --git a/workers/manifestbackfillworker.py b/workers/manifestbackfillworker.py index 4c5c35995..28715bf21 100644 --- a/workers/manifestbackfillworker.py +++ b/workers/manifestbackfillworker.py @@ -83,13 +83,17 @@ class BrokenManifest(ManifestInterface): return self @property - def schema_version(): + def schema_version(self): return 1 @property - def layers_compressed_size(): + def layers_compressed_size(self): return None + @property + def is_manifest_list(self): + return False + class ManifestBackfillWorker(Worker): def __init__(self):