From 8ee5d0a08aa9ae90855889363293c6e756334302 Mon Sep 17 00:00:00 2001 From: Daniel Sheppard Date: Mon, 24 Feb 2025 17:12:00 -0600 Subject: [PATCH] Update serializer, form and tests --- .../api/serializers_/device_components.py | 37 ++++++++- netbox/dcim/forms/common.py | 14 +--- netbox/dcim/tests/test_api.py | 83 +++++++++++++++---- netbox/dcim/tests/test_forms.py | 55 +++++++++++- 4 files changed, 158 insertions(+), 31 deletions(-) diff --git a/netbox/dcim/api/serializers_/device_components.py b/netbox/dcim/api/serializers_/device_components.py index ed1a5156b..339806a08 100644 --- a/netbox/dcim/api/serializers_/device_components.py +++ b/netbox/dcim/api/serializers_/device_components.py @@ -241,16 +241,51 @@ class InterfaceSerializer(NetBoxModelSerializer, CabledObjectSerializer, Connect if self.instance: mode = data.get('mode') if 'mode' in data.keys() else self.instance.mode + untagged_vlan = data.get('untagged_vlan') if 'untagged_vlan' in data.keys() else \ + self.instance.untagged_vlan + qinq_svlan = data.get('qinq_svlan') if 'qinq_svlan' in data.keys() else \ + self.instance.qinq_svlan tagged_vlans = data.get('tagged_vlans') if 'tagged_vlans' in data.keys() else \ self.instance.tagged_vlans.all() else: mode = data.get('mode', None) + untagged_vlan = data.get('untagged_vlan') if 'untagged_vlan' in data.keys() else None + qinq_svlan = data.get('qinq_svlan') if 'qinq_svlan' in data.keys() else None tagged_vlans = data.get('tagged_vlans') if 'tagged_vlans' in data.keys() else None - if mode != InterfaceModeChoices.MODE_TAGGED and tagged_vlans: + if not mode: + if untagged_vlan and tagged_vlans and qinq_svlan: + raise serializers.ValidationError({ + 'untagged_vlan': _("Interface mode does not support untagged vlan"), + 'qinq_svlan': _("Interface mode does not support q-in-q service vlan"), + 'tagged_vlans': _("Interface mode does not support tagged vlans") + }) + elif untagged_vlan and tagged_vlans: + raise serializers.ValidationError({ + 'untagged_vlan': _("Interface mode does not support untagged vlan"), + 'qinq_svlan': _("Interface mode does not support q-in-q service vlan"), + 'tagged_vlans': _("Interface mode does not support tagged vlans") + }) + elif untagged_vlan: + raise serializers.ValidationError({ + 'untagged_vlan': _("Interface mode does not support untagged vlan") + }) + elif tagged_vlans: + raise serializers.ValidationError({ + 'tagged_vlans': _("Interface mode does not support tagged vlans") + }) + elif qinq_svlan: + raise serializers.ValidationError({ + 'qinq_svlan': _("Interface mode does not support q-in-q service vlan") + }) + elif mode in (InterfaceModeChoices.MODE_TAGGED_ALL, InterfaceModeChoices.MODE_ACCESS) and tagged_vlans: raise serializers.ValidationError({ 'tagged_vlans': _("Interface mode does not support tagged vlans") }) + elif mode != InterfaceModeChoices.MODE_Q_IN_Q and qinq_svlan: + raise serializers.ValidationError({ + 'qinq_svlan': _("Interface mode does not support q-in-q service vlan") + }) # Validate many-to-many VLAN assignments device = self.instance.device if self.instance else data.get('device') diff --git a/netbox/dcim/forms/common.py b/netbox/dcim/forms/common.py index ad4064f66..23109f66b 100644 --- a/netbox/dcim/forms/common.py +++ b/netbox/dcim/forms/common.py @@ -49,20 +49,8 @@ class InterfaceCommonForm(forms.Form): else: tagged_vlans = [] - # Untagged interfaces cannot be assigned tagged VLANs - if self.cleaned_data['mode'] == InterfaceModeChoices.MODE_ACCESS and tagged_vlans: - raise forms.ValidationError({ - 'mode': _("An access interface cannot have tagged VLANs assigned.") - }) - - # Remove all tagged VLAN assignments from "tagged all" interfaces - elif self.cleaned_data['mode'] == InterfaceModeChoices.MODE_TAGGED_ALL and tagged_vlans: - raise forms.ValidationError({ - 'mode': _("An tagged-all interface cannot have tagged VLANs assigned.") - }) - # Validate tagged VLANs; must be a global VLAN or in the same site - elif self.cleaned_data['mode'] == InterfaceModeChoices.MODE_TAGGED and tagged_vlans: + if self.cleaned_data['mode'] == InterfaceModeChoices.MODE_TAGGED and tagged_vlans: valid_sites = [None, self.cleaned_data[parent_field].site] invalid_vlans = [str(v) for v in tagged_vlans if v.site not in valid_sites] diff --git a/netbox/dcim/tests/test_api.py b/netbox/dcim/tests/test_api.py index 4e09f3aba..ea0ef5cbc 100644 --- a/netbox/dcim/tests/test_api.py +++ b/netbox/dcim/tests/test_api.py @@ -1750,6 +1750,23 @@ class InterfaceTest(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTestCase }, ] + def _perform_interface_test_with_invalid_data(self, mode: str = None, invalid_data: dict = {}): + device = Device.objects.first() + data = { + 'device': device.pk, + 'name': 'Interface 1', + 'type': InterfaceTypeChoices.TYPE_1GE_FIXED, + } + data.update({'mode': mode}) + data.update(invalid_data) + + response = self.client.post(self._get_list_url(), data, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST) + content = json.loads(response.content) + for key in invalid_data.keys(): + self.assertIn(key, content) + self.assertIsNone(content.get('data')) + def test_bulk_delete_child_interfaces(self): interface1 = Interface.objects.get(name='Interface 1') device = interface1.device @@ -1779,27 +1796,65 @@ class InterfaceTest(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTestCase def test_create_child_interfaces_mode_invalid_data(self): """ - POST a single object without permission. + POST data to test interface mode check and invalid tagged/untagged VLANS. """ self.add_permissions('dcim.add_interface') - device = Device.objects.first() vlans = VLAN.objects.all()[0:3] - create_data = { - 'device': device.pk, - 'name': 'Untagged Interface 1', - 'type': InterfaceTypeChoices.TYPE_1GE_FIXED, - 'mode': InterfaceModeChoices.MODE_ACCESS, - 'tagged_vlans': [vlans[0].pk, vlans[1].pk], - 'untagged_vlan': vlans[2].pk, + # No mode with all vlan fields set + invalid_data = { + 'untagged_vlan': vlans[0].pk, + 'tagged_vlans': [vlans[1].pk, vlans[2].pk], + 'qinq_svlan': vlans[2].pk } + self._perform_interface_test_with_invalid_data(None, invalid_data) - response = self.client.post(self._get_list_url(), create_data, format='json', **self.header) - self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST) - content = json.loads(response.content) - self.assertIn('tagged_vlans', content) - self.assertIsNone(content.get('data')) + # No mode with standard vlan fields set + invalid_data = { + 'untagged_vlan': vlans[0].pk, + 'tagged_vlans': [vlans[1].pk, vlans[2].pk], + } + self._perform_interface_test_with_invalid_data(None, invalid_data) + + # No mode with untagged vlan field set + invalid_data = { + 'untagged_vlan': vlans[0].pk, + } + self._perform_interface_test_with_invalid_data(None, invalid_data) + + # No mode with tagged vlans field set + invalid_data = { + 'tagged_vlans': [vlans[1].pk, vlans[2].pk], + } + self._perform_interface_test_with_invalid_data(None, invalid_data) + + # No mode with tagged vlans field set + invalid_data = { + 'qinq_svlan': vlans[0].pk, + } + self._perform_interface_test_with_invalid_data(None, invalid_data) + + # Access mode with tagged vlans field set + invalid_data = { + 'tagged_vlans': [vlans[1].pk, vlans[2].pk], + } + self._perform_interface_test_with_invalid_data(InterfaceModeChoices.MODE_ACCESS, invalid_data) + + # Tagged-All with tagged vlans field set + self._perform_interface_test_with_invalid_data(InterfaceModeChoices.MODE_TAGGED_ALL, invalid_data) + + # Access mode with tagged qinq_svlan field set + invalid_data = { + 'qinq_svlan': vlans[0].pk, + } + self._perform_interface_test_with_invalid_data(InterfaceModeChoices.MODE_ACCESS, invalid_data) + + # Tagged-All with tagged qinq_svlan field set + self._perform_interface_test_with_invalid_data(InterfaceModeChoices.MODE_TAGGED, invalid_data) + + # Tagged-All with tagged qinq_svlan field set + self._perform_interface_test_with_invalid_data(InterfaceModeChoices.MODE_TAGGED_ALL, invalid_data) class FrontPortTest(APIViewTestCases.APIViewTestCase): diff --git a/netbox/dcim/tests/test_forms.py b/netbox/dcim/tests/test_forms.py index 5495650b8..89b7508f3 100644 --- a/netbox/dcim/tests/test_forms.py +++ b/netbox/dcim/tests/test_forms.py @@ -220,6 +220,9 @@ class InterfaceTestCase(TestCase): form = InterfaceCreateForm(data) self.assertTrue(form.is_valid()) + self.assertIn('untagged_vlan', form.cleaned_data.keys()) + self.assertNotIn('tagged_vlans', form.cleaned_data.keys()) + self.assertNotIn('qinq_svlan', form.cleaned_data.keys()) def test_edit_interface_mode_access_invalid_data(self): """ @@ -235,6 +238,9 @@ class InterfaceTestCase(TestCase): form = InterfaceForm(data, instance=self.interface) self.assertTrue(form.is_valid()) + self.assertIn('untagged_vlan', form.cleaned_data.keys()) + self.assertNotIn('tagged_vlans', form.cleaned_data.keys()) + self.assertNotIn('qinq_svlan', form.cleaned_data.keys()) def test_create_interface_mode_tagged_all_invalid_data(self): """ @@ -245,11 +251,14 @@ class InterfaceTestCase(TestCase): 'name': 'ethernet1/6', 'type': InterfaceTypeChoices.TYPE_1GE_GBIC, 'mode': InterfaceModeChoices.MODE_TAGGED_ALL, - 'tagged_vlans': [self.vlans[0].pk, self.vlans[1].pk, self.vlans[2]] + 'tagged_vlans': [self.vlans[0].pk, self.vlans[1].pk, self.vlans[2].pk] } form = InterfaceCreateForm(data) self.assertTrue(form.is_valid()) + self.assertIn('untagged_vlan', form.cleaned_data.keys()) + self.assertNotIn('tagged_vlans', form.cleaned_data.keys()) + self.assertNotIn('qinq_svlan', form.cleaned_data.keys()) def test_edit_interface_mode_tagged_all_invalid_data(self): """ @@ -260,7 +269,47 @@ class InterfaceTestCase(TestCase): 'name': 'Ethernet 1/7', 'type': InterfaceTypeChoices.TYPE_1GE_GBIC, 'mode': InterfaceModeChoices.MODE_TAGGED_ALL, - 'tagged_vlans': [self.vlans[0].pk, self.vlans[1].pk, self.vlans[2]] + 'tagged_vlans': [self.vlans[0].pk, self.vlans[1].pk, self.vlans[2].pk] } - form = InterfaceForm(data, instance=self.interface) + form = InterfaceForm(data) self.assertTrue(form.is_valid()) + self.assertIn('untagged_vlan', form.cleaned_data.keys()) + self.assertNotIn('tagged_vlans', form.cleaned_data.keys()) + self.assertNotIn('qinq_svlan', form.cleaned_data.keys()) + + def test_create_interface_mode_routed_invalid_data(self): + """ + Test that saving invalid interface mode (routed) and tagged/untagged vlans works properly + """ + data = { + 'device': self.device.pk, + 'name': 'ethernet1/6', + 'type': InterfaceTypeChoices.TYPE_1GE_GBIC, + 'mode': None, + 'untagged_vlan': self.vlans[0].pk, + 'tagged_vlans': [self.vlans[0].pk, self.vlans[1].pk, self.vlans[2].pk] + } + form = InterfaceCreateForm(data) + + self.assertTrue(form.is_valid()) + self.assertNotIn('untagged_vlan', form.cleaned_data.keys()) + self.assertNotIn('tagged_vlans', form.cleaned_data.keys()) + self.assertNotIn('qinq_svlan', form.cleaned_data.keys()) + + def test_edit_interface_mode_routed_invalid_data(self): + """ + Test that saving invalid interface mode (routed) and tagged/untagged vlans works properly + """ + data = { + 'device': self.device.pk, + 'name': 'Ethernet 1/7', + 'type': InterfaceTypeChoices.TYPE_1GE_GBIC, + 'mode': None, + 'untagged_vlan': self.vlans[0].pk, + 'tagged_vlans': [self.vlans[0].pk, self.vlans[1].pk, self.vlans[2].pk] + } + form = InterfaceForm(data) + self.assertTrue(form.is_valid()) + self.assertNotIn('untagged_vlan', form.cleaned_data.keys()) + self.assertNotIn('tagged_vlans', form.cleaned_data.keys()) + self.assertNotIn('qinq_svlan', form.cleaned_data.keys())