From ea1dc164048dc88fbf1f53774e47c5c508422f78 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Mon, 27 Oct 2025 15:36:56 -0400 Subject: [PATCH] Add more fields and validation to twine upload endpoint --- pulp_python/app/pypi/serializers.py | 38 +++++++-- pulp_python/app/utils.py | 1 + pulp_python/pytest_plugin.py | 30 +++++++ .../tests/functional/api/test_pypi_apis.py | 30 ------- .../tests/functional/api/test_upload.py | 80 +++++++++++++++++++ 5 files changed, 144 insertions(+), 35 deletions(-) diff --git a/pulp_python/app/pypi/serializers.py b/pulp_python/app/pypi/serializers.py index 1692072b..84c36dbd 100644 --- a/pulp_python/app/pypi/serializers.py +++ b/pulp_python/app/pypi/serializers.py @@ -2,7 +2,7 @@ from gettext import gettext as _ from rest_framework import serializers -from pulp_python.app.utils import DIST_EXTENSIONS +from pulp_python.app.utils import DIST_EXTENSIONS, SUPPORTED_METADATA_VERSIONS from pulpcore.plugin.models import Artifact from pulpcore.plugin.util import get_domain from django.db.utils import IntegrityError @@ -54,6 +54,22 @@ class PackageUploadSerializer(serializers.Serializer): min_length=64, max_length=64, ) + protocol_version = serializers.ChoiceField( + help_text=_("Protocol version to use for the upload. Only version 1 is supported."), + required=False, + choices=(1,), + default=1, + ) + filetype = serializers.ChoiceField( + help_text=_("Type of artifact to upload."), + required=False, + choices=("bdist_wheel", "sdist"), + ) + metadata_version = serializers.ChoiceField( + help_text=_("Metadata version of the uploaded package."), + required=False, + choices=SUPPORTED_METADATA_VERSIONS, + ) def validate(self, data): """Validates the request.""" @@ -63,14 +79,26 @@ def validate(self, data): file = data.get("content") for ext, packagetype in DIST_EXTENSIONS.items(): if file.name.endswith(ext): + if filetype := data.get("filetype"): + if filetype != packagetype: + raise serializers.ValidationError( + { + "filetype": _( + "filetype {} does not match found filetype {} for file {}" + ).format(filetype, packagetype, file.name) + } + ) break else: raise serializers.ValidationError( - _( - "Extension on {} is not a valid python extension " - "(.whl, .exe, .egg, .tar.gz, .tar.bz2, .zip)" - ).format(file.name) + { + "content": _( + "Extension on {} is not a valid python extension " + "(.whl, .exe, .egg, .tar.gz, .tar.bz2, .zip)" + ).format(file.name) + } ) + sha256 = data.get("sha256_digest") digests = {"sha256": sha256} if sha256 else None artifact = Artifact.init_and_validate(file, expected_digests=digests) diff --git a/pulp_python/app/utils.py b/pulp_python/app/utils.py index 365de503..8bfa304b 100644 --- a/pulp_python/app/utils.py +++ b/pulp_python/app/utils.py @@ -17,6 +17,7 @@ PYPI_LAST_SERIAL = "X-PYPI-LAST-SERIAL" """TODO This serial constant is temporary until Python repositories implements serials""" PYPI_SERIAL_CONSTANT = 1000000000 +SUPPORTED_METADATA_VERSIONS = ("1.0", "1.1", "1.2", "2.0", "2.1", "2.2", "2.3", "2.4") SIMPLE_API_VERSION = "1.0" diff --git a/pulp_python/pytest_plugin.py b/pulp_python/pytest_plugin.py index 1a3f756c..5d50ef9d 100644 --- a/pulp_python/pytest_plugin.py +++ b/pulp_python/pytest_plugin.py @@ -8,6 +8,9 @@ PYTHON_XS_PROJECT_SPECIFIER, PYTHON_EGG_FILENAME, PYTHON_URL, + PYTHON_EGG_URL, + PYTHON_WHEEL_URL, + PYTHON_WHEEL_FILENAME, ) @@ -183,6 +186,20 @@ def _gen_python_content(relative_path=PYTHON_EGG_FILENAME, url=None, **body): yield _gen_python_content +@pytest.fixture +def python_empty_repo_distro(python_repo_factory, python_distribution_factory): + """Returns an empty repo with and distribution serving it.""" + + def _generate_empty_repo_distro(repo_body=None, distro_body=None): + repo_body = repo_body or {} + distro_body = distro_body or {} + repo = python_repo_factory(**repo_body) + distro = python_distribution_factory(repository=repo, **distro_body) + return repo, distro + + yield _generate_empty_repo_distro + + # Utility fixtures @@ -217,3 +234,16 @@ def _gen_summary(repository_version=None, repository=None, version=None): def get_href(item): """Tries to get the href from the given item, whether it is a string or object.""" return item if isinstance(item, str) else item.pulp_href + + +@pytest.fixture(scope="session") +def python_package_dist_directory(tmp_path_factory, http_get): + """Creates a temp dir to hold package distros for uploading.""" + dist_dir = tmp_path_factory.mktemp("dist") + egg_file = dist_dir / PYTHON_EGG_FILENAME + wheel_file = dist_dir / PYTHON_WHEEL_FILENAME + with open(egg_file, "wb") as f: + f.write(http_get(PYTHON_EGG_URL)) + with open(wheel_file, "wb") as f: + f.write(http_get(PYTHON_WHEEL_URL)) + yield dist_dir, egg_file, wheel_file diff --git a/pulp_python/tests/functional/api/test_pypi_apis.py b/pulp_python/tests/functional/api/test_pypi_apis.py index 9ed002cc..58cd7b61 100644 --- a/pulp_python/tests/functional/api/test_pypi_apis.py +++ b/pulp_python/tests/functional/api/test_pypi_apis.py @@ -11,10 +11,7 @@ PYTHON_MD_PROJECT_SPECIFIER, PYTHON_MD_PYPI_SUMMARY, PYTHON_EGG_FILENAME, - PYTHON_EGG_URL, PYTHON_EGG_SHA256, - PYTHON_WHEEL_FILENAME, - PYTHON_WHEEL_URL, PYTHON_WHEEL_SHA256, SHELF_PYTHON_JSON, ) @@ -26,20 +23,6 @@ PYPI_SERIAL_CONSTANT = 1000000000 -@pytest.fixture -def python_empty_repo_distro(python_repo_factory, python_distribution_factory): - """Returns an empty repo with and distribution serving it.""" - - def _generate_empty_repo_distro(repo_body=None, distro_body=None): - repo_body = repo_body or {} - distro_body = distro_body or {} - repo = python_repo_factory(**repo_body) - distro = python_distribution_factory(repository=repo, **distro_body) - return repo, distro - - yield _generate_empty_repo_distro - - @pytest.mark.parallel def test_empty_index(python_bindings, python_empty_repo_distro): """Checks that summary stats are 0 when index is empty.""" @@ -80,19 +63,6 @@ def test_published_index( assert summary.to_dict() == PYTHON_MD_PYPI_SUMMARY -@pytest.fixture(scope="module") -def python_package_dist_directory(tmp_path_factory, http_get): - """Creates a temp dir to hold package distros for uploading.""" - dist_dir = tmp_path_factory.mktemp("dist") - egg_file = dist_dir / PYTHON_EGG_FILENAME - wheel_file = dist_dir / PYTHON_WHEEL_FILENAME - with open(egg_file, "wb") as f: - f.write(http_get(PYTHON_EGG_URL)) - with open(wheel_file, "wb") as f: - f.write(http_get(PYTHON_WHEEL_URL)) - yield dist_dir, egg_file, wheel_file - - @pytest.mark.parallel def test_package_upload( python_content_summary, python_empty_repo_distro, python_package_dist_directory, monitor_task diff --git a/pulp_python/tests/functional/api/test_upload.py b/pulp_python/tests/functional/api/test_upload.py index 9071cf58..33f67517 100644 --- a/pulp_python/tests/functional/api/test_upload.py +++ b/pulp_python/tests/functional/api/test_upload.py @@ -1,10 +1,14 @@ import pytest +import requests from pulp_python.tests.functional.constants import ( PYTHON_EGG_FILENAME, PYTHON_EGG_URL, PYTHON_WHEEL_FILENAME, PYTHON_WHEEL_URL, + PYTHON_EGG_SHA256, + PYTHON_WHEEL_SHA256, ) +from urllib.parse import urljoin @pytest.mark.parametrize( @@ -42,3 +46,79 @@ def test_synchronous_package_upload( with pytest.raises(python_bindings.ApiException) as ctx: python_bindings.ContentPackagesApi.upload(**content_body) assert ctx.value.status == 403 + + +@pytest.mark.parallel +def test_legacy_upload_invalid_protocol_version( + python_empty_repo_distro, python_package_dist_directory +): + _, egg_file, _ = python_package_dist_directory + _, distro = python_empty_repo_distro() + url = urljoin(distro.base_url, "legacy/") + with open(egg_file, "rb") as f: + response = requests.post( + url, + data={"sha256_digest": PYTHON_EGG_SHA256, "protocol_version": 2}, + files={"content": f}, + auth=("admin", "password"), + ) + assert response.status_code == 400 + assert response.json()["protocol_version"] == ['"2" is not a valid choice.'] + + with open(egg_file, "rb") as f: + response = requests.post( + url, + data={"sha256_digest": PYTHON_EGG_SHA256, "protocol_version": 0}, + files={"content": f}, + auth=("admin", "password"), + ) + assert response.status_code == 400 + assert response.json()["protocol_version"] == ['"0" is not a valid choice.'] + + +@pytest.mark.parallel +def test_legacy_upload_invalid_filetype(python_empty_repo_distro, python_package_dist_directory): + _, egg_file, wheel_file = python_package_dist_directory + _, distro = python_empty_repo_distro() + url = urljoin(distro.base_url, "legacy/") + with open(egg_file, "rb") as f: + response = requests.post( + url, + data={"sha256_digest": PYTHON_EGG_SHA256, "filetype": "bdist_wheel"}, + files={"content": f}, + auth=("admin", "password"), + ) + assert response.status_code == 400 + assert response.json()["filetype"] == [ + "filetype bdist_wheel does not match found filetype sdist for file shelf-reader-0.1.tar.gz" + ] + + with open(wheel_file, "rb") as f: + response = requests.post( + url, + data={"sha256_digest": PYTHON_WHEEL_SHA256, "filetype": "sdist"}, + files={"content": f}, + auth=("admin", "password"), + ) + assert response.status_code == 400 + assert response.json()["filetype"] == [ + "filetype sdist does not match found filetype bdist_wheel for file shelf_reader-0.1-py2-none-any.whl" # noqa: E501 + ] + + +@pytest.mark.parallel +def test_legacy_upload_invalid_metadata_version( + python_empty_repo_distro, python_package_dist_directory +): + _, egg_file, _ = python_package_dist_directory + _, distro = python_empty_repo_distro() + url = urljoin(distro.base_url, "legacy/") + with open(egg_file, "rb") as f: + response = requests.post( + url, + data={"sha256_digest": PYTHON_EGG_SHA256, "metadata_version": "3.0"}, + files={"content": f}, + auth=("admin", "password"), + ) + assert response.status_code == 400 + assert response.json()["metadata_version"] == ['"3.0" is not a valid choice.']