Skip to content

Commit

Permalink
Return 404 for all APIs when resource not found
Browse files Browse the repository at this point in the history
  • Loading branch information
mdegat01 committed Dec 11, 2024
1 parent 941a7f9 commit 7a4a282
Show file tree
Hide file tree
Showing 12 changed files with 148 additions and 50 deletions.
4 changes: 2 additions & 2 deletions supervisor/api/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
AddonState,
)
from ..coresys import CoreSysAttributes
from ..exceptions import APIError, APIForbidden
from ..exceptions import APIForbidden, APINotFound
from .utils import api_process, api_validate, require_home_assistant

_LOGGER: logging.Logger = logging.getLogger(__name__)
Expand All @@ -36,7 +36,7 @@ def _extract_message(self, request):
"""Extract discovery message from URL."""
message = self.sys_discovery.get(request.match_info.get("uuid"))
if not message:
raise APIError("Discovery message not found")
raise APINotFound("Discovery message not found")
return message

@api_process
Expand Down
4 changes: 4 additions & 0 deletions supervisor/api/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
ATTR_VERSION,
)
from ..coresys import CoreSysAttributes
from ..exceptions import APINotFound
from .utils import api_process, api_validate

_LOGGER: logging.Logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -58,6 +59,9 @@ async def create_registry(self, request: web.Request):
async def remove_registry(self, request: web.Request):
"""Delete a docker registry."""
hostname = request.match_info.get(ATTR_HOSTNAME)
if hostname not in self.sys_docker.config.registries:
raise APINotFound(f"Hostname {hostname} does not exist in registries")

del self.sys_docker.config.registries[hostname]
self.sys_docker.config.save_data()

Expand Down
27 changes: 16 additions & 11 deletions supervisor/api/mounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from ..const import ATTR_NAME, ATTR_STATE
from ..coresys import CoreSysAttributes
from ..exceptions import APIError
from ..exceptions import APIError, APINotFound
from ..mounts.const import ATTR_DEFAULT_BACKUP_MOUNT, MountUsage
from ..mounts.mount import Mount
from ..mounts.validate import SCHEMA_MOUNT_CONFIG
Expand All @@ -24,6 +24,13 @@
class APIMounts(CoreSysAttributes):
"""Handle REST API for mounting options."""

def _extract_mount(self, request: web.Request) -> Mount:
"""Extract mount from request or raise."""
name = request.match_info.get("mount")
if name not in self.sys_mounts:
raise APINotFound(f"No mount exists with name {name}")
return self.sys_mounts.get(name)

@api_process
async def info(self, request: web.Request) -> dict[str, Any]:
"""Return MountManager info."""
Expand Down Expand Up @@ -85,15 +92,13 @@ async def create_mount(self, request: web.Request) -> None:
@api_process
async def update_mount(self, request: web.Request) -> None:
"""Update an existing mount in supervisor."""
name = request.match_info.get("mount")
current = self._extract_mount(request)
name_schema = vol.Schema(
{vol.Optional(ATTR_NAME, default=name): name}, extra=vol.ALLOW_EXTRA
{vol.Optional(ATTR_NAME, default=current.name): current.name},
extra=vol.ALLOW_EXTRA,
)
body = await api_validate(vol.All(name_schema, SCHEMA_MOUNT_CONFIG), request)

if name not in self.sys_mounts:
raise APIError(f"No mount exists with name {name}")

mount = Mount.from_dict(self.coresys, body)
await self.sys_mounts.create_mount(mount)

Expand All @@ -110,8 +115,8 @@ async def update_mount(self, request: web.Request) -> None:
@api_process
async def delete_mount(self, request: web.Request) -> None:
"""Delete an existing mount in supervisor."""
name = request.match_info.get("mount")
mount = await self.sys_mounts.remove_mount(name)
current = self._extract_mount(request)
mount = await self.sys_mounts.remove_mount(current.name)

# If it was a backup mount, reload backups
if mount.usage == MountUsage.BACKUP:
Expand All @@ -122,9 +127,9 @@ async def delete_mount(self, request: web.Request) -> None:
@api_process
async def reload_mount(self, request: web.Request) -> None:
"""Reload an existing mount in supervisor."""
name = request.match_info.get("mount")
await self.sys_mounts.reload_mount(name)
mount = self._extract_mount(request)
await self.sys_mounts.reload_mount(mount.name)

# If it's a backup mount, reload backups
if self.sys_mounts.get(name).usage == MountUsage.BACKUP:
if mount.usage == MountUsage.BACKUP:
self.sys_create_task(self.sys_backups.reload())
32 changes: 14 additions & 18 deletions supervisor/api/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
REQUEST_FROM,
)
from ..coresys import CoreSysAttributes
from ..exceptions import APIError, APIForbidden
from ..exceptions import APIError, APIForbidden, APINotFound
from ..store.addon import AddonStore
from ..store.repository import Repository
from ..store.validate import validate_repository
Expand All @@ -74,31 +74,27 @@ class APIStore(CoreSysAttributes):
def _extract_addon(self, request: web.Request, installed=False) -> AnyAddon:
"""Return add-on, throw an exception it it doesn't exist."""
addon_slug: str = request.match_info.get("addon")
addon_version: str = request.match_info.get("version", "latest")

if installed:
addon = self.sys_addons.local.get(addon_slug)
if addon is None or not addon.is_installed:
raise APIError(f"Addon {addon_slug} is not installed")
else:
addon = self.sys_addons.store.get(addon_slug)

if not addon:
raise APIError(
f"Addon {addon_slug} with version {addon_version} does not exist in the store"
)

if not (addon := self.sys_addons.get(addon_slug)):
raise APINotFound(f"Addon {addon_slug} does not exist in the store")

if installed and not addon.is_installed:
raise APIError(f"Addon {addon_slug} is not installed")

if not installed:
return addon.store_addon
return addon

def _extract_repository(self, request: web.Request) -> Repository:
"""Return repository, throw an exception it it doesn't exist."""
repository_slug: str = request.match_info.get("repository")

repository = self.sys_store.get(repository_slug)
if not repository:
raise APIError(f"Repository {repository_slug} does not exist in the store")
if repository_slug not in self.sys_store.repositories:
raise APINotFound(
f"Repository {repository_slug} does not exist in the store"
)

return repository
return self.sys_store.get(repository_slug)

def _generate_addon_information(
self, addon: AddonStore, extended: bool = False
Expand Down
2 changes: 1 addition & 1 deletion tests/api/test_addons.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ async def test_api_addon_logs_not_installed(api_client: TestClient):
"""Test error is returned for non-existing add-on."""
resp = await api_client.get("/addons/hic_sunt_leones/logs")

assert resp.status == 400
assert resp.status == 404
assert resp.content_type == "text/plain"
content = await resp.text()
assert content == "Addon hic_sunt_leones does not exist"
Expand Down
12 changes: 12 additions & 0 deletions tests/api/test_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,15 @@ async def test_api_invalid_discovery(api_client: TestClient, install_addon_ssh:

resp = await api_client.post("/discovery", json={"service": "test", "config": None})
assert resp.status == 400


@pytest.mark.parametrize(
("method", "url"),
[("get", "/discovery/bad"), ("delete", "/discovery/bad")],
)
async def test_discovery_not_found(api_client: TestClient, method: str, url: str):
"""Test discovery not found error."""
resp = await api_client.request(method, url)
assert resp.status == 404
resp = await resp.json()
assert resp["message"] == "Discovery message not found"
11 changes: 10 additions & 1 deletion tests/api/test_docker.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
"""Test Docker API."""

from aiohttp.test_utils import TestClient
import pytest


@pytest.mark.asyncio
async def test_api_docker_info(api_client):
async def test_api_docker_info(api_client: TestClient):
"""Test docker info api."""
resp = await api_client.get("/docker/info")
result = await resp.json()

assert result["data"]["logging"] == "journald"
assert result["data"]["storage"] == "overlay2"
assert result["data"]["version"] == "1.0.0"


async def test_registry_not_found(api_client: TestClient):
"""Test registry not found error."""
resp = await api_client.delete("/docker/registries/bad")
assert resp.status == 404
body = await resp.json()
assert body["message"] == "Hostname bad does not exist in registries"
8 changes: 4 additions & 4 deletions tests/api/test_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,18 +214,18 @@ async def test_job_manual_cleanup(self) -> None:

# Confirm it no longer exists
resp = await api_client.get(f"/jobs/{test.job_id}")
assert resp.status == 400
assert resp.status == 404
result = await resp.json()
assert result["message"] == f"No job found with id {test.job_id}"

Check failure on line 219 in tests/api/test_jobs.py

View workflow job for this annotation

GitHub Actions / Run tests Python 3.12.7

test_job_manual_cleanup AssertionError: assert 'Job does not exist' == 'No job found...208ee4dd3e141' - No job found with id d977d6174a944cd1991208ee4dd3e141 + Job does not exist


@pytest.mark.parametrize(
("method", "url"),
[("get", "/services/bad"), ("post", "/services/bad"), ("delete", "/services/bad")],
[("get", "/jobs/bad"), ("delete", "/jobs/bad")],
)
async def test_job_not_found(api_client: TestClient, method: str, url: str):
"""Test job not found error."""
resp = await api_client.request(method, url)
assert resp.status == 404
resp = await resp.json()
assert resp["message"] == "Service does not exist"
body = await resp.json()
assert body["message"] == "Job does not exist"
4 changes: 2 additions & 2 deletions tests/api/test_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -417,5 +417,5 @@ async def test_network_interface_not_found(
"""Test network interface not found error."""
resp = await api_client.request(method, url)
assert resp.status == 404
resp = await resp.json()
assert resp["message"] == "Interface bad does not exist"
body = await resp.json()
assert body["message"] == "Interface bad does not exist"
12 changes: 6 additions & 6 deletions tests/api/test_resolution.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ async def test_issue_not_found(api_client: TestClient, method: str, url: str):
"""Test issue not found error."""
resp = await api_client.request(method, url)
assert resp.status == 404
resp = await resp.json()
assert resp["message"] == "The supplied UUID is not a valid issue"
body = await resp.json()
assert body["message"] == "The supplied UUID is not a valid issue"


@pytest.mark.parametrize(
Expand All @@ -194,8 +194,8 @@ async def test_suggestion_not_found(api_client: TestClient, method: str, url: st
"""Test suggestion not found error."""
resp = await api_client.request(method, url)
assert resp.status == 404
resp = await resp.json()
assert resp["message"] == "The supplied UUID is not a valid suggestion"
body = await resp.json()
assert body["message"] == "The supplied UUID is not a valid suggestion"


@pytest.mark.parametrize(
Expand All @@ -206,5 +206,5 @@ async def test_check_not_found(api_client: TestClient, method: str, url: str):
"""Test check not found error."""
resp = await api_client.request(method, url)
assert resp.status == 404
resp = await resp.json()
assert resp["message"] == "The supplied check slug is not available"
body = await resp.json()
assert body["message"] == "The supplied check slug is not available"
10 changes: 5 additions & 5 deletions tests/api/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@

@pytest.mark.parametrize(
("method", "url"),
[("get", "/jobs/bad"), ("delete", "/jobs/bad")],
[("get", "/services/bad"), ("post", "/services/bad"), ("delete", "/services/bad")],
)
async def test_job_not_found(api_client: TestClient, method: str, url: str):
"""Test job not found error."""
async def test_service_not_found(api_client: TestClient, method: str, url: str):
"""Test service not found error."""
resp = await api_client.request(method, url)
assert resp.status == 404
resp = await resp.json()
assert resp["message"] == "Job does not exist"
body = await resp.json()
assert body["message"] == "Service does not exist"
72 changes: 72 additions & 0 deletions tests/api/test_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from pathlib import Path
from unittest.mock import MagicMock, PropertyMock, patch

from aiohttp import ClientResponse
from aiohttp.test_utils import TestClient
import pytest

Expand Down Expand Up @@ -280,3 +281,74 @@ async def test_api_detached_addon_documentation(
assert resp.status == 200
result = await resp.text()
assert result == "Addon local_ssh with version latest does not exist in the store"


async def get_message(resp: ClientResponse, json_expected: bool) -> str:
"""Get message from response based on response type."""
if json_expected:
body = await resp.json()
return body["message"]
return await resp.text()


@pytest.mark.parametrize(
("method", "url", "json_expected"),
[
("get", "/store/addons/bad", True),
("get", "/store/addons/bad/1", True),
("get", "/store/addons/bad/icon", False),
("get", "/store/addons/bad/logo", False),
("post", "/store/addons/bad/install", True),
("post", "/store/addons/bad/install/1", True),
("post", "/store/addons/bad/update", True),
("post", "/store/addons/bad/update/1", True),
# Legacy paths
("get", "/addons/bad/icon", False),
("get", "/addons/bad/logo", False),
("post", "/addons/bad/install", True),
("post", "/addons/bad/update", True),
],
)
async def test_store_addon_not_found(
api_client: TestClient, method: str, url: str, json_expected: bool
):
"""Test store addon not found error."""
resp = await api_client.request(method, url)
assert resp.status == 404
assert (
await get_message(resp, json_expected)
== "Addon bad does not exist in the store"
)


@pytest.mark.parametrize(
("method", "url"),
[
("post", "/store/addons/local_ssh/update"),
("post", "/store/addons/local_ssh/update/1"),
# Legacy paths
("post", "/addons/local_ssh/update"),
],
)
@pytest.mark.usefixtures("repository")
async def test_store_addon_not_installed(api_client: TestClient, method: str, url: str):
"""Test store addon not installed error."""
resp = await api_client.request(method, url)
assert resp.status == 400
body = await resp.json()
assert body["message"] == "Addon local_ssh is not installed"


@pytest.mark.parametrize(
("method", "url"),
[
("get", "/store/repositories/bad"),
("delete", "/store/repositories/bad"),
],
)
async def test_repository_not_found(api_client: TestClient, method: str, url: str):
"""Test repository not found error."""
resp = await api_client.request(method, url)
assert resp.status == 404
body = await resp.json()
assert body["message"] == "Repository bad does not exist in the store"

0 comments on commit 7a4a282

Please sign in to comment.