Skip to content

Commit 66fbf7b

Browse files
authored
feat: remove module path plus add YAML export option (#34)
* docs: add design spec for remove-module-path-plus-fixes * feat: remove {module_path} feature detection support NetBox decided not to implement MODULE_PATH_TOKEN, so all related code is dead weight. Remove: - utils.py (entire file, only contained supports_module_path()) - InterfaceNameRuleListView.get_extra_context() in views.py - {%if supports_module_path%} banner in interfacenamerule_list.html - docs/configuration.md section and references - engine.py docstring mention - test_misc.py: SupportModulePathTest and UtilsModulePathFalseTest * test: add failing TDD tests for CSV export round-trip and YAML export - ModelCSVExportTest: asserts csv_headers + to_csv() exist and are correct - BulkImportCSVTest: regression for KeyError 'Ch' on CSV import - YAMLExportViewTest: asserts YAML export view returns correct response * fix: add csv_headers/to_csv() to model, fix channel_start verbose_name - InterfaceNameRule.csv_headers: list of field names matching import form - InterfaceNameRule.to_csv(): returns a tuple of values for CSV export, using .model for ModuleType/DeviceType and .name for Platform FKs - tables.py: verbose_name 'Ch. Start' → 'Channel Start' (the dot caused NetBox's import normalizer to split on '.' and produce 'Ch', triggering KeyError: 'Ch' on round-trip import) * feat: add YAML export view for all and selected rules - InterfaceNameRuleYAMLExportView: GET exports all rules, POST exports selected rules identified by pk_<n> form fields - URL: /rules/export/yaml/ (name: interfacenamerule_export_yaml) - 'Export YAML' button added to list view via extra_controls block - Empty/falsy optional fields omitted from YAML output for readability; required fields (name_template) always included * test: fix yaml_export_requires_login assertion for ConditionalLoginRequiredMixin ConditionalLoginRequiredMixin only enforces auth when LOGIN_REQUIRED=True; in the test environment it may be False so we accept 200 as well as redirects * refactor: address review feedback on YAML export and test naming - views.py: simplify _rules_to_yaml_data loop (single condition instead of if/elif); change Content-Type to standard 'application/yaml' - tests/test_views.py: rename test_yaml_export_requires_login to test_yaml_export_unauthenticated_responds with accurate docstring - .gitignore: add docs/superpowers/ (design specs are local-only) - delete docs/superpowers/specs/ design spec from git history * fix: wire YAML export POST to NetBox bulk-select form convention - post() now reads request.POST.getlist('pk') (NetBox convention) - template overrides bulk_buttons block with Export Selected YAML button using formaction to piggyback on the existing bulk-select form - update test to post {pk: [pk]} matching the new convention * fix: use reverse() for YAML export URL, add permission check, fix unauthenticated test - Replace hardcoded YAML_URL with reverse() in setUpTestData - Add view_interfacenamerule permission check to get() and post() - Add self.client.logout() to test_yaml_export_unauthenticated_responds so it actually exercises unauthenticated access - test_yaml_export_unauthenticated_responds: accept [301, 302, 403] instead of [200, 301, 302] since the view raises PermissionDenied for anonymous users - test_yaml_export_all_contains_rules: assert exact count matches DB instead of >= 1 * refactor: align utility views to NetBox base classes Replace ConditionalLoginRequiredMixin + View with proper NetBox bases: - InterfaceNameRuleDuplicateView → generic.ObjectView uses self.get_object() instead of get_object_or_404 - InterfaceNameRuleYAMLExportView → BaseMultiObjectView removes manual has_perm() checks; permission now enforced at dispatch uses self.queryset (permission-restricted) for get() and post() - RuleTestView → BaseMultiObjectView removes _check_permission() method; permission enforced at dispatch - RuleApplyListView → BaseMultiObjectView uses self.queryset for the rule listing - RuleApplicableView → generic.ObjectView uses self.get_object() instead of get_object_or_404 - RuleApplyDetailView → generic.ObjectView removes _check_permission(); uses additional_permissions for dcim.change_interface enforcement at dispatch level Also fix test_views.BulkImportCSVTest: add csv_delimiter=auto to POST data (required by BulkImportForm since NetBox added csv_delimiter field) * fix: deterministic YAML export ordering and view permission guards - Add .order_by("pk") to YAML export querysets for stable diffs - Guard rule prefill and duplicate-check lookups with view permission - Strengthen POST export test to assert returned rule identity * fix: clarify operator precedence, add warning for add-only prefill, add permission tests - Add parentheses around or-condition in _rules_to_yaml_data for clarity - Warn add-only users when rule_id prefill is skipped due to missing view permission - Add comment explaining why duplicate detection is skipped without view permission - Add tests for add-only user: blank form with warning on prefill, and save_rule skipping duplicate check * fix: re-fetch user after adding permission to clear Django cache Django caches permissions on the User instance; after user_permissions.add() the in-memory object still lacks the new permission, causing dispatch to return 403. * refactor: extract _find_existing_rule to reduce cognitive complexity SonarQube flagged _handle_save_rule at complexity 20 (limit 15). Extract the queryset filtering into _find_existing_rule (~6) bringing _handle_save_rule down to ~11. * refactor: extract PERM_VIEW_RULE constant for duplicated permission string * Revert "refactor: extract PERM_VIEW_RULE constant for duplicated permission string" This reverts commit 304ce97. * refactor: wire YAML export into NetBox's built-in Export dropdown Replace the standalone InterfaceNameRuleYAMLExportView with NetBox's native export mechanism: - Add to_yaml() to InterfaceNameRule model so NetBox offers YAML in the Export dropdown - Override export_yaml() on the list view for deterministic pk-ordered output with empty-field stripping - Remove standalone /rules/export/yaml/ URL and custom template buttons - Rewrite tests to use the list view's ?export= parameter * fix: use NetBox ObjectPermission for add-only user tests Django's built-in user_permissions.add() does not work with NetBox's ObjectPermissionRequiredMixin. Create an ObjectPermission with actions=["add"] instead. * fix: remove CSV "Current View" from Export dropdown The table CSV export uses verbose column names that don't match the import form fields, making round-trip import impossible. Replace the BulkExport action with a YAML-only variant that omits the "Current View" option, keeping only "All Data (YAML)". * fix: guard netbox.object_actions import for 4.3.x compatibility - Wrap netbox.object_actions import in try/except so the plugin loads on NetBox 4.3.7 where that module does not exist; _LIST_VIEW_ACTIONS falls back to None so ObjectListView uses its default actions - Use .restrict(user, 'view') in RuleTestView for permission-scoped lookups instead of bare objects.get() / objects.all() - URL-encode et.name in export_yaml_only.html to avoid query-string breakage on names containing &, = or # - Tighten YAML export tests: assert PK-ordered export and that no optional key carries a blank/None value * fix: use related_model detection for ObjectPermission.object_types In NetBox 4.3.7 object_types M2M targets ObjectType not ContentType. Detect the actual related model at runtime so the correct type is passed to .add() regardless of NetBox version.
1 parent f610cc5 commit 66fbf7b

11 files changed

Lines changed: 453 additions & 160 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,6 @@ site/
6060

6161
# direnv
6262
.envrc
63+
64+
# Brainstorming / design specs (local only, not for version control)
65+
docs/superpowers/

docs/configuration.md

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -49,30 +49,14 @@ NetBox installs: interface name = "5" (raw bay position)
4949
Plugin renames: interface name = "et-0/0/5"
5050
```
5151

52-
### `{module_path}` (NetBox ≥ 4.9)
53-
54-
When a module type's interface template uses `{module_path}`, NetBox resolves
55-
the full module bay path at install time. For a transceiver in "X2 Port 1"
56-
inside a line card in "Slot 1", `{module_path}` resolves to `1/1`. The plugin
57-
signal still fires, but may compute the same name the interface already has —
58-
so it becomes a no-op.
59-
60-
For directly-attached pluggables (single bay depth), `{module_path}` resolves to
61-
just the bay position, behaving identically to `{module}`.
62-
63-
!!! tip
64-
New module types should use `{module_path}` for best NetBox compatibility.
65-
Legacy module types using `{module}` continue to work — the plugin handles the rename.
66-
6752
### The `potentially-deprecated` Tag
6853

6954
After installing a module, if the plugin's signal fires but finds the interface
7055
is already correctly named, it automatically tags the rule `potentially-deprecated`.
7156
This means:
7257

7358
- For **new installs**: the rule may no longer be needed (NetBox generates the name)
74-
- For **retroactive applies**: the rule is still useful for modules installed before
75-
`{module_path}` support was added, or before the rule existed
59+
- For **retroactive applies**: the rule is still useful for modules installed before the rule existed
7660

7761
The tag is informational only — the rule remains active.
7862

netbox_interface_name_rules/engine.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -561,8 +561,7 @@ def has_applicable_interfaces(rule) -> bool:
561561
Calls find_interfaces_for_rule(limit=1) to determine if any currently installed
562562
interface would receive a new name. Returns False when:
563563
- no matching modules/interfaces are installed, OR
564-
- all matching interfaces are already correctly named (e.g. NetBox resolved
565-
{module_path} at install time, making the rule a no-op for existing interfaces).
564+
- all matching interfaces are already correctly named.
566565
567566
This is more expensive than a plain EXISTS query but ensures the Applicable
568567
column in the Apply Rules list accurately reflects "would something change?"

netbox_interface_name_rules/models.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,45 @@ def __str__(self):
283283
device = f" on {self.device_type.model}" if self.device_type else ""
284284
platform = f" [{self.platform.name}]" if self.platform else ""
285285
return f"{module}{parent}{device}{platform}{self.name_template}"
286+
287+
csv_headers = [
288+
"module_type",
289+
"module_type_pattern",
290+
"module_type_is_regex",
291+
"parent_module_type",
292+
"device_type",
293+
"platform",
294+
"name_template",
295+
"channel_count",
296+
"channel_start",
297+
"description",
298+
"enabled",
299+
"applies_to_device_interfaces",
300+
]
301+
302+
def to_csv(self):
303+
"""Return a tuple of field values for CSV export (matches csv_headers order)."""
304+
return (
305+
self.module_type.model if self.module_type else "",
306+
self.module_type_pattern,
307+
self.module_type_is_regex,
308+
self.parent_module_type.model if self.parent_module_type else "",
309+
self.device_type.model if self.device_type else "",
310+
self.platform.name if self.platform else "",
311+
self.name_template,
312+
self.channel_count,
313+
self.channel_start,
314+
self.description,
315+
self.enabled,
316+
self.applies_to_device_interfaces,
317+
)
318+
319+
def to_yaml(self):
320+
"""Return a YAML document for this rule (used by NetBox's built-in Export)."""
321+
import yaml
322+
323+
entry = {}
324+
for header, value in zip(self.csv_headers, self.to_csv()):
325+
if (value != "" and value is not None) or header in {"name_template"}:
326+
entry[header] = value
327+
return yaml.dump([entry], default_flow_style=False, allow_unicode=True, sort_keys=False)

netbox_interface_name_rules/tables.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ class InterfaceNameRuleTable(NetBoxTable):
9696
applies_to_device_interfaces = columns.BooleanColumn(verbose_name="Device Ifaces")
9797
name_template = tables.Column(verbose_name="Name Template")
9898
channel_count = tables.Column(verbose_name="Channels")
99-
channel_start = tables.Column(verbose_name="Ch. Start")
99+
channel_start = tables.Column(verbose_name="Channel Start")
100100
description = tables.Column(verbose_name="Description", linkify=False)
101101
actions = columns.ActionsColumn(
102102
actions=("edit", "delete"),
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
{# SPDX-License-Identifier: Apache-2.0 #}
2+
{# Copyright (C) 2025 Marcin Zieba <marcinpsk@gmail.com> #}
3+
{% load i18n %}
4+
<div class="dropdown">
5+
<button type="button" class="btn btn-purple dropdown-toggle" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
6+
<i class="mdi mdi-download" aria-hidden="true"></i> {{ label }}
7+
</button>
8+
<ul class="dropdown-menu dropdown-menu-end">
9+
<li><a class="dropdown-item" href="?{% if url_params %}{{ url_params }}&{% endif %}export">{% trans "All Data" %} ({{ data_format }})</a></li>
10+
{% if export_templates %}
11+
<li>
12+
<hr class="dropdown-divider">
13+
</li>
14+
{% for et in export_templates %}
15+
<li>
16+
<a class="dropdown-item" href="?{% if url_params %}{{ url_params }}&{% endif %}export={{ et.name|urlencode }}"
17+
{% if et.description %} title="{{ et.description }}"{% endif %}
18+
>
19+
{{ et.name }}
20+
</a>
21+
</li>
22+
{% endfor %}
23+
{% endif %}
24+
{% if perms.extras.add_exporttemplate %}
25+
<li>
26+
<hr class="dropdown-divider">
27+
</li>
28+
<li>
29+
<a class="dropdown-item" href="{% url 'extras:exporttemplate_add' %}?object_types={{ object_type.pk }}">{% trans "Add export template" %}...</a>
30+
</li>
31+
{% endif %}
32+
</ul>
33+
</div>

netbox_interface_name_rules/templates/netbox_interface_name_rules/interfacenamerule_list.html

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -78,23 +78,6 @@ <h6 class="mt-3">Examples</h6>
7878
(device-level rule, pattern <code>Ethernet\d+/\d+</code>)</li>
7979
</ul>
8080

81-
{% if supports_module_path %}
82-
<div class="alert alert-success mt-3 mb-0">
83-
<i class="mdi mdi-check-circle me-1"></i>
84-
<strong><code>{module_path}</code> supported:</strong>
85-
Platform naming rules (e.g., <code>et-0/0/{bay_position}</code>)
86-
may be replaceable by using <code>{module_path}</code> in module type interface templates directly.
87-
Converter offset and breakout rules are still needed.
88-
</div>
89-
{% else %}
90-
<div class="alert alert-warning mt-3 mb-0">
91-
<i class="mdi mdi-information-outline me-1"></i>
92-
<strong><code>{module_path}</code> not available:</strong>
93-
Platform naming rules
94-
(e.g., Juniper <code>et-0/0/{bay_position}</code>) are required for correct interface naming.
95-
Upgrade NetBox to get native <code>{module_path}</code> support.
96-
</div>
97-
{% endif %}
9881
</div>
9982
</div>
10083
<script>

netbox_interface_name_rules/tests/test_misc.py

Lines changed: 71 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# SPDX-License-Identifier: Apache-2.0
22
# Copyright (C) 2025 Marcin Zieba <marcinpsk@gmail.com>
3-
"""Tests for utils, jobs, model properties, and API serializer edge-cases."""
3+
"""Tests for jobs, model properties, and API serializer edge-cases."""
44

55
from unittest.mock import MagicMock, patch
66

@@ -9,21 +9,6 @@
99
from django.test import TestCase
1010

1111
from netbox_interface_name_rules.models import InterfaceNameRule
12-
from netbox_interface_name_rules.utils import supports_module_path
13-
14-
# ---------------------------------------------------------------------------
15-
# utils.py
16-
# ---------------------------------------------------------------------------
17-
18-
19-
class SupportModulePathTest(TestCase):
20-
"""Test the supports_module_path feature-detection helper."""
21-
22-
def test_returns_bool(self):
23-
"""supports_module_path() returns a boolean (True or False)."""
24-
result = supports_module_path()
25-
self.assertIsInstance(result, bool)
26-
2712

2813
# ---------------------------------------------------------------------------
2914
# jobs.py
@@ -742,31 +727,77 @@ def test_job_run_exception_reraises_and_logs(self):
742727

743728

744729
# ---------------------------------------------------------------------------
745-
# utils.py — supports_module_path ImportError path (lines 16-17)
730+
# Model csv_headers / to_csv() — regression: KeyError 'Ch' on CSV import
746731
# ---------------------------------------------------------------------------
747732

748733

749-
class UtilsModulePathFalseTest(TestCase):
750-
"""Test supports_module_path() returns False when MODULE_PATH_TOKEN is missing."""
734+
class ModelCSVExportTest(TestCase):
735+
"""Test that InterfaceNameRule exposes csv_headers and to_csv() for round-trip CSV."""
751736

752-
def test_returns_false_when_token_missing(self):
753-
"""supports_module_path() returns False when MODULE_PATH_TOKEN is absent.
754-
755-
Temporarily removes the attribute from dcim.constants (if present) to
756-
trigger the ImportError branch in supports_module_path, then restores it.
757-
Asserts False regardless of whether the attribute existed beforehand.
758-
"""
759-
import dcim.constants as dc
760-
761-
from netbox_interface_name_rules.utils import supports_module_path
762-
763-
had_attr = hasattr(dc, "MODULE_PATH_TOKEN")
764-
original = getattr(dc, "MODULE_PATH_TOKEN", None)
765-
try:
766-
if had_attr:
767-
delattr(dc, "MODULE_PATH_TOKEN")
768-
result = supports_module_path()
769-
self.assertFalse(result)
770-
finally:
771-
if had_attr:
772-
dc.MODULE_PATH_TOKEN = original
737+
@classmethod
738+
def setUpTestData(cls):
739+
manufacturer = Manufacturer.objects.create(name="CSVMfg", slug="csvmfg")
740+
cls.module_type = ModuleType.objects.create(manufacturer=manufacturer, model="CSV-SFP", part_number="CSV-SFP")
741+
cls.rule = InterfaceNameRule.objects.create(
742+
module_type=cls.module_type,
743+
name_template="et-0/0/{bay_position}",
744+
channel_count=4,
745+
channel_start=0,
746+
description="CSV test rule",
747+
)
748+
cls.regex_rule = InterfaceNameRule.objects.create(
749+
module_type_is_regex=True,
750+
module_type_pattern="QSFP-.*",
751+
name_template="{base}/{channel}",
752+
channel_count=4,
753+
channel_start=1,
754+
description="Regex CSV rule",
755+
)
756+
757+
def test_csv_headers_attribute_exists(self):
758+
"""InterfaceNameRule.csv_headers class attribute must exist."""
759+
self.assertTrue(hasattr(InterfaceNameRule, "csv_headers"), "InterfaceNameRule.csv_headers missing")
760+
761+
def test_csv_headers_is_list_or_tuple(self):
762+
"""csv_headers must be a list or tuple of strings."""
763+
self.assertIsInstance(InterfaceNameRule.csv_headers, (list, tuple))
764+
765+
def test_csv_headers_matches_import_form_fields(self):
766+
"""csv_headers must exactly match InterfaceNameRuleImportForm.Meta.fields."""
767+
from netbox_interface_name_rules.forms import InterfaceNameRuleImportForm
768+
769+
form_fields = list(InterfaceNameRuleImportForm.Meta.fields)
770+
self.assertEqual(list(InterfaceNameRule.csv_headers), form_fields)
771+
772+
def test_to_csv_method_exists(self):
773+
"""InterfaceNameRule instances must have a to_csv() method."""
774+
self.assertTrue(callable(getattr(self.rule, "to_csv", None)), "InterfaceNameRule.to_csv() missing")
775+
776+
def test_to_csv_returns_sequence(self):
777+
"""to_csv() must return a tuple or list."""
778+
result = self.rule.to_csv()
779+
self.assertIsInstance(result, (tuple, list))
780+
781+
def test_to_csv_length_matches_csv_headers(self):
782+
"""to_csv() must return exactly len(csv_headers) values."""
783+
result = self.rule.to_csv()
784+
self.assertEqual(len(result), len(InterfaceNameRule.csv_headers))
785+
786+
def test_to_csv_exact_rule_module_type(self):
787+
"""to_csv() for exact rule must include the module_type model string."""
788+
result = self.rule.to_csv()
789+
values = list(result)
790+
idx = list(InterfaceNameRule.csv_headers).index("module_type")
791+
self.assertEqual(values[idx], "CSV-SFP")
792+
793+
def test_to_csv_regex_rule_no_module_type(self):
794+
"""to_csv() for regex rule must have empty string for module_type."""
795+
result = self.regex_rule.to_csv()
796+
values = list(result)
797+
idx = list(InterfaceNameRule.csv_headers).index("module_type")
798+
self.assertEqual(values[idx], "")
799+
800+
def test_to_csv_no_dots_in_headers(self):
801+
"""csv_headers must not contain dots (regression guard for KeyError 'Ch' bug)."""
802+
for header in InterfaceNameRule.csv_headers:
803+
self.assertNotIn(".", header, f"csv_headers entry '{header}' contains a dot — would break import")

0 commit comments

Comments
 (0)