diff --git a/daemon/handlers/dnsmasq.py b/daemon/handlers/dnsmasq.py index 5cff0a2..accdade 100644 --- a/daemon/handlers/dnsmasq.py +++ b/daemon/handlers/dnsmasq.py @@ -26,14 +26,14 @@ from daemon.iface import ( ) from daemon.server import NotFoundError, refresh_state, registry from lib.common import ( - _APPLY_HASH_KEY, - config_hash, deep_merge, ensure_dirs, get_interface_ip, load_json, run, save_json, + stamp_applied, + strip_apply_meta, ) from lib.sync import SyncEvent, bus @@ -139,7 +139,7 @@ def get_config(_request: Any, _body: Any) -> dict[str, Any]: if dm: return dm.get("config", {}) cfg = _get_config() - return {k: v for k, v in cfg.items() if k != _APPLY_HASH_KEY} + return strip_apply_meta(cfg) @registry.register(POST_DNSMASQ_CONFIG) @@ -196,9 +196,10 @@ def apply_config(_request: Any, _body: Any) -> dict[str, Any]: tmp.unlink(missing_ok=True) run(["systemctl", "restart", "dnsmasq"], sudo=True) logger.info("dnsmasq config written and restarted") - # Store the config hash so state collector can detect pending changes + # Store the applied config snapshot + hash so the state collector can + # detect pending changes and report what specifically changed. cfg_after = _get_config() - cfg_after[_APPLY_HASH_KEY] = config_hash(cfg_after) + stamp_applied(cfg_after) _save_config(cfg_after) sync_result = bus.emit( SyncEvent("dnsmasq", "config_saved", {"action": "config_applied"}) diff --git a/daemon/handlers/network.py b/daemon/handlers/network.py index e7b2eb9..a8a3808 100644 --- a/daemon/handlers/network.py +++ b/daemon/handlers/network.py @@ -21,7 +21,7 @@ from daemon.iface import ( POST_NETWORK_SYSCTL_SET, ) from daemon.server import NotFoundError, refresh_state, registry -from lib.common import _APPLY_HASH_KEY, config_hash, run, validate_interface_name +from lib.common import run, stamp_applied, validate_interface_name from lib.dnsmasq import get_config as _get_dm_cfg from lib.dnsmasq import save_config as _save_dm_cfg from lib.dnsmasq import set_upstreams @@ -218,7 +218,7 @@ def save_interface(_request: Any, body: dict[str, Any] | None) -> dict[str, Any] # even when deployment fails (e.g. in containerized environments). # The hash represents the JSON config state, not the system state. cfg_after = get_config() - cfg_after[_APPLY_HASH_KEY] = config_hash(cfg_after) + stamp_applied(cfg_after) save_config(cfg_after) sync_result = bus.emit( SyncEvent( @@ -286,16 +286,17 @@ def apply_all(_request: Any, _body: Any) -> dict[str, Any]: upstreams = collect_upstream_dns(cfg) if upstreams: set_upstreams(upstreams) - # Update dnsmasq apply hash so pending-changes detection stays correct + # Update dnsmasq applied snapshot + hash so pending-changes + # detection stays correct dm_cfg = _get_dm_cfg() - dm_cfg[_APPLY_HASH_KEY] = config_hash(dm_cfg) + stamp_applied(dm_cfg) _save_dm_cfg(dm_cfg) logger.info("Synced %d DNS upstreams to dnsmasq", len(upstreams)) except Exception: logger.warning("Failed to sync DNS upstreams to dnsmasq", exc_info=True) cfg_after = get_config() - cfg_after[_APPLY_HASH_KEY] = config_hash(cfg_after) + stamp_applied(cfg_after) save_config(cfg_after) sync_result = bus.emit( SyncEvent("networkd", "config_saved", {"action": "config_applied"}) diff --git a/daemon/handlers/nginx.py b/daemon/handlers/nginx.py index 36e181a..84a1f94 100644 --- a/daemon/handlers/nginx.py +++ b/daemon/handlers/nginx.py @@ -29,14 +29,14 @@ from daemon.iface import ( from daemon.server import ConflictError, NotFoundError, refresh_state, registry from lib.acme import find_cert_dir from lib.common import ( - _APPLY_HASH_KEY, - config_hash, deep_merge, ensure_dirs, load_json, run, run_proc, save_json, + stamp_applied, + strip_apply_meta, ) from lib.nginx import DEFAULT_SSL, WEBUI_BACKEND from lib.nginx import _resolve_auth as _ngx_resolve_auth @@ -494,7 +494,7 @@ def get_config(_request: Any, _body: Any) -> dict[str, Any]: if ng: return ng.get("config", {}) cfg = _get_config() - return {k: v for k, v in cfg.items() if k != _APPLY_HASH_KEY} + return strip_apply_meta(cfg) @registry.register(POST_NGINX_CONFIG) @@ -741,7 +741,7 @@ def apply(_request: Any, _body: Any) -> dict[str, Any]: raise RuntimeError(f"nginx config test failed: {msg}") _reload_nginx() cfg_after = _get_config() - cfg_after[_APPLY_HASH_KEY] = config_hash(cfg_after) + stamp_applied(cfg_after) _save_config(cfg_after) refresh_state(["nginx"]) return {"applied": True} diff --git a/daemon/handlers/wireguard.py b/daemon/handlers/wireguard.py index 2654433..a16d078 100644 --- a/daemon/handlers/wireguard.py +++ b/daemon/handlers/wireguard.py @@ -28,12 +28,7 @@ from daemon.iface import ( POST_WIREGUARD_PEERS_ADD, ) from daemon.server import ConflictError, NotFoundError, refresh_state, registry -from lib.common import ( - _APPLY_HASH_KEY, - config_hash, - deep_merge, - run, -) +from lib.common import deep_merge, run, stamp_applied, strip_apply_meta from lib.sync import SyncEvent, bus from lib.wireguard import ( _class_interface_name, @@ -82,7 +77,7 @@ def get_config(_request: Any, _body: Any) -> dict[str, Any]: if wg: return wg.get("config", {}) cfg = _get_wireguard_config() - safe = {k: v for k, v in cfg.items() if k != _APPLY_HASH_KEY} + safe = strip_apply_meta(cfg) if "interface" in safe: safe["interface"] = dict(safe["interface"]) safe["interface"].pop("private_key", None) @@ -220,7 +215,7 @@ def apply(_request: Any, _body: Any) -> dict[str, Any]: logger.info("WireGuard tunnel '%s' brought up", ifname) cfg_after = _get_wireguard_config() - cfg_after[_APPLY_HASH_KEY] = config_hash(cfg_after) + stamp_applied(cfg_after) _save_wireguard_config(cfg_after) sync_result = bus.emit( SyncEvent("wireguard", "config_saved", {"action": "config_applied"}) diff --git a/lib/common.py b/lib/common.py index d33cb5f..54436f5 100644 --- a/lib/common.py +++ b/lib/common.py @@ -16,21 +16,87 @@ from typing import Any from passlib.hash import sha256_crypt _APPLY_HASH_KEY = "_last_applied_hash" +_LAST_APPLIED_CONFIG_KEY = "_last_applied_config" +_APPLY_META_KEYS = (_APPLY_HASH_KEY, _LAST_APPLIED_CONFIG_KEY) + + +def strip_apply_meta(cfg: dict[str, Any]) -> dict[str, Any]: + """Return *cfg* without any apply bookkeeping keys. + + Strips both the last-applied hash and the last-applied config snapshot + so the returned dict reflects only real configuration. + """ + return {k: v for k, v in cfg.items() if k not in _APPLY_META_KEYS} def config_hash(cfg: dict[str, Any]) -> str: - """Compute a SHA-256 hash of *cfg* excluding the ``_last_applied_hash`` key. + """Compute a SHA-256 hash of *cfg*, ignoring apply bookkeeping keys. Args: - cfg: Config dict, possibly containing ``_last_applied_hash``. + cfg: Config dict, possibly containing ``_last_applied_hash`` and + ``_last_applied_config``. Returns: Hex digest of the stripped config JSON. """ - clean = {k: v for k, v in cfg.items() if k != _APPLY_HASH_KEY} + clean = strip_apply_meta(cfg) return hashlib.sha256(json.dumps(clean, sort_keys=True).encode()).hexdigest() +def stamp_applied(cfg: dict[str, Any]) -> dict[str, Any]: + """Record that *cfg* is the last applied configuration. + + Writes both the content snapshot (``_last_applied_config``) and its hash + (``_last_applied_hash``) so a later pending check can detect drift and a + diff can report exactly which fields changed. + """ + cfg[_APPLY_HASH_KEY] = config_hash(cfg) + cfg[_LAST_APPLIED_CONFIG_KEY] = strip_apply_meta(cfg) + return cfg + + +def deep_diff(old: Any, new: Any, prefix: str = "") -> list[dict[str, Any]]: + """Return a list of field-level changes between two configurations. + + Each entry is ``{"path", "action", "old", "new"}`` where *action* is one + of ``"added"``, ``"removed"`` or ``"changed"``. Dicts are recursed with + dotted paths; lists of equal length are compared element-by-element, + while any other value that differs is reported as a single change. + Apply bookkeeping keys are ignored. + """ + if isinstance(old, dict): + old = strip_apply_meta(old) + if isinstance(new, dict): + new = strip_apply_meta(new) + out: list[dict[str, Any]] = [] + _diff_nodes(old, new, prefix, out) + return out + + +def _diff_nodes(old: Any, new: Any, path: str, out: list[dict[str, Any]]) -> None: + """Recursively collect field-level changes from *old* into *new*.""" + if isinstance(old, dict) and isinstance(new, dict): + for key in sorted(set(old) | set(new)): + child = f"{path}.{key}" if path else str(key) + if key in old and key in new: + _diff_nodes(old[key], new[key], child, out) + elif key in old: + out.append( + {"path": child, "action": "removed", "old": old[key], "new": None} + ) + else: + out.append( + {"path": child, "action": "added", "old": None, "new": new[key]} + ) + return + if isinstance(old, list) and isinstance(new, list) and len(old) == len(new): + for i, (o, n) in enumerate(zip(old, new, strict=True)): + _diff_nodes(o, n, f"{path}[{i}]", out) + return + if old != new: + out.append({"path": path, "action": "changed", "old": old, "new": new}) + + def validate_interface_name(name: str) -> str: """Validate a Linux network interface name. @@ -210,8 +276,10 @@ def get_interface_ip(iface: str) -> str | None: __all__ = [ "_APPLY_HASH_KEY", + "_LAST_APPLIED_CONFIG_KEY", "_hash_password", "config_hash", + "deep_diff", "deep_merge", "ensure_dirs", "get_interface_ip", @@ -219,5 +287,7 @@ __all__ = [ "run", "run_proc", "save_json", + "stamp_applied", + "strip_apply_meta", "validate_interface_name", ] diff --git a/lib/firewall.py b/lib/firewall.py index d9c5b72..b5a490b 100644 --- a/lib/firewall.py +++ b/lib/firewall.py @@ -22,6 +22,22 @@ CONFIG_FILE: Path = CONFIG_DIR / "config.json" DEFAULT_CONFIG: dict[str, Any] = {"zones": {}} +# Zones firewalld ships by default. They are always present live and are +# never meaningful to flag as "unmanaged (not in config)". +FIREWALLD_BUILTIN_ZONES: frozenset[str] = frozenset( + { + "block", + "dmz", + "drop", + "external", + "home", + "host", + "internal", + "public", + "trusted", + } +) + # --------------------------------------------------------------------------- # Internal helpers @@ -313,17 +329,22 @@ def _compute_pending_changes( } ) - cfg_mq = zone_cfg.get("masquerade", False) - live_mq = live_zone.get("masquerade", False) - if cfg_mq != live_mq: - changes.append( - { - "zone": zone_name, - "type": "masquerade", - "config": cfg_mq, - "live": live_mq, - } - ) + # public zone masquerade is not reconciled by apply (it is driven by + # the nftables propagation step in daemon/handlers/firewall.py), so + # reporting it as pending here would advertise a change that never + # happens. Skip it to keep the diff consistent with apply. + if zone_name != "public": + cfg_mq = zone_cfg.get("masquerade", False) + live_mq = live_zone.get("masquerade", False) + if cfg_mq != live_mq: + changes.append( + { + "zone": zone_name, + "type": "masquerade", + "config": cfg_mq, + "live": live_mq, + } + ) cfg_rules = {r.get("rule") for r in zone_cfg.get("rich_rules", [])} live_rules = set(live_zone.get("rich-rules", [])) @@ -356,7 +377,7 @@ def _compute_pending_changes( ) for zone_name in live_zones: - if zone_name not in cfg_zones: + if zone_name not in cfg_zones and zone_name not in FIREWALLD_BUILTIN_ZONES: unknown_live[zone_name] = { "interfaces": live_zones[zone_name].get("interfaces", []), } @@ -415,6 +436,7 @@ __all__ = [ "CONFIG_FILE", "DATA_DIR", "DEFAULT_CONFIG", + "FIREWALLD_BUILTIN_ZONES", "RULES_FILE", "_compute_pending_changes", "_ensure_config_file", diff --git a/lib/schema.py b/lib/schema.py index c95700f..cbd7550 100644 --- a/lib/schema.py +++ b/lib/schema.py @@ -134,6 +134,23 @@ class DnsmasqDhcpLease(TypedDict): interface: str +class PendingChange(TypedDict): + """One field-level difference between the applied config and the current + saved config (see `lib.common.deep_diff`). + + Attributes: + path: Dotted (or indexed) path to the changed field. + action: "added", "removed", or "changed". + old: Value in the last applied config (None when added). + new: Value in the current config (None when removed). + """ + + path: str + action: str + old: Any + new: Any + + class DnsmasqStatus(TypedDict): """Dnsmasq service status snapshot. @@ -142,12 +159,15 @@ class DnsmasqStatus(TypedDict): config_file_exists: Whether the rendered .conf is on disk. active_leases: Count of currently active leases. pending_changes: Whether the config is dirty vs the applied state. + pending_diff: Field-level changes since the last apply (empty when + up to date or when no applied snapshot is recorded). """ service_active: bool config_file_exists: bool active_leases: int pending_changes: bool + pending_diff: list[PendingChange] class DnsmasqState(TypedDict): @@ -316,6 +336,8 @@ class WgStatus(TypedDict): peers: Legacy single-interface runtime peers. classes: Per-access-class runtime status (keyed by class name). pending_changes: Whether the config is dirty vs the applied state. + pending_diff: Field-level changes since the last apply (empty when + up to date or when no applied snapshot is recorded). """ up: bool @@ -323,6 +345,7 @@ class WgStatus(TypedDict): peers: list[WgStatusPeer] classes: dict[str, WgClassStatus] pending_changes: bool + pending_diff: list[PendingChange] class WgPeer(TypedDict, total=False): diff --git a/lib/state.py b/lib/state.py index 0a4cc31..d955586 100644 --- a/lib/state.py +++ b/lib/state.py @@ -13,7 +13,16 @@ from pathlib import Path from typing import Any, ClassVar from lib import schema -from lib.common import _APPLY_HASH_KEY, config_hash, load_json, run, run_proc +from lib.common import ( + _APPLY_HASH_KEY, + _LAST_APPLIED_CONFIG_KEY, + config_hash, + deep_diff, + load_json, + run, + run_proc, + strip_apply_meta, +) from lib.firewall import ( _parse_active_zones, _parse_all_zones_output, @@ -637,7 +646,13 @@ def _collect_dnsmasq() -> schema.DnsmasqState: cfg ) - safe_cfg = {k: v for k, v in cfg.items() if k != _APPLY_HASH_KEY} + safe_cfg = strip_apply_meta(cfg) + pending_diff: list[dict[str, Any]] = [] + if pending_changes: + snap = cfg.get(_LAST_APPLIED_CONFIG_KEY) + if isinstance(snap, dict): + pending_diff = deep_diff(snap, safe_cfg) + return { "config": safe_cfg, "status": { @@ -645,6 +660,7 @@ def _collect_dnsmasq() -> schema.DnsmasqState: "config_file_exists": conf_exists, "active_leases": len(leases), "pending_changes": pending_changes, + "pending_diff": pending_diff, }, "leases": leases, "timestamp": _now_iso(), @@ -732,11 +748,20 @@ def _collect_nginx() -> schema.NginxState: cfg ) - safe_cfg = {k: v for k, v in cfg.items() if k != _APPLY_HASH_KEY} + safe_cfg = strip_apply_meta(cfg) + nginx_pending_diff: list[dict[str, Any]] = [] + if pending_changes: + snap = cfg.get(_LAST_APPLIED_CONFIG_KEY) + if isinstance(snap, dict): + nginx_pending_diff = deep_diff(snap, safe_cfg) + return { "config": safe_cfg, "domains": domains, - "status": {"pending_changes": pending_changes}, + "status": { + "pending_changes": pending_changes, + "pending_diff": nginx_pending_diff, + }, "timestamp": _now_iso(), } @@ -939,7 +964,7 @@ def _collect_wireguard() -> schema.WgState: ) # Safe config (strip private keys from interface and access classes) - safe = {k: v for k, v in cfg.items() if k != _APPLY_HASH_KEY} + safe = strip_apply_meta(cfg) if "interface" in safe: safe["interface"] = dict(safe["interface"]) safe["interface"].pop("private_key", None) @@ -1116,6 +1141,16 @@ def _collect_wireguard() -> schema.WgState: status["up"] = True status["pending_changes"] = pending_changes + pending_diff: list[dict[str, Any]] = [] + if pending_changes: + snap = cfg.get(_LAST_APPLIED_CONFIG_KEY) + if isinstance(snap, dict): + # `safe` has private keys stripped; drop any private-key paths so + # the pending summary never exposes key material. + pending_diff = [ + d for d in deep_diff(snap, safe) if "private_key" not in d["path"] + ] + status["pending_diff"] = pending_diff return { "config": safe, "status": status, @@ -1164,7 +1199,14 @@ def _collect_networkd() -> schema.NetworkdState: ] != config_hash(net_cfg) result: dict[str, dict[str, Any]] = {} - safe_net_cfg = {k: v for k, v in net_cfg.items() if k != _APPLY_HASH_KEY} + safe_net_cfg = strip_apply_meta(net_cfg) + net_status: dict[str, Any] = {"pending_changes": pending_changes} + net_pending_diff: list[dict[str, Any]] = [] + if pending_changes: + snap = net_cfg.get(_LAST_APPLIED_CONFIG_KEY) + if isinstance(snap, dict): + net_pending_diff = deep_diff(snap, safe_net_cfg) + net_status["pending_diff"] = net_pending_diff try: raw = run(["networkctl", "status", "--json=short", "--all"], sudo=True) @@ -1173,21 +1215,21 @@ def _collect_networkd() -> schema.NetworkdState: return { "interfaces": {}, "config": safe_net_cfg, - "status": {"pending_changes": pending_changes}, + "status": net_status, "timestamp": _now_iso(), } except Exception: return { "interfaces": {}, "config": safe_net_cfg, - "status": {"pending_changes": pending_changes}, + "status": net_status, "timestamp": _now_iso(), } return { "interfaces": result, "config": safe_net_cfg, - "status": {"pending_changes": pending_changes}, + "status": net_status, "timestamp": _now_iso(), } diff --git a/lib/system_import.py b/lib/system_import.py index 008c768..ffcbd4f 100644 --- a/lib/system_import.py +++ b/lib/system_import.py @@ -949,7 +949,7 @@ def import_firewall() -> bool: def _cfgs_equal(a: dict[str, Any], b: dict[str, Any]) -> bool: - """Compare two configs ignoring _last_applied_hash.""" - a_clean = {k: v for k, v in a.items() if k != "_last_applied_hash"} - b_clean = {k: v for k, v in b.items() if k != "_last_applied_hash"} - return a_clean == b_clean + """Compare two configs ignoring apply bookkeeping keys.""" + from lib.common import strip_apply_meta + + return strip_apply_meta(a) == strip_apply_meta(b) diff --git a/tests/test_common.py b/tests/test_common.py new file mode 100644 index 0000000..a3c50cd --- /dev/null +++ b/tests/test_common.py @@ -0,0 +1,84 @@ +"""Tests for lib.common apply-metadata and diff helpers.""" + +from __future__ import annotations + +from lib.common import ( + _APPLY_HASH_KEY, + _LAST_APPLIED_CONFIG_KEY, + config_hash, + deep_diff, + stamp_applied, + strip_apply_meta, +) + + +class TestStripApplyMeta: + def test_strips_both_keys(self): + cfg = {"a": 1, _APPLY_HASH_KEY: "h", _LAST_APPLIED_CONFIG_KEY: {}} + assert strip_apply_meta(cfg) == {"a": 1} + + def test_missing_keys(self): + assert strip_apply_meta({"a": 1}) == {"a": 1} + + def test_does_not_mutate_input(self): + cfg = {"a": 1, _APPLY_HASH_KEY: "h"} + strip_apply_meta(cfg) + assert _APPLY_HASH_KEY in cfg + + +class TestConfigHashIgnoresMeta: + def test_hash_unaffected_by_metadata(self): + cfg = {"a": 1} + stamped = {"a": 1, _APPLY_HASH_KEY: "x", _LAST_APPLIED_CONFIG_KEY: {"a": 1}} + assert config_hash(cfg) == config_hash(stamped) + + +class TestStampApplied: + def test_records_snapshot_and_hash(self): + cfg = {"a": 1} + stamp_applied(cfg) + assert cfg[_LAST_APPLIED_CONFIG_KEY] == {"a": 1} + assert cfg[_APPLY_HASH_KEY] == config_hash(cfg) + + def test_stable(self): + cfg = {"a": 1} + stamp_applied(cfg) + # A pending-style check: hash matches the current (stripped) config. + assert _APPLY_HASH_KEY in cfg and cfg[_APPLY_HASH_KEY] == config_hash(cfg) + # No drift → no diff. + assert ( + deep_diff(cfg.get(_LAST_APPLIED_CONFIG_KEY, {}), strip_apply_meta(cfg)) + == [] + ) + + +class TestDeepDiff: + def test_identical_empty(self): + assert deep_diff({"a": 1, _APPLY_HASH_KEY: "h"}, {"a": 1}) == [] + + def test_changed_scalar(self): + diff = deep_diff({"a": 1}, {"a": 2}) + assert diff == [{"path": "a", "action": "changed", "old": 1, "new": 2}] + + def test_added_removed(self): + added = deep_diff({}, {"a": 1}) + assert added[0]["action"] == "added" and added[0]["new"] == 1 + removed = deep_diff({"a": 1}, {}) + assert removed[0]["action"] == "removed" and removed[0]["old"] == 1 + + def test_nested_and_list_index(self): + old = {"z": {"svc": ["http"], "ranges": [{"ip": "10.0.0.1", "n": 1}]}} + new = {"z": {"svc": ["http", "ssh"], "ranges": [{"ip": "10.0.0.2", "n": 1}]}} + paths = {d["path"] for d in deep_diff(old, new)} + assert "z.svc" in paths + assert "z.ranges[0].ip" in paths + assert not any(p.startswith("z.ranges[0].n") for p in paths) + + +class TestDashboardFallback: + def test_hash_subsystem_unchanged_generic(self): + # Guards that a pending status without a snapshot still yields a + # renderable pending flag (frontend falls back to a generic line). + status = {"pending_changes": True, "pending_diff": []} + assert status["pending_changes"] is True + assert status["pending_diff"] == [] diff --git a/tests/test_firewall.py b/tests/test_firewall.py index cc34259..d02d5dd 100644 --- a/tests/test_firewall.py +++ b/tests/test_firewall.py @@ -244,18 +244,87 @@ class TestConfigPending: @patch("lib.firewall.get_config") def test_detects_unmanaged_zones(self, mock_cfg): + # A custom live zone not in config is flagged as unmanaged. mock_cfg.return_value = {"zones": {}} state = { "zones": { - "public": { - "interfaces": ["eth0"], + "guest": { + "interfaces": ["eth5"], "services": [], "masquerade": False, }, }, } result = firewall.config_pending(state) - assert "public" in result["unmanaged_zones"] + assert "guest" in result["unmanaged_zones"] + + @patch("lib.firewall.get_config") + def test_built_in_zones_not_unmanaged(self, mock_cfg): + # firewalld built-in zones are always present and must not be + # reported as unmanaged, so they never surface as noise. + mock_cfg.return_value = {"zones": {}} + state = { + "zones": { + "public": {"interfaces": ["eth0"], "services": [], "masquerade": True}, + "trusted": {"interfaces": ["lo"], "services": [], "masquerade": False}, + "dmz": {"interfaces": ["eth7"], "services": [], "masquerade": False}, + }, + } + result = firewall.config_pending(state) + assert result["unmanaged_zones"] == {} + + @patch("lib.firewall.get_config") + def test_public_masquerade_not_pending(self, mock_cfg): + # public zone masquerade is driven by apply's propagation step, so a + # config-vs-live masquerade mismatch on public is not a pending change. + mock_cfg.return_value = { + "zones": { + "public": { + "interfaces": ["eth0"], + "services": ["http"], + "masquerade": False, + }, + }, + } + state = { + "zones": { + "public": { + "interfaces": ["eth0"], + "services": ["http"], + "masquerade": True, + }, + }, + } + result = firewall.config_pending(state) + assert not any(c["type"] == "masquerade" for c in result["pending"]) + assert result["needs_apply"] is False + + @patch("lib.firewall.get_config") + def test_non_public_masquerade_is_pending(self, mock_cfg): + # A non-public zone with a masquerade mismatch IS a pending change. + mock_cfg.return_value = { + "zones": { + "internal": { + "interfaces": ["eth1"], + "services": [], + "masquerade": False, + }, + }, + } + state = { + "zones": { + "internal": { + "interfaces": ["eth1"], + "services": [], + "masquerade": True, + }, + }, + } + result = firewall.config_pending(state) + assert any( + c["type"] == "masquerade" and c["zone"] == "internal" + for c in result["pending"] + ) # --------------------------------------------------------------------------- diff --git a/tests/test_state.py b/tests/test_state.py index 2329162..c9c9366 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -1,5 +1,6 @@ """Tests for lib/state.py — state store and collect functions.""" +import json from unittest.mock import patch from lib.state import State, state @@ -155,6 +156,64 @@ class TestCollectAll: assert "config" in result assert "leases" in result + @patch("lib.state.run_proc") + def test_collect_dnsmasq_pending_diff(self, mock_proc, tmp_path, monkeypatch): + from unittest.mock import Mock + + from lib.common import _APPLY_HASH_KEY, _LAST_APPLIED_CONFIG_KEY + from lib.state import _collect_dnsmasq + + (tmp_path / "config" / "dnsmasq").mkdir(parents=True) + applied = { + "dhcp": { + "ranges": [ + { + "interface": "eth1", + "start": "10.4.20.101", + "end": "10.4.20.200", + "lease_time": "1h", + "gateway": "10.4.20.1", + } + ], + "static_leases": [], + }, + "dns": {"upstreams": ["8.8.8.8"], "domain": None, "custom_records": []}, + } + cfg = { + "dhcp": { + "ranges": [ + { + "interface": "eth1", + "start": "10.4.20.100", + "end": "10.4.20.200", + "lease_time": "12h", + "gateway": "10.4.20.1", + } + ], + "static_leases": [], + }, + "dns": { + "upstreams": ["8.8.8.8", "1.1.1.1"], + "domain": None, + "custom_records": [], + }, + _LAST_APPLIED_CONFIG_KEY: applied, + _APPLY_HASH_KEY: "stale-hash", + } + (tmp_path / "config" / "dnsmasq" / "config.json").write_text(json.dumps(cfg)) + monkeypatch.setattr("lib.state.PROJECT_DIR", tmp_path) + # service check -> active; lease file read -> no lines + mock_proc.return_value = Mock(stdout="active\n", returncode=0) + + result = _collect_dnsmasq() + assert result["status"]["pending_changes"] is True + paths = {d["path"] for d in result["status"]["pending_diff"]} + assert "dhcp.ranges[0].start" in paths + assert "dhcp.ranges[0].lease_time" in paths + # Apply metadata must not leak into the returned config. + assert _LAST_APPLIED_CONFIG_KEY not in result["config"] + assert _APPLY_HASH_KEY not in result["config"] + class TestCollectFailure: def test_state_clears_on_failure(self): diff --git a/webui/static/pages/dashboard.js b/webui/static/pages/dashboard.js index 7adeb23..0fdf2c8 100644 --- a/webui/static/pages/dashboard.js +++ b/webui/static/pages/dashboard.js @@ -1,5 +1,45 @@ import { html, PageHeader, definePage, getModel, renderGuardMulti, ServiceStatus, StatCard, Table, Badge, ActionButton, fmtBytes } from '/static/hoover/index.js'; +// Render a single firewall change as "current → new". +// `live` is the currently applied value; `config` is the target value it +// will become on apply. Mirrors lib.firewall.fw_change_summary. +function fwChangeLine(c) { + const zone = c.zone || 'unknown'; + const type = c.type || 'unknown'; + const join = (arr) => (Array.isArray(arr) && arr.length ? arr.join(', ') : '∅'); + switch (type) { + case 'interfaces': + return `Zone ${zone}: interfaces ${join(c.live)} → ${join(c.config)}`; + case 'services': + return `Zone ${zone}: services ${join(c.live)} → ${join(c.config)}`; + case 'rich_rules': + return `Zone ${zone}: rich rules ${c.live_count ?? 0} → ${c.config_count ?? 0}`; + case 'forward_ports': + return `Zone ${zone}: port forwards ${c.live_count ?? 0} → ${c.config_count ?? 0}`; + case 'masquerade': + return `Zone ${zone}: masquerade ${c.live} → ${c.config}`; + case 'target': + return `Zone ${zone}: target ${c.live ?? 'default'} → ${c.config ?? 'default'}`; + default: + return `Zone ${zone}: ${type} changed`; + } +} + +function fmtVal(v) { + if (Array.isArray(v)) return v.length ? v.join(', ') : '∅'; + if (v === null || v === undefined) return '∅'; + if (typeof v === 'boolean') return v; + return v; +} + +// Render a single field-level pending change (applied → current). +function diffLine(d) { + const val = (x) => ` ${fmtVal(x)}`; + if (d.action === 'added') return `${d.path}: added (now${val(d.new)})`; + if (d.action === 'removed') return `${d.path}: removed (was${val(d.old)})`; + return `${d.path}: ${fmtVal(d.old)} → ${fmtVal(d.new)}`; +} + export default definePage({ init() { return { @@ -42,14 +82,34 @@ export default definePage({ // Pending changes — derived from the config-backed subsystem models. // Firewall uses pending.needs_apply (config_pending() output); all // others use status.pending_changes. `system` is metrics-only. - const pendKeys = ['firewall', 'dnsmasq', 'nginx', 'wireguard', 'networkd'].filter(k => { - const model = getModel(k === 'networkd' ? 'network' : k); - const d = model.data || {}; - if (k === 'firewall') return !!d.pending?.needs_apply; - return !!d.status?.pending_changes; - }); - const totalChanges = pendKeys.length; + // + // Each entry is { label, lines: [string] } so the card can render a + // concise per-change summary. Firewall and the hash-based subsystems + // both carry real per-field diffs (applied → current). const pendLabels = { firewall: 'Firewall', dnsmasq: 'DHCP', nginx: 'Proxy', wireguard: 'WireGuard', networkd: 'Network' }; + const pendingBlocks = []; + + const fwPending = state.firewall.data?.pending || {}; + if (fwPending.needs_apply) { + const lines = (fwPending.pending || []).map(fwChangeLine); + for (const z of Object.keys(fwPending.unmanaged_zones || {})) { + lines.push(`Zone ${z}: unmanaged (not in config)`); + } + pendingBlocks.push({ label: pendLabels.firewall, lines }); + } + + for (const k of ['dnsmasq', 'nginx', 'wireguard', 'networkd']) { + const d = getModel(k === 'networkd' ? 'network' : k).data || {}; + if (d.status?.pending_changes) { + const diff = d.status.pending_diff; + const lines = (Array.isArray(diff) && diff.length) + ? diff.map(diffLine) + : ['configuration saved but not applied yet']; + pendingBlocks.push({ label: pendLabels[k], lines }); + } + } + + const totalChanges = pendingBlocks.reduce((n, b) => n + b.lines.length, 0); // Build merged interface list const allNames = [...new Set([...fwIfaces.map(f => f.name), ...Object.keys(netIfaces)])]; @@ -85,11 +145,18 @@ export default definePage({ `; // ── Pending changes ── - const pendingCard = pendKeys.length > 0 + const pendingCard = pendingBlocks.length > 0 ? html`
Pending Changes <${Badge} text=${String(totalChanges)} variant="warning" />
-

Unapplied changes in: ${pendKeys.map(k => pendLabels[k]).join(', ')}

+ <${ActionButton} url="/api/status/apply-all" label="Apply All Changes" successMsg="All changes applied" cls="btn btn-sm btn-primary" /> diff --git a/webui/static/style.css b/webui/static/style.css index dddf596..a11fb73 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -702,6 +702,36 @@ body { flex: 1; } +/* Pending changes summary list */ +.pending-list { + list-style: none; + padding: 0; + margin: 0 0 12px; +} + +.pending-list > li { + padding: 6px 0; + border-bottom: 1px solid var(--border); +} + +.pending-list > li:last-child { + border-bottom: none; +} + +.pending-list > li > strong { + display: block; + margin-bottom: 4px; +} + +.pending-list > li > ul { + margin: 0; + padding-left: 18px; +} + +.pending-list li li { + padding: 1px 0; +} + /* Service status inline badge */ .service-status { display: inline-flex;