From 1c185441314f9e0d4a308050aa40561f95f2fb0f Mon Sep 17 00:00:00 2001 From: Jitka Halova Date: Thu, 30 Jul 2026 12:09:11 +0200 Subject: [PATCH 1/2] Add PEP 592 yank support closes #1270 Assisted By: Claude Opus 4.6 --- CHANGES/1270.feature | 1 + docs/user/guides/package_policies.md | 56 ++++ .../app/migrations/0023_packageyank.py | 50 +++ pulp_python/app/models.py | 30 +- pulp_python/app/pypi/serializers.py | 21 ++ pulp_python/app/pypi/views.py | 84 +++++ pulp_python/app/serializers.py | 19 ++ pulp_python/app/tasks/__init__.py | 1 + pulp_python/app/tasks/sync.py | 10 + pulp_python/app/tasks/yank.py | 64 ++++ pulp_python/app/urls.py | 3 + pulp_python/app/utils.py | 49 ++- pulp_python/app/viewsets.py | 23 ++ pulp_python/tests/functional/api/test_yank.py | 298 ++++++++++++++++++ 14 files changed, 691 insertions(+), 18 deletions(-) create mode 100644 CHANGES/1270.feature create mode 100644 pulp_python/app/migrations/0023_packageyank.py create mode 100644 pulp_python/app/tasks/yank.py create mode 100644 pulp_python/tests/functional/api/test_yank.py diff --git a/CHANGES/1270.feature b/CHANGES/1270.feature new file mode 100644 index 000000000..53eea7d6f --- /dev/null +++ b/CHANGES/1270.feature @@ -0,0 +1 @@ +Added yank support (PEP 592). diff --git a/docs/user/guides/package_policies.md b/docs/user/guides/package_policies.md index e3340016f..328130fd1 100644 --- a/docs/user/guides/package_policies.md +++ b/docs/user/guides/package_policies.md @@ -94,6 +94,62 @@ pulp python repository blocklist list --repository "foo" Once an entry is removed, packages matching it can be added to the repository again. +## Package Yanking + +[PEP 592](https://peps.python.org/pep-0592/) allows marking package versions as "yanked". +Package installers like `pip` will skip yanked versions when resolving dependencies. +However, if a user requests an exact version (e.g. `pip install twine==5.1.0`), +the yanked package will still be installed, with a warning. + +Yank status is per-repository: yanking a package in one repository does not affect other repositories +that contain the same package. + +### Yank a package version + +To yank a package version, send a POST request to the distribution's `/yank/` endpoint: + +```bash +http POST http://localhost:5001/pypi/default//yank/ \ + name=shelf-reader version=0.1 yanked_reason="critical security bug" \ + -a admin:password +``` + +The `yanked_reason` field is optional. If omitted, the package is marked as yanked with no reason. + +Yanking creates a new repository version with the yank marker added. +Yanking a version that is already yanked with the same reason is a no-op (no new repository version is created). +Re-yanking with a different reason will update the reason and create a new repository version. + +### Unyank a package version + +```bash +http POST http://localhost:5001/pypi/default//unyank/ \ + name=shelf-reader version=0.1 \ + -a admin:password +``` + +Unyanking creates a new repository version with the yank marker removed. +Unyanking a version that is not yanked is a no-op. + +### Syncing yanked packages + +When syncing from a remote that has yanked packages (e.g. PyPI), the yank status is preserved automatically. +Pulp creates a yank marker for each yanked version and includes it in the repository version. + +### Viewing yank status + +Yank status is visible in the Simple API and the PyPI Metadata API. + +Yank markers can also be listed via the REST API: + +```bash +# List all yank markers +http GET http://localhost:5001/pulp/default/api/v3/content/python/yanks/ -a admin:password + +# List yank markers for a specific repository version +http GET http://localhost:5001/pulp/default/api/v3/content/python/yanks/?repository_version= -a admin:password +``` + ## Package Substitution By default, Python repositories allow package substitution: uploading, syncing, or adding a package diff --git a/pulp_python/app/migrations/0023_packageyank.py b/pulp_python/app/migrations/0023_packageyank.py new file mode 100644 index 000000000..fab34cab1 --- /dev/null +++ b/pulp_python/app/migrations/0023_packageyank.py @@ -0,0 +1,50 @@ +# Generated by Django 5.2.16 on 2026-07-30 10:44 + +import django.db.models.deletion +from django.db import migrations, models + +import pulpcore.app.util + + +class Migration(migrations.Migration): + + dependencies = [ + ("python", "0022_pythonblocklistentry"), + ] + + operations = [ + migrations.CreateModel( + name="PackageYank", + fields=[ + ( + "content_ptr", + models.OneToOneField( + auto_created=True, + on_delete=django.db.models.deletion.CASCADE, + parent_link=True, + primary_key=True, + serialize=False, + to="core.content", + ), + ), + ("name_normalized", models.TextField()), + ("version", models.TextField()), + ("yanked_reason", models.TextField(default="")), + ( + "_pulp_domain", + models.ForeignKey( + default=pulpcore.app.util.get_domain_pk, + on_delete=django.db.models.deletion.PROTECT, + to="core.domain", + ), + ), + ], + options={ + "default_related_name": "%(app_label)s_%(model_name)s", + "unique_together": { + ("name_normalized", "version", "yanked_reason", "_pulp_domain") + }, + }, + bases=("core.content",), + ), + ] diff --git a/pulp_python/app/models.py b/pulp_python/app/models.py index bdfe9e7ff..9ba9e8c83 100644 --- a/pulp_python/app/models.py +++ b/pulp_python/app/models.py @@ -204,8 +204,6 @@ class PythonPackageContent(Content): sha256 = models.CharField(db_index=True, max_length=64) metadata_sha256 = models.CharField(max_length=64, null=True) size = models.BigIntegerField(default=0) - # yanked and yanked_reason are not implemented because they are mutable - # From pulpcore PROTECTED_FROM_RECLAIM = False TYPE = "python" @@ -289,6 +287,32 @@ class Meta: unique_together = ("sha256", "_pulp_domain") +class PackageYank(Content): + """ + A marker content type indicating a package version is yanked in a repository (PEP 592). + + Its presence in a repository version means all files for the matching + (name_normalized, version) pair are yanked. Yank/unyank operations + add/remove this marker, creating new repository versions. + """ + + TYPE = "python_yank" + repo_key_fields = ("name_normalized", "version") + + name_normalized = models.TextField() + version = models.TextField() + yanked_reason = models.TextField(default="") + + _pulp_domain = models.ForeignKey("core.Domain", default=get_domain_pk, on_delete=models.PROTECT) + + def __str__(self): + return f"<{self._meta.object_name}: {self.name_normalized} [{self.version}]>" + + class Meta: + default_related_name = "%(app_label)s_%(model_name)s" + unique_together = ("name_normalized", "version", "yanked_reason", "_pulp_domain") + + class PythonPublication(Publication, AutoAddObjPermsMixin): """ A Publication for PythonContent. @@ -364,7 +388,7 @@ class PythonRepository(Repository, AutoAddObjPermsMixin): """ TYPE = "python" - CONTENT_TYPES = [PythonPackageContent, PackageProvenance] + CONTENT_TYPES = [PythonPackageContent, PackageProvenance, PackageYank] REMOTE_TYPES = [PythonRemote] PULL_THROUGH_SUPPORTED = True diff --git a/pulp_python/app/pypi/serializers.py b/pulp_python/app/pypi/serializers.py index 746606a9d..bfa1a0ae3 100644 --- a/pulp_python/app/pypi/serializers.py +++ b/pulp_python/app/pypi/serializers.py @@ -136,3 +136,24 @@ class PackageUploadTaskSerializer(serializers.Serializer): session = serializers.CharField(allow_null=True) task = serializers.CharField() task_start_time = serializers.DateTimeField(allow_null=True) + + +class YankSerializer(serializers.Serializer): + """ + A Serializer for yank/unyank requests (PEP 592). + """ + + name = serializers.CharField( + help_text=_("The name of the package to yank or unyank."), + required=True, + ) + version = serializers.CharField( + help_text=_("The version of the package to yank or unyank."), + required=True, + ) + yanked_reason = serializers.CharField( + help_text=_("The reason for yanking the package version."), + required=False, + allow_blank=True, + default="", + ) diff --git a/pulp_python/app/pypi/views.py b/pulp_python/app/pypi/views.py index 102749bdd..c42c520b8 100644 --- a/pulp_python/app/pypi/views.py +++ b/pulp_python/app/pypi/views.py @@ -37,6 +37,7 @@ from pulp_python.app.cache import PythonApiCache, find_base_path_cached from pulp_python.app.models import ( PackageProvenance, + PackageYank, PythonDistribution, PythonPackageContent, PythonPublication, @@ -46,6 +47,7 @@ PackageUploadSerializer, PackageUploadTaskSerializer, SummarySerializer, + YankSerializer, ) from pulp_python.app.utils import ( PYPI_LAST_SERIAL, @@ -356,6 +358,8 @@ def parse_package(release_package): "upload_time": release_package.upload_time, "version": release_package.version, "provenance": release_package.provenance_url, + "yanked": release_package.is_yanked, + "yanked_reason": release_package.yanked_reason or "", } rfilter = get_remote_package_filter(remote) @@ -408,6 +412,11 @@ def retrieve(self, request, path, package): "version", "has_provenance", ) + yank_markers = dict( + PackageYank.objects.filter( + pk__in=repo_ver.content, name_normalized=normalized + ).values_list("version", "yanked_reason") + ) local_releases = { p["filename"]: { **p, @@ -418,6 +427,8 @@ def retrieve(self, request, path, package): if p["has_provenance"] else None ), + "yanked": p["version"] in yank_markers, + "yanked_reason": yank_markers.get(p["version"], ""), } for p in packages } @@ -493,12 +504,18 @@ def retrieve(self, request, path, meta): headers = {PYPI_LAST_SERIAL: str(PYPI_SERIAL_CONSTANT)} if settings.DOMAIN_ENABLED: domain = get_domain() + yank_markers = dict( + PackageYank.objects.filter( + pk__in=repo_ver.content, name_normalized=normalized + ).values_list("version", "yanked_reason") + ) json_body = python_content_to_json( path, package_content, version=version, domain=domain, repository_version=repo_ver, + yank_markers=yank_markers, ) if json_body: return Response(data=json_body, headers=headers) @@ -586,3 +603,70 @@ def retrieve(self, request, path, package, version, filename): if provenance: return Response(data=provenance.provenance) return HttpResponseNotFound(f"{package} {version} {filename} provenance does not exist.") + + +class YankView(PyPIMixin, ViewSet): + """View for yank/unyank requests (PEP 592).""" + + endpoint_name = "yank" + DEFAULT_ACCESS_POLICY = { + "statements": [ + { + "action": ["yank", "unyank"], + "principal": "authenticated", + "effect": "allow", + "condition": "index_has_repo_perm:python.modify_pythonrepository", + }, + ], + } + + @extend_schema(request=YankSerializer, summary="Yank a package version") + def yank(self, request, path): + """Yank a package version, marking all its files with data-yanked.""" + repo = self.distribution.repository + if not repo: + return HttpResponseBadRequest(reason="Index is not pointing to a repository") + + serializer = YankSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + normalized = canonicalize_name(serializer.validated_data["name"]) + version = serializer.validated_data["version"] + repo_ver = self.get_repository_version(self.distribution) + if not PythonPackageContent.objects.filter( + pk__in=repo_ver.content, name_normalized=normalized, version=version + ).exists(): + return HttpResponseNotFound(f"{normalized}=={version} not found in repository") + + result = dispatch( + tasks.ayank_package, + exclusive_resources=[repo], + kwargs={ + "repository_pk": str(repo.pk), + "name": serializer.validated_data["name"], + "version": serializer.validated_data["version"], + "yanked_reason": serializer.validated_data.get("yanked_reason", ""), + }, + ) + return OperationPostponedResponse(result, request) + + @extend_schema(request=YankSerializer, summary="Unyank a package version") + def unyank(self, request, path): + """Unyank a package version, unmarking all its files with data-yanked.""" + repo = self.distribution.repository + if not repo: + return HttpResponseBadRequest(reason="Index is not pointing to a repository") + + serializer = YankSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + result = dispatch( + tasks.aunyank_package, + exclusive_resources=[repo], + kwargs={ + "repository_pk": str(repo.pk), + "name": serializer.validated_data["name"], + "version": serializer.validated_data["version"], + }, + ) + return OperationPostponedResponse(result, request) diff --git a/pulp_python/app/serializers.py b/pulp_python/app/serializers.py index 560d8e932..6dc723ae7 100644 --- a/pulp_python/app/serializers.py +++ b/pulp_python/app/serializers.py @@ -683,6 +683,25 @@ class Meta: model = python_models.PackageProvenance +class PackageYankSerializer(core_serializers.NoArtifactContentSerializer): + """ + Read-only serializer for PackageYank content units (PEP 592). + Used by PackageYankViewSet to expose yank markers via the Pulp REST API. + """ + + name_normalized = serializers.CharField(read_only=True) + version = serializers.CharField(read_only=True) + yanked_reason = serializers.CharField(read_only=True) + + class Meta: + fields = core_serializers.NoArtifactContentSerializer.Meta.fields + ( + "name_normalized", + "version", + "yanked_reason", + ) + model = python_models.PackageYank + + class MultipleChoiceArrayField(serializers.MultipleChoiceField): """ A wrapper to make sure this DRF serializer works properly with ArrayFields. diff --git a/pulp_python/app/tasks/__init__.py b/pulp_python/app/tasks/__init__.py index 6c2949ebe..6c193dddb 100644 --- a/pulp_python/app/tasks/__init__.py +++ b/pulp_python/app/tasks/__init__.py @@ -7,3 +7,4 @@ from .sync import sync # noqa:F401 from .upload import upload, upload_group # noqa:F401 from .vulnerability_report import get_repo_version_content # noqa:F401 +from .yank import aunyank_package, ayank_package # noqa:F401 diff --git a/pulp_python/app/tasks/sync.py b/pulp_python/app/tasks/sync.py index 25a4955d5..dd5e38265 100644 --- a/pulp_python/app/tasks/sync.py +++ b/pulp_python/app/tasks/sync.py @@ -24,6 +24,7 @@ from pulp_python.app.exceptions import UnsupportedProtocolError from pulp_python.app.models import ( PackageProvenance, + PackageYank, PythonPackageContent, PythonRemote, ) @@ -265,6 +266,15 @@ async def create_content(self, pkg): ) d_artifacts.append(metadata_artifact) + if upstream_pkg.is_yanked: + yank_marker = PackageYank( + name_normalized=pkg.name, + version=version, + yanked_reason=upstream_pkg.yanked_reason or "", + ) + yank_dc = DeclarativeContent(content=yank_marker, d_artifacts=[]) + await self.python_stage.put(yank_dc) + dc = DeclarativeContent(content=package, d_artifacts=d_artifacts) declared_contents[entry["filename"]] = dc await self.python_stage.put(dc) diff --git a/pulp_python/app/tasks/yank.py b/pulp_python/app/tasks/yank.py new file mode 100644 index 000000000..44016cf9a --- /dev/null +++ b/pulp_python/app/tasks/yank.py @@ -0,0 +1,64 @@ +from packaging.utils import canonicalize_name + +from pulpcore.plugin.exceptions import ValidationError +from pulpcore.plugin.tasking import aadd_and_remove + +from pulp_python.app.models import PackageYank, PythonPackageContent, PythonRepository + + +async def ayank_package(repository_pk, name, version, yanked_reason=""): + """ + Yank a package version in a repository by adding a PackageYank marker. + Creates a new repository version with the yank marker added. + """ + normalized = canonicalize_name(name) + repository = await PythonRepository.objects.aget(pk=repository_pk) + latest = await repository.alatest_version() + + exists = await PythonPackageContent.objects.filter( + pk__in=latest.content, name_normalized=normalized, version=version + ).aexists() + if not exists: + raise ValidationError(f"Package {name}=={version} not found in repository") + + existing_yank = await PackageYank.objects.filter( + pk__in=latest.content, name_normalized=normalized, version=version + ).afirst() + if existing_yank and existing_yank.yanked_reason == yanked_reason: + return + + yank_marker, _ = await PackageYank.objects.aget_or_create( + name_normalized=normalized, + version=version, + yanked_reason=yanked_reason, + _pulp_domain_id=repository.pulp_domain_id, + ) + + await aadd_and_remove( + repository_pk=repository.pk, + add_content_units=[yank_marker.pk], + remove_content_units=[], + ) + + +async def aunyank_package(repository_pk, name, version): + """ + Unyank a package version in a repository by removing its PackageYank marker. + Creates a new repository version with the yank marker removed. + """ + normalized = canonicalize_name(name) + repository = await PythonRepository.objects.aget(pk=repository_pk) + latest = await repository.alatest_version() + + yank_marker = await PackageYank.objects.filter( + pk__in=latest.content, name_normalized=normalized, version=version + ).afirst() + + if yank_marker is None: + return + + await aadd_and_remove( + repository_pk=repository.pk, + add_content_units=[], + remove_content_units=[yank_marker.pk], + ) diff --git a/pulp_python/app/urls.py b/pulp_python/app/urls.py index 309900b9d..7345bd6e3 100644 --- a/pulp_python/app/urls.py +++ b/pulp_python/app/urls.py @@ -7,6 +7,7 @@ PyPIView, SimpleView, UploadView, + YankView, ) if settings.DOMAIN_ENABLED: @@ -40,5 +41,7 @@ SimpleView.as_view({"get": "list", "post": "create"}), name="simple-detail", ), + path(PYPI_API_URL + "yank/", YankView.as_view({"post": "yank"}), name="yank"), + path(PYPI_API_URL + "unyank/", YankView.as_view({"post": "unyank"}), name="unyank"), path(PYPI_API_URL, PyPIView.as_view({"get": "retrieve"}), name="pypi-detail"), ] diff --git a/pulp_python/app/utils.py b/pulp_python/app/utils.py index e9a3258d8..56f1b5b8c 100644 --- a/pulp_python/app/utils.py +++ b/pulp_python/app/utils.py @@ -50,7 +50,6 @@ """ -# TODO in the future: data-yanked (not implemented yet because it is mutable) simple_detail_template = """ @@ -62,6 +61,7 @@ {%- for pkg in project_packages %} {{ pkg.filename }}
@@ -362,7 +362,12 @@ def fetch_json_release_metadata(name: str, version: str, remotes: set[Remote]) - def python_content_to_json( - base_path, content_query, version=None, domain=None, repository_version=None + base_path, + content_query, + version=None, + domain=None, + repository_version=None, + yank_markers=None, ): """ Converts a QuerySet of PythonPackageContent into the PyPi JSON format @@ -375,6 +380,8 @@ def python_content_to_json( Returns None if version is specified but not found within content_query """ + if yank_markers is None: + yank_markers = {} if repository_version: content_query = content_query.annotate( active_membership=FilteredRelation( @@ -386,13 +393,20 @@ def python_content_to_json( ), repo_added_time=F("active_membership__pulp_created"), ) - full_metadata = {"last_serial": 0} # For now the serial field isn't supported by Pulp - latest_content = latest_content_version(content_query, version) + + all_content = list(content_query) + for content in all_content: + content.yanked = content.version in yank_markers + content.yanked_reason = yank_markers.get(content.version) + + latest_content = latest_content_version(all_content, version) if not latest_content: return None - full_metadata.update({"info": python_content_to_info(latest_content[0])}) - full_metadata.update({"releases": python_content_to_releases(content_query, base_path, domain)}) - full_metadata.update({"urls": python_content_to_urls(latest_content, base_path, domain)}) + + full_metadata = {"last_serial": 0} # For now the serial field isn't supported by Pulp + full_metadata["info"] = python_content_to_info(latest_content[0]) + full_metadata["releases"] = python_content_to_releases(all_content, base_path, domain) + full_metadata["urls"] = python_content_to_urls(latest_content, base_path, domain) return full_metadata @@ -462,8 +476,8 @@ def python_content_to_info(content): "platform": content.platform or "", "requires_dist": json_to_dict(content.requires_dist) or None, "classifiers": json_to_dict(content.classifiers) or None, - "yanked": False, # These are no longer used on PyPI, but are still present - "yanked_reason": None, + "yanked": content.yanked, + "yanked_reason": content.yanked_reason, # New core metadata (Version 2.1, 2.2, 2.4) "provides_extras": json_to_dict(content.provides_extras) or None, "dynamic": json_to_dict(content.dynamic) or None, @@ -472,13 +486,13 @@ def python_content_to_info(content): } -def python_content_to_releases(content_query, base_path, domain=None): +def python_content_to_releases(contents, base_path, domain=None): """ - Takes a QuerySet of PythonPackageContent and returns a dictionary of releases + Takes a list of PythonPackageContent and returns a dictionary of releases with each key being a version and value being a list of content for that version of the package """ releases = defaultdict(lambda: []) - for content in content_query: + for content in contents: releases[content.version].append( python_content_to_download_info(content, base_path, domain) ) @@ -535,8 +549,8 @@ def find_artifact(): (getattr(content, "repo_added_time", None) or content.pulp_created).isoformat() ), "url": url, - "yanked": False, - "yanked_reason": None, + "yanked": content.yanked, + "yanked_reason": content.yanked_reason, } @@ -591,7 +605,12 @@ def write_simple_detail_json(project_name, project_packages): "core-metadata": ( {"sha256": package["metadata_sha256"]} if package["metadata_sha256"] else False ), - # yanked and yanked_reason are not implemented because they are mutable + # PEP 592 + "yanked": ( + package["yanked_reason"] + if package["yanked"] and package["yanked_reason"] + else package["yanked"] + ), # (v1.1, PEP 700) "size": package["size"], "upload-time": format_upload_time(package["upload_time"]), diff --git a/pulp_python/app/viewsets.py b/pulp_python/app/viewsets.py index 58a411874..9a5f3d900 100644 --- a/pulp_python/app/viewsets.py +++ b/pulp_python/app/viewsets.py @@ -615,6 +615,29 @@ class PackageProvenanceViewSet(core_viewsets.NoArtifactContentUploadViewSet): } +class PackageYankViewSet(core_viewsets.ReadOnlyContentViewSet): + """ + Read-only viewset for PackageYank content units (PEP 592). + PackageYank markers indicate that a package version has been yanked in a repository. + Use the /yank/ and /unyank/ PyPI endpoints to create or remove these markers. + """ + + endpoint_name = "yanks" + queryset = python_models.PackageYank.objects.all() + serializer_class = python_serializers.PackageYankSerializer + + DEFAULT_ACCESS_POLICY = { + "statements": [ + { + "action": ["list", "retrieve"], + "principal": "authenticated", + "effect": "allow", + }, + ], + "queryset_scoping": {"function": "scope_queryset"}, + } + + class PythonRemoteViewSet(core_viewsets.RemoteViewSet, core_viewsets.RolesMixin): """ diff --git a/pulp_python/tests/functional/api/test_yank.py b/pulp_python/tests/functional/api/test_yank.py new file mode 100644 index 000000000..58e2f6475 --- /dev/null +++ b/pulp_python/tests/functional/api/test_yank.py @@ -0,0 +1,298 @@ +from urllib.parse import urljoin + +import pytest +import requests + +from pulp_python.tests.functional.constants import ( + PYPI_SIMPLE_V1_JSON, + PYTHON_FIXTURES_URL, + TWINE_EGG_FILENAME, + TWINE_EGG_URL, + TWINE_WHEEL_FILENAME, + TWINE_WHEEL_URL, +) + +YANK_AUTH = ("admin", "password") +TWINE_NAME = "twine" +TWINE_VERSION = "5.1.0" + +TWINE_500_WHEEL_FILENAME = "twine-5.0.0-py3-none-any.whl" +TWINE_500_WHEEL_URL = urljoin(urljoin(PYTHON_FIXTURES_URL, "packages/"), TWINE_500_WHEEL_FILENAME) + + +def yank(distro, name, version, yanked_reason=""): + url = urljoin(distro.base_url, "yank/") + return requests.post( + url, json={"name": name, "version": version, "yanked_reason": yanked_reason}, auth=YANK_AUTH + ) + + +def unyank(distro, name, version): + url = urljoin(distro.base_url, "unyank/") + return requests.post(url, json={"name": name, "version": version}, auth=YANK_AUTH) + + +def test_yank_and_unyank( + delete_orphans_pre, + monitor_task, + python_bindings, + python_content_factory, + python_content_summary, + python_distribution_factory, + python_repo_factory, +): + """ + Yank and unyank lifecycle including idempotency and reason update checks. + + Every yank/unyank that changes state creates a new repo version. + Repeating the same operation with the same reason is a no-op (no new version). + Re-yanking with a different reason updates the reason and creates a new version. + """ + content_sdist = python_content_factory(TWINE_EGG_FILENAME, url=TWINE_EGG_URL) + content_whl = python_content_factory(TWINE_WHEEL_FILENAME, url=TWINE_WHEEL_URL) + repo = python_repo_factory() + body = {"add_content_units": [content_sdist.pulp_href, content_whl.pulp_href]} + monitor_task(python_bindings.RepositoriesPythonApi.modify(repo.pulp_href, body).task) + repo = python_bindings.RepositoriesPythonApi.read(repo.pulp_href) + distro = python_distribution_factory(repository=repo) + version_1 = repo.latest_version_href + + # 1. Yank + response = yank(distro, TWINE_NAME, TWINE_VERSION, yanked_reason="broken") + assert response.status_code == 202 + monitor_task(response.json()["task"]) + + repo = python_bindings.RepositoriesPythonApi.read(repo.pulp_href) + version_2 = repo.latest_version_href + assert version_2 != version_1 + summary = python_content_summary(repository=repo, version=2) + assert summary.added["python.python_yank"]["count"] == 1 + + # Check Simple API JSON - yanked files should have yanked reason + simple_url = urljoin(distro.base_url, f"simple/{TWINE_NAME}") + response = requests.get(simple_url, headers={"Accept": PYPI_SIMPLE_V1_JSON}) + assert response.json()["files"][0]["yanked"] == "broken" + assert response.json()["files"][1]["yanked"] == "broken" + + # Check Simple API HTML - data-yanked attribute should be present + response = requests.get(simple_url) + assert response.text.count('data-yanked="broken"') == 2 + + # Check PyPI Metadata API - yanked info in metadata + pypi_url = urljoin(distro.base_url, f"pypi/{TWINE_NAME}/json") + response = requests.get(pypi_url) + data = response.json() + assert data["info"]["yanked"] is True + assert data["info"]["yanked_reason"] == "broken" + for f in data["releases"][TWINE_VERSION]: + assert f["yanked"] is True + assert f["yanked_reason"] == "broken" + for f in data["urls"]: + assert f["yanked"] is True + assert f["yanked_reason"] == "broken" + + # Yank again with same reason - idempotent, no new repo version + response = yank(distro, TWINE_NAME, TWINE_VERSION, yanked_reason="broken") + assert response.status_code == 202 + monitor_task(response.json()["task"]) + + repo = python_bindings.RepositoriesPythonApi.read(repo.pulp_href) + assert repo.latest_version_href == version_2 + + # Yank again with different reason - updates the reason + response = yank(distro, TWINE_NAME, TWINE_VERSION, yanked_reason="security fix") + assert response.status_code == 202 + monitor_task(response.json()["task"]) + + repo = python_bindings.RepositoriesPythonApi.read(repo.pulp_href) + version_3 = repo.latest_version_href + assert version_3 != version_2 + + # Simple API should show updated reason + response = requests.get(simple_url, headers={"Accept": PYPI_SIMPLE_V1_JSON}) + assert response.json()["files"][0]["yanked"] == "security fix" + assert response.json()["files"][1]["yanked"] == "security fix" + + response = requests.get(simple_url) + assert response.text.count('data-yanked="security fix"') == 2 + + # 2. Unyank + response = unyank(distro, TWINE_NAME, TWINE_VERSION) + assert response.status_code == 202 + monitor_task(response.json()["task"]) + + repo = python_bindings.RepositoriesPythonApi.read(repo.pulp_href) + version_4 = repo.latest_version_href + assert version_4 != version_3 + summary = python_content_summary(repository=repo, version=4) + assert summary.removed["python.python_yank"]["count"] == 1 + + # Check Simple API JSON - yanked should be False after unyank + response = requests.get(simple_url, headers={"Accept": PYPI_SIMPLE_V1_JSON}) + assert response.json()["files"][0]["yanked"] is False + assert response.json()["files"][1]["yanked"] is False + + # Check Simple API HTML - data-yanked attribute should not be present + response = requests.get(simple_url) + assert "data-yanked" not in response.text + + # Check PyPI Metadata API - yanked should be False after unyank + response = requests.get(pypi_url) + data = response.json() + assert data["info"]["yanked"] is False + assert data["info"]["yanked_reason"] is None + for f in data["releases"][TWINE_VERSION]: + assert f["yanked"] is False + assert f["yanked_reason"] is None + for f in data["urls"]: + assert f["yanked"] is False + assert f["yanked_reason"] is None + + # Unyank again - idempotent, no new repo version + response = unyank(distro, TWINE_NAME, TWINE_VERSION) + assert response.status_code == 202 + monitor_task(response.json()["task"]) + + repo = python_bindings.RepositoriesPythonApi.read(repo.pulp_href) + assert repo.latest_version_href == version_4 + + +def test_yank_sync( + delete_orphans_pre, + python_remote_factory, + python_repo_with_sync, + python_content_summary, + python_distribution_factory, +): + """ + Syncing a yanked package from upstream creates a PackageYank marker. + """ + remote = python_remote_factory(includes=[f"{TWINE_NAME}=={TWINE_VERSION}"]) + repo = python_repo_with_sync(remote) + distro = python_distribution_factory(repository=repo) + + # Sync should have created a yank marker for twine 5.1.0 + summary = python_content_summary(repository=repo, version=1) + assert summary.added["python.python"]["count"] == 2 + assert summary.added["python.python_yank"]["count"] == 1 + + # Check Simple API JSON - yanked files should have yanked reason + simple_url = urljoin(distro.base_url, f"simple/{TWINE_NAME}") + response = requests.get(simple_url, headers={"Accept": PYPI_SIMPLE_V1_JSON}) + assert response.json()["files"][0]["yanked"] == "https://github.com/pypa/twine/issues/1125" + assert response.json()["files"][1]["yanked"] == "https://github.com/pypa/twine/issues/1125" + + # Check Simple API HTML - data-yanked attribute should be present + response = requests.get(simple_url) + assert response.text.count("data-yanked=") == 2 + + +@pytest.mark.parallel +def test_partial_yank( + monitor_task, + python_bindings, + python_content_factory, + python_content_summary, + python_distribution_factory, + python_repo_factory, +): + """ + Yanking one version does not affect other versions of the same package. + """ + content_510 = python_content_factory(TWINE_WHEEL_FILENAME, url=TWINE_WHEEL_URL) + content_500 = python_content_factory(TWINE_500_WHEEL_FILENAME, url=TWINE_500_WHEEL_URL) + + repo = python_repo_factory() + body = {"add_content_units": [content_510.pulp_href, content_500.pulp_href]} + monitor_task(python_bindings.RepositoriesPythonApi.modify(repo.pulp_href, body).task) + distro = python_distribution_factory(repository=repo) + + response = yank(distro, TWINE_NAME, TWINE_VERSION, yanked_reason="broken 5.1.0") + assert response.status_code == 202 + monitor_task(response.json()["task"]) + + summary = python_content_summary(repository=repo, version=2) + assert summary.added["python.python_yank"]["count"] == 1 + assert summary.present["python.python_yank"]["count"] == 1 + + # Check Simple API JSON - yanked files should have yanked reason + simple_url = urljoin(distro.base_url, f"simple/{TWINE_NAME}") + response = requests.get(simple_url, headers={"Accept": PYPI_SIMPLE_V1_JSON}) + data = response.json() + file_510 = next(f for f in data["files"] if f["filename"] == TWINE_WHEEL_FILENAME) + file_500 = next(f for f in data["files"] if f["filename"] == TWINE_500_WHEEL_FILENAME) + assert file_510["yanked"] == "broken 5.1.0" + assert file_500["yanked"] is False + + +@pytest.mark.parallel +def test_yank_isolation_across_repositories( + monitor_task, + python_bindings, + python_content_factory, + python_content_summary, + python_distribution_factory, + python_repo_factory, +): + """ + Yanking in one repo does not affect another repo with the same content. + """ + content = python_content_factory(TWINE_WHEEL_FILENAME, url=TWINE_WHEEL_URL) + + repo_a = python_repo_factory() + repo_b = python_repo_factory() + body = {"add_content_units": [content.pulp_href]} + monitor_task(python_bindings.RepositoriesPythonApi.modify(repo_a.pulp_href, body).task) + monitor_task(python_bindings.RepositoriesPythonApi.modify(repo_b.pulp_href, body).task) + + distro_a = python_distribution_factory(repository=repo_a) + distro_b = python_distribution_factory(repository=repo_b) + + # Yank in repo A only + response = yank(distro_a, TWINE_NAME, TWINE_VERSION) + assert response.status_code == 202 + monitor_task(response.json()["task"]) + + # Repo A should have a yank marker, repo B should not + summary_a = python_content_summary(repository=repo_a, version=2) + assert summary_a.present["python.python_yank"]["count"] == 1 + summary_b = python_content_summary(repository=repo_b, version=1) + assert "python.python_yank" not in summary_b.present + + # Yank in repo B too, then unyank only in repo A + response = yank(distro_b, TWINE_NAME, TWINE_VERSION) + assert response.status_code == 202 + monitor_task(response.json()["task"]) + + response = unyank(distro_a, TWINE_NAME, TWINE_VERSION) + assert response.status_code == 202 + monitor_task(response.json()["task"]) + + # Repo A should be not-yanked, repo B should remain yanked + summary_a = python_content_summary(repository=repo_a, version=3) + assert "python.python_yank" not in summary_a.present + summary_b = python_content_summary(repository=repo_b, version=2) + assert summary_b.present["python.python_yank"]["count"] == 1 + + +@pytest.mark.parallel +def test_yank_nonexistent_package(python_repo_factory, python_distribution_factory): + """ + Yanking a package not in the repo should return 404. + """ + repo = python_repo_factory() + distro = python_distribution_factory(repository=repo) + + response = yank(distro, "nonexistent-package", "99.99.99") + assert response.status_code == 404 + + +@pytest.mark.parallel +def test_yank_no_repository(python_distribution_factory): + """ + Yanking on a distribution with no repository should return 400. + """ + distro = python_distribution_factory() + + response = yank(distro, TWINE_NAME, TWINE_VERSION) + assert response.status_code == 400 From 869b6f236173ffa8f1064b93fe1794c3f252aa08 Mon Sep 17 00:00:00 2001 From: Jitka Halova Date: Mon, 3 Aug 2026 15:42:45 +0200 Subject: [PATCH 2/2] tbs feedback --- pulp_python/app/pypi/views.py | 13 ++- pulp_python/app/utils.py | 18 ++-- pulp_python/tests/functional/api/test_yank.py | 100 ++++++++++-------- 3 files changed, 74 insertions(+), 57 deletions(-) diff --git a/pulp_python/app/pypi/views.py b/pulp_python/app/pypi/views.py index c42c520b8..348a91e4e 100644 --- a/pulp_python/app/pypi/views.py +++ b/pulp_python/app/pypi/views.py @@ -29,6 +29,7 @@ from rest_framework.response import Response from rest_framework.viewsets import ViewSet +from pulpcore.plugin.serializers import AsyncOperationResponseSerializer from pulpcore.plugin.tasking import dispatch from pulpcore.plugin.util import get_domain, get_url from pulpcore.plugin.viewsets import OperationPostponedResponse @@ -620,7 +621,11 @@ class YankView(PyPIMixin, ViewSet): ], } - @extend_schema(request=YankSerializer, summary="Yank a package version") + @extend_schema( + request=YankSerializer, + responses={202: AsyncOperationResponseSerializer}, + summary="Yank a package version", + ) def yank(self, request, path): """Yank a package version, marking all its files with data-yanked.""" repo = self.distribution.repository @@ -650,7 +655,11 @@ def yank(self, request, path): ) return OperationPostponedResponse(result, request) - @extend_schema(request=YankSerializer, summary="Unyank a package version") + @extend_schema( + request=YankSerializer, + responses={202: AsyncOperationResponseSerializer}, + summary="Unyank a package version", + ) def unyank(self, request, path): """Unyank a package version, unmarking all its files with data-yanked.""" repo = self.distribution.repository diff --git a/pulp_python/app/utils.py b/pulp_python/app/utils.py index 56f1b5b8c..57e78b3bf 100644 --- a/pulp_python/app/utils.py +++ b/pulp_python/app/utils.py @@ -410,15 +410,15 @@ def python_content_to_json( return full_metadata -def latest_content_version(content_query, version): +def latest_content_version(all_content, version): """ - Walks through the content QuerySet and finds the instances that is the latest version. + Walks through the content list and finds the instances that are the latest version. If 'version' is specified, the function instead tries to find content instances with that version and will return an empty list if nothing is found """ latest_version = version latest_content = [] - for content in content_query: + for content in all_content: if version and parse(version) == parse(content.version): latest_content.append(content) elif not latest_version or parse(content.version) > parse(latest_version): @@ -476,8 +476,8 @@ def python_content_to_info(content): "platform": content.platform or "", "requires_dist": json_to_dict(content.requires_dist) or None, "classifiers": json_to_dict(content.classifiers) or None, - "yanked": content.yanked, - "yanked_reason": content.yanked_reason, + "yanked": getattr(content, "yanked", False), + "yanked_reason": getattr(content, "yanked_reason", None), # New core metadata (Version 2.1, 2.2, 2.4) "provides_extras": json_to_dict(content.provides_extras) or None, "dynamic": json_to_dict(content.dynamic) or None, @@ -486,13 +486,13 @@ def python_content_to_info(content): } -def python_content_to_releases(contents, base_path, domain=None): +def python_content_to_releases(all_content, base_path, domain=None): """ Takes a list of PythonPackageContent and returns a dictionary of releases with each key being a version and value being a list of content for that version of the package """ releases = defaultdict(lambda: []) - for content in contents: + for content in all_content: releases[content.version].append( python_content_to_download_info(content, base_path, domain) ) @@ -549,8 +549,8 @@ def find_artifact(): (getattr(content, "repo_added_time", None) or content.pulp_created).isoformat() ), "url": url, - "yanked": content.yanked, - "yanked_reason": content.yanked_reason, + "yanked": getattr(content, "yanked", False), + "yanked_reason": getattr(content, "yanked_reason", None), } diff --git a/pulp_python/tests/functional/api/test_yank.py b/pulp_python/tests/functional/api/test_yank.py index 58e2f6475..c3940873e 100644 --- a/pulp_python/tests/functional/api/test_yank.py +++ b/pulp_python/tests/functional/api/test_yank.py @@ -12,7 +12,6 @@ TWINE_WHEEL_URL, ) -YANK_AUTH = ("admin", "password") TWINE_NAME = "twine" TWINE_VERSION = "5.1.0" @@ -20,18 +19,6 @@ TWINE_500_WHEEL_URL = urljoin(urljoin(PYTHON_FIXTURES_URL, "packages/"), TWINE_500_WHEEL_FILENAME) -def yank(distro, name, version, yanked_reason=""): - url = urljoin(distro.base_url, "yank/") - return requests.post( - url, json={"name": name, "version": version, "yanked_reason": yanked_reason}, auth=YANK_AUTH - ) - - -def unyank(distro, name, version): - url = urljoin(distro.base_url, "unyank/") - return requests.post(url, json={"name": name, "version": version}, auth=YANK_AUTH) - - def test_yank_and_unyank( delete_orphans_pre, monitor_task, @@ -58,9 +45,11 @@ def test_yank_and_unyank( version_1 = repo.latest_version_href # 1. Yank - response = yank(distro, TWINE_NAME, TWINE_VERSION, yanked_reason="broken") - assert response.status_code == 202 - monitor_task(response.json()["task"]) + response = python_bindings.PypiYankApi.yank( + path=distro.base_path, + yank={"name": TWINE_NAME, "version": TWINE_VERSION, "yanked_reason": "broken"}, + ) + monitor_task(response.task) repo = python_bindings.RepositoriesPythonApi.read(repo.pulp_href) version_2 = repo.latest_version_href @@ -92,17 +81,21 @@ def test_yank_and_unyank( assert f["yanked_reason"] == "broken" # Yank again with same reason - idempotent, no new repo version - response = yank(distro, TWINE_NAME, TWINE_VERSION, yanked_reason="broken") - assert response.status_code == 202 - monitor_task(response.json()["task"]) + response = python_bindings.PypiYankApi.yank( + path=distro.base_path, + yank={"name": TWINE_NAME, "version": TWINE_VERSION, "yanked_reason": "broken"}, + ) + monitor_task(response.task) repo = python_bindings.RepositoriesPythonApi.read(repo.pulp_href) assert repo.latest_version_href == version_2 # Yank again with different reason - updates the reason - response = yank(distro, TWINE_NAME, TWINE_VERSION, yanked_reason="security fix") - assert response.status_code == 202 - monitor_task(response.json()["task"]) + response = python_bindings.PypiYankApi.yank( + path=distro.base_path, + yank={"name": TWINE_NAME, "version": TWINE_VERSION, "yanked_reason": "security fix"}, + ) + monitor_task(response.task) repo = python_bindings.RepositoriesPythonApi.read(repo.pulp_href) version_3 = repo.latest_version_href @@ -117,9 +110,10 @@ def test_yank_and_unyank( assert response.text.count('data-yanked="security fix"') == 2 # 2. Unyank - response = unyank(distro, TWINE_NAME, TWINE_VERSION) - assert response.status_code == 202 - monitor_task(response.json()["task"]) + response = python_bindings.PypiUnyankApi.unyank( + path=distro.base_path, yank={"name": TWINE_NAME, "version": TWINE_VERSION} + ) + monitor_task(response.task) repo = python_bindings.RepositoriesPythonApi.read(repo.pulp_href) version_4 = repo.latest_version_href @@ -149,9 +143,10 @@ def test_yank_and_unyank( assert f["yanked_reason"] is None # Unyank again - idempotent, no new repo version - response = unyank(distro, TWINE_NAME, TWINE_VERSION) - assert response.status_code == 202 - monitor_task(response.json()["task"]) + response = python_bindings.PypiUnyankApi.unyank( + path=distro.base_path, yank={"name": TWINE_NAME, "version": TWINE_VERSION} + ) + monitor_task(response.task) repo = python_bindings.RepositoriesPythonApi.read(repo.pulp_href) assert repo.latest_version_href == version_4 @@ -207,9 +202,11 @@ def test_partial_yank( monitor_task(python_bindings.RepositoriesPythonApi.modify(repo.pulp_href, body).task) distro = python_distribution_factory(repository=repo) - response = yank(distro, TWINE_NAME, TWINE_VERSION, yanked_reason="broken 5.1.0") - assert response.status_code == 202 - monitor_task(response.json()["task"]) + response = python_bindings.PypiYankApi.yank( + path=distro.base_path, + yank={"name": TWINE_NAME, "version": TWINE_VERSION, "yanked_reason": "broken 5.1.0"}, + ) + monitor_task(response.task) summary = python_content_summary(repository=repo, version=2) assert summary.added["python.python_yank"]["count"] == 1 @@ -249,9 +246,10 @@ def test_yank_isolation_across_repositories( distro_b = python_distribution_factory(repository=repo_b) # Yank in repo A only - response = yank(distro_a, TWINE_NAME, TWINE_VERSION) - assert response.status_code == 202 - monitor_task(response.json()["task"]) + response = python_bindings.PypiYankApi.yank( + path=distro_a.base_path, yank={"name": TWINE_NAME, "version": TWINE_VERSION} + ) + monitor_task(response.task) # Repo A should have a yank marker, repo B should not summary_a = python_content_summary(repository=repo_a, version=2) @@ -260,13 +258,15 @@ def test_yank_isolation_across_repositories( assert "python.python_yank" not in summary_b.present # Yank in repo B too, then unyank only in repo A - response = yank(distro_b, TWINE_NAME, TWINE_VERSION) - assert response.status_code == 202 - monitor_task(response.json()["task"]) + response = python_bindings.PypiYankApi.yank( + path=distro_b.base_path, yank={"name": TWINE_NAME, "version": TWINE_VERSION} + ) + monitor_task(response.task) - response = unyank(distro_a, TWINE_NAME, TWINE_VERSION) - assert response.status_code == 202 - monitor_task(response.json()["task"]) + response = python_bindings.PypiUnyankApi.unyank( + path=distro_a.base_path, yank={"name": TWINE_NAME, "version": TWINE_VERSION} + ) + monitor_task(response.task) # Repo A should be not-yanked, repo B should remain yanked summary_a = python_content_summary(repository=repo_a, version=3) @@ -276,23 +276,31 @@ def test_yank_isolation_across_repositories( @pytest.mark.parallel -def test_yank_nonexistent_package(python_repo_factory, python_distribution_factory): +def test_yank_nonexistent_package( + python_bindings, python_repo_factory, python_distribution_factory +): """ Yanking a package not in the repo should return 404. """ repo = python_repo_factory() distro = python_distribution_factory(repository=repo) - response = yank(distro, "nonexistent-package", "99.99.99") - assert response.status_code == 404 + with pytest.raises(python_bindings.ApiException) as exc: + python_bindings.PypiYankApi.yank( + path=distro.base_path, yank={"name": "nonexistent-package", "version": "99.99.99"} + ) + assert exc.value.status == 404 @pytest.mark.parallel -def test_yank_no_repository(python_distribution_factory): +def test_yank_no_repository(python_bindings, python_distribution_factory): """ Yanking on a distribution with no repository should return 400. """ distro = python_distribution_factory() - response = yank(distro, TWINE_NAME, TWINE_VERSION) - assert response.status_code == 400 + with pytest.raises(python_bindings.ApiException) as exc: + python_bindings.PypiYankApi.yank( + path=distro.base_path, yank={"name": TWINE_NAME, "version": TWINE_VERSION} + ) + assert exc.value.status == 400