From 0bfbccdd445355c9127bc1482a6a1f80d1f7a654 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <joseph.schorr@coreos.com>
Date: Thu, 7 Feb 2019 13:36:47 -0500
Subject: [PATCH] Add a handler for broken tags in the tag backfill system

This will generate a tag pointing to an empty manifest; the tag will be broken, but as it is *already* broken, at least the backfill can complete
---
 data/registry_model/registry_pre_oci_model.py |  3 ++
 data/registry_model/shared.py                 |  6 +++-
 workers/tagbackfillworker.py                  | 20 ++++++++-----
 workers/test/test_tagbackfillworker.py        | 30 ++++++++++++++++++-
 4 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/data/registry_model/registry_pre_oci_model.py b/data/registry_model/registry_pre_oci_model.py
index 1a5094ae6..62cd1d31e 100644
--- a/data/registry_model/registry_pre_oci_model.py
+++ b/data/registry_model/registry_pre_oci_model.py
@@ -478,6 +478,9 @@ class PreOCIModel(SharedModel, RegistryDataInterface):
 
     # Write the manifest to the DB.
     manifest = self._build_manifest_for_legacy_image(tag_obj.name, tag_obj.image)
+    if manifest is None:
+      return None
+
     blob_query = model.storage.lookup_repo_storages_by_content_checksum(repo,
                                                                         manifest.checksums)
 
diff --git a/data/registry_model/shared.py b/data/registry_model/shared.py
index 16c87b96c..091061526 100644
--- a/data/registry_model/shared.py
+++ b/data/registry_model/shared.py
@@ -418,7 +418,11 @@ class SharedModel:
     repo_name = repo.name
 
     # Find the v1 metadata for this image and its parents.
-    parents = model.image.get_parent_images(namespace_name, repo_name, legacy_image_row)
+    try:
+      parents = model.image.get_parent_images(namespace_name, repo_name, legacy_image_row)
+    except model.DataModelException:
+      logger.exception('Could not load parent images for legacy image %s', legacy_image_row.id)
+      return None
 
     # If the manifest is being generated under the library namespace, then we make its namespace
     # empty.
diff --git a/workers/tagbackfillworker.py b/workers/tagbackfillworker.py
index 9e6671183..827f5f9de 100644
--- a/workers/tagbackfillworker.py
+++ b/workers/tagbackfillworker.py
@@ -225,15 +225,19 @@ def _get_manifest_id(repositorytag):
     manifest_datatype = pre_oci_model.get_manifest_for_tag(repository_tag_datatype,
                                                            backfill_if_necessary=True)
     if manifest_datatype is None:
-      logger.error('Missing manifest for tag `%s`', repositorytag.id)
-      return None
+      logger.error('Could not load or backfill manifest for tag `%s`', repositorytag.id)
 
-  # Retrieve the new-style Manifest for the TagManifest, if any.
-  try:
-    tag_manifest = TagManifest.get(id=manifest_datatype._db_id)
-  except TagManifest.DoesNotExist:
-    logger.exception('Could not find tag manifest')
-    return None
+      # Create a broken manifest for the tag.
+      tag_manifest = TagManifest.create(tag=repositorytag,
+                                        digest='BROKEN-%s' % repositorytag.id,
+                                        json_data='{}')
+    else:
+      # Retrieve the new-style Manifest for the TagManifest, if any.
+      try:
+        tag_manifest = TagManifest.get(id=manifest_datatype._db_id)
+      except TagManifest.DoesNotExist:
+        logger.exception('Could not find tag manifest')
+        return None
 
   try:
     found = TagManifestToManifest.get(tag_manifest=tag_manifest).manifest
diff --git a/workers/test/test_tagbackfillworker.py b/workers/test/test_tagbackfillworker.py
index 430cba3a8..f9615b064 100644
--- a/workers/test/test_tagbackfillworker.py
+++ b/workers/test/test_tagbackfillworker.py
@@ -2,7 +2,8 @@ from app import docker_v2_signing_key
 from data import model
 from data.database import (TagManifestLabelMap, TagManifestToManifest, Manifest, ManifestBlob,
                            ManifestLegacyImage, ManifestLabel, TagManifest, RepositoryTag, Image,
-                           TagManifestLabel, Tag, TagToRepositoryTag, Repository)
+                           TagManifestLabel, Tag, TagToRepositoryTag, Repository,
+                           ImageStorage)
 from image.docker.schema1 import DockerSchema1ManifestBuilder
 from workers.tagbackfillworker import backfill_tag, _backfill_manifest
 
@@ -251,3 +252,30 @@ def test_manifestbackfillworker_repeat_digest(clear_rows, initialized_db):
   map_row2 = TagManifestToManifest.get(tag_manifest=manifest_2)
 
   assert map_row1.manifest == map_row2.manifest
+
+
+def test_manifest_backfill_broken_tag(clear_rows, initialized_db):
+  """ Tests backfilling a broken tag. """
+  # Delete existing tag manifest so we can reuse the tag.
+  TagManifestLabel.delete().execute()
+  TagManifest.delete().execute()
+
+  # Create a tag with an image referenced missing parent images.
+  repo = model.repository.get_repository('devtable', 'gargantuan')
+  broken_image = Image.create(docker_image_id='foo', repository=repo, ancestors='/348723847234/',
+                              storage=ImageStorage.get())
+  broken_image_tag = RepositoryTag.create(repository=repo, image=broken_image, name='broken')
+
+  # Backfill the tag.
+  assert backfill_tag(broken_image_tag)
+
+  # Ensure we backfilled, even though we reference a broken manifest.
+  tag_manifest = TagManifest.get(tag=broken_image_tag)
+
+  map_row = TagManifestToManifest.get(tag_manifest=tag_manifest)
+  manifest = map_row.manifest
+  assert manifest.manifest_bytes == tag_manifest.json_data
+
+  tag = TagToRepositoryTag.get(repository_tag=broken_image_tag).tag
+  assert tag.name == 'broken'
+  assert tag.manifest == manifest