diff --git a/napalm_opnsense/opnsense.py b/napalm_opnsense/opnsense.py index c8fdc2a..1a7c6a6 100644 --- a/napalm_opnsense/opnsense.py +++ b/napalm_opnsense/opnsense.py @@ -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}" diff --git a/tests/unit/test_driver.py b/tests/unit/test_driver.py index 21b7970..def9e88 100644 --- a/tests/unit/test_driver.py +++ b/tests/unit/test_driver.py @@ -2177,10 +2177,11 @@ KEA_SUBNET_SEARCH_RESPONSE = { ] } -# getSubnet wraps the record and, unlike searchSubnet, carries the options. -# OPNsense renders AsList fields as comma-separated strings. +# getSubnet wraps the record under `subnet4` -- captured from OPNsense 25.x, +# not guessed. Unlike searchSubnet it carries the options, and OPNsense +# renders AsList fields as comma-separated strings. KEA_SUBNET_DETAIL_RESPONSE = { - "subnet": { + "subnet4": { "subnet": "10.10.20.0/24", "description": "Home", "pools": "10.10.20.100-10.10.20.200", @@ -2243,7 +2244,7 @@ class TestGetDhcpSubnets: def test_multiline_pools_become_a_list(self, driver): detail = { - "subnet": { + "subnet4": { "subnet": "10.10.20.0/24", "description": "", "pools": "10.10.20.100-10.10.20.150\n10.10.20.180-10.10.20.200", @@ -2259,7 +2260,7 @@ class TestGetDhcpSubnets: def test_selection_map_shape_is_accepted(self, driver): """Some OPNsense versions render list fields as a selection map.""" detail = { - "subnet": { + "subnet4": { "subnet": "10.10.20.0/24", "description": "", "pools": "", @@ -2400,3 +2401,21 @@ class TestCommitDhcpSubnets: assert driver.commit_dhcp_subnets() == {"success": True} assert calls == ["/api/kea/service/reconfigure"] + + +class TestKeaSubnetRecordKey: + """`getSubnet` wraps its record under `subnet4`. Reading the wrong key is + not a cosmetic miss: the record comes back empty, so every subnet reports + no pools and no options, and the partial-option merge on update has + nothing to preserve.""" + + def test_reads_the_subnet4_wrapper(self, driver): + record = driver._kea_subnet_record({"subnet4": {"subnet": "10.0.0.0/24"}}) + assert record == {"subnet": "10.0.0.0/24"} + + def test_falls_back_to_the_legacy_subnet_wrapper(self, driver): + record = driver._kea_subnet_record({"subnet": {"subnet": "10.0.0.0/24"}}) + assert record == {"subnet": "10.0.0.0/24"} + + def test_unknown_shape_yields_an_empty_record(self, driver): + assert driver._kea_subnet_record({"something_else": {}}) == {}