Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/+add-pulp-exceptions.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add more Pulp Exceptions.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a bug, but rather a feature.

140 changes: 140 additions & 0 deletions pulp_python/app/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
from gettext import gettext as _

from pulpcore.plugin.exceptions import PulpException


class ProvenanceVerificationError(PulpException):
"""
Raised when provenance verification fails.
"""

error_code = "PLPY0001"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we already have a naming convention for plugins? Pulpcore uses "PLP", this introduces "PLPY"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have a strict convention for plugins, but some plugins have already created exceptions with codes, so until we have a reason to make it all stricter we will go with it.


def __init__(self, message):
"""
:param message: Description of the provenance verification error
:type message: str
"""
self.message = message

def __str__(self):
return f"[{self.error_code}] " + _("Provenance verification failed: {message}").format(
message=self.message
)


class AttestationVerificationError(PulpException):
"""
Raised when attestation verification fails.
"""

error_code = "PLPY0002"

def __init__(self, message):
"""
:param message: Description of the attestation verification error
:type message: str
"""
self.message = message

def __str__(self):
return f"[{self.error_code}] " + _("Attestation verification failed: {message}").format(
message=self.message
)


class PackageSubstitutionError(PulpException):
"""
Raised when packages with the same filename but different checksums are being added.
"""

error_code = "PLPY0003"

def __init__(self, duplicates):
"""
:param duplicates: Description of duplicate packages
:type duplicates: str
"""
self.duplicates = duplicates

def __str__(self):
return (
f"[{self.error_code}] "
+ _(
"Found duplicate packages being added with the same filename but different checksums. " # noqa: E501
)
+ _("To allow this, set 'allow_package_substitution' to True on the repository. ")
+ _("Conflicting packages: {duplicates}").format(duplicates=self.duplicates)
)
Comment on lines +61 to +68
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    return f"[{self.error_code}] " + _(
        "Found duplicate packages being added with the same filename but different "
        "checksums. To allow this, set 'allow_package_substitution' to True on the "
        "repository. Conflicting packages: {duplicates}"
    ).format(duplicates=self.duplicates)



class UnsupportedProtocolError(PulpException):
"""
Raised when an unsupported protocol is used for syncing.
"""

error_code = "PLPY0004"

def __init__(self, protocol):
"""
:param protocol: The unsupported protocol
:type protocol: str
"""
self.protocol = protocol

def __str__(self):
return f"[{self.error_code}] " + _(
"Only HTTP(S) is supported for python syncing, got: {protocol}"
).format(protocol=self.protocol)


class MissingRelativePathError(PulpException):
"""
Raised when relative_path field is missing during package upload.
"""

error_code = "PLPY0005"

def __str__(self):
return f"[{self.error_code}] " + _("This field is required: relative_path")


class InvalidPythonExtensionError(PulpException):
"""
Raised when a file has an invalid Python package extension.
"""

error_code = "PLPY0006"

def __init__(self, filename):
"""
:param filename: The filename with invalid extension
:type filename: str
"""
self.filename = filename

def __str__(self):
return f"[{self.error_code}] " + _(
"Extension on {filename} is not a valid python extension "
"(.whl, .exe, .egg, .tar.gz, .tar.bz2, .zip)"
).format(filename=self.filename)


class InvalidProvenanceError(PulpException):
"""
Raised when uploaded provenance data is invalid.
"""

error_code = "PLPY0007"

def __init__(self, message):
"""
:param message: Description of the provenance validation error
:type message: str
"""
self.message = message

def __str__(self):
return f"[{self.error_code}] " + _(
"The uploaded provenance is not valid: {message}"
).format(message=self.message)
12 changes: 4 additions & 8 deletions pulp_python/app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
BEFORE_SAVE,
hook,
)
from rest_framework.serializers import ValidationError
from pulpcore.plugin.models import (
AutoAddObjPermsMixin,
Content,
Expand All @@ -23,6 +22,7 @@
from pulpcore.plugin.responses import ArtifactResponse

from pathlib import PurePath
from .exceptions import PackageSubstitutionError
from .provenance import Provenance
from .utils import (
artifact_to_python_content_data,
Expand Down Expand Up @@ -407,14 +407,10 @@ 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.
Raise a PackageSubstitutionError if newly added packages would replace existing packages
that have the same filename but a different sha256 checksum.
"""
qs = PythonPackageContent.objects.filter(pk__in=new_version.content)
duplicates = collect_duplicates(qs, ("filename",))
if duplicates:
raise ValidationError(
"Found duplicate packages being added with the same filename but different checksums. " # noqa: E501
"To allow this, set 'allow_package_substitution' to True on the repository. "
f"Conflicting packages: {duplicates}"
)
raise PackageSubstitutionError(duplicates)
42 changes: 19 additions & 23 deletions pulp_python/app/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,22 @@
from packaging.requirements import Requirement
from rest_framework import serializers
from pypi_attestations import AttestationError
from pydantic import TypeAdapter, ValidationError
from pydantic import TypeAdapter, ValidationError as PydanticValidationError
from urllib.parse import urljoin

from pulpcore.plugin.exceptions import DigestValidationError
from pulpcore.plugin import models as core_models
from pulpcore.plugin import serializers as core_serializers
from pulpcore.plugin.util import get_domain, get_prn, get_current_authenticated_user

from pulp_python.app import models as python_models
from pulp_python.app.exceptions import (
AttestationVerificationError,
InvalidProvenanceError,
InvalidPythonExtensionError,
MissingRelativePathError,
ProvenanceVerificationError,
)
from pulp_python.app.provenance import (
Attestation,
Provenance,
Expand Down Expand Up @@ -374,7 +382,7 @@ def validate_attestations(self, value):
attestations = TypeAdapter(list[Attestation]).validate_json(value)
else:
attestations = TypeAdapter(list[Attestation]).validate_python(value)
except ValidationError as e:
except PydanticValidationError as e:
raise serializers.ValidationError(_("Invalid attestations: {}".format(e)))
return attestations

Expand All @@ -387,9 +395,7 @@ def handle_attestations(self, filename, sha256, attestations, offline=True):
try:
verify_provenance(filename, sha256, provenance, offline=offline)
except AttestationError as e:
raise serializers.ValidationError(
{"attestations": _("Attestations failed verification: {}".format(e))}
)
raise AttestationVerificationError(str(e))
return provenance.model_dump(mode="json")

def deferred_validate(self, data):
Expand All @@ -408,26 +414,18 @@ def deferred_validate(self, data):
try:
filename = data["relative_path"]
except KeyError:
raise serializers.ValidationError(detail={"relative_path": _("This field is required")})
raise MissingRelativePathError()

artifact = data["artifact"]
try:
_data = artifact_to_python_content_data(filename, artifact, domain=get_domain())
except ValueError:
raise serializers.ValidationError(
_(
"Extension on {} is not a valid python extension "
"(.whl, .exe, .egg, .tar.gz, .tar.bz2, .zip)"
).format(filename)
)
raise InvalidPythonExtensionError(filename)

if data.get("sha256") and data["sha256"] != artifact.sha256:
raise serializers.ValidationError(
detail={
"sha256": _(
"The uploaded artifact's sha256 checksum does not match the one provided"
)
}
raise DigestValidationError(
actual=artifact.sha256,
expected=data["sha256"],
)

data.update(_data)
Expand Down Expand Up @@ -641,15 +639,13 @@ def deferred_validate(self, data):
try:
provenance = Provenance.model_validate_json(data["file"].read())
data["provenance"] = provenance.model_dump(mode="json")
except ValidationError as e:
raise serializers.ValidationError(
_("The uploaded provenance is not valid: {}".format(e))
)
except PydanticValidationError as e:
raise InvalidProvenanceError(str(e))
if data.pop("verify"):
try:
verify_provenance(data["package"].filename, data["package"].sha256, provenance)
except AttestationError as e:
raise serializers.ValidationError(_("Provenance verification failed: {}".format(e)))
raise ProvenanceVerificationError(str(e))
return data

def retrieve(self, validated_data):
Expand Down
10 changes: 5 additions & 5 deletions pulp_python/app/tasks/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@

from aiohttp import ClientResponseError, ClientError
from lxml.etree import LxmlError
from gettext import gettext as _
from functools import partial

from rest_framework import serializers

from pulpcore.plugin.exceptions import SyncError
from pulpcore.plugin.download import HttpDownloader
from pulpcore.plugin.models import Artifact, ProgressReport, Remote, Repository
from pulpcore.plugin.stages import (
Expand All @@ -17,6 +15,7 @@
Stage,
)

from pulp_python.app.exceptions import UnsupportedProtocolError
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this? I do not remember that is was reported by CI. It also looks like sth that should be in pulpcore.

from pulp_python.app.models import (
PythonPackageContent,
PythonRemote,
Expand Down Expand Up @@ -54,7 +53,7 @@ def sync(remote_pk, repository_pk, mirror):
repository = Repository.objects.get(pk=repository_pk)

if not remote.url:
raise serializers.ValidationError(detail=_("A remote must have a url attribute to sync."))
raise SyncError("A remote must have a url attribute to sync.")

first_stage = PythonBanderStage(remote)
DeclarativeVersion(first_stage, repository, mirror).create()
Expand Down Expand Up @@ -117,7 +116,8 @@ async def run(self):
url = self.remote.url.rstrip("/")
downloader = self.remote.get_downloader(url=url)
if not isinstance(downloader, HttpDownloader):
raise ValueError("Only HTTP(S) is supported for python syncing")
protocol = type(downloader).__name__
raise UnsupportedProtocolError(protocol)

async with Master(url, allow_non_https=True) as master:
# Replace the session with the remote's downloader session
Expand Down
4 changes: 2 additions & 2 deletions pulp_python/tests/functional/api/test_attestations.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def test_verify_provenance(python_bindings, twine_package, python_content_factor
with pytest.raises(PulpTaskError) as e:
monitor_task(provenance.task)
assert e.value.task.state == "failed"
assert "twine-6.2.0-py3-none-any.whl != twine-6.2.0.tar.gz" in e.value.task.error["description"]
assert "[PLPY0001]" in e.value.task.error["description"]

# Test creating a provenance without verifying
provenance = python_bindings.ContentProvenanceApi.create(
Expand Down Expand Up @@ -239,4 +239,4 @@ def test_bad_attestation_upload(python_bindings, twine_package, monitor_task):
with pytest.raises(PulpTaskError) as e:
monitor_task(task)
assert e.value.task.state == "failed"
assert "Attestations failed verification" in e.value.task.error["description"]
assert "[PLPY0002]" in e.value.task.error["description"]
4 changes: 3 additions & 1 deletion pulp_python/tests/functional/api/test_crud_content_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def test_content_crud(
with pytest.raises(PulpTaskError) as e:
response = python_bindings.ContentPackagesApi.create(**content_body)
monitor_task(response.task)
msg = "The uploaded artifact's sha256 checksum does not match the one provided"
msg = "[PLP0003]"
assert msg in e.value.task.error["description"]


Expand Down Expand Up @@ -241,6 +241,7 @@ def test_disallow_package_substitution(
repository=repo.pulp_href, **content_body2
)
monitor_task(response.task)
assert "[PLPY0003]" in exc.value.task.error["description"]
assert msg1 in exc.value.task.error["description"]
assert msg2 in exc.value.task.error["description"]

Expand All @@ -257,6 +258,7 @@ def test_disallow_package_substitution(
body = {"add_content_units": [content2.pulp_href], "base_version": repo.latest_version_href}
with pytest.raises(PulpTaskError) as exc:
monitor_task(python_bindings.RepositoriesPythonApi.modify(repo.pulp_href, body).task)
assert "[PLPY0003]" in exc.value.task.error["description"]
assert msg1 in exc.value.task.error["description"]
assert msg2 in exc.value.task.error["description"]

Expand Down
Loading