Refuse genuinely ambiguous bay-type names; close the ModuleBay CSV gap

- dedupe_module_bay_types_by_manufacturer()'s lowest preference tier (a
  bay type belonging to some manufacturer other than the importing
  type's own) previously picked whichever candidate happened to sort
  first when two or more *different* foreign manufacturers shared a
  name. Verified directly: importing 'SFP28' for a Juniper module type,
  with only Cisco's and Arista's same-named types in the database (no
  Juniper or global one), silently linked to Arista's -- a wrong FK with
  no signal to the importer. The permissive fix from the last round only
  needs this tier to be reachable for the single-candidate case, not
  tolerant of a genuine tie; now raises ValidationError, attributed to
  module_bay_types, naming the competing manufacturers.

- ModuleBayTemplateImportForm.module_bay_types was a plain
  ModelMultipleChoiceField (list only), while ModuleTypeImportForm's
  otherwise-identical field is a CSVModelMultipleChoiceField (list or
  comma-separated string), so `module_bay_types: SFP28` was accepted at
  the module-type level and rejected under `module-bays:` within the
  same YAML document. Switched to CSVModelMultipleChoiceField in both,
  which costs nothing here since it passes lists through unchanged.

- ModuleBayImportForm (CSV import for real ModuleBay instances, as
  opposed to ModuleBayTemplateImportForm's templates) still had no
  module_bay_types support -- the same class of round-trip gap this PR
  exists to close, on the instance side rather than the template side.
  Added it, scoped via the importing device's own device_type.manufacturer.

- The ModuleType prefetch query-count test only varied bay count (module
  type count fixed at 1), so it couldn't detect a regression in the
  module_bay_types prefetch on ModuleType itself -- confirmed directly:
  the test stayed green with that prefetch removed entirely. Varying
  module type count instead doesn't work either, since to_yaml() touches
  several other per-instance relations (manufacturer, port_mappings, ...)
  that legitimately scale with row count regardless of this fix and swamp
  an exact-equality comparison -- hit this myself on the first attempt.
  Replaced with a with/without-prefetch comparison on the identical
  queryset, which isolates the saving without that confound; verified it
  fails when the prefetch is removed and passes when it's present.

netbox/dcim/forms/bulk_import.py

@@ -1439,10 +1439,19 @@ class ModuleBayImportForm(OwnerCSVMixin, NetBoxModelImportForm):
         queryset=Device.objects.all(),
         to_field_name='name'
     )
+    module_bay_types = CSVModelMultipleChoiceField(
+        label=_('Module bay types'),
+        queryset=ModuleBayType.objects.all(),
+        to_field_name='name',
+        required=False,
+        help_text=_('Types of module bays this bay accepts (empty = unconstrained)'),
+    )
 
     class Meta:
         model = ModuleBay
-        fields = ('device', 'name', 'label', 'position', 'enabled', 'description', 'owner', 'tags')
+        fields = (
+            'device', 'name', 'label', 'position', 'enabled', 'description', 'owner', 'tags', 'module_bay_types',
+        )
 
     def clean_enabled(self):
         # Make sure enabled is True when it's not included in the uploaded data
@@ -1450,6 +1459,16 @@ class ModuleBayImportForm(OwnerCSVMixin, NetBoxModelImportForm):
             return True
         return self.cleaned_data['enabled']
 
+    def clean(self):
+        super().clean()
+
+        if module_bay_types := self.cleaned_data.get('module_bay_types'):
+            device = self.cleaned_data.get('device')
+            manufacturer = device.device_type.manufacturer if device else None
+            self.cleaned_data['module_bay_types'] = dedupe_module_bay_types_by_manufacturer(
+                module_bay_types, manufacturer,
+            )
+
 
 class DeviceBayImportForm(OwnerCSVMixin, NetBoxModelImportForm):
     device = CSVModelChoiceField(

netbox/dcim/forms/object_import.py

@@ -4,6 +4,7 @@ from django.utils.translation import gettext_lazy as _
 from dcim.choices import InterfacePoEModeChoices, InterfacePoETypeChoices, InterfaceTypeChoices, PortTypeChoices
 from dcim.models import *
 from dcim.utils import dedupe_module_bay_types_by_manufacturer
+from utilities.forms.fields import CSVModelMultipleChoiceField
 from wireless.choices import WirelessRoleChoices
 
 __all__ = (
@@ -214,7 +215,13 @@ class PortTemplateMappingImportForm(forms.ModelForm):
 
 
 class ModuleBayTemplateImportForm(forms.ModelForm):
-    module_bay_types = forms.ModelMultipleChoiceField(
+    # CSVModelMultipleChoiceField (not the plain ModelMultipleChoiceField used elsewhere in
+    # this file) so a scalar name string is accepted alongside a list -- this form is
+    # YAML-only, but ModuleTypeImportForm's equivalent field also serves plain CSV import and
+    # therefore must accept both; keeping the two consistent means `module_bay_types: SFP28`
+    # behaves the same whether it appears at the module-type level or under `module-bays:`
+    # within the same YAML document.
+    module_bay_types = CSVModelMultipleChoiceField(
         label=_('Module bay types'),
         queryset=ModuleBayType.objects.all(),
         to_field_name='name',

netbox/dcim/tests/test_forms.py

@@ -396,6 +396,42 @@ class ModuleBayTemplateImportFormTestCase(TestCase):
         self.assertEqual(list(reimport_form.save().module_bay_types.all()), [cisco_bay_type])
 
 
+class ModuleBayImportFormTestCase(TestCase):
+    """
+    ModuleBayImportForm covers real ModuleBay instances created directly via CSV (as
+    opposed to ModuleBayTemplateImportForm, which covers templates nested under a device or
+    module type's YAML definition) -- the same class of round-trip gap, on the instance side.
+    """
+
+    def test_module_bay_types_csv_import(self):
+        device = create_test_device('Module Bay Import Device')
+        bay_type = ModuleBayType.objects.create(name='SFP28', slug='sfp28')
+
+        form = ModuleBayImportForm({
+            'device': device.name,
+            'name': 'Bay 1',
+            'module_bay_types': 'SFP28',
+        })
+        self.assertTrue(form.is_valid(), form.errors)
+        module_bay = form.save()
+        self.assertEqual(list(module_bay.module_bay_types.all()), [bay_type])
+
+    def test_module_bay_types_prefers_devices_own_manufacturer(self):
+        device = create_test_device('Module Bay Import Device')
+        own_manufacturer = device.device_type.manufacturer
+        other_manufacturer = Manufacturer.objects.create(name='Other Mfr', slug='other-mfr')
+        own_type = ModuleBayType.objects.create(name='SFP28', slug='sfp28-own', manufacturer=own_manufacturer)
+        ModuleBayType.objects.create(name='SFP28', slug='sfp28-other', manufacturer=other_manufacturer)
+
+        form = ModuleBayImportForm({
+            'device': device.name,
+            'name': 'Bay 1',
+            'module_bay_types': 'SFP28',
+        })
+        self.assertTrue(form.is_valid(), form.errors)
+        self.assertEqual(list(form.save().module_bay_types.all()), [own_type])
+
+
 class ModuleTypeImportFormTestCase(TestCase):
 
     def test_module_bay_types_round_trip(self):
@@ -497,6 +533,27 @@ class ModuleTypeImportFormTestCase(TestCase):
         self.assertTrue(reimport_form.is_valid(), reimport_form.errors)
         self.assertEqual(list(reimport_form.save().module_bay_types.all()), [cisco_bay_type])
 
+    def test_module_bay_types_rejects_ambiguous_name_across_two_foreign_manufacturers(self):
+        """
+        A single other-manufacturer match is permitted (see above), but if the name matches
+        two or more *different* foreign manufacturers, there's no principled way to choose
+        one -- silently picking whichever sorts first would create a wrong FK link with no
+        signal to the importer. This must be refused rather than resolved arbitrarily.
+        """
+        juniper = Manufacturer.objects.create(name='Juniper', slug='juniper')
+        cisco = Manufacturer.objects.create(name='Cisco', slug='cisco')
+        arista = Manufacturer.objects.create(name='Arista', slug='arista')
+        ModuleBayType.objects.create(name='SFP28', slug='sfp28-cisco', manufacturer=cisco)
+        ModuleBayType.objects.create(name='SFP28', slug='sfp28-arista', manufacturer=arista)
+
+        form = ModuleTypeImportForm({
+            'manufacturer': juniper.name,
+            'model': 'Juniper Line Card',
+            'module_bay_types': ['SFP28'],
+        })
+        self.assertFalse(form.is_valid())
+        self.assertIn('module_bay_types', form.errors)
+
 
 class ModuleFormTestCase(TestCase):
 

netbox/dcim/tests/test_views.py

@@ -1827,6 +1827,44 @@ module-bays:
 
         self.assertEqual(len(one_bay_queries), len(five_bay_queries))
 
+    def test_bulk_yaml_export_prefetches_module_bay_types_on_the_module_type_itself(self):
+        """
+        Companion to test_..._is_constant above: that test holds the module type count fixed
+        at 1 and varies bay count, so it can't detect a regression in the module_bay_types
+        prefetch on ModuleType itself (which saves one query per module TYPE row, not per
+        bay) -- a 1-row queryset can't show a per-row saving. Comparing module-type COUNTS
+        (e.g. 1 vs. 5) doesn't work either: to_yaml() touches several other per-instance
+        relations (manufacturer, port_mappings, ...) that legitimately scale with row count
+        regardless of this fix, which would swamp the signal. Instead, compare the *same*
+        5-row queryset with and without the module_bay_types prefetch, isolating exactly what
+        it saves.
+        """
+        manufacturer = Manufacturer.objects.create(name='Export Query MT Manufacturer', slug='export-query-mt-mfr')
+        bay_type = ModuleBayType.objects.create(name='Export Query MT SFP28', slug='export-query-mt-sfp28')
+
+        module_types = []
+        for i in range(5):
+            module_type = ModuleType.objects.create(manufacturer=manufacturer, model=f'Export Query MT {i}')
+            module_type.module_bay_types.set([bay_type])
+            module_types.append(module_type)
+        pks = [mt.pk for mt in module_types]
+
+        with CaptureQueriesContext(connection) as unprefetched:
+            [obj.to_yaml() for obj in ModuleType.objects.filter(pk__in=pks)]
+
+        view = ModuleTypeListView()
+        view.queryset = ModuleType.objects.filter(pk__in=pks)
+        with CaptureQueriesContext(connection) as prefetched:
+            view.export_yaml()
+
+        # Without the prefetch, each of the 5 module types issues its own module_bay_types
+        # query. The exact delta isn't asserted -- prefetching modulebaytemplates (even when
+        # empty, as here) also lets to_yaml()'s .exists() check on that relation short-circuit
+        # from the prefetch cache instead of querying, so the totals reflect more than just
+        # module_bay_types -- but dropping the module_bay_types prefetch can only narrow this
+        # gap, never widen it, so a strict inequality still catches that regression.
+        self.assertGreater(len(unprefetched), len(prefetched))
+
     @override_settings(STREAMING_EXPORTS=True)
     def test_export_objects(self):
         url = reverse('dcim:moduletype_list')

netbox/dcim/utils.py

@@ -2,6 +2,7 @@ from collections import defaultdict
 
 from django.apps import apps
 from django.contrib.contenttypes.models import ContentType
+from django.core.exceptions import ValidationError
 from django.db import router, transaction
 from django.utils.translation import gettext as _
 
@@ -12,7 +13,7 @@ def dedupe_module_bay_types_by_manufacturer(module_bay_types, manufacturer=None)
     """
     Collapse an iterable of ModuleBayType instances resolved by name to one entry per name,
     preferring (in order) an exact match on *manufacturer*, then a global (manufacturer-less)
-    type, then any remaining candidate.
+    type.
 
     ModuleBayType's uniqueness is scoped to (manufacturer, name), not name alone, so two
     different manufacturers -- or a global type and a manufacturer-scoped one -- can
@@ -21,6 +22,11 @@ def dedupe_module_bay_types_by_manufacturer(module_bay_types, manufacturer=None)
     type (e.g. a third-party line card), so callers must not scope the underlying queryset
     by manufacturer -- only this preference order, for disambiguating an otherwise-ambiguous
     name, is manufacturer-aware.
+
+    Raises ValidationError if a name resolves to more than one candidate that ties for the
+    best preference tier (e.g. two different manufacturers, neither *manufacturer* nor
+    unset, share the name) -- there's no principled way to pick a winner there, so the
+    import is refused rather than silently linked to an arbitrary one.
     """
     manufacturer_id = manufacturer.pk if manufacturer else None
 
@@ -31,12 +37,26 @@ def dedupe_module_bay_types_by_manufacturer(module_bay_types, manufacturer=None)
             return 1
         return 2
 
-    resolved = {}
+    by_name = defaultdict(list)
     for module_bay_type in module_bay_types:
-        existing = resolved.get(module_bay_type.name)
-        if existing is None or preference(module_bay_type) < preference(existing):
-            resolved[module_bay_type.name] = module_bay_type
-    return list(resolved.values())
+        by_name[module_bay_type.name].append(module_bay_type)
+
+    resolved = []
+    for name, candidates in by_name.items():
+        best_rank = min(preference(c) for c in candidates)
+        best = [c for c in candidates if preference(c) == best_rank]
+        if len(best) > 1:
+            manufacturers = ', '.join(sorted(c.manufacturer.name for c in best))
+            raise ValidationError({
+                'module_bay_types': _(
+                    "Module bay type \"{name}\" is ambiguous: it belongs to more than one "
+                    "manufacturer ({manufacturers}), none of which is this type's own "
+                    "manufacturer."
+                ).format(name=name, manufacturers=manufacturers)
+            })
+        resolved.append(best[0])
+
+    return resolved
 
 
 def inherit_module_token(position, parent_positions):