From 63f904331274f9745db6f0bb031ff0343e1fdf08 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <joseph.schorr@coreos.com>
Date: Wed, 28 Nov 2018 12:56:16 +0200
Subject: [PATCH] Code review small fixes

---
 endpoints/v2/manifest.py         | 24 ++++++++++++------------
 image/docker/interfaces.py       |  4 ++--
 image/docker/schema1.py          |  3 ++-
 image/docker/schema2/manifest.py |  7 ++++---
 4 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/endpoints/v2/manifest.py b/endpoints/v2/manifest.py
index bba45e062..d4d1533c8 100644
--- a/endpoints/v2/manifest.py
+++ b/endpoints/v2/manifest.py
@@ -62,9 +62,9 @@ def fetch_manifest_by_tagname(namespace_name, repo_name, manifest_ref):
     logger.exception('Got exception when trying to parse manifest `%s`', manifest_ref)
     raise ManifestInvalid()
 
-  manifest = _rewrite_to_schema1_if_necessary(namespace_name, repo_name, manifest_ref, manifest,
-                                              parsed)
-  if manifest is None:
+  supported = _rewrite_to_schema1_if_necessary(namespace_name, repo_name, manifest_ref, manifest,
+                                               parsed)
+  if supported is None:
     raise ManifestUnknown()
 
   track_and_log('pull_repo', repository_ref, analytics_name='pull_repo_100x', analytics_sample=0.01,
@@ -72,11 +72,11 @@ def fetch_manifest_by_tagname(namespace_name, repo_name, manifest_ref):
   metric_queue.repository_pull.Inc(labelvalues=[namespace_name, repo_name, 'v2', True])
 
   return Response(
-    manifest.bytes,
+    supported.bytes,
     status=200,
     headers={
-      'Content-Type': manifest.media_type,
-      'Docker-Content-Digest': manifest.digest,
+      'Content-Type': supported.media_type,
+      'Docker-Content-Digest': supported.digest,
     },
   )
 
@@ -101,17 +101,17 @@ def fetch_manifest_by_digest(namespace_name, repo_name, manifest_ref):
     logger.exception('Got exception when trying to parse manifest `%s`', manifest_ref)
     raise ManifestInvalid()
 
-  manifest = _rewrite_to_schema1_if_necessary(namespace_name, repo_name, '$digest', manifest,
-                                              parsed)
-  if manifest is None:
+  supported = _rewrite_to_schema1_if_necessary(namespace_name, repo_name, '$digest', manifest,
+                                               parsed)
+  if supported is None:
     raise ManifestUnknown()
 
   track_and_log('pull_repo', repository_ref, manifest_digest=manifest_ref)
   metric_queue.repository_pull.Inc(labelvalues=[namespace_name, repo_name, 'v2', True])
 
-  return Response(manifest.bytes, status=200, headers={
-    'Content-Type': manifest.media_type,
-    'Docker-Content-Digest': manifest.digest,
+  return Response(supported.bytes, status=200, headers={
+    'Content-Type': supported.media_type,
+    'Docker-Content-Digest': supported.digest,
   })
 
 
diff --git a/image/docker/interfaces.py b/image/docker/interfaces.py
index 2320089dc..aef185f8f 100644
--- a/image/docker/interfaces.py
+++ b/image/docker/interfaces.py
@@ -60,14 +60,14 @@ class ManifestInterface(object):
   def blob_digests(self):
     """ Returns an iterator over all the blob digests referenced by this manifest,
         from base to leaf. The blob digests are strings with prefixes. For manifests that reference
-        config as a blob, the blob will be included here.
+        config as a blob, the blob will be included here as the last entry.
     """
 
   @abstractproperty
   def local_blob_digests(self):
     """ Returns an iterator over all the *non-remote* blob digests referenced by this manifest,
         from base to leaf. The blob digests are strings with prefixes. For manifests that reference
-        config as a blob, the blob will be included here.
+        config as a blob, the blob will be included here as the last entry.
     """
 
   @abstractmethod
diff --git a/image/docker/schema1.py b/image/docker/schema1.py
index e9673916a..874873cbb 100644
--- a/image/docker/schema1.py
+++ b/image/docker/schema1.py
@@ -172,8 +172,9 @@ class DockerSchema1Manifest(ManifestInterface):
       raise MalformedSchema1Manifest('manifest data does not match schema: %s' % ve)
 
     self._signatures = self._parsed.get(DOCKER_SCHEMA1_SIGNATURES_KEY)
+    self._architecture = self._parsed.get(DOCKER_SCHEMA1_ARCH_KEY)
+
     self._tag = self._parsed[DOCKER_SCHEMA1_REPO_TAG_KEY]
-    self._architecture = self._parsed[DOCKER_SCHEMA1_ARCH_KEY]
 
     repo_name = self._parsed[DOCKER_SCHEMA1_REPO_NAME_KEY]
     repo_name_tuple = repo_name.split('/')
diff --git a/image/docker/schema2/manifest.py b/image/docker/schema2/manifest.py
index 62fbd6b83..6f68719e7 100644
--- a/image/docker/schema2/manifest.py
+++ b/image/docker/schema2/manifest.py
@@ -291,7 +291,7 @@ class DockerSchema2Manifest(ManifestInterface):
     if self.has_remote_layer:
       return None
 
-    return list(self._manifest_image_layers(content_retriever))[-1].v1_id
+    return self.get_legacy_image_ids(content_retriever)[-1].v1_id
 
   def get_legacy_image_ids(self, content_retriever):
     if self.has_remote_layer:
@@ -344,8 +344,9 @@ class DockerSchema2Manifest(ManifestInterface):
       raise MalformedSchema2Manifest('Could not load config blob for manifest')
 
     if len(config_bytes) != self.config.size:
-      raise MalformedSchema2Manifest('Size of config does not match that retrieved: %s vs %s',
-                                     len(config_bytes), self.config.size)
+      msg = 'Size of config does not match that retrieved: %s vs %s' % (len(config_bytes),
+                                                                        self.config.size)
+      raise MalformedSchema2Manifest(msg)
 
     self._cached_built_config = DockerSchema2Config(config_bytes)
     return self._cached_built_config