From 6512de76ce38a2aee6228e5b64a2119e19fef820 Mon Sep 17 00:00:00 2001
From: Dan Winship <danwinship@redhat.com>
Date: Tue, 11 Feb 2025 17:43:17 -0500
Subject: [PATCH] Make EndpointSlice mirroring controller always canonicalize
 the IPs it writes out

(Also rearrange some code to avoid parsing the IP twice.)
---
 .../reconciler_helpers.go                     |  3 +-
 .../endpointslicemirroring/reconciler_test.go |  4 +--
 .../endpointslicemirroring/utils.go           | 30 +++++++++----------
 .../endpointslicemirroring/utils_test.go      |  3 +-
 4 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/pkg/controller/endpointslicemirroring/reconciler_helpers.go b/pkg/controller/endpointslicemirroring/reconciler_helpers.go
index fec80578bbe..774db1f0cf2 100644
--- a/pkg/controller/endpointslicemirroring/reconciler_helpers.go
+++ b/pkg/controller/endpointslicemirroring/reconciler_helpers.go
@@ -87,8 +87,7 @@ func (d *desiredCalc) initPorts(subsetPorts []v1.EndpointPort) multiAddrTypePort
 // addAddress adds an EndpointAddress to the desired state if it is valid. It
 // returns false if the address was invalid.
 func (d *desiredCalc) addAddress(address v1.EndpointAddress, multiKey multiAddrTypePortMapKey, ready bool) bool {
-	endpoint := addressToEndpoint(address, ready)
-	addrType := getAddressType(address.IP)
+	endpoint, addrType := addressToEndpoint(address, ready)
 	if addrType == nil {
 		return false
 	}
diff --git a/pkg/controller/endpointslicemirroring/reconciler_test.go b/pkg/controller/endpointslicemirroring/reconciler_test.go
index 3b0387a3033..5c9837d41f0 100644
--- a/pkg/controller/endpointslicemirroring/reconciler_test.go
+++ b/pkg/controller/endpointslicemirroring/reconciler_test.go
@@ -1245,7 +1245,7 @@ func expectMatchingAddresses(t *testing.T, epSubset corev1.EndpointSubset, esEnd
 	expectedEndpoints := map[string]addressInfo{}
 
 	for _, address := range epSubset.Addresses {
-		at := getAddressType(address.IP)
+		_, at := addressToEndpoint(address, true)
 		if at != nil && *at == addrType && len(expectedEndpoints) < maxEndpointsPerSubset {
 			expectedEndpoints[address.IP] = addressInfo{
 				ready:     true,
@@ -1255,7 +1255,7 @@ func expectMatchingAddresses(t *testing.T, epSubset corev1.EndpointSubset, esEnd
 	}
 
 	for _, address := range epSubset.NotReadyAddresses {
-		at := getAddressType(address.IP)
+		_, at := addressToEndpoint(address, true)
 		if at != nil && *at == addrType && len(expectedEndpoints) < maxEndpointsPerSubset {
 			expectedEndpoints[address.IP] = addressInfo{
 				ready:     false,
diff --git a/pkg/controller/endpointslicemirroring/utils.go b/pkg/controller/endpointslicemirroring/utils.go
index 9c5d3ee847b..ebcd160cc7f 100644
--- a/pkg/controller/endpointslicemirroring/utils.go
+++ b/pkg/controller/endpointslicemirroring/utils.go
@@ -49,18 +49,6 @@ func (pk addrTypePortMapKey) addressType() discovery.AddressType {
 	return discovery.AddressTypeIPv4
 }
 
-func getAddressType(address string) *discovery.AddressType {
-	ip := netutils.ParseIPSloppy(address)
-	if ip == nil {
-		return nil
-	}
-	addressType := discovery.AddressTypeIPv4
-	if ip.To4() == nil {
-		addressType = discovery.AddressTypeIPv6
-	}
-	return &addressType
-}
-
 // newEndpointSlice returns an EndpointSlice generated from an Endpoints
 // resource, ports, and address type.
 func newEndpointSlice(endpoints *corev1.Endpoints, ports []discovery.EndpointPort, addrType discovery.AddressType, sliceName string) *discovery.EndpointSlice {
@@ -115,10 +103,20 @@ func getEndpointSlicePrefix(serviceName string) string {
 }
 
 // addressToEndpoint converts an address from an Endpoints resource to an
-// EndpointSlice endpoint.
-func addressToEndpoint(address corev1.EndpointAddress, ready bool) *discovery.Endpoint {
+// EndpointSlice endpoint and AddressType.
+func addressToEndpoint(address corev1.EndpointAddress, ready bool) (*discovery.Endpoint, *discovery.AddressType) {
+	ip := netutils.ParseIPSloppy(address.IP)
+	if ip == nil {
+		return nil, nil
+	}
+	addressType := discovery.AddressTypeIPv4
+	if ip.To4() == nil {
+		addressType = discovery.AddressTypeIPv6
+	}
+
 	endpoint := &discovery.Endpoint{
-		Addresses: []string{address.IP},
+		// We parse and restringify the Endpoints IP in case it is in an irregular format.
+		Addresses: []string{ip.String()},
 		Conditions: discovery.EndpointConditions{
 			Ready: &ready,
 		},
@@ -132,7 +130,7 @@ func addressToEndpoint(address corev1.EndpointAddress, ready bool) *discovery.En
 		endpoint.Hostname = &address.Hostname
 	}
 
-	return endpoint
+	return endpoint, &addressType
 }
 
 // epPortsToEpsPorts converts ports from an Endpoints resource to ports for an
diff --git a/pkg/controller/endpointslicemirroring/utils_test.go b/pkg/controller/endpointslicemirroring/utils_test.go
index 98b101c5fe4..4daa8632e49 100644
--- a/pkg/controller/endpointslicemirroring/utils_test.go
+++ b/pkg/controller/endpointslicemirroring/utils_test.go
@@ -217,8 +217,9 @@ func TestAddressToEndpoint(t *testing.T) {
 		NodeName: pointer.String("node-abc"),
 	}
 
-	ep := addressToEndpoint(epAddress, ready)
+	ep, addrType := addressToEndpoint(epAddress, ready)
 	assert.EqualValues(t, expectedEndpoint, *ep)
+	assert.EqualValues(t, discovery.AddressTypeIPv4, *addrType)
 }
 
 // Test helpers