From 4a7b4ad06ad6566c14849aa1d1394f8e47b8f9c1 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 8 Oct 2018 16:32:09 +0100 Subject: [PATCH] Fix disabled namespace check --- data/cache/cache_key.py | 4 ++-- data/model/tag.py | 9 +++++---- data/registry_model/datatype.py | 9 ++++----- data/registry_model/interface.py | 8 +++++++- data/registry_model/label_handlers.py | 4 ++-- data/registry_model/registry_pre_oci_model.py | 15 +++++++++------ endpoints/v2/blob.py | 2 +- endpoints/v2/manifest.py | 2 +- endpoints/v2/v2auth.py | 2 +- 9 files changed, 32 insertions(+), 23 deletions(-) diff --git a/data/cache/cache_key.py b/data/cache/cache_key.py index 94d6f62a4..924d72b12 100644 --- a/data/cache/cache_key.py +++ b/data/cache/cache_key.py @@ -5,9 +5,9 @@ class CacheKey(namedtuple('CacheKey', ['key', 'expiration'])): pass -def for_repository_blob(namespace_name, repo_name, digest): +def for_repository_blob(namespace_name, repo_name, digest, version): """ Returns a cache key for a blob in a repository. """ - return CacheKey('repo_blob__%s_%s_%s' % (namespace_name, repo_name, digest), '60s') + return CacheKey('repo_blob__%s_%s_%s_%s' % (namespace_name, repo_name, digest, version), '60s') def for_catalog_page(auth_context_key, start_id, limit): diff --git a/data/model/tag.py b/data/model/tag.py index 705ebe6dc..7a92fda6e 100644 --- a/data/model/tag.py +++ b/data/model/tag.py @@ -214,7 +214,8 @@ def list_active_repo_tags(repo, start_id=None, limit=None): .join(ImageStorage) .where(RepositoryTag.repository == repo, RepositoryTag.hidden == False) .switch(RepositoryTag) - .join(TagManifest, JOIN.LEFT_OUTER)) + .join(TagManifest, JOIN.LEFT_OUTER) + .order_by(RepositoryTag.id)) if start_id is not None: query = query.where(RepositoryTag.id >= start_id) @@ -775,11 +776,11 @@ def change_repository_tag_expiration(namespace_name, repo_name, tag_name, expira def set_tag_expiration_for_manifest(tag_manifest, expiration_sec): """ - Changes the expiration of the tag that point to the given manifest to be its lifetime start + + Changes the expiration of the tag that points to the given manifest to be its lifetime start + the expiration seconds. """ - expiration_time_in_seconds = tag_manifest.tag.lifetime_start_ts + expiration_sec - expiration_date = datetime.utcfromtimestamp(expiration_time_in_seconds) + expiration_time_ts = tag_manifest.tag.lifetime_start_ts + expiration_sec + expiration_date = datetime.utcfromtimestamp(expiration_time_ts) return change_tag_expiration(tag_manifest.tag, expiration_date) diff --git a/data/registry_model/datatype.py b/data/registry_model/datatype.py index 1b2768e83..091776bb1 100644 --- a/data/registry_model/datatype.py +++ b/data/registry_model/datatype.py @@ -4,7 +4,7 @@ from functools import wraps, total_ordering class FromDictionaryException(Exception): """ Exception raised if constructing a data type from a dictionary fails due to - a version mismatch or missing data. + missing data. """ def datatype(name, static_fields): @@ -40,9 +40,6 @@ def datatype(name, static_fields): @classmethod def from_dict(cls, dict_data): - if dict_data.get('version') != 1: - raise FromDictionaryException() - try: return cls(**dict_data) except: @@ -50,9 +47,11 @@ def datatype(name, static_fields): def asdict(self): dictionary_rep = dict(self._fields) + assert ('db_id' not in dictionary_rep and + 'inputs' not in dictionary_rep) + dictionary_rep['db_id'] = self._db_id dictionary_rep['inputs'] = self._inputs - dictionary_rep['version'] = 1 return dictionary_rep return DataType diff --git a/data/registry_model/interface.py b/data/registry_model/interface.py index f7a1dadc5..544462c84 100644 --- a/data/registry_model/interface.py +++ b/data/registry_model/interface.py @@ -171,6 +171,10 @@ class RegistryDataInterface(object): it should be removed. """ + @abstractmethod + def is_existing_disabled_namespace(self, namespace_name): + """ Returns whether the given namespace exists and is disabled. """ + @abstractmethod def is_namespace_enabled(self, namespace_name): """ Returns whether the given namespace exists and is enabled. """ @@ -274,7 +278,9 @@ class RegistryDataInterface(object): Mounts the blob from another repository into the specified target repository, and adds an expiration before that blob is automatically GCed. This function is useful during push operations if an existing blob from another repositroy is being pushed. Returns False if - the mounting fails. Note that this function does *not* check security for mounting the blob. + the mounting fails. Note that this function does *not* check security for mounting the blob + and the caller is responsible for doing this check (an example can be found in + endpoints/v2/blob.py). """ @abstractmethod diff --git a/data/registry_model/label_handlers.py b/data/registry_model/label_handlers.py index 07635537a..96afe0d94 100644 --- a/data/registry_model/label_handlers.py +++ b/data/registry_model/label_handlers.py @@ -17,12 +17,12 @@ def _expires_after(label_dict, manifest, model): model.set_tags_expiration_for_manifest(manifest, total_seconds) -_LABEL_HANDLES = { +_LABEL_HANDLERS = { 'quay.expires-after': _expires_after, } def apply_label_to_manifest(label_dict, manifest, model): """ Runs the handler defined, if any, for the given label. """ - handler = _LABEL_HANDLES.get(label_dict['key']) + handler = _LABEL_HANDLERS.get(label_dict['key']) if handler is not None: handler(label_dict, manifest, model) diff --git a/data/registry_model/registry_pre_oci_model.py b/data/registry_model/registry_pre_oci_model.py index e119852b6..f3bb14953 100644 --- a/data/registry_model/registry_pre_oci_model.py +++ b/data/registry_model/registry_pre_oci_model.py @@ -257,8 +257,6 @@ class PreOCIModel(RegistryDataInterface): media_type_name=media_type_name)) yield add_label - if labels_to_add: - pass # TODO: make this truly batch once we've fully transitioned to V2_2 and no longer need # the mapping tables. @@ -537,6 +535,11 @@ class PreOCIModel(RegistryDataInterface): return Manifest.for_tag_manifest(tag_manifest) + def is_existing_disabled_namespace(self, namespace_name): + """ Returns whether the given namespace exists and is disabled. """ + namespace = model.user.get_namespace_user(namespace_name) + return namespace is not None and not namespace.enabled + def is_namespace_enabled(self, namespace_name): """ Returns whether the given namespace exists and is enabled. """ namespace = model.user.get_namespace_user(namespace_name) @@ -732,9 +735,9 @@ class PreOCIModel(RegistryDataInterface): return blob_found.asdict() - blob_cache_key = cache_key.for_repository_blob(namespace_name, repo_name, blob_digest) + blob_cache_key = cache_key.for_repository_blob(namespace_name, repo_name, blob_digest, 2) blob_dict = model_cache.retrieve(blob_cache_key, load_blob) - + try: return Blob.from_dict(blob_dict) if blob_dict is not None else None except FromDictionaryException: @@ -841,7 +844,7 @@ class PreOCIModel(RegistryDataInterface): """ Mounts the blob from another repository into the specified target repository, and adds an expiration before that blob is automatically GCed. This function is useful during push - operations if an existing blob from another repositroy is being pushed. Returns False if + operations if an existing blob from another repository is being pushed. Returns False if the mounting fails. """ repo = model.repository.lookup_repository(target_repository_ref._db_id) @@ -862,7 +865,7 @@ class PreOCIModel(RegistryDataInterface): try: tag_manifest = database.TagManifest.get(id=manifest._db_id) except database.TagManifest.DoesNotExist: - return None + return model.tag.set_tag_expiration_for_manifest(tag_manifest, expiration_sec) diff --git a/endpoints/v2/blob.py b/endpoints/v2/blob.py index 0aa34680a..73ffc553f 100644 --- a/endpoints/v2/blob.py +++ b/endpoints/v2/blob.py @@ -190,7 +190,7 @@ def start_blob_upload(namespace_name, repo_name): raise InvalidRequest() # Check if the blob will be uploaded now or in followup calls. If the `digest` is given, then - # the upload will occur as a monolithic chunk in this call. Ohterwise, we return a redirect + # the upload will occur as a monolithic chunk in this call. Otherwise, we return a redirect # for the client to upload the chunks as distinct operations. digest = request.args.get('digest', None) if digest is None: diff --git a/endpoints/v2/manifest.py b/endpoints/v2/manifest.py index 888a68b9a..f1c897868 100644 --- a/endpoints/v2/manifest.py +++ b/endpoints/v2/manifest.py @@ -44,7 +44,7 @@ def fetch_manifest_by_tagname(namespace_name, repo_name, manifest_ref): if tag is None: if registry_model.has_expired_tag(repository_ref, manifest_ref): logger.debug('Found expired tag %s for repository %s/%s', manifest_ref, namespace_name, - repo_name) + repo_name) msg = 'Tag %s was deleted or has expired. To pull, revive via time machine' % manifest_ref raise TagExpired(msg) diff --git a/endpoints/v2/v2auth.py b/endpoints/v2/v2auth.py index 9cb80371c..e21e61e57 100644 --- a/endpoints/v2/v2auth.py +++ b/endpoints/v2/v2auth.py @@ -163,7 +163,7 @@ def _authorize_or_downscope_request(scope_param, has_valid_auth_context): raise NameInvalid(message='Invalid repository name: %s' % namespace_and_repo) # Ensure the namespace is enabled. - if not registry_model.is_namespace_enabled(namespace): + if registry_model.is_existing_disabled_namespace(namespace): msg = 'Namespace %s has been disabled. Please contact a system administrator.' % namespace raise NamespaceDisabled(message=msg)