Skip to content

Commit

Permalink
Closes #12622: Fix assigning VLAN without site to Prefix (#12784)
Browse files Browse the repository at this point in the history
* Issue #12622: Fix creating Prefix using VLAN without site

* Issue #12622: Fix importing Prefix using VLAN without site

This commit also adds tests to verify the import changes implemented
in this commit.

* Issue #12622: Cleanup code to filter allowed VLANs on a prefix import

* Closes #12622: Switch to VLAN selector dialog when creating Prefix
  • Loading branch information
dhenschen authored Jun 14, 2023
1 parent f7b0e48 commit 0e873a0
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 12 deletions.
34 changes: 25 additions & 9 deletions netbox/ipam/forms/bulk_import.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django import forms
from django.contrib.contenttypes.models import ContentType
from django.core.exceptions import ValidationError
from django.db.models import Q
from django.utils.translation import gettext as _

from dcim.models import Device, Interface, Site
Expand Down Expand Up @@ -181,16 +182,31 @@ class Meta:
def __init__(self, data=None, *args, **kwargs):
super().__init__(data, *args, **kwargs)

if data:
if not data:
return

site = data.get('site')
vlan_group = data.get('vlan_group')

# Limit VLAN queryset by assigned site and/or group (if specified)
query = Q()

if site:
query |= Q(**{
f"site__{self.fields['site'].to_field_name}": site
})
# Don't Forget to include VLANs without a site in the filter
query |= Q(**{
f"site__{self.fields['site'].to_field_name}__isnull": True
})

if vlan_group:
query &= Q(**{
f"group__{self.fields['vlan_group'].to_field_name}": vlan_group
})

# Limit VLAN queryset by assigned site and/or group (if specified)
params = {}
if data.get('site'):
params[f"site__{self.fields['site'].to_field_name}"] = data.get('site')
if data.get('vlan_group'):
params[f"group__{self.fields['vlan_group'].to_field_name}"] = data.get('vlan_group')
if params:
self.fields['vlan'].queryset = self.fields['vlan'].queryset.filter(**params)
queryset = self.fields['vlan'].queryset.filter(query)
self.fields['vlan'].queryset = queryset


class IPRangeImportForm(NetBoxModelImportForm):
Expand Down
4 changes: 1 addition & 3 deletions netbox/ipam/forms/model_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,8 @@ class PrefixForm(TenancyForm, NetBoxModelForm):
vlan = DynamicModelChoiceField(
queryset=VLAN.objects.all(),
required=False,
selector=True,
label=_('VLAN'),
query_params={
'site_id': '$site',
}
)
role = DynamicModelChoiceField(
queryset=Role.objects.all(),
Expand Down
59 changes: 59 additions & 0 deletions netbox/ipam/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,65 @@ def test_prefix_ipaddresses(self):
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
self.assertHttpStatus(self.client.get(url), 200)

@override_settings(EXEMPT_VIEW_PERMISSIONS=['*'])
def test_prefix_import(self):
"""
Custom import test for YAML-based imports (versus CSV)
"""
IMPORT_DATA = """
prefix: 10.1.1.0/24
status: active
vlan: 101
site: Site 1
"""
# Note, a site is not tied to the VLAN to verify the fix for #12622
VLAN.objects.create(vid=101, name='VLAN101')

# Add all required permissions to the test user
self.add_permissions('ipam.view_prefix', 'ipam.add_prefix')

form_data = {
'data': IMPORT_DATA,
'format': 'yaml'
}
response = self.client.post(reverse('ipam:prefix_import'), data=form_data, follow=True)
self.assertHttpStatus(response, 200)

prefix = Prefix.objects.get(prefix='10.1.1.0/24')
self.assertEqual(prefix.status, PrefixStatusChoices.STATUS_ACTIVE)
self.assertEqual(prefix.vlan.vid, 101)
self.assertEqual(prefix.site.name, "Site 1")

@override_settings(EXEMPT_VIEW_PERMISSIONS=['*'])
def test_prefix_import_with_vlan_group(self):
"""
This test covers a unique import edge case where VLAN group is specified during the import.
"""
IMPORT_DATA = """
prefix: 10.1.2.0/24
status: active
vlan: 102
site: Site 1
vlan_group: Group 1
"""
vlan_group = VLANGroup.objects.create(name='Group 1', slug='group-1', scope=Site.objects.get(name="Site 1"))
VLAN.objects.create(vid=102, name='VLAN102', group=vlan_group)

# Add all required permissions to the test user
self.add_permissions('ipam.view_prefix', 'ipam.add_prefix')

form_data = {
'data': IMPORT_DATA,
'format': 'yaml'
}
response = self.client.post(reverse('ipam:prefix_import'), data=form_data, follow=True)
self.assertHttpStatus(response, 200)

prefix = Prefix.objects.get(prefix='10.1.2.0/24')
self.assertEqual(prefix.status, PrefixStatusChoices.STATUS_ACTIVE)
self.assertEqual(prefix.vlan.vid, 102)
self.assertEqual(prefix.site.name, "Site 1")


class IPRangeTestCase(ViewTestCases.PrimaryObjectViewTestCase):
model = IPRange
Expand Down

0 comments on commit 0e873a0

Please sign in to comment.