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.
This commit is contained in:
Dan Williams
2016-09-02 15:55:53 -05:00
parent c5e39a87f7
commit dfc4f7cd2a
2 changed files with 77 additions and 1 deletions

View File

@ -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
}

View File

@ -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")