From 31949aac2cb0911c8d591e5877a6be38014f6c47 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Tue, 13 Dec 2022 23:46:31 +0200 Subject: [PATCH 01/12] Move to HTTP Inspector --- sanic/app.py | 3 + sanic/cli/app.py | 114 +++++++++++++--------- sanic/cli/arguments.py | 6 -- sanic/cli/base.py | 29 ++++++ sanic/cli/inspector.py | 47 +++++++++ sanic/config.py | 4 + sanic/http/tls/context.py | 8 +- sanic/mixins/startup.py | 7 +- sanic/worker/inspector.py | 196 +++++++++++++++++++++++++------------- sanic/worker/serve.py | 6 +- 10 files changed, 292 insertions(+), 128 deletions(-) create mode 100644 sanic/cli/base.py create mode 100644 sanic/cli/inspector.py diff --git a/sanic/app.py b/sanic/app.py index 41f065800f..c951009b7d 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -140,6 +140,7 @@ class Sanic(BaseSanic, StartupMixin, metaclass=TouchUpMeta): "configure_logging", "ctx", "error_handler", + "inspector_class", "go_fast", "listeners", "multiplexer", @@ -176,6 +177,7 @@ def __init__( dumps: Optional[Callable[..., AnyStr]] = None, loads: Optional[Callable[..., Any]] = None, inspector: bool = False, + inspector_class: Optional[Type[Inspector]] = None, ) -> None: super().__init__(name=name) # logging @@ -211,6 +213,7 @@ def __init__( self.configure_logging: bool = configure_logging self.ctx: Any = ctx or SimpleNamespace() self.error_handler: ErrorHandler = error_handler or ErrorHandler() + self.inspector_class: Type[Inspector] = inspector_class or Inspector self.listeners: Dict[str, List[ListenerType[Any]]] = defaultdict(list) self.named_request_middleware: Dict[str, Deque[MiddlewareType]] = {} self.named_response_middleware: Dict[str, Deque[MiddlewareType]] = {} diff --git a/sanic/cli/app.py b/sanic/cli/app.py index 24825e25de..3e8c2f89d4 100644 --- a/sanic/cli/app.py +++ b/sanic/cli/app.py @@ -3,7 +3,6 @@ import shutil import sys -from argparse import ArgumentParser, RawTextHelpFormatter from functools import partial from textwrap import indent from typing import Any, List, Union @@ -11,15 +10,13 @@ from sanic.app import Sanic from sanic.application.logo import get_logo from sanic.cli.arguments import Group -from sanic.log import error_logger -from sanic.worker.inspector import inspect +from sanic.cli.base import SanicArgumentParser, SanicHelpFormatter +from sanic.cli.inspector import make_inspector_parser +from sanic.log import Colors, error_logger +from sanic.worker.inspector import InspectorClient from sanic.worker.loader import AppLoader -class SanicArgumentParser(ArgumentParser): - ... - - class SanicCLI: DESCRIPTION = indent( f""" @@ -46,7 +43,7 @@ def __init__(self) -> None: self.parser = SanicArgumentParser( prog="sanic", description=self.DESCRIPTION, - formatter_class=lambda prog: RawTextHelpFormatter( + formatter_class=lambda prog: SanicHelpFormatter( prog, max_help_position=36 if width > 96 else 24, indent_increment=4, @@ -60,14 +57,25 @@ def __init__(self) -> None: ) self.args: List[Any] = [] self.groups: List[Group] = [] + self.inspecting = False def attach(self): + if sys.argv[1] == "inspect": + self.inspecting = True + self.parser.description = get_logo(True) + make_inspector_parser(self.parser) + return + for group in Group._registry: instance = group.create(self.parser) instance.attach() self.groups.append(instance) def run(self, parse_args=None): + if self.inspecting: + self._inspector() + return + legacy_version = False if not parse_args: # This is to provide backwards compat -v to display version @@ -76,7 +84,7 @@ def run(self, parse_args=None): elif parse_args == ["-v"]: parse_args = ["--version"] - if not legacy_version: + if not legacy_version and not self.inspecting: parsed, unknown = self.parser.parse_known_args(args=parse_args) if unknown and parsed.factory: for arg in unknown: @@ -86,52 +94,21 @@ def run(self, parse_args=None): self.args = self.parser.parse_args(args=parse_args) self._precheck() app_loader = AppLoader( - self.args.module, - self.args.factory, - self.args.simple, - self.args, + self.args.module, self.args.factory, self.args.simple, self.args ) + if self.args.inspect or self.args.inspect_raw or self.args.trigger: + self._inspector_legacy(app_loader) + return + try: app = self._get_app(app_loader) kwargs = self._build_run_kwargs() except ValueError as e: error_logger.exception(f"Failed to run app: {e}") else: - if ( - self.args.inspect - or self.args.inspect_raw - or self.args.trigger - or self.args.scale is not None - ): - os.environ["SANIC_IGNORE_PRODUCTION_WARNING"] = "true" - else: - for http_version in self.args.http: - app.prepare(**kwargs, version=http_version) - - if ( - self.args.inspect - or self.args.inspect_raw - or self.args.trigger - or self.args.scale is not None - ): - if self.args.scale is not None: - if self.args.scale <= 0: - error_logger.error("There must be at least 1 worker") - sys.exit(1) - action = f"scale={self.args.scale}" - else: - action = self.args.trigger or ( - "raw" if self.args.inspect_raw else "pretty" - ) - inspect( - app.config.INSPECTOR_HOST, - app.config.INSPECTOR_PORT, - action, - ) - del os.environ["SANIC_IGNORE_PRODUCTION_WARNING"] - return - + for http_version in self.args.http: + app.prepare(**kwargs, version=http_version) if self.args.single: serve = Sanic.serve_single elif self.args.legacy: @@ -140,6 +117,49 @@ def run(self, parse_args=None): serve = partial(Sanic.serve, app_loader=app_loader) serve(app) + def _inspector_legacy(self, app_loader: AppLoader): + host = port = None + if ":" in self.args.module: + maybe_host, maybe_port = self.args.module.rsplit(":", 1) + if maybe_port.isnumeric(): + host, port = maybe_host, int(maybe_port) + if not host: + app = self._get_app(app_loader) + host, port = app.config.INSPECTOR_HOST, app.config.INSPECTOR_PORT + + action = self.args.trigger or "info" + + InspectorClient(host, port, False, self.args.inspect_raw).do(action) + sys.stdout.write( + f"\n{Colors.BOLD}{Colors.YELLOW}WARNING:{Colors.END} " + "You are using the legacy CLI command that will be removed in " + f"{Colors.RED}v23.3{Colors.END}. See ___ or checkout the new " + "style commands:\n\n\t" + f"{Colors.YELLOW}sanic inspect --help{Colors.END}\n" + ) + + def _inspector(self): + args = sys.argv[2:] + self.args, unknown = self.parser.parse_known_args(args=args) + if unknown: + for arg in unknown: + if arg.startswith("--"): + key, value = arg.split("=") + setattr(self.args, key.strip("-"), value) + + kwargs = {**self.args.__dict__} + host = kwargs.pop("host") + port = kwargs.pop("port") + secure = kwargs.pop("secure") + raw = kwargs.pop("raw") + action = kwargs.pop("action") or "info" + positional = kwargs.pop("positional", None) + if action == "" and positional: + action = positional[0] + if len(positional) > 1: + kwargs["args"] = positional[1:] + InspectorClient(host, port, secure, raw).do(action, **kwargs) + def _precheck(self): # Custom TLS mismatch handling for better diagnostics if self.main_process and ( diff --git a/sanic/cli/arguments.py b/sanic/cli/arguments.py index f01dda9b56..e1fe905adf 100644 --- a/sanic/cli/arguments.py +++ b/sanic/cli/arguments.py @@ -115,12 +115,6 @@ def attach(self): const="shutdown", help=("Trigger all processes to shutdown"), ) - group.add_argument( - "--scale", - dest="scale", - type=int, - help=("Scale number of workers"), - ) class HTTPVersionGroup(Group): diff --git a/sanic/cli/base.py b/sanic/cli/base.py new file mode 100644 index 0000000000..2822263e52 --- /dev/null +++ b/sanic/cli/base.py @@ -0,0 +1,29 @@ +from argparse import ( + Action, + ArgumentParser, + RawTextHelpFormatter, + _SubParsersAction, +) +from typing import Any + + +class SanicArgumentParser(ArgumentParser): + def _check_value(self, action: Action, value: Any) -> None: + if isinstance(action, SanicSubParsersAction): + return + super()._check_value(action, value) + + +class SanicHelpFormatter(RawTextHelpFormatter): + ... + + +class SanicSubParsersAction(_SubParsersAction): + def __call__(self, parser, namespace, values, option_string=None): + self._name_parser_map + parser_name = values[0] + if parser_name not in self._name_parser_map: + self._name_parser_map[parser_name] = parser + values = ["", *values] + + super().__call__(parser, namespace, values, option_string) diff --git a/sanic/cli/inspector.py b/sanic/cli/inspector.py new file mode 100644 index 0000000000..d300f1e52b --- /dev/null +++ b/sanic/cli/inspector.py @@ -0,0 +1,47 @@ +from argparse import ArgumentParser + +from sanic.cli.base import SanicHelpFormatter, SanicSubParsersAction + + +def make_inspector_parser(parser: ArgumentParser) -> None: + parser.add_argument("--host", "-H", default="localhost") + parser.add_argument("--port", "-p", default=6457, type=int) + parser.add_argument("--secure", "-s", action="store_true") + parser.add_argument( + "--raw", + action="store_true", + help="Whether to output the raw response information", + ) + + subparsers = parser.add_subparsers( + action=SanicSubParsersAction, + dest="action", + description=( + "Run one of the below subcommands. If you have created a custom " + "Inspector instance, then you can run custom commands.\nSee ___ " + "for more details." + ), + title="Subcommands", + ) + subparsers.add_parser( + "reload", help="Trigger a reload of the server workers" + ) + subparsers.add_parser( + "shutdown", help="Shutdown the application and all processes" + ) + scale = subparsers.add_parser("scale", help="Scale the number of workers") + scale.add_argument("replicas", type=int) + + custom = subparsers.add_parser( + "", + help="Run a custom command", + description=( + "keyword arguments:\n When running a custom command, you can " + "add keyword arguments by appending them to your command\n\n" + "\tsanic inspect foo --one=1 --two=2" + ), + formatter_class=SanicHelpFormatter, + ) + custom.add_argument( + "positional", nargs="*", help="Add one or more non-keyword args" + ) diff --git a/sanic/config.py b/sanic/config.py index dc14d71017..09cb3c38df 100644 --- a/sanic/config.py +++ b/sanic/config.py @@ -46,6 +46,8 @@ "INSPECTOR": False, "INSPECTOR_HOST": "localhost", "INSPECTOR_PORT": 6457, + "INSPECTOR_TLS_KEY": _default, + "INSPECTOR_TLS_CERT": _default, "KEEP_ALIVE_TIMEOUT": 5, # 5 seconds "KEEP_ALIVE": True, "LOCAL_CERT_CREATOR": LocalCertCreator.AUTO, @@ -93,6 +95,8 @@ class Config(dict, metaclass=DescriptorMeta): INSPECTOR: bool INSPECTOR_HOST: str INSPECTOR_PORT: int + INSPECTOR_TLS_KEY: Union[Path, str, Default] + INSPECTOR_TLS_CERT: Union[Path, str, Default] KEEP_ALIVE_TIMEOUT: int KEEP_ALIVE: bool LOCAL_CERT_CREATOR: Union[str, LocalCertCreator] diff --git a/sanic/http/tls/context.py b/sanic/http/tls/context.py index f77fa56051..2a123e52b5 100644 --- a/sanic/http/tls/context.py +++ b/sanic/http/tls/context.py @@ -24,14 +24,16 @@ def create_context( certfile: Optional[str] = None, keyfile: Optional[str] = None, password: Optional[str] = None, + purpose: ssl.Purpose = ssl.Purpose.CLIENT_AUTH, ) -> ssl.SSLContext: """Create a context with secure crypto and HTTP/1.1 in protocols.""" - context = ssl.create_default_context(purpose=ssl.Purpose.CLIENT_AUTH) + context = ssl.create_default_context(purpose=purpose) context.minimum_version = ssl.TLSVersion.TLSv1_2 context.set_ciphers(":".join(CIPHERS_TLS12)) context.set_alpn_protocols(["http/1.1"]) - context.sni_callback = server_name_callback - if certfile and keyfile: + if purpose is ssl.Purpose.CLIENT_AUTH: + context.sni_callback = server_name_callback + if certfile or keyfile: context.load_cert_chain(certfile, keyfile, password) return context diff --git a/sanic/mixins/startup.py b/sanic/mixins/startup.py index 78abb88439..2adef3b306 100644 --- a/sanic/mixins/startup.py +++ b/sanic/mixins/startup.py @@ -58,7 +58,6 @@ from sanic.server.protocols.websocket_protocol import WebSocketProtocol from sanic.server.runners import serve, serve_multiple, serve_single from sanic.server.socket import configure_socket, remove_unix_socket -from sanic.worker.inspector import Inspector from sanic.worker.loader import AppLoader from sanic.worker.manager import WorkerManager from sanic.worker.multiplexer import WorkerMultiplexer @@ -835,14 +834,16 @@ def serve( "packages": [sanic_version, *packages], "extra": extra, } - inspector = Inspector( + inspector = primary.inspector_class( monitor_pub, app_info, worker_state, primary.config.INSPECTOR_HOST, primary.config.INSPECTOR_PORT, + primary.config.INSPECTOR_TLS_KEY, + primary.config.INSPECTOR_TLS_CERT, ) - manager.manage("Inspector", inspector, {}, transient=False) + manager.manage("Inspector", inspector, {}, transient=True) primary._inspector = inspector primary._manager = manager diff --git a/sanic/worker/inspector.py b/sanic/worker/inspector.py index 769bc784fd..6938bcb6e0 100644 --- a/sanic/worker/inspector.py +++ b/sanic/worker/inspector.py @@ -1,17 +1,23 @@ import sys from datetime import datetime +from http.client import RemoteDisconnected +from inspect import isawaitable from multiprocessing.connection import Connection -from signal import SIGINT, SIGTERM -from signal import signal as signal_func -from socket import AF_INET, SOCK_STREAM, socket, timeout +from os import environ +from pathlib import Path from textwrap import indent -from typing import Any, Dict +from typing import Any, Dict, Union +from urllib.error import URLError +from urllib.request import Request as URequest +from urllib.request import urlopen from sanic.application.logo import get_logo from sanic.application.motd import MOTDTTY -from sanic.log import Colors, error_logger, logger -from sanic.server.socket import configure_socket +from sanic.helpers import Default +from sanic.log import Colors, logger +from sanic.request import Request +from sanic.response import json try: # no cov @@ -28,6 +34,8 @@ def __init__( worker_state: Dict[str, Any], host: str, port: int, + tls_key: Union[Path, str, Default], + tls_cert: Union[Path, str, Default], ): self._publisher = publisher self.run = True @@ -35,59 +43,70 @@ def __init__( self.worker_state = worker_state self.host = host self.port = port + self.tls_key = tls_key + self.tls_cert = tls_cert + + def setup(self): + self.app.get("/")(self.info) + self.app.post("/")(self.action) + environ["SANIC_IGNORE_PRODUCTION_WARNING"] = "true" + + def __call__(self, **_) -> None: + from sanic import Sanic + + self.app = Sanic("Inspector") + self.setup() + self.app.run( + host=self.host, + port=self.port, + single_process=True, + ssl={"key": self.tls_key, "cert": self.tls_cert} + if not isinstance(self.tls_key, Default) + and not isinstance(self.tls_cert, Default) + else None, + ) - def __call__(self) -> None: - sock = configure_socket( - {"host": self.host, "port": self.port, "unix": None, "backlog": 1} + async def info(self, request: Request): + return await self.respond(request, self.state_to_json()) + + async def action(self, request: Request, action: str): + logger.info("Incoming inspector action: %s", action) + output: Any = None + method = getattr(self, f"do_{action}", None) + if method: + output = method(request) + if isawaitable(output): + output = await output + + return await self.respond(request, output) + + async def respond(self, request: Request, output: Any): + name = request.match_info.get("action", "info") + return json( + {"meta": {"action": name}, "result": output}, + escape_forward_slashes=False, ) - assert sock - signal_func(SIGINT, self.stop) - signal_func(SIGTERM, self.stop) - logger.info(f"Inspector started on: {sock.getsockname()}") - sock.settimeout(0.5) - try: - while self.run: - try: - conn, _ = sock.accept() - except timeout: - continue - else: - action = conn.recv(64) - if action == b"reload": - self.reload() - elif action == b"shutdown": - self.shutdown() - elif action.startswith(b"scale"): - num_workers = int(action.split(b"=", 1)[-1]) - logger.info("Scaling to %s", num_workers) - self.scale(num_workers) - else: - data = dumps(self.state_to_json()) - conn.send(data.encode()) - conn.send(b"\n") - conn.close() - finally: - logger.info("Inspector closing") - sock.close() - - def stop(self, *_): - self.run = False - - def state_to_json(self): + def state_to_json(self) -> Dict[str, Any]: output = {"info": self.app_info} output["workers"] = self.make_safe(dict(self.worker_state)) return output - def reload(self): + def do_reload(self, _) -> None: message = "__ALL_PROCESSES__:" self._publisher.send(message) - def scale(self, num_workers: int): + def do_scale(self, request: Request) -> str: + num_workers = 1 + if request.body: + num_workers = request.json.get("replicas") + log_msg = f"Scaling to {num_workers}" + logger.info(log_msg) message = f"__SCALE__:{num_workers}" self._publisher.send(message) + return log_msg - def shutdown(self): + def do_shutdown(self, _) -> None: message = "__TERMINATE__" self._publisher.send(message) @@ -101,35 +120,47 @@ def make_safe(obj: Dict[str, Any]) -> Dict[str, Any]: return obj -def inspect(host: str, port: int, action: str): - out = sys.stdout.write - with socket(AF_INET, SOCK_STREAM) as sock: - try: - sock.connect((host, port)) - except ConnectionRefusedError: - error_logger.error( - f"{Colors.RED}Could not connect to inspector at: " - f"{Colors.YELLOW}{(host, port)}{Colors.END}\n" - "Either the application is not running, or it did not start " - "an inspector instance." +class InspectorClient: + def __init__(self, host: str, port: int, secure: bool, raw: bool) -> None: + self.scheme = "https" if secure else "http" + self.host = host + self.port = port + self.raw = raw + + for scheme in ("http", "https"): + full = f"{scheme}://" + if self.host.startswith(full): + self.scheme = scheme + self.host = self.host[len(full) :] # noqa E203 + + def do(self, action: str, **kwargs: Any) -> None: + if action == "info": + self.info() + return + result = self.request(action, **kwargs).get("result") + if result: + out = ( + dumps(result) + if isinstance(result, (list, dict)) + else str(result) ) - sock.close() - sys.exit(1) - sock.sendall(action.encode()) - data = sock.recv(4096) - if action == "raw": - out(data.decode()) - elif action == "pretty": - loaded = loads(data) - display = loaded.pop("info") + sys.stdout.write(out + "\n") + + def info(self) -> None: + out = sys.stdout.write + response = self.request("", "GET") + if self.raw: + return + data = response["result"] + display = data.pop("info") extra = display.pop("extra", {}) display["packages"] = ", ".join(display["packages"]) - MOTDTTY(get_logo(), f"{host}:{port}", display, extra).display( + MOTDTTY(get_logo(), self.base_url, display, extra).display( version=False, action="Inspecting", out=out, ) - for name, info in loaded["workers"].items(): + for name, info in data["workers"].items(): info = "\n".join( f"\t{key}: {Colors.BLUE}{value}{Colors.END}" for key, value in info.items() @@ -147,3 +178,32 @@ def inspect(host: str, port: int, action: str): ) + "\n" ) + + def request(self, action: str, method: str = "POST", **kwargs: Any) -> Any: + url = f"{self.base_url}/{action}" + params = {"method": method} + if kwargs: + params["data"] = dumps(kwargs).encode() + params["headers"] = {"content-type": "application/json"} + request = URequest(url, **params) + + try: + with urlopen(request) as response: # nosec B310 + raw = response.read() + loaded = loads(raw) + if self.raw: + sys.stdout.write(dumps(loaded.get("result")) + "\n") + return {} + return loaded + except (URLError, RemoteDisconnected) as e: + sys.stderr.write( + f"{Colors.RED}Could not connect to inspector at: " + f"{Colors.YELLOW}{self.base_url}{Colors.END}\n" + "Either the application is not running, or it did not start " + f"an inspector instance.\n{e}\n" + ) + sys.exit(1) + + @property + def base_url(self): + return f"{self.scheme}://{self.host}:{self.port}" diff --git a/sanic/worker/serve.py b/sanic/worker/serve.py index 8d233069ec..39c647b2b2 100644 --- a/sanic/worker/serve.py +++ b/sanic/worker/serve.py @@ -17,6 +17,7 @@ from sanic.server.runners import _serve_http_1, _serve_http_3 from sanic.worker.loader import AppLoader, CertLoader from sanic.worker.multiplexer import WorkerMultiplexer +from sanic.worker.process import Worker, WorkerProcess def worker_serve( @@ -79,7 +80,10 @@ def worker_serve( info.settings["ssl"] = ssl # When in a worker process, do some init - if os.environ.get("SANIC_WORKER_NAME"): + worker_name = os.environ.get("SANIC_WORKER_NAME") + if worker_name and worker_name.startswith( + Worker.WORKER_PREFIX + WorkerProcess.SERVER_LABEL + ): # Hydrate apps with any passed server info if monitor_publisher is None: From 3560b2653133351dacdd0daaaaac27fd58a416f4 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 14 Dec 2022 00:12:59 +0200 Subject: [PATCH 02/12] Rename some methods --- sanic/worker/inspector.py | 61 ++++++++++++++++++---------------- tests/worker/test_inspector.py | 16 ++++----- 2 files changed, 40 insertions(+), 37 deletions(-) diff --git a/sanic/worker/inspector.py b/sanic/worker/inspector.py index 6938bcb6e0..19a57383e8 100644 --- a/sanic/worker/inspector.py +++ b/sanic/worker/inspector.py @@ -46,16 +46,11 @@ def __init__( self.tls_key = tls_key self.tls_cert = tls_cert - def setup(self): - self.app.get("/")(self.info) - self.app.post("/")(self.action) - environ["SANIC_IGNORE_PRODUCTION_WARNING"] = "true" - def __call__(self, **_) -> None: from sanic import Sanic self.app = Sanic("Inspector") - self.setup() + self._setup() self.app.run( host=self.host, port=self.port, @@ -66,59 +61,67 @@ def __call__(self, **_) -> None: else None, ) - async def info(self, request: Request): - return await self.respond(request, self.state_to_json()) + def _setup(self): + self.app.get("/")(self._info) + self.app.post("/")(self._action) + environ["SANIC_IGNORE_PRODUCTION_WARNING"] = "true" - async def action(self, request: Request, action: str): + async def _action(self, request: Request, action: str): logger.info("Incoming inspector action: %s", action) output: Any = None - method = getattr(self, f"do_{action}", None) + method = getattr(self, action, None) if method: - output = method(request) + kwargs = {} + if request.body: + kwargs = request.json + output = method(**kwargs) if isawaitable(output): output = await output - return await self.respond(request, output) + return await self._respond(request, output) - async def respond(self, request: Request, output: Any): + async def _info(self, request: Request): + return await self._respond(request, self._state_to_json()) + + async def _respond(self, request: Request, output: Any): name = request.match_info.get("action", "info") return json( {"meta": {"action": name}, "result": output}, escape_forward_slashes=False, ) - def state_to_json(self) -> Dict[str, Any]: + def _state_to_json(self) -> Dict[str, Any]: output = {"info": self.app_info} - output["workers"] = self.make_safe(dict(self.worker_state)) + output["workers"] = self._make_safe(dict(self.worker_state)) return output - def do_reload(self, _) -> None: + @staticmethod + def _make_safe(obj: Dict[str, Any]) -> Dict[str, Any]: + for key, value in obj.items(): + if isinstance(value, dict): + obj[key] = Inspector._make_safe(value) + elif isinstance(value, datetime): + obj[key] = value.isoformat() + return obj + + def reload(self) -> None: message = "__ALL_PROCESSES__:" self._publisher.send(message) - def do_scale(self, request: Request) -> str: + def scale(self, replicas) -> str: num_workers = 1 - if request.body: - num_workers = request.json.get("replicas") + if replicas: + num_workers = int(replicas) log_msg = f"Scaling to {num_workers}" logger.info(log_msg) message = f"__SCALE__:{num_workers}" self._publisher.send(message) return log_msg - def do_shutdown(self, _) -> None: + def shutdown(self) -> None: message = "__TERMINATE__" self._publisher.send(message) - @staticmethod - def make_safe(obj: Dict[str, Any]) -> Dict[str, Any]: - for key, value in obj.items(): - if isinstance(value, dict): - obj[key] = Inspector.make_safe(value) - elif isinstance(value, datetime): - obj[key] = value.isoformat() - return obj - class InspectorClient: def __init__(self, host: str, port: int, secure: bool, raw: bool) -> None: diff --git a/tests/worker/test_inspector.py b/tests/worker/test_inspector.py index 0c9eb90654..ae10b690fa 100644 --- a/tests/worker/test_inspector.py +++ b/tests/worker/test_inspector.py @@ -86,7 +86,7 @@ def test_run_inspector(configure_socket: Mock, action: bytes): inspector.reload = Mock() # type: ignore inspector.shutdown = Mock() # type: ignore inspector.scale = Mock() # type: ignore - inspector.state_to_json = Mock(return_value="foo") # type: ignore + inspector._state_to_json = Mock(return_value="foo") # type: ignore def accept(): inspector.run = False @@ -106,22 +106,22 @@ def accept(): inspector.reload.assert_called() inspector.shutdown.assert_not_called() inspector.scale.assert_not_called() - inspector.state_to_json.assert_not_called() + inspector._state_to_json.assert_not_called() elif action == b"shutdown": inspector.reload.assert_not_called() inspector.shutdown.assert_called() inspector.scale.assert_not_called() - inspector.state_to_json.assert_not_called() + inspector._state_to_json.assert_not_called() elif action.startswith(b"scale"): inspector.reload.assert_not_called() inspector.shutdown.assert_not_called() inspector.scale.assert_called_once_with(5) - inspector.state_to_json.assert_not_called() + inspector._state_to_json.assert_not_called() else: inspector.reload.assert_not_called() inspector.shutdown.assert_not_called() inspector.scale.assert_not_called() - inspector.state_to_json.assert_called() + inspector._state_to_json.assert_called() @patch("sanic.worker.inspector.configure_socket") @@ -131,7 +131,7 @@ def test_accept_timeout(configure_socket: Mock): inspector = Inspector(Mock(), {}, {}, "localhost", 9999) inspector.reload = Mock() # type: ignore inspector.shutdown = Mock() # type: ignore - inspector.state_to_json = Mock(return_value="foo") # type: ignore + inspector._state_to_json = Mock(return_value="foo") # type: ignore def accept(): inspector.run = False @@ -143,7 +143,7 @@ def accept(): inspector.reload.assert_not_called() inspector.shutdown.assert_not_called() - inspector.state_to_json.assert_not_called() + inspector._state_to_json.assert_not_called() def test_state_to_json(): @@ -152,7 +152,7 @@ def test_state_to_json(): app_info = {"app": "hello"} worker_state = {"Test": {"now": now, "nested": {"foo": now}}} inspector = Inspector(Mock(), app_info, worker_state, "", 0) - state = inspector.state_to_json() + state = inspector._state_to_json() assert state == { "info": app_info, From 42c44677543b4e115e77fe6bc0a89e17124affd1 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 14 Dec 2022 00:46:32 +0200 Subject: [PATCH 03/12] Flex keyord args --- sanic/cli/inspector.py | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/sanic/cli/inspector.py b/sanic/cli/inspector.py index d300f1e52b..e6e9716c1d 100644 --- a/sanic/cli/inspector.py +++ b/sanic/cli/inspector.py @@ -1,9 +1,10 @@ from argparse import ArgumentParser +from sanic.application.logo import get_logo from sanic.cli.base import SanicHelpFormatter, SanicSubParsersAction -def make_inspector_parser(parser: ArgumentParser) -> None: +def _add_shared(parser: ArgumentParser) -> None: parser.add_argument("--host", "-H", default="localhost") parser.add_argument("--port", "-p", default=6457, type=int) parser.add_argument("--secure", "-s", action="store_true") @@ -13,6 +14,17 @@ def make_inspector_parser(parser: ArgumentParser) -> None: help="Whether to output the raw response information", ) + +class InspectorSubParser(ArgumentParser): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + _add_shared(self) + if not self.description: + self.description = get_logo(True) + + +def make_inspector_parser(parser: ArgumentParser) -> None: + _add_shared(parser) subparsers = parser.add_subparsers( action=SanicSubParsersAction, dest="action", @@ -21,15 +33,24 @@ def make_inspector_parser(parser: ArgumentParser) -> None: "Inspector instance, then you can run custom commands.\nSee ___ " "for more details." ), - title="Subcommands", + title="Required\n========\n Subcommands", + parser_class=InspectorSubParser, ) subparsers.add_parser( - "reload", help="Trigger a reload of the server workers" + "reload", + help="Trigger a reload of the server workers", + formatter_class=SanicHelpFormatter, ) subparsers.add_parser( - "shutdown", help="Shutdown the application and all processes" + "shutdown", + help="Shutdown the application and all processes", + formatter_class=SanicHelpFormatter, + ) + scale = subparsers.add_parser( + "scale", + help="Scale the number of workers", + formatter_class=SanicHelpFormatter, ) - scale = subparsers.add_parser("scale", help="Scale the number of workers") scale.add_argument("replicas", type=int) custom = subparsers.add_parser( From 6e4a4491a40037831eccb78fb2a7685bf9ad3400 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 14 Dec 2022 01:14:20 +0200 Subject: [PATCH 04/12] Cleanup help messages --- sanic/cli/base.py | 8 +++++++- sanic/cli/inspector.py | 7 +++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/sanic/cli/base.py b/sanic/cli/base.py index 2822263e52..270c4118b2 100644 --- a/sanic/cli/base.py +++ b/sanic/cli/base.py @@ -1,4 +1,5 @@ from argparse import ( + SUPPRESS, Action, ArgumentParser, RawTextHelpFormatter, @@ -15,7 +16,12 @@ def _check_value(self, action: Action, value: Any) -> None: class SanicHelpFormatter(RawTextHelpFormatter): - ... + def add_usage(self, usage, actions, groups, prefix=None): + if not usage: + usage = SUPPRESS + # Add one linebreak, but not two + self.add_text("\x1b[1A'") + super().add_usage(usage, actions, groups, prefix) class SanicSubParsersAction(_SubParsersAction): diff --git a/sanic/cli/inspector.py b/sanic/cli/inspector.py index e6e9716c1d..1811fe4a31 100644 --- a/sanic/cli/inspector.py +++ b/sanic/cli/inspector.py @@ -20,7 +20,8 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) _add_shared(self) if not self.description: - self.description = get_logo(True) + self.description = "" + self.description = get_logo(True) + self.description def make_inspector_parser(parser: ArgumentParser) -> None: @@ -64,5 +65,7 @@ def make_inspector_parser(parser: ArgumentParser) -> None: formatter_class=SanicHelpFormatter, ) custom.add_argument( - "positional", nargs="*", help="Add one or more non-keyword args" + "positional", + nargs="*", + help="Add one or more non-keyword args to your custom command", ) From 3b72c70f71eed9072167a67e679f1f897644ea6e Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 14 Dec 2022 08:36:31 +0200 Subject: [PATCH 05/12] Add API key authentication --- sanic/cli/app.py | 3 ++- sanic/cli/inspector.py | 1 + sanic/config.py | 2 ++ sanic/mixins/startup.py | 1 + sanic/worker/inspector.py | 27 +++++++++++++++++++++++---- 5 files changed, 29 insertions(+), 5 deletions(-) diff --git a/sanic/cli/app.py b/sanic/cli/app.py index 3e8c2f89d4..b7db1afe0f 100644 --- a/sanic/cli/app.py +++ b/sanic/cli/app.py @@ -153,12 +153,13 @@ def _inspector(self): secure = kwargs.pop("secure") raw = kwargs.pop("raw") action = kwargs.pop("action") or "info" + api_key = kwargs.pop("api_key") positional = kwargs.pop("positional", None) if action == "" and positional: action = positional[0] if len(positional) > 1: kwargs["args"] = positional[1:] - InspectorClient(host, port, secure, raw).do(action, **kwargs) + InspectorClient(host, port, secure, raw, api_key).do(action, **kwargs) def _precheck(self): # Custom TLS mismatch handling for better diagnostics diff --git a/sanic/cli/inspector.py b/sanic/cli/inspector.py index 1811fe4a31..5a1719b49f 100644 --- a/sanic/cli/inspector.py +++ b/sanic/cli/inspector.py @@ -8,6 +8,7 @@ def _add_shared(parser: ArgumentParser) -> None: parser.add_argument("--host", "-H", default="localhost") parser.add_argument("--port", "-p", default=6457, type=int) parser.add_argument("--secure", "-s", action="store_true") + parser.add_argument("--api-key", "-k") parser.add_argument( "--raw", action="store_true", diff --git a/sanic/config.py b/sanic/config.py index 09cb3c38df..0d5eabf80b 100644 --- a/sanic/config.py +++ b/sanic/config.py @@ -48,6 +48,7 @@ "INSPECTOR_PORT": 6457, "INSPECTOR_TLS_KEY": _default, "INSPECTOR_TLS_CERT": _default, + "INSPECTOR_API_KEY": "", "KEEP_ALIVE_TIMEOUT": 5, # 5 seconds "KEEP_ALIVE": True, "LOCAL_CERT_CREATOR": LocalCertCreator.AUTO, @@ -97,6 +98,7 @@ class Config(dict, metaclass=DescriptorMeta): INSPECTOR_PORT: int INSPECTOR_TLS_KEY: Union[Path, str, Default] INSPECTOR_TLS_CERT: Union[Path, str, Default] + INSPECTOR_API_KEY: str KEEP_ALIVE_TIMEOUT: int KEEP_ALIVE: bool LOCAL_CERT_CREATOR: Union[str, LocalCertCreator] diff --git a/sanic/mixins/startup.py b/sanic/mixins/startup.py index 2adef3b306..bc6e581371 100644 --- a/sanic/mixins/startup.py +++ b/sanic/mixins/startup.py @@ -840,6 +840,7 @@ def serve( worker_state, primary.config.INSPECTOR_HOST, primary.config.INSPECTOR_PORT, + primary.config.INSPECTOR_API_KEY, primary.config.INSPECTOR_TLS_KEY, primary.config.INSPECTOR_TLS_CERT, ) diff --git a/sanic/worker/inspector.py b/sanic/worker/inspector.py index 19a57383e8..08e705e13a 100644 --- a/sanic/worker/inspector.py +++ b/sanic/worker/inspector.py @@ -7,13 +7,14 @@ from os import environ from pathlib import Path from textwrap import indent -from typing import Any, Dict, Union +from typing import Any, Dict, Optional, Union from urllib.error import URLError from urllib.request import Request as URequest from urllib.request import urlopen from sanic.application.logo import get_logo from sanic.application.motd import MOTDTTY +from sanic.exceptions import Unauthorized from sanic.helpers import Default from sanic.log import Colors, logger from sanic.request import Request @@ -34,6 +35,7 @@ def __init__( worker_state: Dict[str, Any], host: str, port: int, + api_key: str, tls_key: Union[Path, str, Default], tls_cert: Union[Path, str, Default], ): @@ -43,6 +45,7 @@ def __init__( self.worker_state = worker_state self.host = host self.port = port + self.api_key = api_key self.tls_key = tls_key self.tls_cert = tls_cert @@ -64,8 +67,14 @@ def __call__(self, **_) -> None: def _setup(self): self.app.get("/")(self._info) self.app.post("/")(self._action) + if self.api_key: + self.app.on_request(self._authentication) environ["SANIC_IGNORE_PRODUCTION_WARNING"] = "true" + def _authentication(self, request: Request) -> None: + if request.token != self.api_key: + raise Unauthorized("Bad API key") + async def _action(self, request: Request, action: str): logger.info("Incoming inspector action: %s", action) output: Any = None @@ -124,11 +133,19 @@ def shutdown(self) -> None: class InspectorClient: - def __init__(self, host: str, port: int, secure: bool, raw: bool) -> None: + def __init__( + self, + host: str, + port: int, + secure: bool, + raw: bool, + api_key: Optional[str], + ) -> None: self.scheme = "https" if secure else "http" self.host = host self.port = port self.raw = raw + self.api_key = api_key for scheme in ("http", "https"): full = f"{scheme}://" @@ -184,10 +201,12 @@ def info(self) -> None: def request(self, action: str, method: str = "POST", **kwargs: Any) -> Any: url = f"{self.base_url}/{action}" - params = {"method": method} + params = {"method": method, "headers": {}} if kwargs: params["data"] = dumps(kwargs).encode() - params["headers"] = {"content-type": "application/json"} + params["headers"]["content-type"] = "application/json" + if self.api_key: + params["headers"]["authorization"] = f"Bearer {self.api_key}" request = URequest(url, **params) try: From 199b3bad3d284bbbc84d0fdb37b8f3f362a9a2ba Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 14 Dec 2022 15:21:28 +0200 Subject: [PATCH 06/12] Add some tests --- sanic/worker/inspector.py | 27 ++-- tests/worker/test_inspector.py | 237 ++++++++++++++------------------- 2 files changed, 114 insertions(+), 150 deletions(-) diff --git a/sanic/worker/inspector.py b/sanic/worker/inspector.py index 08e705e13a..31d842ad6c 100644 --- a/sanic/worker/inspector.py +++ b/sanic/worker/inspector.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import sys from datetime import datetime @@ -40,7 +42,6 @@ def __init__( tls_cert: Union[Path, str, Default], ): self._publisher = publisher - self.run = True self.app_info = app_info self.worker_state = worker_state self.host = host @@ -49,20 +50,22 @@ def __init__( self.tls_key = tls_key self.tls_cert = tls_cert - def __call__(self, **_) -> None: + def __call__(self, run=True, **_) -> Inspector: from sanic import Sanic self.app = Sanic("Inspector") self._setup() - self.app.run( - host=self.host, - port=self.port, - single_process=True, - ssl={"key": self.tls_key, "cert": self.tls_cert} - if not isinstance(self.tls_key, Default) - and not isinstance(self.tls_cert, Default) - else None, - ) + if run: + self.app.run( + host=self.host, + port=self.port, + single_process=True, + ssl={"key": self.tls_key, "cert": self.tls_cert} + if not isinstance(self.tls_key, Default) + and not isinstance(self.tls_cert, Default) + else None, + ) + return self def _setup(self): self.app.get("/")(self._info) @@ -169,7 +172,7 @@ def do(self, action: str, **kwargs: Any) -> None: def info(self) -> None: out = sys.stdout.write response = self.request("", "GET") - if self.raw: + if self.raw or not response: return data = response["result"] display = data.pop("info") diff --git a/tests/worker/test_inspector.py b/tests/worker/test_inspector.py index ae10b690fa..8ece2072a5 100644 --- a/tests/worker/test_inspector.py +++ b/tests/worker/test_inspector.py @@ -1,14 +1,19 @@ -import json +try: # no cov + from ujson import dumps +except ModuleNotFoundError: # no cov + from json import dumps # type: ignore from datetime import datetime -from logging import ERROR, INFO -from socket import AF_INET, SOCK_STREAM, timeout from unittest.mock import Mock, patch +from urllib.error import URLError import pytest +from sanic_testing import TestManager + +from sanic.helpers import Default from sanic.log import Colors -from sanic.worker.inspector import Inspector, inspect +from sanic.worker.inspector import Inspector, InspectorClient DATA = { @@ -20,130 +25,95 @@ }, "workers": {"Worker-Name": {"some": "state"}}, } -SERIALIZED = json.dumps(DATA) +FULL_SERIALIZED = dumps({"result": DATA}) +OUT_SERIALIZED = dumps(DATA) + + +class FooInspector(Inspector): + async def foo(self, bar): + return f"bar is {bar}" + + +@pytest.fixture +def urlopen(): + urlopen = Mock() + urlopen.return_value = urlopen + urlopen.__enter__ = Mock(return_value=urlopen) + urlopen.__exit__ = Mock() + urlopen.read = Mock() + with patch("sanic.worker.inspector.urlopen", urlopen): + yield urlopen -def test_inspector_stop(): - inspector = Inspector(Mock(), {}, {}, "", 1) - assert inspector.run is True - inspector.stop() - assert inspector.run is False +@pytest.fixture +def publisher(): + publisher = Mock() + return publisher + + +@pytest.fixture +def inspector(publisher): + inspector = FooInspector( + publisher, {}, {}, "localhost", 9999, "", Default(), Default() + ) + inspector(False) + return inspector + +@pytest.fixture +def http_client(inspector): + manager = TestManager(inspector.app) + return manager.test_client + +@pytest.mark.parametrize("command", ("info",)) @patch("sanic.worker.inspector.sys.stdout.write") -@patch("sanic.worker.inspector.socket") -@pytest.mark.parametrize("command", ("foo", "raw", "pretty")) -def test_send_inspect(socket: Mock, write: Mock, command: str): - socket.return_value = socket - socket.__enter__.return_value = socket - socket.recv.return_value = SERIALIZED.encode() - inspect("localhost", 9999, command) - - socket.sendall.assert_called_once_with(command.encode()) - socket.recv.assert_called_once_with(4096) - socket.connect.assert_called_once_with(("localhost", 9999)) - socket.assert_called_once_with(AF_INET, SOCK_STREAM) - - if command == "raw": - write.assert_called_once_with(SERIALIZED) - elif command == "pretty": - write.assert_called() - else: - write.assert_not_called() +def test_send_inspect(write, urlopen, command: str): + urlopen.read.return_value = FULL_SERIALIZED.encode() + InspectorClient("localhost", 9999, False, False, None).do(command) + write.assert_called() + write.reset_mock() + InspectorClient("localhost", 9999, False, True, None).do(command) + write.assert_called_with(OUT_SERIALIZED + "\n") @patch("sanic.worker.inspector.sys") -@patch("sanic.worker.inspector.socket") -def test_send_inspect_conn_refused(socket: Mock, sys: Mock, caplog): - with caplog.at_level(INFO): - socket.return_value = socket - socket.__enter__.return_value = socket - socket.connect.side_effect = ConnectionRefusedError() - inspect("localhost", 9999, "foo") - - socket.close.assert_called_once() - sys.exit.assert_called_once_with(1) +def test_send_inspect_conn_refused(sys: Mock, urlopen): + urlopen.side_effect = URLError("") + InspectorClient("localhost", 9999, False, False, None).do("info") message = ( f"{Colors.RED}Could not connect to inspector at: " - f"{Colors.YELLOW}('localhost', 9999){Colors.END}\n" + f"{Colors.YELLOW}http://localhost:9999{Colors.END}\n" "Either the application is not running, or it did not start " - "an inspector instance." - ) - assert ("sanic.error", ERROR, message) in caplog.record_tuples - - -@patch("sanic.worker.inspector.configure_socket") -@pytest.mark.parametrize( - "action", (b"reload", b"shutdown", b"scale=5", b"foo") -) -def test_run_inspector(configure_socket: Mock, action: bytes): - sock = Mock() - conn = Mock() - conn.recv.return_value = action - configure_socket.return_value = sock - inspector = Inspector(Mock(), {}, {}, "localhost", 9999) - inspector.reload = Mock() # type: ignore - inspector.shutdown = Mock() # type: ignore - inspector.scale = Mock() # type: ignore - inspector._state_to_json = Mock(return_value="foo") # type: ignore - - def accept(): - inspector.run = False - return conn, ... - - sock.accept = accept - - inspector() - - configure_socket.assert_called_once_with( - {"host": "localhost", "port": 9999, "unix": None, "backlog": 1} + "an inspector instance.\n\n" ) - conn.recv.assert_called_with(64) - - conn.send.assert_called_with(b"\n") - if action == b"reload": - inspector.reload.assert_called() - inspector.shutdown.assert_not_called() - inspector.scale.assert_not_called() - inspector._state_to_json.assert_not_called() - elif action == b"shutdown": - inspector.reload.assert_not_called() - inspector.shutdown.assert_called() - inspector.scale.assert_not_called() - inspector._state_to_json.assert_not_called() - elif action.startswith(b"scale"): - inspector.reload.assert_not_called() - inspector.shutdown.assert_not_called() - inspector.scale.assert_called_once_with(5) - inspector._state_to_json.assert_not_called() - else: - inspector.reload.assert_not_called() - inspector.shutdown.assert_not_called() - inspector.scale.assert_not_called() - inspector._state_to_json.assert_called() - - -@patch("sanic.worker.inspector.configure_socket") -def test_accept_timeout(configure_socket: Mock): - sock = Mock() - configure_socket.return_value = sock - inspector = Inspector(Mock(), {}, {}, "localhost", 9999) - inspector.reload = Mock() # type: ignore - inspector.shutdown = Mock() # type: ignore - inspector._state_to_json = Mock(return_value="foo") # type: ignore - - def accept(): - inspector.run = False - raise timeout - - sock.accept = accept - - inspector() - - inspector.reload.assert_not_called() - inspector.shutdown.assert_not_called() - inspector._state_to_json.assert_not_called() + sys.exit.assert_called_once_with(1) + sys.stderr.write.assert_called_once_with(message) + + +def test_run_inspector_reload(publisher, http_client): + _, response = http_client.post("/reload") + assert response.status == 200 + publisher.send.assert_called_once_with("__ALL_PROCESSES__:") + + +def test_run_inspector_shutdown(publisher, http_client): + _, response = http_client.post("/shutdown") + assert response.status == 200 + publisher.send.assert_called_once_with("__TERMINATE__") + + +def test_run_inspector_scale(publisher, http_client): + _, response = http_client.post("/scale", json={"replicas": 4}) + assert response.status == 200 + publisher.send.assert_called_once_with("__SCALE__:4") + + +def test_run_inspector_arbitrary(http_client): + _, response = http_client.post("/foo", json={"bar": 99}) + assert response.status == 200 + assert response.json == {"meta": {"action": "foo"}, "result": "bar is 99"} def test_state_to_json(): @@ -151,7 +121,9 @@ def test_state_to_json(): now_iso = now.isoformat() app_info = {"app": "hello"} worker_state = {"Test": {"now": now, "nested": {"foo": now}}} - inspector = Inspector(Mock(), app_info, worker_state, "", 0) + inspector = Inspector( + Mock(), app_info, worker_state, "", 0, "", Default(), Default() + ) state = inspector._state_to_json() assert state == { @@ -160,25 +132,14 @@ def test_state_to_json(): } -def test_reload(): - publisher = Mock() - inspector = Inspector(publisher, {}, {}, "", 0) - inspector.reload() - - publisher.send.assert_called_once_with("__ALL_PROCESSES__:") - - -def test_shutdown(): - publisher = Mock() - inspector = Inspector(publisher, {}, {}, "", 0) - inspector.shutdown() - - publisher.send.assert_called_once_with("__TERMINATE__") - - -def test_scale(): - publisher = Mock() - inspector = Inspector(publisher, {}, {}, "", 0) - inspector.scale(3) - - publisher.send.assert_called_once_with("__SCALE__:3") +def test_run_inspector_authentication(): + inspector = Inspector( + Mock(), {}, {}, "", 0, "super-secret", Default(), Default() + )(False) + manager = TestManager(inspector.app) + _, response = manager.test_client.get("/") + assert response.status == 401 + _, response = manager.test_client.get( + "/", headers={"Authorization": "Bearer super-secret"} + ) + assert response.status == 200 From eb207e8039e70f93f385db3c60005b72de01228e Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 14 Dec 2022 16:29:14 +0200 Subject: [PATCH 07/12] CLI tests --- tests/conftest.py | 13 +++++++++- tests/test_cli.py | 46 ++++++++++++++++++++++++++++++++++ tests/worker/test_inspector.py | 11 -------- 3 files changed, 58 insertions(+), 12 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 18e74daf81..5ad14566ae 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,7 +9,7 @@ from contextlib import suppress from logging import LogRecord from typing import List, Tuple -from unittest.mock import MagicMock +from unittest.mock import MagicMock, Mock, patch import pytest @@ -220,3 +220,14 @@ def sanic_ext(ext_instance): # noqa yield sanic_ext with suppress(KeyError): del sys.modules["sanic_ext"] + + +@pytest.fixture +def urlopen(): + urlopen = Mock() + urlopen.return_value = urlopen + urlopen.__enter__ = Mock(return_value=urlopen) + urlopen.__exit__ = Mock() + urlopen.read = Mock() + with patch("sanic.worker.inspector.urlopen", urlopen): + yield urlopen diff --git a/tests/test_cli.py b/tests/test_cli.py index fd055c50ab..b3d775296c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -4,6 +4,7 @@ from pathlib import Path from typing import List, Optional, Tuple +from unittest.mock import patch import pytest @@ -11,6 +12,7 @@ from sanic import __version__ from sanic.__main__ import main +from sanic.worker.inspector import InspectorClient @pytest.fixture(scope="module", autouse=True) @@ -286,3 +288,47 @@ def test_noisy_exceptions(cmd: str, expected: bool, caplog): info = read_app_info(lines) assert info["noisy_exceptions"] is expected + + +def test_inspector_inspect(urlopen, caplog, capsys): + urlopen.read.return_value = json.dumps( + { + "result": { + "info": { + "packages": ["foo"], + }, + "extra": { + "more": "data", + }, + "workers": {"Worker-Name": {"some": "state"}}, + } + } + ).encode() + with patch("sys.argv", ["sanic", "inspect"]): + capture(["inspect"], caplog) + captured = capsys.readouterr() + assert "Inspecting @ http://localhost:6457" in captured.out + assert "Worker-Name" in captured.out + assert captured.err == "" + + +@pytest.mark.parametrize( + "command,params", + ( + (["reload"], {}), + (["shutdown"], {}), + (["scale", "9"], {"replicas": 9}), + (["foo", "--bar=something"], {"bar": "something"}), + (["foo", "positional"], {"args": ["positional"]}), + ( + ["foo", "positional", "--bar=something"], + {"args": ["positional"], "bar": "something"}, + ), + ), +) +def test_inspector_command(command, params): + with patch.object(InspectorClient, "request") as client: + with patch("sys.argv", ["sanic", "inspect", *command]): + main() + + client.assert_called_once_with(command[0], **params) diff --git a/tests/worker/test_inspector.py b/tests/worker/test_inspector.py index 8ece2072a5..6fa36e1725 100644 --- a/tests/worker/test_inspector.py +++ b/tests/worker/test_inspector.py @@ -34,17 +34,6 @@ async def foo(self, bar): return f"bar is {bar}" -@pytest.fixture -def urlopen(): - urlopen = Mock() - urlopen.return_value = urlopen - urlopen.__enter__ = Mock(return_value=urlopen) - urlopen.__exit__ = Mock() - urlopen.read = Mock() - with patch("sanic.worker.inspector.urlopen", urlopen): - yield urlopen - - @pytest.fixture def publisher(): publisher = Mock() From 798b1286899d6713734b0d4dfc202169f34ed5e5 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 14 Dec 2022 16:46:38 +0200 Subject: [PATCH 08/12] Typing fixes --- sanic/application/ext.py | 7 +------ sanic/cli/app.py | 14 +++++++++----- sanic/http/tls/context.py | 2 +- sanic/worker/inspector.py | 2 +- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/sanic/application/ext.py b/sanic/application/ext.py index 405d0a7fe1..0f4bdfb173 100644 --- a/sanic/application/ext.py +++ b/sanic/application/ext.py @@ -8,11 +8,6 @@ if TYPE_CHECKING: from sanic import Sanic - try: - from sanic_ext import Extend # type: ignore - except ImportError: - ... - def setup_ext(app: Sanic, *, fail: bool = False, **kwargs): if not app.config.AUTO_EXTEND: @@ -33,7 +28,7 @@ def setup_ext(app: Sanic, *, fail: bool = False, **kwargs): return if not getattr(app, "_ext", None): - Ext: Extend = getattr(sanic_ext, "Extend") + Ext = getattr(sanic_ext, "Extend") app._ext = Ext(app, **kwargs) return app.ext diff --git a/sanic/cli/app.py b/sanic/cli/app.py index b7db1afe0f..23647d44b9 100644 --- a/sanic/cli/app.py +++ b/sanic/cli/app.py @@ -3,9 +3,10 @@ import shutil import sys +from argparse import Namespace from functools import partial from textwrap import indent -from typing import Any, List, Union +from typing import List, Union, cast from sanic.app import Sanic from sanic.application.logo import get_logo @@ -55,7 +56,7 @@ def __init__(self) -> None: self.main_process = ( os.environ.get("SANIC_RELOADER_PROCESS", "") != "true" ) - self.args: List[Any] = [] + self.args: Namespace = Namespace() self.groups: List[Group] = [] self.inspecting = False @@ -119,8 +120,9 @@ def run(self, parse_args=None): def _inspector_legacy(self, app_loader: AppLoader): host = port = None - if ":" in self.args.module: - maybe_host, maybe_port = self.args.module.rsplit(":", 1) + module = cast(str, self.args.module) + if ":" in module: + maybe_host, maybe_port = module.rsplit(":", 1) if maybe_port.isnumeric(): host, port = maybe_host, int(maybe_port) if not host: @@ -129,7 +131,9 @@ def _inspector_legacy(self, app_loader: AppLoader): action = self.args.trigger or "info" - InspectorClient(host, port, False, self.args.inspect_raw).do(action) + InspectorClient( + str(host), int(port or 6457), False, self.args.inspect_raw, "" + ).do(action) sys.stdout.write( f"\n{Colors.BOLD}{Colors.YELLOW}WARNING:{Colors.END} " "You are using the legacy CLI command that will be removed in " diff --git a/sanic/http/tls/context.py b/sanic/http/tls/context.py index 2a123e52b5..98c090bb34 100644 --- a/sanic/http/tls/context.py +++ b/sanic/http/tls/context.py @@ -33,7 +33,7 @@ def create_context( context.set_alpn_protocols(["http/1.1"]) if purpose is ssl.Purpose.CLIENT_AUTH: context.sni_callback = server_name_callback - if certfile or keyfile: + if certfile and keyfile: context.load_cert_chain(certfile, keyfile, password) return context diff --git a/sanic/worker/inspector.py b/sanic/worker/inspector.py index 31d842ad6c..80aaf816d5 100644 --- a/sanic/worker/inspector.py +++ b/sanic/worker/inspector.py @@ -204,7 +204,7 @@ def info(self) -> None: def request(self, action: str, method: str = "POST", **kwargs: Any) -> Any: url = f"{self.base_url}/{action}" - params = {"method": method, "headers": {}} + params: Dict[str, Any] = {"method": method, "headers": {}} if kwargs: params["data"] = dumps(kwargs).encode() params["headers"]["content-type"] = "application/json" From 3f27c769e886bea53319733951cae512f5905884 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 14 Dec 2022 16:59:09 +0200 Subject: [PATCH 09/12] fix tests --- sanic/mixins/startup.py | 13 +++++++------ sanic/worker/inspector.py | 4 ++-- tests/worker/test_worker_serve.py | 16 +++++++++++----- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/sanic/mixins/startup.py b/sanic/mixins/startup.py index bc6e581371..dbe0179656 100644 --- a/sanic/mixins/startup.py +++ b/sanic/mixins/startup.py @@ -27,6 +27,7 @@ Callable, Dict, List, + Mapping, Optional, Set, Tuple, @@ -124,7 +125,7 @@ def run( register_sys_signals: bool = True, access_log: Optional[bool] = None, unix: Optional[str] = None, - loop: AbstractEventLoop = None, + loop: Optional[AbstractEventLoop] = None, reload_dir: Optional[Union[List[str], str]] = None, noisy_exceptions: Optional[bool] = None, motd: bool = True, @@ -223,7 +224,7 @@ def prepare( register_sys_signals: bool = True, access_log: Optional[bool] = None, unix: Optional[str] = None, - loop: AbstractEventLoop = None, + loop: Optional[AbstractEventLoop] = None, reload_dir: Optional[Union[List[str], str]] = None, noisy_exceptions: Optional[bool] = None, motd: bool = True, @@ -353,12 +354,12 @@ async def create_server( debug: bool = False, ssl: Union[None, SSLContext, dict, str, list, tuple] = None, sock: Optional[socket] = None, - protocol: Type[Protocol] = None, + protocol: Optional[Type[Protocol]] = None, backlog: int = 100, access_log: Optional[bool] = None, unix: Optional[str] = None, return_asyncio_server: bool = False, - asyncio_server_kwargs: Dict[str, Any] = None, + asyncio_server_kwargs: Optional[Dict[str, Any]] = None, noisy_exceptions: Optional[bool] = None, ) -> Optional[AsyncioServer]: """ @@ -479,7 +480,7 @@ def _helper( sock: Optional[socket] = None, unix: Optional[str] = None, workers: int = 1, - loop: AbstractEventLoop = None, + loop: Optional[AbstractEventLoop] = None, protocol: Type[Protocol] = HttpProtocol, backlog: int = 100, register_sys_signals: bool = True, @@ -762,7 +763,7 @@ def serve( ] primary_server_info.settings["run_multiple"] = True monitor_sub, monitor_pub = Pipe(True) - worker_state: Dict[str, Any] = sync_manager.dict() + worker_state: Mapping[str, Any] = sync_manager.dict() kwargs: Dict[str, Any] = { **primary_server_info.settings, "monitor_publisher": monitor_pub, diff --git a/sanic/worker/inspector.py b/sanic/worker/inspector.py index 80aaf816d5..333b599e0e 100644 --- a/sanic/worker/inspector.py +++ b/sanic/worker/inspector.py @@ -9,7 +9,7 @@ from os import environ from pathlib import Path from textwrap import indent -from typing import Any, Dict, Optional, Union +from typing import Any, Dict, Mapping, Optional, Union from urllib.error import URLError from urllib.request import Request as URequest from urllib.request import urlopen @@ -34,7 +34,7 @@ def __init__( self, publisher: Connection, app_info: Dict[str, Any], - worker_state: Dict[str, Any], + worker_state: Mapping[str, Any], host: str, port: int, api_key: str, diff --git a/tests/worker/test_worker_serve.py b/tests/worker/test_worker_serve.py index b24f7e8f42..a33e3cacc6 100644 --- a/tests/worker/test_worker_serve.py +++ b/tests/worker/test_worker_serve.py @@ -8,6 +8,7 @@ from sanic.app import Sanic from sanic.worker.loader import AppLoader from sanic.worker.multiplexer import WorkerMultiplexer +from sanic.worker.process import Worker, WorkerProcess from sanic.worker.serve import worker_serve @@ -40,7 +41,9 @@ def test_config_app(mock_app: Mock): def test_bad_process(mock_app: Mock, caplog): - environ["SANIC_WORKER_NAME"] = "FOO" + environ["SANIC_WORKER_NAME"] = ( + Worker.WORKER_PREFIX + WorkerProcess.SERVER_LABEL + "-FOO" + ) message = "No restart publisher found in worker process" with pytest.raises(RuntimeError, match=message): @@ -58,7 +61,9 @@ def test_bad_process(mock_app: Mock, caplog): def test_has_multiplexer(app: Sanic): - environ["SANIC_WORKER_NAME"] = "FOO" + environ["SANIC_WORKER_NAME"] = ( + Worker.WORKER_PREFIX + WorkerProcess.SERVER_LABEL + "-FOO" + ) Sanic.register_app(app) with patch("sanic.worker.serve._serve_http_1"): @@ -97,12 +102,13 @@ def test_serve_app_factory(wm: Mock, mock_app): @patch("sanic.mixins.startup.WorkerManager") -@patch("sanic.mixins.startup.Inspector") @pytest.mark.parametrize("config", (True, False)) def test_serve_with_inspector( - Inspector: Mock, WorkerManager: Mock, mock_app: Mock, config: bool + WorkerManager: Mock, mock_app: Mock, config: bool ): + Inspector = Mock() mock_app.config.INSPECTOR = config + mock_app.inspector_class = Inspector inspector = Mock() Inspector.return_value = inspector WorkerManager.return_value = WorkerManager @@ -112,7 +118,7 @@ def test_serve_with_inspector( if config: Inspector.assert_called_once() WorkerManager.manage.assert_called_once_with( - "Inspector", inspector, {}, transient=False + "Inspector", inspector, {}, transient=True ) else: Inspector.assert_not_called() From 8370299eaef7c3be66ecca28e9ef73fe8930bcca Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Sat, 17 Dec 2022 20:39:43 +0200 Subject: [PATCH 10/12] Suggestions from Nestor --- sanic/cli/app.py | 6 +- sanic/cli/inspector_client.py | 119 ++++++++++++++++++++++++++++++++ sanic/worker/inspector.py | 120 +-------------------------------- tests/test_cli.py | 2 +- tests/worker/test_inspector.py | 3 +- 5 files changed, 128 insertions(+), 122 deletions(-) create mode 100644 sanic/cli/inspector_client.py diff --git a/sanic/cli/app.py b/sanic/cli/app.py index 23647d44b9..444de0f688 100644 --- a/sanic/cli/app.py +++ b/sanic/cli/app.py @@ -13,8 +13,8 @@ from sanic.cli.arguments import Group from sanic.cli.base import SanicArgumentParser, SanicHelpFormatter from sanic.cli.inspector import make_inspector_parser +from sanic.cli.inspector_client import InspectorClient from sanic.log import Colors, error_logger -from sanic.worker.inspector import InspectorClient from sanic.worker.loader import AppLoader @@ -85,7 +85,7 @@ def run(self, parse_args=None): elif parse_args == ["-v"]: parse_args = ["--version"] - if not legacy_version and not self.inspecting: + if not legacy_version: parsed, unknown = self.parser.parse_known_args(args=parse_args) if unknown and parsed.factory: for arg in unknown: @@ -149,7 +149,7 @@ def _inspector(self): for arg in unknown: if arg.startswith("--"): key, value = arg.split("=") - setattr(self.args, key.strip("-"), value) + setattr(self.args, key.lstrip("-"), value) kwargs = {**self.args.__dict__} host = kwargs.pop("host") diff --git a/sanic/cli/inspector_client.py b/sanic/cli/inspector_client.py new file mode 100644 index 0000000000..fd22bbd869 --- /dev/null +++ b/sanic/cli/inspector_client.py @@ -0,0 +1,119 @@ +from __future__ import annotations + +import sys + +from http.client import RemoteDisconnected +from textwrap import indent +from typing import Any, Dict, Optional +from urllib.error import URLError +from urllib.request import Request as URequest +from urllib.request import urlopen + +from sanic.application.logo import get_logo +from sanic.application.motd import MOTDTTY +from sanic.log import Colors + + +try: # no cov + from ujson import dumps, loads +except ModuleNotFoundError: # no cov + from json import dumps, loads # type: ignore + + +class InspectorClient: + def __init__( + self, + host: str, + port: int, + secure: bool, + raw: bool, + api_key: Optional[str], + ) -> None: + self.scheme = "https" if secure else "http" + self.host = host + self.port = port + self.raw = raw + self.api_key = api_key + + for scheme in ("http", "https"): + full = f"{scheme}://" + if self.host.startswith(full): + self.scheme = scheme + self.host = self.host[len(full) :] # noqa E203 + + def do(self, action: str, **kwargs: Any) -> None: + if action == "info": + self.info() + return + result = self.request(action, **kwargs).get("result") + if result: + out = ( + dumps(result) + if isinstance(result, (list, dict)) + else str(result) + ) + sys.stdout.write(out + "\n") + + def info(self) -> None: + out = sys.stdout.write + response = self.request("", "GET") + if self.raw or not response: + return + data = response["result"] + display = data.pop("info") + extra = display.pop("extra", {}) + display["packages"] = ", ".join(display["packages"]) + MOTDTTY(get_logo(), self.base_url, display, extra).display( + version=False, + action="Inspecting", + out=out, + ) + for name, info in data["workers"].items(): + info = "\n".join( + f"\t{key}: {Colors.BLUE}{value}{Colors.END}" + for key, value in info.items() + ) + out( + "\n" + + indent( + "\n".join( + [ + f"{Colors.BOLD}{Colors.SANIC}{name}{Colors.END}", + info, + ] + ), + " ", + ) + + "\n" + ) + + def request(self, action: str, method: str = "POST", **kwargs: Any) -> Any: + url = f"{self.base_url}/{action}" + params: Dict[str, Any] = {"method": method, "headers": {}} + if kwargs: + params["data"] = dumps(kwargs).encode() + params["headers"]["content-type"] = "application/json" + if self.api_key: + params["headers"]["authorization"] = f"Bearer {self.api_key}" + request = URequest(url, **params) + + try: + with urlopen(request) as response: # nosec B310 + raw = response.read() + loaded = loads(raw) + if self.raw: + sys.stdout.write(dumps(loaded.get("result")) + "\n") + return {} + return loaded + except (URLError, RemoteDisconnected) as e: + sys.stderr.write( + f"{Colors.RED}Could not connect to inspector at: " + f"{Colors.YELLOW}{self.base_url}{Colors.END}\n" + "Either the application is not running, or it did not start " + f"an inspector instance.\n{e}\n" + ) + sys.exit(1) + + @property + def base_url(self): + return f"{self.scheme}://{self.host}:{self.port}" diff --git a/sanic/worker/inspector.py b/sanic/worker/inspector.py index 333b599e0e..d60c5ae3e7 100644 --- a/sanic/worker/inspector.py +++ b/sanic/worker/inspector.py @@ -1,34 +1,19 @@ from __future__ import annotations -import sys - from datetime import datetime -from http.client import RemoteDisconnected from inspect import isawaitable from multiprocessing.connection import Connection from os import environ from pathlib import Path -from textwrap import indent -from typing import Any, Dict, Mapping, Optional, Union -from urllib.error import URLError -from urllib.request import Request as URequest -from urllib.request import urlopen - -from sanic.application.logo import get_logo -from sanic.application.motd import MOTDTTY +from typing import Any, Dict, Mapping, Union + from sanic.exceptions import Unauthorized from sanic.helpers import Default -from sanic.log import Colors, logger +from sanic.log import logger from sanic.request import Request from sanic.response import json -try: # no cov - from ujson import dumps, loads -except ModuleNotFoundError: # no cov - from json import dumps, loads # type: ignore - - class Inspector: def __init__( self, @@ -133,102 +118,3 @@ def scale(self, replicas) -> str: def shutdown(self) -> None: message = "__TERMINATE__" self._publisher.send(message) - - -class InspectorClient: - def __init__( - self, - host: str, - port: int, - secure: bool, - raw: bool, - api_key: Optional[str], - ) -> None: - self.scheme = "https" if secure else "http" - self.host = host - self.port = port - self.raw = raw - self.api_key = api_key - - for scheme in ("http", "https"): - full = f"{scheme}://" - if self.host.startswith(full): - self.scheme = scheme - self.host = self.host[len(full) :] # noqa E203 - - def do(self, action: str, **kwargs: Any) -> None: - if action == "info": - self.info() - return - result = self.request(action, **kwargs).get("result") - if result: - out = ( - dumps(result) - if isinstance(result, (list, dict)) - else str(result) - ) - sys.stdout.write(out + "\n") - - def info(self) -> None: - out = sys.stdout.write - response = self.request("", "GET") - if self.raw or not response: - return - data = response["result"] - display = data.pop("info") - extra = display.pop("extra", {}) - display["packages"] = ", ".join(display["packages"]) - MOTDTTY(get_logo(), self.base_url, display, extra).display( - version=False, - action="Inspecting", - out=out, - ) - for name, info in data["workers"].items(): - info = "\n".join( - f"\t{key}: {Colors.BLUE}{value}{Colors.END}" - for key, value in info.items() - ) - out( - "\n" - + indent( - "\n".join( - [ - f"{Colors.BOLD}{Colors.SANIC}{name}{Colors.END}", - info, - ] - ), - " ", - ) - + "\n" - ) - - def request(self, action: str, method: str = "POST", **kwargs: Any) -> Any: - url = f"{self.base_url}/{action}" - params: Dict[str, Any] = {"method": method, "headers": {}} - if kwargs: - params["data"] = dumps(kwargs).encode() - params["headers"]["content-type"] = "application/json" - if self.api_key: - params["headers"]["authorization"] = f"Bearer {self.api_key}" - request = URequest(url, **params) - - try: - with urlopen(request) as response: # nosec B310 - raw = response.read() - loaded = loads(raw) - if self.raw: - sys.stdout.write(dumps(loaded.get("result")) + "\n") - return {} - return loaded - except (URLError, RemoteDisconnected) as e: - sys.stderr.write( - f"{Colors.RED}Could not connect to inspector at: " - f"{Colors.YELLOW}{self.base_url}{Colors.END}\n" - "Either the application is not running, or it did not start " - f"an inspector instance.\n{e}\n" - ) - sys.exit(1) - - @property - def base_url(self): - return f"{self.scheme}://{self.host}:{self.port}" diff --git a/tests/test_cli.py b/tests/test_cli.py index f0636a3f5c..c8e0c79ba9 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -12,7 +12,7 @@ from sanic import __version__ from sanic.__main__ import main -from sanic.worker.inspector import InspectorClient +from sanic.cli.inspector_client import InspectorClient @pytest.fixture(scope="module", autouse=True) diff --git a/tests/worker/test_inspector.py b/tests/worker/test_inspector.py index 6fa36e1725..73d5c866d3 100644 --- a/tests/worker/test_inspector.py +++ b/tests/worker/test_inspector.py @@ -11,9 +11,10 @@ from sanic_testing import TestManager +from sanic.cli.inspector_client import InspectorClient from sanic.helpers import Default from sanic.log import Colors -from sanic.worker.inspector import Inspector, InspectorClient +from sanic.worker.inspector import Inspector DATA = { From 5ec6a9af027105272e2b3dd081db3a154b8edeab Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Sat, 17 Dec 2022 23:52:20 +0200 Subject: [PATCH 11/12] Update fixture mock location --- tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 223d393bbd..22082fdffc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -230,5 +230,5 @@ def urlopen(): urlopen.__enter__ = Mock(return_value=urlopen) urlopen.__exit__ = Mock() urlopen.read = Mock() - with patch("sanic.worker.inspector.urlopen", urlopen): + with patch("sanic.cli.inspector_client.urlopen", urlopen): yield urlopen From 7a128062b84818f744bb25da919067ca8658f0ef Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Sun, 18 Dec 2022 10:13:53 +0200 Subject: [PATCH 12/12] Change import path on test --- tests/worker/test_inspector.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/worker/test_inspector.py b/tests/worker/test_inspector.py index 73d5c866d3..bc9dd2d527 100644 --- a/tests/worker/test_inspector.py +++ b/tests/worker/test_inspector.py @@ -57,7 +57,7 @@ def http_client(inspector): @pytest.mark.parametrize("command", ("info",)) -@patch("sanic.worker.inspector.sys.stdout.write") +@patch("sanic.cli.inspector_client.sys.stdout.write") def test_send_inspect(write, urlopen, command: str): urlopen.read.return_value = FULL_SERIALIZED.encode() InspectorClient("localhost", 9999, False, False, None).do(command) @@ -67,7 +67,7 @@ def test_send_inspect(write, urlopen, command: str): write.assert_called_with(OUT_SERIALIZED + "\n") -@patch("sanic.worker.inspector.sys") +@patch("sanic.cli.inspector_client.sys") def test_send_inspect_conn_refused(sys: Mock, urlopen): urlopen.side_effect = URLError("") InspectorClient("localhost", 9999, False, False, None).do("info")