dhcp: getSubnet record is read from the wrong key, so subnets report no options and apply blanks them #2

New Issue

2026-08-20T04:49:46Z

christianmanivong commented

2026-08-20 04:49:46 +00:00

Symptom

Against a live OPNsense serving six DHCPv4 subnets, get_dhcp_subnets() returned all six with empty pools, empty option_data and empty description — only the CIDR was right. The device has pools: "10.10.0.100-10.10.0.250" and routers/DNS/NTP set on every one of them.

Root cause

getSubnet wraps its record under subnet4, not subnet. Captured from the device:

{"subnet4": {"subnet": "10.10.0.0/24", "pools": "10.10.0.100-10.10.0.250",
  "option_data": {"routers": {"10.10.0.1": {"value": "10.10.0.1", "selected": 1}}, ...},
  "match-client-id": "0"}}

detail.get("subnet") is therefore None, the record is {}, and everything read from it is empty. The CIDR survived only because it falls back to the searchSubnet row, which is what made the result look almost right.

The serious half

apply_dhcp_subnet() read the same key to merge the options it was not asked to change:

raw = (current.get("subnet") or {}).get("option_data") or {}

An empty record means nothing to preserve. Updating a subnet with only domain_search set would have written back that single option and blanked the routers Kea autocollected — every client on that VLAN left without a gateway on the next lease. That is exactly the failure the partial-update contract exists to prevent, and it would have fired the first time anyone pressed Apply.

Why the tests missed it

KEA_SUBNET_DETAIL_RESPONSE in tests/unit/test_driver.py was written from the contract rather than captured from a device, so it used {"subnet": {...}}. test_unnamed_options_are_preserved_on_update passed against that fixture while the real thing was broken. Correcting the fixture to subnet4 turns that test red against the old parse — which is how the bug was confirmed.

Fix

960aaef. Both call sites go through _kea_subnet_record(), which prefers subnet4 and falls back to subnet for older builds. Fixtures now carry the captured OPNsense 25.x shape, plus tests for both wrappers and for an unknown one.

Found while adding DHCP subnet collection to netOrk's poll (christianmanivong/netork#100).

## Symptom Against a live OPNsense serving six DHCPv4 subnets, `get_dhcp_subnets()` returned all six with empty `pools`, empty `option_data` and empty `description` — only the CIDR was right. The device has `pools: "10.10.0.100-10.10.0.250"` and routers/DNS/NTP set on every one of them. ## Root cause `getSubnet` wraps its record under **`subnet4`**, not `subnet`. Captured from the device: ```json {"subnet4": {"subnet": "10.10.0.0/24", "pools": "10.10.0.100-10.10.0.250", "option_data": {"routers": {"10.10.0.1": {"value": "10.10.0.1", "selected": 1}}, ...}, "match-client-id": "0"}} ``` `detail.get("subnet")` is therefore `None`, the record is `{}`, and everything read from it is empty. The CIDR survived only because it falls back to the `searchSubnet` row, which is what made the result look almost right. ## The serious half `apply_dhcp_subnet()` read the same key to merge the options it was *not* asked to change: ```python raw = (current.get("subnet") or {}).get("option_data") or {} ``` An empty record means nothing to preserve. Updating a subnet with only `domain_search` set would have written back that single option and blanked the `routers` Kea autocollected — every client on that VLAN left without a gateway on the next lease. That is exactly the failure the partial-update contract exists to prevent, and it would have fired the first time anyone pressed Apply. ## Why the tests missed it `KEA_SUBNET_DETAIL_RESPONSE` in `tests/unit/test_driver.py` was written from the contract rather than captured from a device, so it used `{"subnet": {...}}`. `test_unnamed_options_are_preserved_on_update` passed against that fixture while the real thing was broken. Correcting the fixture to `subnet4` turns that test red against the old parse — which is how the bug was confirmed. ## Fix 960aaef. Both call sites go through `_kea_subnet_record()`, which prefers `subnet4` and falls back to `subnet` for older builds. Fixtures now carry the captured OPNsense 25.x shape, plus tests for both wrappers and for an unknown one. Found while adding DHCP subnet collection to netOrk's poll (christianmanivong/netork#100).

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: NAPALM/napalm-opnsense#2