dropbox / nsot

Network Source of Truth is an open source IPAM and network inventory database
https://nsot.readthedocs.io
Other
399 stars 66 forks source link

Networks in assigned state are being offered as available #247

Closed jathanism closed 7 years ago

jathanism commented 7 years ago

In NSoT 1.0.10, when a Network state is set to "assigned" the next_network function will offer the network as available. This is not good and seems to be a regression.

The workaround for now is to forcefully set any networks to state "allocated" and they will no longer be offered as available.

nickpegg commented 7 years ago

Steps to reproduce:

vagrant@nsot-dev:~$ nsot networks add -c 10.1.0.0/24                            
[SUCCESS] Added network!                                                        
vagrant@nsot-dev:~$ nsot networks add -c 10.1.0.1/32                            
[SUCCESS] Added network!                                                        
vagrant@nsot-dev:~$ nsot networks list -c 10.1.0.0/24 next_network -p 32 -n 1   
10.1.0.2/32                                                                     
vagrant@nsot-dev:~$ nsot networks update -c 10.1.0.1/32 -S assigned             
[SUCCESS] Updated network!                                                      
vagrant@nsot-dev:~$ nsot networks list -c 10.1.0.0/24 next_network -p 32 -n 1   
10.1.0.1/32
nickpegg commented 7 years ago

I've confirmed that the regression was caused by this commit: https://github.com/dropbox/nsot/commit/57c04d0da113cb10e551d326237b2a897f2d850b

with 57c04d0d~1 checked out, nsot networks list -c 10.1.0.0/24 next_network -p 32 gives me 10.1.0.2/32 as expected.

nickpegg commented 7 years ago

Re-opening this since my fix didn't cover all cases. Since children consists of networks in a not-busy state and dirty is only checking busy networks of the same prefix length, anything that is in a busy state and not of the same prefix length of what we're looking for are never checked to see if they conflict with candidate networks.

For example, if we have 10.0.0.0/24 with a bunch of /32s assigned (.1, .2, .3, etc.) and we request a /31, 10.0.0.0/31 could get returned.

I've already got a patch in the works. I'll open a PR when it's ready.

jathanism commented 7 years ago

Fixed in #250 and #252