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=<MAC>, lxc hwaddr=<MAC>); 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
This commit is contained in:
Pouzor
@@ -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"]
|
||||
@@ -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")]
|
||||
|
||||
@@ -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=<MAC>,bridge=.."
|
||||
assert svc._extract_net_mac({"net0": "virtio=BC:24:11:AA:BB:CC,bridge=vmbr0"}) == "bc:24:11:aa:bb:cc"
|
||||
# lxc: "..,hwaddr=<MAC>,.."
|
||||
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"
|
||||
|
||||
@@ -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."""
|
||||
|
||||
Reference in New Issue
Block a user