From 81835a56a5da6ebc57ff9592347566e48bf6714e Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Mon, 5 Feb 2024 13:39:37 +0100 Subject: [PATCH 01/14] Support specifyiing which IP addresses to listen on Allow to define which IP addresses to bind the WebSocket server on. This is especially useful to limit which addresses the add-on will listen on since it runs in host network context. --- matter_server/server/__main__.py | 10 ++++++++++ matter_server/server/server.py | 7 ++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/matter_server/server/__main__.py b/matter_server/server/__main__.py index cfe14d5d..362c955e 100644 --- a/matter_server/server/__main__.py +++ b/matter_server/server/__main__.py @@ -13,6 +13,8 @@ DEFAULT_VENDOR_ID = 0xFFF1 DEFAULT_FABRIC_ID = 1 DEFAULT_PORT = 5580 +# Default to None to bind to all addresses on both IPv4 and IPv6 +DEFAULT_LISTEN_ADDRESS = None DEFAULT_STORAGE_PATH = os.path.join(Path.home(), ".matter_server") # Get parsed passed in arguments. @@ -45,6 +47,13 @@ default=DEFAULT_PORT, help=f"TCP Port to run the websocket server, defaults to {DEFAULT_PORT}", ) +parser.add_argument( + "--listen-address", + type=str, + action="append", + default=DEFAULT_LISTEN_ADDRESS, + help=f"IP address to bind the websocket server to, defaults to listen on any interface (IPv4 and IPv6).", +) parser.add_argument( "--log-level", type=str, @@ -95,6 +104,7 @@ def main() -> None: int(args.vendorid), int(args.fabricid), int(args.port), + args.listen_address, args.primary_interface, ) diff --git a/matter_server/server/server.py b/matter_server/server/server.py index 06c5d802..33a85898 100644 --- a/matter_server/server/server.py +++ b/matter_server/server/server.py @@ -62,13 +62,15 @@ def __init__( vendor_id: int, fabric_id: int, port: int, + listen_addresses: list[str], primary_interface: str | None, ) -> None: """Initialize the Matter Server.""" self.storage_path = storage_path self.vendor_id = vendor_id self.fabric_id = fabric_id - self.port = port + self._port = port + self._listen_addresses = listen_addresses self.primary_interface = primary_interface self.logger = logging.getLogger(__name__) self.app = web.Application() @@ -102,8 +104,7 @@ async def start(self) -> None: self.app.router.add_route("GET", "/", self._handle_info) self._runner = web.AppRunner(self.app, access_log=None) await self._runner.setup() - # set host to None to bind to all addresses on both IPv4 and IPv6 - self._http = web.TCPSite(self._runner, host=None, port=self.port) + self._http = web.TCPSite(self._runner, host=self._listen_addresses, port=self._port) await self._http.start() self.logger.debug("Webserver initialized.") From 360a1ed7bab8a3106f28e0affead45fb6194bd27 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Mon, 5 Feb 2024 13:47:05 +0100 Subject: [PATCH 02/14] Address flake8 issue --- matter_server/server/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matter_server/server/__main__.py b/matter_server/server/__main__.py index 362c955e..2ac62c13 100644 --- a/matter_server/server/__main__.py +++ b/matter_server/server/__main__.py @@ -52,7 +52,7 @@ type=str, action="append", default=DEFAULT_LISTEN_ADDRESS, - help=f"IP address to bind the websocket server to, defaults to listen on any interface (IPv4 and IPv6).", + help="IP address to bind the websocket server to, defaults to listen on any interface (IPv4 and IPv6).", ) parser.add_argument( "--log-level", From f693e46d9a262b43592306c53bb2eb9037b952b0 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Mon, 5 Feb 2024 14:34:01 +0100 Subject: [PATCH 03/14] Apply suggestions from code review Co-authored-by: Martin Hjelmare --- matter_server/server/server.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matter_server/server/server.py b/matter_server/server/server.py index 33a85898..63db30a3 100644 --- a/matter_server/server/server.py +++ b/matter_server/server/server.py @@ -62,8 +62,8 @@ def __init__( vendor_id: int, fabric_id: int, port: int, - listen_addresses: list[str], - primary_interface: str | None, + listen_addresses: list[str] | None = None, + primary_interface: str | None = None, ) -> None: """Initialize the Matter Server.""" self.storage_path = storage_path From 2c7974b2b4b77a38af0884550949cee03c532ecc Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Mon, 5 Feb 2024 14:43:22 +0100 Subject: [PATCH 04/14] Run black again --- matter_server/server/server.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/matter_server/server/server.py b/matter_server/server/server.py index 63db30a3..6c3ce814 100644 --- a/matter_server/server/server.py +++ b/matter_server/server/server.py @@ -104,7 +104,9 @@ async def start(self) -> None: self.app.router.add_route("GET", "/", self._handle_info) self._runner = web.AppRunner(self.app, access_log=None) await self._runner.setup() - self._http = web.TCPSite(self._runner, host=self._listen_addresses, port=self._port) + self._http = web.TCPSite( + self._runner, host=self._listen_addresses, port=self._port + ) await self._http.start() self.logger.debug("Webserver initialized.") From ba26d1bc9a775d5458b3a0ba86478810ae3027bb Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 6 Feb 2024 09:21:02 +0100 Subject: [PATCH 05/14] Make port and listen_address non-private --- matter_server/server/server.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/matter_server/server/server.py b/matter_server/server/server.py index 6c3ce814..21ad8cc5 100644 --- a/matter_server/server/server.py +++ b/matter_server/server/server.py @@ -69,8 +69,8 @@ def __init__( self.storage_path = storage_path self.vendor_id = vendor_id self.fabric_id = fabric_id - self._port = port - self._listen_addresses = listen_addresses + self.port = port + self.listen_addresses = listen_addresses self.primary_interface = primary_interface self.logger = logging.getLogger(__name__) self.app = web.Application() @@ -105,7 +105,7 @@ async def start(self) -> None: self._runner = web.AppRunner(self.app, access_log=None) await self._runner.setup() self._http = web.TCPSite( - self._runner, host=self._listen_addresses, port=self._port + self._runner, host=self.listen_addresses, port=self.port ) await self._http.start() self.logger.debug("Webserver initialized.") From 1e38c7dd82c87aeec475bbc88422f84094a41eaf Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 6 Feb 2024 09:23:48 +0100 Subject: [PATCH 06/14] Shorten help message --- matter_server/server/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matter_server/server/__main__.py b/matter_server/server/__main__.py index 2ac62c13..c740fe70 100644 --- a/matter_server/server/__main__.py +++ b/matter_server/server/__main__.py @@ -52,7 +52,7 @@ type=str, action="append", default=DEFAULT_LISTEN_ADDRESS, - help="IP address to bind the websocket server to, defaults to listen on any interface (IPv4 and IPv6).", + help="IP address to bind the websocket server to, defaults to any IPv4 and IPv6 address.", ) parser.add_argument( "--log-level", From f38950580bfcfde45b27b0bc186c5040a23f8502 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 6 Feb 2024 09:41:06 +0100 Subject: [PATCH 07/14] Add MultiHostTCPSite --- .../server/helpers/custom_web_runner.py | 49 +++++++++++++++++++ matter_server/server/server.py | 4 +- 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 matter_server/server/helpers/custom_web_runner.py diff --git a/matter_server/server/helpers/custom_web_runner.py b/matter_server/server/helpers/custom_web_runner.py new file mode 100644 index 00000000..01c5bb29 --- /dev/null +++ b/matter_server/server/helpers/custom_web_runner.py @@ -0,0 +1,49 @@ +"""MultiHost capable aiohttp Site.""" +from __future__ import annotations + +import asyncio +from ssl import SSLContext + +from aiohttp import web +from yarl import URL + + +class MultiHostTCPSite(web.TCPSite): + """MultiHost capable aiohttp Site. + + Vanilla TCPSite accepts only str as host. However, the underlying asyncio's + create_server() implementation does take a list of strings to bind to multiple + host IP's. To support multiple server_host entries (e.g. to enable dual-stack + explicitly), we would like to pass an array of strings. + """ + + __slots__ = ("_host", "_port", "_reuse_address", "_reuse_port", "_hosturl") + + def __init__( + self, + runner: web.BaseRunner, + host: None | str | list[str], + port: int, + *, + ssl_context: SSLContext | None = None, + backlog: int = 128, + reuse_address: bool | None = None, + reuse_port: bool | None = None, + ) -> None: + """Initialize HomeAssistantTCPSite.""" + super().__init__( + runner, + ssl_context=ssl_context, + backlog=backlog, + ) + self._host = host + self._port = port + self._reuse_address = reuse_address + self._reuse_port = reuse_port + + @property + def name(self) -> str: + """Return server URL.""" + scheme = "https" if self._ssl_context else "http" + host = self._host[0] if isinstance(self._host, list) else "0.0.0.0" + return str(URL.build(scheme=scheme, host=host, port=self._port)) diff --git a/matter_server/server/server.py b/matter_server/server/server.py index 21ad8cc5..14b4c3e7 100644 --- a/matter_server/server/server.py +++ b/matter_server/server/server.py @@ -9,6 +9,8 @@ from aiohttp import web +from matter_server.server.helpers.custom_web_runner import MultiHostTCPSite + from ..common.const import SCHEMA_VERSION from ..common.errors import VersionMismatch from ..common.helpers.api import APICommandHandler, api_command @@ -104,7 +106,7 @@ async def start(self) -> None: self.app.router.add_route("GET", "/", self._handle_info) self._runner = web.AppRunner(self.app, access_log=None) await self._runner.setup() - self._http = web.TCPSite( + self._http = MultiHostTCPSite( self._runner, host=self.listen_addresses, port=self.port ) await self._http.start() From 9c893ac631311dd5e0437456b72bbd2e2d36cf0c Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 6 Feb 2024 09:45:03 +0100 Subject: [PATCH 08/14] Remove unused import --- matter_server/server/helpers/custom_web_runner.py | 1 - 1 file changed, 1 deletion(-) diff --git a/matter_server/server/helpers/custom_web_runner.py b/matter_server/server/helpers/custom_web_runner.py index 01c5bb29..42051fc8 100644 --- a/matter_server/server/helpers/custom_web_runner.py +++ b/matter_server/server/helpers/custom_web_runner.py @@ -1,7 +1,6 @@ """MultiHost capable aiohttp Site.""" from __future__ import annotations -import asyncio from ssl import SSLContext from aiohttp import web From a97f464ada7c2cc2aeb0787ac5ea863f3db8b2ad Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 6 Feb 2024 09:51:56 +0100 Subject: [PATCH 09/14] Drop redefinition of slots --- matter_server/server/helpers/custom_web_runner.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/matter_server/server/helpers/custom_web_runner.py b/matter_server/server/helpers/custom_web_runner.py index 42051fc8..f4dafd6a 100644 --- a/matter_server/server/helpers/custom_web_runner.py +++ b/matter_server/server/helpers/custom_web_runner.py @@ -16,8 +16,6 @@ class MultiHostTCPSite(web.TCPSite): explicitly), we would like to pass an array of strings. """ - __slots__ = ("_host", "_port", "_reuse_address", "_reuse_port", "_hosturl") - def __init__( self, runner: web.BaseRunner, From fdd4cc4dac6b9acefdb50356ee255fd7ef40c7b7 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 6 Feb 2024 09:57:57 +0100 Subject: [PATCH 10/14] Derive from BaseSite --- .../server/helpers/custom_web_runner.py | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/matter_server/server/helpers/custom_web_runner.py b/matter_server/server/helpers/custom_web_runner.py index f4dafd6a..dffef5db 100644 --- a/matter_server/server/helpers/custom_web_runner.py +++ b/matter_server/server/helpers/custom_web_runner.py @@ -1,13 +1,14 @@ """MultiHost capable aiohttp Site.""" from __future__ import annotations +import asyncio from ssl import SSLContext from aiohttp import web from yarl import URL -class MultiHostTCPSite(web.TCPSite): +class MultiHostTCPSite(web.BaseSite): """MultiHost capable aiohttp Site. Vanilla TCPSite accepts only str as host. However, the underlying asyncio's @@ -16,6 +17,8 @@ class MultiHostTCPSite(web.TCPSite): explicitly), we would like to pass an array of strings. """ + __slots__ = ("_host", "_port", "_reuse_address", "_reuse_port", "_hosturl") + def __init__( self, runner: web.BaseRunner, @@ -44,3 +47,19 @@ def name(self) -> str: scheme = "https" if self._ssl_context else "http" host = self._host[0] if isinstance(self._host, list) else "0.0.0.0" return str(URL.build(scheme=scheme, host=host, port=self._port)) + + async def start(self) -> None: + """Start server.""" + await super().start() + loop = asyncio.get_running_loop() + server = self._runner.server + assert server is not None + self._server = await loop.create_server( + server, + self._host, + self._port, + ssl=self._ssl_context, + backlog=self._backlog, + reuse_address=self._reuse_address, + reuse_port=self._reuse_port, + ) From b2caff8cb7c705ea62cbd674b13fdf5f1b626d5c Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 6 Feb 2024 10:04:17 +0100 Subject: [PATCH 11/14] Fix pytests --- tests/server/test_server.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/server/test_server.py b/tests/server/test_server.py index 974898c9..4c414c5d 100644 --- a/tests/server/test_server.py +++ b/tests/server/test_server.py @@ -13,7 +13,7 @@ pytestmark = pytest.mark.usefixtures( "application", "app_runner", - "tcp_site", + "multi_host_tcp_site", "chip_native", "chip_logging", "chip_stack", @@ -38,11 +38,11 @@ def app_runner_fixture() -> Generator[MagicMock, None, None]: yield app_runner -@pytest.fixture(name="tcp_site") -def tcp_site_fixture() -> Generator[MagicMock, None, None]: +@pytest.fixture(name="multi_host_tcp_site") +def multi_host_tcp_site_fixture() -> Generator[MagicMock, None, None]: """Return a mocked tcp site.""" - with patch("matter_server.server.server.web.TCPSite", autospec=True) as tcp_site: - yield tcp_site + with patch("matter_server.server.server.helpers.custom_web_runner.MultiHostTCPSite", autospec=True) as multi_host_tcp_site: + yield multi_host_tcp_site @pytest.fixture(name="chip_native") @@ -108,7 +108,7 @@ async def server_fixture() -> AsyncGenerator[MatterServer, None]: async def test_server_start( application: MagicMock, app_runner: MagicMock, - tcp_site: MagicMock, + multi_host_tcp_site: MagicMock, server: MatterServer, storage_controller: MagicMock, ) -> None: @@ -123,13 +123,14 @@ async def test_server_start( assert add_route.call_args_list[1][0][1] == "/" assert app_runner.call_count == 1 assert app_runner.return_value.setup.call_count == 1 - assert tcp_site.call_count == 1 - assert tcp_site.return_value.start.call_count == 1 + assert multi_host_tcp_site.call_count == 1 + assert multi_host_tcp_site.return_value.start.call_count == 1 assert storage_controller.return_value.start.call_count == 1 assert server.storage_path == "test_storage_path" assert server.vendor_id == 1234 assert server.fabric_id == 5678 assert server.port == 5580 + assert server.listen_addresses == None assert APICommand.SERVER_INFO in server.command_handlers assert APICommand.SERVER_DIAGNOSTICS in server.command_handlers assert APICommand.GET_NODES in server.command_handlers From 7230d020d6d50699b01a8d2c89c65a17777886c4 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 6 Feb 2024 10:45:04 +0100 Subject: [PATCH 12/14] Address mypy issue --- matter_server/server/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matter_server/server/server.py b/matter_server/server/server.py index 14b4c3e7..c19f3530 100644 --- a/matter_server/server/server.py +++ b/matter_server/server/server.py @@ -56,7 +56,7 @@ class MatterServer: """Serve Matter stack over WebSockets.""" _runner: web.AppRunner | None = None - _http: web.TCPSite | None = None + _http: MultiHostTCPSite | None = None def __init__( self, From 1a2503c9de390ac80a3b47f711ce996bdc9e5dc6 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 6 Feb 2024 10:56:16 +0100 Subject: [PATCH 13/14] Fix pytests --- tests/server/test_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/server/test_server.py b/tests/server/test_server.py index 4c414c5d..8df54785 100644 --- a/tests/server/test_server.py +++ b/tests/server/test_server.py @@ -41,7 +41,7 @@ def app_runner_fixture() -> Generator[MagicMock, None, None]: @pytest.fixture(name="multi_host_tcp_site") def multi_host_tcp_site_fixture() -> Generator[MagicMock, None, None]: """Return a mocked tcp site.""" - with patch("matter_server.server.server.helpers.custom_web_runner.MultiHostTCPSite", autospec=True) as multi_host_tcp_site: + with patch("matter_server.server.server.MultiHostTCPSite", autospec=True) as multi_host_tcp_site: yield multi_host_tcp_site From 6add10e68a633dae184dd83da0870c2eb952301b Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 6 Feb 2024 10:57:42 +0100 Subject: [PATCH 14/14] Improve MultiHostTCPSite description --- matter_server/server/helpers/custom_web_runner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matter_server/server/helpers/custom_web_runner.py b/matter_server/server/helpers/custom_web_runner.py index dffef5db..20f954e8 100644 --- a/matter_server/server/helpers/custom_web_runner.py +++ b/matter_server/server/helpers/custom_web_runner.py @@ -1,4 +1,4 @@ -"""MultiHost capable aiohttp Site.""" +"""Multiple host capable aiohttp Site.""" from __future__ import annotations import asyncio @@ -9,7 +9,7 @@ class MultiHostTCPSite(web.BaseSite): - """MultiHost capable aiohttp Site. + """Multiple host capable aiohttp Site. Vanilla TCPSite accepts only str as host. However, the underlying asyncio's create_server() implementation does take a list of strings to bind to multiple