diff --git a/pkg/proxy/healthcheck/healthcheck_test.go b/pkg/proxy/healthcheck/healthcheck_test.go index bbb7489e67d..4d6734145d6 100644 --- a/pkg/proxy/healthcheck/healthcheck_test.go +++ b/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) @@ -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) diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 68f259c0e6a..eb50769daf4 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/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) } @@ -1420,15 +1419,6 @@ func (proxier *Proxier) syncProxyRules() { 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) } - // nodeAddresses may contain dual-stack zero-CIDRs if proxier.nodePortAddresses is empty. - // Ensure nodeAddresses only contains the addresses for this proxier's IP family. - for addr := range nodeAddresses { - if utilproxy.IsZeroCIDR(addr) && isIPv6 == netutils.IsIPv6CIDRString(addr) { - // 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) { @@ -1455,12 +1445,6 @@ func (proxier *Proxier) syncProxyRules() { 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) { diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index 328edf8efba..030ab07d828 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/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(` diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index c2c7d316277..b4fc92d3023 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/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) } @@ -1024,9 +1022,7 @@ func (proxier *Proxier) syncProxyRules() { } break } - if getIPFamily(a) == proxier.ipFamily { - nodeIPs = append(nodeIPs, a) - } + nodeIPs = append(nodeIPs, a) } } } diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index f03d805325e..6ef2756dd27 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/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() diff --git a/pkg/proxy/util/nodeport_addresses.go b/pkg/proxy/util/nodeport_addresses.go index fc03f6bfde1..44eabebc4f7 100644 --- a/pkg/proxy/util/nodeport_addresses.go +++ b/pkg/proxy/util/nodeport_addresses.go @@ -20,6 +20,7 @@ import ( "fmt" "net" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" netutils "k8s.io/utils/net" ) @@ -35,27 +36,45 @@ type NodePortAddresses struct { // 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 -// contain only valid CIDRs) and returns a NodePortAddresses object. If cidrStrings is -// empty, this is treated as `["0.0.0.0/0", "::/0"]`. -func NewNodePortAddresses(cidrStrings []string) *NodePortAddresses { - if len(cidrStrings) == 0 { - cidrStrings = []string{IPv4ZeroCIDR, IPv6ZeroCIDR} - } - - npa := &NodePortAddresses{ - cidrStrings: cidrStrings, +// NewNodePortAddresses takes an IP family and the `--nodeport-addresses` value (which is +// assumed to contain only valid CIDRs, potentially of both IP families) and returns a +// NodePortAddresses object for the given family. If there are no CIDRs of the given +// family then the CIDR "0.0.0.0/0" or "::/0" will be added (even if there are CIDRs of +// the other family). +func NewNodePortAddresses(family v1.IPFamily, cidrStrings []string) *NodePortAddresses { + npa := &NodePortAddresses{} + + // Filter CIDRs to correct family + 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} + break + } + + npa.cidrs = append(npa.cidrs, cidr) } return npa @@ -65,10 +84,10 @@ func (npa *NodePortAddresses) String() string { return fmt.Sprintf("%v", npa.cidrStrings) } -// GetNodeAddresses return all matched node IP addresses for npa's CIDRs. -// If npa's CIDRs include "0.0.0.0/0" and/or "::/0", then those values will be returned -// verbatim in the response and no actual IPs of that family will be returned. -// If no matching IPs are found, GetNodeAddresses will return an error. +// GetNodeAddresses returns all matched node IP addresses for npa's IP family. If npa's +// CIDRs include "0.0.0.0/0" or "::/0", then that value will be returned verbatim in +// the response and no actual IPs of that family will be returned. If no matching IPs are +// found, GetNodeAddresses will return an error. // NetworkInterfacer is injected for test purpose. func (npa *NodePortAddresses) GetNodeAddresses(nw NetworkInterfacer) (sets.Set[string], error) { uniqueAddressList := sets.New[string]() @@ -77,6 +96,7 @@ func (npa *NodePortAddresses) GetNodeAddresses(nw NetworkInterfacer) (sets.Set[s for _, cidr := range npa.cidrStrings { if IsZeroCIDR(cidr) { uniqueAddressList.Insert(cidr) + return uniqueAddressList, nil } } @@ -85,12 +105,7 @@ func (npa *NodePortAddresses) GetNodeAddresses(nw NetworkInterfacer) (sets.Set[s return nil, fmt.Errorf("error listing all interfaceAddrs from host, error: %v", err) } - // Second round of iteration to parse IPs based on cidr. 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,12 +119,7 @@ func (npa *NodePortAddresses) GetNodeAddresses(nw NetworkInterfacer) (sets.Set[s } if cidr.Contains(ip) { - if netutils.IsIPv6(ip) && !uniqueAddressList.Has(IPv6ZeroCIDR) { - uniqueAddressList.Insert(ip.String()) - } - if !netutils.IsIPv6(ip) && !uniqueAddressList.Has(IPv4ZeroCIDR) { - uniqueAddressList.Insert(ip.String()) - } + uniqueAddressList.Insert(ip.String()) } } } diff --git a/pkg/proxy/util/nodeport_addresses_test.go b/pkg/proxy/util/nodeport_addresses_test.go index aba782fce13..d6cf286af07 100644 --- a/pkg/proxy/util/nodeport_addresses_test.go +++ b/pkg/proxy/util/nodeport_addresses_test.go @@ -20,6 +20,7 @@ import ( "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" @@ -31,11 +32,15 @@ type InterfaceAddrsPair struct { } func TestGetNodeAddresses(t *testing.T) { + type expectation struct { + 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 +55,14 @@ 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: { + ips: sets.New[string]("::/0"), + }, + }, }, { name: "IPv4 zero CIDR", @@ -65,7 +77,14 @@ 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: { + ips: sets.New[string]("0.0.0.0/0"), + }, + v1.IPv6Protocol: { + ips: sets.New[string]("::/0"), + }, + }, }, { name: "IPv6 multiple", @@ -80,7 +99,14 @@ 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: { + ips: sets.New[string]("0.0.0.0/0"), + }, + v1.IPv6Protocol: { + ips: sets.New[string]("2001:db8::1", "::1"), + }, + }, }, { name: "IPv6 zero CIDR", @@ -95,7 +121,14 @@ 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: { + ips: sets.New[string]("0.0.0.0/0"), + }, + v1.IPv6Protocol: { + ips: sets.New[string]("::/0"), + }, + }, }, { name: "IPv4 localhost exact", @@ -110,7 +143,14 @@ 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: { + ips: sets.New[string]("::/0"), + }, + }, }, { name: "IPv4 localhost subnet", @@ -121,7 +161,14 @@ 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: { + ips: sets.New[string]("::/0"), + }, + }, }, { name: "IPv4 multiple", @@ -136,7 +183,14 @@ 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: { + ips: sets.New[string]("::/0"), + }, + }, }, { name: "IPv4 multiple, no match", @@ -151,7 +205,14 @@ 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: { + ips: sets.New[string]("::/0"), + }, + }, }, { name: "empty list, IPv4 addrs", @@ -166,7 +227,14 @@ 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: { + ips: sets.New[string]("0.0.0.0/0"), + }, + v1.IPv6Protocol: { + ips: sets.New[string]("::/0"), + }, + }, }, { name: "empty list, IPv6 addrs", @@ -181,7 +249,14 @@ 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: { + ips: sets.New[string]("0.0.0.0/0"), + }, + v1.IPv6Protocol: { + ips: sets.New[string]("::/0"), + }, + }, }, { name: "IPv4 redundant CIDRs", @@ -192,7 +267,14 @@ 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: { + ips: sets.New[string]("0.0.0.0/0"), + }, + v1.IPv6Protocol: { + ips: sets.New[string]("::/0"), + }, + }, }, { name: "Dual-stack, redundant IPv4", @@ -213,7 +295,14 @@ func TestGetNodeAddresses(t *testing.T) { }, }, }, - expected: sets.New[string]("0.0.0.0/0", "2001:db8::1"), + expected: map[v1.IPFamily]expectation{ + v1.IPv4Protocol: { + ips: sets.New[string]("0.0.0.0/0"), + }, + v1.IPv6Protocol: { + ips: sets.New[string]("2001:db8::1"), + }, + }, }, { name: "Dual-stack, redundant IPv6", @@ -234,7 +323,14 @@ 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: { + ips: sets.New[string]("::/0"), + }, + }, }, } @@ -245,16 +341,20 @@ func TestGetNodeAddresses(t *testing.T) { nw.AddInterfaceAddr(&pair.itf, pair.addrs) } - npa := NewNodePortAddresses(tc.cidrs) - addrList, err := npa.GetNodeAddresses(nw) - // 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) - } + for _, family := range []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol} { + npa := NewNodePortAddresses(family, tc.cidrs) - if !addrList.Equal(tc.expected) { - t.Errorf("unexpected mismatch, expected: %v, got: %v", tc.expected, addrList) + addrList, err := npa.GetNodeAddresses(nw) + // The fake InterfaceAddrs() never returns an error, so + // the only error GetNodeAddresses will return is "no + // addresses found". + if err != nil && tc.expected[family].ips != nil { + t.Errorf("unexpected error: %v", err) + } + + if !addrList.Equal(tc.expected[family].ips) { + t.Errorf("unexpected mismatch for %s, expected: %v, got: %v", family, tc.expected[family].ips, addrList) + } } }) } @@ -308,9 +408,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) } }) } diff --git a/pkg/proxy/winkernel/proxier.go b/pkg/proxy/winkernel/proxier.go index 8f0dde7d6ea..2f12451896c 100644 --- a/pkg/proxy/winkernel/proxier.go +++ b/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