From 05570ae4ade98eca869a07767f932af23d06e3a6 Mon Sep 17 00:00:00 2001 From: Saria Hajjar Date: Tue, 31 Dec 2019 20:47:13 +0000 Subject: [PATCH 1/9] Clean tagged VLANs --- netbox/dcim/forms.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index 1774fc986..e4bd12c80 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -74,6 +74,15 @@ class InterfaceCommonForm: elif self.cleaned_data['mode'] == IFACE_MODE_TAGGED_ALL: self.cleaned_data['tagged_vlans'] = [] + # Validate tagged VLANs; must be a global VLAN or in the same site + else: + for tagged_vlan in tagged_vlans: + if tagged_vlan.site not in [self.cleaned_data['device'].site, None]: + raise forms.ValidationError({ + 'tagged_vlans': "The tagged VLAN ({}) must belong to the same site as the interface's parent " + "device/VM, or it must be global".format(tagged_vlan) + }) + class BulkRenameForm(forms.Form): """ From aa4f73ffbfa3c23a5d64595753f18a32fc672dfd Mon Sep 17 00:00:00 2001 From: Saria Hajjar Date: Wed, 1 Jan 2020 12:09:51 +0000 Subject: [PATCH 2/9] Removed trailing space --- netbox/dcim/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index e4bd12c80..19fcc30dc 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -74,7 +74,7 @@ class InterfaceCommonForm: elif self.cleaned_data['mode'] == IFACE_MODE_TAGGED_ALL: self.cleaned_data['tagged_vlans'] = [] - # Validate tagged VLANs; must be a global VLAN or in the same site + # Validate tagged VLANs; must be a global VLAN or in the same site else: for tagged_vlan in tagged_vlans: if tagged_vlan.site not in [self.cleaned_data['device'].site, None]: From a110b0badbd8370e06ce8c0edfb1d82640a6bebe Mon Sep 17 00:00:00 2001 From: Saria Hajjar Date: Wed, 1 Jan 2020 16:43:21 +0000 Subject: [PATCH 3/9] Removed no-longer-used choices (now handled via API-based select) --- netbox/dcim/forms.py | 90 -------------------------------------------- 1 file changed, 90 deletions(-) diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index 19fcc30dc..da4beb6bb 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -2236,36 +2236,6 @@ class InterfaceForm(InterfaceCommonForm, BootstrapMixin, forms.ModelForm): device__in=[self.instance.device, self.instance.device.get_vc_master()], type=IFACE_TYPE_LAG ) - # Limit VLan choices to those in: global vlans, global groups, the current site's group, the current site - vlan_choices = [] - global_vlans = VLAN.objects.filter(site=None, group=None) - vlan_choices.append( - ('Global', [(vlan.pk, vlan) for vlan in global_vlans]) - ) - for group in VLANGroup.objects.filter(site=None): - global_group_vlans = VLAN.objects.filter(group=group) - vlan_choices.append( - (group.name, [(vlan.pk, vlan) for vlan in global_group_vlans]) - ) - - site = getattr(self.instance.parent, 'site', None) - if site is not None: - - # Add non-grouped site VLANs - site_vlans = VLAN.objects.filter(site=site, group=None) - vlan_choices.append((site.name, [(vlan.pk, vlan) for vlan in site_vlans])) - - # Add grouped site VLANs - for group in VLANGroup.objects.filter(site=site): - site_group_vlans = VLAN.objects.filter(group=group) - vlan_choices.append(( - '{} / {}'.format(group.site.name, group.name), - [(vlan.pk, vlan) for vlan in site_group_vlans] - )) - - self.fields['untagged_vlan'].choices = [(None, '---------')] + vlan_choices - self.fields['tagged_vlans'].choices = vlan_choices - class InterfaceCreateForm(InterfaceCommonForm, ComponentForm, forms.Form): name_pattern = ExpandableNameField( @@ -2346,36 +2316,6 @@ class InterfaceCreateForm(InterfaceCommonForm, ComponentForm, forms.Form): else: self.fields['lag'].queryset = Interface.objects.none() - # Limit VLan choices to those in: global vlans, global groups, the current site's group, the current site - vlan_choices = [] - global_vlans = VLAN.objects.filter(site=None, group=None) - vlan_choices.append( - ('Global', [(vlan.pk, vlan) for vlan in global_vlans]) - ) - for group in VLANGroup.objects.filter(site=None): - global_group_vlans = VLAN.objects.filter(group=group) - vlan_choices.append( - (group.name, [(vlan.pk, vlan) for vlan in global_group_vlans]) - ) - - site = getattr(self.parent, 'site', None) - if site is not None: - - # Add non-grouped site VLANs - site_vlans = VLAN.objects.filter(site=site, group=None) - vlan_choices.append((site.name, [(vlan.pk, vlan) for vlan in site_vlans])) - - # Add grouped site VLANs - for group in VLANGroup.objects.filter(site=site): - site_group_vlans = VLAN.objects.filter(group=group) - vlan_choices.append(( - '{} / {}'.format(group.site.name, group.name), - [(vlan.pk, vlan) for vlan in site_group_vlans] - )) - - self.fields['untagged_vlan'].choices = [(None, '---------')] + vlan_choices - self.fields['tagged_vlans'].choices = vlan_choices - class InterfaceBulkEditForm(InterfaceCommonForm, BootstrapMixin, AddRemoveTagsForm, BulkEditForm): pk = forms.ModelMultipleChoiceField( @@ -2458,36 +2398,6 @@ class InterfaceBulkEditForm(InterfaceCommonForm, BootstrapMixin, AddRemoveTagsFo else: self.fields['lag'].choices = [] - # Limit VLan choices to those in: global vlans, global groups, the current site's group, the current site - vlan_choices = [] - global_vlans = VLAN.objects.filter(site=None, group=None) - vlan_choices.append( - ('Global', [(vlan.pk, vlan) for vlan in global_vlans]) - ) - for group in VLANGroup.objects.filter(site=None): - global_group_vlans = VLAN.objects.filter(group=group) - vlan_choices.append( - (group.name, [(vlan.pk, vlan) for vlan in global_group_vlans]) - ) - if self.parent_obj is not None: - site = getattr(self.parent_obj, 'site', None) - if site is not None: - - # Add non-grouped site VLANs - site_vlans = VLAN.objects.filter(site=site, group=None) - vlan_choices.append((site.name, [(vlan.pk, vlan) for vlan in site_vlans])) - - # Add grouped site VLANs - for group in VLANGroup.objects.filter(site=site): - site_group_vlans = VLAN.objects.filter(group=group) - vlan_choices.append(( - '{} / {}'.format(group.site.name, group.name), - [(vlan.pk, vlan) for vlan in site_group_vlans] - )) - - self.fields['untagged_vlan'].choices = [(None, '---------')] + vlan_choices - self.fields['tagged_vlans'].choices = vlan_choices - class InterfaceBulkRenameForm(BulkRenameForm): pk = forms.ModelMultipleChoiceField( From d267aeb62112015db2f4b1cdf90afda348bfd700 Mon Sep 17 00:00:00 2001 From: Saria Hajjar Date: Wed, 1 Jan 2020 17:34:26 +0000 Subject: [PATCH 4/9] Filter VLANs to only those in the current site or global --- netbox/dcim/forms.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index da4beb6bb..7fc50f54b 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -2185,6 +2185,9 @@ class InterfaceForm(InterfaceCommonForm, BootstrapMixin, forms.ModelForm): widget=APISelect( api_url="/api/ipam/vlans/", display_field='display_name', + additional_query_params={ + 'site_id': 'null' + }, full=True ) ) @@ -2194,6 +2197,9 @@ class InterfaceForm(InterfaceCommonForm, BootstrapMixin, forms.ModelForm): widget=APISelectMultiple( api_url="/api/ipam/vlans/", display_field='display_name', + additional_query_params={ + 'site_id': 'null' + }, full=True ) ) @@ -2236,6 +2242,10 @@ class InterfaceForm(InterfaceCommonForm, BootstrapMixin, forms.ModelForm): device__in=[self.instance.device, self.instance.device.get_vc_master()], type=IFACE_TYPE_LAG ) + # Add the current site to the list of filtered VLANs + self.fields['untagged_vlan'].widget.attrs['1-data-additional-query-param-site_id'] = self.instance.device.site.pk + self.fields['tagged_vlans'].widget.attrs['1-data-additional-query-param-site_id'] = self.instance.device.site.pk + class InterfaceCreateForm(InterfaceCommonForm, ComponentForm, forms.Form): name_pattern = ExpandableNameField( @@ -2287,6 +2297,9 @@ class InterfaceCreateForm(InterfaceCommonForm, ComponentForm, forms.Form): widget=APISelect( api_url="/api/ipam/vlans/", display_field='display_name', + additional_query_params={ + 'site_id': 'null' + }, full=True ) ) @@ -2296,6 +2309,9 @@ class InterfaceCreateForm(InterfaceCommonForm, ComponentForm, forms.Form): widget=APISelectMultiple( api_url="/api/ipam/vlans/", display_field='display_name', + additional_query_params={ + 'site_id': 'null' + }, full=True ) ) @@ -2316,6 +2332,10 @@ class InterfaceCreateForm(InterfaceCommonForm, ComponentForm, forms.Form): else: self.fields['lag'].queryset = Interface.objects.none() + # Add the current site to the list of filtered VLANs + self.fields['untagged_vlan'].widget.attrs['1-data-additional-query-param-site_id'] = self.parent.site.pk + self.fields['tagged_vlans'].widget.attrs['1-data-additional-query-param-site_id'] = self.parent.site.pk + class InterfaceBulkEditForm(InterfaceCommonForm, BootstrapMixin, AddRemoveTagsForm, BulkEditForm): pk = forms.ModelMultipleChoiceField( @@ -2367,6 +2387,9 @@ class InterfaceBulkEditForm(InterfaceCommonForm, BootstrapMixin, AddRemoveTagsFo widget=APISelect( api_url="/api/ipam/vlans/", display_field='display_name', + additional_query_params={ + 'site_id': 'null' + }, full=True ) ) @@ -2376,6 +2399,9 @@ class InterfaceBulkEditForm(InterfaceCommonForm, BootstrapMixin, AddRemoveTagsFo widget=APISelectMultiple( api_url="/api/ipam/vlans/", display_field='display_name', + additional_query_params={ + 'site_id': 'null' + }, full=True ) ) @@ -2398,6 +2424,10 @@ class InterfaceBulkEditForm(InterfaceCommonForm, BootstrapMixin, AddRemoveTagsFo else: self.fields['lag'].choices = [] + # Add the current site to the list of filtered VLANs + self.fields['untagged_vlan'].widget.attrs['1-data-additional-query-param-site_id'] = self.parent_obj.site.pk + self.fields['tagged_vlans'].widget.attrs['1-data-additional-query-param-site_id'] = self.parent_obj.site.pk + class InterfaceBulkRenameForm(BulkRenameForm): pk = forms.ModelMultipleChoiceField( From 1a57120b789435b1bc27139e0563bfe7a984794a Mon Sep 17 00:00:00 2001 From: Saria Hajjar Date: Wed, 1 Jan 2020 17:43:39 +0000 Subject: [PATCH 5/9] 3589 changelog --- docs/release-notes/version-2.6.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/version-2.6.md b/docs/release-notes/version-2.6.md index 548492438..c6a509ab0 100644 --- a/docs/release-notes/version-2.6.md +++ b/docs/release-notes/version-2.6.md @@ -9,6 +9,7 @@ ## Bug Fixes * [#3106](https://github.com/netbox-community/netbox/issues/3106) - Restrict queryset of chained fields when form validation fails +* [#3589](https://github.com/netbox-community/netbox/issues/3589) - Limit the list of available VLANs to those valid and perform input validation on the tagged VLANs * [#3695](https://github.com/netbox-community/netbox/issues/3695) - Include A/Z termination sites for circuits in global search * [#3712](https://github.com/netbox-community/netbox/issues/3712) - Scrolling to target (hash) did not account for the header size * [#3780](https://github.com/netbox-community/netbox/issues/3780) - Fix AttributeError exception in API docs From 7ad9e8a2fbd5e655143a7a40d2d4b41765dd1f4d Mon Sep 17 00:00:00 2001 From: Saria Hajjar Date: Wed, 1 Jan 2020 19:53:13 +0000 Subject: [PATCH 6/9] More informative error message --- netbox/dcim/forms.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index 7fc50f54b..3c515152e 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -76,12 +76,14 @@ class InterfaceCommonForm: # Validate tagged VLANs; must be a global VLAN or in the same site else: - for tagged_vlan in tagged_vlans: - if tagged_vlan.site not in [self.cleaned_data['device'].site, None]: - raise forms.ValidationError({ - 'tagged_vlans': "The tagged VLAN ({}) must belong to the same site as the interface's parent " - "device/VM, or it must be global".format(tagged_vlan) - }) + valid_sites = [None, self.cleaned_data['device'].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)) + }) class BulkRenameForm(forms.Form): From d3c6caf8a8db5cc643f0f5ed7fab2c78dff97c28 Mon Sep 17 00:00:00 2001 From: hSaria <34197532+hSaria@users.noreply.github.com> Date: Thu, 2 Jan 2020 17:52:22 +0000 Subject: [PATCH 7/9] Limit tagged validation to tagged interfaces Co-Authored-By: Jeremy Stretch --- netbox/dcim/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index 3c515152e..902541353 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -75,7 +75,7 @@ class InterfaceCommonForm: self.cleaned_data['tagged_vlans'] = [] # Validate tagged VLANs; must be a global VLAN or in the same site - else: + elif self.cleaned_data['mode'] == IFACE_MODE_TAGGED: valid_sites = [None, self.cleaned_data['device'].site] invalid_vlans = [str(v) for v in tagged_vlans if v.site not in valid_sites] From c45daca5f2376ce5b061b232f6c3a80b40aa6a0e Mon Sep 17 00:00:00 2001 From: Saria Hajjar Date: Sun, 5 Jan 2020 09:10:46 +0000 Subject: [PATCH 8/9] Removed changes that will be covered in #3840 --- netbox/dcim/forms.py | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index 932e9695a..52f49558f 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -2210,9 +2210,6 @@ class InterfaceForm(InterfaceCommonForm, BootstrapMixin, forms.ModelForm): widget=APISelect( api_url="/api/ipam/vlans/", display_field='display_name', - additional_query_params={ - 'site_id': 'null' - }, full=True ) ) @@ -2222,9 +2219,6 @@ class InterfaceForm(InterfaceCommonForm, BootstrapMixin, forms.ModelForm): widget=APISelectMultiple( api_url="/api/ipam/vlans/", display_field='display_name', - additional_query_params={ - 'site_id': 'null' - }, full=True ) ) @@ -2267,10 +2261,6 @@ class InterfaceForm(InterfaceCommonForm, BootstrapMixin, forms.ModelForm): device__in=[self.instance.device, self.instance.device.get_vc_master()], type=IFACE_TYPE_LAG ) - # Add the current site to the list of filtered VLANs - self.fields['untagged_vlan'].widget.attrs['1-data-additional-query-param-site_id'] = self.instance.device.site.pk - self.fields['tagged_vlans'].widget.attrs['1-data-additional-query-param-site_id'] = self.instance.device.site.pk - class InterfaceCreateForm(InterfaceCommonForm, ComponentForm, forms.Form): name_pattern = ExpandableNameField( @@ -2322,9 +2312,6 @@ class InterfaceCreateForm(InterfaceCommonForm, ComponentForm, forms.Form): widget=APISelect( api_url="/api/ipam/vlans/", display_field='display_name', - additional_query_params={ - 'site_id': 'null' - }, full=True ) ) @@ -2334,9 +2321,6 @@ class InterfaceCreateForm(InterfaceCommonForm, ComponentForm, forms.Form): widget=APISelectMultiple( api_url="/api/ipam/vlans/", display_field='display_name', - additional_query_params={ - 'site_id': 'null' - }, full=True ) ) @@ -2357,10 +2341,6 @@ class InterfaceCreateForm(InterfaceCommonForm, ComponentForm, forms.Form): else: self.fields['lag'].queryset = Interface.objects.none() - # Add the current site to the list of filtered VLANs - self.fields['untagged_vlan'].widget.attrs['1-data-additional-query-param-site_id'] = self.parent.site.pk - self.fields['tagged_vlans'].widget.attrs['1-data-additional-query-param-site_id'] = self.parent.site.pk - class InterfaceBulkEditForm(InterfaceCommonForm, BootstrapMixin, AddRemoveTagsForm, BulkEditForm): pk = forms.ModelMultipleChoiceField( @@ -2412,9 +2392,6 @@ class InterfaceBulkEditForm(InterfaceCommonForm, BootstrapMixin, AddRemoveTagsFo widget=APISelect( api_url="/api/ipam/vlans/", display_field='display_name', - additional_query_params={ - 'site_id': 'null' - }, full=True ) ) @@ -2424,9 +2401,6 @@ class InterfaceBulkEditForm(InterfaceCommonForm, BootstrapMixin, AddRemoveTagsFo widget=APISelectMultiple( api_url="/api/ipam/vlans/", display_field='display_name', - additional_query_params={ - 'site_id': 'null' - }, full=True ) ) @@ -2449,10 +2423,6 @@ class InterfaceBulkEditForm(InterfaceCommonForm, BootstrapMixin, AddRemoveTagsFo else: self.fields['lag'].choices = [] - # Add the current site to the list of filtered VLANs - self.fields['untagged_vlan'].widget.attrs['1-data-additional-query-param-site_id'] = self.parent_obj.site.pk - self.fields['tagged_vlans'].widget.attrs['1-data-additional-query-param-site_id'] = self.parent_obj.site.pk - class InterfaceBulkRenameForm(BulkRenameForm): pk = forms.ModelMultipleChoiceField( From 7dddd4734ce5225aad0bcf1fecbf7d9c7aff1d70 Mon Sep 17 00:00:00 2001 From: Saria Hajjar Date: Sun, 5 Jan 2020 09:11:58 +0000 Subject: [PATCH 9/9] Updated changelog --- docs/release-notes/version-2.6.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/release-notes/version-2.6.md b/docs/release-notes/version-2.6.md index 77c2f42b7..6b4cebae7 100644 --- a/docs/release-notes/version-2.6.md +++ b/docs/release-notes/version-2.6.md @@ -1,3 +1,11 @@ +# v2.6.12 (FUTURE) + +## Bug Fixes + +* [#3589](https://github.com/netbox-community/netbox/issues/3589) - Fix validation on tagged VLANs of an interface + +--- + # v2.6.11 (2020-01-03) ## Bug Fixes @@ -24,7 +32,6 @@ ## Bug Fixes * [#3106](https://github.com/netbox-community/netbox/issues/3106) - Restrict queryset of chained fields when form validation fails -* [#3589](https://github.com/netbox-community/netbox/issues/3589) - Limit the list of available VLANs to those valid and perform input validation on the tagged VLANs * [#3695](https://github.com/netbox-community/netbox/issues/3695) - Include A/Z termination sites for circuits in global search * [#3712](https://github.com/netbox-community/netbox/issues/3712) - Scrolling to target (hash) did not account for the header size * [#3780](https://github.com/netbox-community/netbox/issues/3780) - Fix AttributeError exception in API docs