Redo --nodeport-addresses handling with a set

pkg/proxy/nftables/helpers_test.go

@@ -221,6 +221,17 @@ func (tracer *nftablesTracer) addressMatches(ipStr, not, ruleAddress string) boo
 	}
 }
 
+// matchDestIPOnly checks an "ip daddr" against a set/map, and returns the matching
+// Element, if found.
+func (tracer *nftablesTracer) matchDestIPOnly(elements []*knftables.Element, destIP string) *knftables.Element {
+	for _, element := range elements {
+		if element.Key[0] == destIP {
+			return element
+		}
+	}
+	return nil
+}
+
 // We intentionally don't try to parse arbitrary nftables rules, as the syntax is quite
 // complicated and context sensitive. (E.g., "ip daddr" could be the start of an address
 // comparison, or it could be the start of a set/map lookup.) Instead, we just have
@@ -235,6 +246,7 @@ func (tracer *nftablesTracer) addressMatches(ipStr, not, ruleAddress string) boo
 var destAddrRegexp = regexp.MustCompile(`^ip6* daddr (!= )?(\S+)`)
 var destAddrLocalRegexp = regexp.MustCompile(`^fib daddr type local`)
 var destPortRegexp = regexp.MustCompile(`^(tcp|udp|sctp) dport (\d+)`)
+var destIPOnlyLookupRegexp = regexp.MustCompile(`^ip6* daddr @(\S+)`)
 
 var sourceAddrRegexp = regexp.MustCompile(`^ip6* saddr (!= )?(\S+)`)
 var sourceAddrLocalRegexp = regexp.MustCompile(`^fib saddr type local`)
@@ -287,6 +299,17 @@ func (tracer *nftablesTracer) runChain(chname, sourceIP, protocol, destIP, destP
 			// thing with it.
 
 			switch {
+			case destIPOnlyLookupRegexp.MatchString(rule):
+				// `^ip6* daddr @(\S+)`
+				// Tests whether destIP is a member of the indicated set.
+				match := destIPOnlyLookupRegexp.FindStringSubmatch(rule)
+				rule = strings.TrimPrefix(rule, match[0])
+				set := match[1]
+				if tracer.matchDestIPOnly(tracer.nft.Table.Sets[set].Elements, destIP) == nil {
+					rule = ""
+					break
+				}
+
 			case destAddrRegexp.MatchString(rule):
 				// `^ip6* daddr (!= )?(\S+)`
 				// Tests whether destIP does/doesn't match a literal.

pkg/proxy/nftables/proxier.go

@@ -65,6 +65,9 @@ const (
 	kubeServicesChain  = "services"
 	kubeNodePortsChain = "nodeports"
 
+	// set of IPs that accept NodePort traffic
+	kubeNodePortIPsSet = "nodeport-ips"
+
 	// handling for services with no endpoints
 	kubeServicesFilterChain   = "services-filter"
 	kubeExternalServicesChain = "external-services"
@@ -365,6 +368,11 @@ func ensureChain(chain string, tx *knftables.Transaction, createdChains sets.Set
 }
 
 func (proxier *Proxier) setupNFTables(tx *knftables.Transaction) {
+	ipvX_addr := "ipv4_addr" //nolint:stylecheck // var name intentionally resembles value
+	if proxier.ipFamily == v1.IPv6Protocol {
+		ipvX_addr = "ipv6_addr"
+	}
+
 	tx.Add(&knftables.Table{
 		Comment: ptr.To("rules for kube-proxy"),
 	})
@@ -435,6 +443,40 @@ func (proxier *Proxier) setupNFTables(tx *knftables.Transaction) {
 			Rule:  "ct state invalid drop",
 		})
 	}
+
+	// Fill in nodeport-ips set if needed (or delete it if not). (We do "add+delete"
+	// rather than just "delete" when we want to ensure the set doesn't exist, because
+	// doing just "delete" would return an error if the set didn't exist.)
+	tx.Add(&knftables.Set{
+		Name:    kubeNodePortIPsSet,
+		Type:    ipvX_addr,
+		Comment: ptr.To("IPs that accept NodePort traffic"),
+	})
+	if proxier.nodePortAddresses.MatchAll() {
+		tx.Delete(&knftables.Set{
+			Name: kubeNodePortIPsSet,
+		})
+	} else {
+		tx.Flush(&knftables.Set{
+			Name: kubeNodePortIPsSet,
+		})
+		nodeIPs, err := proxier.nodePortAddresses.GetNodeIPs(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 _, ip := range nodeIPs {
+			if ip.IsLoopback() {
+				klog.ErrorS(nil, "--nodeport-addresses includes localhost but localhost NodePorts are not supported", "address", ip.String())
+				continue
+			}
+			tx.Add(&knftables.Element{
+				Set: kubeNodePortIPsSet,
+				Key: []string{
+					ip.String(),
+				},
+			})
+		}
+	}
 }
 
 // CleanupLeftovers removes all nftables rules and chains created by the Proxier
@@ -1343,26 +1385,14 @@ func (proxier *Proxier) syncProxyRules() {
 			Comment: ptr.To("kubernetes service nodeports; NOTE: this must be the last rule in this chain"),
 		})
 	} else {
-		nodeIPs, err := proxier.nodePortAddresses.GetNodeIPs(proxier.networkInterfacer)
+		tx.Add(&knftables.Rule{
-		if err != nil {
+			Chain: kubeServicesChain,
-			klog.ErrorS(err, "Failed to get node ip address matching nodeport cidrs, services with nodeport may not work as intended", "CIDRs", proxier.nodePortAddresses)
+			Rule: knftables.Concat(
-		}
+				ipX, "daddr", "@", kubeNodePortIPsSet,
-		for _, ip := range nodeIPs {
+				"jump", kubeNodePortsChain,
-			if ip.IsLoopback() {
+			),
-				klog.ErrorS(nil, "--nodeport-addresses includes localhost but localhost NodePorts are not supported", "address", ip.String())
+			Comment: ptr.To("kubernetes service nodeports; NOTE: this must be the last rule in this chain"),
-				continue
+		})
-			}
-
-			// create nodeport rules for each IP one by one
-			tx.Add(&knftables.Rule{
-				Chain: kubeServicesChain,
-				Rule: knftables.Concat(
-					ipX, "daddr", ip,
-					"jump", kubeNodePortsChain,
-				),
-				Comment: ptr.To("kubernetes service nodeports; NOTE: this must be the last rule in this chain"),
-			})
-		}
 	}
 
 	// Figure out which chains are now stale. Unfortunately, we can't delete them