From c868d41322b5a963c46e1d5e4e7ec0a2a359b90c Mon Sep 17 00:00:00 2001 From: Christian Manivong Date: Fri, 16 Oct 2026 11:11:45 +0200 Subject: [PATCH] refactor: the CARP state table lives with the driver that speaks CARP carp_virtual_ip() and its state table moved here from napalm-device-types (4.10.1, NAPALM/napalm-device-types#39): under the agreed structure the base package carries only the shape of the reading, and what a CARP state means is protocol knowledge. Tests moved along (test_carp_states.py). --- napalm_opnsense/virtual_ips.py | 69 ++++++++++++++++++-- pyproject.toml | 2 +- tests/unit/test_carp_states.py | 111 +++++++++++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_carp_states.py diff --git a/napalm_opnsense/virtual_ips.py b/napalm_opnsense/virtual_ips.py index bc37ea9..ea692b4 100644 --- a/napalm_opnsense/virtual_ips.py +++ b/napalm_opnsense/virtual_ips.py @@ -9,17 +9,78 @@ VHID's MASTER. An address without a VHID stays on this node and is no virtual IP each VHID's CARP password in it. The overview has none, so a secret never reaches the caller. -What a state means (MASTER holds, BACKUP stands by) is the same on every CARP -firewall: :func:`napalm_device_types.virtual_ips.carp_virtual_ip`. +What a state means -- MASTER holds, BACKUP stands by -- is CARP protocol +knowledge, so it lives here with the driver that speaks CARP +(:func:`carp_virtual_ip`), not in napalm-device-types, which carries only the +shape of the reading. """ from __future__ import annotations import ipaddress -from typing import Any +from typing import Any, Optional from napalm_device_types.models import VirtualIpDict -from napalm_device_types.virtual_ips import carp_virtual_ip + +#: CARP's state of a VHID on one member -> (running, holds). +_CARP_STATES: dict[str, tuple[bool, bool]] = { + "MASTER": (True, True), + "BACKUP": (True, False), + "INIT": (False, False), + "DISABLED": (False, False), +} + + +def _address(text: str) -> Optional[tuple[str, Optional[int]]]: + """``"10.0.0.1/24"`` -> ``("10.0.0.1", 24)``; None for what is no address.""" + host, slash, prefix = text.partition("/") + try: + address = ipaddress.ip_address(host) + except ValueError: + return None + if not slash: + return str(address), None + if not prefix.isdigit() or int(prefix) > address.max_prefixlen: + return None + return str(address), int(prefix) + + +def carp_virtual_ip( + *, + address: str, + prefix_length: Optional[int], + vhid: object, + state: Optional[str], + instance: str, + interface: Optional[str], +) -> Optional[VirtualIpDict]: + """One address bound to a CARP VHID, as a virtual IP; None for what is no address. + + Who holds it comes from CARP's *state* on this member, not from its + interfaces: MASTER holds it, BACKUP stands by, INIT and DISABLED neither run + nor hold. A state this does not know says nothing (``running`` and ``holds`` + None), and the caller falls back on the interfaces. *address* may carry its + prefix (``"10.0.0.1/24"``), which then wins over *prefix_length*. + """ + parsed = _address(address.strip()) if address else None + if parsed is None: + return None + normalised, written = parsed + prefix = written if written is not None else prefix_length + if prefix is not None and not 0 <= prefix <= ipaddress.ip_address(normalised).max_prefixlen: + prefix = None + vhid_text = str(vhid).strip() if vhid is not None else "" + running, holds = _CARP_STATES.get((state or "").strip().upper(), (None, None)) + return { + "mechanism": "carp", + "instance": instance or f"vhid {vhid_text}", + "address": normalised, + "prefix_length": prefix, + "interface": interface, + "group_key": f"vhid:{vhid_text}" if vhid_text.isdigit() else None, + "running": running, + "holds": holds, + } def _interfaces(overview: Any) -> list[Any]: diff --git a/pyproject.toml b/pyproject.toml index f297c7b..1f96197 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,7 +24,7 @@ classifiers = [ ] dependencies = [ "napalm>=4.0.0", - "napalm_device_types>=4.10.0", + "napalm_device_types>=4.10.1", "requests>=2.28.0", ] diff --git a/tests/unit/test_carp_states.py b/tests/unit/test_carp_states.py new file mode 100644 index 0000000..d6096f2 --- /dev/null +++ b/tests/unit/test_carp_states.py @@ -0,0 +1,111 @@ +"""carp_virtual_ip: one CARP address as a virtual IP, with its holder in the reading. + +CARP differs from vip-manager and keepalived in what the caller can see. The +address is configured on every member, and the interface list of a firewall need +not show it at all -- so the interfaces cannot say who holds it. CARP's own state +can: the member in MASTER holds it, a BACKUP stands by. That is CARP protocol +knowledge, so it lives with the driver that speaks CARP (netOrk #1121), not in +napalm-device-types. +""" + +from __future__ import annotations + +import pytest + +from napalm_opnsense.virtual_ips import carp_virtual_ip + + +def _entry(**overrides): + fields = { + "address": "172.22.8.1", + "prefix_length": 24, + "vhid": "108", + "state": "MASTER", + "instance": "MGMT", + "interface": "vtnet0", + } + fields.update(overrides) + return carp_virtual_ip(**fields) + + +def test_a_master_holds_the_address(): + assert _entry() == { + "mechanism": "carp", + "instance": "MGMT", + "address": "172.22.8.1", + "prefix_length": 24, + "interface": "vtnet0", + "group_key": "vhid:108", + "running": True, + "holds": True, + } + + +def test_a_backup_stands_by(): + entry = _entry(state="BACKUP") + assert entry is not None + assert (entry["running"], entry["holds"]) == (True, False) + + +@pytest.mark.parametrize("state", ["INIT", "DISABLED"]) +def test_a_member_whose_carp_is_not_up_neither_runs_nor_holds(state): + entry = _entry(state=state) + assert entry is not None + assert (entry["running"], entry["holds"]) == (False, False) + + +@pytest.mark.parametrize("state", [None, "", "SOMETHING-NEW"]) +def test_an_unknown_state_says_nothing_about_the_holder(state): + """None leaves the holder to the interfaces, as for the other mechanisms.""" + entry = _entry(state=state) + assert entry is not None + assert (entry["running"], entry["holds"]) == (None, None) + + +def test_the_state_is_read_case_blind(): + entry = _entry(state=" master ") + assert entry is not None + assert entry["holds"] is True + + +def test_the_address_is_normalised(): + entry = _entry(address="FD00::0001", prefix_length=64) + assert entry is not None + assert entry["address"] == "fd00::1" + + +def test_an_address_written_with_its_prefix_takes_that_prefix(): + entry = _entry(address="10.42.100.1/24", prefix_length=None) + assert entry is not None + assert (entry["address"], entry["prefix_length"]) == ("10.42.100.1", 24) + + +@pytest.mark.parametrize("address", ["", "not-an-address", "10.0.0.300"]) +def test_what_is_no_address_is_no_virtual_ip(address): + assert _entry(address=address) is None + + +@pytest.mark.parametrize("prefix", [33, -1]) +def test_an_impossible_prefix_is_dropped(prefix): + entry = _entry(prefix_length=prefix) + assert entry is not None + assert entry["prefix_length"] is None + + +@pytest.mark.parametrize("vhid", [None, "", "abc"]) +def test_without_a_numeric_vhid_there_is_no_group(vhid): + entry = _entry(vhid=vhid) + assert entry is not None + assert entry["group_key"] is None + + +def test_a_vhid_given_as_a_number_groups_the_same_way(): + entry = _entry(vhid=108) + assert entry is not None + assert entry["group_key"] == "vhid:108" + + +def test_an_empty_instance_falls_back_to_the_vhid(): + entry = _entry(instance="") + assert entry is not None + assert entry["instance"] == "vhid 108"