jkoelker / quark

This is my fork, Quark is now at https://github.com/rackerlabs/quark
Apache License 2.0
0 stars 2 forks source link

Subnet Overlap function checks *all* subnets, not just the ones associated with a network #87

Closed MattDietz closed 11 years ago

MattDietz commented 11 years ago

When allow_overlapping_ips is false, the other side of the conditional in _validate_subnet_cidr checks all subnets, not just ones owned by the network or (I think) even owned by the tenant.

Repro:

asadoughi commented 11 years ago

Right, I thought that's how the configuration variable is supposed to work in Quantum, no? When we deploy quark, set your quantum.conf:allow_overlapping_ips to True. Should we make some deployment notes?

MattDietz commented 11 years ago

I would think that allow_overlapping_ips would allow overlaps within the same network, or maybe on the same tenant. Unless I'm mistaken, this actually would prevent anyone from creating a subnet that in someway overlaps with any other one owned by any tenant (since you're the context to elevated before the query, I think it grabs all subnets in the DB)

And a cursory glance at the quantum docs online doesn't show me exactly what that's for... :-(

tr3buchet commented 11 years ago

Let me see if I can understand what is happening here. My take is that we don't want to allow overlapping subnets in the same network in quark. There is a configuration variable allow_overlapping_ips in quantum. This is how it should work:

From the bug report, setting it False will instead ensure that no overlapping subnets can exist at all, regardless of network or tenant. This is broken and needs to be fixed. Also related to this, quark should default allow_overlapping_ips to False. We probably also want add it to the config as False for good measure.

tr3buchet commented 11 years ago

we may even want to raise if it is set to True in the config because it's totes shenanigans.

MattDietz commented 11 years ago

totes!