From 9ac657bb94e076d147603c570d3b359863c3cd39 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sat, 14 Jan 2023 16:54:56 -0500 Subject: [PATCH] Make NodePortAddresses explicitly IP-family-specific Both proxies handle IPv4 and IPv6 nodeport addresses separately, but GetNodeAddresses went out of its way to make that difficult. Fix that. This commit does not change any externally-visible semantics, but it makes the existing weird semantics more obvious. Specifically, if you say "--nodeport-addresses 10.0.0.0/8,192.168.0.0/16", then the dual-stack proxy code would have split that into a list of IPv4 CIDRs (["10.0.0.0/8", "192.168.0.0/16"]) to pass to the IPv4 proxier, and a list of IPv6 CIDRs ([]) to pass to the IPv6 proxier, and then the IPv6 proxier would say "well since the list of nodeport addresses is empty, I'll listen on all IPv6 addresses", which probably isn't what you meant, but that's what it did. --- pkg/proxy/healthcheck/healthcheck_test.go | 5 +- pkg/proxy/iptables/proxier.go | 22 +-- pkg/proxy/iptables/proxier_test.go | 13 +- pkg/proxy/ipvs/proxier.go | 12 +- pkg/proxy/ipvs/proxier_test.go | 10 +- pkg/proxy/util/nodeport_addresses.go | 62 +++++---- pkg/proxy/util/nodeport_addresses_test.go | 155 ++++++++++++++++++---- pkg/proxy/winkernel/proxier.go | 13 +- 8 files changed, 196 insertions(+), 96 deletions(-) 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