Skip to content

Commit

Permalink
Filter dest addresses based on IngressIP
Browse files Browse the repository at this point in the history 
The host nodes of a cluster can have IPv4 and/or IPv6 addresses.
The controller does not distinguish the address families, which leads to
the fact that all addresses will be used as Destination Addresses
independent of the preferred IPFamily of the Service or the Ingress IP.

This commit adjusts getDestinationAddress to return two lists, one for
each supported address family.
To do so, functions to check the correctness of a textual representation
of an IP and to check the address family got implemented.
golang does not natively support the determination of the address family,
which is why the presence of ":" and "." is checked.

The GenerateModel function has been adjusted to use only IP addresses
of the same address family as the IngressIP as destination addresses.

Fixes cloudandheat#25
  • Loading branch information
@sstrk
sstrk committed Apr 27, 2021
1 parent b682256 commit 692ebfe
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 19 deletions.
65 changes: 58 additions & 7 deletions internal/controller/model_nodeport.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,23 @@
package controller

import (
"errors"
"net"
"strings"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/labels"
corelisters "k8s.io/client-go/listers/core/v1"
"k8s.io/klog"

"github.com/cloudandheat/ch-k8s-lbaas/internal/model"
"github.com/cloudandheat/ch-k8s-lbaas/internal/openstack"
)

var (
ErrInvalidIpAddress = errors.New("the string is not a valid textual representation of an IP address")
)

type NodePortLoadBalancerModelGenerator struct {
l3portmanager openstack.L3PortManager
services corelisters.ServiceLister
Expand All @@ -40,27 +49,51 @@ func NewNodePortLoadBalancerModelGenerator(
}
}

func (g *NodePortLoadBalancerModelGenerator) getDestinationAddresses() (result []string, err error) {
func validateIpAddress(ipString string) error {
ipParsed := net.ParseIP(ipString)
if ipParsed != nil {
return nil
}
return ErrInvalidIpAddress
}

func isIPv4Address(ipString string) bool {
return strings.Count(ipString, ":") == 0 && strings.Count(ipString, ".") == 3
}

func isIPv6Address(ipString string) bool {
return strings.Count(ipString, ":") >= 2
}

func (g *NodePortLoadBalancerModelGenerator) getDestinationAddresses() (addressesV4 []string, addressesV6 []string, err error) {
nodes, err := g.nodes.List(labels.Everything())
if err != nil {
return nil, err
return nil, nil, err
}

result = []string{}
for _, node := range nodes {
for _, addr := range node.Status.Addresses {
if addr.Type != corev1.NodeInternalIP {
continue
}
result = append(result, addr.Address)
if validateIpAddress(addr.Address) != nil {
continue
}
if isIPv4Address(addr.Address) {
addressesV4 = append(addressesV4, addr.Address)
} else if isIPv6Address(addr.Address) {
addressesV6 = append(addressesV6, addr.Address)
} else {
continue
}
}
}

return result, nil
return addressesV4, addressesV6, nil
}

func (g *NodePortLoadBalancerModelGenerator) GenerateModel(portAssignment map[string]string) (*model.LoadBalancer, error) {
addresses, err := g.getDestinationAddresses()
addressesV4, addressesV6, err := g.getDestinationAddresses()
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -88,12 +121,30 @@ func (g *NodePortLoadBalancerModelGenerator) GenerateModel(portAssignment map[st
}
}

if validateIpAddress(ingress.Address) != nil {
continue
}

var destAddresses []string

if isIPv4Address(ingress.Address) {
destAddresses = append(destAddresses, addressesV4...)
} else if isIPv6Address(ingress.Address) {
destAddresses = append(destAddresses, addressesV6...)
} else {
klog.Warningf(
"could not determine address family of ingress IP %q for service %q",
ingress.Address,
svc.Name)
continue
}

for _, svcPort := range svc.Spec.Ports {
ingress.Ports = append(ingress.Ports, model.PortForward{
Protocol: svcPort.Protocol,
InboundPort: svcPort.Port,
DestinationPort: svcPort.NodePort,
DestinationAddresses: addresses,
DestinationAddresses: destAddresses,
})
}

Expand Down
24 changes: 12 additions & 12 deletions internal/controller/model_nodeport_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,15 @@ func TestNodePortSinglePortSingleServiceAssignment(t *testing.T) {
model.FromService(svc).ToKey(): "port-id-1",
}

f.l3portmanager.On("GetInternalAddress", "port-id-1").Return("ingress-ip-1", nil).Times(1)
f.l3portmanager.On("GetInternalAddress", "port-id-1").Return("10.0.0.2", nil).Times(1)

f.runWith(func(g *NodePortLoadBalancerModelGenerator) {
m, err := g.GenerateModel(a)
assert.Nil(t, err)
assert.NotNil(t, m)
assert.Equal(t, 1, len(m.Ingress))

anyIngressIP(t, m.Ingress, "ingress-ip-1", func(t *testing.T, i model.IngressIP) {
anyIngressIP(t, m.Ingress, "10.0.0.2", func(t *testing.T, i model.IngressIP) {
assert.Equal(t, 1, len(i.Ports))

anyPort(t, i.Ports, 80, corev1.ProtocolTCP, func(t *testing.T, p model.PortForward) {
Expand Down Expand Up @@ -220,15 +220,15 @@ func TestNodePortSinglePortMultiServiceAssignment(t *testing.T) {
model.FromService(svc2).ToKey(): "port-id-1",
}

f.l3portmanager.On("GetInternalAddress", "port-id-1").Return("ingress-ip-1", nil).Times(1)
f.l3portmanager.On("GetInternalAddress", "port-id-1").Return("10.0.0.2", nil).Times(1)

f.runWith(func(g *NodePortLoadBalancerModelGenerator) {
m, err := g.GenerateModel(a)
assert.Nil(t, err)
assert.NotNil(t, m)
assert.Equal(t, 1, len(m.Ingress))

anyIngressIP(t, m.Ingress, "ingress-ip-1", func(t *testing.T, i model.IngressIP) {
anyIngressIP(t, m.Ingress, "10.0.0.2", func(t *testing.T, i model.IngressIP) {
assert.Equal(t, 3, len(i.Ports))

anyPort(t, i.Ports, 53, corev1.ProtocolUDP, func(t *testing.T, p model.PortForward) {
Expand Down Expand Up @@ -273,16 +273,16 @@ func TestNodePortMultiPortSingleServiceAssignment(t *testing.T) {
model.FromService(svc2).ToKey(): "port-id-2",
}

f.l3portmanager.On("GetInternalAddress", "port-id-1").Return("ingress-ip-1", nil).Times(1)
f.l3portmanager.On("GetInternalAddress", "port-id-2").Return("ingress-ip-2", nil).Times(1)
f.l3portmanager.On("GetInternalAddress", "port-id-1").Return("10.0.0.2", nil).Times(1)
f.l3portmanager.On("GetInternalAddress", "port-id-2").Return("10.0.0.3", nil).Times(1)

f.runWith(func(g *NodePortLoadBalancerModelGenerator) {
m, err := g.GenerateModel(a)
assert.Nil(t, err)
assert.NotNil(t, m)
assert.Equal(t, 2, len(m.Ingress))

anyIngressIP(t, m.Ingress, "ingress-ip-1", func(t *testing.T, i model.IngressIP) {
anyIngressIP(t, m.Ingress, "10.0.0.2", func(t *testing.T, i model.IngressIP) {
assert.Equal(t, 2, len(i.Ports))

anyPort(t, i.Ports, 80, corev1.ProtocolTCP, func(t *testing.T, p model.PortForward) {
Expand All @@ -298,7 +298,7 @@ func TestNodePortMultiPortSingleServiceAssignment(t *testing.T) {
})
})

anyIngressIP(t, m.Ingress, "ingress-ip-2", func(t *testing.T, i model.IngressIP) {
anyIngressIP(t, m.Ingress, "10.0.0.3", func(t *testing.T, i model.IngressIP) {
assert.Equal(t, 1, len(i.Ports))

anyPort(t, i.Ports, 53, corev1.ProtocolUDP, func(t *testing.T, p model.PortForward) {
Expand Down Expand Up @@ -339,16 +339,16 @@ func TestNodePortMultiPortMultiServiceAssignment(t *testing.T) {
model.FromService(svc3).ToKey(): "port-id-2",
}

f.l3portmanager.On("GetInternalAddress", "port-id-1").Return("ingress-ip-1", nil).Times(1)
f.l3portmanager.On("GetInternalAddress", "port-id-2").Return("ingress-ip-2", nil).Times(1)
f.l3portmanager.On("GetInternalAddress", "port-id-1").Return("10.0.0.2", nil).Times(1)
f.l3portmanager.On("GetInternalAddress", "port-id-2").Return("10.0.0.3", nil).Times(1)

f.runWith(func(g *NodePortLoadBalancerModelGenerator) {
m, err := g.GenerateModel(a)
assert.Nil(t, err)
assert.NotNil(t, m)
assert.Equal(t, 2, len(m.Ingress))

anyIngressIP(t, m.Ingress, "ingress-ip-1", func(t *testing.T, i model.IngressIP) {
anyIngressIP(t, m.Ingress, "10.0.0.2", func(t *testing.T, i model.IngressIP) {
assert.Equal(t, 2, len(i.Ports))

anyPort(t, i.Ports, 80, corev1.ProtocolTCP, func(t *testing.T, p model.PortForward) {
Expand All @@ -364,7 +364,7 @@ func TestNodePortMultiPortMultiServiceAssignment(t *testing.T) {
})
})

anyIngressIP(t, m.Ingress, "ingress-ip-2", func(t *testing.T, i model.IngressIP) {
anyIngressIP(t, m.Ingress, "10.0.0.3", func(t *testing.T, i model.IngressIP) {
assert.Equal(t, 3, len(i.Ports))

anyPort(t, i.Ports, 53, corev1.ProtocolUDP, func(t *testing.T, p model.PortForward) {
Expand Down

0 comments on commit 692ebfe

Please sign in to comment.