From 0d746a77aa4d0aabbb8f6520319c01262350125d Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Mon, 25 May 2026 15:46:03 +0200 Subject: [PATCH 01/21] feat: UI improvements extracted from oob-sync - Extract device type mapping form into reusable partial (_dt_mapping_form.html) - Extract platform mapping form into reusable partial (_platform_mapping_form.html) - Add AddPlatformMappingView for creating PlatformMapping from import modal - Register add_platform_mapping URL - device_validation_details.html: replace 100-line inline DT form with include; add DT mapping option in mismatch branch; add no-mapping elif with warning; add platform mapping form in platform row - librenms_sync_base.html: fix VM-without-cluster breadcrumb; add object_model_name guard on platform modal; rename _create_platform_url -> create_platform_url - librenms_import.html: wrap toggles with span+hidden-input so unchecked values submit; rename checkbox IDs to -cb suffix - librenms_import.js: rename toggle IDs to match -cb suffix; fix nested modal dismiss so inner modals (e.g. Promote-to-host picker) are not broken --- .../js/librenms_import.js | 16 ++- .../htmx/_dt_mapping_form.html | 112 +++++++++++++++++ .../htmx/_platform_mapping_form.html | 110 ++++++++++++++++ .../htmx/device_validation_details.html | 117 +++--------------- .../librenms_import.html | 22 ++-- netbox_librenms_plugin/urls.py | 6 + netbox_librenms_plugin/views/__init__.py | 1 + .../views/imports/actions.py | 104 ++++++++++++++++ 8 files changed, 372 insertions(+), 116 deletions(-) create mode 100644 netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html create mode 100644 netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html diff --git a/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js b/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js index a2facc706..edfda206a 100644 --- a/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js +++ b/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js @@ -349,8 +349,8 @@ * Persists toggle state to user preferences on change. */ function initializeTogglePrefs() { - const sysname = document.getElementById('use-sysname-toggle'); - const strip = document.getElementById('strip-domain-toggle'); + const sysname = document.getElementById('use-sysname-toggle-cb'); + const strip = document.getElementById('strip-domain-toggle-cb'); if (sysname) sysname.addEventListener('change', function () { savePref('use_sysname', this.checked); }); if (strip) strip.addEventListener('change', function () { savePref('strip_domain', this.checked); }); } @@ -1199,10 +1199,14 @@ const dismissTrigger = event.target.closest('[data-bs-dismiss="modal"]'); if (dismissTrigger) { - event.preventDefault(); - - // Check if it's in the HTMX modal - if (modalElement.contains(dismissTrigger)) { + // Only handle dismiss triggers whose nearest .modal ancestor IS + // the outer HTMX modal. Buttons inside nested modals (e.g. the + // Promote-to-host modal rendered inside #htmx-modal-content) + // must be left for Bootstrap's own dismiss handler so they + // close the inner modal, not the outer one. + const nearestModal = dismissTrigger.closest('.modal'); + if (nearestModal === modalElement) { + event.preventDefault(); hideModal(modalElement, fallbackBackdropRef); } } diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html new file mode 100644 index 000000000..8fc88a2e2 --- /dev/null +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html @@ -0,0 +1,112 @@ +{% comment %} +Reusable "Add Device Type Mapping" form for the import validation modal. + +Required context: + - libre_device (dict-like with device_id, hardware) +Optional context: + - preselect_device_type: a DeviceType instance to pre-fill the typeahead + (used in the existing-device + mismatch branch so a single click maps + `libre_device.hardware` → `existing_device.device_type`). + - submit_label (defaults to "Add Mapping") +{% endcomment %} +
+ {% csrf_token %} +
+ + + + +
+ {# Disabled until a typeahead item is chosen (or a preselect value is present). #} + +
+ diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html new file mode 100644 index 000000000..27904eb51 --- /dev/null +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html @@ -0,0 +1,110 @@ +{% comment %} +Reusable "Add Platform Mapping" form for the import validation modal. + +Required context: + - libre_device (dict-like with device_id, os) +Optional: + - preselect_platform: a Platform instance to pre-fill the typeahead. + - submit_label +{% endcomment %} +
+ {% csrf_token %} +
+ + + + +
+ {# Disabled until a typeahead item is chosen (or a preselect value is present). #} + +
+ diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html index ade94ebe0..1b2e4fcb4 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html @@ -175,6 +175,9 @@
+ {% if libre_device.hardware and libre_device.hardware != "-" %} + {% include "netbox_librenms_plugin/htmx/_dt_mapping_form.html" with preselect_device_type=validation.existing_device.device_type submit_label="Map LibreNMS hardware to current type" %} + {% endif %} {% elif validation.existing_device and validation.existing_device.device_type %} {{ validation.existing_device.device_type }} {% if sync_info and not sync_info.device_type_synced and sync_info.librenms_device_type %} @@ -189,6 +192,12 @@
+ {% elif sync_info and not sync_info.device_type_synced and not sync_info.librenms_device_type and libre_device.hardware and libre_device.hardware != "-" %} + {# LibreNMS hardware has no NetBox mapping yet -- let the user create one, pre-selected to the existing device type so the common case is one click. #} +
+ No mapping for LibreNMS hardware + {% include "netbox_librenms_plugin/htmx/_dt_mapping_form.html" with preselect_device_type=validation.existing_device.device_type submit_label="Map LibreNMS hardware to current type" %} +
{% elif validation.device_type.device_type %} {% endif %} @@ -201,106 +210,8 @@
{% if libre_device.hardware and libre_device.hardware != "-" %}
No matching type -
- {% csrf_token %} - -
- - - - -
- -
+ {% include "netbox_librenms_plugin/htmx/_dt_mapping_form.html" %}
- {% else %} No matching type {% endif %} @@ -452,9 +363,11 @@
{% else %} Optional {% endif %} - {% endif %} - - {{ libre_device.os|default:"—" }} + {% if libre_device.os and libre_device.os != "-" and sync_info and not sync_info.platform_info.platform_exists %} + {% include "netbox_librenms_plugin/htmx/_platform_mapping_form.html" with preselect_platform=validation.existing_device.platform %} + {% endif %} + + {{ libre_device.os|default:"—" }} {% if validation.import_as_vm and not validation.existing_device or validation.existing_device and existing_device_model_name == "virtualmachine" %} diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.html index d1bdbd549..da9e4a319 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.html @@ -348,18 +348,24 @@
data-save-pref-url="{% url 'plugins:netbox_librenms_plugin:save_user_pref' %}"> Settings:
- -
- -
diff --git a/netbox_librenms_plugin/urls.py b/netbox_librenms_plugin/urls.py index 4d7881c17..73b954c54 100644 --- a/netbox_librenms_plugin/urls.py +++ b/netbox_librenms_plugin/urls.py @@ -13,6 +13,7 @@ from .views import ( AddDeviceToLibreNMSView, AddDeviceTypeMappingView, + AddPlatformMappingView, AssignVCSerialView, BulkImportConfirmView, BulkImportDevicesView, @@ -429,6 +430,11 @@ AddDeviceTypeMappingView.as_view(), name="add_device_type_mapping", ), + path( + "device-import/add-platform-mapping//", + AddPlatformMappingView.as_view(), + name="add_platform_mapping", + ), path( "device-import/create-platform//", CreatePlatformFromImportView.as_view(), diff --git a/netbox_librenms_plugin/views/__init__.py b/netbox_librenms_plugin/views/__init__.py index 4ce30a5c5..ece34847a 100644 --- a/netbox_librenms_plugin/views/__init__.py +++ b/netbox_librenms_plugin/views/__init__.py @@ -110,6 +110,7 @@ PlatformMappingView, ) from .imports.actions import AddDeviceTypeMappingView # noqa: F401 +from .imports.actions import AddPlatformMappingView # noqa: F401 from .object_sync import ( # noqa: F401 DeviceCableTableView, DeviceInterfaceTableView, diff --git a/netbox_librenms_plugin/views/imports/actions.py b/netbox_librenms_plugin/views/imports/actions.py index be442d93d..1a4828bfd 100644 --- a/netbox_librenms_plugin/views/imports/actions.py +++ b/netbox_librenms_plugin/views/imports/actions.py @@ -1789,3 +1789,107 @@ def post(self, request): save_user_pref(request, self.ALLOWED_PREFS[key], value) return JsonResponse({"status": "ok"}) + + +class AddPlatformMappingView( + LibreNMSPermissionMixin, NetBoxObjectPermissionMixin, LibreNMSAPIMixin, DeviceImportHelperMixin, View +): + """HTMX view to create a PlatformMapping from the import validation modal.""" + + def post(self, request, device_id): + """Create a PlatformMapping linking the LibreNMS OS string to a NetBox Platform.""" + if error := self.require_write_permission(): + return error + + from dcim.models import Platform + from netbox_librenms_plugin.librenms_api import LibreNMSAPI + from netbox_librenms_plugin.models import PlatformMapping + + post_server_key = (request.POST.get("server_key") or "").strip() + if post_server_key: + self._librenms_api = LibreNMSAPI(server_key=post_server_key) + + libre_device = fetch_device_with_cache(device_id, self.librenms_api) + if not libre_device: + return HttpResponse( + 'Device not found in LibreNMS.', + status=404, + ) + + librenms_os = (libre_device.get("os") or "").strip() + if not librenms_os: + return HttpResponse( + 'Device has no OS string -- cannot create mapping.', + status=400, + ) + + platform_id = request.POST.get("platform_id", "").strip() + if not platform_id: + return HttpResponse( + 'Please select a platform before submitting.', + status=400, + ) + + try: + platform_id = int(platform_id) + except (ValueError, TypeError): + return HttpResponse( + 'Invalid platform selection.', + status=400, + ) + + try: + platform = Platform.objects.get(pk=platform_id) + except Platform.DoesNotExist: + return HttpResponse( + 'Selected platform not found.', + status=404, + ) + + existing_mapping = PlatformMapping.objects.filter(librenms_os__iexact=librenms_os).first() + self.required_object_permissions = { + "POST": [("change", PlatformMapping) if existing_mapping else ("add", PlatformMapping)] + } + if error := self.require_object_permissions("POST"): + return error + + try: + with transaction.atomic(): + mapping, created = PlatformMapping.objects.get_or_create( + librenms_os=librenms_os.lower(), + defaults={"netbox_platform": platform}, + ) + if not created and existing_mapping is None and mapping.netbox_platform_id != platform_id: + self.required_object_permissions = {"POST": [("change", PlatformMapping)]} + if error := self.require_object_permissions("POST"): + return error + if not created and mapping.netbox_platform_id != platform_id: + mapping.netbox_platform = platform + mapping.save() + except Exception as exc: + logger.warning("AddPlatformMappingView: failed to save mapping: %s", exc) + return HttpResponse( + f'Error saving mapping: {escape(str(exc))}', + status=500, + ) + + cache_key = get_import_device_cache_key(device_id, self.librenms_api.server_key) + cache.delete(cache_key) + + detail_view = DeviceValidationDetailsView() + detail_view._librenms_api = self._librenms_api + modal_html = detail_view.get(request, device_id).content.decode("utf-8") + oob_modal = format_html( + '
{}
', + mark_safe(modal_html), + ) + + libre_device, validation, selections = self.get_validated_device_with_selections(device_id, request) + if libre_device is not None and validation is not None: + row_response = self.render_device_row(request, libre_device, validation, selections) + row_html = row_response.content.decode("utf-8") + row_html = format_html("{}
", mark_safe(row_html)) + else: + row_html = mark_safe("") + + return HttpResponse(oob_modal + row_html, content_type="text/html") From 0d71995d8d02dcb87b38550946d5d8f8bffadcbb Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Mon, 25 May 2026 23:52:34 +0200 Subject: [PATCH 02/21] fix(template): close platform-row if and make new-import mapping form reachable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Outer {% if validation.existing_device and ... %} at line 293 was missing its {% endif %}, breaking template compilation. The new-import branch's platform mapping include was also guarded on sync_info, which is only set when an existing device matches — so the form never rendered for new imports. Drop the sync_info clause and the unreachable preselect_platform. --- .../htmx/device_validation_details.html | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html index 1b2e4fcb4..c5cd487fd 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html @@ -363,9 +363,10 @@
{% else %} Optional {% endif %} - {% if libre_device.os and libre_device.os != "-" and sync_info and not sync_info.platform_info.platform_exists %} - {% include "netbox_librenms_plugin/htmx/_platform_mapping_form.html" with preselect_platform=validation.existing_device.platform %} + {% if libre_device.os and libre_device.os != "-" %} + {% include "netbox_librenms_plugin/htmx/_platform_mapping_form.html" %} {% endif %} + {% endif %} {{ libre_device.os|default:"—" }} From 88635b4b3c621832eabe4327e737240d34c9b74a Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 26 May 2026 18:59:46 +0200 Subject: [PATCH 03/21] fix(AddPlatformMappingView): close TOCTOU window with select_for_update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The upfront permission check at line 1849-1854 selects "add" or "change" based on whether a mapping exists, but the get_or_create that follows could race with a concurrent delete: a caller with only "change" permission could end up creating a new row. Mirror the established AddDeviceTypeMappingView pattern — lock the row with select_for_update, re-derive the required permission from the locked state, and surface IntegrityError on concurrent inserts. --- .../views/imports/actions.py | 42 +++++++++++++------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/netbox_librenms_plugin/views/imports/actions.py b/netbox_librenms_plugin/views/imports/actions.py index 1a4828bfd..d3ed44d94 100644 --- a/netbox_librenms_plugin/views/imports/actions.py +++ b/netbox_librenms_plugin/views/imports/actions.py @@ -1855,23 +1855,39 @@ def post(self, request, device_id): try: with transaction.atomic(): - mapping, created = PlatformMapping.objects.get_or_create( - librenms_os=librenms_os.lower(), - defaults={"netbox_platform": platform}, - ) - if not created and existing_mapping is None and mapping.netbox_platform_id != platform_id: - self.required_object_permissions = {"POST": [("change", PlatformMapping)]} + # Lock the row to close the TOCTOU window between the upfront + # permission check and the actual write. select_for_update cannot + # lock absent rows, so the create branch handles IntegrityError. + locked = PlatformMapping.objects.select_for_update().filter(librenms_os__iexact=librenms_os).first() + if locked and not existing_mapping: + # Concurrent request created the mapping after our upfront read. + # Only escalate to change permission if we would actually mutate. + if locked.netbox_platform_id != platform_id: + self.required_object_permissions = {"POST": [("change", PlatformMapping)]} + if error := self.require_object_permissions("POST"): + return error + if existing_mapping and not locked: + # Mapping was deleted between our upfront read and the lock. + # We are about to CREATE a new row, so require add permission. + self.required_object_permissions = {"POST": [("add", PlatformMapping)]} if error := self.require_object_permissions("POST"): return error - if not created and mapping.netbox_platform_id != platform_id: - mapping.netbox_platform = platform - mapping.save() + if locked: + if locked.netbox_platform_id != platform_id: + locked.netbox_platform = platform + locked.full_clean() + locked.save() + else: + try: + PlatformMapping.objects.create( + librenms_os=librenms_os.lower(), + netbox_platform=platform, + ) + except IntegrityError: + return _htmx_error_response("Mapping was created concurrently. Please try again.") except Exception as exc: logger.warning("AddPlatformMappingView: failed to save mapping: %s", exc) - return HttpResponse( - f'Error saving mapping: {escape(str(exc))}', - status=500, - ) + return _htmx_error_response("Error saving mapping. Please try again.") cache_key = get_import_device_cache_key(device_id, self.librenms_api.server_key) cache.delete(cache_key) From 2bc4457bfdfc19c7e608e0b9e1cea76e13bc466b Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Wed, 27 May 2026 08:47:33 +0200 Subject: [PATCH 04/21] fix(AddPlatformMappingView): use _htmx_error_response and reject "-" OS Mirror the sibling AddDeviceTypeMappingView style for consistent HTMX toast behavior on validation errors, and reject the LibreNMS "-" placeholder OS that previously slipped past the empty-string check. --- .../views/imports/actions.py | 27 +++++-------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/netbox_librenms_plugin/views/imports/actions.py b/netbox_librenms_plugin/views/imports/actions.py index d3ed44d94..bad97f76f 100644 --- a/netbox_librenms_plugin/views/imports/actions.py +++ b/netbox_librenms_plugin/views/imports/actions.py @@ -1811,40 +1811,25 @@ def post(self, request, device_id): libre_device = fetch_device_with_cache(device_id, self.librenms_api) if not libre_device: - return HttpResponse( - 'Device not found in LibreNMS.', - status=404, - ) + return _htmx_error_response("Device not found in LibreNMS.") librenms_os = (libre_device.get("os") or "").strip() - if not librenms_os: - return HttpResponse( - 'Device has no OS string -- cannot create mapping.', - status=400, - ) + if not librenms_os or librenms_os == "-": + return _htmx_error_response("Device has no OS string — cannot create mapping.") platform_id = request.POST.get("platform_id", "").strip() if not platform_id: - return HttpResponse( - 'Please select a platform before submitting.', - status=400, - ) + return _htmx_error_response("Please select a platform before submitting.") try: platform_id = int(platform_id) except (ValueError, TypeError): - return HttpResponse( - 'Invalid platform selection.', - status=400, - ) + return _htmx_error_response("Invalid platform selection.") try: platform = Platform.objects.get(pk=platform_id) except Platform.DoesNotExist: - return HttpResponse( - 'Selected platform not found.', - status=404, - ) + return _htmx_error_response("Selected platform not found.") existing_mapping = PlatformMapping.objects.filter(librenms_os__iexact=librenms_os).first() self.required_object_permissions = { From b72ee932a96d392877c9bf33b54528d3d24775d7 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Sun, 31 May 2026 15:52:01 +0200 Subject: [PATCH 05/21] fix(ui): expose platform-mapping form in existing-device branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Existing devices whose LibreNMS OS has no PlatformMapping previously only got the 'Create Platform' path, nudging users toward duplicate Platform records. Mirror the device-type flow: also offer the platform-mapping form — preselected to the device's current platform when it has one, or unselected when it doesn't — so the LibreNMS OS can be mapped to an existing platform instead. Addresses CodeRabbit feedback surfaced on PR #84 (the file belongs to this branch / PR #299). --- .../htmx/device_validation_details.html | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html index c5cd487fd..4801b2de5 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html @@ -318,6 +318,10 @@
aria-label="Create platform for {{ libre_device.os }}"> + {# Let the user map the LibreNMS OS to the device's current platform instead of creating a duplicate (mirrors the device-type flow). #} +
+ {% include "netbox_librenms_plugin/htmx/_platform_mapping_form.html" with preselect_platform=validation.existing_device.platform submit_label="Map LibreNMS OS to current platform" %} +
{% endif %} {% endif %} {% elif sync_info and sync_info.platform_synced %} @@ -346,6 +350,10 @@
title="Create platform for {{ libre_device.os }}"> Create Platform + {# No current platform to preselect, but still let the user map the OS to an existing platform rather than create one. #} +
+ {% include "netbox_librenms_plugin/htmx/_platform_mapping_form.html" with submit_label="Map LibreNMS OS to a platform" %} +
{% endif %} {% elif validation.platform.platform %} {# New import — platform will be assigned on import #} From 7d7be71f33b9d454364feaaedabe43e2d9e08313 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Sun, 31 May 2026 16:29:45 +0200 Subject: [PATCH 06/21] fix(ui): guard against duplicate PlatformMapping rows with proper row locking AddPlatformMappingView now rejects an ambiguous OS string up front, and inside the transaction materialises the candidate rows with select_for_update()[:2] instead of count(). count() silently drops the FOR UPDATE clause, so the rows were never actually locked before the create/update decision (race window). Addresses CodeRabbit feedback on PR #303; the platform-mapping logic belongs to this branch / PR #299. --- netbox_librenms_plugin/views/imports/actions.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/netbox_librenms_plugin/views/imports/actions.py b/netbox_librenms_plugin/views/imports/actions.py index bad97f76f..496e9f419 100644 --- a/netbox_librenms_plugin/views/imports/actions.py +++ b/netbox_librenms_plugin/views/imports/actions.py @@ -1831,6 +1831,10 @@ def post(self, request, device_id): except Platform.DoesNotExist: return _htmx_error_response("Selected platform not found.") + if PlatformMapping.objects.filter(librenms_os__iexact=librenms_os).count() > 1: + return _htmx_error_response( + "Multiple mappings exist for this OS string. Remove duplicates before updating." + ) existing_mapping = PlatformMapping.objects.filter(librenms_os__iexact=librenms_os).first() self.required_object_permissions = { "POST": [("change", PlatformMapping) if existing_mapping else ("add", PlatformMapping)] @@ -1843,7 +1847,16 @@ def post(self, request, device_id): # Lock the row to close the TOCTOU window between the upfront # permission check and the actual write. select_for_update cannot # lock absent rows, so the create branch handles IntegrityError. - locked = PlatformMapping.objects.select_for_update().filter(librenms_os__iexact=librenms_os).first() + # Materialise the locked rows in one query — count() would drop + # the FOR UPDATE clause, leaving the rows unlocked. + locked_rows = list( + PlatformMapping.objects.select_for_update().filter(librenms_os__iexact=librenms_os)[:2] + ) + if len(locked_rows) > 1: + return _htmx_error_response( + "Multiple mappings exist for this OS string. Remove duplicates before updating." + ) + locked = locked_rows[0] if locked_rows else None if locked and not existing_mapping: # Concurrent request created the mapping after our upfront read. # Only escalate to change permission if we would actually mutate. From 517cc95d215ffdea9e69086735cb86d70e09818d Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Sun, 31 May 2026 19:21:55 +0200 Subject: [PATCH 07/21] ui(platform): compact platform cell with a single edit icon -> combined modal The platform cell in the import validation modal had become cluttered (inline mapping typeahead + Create Platform button + sync button all at once). Replace the inline forms/buttons with a single pencil icon that opens one modal offering BOTH options: - map the LibreNMS OS to an existing platform (typeahead), and - create a new platform. The icon also appears where a platform is already resolved (direct match or via mapping), so the mapping can be changed compactly. The combined modal reuses the existing create-platform modal plus the _platform_mapping_form partial, guarded so the device-sync page (create-only, no libre_device) is unaffected. - _platform_manage_icon.html: the compact pencil-icon trigger - create_platform_modal.html: adds a 'map to existing' section when libre_device is present; neutral alert wording - CreatePlatformFromImportView.get: passes libre_device + current_platform - device_validation_details.html: platform cell rewritten to use the icon; inline _platform_mapping_form includes + Create Platform buttons removed --- .../htmx/_platform_manage_icon.html | 13 ++++++ .../htmx/create_platform_modal.html | 15 +++++-- .../htmx/device_validation_details.html | 45 +++---------------- .../views/imports/actions.py | 5 +++ 4 files changed, 36 insertions(+), 42 deletions(-) create mode 100644 netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.html diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.html new file mode 100644 index 000000000..70b2fee6b --- /dev/null +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.html @@ -0,0 +1,13 @@ +{# Compact pencil icon that opens the combined "manage platform" modal + (map LibreNMS OS to an existing platform, or create a new one). Keeps the + platform cell uncluttered. Requires: libre_device, server_key. #} +{% if libre_device.os and libre_device.os != "-" %} + +{% endif %} diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.html index f2b557e0d..27db062c8 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.html @@ -13,10 +13,19 @@ {# server_key - (optional) LibreNMS server key, forwarded as hidden field #}
+{% if libre_device %} +{# Map the LibreNMS OS to an EXISTING platform (separate form -> add_platform_mapping). #} +
+
Map {{ librenms_os }} to an existing platform
+ {% include "netbox_librenms_plugin/htmx/_platform_mapping_form.html" with preselect_platform=current_platform submit_label="Map to this platform" %} +
— or create a new platform below —
+
+
+{% endif %} {% csrf_token %} @@ -27,9 +36,9 @@

const saveImportBtn = document.getElementById('save-import-btn'); const useSysnameCheckbox = document.getElementById('id_use_sysname_default'); const stripDomainCheckbox = document.getElementById('id_strip_domain_default'); + const autoCreateIpamCheckbox = document.getElementById('id_auto_create_ipam_default'); // Store the initial values to detect changes const initialServerValue = serverSelect.value; const initialVcPatternValue = vcPatternInput.value; const initialUseSysnameValue = useSysnameCheckbox ? useSysnameCheckbox.checked : true; const initialStripDomainValue = stripDomainCheckbox ? stripDomainCheckbox.checked : false; + const initialAutoCreateIpamValue = autoCreateIpamCheckbox ? autoCreateIpamCheckbox.checked : false; // Enable/disable server save button based on changes function updateServerSaveButton() { @@ -319,7 +333,8 @@

const vcPatternChanged = vcPatternInput.value !== initialVcPatternValue; const useSysnameChanged = useSysnameCheckbox ? (useSysnameCheckbox.checked !== initialUseSysnameValue) : false; const stripDomainChanged = stripDomainCheckbox ? (stripDomainCheckbox.checked !== initialStripDomainValue) : false; - const hasChanges = vcPatternChanged || useSysnameChanged || stripDomainChanged; + const autoCreateIpamChanged = autoCreateIpamCheckbox ? (autoCreateIpamCheckbox.checked !== initialAutoCreateIpamValue) : false; + const hasChanges = vcPatternChanged || useSysnameChanged || stripDomainChanged || autoCreateIpamChanged; saveImportBtn.disabled = !hasChanges; saveImportBtn.classList.toggle('btn-primary', hasChanges); saveImportBtn.classList.toggle('btn-secondary', !hasChanges); @@ -330,6 +345,7 @@

vcPatternInput.addEventListener('input', updateImportSaveButton); if (useSysnameCheckbox) useSysnameCheckbox.addEventListener('change', updateImportSaveButton); if (stripDomainCheckbox) stripDomainCheckbox.addEventListener('change', updateImportSaveButton); + if (autoCreateIpamCheckbox) autoCreateIpamCheckbox.addEventListener('change', updateImportSaveButton); // Initialize button states updateServerSaveButton(); diff --git a/netbox_librenms_plugin/tests/test_ip_helpers.py b/netbox_librenms_plugin/tests/test_ip_helpers.py new file mode 100644 index 000000000..cde4d30fd --- /dev/null +++ b/netbox_librenms_plugin/tests/test_ip_helpers.py @@ -0,0 +1,173 @@ +""" +Tests for netbox_librenms_plugin.import_utils.ip_helpers. + +All DB interactions are mocked — no @pytest.mark.django_db used. +""" + +from unittest.mock import MagicMock, patch + + +class TestGetOrCreateGlobalIP: + """Tests for ``get_or_create_global_ip``.""" + + def test_returns_none_for_empty_string(self): + from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip + + assert get_or_create_global_ip("") == (None, False) + assert get_or_create_global_ip(None) == (None, False) + assert get_or_create_global_ip(" ") == (None, False) + + def test_returns_none_for_invalid_ip(self): + from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip + + assert get_or_create_global_ip("not-an-ip") == (None, False) + assert get_or_create_global_ip("192.168.1.999") == (None, False) + + def test_returns_existing_when_present(self): + from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip + + existing = MagicMock(name="existing-ip") + with patch("ipam.models.IPAddress") as mock_model: + mock_model.objects.filter.return_value.first.return_value = existing + + result, created = get_or_create_global_ip("10.0.0.1") + + mock_model.objects.filter.assert_called_once_with(address__net_host="10.0.0.1", vrf__isnull=True) + mock_model.objects.create.assert_not_called() + assert result is existing + assert created is False + + def test_creates_ipv4_with_slash32_when_missing(self): + from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip + + created = MagicMock(name="created-ip") + with patch("ipam.models.IPAddress") as mock_model: + mock_model.objects.filter.return_value.first.return_value = None + mock_model.objects.create.return_value = created + + result, was_created = get_or_create_global_ip("10.0.0.1") + + mock_model.objects.create.assert_called_once_with(address="10.0.0.1/32", status="active") + assert result is created + assert was_created is True + + def test_creates_ipv6_with_slash128_when_missing(self): + from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip + + created = MagicMock(name="created-ip-v6") + with patch("ipam.models.IPAddress") as mock_model: + mock_model.objects.filter.return_value.first.return_value = None + mock_model.objects.create.return_value = created + + result, was_created = get_or_create_global_ip("2001:db8::1") + + mock_model.objects.create.assert_called_once_with(address="2001:db8::1/128", status="active") + assert result is created + assert was_created is True + + def test_strips_whitespace_before_lookup(self): + from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip + + with patch("ipam.models.IPAddress") as mock_model: + mock_model.objects.filter.return_value.first.return_value = None + mock_model.objects.create.return_value = MagicMock() + + result, was_created = get_or_create_global_ip(" 10.1.2.3 ") + + mock_model.objects.filter.assert_called_once_with(address__net_host="10.1.2.3", vrf__isnull=True) + mock_model.objects.create.assert_called_once_with(address="10.1.2.3/32", status="active") + assert was_created is True + + def test_integrity_error_on_create_returns_concurrently_created_record(self): + """A concurrent insert that races us is re-fetched and returned.""" + from django.db import IntegrityError + + from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip + + racing_winner = MagicMock(name="racing-winner") + with patch("ipam.models.IPAddress") as mock_model: + # First filter (pre-create) → None, second filter (post-IntegrityError) → winner. + mock_model.objects.filter.return_value.first.side_effect = [None, racing_winner] + mock_model.objects.create.side_effect = IntegrityError("duplicate key") + + result, was_created = get_or_create_global_ip("10.0.0.1") + + assert result is racing_winner + assert was_created is False + + def test_returns_none_and_logs_when_create_raises(self, caplog): + from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip + + with patch("ipam.models.IPAddress") as mock_model: + mock_model.objects.filter.return_value.first.return_value = None + mock_model.objects.create.side_effect = RuntimeError("integrity failure") + + with caplog.at_level("WARNING"): + result, was_created = get_or_create_global_ip("10.0.0.1") + + assert result is None + assert was_created is False + assert any("failed to auto-create" in r.message for r in caplog.records) + + def test_reexported_from_import_utils_package(self): + """The helper is re-exported from the package root for convenience.""" + from netbox_librenms_plugin.import_utils import get_or_create_global_ip as pkg_helper + from netbox_librenms_plugin.import_utils.ip_helpers import ( + get_or_create_global_ip as module_helper, + ) + + assert pkg_helper is module_helper + + +class TestAutoCreateOptIn: + """``auto_create=False`` opt-out and ``auto_create_ipam_enabled()`` helper.""" + + def test_auto_create_false_returns_existing(self): + from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip + + existing = MagicMock(name="existing-ip") + with patch("ipam.models.IPAddress") as mock_model: + mock_model.objects.filter.return_value.first.return_value = existing + + result, created = get_or_create_global_ip("10.0.0.1", auto_create=False) + + assert result is existing + assert created is False + mock_model.objects.create.assert_not_called() + + def test_auto_create_false_skips_creation_when_missing(self): + from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip + + with patch("ipam.models.IPAddress") as mock_model: + mock_model.objects.filter.return_value.first.return_value = None + + result, created = get_or_create_global_ip("10.0.0.1", auto_create=False) + + assert result is None + assert created is False + mock_model.objects.create.assert_not_called() + + def test_auto_create_ipam_enabled_false_when_no_settings(self): + from netbox_librenms_plugin.import_utils.ip_helpers import auto_create_ipam_enabled + + with patch("netbox_librenms_plugin.models.LibreNMSSettings") as mock_model: + mock_model.objects.first.return_value = None + assert auto_create_ipam_enabled() is False + + def test_auto_create_ipam_enabled_reads_field(self): + from netbox_librenms_plugin.import_utils.ip_helpers import auto_create_ipam_enabled + + with patch("netbox_librenms_plugin.models.LibreNMSSettings") as mock_model: + settings = MagicMock(auto_create_ipam_default=True) + mock_model.objects.first.return_value = settings + assert auto_create_ipam_enabled() is True + + settings.auto_create_ipam_default = False + assert auto_create_ipam_enabled() is False + + def test_auto_create_ipam_enabled_swallows_exceptions(self): + from netbox_librenms_plugin.import_utils.ip_helpers import auto_create_ipam_enabled + + with patch("netbox_librenms_plugin.models.LibreNMSSettings") as mock_model: + mock_model.objects.first.side_effect = RuntimeError("db down") + assert auto_create_ipam_enabled() is False diff --git a/netbox_librenms_plugin/utils.py b/netbox_librenms_plugin/utils.py index 8da32ed15..355440cf8 100644 --- a/netbox_librenms_plugin/utils.py +++ b/netbox_librenms_plugin/utils.py @@ -363,6 +363,59 @@ def _is_truthy(val): return use_sysname, strip_domain +def resolve_auto_create_ipam(request) -> bool: + """Resolve the IPAM auto-create flag for an import-time request. + + Mirrors the cascade used by :func:`resolve_naming_preferences`: + + 1. POST/GET ``auto-create-ipam-toggle`` (or ``auto_create_ipam``) wins + -- set by the import page toggle and propagated via ``hx-include``. + 2. Otherwise the user's saved preference + ``plugins.netbox_librenms_plugin.auto_create_ipam`` is used. + 3. Otherwise the plugin-wide ``LibreNMSSettings.auto_create_ipam_default`` + (which itself defaults to ``False``) is used. + + Use this from request-handling code (HTMX views, sync action POST + handlers). Background paths without a request should keep using + :func:`netbox_librenms_plugin.import_utils.ip_helpers.auto_create_ipam_enabled`, + which is the settings-only fallback. + """ + from netbox_librenms_plugin.models import LibreNMSSettings + + _TRUTHY = frozenset({"on", "true", "1"}) + _KEYS = ("auto-create-ipam-toggle", "auto_create_ipam-toggle", "auto_create_ipam") + + def _is_truthy(val): + return val.lower() in _TRUTHY if val is not None else False + + if request is None: + try: + settings = LibreNMSSettings.objects.first() + return bool(getattr(settings, "auto_create_ipam_default", False)) if settings else False + except Exception: + return False + + post_val = next((request.POST.get(k) for k in _KEYS if k in request.POST), None) + get_val = next((request.GET.get(k) for k in _KEYS if k in request.GET), None) + + if post_val is not None: + return _is_truthy(post_val) + if get_val is not None: + return _is_truthy(get_val) + + pref = get_user_pref(request, "plugins.netbox_librenms_plugin.auto_create_ipam") + if pref is not None: + if isinstance(pref, str): + return _is_truthy(pref) + return bool(pref) + + try: + settings = LibreNMSSettings.objects.first() + return bool(getattr(settings, "auto_create_ipam_default", False)) if settings else False + except Exception: + return False + + def get_interface_name_field(request: Optional[HttpRequest] = None) -> str: """ Get interface name field with request override support. diff --git a/netbox_librenms_plugin/views/imports/actions.py b/netbox_librenms_plugin/views/imports/actions.py index 39b208a5e..f6d838ac4 100644 --- a/netbox_librenms_plugin/views/imports/actions.py +++ b/netbox_librenms_plugin/views/imports/actions.py @@ -35,7 +35,12 @@ fetch_model_by_id, ) from netbox_librenms_plugin.tables.device_status import DeviceImportTable -from netbox_librenms_plugin.utils import resolve_naming_preferences, save_user_pref, set_librenms_device_id +from netbox_librenms_plugin.utils import ( + resolve_auto_create_ipam, + resolve_naming_preferences, + save_user_pref, + set_librenms_device_id, +) from netbox_librenms_plugin.views.mixins import LibreNMSAPIMixin, LibreNMSPermissionMixin, NetBoxObjectPermissionMixin logger = logging.getLogger(__name__) @@ -559,6 +564,7 @@ def post(self, request): # noqa: PLR0912 - branching keeps responses explicit return HttpResponse("Invalid device identifier", status=400) use_sysname, strip_domain = resolve_naming_preferences(request) + auto_create_ipam = resolve_auto_create_ipam(request) vc_detection_enabled = _resolve_vc_detection_enabled(request) sync_options = { "sync_interfaces": request.POST.get("sync_interfaces") == "on", @@ -567,6 +573,7 @@ def post(self, request): # noqa: PLR0912 - branching keeps responses explicit "vc_detection_enabled": vc_detection_enabled, "use_sysname": use_sysname, "strip_domain": strip_domain, + "auto_create_ipam": auto_create_ipam, } manual_mappings_per_device: dict[int, dict[str, int]] = {} @@ -655,8 +662,6 @@ def post(self, request): # noqa: PLR0912 - branching keeps responses explicit ) # Show notification and redirect - matching NetBox's native pattern - from django.utils.safestring import mark_safe - messages.info( request, mark_safe( @@ -744,21 +749,43 @@ def post(self, request): # noqa: PLR0912 - branching keeps responses explicit failed_count = len(device_result.get("failed", [])) + len(vm_result.get("failed", [])) skipped_count = len(device_result.get("skipped", [])) + len(vm_result.get("skipped", [])) + # Build summary messages for both paths. For HTMX responses, messages.* + # is consumed/cleared by device_import_row.html before reaching the browser; + # htmx_toasts carries the same text as an explicit OOB swap instead. + htmx_toasts = [] # [(bg_class, mdi_class, label, text), ...] + if success_count: - messages.success( - request, - f"Successfully imported {success_count} LibreNMS device{'s' if success_count != 1 else ''}", + _msg = f"Successfully imported {success_count} LibreNMS device{'s' if success_count != 1 else ''}" + messages.success(request, _msg) + htmx_toasts.append(("text-bg-success", "mdi-check-circle", "Success", _msg)) + + # Aggregate auto-created IPAM entries across the batch and surface a + # single info toast so the user knows a side-effect happened on import. + created_ips_all = [] + for item in device_result.get("success", []): + created_ips_all.extend(item.get("created_ips") or []) + for item in vm_result.get("success", []): + vm_obj = item.get("device") or item.get("vm") + ips = getattr(vm_obj, "_librenms_created_ips", None) if vm_obj is not None else None + if ips: + created_ips_all.extend(ips) + if created_ips_all: + unique_ips = sorted(set(created_ips_all)) + preview = ", ".join(unique_ips[:5]) + (f" (+{len(unique_ips) - 5} more)" if len(unique_ips) > 5 else "") + _msg = ( + f"Auto-created {len(unique_ips)} IPAM entr{'y' if len(unique_ips) == 1 else 'ies'} " + f"in the global scope (unassigned): {preview}." ) + messages.info(request, _msg) + htmx_toasts.append(("text-bg-info", "mdi-information", "Info", _msg)) if failed_count: - messages.error( - request, - f"Failed to import {failed_count} device{'s' if failed_count != 1 else ''}", - ) + _msg = f"Failed to import {failed_count} device{'s' if failed_count != 1 else ''}" + messages.error(request, _msg) + htmx_toasts.append(("text-bg-danger", "mdi-alert-circle", "Error", _msg)) if skipped_count: - messages.warning( - request, - f"Skipped {skipped_count} existing device{'s' if skipped_count != 1 else ''}", - ) + _msg = f"Skipped {skipped_count} existing device{'s' if skipped_count != 1 else ''}" + messages.warning(request, _msg) + htmx_toasts.append(("text-bg-warning", "mdi-alert", "Warning", _msg)) if request.headers.get("HX-Request"): # Return updated rows for all imported devices using HTMX OOB swaps @@ -819,7 +846,38 @@ def post(self, request): # noqa: PLR0912 - branching keeps responses explicit ).content.decode("utf-8") updated_rows_html.append(row_html) - # Return concatenated row HTML with closeModal trigger + # Append all summary toasts as a single OOB swap. The messages.* + # queue is consumed/cleared by device_import_row.html, so they never + # reach the browser in the HTMX path; this OOB fragment takes over. + if htmx_toasts: + toast_items = mark_safe( + "".join( + format_html( + '", + bg_cls, + icon_cls, + label, + text, + ) + for bg_cls, icon_cls, label, text in htmx_toasts + ) + ) + updated_rows_html.append( + format_html( + '
{}
', + toast_items, + ) + ) return HttpResponse( "\n".join(updated_rows_html), headers={"HX-Trigger": '{"closeModal": null}'}, @@ -1776,6 +1834,7 @@ class SaveUserPrefView(LibreNMSPermissionMixin, View): ALLOWED_PREFS = { "use_sysname": "plugins.netbox_librenms_plugin.use_sysname", "strip_domain": "plugins.netbox_librenms_plugin.strip_domain", + "auto_create_ipam": "plugins.netbox_librenms_plugin.auto_create_ipam", "interface_name_field": "plugins.netbox_librenms_plugin.interface_name_field", } diff --git a/netbox_librenms_plugin/views/imports/list.py b/netbox_librenms_plugin/views/imports/list.py index d2459a439..419f3f487 100644 --- a/netbox_librenms_plugin/views/imports/list.py +++ b/netbox_librenms_plugin/views/imports/list.py @@ -15,7 +15,7 @@ ) from netbox_librenms_plugin.models import LibreNMSSettings from netbox_librenms_plugin.tables.device_status import DeviceImportTable -from netbox_librenms_plugin.utils import get_user_pref +from netbox_librenms_plugin.utils import get_user_pref, resolve_auto_create_ipam from netbox_librenms_plugin.views.mixins import LibreNMSAPIMixin, LibreNMSPermissionMixin logger = logging.getLogger(__name__) @@ -162,9 +162,11 @@ def get(self, request, *args, **kwargs): # noqa: D401 - inherited doc _use_sysname = getattr(settings_obj, "use_sysname_default", True) if settings_obj else True if _strip_domain is None: _strip_domain = getattr(settings_obj, "strip_domain_default", False) if settings_obj else False + _auto_create_ipam = resolve_auto_create_ipam(request) self._use_sysname = _use_sysname self._strip_domain = _strip_domain + self._auto_create_ipam = _auto_create_ipam self._settings = settings_obj # Determine if new filters are being submitted @@ -350,6 +352,7 @@ def get(self, request, *args, **kwargs): # noqa: D401 - inherited doc "settings": self._settings, "use_sysname": self._use_sysname, "strip_domain": self._strip_domain, + "auto_create_ipam": self._auto_create_ipam, "vc_detection_enabled": getattr(self, "_vc_detection_enabled", False), "cache_cleared": getattr(self, "_cache_cleared", False), "from_cache": getattr(self, "_from_cache", False), From eeb9099767302ef618a46d4f22703e57e246b4e5 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Sun, 31 May 2026 14:50:42 +0200 Subject: [PATCH 12/21] fix(ip_helpers): wrap create() in nested savepoint to avoid poisoning outer transaction IntegrityError caught without a nested savepoint leaves the outer transaction.atomic() marked as needing rollback, causing TransactionManagementError on any subsequent ORM query in the same block. Wrapping create() in transaction.atomic() creates a savepoint so the IntegrityError is absorbed without affecting the outer transaction. Also update docs to remove device_type normalization scope from mapping_rules.md since apply_normalization_rules() is not yet called by match_librenms_hardware_to_device_type(). Deferred to issue #90. --- netbox_librenms_plugin/import_utils/ip_helpers.py | 5 +++-- netbox_librenms_plugin/tests/test_ip_helpers.py | 15 ++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/netbox_librenms_plugin/import_utils/ip_helpers.py b/netbox_librenms_plugin/import_utils/ip_helpers.py index eec404813..d8c9a024d 100644 --- a/netbox_librenms_plugin/import_utils/ip_helpers.py +++ b/netbox_librenms_plugin/import_utils/ip_helpers.py @@ -78,7 +78,7 @@ def get_or_create_global_ip(ip_str: str | None, *, auto_create: bool = True) -> logger.debug("get_or_create_global_ip: invalid IP %r", ip_str) return None, False - from django.db import IntegrityError + from django.db import IntegrityError, transaction from ipam.models import IPAddress @@ -91,7 +91,8 @@ def get_or_create_global_ip(ip_str: str | None, *, auto_create: bool = True) -> mask = "/128" if parsed.version == 6 else "/32" try: - return IPAddress.objects.create(address=f"{ip_str}{mask}", status="active"), True + with transaction.atomic(): + return IPAddress.objects.create(address=f"{ip_str}{mask}", status="active"), True except IntegrityError: # Concurrent create won the race; re-query the global record and return it. existing = IPAddress.objects.filter(address__net_host=ip_str, vrf__isnull=True).first() diff --git a/netbox_librenms_plugin/tests/test_ip_helpers.py b/netbox_librenms_plugin/tests/test_ip_helpers.py index cde4d30fd..8b2297e00 100644 --- a/netbox_librenms_plugin/tests/test_ip_helpers.py +++ b/netbox_librenms_plugin/tests/test_ip_helpers.py @@ -41,7 +41,8 @@ def test_creates_ipv4_with_slash32_when_missing(self): from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip created = MagicMock(name="created-ip") - with patch("ipam.models.IPAddress") as mock_model: + with patch("ipam.models.IPAddress") as mock_model, patch("django.db.transaction.atomic") as mock_atomic: + mock_atomic.return_value.__exit__.return_value = False mock_model.objects.filter.return_value.first.return_value = None mock_model.objects.create.return_value = created @@ -55,7 +56,8 @@ def test_creates_ipv6_with_slash128_when_missing(self): from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip created = MagicMock(name="created-ip-v6") - with patch("ipam.models.IPAddress") as mock_model: + with patch("ipam.models.IPAddress") as mock_model, patch("django.db.transaction.atomic") as mock_atomic: + mock_atomic.return_value.__exit__.return_value = False mock_model.objects.filter.return_value.first.return_value = None mock_model.objects.create.return_value = created @@ -68,7 +70,8 @@ def test_creates_ipv6_with_slash128_when_missing(self): def test_strips_whitespace_before_lookup(self): from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip - with patch("ipam.models.IPAddress") as mock_model: + with patch("ipam.models.IPAddress") as mock_model, patch("django.db.transaction.atomic") as mock_atomic: + mock_atomic.return_value.__exit__.return_value = False mock_model.objects.filter.return_value.first.return_value = None mock_model.objects.create.return_value = MagicMock() @@ -85,7 +88,8 @@ def test_integrity_error_on_create_returns_concurrently_created_record(self): from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip racing_winner = MagicMock(name="racing-winner") - with patch("ipam.models.IPAddress") as mock_model: + with patch("ipam.models.IPAddress") as mock_model, patch("django.db.transaction.atomic") as mock_atomic: + mock_atomic.return_value.__exit__.return_value = False # First filter (pre-create) → None, second filter (post-IntegrityError) → winner. mock_model.objects.filter.return_value.first.side_effect = [None, racing_winner] mock_model.objects.create.side_effect = IntegrityError("duplicate key") @@ -98,7 +102,8 @@ def test_integrity_error_on_create_returns_concurrently_created_record(self): def test_returns_none_and_logs_when_create_raises(self, caplog): from netbox_librenms_plugin.import_utils.ip_helpers import get_or_create_global_ip - with patch("ipam.models.IPAddress") as mock_model: + with patch("ipam.models.IPAddress") as mock_model, patch("django.db.transaction.atomic") as mock_atomic: + mock_atomic.return_value.__exit__.return_value = False mock_model.objects.filter.return_value.first.return_value = None mock_model.objects.create.side_effect = RuntimeError("integrity failure") From ee95c05292cc644a4f922b17260d5335c4150037 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Sun, 31 May 2026 14:50:42 +0200 Subject: [PATCH 13/21] fix: duplicate PlatformMapping guard and expose VM created_ips in bulk result AddPlatformMappingView now fails closed (error response) when more than one PlatformMapping row exists for the same librenms_os string, both before the permission check (fast path) and inside the transaction.atomic() block after SELECT FOR UPDATE (to catch concurrent duplicates). bulk_import_vms now includes 'created_ips' in each success entry, populated from vm._librenms_created_ips, mirroring bulk_import_devices_shared so callers receive consistent IPAM creation feedback for VMs. --- netbox_librenms_plugin/import_utils/vm_operations.py | 1 + 1 file changed, 1 insertion(+) diff --git a/netbox_librenms_plugin/import_utils/vm_operations.py b/netbox_librenms_plugin/import_utils/vm_operations.py index a745d74b9..9a6363b34 100644 --- a/netbox_librenms_plugin/import_utils/vm_operations.py +++ b/netbox_librenms_plugin/import_utils/vm_operations.py @@ -276,6 +276,7 @@ def bulk_import_vms( "device_id": vm_id, "device": vm, "message": f"VM {vm.name} created successfully", + "created_ips": getattr(vm, "_librenms_created_ips", []), } ) log.info(f"Successfully imported VM {vm.name} (ID: {vm_id})") From 506c40d23ac6db618ce4fd57f0e95dd200491223 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Sun, 31 May 2026 14:50:42 +0200 Subject: [PATCH 14/21] =?UTF-8?q?fix(ipam):=20address=20PR=20#84=20review?= =?UTF-8?q?=20=E2=80=94=20server=5Fkey=20in=20HTMX=20mapping=20forms,=20st?= =?UTF-8?q?ale-lookup=20guards,=20simplify=20auto=5Fcreate=5Fipam=20coerci?= =?UTF-8?q?on?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add hidden server_key input to device-type and platform mapping forms so multi-server POSTs resolve against the correct LibreNMS API/cache namespace (AddDeviceTypeMappingView/AddPlatformMappingView read request.POST[server_key]) - Guard the typeahead .catch handlers with seq !== requestSeq so a stale failed lookup can't hide the dropdown from a newer successful one - Drop redundant string truthy-parsing of auto_create_ipam in bulk_import_vms and import_single_device; the flag is already normalised to bool by resolve_auto_create_ipam() at the request boundary --- netbox_librenms_plugin/import_utils/device_operations.py | 6 ++++-- netbox_librenms_plugin/import_utils/vm_operations.py | 6 ++++-- .../netbox_librenms_plugin/htmx/_dt_mapping_form.html | 1 + .../netbox_librenms_plugin/htmx/_platform_mapping_form.html | 1 + 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/netbox_librenms_plugin/import_utils/device_operations.py b/netbox_librenms_plugin/import_utils/device_operations.py index 5e02e744f..9a6b58136 100644 --- a/netbox_librenms_plugin/import_utils/device_operations.py +++ b/netbox_librenms_plugin/import_utils/device_operations.py @@ -910,10 +910,12 @@ def import_single_device( if primary_ip: from .ip_helpers import auto_create_ipam_enabled, get_or_create_global_ip + # Already normalised to a bool by resolve_auto_create_ipam() at + # the request boundary; coerce defensively and fall back to the + # plugin default when the key is absent (e.g. background paths). _opts = sync_options or {} if "auto_create_ipam" in _opts: - _raw = _opts.get("auto_create_ipam") - _auto_create = _raw.strip().lower() in {"1", "true", "on"} if isinstance(_raw, str) else bool(_raw) + _auto_create = bool(_opts.get("auto_create_ipam")) else: _auto_create = auto_create_ipam_enabled() _ip, was_created = get_or_create_global_ip(primary_ip, auto_create=_auto_create) diff --git a/netbox_librenms_plugin/import_utils/vm_operations.py b/netbox_librenms_plugin/import_utils/vm_operations.py index 9a6363b34..1970bf1a7 100644 --- a/netbox_librenms_plugin/import_utils/vm_operations.py +++ b/netbox_librenms_plugin/import_utils/vm_operations.py @@ -174,9 +174,11 @@ def bulk_import_vms( # Resolve options once before the loop — they do not change per-VM use_sysname_opt = sync_options.get("use_sysname", True) if sync_options else True strip_domain_opt = sync_options.get("strip_domain", False) if sync_options else False + # The flag is already normalised to a bool by resolve_auto_create_ipam() + # at the request boundary, so we only need a bool() coercion here. When the + # key is absent (e.g. background paths), fall back to the plugin default. if sync_options and "auto_create_ipam" in sync_options: - _raw = sync_options["auto_create_ipam"] - auto_create_ipam_opt = _raw.strip().lower() in {"1", "true", "on"} if isinstance(_raw, str) else bool(_raw) + auto_create_ipam_opt = bool(sync_options["auto_create_ipam"]) else: from .ip_helpers import auto_create_ipam_enabled diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html index ae6013608..5d6a8eef1 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html @@ -91,6 +91,7 @@ dropdownEl.style.display = "block"; }) .catch(function (error) { + if (seq !== requestSeq) return; console.debug("Device type lookup failed:", error.message); dropdownEl.style.display = "none"; }); diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html index 2f0795125..aa3c993fa 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html @@ -89,6 +89,7 @@ dropdownEl.style.display = "block"; }) .catch(function (error) { + if (seq !== requestSeq) return; console.debug("Platform lookup failed:", error.message); dropdownEl.style.display = "none"; }); From 4bced092cd2f23160e50207e1c0e9cbdf4587618 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Sun, 31 May 2026 14:50:42 +0200 Subject: [PATCH 15/21] refactor(ipam): remove generic auto-create-IPAM-on-import feature Drops the unassigned-global-/32 pre-create behaviour from device/VM import. It added busy import side-effects, was stuck in the default VRF, and never actually set primary_ip (NetBox requires the IP be interface-assigned), so it only parked records for manual wiring. Replaced (next commit) by Primary-IP auto-match on the IP-sync tab. Removed: ip_helpers.py + tests, migration 0011 + the auto_create_ipam_default model/form field, resolve_auto_create_ipam, the import-utils re-export; the pre-create blocks + created_ips plumbing in device_operations/vm_operations/ bulk_import; the import-page and settings-page toggles + their JS; and Kept: PR #84 server_key inputs + .catch stale-lookup guards, the PlatformMapping duplicate guard, and the htmx_toasts success/failed/skipped mechanism. oob-sync still references the removed helper for OOB/promote; that is reworked to interface-assigned IPs in a later commit on feat/oob-sync. --- netbox_librenms_plugin/forms.py | 12 -- .../import_utils/__init__.py | 1 - .../import_utils/bulk_import.py | 1 - .../import_utils/device_operations.py | 30 --- .../import_utils/ip_helpers.py | 112 ----------- .../import_utils/vm_operations.py | 31 --- ...brenmssettings_auto_create_ipam_default.py | 23 --- netbox_librenms_plugin/models.py | 9 - .../js/librenms_import.js | 2 - .../htmx/_dt_mapping_form.html | 2 +- .../htmx/_platform_mapping_form.html | 2 +- .../librenms_import.html | 13 +- .../netbox_librenms_plugin/settings.html | 17 +- .../tests/test_ip_helpers.py | 178 ------------------ netbox_librenms_plugin/utils.py | 53 ------ .../views/imports/actions.py | 23 --- netbox_librenms_plugin/views/imports/list.py | 5 +- 17 files changed, 5 insertions(+), 509 deletions(-) delete mode 100644 netbox_librenms_plugin/import_utils/ip_helpers.py delete mode 100644 netbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.py delete mode 100644 netbox_librenms_plugin/tests/test_ip_helpers.py diff --git a/netbox_librenms_plugin/forms.py b/netbox_librenms_plugin/forms.py index e144da69e..1dbc10c87 100644 --- a/netbox_librenms_plugin/forms.py +++ b/netbox_librenms_plugin/forms.py @@ -162,24 +162,12 @@ class ImportSettingsForm(NetBoxModelForm): help_text="Remove domain suffix from device names during import", ) - auto_create_ipam_default = forms.BooleanField( - label="Auto-create IPAM entries", - required=False, - widget=forms.CheckboxInput(attrs={"class": "form-check-input"}), - help_text=( - "When enabled, missing IP addresses reported by LibreNMS are auto-created " - "as global /32 (IPv4) or /128 (IPv6) IPAM records during initial import, " - "OOB-attach, and promote-to-host actions. Existing IPAM records are always reused." - ), - ) - class Meta: model = LibreNMSSettings fields = [ "vc_member_name_pattern", "use_sysname_default", "strip_domain_default", - "auto_create_ipam_default", ] def clean_vc_member_name_pattern(self): diff --git a/netbox_librenms_plugin/import_utils/__init__.py b/netbox_librenms_plugin/import_utils/__init__.py index a01eec32f..81c24025e 100644 --- a/netbox_librenms_plugin/import_utils/__init__.py +++ b/netbox_librenms_plugin/import_utils/__init__.py @@ -50,5 +50,4 @@ prefetch_vc_data_for_devices, update_vc_member_suggested_names, ) -from .ip_helpers import auto_create_ipam_enabled, get_or_create_global_ip # noqa: F401 from .vm_operations import bulk_import_vms, create_vm_from_librenms # noqa: F401 diff --git a/netbox_librenms_plugin/import_utils/bulk_import.py b/netbox_librenms_plugin/import_utils/bulk_import.py index 7804acd77..93962432e 100644 --- a/netbox_librenms_plugin/import_utils/bulk_import.py +++ b/netbox_librenms_plugin/import_utils/bulk_import.py @@ -200,7 +200,6 @@ def bulk_import_devices_shared( "device_id": device_id, "device": result["device"], "message": result["message"], - "created_ips": result.get("created_ips", []), } ) # Log progress after each successful import diff --git a/netbox_librenms_plugin/import_utils/device_operations.py b/netbox_librenms_plugin/import_utils/device_operations.py index 9a6b58136..6b87935b0 100644 --- a/netbox_librenms_plugin/import_utils/device_operations.py +++ b/netbox_librenms_plugin/import_utils/device_operations.py @@ -750,12 +750,10 @@ def import_single_device( 'cables': int, 'ip_addresses': int }, - 'created_ips': list[str] } """ try: api = LibreNMSAPI(server_key=server_key) - created_ips: list[str] = [] # Use pre-fetched device data if provided, otherwise fetch from API if libre_device is None: @@ -767,7 +765,6 @@ def import_single_device( "message": "", "error": f"Failed to retrieve device {device_id} from LibreNMS", "synced": {}, - "created_ips": [], } # Validate device if validation not provided @@ -790,7 +787,6 @@ def import_single_device( "message": "", "error": f"Device already exists: {validation['existing_device'].name}", "synced": {}, - "created_ips": [], } # Use validation-derived matches, allow manual mappings to override specific fields @@ -823,7 +819,6 @@ def import_single_device( "message": "", "error": "Site is required but not provided", "synced": {}, - "created_ips": [], } if not device_type: return { @@ -832,7 +827,6 @@ def import_single_device( "message": "", "error": "Device type is required but not provided", "synced": {}, - "created_ips": [], } if not device_role: return { @@ -841,7 +835,6 @@ def import_single_device( "message": "", "error": "Device role is required but not provided", "synced": {}, - "created_ips": [], } # Create device in NetBox @@ -901,27 +894,6 @@ def import_single_device( device.full_clean() device.save() - # Pre-create the LibreNMS-known IP in IPAM (global /32 or /128) - # so the user can later attach it to an interface and assign as - # primary_ip4/6. We do not auto-set primary_ip4 here because - # NetBox's Device.clean() requires the IP be assigned to one of - # the device's interfaces, which doesn't exist on a fresh import. - primary_ip = libre_device.get("ip") - if primary_ip: - from .ip_helpers import auto_create_ipam_enabled, get_or_create_global_ip - - # Already normalised to a bool by resolve_auto_create_ipam() at - # the request boundary; coerce defensively and fall back to the - # plugin default when the key is absent (e.g. background paths). - _opts = sync_options or {} - if "auto_create_ipam" in _opts: - _auto_create = bool(_opts.get("auto_create_ipam")) - else: - _auto_create = auto_create_ipam_enabled() - _ip, was_created = get_or_create_global_ip(primary_ip, auto_create=_auto_create) - if was_created and _ip is not None: - created_ips.append(str(_ip.address.ip)) - # Sync additional data based on options sync_options = sync_options or {} synced = {"interfaces": 0, "cables": 0, "ip_addresses": 0} @@ -951,7 +923,6 @@ def import_single_device( "message": f"Successfully imported device: {device.name}", "error": None, "synced": synced, - "created_ips": created_ips, } except Exception as e: @@ -962,7 +933,6 @@ def import_single_device( "message": "", "error": str(e), "synced": {}, - "created_ips": [], } diff --git a/netbox_librenms_plugin/import_utils/ip_helpers.py b/netbox_librenms_plugin/import_utils/ip_helpers.py deleted file mode 100644 index d8c9a024d..000000000 --- a/netbox_librenms_plugin/import_utils/ip_helpers.py +++ /dev/null @@ -1,112 +0,0 @@ -""" -Helpers for ensuring LibreNMS-known IP addresses exist in NetBox IPAM. - -When the plugin links a LibreNMS device to a NetBox device (during initial -import, OOB attach, or promote-to-host), the IP that LibreNMS reaches the -device on may not yet exist in NetBox. To let users later assign that IP as -``primary_ip4`` / ``primary_ip6`` / ``oob_ip``, we first need an -``IPAddress`` record. This module centralises that logic so all import -paths behave the same way. - -The helper never overwrites an existing IPAM record: if an ``IPAddress`` -already exists for the host (matched via ``net_host`` so any prefix length -is acceptable), it is returned as-is. Only when no record exists is a new -``/32`` (IPv4) or ``/128`` (IPv6) entry created in the global scope. -""" - -from __future__ import annotations - -import logging -from ipaddress import ip_address as _ipaddr_parse -from typing import TYPE_CHECKING - -if TYPE_CHECKING: # pragma: no cover - import only for type hints - from ipam.models import IPAddress - -logger = logging.getLogger(__name__) - - -def auto_create_ipam_enabled() -> bool: - """Return the value of the ``auto_create_ipam_default`` plugin setting. - - Defaults to ``False`` if the settings row does not exist or the field is - missing (e.g. during a migration). All callers should consult this before - asking ``get_or_create_global_ip(..., auto_create=True)`` so the user's - opt-in choice on the plugin settings page is honoured. - """ - try: - from netbox_librenms_plugin.models import LibreNMSSettings - - settings = LibreNMSSettings.objects.first() - return bool(getattr(settings, "auto_create_ipam_default", False)) if settings else False - except Exception: # pragma: no cover - defensive (migrations / startup) - return False - - -def get_or_create_global_ip(ip_str: str | None, *, auto_create: bool = True) -> "tuple[IPAddress | None, bool]": - """Return ``(ipam_record, created)`` for ``ip_str``. - - Creates a ``/32`` (IPv4) or ``/128`` (IPv6) global-scope record if no - matching ``IPAddress`` exists (matched via ``net_host`` so any prefix - length is acceptable). The returned ``IPAddress`` is unassigned (no - ``assigned_object``) and has no VRF. Callers may attach it to an - interface or assign it as ``oob_ip`` afterwards. - - The ``created`` flag is ``True`` only when a new IPAM record was - inserted by this call, so callers can surface a user-visible toast - only on creation (and stay silent when reusing an existing record). - - When ``auto_create`` is ``False``, no new record is ever inserted: - only existing records are returned. This lets callers honour the - ``auto_create_ipam_default`` plugin setting without having to - duplicate the lookup logic. - - Returns ``(None, False)`` if ``ip_str`` is empty, malformed, if - ``auto_create=False`` and no record exists, or if creation fails - (the failure is logged but never raised, so callers can treat this - as best-effort). - """ - if not ip_str: - return None, False - ip_str = ip_str.strip() - if not ip_str: - return None, False - - try: - parsed = _ipaddr_parse(ip_str) - except ValueError: - logger.debug("get_or_create_global_ip: invalid IP %r", ip_str) - return None, False - - from django.db import IntegrityError, transaction - - from ipam.models import IPAddress - - existing = IPAddress.objects.filter(address__net_host=ip_str, vrf__isnull=True).first() - if existing is not None: - return existing, False - - if not auto_create: - return None, False - - mask = "/128" if parsed.version == 6 else "/32" - try: - with transaction.atomic(): - return IPAddress.objects.create(address=f"{ip_str}{mask}", status="active"), True - except IntegrityError: - # Concurrent create won the race; re-query the global record and return it. - existing = IPAddress.objects.filter(address__net_host=ip_str, vrf__isnull=True).first() - if existing is not None: - return existing, False - logger.warning( - "get_or_create_global_ip: IntegrityError but no global record found for %s", - ip_str, - ) - return None, False - except Exception: # pragma: no cover - defensive (validation etc.) - logger.warning( - "get_or_create_global_ip: failed to auto-create %s", - ip_str, - exc_info=True, - ) - return None, False diff --git a/netbox_librenms_plugin/import_utils/vm_operations.py b/netbox_librenms_plugin/import_utils/vm_operations.py index 1970bf1a7..99ae10a8a 100644 --- a/netbox_librenms_plugin/import_utils/vm_operations.py +++ b/netbox_librenms_plugin/import_utils/vm_operations.py @@ -22,7 +22,6 @@ def create_vm_from_librenms( use_sysname: bool = True, strip_domain: bool = False, role=None, - auto_create_ipam: bool | None = None, ): """ Create a NetBox VirtualMachine from LibreNMS device data. @@ -85,25 +84,6 @@ def create_vm_from_librenms( set_librenms_device_id(vm, librenms_device_id, server_key) vm.save() - # Pre-create the LibreNMS-known IP in IPAM (global /32 or /128) so - # the user can later attach it to a VM interface and assign it as - # primary_ip4/6. We do not auto-set primary_ip4 here because - # VirtualMachine.clean() requires the IP be assigned to one of the - # VM's interfaces, which doesn't exist on a fresh import. - primary_ip = libre_device.get("ip") - if primary_ip: - from .ip_helpers import auto_create_ipam_enabled, get_or_create_global_ip - - if auto_create_ipam is None: - _auto_create = auto_create_ipam_enabled() - elif isinstance(auto_create_ipam, str): - _auto_create = auto_create_ipam.strip().lower() in {"1", "true", "on"} - else: - _auto_create = bool(auto_create_ipam) - _ip, was_created = get_or_create_global_ip(primary_ip, auto_create=_auto_create) - if was_created and _ip is not None: - vm._librenms_created_ips = [str(_ip.address.ip)] - logger.info(f"Created VM {vm.name} (ID: {vm.pk}) from LibreNMS device {libre_device['device_id']}") return vm @@ -174,15 +154,6 @@ def bulk_import_vms( # Resolve options once before the loop — they do not change per-VM use_sysname_opt = sync_options.get("use_sysname", True) if sync_options else True strip_domain_opt = sync_options.get("strip_domain", False) if sync_options else False - # The flag is already normalised to a bool by resolve_auto_create_ipam() - # at the request boundary, so we only need a bool() coercion here. When the - # key is absent (e.g. background paths), fall back to the plugin default. - if sync_options and "auto_create_ipam" in sync_options: - auto_create_ipam_opt = bool(sync_options["auto_create_ipam"]) - else: - from .ip_helpers import auto_create_ipam_enabled - - auto_create_ipam_opt = auto_create_ipam_enabled() for idx, vm_id in enumerate(vm_ids, start=1): # Check for job cancellation before first VM and every 5 thereafter @@ -270,7 +241,6 @@ def bulk_import_vms( use_sysname=use_sysname_opt, strip_domain=strip_domain_opt, server_key=api.server_key, - auto_create_ipam=auto_create_ipam_opt, ) result["success"].append( @@ -278,7 +248,6 @@ def bulk_import_vms( "device_id": vm_id, "device": vm, "message": f"VM {vm.name} created successfully", - "created_ips": getattr(vm, "_librenms_created_ips", []), } ) log.info(f"Successfully imported VM {vm.name} (ID: {vm_id})") diff --git a/netbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.py b/netbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.py deleted file mode 100644 index ec6102dbd..000000000 --- a/netbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.py +++ /dev/null @@ -1,23 +0,0 @@ -from django.db import migrations, models - - -class Migration(migrations.Migration): - dependencies = [ - ("netbox_librenms_plugin", "0010_inventory_and_mapping_models"), - ] - - operations = [ - migrations.AddField( - model_name="librenmssettings", - name="auto_create_ipam_default", - field=models.BooleanField( - default=False, - help_text=( - "When enabled, missing IP addresses reported by LibreNMS are " - "auto-created as global /32 (IPv4) or /128 (IPv6) IPAM records " - "during initial import, OOB-attach, and promote-to-host actions. " - "Existing IPAM records are always reused." - ), - ), - ), - ] diff --git a/netbox_librenms_plugin/models.py b/netbox_librenms_plugin/models.py index 8453d7ef4..16c8b6895 100644 --- a/netbox_librenms_plugin/models.py +++ b/netbox_librenms_plugin/models.py @@ -79,15 +79,6 @@ class LibreNMSSettings(models.Model): help_text="Remove domain suffix from device names during import", ) - auto_create_ipam_default = models.BooleanField( - default=False, - help_text=( - "When enabled, missing IP addresses reported by LibreNMS are auto-created " - "as global /32 (IPv4) or /128 (IPv6) IPAM records during initial import, " - "OOB-attach, and promote-to-host actions. Existing IPAM records are always reused." - ), - ) - def save(self, *args, **kwargs): self.pk = 1 super().save(*args, **kwargs) diff --git a/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js b/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js index 076022df1..edfda206a 100644 --- a/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js +++ b/netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js @@ -351,10 +351,8 @@ function initializeTogglePrefs() { const sysname = document.getElementById('use-sysname-toggle-cb'); const strip = document.getElementById('strip-domain-toggle-cb'); - const ipam = document.getElementById('auto-create-ipam-toggle-cb'); if (sysname) sysname.addEventListener('change', function () { savePref('use_sysname', this.checked); }); if (strip) strip.addEventListener('change', function () { savePref('strip_domain', this.checked); }); - if (ipam) ipam.addEventListener('change', function () { savePref('auto_create_ipam', this.checked); }); } // ============================================ diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html index 5d6a8eef1..ee846ed00 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html @@ -11,7 +11,7 @@ {% endcomment %} {% csrf_token %} diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html index aa3c993fa..9de8f381b 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html @@ -9,7 +9,7 @@ {% endcomment %} {% csrf_token %} diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.html index 288ddfb0b..da9e4a319 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.html @@ -369,23 +369,12 @@
Strip domain -
- - - - - -
+
+ + + +
{% if ip_sync.cache_expiry %}
@@ -74,6 +84,25 @@
+ {% else %}
diff --git a/netbox_librenms_plugin/tests/test_coverage_sync_views.py b/netbox_librenms_plugin/tests/test_coverage_sync_views.py index 552b67e25..131b8abc0 100644 --- a/netbox_librenms_plugin/tests/test_coverage_sync_views.py +++ b/netbox_librenms_plugin/tests/test_coverage_sync_views.py @@ -2583,3 +2583,92 @@ def test_no_vlans_created_shows_warning(self): with patch.object(view, "_redirect", return_value=MagicMock()): view._handle_create_vlans(req, mock_obj, "device", 1) mock_msg.warning.assert_called_once() + + +class TestSyncIPAddressesViewSetPrimaryIp: + """Phase 1: auto-match the LibreNMS management IP and set it as Primary IP.""" + + def _setup_view(self): + from netbox_librenms_plugin.views.sync.ip_addresses import SyncIPAddressesView + + view = _make_view(SyncIPAddressesView) + view._post_server_key = "default" + return view + + def test_same_host(self): + from netbox_librenms_plugin.views.sync.ip_addresses import SyncIPAddressesView as V + + assert V._same_host("10.0.0.1", "10.0.0.1") is True + assert V._same_host("10.0.0.1", "10.0.0.2") is False + assert V._same_host("not-an-ip", "10.0.0.1") is False + # IPv6 equality across differing textual forms + assert V._same_host("2001:db8::1", "2001:0db8:0000:0000:0000:0000:0000:0001") is True + + def test_set_primary_ip_sets_ipv4_and_is_idempotent(self): + from netbox_librenms_plugin.views.sync.ip_addresses import SyncIPAddressesView as V + + ip_obj = MagicMock(family=4, pk=42) + obj = MagicMock() + obj.primary_ip4_id = None + assert V._set_primary_ip(obj, ip_obj) is True + assert obj.primary_ip4 is ip_obj + obj.save.assert_called_once() + + # Already pointing at this IP -> no change, no extra save + obj.save.reset_mock() + obj.primary_ip4_id = 42 + assert V._set_primary_ip(obj, ip_obj) is False + obj.save.assert_not_called() + + def test_set_primary_ip_uses_v6_field(self): + from netbox_librenms_plugin.views.sync.ip_addresses import SyncIPAddressesView as V + + ip_obj = MagicMock(family=6, pk=7) + obj = MagicMock() + obj.primary_ip6_id = None + assert V._set_primary_ip(obj, ip_obj) is True + assert obj.primary_ip6 is ip_obj + + def _run_process(self, view, cached, *, mgmt_ip, set_primary=True, interface=True): + selected = ["10.0.0.1"] + created_ip = MagicMock(family=4, pk=42) + obj = MagicMock() + obj.primary_ip4_id = None + with patch("netbox_librenms_plugin.views.sync.ip_addresses.resolve_set_primary_ip", return_value=set_primary): + with patch.object(view, "get_management_ip", return_value=mgmt_ip) as mock_mgmt: + with patch("netbox_librenms_plugin.views.sync.ip_addresses.transaction", _atomic_txn()): + with patch("netbox_librenms_plugin.views.sync.ip_addresses.IPAddress") as mock_ip_cls: + mock_ip_cls.objects.filter.return_value.first.return_value = None + mock_ip_cls.objects.create.return_value = created_ip + with patch("netbox_librenms_plugin.views.sync.ip_addresses.Interface") as mock_iface_cls: + mock_iface_cls.objects.get.return_value = MagicMock() + with patch.object(view, "get_vrf_selection", return_value=None): + results = view.process_ip_sync(view.request, selected, cached, obj, "device") + return results, obj, created_ip, mock_mgmt + + def test_primary_set_when_matched_and_interface_assigned(self): + view = self._setup_view() + cached = [{"ip_address": "10.0.0.1", "ip_with_mask": "10.0.0.1/24", "interface_url": "/api/dcim/interfaces/5/"}] + results, obj, created_ip, _ = self._run_process(view, cached, mgmt_ip="10.0.0.1") + assert results["primary_set"] == ["10.0.0.1"] + assert obj.primary_ip4 is created_ip + + def test_primary_skipped_when_no_interface(self): + view = self._setup_view() + cached = [{"ip_address": "10.0.0.1", "ip_with_mask": "10.0.0.1/24", "interface_url": None}] + results, obj, _, _ = self._run_process(view, cached, mgmt_ip="10.0.0.1") + assert results["primary_set"] == [] + obj.save.assert_not_called() + + def test_primary_skipped_when_ip_does_not_match_mgmt(self): + view = self._setup_view() + cached = [{"ip_address": "10.0.0.1", "ip_with_mask": "10.0.0.1/24", "interface_url": "/api/dcim/interfaces/5/"}] + results, obj, _, _ = self._run_process(view, cached, mgmt_ip="10.9.9.9") + assert results["primary_set"] == [] + + def test_toggle_off_skips_mgmt_lookup_and_primary(self): + view = self._setup_view() + cached = [{"ip_address": "10.0.0.1", "ip_with_mask": "10.0.0.1/24", "interface_url": "/api/dcim/interfaces/5/"}] + results, obj, _, mock_mgmt = self._run_process(view, cached, mgmt_ip="10.0.0.1", set_primary=False) + assert results["primary_set"] == [] + mock_mgmt.assert_not_called() diff --git a/netbox_librenms_plugin/tests/test_import_utils.py b/netbox_librenms_plugin/tests/test_import_utils.py index 27551436f..f7d8620c0 100644 --- a/netbox_librenms_plugin/tests/test_import_utils.py +++ b/netbox_librenms_plugin/tests/test_import_utils.py @@ -5895,3 +5895,70 @@ def _capture_create(**kwargs): create_virtual_chassis_with_members(master_device, members_info, {"device_id": 1}) assert mock_device_cls.objects.create.call_count == 1 + + +class TestResolveSetPrimaryIp: + """Phase 1: resolve_set_primary_ip cascade (POST/GET toggle -> user pref -> False).""" + + def _make_request(self, post=None, get=None): + from unittest.mock import MagicMock + + request = MagicMock() + request.POST = post or {} + request.GET = get or {} + request.user = MagicMock() + return request + + def test_defaults_false_when_nothing_set(self): + from unittest.mock import patch + + from netbox_librenms_plugin.utils import resolve_set_primary_ip + + request = self._make_request() + with patch("netbox_librenms_plugin.utils.get_user_pref", return_value=None): + assert resolve_set_primary_ip(request) is False + + def test_post_toggle_on(self): + from unittest.mock import patch + + from netbox_librenms_plugin.utils import resolve_set_primary_ip + + request = self._make_request(post={"set-primary-ip-toggle": "on"}) + with patch("netbox_librenms_plugin.utils.get_user_pref", return_value=None): + assert resolve_set_primary_ip(request) is True + + def test_post_toggle_off_overrides_pref(self): + from unittest.mock import patch + + from netbox_librenms_plugin.utils import resolve_set_primary_ip + + request = self._make_request(post={"set-primary-ip-toggle": "off"}) + with patch("netbox_librenms_plugin.utils.get_user_pref", return_value=True): + assert resolve_set_primary_ip(request) is False + + def test_underscore_key_variant(self): + from unittest.mock import patch + + from netbox_librenms_plugin.utils import resolve_set_primary_ip + + request = self._make_request(post={"set_primary_ip": "1"}) + with patch("netbox_librenms_plugin.utils.get_user_pref", return_value=None): + assert resolve_set_primary_ip(request) is True + + def test_get_used_when_not_in_post(self): + from unittest.mock import patch + + from netbox_librenms_plugin.utils import resolve_set_primary_ip + + request = self._make_request(get={"set-primary-ip-toggle": "true"}) + with patch("netbox_librenms_plugin.utils.get_user_pref", return_value=None): + assert resolve_set_primary_ip(request) is True + + def test_user_pref_bool_used_when_no_toggle(self): + from unittest.mock import patch + + from netbox_librenms_plugin.utils import resolve_set_primary_ip + + request = self._make_request() + with patch("netbox_librenms_plugin.utils.get_user_pref", return_value=True): + assert resolve_set_primary_ip(request) is True diff --git a/netbox_librenms_plugin/utils.py b/netbox_librenms_plugin/utils.py index 8da32ed15..2556b09c1 100644 --- a/netbox_librenms_plugin/utils.py +++ b/netbox_librenms_plugin/utils.py @@ -363,6 +363,43 @@ def _is_truthy(val): return use_sysname, strip_domain +def resolve_set_primary_ip(request) -> bool: + """Resolve the "set Primary IP from the LibreNMS management IP" flag. + + Cascade (mirrors :func:`resolve_naming_preferences`, minus a settings + default since this is a per-sync action choice): + + 1. POST/GET ``set-primary-ip-toggle`` (or ``set_primary_ip``) wins + -- set by the IP-sync tab toggle. + 2. Otherwise the user's saved preference + ``plugins.netbox_librenms_plugin.set_primary_ip``. + 3. Otherwise ``False`` (opt-in). + + When enabled, :class:`SyncIPAddressesView` sets ``primary_ip4``/``primary_ip6`` + on the device/VM for the synced IP that matches the LibreNMS management IP, + provided that IP ends up assigned to one of the object's interfaces. + """ + _TRUTHY = frozenset({"on", "true", "1"}) + _KEYS = ("set-primary-ip-toggle", "set_primary_ip-toggle", "set_primary_ip") + + def _is_truthy(val): + return val.lower() in _TRUTHY if val is not None else False + + post_val = next((request.POST.get(k) for k in _KEYS if k in request.POST), None) + get_val = next((request.GET.get(k) for k in _KEYS if k in request.GET), None) + + if post_val is not None: + return _is_truthy(post_val) + if get_val is not None: + return _is_truthy(get_val) + + pref = get_user_pref(request, "plugins.netbox_librenms_plugin.set_primary_ip") + if pref is not None: + return _is_truthy(pref) if isinstance(pref, str) else bool(pref) + + return False + + def get_interface_name_field(request: Optional[HttpRequest] = None) -> str: """ Get interface name field with request override support. diff --git a/netbox_librenms_plugin/views/base/ip_addresses_view.py b/netbox_librenms_plugin/views/base/ip_addresses_view.py index a11edc82f..8188bb3f1 100644 --- a/netbox_librenms_plugin/views/base/ip_addresses_view.py +++ b/netbox_librenms_plugin/views/base/ip_addresses_view.py @@ -12,7 +12,7 @@ from virtualization.models import VirtualMachine from netbox_librenms_plugin.tables.ipaddresses import IPAddressTable -from netbox_librenms_plugin.utils import get_interface_name_field, get_librenms_device_id +from netbox_librenms_plugin.utils import get_interface_name_field, get_librenms_device_id, resolve_set_primary_ip from netbox_librenms_plugin.views.mixins import CacheMixin, LibreNMSAPIMixin, LibreNMSPermissionMixin logger = logging.getLogger(__name__) @@ -275,6 +275,7 @@ def _prepare_context(self, request, obj, interface_name_field, fetch_fresh=False "object": obj, "cache_expiry": cache_expiry, "server_key": server_key, + "set_primary_ip": resolve_set_primary_ip(request), } def get_context_data(self, request, obj): diff --git a/netbox_librenms_plugin/views/imports/actions.py b/netbox_librenms_plugin/views/imports/actions.py index 3fcc783f9..f0ccd016c 100644 --- a/netbox_librenms_plugin/views/imports/actions.py +++ b/netbox_librenms_plugin/views/imports/actions.py @@ -1812,6 +1812,7 @@ class SaveUserPrefView(LibreNMSPermissionMixin, View): ALLOWED_PREFS = { "use_sysname": "plugins.netbox_librenms_plugin.use_sysname", "strip_domain": "plugins.netbox_librenms_plugin.strip_domain", + "set_primary_ip": "plugins.netbox_librenms_plugin.set_primary_ip", "interface_name_field": "plugins.netbox_librenms_plugin.interface_name_field", } diff --git a/netbox_librenms_plugin/views/sync/ip_addresses.py b/netbox_librenms_plugin/views/sync/ip_addresses.py index 3fb43820c..63e899c8d 100644 --- a/netbox_librenms_plugin/views/sync/ip_addresses.py +++ b/netbox_librenms_plugin/views/sync/ip_addresses.py @@ -1,3 +1,4 @@ +from ipaddress import ip_address as _ip_addr from urllib.parse import quote_plus from dcim.models import Device, Interface @@ -11,6 +12,7 @@ from ipam.models import VRF, IPAddress from virtualization.models import VirtualMachine, VMInterface +from netbox_librenms_plugin.utils import resolve_set_primary_ip from netbox_librenms_plugin.views.mixins import ( CacheMixin, LibreNMSAPIMixin, @@ -100,9 +102,57 @@ def post(self, request, object_type, pk): return redirect(self.get_ip_tab_url(obj)) + def get_management_ip(self, obj): + """Return the LibreNMS management/polling IP for *obj*, or None. + + Used to decide which synced IP (if any) should become the object's + Primary IP. Best-effort: any lookup failure yields None so the sync + itself is never blocked. + """ + try: + librenms_id = self.librenms_api.get_librenms_id(obj) + if not librenms_id: + return None + success, info = self.librenms_api.get_device_info(librenms_id) + if not success or not isinstance(info, dict): + return None + return (info.get("ip") or "").strip() or None + except Exception: # pragma: no cover - defensive + return None + + @staticmethod + def _same_host(a, b): + """True if two address strings refer to the same host IP.""" + try: + return _ip_addr(a) == _ip_addr(b) + except ValueError: + return False + + @staticmethod + def _set_primary_ip(obj, ip_obj): + """Point obj.primary_ip4/6 (by family) at *ip_obj*. Returns True if changed. + + The caller guarantees ``ip_obj`` is assigned to one of the object's + interfaces, so this satisfies NetBox's ``primary_ip`` constraint. + """ + field = "primary_ip6" if ip_obj.family == 6 else "primary_ip4" + if getattr(obj, f"{field}_id") == ip_obj.pk: + return False + setattr(obj, field, ip_obj) + obj.save() + return True + def process_ip_sync(self, request, selected_ips, cached_ips, obj, object_type): - """Create or update IP addresses in NetBox from cached LibreNMS data.""" - results = {"created": [], "updated": [], "unchanged": [], "failed": []} + """Create or update IP addresses in NetBox from cached LibreNMS data. + + When the "set Primary IP" toggle is on, the synced IP that matches the + LibreNMS management IP — and ends up assigned to one of the object's + interfaces — is also set as the object's ``primary_ip4``/``primary_ip6``. + """ + results = {"created": [], "updated": [], "unchanged": [], "failed": [], "primary_set": []} + + set_primary = resolve_set_primary_ip(request) + mgmt_ip = self.get_management_ip(obj) if set_primary else None with transaction.atomic(): for ip_address in selected_ips: @@ -131,8 +181,9 @@ def process_ip_sync(self, request, selected_ips, cached_ips, obj, object_type): results["updated"].append(ip_address) else: results["unchanged"].append(ip_address) + ip_obj = existing_ip else: - IPAddress.objects.create( + ip_obj = IPAddress.objects.create( address=ip_with_mask, assigned_object=interface, status="active", @@ -140,6 +191,16 @@ def process_ip_sync(self, request, selected_ips, cached_ips, obj, object_type): ) results["created"].append(ip_address) + # Primary-IP auto-match: only when the IP is interface-assigned + # (NetBox requires that for primary_ip) and equals the mgmt IP. + if ( + mgmt_ip + and interface is not None + and self._same_host(ip_data["ip_address"], mgmt_ip) + and self._set_primary_ip(obj, ip_obj) + ): + results["primary_set"].append(ip_address) + except Exception: # pragma: no cover - defensive results["failed"].append(ip_address) @@ -151,6 +212,8 @@ def display_sync_results(self, request, results): messages.success(request, f"Created IP addresses: {', '.join(results['created'])}") if results["updated"]: messages.success(request, f"Updated IP addresses: {', '.join(results['updated'])}") + if results.get("primary_set"): + messages.success(request, f"Set as Primary IP: {', '.join(results['primary_set'])}") if results["unchanged"]: messages.warning( request, From a48ff0d13e6e9efbed0fa152df09f7760284659f Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Sun, 31 May 2026 14:50:42 +0200 Subject: [PATCH 17/21] ui(ipam): rename toggle label to 'Set Primary IP' --- .../netbox_librenms_plugin/_ipaddress_sync_content.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_ipaddress_sync_content.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_ipaddress_sync_content.html index 26baf36d8..f9fd51a8a 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_ipaddress_sync_content.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_ipaddress_sync_content.html @@ -22,7 +22,7 @@ data-bs-toggle="tooltip" title="When the synced IP equals the device's LibreNMS management IP and is assigned to an interface, set it as the Primary IP.">
From dd0c62198ab6731b006b7e9423f08068443eb3ac Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Sun, 31 May 2026 14:50:42 +0200 Subject: [PATCH 18/21] feat(ipam): auto-select the management-IP row when 'Set Primary IP' is on Flags the IP-sync row whose IP equals the device/VM's LibreNMS management IP (enrich_ip_data -> data-mgmt-ip row attribute) and auto-ticks its select box when the 'Set Primary IP' toggle is enabled, so syncing it sets the Primary IP in one action. Mirrors the parent-child row-auto-select construct (data-* attribute + JS), keyed off the LibreNMS-mapped interface. - utils.same_host: shared host-equality helper (SyncIPAddressesView._same_host now delegates to it) - BaseIPAddressTableView._flag_management_ip: marks the matching enriched entry - IPAddressTable: data-mgmt-ip row attribute - _ipaddress_sync_content.html: toggle JS now also auto-checks the mgmt row - tests for _flag_management_ip --- netbox_librenms_plugin/tables/ipaddresses.py | 1 + .../_ipaddress_sync_content.html | 20 ++++++- .../tests/test_coverage_base_views.py | 54 +++++++++++++++++++ netbox_librenms_plugin/utils.py | 10 ++++ .../views/base/ip_addresses_view.py | 32 ++++++++++- .../views/sync/ip_addresses.py | 8 +-- 6 files changed, 116 insertions(+), 9 deletions(-) diff --git a/netbox_librenms_plugin/tables/ipaddresses.py b/netbox_librenms_plugin/tables/ipaddresses.py index 8c9e1bfe2..0324c6de2 100644 --- a/netbox_librenms_plugin/tables/ipaddresses.py +++ b/netbox_librenms_plugin/tables/ipaddresses.py @@ -33,6 +33,7 @@ class Meta: row_attrs = { "data-interface": lambda record: record["ip_address"], "data-name": lambda record: record["ip_address"], + "data-mgmt-ip": lambda record: "true" if record.get("is_mgmt_ip") else "", } selection = ToggleColumn( diff --git a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_ipaddress_sync_content.html b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_ipaddress_sync_content.html index f9fd51a8a..a30a1c591 100644 --- a/netbox_librenms_plugin/templates/netbox_librenms_plugin/_ipaddress_sync_content.html +++ b/netbox_librenms_plugin/templates/netbox_librenms_plugin/_ipaddress_sync_content.html @@ -85,13 +85,29 @@ From c7acffa66b79f5e71a0901f50ac1f2b4b281f9de Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Mon, 1 Jun 2026 13:48:55 +0200 Subject: [PATCH 21/21] fix(ip-sync): scope existing-IP lookup to the target VRF filter(address=...) alone can match the same address in a different VRF and then rewrite its VRF on save, hijacking an unrelated IP. Scope the lookup to the selected VRF so a new IP is created in the target VRF instead. Addresses PR #79 CodeRabbit. --- netbox_librenms_plugin/views/sync/ip_addresses.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/netbox_librenms_plugin/views/sync/ip_addresses.py b/netbox_librenms_plugin/views/sync/ip_addresses.py index 7928438e9..e1b602af3 100644 --- a/netbox_librenms_plugin/views/sync/ip_addresses.py +++ b/netbox_librenms_plugin/views/sync/ip_addresses.py @@ -180,7 +180,10 @@ def process_ip_sync(self, request, selected_ips, cached_ips, obj, object_type): ip_with_mask = ip_data["ip_with_mask"] - existing_ip = IPAddress.objects.filter(address=ip_with_mask).first() + # Scope the lookup to the target VRF: the same address can + # legitimately exist in multiple VRFs, and matching on address + # alone would hijack an unrelated IP and rewrite its VRF. + existing_ip = IPAddress.objects.filter(address=ip_with_mask, vrf=vrf).first() if existing_ip: if existing_ip.assigned_object != interface or existing_ip.vrf != vrf: