diff --git a/.gitignore b/.gitignore index 25c298c23..82c671986 100644 --- a/.gitignore +++ b/.gitignore @@ -290,3 +290,6 @@ cython_debug/ ca-bundle.crt *.pem .github/hooks/ + +# Playwright MCP session artifacts (page snapshots / console logs) +.playwright-mcp/ diff --git a/docs/SUMMARY.md b/docs/SUMMARY.md index 8f1a38b80..f3cbe444f 100644 --- a/docs/SUMMARY.md +++ b/docs/SUMMARY.md @@ -16,6 +16,7 @@ * [Background Jobs & Caching](librenms_import/background_jobs.md) * [Sync & Configuration](usage_tips/virtual_chassis.md) * [Virtual Chassis](usage_tips/virtual_chassis.md) + * [Out-of-Band Management](usage_tips/oob_management.md) * [Interface Mappings](usage_tips/interface_mappings.md) * [Module Sync](usage_tips/module_sync.md) * [Mapping Rules](usage_tips/mapping_rules.md) diff --git a/docs/feature_list.md b/docs/feature_list.md index c46a70702..eeb4ac5bb 100644 --- a/docs/feature_list.md +++ b/docs/feature_list.md @@ -10,6 +10,15 @@ * Background job processing for large device sets * Duplicate detection to prevent re-importing existing devices +### [Out-of-Band (OOB) Management](usage_tips/oob_management.md) + +* Detects when a LibreNMS device (iDRAC/iLO/BMC/IPMI/CIMC) is the OOB controller of an existing NetBox device +* **Add as OOB** — link the controller to the host and set `oob_ip` on a chosen (or new) interface +* **Promote to host** — re-point a device currently linked to its OOB controller onto the incoming host device +* **Merge NetBox devices** — reconcile two devices (hostname-matched vs serial-matched) that represent one physical box +* Per-server linkage stored in the `librenms_id` custom field as `{"": {"id": N, "oob": {"id": M, "type": "drac"}}}` +* Post-merge **Move to winner** actions to migrate interfaces, IP addresses, and primary/OOB IPs at your own pace + ### [Module / Inventory Sync](usage_tips/module_sync.md) * Compare LibreNMS ENTITY-MIB inventory to NetBox module bays and installed modules diff --git a/docs/librenms_import/validation.md b/docs/librenms_import/validation.md index 4dd9158aa..a1d7aaddb 100644 --- a/docs/librenms_import/validation.md +++ b/docs/librenms_import/validation.md @@ -46,6 +46,11 @@ The plugin checks for existing devices using: If both a VM and Device with the same hostname exist, the plugin cannot determine which to match and allows import. Set the `librenms_id` custom field on the correct existing object to clarify the match. +## Out-of-Band (OOB) Detection + +When an incoming LibreNMS device looks like an out-of-band controller (iDRAC, iLO, BMC, …) and matches an existing NetBox device, the validation details show an **OOB Detected** panel instead of a plain import button. Rather than creating a duplicate device, the plugin offers the appropriate reconciliation action — **Add as OOB**, **Promote to host**, or **Merge NetBox devices**. See [Out-of-Band (OOB) Management](../usage_tips/oob_management.md) for the full flow. + ## Next Steps - [Import Settings](import_settings.md) - Configure device naming and import options +- [Out-of-Band Management](../usage_tips/oob_management.md) - Reconcile OOB controllers with their host devices diff --git a/docs/usage_tips/custom_field.md b/docs/usage_tips/custom_field.md index 0295a1e05..1b5757571 100644 --- a/docs/usage_tips/custom_field.md +++ b/docs/usage_tips/custom_field.md @@ -49,6 +49,13 @@ If the field was not created automatically (fallback): follow these steps to cre ```json {"production": 42, "staging": 17} ``` + - Out-of-band (OOB) form — when a device is linked to its OOB controller, the per-server value is an object holding the host id and the controller's id/type: + + ```json + {"production": {"id": 42, "oob": {"id": 99, "type": "drac"}}} + ``` + + This shape is written automatically by the OOB flows — see [Out-of-Band Management](oob_management.md). You don't normally edit it by hand. - Legacy single-server example (integer) — read-only/deprecated; do not use for new entries: ``` 42 diff --git a/docs/usage_tips/oob_management.md b/docs/usage_tips/oob_management.md new file mode 100644 index 000000000..3d1e91284 --- /dev/null +++ b/docs/usage_tips/oob_management.md @@ -0,0 +1,79 @@ +# Out-of-Band (OOB) Management + +Many servers expose a dedicated **out-of-band management controller** — iDRAC, iLO, BMC, IPMI, CIMC, and similar. LibreNMS usually polls that controller as its **own device**, separate from the host it lives in. NetBox models the same relationship differently: the controller is not a separate Device — its address is the host Device's **OOB IP** (`oob_ip`). + +This plugin bridges the two models. During import it detects when an incoming LibreNMS device is really the OOB side of a host you already have, and offers the right action to reconcile them instead of creating a duplicate device. + +## How the link is stored + +OOB linkage is recorded in the `librenms_id` [custom field](custom_field.md) alongside the host's own LibreNMS ID. The per-server value is promoted from a bare integer to a small object: + +```json +{ + "production": { + "id": 42, + "oob": { "id": 99, "type": "drac" } + } +} +``` + +- `id` — the LibreNMS device ID of the **host**. +- `oob.id` — the LibreNMS device ID of the **OOB controller**. +- `oob.type` — a short label for the controller (`drac`, `ilo`, `bmc`, `ipmi`, `cimc`, …), or the generic `oob` when the specific type can't be determined. + +Only these identity essentials are stored. The controller's IP and firmware version are intentionally **not** persisted here — the IP's source of truth is the host Device's interface-assigned `oob_ip`, and the version lives in LibreNMS and can be read back any time from `oob.id`. + +## OOB detection during import + +When a searched LibreNMS device looks like an OOB controller (by its OS/hardware strings, e.g. an iDRAC) and matches an existing NetBox device, the validation details show an **OOB Detected** panel instead of a plain import button. From there one of three resolution flows is offered, depending on what already exists. + +### Add as OOB + +Use when the existing NetBox device is the **host** and the incoming LibreNMS device is its OOB controller. + +The **Add as OOB to *device*** action links the controller's LibreNMS ID into the host's `oob.id` slot. NetBox requires `oob_ip` to be assigned to one of the device's interfaces, so the form includes an **OOB IP interface** picker: + +- A sensible interface is **pre-selected** (matched by name — `idrac`/`ilo`/`bmc`-style). Because the OOB IP is frequently *not* physically on that interface, the selection is **overridable**. +- Choose **+ Create new interface…** to create one (default name suggested) to hang the OOB IP on. + +The OOB IP is then created (or re-homed) assigned to the chosen interface and set as the device's `oob_ip`. If you make no interface selection, the link is still recorded and the OOB IP is left for you to set later. + +!!! note "Permissions" + Setting the OOB IP can create an Interface, create an IPAddress, or re-home an existing one. The action requires the matching NetBox `add`/`change` permissions for those models; if you lack them the link is still recorded and the IP step is skipped with a warning. See [Permissions & Access](permissions.md). + +### Promote to host + +Use when the existing NetBox device is currently linked to the **OOB controller** (its `librenms_id` points at the controller) and the incoming LibreNMS device is the **host** side. + +**Promote to host of *device*** re-points the linkage: the incoming host's LibreNMS ID becomes the device's `id`, and the previously-linked controller ID is demoted into the `oob` slot. No new device is created. A pre-promote modal lets you optionally override the device's **name**, **device type**, and **platform** — all default to **Keep current**, so the original promote behaviour is unchanged unless you explicitly choose **Use new**. + +### Merge NetBox devices + +Use when **two different NetBox devices** turn out to represent one physical box — typically one created from the LibreNMS hostname and another from the chassis serial, where at least one already carries a LibreNMS link. + +The validation modal lists both candidates (hostname-matched and serial-matched) with their current linkage, and you pick which one to **keep** (the *winner*) and which to absorb (the *donor*). Merging consolidates the donor's LibreNMS link state under the active server key into the winner, clears the donor's active link, and writes a `_migrated_to` marker on the donor pointing at the winner. Interfaces, cables, and primary/OOB IPs are **not** moved automatically — you re-home those incrementally (see below). + +## Migrating a donor device after a merge + +A donor device (one with a `_migrated_to` marker) shows a banner on its LibreNMS sync page with **Move to winner** actions, so you can move resources over at your own pace: + +- **Move interface to winner** — reassigns an interface (and the cables, IPs, and MACs that hang off it) to the winner. Fails if the winner already has an interface with the same name — rename or remove that one first. +- **Move IP address to winner** — re-homes an interface-assigned IP to the winner's same-named interface (move the interface first if it doesn't exist on the winner yet). +- **Transfer primary IPv4 / IPv6 / OOB IP** — points the winner's `primary_ip4` / `primary_ip6` / `oob_ip` foreign key at the donor's value and clears it on the donor. Refuses to overwrite a value already set on the winner — clear it there first. + +Each action runs under a row lock and verifies the `_migrated_to` marker before touching anything. Once the donor has nothing left to migrate you can delete it. + +## Setting Primary and OOB IPs in general + +Outside the OOB import flows, both `primary_ip` and `oob_ip` are driven from interface-assigned addresses: + +- **Primary IP** is set on the device's **IP Addresses** sync tab: with **Set Primary IP** enabled, a synced IP that matches the LibreNMS management IP and is interface-assigned becomes the device's primary. +- **OOB IP** is set through the **Add as OOB** flow above. + +This keeps every IP relationship valid against NetBox's requirement that primary/OOB IPs be assigned to one of the device's own interfaces. + +## See also + +- [Custom Field Setup](custom_field.md) — the `librenms_id` field that stores the linkage. +- [Validation & Configuration](../librenms_import/validation.md) — where OOB is detected during import. +- [Permissions & Access](permissions.md) — permissions required for the OOB/IP actions. diff --git a/mkdocs.yml b/mkdocs.yml index e38dd729a..efa426418 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -20,6 +20,7 @@ nav: - Background Jobs & Caching: librenms_import/background_jobs.md - Sync & Configuration: - Virtual Chassis: usage_tips/virtual_chassis.md + - Out-of-Band Management: usage_tips/oob_management.md - Interface Mappings: usage_tips/interface_mappings.md - Module Sync: usage_tips/module_sync.md - Mapping Rules: usage_tips/mapping_rules.md diff --git a/netbox_librenms_plugin/constants.py b/netbox_librenms_plugin/constants.py index 4e542f9d1..bd7c311ba 100644 --- a/netbox_librenms_plugin/constants.py +++ b/netbox_librenms_plugin/constants.py @@ -1,6 +1,42 @@ +import re + # Plugin permissions (from LibreNMSSettings model) PERM_VIEW_PLUGIN = "netbox_librenms_plugin.view_librenmssettings" PERM_CHANGE_PLUGIN = "netbox_librenms_plugin.change_librenmssettings" # LibreNMS VLAN state values LIBRENMS_VLAN_STATE_ACTIVE = 1 + +# OOB management controller detection +# Trailing \d*\b restricts matches to whole tokens (optionally with a numeric suffix like +# iDRAC9 / drac9) so a prefix collision inside an unrelated word — e.g. "dracut", "ipmitool" +# — can't misclassify a normal device as an OOB controller. +OOB_TYPE_PATTERN = re.compile(r"\b(idrac|ilo|ipmi|bmc|drac|cimc|oob)\d*\b", re.IGNORECASE) +OOB_TYPES = ("idrac", "ilo", "ipmi", "bmc", "drac", "cimc", "oob") + + +def normalize_oob_type(os_str: str, hardware_str: str = "") -> str | None: + """ + Extract and normalize the OOB controller type from LibreNMS os/hardware strings. + + Returns the canonical lowercase token (one of OOB_TYPES) or None if no match. + + A vendor-specific match (idrac/ilo/ipmi/bmc/drac/cimc) always wins over the + generic ``oob`` token, even when ``oob`` appears earlier in the text, so e.g. + ``normalize_oob_type("oob", "iDRAC9")`` resolves to ``"idrac"`` rather than + being masked by the generic token. + + Examples: + normalize_oob_type("drac9", "iDRAC9") → "drac" + normalize_oob_type("oob", "iDRAC9") → "idrac" + normalize_oob_type("ilo", "") → "ilo" + normalize_oob_type("ubuntu", "") → None + """ + generic = None + for text in (os_str or "", hardware_str or ""): + for m in OOB_TYPE_PATTERN.finditer(text): + token = m.group(1).lower() + if token != "oob": + return token # vendor-specific match wins immediately + generic = generic or "oob" # remember the generic fallback, keep scanning + return generic diff --git a/netbox_librenms_plugin/import_utils/__init__.py b/netbox_librenms_plugin/import_utils/__init__.py index 81c24025e..1028a7f0a 100644 --- a/netbox_librenms_plugin/import_utils/__init__.py +++ b/netbox_librenms_plugin/import_utils/__init__.py @@ -33,6 +33,7 @@ import_single_device, validate_device_for_import, ) +from .collisions import detect_bulk_collisions # noqa: F401 from .filters import ( # noqa: F401 _apply_client_filters, get_device_count_for_filters, diff --git a/netbox_librenms_plugin/import_utils/bulk_import.py b/netbox_librenms_plugin/import_utils/bulk_import.py index 93962432e..42f15e0a3 100644 --- a/netbox_librenms_plugin/import_utils/bulk_import.py +++ b/netbox_librenms_plugin/import_utils/bulk_import.py @@ -8,9 +8,13 @@ from ..import_validation_helpers import apply_role_to_validation, recalculate_validation_status, remove_validation_issue from ..librenms_api import LibreNMSAPI -from ..utils import find_by_librenms_id +from ..utils import AmbiguousLibreNMSIdError, coerce_librenms_id, find_by_librenms_id, get_librenms_oob from .cache import get_cache_metadata_key, get_import_device_cache_key, get_validated_device_cache_key -from .device_operations import import_single_device, validate_device_for_import +from .device_operations import ( + _describe_existing_librenms_link, + import_single_device, + validate_device_for_import, +) from .filters import _safe_disabled, get_librenms_devices_for_import from .permissions import check_user_permissions, require_permissions from .virtual_chassis import ( @@ -21,6 +25,12 @@ logger = logging.getLogger(__name__) +# Stable fragment of the ambiguous-librenms_id blocker message. Shared by the writer +# (the AmbiguousLibreNMSIdError handler) and the cleaner (the pre-lookup reset in +# _refresh_existing_device) so a resolved duplicate's stale message is reliably removed +# regardless of which librenms_id value was interpolated into it. +_AMBIGUOUS_LIBRENMS_ID_MARKER = "matches more than one existing NetBox record" + def _is_job_cancelled(job) -> bool: """ @@ -334,6 +344,88 @@ def bulk_import_devices( ) +def _refresh_librenms_linkage(validation: dict, device, libre_device: dict, server_key: str) -> None: + """Re-derive the LibreNMS-id linkage fields for a refreshed *device*. + + Cheap and DB-only (reads the device's ``librenms_id`` custom field) — no + LibreNMS API call — so a cached import row picks up OOB-link / host-link + changes made in NetBox since the row was cached. Without this, the cache-hit + path keeps the stale ``existing_match_type``/badge (e.g. an OOB controller + linked after caching still rendered as a conflict until the cache expired). + + Mirrors ``validate_device_for_import``'s linkage logic: always refreshes + ``existing_librenms_link``, and when the device is matched to the scanned + LibreNMS id it classifies the match as ``librenms_oob`` (matched via the OOB + sub-key) or ``librenms_id`` (matched as the host). + """ + link = _describe_existing_librenms_link(device, server_key) + validation["existing_librenms_link"] = link + # Only re-classify librenms-id-based matches; leave serial/hostname/primary_ip + # match types untouched. + if validation.get("existing_match_type") in ("librenms_id", "librenms_oob"): + scanned_id = coerce_librenms_id((libre_device or {}).get("device_id")) + oob = get_librenms_oob(device, server_key=server_key) + oob_id = coerce_librenms_id(oob.get("id")) if oob else None + if scanned_id is not None and oob_id is not None and oob_id == scanned_id: + validation["existing_match_type"] = "librenms_oob" + elif scanned_id is not None and link["host_id"] is not None and link["host_id"] == scanned_id: + # Host id still matches the scanned device — a genuine host-side link. + validation["existing_match_type"] = "librenms_id" + else: + # Linkage changed since caching: neither the host id nor the OOB id matches + # the scanned device anymore, so don't keep a stale librenms_id badge. + validation["existing_match_type"] = None + + +def _clear_existing_match_derived_fields(validation: dict) -> None: + """Reset the fields produced from an existing match so stale serial/OOB/merge/promote + actions don't linger after that match is dropped (device deleted, or librenms/OOB link + removed since caching). The subsequent fresh lookup re-populates them if it re-matches. + """ + validation["serial_action"] = None + validation["oob_candidate"] = None + validation["serial_confirmed"] = False + validation["serial_duplicate"] = False + validation["serial_role_choice_available"] = False + # Name-sync / migration / device-type state is also derived from the (now dropped) match; + # leaving it set would render a migrate/name-sync action for the old object. The fresh + # lookup below re-derives these only on a re-match, so reset them here. + validation["librenms_id_needs_migration"] = False + validation["name_matches"] = False + validation["name_sync_available"] = False + validation["suggested_name"] = None + validation["device_type_mismatch"] = False + # promote_to_host follows the "absent otherwise" contract (see apply_oob_detection_result). + validation.pop("promote_to_host", None) + validation.pop("merge_candidates", None) + + +def _reassert_new_import_blockers(validation: dict) -> None: + """Re-add the create-time role/cluster blocker that validate_device_for_import() attaches + to unmatched rows. + + When a refresh drops a cached match (or never had one) and the fresh lookup finds nothing, + the row is back in the "new import" path. recalculate_validation_status() recomputes + can_import purely from the issues list, so without re-adding this blocker a row that still + has no role/cluster selected could flip back to importable and then fail at import time. + + Guarded by the selection state (found/role/cluster), so a row where the user *has* picked a + role/cluster — which sets found=True and removed the issue — is left importable. + """ + if validation.get("import_as_vm"): + cluster = validation.get("cluster") or {} + if not cluster.get("found") and not cluster.get("cluster"): + msg = "Cluster must be manually selected before importing as VM" + if msg not in validation.setdefault("issues", []): + validation["issues"].append(msg) + else: + role = validation.get("device_role") or {} + if not role.get("found") and not role.get("role"): + msg = "Device role must be manually selected before import" + if msg not in validation.setdefault("issues", []): + validation["issues"].append(msg) + + def _refresh_existing_device(validation: dict, libre_device: dict = None, server_key: str = "default") -> None: """ Refresh existing_device from DB to pick up changes made in NetBox since caching. @@ -354,26 +446,63 @@ def _refresh_existing_device(validation: dict, libre_device: dict = None, server if refreshed: validation["existing_device"] = refreshed - if hasattr(refreshed, "role") and refreshed.role: - apply_role_to_validation(validation, refreshed.role, is_vm=bool(validation.get("import_as_vm"))) - elif not validation.get("import_as_vm"): - validation["device_role"] = { - "found": False, - "role": None, - "available_roles": validation.get("device_role", {}).get("available_roles", []), - } - remove_validation_issue(validation, "role") - recalculate_validation_status(validation, is_vm=bool(validation.get("import_as_vm"))) - # Re-assert non-importable state: recalculate bases can_import on - # issues alone, but an existing matched device must never be import-ready. - validation["can_import"] = False - validation["is_ready"] = False - return + # Re-derive linkage so an OOB-link/host-link change since caching + # is reflected in the badge (DB-only; no LibreNMS API call). + prior_match = validation.get("existing_match_type") + _refresh_librenms_linkage(validation, refreshed, libre_device, server_key) + if prior_match in ("librenms_id", "librenms_oob") and validation.get("existing_match_type") is None: + # The librenms-id/OOB link that made this the cached match is gone + # (removed/repointed in NetBox since caching). Treat it like a vanished + # match — clear it and recompute readiness, then fall through to the fresh + # lookup below so the row is re-evaluated under current rules (it may now + # match by hostname/serial/IP, or become importable as new) instead of + # staying blocked until cache expiry. Mirrors the deleted-device branch. + validation["existing_device"] = None + validation["existing_librenms_link"] = None + _clear_existing_match_derived_fields(validation) + if not validation.get("import_as_vm"): + validation["device_role"] = { + "found": False, + "role": None, + "available_roles": validation.get("device_role", {}).get("available_roles", []), + } + else: + # VM rows are gated on cluster, not role: a dropped match must also clear + # the stale cluster selection (preserving available_clusters), or + # _reassert_new_import_blockers() sees found/cluster still set and lets + # the row re-enter the new-import path without a fresh cluster choice. + validation["cluster"] = { + "found": False, + "cluster": None, + "available_clusters": validation.get("cluster", {}).get("available_clusters", []), + } + recalculate_validation_status(validation, is_vm=bool(validation.get("import_as_vm"))) + else: + if hasattr(refreshed, "role") and refreshed.role: + apply_role_to_validation(validation, refreshed.role, is_vm=bool(validation.get("import_as_vm"))) + elif not validation.get("import_as_vm"): + validation["device_role"] = { + "found": False, + "role": None, + "available_roles": validation.get("device_role", {}).get("available_roles", []), + } + remove_validation_issue(validation, "role") + recalculate_validation_status(validation, is_vm=bool(validation.get("import_as_vm"))) + # Re-assert non-importable state: recalculate bases can_import on + # issues alone, but an existing matched device must never be import-ready. + validation["can_import"] = False + validation["is_ready"] = False + return else: # Device was deleted since caching — recompute readiness to match # validate_device_for_import logic. validation["existing_device"] = None validation["existing_match_type"] = None + # Nothing is linked anymore — clear the linkage so the row can't + # keep rendering a stale host/OOB badge. + validation["existing_librenms_link"] = None + # Drop serial/OOB/merge/promote actions that pointed at the deleted device. + _clear_existing_match_derived_fields(validation) # Clear stale device_role so is_ready is computed from scratch. # Guard: VMs don't use device_role for readiness, so preserve any # user-selected role rather than silently dropping it. @@ -383,13 +512,26 @@ def _refresh_existing_device(validation: dict, libre_device: dict = None, server "role": None, "available_roles": validation.get("device_role", {}).get("available_roles", []), } + else: + # Mirror the stale-match branch: a deleted cached VM match must drop the + # stale cluster selection (keeping available_clusters) so the row returns to + # the same create-time state as a brand-new VM import row. + validation["cluster"] = { + "found": False, + "cluster": None, + "available_clusters": validation.get("cluster", {}).get("available_clusters", []), + } recalculate_validation_status(validation, is_vm=bool(validation.get("import_as_vm"))) except Exception as e: existing_id = getattr(existing, "pk", "unknown") if existing else "none" logger.error(f"Failed to refresh existing device (pk={existing_id}): {e}") return - # existing_device was None at cache time — check if device was imported since + # Re-evaluate the match under current DB state. Reached when existing_device was None at + # cache time, or when a cached librenms_id/OOB link disappeared (cleared above) or its + # device was deleted — in every case re-check whether a matching NetBox object exists now, + # using the full id/name/serial/IP breadth so the row can't flip to importable and create + # a duplicate of a device that still exists under a different identity. if not libre_device: return try: @@ -402,23 +544,37 @@ def _refresh_existing_device(validation: dict, libre_device: dict = None, server # imported as a VM even though import_as_vm=False (or vice versa). CrossModel = Device if import_as_vm else VirtualMachine - librenms_id = libre_device.get("device_id") + # Coerce up front so malformed values (e.g. "42.0", floats, booleans) are rejected + # rather than truncated by int() and matched to the wrong record. + librenms_id = coerce_librenms_id(libre_device.get("device_id")) hostname = libre_device.get("hostname", "") sys_name = libre_device.get("sysName", "") + # Clear any stale ambiguous-librenms_id blocker set by a prior refresh before + # re-running the lookup. If the duplicate still exists, _lookup_in_model() below + # re-raises AmbiguousLibreNMSIdError and the except handler re-adds the blocker; + # if it was resolved since, the row must not stay blocked until cache expiry. + if validation.get("ambiguous_librenms_id"): + validation["ambiguous_librenms_id"] = False + if validation.get("existing_match_type") == "ambiguous_librenms_id": + validation["existing_match_type"] = None + for _key in ("issues", "warnings"): + msgs = validation.get(_key) + if isinstance(msgs, list): + validation[_key] = [ + m for m in msgs if not (isinstance(m, str) and _AMBIGUOUS_LIBRENMS_ID_MARKER in m) + ] + new_device = None match_type = None found_as_cross_model = False def _lookup_in_model(m): """Return (device, match_type) for model m, or (None, None).""" - if librenms_id is not None and not isinstance(librenms_id, bool): - try: - dev = find_by_librenms_id(m, int(librenms_id), server_key) - if dev: - return dev, "librenms_id" - except (ValueError, TypeError): - pass + if librenms_id is not None: + dev = find_by_librenms_id(m, librenms_id, server_key) + if dev: + return dev, "librenms_id" resolved_name = validation.get("resolved_name") if resolved_name: dev = m.objects.filter(name__iexact=resolved_name).first() @@ -443,9 +599,40 @@ def _lookup_in_model(m): if new_device: found_as_cross_model = True + if not new_device and not import_as_vm: + # Serial- and IP-based matches: validate_device_for_import() catches these, so the + # refresh re-check must have the same breadth. Without them a row whose + # librenms_id/name link disappeared (or that never matched) can flip to importable + # and re-import a device that already exists in NetBox under a different name — + # matched only by hardware serial or management IP. Device-only (VMs have no serial + # or primary-IP identity here). The richer serial_action/OOB-candidate heuristics + # stay in the full validation path; here the contract is simply: block the import. + from dcim.models import Device as _Device + + serial = (libre_device.get("serial") or "").strip() + if serial and serial != "-": + dev = _Device.objects.filter(serial=serial).first() + if dev: + new_device, match_type = dev, "serial" + + if not new_device: + primary_ip = libre_device.get("ip") + if primary_ip: + from ipam.models import IPAddress + + existing_ip = IPAddress.objects.filter(address__net_host=primary_ip).first() + assigned = getattr(existing_ip, "assigned_object", None) if existing_ip else None + dev = getattr(assigned, "device", None) if assigned else None + if dev: + new_device, match_type = dev, "primary_ip" + if new_device: validation["existing_device"] = new_device validation["existing_match_type"] = match_type + # Re-derive linkage so a librenms_id match is correctly shown as the + # host vs. OOB half, and existing_librenms_link is populated for the + # paired badge (DB-only; no LibreNMS API call). + _refresh_librenms_linkage(validation, new_device, libre_device, server_key) validation["can_import"] = False validation["is_ready"] = False # Determine actual model from the found object, not from import_as_vm flag @@ -459,11 +646,41 @@ def _lookup_in_model(m): "role": None, "available_roles": validation.get("device_role", {}).get("available_roles", []), } + # A previously-unmatched row may still carry the "Device role must be manually + # selected before import" blocker; clear it now that the row resolves to an + # existing object, so the stale message doesn't linger in the UI. + remove_validation_issue(validation, "role") recalculate_validation_status(validation, is_vm=actual_is_vm) # Re-assert non-importable: recalculate sets can_import from issues list, # but a late-found existing match must never be import-ready. validation["can_import"] = False validation["is_ready"] = False + else: + # No existing match at all — the row is a genuine new import. If a cached match was + # just cleared above, its create-time role/cluster blocker was lost; re-add it so the + # row can't flip to importable while still missing a required selection. + _reassert_new_import_blockers(validation) + recalculate_validation_status(validation, is_vm=import_as_vm) + except AmbiguousLibreNMSIdError as exc: + # An ambiguous librenms_id (matching multiple records) must block import rather + # than fall through as "not found" and stay importable. + logger.warning("Bulk re-check blocked — ambiguous librenms_id %r: %s", librenms_id, exc) + validation["can_import"] = False + validation["is_ready"] = False + validation["ambiguous_librenms_id"] = True + validation["existing_match_type"] = "ambiguous_librenms_id" + message = ( + f"LibreNMS ID {librenms_id} {_AMBIGUOUS_LIBRENMS_ID_MARKER}; import " + "blocked to avoid binding to the wrong object. Resolve the duplicate librenms_id " + "assignment, then retry." + ) + # Append to issues (not just warnings) — a later recalculate_validation_status() + # recomputes can_import from issues, so a warning alone would be silently re-enabled. + # Dedup so repeated refreshes don't stack the same message. + if message not in validation.setdefault("warnings", []): + validation["warnings"].append(message) + if message not in validation.setdefault("issues", []): + validation["issues"].append(message) except Exception as e: logger.error(f"Failed to check for newly imported device: {e}") diff --git a/netbox_librenms_plugin/import_utils/collisions.py b/netbox_librenms_plugin/import_utils/collisions.py new file mode 100644 index 000000000..d0d4976ce --- /dev/null +++ b/netbox_librenms_plugin/import_utils/collisions.py @@ -0,0 +1,169 @@ +""" +Bulk-import collision detection. + +When a user selects multiple LibreNMS devices for bulk import, two or more +rows in the same batch may resolve to the *same* NetBox device — for +example, one row would be linked as the host and another as the OOB +controller, or two rows both want to promote to the same existing host. +Importing all of them blindly would race for the same custom-field slot +and produce inconsistent state. + +`detect_bulk_collisions` walks the per-row validation results and groups +rows that target the same NetBox device pk so the bulk-confirm view can +block the import and let the user adjust their selection. +""" + +from __future__ import annotations + + +def _candidate_pks_for_row(validation: dict) -> list[tuple[int, str, str, str]]: + """Return [(nb_device_pk, nb_device_name, role, model_name)] candidates for a single row. + + ``role`` is a short human-readable label describing how this LibreNMS + row would touch the NetBox device: + + * ``"host"`` — the device is the existing NetBox device this row would + update / link to (``existing_device``). + * ``"oob"`` — this LibreNMS row would be installed as the OOB + controller of the NetBox device (``oob_candidate.device``). + * ``"merge_host_named"`` / ``"merge_oob_named"`` — the row would feed + a Stage-2 merge of two NetBox devices. + * ``"promote_target"`` — the row should be promoted to host of an + existing NetBox device that currently only has an OOB link + (``promote_to_host``). + + ``model_name`` is the Python class name of the NetBox object + (e.g. ``"Device"``, ``"VirtualMachine"``) so that two objects of + different types that happen to share the same pk are not grouped as + collisions. + + Duplicate ``(pk, role, model_name)`` tuples are de-duplicated; a + single row may legitimately surface the same pk under different roles. + """ + candidates: list[tuple[int, str, str, str]] = [] + seen: set[tuple[int, str, str]] = set() + + def _add(pk, name, role, model_name="Device"): + try: + pk_int = int(pk) + except (TypeError, ValueError): + return + key = (pk_int, role, model_name) + if key in seen: + return + seen.add(key) + candidates.append((pk_int, str(name or f"device-{pk_int}"), role, model_name)) + + existing = validation.get("existing_device") + if existing is not None and getattr(existing, "pk", None) is not None: + _add(existing.pk, getattr(existing, "name", None), "host", type(existing).__name__) + + oob_candidate = validation.get("oob_candidate") or {} + oob_device = oob_candidate.get("device") if isinstance(oob_candidate, dict) else None + if oob_device is not None and getattr(oob_device, "pk", None) is not None: + _add(oob_device.pk, getattr(oob_device, "name", None), "oob", type(oob_device).__name__) + + merge = validation.get("merge_candidates") or {} + if isinstance(merge, dict): + for slot, role in (("host_named", "merge_host_named"), ("oob_named", "merge_oob_named")): + entry = merge.get(slot) or {} + pk = entry.get("pk") if isinstance(entry, dict) else None + name = entry.get("name") if isinstance(entry, dict) else None + if pk is not None: + _add(pk, name, role) + + promote = validation.get("promote_to_host") or {} + if isinstance(promote, dict): + target = promote.get("existing_device") + if target is not None and getattr(target, "pk", None) is not None: + _add(target.pk, getattr(target, "name", None), "promote_target", type(target).__name__) + + return candidates + + +def detect_bulk_collisions(devices: list[dict] | None) -> list[dict]: + """Find groups of LibreNMS rows in *devices* that resolve to the same NetBox device. + + *devices* matches the list assembled by ``BulkImportConfirmView`` — + each item is a dict with at least ``device_id``, ``device_name`` and + ``validation`` keys. ``None`` is accepted and treated as an empty list. + + Returns a list of collision groups (one per offending NetBox device), + sorted by model type then ``nb_device_pk`` for stable rendering (the + composite key prevents false collisions when a Device and a + VirtualMachine share the same integer pk). Each group: + + .. code-block:: python + + { + "nb_device_pk": int, + "nb_device_name": str, + "librenms_rows": [ + {"device_id": int, "hostname": str, "role": str}, + ... + ], + } + + Rows are de-duplicated by ``device_id`` within a group (same LibreNMS + row touching the same NetBox device under multiple roles only appears + once, with all matching role labels joined by ``", "``). + + A group is only emitted when at least two distinct LibreNMS + ``device_id`` values target the same NetBox pk. + """ + # Key by (model_name, nb_pk) to avoid false collisions when a Device + # and a VirtualMachine happen to share the same integer pk. + by_nb_pk: dict[tuple[str, int], dict] = {} + + for entry in devices or []: + validation = entry.get("validation") or {} + try: + libre_id = int(entry.get("device_id")) + except (TypeError, ValueError): + continue + hostname = entry.get("device_name") or f"device-{libre_id}" + + for nb_pk, nb_name, role, model_name in _candidate_pks_for_row(validation): + bucket_key = (model_name, nb_pk) + bucket = by_nb_pk.setdefault( + bucket_key, + {"nb_device_pk": nb_pk, "nb_device_name": nb_name, "nb_model_name": model_name, "_rows": {}}, + ) + # Keep the first non-default name we see — rows often disagree + # on the cached display string, but the underlying pk is the + # source of truth. + if bucket["nb_device_name"].startswith("device-") and not nb_name.startswith("device-"): + bucket["nb_device_name"] = nb_name + + row = bucket["_rows"].setdefault( + libre_id, + {"device_id": libre_id, "hostname": hostname, "roles": []}, + ) + if role not in row["roles"]: + row["roles"].append(role) + + collisions: list[dict] = [] + for _model_name, nb_pk in sorted(by_nb_pk.keys()): + bucket = by_nb_pk[(_model_name, nb_pk)] + rows = list(bucket["_rows"].values()) + if len(rows) < 2: + continue + rows.sort(key=lambda r: r["device_id"]) + collisions.append( + { + "nb_device_pk": bucket["nb_device_pk"], + "nb_device_name": bucket["nb_device_name"], + # Class name ("Device"/"VirtualMachine") so the template links to the right + # object type — a VM collision must not render a dcim:device URL. + "nb_model_name": bucket["nb_model_name"], + "librenms_rows": [ + { + "device_id": r["device_id"], + "hostname": r["hostname"], + "role": ", ".join(r["roles"]), + } + for r in rows + ], + } + ) + return collisions diff --git a/netbox_librenms_plugin/import_utils/device_operations.py b/netbox_librenms_plugin/import_utils/device_operations.py index 6b87935b0..f5363432c 100644 --- a/netbox_librenms_plugin/import_utils/device_operations.py +++ b/netbox_librenms_plugin/import_utils/device_operations.py @@ -12,12 +12,17 @@ from ..librenms_api import LibreNMSAPI from ..utils import ( + AmbiguousLibreNMSIdError, + coerce_librenms_id, find_by_librenms_id, find_matching_platform, find_matching_site, + get_librenms_oob, match_librenms_hardware_to_device_type, set_librenms_device_id, ) +from ..constants import normalize_oob_type +from ..import_validation_helpers import apply_merge_candidates, apply_oob_detection_result from .cache import get_import_device_cache_key from .virtual_chassis import ( _generate_vc_member_name, @@ -29,6 +34,56 @@ logger = logging.getLogger(__name__) +def _detect_oob_type_from_name(name): + """Return canonical OOB type token (idrac/ilo/ipmi/bmc/drac) found in *name*, or None. + + Routes through normalize_oob_type() so a vendor-specific token wins over the generic + "oob" even when "oob" appears earlier in the name (e.g. "leaf01-oob-idrac9" -> "idrac", + not "oob"). A bare re.search() returns the first token and would downgrade the hint. + """ + if not name: + return None + return normalize_oob_type(name, "") + + +def _describe_existing_librenms_link(obj, server_key): + """ + Describe the current LibreNMS linkage on a NetBox object. + + Returns a dict ``{"host_id": int|None, "oob_id": int|None, "oob_type": str|None}`` + summarising the ``librenms_id`` custom field for *server_key*. Always returns a + dict (with all-None values if nothing is linked) so callers can treat it as a + plain status object. Tolerates legacy bare-int and dict-form custom field values. + """ + info = {"host_id": None, "oob_id": None, "oob_type": None} + cf_value = obj.cf.get("librenms_id") if hasattr(obj, "cf") else None + # Legacy bare-int OR string-digit (pre-JSON format). + if not isinstance(cf_value, dict): + info["host_id"] = coerce_librenms_id(cf_value) + return info + entry = cf_value.get(server_key) + # Per-server simple form: legacy bare-int or string-digit under the server key. + if not isinstance(entry, dict): + info["host_id"] = coerce_librenms_id(entry) + return info + # New dict-form: {"id": , "oob": {"id": , "type": , ...}}. + # Use coerce_librenms_id so string-digit values (e.g. "42") stored by older + # plugin versions are still recognized, matching the behaviour of + # find_by_librenms_id() and get_librenms_device_id(). + host_id = coerce_librenms_id(entry.get("id")) + if host_id is not None and host_id > 0: + info["host_id"] = host_id + oob = entry.get("oob") + if isinstance(oob, dict): + oob_id = coerce_librenms_id(oob.get("id")) + if oob_id is not None and oob_id > 0: + info["oob_id"] = oob_id + oob_type = oob.get("type") + if isinstance(oob_type, str) and oob_type: + info["oob_type"] = oob_type + return info + + def _try_chassis_device_type_match(api, device_id): """ Attempt device type matching using chassis inventory fields. @@ -125,6 +180,30 @@ def _determine_device_name( return name +def _flag_ambiguous_librenms_id(result, librenms_id, exc): + """Block import when a librenms_id resolves to more than one NetBox object. + + An ambiguous id is a data-integrity violation; treating it as "not found" would let + the device import as new (or bind to an arbitrary row), so fail closed instead. + + The message is appended to ``issues`` (not just ``warnings``) because the readiness + step recomputes ``can_import`` from ``issues`` — a warning alone would be silently + overridden back to importable when no other issue is present. + """ + logger.warning("Import validation blocked — ambiguous librenms_id %r: %s", librenms_id, exc) + result["ambiguous_librenms_id"] = True + result["can_import"] = False + if result.get("existing_match_type") != "ambiguous_librenms_id": + result["existing_match_type"] = "ambiguous_librenms_id" + message = ( + f"LibreNMS ID {librenms_id} matches more than one existing NetBox record; import " + "blocked to avoid binding to the wrong object. Resolve the duplicate librenms_id " + "assignment, then retry." + ) + result["warnings"].append(message) + result["issues"].append(message) + + def validate_device_for_import( libre_device: dict, import_as_vm: bool = False, @@ -206,10 +285,16 @@ def validate_device_for_import( "resolved_name": None, # Final device name after applying user preferences "existing_device": None, "existing_match_type": None, # Track how existing device was matched - "serial_action": None, # None, "link", "conflict", "update_serial", "hostname_differs" + "ambiguous_librenms_id": False, # True when the librenms_id matches >1 NetBox object (import blocked) + "serial_action": None, # None, "link", "conflict", "update_serial", "hostname_differs", "oob_candidate", "promote_to_host", "merge_netbox_devices" "serial_confirmed": False, # True when librenms_id match and serial matches "serial_duplicate": False, # True when incoming serial is already on a different device + "serial_role_choice_available": False, # True when both oob_candidate and promote_to_host are valid choices "librenms_id_needs_migration": False, # True when existing device has legacy bare-int ID + "oob_candidate": None, # dict {device, type, version, ip} when oob_candidate detected + # promote_to_host is only set when the host-promotion path is available; absent otherwise. + "existing_librenms_link": None, # dict {host_id, oob_id, oob_type} describing existing device's current LibreNMS linkage + "merge_candidates": None, # dict {host_named: {pk,name,librenms_link}, oob_named: {pk,name,librenms_link}} when two NB devices look like the same physical box "name_matches": False, # True when existing device name matches LibreNMS sysName "name_sync_available": False, # True when existing device name differs from sysName "suggested_name": None, # sysName to suggest when name_sync_available is True @@ -286,8 +371,30 @@ def validate_device_for_import( # Check for existing VM first (by librenms_id custom field). # find_by_librenms_id() covers both the new per-server JSON format - # and legacy bare-integer values so neither is missed. - existing_vm = find_by_librenms_id(VirtualMachine, librenms_id, server_key) + # and legacy bare-integer values so neither is missed. An ambiguous id + # (matching multiple records) blocks the import — see _flag_ambiguous_librenms_id. + try: + existing_vm = find_by_librenms_id(VirtualMachine, librenms_id, server_key) + except AmbiguousLibreNMSIdError as exc: + existing_vm = None + _flag_ambiguous_librenms_id(result, librenms_id, exc) + + # Cross-model collision: the same librenms_id on both a VM and a Device is ambiguous. + # Without this, the VM lookup wins and the Device lookup below is skipped, silently + # binding to the VM. Detect it and fail closed (the device block is gated on the flag). + if existing_vm is not None: + try: + _device_collision = find_by_librenms_id(Device, librenms_id, server_key) + except AmbiguousLibreNMSIdError as exc: + # An ambiguous device lookup is itself a fail-closed condition: drop the + # VM binding too so the block below cannot rebind it as a definitive + # "librenms_id" match (the import is already flagged ambiguous). + _device_collision = None + _flag_ambiguous_librenms_id(result, librenms_id, exc) + existing_vm = None + if _device_collision is not None: + _flag_ambiguous_librenms_id(result, librenms_id, "matches both a VirtualMachine and a Device") + existing_vm = None if existing_vm: logger.info(f"Found existing VM: {existing_vm.name} (matched by librenms_id={librenms_id})") @@ -295,6 +402,9 @@ def validate_device_for_import( result["existing_match_type"] = "librenms_id" result["import_as_vm"] = True # Force VM mode since VM exists result["can_import"] = False + # Surface the host/OOB linkage so a librenms_id-matched VM renders as linked + # (mirrors the device path); otherwise the UI shows the VM as unlinked. + result["existing_librenms_link"] = _describe_existing_librenms_link(existing_vm, server_key) # Detect legacy bare-integer or string-digit format so UI can offer a migration action. # Direct access needed to detect legacy format for migration prompt: @@ -315,9 +425,14 @@ def validate_device_for_import( # Check for existing Device (by librenms_id custom field). # find_by_librenms_id() covers both the new per-server JSON format - # and legacy bare-integer values so neither is missed. - if not result["existing_device"]: - existing_device = find_by_librenms_id(Device, librenms_id, server_key) + # and legacy bare-integer values so neither is missed. Skip when an ambiguity + # (intra-model or cross-model) was already flagged — binding must fail closed. + if not result["existing_device"] and not result["ambiguous_librenms_id"]: + try: + existing_device = find_by_librenms_id(Device, librenms_id, server_key) + except AmbiguousLibreNMSIdError as exc: + existing_device = None + _flag_ambiguous_librenms_id(result, librenms_id, exc) if existing_device: logger.info(f"Found existing device: {existing_device.name} (matched by librenms_id={librenms_id})") @@ -325,6 +440,15 @@ def validate_device_for_import( result["existing_match_type"] = "librenms_id" result["can_import"] = False + # If the match was via the OOB sub-key, mark it so the UI shows no duplicate warning. + _existing_oob = get_librenms_oob(existing_device, server_key=server_key) + if _existing_oob and coerce_librenms_id(_existing_oob.get("id")) == coerce_librenms_id(librenms_id): + result["existing_match_type"] = "librenms_oob" + + # Surface the full host/OOB linkage so the import table can render + # both halves of an existing pair with consistent paired styling. + result["existing_librenms_link"] = _describe_existing_librenms_link(existing_device, server_key) + # Detect legacy bare-integer or string-digit format so UI can offer a migration action. # Direct access needed to detect legacy format for migration prompt: # LibreNMSAPI.get_librenms_id() returns an int in both formats, so only the @@ -356,9 +480,12 @@ def validate_device_for_import( result["name_sync_available"] = True result["suggested_name"] = hostname - # Check for serial drift on the linked device + # Check for serial drift on the linked device. Skip when the match was via the + # OOB sub-key (existing_match_type == "librenms_oob"): the incoming payload is the + # OOB controller's, so comparing it against the host record's serial would surface + # bogus replacement/conflict warnings on a row that is already correctly linked. incoming_serial = libre_device.get("serial") or "" - if incoming_serial and incoming_serial != "-": + if result["existing_match_type"] != "librenms_oob" and incoming_serial and incoming_serial != "-": if existing_device.serial and existing_device.serial == incoming_serial: result["serial_confirmed"] = True elif existing_device.serial and existing_device.serial != incoming_serial: @@ -380,8 +507,11 @@ def validate_device_for_import( f"LibreNMS: '{incoming_serial}'). Hardware may have been replaced." ) - # Only check hostname/serial/IP if not already matched by librenms_id - if not result["existing_device"]: + # Only check hostname/serial/IP if not already matched by librenms_id. + # Skip when an ambiguous librenms_id was flagged — hostname/serial/IP matching + # would otherwise rebind existing_device/existing_match_type and defeat the + # fail-closed ambiguity contract (mirrors the librenms_id block guard above). + if not result["existing_device"] and not result["ambiguous_librenms_id"]: # Check by hostname/name - Check both VMs and Devices for conflicts existing_vm = VirtualMachine.objects.filter(name__iexact=hostname).first() existing_device = Device.objects.filter(name__iexact=hostname).first() @@ -403,14 +533,28 @@ def validate_device_for_import( result["existing_device"] = existing_vm result["existing_match_type"] = "hostname" result["import_as_vm"] = True # Force VM mode since VM exists + # Describe the VM's current LibreNMS linkage and word the warning to match it — + # a hostname-matched VM can already carry a librenms_id (to a different id/server), + # so a flat "not linked" would contradict the badge. Mirrors the primary-IP path. + existing_link = _describe_existing_librenms_link(existing_vm, server_key) + result["existing_librenms_link"] = existing_link + if existing_link.get("host_id"): + link_note = f"already linked to LibreNMS device #{existing_link['host_id']}" + elif existing_link.get("oob_id"): + link_note = "already linked to LibreNMS as an OOB controller" + else: + link_note = "not linked to LibreNMS" result["warnings"].append( - f"VM with same hostname exists in NetBox as '{existing_vm.name}' (not linked to LibreNMS)" + f"VM with same hostname exists in NetBox as '{existing_vm.name}' ({link_note})" ) result["can_import"] = False elif existing_device: logger.info(f"Found existing device by hostname: {existing_device.name}") result["existing_device"] = existing_device result["existing_match_type"] = "hostname" + # Surface the current host/OOB linkage so a hostname-matched device that + # is already linked to LibreNMS isn't mislabelled as "not linked". + result["existing_librenms_link"] = _describe_existing_librenms_link(existing_device, server_key) # Check for serial conflict on hostname-matched device incoming_serial = libre_device.get("serial") or "" @@ -433,8 +577,15 @@ def validate_device_for_import( f"LibreNMS: '{incoming_serial}'). Hardware may have been replaced." ) else: + existing_link = result["existing_librenms_link"] or {} + if existing_link.get("host_id"): + link_note = f"currently linked to LibreNMS device #{existing_link['host_id']}" + elif existing_link.get("oob_id"): + link_note = "OOB already linked" + else: + link_note = "not linked to LibreNMS" result["warnings"].append( - f"Device with same hostname exists in NetBox as '{existing_device.name}' (not linked to LibreNMS)" + f"Device with same hostname exists in NetBox as '{existing_device.name}' ({link_note})" ) result["can_import"] = False @@ -450,18 +601,216 @@ def validate_device_for_import( result["existing_match_type"] = "serial" result["can_import"] = False - if existing_by_serial.name and existing_by_serial.name.lower() == hostname.lower(): - result["warnings"].append( - f"Device with same serial and hostname exists as '{existing_by_serial.name}' " - f"(not linked to LibreNMS)" + # Capture existing device's current LibreNMS linkage so the UI can + # present accurate state (NOT just "not linked to LibreNMS"). + existing_link = _describe_existing_librenms_link(existing_by_serial, server_key) + result["existing_librenms_link"] = existing_link + + # Compute both possible roles for the incoming LibreNMS device against + # the existing NetBox device, then pick a heuristic default. The UI + # offers a manual toggle whenever both roles are feasible so the user + # can override the heuristic (e.g. mark a "linux"-OS device as OOB or + # demote an apparent host into the OOB slot). + oob_type_from_libre = normalize_oob_type( + libre_device.get("os", ""), + libre_device.get("hardware", ""), + ) + existing_oob = get_librenms_oob(existing_by_serial, server_key=server_key) + + # Only treat this as a possible host/OOB chassis-pair situation when + # there is a real ambiguity: either the existing NetBox device's name + # differs from the incoming LibreNMS hostname (so they likely represent + # two sides of one physical box), or the existing device is already + # linked to a different LibreNMS id. When names match exactly and the + # existing has no link, the user almost certainly just wants to link. + names_match = bool( + existing_by_serial.name and existing_by_serial.name.lower() == hostname.lower() + ) + # Normalize to int so that a string device_id from the API + # (e.g. "17") doesn't cause a false "linked elsewhere" result + # when compared to the int host_id from coerce_librenms_id. + normalized_device_id = coerce_librenms_id(libre_device.get("device_id")) + already_linked_elsewhere = bool( + existing_link + and existing_link["host_id"] + and existing_link["host_id"] != normalized_device_id + ) + chassis_pair_likely = (not names_match) or already_linked_elsewhere + + oob_possible = chassis_pair_likely and existing_oob is None + host_possible = chassis_pair_likely and bool( + existing_link + and existing_link["host_id"] + and existing_link["host_id"] != normalized_device_id + and not existing_link.get("oob_id") + ) + existing_oob_from_name = _detect_oob_type_from_name(existing_by_serial.name) + + # --- Compute all values before mutating result --- + oob_candidate_data = None + if oob_possible: + inferred_oob_type = ( + oob_type_from_libre + or _detect_oob_type_from_name( + libre_device.get("hostname") or libre_device.get("sysName") or "" + ) + or "oob" ) - result["serial_action"] = "link" + oob_candidate_data = { + "device": existing_by_serial, + "type": inferred_oob_type, + "version": libre_device.get("version") or None, + "ip": libre_device.get("ip") or None, + } + + promote_to_host_data = None + if host_possible: + promote_to_host_data = { + "existing_libre_id": existing_link["host_id"], + "existing_oob_type": existing_oob_from_name or "oob", + # Included for bulk-collision detection: lets + # detect_bulk_collisions identify which NetBox device + # would be modified without an extra DB round-trip. + "existing_device": existing_by_serial, + } + + # Heuristic default: incoming-OS clearly OOB -> oob; otherwise if the + # existing device's NAME suggests it is the OOB and a host link can be + # demoted, offer promote; otherwise fall back to whichever is feasible. + if oob_type_from_libre and oob_possible: + serial_action_value = "oob_candidate" + elif host_possible and existing_oob_from_name: + serial_action_value = "promote_to_host" + elif oob_possible and host_possible: + # Both feasible but neither heuristic matches strongly -- + # default to oob_candidate (least-destructive), let the user flip. + serial_action_value = "oob_candidate" + elif oob_possible: + serial_action_value = "oob_candidate" + elif host_possible: + serial_action_value = "promote_to_host" else: + serial_action_value = None + + block_warnings: list = [] + if oob_type_from_libre and existing_oob is not None: + # OOB-typed incoming but existing already has an OOB linked -- + # inform without blocking. No actionable button in this branch. + serial_action_value = "link" + block_warnings.append( + f"Device '{existing_by_serial.name}' already has an OOB controller linked. " + f"Re-import will update the existing OOB entry." + ) + elif not oob_possible and not host_possible: + # Neither role is feasible -- fall back to legacy hostname/serial + # warning behaviour so the user still sees a useful message. + if existing_by_serial.name and existing_by_serial.name.lower() == hostname.lower(): + if existing_link and existing_link["host_id"]: + block_warnings.append( + f"Device with same serial and hostname exists as '{existing_by_serial.name}' " + f"(currently linked to LibreNMS device #{existing_link['host_id']})" + ) + else: + block_warnings.append( + f"Device with same serial and hostname exists as '{existing_by_serial.name}' " + f"(not linked to LibreNMS)" + ) + serial_action_value = "link" + else: + block_warnings.append( + f"Device with same serial ({serial}) exists as '{existing_by_serial.name}' " + f"but hostname differs (LibreNMS: '{hostname}'). Device may have been reinstalled." + ) + serial_action_value = "hostname_differs" + + apply_oob_detection_result( + result, + serial_action=serial_action_value, + oob_candidate=oob_candidate_data, + promote_to_host=promote_to_host_data, + serial_role_choice_available=oob_possible and host_possible, + warnings=block_warnings, + ) + + # Refresh local variable to reflect any VM-mode adjustments made during detection + # (e.g. existing VM found by hostname sets result["import_as_vm"] = True). + # Must happen before the merge-candidates block below so a VM hostname-match + # doesn't fall through to Device-only merge logic. + import_as_vm = result["import_as_vm"] + + # Stage 2 — merge-candidates detection. + # When the hostname-matched device and the serial-matched device are + # DIFFERENT NetBox objects, the two probably represent the same + # physical box (host + OOB) imported as separate entries. Surface + # this as a merge action instead of silently picking one. + try: + _serial_for_pair = (libre_device.get("serial") or "").strip() + if ( + _serial_for_pair + and _serial_for_pair != "-" + and not import_as_vm + and result.get("existing_device") is not None + and result.get("existing_match_type") in ("hostname", "serial") + ): + _hostname_match = ( + result["existing_device"] if result.get("existing_match_type") == "hostname" else None + ) + _serial_match = result["existing_device"] if result.get("existing_match_type") == "serial" else None + # Whichever path landed first, look the other one up too. Require a UNIQUE + # peer: serial isn't unique in NetBox (and names are only unique per site), + # so a bare .first() could pair the matched device with an arbitrary row and + # surface the wrong merge target. Fetch up to 2; only pair on exactly one, + # otherwise skip the suggestion and warn. + if _hostname_match and not _serial_match: + _serial_peers = list( + Device.objects.filter(serial=_serial_for_pair).exclude(pk=_hostname_match.pk)[:2] + ) + if len(_serial_peers) == 1: + _serial_match = _serial_peers[0] + elif len(_serial_peers) > 1: + result["warnings"].append( + f"Multiple NetBox devices share serial '{_serial_for_pair}'; merge suggestion skipped." + ) + elif _serial_match and not _hostname_match and hostname: + _hostname_peers = list( + Device.objects.filter(name__iexact=hostname).exclude(pk=_serial_match.pk)[:2] + ) + if len(_hostname_peers) == 1: + _hostname_match = _hostname_peers[0] + elif len(_hostname_peers) > 1: result["warnings"].append( - f"Device with same serial ({serial}) exists as '{existing_by_serial.name}' " - f"but hostname differs (LibreNMS: '{hostname}'). Device may have been reinstalled." + f"Multiple NetBox devices share hostname '{hostname}'; merge suggestion skipped." + ) + + if _hostname_match and _serial_match and _hostname_match.pk != _serial_match.pk: + host_link = _describe_existing_librenms_link(_hostname_match, server_key) + oob_link = _describe_existing_librenms_link(_serial_match, server_key) + # Conservative guard: at least one side must already be linked, + # otherwise this is more likely two unrelated devices that share + # serial data by coincidence (test fixtures, mis-keyed assets). + # A LibreNMS link counts whether it's a host link or an OOB link. + if any(link and (link.get("host_id") or link.get("oob_id")) for link in (host_link, oob_link)): + apply_merge_candidates( + result, + host_named={ + "pk": _hostname_match.pk, + "name": _hostname_match.name, + "librenms_link": host_link, + }, + oob_named={ + "pk": _serial_match.pk, + "name": _serial_match.name, + "librenms_link": oob_link, + }, + warning=( + f"Two NetBox devices appear to represent this physical box: " + f"'{_hostname_match.name}' (matches LibreNMS hostname) and " + f"'{_serial_match.name}' (matches chassis serial). " + f"Choose which one to keep and merge the other into it." + ), ) - result["serial_action"] = "hostname_differs" + except Exception: # pragma: no cover - defensive: never break validation + logger.exception("merge-candidate detection failed") # Check by primary IP (weaker match, IP could be reassigned) - only for devices if not result["existing_device"]: @@ -477,17 +826,76 @@ def validate_device_for_import( else None ) if device: - result["existing_device"] = device - result["existing_match_type"] = "primary_ip" - result["warnings"].append( - f"IP address {primary_ip} already assigned to device '{device.name}' (not linked to LibreNMS)" + # Surface any existing host/OOB linkage so the import UI renders the + # correct row state (the librenms_id / serial branches do the same; + # without this an already-linked device shows as "not linked" here). + result["existing_librenms_link"] = _describe_existing_librenms_link(device, server_key) + # Check if this is an OOB candidate via the IP path. + # The OOB controller's IP may already be the device's oob_ip, or the + # LibreNMS device may identify itself as an OOB type (iDRAC/iLO/etc.). + oob_type = normalize_oob_type( + libre_device.get("os", ""), + libre_device.get("hardware", ""), ) - result["can_import"] = False - - # Refresh local variable to reflect any VM-mode adjustments made during detection - # (e.g. existing VM found by hostname sets result["import_as_vm"] = True) + is_oob_ip = device.oob_ip_id is not None and existing_ip.pk == device.oob_ip_id + has_primary_ip = bool(device.primary_ip4_id or device.primary_ip6_id) + if oob_type and (is_oob_ip or not has_primary_ip): + existing_oob = get_librenms_oob(device, server_key=server_key) + if existing_oob is None: + result["existing_device"] = device + result["existing_match_type"] = "primary_ip" + result["serial_action"] = "oob_candidate" + result["oob_candidate"] = { + "device": device, + "type": oob_type, + "version": libre_device.get("version") or None, + "ip": libre_device.get("ip") or None, + } + result["can_import"] = False + else: + result["existing_device"] = device + result["existing_match_type"] = "primary_ip" + result["warnings"].append( + f"IP address {primary_ip} already assigned to device '{device.name}' " + f"(OOB already linked)" + ) + result["can_import"] = False + else: + result["existing_device"] = device + result["existing_match_type"] = "primary_ip" + # Line 728 may already have populated a host/OOB + # linkage; describe it accurately instead of always + # claiming "not linked to LibreNMS". + existing_link = result.get("existing_librenms_link") or {} + if existing_link.get("host_id"): + link_note = f"currently linked to LibreNMS device #{existing_link['host_id']}" + elif existing_link.get("oob_id"): + link_note = "OOB already linked" + else: + link_note = "not linked to LibreNMS" + result["warnings"].append( + f"IP address {primary_ip} already assigned to device '{device.name}' ({link_note})" + ) + result["can_import"] = False + + # Refresh local mode after ALL detection branches. The refresh at the top of the + # unmatched-device block only runs when nothing matched by librenms_id; an existing + # VM matched directly by librenms_id (above) sets result["import_as_vm"]=True but + # skips that block, so without this a linked VM would wrongly take the Device path + # (missing cluster["available_clusters"], running device-only validation/VC detection). import_as_vm = result["import_as_vm"] + # An ambiguous librenms_id (matches >1 NetBox record) is the terminal blocker for this + # row — the user must resolve the duplicate id. Don't run the new-import site/device_type/ + # role/cluster validation below: with existing_device fail-closed to None, it would pile + # unrelated "must select ..." blockers onto a row whose real problem is the ambiguous id + # (mirrors bulk_import.py treating ambiguity as the terminal state). existing_match_type + # is already "ambiguous_librenms_id" (set by _flag_ambiguous_librenms_id). + if result["ambiguous_librenms_id"]: + result["can_import"] = False + result["is_ready"] = False + return result + # Validate based on import type (Device or VM) if import_as_vm: # Always populate available clusters for all VMs (new or existing) so @@ -670,8 +1078,15 @@ def validate_device_for_import( result["device_role"]["found"] = True result["device_role"]["role"] = existing.role - # Check for device type mismatch between existing device and LibreNMS - if hasattr(existing, "device_type") and existing.device_type: + # Check for device type mismatch between existing device and LibreNMS. Skip for an + # OOB-sub-key match (existing_match_type == "librenms_oob"): the LibreNMS payload is + # the OOB controller's, so a host-vs-OOB device-type compare is a bogus "wrong device" + # warning on a correctly-linked row. + if ( + result.get("existing_match_type") != "librenms_oob" + and hasattr(existing, "device_type") + and existing.device_type + ): librenms_dt = result["device_type"].get("device_type") if librenms_dt and existing.device_type.pk != librenms_dt.pk: result["device_type_mismatch"] = True @@ -1012,3 +1427,18 @@ def fetch_device_with_cache( cache.set(cache_key, libre_device, timeout=api.cache_timeout) return libre_device + + +def __getattr__(name): + """Lazily re-export ``bulk_import_devices_shared`` to satisfy the + ``import_utils/device_operations.py`` export contract. + + A top-level ``from .bulk_import import bulk_import_devices_shared`` would be a + circular import (``bulk_import`` imports ``validate_device_for_import`` from this + module at load time), so PEP 562 defers the import until the attribute is accessed. + """ + if name == "bulk_import_devices_shared": + from .bulk_import import bulk_import_devices_shared + + return bulk_import_devices_shared + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") diff --git a/netbox_librenms_plugin/import_validation_helpers.py b/netbox_librenms_plugin/import_validation_helpers.py index cf26c6a40..1509f9217 100644 --- a/netbox_librenms_plugin/import_validation_helpers.py +++ b/netbox_librenms_plugin/import_validation_helpers.py @@ -137,6 +137,98 @@ def remove_validation_issue(validation: dict, keyword: str) -> None: validation["issues"] = [issue for issue in validation["issues"] if keyword.lower() not in issue.lower()] +def apply_oob_detection_result( + result: dict, + *, + serial_action: "str | None", + oob_candidate: "dict | None", + promote_to_host: "dict | None", + serial_role_choice_available: bool, + warnings: "list | None" = None, +) -> None: + """Apply OOB/promote-to-host serial detection results to the validation dict. + + Call this after computing all OOB/promote-to-host flags from the LibreNMS + and NetBox data. All mutations to ``result["oob_candidate"]``, + ``result["promote_to_host"]``, ``result["serial_action"]``, + ``result["serial_role_choice_available"]``, and their associated warnings + are routed through here so the mutation pattern stays consistent and + testable independently of the DB-heavy computation in device_operations. + + Args: + result: Validation dict produced by validate_device_for_import() + serial_action: The resolved action string, or None + oob_candidate: Dict {device, type, version, ip} when OOB role is available + promote_to_host: Dict {existing_libre_id, existing_oob_type, existing_device} + when host-promotion is available + serial_role_choice_available: True when both oob_candidate and + promote_to_host are feasible and the UI should offer a toggle + warnings: Optional list of warning strings to append to result["warnings"] + """ + result["serial_action"] = serial_action + result["oob_candidate"] = oob_candidate + # Honor the "absent otherwise" contract: only carry promote_to_host when a real + # promotion target exists, clearing any stale key rather than storing a None sentinel. + if promote_to_host is None: + result.pop("promote_to_host", None) + else: + result["promote_to_host"] = promote_to_host + result["serial_role_choice_available"] = serial_role_choice_available + # Clear merge-only state: this is the non-merge path, so if the same result + # dict was previously marked a merge candidate, the stale merge UI data must + # not linger (apply_merge_candidates is the only writer of merge_candidates). + result["merge_candidates"] = None + result.setdefault("warnings", []) + for warning in warnings or []: + result["warnings"].append(warning) + + +def apply_merge_candidates( + result: dict, + *, + host_named: dict, + oob_named: dict, + warning: str, +) -> None: + """Apply merge-candidates detection results to the validation dict. + + Called when the hostname-matched and serial-matched NetBox devices are + different objects and at least one already has a LibreNMS linkage, + indicating they likely represent the two sides of a single physical box. + + Sets ``serial_action`` to ``"merge_netbox_devices"``, populates + ``merge_candidates``, sets ``can_import`` to False, and appends the + supplied warning so callers do not need to know the dict shape. + + Args: + result: Validation dict produced by validate_device_for_import() + host_named: Dict {pk, name, librenms_link} for the hostname-matched device + oob_named: Dict {pk, name, librenms_link} for the serial-matched device + warning: Warning string describing the merge situation + """ + result["serial_action"] = "merge_netbox_devices" + result["merge_candidates"] = { + "host_named": host_named, + "oob_named": oob_named, + } + result["can_import"] = False + result["oob_candidate"] = None + # Clear earlier serial-conflict state so the merge path is the single source of truth: + # a hostname-first row may have already set serial_duplicate / serial_confirmed, which + # would otherwise leave a stale "serial conflict" signal alongside "merge these devices". + result["serial_duplicate"] = False + result["serial_confirmed"] = False + # "absent otherwise" contract — the merge path has no promotion target. + result.pop("promote_to_host", None) + result["serial_role_choice_available"] = False + # Merge supersedes the serial/hostname-detection signals that ran earlier in this + # validation pass, so their warnings (e.g. "hostname differs", "already has an OOB + # controller linked", "serial conflict") would now contradict the merge guidance. + # Reset to just the merge warning; later validation stages (role/platform/cluster) + # append their own warnings after this point, so nothing actionable is lost. + result["warnings"] = [warning] + + def recalculate_validation_status(validation: dict, is_vm: bool = False) -> None: """ Recalculate can_import and is_ready flags based on current validation state. diff --git a/netbox_librenms_plugin/librenms_api.py b/netbox_librenms_plugin/librenms_api.py index 86eba28f3..a8c3e9b73 100644 --- a/netbox_librenms_plugin/librenms_api.py +++ b/netbox_librenms_plugin/librenms_api.py @@ -12,6 +12,22 @@ logger = logging.getLogger(__name__) +def build_librenms_api(server_key): + """Return a :class:`LibreNMSAPI` for *server_key*, or ``None`` when the key is + unknown or the server is misconfigured. + + ``LibreNMSAPI(server_key=...)`` raises ``KeyError`` for an unknown non-default + key and ``ValueError`` when the URL/token is missing. Views take ``server_key`` + from request POST, where a stale page or tampered request can carry a key that + no longer exists — returning ``None`` lets the caller surface a user-facing + error instead of an unhandled 500. + """ + try: + return LibreNMSAPI(server_key=server_key) + except (KeyError, ValueError): + return None + + class LibreNMSAPI: """ Client to interact with the LibreNMS API and retrieve interface data for devices. @@ -65,8 +81,17 @@ def __init__(self, server_key=None): if servers_config and isinstance(servers_config, dict) and server_key in servers_config: # Multi-server configuration config = servers_config[server_key] - self.librenms_url = config["librenms_url"] - self.api_token = config["api_token"] + # A structurally invalid entry (None / non-mapping) would raise TypeError on + # the dict access below and bypass the ValueError contract that callers like + # build_librenms_api() rely on to fall back to None. Fail with ValueError, and + # read keys with .get() so a missing url/token is caught by the check below. + if not isinstance(config, dict): + raise ValueError( + f"LibreNMS server '{server_key}' is misconfigured " + f"(expected a mapping, got {type(config).__name__})." + ) + self.librenms_url = config.get("librenms_url") + self.api_token = config.get("api_token") self.cache_timeout = config.get("cache_timeout", 300) self.verify_ssl = config.get("verify_ssl", True) else: @@ -160,6 +185,12 @@ def get_available_servers(cls): # Multi-server configuration result = {} for key, config in servers_config.items(): + # Skip structurally invalid entries (e.g. {"prod": None}); mirrors the + # mapping guard in __init__ so a malformed config can't crash the server + # selector here with AttributeError on config.get(). + if not isinstance(config, dict): + logger.warning("Skipping malformed LibreNMS server config %r (expected a mapping).", key) + continue display_name = config.get("display_name", key) result[key] = display_name return result @@ -226,7 +257,7 @@ def get_librenms_id(self, obj): primary_ip_address = getattr(primary_ip, "address", None) ip_address = getattr(primary_ip_address, "ip", None) if primary_ip else None dns_name = getattr(primary_ip, "dns_name", None) if primary_ip else None - hostname = getattr(obj, "name", None) or None + hostname = getattr(obj, "name", None) # Try IP address if ip_address: @@ -255,15 +286,12 @@ def get_librenms_id(self, obj): def _normalize_librenms_id(value): """Coerce a raw LibreNMS ID value to int or None. - Booleans are rejected because bool is a subclass of int in Python, - so int(True) silently becomes 1 — a valid-looking device ID. + Thin wrapper around :func:`netbox_librenms_plugin.utils.coerce_librenms_id` + kept for back-compat with internal callers in this module. """ - if value is None or isinstance(value, bool): - return None - try: - return int(value) - except (ValueError, TypeError): - return None + from netbox_librenms_plugin.utils import coerce_librenms_id + + return coerce_librenms_id(value) def _get_cache_key(self, obj): """ diff --git a/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js b/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js index edfda206a..4845e031e 100644 --- a/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js +++ b/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js @@ -1203,12 +1203,77 @@ // the outer HTMX modal. Buttons inside nested modals (e.g. the // Promote-to-host modal rendered inside #htmx-modal-content) // must be left for Bootstrap's own dismiss handler so they - // close the inner modal, not the outer one. + // close the inner modal, not the outer one. We also avoid + // preventDefault here so form submit buttons that happen to + // carry data-bs-dismiss="modal" in nested modals still submit. const nearestModal = dismissTrigger.closest('.modal'); if (nearestModal === modalElement) { event.preventDefault(); hideModal(modalElement, fallbackBackdropRef); + } else if ( + nearestModal && + !(typeof bootstrap !== 'undefined' && bootstrap.Modal) && + !(typeof window.bootstrap !== 'undefined' && window.bootstrap.Modal) + ) { + // No-Bootstrap fallback: Bootstrap's own dismiss handler isn't available to + // close the nested modal, so the user would otherwise be stuck inside it. + // Manually hide just the nested modal (not the outer HTMX modal). + event.preventDefault(); + nearestModal.classList.remove('show'); + nearestModal.style.display = 'none'; + nearestModal.setAttribute('aria-hidden', 'true'); + nearestModal.removeAttribute('aria-modal'); + } + } + }); + + // Refresh the validation modal in place (used after promote / OOB + // attach actions that mutate device link state but should leave the + // user inside the modal so they can see the new state). Also closes + // any nested modals (e.g. the Promote-to-host pick modal) before + // re-fetching so the user sees the refreshed validation directly. + document.body.addEventListener('validationRefresh', function (event) { + // Close any nested Bootstrap modals currently open inside the + // outer validation modal content. Use the same detection order as + // the rest of the plugin: bare `bootstrap` global first (preferred), + // then `window.bootstrap` as fallback, then plain DOM toggling. + document.querySelectorAll('#htmx-modal-content .modal.show').forEach(function (nested) { + try { + // Prefer Bootstrap: it tracks stacked modals and leaves the + // outer HTMX modal's backdrop/body state intact. Only fall + // back to a minimal DOM hide — never hideModal()/_hideManual, + // which strips body.modal-open and the (shared, outer) backdrop + // and would break the still-open outer modal. + if (typeof bootstrap !== 'undefined' && bootstrap.Modal) { + bootstrap.Modal.getOrCreateInstance(nested).hide(); + } else if (typeof window.bootstrap !== 'undefined' && window.bootstrap.Modal) { + window.bootstrap.Modal.getOrCreateInstance(nested).hide(); + } else { + nested.classList.remove('show'); + nested.style.display = 'none'; + nested.setAttribute('aria-hidden', 'true'); + nested.removeAttribute('aria-modal'); + } + } catch (err) { + // Swallow - we still want to refresh the validation panel. } + }); + + const deviceId = event.detail && (event.detail.deviceId || event.detail.device_id); + if (!deviceId) { + return; + } + const btn = document.querySelector( + 'tr#device-row-' + deviceId + ' button[hx-get*="/validation/' + deviceId + '/"]' + ); + if (btn) { + // htmx registers a delegated click handler on document, so a + // synthetic MouseEvent click on the row's "View details" + // button re-triggers the validation GET and swaps the new + // content into #htmx-modal-content. We cannot call + // `htmx.trigger()` directly because NetBox does not expose + // the htmx global to user scripts. + btn.dispatchEvent(new MouseEvent('click', {bubbles: true, cancelable: true, view: window})); } }); @@ -1222,6 +1287,12 @@ // Handle Escape key to close modal document.addEventListener('keydown', function (event) { if (event.key === 'Escape') { + // A nested dialog (e.g. the promote-to-host modal rendered inside + // #htmx-modal-content) owns Escape while it is open — let Bootstrap close + // the topmost child, don't tear down the whole validation modal underneath it. + if (document.querySelector('#htmx-modal-content .modal.show')) { + return; + } if (modalElement?.classList.contains('show')) { hideModal(modalElement, fallbackBackdropRef); } diff --git a/netbox_librenms_plugin/tables/cables.py b/netbox_librenms_plugin/tables/cables.py index cad4660b3..d8bf89367 100644 --- a/netbox_librenms_plugin/tables/cables.py +++ b/netbox_librenms_plugin/tables/cables.py @@ -61,9 +61,16 @@ def render_remote_device(self, value, record): def render_local_port(self, value, record): """Render local port name as a link if URL is available.""" + # Static trusted markup — use mark_safe, not format_html (which requires + # interpolation args and raises TypeError when given a bare string in Django 6+). + oob_badge = ( + mark_safe(' OOB') # noqa: S308 + if record.get("_source") == "oob" + else "" + ) if url := record.get("local_port_url"): - return format_html('{}', url, value) - return value + return format_html('{}{}', url, value, oob_badge) + return format_html("{}{}", value or "", oob_badge) def render_remote_port(self, value, record): """Render remote port name as a link if URL is available.""" diff --git a/netbox_librenms_plugin/tables/device_status.py b/netbox_librenms_plugin/tables/device_status.py index cb71b7ef1..92aa1ada6 100644 --- a/netbox_librenms_plugin/tables/device_status.py +++ b/netbox_librenms_plugin/tables/device_status.py @@ -467,11 +467,47 @@ def render_actions(self, value, record): match_type = validation.get("existing_match_type", "") serial_action = validation.get("serial_action") has_mismatch = validation.get("device_type_mismatch", False) - has_actions = match_type == "hostname" or (match_type == "serial" and serial_action is not None) + is_oob_candidate = serial_action == "oob_candidate" + is_oob_linked = match_type == "librenms_oob" + has_actions = match_type == "hostname" or ( + match_type == "serial" and serial_action is not None and not is_oob_candidate + ) has_name_sync = validation.get("name_sync_available", False) has_sync_needed = match_type == "librenms_id" and serial_action in ("update_serial", "conflict") - if has_mismatch: + existing_link = validation.get("existing_librenms_link") or {} + paired_oob_id = existing_link.get("oob_id") + paired_host_id = existing_link.get("host_id") + paired_oob_type = existing_link.get("oob_type") or "OOB" + + def _coerce_pair_id(value): + try: + return int(value) + except (TypeError, ValueError): + return None + + if is_oob_candidate: + btn_class = "btn-outline-purple" + btn_icon = "mdi-chip" + btn_label = " OOB" + btn_title = "Add as OOB controller" + elif is_oob_linked: + # This LibreNMS row is the OOB half of an existing pair. + btn_class = "btn-outline-info" + btn_icon = "mdi-chip" + btn_label = " OOB" + if paired_host_id is not None: + try: + paired_host_id_int = int(paired_host_id) + except (TypeError, ValueError): + paired_host_id_int = None + if paired_host_id_int is not None: + btn_title = f"Linked as OOB controller (paired host: LibreNMS #{paired_host_id_int})" + else: + btn_title = "Linked as OOB controller" + else: + btn_title = "Linked as OOB controller" + elif has_mismatch: btn_class = "btn-outline-danger" btn_icon = "mdi-alert-circle" btn_label = " Conflict" @@ -481,6 +517,31 @@ def render_actions(self, value, record): btn_icon = "mdi-alert" btn_label = " Conflict" btn_title = "View conflict details" + elif ( + match_type == "librenms_id" + and paired_oob_id is not None + and _coerce_pair_id(paired_oob_id) != _coerce_pair_id(paired_host_id) + ): + # This LibreNMS row is the host half of an existing host/OOB + # pair. Render it with the same info-tinted styling as the OOB + # row so the user sees them as one paired device rather than + # two unrelated statuses (one green "ready", one blue "OOB"). + # Evaluated BEFORE the generic name-sync/sync-needed branch so a + # paired host that also needs review still shows the Host state + # rather than falling back to the generic warning button. + btn_class = "btn-outline-info" + btn_icon = "mdi-server-network" + btn_label = " Host" + # paired_oob_type comes from a user-editable custom field + # (librenms_id..oob.type) and is only string-type-checked, + # not sanitised, on the read path. Escape before interpolating + # into the title attribute to prevent stored XSS. + # paired_oob_id may be a string-digit from older serialization; + # coerce to int so display is clean. + _oob_id_fmt = ( + _coerce_pair_id(paired_oob_id) if _coerce_pair_id(paired_oob_id) is not None else paired_oob_id + ) + btn_title = f"Linked as host (paired OOB: LibreNMS #{escape(str(_oob_id_fmt))}, {escape(paired_oob_type or '')})" elif has_name_sync or has_sync_needed: btn_class = "btn-outline-warning" btn_icon = "mdi-information-outline" diff --git a/netbox_librenms_plugin/tables/interfaces.py b/netbox_librenms_plugin/tables/interfaces.py index d99d2a7bf..5a3da85a0 100644 --- a/netbox_librenms_plugin/tables/interfaces.py +++ b/netbox_librenms_plugin/tables/interfaces.py @@ -300,7 +300,15 @@ def render_speed(self, value, record): def render_name(self, value, record): """Render interface name with appropriate styling based on comparison with NetBox""" - return self._render_field(value, record, self.interface_name_field, "name") + rendered = self._render_field(value, record, self.interface_name_field, "name") + badges = "" + if record.get("_source") == "oob": + badges += 'OOB' + if record.get("_dedup_conflict"): + badges += 'Shared LOM' + if badges: + return format_html("{}{}", rendered, mark_safe(badges)) + return rendered def _get_interface_status_display(self, enabled, record): """ diff --git a/netbox_librenms_plugin/tables/ipaddresses.py b/netbox_librenms_plugin/tables/ipaddresses.py index 0324c6de2..241698463 100644 --- a/netbox_librenms_plugin/tables/ipaddresses.py +++ b/netbox_librenms_plugin/tables/ipaddresses.py @@ -14,6 +14,17 @@ class IPAddressTable(tables.Table): def __init__(self, *args, **kwargs): """Initialize IP address table.""" super().__init__(*args, **kwargs) + # Identify the owning sync tab so the paginator links (inc/paginator.html builds + # ?tab={{ table.tab }}) keep the user on the IP Addresses tab. Without this, table.tab + # renders empty and paging falls back to the default (Interfaces) tab. + self.tab = "ipaddresses" + # Give this table its own pagination namespace. configure() passes self.prefix to + # get_table_paginate_count() (and RequestConfig), which keys per_page on + # "{prefix}per_page"; left empty, the IP table shares the generic per-page param with + # other tabs instead of reading "ipaddresses_per_page". Orthogonal to self.tab. Matches + # the cables/modules/interfaces/vlans tables; preserve an explicit caller override. + if not self.prefix: + self.prefix = "ipaddresses_" class Meta: """Meta options for IPAddressTable.""" diff --git a/netbox_librenms_plugin/tables/modules.py b/netbox_librenms_plugin/tables/modules.py index b3f7f72f8..015c719f7 100644 --- a/netbox_librenms_plugin/tables/modules.py +++ b/netbox_librenms_plugin/tables/modules.py @@ -147,16 +147,26 @@ def render_name(self, value, record): rendered_name = display_name depth = record.get("depth", 0) + # Static trusted markup — use mark_safe, not format_html (which requires + # interpolation args and raises TypeError when given a bare string). + oob_badge = ( + mark_safe('OOB') # noqa: S308 + if record.get("_source") == "oob" + else "" + ) if depth == 0: - return rendered_name + return format_html("{}{}", rendered_name, oob_badge) # Build visual tree prefix based on nesting depth padding_px = depth * 20 prefix = "└─ " + # Keep the OOB badge inside the padded container so it stays indented + # with the module name on nested rows (was rendering at column 0). return format_html( - '{}{}', + '{}{}{}', padding_px, prefix, rendered_name, + oob_badge, ) def render_model(self, value, record): diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_cable_sync_content.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_cable_sync_content.html index 485676360..57aafb6e7 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_cable_sync_content.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_cable_sync_content.html @@ -3,16 +3,24 @@ {% if cable_sync.table %} +{# Migrated donors must not sync: hiding only the button leaves the POST form live (Enter in a filter still submits), so drop the form in migrated mode. #} +{% if migrated_to_marker %} +
+{% else %}
+ {# Form-specific markup belongs only inside the ; in migrated mode the wrapper is a bare
and a CSRF token / hidden inputs there are dead, misleading markup. #} {% csrf_token %} {% if cable_sync.server_key %}{% endif %} +{% endif %}
+ {% if not migrated_to_marker %} + {% endif %} info @@ -70,7 +78,11 @@
+{% if migrated_to_marker %} +
+{% else %} +{% endif %} {% else %}
diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_interface_sync.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_interface_sync.html index fa1d3c0d7..2abe00136 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_interface_sync.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_interface_sync.html @@ -10,14 +10,16 @@

Interface Sync

{% if has_librenms_id %} {% with model_name=object|meta:"model_name" %} {% if model_name == "device" %} - {% elif model_name == "virtualmachine" %} - + {% endif %} info @@ -198,7 +221,11 @@
+{% if migrated_to_marker %} + +{% else %} +{% endif %} {% else %}
@@ -311,6 +338,7 @@