From 6c4318d3ae4ad42b0034521a9483747698332d4c Mon Sep 17 00:00:00 2001 From: Michal Date: Sat, 15 Aug 2026 23:07:07 +0100 Subject: [PATCH] feat(migration): reserve every active client at its current address kea does not inherit UniFi's lease database. At cutover it starts with an empty view of who holds what, so it can hand an address that is currently in use to a different device. Reservations are what carry "this device has this address" across the switch, because they live in config rather than in lease state. unifi-reserve-all.py creates one per active client, dry run by default. 51 written, 51/51 verified live by reading the records back; the controller now holds 85 reservations and the generator emits all 85 with unique, valid hostnames and no duplicate addresses. Active clients with no reservation went from 48 to 4. Three guards, each of which caught something real in the dry run: - VRRP virtual addresses are excluded. UniFi reports them as ordinary client addresses because the firewalls' bond MACs answer for them, and their apparent IP flips between the real interface address and the VIP. Without this, 192.168.1.254 -- the gateway VIP itself -- would have been given a DHCP reservation. - The firewalls' own interface MACs are excluded; those are statically configured routers, not DHCP clients. - Any address claimed by more than one MAC is dropped rather than guessed at. This is how the VIPs surfaced in the first place. Also skipped: addresses already reserved to another MAC, network gateways, and anything on a network that does not serve DHCP (which excludes the WAN transit VLANs automatically). labsim-dhcp-test.sh gained a lease-database flush, and it is not tidiness. Two findings, both of which first appeared as a PASSING test: - Re-running against stale leases, kea gave dynamic addresses to three devices that have reservations. The reservations were present and correct in kea's own config throughout. Kea saw the reserved address as leased to "another client" -- same MAC, different client-id from the earlier boot -- and allocated elsewhere. Cutover starts with an empty lease database so this is a testing artifact, but a reservation is evidently not unconditional once leases exist. - Removing only dhcp4-leases.csv does nothing: kea's memfile backend keeps lease-file-cleanup rotations (.csv.2) and restores from them on start. The verdict logic no longer takes the first matching lease row. Doing so reported an hours-old lease as the current answer and scored three failures as passes, including one where the device had plainly been given a dynamic address. A MAC with more than one lease is now an explicit failure rather than a guess. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DMVzWZgiKW2wquf5z8S1yH --- labsim/README.md | 26 ++++- labsim/labsim-dhcp-test.sh | 36 ++++++- migration/unifi-reserve-all.py | 186 +++++++++++++++++++++++++++++++++ 3 files changed, 243 insertions(+), 5 deletions(-) create mode 100755 migration/unifi-reserve-all.py diff --git a/labsim/README.md b/labsim/README.md index 05af260..497d2ee 100644 --- a/labsim/README.md +++ b/labsim/README.md @@ -104,10 +104,32 @@ python3 unifi-to-vyos.py --mode sim -o /tmp/sim.conf # 6 subnets, 31 mappings ``` **Result on VyOS 2026.08 (kea): all four cases pass.** The one that mattered: -30 of the 31 UniFi reservations sit *inside* the DHCP pool, and **kea honours -in-pool host reservations** — `printer1` received `172.31.10.46` from within the +most UniFi reservations sit *inside* the DHCP pool, and **kea honours in-pool +host reservations** — `printer1` received `172.31.10.46` from within the `.10.11–.11.254` pool. That was the open question blocking the cutover. +### The lease database will lie to you + +The script wipes `/config/dhcp/dhcp4-leases.csv*` before every run, and both +halves of that matter: + +- **Stale leases defeat reservations.** Re-running against yesterday's leases, + kea handed dynamic addresses to three devices that have reservations. The + reservation was present and correct in `/run/kea/kea-dhcp4.conf` the whole + time. Kea saw the reserved address as already leased to "another client" — + same MAC, but a different client-id from the earlier boot — and allocated + elsewhere. The cutover itself starts with an empty lease database, so this is + a *testing* artifact, but it is worth knowing that a reservation is not an + unconditional guarantee once leases exist. +- **The `*` is load-bearing.** Kea's memfile backend keeps lease-file-cleanup + rotations (`dhcp4-leases.csv.2`) and restores from them on start, so + truncating only the primary file changes nothing. + +Both of those first appeared as a *passing* test. The verdict logic now refuses +to score a MAC with more than one lease, because taking the first match had +reported an hours-old lease as the current answer and turned three failures +into apparent passes. + Still open: whether kea will hand a *reserved* address to a *different* client while the reserved device is offline. The negative case here only proves an unreserved MAC gets an unreserved address. diff --git a/labsim/labsim-dhcp-test.sh b/labsim/labsim-dhcp-test.sh index 3d79626..075bcc4 100755 --- a/labsim/labsim-dhcp-test.sh +++ b/labsim/labsim-dhcp-test.sh @@ -126,6 +126,24 @@ log " router has ${subnets:-0} subnets and ${maps:-0} static-mappings" [ "${maps:-0}" -gt 0 ] || die "router has no static-mappings -- apply the generated config first" cleanup_vms + +# Flush the lease database first. This is not tidiness -- it is the condition +# the cutover actually runs under, because kea does not inherit UniFi's leases +# and starts empty. It also makes the test deterministic: with stale leases +# present, kea saw the reserved address as held by "another client" (the same +# MAC but a different client-id from a previous boot) and allocated a dynamic +# address instead, which produced three misleading results before this existed. +log "flushing the router's lease database (cutover starts with an empty one)" +# Every dhcp4-leases.csv* must go, not just the main file: kea's memfile +# backend keeps lease-file-cleanup rotations (.1/.2) and restores from them on +# start, so truncating only the primary leaves the old leases intact. +router 'sudo systemctl stop isc-kea-dhcp4-server; + sudo sh -c "rm -f /config/dhcp/dhcp4-leases.csv*"; + sudo systemctl start isc-kea-dhcp4-server' >/dev/null +sleep 5 +remaining="$(router '/opt/vyatta/bin/vyatta-op-cmd-wrapper show dhcp server leases' | sed -n '3,$p' | grep -c .)" +[ "${remaining:-0}" -eq 0 ] || warn "lease table still has ${remaining} row(s) after flush" + SSH_PUB="$(find_ssh_pubkey)" sudo mkdir -p "$IMG_DIR" @@ -165,9 +183,21 @@ pass=0; fail=0 printf '%-19s %-16s %-16s %s\n' "MAC" "EXPECTED" "GOT" "RESULT" for c in "${CASES[@]}"; do IFS='|' read -r mac expected why <<<"$c" - got="$(echo "$leases" | awk -v m="$mac" 'tolower($0) ~ tolower(m) {print $1; exit}')" - got="${got:-}" - if [ "$expected" = "POOL" ]; then + # Never guess which lease is "the" lease. Taking the first match is how an + # hours-old lease was once reported as the current answer, turning three + # failures into apparent passes. + matches="$(echo "$leases" | awk -v m="$mac" 'tolower($2) == tolower(m) {print $1}')" + n_match="$(echo "$matches" | grep -c . )" + if [ "$n_match" -gt 1 ]; then + got="AMBIGUOUS($(echo "$matches" | tr '\n' ',' | sed 's/,$//'))" + else + got="${matches:-}" + fi + if [ "${got#AMBIGUOUS}" != "$got" ]; then + # More than one lease for this MAC means the flush did not take. Any + # verdict from here is a guess, so refuse to give one. + result="FAIL (multiple leases -- flush did not take)" + elif [ "$expected" = "POOL" ]; then if [ "$got" = "" ]; then result="FAIL (no lease at all)" elif echo "$reserved_ips" | grep -qx "$got"; then diff --git a/migration/unifi-reserve-all.py b/migration/unifi-reserve-all.py new file mode 100755 index 0000000..28c3963 --- /dev/null +++ b/migration/unifi-reserve-all.py @@ -0,0 +1,186 @@ +#!/usr/bin/env python3 +"""Reserve every active client at the address it already has. + +Why this exists: kea does not inherit UniFi's lease database. At cutover it +starts with an empty view of who holds what, so it can hand an address that is +currently in use to a different device. Reservations are what carry "this +device has this address" across the switch, because they live in config rather +than in lease state. + +Dry run by default -- this writes to the live controller, and 40-odd writes is +not something to trigger by accident. + + ./unifi-reserve-all.py # show the plan, change nothing + ./unifi-reserve-all.py --apply # write them + ./unifi-reserve-all.py --skip-random # omit randomised/private MACs + +Only clients on networks that actually run DHCP are considered, which +automatically excludes WAN transit VLANs where a reservation is meaningless. +Anything already reserved is left alone, and an address already reserved to a +different MAC is reported and skipped rather than stolen. +""" +from __future__ import annotations + +import argparse +import ipaddress +import sys + +import _unifi + +# The VRRP virtual addresses, read from `show configuration commands` on +# vyos001. UniFi sees these as ordinary client addresses because the firewalls' +# bond MACs answer for them, and their reported IP flips between the real +# interface address and the VIP. Reserving one would put a DHCP reservation on +# the gateway address itself. +VIPS = { + "192.168.1.254", "192.168.9.254", "192.168.3.254", + "10.0.9.254", "10.0.1.254", "192.168.2.254", +} + +# Every MAC the two firewalls own (bond0/eth0/eth1 share one, eth2 and eth3 +# have their own). These interfaces are statically configured routers, not DHCP +# clients -- except eth2, which is deliberately reserved and already handled. +ROUTER_MACS = { + "64:62:66:25:96:45", "64:62:66:25:96:46", "64:62:66:25:96:48", # vyos001 + "64:62:66:25:96:51", "64:62:66:25:96:52", "64:62:66:25:96:54", # vyos002 +} + + +def is_random_mac(mac: str) -> bool: + """Locally-administered bit set => a privacy/randomised MAC. + + Worth calling out: such a device re-randomises periodically, so the + reservation stops matching it and becomes dead config. Harmless, but it + will never do what it looks like it does. + """ + try: + return bool(int(mac.split(":")[0], 16) & 0x02) + except (ValueError, IndexError): + return False + + +def main() -> int: + ap = argparse.ArgumentParser() + ap.add_argument("--apply", action="store_true", help="actually write (default: dry run)") + ap.add_argument("--skip-random", action="store_true", help="omit randomised MACs") + args = ap.parse_args() + + opener, base, site = _unifi.client() + users = _unifi.get(opener, base, site, "rest/user") + nets = _unifi.get(opener, base, site, "rest/networkconf") + sta = _unifi.get(opener, base, site, "stat/sta") + for blob in (users, nets, sta): + if isinstance(blob, dict): + print(f"error reading controller: {blob['__error__']}", file=sys.stderr) + return 1 + + # Only networks that serve DHCP: a reservation on a WAN transit VLAN means + # nothing, and those are exactly the ones without dhcpd_enabled. + serving = [] + for n in nets: + if not (n.get("dhcpd_enabled") and n.get("ip_subnet")): + continue + try: + serving.append((ipaddress.ip_interface(n["ip_subnet"]).network, n)) + except ValueError: + continue + + by_mac = {(u.get("mac") or "").lower(): u for u in users} + taken = {u.get("fixed_ip"): (u.get("mac") or "").lower() + for u in users if u.get("use_fixedip")} + + plan, skipped = [], [] + for c in sta: + mac, ip = (c.get("mac") or "").lower(), c.get("ip") + if not mac or not ip: + continue + label = c.get("name") or c.get("hostname") or "?" + user = by_mac.get(mac) + + if ip in VIPS: + skipped.append((ip, mac, label, "VRRP virtual address - not a client")) + continue + if mac in ROUTER_MACS: + skipped.append((ip, mac, label, "firewall's own interface - statically configured")) + continue + + net = next((n for netw, n in serving if ipaddress.ip_address(ip) in netw), None) + if net is None: + skipped.append((ip, mac, label, "not on a DHCP-serving network")) + continue + # The gateway is the router, not a lease. + if ip == str(ipaddress.ip_interface(net["ip_subnet"]).ip): + skipped.append((ip, mac, label, "network gateway address")) + continue + if user is None: + skipped.append((ip, mac, label, "not a known client on the controller")) + continue + if user.get("use_fixedip"): + if user.get("fixed_ip") != ip: + skipped.append((ip, mac, label, + f"already reserved at {user.get('fixed_ip')} - left alone")) + continue + if ip in taken and taken[ip] != mac: + skipped.append((ip, mac, label, + f"address already reserved to {taken[ip]}")) + continue + if args.skip_random and is_random_mac(mac): + skipped.append((ip, mac, label, "randomised MAC (--skip-random)")) + continue + plan.append((ip, mac, label, user, net)) + + # Generic safety net: if two MACs report the same current address, at most + # one of them can legitimately keep it and we cannot tell which. Drop both + # and say so -- this is exactly how the VRRP VIPs first showed up. + counts: dict[str, int] = {} + for ip, *_ in plan: + counts[ip] = counts.get(ip, 0) + 1 + contested = {ip for ip, n in counts.items() if n > 1} + if contested: + for ip, mac, label, _u, _n in [p for p in plan if p[0] in contested]: + skipped.append((ip, mac, label, "address claimed by more than one MAC")) + plan = [p for p in plan if p[0] not in contested] + + plan.sort(key=lambda r: ipaddress.ip_address(r[0])) + + print(f"=== plan: {len(plan)} new reservation(s) ===") + for ip, mac, label, _u, net in plan: + flag = " [randomised MAC]" if is_random_mac(mac) else "" + print(f" {ip:16} {mac:18} {label[:28]:28} {net.get('name')}{flag}") + if skipped: + print(f"\n=== skipped ({len(skipped)}) ===") + for ip, mac, label, why in sorted(skipped): + print(f" {ip:16} {mac:18} {label[:24]:24} {why}") + + n_rand = sum(1 for p in plan if is_random_mac(p[1])) + if n_rand: + print(f"\nnote: {n_rand} of these use randomised MACs. The reservation " + f"stops matching once the device re-randomises.") + + if not args.apply: + print(f"\ndry run -- nothing written. Re-run with --apply to create {len(plan)}.") + return 0 + + print() + ok = fail = 0 + for ip, mac, label, user, net in plan: + res = _unifi.put(opener, base, site, f"rest/user/{user['_id']}", + {"use_fixedip": True, "fixed_ip": ip, "network_id": net["_id"]}) + if isinstance(res, dict): + print(f" FAILED {ip:16} {mac} {res['__error__'][:70]}") + fail += 1 + else: + ok += 1 + + # Read back rather than trusting the write responses. + after = _unifi.get(opener, base, site, "rest/user") + live = {(u.get("mac") or "").lower() for u in after if u.get("use_fixedip")} + verified = sum(1 for _ip, mac, _l, _u, _n in plan if mac in live) + print(f"\nwrote {ok}, failed {fail}, verified live {verified}/{len(plan)}") + print(f"total reservations on the controller now: " + f"{sum(1 for u in after if u.get('use_fixedip'))}") + return 0 if fail == 0 and verified == len(plan) else 1 + + +if __name__ == "__main__": + sys.exit(main())