diff --git a/netbox/dcim/api/serializers.py b/netbox/dcim/api/serializers.py index 979b580a0..903953cd2 100644 --- a/netbox/dcim/api/serializers.py +++ b/netbox/dcim/api/serializers.py @@ -584,22 +584,15 @@ class InterfaceSerializer(TaggedObjectSerializer, CableTerminationSerializer, Co 'count_ipaddresses', ] - # TODO: This validation should be handled by Interface.clean() def validate(self, data): - # All associated VLANs be global or assigned to the parent device's site. + # Validate many-to-many VLAN assignments device = self.instance.device if self.instance else data.get('device') - untagged_vlan = data.get('untagged_vlan') - if untagged_vlan and untagged_vlan.site not in [device.site, None]: - raise serializers.ValidationError({ - 'untagged_vlan': "VLAN {} must belong to the same site as the interface's parent device, or it must be " - "global.".format(untagged_vlan) - }) for vlan in data.get('tagged_vlans', []): if vlan.site not in [device.site, None]: raise serializers.ValidationError({ - 'tagged_vlans': "VLAN {} must belong to the same site as the interface's parent device, or it must " - "be global.".format(vlan) + 'tagged_vlans': f"VLAN {vlan} must belong to the same site as the interface's parent device, or " + f"it must be global." }) return super().validate(data) diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index 180a3d9cd..8320da2f0 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -89,13 +89,12 @@ class DeviceComponentFilterForm(BootstrapMixin, forms.Form): ) -class InterfaceCommonForm: +class InterfaceCommonForm(forms.Form): def clean(self): - super().clean() - # Validate VLAN assignments + parent_field = 'device' if 'device' in self.cleaned_data else 'virtual_machine' tagged_vlans = self.cleaned_data['tagged_vlans'] # Untagged interfaces cannot be assigned tagged VLANs @@ -110,13 +109,13 @@ class InterfaceCommonForm: # Validate tagged VLANs; must be a global VLAN or in the same site elif self.cleaned_data['mode'] == InterfaceModeChoices.MODE_TAGGED: - valid_sites = [None, self.cleaned_data['device'].site] + 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] if invalid_vlans: raise forms.ValidationError({ - 'tagged_vlans': "The tagged VLANs ({}) must belong to the same site as the interface's parent " - "device/VM, or they must be global".format(', '.join(invalid_vlans)) + 'tagged_vlans': f"The tagged VLANs ({', '.join(invalid_vlans)}) must belong to the same site as " + f"the interface's parent device/VM, or they must be global" }) @@ -2682,7 +2681,7 @@ class InterfaceFilterForm(DeviceComponentFilterForm): tag = TagFilterField(model) -class InterfaceForm(InterfaceCommonForm, BootstrapMixin, forms.ModelForm): +class InterfaceForm(BootstrapMixin, InterfaceCommonForm, forms.ModelForm): untagged_vlan = DynamicModelChoiceField( queryset=VLAN.objects.all(), required=False, diff --git a/netbox/virtualization/api/serializers.py b/netbox/virtualization/api/serializers.py index 3e977d94d..518b7086c 100644 --- a/netbox/virtualization/api/serializers.py +++ b/netbox/virtualization/api/serializers.py @@ -116,3 +116,16 @@ class VMInterfaceSerializer(TaggedObjectSerializer, ValidatedModelSerializer): 'id', 'url', 'virtual_machine', 'name', 'enabled', 'mtu', 'mac_address', 'description', 'mode', 'untagged_vlan', 'tagged_vlans', 'tags', ] + + def validate(self, data): + + # Validate many-to-many VLAN assignments + virtual_machine = self.instance.virtual_machine if self.instance else data.get('virtual_machine') + for vlan in data.get('tagged_vlans', []): + if vlan.site not in [virtual_machine.site, None]: + raise serializers.ValidationError({ + 'tagged_vlans': f"VLAN {vlan} must belong to the same site as the interface's parent virtual " + f"machine, or it must be global." + }) + + return super().validate(data) diff --git a/netbox/virtualization/forms.py b/netbox/virtualization/forms.py index ce4ca3e3c..edacb3e07 100644 --- a/netbox/virtualization/forms.py +++ b/netbox/virtualization/forms.py @@ -4,7 +4,7 @@ from django.core.exceptions import ValidationError from dcim.choices import InterfaceModeChoices from dcim.constants import INTERFACE_MTU_MAX, INTERFACE_MTU_MIN -from dcim.forms import INTERFACE_MODE_HELP_TEXT +from dcim.forms import InterfaceCommonForm, INTERFACE_MODE_HELP_TEXT from dcim.models import Device, DeviceRole, Platform, Rack, Region, Site from extras.forms import ( AddRemoveTagsForm, CustomFieldBulkEditForm, CustomFieldModelCSVForm, CustomFieldModelForm, CustomFieldFilterForm, @@ -543,10 +543,11 @@ class VirtualMachineFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFil # VM interfaces # -class VMInterfaceForm(BootstrapMixin, forms.ModelForm): +class VMInterfaceForm(BootstrapMixin, InterfaceCommonForm, forms.ModelForm): untagged_vlan = DynamicModelChoiceField( queryset=VLAN.objects.all(), required=False, + label='Untagged VLAN', display_field='display_name', brief_mode=False, query_params={ @@ -556,6 +557,7 @@ class VMInterfaceForm(BootstrapMixin, forms.ModelForm): tagged_vlans = DynamicModelMultipleChoiceField( queryset=VLAN.objects.all(), required=False, + label='Tagged VLANs', display_field='display_name', brief_mode=False, query_params={ @@ -597,24 +599,8 @@ class VMInterfaceForm(BootstrapMixin, forms.ModelForm): self.fields['untagged_vlan'].widget.add_query_param('site_id', site.pk) self.fields['tagged_vlans'].widget.add_query_param('site_id', site.pk) - def clean(self): - super().clean() - # Validate VLAN assignments - tagged_vlans = self.cleaned_data['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: - self.cleaned_data['tagged_vlans'] = [] - - -class VMInterfaceCreateForm(BootstrapMixin, forms.Form): +class VMInterfaceCreateForm(BootstrapMixin, InterfaceCommonForm): virtual_machine = DynamicModelChoiceField( queryset=VirtualMachine.objects.all() ) diff --git a/netbox/virtualization/models.py b/netbox/virtualization/models.py index 701eb8b79..f6986a6eb 100644 --- a/netbox/virtualization/models.py +++ b/netbox/virtualization/models.py @@ -5,7 +5,6 @@ from django.db import models from django.urls import reverse from taggit.managers import TaggableManager -from dcim.choices import InterfaceModeChoices from dcim.models import BaseInterface, Device from extras.models import ChangeLoggedModel, ConfigContextModel, CustomFieldModel, ObjectChange, TaggedItem from extras.querysets import ConfigContextModelQuerySet @@ -449,8 +448,8 @@ class VMInterface(BaseInterface): # Validate untagged VLAN if self.untagged_vlan and self.untagged_vlan.site not in [self.virtual_machine.site, None]: raise ValidationError({ - 'untagged_vlan': "The untagged VLAN ({}) must belong to the same site as the interface's parent " - "virtual machine, or it must be global".format(self.untagged_vlan) + 'untagged_vlan': f"The untagged VLAN ({self.untagged_vlan}) must belong to the same site as the " + f"interface's parent virtual machine, or it must be global" }) def to_objectchange(self, action):