diff --git a/plugins/ipam/host-local/allocator.go b/plugins/ipam/host-local/allocator.go index 3610646b..a250ac01 100644 --- a/plugins/ipam/host-local/allocator.go +++ b/plugins/ipam/host-local/allocator.go @@ -25,13 +25,22 @@ 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 } 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 @@ -46,25 +55,76 @@ func NewIPAllocator(conf *IPAMConfig, store backend.Store) (*IPAllocator, error) start = ip.NextIP(start) if conf.RangeStart != nil { - if err := validateRangeIP(conf.RangeStart, (*net.IPNet)(&conf.Subnet)); err != nil { + if err := validateRangeIP(conf.RangeStart, (*net.IPNet)(&conf.Subnet), nil, nil); err != nil { return nil, err } start = conf.RangeStart } if conf.RangeEnd != nil { - if err := validateRangeIP(conf.RangeEnd, (*net.IPNet)(&conf.Subnet)); err != nil { + if err := validateRangeIP(conf.RangeEnd, (*net.IPNet)(&conf.Subnet), start, nil); err != nil { return nil, err } - // RangeEnd is inclusive - end = ip.NextIP(conf.RangeEnd) + end = conf.RangeEnd } return &IPAllocator{start, end, conf, store}, nil } -func validateRangeIP(ip net.IP, ipnet *net.IPNet) error { +func canonicalizeIP(ip net.IP) (net.IP, error) { + if ip.To4() != nil { + return ip.To4(), nil + } else if ip.To16() != nil { + return ip.To16(), nil + } + return nil, fmt.Errorf("IP %s not v4 nor v6", ip) +} + +// Ensures @ip is within @ipnet, and (if given) inclusive of @start and @end +func validateRangeIP(ip net.IP, ipnet *net.IPNet, start net.IP, end net.IP) error { + var err error + + // Make sure we can compare IPv4 addresses directly + ip, err = canonicalizeIP(ip) + if err != nil { + return err + } + if !ipnet.Contains(ip) { return fmt.Errorf("%s not in network: %s", ip, ipnet) } + + if start != nil { + start, err = canonicalizeIP(start) + if err != nil { + return err + } + if len(ip) != len(start) { + return fmt.Errorf("%s %d not same size IP address as start %s %d", ip, len(ip), start, len(start)) + } + for i := 0; i < len(ip); i++ { + if ip[i] > start[i] { + break + } else if ip[i] < start[i] { + return fmt.Errorf("%s outside of network %s with start %s", ip, ipnet, start) + } + } + } + + if end != nil { + end, err = canonicalizeIP(end) + if err != nil { + return err + } + if len(ip) != len(end) { + return fmt.Errorf("%s %d not same size IP address as end %s %d", ip, len(ip), end, len(end)) + } + for i := 0; i < len(ip); i++ { + if ip[i] < end[i] { + break + } else if ip[i] > end[i] { + return fmt.Errorf("%s outside of network %s with end %s", ip, ipnet, end) + } + } + } return nil } @@ -92,7 +152,7 @@ func (a *IPAllocator) Get(id string) (*types.IPConfig, error) { IP: a.conf.Subnet.IP, Mask: a.conf.Subnet.Mask, } - err := validateRangeIP(requestedIP, &subnet) + err := validateRangeIP(requestedIP, &subnet, a.start, a.end) if err != nil { return nil, err } @@ -142,16 +202,15 @@ 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") } - ip := ipnet.IP.To4() - if ip == nil { - ip = ipnet.IP.To16() - if ip == nil { - return nil, nil, fmt.Errorf("IP not v4 nor v6") - } + ip, err := canonicalizeIP(ipnet.IP) + if err != nil { + return nil, nil, fmt.Errorf("IP not v4 nor v6") } if len(ip) != len(ipnet.Mask) { @@ -162,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 } @@ -186,7 +251,7 @@ func (a *IPAllocator) getSearchRange() (net.IP, net.IP) { IP: a.conf.Subnet.IP, Mask: a.conf.Subnet.Mask, } - err := validateRangeIP(lastReservedIP, &subnet) + err := validateRangeIP(lastReservedIP, &subnet, a.start, a.end) if err == nil { startFromLastReservedIP = true } diff --git a/plugins/ipam/host-local/allocator_test.go b/plugins/ipam/host-local/allocator_test.go index 200618a9..2bcd2776 100644 --- 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" @@ -76,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 { @@ -115,6 +116,79 @@ var _ = Describe("host-local ip allocator", func() { } }) + It("should not allocate the broadcast address", 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 < 254; 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") @@ -133,6 +207,86 @@ var _ = Describe("host-local ip allocator", func() { Expect(err).ToNot(HaveOccurred()) Expect(res.IP.IP.String()).To(Equal(requestedIP.String())) }) + + It("must return an error when the requested IP is after RangeEnd", func() { + subnet, err := types.ParseCIDR("192.168.1.0/24") + Expect(err).ToNot(HaveOccurred()) + ipmap := map[string]string{} + conf := IPAMConfig{ + Name: "test", + Type: "host-local", + Subnet: types.IPNet{IP: subnet.IP, Mask: subnet.Mask}, + Args: &IPAMArgs{IP: net.ParseIP("192.168.1.50")}, + RangeEnd: net.ParseIP("192.168.1.20"), + } + store := fakestore.NewFakeStore(ipmap, nil) + alloc, _ := NewIPAllocator(&conf, store) + _, err = alloc.Get("ID") + Expect(err).To(HaveOccurred()) + }) + + It("must return an error when the requested IP is before RangeStart", func() { + subnet, err := types.ParseCIDR("192.168.1.0/24") + Expect(err).ToNot(HaveOccurred()) + ipmap := map[string]string{} + conf := IPAMConfig{ + Name: "test", + Type: "host-local", + Subnet: types.IPNet{IP: subnet.IP, Mask: subnet.Mask}, + Args: &IPAMArgs{IP: net.ParseIP("192.168.1.3")}, + RangeStart: net.ParseIP("192.168.1.10"), + } + store := fakestore.NewFakeStore(ipmap, nil) + alloc, _ := NewIPAllocator(&conf, store) + _, err = alloc.Get("ID") + Expect(err).To(HaveOccurred()) + }) + }) + + It("RangeStart must be in the given subnet", 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("10.0.0.1"), + } + store := fakestore.NewFakeStore(map[string]string{}, net.ParseIP("")) + _, err = NewIPAllocator(&conf, store) + Expect(err).To(HaveOccurred()) + }) + + It("RangeEnd must be in the given subnet", 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("10.0.0.1"), + } + store := fakestore.NewFakeStore(map[string]string{}, net.ParseIP("")) + _, err = NewIPAllocator(&conf, store) + Expect(err).To(HaveOccurred()) + }) + + It("RangeEnd must be after RangeStart in the given subnet", 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"), + RangeEnd: net.ParseIP("192.168.1.3"), + } + store := fakestore.NewFakeStore(map[string]string{}, net.ParseIP("")) + _, err = NewIPAllocator(&conf, store) + Expect(err).To(HaveOccurred()) }) }) @@ -164,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()) + }) + }) })