Repository: cloudnativelabs/kube-router
Pull request #1699 (2024-07-05 19:28:32)

fix(NSC): ensure kube-router owns kube-router-svip
Currently, kube-router just lists all IPVS services on the host and then adds the load balancing service IPs to kube-router-svip blindly. However, this assumes that the only IPVS entries are entries that kube-router has originated and that the user isn't using IPVS.

We want to make sure that we are only creating rules for services that we are authoritative for. So to this end, we now double-check that this is one of our services before adding rules that may effect it.

Fixes: #1685 


Patch:
diff --git a/pkg/controllers/proxy/network_services_controller.go b/pkg/controllers/proxy/network_services_controller.go
index cbe57ffc..4410a32d 100644
--- a/pkg/controllers/proxy/network_services_controller.go
+++ b/pkg/controllers/proxy/network_services_controller.go
@@ -673,13 +673,25 @@ func (nsc *NetworkServicesController) syncIpvsFirewall() error {
 		var port int
 		if ipvsService.Address != nil {
 			address = ipvsService.Address
+			port = int(ipvsService.Port)
+
+			isValid, err := nsc.isValidKubeRouterServiceArtifact(address, port)
+			if err != nil {
+				klog.Infof("failed to lookup service by address %s: %v - this does not appear to be a kube-router "+
+					"controlled service, skipping...", address, err)
+				continue
+			}
+			if !isValid {
+				klog.Infof("address %s is not a valid kube-router controlled service, skipping...", address)
+				continue
+			}
+
 			protocol = convertSysCallProtoToSvcProto(ipvsService.Protocol)
 			if protocol == noneProtocol {
 				klog.Warningf("failed to convert protocol %d to a valid IPVS protocol for service: %s skipping",
 					ipvsService.Protocol, ipvsService.Address.String())
 				continue
 			}
-			port = int(ipvsService.Port)
 		} else if ipvsService.FWMark != 0 {
 			var ipString string
 			ipString, protocol, port, err = nsc.lookupServiceByFWMark(ipvsService.FWMark)
@@ -2103,6 +2115,11 @@ func NewNetworkServicesController(clientset kubernetes.Interface,
 		return nil, err
 	}
 
+	// This function is responsible for quite a bit:
+	// * Sets nsc.nodeIPv4Addrs & nsc.isIPv4Capable
+	// * Sets nsc.nodeIPv6Addr & nsc.isIPv6Capable
+	// * Creates the iptables handlers for ipv4 & ipv6
+	// * Creates the ipset handlers for ipv4 & ipv6
 	err = nsc.setupHandlers(config, node)
 	if err != nil {
 		return nil, err
diff --git a/pkg/controllers/proxy/utils.go b/pkg/controllers/proxy/utils.go
index 5afe6089..c0e3e35d 100644
--- a/pkg/controllers/proxy/utils.go
+++ b/pkg/controllers/proxy/utils.go
@@ -122,6 +122,50 @@ func (nsc *NetworkServicesController) lookupServiceByFWMark(fwMark uint32) (stri
 	return serviceKeySplit[0], serviceKeySplit[1], int(port), nil
 }
 
+// isValidKubeRouterServiceArtifact looks up a service by its clusterIP, externalIP, or loadBalancerIP. It returns
+// truthy
+func (nsc *NetworkServicesController) isValidKubeRouterServiceArtifact(address net.IP, nodePort int) (bool, error) {
+	for _, svc := range nsc.serviceMap {
+		for _, clIP := range svc.clusterIPs {
+			if net.ParseIP(clIP).Equal(address) {
+				return true, nil
+			}
+		}
+		for _, exIP := range svc.externalIPs {
+			if net.ParseIP(exIP).Equal(address) {
+				return true, nil
+			}
+		}
+		for _, lbIP := range svc.loadBalancerIPs {
+			if net.ParseIP(lbIP).Equal(address) {
+				return true, nil
+			}
+		}
+		if nodePort != 0 && svc.nodePort == nodePort {
+			if nsc.nodeportBindOnAllIP {
+				addrMap, err := getAllLocalIPs()
+				if err != nil {
+					return false, fmt.Errorf("failed to get all local IPs: %v", err)
+				}
+				var addresses []net.IP
+				if address.To4() != nil {
+					addresses = addrMap[v1.IPv4Protocol]
+				} else {
+					addresses = addrMap[v1.IPv6Protocol]
+				}
+				for _, addr := range addresses {
+					if addr.Equal(address) {
+						return true, nil
+					}
+				}
+			} else if address.Equal(nsc.primaryIP) {
+				return true, nil
+			}
+		}
+	}
+	return false, fmt.Errorf("service not found for address %s", address.String())
+}
+
 // unsortedListsEquivalent compares two lists of endpointsInfo and considers them the same if they contains the same
 // contents regardless of order. Returns true if both lists contain the same contents.
 func unsortedListsEquivalent(a, b []endpointSliceInfo) bool {
@@ -519,7 +563,7 @@ func getAllLocalIPs() (map[v1.IPFamily][]net.IP, error) {
 			continue
 		}
 
-		linkAddrs, err := netlink.AddrList(link, netlink.FAMILY_V4)
+		linkAddrs, err := netlink.AddrList(link, netlink.FAMILY_ALL)
 		if err != nil {
 			return nil, fmt.Errorf("failed to get IPs for interface: %v", err)
 		}


Test patch:
diff --git a/pkg/controllers/proxy/utils_test.go b/pkg/controllers/proxy/utils_test.go
index 3bc2f888..a313ed6f 100644
--- a/pkg/controllers/proxy/utils_test.go
+++ b/pkg/controllers/proxy/utils_test.go
@@ -165,3 +165,59 @@ func TestNetworkServicesController_getLabelFromMap(t *testing.T) {
 		assert.Nil(t, err, "error should be nil when the label exists")
 	})
 }
+
+func TestIsValidKubeRouterServiceArtifact(t *testing.T) {
+	// Mock data
+	service1 := &serviceInfo{
+		clusterIPs:      []string{"10.0.0.1"},
+		externalIPs:     []string{"192.168.1.1"},
+		loadBalancerIPs: []string{"172.16.0.1"},
+		nodePort:        30000,
+	}
+	service2 := &serviceInfo{
+		clusterIPs:      []string{"10.0.0.2"},
+		externalIPs:     []string{"192.168.1.2"},
+		loadBalancerIPs: []string{"172.16.0.2"},
+		nodePort:        30001,
+	}
+	service3 := &serviceInfo{
+		clusterIPs:      []string{"10.0.0.3"},
+		externalIPs:     []string{"192.168.1.3"},
+		loadBalancerIPs: []string{"172.16.0.3"},
+	}
+
+	nsc := &NetworkServicesController{
+		serviceMap: map[string]*serviceInfo{
+			"service1": service1,
+			"service2": service2,
+			"service3": service3,
+		},
+		nodeportBindOnAllIP: false,
+		primaryIP:           net.ParseIP("192.168.1.10"),
+	}
+
+	tests := []struct {
+		address  net.IP
+		port     int
+		expected bool
+		err      error
+	}{
+		{net.ParseIP("10.0.0.1"), 0, true, nil},
+		{net.ParseIP("192.168.1.1"), 0, true, nil},
+		{net.ParseIP("172.16.0.1"), 0, true, nil},
+		{net.ParseIP("10.0.0.2"), 0, true, nil},
+		{net.ParseIP("192.168.1.2"), 0, true, nil},
+		{net.ParseIP("172.16.0.2"), 0, true, nil},
+		{net.ParseIP("192.168.1.10"), 30000, true, nil},
+		{net.ParseIP("192.168.1.10"), 30001, true, nil},
+		{net.ParseIP("192.168.1.4"), 0, false, fmt.Errorf("service not found for address 192.168.1.4")},
+		{net.ParseIP("192.168.1.10"), 0, false, fmt.Errorf("service not found for address 192.168.1.10")},
+	}
+
+	for _, test := range tests {
+		result, err := nsc.isValidKubeRouterServiceArtifact(test.address, test.port)
+		if result != test.expected || (err != nil && err.Error() != test.err.Error()) {
+			t.Errorf("lookupServiceByAddress(%v) = %v, %v; want %v, %v", test.address, result, err, test.expected, test.err)
+		}
+	}
+}