Skip to content
Draft
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/+no-traceback-task.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Stopped leaking sensitive information of failures in the task API.
10 changes: 7 additions & 3 deletions pulpcore/app/models/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ def set_completed(self, result=None):
)
self._cleanup_progress_reports(TASK_STATES.COMPLETED)

def set_failed(self, exc, tb):
def set_failed(self, exc, tb=None):
"""
Set this Task to the failed state and save it.

Expand All @@ -284,8 +284,12 @@ def set_failed(self, exc, tb):
tb (traceback): Traceback instance for the current exception.
"""
finished_at = timezone.now()
tb_str = "".join(traceback.format_tb(tb))
error = exception_to_dict(exc, tb_str)
error = {}
if tb:
tb_str = "".join(traceback.format_tb(tb))
error = exception_to_dict(exc, tb_str)
else:
error = exception_to_dict(exc)
rows = Task.objects.filter(
pk=self.pk,
state=TASK_STATES.RUNNING,
Expand Down
9 changes: 2 additions & 7 deletions pulpcore/app/tasks/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from asgiref.sync import sync_to_async
from django.db import transaction
from rest_framework.serializers import ValidationError
from pulpcore.exceptions import RepositoryVersionDeleteError

from pulpcore.app import models
from pulpcore.app.models import ProgressReport
Expand Down Expand Up @@ -44,12 +44,7 @@ def delete_version(pk):
return

if version.repository.versions.complete().count() <= 1:
raise ValidationError(
_(
"Cannot delete repository version. Repositories must have at least one "
"repository version."
)
)
raise RepositoryVersionDeleteError

log.info(
"Deleting and squashing version {num} of repository '{repo}'".format(
Expand Down
4 changes: 2 additions & 2 deletions pulpcore/download/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import asyncio
import atexit
import copy
from gettext import gettext as _
from multidict import MultiDict
import platform
import ssl
Expand All @@ -15,6 +14,7 @@
from pulpcore.app.apps import PulpAppConfig
from .http import HttpDownloader
from .file import FileDownloader
from pulpcore.exceptions import UrlSchemeNotSupportedError


PROTOCOL_MAP = {
Expand Down Expand Up @@ -177,7 +177,7 @@ def build(self, url, **kwargs):
builder = self._handler_map[scheme]
download_class = self._download_class_map[scheme]
except KeyError:
raise ValueError(_("URL: {u} not supported.".format(u=url)))
raise UrlSchemeNotSupportedError(url)
else:
return builder(download_class, url, **kwargs)

Expand Down
20 changes: 14 additions & 6 deletions pulpcore/download/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
DigestValidationError,
SizeValidationError,
TimeoutException,
DnsDomainNameException,
ProxyAuthenticationRequiredError,
)


Expand Down Expand Up @@ -236,6 +238,7 @@ async def run(self, extra_data=None):
aiohttp.ClientPayloadError,
aiohttp.ClientResponseError,
aiohttp.ServerDisconnectedError,
DnsDomainNameException,
TimeoutError,
TimeoutException,
DigestValidationError,
Expand Down Expand Up @@ -289,12 +292,17 @@ async def _run(self, extra_data=None):
"""
if self.download_throttler:
await self.download_throttler.acquire()
async with self.session.get(
self.url, proxy=self.proxy, proxy_auth=self.proxy_auth, auth=self.auth
) as response:
self.raise_for_status(response)
to_return = await self._handle_response(response)
await response.release()
try:
async with self.session.get(
self.url, proxy=self.proxy, proxy_auth=self.proxy_auth, auth=self.auth
) as response:
self.raise_for_status(response)
to_return = await self._handle_response(response)
await response.release()
except aiohttp.ClientConnectorDNSError:
raise DnsDomainNameException(self.url)
except aiohttp.ClientHttpProxyError:
raise ProxyAuthenticationRequiredError(self.proxy)
if self._close_session_on_finalize:
await self.session.close()
return to_return
Expand Down
4 changes: 4 additions & 0 deletions pulpcore/exceptions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
TimeoutException,
exception_to_dict,
DomainProtectedError,
DnsDomainNameException,
UrlSchemeNotSupportedError,
ProxyAuthenticationRequiredError,
RepositoryVersionDeleteError,
)
from .validation import (
DigestValidationError,
Expand Down
72 changes: 72 additions & 0 deletions pulpcore/exceptions/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,75 @@ def __init__(self):

def __str__(self):
return _("You cannot delete a domain that still contains repositories with content.")


class DnsDomainNameException(PulpException):
"""
Exception to signal that dns could not resolve the domain name for specified url.
"""

def __init__(self, url):
"""
:param url: the url that dns could not resolve
:type url: str
"""
super().__init__("PLP0008")
self.url = url

def __str__(self):
return _("Domain name was not found for {}. Check if specified url is valid.").format(
self.url
)


class UrlSchemeNotSupportedError(PulpException):
"""
Exception raised when a URL scheme (e.g. 'ftp://') is provided that
Pulp does not have a registered handler for.
"""

def __init__(self, url):
"""
:param url: The full URL that failed validation.
:type url: str
"""
super().__init__("PLP0009")
self.url = url

def __str__(self):
return _("URL: {u} not supported.").format(u=self.url)


class ProxyAuthenticationRequiredError(PulpException):
"""
Exception to signal that the proxy server requires authentication
but it was not provided or is invalid (HTTP 407).
"""

def __init__(self, proxy_url):
"""
:param proxy_url: The URL of the proxy server.
:type proxy_url: str
"""
super().__init__("PLP0010")
self.proxy_url = proxy_url

def __str__(self):
return _(
"Proxy authentication failed for {proxy_url}. Please check your proxy credentials."
).format(proxy_url=self.proxy_url)


class RepositoryVersionDeleteError(PulpException):
"""
Raised when attempting to delete a repository version that cannot be deleted
"""

def __init__(self):
super().__init__("PLP0011")

def __str__(self):
return _(
"Cannot delete repository version. Repositories must have at least one "
"repository version."
)
25 changes: 22 additions & 3 deletions pulpcore/tasking/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,13 @@
TASK_WAKEUP_HANDLE,
TASK_WAKEUP_UNBLOCK,
)
from pulpcore.exceptions.base import PulpException
from pulp_glue.common.exceptions import PulpException as PulpGlueException

from pulpcore.middleware import x_task_diagnostics_var
from pulpcore.tasking.kafka import send_task_notification


_logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -70,10 +74,17 @@ def _execute_task(task):
log_task_start(task, domain)
task_function = get_task_function(task)
result = task_function()
except (PulpException, PulpGlueException):
exc_type, exc, _ = sys.exc_info()
log_task_failed(task, exc_type, exc, None, domain) # Leave no traceback in logs
task.set_failed(exc)
send_task_notification(task)
except Exception:
exc_type, exc, tb = sys.exc_info()
task.set_failed(exc, tb)
log_task_failed(task, exc_type, exc, tb, domain)
# Generic exception for user
safe_exc = Exception("An internal error occured.")
task.set_failed(safe_exc)
send_task_notification(task)
else:
task.set_completed(result)
Expand All @@ -95,10 +106,17 @@ async def _aexecute_task(task):
try:
task_coroutine_fn = await aget_task_function(task)
result = await task_coroutine_fn()
except (PulpException, PulpGlueException):
exc_type, exc, _ = sys.exc_info()
log_task_failed(task, exc_type, exc, None, domain) # Leave no traceback in logs
await sync_to_async(task.set_failed)(exc)
send_task_notification(task)
except Exception:
exc_type, exc, tb = sys.exc_info()
await sync_to_async(task.set_failed)(exc, tb)
log_task_failed(task, exc_type, exc, tb, domain)
# Generic exception for user
safe_exc = Exception("An internal error occured.")
await sync_to_async(task.set_failed)(safe_exc)
send_task_notification(task)
else:
await sync_to_async(task.set_completed)(result)
Expand Down Expand Up @@ -144,7 +162,8 @@ def log_task_failed(task, exc_type, exc, tb, domain):
domain=domain.name,
)
)
_logger.info("\n".join(traceback.format_list(traceback.extract_tb(tb))))
if tb:
_logger.info("\n".join(traceback.format_list(traceback.extract_tb(tb))))


async def aget_task_function(task):
Expand Down
10 changes: 7 additions & 3 deletions pulpcore/tests/functional/api/test_tasking.py
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,9 @@ def test_executes_on_api_worker_when_no_async(
"pulpcore.app.tasks.test.sleep", args=(LT_TIMEOUT,), immediate=True
)
monitor_task(task_href)
assert "Immediate tasks must be async functions" in ctx.value.task.error["description"]
# Assert masked internal error
# Underlying cause is ValueError("Immediate tasks must be async functions")
assert "An internal error occured." in ctx.value.task.error["description"]

@pytest.mark.parallel
def test_timeouts_on_api_worker(self, pulpcore_bindings, dispatch_task):
Expand All @@ -484,7 +486,8 @@ def test_timeouts_on_api_worker(self, pulpcore_bindings, dispatch_task):
)
task = pulpcore_bindings.TasksApi.read(task_href)
assert task.state == "failed"
assert "timed out after" in task.error["description"]
# Assert masked internal error; underlying cause is asyncio.TimeoutError
assert "An internal error occured." in task.error["description"]


@pytest.fixture
Expand Down Expand Up @@ -576,4 +579,5 @@ def test_times_out_on_task_worker(
exclusive_resources=[COMMON_RESOURCE],
)
monitor_task(task_href)
assert "timed out after" in ctx.value.task.error["description"]
# Assert masked internal error; underlying cause is asyncio.TimeoutError
assert "An internal error occured." in ctx.value.task.error["description"]
2 changes: 1 addition & 1 deletion pulpcore/tests/functional/api/using_plugin/test_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def test_sync_https_through_http_proxy_with_auth_but_auth_not_configured(
try:
_run_basic_sync_and_assert(file_bindings, monitor_task, remote_on_demand, file_repo)
except PulpTaskError as exc:
assert "407, message='Proxy Authentication Required'" in exc.task.error["description"]
assert "Proxy authentication failed for" in exc.task.error["description"]


@pytest.mark.parallel
Expand Down
3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,10 @@ dependencies = [
"opentelemetry-sdk>=1.27.0,<1.39",
"opentelemetry-exporter-otlp-proto-http>=1.27.0,<1.39",
"protobuf>=4.21.1,<7.0",
"pulp-glue>=0.28.0,<0.37",
"pulp-glue>=0.30.0,<0.37",
"pygtrie>=2.5,<=2.5.0",
"psycopg[binary]>=3.1.8,<3.4", # SemVer, not explicitely stated, but mentioned on multiple changes.
"pycares>=4.0.0,<5.0", # Explicit dependency to ensure compatibility with aiodns.
"pyparsing>=3.1.0,<3.3", # Looks like only bugfixes in z-Stream.
"python-gnupg>=0.5.0,<0.6", # Looks like only bugfixes in z-Stream [changelog only in git]
"PyYAML>=5.1.1,<6.1", # Looks like only bugfixes in z-Stream.
Expand Down
Loading