From 6b7f0cdf68a1fa3d703e7b32cb73484df80a237b Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Tue, 28 Jul 2026 22:38:56 -0400 Subject: [PATCH] Add error_on_reject for partial package policy rejection Allow repositories to skip packages rejected by blocklist or substitution policies instead of failing the entire version. closes #1278 Assisted By: Cursor Grok 4.5 Co-authored-by: Cursor --- CHANGES/1278.feature | 4 + docs/user/guides/package_policies.md | 31 +++++ .../0023_pythonrepository_error_on_reject.py | 16 +++ pulp_python/app/models.py | 124 +++++++++++++++--- pulp_python/app/serializers.py | 12 ++ pulp_python/app/viewsets.py | 14 +- .../tests/functional/api/test_blocklist.py | 51 ++++++- .../functional/api/test_crud_content_unit.py | 57 ++++++++ 8 files changed, 287 insertions(+), 22 deletions(-) create mode 100644 CHANGES/1278.feature create mode 100644 pulp_python/app/migrations/0023_pythonrepository_error_on_reject.py diff --git a/CHANGES/1278.feature b/CHANGES/1278.feature new file mode 100644 index 00000000..74c96fee --- /dev/null +++ b/CHANGES/1278.feature @@ -0,0 +1,4 @@ +Added an `error_on_reject` boolean field to PythonRepository (default: `True`). +When `False`, packages rejected by the blocklist or package substitution policies are skipped +instead of failing the entire repository version; skipped packages are recorded in a task +progress report. diff --git a/docs/user/guides/package_policies.md b/docs/user/guides/package_policies.md index e3340016..bedaed36 100644 --- a/docs/user/guides/package_policies.md +++ b/docs/user/guides/package_policies.md @@ -4,6 +4,9 @@ Python repositories offer two mechanisms for controlling which packages they acc **blocklists** to prevent specific packages from being added, and **package substitution control** to prevent silent replacement of existing packages. +By default, when either policy rejects a package, the entire repository version operation fails. +Set `error_on_reject` to `False` to instead skip rejected packages and continue adding the rest. + ## Setup If you do not already have a repository, create one: @@ -122,3 +125,31 @@ pulp python repository update --repository "foo" --allow-package-substitution ``` Once re-enabled, packages with duplicate filenames can replace existing content again. + +## Partial rejection (`error_on_reject`) + +When a package is rejected by the blocklist or by the package substitution policy +(`allow_package_substitution=False`), the default behavior (`error_on_reject=True`) is to fail +the entire operation. No packages from the request are added. + +Setting `error_on_reject` to `False` changes this: rejected packages are skipped, remaining +packages are added, and skipped packages are recorded in a task progress report (including +filenames and, for substitution conflicts, the existing vs rejected checksums). + +### Disable failing on rejected packages + +```bash +pulp python repository update --repository "foo" --no-error-on-reject +``` + +You can also set this when creating a repository: + +```bash +pulp python repository create --name "foo3" --no-error-on-reject --block-package-substitution +``` + +### Re-enable failing on rejected packages + +```bash +pulp python repository update --repository "foo" --error-on-reject +``` diff --git a/pulp_python/app/migrations/0023_pythonrepository_error_on_reject.py b/pulp_python/app/migrations/0023_pythonrepository_error_on_reject.py new file mode 100644 index 00000000..d5bf94cc --- /dev/null +++ b/pulp_python/app/migrations/0023_pythonrepository_error_on_reject.py @@ -0,0 +1,16 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("python", "0022_pythonblocklistentry"), + ] + + operations = [ + migrations.AddField( + model_name="pythonrepository", + name="error_on_reject", + field=models.BooleanField(default=True), + ), + ] diff --git a/pulp_python/app/models.py b/pulp_python/app/models.py index bdfe9e7f..3742ac63 100644 --- a/pulp_python/app/models.py +++ b/pulp_python/app/models.py @@ -19,6 +19,7 @@ BaseModel, Content, Distribution, + ProgressReport, Publication, Remote, Repository, @@ -370,6 +371,7 @@ class PythonRepository(Repository, AutoAddObjPermsMixin): autopublish = models.BooleanField(default=False) allow_package_substitution = models.BooleanField(default=True) + error_on_reject = models.BooleanField(default=True) class Meta: default_related_name = "%(app_label)s_%(model_name)s" @@ -400,7 +402,8 @@ def finalize_new_version(self, new_version): Remove duplicate packages that have the same filename. When allow_package_substitution is False, reject any new version that would implicitly - replace existing content with different checksums (content substitution). + replace existing content with different checksums (content substitution), unless + error_on_reject is False, in which case the conflicting packages are skipped. Also checks newly added content against the repository's blocklist entries. """ @@ -412,53 +415,144 @@ def finalize_new_version(self, new_version): def _check_for_package_substitution(self, new_version): """ - Raise a ValidationError if newly added packages would replace existing packages - that have the same filename but a different sha256 checksum. + Handle packages that would replace existing packages with the same filename but a + different sha256 checksum. + + When error_on_reject is True, raise a ValidationError. When False, remove the + newly added conflicting packages from the version and record them in a progress report. """ qs = PythonPackageContent.objects.filter(pk__in=new_version.content) duplicates = collect_duplicates(qs, ("filename",)) - if duplicates: + if not duplicates: + return + + if self.error_on_reject: raise ValidationError( "Found duplicate packages being added with the same filename but different " "checksums. To allow this, set 'allow_package_substitution' to True on the " f"repository. Conflicting packages: {duplicates}" ) + added_pks = { + str(pk) + for pk in new_version.added(base_version=new_version.base_version).values_list( + "pk", flat=True + ) + } + to_remove_pks = [] + messages = [] + for dup in duplicates: + filename = dup.keyset_value[0] + pkgs = { + str(pkg.pk): pkg + for pkg in PythonPackageContent.objects.filter(pk__in=dup.duplicate_pks).only( + "pk", "filename", "sha256" + ) + } + existing = [pkgs[pk] for pk in dup.duplicate_pks if pk not in added_pks] + added = [pkgs[pk] for pk in dup.duplicate_pks if pk in added_pks] + if existing: + kept_sha256 = existing[0].sha256 + for pkg in added: + to_remove_pks.append(pkg.pk) + messages.append( + f"{filename} (existing sha256={kept_sha256}, rejected sha256={pkg.sha256})" + ) + elif len(added) > 1: + kept = added[0] + for pkg in added[1:]: + to_remove_pks.append(pkg.pk) + messages.append( + f"{filename} (kept sha256={kept.sha256}, rejected sha256={pkg.sha256})" + ) + + if to_remove_pks: + new_version.remove_content(PythonPackageContent.objects.filter(pk__in=to_remove_pks)) + self._report_rejected_packages( + messages, + message="Skipping packages rejected by package substitution policy", + code="python.reject.substitution", + ) + def _check_blocklist(self, new_version): """ Check newly added content in a repository version against the blocklist. + + When error_on_reject is True, raise a ValidationError. When False, remove the + blocklisted packages from the version and record them in a progress report. """ added_content = PythonPackageContent.objects.filter( pk__in=new_version.added().values_list("pk", flat=True) - ).only("filename", "name_normalized", "version") - if added_content.exists(): - self.check_blocklist_for_packages(added_content) + ).only("pk", "filename", "name_normalized", "version") + if not added_content.exists(): + return - def check_blocklist_for_packages(self, packages): + blocked = self.find_blocklisted_packages(added_content) + if not blocked: + return + + if self.error_on_reject: + raise ValidationError( + "Blocklisted packages cannot be added to this repository: {}".format( + ", ".join(pkg.filename for pkg in blocked) + ) + ) + + new_version.remove_content( + PythonPackageContent.objects.filter(pk__in=[p.pk for p in blocked]) + ) + self._report_rejected_packages( + [pkg.filename for pkg in blocked], + message="Skipping packages rejected by blocklist policy", + code="python.reject.blocklist", + ) + + def find_blocklisted_packages(self, packages): """ - Raise a ValidationError if any of the given packages match a blocklist entry. + Return the packages from ``packages`` that match a blocklist entry. """ - entries = PythonBlocklistEntry.objects.filter(repository=self) - if not entries.exists(): - return + entries = list(PythonBlocklistEntry.objects.filter(repository=self)) + if not entries: + return [] blocked = [] for pkg in packages: for entry in entries: if entry.filename and entry.filename == pkg.filename: - blocked.append(pkg.filename) + blocked.append(pkg) break if entry.name == pkg.name_normalized: if not entry.version or entry.version == pkg.version: - blocked.append(pkg.filename) + blocked.append(pkg) break + return blocked + + def check_blocklist_for_packages(self, packages): + """ + Raise a ValidationError if any of the given packages match a blocklist entry. + """ + blocked = self.find_blocklisted_packages(packages) if blocked: raise ValidationError( "Blocklisted packages cannot be added to this repository: {}".format( - ", ".join(blocked) + ", ".join(pkg.filename for pkg in blocked) ) ) + def _report_rejected_packages(self, details, message, code): + """ + Record skipped packages in a task progress report. + """ + suffix = "; ".join(details) + log.info("%s: %s", message, suffix) + with ProgressReport( + message=message, + code=code, + total=len(details), + suffix=suffix, + ) as pb: + pb.increase_by(len(details)) + class PythonBlocklistEntry(BaseModel): """ diff --git a/pulp_python/app/serializers.py b/pulp_python/app/serializers.py index 560d8e93..4b455df8 100644 --- a/pulp_python/app/serializers.py +++ b/pulp_python/app/serializers.py @@ -73,6 +73,17 @@ class PythonRepositorySerializer(core_serializers.RepositorySerializer): default=True, required=False, ) + error_on_reject = serializers.BooleanField( + help_text=_( + "Whether to fail the entire repository version when packages are rejected by the " + "package substitution or blocklist policies. When True (the default), a ValidationError " + "is raised and no packages from the request are added. When False, rejected packages " + "are skipped and remaining packages are added; skipped packages are recorded in a " + "task progress report." + ), + default=True, + required=False, + ) def get_blocklist_entries_href(self, obj): repo_href = reverse("repositories-python/python-detail", kwargs={"pk": obj.pk}) @@ -82,6 +93,7 @@ class Meta: fields = core_serializers.RepositorySerializer.Meta.fields + ( "autopublish", "allow_package_substitution", + "error_on_reject", "blocklist_entries_href", ) model = python_models.PythonRepository diff --git a/pulp_python/app/viewsets.py b/pulp_python/app/viewsets.py index 58a41187..f939c0e1 100644 --- a/pulp_python/app/viewsets.py +++ b/pulp_python/app/viewsets.py @@ -147,19 +147,21 @@ def modify(self, request, pk): """ Queues a task that creates a new RepositoryVersion by adding and removing content units. - If allow_package_substitution is False and the request is **only** adding packages, then a - package substitution check is performed to provide a quicker error response. Otherwise, the - check is delegated to the task. + If allow_package_substitution is False, error_on_reject is True, and the request is + **only** adding packages, then a package substitution check is performed to provide a + quicker error response. Otherwise, the check is delegated to the task. - Also performs an early blocklist check on added packages. + Also performs an early blocklist check on added packages when error_on_reject is True. + When error_on_reject is False, rejected packages are skipped during task finalization. """ repository = self.get_object() add_content_units = request.data.get("add_content_units", []) content_ids = [extract_pk(x) for x in add_content_units] - self._early_blocklist_check(repository, content_ids) + if repository.error_on_reject: + self._early_blocklist_check(repository, content_ids) - if not repository.allow_package_substitution: + if not repository.allow_package_substitution and repository.error_on_reject: remove_content_units = request.data.get("remove_content_units", []) if remove_content_units or "base_version" in request.data: return super().modify(request, pk) diff --git a/pulp_python/tests/functional/api/test_blocklist.py b/pulp_python/tests/functional/api/test_blocklist.py index 8aeed2ec..317f0beb 100644 --- a/pulp_python/tests/functional/api/test_blocklist.py +++ b/pulp_python/tests/functional/api/test_blocklist.py @@ -2,7 +2,12 @@ from pulpcore.tests.functional.utils import PulpTaskError -from pulp_python.tests.functional.constants import PYTHON_EGG_FILENAME, PYTHON_EGG_URL +from pulp_python.tests.functional.constants import ( + PYTHON_EGG_FILENAME, + PYTHON_EGG_URL, + PYTHON_WHEEL_FILENAME, + PYTHON_WHEEL_URL, +) CONTENT_BODY = {"relative_path": PYTHON_EGG_FILENAME, "file_url": PYTHON_EGG_URL} BLOCKED_MSG = "Blocklisted packages cannot be added to this repository" @@ -150,3 +155,47 @@ def test_modify_blocked(monitor_task, python_bindings, python_repo): repo = python_bindings.RepositoriesPythonApi.read(python_repo.pulp_href) assert repo.latest_version_href.endswith("/0/") + + +@pytest.mark.parallel +def test_error_on_reject_false_skips_blocklisted( + monitor_task, python_bindings, python_repo_factory +): + """ + When error_on_reject=False, blocklisted packages in a batch modify are skipped while + non-blocklisted packages are still added. + """ + repo = python_repo_factory(error_on_reject=False) + python_bindings.RepositoriesPythonBlocklistEntriesApi.create( + repo.pulp_href, + python_bindings.PythonPythonBlocklistEntry(filename=PYTHON_EGG_FILENAME), + ) + + response = python_bindings.ContentPackagesApi.create(**CONTENT_BODY) + blocked = python_bindings.ContentPackagesApi.read( + monitor_task(response.task).created_resources[0] + ) + response = python_bindings.ContentPackagesApi.create( + relative_path=PYTHON_WHEEL_FILENAME, file_url=PYTHON_WHEEL_URL + ) + allowed = python_bindings.ContentPackagesApi.read( + monitor_task(response.task).created_resources[0] + ) + + body = {"add_content_units": [blocked.pulp_href, allowed.pulp_href]} + task = monitor_task(python_bindings.RepositoriesPythonApi.modify(repo.pulp_href, body).task) + + reports = {report.code: report for report in task.progress_reports} + assert "python.reject.blocklist" in reports + report = reports["python.reject.blocklist"] + assert report.done == 1 + assert PYTHON_EGG_FILENAME in report.suffix + + repo = python_bindings.RepositoriesPythonApi.read(repo.pulp_href) + content_list = python_bindings.ContentPackagesApi.list( + repository_version=repo.latest_version_href + ) + hrefs = {c.pulp_href for c in content_list.results} + assert allowed.pulp_href in hrefs + assert blocked.pulp_href not in hrefs + assert content_list.count == 1 diff --git a/pulp_python/tests/functional/api/test_crud_content_unit.py b/pulp_python/tests/functional/api/test_crud_content_unit.py index 3641803a..ae54270f 100644 --- a/pulp_python/tests/functional/api/test_crud_content_unit.py +++ b/pulp_python/tests/functional/api/test_crud_content_unit.py @@ -304,3 +304,60 @@ def test_package_substitution_allowed_by_default( ) assert content_list.count == 1 assert content_list.results[0].sha256 == content2.sha256 + + +def test_error_on_reject_false_skips_substitution( + monitor_task, + python_bindings, + python_repo_factory, +): + """ + When error_on_reject=False and allow_package_substitution=False, conflicting packages + in a batch modify are skipped while non-conflicting packages are still added. + """ + repo = python_repo_factory(allow_package_substitution=False, error_on_reject=False) + assert repo.error_on_reject is False + + content_body = {"relative_path": PYTHON_EGG_FILENAME, "file_url": PYTHON_EGG_URL} + response = python_bindings.ContentPackagesApi.create(repository=repo.pulp_href, **content_body) + task = monitor_task(response.task) + original = python_bindings.ContentPackagesApi.read(task.created_resources[-1]) + + # Conflicting package: same filename, different checksum + conflict_filename = "pip-26.0.1.tar.gz" + conflict_url = get_package_url("pip", conflict_filename) + response = python_bindings.ContentPackagesApi.create( + relative_path=PYTHON_EGG_FILENAME, file_url=conflict_url + ) + conflict = python_bindings.ContentPackagesApi.read( + monitor_task(response.task).created_resources[0] + ) + + # Non-conflicting package + response = python_bindings.ContentPackagesApi.create( + relative_path=PYTHON_WHEEL_FILENAME, file_url=PYTHON_WHEEL_URL + ) + wheel = python_bindings.ContentPackagesApi.read( + monitor_task(response.task).created_resources[0] + ) + + body = {"add_content_units": [conflict.pulp_href, wheel.pulp_href]} + task = monitor_task(python_bindings.RepositoriesPythonApi.modify(repo.pulp_href, body).task) + + reports = {report.code: report for report in task.progress_reports} + assert "python.reject.substitution" in reports + report = reports["python.reject.substitution"] + assert report.done == 1 + assert PYTHON_EGG_FILENAME in report.suffix + assert original.sha256 in report.suffix + assert conflict.sha256 in report.suffix + + repo = python_bindings.RepositoriesPythonApi.read(repo.pulp_href) + content_list = python_bindings.ContentPackagesApi.list( + repository_version=repo.latest_version_href + ) + hrefs = {c.pulp_href for c in content_list.results} + assert original.pulp_href in hrefs + assert wheel.pulp_href in hrefs + assert conflict.pulp_href not in hrefs + assert content_list.count == 2