From c0437f4b695cc4423254a80292d3d80d5a0c6046 Mon Sep 17 00:00:00 2001 From: Vasek Sraier Date: Sun, 27 Jun 2021 15:30:28 +0200 Subject: [PATCH] api: implemented Caddy-style partial config updates (subtree specified in URL) --- manager/knot_resolver_manager/kres_manager.py | 13 ++- manager/knot_resolver_manager/server.py | 25 ++--- .../utils/dataclasses_parservalidator.py | 96 +++++++++++++++++-- manager/knot_resolver_manager/utils/types.py | 8 ++ manager/poetry.lock | 12 +-- manager/pyproject.toml | 3 + .../utils/test_dataclasses_parservalidator.py | 38 +++++++- 7 files changed, 156 insertions(+), 39 deletions(-) diff --git a/manager/knot_resolver_manager/kres_manager.py b/manager/knot_resolver_manager/kres_manager.py index 853d68860..017e612c8 100644 --- a/manager/knot_resolver_manager/kres_manager.py +++ b/manager/knot_resolver_manager/kres_manager.py @@ -1,5 +1,5 @@ import asyncio -from typing import Any, List, Type +from typing import Any, List, Optional, Type from uuid import uuid4 from knot_resolver_manager.constants import KRESD_CONFIG_FILE @@ -31,11 +31,12 @@ class KresManager: def __init__(self): self._children: List[Subprocess] = [] - self._children_lock = asyncio.Lock() + self._manager_lock = asyncio.Lock() self._controller: SubprocessController + self._last_used_config: Optional[KresConfig] = None async def load_system_state(self): - async with self._children_lock: + async with self._manager_lock: await self._collect_already_running_children() async def _spawn_new_child(self): @@ -72,10 +73,14 @@ class KresManager: await writefile(KRESD_CONFIG_FILE, lua_config) async def apply_config(self, config: KresConfig): - async with self._children_lock: + async with self._manager_lock: await self._write_config(config) + self._last_used_config = config await self._ensure_number_of_children(config.server.get_instances()) await self._rolling_restart() async def stop(self): await self._ensure_number_of_children(0) + + def get_last_used_config(self) -> Optional[KresConfig]: + return self._last_used_config diff --git a/manager/knot_resolver_manager/server.py b/manager/knot_resolver_manager/server.py index 9f82634d0..e5173df19 100644 --- a/manager/knot_resolver_manager/server.py +++ b/manager/knot_resolver_manager/server.py @@ -11,7 +11,7 @@ from aiohttp.web import middleware from knot_resolver_manager.constants import MANAGER_CONFIG_FILE from knot_resolver_manager.utils.async_utils import readfile -from knot_resolver_manager.utils.dataclasses_parservalidator import ValidationException +from knot_resolver_manager.utils.dataclasses_parservalidator import Format, ValidationException from .datamodel import KresConfig from .kres_manager import KresManager @@ -34,6 +34,8 @@ async def _apply_config(request: web.Request) -> web.Response: Route handler for changing resolver configuration """ + document_path = request.match_info["path"] + manager: KresManager = get_kres_manager(request.app) if manager is None: # handle the case when the manager is not yet initialized @@ -42,22 +44,9 @@ async def _apply_config(request: web.Request) -> web.Response: ) # parse the incoming data - - # JSON or not-set - # - # aiohttp docs https://docs.aiohttp.org/en/stable/web_reference.html#aiohttp.web.BaseRequest.content_type: - # - # "Returns value is 'application/octet-stream' if no Content-Type header present in HTTP headers according to - # RFC 2616" - if request.content_type == "application/json" or request.content_type == "application/octet-stream": - config = KresConfig.from_json(await request.text()) - elif "yaml" in request.content_type: - config = KresConfig.from_yaml(await request.text()) - else: - return web.Response( - text="Unsupported content-type header. Use application/json or text/x-yaml", - status=HTTPStatus.BAD_REQUEST, - ) + last: KresConfig = manager.get_last_used_config() or KresConfig() + fmt = Format.from_mime_type(request.content_type) + config = last.copy_with_changed_subtree(fmt, document_path, await request.text()) # apply config await manager.apply_config(config) @@ -92,7 +81,7 @@ async def error_handler(request: web.Request, handler: Any): def setup_routes(app: web.Application): - app.add_routes([web.get("/", _index), web.post("/config", _apply_config), web.post("/stop", _stop)]) + app.add_routes([web.get("/", _index), web.post(r"/config{path:.*}", _apply_config), web.post("/stop", _stop)]) def stop_server(app: web.Application): diff --git a/manager/knot_resolver_manager/utils/dataclasses_parservalidator.py b/manager/knot_resolver_manager/utils/dataclasses_parservalidator.py index ab6256882..4b6c01571 100644 --- a/manager/knot_resolver_manager/utils/dataclasses_parservalidator.py +++ b/manager/knot_resolver_manager/utils/dataclasses_parservalidator.py @@ -1,4 +1,7 @@ +import copy import json +import re +from enum import Enum, auto from typing import Any, Dict, List, Optional, Tuple, Type, TypeVar, Union import yaml @@ -6,6 +9,7 @@ from yaml.constructor import ConstructorError from yaml.nodes import MappingNode from knot_resolver_manager.utils.types import ( + get_attr_type, get_generic_type_argument, get_generic_type_arguments, is_dict, @@ -188,6 +192,35 @@ class RaiseDuplicatesLoader(yaml.SafeLoader): _T = TypeVar("_T", bound="DataclassParserValidatorMixin") +_SUBTREE_MUTATION_PATH_PATTERN = re.compile(r"^(/[^/]+)*/?$") + + +class Format(Enum): + YAML = auto() + JSON = auto() + + def parse_to_dict(self, text: str) -> Any: + if self is Format.YAML: + # RaiseDuplicatesLoader extends yaml.SafeLoader, so this should be safe + # https://python.land/data-processing/python-yaml#PyYAML_safe_load_vs_load + return yaml.load(text, Loader=RaiseDuplicatesLoader) # type: ignore + elif self is Format.JSON: + return json.loads(text, object_pairs_hook=json_raise_duplicates) + else: + raise NotImplementedError(f"Parsing of format '{self}' is not implemented") + + @staticmethod + def from_mime_type(mime_type: str) -> "Format": + formats = { + "application/json": Format.JSON, + "application/octet-stream": Format.JSON, # default in aiohttp + "text/vnd.yaml": Format.YAML, + } + if mime_type not in formats: + raise ValidationException("Unsupported MIME type") + return formats[mime_type] + + class DataclassParserValidatorMixin: def __init__(self, *args: Any, **kwargs: Any): """ @@ -215,17 +248,60 @@ class DataclassParserValidatorMixin: raise NotImplementedError(f"Validation function is not implemented in class {type(self).__name__}") @classmethod - def from_yaml(cls: Type[_T], text: str, default: _T = ..., use_default: bool = False) -> _T: - # RaiseDuplicatesLoader extends yaml.SafeLoader, so this should be safe - # https://python.land/data-processing/python-yaml#PyYAML_safe_load_vs_load - data = yaml.load(text, Loader=RaiseDuplicatesLoader) # type: ignore - config: _T = _from_dictlike_obj(cls, data, default, use_default) + def parse_from(cls: Type[_T], fmt: Format, text: str): + data = fmt.parse_to_dict(text) + config: _T = _from_dictlike_obj(cls, data, ..., False) config.validate() return config @classmethod - def from_json(cls: Type[_T], text: str, default: _T = ..., use_default: bool = False) -> _T: - data = json.loads(text, object_pairs_hook=json_raise_duplicates) - config: _T = _from_dictlike_obj(cls, data, default, use_default) - config.validate() - return config + def from_yaml(cls: Type[_T], text: str) -> _T: + return cls.parse_from(Format.YAML, text) + + @classmethod + def from_json(cls: Type[_T], text: str) -> _T: + return cls.parse_from(Format.JSON, text) + + def copy_with_changed_subtree(self: _T, fmt: Format, path: str, text: str) -> _T: + cls = self.__class__ + + # prepare and validate the path object + path = path[:-1] if path.endswith("/") else path + if re.match(_SUBTREE_MUTATION_PATH_PATTERN, path) is None: + raise ValidationException("Provided object path for mutation is invalid.") + path = path[1:] if path.startswith("/") else path + + # now, the path variable should contain '/' separated field names + + # check if we should mutate whole object + if path == "": + return cls.parse_from(fmt, text) + + # find the subtree we will replace in a copy of the original object + to_mutate = copy.deepcopy(self) + obj = to_mutate + parent = None + for segment in path.split("/"): + if segment == "": + raise ValidationException(f"Unexpectedly empty segment in path '{path}'") + elif segment.startswith("_"): + raise ValidationException("No, changing internal fields (starting with _) is not allowed. Nice try.") + elif hasattr(obj, segment): + parent = obj + obj = getattr(parent, segment) + else: + raise ValidationException( + f"Path segment '{segment}' does not match any field on the provided parent object" + ) + assert parent is not None + + # assign the subtree + last_name = path.split("/")[-1] + data = fmt.parse_to_dict(text) + tp = get_attr_type(parent, last_name) + parsed_value = _from_dictlike_obj(tp, data, ..., False) + setattr(parent, last_name, parsed_value) + + to_mutate.validate() + + return to_mutate diff --git a/manager/knot_resolver_manager/utils/types.py b/manager/knot_resolver_manager/utils/types.py index 0929f1903..fb2246708 100644 --- a/manager/knot_resolver_manager/utils/types.py +++ b/manager/knot_resolver_manager/utils/types.py @@ -50,6 +50,14 @@ def is_none_type(tp: Any) -> bool: return tp is None or tp == NoneType +def get_attr_type(obj: Any, attr_name: str) -> Any: + assert hasattr(obj, attr_name) + assert hasattr(obj, "__annotations__") + annot = getattr(type(obj), "__annotations__") + assert attr_name in annot + return annot[attr_name] + + class _LiteralEnum: def __getitem__(self, args: Tuple[Union[str, int, bytes], ...]) -> Any: lits = tuple(Literal[x] for x in args) diff --git a/manager/poetry.lock b/manager/poetry.lock index ada0b9a60..cf7fe1c3f 100644 --- a/manager/poetry.lock +++ b/manager/poetry.lock @@ -28,7 +28,7 @@ python-versions = "*" [[package]] name = "apkg" -version = "0.1.1.dev1+g08244e6" +version = "0.1.1.dev4+g8adc3eb" description = "cross-distro upstream packaging automation tool" category = "dev" optional = false @@ -50,7 +50,7 @@ toml = "*" type = "git" url = "https://gitlab.nic.cz/packaging/apkg.git" reference = "master" -resolved_reference = "08244e6e0b5842931a5ab27ba976c50e66d887d7" +resolved_reference = "8adc3eb13d73dccf535c46e54aba632ffe20dbd8" [[package]] name = "appdirs" @@ -191,7 +191,7 @@ python-versions = "*" name = "certifi" version = "2020.12.5" description = "Python package for providing Mozilla's CA Bundle." -category = "dev" +category = "main" optional = false python-versions = "*" @@ -852,7 +852,7 @@ python-versions = "*" name = "requests" version = "2.25.1" description = "Python HTTP for Humans." -category = "dev" +category = "main" optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*" @@ -1162,7 +1162,7 @@ python-versions = "*" name = "urllib3" version = "1.26.4" description = "HTTP library with thread-safe connection pooling, file post, and more." -category = "dev" +category = "main" optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, <4" @@ -1243,7 +1243,7 @@ testing = ["pytest (>=4.6)", "pytest-checkdocs (>=1.2.3)", "pytest-flake8", "pyt [metadata] lock-version = "1.1" python-versions = "^3.6.12" -content-hash = "32f7e392d56071f55fff1d440f2c90327b26caa637e91d93601dce19fcd48a8e" +content-hash = "d9de1979aaeaee5d2774e7dbf67436c73f6ef2e3080ccb64c0c382f38cee5a1e" [metadata.files] aiohttp = [ diff --git a/manager/pyproject.toml b/manager/pyproject.toml index 1aaea3791..0f5a0158f 100644 --- a/manager/pyproject.toml +++ b/manager/pyproject.toml @@ -16,6 +16,7 @@ PyGObject = "^3.38.0" Jinja2 = "^2.11.3" click = "^7.1.2" PyYAML = "^5.4.1" +requests = "^2.25.1" [tool.poetry.dev-dependencies] pytest = "^5.2" @@ -45,6 +46,7 @@ format = { shell = "poetry run black knot_resolver_manager/ tests/; isort -rc ." fixdeps = { shell = "poetry install; npm install; npm update", help = "Install/update dependencies according to configuration files"} commit = { shell = "scripts/commit", help = "Invoke every single check before commiting" } container = { cmd = "scripts/container.py", help = "Manage containers" } +client = { script = "knot_resolver_manager.client.__main__:main", help="Run Managers API client CLI" } clean = """ rm -rf .coverage .mypy_cache @@ -107,6 +109,7 @@ disable= [ "consider-using-in", # pyright can't see through in expressions, "too-many-return-statements", # would prevent us from using recursive tree traversals "logging-fstring-interpolation", # see https://github.com/PyCQA/pylint/issues/1788 + "no-else-raise", # not helpful for readability, when we want explicit branches ] [tool.pylint.SIMILARITIES] diff --git a/manager/tests/utils/test_dataclasses_parservalidator.py b/manager/tests/utils/test_dataclasses_parservalidator.py index ed9798802..277133087 100644 --- a/manager/tests/utils/test_dataclasses_parservalidator.py +++ b/manager/tests/utils/test_dataclasses_parservalidator.py @@ -1,7 +1,7 @@ from dataclasses import dataclass from typing import Dict, List, Optional, Tuple -from knot_resolver_manager.utils.dataclasses_parservalidator import DataclassParserValidatorMixin +from knot_resolver_manager.utils.dataclasses_parservalidator import DataclassParserValidatorMixin, Format def test_parsing_primitive(): @@ -132,3 +132,39 @@ lua_config: | assert data.num_workers == 4 assert type(data.lua_config) == str assert data.lua_config == "dummy" + + +def test_partial_mutations(): + @dataclass + class Inner(DataclassParserValidatorMixin): + number: int + + def _validate(self): + pass + + @dataclass + class ConfData(DataclassParserValidatorMixin): + num_workers: int = 1 + lua_config: Optional[str] = None + inner: Inner = Inner(5) + + def _validate(self): + if self.num_workers < 0: + raise Exception("Number of workers must be non-negative") + + data = ConfData(5, "something", Inner(10)) + + x = data.copy_with_changed_subtree(Format.JSON, "/lua_config", '"new_value"') + assert x.lua_config == "new_value" + assert x.num_workers == 5 + assert x.inner.number == 10 + + x = data.copy_with_changed_subtree(Format.JSON, "/inner", '{"number": 55}') + assert x.lua_config == "something" + assert x.num_workers == 5 + assert x.inner.number == 55 + + x = data.copy_with_changed_subtree(Format.JSON, "/", '{"inner": {"number": 55}}') + assert x.lua_config is None + assert x.num_workers == 1 + assert x.inner.number == 55 -- 2.47.3