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