diff --git a/data/cache/cache_key.py b/data/cache/cache_key.py index e1c8ce2767..3de4572b3a 100644 --- a/data/cache/cache_key.py +++ b/data/cache/cache_key.py @@ -65,3 +65,19 @@ def for_security_report(digest, cache_config): # Security reports don't change often so a longer TTL can be justified. cache_ttl = cache_config.get("security_report_cache_ttl", "300s") return CacheKey(f"security_report_{digest}", cache_ttl) + + +def for_repository_lookup(namespace_name, repo_name, manifest_ref, kind_filter, cache_config): + """ + Returns a cache key for repository lookup. + """ + + cache_ttl = cache_config.get("repository_lookup_cache_ttl", "120s") + cache_key = f"repository_lookup_{namespace_name}_{repo_name}" + + if manifest_ref is not None: + cache_key = f"{cache_key}_{manifest_ref}" + if kind_filter is not None: + cache_key = f"{cache_key}_{kind_filter}" + + return CacheKey(cache_key, cache_ttl) diff --git a/data/registry_model/interface.py b/data/registry_model/interface.py index 1f2c749352..cd8cdcd908 100644 --- a/data/registry_model/interface.py +++ b/data/registry_model/interface.py @@ -43,7 +43,13 @@ def get_most_recent_tag(self, repository_ref): @abstractmethod def lookup_repository( - self, namespace_name, repo_name, kind_filter=None, raise_on_error=False, manifest_ref=None + self, + namespace_name, + repo_name, + kind_filter=None, + raise_on_error=False, + manifest_ref=None, + model_cache=None, ): """ Looks up and returns a reference to the repository with the given namespace and name, or diff --git a/data/registry_model/registry_oci_model.py b/data/registry_model/registry_oci_model.py index 0378a6659a..69f8aeb8a1 100644 --- a/data/registry_model/registry_oci_model.py +++ b/data/registry_model/registry_oci_model.py @@ -6,7 +6,15 @@ from data import database, model from data.cache import cache_key -from data.database import db_disallow_replica_use, db_transaction +from data.database import ( + Repository, + RepositoryKind, + RepositoryState, + User, + Visibility, + db_disallow_replica_use, + db_transaction, +) from data.model import DataModelException, QuotaExceededException, namespacequota, oci from data.model.oci.retriever import RepositoryContentRetriever from data.readreplica import ReadOnlyModeException @@ -732,19 +740,68 @@ def find_repository_with_garbage(self, limit_to_gc_policy_s): return RepositoryReference.for_repo_obj(repo) + @staticmethod + def get_repository_response_as_json(val): + return { + "id": val.id, + "visibility": { + "id": val.visibility.id, + "name": val.visibility.name, + }, + "kind": { + "id": val.kind.id, + "name": val.kind.name, + }, + "state": val.state, + "namespace_user": { + "stripe_id": val.namespace_user.stripe_id, + }, + } + + @staticmethod + def get_repository_response_to_object(val): + return Repository( + id=val["id"], + state=RepositoryState(val["state"]), + kind=RepositoryKind(id=val["kind"]["id"], name=val["kind"]["name"]), + visibility=Visibility(id=val["visibility"]["id"], name=val["visibility"]["name"]), + namespace_user=User(stripe_id=val["namespace_user"]["stripe_id"]), + ) + def lookup_repository( - self, namespace_name, repo_name, kind_filter=None, raise_on_error=False, manifest_ref=None + self, + namespace_name, + repo_name, + kind_filter=None, + raise_on_error=False, + manifest_ref=None, + model_cache=None, ): """ Looks up and returns a reference to the repository with the given namespace and name, or None if none. """ - repo = model.repository.get_repository(namespace_name, repo_name, kind_filter=kind_filter) + + def get_repository_loader(): + result = model.repository.get_repository( + namespace_name, repo_name, kind_filter=kind_filter + ) + return OCIModel.get_repository_response_as_json(result) if result else None + + if model_cache is not None: + repository_lookup_key = cache_key.for_repository_lookup( + namespace_name, repo_name, manifest_ref, kind_filter, model_cache.cache_config + ) + repo = model_cache.retrieve(repository_lookup_key, get_repository_loader) + else: + repo = get_repository_loader() + if repo is None: if raise_on_error: raise model.RepositoryDoesNotExist() return None + repo = OCIModel.get_repository_response_to_object(repo) state = repo.state return RepositoryReference.for_repo_obj( repo, diff --git a/data/registry_model/registry_proxy_model.py b/data/registry_model/registry_proxy_model.py index bf02b324c0..013873efcc 100644 --- a/data/registry_model/registry_proxy_model.py +++ b/data/registry_model/registry_proxy_model.py @@ -92,7 +92,13 @@ def __init__(self, namespace_name, repo_name, user): self._proxy = Proxy(self._config, repo_name) def lookup_repository( - self, namespace_name, repo_name, kind_filter=None, raise_on_error=False, manifest_ref=None + self, + namespace_name, + repo_name, + kind_filter=None, + raise_on_error=False, + manifest_ref=None, + model_cache=None, ): """ Looks up and returns a reference to the repository with the given namespace and name, or diff --git a/data/registry_model/test/test_interface.py b/data/registry_model/test/test_interface.py index 8421340451..335cdb19b8 100644 --- a/data/registry_model/test/test_interface.py +++ b/data/registry_model/test/test_interface.py @@ -13,6 +13,7 @@ from app import docker_v2_signing_key, storage from data import model +from data.cache import cache_key from data.cache.impl import InMemoryDataModelCache from data.cache.test.test_cache import TEST_CACHE_CONFIG from data.database import ( @@ -114,6 +115,33 @@ def test_lookup_repository(repo_namespace, repo_name, expected, registry_model): assert repo_ref is None +@pytest.mark.parametrize( + "repo_namespace, repo_name, expected", + [ + ("devtable", "simple", True), + ("buynlarge", "orgrepo", True), + ("buynlarge", "unknownrepo", False), + ], +) +def test_lookup_repository_with_cache(repo_namespace, repo_name, expected, registry_model): + model_cache = InMemoryDataModelCache(TEST_CACHE_CONFIG) + model_cache.empty_for_testing() + + cache_key_for_repository_lookup = cache_key.for_repository_lookup( + repo_namespace, repo_name, None, None, {} + ) + repo_ref = registry_model.lookup_repository(repo_namespace, repo_name, model_cache=model_cache) + cached_result = model_cache.cache.get(cache_key_for_repository_lookup.key) + + if expected: + res_obj = OCIModel.get_repository_response_to_object(json.loads(cached_result)) + assert repo_ref._db_id is res_obj.id + assert repo_ref.state == res_obj.state + assert repo_ref.namespace_name == repo_namespace + else: + assert repo_ref is None + + @pytest.mark.parametrize( "repo_namespace, repo_name", [ diff --git a/endpoints/v2/manifest.py b/endpoints/v2/manifest.py index 19a03da816..57b2fd4d7b 100644 --- a/endpoints/v2/manifest.py +++ b/endpoints/v2/manifest.py @@ -4,7 +4,7 @@ from flask import Response, request, url_for import features -from app import app, storage +from app import app, model_cache, storage from auth.registry_jwt_auth import process_registry_jwt_auth from data.database import db_disallow_replica_use from data.model import ( @@ -68,7 +68,11 @@ def fetch_manifest_by_tagname(namespace_name, repo_name, manifest_ref, registry_model): try: repository_ref = registry_model.lookup_repository( - namespace_name, repo_name, raise_on_error=True, manifest_ref=manifest_ref + namespace_name, + repo_name, + raise_on_error=True, + manifest_ref=manifest_ref, + model_cache=model_cache, ) except RepositoryDoesNotExist as e: image_pulls.labels("v2", "tag", 404).inc() @@ -137,7 +141,11 @@ def fetch_manifest_by_tagname(namespace_name, repo_name, manifest_ref, registry_ def fetch_manifest_by_digest(namespace_name, repo_name, manifest_ref, registry_model): try: repository_ref = registry_model.lookup_repository( - namespace_name, repo_name, raise_on_error=True, manifest_ref=manifest_ref + namespace_name, + repo_name, + raise_on_error=True, + manifest_ref=manifest_ref, + model_cache=model_cache, ) except RepositoryDoesNotExist as e: image_pulls.labels("v2", "manifest", 404).inc() @@ -293,7 +301,9 @@ def write_manifest_by_digest(namespace_name, repo_name, manifest_ref): # manifest does not contain the tag and this call was not given a tag name. # Instead, we write the manifest with a temporary tag, as it is being pushed # as part of a call for a manifest list. - repository_ref = registry_model.lookup_repository(namespace_name, repo_name) + repository_ref = registry_model.lookup_repository( + namespace_name, repo_name, model_cache=model_cache + ) if repository_ref is None: image_pushes.labels("v2", 404, "").inc() raise NameUnknown("repository not found") @@ -362,7 +372,9 @@ def delete_manifest_by_digest(namespace_name, repo_name, manifest_ref): forbidden by the spec. """ with db_disallow_replica_use(): - repository_ref = registry_model.lookup_repository(namespace_name, repo_name) + repository_ref = registry_model.lookup_repository( + namespace_name, repo_name, model_cache=model_cache + ) if repository_ref is None: raise NameUnknown("repository not found") @@ -413,7 +425,9 @@ def _write_manifest( namespace_name, repo_name, tag_name, manifest_impl, registry_model=registry_model ): # Ensure that the repository exists. - repository_ref = registry_model.lookup_repository(namespace_name, repo_name) + repository_ref = registry_model.lookup_repository( + namespace_name, repo_name, model_cache=model_cache + ) if repository_ref is None: raise NameUnknown("repository not found")