github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-09 20:17:41 +00:00
Code Issues Projects Releases Wiki Activity

Add NodePortAddresses.MatchAll()

Browse Source 
Rather than having GetNodeAddresses() return a special magic value
indicating that it matches all IPs, add a separate method to check
that. (And have GetNodeAddresses() just return the IPs as expected
instead.)
This commit is contained in:
Dan Winship 2022-12-24 13:21:17 -05:00
parent 9ac657bb94
commit 2ca215fd99
5 changed files with 115 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
pkg/proxy/healthcheck/service_health.go
View File

@ -59,20 +59,18 @@ type proxierHealthChecker interface {
} }
func newServiceHealthServer(hostname string, recorder events.EventRecorder, listener listener, factory httpServerFactory, nodePortAddresses *utilproxy.NodePortAddresses, healthzServer proxierHealthChecker) ServiceHealthServer { func newServiceHealthServer(hostname string, recorder events.EventRecorder, listener listener, factory httpServerFactory, nodePortAddresses *utilproxy.NodePortAddresses, healthzServer proxierHealthChecker) ServiceHealthServer {
var nodeAddresses sets.Set[string]
nodeAddresses, err := nodePortAddresses.GetNodeAddresses(utilproxy.RealNetwork{})
if err != nil || nodeAddresses.Len() == 0 {
klog.ErrorS(err, "Failed to get node ip address matching node port addresses, health check port will listen to all node addresses", "nodePortAddresses", nodePortAddresses)
nodeAddresses = sets.New[string]()
nodeAddresses.Insert(utilproxy.IPv4ZeroCIDR)
}
// if any of the addresses is zero cidr then we listen // if any of the addresses is zero cidr then we listen
// to old style :<port> // to old style :<port>
for _, addr := range nodeAddresses.UnsortedList() { if nodePortAddresses.MatchAll() {
if utilproxy.IsZeroCIDR(addr) { nodeAddresses = sets.New("")
nodeAddresses = sets.New[string]("") } else {
break var err error
nodeAddresses, err = nodePortAddresses.GetNodeAddresses(utilproxy.RealNetwork{})
if err != nil || nodeAddresses.Len() == 0 {
klog.ErrorS(err, "Failed to get node ip address matching node port addresses, health check port will listen to all node addresses", "nodePortAddresses", nodePortAddresses)
nodeAddresses = sets.New(utilproxy.IPv4ZeroCIDR)
} }
} }

82
pkg/proxy/iptables/proxier.go
View File

@ -1415,55 +1415,53 @@ func (proxier *Proxier) syncProxyRules() {
// Finally, tail-call to the nodePorts chain. This needs to be after all // Finally, tail-call to the nodePorts chain. This needs to be after all
// other service portal rules. // other service portal rules.
nodeAddresses, err := proxier.nodePortAddresses.GetNodeAddresses(proxier.networkInterfacer) if proxier.nodePortAddresses.MatchAll() {
if err != nil { destinations := []string{"-m", "addrtype", "--dst-type", "LOCAL"}
klog.ErrorS(err, "Failed to get node ip address matching nodeport cidrs, services with nodeport may not work as intended", "CIDRs", proxier.nodePortAddresses) if isIPv6 {
} // For IPv6, Regardless of the value of localhostNodePorts is true
// or false, we should disable access to the nodePort via localhost. Since it never works and always
for address := range nodeAddresses { // cause kernel warnings.
if utilproxy.IsZeroCIDR(address) { destinations = append(destinations, "!", "-d", "::1/128")
destinations := []string{"-m", "addrtype", "--dst-type", "LOCAL"}
if isIPv6 {
// For IPv6, Regardless of the value of localhostNodePorts is true
// or false, we should disable access to the nodePort via localhost. Since it never works and always
// cause kernel warnings.
destinations = append(destinations, "!", "-d", "::1/128")
}
if !proxier.localhostNodePorts && !isIPv6 {
// If set localhostNodePorts to "false"(route_localnet=0), We should generate iptables rules that
// disable NodePort services to be accessed via localhost. Since it doesn't work and causes
// the kernel to log warnings if anyone tries.
destinations = append(destinations, "!", "-d", "127.0.0.0/8")
}
proxier.natRules.Write(
"-A", string(kubeServicesChain),
"-m", "comment", "--comment", `"kubernetes service nodeports; NOTE: this must be the last rule in this chain"`,
destinations,
"-j", string(kubeNodePortsChain))
break
} }
// For ipv6, Regardless of the value of localhostNodePorts is true or false, we should disallow access if !proxier.localhostNodePorts && !isIPv6 {
// to the nodePort via lookBack address. // If set localhostNodePorts to "false"(route_localnet=0), We should generate iptables rules that
if isIPv6 && utilproxy.IsLoopBack(address) { // disable NodePort services to be accessed via localhost. Since it doesn't work and causes
klog.ErrorS(nil, "disallow nodePort services to be accessed via ipv6 localhost address", "IP", address) // the kernel to log warnings if anyone tries.
continue destinations = append(destinations, "!", "-d", "127.0.0.0/8")
} }
// For ipv4, When localhostNodePorts is set to false, Ignore ipv4 lookBack address
if !isIPv6 && utilproxy.IsLoopBack(address) && !proxier.localhostNodePorts {
klog.ErrorS(nil, "disallow nodePort services to be accessed via ipv4 localhost address", "IP", address)
continue
}
// create nodeport rules for each IP one by one
proxier.natRules.Write( proxier.natRules.Write(
"-A", string(kubeServicesChain), "-A", string(kubeServicesChain),
"-m", "comment", "--comment", `"kubernetes service nodeports; NOTE: this must be the last rule in this chain"`, "-m", "comment", "--comment", `"kubernetes service nodeports; NOTE: this must be the last rule in this chain"`,
"-d", address, destinations,
"-j", string(kubeNodePortsChain)) "-j", string(kubeNodePortsChain))
} else {
nodeAddresses, err := proxier.nodePortAddresses.GetNodeAddresses(proxier.networkInterfacer)
if err != nil {
klog.ErrorS(err, "Failed to get node ip address matching nodeport cidrs, services with nodeport may not work as intended", "CIDRs", proxier.nodePortAddresses)
}
for address := range nodeAddresses {
// For ipv6, Regardless of the value of localhostNodePorts is true or false, we should disallow access
// to the nodePort via lookBack address.
if isIPv6 && utilproxy.IsLoopBack(address) {
klog.ErrorS(nil, "disallow nodePort services to be accessed via ipv6 localhost address", "IP", address)
continue
}
// For ipv4, When localhostNodePorts is set to false, Ignore ipv4 lookBack address
if !isIPv6 && utilproxy.IsLoopBack(address) && !proxier.localhostNodePorts {
klog.ErrorS(nil, "disallow nodePort services to be accessed via ipv4 localhost address", "IP", address)
continue
}
// create nodeport rules for each IP one by one
proxier.natRules.Write(
"-A", string(kubeServicesChain),
"-m", "comment", "--comment", `"kubernetes service nodeports; NOTE: this must be the last rule in this chain"`,
"-d", address,
"-j", string(kubeNodePortsChain))
}
} }
// Drop the packets in INVALID state, which would potentially cause // Drop the packets in INVALID state, which would potentially cause
@ -1521,7 +1519,7 @@ func (proxier *Proxier) syncProxyRules() {
klog.V(9).InfoS("Restoring iptables", "rules", proxier.iptablesData.Bytes()) klog.V(9).InfoS("Restoring iptables", "rules", proxier.iptablesData.Bytes())
// NOTE: NoFlushTables is used so we don't flush non-kubernetes chains in the table // NOTE: NoFlushTables is used so we don't flush non-kubernetes chains in the table
err = proxier.iptables.RestoreAll(proxier.iptablesData.Bytes(), utiliptables.NoFlushTables, utiliptables.RestoreCounters) err := proxier.iptables.RestoreAll(proxier.iptablesData.Bytes(), utiliptables.NoFlushTables, utiliptables.RestoreCounters)
if err != nil { if err != nil {
if pErr, ok := err.(utiliptables.ParseError); ok { if pErr, ok := err.(utiliptables.ParseError); ok {
lines := utiliptables.ExtractLines(proxier.iptablesData.Bytes(), pErr.Line(), 3) lines := utiliptables.ExtractLines(proxier.iptablesData.Bytes(), pErr.Line(), 3)

27
pkg/proxy/ipvs/proxier.go
View File

@ -1006,23 +1006,22 @@ func (proxier *Proxier) syncProxyRules() {
// can be reused for all nodePort services. // can be reused for all nodePort services.
var nodeIPs []net.IP var nodeIPs []net.IP
if hasNodePort { if hasNodePort {
nodeAddrSet, err := proxier.nodePortAddresses.GetNodeAddresses(proxier.networkInterfacer) if proxier.nodePortAddresses.MatchAll() {
if err != nil { for _, ipStr := range nodeAddressSet.UnsortedList() {
klog.ErrorS(err, "Failed to get node IP address matching nodeport cidr") nodeIPs = append(nodeIPs, netutils.ParseIPSloppy(ipStr))
}
} else { } else {
for _, address := range nodeAddrSet.UnsortedList() { nodeAddrSet, err := proxier.nodePortAddresses.GetNodeAddresses(proxier.networkInterfacer)
a := netutils.ParseIPSloppy(address) if err != nil {
if a.IsLoopback() { klog.ErrorS(err, "Failed to get node IP address matching nodeport cidr")
continue } else {
} for address := range nodeAddrSet {
if utilproxy.IsZeroCIDR(address) { a := netutils.ParseIPSloppy(address)
nodeIPs = nil if a.IsLoopback() {
for _, ipStr := range nodeAddressSet.UnsortedList() { continue
nodeIPs = append(nodeIPs, netutils.ParseIPSloppy(ipStr))
} }
break nodeIPs = append(nodeIPs, a)
} }
nodeIPs = append(nodeIPs, a)
} }
} }
} }

28
pkg/proxy/util/nodeport_addresses.go
View File

@ -31,6 +31,7 @@ type NodePortAddresses struct {
cidrs []*net.IPNet cidrs []*net.IPNet
containsIPv4Loopback bool containsIPv4Loopback bool
matchAll bool
} }
// 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
@ -71,6 +72,7 @@ func NewNodePortAddresses(family v1.IPFamily, cidrStrings []string) *NodePortAdd
if IsZeroCIDR(str) { if IsZeroCIDR(str) {
// Ignore everything else // Ignore everything else
npa.cidrs = []*net.IPNet{cidr} npa.cidrs = []*net.IPNet{cidr}
npa.matchAll = true
break break
} }
@ -84,27 +86,21 @@ func (npa *NodePortAddresses) String() string {
return fmt.Sprintf("%v", npa.cidrStrings) return fmt.Sprintf("%v", npa.cidrStrings)
} }
// GetNodeAddresses returns all matched node IP addresses for npa's IP family. If npa's // MatchAll returns true if npa matches all node IPs (of npa's given family)
// CIDRs include "0.0.0.0/0" or "::/0", then that value will be returned verbatim in func (npa *NodePortAddresses) MatchAll() bool {
// the response and no actual IPs of that family will be returned. If no matching IPs are return npa.matchAll
// found, GetNodeAddresses will return an error. }
// GetNodeAddresses return all matched node IP addresses for npa's CIDRs. If no matching
// IPs are found, it returns an empty list.
// NetworkInterfacer is injected for test purpose. // 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]()
// First round of iteration to pick out `0.0.0.0/0` or `::/0` for the sake of excluding non-zero IPs.
for _, cidr := range npa.cidrStrings {
if IsZeroCIDR(cidr) {
uniqueAddressList.Insert(cidr)
return uniqueAddressList, nil
}
}
addrs, err := nw.InterfaceAddrs() addrs, err := nw.InterfaceAddrs()
if err != nil { if err != nil {
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)
} }
uniqueAddressList := sets.New[string]()
for _, cidr := range npa.cidrs { for _, cidr := range npa.cidrs {
for _, addr := range addrs { for _, addr := range addrs {
var ip net.IP var ip net.IP
@ -124,10 +120,6 @@ func (npa *NodePortAddresses) GetNodeAddresses(nw NetworkInterfacer) (sets.Set[s
} }
} }
if uniqueAddressList.Len() == 0 {
return nil, fmt.Errorf("no addresses found for cidrs %v", npa.cidrStrings)
}
return uniqueAddressList, nil return uniqueAddressList, nil
} }

63
pkg/proxy/util/nodeport_addresses_test.go
View File

@ -33,7 +33,8 @@ type InterfaceAddrsPair struct {
func TestGetNodeAddresses(t *testing.T) { func TestGetNodeAddresses(t *testing.T) {
type expectation struct { type expectation struct {
ips sets.Set[string] matchAll bool
ips sets.Set[string]
} }
testCases := []struct { testCases := []struct {
@ -60,7 +61,8 @@ func TestGetNodeAddresses(t *testing.T) {
ips: sets.New[string]("10.20.30.51"), ips: sets.New[string]("10.20.30.51"),
}, },
v1.IPv6Protocol: { v1.IPv6Protocol: {
ips: sets.New[string]("::/0"), matchAll: true,
ips: nil,
}, },
}, },
}, },
@ -79,10 +81,12 @@ func TestGetNodeAddresses(t *testing.T) {
}, },
expected: map[v1.IPFamily]expectation{ expected: map[v1.IPFamily]expectation{
v1.IPv4Protocol: { v1.IPv4Protocol: {
ips: sets.New[string]("0.0.0.0/0"), matchAll: true,
ips: sets.New[string]("10.20.30.51", "127.0.0.1"),
}, },
v1.IPv6Protocol: { v1.IPv6Protocol: {
ips: sets.New[string]("::/0"), matchAll: true,
ips: nil,
}, },
}, },
}, },
@ -101,7 +105,8 @@ func TestGetNodeAddresses(t *testing.T) {
}, },
expected: map[v1.IPFamily]expectation{ expected: map[v1.IPFamily]expectation{
v1.IPv4Protocol: { v1.IPv4Protocol: {
ips: sets.New[string]("0.0.0.0/0"), matchAll: true,
ips: nil,
}, },
v1.IPv6Protocol: { v1.IPv6Protocol: {
ips: sets.New[string]("2001:db8::1", "::1"), ips: sets.New[string]("2001:db8::1", "::1"),
@ -123,10 +128,12 @@ func TestGetNodeAddresses(t *testing.T) {
}, },
expected: map[v1.IPFamily]expectation{ expected: map[v1.IPFamily]expectation{
v1.IPv4Protocol: { v1.IPv4Protocol: {
ips: sets.New[string]("0.0.0.0/0"), matchAll: true,
ips: nil,
}, },
v1.IPv6Protocol: { v1.IPv6Protocol: {
ips: sets.New[string]("::/0"), matchAll: true,
ips: sets.New[string]("2001:db8::1", "::1"),
}, },
}, },
}, },
@ -148,7 +155,8 @@ func TestGetNodeAddresses(t *testing.T) {
ips: sets.New[string]("127.0.0.1"), ips: sets.New[string]("127.0.0.1"),
}, },
v1.IPv6Protocol: { v1.IPv6Protocol: {
ips: sets.New[string]("::/0"), matchAll: true,
ips: nil,
}, },
}, },
}, },
@ -166,7 +174,8 @@ func TestGetNodeAddresses(t *testing.T) {
ips: sets.New[string]("127.0.1.1"), ips: sets.New[string]("127.0.1.1"),
}, },
v1.IPv6Protocol: { v1.IPv6Protocol: {
ips: sets.New[string]("::/0"), matchAll: true,
ips: nil,
}, },
}, },
}, },
@ -188,7 +197,8 @@ func TestGetNodeAddresses(t *testing.T) {
ips: sets.New[string]("10.20.30.51", "100.200.201.1"), ips: sets.New[string]("10.20.30.51", "100.200.201.1"),
}, },
v1.IPv6Protocol: { v1.IPv6Protocol: {
ips: sets.New[string]("::/0"), matchAll: true,
ips: nil,
}, },
}, },
}, },
@ -210,7 +220,8 @@ func TestGetNodeAddresses(t *testing.T) {
ips: nil, ips: nil,
}, },
v1.IPv6Protocol: { v1.IPv6Protocol: {
ips: sets.New[string]("::/0"), matchAll: true,
ips: nil,
}, },
}, },
}, },
@ -229,10 +240,12 @@ func TestGetNodeAddresses(t *testing.T) {
}, },
expected: map[v1.IPFamily]expectation{ expected: map[v1.IPFamily]expectation{
v1.IPv4Protocol: { v1.IPv4Protocol: {
ips: sets.New[string]("0.0.0.0/0"), matchAll: true,
ips: sets.New[string]("192.168.1.2", "127.0.0.1"),
}, },
v1.IPv6Protocol: { v1.IPv6Protocol: {
ips: sets.New[string]("::/0"), matchAll: true,
ips: nil,
}, },
}, },
}, },
@ -251,10 +264,12 @@ func TestGetNodeAddresses(t *testing.T) {
}, },
expected: map[v1.IPFamily]expectation{ expected: map[v1.IPFamily]expectation{
v1.IPv4Protocol: { v1.IPv4Protocol: {
ips: sets.New[string]("0.0.0.0/0"), matchAll: true,
ips: nil,
}, },
v1.IPv6Protocol: { v1.IPv6Protocol: {
ips: sets.New[string]("::/0"), matchAll: true,
ips: sets.New[string]("2001:db8::1", "::1"),
}, },
}, },
}, },
@ -269,10 +284,12 @@ func TestGetNodeAddresses(t *testing.T) {
}, },
expected: map[v1.IPFamily]expectation{ expected: map[v1.IPFamily]expectation{
v1.IPv4Protocol: { v1.IPv4Protocol: {
ips: sets.New[string]("0.0.0.0/0"), matchAll: true,
ips: sets.New[string]("1.2.3.4"),
}, },
v1.IPv6Protocol: { v1.IPv6Protocol: {
ips: sets.New[string]("::/0"), matchAll: true,
ips: nil,
}, },
}, },
}, },
@ -297,7 +314,8 @@ func TestGetNodeAddresses(t *testing.T) {
}, },
expected: map[v1.IPFamily]expectation{ expected: map[v1.IPFamily]expectation{
v1.IPv4Protocol: { v1.IPv4Protocol: {
ips: sets.New[string]("0.0.0.0/0"), matchAll: true,
ips: sets.New[string]("1.2.3.4", "127.0.0.1"),
}, },
v1.IPv6Protocol: { v1.IPv6Protocol: {
ips: sets.New[string]("2001:db8::1"), ips: sets.New[string]("2001:db8::1"),
@ -328,7 +346,8 @@ func TestGetNodeAddresses(t *testing.T) {
ips: sets.New[string]("1.2.3.4"), ips: sets.New[string]("1.2.3.4"),
}, },
v1.IPv6Protocol: { v1.IPv6Protocol: {
ips: sets.New[string]("::/0"), matchAll: true,
ips: sets.New[string]("2001:db8::1", "::1"),
}, },
}, },
}, },
@ -344,11 +363,15 @@ func TestGetNodeAddresses(t *testing.T) {
for _, family := range []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol} { for _, family := range []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol} {
npa := NewNodePortAddresses(family, tc.cidrs) npa := NewNodePortAddresses(family, tc.cidrs)
if npa.MatchAll() != tc.expected[family].matchAll {
t.Errorf("unexpected MatchAll(%s), expected: %v", family, tc.expected[family].matchAll)
}
addrList, err := npa.GetNodeAddresses(nw) addrList, err := npa.GetNodeAddresses(nw)
// The fake InterfaceAddrs() never returns an error, so // The fake InterfaceAddrs() never returns an error, so
// the only error GetNodeAddresses will return is "no // the only error GetNodeAddresses will return is "no
// addresses found". // addresses found".
if err != nil && tc.expected[family].ips != nil { if err != nil {
t.Errorf("unexpected error: %v", err) t.Errorf("unexpected error: %v", err)
} }