diff --git a/netbox_librenms_plugin/api/serializers.py b/netbox_librenms_plugin/api/serializers.py index fe576d712a..89b1436985 100644 --- a/netbox_librenms_plugin/api/serializers.py +++ b/netbox_librenms_plugin/api/serializers.py @@ -9,6 +9,7 @@ ModuleTypeMapping, NormalizationRule, PlatformMapping, + PortStackLagPattern, ) @@ -129,3 +130,13 @@ class Meta: "carrier_module_type", "description", ] + + +class PortStackLagPatternSerializer(NetBoxModelSerializer): + """Serialize PortStackLagPattern model for REST API.""" + + class Meta: + """Meta options for PortStackLagPatternSerializer.""" + + model = PortStackLagPattern + fields = ["id", "librenms_os", "lag_name_pattern", "description"] diff --git a/netbox_librenms_plugin/api/urls.py b/netbox_librenms_plugin/api/urls.py index de5f82b4aa..e6fe47d897 100644 --- a/netbox_librenms_plugin/api/urls.py +++ b/netbox_librenms_plugin/api/urls.py @@ -14,6 +14,7 @@ router.register("inventory-ignore-rules", views.InventoryIgnoreRuleViewSet) router.register("platform-mappings", views.PlatformMappingViewSet) router.register("carrier-auto-install-rules", views.CarrierAutoInstallRuleViewSet) +router.register("port-stack-lag-patterns", views.PortStackLagPatternViewSet) urlpatterns = [ path("jobs//sync-status/", views.sync_job_status, name="sync_job_status"), diff --git a/netbox_librenms_plugin/api/views.py b/netbox_librenms_plugin/api/views.py index a73fb8069f..f37cef36b6 100644 --- a/netbox_librenms_plugin/api/views.py +++ b/netbox_librenms_plugin/api/views.py @@ -21,6 +21,7 @@ ModuleTypeMappingFilterSet, NormalizationRuleFilterSet, PlatformMappingFilterSet, + PortStackLagPatternFilterSet, ) from netbox_librenms_plugin.jobs import FilterDevicesJob, ImportDevicesJob from netbox_librenms_plugin.models import ( @@ -32,6 +33,7 @@ ModuleTypeMapping, NormalizationRule, PlatformMapping, + PortStackLagPattern, ) from .serializers import ( @@ -43,6 +45,7 @@ ModuleTypeMappingSerializer, NormalizationRuleSerializer, PlatformMappingSerializer, + PortStackLagPatternSerializer, ) logger = logging.getLogger(__name__) @@ -144,6 +147,16 @@ class CarrierAutoInstallRuleViewSet(NetBoxModelViewSet): serializer_class = CarrierAutoInstallRuleSerializer +class PortStackLagPatternViewSet(NetBoxModelViewSet): + """API viewset for PortStackLagPattern CRUD operations.""" + + permission_classes = [LibreNMSPluginPermission] + filterset_class = PortStackLagPatternFilterSet + + queryset = PortStackLagPattern.objects.all() + serializer_class = PortStackLagPatternSerializer + + @api_view(["POST"]) @permission_classes([LibreNMSPluginPermission]) def sync_job_status(request, job_pk): diff --git a/netbox_librenms_plugin/filters.py b/netbox_librenms_plugin/filters.py index 1edb57773c..a00fd1d486 100644 --- a/netbox_librenms_plugin/filters.py +++ b/netbox_librenms_plugin/filters.py @@ -1,5 +1,6 @@ import django_filters from dcim.models import Manufacturer +from django.db.models import Q from .models import ( CarrierAutoInstallRule, @@ -10,6 +11,7 @@ ModuleTypeMapping, NormalizationRule, PlatformMapping, + PortStackLagPattern, ) @@ -143,3 +145,24 @@ class Meta: "librenms_child_name_pattern", "netbox_bay_name_pattern", ] + + +class PortStackLagPatternFilterSet(django_filters.FilterSet): + """Filter set for PortStackLagPattern model.""" + + q = django_filters.CharFilter(method="search") + librenms_os = django_filters.CharFilter(lookup_expr="icontains") + lag_name_pattern = django_filters.CharFilter(lookup_expr="icontains") + description = django_filters.CharFilter(lookup_expr="icontains") + + def search(self, queryset, _name, value): + """Search the fields exposed by the pattern list.""" + return queryset.filter( + Q(librenms_os__icontains=value) | Q(lag_name_pattern__icontains=value) | Q(description__icontains=value) + ) + + class Meta: + """Meta options.""" + + model = PortStackLagPattern + fields = ["librenms_os", "lag_name_pattern", "description"] diff --git a/netbox_librenms_plugin/forms.py b/netbox_librenms_plugin/forms.py index 1dbc10c87c..1260488ae4 100644 --- a/netbox_librenms_plugin/forms.py +++ b/netbox_librenms_plugin/forms.py @@ -32,6 +32,7 @@ ModuleTypeMapping, NormalizationRule, PlatformMapping, + PortStackLagPattern, ) logger = logging.getLogger(__name__) @@ -705,6 +706,36 @@ class PlatformMappingFilterForm(NetBoxModelFilterSetForm): model = PlatformMapping +class PortStackLagPatternForm(NetBoxModelForm): + """Form for creating and editing PortStackLagPattern objects.""" + + class Meta: + """Meta options.""" + + model = PortStackLagPattern + fields = ["librenms_os", "lag_name_pattern", "description"] + + +class PortStackLagPatternImportForm(NetBoxModelImportForm): + """Form for bulk importing PortStackLagPattern objects from CSV/JSON/YAML.""" + + class Meta: + """Meta options.""" + + model = PortStackLagPattern + fields = ["librenms_os", "lag_name_pattern", "description"] + + +class PortStackLagPatternFilterForm(NetBoxModelFilterSetForm): + """Form for filtering PortStackLagPattern objects.""" + + librenms_os = forms.CharField(required=False, label="LibreNMS OS") + lag_name_pattern = forms.CharField(required=False, label="LAG Name Pattern") + description = forms.CharField(required=False, label="Description") + + model = PortStackLagPattern + + class BaseSNMPForm(forms.Form): """ Base form with fields shared by both SNMPv1/v2c and SNMPv3 LibreNMS device forms. diff --git a/netbox_librenms_plugin/interface_relationships.py b/netbox_librenms_plugin/interface_relationships.py new file mode 100644 index 0000000000..d4bdba7687 --- /dev/null +++ b/netbox_librenms_plugin/interface_relationships.py @@ -0,0 +1,450 @@ +"""Shared interface relationship discovery and row resolution.""" + +from dataclasses import dataclass + +from dcim.models import Device, Interface +from django.db.models import Q +from virtualization.models import VirtualMachine, VMInterface + +from netbox_librenms_plugin.utils import ( + build_librenms_id_qs, + get_librenms_device_id, + interface_name_fallback_matches_port, + is_list_of_dicts, + normalize_librenms_port_id, + normalize_relationship_maps, +) + + +RELATIONSHIP_CANDIDATE_BATCH_SIZE = 64 + + +@dataclass(frozen=True) +class RelationshipMaps: + """Normalized cached relationship data for one LibreNMS snapshot.""" + + lag_members: dict + sub_interfaces: dict + ports_by_id: dict + + +@dataclass(frozen=True) +class RelationshipResolutionContext: + """Indexes and permission state shared by every row resolver.""" + + obj: object + server_key: str + catalog_index: dict + display_index: dict + related_index: dict + source_index: dict + actionable_owner_ids: set + changeable_interface_ids: set + can_write: bool + + +def interface_owner(interface): + """Return the ``(device_id, virtual_machine_id)`` owner tuple for an interface.""" + return (getattr(interface, "device_id", None), getattr(interface, "virtual_machine_id", None)) + + +def interface_owner_for_object(obj): + """Return the owner tuple for a Device or VirtualMachine.""" + if isinstance(obj, Device): + return (obj.pk, None) + return (None, obj.pk) + + +def interface_queryset_for_object(obj): + """Return the interfaces in a Device chassis or VirtualMachine scope.""" + if isinstance(obj, Device): + if obj.virtual_chassis_id is not None: + member_ids = obj.virtual_chassis.members.values_list("id", flat=True) + return Interface.objects.filter(device__in=member_ids) + return Interface.objects.filter(device=obj) + if isinstance(obj, VirtualMachine): + return VMInterface.objects.filter(virtual_machine=obj) + return None + + +def relationship_candidate_q(server_key, port_ids, names): + """Build one query for stable IDs and safe name hints.""" + candidate_q = Q(pk__in=[]) + unique_port_ids = { + (type(port_id).__name__, str(port_id)): port_id + for port_id in port_ids + if normalize_librenms_port_id(port_id) is not None + } + for marker in sorted(unique_port_ids): + port_id = unique_port_ids[marker] + host_q, oob_q = build_librenms_id_qs(server_key, port_id) + candidate_q |= host_q | oob_q + unique_names = sorted({name for name in names if isinstance(name, str) and name}) + if unique_names: + candidate_q |= Q(name__in=unique_names) + return candidate_q + + +def relationship_candidate_ids(obj, server_key, port_ids, names): + """Find relationship candidates in bounded batches.""" + interface_queryset = interface_queryset_for_object(obj) + candidate_ids = set() + unique_port_ids = sorted( + {port_id for raw_port_id in port_ids if (port_id := normalize_librenms_port_id(raw_port_id)) is not None} + ) + unique_names = sorted({name for name in names if isinstance(name, str) and name}) + + for start in range(0, len(unique_port_ids), RELATIONSHIP_CANDIDATE_BATCH_SIZE): + batch = unique_port_ids[start : start + RELATIONSHIP_CANDIDATE_BATCH_SIZE] + candidate_ids.update( + interface_queryset.filter(relationship_candidate_q(server_key, batch, ())).values_list("pk", flat=True) + ) + + for start in range(0, len(unique_names), RELATIONSHIP_CANDIDATE_BATCH_SIZE): + batch = unique_names[start : start + RELATIONSHIP_CANDIDATE_BATCH_SIZE] + candidate_ids.update(interface_queryset.filter(name__in=batch).values_list("pk", flat=True)) + + return candidate_ids + + +def build_interface_index(obj, server_key, user=None, action="change", *, lock=False, allowed_ids=None): + """Build an ambiguity-preserving interface index for repeated resolution.""" + interface_queryset = interface_queryset_for_object(obj) + if interface_queryset is None: + return None + + if allowed_ids is not None: + interface_queryset = interface_queryset.filter(pk__in=allowed_ids) + elif user is not None: + interface_queryset = interface_queryset.restrict(user, action) + if isinstance(obj, Device): + interface_queryset = interface_queryset.select_related( + "bridge__device__virtual_chassis", + "device__virtual_chassis", + "device__location", + "device__rack", + "device__site", + "lag__device__virtual_chassis", + "parent__device__virtual_chassis", + "untagged_vlan__site", + ) + else: + interface_queryset = interface_queryset.select_related( + "bridge__virtual_machine", + "virtual_machine", + "virtual_machine__site", + "parent__virtual_machine", + "untagged_vlan__site", + ) + if lock: + interface_queryset = interface_queryset.select_for_update(of=("self",)).order_by("pk") + + by_librenms_id = {} + by_name = {} + for interface in interface_queryset: + stored_id = normalize_librenms_port_id(get_librenms_device_id(interface, server_key, auto_save=False)) + if stored_id is not None: + by_librenms_id.setdefault(stored_id, []).append(interface) + by_name.setdefault(interface.name, []).append(interface) + return {"by_lnms_id": by_librenms_id, "by_name": by_name} + + +def filter_interface_index(index, allowed_ids): + """Return an interface index limited to the supplied primary keys.""" + return { + key: { + value: [interface for interface in interfaces if interface.pk in allowed_ids] + for value, interfaces in mapping.items() + if any(interface.pk in allowed_ids for interface in interfaces) + } + for key, mapping in index.items() + } + + +def resolve_interface_by_port_id( + obj, port_id: str, server_key: str, name_hint: str = "", expected_owner=None, index=None +): + """Resolve one stable LibreNMS port ID, with a safe exact-name fallback.""" + if not port_id: + return None, "port_id is required" + + if index is None: + index = build_interface_index(obj, server_key) + if index is None: + return None, f"Unsupported object type: {type(obj).__name__}" + + target_id = normalize_librenms_port_id(port_id) + matches = list(index["by_lnms_id"].get(target_id, [])) if target_id is not None else [] + if len(matches) > 1: + return None, f"LibreNMS port_id {port_id} is ambiguous on {obj} (matches multiple interfaces)" + if expected_owner is not None: + owned = [match for match in matches if interface_owner(match) == expected_owner] + if len(owned) == 1: + return owned[0], None + elif len(matches) == 1: + return matches[0], None + + if name_hint: + interface, error = _resolve_interface_by_name_hint( + obj, + name_hint, + index=index, + expected_owner=expected_owner, + ) + if error: + return None, error + if interface is not None: + if expected_owner is not None and interface_owner(interface) != expected_owner: + return None, f"Interface name '{name_hint}' resolves to a different owner than the selected row" + if not interface_name_fallback_matches_port(interface, target_id, server_key): + stored_id = normalize_librenms_port_id(get_librenms_device_id(interface, server_key, auto_save=False)) + return None, f"Interface name '{name_hint}' is already bound to LibreNMS port_id {stored_id}" + return interface, None + + if expected_owner is not None and matches: + return None, f"LibreNMS port_id {port_id} resolves to a different owner than the selected row" + return None, f"Interface with LibreNMS port_id {port_id} not found on {obj}" + + +def _resolve_interface_by_name_hint(obj, name_hint, index=None, expected_owner=None): + """Resolve one exact interface name while preserving ambiguity.""" + if index is not None: + matches = index["by_name"].get(name_hint, []) + if expected_owner is not None: + owned = [match for match in matches if interface_owner(match) == expected_owner] + if owned: + matches = owned + if not matches: + return None, None + if len(matches) > 1: + return None, f"Interface name '{name_hint}' is ambiguous on {obj}" + return matches[0], None + try: + if isinstance(obj, Device): + if expected_owner is not None and expected_owner[0] is not None: + interface = Interface.objects.get(device_id=expected_owner[0], name=name_hint) + elif obj.virtual_chassis_id is not None: + member_ids = obj.virtual_chassis.members.values_list("id", flat=True) + interface = Interface.objects.get(device__in=member_ids, name=name_hint) + else: + interface = Interface.objects.get(device=obj, name=name_hint) + else: + interface = VMInterface.objects.get(virtual_machine=obj, name=name_hint) + return interface, None + except (Interface.DoesNotExist, VMInterface.DoesNotExist): + return None, None + except (Interface.MultipleObjectsReturned, VMInterface.MultipleObjectsReturned): + return None, f"Interface name '{name_hint}' is ambiguous on {obj}" + + +def build_relationship_maps(cached_data): + """Normalize relationship maps and index host ports by stable ID.""" + lag_members, sub_interfaces = normalize_relationship_maps(cached_data.get("port_stack_relationships")) + ports = cached_data.get("ports", []) + if not is_list_of_dicts(ports): + ports = [] + ports_by_id = {} + for port in ports: + if port.get("_source") == "oob": + continue + port_id = normalize_librenms_port_id(port.get("port_id")) + if port_id is not None: + ports_by_id[port_id] = port + return RelationshipMaps(lag_members, sub_interfaces, ports_by_id) + + +def build_candidate_relationship_context(obj, server_key, user, can_write, port_ids, names): + """Build one permission-aware relationship context for a bounded row set.""" + candidate_queryset = interface_queryset_for_object(obj).filter( + relationship_candidate_q(server_key, port_ids, names) + ) + catalog_ids = set(candidate_queryset.values_list("pk", flat=True)) + catalog_index = build_interface_index(obj, server_key, allowed_ids=catalog_ids) + + owner_model = Device if isinstance(obj, Device) else VirtualMachine + owner_field = "device_id" if isinstance(obj, Device) else "virtual_machine_id" + owner_ids = candidate_queryset.values_list(owner_field, flat=True) + actionable_owner_ids = set( + owner_model.objects.restrict(user, "view").filter(pk__in=owner_ids).values_list("pk", flat=True) + ) + permitted_queryset = candidate_queryset.filter(**{f"{owner_field}__in": actionable_owner_ids}) + viewable_ids = set(permitted_queryset.restrict(user, "view").values_list("pk", flat=True)) + changeable_ids = set(permitted_queryset.restrict(user, "change").values_list("pk", flat=True)) + display_index = build_interface_index(obj, server_key, allowed_ids=viewable_ids | changeable_ids) + source_index = filter_interface_index(display_index, changeable_ids) + return RelationshipResolutionContext( + obj=obj, + server_key=server_key, + catalog_index=catalog_index, + display_index=display_index, + related_index=display_index, + source_index=source_index, + actionable_owner_ids=actionable_owner_ids, + changeable_interface_ids=changeable_ids, + can_write=can_write, + ) + + +def enrich_port_relationships( + port, + relationship_maps, + interface_name_field="ifName", + server_key="", +): + """Add LAG and parent comparison fields to a cached port row.""" + port_id = normalize_librenms_port_id(port.get("port_id")) + netbox_interface = port.get("netbox_interface") + + def related_interface_matches(netbox_related, librenms_related): + if netbox_related is None or librenms_related is None: + return False + stored_id = normalize_librenms_port_id( + get_librenms_device_id(netbox_related, server_key or "default", auto_save=False) + ) + target_id = normalize_librenms_port_id(librenms_related.get("port_id")) + if stored_id is not None and target_id is not None: + return stored_id == target_id + return netbox_related.name in ( + librenms_related.get("ifName"), + librenms_related.get("ifDescr"), + librenms_related.get(interface_name_field), + ) + + def relationship_context(port_id_to_related, related_attribute): + related_port_id = normalize_librenms_port_id(port_id_to_related.get(port_id)) if port_id else None + related_port = relationship_maps.ports_by_id.get(related_port_id) if related_port_id else None + related_name = related_port.get(interface_name_field) if related_port else None + netbox_related = getattr(netbox_interface, related_attribute, None) if netbox_interface else None + if related_port_id and netbox_interface: + if netbox_related and related_interface_matches(netbox_related, related_port): + status = "match" + elif netbox_related: + status = "mismatch" + else: + status = "missing_nb" + elif related_port_id: + status = "missing_nb" + elif netbox_related: + status = "missing_lnms" + else: + status = None + return related_name, related_port_id, status + + lag_name, lag_port_id, lag_status = relationship_context(relationship_maps.lag_members, "lag") + port["librenms_lag_name"] = lag_name + port["librenms_lag_port_id"] = lag_port_id + port["lag_sync_status"] = lag_status + + parent_name, parent_port_id, parent_status = relationship_context( + relationship_maps.sub_interfaces, + "parent", + ) + port["librenms_parent_name"] = parent_name + port["librenms_parent_port_id"] = parent_port_id + port["parent_sync_status"] = parent_status + + +def resolve_relationship_row( + context, + port, + owner, + interface_name_field, + unique_host_port_ids, + unambiguous_name_port_ids, + relationship_maps, +): + """Resolve and enrich one relationship row using the shared table and verify rules.""" + port_id = normalize_librenms_port_id(port.get("port_id")) + if port_id is not None: + port["port_id"] = port_id + if port.get("_source") == "oob": + port["netbox_interface"] = None + port["exists_in_netbox"] = False + port["name_fallback_allowed"] = False + port["relationship_source_resolvable"] = False + port["lag_target_resolvable"] = False + port["parent_target_resolvable"] = False + return None + + name_fallback_allowed = port_id in unambiguous_name_port_ids + name_hint = (port.get(interface_name_field) or "") if name_fallback_allowed else "" + expected_owner = interface_owner_for_object(owner) + resolved_interface = None + if port_id is not None: + resolved_interface, _ = resolve_interface_by_port_id( + context.obj, + str(port_id), + context.server_key, + name_hint=name_hint, + expected_owner=expected_owner, + index=context.display_index, + ) + port["netbox_interface"] = resolved_interface + port["exists_in_netbox"] = resolved_interface is not None + port["name_fallback_allowed"] = name_fallback_allowed and resolved_interface is not None + + source_is_resolvable = False + if ( + context.can_write + and owner.pk in context.actionable_owner_ids + and port_id in unique_host_port_ids + and resolved_interface is not None + ): + catalog_matches = context.catalog_index["by_lnms_id"].get(port_id, []) + if catalog_matches: + source_is_resolvable = ( + len(catalog_matches) == 1 + and catalog_matches[0].pk == resolved_interface.pk + and resolved_interface.pk in context.changeable_interface_ids + ) + elif name_fallback_allowed: + source_interface, error = resolve_interface_by_port_id( + context.obj, + str(port_id), + context.server_key, + name_hint=name_hint, + expected_owner=expected_owner, + index=context.source_index, + ) + source_is_resolvable = error is None and getattr(source_interface, "pk", None) == resolved_interface.pk + port["relationship_source_resolvable"] = source_is_resolvable + + enrich_port_relationships(port, relationship_maps, interface_name_field, context.server_key) + for relation in ("lag", "parent"): + related_port_id = port.get(f"librenms_{relation}_port_id") + related_port = relationship_maps.ports_by_id.get(related_port_id) + related_interface = None + if related_port is not None and related_port_id in unique_host_port_ids: + related_name_hint = ( + (related_port.get(interface_name_field) or "") if related_port_id in unambiguous_name_port_ids else "" + ) + catalog_interface, catalog_error = resolve_interface_by_port_id( + context.obj, + str(related_port_id), + context.server_key, + name_hint=related_name_hint, + index=context.catalog_index, + ) + if catalog_error is None: + permitted_interface, permitted_error = resolve_interface_by_port_id( + context.obj, + str(related_port_id), + context.server_key, + name_hint=related_name_hint, + index=context.related_index, + ) + if ( + permitted_error is None + and catalog_interface is not None + and getattr(permitted_interface, "pk", None) == catalog_interface.pk + ): + related_interface = permitted_interface + if ( + relation == "lag" + and related_interface is not None + and getattr(related_interface, "type", None) != "lag" + and related_interface.pk not in context.changeable_interface_ids + ): + related_interface = None + port[f"{relation}_target_resolvable"] = related_interface is not None + return resolved_interface diff --git a/netbox_librenms_plugin/librenms_api.py b/netbox_librenms_plugin/librenms_api.py index dcfe152a5a..bdf7d7da2e 100644 --- a/netbox_librenms_plugin/librenms_api.py +++ b/netbox_librenms_plugin/librenms_api.py @@ -545,6 +545,398 @@ def get_ports(self, device_id, with_vlans=True): except requests.exceptions.RequestException as e: return False, f"Error connecting to LibreNMS: {str(e)}" + def get_port_stack(self, device_id: int): + """ + Fetch ifStackTable relationships from LibreNMS for a device. + + Returns port_stack pairs showing parent/child interface relationships + (LAG membership and sub-interface nesting). + + Args: + device_id: LibreNMS device ID + + Returns: + tuple: (success: bool, data: list[dict] | str) + On success: list of {high_port_id, low_port_id, high_ifIndex, low_ifIndex} dicts + On failure: error string + """ + try: + response = requests.get( + f"{self.librenms_url}/api/v0/devices/{device_id}/port_stack", + headers=self.headers, + timeout=DEFAULT_API_TIMEOUT, + verify=self.verify_ssl, + ) + response.raise_for_status() + data = response.json() + # A non-object top-level payload (list, string, null) is malformed — not a valid + # "no relationships" answer. Fail rather than silently returning (True, []), which + # would be indistinguishable from "no relationships" and skip valid sync updates. + if not isinstance(data, dict): + logger.warning("Unexpected port_stack response for device %s: %r", device_id, data) + return False, "Unexpected response format from LibreNMS (non-object payload)" + # Honor an explicit error status *before* consuming mappings: an error payload can + # still carry mappings (e.g. {"status": "error", "message": ..., "mappings": []}), + # and treating that as "no relationships" would mask a real API failure and silently + # skip valid LAG/sub-interface sync. A genuine answer has no status (or "ok"). + status = data.get("status") + if status is not None and (not isinstance(status, str) or status.lower() != "ok"): + # Only an absent status or a case-insensitive "ok" string is a genuine answer. + # A non-string status (e.g. {"status": false, "mappings": []}) is malformed and + # must fail the call, not be accepted as an empty "no relationships" result. + message = data.get("message") or "LibreNMS reported an error fetching port stack" + logger.warning("port_stack error status for device %s: %r", device_id, data) + return False, str(message) + mappings = data.get("mappings") + # The documented success envelope always contains a list-valued mappings field, + # including when no relationships exist. Missing, null, non-list, or mixed-list data + # is malformed. It must not become an authoritative empty relationship snapshot. + if not isinstance(mappings, list) or any(not isinstance(item, dict) for item in mappings): + logger.warning("Unexpected port_stack response for device %s: %r", device_id, data) + return False, "Unexpected response format from LibreNMS (invalid 'mappings' payload)" + return True, mappings + except requests.exceptions.HTTPError as e: + if e.response.status_code == 404: + return False, "Device not found in LibreNMS" + return False, f"HTTP error: {str(e)}" + except ValueError as e: + # response.json() raises ValueError (older requests) / JSONDecodeError on a non-JSON + # body. requests.exceptions.JSONDecodeError subclasses BOTH ValueError and + # RequestException, so this must precede the RequestException handler — otherwise the + # broad handler swallows JSON decode failures and reports "Error connecting" instead. + return False, f"Invalid JSON from LibreNMS: {str(e)}" + except requests.exceptions.RequestException as e: + return False, f"Error connecting to LibreNMS: {str(e)}" + + def resolve_port_relationships( + self, + ports: list, + port_stack: list, + lag_patterns: dict | None = None, + device_os: str | None = None, + interface_name_field: str = "ifName", + compiled_lag_patterns: list | None = None, + ) -> dict: + """ + Resolve LAG membership and sub-interface parent relationships from LibreNMS data. + + Universal rules (vendor-agnostic, hardcoded): + 1. The LAG aggregate is normally the 'low' entry in a port_stack pair, but the + aggregate side is determined authoritatively by _is_lag_aggregate() (ifType + ieee8023adLag or a configured name pattern), not by position — so a pair whose + aggregate is on the 'high' side is still mapped member->aggregate correctly. + 2. Skip any pair where either port name contains ':' (Nokia SAP entries). + 3. Strip '.N' suffix to resolve Junos sub-unit names to physical-level ports. + 4. Sub-interface detection: a pair where one name is the other plus a numeric '.N' + suffix is a parent/child pair — and, like the LAG rule, the relationship is + position-independent, so the child may be on either the high or the low side. + + Configurable via PortStackLagPattern model: + - Per-OS regex patterns identify LAG aggregates when ifType is not 'ieee8023adLag'. + + Args: + ports: Port dicts from get_ports(), each with port_id, ifName, ifType keys. + port_stack: Port stack dicts from get_port_stack(), each with + high_port_id and low_port_id keys. + lag_patterns: Optional dict of {librenms_os: pattern_str} overriding DB lookup. + Pass an empty dict to disable name-pattern matching entirely. + When None (default), patterns are fetched from PortStackLagPattern, + scoped to device_os when that is provided. + device_os: LibreNMS OS of the device being resolved. When set (and lag_patterns + is None), only that OS's pattern is loaded so a vendor-specific regex + can't misclassify an interface on another platform. When None, all + stored patterns are loaded (legacy, unscoped behaviour). + interface_name_field: Interface-name field selected for this device + ('ifName' or 'ifDescr'). Names are scanned across this + field first, then ifName and ifDescr as fallbacks, so an + ifDescr-mode device still resolves its LAG and sub-interface + relationships. + compiled_lag_patterns: Optional list of pre-compiled name-pattern regexes. When + provided, the PortStackLagPattern DB read and per-call + compile are skipped and this list is used directly, taking + priority over lag_patterns/device_os. + + Returns: + dict with keys (port_ids are canonical normalized positive ints, so every + consumer can look up by ``normalize_librenms_port_id(...)`` without re-deriving + str/int fallbacks): + 'lag_members': {member_port_id: aggregate_port_id} + 'sub_interfaces': {child_port_id: parent_port_id} + """ + # Guard against malformed payload items (non-dict) so a single bad entry from + # LibreNMS doesn't crash the whole relationship resolution with AttributeError. + safe_ports = [p for p in ports if isinstance(p, dict)] + + # Scan names across the selected interface_name_field plus ifName/ifDescr, mirroring + # the refresh relationship signal. On an ifDescr-mode device the aggregate or sub-unit name lives in + # ifDescr (ifName may be empty), so keying only on ifName here would drop every such port + # from the lookup maps and silently resolve no relationships even though the port_stack + # fetch was triggered. dict.fromkeys de-dups while preserving precedence order. + name_fields = tuple(dict.fromkeys((interface_name_field or "ifName", "ifName", "ifDescr"))) + + def _port_name_items(port: dict) -> list[tuple[str, str]]: + return [(field, name) for field in name_fields if isinstance(name := port.get(field), str) and name] + + def _port_names(port: dict) -> list[str]: + return [name for _, name in _port_name_items(port)] + + from netbox_librenms_plugin.utils import normalize_librenms_port_id + + # by_id indexes EVERY port with a usable port_id, even a nameless one: a port_stack pair + # references ports by id, and _is_lag_aggregate classifies an aggregate authoritatively by + # ifType (ieee8023adLag) which needs no name. Excluding nameless ports here would drop a + # LAG relationship the port_stack + ifType define, even though ifType alone is enough to + # resolve it. (All downstream NAME ops iterate _port_names(port), which is empty for a + # nameless port, so it simply won't match any name-based rule — no string op sees a None.) + by_id = {} + ambiguous_port_ids = set() + for port in safe_ports: + port_id = normalize_librenms_port_id(port.get("port_id")) + if port_id is None or port_id in ambiguous_port_ids: + continue + if port_id in by_id: + by_id.pop(port_id) + ambiguous_port_ids.add(port_id) + continue + by_id[port_id] = port + ports_with_id = list(by_id.values()) + # Index each name field separately. A sub-unit in ifName must resolve only through an + # ifName base. An unrelated ifDescr with the same text is not evidence that both rows + # describe one physical interface. Drop names that are ambiguous within one field. + by_name_by_field: dict[str, dict[str, dict]] = {field: {} for field in name_fields} + ambiguous_names_by_field: dict[str, set[str]] = {field: set() for field in name_fields} + for p in ports_with_id: + for field, name in _port_name_items(p): + field_index = by_name_by_field[field] + ambiguous_names = ambiguous_names_by_field[field] + if name in ambiguous_names: + continue + existing = field_index.get(name) + if existing is not None and normalize_librenms_port_id( + existing.get("port_id") + ) != normalize_librenms_port_id(p.get("port_id")): + del field_index[name] + ambiguous_names.add(name) + continue + field_index[name] = p + + if compiled_lag_patterns is not None: + # Caller (e.g. the interface-refresh gating) already loaded + compiled the OS-scoped + # patterns for this device_os and shares them here, so the DB read + regex compile + # happen once per refresh instead of once in the signal check and again here. + compiled_patterns = compiled_lag_patterns + elif lag_patterns is None: + # OS-scoped pattern loading + compile-with-skip lives on the model so the resolver + # and the refresh fetch trigger can't diverge on which patterns apply. A + # PRESENT but unusable device_os disables name-pattern matching (returns []), so a + # stale vendor regex can't be re-globalized; structural ieee8023adLag detection is + # unaffected. device_os None preserves the legacy unscoped behaviour. + from netbox_librenms_plugin.models import PortStackLagPattern + + compiled_patterns = PortStackLagPattern.compiled_patterns_for_os(device_os) + else: + import re as _re + + compiled_patterns = [] + for pattern_str in lag_patterns.values(): + try: + compiled_patterns.append(_re.compile(pattern_str)) + except (_re.error, TypeError) as exc: + # A configured pattern with a typo'd regex (re.error) or a non-string value + # (TypeError — the caller-supplied dict isn't guaranteed strings like the + # DB-backed path is) is skipped rather than crashing resolution — logged so the + # user can tell why LAG detection isn't working. + logger.warning("Skipping invalid LAG name pattern %r: %s", pattern_str, exc) + + lag_members: dict = {} + sub_interfaces: dict = {} + conflicted_lag_members: set = set() + conflicted_sub_interfaces: set = set() + + def _is_lag_aggregate(port: dict) -> bool: + if port.get("ifType") == "ieee8023adLag": + return True + return any(pat.search(name) for pat in compiled_patterns for name in _port_names(port)) + + def _relate(mapping: dict, conflicted_keys: set, key_port: dict, value_port: dict) -> None: + """ + Store a normalized ``port_id -> port_id`` edge in ``mapping``. + + The maps are keyed AND valued by the canonical normalized port_id (a positive int + via ``normalize_librenms_port_id``), so every consumer can look up by the same + normalizer without re-deriving str/int fallbacks per call. ports and port_stack are + independent LibreNMS payloads (one may carry string ids, the other ints); + normalizing both sides at the source makes the maps type-agnostic and + self-consistent. A side whose id won't normalize is dropped rather than stored raw. + """ + key_id = normalize_librenms_port_id(key_port.get("port_id")) + value_id = normalize_librenms_port_id(value_port.get("port_id")) + if key_id is None or value_id is None or key_id in conflicted_keys: + return + existing = mapping.get(key_id) + if existing is None: + mapping[key_id] = value_id + elif existing != value_id: + mapping.pop(key_id, None) + conflicted_keys.add(key_id) + + def _resolve_physical_port(port: dict): + """ + Resolve a port to its physical-level port, stripping a Junos sub-unit ``.N`` suffix. + + Scans ALL known names (not just the primary), mirroring the SAP and sub-unit guards: + with interface_name_field="ifDescr" the structured ``xe-0/0/0.0`` / ``ae1.0`` name can + live in ifName while the primary (ifDescr) carries an arbitrary label. A primary-only + check would then fail to collapse the logical pair, and LAG sync would bind the wrong + (logical) ports — e.g. ``202 -> 204`` instead of the physical ``201 -> 203``. + """ + for field, name in _port_name_items(port): + if "." not in name: + continue + base, suffix = name.rsplit(".", 1) + # Only treat a NUMERIC suffix as a sub-unit (e.g. Gi0/1.100, ae1.0). A dotted name + # whose suffix isn't a number is a legitimate physical name, not a sub-interface, + # so it must not be remapped to a spurious base. + if suffix.isdigit() and base in by_name_by_field[field]: + return by_name_by_field[field][base] + return port + + def _is_sub_unit_of(child_name: str, parent_name: str) -> bool: + """True when child_name is parent_name + '.' (a numeric sub-interface).""" + if not child_name.startswith(parent_name + "."): + return False + return child_name[len(parent_name) + 1 :].isdigit() + + def _has_sub_unit_relationship(child_port: dict, parent_port: dict) -> bool: + """ + Return True when child_port has a numeric sub-unit name relationship to parent_port. + + Scans ALL known names of each port (not just the primary), mirroring the SAP guard + below: with interface_name_field="ifDescr" the ``.N`` sub-unit marker can live in ifName + while the primary (ifDescr) carries a clean name, so a primary-only check would miss it. + """ + return any( + _is_sub_unit_of(child_name, parent_name) + for field in name_fields + if isinstance(child_name := child_port.get(field), str) + and isinstance(parent_name := parent_port.get(field), str) + and child_name + and parent_name + ) + + for entry in port_stack: + if not isinstance(entry, dict): + continue + # LibreNMS returns the ports_stack rows verbatim (api_success($device->portsStack)), + # so the keys are the table columns: high_port_id / low_port_id. The API docs show + # port_id_high / port_id_low, which no server actually sends — verified against a live + # instance, where an entry is {id, device_id, high_ifIndex, high_port_id, low_ifIndex, + # low_port_id, ifStackStatus}. Reading the documented spelling silently resolved every + # relationship to nothing. + if "high_port_id" not in entry and "low_port_id" not in entry: + # Neither key present: a shape change would otherwise zero out every LAG and + # sub-interface relationship without a single error. + logger.warning("Unrecognized port_stack entry shape, keys: %s", sorted(entry)) + continue + high_id = entry.get("high_port_id") + low_id = entry.get("low_port_id") + # 0 (int OR string) is the ifStack sentinel for "no port" (stack top/bottom). + # normalize_librenms_port_id treats 0/negative/non-numeric as invalid whether the API + # returned the id as an int or a string, so the sentinel skip stays consistent with the + # canonical integer keys in by_id. A bare ``not low_id`` would let a truthy string "0" + # fall through to the by_id lookup. + normalized_high_id = normalize_librenms_port_id(high_id) + normalized_low_id = normalize_librenms_port_id(low_id) + if normalized_high_id is None or normalized_low_id is None: + continue + + high_port = by_id.get(normalized_high_id) + low_port = by_id.get(normalized_low_id) + if not high_port or not low_port: + continue + + # Universal rule: skip Nokia SAP entries (colon notation: lag1:0, lag-1:10). Check + # ALL known names, not just the primary: when interface_name_field="ifDescr" a SAP port + # can carry a clean ifDescr but the real lag1:0 marker in ifName, so a primary-only + # check would miss it and misclassify the row as a LAG/sub-interface relationship. + if any(":" in name for name in _port_names(high_port)) or any( + ":" in name for name in _port_names(low_port) + ): + continue + + # Sub-interface detection: the child name is parent + '.'. port_stack + # ordering is NOT guaranteed (the LAG branch below is already position-independent, + # and ifStack can emit the sub-interface as either the high or the low side), so + # check BOTH directions. Without the reverse check a parent=low/child=high pair + # falls through to the LAG branch, where _resolve_physical_port collapses both ports to + # the same base and the self-reference guard silently drops the relationship. + low_is_child = _has_sub_unit_relationship(low_port, high_port) + high_is_child = _has_sub_unit_relationship(high_port, low_port) + if low_is_child and high_is_child: + continue + if low_is_child: + _relate( + sub_interfaces, + conflicted_sub_interfaces, + low_port, + high_port, + ) # low is the child, high the parent + continue + if high_is_child: + _relate( + sub_interfaces, + conflicted_sub_interfaces, + high_port, + low_port, + ) # high is the child, low the parent + continue + + # LAG membership: resolve each side to its physical-level port (strips the Junos + # sub-unit .N suffix), checking every known name so an ifDescr-mode device whose + # structured name lives in ifName still collapses to the physical port. + high_phys = _resolve_physical_port(high_port) + low_phys = _resolve_physical_port(low_port) + if not high_phys or not low_phys: + continue + + if normalize_librenms_port_id(high_phys.get("port_id")) == normalize_librenms_port_id( + low_phys.get("port_id") + ): + continue # Same port after physical resolution — skip self-references + + low_is_agg = _is_lag_aggregate(low_phys) + high_is_agg = _is_lag_aggregate(high_phys) + + if low_is_agg and high_is_agg: + # Both sides look like aggregates — a too-broad configured lag_name_pattern + # matched the member's name too (e.g. 'bond' matching both bond0 and + # bond0-slave). Disambiguate with the authoritative STRUCTURAL signal: the port + # whose ifType is ieee8023adLag is the real aggregate. Resolve only when exactly + # one side is structural; if neither (or both) is, we genuinely can't tell which + # is the aggregate, so skip rather than guess a wrong membership. + low_struct = low_phys.get("ifType") == "ieee8023adLag" + high_struct = high_phys.get("ifType") == "ieee8023adLag" + if low_struct and not high_struct: + _relate(lag_members, conflicted_lag_members, high_phys, low_phys) + elif high_struct and not low_struct: + _relate(lag_members, conflicted_lag_members, low_phys, high_phys) + elif low_is_agg: + _relate( + lag_members, + conflicted_lag_members, + high_phys, + low_phys, + ) # high is the member, low the aggregate + elif high_is_agg: + _relate( + lag_members, + conflicted_lag_members, + low_phys, + high_phys, + ) # low is the member, high the aggregate + + return {"lag_members": lag_members, "sub_interfaces": sub_interfaces} + def add_device(self, data): """ Add a device to LibreNMS. diff --git a/netbox_librenms_plugin/migrations/0013_portstacklagpattern.py b/netbox_librenms_plugin/migrations/0013_portstacklagpattern.py new file mode 100644 index 0000000000..81761457dc --- /dev/null +++ b/netbox_librenms_plugin/migrations/0013_portstacklagpattern.py @@ -0,0 +1,94 @@ +import netbox.models.deletion +import netbox_librenms_plugin.models +import taggit.managers +import utilities.json +from django.db import migrations, models + +INITIAL_LAG_PATTERNS = [ + ("ios", r"^Po\d+$"), + ("iosxe", r"^Po\d+$"), + ("iosxr", r"^Bundle-Ether\d+$"), + ("timos", r"^lag-\d+$"), + ("junos", r"^ae\d+$"), + ("arcos", r"^bond\d+$"), +] + + +def populate_patterns(apps, schema_editor): + db_alias = schema_editor.connection.alias + PortStackLagPattern = apps.get_model("netbox_librenms_plugin", "PortStackLagPattern") + for os_name, pattern in INITIAL_LAG_PATTERNS: + PortStackLagPattern.objects.using(db_alias).get_or_create( + librenms_os=os_name, + defaults={"lag_name_pattern": pattern}, + ) + + +def remove_patterns(apps, schema_editor): + db_alias = schema_editor.connection.alias + PortStackLagPattern = apps.get_model("netbox_librenms_plugin", "PortStackLagPattern") + for os_name, pattern in INITIAL_LAG_PATTERNS: + PortStackLagPattern.objects.using(db_alias).filter( + librenms_os=os_name, + lag_name_pattern=pattern, + ).delete() + + +class Migration(migrations.Migration): + dependencies = [ + # Pinned to the NetBox 4.2 floor declared by ``min_version`` in + # ``netbox_librenms_plugin/__init__.py`` (and the 4.2–4.5 range in README), matching + # sibling 0010. PortStackLagPattern is a plain NetBoxModel that only references + # ``extras.Tag``/``TaggedItem`` (via taggit) — all present in 4.2.x — so it needs + # nothing from the 4.3-era 0138. ``makemigrations`` will try to bump this to the dev + # environment's NetBox tip; revert it unless we actually start depending on a newer field. + ("extras", "0122_charfield_null_choices"), + ("netbox_librenms_plugin", "0012_normalize_device_serials"), + ] + + operations = [ + migrations.CreateModel( + name="PortStackLagPattern", + fields=[ + ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False)), + ("created", models.DateTimeField(auto_now_add=True, null=True)), + ("last_updated", models.DateTimeField(auto_now=True, null=True)), + ( + "custom_field_data", + models.JSONField(blank=True, default=dict, encoder=utilities.json.CustomFieldJSONEncoder), + ), + ( + "librenms_os", + models.CharField( + help_text="LibreNMS OS identifier (e.g. 'ios', 'timos', 'junos')", + max_length=50, + unique=True, + ), + ), + ( + "lag_name_pattern", + models.CharField( + help_text=( + "Regular expression matching LAG aggregate interface names. " + "Used as fallback when ifType is not 'ieee8023adLag'. " + r"Example: ^Po\d+$" + ), + max_length=200, + ), + ), + ("description", models.TextField(blank=True)), + ("tags", taggit.managers.TaggableManager(through="extras.TaggedItem", to="extras.Tag")), + ], + options={ + "verbose_name": "Port Stack LAG Pattern", + "verbose_name_plural": "Port Stack LAG Patterns", + "ordering": ["librenms_os"], + }, + bases=( + netbox_librenms_plugin.models.FullCleanOnSaveMixin, + netbox.models.deletion.DeleteMixin, + models.Model, + ), + ), + migrations.RunPython(populate_patterns, remove_patterns), + ] diff --git a/netbox_librenms_plugin/migrations/0014_portstacklagpattern_ci_unique.py b/netbox_librenms_plugin/migrations/0014_portstacklagpattern_ci_unique.py new file mode 100644 index 0000000000..9860b0041f --- /dev/null +++ b/netbox_librenms_plugin/migrations/0014_portstacklagpattern_ci_unique.py @@ -0,0 +1,98 @@ +from django.db import migrations, models +from django.db.models.functions import Lower + + +_PYTHON_STRIP_WHITESPACE = ( + "\t\n\v\f\r\x1c\x1d\x1e\x1f \x85\xa0" + "\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u2028\u2029\u202f\u205f\u3000" +) + + +def normalize_librenms_os_case(apps, schema_editor): + """Canonicalize librenms_os and fail clearly on real duplicates before the CI-unique. + + UniqueConstraint(Lower("librenms_os")) is validated against existing rows the instant it is + added, so a database the old case-sensitive unique let accumulate both "ios" and "IOS" would + fail this migration with an opaque IntegrityError at deploy time. Detect genuine + case-insensitive collisions first and abort with an actionable message (they need a human + merge — the migration can't know which pattern wins); then rewrite the surviving rows to the + same canonical form clean() already writes on every save, so a full_clean-bypassing insert + (bulk_create / raw SQL / loaddata) can't leave a noncanonical value behind the constraint. + + Normalize with ``.strip().lower()`` — exactly what ``clean()`` applies — NOT a bare ``Lower()``: + rows a bypassing path left as ``" IOS "`` and ``"ios"`` are the same pattern to ``clean()`` but + differ under ``Lower()`` alone, so a Lower()-only collision check would miss them and a + Lower()-only rewrite would leave the surrounding whitespace behind the new constraint. + + All reads and writes are pinned to the migration's database alias (like sibling migrations + 0013/0015): unrouted ``objects.all()`` would consult the default router, so + ``migrate --database=other`` would normalize rows on the wrong database and leave the target + database to fail the AddConstraint with an opaque IntegrityError. + """ + PortStackLagPattern = apps.get_model("netbox_librenms_plugin", "PortStackLagPattern") + db_alias = schema_editor.connection.alias + seen_pk_by_value = {} + collisions = set() + normalized_by_pk = {} + for pattern in PortStackLagPattern.objects.using(db_alias).all(): + normalized = (pattern.librenms_os or "").strip().lower() + if not normalized: + raise RuntimeError( + "Cannot add the case-insensitive PortStackLagPattern.librenms_os uniqueness: a row " + "has a blank librenms_os after normalization; fix it by hand first." + ) + if seen_pk_by_value.get(normalized, pattern.pk) != pattern.pk: + collisions.add(normalized) + seen_pk_by_value.setdefault(normalized, pattern.pk) + normalized_by_pk[pattern.pk] = normalized + if collisions: + raise RuntimeError( + "Cannot add the case-insensitive PortStackLagPattern.librenms_os uniqueness: these " + "values already have case-variant duplicates that must be merged by hand first: " + + ", ".join(sorted(collisions)) + ) + for pattern in PortStackLagPattern.objects.using(db_alias).all(): + normalized = normalized_by_pk[pattern.pk] + if pattern.librenms_os != normalized: + pattern.librenms_os = normalized + pattern.save(using=db_alias, update_fields=["librenms_os"]) + + +class Migration(migrations.Migration): + # Depend only on 0013_portstacklagpattern (which already pins extras to 0122 for the + # supported NetBox 4.2+ range). makemigrations tried to bump the extras dependency to a + # 4.3-era tip, but this migration adds nothing that needs a newer field, so keep it minimal + # — same rationale as the dependency note in 0013_portstacklagpattern. + dependencies = [ + ("netbox_librenms_plugin", "0013_portstacklagpattern"), + ] + + operations = [ + # Drop the case-sensitive column unique: at the DB level it let "ios" and "IOS" coexist + # even though compiled_patterns_for_os reads librenms_os case-insensitively + # (librenms_os__iexact), making the per-OS LAG-pattern fallback ambiguous. help_text is + # carried verbatim from the model so makemigrations sees no residual field drift. + migrations.AlterField( + model_name="portstacklagpattern", + name="librenms_os", + field=models.CharField( + help_text="LibreNMS OS identifier (e.g. 'ios', 'timos', 'junos')", + max_length=50, + ), + ), + # Pre-clean before the constraint is validated: canonicalize any mixed-case rows an old + # full_clean-bypassing path may have left and abort with a clear message if genuine + # case-variant duplicates exist, instead of letting AddConstraint fail with an opaque + # IntegrityError at deploy time. noop reverse (lowercasing isn't reversible). + migrations.RunPython(normalize_librenms_os_case, migrations.RunPython.noop), + # Use BTRIM with Python's full whitespace set. PostgreSQL's default removes only spaces, + # while clean() uses str.strip(). The database must enforce the same canonical key for + # paths that bypass full_clean (bulk_create, raw SQL, or loaddata). + migrations.AddConstraint( + model_name="portstacklagpattern", + constraint=models.UniqueConstraint( + Lower(models.Func("librenms_os", models.Value(_PYTHON_STRIP_WHITESPACE), function="BTRIM")), + name="unique_portstacklagpattern_librenms_os_ci", + ), + ), + ] diff --git a/netbox_librenms_plugin/models.py b/netbox_librenms_plugin/models.py index e8aa8dec77..4a0623dd6a 100644 --- a/netbox_librenms_plugin/models.py +++ b/netbox_librenms_plugin/models.py @@ -7,6 +7,7 @@ from dcim.models import DeviceType, Manufacturer, ModuleType, Platform from django.core.exceptions import ValidationError from django.db import models +from django.db.models.functions import Lower from django.urls import reverse from netbox.models import NetBoxModel @@ -14,6 +15,19 @@ logger = logging.getLogger(__name__) +# Characters removed by Python's no-argument str.strip(). PostgreSQL BTRIM removes only ASCII +# spaces when its second argument is absent, so pass this set explicitly wherever the database +# must enforce the same normalization as PortStackLagPattern.clean(). +_PYTHON_STRIP_WHITESPACE = ( + "\t\n\v\f\r\x1c\x1d\x1e\x1f \x85\xa0" + "\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u2028\u2029\u202f\u205f\u3000" +) + + +def _trim_python_whitespace(expression): + """Return a PostgreSQL expression with Python ``str.strip()`` semantics.""" + return models.Func(expression, models.Value(_PYTHON_STRIP_WHITESPACE), function="BTRIM") + def _validate_replacement_template(compiled: re.Pattern, replacement: str) -> None: """Verify that *replacement* is a valid back-reference template for *compiled*. @@ -916,3 +930,129 @@ def to_yaml(self): "description": self.description, } return yaml.dump(data, sort_keys=False) + + +class PortStackLagPattern(FullCleanOnSaveMixin, NetBoxModel): + """ + Maps a LibreNMS OS name to the regex identifying LAG aggregate interfaces. + + Used as fallback when a port's ifType is not 'ieee8023adLag'. + Example: Cisco IOS port-channels have ifType='propVirtual' and need name-based + identification via pattern '^Po\\d+$'. + + Universal rules (hardcoded, vendor-agnostic): + - LAG aggregate is normally the 'low' entry in a port_stack pair, but the + aggregate side is determined authoritatively by ifType or this pattern, so a + pair whose aggregate is on the 'high' side is still mapped member->aggregate. + - Pairs where either name contains ':' are skipped (Nokia SAP entries). + - .N suffix is stripped for name resolution (handles Junos sub-unit pairing). + """ + + librenms_os = models.CharField( + max_length=50, + help_text="LibreNMS OS identifier (e.g. 'ios', 'timos', 'junos')", + ) + lag_name_pattern = models.CharField( + max_length=200, + help_text=( + "Regular expression matching LAG aggregate interface names. " + "Used as fallback when ifType is not 'ieee8023adLag'. " + r"Example: ^Po\d+$" + ), + ) + description = models.TextField(blank=True) + + @functools.cached_property + def _compiled_pattern(self): + """Compiled lag_name_pattern regex, or None when it doesn't compile (skipped, not fatal).""" + try: + return re.compile(self.lag_name_pattern) + except re.error: + return None + + @classmethod + def compiled_patterns_for_os(cls, device_os): + """ + Return the compiled lag_name_pattern regexes scoped to *device_os*. + + Single home for the OS-scoping + compile-with-skip used by both the relationship + resolver and the lazy port_stack-fetch trigger, so the two can't + disagree on which patterns apply. ``device_os=None`` loads every stored pattern (legacy + unscoped behaviour); a present-but-blank/non-string OS returns none (an unknown OS must + not re-globalize every vendor's regex); otherwise the patterns whose ``librenms_os`` + matches after the same trim/lower normalization as the database constraint. Patterns + that fail to compile are skipped and logged. + """ + if device_os is None: + queryset = cls.objects.all() + else: + os_filter = device_os.strip() if isinstance(device_os, str) else "" + if not os_filter: + return [] + queryset = cls.objects.annotate( + normalized_librenms_os=Lower(_trim_python_whitespace("librenms_os")) + ).filter(normalized_librenms_os=os_filter.lower()) + compiled = [] + for pattern in queryset: + regex = pattern._compiled_pattern + if regex is None: + logger.warning( + "Skipping invalid LAG name pattern for OS %r: %r", + pattern.librenms_os, + pattern.lag_name_pattern, + ) + continue + compiled.append(regex) + return compiled + + def clean(self): + """Validate OS name is non-blank and lag_name_pattern is a valid regex.""" + super().clean() + # Invalidate the cached compiled pattern so it recompiles from the edited value. + self.__dict__.pop("_compiled_pattern", None) + os_name = (self.librenms_os or "").strip().lower() + if not os_name: + raise ValidationError({"librenms_os": "OS name must not be blank."}) + self.librenms_os = os_name + lag_pattern = (self.lag_name_pattern or "").strip() + if not lag_pattern: + raise ValidationError({"lag_name_pattern": "Pattern must not be blank."}) + self.lag_name_pattern = lag_pattern + # ReDoS note (low severity, accepted): see validate_regex_field, which documents + # the admin-supplied-pattern risk and centralizes the compile/validation path. + validate_regex_field(self.lag_name_pattern, "lag_name_pattern") + + def get_absolute_url(self): + """Return URL for this pattern's detail page.""" + return reverse("plugins:netbox_librenms_plugin:portstacklagpattern_detail", args=[self.pk]) + + def to_yaml(self): + data = { + "librenms_os": self.librenms_os, + "lag_name_pattern": self.lag_name_pattern, + "description": self.description, + } + return yaml.dump(data, sort_keys=False) + + class Meta: + """Meta options for PortStackLagPattern.""" + + ordering = ["librenms_os"] + verbose_name = "Port Stack LAG Pattern" + verbose_name_plural = "Port Stack LAG Patterns" + constraints = [ + # Enforce the same trimmed, case-insensitive key that the OS-scoped pattern lookup + # reads. A plain unique=True is case-sensitive, so at the DB level "ios" and "IOS" + # could coexist and both apply to one device, making the per-OS fallback ambiguous. + # clean() already normalizes with .strip().lower() on every save. BTRIM with Python's + # full whitespace set makes the database enforce the same canonical form for paths + # that skip full_clean, such as bulk_create, raw SQL, or loaddata. Lower() alone would + # let " ios " coexist with "ios". + models.UniqueConstraint( + Lower(_trim_python_whitespace("librenms_os")), + name="unique_portstacklagpattern_librenms_os_ci", + ), + ] + + def __str__(self): + return f"{self.librenms_os} -> {self.lag_name_pattern}" diff --git a/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_sync.js b/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_sync.js index 78a7685888..15283635bb 100644 --- a/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_sync.js +++ b/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_sync.js @@ -307,7 +307,7 @@ function initializeTableCheckboxes(tableId) { // toggle/shift handlers from re-binding on a SURVIVING toggle; a NodeList captured once // would then go stale, so select-all / shift-range would iterate detached checkboxes and miss // the rows a later row-level swap injected. - const liveCheckboxes = () => Array.from(table.querySelectorAll('td input[name="select"]')); + const liveCheckboxes = () => Array.from(table.querySelectorAll('td input[name="select"]:not(:disabled)')); const toggleAll = table.querySelector('th input.toggle'); // Persist the shift-range anchor on the TABLE element, not in a per-call closure. This // initializer re-runs on every htmx:afterSwap: checkboxes bound in an earlier run keep their @@ -327,6 +327,14 @@ function initializeTableCheckboxes(tableId) { toggleAll.addEventListener('change', function () { liveCheckboxes().forEach(checkbox => { checkbox.checked = toggleAll.checked; + // Explicitly (de)selecting every box: clear any data-auto-selected marker a child + // set on a parent before this loop reached it, so unchecking the last child can't + // later auto-deselect a parent the user included via select-all. + delete checkbox.dataset.autoSelected; + // Fire a bubbling change so the auto-select handler (cross-page parent / + // LAG member inclusion) runs for select-all too, not just single clicks. + // That handler is idempotent, so a double fire is harmless. + checkbox.dispatchEvent(new Event('change', { bubbles: true })); }); }); } @@ -350,6 +358,12 @@ function initializeTableCheckboxes(tableId) { if (start !== -1 && end !== -1) { current.slice(Math.min(start, end), Math.max(start, end) + 1).forEach(cb => { cb.checked = anchor.checked; + // Explicit range selection: clear a stale data-auto-selected marker so a later + // last-child uncheck can't auto-deselect a parent the user shift-selected. + delete cb.dataset.autoSelected; + // Fire change so shift-range selection runs the same auto-select logic + // (cross-page parent / LAG member inclusion) as single clicks / select-all. + cb.dispatchEvent(new Event('change', { bubbles: true })); }); } } @@ -373,6 +387,209 @@ function initializeCheckboxes() { initializeTableCheckboxes('librenms-module-table'); } +/** + * Auto-select LAG member rows when a LAG row checkbox is toggled. + * Reads data-port-id on the LAG row and checks all rows with + * data-member-of-lag matching that port_id. + * + * Also auto-selects the parent interface row when a sub-interface checkbox + * is toggled. Reads data-parent-port-id on the sub-interface row and finds + * the parent row by tr[data-port-id]. When the parent is on a different page, + * the server expands the cached relationship map and a brief notice is shown. + * + * Both behaviours are controlled by the #autoSelectLagMembers toggle. + */ +document.addEventListener('change', function (e) { + const checkbox = e.target; + if (!checkbox.matches('input[name="select"]') || checkbox.disabled) return; + + const toggle = document.getElementById('autoSelectLagMembers'); + const autoSelectEnabled = Boolean(toggle && toggle.checked); + + const row = checkbox.closest('tr'); + if (!row) return; + + let changed = false; + + // A checkbox that is now unchecked is no longer an auto-kept selection; drop the marker so a + // later manual re-check is treated as intentional (and not auto-undone with its children). + if (!checkbox.checked && checkbox.dataset.autoSelected) { + delete checkbox.dataset.autoSelected; + } + + // --- LAG: check/uncheck all members --- + // Each member the aggregate pulls in is marked data-auto-selected, and the uncheck side + // retracts ONLY marked members — so a member the user ticked manually before touching the + // aggregate survives toggling it off. Still skipped for synthetic parent-UNWIND events (see + // the uncheck path below): that event already retracted an auto-selected parent directly, so + // cascading it through here would only redundantly re-toggle the aggregate's members. + const portId = row.dataset.portId; + if (autoSelectEnabled && portId && !(e.detail && e.detail.lnmsParentUnwind)) { + const memberRows = document.querySelectorAll('tr[data-member-of-lag="' + CSS.escape(portId) + '"]'); + memberRows.forEach(function (memberRow) { + const memberCheckbox = memberRow.querySelector('input[name="select"]'); + if (!memberCheckbox || memberCheckbox.disabled) return; + if (checkbox.checked) { + // Aggregate checked: pull in each member not already selected, marking it + // auto-selected so unchecking the aggregate can retract ONLY what it added. A + // member already checked (manually or otherwise) keeps its state and marker. + if (!memberCheckbox.checked) { + memberCheckbox.checked = true; + memberCheckbox.dataset.autoSelected = 'true'; + // Run the same handler for the member so its own second-order rules apply + // (a member that is also a sub-interface child injects/removes its parent). + memberCheckbox.dispatchEvent(new Event('change', { bubbles: true })); + changed = true; + } + } else if (memberCheckbox.checked && memberCheckbox.dataset.autoSelected) { + // Aggregate unchecked: retract ONLY members it auto-selected — a member the user + // ticked themselves (no marker) is preserved. + delete memberCheckbox.dataset.autoSelected; + memberCheckbox.checked = false; + memberCheckbox.dispatchEvent(new Event('change', { bubbles: true })); + changed = true; + } + }); + } + + // --- Sub-interface: select parent when checking --- + const parentPortId = row.dataset.parentPortId; + if (autoSelectEnabled && parentPortId && checkbox.checked) { + const parentRow = document.querySelector('tr[data-port-id="' + CSS.escape(parentPortId) + '"]'); + if (parentRow) { + // Parent is on the same page - check it directly + const parentCheckbox = parentRow.querySelector('input[name="select"]'); + if (parentCheckbox && !parentCheckbox.disabled && !parentCheckbox.checked) { + parentCheckbox.checked = true; + // Mark as auto-selected so the uncheck path below can undo it when the last child + // is cleared — without clobbering a parent the user checked themselves. + parentCheckbox.dataset.autoSelected = 'true'; + // Propagate up a nested parent chain. + parentCheckbox.dispatchEvent(new Event('change', { bubbles: true })); + changed = true; + } + } else { + _showParentCrossPageNotice(row.dataset.parentName || parentPortId); + } + } + + // --- Sub-interface: undo parent auto-selection when the last child is unchecked --- + // Only act once NO other still-checked child on this page references the same parent — + // otherwise unchecking one sibling would drop the parent the remaining siblings still need. + if (parentPortId && !checkbox.checked) { + const siblingStillChecked = Array.prototype.some.call( + document.querySelectorAll( + 'tr[data-parent-port-id="' + CSS.escape(parentPortId) + '"] input[name="select"]:not(:disabled)' + ), + function (cb) { return cb !== checkbox && cb.checked; } + ); + if (!siblingStillChecked) { + // Same-page parent: uncheck it ONLY if we auto-selected it (data-auto-selected) — a + // parent the user checked themselves carries no marker and is preserved. Gated on the + // toggle like the auto-SELECT above: with #autoSelectLagMembers off the user has taken + // manual control, so a leftover marker from when the toggle was on must not let this + // uncheck a parent they are deliberately keeping (only the hidden-input cleanup above + // stays always-run). Dispatch so a nested grandparent chain unwinds too — as a + // CustomEvent flagged lnmsParentUnwind so the LAG member propagation ignores it (it + // would otherwise uncheck manually-selected member rows of an aggregate parent). + if (autoSelectEnabled) { + const parentRow = document.querySelector('tr[data-port-id="' + CSS.escape(parentPortId) + '"]'); + if (parentRow) { + const parentCheckbox = parentRow.querySelector('input[name="select"]'); + if ( + parentCheckbox && + !parentCheckbox.disabled && + parentCheckbox.checked && + parentCheckbox.dataset.autoSelected + ) { + delete parentCheckbox.dataset.autoSelected; + parentCheckbox.checked = false; + parentCheckbox.dispatchEvent( + new CustomEvent('change', { bubbles: true, detail: { lnmsParentUnwind: true } }) + ); + changed = true; + } + } + } + } + } + + if (changed) { + updateBulkActionButton(); + } +}); + +// Keep cross-page parent notices symmetric with #autoSelectLagMembers. Turning it back on replays +// checked child rows because their own change handlers do not otherwise run again. +document.addEventListener('change', function (e) { + const toggle = e.target; + if (!toggle.matches('#autoSelectLagMembers')) return; + + if (toggle.checked) { + document.querySelectorAll('input[name="select"]:checked:not(:disabled)').forEach(function (checkbox) { + checkbox.dispatchEvent(new Event('change', { bubbles: true })); + }); + return; + } + + const noticeContainer = document.getElementById('parent-cross-page-notices'); + if (noticeContainer) { + noticeContainer.remove(); + } +}); + +/** + * Show a brief inline notice when a sub-interface's parent is auto-included + * from a different page (cross-page parent selection). + * The notice auto-dismisses after 5 seconds. + * @param {string} parentName - Name of the parent interface + */ +function _showParentCrossPageNotice(parentName) { + const containerId = 'parent-cross-page-notices'; + let container = document.getElementById(containerId); + if (!container) { + // Insert before the table (find a stable anchor inside the form) + const table = document.getElementById('librenms-interface-table') || + document.getElementById('librenms-interface-table-vm'); + if (!table) return; + container = document.createElement('div'); + container.id = containerId; + table.parentNode.insertBefore(container, table); + } + + // Avoid duplicate notices for the same parent + if (container.querySelector('[data-parent="' + CSS.escape(parentName) + '"]')) return; + + const notice = document.createElement('div'); + notice.className = 'alert alert-info alert-dismissible py-1 px-2 small mb-1'; + notice.dataset.parent = parentName; + + // Build the notice via DOM nodes rather than innerHTML: parentName is interface + // data and must never be interpreted as HTML (DOM-XSS). textContent escapes it. + const icon = document.createElement('i'); + icon.className = 'mdi mdi-information-outline me-1'; + notice.appendChild(icon); + notice.appendChild(document.createTextNode('Parent interface ')); + const strong = document.createElement('strong'); + strong.textContent = parentName; + notice.appendChild(strong); + notice.appendChild( + document.createTextNode(' is on another page and will be included in the sync automatically.') + ); + const closeBtn = document.createElement('button'); + closeBtn.type = 'button'; + closeBtn.className = 'btn-close btn-sm'; + closeBtn.setAttribute('data-bs-dismiss', 'alert'); + closeBtn.setAttribute('aria-label', 'Close'); + notice.appendChild(closeBtn); + + container.appendChild(notice); + + setTimeout(function () { + if (notice.parentNode) notice.parentNode.removeChild(notice); + }, 5000); +} + // ============================================ // VIRTUAL CHASSIS & VRF HANDLING // ============================================ @@ -393,6 +610,15 @@ function initializeVCMemberSelect() { interfaceSelects.forEach(select => { if (select.tomselect && !select.dataset.interfaceSelectInitialized) { select.dataset.interfaceSelectInitialized = 'true'; + // Seed the rollback baseline HERE, before the change listener is attached: + // at init time select.value still reflects the originally rendered (verified) + // assignment. Seeding it lazily inside handleInterfaceChange instead would run + // after select.value already equals the newly-selected member, so a verify + // failure would "roll back" to the rejected member. + if (typeof select._lastVerifiedMember === 'undefined') { + const selectedOption = select.querySelector('option[selected]'); + select._lastVerifiedMember = selectedOption ? selectedOption.value : select.value; + } select.tomselect.on('change', function (value) { handleInterfaceChange(select, value); }); @@ -491,7 +717,7 @@ function initializeVlanGroupSelects() { */ function openVlanDetailModal(btn) { const interfaceName = btn.dataset.interface; - const safeName = btn.dataset.safeName; + const rowKey = btn.dataset.rowKey; const deviceId = btn.dataset.deviceId; const vlans = JSON.parse(btn.dataset.vlans); const vlanGroups = JSON.parse(btn.dataset.vlanGroups); @@ -502,7 +728,7 @@ function openVlanDetailModal(btn) { // Store current interface context on modal for save handler const modal = document.getElementById('vlanDetailModal'); modal.dataset.currentInterface = interfaceName; - modal.dataset.currentSafeName = safeName; + modal.dataset.currentRowKey = rowKey; modal.dataset.currentDeviceId = deviceId; // Clear any stale error from a previous save attempt @@ -540,7 +766,7 @@ function openVlanDetailModal(btn) { select.className = 'form-select form-select-sm vlan-modal-group-select'; select.dataset.vid = vlan.vid; select.dataset.interface = interfaceName; - select.dataset.safeName = safeName; + select.dataset.rowKey = rowKey; vlanGroups.forEach(group => { const option = document.createElement('option'); @@ -554,7 +780,7 @@ function openVlanDetailModal(btn) { // On change, update the hidden input for this VLAN immediately select.addEventListener('change', function () { - updateHiddenVlanGroupInput(safeName, vlan.vid, this.value); + updateHiddenVlanGroupInput(rowKey, vlan.vid, this.value); // Re-verify VLAN colors after group change verifyVlanInGroup(this, deviceId, vlan.vid, vlan.type, this.value); @@ -579,13 +805,13 @@ function openVlanDetailModal(btn) { /** * Update the hidden input for a specific VLAN group assignment. * - * @param {string} safeName - Safe interface name (slashes replaced) + * @param {string} rowKey - Stable LibreNMS port ID * @param {number} vid - VLAN ID * @param {string} groupId - Selected group ID */ -function updateHiddenVlanGroupInput(safeName, vid, groupId) { +function updateHiddenVlanGroupInput(rowKey, vid, groupId) { const input = document.querySelector( - `input.vlan-group-hidden[name="vlan_group_${safeName}_${vid}"]` + `input.vlan-group-hidden[name="vlan_group_${rowKey}_${vid}"]` ); if (input) { input.value = groupId; @@ -620,10 +846,10 @@ function verifyVlanInGroup(select, deviceId, vid, vlanType, groupId) { const saveBtn = document.getElementById('saveVlanGroups'); _vlanVerifyStart(saveBtn); - // Capture safeName before the async fetch to avoid stale closure if the modal + // Capture rowKey before the async fetch to avoid stale closure if the modal // is opened for a different interface while this request is in flight. const modal = document.getElementById('vlanDetailModal'); - const capturedSafeName = modal?.dataset.currentSafeName; + const capturedRowKey = modal?.dataset.currentRowKey; const csrfToken = getCsrfToken(); if (!csrfToken) { @@ -679,8 +905,8 @@ function verifyVlanInGroup(select, deviceId, vid, vlanType, groupId) { } // Update the css in the source edit button's data-vlans - if (capturedSafeName) { - const btn = document.querySelector(`.vlan-edit-btn[data-safe-name="${capturedSafeName}"]`); + if (capturedRowKey) { + const btn = document.querySelector(`.vlan-edit-btn[data-row-key="${capturedRowKey}"]`); if (btn) { try { const btnVlans = JSON.parse(btn.dataset.vlans); @@ -716,7 +942,7 @@ function initializeVlanModalSave() { saveBtn.addEventListener('click', function () { const applyToAll = document.getElementById('applyVlanGroupToAll')?.checked; const modalEl = document.getElementById('vlanDetailModal'); - const currentSafeName = modalEl.dataset.currentSafeName; + const currentRowKey = modalEl.dataset.currentRowKey; // Collect all group selections and resolved CSS from the modal const modalSelects = document.querySelectorAll('#vlanDetailTableBody .vlan-modal-group-select'); @@ -736,7 +962,7 @@ function initializeVlanModalSave() { // Determine which buttons to update const buttonsToUpdate = applyToAll ? document.querySelectorAll('.vlan-edit-btn') - : document.querySelectorAll(`.vlan-edit-btn[data-safe-name="${currentSafeName}"]`); + : document.querySelectorAll(`.vlan-edit-btn[data-row-key="${currentRowKey}"]`); // Apply DOM mutations (btn.dataset.vlans, hidden inputs, summary spans) // Called only after a successful server response when persisting, or immediately otherwise. @@ -745,12 +971,16 @@ function initializeVlanModalSave() { try { const btnVlans = JSON.parse(btn.dataset.vlans); const groups = JSON.parse(btn.dataset.vlanGroups); - const btnSafeName = btn.dataset.safeName; + const btnRowKey = btn.dataset.rowKey; let changed = false; btnVlans.forEach(v => { if (vidGroupMap.hasOwnProperty(String(v.vid))) { const newGroupId = vidGroupMap[String(v.vid)]; + const matchedGroup = groups.find(g => String(g.id) === String(newGroupId)); + // A VC member can expose a different scoped group for the same VID. + // Do not copy a source row's scoped group into a row that cannot select it. + if (newGroupId && !matchedGroup) return; v.group_id = newGroupId; // Apply resolved missing/css state BEFORE computing group_name @@ -763,7 +993,6 @@ function initializeVlanModalSave() { if (v.missing) { v.group_name = 'Not in NetBox'; } else { - const matchedGroup = groups.find(g => String(g.id) === String(newGroupId)); v.group_name = matchedGroup ? matchedGroup.name : '-- No Group (Global) --'; } @@ -771,7 +1000,7 @@ function initializeVlanModalSave() { // Update the hidden input for this VID on this interface const input = document.querySelector( - `input.vlan-group-hidden[name="vlan_group_${btnSafeName}_${v.vid}"]` + `input.vlan-group-hidden[name="vlan_group_${btnRowKey}_${v.vid}"]` ); if (input) { input.value = newGroupId; @@ -1018,16 +1247,97 @@ function handleVRFChange(select, value) { function handleInterfaceChange(select, value) { const csrfToken = getCsrfToken(); if (!csrfToken) return; // missing token → abort rather than throw on `.value` + // Abort any still-in-flight verification for this select: on rapid VC-member changes an + // older /verify-interface/ response can otherwise arrive after a newer one and repaint the + // row with stale cells/relationship controls. Mirrors handleModuleChange's AbortController. + if (select._interfaceVerifyController) { + select._interfaceVerifyController.abort(); + } + const controller = new AbortController(); + select._interfaceVerifyController = controller; + + // Resolve the row from the changed ', - safe_name, + row_key, vid, group_id, interface_name, @@ -263,11 +316,11 @@ def render_vlans(self, value, record): ) vlan_json = json_module.dumps(vlan_json_items) - device_id = self.device.pk if self.device else "" + device_id = record.get("selected_object_id") or (self.device.pk if self.device else "") # Build vlan_groups JSON for modal dropdowns group_options = [{"id": "", "name": "-- No Group (Global) --", "scope": ""}] - for group in self.vlan_groups: + for group in record.get("vlan_groups", self.vlan_groups): scope_info = str(group.scope) if hasattr(group, "scope") and group.scope else "" group_options.append({"id": str(group.pk), "name": group.name, "scope": scope_info}) @@ -280,14 +333,14 @@ def render_vlans(self, value, record): edit_btn = format_html( '', interface_name, - safe_name, + row_key, device_id, escaped_vlan_json, escaped_groups_json, @@ -374,10 +427,20 @@ def render_mtu(self, value, record): return self._render_field(value, record, "ifMtu", "mtu") def render_librenms_id(self, value, record): - """Render the 'librenms_id' field with appropriate styling based on comparison with NetBox.""" + """ + Render the LibreNMS port_id, coloured by how it compares to NetBox. + + Red when the interface doesn't exist in NetBox or carries no librenms_id custom + field, orange when the stored id differs from this LibreNMS port_id, green when + they match. + + Args: + value: The LibreNMS port_id to render. + record (dict): The table row, read for NetBox interface/existence state. - # Same XSS guard as _render_field: value/netbox_librenms_id originate outside NetBox, so - # use format_html to auto-escape both the body and the title attribute (issue #105). + Returns: + SafeString: The coloured ```` markup for the port_id. + """ if not record.get("exists_in_netbox"): return format_html('{}', value) @@ -386,21 +449,257 @@ def render_librenms_id(self, value, record): return format_html('{}', value) netbox_librenms_id = get_librenms_device_id(netbox_interface, self.server_key, auto_save=False) - if netbox_librenms_id is None: return format_html( '{}', value ) - - # Compare the IDs if str(value) != str(netbox_librenms_id): - # IDs do not match return format_html( '{}', netbox_librenms_id, value ) + return format_html('{}', value) + + def render_parent(self, value, record): + """ + Render the combined Parent / LAG relationship column. + + Shows LAG membership (if any) and parent interface (if any) stacked vertically, + each rendered as a single compact badge combining the relationship type, LibreNMS + name, and status icon (see ``_render_relationship_column``). The sync buttons keep + their existing CSS classes (lag-sync-btn / parent-sync-btn) so the JS handler still + works without changes. + + Args: + value: The cell value (unused; the row drives rendering). + record (dict): The table row, read for LAG/parent sync status and names. + + Returns: + SafeString: The stacked relationship markup, or empty when neither LAG nor + parent applies. + """ + parts = [] + + lag_status = record.get("lag_sync_status") + # LAG membership is device-only — VMInterface has no `lag` field and SyncInterfaceLagView + # 404s virtualmachine, so never render a LAG line/button on a VM table (it could only + # error). Parent/sub-interface sync is still supported for VMs and rendered below. + if lag_status is not None and self.sync_object_type != "virtualmachine": + parts.append( + self._render_relationship_column( + type_label="LAG", + lnms_name=record.get("librenms_lag_name"), + lnms_port_id=record.get("librenms_lag_port_id"), + sync_status=lag_status, + record=record, + btn_class="lag-sync-btn", + data_related_key="data-lag-port-id", + target_resolvable=record.get("lag_target_resolvable", True), + ) + ) + + parent_status = record.get("parent_sync_status") + if parent_status is not None: + parts.append( + self._render_relationship_column( + type_label="Parent", + lnms_name=record.get("librenms_parent_name"), + lnms_port_id=record.get("librenms_parent_port_id"), + sync_status=parent_status, + record=record, + btn_class="parent-sync-btn", + data_related_key="data-parent-port-id", + target_resolvable=record.get("parent_target_resolvable", True), + ) + ) + + if not parts: + return mark_safe("") + + return mark_safe("".join(str(p) for p in parts)) + + @cached_property + def _vc_members(self): + """ + Prefetch the chassis member Devices once per table render. + + Both :meth:`_vc_members_by_position` (per-row owner resolution) and + :meth:`VCInterfaceTable.render_device_selection` (the per-row member dropdown) need the + member list; resolving it here keeps ``members.all()`` to a single query per render + instead of one per row (an N+1 on a large chassis table). + """ + device = self.device + if device is None or not getattr(device, "virtual_chassis", None): + return [] + try: + members = list(device.virtual_chassis.members.all()) + allowed_ids = getattr(self, "allowed_vc_member_ids", None) + return members if allowed_ids is None else [member for member in members if member.pk in allowed_ids] + except (TypeError, AttributeError): + # A non-iterable or attribute-less stand-in device in a unit test. + return [] + + @cached_property + def _vc_members_by_position(self): + """ + Prefetch ``{vc_position: member Device}`` once per table render. + + :meth:`_resolve_row_member_id` is hit per row from BOTH the relationship sync button and + the VC member dropdown, and its name-based fallback otherwise issues a + ``members.get(vc_position=...)`` query per unresolved row — quadratic query load on a + large chassis table. Resolving from this map keeps it O(1) per row (one prefetch total). + """ + return {member.vc_position: member for member in self._vc_members if member.vc_position is not None} + + def _resolve_row_member_id(self, record): + """ + Resolve the id of the device/VM that owns this row's interface. + + The relationship sync button (``data-object-id``) and the VC member dropdown + (:meth:`VCInterfaceTable.render_device_selection`) must agree on the owner: the JS posts + the dropdown's value as the object id, so if the button resolved a different device the + sync POSTs to the wrong member and 404s (a non-ethernet sub-interface owned by another + member is the classic case). Both call this. Preference, most to least authoritative: + (1) the matched NetBox interface's device, (2) the row-selected object stamped during + enrichment or the cross-page verify path, (3) the shared guarded name heuristic for an + unbound physical row, (4) the viewed device. + """ + nb_iface = record.get("netbox_interface") + if nb_iface is not None and getattr(nb_iface, "device_id", None): + return nb_iface.device_id + row_object_id = record.get("selected_object_id") + if row_object_id: + return row_object_id + if self.device is not None and getattr(self.device, "virtual_chassis", None): + return resolve_interface_row_device( + self.device, + record, + self.interface_name_field, + members_by_position=self._vc_members_by_position or None, + ).pk + return self.device.pk if self.device else "" + + def _render_relationship_column( + self, + lnms_name, + lnms_port_id, + sync_status, + record, + btn_class, + data_related_key, + type_label="", + target_resolvable=True, + ): + """ + Render one compact pill for a LAG or Parent relationship line. + + Renders a Tabler light (``-lt``) badge holding a status icon + the relationship + ``type_label`` + the LibreNMS name, with the full status text in the badge + ``title``. Status is conveyed by colour + icon rather than a long inline word + (e.g. "Not in LibreNMS"), so the column stays glanceable and doesn't clump/wrap + to several lines on narrow screens. The ``-lt`` variants ship their own + readable text colour in both light and dark themes (and are exempt from the + bare-``bg-*`` badge guard). + + Args: + lnms_name: The LibreNMS-side relationship name to display. + lnms_port_id: The LibreNMS port_id of the related interface (drives the + sync button). + sync_status: The relationship sync status (match/mismatch/missing_nb/ + missing_lnms), or None to render nothing. + record (dict): The table row, read for port/interface context. + btn_class (str): The sync-button CSS class (lag-sync-btn / parent-sync-btn). + data_related_key (str): The data attribute carrying the related port_id. + type_label (str): The short relationship label ("LAG" / "Parent"). + + Returns: + SafeString: The pill markup (plus a sync button when applicable). + """ + # Colour + icon read at a glance; the text is the tooltip. Map hoisted to the module-level + # _RELATIONSHIP_STATUS_MAP so it isn't rebuilt on every call. + color, icon, status_text = _RELATIONSHIP_STATUS_MAP.get( + sync_status, ("secondary", "mdi-help-circle", sync_status) + ) + badge_css = f"bg-{color}-lt" + + # format_html() escapes its args, so it's the single escape point for the name. + # (A manual escape() here was redundant — it returns a SafeString that format_html's + # conditional_escape passes through, so it didn't double-encode, just obscured intent.) + display_name = lnms_name or "" + if type_label and display_name: + badge_text = format_html("{} {}", type_label, display_name) + elif display_name: + badge_text = display_name else: - # IDs match - return format_html('{}', value) + badge_text = type_label # may be "" (e.g. missing_lnms with no name) → icon-only pill + title = f"{type_label}: {status_text}" if type_label else status_text + badge = format_html( + '' + '{}', + badge_css, + title, + icon, + badge_text, + ) + + # Show the inline sync button when LibreNMS has a relationship to apply (lnms_port_id + # set) and NetBox either lacks it (missing_nb) or holds a DIFFERENT one (mismatch) — + # in both cases the row can be reconciled to the LibreNMS value from here. missing_lnms + # is excluded by the lnms_port_id guard (nothing to sync to), and a migrated donor page + # suppresses the control entirely: the per-row .lag-sync-btn/.parent-sync-btn POST + # directly via librenms_sync.js, so leaving it active would let a migrated donor mutate + # parent/LAG state despite the bulk form being hidden. + if ( + sync_status in ("missing_nb", "mismatch") + and lnms_port_id + and record.get("netbox_interface") is not None + and record.get("relationship_source_resolvable", True) + and target_resolvable + and not self.migrated_to_marker + ): + port_id = record.get("port_id", "") + # Resolve the owning member the same way the VC member dropdown does, so the button's + # data-object-id and the dropdown agree (the JS posts the dropdown value, so a + # disagreement would 404). See _resolve_row_member_id. + object_id = self._resolve_row_member_id(record) + if not object_id: + # No resolvable owner. reverse() would raise NoReverseMatch and take down the + # whole table render, so degrade this one cell the way target_resolvable does. + return format_html('
{}
', badge) + object_type = record.get("selected_object_type") or self.sync_object_type + route_name = "sync_interface_lag" if btn_class == "lag-sync-btn" else "sync_interface_parent" + sync_url = reverse( + f"plugins:netbox_librenms_plugin:{route_name}", + kwargs={"object_type": object_type, "object_id": object_id}, + ) + # A mismatch click OVERWRITES the differing NetBox lag/parent with the LibreNMS + # value, so spell that out in the tooltip rather than the generic "Sync". + sync_title = ( + f"Update {type_label or 'relationship'} to match LibreNMS" + if sync_status == "mismatch" + else "Sync relationship" + ) + btn = format_html( + ' ', + btn_class, + port_id, + data_related_key, + lnms_port_id, + object_type, + object_id, + sync_url, + sync_title, + sync_title, + ) + # text-nowrap keeps the pill + sync button on one line (no mid-line wrap); lh-sm keeps + # the LAG/Parent lines tightly stacked. + return format_html('
{} {}
', badge, btn) + + return format_html('
{}
', badge) def _compare_mac_addresses(self, librenms_mac, netbox_interface): """ @@ -517,10 +816,33 @@ def format_interface_data(self, port_data, device): # unrelated host interface and inviting a sync the server then silently skips. if port_data.get("_source") == "oob": port_data["netbox_interface"] = None - else: - port_data["netbox_interface"] = device.interfaces.filter(name=interface_name).first() + # Preserve a netbox_interface already resolved by the stable port_id (e.g. the single- + # interface verify view resolves by port_id first). Only fall back to the fragile name + # lookup when nothing has been resolved yet, so a display-name change or collision can't + # clobber the correct port-id match with the wrong (or no) name-matched interface. + elif not port_data.get("netbox_interface"): + candidate = device.interfaces.filter(name=interface_name).first() + port_data["netbox_interface"] = ( + candidate + if candidate + and port_data.get("name_fallback_allowed", False) + and interface_name_fallback_matches_port( + candidate, + port_data.get("port_id"), + self.server_key, + ) + else None + ) port_data["exists_in_netbox"] = bool(port_data["netbox_interface"]) + # Stamp the row's actual object so the relationship sync button targets it even when the + # row has no matching NetBox interface yet (missing_nb). This is set here, where the + # caller passes the row-selected device (e.g. the cross-page VC member switch), so the + # missing_nb branch in _render_relationship_column can prefer it over the + # name-based VC heuristic, which would otherwise post to the wrong device. + port_data["selected_object_id"] = getattr(device, "pk", None) + port_data["selected_object_type"] = self.sync_object_type + # Clear description if it matches interface name if port_data["ifAlias"] == port_data["ifName"] or port_data["ifAlias"] == port_data["ifDescr"]: port_data["ifAlias"] = "" @@ -533,6 +855,15 @@ def format_interface_data(self, port_data, device): "mtu": self.render_mtu(port_data["ifMtu"], port_data), "enabled": self.render_enabled(port_data["ifAdminStatus"], port_data), "description": self.render_description(port_data["ifAlias"], port_data), + "vlans": self.render_vlans(None, port_data), + # The librenms_id badge's colour is member-specific (it compares this port_id + # against the resolved NetBox interface's device librenms_id), so a VC member + # switch must repaint it too — otherwise it keeps the previous member's + # match/mismatch state. The column accessor is "port_id" (see the column def). + "librenms_id": self.render_librenms_id(port_data.get("port_id"), port_data), + # Renders from the lag/parent enrichment keys the caller stamps onto + # port_data; absent enrichment it returns "" (safe empty cell). + "parent": self.render_parent(None, port_data), } return formatted_data @@ -568,8 +899,6 @@ def __init__(self, *args, device=None, interface_name_field=None, vlan_groups=No # Ensure device_selection column is visible if hasattr(self.device, "virtual_chassis") and self.device.virtual_chassis: self.columns.show("device_selection") - # Update selection column accessor to match interface_name_field - self.base_columns["selection"].accessor = self.interface_name_field def render_device_selection(self, value, record): """ @@ -577,24 +906,31 @@ def render_device_selection(self, value, record): Determines the selected member based on interface type and name. Returns an HTML select element with appropriate member options. """ - members = self.device.virtual_chassis.members.all() - if_type = record.get("ifType", "").lower() + # Reuse the per-render member prefetch (see _vc_members) so the dropdown doesn't re-query + # the chassis members for every row (N+1 on a large chassis). + members = self._vc_members interface_name = record.get(self.interface_name_field) + port_id = record.get("port_id", "") - if "ethernet" in if_type: - chassis_member = get_virtual_chassis_member(self.device, interface_name) - selected_member_id = chassis_member.id if chassis_member else self.device.id - else: - selected_member_id = self.device.id + # Default the dropdown to the same owner the relationship sync button resolves (matched + # NetBox interface's device → cross-page selection → name heuristic), so the JS — which + # posts this dropdown's value as the sync object id — can't disagree with the button and + # 404. Previously non-ethernet rows always defaulted to the viewed member, breaking sync + # for a sub-interface owned by a different VC member. + selected_member_id = self._resolve_row_member_id(record) or self.device.id # Create unique base ID for TomSelect components - base_id = f"device_selection_{interface_name}_{hash(interface_name)}" + base_id = f"device_selection_{port_id}" + disabled = mark_safe(' disabled="disabled"') if not record.get("sync_target_resolvable", True) else "" return format_html( - '', - interface_name, + '', + port_id, base_id, + interface_name, render_vc_member_options(members, selected_member_id), + disabled, ) def format_interface_data(self, port_data, device): @@ -617,6 +953,8 @@ class Meta: "mtu", "enabled", "description", + "librenms_id", + "parent", ] attrs = { "class": "table table-hover object-list", @@ -629,6 +967,9 @@ class LibreNMSVMInterfaceTable(LibreNMSInterfaceTable): Table for displaying LibreNMS VM interface data. """ + # These rows sync against VirtualMachine objects regardless of whether the VM has a cluster. + sync_object_type = "virtualmachine" + class Meta(LibreNMSInterfaceTable.Meta): """Meta options for LibreNMSVMInterfaceTable.""" @@ -640,6 +981,11 @@ class Meta(LibreNMSInterfaceTable.Meta): "mtu", "enabled", "description", + "librenms_id", + # VMInterface supports sub-interface parents (LAG is skipped for VMs), and the + # relationship sync path resolves VMInterface targets — so the Parent/LAG column + # must be exposed here too, otherwise the feature is unreachable on VM pages. + "parent", ] attrs = { "class": "table table-hover object-list", diff --git a/netbox_librenms_plugin/tables/mappings.py b/netbox_librenms_plugin/tables/mappings.py index 1a270d144f..549bac2944 100644 --- a/netbox_librenms_plugin/tables/mappings.py +++ b/netbox_librenms_plugin/tables/mappings.py @@ -11,6 +11,7 @@ ModuleTypeMapping, NormalizationRule, PlatformMapping, + PortStackLagPattern, ) @@ -327,3 +328,39 @@ class Meta: "actions", ) attrs = {"class": "table table-hover table-headings table-striped"} + + +class PortStackLagPatternTable(NetBoxTable): + """Table for displaying PortStackLagPattern data.""" + + # Use NetBoxTable's default pk ToggleColumn (input name="pk"). This table is rendered only by + # generic NetBox views (ObjectListView, BulkDeleteView) and the plugin's BulkExportYAMLView, + # all of which read request.POST.getlist("pk"); the generic list pages don't load the plugin's + # sync/import selection JS. Overriding the input name to "select" silently broke select-all, + # bulk delete, and "Export Selected (YAML)" (the view always saw zero selected pks). + librenms_os = tables.Column(verbose_name="LibreNMS OS", linkify=True) + lag_name_pattern = tables.Column(verbose_name="LAG Name Pattern (regex)") + description = tables.Column(verbose_name="Description", linkify=False) + actions = columns.ActionsColumn(actions=("edit", "delete")) + + class Meta: + """Meta options for PortStackLagPatternTable.""" + + model = PortStackLagPattern + fields = ( + "pk", + "id", + "librenms_os", + "lag_name_pattern", + "description", + "actions", + ) + default_columns = ( + "pk", + "id", + "librenms_os", + "lag_name_pattern", + "description", + "actions", + ) + attrs = {"class": "table table-hover table-headings table-striped"} 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 c6afd05224..36254beb78 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_interface_sync.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_interface_sync.html @@ -15,20 +15,21 @@

Interface Sync

{% include "netbox_librenms_plugin/inc/_hidden_server_key.html" %} {% if has_librenms_id %} {% with model_name=object|meta:"model_name" %} + {% comment %} + Resolve the refresh URL per object type, then render ONE button so the shared + hx-vals (pagination + server_key forwarding) lives in a single place. + {% endcomment %} {% if model_name == "device" %} - {# type="button": HTMX drives the POST via hx-post; the default type="submit" would also fire a native form submit. #} - + {% url 'plugins:netbox_librenms_plugin:device_interface_sync' pk=object.pk as interface_sync_url %} {% elif model_name == "virtualmachine" %} + {% url 'plugins:netbox_librenms_plugin:vm_interface_sync' pk=object.pk as interface_sync_url %} + {% endif %} + {% if interface_sync_url %} diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_interface_sync_content.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_interface_sync_content.html index 12c5ab9357..08b35f0407 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_interface_sync_content.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_interface_sync_content.html @@ -11,16 +11,26 @@ {% endif %} +{% if interface_sync.relationship_data_incomplete %} + +{% endif %} + {# 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. #} {% with model_name=interface_sync.object|meta:"model_name" %} {% if migrated_to_marker %} -
+
{% comment %} No POST form in migrated mode, but the interface table still renders interactive relationship/VC-member dropdowns whose verify-interface POST reads the token via document.querySelector('[name=csrfmiddlewaretoken]'). Emit a standalone token so those JS-driven requests don't hit a null token (TypeError/403). A bare hidden input never auto-submits, so it doesn't reintroduce the live-form problem flagged above. + The same JS handlers read input[name="server_key"] to scope verify/sync POSTs to the + right LibreNMS server; without it, migrated-mode requests on a non-default server fall + back to null and hit the wrong server/cache. Emit it here too (still no auto-submit). {% endcomment %} {% comment %} @@ -33,6 +43,7 @@ {% endwith %} {% else %}
{# Form-only inputs live inside the form branch so migrated (donor) mode, which renders a plain
wrapper, never emits hidden fields outside any . #} {% csrf_token %} @@ -41,7 +52,7 @@ {% endwith %} {% block table_actions %}
-
+
{% if not migrated_to_marker %}
{% if not migrated_to_marker %}
@@ -331,9 +346,11 @@