feat: proxmox import diagnostics, node style fix, and cluster edges
Scan History: - Proxmox is now a first-class scan kind (badge, filter chip, Server icon, completion toast) instead of being mislabeled as an IP scan. - A done run carrying a non-fatal advisory renders amber (info) with a warning toast, distinct from red failures. Import diagnostics: - test-connection probes /access/permissions and warns when the API token has no ACL (VMs/LXC would be invisible) — points at the PVEAuditor grant. - import surfaces an advisory when hosts import but no guests are visible (privilege-separated token whose rights are the intersection with the user), rather than a silent "done". Node style: - Proxmox container mode is now opt-in (container_mode === true), matching the rest of the codebase (App.tsx nesting logic). Imported proxmox nodes leave the flag unset and render like a manually-created node instead of an empty group container. Cluster edges: - Hosts from one import are chained with 'cluster' edges via left/right handles, distinct from the vertical host->guest 'virtual' edges. Wired for both the direct "Add to Canvas" path and the pending -> approve path (host<->host proxmox_cluster links, resolved to cluster edges on approve; cluster hosts get left/right handles). Tests added on both sides. ha-relevant: maybe
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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")], [])
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<NodeData> = {
|
||||
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) })),
|
||||
|
||||
@@ -23,9 +23,12 @@ export function ProxmoxGroupNode(props: NodeProps<Node<NodeData>>) {
|
||||
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 <BaseNode {...props} icon={Layers} />
|
||||
}
|
||||
|
||||
|
||||
@@ -13,6 +13,9 @@ function renderNode(data: Partial<NodeData> = {}, 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()
|
||||
})
|
||||
|
||||
@@ -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 (
|
||||
<div key={r.id} className="rounded-lg border border-border bg-[#161b22] p-3">
|
||||
<div className="flex items-center gap-2">
|
||||
@@ -286,7 +297,15 @@ export function ScanHistoryModal({ open, onClose }: ScanHistoryModalProps) {
|
||||
)}
|
||||
|
||||
{r.error && (
|
||||
<div className="mt-2 text-[11px] text-[#f85149] leading-tight whitespace-pre-wrap break-words rounded bg-[#f85149]/10 px-2 py-1.5">
|
||||
// A 'done' run with a message is a non-fatal advisory → amber,
|
||||
// not the red used for a genuine failure.
|
||||
<div
|
||||
className={`mt-2 text-[11px] leading-tight whitespace-pre-wrap break-words rounded px-2 py-1.5 ${
|
||||
r.status === 'done'
|
||||
? 'text-[#e3b341] bg-[#e3b341]/10'
|
||||
: 'text-[#f85149] bg-[#f85149]/10'
|
||||
}`}
|
||||
>
|
||||
{r.error}
|
||||
</div>
|
||||
)}
|
||||
|
||||
@@ -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(
|
||||
<TooltipProvider>
|
||||
@@ -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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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
|
||||
}
|
||||
Reference in New Issue
Block a user