From afca5fd9db19fc3781020ee45a3cd819a389a794 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 16 Jun 2026 10:52:27 +0200 Subject: [PATCH 001/163] fix(import,tables,templates): harden malformed payloads + migrated-mode CSRF/transfer gating - collisions.detect_bulk_collisions: skip non-dict rows / non-dict validation instead of crashing the whole bulk-confirm flow on .get(). - device_status actions render: type-check existing_librenms_link before reading pairing keys so a malformed payload can't break the table render. - _vlan_sync_content.html: render CSRF + server_key in migrated mode too (the VLAN verify JS reads csrfmiddlewaretoken and posts server_key); only the form-submit action input stays gated. - librenms_sync_base.html: gate the donor transfer-IP buttons to Device pages (object|meta model_name == device) so a VM can't drive device_transfer_ip on a same-pk Device. - tests: red->green coverage for each; add non-dict main-ports row case; pin OOB-only fixture librenms_id=None; drop brittle '?tab=' >=5 sanity threshold. --- .../tests/test_vlan_sync_content_template.py | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 netbox_librenms_plugin/tests/test_vlan_sync_content_template.py diff --git a/netbox_librenms_plugin/tests/test_vlan_sync_content_template.py b/netbox_librenms_plugin/tests/test_vlan_sync_content_template.py new file mode 100644 index 0000000000..8d9e31570e --- /dev/null +++ b/netbox_librenms_plugin/tests/test_vlan_sync_content_template.py @@ -0,0 +1,69 @@ +"""Render the real _vlan_sync_content.html template in both modes. + +In migrated mode the POST form is replaced by a plain
(a migrated donor must not POST a +VLAN sync). But the VLAN table still renders interactive per-row group selects whose verify JS +(librenms_sync.js verify-vlan-group / verify-vlan-sync-group) reads +document.querySelector('[name=csrfmiddlewaretoken]').value and posts server_key — so standalone +CSRF + server_key hidden inputs must be emitted in migrated mode too, or those JS requests hit a +null token (TypeError/403) / the wrong server. Only the form-submit ``action`` input is form-only. +""" + +import pytest + + +@pytest.mark.django_db +class TestVlanSyncContentTemplateMigratedMode: + def _render(self, *, migrated, server_key="default"): + from django.contrib.auth.models import AnonymousUser + from django.template.loader import render_to_string + from django.test import RequestFactory + from django_tables2 import RequestConfig + + from netbox_librenms_plugin.tables.vlans import LibreNMSVLANTable + from netbox_librenms_plugin.tests.conftest import make_device + + device = make_device("vlan-tmpl-dev") + request = RequestFactory().get("/") + request.user = AnonymousUser() # NetBox context processors read request.user + # At least one row so vlan_table.rows is truthy and the form/CSRF branch renders. + table = LibreNMSVLANTable( + [{"vlan_id": 10, "name": "v10", "type": "ethernet", "state": "active"}], + vlan_groups=[], + ) + RequestConfig(request).configure(table) + vlan_sync = { + "object": device, + "vlan_table": table, + "server_key": server_key, + "cache_expiry": None, + } + ctx = { + "vlan_sync": vlan_sync, + "migrated_to_marker": migrated, + "migrated_to_winner": None, + "has_write_permission": False, + } + return render_to_string("netbox_librenms_plugin/_vlan_sync_content.html", ctx, request=request) + + def test_migrated_mode_drops_form_but_keeps_csrf_and_server_key(self): + # Non-default server so the assertion proves the actual value is emitted. + html = self._render( + migrated={"server_key": "prod", "device_id": 1, "at": "now"}, + server_key="prod", + ) + # The live POST form must be gone in migrated mode (a donor must not POST a sync). + assert " Date: Sat, 30 May 2026 20:10:33 +0200 Subject: [PATCH 002/163] feat(pci): parent/LAG/child interface UI and PortStackLagPattern - Add PortStackLagPattern model for vendor LAG name patterns with CRUD UI (list, create, edit, delete, import, changelog, YAML export) - Populate PortStackLagPattern with known vendor patterns - Merge LAG column into Parent/LAG column showing relationship inline - Auto-select parent interface when sub-interface is checked - Detect sub-interfaces via port_stack signal (not just LAGs) - Fix LAG member auto-select for Nokia TiMOS and paginated tables - Show relationship name inline in Parent/LAG badges - Split interface name into separate badge with better contrast - Remove librenms_id column (not needed outside debugging) - Fix stale and broken tests (TestRenderLibreNMSId, VC mock, POST mock) --- netbox_librenms_plugin/filters.py | 15 + netbox_librenms_plugin/forms.py | 30 ++ netbox_librenms_plugin/librenms_api.py | 149 ++++++++++ .../migrations/0011_portstacklagpattern.py | 71 +++++ netbox_librenms_plugin/models.py | 67 +++++ netbox_librenms_plugin/navigation.py | 19 ++ .../js/librenms_sync.js | 185 +++++++++++++ netbox_librenms_plugin/tables/interfaces.py | 131 +++++++-- netbox_librenms_plugin/tables/mappings.py | 32 +++ .../_interface_sync.html | 2 + .../_interface_sync_content.html | 6 +- .../portstacklagpattern.html | 28 ++ .../portstacklagpattern_list.html | 21 ++ .../tests/test_coverage_base_views.py | 8 + .../tests/test_coverage_sync_interfaces.py | 72 +++++ .../tests/test_coverage_tables.py | 70 ----- .../tests/test_librenms_api.py | 200 +++++++++++++- .../tests/test_port_stack_lag_pattern.py | 58 ++++ netbox_librenms_plugin/tests/test_utils.py | 1 + netbox_librenms_plugin/urls.py | 71 +++++ netbox_librenms_plugin/utils.py | 7 +- netbox_librenms_plugin/views/__init__.py | 16 +- .../views/base/interfaces_view.py | 151 ++++++++++ netbox_librenms_plugin/views/mapping_views.py | 70 +++++ .../views/sync/interfaces.py | 259 ++++++++++++++++++ 25 files changed, 1634 insertions(+), 105 deletions(-) create mode 100644 netbox_librenms_plugin/migrations/0011_portstacklagpattern.py create mode 100644 netbox_librenms_plugin/templates/netbox_librenms_plugin/portstacklagpattern.html create mode 100644 netbox_librenms_plugin/templates/netbox_librenms_plugin/portstacklagpattern_list.html create mode 100644 netbox_librenms_plugin/tests/test_port_stack_lag_pattern.py diff --git a/netbox_librenms_plugin/filters.py b/netbox_librenms_plugin/filters.py index 1edb57773c..d601bc99bb 100644 --- a/netbox_librenms_plugin/filters.py +++ b/netbox_librenms_plugin/filters.py @@ -10,6 +10,7 @@ ModuleTypeMapping, NormalizationRule, PlatformMapping, + PortStackLagPattern, ) @@ -143,3 +144,17 @@ class Meta: "librenms_child_name_pattern", "netbox_bay_name_pattern", ] + + +class PortStackLagPatternFilterSet(django_filters.FilterSet): + """Filter set for PortStackLagPattern model.""" + + librenms_os = django_filters.CharFilter(lookup_expr="icontains") + lag_name_pattern = django_filters.CharFilter(lookup_expr="icontains") + description = django_filters.CharFilter(lookup_expr="icontains") + + 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..cc953cb4f7 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,35 @@ 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") + + 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/librenms_api.py b/netbox_librenms_plugin/librenms_api.py index dcfe152a5a..954b80b754 100644 --- a/netbox_librenms_plugin/librenms_api.py +++ b/netbox_librenms_plugin/librenms_api.py @@ -545,6 +545,155 @@ 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() + return True, data.get("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 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, + ) -> dict: + """ + Resolve LAG membership and sub-interface parent relationships from LibreNMS data. + + Universal rules (vendor-agnostic, hardcoded): + 1. LAG aggregate is always the 'low' entry in a port_stack pair. + 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: if low_name starts with high_name + '.' and the + suffix is numeric, it is a sub-interface parent/child pair. + + 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. + + Returns: + dict with keys: + 'lag_members': {member_port_id: aggregate_port_id} + 'sub_interfaces': {child_port_id: parent_port_id} + """ + import re as _re + + if lag_patterns is None: + from netbox_librenms_plugin.models import PortStackLagPattern + + lag_patterns = {p.librenms_os: p.lag_name_pattern for p in PortStackLagPattern.objects.all()} + + by_id = {p["port_id"]: p for p in ports if p.get("port_id")} + by_name = {p["ifName"]: p for p in ports if p.get("ifName")} + + compiled_patterns = [] + for pattern_str in lag_patterns.values(): + try: + compiled_patterns.append(_re.compile(pattern_str)) + except _re.error: + pass + + lag_members: dict = {} + sub_interfaces: dict = {} + + def _is_lag_aggregate(port: dict) -> bool: + if port.get("ifType") == "ieee8023adLag": + return True + name = port.get("ifName", "") + return any(pat.search(name) for pat in compiled_patterns) + + def _resolve_physical(name: str): + """Strip .N suffix and return the physical-level port if its base name exists.""" + if "." in name: + base = name.rsplit(".", 1)[0] + if base in by_name: + return by_name[base] + return by_name.get(name) + + for entry in port_stack: + if not isinstance(entry, dict): + continue + high_id = entry.get("high_port_id") + low_id = entry.get("low_port_id") + if not high_id or not low_id: + continue + + high_port = by_id.get(high_id) + low_port = by_id.get(low_id) + if not high_port or not low_port: + continue + + h_name = high_port.get("ifName", "") + l_name = low_port.get("ifName", "") + + # Universal rule: skip Nokia SAP entries (colon notation: lag1:0, lag-1:10) + if ":" in h_name or ":" in l_name: + continue + + # Sub-interface detection: low is child of high when name follows parent.N pattern + if l_name.startswith(h_name + "."): + suffix = l_name[len(h_name) + 1 :] + try: + int(suffix) # Only numeric suffixes qualify as sub-interfaces + sub_interfaces[low_id] = high_id + continue + except ValueError: + pass # Non-numeric suffix — fall through to LAG check + + # LAG membership: resolve physical-level names (strips Junos sub-unit .N suffix) + high_phys = _resolve_physical(h_name) + low_phys = _resolve_physical(l_name) + if not high_phys or not low_phys: + continue + + high_phys_id = high_phys["port_id"] + low_phys_id = low_phys["port_id"] + if high_phys_id == low_phys_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 not high_is_agg: + lag_members[high_phys_id] = low_phys_id + elif high_is_agg and not low_is_agg: + lag_members[low_phys_id] = high_phys_id + + 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/0011_portstacklagpattern.py b/netbox_librenms_plugin/migrations/0011_portstacklagpattern.py new file mode 100644 index 0000000000..bda277e649 --- /dev/null +++ b/netbox_librenms_plugin/migrations/0011_portstacklagpattern.py @@ -0,0 +1,71 @@ +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 = [ + ("extras", "0138_customfieldchoiceset_choice_colors"), + ("netbox_librenms_plugin", "0010_inventory_and_mapping_models"), + ] + + 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(max_length=50, unique=True)), + ("lag_name_pattern", models.CharField(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/models.py b/netbox_librenms_plugin/models.py index e8aa8dec77..f52367b30f 100644 --- a/netbox_librenms_plugin/models.py +++ b/netbox_librenms_plugin/models.py @@ -916,3 +916,70 @@ def to_yaml(self): "description": self.description, } return yaml.dump(data, sort_keys=False) + + +class PortStackLagPattern(FullCleanOnSaveMixin, NetBoxModel): + """Maps LibreNMS OS name to the regex pattern 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 always in the 'low' position of a port_stack pair. + - 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, + unique=True, + 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) + + def clean(self): + """Validate OS name is non-blank and lag_name_pattern is a valid regex.""" + super().clean() + 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 + try: + re.compile(self.lag_name_pattern) + except re.error as exc: + raise ValidationError({"lag_name_pattern": f"Invalid regular expression: {exc}"}) + + 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" + + def __str__(self): + return f"{self.librenms_os} -> {self.lag_name_pattern}" diff --git a/netbox_librenms_plugin/navigation.py b/netbox_librenms_plugin/navigation.py index b10cbb9deb..7e7572bdb9 100644 --- a/netbox_librenms_plugin/navigation.py +++ b/netbox_librenms_plugin/navigation.py @@ -59,6 +59,25 @@ link_text="Rules & Patterns", permissions=[PERM_VIEW_PLUGIN], ), + PluginMenuItem( + link="plugins:netbox_librenms_plugin:portstacklagpattern_list", + link_text="Port Stack LAG Patterns", + permissions=[PERM_VIEW_PLUGIN], + buttons=( + PluginMenuButton( + link="plugins:netbox_librenms_plugin:portstacklagpattern_add", + title="Add", + icon_class="mdi mdi-plus-thick", + permissions=[PERM_CHANGE_PLUGIN], + ), + PluginMenuButton( + link="plugins:netbox_librenms_plugin:portstacklagpattern_bulk_import", + title="Import", + icon_class="mdi mdi-upload", + permissions=[PERM_CHANGE_PLUGIN], + ), + ), + ), ), ), ( 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..c184cb96c4 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 @@ -373,6 +373,127 @@ 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 + * (not in DOM), a hidden is injected into the form so + * the parent is still included in the sync POST, 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"]')) return; + + const toggle = document.getElementById('autoSelectLagMembers'); + if (!toggle || !toggle.checked) return; + + const row = checkbox.closest('tr'); + if (!row) return; + + let changed = false; + + // --- LAG: check/uncheck all members --- + const portId = row.dataset.portId; + if (portId) { + const memberRows = document.querySelectorAll('tr[data-member-of-lag="' + portId + '"]'); + memberRows.forEach(function (memberRow) { + const memberCheckbox = memberRow.querySelector('input[name="select"]'); + if (memberCheckbox) { + memberCheckbox.checked = checkbox.checked; + changed = true; + } + }); + } + + // --- Sub-interface: select parent when checking --- + const parentPortId = row.dataset.parentPortId; + if (parentPortId && checkbox.checked) { + const parentRow = document.querySelector('tr[data-port-id="' + parentPortId + '"]'); + if (parentRow) { + // Parent is on the same page - check it directly + const parentCheckbox = parentRow.querySelector('input[name="select"]'); + if (parentCheckbox && !parentCheckbox.checked) { + parentCheckbox.checked = true; + changed = true; + } + } else { + // Parent is on a different page - inject a hidden input into the form + const parentName = row.dataset.parentName; + if (parentName) { + const form = checkbox.closest('form'); + if (form) { + const hiddenId = 'auto-parent-' + parentPortId; + if (!form.querySelector('#' + hiddenId)) { + const hidden = document.createElement('input'); + hidden.type = 'hidden'; + hidden.name = 'select'; + hidden.value = parentName; + hidden.id = hiddenId; + form.appendChild(hidden); + _showParentCrossPageNotice(parentName); + } + } + } + } + } + + // --- Sub-interface: remove cross-page hidden input when unchecking --- + if (parentPortId && !checkbox.checked) { + const form = checkbox.closest('form'); + if (form) { + const hidden = form.querySelector('#auto-parent-' + parentPortId); + if (hidden) hidden.remove(); + } + } + + if (changed) { + updateBulkActionButton(); + } +}); + +/** + * 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; + notice.innerHTML = + '' + + 'Parent interface ' + parentName + ' is on another page ' + + 'and will be included in the sync automatically.' + + ''; + container.appendChild(notice); + + setTimeout(function () { + if (notice.parentNode) notice.parentNode.removeChild(notice); + }, 5000); +} + // ============================================ // VIRTUAL CHASSIS & VRF HANDLING // ============================================ @@ -2089,3 +2210,67 @@ document.addEventListener('htmx:afterSettle', function (event) { } } }); + +// Event delegation for LAG and parent interface sync buttons. +// Buttons are rendered inline in the interface table cells (data-col="lag" / "parent") +// and carry data attributes: port-id, lag-port-id / parent-port-id, object-type, object-id. +document.addEventListener('click', function (e) { + const btn = e.target.closest('.lag-sync-btn, .parent-sync-btn'); + if (!btn) return; + e.preventDefault(); + + const isLag = btn.classList.contains('lag-sync-btn'); + const portId = btn.dataset.portId || ''; + const relatedPortId = isLag ? (btn.dataset.lagPortId || '') : (btn.dataset.parentPortId || ''); + const relatedName = btn.dataset.relatedName || ''; + const objectType = btn.dataset.objectType || ''; + const objectId = btn.dataset.objectId || ''; + const relatedKey = isLag ? 'lag_port_id' : 'parent_port_id'; + const relatedNameKey = isLag ? 'lag_name' : 'parent_name'; + const urlSuffix = isLag ? 'sync-interface-lag' : 'sync-interface-parent'; + + if (!portId || !relatedPortId || !objectType || !objectId) return; + + const url = `/plugins/librenms_plugin/${objectType}/${objectId}/${urlSuffix}/`; + + const csrfInput = document.querySelector('[name=csrfmiddlewaretoken]'); + const csrf = csrfInput ? csrfInput.value : ''; + + const serverKeyInput = document.querySelector('[name=server_key]'); + const serverKey = serverKeyInput ? serverKeyInput.value : ''; + + const body = new URLSearchParams({ + csrfmiddlewaretoken: csrf, + port_id: portId, + [relatedKey]: relatedPortId, + [relatedNameKey]: relatedName, + server_key: serverKey, + }); + + btn.disabled = true; + btn.innerHTML = ''; + + fetch(url, { + method: 'POST', + headers: { 'X-CSRFToken': csrf, 'Content-Type': 'application/x-www-form-urlencoded' }, + body: body.toString(), + }) + .then(function (r) { + return r.json(); + }) + .then(function (data) { + if (data.status === 'success') { + btn.innerHTML = ''; + btn.title = data.message || 'Synced'; + } else { + btn.disabled = false; + btn.innerHTML = ''; + btn.title = data.error || 'Sync failed'; + } + }) + .catch(function (e) { + btn.disabled = false; + btn.innerHTML = ''; + btn.title = e.message || 'Request failed'; + }); +}); diff --git a/netbox_librenms_plugin/tables/interfaces.py b/netbox_librenms_plugin/tables/interfaces.py index fad34b4810..8374beb3d8 100644 --- a/netbox_librenms_plugin/tables/interfaces.py +++ b/netbox_librenms_plugin/tables/interfaces.py @@ -13,7 +13,6 @@ convert_speed_to_kbps, format_mac_address, get_interface_name_field, - get_librenms_device_id, get_missing_vlan_warning, get_table_paginate_count, get_tagged_vlan_css_class, @@ -42,7 +41,7 @@ class Meta: "mtu", "enabled", "description", - "librenms_id", + "parent", ] attrs = { "class": "table table-hover object-list", @@ -72,6 +71,10 @@ def __init__(self, *args, device=None, interface_name_field=None, vlan_groups=No "data-enabled": lambda record: ( str(record.get("ifAdminStatus")).lower() if record.get("ifAdminStatus") is not None else "" ), + "data-port-id": lambda record: str(record.get("port_id", "")), + "data-member-of-lag": lambda record: str(record.get("librenms_lag_port_id") or ""), + "data-parent-port-id": lambda record: str(record.get("librenms_parent_port_id") or ""), + "data-parent-name": lambda record: str(record.get("librenms_parent_name") or ""), } super().__init__(*args, **kwargs) @@ -103,10 +106,11 @@ def __init__(self, *args, device=None, interface_name_field=None, vlan_groups=No verbose_name="Description", attrs={"td": {"data-col": "description"}}, ) - librenms_id = tables.Column( - accessor="port_id", - verbose_name="LibreNMS ID", - attrs={"td": {"data-col": "librenms_id"}}, + parent = tables.Column( + verbose_name="Parent / LAG", + orderable=False, + empty_values=(), + attrs={"td": {"data-col": "parent"}}, ) vlans = tables.Column( verbose_name="VLANs", @@ -373,34 +377,104 @@ def render_mtu(self, value, record): """Render MTU with appropriate styling based on comparison with NetBox""" 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.""" + def render_parent(self, value, record): + """Render combined Parent / LAG relationship column. - # 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). - if not record.get("exists_in_netbox"): - return format_html('{}', value) + Shows LAG membership (if any) and parent interface (if any) stacked + vertically, each prefixed with a small muted label so the type is clear. + The sync buttons keep their existing CSS classes (lag-sync-btn / + parent-sync-btn) so the JS handler still works without changes. + """ + parts = [] + + lag_status = record.get("lag_sync_status") + if lag_status is not None: + lag_content = self._render_relationship_column( + 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", + ) + parts.append( + format_html( + '
LAG{}
', + lag_content, + ) + ) - netbox_interface = record.get("netbox_interface") - if not netbox_interface: - return format_html('{}', value) + parent_status = record.get("parent_sync_status") + if parent_status is not None: + parent_content = self._render_relationship_column( + 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", + ) + parts.append( + format_html( + '
Parent{}
', + parent_content, + ) + ) - netbox_librenms_id = get_librenms_device_id(netbox_interface, self.server_key, auto_save=False) + if not parts: + return mark_safe("") - if netbox_librenms_id is None: - return format_html( - '{}', value - ) + return mark_safe("".join(str(p) for p in parts)) + + def _render_relationship_column(self, lnms_name, lnms_port_id, sync_status, record, btn_class, data_related_key): + """Shared renderer for LAG and Parent relationship columns.""" + if sync_status is None: + return mark_safe("") - # Compare the IDs - if str(value) != str(netbox_librenms_id): - # IDs do not match - return format_html( - '{}', netbox_librenms_id, value + status_map = { + "match": ("bg-success", "Match"), + "mismatch": ("bg-warning text-dark", "Mismatch"), + "missing_nb": ("bg-info text-dark", "Not in NetBox"), + "missing_lnms": ("bg-secondary", "Not in LibreNMS"), + } + badge_css, badge_label = status_map.get(sync_status, ("bg-secondary", sync_status)) + + display_name = escape(lnms_name or "") + status_badge = format_html('{}', badge_css, badge_label) + name_badge = ( + format_html(' {}', display_name) + if display_name + else "" + ) + badge = format_html("{}{}", status_badge, name_badge) + + if sync_status == "missing_nb" and lnms_port_id: + port_id = record.get("port_id", "") + nb_iface = record.get("netbox_interface") + object_id = ( + nb_iface.device_id + if nb_iface and hasattr(nb_iface, "device_id") + else (self.device.pk if self.device else "") ) - else: - # IDs match - return format_html('{}', value) + object_type = "virtualmachine" if hasattr(self.device, "cluster") and self.device.cluster else "device" + btn = format_html( + ' ', + btn_class, + port_id, + data_related_key, + lnms_port_id, + lnms_name or "", + object_type, + object_id, + ) + return format_html("{} {}", badge, btn) + + return badge def _compare_mac_addresses(self, librenms_mac, netbox_interface): """ @@ -617,6 +691,7 @@ class Meta: "mtu", "enabled", "description", + "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..403a6a944c 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,34 @@ class Meta: "actions", ) attrs = {"class": "table table-hover table-headings table-striped"} + + +class PortStackLagPatternTable(NetBoxTable): + """Table for displaying PortStackLagPattern data.""" + + 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..54bf628d75 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_interface_sync.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_interface_sync.html @@ -21,6 +21,7 @@

Interface Sync

hx-post="{% url 'plugins:netbox_librenms_plugin:device_interface_sync' pk=object.pk %}" hx-target="#interface-sync-content" hx-include="[name='interface_name_field'], [name='server_key']" + hx-vals="js:{interfaces_per_page: new URLSearchParams(window.location.search).get('interfaces_per_page') || ''}" class="btn btn-outline-primary"> Refresh Interfaces @@ -29,6 +30,7 @@

Interface Sync

hx-post="{% url 'plugins:netbox_librenms_plugin:vm_interface_sync' pk=object.pk %}" hx-target="#interface-sync-content" hx-include="[name='interface_name_field'], [name='server_key']" + hx-vals="js:{interfaces_per_page: new URLSearchParams(window.location.search).get('interfaces_per_page') || ''}" class="btn btn-outline-primary"> Refresh Interfaces 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..92c16dc869 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 @@ -41,7 +41,7 @@ {% endwith %} {% block table_actions %}
-
+
{% if not migrated_to_marker %}
{% if not migrated_to_marker %}
diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/portstacklagpattern.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/portstacklagpattern.html new file mode 100644 index 0000000000..9c921b249f --- /dev/null +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/portstacklagpattern.html @@ -0,0 +1,28 @@ +{% extends 'generic/object.html' %} +{% load helpers %} +{% load plugins %} + +{% block content %} +
+
+
+ + + + + + + + + + + + + + + +
LibreNMS OSLAG Name Pattern (regex)Description
{{ object.librenms_os }}{{ object.lag_name_pattern }}{{ object.description|default:"—" }}
+
+
+
+{% endblock %} diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/portstacklagpattern_list.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/portstacklagpattern_list.html new file mode 100644 index 0000000000..444dabf44d --- /dev/null +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/portstacklagpattern_list.html @@ -0,0 +1,21 @@ +{% extends 'generic/object_list.html' %} + +{% block content %} +
+

Port Stack LAG Patterns

+

Define regular expression patterns to identify LAG (Link Aggregation Group) aggregate + interfaces by operating system when the interface type is not ieee8023adLag. + These patterns are used as a fallback during interface synchronization to correctly + identify parent-child relationships in port stacks.

+

Example: Cisco IOS port-channels have ifType='propVirtual' and need + name-based identification via pattern ^Po\d+$.

+
+ {{ block.super }} +{% endblock %} + +{% block bulk_buttons %} + {{ block.super }} + +{% endblock %} diff --git a/netbox_librenms_plugin/tests/test_coverage_base_views.py b/netbox_librenms_plugin/tests/test_coverage_base_views.py index 10a7eabe6f..8bafa042bb 100644 --- a/netbox_librenms_plugin/tests/test_coverage_base_views.py +++ b/netbox_librenms_plugin/tests/test_coverage_base_views.py @@ -1723,6 +1723,9 @@ def _record_get_ports(_id): patch.object(view, "get_object", return_value=obj), patch.object(view, "get_redirect_url", return_value="/device/1/"), patch.object(view, "_enrich_ports_with_vlan_data", return_value=[]), + # No LAG/sub-interface enrichment under test here — short-circuit the + # port_stack fetch (which would otherwise hit PortStackLagPattern in the DB). + patch.object(view, "_has_lag_signals", return_value=False), patch.object(view, "get_context_data", return_value={}), patch.object(view, "get_cache_key", return_value="cache-key"), patch.object(view, "get_last_fetched_key", return_value="last-key"), @@ -1761,6 +1764,9 @@ def test_post_oob_fetch_failure_caches_incomplete_snapshot(self): patch.object(view, "get_object", return_value=obj), patch.object(view, "get_redirect_url", return_value="/device/1/"), patch.object(view, "_enrich_ports_with_vlan_data", side_effect=lambda ports, field: ports), + # No LAG/sub-interface enrichment under test here — short-circuit the + # port_stack fetch (which would otherwise hit PortStackLagPattern in the DB). + patch.object(view, "_has_lag_signals", return_value=False), patch.object(view, "get_context_data", return_value={}), patch.object(view, "get_cache_key", return_value="cache-key"), patch.object(view, "get_last_fetched_key", return_value="last-key"), @@ -2021,6 +2027,7 @@ def test_post_success_caches_and_renders(self): patch.object(view, "get_object", return_value=obj), patch.object(view, "get_redirect_url", return_value="/device/1/"), patch.object(view, "_enrich_ports_with_vlan_data", return_value=[]), + patch.object(view, "_has_lag_signals", return_value=False), patch.object(view, "get_context_data", return_value={}), patch.object(view, "get_cache_key", return_value="cache-key") as mock_get_cache_key, patch.object(view, "get_last_fetched_key", return_value="last-key") as mock_get_last_fetched_key, @@ -2028,6 +2035,7 @@ def test_post_success_caches_and_renders(self): # Ports cache scopes to the VC sync device; pin it to obj so this caching test # isn't entangled with VC-routing (cache_device == obj for non-VC anyway). patch("netbox_librenms_plugin.views.base.interfaces_view.get_librenms_sync_device", return_value=obj), + patch("netbox_librenms_plugin.views.base.interfaces_view.get_librenms_oob", return_value=None), patch("netbox_librenms_plugin.views.base.interfaces_view.messages") as mock_messages, patch("netbox_librenms_plugin.views.mixins.render") as mock_render, patch("netbox_librenms_plugin.views.base.interfaces_view.cache") as mock_cache, diff --git a/netbox_librenms_plugin/tests/test_coverage_sync_interfaces.py b/netbox_librenms_plugin/tests/test_coverage_sync_interfaces.py index 8e9d655859..d8498d8e91 100644 --- a/netbox_librenms_plugin/tests/test_coverage_sync_interfaces.py +++ b/netbox_librenms_plugin/tests/test_coverage_sync_interfaces.py @@ -1138,3 +1138,75 @@ def test_syncs_matching_ports(self): # its coverage lives with the rebind seam in # test_coverage_sync_views.TestSyncInterfacesViewServerRebind # (test_posted_server_key_is_bound_for_the_sync / test_stale_server_key_fails_closed_without_sync). + +# =========================================================================== +# _resolve_interface_by_port_id: correct librenms_id dict lookup +# =========================================================================== + + +class TestResolveInterfaceByPortId: + """The function must correctly read the nested {'server_key': port_id} dict format.""" + + def test_finds_interface_by_server_keyed_dict(self): + """When librenms_id = {'production': 42}, resolves for port_id=42 and server_key='production'.""" + from unittest.mock import MagicMock, patch + from netbox_librenms_plugin.views.sync.interfaces import _resolve_interface_by_port_id + from dcim.models import Device, Interface + + mock_device = MagicMock(spec=Device) + mock_iface = MagicMock(spec=Interface) + + with ( + patch("netbox_librenms_plugin.views.sync.interfaces.Interface") as mock_intf_cls, + patch("netbox_librenms_plugin.views.sync.interfaces.get_librenms_device_id") as mock_get_id, + ): + mock_intf_cls.objects.filter.return_value = [mock_iface] + mock_get_id.return_value = 42 # correctly extracts 42 from {"production": 42} + + iface, err = _resolve_interface_by_port_id(mock_device, "42", "production") + + assert err is None + assert iface is mock_iface + mock_get_id.assert_called_once_with(mock_iface, "production", auto_save=False) + + def test_returns_error_when_not_found(self): + """Returns (None, error) when no interface has matching port_id.""" + from unittest.mock import MagicMock, patch + from netbox_librenms_plugin.views.sync.interfaces import _resolve_interface_by_port_id + from dcim.models import Device + + mock_device = MagicMock(spec=Device) + + with ( + patch("netbox_librenms_plugin.views.sync.interfaces.Interface") as mock_intf_cls, + patch("netbox_librenms_plugin.views.sync.interfaces.get_librenms_device_id", return_value=None), + ): + mock_intf_cls.objects.filter.return_value = [MagicMock()] + + iface, err = _resolve_interface_by_port_id(mock_device, "99", "production") + + assert iface is None + assert err is not None + + def test_name_hint_fallback_when_no_librenms_id(self): + """Falls back to name lookup when no interface has matching librenms_id.""" + from unittest.mock import MagicMock, patch + from netbox_librenms_plugin.views.sync.interfaces import _resolve_interface_by_port_id + from dcim.models import Device, Interface + + mock_device = MagicMock(spec=Device) + mock_device.virtual_chassis = None + mock_iface_by_name = MagicMock(spec=Interface) + + with ( + patch("netbox_librenms_plugin.views.sync.interfaces.Interface") as mock_intf_cls, + patch("netbox_librenms_plugin.views.sync.interfaces.get_librenms_device_id", return_value=None), + ): + mock_intf_cls.objects.filter.return_value = [] + mock_intf_cls.objects.get.return_value = mock_iface_by_name + + iface, err = _resolve_interface_by_port_id(mock_device, "42", "production", name_hint="lag-1") + + assert err is None + assert iface is mock_iface_by_name + mock_intf_cls.objects.get.assert_called_once_with(device=mock_device, name="lag-1") diff --git a/netbox_librenms_plugin/tests/test_coverage_tables.py b/netbox_librenms_plugin/tests/test_coverage_tables.py index 4ca1428d43..48df58e942 100644 --- a/netbox_librenms_plugin/tests/test_coverage_tables.py +++ b/netbox_librenms_plugin/tests/test_coverage_tables.py @@ -2254,76 +2254,6 @@ def test_disabled_up_matching_in_netbox(self): assert "text-success" in result -# =========================================================================== -# LibreNMSInterfaceTable render_librenms_id tests -# =========================================================================== - - -class TestRenderLibreNMSId: - """Tests for LibreNMSInterfaceTable.render_librenms_id().""" - - def _table(self, server_key="default"): - from netbox_librenms_plugin.tables.interfaces import LibreNMSInterfaceTable - - t = object.__new__(LibreNMSInterfaceTable) - t.server_key = server_key - return t - - def test_not_in_netbox_returns_danger(self): - table = self._table() - record = {"exists_in_netbox": False, "netbox_interface": None} - result = str(table.render_librenms_id(value=123, record=record)) - assert "text-danger" in result - assert "123" in result - - def test_no_netbox_interface_returns_danger(self): - table = self._table() - record = {"exists_in_netbox": True, "netbox_interface": None} - result = str(table.render_librenms_id(value=123, record=record)) - assert "text-danger" in result - - def test_netbox_librenms_id_is_none_returns_danger(self): - table = self._table() - nb_iface = MagicMock() - record = {"exists_in_netbox": True, "netbox_interface": nb_iface} - - with patch( - "netbox_librenms_plugin.tables.interfaces.get_librenms_device_id", - return_value=None, - ): - result = str(table.render_librenms_id(value=456, record=record)) - - assert "text-danger" in result - assert "No librenms_id" in result - - def test_ids_match_returns_success(self): - table = self._table() - nb_iface = MagicMock() - record = {"exists_in_netbox": True, "netbox_interface": nb_iface} - - with patch( - "netbox_librenms_plugin.tables.interfaces.get_librenms_device_id", - return_value=42, - ): - result = str(table.render_librenms_id(value=42, record=record)) - - assert "text-success" in result - - def test_ids_mismatch_returns_warning(self): - table = self._table() - nb_iface = MagicMock() - record = {"exists_in_netbox": True, "netbox_interface": nb_iface} - - with patch( - "netbox_librenms_plugin.tables.interfaces.get_librenms_device_id", - return_value=99, - ): - result = str(table.render_librenms_id(value=42, record=record)) - - assert "text-warning" in result - assert "Existing LibreNMS ID: 99" in result - - # =========================================================================== # LibreNMSInterfaceTable._compare_mac_addresses tests # =========================================================================== diff --git a/netbox_librenms_plugin/tests/test_librenms_api.py b/netbox_librenms_plugin/tests/test_librenms_api.py index 09ea26a5dd..ae304ea24d 100644 --- a/netbox_librenms_plugin/tests/test_librenms_api.py +++ b/netbox_librenms_plugin/tests/test_librenms_api.py @@ -10,9 +10,6 @@ import pytest import requests -# Import the autouse fixture from helpers -pytest_plugins = ["netbox_librenms_plugin.tests.test_librenms_api_helpers"] - # ============================================================================= # Test Class 1: Initialization (3 tests) @@ -2084,3 +2081,200 @@ def test_request_exception_returns_failure(self, mock_get, mock_librenms_config) assert success is False assert "net down" in msg + + +# ============================================================================= +# Test Class: get_port_stack() (3 tests) +# ============================================================================= + + +class TestGetPortStack: + """Tests for LibreNMSAPI.get_port_stack().""" + + def test_returns_mappings_list_on_success(self, mock_librenms_api): + """get_port_stack returns (True, list) on HTTP 200.""" + from unittest.mock import MagicMock, patch + + fake_response = MagicMock() + fake_response.json.return_value = { + "status": "ok", + "mappings": [ + {"high_port_id": 1, "low_port_id": 2, "high_ifIndex": 1, "low_ifIndex": 2}, + ], + } + fake_response.raise_for_status = MagicMock() + with patch("netbox_librenms_plugin.librenms_api.requests.get", return_value=fake_response) as mock_get: + success, data = mock_librenms_api.get_port_stack(42) + + assert success is True + assert data == [{"high_port_id": 1, "low_port_id": 2, "high_ifIndex": 1, "low_ifIndex": 2}] + mock_get.assert_called_once() + call_url = mock_get.call_args[0][0] + assert "/api/v0/devices/42/port_stack" in call_url + + def test_returns_false_on_404(self, mock_librenms_api): + """get_port_stack returns (False, error_str) when device not found.""" + import requests as _requests + from unittest.mock import MagicMock, patch + + fake_resp = MagicMock() + fake_resp.status_code = 404 + http_error = _requests.exceptions.HTTPError(response=fake_resp) + fake_resp.raise_for_status.side_effect = http_error + with patch("netbox_librenms_plugin.librenms_api.requests.get", return_value=fake_resp): + success, data = mock_librenms_api.get_port_stack(99) + + assert success is False + assert "not found" in data.lower() + + def test_returns_empty_list_when_no_mappings_key(self, mock_librenms_api): + """get_port_stack returns (True, []) if API response omits 'mappings' key.""" + from unittest.mock import MagicMock, patch + + fake_response = MagicMock() + fake_response.json.return_value = {"status": "ok"} + fake_response.raise_for_status = MagicMock() + with patch("netbox_librenms_plugin.librenms_api.requests.get", return_value=fake_response): + success, data = mock_librenms_api.get_port_stack(5) + + assert success is True + assert data == [] + + +# ============================================================================= +# Fixture port data for resolve_port_relationships tests +# ============================================================================= + +# Fixture port data for resolve_port_relationships tests +NOKIA_PORTS = [ + {"port_id": 101, "ifName": "1/1/c1/1", "ifType": "ethernetCsmacd"}, + {"port_id": 102, "ifName": "lag-1", "ifType": "ieee8023adLag"}, +] +NOKIA_PORT_STACK = [ + {"high_port_id": 101, "low_port_id": 102}, # valid LAG membership + {"high_port_id": 102, "low_port_id": 200}, # low_id 200 not in ports (missing = skip) +] +NOKIA_SAP_PORTS = [ + {"port_id": 101, "ifName": "1/1/c1/1", "ifType": "ethernetCsmacd"}, + {"port_id": 102, "ifName": "lag-1", "ifType": "ieee8023adLag"}, + {"port_id": 200, "ifName": "lag1:0", "ifType": "ipForward"}, # SAP entry with colon +] +NOKIA_SAP_PORT_STACK = [ + {"high_port_id": 101, "low_port_id": 102}, # valid LAG + {"high_port_id": 102, "low_port_id": 200}, # SAP — should be excluded +] +JUNOS_PORTS = [ + {"port_id": 201, "ifName": "xe-0/0/0", "ifType": "ethernetCsmacd"}, + {"port_id": 202, "ifName": "xe-0/0/0.0", "ifType": "propVirtual"}, + {"port_id": 203, "ifName": "ae1", "ifType": "ieee8023adLag"}, + {"port_id": 204, "ifName": "ae1.0", "ifType": "ieee8023adLag"}, + {"port_id": 205, "ifName": "ae10", "ifType": "ieee8023adLag"}, + {"port_id": 206, "ifName": "ae10.2221", "ifType": "l2vlan"}, +] +JUNOS_PORT_STACK = [ + {"high_port_id": 202, "low_port_id": 204}, # xe-0/0/0.0 -> ae1.0 resolves to xe-0/0/0 in ae1 + {"high_port_id": 205, "low_port_id": 206}, # ae10 -> ae10.2221 (sub-interface) +] +CISCO_IOS_PORTS = [ + {"port_id": 301, "ifName": "Te1/1", "ifType": "ethernetCsmacd"}, + {"port_id": 302, "ifName": "Po10", "ifType": "propVirtual"}, # IOS port-channel + {"port_id": 303, "ifName": "Po10.100", "ifType": "l2vlan"}, +] +CISCO_IOS_PORT_STACK = [ + {"high_port_id": 301, "low_port_id": 302}, # LAG membership + {"high_port_id": 302, "low_port_id": 303}, # sub-interface +] +ARCOS_PORTS = [ + {"port_id": 401, "ifName": "swp4", "ifType": "ethernetCsmacd"}, + {"port_id": 402, "ifName": "bond1", "ifType": "ieee8023adLag"}, + {"port_id": 403, "ifName": "swp15", "ifType": "ethernetCsmacd"}, + {"port_id": 404, "ifName": "swp15.3", "ifType": "ethernetCsmacd"}, # sub-if, not propVirtual +] +ARCOS_PORT_STACK = [ + {"high_port_id": 401, "low_port_id": 402}, # LAG membership + {"high_port_id": 403, "low_port_id": 404}, # sub-interface +] + + +# ============================================================================= +# Test Class: resolve_port_relationships() (10 tests) +# ============================================================================= + + +@pytest.fixture +def ios_lag_patterns(): + """LAG patterns dict for Cisco IOS (propVirtual LAGs identified by name).""" + return {"ios": r"^Po\d+$"} + + +@pytest.fixture +def arcos_lag_patterns(): + """LAG patterns dict for ArcOS bonds.""" + return {"arcos": r"^bond\d+$"} + + +@pytest.fixture +def combined_lag_patterns(): + """LAG patterns for both Cisco IOS and ArcOS.""" + return {"ios": r"^Po\d+$", "arcos": r"^bond\d+$"} + + +class TestResolvePortRelationships: + """Tests for LibreNMSAPI.resolve_port_relationships().""" + + def test_nokia_lag_membership(self, mock_librenms_api): + """Nokia: high=physical, low=lag-1 (ieee8023adLag) -> member in lag_members.""" + result = mock_librenms_api.resolve_port_relationships(NOKIA_PORTS, NOKIA_PORT_STACK[:1], lag_patterns={}) + assert result["lag_members"] == {101: 102} + assert result["sub_interfaces"] == {} + + def test_nokia_sap_excluded_when_colon_in_name(self, mock_librenms_api): + """Nokia SAP entries (colon in name) must be excluded from output.""" + result = mock_librenms_api.resolve_port_relationships(NOKIA_SAP_PORTS, NOKIA_SAP_PORT_STACK, lag_patterns={}) + assert result["lag_members"] == {101: 102} + assert 200 not in result["lag_members"].values() + + def test_junos_sub_unit_stripping(self, mock_librenms_api): + """Junos: xe-0/0/0.0 -> ae1.0 pair strips to xe-0/0/0 member of ae1.""" + result = mock_librenms_api.resolve_port_relationships(JUNOS_PORTS, JUNOS_PORT_STACK[:1], lag_patterns={}) + assert result["lag_members"].get(201) == 203 + + def test_junos_ae_sub_interface(self, mock_librenms_api): + """Junos: ae10 -> ae10.2221 detected as sub-interface.""" + result = mock_librenms_api.resolve_port_relationships(JUNOS_PORTS, JUNOS_PORT_STACK[1:], lag_patterns={}) + assert result["sub_interfaces"] == {206: 205} + + def test_cisco_ios_lag_via_name_pattern(self, mock_librenms_api, ios_lag_patterns): + """Cisco IOS: Po10 has propVirtual type but is a LAG via name pattern.""" + result = mock_librenms_api.resolve_port_relationships( + CISCO_IOS_PORTS, CISCO_IOS_PORT_STACK[:1], lag_patterns=ios_lag_patterns + ) + assert result["lag_members"] == {301: 302} + + def test_cisco_ios_sub_interface(self, mock_librenms_api, ios_lag_patterns): + """Cisco IOS: Po10 -> Po10.100 detected as sub-interface.""" + result = mock_librenms_api.resolve_port_relationships( + CISCO_IOS_PORTS, CISCO_IOS_PORT_STACK[1:], lag_patterns=ios_lag_patterns + ) + assert result["sub_interfaces"] == {303: 302} + + def test_arcos_lag_membership(self, mock_librenms_api): + """ArcOS: swp4 member of bond1 (ieee8023adLag).""" + result = mock_librenms_api.resolve_port_relationships(ARCOS_PORTS, ARCOS_PORT_STACK[:1], lag_patterns={}) + assert result["lag_members"] == {401: 402} + + def test_arcos_sub_interface_ethernetcsmacd(self, mock_librenms_api): + """ArcOS: swp15.3 sub-interface of swp15 (both ethernetCsmacd -- not propVirtual).""" + result = mock_librenms_api.resolve_port_relationships(ARCOS_PORTS, ARCOS_PORT_STACK[1:], lag_patterns={}) + assert result["sub_interfaces"] == {404: 403} + + def test_empty_port_stack_returns_empty_maps(self, mock_librenms_api): + """Empty port_stack returns empty dicts.""" + result = mock_librenms_api.resolve_port_relationships(NOKIA_PORTS, [], lag_patterns={}) + assert result == {"lag_members": {}, "sub_interfaces": {}} + + def test_missing_port_ids_are_skipped(self, mock_librenms_api): + """Entries where high_port_id or low_port_id is absent from ports list are skipped.""" + stack = [{"high_port_id": 9999, "low_port_id": 101}] + result = mock_librenms_api.resolve_port_relationships(NOKIA_PORTS, stack, lag_patterns={}) + assert result["lag_members"] == {} diff --git a/netbox_librenms_plugin/tests/test_port_stack_lag_pattern.py b/netbox_librenms_plugin/tests/test_port_stack_lag_pattern.py new file mode 100644 index 0000000000..92bf34094d --- /dev/null +++ b/netbox_librenms_plugin/tests/test_port_stack_lag_pattern.py @@ -0,0 +1,58 @@ +"""Tests for PortStackLagPattern model.""" + +import pytest +from django.core.exceptions import ValidationError +from unittest.mock import patch + + +class TestPortStackLagPattern: + def _make(self, librenms_os="ios", lag_name_pattern=r"^Po\d+$"): + from netbox_librenms_plugin.models import PortStackLagPattern + + obj = PortStackLagPattern.__new__(PortStackLagPattern) + obj.librenms_os = librenms_os + obj.lag_name_pattern = lag_name_pattern + obj.description = "" + return obj + + def test_str_representation(self): + obj = self._make(librenms_os="ios", lag_name_pattern=r"^Po\d+$") + assert str(obj) == r"ios -> ^Po\d+$" + + def test_clean_rejects_invalid_regex(self): + obj = self._make(lag_name_pattern="[invalid(regex") + with patch("netbox.models.NetBoxModel.clean"): + with pytest.raises(ValidationError) as exc_info: + obj.clean() + assert "lag_name_pattern" in exc_info.value.message_dict + + def test_clean_accepts_valid_regex(self): + obj = self._make() + with patch("netbox.models.NetBoxModel.clean"): + obj.clean() # should not raise + + def test_clean_rejects_blank_os(self): + obj = self._make(librenms_os="") + with patch("netbox.models.NetBoxModel.clean"): + with pytest.raises(ValidationError) as exc_info: + obj.clean() + assert "librenms_os" in exc_info.value.message_dict + + def test_clean_rejects_blank_pattern(self): + obj = self._make(lag_name_pattern="") + with patch("netbox.models.NetBoxModel.clean"): + with pytest.raises(ValidationError) as exc_info: + obj.clean() + assert "lag_name_pattern" in exc_info.value.message_dict + + def test_clean_normalizes_os(self): + obj = self._make(librenms_os=" IOS ") + with patch("netbox.models.NetBoxModel.clean"): + obj.clean() + assert obj.librenms_os == "ios" + + def test_clean_normalizes_lag_pattern(self): + obj = self._make(lag_name_pattern=r" ^Po\d+$ ") + with patch("netbox.models.NetBoxModel.clean"): + obj.clean() + assert obj.lag_name_pattern == r"^Po\d+$" diff --git a/netbox_librenms_plugin/tests/test_utils.py b/netbox_librenms_plugin/tests/test_utils.py index f6a8ee8b9a..e5b261f686 100644 --- a/netbox_librenms_plugin/tests/test_utils.py +++ b/netbox_librenms_plugin/tests/test_utils.py @@ -805,6 +805,7 @@ def test_get_table_paginate_count_default(self, mock_netbox_paginate, mock_confi mock_netbox_paginate.return_value = 25 mock_request = MagicMock() mock_request.GET = {} + mock_request.POST = {} result = get_table_paginate_count(mock_request, "table1_") diff --git a/netbox_librenms_plugin/urls.py b/netbox_librenms_plugin/urls.py index 2b1d4d05a9..6a69413474 100644 --- a/netbox_librenms_plugin/urls.py +++ b/netbox_librenms_plugin/urls.py @@ -9,6 +9,7 @@ ModuleTypeMapping, NormalizationRule, PlatformMapping, + PortStackLagPattern, ) from .views import ( AddDeviceToLibreNMSView, @@ -123,6 +124,15 @@ PlatformMappingEditView, PlatformMappingListView, PlatformMappingView, + PortStackLagPatternBulkDeleteView, + PortStackLagPatternBulkExportYAMLView, + PortStackLagPatternBulkImportView, + PortStackLagPatternChangeLogView, + PortStackLagPatternCreateView, + PortStackLagPatternDeleteView, + PortStackLagPatternEditView, + PortStackLagPatternListView, + PortStackLagPatternView, RemoveServerMappingView, SaveUserPrefView, SingleCableVerifyView, @@ -134,6 +144,8 @@ VerifyVlanSyncGroupView, SyncCablesView, SyncInterfacesView, + SyncInterfaceLagView, + SyncInterfaceParentView, SyncIPAddressesView, SyncSiteLocationView, SyncVLANsView, @@ -289,6 +301,18 @@ SyncInterfacesView.as_view(), name="sync_selected_interfaces", ), + # Sync interface LAG membership URL + path( + "//sync-interface-lag/", + SyncInterfaceLagView.as_view(), + name="sync_interface_lag", + ), + # Sync interface parent (sub-interface) URL + path( + "//sync-interface-parent/", + SyncInterfaceParentView.as_view(), + name="sync_interface_parent", + ), # Delete NetBox-only interfaces URL path( "//delete-netbox-interfaces/", @@ -881,5 +905,52 @@ CarrierAutoInstallRuleBulkExportYAMLView.as_view(), name="carrierautoinstallrule_bulk_export_yaml", ), + # PortStackLagPattern + path( + "port-stack-lag-patterns/", + PortStackLagPatternListView.as_view(), + name="portstacklagpattern_list", + ), + path( + "port-stack-lag-patterns//", + PortStackLagPatternView.as_view(), + name="portstacklagpattern_detail", + ), + path( + "port-stack-lag-patterns/add/", + PortStackLagPatternCreateView.as_view(), + name="portstacklagpattern_add", + ), + path( + "port-stack-lag-patterns/import/", + PortStackLagPatternBulkImportView.as_view(), + name="portstacklagpattern_bulk_import", + ), + path( + "port-stack-lag-patterns//edit/", + PortStackLagPatternEditView.as_view(), + name="portstacklagpattern_edit", + ), + path( + "port-stack-lag-patterns//delete/", + PortStackLagPatternDeleteView.as_view(), + name="portstacklagpattern_delete", + ), + path( + "port-stack-lag-patterns/delete/", + PortStackLagPatternBulkDeleteView.as_view(), + name="portstacklagpattern_bulk_delete", + ), + path( + "port-stack-lag-patterns/export-yaml/", + PortStackLagPatternBulkExportYAMLView.as_view(), + name="portstacklagpattern_bulk_export_yaml", + ), + path( + "port-stack-lag-patterns//changelog/", + PortStackLagPatternChangeLogView.as_view(), + name="portstacklagpattern_changelog", + kwargs={"model": PortStackLagPattern}, + ), path("api/", include("netbox_librenms_plugin.api.urls")), ] diff --git a/netbox_librenms_plugin/utils.py b/netbox_librenms_plugin/utils.py index 5b48037996..78e10dfcc9 100644 --- a/netbox_librenms_plugin/utils.py +++ b/netbox_librenms_plugin/utils.py @@ -657,9 +657,12 @@ def get_table_paginate_count(request: HttpRequest, table_prefix: str) -> int: int: Number of items to display per page """ config = get_config() - if f"{table_prefix}per_page" in request.GET: + # Check GET first, then POST (HTMX refresh requests send pagination via POST body) + param_key = f"{table_prefix}per_page" + param_value = request.GET.get(param_key) or request.POST.get(param_key) + if param_value: try: - per_page = int(request.GET.get(f"{table_prefix}per_page")) + per_page = int(param_value) max_page_size = config.MAX_PAGE_SIZE # MAX_PAGE_SIZE 0/None disables the NetBox ceiling; don't clamp to it (min() with 0 # would zero the page size, and with None it TypeErrors). diff --git a/netbox_librenms_plugin/views/__init__.py b/netbox_librenms_plugin/views/__init__.py index 5f1462a3c1..c1733d4b6e 100644 --- a/netbox_librenms_plugin/views/__init__.py +++ b/netbox_librenms_plugin/views/__init__.py @@ -109,6 +109,15 @@ PlatformMappingEditView, PlatformMappingListView, PlatformMappingView, + PortStackLagPatternBulkDeleteView, + PortStackLagPatternBulkExportYAMLView, + PortStackLagPatternBulkImportView, + PortStackLagPatternChangeLogView, + PortStackLagPatternCreateView, + PortStackLagPatternDeleteView, + PortStackLagPatternEditView, + PortStackLagPatternListView, + PortStackLagPatternView, ) from .imports.actions import ( # noqa: F401 AddAsOOBView, @@ -147,7 +156,12 @@ UpdateDeviceTypeView, ) from .sync.devices import AddDeviceToLibreNMSView, UpdateDeviceLocationView # noqa: F401 -from .sync.interfaces import DeleteNetBoxInterfacesView, SyncInterfacesView # noqa: F401 +from .sync.interfaces import ( + DeleteNetBoxInterfacesView, + SyncInterfacesView, + SyncInterfaceLagView, + SyncInterfaceParentView, +) # noqa: F401 from .sync.ip_addresses import SyncIPAddressesView # noqa: F401 from .sync.locations import SyncSiteLocationView # noqa: F401 from .sync.migrate import ( # noqa: F401 diff --git a/netbox_librenms_plugin/views/base/interfaces_view.py b/netbox_librenms_plugin/views/base/interfaces_view.py index 1409066338..c75999f221 100644 --- a/netbox_librenms_plugin/views/base/interfaces_view.py +++ b/netbox_librenms_plugin/views/base/interfaces_view.py @@ -9,6 +9,7 @@ cache_remaining_ttl, coerce_librenms_id, get_interface_name_field, + get_librenms_device_id, get_librenms_oob, get_librenms_sync_device, get_virtual_chassis_member, @@ -322,6 +323,16 @@ def _normalized_mac(port): "showing host interfaces only. See server logs for details.", ) oob_ports_failed = True + # Lazy port_stack fetch — only when device has LAG or sub-interface relationships. + # Enriches the host ports we fetched regardless of OOB outcome (it's independent of + # the OOB controller); the oob_incomplete tagging below still applies on an OOB failure. + all_ports_final = librenms_data.get("ports", []) + if self._has_lag_signals(all_ports_final): + ps_success, ps_data = self.librenms_api.get_port_stack(self.librenms_id) + if ps_success: + relationships = self.librenms_api.resolve_port_relationships(all_ports_final, ps_data) + librenms_data["port_stack_relationships"] = relationships + # On an OOB-ports fetch failure the snapshot is host-only. Rather than dropping it # (which would leave downstream views — SingleInterfaceVerifyView, # SaveVlanGroupOverridesView — with no backing snapshot), tag it `oob_incomplete` @@ -495,6 +506,15 @@ def get_context_data(self, request, obj, interface_name_field, server_key=None, ports_data = [] matched_interface_ids = set() + # Build port_stack relationship maps from cached data + port_stack_relationships = cached_data.get("port_stack_relationships", {}) + lag_members = port_stack_relationships.get("lag_members", {}) + sub_interfaces = port_stack_relationships.get("sub_interfaces", {}) + by_port_id = {p["port_id"]: p for p in ports_data if p.get("port_id")} + + # For device interfaces (not VMs), also select lag and parent FKs + _extra_related = [] if self.get_select_related_field(obj) == "virtual_machine" else ["lag", "parent"] + # Pre-fetch all interfaces for all potential chassis members interfaces_by_device = {} if hasattr(obj, "virtual_chassis") and obj.virtual_chassis: @@ -554,6 +574,11 @@ def get_context_data(self, request, obj, interface_name_field, server_key=None, # Add missing VLANs info for warning display self._add_missing_vlans_info(port, lookup_maps) + # Enrich port with LAG/parent relationship context + self._enrich_port_with_lag_parent( + port, lag_members, sub_interfaces, by_port_id, interface_name_field, server_key or "" + ) + table = self.get_table(ports_data, obj, interface_name_field, vlan_groups=vlan_groups) table.configure(request) @@ -732,3 +757,129 @@ def _add_missing_vlans_info(self, port, lookup_maps): missing_vlans.append(vid) port["missing_vlans"] = missing_vlans + + def _has_lag_signals(self, ports: list) -> bool: + """Return True if any port appears to be a LAG interface or sub-interface. + + Triggers lazy port_stack API fetch only when needed. Checks: + - ifType == 'ieee8023adLag' (definitive) + - ifType == 'propVirtual' (Cisco IOS port-channels / Junos sub-units) + - Name matches any PortStackLagPattern regex + - Any port name ends with '.' AND the base name also exists + (sub-interface detection, e.g. ge-0/0/0.100 with ge-0/0/0 present) + """ + import re as _re + + from netbox_librenms_plugin.models import PortStackLagPattern + + lag_patterns = [] + for pat_obj in PortStackLagPattern.objects.all(): + try: + lag_patterns.append(_re.compile(pat_obj.lag_name_pattern)) + except _re.error: + pass + + port_names = {p.get("ifName", "") for p in ports if p.get("ifName")} + sub_iface_re = _re.compile(r"^(.+)\.\d+$") + + for port in ports: + if_type = port.get("ifType", "") + if if_type in ("ieee8023adLag", "propVirtual"): + return True + name = port.get("ifName", "") + if any(pat.search(name) for pat in lag_patterns): + return True + # Sub-interface: name ends with '.' and parent name also present + m = sub_iface_re.match(name) + if m and m.group(1) in port_names: + return True + return False + + def _enrich_port_with_lag_parent( + self, + port: dict, + port_id_to_lag: dict, + port_id_to_parent: dict, + by_id: dict, + interface_name_field: str = "ifName", + server_key: str = "", + ) -> None: + """Add LAG/parent context keys to a port dict in-place. + + Sets six keys on the port dict: + port['librenms_lag_name'] -- name of LAG aggregate in LibreNMS, or None + port['librenms_lag_port_id'] -- port_id of LAG aggregate in LibreNMS, or None + port['lag_sync_status'] -- 'match'|'mismatch'|'missing_nb'|'missing_lnms'|None + port['librenms_parent_name'] -- name of parent interface in LibreNMS, or None + port['librenms_parent_port_id'] -- port_id of parent interface in LibreNMS, or None + port['parent_sync_status'] -- same values as lag_sync_status + + Matching strategy (most-to-least reliable): + 1. librenms_id stored on the NetBox related interface equals the LibreNMS port_id + 2. NetBox interface name matches the LibreNMS ifName field + 3. NetBox interface name matches the LibreNMS ifDescr field + """ + port_id = port.get("port_id") + nb_iface = port.get("netbox_interface") + + def _related_iface_matches(nb_rel_iface, lnms_port_dict) -> bool: + """Return True if nb_rel_iface corresponds to lnms_port_dict.""" + if nb_rel_iface is None or lnms_port_dict is None: + return False + # Primary: stored librenms_id (port_id) comparison — field-name-independent + if server_key: + stored_id = get_librenms_device_id(nb_rel_iface, server_key, auto_save=False) + lnms_pid = lnms_port_dict.get("port_id") + if stored_id is not None and lnms_pid is not None: + target = int(lnms_pid) if str(lnms_pid).isdigit() else None + if target is not None: + return stored_id == target + # Fallback: name match — try both name fields to be field-agnostic + nb_name = nb_rel_iface.name + return nb_name == lnms_port_dict.get("ifName") or nb_name == lnms_port_dict.get("ifDescr") + + # --- LAG --- + lnms_lag_port_id = port_id_to_lag.get(port_id) if port_id else None + agg_port = by_id.get(lnms_lag_port_id) if lnms_lag_port_id else None + lnms_lag_name = agg_port.get(interface_name_field) if agg_port else None + + port["librenms_lag_name"] = lnms_lag_name + port["librenms_lag_port_id"] = lnms_lag_port_id + + nb_lag = getattr(nb_iface, "lag", None) if nb_iface else None + if lnms_lag_port_id and nb_iface: + if nb_lag and _related_iface_matches(nb_lag, agg_port): + port["lag_sync_status"] = "match" + elif nb_lag: + port["lag_sync_status"] = "mismatch" + else: + port["lag_sync_status"] = "missing_nb" + elif lnms_lag_port_id and not nb_iface: + port["lag_sync_status"] = "missing_nb" + elif not lnms_lag_port_id and nb_lag: + port["lag_sync_status"] = "missing_lnms" + else: + port["lag_sync_status"] = None + + # --- Parent --- + lnms_parent_port_id = port_id_to_parent.get(port_id) if port_id else None + parent_port = by_id.get(lnms_parent_port_id) if lnms_parent_port_id else None + lnms_parent_name = parent_port.get(interface_name_field) if parent_port else None + + port["librenms_parent_name"] = lnms_parent_name + port["librenms_parent_port_id"] = lnms_parent_port_id + + nb_parent = getattr(nb_iface, "parent", None) if nb_iface else None + if lnms_parent_port_id and nb_iface: + if nb_parent and _related_iface_matches(nb_parent, parent_port): + port["parent_sync_status"] = "match" + elif nb_parent: + port["parent_sync_status"] = "mismatch" + else: + port["parent_sync_status"] = "missing_nb" + elif lnms_parent_port_id and not nb_iface: + port["parent_sync_status"] = "missing_nb" + elif not lnms_parent_port_id and nb_parent: + port["parent_sync_status"] = "missing_lnms" + else: + port["parent_sync_status"] = None diff --git a/netbox_librenms_plugin/views/mapping_views.py b/netbox_librenms_plugin/views/mapping_views.py index 60f792360e..3e92dd51ad 100644 --- a/netbox_librenms_plugin/views/mapping_views.py +++ b/netbox_librenms_plugin/views/mapping_views.py @@ -12,6 +12,7 @@ ModuleTypeMappingFilterSet, NormalizationRuleFilterSet, PlatformMappingFilterSet, + PortStackLagPatternFilterSet, ) from netbox_librenms_plugin.forms import ( CarrierAutoInstallRuleFilterForm, @@ -38,6 +39,9 @@ PlatformMappingFilterForm, PlatformMappingForm, PlatformMappingImportForm, + PortStackLagPatternFilterForm, + PortStackLagPatternForm, + PortStackLagPatternImportForm, ) from netbox_librenms_plugin.models import ( CarrierAutoInstallRule, @@ -48,6 +52,7 @@ ModuleTypeMapping, NormalizationRule, PlatformMapping, + PortStackLagPattern, ) from netbox_librenms_plugin.tables.mappings import ( CarrierAutoInstallRuleTable, @@ -58,6 +63,7 @@ ModuleTypeMappingTable, NormalizationRuleTable, PlatformMappingTable, + PortStackLagPatternTable, ) from netbox_librenms_plugin.views.mixins import ( LibreNMSPermissionMixin, @@ -622,3 +628,67 @@ class CarrierAutoInstallRuleChangeLogView(LibreNMSPermissionMixin, generic.Objec class CarrierAutoInstallRuleBulkExportYAMLView(BulkExportYAMLView): queryset = CarrierAutoInstallRule.objects.select_related("manufacturer", "carrier_module_type") + + +# --- PortStackLagPattern views --- + + +class PortStackLagPatternListView(LibreNMSPermissionMixin, generic.ObjectListView): + """Provides a view for listing all PortStackLagPattern objects.""" + + queryset = PortStackLagPattern.objects.all() + table = PortStackLagPatternTable + filterset = PortStackLagPatternFilterSet + filterset_form = PortStackLagPatternFilterForm + template_name = "netbox_librenms_plugin/portstacklagpattern_list.html" + + +class PortStackLagPatternCreateView(LibreNMSWritePermissionMixin, generic.ObjectEditView): + """Provides a view for creating a new PortStackLagPattern object.""" + + queryset = PortStackLagPattern.objects.all() + form = PortStackLagPatternForm + + +@register_model_view(PortStackLagPattern, "bulk_import", path="import", detail=False) +class PortStackLagPatternBulkImportView(LibreNMSWritePermissionMixin, generic.BulkImportView): + """Provides a view for bulk importing PortStackLagPattern objects from CSV/JSON/YAML.""" + + queryset = PortStackLagPattern.objects.all() + model_form = PortStackLagPatternImportForm + + +class PortStackLagPatternView(LibreNMSPermissionMixin, generic.ObjectView): + """Provides a view for displaying a PortStackLagPattern object.""" + + queryset = PortStackLagPattern.objects.all() + + +class PortStackLagPatternEditView(LibreNMSWritePermissionMixin, generic.ObjectEditView): + """Provides a view for editing a PortStackLagPattern object.""" + + queryset = PortStackLagPattern.objects.all() + form = PortStackLagPatternForm + + +class PortStackLagPatternDeleteView(LibreNMSWritePermissionMixin, generic.ObjectDeleteView): + """Provides a view for deleting a PortStackLagPattern object.""" + + queryset = PortStackLagPattern.objects.all() + + +class PortStackLagPatternBulkDeleteView(LibreNMSWritePermissionMixin, generic.BulkDeleteView): + """Provides a view for bulk deleting PortStackLagPattern objects.""" + + queryset = PortStackLagPattern.objects.all() + table = PortStackLagPatternTable + + +class PortStackLagPatternChangeLogView(LibreNMSPermissionMixin, generic.ObjectChangeLogView): + """Provides a view for displaying the changelog of a PortStackLagPattern object.""" + + queryset = PortStackLagPattern.objects.all() + + +class PortStackLagPatternBulkExportYAMLView(BulkExportYAMLView): + queryset = PortStackLagPattern.objects.all() diff --git a/netbox_librenms_plugin/views/sync/interfaces.py b/netbox_librenms_plugin/views/sync/interfaces.py index a09c066eb5..90b6fe41a1 100644 --- a/netbox_librenms_plugin/views/sync/interfaces.py +++ b/netbox_librenms_plugin/views/sync/interfaces.py @@ -112,6 +112,10 @@ def post(self, request, object_type, object_id): self._skipped_conflicts = [] self.sync_selected_interfaces(obj, selected_interfaces, ports_data, exclude_columns, interface_name_field) + # After all interfaces are created/updated, set LAG and parent relationships + relationships = self._get_cached_relationships(obj, server_key) + self._sync_lag_and_parent_relationships(obj, selected_interfaces, ports_data, relationships, server_key) + if self._skipped_conflicts: skipped = ", ".join(self._skipped_conflicts) messages.warning( @@ -170,6 +174,108 @@ def get_cached_ports_data(self, request, obj, server_key=None): return None return ports_data + def _get_cached_relationships(self, obj, server_key): + """Return port_stack_relationships from the cached port data, or empty dict.""" + cache_obj = obj + if isinstance(obj, Device) and not get_librenms_device_id(obj, server_key, auto_save=False): + sync_device = get_librenms_sync_device(obj, server_key=server_key) + if sync_device is not None: + cache_obj = sync_device + cached_data = cache.get(self.get_cache_key(cache_obj, "ports", server_key)) + if cached_data: + return cached_data.get("port_stack_relationships", {}) + return {} + + def _sync_lag_and_parent_relationships(self, obj, selected_interfaces, ports_data, relationships, server_key): + """Set LAG member and sub-interface parent relationships for synced interfaces. + + Runs after sync_selected_interfaces() so all interfaces already exist in NetBox. + Only processes relationships where this interface is a member/child — the + aggregate/parent may or may not be in the selected set (it just needs to exist in NB). + """ + if not relationships: + return + + lag_members = relationships.get("lag_members", {}) + sub_interfaces = relationships.get("sub_interfaces", {}) + if not lag_members and not sub_interfaces: + return + + interface_name_field = self.interface_name_field + + # Build lookups: str(port_id) -> port_dict, and interface_name -> str(port_id) + port_by_id = {} + port_id_by_name = {} + for port in ports_data: + pid = port.get("port_id") + name = port.get(interface_name_field) + if pid is not None: + port_by_id[str(pid)] = port + if name and pid is not None: + port_id_by_name[name] = str(pid) + + selected_set = set(selected_interfaces) + + with transaction.atomic(): + for iface_name in selected_set: + port_id = port_id_by_name.get(iface_name) + if not port_id: + continue + + # LAG membership: this interface is a member of a LAG aggregate + raw_lag = lag_members.get(port_id, lag_members.get(int(port_id) if port_id.isdigit() else None)) + if raw_lag is not None: + lag_port_id = str(raw_lag) + lag_entry = port_by_id.get(lag_port_id, {}) + lag_name = lag_entry.get("ifName", "") + + member_iface, err = _resolve_interface_by_port_id(obj, port_id, server_key) + if err: + logger.debug("LAG member lookup failed during bulk sync: %s", err) + continue + + if not isinstance(member_iface, Interface): + continue # VMInterface does not support lag + + agg_iface, err = _resolve_interface_by_port_id(obj, lag_port_id, server_key, name_hint=lag_name) + if err: + logger.debug("LAG aggregate lookup failed during bulk sync: %s", err) + continue + + if member_iface.lag_id != agg_iface.pk: + if isinstance(agg_iface, Interface) and agg_iface.type != "lag": + agg_iface.type = "lag" + agg_iface.save() + member_iface.lag = agg_iface + member_iface.save() + logger.info("Bulk sync: set %s.lag = %s", member_iface.name, agg_iface.name) + + # Sub-interface parent: this interface is a child of a parent interface + raw_parent = sub_interfaces.get( + port_id, sub_interfaces.get(int(port_id) if port_id.isdigit() else None) + ) + if raw_parent is not None: + parent_port_id = str(raw_parent) + parent_entry = port_by_id.get(parent_port_id, {}) + parent_name = parent_entry.get("ifName", "") + + child_iface, err = _resolve_interface_by_port_id(obj, port_id, server_key) + if err: + logger.debug("Sub-iface child lookup failed during bulk sync: %s", err) + continue + + parent_iface, err = _resolve_interface_by_port_id( + obj, parent_port_id, server_key, name_hint=parent_name + ) + if err: + logger.debug("Sub-iface parent lookup failed during bulk sync: %s", err) + continue + + if child_iface.parent_id != parent_iface.pk: + child_iface.parent = parent_iface + child_iface.save() + logger.info("Bulk sync: set %s.parent = %s", child_iface.name, parent_iface.name) + def sync_selected_interfaces( self, obj, @@ -589,3 +695,156 @@ def post(self, request, object_type, object_id): response_data["message"] += f" with {len(errors)} error(s)" return JsonResponse(response_data) + + +def _resolve_interface_by_port_id(obj, port_id: str, server_key: str, name_hint: str = ""): + """Resolve a LibreNMS port_id to a NetBox Interface/VMInterface. + + 1. Searches obj's interfaces for one whose librenms_id custom field matches port_id. + For Devices in a Virtual Chassis, searches all VC member interfaces. + 2. Falls back to exact name match when name_hint is provided (e.g. interface was + created manually without a librenms_id). + Returns (interface, None) on success or (None, error_str) on failure. + """ + if not port_id: + return None, "port_id is required" + + if isinstance(obj, Device): + if hasattr(obj, "virtual_chassis") and obj.virtual_chassis: + member_ids = obj.virtual_chassis.members.values_list("id", flat=True) + iface_qs = Interface.objects.filter(device__in=member_ids) + else: + iface_qs = Interface.objects.filter(device=obj) + elif isinstance(obj, VirtualMachine): + iface_qs = VMInterface.objects.filter(virtual_machine=obj) + else: + return None, f"Unsupported object type: {type(obj).__name__}" + + target_id = int(port_id) if str(port_id).isdigit() else None + for iface in iface_qs: + stored_id = get_librenms_device_id(iface, server_key, auto_save=False) + if stored_id is not None and target_id is not None and stored_id == target_id: + return iface, None + + if name_hint: + try: + if isinstance(obj, Device): + if hasattr(obj, "virtual_chassis") and obj.virtual_chassis: + member_ids = obj.virtual_chassis.members.values_list("id", flat=True) + iface = Interface.objects.get(device__in=member_ids, name=name_hint) + else: + iface = Interface.objects.get(device=obj, name=name_hint) + else: + iface = VMInterface.objects.get(virtual_machine=obj, name=name_hint) + return iface, None + except Exception: + pass + + return None, f"Interface with LibreNMS port_id {port_id} not found on {obj}" + + +class _PortIdResolveMixin: + """Mixin to resolve a LibreNMS port_id to a NetBox interface by librenms_id custom field, then name fallback.""" + + _server_key: str + + def _resolve_interface_by_port_id(self, obj, port_id: str, server_key: str, name_hint: str = ""): + return _resolve_interface_by_port_id(obj, port_id, server_key, name_hint) + + +class SyncInterfaceLagView(LibreNMSPermissionMixin, NetBoxObjectPermissionMixin, LibreNMSAPIMixin, View): + """Set Interface.lag (member -> aggregate) based on LibreNMS port_stack data.""" + + required_object_permissions = {"POST": [("change", Interface)]} + + def _get_object(self, object_type, object_id): + if object_type == "device": + return get_object_or_404(Device, pk=object_id) + if object_type == "virtualmachine": + return get_object_or_404(VirtualMachine, pk=object_id) + raise Http404("Invalid object type.") + + def post(self, request, object_type, object_id): + if error := self.require_all_permissions("POST"): + return error + + obj = self._get_object(object_type, object_id) + server_key = request.POST.get("server_key") or self.librenms_api.server_key + port_id = request.POST.get("port_id", "").strip() + lag_port_id = request.POST.get("lag_port_id", "").strip() + lag_name = request.POST.get("lag_name", "").strip() + + if not port_id or not lag_port_id: + return JsonResponse({"error": "port_id and lag_port_id are required"}, status=400) + + member_iface, err = _PortIdResolveMixin._resolve_interface_by_port_id(self, obj, port_id, server_key) + if err: + return JsonResponse({"error": f"Member interface: {err}"}, status=404) + + agg_iface, err = _PortIdResolveMixin._resolve_interface_by_port_id( + self, obj, lag_port_id, server_key, name_hint=lag_name + ) + if err: + return JsonResponse({"error": f"Aggregate interface: {err}"}, status=404) + + with transaction.atomic(): + if not isinstance(member_iface, Interface): + return JsonResponse( + {"error": "LAG membership sync is only supported for device interfaces, not VM interfaces"}, + status=400, + ) + + if isinstance(agg_iface, Interface) and agg_iface.type != "lag": + agg_iface.type = "lag" + agg_iface.save() + logger.info("Set interface %s type=lag", agg_iface.name) + + member_iface.lag = agg_iface + member_iface.save() + logger.info("Set %s.lag = %s", member_iface.name, agg_iface.name) + + return JsonResponse({"status": "success", "message": f"Linked {member_iface.name} to LAG {agg_iface.name}"}) + + +class SyncInterfaceParentView(LibreNMSPermissionMixin, NetBoxObjectPermissionMixin, LibreNMSAPIMixin, View): + """Set Interface.parent (sub-interface -> parent) based on LibreNMS port_stack data.""" + + required_object_permissions = {"POST": [("change", Interface)]} + + def _get_object(self, object_type, object_id): + if object_type == "device": + return get_object_or_404(Device, pk=object_id) + if object_type == "virtualmachine": + return get_object_or_404(VirtualMachine, pk=object_id) + raise Http404("Invalid object type.") + + def post(self, request, object_type, object_id): + if error := self.require_all_permissions("POST"): + return error + + obj = self._get_object(object_type, object_id) + server_key = request.POST.get("server_key") or self.librenms_api.server_key + port_id = request.POST.get("port_id", "").strip() + parent_port_id = request.POST.get("parent_port_id", "").strip() + parent_name = request.POST.get("parent_name", "").strip() + + if not port_id or not parent_port_id: + return JsonResponse({"error": "port_id and parent_port_id are required"}, status=400) + + child_iface, err = _PortIdResolveMixin._resolve_interface_by_port_id(self, obj, port_id, server_key) + if err: + return JsonResponse({"error": f"Child interface: {err}"}, status=404) + + parent_iface, err = _PortIdResolveMixin._resolve_interface_by_port_id( + self, obj, parent_port_id, server_key, name_hint=parent_name + ) + if err: + return JsonResponse({"error": f"Parent interface: {err}"}, status=404) + + child_iface.parent = parent_iface + child_iface.save() + logger.info("Set %s.parent = %s", child_iface.name, parent_iface.name) + + return JsonResponse( + {"status": "success", "message": f"Linked {child_iface.name} to parent {parent_iface.name}"} + ) From eab0a147425967b802163b4ae14e7e81f155e2fe Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Mon, 8 Jun 2026 20:09:37 +0200 Subject: [PATCH 003/163] fix(pci): address CodeRabbit review (sub-interface notice XSS, sibling-aware cross-page parent cleanup, device-only LAG sync) --- .../js/librenms_sync.js | 37 +++++++++++++++---- .../views/sync/interfaces.py | 18 ++++----- 2 files changed, 38 insertions(+), 17 deletions(-) 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 c184cb96c4..843ee01258 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 @@ -444,11 +444,20 @@ document.addEventListener('change', function (e) { } // --- Sub-interface: remove cross-page hidden input when unchecking --- + // Only drop the injected parent input when NO other still-checked child on this + // page references the same cross-page parent — otherwise unchecking one sibling + // would strip the auto-included parent the remaining siblings still need. if (parentPortId && !checkbox.checked) { const form = checkbox.closest('form'); if (form) { - const hidden = form.querySelector('#auto-parent-' + parentPortId); - if (hidden) hidden.remove(); + const siblingStillChecked = Array.prototype.some.call( + document.querySelectorAll('tr[data-parent-port-id="' + parentPortId + '"] input[name="select"]'), + function (cb) { return cb !== checkbox && cb.checked; } + ); + if (!siblingStillChecked) { + const hidden = form.querySelector('#auto-parent-' + parentPortId); + if (hidden) hidden.remove(); + } } } @@ -482,11 +491,25 @@ function _showParentCrossPageNotice(parentName) { const notice = document.createElement('div'); notice.className = 'alert alert-info alert-dismissible py-1 px-2 small mb-1'; notice.dataset.parent = parentName; - notice.innerHTML = - '' + - 'Parent interface ' + parentName + ' is on another page ' + - 'and will be included in the sync automatically.' + - ''; + + // 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'); + notice.appendChild(closeBtn); + container.appendChild(notice); setTimeout(function () { diff --git a/netbox_librenms_plugin/views/sync/interfaces.py b/netbox_librenms_plugin/views/sync/interfaces.py index 90b6fe41a1..89038b2656 100644 --- a/netbox_librenms_plugin/views/sync/interfaces.py +++ b/netbox_librenms_plugin/views/sync/interfaces.py @@ -760,9 +760,11 @@ class SyncInterfaceLagView(LibreNMSPermissionMixin, NetBoxObjectPermissionMixin, def _get_object(self, object_type, object_id): if object_type == "device": return get_object_or_404(Device, pk=object_id) - if object_type == "virtualmachine": - return get_object_or_404(VirtualMachine, pk=object_id) - raise Http404("Invalid object type.") + # VMInterface has no `lag` field, so LAG membership sync is device-only. Reject + # VMs up front rather than resolving one and failing later — that path also ran a + # mismatched ("change", Interface) permission check. Keeps the view honestly + # device-only and consistent with required_object_permissions. + raise Http404("LAG membership sync is only supported for device interfaces.") def post(self, request, object_type, object_id): if error := self.require_all_permissions("POST"): @@ -788,13 +790,9 @@ def post(self, request, object_type, object_id): return JsonResponse({"error": f"Aggregate interface: {err}"}, status=404) with transaction.atomic(): - if not isinstance(member_iface, Interface): - return JsonResponse( - {"error": "LAG membership sync is only supported for device interfaces, not VM interfaces"}, - status=400, - ) - - if isinstance(agg_iface, Interface) and agg_iface.type != "lag": + # obj is always a Device here (VMs are 404'd above), so both resolved + # interfaces are Interface instances. + if agg_iface.type != "lag": agg_iface.type = "lag" agg_iface.save() logger.info("Set interface %s type=lag", agg_iface.name) From 245e47ef0c50e11052f2c27f58b8b1ad2774bdea Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 9 Jun 2026 16:58:35 +0200 Subject: [PATCH 004/163] fix(pci): refresh Parent/LAG cell on VC member switch; prefetch lag/parent FKs Wire the lag/parent select_related into _build_interface_lookup_maps (drops a dead _extra_related var / F841 and avoids N+1 on nb_iface.lag/.parent), recompute the relationship for the selected member in the single-interface verify path, and patch the parent cell client-side so it no longer shows the prior device's status. --- .../js/librenms_sync.js | 6 ++++++ netbox_librenms_plugin/tables/interfaces.py | 3 +++ .../views/base/interfaces_view.py | 20 ++++++++++++------- .../views/object_sync/devices.py | 15 ++++++++++++++ 4 files changed, 37 insertions(+), 7 deletions(-) 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 843ee01258..3ab307f5ab 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 @@ -1193,6 +1193,12 @@ function handleInterfaceChange(select, value) { row.querySelector('td[data-col="mtu"]').innerHTML = formattedRow.mtu; row.querySelector('td[data-col="enabled"]').innerHTML = formattedRow.enabled; row.querySelector('td[data-col="description"]').innerHTML = formattedRow.description; + // Parent/LAG relationship is device-specific, so refresh it too — otherwise + // it keeps the previously-selected member's status and sync button. + const parentCell = row.querySelector('td[data-col="parent"]'); + if (parentCell && typeof formattedRow.parent !== 'undefined') { + parentCell.innerHTML = formattedRow.parent; + } initializeFilters(); } }) diff --git a/netbox_librenms_plugin/tables/interfaces.py b/netbox_librenms_plugin/tables/interfaces.py index 8374beb3d8..20995ca620 100644 --- a/netbox_librenms_plugin/tables/interfaces.py +++ b/netbox_librenms_plugin/tables/interfaces.py @@ -607,6 +607,9 @@ 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), + # 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 diff --git a/netbox_librenms_plugin/views/base/interfaces_view.py b/netbox_librenms_plugin/views/base/interfaces_view.py index c75999f221..03eeb42fcb 100644 --- a/netbox_librenms_plugin/views/base/interfaces_view.py +++ b/netbox_librenms_plugin/views/base/interfaces_view.py @@ -116,17 +116,25 @@ def _get_object_librenms_id(self, obj): return normalize_librenms_port_id(librenms_id) def _build_interface_lookup_maps(self, obj): - """Build name and LibreNMS ID indexes, dropping conflicting IDs entirely.""" + """Build name and LibreNMS ID indexes, dropping conflicting IDs entirely. + + For device interfaces (not VMs), also select_related the lag/parent FKs so + _enrich_port_with_lag_parent() doesn't issue N+1 queries when reading + nb_iface.lag / nb_iface.parent for each port. VMInterface has no such FKs. + """ by_name = {} by_librenms_id = {} duplicate_librenms_ids = set() # Prefetch the M2M relations the table renderers dereference per matched row # (render_vlans -> tagged_vlans, render_mac_address -> mac_addresses); without this each - # rendered interface row issues its own query for these. + # rendered interface row issues its own query for these. Also select_related the lag/parent + # FKs (render_parent dereferences them) — skipped for VMs, which have no such fields. + related_field = self.get_select_related_field(obj) + extra_related = [] if related_field == "virtual_machine" else ["lag", "parent"] interfaces = ( self.get_interfaces(obj) - .select_related(self.get_select_related_field(obj)) + .select_related(related_field, *extra_related) .prefetch_related("tagged_vlans", "tagged_vlans__group", "mac_addresses") ) for interface in interfaces: @@ -512,10 +520,8 @@ def get_context_data(self, request, obj, interface_name_field, server_key=None, sub_interfaces = port_stack_relationships.get("sub_interfaces", {}) by_port_id = {p["port_id"]: p for p in ports_data if p.get("port_id")} - # For device interfaces (not VMs), also select lag and parent FKs - _extra_related = [] if self.get_select_related_field(obj) == "virtual_machine" else ["lag", "parent"] - # Pre-fetch all interfaces for all potential chassis members + # (_build_interface_lookup_maps select_relateds lag/parent for devices). interfaces_by_device = {} if hasattr(obj, "virtual_chassis") and obj.virtual_chassis: for member in obj.virtual_chassis.members.all(): @@ -795,8 +801,8 @@ def _has_lag_signals(self, ports: list) -> bool: return True return False + @staticmethod def _enrich_port_with_lag_parent( - self, port: dict, port_id_to_lag: dict, port_id_to_parent: dict, diff --git a/netbox_librenms_plugin/views/object_sync/devices.py b/netbox_librenms_plugin/views/object_sync/devices.py index 1097efb983..26ee648e11 100644 --- a/netbox_librenms_plugin/views/object_sync/devices.py +++ b/netbox_librenms_plugin/views/object_sync/devices.py @@ -183,6 +183,21 @@ def post(self, request): interface_name_field=interface_name_field, server_key=server_key, ) + # Recompute the LAG/parent relationship for the *selected* member so the + # Parent/LAG cell isn't left showing the previously-rendered device's status. + # netbox_interface must be set first — the enrichment reads it to compare + # NetBox lag/parent against LibreNMS. format_interface_data re-sets it (no-op). + port_data["netbox_interface"] = selected_device.interfaces.filter(name=interface_name).first() + relationships = cached_data.get("port_stack_relationships", {}) + by_port_id = {p["port_id"]: p for p in cached_data.get("ports", []) if p.get("port_id")} + BaseInterfaceTableView._enrich_port_with_lag_parent( + port_data, + relationships.get("lag_members", {}), + relationships.get("sub_interfaces", {}), + by_port_id, + interface_name_field, + server_key or "", + ) formatted_row = table.format_interface_data(port_data, selected_device) return JsonResponse({"status": "success", "formatted_row": formatted_row}) From c0206c797e16e1a7ba6201751971c03650d87222 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 9 Jun 2026 20:14:06 +0200 Subject: [PATCH 005/163] fix(pci): scope perms/lookups correctly for VM parent sync, OOB rows, paginate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - SyncInterfaceParentView: set POST permission by object_type — VMInterface has a parent field, so VM parent sync needs (change, VMInterface), not (change, Interface). - get_context_data: scope by_port_id to host rows (_source != oob) so an OOB controller reusing a host port_id can't attach the wrong aggregate/parent. - get_table_paginate_count: clamp non-positive per_page to the NetBox default. --- .../tests/test_coverage_sync_interfaces.py | 43 +++++++++++++++++++ .../tests/test_coverage_utils.py | 13 ++++++ netbox_librenms_plugin/utils.py | 3 ++ .../views/base/interfaces_view.py | 5 ++- .../views/sync/interfaces.py | 10 ++++- 5 files changed, 72 insertions(+), 2 deletions(-) diff --git a/netbox_librenms_plugin/tests/test_coverage_sync_interfaces.py b/netbox_librenms_plugin/tests/test_coverage_sync_interfaces.py index d8498d8e91..38eb0da641 100644 --- a/netbox_librenms_plugin/tests/test_coverage_sync_interfaces.py +++ b/netbox_librenms_plugin/tests/test_coverage_sync_interfaces.py @@ -76,6 +76,49 @@ def test_invalid_type_raises_http404(self): view.get_required_permissions_for_object_type("invalid") +class TestSyncInterfaceParentViewPermissions: + """SyncInterfaceParentView supports VMs (VMInterface has a parent field), so its + POST permission must be scoped to the object type, not hardcoded to Interface.""" + + def _stop_after_perms(self): + """Patch require_all_permissions to short-circuit post() right after the + dynamic permission dict is set, returning a sentinel response.""" + return patch.object( + __import__( + "netbox_librenms_plugin.views.sync.interfaces", fromlist=["SyncInterfaceParentView"] + ).SyncInterfaceParentView, + "require_all_permissions", + return_value=_denied_response(), + ) + + def test_device_post_requires_interface_change(self): + from netbox_librenms_plugin.views.sync.interfaces import SyncInterfaceParentView + from dcim.models import Interface + + view = object.__new__(SyncInterfaceParentView) + with self._stop_after_perms(): + view.post(_make_request(), "device", 1) + assert view.required_object_permissions["POST"] == [("change", Interface)] + + def test_vm_post_requires_vminterface_change(self): + from netbox_librenms_plugin.views.sync.interfaces import SyncInterfaceParentView + from virtualization.models import VMInterface + + view = object.__new__(SyncInterfaceParentView) + with self._stop_after_perms(): + view.post(_make_request(), "virtualmachine", 1) + assert view.required_object_permissions["POST"] == [("change", VMInterface)] + + def test_invalid_type_raises_http404(self): + from netbox_librenms_plugin.views.sync.interfaces import SyncInterfaceParentView + from django.http import Http404 + import pytest + + view = object.__new__(SyncInterfaceParentView) + with pytest.raises(Http404): + view.post(_make_request(), "invalid", 1) + + # =========================================================================== # SyncInterfacesView.get_object # =========================================================================== diff --git a/netbox_librenms_plugin/tests/test_coverage_utils.py b/netbox_librenms_plugin/tests/test_coverage_utils.py index 0edba1f61a..a0a4094d6e 100644 --- a/netbox_librenms_plugin/tests/test_coverage_utils.py +++ b/netbox_librenms_plugin/tests/test_coverage_utils.py @@ -279,6 +279,19 @@ def test_invalid_per_page_falls_back_to_default(self): result = get_table_paginate_count(request, "table_") assert result == 50 + def test_non_positive_per_page_falls_back_to_default(self): + """0 or negative input must not propagate to the paginator.""" + from netbox_librenms_plugin.utils import get_table_paginate_count + + for raw in ("0", "-5"): + request = MagicMock() + request.GET = {"table_per_page": raw} + with patch("netbox_librenms_plugin.utils.get_config"): + with patch("netbox_librenms_plugin.utils.netbox_get_paginate_count") as mock_paginate: + mock_paginate.return_value = 50 + result = get_table_paginate_count(request, "table_") + assert result == 50, f"per_page={raw!r} should fall back to default" + class TestGetUserPrefNoConfig: """Tests for get_user_pref when user has no config (line 179).""" diff --git a/netbox_librenms_plugin/utils.py b/netbox_librenms_plugin/utils.py index 78e10dfcc9..78dd40bf1e 100644 --- a/netbox_librenms_plugin/utils.py +++ b/netbox_librenms_plugin/utils.py @@ -663,6 +663,9 @@ def get_table_paginate_count(request: HttpRequest, table_prefix: str) -> int: if param_value: try: per_page = int(param_value) + # Guard against 0/negative input, which would break the paginator. + if per_page < 1: + return netbox_get_paginate_count(request) max_page_size = config.MAX_PAGE_SIZE # MAX_PAGE_SIZE 0/None disables the NetBox ceiling; don't clamp to it (min() with 0 # would zero the page size, and with None it TypeErrors). diff --git a/netbox_librenms_plugin/views/base/interfaces_view.py b/netbox_librenms_plugin/views/base/interfaces_view.py index 03eeb42fcb..4e9145527d 100644 --- a/netbox_librenms_plugin/views/base/interfaces_view.py +++ b/netbox_librenms_plugin/views/base/interfaces_view.py @@ -518,7 +518,10 @@ def get_context_data(self, request, obj, interface_name_field, server_key=None, port_stack_relationships = cached_data.get("port_stack_relationships", {}) lag_members = port_stack_relationships.get("lag_members", {}) sub_interfaces = port_stack_relationships.get("sub_interfaces", {}) - by_port_id = {p["port_id"]: p for p in ports_data if p.get("port_id")} + # Scope to host rows: ports_data is merged host + OOB, and an OOB controller + # can reuse a host port_id. Letting an OOB row win here would attach the wrong + # aggregate/parent to the host interface during lag/parent enrichment. + by_port_id = {p["port_id"]: p for p in ports_data if p.get("port_id") and p.get("_source") != "oob"} # Pre-fetch all interfaces for all potential chassis members # (_build_interface_lookup_maps select_relateds lag/parent for devices). diff --git a/netbox_librenms_plugin/views/sync/interfaces.py b/netbox_librenms_plugin/views/sync/interfaces.py index 89038b2656..f93156c19c 100644 --- a/netbox_librenms_plugin/views/sync/interfaces.py +++ b/netbox_librenms_plugin/views/sync/interfaces.py @@ -807,7 +807,8 @@ def post(self, request, object_type, object_id): class SyncInterfaceParentView(LibreNMSPermissionMixin, NetBoxObjectPermissionMixin, LibreNMSAPIMixin, View): """Set Interface.parent (sub-interface -> parent) based on LibreNMS port_stack data.""" - required_object_permissions = {"POST": [("change", Interface)]} + # Permissions are set dynamically in post() based on object_type — both Devices + # (Interface) and VMs (VMInterface, which also has a parent field) are supported. def _get_object(self, object_type, object_id): if object_type == "device": @@ -817,6 +818,13 @@ def _get_object(self, object_type, object_id): raise Http404("Invalid object type.") def post(self, request, object_type, object_id): + if object_type == "device": + self.required_object_permissions = {"POST": [("change", Interface)]} + elif object_type == "virtualmachine": + self.required_object_permissions = {"POST": [("change", VMInterface)]} + else: + raise Http404("Invalid object type.") + if error := self.require_all_permissions("POST"): return error From 7921d16b154ea036205fefcea624500d4c8f99c1 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 9 Jun 2026 21:00:14 +0200 Subject: [PATCH 006/163] fix(pci): scope LAG name-pattern matching to the device's LibreNMS OS resolve_port_relationships compiled and applied every stored PortStackLagPattern regardless of platform, so a vendor-specific regex could misclassify an interface on another device's OS as a LAG aggregate. Add a device_os argument that scopes the DB pattern lookup to that OS (filter on librenms_os); the interfaces view resolves the OS via get_device_info (best-effort) and passes it. device_os=None preserves the prior unscoped behaviour. --- netbox_librenms_plugin/librenms_api.py | 13 +++++-- .../tests/test_librenms_api.py | 34 +++++++++++++++++++ .../views/base/interfaces_view.py | 11 +++++- 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/netbox_librenms_plugin/librenms_api.py b/netbox_librenms_plugin/librenms_api.py index 954b80b754..f5f81329a5 100644 --- a/netbox_librenms_plugin/librenms_api.py +++ b/netbox_librenms_plugin/librenms_api.py @@ -582,6 +582,7 @@ def resolve_port_relationships( ports: list, port_stack: list, lag_patterns: dict | None = None, + device_os: str | None = None, ) -> dict: """ Resolve LAG membership and sub-interface parent relationships from LibreNMS data. @@ -602,7 +603,12 @@ def resolve_port_relationships( 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. + 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). Returns: dict with keys: @@ -614,7 +620,10 @@ def resolve_port_relationships( if lag_patterns is None: from netbox_librenms_plugin.models import PortStackLagPattern - lag_patterns = {p.librenms_os: p.lag_name_pattern for p in PortStackLagPattern.objects.all()} + queryset = PortStackLagPattern.objects.all() + if device_os: + queryset = queryset.filter(librenms_os__iexact=device_os.strip()) + lag_patterns = {p.librenms_os: p.lag_name_pattern for p in queryset} by_id = {p["port_id"]: p for p in ports if p.get("port_id")} by_name = {p["ifName"]: p for p in ports if p.get("ifName")} diff --git a/netbox_librenms_plugin/tests/test_librenms_api.py b/netbox_librenms_plugin/tests/test_librenms_api.py index ae304ea24d..c04a684075 100644 --- a/netbox_librenms_plugin/tests/test_librenms_api.py +++ b/netbox_librenms_plugin/tests/test_librenms_api.py @@ -2278,3 +2278,37 @@ def test_missing_port_ids_are_skipped(self, mock_librenms_api): stack = [{"high_port_id": 9999, "low_port_id": 101}] result = mock_librenms_api.resolve_port_relationships(NOKIA_PORTS, stack, lag_patterns={}) assert result["lag_members"] == {} + + @pytest.mark.django_db + def test_db_patterns_scoped_to_device_os(self, mock_librenms_api): + """With device_os set, only that OS's stored pattern is loaded — a pattern from a + different platform must not classify this device's interfaces (the round-12 finding). + + Uses test-unique OS names so the device_os filter excludes the migration-seeded + defaults (e.g. the real ``ios`` pattern, which is also ``^Po\\d+$``).""" + from netbox_librenms_plugin.models import PortStackLagPattern + + PortStackLagPattern.objects.create(librenms_os="ztest_pochannel", lag_name_pattern=r"^Po\d+$") + PortStackLagPattern.objects.create(librenms_os="ztest_bond", lag_name_pattern=r"^bond\d+$") + + # device_os matches the Po-channel pattern → Po10 (propVirtual) classified as a LAG. + scoped = mock_librenms_api.resolve_port_relationships( + CISCO_IOS_PORTS, CISCO_IOS_PORT_STACK[:1], device_os="ztest_pochannel" + ) + assert scoped["lag_members"] == {301: 302} + + # device_os scopes to the bond pattern only; Po10 is not matched → no LAG. + other = mock_librenms_api.resolve_port_relationships( + CISCO_IOS_PORTS, CISCO_IOS_PORT_STACK[:1], device_os="ztest_bond" + ) + assert other["lag_members"] == {} + + @pytest.mark.django_db + def test_db_patterns_unscoped_when_no_device_os(self, mock_librenms_api): + """Without device_os, every stored pattern is loaded (legacy behaviour).""" + from netbox_librenms_plugin.models import PortStackLagPattern + + PortStackLagPattern.objects.create(librenms_os="ztest_pochannel", lag_name_pattern=r"^Po\d+$") + + result = mock_librenms_api.resolve_port_relationships(CISCO_IOS_PORTS, CISCO_IOS_PORT_STACK[:1]) + assert result["lag_members"] == {301: 302} diff --git a/netbox_librenms_plugin/views/base/interfaces_view.py b/netbox_librenms_plugin/views/base/interfaces_view.py index 4e9145527d..228eb159e0 100644 --- a/netbox_librenms_plugin/views/base/interfaces_view.py +++ b/netbox_librenms_plugin/views/base/interfaces_view.py @@ -338,7 +338,16 @@ def _normalized_mac(port): if self._has_lag_signals(all_ports_final): ps_success, ps_data = self.librenms_api.get_port_stack(self.librenms_id) if ps_success: - relationships = self.librenms_api.resolve_port_relationships(all_ports_final, ps_data) + # Scope LAG name-pattern matching to this device's OS so a vendor-specific + # regex can't misclassify an interface on another platform. Best-effort: + # on a failed/odd device-info fetch we fall back to all patterns. + device_os = None + info_success, device_info = self.librenms_api.get_device_info(self.librenms_id) + if info_success and isinstance(device_info, dict): + device_os = device_info.get("os") + relationships = self.librenms_api.resolve_port_relationships( + all_ports_final, ps_data, device_os=device_os + ) librenms_data["port_stack_relationships"] = relationships # On an OOB-ports fetch failure the snapshot is host-only. Rather than dropping it From f55eeae640d48d085614a71b55d61a4dd2a7641b Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 9 Jun 2026 22:28:26 +0200 Subject: [PATCH 007/163] fix(pci): harden LAG/parent sync fetch, port_stack, VC target, message peek - librenms_sync.js: check response.ok on the LAG/parent sync fetch so backend error text surfaces instead of a generic JSON parse failure. - get_port_stack: normalize a null 'mappings' body to [] so resolve_port_ relationships never iterates a None port_stack. - _render_relationship_column: for missing_nb rows on a VC page, resolve the row's member device (get_virtual_chassis_member) so inline parent/LAG sync targets the right device, not the viewed member. - SingleInterfaceVerifyView: host-scope by_port_id (_source != oob) to match get_context_data so OOB rows can't skew the verify-time enrichment. - _attach_messages_oob: restore storage.used after the peek so messages aren't consumed before the renderer emits them; test storage mirrors that semantics. --- netbox_librenms_plugin/librenms_api.py | 5 ++++- .../netbox_librenms_plugin/js/librenms_sync.js | 7 +++++++ netbox_librenms_plugin/tables/interfaces.py | 15 ++++++++++----- .../tests/test_coverage_actions.py | 10 ++++++++-- netbox_librenms_plugin/tests/test_librenms_api.py | 14 ++++++++++++++ netbox_librenms_plugin/views/imports/actions.py | 7 +++++-- .../views/object_sync/devices.py | 8 +++++++- 7 files changed, 55 insertions(+), 11 deletions(-) diff --git a/netbox_librenms_plugin/librenms_api.py b/netbox_librenms_plugin/librenms_api.py index f5f81329a5..88b8544ccd 100644 --- a/netbox_librenms_plugin/librenms_api.py +++ b/netbox_librenms_plugin/librenms_api.py @@ -569,7 +569,10 @@ def get_port_stack(self, device_id: int): ) response.raise_for_status() data = response.json() - return True, data.get("mappings", []) + # data.get("mappings", []) still yields None when the key is present but null; + # normalise so callers (resolve_port_relationships) never iterate a non-list. + mappings = data.get("mappings") if isinstance(data, dict) else None + return True, mappings if isinstance(mappings, list) else [] except requests.exceptions.HTTPError as e: if e.response.status_code == 404: return False, "Device not found in LibreNMS" 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 3ab307f5ab..0e5e3490a9 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 @@ -2285,6 +2285,13 @@ document.addEventListener('click', function (e) { body: body.toString(), }) .then(function (r) { + if (!r.ok) { + // Surface the backend error text (403/500/HTML page) instead of a + // generic JSON parse failure. + return fetchErrorMessage(r).then(function (msg) { + throw new Error(msg); + }); + } return r.json(); }) .then(function (data) { diff --git a/netbox_librenms_plugin/tables/interfaces.py b/netbox_librenms_plugin/tables/interfaces.py index 20995ca620..09afb6938e 100644 --- a/netbox_librenms_plugin/tables/interfaces.py +++ b/netbox_librenms_plugin/tables/interfaces.py @@ -451,11 +451,16 @@ def _render_relationship_column(self, lnms_name, lnms_port_id, sync_status, reco if sync_status == "missing_nb" and lnms_port_id: port_id = record.get("port_id", "") nb_iface = record.get("netbox_interface") - object_id = ( - nb_iface.device_id - if nb_iface and hasattr(nb_iface, "device_id") - else (self.device.pk if self.device else "") - ) + # Resolve the row's member device first. On a VC page self.device is the viewed + # member, which may not own this row's interface — without this, a missing_nb + # sync would target the wrong device. + if nb_iface and hasattr(nb_iface, "device_id"): + object_id = nb_iface.device_id + elif self.device is not None and getattr(self.device, "virtual_chassis", None): + member = get_virtual_chassis_member(self.device, record.get(self.interface_name_field)) + object_id = (member or self.device).pk + else: + object_id = self.device.pk if self.device else "" object_type = "virtualmachine" if hasattr(self.device, "cluster") and self.device.cluster else "device" btn = format_html( '
+ {# Hide bulk delete in migrated mode: the per-row "Move to winner" path preserves + cables/IPs/MACs, so a blanket delete here would drop interfaces that should be + transferred to the winner instead. #} {% if interface_sync.netbox_only_interfaces and not migrated_to_marker %}