diff --git a/CHANGES/2417.bugfix b/CHANGES/2417.bugfix new file mode 100644 index 000000000..6d394c6ee --- /dev/null +++ b/CHANGES/2417.bugfix @@ -0,0 +1 @@ +Fixed tag-based manifest fetches returning 404 when the client Accept header included q-values or when the manifest media type did not exactly match the parsed header value. diff --git a/pulp_container/app/registry_api.py b/pulp_container/app/registry_api.py index 616522d93..409476842 100644 --- a/pulp_container/app/registry_api.py +++ b/pulp_container/app/registry_api.py @@ -95,6 +95,7 @@ get_accepted_media_types, get_full_path, has_task_completed, + is_media_type_accepted, validate_manifest, ) from pulp_container.constants import ( @@ -166,11 +167,11 @@ def from_tag(cls, tag, request=None): manifest_media_type = manifest.media_type if request: accepted_media_types = get_accepted_media_types(request.headers) - if ( - manifest_media_type not in accepted_media_types - and manifest_media_type != MEDIA_TYPE.MANIFEST_V1 + if accepted_media_types and not is_media_type_accepted( + accepted_media_types, manifest_media_type ): - raise ManifestNotFound(reference=tag.name) + if manifest_media_type != MEDIA_TYPE.MANIFEST_V1: + raise ManifestNotFound(reference=tag.name) # Schema v1 manifests are always returned with the signed content type if manifest_media_type == MEDIA_TYPE.MANIFEST_V1: diff --git a/pulp_container/app/utils.py b/pulp_container/app/utils.py index d578bd62e..fee533acb 100644 --- a/pulp_container/app/utils.py +++ b/pulp_container/app/utils.py @@ -42,6 +42,12 @@ def get_full_path(base_path, pulp_domain=None): return base_path +def _parse_accept_entry(value): + """Return the media type from an Accept header entry, without parameters such as q-values.""" + media_type = value.strip().split(";", maxsplit=1)[0].strip() + return media_type or None + + def get_accepted_media_types(headers): """ Returns a list of media types from the Accept headers. @@ -56,12 +62,25 @@ def get_accepted_media_types(headers): """ accepted_media_types = [] for header, values in headers.items(): - if header == "Accept": - values = [v.strip() for v in values.split(",")] - accepted_media_types.extend(values) + if header.lower() == "accept": + for value in values.split(","): + if media_type := _parse_accept_entry(value): + accepted_media_types.append(media_type) return accepted_media_types +def is_media_type_accepted(accepted_media_types, media_type): + """ + Return whether a manifest media type satisfies an Accept header. + + Supports exact media type matches and the ``*/*`` wildcard used by some clients. + """ + for accepted in accepted_media_types: + if accepted in ("*/*", media_type): + return True + return False + + def urlpath_sanitize(*args): """ Join an arbitrary number of strings into a /-separated path. diff --git a/pulp_container/tests/functional/api/test_domains.py b/pulp_container/tests/functional/api/test_domains.py index e90cb32db..c78241de5 100644 --- a/pulp_container/tests/functional/api/test_domains.py +++ b/pulp_container/tests/functional/api/test_domains.py @@ -1,17 +1,22 @@ import uuid +from contextlib import suppress from subprocess import CalledProcessError +from urllib.parse import urljoin import pytest +import requests +from pulp_container.constants import MEDIA_TYPE from pulp_container.tests.functional.constants import ( PULP_FIXTURE_1, PULP_HELLO_WORLD_REPO, REGISTRY_V2_REPO_PULP, ) +from pulp_container.tests.functional.utils import get_auth_for_url @pytest.fixture -def cdomain_factory(domain_factory, pulpcore_bindings): +def cdomain_factory(domain_factory, pulpcore_bindings, container_bindings, monitor_task): domains = [] def _domain_factory(*args, **kwargs): @@ -22,9 +27,18 @@ def _domain_factory(*args, **kwargs): yield _domain_factory for domain in domains: + namespaces = container_bindings.PulpContainerNamespacesApi.list( + pulp_domain=domain.name + ).results + for namespace in namespaces: + with suppress(Exception): + response = container_bindings.PulpContainerNamespacesApi.delete(namespace.pulp_href) + monitor_task(response.task) + guards = pulpcore_bindings.ContentguardsContentRedirectApi.list(pulp_domain=domain.name) for guard in guards.results: - pulpcore_bindings.ContentguardsContentRedirectApi.delete(guard.pulp_href) + with suppress(Exception): + pulpcore_bindings.ContentguardsContentRedirectApi.delete(guard.pulp_href) def test_push_in_domain( @@ -90,6 +104,59 @@ def test_pull_in_domain( local_registry.pull(local_path) +@pytest.mark.parallel +def test_domain_on_demand_manifest_fetch_by_tag_and_digest( + cdomain_factory, + container_repository_factory, + container_remote_factory, + container_sync, + container_distribution_factory, + container_bindings, + bindings_cfg, + pulp_settings, +): + """Tag- and digest-based manifest fetches must both succeed for on-demand repos in a domain.""" + if not pulp_settings.DOMAIN_ENABLED: + pytest.skip("This test requires domains to be enabled.") + + domain = cdomain_factory() + repo = container_repository_factory(pulp_domain=domain.name) + remote = container_remote_factory( + pulp_domain=domain.name, + upstream_name=PULP_FIXTURE_1, + policy="on_demand", + ) + container_sync(repo, remote) + repo = container_bindings.RepositoriesContainerApi.read(repo.pulp_href) + distribution = container_distribution_factory( + repository=repo.pulp_href, pulp_domain=domain.name + ) + + manifest_list_tag = None + for tag in container_bindings.ContentTagsApi.list( + repository_version=repo.latest_version_href, pulp_domain=domain.name + ).results: + manifest = container_bindings.ContentManifestsApi.read(tag.tagged_manifest) + if manifest.media_type in (MEDIA_TYPE.MANIFEST_LIST, MEDIA_TYPE.INDEX_OCI): + manifest_list_tag = tag + break + assert manifest_list_tag is not None, "Expected a synced manifest list tag in the fixture repo." + + manifest = container_bindings.ContentManifestsApi.read(manifest_list_tag.tagged_manifest) + accept = f"{MEDIA_TYPE.MANIFEST_LIST};q=0.8, {MEDIA_TYPE.MANIFEST_V2};q=0.9" + base_path = f"/v2/{domain.name}/{distribution.base_path}" + + tag_url = urljoin(bindings_cfg.host, f"{base_path}/manifests/{manifest_list_tag.name}") + digest_url = urljoin(bindings_cfg.host, f"{base_path}/manifests/{manifest.digest}") + auth = get_auth_for_url(tag_url) + + tag_response = requests.get(tag_url, auth=auth, headers={"Accept": accept}) + assert tag_response.status_code == 200, tag_response.text + + digest_response = requests.get(digest_url, auth=auth, headers={"Accept": accept}) + assert digest_response.status_code == 200, digest_response.text + + def test_domain_permissions( cdomain_factory, container_bindings,