diff --git a/internal/cni/ipam/ipam.go b/internal/cni/ipam/ipam.go index da8e63c..b4a90ea 100644 --- a/internal/cni/ipam/ipam.go +++ b/internal/cni/ipam/ipam.go @@ -32,6 +32,7 @@ type PoolAllocator struct { subnetLen int // prefix length per allocation (e.g. 96) gateway net.IP // gateway IP address poolIP net.IP // immutable copy of pool.IP for boundary checks + reserved string // subnet CIDR string containing the gateway; never allocated allocations sync.Map // allocated subnet CIDR string -> struct{}{} mu sync.Mutex // serializes Allocate calls } @@ -42,7 +43,11 @@ type PoolAllocator struct { // subnet when subnetLen is the default /96, though any pool length <= // subnetLen is accepted). If gateway is empty, the first address in the pool // (host bits = 1) is used as the gateway. If subnetLen is 0, DefaultSubnetLen -// (96) is used. +// (96) is used. The subnet containing the gateway is reserved and never +// handed out by Allocate — otherwise the endpoint owning that subnet could +// self-assign the gateway's own address to one of its secondary/pod +// addresses, colliding with the address every other endpoint in the pool +// routes its default route through. func NewPoolAllocator(poolCIDR, gateway string, subnetLen int) (*PoolAllocator, error) { _, pool, err := net.ParseCIDR(poolCIDR) if err != nil { @@ -86,12 +91,19 @@ func NewPoolAllocator(poolCIDR, gateway string, subnetLen int) (*PoolAllocator, pa.gateway = gw } + reservedSubnet := &net.IPNet{ + IP: pa.gateway.Mask(net.CIDRMask(subnetLen, ipv6Bits)), + Mask: net.CIDRMask(subnetLen, ipv6Bits), + } + pa.reserved = reservedSubnet.String() + return pa, nil } // Allocate assigns the next available IPv6 subnet from the pool for the -// given container ID. Returns the allocated subnet CIDR or an error if the -// pool is exhausted. Thread-safe. +// given container ID, skipping the subnet that contains the pool's gateway +// address. Returns the allocated subnet CIDR or an error if the pool is +// exhausted. Thread-safe. func (a *PoolAllocator) Allocate(_ string) (*net.IPNet, error) { a.mu.Lock() defer a.mu.Unlock() @@ -116,6 +128,11 @@ func (a *PoolAllocator) Allocate(_ string) (*net.IPNet, error) { copy(subnet.IP, subnetStart) subnetStr := subnet.String() + // Skip the subnet reserved for the gateway. + if subnetStr == a.reserved { + continue + } + // Skip already allocated. if _, ok := used[subnetStr]; ok { continue diff --git a/internal/cni/ipam/ipam_test.go b/internal/cni/ipam/ipam_test.go index 34c0741..3c23bfe 100644 --- a/internal/cni/ipam/ipam_test.go +++ b/internal/cni/ipam/ipam_test.go @@ -6,16 +6,23 @@ package ipam import ( "net" + "strconv" "testing" ) const ( - testPoolCIDR = "fd00:10:ff01::/64" - testPoolGw = "fd00:10:ff01::1" - testSubnetLen = 96 - testAllocatedSub = "fd00:10:ff01::/96" - // nextSubnet is the second /96 subnet from a /64 pool, used across tests. - nextSubnet = "fd00:10:ff01::100:0/96" + testPoolCIDR = "fd00:10:ff01::/64" + testPoolGw = "fd00:10:ff01::1" + testSubnetLen = 96 + // testReservedSub is the /96 subnet containing testPoolGw. It is never + // handed out by Allocate — see TestPoolAllocatorReservesGatewaySubnet. + testReservedSub = "fd00:10:ff01::/96" + // testAllocatedSub is the first /96 subnet Allocate actually hands out + // from testPoolCIDR, once testReservedSub is skipped. + testAllocatedSub = "fd00:10:ff01::100:0/96" + // nextSubnet is the second /96 subnet Allocate hands out from + // testPoolCIDR, used across tests. + nextSubnet = "fd00:10:ff01::200:0/96" // testInvalidCIDR and testInvalidGateway are shared across this // package's test files to avoid duplicate string literals. @@ -159,7 +166,7 @@ func TestPoolAllocatorSkipsAllocatedSubnets(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - // First container gets the first /80. + // First container gets the first allocatable /96 (the gateway's /96 is reserved). subnet1, err := pa.Allocate("container-x") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -168,7 +175,7 @@ func TestPoolAllocatorSkipsAllocatedSubnets(t *testing.T) { t.Errorf("first alloc = %q, want %q", subnet1, testAllocatedSub) } - // Second container should get the next /80, not the first (already taken). + // Second container should get the next /96, not the first (already taken). subnet2, err := pa.Allocate("container-y") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -179,6 +186,35 @@ func TestPoolAllocatorSkipsAllocatedSubnets(t *testing.T) { } } +func TestPoolAllocatorReservesGatewaySubnet(t *testing.T) { + pa, err := NewPoolAllocator(testPoolCIDR, testPoolGw, testSubnetLen) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // The very first /96 in the pool is the gateway's own subnet + // (testReservedSub) — the case that matters, since Allocate walks the + // pool from its start. Confirm it's skipped in favor of the next /96, + // and re-confirm across a further run of allocations: an endpoint that + // owned the gateway's /96 could self-assign the gateway's own address + // to one of its secondary/pod addresses, colliding with the address + // every other endpoint in the pool routes its default route through. + const numAllocations = 1000 + for i := range numAllocations { + subnet, err := pa.Allocate(strconv.Itoa(i)) + if err != nil { + t.Fatalf("unexpected error on allocation %d: %v", i, err) + } + if subnet.String() == testReservedSub { + t.Fatalf("Allocate() returned reserved gateway subnet %q on allocation %d", testReservedSub, i) + } + } + + if pa.IsAllocated(testReservedSub) { + t.Error("IsAllocated() = true for the reserved gateway subnet, want false") + } +} + func TestPoolAllocatorDeallocate(t *testing.T) { pa, err := NewPoolAllocator(testPoolCIDR, testPoolGw, testSubnetLen) if err != nil {