From aba9748ffd1a6bcb98efda777781ae5f14bfe821 Mon Sep 17 00:00:00 2001 From: Shawn Peng Date: Tue, 21 Feb 2017 10:27:24 -0800 Subject: [PATCH 1/2] Fix #235: Enable global vlan (#904) * Fix #235: Enable global vlan Decouple site/vlan, make site optional for vlan/vlangroup Change html generation code to check site existence before dereference Create site search function, if site is None for a VLAN, view it as global VLAN * commit1 * commit2 * commit3 * Add migration file for VLAN&VLAN group * Revert unintentional commits --- netbox/ipam/filters.py | 24 ++++++++++++++--- netbox/ipam/forms.py | 17 +++++++----- .../migrations/0015_auto_20170219_0726.py | 26 +++++++++++++++++++ netbox/ipam/models.py | 9 ++++--- netbox/templates/ipam/vlan.html | 14 ++++++++-- 5 files changed, 73 insertions(+), 17 deletions(-) create mode 100644 netbox/ipam/migrations/0015_auto_20170219_0726.py diff --git a/netbox/ipam/filters.py b/netbox/ipam/filters.py index a0dc9f633..558f5eade 100644 --- a/netbox/ipam/filters.py +++ b/netbox/ipam/filters.py @@ -262,37 +262,47 @@ class IPAddressFilter(CustomFieldFilterSet, django_filters.FilterSet): class VLANGroupFilter(django_filters.FilterSet): - site_id = django_filters.ModelMultipleChoiceFilter( + site_id = NullableModelMultipleChoiceFilter( name='site', queryset=Site.objects.all(), label='Site (ID)', + method='site_search', ) - site = django_filters.ModelMultipleChoiceFilter( + site = NullableModelMultipleChoiceFilter( name='site__slug', queryset=Site.objects.all(), to_field_name='slug', label='Site (slug)', + method='site_search', ) class Meta: model = VLANGroup + def site_search(self, queryset, name, value): + q = Q(**{name: None}) + for v in value: + q |= Q(**{name: v}) + return queryset.filter(q) + class VLANFilter(CustomFieldFilterSet, django_filters.FilterSet): q = django_filters.MethodFilter( action='search', label='Search', ) - site_id = django_filters.ModelMultipleChoiceFilter( + site_id = NullableModelMultipleChoiceFilter( name='site', queryset=Site.objects.all(), label='Site (ID)', + method='site_search', ) - site = django_filters.ModelMultipleChoiceFilter( + site = NullableModelMultipleChoiceFilter( name='site__slug', queryset=Site.objects.all(), to_field_name='slug', label='Site (slug)', + method='site_search', ) group_id = NullableModelMultipleChoiceFilter( name='group', @@ -349,6 +359,12 @@ class VLANFilter(CustomFieldFilterSet, django_filters.FilterSet): pass return queryset.filter(qs_filter) + def site_search(self, queryset, name, value): + q = Q(**{name: None}) + for v in value: + q |= Q(**{name: v}) + return queryset.filter(q) + class ServiceFilter(django_filters.FilterSet): device_id = django_filters.ModelMultipleChoiceFilter( diff --git a/netbox/ipam/forms.py b/netbox/ipam/forms.py index 83be76169..d6db8c9bc 100644 --- a/netbox/ipam/forms.py +++ b/netbox/ipam/forms.py @@ -153,7 +153,8 @@ class RoleForm(BootstrapMixin, forms.ModelForm): class PrefixForm(BootstrapMixin, CustomFieldForm): site = forms.ModelChoiceField(queryset=Site.objects.all(), required=False, label='Site', - widget=forms.Select(attrs={'filter-for': 'vlan'})) + widget=forms.Select(attrs={'filter-for': 'vlan', + 'default_value': '0'})) vlan = forms.ModelChoiceField(queryset=VLAN.objects.all(), required=False, label='VLAN', widget=APISelect(api_url='/api/ipam/vlans/?site_id={{site}}', display_field='display_name')) @@ -173,7 +174,7 @@ class PrefixForm(BootstrapMixin, CustomFieldForm): elif self.initial.get('site'): self.fields['vlan'].queryset = VLAN.objects.filter(site=self.initial['site']) else: - self.fields['vlan'].choices = [] + self.fields['vlan'].queryset = VLAN.objects.filter(site=None) class PrefixFromCSVForm(forms.ModelForm): @@ -508,7 +509,8 @@ class VLANGroupForm(BootstrapMixin, forms.ModelForm): class VLANGroupFilterForm(BootstrapMixin, forms.Form): - site = FilterChoiceField(queryset=Site.objects.annotate(filter_count=Count('vlan_groups')), to_field_name='slug') + site = FilterChoiceField(queryset=Site.objects.annotate(filter_count=Count('vlan_groups')), to_field_name='slug', + null_option=(0, 'Global')) # @@ -532,7 +534,7 @@ class VLANForm(BootstrapMixin, CustomFieldForm): 'role': "The primary function of this VLAN", } widgets = { - 'site': forms.Select(attrs={'filter-for': 'group'}), + 'site': forms.Select(attrs={'filter-for': 'group', 'default_value': '0'}), } def __init__(self, *args, **kwargs): @@ -545,11 +547,11 @@ class VLANForm(BootstrapMixin, CustomFieldForm): elif self.initial.get('site'): self.fields['group'].queryset = VLANGroup.objects.filter(site=self.initial['site']) else: - self.fields['group'].choices = [] + self.fields['group'].queryset = VLANGroup.objects.filter(site=None) class VLANFromCSVForm(forms.ModelForm): - site = forms.ModelChoiceField(queryset=Site.objects.all(), to_field_name='name', + site = forms.ModelChoiceField(queryset=Site.objects.all(), required=False, to_field_name='name', error_messages={'invalid_choice': 'Site not found.'}) group = forms.ModelChoiceField(queryset=VLANGroup.objects.all(), required=False, to_field_name='name', error_messages={'invalid_choice': 'VLAN group not found.'}) @@ -599,7 +601,8 @@ def vlan_status_choices(): class VLANFilterForm(BootstrapMixin, CustomFieldFilterForm): model = VLAN q = forms.CharField(required=False, label='Search') - site = FilterChoiceField(queryset=Site.objects.annotate(filter_count=Count('vlans')), to_field_name='slug') + site = FilterChoiceField(queryset=Site.objects.annotate(filter_count=Count('vlans')), to_field_name='slug', + null_option=(0, 'Global')) group_id = FilterChoiceField(queryset=VLANGroup.objects.annotate(filter_count=Count('vlans')), label='VLAN group', null_option=(0, 'None')) tenant = FilterChoiceField(queryset=Tenant.objects.annotate(filter_count=Count('vlans')), to_field_name='slug', diff --git a/netbox/ipam/migrations/0015_auto_20170219_0726.py b/netbox/ipam/migrations/0015_auto_20170219_0726.py new file mode 100644 index 000000000..38003b2b8 --- /dev/null +++ b/netbox/ipam/migrations/0015_auto_20170219_0726.py @@ -0,0 +1,26 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.5 on 2017-02-19 07:26 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('ipam', '0014_ipaddress_status_add_deprecated'), + ] + + operations = [ + migrations.AlterField( + model_name='vlan', + name='site', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='vlans', to='dcim.Site'), + ), + migrations.AlterField( + model_name='vlangroup', + name='site', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='vlan_groups', to='dcim.Site'), + ), + ] diff --git a/netbox/ipam/models.py b/netbox/ipam/models.py index d37fdec25..fa9e7834b 100644 --- a/netbox/ipam/models.py +++ b/netbox/ipam/models.py @@ -485,7 +485,7 @@ class VLANGroup(models.Model): """ name = models.CharField(max_length=50) slug = models.SlugField() - site = models.ForeignKey('dcim.Site', related_name='vlan_groups') + site = models.ForeignKey('dcim.Site', related_name='vlan_groups', on_delete=models.SET_NULL, blank=True, null=True) class Meta: ordering = ['site', 'name'] @@ -497,7 +497,8 @@ class VLANGroup(models.Model): verbose_name_plural = 'VLAN groups' def __str__(self): - return u'{} - {}'.format(self.site.name, self.name) + site_name = self.site.name if self.site else '__global' + return u'{} - {}'.format(site_name, self.name) def get_absolute_url(self): return "{}?group_id={}".format(reverse('ipam:vlan_list'), self.pk) @@ -513,7 +514,7 @@ class VLAN(CreatedUpdatedModel, CustomFieldModel): Like Prefixes, each VLAN is assigned an operational status and optionally a user-defined Role. A VLAN can have zero or more Prefixes assigned to it. """ - site = models.ForeignKey('dcim.Site', related_name='vlans', on_delete=models.PROTECT) + site = models.ForeignKey('dcim.Site', related_name='vlans', on_delete=models.PROTECT, blank=True, null=True) group = models.ForeignKey('VLANGroup', related_name='vlans', blank=True, null=True, on_delete=models.PROTECT) vid = models.PositiveSmallIntegerField(verbose_name='ID', validators=[ MinValueValidator(1), @@ -551,7 +552,7 @@ class VLAN(CreatedUpdatedModel, CustomFieldModel): def to_csv(self): return csv_format([ - self.site.name, + self.site.name if self.site else None, self.group.name if self.group else None, self.vid, self.name, diff --git a/netbox/templates/ipam/vlan.html b/netbox/templates/ipam/vlan.html index 3d81392e0..5d9f219fc 100644 --- a/netbox/templates/ipam/vlan.html +++ b/netbox/templates/ipam/vlan.html @@ -8,7 +8,11 @@
  1. VLANs
    2. -
  2. {{ vlan.site }}
    3. + {% if vlan.site %} +
  3. {{ vlan.site }}
    4. + {% else %} +
  4. Global
    5. + {% endif %} {% if vlan.group %}
  5. {{ vlan.group.name }}
    6. {% endif %} @@ -53,7 +57,13 @@ - + From b0f9035e2ddcac88ba935d20e843beddbd61b428 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 21 Feb 2017 14:53:22 -0500 Subject: [PATCH 2/2] Additional work on #904 --- docs/data-model/ipam.md | 6 ++++-- netbox/ipam/filters.py | 20 ++----------------- netbox/ipam/forms.py | 14 +++++++------ ..._20170219_0726.py => 0015_global_vlans.py} | 6 +++--- netbox/ipam/models.py | 7 ++++--- netbox/templates/ipam/vlan.html | 4 +--- 6 files changed, 22 insertions(+), 35 deletions(-) rename netbox/ipam/migrations/{0015_auto_20170219_0726.py => 0015_global_vlans.py} (74%) diff --git a/docs/data-model/ipam.md b/docs/data-model/ipam.md index ee54e74d2..6e72d5b87 100644 --- a/docs/data-model/ipam.md +++ b/docs/data-model/ipam.md @@ -83,9 +83,11 @@ One IP address can be designated as the network address translation (NAT) IP add # VLANs -A VLAN represents an isolated layer two domain, identified by a name and a numeric ID (1-4094). Note that while it is good practice, neither VLAN names nor IDs must be unique within a site. This is to accommodate the fact that many real-world network use less-than-optimal VLAN allocations and may have overlapping VLAN ID assignments in practice. +A VLAN represents an isolated layer two domain, identified by a name and a numeric ID (1-4094) as defined in [IEEE 802.1Q](https://en.wikipedia.org/wiki/IEEE_802.1Q). VLANs may be assigned to a site and/or VLAN group. Like prefixes, each VLAN is assigned an operational status and (optionally) a functional role. -Like prefixes, each VLAN is assigned an operational status and (optionally) a functional role. +### VLAN Groups + +VLAN groups can be employed for administrative organization within NetBox. Each VLAN within a group must have a unique ID and name. VLANs which are not assigned to a group may have overlapping names and IDs, including within a site. --- diff --git a/netbox/ipam/filters.py b/netbox/ipam/filters.py index 558f5eade..b83228f9c 100644 --- a/netbox/ipam/filters.py +++ b/netbox/ipam/filters.py @@ -266,25 +266,17 @@ class VLANGroupFilter(django_filters.FilterSet): name='site', queryset=Site.objects.all(), label='Site (ID)', - method='site_search', ) site = NullableModelMultipleChoiceFilter( - name='site__slug', + name='site', queryset=Site.objects.all(), to_field_name='slug', label='Site (slug)', - method='site_search', ) class Meta: model = VLANGroup - def site_search(self, queryset, name, value): - q = Q(**{name: None}) - for v in value: - q |= Q(**{name: v}) - return queryset.filter(q) - class VLANFilter(CustomFieldFilterSet, django_filters.FilterSet): q = django_filters.MethodFilter( @@ -295,14 +287,12 @@ class VLANFilter(CustomFieldFilterSet, django_filters.FilterSet): name='site', queryset=Site.objects.all(), label='Site (ID)', - method='site_search', ) site = NullableModelMultipleChoiceFilter( - name='site__slug', + name='site', queryset=Site.objects.all(), to_field_name='slug', label='Site (slug)', - method='site_search', ) group_id = NullableModelMultipleChoiceFilter( name='group', @@ -359,12 +349,6 @@ class VLANFilter(CustomFieldFilterSet, django_filters.FilterSet): pass return queryset.filter(qs_filter) - def site_search(self, queryset, name, value): - q = Q(**{name: None}) - for v in value: - q |= Q(**{name: v}) - return queryset.filter(q) - class ServiceFilter(django_filters.FilterSet): device_id = django_filters.ModelMultipleChoiceFilter( diff --git a/netbox/ipam/forms.py b/netbox/ipam/forms.py index d6db8c9bc..123ca0131 100644 --- a/netbox/ipam/forms.py +++ b/netbox/ipam/forms.py @@ -153,8 +153,7 @@ class RoleForm(BootstrapMixin, forms.ModelForm): class PrefixForm(BootstrapMixin, CustomFieldForm): site = forms.ModelChoiceField(queryset=Site.objects.all(), required=False, label='Site', - widget=forms.Select(attrs={'filter-for': 'vlan', - 'default_value': '0'})) + widget=forms.Select(attrs={'filter-for': 'vlan', 'nullable': 'true'})) vlan = forms.ModelChoiceField(queryset=VLAN.objects.all(), required=False, label='VLAN', widget=APISelect(api_url='/api/ipam/vlans/?site_id={{site}}', display_field='display_name')) @@ -509,8 +508,11 @@ class VLANGroupForm(BootstrapMixin, forms.ModelForm): class VLANGroupFilterForm(BootstrapMixin, forms.Form): - site = FilterChoiceField(queryset=Site.objects.annotate(filter_count=Count('vlan_groups')), to_field_name='slug', - null_option=(0, 'Global')) + site = FilterChoiceField( + queryset=Site.objects.annotate(filter_count=Count('vlan_groups')), + to_field_name='slug', + null_option=(0, 'Global') + ) # @@ -526,7 +528,7 @@ class VLANForm(BootstrapMixin, CustomFieldForm): model = VLAN fields = ['site', 'group', 'vid', 'name', 'tenant', 'status', 'role', 'description'] help_texts = { - 'site': "The site at which this VLAN exists", + 'site': "Leave blank if this VLAN spans multiple sites", 'group': "VLAN group (optional)", 'vid': "Configured VLAN ID", 'name': "Configured VLAN name", @@ -534,7 +536,7 @@ class VLANForm(BootstrapMixin, CustomFieldForm): 'role': "The primary function of this VLAN", } widgets = { - 'site': forms.Select(attrs={'filter-for': 'group', 'default_value': '0'}), + 'site': forms.Select(attrs={'filter-for': 'group', 'nullable': 'true'}), } def __init__(self, *args, **kwargs): diff --git a/netbox/ipam/migrations/0015_auto_20170219_0726.py b/netbox/ipam/migrations/0015_global_vlans.py similarity index 74% rename from netbox/ipam/migrations/0015_auto_20170219_0726.py rename to netbox/ipam/migrations/0015_global_vlans.py index 38003b2b8..18d82cbaf 100644 --- a/netbox/ipam/migrations/0015_auto_20170219_0726.py +++ b/netbox/ipam/migrations/0015_global_vlans.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -# Generated by Django 1.10.5 on 2017-02-19 07:26 +# Generated by Django 1.10.4 on 2017-02-21 18:45 from __future__ import unicode_literals from django.db import migrations, models @@ -16,11 +16,11 @@ class Migration(migrations.Migration): migrations.AlterField( model_name='vlan', name='site', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='vlans', to='dcim.Site'), + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, related_name='vlans', to='dcim.Site'), ), migrations.AlterField( model_name='vlangroup', name='site', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='vlan_groups', to='dcim.Site'), + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, related_name='vlan_groups', to='dcim.Site'), ), ] diff --git a/netbox/ipam/models.py b/netbox/ipam/models.py index fa9e7834b..a04e4b9ce 100644 --- a/netbox/ipam/models.py +++ b/netbox/ipam/models.py @@ -485,7 +485,7 @@ class VLANGroup(models.Model): """ name = models.CharField(max_length=50) slug = models.SlugField() - site = models.ForeignKey('dcim.Site', related_name='vlan_groups', on_delete=models.SET_NULL, blank=True, null=True) + site = models.ForeignKey('dcim.Site', related_name='vlan_groups', on_delete=models.PROTECT, blank=True, null=True) class Meta: ordering = ['site', 'name'] @@ -497,8 +497,9 @@ class VLANGroup(models.Model): verbose_name_plural = 'VLAN groups' def __str__(self): - site_name = self.site.name if self.site else '__global' - return u'{} - {}'.format(site_name, self.name) + if self.site is None: + return self.name + return u'{} - {}'.format(self.site.name, self.name) def get_absolute_url(self): return "{}?group_id={}".format(reverse('ipam:vlan_list'), self.pk) diff --git a/netbox/templates/ipam/vlan.html b/netbox/templates/ipam/vlan.html index 5d9f219fc..1b6ddb5dd 100644 --- a/netbox/templates/ipam/vlan.html +++ b/netbox/templates/ipam/vlan.html @@ -10,11 +10,9 @@
  6. VLANs
    7. {% if vlan.site %}
  7. {{ vlan.site }}
    8. - {% else %} -
  8. Global
    9. {% endif %} {% if vlan.group %} -
  9. {{ vlan.group.name }}
    10. +
  10. {{ vlan.group.name }}
    11. {% endif %}
  11. {{ vlan.name }} ({{ vlan.vid }})
    12. Site{{ vlan.site }} + {% if vlan.site %} + {{ vlan.site }} + {% else %} + None + {% endif %} +
    Group