diff --git a/pkg/proxy/healthcheck/healthcheck_test.go b/pkg/proxy/healthcheck/healthcheck_test.go index fc82049dcfe..c1c451ef8c2 100644 --- a/pkg/proxy/healthcheck/healthcheck_test.go +++ b/pkg/proxy/healthcheck/healthcheck_test.go @@ -150,7 +150,7 @@ func (fake fakeProxierHealthChecker) IsHealthy() bool { func TestServer(t *testing.T) { listener := newFakeListener() httpFactory := newFakeHTTPServerFactory() - nodePortAddresses := proxyutil.NewNodePortAddresses(v1.IPv4Protocol, []string{}, nil) + nodePortAddresses := proxyutil.NewNodePortAddresses(v1.IPv4Protocol, []string{}) proxyChecker := &fakeProxierHealthChecker{true} hcsi := newServiceHealthServer("hostname", nil, listener, httpFactory, nodePortAddresses, proxyChecker) @@ -664,7 +664,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 := proxyutil.NewNodePortAddresses(v1.IPv4Protocol, []string{"127.0.0.0/8"}, nil) + nodePortAddresses := proxyutil.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 d0c427d5e41..2771801b69c 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -232,7 +232,7 @@ func NewProxier(ipFamily v1.IPFamily, nodePortAddressStrings []string, initOnly bool, ) (*Proxier, error) { - nodePortAddresses := proxyutil.NewNodePortAddresses(ipFamily, nodePortAddressStrings, nil) + nodePortAddresses := proxyutil.NewNodePortAddresses(ipFamily, nodePortAddressStrings) if !nodePortAddresses.ContainsIPv4Loopback() { localhostNodePorts = false diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index 45f75be1ae4..cb434c1263c 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -133,7 +133,7 @@ func NewFakeProxier(ipt utiliptables.Interface) *Proxier { natRules: proxyutil.NewLineBuffer(), nodeIP: netutils.ParseIPSloppy(testNodeIP), localhostNodePorts: true, - nodePortAddresses: proxyutil.NewNodePortAddresses(ipfamily, nil, nil), + nodePortAddresses: proxyutil.NewNodePortAddresses(ipfamily, nil), networkInterfacer: networkInterfacer, } p.setInitialized(true) @@ -2342,7 +2342,7 @@ func TestNodePorts(t *testing.T) { fp := NewFakeProxier(ipt) fp.localhostNodePorts = tc.localhostNodePorts if tc.nodePortAddresses != nil { - fp.nodePortAddresses = proxyutil.NewNodePortAddresses(tc.family, tc.nodePortAddresses, nil) + fp.nodePortAddresses = proxyutil.NewNodePortAddresses(tc.family, tc.nodePortAddresses) } makeServiceMap(fp, @@ -2490,7 +2490,7 @@ func TestNodePorts(t *testing.T) { func TestHealthCheckNodePort(t *testing.T) { ipt := iptablestest.NewFake() fp := NewFakeProxier(ipt) - fp.nodePortAddresses = proxyutil.NewNodePortAddresses(v1.IPv4Protocol, []string{"127.0.0.0/8"}, nil) + fp.nodePortAddresses = proxyutil.NewNodePortAddresses(v1.IPv4Protocol, []string{"127.0.0.0/8"}) svcIP := "172.30.0.42" svcPort := 80 diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 5c492eb7b66..daf28fee6be 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -359,7 +359,7 @@ func NewProxier(ipFamily v1.IPFamily, scheduler = defaultScheduler } - nodePortAddresses := proxyutil.NewNodePortAddresses(ipFamily, nodePortAddressStrings, nil) + nodePortAddresses := proxyutil.NewNodePortAddresses(ipFamily, nodePortAddressStrings) serviceHealthServer := healthcheck.NewServiceHealthServer(hostname, recorder, nodePortAddresses, healthzServer) diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index 55cc280a831..33af6d72b61 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -158,7 +158,7 @@ func NewFakeProxier(ipt utiliptables.Interface, ipvs utilipvs.Interface, ipset u filterRules: proxyutil.NewLineBuffer(), netlinkHandle: netlinkHandle, ipsetList: ipsetList, - nodePortAddresses: proxyutil.NewNodePortAddresses(ipFamily, nil, nil), + nodePortAddresses: proxyutil.NewNodePortAddresses(ipFamily, nil), networkInterfacer: proxyutiltest.NewFakeNetwork(), gracefuldeleteManager: NewGracefulTerminationManager(ipvs), ipFamily: ipFamily, @@ -945,7 +945,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 = proxyutil.NewNodePortAddresses(v1.IPv4Protocol, test.nodePortAddresses, nil) + fp.nodePortAddresses = proxyutil.NewNodePortAddresses(v1.IPv4Protocol, test.nodePortAddresses) makeServiceMap(fp, test.services...) populateEndpointSlices(fp, test.endpoints...) @@ -1287,7 +1287,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 = proxyutil.NewNodePortAddresses(v1.IPv6Protocol, test.nodePortAddresses, nil) + fp.nodePortAddresses = proxyutil.NewNodePortAddresses(v1.IPv6Protocol, test.nodePortAddresses) makeServiceMap(fp, test.services...) populateEndpointSlices(fp, test.endpoints...) @@ -2040,7 +2040,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 = proxyutil.NewNodePortAddresses(v1.IPv4Protocol, []string{"100.101.102.0/24"}, nil) + fp.nodePortAddresses = proxyutil.NewNodePortAddresses(v1.IPv4Protocol, []string{"100.101.102.0/24"}) fp.syncProxyRules() @@ -2128,7 +2128,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 = proxyutil.NewNodePortAddresses(v1.IPv4Protocol, []string{"100.101.102.0/24"}, nil) + fp.nodePortAddresses = proxyutil.NewNodePortAddresses(v1.IPv4Protocol, []string{"100.101.102.0/24"}) fp.syncProxyRules() diff --git a/pkg/proxy/nftables/proxier.go b/pkg/proxy/nftables/proxier.go index 3e54e7ee4c9..490ea19d3ba 100644 --- a/pkg/proxy/nftables/proxier.go +++ b/pkg/proxy/nftables/proxier.go @@ -211,8 +211,6 @@ func NewProxier(ipFamily v1.IPFamily, nodePortAddressStrings []string, initOnly bool, ) (*Proxier, error) { - nodePortAddresses := proxyutil.NewNodePortAddresses(ipFamily, nodePortAddressStrings, nodeIP) - if initOnly { klog.InfoS("System initialized and --init-only specified") return nil, nil @@ -223,6 +221,8 @@ func NewProxier(ipFamily v1.IPFamily, masqueradeMark := fmt.Sprintf("%#08x", masqueradeValue) klog.V(2).InfoS("Using nftables mark for masquerade", "ipFamily", ipFamily, "mark", masqueradeMark) + nodePortAddresses := proxyutil.NewNodePortAddresses(ipFamily, nodePortAddressStrings) + serviceHealthServer := healthcheck.NewServiceHealthServer(hostname, recorder, nodePortAddresses, healthzServer) var nftablesFamily knftables.Family diff --git a/pkg/proxy/nftables/proxier_test.go b/pkg/proxy/nftables/proxier_test.go index 1b67de4f75c..fcf67d05afc 100644 --- a/pkg/proxy/nftables/proxier_test.go +++ b/pkg/proxy/nftables/proxier_test.go @@ -126,7 +126,7 @@ func NewFakeProxier(ipFamily v1.IPFamily) (*knftables.Fake, *Proxier) { hostname: testHostname, serviceHealthServer: healthcheck.NewFakeServiceHealthServer(), nodeIP: nodeIP, - nodePortAddresses: proxyutil.NewNodePortAddresses(ipFamily, nodePortAddresses, nodeIP), + nodePortAddresses: proxyutil.NewNodePortAddresses(ipFamily, nodePortAddresses), networkInterfacer: networkInterfacer, staleChains: make(map[string]time.Time), serviceCIDRs: serviceCIDRs, @@ -952,7 +952,7 @@ func TestNodePorts(t *testing.T) { nodeIP = testNodeIPv6 } if tc.nodePortAddresses != nil { - fp.nodePortAddresses = proxyutil.NewNodePortAddresses(tc.family, tc.nodePortAddresses, netutils.ParseIPSloppy(nodeIP)) + fp.nodePortAddresses = proxyutil.NewNodePortAddresses(tc.family, tc.nodePortAddresses) } makeServiceMap(fp, diff --git a/pkg/proxy/util/nodeport_addresses.go b/pkg/proxy/util/nodeport_addresses.go index ab11d08917b..c5332a07958 100644 --- a/pkg/proxy/util/nodeport_addresses.go +++ b/pkg/proxy/util/nodeport_addresses.go @@ -20,7 +20,7 @@ import ( "fmt" "net" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" netutils "k8s.io/utils/net" ) @@ -37,12 +37,11 @@ type NodePortAddresses struct { var ipv4LoopbackStart = net.IPv4(127, 0, 0, 0) // NewNodePortAddresses takes an IP family and the `--nodeport-addresses` value (which is -// assumed to contain only valid CIDRs, potentially of both IP families) and the primary IP -// (which will be used as node port address when `--nodeport-addresses` is empty). -// It will return 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, primaryIP net.IP) *NodePortAddresses { +// 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 @@ -52,24 +51,17 @@ func NewNodePortAddresses(family v1.IPFamily, cidrStrings []string, primaryIP ne } } if len(npa.cidrStrings) == 0 { - if primaryIP == nil { - if family == v1.IPv4Protocol { - npa.cidrStrings = []string{IPv4ZeroCIDR} - } else { - npa.cidrStrings = []string{IPv6ZeroCIDR} - } + if family == v1.IPv4Protocol { + npa.cidrStrings = []string{IPv4ZeroCIDR} } else { - if family == v1.IPv4Protocol { - npa.cidrStrings = []string{fmt.Sprintf("%s/32", primaryIP.String())} - } else { - npa.cidrStrings = []string{fmt.Sprintf("%s/128", primaryIP.String())} - } + npa.cidrStrings = []string{IPv6ZeroCIDR} } } // Now parse for _, str := range npa.cidrStrings { _, cidr, _ := netutils.ParseCIDRSloppy(str) + if netutils.IsIPv4CIDR(cidr) { if cidr.IP.IsLoopback() || cidr.Contains(ipv4LoopbackStart) { npa.containsIPv4Loopback = true diff --git a/pkg/proxy/util/nodeport_addresses_test.go b/pkg/proxy/util/nodeport_addresses_test.go index fad92ef6a54..c66db1b024f 100644 --- a/pkg/proxy/util/nodeport_addresses_test.go +++ b/pkg/proxy/util/nodeport_addresses_test.go @@ -21,7 +21,7 @@ import ( "net" "testing" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" fake "k8s.io/kubernetes/pkg/proxy/util/testing" netutils "k8s.io/utils/net" @@ -60,8 +60,6 @@ func TestGetNodeIPs(t *testing.T) { cidrs []string itfAddrsPairs []InterfaceAddrsPair expected map[v1.IPFamily]expectation - // nodeIP will take effect when `--nodeport-addresses` is empty - nodeIP net.IP }{ { name: "IPv4 single", @@ -371,53 +369,6 @@ func TestGetNodeIPs(t *testing.T) { }, }, }, - { - name: "ipv4 with nodeIP", - itfAddrsPairs: []InterfaceAddrsPair{ - { - itf: net.Interface{Index: 0, MTU: 0, Name: "eth0", HardwareAddr: nil, Flags: 0}, - addrs: []net.Addr{ - &net.IPNet{IP: netutils.ParseIPSloppy("1.2.3.4"), Mask: net.CIDRMask(30, 32)}, - }, - }, - { - itf: net.Interface{Index: 1, MTU: 0, Name: "lo", HardwareAddr: nil, Flags: 0}, - addrs: []net.Addr{ - &net.IPNet{IP: netutils.ParseIPSloppy("127.0.0.1"), Mask: net.CIDRMask(8, 32)}, - }, - }, - }, - expected: map[v1.IPFamily]expectation{ - v1.IPv4Protocol: { - ips: sets.New[string]("1.2.3.4"), - }, - }, - nodeIP: netutils.ParseIPSloppy("1.2.3.4"), - }, - { - name: "ipv6 with nodeIP", - itfAddrsPairs: []InterfaceAddrsPair{ - { - itf: net.Interface{Index: 0, MTU: 0, Name: "eth0", HardwareAddr: nil, Flags: 0}, - addrs: []net.Addr{ - &net.IPNet{IP: netutils.ParseIPSloppy("2001:db8::1"), Mask: net.CIDRMask(64, 128)}, - }, - }, - { - itf: net.Interface{Index: 1, MTU: 0, Name: "lo", HardwareAddr: nil, Flags: 0}, - addrs: []net.Addr{ - &net.IPNet{IP: netutils.ParseIPSloppy("::1"), Mask: net.CIDRMask(128, 128)}, - }, - }, - }, - expected: map[v1.IPFamily]expectation{ - v1.IPv6Protocol: { - matchAll: true, - ips: sets.New[string]("2001:db8::1", "::1"), - }, - }, - nodeIP: netutils.ParseIPSloppy("1.2.3.4"), - }, } for _, tc := range testCases { @@ -428,10 +379,7 @@ func TestGetNodeIPs(t *testing.T) { } for _, family := range []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol} { - if tc.nodeIP != nil && v1.IPFamily(fmt.Sprintf("IPv%s", netutils.IPFamilyOf(tc.nodeIP))) != family { - continue - } - npa := NewNodePortAddresses(family, tc.cidrs, tc.nodeIP) + npa := NewNodePortAddresses(family, tc.cidrs) if npa.MatchAll() != tc.expected[family].matchAll { t.Errorf("unexpected MatchAll(%s), expected: %v", family, tc.expected[family].matchAll) @@ -503,12 +451,12 @@ func TestContainsIPv4Loopback(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - npa := NewNodePortAddresses(v1.IPv4Protocol, tt.cidrStrings, nil) + npa := NewNodePortAddresses(v1.IPv4Protocol, tt.cidrStrings) if got := npa.ContainsIPv4Loopback(); 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, nil) + 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/util/utils.go b/pkg/proxy/util/utils.go index da232fef982..260894461d5 100644 --- a/pkg/proxy/util/utils.go +++ b/pkg/proxy/util/utils.go @@ -29,10 +29,11 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/tools/events" utilsysctl "k8s.io/component-helpers/node/util/sysctl" - "k8s.io/klog/v2" helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/features" netutils "k8s.io/utils/net" + + "k8s.io/klog/v2" ) const ( diff --git a/pkg/proxy/winkernel/proxier.go b/pkg/proxy/winkernel/proxier.go index a09a693f5ff..a1f9bade5f2 100644 --- a/pkg/proxy/winkernel/proxier.go +++ b/pkg/proxy/winkernel/proxier.go @@ -666,7 +666,7 @@ func NewProxier( } // windows listens to all node addresses - nodePortAddresses := proxyutil.NewNodePortAddresses(ipFamily, nil, nil) + nodePortAddresses := proxyutil.NewNodePortAddresses(ipFamily, nil) serviceHealthServer := healthcheck.NewServiceHealthServer(hostname, recorder, nodePortAddresses, healthzServer) var healthzPort int