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

netbox/dcim/forms/object_create.py

@@ -385,11 +385,19 @@ class VirtualChassisCreateForm(OwnerMixin, NetBoxModelForm):
     def clean(self):
         super().clean()
 
-        if self.cleaned_data['members'] and self.cleaned_data['initial_position'] is None:
+        members = self.cleaned_data.get('members')
+        if members and self.cleaned_data['initial_position'] is None:
             raise forms.ValidationError({
                 'initial_position': _("A position must be specified for the first VC member.")
             })
 
+        # Members are saved without full_clean(), so check cross-chassis LAGs here
+        for member in members or []:
+            try:
+                member.clean_vc_lag_membership(members=members)
+            except forms.ValidationError as e:
+                raise forms.ValidationError({'members': e.messages})
+
     def save(self, *args, **kwargs):
         instance = super().save(*args, **kwargs)
 

netbox/dcim/models/devices.py

@@ -1018,6 +1018,42 @@ class Device(
                 ).format(virtual_chassis=self.vc_master_for)
             })
 
+        # Validate cross-chassis LAGs only when virtual chassis membership changes
+        if self.pk:
+            stored_vc_id = Device.objects.filter(pk=self.pk).values_list('virtual_chassis_id', flat=True).first()
+            if self.virtual_chassis_id != stored_vc_id:
+                self.clean_vc_lag_membership()
+
+    def clean_vc_lag_membership(self, members=None):
+        """
+        Raise ValidationError if any interface would form a LAG with a device outside this device's virtual chassis.
+        `members` overrides the set of fellow chassis members (used when the chassis has not been saved yet).
+        """
+        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 members is not None:
+            lag_members = lag_members.exclude(lag__device__in=members)
+            lag_peers = lag_peers.exclude(device__in=members)
+        elif self.virtual_chassis_id:
+            lag_members = lag_members.exclude(lag__device__virtual_chassis=self.virtual_chassis_id)
+            lag_peers = lag_peers.exclude(device__virtual_chassis=self.virtual_chassis_id)
+        interfaces = [
+            *lag_members.select_related('device', 'lag__device'),
+            *lag_peers.select_related('device', 'lag__device'),
+        ]
+        if interfaces:
+            # Cap the listed interfaces to keep the message readable
+            max_listed = 5
+            listed = ', '.join(f'{i.device}:{i} ({i.lag.device}:{i.lag})' for i in interfaces[:max_listed])
+            if len(interfaces) > max_listed:
+                listed += _(' (and {count} more)').format(count=len(interfaces) - max_listed)
+            raise ValidationError({
+                'virtual_chassis': _(
+                    'Virtual chassis membership cannot be changed while interfaces form a LAG with another device '
+                    'outside the virtual chassis: {interfaces}'
+                ).format(interfaces=listed)
+            })
+
     def _check_duplicate_component_names(self, components):
         """
         Check for duplicate component names after resolving {vc_position} placeholders.

netbox/dcim/tests/test_api.py

@@ -6310,6 +6310,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

netbox/dcim/tests/test_forms.py

@@ -154,6 +154,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',

netbox/dcim/tests/test_models.py

@@ -3688,6 +3688,114 @@ 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('eth0', str(cm.exception.error_dict['virtual_chassis']))
+
+        # LAG interface side (refresh clears the cached vc_master_for so the master guard doesn't fire first)
+        VirtualChassis.objects.filter(pk=vc.pk).update(master=device2)
+        device1.refresh_from_db()
+        device1.virtual_chassis = None
+        device1.vc_position = None
+        with self.assertRaises(ValidationError) as cm:
+            device1.full_clean()
+        self.assertIn('eth0', str(cm.exception.error_dict['virtual_chassis']))
+
+    def test_member_removal_error_lists_at_most_five_interfaces(self):
+        """
+        The cross-chassis LAG error names at most five interfaces and summarizes the rest.
+        """
+        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=device1, name='lag0', type=InterfaceTypeChoices.TYPE_LAG)
+        Interface.objects.bulk_create([
+            Interface(device=device2, name=f'eth{i}', type=InterfaceTypeChoices.TYPE_1GE_FIXED, lag=lag)
+            for i in range(7)
+        ])
+
+        device2.virtual_chassis = None
+        device2.vc_position = None
+        with self.assertRaises(ValidationError) as cm:
+            device2.full_clean()
+        message = str(cm.exception.error_dict['virtual_chassis'])
+        self.assertEqual(message.count('lag0'), 5)
+        self.assertIn('(and 2 more)', message)
+
+    def test_unrelated_edit_allowed_with_existing_cross_chassis_lag(self):
+        """
+        Pre-existing cross-chassis LAG data must not block edits that leave VC membership unchanged.
+        """
+        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)
+        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
+        )
+
+        device1.refresh_from_db()
+        device2.refresh_from_db()
+        device1.description = 'changed'
+        device1.full_clean()
+        device2.description = 'changed'
+        device2.full_clean()
+
+    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

netbox/dcim/tests/test_views.py

@@ -5665,6 +5665,89 @@ 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)
+
+    @tag('regression')  # Ref: #23212
+    def test_add_member_blocked_by_cross_chassis_lag(self):
+        self.add_permissions('dcim.change_virtualchassis')
+        vc1_member = Device.objects.get(name='Device 1')
+        device = Device.objects.get(name='Device 10')
+        lag = Interface.objects.create(device=vc1_member, name='lag0', type=InterfaceTypeChoices.TYPE_LAG)
+        Interface.objects.create(device=device, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED, lag=lag)
+
+        vc2 = VirtualChassis.objects.get(name='VC2')
+        url = reverse('dcim:virtualchassis_add_member', kwargs={'pk': vc2.pk})
+        response = self.client.post(url, {'device': device.pk, 'vc_position': 4, 'vc_priority': ''})
+        self.assertHttpStatus(response, 200)
+        self.assertIn('outside the virtual chassis', str(response.content))
+
+        device.refresh_from_db()
+        self.assertIsNone(device.virtual_chassis)
+        self.assertIsNone(device.vc_position)
+
+    @tag('regression')  # Ref: #23212
+    def test_create_with_members_blocked_by_cross_chassis_lag(self):
+        self.add_permissions('dcim.add_virtualchassis', 'dcim.view_device')
+        vc1_member = Device.objects.get(name='Device 1')
+        device = Device.objects.get(name='Device 10')
+        lag = Interface.objects.create(device=vc1_member, name='lag0', type=InterfaceTypeChoices.TYPE_LAG)
+        Interface.objects.create(device=device, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED, lag=lag)
+
+        data = {'name': 'VC4', 'members': [device.pk], 'initial_position': 1}
+        response = self.client.post(reverse('dcim:virtualchassis_add'), data)
+        self.assertHttpStatus(response, 200)
+        self.assertIn('outside the virtual chassis', str(response.content))
+        self.assertFalse(VirtualChassis.objects.filter(name='VC4').exists())
+
+        device.refresh_from_db()
+        self.assertIsNone(device.virtual_chassis)
+
+    def test_create_with_members_allowed_with_lag_between_new_members(self):
+        self.add_permissions('dcim.add_virtualchassis', 'dcim.view_device')
+        device1 = Device.objects.get(name='Device 10')
+        device2 = Device.objects.get(name='Device 11')
+        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)
+
+        data = {'name': 'VC4', 'members': [device1.pk, device2.pk], 'initial_position': 1}
+        response = self.client.post(reverse('dcim:virtualchassis_add'), data)
+        self.assertHttpStatus(response, 302)
+
+        vc = VirtualChassis.objects.get(name='VC4')
+        self.assertEqual(set(vc.members.values_list('name', flat=True)), {'Device 10', 'Device 11'})
+
+    def test_remove_member(self):
+        self.add_permissions('dcim.change_device')
+        master = Device.objects.get(name='Device 1')
+        member = Device.objects.get(name='Device 2')
+
+        # A primary IP on another member's interface must not block removal
+        interface = Interface.objects.create(device=master, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED)
+        ip = IPAddress.objects.create(address='192.0.2.1/24', assigned_object=interface)
+        Device.objects.filter(pk=member.pk).update(primary_ip4=ip)
+
+        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

netbox/dcim/views.py

@@ -5303,6 +5303,13 @@ class VirtualChassisAddMemberView(ObjectPermissionRequiredMixin, GetReturnURLMix
             }
             membership_form = forms.DeviceVCMembershipForm(data=data, validate_vc_position=True, instance=device)
 
+            # DeviceVCMembershipForm has no virtual_chassis field, so this error cannot surface through is_valid()
+            try:
+                device.clean_vc_lag_membership()
+            except ValidationError as e:
+                membership_form = forms.DeviceVCMembershipForm(data=data)
+                membership_form.add_error(None, e.messages)
+
             if membership_form.is_valid():
                 membership_form.save()
                 messages.success(request, mark_safe(
@@ -5356,15 +5363,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.clean_vc_lag_membership()
+            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)