http api improvements and tests

- adds versions to URL's (closes #759)
- adds a basic packaging test for consistent etags
- adds a basic packaging test for working metrics
- fix bugs with API implementation
- fix bugs with /metrics endpoint

diff --git a/distro/tests/manager-packaging/control b/distro/tests/manager-packaging/control
index c820dbe61b78b22ab354bd1ff0ac1c73a2ec83d9..f70a0b33a182816566dd94fb9b963e06230d761b 100644 (file)
--- a/distro/tests/manager-packaging/control
+++ b/distro/tests/manager-packaging/control
@@ -13,17 +13,17 @@ Tests: systemd_service.sh
 Tests-Directory: manager/tests/packaging
 Restrictions: needs-root
 {% if distro.match('fedora') -%}
-Depends: knot-utils
+Depends: knot-utils, jq, curl
 {% elif distro.match('debian') or distro.match('ubuntu') -%}
-Depends: knot-dnsutils
+Depends: knot-dnsutils, jq, curl
 {% elif distro.match('arch') -%}
-Depends: knot
+Depends: knot, jq, curl
 {% elif distro.match('opensuse') -%}
-Depends: knot-utils
+Depends: knot-utils, jq, curl
 {% elif distro.match('rocky') -%}
-Depends: knot-utils
+Depends: knot-utils, jq, curl
 {% elif distro.match('centos') -%}
-Depends: knot-utils
+Depends: knot-utils, jq, curl
 {% else -%}
 Depends: unsupported-distro-this-package-does-not-exist-and-the-test-should-fail
 {%- endif %}

diff --git a/manager/knot-resolver.service b/manager/knot-resolver.service
index 8295f3668406590cd8e304061ba09cdf2913ae5d..00be7d48e3872346000e34209c49816701e4cbbd 100644 (file)
--- a/manager/knot-resolver.service
+++ b/manager/knot-resolver.service
@@ -6,6 +6,7 @@ Type=notify
 TimeoutStartSec=10s
 ExecStart=/usr/bin/env python3 -m knot_resolver_manager --config=/etc/knot-resolver/config.yml
 KillSignal=SIGINT
+WorkingDirectory=/var/run/knot-resolver/
 # See systemd.service(5) for explanation, why we should replace this with a blocking request
 # ExecReload=/usr/bin/env kill -HUP $MAINPID
 

diff --git a/manager/knot_resolver_manager/kresd_controller/interface.py b/manager/knot_resolver_manager/kresd_controller/interface.py
index 3db894fcb611ad2be09e08c2c0b97f273b42fe7a..081c3221224daae4eaca126aafef65bddd8cad76 100644 (file)
--- a/manager/knot_resolver_manager/kresd_controller/interface.py
+++ b/manager/knot_resolver_manager/kresd_controller/interface.py
@@ -163,7 +163,7 @@ class Subprocess:
         reader: asyncio.StreamReader
         writer: Optional[asyncio.StreamWriter] = None
         try:
-            reader, writer = await asyncio.open_unix_connection(f"./control/{self.id}")
+            reader, writer = await asyncio.open_unix_connection(f"./control/{int(self.id)}")
 
             # drop prompt
             _ = await reader.read(2)

diff --git a/manager/knot_resolver_manager/server.py b/manager/knot_resolver_manager/server.py
index 38851a456a711a16550182f9ef54a3d78c3b87f8..edcbda728972705fab8d0df3027164a50ec14e7b 100644 (file)
--- a/manager/knot_resolver_manager/server.py
+++ b/manager/knot_resolver_manager/server.py
@@ -7,9 +7,9 @@ import sys
 from http import HTTPStatus
 from pathlib import Path
 from time import time
-from typing import Any, Optional, Set, Union, cast
+from typing import Any, List, Optional, Set, Union, cast
 
-from aiohttp import ETag, web
+from aiohttp import web
 from aiohttp.web import middleware
 from aiohttp.web_app import Application
 from aiohttp.web_response import json_response
@@ -25,6 +25,7 @@ from knot_resolver_manager.datamodel.config_schema import KresConfig
 from knot_resolver_manager.datamodel.management_schema import ManagementSchema
 from knot_resolver_manager.exceptions import CancelStartupExecInsteadException, KresManagerException
 from knot_resolver_manager.kresd_controller import get_best_controller_implementation
+from knot_resolver_manager.utils import ignore_exceptions_optional
 from knot_resolver_manager.utils.async_utils import readfile
 from knot_resolver_manager.utils.functional import Result
 from knot_resolver_manager.utils.modeling import ParsedTree, parse, parse_yaml
@@ -165,28 +166,45 @@ class Server:
         """
         Route handler for changing resolver configuration
         """
+        # There are a lot of local variables in here, but they are usually immutable (almost SSA form :) )
+        # pylint: disable=too-many-locals
 
         # parse the incoming data
         document_path = request.match_info["path"]
-        etags = request.if_match
-        last: ParsedTree = self.config_store.get().get_unparsed_data()
-        update_with: ParsedTree = parse(await request.text(), request.content_type)
+        getheaders = ignore_exceptions_optional(List[str], None, KeyError)(request.headers.getall)
+        etags = getheaders("if-match")
+        not_etags = getheaders("if-none-match")
+        current_config: ParsedTree = self.config_store.get().get_unparsed_data()
+        if request.method == "GET":
+            update_with: Optional[ParsedTree] = None
+        else:
+            update_with = parse(await request.text(), request.content_type)
 
-        if etags is not None and last.etag not in map(str, etags):
-            return web.Response(status=HTTPStatus.PRECONDITION_FAILED)
+        # stop processing if etags
+        def strip_quotes(s: str) -> str:
+            return s.strip('"')
 
-        op = cast(Literal["get", "post", "delete", "patch", "put"], request.method.lower())
-        root, to_return = last.query(op, document_path, update_with)
+        status = HTTPStatus.NOT_MODIFIED if request.method in ("GET", "HEAD") else HTTPStatus.PRECONDITION_FAILED
+        if etags is not None and current_config.etag not in map(strip_quotes, etags):
+            return web.Response(status=status)
+        if not_etags is not None and current_config.etag in map(strip_quotes, not_etags):
+            return web.Response(status=status)
 
-        # validate config
-        config_validated = KresConfig(root)
+        # run query
+        op = cast(Literal["get", "post", "delete", "patch", "put"], request.method.lower())
+        new_config, to_return = current_config.query(op, document_path, update_with)
 
-        # apply config
-        await self.config_store.update(config_validated)
+        # update the config
+        if request.method != "GET":
+            # validate
+            config_validated = KresConfig(new_config)
+            # apply
+            await self.config_store.update(config_validated)
 
         # return success
-        res = web.Response(status=HTTPStatus.OK, text=str(to_return))
-        res.etag = ETag(config_validated.get_unparsed_data().etag)
+        resp_text: Optional[str] = str(to_return) if to_return is not None else None
+        res = web.Response(status=HTTPStatus.OK, text=resp_text)
+        res.headers.add("ETag", f'"{new_config.etag}"')
         return res
 
     async def _handler_metrics(self, _request: web.Request) -> web.Response:
@@ -241,11 +259,11 @@ class Server:
         self.app.add_routes(
             [
                 web.get("/", self._handler_index),
-                web.post(r"/config{path:.*}", self._handler_config_query),
-                web.put(r"/config{path:.*}", self._handler_config_query),
-                web.patch(r"/config{path:.*}", self._handler_config_query),
-                web.get(r"/config{path:.*}", self._handler_config_query),
-                web.delete(r"/config{path:.*}", self._handler_config_query),
+                web.post(r"/v1/config{path:.*}", self._handler_config_query),
+                web.put(r"/v1/config{path:.*}", self._handler_config_query),
+                web.patch(r"/v1/config{path:.*}", self._handler_config_query),
+                web.get(r"/v1/config{path:.*}", self._handler_config_query),
+                web.delete(r"/v1/config{path:.*}", self._handler_config_query),
                 web.post("/stop", self._handler_stop),
                 web.get("/schema", self._handler_schema),
                 web.get("/schema/ui", self._handle_view_schema),

diff --git a/manager/knot_resolver_manager/utils/modeling/parsing.py b/manager/knot_resolver_manager/utils/modeling/parsing.py
index ec14809ca98ca76e442a68bfeb758e854bbb483e..eb26c851ee60a3ade31213d70543bdc65fee3282 100644 (file)
--- a/manager/knot_resolver_manager/utils/modeling/parsing.py
+++ b/manager/knot_resolver_manager/utils/modeling/parsing.py
@@ -68,7 +68,7 @@ class ParsedTree:
 
     @property
     def etag(self) -> str:
-        m = blake2b(digest_size=9)
+        m = blake2b(digest_size=15)
         m.update(json.dumps(self._data, sort_keys=True).encode("utf8"))
         return base64.urlsafe_b64encode(m.digest()).decode("utf8")
 

diff --git a/manager/tests/packaging/interactive/etag.sh b/manager/tests/packaging/interactive/etag.sh
new file mode 100755 (executable)
index 0000000..a4c49ed
--- /dev/null
+++ b/manager/tests/packaging/interactive/etag.sh
@@ -0,0 +1,10 @@
+#!/bin/bash
+
+set -e
+
+socket_opt="--unix-socket /var/run/knot-resolver/manager.sock"
+
+etag="$(curl --silent $socket_opt --fail http://localhost:5000/v1/config -o /dev/null -v 2>&1 | grep ETag | sed 's/< ETag: //;s/\s//')"
+status=$(curl --silent $socket_opt --fail  http://localhost:5000/v1/config --header "If-None-Match: $etag" -w "%{http_code}" -o /dev/null)
+
+test "$status" -eq 304

diff --git a/manager/tests/packaging/interactive/metrics.sh b/manager/tests/packaging/interactive/metrics.sh
new file mode 100755 (executable)
index 0000000..a3e8748
--- /dev/null
+++ b/manager/tests/packaging/interactive/metrics.sh
@@ -0,0 +1,3 @@
+#!/bin/bash
+
+curl --silent --fail --unix-socket /var/run/knot-resolver/manager.sock http://localhost/metrics > /dev/null
\ No newline at end of file

diff --git a/manager/tests/packaging/systemd_service.sh b/manager/tests/packaging/systemd_service.sh
index e942b7075be4e876e9eb500f315624bfca76690b..99835eedbeb3400733a54d6b6f07b11f464fd2ef 100755 (executable)
--- a/manager/tests/packaging/systemd_service.sh
+++ b/manager/tests/packaging/systemd_service.sh
@@ -22,5 +22,11 @@ if ! systemctl start knot-resolver.service; then
 else
        # check that the resolvers are actually running
        kdig @127.0.0.1 nic.cz
+
+       echo "Running interactive tests..."
+       for test in "$(dirname $0)"/interactive/*; do
+               echo "[test] $test"
+               $test
+       done
 fi