From f2c4f63e87593b2aa8e004f451f440d4d90bc494 Mon Sep 17 00:00:00 2001 From: Aliaksei Klimau Date: Mon, 13 Apr 2026 13:57:39 +0200 Subject: [PATCH] Add more Pulp Exceptions. --- CHANGES/+add-pulp-exceptions.feature | 1 + pulp_ansible/app/serializers.py | 18 +- pulp_ansible/app/tasks/collections.py | 13 +- pulp_ansible/app/tasks/git.py | 3 +- pulp_ansible/app/tasks/roles.py | 3 +- pulp_ansible/app/tasks/signature.py | 10 +- pulp_ansible/app/tasks/utils.py | 52 ++-- pulp_ansible/exceptions.py | 258 ++++++++++++++++++ .../api/collection/v3/test_proxy.py | 14 +- .../functional/api/collection/v3/test_sync.py | 3 +- .../tests/functional/api/role/test_sync.py | 10 +- 11 files changed, 314 insertions(+), 71 deletions(-) create mode 100644 CHANGES/+add-pulp-exceptions.feature diff --git a/CHANGES/+add-pulp-exceptions.feature b/CHANGES/+add-pulp-exceptions.feature new file mode 100644 index 000000000..b0fd64373 --- /dev/null +++ b/CHANGES/+add-pulp-exceptions.feature @@ -0,0 +1 @@ +Add more Pulp Exceptions. diff --git a/pulp_ansible/app/serializers.py b/pulp_ansible/app/serializers.py index fdb2c0c17..2bede4ab3 100644 --- a/pulp_ansible/app/serializers.py +++ b/pulp_ansible/app/serializers.py @@ -11,7 +11,12 @@ from drf_spectacular.utils import extend_schema_field from galaxy_importer.constants import NAME_REGEXP +from pulpcore.plugin.exceptions import DigestValidationError from pulpcore.plugin.models import Artifact, Content, ContentArtifact, SigningService +from pulp_ansible.exceptions import ( + CollectionFilenameParseError, + MissingExpectedFieldsError, +) from pulpcore.plugin.serializers import ( DetailRelatedField, ContentChecksumSerializer, @@ -484,19 +489,12 @@ def validate(self, data): fields = ("namespace", "name", "version") if not all((f"expected_{x}" in data for x in fields)): if not ("file" in data or "filename" in self.context): - raise ValidationError( - _( - "expected_namespace, expected_name, and expected_version must be " - "specified when using artifact or upload objects" - ) - ) + raise MissingExpectedFieldsError() filename = self.context.get("filename") or data["file"].name try: collection = parse_collection_filename(filename) except ValueError: - raise ValidationError( - _("Failed to parse Collection file upload '{}'").format(filename) - ) + raise CollectionFilenameParseError(filename) data["expected_namespace"] = collection.namespace data["expected_name"] = collection.name data["expected_version"] = collection.version @@ -511,7 +509,7 @@ def deferred_validate(self, data): artifact = data.get("artifact") if (sha256 := data.get("sha256")) and sha256 != artifact.sha256: - raise ValidationError(_("Expected sha256 did not match uploaded artifact's sha256")) + raise DigestValidationError(actual=artifact.sha256, expected=sha256) collection_info = process_collection_artifact( artifact=artifact, diff --git a/pulp_ansible/app/tasks/collections.py b/pulp_ansible/app/tasks/collections.py index 4b8979b19..7cf52c948 100644 --- a/pulp_ansible/app/tasks/collections.py +++ b/pulp_ansible/app/tasks/collections.py @@ -59,7 +59,12 @@ from semantic_version.base import Always from pulp_ansible.app.constants import PAGE_SIZE -from pulp_ansible.exceptions import CollectionNotFound +from pulp_ansible.exceptions import ( + AvailableVersionsNotFoundError, + RemoteURLRequiredError, + UnsupportedAPIVersionError, + CollectionNotFound, +) from pulp_ansible.app.models import ( AnsibleCollectionDeprecated, AnsibleNamespace, @@ -228,7 +233,7 @@ def sync(remote_pk, repository_pk, mirror, optimize): is_repo_remote = repository.remote is not None and remote.pk == repository.remote.pk if not remote.url: - raise ValueError(_("A CollectionRemote must have a 'url' specified to synchronize.")) + raise RemoteURLRequiredError() first_stage = CollectionSyncFirstStage(remote, repository, is_repo_remote, optimize) if first_stage.should_sync: @@ -594,14 +599,14 @@ async def _get_root_api(self, root): api_data = parse_metadata(await downloader.run()) if "available_versions" not in api_data: - raise RuntimeError(_("Could not find 'available_versions' at {}").format(root)) + raise AvailableVersionsNotFoundError(root) if "v3" in api_data.get("available_versions", {}): api_version = 3 elif "v2" in api_data.get("available_versions", {}): api_version = 2 else: - raise RuntimeError(_("Unsupported API versions at {}").format(root)) + raise UnsupportedAPIVersionError(root) endpoint = f"{root}v{api_version}" diff --git a/pulp_ansible/app/tasks/git.py b/pulp_ansible/app/tasks/git.py index 59d7724ca..60d8d49c2 100644 --- a/pulp_ansible/app/tasks/git.py +++ b/pulp_ansible/app/tasks/git.py @@ -12,6 +12,7 @@ ArtifactSaver, RemoteArtifactSaver, ) +from pulp_ansible.exceptions import RemoteURLRequiredError from pulp_ansible.app.models import AnsibleRepository, GitRemote from pulp_ansible.app.tasks.collections import ( declarative_content_from_git_repo, @@ -46,7 +47,7 @@ def synchronize(remote_pk, repository_pk, mirror=False): repository = AnsibleRepository.objects.get(pk=repository_pk) if not remote.url: - raise ValueError(_("A remote must have a url specified to synchronize.")) + raise RemoteURLRequiredError() log.info( _("Synchronizing: repository=%(r)s remote=%(p)s"), {"r": repository.name, "p": remote.name} diff --git a/pulp_ansible/app/tasks/roles.py b/pulp_ansible/app/tasks/roles.py index b2968d691..4e3ea141b 100644 --- a/pulp_ansible/app/tasks/roles.py +++ b/pulp_ansible/app/tasks/roles.py @@ -13,6 +13,7 @@ Stage, ) from pulp_ansible.app.constants import PAGE_SIZE +from pulp_ansible.exceptions import RemoteURLRequiredError from pulp_ansible.app.models import AnsibleRepository, RoleRemote, Role from pulp_ansible.app.tasks.utils import get_api_version, get_page_url, parse_metadata @@ -48,7 +49,7 @@ def synchronize(remote_pk, repository_pk, mirror=False): repository = AnsibleRepository.objects.get(pk=repository_pk) if not remote.url: - raise ValueError(_("A remote must have a url specified to synchronize.")) + raise RemoteURLRequiredError() log.info( _("Synchronizing: repository=%(r)s remote=%(p)s"), {"r": repository.name, "p": remote.name} diff --git a/pulp_ansible/app/tasks/signature.py b/pulp_ansible/app/tasks/signature.py index 53f9369b7..4adcd14b5 100644 --- a/pulp_ansible/app/tasks/signature.py +++ b/pulp_ansible/app/tasks/signature.py @@ -26,7 +26,7 @@ from pulpcore.plugin.util import gpg_verify from pulpcore.plugin.exceptions import InvalidSignatureError from pulp_ansible.app.tasks.utils import get_file_obj_from_tarball -from rest_framework import serializers +from pulp_ansible.exceptions import SignatureVerificationError log = logging.getLogger(__name__) @@ -50,13 +50,9 @@ def verify_signature_upload(data): verified = gpg_verify(gpgkey, file, manifest_file.name) except InvalidSignatureError as e: if gpgkey: - raise serializers.ValidationError( - _("Signature verification failed: {}").format(e.verified.status) - ) + raise SignatureVerificationError(str(e.verified.status)) elif settings.ANSIBLE_SIGNATURE_REQUIRE_VERIFICATION: - raise serializers.ValidationError( - _("Signature verification failed: No key available.") - ) + raise SignatureVerificationError("No key available.") else: # We have no key configured. So we simply accept the signature as is verified = e.verified diff --git a/pulp_ansible/app/tasks/utils.py b/pulp_ansible/app/tasks/utils.py index 47134f9a9..547ac8b85 100644 --- a/pulp_ansible/app/tasks/utils.py +++ b/pulp_ansible/app/tasks/utils.py @@ -5,11 +5,20 @@ import re import yaml from urllib.parse import parse_qs, urlencode, urlparse, urlunparse -from rest_framework.serializers import ValidationError from yaml.error import YAMLError from galaxy_importer.schema import MAX_LENGTH_NAME, MAX_LENGTH_VERSION from pulp_ansible.app.constants import PAGE_SIZE +from pulp_ansible.exceptions import ( + APIVersionNotFoundError, + CollectionFieldTooLongError, + CollectionNameRequiredError, + InvalidCollectionFilenameError, + InvalidCollectionNameFormatError, + InvalidCollectionVersionError, + InvalidRequirementsFormatError, + RequirementsFileParseError, +) log = logging.getLogger(__name__) @@ -46,25 +55,20 @@ def parse_collection_filename(filename): match = FILENAME_REGEXP.match(filename) if not match: - msg = _("Invalid filename {filename}. Expected format: namespace-name-version.tar.gz") - raise ValueError(msg.format(filename=filename)) + raise InvalidCollectionFilenameError(filename) namespace, name, version = match.groups() match = VERSION_REGEXP.match(version) if not match: - msg = _( - "Invalid version string {version} from filename {filename}. " - "Expected semantic version format." - ) - raise ValueError(msg.format(version=version, filename=filename)) + raise InvalidCollectionVersionError(version, filename) if len(namespace) > MAX_LENGTH_NAME: - raise ValueError(_("Expected namespace to be max length of %s") % MAX_LENGTH_NAME) + raise CollectionFieldTooLongError("namespace", MAX_LENGTH_NAME) if len(name) > MAX_LENGTH_NAME: - raise ValueError(_("Expected name to be max length of %s") % MAX_LENGTH_NAME) + raise CollectionFieldTooLongError("name", MAX_LENGTH_NAME) if len(version) > MAX_LENGTH_VERSION: - raise ValueError(_("Expected version to be max length of %s") % MAX_LENGTH_VERSION) + raise CollectionFieldTooLongError("version", MAX_LENGTH_VERSION) return CollectionFilename(namespace, name, version) @@ -73,7 +77,7 @@ def get_api_version(url): """Get API version.""" result = re.findall(r"/v(\d)/", url) if len(result) == 0: - raise RuntimeError(f"Could not determine API version for: {url}") + raise APIVersionNotFoundError(url) return int(result[0]) @@ -132,19 +136,12 @@ def parse_collections_requirements_file(requirements_file_string): try: requirements = yaml.safe_load(requirements_file_string) except YAMLError as err: - raise ValidationError( - _( - "Failed to parse the collection requirements yml: {file} " - "with the following error: {error}".format( - file=requirements_file_string, error=err - ) - ) - ) + raise RequirementsFileParseError(requirements_file_string, str(err)) else: requirements = requirements_file_string if not isinstance(requirements, dict) or "collections" not in requirements: - raise ValidationError( + raise InvalidRequirementsFormatError( _( "Expecting collections requirements file to be a dict with the key " "collections that contains a list of collections to install." @@ -152,7 +149,7 @@ def parse_collections_requirements_file(requirements_file_string): ) if not isinstance(requirements["collections"], list): - raise ValidationError( + raise InvalidRequirementsFormatError( _( "Expecting collections requirements file to be a dict with the key " "collections that contains a list of collections to install." @@ -163,9 +160,7 @@ def parse_collections_requirements_file(requirements_file_string): if isinstance(collection_req, dict): req_name = collection_req.get("name", None) if req_name is None: - raise ValidationError( - _("Collections requirement entry should contain the key name.") - ) + raise CollectionNameRequiredError() req_version = collection_req.get("version", "*") req_source = collection_req.get("source", None) @@ -174,12 +169,7 @@ def parse_collections_requirements_file(requirements_file_string): else: entry = RequirementsFileEntry(name=collection_req, version="*", source=None) if "." not in entry.name: - raise ValidationError( - _( - "Collections requirement entry should contain the collection name in the " - "format namespace.name" - ) - ) + raise InvalidCollectionNameFormatError() collection_info.append(entry) return collection_info diff --git a/pulp_ansible/exceptions.py b/pulp_ansible/exceptions.py index b4bb1e1e0..b649b3359 100644 --- a/pulp_ansible/exceptions.py +++ b/pulp_ansible/exceptions.py @@ -24,3 +24,261 @@ def __str__(self): return f"[{self.error_code}] " + _( "Collection {namespace}.{name} does not exist on {url}" ).format(namespace=self.namespace, name=self.name, url=self.url) + + +class RemoteURLRequiredError(PulpException): + """ + Raised when a remote is missing a required URL for synchronization. + """ + + error_code = "PLPAN02" + + def __str__(self): + return f"[{self.error_code}] " + _("A remote must have a url specified to synchronize.") + + +class CollectionFilenameParseError(PulpException): + """ + Raised when unable to parse a collection filename. + """ + + error_code = "PLPAN03" + + def __init__(self, filename): + """ + :param filename: The filename that failed to parse + :type filename: str + """ + self.filename = filename + + def __str__(self): + return f"[{self.error_code}] " + _( + "Failed to parse Collection file upload '{filename}'" + ).format(filename=self.filename) + + +class InvalidCollectionFilenameError(PulpException): + """ + Raised when collection filename format is invalid. + """ + + error_code = "PLPAN04" + + def __init__(self, filename): + """ + :param filename: The invalid filename + :type filename: str + """ + self.filename = filename + + def __str__(self): + return f"[{self.error_code}] " + _( + "Invalid filename {filename}. Expected format: namespace-name-version.tar.gz" + ).format(filename=self.filename) + + +class InvalidCollectionVersionError(PulpException): + """ + Raised when collection version string is invalid. + """ + + error_code = "PLPAN05" + + def __init__(self, version, filename): + """ + :param version: The invalid version string + :type version: str + :param filename: The filename containing the version + :type filename: str + """ + self.version = version + self.filename = filename + + def __str__(self): + return f"[{self.error_code}] " + _( + "Invalid version string {version} from filename {filename}. " + "Expected semantic version format." + ).format(version=self.version, filename=self.filename) + + +class CollectionFieldTooLongError(PulpException): + """ + Raised when a collection field exceeds maximum length. + """ + + error_code = "PLPAN06" + + def __init__(self, field_name, max_length): + """ + :param field_name: Name of the field that's too long + :type field_name: str + :param max_length: Maximum allowed length + :type max_length: int + """ + self.field_name = field_name + self.max_length = max_length + + def __str__(self): + return f"[{self.error_code}] " + _( + "Expected {field_name} to be max length of {max_length}" + ).format(field_name=self.field_name, max_length=self.max_length) + + +class APIVersionNotFoundError(PulpException): + """ + Raised when API version cannot be determined from URL. + """ + + error_code = "PLPAN07" + + def __init__(self, url): + """ + :param url: The URL where version couldn't be found + :type url: str + """ + self.url = url + + def __str__(self): + return f"[{self.error_code}] " + _("Could not determine API version for: {url}").format( + url=self.url + ) + + +class AvailableVersionsNotFoundError(PulpException): + """ + Raised when 'available_versions' field is missing from API response. + """ + + error_code = "PLPAN08" + + def __init__(self, url): + """ + :param url: The URL where available_versions wasn't found + :type url: str + """ + self.url = url + + def __str__(self): + return f"[{self.error_code}] " + _("Could not find 'available_versions' at {url}").format( + url=self.url + ) + + +class UnsupportedAPIVersionError(PulpException): + """ + Raised when API only supports unsupported versions. + """ + + error_code = "PLPAN09" + + def __init__(self, url): + """ + :param url: The URL with unsupported API versions + :type url: str + """ + self.url = url + + def __str__(self): + return f"[{self.error_code}] " + _("Unsupported API versions at {url}").format(url=self.url) + + +class RequirementsFileParseError(PulpException): + """ + Raised when unable to parse requirements file YAML. + """ + + error_code = "PLPAN10" + + def __init__(self, filename, error): + """ + :param filename: The requirements filename + :type filename: str + :param error: The parsing error details + :type error: str + """ + self.filename = filename + self.error = error + + def __str__(self): + return f"[{self.error_code}] " + _( + "Failed to parse the collection requirements yml: {file} with error {error}" + ).format(file=self.filename, error=self.error) + + +class InvalidRequirementsFormatError(PulpException): + """ + Raised when requirements file has invalid format or structure. + """ + + error_code = "PLPAN11" + + def __init__(self, message): + """ + :param message: Description of the format error + :type message: str + """ + self.message = message + + def __str__(self): + return f"[{self.error_code}] " + self.message + + +class CollectionNameRequiredError(PulpException): + """ + Raised when collection name is missing from requirements entry. + """ + + error_code = "PLPAN12" + + def __str__(self): + return f"[{self.error_code}] " + _( + "Collections requirement entry should contain the key name." + ) + + +class InvalidCollectionNameFormatError(PulpException): + """ + Raised when collection name doesn't follow namespace.name format. + """ + + error_code = "PLPAN13" + + def __str__(self): + return f"[{self.error_code}] " + _( + "Collections requirement entry should contain the collection name in the " + "format .." + ) + + +class MissingExpectedFieldsError(PulpException): + """ + Raised when expected_namespace, expected_name, expected_version are missing. + """ + + error_code = "PLPAN14" + + def __str__(self): + return f"[{self.error_code}] " + _( + "expected_namespace, expected_name, and expected_version must be " + "specified when using artifact or upload objects" + ) + + +class SignatureVerificationError(PulpException): + """ + Raised when signature verification fails. + """ + + error_code = "PLPAN15" + + def __init__(self, message): + """ + :param message: Description of the verification failure + :type message: str + """ + self.message = message + + def __str__(self): + return f"[{self.error_code}] " + _("Signature verification failed: {message}").format( + message=self.message + ) diff --git a/pulp_ansible/tests/functional/api/collection/v3/test_proxy.py b/pulp_ansible/tests/functional/api/collection/v3/test_proxy.py index bf5772967..68049e0b3 100644 --- a/pulp_ansible/tests/functional/api/collection/v3/test_proxy.py +++ b/pulp_ansible/tests/functional/api/collection/v3/test_proxy.py @@ -99,10 +99,10 @@ def test_sync_through_http_proxy_with_auth_but_auth_not_configured( ansible_repo, monitor_task, ) - if has_pulp_plugin("core", min="3.102", max="3.103.3"): - assert "[PLP0010]" in exc_info.value.task.error["description"] - else: - assert ( - "407, message='Proxy Authentication Required'" - in exc_info.value.task.error["description"] - ) + + # For compatibility with old pulpcore exception + assert ( + "[PLP0010]" in exc_info.value.task.error["description"] + or "Proxy Authentication Required" in exc_info.value.task.error["description"] + or "Proxy authentication failed" in exc_info.value.task.error["description"] + ) diff --git a/pulp_ansible/tests/functional/api/collection/v3/test_sync.py b/pulp_ansible/tests/functional/api/collection/v3/test_sync.py index 2f70ed1a5..dff5abb80 100644 --- a/pulp_ansible/tests/functional/api/collection/v3/test_sync.py +++ b/pulp_ansible/tests/functional/api/collection/v3/test_sync.py @@ -91,8 +91,7 @@ def test_sync_absent_collection_from_pulp_fails( ) with pytest.raises(Exception) as exc_info: monitor_task(result.task) - if has_pulp_plugin("core", min="3.102"): - assert "[PLPAN01]" in exc_info.value.task.error["description"] + assert "[PLPAN01]" in exc_info.value.task.error["description"] assert "absent.not_present" in exc_info.value.task.error["description"] @pytest.mark.parametrize("mirror", (True, False)) diff --git a/pulp_ansible/tests/functional/api/role/test_sync.py b/pulp_ansible/tests/functional/api/role/test_sync.py index c8d9f45a8..10263fd33 100644 --- a/pulp_ansible/tests/functional/api/role/test_sync.py +++ b/pulp_ansible/tests/functional/api/role/test_sync.py @@ -42,11 +42,5 @@ def test_role_sync_invalid( with pytest.raises(PulpTaskError) as exc_info: ansible_sync_factory(remote=remote.pulp_href) - if has_pulp_plugin("core", min="3.102", max="3.103.3"): - # This should actually have been caught by validation of the remote. - assert "[PLP0000]" in exc_info.value.task.error["description"] - else: - assert ( - exc_info.value.task.error["description"] - == "Could not determine API version for: http://i-am-an-invalid-url.com/invalid/" - ) + assert "[PLPAN07]" in exc_info.value.task.error["description"] + assert "Could not determine API version for" in exc_info.value.task.error["description"]