Unify the get_layers calls across all implements of manifest schemas to ensure we have a common type returned

Also renames some methods to make it more clear what kind of information they return

image/docker/interfaces.py

@@ -32,18 +32,18 @@ class ManifestInterface(object):
     """ Returns the bytes of the manifest. """
     pass
 
-  @abstractproperty
-  def layers(self):
-    """ Returns the layers of this manifest, from base to leaf or None if this kind of manifest
-        does not support layers. """
-    pass
-
   @abstractproperty
   def layers_compressed_size(self):
     """ Returns the total compressed size of all the layers in this manifest. Returns None if this
         cannot be computed locally.
     """
 
+  @abstractmethod
+  def get_layers(self, content_retriever):
+    """ Returns the layers of this manifest, from base to leaf or None if this kind of manifest
+        does not support layers. The layer must be of type ManifestImageLayer. """
+    pass
+
   @abstractmethod
   def get_leaf_layer_v1_image_id(self, content_retriever):
     """ Returns the Docker V1 image ID for the leaf (top) layer, if any, or None if

image/docker/schema1.py

@@ -20,6 +20,7 @@ from jwt.utils import base64url_encode, base64url_decode
 
 from digest import digest_tools
 from image.docker import ManifestException
+from image.docker.types import ManifestImageLayer
 from image.docker.interfaces import ManifestInterface
 from image.docker.v1 import DockerV1Metadata
 
@@ -73,7 +74,7 @@ class InvalidSchema1Signature(ManifestException):
 
 
 class Schema1Layer(namedtuple('Schema1Layer', ['digest', 'v1_metadata', 'raw_v1_metadata',
-                                               'compressed_size', 'is_remote'])):
+                                               'compressed_size', 'is_remote', 'urls'])):
   """
   Represents all of the data about an individual layer in a given Manifest.
   This is the union of the fsLayers (digest) and the history entries (v1_compatibility).
@@ -298,6 +299,25 @@ class DockerSchema1Manifest(ManifestInterface):
       self._layers = list(self._generate_layers())
     return self._layers
 
+  def get_layers(self, content_retriever):
+    """ Returns the layers of this manifest, from base to leaf or None if this kind of manifest
+        does not support layers. """
+    for layer in self.layers:
+      created_datetime = None
+      try:
+        created_datetime = dateutil.parser.parse(layer.v1_metadata.created).replace(tzinfo=None)
+      except:
+        pass
+
+      yield ManifestImageLayer(layer_id=layer.v1_metadata.image_id,
+                               compressed_size=layer.compressed_size,
+                               is_remote=False,
+                               urls=None,
+                               command=layer.v1_metadata.command,
+                               blob_digest=layer.digest,
+                               created_datetime=created_datetime,
+                               internal_layer=layer)
+
   @property
   def blob_digests(self):
     return [str(layer.digest) for layer in self.layers]
@@ -356,7 +376,7 @@ class DockerSchema1Manifest(ManifestInterface):
                                     command, labels)
 
       compressed_size = v1_metadata.get('Size')
-      yield Schema1Layer(image_digest, extracted, metadata_string, compressed_size, False)
+      yield Schema1Layer(image_digest, extracted, metadata_string, compressed_size, False, None)
 
   @property
   def _payload(self):

image/docker/schema2/list.py

@@ -211,8 +211,9 @@ class DockerSchema2ManifestList(ManifestInterface):
   def bytes(self):
     return self._manifest_bytes
 
-  @property
-  def layers(self):
+  def get_layers(self, content_retriever):
+    """ Returns the layers of this manifest, from base to leaf or None if this kind of manifest
+        does not support layers. """
     return None
 
   @property

image/docker/schema2/manifest.py

@@ -8,6 +8,7 @@ from jsonschema import validate as validate_schema, ValidationError
 from digest import digest_tools
 from image.docker import ManifestException
 from image.docker.interfaces import ManifestInterface
+from image.docker.types import ManifestImageLayer
 from image.docker.schema2 import (DOCKER_SCHEMA2_MANIFEST_CONTENT_TYPE,
                                   DOCKER_SCHEMA2_CONFIG_CONTENT_TYPE,
                                   DOCKER_SCHEMA2_LAYER_CONTENT_TYPE,
@@ -31,9 +32,10 @@ DockerV2ManifestLayer = namedtuple('DockerV2ManifestLayer', ['index', 'digest',
                                                              'is_remote', 'urls',
                                                              'compressed_size'])
 
-ManifestImageLayer = namedtuple('ManifestImageLayer', ['history', 'blob_layer', 'v1_id',
-                                                       'v1_parent_id', 'compressed_size',
-                                                       'blob_digest'])
+DockerV2ManifestImageLayer = namedtuple('DockerV2ManifestImageLayer', ['history', 'blob_layer',
+                                                                       'v1_id', 'v1_parent_id',
+                                                                       'compressed_size',
+                                                                       'blob_digest'])
 
 logger = logging.getLogger(__name__)
 
@@ -126,7 +128,7 @@ class DockerSchema2Manifest(ManifestInterface):
   }
 
   def __init__(self, manifest_bytes):
-    self._layers = None
+    self._filesystem_layers = None
     self._payload = manifest_bytes
     self._cached_built_config = None
 
@@ -140,7 +142,7 @@ class DockerSchema2Manifest(ManifestInterface):
     except ValidationError as ve:
       raise MalformedSchema2Manifest('manifest data does not match schema: %s' % ve)
 
-    for layer in self.layers:
+    for layer in self.filesystem_layers:
       if layer.is_remote and not layer.urls:
         raise MalformedSchema2Manifest('missing `urls` for remote layer')
 
@@ -171,24 +173,24 @@ class DockerSchema2Manifest(ManifestInterface):
                                   digest=config[DOCKER_SCHEMA2_MANIFEST_DIGEST_KEY])
 
   @property
-  def layers(self):
-    """ Returns the layers of this manifest, from base to leaf. """
-    if self._layers is None:
-      self._layers = list(self._generate_layers())
-    return self._layers
+  def filesystem_layers(self):
+    """ Returns the file system layers of this manifest, from base to leaf. """
+    if self._filesystem_layers is None:
+      self._filesystem_layers = list(self._generate_filesystem_layers())
+    return self._filesystem_layers
 
   @property
-  def leaf_layer(self):
-    """ Returns the leaf layer for this manifest. """
-    return self.layers[-1]
+  def leaf_filesystem_layer(self):
+    """ Returns the leaf file system layer for this manifest. """
+    return self.filesystem_layers[-1]
 
   @property
   def layers_compressed_size(self):
-    return sum(layer.compressed_size for layer in self.layers)
+    return sum(layer.compressed_size for layer in self.filesystem_layers)
 
   @property
   def has_remote_layer(self):
-    for layer in self.layers:
+    for layer in self.filesystem_layers:
       if layer.is_remote:
         return True
 
@@ -196,16 +198,31 @@ class DockerSchema2Manifest(ManifestInterface):
 
   @property
   def blob_digests(self):
-    return [str(layer.digest) for layer in self.layers] + [str(self.config.digest)]
+    return [str(layer.digest) for layer in self.filesystem_layers] + [str(self.config.digest)]
 
   @property
   def local_blob_digests(self):
-    return ([str(layer.digest) for layer in self.layers if not layer.urls] +
+    return ([str(layer.digest) for layer in self.filesystem_layers if not layer.urls] +
             [str(self.config.digest)])
 
   def get_manifest_labels(self, content_retriever):
     return self._get_built_config(content_retriever).labels
 
+  def get_layers(self, content_retriever):
+    """ Returns the layers of this manifest, from base to leaf or None if this kind of manifest
+        does not support layers. """
+    for image_layer in self._manifest_image_layers(content_retriever):
+      is_remote = image_layer.blob_layer.is_remote if image_layer.blob_layer else False
+      urls = image_layer.blob_layer.urls if image_layer.blob_layer else None
+      yield ManifestImageLayer(layer_id=image_layer.v1_id,
+                               compressed_size=image_layer.compressed_size,
+                               is_remote=is_remote,
+                               urls=urls,
+                               command=image_layer.history.command,
+                               blob_digest=image_layer.blob_digest,
+                               created_datetime=image_layer.history.created_datetime,
+                               internal_layer=image_layer)
+
   @property
   def bytes(self):
     return self._payload
@@ -214,12 +231,10 @@ class DockerSchema2Manifest(ManifestInterface):
     return None
 
   def _manifest_image_layers(self, content_retriever):
-    assert not self.has_remote_layer
-
     # Retrieve the configuration for the manifest.
     config = self._get_built_config(content_retriever)
     history = list(config.history)
-    if len(history) < len(self.layers):
+    if len(history) < len(self.filesystem_layers):
       raise MalformedSchema2Manifest('Found less history than layer blobs')
 
     digest_history = hashlib.sha256()
@@ -228,11 +243,11 @@ class DockerSchema2Manifest(ManifestInterface):
     blob_index = 0
 
     for history_index, history_entry in enumerate(history):
-      if not history_entry.is_empty and blob_index >= len(self.layers):
+      if not history_entry.is_empty and blob_index >= len(self.filesystem_layers):
         raise MalformedSchema2Manifest('Missing history entry #%s' % blob_index)
 
       v1_layer_parent_id = v1_layer_id
-      blob_layer = None if history_entry.is_empty else self.layers[blob_index]
+      blob_layer = None if history_entry.is_empty else self.filesystem_layers[blob_index]
       blob_digest = EMPTY_LAYER_BLOB_DIGEST if blob_layer is None else str(blob_layer.digest)
       compressed_size = EMPTY_LAYER_SIZE if blob_layer is None else blob_layer.compressed_size
 
@@ -246,12 +261,12 @@ class DockerSchema2Manifest(ManifestInterface):
       digest_history.update("||")
 
       v1_layer_id = digest_history.hexdigest()
-      yield ManifestImageLayer(history=history_entry,
-                               blob_layer=blob_layer,
-                               blob_digest=blob_digest,
-                               v1_id=v1_layer_id,
-                               v1_parent_id=v1_layer_parent_id,
-                               compressed_size=compressed_size)
+      yield DockerV2ManifestImageLayer(history=history_entry,
+                                       blob_layer=blob_layer,
+                                       blob_digest=blob_digest,
+                                       v1_id=v1_layer_id,
+                                       v1_parent_id=v1_layer_parent_id,
+                                       compressed_size=compressed_size)
 
       if not history_entry.is_empty:
         blob_index += 1
@@ -335,7 +350,7 @@ class DockerSchema2Manifest(ManifestInterface):
     self._cached_built_config = DockerSchema2Config(config_bytes)
     return self._cached_built_config
 
-  def _generate_layers(self):
+  def _generate_filesystem_layers(self):
     for index, layer in enumerate(self._parsed[DOCKER_SCHEMA2_MANIFEST_LAYERS_KEY]):
       content_type = layer[DOCKER_SCHEMA2_MANIFEST_MEDIATYPE_KEY]
       is_remote = content_type == DOCKER_SCHEMA2_REMOTE_LAYER_CONTENT_TYPE
@@ -359,7 +374,7 @@ class DockerSchema2ManifestBuilder(object):
   """
   def __init__(self):
     self.config = None
-    self.layers = []
+    self.filesystem_layers = []
 
   def set_config(self, schema2_config):
     """ Sets the configuration for the manifest being built. """
@@ -370,16 +385,16 @@ class DockerSchema2ManifestBuilder(object):
     self.config = DockerV2ManifestConfig(size=config_size, digest=config_digest)
 
   def add_layer(self, digest, size, urls=None):
-    """ Adds a layer to the manifest. """
-    self.layers.append(DockerV2ManifestLayer(index=len(self.layers),
-                                             digest=digest,
-                                             compressed_size=size,
-                                             urls=urls,
-                                             is_remote=bool(urls)))
+    """ Adds a filesystem layer to the manifest. """
+    self.filesystem_layers.append(DockerV2ManifestLayer(index=len(self.filesystem_layers),
+                                                        digest=digest,
+                                                        compressed_size=size,
+                                                        urls=urls,
+                                                        is_remote=bool(urls)))
 
   def build(self):
     """ Builds and returns the DockerSchema2Manifest. """
-    assert self.layers
+    assert self.filesystem_layers
     assert self.config
 
     def _build_layer(layer):
@@ -410,7 +425,7 @@ class DockerSchema2ManifestBuilder(object):
 
       # Layers
       DOCKER_SCHEMA2_MANIFEST_LAYERS_KEY: [
-        _build_layer(layer) for layer in self.layers
+        _build_layer(layer) for layer in self.filesystem_layers
       ],
     }
     return DockerSchema2Manifest(json.dumps(manifest_dict, indent=3))

image/docker/schema2/test/test_list.py

@@ -79,7 +79,7 @@ def test_valid_manifestlist():
   assert manifestlist.media_type == 'application/vnd.docker.distribution.manifest.list.v2+json'
   assert manifestlist.bytes == MANIFESTLIST_BYTES
   assert manifestlist.manifest_dict == json.loads(MANIFESTLIST_BYTES)
-  assert manifestlist.layers is None
+  assert manifestlist.get_layers(retriever) is None
   assert not manifestlist.blob_digests
 
   for index, manifest in enumerate(manifestlist.manifests(retriever)):

image/docker/schema2/test/test_manifest.py

@@ -98,20 +98,81 @@ def test_valid_manifest():
   assert not manifest.has_remote_layer
   assert manifest.has_legacy_image
 
-  assert len(manifest.layers) == 4
-  assert manifest.layers[0].compressed_size == 1234
-  assert str(manifest.layers[0].digest) == 'sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736'
-  assert not manifest.layers[0].is_remote
+  retriever = ContentRetrieverForTesting.for_config({
+    "config": {
+      "Labels": {},
+    },
+    "rootfs": {"type": "layers", "diff_ids": []},
+    "history": [
+      {
+        "created": "2018-04-03T18:37:09.284840891Z",
+        "created_by": "foo"
+      },
+      {
+        "created": "2018-04-12T18:37:09.284840891Z",
+        "created_by": "bar"
+      },
+      {
+        "created": "2018-04-03T18:37:09.284840891Z",
+        "created_by": "foo"
+      },
+      {
+        "created": "2018-04-12T18:37:09.284840891Z",
+        "created_by": "bar"
+      },
+    ],
+  }, 'sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7', 1885)
 
-  assert manifest.leaf_layer == manifest.layers[3]
-  assert not manifest.leaf_layer.is_remote
-  assert manifest.leaf_layer.compressed_size == 73109
+  assert len(manifest.filesystem_layers) == 4
+  assert manifest.filesystem_layers[0].compressed_size == 1234
+  assert str(manifest.filesystem_layers[0].digest) == 'sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736'
+  assert not manifest.filesystem_layers[0].is_remote
+
+  assert manifest.leaf_filesystem_layer == manifest.filesystem_layers[3]
+  assert not manifest.leaf_filesystem_layer.is_remote
+  assert manifest.leaf_filesystem_layer.compressed_size == 73109
 
   blob_digests = list(manifest.blob_digests)
-  expected = [str(layer.digest) for layer in manifest.layers] + [manifest.config.digest]
+  expected = [str(layer.digest) for layer in manifest.filesystem_layers] + [manifest.config.digest]
   assert blob_digests == expected
   assert list(manifest.local_blob_digests) == expected
 
+  manifest_image_layers = list(manifest.get_layers(retriever))
+  assert len(manifest_image_layers) == len(list(manifest.filesystem_layers))
+  for index in range(0, 4):
+    assert manifest_image_layers[index].blob_digest == str(manifest.filesystem_layers[index].digest)
+
+
+def test_valid_remote_manifest():
+  manifest = DockerSchema2Manifest(REMOTE_MANIFEST_BYTES)
+  assert manifest.config.size == 1885
+  assert str(manifest.config.digest) == 'sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7'
+  assert manifest.media_type == "application/vnd.docker.distribution.manifest.v2+json"
+  assert manifest.has_remote_layer
+
+  assert len(manifest.filesystem_layers) == 4
+  assert manifest.filesystem_layers[0].compressed_size == 1234
+  assert str(manifest.filesystem_layers[0].digest) == 'sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736'
+  assert manifest.filesystem_layers[0].is_remote
+  assert manifest.filesystem_layers[0].urls == ['http://some/url']
+
+  assert manifest.leaf_filesystem_layer == manifest.filesystem_layers[3]
+  assert not manifest.leaf_filesystem_layer.is_remote
+  assert manifest.leaf_filesystem_layer.compressed_size == 73109
+
+  expected = set([str(layer.digest) for layer in manifest.filesystem_layers] + 
+                 [manifest.config.digest])
+
+  blob_digests = set(manifest.blob_digests)
+  local_digests = set(manifest.local_blob_digests)
+
+  assert blob_digests == expected
+  assert local_digests == (expected - {manifest.filesystem_layers[0].digest})
+
+  assert manifest.has_remote_layer
+  assert manifest.get_leaf_layer_v1_image_id(None) is None
+  assert manifest.get_legacy_image_ids(None) is None
+
   retriever = ContentRetrieverForTesting.for_config({
     "config": {
       "Labels": {},
@@ -137,40 +198,10 @@ def test_valid_manifest():
     ],
   }, 'sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7', 1885)
 
-  manifest_image_layers = list(manifest._manifest_image_layers(retriever))
-  assert len(manifest_image_layers) == len(list(manifest.layers))
+  manifest_image_layers = list(manifest.get_layers(retriever))
+  assert len(manifest_image_layers) == len(list(manifest.filesystem_layers))
   for index in range(0, 4):
-    assert manifest_image_layers[index].blob_digest == str(manifest.layers[index].digest)
-
-
-def test_valid_remote_manifest():
-  manifest = DockerSchema2Manifest(REMOTE_MANIFEST_BYTES)
-  assert manifest.config.size == 1885
-  assert str(manifest.config.digest) == 'sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7'
-  assert manifest.media_type == "application/vnd.docker.distribution.manifest.v2+json"
-  assert manifest.has_remote_layer
-
-  assert len(manifest.layers) == 4
-  assert manifest.layers[0].compressed_size == 1234
-  assert str(manifest.layers[0].digest) == 'sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736'
-  assert manifest.layers[0].is_remote
-  assert manifest.layers[0].urls == ['http://some/url']
-
-  assert manifest.leaf_layer == manifest.layers[3]
-  assert not manifest.leaf_layer.is_remote
-  assert manifest.leaf_layer.compressed_size == 73109
-
-  expected = set([str(layer.digest) for layer in manifest.layers] + [manifest.config.digest])
-
-  blob_digests = set(manifest.blob_digests)
-  local_digests = set(manifest.local_blob_digests)
-
-  assert blob_digests == expected
-  assert local_digests == (expected - {manifest.layers[0].digest})
-
-  assert manifest.has_remote_layer
-  assert manifest.get_leaf_layer_v1_image_id(None) is None
-  assert manifest.get_legacy_image_ids(None) is None
+    assert manifest_image_layers[index].blob_digest == str(manifest.filesystem_layers[index].digest)
 
 
 def test_schema2_builder():
@@ -179,11 +210,11 @@ def test_schema2_builder():
   builder = DockerSchema2ManifestBuilder()
   builder.set_config_digest(manifest.config.digest, manifest.config.size)
 
-  for layer in manifest.layers:
+  for layer in manifest.filesystem_layers:
     builder.add_layer(layer.digest, layer.compressed_size, urls=layer.urls)
 
   built = builder.build()
-  assert built.layers == manifest.layers
+  assert built.filesystem_layers == manifest.filesystem_layers
   assert built.config == manifest.config
 
 

image/docker/test/test_schema1.py

@@ -91,6 +91,13 @@ def test_valid_manifest():
   assert unsigned.blob_digests == manifest.blob_digests
   assert unsigned.digest != manifest.digest
 
+  image_layers = list(manifest.get_layers(None))
+  assert len(image_layers) == 2
+  for index in range(0, 2):
+    assert image_layers[index].layer_id == manifest.layers[index].v1_metadata.image_id
+    assert image_layers[index].blob_digest == manifest.layers[index].digest
+    assert image_layers[index].command == manifest.layers[index].v1_metadata.command
+
 
 def test_validate_manifest():
   test_dir = os.path.dirname(os.path.abspath(__file__))

image/docker/types.py

@@ -0,0 +1,6 @@
+from collections import namedtuple
+
+ManifestImageLayer = namedtuple('ManifestImageLayer', ['layer_id', 'compressed_size',
+                                                       'is_remote', 'urls', 'command',
+                                                       'blob_digest', 'created_datetime',
+                                                       'internal_layer'])