diff --git a/ipam/allocator.go b/ipam/allocator.go index 7213148bcc..44070119dc 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -203,6 +203,10 @@ func (a *Allocator) GetDefaultAddressSpaces() (string, string, error) { } // RequestPool returns an address pool along with its unique id. +// addressSpace must be a valid address space name and must not be the empty string. +// If pool is the empty string then the default predefined pool for addressSpace will be used, otherwise pool must be a valid IP address and length in CIDR notation. +// If subPool is not empty, it must be a valid IP address and length in CIDR notation which is a sub-range of pool. +// subPool must be empty if pool is empty. func (a *Allocator) RequestPool(addressSpace, pool, subPool string, options map[string]string, v6 bool) (string, *net.IPNet, map[string]string, error) { logrus.Debugf("RequestPool(%s, %s, %s, %v, %t)", addressSpace, pool, subPool, options, v6) @@ -283,8 +287,8 @@ retry: return remove() } -// Given the address space, returns the local or global PoolConfig based on the -// address space is local or global. AddressSpace locality is being registered with IPAM out of band. +// Given the address space, returns the local or global PoolConfig based on whether the +// address space is local or global. AddressSpace locality is registered with IPAM out of band. func (a *Allocator) getAddrSpace(as string) (*addrSpace, error) { a.Lock() defer a.Unlock() @@ -295,6 +299,8 @@ func (a *Allocator) getAddrSpace(as string) (*addrSpace, error) { return aSpace, nil } +// parsePoolRequest parses and validates a request to create a new pool under addressSpace and returns +// a SubnetKey, network and range describing the request. func (a *Allocator) parsePoolRequest(addressSpace, pool, subPool string, v6 bool) (*SubnetKey, *net.IPNet, *AddressRange, error) { var ( nw *net.IPNet diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index 65f63f8643..5eea16c9f9 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -286,28 +286,17 @@ func TestAddSubnets(t *testing.T) { // been released raises an error. func TestDoublePoolRelease(t *testing.T) { for _, store := range []bool{false, true} { - for _, repeats := range []int{0, 1, 10} { - a, err := getAllocator(store) - assert.NoError(t, err) - - // Request initial pool allocation - pid0, _, _, err := a.RequestPool(localAddressSpace, "10.0.0.0/8", "", nil, false) - assert.NoError(t, err) + a, err := getAllocator(store) + assert.NoError(t, err) - // Re-request the same pool - for i := 0; i < repeats; i++ { - _, _, _, err := a.RequestPool(localAddressSpace, "10.0.0.0/8", "", nil, false) - assert.Error(t, err) - } + pid0, _, _, err := a.RequestPool(localAddressSpace, "10.0.0.0/8", "", nil, false) + assert.NoError(t, err) - // Release the initial request - err = a.ReleasePool(pid0) - assert.NoError(t, err) + err = a.ReleasePool(pid0) + assert.NoError(t, err) - // Releasing again fails - err = a.ReleasePool(pid0) - assert.Error(t, err) - } + err = a.ReleasePool(pid0) + assert.Error(t, err) } } @@ -987,6 +976,85 @@ func TestRequest(t *testing.T) { } } +// TestOverlappingRequests tests that overlapping subnets cannot be allocated. +// Requests for subnets which are supersets or subsets of existing allocations, +// or which overlap at the beginning or end, should not be permitted. +func TestOverlappingRequests(t *testing.T) { + input := []struct { + environment []string + subnet string + ok bool + }{ + // IPv4 + // Previously allocated network does not overlap with request + {[]string{"10.0.0.0/8"}, "11.0.0.0/8", true}, + {[]string{"74.0.0.0/7"}, "9.111.99.72/30", true}, + {[]string{"110.192.0.0/10"}, "16.0.0.0/10", true}, + + // Previously allocated network entirely contains request + {[]string{"10.0.0.0/8"}, "10.0.0.0/8", false}, // exact overlap + {[]string{"0.0.0.0/1"}, "16.182.0.0/15", false}, + {[]string{"16.0.0.0/4"}, "17.11.66.0/23", false}, + + // Previously allocated network overlaps beginning of request + {[]string{"0.0.0.0/1"}, "0.0.0.0/0", false}, + {[]string{"64.0.0.0/6"}, "64.0.0.0/3", false}, + {[]string{"112.0.0.0/6"}, "112.0.0.0/4", false}, + + // Previously allocated network overlaps end of request + {[]string{"96.0.0.0/3"}, "0.0.0.0/1", false}, + {[]string{"192.0.0.0/2"}, "128.0.0.0/1", false}, + {[]string{"95.0.0.0/8"}, "92.0.0.0/6", false}, + + // Previously allocated network entirely contained within request + {[]string{"10.0.0.0/8"}, "10.0.0.0/6", false}, // non-canonical + {[]string{"10.0.0.0/8"}, "8.0.0.0/6", false}, // canonical + {[]string{"25.173.144.0/20"}, "0.0.0.0/0", false}, + + // IPv6 + // Previously allocated network entirely contains request + {[]string{"::/0"}, "f656:3484:c878:a05:e540:a6ed:4d70:3740/123", false}, + {[]string{"8000::/1"}, "8fe8:e7c4:5779::/49", false}, + {[]string{"f000::/4"}, "ffc7:6000::/19", false}, + + // Previously allocated network overlaps beginning of request + {[]string{"::/2"}, "::/0", false}, + {[]string{"::/3"}, "::/1", false}, + {[]string{"::/6"}, "::/5", false}, + + // Previously allocated network overlaps end of request + {[]string{"c000::/2"}, "8000::/1", false}, + {[]string{"7c00::/6"}, "::/1", false}, + {[]string{"cf80::/9"}, "c000::/4", false}, + + // Previously allocated network entirely contained within request + {[]string{"ff77:93f8::/29"}, "::/0", false}, + {[]string{"9287:2e20:5134:fab6:9061:a0c6:bfe3:9400/119"}, "8000::/1", false}, + {[]string{"3ea1:bfa9:8691:d1c6:8c46:519b:db6d:e700/120"}, "3000::/4", false}, + } + + for _, store := range []bool{false, true} { + for _, tc := range input { + a, err := getAllocator(store) + assert.NoError(t, err) + + // Set up some existing allocations. This should always succeed. + for _, env := range tc.environment { + _, _, _, err = a.RequestPool(localAddressSpace, env, "", nil, false) + assert.NoError(t, err) + } + + // Make the test allocation. + _, _, _, err = a.RequestPool(localAddressSpace, tc.subnet, "", nil, false) + if tc.ok { + assert.NoError(t, err) + } else { + assert.Error(t, err) + } + } + } +} + func TestRelease(t *testing.T) { var ( subnet = "192.168.0.0/23" diff --git a/ipam/structures.go b/ipam/structures.go index 455a16ca65..2e6d75eaa4 100644 --- a/ipam/structures.go +++ b/ipam/structures.go @@ -257,6 +257,7 @@ func (aSpace *addrSpace) New() datastore.KVObject { } } +// updatePoolDBOnAdd returns a closure which will add the subnet k to the address space when executed. func (aSpace *addrSpace) updatePoolDBOnAdd(k SubnetKey, nw *net.IPNet, ipr *AddressRange, pdf bool) (func() error, error) { aSpace.Lock() defer aSpace.Unlock() @@ -281,7 +282,7 @@ func (aSpace *addrSpace) updatePoolDBOnAdd(k SubnetKey, nw *net.IPNet, ipr *Addr return func() error { return aSpace.alloc.insertBitMask(k, nw) }, nil } - // This is a new non-master pool + // This is a new non-master pool (subPool) p := &PoolData{ ParentKey: SubnetKey{AddressSpace: k.AddressSpace, Subnet: k.Subnet}, Pool: nw, diff --git a/types/types.go b/types/types.go index 5968545ba5..b102ba4c39 100644 --- a/types/types.go +++ b/types/types.go @@ -332,6 +332,8 @@ func CompareIPNet(a, b *net.IPNet) bool { } // GetMinimalIP returns the address in its shortest form +// If ip contains an IPv4-mapped IPv6 address, the 4-octet form of the IPv4 address will be returned. +// Otherwise ip is returned unchanged. func GetMinimalIP(ip net.IP) net.IP { if ip != nil && ip.To4() != nil { return ip.To4()