kube-proxy: LoadBalancerSourceRanges as *net.IPNet

cmd/kube-proxy/app/server_linux.go

@@ -508,7 +508,7 @@ func getLocalDetector(logger klog.Logger, ipFamily v1.IPFamily, mode proxyconfig
 
 		cidrsByFamily := proxyutil.MapCIDRsByIPFamily(strings.Split(clusterCIDRs, ","))
 		if len(cidrsByFamily[ipFamily]) != 0 {
-			return proxyutiliptables.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0])
+			return proxyutiliptables.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0].String())
 		}
 
 		logger.Info("Detect-local-mode set to ClusterCIDR, but no cluster CIDR for family", "ipFamily", ipFamily)
@@ -516,7 +516,7 @@ func getLocalDetector(logger klog.Logger, ipFamily v1.IPFamily, mode proxyconfig
 	case proxyconfigapi.LocalModeNodeCIDR:
 		cidrsByFamily := proxyutil.MapCIDRsByIPFamily(nodePodCIDRs)
 		if len(cidrsByFamily[ipFamily]) != 0 {
-			return proxyutiliptables.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0])
+			return proxyutiliptables.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0].String())
 		}
 
 		logger.Info("Detect-local-mode set to NodeCIDR, but no PodCIDR defined at node for family", "ipFamily", ipFamily)

pkg/proxy/iptables/proxier.go

@@ -54,7 +54,6 @@ import (
 	"k8s.io/kubernetes/pkg/util/async"
 	utiliptables "k8s.io/kubernetes/pkg/util/iptables"
 	utilexec "k8s.io/utils/exec"
-	netutils "k8s.io/utils/net"
 )
 
 const (
@@ -1294,12 +1293,9 @@ func (proxier *Proxier) syncProxyRules() {
 
 			// firewall filter based on each source range
 			allowFromNode := false
-			for _, src := range svcInfo.LoadBalancerSourceRanges() {
+			for _, cidr := range svcInfo.LoadBalancerSourceRanges() {
-				natRules.Write(args, "-s", src, "-j", string(externalTrafficChain))
+				natRules.Write(args, "-s", cidr.String(), "-j", string(externalTrafficChain))
-				_, cidr, err := netutils.ParseCIDRSloppy(src)
+				if cidr.Contains(proxier.nodeIP) {
-				if err != nil {
-					klog.ErrorS(err, "Error parsing CIDR in LoadBalancerSourceRanges, dropping it", "cidr", cidr)
-				} else if cidr.Contains(proxier.nodeIP) {
 					allowFromNode = true
 				}
 			}

pkg/proxy/ipvs/proxier.go

@@ -1187,13 +1187,13 @@ func (proxier *Proxier) syncProxyRules() {
 				}
 				proxier.ipsetList[kubeLoadBalancerFWSet].activeEntries.Insert(entry.String())
 				allowFromNode := false
-				for _, src := range svcInfo.LoadBalancerSourceRanges() {
+				for _, cidr := range svcInfo.LoadBalancerSourceRanges() {
 					// ipset call
 					entry = &utilipset.Entry{
 						IP:       ingress.String(),
 						Port:     svcInfo.Port(),
 						Protocol: protocol,
-						Net:      src,
+						Net:      cidr.String(),
 						SetType:  utilipset.HashIPPortNet,
 					}
 					// enumerate all white list source cidr
@@ -1203,8 +1203,6 @@ func (proxier *Proxier) syncProxyRules() {
 					}
 					proxier.ipsetList[kubeLoadBalancerSourceCIDRSet].activeEntries.Insert(entry.String())
 
-					// ignore error because it has been validated
-					_, cidr, _ := netutils.ParseCIDRSloppy(src)
 					if cidr.Contains(proxier.nodeIP) {
 						allowFromNode = true
 					}

pkg/proxy/nftables/proxier.go

@@ -55,7 +55,6 @@ import (
 	proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables"
 	"k8s.io/kubernetes/pkg/util/async"
 	utilexec "k8s.io/utils/exec"
-	netutils "k8s.io/utils/net"
 	"k8s.io/utils/ptr"
 )
 
@@ -1194,15 +1193,11 @@ func (proxier *Proxier) syncProxyRules() {
 			ensureChain(fwChain, tx, activeChains)
 			var sources []string
 			allowFromNode := false
-			for _, src := range svcInfo.LoadBalancerSourceRanges() {
+			for _, cidr := range svcInfo.LoadBalancerSourceRanges() {
-				_, cidr, _ := netutils.ParseCIDRSloppy(src)
-				if cidr == nil {
-					continue
-				}
 				if len(sources) > 0 {
 					sources = append(sources, ",")
 				}
-				sources = append(sources, src)
+				sources = append(sources, cidr.String())
 				if cidr.Contains(proxier.nodeIP) {
 					allowFromNode = true
 				}

pkg/proxy/servicechangetracker_test.go

@@ -27,7 +27,6 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/dump"
 	"k8s.io/apimachinery/pkg/util/intstr"
-	"k8s.io/apimachinery/pkg/util/sets"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	"k8s.io/kubernetes/pkg/features"
@@ -94,6 +93,17 @@ func makeIPs(ipStr ...string) []net.IP {
 	}
 	return ips
 }
+func mustMakeCIDRs(cidrStr ...string) []*net.IPNet {
+	var cidrs []*net.IPNet
+	for _, s := range cidrStr {
+		if _, n, err := netutils.ParseCIDRSloppy(s); err == nil {
+			cidrs = append(cidrs, n)
+		} else {
+			panic(err)
+		}
+	}
+	return cidrs
+}
 
 func TestServiceToServiceMap(t *testing.T) {
 	testClusterIPv4 := "10.0.0.1"
@@ -412,7 +422,7 @@ func TestServiceToServiceMap(t *testing.T) {
 			expected: map[ServicePortName]*BaseServicePortInfo{
 				makeServicePortName("test", "validIPv4", "testPort", v1.ProtocolTCP): makeTestServiceInfo(testClusterIPv4, 12345, "TCP", 0, func(bsvcPortInfo *BaseServicePortInfo) {
 					bsvcPortInfo.externalIPs = makeIPs(testExternalIPv4)
-					bsvcPortInfo.loadBalancerSourceRanges = []string{testSourceRangeIPv4}
+					bsvcPortInfo.loadBalancerSourceRanges = mustMakeCIDRs(testSourceRangeIPv4)
 					bsvcPortInfo.loadBalancerVIPs = makeIPs(testExternalIPv4)
 				}),
 			},
@@ -450,7 +460,7 @@ func TestServiceToServiceMap(t *testing.T) {
 			expected: map[ServicePortName]*BaseServicePortInfo{
 				makeServicePortName("test", "validIPv6", "testPort", v1.ProtocolTCP): makeTestServiceInfo(testClusterIPv6, 12345, "TCP", 0, func(bsvcPortInfo *BaseServicePortInfo) {
 					bsvcPortInfo.externalIPs = makeIPs(testExternalIPv6)
-					bsvcPortInfo.loadBalancerSourceRanges = []string{testSourceRangeIPv6}
+					bsvcPortInfo.loadBalancerSourceRanges = mustMakeCIDRs(testSourceRangeIPv6)
 					bsvcPortInfo.loadBalancerVIPs = makeIPs(testExternalIPv6)
 				}),
 			},
@@ -488,7 +498,7 @@ func TestServiceToServiceMap(t *testing.T) {
 			expected: map[ServicePortName]*BaseServicePortInfo{
 				makeServicePortName("test", "filterIPv6InIPV4Mode", "testPort", v1.ProtocolTCP): makeTestServiceInfo(testClusterIPv4, 12345, "TCP", 0, func(bsvcPortInfo *BaseServicePortInfo) {
 					bsvcPortInfo.externalIPs = makeIPs(testExternalIPv4)
-					bsvcPortInfo.loadBalancerSourceRanges = []string{testSourceRangeIPv4}
+					bsvcPortInfo.loadBalancerSourceRanges = mustMakeCIDRs(testSourceRangeIPv4)
 					bsvcPortInfo.loadBalancerVIPs = makeIPs(testExternalIPv4)
 				}),
 			},
@@ -526,7 +536,7 @@ func TestServiceToServiceMap(t *testing.T) {
 			expected: map[ServicePortName]*BaseServicePortInfo{
 				makeServicePortName("test", "filterIPv4InIPV6Mode", "testPort", v1.ProtocolTCP): makeTestServiceInfo(testClusterIPv6, 12345, "TCP", 0, func(bsvcPortInfo *BaseServicePortInfo) {
 					bsvcPortInfo.externalIPs = makeIPs(testExternalIPv6)
-					bsvcPortInfo.loadBalancerSourceRanges = []string{testSourceRangeIPv6}
+					bsvcPortInfo.loadBalancerSourceRanges = mustMakeCIDRs(testSourceRangeIPv6)
 					bsvcPortInfo.loadBalancerVIPs = makeIPs(testExternalIPv6)
 				}),
 			},
@@ -554,7 +564,7 @@ func TestServiceToServiceMap(t *testing.T) {
 			},
 			expected: map[ServicePortName]*BaseServicePortInfo{
 				makeServicePortName("test", "extra-space", "testPort", v1.ProtocolTCP): makeTestServiceInfo(testClusterIPv4, 12345, "TCP", 0, func(bsvcPortInfo *BaseServicePortInfo) {
-					bsvcPortInfo.loadBalancerSourceRanges = []string{"10.1.2.0/28"}
+					bsvcPortInfo.loadBalancerSourceRanges = mustMakeCIDRs("10.1.2.0/28")
 				}),
 			},
 		},
@@ -581,7 +591,7 @@ func TestServiceToServiceMap(t *testing.T) {
 					svcInfo.protocol != expectedInfo.protocol ||
 					svcInfo.healthCheckNodePort != expectedInfo.healthCheckNodePort ||
 					!reflect.DeepEqual(svcInfo.externalIPs, expectedInfo.externalIPs) ||
-					!sets.New[string](svcInfo.loadBalancerSourceRanges...).Equal(sets.New[string](expectedInfo.loadBalancerSourceRanges...)) ||
+					!reflect.DeepEqual(svcInfo.loadBalancerSourceRanges, expectedInfo.loadBalancerSourceRanges) ||
 					!reflect.DeepEqual(svcInfo.loadBalancerVIPs, expectedInfo.loadBalancerVIPs) {
 					t.Errorf("[%s] expected new[%v]to be %v, got %v", tc.desc, svcKey, expectedInfo, *svcInfo)
 				}
@@ -592,7 +602,7 @@ func TestServiceToServiceMap(t *testing.T) {
 						svcInfo.protocol != expectedInfo.protocol ||
 						svcInfo.healthCheckNodePort != expectedInfo.healthCheckNodePort ||
 						!reflect.DeepEqual(svcInfo.externalIPs, expectedInfo.externalIPs) ||
-						!sets.New[string](svcInfo.loadBalancerSourceRanges...).Equal(sets.New[string](expectedInfo.loadBalancerSourceRanges...)) ||
+						!reflect.DeepEqual(svcInfo.loadBalancerSourceRanges, expectedInfo.loadBalancerSourceRanges) ||
 						!reflect.DeepEqual(svcInfo.loadBalancerVIPs, expectedInfo.loadBalancerVIPs) {
 						t.Errorf("expected new[%v]to be %v, got %v", svcKey, expectedInfo, *svcInfo)
 					}

pkg/proxy/serviceport.go

@@ -19,7 +19,6 @@ package proxy
 import (
 	"fmt"
 	"net"
-	"strings"
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/klog/v2"
@@ -47,7 +46,7 @@ type ServicePort interface {
 	// Protocol returns service protocol.
 	Protocol() v1.Protocol
 	// LoadBalancerSourceRanges returns service LoadBalancerSourceRanges if present empty array if not
-	LoadBalancerSourceRanges() []string
+	LoadBalancerSourceRanges() []*net.IPNet
 	// HealthCheckNodePort returns service health check node port if present.  If return 0, it means not present.
 	HealthCheckNodePort() int
 	// NodePort returns a service Node port if present. If return 0, it means not present.
@@ -82,7 +81,7 @@ type BaseServicePortInfo struct {
 	sessionAffinityType      v1.ServiceAffinity
 	stickyMaxAgeSeconds      int
 	externalIPs              []net.IP
-	loadBalancerSourceRanges []string
+	loadBalancerSourceRanges []*net.IPNet
 	healthCheckNodePort      int
 	externalPolicyLocal      bool
 	internalPolicyLocal      bool
@@ -122,7 +121,7 @@ func (bsvcPortInfo *BaseServicePortInfo) Protocol() v1.Protocol {
 }
 
 // LoadBalancerSourceRanges is part of ServicePort interface
-func (bsvcPortInfo *BaseServicePortInfo) LoadBalancerSourceRanges() []string {
+func (bsvcPortInfo *BaseServicePortInfo) LoadBalancerSourceRanges() []*net.IPNet {
 	return bsvcPortInfo.loadBalancerSourceRanges
 }
 
@@ -208,10 +207,6 @@ func newBaseServiceInfo(service *v1.Service, ipFamily v1.IPFamily, port *v1.Serv
 		info.hintsAnnotation = service.Annotations[v1.AnnotationTopologyMode]
 	}
 
-	loadBalancerSourceRanges := make([]string, len(service.Spec.LoadBalancerSourceRanges))
-	for i, sourceRange := range service.Spec.LoadBalancerSourceRanges {
-		loadBalancerSourceRanges[i] = strings.TrimSpace(sourceRange)
-	}
 	// filter external ips, source ranges and ingress ips
 	// prior to dual stack services, this was considered an error, but with dual stack
 	// services, this is actually expected. Hence we downgraded from reporting by events
@@ -225,12 +220,12 @@ func newBaseServiceInfo(service *v1.Service, ipFamily v1.IPFamily, port *v1.Serv
 			"ipFamily", ipFamily, "externalIPs", ips, "service", klog.KObj(service))
 	}
 
-	cidrFamilyMap := proxyutil.MapCIDRsByIPFamily(loadBalancerSourceRanges)
+	cidrFamilyMap := proxyutil.MapCIDRsByIPFamily(service.Spec.LoadBalancerSourceRanges)
 	info.loadBalancerSourceRanges = cidrFamilyMap[ipFamily]
 	// Log the CIDRs not matching the ipFamily
 	if cidrs, ok := cidrFamilyMap[proxyutil.OtherIPFamily(ipFamily)]; ok && len(cidrs) > 0 {
 		klog.V(4).InfoS("Service change tracker ignored the following load balancer source ranges for given Service as they don't match IP Family",
-			"ipFamily", ipFamily, "loadBalancerSourceRanges", strings.Join(cidrs, ", "), "service", klog.KObj(service))
+			"ipFamily", ipFamily, "loadBalancerSourceRanges", cidrs, "service", klog.KObj(service))
 	}
 
 	// Obtain Load Balancer Ingress

pkg/proxy/util/utils.go

@@ -200,16 +200,22 @@ func MapIPsByIPFamily(ipStrings []string) map[v1.IPFamily][]net.IP {
 	return ipFamilyMap
 }
 
-// MapCIDRsByIPFamily maps a slice of IPs to their respective IP families (v4 or v6)
+// MapCIDRsByIPFamily maps a slice of CIDRs to their respective IP families (v4 or v6)
-func MapCIDRsByIPFamily(cidrStrings []string) map[v1.IPFamily][]string {
+func MapCIDRsByIPFamily(cidrsStrings []string) map[v1.IPFamily][]*net.IPNet {
-	ipFamilyMap := map[v1.IPFamily][]string{}
+	ipFamilyMap := map[v1.IPFamily][]*net.IPNet{}
-	for _, cidr := range cidrStrings {
+	for _, cidrStrUntrimmed := range cidrsStrings {
-		// Handle only the valid CIDRs
+		cidrStr := strings.TrimSpace(cidrStrUntrimmed)
-		if ipFamily := getIPFamilyFromCIDR(cidr); ipFamily != v1.IPFamilyUnknown {
+		_, cidr, err := netutils.ParseCIDRSloppy(cidrStr)
-			ipFamilyMap[ipFamily] = append(ipFamilyMap[ipFamily], cidr)
+		if err != nil {
-		} else {
+			// Ignore empty strings. Same as in MapIPsByIPFamily
-			klog.ErrorS(nil, "Skipping invalid CIDR", "cidr", cidr)
+			if len(cidrStr) != 0 {
+				klog.ErrorS(err, "Invalid CIDR ignored", "CIDR", cidrStr)
+			}
+			continue
 		}
+		// since we just succefully parsed the CIDR, IPFamilyOfCIDR will never return "IPFamilyUnknown"
+		ipFamily := convertToV1IPFamily(netutils.IPFamilyOfCIDR(cidr))
+		ipFamilyMap[ipFamily] = append(ipFamilyMap[ipFamily], cidr)
 	}
 	return ipFamilyMap
 }
@@ -219,11 +225,6 @@ func GetIPFamilyFromIP(ip net.IP) v1.IPFamily {
 	return convertToV1IPFamily(netutils.IPFamilyOf(ip))
 }
 
-// Returns the IP family of cidrStr, or IPFamilyUnknown if cidrStr can't be parsed as a CIDR
-func getIPFamilyFromCIDR(cidrStr string) v1.IPFamily {
-	return convertToV1IPFamily(netutils.IPFamilyOfCIDRString(cidrStr))
-}
-
 // Convert netutils.IPFamily to v1.IPFamily
 func convertToV1IPFamily(ipFamily netutils.IPFamily) v1.IPFamily {
 	switch ipFamily {

pkg/proxy/util/utils_test.go

@@ -448,11 +448,20 @@ func TestMapCIDRsByIPFamily(t *testing.T) {
 
 			cidrMap := MapCIDRsByIPFamily(testcase.ipString)
 
-			if !reflect.DeepEqual(testcase.expectCorrect, cidrMap[ipFamily]) {
+			var cidrStr []string
-				t.Errorf("Test %v failed: expected %v, got %v", testcase.desc, testcase.expectCorrect, cidrMap[ipFamily])
+			for _, cidr := range cidrMap[ipFamily] {
+				cidrStr = append(cidrStr, cidr.String())
 			}
-			if !reflect.DeepEqual(testcase.expectIncorrect, cidrMap[otherIPFamily]) {
+			var cidrStrOther []string
-				t.Errorf("Test %v failed: expected %v, got %v", testcase.desc, testcase.expectIncorrect, cidrMap[otherIPFamily])
+			for _, cidr := range cidrMap[otherIPFamily] {
+				cidrStrOther = append(cidrStrOther, cidr.String())
+			}
+
+			if !reflect.DeepEqual(testcase.expectCorrect, cidrStr) {
+				t.Errorf("Test %v failed: expected %v, got %v", testcase.desc, testcase.expectCorrect, cidrStr)
+			}
+			if !reflect.DeepEqual(testcase.expectIncorrect, cidrStrOther) {
+				t.Errorf("Test %v failed: expected %v, got %v", testcase.desc, testcase.expectIncorrect, cidrStrOther)
 			}
 		})
 	}