Skip to content

Commit

Permalink
Display a meaningful error message
Browse files Browse the repository at this point in the history
... in case there are any connection related errors.

Fixes: https://github.com/aquarist-labs/s3gw/issues/823

Signed-off-by: Volker Theile <vtheile@suse.com>
  • Loading branch information
votdev committed Nov 28, 2023
1 parent 0e5df4a commit 580a136
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 29 deletions.
22 changes: 18 additions & 4 deletions src/backend/admin_ops/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
from botocore.auth import HmacV1Auth
from botocore.awsrequest import AWSRequest
from botocore.credentials import Credentials
from fastapi import HTTPException, status
from fastapi.logger import logger

from backend.admin_ops.errors import error_from_response

Expand Down Expand Up @@ -63,10 +65,22 @@ def signed_request(

async def send_request(req: httpx.Request) -> httpx.Response:
async with httpx.AsyncClient(verify=False) as client:
res: httpx.Response = await client.send(req)
if not res.is_success:
raise error_from_response(res)
return res
try:
res: httpx.Response = await client.send(req)
if not res.is_success:
raise error_from_response(res)
return res
except httpx.ConnectError as e:
raise HTTPException(
status_code=status.HTTP_502_BAD_GATEWAY,
detail=str(e),
)
except Exception as e:
detail: str | None = str(e) or None
logger.error(f"Unknown exception ({type(e)}): {detail}")
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=detail
)


async def do_request(
Expand Down
7 changes: 4 additions & 3 deletions src/backend/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,11 @@ async def conn(self, attempts: int = 1) -> AsyncGenerator[S3Client, None]:
# propagate it.
raise e
except Exception as e:
logger.error(f"Unknown error: {e}")
logger.error(f" exception: {type(e)}")
detail: str | None = str(e) or None
logger.error(f"Unknown exception ({type(e)}): {detail}")
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail=detail,
)


Expand Down
28 changes: 15 additions & 13 deletions src/backend/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

from typing import Annotated

from fastapi import Depends, HTTPException, status
from fastapi import Depends, HTTPException
from fastapi.routing import APIRouter
from types_aiobotocore_s3.type_defs import ListBucketsOutputTypeDef

Expand All @@ -36,8 +36,8 @@
async def authenticate(conn: S3GWClientDep) -> AuthUser:
# Try to access a RGW Admin Ops endpoint first. If that works, the
# given credentials have `admin` privileges. If it fails, try to
# access a RGW endpoint. If that works, the given credentials can
# be used to sign in as `regular` user.
# access a RGW endpoint via AWS S3 API. If that works, the given
# credentials can be used to sign in as `regular` user.
try:
admin_ops_res: admin_ops_types.UserInfo = (
await admin_ops_users.get_user_info(
Expand All @@ -53,14 +53,16 @@ async def authenticate(conn: S3GWClientDep) -> AuthUser:
IsAdmin=admin_ops_res.admin,
)
except HTTPException:
try:
async with conn.conn() as s3:
s3_res: ListBucketsOutputTypeDef = await s3.list_buckets()
res = AuthUser(
ID=s3_res["Owner"]["ID"],
DisplayName=s3_res["Owner"]["DisplayName"],
IsAdmin=False,
)
except HTTPException:
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN)
# Handle ALL HTTP related exceptions here, e.g. connection errors.
# The `httpx` library used for calling the Admin Ops API does not
# provide meaningful error messages, whereas `botocore` provides
# connection error messages that might be more helpful when they
# are displayed in the UI.
async with conn.conn() as s3:
s3_res: ListBucketsOutputTypeDef = await s3.list_buckets()
res = AuthUser(
ID=s3_res["Owner"]["ID"],
DisplayName=s3_res["Owner"]["DisplayName"],
IsAdmin=False,
)
return res
10 changes: 6 additions & 4 deletions src/backend/tests/unit/admin_ops/test_buckets.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import httpx
import pytest
from fastapi import HTTPException
from fastapi import HTTPException, status
from pytest_httpx import HTTPXMock

import backend.admin_ops.buckets as buckets
Expand Down Expand Up @@ -105,7 +105,7 @@ async def test_bucket_list(httpx_mock: HTTPXMock) -> None:
@pytest.mark.anyio
async def test_bucket_list_failure(httpx_mock: HTTPXMock) -> None:
httpx_mock.add_response( # pyright: ignore [reportUnknownMemberType]
status_code=404 # any error, really
status_code=status.HTTP_404_NOT_FOUND # any error, really
)

with pytest.raises(HTTPException) as e:
Expand All @@ -115,7 +115,7 @@ async def test_bucket_list_failure(httpx_mock: HTTPXMock) -> None:
secret_key="qwe",
uid=None,
)
assert e.value.status_code == 404
assert e.value.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR


@pytest.mark.anyio
Expand All @@ -126,7 +126,9 @@ def check_uid(req: httpx.Request) -> httpx.Response:
nonlocal called_cb
called_cb = True
assert str(req.url.query).find("uid=asdasd") >= 0
return httpx.Response(status_code=200, json=bucket_list_response)
return httpx.Response(
status_code=status.HTTP_200_OK, json=bucket_list_response
)

httpx_mock.add_callback( # pyright: ignore [reportUnknownMemberType]
check_uid
Expand Down
6 changes: 3 additions & 3 deletions src/backend/tests/unit/admin_ops/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

import httpx
import pytest
from fastapi import HTTPException
from fastapi import HTTPException, status
from pytest_httpx import HTTPXMock
from pytest_mock import MockerFixture

Expand Down Expand Up @@ -128,14 +128,14 @@ async def test_get_user_info_3(httpx_mock: HTTPXMock) -> None:
@pytest.mark.anyio
async def test_get_user_info_failure(httpx_mock: HTTPXMock) -> None:
httpx_mock.add_response( # pyright: ignore [reportUnknownMemberType]
status_code=404, # any error, really
status_code=status.HTTP_404_NOT_FOUND, # any error, really
)

with pytest.raises(HTTPException) as e:
await get_user_info(
url="http://foo.bar:123", access_key="asd", secret_key="qwe"
)
assert e.value.status_code == 404
assert e.value.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR


@pytest.mark.anyio
Expand Down
27 changes: 25 additions & 2 deletions src/backend/tests/unit/api/test_api_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
# limitations under the License.

import pytest
from fastapi import HTTPException
from botocore.exceptions import EndpointConnectionError
from fastapi import HTTPException, status
from httpx import ConnectError
from pytest_httpx import HTTPXMock
from pytest_mock import MockerFixture

Expand Down Expand Up @@ -105,4 +107,25 @@ async def test_authenticate_3(

with pytest.raises(HTTPException) as e:
await auth.authenticate(s3_client)
assert e.value.status_code == 403
assert e.value.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR


@pytest.mark.anyio
async def test_authenticate_4(
s3_client: S3GWClient, mocker: MockerFixture, httpx_mock: HTTPXMock
) -> None:
httpx_mock.add_exception(
exception=ConnectError(message="All connection attempts failed")
)

s3api_mock = S3ApiMock(s3_client, mocker)
s3api_mock.patch(
"list_buckets",
side_effect=EndpointConnectionError(
endpoint_url="http://127.0.0.1:7481/"
),
)

with pytest.raises(HTTPException) as e:
await auth.authenticate(s3_client)
assert e.value.status_code == status.HTTP_502_BAD_GATEWAY

0 comments on commit 580a136

Please sign in to comment.