Fix #11478 - Add vc_interfaces flag to control selection of VC interfaces (#13296)

* Add `vc_interfaces` flag to control interface queryset

* Fix test failure

* Add new filters instead of using undocumented query params

* Cleanup filterset, add test

* Rename filter and re-introduce virtual_chassis filtering method (required)

* Fix test

* Adjust tests to more accurately provide coverage

* Add breaking change note

* Misc cleanup

---------

Co-authored-by: Jeremy Stretch <[email protected]>

Author: DanSheps

docs/release-notes/version-3.6.md

@@ -23,6 +23,7 @@
 * The `device_role` field on the Device model has been renamed to `role`. The `device_role` field has been temporarily retained on the REST API serializer for devices for backward compatibility, but is read-only.
 * The `choices` array field has been removed from the CustomField model. Any defined choices are automatically migrated to CustomFieldChoiceSets, accessible via the new `choice_set` field on the CustomField model.
 * The `napalm_driver` and `napalm_args` fields (which were deprecated in v3.5) have been removed from the Platform model.
+* The `device` and `device_id` filter for interfaces will no longer include interfaces from virtual chassis peers. Two new filters, `virtual_chassis_member` and `virtual_chassis_member_id`, have been introduced to match all interfaces belonging to the specified device's virtual chassis (if any).
 * Reports and scripts are now returned within a `results` list when fetched via the REST API, consistent with other models.
 * Superusers can no longer retrieve API token keys via the web UI if [`ALLOW_TOKEN_RETRIEVAL`](https://docs.netbox.dev/en/stable/configuration/security/#allow_token_retrieval) is disabled. (The admin view has been removed per [#13044](https://github.com/netbox-community/netbox/issues/13044).)
 

netbox/dcim/filtersets.py

@@ -1462,17 +1462,15 @@ class InterfaceFilterSet(
     PathEndpointFilterSet,
     CommonInterfaceFilterSet
 ):
-    # Override device and device_id filters from DeviceComponentFilterSet to match against any peer virtual chassis
-    # members
-    device = MultiValueCharFilter(
-        method='filter_device',
+    virtual_chassis_member = MultiValueCharFilter(
+        method='filter_virtual_chassis_member',
         field_name='name',
-        label=_('Device'),
+        label=_('Virtual Chassis Interfaces for Device')
     )
-    device_id = MultiValueNumberFilter(
-        method='filter_device_id',
+    virtual_chassis_member_id = MultiValueNumberFilter(
+        method='filter_virtual_chassis_member',
         field_name='pk',
-        label=_('Device (ID)'),
+        label=_('Virtual Chassis Interfaces for Device (ID)')
     )
     kind = django_filters.CharFilter(
         method='filter_kind',
@@ -1540,23 +1538,11 @@ class Meta:
             'rf_channel', 'rf_channel_frequency', 'rf_channel_width', 'tx_power', 'description', 'cable_end',
         ]
 
-    def filter_device(self, queryset, name, value):
+    def filter_virtual_chassis_member(self, queryset, name, value):
         try:
-            devices = Device.objects.filter(**{'{}__in'.format(name): value})
             vc_interface_ids = []
-            for device in devices:
-                vc_interface_ids.extend(device.vc_interfaces().values_list('id', flat=True))
-            return queryset.filter(pk__in=vc_interface_ids)
-        except Device.DoesNotExist:
-            return queryset.none()
-
-    def filter_device_id(self, queryset, name, id_list):
-        # Include interfaces belonging to peer virtual chassis members
-        vc_interface_ids = []
-        try:
-            devices = Device.objects.filter(pk__in=id_list)
-            for device in devices:
-                vc_interface_ids += device.vc_interfaces(if_master=False).values_list('id', flat=True)
+            for device in Device.objects.filter(**{f'{name}__in': value}):
+                vc_interface_ids.extend(device.vc_interfaces(if_master=False).values_list('id', flat=True))
             return queryset.filter(pk__in=vc_interface_ids)
         except Device.DoesNotExist:
             return queryset.none()

netbox/dcim/forms/model_forms.py

@@ -1111,23 +1111,23 @@ class InterfaceForm(InterfaceCommonForm, ModularDeviceComponentForm):
         required=False,
         label=_('Parent interface'),
         query_params={
-            'device_id': '$device',
+            'virtual_chassis_member_id': '$device',
         }
     )
     bridge = DynamicModelChoiceField(
         queryset=Interface.objects.all(),
         required=False,
         label=_('Bridged interface'),
         query_params={
-            'device_id': '$device',
+            'virtual_chassis_member_id': '$device',
         }
     )
     lag = DynamicModelChoiceField(
         queryset=Interface.objects.all(),
         required=False,
         label=_('LAG interface'),
         query_params={
-            'device_id': '$device',
+            'virtual_chassis_member_id': '$device',
             'type': 'lag',
         }
     )

netbox/dcim/tests/test_filtersets.py

@@ -2822,25 +2822,72 @@ def setUpTestData(cls):
         )
         Rack.objects.bulk_create(racks)
 
+        # VirtualChassis assignment for filtering
+        virtual_chassis = VirtualChassis(name='Virtual Chassis')
+        virtual_chassis.save()
+
         devices = (
-            Device(name='Device 1', device_type=device_types[0], role=roles[0], site=sites[0], location=locations[0], rack=racks[0]),
-            Device(name='Device 2', device_type=device_types[1], role=roles[1], site=sites[1], location=locations[1], rack=racks[1]),
-            Device(name='Device 3', device_type=device_types[2], role=roles[2], site=sites[2], location=locations[2], rack=racks[2]),
-            Device(name=None, device_type=device_types[2], role=roles[2], site=sites[3]),  # For cable connections
+            Device(
+                name='Device 1A',
+                device_type=device_types[0],
+                role=roles[0],
+                site=sites[0],
+                location=locations[0],
+                rack=racks[0],
+                virtual_chassis=virtual_chassis,
+                vc_position=1,
+                vc_priority=1
+            ),
+            Device(
+                name='Device 1B',
+                device_type=device_types[2],
+                role=roles[2],
+                site=sites[2],
+                location=locations[2],
+                rack=racks[2],
+                virtual_chassis=virtual_chassis,
+                vc_position=2,
+                vc_priority=1
+            ),
+            Device(
+                name='Device 2',
+                device_type=device_types[1],
+                role=roles[1],
+                site=sites[1],
+                location=locations[1],
+                rack=racks[1]
+            ),
+            Device(
+                name='Device 3',
+                device_type=device_types[2],
+                role=roles[2],
+                site=sites[2],
+                location=locations[2],
+                rack=racks[2]
+            ),
+            # For cable connections
+            Device(
+                name=None,
+                device_type=device_types[2],
+                role=roles[2],
+                site=sites[3]
+            ),
         )
         Device.objects.bulk_create(devices)
 
         module_bays = (
             ModuleBay(device=devices[0], name='Module Bay 1'),
             ModuleBay(device=devices[1], name='Module Bay 2'),
             ModuleBay(device=devices[2], name='Module Bay 3'),
+            ModuleBay(device=devices[3], name='Module Bay 4'),
         )
         ModuleBay.objects.bulk_create(module_bays)
 
         modules = (
             Module(device=devices[0], module_bay=module_bays[0], module_type=module_type),
             Module(device=devices[1], module_bay=module_bays[1], module_type=module_type),
             Module(device=devices[2], module_bay=module_bays[2], module_type=module_type),
+            Module(device=devices[3], module_bay=module_bays[3], module_type=module_type),
         )
         Module.objects.bulk_create(modules)
 
@@ -2853,16 +2900,11 @@ def setUpTestData(cls):
 
         # Virtual Device Context Creation
         vdcs = (
-            VirtualDeviceContext(device=devices[3], name='VDC 1', identifier=1, status=VirtualDeviceContextStatusChoices.STATUS_ACTIVE),
-            VirtualDeviceContext(device=devices[3], name='VDC 2', identifier=2, status=VirtualDeviceContextStatusChoices.STATUS_PLANNED),
+            VirtualDeviceContext(device=devices[4], name='VDC 1', identifier=1, status=VirtualDeviceContextStatusChoices.STATUS_ACTIVE),
+            VirtualDeviceContext(device=devices[4], name='VDC 2', identifier=2, status=VirtualDeviceContextStatusChoices.STATUS_PLANNED),
         )
         VirtualDeviceContext.objects.bulk_create(vdcs)
 
-        # VirtualChassis assignment for filtering
-        virtual_chassis = VirtualChassis.objects.create(master=devices[0])
-        Device.objects.filter(pk=devices[0].pk).update(virtual_chassis=virtual_chassis, vc_position=1, vc_priority=1)
-        Device.objects.filter(pk=devices[1].pk).update(virtual_chassis=virtual_chassis, vc_position=2, vc_priority=2)
-
         interfaces = (
             Interface(
                 device=devices[0],
@@ -2885,6 +2927,13 @@ def setUpTestData(cls):
             Interface(
                 device=devices[1],
                 module=modules[1],
+                name='VC Chassis Interface',
+                type=InterfaceTypeChoices.TYPE_1GE_SFP,
+                enabled=True
+            ),
+            Interface(
+                device=devices[2],
+                module=modules[2],
                 name='Interface 2',
                 label='B',
                 type=InterfaceTypeChoices.TYPE_1GE_GBIC,
@@ -2901,8 +2950,8 @@ def setUpTestData(cls):
                 poe_type=InterfacePoETypeChoices.TYPE_1_8023AF
             ),
             Interface(
-                device=devices[2],
-                module=modules[2],
+                device=devices[3],
+                module=modules[3],
                 name='Interface 3',
                 label='C',
                 type=InterfaceTypeChoices.TYPE_1GE_FIXED,
@@ -2919,7 +2968,7 @@ def setUpTestData(cls):
                 poe_type=InterfacePoETypeChoices.TYPE_2_8023AT
             ),
             Interface(
-                device=devices[3],
+                device=devices[4],
                 name='Interface 4',
                 label='D',
                 type=InterfaceTypeChoices.TYPE_OTHER,
@@ -2932,7 +2981,7 @@ def setUpTestData(cls):
                 poe_type=InterfacePoETypeChoices.TYPE_2_8023AT
             ),
             Interface(
-                device=devices[3],
+                device=devices[4],
                 name='Interface 5',
                 label='E',
                 type=InterfaceTypeChoices.TYPE_OTHER,
@@ -2941,7 +2990,7 @@ def setUpTestData(cls):
                 tx_power=40
             ),
             Interface(
-                device=devices[3],
+                device=devices[4],
                 name='Interface 6',
                 label='F',
                 type=InterfaceTypeChoices.TYPE_OTHER,
@@ -2950,7 +2999,7 @@ def setUpTestData(cls):
                 tx_power=40
             ),
             Interface(
-                device=devices[3],
+                device=devices[4],
                 name='Interface 7',
                 type=InterfaceTypeChoices.TYPE_80211AC,
                 rf_role=WirelessRoleChoices.ROLE_AP,
@@ -2959,7 +3008,7 @@ def setUpTestData(cls):
                 rf_channel_width=22
             ),
             Interface(
-                device=devices[3],
+                device=devices[4],
                 name='Interface 8',
                 type=InterfaceTypeChoices.TYPE_80211AC,
                 rf_role=WirelessRoleChoices.ROLE_STATION,
@@ -2977,8 +3026,8 @@ def setUpTestData(cls):
         interfaces[7].vdcs.set([vdcs[1]])
 
         # Cables
-        Cable(a_terminations=[interfaces[0]], b_terminations=[interfaces[3]]).save()
-        Cable(a_terminations=[interfaces[1]], b_terminations=[interfaces[4]]).save()
+        Cable(a_terminations=[interfaces[0]], b_terminations=[interfaces[5]]).save()
+        Cable(a_terminations=[interfaces[1]], b_terminations=[interfaces[6]]).save()
         # Third pair is not connected
 
     def test_name(self):
@@ -2991,7 +3040,7 @@ def test_label(self):
 
     def test_enabled(self):
         params = {'enabled': 'true'}
-        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 6)
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 7)
         params = {'enabled': 'false'}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
@@ -3011,7 +3060,7 @@ def test_mgmt_only(self):
         params = {'mgmt_only': 'true'}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
         params = {'mgmt_only': 'false'}
-        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 5)
 
     def test_poe_mode(self):
         params = {'poe_mode': [InterfacePoEModeChoices.MODE_PD, InterfacePoEModeChoices.MODE_PSE]}
@@ -3116,6 +3165,14 @@ def test_device(self):
         params = {'device': [devices[0].name, devices[1].name]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_virtual_chassis_member(self):
+        # Device 1A & 3 have 1 management interface, Device 1B has 1 interfaces
+        devices = Device.objects.filter(name__in=['Device 1A', 'Device 3'])
+        params = {'virtual_chassis_member_id': [devices[0].pk, devices[1].pk]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
+        params = {'virtual_chassis_member': [devices[0].name, devices[1].name]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
+
     def test_module(self):
         modules = Module.objects.all()[:2]
         params = {'module_id': [modules[0].pk, modules[1].pk]}
@@ -3125,23 +3182,23 @@ def test_cabled(self):
         params = {'cabled': True}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
         params = {'cabled': False}
-        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 5)
 
     def test_occupied(self):
         params = {'occupied': True}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
         params = {'occupied': False}
-        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 5)
 
     def test_connected(self):
         params = {'connected': True}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
         params = {'connected': False}
-        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 5)
 
     def test_kind(self):
         params = {'kind': 'physical'}
-        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 6)
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 7)
         params = {'kind': 'virtual'}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 0)