Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closes #9249: Ignore case for device/VM names #10488

Merged
merged 3 commits into from
Sep 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/release-notes/version-3.4.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@
!!! warning "PostgreSQL 11 Required"
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

* [#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

### Plugins API
Expand Down
5 changes: 4 additions & 1 deletion netbox/dcim/filtersets.py
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,9 @@ class DeviceFilterSet(NetBoxModelFilterSet, TenancyFilterSet, ContactModelFilter
to_field_name='slug',
label='Device model (slug)',
)
name = MultiValueCharFilter(
lookup_expr='iexact'
)
status = django_filters.MultipleChoiceFilter(
choices=DeviceStatusChoices,
null_value=None
Expand Down Expand Up @@ -950,7 +953,7 @@ class DeviceFilterSet(NetBoxModelFilterSet, TenancyFilterSet, ContactModelFilter

class Meta:
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):
if not value.strip():
Expand Down
5 changes: 3 additions & 2 deletions netbox/dcim/migrations/0162_unique_constraints.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from django.db import migrations, models
import django.db.models.functions.text


class Migration(migrations.Migration):
Expand Down Expand Up @@ -170,11 +171,11 @@ class Migration(migrations.Migration):
),
migrations.AddConstraint(
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(
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(
model_name='device',
Expand Down
5 changes: 3 additions & 2 deletions netbox/dcim/models/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from django.core.validators import MaxValueValidator, MinValueValidator
from django.db import models
from django.db.models import F, ProtectedError
from django.db.models.functions import Lower
from django.urls import reverse
from django.utils.safestring import mark_safe

Expand Down Expand Up @@ -662,11 +663,11 @@ class Meta:
ordering = ('_name', 'pk') # Name may be null
constraints = (
models.UniqueConstraint(
fields=('name', 'site', 'tenant'),
Lower('name'), 'site', 'tenant',
name='%(app_label)s_%(class)s_unique_name_site_tenant'
),
models.UniqueConstraint(
fields=('name', 'site'),
Lower('name'), 'site',
name='%(app_label)s_%(class)s_unique_name_site',
condition=Q(tenant__isnull=True),
violation_error_message="Device name must be unique per site."
Expand Down
3 changes: 3 additions & 0 deletions netbox/dcim/tests/test_filtersets.py
Original file line number Diff line number Diff line change
Expand Up @@ -1611,6 +1611,9 @@ def setUpTestData(cls):
def test_name(self):
params = {'name': ['Device 1', 'Device 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):
params = {'asset_tag': ['1001', '1002']}
Expand Down
21 changes: 21 additions & 0 deletions netbox/dcim/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,27 @@ def test_multiple_unnamed_devices(self):

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):

device1 = Device(
Expand Down
7 changes: 5 additions & 2 deletions netbox/virtualization/filtersets.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from ipam.models import VRF
from netbox.filtersets import OrganizationalModelFilterSet, NetBoxModelFilterSet
from tenancy.filtersets import TenancyFilterSet, ContactModelFilterSet
from utilities.filters import MultiValueMACAddressFilter, TreeNodeMultipleChoiceFilter
from utilities.filters import MultiValueCharFilter, MultiValueMACAddressFilter, TreeNodeMultipleChoiceFilter
from .choices import *
from .models import Cluster, ClusterGroup, ClusterType, VirtualMachine, VMInterface

Expand Down Expand Up @@ -196,6 +196,9 @@ class VirtualMachineFilterSet(
to_field_name='slug',
label='Site (slug)',
)
name = MultiValueCharFilter(
lookup_expr='iexact'
)
role_id = django_filters.ModelMultipleChoiceFilter(
queryset=DeviceRole.objects.all(),
label='Role (ID)',
Expand Down Expand Up @@ -227,7 +230,7 @@ class VirtualMachineFilterSet(

class Meta:
model = VirtualMachine
fields = ['id', 'name', 'cluster', 'vcpus', 'memory', 'disk']
fields = ['id', 'cluster', 'vcpus', 'memory', 'disk']

def search(self, queryset, name, value):
if not value.strip():
Expand Down
5 changes: 3 additions & 2 deletions netbox/virtualization/migrations/0033_unique_constraints.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from django.db import migrations, models
import django.db.models.functions.text


class Migration(migrations.Migration):
Expand Down Expand Up @@ -30,11 +31,11 @@ class Migration(migrations.Migration):
),
migrations.AddConstraint(
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(
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(
model_name='vminterface',
Expand Down
7 changes: 4 additions & 3 deletions netbox/virtualization/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from django.core.validators import MinValueValidator
from django.db import models
from django.db.models import Q
from django.db.models.functions import Lower
from django.urls import reverse

from dcim.models import BaseInterface, Device
Expand Down Expand Up @@ -318,14 +319,14 @@ class Meta:
ordering = ('_name', 'pk') # Name may be non-unique
constraints = (
models.UniqueConstraint(
fields=('name', 'cluster', 'tenant'),
Lower('name'), 'cluster', 'tenant',
name='%(app_label)s_%(class)s_unique_name_cluster_tenant'
),
models.UniqueConstraint(
fields=('name', 'cluster'),
Lower('name'), 'cluster',
name='%(app_label)s_%(class)s_unique_name_cluster',
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."
),
)

Expand Down
3 changes: 3 additions & 0 deletions netbox/virtualization/tests/test_filtersets.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,9 @@ def setUpTestData(cls):
def test_name(self):
params = {'name': ['Virtual Machine 1', 'Virtual Machine 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):
params = {'vcpus': [1, 2]}
Expand Down
26 changes: 22 additions & 4 deletions netbox/virtualization/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@

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 = 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(
cluster=cluster,
cluster=Cluster.objects.first(),
name='Test VM 1'
)
vm1.save()
Expand Down Expand Up @@ -43,7 +45,7 @@ def test_vm_duplicate_name_per_cluster(self):
vm2.save()

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 = (
Site(name='Site 1', slug='site-1'),
Expand Down Expand Up @@ -71,3 +73,19 @@ def test_vm_mismatched_site_cluster(self):
# VM with cluster site but no direct site should fail
with self.assertRaises(ValidationError):
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()