diff --git a/netbox/dcim/constants.py b/netbox/dcim/constants.py index 19663f722..56c07f505 100644 --- a/netbox/dcim/constants.py +++ b/netbox/dcim/constants.py @@ -79,40 +79,54 @@ NONCONNECTABLE_IFACE_TYPES = VIRTUAL_IFACE_TYPES + WIRELESS_IFACE_TYPES # MODULE_TOKEN = '{module}' +MODULE_PATH_TOKEN = '{module_path}' MODULE_TOKEN_SEPARATOR = '/' -def resolve_module_token(text, positions): +def resolve_module_placeholders(text, positions): """ - Substitute {module} tokens in text with position values. + Substitute {module} and {module_path} placeholders in text with position values. Args: - text: String potentially containing {module} tokens + text: String potentially containing {module} or {module_path} placeholders positions: List of position strings from the module tree (root to leaf) Returns: - Text with {module} tokens replaced according to these rules: - - Single token: replaced with full path (positions joined by MODULE_TOKEN_SEPARATOR) - - Multiple tokens: replaced level-by-level (first token gets first position, etc.) + Text with placeholders replaced according to these rules: - This centralizes the substitution logic used by both ModuleCommonForm.clean() - and ModularComponentTemplateModel.resolve_*() methods. + {module_path}: Always expands to full path (positions joined by MODULE_TOKEN_SEPARATOR). + Can only appear once in the text. + + {module}: If used once, expands to the PARENT module bay position only (last in positions). + If used multiple times, each token is replaced level-by-level. + + This design (Option 2 per sigprof's feedback) allows two approaches: + 1. Use {module_path} for automatic full-path expansion (hardcodes '/' separator) + 2. Use {module} in position fields to build custom paths with user-controlled separators """ - if not text or MODULE_TOKEN not in text: + if not text: return text - token_count = text.count(MODULE_TOKEN) + result = text - if token_count == 1: - # Single token: substitute with full path (e.g., "1/1" for depth 2) - full_path = MODULE_TOKEN_SEPARATOR.join(positions) - return text.replace(MODULE_TOKEN, full_path, 1) - else: - # Multiple tokens: substitute level-by-level (existing behavior) - result = text - for pos in positions: - result = result.replace(MODULE_TOKEN, pos, 1) - return result + # Handle {module_path} - always expands to full path + if MODULE_PATH_TOKEN in result: + full_path = MODULE_TOKEN_SEPARATOR.join(positions) if positions else '' + result = result.replace(MODULE_PATH_TOKEN, full_path) + + # Handle {module} - parent-only for single token, level-by-level for multiple + if MODULE_TOKEN in result: + token_count = result.count(MODULE_TOKEN) + if token_count == 1 and positions: + # Single {module}: substitute with parent (immediate) bay position only + parent_position = positions[-1] if positions else '' + result = result.replace(MODULE_TOKEN, parent_position, 1) + else: + # Multiple {module}: substitute level-by-level (existing behavior) + for pos in positions: + result = result.replace(MODULE_TOKEN, pos, 1) + + return result MODULAR_COMPONENT_TEMPLATE_MODELS = Q( diff --git a/netbox/dcim/forms/common.py b/netbox/dcim/forms/common.py index 0c9edaeb4..ed23f7346 100644 --- a/netbox/dcim/forms/common.py +++ b/netbox/dcim/forms/common.py @@ -119,29 +119,41 @@ class ModuleCommonForm(forms.Form): # Get the templates for the module type. for template in getattr(module_type, templates).all(): resolved_name = template.name + has_module_token = MODULE_TOKEN in template.name + has_module_path_token = MODULE_PATH_TOKEN in template.name + # Installing modules with placeholders require that the bay has a position value - if MODULE_TOKEN in template.name: + if has_module_token or has_module_path_token: if not module_bay.position: raise forms.ValidationError( _("Cannot install module with placeholder values in a module bay with no position defined.") ) - token_count = template.name.count(MODULE_TOKEN) - # A single token which gets expanded to the full path is always - # allowed; otherwise the number of tokens needs to match the path length. - if token_count != 1 and token_count != len(module_bays): - 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=token_count + # Validate {module_path} - can only appear once + if has_module_path_token: + path_token_count = template.name.count(MODULE_PATH_TOKEN) + if path_token_count > 1: + raise forms.ValidationError( + _("The {module_path} placeholder can only be used once per template.") ) - ) - # Use centralized helper for token substitution + # Validate {module} - multi-token must match depth exactly + if has_module_token: + token_count = template.name.count(MODULE_TOKEN) + # Multiple {module} tokens must match the tree depth exactly + if token_count > 1 and token_count != len(module_bays): + raise forms.ValidationError( + _( + "Cannot install module with placeholder values in a module bay tree {level} deep " + "but {tokens} placeholders given." + ).format( + level=len(module_bays), tokens=token_count + ) + ) + + # Use centralized helper for placeholder substitution positions = [mb.position for mb in module_bays] - resolved_name = resolve_module_token(resolved_name, positions) + resolved_name = resolve_module_placeholders(resolved_name, positions) 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 951f5453b..cbcd1389a 100644 --- a/netbox/dcim/models/device_component_templates.py +++ b/netbox/dcim/models/device_component_templates.py @@ -170,17 +170,17 @@ class ModularComponentTemplateModel(ComponentTemplateModel): return modules def resolve_name(self, module): - """Resolve {module} placeholder(s) in component name.""" + """Resolve {module} and {module_path} placeholders in component name.""" if module: positions = [m.module_bay.position for m in self._get_module_tree(module)] - return resolve_module_token(self.name, positions) + return resolve_module_placeholders(self.name, positions) return self.name def resolve_label(self, module): - """Resolve {module} placeholder(s) in component label.""" + """Resolve {module} and {module_path} placeholders in component label.""" if module: positions = [m.module_bay.position for m in self._get_module_tree(module)] - return resolve_module_token(self.label, positions) + return resolve_module_placeholders(self.label, positions) return self.label def resolve_position(self, position, module): @@ -194,7 +194,7 @@ class ModularComponentTemplateModel(ComponentTemplateModel): """ if module: positions = [m.module_bay.position for m in self._get_module_tree(module)] - return resolve_module_token(position, positions) + return resolve_module_placeholders(position, positions) return position diff --git a/netbox/dcim/tests/test_models.py b/netbox/dcim/tests/test_models.py index 80199c363..f788181e8 100644 --- a/netbox/dcim/tests/test_models.py +++ b/netbox/dcim/tests/test_models.py @@ -877,9 +877,11 @@ class ModuleBayTestCase(TestCase): def test_nested_module_single_placeholder_full_path(self): """ - Test that installing a module at depth=2 with a single {module} placeholder + Test that installing a module at depth=2 with a {module_path} placeholder in the interface template name resolves to the full path (e.g., "1/1"). Regression test for transceiver modeling use case. + + Updated for Option 2: Use {module_path} for full path, {module} for parent-only. """ manufacturer = Manufacturer.objects.first() site = Site.objects.first() @@ -915,15 +917,15 @@ class ModuleBayTestCase(TestCase): position='2' ) - # Create SFP module type with interface using single {module} placeholder + # Create SFP module type with interface using {module_path} for full path sfp_type = ModuleType.objects.create( manufacturer=manufacturer, model='SFP Transceiver' ) InterfaceTemplate.objects.create( module_type=sfp_type, - name='SFP {module}', - label='{module}', + name='SFP {module_path}', + label='{module_path}', type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS ) @@ -1377,15 +1379,15 @@ class ModuleBayTestCase(TestCase): position='2' ) - # Create leaf module type with single-token name AND label + # Create leaf module type with {module_path} for full path in name AND label leaf_type = ModuleType.objects.create( manufacturer=manufacturer, model='T4 Leaf' ) InterfaceTemplate.objects.create( module_type=leaf_type, - name='SFP {module}', - label='LBL {module}', + name='SFP {module_path}', + label='LBL {module_path}', type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS ) @@ -1455,8 +1457,8 @@ class ModuleBayTestCase(TestCase): ) ConsolePortTemplate.objects.create( module_type=leaf_type, - name='Console {module}', - label='{module}' + name='Console {module_path}', + label='{module_path}' ) # Create device and install modules @@ -1518,14 +1520,14 @@ class ModuleBayTestCase(TestCase): position='2' ) - # Create leaf module type with single-token interface template + # Create leaf module type with {module_path} for full path leaf_type = ModuleType.objects.create( manufacturer=manufacturer, model='T6 Leaf' ) InterfaceTemplate.objects.create( module_type=leaf_type, - name='Gi{module}', + name='Gi{module_path}', type=InterfaceTypeChoices.TYPE_1GE_FIXED ) @@ -1674,6 +1676,375 @@ class ModuleBayTestCase(TestCase): interface = leaf_module.interfaces.first() self.assertEqual(interface.name, 'X1-Y2') + # + # Option 2 Tests: {module_path} for full path, {module} for parent-only + # + + def test_module_path_placeholder_depth_1(self): + """ + Test {module_path} at depth 1 resolves to just the bay position. + """ + manufacturer = Manufacturer.objects.first() + site = Site.objects.first() + device_role = DeviceRole.objects.first() + + device_type = DeviceType.objects.create( + manufacturer=manufacturer, + model='MP Depth1 Chassis', + slug='mp-depth1-chassis' + ) + ModuleBayTemplate.objects.create( + device_type=device_type, + name='Bay 1', + position='A' + ) + + module_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='MP Depth1 Module' + ) + InterfaceTemplate.objects.create( + module_type=module_type, + name='Port {module_path}', + type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS + ) + + device = Device.objects.create( + name='MP Depth1 Device', + device_type=device_type, + role=device_role, + site=site + ) + + bay = device.modulebays.get(name='Bay 1') + module = Module.objects.create( + device=device, + module_bay=bay, + module_type=module_type + ) + + interface = module.interfaces.first() + self.assertEqual(interface.name, 'Port A') + + def test_module_path_placeholder_depth_2(self): + """ + Test {module_path} at depth 2 resolves to full path (e.g., "1/1"). + This is the key use case for SFP/transceiver modeling. + """ + manufacturer = Manufacturer.objects.first() + site = Site.objects.first() + device_role = DeviceRole.objects.first() + + device_type = DeviceType.objects.create( + manufacturer=manufacturer, + model='MP Depth2 Chassis', + slug='mp-depth2-chassis' + ) + ModuleBayTemplate.objects.create( + device_type=device_type, + name='Line Card Bay', + position='1' + ) + + line_card_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='MP Line Card' + ) + ModuleBayTemplate.objects.create( + module_type=line_card_type, + name='SFP Bay', + position='2' + ) + + sfp_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='MP SFP' + ) + InterfaceTemplate.objects.create( + module_type=sfp_type, + name='SFP {module_path}', + label='{module_path}', + type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS + ) + + device = Device.objects.create( + name='MP Depth2 Device', + device_type=device_type, + role=device_role, + site=site + ) + + # Install line card + lc_bay = device.modulebays.get(name='Line Card Bay') + line_card = Module.objects.create( + device=device, + module_bay=lc_bay, + module_type=line_card_type + ) + + # Install SFP in nested bay + sfp_bay = line_card.modulebays.get(name='SFP Bay') + sfp = Module.objects.create( + device=device, + module_bay=sfp_bay, + module_type=sfp_type + ) + + interface = sfp.interfaces.first() + # {module_path} should give full path: 1/2 + self.assertEqual(interface.name, 'SFP 1/2') + self.assertEqual(interface.label, '1/2') + + def test_module_path_placeholder_depth_3(self): + """ + Test {module_path} at depth 3 resolves to full path (e.g., "1/2/3"). + """ + manufacturer = Manufacturer.objects.first() + site = Site.objects.first() + device_role = DeviceRole.objects.first() + + device_type = DeviceType.objects.create( + manufacturer=manufacturer, + model='MP Depth3 Chassis', + slug='mp-depth3-chassis' + ) + ModuleBayTemplate.objects.create( + device_type=device_type, + name='Slot', + position='1' + ) + + level2_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='MP Level2' + ) + ModuleBayTemplate.objects.create( + module_type=level2_type, + name='SubSlot', + position='2' + ) + + level3_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='MP Level3' + ) + ModuleBayTemplate.objects.create( + module_type=level3_type, + name='Port', + position='3' + ) + + leaf_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='MP Leaf' + ) + InterfaceTemplate.objects.create( + module_type=leaf_type, + name='Interface {module_path}', + type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS + ) + + device = Device.objects.create( + name='MP Depth3 Device', + device_type=device_type, + role=device_role, + site=site + ) + + # Install 3 levels + bay1 = device.modulebays.get(name='Slot') + mod1 = Module.objects.create(device=device, module_bay=bay1, module_type=level2_type) + + bay2 = mod1.modulebays.get(name='SubSlot') + mod2 = Module.objects.create(device=device, module_bay=bay2, module_type=level3_type) + + bay3 = mod2.modulebays.get(name='Port') + mod3 = Module.objects.create(device=device, module_bay=bay3, module_type=leaf_type) + + interface = mod3.interfaces.first() + # {module_path} should give full path: 1/2/3 + self.assertEqual(interface.name, 'Interface 1/2/3') + + def test_single_module_placeholder_parent_only_depth_2(self): + """ + Test that single {module} at depth 2 resolves to PARENT position only, + not the full path. This is the key difference from {module_path}. + """ + manufacturer = Manufacturer.objects.first() + site = Site.objects.first() + device_role = DeviceRole.objects.first() + + device_type = DeviceType.objects.create( + manufacturer=manufacturer, + model='Parent Only Chassis', + slug='parent-only-chassis' + ) + ModuleBayTemplate.objects.create( + device_type=device_type, + name='Bay 1', + position='X' + ) + + line_card_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='Parent Only Line Card' + ) + ModuleBayTemplate.objects.create( + module_type=line_card_type, + name='Nested Bay', + position='Y' + ) + + leaf_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='Parent Only Leaf' + ) + InterfaceTemplate.objects.create( + module_type=leaf_type, + name='Port {module}', + type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS + ) + + device = Device.objects.create( + name='Parent Only Device', + device_type=device_type, + role=device_role, + site=site + ) + + bay1 = device.modulebays.get(name='Bay 1') + line_card = Module.objects.create(device=device, module_bay=bay1, module_type=line_card_type) + + bay2 = line_card.modulebays.get(name='Nested Bay') + leaf = Module.objects.create(device=device, module_bay=bay2, module_type=leaf_type) + + interface = leaf.interfaces.first() + # Single {module} should give PARENT position only: Y (not X/Y) + self.assertEqual(interface.name, 'Port Y') + + def test_sigprof_nested_position_no_duplication(self): + """ + Test sigprof's scenario: position field uses {module} to build path, + then child module uses {module} which should NOT cause duplication. + + Scenario: + - device + - module bay (position=`2`) + - extension module + - module bay (position=`{module}/1` → `2/1`) + - tape drive (name=`Drive {module}`) + → Should be `Drive 2/1`, NOT `Drive 2/2/1` + """ + manufacturer = Manufacturer.objects.first() + site = Site.objects.first() + device_role = DeviceRole.objects.first() + + device_type = DeviceType.objects.create( + manufacturer=manufacturer, + model='Sigprof Chassis', + slug='sigprof-chassis' + ) + ModuleBayTemplate.objects.create( + device_type=device_type, + name='Extension Slot', + position='2' + ) + + extension_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='Sigprof Extension' + ) + ModuleBayTemplate.objects.create( + module_type=extension_type, + name='Drive Bay', + position='{module}/1' # Should resolve to 2/1 + ) + + drive_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='Sigprof Drive' + ) + InterfaceTemplate.objects.create( + module_type=drive_type, + name='Drive {module}', + type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS + ) + + device = Device.objects.create( + name='Sigprof Device', + device_type=device_type, + role=device_role, + site=site + ) + + # Install extension module + ext_bay = device.modulebays.get(name='Extension Slot') + extension = Module.objects.create(device=device, module_bay=ext_bay, module_type=extension_type) + + # Verify nested bay position resolved correctly + drive_bay = extension.modulebays.get(name='Drive Bay') + self.assertEqual(drive_bay.position, '2/1') + + # Install drive module + drive = Module.objects.create(device=device, module_bay=drive_bay, module_type=drive_type) + + # Single {module} should give parent bay position: 2/1 (NOT 2/2/1) + interface = drive.interfaces.first() + self.assertEqual(interface.name, 'Drive 2/1') + + def test_module_path_validation_only_once(self): + """ + Test that {module_path} can only appear once in a template. + Using it multiple times should fail validation. + """ + from dcim.forms import ModuleForm + + manufacturer = Manufacturer.objects.first() + site = Site.objects.first() + device_role = DeviceRole.objects.first() + + device_type = DeviceType.objects.create( + manufacturer=manufacturer, + model='MP Validation Chassis', + slug='mp-validation-chassis' + ) + ModuleBayTemplate.objects.create( + device_type=device_type, + name='Bay 1', + position='1' + ) + + # Module type with {module_path} used twice - should fail + bad_module_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='Bad Module' + ) + InterfaceTemplate.objects.create( + module_type=bad_module_type, + name='{module_path}/{module_path}', + type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS + ) + + device = Device.objects.create( + name='MP Validation Device', + device_type=device_type, + role=device_role, + site=site + ) + + bay = device.modulebays.get(name='Bay 1') + + form = ModuleForm(data={ + 'device': device.pk, + 'module_bay': bay.pk, + 'module_type': bad_module_type.pk, + 'status': 'active', + 'replicate_components': True, + 'adopt_components': False, + }) + + self.assertFalse(form.is_valid()) + class CableTestCase(TestCase):