OCPBUGS-79354: Canonicalize all IP Address comparisons

1. pkg/operator/ceohelpers/common.go

  - Added CanonicalizeIP() function (lines 37-46): Converts IP addresses to canonical form using net.ParseIP().String()
    - Normalizes IPv6 addresses (e.g., 2001:0db8::1 → 2001:db8::1)
    - Auto-converts IPv4-mapped IPv6 to IPv4 (e.g., ::ffff:192.0.2.1 → 192.0.2.1)
    - Returns original string if invalid IP
  - Updated 4 functions to use canonicalization:
    - MemberToNodeInternalIP() - canonicalizes IP extracted from etcd member
    - IndexMachinesByNodeInternalIP() - canonicalizes map keys
    - FindMachineByNodeInternalIP() - canonicalizes both sides of comparison
    - VotingMemberIPListSet() - canonicalizes IPs before inserting into set
    - MemberIPSetFromConfigMap() - canonicalizes IPs from configmap

  2. pkg/operator/clustermemberremovalcontroller/clustermemberremovalcontroller.go

  - Updated hasInternalIP() function (line 528): Canonicalizes both machine and member IPs before comparison

  3. pkg/operator/machinedeletionhooks/machinedeletionhooks.go

  - Updated line 149: Canonicalizes machine address before set lookup

  4. pkg/operator/ceohelpers/common_test.go

  - Added comprehensive TestCanonicalizeIP() with 12 test cases covering:
    - IPv4 addresses
    - IPv6 compressed and full forms
    - IPv6 with leading zeros and mixed case
    - IPv4-mapped IPv6 conversion
    - Loopback addresses
    - Invalid IP handling

  Impact

  This fixes potential bugs where IPv6 addresses in different representations (e.g., 2001:db8::1 vs 2001:0db8:0000:0000:0000:0000:0000:0001) would fail to match, causing:
  - Machine lookups to fail
  - Member removal decisions to be incorrect
  - Deletion hooks to be misapplied

Note: This PR is opened with claude based on findings by the openshift-installer team. Review of claude's work and instructions given by @barbacbd.

Author: barbacbd

pkg/cmd/render/bootstrap_ip.go

@@ -139,8 +139,10 @@ func ContainedByCIDR(cidr string) AddressFilter {
 
 func AddressNotIn(ips ...string) AddressFilter {
 	return func(addr netlink.Addr) bool {
+		canonicalAddr := addr.IP.String()
 		for _, ip := range ips {
-			if addr.IP.String() == ip {
+			parsedIP := net.ParseIP(ip)
+			if parsedIP != nil && canonicalAddr == parsedIP.String() {
 				return false
 			}
 		}

pkg/operator/ceohelpers/common.go

@@ -33,6 +33,20 @@ const MachineDeletionHookOwner = "clusteroperator/etcd"
 
 var RevisionRolloutInProgressErr = fmt.Errorf("revision rollout in progress, can't establish current revision")
 
+// CanonicalizeIP returns the canonical string representation of an IP address.
+// This ensures consistent IP comparisons by:
+// - Normalizing IPv6 addresses to their compressed form (e.g., "2001:0db8::1" → "2001:db8::1")
+// - Converting IPv4-mapped IPv6 addresses to IPv4 (e.g., "::ffff:192.0.2.1" → "192.0.2.1")
+// - Preserving IPv4 addresses as-is
+// If the input is not a valid IP address, it returns the original string unchanged.
+func CanonicalizeIP(ip string) string {
+	parsed := net.ParseIP(ip)
+	if parsed == nil {
+		return ip // Return original if invalid
+	}
+	return parsed.String() // Returns canonical representation
+}
+
 // ReadDesiredControlPlaneReplicasCount reads the current Control Plane replica count
 func ReadDesiredControlPlaneReplicasCount(operatorClient v1helpers.StaticPodOperatorClient) (int, error) {
 	operatorSpec, _, _, err := operatorClient.GetStaticPodOperatorState()
@@ -79,7 +93,7 @@ func MemberToNodeInternalIP(member *etcdserverpb.Member) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	return host, nil
+	return CanonicalizeIP(host), nil
 }
 
 // FilterMachinesWithMachineDeletionHook a convenience function for filtering only machines with the machine deletion hook present
@@ -133,7 +147,7 @@ func IndexMachinesByNodeInternalIP(machines []*machinev1beta1.Machine) map[strin
 	for _, machine := range machines {
 		for _, addr := range machine.Status.Addresses {
 			if addr.Type == corev1.NodeInternalIP {
-				index[addr.Address] = machine
+				index[CanonicalizeIP(addr.Address)] = machine
 				// do not stop on first match
 				// machines can have multiple network interfaces
 			}
@@ -164,10 +178,11 @@ func FindMachineByNodeInternalIP(nodeInternalIP string, machineSelector labels.S
 		return nil, err
 	}
 
+	canonicalNodeIP := CanonicalizeIP(nodeInternalIP)
 	for _, machine := range machines {
 		for _, addr := range machine.Status.Addresses {
 			if addr.Type == corev1.NodeInternalIP {
-				if addr.Address == nodeInternalIP {
+				if CanonicalizeIP(addr.Address) == canonicalNodeIP {
 					return machine, nil
 				}
 			}
@@ -196,7 +211,7 @@ func VotingMemberIPListSet(ctx context.Context, cli etcdcli.EtcdClient) (sets.St
 		if err != nil {
 			return sets.NewString(), err
 		}
-		currentVotingMemberIPListSet.Insert(ip)
+		currentVotingMemberIPListSet.Insert(CanonicalizeIP(ip))
 	}
 
 	return currentVotingMemberIPListSet, nil
@@ -207,7 +222,7 @@ func VotingMemberIPListSet(ctx context.Context, cli etcdcli.EtcdClient) (sets.St
 func MemberIPSetFromConfigMap(cm *corev1.ConfigMap) sets.String {
 	memberIPs := sets.NewString()
 	for _, ip := range cm.Data {
-		memberIPs.Insert(ip)
+		memberIPs.Insert(CanonicalizeIP(ip))
 	}
 	return memberIPs
 }

pkg/operator/ceohelpers/common_test.go

@@ -228,3 +228,79 @@ func TestCurrentRevision(t *testing.T) {
 		})
 	}
 }
+
+func TestCanonicalizeIP(t *testing.T) {
+	scenarios := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "valid IPv4",
+			input:    "192.168.1.10",
+			expected: "192.168.1.10",
+		},
+		{
+			name:     "valid IPv6 compressed",
+			input:    "2001:db8::1",
+			expected: "2001:db8::1",
+		},
+		{
+			name:     "IPv6 with leading zeros",
+			input:    "2001:0db8:0000:0000:0000:0000:0000:0001",
+			expected: "2001:db8::1",
+		},
+		{
+			name:     "IPv6 mixed case",
+			input:    "2001:DB8::1",
+			expected: "2001:db8::1",
+		},
+		{
+			name:     "IPv4-mapped IPv6 converts to IPv4",
+			input:    "::ffff:192.0.2.1",
+			expected: "192.0.2.1",
+		},
+		{
+			name:     "IPv4-mapped IPv6 with zeros converts to IPv4",
+			input:    "::ffff:c000:0201",
+			expected: "192.0.2.1",
+		},
+		{
+			name:     "IPv6 loopback",
+			input:    "::1",
+			expected: "::1",
+		},
+		{
+			name:     "IPv4 loopback",
+			input:    "127.0.0.1",
+			expected: "127.0.0.1",
+		},
+		{
+			name:     "invalid IP returns original",
+			input:    "not-an-ip",
+			expected: "not-an-ip",
+		},
+		{
+			name:     "empty string returns original",
+			input:    "",
+			expected: "",
+		},
+		{
+			name:     "IPv6 with redundant zeros",
+			input:    "fe80:0000:0000:0000:0202:b3ff:fe1e:8329",
+			expected: "fe80::202:b3ff:fe1e:8329",
+		},
+		{
+			name:     "IPv6 full form",
+			input:    "2001:0db8:85a3:0000:0000:8a2e:0370:7334",
+			expected: "2001:db8:85a3::8a2e:370:7334",
+		},
+	}
+
+	for _, scenario := range scenarios {
+		t.Run(scenario.name, func(t *testing.T) {
+			actual := CanonicalizeIP(scenario.input)
+			require.Equal(t, scenario.expected, actual)
+		})
+	}
+}

pkg/operator/clustermemberremovalcontroller/clustermemberremovalcontroller.go

@@ -362,7 +362,7 @@ func (c *clusterMemberRemovalController) removeMemberWithoutMachine(ctx context.
 			if err != nil {
 				return err
 			}
-			if memberIP != potentialMemberToRemoveIP {
+			if ceohelpers.CanonicalizeIP(memberIP) != ceohelpers.CanonicalizeIP(potentialMemberToRemoveIP) {
 				continue
 			}
 			// a member without a node must be reported as unhealthy
@@ -479,7 +479,7 @@ func (c *clusterMemberRemovalController) getNodeForMember(memberInternalIP strin
 		if err != nil {
 			return nil, fmt.Errorf("failed to get internal IP for node: %w", err)
 		}
-		if memberInternalIP == internalNodeIP {
+		if ceohelpers.CanonicalizeIP(memberInternalIP) == ceohelpers.CanonicalizeIP(internalNodeIP) {
 			return masterNode, nil
 		}
 	}
@@ -526,8 +526,9 @@ func (c *clusterMemberRemovalController) getAllVotingMembers(ctx context.Context
 }
 
 func hasInternalIP(machine *machinev1beta1.Machine, memberInternalIP string) bool {
+	canonicalMemberIP := ceohelpers.CanonicalizeIP(memberInternalIP)
 	for _, addr := range machine.Status.Addresses {
-		if addr.Type == corev1.NodeInternalIP && addr.Address == memberInternalIP {
+		if addr.Type == corev1.NodeInternalIP && ceohelpers.CanonicalizeIP(addr.Address) == canonicalMemberIP {
 			return true
 		}
 	}

pkg/operator/machinedeletionhooks/machinedeletionhooks.go

@@ -146,7 +146,7 @@ func (c *machineDeletionHooksController) attemptToDeleteMachineDeletionHook(ctx
 		skipNode := false
 		for _, addr := range machinePendingDeletion.Status.Addresses {
 			if addr.Type == corev1.NodeInternalIP {
-				if currentMemberIPListSet.Has(addr.Address) {
+				if currentMemberIPListSet.Has(ceohelpers.CanonicalizeIP(addr.Address)) {
 					skipNode = true
 					break
 				}

pkg/tlshelpers/tlshelpers.go

@@ -9,6 +9,7 @@ import (
 	"k8s.io/client-go/util/cert"
 
 	"github.com/openshift/cluster-etcd-operator/pkg/dnshelpers"
+	"github.com/openshift/cluster-etcd-operator/pkg/operator/ceohelpers"
 	"github.com/openshift/cluster-etcd-operator/pkg/operator/operatorclient"
 	"github.com/openshift/library-go/pkg/operator/certrotation"
 	"github.com/openshift/library-go/pkg/operator/events"
@@ -60,6 +61,10 @@ func GetServingMetricsSecretNameForNode(nodeName string) string {
 }
 
 func getServerHostNames(nodeInternalIPs []string) []string {
+	canonicalIPs := make([]string, 0, len(nodeInternalIPs))
+	for _, ip := range nodeInternalIPs {
+		canonicalIPs = append(canonicalIPs, ceohelpers.CanonicalizeIP(ip))
+	}
 	return append([]string{
 		"localhost",
 		"etcd.kube-system.svc",
@@ -69,7 +74,7 @@ func getServerHostNames(nodeInternalIPs []string) []string {
 		"127.0.0.1",
 		"::1",
 		// "0:0:0:0:0:0:0:1" will be automatically collapsed to "::1", so we don't have to add it on top
-	}, nodeInternalIPs...)
+	}, canonicalIPs...)
 }
 
 func CreateSignerCertRotationBundleConfigMap(