From 3fac4c4284dd710d16e44685180c9f5fffaa5f0c Mon Sep 17 00:00:00 2001 From: Lubos Mjachky Date: Fri, 19 Nov 2021 17:55:52 +0100 Subject: [PATCH] Store content in the Redis cache ref #9500 Required PR: https://github.com/pulp/pulpcore/pull/1751 --- CHANGES/9500.feature | 1 + pulp_container/app/cache.py | 125 +++++++++++ pulp_container/app/models.py | 24 ++- pulp_container/app/redirects.py | 25 ++- pulp_container/app/registry.py | 10 + pulp_container/app/registry_api.py | 6 +- .../functional/api/test_content_cache.py | 201 ++++++++++++++++++ .../tests/functional/api/test_pull_content.py | 44 +--- .../functional/api/test_repositories_list.py | 22 +- .../api/test_token_authentication.py | 23 +- pulp_container/tests/functional/constants.py | 5 + pulp_container/tests/functional/utils.py | 23 ++ 12 files changed, 412 insertions(+), 97 deletions(-) create mode 100644 CHANGES/9500.feature create mode 100644 pulp_container/app/cache.py create mode 100644 pulp_container/tests/functional/api/test_content_cache.py diff --git a/CHANGES/9500.feature b/CHANGES/9500.feature new file mode 100644 index 000000000..951608b0d --- /dev/null +++ b/CHANGES/9500.feature @@ -0,0 +1 @@ +Enabled caching responses from the registry. diff --git a/pulp_container/app/cache.py b/pulp_container/app/cache.py new file mode 100644 index 000000000..714654fea --- /dev/null +++ b/pulp_container/app/cache.py @@ -0,0 +1,125 @@ +from aiohttp.web_exceptions import HTTPForbidden + +from pulpcore.plugin.cache import CacheKeys, ContentCache, SyncContentCache + +from pulp_container.app.models import ContainerDistribution + +ACCEPT_HEADER_KEY = "accept_header" + + +class RegistryCache: + """A class that overrides the default key specs.""" + + def __init__(self, base_key=None, expires_ttl=None, auth=None): + """Initialize the parent class with the plugin's specific keys.""" + updated_keys = (CacheKeys.path, CacheKeys.method, ACCEPT_HEADER_KEY) + super().__init__(base_key=base_key, expires_ttl=expires_ttl, keys=updated_keys, auth=auth) + + +class RegistryContentCache(RegistryCache, ContentCache): + """A wrapper around the Redis content cache handler tailored for the content application.""" + + def make_key(self, request): + """Make a key composed of the request's path, method, host, and accept header.""" + all_keys = { + CacheKeys.path: request.path, + CacheKeys.method: request.method, + CacheKeys.host: request.url.host, + ACCEPT_HEADER_KEY: request.headers["accept"], + } + key = ":".join(all_keys[k] for k in self.keys) + return key + + +class RegistryApiCache(RegistryCache, SyncContentCache): + """A wrapper around the Redis content cache handler tailored for the registry API.""" + + def make_key(self, request): + """Make a key composed of the request's path, method, host, and accept header.""" + all_keys = { + CacheKeys.path: request.path, + CacheKeys.method: request.method, + CacheKeys.host: request.get_host(), + ACCEPT_HEADER_KEY: request.headers["accept"], + } + key = ":".join(all_keys[k] for k in self.keys) + return key + + +def auth_cached(request, cached, base_key): + """ + Authentication check for the cached stream_content handler + + Args: + request (:class:`aiohttp.web.request`): The request from the client. + cached (:class:`CacheAiohttp`): The Pulp cache + base_key (str): The base_key associated with this response + """ + guard_key = "DISTRO#GUARD#PRESENT" + present = cached.get(guard_key, base_key=base_key) + if present == b"True" or present is None: + path = request.resolver_match.kwargs["path"] + distro = ContainerDistribution.objects.select_related( + "repository", "repository_version", "publication", "remote" + ).get(base_path=path) + try: + guard = _permit(request, distro) + except HTTPForbidden: + guard = True + raise + finally: + if not present: + cached.set(guard_key, str(guard), base_key=base_key) + + +def _permit(request, distribution): + """ + Permit the request. + + Authorization is delegated to the optional content-guard associated with the distribution. + + Args: + request (:class:`aiohttp.web.Request`): A request for a published file. + distribution (detail of :class:`pulpcore.plugin.models.Distribution`): The matched + distribution. + + Raises: + :class:`aiohttp.web_exceptions.HTTPForbidden`: When not permitted. + + """ + guard = distribution.content_guard + if not guard: + return False + try: + guard.cast().permit(request) + except PermissionError as pe: + raise HTTPForbidden(reason=str(pe)) + return True + + +def find_base_path_cached(request, cached): + """ + Finds the base-path to use for the base-key in the cache + + Args: + request (:class:`aiohttp.web.request`): The request from the client. + cached (:class:`CacheAiohttp`): The Pulp cache + + Returns: + str: The base-path associated with this request + + """ + path = request.resolver_match.kwargs["path"] + base_paths = [path] + multiplied_base_paths = [] + for i, base_path in enumerate(base_paths): + copied_by_index_base_path = [base_path for _ in range(i + 1)] + multiplied_base_paths.extend(copied_by_index_base_path) + index_p1 = cached.exists(base_key=multiplied_base_paths) + if index_p1: + return base_paths[index_p1 - 1] + else: + distro = ContainerDistribution.objects.select_related( + "repository", "repository_version", "publication", "remote" + ).get(base_path=path) + return distro.base_path diff --git a/pulp_container/app/models.py b/pulp_container/app/models.py index 6c98eb972..4eea9f7ec 100644 --- a/pulp_container/app/models.py +++ b/pulp_container/app/models.py @@ -9,6 +9,7 @@ from urllib.parse import urlparse from django.db import models +from django.conf import settings from django.contrib.auth.models import Group from django.contrib.postgres import fields from django.shortcuts import redirect @@ -598,17 +599,20 @@ def permit(self, request): """ Permit preauthenticated redirects from pulp-api. """ - try: - signed_url = request.url - validate_token = request.query["validate_token"] - hex_salt, hex_digest = validate_token.split(":", 1) - salt = bytes.fromhex(hex_salt) - digest = bytes.fromhex(hex_digest) - url = re.sub(r"\?validate_token=.*$", "", str(signed_url)) - if not digest == self._get_digest(salt, url): + if settings.DEFAULT_FILE_STORAGE == "storages.backends.s3boto3.S3Boto3Storage": + pass + else: + try: + signed_url = request.url + validate_token = request.query["validate_token"] + hex_salt, hex_digest = validate_token.split(":", 1) + salt = bytes.fromhex(hex_salt) + digest = bytes.fromhex(hex_digest) + url = re.sub(r"\?validate_token=.*$", "", str(signed_url)) + if not digest == self._get_digest(salt, url): + raise PermissionError("Access not authenticated") + except (KeyError, ValueError): raise PermissionError("Access not authenticated") - except (KeyError, ValueError): - raise PermissionError("Access not authenticated") def preauthenticate_url(self, url, salt=None): """ diff --git a/pulp_container/app/redirects.py b/pulp_container/app/redirects.py index 577bdac1b..eaa61f5db 100644 --- a/pulp_container/app/redirects.py +++ b/pulp_container/app/redirects.py @@ -5,6 +5,7 @@ from django.http import Http404 from django.shortcuts import redirect +from pulp_container.app.cache import RegistryApiCache, find_base_path_cached, auth_cached from pulp_container.app.utils import get_accepted_media_types from pulp_container.constants import BLOB_CONTENT_TYPE, MEDIA_TYPE @@ -36,19 +37,19 @@ class FileStorageRedirects(CommonRedirects): A class which contains methods used for redirecting to the default django's file storage. """ - def issue_tag_redirect(self, tag): + def issue_tag_redirect(self, request, tag): """ Issue a redirect for the passed tag. """ return self.redirect_to_content_app("manifests", tag.name) - def issue_manifest_redirect(self, manifest): + def issue_manifest_redirect(self, request, manifest): """ Issue a redirect for the passed manifest. """ return self.redirect_to_content_app("manifests", manifest.digest) - def issue_blob_redirect(self, blob): + def issue_blob_redirect(self, request, blob): """ Issue a redirect for the passed blob. """ @@ -60,7 +61,11 @@ class S3StorageRedirects(CommonRedirects): A class that implements methods for the direct retrieval of manifest objects. """ - def issue_tag_redirect(self, tag): + @RegistryApiCache( + base_key=lambda req, cac: find_base_path_cached(req, cac), + auth=lambda req, cac, bk: auth_cached(req, cac, bk), + ) + def issue_tag_redirect(self, request, tag): """ Issue a redirect or perform a schema conversion if an accepted media type requires it. """ @@ -75,7 +80,11 @@ def issue_tag_redirect(self, tag): # execute the schema conversion return self.redirect_to_content_app("manifests", tag.name) - def issue_manifest_redirect(self, manifest): + @RegistryApiCache( + base_key=lambda req, cac: find_base_path_cached(req, cac), + auth=lambda req, cac, bk: auth_cached(req, cac, bk), + ) + def issue_manifest_redirect(self, request, manifest): """ Directly redirect to an associated manifest's artifact. """ @@ -92,7 +101,11 @@ def redirect_to_artifact(self, content_name, manifest, manifest_media_type): return self.redirect_to_object_storage(artifact, manifest_media_type) - def issue_blob_redirect(self, blob): + @RegistryApiCache( + base_key=lambda req, cac: find_base_path_cached(req, cac), + auth=lambda req, cac, bk: auth_cached(req, cac, bk), + ) + def issue_blob_redirect(self, request, blob): """ Redirect to the passed blob or stream content when an associated artifact is not present. """ diff --git a/pulp_container/app/registry.py b/pulp_container/app/registry.py index d7e9a7a4d..c389aab0f 100644 --- a/pulp_container/app/registry.py +++ b/pulp_container/app/registry.py @@ -10,6 +10,8 @@ from pulpcore.plugin.content import Handler, PathNotResolved from pulpcore.plugin.models import ContentArtifact + +from pulp_container.app.cache import RegistryContentCache from pulp_container.app.models import ContainerDistribution, Tag, Blob from pulp_container.app.schema_convert import Schema2toSchema1ConverterWrapper from pulp_container.app.utils import get_accepted_media_types @@ -76,6 +78,10 @@ async def _dispatch(file, headers): file_response = web.FileResponse(path, headers=full_headers) return file_response + @RegistryContentCache( + base_key=lambda req, cac: Registry.find_base_path_cached(req, cac), + auth=lambda req, cac, bk: Registry.auth_cached(req, cac, bk), + ) async def get_tag(self, request): """ Match the path and stream either Manifest or ManifestList. @@ -200,6 +206,10 @@ async def dispatch_converted_schema(tag, accepted_media_types, path): } return web.Response(text=result.text, headers=response_headers) + @RegistryContentCache( + base_key=lambda req, cac: Registry.find_base_path_cached(req, cac), + auth=lambda req, cac, bk: Registry.auth_cached(req, cac, bk), + ) async def get_by_digest(self, request): """ Return a response to the "GET" action. diff --git a/pulp_container/app/registry_api.py b/pulp_container/app/registry_api.py index dca2a781a..f739d71af 100644 --- a/pulp_container/app/registry_api.py +++ b/pulp_container/app/registry_api.py @@ -805,7 +805,7 @@ def get(self, request, path, pk=None): if pk == EMPTY_BLOB: return redirects.redirect_to_content_app("blobs", pk) raise BlobNotFound(digest=pk) - return redirects.issue_blob_redirect(blob) + return redirects.issue_blob_redirect(request, blob) class Manifests(RedirectsMixin, ContainerRegistryApiMixin, ViewSet): @@ -834,13 +834,13 @@ def get(self, request, path, pk=None): try: if pk[:7] != "sha256:": tag = models.Tag.objects.get(name=pk, pk__in=repository_version.content) - return redirects.issue_tag_redirect(tag) + return redirects.issue_tag_redirect(request, tag) else: manifest = models.Manifest.objects.get(digest=pk, pk__in=repository_version.content) except (models.Tag.DoesNotExist, models.Manifest.DoesNotExist): raise ManifestNotFound(reference=pk) - return redirects.issue_manifest_redirect(manifest) + return redirects.issue_manifest_redirect(request, manifest) def put(self, request, path, pk=None): """ diff --git a/pulp_container/tests/functional/api/test_content_cache.py b/pulp_container/tests/functional/api/test_content_cache.py new file mode 100644 index 000000000..c0113ffcb --- /dev/null +++ b/pulp_container/tests/functional/api/test_content_cache.py @@ -0,0 +1,201 @@ +"""Tests related to the Redis content caching.""" +import os +import requests +import unittest +from urllib.parse import urljoin + +from pulp_smash import cli, config +from pulp_smash.pulp3.bindings import delete_orphans, monitor_task +from pulp_smash.pulp3.utils import gen_distribution, gen_repo +from pulp_smash.utils import get_pulp_setting + +from pulpcore.client.pulp_container import ( + RepositorySyncURL, + RepositoriesContainerApi, + RemotesContainerApi, + DistributionsContainerApi, + PatchedcontainerContainerDistribution, + ContentBlobsApi, + ContentManifestsApi, + ContentTagsApi, + UnTagImage, +) + +from pulp_container.tests.functional.constants import PULP_CONTENT_HOST_BASE_URL +from pulp_container.tests.functional.utils import ( + gen_container_client, + gen_container_remote, + get_auth_for_url, +) + +STANDARD_FILE_STORAGE_FRAMEWORKS = [ + "django.core.files.storage.FileSystemStorage", + "pulpcore.app.models.storage.FileSystem", +] + + +class ContentCacheTestCache(unittest.TestCase): + """A test case that verifies the functionality of the Redis caching machinery.""" + + @classmethod + def setUpClass(cls): + """Sync a remote repository and create a new distribution pointing to the repository.""" + cls.cli_client = cli.Client(config.get_config()) + + client_api = gen_container_client() + cls.blobs_api = ContentBlobsApi(client_api) + cls.manifests_api = ContentManifestsApi(client_api) + cls.tags_api = ContentTagsApi(client_api) + cls.repo_api = RepositoriesContainerApi(client_api) + cls.remote_api = RemotesContainerApi(client_api) + cls.dist_api = DistributionsContainerApi(client_api) + + cls.repo = cls.repo_api.create(gen_repo()) + cls.remote = cls.remote_api.create(gen_container_remote()) + body = RepositorySyncURL(remote=cls.remote.pulp_href) + response = cls.repo_api.sync(cls.repo.pulp_href, body) + monitor_task(response.task) + + cls.repo = cls.repo_api.read(cls.repo.pulp_href) + + response = cls.dist_api.create(gen_distribution(repository=cls.repo.pulp_href)) + cls.distro = cls.dist_api.read(monitor_task(response.task).created_resources[0]) + + relative_path = os.path.join("v2/", f"{cls.distro.base_path}/") + cls.dist_url = urljoin(PULP_CONTENT_HOST_BASE_URL, relative_path) + + @classmethod + def tearDownClass(cls): + """Remove the created distribution, remote, and repository.""" + cls.dist_api.delete(cls.distro.pulp_href) + cls.remote_api.delete(cls.remote.pulp_href) + + delete_orphans() + + def test_01_basic_cache_access(self): + """Test whether responses are cached for initial querying.""" + self.check_content(cache_status_first_func) + + def test_02_remove_repository_invalidates(self): + """Test if removing the repository from the distribution invalidates the cache.""" + body = PatchedcontainerContainerDistribution(repository="") + response = self.dist_api.partial_update(self.distro.pulp_href, body) + monitor_task(response.task) + + self.check_content(cache_status_not_found_func) + + def test_03_restore_repository(self): + """Test if responses are cacheable when the repository is added back.""" + body = PatchedcontainerContainerDistribution(repository=self.repo.pulp_href) + response = self.dist_api.partial_update(self.distro.pulp_href, body) + monitor_task(response.task) + + self.check_content(cache_status_first_func) + + def test_04_multiple_distributions(self): + """Add a new distribution and check if its responses are cached separately.""" + response = self.dist_api.create(gen_distribution(repository=self.repo.pulp_href)) + distro2_pulp_url = monitor_task(response.task).created_resources[0] + self.__class__.distro2 = self.dist_api.read(distro2_pulp_url) + + relative_path = os.path.join("v2/", f"{self.distro2.base_path}/") + self.__class__.dist_url2 = urljoin(PULP_CONTENT_HOST_BASE_URL, relative_path) + + self.check_content(cache_status_found_func) + self.check_content(cache_status_first_func, dist_url=self.dist_url2) + + def test_05_different_headers(self): + """Simulate a scenario where a user queries manifests with different Accept headers.""" + self.check_content(cache_status_found_func) + self.check_content(cache_status_first_func, headers={"Accept": "*/*"}) + + def test_06_invalidate_multiple_distributions(self): + """Test if updating the repository referenced by multiple distributions invalidates all.""" + untag_data = UnTagImage(tag="nanoserver-1809") + response = self.repo_api.untag(self.repo.pulp_href, untag_data) + monitor_task(response.task) + + self.check_content(cache_status_first_func) + self.check_content(cache_status_first_func, dist_url=self.dist_url2) + + def test_07_delete_distribution_invalidates_one(self): + """Test that deleting one distribution sharing the repository only invalidates its cache.""" + response = self.dist_api.delete(self.distro2.pulp_href) + monitor_task(response.task) + + self.check_content(cache_status_found_func) + self.check_content(cache_status_not_found_func, dist_url=self.dist_url2) + + def test_08_delete_repo_invalidates(self): + """Tests that deleting the repository invalidates the cache.""" + response = self.repo_api.delete(self.repo.pulp_href) + monitor_task(response.task) + self.check_content(cache_status_not_found_func) + + def test_09_no_error_when_accessing_invalid_file(self): + """Tests that accessing content, that does not exist, gives an HTTP 404 error.""" + files = ["invalid", "another/bad-one", "DNE/"] + for f in files: + url = urljoin(self.dist_url, f) + response = requests.get(url) + + response_metadata = self.fetch_response_metadata(response) + self.assertEqual(cache_status_not_found_func(0), response_metadata, url) + + def check_content(self, expect_metadata, tag_name="latest", dist_url=None, headers=None): + """Check a manifest and blob referenced by the passed tag with the expected assertions.""" + latest_tag = self.tags_api.list(name=tag_name).to_dict()["results"][0] + manifest_by_tag = f"manifests/{tag_name}" + + latest_manifest = self.manifests_api.read(latest_tag["tagged_manifest"]) + manifest_by_digest = f"manifests/{latest_manifest.digest}" + + first_listed_manifest = self.manifests_api.read(latest_manifest.listed_manifests[0]) + latest_first_blob = self.blobs_api.read(first_listed_manifest.blobs[0]) + blob_by_digest = f"blobs/{latest_first_blob.digest}" + + headers = headers if headers else {"Accept": latest_manifest.media_type} + + duplicated_content_units = sorted([manifest_by_tag, manifest_by_digest] * 2) + for i, c in enumerate(duplicated_content_units): + url = urljoin(dist_url or self.dist_url, c) + self.check_cache(url, expect_metadata(i), headers) + + duplicated_content_units = [blob_by_digest] * 2 + for i, c in enumerate(duplicated_content_units): + url = urljoin(dist_url or self.dist_url, c) + self.check_cache(url, expect_metadata(i), headers) + + def check_cache(self, url, expected_metadata, headers): + """A helper function to check if cache miss or hit occurred.""" + auth = get_auth_for_url(url) + + response = requests.get(url, auth=auth, headers=headers) + response_metadata = self.fetch_response_metadata(response) + self.assertEqual(expected_metadata, response_metadata, url) + + def fetch_response_metadata(self, response): + """Retrieve metadata from the passed response and normalize status code for redirects.""" + storage = get_pulp_setting(self.cli_client, " DEFAULT_FILE_STORAGE") + if storage in STANDARD_FILE_STORAGE_FRAMEWORKS: + return response.status_code, response.headers.get("X-PULP-CACHE") + else: + if response.history: + response = response.history[0] + response.status_code = 200 + return response.status_code, response.headers.get("X-PULP-CACHE") + + +def cache_status_first_func(i): + """Miss at first, then hit.""" + return 200, "HIT" if i % 2 == 1 else "MISS" + + +def cache_status_found_func(_): + """Hit all the time.""" + return 200, "HIT" + + +def cache_status_not_found_func(_): + """End with does not exist.""" + return 404, None diff --git a/pulp_container/tests/functional/api/test_pull_content.py b/pulp_container/tests/functional/api/test_pull_content.py index e4ee7208e..fb59ca94c 100644 --- a/pulp_container/tests/functional/api/test_pull_content.py +++ b/pulp_container/tests/functional/api/test_pull_content.py @@ -5,6 +5,7 @@ import json import requests import unittest + from urllib.parse import urljoin, urlparse from pulp_smash import api, cli, config, exceptions @@ -16,13 +17,11 @@ ) from pulp_container.tests.functional.utils import ( - TOKEN_AUTH_DISABLED, core_client, gen_container_client, gen_container_remote, get_docker_hub_remote_blobsums, - BearerTokenAuth, - AuthenticationHeaderQueries, + get_auth_for_url, ) from pulp_container.tests.functional.constants import ( CONTAINER_CONTENT_NAME, @@ -148,27 +147,9 @@ def test_api_performes_schema_conversion(self): image_path = "/v2/{}/manifests/{}".format(self.distribution_with_repo.base_path, "latest") latest_image_url = urljoin(self.cfg.get_base_url(), image_path) - if TOKEN_AUTH_DISABLED: - auth = () - else: - with self.assertRaises(requests.HTTPError) as cm: - self.client.get(latest_image_url, headers={"Accept": MEDIA_TYPE.MANIFEST_V1}) - - content_response = cm.exception.response - self.assertEqual(content_response.status_code, 401) - - authenticate_header = content_response.headers["Www-Authenticate"] - queries = AuthenticationHeaderQueries(authenticate_header) - content_response = requests.get( - queries.realm, params={"service": queries.service, "scope": queries.scope} - ) - content_response.raise_for_status() - token = content_response.json()["token"] - auth = BearerTokenAuth(token) + auth = get_auth_for_url(latest_image_url) content_response = requests.get( - latest_image_url, - auth=auth, - headers={"Accept": MEDIA_TYPE.MANIFEST_V1}, + latest_image_url, auth=auth, headers={"Accept": MEDIA_TYPE.MANIFEST_V1} ) content_response.raise_for_status() base_content_type = content_response.headers["Content-Type"].split(";")[0] @@ -194,22 +175,7 @@ def test_create_empty_blob_on_the_fly(self): blob_path = "/v2/{}/blobs/{}".format(self.distribution_with_repo.base_path, EMPTY_BLOB) empty_blob_url = urljoin(self.cfg.get_base_url(), blob_path) - if TOKEN_AUTH_DISABLED: - auth = () - else: - with self.assertRaises(requests.HTTPError) as cm: - requests.get(empty_blob_url).raise_for_status() - - content_response = cm.exception.response - self.assertEqual(content_response.status_code, 401) - - authenticate_header = content_response.headers["Www-Authenticate"] - queries = AuthenticationHeaderQueries(authenticate_header) - content_response = requests.get( - queries.realm, params={"service": queries.service, "scope": queries.scope} - ) - content_response.raise_for_status() - auth = BearerTokenAuth(content_response.json()["token"]) + auth = get_auth_for_url(empty_blob_url) content_response = requests.get(empty_blob_url, auth=auth) content_response.raise_for_status() # calculate digest of the payload diff --git a/pulp_container/tests/functional/api/test_repositories_list.py b/pulp_container/tests/functional/api/test_repositories_list.py index 7c606dd5c..5632fb863 100644 --- a/pulp_container/tests/functional/api/test_repositories_list.py +++ b/pulp_container/tests/functional/api/test_repositories_list.py @@ -4,7 +4,6 @@ from urllib.parse import urljoin import requests -from requests.exceptions import HTTPError from pulp_smash import api, config from pulp_smash.pulp3.bindings import delete_orphans, monitor_task @@ -17,8 +16,7 @@ gen_container_remote, gen_container_client, gen_user, - BearerTokenAuth, - AuthenticationHeaderQueries, + get_auth_for_url, ) from pulpcore.client.pulp_container import ( @@ -61,22 +59,8 @@ def get_listed_repositories(self, auth=None): """Fetch repositories from the catalog endpoint.""" repositories_list_endpoint = urljoin(self.cfg.get_base_url(), "/v2/_catalog") - with self.assertRaises(HTTPError) as cm: - requests.get(repositories_list_endpoint).raise_for_status() - content_response = cm.exception.response - authenticate_header = content_response.headers["Www-Authenticate"] - - queries = AuthenticationHeaderQueries(authenticate_header) - self.assertEqual(queries.scope, "registry:catalog:*") - - content_response = requests.get( - queries.realm, params={"service": queries.service, "scope": queries.scope}, auth=auth - ) - content_response.raise_for_status() - - repositories = requests.get( - repositories_list_endpoint, auth=BearerTokenAuth(content_response.json()["token"]) - ) + auth = get_auth_for_url(repositories_list_endpoint) + repositories = requests.get(repositories_list_endpoint, auth=auth) repositories.raise_for_status() return repositories diff --git a/pulp_container/tests/functional/api/test_token_authentication.py b/pulp_container/tests/functional/api/test_token_authentication.py index 67f52f247..4497334bb 100644 --- a/pulp_container/tests/functional/api/test_token_authentication.py +++ b/pulp_container/tests/functional/api/test_token_authentication.py @@ -4,7 +4,6 @@ from urllib.parse import urljoin, urlparse import requests -from requests.exceptions import HTTPError from pulp_smash import api, config, cli from pulp_smash.pulp3.bindings import delete_orphans, monitor_task @@ -13,8 +12,7 @@ from pulp_container.tests.functional.utils import ( gen_container_remote, gen_container_client, - BearerTokenAuth, - AuthenticationHeaderQueries, + get_auth_for_url, ) from pulp_container.tests.functional.constants import ( CONTAINER_TAG_PATH, @@ -88,24 +86,9 @@ def test_pull_image_with_raw_http_requests(self): image_path = "/v2/{}/manifests/{}".format(self.distribution.base_path, "manifest_a") latest_image_url = urljoin(self.cfg.get_base_url(), image_path) - with self.assertRaises(HTTPError) as cm: - requests.get( - latest_image_url, headers={"Accept": MEDIA_TYPE.MANIFEST_V2} - ).raise_for_status() - - content_response = cm.exception.response - self.assertEqual(content_response.status_code, 401) - - authenticate_header = content_response.headers["Www-Authenticate"] - queries = AuthenticationHeaderQueries(authenticate_header) - content_response = requests.get( - queries.realm, params={"service": queries.service, "scope": queries.scope} - ) - content_response.raise_for_status() + auth = get_auth_for_url(latest_image_url) content_response = requests.get( - latest_image_url, - auth=BearerTokenAuth(content_response.json()["token"]), - headers={"Accept": MEDIA_TYPE.MANIFEST_V2}, + latest_image_url, auth=auth, headers={"Accept": MEDIA_TYPE.MANIFEST_V2} ) content_response.raise_for_status() self.compare_config_blob_digests(content_response.json()["config"]["digest"]) diff --git a/pulp_container/tests/functional/constants.py b/pulp_container/tests/functional/constants.py index b02f8cf69..80af83f91 100644 --- a/pulp_container/tests/functional/constants.py +++ b/pulp_container/tests/functional/constants.py @@ -1,6 +1,7 @@ # coding=utf-8 from urllib.parse import urljoin +from pulp_smash import config from pulp_smash.constants import PULP_FIXTURES_BASE_URL from pulp_smash.pulp3.constants import ( BASE_DISTRIBUTION_PATH, @@ -26,6 +27,10 @@ CONTAINER_IMAGE_URL = urljoin(PULP_FIXTURES_BASE_URL, "container/busybox:latest.tar") """The URL to a Container image as created by ``docker save``.""" +PULP_CONTENT_HOST_BASE_URL = config.get_config().get_content_host_base_url() + +PULP_CONTENT_BASE_URL = urljoin(PULP_CONTENT_HOST_BASE_URL, "pulp/content/") + # hello-world is the smalest container image available on docker hub 1.84kB REPO_UPSTREAM_NAME = "hello-world" """The name of a Container repository. diff --git a/pulp_container/tests/functional/utils.py b/pulp_container/tests/functional/utils.py index 65406e900..7d5f01820 100644 --- a/pulp_container/tests/functional/utils.py +++ b/pulp_container/tests/functional/utils.py @@ -1,5 +1,6 @@ # coding=utf-8 """Utilities for tests for the container plugin.""" +import pytest import requests from requests.auth import AuthBase @@ -264,3 +265,25 @@ def gen_artifact(url=CONTAINER_IMAGE_URL): temp_file.write(response.content) artifact = ArtifactsApi(core_client).create(file=temp_file.name) return artifact.to_dict() + + +def get_auth_for_url(latest_image_url): + """Return authentication details based on the the status of token authentication.""" + if TOKEN_AUTH_DISABLED: + auth = () + else: + with pytest.raises(requests.HTTPError): + response = requests.get(latest_image_url) + response.raise_for_status() + assert response.status_code == 401 + + authenticate_header = response.headers["WWW-Authenticate"] + queries = AuthenticationHeaderQueries(authenticate_header) + content_response = requests.get( + queries.realm, params={"service": queries.service, "scope": queries.scope} + ) + content_response.raise_for_status() + token = content_response.json()["token"] + auth = BearerTokenAuth(token) + + return auth