Merge pull request #116016 from danwinship/nodeport-addresses-3

clean up kube-proxy nodeport address handling (part 3)

pkg/proxy/healthcheck/healthcheck_test.go

@@ -24,6 +24,7 @@ import (
 	"testing"
 	"time"
 
+	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/dump"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -140,7 +141,7 @@ func (fake fakeProxierHealthChecker) IsHealthy() bool {
 func TestServer(t *testing.T) {
 	listener := newFakeListener()
 	httpFactory := newFakeHTTPServerFactory()
-	nodePortAddresses := utilproxy.NewNodePortAddresses([]string{})
+	nodePortAddresses := utilproxy.NewNodePortAddresses(v1.IPv4Protocol, []string{})
 	proxyChecker := &fakeProxierHealthChecker{true}
 
 	hcsi := newServiceHealthServer("hostname", nil, listener, httpFactory, nodePortAddresses, proxyChecker)
@@ -177,7 +178,7 @@ func TestServer(t *testing.T) {
 	if len(listener.openPorts) != 1 {
 		t.Errorf("expected 1 open port, got %d\n%s", len(listener.openPorts), dump.Pretty(listener.openPorts))
 	}
-	if !listener.hasPort(":9376") {
+	if !listener.hasPort("0.0.0.0:9376") {
 		t.Errorf("expected port :9376 to be open\n%s", dump.Pretty(listener.openPorts))
 	}
 	// test the handler
@@ -470,7 +471,7 @@ func TestServerWithSelectiveListeningAddress(t *testing.T) {
 
 	// limiting addresses to loop back. We don't want any cleverness here around getting IP for
 	// machine nor testing ipv6 || ipv4. using loop back guarantees the test will work on any machine
-	nodePortAddresses := utilproxy.NewNodePortAddresses([]string{"127.0.0.0/8"})
+	nodePortAddresses := utilproxy.NewNodePortAddresses(v1.IPv4Protocol, []string{"127.0.0.0/8"})
 
 	hcsi := newServiceHealthServer("hostname", nil, listener, httpFactory, nodePortAddresses, proxyChecker)
 	hcs := hcsi.(*server)

pkg/proxy/healthcheck/service_health.go

@@ -32,7 +32,6 @@ import (
 	api "k8s.io/kubernetes/pkg/apis/core"
 
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/apimachinery/pkg/util/sets"
 	utilproxy "k8s.io/kubernetes/pkg/proxy/util"
 )
 
@@ -59,20 +58,16 @@ type proxierHealthChecker interface {
 }
 
 func newServiceHealthServer(hostname string, recorder events.EventRecorder, listener listener, factory httpServerFactory, nodePortAddresses *utilproxy.NodePortAddresses, healthzServer proxierHealthChecker) ServiceHealthServer {
+	// It doesn't matter whether we listen on "0.0.0.0", "::", or ""; go
+	// treats them all the same.
+	nodeIPs := []net.IP{net.IPv4zero}
 
-	nodeAddresses, err := nodePortAddresses.GetNodeAddresses(utilproxy.RealNetwork{})
+	if !nodePortAddresses.MatchAll() {
-	if err != nil || nodeAddresses.Len() == 0 {
+		ips, err := nodePortAddresses.GetNodeIPs(utilproxy.RealNetwork{})
-		klog.ErrorS(err, "Failed to get node ip address matching node port addresses, health check port will listen to all node addresses", "nodePortAddresses", nodePortAddresses)
+		if err == nil {
-		nodeAddresses = sets.New[string]()
+			nodeIPs = ips
-		nodeAddresses.Insert(utilproxy.IPv4ZeroCIDR)
+		} else {
-	}
+			klog.ErrorS(err, "Failed to get node ip address matching node port addresses, health check port will listen to all node addresses", "nodePortAddresses", nodePortAddresses)
-
-	// if any of the addresses is zero cidr then we listen
-	// to old style :<port>
-	for _, addr := range nodeAddresses.UnsortedList() {
-		if utilproxy.IsZeroCIDR(addr) {
-			nodeAddresses = sets.New[string]("")
-			break
 		}
 	}
 
@@ -83,7 +78,7 @@ func newServiceHealthServer(hostname string, recorder events.EventRecorder, list
 		httpFactory:   factory,
 		healthzServer: healthzServer,
 		services:      map[types.NamespacedName]*hcInstance{},
-		nodeAddresses: nodeAddresses,
+		nodeIPs:       nodeIPs,
 	}
 }
 
@@ -95,10 +90,10 @@ func NewServiceHealthServer(hostname string, recorder events.EventRecorder, node
 type server struct {
 	hostname string
 	// node addresses where health check port will listen on
-	nodeAddresses sets.Set[string]
+	nodeIPs     []net.IP
-	recorder      events.EventRecorder // can be nil
+	recorder    events.EventRecorder // can be nil
-	listener      listener
+	listener    listener
-	httpFactory   httpServerFactory
+	httpFactory httpServerFactory
 
 	healthzServer proxierHealthChecker
 
@@ -169,12 +164,11 @@ func (hcI *hcInstance) listenAndServeAll(hcs *server) error {
 	var err error
 	var listener net.Listener
 
-	addresses := hcs.nodeAddresses.UnsortedList()
+	hcI.httpServers = make([]httpServer, 0, len(hcs.nodeIPs))
-	hcI.httpServers = make([]httpServer, 0, len(addresses))
 
 	// for each of the node addresses start listening and serving
-	for _, address := range addresses {
+	for _, ip := range hcs.nodeIPs {
-		addr := net.JoinHostPort(address, fmt.Sprint(hcI.port))
+		addr := net.JoinHostPort(ip.String(), fmt.Sprint(hcI.port))
 		// create http server
 		httpSrv := hcs.httpFactory.New(addr, hcHandler{name: hcI.nsn, hcs: hcs})
 		// start listener

pkg/proxy/iptables/proxier.go

@@ -240,7 +240,7 @@ func NewProxier(ipFamily v1.IPFamily,
 	healthzServer healthcheck.ProxierHealthUpdater,
 	nodePortAddressStrings []string,
 ) (*Proxier, error) {
-	nodePortAddresses := utilproxy.NewNodePortAddresses(nodePortAddressStrings)
+	nodePortAddresses := utilproxy.NewNodePortAddresses(ipFamily, nodePortAddressStrings)
 
 	if !nodePortAddresses.ContainsIPv4Loopback() {
 		localhostNodePorts = false
@@ -334,17 +334,16 @@ func NewDualStackProxier(
 	nodePortAddresses []string,
 ) (proxy.Provider, error) {
 	// Create an ipv4 instance of the single-stack proxier
-	ipFamilyMap := utilproxy.MapCIDRsByIPFamily(nodePortAddresses)
 	ipv4Proxier, err := NewProxier(v1.IPv4Protocol, ipt[0], sysctl,
 		exec, syncPeriod, minSyncPeriod, masqueradeAll, localhostNodePorts, masqueradeBit, localDetectors[0], hostname,
-		nodeIP[0], recorder, healthzServer, ipFamilyMap[v1.IPv4Protocol])
+		nodeIP[0], recorder, healthzServer, nodePortAddresses)
 	if err != nil {
 		return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err)
 	}
 
 	ipv6Proxier, err := NewProxier(v1.IPv6Protocol, ipt[1], sysctl,
 		exec, syncPeriod, minSyncPeriod, masqueradeAll, false, masqueradeBit, localDetectors[1], hostname,
-		nodeIP[1], recorder, healthzServer, ipFamilyMap[v1.IPv6Protocol])
+		nodeIP[1], recorder, healthzServer, nodePortAddresses)
 	if err != nil {
 		return nil, fmt.Errorf("unable to create ipv6 proxier: %v", err)
 	}
@@ -1416,70 +1415,44 @@ func (proxier *Proxier) syncProxyRules() {
 
 	// Finally, tail-call to the nodePorts chain.  This needs to be after all
 	// other service portal rules.
-	nodeAddresses, err := proxier.nodePortAddresses.GetNodeAddresses(proxier.networkInterfacer)
+	if proxier.nodePortAddresses.MatchAll() {
-	if err != nil {
+		destinations := []string{"-m", "addrtype", "--dst-type", "LOCAL"}
-		klog.ErrorS(err, "Failed to get node ip address matching nodeport cidrs, services with nodeport may not work as intended", "CIDRs", proxier.nodePortAddresses)
+		// Block localhost nodePorts if they are not supported. (For IPv6 they never
-	}
+		// work, and for IPv4 they only work if we previously set `route_localnet`.)
-	// nodeAddresses may contain dual-stack zero-CIDRs if proxier.nodePortAddresses is empty.
+		if isIPv6 {
-	// Ensure nodeAddresses only contains the addresses for this proxier's IP family.
+			destinations = append(destinations, "!", "-d", "::1/128")
-	for addr := range nodeAddresses {
+		} else if !proxier.localhostNodePorts {
-		if utilproxy.IsZeroCIDR(addr) && isIPv6 == netutils.IsIPv6CIDRString(addr) {
+			destinations = append(destinations, "!", "-d", "127.0.0.0/8")
-			// if any of the addresses is zero cidr of this IP family, non-zero IPs can be excluded.
-			nodeAddresses = sets.New[string](addr)
-			break
-		}
-	}
-
-	for address := range nodeAddresses {
-		if utilproxy.IsZeroCIDR(address) {
-			destinations := []string{"-m", "addrtype", "--dst-type", "LOCAL"}
-			if isIPv6 {
-				// For IPv6, Regardless of the value of localhostNodePorts is true
-				// or false, we should disable access to the nodePort via localhost. Since it never works and always
-				// cause kernel warnings.
-				destinations = append(destinations, "!", "-d", "::1/128")
-			}
-
-			if !proxier.localhostNodePorts && !isIPv6 {
-				// If set localhostNodePorts to "false"(route_localnet=0), We should generate iptables rules that
-				// disable NodePort services to be accessed via localhost. Since it doesn't work and causes
-				// the kernel to log warnings if anyone tries.
-				destinations = append(destinations, "!", "-d", "127.0.0.0/8")
-			}
-
-			proxier.natRules.Write(
-				"-A", string(kubeServicesChain),
-				"-m", "comment", "--comment", `"kubernetes service nodeports; NOTE: this must be the last rule in this chain"`,
-				destinations,
-				"-j", string(kubeNodePortsChain))
-			break
 		}
 
-		// Ignore IP addresses with incorrect version
-		if isIPv6 && !netutils.IsIPv6String(address) || !isIPv6 && netutils.IsIPv6String(address) {
-			klog.ErrorS(nil, "IP has incorrect IP version", "IP", address)
-			continue
-		}
-
-		// For ipv6, Regardless of the value of localhostNodePorts is true or false, we should disallow access
-		// to the nodePort via lookBack address.
-		if isIPv6 && utilproxy.IsLoopBack(address) {
-			klog.ErrorS(nil, "disallow nodePort services to be accessed via ipv6 localhost address", "IP", address)
-			continue
-		}
-
-		// For ipv4, When localhostNodePorts is set to false, Ignore ipv4 lookBack address
-		if !isIPv6 && utilproxy.IsLoopBack(address) && !proxier.localhostNodePorts {
-			klog.ErrorS(nil, "disallow nodePort services to be accessed via ipv4 localhost address", "IP", address)
-			continue
-		}
-
-		// create nodeport rules for each IP one by one
 		proxier.natRules.Write(
 			"-A", string(kubeServicesChain),
 			"-m", "comment", "--comment", `"kubernetes service nodeports; NOTE: this must be the last rule in this chain"`,
-			"-d", address,
+			destinations,
 			"-j", string(kubeNodePortsChain))
+	} else {
+		nodeIPs, err := proxier.nodePortAddresses.GetNodeIPs(proxier.networkInterfacer)
+		if err != nil {
+			klog.ErrorS(err, "Failed to get node ip address matching nodeport cidrs, services with nodeport may not work as intended", "CIDRs", proxier.nodePortAddresses)
+		}
+		for _, ip := range nodeIPs {
+			if ip.IsLoopback() {
+				if isIPv6 {
+					klog.ErrorS(nil, "--nodeport-addresses includes localhost but localhost NodePorts are not supported on IPv6", "address", ip.String())
+					continue
+				} else if !proxier.localhostNodePorts {
+					klog.ErrorS(nil, "--nodeport-addresses includes localhost but --iptables-localhost-nodeports=false was passed", "address", ip.String())
+					continue
+				}
+			}
+
+			// create nodeport rules for each IP one by one
+			proxier.natRules.Write(
+				"-A", string(kubeServicesChain),
+				"-m", "comment", "--comment", `"kubernetes service nodeports; NOTE: this must be the last rule in this chain"`,
+				"-d", ip.String(),
+				"-j", string(kubeNodePortsChain))
+		}
 	}
 
 	// Drop the packets in INVALID state, which would potentially cause
@@ -1537,7 +1510,7 @@ func (proxier *Proxier) syncProxyRules() {
 	klog.V(9).InfoS("Restoring iptables", "rules", proxier.iptablesData.Bytes())
 
 	// NOTE: NoFlushTables is used so we don't flush non-kubernetes chains in the table
-	err = proxier.iptables.RestoreAll(proxier.iptablesData.Bytes(), utiliptables.NoFlushTables, utiliptables.RestoreCounters)
+	err := proxier.iptables.RestoreAll(proxier.iptablesData.Bytes(), utiliptables.NoFlushTables, utiliptables.RestoreCounters)
 	if err != nil {
 		if pErr, ok := err.(utiliptables.ParseError); ok {
 			lines := utiliptables.ExtractLines(proxier.iptablesData.Bytes(), pErr.Line(), 3)

pkg/proxy/iptables/proxier_test.go

@@ -303,6 +303,9 @@ func NewFakeProxier(ipt utiliptables.Interface) *Proxier {
 	itf1 := net.Interface{Index: 1, MTU: 0, Name: "eth0", HardwareAddr: nil, Flags: 0}
 	addrs1 := []net.Addr{
 		&net.IPNet{IP: netutils.ParseIPSloppy(testNodeIP), Mask: net.CIDRMask(24, 32)},
+		// (This IP never actually gets used; it's only here to test that it gets
+		// filtered out correctly in the IPv4 nodeport tests.)
+		&net.IPNet{IP: netutils.ParseIPSloppy("2001:db8::1"), Mask: net.CIDRMask(64, 128)},
 	}
 	networkInterfacer.AddInterfaceAddr(&itf1, addrs1)
 
@@ -327,7 +330,7 @@ func NewFakeProxier(ipt utiliptables.Interface) *Proxier {
 		natRules:                 utilproxy.LineBuffer{},
 		nodeIP:                   netutils.ParseIPSloppy(testNodeIP),
 		localhostNodePorts:       true,
-		nodePortAddresses:        utilproxy.NewNodePortAddresses(nil),
+		nodePortAddresses:        utilproxy.NewNodePortAddresses(ipfamily, nil),
 		networkInterfacer:        networkInterfacer,
 	}
 	p.setInitialized(true)
@@ -2461,7 +2464,7 @@ func TestNodePort(t *testing.T) {
 func TestHealthCheckNodePort(t *testing.T) {
 	ipt := iptablestest.NewFake()
 	fp := NewFakeProxier(ipt)
-	fp.nodePortAddresses = utilproxy.NewNodePortAddresses([]string{"127.0.0.0/8"})
+	fp.nodePortAddresses = utilproxy.NewNodePortAddresses(v1.IPv4Protocol, []string{"127.0.0.0/8"})
 
 	svcIP := "172.30.0.42"
 	svcPort := 80
@@ -3390,7 +3393,7 @@ func TestDisableLocalhostNodePortsIPv4WithNodeAddress(t *testing.T) {
 	fp.localDetector = proxyutiliptables.NewNoOpLocalDetector()
 	fp.localhostNodePorts = false
 	fp.networkInterfacer.InterfaceAddrs()
-	fp.nodePortAddresses = utilproxy.NewNodePortAddresses([]string{"127.0.0.0/8"})
+	fp.nodePortAddresses = utilproxy.NewNodePortAddresses(v1.IPv4Protocol, []string{"127.0.0.0/8"})
 
 	expected := dedent.Dedent(`
 		*filter
@@ -3671,7 +3674,7 @@ func TestOnlyLocalNodePortsNoClusterCIDR(t *testing.T) {
 	ipt := iptablestest.NewFake()
 	fp := NewFakeProxier(ipt)
 	fp.localDetector = proxyutiliptables.NewNoOpLocalDetector()
-	fp.nodePortAddresses = utilproxy.NewNodePortAddresses([]string{"192.168.0.0/24"})
+	fp.nodePortAddresses = utilproxy.NewNodePortAddresses(v1.IPv4Protocol, []string{"192.168.0.0/24", "2001:db8::/64"})
 	fp.localhostNodePorts = false
 
 	expected := dedent.Dedent(`
@@ -3720,7 +3723,7 @@ func TestOnlyLocalNodePortsNoClusterCIDR(t *testing.T) {
 func TestOnlyLocalNodePorts(t *testing.T) {
 	ipt := iptablestest.NewFake()
 	fp := NewFakeProxier(ipt)
-	fp.nodePortAddresses = utilproxy.NewNodePortAddresses([]string{"192.168.0.0/24"})
+	fp.nodePortAddresses = utilproxy.NewNodePortAddresses(v1.IPv4Protocol, []string{"192.168.0.0/24", "2001:db8::/64"})
 	fp.localhostNodePorts = false
 
 	expected := dedent.Dedent(`

pkg/proxy/ipvs/proxier.go

@@ -409,7 +409,7 @@ func NewProxier(ipFamily v1.IPFamily,
 		scheduler = defaultScheduler
 	}
 
-	nodePortAddresses := utilproxy.NewNodePortAddresses(nodePortAddressStrings)
+	nodePortAddresses := utilproxy.NewNodePortAddresses(ipFamily, nodePortAddressStrings)
 
 	serviceHealthServer := healthcheck.NewServiceHealthServer(hostname, recorder, nodePortAddresses, healthzServer)
 
@@ -490,14 +490,12 @@ func NewDualStackProxier(
 
 	safeIpset := newSafeIpset(ipset)
 
-	ipFamilyMap := utilproxy.MapCIDRsByIPFamily(nodePortAddresses)
-
 	// Create an ipv4 instance of the single-stack proxier
 	ipv4Proxier, err := NewProxier(v1.IPv4Protocol, ipt[0], ipvs, safeIpset, sysctl,
 		exec, syncPeriod, minSyncPeriod, filterCIDRs(false, excludeCIDRs), strictARP,
 		tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit,
 		localDetectors[0], hostname, nodeIP[0],
-		recorder, healthzServer, scheduler, ipFamilyMap[v1.IPv4Protocol], kernelHandler)
+		recorder, healthzServer, scheduler, nodePortAddresses, kernelHandler)
 	if err != nil {
 		return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err)
 	}
@@ -506,7 +504,7 @@ func NewDualStackProxier(
 		exec, syncPeriod, minSyncPeriod, filterCIDRs(true, excludeCIDRs), strictARP,
 		tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit,
 		localDetectors[1], hostname, nodeIP[1],
-		recorder, healthzServer, scheduler, ipFamilyMap[v1.IPv6Protocol], kernelHandler)
+		recorder, healthzServer, scheduler, nodePortAddresses, kernelHandler)
 	if err != nil {
 		return nil, fmt.Errorf("unable to create ipv6 proxier: %v", err)
 	}
@@ -1004,35 +1002,23 @@ func (proxier *Proxier) syncProxyRules() {
 		}
 	}
 
-	// Both nodeAddresses and nodeIPs can be reused for all nodePort services
+	// List of node IP addresses to be used as IPVS services if nodePort is set. This
-	// and only need to be computed if we have at least one nodePort service.
+	// can be reused for all nodePort services.
-	var (
+	var nodeIPs []net.IP
-		// List of node addresses to listen on if a nodePort is set.
-		nodeAddresses []string
-		// List of node IP addresses to be used as IPVS services if nodePort is set.
-		nodeIPs []net.IP
-	)
-
 	if hasNodePort {
-		nodeAddrSet, err := proxier.nodePortAddresses.GetNodeAddresses(proxier.networkInterfacer)
+		if proxier.nodePortAddresses.MatchAll() {
-		if err != nil {
+			for _, ipStr := range nodeAddressSet.UnsortedList() {
-			klog.ErrorS(err, "Failed to get node IP address matching nodeport cidr")
+				nodeIPs = append(nodeIPs, netutils.ParseIPSloppy(ipStr))
+			}
 		} else {
-			nodeAddresses = nodeAddrSet.UnsortedList()
+			allNodeIPs, err := proxier.nodePortAddresses.GetNodeIPs(proxier.networkInterfacer)
-			for _, address := range nodeAddresses {
+			if err != nil {
-				a := netutils.ParseIPSloppy(address)
+				klog.ErrorS(err, "Failed to get node IP address matching nodeport cidr")
-				if a.IsLoopback() {
+			} else {
-					continue
+				for _, ip := range allNodeIPs {
-				}
+					if !ip.IsLoopback() {
-				if utilproxy.IsZeroCIDR(address) {
+						nodeIPs = append(nodeIPs, ip)
-					nodeIPs = nil
-					for _, ipStr := range nodeAddressSet.UnsortedList() {
-						nodeIPs = append(nodeIPs, netutils.ParseIPSloppy(ipStr))
 					}
-					break
-				}
-				if getIPFamily(a) == proxier.ipFamily {
-					nodeIPs = append(nodeIPs, a)
 				}
 			}
 		}
@@ -1292,7 +1278,7 @@ func (proxier *Proxier) syncProxyRules() {
 		}
 
 		if svcInfo.NodePort() != 0 {
-			if len(nodeAddresses) == 0 || len(nodeIPs) == 0 {
+			if len(nodeIPs) == 0 {
 				// Skip nodePort configuration since an error occurred when
 				// computing nodeAddresses or nodeIPs.
 				continue

pkg/proxy/ipvs/proxier_test.go

@@ -169,7 +169,7 @@ func NewFakeProxier(ipt utiliptables.Interface, ipvs utilipvs.Interface, ipset u
 		filterRules:           utilproxy.LineBuffer{},
 		netlinkHandle:         netlinkHandle,
 		ipsetList:             ipsetList,
-		nodePortAddresses:     utilproxy.NewNodePortAddresses(nil),
+		nodePortAddresses:     utilproxy.NewNodePortAddresses(ipFamily, nil),
 		networkInterfacer:     proxyutiltest.NewFakeNetwork(),
 		gracefuldeleteManager: NewGracefulTerminationManager(ipvs),
 		ipFamily:              ipFamily,
@@ -960,7 +960,7 @@ func TestNodePortIPv4(t *testing.T) {
 			ipvs := ipvstest.NewFake()
 			ipset := ipsettest.NewFake(testIPSetVersion)
 			fp := NewFakeProxier(ipt, ipvs, ipset, test.nodeIPs, nil, v1.IPv4Protocol)
-			fp.nodePortAddresses = utilproxy.NewNodePortAddresses(test.nodePortAddresses)
+			fp.nodePortAddresses = utilproxy.NewNodePortAddresses(v1.IPv4Protocol, test.nodePortAddresses)
 
 			makeServiceMap(fp, test.services...)
 			populateEndpointSlices(fp, test.endpoints...)
@@ -1305,7 +1305,7 @@ func TestNodePortIPv6(t *testing.T) {
 			ipvs := ipvstest.NewFake()
 			ipset := ipsettest.NewFake(testIPSetVersion)
 			fp := NewFakeProxier(ipt, ipvs, ipset, test.nodeIPs, nil, v1.IPv6Protocol)
-			fp.nodePortAddresses = utilproxy.NewNodePortAddresses(test.nodePortAddresses)
+			fp.nodePortAddresses = utilproxy.NewNodePortAddresses(v1.IPv6Protocol, test.nodePortAddresses)
 
 			makeServiceMap(fp, test.services...)
 			populateEndpointSlices(fp, test.endpoints...)
@@ -2068,7 +2068,7 @@ func TestOnlyLocalNodePorts(t *testing.T) {
 	addrs1 := []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("2001:db8::"), Mask: net.CIDRMask(64, 128)}}
 	fp.networkInterfacer.(*proxyutiltest.FakeNetwork).AddInterfaceAddr(&itf, addrs)
 	fp.networkInterfacer.(*proxyutiltest.FakeNetwork).AddInterfaceAddr(&itf1, addrs1)
-	fp.nodePortAddresses = utilproxy.NewNodePortAddresses([]string{"100.101.102.0/24"})
+	fp.nodePortAddresses = utilproxy.NewNodePortAddresses(v1.IPv4Protocol, []string{"100.101.102.0/24"})
 
 	fp.syncProxyRules()
 
@@ -2156,7 +2156,7 @@ func TestHealthCheckNodePort(t *testing.T) {
 	addrs1 := []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("2001:db8::"), Mask: net.CIDRMask(64, 128)}}
 	fp.networkInterfacer.(*proxyutiltest.FakeNetwork).AddInterfaceAddr(&itf, addrs)
 	fp.networkInterfacer.(*proxyutiltest.FakeNetwork).AddInterfaceAddr(&itf1, addrs1)
-	fp.nodePortAddresses = utilproxy.NewNodePortAddresses([]string{"100.101.102.0/24"})
+	fp.nodePortAddresses = utilproxy.NewNodePortAddresses(v1.IPv4Protocol, []string{"100.101.102.0/24"})
 
 	fp.syncProxyRules()
 

pkg/proxy/util/nodeport_addresses.go

@@ -20,7 +20,7 @@ import (
 	"fmt"
 	"net"
 
-	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/api/core/v1"
 	netutils "k8s.io/utils/net"
 )
 
@@ -30,32 +30,52 @@ type NodePortAddresses struct {
 
 	cidrs                []*net.IPNet
 	containsIPv4Loopback bool
+	matchAll             bool
 }
 
 // RFC 5735 127.0.0.0/8 - This block is assigned for use as the Internet host loopback address
 var ipv4LoopbackStart = net.IPv4(127, 0, 0, 0)
 
-// NewNodePortAddresses takes the `--nodeport-addresses` value (which is assumed to
+// NewNodePortAddresses takes an IP family and the `--nodeport-addresses` value (which is
-// contain only valid CIDRs) and returns a NodePortAddresses object. If cidrStrings is
+// assumed to contain only valid CIDRs, potentially of both IP families) and returns a
-// empty, this is treated as `["0.0.0.0/0", "::/0"]`.
+// NodePortAddresses object for the given family. If there are no CIDRs of the given
-func NewNodePortAddresses(cidrStrings []string) *NodePortAddresses {
+// family then the CIDR "0.0.0.0/0" or "::/0" will be added (even if there are CIDRs of
-	if len(cidrStrings) == 0 {
+// the other family).
-		cidrStrings = []string{IPv4ZeroCIDR, IPv6ZeroCIDR}
+func NewNodePortAddresses(family v1.IPFamily, cidrStrings []string) *NodePortAddresses {
-	}
+	npa := &NodePortAddresses{}
-
+
-	npa := &NodePortAddresses{
+	// Filter CIDRs to correct family
-		cidrStrings: cidrStrings,
+	for _, str := range cidrStrings {
+		if (family == v1.IPv4Protocol) == netutils.IsIPv4CIDRString(str) {
+			npa.cidrStrings = append(npa.cidrStrings, str)
+		}
+	}
+	if len(npa.cidrStrings) == 0 {
+		if family == v1.IPv4Protocol {
+			npa.cidrStrings = []string{IPv4ZeroCIDR}
+		} else {
+			npa.cidrStrings = []string{IPv6ZeroCIDR}
+		}
 	}
 
+	// Now parse
 	for _, str := range npa.cidrStrings {
 		_, cidr, _ := netutils.ParseCIDRSloppy(str)
-		npa.cidrs = append(npa.cidrs, cidr)
 
 		if netutils.IsIPv4CIDR(cidr) {
 			if cidr.IP.IsLoopback() || cidr.Contains(ipv4LoopbackStart) {
 				npa.containsIPv4Loopback = true
 			}
 		}
+
+		if IsZeroCIDR(str) {
+			// Ignore everything else
+			npa.cidrs = []*net.IPNet{cidr}
+			npa.matchAll = true
+			break
+		}
+
+		npa.cidrs = append(npa.cidrs, cidr)
 	}
 
 	return npa
@@ -65,32 +85,23 @@ func (npa *NodePortAddresses) String() string {
 	return fmt.Sprintf("%v", npa.cidrStrings)
 }
 
-// GetNodeAddresses return all matched node IP addresses for npa's CIDRs.
+// MatchAll returns true if npa matches all node IPs (of npa's given family)
-// If npa's CIDRs include "0.0.0.0/0" and/or "::/0", then those values will be returned
+func (npa *NodePortAddresses) MatchAll() bool {
-// verbatim in the response and no actual IPs of that family will be returned.
+	return npa.matchAll
-// If no matching IPs are found, GetNodeAddresses will return an error.
+}
+
+// GetNodeIPs return all matched node IP addresses for npa's CIDRs. If no matching
+// IPs are found, it returns an empty list.
 // NetworkInterfacer is injected for test purpose.
-func (npa *NodePortAddresses) GetNodeAddresses(nw NetworkInterfacer) (sets.Set[string], error) {
+func (npa *NodePortAddresses) GetNodeIPs(nw NetworkInterfacer) ([]net.IP, error) {
-	uniqueAddressList := sets.New[string]()
-
-	// First round of iteration to pick out `0.0.0.0/0` or `::/0` for the sake of excluding non-zero IPs.
-	for _, cidr := range npa.cidrStrings {
-		if IsZeroCIDR(cidr) {
-			uniqueAddressList.Insert(cidr)
-		}
-	}
-
 	addrs, err := nw.InterfaceAddrs()
 	if err != nil {
 		return nil, fmt.Errorf("error listing all interfaceAddrs from host, error: %v", err)
 	}
 
-	// Second round of iteration to parse IPs based on cidr.
+	// Use a map to dedup matches
+	addresses := make(map[string]net.IP)
 	for _, cidr := range npa.cidrs {
-		if IsZeroCIDR(cidr.String()) {
-			continue
-		}
-
 		for _, addr := range addrs {
 			var ip net.IP
 			// nw.InterfaceAddrs may return net.IPAddr or net.IPNet on windows, and it will return net.IPNet on linux.
@@ -104,21 +115,17 @@ func (npa *NodePortAddresses) GetNodeAddresses(nw NetworkInterfacer) (sets.Set[s
 			}
 
 			if cidr.Contains(ip) {
-				if netutils.IsIPv6(ip) && !uniqueAddressList.Has(IPv6ZeroCIDR) {
+				addresses[ip.String()] = ip
-					uniqueAddressList.Insert(ip.String())
-				}
-				if !netutils.IsIPv6(ip) && !uniqueAddressList.Has(IPv4ZeroCIDR) {
-					uniqueAddressList.Insert(ip.String())
-				}
 			}
 		}
 	}
 
-	if uniqueAddressList.Len() == 0 {
+	ips := make([]net.IP, 0, len(addresses))
-		return nil, fmt.Errorf("no addresses found for cidrs %v", npa.cidrStrings)
+	for _, ip := range addresses {
+		ips = append(ips, ip)
 	}
 
-	return uniqueAddressList, nil
+	return ips, nil
 }
 
 // ContainsIPv4Loopback returns true if npa's CIDRs contain an IPv4 loopback address.

pkg/proxy/util/nodeport_addresses_test.go

@@ -17,9 +17,11 @@ limitations under the License.
 package util
 
 import (
+	"fmt"
 	"net"
 	"testing"
 
+	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	fake "k8s.io/kubernetes/pkg/proxy/util/testing"
 	netutils "k8s.io/utils/net"
@@ -30,12 +32,34 @@ type InterfaceAddrsPair struct {
 	addrs []net.Addr
 }
 
-func TestGetNodeAddresses(t *testing.T) {
+func checkNodeIPs(expected sets.Set[string], actual []net.IP) error {
+	notFound := expected.Clone()
+	extra := sets.New[string]()
+	for _, ip := range actual {
+		str := ip.String()
+		if notFound.Has(str) {
+			notFound.Delete(str)
+		} else {
+			extra.Insert(str)
+		}
+	}
+	if len(notFound) != 0 || len(extra) != 0 {
+		return fmt.Errorf("not found: %v, extra: %v", notFound.UnsortedList(), extra.UnsortedList())
+	}
+	return nil
+}
+
+func TestGetNodeIPs(t *testing.T) {
+	type expectation struct {
+		matchAll bool
+		ips      sets.Set[string]
+	}
+
 	testCases := []struct {
 		name          string
 		cidrs         []string
 		itfAddrsPairs []InterfaceAddrsPair
-		expected      sets.Set[string]
+		expected      map[v1.IPFamily]expectation
 	}{
 		{
 			name:  "IPv4 single",
@@ -50,7 +74,15 @@ func TestGetNodeAddresses(t *testing.T) {
 					addrs: []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("100.200.201.1"), Mask: net.CIDRMask(24, 32)}},
 				},
 			},
-			expected: sets.New[string]("10.20.30.51"),
+			expected: map[v1.IPFamily]expectation{
+				v1.IPv4Protocol: {
+					ips: sets.New[string]("10.20.30.51"),
+				},
+				v1.IPv6Protocol: {
+					matchAll: true,
+					ips:      nil,
+				},
+			},
 		},
 		{
 			name:  "IPv4 zero CIDR",
@@ -65,7 +97,16 @@ func TestGetNodeAddresses(t *testing.T) {
 					addrs: []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("127.0.0.1"), Mask: net.CIDRMask(8, 32)}},
 				},
 			},
-			expected: sets.New[string]("0.0.0.0/0"),
+			expected: map[v1.IPFamily]expectation{
+				v1.IPv4Protocol: {
+					matchAll: true,
+					ips:      sets.New[string]("10.20.30.51", "127.0.0.1"),
+				},
+				v1.IPv6Protocol: {
+					matchAll: true,
+					ips:      nil,
+				},
+			},
 		},
 		{
 			name:  "IPv6 multiple",
@@ -80,7 +121,15 @@ func TestGetNodeAddresses(t *testing.T) {
 					addrs: []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("::1"), Mask: net.CIDRMask(128, 128)}},
 				},
 			},
-			expected: sets.New[string]("2001:db8::1", "::1"),
+			expected: map[v1.IPFamily]expectation{
+				v1.IPv4Protocol: {
+					matchAll: true,
+					ips:      nil,
+				},
+				v1.IPv6Protocol: {
+					ips: sets.New[string]("2001:db8::1", "::1"),
+				},
+			},
 		},
 		{
 			name:  "IPv6 zero CIDR",
@@ -95,7 +144,16 @@ func TestGetNodeAddresses(t *testing.T) {
 					addrs: []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("::1"), Mask: net.CIDRMask(128, 128)}},
 				},
 			},
-			expected: sets.New[string]("::/0"),
+			expected: map[v1.IPFamily]expectation{
+				v1.IPv4Protocol: {
+					matchAll: true,
+					ips:      nil,
+				},
+				v1.IPv6Protocol: {
+					matchAll: true,
+					ips:      sets.New[string]("2001:db8::1", "::1"),
+				},
+			},
 		},
 		{
 			name:  "IPv4 localhost exact",
@@ -110,7 +168,15 @@ func TestGetNodeAddresses(t *testing.T) {
 					addrs: []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("127.0.0.1"), Mask: net.CIDRMask(8, 32)}},
 				},
 			},
-			expected: sets.New[string]("127.0.0.1"),
+			expected: map[v1.IPFamily]expectation{
+				v1.IPv4Protocol: {
+					ips: sets.New[string]("127.0.0.1"),
+				},
+				v1.IPv6Protocol: {
+					matchAll: true,
+					ips:      nil,
+				},
+			},
 		},
 		{
 			name:  "IPv4 localhost subnet",
@@ -121,7 +187,15 @@ func TestGetNodeAddresses(t *testing.T) {
 					addrs: []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("127.0.1.1"), Mask: net.CIDRMask(8, 32)}},
 				},
 			},
-			expected: sets.New[string]("127.0.1.1"),
+			expected: map[v1.IPFamily]expectation{
+				v1.IPv4Protocol: {
+					ips: sets.New[string]("127.0.1.1"),
+				},
+				v1.IPv6Protocol: {
+					matchAll: true,
+					ips:      nil,
+				},
+			},
 		},
 		{
 			name:  "IPv4 multiple",
@@ -136,7 +210,15 @@ func TestGetNodeAddresses(t *testing.T) {
 					addrs: []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("100.200.201.1"), Mask: net.CIDRMask(24, 32)}},
 				},
 			},
-			expected: sets.New[string]("10.20.30.51", "100.200.201.1"),
+			expected: map[v1.IPFamily]expectation{
+				v1.IPv4Protocol: {
+					ips: sets.New[string]("10.20.30.51", "100.200.201.1"),
+				},
+				v1.IPv6Protocol: {
+					matchAll: true,
+					ips:      nil,
+				},
+			},
 		},
 		{
 			name:  "IPv4 multiple, no match",
@@ -151,7 +233,15 @@ func TestGetNodeAddresses(t *testing.T) {
 					addrs: []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("127.0.0.1"), Mask: net.CIDRMask(8, 32)}},
 				},
 			},
-			expected: nil,
+			expected: map[v1.IPFamily]expectation{
+				v1.IPv4Protocol: {
+					ips: nil,
+				},
+				v1.IPv6Protocol: {
+					matchAll: true,
+					ips:      nil,
+				},
+			},
 		},
 		{
 			name:  "empty list, IPv4 addrs",
@@ -166,7 +256,16 @@ func TestGetNodeAddresses(t *testing.T) {
 					addrs: []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("127.0.0.1"), Mask: net.CIDRMask(8, 32)}},
 				},
 			},
-			expected: sets.New[string]("0.0.0.0/0", "::/0"),
+			expected: map[v1.IPFamily]expectation{
+				v1.IPv4Protocol: {
+					matchAll: true,
+					ips:      sets.New[string]("192.168.1.2", "127.0.0.1"),
+				},
+				v1.IPv6Protocol: {
+					matchAll: true,
+					ips:      nil,
+				},
+			},
 		},
 		{
 			name:  "empty list, IPv6 addrs",
@@ -181,7 +280,16 @@ func TestGetNodeAddresses(t *testing.T) {
 					addrs: []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("::1"), Mask: net.CIDRMask(128, 128)}},
 				},
 			},
-			expected: sets.New[string]("0.0.0.0/0", "::/0"),
+			expected: map[v1.IPFamily]expectation{
+				v1.IPv4Protocol: {
+					matchAll: true,
+					ips:      nil,
+				},
+				v1.IPv6Protocol: {
+					matchAll: true,
+					ips:      sets.New[string]("2001:db8::1", "::1"),
+				},
+			},
 		},
 		{
 			name:  "IPv4 redundant CIDRs",
@@ -192,7 +300,16 @@ func TestGetNodeAddresses(t *testing.T) {
 					addrs: []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("1.2.3.4"), Mask: net.CIDRMask(30, 32)}},
 				},
 			},
-			expected: sets.New[string]("0.0.0.0/0"),
+			expected: map[v1.IPFamily]expectation{
+				v1.IPv4Protocol: {
+					matchAll: true,
+					ips:      sets.New[string]("1.2.3.4"),
+				},
+				v1.IPv6Protocol: {
+					matchAll: true,
+					ips:      nil,
+				},
+			},
 		},
 		{
 			name:  "Dual-stack, redundant IPv4",
@@ -213,7 +330,15 @@ func TestGetNodeAddresses(t *testing.T) {
 					},
 				},
 			},
-			expected: sets.New[string]("0.0.0.0/0", "2001:db8::1"),
+			expected: map[v1.IPFamily]expectation{
+				v1.IPv4Protocol: {
+					matchAll: true,
+					ips:      sets.New[string]("1.2.3.4", "127.0.0.1"),
+				},
+				v1.IPv6Protocol: {
+					ips: sets.New[string]("2001:db8::1"),
+				},
+			},
 		},
 		{
 			name:  "Dual-stack, redundant IPv6",
@@ -234,7 +359,15 @@ func TestGetNodeAddresses(t *testing.T) {
 					},
 				},
 			},
-			expected: sets.New[string]("::/0", "1.2.3.4"),
+			expected: map[v1.IPFamily]expectation{
+				v1.IPv4Protocol: {
+					ips: sets.New[string]("1.2.3.4"),
+				},
+				v1.IPv6Protocol: {
+					matchAll: true,
+					ips:      sets.New[string]("2001:db8::1", "::1"),
+				},
+			},
 		},
 	}
 
@@ -245,16 +378,26 @@ func TestGetNodeAddresses(t *testing.T) {
 				nw.AddInterfaceAddr(&pair.itf, pair.addrs)
 			}
 
-			npa := NewNodePortAddresses(tc.cidrs)
+			for _, family := range []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol} {
-			addrList, err := npa.GetNodeAddresses(nw)
+				npa := NewNodePortAddresses(family, tc.cidrs)
-			// The fake InterfaceAddrs() never returns an error, so the only
-			// error GetNodeAddresses will return is "no addresses found".
-			if err != nil && tc.expected != nil {
-				t.Errorf("unexpected error: %v", err)
-			}
 
-			if !addrList.Equal(tc.expected) {
+				if npa.MatchAll() != tc.expected[family].matchAll {
-				t.Errorf("unexpected mismatch, expected: %v, got: %v", tc.expected, addrList)
+					t.Errorf("unexpected MatchAll(%s), expected: %v", family, tc.expected[family].matchAll)
+				}
+
+				ips, err := npa.GetNodeIPs(nw)
+				expectedIPs := tc.expected[family].ips
+
+				// The fake InterfaceAddrs() never returns an error, so
+				// the only error GetNodeIPs will return is "no
+				// addresses found".
+				if err != nil {
+					t.Errorf("unexpected error: %v", err)
+				}
+				err = checkNodeIPs(expectedIPs, ips)
+				if err != nil {
+					t.Errorf("unexpected mismatch for %s: %v", family, err)
+				}
 			}
 		})
 	}
@@ -308,9 +451,14 @@ func TestContainsIPv4Loopback(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			npa := NewNodePortAddresses(tt.cidrStrings)
+			npa := NewNodePortAddresses(v1.IPv4Protocol, tt.cidrStrings)
 			if got := npa.ContainsIPv4Loopback(); got != tt.want {
-				t.Errorf("ContainsIPv4Loopback() = %v, want %v", got, tt.want)
+				t.Errorf("IPv4 ContainsIPv4Loopback() = %v, want %v", got, tt.want)
+			}
+			// ContainsIPv4Loopback should always be false for family=IPv6
+			npa = NewNodePortAddresses(v1.IPv6Protocol, tt.cidrStrings)
+			if got := npa.ContainsIPv4Loopback(); got {
+				t.Errorf("IPv6 ContainsIPv4Loopback() = %v, want %v", got, false)
 			}
 		})
 	}

pkg/proxy/winkernel/proxier.go

@@ -685,8 +685,14 @@ func NewProxier(
 		klog.InfoS("ClusterCIDR not specified, unable to distinguish between internal and external traffic")
 	}
 
+	isIPv6 := netutils.IsIPv6(nodeIP)
+	ipFamily := v1.IPv4Protocol
+	if isIPv6 {
+		ipFamily = v1.IPv6Protocol
+	}
+
 	// windows listens to all node addresses
-	nodePortAddresses := utilproxy.NewNodePortAddresses(nil)
+	nodePortAddresses := utilproxy.NewNodePortAddresses(ipFamily, nil)
 	serviceHealthServer := healthcheck.NewServiceHealthServer(hostname, recorder, nodePortAddresses, healthzServer)
 
 	hns, supportedFeatures := newHostNetworkService()
@@ -764,7 +770,6 @@ func NewProxier(
 		}
 	}
 
-	isIPv6 := netutils.IsIPv6(nodeIP)
 	proxier := &Proxier{
 		endPointsRefCount:     make(endPointsReferenceCountMap),
 		svcPortMap:            make(proxy.ServicePortMap),
@@ -788,10 +793,6 @@ func NewProxier(
 		mapStaleLoadbalancers: make(map[string]bool),
 	}
 
-	ipFamily := v1.IPv4Protocol
-	if isIPv6 {
-		ipFamily = v1.IPv6Protocol
-	}
 	serviceChanges := proxy.NewServiceChangeTracker(proxier.newServiceInfo, ipFamily, recorder, proxier.serviceMapChange)
 	endPointChangeTracker := proxy.NewEndpointChangeTracker(hostname, proxier.newEndpointInfo, ipFamily, recorder, proxier.endpointsMapChange)
 	proxier.endpointsChanges = endPointChangeTracker