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

Fixes #23212: Prevent removing a virtual chassis member with cross-chassis LAG interfaces

Arthur 3 дней назад
Родитель
Сommit
3c21f231bf

+ 15 - 0
netbox/dcim/models/devices.py

@@ -1018,6 +1018,21 @@ class Device(
                 ).format(virtual_chassis=self.vc_master_for)
             })
 
+        # Prevent leaving a virtual chassis while interfaces form a LAG with another member
+        if self.pk:
+            lag_members = Interface.objects.filter(device=self, lag__isnull=False).exclude(lag__device=self)
+            lag_peers = Interface.objects.filter(lag__device=self).exclude(device=self)
+            if self.virtual_chassis:
+                lag_members = lag_members.exclude(lag__device__virtual_chassis=self.virtual_chassis)
+                lag_peers = lag_peers.exclude(device__virtual_chassis=self.virtual_chassis)
+            if lag_members.exists() or lag_peers.exists():
+                raise ValidationError({
+                    'virtual_chassis': _(
+                        'Device cannot be removed from its virtual chassis because one or more of its interfaces '
+                        'form a LAG with another member device.'
+                    )
+                })
+
     def _check_duplicate_component_names(self, components):
         """
         Check for duplicate component names after resolving {vc_position} placeholders.

+ 18 - 0
netbox/dcim/tests/test_api.py

@@ -6042,6 +6042,24 @@ class VirtualChassisTestCase(APIViewTestCases.APIViewTestCase):
             'owner': 99999,
         }
 
+    @tag('regression')  # Ref: #23212
+    def test_remove_member_blocked_by_cross_chassis_lag(self):
+        self.add_permissions('dcim.change_device')
+        master = Device.objects.get(name='Device 1')
+        member = Device.objects.get(name='Device 2')
+        Interface.objects.filter(device=member, name='2/1').update(
+            lag=Interface.objects.create(device=master, name='lag0', type=InterfaceTypeChoices.TYPE_LAG)
+        )
+
+        url = reverse('dcim-api:device-detail', kwargs={'pk': member.pk})
+        data = {'virtual_chassis': None, 'vc_position': None}
+        response = self.client.patch(url, data, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+        self.assertIn('virtual_chassis', response.data)
+
+        member.refresh_from_db()
+        self.assertEqual(member.virtual_chassis.name, 'Virtual Chassis 1')
+
 
 class PowerPanelTestCase(APIViewTestCases.APIViewTestCase):
     model = PowerPanel

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

@@ -152,6 +152,30 @@ class DeviceTestCase(TestCase):
         self.assertTrue(form.is_valid())
         self.assertTrue(form.save())
 
+    def test_vc_member_removal_blocked_by_cross_chassis_lag(self):
+        device1 = Device.objects.get(name='Device 1')
+        device2 = Device.objects.create(
+            name='Device 2', device_type=device1.device_type, role=device1.role, site=device1.site
+        )
+        vc = VirtualChassis.objects.create(name='VC 1', master=device1)
+        Device.objects.filter(pk=device1.pk).update(virtual_chassis=vc, vc_position=1)
+        Device.objects.filter(pk=device2.pk).update(virtual_chassis=vc, vc_position=2)
+        lag = Interface.objects.create(device=device1, name='lag0', type=InterfaceTypeChoices.TYPE_LAG)
+        Interface.objects.create(device=device2, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED, lag=lag)
+
+        device2.refresh_from_db()
+        form = DeviceForm(instance=device2, data={
+            'name': device2.name,
+            'role': device2.role.pk,
+            'device_type': device2.device_type.pk,
+            'site': device2.site.pk,
+            'status': DeviceStatusChoices.STATUS_ACTIVE,
+            'virtual_chassis': None,
+            'vc_position': None,
+        })
+        self.assertFalse(form.is_valid())
+        self.assertIn('virtual_chassis', form.errors)
+
     def test_non_racked_device_with_face(self):
         form = DeviceForm(data={
             'name': 'New Device',

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

@@ -3688,6 +3688,66 @@ class VirtualChassisTestCase(TestCase):
         self.assertEqual(device2.vc_position, 2)
         self.assertEqual(device2.vc_priority, 20)
 
+    @tag('regression')  # Ref: #23212
+    def test_member_removal_blocked_by_cross_chassis_lag(self):
+        """
+        Removing a device from a VirtualChassis must fail validation while any of its
+        interfaces form a LAG with another member, from either side of the LAG.
+        """
+        device1 = Device.objects.get(name='TestDevice1')
+        device2 = Device.objects.get(name='TestDevice2')
+
+        vc = VirtualChassis.objects.create(name='Test VC', master=device1)
+        Device.objects.filter(pk=device1.pk).update(virtual_chassis=vc, vc_position=1)
+        Device.objects.filter(pk=device2.pk).update(virtual_chassis=vc, vc_position=2)
+        device1.refresh_from_db()
+        device2.refresh_from_db()
+
+        lag = Interface.objects.create(device=device1, name='lag0', type=InterfaceTypeChoices.TYPE_LAG)
+        Interface.objects.create(
+            device=device2, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED, lag=lag
+        )
+
+        # Both devices remain valid while in the same VC
+        device1.full_clean()
+        device2.full_clean()
+
+        # Member interface side
+        device2.virtual_chassis = None
+        device2.vc_position = None
+        with self.assertRaises(ValidationError) as cm:
+            device2.full_clean()
+        self.assertIn('virtual_chassis', cm.exception.error_dict)
+
+        # LAG interface side
+        VirtualChassis.objects.filter(pk=vc.pk).update(master=device2)
+        device1.virtual_chassis = None
+        device1.vc_position = None
+        with self.assertRaises(ValidationError) as cm:
+            device1.full_clean()
+        self.assertIn('virtual_chassis', cm.exception.error_dict)
+
+    def test_member_removal_allowed_with_local_lag(self):
+        """
+        A device may leave a VirtualChassis when its LAG members are all local.
+        """
+        device1 = Device.objects.get(name='TestDevice1')
+        device2 = Device.objects.get(name='TestDevice2')
+
+        vc = VirtualChassis.objects.create(name='Test VC', master=device1)
+        Device.objects.filter(pk=device1.pk).update(virtual_chassis=vc, vc_position=1)
+        Device.objects.filter(pk=device2.pk).update(virtual_chassis=vc, vc_position=2)
+        device2.refresh_from_db()
+
+        lag = Interface.objects.create(device=device2, name='lag0', type=InterfaceTypeChoices.TYPE_LAG)
+        Interface.objects.create(
+            device=device2, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED, lag=lag
+        )
+
+        device2.virtual_chassis = None
+        device2.vc_position = None
+        device2.full_clean()
+
     def test_virtualchassis_duplicate_vc_position(self):
         """
         Test that two devices cannot be assigned to the same vc_position

+ 28 - 0
netbox/dcim/tests/test_views.py

@@ -5602,6 +5602,34 @@ class VirtualChassisTestCase(ViewTestCases.PrimaryObjectViewTestCase):
             'domain': 'domain-x',
         }
 
+    @tag('regression')  # Ref: #23212
+    def test_remove_member_blocked_by_cross_chassis_lag(self):
+        self.add_permissions('dcim.change_device')
+        master = Device.objects.get(name='Device 1')
+        member = Device.objects.get(name='Device 2')
+        lag = Interface.objects.create(device=master, name='lag0', type=InterfaceTypeChoices.TYPE_LAG)
+        Interface.objects.create(device=member, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED, lag=lag)
+
+        url = reverse('dcim:virtualchassis_remove_member', kwargs={'pk': member.pk})
+        response = self.client.post(url, {'confirm': True})
+        self.assertEqual(response.status_code, 302)
+
+        member.refresh_from_db()
+        self.assertEqual(member.virtual_chassis.name, 'VC1')
+        self.assertEqual(member.vc_position, 2)
+
+    def test_remove_member(self):
+        self.add_permissions('dcim.change_device')
+        member = Device.objects.get(name='Device 2')
+
+        url = reverse('dcim:virtualchassis_remove_member', kwargs={'pk': member.pk})
+        response = self.client.post(url, {'confirm': True})
+        self.assertEqual(response.status_code, 302)
+
+        member.refresh_from_db()
+        self.assertIsNone(member.virtual_chassis)
+        self.assertIsNone(member.vc_position)
+
 
 class PowerPanelTestCase(ViewTestCases.PrimaryObjectViewTestCase):
     model = PowerPanel

+ 8 - 1
netbox/dcim/views.py

@@ -5356,15 +5356,22 @@ class VirtualChassisRemoveMemberView(ObjectPermissionRequiredMixin, GetReturnURL
             return redirect(device.get_absolute_url())
 
         if form.is_valid():
+            virtual_chassis = device.virtual_chassis
             device.snapshot()
             device.virtual_chassis = None
             device.vc_position = None
             device.vc_priority = None
+            try:
+                device.full_clean()
+            except ValidationError as e:
+                for message in e.messages:
+                    messages.error(request, message)
+                return redirect(self.get_return_url(request, device))
             device.save()
 
             msg = _('Removed {device} from virtual chassis {chassis}').format(
                 device=device,
-                chassis=device.virtual_chassis
+                chassis=virtual_chassis
             )
             messages.success(request, msg)