Просмотр исходного кода

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")
Mark Robert Coleman 2 дней назад
Родитель
Сommit
a06a300913

+ 20 - 0
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

+ 6 - 18
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)
 

+ 6 - 27
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)
 

+ 16 - 24
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
         )

+ 267 - 0
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"""

+ 53 - 0
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):