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.

diff --git a/plugins/ipam/host-local/allocator.go b/plugins/ipam/host-local/allocator.go
index ad5f0d2..a250ac0 100644 (file)
--- 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 9b7d19b..2bcd277 100644 (file)
--- 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())
+               })
+       })
 })