From 4e1ff90cb2117cbc733583c894b7d483cfaab7dd Mon Sep 17 00:00:00 2001
From: Joseph Schorr <joseph.schorr@coreos.com>
Date: Mon, 26 Nov 2018 17:58:48 +0200
Subject: [PATCH] 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
---
 data/model/oci/manifest.py                    |   6 -
 data/registry_model/interface.py              |   6 +-
 data/registry_model/registry_oci_model.py     |   6 +-
 data/registry_model/registry_pre_oci_model.py |   6 +-
 data/registry_model/shared.py                 |  16 ++-
 data/registry_model/test/test_interface.py    |  14 +--
 endpoints/verbs/__init__.py                   |   2 +-
 image/docker/interfaces.py                    |  12 +-
 image/docker/schema1.py                       |  24 +++-
 image/docker/schema2/list.py                  |   5 +-
 image/docker/schema2/manifest.py              |  91 ++++++++------
 image/docker/schema2/test/test_list.py        |   2 +-
 image/docker/schema2/test/test_manifest.py    | 117 +++++++++++-------
 image/docker/test/test_schema1.py             |   7 ++
 image/docker/types.py                         |   6 +
 test/registry/protocol_v2.py                  |   9 +-
 workers/manifestbackfillworker.py             |   5 +-
 17 files changed, 210 insertions(+), 124 deletions(-)
 create mode 100644 image/docker/types.py

diff --git a/data/model/oci/manifest.py b/data/model/oci/manifest.py
index 3181ef5e7..cf1e573aa 100644
--- a/data/model/oci/manifest.py
+++ b/data/model/oci/manifest.py
@@ -96,12 +96,6 @@ def _create_manifest(repository_id, manifest_interface_instance, storage):
         logger.exception('Could not load manifest labels for child manifest')
         return None
 
-      # NOTE: Content type restrictions in manifest lists ensure that the child manifests
-      # must be image manifests, as opposed to lists themselves. We put this check here to
-      # be extra careful in ensuring we don't create empty manifests. If this reality changes,
-      # should remove this check.
-      assert list(child_manifest.layers)
-
       # Get/create the child manifest in the database.
       child_manifest_info = get_or_create_manifest(repository_id, child_manifest, storage)
       if child_manifest_info is None:
diff --git a/data/registry_model/interface.py b/data/registry_model/interface.py
index 3e6ebe6c0..5fcf8f2d7 100644
--- a/data/registry_model/interface.py
+++ b/data/registry_model/interface.py
@@ -195,10 +195,12 @@ class RegistryDataInterface(object):
     """ Returns the set of local blobs for the given manifest or None if none. """
 
   @abstractmethod
-  def list_parsed_manifest_layers(self, repository_ref, parsed_manifest, include_placements=False):
+  def list_parsed_manifest_layers(self, repository_ref, parsed_manifest, storage,
+                                  include_placements=False):
     """ Returns an *ordered list* of the layers found in the parsed manifest, starting at the base
         and working towards the leaf, including the associated Blob and its placements
-        (if specified).
+        (if specified). The layer information in `layer_info` will be of type
+        `image.docker.types.ManifestImageLayer`.
     """
 
   @abstractmethod
diff --git a/data/registry_model/registry_oci_model.py b/data/registry_model/registry_oci_model.py
index 94916bf42..a4882a10c 100644
--- a/data/registry_model/registry_oci_model.py
+++ b/data/registry_model/registry_oci_model.py
@@ -523,12 +523,14 @@ class OCIModel(SharedModel, RegistryDataInterface):
                                   storage_path=model.storage.get_layer_path(image_storage),
                                   placements=placements)
 
-  def list_parsed_manifest_layers(self, repository_ref, parsed_manifest, include_placements=False):
+  def list_parsed_manifest_layers(self, repository_ref, parsed_manifest, storage,
+                                  include_placements=False):
     """ Returns an *ordered list* of the layers found in the parsed manifest, starting at the base
         and working towards the leaf, including the associated Blob and its placements
         (if specified).
     """
-    return self._list_manifest_layers(repository_ref._db_id, parsed_manifest, include_placements,
+    return self._list_manifest_layers(repository_ref._db_id, parsed_manifest, storage,
+                                      include_placements=include_placements,
                                       by_manifest=True)
 
   def get_manifest_local_blobs(self, manifest, include_placements=False):
diff --git a/data/registry_model/registry_pre_oci_model.py b/data/registry_model/registry_pre_oci_model.py
index 0cb082318..907d34ad5 100644
--- a/data/registry_model/registry_pre_oci_model.py
+++ b/data/registry_model/registry_pre_oci_model.py
@@ -577,12 +577,14 @@ class PreOCIModel(SharedModel, RegistryDataInterface):
                                   storage_path=model.storage.get_layer_path(image_storage),
                                   placements=placements)
 
-  def list_parsed_manifest_layers(self, repository_ref, parsed_manifest, include_placements=False):
+  def list_parsed_manifest_layers(self, repository_ref, parsed_manifest, storage,
+                                  include_placements=False):
     """ Returns an *ordered list* of the layers found in the parsed manifest, starting at the base
         and working towards the leaf, including the associated Blob and its placements
         (if specified).
     """
-    return self._list_manifest_layers(repository_ref._db_id, parsed_manifest, include_placements)
+    return self._list_manifest_layers(repository_ref._db_id, parsed_manifest, storage,
+                                      include_placements=include_placements)
 
   def get_manifest_local_blobs(self, manifest, include_placements=False):
     """ Returns the set of local blobs for the given manifest or None if none. """
diff --git a/data/registry_model/shared.py b/data/registry_model/shared.py
index b4438bda7..015b8df6d 100644
--- a/data/registry_model/shared.py
+++ b/data/registry_model/shared.py
@@ -7,6 +7,7 @@ from collections import defaultdict
 from data import database
 from data import model
 from data.cache import cache_key
+from data.model.oci.retriever import RepositoryContentRetriever
 from data.registry_model.datatype import FromDictionaryException
 from data.registry_model.datatypes import (RepositoryReference, Blob, TorrentInfo, BlobUpload,
                                            LegacyImage, ManifestLayer, DerivedImage)
@@ -316,7 +317,8 @@ class SharedModel:
 
     return blobs
 
-  def _list_manifest_layers(self, repo_id, parsed, include_placements=False, by_manifest=False):
+  def _list_manifest_layers(self, repo_id, parsed, storage, include_placements=False,
+                            by_manifest=False):
     """ Returns an *ordered list* of the layers found in the manifest, starting at the base and
         working towards the leaf, including the associated Blob and its placements (if specified).
         Returns None if the manifest could not be parsed and validated.
@@ -328,15 +330,21 @@ class SharedModel:
                                                                           by_manifest=by_manifest)
       storage_map = {blob.content_checksum: blob for blob in blob_query}
 
+    retriever = RepositoryContentRetriever(repo_id, storage)
+    layers = parsed.get_layers(retriever)
+    if layers is None:
+      logger.error('Could not load layers for manifest `%s`', parsed.digest)
+      return None
+
     manifest_layers = []
-    for layer in parsed.layers:
+    for layer in layers:
       if layer.is_remote:
         manifest_layers.append(ManifestLayer(layer, None))
         continue
 
-      digest_str = str(layer.digest)
+      digest_str = str(layer.blob_digest)
       if digest_str not in storage_map:
-        logger.error('Missing digest `%s` for manifest `%s`', layer.digest, parsed.digest)
+        logger.error('Missing digest `%s` for manifest `%s`', layer.blob_digest, parsed.digest)
         return None
 
       image_storage = storage_map[digest_str]
diff --git a/data/registry_model/test/test_interface.py b/data/registry_model/test/test_interface.py
index 88cc65367..eb291865f 100644
--- a/data/registry_model/test/test_interface.py
+++ b/data/registry_model/test/test_interface.py
@@ -21,6 +21,7 @@ from data.registry_model.registry_pre_oci_model import PreOCIModel
 from data.registry_model.registry_oci_model import OCIModel
 from data.registry_model.datatypes import RepositoryReference
 from data.registry_model.blobuploader import upload_blob, BlobUploadSettings
+from image.docker.types import ManifestImageLayer
 from image.docker.schema1 import DockerSchema1ManifestBuilder
 from image.docker.schema2.manifest import DockerSchema2ManifestBuilder
 
@@ -474,19 +475,14 @@ def test_layers_and_blobs(repo_namespace, repo_name, registry_model):
     parsed = manifest.get_parsed_manifest()
     assert parsed
 
-    layers = registry_model.list_parsed_manifest_layers(repository_ref, parsed)
+    layers = registry_model.list_parsed_manifest_layers(repository_ref, parsed, storage)
     assert layers
 
-    layers = registry_model.list_parsed_manifest_layers(repository_ref, parsed,
+    layers = registry_model.list_parsed_manifest_layers(repository_ref, parsed, storage,
                                                         include_placements=True)
     assert layers
 
-    parsed_layers = list(manifest.get_parsed_manifest().layers)
-    assert len(layers) == len(parsed_layers)
-
     for index, manifest_layer in enumerate(layers):
-      assert manifest_layer.layer_info == parsed_layers[index]
-      assert manifest_layer.blob.digest == str(parsed_layers[index].digest)
       assert manifest_layer.blob.storage_path
       assert manifest_layer.blob.placements
 
@@ -494,6 +490,7 @@ def test_layers_and_blobs(repo_namespace, repo_name, registry_model):
       assert repo_blob.digest == manifest_layer.blob.digest
 
       assert manifest_layer.estimated_size(1) is not None
+      assert isinstance(manifest_layer.layer_info, ManifestImageLayer)
 
     blobs = registry_model.get_manifest_local_blobs(manifest, include_placements=True)
     assert {b.digest for b in blobs} == set(parsed.local_blob_digests)
@@ -532,7 +529,8 @@ def test_manifest_remote_layers(oci_model):
   assert created_manifest
 
   layers = oci_model.list_parsed_manifest_layers(repository_ref,
-                                                 created_manifest.get_parsed_manifest())
+                                                 created_manifest.get_parsed_manifest(),
+                                                 storage)
   assert len(layers) == 1
   assert layers[0].layer_info.is_remote
   assert layers[0].layer_info.urls == ['http://hello/world']
diff --git a/endpoints/verbs/__init__.py b/endpoints/verbs/__init__.py
index b7d8330c4..cb1266b76 100644
--- a/endpoints/verbs/__init__.py
+++ b/endpoints/verbs/__init__.py
@@ -51,7 +51,7 @@ def _open_stream(formatter, tag, schema1_manifest, derived_image_id, handlers, r
   # For performance reasons, we load the full image list here, cache it, then disconnect from
   # the database.
   with database.UseThenDisconnect(app.config):
-    layers = registry_model.list_parsed_manifest_layers(tag.repository, schema1_manifest,
+    layers = registry_model.list_parsed_manifest_layers(tag.repository, schema1_manifest, storage,
                                                         include_placements=True)
 
   def image_stream_getter(store, blob):
diff --git a/image/docker/interfaces.py b/image/docker/interfaces.py
index 909ef1940..2320089dc 100644
--- a/image/docker/interfaces.py
+++ b/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
diff --git a/image/docker/schema1.py b/image/docker/schema1.py
index 4523df0d2..e9673916a 100644
--- a/image/docker/schema1.py
+++ b/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):
diff --git a/image/docker/schema2/list.py b/image/docker/schema2/list.py
index f02c55072..fe64341fa 100644
--- a/image/docker/schema2/list.py
+++ b/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
diff --git a/image/docker/schema2/manifest.py b/image/docker/schema2/manifest.py
index b30ea6535..62fbd6b83 100644
--- a/image/docker/schema2/manifest.py
+++ b/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))
diff --git a/image/docker/schema2/test/test_list.py b/image/docker/schema2/test/test_list.py
index 0960e2b44..d70f6b75f 100644
--- a/image/docker/schema2/test/test_list.py
+++ b/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)):
diff --git a/image/docker/schema2/test/test_manifest.py b/image/docker/schema2/test/test_manifest.py
index 8f6327a83..67d7678d2 100644
--- a/image/docker/schema2/test/test_manifest.py
+++ b/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
 
 
diff --git a/image/docker/test/test_schema1.py b/image/docker/test/test_schema1.py
index 414da365b..950de0049 100644
--- a/image/docker/test/test_schema1.py
+++ b/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__))
diff --git a/image/docker/types.py b/image/docker/types.py
new file mode 100644
index 000000000..088ff9887
--- /dev/null
+++ b/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'])
diff --git a/test/registry/protocol_v2.py b/test/registry/protocol_v2.py
index a79dc4101..4f9c53277 100644
--- a/test/registry/protocol_v2.py
+++ b/test/registry/protocol_v2.py
@@ -543,20 +543,21 @@ class V2Protocol(RegistryProtocol):
       if manifest.schema_version == 1:
         image_ids[tag_name] = manifest.leaf_layer_v1_image_id
 
-      # Verify the layers.
+      # Verify the blobs.
       layer_index = 0
       empty_count = 0
+      blob_digests = list(manifest.blob_digests)
       for image in images:
         if manifest.schema_version == 2 and image.is_empty:
           empty_count += 1
           continue
 
         # If the layer is remote, then we expect the blob to *not* exist in the system.
-        layer = manifest.layers[layer_index]
+        blob_digest = blob_digests[layer_index]
         expected_status = 404 if image.urls else 200
         result = self.conduct(session, 'GET',
                               '/v2/%s/blobs/%s' % (self.repo_name(namespace, repo_name),
-                                                   layer.digest),
+                                                   blob_digest),
                               expected_status=expected_status,
                               headers=headers)
 
@@ -565,7 +566,7 @@ class V2Protocol(RegistryProtocol):
 
         layer_index += 1
 
-      assert (len(manifest.layers) + empty_count) == len(images)
+      assert (len(blob_digests) + empty_count) >= len(images) # Schema 2 has 1 extra for config
 
     return PullResult(manifests=manifests, image_ids=image_ids)
 
diff --git a/workers/manifestbackfillworker.py b/workers/manifestbackfillworker.py
index b92e58a72..daf06076d 100644
--- a/workers/manifestbackfillworker.py
+++ b/workers/manifestbackfillworker.py
@@ -47,9 +47,8 @@ class BrokenManifest(ManifestInterface):
   def bytes(self):
     return self._payload
 
-  @property
-  def layers(self):
-    return []
+  def get_layers(self, content_retriever):
+    return None
 
   def get_legacy_image_ids(self, cr):
     return []