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

Make Device/VM unique name enforcement case insensitive #9249

Closed
Mizarv opened this issue Apr 28, 2022 · 8 comments
Closed

Make Device/VM unique name enforcement case insensitive #9249

Mizarv opened this issue Apr 28, 2022 · 8 comments
Assignees
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Milestone

Comments

@Mizarv
Copy link

Mizarv commented Apr 28, 2022

NetBox version

v3.2.0

Feature type

Change to existing functionality

Proposed functionality

Presently, Netbox validates device/VM names to ensure they are unique, but the enforcement of this is case sensitive. This allows device1 and DEVICE1 to coexist but not DEVICE2 and DEVICE2

I propose that the existing validation is extended to be case insensitive to prevent duplicate devices/VMs being created. This could be a change to the default behaviour or perhaps a setting in configuration.py

Use case

Extending the existing validation would prevent duplicate devices/VMs being created where the user is unaware that the intended device already exists within Netbox.

I can't think of a use case for the existing behaviour over the proposed behaviour

Database changes

No response

External dependencies

No response

@Mizarv Mizarv added the type: feature Introduction of new functionality to the application label Apr 28, 2022
@jeremystretch jeremystretch added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Apr 28, 2022
@falz
Copy link

falz commented Apr 28, 2022

$.02 from an outsider - i consider hostnames as case INsensitive (as with dns), so enforcing unique vm names (vm1 == VM1) seems most sensible and the least confusing.

Taking this a step further, i am surprised to see that netbox devices have this same potential 'issue' - device1 and DEVICE1 are both allowed to be created at same site and tenant.

Possibly something that crept up after slugs were dropped?

@tyler-8
Copy link
Contributor

tyler-8 commented Apr 28, 2022

Related: #2669

Some people want to be able to duplicate names.

Device name != DNS name

FWIW I made a custom validator that provides this functionality should this FR not make it into NetBox:

https://gist.github.com/tyler-8/0a99763cae01c97e8f80f5aca09db968

@Mizarv
Copy link
Author

Mizarv commented Apr 28, 2022

You can have duplicate names at the moment regardless of case, for a device it needs to be in a different site, for a virtual machine it needs to be in a different cluster.

We could keep that functionality but the FR is really to just change the enforcement to case insensitive where currently enforced,

@tyler-8
Copy link
Contributor

tyler-8 commented Apr 28, 2022

change the enforcement to case insensitive where currently enforced

Right - this makes sense to me. The uniqueness is already enforced in certain scenarios today in core NetBox, and in those instances, it should be case-insensitive.

The validator I made would be for those who want global uniqueness.

@jeremystretch
Copy link
Member

The corollary of this proposal is that there is no supported use case for differentiating among device/VM names by letter case. IMO that's a fair assertion but I'd like to leave this open for a while for the community to review.

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Jun 28, 2022
@jeremystretch jeremystretch added this to the v3.3 milestone Jun 28, 2022
@jeremystretch
Copy link
Member

jeremystretch commented Jun 30, 2022

After digging into this a bit, the ideal solution seems to be converting the relevant name fields to PostgreSQL-specific CICharFields. This is a special database type (citext) which effectively calls LOWER() on the field value when performing comparisons.

However, there's a catch: Enabling this field type in the database requires the citext extension, which itself requires PostgreSQL superuser permission to install. This may not be tenable for a significant portion of the NetBox user base.

Alternatively, PostgreSQL 12+ supports non-deterministic collations that can be used for case-insensitive matching as well as disregarding accents and diacritics (see #9613). (The PostgreSQL documentation specifically recommends the use of non-deterministic collations over citext where available.) This appears to be straightforward to implement using Django's CreateCollation() and Collate() functions. Of course, this would require us to raise the minimum PostgreSQL version from 10 to 12.

While it would theoretically be possible to tackle this in PostgreSQL 10 without using the citext extension, it would require us to manually invoke LOWER() in several places and to ensure we're also specifically employing case insensitive filters when querying against the name field. IMO we should defer this work until we're okay with bumping the minimum PostgreSQL version to 12 (which won't happen in NetBox v3.3, but could be done for the next release).

@jeremystretch jeremystretch removed this from the v3.3 milestone Jun 30, 2022
@jeremystretch jeremystretch added needs milestone Awaiting prioritization for inclusion with a future NetBox release and removed status: accepted This issue has been accepted for implementation labels Jun 30, 2022
@ZPrimed
Copy link

ZPrimed commented Jun 30, 2022

I'm all for making names case-insensitive.

Re: #2669 - I do think it is nice ("MAY implement") to be able to have identically named devices (or VMs) at different sites/locations though, even though I don't think that is a good practice. I've seen people who have lots of tiny branch sites (e.g. big restaurant chain) and they just want to call all of the switches "switch" rather than including the site name or ID in the device's name. (Again, I don't think this is wise, but the use case is there.)

I was originally going to say something about bumping minimum PGSQL version potentially being annoying for people who are deployed on a "long-term-stable" OS that doesn't provide PGSQL 12+... But Ubuntu 20.04 (which I think is probably one of the more common "LTS" distributions) includes pgsql12. 😛

This could be minorly annoying to people still on CentOS7 or something, but that is close to EOL (and there are ways to get newer versions of pgsql, just requires some extra effort).

And as someone who isn't good at data modelling and relational DB intricacies, I don't envy the work but certainly appreciate it all. 😄

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed needs milestone Awaiting prioritization for inclusion with a future NetBox release labels Jul 27, 2022
@jeremystretch jeremystretch added this to the v3.4 milestone Jul 27, 2022
@jeremystretch
Copy link
Member

I think this is doable just using conditional UniqueConstraints, wrapping the device/VM name with Lower() to ignore case. We'll also have to tweak some filters to ignore case when matching these objects by name. That should be sufficient to meet the cited use case, I think.

Marking this as blocked by #10361 for now.

@jeremystretch jeremystretch added status: blocked Another issue or external requirement is preventing implementation and removed status: accepted This issue has been accepted for implementation labels Sep 15, 2022
@jeremystretch jeremystretch self-assigned this Sep 27, 2022
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: blocked Another issue or external requirement is preventing implementation labels Sep 27, 2022
jeremystretch added a commit that referenced this issue Sep 27, 2022
jeremystretch added a commit that referenced this issue Sep 28, 2022
jsenecal pushed a commit to jsenecal/netbox that referenced this issue Sep 28, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

No branches or pull requests

5 participants