Add error_on_reject for partial package policy rejection

gerrod3 · cursoragent · gerrod3 · commit cfd4010795d2 · 2026-07-28T22:44:39.000-04:00
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 <cursoragent@cursor.com>
diff --git a/CHANGES/1278.feature 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
