starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-11 21:10:29 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

Rack name uniqueness not enforced if rackgroups are not being used #2371

New Issue
Closed
opened 2025-12-29 17:25:24 +01:00 by adam · 5 comments
adam commented 2025-12-29 17:25:24 +01:00
Owner
Copy Link

Originally created by @candlerb on GitHub (Feb 13, 2019).

  • Python version: 3.5.2
  • NetBox version: 2.5.5

Steps to Reproduce

  1. Create a rack with name "foo" in a particular site. Do not add it to a rack group.
  2. Create another rack with name "foo" in the same site, also not in a rack group.

Repeat for facility_id

Expected Behavior

Duplicate names and facility_ids would be rejected, at least if the racks are within the same Site.

Observed Behavior

Duplicate names and facility_ids are accepted, for all racks which are not in rackgroups, whether or not they are in the same Site. Relates to: #238 and this commit (previously, rack names were unique within a site)

Analysis

The fundamental issue is that null values are not considered equal in Postgres composite unique indexes. "A multicolumn unique index will only reject cases where all indexed columns are equal in multiple rows."

The index could use COALESCE(group_id,0) instead of group_id. It is possible to create a unique index on an expression, but I don't know about composite indexes. At worst it might be necessary to change this to an exclusion constraint.

In addition, it will be necessary to include site_id as a column within the unique index, so that you can re-use the same rack name and facility_id in different sites. Or you could do something funky like COALESCE(group_id,-site_id)

Note also that "Steps to reproduce" was done via the web interface. So neither Django's ORM nor the database are enforcing name uniqueness, when rackgroup is null.

Originally created by @candlerb on GitHub (Feb 13, 2019). * Python version: 3.5.2 * NetBox version: 2.5.5 ### Steps to Reproduce 1. Create a rack with name "foo" in a particular site. Do not add it to a rack group. 2. Create another rack with name "foo" in the same site, also not in a rack group. Repeat for `facility_id` ### Expected Behavior Duplicate names and facility_ids would be rejected, at least if the racks are within the same Site. ### Observed Behavior Duplicate names and facility_ids are accepted, for all racks which are not in rackgroups, whether or not they are in the same Site. Relates to: #238 and [this commit](https://github.com/digitalocean/netbox/commit/4fd52d46bfa9ced328b6f277f5150ffe232ce3fd) (previously, rack names were unique within a site) ### Analysis The fundamental issue is that [null values are not considered equal](https://www.postgresql.org/docs/9.4/indexes-unique.html) in Postgres composite unique indexes. *"A multicolumn unique index will only reject cases where all indexed columns are equal in multiple rows."* The index could use `COALESCE(group_id,0)` instead of `group_id`. It is possible to create a [unique index on an expression](https://www.postgresql.org/docs/9.4/indexes-expressional.html), but I don't know about composite indexes. At worst it might be necessary to change this to an [exclusion constraint](http://nathanmlong.com/2016/01/protect-your-data-with-postgresql-constraints/). In addition, it will be necessary to include `site_id` as a column within the unique index, so that you can re-use the same rack name and facility_id in different sites. Or you could do something funky like `COALESCE(group_id,-site_id)` Note also that "Steps to reproduce" was done via the web interface. So neither Django's ORM *nor* the database are enforcing name uniqueness, when rackgroup is null.
adam closed this issue 2025-12-29 17:25:24 +01:00
adam commented 2025-12-29 17:25:25 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Feb 20, 2019):

I'm worried that this has been "broken" long enough that restoring the original behavior would result in unpleasant surprise for some users. I'm okay leaving this as-is to be honest. Leaving it open for feedback.

@jeremystretch commented on GitHub (Feb 20, 2019): I'm worried that this has been "broken" long enough that restoring the original behavior would result in unpleasant surprise for some users. I'm okay leaving this as-is to be honest. Leaving it open for feedback.
adam commented 2025-12-29 17:25:25 +01:00
Author
Owner
Copy Link

@candlerb commented on GitHub (Feb 20, 2019):

Well, if you want to leave weird behaviour then it should at least be clearly documented: "duplicate rack names within a site are permitted, but only when those racks are not in rackgroups".

I suppose you could be consistent, and drop all uniqueness checks on rack names and facility ids.

Beware the negative impact on CSV upload though. When importing devices, you give a site and rack_name. If the rack name is non-unique, what happens?

I've just tested this:

  • Create two racks called rack1 in site test
  • CSV upload devices:
device_role,manufacturer,model_name,status,site,rack_name,name
Core Switch,Netgear,GS724Tv4,active,test,rack1,foo
Core Switch,Netgear,GS724Tv4,active,test,rack1,bar

The result is an exception:

image

This is the sort of thing database integrity checks are there to prevent.

@candlerb commented on GitHub (Feb 20, 2019): Well, if you want to leave weird behaviour then it should at least be clearly documented: "duplicate rack names within a site are permitted, but only when those racks are not in rackgroups". I suppose you could be consistent, and drop *all* uniqueness checks on rack names and facility ids. Beware the negative impact on CSV upload though. When importing devices, you give a site and rack_name. If the rack name is non-unique, what happens? I've just tested this: * Create two racks called `rack1` in site `test` * CSV upload devices: ``` device_role,manufacturer,model_name,status,site,rack_name,name Core Switch,Netgear,GS724Tv4,active,test,rack1,foo Core Switch,Netgear,GS724Tv4,active,test,rack1,bar ``` The result is an exception: ![image](https://user-images.githubusercontent.com/44789/53107476-011d2b00-352d-11e9-8d58-363bf9342c7c.png) This is the sort of thing database integrity checks are there to prevent.
adam commented 2025-12-29 17:25:25 +01:00
Author
Owner
Copy Link

@candlerb commented on GitHub (Feb 20, 2019):

It would be straightforward to find any affected racks in a migration:

select name,site_id,count(*) from dcim_rack where group_id is null
group by name,site_id having count(*) > 1;

and then update name to concat(name,'//',id) (or something more complex unlikely to clash with existing rack names)

@candlerb commented on GitHub (Feb 20, 2019): It would be straightforward to find any affected racks in a migration: ``` select name,site_id,count(*) from dcim_rack where group_id is null group by name,site_id having count(*) > 1; ``` and then update name to `concat(name,'//',id)` (or something more complex unlikely to clash with existing rack names)
adam commented 2025-12-29 17:25:25 +01:00
Author
Owner
Copy Link

@gjvc commented on GitHub (Feb 23, 2019):

+1 for having rack names globally unique

@gjvc commented on GitHub (Feb 23, 2019): +1 for having rack names globally unique
adam commented 2025-12-29 17:25:25 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Dec 13, 2019):

Closing due to lack of activity/feedback.

@jeremystretch commented on GitHub (Dec 13, 2019): Closing due to lack of activity/feedback.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#2371
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?