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 4f3e706 commit b217e7b
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 1 deletion.
14 changes: 13 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,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)
Expand Down
44 changes: 44 additions & 0 deletions pkg/controllers/proxy/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
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 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)
}
}
}

0 comments on commit b217e7b

Please sign in to comment.