Missing validation of Site consistency between Device, Rack, Rackgroup, and PowerPanel

[candlerb]

(Extracted from #4971)

Environment

• Python version: 3.6.9
• NetBox version: 2.9.8

Summary: missing validation of the consistency of links from Device, Rack and Rackgroup tree to Site.

Steps to Reproduce

Case 1: Device-Site not matching Rack-Site

1. Create sites A and B
2. Create rack R in site A
3. Create device D in site A and rack R
4. Change rack R to site B

Case 2: Rack-Site not matching Rackgroup-Site

1. Create sites A and B
2. Create rackgroup G in site A
3. Create rack R in site A and rackgroup G
4. Change rackgroup G to site B

Case 3: Rackgroup-Site not matching parent Rackgroup-Site

1. Create sites A and B
2. Create rackgroup G1 in site A
3. Create rackgroup G2 in site A with parent rackgroup G1
4. Change rackgroup G1 to site B

Case 4: These relationships also apply transitively: parent rackgroup -> child rackgroup -> rack -> device

1. Create sites A and B
2. Create rackgroup G1 in site A
3. Create rackgroup G2 in site A with parent rackgroup G1
4. Create rack R in site A and rackgroup G2
5. Create device D in site A and rack R
6. Change rackgroup G1 to site B

Expected Behavior

In case 1: either change to be rejected, or rack R's new site to be propagated to all Devices underneath it
In case 2: either change to be rejected, or rack group G's new site to be propagated to all Racks underneath it
In case 3: either change to be rejected, or rack group G1's new site to be propagated to all child and descendant rackgroups
In case 4: either change to be rejected, or rack group G1's new site to be propagated recursively to all child and descendant rackgroups, racks and devices

Observed Behavior

In case 1: device D is in site A but its rack is in site B
In case 2: rack R is in site A but its rackgroup is in site B
In case 3: rackgroup G2 is in site A but its parent rackgroup G1 is in site B
In case 4: device D, rack R, rackgroup G2 are all in site A but the ancestor rackgroup G1 is in site B

Additional Note

PowerPanel also has a link to Site and RackGroup, so a similar issue applies there too.

[DanSheps]

I think the solution to this, would be to really use a concrete base model and have every other hierarchical model inherit off of that.

Example:

class Location(MPTTModel):
    name = models.CharField

class Region(Location):
    # Region specific stuff here

class Site(Location):
    # Site specific stuff here

class RackGroup(Location):
    # RG specific stuff here

This would allow you to enforce the integrity properly no matter where you are.

[DanSheps]

Here is the idea so far:

• Move to a base model, "Location"; location would be MPTT
• Name & Slug would be moved to location; all other data would remain on the inheritees
• Possibly use polymorphic and polymorphic-tree
• All organizational models would inherit "Location"
  • Region
  • Site
  • Rack Groups
  • Rack??
• API Changish (we can point existing API's fields to new model and make them read_only)
• Would close Add ability to assign (non racked) devices to rack groups (aka rooms) #4971 as well
• Proposed Data Model:

[jeremystretch]

I think we should evaluate less disruptive potential solutions before considering a major overhaul to these models. It's likely that much of this can be resolved without much effort.

Additionally, this is quite a bit to pack into one issue. IMO it should be broken into four separate concerns, as it's unlikely that all four cited behaviors will be addressed with a single change.

[jeremystretch]

Case 1: Device-Site not matching Rack-Site

I'm not able to reproduce this behavior on v2.10.2. After assigning the rack to the new site, the device's site is updated correctly, and a change record is created to reflect the change. (There is logic within Rack.save() which addresses this.)

[candlerb]

OK I probably got case 1 wrong then, so I have now struck it out. I'd checked all the clean logic but not save. I believe the rest are still valid.

[jeremystretch]

@candlerb I think I've covered all of your cases in PR #5522. Please let me know what you think. Thanks for putting together such a detailed compilation!

[candlerb]

I've cherry-picked tested this onto 2.10.2 and done a brief test; it works.

The same issue remains for Power Panels - which are not associated with a rack, but are associated with a rackgroup and site. I can replicate as follows:

1. Create sites A and B
2. Create rackgroup G in site A
3. Create power panel P in site A and rackgroup G
4. Change rackgroup G to site B

Expected result: power panel P is now associated with site B (or update rejected)
Actual result: power panel P is still explicitly associated with site A

---

Depending on how that's fixed, I believe there's another problem.

Power feeds are associated with a PowerPanel and a nullable Rack (but not a site, since that's implied from the PowerPanel). The UI currently enforces that when you create a PowerFeed it must be in a rack in the same site as its parent PowerPanel.

However, if you edit a PowerPanel to change its Site (and RackGroup, if any), I think this could end up with its child PowerFeeds being in a Rack in a different Site. Since there's no way to choose the correct new Rack for the PowerFeeds, I think it would be necessary either to set all the child PowerFeeds Racks to None - or prevent the update in the first place.

A related problem occurs when you consider that a Rack may also be moved from one Site (and RackGroup) to another. You have:

      ,----------------------<
      |                   ,--< panel P1------.
      |                  +                   ^
site S1 ---< rackgroup G1                  feed F1
      |                  +                   v
      |                   `--< rack R1 +-----'
       `---------------------<

If you change rack R1 to a different rackgroup and site, then the feeds will be in a rack in the new site, while the panel is still in the old site.

However, whilst this is a violation of policy ("a power feed ought not to be in a rack in a different site than its power panel"), it's not an actual database inconsistency (such as the very first example in this post, which resulted in a power panel linked to site A and simultaneously to a rackgroup in site B)

---

Unrelated: I came across some other cases of def clean which don't have super().clean(). Should those be added?

[jeremystretch]

The same issue remains for Power Panels

Okay, simple enough to include that model in the signal receiver as well.

Depending on how that's fixed, I believe there's another problem.

Could you copy your description above into a separate issue please? It's certainly related, but worthy of its own issue.

Unrelated: I came across some other cases of def clean which don't have super().clean(). Should those be added?

Sure. If you want to open a housekeeping issue calling out any specific locations I'll look into them.