diff --git a/plugins/ipam/host-local/allocator.go b/plugins/ipam/host-local/allocator.go index ad5f0d20..a250ac01 100644 --- a/plugins/ipam/host-local/allocator.go +++ b/plugins/ipam/host-local/allocator.go @@ -34,6 +34,13 @@ type IPAllocator struct { } func NewIPAllocator(conf *IPAMConfig, store backend.Store) (*IPAllocator, error) { + // Can't create an allocator for a network with no addresses, eg + // a /32 or /31 + ones, masklen := conf.Subnet.Mask.Size() + if ones > masklen-2 { + return nil, fmt.Errorf("Network %v too small to allocate from", conf.Subnet) + } + var ( start net.IP end net.IP @@ -195,6 +202,8 @@ func (a *IPAllocator) Release(id string) error { return a.store.ReleaseByID(id) } +// Return the start and end IP addresses of a given subnet, excluding +// the broadcast address (eg, 192.168.1.255) func networkRange(ipnet *net.IPNet) (net.IP, net.IP, error) { if ipnet.IP == nil { return nil, nil, fmt.Errorf("missing field %q in IPAM configuration", "subnet") @@ -212,6 +221,12 @@ func networkRange(ipnet *net.IPNet) (net.IP, net.IP, error) { for i := 0; i < len(ip); i++ { end = append(end, ip[i]|^ipnet.Mask[i]) } + + // Exclude the broadcast address for IPv4 + if ip.To4() != nil { + end[3]-- + } + return ipnet.IP, end, nil } diff --git a/plugins/ipam/host-local/allocator_test.go b/plugins/ipam/host-local/allocator_test.go index 9b7d19bf..2bcd2776 100644 --- a/plugins/ipam/host-local/allocator_test.go +++ b/plugins/ipam/host-local/allocator_test.go @@ -77,26 +77,26 @@ var _ = Describe("host-local ip allocator", func() { { subnet: "10.0.0.0/29", ipmap: map[string]string{}, - expectResult: "10.0.0.7", - lastIP: "10.0.0.6", + expectResult: "10.0.0.6", + lastIP: "10.0.0.5", }, { subnet: "10.0.0.0/29", ipmap: map[string]string{ + "10.0.0.4": "id", "10.0.0.5": "id", - "10.0.0.6": "id", }, - expectResult: "10.0.0.7", - lastIP: "10.0.0.4", + expectResult: "10.0.0.6", + lastIP: "10.0.0.3", }, // round robin to the beginning { subnet: "10.0.0.0/29", ipmap: map[string]string{ - "10.0.0.7": "id", + "10.0.0.6": "id", }, expectResult: "10.0.0.2", - lastIP: "10.0.0.6", + lastIP: "10.0.0.5", }, // lastIP is out of range { @@ -116,7 +116,7 @@ var _ = Describe("host-local ip allocator", func() { } }) - It("should allocate the last address if RangeEnd not given", func() { + It("should not allocate the broadcast address", func() { subnet, err := types.ParseCIDR("192.168.1.0/24") Expect(err).ToNot(HaveOccurred()) @@ -129,7 +129,7 @@ var _ = Describe("host-local ip allocator", func() { alloc, err := NewIPAllocator(&conf, store) Expect(err).ToNot(HaveOccurred()) - for i := 1; i < 255; i++ { + for i := 1; i < 254; i++ { res, err := alloc.Get("ID") Expect(err).ToNot(HaveOccurred()) // i+1 because the gateway address is skipped @@ -318,4 +318,20 @@ var _ = Describe("host-local ip allocator", func() { } }) }) + + Context("when given an invalid subnet", func() { + It("returns a meaningful error", func() { + subnet, err := types.ParseCIDR("192.168.1.0/31") + Expect(err).ToNot(HaveOccurred()) + + conf := IPAMConfig{ + Name: "test", + Type: "host-local", + Subnet: types.IPNet{IP: subnet.IP, Mask: subnet.Mask}, + } + store := fakestore.NewFakeStore(map[string]string{}, net.ParseIP("")) + _, err = NewIPAllocator(&conf, store) + Expect(err).To(HaveOccurred()) + }) + }) })