Raise a 409 if we try to insert a tag twice at the same time

Also fixes handling of labels for existing manifests

Fixes #1775

data/model/__init__.py

@@ -97,6 +97,10 @@ class ServiceNameInvalid(DataModelException):
   pass
 
 
+class TagAlreadyCreatedException(DataModelException):
+  pass
+
+
 class TooManyLoginAttemptsException(Exception):
   def __init__(self, message, retry_after):
     super(TooManyLoginAttemptsException, self).__init__(message)

data/model/tag.py

@@ -2,8 +2,9 @@ import logging
 
 from uuid import uuid4
 
+from peewee import IntegrityError
 from data.model import (image, db_transaction, DataModelException, _basequery,
-                        InvalidManifestException)
+                        InvalidManifestException, TagAlreadyCreatedException)
 from data.database import (RepositoryTag, Repository, Image, ImageStorage, Namespace, TagManifest,
                            RepositoryNotification, Label, TagManifestLabel, get_epoch_timestamp,
                            db_for_update)
@@ -96,8 +97,12 @@ def create_or_update_tag(namespace_name, repository_name, tag_name, tag_docker_i
     except Image.DoesNotExist:
       raise DataModelException('Invalid image with id: %s' % tag_docker_image_id)
 
-    return RepositoryTag.create(repository=repo, image=image_obj, name=tag_name,
-                                lifetime_start_ts=now_ts, reversion=reversion)
+    try:
+      return RepositoryTag.create(repository=repo, image=image_obj, name=tag_name,
+                                  lifetime_start_ts=now_ts, reversion=reversion)
+    except IntegrityError:
+      msg = 'Tag with name %s and lifetime start %s under repository %s/%s already exists'
+      raise TagAlreadyCreatedException(msg % (tag_name, now_ts, namespace_name, repository_name))
 
 
 def create_temporary_hidden_tag(repo, image_obj, expiration_s):
@@ -255,19 +260,16 @@ def revert_tag(repo_obj, tag_name, docker_image_id):
 def store_tag_manifest(namespace, repo_name, tag_name, docker_image_id, manifest_digest,
                        manifest_data):
   """ Stores a tag manifest for a specific tag name in the database. Returns the TagManifest
-      object, as well as a boolean indicating whether the TagManifest was created or updated.
+      object, as well as a boolean indicating whether the TagManifest was created.
   """
   with db_transaction():
     tag = create_or_update_tag(namespace, repo_name, tag_name, docker_image_id)
 
     try:
       manifest = TagManifest.get(digest=manifest_digest)
-      if manifest.tag == tag:
-        return manifest, False
-
       manifest.tag = tag
       manifest.save()
-      return manifest, True
+      return manifest, False
     except TagManifest.DoesNotExist:
       return TagManifest.create(tag=tag, digest=manifest_digest, json_data=manifest_data), True
 

endpoints/v2/errors.py

@@ -99,6 +99,13 @@ class SizeInvalid(V2RegistryException):
                                       detail)
 
 
+class TagAlreadyExists(V2RegistryException):
+  def __init__(self, detail=None):
+    super(TagAlreadyExists, self).__init__('TAG_ALREADY_EXISTS',
+                                           'tag was already pushed',
+                                           detail,
+                                           409)
+
 class TagInvalid(V2RegistryException):
   def __init__(self, detail=None):
     super(TagInvalid, self).__init__('TAG_INVALID',

endpoints/v2/manifest.py

@@ -20,13 +20,14 @@ from endpoints.common import parse_repository_name
 from endpoints.decorators import anon_protect
 from endpoints.v2 import v2_bp, require_repo_read, require_repo_write
 from endpoints.v2.errors import (BlobUnknown, ManifestInvalid, ManifestUnknown, TagInvalid,
-                                 NameInvalid)
+                                 NameInvalid, TagAlreadyExists)
 from endpoints.trackhelper import track_and_log
 from endpoints.notificationhelper import spawn_notification
 from util.registry.replication import queue_storage_replication
 from util.names import VALID_TAG_PATTERN
 from digest import digest_tools
 from data import model
+from data.model import TagAlreadyCreatedException
 from data.database import RepositoryTag
 
 logger = logging.getLogger(__name__)
@@ -451,9 +452,16 @@ def _write_manifest_itself(namespace_name, repo_name, manifest):
   # Store the manifest pointing to the tag.
   manifest_digest = manifest.digest
   leaf_layer_id = images_map[layers[-1].v1_metadata.docker_id].docker_image_id
-  tag_manifest, manifest_created = model.tag.store_tag_manifest(namespace_name, repo_name, tag_name,
-                                                                leaf_layer_id, manifest_digest,
-                                                                manifest.bytes)
+
+  try:
+    tag_manifest, manifest_created = model.tag.store_tag_manifest(namespace_name, repo_name,
+                                                                  tag_name, leaf_layer_id,
+                                                                  manifest_digest, manifest.bytes)
+  except TagAlreadyCreatedException:
+    logger.warning('Tag %s was already created under repository %s/%s pointing to image %s',
+                   tag_name, namespace_name, repo_name, leaf_layer_id)
+    raise TagAlreadyExists()
+
   if manifest_created:
     for key, value in layers[-1].v1_metadata.labels.iteritems():
       model.label.create_manifest_label(tag_manifest, key, value, 'manifest')

test/registry_tests.py

@@ -1149,6 +1149,9 @@ class V2RegistryTests(V2RegistryPullMixin, V2RegistryPushMixin, RegistryTestsMix
 
     (_, digest) = self.do_push('devtable', 'newrepo', 'devtable', 'password', images=images)
 
+    # Push again to verify no duplication.
+    (_, digest) = self.do_push('devtable', 'newrepo', 'devtable', 'password', images=images)
+
     self.conduct_api_login('devtable', 'password')
     labels = self.conduct('GET', '/api/v1/repository/devtable/newrepo/manifest/' + digest + '/labels').json()
     self.assertEquals(3, len(labels['labels']))