From a894c4589eed3c3ac03370d9d74284c1a6f4086b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cerm=C3=A1k?= Date: Thu, 4 Apr 2024 12:09:08 +0200 Subject: [PATCH] Use Systemd Journal API for all logs endpoints in API (#4972) * Use Systemd Journal API for all logs endpoints in API Replace all logs endpoints using container logs with wrapped advanced_logs function, adding possibility to get logs from previous boots and following the logs. Supervisor logs are an excetion where Docker logs are still used - in case an exception is raised while accessing the Systemd logs, they're used as fallback - otherwise we wouldn't have an easy way to see what went wrong. * Refactor testing of advanced logs endpoints to a common method * Send error while fetching Supervisor logs to Sentry; minor cleanup * Properly handle errors and use consistent content type in logs endpoints * Replace api_process_custom with reworked api_process_raw per @mdegat01 suggestion --- supervisor/api/__init__.py | 113 ++++++++++++++++++++++++++++---- supervisor/api/addons.py | 38 +++++------ supervisor/api/audio.py | 8 +-- supervisor/api/dns.py | 9 +-- supervisor/api/homeassistant.py | 8 +-- supervisor/api/host.py | 4 +- supervisor/api/multicast.py | 8 +-- supervisor/api/supervisor.py | 4 +- supervisor/api/utils.py | 49 +++++++++----- tests/api/__init__.py | 65 ++++++++++++++++++ tests/api/test_addons.py | 39 ++++++++--- tests/api/test_audio.py | 15 ++--- tests/api/test_dns.py | 12 +--- tests/api/test_homeassistant.py | 17 +++-- tests/api/test_host.py | 14 ++-- tests/api/test_multicast.py | 15 ++--- tests/api/test_supervisor.py | 23 ++++++- 17 files changed, 304 insertions(+), 137 deletions(-) diff --git a/supervisor/api/__init__.py b/supervisor/api/__init__.py index 935e6ed53..88baaeb36 100644 --- a/supervisor/api/__init__.py +++ b/supervisor/api/__init__.py @@ -10,11 +10,13 @@ from aiohttp_fast_url_dispatcher import FastUrlDispatcher, attach_fast_url_dispa from ..const import AddonState from ..coresys import CoreSys, CoreSysAttributes from ..exceptions import APIAddonNotInstalled +from ..utils.sentry import capture_exception from .addons import APIAddons from .audio import APIAudio from .auth import APIAuth from .backups import APIBackups from .cli import APICli +from .const import CONTENT_TYPE_TEXT from .discovery import APIDiscovery from .dns import APICoreDNS from .docker import APIDocker @@ -36,7 +38,7 @@ from .security import APISecurity from .services import APIServices from .store import APIStore from .supervisor import APISupervisor -from .utils import api_process +from .utils import api_process, api_process_raw _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -71,8 +73,14 @@ class RestAPI(CoreSysAttributes): self._runner: web.AppRunner = web.AppRunner(self.webapp, shutdown_timeout=5) self._site: web.TCPSite | None = None + # share single host API handler for reuse in logging endpoints + self._api_host: APIHost | None = None + async def load(self) -> None: """Register REST API Calls.""" + self._api_host = APIHost() + self._api_host.coresys = self.coresys + self._register_addons() self._register_audio() self._register_auth() @@ -102,10 +110,41 @@ class RestAPI(CoreSysAttributes): await self.start() + def _register_advanced_logs(self, path: str, syslog_identifier: str): + """Register logs endpoint for a given path, returning logs for single syslog identifier.""" + + self.webapp.add_routes( + [ + web.get( + f"{path}/logs", + partial(self._api_host.advanced_logs, identifier=syslog_identifier), + ), + web.get( + f"{path}/logs/follow", + partial( + self._api_host.advanced_logs, + identifier=syslog_identifier, + follow=True, + ), + ), + web.get( + f"{path}/logs/boots/{{bootid}}", + partial(self._api_host.advanced_logs, identifier=syslog_identifier), + ), + web.get( + f"{path}/logs/boots/{{bootid}}/follow", + partial( + self._api_host.advanced_logs, + identifier=syslog_identifier, + follow=True, + ), + ), + ] + ) + def _register_host(self) -> None: """Register hostcontrol functions.""" - api_host = APIHost() - api_host.coresys = self.coresys + api_host = self._api_host self.webapp.add_routes( [ @@ -261,11 +300,11 @@ class RestAPI(CoreSysAttributes): [ web.get("/multicast/info", api_multicast.info), web.get("/multicast/stats", api_multicast.stats), - web.get("/multicast/logs", api_multicast.logs), web.post("/multicast/update", api_multicast.update), web.post("/multicast/restart", api_multicast.restart), ] ) + self._register_advanced_logs("/multicast", "hassio_multicast") def _register_hardware(self) -> None: """Register hardware functions.""" @@ -352,7 +391,6 @@ class RestAPI(CoreSysAttributes): web.get("/supervisor/ping", api_supervisor.ping), web.get("/supervisor/info", api_supervisor.info), web.get("/supervisor/stats", api_supervisor.stats), - web.get("/supervisor/logs", api_supervisor.logs), web.post("/supervisor/update", api_supervisor.update), web.post("/supervisor/reload", api_supervisor.reload), web.post("/supervisor/restart", api_supervisor.restart), @@ -361,6 +399,35 @@ class RestAPI(CoreSysAttributes): ] ) + async def get_supervisor_logs(*args, **kwargs): + try: + return await self._api_host.advanced_logs( + *args, identifier="hassio_supervisor", **kwargs + ) + except Exception as err: # pylint: disable=broad-exception-caught + # Supervisor logs are critical, so catch everything, log the exception + # and try to return Docker container logs as the fallback + _LOGGER.exception( + "Failed to get supervisor logs using advanced_logs API" + ) + capture_exception(err) + return await api_supervisor.logs(*args, **kwargs) + + self.webapp.add_routes( + [ + web.get("/supervisor/logs", get_supervisor_logs), + web.get( + "/supervisor/logs/follow", + partial(get_supervisor_logs, follow=True), + ), + web.get("/supervisor/logs/boots/{bootid}", get_supervisor_logs), + web.get( + "/supervisor/logs/boots/{bootid}/follow", + partial(get_supervisor_logs, follow=True), + ), + ] + ) + def _register_homeassistant(self) -> None: """Register Home Assistant functions.""" api_hass = APIHomeAssistant() @@ -369,7 +436,6 @@ class RestAPI(CoreSysAttributes): self.webapp.add_routes( [ web.get("/core/info", api_hass.info), - web.get("/core/logs", api_hass.logs), web.get("/core/stats", api_hass.stats), web.post("/core/options", api_hass.options), web.post("/core/update", api_hass.update), @@ -381,11 +447,12 @@ class RestAPI(CoreSysAttributes): ] ) + self._register_advanced_logs("/core", "homeassistant") + # Reroute from legacy self.webapp.add_routes( [ web.get("/homeassistant/info", api_hass.info), - web.get("/homeassistant/logs", api_hass.logs), web.get("/homeassistant/stats", api_hass.stats), web.post("/homeassistant/options", api_hass.options), web.post("/homeassistant/restart", api_hass.restart), @@ -397,6 +464,8 @@ class RestAPI(CoreSysAttributes): ] ) + self._register_advanced_logs("/homeassistant", "homeassistant") + def _register_proxy(self) -> None: """Register Home Assistant API Proxy.""" api_proxy = APIProxy() @@ -443,13 +512,33 @@ class RestAPI(CoreSysAttributes): ), web.get("/addons/{addon}/options/config", api_addons.options_config), web.post("/addons/{addon}/rebuild", api_addons.rebuild), - web.get("/addons/{addon}/logs", api_addons.logs), web.post("/addons/{addon}/stdin", api_addons.stdin), web.post("/addons/{addon}/security", api_addons.security), web.get("/addons/{addon}/stats", api_addons.stats), ] ) + @api_process_raw(CONTENT_TYPE_TEXT, error_type=CONTENT_TYPE_TEXT) + async def get_addon_logs(request, *args, **kwargs): + addon = api_addons.get_addon_for_request(request) + kwargs["identifier"] = f"addon_{addon.slug}" + return await self._api_host.advanced_logs(request, *args, **kwargs) + + self.webapp.add_routes( + [ + web.get("/addons/{addon}/logs", get_addon_logs), + web.get( + "/addons/{addon}/logs/follow", + partial(get_addon_logs, follow=True), + ), + web.get("/addons/{addon}/logs/boots/{bootid}", get_addon_logs), + web.get( + "/addons/{addon}/logs/boots/{bootid}/follow", + partial(get_addon_logs, follow=True), + ), + ] + ) + # Legacy routing to support requests for not installed addons api_store = APIStore() api_store.coresys = self.coresys @@ -547,7 +636,6 @@ class RestAPI(CoreSysAttributes): [ web.get("/dns/info", api_dns.info), web.get("/dns/stats", api_dns.stats), - web.get("/dns/logs", api_dns.logs), web.post("/dns/update", api_dns.update), web.post("/dns/options", api_dns.options), web.post("/dns/restart", api_dns.restart), @@ -555,18 +643,17 @@ class RestAPI(CoreSysAttributes): ] ) + self._register_advanced_logs("/dns", "hassio_dns") + def _register_audio(self) -> None: """Register Audio functions.""" api_audio = APIAudio() api_audio.coresys = self.coresys - api_host = APIHost() - api_host.coresys = self.coresys self.webapp.add_routes( [ web.get("/audio/info", api_audio.info), web.get("/audio/stats", api_audio.stats), - web.get("/audio/logs", api_audio.logs), web.post("/audio/update", api_audio.update), web.post("/audio/restart", api_audio.restart), web.post("/audio/reload", api_audio.reload), @@ -579,6 +666,8 @@ class RestAPI(CoreSysAttributes): ] ) + self._register_advanced_logs("/audio", "hassio_audio") + def _register_mounts(self) -> None: """Register mounts endpoints.""" api_mounts = APIMounts() diff --git a/supervisor/api/addons.py b/supervisor/api/addons.py index 388f97d25..b828d3007 100644 --- a/supervisor/api/addons.py +++ b/supervisor/api/addons.py @@ -106,8 +106,8 @@ from ..exceptions import ( PwnedSecret, ) from ..validate import docker_ports -from .const import ATTR_REMOVE_CONFIG, ATTR_SIGNED, CONTENT_TYPE_BINARY -from .utils import api_process, api_process_raw, api_validate, json_loads +from .const import ATTR_REMOVE_CONFIG, ATTR_SIGNED +from .utils import api_process, api_validate, json_loads _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -137,8 +137,8 @@ SCHEMA_UNINSTALL = vol.Schema( class APIAddons(CoreSysAttributes): """Handle RESTful API for add-on functions.""" - def _extract_addon(self, request: web.Request) -> Addon: - """Return addon, throw an exception it it doesn't exist.""" + def get_addon_for_request(self, request: web.Request) -> Addon: + """Return addon, throw an exception if it doesn't exist.""" addon_slug: str = request.match_info.get("addon") # Lookup itself @@ -191,7 +191,7 @@ class APIAddons(CoreSysAttributes): async def info(self, request: web.Request) -> dict[str, Any]: """Return add-on information.""" - addon: AnyAddon = self._extract_addon(request) + addon: AnyAddon = self.get_addon_for_request(request) data = { ATTR_NAME: addon.name, @@ -272,7 +272,7 @@ class APIAddons(CoreSysAttributes): @api_process async def options(self, request: web.Request) -> None: """Store user options for add-on.""" - addon = self._extract_addon(request) + addon = self.get_addon_for_request(request) # Update secrets for validation await self.sys_homeassistant.secrets.reload() @@ -307,7 +307,7 @@ class APIAddons(CoreSysAttributes): @api_process async def options_validate(self, request: web.Request) -> None: """Validate user options for add-on.""" - addon = self._extract_addon(request) + addon = self.get_addon_for_request(request) data = {ATTR_MESSAGE: "", ATTR_VALID: True, ATTR_PWNED: False} options = await request.json(loads=json_loads) or addon.options @@ -349,7 +349,7 @@ class APIAddons(CoreSysAttributes): slug: str = request.match_info.get("addon") if slug != "self": raise APIForbidden("This can be only read by the Add-on itself!") - addon = self._extract_addon(request) + addon = self.get_addon_for_request(request) # Lookup/reload secrets await self.sys_homeassistant.secrets.reload() @@ -361,7 +361,7 @@ class APIAddons(CoreSysAttributes): @api_process async def security(self, request: web.Request) -> None: """Store security options for add-on.""" - addon = self._extract_addon(request) + addon = self.get_addon_for_request(request) body: dict[str, Any] = await api_validate(SCHEMA_SECURITY, request) if ATTR_PROTECTED in body: @@ -373,7 +373,7 @@ class APIAddons(CoreSysAttributes): @api_process async def stats(self, request: web.Request) -> dict[str, Any]: """Return resource information.""" - addon = self._extract_addon(request) + addon = self.get_addon_for_request(request) stats: DockerStats = await addon.stats() @@ -391,7 +391,7 @@ class APIAddons(CoreSysAttributes): @api_process async def uninstall(self, request: web.Request) -> Awaitable[None]: """Uninstall add-on.""" - addon = self._extract_addon(request) + addon = self.get_addon_for_request(request) body: dict[str, Any] = await api_validate(SCHEMA_UNINSTALL, request) return await asyncio.shield( self.sys_addons.uninstall( @@ -402,40 +402,34 @@ class APIAddons(CoreSysAttributes): @api_process async def start(self, request: web.Request) -> None: """Start add-on.""" - addon = self._extract_addon(request) + addon = self.get_addon_for_request(request) if start_task := await asyncio.shield(addon.start()): await start_task @api_process def stop(self, request: web.Request) -> Awaitable[None]: """Stop add-on.""" - addon = self._extract_addon(request) + addon = self.get_addon_for_request(request) return asyncio.shield(addon.stop()) @api_process async def restart(self, request: web.Request) -> None: """Restart add-on.""" - addon: Addon = self._extract_addon(request) + addon: Addon = self.get_addon_for_request(request) if start_task := await asyncio.shield(addon.restart()): await start_task @api_process async def rebuild(self, request: web.Request) -> None: """Rebuild local build add-on.""" - addon = self._extract_addon(request) + addon = self.get_addon_for_request(request) if start_task := await asyncio.shield(self.sys_addons.rebuild(addon.slug)): await start_task - @api_process_raw(CONTENT_TYPE_BINARY) - def logs(self, request: web.Request) -> Awaitable[bytes]: - """Return logs from add-on.""" - addon = self._extract_addon(request) - return addon.logs() - @api_process async def stdin(self, request: web.Request) -> None: """Write to stdin of add-on.""" - addon = self._extract_addon(request) + addon = self.get_addon_for_request(request) if not addon.with_stdin: raise APIError(f"STDIN not supported the {addon.slug} add-on") diff --git a/supervisor/api/audio.py b/supervisor/api/audio.py index ebd8a6bf0..f629c8867 100644 --- a/supervisor/api/audio.py +++ b/supervisor/api/audio.py @@ -35,8 +35,7 @@ from ..coresys import CoreSysAttributes from ..exceptions import APIError from ..host.sound import StreamType from ..validate import version_tag -from .const import CONTENT_TYPE_BINARY -from .utils import api_process, api_process_raw, api_validate +from .utils import api_process, api_validate _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -111,11 +110,6 @@ class APIAudio(CoreSysAttributes): raise APIError(f"Version {version} is already in use") await asyncio.shield(self.sys_plugins.audio.update(version)) - @api_process_raw(CONTENT_TYPE_BINARY) - def logs(self, request: web.Request) -> Awaitable[bytes]: - """Return Audio Docker logs.""" - return self.sys_plugins.audio.logs() - @api_process def restart(self, request: web.Request) -> Awaitable[None]: """Restart Audio plugin.""" diff --git a/supervisor/api/dns.py b/supervisor/api/dns.py index cf0ca46ed..b97df30e2 100644 --- a/supervisor/api/dns.py +++ b/supervisor/api/dns.py @@ -26,8 +26,8 @@ from ..const import ( from ..coresys import CoreSysAttributes from ..exceptions import APIError from ..validate import dns_server_list, version_tag -from .const import ATTR_FALLBACK, ATTR_LLMNR, ATTR_MDNS, CONTENT_TYPE_BINARY -from .utils import api_process, api_process_raw, api_validate +from .const import ATTR_FALLBACK, ATTR_LLMNR, ATTR_MDNS +from .utils import api_process, api_validate _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -105,11 +105,6 @@ class APICoreDNS(CoreSysAttributes): raise APIError(f"Version {version} is already in use") await asyncio.shield(self.sys_plugins.dns.update(version)) - @api_process_raw(CONTENT_TYPE_BINARY) - def logs(self, request: web.Request) -> Awaitable[bytes]: - """Return DNS Docker logs.""" - return self.sys_plugins.dns.logs() - @api_process def restart(self, request: web.Request) -> Awaitable[None]: """Restart CoreDNS plugin.""" diff --git a/supervisor/api/homeassistant.py b/supervisor/api/homeassistant.py index 9ed870a0d..20dd353d1 100644 --- a/supervisor/api/homeassistant.py +++ b/supervisor/api/homeassistant.py @@ -36,8 +36,7 @@ from ..const import ( from ..coresys import CoreSysAttributes from ..exceptions import APIError from ..validate import docker_image, network_port, version_tag -from .const import CONTENT_TYPE_BINARY -from .utils import api_process, api_process_raw, api_validate +from .utils import api_process, api_validate _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -173,11 +172,6 @@ class APIHomeAssistant(CoreSysAttributes): """Rebuild Home Assistant.""" return asyncio.shield(self.sys_homeassistant.core.rebuild()) - @api_process_raw(CONTENT_TYPE_BINARY) - def logs(self, request: web.Request) -> Awaitable[bytes]: - """Return Home Assistant Docker logs.""" - return self.sys_homeassistant.core.logs() - @api_process async def check(self, request: web.Request) -> None: """Check configuration of Home Assistant.""" diff --git a/supervisor/api/host.py b/supervisor/api/host.py index 0e8ca9204..ee7b395e5 100644 --- a/supervisor/api/host.py +++ b/supervisor/api/host.py @@ -53,7 +53,7 @@ from .const import ( CONTENT_TYPE_TEXT, CONTENT_TYPE_X_LOG, ) -from .utils import api_process, api_validate +from .utils import api_process, api_process_raw, api_validate _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -163,7 +163,7 @@ class APIHost(CoreSysAttributes): raise APIError() from err return possible_offset - @api_process + @api_process_raw(CONTENT_TYPE_TEXT, error_type=CONTENT_TYPE_TEXT) async def advanced_logs( self, request: web.Request, identifier: str | None = None, follow: bool = False ) -> web.StreamResponse: diff --git a/supervisor/api/multicast.py b/supervisor/api/multicast.py index b715a3948..0742dea1a 100644 --- a/supervisor/api/multicast.py +++ b/supervisor/api/multicast.py @@ -23,8 +23,7 @@ from ..const import ( from ..coresys import CoreSysAttributes from ..exceptions import APIError from ..validate import version_tag -from .const import CONTENT_TYPE_BINARY -from .utils import api_process, api_process_raw, api_validate +from .utils import api_process, api_validate _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -69,11 +68,6 @@ class APIMulticast(CoreSysAttributes): raise APIError(f"Version {version} is already in use") await asyncio.shield(self.sys_plugins.multicast.update(version)) - @api_process_raw(CONTENT_TYPE_BINARY) - def logs(self, request: web.Request) -> Awaitable[bytes]: - """Return Multicast Docker logs.""" - return self.sys_plugins.multicast.logs() - @api_process def restart(self, request: web.Request) -> Awaitable[None]: """Restart Multicast plugin.""" diff --git a/supervisor/api/supervisor.py b/supervisor/api/supervisor.py index dd8de4389..26232506d 100644 --- a/supervisor/api/supervisor.py +++ b/supervisor/api/supervisor.py @@ -49,7 +49,7 @@ from ..store.validate import repositories from ..utils.sentry import close_sentry, init_sentry from ..utils.validate import validate_timezone from ..validate import version_tag, wait_boot -from .const import CONTENT_TYPE_BINARY +from .const import CONTENT_TYPE_TEXT from .utils import api_process, api_process_raw, api_validate _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -229,7 +229,7 @@ class APISupervisor(CoreSysAttributes): """Soft restart Supervisor.""" return asyncio.shield(self.sys_supervisor.restart()) - @api_process_raw(CONTENT_TYPE_BINARY) + @api_process_raw(CONTENT_TYPE_TEXT, error_type=CONTENT_TYPE_TEXT) def logs(self, request: web.Request) -> Awaitable[bytes]: """Return supervisor Docker logs.""" return self.sys_supervisor.logs() diff --git a/supervisor/api/utils.py b/supervisor/api/utils.py index deae62ba1..b4218aa1c 100644 --- a/supervisor/api/utils.py +++ b/supervisor/api/utils.py @@ -25,7 +25,7 @@ from ..exceptions import APIError, APIForbidden, DockerAPIError, HassioError from ..utils import check_exception_chain, get_message_from_exception_chain from ..utils.json import json_dumps, json_loads as json_loads_util from ..utils.log_format import format_message -from .const import CONTENT_TYPE_BINARY +from . import const def excract_supervisor_token(request: web.Request) -> str | None: @@ -91,7 +91,7 @@ def require_home_assistant(method): return wrap_api -def api_process_raw(content): +def api_process_raw(content, *, error_type=None): """Wrap content_type into function.""" def wrap_method(method): @@ -101,15 +101,15 @@ def api_process_raw(content): """Return api information.""" try: msg_data = await method(api, *args, **kwargs) - msg_type = content - except (APIError, APIForbidden) as err: - msg_data = str(err).encode() - msg_type = CONTENT_TYPE_BINARY - except HassioError: - msg_data = b"" - msg_type = CONTENT_TYPE_BINARY + except HassioError as err: + return api_return_error( + err, error_type=error_type or const.CONTENT_TYPE_BINARY + ) - return web.Response(body=msg_data, content_type=msg_type) + if isinstance(msg_data, (web.Response, web.StreamResponse)): + return msg_data + + return web.Response(body=msg_data, content_type=content) return wrap_api @@ -117,23 +117,36 @@ def api_process_raw(content): def api_return_error( - error: Exception | None = None, message: str | None = None + error: Exception | None = None, + message: str | None = None, + error_type: str | None = None, ) -> web.Response: """Return an API error message.""" if error and not message: message = get_message_from_exception_chain(error) if check_exception_chain(error, DockerAPIError): message = format_message(message) + if not message: + message = "Unknown error, see supervisor" - result = { - JSON_RESULT: RESULT_ERROR, - JSON_MESSAGE: message or "Unknown error, see supervisor", - } status = 400 - if isinstance(error, APIError): + if is_api_error := isinstance(error, APIError): status = error.status - if error.job_id: - result[JSON_JOB_ID] = error.job_id + + match error_type: + case const.CONTENT_TYPE_TEXT: + return web.Response(body=message, content_type=error_type, status=status) + case const.CONTENT_TYPE_BINARY: + return web.Response( + body=message.encode(), content_type=error_type, status=status + ) + case _: + result = { + JSON_RESULT: RESULT_ERROR, + JSON_MESSAGE: message, + } + if is_api_error and error.job_id: + result[JSON_JOB_ID] = error.job_id return web.json_response( result, diff --git a/tests/api/__init__.py b/tests/api/__init__.py index 0b5314750..b395105c7 100644 --- a/tests/api/__init__.py +++ b/tests/api/__init__.py @@ -1 +1,66 @@ """Test for API calls.""" +from unittest.mock import MagicMock + +from aiohttp.test_utils import TestClient + +from supervisor.host.const import LogFormat + +DEFAULT_LOG_RANGE = "entries=:-100:" + + +async def common_test_api_advanced_logs( + path_prefix: str, + syslog_identifier: str, + api_client: TestClient, + journald_logs: MagicMock, +): + """Template for tests of endpoints using advanced logs.""" + resp = await api_client.get(f"{path_prefix}/logs") + assert resp.status == 200 + assert resp.content_type == "text/plain" + + journald_logs.assert_called_once_with( + params={"SYSLOG_IDENTIFIER": syslog_identifier}, + range_header=DEFAULT_LOG_RANGE, + accept=LogFormat.JOURNAL, + ) + + journald_logs.reset_mock() + + resp = await api_client.get(f"{path_prefix}/logs/follow") + assert resp.status == 200 + assert resp.content_type == "text/plain" + + journald_logs.assert_called_once_with( + params={"SYSLOG_IDENTIFIER": syslog_identifier, "follow": ""}, + range_header=DEFAULT_LOG_RANGE, + accept=LogFormat.JOURNAL, + ) + + journald_logs.reset_mock() + + resp = await api_client.get(f"{path_prefix}/logs/boots/0") + assert resp.status == 200 + assert resp.content_type == "text/plain" + + journald_logs.assert_called_once_with( + params={"SYSLOG_IDENTIFIER": syslog_identifier, "_BOOT_ID": "ccc"}, + range_header=DEFAULT_LOG_RANGE, + accept=LogFormat.JOURNAL, + ) + + journald_logs.reset_mock() + + resp = await api_client.get(f"{path_prefix}/logs/boots/0/follow") + assert resp.status == 200 + assert resp.content_type == "text/plain" + + journald_logs.assert_called_once_with( + params={ + "SYSLOG_IDENTIFIER": syslog_identifier, + "_BOOT_ID": "ccc", + "follow": "", + }, + range_header=DEFAULT_LOG_RANGE, + accept=LogFormat.JOURNAL, + ) diff --git a/tests/api/test_addons.py b/tests/api/test_addons.py index c73a839a6..c07033cfb 100644 --- a/tests/api/test_addons.py +++ b/tests/api/test_addons.py @@ -13,9 +13,11 @@ from supervisor.coresys import CoreSys from supervisor.docker.addon import DockerAddon from supervisor.docker.const import ContainerState from supervisor.docker.monitor import DockerContainerStateEvent +from supervisor.exceptions import HassioError from supervisor.store.repository import Repository from ..const import TEST_ADDON_SLUG +from . import common_test_api_advanced_logs def _create_test_event(name: str, state: ContainerState) -> DockerContainerStateEvent: @@ -67,17 +69,38 @@ async def test_addons_info_not_installed( async def test_api_addon_logs( - api_client: TestClient, docker_logs: MagicMock, install_addon_ssh: Addon + api_client: TestClient, journald_logs: MagicMock, install_addon_ssh: Addon ): """Test addon logs.""" + await common_test_api_advanced_logs( + "/addons/local_ssh", "addon_local_ssh", api_client, journald_logs + ) + + +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.content_type == "text/plain" + content = await resp.text() + assert content == "Addon hic_sunt_leones does not exist" + + +async def test_api_addon_logs_error( + api_client: TestClient, + journald_logs: MagicMock, + docker_logs: MagicMock, + install_addon_ssh: Addon, +): + """Test errors are properly handled for add-on logs.""" + journald_logs.side_effect = HassioError("Something bad happened!") resp = await api_client.get("/addons/local_ssh/logs") - assert resp.status == 200 - assert resp.content_type == "application/octet-stream" - content = await resp.read() - assert content.split(b"\n")[0:2] == [ - b"\x1b[36m22-10-11 14:04:23 DEBUG (MainThread) [supervisor.utils.dbus] D-Bus call - org.freedesktop.DBus.Properties.call_get_all on /io/hass/os\x1b[0m", - b"\x1b[36m22-10-11 14:04:23 DEBUG (MainThread) [supervisor.utils.dbus] D-Bus call - org.freedesktop.DBus.Properties.call_get_all on /io/hass/os/AppArmor\x1b[0m", - ] + + assert resp.status == 400 + assert resp.content_type == "text/plain" + content = await resp.text() + assert content == "Something bad happened!" async def test_api_addon_start_healthcheck( diff --git a/tests/api/test_audio.py b/tests/api/test_audio.py index 9f7d31264..0633b3b16 100644 --- a/tests/api/test_audio.py +++ b/tests/api/test_audio.py @@ -4,14 +4,11 @@ from unittest.mock import MagicMock from aiohttp.test_utils import TestClient +from tests.api import common_test_api_advanced_logs -async def test_api_audio_logs(api_client: TestClient, docker_logs: MagicMock): + +async def test_api_audio_logs(api_client: TestClient, journald_logs: MagicMock): """Test audio logs.""" - resp = await api_client.get("/audio/logs") - assert resp.status == 200 - assert resp.content_type == "application/octet-stream" - content = await resp.read() - assert content.split(b"\n")[0:2] == [ - b"\x1b[36m22-10-11 14:04:23 DEBUG (MainThread) [supervisor.utils.dbus] D-Bus call - org.freedesktop.DBus.Properties.call_get_all on /io/hass/os\x1b[0m", - b"\x1b[36m22-10-11 14:04:23 DEBUG (MainThread) [supervisor.utils.dbus] D-Bus call - org.freedesktop.DBus.Properties.call_get_all on /io/hass/os/AppArmor\x1b[0m", - ] + await common_test_api_advanced_logs( + "/audio", "hassio_audio", api_client, journald_logs + ) diff --git a/tests/api/test_dns.py b/tests/api/test_dns.py index 4c6c3b034..72a4eb3e4 100644 --- a/tests/api/test_dns.py +++ b/tests/api/test_dns.py @@ -6,6 +6,7 @@ from aiohttp.test_utils import TestClient from supervisor.coresys import CoreSys from supervisor.dbus.resolved import Resolved +from tests.api import common_test_api_advanced_logs from tests.dbus_service_mocks.base import DBusServiceMock from tests.dbus_service_mocks.resolved import Resolved as ResolvedService @@ -64,13 +65,6 @@ async def test_options(api_client: TestClient, coresys: CoreSys): restart.assert_called_once() -async def test_api_dns_logs(api_client: TestClient, docker_logs: MagicMock): +async def test_api_dns_logs(api_client: TestClient, journald_logs: MagicMock): """Test dns logs.""" - resp = await api_client.get("/dns/logs") - assert resp.status == 200 - assert resp.content_type == "application/octet-stream" - content = await resp.read() - assert content.split(b"\n")[0:2] == [ - b"\x1b[36m22-10-11 14:04:23 DEBUG (MainThread) [supervisor.utils.dbus] D-Bus call - org.freedesktop.DBus.Properties.call_get_all on /io/hass/os\x1b[0m", - b"\x1b[36m22-10-11 14:04:23 DEBUG (MainThread) [supervisor.utils.dbus] D-Bus call - org.freedesktop.DBus.Properties.call_get_all on /io/hass/os/AppArmor\x1b[0m", - ] + await common_test_api_advanced_logs("/dns", "hassio_dns", api_client, journald_logs) diff --git a/tests/api/test_homeassistant.py b/tests/api/test_homeassistant.py index 15ed7cbbe..4f8854b6a 100644 --- a/tests/api/test_homeassistant.py +++ b/tests/api/test_homeassistant.py @@ -8,22 +8,21 @@ import pytest from supervisor.coresys import CoreSys from supervisor.homeassistant.module import HomeAssistant +from tests.api import common_test_api_advanced_logs from tests.common import load_json_fixture @pytest.mark.parametrize("legacy_route", [True, False]) async def test_api_core_logs( - api_client: TestClient, docker_logs: MagicMock, legacy_route: bool + api_client: TestClient, journald_logs: MagicMock, legacy_route: bool ): """Test core logs.""" - resp = await api_client.get(f"/{'homeassistant' if legacy_route else 'core'}/logs") - assert resp.status == 200 - assert resp.content_type == "application/octet-stream" - content = await resp.read() - assert content.split(b"\n")[0:2] == [ - b"\x1b[36m22-10-11 14:04:23 DEBUG (MainThread) [supervisor.utils.dbus] D-Bus call - org.freedesktop.DBus.Properties.call_get_all on /io/hass/os\x1b[0m", - b"\x1b[36m22-10-11 14:04:23 DEBUG (MainThread) [supervisor.utils.dbus] D-Bus call - org.freedesktop.DBus.Properties.call_get_all on /io/hass/os/AppArmor\x1b[0m", - ] + await common_test_api_advanced_logs( + f"/{'homeassistant' if legacy_route else 'core'}", + "homeassistant", + api_client, + journald_logs, + ) async def test_api_stats(api_client: TestClient, coresys: CoreSys): diff --git a/tests/api/test_host.py b/tests/api/test_host.py index eca644cd6..902701a64 100644 --- a/tests/api/test_host.py +++ b/tests/api/test_host.py @@ -310,15 +310,17 @@ async def test_advanced_logs_errors(api_client: TestClient): """Test advanced logging API errors.""" # coresys = coresys_logs_control resp = await api_client.get("/host/logs") - result = await resp.json() - assert result["result"] == "error" - assert result["message"] == "No systemd-journal-gatewayd Unix socket available" + assert resp.content_type == "text/plain" + assert resp.status == 400 + content = await resp.text() + assert content == "No systemd-journal-gatewayd Unix socket available" headers = {"Accept": "application/json"} resp = await api_client.get("/host/logs", headers=headers) - result = await resp.json() - assert result["result"] == "error" + assert resp.content_type == "text/plain" + assert resp.status == 400 + content = await resp.text() assert ( - result["message"] + content == "Invalid content type requested. Only text/plain and text/x-log supported for now." ) diff --git a/tests/api/test_multicast.py b/tests/api/test_multicast.py index 5ef1d8cf1..125acb865 100644 --- a/tests/api/test_multicast.py +++ b/tests/api/test_multicast.py @@ -4,14 +4,11 @@ from unittest.mock import MagicMock from aiohttp.test_utils import TestClient +from tests.api import common_test_api_advanced_logs -async def test_api_multicast_logs(api_client: TestClient, docker_logs: MagicMock): + +async def test_api_multicast_logs(api_client: TestClient, journald_logs: MagicMock): """Test multicast logs.""" - resp = await api_client.get("/multicast/logs") - assert resp.status == 200 - assert resp.content_type == "application/octet-stream" - content = await resp.read() - assert content.split(b"\n")[0:2] == [ - b"\x1b[36m22-10-11 14:04:23 DEBUG (MainThread) [supervisor.utils.dbus] D-Bus call - org.freedesktop.DBus.Properties.call_get_all on /io/hass/os\x1b[0m", - b"\x1b[36m22-10-11 14:04:23 DEBUG (MainThread) [supervisor.utils.dbus] D-Bus call - org.freedesktop.DBus.Properties.call_get_all on /io/hass/os/AppArmor\x1b[0m", - ] + await common_test_api_advanced_logs( + "/multicast", "hassio_multicast", api_client, journald_logs + ) diff --git a/tests/api/test_supervisor.py b/tests/api/test_supervisor.py index b4fb8f6d2..3573d4cff 100644 --- a/tests/api/test_supervisor.py +++ b/tests/api/test_supervisor.py @@ -9,6 +9,7 @@ from supervisor.coresys import CoreSys from supervisor.exceptions import StoreGitError, StoreNotFound from supervisor.store.repository import Repository +from tests.api import common_test_api_advanced_logs from tests.dbus_service_mocks.base import DBusServiceMock from tests.dbus_service_mocks.os_agent import OSAgent as OSAgentService @@ -148,11 +149,27 @@ async def test_api_supervisor_options_diagnostics( assert coresys.dbus.agent.diagnostics is False -async def test_api_supervisor_logs(api_client: TestClient, docker_logs: MagicMock): +async def test_api_supervisor_logs(api_client: TestClient, journald_logs: MagicMock): """Test supervisor logs.""" - resp = await api_client.get("/supervisor/logs") + await common_test_api_advanced_logs( + "/supervisor", "hassio_supervisor", api_client, journald_logs + ) + + +async def test_api_supervisor_fallback( + api_client: TestClient, journald_logs: MagicMock, docker_logs: MagicMock +): + """Check that supervisor logs read from container logs if reading from journald gateway fails badly.""" + journald_logs.side_effect = OSError("Something bad happened!") + + with patch("supervisor.api._LOGGER.exception") as logger: + resp = await api_client.get("/supervisor/logs") + logger.assert_called_once_with( + "Failed to get supervisor logs using advanced_logs API" + ) + assert resp.status == 200 - assert resp.content_type == "application/octet-stream" + assert resp.content_type == "text/plain" content = await resp.read() assert content.split(b"\n")[0:2] == [ b"\x1b[36m22-10-11 14:04:23 DEBUG (MainThread) [supervisor.utils.dbus] D-Bus call - org.freedesktop.DBus.Properties.call_get_all on /io/hass/os\x1b[0m",