Merge pull request #109060 from thockin/kube-proxy-rule-cleanups-after-106497

Kube proxy rule reorg XLB->EXT

pkg/api/v1/service/util.go

@@ -67,8 +67,8 @@ func GetLoadBalancerSourceRanges(service *v1.Service) (utilnet.IPNetSet, error)
 	return ipnets, nil
 }
 
-// RequestsOnlyLocalTraffic checks if service requests OnlyLocal traffic.
+// ExternalPolicyLocal checks if service has ETP = Local.
-func RequestsOnlyLocalTraffic(service *v1.Service) bool {
+func ExternalPolicyLocal(service *v1.Service) bool {
 	if service.Spec.Type != v1.ServiceTypeLoadBalancer &&
 		service.Spec.Type != v1.ServiceTypeNodePort {
 		return false
@@ -76,9 +76,8 @@ func RequestsOnlyLocalTraffic(service *v1.Service) bool {
 	return service.Spec.ExternalTrafficPolicy == v1.ServiceExternalTrafficPolicyTypeLocal
 }
 
-// RequestsOnlyLocalTrafficForInternal checks if service prefers Node Local
+// InternalPolicyLocal checks if service has ITP = Local.
-// endpoints for internal traffic
+func InternalPolicyLocal(service *v1.Service) bool {
-func RequestsOnlyLocalTrafficForInternal(service *v1.Service) bool {
 	if service.Spec.InternalTrafficPolicy == nil {
 		return false
 	}
@@ -90,7 +89,7 @@ func NeedsHealthCheck(service *v1.Service) bool {
 	if service.Spec.Type != v1.ServiceTypeLoadBalancer {
 		return false
 	}
-	return RequestsOnlyLocalTraffic(service)
+	return ExternalPolicyLocal(service)
 }
 
 // GetServiceHealthCheckPathPort returns the path and nodePort programmed into the Cloud LB Health Check

pkg/api/v1/service/util_test.go

@@ -129,45 +129,45 @@ func TestAllowAll(t *testing.T) {
 	checkAllowAll(true, "192.168.0.1/32", "0.0.0.0/0")
 }
 
-func TestRequestsOnlyLocalTraffic(t *testing.T) {
+func TestExternalPolicyLocal(t *testing.T) {
-	checkRequestsOnlyLocalTraffic := func(requestsOnlyLocalTraffic bool, service *v1.Service) {
+	checkExternalPolicyLocal := func(requestsOnlyLocalTraffic bool, service *v1.Service) {
-		res := RequestsOnlyLocalTraffic(service)
+		res := ExternalPolicyLocal(service)
 		if res != requestsOnlyLocalTraffic {
 			t.Errorf("Expected requests OnlyLocal traffic = %v, got %v",
 				requestsOnlyLocalTraffic, res)
 		}
 	}
 
-	checkRequestsOnlyLocalTraffic(false, &v1.Service{})
+	checkExternalPolicyLocal(false, &v1.Service{})
-	checkRequestsOnlyLocalTraffic(false, &v1.Service{
+	checkExternalPolicyLocal(false, &v1.Service{
 		Spec: v1.ServiceSpec{
 			Type: v1.ServiceTypeClusterIP,
 		},
 	})
-	checkRequestsOnlyLocalTraffic(false, &v1.Service{
+	checkExternalPolicyLocal(false, &v1.Service{
 		Spec: v1.ServiceSpec{
 			Type: v1.ServiceTypeNodePort,
 		},
 	})
-	checkRequestsOnlyLocalTraffic(false, &v1.Service{
+	checkExternalPolicyLocal(false, &v1.Service{
 		Spec: v1.ServiceSpec{
 			Type:                  v1.ServiceTypeNodePort,
 			ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster,
 		},
 	})
-	checkRequestsOnlyLocalTraffic(true, &v1.Service{
+	checkExternalPolicyLocal(true, &v1.Service{
 		Spec: v1.ServiceSpec{
 			Type:                  v1.ServiceTypeNodePort,
 			ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal,
 		},
 	})
-	checkRequestsOnlyLocalTraffic(false, &v1.Service{
+	checkExternalPolicyLocal(false, &v1.Service{
 		Spec: v1.ServiceSpec{
 			Type:                  v1.ServiceTypeLoadBalancer,
 			ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster,
 		},
 	})
-	checkRequestsOnlyLocalTraffic(true, &v1.Service{
+	checkExternalPolicyLocal(true, &v1.Service{
 		Spec: v1.ServiceSpec{
 			Type:                  v1.ServiceTypeLoadBalancer,
 			ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal,
@@ -215,9 +215,9 @@ func TestNeedsHealthCheck(t *testing.T) {
 	})
 }
 
-func TestRequestsOnlyLocalTrafficForInternal(t *testing.T) {
+func TestInternalPolicyLocal(t *testing.T) {
-	checkRequestsOnlyLocalTrafficForInternal := func(expected bool, service *v1.Service) {
+	checkInternalPolicyLocal := func(expected bool, service *v1.Service) {
-		res := RequestsOnlyLocalTrafficForInternal(service)
+		res := InternalPolicyLocal(service)
 		if res != expected {
 			t.Errorf("Expected internal local traffic = %v, got %v",
 				expected, res)
@@ -225,17 +225,17 @@ func TestRequestsOnlyLocalTrafficForInternal(t *testing.T) {
 	}
 
 	// default InternalTrafficPolicy is nil
-	checkRequestsOnlyLocalTrafficForInternal(false, &v1.Service{})
+	checkInternalPolicyLocal(false, &v1.Service{})
 
 	local := v1.ServiceInternalTrafficPolicyLocal
-	checkRequestsOnlyLocalTrafficForInternal(true, &v1.Service{
+	checkInternalPolicyLocal(true, &v1.Service{
 		Spec: v1.ServiceSpec{
 			InternalTrafficPolicy: &local,
 		},
 	})
 
 	cluster := v1.ServiceInternalTrafficPolicyCluster
-	checkRequestsOnlyLocalTrafficForInternal(false, &v1.Service{
+	checkInternalPolicyLocal(false, &v1.Service{
 		Spec: v1.ServiceSpec{
 			InternalTrafficPolicy: &cluster,
 		},

pkg/proxy/iptables/proxier.go

@@ -115,11 +115,11 @@ const sysctlBridgeCallIPTables = "net/bridge/bridge-nf-call-iptables"
 type serviceInfo struct {
 	*proxy.BaseServiceInfo
 	// The following fields are computed and stored for performance reasons.
-	serviceNameString        string
+	nameString             string
-	servicePortChainName     utiliptables.Chain
+	clusterPolicyChainName utiliptables.Chain
-	serviceLocalChainName    utiliptables.Chain
+	localPolicyChainName   utiliptables.Chain
-	serviceFirewallChainName utiliptables.Chain
+	firewallChainName      utiliptables.Chain
-	serviceLBChainName       utiliptables.Chain
+	externalChainName      utiliptables.Chain
 }
 
 // returns a new proxy.ServicePort which abstracts a serviceInfo
@@ -130,11 +130,11 @@ func newServiceInfo(port *v1.ServicePort, service *v1.Service, baseInfo *proxy.B
 	svcName := types.NamespacedName{Namespace: service.Namespace, Name: service.Name}
 	svcPortName := proxy.ServicePortName{NamespacedName: svcName, Port: port.Name}
 	protocol := strings.ToLower(string(info.Protocol()))
-	info.serviceNameString = svcPortName.String()
+	info.nameString = svcPortName.String()
-	info.servicePortChainName = servicePortChainName(info.serviceNameString, protocol)
+	info.clusterPolicyChainName = servicePortPolicyClusterChain(info.nameString, protocol)
-	info.serviceLocalChainName = serviceLocalChainName(info.serviceNameString, protocol)
+	info.localPolicyChainName = servicePortPolicyLocalChainName(info.nameString, protocol)
-	info.serviceFirewallChainName = serviceFirewallChainName(info.serviceNameString, protocol)
+	info.firewallChainName = serviceFirewallChainName(info.nameString, protocol)
-	info.serviceLBChainName = serviceLBChainName(info.serviceNameString, protocol)
+	info.externalChainName = serviceExternalChainName(info.nameString, protocol)
 
 	return info
 }
@@ -684,25 +684,28 @@ func portProtoHash(servicePortName string, protocol string) string {
 }
 
 const (
-	servicePortChainNamePrefix         = "KUBE-SVC-"
+	servicePortPolicyClusterChainNamePrefix = "KUBE-SVC-"
-	serviceLocalChainNamePrefix        = "KUBE-SVL-"
+	servicePortPolicyLocalChainNamePrefix   = "KUBE-SVL-"
 	serviceFirewallChainNamePrefix          = "KUBE-FW-"
-	serviceLBChainNamePrefix           = "KUBE-XLB-"
+	serviceExternalChainNamePrefix          = "KUBE-EXT-"
 	servicePortEndpointChainNamePrefix      = "KUBE-SEP-"
+
+	// For cleanup.  This can be removed after 1.26 is released.
+	deprecatedServiceLBChainNamePrefix = "KUBE-XLB-"
 )
 
-// servicePortChainName returns the name of the KUBE-SVC-XXXX chain for a service, which is the
+// servicePortPolicyClusterChain returns the name of the KUBE-SVC-XXXX chain for a service, which is the
 // main iptables chain for that service, used for dispatching to endpoints when using `Cluster`
 // traffic policy.
-func servicePortChainName(servicePortName string, protocol string) utiliptables.Chain {
+func servicePortPolicyClusterChain(servicePortName string, protocol string) utiliptables.Chain {
-	return utiliptables.Chain(servicePortChainNamePrefix + portProtoHash(servicePortName, protocol))
+	return utiliptables.Chain(servicePortPolicyClusterChainNamePrefix + portProtoHash(servicePortName, protocol))
 }
 
-// serviceLocalChainName returns the name of the KUBE-SVL-XXXX chain for a service, which
+// servicePortPolicyLocalChainName returns the name of the KUBE-SVL-XXXX chain for a service, which
 // handles dispatching to local endpoints when using `Local` traffic policy. This chain only
 // exists if the service has `Local` internal or external traffic policy.
-func serviceLocalChainName(servicePortName string, protocol string) utiliptables.Chain {
+func servicePortPolicyLocalChainName(servicePortName string, protocol string) utiliptables.Chain {
-	return utiliptables.Chain(serviceLocalChainNamePrefix + portProtoHash(servicePortName, protocol))
+	return utiliptables.Chain(servicePortPolicyLocalChainNamePrefix + portProtoHash(servicePortName, protocol))
 }
 
 // serviceFirewallChainName returns the name of the KUBE-FW-XXXX chain for a service, which
@@ -711,12 +714,12 @@ func serviceFirewallChainName(servicePortName string, protocol string) utiliptab
 	return utiliptables.Chain(serviceFirewallChainNamePrefix + portProtoHash(servicePortName, protocol))
 }
 
-// serviceLBChainName returns the name of the KUBE-XLB-XXXX chain for a service, which
+// serviceExternalChainName returns the name of the KUBE-EXT-XXXX chain for a service, which
-// implements "short-circuiting" for internally-originated load balancer traffic when using
+// implements "short-circuiting" for internally-originated external-destination traffic when using
 // `Local` external traffic policy.  It forwards traffic from local sources to the KUBE-SVC-XXXX
 // chain and traffic from external sources to the KUBE-SVL-XXXX chain.
-func serviceLBChainName(servicePortName string, protocol string) utiliptables.Chain {
+func serviceExternalChainName(servicePortName string, protocol string) utiliptables.Chain {
-	return utiliptables.Chain(serviceLBChainNamePrefix + portProtoHash(servicePortName, protocol))
+	return utiliptables.Chain(serviceExternalChainNamePrefix + portProtoHash(servicePortName, protocol))
 }
 
 // servicePortEndpointChainName returns the name of the KUBE-SEP-XXXX chain for a particular
@@ -729,11 +732,12 @@ func servicePortEndpointChainName(servicePortName string, protocol string, endpo
 
 func isServiceChainName(chainString string) bool {
 	prefixes := []string{
-		servicePortChainNamePrefix,
+		servicePortPolicyClusterChainNamePrefix,
-		serviceLocalChainNamePrefix,
+		servicePortPolicyLocalChainNamePrefix,
 		servicePortEndpointChainNamePrefix,
 		serviceFirewallChainNamePrefix,
-		serviceLBChainNamePrefix,
+		serviceExternalChainNamePrefix,
+		deprecatedServiceLBChainNamePrefix,
 	}
 
 	for _, p := range prefixes {
@@ -989,16 +993,15 @@ func (proxier *Proxier) syncProxyRules() {
 		}
 	}
 
-	// Build rules for each service.
+	// Build rules for each service-port.
 	for svcName, svc := range proxier.serviceMap {
 		svcInfo, ok := svc.(*serviceInfo)
 		if !ok {
 			klog.ErrorS(nil, "Failed to cast serviceInfo", "serviceName", svcName)
 			continue
 		}
-		isIPv6 := netutils.IsIPv6(svcInfo.ClusterIP())
 		protocol := strings.ToLower(string(svcInfo.Protocol()))
-		svcNameString := svcInfo.serviceNameString
+		svcNameString := svcInfo.nameString
 
 		allEndpoints := proxier.endpointsMap[svcName]
 
@@ -1042,80 +1045,108 @@ func (proxier *Proxier) syncProxyRules() {
 			proxier.natRules.Write(args)
 		}
 
-		policyClusterChain := svcInfo.servicePortChainName
+		// These chains represent the sets of endpoints to use when internal or
-		policyLocalChain := svcInfo.serviceLocalChainName
+		// external traffic policy is "Cluster" vs "Local".
-		svcXlbChain := svcInfo.serviceLBChainName
+		clusterPolicyChain := svcInfo.clusterPolicyChainName
+		localPolicyChain := svcInfo.localPolicyChainName
 
-		internalTrafficChain := policyClusterChain
+		// These chains designate which policy chain to use for internal- and
-		externalTrafficChain := policyClusterChain
+		// external-destination traffic.
-
+		internalPolicyChain := clusterPolicyChain
-		if svcInfo.NodeLocalInternal() {
+		externalPolicyChain := clusterPolicyChain
-			internalTrafficChain = policyLocalChain
+		if svcInfo.InternalPolicyLocal() {
+			internalPolicyChain = localPolicyChain
 		}
-		if svcInfo.NodeLocalExternal() {
+		if svcInfo.ExternalPolicyLocal() {
-			externalTrafficChain = svcXlbChain
+			externalPolicyChain = localPolicyChain
 		}
 
+		// These chains are where *ALL* rules which match traffic that is
+		// service-destined should jump.  ClusterIP traffic is considered
+		// "internal" while NodePort, LoadBalancer, and ExternalIPs traffic is
+		// considered "external".
+		internalTrafficChain := internalPolicyChain
+		externalTrafficChain := svcInfo.externalChainName // eventually jumps to externalPolicyChain
+
+		// Declare the clusterPolicyChain if needed.
 		if hasEndpoints && svcInfo.UsesClusterEndpoints() {
 			// Create the Cluster traffic policy chain, retaining counters if possible.
-			if chain, ok := existingNATChains[policyClusterChain]; ok {
+			if chain, ok := existingNATChains[clusterPolicyChain]; ok {
 				proxier.natChains.WriteBytes(chain)
 			} else {
-				proxier.natChains.Write(utiliptables.MakeChainLine(policyClusterChain))
+				proxier.natChains.Write(utiliptables.MakeChainLine(clusterPolicyChain))
 			}
-			activeNATChains[policyClusterChain] = true
+			activeNATChains[clusterPolicyChain] = true
 		}
 
-		if hasEndpoints && svcInfo.ExternallyAccessible() && svcInfo.NodeLocalExternal() {
+		// Declare the localPolicyChain if needed.
-			if chain, ok := existingNATChains[svcXlbChain]; ok {
+		if hasEndpoints && svcInfo.UsesLocalEndpoints() {
+			if chain, ok := existingNATChains[localPolicyChain]; ok {
 				proxier.natChains.WriteBytes(chain)
 			} else {
-				proxier.natChains.Write(utiliptables.MakeChainLine(svcXlbChain))
+				proxier.natChains.Write(utiliptables.MakeChainLine(localPolicyChain))
+			}
+			activeNATChains[localPolicyChain] = true
 		}
-			activeNATChains[svcXlbChain] = true
 
-			// The XLB chain redirects all pod -> external VIP
+		// If any "external" destinations are enabled, set up external traffic
-			// traffic to the Service's ClusterIP instead. This happens
+		// handling.  All captured traffic for all external destinations should
-			// whether or not we have local endpoints; only if localDetector
+		// jump to externalTrafficChain, which will handle some special-cases
-			// is implemented
+		// and then jump to externalPolicyChain.
+		if hasEndpoints && svcInfo.ExternallyAccessible() {
+			if chain, ok := existingNATChains[externalTrafficChain]; ok {
+				proxier.natChains.WriteBytes(chain)
+			} else {
+				proxier.natChains.Write(utiliptables.MakeChainLine(externalTrafficChain))
+			}
+			activeNATChains[externalTrafficChain] = true
+
+			if !svcInfo.ExternalPolicyLocal() {
+				// If we are using non-local endpoints we need to masquerade,
+				// in case we cross nodes.
+				proxier.natRules.Write(
+					"-A", string(externalTrafficChain),
+					"-m", "comment", "--comment", fmt.Sprintf(`"masquerade traffic for %s external destinations"`, svcNameString),
+					"-j", string(KubeMarkMasqChain))
+			} else {
+				// If we are only using same-node endpoints, we can retain the
+				// source IP in most cases.
+
 				if proxier.localDetector.IsImplemented() {
+					// Treat all locally-originated pod -> external destination
+					// traffic as a special-case.  It is subject to neither
+					// form of traffic policy, which simulates going up-and-out
+					// to an external load-balancer and coming back in.
 					proxier.natRules.Write(
-					"-A", string(svcXlbChain),
+						"-A", string(externalTrafficChain),
-					"-m", "comment", "--comment",
+						"-m", "comment", "--comment", fmt.Sprintf(`"pod traffic for %s external destinations"`, svcNameString),
-					`"Redirect pods trying to reach external loadbalancer VIP to clusterIP"`,
 						proxier.localDetector.IfLocal(),
-					"-j", string(policyClusterChain))
+						"-j", string(clusterPolicyChain))
 				}
 
-			// Next, redirect all src-type=LOCAL -> LB IP to the service chain
+				// Locally originated traffic (not a pod, but the host node)
-			// for externalTrafficPolicy=Local This allows traffic originating
+				// still needs masquerade because the LBIP itself is a local
-			// from the host to be redirected to the service correctly,
+				// address, so that will be the chosen source IP.
-			// otherwise traffic to LB IPs are dropped if there are no local
-			// endpoints.
 				proxier.natRules.Write(
-				"-A", string(svcXlbChain),
+					"-A", string(externalTrafficChain),
-				"-m", "comment", "--comment", fmt.Sprintf(`"masquerade LOCAL traffic for %s LB IP"`, svcNameString),
+					"-m", "comment", "--comment", fmt.Sprintf(`"masquerade LOCAL traffic for %s external destinations"`, svcNameString),
 					"-m", "addrtype", "--src-type", "LOCAL",
 					"-j", string(KubeMarkMasqChain))
+
+				// Redirect all src-type=LOCAL -> external destination to the
+				// policy=cluster chain. This allows traffic originating
+				// from the host to be redirected to the service correctly.
 				proxier.natRules.Write(
-				"-A", string(svcXlbChain),
+					"-A", string(externalTrafficChain),
-				"-m", "comment", "--comment", fmt.Sprintf(`"route LOCAL traffic for %s LB IP to service chain"`, svcNameString),
+					"-m", "comment", "--comment", fmt.Sprintf(`"route LOCAL traffic for %s external destinations"`, svcNameString),
 					"-m", "addrtype", "--src-type", "LOCAL",
-				"-j", string(policyClusterChain))
+					"-j", string(clusterPolicyChain))
+			}
 
-			// Everything else goes to the SVL chain
+			// Anything else falls thru to the appropriate policy chain.
 			proxier.natRules.Write(
-				"-A", string(svcXlbChain),
+				"-A", string(externalTrafficChain),
-				"-j", string(policyLocalChain))
+				"-j", string(externalPolicyChain))
-		}
-
-		if hasEndpoints && svcInfo.UsesLocalEndpoints() {
-			if chain, ok := existingNATChains[policyLocalChain]; ok {
-				proxier.natChains.WriteBytes(chain)
-			} else {
-				proxier.natChains.Write(utiliptables.MakeChainLine(policyLocalChain))
-			}
-			activeNATChains[policyLocalChain] = true
 		}
 
 		// Capture the clusterIP.
@@ -1161,37 +1192,14 @@ func (proxier *Proxier) syncProxyRules() {
 		// Capture externalIPs.
 		for _, externalIP := range svcInfo.ExternalIPStrings() {
 			if hasEndpoints {
-				args = append(args[:0],
+				// Send traffic bound for external IPs to the "external
+				// destinations" chain.
+				proxier.natRules.Write(
+					"-A", string(kubeServicesChain),
 					"-m", "comment", "--comment", fmt.Sprintf(`"%s external IP"`, svcNameString),
 					"-m", protocol, "-p", protocol,
 					"-d", externalIP,
 					"--dport", strconv.Itoa(svcInfo.Port()),
-				)
-
-				// We have to SNAT packets to external IPs if externalTrafficPolicy is cluster
-				// and the traffic is NOT Local. Local traffic coming from Pods and Nodes will
-				// be always forwarded to the corresponding Service, so no need to SNAT
-				// If we can't differentiate the local traffic we always SNAT.
-				if !svcInfo.NodeLocalExternal() {
-					appendTo := []string{"-A", string(policyClusterChain)}
-					// This masquerades off-cluster traffic to a External IP.
-					if proxier.localDetector.IsImplemented() {
-						proxier.natRules.Write(
-							appendTo,
-							args,
-							proxier.localDetector.IfNotLocal(),
-							"-j", string(KubeMarkMasqChain))
-					} else {
-						proxier.natRules.Write(
-							appendTo,
-							args,
-							"-j", string(KubeMarkMasqChain))
-					}
-				}
-				// Send traffic bound for external IPs to the service chain.
-				proxier.natRules.Write(
-					"-A", string(kubeServicesChain),
-					args,
 					"-j", string(externalTrafficChain))
 
 			} else {
@@ -1208,45 +1216,48 @@ func (proxier *Proxier) syncProxyRules() {
 		}
 
 		// Capture load-balancer ingress.
-		fwChain := svcInfo.serviceFirewallChainName
+		if len(svcInfo.LoadBalancerIPStrings()) > 0 && hasEndpoints {
-		for _, ingress := range svcInfo.LoadBalancerIPStrings() {
+			// Normally we send LB matches to the "external destination" chain.
-			if hasEndpoints {
+			nextChain := externalTrafficChain
-				// create service firewall chain
+
+			// If the service specifies any LB source ranges, we need to insert
+			// a firewall chain first.
+			if len(svcInfo.LoadBalancerSourceRanges()) > 0 {
+				fwChain := svcInfo.firewallChainName
+
+				// Declare the service firewall chain.
 				if chain, ok := existingNATChains[fwChain]; ok {
 					proxier.natChains.WriteBytes(chain)
 				} else {
 					proxier.natChains.Write(utiliptables.MakeChainLine(fwChain))
 				}
 				activeNATChains[fwChain] = true
-				// The service firewall rules are created based on ServiceSpec.loadBalancerSourceRanges field.
-				// This currently works for loadbalancers that preserves source ips.
-				// For loadbalancers which direct traffic to service NodePort, the firewall rules will not apply.
 
-				args = append(args[:0],
+				// The firewall chain will jump to the "external destination"
+				// chain.
+				nextChain = svcInfo.firewallChainName
+			}
+
+			for _, lbip := range svcInfo.LoadBalancerIPStrings() {
+				proxier.natRules.Write(
 					"-A", string(kubeServicesChain),
 					"-m", "comment", "--comment", fmt.Sprintf(`"%s loadbalancer IP"`, svcNameString),
 					"-m", protocol, "-p", protocol,
-					"-d", ingress,
+					"-d", lbip,
 					"--dport", strconv.Itoa(svcInfo.Port()),
-				)
+					"-j", string(nextChain))
-				// jump to service firewall chain
-				proxier.natRules.Write(args, "-j", string(fwChain))
 
+				// The service firewall rules are created based on the
+				// loadBalancerSourceRanges field.  This only works for
+				// VIP-like loadbalancers that preserve source IPs.  For
+				// loadbalancers which direct traffic to service NodePort, the
+				// firewall rules will not apply.
+				if len(svcInfo.LoadBalancerSourceRanges()) > 0 {
 					args = append(args[:0],
-					"-A", string(fwChain),
+						"-A", string(nextChain),
 						"-m", "comment", "--comment", fmt.Sprintf(`"%s loadbalancer IP"`, svcNameString),
 					)
 
-				// If we are proxying globally, we need to masquerade in case we cross nodes.
-				// If we are proxying only locally, we can retain the source IP.
-				if !svcInfo.NodeLocalExternal() {
-					proxier.natRules.Write(args, "-j", string(KubeMarkMasqChain))
-				}
-
-				if len(svcInfo.LoadBalancerSourceRanges()) == 0 {
-					// allow all sources, so jump directly to the KUBE-SVC or KUBE-XLB chain
-					proxier.natRules.Write(args, "-j", string(externalTrafficChain))
-				} else {
 					// firewall filter based on each source range
 					allowFromNode := false
 					for _, src := range svcInfo.LoadBalancerSourceRanges() {
@@ -1258,67 +1269,48 @@ func (proxier *Proxier) syncProxyRules() {
 							allowFromNode = true
 						}
 					}
-					// generally, ip route rule was added to intercept request to loadbalancer vip from the
+					// For VIP-like LBs, the VIP is often added as a local
-					// loadbalancer's backend hosts. In this case, request will not hit the loadbalancer but loop back directly.
+					// address (via an IP route rule).  In that case, a request
-					// Need to add the following rule to allow request on host.
+					// from a node to the VIP will not hit the loadbalancer but
+					// will loop back with the source IP set to the VIP.  We
+					// need the following rule to allow requests from this node.
 					if allowFromNode {
 						proxier.natRules.Write(
 							args,
-							"-s", ingress,
+							"-s", lbip,
 							"-j", string(externalTrafficChain))
 					}
-				}
+					// If the packet was able to reach the end of firewall chain,
-
+					// then it did not get DNATed.  It means the packet cannot go
-				// If the packet was able to reach the end of firewall chain, then it did not get DNATed.
+					// thru the firewall, then mark it for DROP.
-				// It means the packet cannot go thru the firewall, then mark it for DROP
 					proxier.natRules.Write(args, "-j", string(KubeMarkDropChain))
+				}
+			}
 		} else {
 			// No endpoints.
+			for _, lbip := range svcInfo.LoadBalancerIPStrings() {
 				proxier.filterRules.Write(
 					"-A", string(kubeExternalServicesChain),
 					"-m", "comment", "--comment", fmt.Sprintf(`"%s has no endpoints"`, svcNameString),
 					"-m", protocol, "-p", protocol,
-					"-d", ingress,
+					"-d", lbip,
 					"--dport", strconv.Itoa(svcInfo.Port()),
 					"-j", "REJECT",
 				)
 			}
 		}
 
-		// Capture nodeports.  If we had more than 2 rules it might be
+		// Capture nodeports.
-		// worthwhile to make a new per-service chain for nodeport rules, but
-		// with just 2 rules it ends up being a waste and a cognitive burden.
 		if svcInfo.NodePort() != 0 && len(nodeAddresses) != 0 {
 			if hasEndpoints {
-				args = append(args[:0],
+				// Jump to the external destination chain.  For better or for
+				// worse, nodeports are not subect to loadBalancerSourceRanges,
+				// and we can't change that.
+				proxier.natRules.Write(
+					"-A", string(kubeNodePortsChain),
 					"-m", "comment", "--comment", svcNameString,
 					"-m", protocol, "-p", protocol,
 					"--dport", strconv.Itoa(svcInfo.NodePort()),
-				)
-				if !svcInfo.NodeLocalExternal() {
-					// Nodeports need SNAT, unless they're local.
-					proxier.natRules.Write(
-						"-A", string(policyClusterChain),
-						args,
-						"-j", string(KubeMarkMasqChain))
-				} else {
-					// TODO: Make all nodePorts jump to the firewall chain.
-					// Currently we only create it for loadbalancers (#33586).
-
-					// Fix localhost martian source error
-					loopback := "127.0.0.0/8"
-					if isIPv6 {
-						loopback = "::1/128"
-					}
-					proxier.natRules.Write(
-						"-A", string(kubeNodePortsChain),
-						args,
-						"-s", loopback, "-j", string(KubeMarkMasqChain))
-				}
-				// Jump to the service chain.
-				proxier.natRules.Write(
-					"-A", string(kubeNodePortsChain),
-					args,
 					"-j", string(externalTrafficChain))
 			} else {
 				// No endpoints.
@@ -1347,18 +1339,18 @@ func (proxier *Proxier) syncProxyRules() {
 		}
 
 		if svcInfo.UsesClusterEndpoints() {
-			// Write rules jumping from policyClusterChain to clusterEndpoints
+			// Write rules jumping from clusterPolicyChain to clusterEndpoints
-			proxier.writeServiceToEndpointRules(svcNameString, svcInfo, policyClusterChain, clusterEndpoints, args)
+			proxier.writeServiceToEndpointRules(svcNameString, svcInfo, clusterPolicyChain, clusterEndpoints, args)
 		}
 
 		if svcInfo.UsesLocalEndpoints() {
 			if len(localEndpoints) != 0 {
-				// Write rules jumping from policyLocalChain to localEndpointChains
+				// Write rules jumping from localPolicyChain to localEndpointChains
-				proxier.writeServiceToEndpointRules(svcNameString, svcInfo, policyLocalChain, localEndpoints, args)
+				proxier.writeServiceToEndpointRules(svcNameString, svcInfo, localPolicyChain, localEndpoints, args)
 			} else if hasEndpoints {
 				// Blackhole all traffic since there are no local endpoints
 				args = append(args[:0],
-					"-A", string(policyLocalChain),
+					"-A", string(localPolicyChain),
 					"-m", "comment", "--comment",
 					fmt.Sprintf(`"%s has no local endpoints"`, svcNameString),
 					"-j",
@@ -1388,14 +1380,12 @@ func (proxier *Proxier) syncProxyRules() {
 	// Finally, tail-call to the nodeports chain.  This needs to be after all
 	// other service portal rules.
 	for address := range nodeAddresses {
-		// TODO(thockin, m1093782566): If/when we have dual-stack support we will want to distinguish v4 from v6 zero-CIDRs.
 		if utilproxy.IsZeroCIDR(address) {
-			args = append(args[:0],
+			proxier.natRules.Write(
 				"-A", string(kubeServicesChain),
 				"-m", "comment", "--comment", `"kubernetes service nodeports; NOTE: this must be the last rule in this chain"`,
 				"-m", "addrtype", "--dst-type", "LOCAL",
 				"-j", string(kubeNodePortsChain))
-			proxier.natRules.Write(args)
 			// Nothing else matters after the zero CIDR.
 			break
 		}
@@ -1405,12 +1395,11 @@ func (proxier *Proxier) syncProxyRules() {
 			continue
 		}
 		// create nodeport rules for each IP one by one
-		args = append(args[:0],
+		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))
-		proxier.natRules.Write(args)
 	}
 
 	// Drop the packets in INVALID state, which would potentially cause
@@ -1532,10 +1521,12 @@ func (proxier *Proxier) writeServiceToEndpointRules(svcNameString string, svcInf
 			if !ok {
 				continue
 			}
+			comment := fmt.Sprintf(`"%s -> %s"`, svcNameString, epInfo.Endpoint)
+
 			args = append(args[:0],
 				"-A", string(svcChain),
 			)
-			args = proxier.appendServiceCommentLocked(args, svcNameString)
+			args = proxier.appendServiceCommentLocked(args, comment)
 			args = append(args,
 				"-m", "recent", "--name", string(epInfo.ChainName),
 				"--rcheck", "--seconds", strconv.Itoa(svcInfo.StickyMaxAgeSeconds()), "--reap",
@@ -1552,9 +1543,10 @@ func (proxier *Proxier) writeServiceToEndpointRules(svcNameString string, svcInf
 		if !ok {
 			continue
 		}
+		comment := fmt.Sprintf(`"%s -> %s"`, svcNameString, epInfo.Endpoint)
 
 		args = append(args[:0], "-A", string(svcChain))
-		args = proxier.appendServiceCommentLocked(args, svcNameString)
+		args = proxier.appendServiceCommentLocked(args, comment)
 		if i < (numEndpoints - 1) {
 			// Each rule is a probabilistic match.
 			args = append(args,

pkg/proxy/ipvs/proxier.go

@@ -1202,7 +1202,7 @@ func (proxier *Proxier) syncProxyRules() {
 			// ExternalTrafficPolicy only works for NodePort and external LB traffic, does not affect ClusterIP
 			// So we still need clusterIP rules in onlyNodeLocalEndpoints mode.
 			internalNodeLocal := false
-			if utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) && svcInfo.NodeLocalInternal() {
+			if utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) && svcInfo.InternalPolicyLocal() {
 				internalNodeLocal = true
 			}
 			if err := proxier.syncEndpoint(svcName, internalNodeLocal, serv); err != nil {
@@ -1222,7 +1222,7 @@ func (proxier *Proxier) syncProxyRules() {
 				SetType:  utilipset.HashIPPort,
 			}
 
-			if svcInfo.NodeLocalExternal() {
+			if svcInfo.ExternalPolicyLocal() {
 				if valid := proxier.ipsetList[kubeExternalIPLocalSet].validateEntry(entry); !valid {
 					klog.ErrorS(nil, "Error adding entry to ipset", "entry", entry, "ipset", proxier.ipsetList[kubeExternalIPLocalSet].Name)
 					continue
@@ -1252,7 +1252,7 @@ func (proxier *Proxier) syncProxyRules() {
 				activeIPVSServices[serv.String()] = true
 				activeBindAddrs[serv.Address.String()] = true
 
-				if err := proxier.syncEndpoint(svcName, svcInfo.NodeLocalExternal(), serv); err != nil {
+				if err := proxier.syncEndpoint(svcName, svcInfo.ExternalPolicyLocal(), serv); err != nil {
 					klog.ErrorS(err, "Failed to sync endpoint for service", "serviceName", svcName, "virtualServer", serv)
 				}
 			} else {
@@ -1279,7 +1279,7 @@ func (proxier *Proxier) syncProxyRules() {
 			}
 			proxier.ipsetList[kubeLoadBalancerSet].activeEntries.Insert(entry.String())
 			// insert loadbalancer entry to lbIngressLocalSet if service externaltrafficpolicy=local
-			if svcInfo.NodeLocalExternal() {
+			if svcInfo.ExternalPolicyLocal() {
 				if valid := proxier.ipsetList[kubeLoadBalancerLocalSet].validateEntry(entry); !valid {
 					klog.ErrorS(nil, "Error adding entry to ipset", "entry", entry, "ipset", proxier.ipsetList[kubeLoadBalancerLocalSet].Name)
 					continue
@@ -1351,7 +1351,7 @@ func (proxier *Proxier) syncProxyRules() {
 			if err := proxier.syncService(svcNameString, serv, true, bindedAddresses); err == nil {
 				activeIPVSServices[serv.String()] = true
 				activeBindAddrs[serv.Address.String()] = true
-				if err := proxier.syncEndpoint(svcName, svcInfo.NodeLocalExternal(), serv); err != nil {
+				if err := proxier.syncEndpoint(svcName, svcInfo.ExternalPolicyLocal(), serv); err != nil {
 					klog.ErrorS(err, "Failed to sync endpoint for service", "serviceName", svcName, "virtualServer", serv)
 				}
 			} else {
@@ -1449,7 +1449,7 @@ func (proxier *Proxier) syncProxyRules() {
 			}
 
 			// Add externaltrafficpolicy=local type nodeport entry
-			if svcInfo.NodeLocalExternal() {
+			if svcInfo.ExternalPolicyLocal() {
 				var nodePortLocalSet *IPSet
 				switch protocol {
 				case utilipset.ProtocolTCP:
@@ -1494,7 +1494,7 @@ func (proxier *Proxier) syncProxyRules() {
 				// There is no need to bind Node IP to dummy interface, so set parameter `bindAddr` to `false`.
 				if err := proxier.syncService(svcNameString, serv, false, bindedAddresses); err == nil {
 					activeIPVSServices[serv.String()] = true
-					if err := proxier.syncEndpoint(svcName, svcInfo.NodeLocalExternal(), serv); err != nil {
+					if err := proxier.syncEndpoint(svcName, svcInfo.ExternalPolicyLocal(), serv); err != nil {
 						klog.ErrorS(err, "Failed to sync endpoint for service", "serviceName", svcName, "virtualServer", serv)
 					}
 				} else {

pkg/proxy/service.go

@@ -52,8 +52,8 @@ type BaseServiceInfo struct {
 	externalIPs              []string
 	loadBalancerSourceRanges []string
 	healthCheckNodePort      int
-	nodeLocalExternal        bool
+	externalPolicyLocal      bool
-	nodeLocalInternal        bool
+	internalPolicyLocal      bool
 	internalTrafficPolicy    *v1.ServiceInternalTrafficPolicyType
 	hintsAnnotation          string
 }
@@ -119,14 +119,14 @@ func (info *BaseServiceInfo) LoadBalancerIPStrings() []string {
 	return ips
 }
 
-// NodeLocalExternal is part of ServicePort interface.
+// ExternalPolicyLocal is part of ServicePort interface.
-func (info *BaseServiceInfo) NodeLocalExternal() bool {
+func (info *BaseServiceInfo) ExternalPolicyLocal() bool {
-	return info.nodeLocalExternal
+	return info.externalPolicyLocal
 }
 
-// NodeLocalInternal is part of ServicePort interface
+// InternalPolicyLocal is part of ServicePort interface
-func (info *BaseServiceInfo) NodeLocalInternal() bool {
+func (info *BaseServiceInfo) InternalPolicyLocal() bool {
-	return info.nodeLocalInternal
+	return info.internalPolicyLocal
 }
 
 // InternalTrafficPolicy is part of ServicePort interface
@@ -149,22 +149,22 @@ func (info *BaseServiceInfo) UsesClusterEndpoints() bool {
 	// The service port uses Cluster endpoints if the internal traffic policy is "Cluster",
 	// or if it accepts external traffic at all. (Even if the external traffic policy is
 	// "Local", we need Cluster endpoints to implement short circuiting.)
-	return !info.nodeLocalInternal || info.ExternallyAccessible()
+	return !info.internalPolicyLocal || info.ExternallyAccessible()
 }
 
 // UsesLocalEndpoints is part of ServicePort interface.
 func (info *BaseServiceInfo) UsesLocalEndpoints() bool {
-	return info.nodeLocalInternal || (info.nodeLocalExternal && info.ExternallyAccessible())
+	return info.internalPolicyLocal || (info.externalPolicyLocal && info.ExternallyAccessible())
 }
 
 func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, service *v1.Service) *BaseServiceInfo {
-	nodeLocalExternal := false
+	externalPolicyLocal := false
-	if apiservice.RequestsOnlyLocalTraffic(service) {
+	if apiservice.ExternalPolicyLocal(service) {
-		nodeLocalExternal = true
+		externalPolicyLocal = true
 	}
-	nodeLocalInternal := false
+	internalPolicyLocal := false
 	if utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) {
-		nodeLocalInternal = apiservice.RequestsOnlyLocalTrafficForInternal(service)
+		internalPolicyLocal = apiservice.InternalPolicyLocal(service)
 	}
 	var stickyMaxAgeSeconds int
 	if service.Spec.SessionAffinity == v1.ServiceAffinityClientIP {
@@ -180,8 +180,8 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic
 		nodePort:              int(port.NodePort),
 		sessionAffinityType:   service.Spec.SessionAffinity,
 		stickyMaxAgeSeconds:   stickyMaxAgeSeconds,
-		nodeLocalExternal:     nodeLocalExternal,
+		externalPolicyLocal:   externalPolicyLocal,
-		nodeLocalInternal:     nodeLocalInternal,
+		internalPolicyLocal:   internalPolicyLocal,
 		internalTrafficPolicy: service.Spec.InternalTrafficPolicy,
 		hintsAnnotation:       service.Annotations[v1.AnnotationTopologyAwareHints],
 	}

pkg/proxy/topology_test.go

@@ -120,7 +120,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 		name:         "externalTrafficPolicy: Local, topology ignored for Local endpoints",
 		hintsEnabled: true,
 		nodeLabels:   map[string]string{v1.LabelTopologyZone: "zone-a"},
-		serviceInfo:  &BaseServiceInfo{nodeLocalExternal: true, nodePort: 8080, hintsAnnotation: "auto"},
+		serviceInfo:  &BaseServiceInfo{externalPolicyLocal: true, nodePort: 8080, hintsAnnotation: "auto"},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true, IsLocal: true},
 			&BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true, IsLocal: true},
@@ -134,7 +134,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 		name:         "internalTrafficPolicy: Local, topology ignored for Local endpoints",
 		hintsEnabled: true,
 		nodeLabels:   map[string]string{v1.LabelTopologyZone: "zone-a"},
-		serviceInfo:  &BaseServiceInfo{nodeLocalInternal: true, hintsAnnotation: "auto", nodeLocalExternal: false, nodePort: 8080},
+		serviceInfo:  &BaseServiceInfo{internalPolicyLocal: true, hintsAnnotation: "auto", externalPolicyLocal: false, nodePort: 8080},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true, IsLocal: true},
 			&BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true, IsLocal: true},
@@ -282,7 +282,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 		name:         "conflicting topology and localness require merging allEndpoints",
 		hintsEnabled: true,
 		nodeLabels:   map[string]string{v1.LabelTopologyZone: "zone-a"},
-		serviceInfo:  &BaseServiceInfo{nodeLocalInternal: false, nodeLocalExternal: true, nodePort: 8080, hintsAnnotation: "auto"},
+		serviceInfo:  &BaseServiceInfo{internalPolicyLocal: false, externalPolicyLocal: true, nodePort: 8080, hintsAnnotation: "auto"},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", ZoneHints: sets.NewString("zone-a"), Ready: true, IsLocal: true},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", ZoneHints: sets.NewString("zone-b"), Ready: true, IsLocal: true},
@@ -294,13 +294,13 @@ func TestCategorizeEndpoints(t *testing.T) {
 		allEndpoints:     sets.NewString("10.0.0.0:80", "10.0.0.1:80", "10.0.0.2:80"),
 	}, {
 		name:             "internalTrafficPolicy: Local, with empty endpoints",
-		serviceInfo:      &BaseServiceInfo{nodeLocalInternal: true},
+		serviceInfo:      &BaseServiceInfo{internalPolicyLocal: true},
 		endpoints:        []Endpoint{},
 		clusterEndpoints: nil,
 		localEndpoints:   sets.NewString(),
 	}, {
 		name:        "internalTrafficPolicy: Local, but all endpoints are remote",
-		serviceInfo: &BaseServiceInfo{nodeLocalInternal: true},
+		serviceInfo: &BaseServiceInfo{internalPolicyLocal: true},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: false},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: false},
@@ -310,7 +310,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 		onlyRemoteEndpoints: true,
 	}, {
 		name:        "internalTrafficPolicy: Local, all endpoints are local",
-		serviceInfo: &BaseServiceInfo{nodeLocalInternal: true},
+		serviceInfo: &BaseServiceInfo{internalPolicyLocal: true},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: true},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: true},
@@ -319,7 +319,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 		localEndpoints:   sets.NewString("10.0.0.0:80", "10.0.0.1:80"),
 	}, {
 		name:        "internalTrafficPolicy: Local, some endpoints are local",
-		serviceInfo: &BaseServiceInfo{nodeLocalInternal: true},
+		serviceInfo: &BaseServiceInfo{internalPolicyLocal: true},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: true},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: false},
@@ -366,7 +366,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 		localEndpoints:   nil,
 	}, {
 		name:        "iTP: Local, eTP: Cluster, some endpoints local",
-		serviceInfo: &BaseServiceInfo{nodeLocalInternal: true, nodeLocalExternal: false, nodePort: 8080},
+		serviceInfo: &BaseServiceInfo{internalPolicyLocal: true, externalPolicyLocal: false, nodePort: 8080},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: true},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: false},
@@ -376,7 +376,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 		allEndpoints:     sets.NewString("10.0.0.0:80", "10.0.0.1:80"),
 	}, {
 		name:        "iTP: Cluster, eTP: Local, some endpoints local",
-		serviceInfo: &BaseServiceInfo{nodeLocalInternal: false, nodeLocalExternal: true, nodePort: 8080},
+		serviceInfo: &BaseServiceInfo{internalPolicyLocal: false, externalPolicyLocal: true, nodePort: 8080},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: true},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: false},
@@ -386,7 +386,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 		allEndpoints:     sets.NewString("10.0.0.0:80", "10.0.0.1:80"),
 	}, {
 		name:        "iTP: Local, eTP: Local, some endpoints local",
-		serviceInfo: &BaseServiceInfo{nodeLocalInternal: true, nodeLocalExternal: true, nodePort: 8080},
+		serviceInfo: &BaseServiceInfo{internalPolicyLocal: true, externalPolicyLocal: true, nodePort: 8080},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: true},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: false},
@@ -396,7 +396,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 		allEndpoints:     sets.NewString("10.0.0.0:80", "10.0.0.1:80"),
 	}, {
 		name:        "iTP: Local, eTP: Local, all endpoints remote",
-		serviceInfo: &BaseServiceInfo{nodeLocalInternal: true, nodeLocalExternal: true, nodePort: 8080},
+		serviceInfo: &BaseServiceInfo{internalPolicyLocal: true, externalPolicyLocal: true, nodePort: 8080},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: false},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: false},
@@ -406,7 +406,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 		allEndpoints:     sets.NewString("10.0.0.0:80", "10.0.0.1:80"),
 	}, {
 		name:        "iTP: Local, eTP: Local, PTE disabled, all endpoints remote and terminating",
-		serviceInfo: &BaseServiceInfo{nodeLocalInternal: true, nodeLocalExternal: true, nodePort: 8080},
+		serviceInfo: &BaseServiceInfo{internalPolicyLocal: true, externalPolicyLocal: true, nodePort: 8080},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: false, Serving: true, Terminating: true, IsLocal: false},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: false, Serving: true, Terminating: true, IsLocal: false},
@@ -417,7 +417,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 	}, {
 		name:        "iTP: Local, eTP: Local, PTE enabled, all endpoints remote and terminating",
 		pteEnabled:  true,
-		serviceInfo: &BaseServiceInfo{nodeLocalInternal: true, nodeLocalExternal: true, nodePort: 8080},
+		serviceInfo: &BaseServiceInfo{internalPolicyLocal: true, externalPolicyLocal: true, nodePort: 8080},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: false, Serving: true, Terminating: true, IsLocal: false},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: false, Serving: true, Terminating: true, IsLocal: false},
@@ -428,7 +428,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 		onlyRemoteEndpoints: true,
 	}, {
 		name:        "iTP: Cluster, eTP: Local, PTE disabled, with terminating endpoints",
-		serviceInfo: &BaseServiceInfo{nodeLocalInternal: false, nodeLocalExternal: true, nodePort: 8080},
+		serviceInfo: &BaseServiceInfo{internalPolicyLocal: false, externalPolicyLocal: true, nodePort: 8080},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: false},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: false, Serving: false, IsLocal: true},
@@ -441,7 +441,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 	}, {
 		name:        "iTP: Cluster, eTP: Local, PTE enabled, with terminating endpoints",
 		pteEnabled:  true,
-		serviceInfo: &BaseServiceInfo{nodeLocalInternal: false, nodeLocalExternal: true, nodePort: 8080},
+		serviceInfo: &BaseServiceInfo{internalPolicyLocal: false, externalPolicyLocal: true, nodePort: 8080},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: false},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: false, Serving: false, IsLocal: true},
@@ -453,7 +453,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 		allEndpoints:     sets.NewString("10.0.0.0:80", "10.0.0.2:80"),
 	}, {
 		name:        "externalTrafficPolicy ignored if not externally accessible",
-		serviceInfo: &BaseServiceInfo{nodeLocalExternal: true},
+		serviceInfo: &BaseServiceInfo{externalPolicyLocal: true},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: false},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: true},
@@ -463,7 +463,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 		allEndpoints:     sets.NewString("10.0.0.0:80", "10.0.0.1:80"),
 	}, {
 		name:        "no cluster endpoints for iTP:Local internal-only service",
-		serviceInfo: &BaseServiceInfo{nodeLocalInternal: true},
+		serviceInfo: &BaseServiceInfo{internalPolicyLocal: true},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: false},
 			&BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: true},

pkg/proxy/types.go

@@ -83,10 +83,10 @@ type ServicePort interface {
 	HealthCheckNodePort() int
 	// GetNodePort returns a service Node port if present. If return 0, it means not present.
 	NodePort() int
-	// NodeLocalExternal returns if a service has only node local endpoints for external traffic.
+	// ExternalPolicyLocal returns if a service has only node local endpoints for external traffic.
-	NodeLocalExternal() bool
+	ExternalPolicyLocal() bool
-	// NodeLocalInternal returns if a service has only node local endpoints for internal traffic.
+	// InternalPolicyLocal returns if a service has only node local endpoints for internal traffic.
-	NodeLocalInternal() bool
+	InternalPolicyLocal() bool
 	// InternalTrafficPolicy returns service InternalTrafficPolicy
 	InternalTrafficPolicy() *v1.ServiceInternalTrafficPolicyType
 	// HintsAnnotation returns the value of the v1.AnnotationTopologyAwareHints annotation.