diff --git a/backend/app/api/routes/proxmox.py b/backend/app/api/routes/proxmox.py index 6abf8c5..2986777 100644 --- a/backend/app/api/routes/proxmox.py +++ b/backend/app/api/routes/proxmox.py @@ -34,6 +34,7 @@ from app.schemas.proxmox import ( from app.schemas.scan import ScanRunResponse from app.services.node_dedupe import dedupe_nodes_by_ieee from app.services.proxmox_service import ( + build_proxmox_cluster_links, build_proxmox_properties, fetch_proxmox_inventory, merge_proxmox_properties, @@ -43,6 +44,11 @@ from app.services.proxmox_service import ( logger = logging.getLogger(__name__) router = APIRouter() +# Discovery sources for the two proxmox link shapes. Host→guest links render as +# 'virtual' edges; host↔host cluster links render as 'cluster' edges. +_PROXMOX_GUEST_SOURCE = "proxmox" +_PROXMOX_CLUSTER_SOURCE = "proxmox_cluster" + def _resolve_credentials(payload: ProxmoxConnectionRequest) -> tuple[str, str]: """Pick the API token: request body first, else server env config. @@ -156,6 +162,7 @@ async def _background_proxmox_import( run.status = "done" run.devices_found = result.device_count run.finished_at = datetime.now(timezone.utc) + run.error = _guest_visibility_advisory(nodes_raw) await db.commit() except Exception as exc: logger.exception("Proxmox import %s failed", run_id) @@ -168,6 +175,27 @@ async def _background_proxmox_import( await db.commit() +def _guest_visibility_advisory(nodes_raw: list[dict[str, Any]]) -> str | None: + """Non-fatal advisory when hosts import but no VMs/LXC were visible. + + A Proxmox API token that lacks ``VM.Audit`` sees empty ``qemu``/``lxc`` lists + (HTTP 200, no error), so only host nodes come through. The usual cause is a + privilege-separated token whose effective rights are the *intersection* with + the user's rights — granting PVEAuditor to the token alone is not enough when + the user has none. Surface that instead of a silent success. + """ + hosts = sum(1 for n in nodes_raw if n.get("type") == "proxmox") + guests = len(nodes_raw) - hosts + if hosts and not guests: + return ( + f"Imported {hosts} host(s) but no VMs or LXC were visible to the API " + "token. Grant the PVEAuditor role at path '/' to BOTH the token and " + "the user (privilege-separated tokens get the intersection of token " + "and user rights), then re-import." + ) + return None + + async def _persist_pending_import( db: AsyncSession, nodes_raw: list[dict[str, Any]], @@ -184,6 +212,9 @@ async def _persist_pending_import( """ await dedupe_nodes_by_ieee(db) + cluster_pairs = build_proxmox_cluster_links(nodes_raw) + cluster_members = {ieee for pair in cluster_pairs for ieee in pair} + pending_created = 0 pending_updated = 0 @@ -215,6 +246,11 @@ async def _persist_pending_import( en.cpu_count = en.cpu_count or n.get("cpu_count") en.ram_gb = en.ram_gb or n.get("ram_gb") en.disk_gb = en.disk_gb or n.get("disk_gb") + # A cluster host needs one left + one right handle for the + # cluster edge endpoints (both default to 0). + 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) pending_updated += 1 continue @@ -228,7 +264,7 @@ async def _persist_pending_import( _refresh_pending(pending, ieee, ip, n, props) pending_updated += 1 - links_recorded = await _replace_links(db, edges_raw) + links_recorded = await _replace_links(db, edges_raw, cluster_pairs) await db.commit() return ProxmoxImportPendingResponse( @@ -309,27 +345,37 @@ async def _ensure_inventory_row( inv.properties = merge_proxmox_properties(list(inv.properties or []), props) -async def _replace_links(db: AsyncSession, edges_raw: list[dict[str, Any]]) -> int: - """Wipe all proxmox-source links and re-insert the freshly discovered set.""" +async def _replace_links( + db: AsyncSession, + edges_raw: list[dict[str, Any]], + cluster_pairs: list[tuple[str, str]], +) -> int: + """Wipe all proxmox-source links and re-insert the freshly discovered set. + + Two link shapes: host→guest (``proxmox`` → 'virtual' edges) and host↔host + (``proxmox_cluster`` → 'cluster' edges). + """ await db.execute( - sa_delete(PendingDeviceLink).where(PendingDeviceLink.discovery_source == "proxmox") + sa_delete(PendingDeviceLink).where( + PendingDeviceLink.discovery_source.in_([_PROXMOX_GUEST_SOURCE, _PROXMOX_CLUSTER_SOURCE]) + ) ) recorded = 0 seen: set[tuple[str, str]] = set() - for e in edges_raw: - src = e.get("source") - tgt = e.get("target") + + def _add(src: str | None, tgt: str | None, source: str) -> None: + nonlocal recorded if not src or not tgt or (src, tgt) in seen: - continue + return seen.add((src, tgt)) - db.add( - PendingDeviceLink( - source_ieee=src, - target_ieee=tgt, - discovery_source="proxmox", - ) - ) + db.add(PendingDeviceLink(source_ieee=src, target_ieee=tgt, discovery_source=source)) recorded += 1 + + for e in edges_raw: + _add(e.get("source"), e.get("target"), _PROXMOX_GUEST_SOURCE) + for src, tgt in cluster_pairs: + _add(src, tgt, _PROXMOX_CLUSTER_SOURCE) + return recorded diff --git a/backend/app/api/routes/scan.py b/backend/app/api/routes/scan.py index 501d95e..219a26b 100644 --- a/backend/app/api/routes/scan.py +++ b/backend/app/api/routes/scan.py @@ -356,6 +356,7 @@ async def bulk_approve_devices( device.status = "approved" node_type = device.suggested_type or "generic" is_wireless = _is_wireless(node_type) + cluster_host = await _is_proxmox_cluster_member(db, device.ieee_address) node = Node( label=device.hostname or device.friendly_name or device.ip or "device", type=node_type, @@ -371,6 +372,9 @@ async def bulk_approve_devices( # Default to ping so the status checker actually polls the new node. # Without this the scheduler skips it (check_method NULL → no check). check_method="none" if is_wireless else ("ping" if device.ip else None), + # Cluster hosts get side handles for their host↔host cluster edge. + left_handles=1 if cluster_host else 0, + right_handles=1 if cluster_host else 0, design_id=default_design_id, ) db.add(node) @@ -508,6 +512,7 @@ async def approve_device( # Prefer the MAC discovered during the scan (stored on the pending device); # fall back to whatever the approve payload carried. _mac = device.mac or node_data.mac + cluster_host = await _is_proxmox_cluster_member(db, device.ieee_address) node = Node( label=node_data.label, type=node_data.type, @@ -525,6 +530,9 @@ async def approve_device( ), check_method="none" if wireless else (node_data.check_method or ("ping" if node_data.ip else None)), check_target=None if wireless else node_data.check_target, + # Cluster hosts get side handles for their host↔host cluster edge. + left_handles=1 if cluster_host else 0, + right_handles=1 if cluster_host else 0, design_id=node_design_id, ) db.add(node) @@ -542,6 +550,28 @@ async def approve_device( } +async def _is_proxmox_cluster_member(db: AsyncSession, ieee: str | None) -> bool: + """True if ``ieee`` participates in a proxmox_cluster link (host↔host). + + Such a host needs one left + one right handle for the cluster edge endpoints + (both default to 0). Checked at approve time, before the link is consumed by + ``_resolve_pending_links_for_ieee``. + """ + if not ieee: + return False + found = ( + await db.execute( + select(PendingDeviceLink.id) + .where( + PendingDeviceLink.discovery_source == "proxmox_cluster", + (PendingDeviceLink.source_ieee == ieee) | (PendingDeviceLink.target_ieee == ieee), + ) + .limit(1) + ) + ).scalar() + return found is not None + + async def _resolve_pending_links_for_ieee( db: AsyncSession, ieee: str | None ) -> list[dict[str, str]]: @@ -612,15 +642,22 @@ async def _resolve_pending_links_for_ieee( if edge_design_id is None: first = (await db.execute(select(Design).order_by(Design.created_at).limit(1))).scalar() edge_design_id = first.id if first else None - # Proxmox host→guest links render as 'virtual' (VM/LXC ↔ host); mesh - # links stay 'iot'. Default to iot for any legacy/other source. - edge_type = "virtual" if link.discovery_source == "proxmox" else "iot" + # Edge shape by link source: + # proxmox → 'virtual' host→guest, vertical (bottom → top) + # proxmox_cluster → 'cluster' host↔host, horizontal (right → left) + # anything else → 'iot' mesh link, vertical + if link.discovery_source == "proxmox": + edge_type, src_handle, tgt_handle = "virtual", "bottom", "top-t" + elif link.discovery_source == "proxmox_cluster": + edge_type, src_handle, tgt_handle = "cluster", "right", "left-t" + else: + edge_type, src_handle, tgt_handle = "iot", "bottom", "top-t" edge = Edge( source=src_id, target=tgt_id, type=edge_type, - source_handle="bottom", - target_handle="top-t", + source_handle=src_handle, + target_handle=tgt_handle, design_id=edge_design_id, ) db.add(edge) diff --git a/backend/app/services/proxmox_service.py b/backend/app/services/proxmox_service.py index 423be85..e246961 100644 --- a/backend/app/services/proxmox_service.py +++ b/backend/app/services/proxmox_service.py @@ -192,6 +192,21 @@ def build_proxmox_properties(node: dict[str, Any]) -> list[dict[str, Any]]: return props +def build_proxmox_cluster_links(nodes: list[dict[str, Any]]) -> list[tuple[str, str]]: + """Chain host nodes (``type == 'proxmox'``) into cluster links. + + Hosts from one import belong to the same cluster, so they are linked + host↔host (rendered as ``cluster`` edges via left/right handles, distinct + from the vertical host→guest ``virtual`` edges). Returns consecutive + ``(source_ieee, target_ieee)`` pairs, or ``[]`` for a single host. Mirrors + the frontend ``buildProxmoxClusterEdges``. + """ + hosts = [n["ieee_address"] for n in nodes if n.get("type") == "proxmox" and n.get("ieee_address")] + if len(hosts) < 2: + return [] + return [(hosts[i], hosts[i + 1]) for i in range(len(hosts) - 1)] + + def _parse_inventory( hosts_raw: list[dict[str, Any]], guests_by_host: dict[str, list[dict[str, Any]]], @@ -231,6 +246,23 @@ async def _get_json(client: httpx.AsyncClient, path: str) -> Any: return resp.json().get("data") +async def _token_has_permissions(client: httpx.AsyncClient) -> bool: + """True if the API token holds *any* ACL. + + Proxmox list endpoints (``/qemu``, ``/lxc``) silently return an empty + ``200`` when the token lacks ``VM.Audit`` — indistinguishable from a host + that genuinely has no guests. ``GET /access/permissions`` returns ``{}`` for + a token with no ACL at all, which is the common misconfiguration (a + privilege-separated token created without its own permission). Best-effort: + on any error assume permissions exist so we never block a valid import. + """ + try: + perms = await _get_json(client, "/access/permissions") + except httpx.HTTPError: + return True + return bool(perms) if isinstance(perms, dict) else True + + async def fetch_proxmox_inventory( host: str, port: int, @@ -339,8 +371,16 @@ async def test_proxmox_connection( timeout=timeout, ) as client: data = await _get_json(client, "/version") + has_perms = await _token_has_permissions(client) version = (data or {}).get("version", "?") if isinstance(data, dict) else "?" - return True, f"Connected to Proxmox VE {version}" + message = f"Connected to Proxmox VE {version}" + if not has_perms: + message += ( + " — warning: this API token has no permissions, so VMs and LXC " + "will not be visible. Assign the PVEAuditor role at path '/' to the " + "token in Proxmox (Datacenter → Permissions → API Token Permission)." + ) + return True, message except httpx.HTTPError as exc: return False, _sanitize_proxmox_error(exc) except Exception as exc: # noqa: BLE001 — surface a safe message, log the rest diff --git a/backend/tests/test_proxmox_router.py b/backend/tests/test_proxmox_router.py index 115d03d..7d6c135 100644 --- a/backend/tests/test_proxmox_router.py +++ b/backend/tests/test_proxmox_router.py @@ -9,9 +9,10 @@ import pytest from httpx import AsyncClient from sqlalchemy import select -from app.api.routes.proxmox import _persist_pending_import +from app.api.routes.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 Node, PendingDevice +from app.db.models import Design, Edge, Node, PendingDevice, PendingDeviceLink @pytest.fixture @@ -115,6 +116,22 @@ async def test_enable_sync_without_token_rejected(client: AsyncClient, headers: assert res.status_code == 400 +# --- guest-visibility advisory --------------------------------------------- + +def test_advisory_when_hosts_only() -> None: + msg = _guest_visibility_advisory([_host_node()]) + assert msg is not None + assert "PVEAuditor" in msg + + +def test_no_advisory_when_guests_present() -> None: + assert _guest_visibility_advisory([_host_node(), _guest_node(101, "10.0.0.5")]) is None + + +def test_no_advisory_when_nothing_imported() -> None: + assert _guest_visibility_advisory([]) is None + + # --- persistence / dedupe -------------------------------------------------- @pytest.mark.asyncio @@ -193,6 +210,64 @@ async def test_pending_endpoint_tolerates_legacy_null_properties(client: AsyncCl assert res.json()[0]["properties"] == [] +# --- cluster links --------------------------------------------------------- + +@pytest.mark.asyncio +async def test_persist_records_cluster_links_between_hosts(db_session) -> None: + # Two hosts + a guest → one host↔host cluster link, one host→guest link. + nodes = [ + {**_host_node(), "id": "pve-node-a", "ieee_address": "pve-node-a", "hostname": "a", "label": "a"}, + {**_host_node(), "id": "pve-node-b", "ieee_address": "pve-node-b", "hostname": "b", "label": "b"}, + _guest_node(101, "10.0.0.5"), + ] + edges = [{"source": "pve-node-pve1", "target": "pve-pve1-101"}] + await _persist_pending_import(db_session, nodes, edges) + + links = (await db_session.execute(select(PendingDeviceLink))).scalars().all() + cluster = [ln for ln in links if ln.discovery_source == "proxmox_cluster"] + assert len(cluster) == 1 + assert (cluster[0].source_ieee, cluster[0].target_ieee) == ("pve-node-a", "pve-node-b") + # Membership helper sees both endpoints. + assert await _is_proxmox_cluster_member(db_session, "pve-node-a") is True + assert await _is_proxmox_cluster_member(db_session, "pve-node-b") is True + assert await _is_proxmox_cluster_member(db_session, "pve-pve1-101") is False + + +@pytest.mark.asyncio +async def test_single_host_records_no_cluster_link(db_session) -> None: + await _persist_pending_import(db_session, [_host_node()], []) + links = (await db_session.execute( + select(PendingDeviceLink).where(PendingDeviceLink.discovery_source == "proxmox_cluster") + )).scalars().all() + assert links == [] + + +@pytest.mark.asyncio +async def test_cluster_link_resolves_to_cluster_edge(db_session) -> None: + # Two host nodes already on a canvas + a pending cluster link between them. + design = Design(id=str(uuid.uuid4()), name="d") + db_session.add(design) + a = Node(id=str(uuid.uuid4()), type="proxmox", label="a", ieee_address="pve-node-a", + status="online", pos_x=0, pos_y=0, design_id=design.id, + left_handles=1, right_handles=1) + b = Node(id=str(uuid.uuid4()), type="proxmox", label="b", ieee_address="pve-node-b", + status="online", pos_x=0, pos_y=0, design_id=design.id, + left_handles=1, right_handles=1) + db_session.add_all([a, b]) + db_session.add(PendingDeviceLink( + id=str(uuid.uuid4()), source_ieee="pve-node-a", target_ieee="pve-node-b", + discovery_source="proxmox_cluster", + )) + await db_session.commit() + + await _resolve_pending_links_for_ieee(db_session, "pve-node-a") + + edge = (await db_session.execute(select(Edge))).scalars().one() + assert edge.type == "cluster" + assert edge.source_handle == "right" + assert edge.target_handle == "left-t" + + @pytest.mark.asyncio async def test_persist_never_deletes(db_session) -> None: await _persist_pending_import(db_session, [_guest_node(101, "10.0.0.5")], []) diff --git a/backend/tests/test_proxmox_service.py b/backend/tests/test_proxmox_service.py index ca2913e..afda931 100644 --- a/backend/tests/test_proxmox_service.py +++ b/backend/tests/test_proxmox_service.py @@ -75,6 +75,25 @@ def test_parse_inventory_builds_host_guest_edges() -> None: assert edges == [{"source": "pve-node-pve1", "target": "pve-pve1-101"}] +def test_build_cluster_links_chains_hosts() -> None: + nodes = [ + svc._host_node({"node": "pve-a", "status": "online"}), + svc._guest_node({"vmid": 101, "status": "running"}, "pve-a", "qemu", None), + svc._host_node({"node": "pve-b", "status": "online"}), + svc._host_node({"node": "pve-c", "status": "online"}), + ] + pairs = svc.build_proxmox_cluster_links(nodes) + assert pairs == [("pve-node-pve-a", "pve-node-pve-b"), ("pve-node-pve-b", "pve-node-pve-c")] + + +def test_build_cluster_links_single_host_is_not_a_cluster() -> None: + nodes = [svc._host_node({"node": "pve-a", "status": "online"})] + assert svc.build_proxmox_cluster_links(nodes) == [] + # Guests alone never form a cluster. + guest = svc._guest_node({"vmid": 1, "status": "running"}, "pve-a", "qemu", None) + assert svc.build_proxmox_cluster_links([guest]) == [] + + def test_sanitize_error_hides_credentials() -> None: exc = httpx.HTTPStatusError( "boom", request=httpx.Request("GET", "https://h/api2/json"), @@ -115,9 +134,46 @@ async def test_fetch_inventory_happy_path() -> None: @pytest.mark.asyncio async def test_test_connection_returns_message() -> None: async def fake_get_json(client, path: str): + if path == "/access/permissions": + return {"/": {"VM.Audit": 1}} # token has an ACL return {"version": "8.2.2"} with patch.object(svc, "_get_json", new=AsyncMock(side_effect=fake_get_json)): ok, msg = await svc.test_proxmox_connection("h", 8006, "u@pam!t", "sec") assert ok is True assert "8.2.2" in msg + assert "warning" not in msg.lower() + + +@pytest.mark.asyncio +async def test_test_connection_warns_when_token_has_no_permissions() -> None: + async def fake_get_json(client, path: str): + if path == "/access/permissions": + return {} # privilege-separated token with no effective ACL + return {"version": "8.4.19"} + + with patch.object(svc, "_get_json", new=AsyncMock(side_effect=fake_get_json)): + ok, msg = await svc.test_proxmox_connection("h", 8006, "u@pam!t", "sec") + # Auth still succeeds; the message flags the permission gap. + assert ok is True + assert "8.4.19" in msg + assert "PVEAuditor" in msg + + +@pytest.mark.asyncio +async def test_token_has_permissions_treats_empty_as_no_perms() -> None: + async def fake_get_json(client, path: str): + return {} + + with patch.object(svc, "_get_json", new=AsyncMock(side_effect=fake_get_json)): + assert await svc._token_has_permissions(object()) is False + + +@pytest.mark.asyncio +async def test_token_has_permissions_assumes_ok_on_error() -> None: + async def boom(client, path: str): + raise httpx.ConnectError("nope") + + with patch.object(svc, "_get_json", new=AsyncMock(side_effect=boom)): + # Never block a valid import on a permissions-probe failure. + assert await svc._token_has_permissions(object()) is True diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index b157aac..14a28e9 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -47,6 +47,7 @@ import type { NodeData, EdgeData, CustomStyleDef, FloorMapConfig, NodeType } fro import type { ZigbeeNode, ZigbeeEdge } from '@/components/zigbee/types' import type { ZwaveNode, ZwaveEdge } from '@/components/zwave/types' import type { ProxmoxNode, ProxmoxEdge } from '@/components/proxmox/types' +import { buildProxmoxClusterEdges } from '@/components/proxmox/clusterEdges' const STANDALONE = import.meta.env.VITE_STANDALONE === 'true' @@ -650,10 +651,16 @@ export default function App() { const cols = Math.min(COLS, pmNodes.length) const rows = Math.ceil(pmNodes.length / COLS) const origin = getCenteredPosition(cols * SPACING_X, rows * SPACING_Y) + // Multiple hosts from one import = a cluster → chain them via left/right + // 'cluster' edges. Those endpoints need one left + one right handle each + // (both default to 0), so grant them to the host nodes up front. + const clusterEdges = buildProxmoxClusterEdges(pmNodes) + const cluster = clusterEdges.length > 0 pmNodes.forEach((pn, i) => { const col = i % COLS const row = Math.floor(i / COLS) const position = { x: origin.x + col * SPACING_X, y: origin.y + row * SPACING_Y } + const isClusterHost = cluster && pn.type === 'proxmox' const newNode: import('@xyflow/react').Node = { id: pn.id, type: pn.type, @@ -665,6 +672,7 @@ export default function App() { services: [], ...(pn.ip ? { ip: pn.ip } : {}), ...(pn.hostname ? { hostname: pn.hostname } : {}), + ...(isClusterHost ? { left_handles: 1, right_handles: 1 } : {}), }, } addNode(newNode) @@ -679,6 +687,16 @@ export default function App() { type: 'virtual', } as unknown as import('@xyflow/react').Connection) }) + // Host ↔ host links render as 'cluster' edges (left → right chain). + clusterEdges.forEach((ce) => { + onConnect({ + source: ce.source, + sourceHandle: ce.sourceHandle, + target: ce.target, + targetHandle: ce.targetHandle, + type: 'cluster', + } as unknown as import('@xyflow/react').Connection) + }) const importedIds = new Set(pmNodes.map((pn) => pn.id)) useCanvasStore.setState((state) => ({ nodes: state.nodes.map((n) => ({ ...n, selected: importedIds.has(n.id) })), diff --git a/frontend/src/components/canvas/nodes/ProxmoxGroupNode.tsx b/frontend/src/components/canvas/nodes/ProxmoxGroupNode.tsx index 8d30797..3725e93 100644 --- a/frontend/src/components/canvas/nodes/ProxmoxGroupNode.tsx +++ b/frontend/src/components/canvas/nodes/ProxmoxGroupNode.tsx @@ -23,9 +23,12 @@ export function ProxmoxGroupNode(props: NodeProps>) { const theme = THEMES[activeTheme] const colors = resolveNodeColors(data, activeTheme) - // Render as a regular node when container mode is disabled. Cluster links now + // Container mode is opt-in — a proxmox node renders as a regular card unless + // it is explicitly a container (matches the rest of the codebase, which gates + // nesting on `container_mode === true`; see App.tsx). Imported nodes leave the + // flag unset and so render like a manually-created proxmox node. Cluster links // use the configurable per-side connection points (see BaseNode / SideHandles). - if (data.container_mode === false) { + if (data.container_mode !== true) { return } diff --git a/frontend/src/components/canvas/nodes/__tests__/ProxmoxGroupNode.test.tsx b/frontend/src/components/canvas/nodes/__tests__/ProxmoxGroupNode.test.tsx index d54ecab..5fc9763 100644 --- a/frontend/src/components/canvas/nodes/__tests__/ProxmoxGroupNode.test.tsx +++ b/frontend/src/components/canvas/nodes/__tests__/ProxmoxGroupNode.test.tsx @@ -13,6 +13,9 @@ function renderNode(data: Partial = {}, selected = false) { type: 'proxmox', status: 'online', services: [], + // Default tests to the container/group path (the branch this file covers); + // individual tests override with container_mode: false / unset as needed. + container_mode: true, ...data, } const props = { @@ -51,7 +54,7 @@ describe('ProxmoxGroupNode', () => { }) it('renders the node label', () => { - const { getByText } = renderNode({ label: 'My Proxmox' }) + const { getByText } = renderNode({ label: 'My Proxmox', container_mode: true }) expect(getByText('My Proxmox')).toBeDefined() }) @@ -90,14 +93,21 @@ describe('ProxmoxGroupNode', () => { expect(dot).not.toBeNull() }) - it('container_mode === false renders as BaseNode (no resizer group border)', () => { + it('container_mode === false renders as BaseNode (no group border)', () => { const { container } = renderNode({ container_mode: false }) - // NodeResizer should not be present when not group-rendered - expect(container.querySelector('.react-flow__resize-control')).toBeNull() + // The group container uses rounded-xl border-2; BaseNode does not. + expect(container.querySelector('.rounded-xl.border-2')).toBeNull() }) - it('container_mode default renders the group border container', () => { - const { container } = renderNode({}) + it('container mode is opt-in: default (unset) renders as a regular BaseNode', () => { + // Imported proxmox nodes leave container_mode unset and must look like a + // manually-created node (BaseNode), not an empty group container. + const { container } = renderNode({ container_mode: undefined }) + expect(container.querySelector('.rounded-xl.border-2')).toBeNull() + }) + + it('container_mode === true renders the group border container', () => { + const { container } = renderNode({ container_mode: true }) // Group border div has rounded-xl border-2 classes expect(container.querySelector('.rounded-xl.border-2')).not.toBeNull() }) diff --git a/frontend/src/components/modals/ScanHistoryModal.tsx b/frontend/src/components/modals/ScanHistoryModal.tsx index dcbb348..8dabc09 100644 --- a/frontend/src/components/modals/ScanHistoryModal.tsx +++ b/frontend/src/components/modals/ScanHistoryModal.tsx @@ -1,5 +1,5 @@ import { useState, useEffect, useCallback, useRef } from 'react' -import { RefreshCw, X, Loader2, StopCircle, Clock, ScanLine, Network, RadioTower, Inbox } from 'lucide-react' +import { RefreshCw, X, Loader2, StopCircle, Clock, ScanLine, Network, RadioTower, Server, Inbox } from 'lucide-react' import { Dialog, DialogClose, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { scanApi } from '@/api/client' @@ -22,17 +22,21 @@ interface ScanHistoryModalProps { onClose: () => void } -type KindFilter = 'all' | 'ip' | 'zigbee' | 'zwave' +type KindFilter = 'all' | 'ip' | 'zigbee' | 'zwave' | 'proxmox' /** Normalise a ScanRun.kind into one of the known display kinds. */ -function runKind(kind: string | undefined): 'ip' | 'zigbee' | 'zwave' { - return kind === 'zigbee' ? 'zigbee' : kind === 'zwave' ? 'zwave' : 'ip' +function runKind(kind: string | undefined): 'ip' | 'zigbee' | 'zwave' | 'proxmox' { + return kind === 'zigbee' ? 'zigbee' + : kind === 'zwave' ? 'zwave' + : kind === 'proxmox' ? 'proxmox' + : 'ip' } const KIND_META = { ip: { label: 'IP', color: '#a855f7' }, zigbee: { label: 'Zigbee', color: '#00d4ff' }, zwave: { label: 'Z-Wave', color: '#ff6e00' }, + proxmox: { label: 'Proxmox', color: '#e57000' }, } as const type StatusFilter = 'all' | 'running' | 'done' | 'error' | 'cancelled' @@ -49,6 +53,7 @@ const KIND_FILTERS: { key: KindFilter; label: string }[] = [ { key: 'ip', label: 'IP' }, { key: 'zigbee', label: 'Zigbee' }, { key: 'zwave', label: 'Z-Wave' }, + { key: 'proxmox', label: 'Proxmox' }, ] function statusColor(s: string): string { @@ -102,9 +107,15 @@ export function ScanHistoryModal({ open, onClose }: ScanHistoryModalProps) { toast.error(`Scan failed: ${run.error ?? 'unknown error'}`) } if (prev?.status === 'running' && run.status === 'done') { - if (run.kind === 'zigbee' || run.kind === 'zwave') { - const label = run.kind === 'zwave' ? 'Z-Wave' : 'Zigbee' - toast.success(`${label} import done — ${run.devices_found} device${run.devices_found !== 1 ? 's' : ''}`) + if (run.kind === 'zigbee' || run.kind === 'zwave' || run.kind === 'proxmox') { + const label = run.kind === 'zwave' ? 'Z-Wave' : run.kind === 'proxmox' ? 'Proxmox' : 'Zigbee' + // A done run can still carry a non-fatal advisory (e.g. Proxmox + // imported hosts but the token couldn't see any VMs/LXC). + if (run.error) { + toast.warning(`${label} import: ${run.error}`) + } else { + toast.success(`${label} import done — ${run.devices_found} device${run.devices_found !== 1 ? 's' : ''}`) + } } useCanvasStore.getState().notifyScanDeviceFound() } @@ -231,7 +242,7 @@ export function ScanHistoryModal({ open, onClose }: ScanHistoryModalProps) { {filtered.map((r) => { const kind = runKind(r.kind) const meta = KIND_META[kind] - const KindIcon = kind === 'zigbee' ? Network : kind === 'zwave' ? RadioTower : ScanLine + const KindIcon = kind === 'zigbee' ? Network : kind === 'zwave' ? RadioTower : kind === 'proxmox' ? Server : ScanLine return (
@@ -286,7 +297,15 @@ export function ScanHistoryModal({ open, onClose }: ScanHistoryModalProps) { )} {r.error && ( -
+ // A 'done' run with a message is a non-fatal advisory → amber, + // not the red used for a genuine failure. +
{r.error}
)} diff --git a/frontend/src/components/modals/__tests__/ScanHistoryModal.test.tsx b/frontend/src/components/modals/__tests__/ScanHistoryModal.test.tsx index 229eea7..72e1a67 100644 --- a/frontend/src/components/modals/__tests__/ScanHistoryModal.test.tsx +++ b/frontend/src/components/modals/__tests__/ScanHistoryModal.test.tsx @@ -3,7 +3,7 @@ import { render, screen, fireEvent, waitFor } from '@testing-library/react' import { ScanHistoryModal } from '../ScanHistoryModal' import { TooltipProvider } from '@/components/ui/tooltip' -vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })) +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn(), warning: vi.fn() } })) vi.mock('@/stores/canvasStore', () => ({ useCanvasStore: { getState: () => ({ notifyScanDeviceFound: vi.fn() }) }, })) @@ -72,6 +72,17 @@ const ZWAVE_RUN = { error: null, } +const PROXMOX_RUN = { + id: 'run-6', + status: 'done', + kind: 'proxmox', + ranges: ['pve:8006'], + devices_found: 9, + started_at: new Date().toISOString(), + finished_at: new Date().toISOString(), + error: null, +} + function renderModal() { return render( @@ -84,6 +95,7 @@ describe('ScanHistoryModal', () => { beforeEach(() => { vi.mocked(toast.success).mockReset() vi.mocked(toast.error).mockReset() + vi.mocked(toast.warning).mockReset() vi.mocked(scanApi.stop).mockReset() vi.mocked(scanApi.runs).mockResolvedValue({ data: [] } as never) }) @@ -176,4 +188,31 @@ describe('ScanHistoryModal', () => { expect(screen.getByText('5 found')).toBeDefined() expect(screen.queryByText('3 found')).toBeNull() }) + + it('renders a done proxmox run with an advisory as info, not a failure', async () => { + const ADVISORY_RUN = { + ...PROXMOX_RUN, + id: 'run-7', + devices_found: 3, + error: 'Imported 3 host(s) but no VMs or LXC were visible to the API token. Grant PVEAuditor…', + } + vi.mocked(scanApi.runs).mockResolvedValue({ data: [ADVISORY_RUN] } as never) + renderModal() + // Status stays "done" (success), yet the advisory text is surfaced. + await waitFor(() => expect(screen.getByText('done')).toBeDefined()) + expect(screen.getByText(/no VMs or LXC were visible/)).toBeDefined() + }) + + it('shows a proxmox run under its own kind, not IP', async () => { + vi.mocked(scanApi.runs).mockResolvedValue({ data: [DONE_RUN, PROXMOX_RUN] } as never) + renderModal() + await waitFor(() => expect(screen.getAllByText('done').length).toBe(2)) + // A dedicated Proxmox badge is rendered on the run (would be mislabeled "IP" + // before the fix). Both the filter chip and the run badge carry the label. + expect(screen.getAllByText('Proxmox').length).toBeGreaterThanOrEqual(2) + // Filtering to Proxmox keeps only the proxmox run (9 found), drops the IP run. + fireEvent.click(screen.getByRole('button', { name: 'Proxmox' })) + expect(screen.getByText('9 found')).toBeDefined() + expect(screen.queryByText('3 found')).toBeNull() + }) }) diff --git a/frontend/src/components/proxmox/__tests__/clusterEdges.test.ts b/frontend/src/components/proxmox/__tests__/clusterEdges.test.ts new file mode 100644 index 0000000..025c1f4 --- /dev/null +++ b/frontend/src/components/proxmox/__tests__/clusterEdges.test.ts @@ -0,0 +1,45 @@ +import { describe, it, expect } from 'vitest' +import { + buildProxmoxClusterEdges, + isProxmoxCluster, + CLUSTER_SOURCE_HANDLE, + CLUSTER_TARGET_HANDLE, +} from '../clusterEdges' +import type { ProxmoxNode } from '../types' + +function host(id: string): ProxmoxNode { + return { id, label: id, type: 'proxmox', ieee_address: id, status: 'online' } +} +function guest(id: string, type: 'vm' | 'lxc' = 'vm'): ProxmoxNode { + return { id, label: id, type, ieee_address: id, status: 'online' } +} + +describe('buildProxmoxClusterEdges', () => { + it('chains multiple hosts left→right, ignoring guests', () => { + const nodes = [host('pve-a'), guest('vm-1'), host('pve-b'), host('pve-c'), guest('ct-1', 'lxc')] + const edges = buildProxmoxClusterEdges(nodes) + expect(edges.map((e) => [e.source, e.target])).toEqual([ + ['pve-a', 'pve-b'], + ['pve-b', 'pve-c'], + ]) + // Endpoints use the left/right handles. + for (const e of edges) { + expect(e.sourceHandle).toBe(CLUSTER_SOURCE_HANDLE) + expect(e.targetHandle).toBe(CLUSTER_TARGET_HANDLE) + } + }) + + it('returns no edges for a single host', () => { + expect(buildProxmoxClusterEdges([host('pve-a'), guest('vm-1')])).toEqual([]) + }) + + it('returns no edges when there are no hosts', () => { + expect(buildProxmoxClusterEdges([guest('vm-1'), guest('ct-1', 'lxc')])).toEqual([]) + }) + + it('isProxmoxCluster is true only with 2+ hosts', () => { + expect(isProxmoxCluster([host('a')])).toBe(false) + expect(isProxmoxCluster([host('a'), host('b')])).toBe(true) + expect(isProxmoxCluster([host('a'), guest('vm-1')])).toBe(false) + }) +}) diff --git a/frontend/src/components/proxmox/clusterEdges.ts b/frontend/src/components/proxmox/clusterEdges.ts new file mode 100644 index 0000000..5a6337a --- /dev/null +++ b/frontend/src/components/proxmox/clusterEdges.ts @@ -0,0 +1,46 @@ +/** Cluster-edge wiring for a Proxmox import. + * + * Proxmox host nodes discovered in the same import belong to one cluster, so we + * chain them together with `cluster` edges. The chain uses the left/right + * connection points (right source → left target) to keep them visually distinct + * from the vertical host→guest `virtual` edges (bottom → top). Left/right + * handles default to 0, so the hosts must opt into one handle per side for the + * edge endpoints to exist (see `sideDefault` in handleUtils). + */ +import type { ProxmoxNode } from './types' + +/** Source/target handle IDs for a cluster link (see handleUtils.handleId). */ +export const CLUSTER_SOURCE_HANDLE = 'right' +export const CLUSTER_TARGET_HANDLE = 'left-t' + +export interface ClusterEdgeSpec { + source: string + target: string + sourceHandle: typeof CLUSTER_SOURCE_HANDLE + targetHandle: typeof CLUSTER_TARGET_HANDLE +} + +/** + * Chain all Proxmox host nodes (`type === 'proxmox'`) from one import into a + * left→right cluster line. Returns `[]` when fewer than two hosts are present + * (a single host is not a cluster). Guests (vm/lxc) are ignored. + */ +export function buildProxmoxClusterEdges(nodes: ProxmoxNode[]): ClusterEdgeSpec[] { + const hosts = nodes.filter((n) => n.type === 'proxmox') + if (hosts.length < 2) return [] + const edges: ClusterEdgeSpec[] = [] + for (let i = 0; i < hosts.length - 1; i++) { + edges.push({ + source: hosts[i].id, + target: hosts[i + 1].id, + sourceHandle: CLUSTER_SOURCE_HANDLE, + targetHandle: CLUSTER_TARGET_HANDLE, + }) + } + return edges +} + +/** True when the import contains a Proxmox cluster (≥2 host nodes). */ +export function isProxmoxCluster(nodes: ProxmoxNode[]): boolean { + return nodes.filter((n) => n.type === 'proxmox').length >= 2 +}