Browse Source

fix(dcim): Validate {module} in labels and bays

Installing a module checked {module} placeholders only in component
names, so a label or a module bay template raised a server error or
resolved to an empty string. resolve_module_placeholder() now rejects
a placeholder that refers to a bay without a position, and the API,
the module form and bulk import check every templated field with it.
Moving a module into such a bay is refused as well.

Fixes #23478
Martin Hauser 9 giờ trước cách đây
mục cha
commit
0ec43323ee

+ 1 - 1
docs/models/dcim/module.md

@@ -10,7 +10,7 @@ Similar to devices, modules are instantiated from [module types](./moduletype.md
 
 An installed module can be moved to a different module bay after creation. The destination bay must be enabled and unoccupied. Moving a module relocates its entire subtree: the components installed by the module, the module bays belonging to it, and any child modules installed within those bays.
 
-Component names, labels, and module bay positions derived from the module type's templates (for example, names containing `{module}`) are re-resolved for the destination bay. A component is renamed only when its current name matches exactly one of the module type's templates as resolved for the source bay; components whose names do not match any template resolution (including manually renamed components) are preserved as-is. All resulting names are validated against the destination device before the move is applied. A move is rejected when a template-derived name, label, or position would exceed the destination field's maximum length. A move is also rejected when a component's current value matched a template for the source bay but that template cannot be resolved for the destination bay's nesting depth.
+Component names, labels, and module bay positions derived from the module type's templates (for example, names containing `{module}`) are re-resolved for the destination bay. A component is renamed only when its current name matches exactly one of the module type's templates as resolved for the source bay; components whose names do not match any template resolution (including manually renamed components) are preserved as-is. All resulting names are validated against the destination device before the move is applied. A move is rejected when a template-derived name, label, or position would exceed the destination field's maximum length. A move is also rejected when a component's current value matched a template for the source bay but that template cannot be resolved for the destination bay, because the bay's nesting depth does not match the template's placeholders or a bay position that a placeholder refers to is not defined.
 
 Moving a module to a different device is supported only when the moved components carry no active topology or device-scoped configuration. A cross-device move is rejected while any moved component is cabled or marked as connected, has attached inventory items, or any moved interface has IP addresses, FHRP group assignments, tunnel terminations, L2VPN terminations, virtual circuit terminations, wireless links, wireless LAN assignments, VLANs (untagged, tagged, or Q-in-Q service), a VLAN translation policy, VDC assignments, or a VRF. A parent, bridge, LAG, power outlet to power port, cooling outflow to cooling intake, or front/rear port mapping relation crossing the moved module's boundary in either direction also blocks the move. MAC addresses move together with their interfaces.
 

+ 14 - 11
netbox/dcim/api/serializers_/devices.py

@@ -7,7 +7,7 @@ from drf_spectacular.utils import extend_schema_field
 from rest_framework import serializers
 
 from dcim.choices import *
-from dcim.constants import MACADDRESS_ASSIGNMENT_MODELS, MODULE_TOKEN
+from dcim.constants import MACADDRESS_ASSIGNMENT_MODELS
 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
@@ -274,16 +274,11 @@ class ModuleSerializer(PrimaryModelSerializer):
             }
 
             for template in getattr(module_type, templates_attr).all():
-                resolved_name = template.name
-                if MODULE_TOKEN in template.name:
-                    if not module_bay.position:
-                        raise serializers.ValidationError(
-                            _("Cannot install module with placeholder values in a module bay with no position defined.")
-                        )
-                    try:
-                        resolved_name = resolve_module_placeholder(template.name, positions)
-                    except ValueError as e:
-                        raise serializers.ValidationError(str(e))
+                try:
+                    resolved_name = resolve_module_placeholder(template.name, positions)
+                    resolve_module_placeholder(template.label, positions)
+                except ValueError as e:
+                    raise serializers.ValidationError(str(e)) from e
 
                 existing_item = installed_components.get(resolved_name)
 
@@ -303,6 +298,14 @@ class ModuleSerializer(PrimaryModelSerializer):
                         )
                     )
 
+        # Module bay names are unique per module, not per device, so only their placeholders are checked
+        for template in module_type.modulebaytemplates.all():
+            try:
+                for value in (template.name, template.label, template.position):
+                    resolve_module_placeholder(value, positions)
+            except ValueError as e:
+                raise serializers.ValidationError(str(e)) from e
+
         return data
 
     def create(self, validated_data):

+ 22 - 13
netbox/dcim/forms/common.py

@@ -123,12 +123,18 @@ class ModuleCommonForm(forms.Form):
         if adopt_components:
             self.instance._adopt_components = True
 
-        # Bail out if we are not installing a new module or if we are not replicating components (or if
-        # validation has already failed)
-        if self.errors or self.instance.pk or not replicate_components:
+        # Bail out if we are not installing a new module (or if validation has already failed)
+        if self.errors or self.instance.pk:
             self.instance._disable_replication = True
             return
 
+        if not replicate_components:
+            self.instance._disable_replication = True
+
+        # Adoption also instantiates every template, so skip the checks only when neither operation is requested
+        if not replicate_components and not adopt_components:
+            return
+
         try:
             positions = get_module_bay_positions(module_bay)
         except ValueError as e:
@@ -152,16 +158,11 @@ class ModuleCommonForm(forms.Form):
 
             # Get the templates for the module type.
             for template in getattr(module_type, templates).all():
-                resolved_name = template.name
-                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.")
-                        )
-                    try:
-                        resolved_name = resolve_module_placeholder(template.name, positions)
-                    except ValueError as e:
-                        raise forms.ValidationError(str(e))
+                try:
+                    resolved_name = resolve_module_placeholder(template.name, positions)
+                    resolve_module_placeholder(template.label, positions)
+                except ValueError as e:
+                    raise forms.ValidationError(str(e)) from e
 
                 existing_item = installed_components.get(resolved_name)
 
@@ -182,3 +183,11 @@ class ModuleCommonForm(forms.Form):
                             name=resolved_name
                         )
                     )
+
+        # Module bay names are unique per module, not per device, so only their placeholders are checked
+        for template in module_type.modulebaytemplates.all():
+            try:
+                for value in (template.name, template.label, template.position):
+                    resolve_module_placeholder(value, positions)
+            except ValueError as e:
+                raise forms.ValidationError(str(e)) from e

+ 1 - 1
netbox/dcim/models/device_component_templates.py

@@ -204,9 +204,9 @@ class ModularComponentTemplateModel(ComponentTemplateModel):
             # Reached only from Module._save_new(); AbortRequest is what the view/viewset catches.
             try:
                 positions = get_module_bay_positions(module.module_bay)
+                value = resolve_module_placeholder(value, positions)
             except ValueError as e:
                 raise AbortRequest(str(e)) from e
-            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)

+ 2 - 1
netbox/dcim/models/module_moves.py

@@ -307,7 +307,8 @@ class ModuleMovePlan:
     def _resolve(template, value, positions, device):
         """
         Resolve {module} and {vc_position} tokens in a template value against an explicit
-        position chain and device. Raises ValueError on a token-count mismatch.
+        position chain and device. Raises ValueError on a token-count mismatch or a missing
+        bay position.
         """
         if MODULE_TOKEN in value:
             value = resolve_module_placeholder(value, positions)

+ 134 - 21
netbox/dcim/tests/test_api.py

@@ -3901,6 +3901,7 @@ class ModuleTestCase(APIViewTestCases.APIViewTestCase):
         # eth1 should have been created
         self.assertTrue(device.interfaces.filter(name='eth1').exists())
 
+    @tag('regression')  # Issue #23478
     def test_module_token_no_position(self):
         """
         Installing a module whose type has a template with a MODULE_TOKEN placeholder into a
@@ -3909,46 +3910,158 @@ class ModuleTestCase(APIViewTestCases.APIViewTestCase):
         self.add_permissions('dcim.add_module')
         manufacturer = Manufacturer.objects.get(name='Generic')
         device = create_test_device('Device for Token No-Position Test')
-        module_type = ModuleType.objects.create(manufacturer=manufacturer, model='Token No-Position Module Type')
-        # Template name contains the MODULE_TOKEN placeholder
-        InterfaceTemplate.objects.create(
-            module_type=module_type, name=f'{MODULE_TOKEN}-eth0', type='1000base-t'
-        )
-        # Module bay has no position
-        module_bay = ModuleBay.objects.create(device=device, name='No-Position Bay')
+        url = reverse('dcim-api:module-list')
+        # One placeholder in each field that installation resolves, each on its own module type
+        templates = (
+            (InterfaceTemplate, {'name': f'{MODULE_TOKEN}-eth0', 'type': '1000base-t'}),
+            (InterfaceTemplate, {'name': 'eth0', 'label': f'Port {MODULE_TOKEN}', 'type': '1000base-t'}),
+            (ModuleBayTemplate, {'name': f'Bay {MODULE_TOKEN}'}),
+            (ModuleBayTemplate, {'name': 'Bay', 'label': f'Bay {MODULE_TOKEN}'}),
+            (ModuleBayTemplate, {'name': 'Bay', 'position': MODULE_TOKEN}),
+        )
+        for i, (template_model, attrs) in enumerate(templates, start=1):
+            with self.subTest(template=template_model.__name__, attrs=attrs):
+                module_type = ModuleType.objects.create(
+                    manufacturer=manufacturer, model=f'Token No-Position Module Type {i}'
+                )
+                template_model.objects.create(module_type=module_type, **attrs)
+                module_bay = ModuleBay.objects.create(device=device, name=f'No-Position Bay {i}')
+
+                data = {
+                    'device': device.pk,
+                    'module_bay': module_bay.pk,
+                    'module_type': module_type.pk,
+                }
+                response = self.client.post(url, data, format='json', **self.header)
+                self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+                self.assertIn('no position defined', str(response.data.get('__all__')))
+                self.assertFalse(Module.objects.filter(module_bay=module_bay).exists())
+
+    @tag('regression')  # Issue #23478
+    def test_module_token_depth_mismatch(self):
+        """
+        Installing a module whose template has more MODULE_TOKEN placeholders in a field than the
+        depth of the module bay tree should return a validation error.
+        """
+        self.add_permissions('dcim.add_module')
+        manufacturer = Manufacturer.objects.get(name='Generic')
+        device = create_test_device('Device for Token Depth Mismatch Test')
+        url = reverse('dcim-api:module-list')
+        # Two placeholders in each field that installation resolves, while every bay is one level deep
+        tokens = f'{MODULE_TOKEN}/{MODULE_TOKEN}'
+        templates = (
+            (InterfaceTemplate, {'name': f'{MODULE_TOKEN}-{MODULE_TOKEN}-eth0', 'type': '1000base-t'}),
+            (InterfaceTemplate, {'name': 'eth0', 'label': tokens, 'type': '1000base-t'}),
+            (ModuleBayTemplate, {'name': tokens}),
+            (ModuleBayTemplate, {'name': 'Bay', 'label': tokens}),
+            (ModuleBayTemplate, {'name': 'Bay', 'position': tokens}),
+        )
+        for i, (template_model, attrs) in enumerate(templates, start=1):
+            with self.subTest(template=template_model.__name__, attrs=attrs):
+                module_type = ModuleType.objects.create(
+                    manufacturer=manufacturer, model=f'Token Depth Mismatch Module Type {i}'
+                )
+                template_model.objects.create(module_type=module_type, **attrs)
+                module_bay = ModuleBay.objects.create(device=device, name=f'Depth 1 Bay {i}', position='1')
 
+                data = {
+                    'device': device.pk,
+                    'module_bay': module_bay.pk,
+                    'module_type': module_type.pk,
+                }
+                response = self.client.post(url, data, format='json', **self.header)
+                self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+                # Rejected by validation, not by the AbortRequest that component creation would raise
+                self.assertIn('2 placeholders given', str(response.data.get('__all__')))
+                self.assertFalse(Module.objects.filter(module_bay=module_bay).exists())
+
+    @tag('regression')  # Issue #23478
+    def test_module_token_empty_ancestor_position(self):
+        """One placeholder per level needs every bay position, a single placeholder only the immediate one."""
+        self.add_permissions('dcim.add_module')
+        manufacturer = Manufacturer.objects.get(name='Generic')
+        device = create_test_device('Device for Token Ancestor Test')
+        parent_type = ModuleType.objects.create(manufacturer=manufacturer, model='Token Ancestor Parent Type')
+        parent_bay = ModuleBay.objects.create(device=device, name='Parent Bay')
+        parent_module = Module.objects.create(device=device, module_bay=parent_bay, module_type=parent_type)
+        child_bay = ModuleBay.objects.create(device=device, module=parent_module, name='Child Bay', position='2')
         url = reverse('dcim-api:module-list')
+
+        level_type = ModuleType.objects.create(manufacturer=manufacturer, model='Token Ancestor Level Type')
+        InterfaceTemplate.objects.create(
+            module_type=level_type, name='eth0', label=f'{MODULE_TOKEN}/{MODULE_TOKEN}', type='1000base-t'
+        )
         data = {
             'device': device.pk,
-            'module_bay': module_bay.pk,
-            'module_type': module_type.pk,
+            'module_bay': child_bay.pk,
+            'module_type': level_type.pk,
         }
         response = self.client.post(url, data, format='json', **self.header)
         self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+        self.assertIn('no position defined', str(response.data.get('__all__')))
 
-    def test_module_token_depth_mismatch(self):
-        """
-        Installing a module whose template name has more MODULE_TOKEN placeholders than the
-        depth of the module bay tree should return a validation error.
-        """
+        leaf_type = ModuleType.objects.create(manufacturer=manufacturer, model='Token Ancestor Leaf Type')
+        InterfaceTemplate.objects.create(
+            module_type=leaf_type, name='eth0', label=f'Port {MODULE_TOKEN}', type='1000base-t'
+        )
+        data['module_type'] = leaf_type.pk
+        response = self.client.post(url, data, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_201_CREATED)
+        module = Module.objects.get(pk=response.data['id'])
+        self.assertEqual(module.interfaces.get().label, 'Port 2')
+
+    def test_module_token_resolves_in_label_and_module_bay(self):
+        """Placeholders in a label and in each module bay template field resolve against a nested bay."""
         self.add_permissions('dcim.add_module')
         manufacturer = Manufacturer.objects.get(name='Generic')
-        device = create_test_device('Device for Token Depth Mismatch Test')
-        module_type = ModuleType.objects.create(manufacturer=manufacturer, model='Token Depth Mismatch Module Type')
-        # Template name has two placeholders but the bay is at depth 1
+        device = create_test_device('Device for Token Resolution Test')
+        parent_type = ModuleType.objects.create(manufacturer=manufacturer, model='Token Resolution Parent Type')
+        parent_bay = ModuleBay.objects.create(device=device, name='Parent Bay', position='1')
+        parent_module = Module.objects.create(device=device, module_bay=parent_bay, module_type=parent_type)
+        child_bay = ModuleBay.objects.create(device=device, module=parent_module, name='Child Bay', position='2')
+        module_type = ModuleType.objects.create(manufacturer=manufacturer, model='Token Resolution Module Type')
         InterfaceTemplate.objects.create(
-            module_type=module_type, name=f'{MODULE_TOKEN}-{MODULE_TOKEN}-eth0', type='1000base-t'
+            module_type=module_type, name='eth0', label=f'Port {MODULE_TOKEN}/{MODULE_TOKEN}', type='1000base-t'
+        )
+        ModuleBayTemplate.objects.create(
+            module_type=module_type,
+            name=f'SFP {MODULE_TOKEN}',
+            label=f'{MODULE_TOKEN}/{MODULE_TOKEN}',
+            position=f'{MODULE_TOKEN}/1',
         )
-        module_bay = ModuleBay.objects.create(device=device, name='Depth 1 Bay', position='1')
 
         url = reverse('dcim-api:module-list')
         data = {
             'device': device.pk,
-            'module_bay': module_bay.pk,
+            'module_bay': child_bay.pk,
             'module_type': module_type.pk,
         }
         response = self.client.post(url, data, format='json', **self.header)
-        self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+        self.assertHttpStatus(response, status.HTTP_201_CREATED)
+        module = Module.objects.get(pk=response.data['id'])
+        self.assertEqual(module.interfaces.get().label, 'Port 1/2')
+        module_bay = module.modulebays.get()
+        self.assertEqual((module_bay.name, module_bay.label, module_bay.position), ('SFP 2', '1/2', '2/1'))
+
+    def test_module_bay_template_names_repeat_across_modules(self):
+        """Child module bays from one template may share a name across modules on a device."""
+        self.add_permissions('dcim.add_module')
+        manufacturer = Manufacturer.objects.get(name='Generic')
+        device = create_test_device('Device for Repeated Child Bay Test')
+        module_type = ModuleType.objects.create(manufacturer=manufacturer, model='Repeated Child Bay Module Type')
+        ModuleBayTemplate.objects.create(module_type=module_type, name='SFP 1')
+
+        url = reverse('dcim-api:module-list')
+        for name in ('Repeated Bay 1', 'Repeated Bay 2'):
+            module_bay = ModuleBay.objects.create(device=device, name=name)
+            data = {
+                'device': device.pk,
+                'module_bay': module_bay.pk,
+                'module_type': module_type.pk,
+            }
+            response = self.client.post(url, data, format='json', **self.header)
+            self.assertHttpStatus(response, status.HTTP_201_CREATED)
+        self.assertEqual(device.modulebays.filter(name='SFP 1').count(), 2)
 
     def test_list_objects_by_profile_id(self):
         profiles = ModuleTypeProfile.objects.filter(name__startswith='Test').order_by('name')

+ 133 - 0
netbox/dcim/tests/test_forms.py

@@ -664,6 +664,139 @@ class ModuleFormTestCase(TestCase):
         intake.refresh_from_db()
         self.assertEqual(intake.module, module)
 
+    def test_module_form_rejects_module_token_without_bay_position(self):
+        """Replication and adoption reject a {module} token in any resolved field when the bay has no position."""
+        interface_type = InterfaceTypeChoices.TYPE_1GE_FIXED
+        templates = (
+            (InterfaceTemplate, {'name': 'eth{module}', 'type': interface_type}),
+            (InterfaceTemplate, {'name': 'eth0', 'label': 'Port {module}', 'type': interface_type}),
+            (ModuleBayTemplate, {'name': 'Bay {module}'}),
+            (ModuleBayTemplate, {'name': 'Bay', 'label': 'Bay {module}'}),
+            (ModuleBayTemplate, {'name': 'Bay', 'position': '{module}'}),
+        )
+        for i, (template_model, attrs) in enumerate(templates, start=1):
+            module_type = ModuleType.objects.create(
+                manufacturer=self.module_type.manufacturer, model=f'Token No-Position Form Type {i}'
+            )
+            template_model.objects.create(module_type=module_type, **attrs)
+            for flags in ({'replicate_components': True}, {'adopt_components': True}):
+                with self.subTest(template=template_model.__name__, attrs=attrs, flags=flags):
+                    form = ModuleForm(data={
+                        'device': self.device.pk,
+                        'module_bay': self.bay_b.pk,
+                        'module_type': module_type.pk,
+                        'status': 'active',
+                        **flags,
+                    })
+                    self.assertFalse(form.is_valid())
+                    self.assertIn('no position defined', str(form.errors))
+
+    def test_module_form_rejects_module_token_depth_mismatch(self):
+        """Replication and adoption reject more {module} tokens in a resolved field than the bay depth."""
+        module_bay = ModuleBay.objects.create(device=self.device, name='Bay D', position='1')
+        interface_type = InterfaceTypeChoices.TYPE_1GE_FIXED
+        tokens = '{module}/{module}'
+        templates = (
+            (InterfaceTemplate, {'name': f'eth{tokens}', 'type': interface_type}),
+            (InterfaceTemplate, {'name': 'eth0', 'label': tokens, 'type': interface_type}),
+            (ModuleBayTemplate, {'name': tokens}),
+            (ModuleBayTemplate, {'name': 'Bay', 'label': tokens}),
+            (ModuleBayTemplate, {'name': 'Bay', 'position': tokens}),
+        )
+        for i, (template_model, attrs) in enumerate(templates, start=1):
+            module_type = ModuleType.objects.create(
+                manufacturer=self.module_type.manufacturer, model=f'Token Depth Form Type {i}'
+            )
+            template_model.objects.create(module_type=module_type, **attrs)
+            for flags in ({'replicate_components': True}, {'adopt_components': True}):
+                with self.subTest(template=template_model.__name__, attrs=attrs, flags=flags):
+                    form = ModuleForm(data={
+                        'device': self.device.pk,
+                        'module_bay': module_bay.pk,
+                        'module_type': module_type.pk,
+                        'status': 'active',
+                        **flags,
+                    })
+                    self.assertFalse(form.is_valid())
+                    self.assertIn('2 placeholders given', str(form.errors))
+
+    def test_module_form_adopt_only_creates_no_components(self):
+        """Adoption without replication adopts matching components and creates no others."""
+        adopt_type = ModuleType.objects.create(
+            manufacturer=self.module_type.manufacturer, model='Adopt Only Form Type'
+        )
+        InterfaceTemplate.objects.create(module_type=adopt_type, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED)
+        InterfaceTemplate.objects.create(module_type=adopt_type, name='eth1', type=InterfaceTypeChoices.TYPE_1GE_FIXED)
+        interface = Interface.objects.create(
+            device=self.device, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED
+        )
+        form = ModuleForm(data={
+            'device': self.device.pk,
+            'module_bay': self.bay_b.pk,
+            'module_type': adopt_type.pk,
+            'status': 'active',
+            'adopt_components': True,
+        })
+        self.assertTrue(form.is_valid(), form.errors)
+        module = form.save()
+        interface.refresh_from_db()
+        self.assertEqual(interface.module, module)
+        self.assertFalse(self.device.interfaces.filter(name='eth1').exists())
+
+    def test_module_form_adopt_only_rejects_component_of_another_module(self):
+        """Adoption without replication refuses a component that already belongs to another module."""
+        adopt_type = ModuleType.objects.create(
+            manufacturer=self.module_type.manufacturer, model='Owned Adopt Form Type'
+        )
+        InterfaceTemplate.objects.create(module_type=adopt_type, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED)
+        Interface.objects.create(
+            device=self.device, module=self.module, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED
+        )
+        form = ModuleForm(data={
+            'device': self.device.pk,
+            'module_bay': self.bay_b.pk,
+            'module_type': adopt_type.pk,
+            'status': 'active',
+            'adopt_components': True,
+        })
+        self.assertFalse(form.is_valid())
+        self.assertIn('already belongs to a module', str(form.errors))
+
+    def test_module_form_skips_template_checks_without_component_operations(self):
+        """Without replication or adoption, templates are neither checked nor instantiated."""
+        token_type = ModuleType.objects.create(
+            manufacturer=self.module_type.manufacturer, model='No Operations Form Type'
+        )
+        InterfaceTemplate.objects.create(
+            module_type=token_type, name='eth0', label='Port {module}', type=InterfaceTypeChoices.TYPE_1GE_FIXED
+        )
+        form = ModuleForm(data={
+            'device': self.device.pk,
+            'module_bay': self.bay_b.pk,
+            'module_type': token_type.pk,
+            'status': 'active',
+        })
+        self.assertTrue(form.is_valid(), form.errors)
+        module = form.save()
+        self.assertFalse(module.interfaces.exists())
+
+    def test_module_import_form_rejects_module_token_without_bay_position(self):
+        """Bulk import applies the module form's placeholder checks."""
+        token_type = ModuleType.objects.create(
+            manufacturer=self.module_type.manufacturer, model='Token Import Type'
+        )
+        InterfaceTemplate.objects.create(
+            module_type=token_type, name='eth0', label='Port {module}', type=InterfaceTypeChoices.TYPE_1GE_FIXED
+        )
+        form = ModuleImportForm(data={
+            'device': self.device.name,
+            'module_bay': self.bay_b.name,
+            'module_type': token_type.model,
+            'status': 'active',
+        })
+        self.assertFalse(form.is_valid())
+        self.assertIn('no position defined', str(form.errors))
+
 
 class VCPositionTokenFormTestCase(TestCase):
 

+ 37 - 0
netbox/dcim/tests/test_models.py

@@ -17,6 +17,7 @@ from core.choices import ObjectChangeActionChoices
 from core.models import ObjectChange, ObjectType
 from dcim.choices import *
 from dcim.models import *
+from dcim.utils import resolve_module_placeholder
 from extras.events import serialize_for_event
 from extras.models import CustomField
 from ipam.models import IPAddress, Prefix
@@ -25,6 +26,7 @@ from netbox.context_managers import event_tracking
 from tenancy.models import Tenant
 from users.models import User
 from utilities.data import drange
+from utilities.exceptions import AbortRequest
 from utilities.testing import create_test_device, create_test_image
 from virtualization.models import Cluster, ClusterType
 
@@ -2157,6 +2159,41 @@ class ModuleBayTestCase(TestCase):
         interface = iface_module.interfaces.first()
         self.assertEqual(interface.name, 'Gi1/2')
 
+    @tag('regression')  # #23478
+    def test_module_token_depth_mismatch_in_label_raises_abort_request(self):
+        """An unresolvable {module} label aborts a direct module install with AbortRequest."""
+        device = Device.objects.first()
+        module_bay = ModuleBay.objects.create(device=device, name='Token Label Bay', position='1')
+        module_type = ModuleType.objects.create(manufacturer=Manufacturer.objects.first(), model='Token Label Type')
+        InterfaceTemplate.objects.create(
+            module_type=module_type, name='eth0', label='{module}/{module}', type=InterfaceTypeChoices.TYPE_1GE_FIXED
+        )
+        module = Module(device=device, module_bay=module_bay, module_type=module_type)
+        with self.assertRaises(AbortRequest) as cm:
+            module.save()
+        self.assertIn('2 placeholders given', cm.exception.message)
+
+    @tag('regression')  # #23478
+    def test_module_token_without_bay_position_raises_abort_request(self):
+        """A {module} name in a bay without a position aborts a direct module install with AbortRequest."""
+        device = Device.objects.first()
+        module_bay = ModuleBay.objects.create(device=device, name='Token Name Bay')
+        module_type = ModuleType.objects.create(manufacturer=Manufacturer.objects.first(), model='Token Name Type')
+        InterfaceTemplate.objects.create(
+            module_type=module_type, name='eth{module}', type=InterfaceTypeChoices.TYPE_1GE_FIXED
+        )
+        module = Module(device=device, module_bay=module_bay, module_type=module_type)
+        with self.assertRaises(AbortRequest) as cm:
+            module.save()
+        self.assertIn('no position defined', cm.exception.message)
+
+    @tag('regression')  # #23478
+    def test_module_token_without_positions_raises_value_error(self):
+        """A {module} token with no bay position to resolve against raises ValueError."""
+        with self.assertRaises(ValueError) as cm:
+            resolve_module_placeholder('eth{module}', [])
+        self.assertIn('no position defined', str(cm.exception))
+
     @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"""

+ 11 - 1
netbox/dcim/tests/test_module_moves.py

@@ -1059,6 +1059,15 @@ class ModuleMoveRenameTestCase(TestCase):
         with self.assertRaises(ValidationError):
             pos_module.full_clean()
 
+    def test_move_to_bay_without_position_rejects_templated_values(self):
+        """A move whose template-derived values would need a missing bay position is rejected."""
+        bare_slot = ModuleBay.objects.create(device=self.device, name='Bare Slot')
+
+        self.line_card.module_bay = bare_slot
+        with self.assertRaises(ValidationError) as cm:
+            self.line_card.full_clean()
+        self.assertIn('cannot be resolved for the destination', str(cm.exception))
+
 
 class ModuleCrossDeviceBlockerTestCase(TestCase):
 
@@ -1568,7 +1577,8 @@ class ModuleCrossDeviceMoveTestCase(TestCase):
         ModuleBay.objects.bulk_create([
             ModuleBay(device=big_device, name=f'Storage Bay {i}') for i in range(1, 201)
         ])
-        target_bay = ModuleBay.objects.create(device=big_device, name='Target Bay')
+        # Mirrors slot_2_b, the SFP name template needs a bay position
+        target_bay = ModuleBay.objects.create(device=big_device, name='Target Bay', position='2')
         big_count = move_and_count(big_device, target_bay)
         self.assertEqual(small_count, big_count)
 

+ 18 - 11
netbox/dcim/utils.py

@@ -82,24 +82,31 @@ def resolve_module_placeholder(value, positions):
     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.
+    Raises ValueError if token count is greater than 1 and doesn't match tree depth,
+    or if a position a token resolves to is missing or empty.
     """
     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
+        referenced = positions[-1:]
+    elif token_count == len(positions):
+        referenced = positions
+    else:
+        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
+            )
+        )
+    if not referenced or not all(referenced):
+        raise ValueError(
+            _("Cannot install module with placeholder values in a module bay with no position defined.")
         )
-    )
+    for pos in referenced:
+        value = value.replace(MODULE_TOKEN, pos, 1)
+    return value
 
 
 def compile_path_node(ct_id, object_id):