napalm_opnsense/opnsense.py

@@ -55,6 +55,7 @@ import requests
 from requests.exceptions import RequestException
 
 from napalm_device_types import ConfigSyncMixin, FingerprintRule, FirewallDriver, SnmpAgentMixin
+from napalm_device_types.dhcp import normalize_mac
 from napalm_device_types.models import (
     FixStepOutcomeDict,
     SnmpAgentSettingsDict,
@@ -1393,6 +1394,11 @@ class OPNsenseDriver(
         plugin as "reservations unsupported" (``RuntimeError``), not fall
         back to plain DHCP silently.
 
+        A reservation that already reads exactly as requested is left alone:
+        no ``setReservation`` and no ``service/reconfigure``. The reconfigure
+        stops and starts Kea rather than reloading it, so restarting the
+        DHCP server for a write that changes nothing is pure risk.
+
         :param mac: NIC MAC address (any common formatting; sent as-is to
             Kea's ``hw_address`` field).
         :param ip: IP address to reserve; must fall inside a subnet Kea
@@ -1409,10 +1415,12 @@ class OPNsenseDriver(
 
         ip_obj = ip_address(ip)
         subnet_uuid = None
+        subnet_cidr = ""
         for row in subnets:
             try:
                 if ip_obj in ip_network(row["subnet"], strict=False):
                     subnet_uuid = row["uuid"]
+                    subnet_cidr = str(row["subnet"]).strip()
                     break
             except ValueError:
                 continue
@@ -1422,7 +1430,7 @@ class OPNsenseDriver(
         # Idempotency: reuse an existing reservation for this IP if one
         # already exists (e.g. a retried provisioning job), rather than
         # creating a duplicate Kea rejects anyway.
-        existing_uuid = None
+        existing_row: dict[str, Any] | None = None
         try:
             existing = self._post(
                 "/api/kea/dhcpv4/searchReservation",
@@ -1430,7 +1438,7 @@ class OPNsenseDriver(
             )
             for row in existing.get("rows") or []:
                 if row.get("ip_address") == ip:
-                    existing_uuid = row.get("uuid")
+                    existing_row = row
                     break
         except Exception as exc:
             logger.debug("Kea reservation search failed, proceeding to add: %s", exc)
@@ -1444,6 +1452,13 @@ class OPNsenseDriver(
                 "description": self._NETORK_TAG,
             }
         }
+        if existing_row is not None and self._kea_reservation_unchanged(
+            existing_row, payload["reservation"], subnet_cidr
+        ):
+            logger.debug("Kea reservation for %s is already as requested, not touched", ip)
+            return
+
+        existing_uuid = existing_row.get("uuid") if existing_row is not None else None
         path = (
             f"/api/kea/dhcpv4/setReservation/{existing_uuid}"
             if existing_uuid
@@ -1455,6 +1470,27 @@ class OPNsenseDriver(
 
         self._post("/api/kea/service/reconfigure")
 
+    @staticmethod
+    def _kea_reservation_unchanged(
+        row: dict[str, Any], wanted: dict[str, Any], subnet_cidr: str
+    ) -> bool:
+        """Whether a ``searchReservation`` row already holds every field we would write.
+
+        The MAC is compared normalised: Kea keeps it as it was entered, in
+        either case and with ``:`` or ``-``. The subnet is a model relation that
+        comes back as the subnet's CIDR or its UUID depending on the OPNsense
+        version, so either counts. A field the row does not carry counts as a
+        difference -- writing once too often is the safe side.
+        """
+        if str(row.get("subnet") or "").strip() not in {wanted["subnet"], subnet_cidr}:
+            return False
+        if normalize_mac(row.get("hw_address")) != normalize_mac(wanted["hw_address"]):
+            return False
+        return all(
+            str(row.get(field) or "") == str(wanted[field] or "")
+            for field in ("ip_address", "hostname", "description")
+        )
+
     def delete_dhcp_reservation_and_lease(self, mac: str, ip: str) -> dict[str, Any]:
         """Remove a Kea DHCPv4 static reservation and any active lease for (mac, ip).
 

tests/unit/test_driver.py

@@ -1787,6 +1787,83 @@ class TestCreateDhcpReservation:
             == "https://opnsense.example.com/api/kea/dhcpv4/setReservation/existing-uuid-456"
         )
 
+    # A reconfigure stops and starts Kea (ApiMutableServiceControllerBase,
+    # reconfigureForceRestart() is true for Kea). Writing a reservation that
+    # is already there, and restarting the daemon for it, once left a
+    # firewall without DHCP (#1187) -- so an unchanged one is not touched.
+
+    @pytest.mark.parametrize(
+        "subnet",
+        ["172.22.8.0/24", "6854cab3-ebb5-4031-b987-0edcc6723546"],
+        ids=["subnet-as-cidr", "subnet-as-uuid"],
+    )
+    def test_unchanged_reservation_is_neither_written_nor_reconfigured(self, driver, subnet):
+        driver.session.get.return_value = _make_json_response(KEA_SUBNETS_RESPONSE)
+        driver.session.post.side_effect = [
+            _make_json_response(
+                {
+                    "rows": [
+                        {
+                            "uuid": "existing-uuid-456",
+                            "subnet": subnet,
+                            "ip_address": "172.22.8.253",
+                            # Kea keeps the MAC as it was entered -- case and
+                            # separator do not make it a different reservation.
+                            "hw_address": "02-AA-BB-CC-DD-EE",
+                            "hostname": "new-vm",
+                            "description": "[netork]",
+                        }
+                    ]
+                }
+            ),
+        ]
+
+        driver.create_dhcp_reservation(
+            mac="02:aa:bb:cc:dd:ee", ip="172.22.8.253", hostname="new-vm"
+        )
+
+        # searchReservation only -- no setReservation, no service/reconfigure.
+        assert driver.session.post.call_count == 1
+
+    @pytest.mark.parametrize(
+        "field, live_value",
+        [
+            ("hw_address", "02:aa:bb:cc:dd:ff"),
+            ("hostname", "old-vm"),
+            ("description", "made by hand"),
+            ("subnet", "172.22.0.0/24"),
+        ],
+    )
+    def test_reservation_differing_in_one_field_is_written_and_reconfigured(
+        self, driver, field, live_value
+    ):
+        row = {
+            "uuid": "existing-uuid-456",
+            "subnet": "172.22.8.0/24",
+            "ip_address": "172.22.8.253",
+            "hw_address": "02:aa:bb:cc:dd:ee",
+            "hostname": "new-vm",
+            "description": "[netork]",
+        }
+        row[field] = live_value
+        driver.session.get.return_value = _make_json_response(KEA_SUBNETS_RESPONSE)
+        driver.session.post.side_effect = [
+            _make_json_response({"rows": [row]}),
+            _make_json_response({"result": "saved", "uuid": "existing-uuid-456"}),
+            _make_json_response({"status": "ok"}),
+        ]
+
+        driver.create_dhcp_reservation(
+            mac="02:aa:bb:cc:dd:ee", ip="172.22.8.253", hostname="new-vm"
+        )
+
+        paths = [c.args[0] for c in driver.session.post.call_args_list]
+        assert paths == [
+            "https://opnsense.example.com/api/kea/dhcpv4/searchReservation",
+            "https://opnsense.example.com/api/kea/dhcpv4/setReservation/existing-uuid-456",
+            "https://opnsense.example.com/api/kea/service/reconfigure",
+        ]
+
     def test_raises_when_ip_not_in_any_kea_subnet(self, driver):
         driver.session.get.return_value = _make_json_response(KEA_SUBNETS_RESPONSE)