Fix IPv6 preAllocation canonicalization in IPAM

- Compare IP addresses by parsed value instead of raw strings to handle
non-canonical IPv6 formats (::0005 vs ::5).
- Also adding regression tests.

Signed-off-by: Kashif Khan <[email protected]>

Author: kashifest

ipam/ippool_manager.go

@@ -18,6 +18,7 @@ package ipam
 
 import (
 	"context"
+	"net"
 	"reflect"
 	"strings"
 
@@ -86,6 +87,27 @@ func (m *IPPoolManager) UnsetFinalizer() {
 	)
 }
 
+// ipEqual compares two IP addresses by their parsed values to handle non-canonical
+// IPv6 formats (e.g., ::0005 vs ::5). Returns false if either address fails to parse,
+// logging a warning to aid in diagnosing configuration issues.
+func (m *IPPoolManager) ipEqual(a, b ipamv1.IPAddressStr) bool {
+	ipA := net.ParseIP(string(a))
+	ipB := net.ParseIP(string(b))
+	if ipA == nil {
+		if a != "" {
+			m.Log.Info("Failed to parse IP address", "address", a)
+		}
+		return false
+	}
+	if ipB == nil {
+		if b != "" {
+			m.Log.Info("Failed to parse IP address", "address", b)
+		}
+		return false
+	}
+	return ipA.Equal(ipB)
+}
+
 func (m *IPPoolManager) SetClusterOwnerRef(cluster *clusterv1beta1.Cluster) error {
 	if cluster == nil {
 		return errors.New("Missing cluster")
@@ -432,7 +454,7 @@ func (m *IPPoolManager) allocateAddress(addressClaim *ipamv1.IPClaim,
 	isRequestedIPAllocated := false
 
 	// Conflict-case, claim is preAllocated but has requested different IP
-	if requestedIP != "" && ipPreAllocated && requestedIP != preAllocatedAddress {
+	if requestedIP != "" && ipPreAllocated && !m.ipEqual(requestedIP, preAllocatedAddress) {
 		addressClaim.Status.ErrorMessage = ptr.To("PreAllocation and requested ip address are conflicting")
 		return "", 0, nil, []ipamv1.IPAddressStr{}, errors.New("PreAllocation and requested ip address are conflicting")
 	}
@@ -449,15 +471,15 @@ func (m *IPPoolManager) allocateAddress(addressClaim *ipamv1.IPClaim,
 			}
 			index++
 			// Check if requestedIP is present and matches the current address
-			if requestedIP != "" && allocatedAddress != requestedIP {
+			if requestedIP != "" && !m.ipEqual(allocatedAddress, requestedIP) {
 				continue
 			}
-			if requestedIP != "" && allocatedAddress == requestedIP {
+			if requestedIP != "" && m.ipEqual(allocatedAddress, requestedIP) {
 				isRequestedIPAllocated = true
 			}
 			// We have a pre-allocated ip, we just need to ensure that it matches the current address
 			// if it does not, continue and try the next address
-			if ipPreAllocated && allocatedAddress != preAllocatedAddress {
+			if ipPreAllocated && !m.ipEqual(allocatedAddress, preAllocatedAddress) {
 				continue
 			}
 			// Here the two addresses match, so we continue with that one
@@ -523,7 +545,7 @@ func (m *IPPoolManager) capiAllocateAddress(addressClaim *capipamv1beta1.IPAddre
 	isRequestedIPAllocated := false
 
 	// Conflict-case, claim is preAllocated but has requested different IP
-	if requestedIP != "" && ipPreAllocated && requestedIP != preAllocatedAddress {
+	if requestedIP != "" && ipPreAllocated && !m.ipEqual(requestedIP, preAllocatedAddress) {
 		conditions := clusterv1beta1.Conditions{}
 		conditions = append(conditions, clusterv1beta1.Condition{
 			Type:               "ErrorMessage",
@@ -549,15 +571,15 @@ func (m *IPPoolManager) capiAllocateAddress(addressClaim *capipamv1beta1.IPAddre
 			}
 			index++
 			// Check if requestedIP is present and matches the current address
-			if requestedIP != "" && allocatedAddress != requestedIP {
+			if requestedIP != "" && !m.ipEqual(allocatedAddress, requestedIP) {
 				continue
 			}
-			if requestedIP != "" && allocatedAddress == requestedIP {
+			if requestedIP != "" && m.ipEqual(allocatedAddress, requestedIP) {
 				isRequestedIPAllocated = true
 			}
 			// We have a pre-allocated ip, we just need to ensure that it matches the current address
 			// if it does not, continue and try the next address
-			if ipPreAllocated && allocatedAddress != preAllocatedAddress {
+			if ipPreAllocated && !m.ipEqual(allocatedAddress, preAllocatedAddress) {
 				continue
 			}
 			// Here the two addresses match, so we continue with that one

ipam/ippool_manager_test.go

@@ -2011,6 +2011,37 @@ var _ = Describe("IPPool manager", func() {
 			},
 			expectedPrefix: 24,
 		}),
+		Entry("One pool, IPv6 pre-allocated (non-canonical)", testCaseAllocateAddress{
+			ipPool: &ipamv1.IPPool{
+				Spec: ipamv1.IPPoolSpec{
+					Pools: []ipamv1.Pool{
+						{
+							Start: (*ipamv1.IPAddressStr)(ptr.To("2001:db8::1")),
+							End:   (*ipamv1.IPAddressStr)(ptr.To("2001:db8::a")),
+						},
+					},
+					PreAllocations: map[string]ipamv1.IPAddressStr{
+						"TestRef": ipamv1.IPAddressStr("2001:db8::0005"),
+					},
+					Prefix:  64,
+					Gateway: (*ipamv1.IPAddressStr)(ptr.To("2001:db8::ffff")),
+					DNSServers: []ipamv1.IPAddressStr{
+						ipamv1.IPAddressStr("2001:4860:4860::8888"),
+					},
+				},
+			},
+			ipClaim: &ipamv1.IPClaim{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "TestRef",
+				},
+			},
+			expectedAddress: ipamv1.IPAddressStr("2001:db8::5"),
+			expectedGateway: (*ipamv1.IPAddressStr)(ptr.To("2001:db8::ffff")),
+			expectedDNSServers: []ipamv1.IPAddressStr{
+				ipamv1.IPAddressStr("2001:4860:4860::8888"),
+			},
+			expectedPrefix: 64,
+		}),
 		Entry("One pool, pre-allocated, with overrides", testCaseAllocateAddress{
 			ipPool: &ipamv1.IPPool{
 				Spec: ipamv1.IPPoolSpec{
@@ -2459,6 +2490,31 @@ var _ = Describe("IPPool manager", func() {
 			expectedGateway: (*ipamv1.IPAddressStr)(ptr.To("192.168.0.1")),
 			expectedPrefix:  24,
 		}),
+		Entry("One pool, IPv6 pre-allocated (non-canonical)", testCapiCaseAllocateAddress{
+			ipPool: &ipamv1.IPPool{
+				Spec: ipamv1.IPPoolSpec{
+					Pools: []ipamv1.Pool{
+						{
+							Start: (*ipamv1.IPAddressStr)(ptr.To("2001:db8::1")),
+							End:   (*ipamv1.IPAddressStr)(ptr.To("2001:db8::a")),
+						},
+					},
+					PreAllocations: map[string]ipamv1.IPAddressStr{
+						"TestRef": ipamv1.IPAddressStr("2001:db8::0005"),
+					},
+					Prefix:  64,
+					Gateway: (*ipamv1.IPAddressStr)(ptr.To("2001:db8::ffff")),
+				},
+			},
+			ipAddressClaim: &capipamv1beta1.IPAddressClaim{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "TestRef",
+				},
+			},
+			expectedAddress: ipamv1.IPAddressStr("2001:db8::5"),
+			expectedGateway: (*ipamv1.IPAddressStr)(ptr.To("2001:db8::ffff")),
+			expectedPrefix:  64,
+		}),
 		Entry("One pool, pre-allocated, with overrides", testCapiCaseAllocateAddress{
 			ipPool: &ipamv1.IPPool{
 				Spec: ipamv1.IPPoolSpec{