Skip to content

Fix 404 on tag fetch due to overly strict Accept header matching #2424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gerrod3 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix 404 on tag fetch due to overly strict Accept header matching #2424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/2417.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
9 changes: 5 additions & 4 deletions pulp_container/app/registry_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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:
Expand Down
25 changes: 22 additions & 3 deletions pulp_container/app/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
71 changes: 69 additions & 2 deletions pulp_container/tests/functional/api/test_domains.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
Loading