From a06a3009134fc18ec3c593d47697e1c015f75a6c Mon Sep 17 00:00:00 2001 From: Mark Robert Coleman Date: Thu, 2 Apr 2026 02:58:16 +0200 Subject: [PATCH] Implement {module} position inheritance for nested module bays (#21753) * Implement {module} position inheritance for nested module bays (#19796) Enables a single ModuleType to produce correctly named components at any nesting depth by resolving {module} in module bay position fields during tree traversal. The user controls the separator through the position field template itself (e.g. {module}/1 vs {module}-1 vs {module}.1). Model layer: - Add _get_inherited_positions() to resolve {module} in positions as the module tree is walked from root to leaf - Update _resolve_module_placeholder() with single-token logic: one {module} resolves to the leaf bay's inherited position; multi-token continues level-by-level replacement for backwards compatibility Form layer: - Update _get_module_bay_tree() to resolve {module} in positions during traversal, propagating parent positions through the tree - Extract validation into _validate_module_tokens() private method Tests: - Position inheritance at depth 2 and 3 - Custom separator (dot notation) - Multi-token backwards compatibility - Documentation for position inheritance Fixes: #19796 * Consolidate {module} placeholder logic into shared utilities and add API validation Extract get_module_bay_positions() and resolve_module_placeholder() into dcim/utils.py as shared routines used by the model, form, and API serializer. This eliminates duplicated traversal and resolution logic across three layers. Key changes: - Add position inheritance: {module} tokens in bay position fields resolve using the parent bay's position during hierarchy traversal - Single {module} token now resolves to the leaf bay's inherited position - Mismatched token count vs tree depth now raises ValueError instead of silently producing partial strings - API serializer validation uses shared utilities for parity with the form - Fix error message wording ("levels deep" instead of "in tree") --- docs/models/dcim/moduletype.md | 20 ++ netbox/dcim/api/serializers_/devices.py | 24 +- netbox/dcim/forms/common.py | 33 +-- .../dcim/models/device_component_templates.py | 40 ++- netbox/dcim/tests/test_models.py | 267 ++++++++++++++++++ netbox/dcim/utils.py | 53 ++++ 6 files changed, 368 insertions(+), 69 deletions(-) diff --git a/docs/models/dcim/moduletype.md b/docs/models/dcim/moduletype.md index 790e4a8de..dca2b4af4 100644 --- a/docs/models/dcim/moduletype.md +++ b/docs/models/dcim/moduletype.md @@ -32,6 +32,26 @@ For example, `{vc_position:1}` will render as `1` when no Virtual Chassis positi Automatic renaming is supported for all modular component types (those listed above). +### Position Inheritance for Nested Modules + +When using nested module bays (modules installed inside other modules), the `{module}` placeholder +can also be used in the **position** field of module bay templates to inherit the parent bay's +position. This allows a single module type to produce correctly named components at any nesting +depth, with a user-controlled separator. + +For example, a line card module type might define sub-bay positions as `{module}/1`, `{module}/2`, +etc. When the line card is installed in a device bay with position `3`, these sub-bay positions +resolve to `3/1`, `3/2`, etc. An SFP module type with interface template `SFP {module}` installed +in sub-bay `3/2` then produces interface `SFP 3/2`. + +The separator between levels is defined by the user in the position field template itself. Using +`{module}-1` produces positions like `3-1`, while `{module}.1` produces `3.1`. This provides +full flexibility without requiring a global separator configuration. + +!!! note + If the position field does not contain `{module}`, no inheritance occurs and behavior is + unchanged from previous versions. + ## Fields ### Manufacturer diff --git a/netbox/dcim/api/serializers_/devices.py b/netbox/dcim/api/serializers_/devices.py index 4da97ee78..f860c7879 100644 --- a/netbox/dcim/api/serializers_/devices.py +++ b/netbox/dcim/api/serializers_/devices.py @@ -8,6 +8,7 @@ from rest_framework import serializers from dcim.choices import * from dcim.constants import MACADDRESS_ASSIGNMENT_MODELS, MODULE_TOKEN from dcim.models import Device, DeviceBay, MACAddress, Module, VirtualDeviceContext +from dcim.utils import get_module_bay_positions, resolve_module_placeholder from extras.api.serializers_.configtemplates import ConfigTemplateSerializer from ipam.api.serializers_.ip import IPAddressSerializer from netbox.api.fields import ChoiceField, ContentTypeField, RelatedObjectCountField @@ -207,13 +208,7 @@ class ModuleSerializer(PrimaryModelSerializer): if not all([device, module_type, module_bay]): return data - # Build module bay tree for MODULE_TOKEN placeholder resolution (outermost to innermost) - module_bays = [] - current_bay = module_bay - while current_bay: - module_bays.append(current_bay) - current_bay = current_bay.module.module_bay if current_bay.module else None - module_bays.reverse() + positions = get_module_bay_positions(module_bay) for templates_attr, component_attr in [ ('consoleporttemplates', 'consoleports'), @@ -236,17 +231,10 @@ class ModuleSerializer(PrimaryModelSerializer): raise serializers.ValidationError( _("Cannot install module with placeholder values in a module bay with no position defined.") ) - if template.name.count(MODULE_TOKEN) != len(module_bays): - raise serializers.ValidationError( - _( - "Cannot install module with placeholder values in a module bay tree {level} in tree " - "but {tokens} placeholders given." - ).format( - level=len(module_bays), tokens=template.name.count(MODULE_TOKEN) - ) - ) - for bay in module_bays: - resolved_name = resolved_name.replace(MODULE_TOKEN, bay.position, 1) + try: + resolved_name = resolve_module_placeholder(template.name, positions) + except ValueError as e: + raise serializers.ValidationError(str(e)) existing_item = installed_components.get(resolved_name) diff --git a/netbox/dcim/forms/common.py b/netbox/dcim/forms/common.py index a3a781be5..f3df113b4 100644 --- a/netbox/dcim/forms/common.py +++ b/netbox/dcim/forms/common.py @@ -3,6 +3,7 @@ from django.utils.translation import gettext_lazy as _ from dcim.choices import * from dcim.constants import * +from dcim.utils import get_module_bay_positions, resolve_module_placeholder from utilities.forms import get_field_value __all__ = ( @@ -70,18 +71,6 @@ class InterfaceCommonForm(forms.Form): class ModuleCommonForm(forms.Form): - def _get_module_bay_tree(self, module_bay): - module_bays = [] - while module_bay: - module_bays.append(module_bay) - if module_bay.module: - module_bay = module_bay.module.module_bay - else: - module_bay = None - - module_bays.reverse() - return module_bays - def clean(self): super().clean() @@ -100,7 +89,7 @@ class ModuleCommonForm(forms.Form): self.instance._disable_replication = True return - module_bays = self._get_module_bay_tree(module_bay) + positions = get_module_bay_positions(module_bay) for templates, component_attribute in [ ("consoleporttemplates", "consoleports"), @@ -119,25 +108,15 @@ class ModuleCommonForm(forms.Form): # Get the templates for the module type. for template in getattr(module_type, templates).all(): resolved_name = template.name - # Installing modules with placeholders require that the bay has a position value if MODULE_TOKEN in template.name: if not module_bay.position: raise forms.ValidationError( _("Cannot install module with placeholder values in a module bay with no position defined.") ) - - if len(module_bays) != template.name.count(MODULE_TOKEN): - raise forms.ValidationError( - _( - "Cannot install module with placeholder values in a module bay tree {level} in tree " - "but {tokens} placeholders given." - ).format( - level=len(module_bays), tokens=template.name.count(MODULE_TOKEN) - ) - ) - - for module_bay in module_bays: - resolved_name = resolved_name.replace(MODULE_TOKEN, module_bay.position, 1) + try: + resolved_name = resolve_module_placeholder(template.name, positions) + except ValueError as e: + raise forms.ValidationError(str(e)) existing_item = installed_components.get(resolved_name) diff --git a/netbox/dcim/models/device_component_templates.py b/netbox/dcim/models/device_component_templates.py index 08bb6e952..fba5ce809 100644 --- a/netbox/dcim/models/device_component_templates.py +++ b/netbox/dcim/models/device_component_templates.py @@ -9,6 +9,7 @@ from dcim.choices import * from dcim.constants import * from dcim.models.base import PortMappingBase from dcim.models.mixins import InterfaceValidationMixin +from dcim.utils import get_module_bay_positions, resolve_module_placeholder from netbox.models import ChangeLoggedModel from utilities.fields import ColorField, NaturalOrderingField from utilities.mptt import TreeManager @@ -185,33 +186,27 @@ class ModularComponentTemplateModel(ComponentTemplateModel): return VC_POSITION_RE.sub(replacer, value) - def _get_module_tree(self, module): - modules = [] - while module: - modules.append(module) - if module.module_bay: - module = module.module_bay.module - else: - module = None - - modules.reverse() - return modules - - def _resolve_module_placeholder(self, value, module=None, device=None): - if MODULE_TOKEN in value and module: - modules = self._get_module_tree(module) - for m in modules: - value = value.replace(MODULE_TOKEN, m.module_bay.position, 1) - if VC_POSITION_RE.search(value) is not None: + def _resolve_all_placeholders(self, value, module=None, device=None): + has_module = MODULE_TOKEN in value + has_vc = VC_POSITION_RE.search(value) is not None + if not has_module and not has_vc: + return value + if has_module and module: + positions = get_module_bay_positions(module.module_bay) + value = resolve_module_placeholder(value, positions) + if has_vc: resolved_device = (module.device if module else None) or device value = self._resolve_vc_position(value, resolved_device) return value def resolve_name(self, module=None, device=None): - return self._resolve_module_placeholder(self.name, module, device) + return self._resolve_all_placeholders(self.name, module, device) def resolve_label(self, module=None, device=None): - return self._resolve_module_placeholder(self.label, module, device) + return self._resolve_all_placeholders(self.label, module, device) + + def resolve_position(self, module=None, device=None): + return self._resolve_all_placeholders(self.position, module, device) class ConsolePortTemplate(ModularComponentTemplateModel): @@ -745,14 +740,11 @@ class ModuleBayTemplate(ModularComponentTemplateModel): verbose_name = _('module bay template') verbose_name_plural = _('module bay templates') - def resolve_position(self, module): - return self._resolve_module_placeholder(self.position, module) - def instantiate(self, **kwargs): return self.component_model( name=self.resolve_name(kwargs.get('module'), kwargs.get('device')), label=self.resolve_label(kwargs.get('module'), kwargs.get('device')), - position=self.resolve_position(kwargs.get('module')), + position=self.resolve_position(kwargs.get('module'), kwargs.get('device')), enabled=self.enabled, **kwargs ) diff --git a/netbox/dcim/tests/test_models.py b/netbox/dcim/tests/test_models.py index 3bbc624f6..bfdaa5efe 100644 --- a/netbox/dcim/tests/test_models.py +++ b/netbox/dcim/tests/test_models.py @@ -999,6 +999,273 @@ class ModuleBayTestCase(TestCase): nested_bay = module.modulebays.get(name='Sub-bay 1-1') self.assertEqual(nested_bay.position, '1-1') + # + # Position inheritance tests (#19796) + # + + def test_position_inheritance_depth_2(self): + """ + A module bay with position '{module}/2' under a parent bay with position '1' + should resolve to position '1/2'. A single {module} in the interface template + should then resolve to '1/2'. + """ + manufacturer = Manufacturer.objects.first() + site = Site.objects.first() + device_role = DeviceRole.objects.first() + + device_type = DeviceType.objects.create( + manufacturer=manufacturer, + model='Chassis for Inheritance', + slug='chassis-for-inheritance' + ) + ModuleBayTemplate.objects.create( + device_type=device_type, + name='Line card slot 1', + position='1' + ) + + line_card_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='Line Card with Inherited Bays' + ) + ModuleBayTemplate.objects.create( + module_type=line_card_type, + name='SFP bay {module}/1', + position='{module}/1' + ) + ModuleBayTemplate.objects.create( + module_type=line_card_type, + name='SFP bay {module}/2', + position='{module}/2' + ) + + sfp_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='SFP with Inherited Path' + ) + InterfaceTemplate.objects.create( + module_type=sfp_type, + name='SFP {module}', + type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS + ) + + device = Device.objects.create( + name='Inheritance Chassis', + device_type=device_type, + role=device_role, + site=site + ) + + lc_bay = device.modulebays.get(name='Line card slot 1') + line_card = Module.objects.create( + device=device, + module_bay=lc_bay, + module_type=line_card_type + ) + + sfp_bay = line_card.modulebays.get(name='SFP bay 1/2') + sfp_module = Module.objects.create( + device=device, + module_bay=sfp_bay, + module_type=sfp_type + ) + + interface = sfp_module.interfaces.first() + self.assertEqual(interface.name, 'SFP 1/2') + + def test_position_inheritance_depth_3(self): + """ + Position inheritance at depth 3: positions should chain through the tree. + """ + manufacturer = Manufacturer.objects.first() + site = Site.objects.first() + device_role = DeviceRole.objects.first() + + device_type = DeviceType.objects.create( + manufacturer=manufacturer, + model='Deep Chassis', + slug='deep-chassis' + ) + ModuleBayTemplate.objects.create( + device_type=device_type, + name='Slot A', + position='A' + ) + + mid_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='Mid Module' + ) + ModuleBayTemplate.objects.create( + module_type=mid_type, + name='Sub {module}-1', + position='{module}-1' + ) + + leaf_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='Leaf Module' + ) + InterfaceTemplate.objects.create( + module_type=leaf_type, + name='Port {module}', + type=InterfaceTypeChoices.TYPE_1GE_FIXED + ) + + device = Device.objects.create( + name='Deep Device', + device_type=device_type, + role=device_role, + site=site + ) + + slot_a = device.modulebays.get(name='Slot A') + mid_module = Module.objects.create( + device=device, + module_bay=slot_a, + module_type=mid_type + ) + + sub_bay = mid_module.modulebays.get(name='Sub A-1') + self.assertEqual(sub_bay.position, 'A-1') + + leaf_module = Module.objects.create( + device=device, + module_bay=sub_bay, + module_type=leaf_type + ) + + interface = leaf_module.interfaces.first() + self.assertEqual(interface.name, 'Port A-1') + + def test_position_inheritance_custom_separator(self): + """ + Users control the separator through the position field template. + Using '.' instead of '/' should work correctly. + """ + manufacturer = Manufacturer.objects.first() + site = Site.objects.first() + device_role = DeviceRole.objects.first() + + device_type = DeviceType.objects.create( + manufacturer=manufacturer, + model='Dot Separator Chassis', + slug='dot-separator-chassis' + ) + ModuleBayTemplate.objects.create( + device_type=device_type, + name='Bay 1', + position='1' + ) + + card_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='Card with Dot Separator' + ) + ModuleBayTemplate.objects.create( + module_type=card_type, + name='Port {module}.1', + position='{module}.1' + ) + + sfp_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='SFP Dot' + ) + InterfaceTemplate.objects.create( + module_type=sfp_type, + name='eth{module}', + type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS + ) + + device = Device.objects.create( + name='Dot Device', + device_type=device_type, + role=device_role, + site=site + ) + + bay = device.modulebays.get(name='Bay 1') + card = Module.objects.create( + device=device, + module_bay=bay, + module_type=card_type + ) + + port_bay = card.modulebays.get(name='Port 1.1') + sfp = Module.objects.create( + device=device, + module_bay=port_bay, + module_type=sfp_type + ) + + interface = sfp.interfaces.first() + self.assertEqual(interface.name, 'eth1.1') + + def test_multi_token_backwards_compat(self): + """ + Multi-token {module}/{module} at matching depth should still resolve + level-by-level (backwards compatibility). + """ + manufacturer = Manufacturer.objects.first() + site = Site.objects.first() + device_role = DeviceRole.objects.first() + + device_type = DeviceType.objects.create( + manufacturer=manufacturer, + model='Multi Token Chassis', + slug='multi-token-chassis' + ) + ModuleBayTemplate.objects.create( + device_type=device_type, + name='Slot 1', + position='1' + ) + + card_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='Card for Multi Token' + ) + ModuleBayTemplate.objects.create( + module_type=card_type, + name='Port 1', + position='2' + ) + + iface_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='Interface Module Multi Token' + ) + InterfaceTemplate.objects.create( + module_type=iface_type, + name='Gi{module}/{module}', + type=InterfaceTypeChoices.TYPE_1GE_FIXED + ) + + device = Device.objects.create( + name='Multi Token Device', + device_type=device_type, + role=device_role, + site=site + ) + + slot = device.modulebays.get(name='Slot 1') + card = Module.objects.create( + device=device, + module_bay=slot, + module_type=card_type + ) + + port = card.modulebays.get(name='Port 1') + iface_module = Module.objects.create( + device=device, + module_bay=port, + module_type=iface_type + ) + + interface = iface_module.interfaces.first() + self.assertEqual(interface.name, 'Gi1/2') + @tag('regression') # #20912 def test_module_bay_parent_cleared_when_module_removed(self): """Test that the parent field is properly cleared when a module bay's module assignment is removed""" diff --git a/netbox/dcim/utils.py b/netbox/dcim/utils.py index 5dcc20035..ce4dd15ef 100644 --- a/netbox/dcim/utils.py +++ b/netbox/dcim/utils.py @@ -3,6 +3,59 @@ from collections import defaultdict from django.apps import apps from django.contrib.contenttypes.models import ContentType from django.db import router, transaction +from django.utils.translation import gettext as _ + +from dcim.constants import MODULE_TOKEN + + +def get_module_bay_positions(module_bay): + """ + Given a module bay, traverse up the module hierarchy and return + a list of bay position strings from root to leaf, resolving any + {module} tokens in each position using the parent position + (position inheritance). + """ + positions = [] + while module_bay: + pos = module_bay.position or '' + if positions and MODULE_TOKEN in pos: + pos = pos.replace(MODULE_TOKEN, positions[-1]) + positions.append(pos) + if module_bay.module: + module_bay = module_bay.module.module_bay + else: + module_bay = None + positions.reverse() + return positions + + +def resolve_module_placeholder(value, positions): + """ + Resolve {module} placeholder tokens in a string using the given + list of module bay positions (ordered root to leaf). + + A single {module} token resolves to the leaf (immediate parent) bay's position. + Multiple tokens must match the tree depth and resolve level-by-level. + + Returns the resolved string. + Raises ValueError if token count is greater than 1 and doesn't match tree depth. + """ + if MODULE_TOKEN not in value: + return value + + token_count = value.count(MODULE_TOKEN) + if token_count == 1: + return value.replace(MODULE_TOKEN, positions[-1]) + if token_count == len(positions): + for pos in positions: + value = value.replace(MODULE_TOKEN, pos, 1) + return value + raise ValueError( + _("Cannot install module with placeholder values in a module bay tree " + "{level} levels deep but {tokens} placeholders given.").format( + level=len(positions), tokens=token_count + ) + ) def compile_path_node(ct_id, object_id):