Merge branch 'tls-cert-files-watchdog-config' into 'master'

manager: datamodel: 'files-watchdog' config for TLS certificate files

See merge request knot/knot-resolver!1645

11 files changed, 163 insertions, 59 deletions

diff --git a/NEWS b/NEWS
index d6dd1652..d7f2cf53 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,7 @@ Improvements
 - avoid multiple log lines when IPv6 isn't available (!1633)
 - manager: fix startup on Linux without libsystemd (!1608)
 - auto-reload TLS certificate files (!1626)
+  - Can be configured using the ``/network/tls/files-watchdog`` option. (!1645)
 - reload TLS certificate files even if the configuration has not changed (!1644)
 - kresctl: bash command-line TAB completion (!1622)
 - add request prioritization (defer) to mitigate DoS attacks (!1641)
diff --git a/doc/_static/config.schema.json b/doc/_static/config.schema.json
index 1aa80cf9..9c214885 100644
--- a/doc/_static/config.schema.json
+++ b/doc/_static/config.schema.json
@@ -305,6 +305,21 @@
                     "description": "TLS configuration, also affects DNS over TLS and DNS over HTTPS.",
                     "type": "object",
                     "properties": {
+                        "files-watchdog": {
+                            "anyOf": [
+                                {
+                                    "type": "string",
+                                    "enum": [
+                                        "auto"
+                                    ]
+                                },
+                                {
+                                    "type": "boolean"
+                                }
+                            ],
+                            "description": "Enables files watchdog for TLS certificate files. Requires the optional 'watchdog' dependency.",
+                            "default": "auto"
+                        },
                         "cert-file": {
                             "type": [
                                 "string",
@@ -359,6 +374,7 @@
                         }
                     },
                     "default": {
+                        "files_watchdog": true,
                         "cert_file": null,
                         "key_file": null,
                         "sticket_secret": null,
@@ -517,6 +533,7 @@
                 },
                 "address_renumbering": null,
                 "tls": {
+                    "files_watchdog": true,
                     "cert_file": null,
                     "key_file": null,
                     "sticket_secret": null,
diff --git a/doc/user/config-network-server-tls.rst b/doc/user/config-network-server-tls.rst
index 8fc84878..ea63482f 100644
--- a/doc/user/config-network-server-tls.rst
+++ b/doc/user/config-network-server-tls.rst
@@ -128,14 +128,22 @@ policies.
           cert-file: /etc/knot-resolver/server-cert.pem
           key-file: /etc/knot-resolver/server-key.pem
 
-   .. tip::
+   .. option:: files-watchdog: auto|true|false
 
-      If you have ``python-watchdog`` installed on your system,
+      :default: auto
+
+      By default, if you have ``python-watchdog`` installed on your system,
       the certificate files are automatically reloaded on change.
-      If you update the certificate files, e.g. using ACME,
-      the manager is notified about changes and commands all workers
-      to reload their certificate files. If you don't have ``python-watchdog``,
-      you have to restart the ``knot-resolver`` service manually.
+      When you update the certificate files, e.g. using ACME,
+      the manager is notified of the changes and commands all workers
+      to reload their certificate files.
+
+      If you don't have ``python-watchdog`` installed, this feature is not available
+      and you will have to restart the ``knot-resolver`` service manually.
+
+      You can also manually enable (``true``) and disable (``false``) this feature in the config,
+      but if it is enabled and ``python-watchdog`` is not installed,
+      the resolver will fail to start with a configuration validation error.
 
    .. option:: sticket-secret: <str>
 
diff --git a/python/knot_resolver/constants.py b/python/knot_resolver/constants.py
index f37bb2af..b4cfa59e 100644
--- a/python/knot_resolver/constants.py
+++ b/python/knot_resolver/constants.py
@@ -1,3 +1,4 @@
+import importlib.util
 from pathlib import Path
 
 VERSION = "6.0.9"
@@ -17,3 +18,11 @@ API_SOCK_FILE = RUN_DIR / "kres-api.sock"
 # executables paths
 KRESD_EXECUTABLE = SBIN_DIR / "kresd"
 KRES_CACHE_GC_EXECUTABLE = SBIN_DIR / "kres-cache-gc"
+
+WATCHDOG_LIB = False
+if importlib.util.find_spec("watchdog"):
+    WATCHDOG_LIB = True
+
+PROMETHEUS_LIB = False
+if importlib.util.find_spec("prometheus_client"):
+    PROMETHEUS_LIB = True
diff --git a/python/knot_resolver/constants.py.in b/python/knot_resolver/constants.py.in
index 0f1c3a88..35c25ac6 100644
--- a/python/knot_resolver/constants.py.in
+++ b/python/knot_resolver/constants.py.in
@@ -1,3 +1,4 @@
+import importlib.util
 from pathlib import Path
 
 VERSION = "@version@"
@@ -17,3 +18,11 @@ API_SOCK_FILE = RUN_DIR / "kres-api.sock"
 # executables paths
 KRESD_EXECUTABLE = SBIN_DIR / "kresd"
 KRES_CACHE_GC_EXECUTABLE = SBIN_DIR / "kres-cache-gc"
+
+WATCHDOG_LIB = False
+if importlib.util.find_spec("watchdog"):
+    WATCHDOG_LIB = True
+
+PROMETHEUS_LIB = False
+if importlib.util.find_spec("prometheus_client"):
+    PROMETHEUS_LIB = True
diff --git a/python/knot_resolver/datamodel/network_schema.py b/python/knot_resolver/datamodel/network_schema.py
index a71c006b..e2753a85 100644
--- a/python/knot_resolver/datamodel/network_schema.py
+++ b/python/knot_resolver/datamodel/network_schema.py
@@ -1,5 +1,6 @@
-from typing import List, Literal, Optional, Union
+from typing import Any, List, Literal, Optional, Union
 
+from knot_resolver.constants import WATCHDOG_LIB
 from knot_resolver.datamodel.types import (
     EscapedStr32B,
     Int0_512,
@@ -48,18 +49,31 @@ class AddressRenumberingSchema(ConfigSchema):
 
 
 class TLSSchema(ConfigSchema):
-    """
-    TLS configuration, also affects DNS over TLS and DNS over HTTPS.
+    class Raw(ConfigSchema):
+        """
+        TLS configuration, also affects DNS over TLS and DNS over HTTPS.
 
-    ---
-    cert_file: Path to certificate file.
-    key_file: Path to certificate key file.
-    sticket_secret: Secret for TLS session resumption via tickets. (RFC 5077).
-    sticket_secret_file: Path to file with secret for TLS session resumption via tickets. (RFC 5077).
-    auto_discovery: Experimental automatic discovery of authoritative servers supporting DNS-over-TLS.
-    padding: EDNS(0) padding of queries and answers sent over an encrypted channel.
-    """
+        ---
+        files_watchdog: Enables files watchdog for TLS certificate files. Requires the optional 'watchdog' dependency.
+        cert_file: Path to certificate file.
+        key_file: Path to certificate key file.
+        sticket_secret: Secret for TLS session resumption via tickets. (RFC 5077).
+        sticket_secret_file: Path to file with secret for TLS session resumption via tickets. (RFC 5077).
+        auto_discovery: Experimental automatic discovery of authoritative servers supporting DNS-over-TLS.
+        padding: EDNS(0) padding of queries and answers sent over an encrypted channel.
+        """
 
+        files_watchdog: Union[Literal["auto"], bool] = "auto"
+        cert_file: Optional[ReadableFile] = None
+        key_file: Optional[ReadableFile] = None
+        sticket_secret: Optional[EscapedStr32B] = None
+        sticket_secret_file: Optional[ReadableFile] = None
+        auto_discovery: bool = False
+        padding: Union[bool, Int0_512] = True
+
+    _LAYER = Raw
+
+    files_watchdog: bool
     cert_file: Optional[ReadableFile] = None
     key_file: Optional[ReadableFile] = None
     sticket_secret: Optional[EscapedStr32B] = None
@@ -67,9 +81,20 @@ class TLSSchema(ConfigSchema):
     auto_discovery: bool = False
     padding: Union[bool, Int0_512] = True
 
+    def _files_watchdog(self, obj: Raw) -> Any:
+        if obj.files_watchdog == "auto":
+            return WATCHDOG_LIB
+        return obj.files_watchdog
+
     def _validate(self):
         if self.sticket_secret and self.sticket_secret_file:
             raise ValueError("'sticket_secret' and 'sticket_secret_file' are both defined, only one can be used")
+        if bool(self.cert_file) != bool(self.key_file):
+            raise ValueError("'cert-file' and 'key-file' must be configured together")
+        if self.cert_file and self.key_file and self.files_watchdog and not WATCHDOG_LIB:
+            raise ValueError(
+                "'files-watchdog' is enabled, but the required 'watchdog' dependency (optional) is not installed"
+            )
 
 
 class ListenSchema(ConfigSchema):
diff --git a/python/knot_resolver/datamodel/types/files.py b/python/knot_resolver/datamodel/types/files.py
index 2d22d075..9e326999 100644
--- a/python/knot_resolver/datamodel/types/files.py
+++ b/python/knot_resolver/datamodel/types/files.py
@@ -89,9 +89,12 @@ class Dir(UncheckedPath):
     def __init__(
         self, source_value: Any, parents: Tuple["UncheckedPath", ...] = tuple(), object_path: str = "/"
     ) -> None:
-        super().__init__(source_value, parents=parents, object_path=object_path)
-        if self.strict_validation and not self._value.is_dir():
-            raise ValueError(f"path '{self._value}' does not point to an existing directory")
+        try:
+            super().__init__(source_value, parents=parents, object_path=object_path)
+            if self.strict_validation and not self._value.is_dir():
+                raise ValueError(f"path '{self._value}' does not point to an existing directory")
+        except PermissionError as e:
+            raise ValueError(str(e)) from e
 
 
 class AbsoluteDir(Dir):
@@ -118,11 +121,14 @@ class File(UncheckedPath):
     def __init__(
         self, source_value: Any, parents: Tuple["UncheckedPath", ...] = tuple(), object_path: str = "/"
     ) -> None:
-        super().__init__(source_value, parents=parents, object_path=object_path)
-        if self.strict_validation and not self._value.exists():
-            raise ValueError(f"file '{self._value}' does not exist")
-        if self.strict_validation and not self._value.is_file():
-            raise ValueError(f"path '{self._value}' is not a file")
+        try:
+            super().__init__(source_value, parents=parents, object_path=object_path)
+            if self.strict_validation and not self._value.exists():
+                raise ValueError(f"file '{self._value}' does not exist")
+            if self.strict_validation and not self._value.is_file():
+                raise ValueError(f"path '{self._value}' is not a file")
+        except PermissionError as e:
+            raise ValueError(str(e)) from e
 
 
 class FilePath(UncheckedPath):
@@ -135,13 +141,15 @@ class FilePath(UncheckedPath):
     def __init__(
         self, source_value: Any, parents: Tuple["UncheckedPath", ...] = tuple(), object_path: str = "/"
     ) -> None:
-        super().__init__(source_value, parents=parents, object_path=object_path)
-        p = self._value.parent
-        if self.strict_validation and (not p.exists() or not p.is_dir()):
-            raise ValueError(f"path '{self._value}' does not point inside an existing directory")
-
-        if self.strict_validation and self._value.is_dir():
-            raise ValueError(f"path '{self._value}' points to a directory when we expected a file")
+        try:
+            super().__init__(source_value, parents=parents, object_path=object_path)
+            p = self._value.parent
+            if self.strict_validation and (not p.exists() or not p.is_dir()):
+                raise ValueError(f"path '{self._value}' does not point inside an existing directory")
+            if self.strict_validation and self._value.is_dir():
+                raise ValueError(f"path '{self._value}' points to a directory when we expected a file")
+        except PermissionError as e:
+            raise ValueError(str(e)) from e
 
 
 class _PermissionMode(Flag):
diff --git a/python/knot_resolver/manager/files/watchdog.py b/python/knot_resolver/manager/files/watchdog.py
index 64547192..e0abf56c 100644
--- a/python/knot_resolver/manager/files/watchdog.py
+++ b/python/knot_resolver/manager/files/watchdog.py
@@ -1,31 +1,26 @@
-import importlib
 import logging
 from pathlib import Path
 from threading import Timer
-from typing import List, Optional
+from typing import Any, List, Optional
 
+from knot_resolver.constants import WATCHDOG_LIB
 from knot_resolver.controller.registered_workers import command_registered_workers
 from knot_resolver.datamodel import KresConfig
-from knot_resolver.datamodel.types import File
 from knot_resolver.manager.config_store import ConfigStore, only_on_real_changes_update
 from knot_resolver.utils import compat
 
-_watchdog = False
-if importlib.util.find_spec("watchdog"):
-    _watchdog = True
-
 logger = logging.getLogger(__name__)
 
 
-def tls_cert_paths(config: KresConfig) -> List[str]:
-    files: List[Optional[File]] = [
+def tls_cert_files_config(config: KresConfig) -> List[Any]:
+    return [
+        config.network.tls.files_watchdog,
         config.network.tls.cert_file,
         config.network.tls.key_file,
     ]
-    return [str(file) for file in files if file is not None]
 
 
-if _watchdog:
+if WATCHDOG_LIB:
     from watchdog.events import (
         FileSystemEvent,
         FileSystemEventHandler,
@@ -112,13 +107,16 @@ if _watchdog:
             self._observer.stop()
             self._observer.join()
 
-    @only_on_real_changes_update(tls_cert_paths)
-    async def _init_tls_cert_watchdog(config: KresConfig) -> None:
+
+@only_on_real_changes_update(tls_cert_files_config)
+async def _init_tls_cert_watchdog(config: KresConfig) -> None:
+    if WATCHDOG_LIB:
         global _tls_cert_watchdog
+
         if _tls_cert_watchdog:
             _tls_cert_watchdog.stop()
 
-        if config.network.tls.cert_file and config.network.tls.key_file:
+        if config.network.tls.files_watchdog and config.network.tls.cert_file and config.network.tls.key_file:
             logger.info("Initializing TLS certificate files WatchDog")
             _tls_cert_watchdog = TLSCertWatchDog(
                 config.network.tls.cert_file.to_path(),
@@ -128,6 +126,5 @@ if _watchdog:
 
 
 async def init_files_watchdog(config_store: ConfigStore) -> None:
-    if _watchdog:
-        # watchdog for TLS certificate files
-        await config_store.register_on_change_callback(_init_tls_cert_watchdog)
+    # watchdog for TLS certificate files
+    await config_store.register_on_change_callback(_init_tls_cert_watchdog)
diff --git a/python/knot_resolver/manager/metrics/prometheus.py b/python/knot_resolver/manager/metrics/prometheus.py
index 5dd0d171..4242d960 100644
--- a/python/knot_resolver/manager/metrics/prometheus.py
+++ b/python/knot_resolver/manager/metrics/prometheus.py
@@ -1,8 +1,8 @@
 import asyncio
-import importlib
 import logging
 from typing import Any, Dict, Generator, List, Optional, Tuple
 
+from knot_resolver.constants import PROMETHEUS_LIB
 from knot_resolver.controller.interface import KresID
 from knot_resolver.controller.registered_workers import get_registered_workers_kresids
 from knot_resolver.datamodel.config_schema import KresConfig
@@ -12,13 +12,9 @@ from knot_resolver.utils.functional import Result
 
 from .collect import collect_kresd_workers_metrics
 
-_prometheus_client = False
-if importlib.util.find_spec("prometheus_client"):
-    _prometheus_client = True
-
 logger = logging.getLogger(__name__)
 
-if _prometheus_client:
+if PROMETHEUS_LIB:
     from prometheus_client import exposition  # type: ignore
     from prometheus_client.bridge.graphite import GraphiteBridge  # type: ignore
     from prometheus_client.core import (
@@ -359,7 +355,7 @@ async def init_prometheus(config_store: ConfigStore) -> None:
     """
     Initialize metrics collection. Must be called before any other function from this module.
     """
-    if _prometheus_client:
+    if PROMETHEUS_LIB:
         # init and register metrics collector
         global _metrics_collector
         _metrics_collector = KresPrometheusMetricsCollector(config_store)
@@ -371,7 +367,7 @@ async def init_prometheus(config_store: ConfigStore) -> None:
 
 
 async def report_prometheus() -> Optional[bytes]:
-    if _prometheus_client:
+    if PROMETHEUS_LIB:
         # manually trigger stat collection so that we do not have to wait for it
         if _metrics_collector is not None:
             await _metrics_collector.collect_kresd_stats()
diff --git a/tests/manager/datamodel/test_network_schema.py b/tests/manager/datamodel/test_network_schema.py
index aed09310..1451ac20 100644
--- a/tests/manager/datamodel/test_network_schema.py
+++ b/tests/manager/datamodel/test_network_schema.py
@@ -3,7 +3,8 @@ from typing import Any, Dict, Optional
 import pytest
 from pytest import raises
 
-from knot_resolver.datamodel.network_schema import ListenSchema, NetworkSchema
+from knot_resolver.constants import WATCHDOG_LIB
+from knot_resolver.datamodel.network_schema import ListenSchema, NetworkSchema, TLSSchema
 from knot_resolver.datamodel.types import InterfaceOptionalPort, PortNumber
 from knot_resolver.utils.modeling.exceptions import DataValidationError
 
@@ -77,3 +78,16 @@ def test_listen_valid(listen: Dict[str, Any]):
 def test_listen_invalid(listen: Dict[str, Any]):
     with raises(DataValidationError):
         ListenSchema(listen)
+
+
+@pytest.mark.parametrize(
+    "tls",
+    [
+        {"files-watchdog": "auto"},
+        {"files-watchdog": True},
+        {"files-watchdog": False},
+    ],
+)
+def test_tls_files_watchdog(tls: Dict[str, Any]):
+    expected: bool = WATCHDOG_LIB if tls["files-watchdog"] == "auto" else tls["files-watchdog"]
+    assert TLSSchema(tls).files_watchdog == expected
diff --git a/tests/packaging/interactive/watchdog.sh b/tests/packaging/interactive/watchdog.sh
index 6e5e506a..ffc76e92 100755
--- a/tests/packaging/interactive/watchdog.sh
+++ b/tests/packaging/interactive/watchdog.sh
@@ -29,11 +29,31 @@ function count_reloads(){
     echo "$(journalctl -u knot-resolver.service | grep -c "Reloading of TLS certificate files has finished")"
 }
 
+# test that files watchdog is turned off
+# {{
+
+err_count=$(count_errors)
+rel_count=$(count_reloads)
+sleep 6
+
+if [ $(count_errors) -ne $err_count ] || [ $(count_reloads) -ne $rel_count ]; then
+    echo "TLS certificate files watchdog is running (should not) or other errors occurred."
+    exit 1
+fi
+
+# }}
+
+# configure TLS certificate files and turn on watchdog
+kresctl config set -p /network/tls/files-watchdog true
+if [ "$?" -ne "0" ]; then
+    echo "Could not turn on TLS certificate files watchdog."
+    exit 1
+fi
+
 # test modification
 # {{
 
 # modify certificate files with '-', it will trigger reload
-err_count=$(count_errors)
 rel_count=$(count_reloads)
 echo "-----------" >> $cert_file
 echo "-----------" >> $key_file