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.
This commit is contained in:
Dan Winship 2023-01-14 16:54:56 -05:00
parent f7bb9a9a0a
commit 9ac657bb94
8 changed files with 196 additions and 96 deletions

View File

@ -24,6 +24,7 @@ import (
"testing" "testing"
"time" "time"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/dump" "k8s.io/apimachinery/pkg/util/dump"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
@ -140,7 +141,7 @@ func (fake fakeProxierHealthChecker) IsHealthy() bool {
func TestServer(t *testing.T) { func TestServer(t *testing.T) {
listener := newFakeListener() listener := newFakeListener()
httpFactory := newFakeHTTPServerFactory() httpFactory := newFakeHTTPServerFactory()
nodePortAddresses := utilproxy.NewNodePortAddresses([]string{}) nodePortAddresses := utilproxy.NewNodePortAddresses(v1.IPv4Protocol, []string{})
proxyChecker := &fakeProxierHealthChecker{true} proxyChecker := &fakeProxierHealthChecker{true}
hcsi := newServiceHealthServer("hostname", nil, listener, httpFactory, nodePortAddresses, proxyChecker) 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 // 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 // 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) hcsi := newServiceHealthServer("hostname", nil, listener, httpFactory, nodePortAddresses, proxyChecker)
hcs := hcsi.(*server) hcs := hcsi.(*server)

View File

@ -240,7 +240,7 @@ func NewProxier(ipFamily v1.IPFamily,
healthzServer healthcheck.ProxierHealthUpdater, healthzServer healthcheck.ProxierHealthUpdater,
nodePortAddressStrings []string, nodePortAddressStrings []string,
) (*Proxier, error) { ) (*Proxier, error) {
nodePortAddresses := utilproxy.NewNodePortAddresses(nodePortAddressStrings) nodePortAddresses := utilproxy.NewNodePortAddresses(ipFamily, nodePortAddressStrings)
if !nodePortAddresses.ContainsIPv4Loopback() { if !nodePortAddresses.ContainsIPv4Loopback() {
localhostNodePorts = false localhostNodePorts = false
@ -334,17 +334,16 @@ func NewDualStackProxier(
nodePortAddresses []string, nodePortAddresses []string,
) (proxy.Provider, error) { ) (proxy.Provider, error) {
// Create an ipv4 instance of the single-stack proxier // Create an ipv4 instance of the single-stack proxier
ipFamilyMap := utilproxy.MapCIDRsByIPFamily(nodePortAddresses)
ipv4Proxier, err := NewProxier(v1.IPv4Protocol, ipt[0], sysctl, ipv4Proxier, err := NewProxier(v1.IPv4Protocol, ipt[0], sysctl,
exec, syncPeriod, minSyncPeriod, masqueradeAll, localhostNodePorts, masqueradeBit, localDetectors[0], hostname, exec, syncPeriod, minSyncPeriod, masqueradeAll, localhostNodePorts, masqueradeBit, localDetectors[0], hostname,
nodeIP[0], recorder, healthzServer, ipFamilyMap[v1.IPv4Protocol]) nodeIP[0], recorder, healthzServer, nodePortAddresses)
if err != nil { if err != nil {
return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err) return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err)
} }
ipv6Proxier, err := NewProxier(v1.IPv6Protocol, ipt[1], sysctl, ipv6Proxier, err := NewProxier(v1.IPv6Protocol, ipt[1], sysctl,
exec, syncPeriod, minSyncPeriod, masqueradeAll, false, masqueradeBit, localDetectors[1], hostname, exec, syncPeriod, minSyncPeriod, masqueradeAll, false, masqueradeBit, localDetectors[1], hostname,
nodeIP[1], recorder, healthzServer, ipFamilyMap[v1.IPv6Protocol]) nodeIP[1], recorder, healthzServer, nodePortAddresses)
if err != nil { if err != nil {
return nil, fmt.Errorf("unable to create ipv6 proxier: %v", err) return nil, fmt.Errorf("unable to create ipv6 proxier: %v", err)
} }
@ -1420,15 +1419,6 @@ func (proxier *Proxier) syncProxyRules() {
if err != nil { 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) 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 { for address := range nodeAddresses {
if utilproxy.IsZeroCIDR(address) { if utilproxy.IsZeroCIDR(address) {
@ -1455,12 +1445,6 @@ func (proxier *Proxier) syncProxyRules() {
break 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 // For ipv6, Regardless of the value of localhostNodePorts is true or false, we should disallow access
// to the nodePort via lookBack address. // to the nodePort via lookBack address.
if isIPv6 && utilproxy.IsLoopBack(address) { if isIPv6 && utilproxy.IsLoopBack(address) {

View File

@ -303,6 +303,9 @@ func NewFakeProxier(ipt utiliptables.Interface) *Proxier {
itf1 := net.Interface{Index: 1, MTU: 0, Name: "eth0", HardwareAddr: nil, Flags: 0} itf1 := net.Interface{Index: 1, MTU: 0, Name: "eth0", HardwareAddr: nil, Flags: 0}
addrs1 := []net.Addr{ addrs1 := []net.Addr{
&net.IPNet{IP: netutils.ParseIPSloppy(testNodeIP), Mask: net.CIDRMask(24, 32)}, &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) networkInterfacer.AddInterfaceAddr(&itf1, addrs1)
@ -327,7 +330,7 @@ func NewFakeProxier(ipt utiliptables.Interface) *Proxier {
natRules: utilproxy.LineBuffer{}, natRules: utilproxy.LineBuffer{},
nodeIP: netutils.ParseIPSloppy(testNodeIP), nodeIP: netutils.ParseIPSloppy(testNodeIP),
localhostNodePorts: true, localhostNodePorts: true,
nodePortAddresses: utilproxy.NewNodePortAddresses(nil), nodePortAddresses: utilproxy.NewNodePortAddresses(ipfamily, nil),
networkInterfacer: networkInterfacer, networkInterfacer: networkInterfacer,
} }
p.setInitialized(true) p.setInitialized(true)
@ -2461,7 +2464,7 @@ func TestNodePort(t *testing.T) {
func TestHealthCheckNodePort(t *testing.T) { func TestHealthCheckNodePort(t *testing.T) {
ipt := iptablestest.NewFake() ipt := iptablestest.NewFake()
fp := NewFakeProxier(ipt) 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" svcIP := "172.30.0.42"
svcPort := 80 svcPort := 80
@ -3390,7 +3393,7 @@ func TestDisableLocalhostNodePortsIPv4WithNodeAddress(t *testing.T) {
fp.localDetector = proxyutiliptables.NewNoOpLocalDetector() fp.localDetector = proxyutiliptables.NewNoOpLocalDetector()
fp.localhostNodePorts = false fp.localhostNodePorts = false
fp.networkInterfacer.InterfaceAddrs() 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(` expected := dedent.Dedent(`
*filter *filter
@ -3671,7 +3674,7 @@ func TestOnlyLocalNodePortsNoClusterCIDR(t *testing.T) {
ipt := iptablestest.NewFake() ipt := iptablestest.NewFake()
fp := NewFakeProxier(ipt) fp := NewFakeProxier(ipt)
fp.localDetector = proxyutiliptables.NewNoOpLocalDetector() 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 fp.localhostNodePorts = false
expected := dedent.Dedent(` expected := dedent.Dedent(`
@ -3720,7 +3723,7 @@ func TestOnlyLocalNodePortsNoClusterCIDR(t *testing.T) {
func TestOnlyLocalNodePorts(t *testing.T) { func TestOnlyLocalNodePorts(t *testing.T) {
ipt := iptablestest.NewFake() ipt := iptablestest.NewFake()
fp := NewFakeProxier(ipt) 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 fp.localhostNodePorts = false
expected := dedent.Dedent(` expected := dedent.Dedent(`

View File

@ -409,7 +409,7 @@ func NewProxier(ipFamily v1.IPFamily,
scheduler = defaultScheduler scheduler = defaultScheduler
} }
nodePortAddresses := utilproxy.NewNodePortAddresses(nodePortAddressStrings) nodePortAddresses := utilproxy.NewNodePortAddresses(ipFamily, nodePortAddressStrings)
serviceHealthServer := healthcheck.NewServiceHealthServer(hostname, recorder, nodePortAddresses, healthzServer) serviceHealthServer := healthcheck.NewServiceHealthServer(hostname, recorder, nodePortAddresses, healthzServer)
@ -490,14 +490,12 @@ func NewDualStackProxier(
safeIpset := newSafeIpset(ipset) safeIpset := newSafeIpset(ipset)
ipFamilyMap := utilproxy.MapCIDRsByIPFamily(nodePortAddresses)
// Create an ipv4 instance of the single-stack proxier // Create an ipv4 instance of the single-stack proxier
ipv4Proxier, err := NewProxier(v1.IPv4Protocol, ipt[0], ipvs, safeIpset, sysctl, ipv4Proxier, err := NewProxier(v1.IPv4Protocol, ipt[0], ipvs, safeIpset, sysctl,
exec, syncPeriod, minSyncPeriod, filterCIDRs(false, excludeCIDRs), strictARP, exec, syncPeriod, minSyncPeriod, filterCIDRs(false, excludeCIDRs), strictARP,
tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit, tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit,
localDetectors[0], hostname, nodeIP[0], localDetectors[0], hostname, nodeIP[0],
recorder, healthzServer, scheduler, ipFamilyMap[v1.IPv4Protocol], kernelHandler) recorder, healthzServer, scheduler, nodePortAddresses, kernelHandler)
if err != nil { if err != nil {
return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err) return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err)
} }
@ -506,7 +504,7 @@ func NewDualStackProxier(
exec, syncPeriod, minSyncPeriod, filterCIDRs(true, excludeCIDRs), strictARP, exec, syncPeriod, minSyncPeriod, filterCIDRs(true, excludeCIDRs), strictARP,
tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit, tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit,
localDetectors[1], hostname, nodeIP[1], localDetectors[1], hostname, nodeIP[1],
recorder, healthzServer, scheduler, ipFamilyMap[v1.IPv6Protocol], kernelHandler) recorder, healthzServer, scheduler, nodePortAddresses, kernelHandler)
if err != nil { if err != nil {
return nil, fmt.Errorf("unable to create ipv6 proxier: %v", err) return nil, fmt.Errorf("unable to create ipv6 proxier: %v", err)
} }
@ -1024,9 +1022,7 @@ func (proxier *Proxier) syncProxyRules() {
} }
break break
} }
if getIPFamily(a) == proxier.ipFamily { nodeIPs = append(nodeIPs, a)
nodeIPs = append(nodeIPs, a)
}
} }
} }
} }

View File

@ -169,7 +169,7 @@ func NewFakeProxier(ipt utiliptables.Interface, ipvs utilipvs.Interface, ipset u
filterRules: utilproxy.LineBuffer{}, filterRules: utilproxy.LineBuffer{},
netlinkHandle: netlinkHandle, netlinkHandle: netlinkHandle,
ipsetList: ipsetList, ipsetList: ipsetList,
nodePortAddresses: utilproxy.NewNodePortAddresses(nil), nodePortAddresses: utilproxy.NewNodePortAddresses(ipFamily, nil),
networkInterfacer: proxyutiltest.NewFakeNetwork(), networkInterfacer: proxyutiltest.NewFakeNetwork(),
gracefuldeleteManager: NewGracefulTerminationManager(ipvs), gracefuldeleteManager: NewGracefulTerminationManager(ipvs),
ipFamily: ipFamily, ipFamily: ipFamily,
@ -960,7 +960,7 @@ func TestNodePortIPv4(t *testing.T) {
ipvs := ipvstest.NewFake() ipvs := ipvstest.NewFake()
ipset := ipsettest.NewFake(testIPSetVersion) ipset := ipsettest.NewFake(testIPSetVersion)
fp := NewFakeProxier(ipt, ipvs, ipset, test.nodeIPs, nil, v1.IPv4Protocol) 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...) makeServiceMap(fp, test.services...)
populateEndpointSlices(fp, test.endpoints...) populateEndpointSlices(fp, test.endpoints...)
@ -1305,7 +1305,7 @@ func TestNodePortIPv6(t *testing.T) {
ipvs := ipvstest.NewFake() ipvs := ipvstest.NewFake()
ipset := ipsettest.NewFake(testIPSetVersion) ipset := ipsettest.NewFake(testIPSetVersion)
fp := NewFakeProxier(ipt, ipvs, ipset, test.nodeIPs, nil, v1.IPv6Protocol) 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...) makeServiceMap(fp, test.services...)
populateEndpointSlices(fp, test.endpoints...) 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)}} 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(&itf, addrs)
fp.networkInterfacer.(*proxyutiltest.FakeNetwork).AddInterfaceAddr(&itf1, addrs1) 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() 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)}} 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(&itf, addrs)
fp.networkInterfacer.(*proxyutiltest.FakeNetwork).AddInterfaceAddr(&itf1, addrs1) 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() fp.syncProxyRules()

View File

@ -20,6 +20,7 @@ import (
"fmt" "fmt"
"net" "net"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
netutils "k8s.io/utils/net" 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 // 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) 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 { for _, str := range npa.cidrStrings {
_, cidr, _ := netutils.ParseCIDRSloppy(str) _, cidr, _ := netutils.ParseCIDRSloppy(str)
npa.cidrs = append(npa.cidrs, cidr)
if netutils.IsIPv4CIDR(cidr) { if netutils.IsIPv4CIDR(cidr) {
if cidr.IP.IsLoopback() || cidr.Contains(ipv4LoopbackStart) { if cidr.IP.IsLoopback() || cidr.Contains(ipv4LoopbackStart) {
npa.containsIPv4Loopback = true npa.containsIPv4Loopback = true
} }
} }
if IsZeroCIDR(str) {
// Ignore everything else
npa.cidrs = []*net.IPNet{cidr}
break
}
npa.cidrs = append(npa.cidrs, cidr)
} }
return npa return npa
@ -65,10 +84,10 @@ func (npa *NodePortAddresses) String() string {
return fmt.Sprintf("%v", npa.cidrStrings) return fmt.Sprintf("%v", npa.cidrStrings)
} }
// GetNodeAddresses return all matched node IP addresses for npa's CIDRs. // GetNodeAddresses returns all matched node IP addresses for npa's IP family. If npa's
// If npa's CIDRs include "0.0.0.0/0" and/or "::/0", then those values will be returned // CIDRs include "0.0.0.0/0" or "::/0", then that value will be returned verbatim in
// verbatim in the response and no actual IPs of that family will be returned. // the response and no actual IPs of that family will be returned. If no matching IPs are
// If no matching IPs are found, GetNodeAddresses will return an error. // found, GetNodeAddresses will return an error.
// NetworkInterfacer is injected for test purpose. // NetworkInterfacer is injected for test purpose.
func (npa *NodePortAddresses) GetNodeAddresses(nw NetworkInterfacer) (sets.Set[string], error) { func (npa *NodePortAddresses) GetNodeAddresses(nw NetworkInterfacer) (sets.Set[string], error) {
uniqueAddressList := sets.New[string]() uniqueAddressList := sets.New[string]()
@ -77,6 +96,7 @@ func (npa *NodePortAddresses) GetNodeAddresses(nw NetworkInterfacer) (sets.Set[s
for _, cidr := range npa.cidrStrings { for _, cidr := range npa.cidrStrings {
if IsZeroCIDR(cidr) { if IsZeroCIDR(cidr) {
uniqueAddressList.Insert(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) 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 { for _, cidr := range npa.cidrs {
if IsZeroCIDR(cidr.String()) {
continue
}
for _, addr := range addrs { for _, addr := range addrs {
var ip net.IP var ip net.IP
// nw.InterfaceAddrs may return net.IPAddr or net.IPNet on windows, and it will return net.IPNet on linux. // 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 cidr.Contains(ip) {
if netutils.IsIPv6(ip) && !uniqueAddressList.Has(IPv6ZeroCIDR) { uniqueAddressList.Insert(ip.String())
uniqueAddressList.Insert(ip.String())
}
if !netutils.IsIPv6(ip) && !uniqueAddressList.Has(IPv4ZeroCIDR) {
uniqueAddressList.Insert(ip.String())
}
} }
} }
} }

View File

@ -20,6 +20,7 @@ import (
"net" "net"
"testing" "testing"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
fake "k8s.io/kubernetes/pkg/proxy/util/testing" fake "k8s.io/kubernetes/pkg/proxy/util/testing"
netutils "k8s.io/utils/net" netutils "k8s.io/utils/net"
@ -31,11 +32,15 @@ type InterfaceAddrsPair struct {
} }
func TestGetNodeAddresses(t *testing.T) { func TestGetNodeAddresses(t *testing.T) {
type expectation struct {
ips sets.Set[string]
}
testCases := []struct { testCases := []struct {
name string name string
cidrs []string cidrs []string
itfAddrsPairs []InterfaceAddrsPair itfAddrsPairs []InterfaceAddrsPair
expected sets.Set[string] expected map[v1.IPFamily]expectation
}{ }{
{ {
name: "IPv4 single", 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)}}, 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", 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)}}, 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", 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)}}, 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", 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)}}, 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", 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)}}, 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", 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)}}, 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", 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)}}, 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", 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)}}, 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", 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)}}, 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", 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)}}, 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", 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)}}, 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", 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", 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) 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) { addrList, err := npa.GetNodeAddresses(nw)
t.Errorf("unexpected mismatch, expected: %v, got: %v", tc.expected, addrList) // 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 { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { 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 { 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)
} }
}) })
} }

View File

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