From c66dd80f4113272c41d8eed8bd09da1ca26ec300 Mon Sep 17 00:00:00 2001 From: David Davis <86290+daviddavis@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:56:43 +0000 Subject: [PATCH] Scope wildcard content removal Define wildcard repository removal according to the supplied distribution and component selectors: - remove_content_units=['*'] with a specified distribution and component removes all content from that release component. - remove_content_units=['*'] with a specified distribution and component='*' removes all content from that distribution. - remove_content_units=['*'] with distribution='*' and component='*' removes every content unit from the repository. - remove_content_units=['*'] without either selector preserves the existing behavior and removes every content unit from the repository. Scoped wildcards are expanded through binary and source package component relationships in the base repository version. Content that remain linked outside the selected scope are preserved, while matching relationships are removed. Also, add validation to prevent release and components from being named `*`, which seems unlikely but could present a real problem. Document the wildcard contract in the modify serializer and changelog, and add functional coverage for component-scoped, distribution-scoped, and complete repository removal. Assisted-by: GitHub Copilot --- CHANGES/+scoped-wildcard-removal.feature | 1 + .../app/serializers/content_serializers.py | 46 +++ .../app/serializers/repository_serializers.py | 22 +- pulp_deb/app/tasks/signing.py | 80 +++++- .../functional/api/test_repository_modify.py | 267 +++++++++++++++++- 5 files changed, 400 insertions(+), 16 deletions(-) create mode 100644 CHANGES/+scoped-wildcard-removal.feature diff --git a/CHANGES/+scoped-wildcard-removal.feature b/CHANGES/+scoped-wildcard-removal.feature new file mode 100644 index 000000000..ca0d45deb --- /dev/null +++ b/CHANGES/+scoped-wildcard-removal.feature @@ -0,0 +1 @@ +Added scoped wildcard removal for APT repositories. Using `remove_content_units` with `distribution` and `component` can now empty a single component or a whole distribution, removing the covered packages along with their `ReleaseComponent` and, for a whole distribution, its `Release` and `ReleaseArchitectures`. Setting both selectors to `*` removes all content units from the repository. Since `*` is now a selector, it is rejected as a `distribution` or `component` name. diff --git a/pulp_deb/app/serializers/content_serializers.py b/pulp_deb/app/serializers/content_serializers.py index 656d038c1..63085ef1b 100644 --- a/pulp_deb/app/serializers/content_serializers.py +++ b/pulp_deb/app/serializers/content_serializers.py @@ -57,6 +57,21 @@ log = logging.getLogger(__name__) +def validate_no_wildcards(**fields): + """Reject '*' as a name, since it is reserved as a selector when modifying repositories. + + Each keyword argument maps a field name onto a single name or a list of names. + """ + message = "This field does not accept the special value '*'!" + errors = { + name: _(message) + for name, values in fields.items() + if "*" in ([values] if isinstance(values, str) else values or []) + } + if errors: + raise ValidationError(errors) + + class YesNoField(Field): """ A serializer field that accepts 'yes' or 'no' as boolean. @@ -259,6 +274,16 @@ class SinglePackageUploadSerializer(SingleArtifactContentUploadSerializer): distribution = CharField(help_text="Name of the distribution.", required=False) component = CharField(help_text="Name of the component.", required=False) + def validate(self, data): + """ + Ensure the upload does not create structure content named '*'. + """ + data = super().validate(data) + validate_no_wildcards( + distribution=data.get("distribution"), component=data.get("component") + ) + return data + @staticmethod def _get_or_create_content_and_qs(model, **data): content, created = model.objects.get_or_create(**data) @@ -830,6 +855,16 @@ def get_unique_together_validators(self): architectures = ListField(child=CharField(), required=False) components = ListField(child=CharField(), required=False) + def validate(self, data): + """ + Ensure we do not create a Release or ReleaseComponent object named '*'. + """ + data = super().validate(data) + validate_no_wildcards( + distribution=data.get("distribution"), components=data.get("components") + ) + return data + @staticmethod def _get_or_create_content_pk(model, **data): content, created = model.objects.get_or_create(**data) @@ -921,6 +956,7 @@ def validate(self, data): if data.get("architecture") == "all": message = "This field does not accept the special value 'all'!" raise ValidationError({"architecture": _(message)}) + validate_no_wildcards(distribution=data.get("distribution")) return data def get_unique_together_validators(self): @@ -954,6 +990,16 @@ class ReleaseComponentSerializer(NoArtifactContentSerializer): A Serializer for ReleaseComponent. """ + def validate(self, data): + """ + Ensure we do not create a ReleaseComponent object named '*'. + """ + data = super().validate(data) + validate_no_wildcards( + distribution=data.get("distribution"), component=data.get("component") + ) + return data + def get_unique_together_validators(self): """ We do not want UniqueTogetherValidator since we have retrieve logic! diff --git a/pulp_deb/app/serializers/repository_serializers.py b/pulp_deb/app/serializers/repository_serializers.py index a052bb74e..883663913 100644 --- a/pulp_deb/app/serializers/repository_serializers.py +++ b/pulp_deb/app/serializers/repository_serializers.py @@ -25,6 +25,7 @@ AptRepositoryReleaseServiceOverride, ) from pulp_deb.app.schema import COPY_CONFIG_SCHEMA +from pulp_deb.app.serializers.content_serializers import validate_no_wildcards class AptRepositoryAddRemoveContentSerializer(RepositoryAddRemoveContentSerializer): @@ -32,7 +33,10 @@ class AptRepositoryAddRemoveContentSerializer(RepositoryAddRemoveContentSerializ help_text=_( "Name of the distribution any packages from add_content_units or remove_content_units " "should be added to or removed from. Defaults to DEFAULT_DISTRIBUTION if only a " - "component is provided." + "component is provided. When remove_content_units is ['*'], a distribution limits " + "the removal to packages in that distribution, along with its Release and " + "ReleaseArchitectures. Set both distribution and component to '*' to remove all " + "content units from the repository." ), required=False, ) @@ -40,11 +44,25 @@ class AptRepositoryAddRemoveContentSerializer(RepositoryAddRemoveContentSerializ help_text=_( "Name of the component any packages from add_content_units or remove_content_units " "should be added to or removed from. Defaults to DEFAULT_COMPONENT if only a " - "distribution is provided.." + "distribution is provided. When remove_content_units is ['*'], a component limits " + "the removal to packages in that component, along with its ReleaseComponent. Set " + "component to '*' to remove packages from every component in the selected " + "distribution." ), required=False, ) + def validate(self, data): + """ + Ensure adding content does not create structure content named '*'. + """ + data = super().validate(data) + if data.get("add_content_units"): + validate_no_wildcards( + distribution=data.get("distribution"), component=data.get("component") + ) + return data + class Meta(RepositoryAddRemoveContentSerializer.Meta): fields = RepositoryAddRemoveContentSerializer.Meta.fields + ["distribution", "component"] diff --git a/pulp_deb/app/tasks/signing.py b/pulp_deb/app/tasks/signing.py index 011e9f8d1..ac96d2beb 100644 --- a/pulp_deb/app/tasks/signing.py +++ b/pulp_deb/app/tasks/signing.py @@ -29,6 +29,7 @@ AptRepository, Package, PackageReleaseComponent, + Release, ReleaseArchitecture, ReleaseComponent, SourcePackage, @@ -42,6 +43,37 @@ log = logging.getLogger(__name__) +def _filter_by_scope(queryset, distribution, component, prefix=""): + """Narrow a queryset to a distribution/component, where "*" matches every value.""" + if distribution != "*": + queryset = queryset.filter(**{f"{prefix}distribution": distribution}) + if component != "*": + queryset = queryset.filter(**{f"{prefix}component": component}) + return queryset + + +def _prepare_release_removals(repository_version, remove_content_units, distribution, component): + """Expand a scoped wildcard removal to the release metadata it covers. + + Emptying a single component drops only that ReleaseComponent, while emptying a whole + distribution ("*" component) also drops its Release and ReleaseArchitectures. + """ + release_components = _filter_by_scope( + ReleaseComponent.objects.filter(pk__in=repository_version.content), + distribution, + component, + ) + remove_content_units.extend(str(pk) for pk in release_components.values_list("pk", flat=True)) + if component != "*": + return + + for model in (Release, ReleaseArchitecture): + units = model.objects.filter(pk__in=repository_version.content) + if distribution != "*": + units = units.filter(distribution=distribution) + remove_content_units.extend(str(pk) for pk in units.values_list("pk", flat=True)) + + def _prepare_package_removals(repo, remove_content_units, base_version_pk, distribution, component): """Expand the removal list to include the release component relationships of each package. @@ -49,9 +81,16 @@ def _prepare_package_removals(repo, remove_content_units, base_version_pk, distr SourcePackageReleaseComponent links. When a distribution/component is given, the removal is scoped to that component: a package is only removed from the repository if the scope held its last relationship, so packages linked elsewhere or not linked at all are kept. + + A "*" removal names every package in scope rather than an explicit list, and additionally + removes the release metadata that scope covers. """ - # "*" removes all content, so there is nothing to resolve here. - if not remove_content_units or "*" in remove_content_units: + if not remove_content_units: + return + + wildcard_removal = "*" in remove_content_units + # An unscoped wildcard removes all repository content through pulpcore. + if wildcard_removal and distribution in (None, "*") and component in (None, "*"): return repository_version = ( @@ -64,23 +103,35 @@ def _prepare_package_removals(repo, remove_content_units, base_version_pk, distr if scoped: distribution = distribution or DEFAULT_DISTRIBUTION component = component or DEFAULT_COMPONENT + if wildcard_removal: + remove_content_units.clear() for model, relationship_model, relationship_field in ( (Package, PackageReleaseComponent, "package"), (SourcePackage, SourcePackageReleaseComponent, "source_package"), ): - units = model.objects.filter(pk__in=remove_content_units) - relationships = relationship_model.objects.filter( - **{ - f"{relationship_field}__in": units, - "pk__in": repository_version.content, - } - ) - if scoped: - scoped_relationships = relationships.filter( - release_component__distribution=distribution, - release_component__component=component, + if wildcard_removal: + relationships = relationship_model.objects.filter(pk__in=repository_version.content) + scoped_relationships = _filter_by_scope( + relationships, distribution, component, "release_component__" + ) + units = model.objects.filter( + pk__in=scoped_relationships.values_list(f"{relationship_field}_id", flat=True) ) + remove_content_units.extend(str(pk) for pk in units.values_list("pk", flat=True)) + else: + units = model.objects.filter(pk__in=remove_content_units) + relationships = relationship_model.objects.filter( + **{ + f"{relationship_field}__in": units, + "pk__in": repository_version.content, + } + ) + if scoped: + scoped_relationships = _filter_by_scope( + relationships, distribution, component, "release_component__" + ) + if scoped: # Relationships named in the request are removed alongside the scoped ones. removed_relationship_ids = set( relationship_model.objects.filter(pk__in=remove_content_units).values_list( @@ -102,6 +153,9 @@ def _prepare_package_removals(repo, remove_content_units, base_version_pk, distr relationships = scoped_relationships remove_content_units.extend(str(pk) for pk in relationships.values_list("pk", flat=True)) + if wildcard_removal: + _prepare_release_removals(repository_version, remove_content_units, distribution, component) + def _prepare_package_additions(add_content_units, distribution, component): """Expand the addition list with the metadata needed to publish the packages in a component. diff --git a/pulp_deb/tests/functional/api/test_repository_modify.py b/pulp_deb/tests/functional/api/test_repository_modify.py index 644c9ff1f..20eddf809 100644 --- a/pulp_deb/tests/functional/api/test_repository_modify.py +++ b/pulp_deb/tests/functional/api/test_repository_modify.py @@ -2,6 +2,7 @@ import pytest +from pulpcore.client.pulp_deb.exceptions import ApiException from pulpcore.tests.functional.utils import PulpTaskError from pulp_deb.app.constants import ( @@ -163,6 +164,74 @@ def test_remove_package_from_component( assert apt_package_release_components_api.list(**filters).count == expected_count +def test_remove_package_from_all_distributions_and_components( + apt_package_api, + apt_package_release_components_api, + apt_release_component_api, + deb_get_repository_by_href, + deb_modify_repository, + deb_package_factory, + deb_release_factory, + deb_repository_factory, +): + repository = deb_repository_factory() + removed_package = deb_package_factory( + file=str(get_local_package_absolute_path(DEB_PACKAGE_RELPATH)) + ) + kept_package = deb_package_factory( + file=str( + get_local_package_absolute_path("odin_1.0_ppc64.deb", "data/debian/pool/asgard/o/odin/") + ) + ) + distributions = [str(uuid4()), str(uuid4())] + components = ["main", str(uuid4())] + for distribution in distributions: + deb_release_factory( + codename=distribution, + suite=distribution, + distribution=distribution, + repository=repository.pulp_href, + ) + for distribution, component in ( + (distributions[0], components[0]), + (distributions[0], components[1]), + (distributions[1], components[0]), + ): + _modify_with_package( + repository, + removed_package, + deb_modify_repository, + distribution=distribution, + component=component, + ) + _modify_with_package( + repository, + kept_package, + deb_modify_repository, + distribution=distributions[1], + component=components[1], + ) + + deb_modify_repository( + repository, + { + "remove_content_units": [removed_package.pulp_href], + "distribution": "*", + "component": "*", + }, + ) + repository = deb_get_repository_by_href(repository.pulp_href) + + filters = {"repository_version": repository.latest_version_href} + assert [item.pulp_href for item in apt_package_api.list(**filters).results] == [ + kept_package.pulp_href + ] + assert [ + item.package for item in apt_package_release_components_api.list(**filters).results + ] == [kept_package.pulp_href] + assert apt_release_component_api.list(**filters).count == 4 + + def test_add_and_remove_packages_in_same_request( apt_package_api, apt_package_release_components_api, @@ -210,6 +279,135 @@ def test_add_and_remove_packages_in_same_request( ] +def test_remove_all_packages_from_component( + apt_package_api, + apt_package_release_components_api, + apt_release_api, + apt_release_architecture_api, + apt_release_component_api, + deb_get_repository_by_href, + deb_modify_repository, + deb_package_factory, + deb_release_factory, + deb_repository_factory, +): + repository = deb_repository_factory() + distribution = str(uuid4()) + components = ["main", str(uuid4())] + packages = [ + deb_package_factory(file=str(get_local_package_absolute_path(DEB_PACKAGE_RELPATH))), + deb_package_factory( + file=str( + get_local_package_absolute_path( + "odin_1.0_ppc64.deb", "data/debian/pool/asgard/o/odin/" + ) + ) + ), + ] + deb_release_factory( + codename=distribution, + suite=distribution, + distribution=distribution, + repository=repository.pulp_href, + ) + for package, component in zip(packages, components): + _modify_with_package( + repository, + package, + deb_modify_repository, + distribution=distribution, + component=component, + ) + + deb_modify_repository( + repository, + { + "remove_content_units": ["*"], + "distribution": distribution, + "component": components[0], + }, + ) + repository = deb_get_repository_by_href(repository.pulp_href) + + filters = {"repository_version": repository.latest_version_href} + assert [item.pulp_href for item in apt_package_api.list(**filters).results] == [ + packages[1].pulp_href + ] + assert apt_package_release_components_api.list(**filters).count == 1 + # Only the emptied component goes away; the rest of the release is still in use. + assert [item.component for item in apt_release_component_api.list(**filters).results] == [ + components[1] + ] + assert apt_release_api.list(**filters).count == 1 + assert apt_release_architecture_api.list(**filters).count == 1 + + +def test_remove_all_packages_from_distribution( + apt_package_api, + apt_package_release_components_api, + apt_release_api, + apt_release_architecture_api, + apt_release_component_api, + deb_get_repository_by_href, + deb_modify_repository, + deb_package_factory, + deb_release_factory, + deb_repository_factory, +): + repository = deb_repository_factory() + distributions = [str(uuid4()), str(uuid4())] + packages = [ + deb_package_factory(file=str(get_local_package_absolute_path(DEB_PACKAGE_RELPATH))), + deb_package_factory( + file=str( + get_local_package_absolute_path( + "odin_1.0_ppc64.deb", "data/debian/pool/asgard/o/odin/" + ) + ) + ), + ] + for package, distribution in zip(packages, distributions): + deb_release_factory( + codename=distribution, + suite=distribution, + distribution=distribution, + repository=repository.pulp_href, + ) + _modify_with_package( + repository, + package, + deb_modify_repository, + distribution=distribution, + component="main", + ) + + deb_modify_repository( + repository, + { + "remove_content_units": ["*"], + "distribution": distributions[0], + "component": "*", + }, + ) + repository = deb_get_repository_by_href(repository.pulp_href) + + filters = {"repository_version": repository.latest_version_href} + assert [item.pulp_href for item in apt_package_api.list(**filters).results] == [ + packages[1].pulp_href + ] + assert apt_package_release_components_api.list(**filters).count == 1 + # Emptying a distribution takes its whole release structure with it. + assert [item.distribution for item in apt_release_api.list(**filters).results] == [ + distributions[1] + ] + assert [item.distribution for item in apt_release_component_api.list(**filters).results] == [ + distributions[1] + ] + assert [item.distribution for item in apt_release_architecture_api.list(**filters).results] == [ + distributions[1] + ] + + def test_remove_all_content_units( apt_package_api, apt_package_release_components_api, @@ -238,7 +436,14 @@ def test_remove_all_content_units( component=str(uuid4()), ) - deb_modify_repository(repository, {"remove_content_units": ["*"]}) + deb_modify_repository( + repository, + { + "remove_content_units": ["*"], + "distribution": "*", + "component": "*", + }, + ) repository = deb_get_repository_by_href(repository.pulp_href) filters = {"repository_version": repository.latest_version_href} @@ -379,3 +584,63 @@ def test_modify_forwards_overwrite( {"add_content_units": [conflicting_release.pulp_href], "overwrite": False}, ) assert "Content overwrite rejected" in exception.value.task.error["description"] + + +@pytest.mark.parametrize("distribution,component", [("*", str(uuid4())), (str(uuid4()), "*")]) +def test_add_package_to_wildcard_scope_fails( + deb_modify_repository, + deb_package_factory, + deb_repository_factory, + distribution, + component, +): + """'*' is a removal selector, so it must not be turned into a name by an addition.""" + repository = deb_repository_factory() + package = deb_package_factory(file=str(get_local_package_absolute_path(DEB_PACKAGE_RELPATH))) + + with pytest.raises(ApiException) as exception: + _modify_with_package( + repository, + package, + deb_modify_repository, + distribution=distribution, + component=component, + ) + + assert exception.value.status == 400 + assert "does not accept the special value '*'" in exception.value.body + + +def test_create_release_with_wildcard_name_fails(deb_release_factory): + codename = str(uuid4()) + + with pytest.raises(ApiException) as exception: + deb_release_factory(codename=codename, suite=codename, distribution="*") + + assert exception.value.status == 400 + assert "does not accept the special value '*'" in exception.value.body + + +def test_create_release_with_wildcard_component_fails(deb_release_factory): + codename = str(uuid4()) + + with pytest.raises(ApiException) as exception: + deb_release_factory( + codename=codename, suite=codename, distribution=codename, components=["*"] + ) + + assert exception.value.status == 400 + assert "does not accept the special value '*'" in exception.value.body + + +@pytest.mark.parametrize("distribution,component", [("*", str(uuid4())), (str(uuid4()), "*")]) +def test_create_release_component_with_wildcard_name_fails( + deb_release_component_factory, + distribution, + component, +): + with pytest.raises(ApiException) as exception: + deb_release_component_factory(component=component, distribution=distribution) + + assert exception.value.status == 400 + assert "does not accept the special value '*'" in exception.value.body