host-local: don't allocate past RangeEnd

When RangeEnd is given, a.end = RangeEnd+1.

If when getSearchRange() is called and lastReservedIP equals
RangeEnd, a.nextIP() only compares lastReservedIP (which in this
example is RangeEnd) against a.end (which in this example is
RangeEnd+1) and they clearly don't match, so a.nextIP() returns
start=RangeEnd+1 and end=RangeEnd.

Get() happily allocates RangeEnd+1 because it only compares 'cur'
to the end returned by getSearchRange(), not to a.end, and thus
allocates past RangeEnd.

Since a.end is inclusive (eg, host-local will allocate a.end) the
fix is to simply set a.end equal to RangeEnd.

diff --git a/plugins/ipam/host-local/allocator.go b/plugins/ipam/host-local/allocator.go
index 3610646..e0915ed 100644 (file)
--- a/plugins/ipam/host-local/allocator.go
+++ b/plugins/ipam/host-local/allocator.go
@@ -25,7 +25,9 @@ import (
 )
 
 type IPAllocator struct {
+       // start is inclusive and may be allocated
        start net.IP
+       // end is inclusive and may be allocated
        end   net.IP
        conf  *IPAMConfig
        store backend.Store
@@ -56,7 +58,7 @@ func NewIPAllocator(conf *IPAMConfig, store backend.Store) (*IPAllocator, error)
                        return nil, err
                }
                // RangeEnd is inclusive
-               end = ip.NextIP(conf.RangeEnd)
+               end = conf.RangeEnd
        }
        return &IPAllocator{start, end, conf, store}, nil
 }

diff --git a/plugins/ipam/host-local/allocator_test.go b/plugins/ipam/host-local/allocator_test.go
index 200618a..bb2b4f6 100644 (file)
--- a/plugins/ipam/host-local/allocator_test.go
+++ b/plugins/ipam/host-local/allocator_test.go
@@ -15,6 +15,7 @@
 package main
 
 import (
+       "fmt"
        "github.com/containernetworking/cni/pkg/types"
        fakestore "github.com/containernetworking/cni/plugins/ipam/host-local/backend/testing"
        . "github.com/onsi/ginkgo"
@@ -115,6 +116,79 @@ var _ = Describe("host-local ip allocator", func() {
                        }
                })
 
+               It("should allocate the last address if RangeEnd not given", func() {
+                       subnet, err := types.ParseCIDR("192.168.1.0/24")
+                       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(""))
+                       alloc, err := NewIPAllocator(&conf, store)
+                       Expect(err).ToNot(HaveOccurred())
+
+                       for i := 1; i < 255; i++ {
+                               res, err := alloc.Get("ID")
+                               Expect(err).ToNot(HaveOccurred())
+                               // i+1 because the gateway address is skipped
+                               s := fmt.Sprintf("192.168.1.%d/24", i+1)
+                               Expect(s).To(Equal(res.IP.String()))
+                       }
+
+                       _, err = alloc.Get("ID")
+                       Expect(err).To(HaveOccurred())
+               })
+
+               It("should allocate RangeStart first", func() {
+                       subnet, err := types.ParseCIDR("192.168.1.0/24")
+                       Expect(err).ToNot(HaveOccurred())
+
+                       conf := IPAMConfig{
+                               Name:       "test",
+                               Type:       "host-local",
+                               Subnet:     types.IPNet{IP: subnet.IP, Mask: subnet.Mask},
+                               RangeStart: net.ParseIP("192.168.1.10"),
+                       }
+                       store := fakestore.NewFakeStore(map[string]string{}, net.ParseIP(""))
+                       alloc, err := NewIPAllocator(&conf, store)
+                       Expect(err).ToNot(HaveOccurred())
+
+                       res, err := alloc.Get("ID")
+                       Expect(err).ToNot(HaveOccurred())
+                       Expect(res.IP.String()).To(Equal("192.168.1.10/24"))
+
+                       res, err = alloc.Get("ID")
+                       Expect(err).ToNot(HaveOccurred())
+                       Expect(res.IP.String()).To(Equal("192.168.1.11/24"))
+               })
+
+               It("should allocate RangeEnd but not past RangeEnd", func() {
+                       subnet, err := types.ParseCIDR("192.168.1.0/24")
+                       Expect(err).ToNot(HaveOccurred())
+
+                       conf := IPAMConfig{
+                               Name:     "test",
+                               Type:     "host-local",
+                               Subnet:   types.IPNet{IP: subnet.IP, Mask: subnet.Mask},
+                               RangeEnd: net.ParseIP("192.168.1.5"),
+                       }
+                       store := fakestore.NewFakeStore(map[string]string{}, net.ParseIP(""))
+                       alloc, err := NewIPAllocator(&conf, store)
+                       Expect(err).ToNot(HaveOccurred())
+
+                       for i := 1; i < 5; i++ {
+                               res, err := alloc.Get("ID")
+                               Expect(err).ToNot(HaveOccurred())
+                               // i+1 because the gateway address is skipped
+                               Expect(res.IP.String()).To(Equal(fmt.Sprintf("192.168.1.%d/24", i+1)))
+                       }
+
+                       _, err = alloc.Get("ID")
+                       Expect(err).To(HaveOccurred())
+               })
+
                Context("when requesting a specific IP", func() {
                        It("must allocate the requested IP", func() {
                                subnet, err := types.ParseCIDR("10.0.0.0/29")