OCPBUGS-79354: Canonicalize all IP Address comparisons

pkg/cmd/render/bootstrap_ip.go

@@ -137,9 +137,71 @@ func ContainedByCIDR(cidr string) AddressFilter {
 	}
 }
 
+// normalizeIPv4 removes leading zeros from IPv4 addresses.
+// For example, "192.000.002.001" becomes "192.0.2.1".
+// Returns the normalized string if it's a valid IPv4, otherwise returns the original.
+func normalizeIPv4(ip string) string {
+	// Try parsing as-is first
+	if parsed := net.ParseIP(ip); parsed != nil {
+		return ip
+	}
+
+	// Check if it looks like an IPv4 with potential leading zeros
+	parts := make([]string, 0, 4)
+	octet := ""
+	dotCount := 0
+
+	for _, ch := range ip {
+		if ch == '.' {
+			if octet == "" {
+				return ip // Invalid: empty octet
+			}
+			// Remove leading zeros from octet
+			normalized := octet
+			for len(normalized) > 1 && normalized[0] == '0' {
+				normalized = normalized[1:]
+			}
+			parts = append(parts, normalized)
+			octet = ""
+			dotCount++
+		} else if ch >= '0' && ch <= '9' {
+			octet += string(ch)
+		} else {
+			return ip // Invalid character
+		}
+	}
+
+	// Add the last octet
+	if octet == "" || dotCount != 3 {
+		return ip // Invalid format
+	}
+	normalized := octet
+	for len(normalized) > 1 && normalized[0] == '0' {
+		normalized = normalized[1:]
+	}
+	parts = append(parts, normalized)
+
+	result := fmt.Sprintf("%s.%s.%s.%s", parts[0], parts[1], parts[2], parts[3])
+
+	// Verify it's valid
+	if net.ParseIP(result) == nil {
+		return ip // Return original if normalization failed
+	}
+
+	return result
+}
+
 func AddressNotIn(ips ...string) AddressFilter {
 	return func(addr netlink.Addr) bool {
-		return !slices.Contains(ips, addr.IP.String())
+		canonicalAddr := addr.IP.String()
+		for _, ip := range ips {
+			normalizedIP := normalizeIPv4(ip)
+			parsedIP := net.ParseIP(normalizedIP)
+			if parsedIP != nil && canonicalAddr == parsedIP.String() {
+				return false
+			}
+		}
+		return true
 	}
 }
 

pkg/cmd/render/bootstrap_ip_test.go

@@ -234,3 +234,194 @@ func withRoute(linkIndex int, dst string, protocol int) LinkRoutes {
 		},
 	}
 }
+
+func TestAddressNotIn(t *testing.T) {
+	scenarios := []struct {
+		name        string
+		excludedIPs []string
+		testAddr    string
+		expected    bool
+	}{
+		{
+			name:        "address not in empty exclusion list",
+			excludedIPs: []string{},
+			testAddr:    "192.0.2.1",
+			expected:    true,
+		},
+		{
+			name:        "address not in exclusion list",
+			excludedIPs: []string{"192.0.2.2", "192.0.2.3"},
+			testAddr:    "192.0.2.1",
+			expected:    true,
+		},
+		{
+			name:        "address in exclusion list",
+			excludedIPs: []string{"192.0.2.1", "192.0.2.2"},
+			testAddr:    "192.0.2.1",
+			expected:    false,
+		},
+		{
+			name:        "IPv4 canonicalization - leading zeros",
+			excludedIPs: []string{"192.000.002.001"},
+			testAddr:    "192.0.2.1",
+			expected:    false,
+		},
+		{
+			name:        "IPv6 address not in exclusion list",
+			excludedIPs: []string{"2001:db8::2", "2001:db8::3"},
+			testAddr:    "2001:db8::1",
+			expected:    true,
+		},
+		{
+			name:        "IPv6 address in exclusion list",
+			excludedIPs: []string{"2001:db8::1", "2001:db8::2"},
+			testAddr:    "2001:db8::1",
+			expected:    false,
+		},
+		{
+			name:        "IPv6 canonicalization - expanded form",
+			excludedIPs: []string{"2001:0db8:0000:0000:0000:0000:0000:0001"},
+			testAddr:    "2001:db8::1",
+			expected:    false,
+		},
+		{
+			name:        "IPv6 canonicalization - compressed form matches expanded",
+			excludedIPs: []string{"2001:db8::1"},
+			testAddr:    "2001:0db8:0000:0000:0000:0000:0000:0001",
+			expected:    false,
+		},
+		{
+			name:        "invalid IP in exclusion list is ignored",
+			excludedIPs: []string{"not-an-ip", "192.0.2.2"},
+			testAddr:    "192.0.2.1",
+			expected:    true,
+		},
+		{
+			name:        "multiple exclusions with mixed valid and invalid",
+			excludedIPs: []string{"invalid", "192.0.2.1", "also-invalid"},
+			testAddr:    "192.0.2.1",
+			expected:    false,
+		},
+		{
+			name:        "IPv4-mapped IPv6 address",
+			excludedIPs: []string{"::ffff:192.0.2.1"},
+			testAddr:    "::ffff:192.0.2.1",
+			expected:    false,
+		},
+		{
+			name:        "single IP exclusion",
+			excludedIPs: []string{"10.0.0.1"},
+			testAddr:    "10.0.0.1",
+			expected:    false,
+		},
+		{
+			name:        "many exclusions, address not present",
+			excludedIPs: []string{"10.0.0.1", "10.0.0.2", "10.0.0.3", "10.0.0.4", "10.0.0.5"},
+			testAddr:    "10.0.0.100",
+			expected:    true,
+		},
+		{
+			name:        "many exclusions, address present at end",
+			excludedIPs: []string{"10.0.0.1", "10.0.0.2", "10.0.0.3", "10.0.0.4", "10.0.0.100"},
+			testAddr:    "10.0.0.100",
+			expected:    false,
+		},
+	}
+
+	for _, scenario := range scenarios {
+		t.Run(scenario.name, func(t *testing.T) {
+			filter := AddressNotIn(scenario.excludedIPs...)
+			testNetlinkAddr := netlink.Addr{
+				IPNet: netlink.NewIPNet(net.ParseIP(scenario.testAddr)),
+			}
+			result := filter(testNetlinkAddr)
+			if result != scenario.expected {
+				t.Errorf("expected %v, got %v for address %s with exclusions %v",
+					scenario.expected, result, scenario.testAddr, scenario.excludedIPs)
+			}
+		})
+	}
+}
+
+func TestNormalizeIPv4(t *testing.T) {
+	scenarios := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "IPv4 with leading zeros",
+			input:    "192.000.002.001",
+			expected: "192.0.2.1",
+		},
+		{
+			name:     "IPv4 without leading zeros",
+			input:    "192.0.2.1",
+			expected: "192.0.2.1",
+		},
+		{
+			name:     "IPv4 with mixed leading zeros",
+			input:    "010.000.002.100",
+			expected: "10.0.2.100",
+		},
+		{
+			name:     "IPv4 all zeros",
+			input:    "000.000.000.000",
+			expected: "0.0.0.0",
+		},
+		{
+			name:     "IPv4 with all leading zeros in last octet",
+			input:    "192.168.1.001",
+			expected: "192.168.1.1",
+		},
+		{
+			name:     "IPv6 address - should return as-is",
+			input:    "2001:db8::1",
+			expected: "2001:db8::1",
+		},
+		{
+			name:     "IPv6 with leading zeros - should return as-is",
+			input:    "2001:0db8:0000:0000:0000:0000:0000:0001",
+			expected: "2001:0db8:0000:0000:0000:0000:0000:0001",
+		},
+		{
+			name:     "invalid IP - should return as-is",
+			input:    "not-an-ip",
+			expected: "not-an-ip",
+		},
+		{
+			name:     "empty string - should return as-is",
+			input:    "",
+			expected: "",
+		},
+		{
+			name:     "IPv4 with too few octets - should return as-is",
+			input:    "192.168.1",
+			expected: "192.168.1",
+		},
+		{
+			name:     "IPv4 with too many octets - should return as-is",
+			input:    "192.168.1.1.1",
+			expected: "192.168.1.1.1",
+		},
+		{
+			name:     "IPv4 with invalid characters - should return as-is",
+			input:    "192.168.1.a",
+			expected: "192.168.1.a",
+		},
+		{
+			name:     "IPv4 with out-of-range octet after normalization - should return as-is",
+			input:    "192.168.1.999",
+			expected: "192.168.1.999",
+		},
+	}
+
+	for _, scenario := range scenarios {
+		t.Run(scenario.name, func(t *testing.T) {
+			result := normalizeIPv4(scenario.input)
+			if result != scenario.expected {
+				t.Errorf("normalizeIPv4(%q) = %q, expected %q", scenario.input, result, scenario.expected)
+			}
+		})
+	}
+}

pkg/dnshelpers/util.go

@@ -40,15 +40,15 @@ func GetPreferredInternalIPAddressForNodeName(network *configv1.Network, node *c
 					return "", "", err
 				}
 				if isIPv4 {
-					return currAddress.Address, ipFamily, nil
+					return CanonicalizeIP(currAddress.Address), ipFamily, nil
 				}
 			case "tcp6":
 				isIPv4, err := IsIPv4(currAddress.Address)
 				if err != nil {
 					return "", "", err
 				}
 				if !isIPv4 {
-					return currAddress.Address, ipFamily, nil
+					return CanonicalizeIP(currAddress.Address), ipFamily, nil
 				}
 			default:
 				return "", "", fmt.Errorf("unexpected ip family: %q", ipFamily)
@@ -109,7 +109,7 @@ func IsIPv4(ipString string) (bool, error) {
 func GetInternalIPAddressesForNodeName(node *corev1.Node) ([]string, error) {
 	addresses := []string{}
 	for _, currAddress := range node.Status.Addresses {
-		if currAddress.Type == corev1.NodeInternalIP {
+		if currAddress.Type == corev1.NodeInternalIP && currAddress.Address != "" {
 			addresses = append(addresses, currAddress.Address)
 		}
 	}
@@ -120,7 +120,22 @@ func GetInternalIPAddressesForNodeName(node *corev1.Node) ([]string, error) {
 	return addresses, nil
 }
 
-// GetIPFromAddress takes a client or peer address and returns the IP address (unescaped if IPv6).
+// 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
+}
+
+// GetIPFromAddress takes a client or peer address and returns the canonical IP address (unescaped if IPv6).
+// The returned IP is in canonical form for consistent comparisons.
 func GetIPFromAddress(address string) (string, error) {
 	u, err := url.Parse(address)
 	if err != nil {
@@ -131,5 +146,5 @@ func GetIPFromAddress(address string) (string, error) {
 		return "", fmt.Errorf("failed to split host port: %s: %w", u.Host, err)
 	}
 
-	return host, nil
+	return CanonicalizeIP(host), nil
 }

pkg/etcdcli/etcdcli.go

@@ -534,10 +534,22 @@ func endpoints(nodeLister corev1listers.NodeLister,
 		}
 	} else {
 		if bootstrapIP, ok := configmap.Annotations[BootstrapIPAnnotationKey]; ok && bootstrapIP != "" {
-			etcdEndpoints = append(etcdEndpoints, fmt.Sprintf("https://%s", net.JoinHostPort(bootstrapIP, "2379")))
+			canonicalIP := dnshelpers.CanonicalizeIP(bootstrapIP)
+			// Add canonical version
+			etcdEndpoints = append(etcdEndpoints, fmt.Sprintf("https://%s", net.JoinHostPort(canonicalIP, "2379")))
+			// Add non-canonical version for backward compatibility if different
+			if canonicalIP != bootstrapIP {
+				etcdEndpoints = append(etcdEndpoints, fmt.Sprintf("https://%s", net.JoinHostPort(bootstrapIP, "2379")))
+			}
 		}
 		for _, etcdEndpointIp := range configmap.Data {
-			etcdEndpoints = append(etcdEndpoints, fmt.Sprintf("https://%s", net.JoinHostPort(etcdEndpointIp, "2379")))
+			canonicalIP := dnshelpers.CanonicalizeIP(etcdEndpointIp)
+			// Add canonical version
+			etcdEndpoints = append(etcdEndpoints, fmt.Sprintf("https://%s", net.JoinHostPort(canonicalIP, "2379")))
+			// Add non-canonical version for backward compatibility if different
+			if canonicalIP != etcdEndpointIp {
+				etcdEndpoints = append(etcdEndpoints, fmt.Sprintf("https://%s", net.JoinHostPort(etcdEndpointIp, "2379")))
+			}
 		}
 	}
 

pkg/etcdcli/etcdcli_test.go

@@ -73,8 +73,8 @@ func TestEndpointFunc(t *testing.T) {
 				u.FakeNode("2", u.WithMasterLabel(), u.WithNodeInternalIP("fda6:cfed:b298:2514:0000:0000:0000:0001")),
 			},
 			[]string{
-				"https://[fda6:cfed:b298:2514:0000:0000:0000:0000]:2379",
-				"https://[fda6:cfed:b298:2514:0000:0000:0000:0001]:2379",
+				"https://[fda6:cfed:b298:2514::1]:2379",
+				"https://[fda6:cfed:b298:2514::]:2379",
 			},
 			nil,
 		},

pkg/etcdenvvar/envvarcontroller_test.go

@@ -37,7 +37,11 @@ var (
 		[]configv1.FeatureGateName{backendquotahelpers.BackendQuotaFeatureGateName},
 		[]configv1.FeatureGateName{})
 
-	defaultEnvResult = map[string]string{
+	defaultEnvResult = getDefaultEnvResult()
+)
+
+func getDefaultEnvResult() map[string]string {
+	result := map[string]string{
 		"ALL_ETCD_ENDPOINTS":                  "https://192.168.2.0:2379,https://192.168.2.1:2379,https://192.168.2.2:2379",
 		"ETCDCTL_CACERT":                      "/etc/kubernetes/static-pod-certs/configmaps/etcd-all-bundles/server-ca-bundle.crt",
 		"ETCDCTL_CERT":                        "/etc/kubernetes/static-pod-certs/secrets/etcd-all-certs/etcd-peer-NODE_NAME.crt",
@@ -66,7 +70,9 @@ var (
 		"NODE_master_2_ETCD_URL_HOST":         "192.168.2.2",
 		"NODE_master_2_IP":                    "192.168.2.2",
 	}
-)
+
+	return result
+}
 
 func TestEnvVarController(t *testing.T) {
 	scenarios := []struct {