From 855d6b9102659f6fb7f00de5cbf77f419010b255 Mon Sep 17 00:00:00 2001 From: Yasen Date: Wed, 29 Jul 2026 10:52:37 +0200 Subject: [PATCH 1/3] Add ContentView resource with cross-domain scatter/gather utility Introduces a new first-class Pulp resource, ContentView: a named, domain-scoped object composed of Distributions that may span multiple domains, with full CRUD and RBAC. Plugins build cross-domain search endpoints on top of it using the new resolve_content_view_distributions/ group_versions_by_domain/scatter_gather utilities, exposed via pulpcore.plugin.*, without querying the database directly or bypassing RBAC. Co-authored-by: Cursor --- CHANGES/6001.feature | 1 + CHANGES/plugin_api/6001.feature | 1 + pulpcore/app/migrations/0155_contentview.py | 57 ++++ pulpcore/app/models/__init__.py | 6 + pulpcore/app/models/content_view.py | 52 +++ pulpcore/app/serializers/__init__.py | 4 + pulpcore/app/serializers/content_view.py | 106 ++++++ pulpcore/app/util_content_view.py | 209 ++++++++++++ pulpcore/app/viewsets/__init__.py | 1 + pulpcore/app/viewsets/content_view.py | 103 ++++++ pulpcore/plugin/models/__init__.py | 2 + pulpcore/plugin/serializers/__init__.py | 4 + pulpcore/plugin/util.py | 12 + pulpcore/plugin/viewsets/__init__.py | 4 + pulpcore/tests/unit/test_content_view.py | 339 ++++++++++++++++++++ 15 files changed, 901 insertions(+) create mode 100644 CHANGES/6001.feature create mode 100644 CHANGES/plugin_api/6001.feature create mode 100644 pulpcore/app/migrations/0155_contentview.py create mode 100644 pulpcore/app/models/content_view.py create mode 100644 pulpcore/app/serializers/content_view.py create mode 100644 pulpcore/app/util_content_view.py create mode 100644 pulpcore/app/viewsets/content_view.py create mode 100644 pulpcore/tests/unit/test_content_view.py diff --git a/CHANGES/6001.feature b/CHANGES/6001.feature new file mode 100644 index 00000000000..73cfb18672c --- /dev/null +++ b/CHANGES/6001.feature @@ -0,0 +1 @@ +Added the ``ContentView`` resource: a named, persistable scope composed of Distributions -- potentially spanning multiple domains -- with full CRUD and RBAC. This lets plugins implement RBAC-respecting, cross-domain search over the content served by those Distributions without querying the database directly or passing raw repository version hrefs on every request. diff --git a/CHANGES/plugin_api/6001.feature b/CHANGES/plugin_api/6001.feature new file mode 100644 index 00000000000..2ed4ffaa247 --- /dev/null +++ b/CHANGES/plugin_api/6001.feature @@ -0,0 +1 @@ +Added ``ContentView`` to ``pulpcore.plugin.models`` and ``ContentViewViewSet``/``ContentViewFilter`` to ``pulpcore.plugin.viewsets``, along with ``resolve_content_view_distributions``, ``group_versions_by_domain``, ``scatter_gather``, and ``user_can_view_domain`` in ``pulpcore.plugin.util`` (plus ``with_domain``, now also re-exported there). Together these let a plugin implement its own nested, RBAC-respecting, cross-domain search endpoints under a ``ContentView``. diff --git a/pulpcore/app/migrations/0155_contentview.py b/pulpcore/app/migrations/0155_contentview.py new file mode 100644 index 00000000000..45bba64506b --- /dev/null +++ b/pulpcore/app/migrations/0155_contentview.py @@ -0,0 +1,57 @@ +# Generated by Django 5.2.14 on 2026-07-29 00:00 + +import django.contrib.postgres.fields.hstore +import django.db.models.deletion +import django_lifecycle.mixins +from django.db import migrations, models + +import pulpcore.app.models.base +import pulpcore.app.util + + +class Migration(migrations.Migration): + dependencies = [ + ("core", "0154_task_api_version"), + ] + + operations = [ + migrations.CreateModel( + name="ContentView", + fields=[ + ( + "pulp_id", + models.UUIDField( + default=pulpcore.app.models.base.pulp_uuid, + editable=False, + primary_key=True, + serialize=False, + ), + ), + ("pulp_created", models.DateTimeField(auto_now_add=True)), + ("pulp_last_updated", models.DateTimeField(auto_now=True, null=True)), + ("name", models.TextField(db_index=True)), + ("description", models.TextField(null=True)), + ("pulp_labels", django.contrib.postgres.fields.hstore.HStoreField(default=dict)), + ( + "distributions", + models.ManyToManyField(related_name="content_views", to="core.distribution"), + ), + ( + "pulp_domain", + models.ForeignKey( + default=pulpcore.app.util.get_domain_pk, + on_delete=django.db.models.deletion.PROTECT, + to="core.domain", + ), + ), + ], + options={ + "abstract": False, + "permissions": [ + ("manage_roles_contentview", "Can manage role assignments on content view") + ], + "unique_together": {("name", "pulp_domain")}, + }, + bases=(django_lifecycle.mixins.LifecycleModelMixin, models.Model), + ), + ] diff --git a/pulpcore/app/models/__init__.py b/pulpcore/app/models/__init__.py index a1bf1ca1147..a0e88c377d2 100644 --- a/pulpcore/app/models/__init__.py +++ b/pulpcore/app/models/__init__.py @@ -15,6 +15,11 @@ Group, ) +# Must be imported before any module that imports `pulpcore.plugin.models` (which re-exports +# ContentView), e.g. `.replica` below -- otherwise that triggers a circular import back into +# this partially-initialized module. +from .content_view import ContentView + from .domain import Domain from .acs import AlternateContentSource, AlternateContentSourcePath @@ -166,6 +171,7 @@ "GroupProgressReport", "ProgressReport", "UpstreamPulp", + "ContentView", "OpenPGPDistribution", "OpenPGPKeyring", "OpenPGPPublicKey", diff --git a/pulpcore/app/models/content_view.py b/pulpcore/app/models/content_view.py new file mode 100644 index 00000000000..ff4c8f85941 --- /dev/null +++ b/pulpcore/app/models/content_view.py @@ -0,0 +1,52 @@ +""" +Check `Plugin Writer's Guide`_ for more details. + +Plugin Writer's Guide: +https://pulpproject.org/pulpcore/docs/dev/learn/plugin-concepts/ +""" + +from django.contrib.postgres.fields import HStoreField +from django.db import models + +from pulpcore.app.models import AutoAddObjPermsMixin, BaseModel +from pulpcore.app.util import get_domain_pk + + +class ContentView(BaseModel, AutoAddObjPermsMixin): + """ + A named, persistable scope composed of Distributions, searchable across domains. + + A ContentView lets API clients search across the content served by many Distributions -- + which may span domains other than the ContentView's own -- without passing raw lists of + repository version hrefs on every request, and without bypassing Pulp's RBAC by querying + the database directly. Each linked Distribution already carries version-tracking semantics + (it can point to a Repository to track its latest version, a pinned RepositoryVersion, or a + Publication), so the ContentView itself only needs to store *which* Distributions are in + scope; resolving them to concrete RepositoryVersions happens at query time. + + Fields: + name (models.TextField): The content view's name, unique within its domain. + description (models.TextField): Optional human-readable description. + pulp_labels (HStoreField): Dictionary of string values. + + Relations: + pulp_domain (models.ForeignKey): The domain this ContentView is stored in. Standard + domain-scoped resource: read/update/delete is governed by RBAC on the ContentView + itself, same as any other Pulp resource. + distributions (models.ManyToManyField): Distributions this ContentView searches across. + These may belong to any domain the referencing user has read access to at the time + they are added -- not just the ContentView's own domain -- which is what makes + cross-domain search possible. + """ + + name = models.TextField(db_index=True) + description = models.TextField(null=True) + pulp_labels = HStoreField(default=dict) + pulp_domain = models.ForeignKey("Domain", default=get_domain_pk, on_delete=models.PROTECT) + distributions = models.ManyToManyField("Distribution", related_name="content_views") + + class Meta: + unique_together = ("name", "pulp_domain") + permissions = [ + ("manage_roles_contentview", "Can manage role assignments on content view"), + ] diff --git a/pulpcore/app/serializers/__init__.py b/pulpcore/app/serializers/__init__.py index f0a99ee4663..5e8fa65e3c4 100644 --- a/pulpcore/app/serializers/__init__.py +++ b/pulpcore/app/serializers/__init__.py @@ -58,6 +58,10 @@ SigningServiceSerializer, SingleArtifactContentSerializer, ) +from .content_view import ( + ContentViewDistributionStatusSerializer, + ContentViewSerializer, +) from .domain import DomainSerializer, DomainBackendMigratorSerializer from .exporter import ( ExporterSerializer, diff --git a/pulpcore/app/serializers/content_view.py b/pulpcore/app/serializers/content_view.py new file mode 100644 index 00000000000..c9360f1c4e7 --- /dev/null +++ b/pulpcore/app/serializers/content_view.py @@ -0,0 +1,106 @@ +from gettext import gettext as _ + +from rest_framework import serializers + +from pulpcore.app import models +from pulpcore.app.serializers import ( + DetailRelatedField, + DomainUniqueValidator, + IdentityField, + ModelSerializer, + RepositoryVersionRelatedField, + pulp_labels_validator, +) +from pulpcore.app.util_content_view import resolve_content_view_distributions + + +class ContentViewDistributionStatusSerializer(serializers.Serializer): + """Per-distribution resolution status, shown on the ContentView detail/list endpoints.""" + + distribution = DetailRelatedField( + read_only=True, + view_name_pattern=r"distributions(-.*/.*)?-detail", + help_text=_("The distribution this status entry describes."), + ) + domain = serializers.CharField( + source="domain.name", help_text=_("The name of the domain the distribution belongs to.") + ) + status = serializers.ChoiceField( + choices=["ok", "no_domain_access", "no_version"], + help_text=_( + "'ok' if the distribution currently resolves to a repository version the caller can " + "search; 'no_domain_access' if the caller does not (or no longer) have read access " + "to the distribution's domain; 'no_version' if the distribution or the repository " + "version/publication it pointed to has been deleted." + ), + ) + repository_version = RepositoryVersionRelatedField( + read_only=True, + allow_null=True, + queryset=None, + help_text=_("The repository version currently resolved for this distribution, if any."), + ) + + +class ContentViewSerializer(ModelSerializer): + """ + Serializer for a ContentView -- a named, persistable scope composed of Distributions that + may span multiple domains, used to search across their content without exposing raw + repository version hrefs on every request. + """ + + # Distributions referenced by a ContentView may legitimately live in a domain other than + # the ContentView's own -- that's the entire point of this resource -- so the default + # same-domain cross-field validation (ValidateFieldsMixin.check_cross_domains) must not + # apply here. + CHECK_SAME_DOMAIN = False + + pulp_href = IdentityField(view_name="content-views-detail") + + name = serializers.CharField( + help_text=_("A unique name for this content view."), + validators=[DomainUniqueValidator(queryset=models.ContentView.objects.all())], + ) + description = serializers.CharField( + help_text=_("An optional description of this content view."), + required=False, + allow_null=True, + ) + pulp_labels = serializers.HStoreField(required=False, validators=[pulp_labels_validator]) + distributions = DetailRelatedField( + many=True, + required=False, + queryset=models.Distribution.objects.all(), + view_name_pattern=r"distributions(-.*/.*)?-detail", + help_text=_( + "Distributions this content view searches across. May reference distributions " + "belonging to any domain the user has read access to, not just this content view's " + "own domain." + ), + ) + distributions_status = serializers.SerializerMethodField( + help_text=_( + "Per-distribution resolution status: whether each linked distribution's domain is " + "currently accessible and whether it resolves to a repository version." + ) + ) + + def get_distributions_status(self, obj): + request = self.context.get("request") + user = getattr(request, "user", None) if request else None + if user is None: + return [] + resolutions = resolve_content_view_distributions(obj, user) + return ContentViewDistributionStatusSerializer( + resolutions, many=True, context=self.context + ).data + + class Meta: + model = models.ContentView + fields = ModelSerializer.Meta.fields + ( + "name", + "description", + "pulp_labels", + "distributions", + "distributions_status", + ) diff --git a/pulpcore/app/util_content_view.py b/pulpcore/app/util_content_view.py new file mode 100644 index 00000000000..fdf0ec6b26d --- /dev/null +++ b/pulpcore/app/util_content_view.py @@ -0,0 +1,209 @@ +""" +Utilities for resolving a ContentView's Distributions into RepositoryVersions grouped by +domain, and for executing cross-domain "scatter/gather" queries against them. + +These are the pieces a plugin (e.g. pulp_rpm) composes to implement its own nested +``content-views/{uuid}/search/...`` endpoints on top of the generic ``ContentView`` resource: + + resolutions = resolve_content_view_distributions(content_view, request.user) + versions_by_domain = group_versions_by_domain(resolutions) + page, total = scatter_gather( + versions_by_domain, + build_queryset=lambda versions: Package.objects.filter( + pk__in=functools.reduce(operator.or_, (v.content for v in versions)) + ).order_by("name"), + order_by=("name",), + limit=limit, + offset=offset, + ) + +See the Plugin Writer's Guide for more on plugin/pulpcore boundaries: +https://pulpproject.org/pulpcore/docs/dev/learn/plugin-concepts/ +""" + +from collections import defaultdict +from dataclasses import dataclass +from typing import Any, Callable, Optional + +from pulpcore.app.contexts import with_domain + +# Distribution resolution statuses. +STATUS_OK = "ok" +STATUS_NO_DOMAIN_ACCESS = "no_domain_access" +STATUS_NO_VERSION = "no_version" + + +def user_can_view_domain(user, domain): + """ + Returns True if ``user`` has read (view) access to ``domain``. + + This mirrors the same model/domain/object permission check used by + ``pulpcore.app.global_access_conditions.has_domain_perms``, generalized to an arbitrary + domain instead of only the current request's domain. This is what allows a ContentView to + reference Distributions living in other domains while still enforcing RBAC, per-domain, at + query time -- rather than at write time only. + + Deployments with a custom domain-membership model (e.g. an org-based permission backend) + can make this reflect their own semantics by registering a Django authentication backend + whose ``has_perm(user, "core.view_domain", obj=domain)`` answers accordingly; no changes to + pulpcore or its plugins are required for that. + """ + if user is None or not getattr(user, "is_authenticated", False): + return False + if user.is_superuser: + return True + return user.has_perm("core.view_domain") or user.has_perm("core.view_domain", obj=domain) + + +@dataclass +class DistributionResolution: + """The result of resolving a single Distribution linked to a ContentView.""" + + distribution: Any + domain: Any + repository_version: Optional[Any] + status: str + + +def resolve_content_view_distributions(content_view, user): + """ + Resolve every Distribution linked to ``content_view`` to its current RepositoryVersion, + classifying each one by accessibility/staleness. Never raises: distributions whose domain + is no longer accessible, or that no longer resolve to a version (deleted distribution or + repository version), are returned with a status instead of causing an error -- callers + (search endpoints, and the ContentView detail serializer) decide how to surface that. + + Args: + content_view (pulpcore.app.models.ContentView): The content view to resolve. + user: The requesting user, used for the per-domain accessibility check. + + Returns: + list[DistributionResolution] + """ + resolutions = [] + for distribution in content_view.distributions.select_related("pulp_domain").all(): + domain = distribution.pulp_domain + if not user_can_view_domain(user, domain): + resolutions.append( + DistributionResolution(distribution, domain, None, STATUS_NO_DOMAIN_ACCESS) + ) + continue + + _repository, repository_version, _publication = ( + distribution.cast().get_repository_publication_and_version() + ) + if repository_version is None: + resolutions.append( + DistributionResolution(distribution, domain, None, STATUS_NO_VERSION) + ) + else: + resolutions.append( + DistributionResolution(distribution, domain, repository_version, STATUS_OK) + ) + return resolutions + + +def group_versions_by_domain(resolutions): + """ + Group the RepositoryVersions of "ok" resolutions by domain. + + This is the input scatter_gather (or a plugin's own equivalent loop) consumes -- lost-access + and stale/deleted distributions (any non-"ok" status) are already excluded here, satisfying + the "search silently excludes inaccessible distributions" requirement. + + Args: + resolutions (list[DistributionResolution]): + + Returns: + dict: Domain -> list[RepositoryVersion] + """ + by_domain = defaultdict(list) + for resolution in resolutions: + if resolution.status == STATUS_OK: + by_domain[resolution.domain].append(resolution.repository_version) + return dict(by_domain) + + +def _sort_key(fields): + def key(row): + return tuple( + ( + (value := (row[field] if isinstance(row, dict) else getattr(row, field))) is None, + value, + ) + for field in fields + ) + + return key + + +def scatter_gather( + versions_by_domain: dict, + build_queryset: Callable[[list], Any], + *, + order_by, + limit: int, + offset: int = 0, + descending: bool = False, + count: bool = True, +): + """ + Execute ``build_queryset(versions)`` once per domain, inside that domain's routing context + (``with_domain``), and merge the results into a single page. + + For the common case -- a ContentView resolving to a single domain -- this executes exactly + one query with native ``ORDER BY``/``LIMIT``/``OFFSET`` (plus one ``COUNT`` if requested), + identical in cost to a single-domain query. For multiple domains, each domain's queryset is + over-fetched to ``limit + offset`` rows (sufficient, since a single domain can supply at most + the entire final page), concatenated, re-sorted in Python, and sliced -- bounding worst-case + cost to ``len(versions_by_domain) * (limit + offset)`` rather than the full dataset size. + + Args: + versions_by_domain (dict): Domain -> list[RepositoryVersion], as returned by + ``group_versions_by_domain``. + build_queryset (callable): Given a list of RepositoryVersions (all belonging to the same + domain), returns a QuerySet already filtered *and ordered* by ``order_by`` + (ascending), but not sliced. May return a ``.values()``/``.values_list()`` queryset. + order_by (str or tuple[str]): One or more field names (without ``-`` prefixes) that + ``build_queryset`` already orders by; used here to merge-sort rows fetched from + multiple domains. Must match the ordering ``build_queryset`` applies. + limit (int): Maximum number of rows to return. + offset (int): Number of rows to skip. + descending (bool): Whether the ordering above is descending. Applies uniformly to all + ``order_by`` fields (sufficient for every current use case -- callers needing mixed + per-field directions should pre-negate/annotate instead). + count (bool): If True, also compute an exact total count across all domains. + + Returns: + tuple: ``(page, total)``. ``page`` is a list of model instances (or dicts, if + ``build_queryset`` uses ``.values()``/``.values_list()``) of length <= ``limit``. + ``total`` is an int, or None if ``count=False`` (mirroring tang's typeahead search + endpoints, which never compute a total). + """ + fields = (order_by,) if isinstance(order_by, str) else tuple(order_by) + domains = list(versions_by_domain.items()) + + if not domains: + return [], (0 if count else None) + + if len(domains) == 1: + domain, versions = domains[0] + with with_domain(domain): + qs = build_queryset(versions) + total = qs.count() if count else None + page = list(qs[offset : offset + limit]) + return page, total + + rows = [] + total = 0 if count else None + fetch_bound = limit + offset + for domain, versions in domains: + with with_domain(domain): + qs = build_queryset(versions) + if count: + total += qs.count() + rows.extend(qs[:fetch_bound]) + + rows.sort(key=_sort_key(fields), reverse=descending) + page = rows[offset : offset + limit] + return page, total diff --git a/pulpcore/app/viewsets/__init__.py b/pulpcore/app/viewsets/__init__.py index 379ee75c7fc..abb2a2d0734 100644 --- a/pulpcore/app/viewsets/__init__.py +++ b/pulpcore/app/viewsets/__init__.py @@ -25,6 +25,7 @@ ReadOnlyContentViewSet, SigningServiceViewSet, ) +from .content_view import ContentViewFilter, ContentViewViewSet from .custom_filters import ( RepoVersionHrefPrnFilter, RepositoryVersionFilter, diff --git a/pulpcore/app/viewsets/content_view.py b/pulpcore/app/viewsets/content_view.py new file mode 100644 index 00000000000..6c92703ab1c --- /dev/null +++ b/pulpcore/app/viewsets/content_view.py @@ -0,0 +1,103 @@ +from rest_framework import mixins + +from pulpcore.app.models import ContentView +from pulpcore.app.serializers import ContentViewSerializer +from pulpcore.app.viewsets import LabelsMixin, NamedModelViewSet, RolesMixin +from pulpcore.app.viewsets.base import NAME_FILTER_OPTIONS +from pulpcore.app.viewsets.custom_filters import LabelFilter +from pulpcore.filters import BaseFilterSet + + +class ContentViewFilter(BaseFilterSet): + """FilterSet for ContentView.""" + + pulp_label_select = LabelFilter() + + class Meta: + model = ContentView + fields = {"name": NAME_FILTER_OPTIONS} + + +class ContentViewViewSet( + NamedModelViewSet, + mixins.CreateModelMixin, + mixins.RetrieveModelMixin, + mixins.ListModelMixin, + mixins.UpdateModelMixin, + mixins.DestroyModelMixin, + RolesMixin, + LabelsMixin, +): + """ + ViewSet for ContentView. + + A ContentView is a named, persistable scope composed of Distributions -- potentially + spanning multiple domains -- that plugins can search across (see each plugin's + ``content-views/{content_view_pk}/search/...`` nested endpoints for the actual search + operations; this viewset only provides the standard CRUD lifecycle for the resource itself). + """ + + queryset = ContentView.objects.all() + endpoint_name = "content-views" + serializer_class = ContentViewSerializer + filterset_class = ContentViewFilter + ordering = "-pulp_created" + queryset_filtering_required_permission = "core.view_contentview" + + DEFAULT_ACCESS_POLICY = { + "statements": [ + { + "action": ["list", "my_permissions"], + "principal": "authenticated", + "effect": "allow", + }, + { + "action": ["create"], + "principal": "authenticated", + "effect": "allow", + "condition": "has_model_or_domain_perms:core.add_contentview", + }, + { + "action": ["retrieve"], + "principal": "authenticated", + "effect": "allow", + "condition": "has_model_or_domain_or_obj_perms:core.view_contentview", + }, + { + "action": ["update", "partial_update", "set_label", "unset_label"], + "principal": "authenticated", + "effect": "allow", + "condition": "has_model_or_domain_or_obj_perms:core.change_contentview", + }, + { + "action": ["destroy"], + "principal": "authenticated", + "effect": "allow", + "condition": "has_model_or_domain_or_obj_perms:core.delete_contentview", + }, + { + "action": ["list_roles", "add_role", "remove_role"], + "principal": "authenticated", + "effect": "allow", + "condition": "has_model_or_domain_or_obj_perms:core.manage_roles_contentview", + }, + ], + "creation_hooks": [ + { + "function": "add_roles_for_object_creator", + "parameters": {"roles": "core.contentview_owner"}, + }, + ], + "queryset_scoping": {"function": "scope_queryset"}, + } + + LOCKED_ROLES = { + "core.contentview_creator": ["core.add_contentview"], + "core.contentview_owner": [ + "core.view_contentview", + "core.change_contentview", + "core.delete_contentview", + "core.manage_roles_contentview", + ], + "core.contentview_viewer": ["core.view_contentview"], + } diff --git a/pulpcore/plugin/models/__init__.py b/pulpcore/plugin/models/__init__.py index ac699d1cb7d..668c1a38277 100644 --- a/pulpcore/plugin/models/__init__.py +++ b/pulpcore/plugin/models/__init__.py @@ -16,6 +16,7 @@ ContentManager, ContentGuard, ContentRedirectContentGuard, + ContentView, CreatedResource, Distribution, Domain, @@ -63,6 +64,7 @@ "ContentManager", "ContentGuard", "ContentRedirectContentGuard", + "ContentView", "CreatedResource", "Distribution", "Domain", diff --git a/pulpcore/plugin/serializers/__init__.py b/pulpcore/plugin/serializers/__init__.py index 450442fea91..3841eafa1fe 100644 --- a/pulpcore/plugin/serializers/__init__.py +++ b/pulpcore/plugin/serializers/__init__.py @@ -9,6 +9,8 @@ ContentChecksumSerializer, ContentGuardSerializer, ContentRedirectContentGuardSerializer, + ContentViewDistributionStatusSerializer, + ContentViewSerializer, DetailRelatedField, DistributionSerializer, DomainUniqueValidator, @@ -61,6 +63,8 @@ "ContentChecksumSerializer", "ContentGuardSerializer", "ContentRedirectContentGuardSerializer", + "ContentViewDistributionStatusSerializer", + "ContentViewSerializer", "DetailRelatedField", "DistributionSerializer", "DomainUniqueValidator", diff --git a/pulpcore/plugin/util.py b/pulpcore/plugin/util.py index b8e63db206f..908d1678b1d 100644 --- a/pulpcore/plugin/util.py +++ b/pulpcore/plugin/util.py @@ -1,3 +1,4 @@ +from pulpcore.app.contexts import with_domain from pulpcore.app.role_util import ( assign_role, get_groups_with_perms, @@ -30,8 +31,19 @@ set_current_user, set_domain, ) +from pulpcore.app.util_content_view import ( + group_versions_by_domain, + resolve_content_view_distributions, + scatter_gather, + user_can_view_domain, +) __all__ = [ + "with_domain", + "group_versions_by_domain", + "resolve_content_view_distributions", + "scatter_gather", + "user_can_view_domain", "assign_role", "get_groups_with_perms", "get_groups_with_perms_attached_perms", diff --git a/pulpcore/plugin/viewsets/__init__.py b/pulpcore/plugin/viewsets/__init__.py index 437af8144ba..62832a7aaae 100644 --- a/pulpcore/plugin/viewsets/__init__.py +++ b/pulpcore/plugin/viewsets/__init__.py @@ -13,6 +13,8 @@ ContentGuardFilter, ContentGuardViewSet, ContentViewSet, + ContentViewFilter, + ContentViewViewSet, DATETIME_FILTER_OPTIONS, DistributionFilter, DistributionViewSet, @@ -64,6 +66,8 @@ "ContentGuardFilter", "ContentGuardViewSet", "ContentViewSet", + "ContentViewFilter", + "ContentViewViewSet", "DATETIME_FILTER_OPTIONS", "DistributionFilter", "DistributionViewSet", diff --git a/pulpcore/tests/unit/test_content_view.py b/pulpcore/tests/unit/test_content_view.py new file mode 100644 index 00000000000..b3e547bd652 --- /dev/null +++ b/pulpcore/tests/unit/test_content_view.py @@ -0,0 +1,339 @@ +from uuid import uuid4 + +import pytest +from django.contrib.auth import get_user_model +from django.contrib.auth.models import AnonymousUser +from django.db import IntegrityError + +from pulpcore.app.models import ContentView, Distribution, Domain, Repository +from pulpcore.app.util_content_view import ( + STATUS_NO_DOMAIN_ACCESS, + STATUS_NO_VERSION, + STATUS_OK, + group_versions_by_domain, + resolve_content_view_distributions, + scatter_gather, + user_can_view_domain, +) + + +@pytest.fixture +def domain(db): + return Domain.objects.create(name=str(uuid4())) + + +@pytest.fixture +def other_domain(db): + return Domain.objects.create(name=str(uuid4())) + + +@pytest.fixture +def repository(domain): + return Repository.objects.create(name=str(uuid4()), pulp_domain=domain) + + +@pytest.fixture +def other_repository(other_domain): + return Repository.objects.create(name=str(uuid4()), pulp_domain=other_domain) + + +def make_user(superuser=False): + return get_user_model().objects.create(username=str(uuid4()), is_superuser=superuser) + + +class TestContentViewModel: + def test_create_defaults(self, domain): + content_view = ContentView.objects.create(name="cv1", pulp_domain=domain) + + assert content_view.pulp_domain == domain + assert content_view.pulp_labels == {} + assert content_view.description is None + assert list(content_view.distributions.all()) == [] + + def test_name_unique_per_domain_only(self, domain, other_domain): + ContentView.objects.create(name="cv1", pulp_domain=domain) + # Same name is fine in a different domain. + ContentView.objects.create(name="cv1", pulp_domain=other_domain) + + with pytest.raises(IntegrityError): + ContentView.objects.create(name="cv1", pulp_domain=domain) + + def test_distributions_m2m_is_bidirectional(self, domain, repository): + content_view = ContentView.objects.create(name="cv1", pulp_domain=domain) + distribution = Distribution.objects.create( + name="dist1", base_path="dist1", pulp_domain=domain, repository=repository + ) + + content_view.distributions.add(distribution) + + assert list(content_view.distributions.all()) == [distribution] + assert list(distribution.content_views.all()) == [content_view] + + def test_distributions_may_belong_to_other_domains( + self, domain, other_domain, other_repository + ): + content_view = ContentView.objects.create(name="cv1", pulp_domain=domain) + foreign_distribution = Distribution.objects.create( + name="dist1", base_path="dist1", pulp_domain=other_domain, repository=other_repository + ) + + # No FK/domain constraint prevents linking a distribution from a different domain. + content_view.distributions.add(foreign_distribution) + + assert list(content_view.distributions.all()) == [foreign_distribution] + + +class TestUserCanViewDomain: + def test_none_user_denied(self, domain): + assert user_can_view_domain(None, domain) is False + + def test_unauthenticated_user_denied(self, domain): + assert user_can_view_domain(AnonymousUser(), domain) is False + + def test_superuser_always_allowed(self, domain): + user = make_user(superuser=True) + assert user_can_view_domain(user, domain) is True + + def test_model_level_permission_allowed(self, domain): + user = make_user() + user.has_perm = lambda perm, obj=None: perm == "core.view_domain" and obj is None + assert user_can_view_domain(user, domain) is True + + def test_object_level_permission_allowed(self, domain): + user = make_user() + user.has_perm = lambda perm, obj=None: perm == "core.view_domain" and obj is domain + assert user_can_view_domain(user, domain) is True + + def test_no_permission_denied(self, domain): + user = make_user() + user.has_perm = lambda perm, obj=None: False + assert user_can_view_domain(user, domain) is False + + +class TestResolveContentViewDistributions: + def test_ok_status_for_accessible_resolvable_distribution(self, domain, repository): + user = make_user(superuser=True) + content_view = ContentView.objects.create(name="cv", pulp_domain=domain) + distribution = Distribution.objects.create( + name="d1", base_path="d1", pulp_domain=domain, repository=repository + ) + content_view.distributions.add(distribution) + + resolutions = resolve_content_view_distributions(content_view, user) + + assert len(resolutions) == 1 + [resolution] = resolutions + assert resolution.distribution == distribution + assert resolution.domain == domain + assert resolution.status == STATUS_OK + assert resolution.repository_version == repository.latest_version() + + def test_no_version_status_for_distribution_without_source(self, domain): + user = make_user(superuser=True) + content_view = ContentView.objects.create(name="cv", pulp_domain=domain) + # No repository, repository_version, or publication set -- nothing to resolve. + distribution = Distribution.objects.create(name="d2", base_path="d2", pulp_domain=domain) + content_view.distributions.add(distribution) + + [resolution] = resolve_content_view_distributions(content_view, user) + + assert resolution.status == STATUS_NO_VERSION + assert resolution.repository_version is None + + def test_no_domain_access_status_for_inaccessible_domain( + self, domain, other_domain, other_repository + ): + user = make_user() + user.has_perm = lambda perm, obj=None: False # no access anywhere + content_view = ContentView.objects.create(name="cv", pulp_domain=domain) + distribution = Distribution.objects.create( + name="d3", base_path="d3", pulp_domain=other_domain, repository=other_repository + ) + content_view.distributions.add(distribution) + + [resolution] = resolve_content_view_distributions(content_view, user) + + assert resolution.status == STATUS_NO_DOMAIN_ACCESS + assert resolution.repository_version is None + + def test_mixed_statuses_and_grouping_excludes_non_ok( + self, domain, other_domain, repository, other_repository + ): + user = make_user() + # User can view `domain` but not `other_domain`. + user.has_perm = lambda perm, obj=None: perm == "core.view_domain" and obj == domain + + content_view = ContentView.objects.create(name="cv", pulp_domain=domain) + ok_distribution = Distribution.objects.create( + name="ok", base_path="ok", pulp_domain=domain, repository=repository + ) + no_access_distribution = Distribution.objects.create( + name="na", base_path="na", pulp_domain=other_domain, repository=other_repository + ) + no_version_distribution = Distribution.objects.create( + name="nv", base_path="nv", pulp_domain=domain + ) + content_view.distributions.add( + ok_distribution, no_access_distribution, no_version_distribution + ) + + resolutions = resolve_content_view_distributions(content_view, user) + by_pk = {r.distribution.pk: r for r in resolutions} + + assert by_pk[ok_distribution.pk].status == STATUS_OK + assert by_pk[no_access_distribution.pk].status == STATUS_NO_DOMAIN_ACCESS + assert by_pk[no_version_distribution.pk].status == STATUS_NO_VERSION + + # Edge case: all sources inaccessible/stale except one -- grouping must silently + # exclude the rest rather than erroring. + grouped = group_versions_by_domain(resolutions) + assert list(grouped.keys()) == [domain] + assert grouped[domain] == [repository.latest_version()] + + def test_all_inaccessible_yields_empty_grouping(self, domain, other_domain, other_repository): + user = make_user() + user.has_perm = lambda perm, obj=None: False + + content_view = ContentView.objects.create(name="cv", pulp_domain=domain) + distribution = Distribution.objects.create( + name="na", base_path="na", pulp_domain=other_domain, repository=other_repository + ) + content_view.distributions.add(distribution) + + resolutions = resolve_content_view_distributions(content_view, user) + assert group_versions_by_domain(resolutions) == {} + + +class TestScatterGather: + @staticmethod + def _make_repos(domain, names): + return [Repository.objects.create(name=n, pulp_domain=domain) for n in names] + + def test_no_domains_returns_empty(self): + page, total = scatter_gather( + {}, lambda versions: Repository.objects.none(), order_by="name", limit=10 + ) + assert page == [] + assert total == 0 + + def test_no_domains_returns_empty_without_count(self): + page, total = scatter_gather( + {}, + lambda versions: Repository.objects.none(), + order_by="name", + limit=10, + count=False, + ) + assert page == [] + assert total is None + + def test_single_domain_uses_native_ordering_and_slicing(self, domain): + self._make_repos(domain, ["c", "a", "b"]) + + def build_queryset(versions): + return Repository.objects.filter(pulp_domain=domain).order_by("name") + + page, total = scatter_gather( + {domain: ["v1"]}, build_queryset, order_by="name", limit=2, offset=1 + ) + + assert total == 3 + assert [r.name for r in page] == ["b", "c"] + + def test_single_domain_skips_count_when_requested(self, domain): + self._make_repos(domain, ["a", "b"]) + + def build_queryset(versions): + return Repository.objects.filter(pulp_domain=domain).order_by("name") + + page, total = scatter_gather( + {domain: ["v1"]}, build_queryset, order_by="name", limit=10, count=False + ) + + assert total is None + assert [r.name for r in page] == ["a", "b"] + + def test_multi_domain_merges_sorts_and_paginates(self, domain, other_domain): + self._make_repos(domain, ["apple", "cherry"]) + self._make_repos(other_domain, ["banana", "date"]) + + def build_queryset(versions): + (current_domain,) = versions + return Repository.objects.filter(pulp_domain=current_domain).order_by("name") + + versions_by_domain = {domain: [domain], other_domain: [other_domain]} + + page1, total1 = scatter_gather( + versions_by_domain, build_queryset, order_by="name", limit=2, offset=0 + ) + assert total1 == 4 + assert [r.name for r in page1] == ["apple", "banana"] + + page2, total2 = scatter_gather( + versions_by_domain, build_queryset, order_by="name", limit=2, offset=2 + ) + assert total2 == 4 + assert [r.name for r in page2] == ["cherry", "date"] + + def test_multi_domain_descending_order(self, domain, other_domain): + self._make_repos(domain, ["apple", "cherry"]) + self._make_repos(other_domain, ["banana", "date"]) + + def build_queryset(versions): + (current_domain,) = versions + return Repository.objects.filter(pulp_domain=current_domain).order_by("-name") + + versions_by_domain = {domain: [domain], other_domain: [other_domain]} + + page, total = scatter_gather( + versions_by_domain, + build_queryset, + order_by="name", + descending=True, + limit=4, + ) + + assert total == 4 + assert [r.name for r in page] == ["date", "cherry", "banana", "apple"] + + def test_multi_domain_multi_field_order(self, domain, other_domain): + Repository.objects.create(name="same", pulp_domain=domain, description="b") + Repository.objects.create(name="same", pulp_domain=other_domain, description="a") + + def build_queryset(versions): + (current_domain,) = versions + return Repository.objects.filter(pulp_domain=current_domain).order_by( + "name", "description" + ) + + versions_by_domain = {domain: [domain], other_domain: [other_domain]} + + page, total = scatter_gather( + versions_by_domain, + build_queryset, + order_by=("name", "description"), + limit=2, + ) + + assert total == 2 + assert [r.description for r in page] == ["a", "b"] + + def test_multi_domain_over_fetch_bound_excludes_out_of_range_rows(self, domain, other_domain): + # Each domain is over-fetched only to `limit + offset` rows -- a lower-ranked row + # from one domain that wouldn't make the final page must not be fetched needlessly, + # but a higher-ranked one that does make the page must still show up correctly. + self._make_repos(domain, ["a1", "a2", "a3"]) + self._make_repos(other_domain, ["b1"]) + + def build_queryset(versions): + (current_domain,) = versions + return Repository.objects.filter(pulp_domain=current_domain).order_by("name") + + versions_by_domain = {domain: [domain], other_domain: [other_domain]} + + page, total = scatter_gather( + versions_by_domain, build_queryset, order_by="name", limit=2, offset=0 + ) + + assert total == 4 + assert [r.name for r in page] == ["a1", "a2"] From 31b6d6c68d936d5e7677cd92bb59d0f3acd14ebd Mon Sep 17 00:00:00 2001 From: Yasen Date: Wed, 29 Jul 2026 11:27:06 +0200 Subject: [PATCH 2/3] Add router_lookup to ContentViewViewSet for nested search routing Co-authored-by: Cursor --- pulpcore/app/viewsets/content_view.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pulpcore/app/viewsets/content_view.py b/pulpcore/app/viewsets/content_view.py index 6c92703ab1c..603742b53df 100644 --- a/pulpcore/app/viewsets/content_view.py +++ b/pulpcore/app/viewsets/content_view.py @@ -39,6 +39,7 @@ class ContentViewViewSet( queryset = ContentView.objects.all() endpoint_name = "content-views" + router_lookup = "content_view" serializer_class = ContentViewSerializer filterset_class = ContentViewFilter ordering = "-pulp_created" From 813dcf40c3a6e477057b87154fd1643546c3da18 Mon Sep 17 00:00:00 2001 From: Yasen Date: Wed, 29 Jul 2026 12:11:27 +0200 Subject: [PATCH 3/3] Fix get_viewset_for_model ambiguity for nested search viewsets A plugin may register an additional read-only, nested viewset that reuses an existing content type's queryset for its own purposes (e.g. the RPM ContentView search endpoints reusing Package/UpdateRecord/etc.) without intending to compete for that model's canonical viewset. Since such viewsets are always nested (they declare parent_viewset), exclude them from the ambiguity check when exactly one non-nested candidate remains. Without this fix, registering a second viewset against an existing content model made get_viewset_for_model raise LookupError for that model unconditionally, breaking RepositoryVersion content_summary hrefs and master-viewset queryset scoping for every affected content type. Co-authored-by: Cursor --- CHANGES/6001.bugfix | 1 + pulpcore/app/util.py | 12 +++++-- pulpcore/tests/unit/test_util.py | 60 ++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 CHANGES/6001.bugfix diff --git a/CHANGES/6001.bugfix b/CHANGES/6001.bugfix new file mode 100644 index 00000000000..36eb2ffdba3 --- /dev/null +++ b/CHANGES/6001.bugfix @@ -0,0 +1 @@ +Fixed ``get_viewset_for_model``/``get_view_name_for_model`` to correctly resolve a content type's canonical viewset even when a plugin registers additional read-only, nested viewsets that reuse that model's queryset for their own purposes (e.g. a ``ContentView`` search endpoint). Previously, any such additional nested registration made the model's viewset unresolvable, breaking ``content_summary`` hrefs and master-viewset queryset scoping for that content type. diff --git a/pulpcore/app/util.py b/pulpcore/app/util.py index 76f2a5b47fb..273bb403572 100644 --- a/pulpcore/app/util.py +++ b/pulpcore/app/util.py @@ -260,8 +260,16 @@ def get_viewset_for_model(model_obj, ignore_error=False): # go through the viewset registry to find the viewset for the passed-in model for app in pulp_plugin_configs(): for model, viewsets in app.named_viewsets.items(): - # There may be multiple viewsets for a model. In this - # case, we can't reverse the mapping. + # There may be multiple viewsets for a model, e.g. a plugin may register an + # additional read-only, nested viewset that reuses an existing content type's + # queryset for its own purposes (a ContentView search endpoint, for example), + # without intending to compete for that model's canonical viewset. Such viewsets + # are always nested (they declare a parent_viewset), so if excluding them leaves + # exactly one candidate, that candidate is unambiguously the canonical viewset. + if len(viewsets) > 1: + non_nested = [vs for vs in viewsets if getattr(vs, "parent_viewset", None) is None] + if len(non_nested) == 1: + viewsets = non_nested if len(viewsets) == 1: viewset = viewsets[0] _model_viewset_cache.setdefault(model, viewset) diff --git a/pulpcore/tests/unit/test_util.py b/pulpcore/tests/unit/test_util.py index 6b2943ceae3..ad3edc45b9e 100644 --- a/pulpcore/tests/unit/test_util.py +++ b/pulpcore/tests/unit/test_util.py @@ -94,6 +94,66 @@ def test_get_view_name_for_model_not_found(monkeypatch): util.get_view_name_for_model(mock.Mock(), "foo") +class _FakeModel: + """A standalone model stand-in, distinct per test, so the real _model_viewset_cache + (keyed by model class) never collides across test runs or with real Pulp models.""" + + _meta = mock.Mock() + + +_FakeModel._meta.model = _FakeModel + + +def _fake_viewset(model, *, parent_viewset=None): + viewset = mock.Mock() + viewset.queryset.model = model + viewset.parent_viewset = parent_viewset + return viewset + + +def _fake_app_config(named_viewsets): + app_config = mock.Mock() + app_config.named_viewsets = named_viewsets + return app_config + + +def test_get_viewset_for_model_ignores_nested_viewsets_when_disambiguating(monkeypatch): + """ + A plugin may register an additional read-only, nested viewset that reuses an existing + content type's queryset for its own purposes (e.g. a ContentView search endpoint reusing + Package's queryset) without intending to compete for that model's canonical viewset. Since + such viewsets are always nested (they declare parent_viewset), the canonical, non-nested + viewset should still be resolvable. + """ + model = _FakeModel + canonical = _fake_viewset(model) + nested_1 = _fake_viewset(model, parent_viewset=mock.Mock()) + nested_2 = _fake_viewset(model, parent_viewset=mock.Mock()) + app_config = _fake_app_config({model: [canonical, nested_1, nested_2]}) + + monkeypatch.setattr(util, "pulp_plugin_configs", lambda: [app_config]) + monkeypatch.setattr(util, "_model_viewset_cache", {}) + + assert util.get_viewset_for_model(model) is canonical + + +def test_get_viewset_for_model_still_ambiguous_without_unique_non_nested_viewset(monkeypatch): + """ + If there isn't exactly one non-nested candidate (e.g. two genuinely competing top-level + viewsets), the mapping is still ambiguous and raises LookupError, same as before. + """ + model = _FakeModel + viewset_a = _fake_viewset(model) + viewset_b = _fake_viewset(model) + app_config = _fake_app_config({model: [viewset_a, viewset_b]}) + + monkeypatch.setattr(util, "pulp_plugin_configs", lambda: [app_config]) + monkeypatch.setattr(util, "_model_viewset_cache", {}) + + with pytest.raises(LookupError): + util.get_viewset_for_model(model) + + class TestHashingFileWriter(unittest.TestCase): def setUp(self) -> None: self.test_dir_obj = tempfile.TemporaryDirectory()