From adf82f8f01a08d549027ae099d030de74d336da8 Mon Sep 17 00:00:00 2001 From: Pouzor Date: Mon, 6 Jul 2026 20:11:03 +0200 Subject: [PATCH] feat: merge IP-scanned and Proxmox-imported devices by MAC Reconcile the same physical device discovered by both the nmap IP scan and the Proxmox importer into a single inventory row, keyed on MAC. Previously each path only deduped by IP, and the importer captured no MAC (and no IP for stopped guests), so most guests double-listed. Backend: - mac_utils.normalize_mac: canonical MAC (lowercase, ':'-separated), the cross-source join key. Normalized on write and on compare. - proxmox_service: capture the guest NIC MAC agent-free from the net0 config (qemu virtio=, lxc hwaddr=); works for stopped guests. Resolver now returns (ip, mac). - proxmox persist: match existing Node/PendingDevice by ieee OR ip OR MAC; fill mac, keep the vm/lxc type, union sources. - scanner persist: match PendingDevice by ip OR MAC; fill the IP a Proxmox import lacked, keep a pve row's type, union the scan source. Stamp query matches raw + normalized MAC (legacy-safe). - Multi-source tags: new PendingDevice.discovery_sources JSON column so a merged device shows under both the IP and Proxmox filters. Idempotent migration backfills from discovery_source (legacy NULL-scalar rows with an IP become ["arp"]). _sources_after_merge preserves a scanned row's IP origin through the merge without tagging a pure Proxmox guest. - Import now broadcasts a scan update on completion so an open inventory reloads without a manual refresh. Frontend: - pendingSources: sourceBuckets/orderedSources map discovery_sources to filter buckets; a device with ["arp","proxmox"] matches both filters and renders both badges. PendingDevicesModal filter + badges use them. Tests: MAC normalization, config MAC capture, cross-source merge both directions, legacy-row IP-tag preservation, no-false-IP-tag guard, refresh broadcast, and the frontend bucket mapping. ha-relevant: maybe --- backend/app/api/routes/proxmox.py | 71 +++++++-- backend/app/db/database.py | 31 ++++ backend/app/db/models.py | 6 + backend/app/schemas/scan.py | 9 +- backend/app/services/discovery_sources.py | 19 +++ backend/app/services/mac_utils.py | 18 +++ backend/app/services/proxmox_service.py | 70 ++++++--- backend/app/services/scanner.py | 42 ++++-- backend/tests/test_mac_utils.py | 27 ++++ backend/tests/test_proxmox_router.py | 135 +++++++++++++++++- backend/tests/test_proxmox_service.py | 41 ++++++ backend/tests/test_scanner.py | 38 +++++ .../components/modals/PendingDeviceModal.tsx | 4 + .../components/modals/PendingDevicesModal.tsx | 41 ++---- .../utils/__tests__/pendingSources.test.ts | 49 +++++++ frontend/src/utils/pendingSources.ts | 47 ++++++ 16 files changed, 572 insertions(+), 76 deletions(-) create mode 100644 backend/app/services/discovery_sources.py create mode 100644 backend/app/services/mac_utils.py create mode 100644 backend/tests/test_mac_utils.py create mode 100644 frontend/src/utils/__tests__/pendingSources.test.ts create mode 100644 frontend/src/utils/pendingSources.ts diff --git a/backend/app/api/routes/proxmox.py b/backend/app/api/routes/proxmox.py index 2986777..e261708 100644 --- a/backend/app/api/routes/proxmox.py +++ b/backend/app/api/routes/proxmox.py @@ -32,6 +32,8 @@ from app.schemas.proxmox import ( ProxmoxTestConnectionResponse, ) from app.schemas.scan import ScanRunResponse +from app.services.discovery_sources import add_source +from app.services.mac_utils import normalize_mac from app.services.node_dedupe import dedupe_nodes_by_ieee from app.services.proxmox_service import ( build_proxmox_cluster_links, @@ -164,6 +166,10 @@ async def _background_proxmox_import( run.finished_at = datetime.now(timezone.utc) run.error = _guest_visibility_advisory(nodes_raw) await db.commit() + # Nudge the frontend to reload the inventory (same signal the IP scan + # emits) so imported/merged devices appear without a manual refresh. + from app.api.routes.status import broadcast_scan_update + await broadcast_scan_update(run_id=run_id, devices_found=result.device_count) except Exception as exc: logger.exception("Proxmox import %s failed", run_id) await db.rollback() @@ -223,14 +229,19 @@ async def _persist_pending_import( if not ieee: continue ip = n.get("ip") + mac = normalize_mac(n.get("mac")) props = build_proxmox_properties(n) - # 1) Already on a canvas? Match by ieee OR (ip when known). Refresh in - # place: merge properties, adopt the pve identity onto a scanned node, - # backfill blank specs/hostname. Do NOT stomp user-set type/status. + # 1) Already on a canvas? Match by ieee OR ip OR mac (the cross-source + # dedup key — a stopped VM has no IP but its configured NIC MAC still + # matches an ARP-scanned node). Refresh in place: merge properties, adopt + # the pve identity onto a scanned node, backfill blank specs/hostname/mac. + # Do NOT stomp user-set type/status. node_filter = [Node.ieee_address == ieee] if ip: node_filter.append(Node.ip == ip) + if mac: + node_filter.append(Node.mac == mac) existing_nodes = ( await db.execute(select(Node).where(or_(*node_filter)).order_by(Node.id)) ).scalars().all() @@ -242,6 +253,8 @@ async def _persist_pending_import( en.ieee_address = ieee if ip and not en.ip: en.ip = ip + if mac and not en.mac: + en.mac = mac en.hostname = en.hostname or n.get("hostname") en.cpu_count = en.cpu_count or n.get("cpu_count") en.ram_gb = en.ram_gb or n.get("ram_gb") @@ -251,17 +264,17 @@ async def _persist_pending_import( if ieee in cluster_members: en.left_handles = max(en.left_handles or 0, 1) en.right_handles = max(en.right_handles or 0, 1) - await _ensure_inventory_row(db, ieee, ip, n, props, approved=True) + await _ensure_inventory_row(db, ieee, ip, mac, n, props, approved=True) pending_updated += 1 continue # 2) Not on canvas — upsert the pending inventory row. - pending = await _find_pending(db, ieee, ip) + pending = await _find_pending(db, ieee, ip, mac) if pending is None: - db.add(_new_pending(ieee, ip, n, props, status="pending")) + db.add(_new_pending(ieee, ip, mac, n, props, status="pending")) pending_created += 1 else: - _refresh_pending(pending, ieee, ip, n, props) + _refresh_pending(pending, ieee, ip, mac, n, props) pending_updated += 1 links_recorded = await _replace_links(db, edges_raw, cluster_pairs) @@ -276,22 +289,30 @@ async def _persist_pending_import( async def _find_pending( - db: AsyncSession, ieee: str, ip: str | None + db: AsyncSession, ieee: str, ip: str | None, mac: str | None ) -> PendingDevice | None: filters = [PendingDevice.ieee_address == ieee] if ip: filters.append(PendingDevice.ip == ip) + if mac: + filters.append(PendingDevice.mac == mac) return ( await db.execute(select(PendingDevice).where(or_(*filters))) ).scalars().first() def _new_pending( - ieee: str, ip: str | None, n: dict[str, Any], props: list[dict[str, Any]], status: str + ieee: str, + ip: str | None, + mac: str | None, + n: dict[str, Any], + props: list[dict[str, Any]], + status: str, ) -> PendingDevice: return PendingDevice( ieee_address=ieee, ip=ip, + mac=mac, hostname=n.get("hostname"), friendly_name=n.get("label"), suggested_type=n.get("type"), @@ -299,19 +320,41 @@ def _new_pending( model=n.get("model"), properties=props, status=status, - discovery_source="proxmox", + discovery_source=_PROXMOX_GUEST_SOURCE, + discovery_sources=[_PROXMOX_GUEST_SOURCE], ) +def _sources_after_merge(row: PendingDevice) -> list[str]: + """Discovery sources for an inventory row after a Proxmox import merges in. + + Must run BEFORE the ``pve-`` ieee is adopted onto the row, so it can tell + whether the row was originally a scanned device. Preserves the prior scan + origin — including legacy rows created before ``discovery_sources`` existed + (empty list) and possibly with a NULL ``discovery_source`` — so the IP tag + survives the merge. A row that carries an IP but was not itself a Proxmox + device (no ``pve-`` ieee) was found by a scan; keep an IP-scan source. + """ + sources = add_source(row.discovery_sources, row.discovery_source) + was_scanned = not (row.ieee_address or "").startswith("pve-") + if was_scanned and row.ip and not any(s in ("arp", "mdns") for s in sources): + sources = add_source(sources, "arp") + return add_source(sources, _PROXMOX_GUEST_SOURCE) + + def _refresh_pending( pending: PendingDevice, ieee: str, ip: str | None, + mac: str | None, n: dict[str, Any], props: list[dict[str, Any]], ) -> None: + # Compute sources before adopting the pve ieee (needs the pre-merge origin). + pending.discovery_sources = _sources_after_merge(pending) pending.ieee_address = pending.ieee_address or ieee pending.ip = ip or pending.ip + pending.mac = pending.mac or mac pending.hostname = n.get("hostname") or pending.hostname pending.friendly_name = n.get("label") or pending.friendly_name pending.suggested_type = n.get("type") or pending.suggested_type @@ -328,18 +371,22 @@ async def _ensure_inventory_row( db: AsyncSession, ieee: str, ip: str | None, + mac: str | None, n: dict[str, Any], props: list[dict[str, Any]], approved: bool, ) -> None: """Ensure an inventory row exists for a device already on a canvas, so it shows in the inventory with an 'In N canvas' badge. Never changes status.""" - inv = await _find_pending(db, ieee, ip) + inv = await _find_pending(db, ieee, ip, mac) if inv is None: - db.add(_new_pending(ieee, ip, n, props, status="approved" if approved else "pending")) + db.add(_new_pending(ieee, ip, mac, n, props, status="approved" if approved else "pending")) else: + # Compute sources before adopting the pve ieee (needs the pre-merge origin). + inv.discovery_sources = _sources_after_merge(inv) inv.ieee_address = inv.ieee_address or ieee inv.ip = ip or inv.ip + inv.mac = inv.mac or mac inv.hostname = n.get("hostname") or inv.hostname inv.suggested_type = n.get("type") or inv.suggested_type inv.properties = merge_proxmox_properties(list(inv.properties or []), props) diff --git a/backend/app/db/database.py b/backend/app/db/database.py index e7177b8..79088f4 100644 --- a/backend/app/db/database.py +++ b/backend/app/db/database.py @@ -321,6 +321,37 @@ async def init_db() -> None: with suppress(OperationalError): sql = "UPDATE edges SET animated = 'none' WHERE animated = '0' OR animated = 0 OR animated IS NULL" await conn.exec_driver_sql(sql) + # Multi-source discovery tags: a device found by both an IP scan and a + # Proxmox import carries every source. Backfill from the legacy single + # discovery_source so existing rows show under their filter. + with suppress(OperationalError): + await conn.exec_driver_sql("ALTER TABLE pending_devices ADD COLUMN discovery_sources JSON") + with suppress(OperationalError): + await conn.exec_driver_sql( + "UPDATE pending_devices SET discovery_sources = json_array(discovery_source) " + "WHERE discovery_sources IS NULL AND discovery_source IS NOT NULL" + ) + # Legacy IP-scanned rows predating discovery_source have a NULL scalar + # but a real IP — treat them as an ARP scan so they keep the IP tag. + with suppress(OperationalError): + await conn.exec_driver_sql( + "UPDATE pending_devices SET discovery_sources = json_array('arp') " + "WHERE discovery_sources IS NULL AND discovery_source IS NULL AND ip IS NOT NULL" + ) + with suppress(OperationalError): + await conn.exec_driver_sql( + "UPDATE pending_devices SET discovery_sources = '[]' WHERE discovery_sources IS NULL" + ) + # Canonicalize stored MACs (lowercase, ':' separators) so cross-source + # dedup can match a Proxmox NIC MAC against an ARP-scanned one by equality. + with suppress(OperationalError): + await conn.exec_driver_sql( + "UPDATE pending_devices SET mac = lower(replace(mac, '-', ':')) WHERE mac IS NOT NULL" + ) + with suppress(OperationalError): + await conn.exec_driver_sql( + "UPDATE nodes SET mac = lower(replace(mac, '-', ':')) WHERE mac IS NOT NULL" + ) async def get_db() -> AsyncGenerator[AsyncSession, None]: diff --git a/backend/app/db/models.py b/backend/app/db/models.py index ac99f89..c656202 100644 --- a/backend/app/db/models.py +++ b/backend/app/db/models.py @@ -119,7 +119,13 @@ class PendingDevice(Base): services: Mapped[list[Any]] = mapped_column(JSON, default=list) suggested_type: Mapped[str | None] = mapped_column(String) status: Mapped[str] = mapped_column(String, default="pending") + # Origin/primary source (first discovery): "arp"/"mdns"/"zigbee"/"zwave"/ + # "proxmox". Kept for back-compat; `discovery_sources` is the full set. discovery_source: Mapped[str | None] = mapped_column(String) + # All sources that have observed this device. A device found by both an IP + # scan and a Proxmox import carries e.g. ["arp", "proxmox"] and shows under + # both inventory filters. Source of truth for the frontend source badges. + discovery_sources: Mapped[list[Any]] = mapped_column(JSON, default=list) ieee_address: Mapped[str | None] = mapped_column(String, index=True, nullable=True, unique=True) friendly_name: Mapped[str | None] = mapped_column(String, nullable=True) device_subtype: Mapped[str | None] = mapped_column(String, nullable=True) diff --git a/backend/app/schemas/scan.py b/backend/app/schemas/scan.py index 0b0827c..8660e64 100644 --- a/backend/app/schemas/scan.py +++ b/backend/app/schemas/scan.py @@ -14,6 +14,9 @@ class PendingDeviceResponse(BaseModel): suggested_type: str | None status: str discovery_source: str | None + # All sources that have observed this device (e.g. ["arp", "proxmox"]). Drives + # the inventory source filter + badges; falls back to [discovery_source]. + discovery_sources: list[str] = [] ieee_address: str | None = None friendly_name: str | None = None device_subtype: str | None = None @@ -35,10 +38,10 @@ class PendingDeviceResponse(BaseModel): node_last_modified: datetime | None = None node_last_seen: datetime | None = None - @field_validator("properties", mode="before") + @field_validator("properties", "discovery_sources", mode="before") @classmethod - def _coerce_properties(cls, v: Any) -> list[Any]: - # Legacy rows (column added by migration) have properties = NULL. + def _coerce_list(cls, v: Any) -> list[Any]: + # Legacy rows (columns added by migration) have these = NULL. return v if isinstance(v, list) else [] model_config = {"from_attributes": True} diff --git a/backend/app/services/discovery_sources.py b/backend/app/services/discovery_sources.py new file mode 100644 index 0000000..e0e5344 --- /dev/null +++ b/backend/app/services/discovery_sources.py @@ -0,0 +1,19 @@ +"""Helpers for the multi-valued ``PendingDevice.discovery_sources`` set. + +A device discovered by more than one path (e.g. an IP scan *and* a Proxmox +import) accumulates every source that has seen it, so it surfaces under each +matching inventory filter. Order is preserved (origin first) and duplicates are +dropped. +""" + +from __future__ import annotations + +from collections.abc import Iterable + + +def add_source(sources: Iterable[str] | None, source: str | None) -> list[str]: + """Return ``sources`` with ``source`` appended if not already present.""" + out = [s for s in (sources or []) if s] + if source and source not in out: + out.append(source) + return out diff --git a/backend/app/services/mac_utils.py b/backend/app/services/mac_utils.py new file mode 100644 index 0000000..b1ed5c3 --- /dev/null +++ b/backend/app/services/mac_utils.py @@ -0,0 +1,18 @@ +"""MAC-address normalization, shared by the scan + Proxmox persist paths. + +Different discovery sources emit MACs in different casing/separators (ARP is +lowercase ``bc:24:11:..``, Proxmox config is often uppercase ``BC:24:11:..``). +Canonicalizing on write *and* on compare lets cross-source dedup match a device +by MAC with a plain ``==`` — the join key for merging an IP-scanned row with a +Proxmox-imported one. +""" + +from __future__ import annotations + + +def normalize_mac(mac: str | None) -> str | None: + """Canonical MAC: lowercase, ``-`` → ``:``, stripped. Blank/None → None.""" + if not mac: + return None + normalized = mac.strip().lower().replace("-", ":") + return normalized or None diff --git a/backend/app/services/proxmox_service.py b/backend/app/services/proxmox_service.py index e246961..9f62662 100644 --- a/backend/app/services/proxmox_service.py +++ b/backend/app/services/proxmox_service.py @@ -18,6 +18,7 @@ from typing import Any import httpx +from app.services.mac_utils import normalize_mac from app.services.zigbee_service import merge_zigbee_properties logger = logging.getLogger(__name__) @@ -32,6 +33,9 @@ _BYTES_PER_GB = 1024 ** 3 # net0 config line: "name=eth0,bridge=vmbr0,ip=192.168.1.5/24,gw=..." _LXC_IP_RE = re.compile(r"(?:^|,)ip=([0-9]{1,3}(?:\.[0-9]{1,3}){3})(?:/\d+)?") +# NIC MAC inside a net0 string — qemu "virtio=BC:24:11:..,bridge=.." or lxc +# "..,hwaddr=BC:24:11:..,..". A bare 6-octet MAC match works for both forms. +_NET_MAC_RE = re.compile(r"([0-9a-fA-F]{2}(?::[0-9a-fA-F]{2}){5})") def _sanitize_proxmox_error(exc: BaseException) -> str: @@ -121,6 +125,21 @@ def _extract_lxc_ip(config_payload: dict[str, Any] | None) -> str | None: return match.group(1) if match else None +def _extract_net_mac(config_payload: dict[str, Any] | None) -> str | None: + """Parse the NIC MAC from a qemu/lxc ``net0`` config string (normalized). + + Works agent-free for both guest kinds and for stopped guests — the sole + identity we can reliably cross-match against an ARP-scanned device. + """ + if not config_payload: + return None + net0 = config_payload.get("net0") + if not isinstance(net0, str): + return None + match = _NET_MAC_RE.search(net0) + return normalize_mac(match.group(1)) if match else None + + def _host_node(raw: dict[str, Any]) -> dict[str, Any] | None: """Build a homelable ``proxmox`` host node from a ``/nodes`` entry.""" name = raw.get("node") @@ -144,7 +163,9 @@ def _host_node(raw: dict[str, Any]) -> dict[str, Any] | None: } -def _guest_node(raw: dict[str, Any], host_name: str, kind: str, ip: str | None) -> dict[str, Any] | None: +def _guest_node( + raw: dict[str, Any], host_name: str, kind: str, ip: str | None, mac: str | None = None +) -> dict[str, Any] | None: """Build a homelable vm/lxc node from a ``/qemu`` or ``/lxc`` list entry.""" vmid = raw.get("vmid") if vmid is None: @@ -159,6 +180,7 @@ def _guest_node(raw: dict[str, Any], host_name: str, kind: str, ip: str | None) "ieee_address": ieee, "hostname": name, "ip": ip, + "mac": mac, "status": "online" if raw.get("status") == "running" else "offline", "cpu_count": _int_or_none(raw.get("maxcpu") or raw.get("cpus")), "ram_gb": _gb(raw.get("maxmem")), @@ -320,34 +342,48 @@ async def _fetch_host_guests( if not isinstance(entries, list): continue for raw in entries: - ip = await _resolve_guest_ip(client, host_name, kind, raw) - node = _guest_node(raw, host_name, kind, ip) + ip, mac = await _resolve_guest_net(client, host_name, kind, raw) + node = _guest_node(raw, host_name, kind, ip, mac) if node: guests.append(node) return guests -async def _resolve_guest_ip( +async def _resolve_guest_net( client: httpx.AsyncClient, host_name: str, kind: str, raw: dict[str, Any] -) -> str | None: - """Best-effort guest IP. qemu → guest agent, lxc → net0 config. Never raises.""" +) -> tuple[str | None, str | None]: + """Best-effort (ip, mac) for a guest. Never raises. + + - MAC comes from the guest ``/config`` net0 line for both kinds (agent-free, + works for stopped guests) — the cross-source dedup key. + - IP: qemu → guest agent (running only); lxc → static net0 config. + Partial results are returned even if a later call fails (e.g. config MAC is + kept when the qemu agent call errors). + """ vmid = raw.get("vmid") if vmid is None: - return None + return None, None + ip: str | None = None + mac: str | None = None try: if kind == "qemu": - if raw.get("status") != "running": - return None - data = await _get_json( - client, f"/nodes/{host_name}/qemu/{vmid}/agent/network-get-interfaces" - ) - return _extract_qemu_ip(data) - data = await _get_json(client, f"/nodes/{host_name}/lxc/{vmid}/config") - return _extract_lxc_ip(data) + config = await _get_json(client, f"/nodes/{host_name}/qemu/{vmid}/config") + mac = _extract_net_mac(config) + if raw.get("status") == "running": + data = await _get_json( + client, f"/nodes/{host_name}/qemu/{vmid}/agent/network-get-interfaces" + ) + ip = _extract_qemu_ip(data) + else: + config = await _get_json(client, f"/nodes/{host_name}/lxc/{vmid}/config") + ip = _extract_lxc_ip(config) + mac = _extract_net_mac(config) except httpx.HTTPError: - # Guest agent not installed / container stopped / no perms → no IP. Fine. - return None + # Guest agent not installed / container stopped / no perms. Keep whatever + # was resolved before the failure. + pass + return ip, mac async def test_proxmox_connection( diff --git a/backend/app/services/scanner.py b/backend/app/services/scanner.py index 2287773..b616f93 100644 --- a/backend/app/services/scanner.py +++ b/backend/app/services/scanner.py @@ -15,8 +15,10 @@ from sqlalchemy import or_, select from sqlalchemy.ext.asyncio import AsyncSession from app.db.models import Node, PendingDevice, ScanRun +from app.services.discovery_sources import add_source from app.services.fingerprint import fingerprint_ports, suggest_node_type from app.services.http_probe import probe_open_ports +from app.services.mac_utils import normalize_mac logger = logging.getLogger(__name__) @@ -522,16 +524,21 @@ async def run_scan( ip, open_ports, verify_tls=deep_scan.verify_tls ) + norm_mac = normalize_mac(host.get("mac")) services = fingerprint_ports(open_ports) - suggested_type = suggest_node_type(open_ports, host.get("mac")) + suggested_type = suggest_node_type(open_ports, norm_mac) - # One inventory row per device (by IP). Match across pending AND - # approved so a re-scan of an already-approved device refreshes its - # row instead of spawning a fresh "pending" duplicate. Hidden rows - # are already skipped above. + # One inventory row per device. Match by IP OR MAC across pending AND + # approved so a re-scan refreshes the existing row instead of spawning + # a duplicate — and so a device previously imported from Proxmox (which + # may have no IP but a known NIC MAC) reconciles with this scan instead + # of doubling up. Hidden rows are already skipped above. + match_cond = [PendingDevice.ip == ip] + if norm_mac: + match_cond.append(PendingDevice.mac == norm_mac) existing_rows = (await db.execute( select(PendingDevice) - .where(PendingDevice.ip == ip, PendingDevice.status != "hidden") + .where(or_(*match_cond), PendingDevice.status != "hidden") .order_by(PendingDevice.discovered_at) )).scalars().all() @@ -543,32 +550,41 @@ async def run_scan( for dup in existing_rows: if dup is not keep: await db.delete(dup) - keep.mac = host.get("mac") or keep.mac + keep.ip = keep.ip or ip # fill an IP a Proxmox import lacked + keep.mac = norm_mac or keep.mac keep.hostname = host.get("hostname") or keep.hostname keep.os = host.get("os") or keep.os keep.services = services - keep.suggested_type = suggested_type + # Don't downgrade a Proxmox-typed guest (vm/lxc) to the generic + # scan guess; the importer knows the true type. + if not (keep.ieee_address or "").startswith("pve-"): + keep.suggested_type = suggested_type + # Merged row carries both sources (e.g. ["proxmox", "arp"]). + keep.discovery_sources = add_source(keep.discovery_sources, discovery_source) # status preserved — an approved device stays approved. else: db.add(PendingDevice( ip=ip, - mac=host.get("mac"), + mac=norm_mac, hostname=host.get("hostname"), os=host.get("os"), services=services, suggested_type=suggested_type, status="pending", discovery_source=discovery_source, + discovery_sources=[discovery_source], )) devices_found += 1 # Stamp last_scan on any canvas node that matches this device by IP # (or MAC, when known) so the inventory shows when the scanner last - # observed it. Matches across designs. - host_mac = host.get("mac") + # observed it. Match both the normalized and raw MAC so a legacy + # canvas node whose mac predates normalization still matches. Across + # designs. node_match = [Node.ip == ip] - if host_mac: - node_match.append(Node.mac == host_mac) + for m in {norm_mac, host.get("mac")}: + if m: + node_match.append(Node.mac == m) matching_nodes = (await db.execute( select(Node).where(or_(*node_match)) )).scalars().all() diff --git a/backend/tests/test_mac_utils.py b/backend/tests/test_mac_utils.py new file mode 100644 index 0000000..465b157 --- /dev/null +++ b/backend/tests/test_mac_utils.py @@ -0,0 +1,27 @@ +"""Unit tests for MAC normalization (the cross-source dedup key).""" + +from __future__ import annotations + +from app.services.discovery_sources import add_source +from app.services.mac_utils import normalize_mac + + +def test_normalize_mac_lowercases_and_unifies_separators() -> None: + assert normalize_mac("BC:24:11:AA:BB:CC") == "bc:24:11:aa:bb:cc" + assert normalize_mac("bc-24-11-aa-bb-cc") == "bc:24:11:aa:bb:cc" + assert normalize_mac(" BC:24:11:AA:BB:CC ") == "bc:24:11:aa:bb:cc" + + +def test_normalize_mac_blank_is_none() -> None: + assert normalize_mac(None) is None + assert normalize_mac("") is None + assert normalize_mac(" ") is None + + +def test_add_source_unions_without_duplicates() -> None: + assert add_source(None, "arp") == ["arp"] + assert add_source(["arp"], "proxmox") == ["arp", "proxmox"] + assert add_source(["arp", "proxmox"], "proxmox") == ["arp", "proxmox"] + assert add_source(["arp"], None) == ["arp"] + # Drops falsy members already present. + assert add_source(["arp", ""], "proxmox") == ["arp", "proxmox"] diff --git a/backend/tests/test_proxmox_router.py b/backend/tests/test_proxmox_router.py index a7a9bb7..b9fe684 100644 --- a/backend/tests/test_proxmox_router.py +++ b/backend/tests/test_proxmox_router.py @@ -3,13 +3,18 @@ from __future__ import annotations import uuid -from unittest.mock import AsyncMock, patch +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock, patch import pytest from httpx import AsyncClient from sqlalchemy import select -from app.api.routes.proxmox import _guest_visibility_advisory, _persist_pending_import +from app.api.routes.proxmox import ( + _background_proxmox_import, + _guest_visibility_advisory, + _persist_pending_import, +) from app.api.routes.scan import _is_proxmox_cluster_member, _resolve_pending_links_for_ieee from app.core.config import settings from app.db.models import Design, Edge, Node, PendingDevice, PendingDeviceLink @@ -41,10 +46,11 @@ def _host_node() -> dict: } -def _guest_node(vmid: int, ip: str | None, status: str = "online") -> dict: +def _guest_node(vmid: int, ip: str | None, status: str = "online", mac: str | None = None) -> dict: return { "id": f"pve-pve1-{vmid}", "label": f"vm{vmid}", "type": "vm", "ieee_address": f"pve-pve1-{vmid}", "hostname": f"vm{vmid}", "ip": ip, + "mac": mac, "status": status, "cpu_count": 2, "ram_gb": 4.0, "disk_gb": 32.0, "vendor": "Proxmox VE", "model": "QEMU", "vmid": vmid, "parent_ieee": "pve-node-pve1", @@ -116,6 +122,27 @@ async def test_enable_sync_without_token_rejected(client: AsyncClient, headers: assert res.status_code == 400 +@pytest.mark.asyncio +async def test_background_import_broadcasts_refresh() -> None: + """After persisting, the import emits a scan update so an open inventory + reloads without a manual refresh (same signal the IP scan uses).""" + fake_db = AsyncMock() + fake_db.get = AsyncMock(return_value=None) # no ScanRun row → skip status update + cm = AsyncMock() + cm.__aenter__.return_value = fake_db + cm.__aexit__.return_value = False + + with patch("app.api.routes.proxmox.AsyncSessionLocal", MagicMock(return_value=cm)), \ + patch("app.api.routes.proxmox.fetch_proxmox_inventory", new=AsyncMock(return_value=([], []))), \ + patch("app.api.routes.proxmox._persist_pending_import", + new=AsyncMock(return_value=SimpleNamespace(device_count=3))), \ + patch("app.api.routes.status.broadcast_scan_update", new=AsyncMock()) as bcast: + await _background_proxmox_import("run1", "h", 8006, "u@pam!t", "s", True) + + bcast.assert_awaited_once() + assert bcast.await_args.kwargs["devices_found"] == 3 + + # --- guest-visibility advisory --------------------------------------------- def test_advisory_when_hosts_only() -> None: @@ -172,6 +199,108 @@ async def test_persist_merges_existing_scanned_node_by_ip(db_session) -> None: assert inv.status == "approved" +@pytest.mark.asyncio +async def test_persist_merges_pending_scan_row_by_mac(db_session) -> None: + # Device previously found by an IP scan: arp source, MAC known, no ieee. + db_session.add(PendingDevice( + id=str(uuid.uuid4()), ip="10.0.0.5", mac="bc:24:11:aa:bb:cc", + suggested_type="generic", status="pending", + discovery_source="arp", discovery_sources=["arp"], + )) + await db_session.commit() + + # Proxmox import of the same box (stopped VM → no IP) but same NIC MAC in a + # different casing. Must merge, not duplicate. + await _persist_pending_import(db_session, [_guest_node(101, None, mac="BC:24:11:AA:BB:CC")], []) + + rows = (await db_session.execute(select(PendingDevice))).scalars().all() + assert len(rows) == 1 + row = rows[0] + assert row.ieee_address == "pve-pve1-101" # adopted proxmox identity + assert row.suggested_type == "vm" # kept proxmox type + assert set(row.discovery_sources) == {"arp", "proxmox"} # shows in both filters + + +@pytest.mark.asyncio +async def test_persist_preserves_ip_tag_for_legacy_null_source_row(db_session) -> None: + # Legacy inventory row from an old IP scan, before discovery_source(s) were + # recorded: scalar NULL, sources empty — but it has an IP + MAC. + db_session.add(PendingDevice( + id=str(uuid.uuid4()), ip="192.168.1.108", mac="bc:24:11:6c:96:52", + suggested_type="lxc", status="pending", + discovery_source=None, discovery_sources=[], + )) + await db_session.commit() + + await _persist_pending_import( + db_session, [_guest_node(108, "192.168.1.108", mac="BC:24:11:6C:96:52")], [] + ) + + rows = (await db_session.execute(select(PendingDevice))).scalars().all() + assert len(rows) == 1 + # The IP tag must survive the proxmox merge even with no recorded origin. + assert set(rows[0].discovery_sources) == {"arp", "proxmox"} + + +@pytest.mark.asyncio +async def test_persist_preserves_ip_tag_on_canvas_merge_legacy_row(db_session) -> None: + # The immich case: an on-canvas node from an old scan, with a legacy + # inventory row (NULL source). Merge must keep the IP tag on the row. + node = Node( + id=str(uuid.uuid4()), type="lxc", label="immich", + ip="192.168.1.108", mac="bc:24:11:6c:96:52", + status="online", pos_x=0, pos_y=0, + ) + inv = PendingDevice( + id=str(uuid.uuid4()), ip="192.168.1.108", mac="bc:24:11:6c:96:52", + suggested_type="lxc", status="approved", + discovery_source=None, discovery_sources=[], + ) + db_session.add_all([node, inv]) + await db_session.commit() + + await _persist_pending_import( + db_session, [_guest_node(108, "192.168.1.108", mac="BC:24:11:6C:96:52")], [] + ) + + inv_row = (await db_session.execute( + select(PendingDevice).where(PendingDevice.mac == "bc:24:11:6c:96:52") + )).scalar_one() + assert set(inv_row.discovery_sources) == {"arp", "proxmox"} + + +@pytest.mark.asyncio +async def test_persist_does_not_add_ip_tag_to_pure_proxmox_guest(db_session) -> None: + # A guest first seen via Proxmox (agent IP, pve ieee) must NOT gain a spurious + # IP tag on re-sync — it was never IP-scanned. + await _persist_pending_import(db_session, [_guest_node(101, "10.0.0.5")], []) + await _persist_pending_import(db_session, [_guest_node(101, "10.0.0.5")], []) # re-sync + row = (await db_session.execute( + select(PendingDevice).where(PendingDevice.ieee_address == "pve-pve1-101") + )).scalar_one() + assert set(row.discovery_sources) == {"proxmox"} + + +@pytest.mark.asyncio +async def test_persist_merges_canvas_node_by_mac(db_session) -> None: + # A scanned canvas node with a MAC but no IP recorded for the guest. + scanned = Node( + id=str(uuid.uuid4()), type="generic", label="box", + mac="bc:24:11:aa:bb:cc", status="online", pos_x=0, pos_y=0, + ) + db_session.add(scanned) + await db_session.commit() + + await _persist_pending_import(db_session, [_guest_node(101, None, mac="BC:24:11:AA:BB:CC")], []) + + nodes = (await db_session.execute(select(Node))).scalars().all() + assert len(nodes) == 1 # no duplicate node + merged = nodes[0] + assert merged.ieee_address == "pve-pve1-101" + assert merged.mac == "bc:24:11:aa:bb:cc" + assert merged.cpu_count == 2 # specs backfilled + + @pytest.mark.asyncio async def test_persist_resync_updates_in_place(db_session) -> None: nodes = [_guest_node(101, "10.0.0.5")] diff --git a/backend/tests/test_proxmox_service.py b/backend/tests/test_proxmox_service.py index afda931..321a6be 100644 --- a/backend/tests/test_proxmox_service.py +++ b/backend/tests/test_proxmox_service.py @@ -38,6 +38,47 @@ def test_extract_lxc_ip_parses_net0_static() -> None: assert svc._extract_lxc_ip(None) is None +def test_extract_net_mac_parses_qemu_and_lxc_forms() -> None: + # qemu: "virtio=,bridge=.." + assert svc._extract_net_mac({"net0": "virtio=BC:24:11:AA:BB:CC,bridge=vmbr0"}) == "bc:24:11:aa:bb:cc" + # lxc: "..,hwaddr=,.." + assert ( + svc._extract_net_mac({"net0": "name=eth0,bridge=vmbr0,hwaddr=BC:24:11:11:22:33,ip=dhcp"}) + == "bc:24:11:11:22:33" + ) + # No MAC / missing net0 / None → None + assert svc._extract_net_mac({"net0": "name=eth0,ip=dhcp"}) is None + assert svc._extract_net_mac({}) is None + assert svc._extract_net_mac(None) is None + + +@pytest.mark.asyncio +async def test_fetch_inventory_captures_guest_mac_from_config() -> None: + """Guests carry a normalized NIC MAC read agent-free from their config.""" + async def fake_get_json(client, path: str): + if path == "/nodes": + return [{"node": "pve1", "status": "online", "maxcpu": 8}] + if path == "/nodes/pve1/qemu": + return [{"vmid": 101, "name": "web", "status": "stopped"}] # stopped: no agent IP + if path == "/nodes/pve1/lxc": + return [{"vmid": 200, "name": "db", "status": "running"}] + if path == "/nodes/pve1/qemu/101/config": + return {"net0": "virtio=AA:BB:CC:DD:EE:FF,bridge=vmbr0"} + if path == "/nodes/pve1/lxc/200/config": + return {"net0": "name=eth0,bridge=vmbr0,hwaddr=11:22:33:44:55:66,ip=10.0.0.6/24"} + return None + + with patch.object(svc, "_get_json", new=AsyncMock(side_effect=fake_get_json)): + nodes, _ = await svc.fetch_proxmox_inventory("h", 8006, "u@pam!t", "sec") + + vm = next(n for n in nodes if n["type"] == "vm") + assert vm["mac"] == "aa:bb:cc:dd:ee:ff" # captured even though VM is stopped (no IP) + assert vm["ip"] is None + ct = next(n for n in nodes if n["type"] == "lxc") + assert ct["mac"] == "11:22:33:44:55:66" + assert ct["ip"] == "10.0.0.6" + + def test_host_and_guest_node_mapping() -> None: host = svc._host_node({"node": "pve1", "status": "online", "maxcpu": 8, "maxmem": 16 * 1024 ** 3, "maxdisk": 500 * 1024 ** 3}) assert host["type"] == "proxmox" diff --git a/backend/tests/test_scanner.py b/backend/tests/test_scanner.py index 5153282..2b296ef 100644 --- a/backend/tests/test_scanner.py +++ b/backend/tests/test_scanner.py @@ -577,6 +577,44 @@ async def test_run_scan_mdns_only_device_added(mem_db): assert device.discovery_source == "mdns" +@pytest.mark.asyncio +async def test_run_scan_merges_proxmox_row_by_mac(mem_db): + """A scan reconciles a prior Proxmox-imported row by MAC: fills the IP, + unions the source, keeps the vm type, and does not duplicate.""" + from app.services.scanner import run_scan + + run_id = _make_run_id() + async with mem_db() as session: + session.add(_make_scan_run(run_id)) + # Previously imported from Proxmox: no IP, known NIC MAC, vm type. + session.add(PendingDevice( + id="pve-row", ieee_address="pve-pve1-101", ip=None, + mac="bc:24:11:aa:bb:cc", suggested_type="vm", status="pending", + discovery_source="proxmox", discovery_sources=["proxmox"], + )) + await session.commit() + + # Scan sees the same box (same MAC, different casing) with a live IP. + nmap_hosts = [{"ip": "192.168.1.50", "hostname": "web.lan", + "mac": "BC:24:11:AA:BB:CC", "os": None, "open_ports": []}] + + async with mem_db() as session: + with patch("app.services.scanner._nmap_scan", return_value=nmap_hosts), \ + patch("app.services.scanner._mdns_discover", new_callable=AsyncMock, return_value=[]), \ + patch("app.api.routes.status.broadcast_scan_update", new_callable=AsyncMock): + await run_scan(["192.168.1.0/24"], session, run_id) + + async with mem_db() as session: + rows = (await session.execute(sa_select(PendingDevice))).scalars().all() + + assert len(rows) == 1 # merged, not duplicated + row = rows[0] + assert row.ip == "192.168.1.50" # scan filled the IP + assert row.mac == "bc:24:11:aa:bb:cc" # normalized + assert row.suggested_type == "vm" # kept proxmox type + assert set(row.discovery_sources) == {"proxmox", "arp"} # both filters + + @pytest.mark.asyncio async def test_run_scan_mdns_skipped_if_already_in_nmap(mem_db): """If nmap and mDNS both find the same IP, it should not be double-counted.""" diff --git a/frontend/src/components/modals/PendingDeviceModal.tsx b/frontend/src/components/modals/PendingDeviceModal.tsx index 3d9170b..8f98975 100644 --- a/frontend/src/components/modals/PendingDeviceModal.tsx +++ b/frontend/src/components/modals/PendingDeviceModal.tsx @@ -21,6 +21,10 @@ export interface PendingDevice { suggested_type: string | null status: string discovery_source: string | null + // All sources that have observed this device (e.g. ["arp", "proxmox"]). A + // merged device shows under every matching filter. Falls back to + // [discovery_source] when absent (older rows). + discovery_sources?: string[] ieee_address?: string | null friendly_name?: string | null device_subtype?: string | null diff --git a/frontend/src/components/modals/PendingDevicesModal.tsx b/frontend/src/components/modals/PendingDevicesModal.tsx index 338384e..5aeadc9 100644 --- a/frontend/src/components/modals/PendingDevicesModal.tsx +++ b/frontend/src/components/modals/PendingDevicesModal.tsx @@ -18,6 +18,7 @@ import { buildZwaveProperties, isZwaveType } from '@/utils/zwaveProperties' import { buildMacProperty } from '@/utils/macProperty' import { formatRelative, formatTimestamp } from '@/utils/timeFormat' import { getCenteredPosition } from '@/utils/viewportCenter' +import { sourceBuckets, orderedSources, SOURCE_META, type SourceBucket } from '@/utils/pendingSources' interface PendingDevicesModalProps { open: boolean @@ -74,18 +75,9 @@ const TYPE_ICONS: Record = { generic: Circle, } -type SourceFilter = 'all' | 'ip' | 'zigbee' | 'zwave' | 'proxmox' +type SourceFilter = 'all' | SourceBucket type StatusFilter = 'pending' | 'hidden' -function inferSource(d: PendingDevice): 'zigbee' | 'zwave' | 'proxmox' | 'ip' { - if (d.discovery_source === 'zwave') return 'zwave' - if (d.discovery_source === 'zigbee') return 'zigbee' - if (d.discovery_source === 'proxmox') return 'proxmox' - // Proxmox devices carry a synthetic 'pve-' ieee but are IP hosts, not mesh. - if (d.ieee_address && !d.ieee_address.startsWith('pve-')) return 'zigbee' - return 'ip' -} - const COMMON_PORTS = new Set([22, 80, 443]) function specialServiceName(d: PendingDevice): string | undefined { @@ -162,7 +154,7 @@ export function PendingDevicesModal({ open, onClose, highlightId, initialStatus const filtered = useMemo(() => { const q = search.trim().toLowerCase() return devices.filter((d) => { - if (sourceFilter !== 'all' && inferSource(d) !== sourceFilter) return false + if (sourceFilter !== 'all' && !sourceBuckets(d).has(sourceFilter)) return false if (typeFilter !== 'all' && d.suggested_type !== typeFilter) return false // Inventory-only: optionally hide devices already placed on a canvas. if (statusFilter === 'pending' && !showOnCanvas && (d.canvas_count ?? 0) > 0) return false @@ -656,7 +648,7 @@ interface DeviceCardProps { } function DeviceCard({ device, selected, selectMode, highlighted, onClick, cardRef }: DeviceCardProps) { - const source = inferSource(device) + const sources = orderedSources(device) const roleType = (device.suggested_type ?? 'generic') as NodeType const Icon = TYPE_ICONS[roleType] ?? Circle const activeTheme = useThemeStore((s) => s.activeTheme) @@ -664,16 +656,6 @@ function DeviceCard({ device, selected, selectMode, highlighted, onClick, cardRe // (from the active theme / style section), instead of a flat grey. const roleColor = resolveNodeColors({ type: roleType, custom_colors: undefined }, activeTheme).border const label = deviceLabel(device) - const sourceColor = - source === 'zigbee' ? '#00d4ff' - : source === 'zwave' ? '#ff6e00' - : source === 'proxmox' ? '#e57000' - : '#a855f7' - const sourceLabel = - source === 'zigbee' ? 'ZIGBEE' - : source === 'zwave' ? 'Z-WAVE' - : source === 'proxmox' ? 'PROXMOX' - : (device.discovery_source ?? 'IP').toUpperCase() const services = device.services ?? [] const visibleServices = services.slice(0, 4) const moreServices = services.length - visibleServices.length @@ -737,12 +719,15 @@ function DeviceCard({ device, selected, selectMode, highlighted, onClick, cardRe
{label}
- - {sourceLabel} - + {sources.map((s) => ( + + {SOURCE_META[s].label} + + ))} {device.suggested_type && ( = {}): PendingDevice { + return { + id: 'd1', + ip: null, + mac: null, + hostname: null, + os: null, + services: [], + suggested_type: null, + status: 'pending', + discovery_source: null, + discovered_at: '2026-07-05T00:00:00Z', + ...overrides, + } +} + +describe('sourceBuckets', () => { + it('returns both IP and Proxmox for a merged device', () => { + const buckets = sourceBuckets(device({ discovery_sources: ['arp', 'proxmox'] })) + expect([...buckets].sort()).toEqual(['ip', 'proxmox']) + }) + + it('maps arp and mdns to the single ip bucket', () => { + expect([...sourceBuckets(device({ discovery_sources: ['arp'] }))]).toEqual(['ip']) + expect([...sourceBuckets(device({ discovery_sources: ['mdns'] }))]).toEqual(['ip']) + }) + + it('falls back to legacy discovery_source when discovery_sources is empty', () => { + expect([...sourceBuckets(device({ discovery_source: 'zigbee' }))]).toEqual(['zigbee']) + expect([...sourceBuckets(device({ discovery_source: 'proxmox' }))]).toEqual(['proxmox']) + }) + + it('uses the ieee heuristic when no source is recorded', () => { + // Mesh device (non-pve ieee) with no discovery_source → zigbee. + expect([...sourceBuckets(device({ ieee_address: '0x00124b00' }))]).toEqual(['zigbee']) + // Nothing at all → ip. + expect([...sourceBuckets(device())]).toEqual(['ip']) + }) +}) + +describe('orderedSources', () => { + it('renders IP before Proxmox regardless of input order', () => { + expect(orderedSources(device({ discovery_sources: ['proxmox', 'arp'] }))).toEqual(['ip', 'proxmox']) + }) +}) diff --git a/frontend/src/utils/pendingSources.ts b/frontend/src/utils/pendingSources.ts new file mode 100644 index 0000000..a2e4f1b --- /dev/null +++ b/frontend/src/utils/pendingSources.ts @@ -0,0 +1,47 @@ +/** Discovery-source bucketing for pending inventory devices. + * + * A device may be observed by more than one discovery path (e.g. an IP scan and + * a Proxmox import); `discovery_sources` holds every one. These helpers map that + * raw list to the UI's filter/badge buckets so a merged device shows under each + * matching filter and renders one badge per source. + */ +import type { PendingDevice } from '@/components/modals/PendingDeviceModal' + +export type SourceBucket = 'ip' | 'zigbee' | 'zwave' | 'proxmox' + +export const SOURCE_META: Record = { + zigbee: { color: '#00d4ff', label: 'ZIGBEE' }, + zwave: { color: '#ff6e00', label: 'Z-WAVE' }, + proxmox: { color: '#e57000', label: 'PROXMOX' }, + ip: { color: '#a855f7', label: 'IP' }, +} + +// Stable badge order (IP first — it's the primary discovery path). +const SOURCE_ORDER: SourceBucket[] = ['ip', 'proxmox', 'zigbee', 'zwave'] + +/** Every source bucket that has observed this device. A device found by both an + * IP scan and a Proxmox import returns {ip, proxmox}. */ +export function sourceBuckets(d: PendingDevice): Set { + const raw = d.discovery_sources && d.discovery_sources.length > 0 + ? d.discovery_sources + : d.discovery_source ? [d.discovery_source] : [] + const buckets = new Set() + for (const s of raw) { + if (s === 'zwave') buckets.add('zwave') + else if (s === 'zigbee') buckets.add('zigbee') + else if (s === 'proxmox') buckets.add('proxmox') + else buckets.add('ip') // arp / mdns / anything else → IP scan + } + if (buckets.size === 0) { + // No source recorded — legacy heuristic (mesh rows carry a non-pve ieee). + if (d.ieee_address && !d.ieee_address.startsWith('pve-')) buckets.add('zigbee') + else buckets.add('ip') + } + return buckets +} + +/** Ordered bucket list for badge rendering. */ +export function orderedSources(d: PendingDevice): SourceBucket[] { + const buckets = sourceBuckets(d) + return SOURCE_ORDER.filter((b) => buckets.has(b)) +}