fix(dhcp): read the Kea subnet record from subnet4, not subnet
CI / test (3.10) (push) Failing after 8s
CI / test (3.11) (push) Failing after 8s
CI / test (3.12) (push) Failing after 13s
CI / test (3.9) (push) Failing after 8s

getSubnet wraps its record under `subnet4`. The driver read `subnet`, got
nothing, and carried on:

  - get_dhcp_subnets() reported every subnet with no pools, no options and no
    description. Only the CIDR survived, and only because it falls back to the
    searchSubnet row. Confirmed against a live OPNsense serving six subnets:
    all six came back with empty pools while the device had
    "10.10.0.100-10.10.0.250" and routers/DNS/NTP set on each.

  - apply_dhcp_subnet() read the same key to merge the options it was not
    asked to change. An empty record means nothing to preserve, so updating a
    subnet with only domain_search set would have written back only that one
    option and blanked the routers Kea autocollected — stranding every client
    on that VLAN without a gateway. That is precisely the failure the merge
    exists to prevent.

The unit fixtures encoded the wrong shape, which is why the safety test
test_unnamed_options_are_preserved_on_update passed while the real thing was
broken. They now carry the response captured from OPNsense 25.x, and correcting
them turns that test red against the old parse.

Both call sites go through _kea_subnet_record(), which prefers `subnet4` and
falls back to `subnet` for older builds.
This commit is contained in:
Christian Manivong
2026-08-20 11:49:16 +07:00
parent 0c5670981d
commit 960aaefa13
2 changed files with 40 additions and 7 deletions
+16 -2
View File
@@ -1618,6 +1618,20 @@ class OPNsenseDriver(OPNsensePingMixin, FirewallDriver):
return selected[0] if selected else ""
return str(raw or "").strip()
@staticmethod
def _kea_subnet_record(detail: dict[str, Any]) -> dict[str, Any]:
"""The subnet record out of a ``getSubnet`` response.
OPNsense wraps it under ``subnet4``; older builds used ``subnet``, and
keeping the fallback costs nothing. Reading the wrong key costs a
great deal: the record comes back empty, so a subnet reports no pools
and no options at all, and on update the partial-option merge has
nothing to preserve and blanks every option Kea autocollected --
stranding a whole VLAN without a gateway, which is the exact failure
that merge exists to prevent.
"""
return detail.get("subnet4") or detail.get("subnet") or {}
def get_dhcp_subnets(self) -> list[dict[str, Any]]:
"""Return every Kea DHCPv4 subnet with its pools and options.
@@ -1650,7 +1664,7 @@ class OPNsenseDriver(OPNsensePingMixin, FirewallDriver):
logger.warning("Kea subnet %s detail fetch failed, skipping: %s", uuid, exc)
continue
record = detail.get("subnet") or {}
record = self._kea_subnet_record(detail)
raw_options = record.get("option_data") or {}
option_data: dict[str, Any] = {}
@@ -1706,7 +1720,7 @@ class OPNsenseDriver(OPNsensePingMixin, FirewallDriver):
if uuid and desired_options:
try:
current = self._get(f"/api/kea/dhcpv4/getSubnet/{uuid}")
raw = (current.get("subnet") or {}).get("option_data") or {}
raw = self._kea_subnet_record(current).get("option_data") or {}
except Exception as exc:
raise RuntimeError(
f"Kea subnet {uuid} could not be read before update: {exc}"