diff --git a/steward/mcp/tools/monitors.py b/steward/mcp/tools/monitors.py index e072509..2224f82 100644 --- a/steward/mcp/tools/monitors.py +++ b/steward/mcp/tools/monitors.py @@ -174,7 +174,9 @@ async def create_monitor(name: str, type: str, target: str, host: str = "", free-standing destination. config, by type (every key optional, defaults shown): tcp: {"port": 80} - dns: {"expected_ip": null} (set to fail when the name resolves elsewhere) + dns: {"expected_ip": null, "resolver": null} (expected_ip: fail when the + name resolves elsewhere; resolver: a DNS server's IP to ask + directly, timing its answer; null asks the container's own) http: {"method": "GET" | "HEAD" | "POST", "expected_status": 200, "content_match": "", "headers": {}, "timeout_seconds": 10 (1-60), "follow_redirects": true, "verify_ssl": true} diff --git a/steward/models/monitors.py b/steward/models/monitors.py index c75ed65..c3da490 100644 --- a/steward/models/monitors.py +++ b/steward/models/monitors.py @@ -53,7 +53,7 @@ class Monitor(Base): # Type-specific config as a JSON object. Keys by type: # icmp -> {} # tcp -> {"port": int} - # dns -> {"expected_ip": str | None} + # dns -> {"expected_ip": str | None, "resolver": str | None (a server IP; None = the container's)} # http -> {"method", "expected_status", "content_match", "headers", # "timeout_seconds", "follow_redirects", "verify_ssl"} config_json: Mapped[str] = mapped_column(Text, nullable=False, default="{}") diff --git a/steward/monitors/dns.py b/steward/monitors/dns.py index 55e8b5b..02724ce 100644 --- a/steward/monitors/dns.py +++ b/steward/monitors/dns.py @@ -1,30 +1,53 @@ -"""DNS resolution check — pure probe returning a result dict.""" +"""DNS resolution check — pure probe returning a result dict. + +With no resolver, the name is looked up the way the container looks names +up (getaddrinfo: /etc/hosts, then the resolvers in resolv.conf). With one, +an A and an AAAA query go straight to that server over UDP, and the time +it took to answer is the check's response time — so "does my local DNS +still answer for public names" and "does 1.1.1.1" are separate monitors. +""" from __future__ import annotations import asyncio +import ipaddress import logging +import os import socket +import struct +import time logger = logging.getLogger(__name__) +QUERY_TIMEOUT_S = 5.0 +_RCODES = {1: "FORMERR", 2: "SERVFAIL", 3: "NXDOMAIN", 4: "NOTIMP", 5: "REFUSED"} +_A, _AAAA = 1, 28 -async def dns_check(address: str, expected_ip: str | None = None) -> dict: - """Resolve `address`. Returns {is_up, resolved_ip, error_msg}. + +async def dns_check(address: str, expected_ip: str | None = None, + resolver: str | None = None) -> dict: + """Resolve `address`. Returns {is_up, resolved_ip, error_msg} and, when a + resolver is given, response_ms. Up = resolution succeeded AND (no expected_ip, or at least one returned record matches expected_ip). resolved_ip is the first A/AAAA record. """ - result = await _resolve(address) + if resolver: + result = await _query_server(address, resolver) + else: + result = await _resolve(address) + out = {"response_ms": result["ms"]} if result.get("ms") is not None else {} if not result["resolved"]: - return {"is_up": False, "resolved_ip": None, "error_msg": "Resolution failed"} + return {**out, "is_up": False, "resolved_ip": None, + "error_msg": result.get("error") or "Resolution failed"} resolved_ip = result["ip"] if expected_ip and expected_ip not in result["all_ips"]: return { + **out, "is_up": False, "resolved_ip": resolved_ip, "error_msg": f"Expected {expected_ip} not in {', '.join(result['all_ips'])}", } - return {"is_up": True, "resolved_ip": resolved_ip, "error_msg": None} + return {**out, "is_up": True, "resolved_ip": resolved_ip, "error_msg": None} async def _resolve(address: str) -> dict: @@ -41,3 +64,97 @@ async def _resolve(address: str) -> dict: except Exception as exc: logger.debug("DNS resolution failed for %s: %s", address, exc) return {"resolved": False, "ip": None, "all_ips": []} + + +async def _query_server(address: str, resolver: str) -> dict: + """Ask `resolver` directly for A and AAAA, together; the slower answer is the time.""" + started = time.monotonic() + answers = await asyncio.gather(_ask(address, resolver, _A), _ask(address, resolver, _AAAA), + return_exceptions=True) + ms = round((time.monotonic() - started) * 1000, 1) + ips, errors = [], [] + for got in answers: + (errors if isinstance(got, Exception) else ips).append(got) + all_ips = [ip for batch in ips for ip in batch] + if all_ips: + return {"resolved": True, "ip": all_ips[0], "all_ips": all_ips, "ms": ms} + if errors: + # A timeout on both queries leaves no time worth recording. + timed_out = all(isinstance(e, TimeoutError) for e in errors) and not ips + return {"resolved": False, "ip": None, "all_ips": [], "ms": None if timed_out else ms, + "error": f"{resolver}: {errors[0]}"} + return {"resolved": False, "ip": None, "all_ips": [], "ms": ms, + "error": f"{resolver} has no A or AAAA record for {address}"} + + +class _Reply(asyncio.DatagramProtocol): + def __init__(self, qid: int): + self.qid = qid + self.done = asyncio.get_running_loop().create_future() + + def datagram_received(self, data, addr): + if len(data) >= 2 and struct.unpack("!H", data[:2])[0] == self.qid and not self.done.done(): + self.done.set_result(data) + + def error_received(self, exc): + if not self.done.done(): + self.done.set_exception(exc) + + +async def _ask(name: str, resolver: str, qtype: int) -> list[str]: + """One UDP query; the addresses of `qtype` in the answer, or an exception saying why not.""" + qid = struct.unpack("!H", os.urandom(2))[0] + loop = asyncio.get_running_loop() + transport, proto = await loop.create_datagram_endpoint( + lambda: _Reply(qid), remote_addr=(resolver, 53)) + try: + transport.sendto(build_query(qid, name, qtype)) + try: + data = await asyncio.wait_for(proto.done, QUERY_TIMEOUT_S) + except asyncio.TimeoutError: + raise TimeoutError(f"no answer within {QUERY_TIMEOUT_S:.0f}s") from None + finally: + transport.close() + return parse_answer(data, qtype) + + +def build_query(qid: int, name: str, qtype: int) -> bytes: + """A standard recursive query for `name`.""" + header = struct.pack("!HHHHHH", qid, 0x0100, 1, 0, 0, 0) # RD set, one question + labels = b"".join(bytes([len(p)]) + p.encode("idna") for p in name.strip(".").split(".") if p) + return header + labels + b"\x00" + struct.pack("!HH", qtype, 1) + + +def parse_answer(data: bytes, qtype: int) -> list[str]: + """The `qtype` addresses in a reply; ValueError naming the server's error code.""" + if len(data) < 12: + raise ValueError("short reply") + _, flags, qd, an, _, _ = struct.unpack("!HHHHHH", data[:12]) + rcode = flags & 0x000F + if rcode: + raise ValueError(_RCODES.get(rcode, f"error code {rcode}")) + at = 12 + for _ in range(qd): + at = _skip_name(data, at) + 4 + ips = [] + for _ in range(an): + at = _skip_name(data, at) + rtype, _, _, length = struct.unpack("!HHIH", data[at:at + 10]) + at += 10 + rdata = data[at:at + length] + at += length + if rtype == qtype == _A and length == 4: + ips.append(str(ipaddress.IPv4Address(rdata))) + elif rtype == qtype == _AAAA and length == 16: + ips.append(str(ipaddress.IPv6Address(rdata))) + return ips + + +def _skip_name(data: bytes, at: int) -> int: + while True: + n = data[at] + if n == 0: + return at + 1 + if n & 0xC0 == 0xC0: # compression pointer ends the name + return at + 2 + at += n + 1 diff --git a/steward/monitors/routes.py b/steward/monitors/routes.py index 5617b82..ebea671 100644 --- a/steward/monitors/routes.py +++ b/steward/monitors/routes.py @@ -28,7 +28,7 @@ def _form_opts(form, mtype: str) -> dict: if mtype == MonitorType.tcp.value: return {"port": form.get("port")} if mtype == MonitorType.dns.value: - return {"expected_ip": form.get("expected_ip")} + return {"expected_ip": form.get("expected_ip"), "resolver": form.get("resolver")} if mtype == MonitorType.http.value: return { "method": form.get("method"), diff --git a/steward/monitors/runner.py b/steward/monitors/runner.py index 0a0d446..c824a3f 100644 --- a/steward/monitors/runner.py +++ b/steward/monitors/runner.py @@ -36,7 +36,8 @@ async def run_monitor(monitor: Monitor) -> dict: elif monitor.type == MonitorType.tcp.value: result.update(await tcp_check(monitor.target, int(cfg.get("port") or DEFAULT_TCP_PORT))) elif monitor.type == MonitorType.dns.value: - result.update(await dns_check(monitor.target, cfg.get("expected_ip") or None)) + result.update(await dns_check(monitor.target, cfg.get("expected_ip") or None, + cfg.get("resolver") or None)) elif monitor.type == MonitorType.http.value: result.update(await http_check( url=monitor.target, diff --git a/steward/monitors/service.py b/steward/monitors/service.py index f3efc53..ba91d3e 100644 --- a/steward/monitors/service.py +++ b/steward/monitors/service.py @@ -6,6 +6,7 @@ pipeline) call the same code and refuse the same input for the same reasons. """ from __future__ import annotations +import ipaddress import json from collections.abc import Mapping from datetime import datetime, timedelta, timezone @@ -21,7 +22,7 @@ HTTP_METHODS = ("GET", "HEAD", "POST") CONFIG_KEYS: dict[str, tuple[str, ...]] = { MonitorType.icmp.value: (), MonitorType.tcp.value: ("port",), - MonitorType.dns.value: ("expected_ip",), + MonitorType.dns.value: ("expected_ip", "resolver"), MonitorType.http.value: ("method", "expected_status", "content_match", "headers", "timeout_seconds", "follow_redirects", "verify_ssl"), } @@ -56,6 +57,17 @@ def _headers(value) -> dict: return value +def _resolver(value) -> str | None: + """A DNS server's IP address, or None for the container's own resolver.""" + value = str(value or "").strip() + if not value: + return None + try: + return str(ipaddress.ip_address(value)) + except ValueError: + raise ValueError(f"resolver must be a DNS server's IP address (e.g. 1.1.1.1), got {value!r}.") from None + + def normalise_target(mtype: str, target: str) -> str: """For http, default a bare host to https://; others pass through.""" target = (target or "").strip() @@ -77,7 +89,8 @@ def build_config(mtype: str, opts: Mapping) -> dict: if mtype == MonitorType.tcp.value: return {"port": _whole("port", opts.get("port"), 80, 1, 65535)} if mtype == MonitorType.dns.value: - return {"expected_ip": (str(opts.get("expected_ip") or "").strip() or None)} + return {"expected_ip": (str(opts.get("expected_ip") or "").strip() or None), + "resolver": _resolver(opts.get("resolver"))} if mtype == MonitorType.http.value: method = str(opts.get("method") or "GET").strip().upper() if method not in HTTP_METHODS: diff --git a/steward/templates/hosts/detail.html b/steward/templates/hosts/detail.html index a0d9257..3d17ee6 100644 --- a/steward/templates/hosts/detail.html +++ b/steward/templates/hosts/detail.html @@ -150,13 +150,7 @@ {# Monitors + Agent grid #} -{# ── Docker (docker plugin fragment; renders nothing if the host has none); - above Ansible: containers are state to read, Ansible is something to do ──── #} -{% if "docker" in enabled_plugins %} -
-{% endif %} - -{# ── Ansible (full width) ─────────────────────────────────────────────────── #} +{# ── Ansible (full width), before Docker ───────────────────────────────────── #}

Ansible

{% if linked_target %} @@ -243,6 +237,13 @@ {% endif %}
{# Ansible card #} +{# ── Docker (docker plugin fragment; renders nothing if the host has none). + After Ansible: the container list can run long, and Ansible beneath it was + easy to miss (#5214). ─────────────────────────────────────────────────────── #} +{% if "docker" in enabled_plugins %} +
+{% endif %} + {# ── SNMP (renders nothing unless a configured SNMP device maps to this host) ── #} {% if "snmp" in enabled_plugins %}
diff --git a/steward/templates/monitors/_fields.html b/steward/templates/monitors/_fields.html index 372c19c..cafbe4f 100644 --- a/steward/templates/monitors/_fields.html +++ b/steward/templates/monitors/_fields.html @@ -10,7 +10,7 @@

HTTP requests the target URL (http:// or https://). Up when the status matches, and the page contains the text below if you set one. For https, the certificate's expiry is tracked too.

- DNS looks the target name up (google.com, nas.lan, …) through the resolver Steward's container uses. Up when it resolves, and to the expected IP if you set one. No response time is recorded.

+ DNS looks the target name up (google.com, nas.lan, …). Up when it resolves, and to the expected IP if you set one. With no resolver it asks the way Steward's container does (its resolv.conf, so usually your local DNS) and records no response time; name a resolver (1.1.1.1, your router, …) to ask that server directly and time its answer.

{# tcp #}
@@ -24,6 +24,12 @@
+
+ + +
{# http #}
diff --git a/steward/templates/monitors/detail.html b/steward/templates/monitors/detail.html index ab0d60e..3dbab84 100644 --- a/steward/templates/monitors/detail.html +++ b/steward/templates/monitors/detail.html @@ -134,6 +134,9 @@
{{ value|join(", ") if value is mapping else ("yes" if value is sameas true else ("no" if value is sameas false else value)) }}
{% endif %} {% endfor %} + {% if monitor.type == "dns" and not monitor.config.get("resolver") %} +
Resolver
the container's own (its resolv.conf)
+ {% endif %} {% endblock %} diff --git a/tests/core/test_dns_check.py b/tests/core/test_dns_check.py new file mode 100644 index 0000000..91397a0 --- /dev/null +++ b/tests/core/test_dns_check.py @@ -0,0 +1,78 @@ +"""DNS monitors that name a resolver ask it directly (#5214).""" +import asyncio +import struct + +import pytest + +from steward.monitors import dns + + +def _reply(qid, name, records, rcode=0): + """A reply to build_query(qid, name, A): the question, then `records` + as (type, rdata), each owner name a pointer back to the question's.""" + question = dns.build_query(qid, name, 1)[12:] + head = struct.pack("!HHHHHH", qid, 0x8180 | rcode, 1, len(records), 0, 0) + body = b"".join(b"\xc0\x0c" + struct.pack("!HHIH", rtype, 1, 60, len(rdata)) + rdata + for rtype, rdata in records) + return head + question + body + + +def test_the_query_asks_one_recursive_question(): + q = dns.build_query(0x1234, "google.com.", 28) + assert q[:12] == struct.pack("!HHHHHH", 0x1234, 0x0100, 1, 0, 0, 0) + assert q[12:] == b"\x06google\x03com\x00" + struct.pack("!HH", 28, 1) + + +def test_an_answer_yields_its_addresses_past_a_cname(): + cname = b"\x03www\xc0\x0c" + data = _reply(7, "example.com", [(5, cname), (1, bytes([93, 184, 216, 34])), (1, bytes([10, 0, 0, 1]))]) + assert dns.parse_answer(data, 1) == ["93.184.216.34", "10.0.0.1"] + assert dns.parse_answer(data, 28) == [] + + +def test_a_server_error_is_named(): + with pytest.raises(ValueError, match="NXDOMAIN"): + dns.parse_answer(_reply(7, "nope.example", [], rcode=3), 1) + + +def _fake_ask(answers): + async def ask(name, resolver, qtype): + got = answers[qtype] + if isinstance(got, Exception): + raise got + return got + return ask + + +def test_a_resolver_check_is_up_and_timed(monkeypatch): + monkeypatch.setattr(dns, "_ask", _fake_ask({1: ["142.250.80.46"], 28: ["2607:f8b0::200e"]})) + res = asyncio.run(dns.dns_check("google.com", resolver="1.1.1.1")) + assert res["is_up"] and res["resolved_ip"] == "142.250.80.46" + assert isinstance(res["response_ms"], float) + + +def test_a_resolver_that_says_no_such_name_is_down_with_why(monkeypatch): + monkeypatch.setattr(dns, "_ask", _fake_ask({1: ValueError("NXDOMAIN"), 28: ValueError("NXDOMAIN")})) + res = asyncio.run(dns.dns_check("nope.example", resolver="192.168.0.9")) + assert not res["is_up"] and res["error_msg"] == "192.168.0.9: NXDOMAIN" + + +def test_a_silent_resolver_records_no_time(monkeypatch): + silent = TimeoutError("no answer within 5s") + monkeypatch.setattr(dns, "_ask", _fake_ask({1: silent, 28: silent})) + res = asyncio.run(dns.dns_check("google.com", resolver="10.9.9.9")) + assert not res["is_up"] and "response_ms" not in res + assert res["error_msg"] == "10.9.9.9: no answer within 5s" + + +def test_no_resolver_keeps_the_containers_lookup(monkeypatch): + called = {} + + async def resolve(address): + called["address"] = address + return {"resolved": True, "ip": "10.0.0.2", "all_ips": ["10.0.0.2"]} + + monkeypatch.setattr(dns, "_resolve", resolve) + monkeypatch.setattr(dns, "_ask", _fake_ask({})) # would KeyError if reached + res = asyncio.run(dns.dns_check("nas.lan")) + assert called == {"address": "nas.lan"} and res["is_up"] and "response_ms" not in res diff --git a/tests/core/test_host_page.py b/tests/core/test_host_page.py new file mode 100644 index 0000000..ebde741 --- /dev/null +++ b/tests/core/test_host_page.py @@ -0,0 +1,19 @@ +"""The host page: what sits where (#5214).""" +from types import SimpleNamespace + +from quart import render_template, session + + +async def test_ansible_comes_before_the_docker_list(app, monkeypatch): + # The Docker list can run to dozens of containers; Ansible under it was + # easy to miss, so it sits above. + monkeypatch.setitem(app.config, "PLUGINS", {"docker": {"enabled": True}}) + host = SimpleNamespace(id="h1", name="box", address="10.0.0.5") + async with app.test_request_context("/hosts/h1"): + session["user_role"] = "operator" + page = await render_template( + "hosts/detail.html", host=host, monitors=[], uptime=None, alerts=[], + monitor_types=[], linked_target=None, linkable_targets=[], ansible_sources=[]) + docker = page.index('hx-get="/plugins/docker/host/h1"') + assert page.index(">Ansible") < docker + assert page.index('hx-get="/plugins/host_agent/panel/h1"') < docker diff --git a/tests/core/test_monitors.py b/tests/core/test_monitors.py index 3c662c9..49b04ef 100644 --- a/tests/core/test_monitors.py +++ b/tests/core/test_monitors.py @@ -78,14 +78,16 @@ def test_run_monitor_http_passes_config(monkeypatch): def test_run_monitor_dns_passes_expected_ip(monkeypatch): seen = {} - async def fake_dns(address, expected_ip=None): - seen["address"], seen["expected_ip"] = address, expected_ip - return {"is_up": True, "resolved_ip": "192.0.2.1"} + async def fake_dns(address, expected_ip=None, resolver=None): + seen["address"], seen["expected_ip"], seen["resolver"] = address, expected_ip, resolver + return {"is_up": True, "resolved_ip": "192.0.2.1", "response_ms": 12.5} monkeypatch.setattr(runner_mod, "dns_check", fake_dns) res = asyncio.run(runner_mod.run_monitor(_monitor("dns", "host.test", {"expected_ip": "192.0.2.1"}))) - assert seen == {"address": "host.test", "expected_ip": "192.0.2.1"} + assert seen == {"address": "host.test", "expected_ip": "192.0.2.1", "resolver": None} assert res["resolved_ip"] == "192.0.2.1" + asyncio.run(runner_mod.run_monitor(_monitor("dns", "host.test", {"resolver": "1.1.1.1"}))) + assert seen["resolver"] == "1.1.1.1" # ── config validation (shared by the form and the MCP tools) ───────────────── @@ -107,8 +109,9 @@ def _from_form(form, mtype): def test_build_config_from_the_form_per_type(): assert _from_form(_Form(port="8080"), "tcp") == {"port": 8080} assert _from_form(_Form(), "tcp") == {"port": 80} - assert _from_form(_Form(expected_ip="10.0.0.1"), "dns") == {"expected_ip": "10.0.0.1"} - assert _from_form(_Form(), "dns") == {"expected_ip": None} + assert _from_form(_Form(expected_ip="10.0.0.1"), "dns") == {"expected_ip": "10.0.0.1", "resolver": None} + assert _from_form(_Form(resolver=" 1.1.1.1 "), "dns") == {"expected_ip": None, "resolver": "1.1.1.1"} + assert _from_form(_Form(), "dns") == {"expected_ip": None, "resolver": None} assert _from_form(_Form(), "icmp") == {} http = _from_form(_Form(method="get", expected_status="201", follow_redirects="on"), "http") @@ -128,6 +131,7 @@ def test_build_config_from_typed_values_defaults_to_safe_checks(): ("tcp", {"port": "http"}, "port must be a whole number"), ("tcp", {"expected_ip": "1.2.3.4"}, "does not take expected_ip; it takes port"), ("icmp", {"port": 22}, "takes no config"), + ("dns", {"resolver": "dns.google"}, "resolver must be a DNS server's IP address"), ("http", {"method": "DELETE"}, "not one of GET, HEAD, POST"), ("http", {"expected_status": 42}, "between 100 and 599"), ("http", {"timeout_seconds": 0}, "between 1 and 60"),