Skip to content

Commit

Permalink
fix(NSC): ensure kube-router owns kube-router-svip
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aauren committed Jul 31, 2024
1 parent e8962dd commit 64061c1
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 1 deletion.
10 changes: 9 additions & 1 deletion pkg/controllers/proxy/network_services_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -673,13 +673,21 @@ func (nsc *NetworkServicesController) syncIpvsFirewall() error {
var port int
if ipvsService.Address != nil {
address = ipvsService.Address
port = int(ipvsService.Port)

_, err := nsc.lookupServiceByAddressOrNodePort(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
}

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)
Expand Down
46 changes: 46 additions & 0 deletions pkg/controllers/proxy/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,52 @@ func (nsc *NetworkServicesController) lookupServiceByFWMark(fwMark uint32) (stri
return serviceKeySplit[0], serviceKeySplit[1], int(port), nil
}

// lookupServiceByAddressOrNodePort looks up a service by its clusterIP, externalIP, or loadBalancerIP. It returns the serviceInfo
func (nsc *NetworkServicesController) lookupServiceByAddressOrNodePort(address net.IP,
nodePort int) (*serviceInfo, error) {
for _, svc := range nsc.serviceMap {
for _, clIP := range svc.clusterIPs {
if net.ParseIP(clIP).Equal(address) {
return svc, nil
}
}
for _, exIP := range svc.externalIPs {
if net.ParseIP(exIP).Equal(address) {
return svc, nil
}
}
for _, lbIP := range svc.loadBalancerIPs {
if net.ParseIP(lbIP).Equal(address) {
return svc, nil
}
}
if nodePort != 0 && svc.nodePort == nodePort {
if nsc.nodeportBindOnAllIP {
addrMap, err := getAllLocalIPs()
if err != nil {
return nil, 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 svc, nil
}
}
} else {
if address.Equal(nsc.primaryIP) {
return svc, nil
}
}
}
}
return nil, 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 {
Expand Down
56 changes: 56 additions & 0 deletions pkg/controllers/proxy/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,3 +165,59 @@ func TestNetworkServicesController_getLabelFromMap(t *testing.T) {
assert.Nil(t, err, "error should be nil when the label exists")
})
}

func TestLookupServiceByAddress(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.2"},
externalIPs: []string{"192.168.1.2"},
loadBalancerIPs: []string{"172.16.0.2"},
}

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 *serviceInfo
err error
}{
{net.ParseIP("10.0.0.1"), 0, service1, nil},
{net.ParseIP("192.168.1.1"), 0, service1, nil},
{net.ParseIP("172.16.0.1"), 0, service1, nil},
{net.ParseIP("10.0.0.2"), 0, service2, nil},
{net.ParseIP("192.168.1.2"), 0, service2, nil},
{net.ParseIP("172.16.0.2"), 0, service2, nil},
{net.ParseIP("192.168.1.10"), 30000, service1, nil},
{net.ParseIP("192.168.1.10"), 30001, service2, nil},
{net.ParseIP("192.168.1.3"), 0, nil, fmt.Errorf("service not found for address 192.168.1.3")},
{net.ParseIP("192.168.1.10"), 0, nil, fmt.Errorf("service not found for address 192.168.1.10")},
}

for _, test := range tests {
result, err := nsc.lookupServiceByAddressOrNodePort(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)
}
}
}

0 comments on commit 64061c1

Please sign in to comment.