host-local: don't allocate the broadcast address or allow invalid networks

There aren't any IPs to allocate in /32 or /31 networks, so don't allow them.

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
 }
 

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",
+					expectResult: "10.0.0.6",
-					lastIP:       "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",
+					expectResult: "10.0.0.6",
-					lastIP:       "10.0.0.4",
+					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())
+		})
+	})
 })