From 4dae781be0a8f6aae995681cb93f25550b192379 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 9 Mar 2021 14:13:50 -0500 Subject: [PATCH 1/8] Change VLANGroup site to scope (GFK) --- netbox/ipam/api/serializers.py | 20 ++++- netbox/ipam/api/views.py | 2 +- netbox/ipam/filters.py | 55 +++++-------- netbox/ipam/forms.py | 77 ++++++++++++++++--- .../ipam/migrations/0045_vlangroup_scope.py | 36 +++++++++ netbox/ipam/models/vlans.py | 32 +++++--- netbox/ipam/tables.py | 6 +- 7 files changed, 169 insertions(+), 59 deletions(-) create mode 100644 netbox/ipam/migrations/0045_vlangroup_scope.py diff --git a/netbox/ipam/api/serializers.py b/netbox/ipam/api/serializers.py index 002ad3b89..aa0255e30 100644 --- a/netbox/ipam/api/serializers.py +++ b/netbox/ipam/api/serializers.py @@ -114,14 +114,20 @@ class RoleSerializer(OrganizationalModelSerializer): class VLANGroupSerializer(OrganizationalModelSerializer): url = serializers.HyperlinkedIdentityField(view_name='ipam-api:vlangroup-detail') - site = NestedSiteSerializer(required=False, allow_null=True) + scope_type = ContentTypeField( + queryset=ContentType.objects.filter( + app_label='dcim', + model__in=['region', 'sitegroup', 'site', 'location', 'rack'] + ) + ) + scope = serializers.SerializerMethodField(read_only=True) vlan_count = serializers.IntegerField(read_only=True) class Meta: model = VLANGroup fields = [ - 'id', 'url', 'name', 'slug', 'site', 'description', 'custom_fields', 'created', 'last_updated', - 'vlan_count', + 'id', 'url', 'name', 'slug', 'scope_type', 'scope_id', 'scope', 'description', 'custom_fields', 'created', + 'last_updated', 'vlan_count', ] validators = [] @@ -138,6 +144,14 @@ class VLANGroupSerializer(OrganizationalModelSerializer): return data + def get_scope(self, obj): + if obj.scope_id is None: + return None + serializer = get_serializer_for_model(obj.scope, prefix='Nested') + context = {'request': self.context['request']} + + return serializer(obj.scope, context=context).data + class VLANSerializer(TaggedObjectSerializer, CustomFieldModelSerializer): url = serializers.HyperlinkedIdentityField(view_name='ipam-api:vlan-detail') diff --git a/netbox/ipam/api/views.py b/netbox/ipam/api/views.py index b6f0a7463..1e1177772 100644 --- a/netbox/ipam/api/views.py +++ b/netbox/ipam/api/views.py @@ -283,7 +283,7 @@ class IPAddressViewSet(CustomFieldModelViewSet): # class VLANGroupViewSet(CustomFieldModelViewSet): - queryset = VLANGroup.objects.prefetch_related('site').annotate( + queryset = VLANGroup.objects.annotate( vlan_count=count_related(VLAN, 'group') ) serializer_class = serializers.VLANGroupSerializer diff --git a/netbox/ipam/filters.py b/netbox/ipam/filters.py index 5ca487065..1dff03144 100644 --- a/netbox/ipam/filters.py +++ b/netbox/ipam/filters.py @@ -1,5 +1,6 @@ import django_filters import netaddr +from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError from django.db.models import Q from netaddr.core import AddrFormatError @@ -8,8 +9,8 @@ from dcim.models import Device, Interface, Region, Site, SiteGroup from extras.filters import CustomFieldModelFilterSet, CreatedUpdatedFilterSet from tenancy.filters import TenancyFilterSet from utilities.filters import ( - BaseFilterSet, MultiValueCharFilter, MultiValueNumberFilter, NameSlugSearchFilterSet, NumericArrayFilter, TagFilter, - TreeNodeMultipleChoiceFilter, + BaseFilterSet, ContentTypeFilter, MultiValueCharFilter, MultiValueNumberFilter, NameSlugSearchFilterSet, + NumericArrayFilter, TagFilter, TreeNodeMultipleChoiceFilter, ) from virtualization.models import VirtualMachine, VMInterface from .choices import * @@ -535,46 +536,32 @@ class IPAddressFilterSet(BaseFilterSet, TenancyFilterSet, CustomFieldModelFilter class VLANGroupFilterSet(BaseFilterSet, NameSlugSearchFilterSet): - region_id = TreeNodeMultipleChoiceFilter( - queryset=Region.objects.all(), - field_name='site__region', - lookup_expr='in', - label='Region (ID)', + scope_type = ContentTypeFilter() + region = django_filters.NumberFilter( + method='filter_scope' ) - region = TreeNodeMultipleChoiceFilter( - queryset=Region.objects.all(), - field_name='site__region', - lookup_expr='in', - to_field_name='slug', - label='Region (slug)', + sitegroup = django_filters.NumberFilter( + method='filter_scope' ) - site_group_id = TreeNodeMultipleChoiceFilter( - queryset=SiteGroup.objects.all(), - field_name='site__group', - lookup_expr='in', - label='Site group (ID)', + site = django_filters.NumberFilter( + method='filter_scope' ) - site_group = TreeNodeMultipleChoiceFilter( - queryset=SiteGroup.objects.all(), - field_name='site__group', - lookup_expr='in', - to_field_name='slug', - label='Site group (slug)', + location = django_filters.NumberFilter( + method='filter_scope' ) - site_id = django_filters.ModelMultipleChoiceFilter( - queryset=Site.objects.all(), - label='Site (ID)', - ) - site = django_filters.ModelMultipleChoiceFilter( - field_name='site__slug', - queryset=Site.objects.all(), - to_field_name='slug', - label='Site (slug)', + rack = django_filters.NumberFilter( + method='filter_scope' ) class Meta: model = VLANGroup - fields = ['id', 'name', 'slug', 'description'] + fields = ['id', 'name', 'slug', 'description', 'scope_id'] + + def filter_scope(self, queryset, name, value): + return queryset.filter( + scope_type=ContentType.objects.get(app_label='dcim', model=name), + scope_id=value + ) class VLANFilterSet(BaseFilterSet, TenancyFilterSet, CustomFieldModelFilterSet, CreatedUpdatedFilterSet): diff --git a/netbox/ipam/forms.py b/netbox/ipam/forms.py index ab25833ad..4099a6027 100644 --- a/netbox/ipam/forms.py +++ b/netbox/ipam/forms.py @@ -1,7 +1,8 @@ from django import forms +from django.contrib.contenttypes.models import ContentType from django.utils.translation import gettext as _ -from dcim.models import Device, Interface, Rack, Region, Site, SiteGroup +from dcim.models import Device, Interface, Location, Rack, Region, Site, SiteGroup from extras.forms import ( AddRemoveTagsForm, CustomFieldBulkEditForm, CustomFieldModelCSVForm, CustomFieldModelForm, CustomFieldFilterForm, ) @@ -1126,18 +1127,70 @@ class VLANGroupForm(BootstrapMixin, CustomFieldModelForm): site = DynamicModelChoiceField( queryset=Site.objects.all(), required=False, + initial_params={ + 'locations': '$location' + }, query_params={ 'region_id': '$region', 'group_id': '$site_group', } ) + location = DynamicModelChoiceField( + queryset=Location.objects.all(), + required=False, + initial_params={ + 'racks': '$rack' + }, + query_params={ + 'site_id': '$site', + } + ) + rack = DynamicModelChoiceField( + queryset=Rack.objects.all(), + required=False, + query_params={ + 'site_id': '$site', + 'location_id': '$location', + } + ) slug = SlugField() class Meta: model = VLANGroup fields = [ - 'region', 'site', 'name', 'slug', 'description', + 'name', 'slug', 'description', 'region', 'site_group', 'site', 'location', 'rack', ] + fieldsets = ( + ('VLAN Group', ('name', 'slug', 'description')), + ('Scope', ('region', 'site_group', 'site', 'location', 'rack')), + ) + + def __init__(self, *args, **kwargs): + instance = kwargs.get('instance') + initial = kwargs.get('initial', {}) + + if instance is not None and instance.scope: + if type(instance.scope) is Rack: + initial['rack'] = instance.scope + elif type(instance.scope) is Location: + initial['location'] = instance.scope + elif type(instance.scope) is Site: + initial['site'] = instance.scope + elif type(instance.scope) is SiteGroup: + initial['site_group'] = instance.scope + elif type(instance.scope) is Region: + initial['region'] = instance.scope + + kwargs['initial'] = initial + + super().__init__(*args, **kwargs) + + def clean(self): + super().clean() + + # Assign scope object + self.instance.scope = self.cleaned_data['rack'] or self.cleaned_data['location'] or self.cleaned_data['site'] \ + or self.cleaned_data['site_group'] or self.cleaned_data['region'] or None class VLANGroupCSVForm(CustomFieldModelCSVForm): @@ -1155,25 +1208,31 @@ class VLANGroupCSVForm(CustomFieldModelCSVForm): class VLANGroupFilterForm(BootstrapMixin, forms.Form): - region_id = DynamicModelMultipleChoiceField( + region = DynamicModelMultipleChoiceField( queryset=Region.objects.all(), required=False, label=_('Region') ) - site_group_id = DynamicModelMultipleChoiceField( + sitegroup = DynamicModelMultipleChoiceField( queryset=SiteGroup.objects.all(), required=False, label=_('Site group') ) - site_id = DynamicModelMultipleChoiceField( + site = DynamicModelMultipleChoiceField( queryset=Site.objects.all(), required=False, - null_option='None', - query_params={ - 'region_id': '$region_id' - }, label=_('Site') ) + location = DynamicModelMultipleChoiceField( + queryset=Location.objects.all(), + required=False, + label=_('Location') + ) + rack = DynamicModelMultipleChoiceField( + queryset=Rack.objects.all(), + required=False, + label=_('Rack') + ) # diff --git a/netbox/ipam/migrations/0045_vlangroup_scope.py b/netbox/ipam/migrations/0045_vlangroup_scope.py new file mode 100644 index 000000000..0b658219b --- /dev/null +++ b/netbox/ipam/migrations/0045_vlangroup_scope.py @@ -0,0 +1,36 @@ +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('contenttypes', '0002_remove_content_type_name'), + ('ipam', '0044_standardize_models'), + ] + + operations = [ + migrations.RenameField( + model_name='vlangroup', + old_name='site', + new_name='scope_id', + ), + migrations.AlterField( + model_name='vlangroup', + name='scope_id', + field=models.PositiveBigIntegerField(blank=True, null=True), + ), + migrations.AddField( + model_name='vlangroup', + name='scope_type', + field=models.ForeignKey(blank=True, limit_choices_to=models.Q(('app_label', 'dcim'), ('model__in', ['region', 'sitegroup', 'site', 'location', 'rack'])), null=True, on_delete=django.db.models.deletion.CASCADE, to='contenttypes.contenttype'), + ), + migrations.AlterModelOptions( + name='vlangroup', + options={'ordering': ('name', 'pk'), 'verbose_name': 'VLAN group', 'verbose_name_plural': 'VLAN groups'}, + ), + migrations.AlterUniqueTogether( + name='vlangroup', + unique_together={('scope_type', 'scope_id', 'name'), ('scope_type', 'scope_id', 'slug')}, + ), + ] diff --git a/netbox/ipam/models/vlans.py b/netbox/ipam/models/vlans.py index 131212564..3cf177703 100644 --- a/netbox/ipam/models/vlans.py +++ b/netbox/ipam/models/vlans.py @@ -1,3 +1,5 @@ +from django.contrib.contenttypes.fields import GenericForeignKey +from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError from django.core.validators import MaxValueValidator, MinValueValidator from django.db import models @@ -31,13 +33,24 @@ class VLANGroup(OrganizationalModel): slug = models.SlugField( max_length=100 ) - site = models.ForeignKey( - to='dcim.Site', - on_delete=models.PROTECT, - related_name='vlan_groups', + scope_type = models.ForeignKey( + to=ContentType, + on_delete=models.CASCADE, + limit_choices_to=Q( + app_label='dcim', + model__in=['region', 'sitegroup', 'site', 'location', 'rack'] + ), blank=True, null=True ) + scope_id = models.PositiveBigIntegerField( + blank=True, + null=True + ) + scope = GenericForeignKey( + ct_field='scope_type', + fk_field='scope_id' + ) description = models.CharField( max_length=200, blank=True @@ -45,13 +58,13 @@ class VLANGroup(OrganizationalModel): objects = RestrictedQuerySet.as_manager() - csv_headers = ['name', 'slug', 'site', 'description'] + csv_headers = ['name', 'slug', 'scope_type', 'scope_id', 'description'] class Meta: - ordering = ('site', 'name', 'pk') # (site, name) may be non-unique + ordering = ('name', 'pk') # Name may be non-unique unique_together = [ - ['site', 'name'], - ['site', 'slug'], + ['scope_type', 'scope_id', 'name'], + ['scope_type', 'scope_id', 'slug'], ] verbose_name = 'VLAN group' verbose_name_plural = 'VLAN groups' @@ -66,7 +79,8 @@ class VLANGroup(OrganizationalModel): return ( self.name, self.slug, - self.site.name if self.site else None, + f'{self.scope_type.app_label}.{self.scope_type.model}', + self.scope_id, self.description, ) diff --git a/netbox/ipam/tables.py b/netbox/ipam/tables.py index 6553480c6..b8b166cdc 100644 --- a/netbox/ipam/tables.py +++ b/netbox/ipam/tables.py @@ -414,7 +414,7 @@ class InterfaceIPAddressTable(BaseTable): class VLANGroupTable(BaseTable): pk = ToggleColumn() name = tables.Column(linkify=True) - site = tables.Column( + scope = tables.Column( linkify=True ) vlan_count = LinkedCountColumn( @@ -429,8 +429,8 @@ class VLANGroupTable(BaseTable): class Meta(BaseTable.Meta): model = VLANGroup - fields = ('pk', 'name', 'site', 'vlan_count', 'slug', 'description', 'actions') - default_columns = ('pk', 'name', 'site', 'vlan_count', 'description', 'actions') + fields = ('pk', 'name', 'scope_type', 'scope', 'vlan_count', 'slug', 'description', 'actions') + default_columns = ('pk', 'name', 'scope_type', 'scope', 'vlan_count', 'description', 'actions') # From d3fee54644bd44b8b282101b04b2258148ff3084 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 10 Mar 2021 16:26:27 -0500 Subject: [PATCH 2/8] Fix VLAN.clean() --- netbox/ipam/models/vlans.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/netbox/ipam/models/vlans.py b/netbox/ipam/models/vlans.py index a33e4c0d3..30c76d6cf 100644 --- a/netbox/ipam/models/vlans.py +++ b/netbox/ipam/models/vlans.py @@ -173,10 +173,11 @@ class VLAN(PrimaryModel): def clean(self): super().clean() - # Validate VLAN group - if self.group and self.group.site != self.site: + # Validate VLAN group (if assigned) + if self.group and self.site and self.group.scope != self.site: raise ValidationError({ - 'group': "VLAN group must belong to the assigned site ({}).".format(self.site) + 'group': f"VLAN is assigned to group {self.group} (scope: {self.group.scope}); cannot also assign to " + f"site {self.site}." }) def to_csv(self): From 6ab1c060368403baa29a0bee73394f10eb0ccd3d Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 10 Mar 2021 16:50:04 -0500 Subject: [PATCH 3/8] Extend VLAN group assignment form --- netbox/ipam/forms.py | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/netbox/ipam/forms.py b/netbox/ipam/forms.py index 4099a6027..8714a03d9 100644 --- a/netbox/ipam/forms.py +++ b/netbox/ipam/forms.py @@ -1263,11 +1263,46 @@ class VLANForm(BootstrapMixin, TenancyForm, CustomFieldModelForm): 'group_id': '$site_group', } ) + location = DynamicModelChoiceField( + queryset=Location.objects.all(), + required=False, + null_option='None', + query_params={ + 'site_id': '$site', + } + ) + rack = DynamicModelChoiceField( + queryset=Rack.objects.all(), + required=False, + null_option='None', + query_params={ + 'site_id': '$site', + 'location_id': '$location', + } + ) + group_scope = forms.ChoiceField( + choices=( + ('', ''), + ('dcim.region', 'Region'), + ('dcim.sitegroup', 'Site group'), + ('dcim.site', 'Site'), + ('dcim.location', 'Location'), + ('dcim.rack', 'Rack'), + ), + required=False, + widget=StaticSelect2, + label='Group scope' + ) group = DynamicModelChoiceField( queryset=VLANGroup.objects.all(), required=False, query_params={ - 'site_id': '$site' + 'scope_type': '$group_scope', + 'region': '$region', + 'sitegroup': '$site_group', + 'site': '$site', + 'location': '$location', + 'rack': '$rack', } ) role = DynamicModelChoiceField( @@ -1286,7 +1321,7 @@ class VLANForm(BootstrapMixin, TenancyForm, CustomFieldModelForm): ] fieldsets = ( ('VLAN', ('vid', 'name', 'status', 'role', 'description', 'tags')), - ('Assignment', ('region', 'site_group', 'site', 'group')), + ('Assignment', ('region', 'site_group', 'site', 'location', 'rack', 'group_scope', 'group')), ('Tenancy', ('tenant_group', 'tenant')), ) help_texts = { From fadf15dbc0cfd3d8eae6e17bde5ec6c0787c045d Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 11 Mar 2021 11:07:27 -0500 Subject: [PATCH 4/8] Fix VLAN group assignment for VLANs --- netbox/ipam/forms.py | 85 +++++++++++----------------- netbox/ipam/tables.py | 2 +- netbox/ipam/views.py | 1 + netbox/templates/ipam/vlan_edit.html | 57 +++++++++++++++++++ 4 files changed, 92 insertions(+), 53 deletions(-) create mode 100644 netbox/templates/ipam/vlan_edit.html diff --git a/netbox/ipam/forms.py b/netbox/ipam/forms.py index 8714a03d9..9003f295c 100644 --- a/netbox/ipam/forms.py +++ b/netbox/ipam/forms.py @@ -1240,47 +1240,8 @@ class VLANGroupFilterForm(BootstrapMixin, forms.Form): # class VLANForm(BootstrapMixin, TenancyForm, CustomFieldModelForm): - region = DynamicModelChoiceField( - queryset=Region.objects.all(), - required=False, - initial_params={ - 'sites': '$site' - } - ) - site_group = DynamicModelChoiceField( - queryset=SiteGroup.objects.all(), - required=False, - initial_params={ - 'sites': '$site' - } - ) - site = DynamicModelChoiceField( - queryset=Site.objects.all(), - required=False, - null_option='None', - query_params={ - 'region_id': '$region', - 'group_id': '$site_group', - } - ) - location = DynamicModelChoiceField( - queryset=Location.objects.all(), - required=False, - null_option='None', - query_params={ - 'site_id': '$site', - } - ) - rack = DynamicModelChoiceField( - queryset=Rack.objects.all(), - required=False, - null_option='None', - query_params={ - 'site_id': '$site', - 'location_id': '$location', - } - ) - group_scope = forms.ChoiceField( + # VLANGroup assignment fields + scope_type = forms.ChoiceField( choices=( ('', ''), ('dcim.region', 'Region'), @@ -1297,14 +1258,39 @@ class VLANForm(BootstrapMixin, TenancyForm, CustomFieldModelForm): queryset=VLANGroup.objects.all(), required=False, query_params={ - 'scope_type': '$group_scope', - 'region': '$region', - 'sitegroup': '$site_group', - 'site': '$site', - 'location': '$location', - 'rack': '$rack', + 'scope_type': '$scope_type', + }, + label='VLAN Group' + ) + + # Site assignment fields + region = DynamicModelChoiceField( + queryset=Region.objects.all(), + required=False, + initial_params={ + 'sites': '$site' + }, + label='Region' + ) + sitegroup = DynamicModelChoiceField( + queryset=SiteGroup.objects.all(), + required=False, + initial_params={ + 'sites': '$site' + }, + label='Site group' + ) + site = DynamicModelChoiceField( + queryset=Site.objects.all(), + required=False, + null_option='None', + query_params={ + 'region_id': '$region', + 'group_id': '$sitegroup', } ) + + # Other fields role = DynamicModelChoiceField( queryset=Role.objects.all(), required=False @@ -1319,11 +1305,6 @@ class VLANForm(BootstrapMixin, TenancyForm, CustomFieldModelForm): fields = [ 'site', 'group', 'vid', 'name', 'status', 'role', 'description', 'tenant_group', 'tenant', 'tags', ] - fieldsets = ( - ('VLAN', ('vid', 'name', 'status', 'role', 'description', 'tags')), - ('Assignment', ('region', 'site_group', 'site', 'location', 'rack', 'group_scope', 'group')), - ('Tenancy', ('tenant_group', 'tenant')), - ) help_texts = { 'site': "Leave blank if this VLAN spans multiple sites", 'group': "VLAN group (optional)", diff --git a/netbox/ipam/tables.py b/netbox/ipam/tables.py index 0e712817b..7e618ad1c 100644 --- a/netbox/ipam/tables.py +++ b/netbox/ipam/tables.py @@ -90,7 +90,7 @@ VLAN_ROLE_LINK = """ VLANGROUP_ADD_VLAN = """ {% with next_vid=record.get_next_available_vid %} {% if next_vid and perms.ipam.add_vlan %} - + {% endif %} diff --git a/netbox/ipam/views.py b/netbox/ipam/views.py index 63d81d58e..f262a6a17 100644 --- a/netbox/ipam/views.py +++ b/netbox/ipam/views.py @@ -768,6 +768,7 @@ class VLANVMInterfacesView(generic.ObjectView): class VLANEditView(generic.ObjectEditView): queryset = VLAN.objects.all() model_form = forms.VLANForm + template_name = 'ipam/vlan_edit.html' class VLANDeleteView(generic.ObjectDeleteView): diff --git a/netbox/templates/ipam/vlan_edit.html b/netbox/templates/ipam/vlan_edit.html new file mode 100644 index 000000000..c86906e6c --- /dev/null +++ b/netbox/templates/ipam/vlan_edit.html @@ -0,0 +1,57 @@ +{% extends 'generic/object_edit.html' %} +{% load static %} +{% load form_helpers %} +{% load helpers %} + +{% block form %} +
+
VLAN
+
+ {% render_field form.vid %} + {% render_field form.name %} + {% render_field form.status %} + {% render_field form.role %} + {% render_field form.description %} + {% render_field form.tags %} +
+
+
+
Tenancy
+
+ {% render_field form.tenant_group %} + {% render_field form.tenant %} +
+
+
+
+ Assignment +
+
+ {% with site_tab_active=form.initial.site %} + +
+
+ {% render_field form.scope_type %} + {% render_field form.group %} +
+
+ {% render_field form.region %} + {% render_field form.sitegroup %} + {% render_field form.site %} +
+
+ {% endwith %} +
+
+ {% if form.custom_fields %} +
+
Custom Fields
+
+ {% render_custom_fields form %} +
+
+ {% endif %} +{% endblock %} From 0115a61ab7eea34439bdee1a15d10ca610f4d866 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 11 Mar 2021 11:13:41 -0500 Subject: [PATCH 5/8] Add changelog for #5284 --- docs/release-notes/version-2.11.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/release-notes/version-2.11.md b/docs/release-notes/version-2.11.md index 69023ad83..3eb918b5f 100644 --- a/docs/release-notes/version-2.11.md +++ b/docs/release-notes/version-2.11.md @@ -52,6 +52,12 @@ When exporting a list of objects in NetBox, users now have the option of selecti The legacy static export behavior has been retained to ensure backward compatibility for dependent integrations. However, users are strongly encouraged to adapt custom export templates where needed as this functionality will be removed in v2.12. +#### Variable Scope Support for VLAN Groups ([#5284](https://github.com/netbox-community/netbox/issues/5284)) + +In previous releases, VLAN groups could be assigned only to a site. To afford more flexibility in conveying the true scope of an L2 domain, a VLAN group can now be assigned to a region, site group (new in v2.11), site, location, or rack. VLANs assigned to a group will be available only to devices and virtual machines which exist within its scope. + +For example, a VLAN within a group assigned to a location will be available only to devices assigned to that location (or one of its child locations), or to a rack within that location. + #### New Site Group Model ([#5892](https://github.com/netbox-community/netbox/issues/5892)) This release introduces the new Site Group model, which can be used to organize sites similar to the existing Region model. Whereas regions are intended for geographically arranging sites into countries, states, and so on, the new site group model can be used to organize sites by role or other arbitrary classification. Using regions and site groups in conjunction provides two dimensions along which sites can be organized, offering greater flexibility to the user. @@ -115,3 +121,6 @@ The ObjectChange model (which is used to record the creation, modification, and * Renamed `object_data` to `postchange_data` * extras.Webhook * Added the `/api/extras/webhooks/` endpoint +* ipam.VLANGroup + * Added the `scope_type`, `scope_id`, and `scope` fields (`scope` is a generic foreign key) + * Dropped the `site` foreign key field From c0c4eed3a85964bce9427d556496a6391fedf4b8 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 15 Mar 2021 16:25:44 -0400 Subject: [PATCH 6/8] Extend VLANGroup to support cluster/cluster group assignment --- netbox/ipam/filters.py | 8 ++- netbox/ipam/forms.py | 34 +++++++++---- .../ipam/migrations/0045_vlangroup_scope.py | 2 +- netbox/ipam/models/vlans.py | 3 +- netbox/ipam/views.py | 3 +- netbox/templates/ipam/vlangroup_edit.html | 49 +++++++++++++++++++ 6 files changed, 85 insertions(+), 14 deletions(-) create mode 100644 netbox/templates/ipam/vlangroup_edit.html diff --git a/netbox/ipam/filters.py b/netbox/ipam/filters.py index f4a6236cb..5c5b9e8d3 100644 --- a/netbox/ipam/filters.py +++ b/netbox/ipam/filters.py @@ -552,6 +552,12 @@ class VLANGroupFilterSet(BaseFilterSet, NameSlugSearchFilterSet): rack = django_filters.NumberFilter( method='filter_scope' ) + clustergroup = django_filters.NumberFilter( + method='filter_scope' + ) + cluster = django_filters.NumberFilter( + method='filter_scope' + ) class Meta: model = VLANGroup @@ -559,7 +565,7 @@ class VLANGroupFilterSet(BaseFilterSet, NameSlugSearchFilterSet): def filter_scope(self, queryset, name, value): return queryset.filter( - scope_type=ContentType.objects.get(app_label='dcim', model=name), + scope_type=ContentType.objects.get(model=name), scope_id=value ) diff --git a/netbox/ipam/forms.py b/netbox/ipam/forms.py index 9003f295c..37a3a9d2c 100644 --- a/netbox/ipam/forms.py +++ b/netbox/ipam/forms.py @@ -1,5 +1,4 @@ from django import forms -from django.contrib.contenttypes.models import ContentType from django.utils.translation import gettext as _ from dcim.models import Device, Interface, Location, Rack, Region, Site, SiteGroup @@ -14,7 +13,7 @@ from utilities.forms import ( DynamicModelChoiceField, DynamicModelMultipleChoiceField, ExpandableIPAddressField, NumericArrayField, ReturnURLForm, SlugField, StaticSelect2, StaticSelect2Multiple, TagFilterField, BOOLEAN_WITH_BLANK_CHOICES, ) -from virtualization.models import Cluster, VirtualMachine, VMInterface +from virtualization.models import Cluster, ClusterGroup, VirtualMachine, VMInterface from .choices import * from .constants import * from .models import Aggregate, IPAddress, Prefix, RIR, Role, RouteTarget, Service, VLAN, VLANGroup, VRF @@ -1153,17 +1152,28 @@ class VLANGroupForm(BootstrapMixin, CustomFieldModelForm): 'location_id': '$location', } ) + cluster_group = DynamicModelChoiceField( + queryset=ClusterGroup.objects.all(), + required=False, + initial_params={ + 'clusters': '$cluster' + } + ) + cluster = DynamicModelChoiceField( + queryset=Cluster.objects.all(), + required=False, + query_params={ + 'group_id': '$cluster_group', + } + ) slug = SlugField() class Meta: model = VLANGroup fields = [ - 'name', 'slug', 'description', 'region', 'site_group', 'site', 'location', 'rack', + 'name', 'slug', 'description', 'region', 'site_group', 'site', 'location', 'rack', 'cluster_group', + 'cluster', ] - fieldsets = ( - ('VLAN Group', ('name', 'slug', 'description')), - ('Scope', ('region', 'site_group', 'site', 'location', 'rack')), - ) def __init__(self, *args, **kwargs): instance = kwargs.get('instance') @@ -1180,6 +1190,10 @@ class VLANGroupForm(BootstrapMixin, CustomFieldModelForm): initial['site_group'] = instance.scope elif type(instance.scope) is Region: initial['region'] = instance.scope + elif type(instance.scope) is Cluster: + initial['cluster'] = instance.scope + elif type(instance.scope) is ClusterGroup: + initial['cluster_group'] = instance.scope kwargs['initial'] = initial @@ -1189,8 +1203,10 @@ class VLANGroupForm(BootstrapMixin, CustomFieldModelForm): super().clean() # Assign scope object - self.instance.scope = self.cleaned_data['rack'] or self.cleaned_data['location'] or self.cleaned_data['site'] \ - or self.cleaned_data['site_group'] or self.cleaned_data['region'] or None + self.instance.scope = self.cleaned_data['rack'] or self.cleaned_data['location'] or \ + self.cleaned_data['site'] or self.cleaned_data['site_group'] or \ + self.cleaned_data['region'] or self.cleaned_data['cluster'] or \ + self.cleaned_data['cluster_group'] or None class VLANGroupCSVForm(CustomFieldModelCSVForm): diff --git a/netbox/ipam/migrations/0045_vlangroup_scope.py b/netbox/ipam/migrations/0045_vlangroup_scope.py index 0b658219b..8795750d2 100644 --- a/netbox/ipam/migrations/0045_vlangroup_scope.py +++ b/netbox/ipam/migrations/0045_vlangroup_scope.py @@ -23,7 +23,7 @@ class Migration(migrations.Migration): migrations.AddField( model_name='vlangroup', name='scope_type', - field=models.ForeignKey(blank=True, limit_choices_to=models.Q(('app_label', 'dcim'), ('model__in', ['region', 'sitegroup', 'site', 'location', 'rack'])), null=True, on_delete=django.db.models.deletion.CASCADE, to='contenttypes.contenttype'), + field=models.ForeignKey(blank=True, limit_choices_to=models.Q(model__in=['region', 'sitegroup', 'site', 'location', 'rack', 'clustergroup', 'cluster']), null=True, on_delete=django.db.models.deletion.CASCADE, to='contenttypes.contenttype'), ), migrations.AlterModelOptions( name='vlangroup', diff --git a/netbox/ipam/models/vlans.py b/netbox/ipam/models/vlans.py index 30c76d6cf..aa25fde46 100644 --- a/netbox/ipam/models/vlans.py +++ b/netbox/ipam/models/vlans.py @@ -35,8 +35,7 @@ class VLANGroup(OrganizationalModel): to=ContentType, on_delete=models.CASCADE, limit_choices_to=Q( - app_label='dcim', - model__in=['region', 'sitegroup', 'site', 'location', 'rack'] + model__in=['region', 'sitegroup', 'site', 'location', 'rack', 'clustergroup', 'cluster'] ), blank=True, null=True diff --git a/netbox/ipam/views.py b/netbox/ipam/views.py index f262a6a17..822aca247 100644 --- a/netbox/ipam/views.py +++ b/netbox/ipam/views.py @@ -642,6 +642,7 @@ class VLANGroupListView(generic.ObjectListView): class VLANGroupEditView(generic.ObjectEditView): queryset = VLANGroup.objects.all() model_form = forms.VLANGroupForm + template_name = 'ipam/vlangroup_edit.html' class VLANGroupDeleteView(generic.ObjectDeleteView): @@ -655,7 +656,7 @@ class VLANGroupBulkImportView(generic.BulkImportView): class VLANGroupBulkDeleteView(generic.BulkDeleteView): - queryset = VLANGroup.objects.prefetch_related('site').annotate( + queryset = VLANGroup.objects.annotate( vlan_count=count_related(VLAN, 'group') ) filterset = filters.VLANGroupFilterSet diff --git a/netbox/templates/ipam/vlangroup_edit.html b/netbox/templates/ipam/vlangroup_edit.html new file mode 100644 index 000000000..2afedd981 --- /dev/null +++ b/netbox/templates/ipam/vlangroup_edit.html @@ -0,0 +1,49 @@ +{% extends 'generic/object_edit.html' %} +{% load form_helpers %} +{% load helpers %} + +{% block form %} +
+
VLAN Group
+
+ {% render_field form.name %} + {% render_field form.slug %} + {% render_field form.description %} +
+
+
+
+ Scope +
+
+ {% with virtual_tab_active=form.initial.cluster %} + +
+
+ {% render_field form.region %} + {% render_field form.site_group %} + {% render_field form.site %} + {% render_field form.location %} + {% render_field form.rack %} +
+
+ {% render_field form.cluster_group %} + {% render_field form.cluster %} +
+
+ The VLAN group will be limited in scope to the most-specific object selected above. + {% endwith %} +
+
+ {% if form.custom_fields %} +
+
Custom Fields
+
+ {% render_custom_fields form %} +
+
+ {% endif %} +{% endblock %} From bb6360cad4f2a99474892034d614ea16aa68d2b6 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 15 Mar 2021 20:35:18 -0400 Subject: [PATCH 7/8] Fix up VLANGroup tests --- netbox/ipam/api/serializers.py | 3 +- netbox/ipam/forms.py | 11 +--- netbox/ipam/models/vlans.py | 9 +++ netbox/ipam/tests/test_filters.py | 102 +++++++++++++++++------------- netbox/ipam/tests/test_views.py | 18 ++++-- netbox/ipam/views.py | 2 +- 6 files changed, 82 insertions(+), 63 deletions(-) diff --git a/netbox/ipam/api/serializers.py b/netbox/ipam/api/serializers.py index aa0255e30..f9d8a1652 100644 --- a/netbox/ipam/api/serializers.py +++ b/netbox/ipam/api/serializers.py @@ -118,7 +118,8 @@ class VLANGroupSerializer(OrganizationalModelSerializer): queryset=ContentType.objects.filter( app_label='dcim', model__in=['region', 'sitegroup', 'site', 'location', 'rack'] - ) + ), + required=False ) scope = serializers.SerializerMethodField(read_only=True) vlan_count = serializers.IntegerField(read_only=True) diff --git a/netbox/ipam/forms.py b/netbox/ipam/forms.py index 37a3a9d2c..db7077848 100644 --- a/netbox/ipam/forms.py +++ b/netbox/ipam/forms.py @@ -1265,6 +1265,8 @@ class VLANForm(BootstrapMixin, TenancyForm, CustomFieldModelForm): ('dcim.site', 'Site'), ('dcim.location', 'Location'), ('dcim.rack', 'Rack'), + ('virtualization.clustergroup', 'Cluster group'), + ('virtualization.cluster', 'Cluster'), ), required=False, widget=StaticSelect2, @@ -1372,15 +1374,6 @@ class VLANCSVForm(CustomFieldModelCSVForm): 'name': 'VLAN name', } - def __init__(self, data=None, *args, **kwargs): - super().__init__(data, *args, **kwargs) - - if data: - - # Limit vlan queryset by assigned group - params = {f"site__{self.fields['site'].to_field_name}": data.get('site')} - self.fields['group'].queryset = self.fields['group'].queryset.filter(**params) - class VLANBulkEditForm(BootstrapMixin, AddRemoveTagsForm, CustomFieldBulkEditForm): pk = forms.ModelMultipleChoiceField( diff --git a/netbox/ipam/models/vlans.py b/netbox/ipam/models/vlans.py index aa25fde46..26cb5299f 100644 --- a/netbox/ipam/models/vlans.py +++ b/netbox/ipam/models/vlans.py @@ -72,6 +72,15 @@ class VLANGroup(OrganizationalModel): def get_absolute_url(self): return reverse('ipam:vlangroup_vlans', args=[self.pk]) + def clean(self): + super().clean() + + # Validate scope assignment + if self.scope_type and not self.scope_id: + raise ValidationError("Cannot set scope_type without scope_id.") + if self.scope_id and not self.scope_type: + raise ValidationError("Cannot set scope_id without scope_type.") + def to_csv(self): return ( self.name, diff --git a/netbox/ipam/tests/test_filters.py b/netbox/ipam/tests/test_filters.py index b83ea6efe..90af26e9b 100644 --- a/netbox/ipam/tests/test_filters.py +++ b/netbox/ipam/tests/test_filters.py @@ -1,10 +1,10 @@ from django.test import TestCase -from dcim.models import Device, DeviceRole, DeviceType, Interface, Manufacturer, Region, Site, SiteGroup +from dcim.models import Device, DeviceRole, DeviceType, Interface, Location, Manufacturer, Rack, Region, Site, SiteGroup from ipam.choices import * from ipam.filters import * from ipam.models import Aggregate, IPAddress, Prefix, RIR, Role, RouteTarget, Service, VLAN, VLANGroup, VRF -from virtualization.models import Cluster, ClusterType, VirtualMachine, VMInterface +from virtualization.models import Cluster, ClusterGroup, ClusterType, VirtualMachine, VMInterface from tenancy.models import Tenant, TenantGroup @@ -715,34 +715,39 @@ class VLANGroupTestCase(TestCase): @classmethod def setUpTestData(cls): - regions = ( - Region(name='Test Region 1', slug='test-region-1'), - Region(name='Test Region 2', slug='test-region-2'), - Region(name='Test Region 3', slug='test-region-3'), - ) - for r in regions: - r.save() + region = Region(name='Region 1', slug='region-1') + region.save() - site_groups = ( - SiteGroup(name='Site Group 1', slug='site-group-1'), - SiteGroup(name='Site Group 2', slug='site-group-2'), - SiteGroup(name='Site Group 3', slug='site-group-3'), - ) - for site_group in site_groups: - site_group.save() + sitegroup = SiteGroup(name='Site Group 1', slug='site-group-1') + sitegroup.save() - sites = ( - Site(name='Test Site 1', slug='test-site-1', region=regions[0], group=site_groups[0]), - Site(name='Test Site 2', slug='test-site-2', region=regions[1], group=site_groups[1]), - Site(name='Test Site 3', slug='test-site-3', region=regions[2], group=site_groups[2]), - ) - Site.objects.bulk_create(sites) + site = Site(name='Site 1', slug='site-1') + site.save() + + location = Location(name='Location 1', slug='location-1', site=site) + location.save() + + rack = Rack(name='Rack 1', site=site) + rack.save() + + clustertype = ClusterType(name='Cluster Type 1', slug='cluster-type-1') + clustertype.save() + + clustergroup = ClusterGroup(name='Cluster Group 1', slug='cluster-group-1') + clustergroup.save() + + cluster = Cluster(name='Cluster 1', type=clustertype) + cluster.save() vlan_groups = ( - VLANGroup(name='VLAN Group 1', slug='vlan-group-1', site=sites[0], description='A'), - VLANGroup(name='VLAN Group 2', slug='vlan-group-2', site=sites[1], description='B'), - VLANGroup(name='VLAN Group 3', slug='vlan-group-3', site=sites[2], description='C'), - VLANGroup(name='VLAN Group 4', slug='vlan-group-4', site=None), + VLANGroup(name='VLAN Group 1', slug='vlan-group-1', scope=region, description='A'), + VLANGroup(name='VLAN Group 2', slug='vlan-group-2', scope=sitegroup, description='B'), + VLANGroup(name='VLAN Group 3', slug='vlan-group-3', scope=site, description='C'), + VLANGroup(name='VLAN Group 4', slug='vlan-group-4', scope=location, description='D'), + VLANGroup(name='VLAN Group 5', slug='vlan-group-5', scope=rack, description='E'), + VLANGroup(name='VLAN Group 6', slug='vlan-group-6', scope=clustergroup, description='F'), + VLANGroup(name='VLAN Group 7', slug='vlan-group-7', scope=cluster, description='G'), + VLANGroup(name='VLAN Group 8', slug='vlan-group-8'), ) VLANGroup.objects.bulk_create(vlan_groups) @@ -763,25 +768,32 @@ class VLANGroupTestCase(TestCase): self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) def test_region(self): - regions = Region.objects.all()[:2] - params = {'region_id': [regions[0].pk, regions[1].pk]} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) - params = {'region': [regions[0].slug, regions[1].slug]} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + params = {'region': Region.objects.first().pk} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) - def test_site_group(self): - site_groups = SiteGroup.objects.all()[:2] - params = {'site_group_id': [site_groups[0].pk, site_groups[1].pk]} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) - params = {'site_group': [site_groups[0].slug, site_groups[1].slug]} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + def test_sitegroup(self): + params = {'sitegroup': SiteGroup.objects.first().pk} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) def test_site(self): - sites = Site.objects.all()[:2] - params = {'site_id': [sites[0].pk, sites[1].pk]} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) - params = {'site': [sites[0].slug, sites[1].slug]} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + params = {'site': Site.objects.first().pk} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) + + def test_location(self): + params = {'location': Location.objects.first().pk} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) + + def test_rack(self): + params = {'rack': Rack.objects.first().pk} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) + + def test_clustergroup(self): + params = {'clustergroup': ClusterGroup.objects.first().pk} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) + + def test_cluster(self): + params = {'cluster': Cluster.objects.first().pk} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) class VLANTestCase(TestCase): @@ -822,9 +834,9 @@ class VLANTestCase(TestCase): Role.objects.bulk_create(roles) groups = ( - VLANGroup(name='VLAN Group 1', slug='vlan-group-1', site=sites[0]), - VLANGroup(name='VLAN Group 2', slug='vlan-group-2', site=sites[1]), - VLANGroup(name='VLAN Group 3', slug='vlan-group-3', site=None), + VLANGroup(name='VLAN Group 1', slug='vlan-group-1', scope=sites[0]), + VLANGroup(name='VLAN Group 2', slug='vlan-group-2', scope=sites[1]), + VLANGroup(name='VLAN Group 3', slug='vlan-group-3', scope=None), ) VLANGroup.objects.bulk_create(groups) diff --git a/netbox/ipam/tests/test_views.py b/netbox/ipam/tests/test_views.py index b105ea7d9..7d099e762 100644 --- a/netbox/ipam/tests/test_views.py +++ b/netbox/ipam/tests/test_views.py @@ -306,18 +306,22 @@ class VLANGroupTestCase(ViewTestCases.OrganizationalObjectViewTestCase): @classmethod def setUpTestData(cls): - site = Site.objects.create(name='Site 1', slug='site-1') + sites = ( + Site(name='Site 1', slug='site-1'), + Site(name='Site 2', slug='site-2'), + ) + Site.objects.bulk_create(sites) VLANGroup.objects.bulk_create([ - VLANGroup(name='VLAN Group 1', slug='vlan-group-1', site=site), - VLANGroup(name='VLAN Group 2', slug='vlan-group-2', site=site), - VLANGroup(name='VLAN Group 3', slug='vlan-group-3', site=site), + VLANGroup(name='VLAN Group 1', slug='vlan-group-1', scope=sites[0]), + VLANGroup(name='VLAN Group 2', slug='vlan-group-2', scope=sites[0]), + VLANGroup(name='VLAN Group 3', slug='vlan-group-3', scope=sites[0]), ]) cls.form_data = { 'name': 'VLAN Group X', 'slug': 'vlan-group-x', - 'site': site.pk, + 'site': sites[1].pk, 'description': 'A new VLAN group', } @@ -342,8 +346,8 @@ class VLANTestCase(ViewTestCases.PrimaryObjectViewTestCase): Site.objects.bulk_create(sites) vlangroups = ( - VLANGroup(name='VLAN Group 1', slug='vlan-group-1', site=sites[0]), - VLANGroup(name='VLAN Group 2', slug='vlan-group-2', site=sites[1]), + VLANGroup(name='VLAN Group 1', slug='vlan-group-1', scope=sites[0]), + VLANGroup(name='VLAN Group 2', slug='vlan-group-2', scope=sites[1]), ) VLANGroup.objects.bulk_create(vlangroups) diff --git a/netbox/ipam/views.py b/netbox/ipam/views.py index 822aca247..3f283a932 100644 --- a/netbox/ipam/views.py +++ b/netbox/ipam/views.py @@ -631,7 +631,7 @@ class IPAddressBulkDeleteView(generic.BulkDeleteView): # class VLANGroupListView(generic.ObjectListView): - queryset = VLANGroup.objects.prefetch_related('site').annotate( + queryset = VLANGroup.objects.annotate( vlan_count=count_related(VLAN, 'group') ) filterset = filters.VLANGroupFilterSet From f64f205e810652c54508b497145c235c282183b7 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 15 Mar 2021 20:58:23 -0400 Subject: [PATCH 8/8] Omit prefetch_related() for VLANGroup --- netbox/ipam/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox/ipam/views.py b/netbox/ipam/views.py index 5de6f847a..d5332129c 100644 --- a/netbox/ipam/views.py +++ b/netbox/ipam/views.py @@ -672,7 +672,7 @@ class VLANGroupBulkImportView(generic.BulkImportView): class VLANGroupBulkEditView(generic.BulkEditView): - queryset = VLANGroup.objects.prefetch_related('site').annotate( + queryset = VLANGroup.objects.annotate( vlan_count=count_related(VLAN, 'group') ) filterset = filters.VLANGroupFilterSet