Merge pull request #10488 from netbox-community/9249-device-vm-names

Closes #9249: Ignore case for device/VM names
This commit is contained in:
Jeremy Stretch 2022-09-28 17:05:57 -04:00 committed by GitHub
commit 5382ac20b6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 76 additions and 16 deletions

View File

@ -3,8 +3,13 @@
!!! warning "PostgreSQL 11 Required" !!! warning "PostgreSQL 11 Required"
NetBox v3.4 requires PostgreSQL 11 or later. NetBox v3.4 requires PostgreSQL 11 or later.
### Breaking Changes
* Device and virtual machine names are no longer case-sensitive. Attempting to create e.g. "device1" and "DEVICE1" will raise a validation error.
### Enhancements ### Enhancements
* [#9249](https://github.com/netbox-community/netbox/issues/9249) - Device and virtual machine names are no longer case-sensitive
* [#9892](https://github.com/netbox-community/netbox/issues/9892) - Add optional `name` field for FHRP groups * [#9892](https://github.com/netbox-community/netbox/issues/9892) - Add optional `name` field for FHRP groups
### Plugins API ### Plugins API

View File

@ -887,6 +887,9 @@ class DeviceFilterSet(NetBoxModelFilterSet, TenancyFilterSet, ContactModelFilter
to_field_name='slug', to_field_name='slug',
label='Device model (slug)', label='Device model (slug)',
) )
name = MultiValueCharFilter(
lookup_expr='iexact'
)
status = django_filters.MultipleChoiceFilter( status = django_filters.MultipleChoiceFilter(
choices=DeviceStatusChoices, choices=DeviceStatusChoices,
null_value=None null_value=None
@ -950,7 +953,7 @@ class DeviceFilterSet(NetBoxModelFilterSet, TenancyFilterSet, ContactModelFilter
class Meta: class Meta:
model = Device model = Device
fields = ['id', 'name', 'asset_tag', 'face', 'position', 'airflow', 'vc_position', 'vc_priority'] fields = ['id', 'asset_tag', 'face', 'position', 'airflow', 'vc_position', 'vc_priority']
def search(self, queryset, name, value): def search(self, queryset, name, value):
if not value.strip(): if not value.strip():

View File

@ -1,4 +1,5 @@
from django.db import migrations, models from django.db import migrations, models
import django.db.models.functions.text
class Migration(migrations.Migration): class Migration(migrations.Migration):
@ -170,11 +171,11 @@ class Migration(migrations.Migration):
), ),
migrations.AddConstraint( migrations.AddConstraint(
model_name='device', model_name='device',
constraint=models.UniqueConstraint(fields=('name', 'site', 'tenant'), name='dcim_device_unique_name_site_tenant'), constraint=models.UniqueConstraint(django.db.models.functions.text.Lower('name'), models.F('site'), models.F('tenant'), name='dcim_device_unique_name_site_tenant'),
), ),
migrations.AddConstraint( migrations.AddConstraint(
model_name='device', model_name='device',
constraint=models.UniqueConstraint(condition=models.Q(('tenant__isnull', True)), fields=('name', 'site'), name='dcim_device_unique_name_site', violation_error_message='Device name must be unique per site.'), constraint=models.UniqueConstraint(django.db.models.functions.text.Lower('name'), models.F('site'), condition=models.Q(('tenant__isnull', True)), name='dcim_device_unique_name_site', violation_error_message='Device name must be unique per site.'),
), ),
migrations.AddConstraint( migrations.AddConstraint(
model_name='device', model_name='device',

View File

@ -8,6 +8,7 @@ from django.core.exceptions import ValidationError
from django.core.validators import MaxValueValidator, MinValueValidator from django.core.validators import MaxValueValidator, MinValueValidator
from django.db import models from django.db import models
from django.db.models import F, ProtectedError from django.db.models import F, ProtectedError
from django.db.models.functions import Lower
from django.urls import reverse from django.urls import reverse
from django.utils.safestring import mark_safe from django.utils.safestring import mark_safe
@ -662,11 +663,11 @@ class Device(NetBoxModel, ConfigContextModel):
ordering = ('_name', 'pk') # Name may be null ordering = ('_name', 'pk') # Name may be null
constraints = ( constraints = (
models.UniqueConstraint( models.UniqueConstraint(
fields=('name', 'site', 'tenant'), Lower('name'), 'site', 'tenant',
name='%(app_label)s_%(class)s_unique_name_site_tenant' name='%(app_label)s_%(class)s_unique_name_site_tenant'
), ),
models.UniqueConstraint( models.UniqueConstraint(
fields=('name', 'site'), Lower('name'), 'site',
name='%(app_label)s_%(class)s_unique_name_site', name='%(app_label)s_%(class)s_unique_name_site',
condition=Q(tenant__isnull=True), condition=Q(tenant__isnull=True),
violation_error_message="Device name must be unique per site." violation_error_message="Device name must be unique per site."

View File

@ -1611,6 +1611,9 @@ class DeviceTestCase(TestCase, ChangeLoggedFilterSetTests):
def test_name(self): def test_name(self):
params = {'name': ['Device 1', 'Device 2']} params = {'name': ['Device 1', 'Device 2']}
self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
# Test case insensitivity
params = {'name': ['DEVICE 1', 'DEVICE 2']}
self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
def test_asset_tag(self): def test_asset_tag(self):
params = {'asset_tag': ['1001', '1002']} params = {'asset_tag': ['1001', '1002']}

View File

@ -399,6 +399,27 @@ class DeviceTestCase(TestCase):
self.assertEqual(Device.objects.filter(name__isnull=True).count(), 2) self.assertEqual(Device.objects.filter(name__isnull=True).count(), 2)
def test_device_name_case_sensitivity(self):
device1 = Device(
site=self.site,
device_type=self.device_type,
device_role=self.device_role,
name='device 1'
)
device1.save()
device2 = Device(
site=device1.site,
device_type=device1.device_type,
device_role=device1.device_role,
name='DEVICE 1'
)
# Uniqueness validation for name should ignore case
with self.assertRaises(ValidationError):
device2.full_clean()
def test_device_duplicate_names(self): def test_device_duplicate_names(self):
device1 = Device( device1 = Device(

View File

@ -6,7 +6,7 @@ from extras.filtersets import LocalConfigContextFilterSet
from ipam.models import VRF from ipam.models import VRF
from netbox.filtersets import OrganizationalModelFilterSet, NetBoxModelFilterSet from netbox.filtersets import OrganizationalModelFilterSet, NetBoxModelFilterSet
from tenancy.filtersets import TenancyFilterSet, ContactModelFilterSet from tenancy.filtersets import TenancyFilterSet, ContactModelFilterSet
from utilities.filters import MultiValueMACAddressFilter, TreeNodeMultipleChoiceFilter from utilities.filters import MultiValueCharFilter, MultiValueMACAddressFilter, TreeNodeMultipleChoiceFilter
from .choices import * from .choices import *
from .models import Cluster, ClusterGroup, ClusterType, VirtualMachine, VMInterface from .models import Cluster, ClusterGroup, ClusterType, VirtualMachine, VMInterface
@ -196,6 +196,9 @@ class VirtualMachineFilterSet(
to_field_name='slug', to_field_name='slug',
label='Site (slug)', label='Site (slug)',
) )
name = MultiValueCharFilter(
lookup_expr='iexact'
)
role_id = django_filters.ModelMultipleChoiceFilter( role_id = django_filters.ModelMultipleChoiceFilter(
queryset=DeviceRole.objects.all(), queryset=DeviceRole.objects.all(),
label='Role (ID)', label='Role (ID)',
@ -227,7 +230,7 @@ class VirtualMachineFilterSet(
class Meta: class Meta:
model = VirtualMachine model = VirtualMachine
fields = ['id', 'name', 'cluster', 'vcpus', 'memory', 'disk'] fields = ['id', 'cluster', 'vcpus', 'memory', 'disk']
def search(self, queryset, name, value): def search(self, queryset, name, value):
if not value.strip(): if not value.strip():

View File

@ -1,4 +1,5 @@
from django.db import migrations, models from django.db import migrations, models
import django.db.models.functions.text
class Migration(migrations.Migration): class Migration(migrations.Migration):
@ -30,11 +31,11 @@ class Migration(migrations.Migration):
), ),
migrations.AddConstraint( migrations.AddConstraint(
model_name='virtualmachine', model_name='virtualmachine',
constraint=models.UniqueConstraint(fields=('name', 'cluster', 'tenant'), name='virtualization_virtualmachine_unique_name_cluster_tenant'), constraint=models.UniqueConstraint(django.db.models.functions.text.Lower('name'), models.F('cluster'), models.F('tenant'), name='virtualization_virtualmachine_unique_name_cluster_tenant'),
), ),
migrations.AddConstraint( migrations.AddConstraint(
model_name='virtualmachine', model_name='virtualmachine',
constraint=models.UniqueConstraint(condition=models.Q(('tenant__isnull', True)), fields=('name', 'cluster'), name='virtualization_virtualmachine_unique_name_cluster', violation_error_message='Virtual machine name must be unique per site.'), constraint=models.UniqueConstraint(django.db.models.functions.text.Lower('name'), models.F('cluster'), condition=models.Q(('tenant__isnull', True)), name='virtualization_virtualmachine_unique_name_cluster', violation_error_message='Virtual machine name must be unique per cluster.'),
), ),
migrations.AddConstraint( migrations.AddConstraint(
model_name='vminterface', model_name='vminterface',

View File

@ -3,6 +3,7 @@ from django.core.exceptions import ValidationError
from django.core.validators import MinValueValidator from django.core.validators import MinValueValidator
from django.db import models from django.db import models
from django.db.models import Q from django.db.models import Q
from django.db.models.functions import Lower
from django.urls import reverse from django.urls import reverse
from dcim.models import BaseInterface, Device from dcim.models import BaseInterface, Device
@ -318,14 +319,14 @@ class VirtualMachine(NetBoxModel, ConfigContextModel):
ordering = ('_name', 'pk') # Name may be non-unique ordering = ('_name', 'pk') # Name may be non-unique
constraints = ( constraints = (
models.UniqueConstraint( models.UniqueConstraint(
fields=('name', 'cluster', 'tenant'), Lower('name'), 'cluster', 'tenant',
name='%(app_label)s_%(class)s_unique_name_cluster_tenant' name='%(app_label)s_%(class)s_unique_name_cluster_tenant'
), ),
models.UniqueConstraint( models.UniqueConstraint(
fields=('name', 'cluster'), Lower('name'), 'cluster',
name='%(app_label)s_%(class)s_unique_name_cluster', name='%(app_label)s_%(class)s_unique_name_cluster',
condition=Q(tenant__isnull=True), condition=Q(tenant__isnull=True),
violation_error_message="Virtual machine name must be unique per site." violation_error_message="Virtual machine name must be unique per cluster."
), ),
) )

View File

@ -299,6 +299,9 @@ class VirtualMachineTestCase(TestCase, ChangeLoggedFilterSetTests):
def test_name(self): def test_name(self):
params = {'name': ['Virtual Machine 1', 'Virtual Machine 2']} params = {'name': ['Virtual Machine 1', 'Virtual Machine 2']}
self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
# Test case insensitivity
params = {'name': ['VIRTUAL MACHINE 1', 'VIRTUAL MACHINE 2']}
self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
def test_vcpus(self): def test_vcpus(self):
params = {'vcpus': [1, 2]} params = {'vcpus': [1, 2]}

View File

@ -8,12 +8,14 @@ from tenancy.models import Tenant
class VirtualMachineTestCase(TestCase): class VirtualMachineTestCase(TestCase):
def test_vm_duplicate_name_per_cluster(self): @classmethod
def setUpTestData(cls):
cluster_type = ClusterType.objects.create(name='Cluster Type 1', slug='cluster-type-1') cluster_type = ClusterType.objects.create(name='Cluster Type 1', slug='cluster-type-1')
cluster = Cluster.objects.create(name='Cluster 1', type=cluster_type) Cluster.objects.create(name='Cluster 1', type=cluster_type)
def test_vm_duplicate_name_per_cluster(self):
vm1 = VirtualMachine( vm1 = VirtualMachine(
cluster=cluster, cluster=Cluster.objects.first(),
name='Test VM 1' name='Test VM 1'
) )
vm1.save() vm1.save()
@ -43,7 +45,7 @@ class VirtualMachineTestCase(TestCase):
vm2.save() vm2.save()
def test_vm_mismatched_site_cluster(self): def test_vm_mismatched_site_cluster(self):
cluster_type = ClusterType.objects.create(name='Cluster Type 1', slug='cluster-type-1') cluster_type = ClusterType.objects.first()
sites = ( sites = (
Site(name='Site 1', slug='site-1'), Site(name='Site 1', slug='site-1'),
@ -71,3 +73,19 @@ class VirtualMachineTestCase(TestCase):
# VM with cluster site but no direct site should fail # VM with cluster site but no direct site should fail
with self.assertRaises(ValidationError): with self.assertRaises(ValidationError):
VirtualMachine(name='vm1', site=None, cluster=clusters[0]).full_clean() VirtualMachine(name='vm1', site=None, cluster=clusters[0]).full_clean()
def test_vm_name_case_sensitivity(self):
vm1 = VirtualMachine(
cluster=Cluster.objects.first(),
name='virtual machine 1'
)
vm1.save()
vm2 = VirtualMachine(
cluster=vm1.cluster,
name='VIRTUAL MACHINE 1'
)
# Uniqueness validation for name should ignore case
with self.assertRaises(ValidationError):
vm2.full_clean()