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 #25
  • Loading branch information
@sstrk
sstrk committed Apr 23, 2021
1 parent b682256 commit f927006
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 19 deletions.
80 changes: 73 additions & 7 deletions internal/controller/model_nodeport.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
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"
Expand All @@ -23,6 +27,14 @@ import (
"github.com/cloudandheat/ch-k8s-lbaas/internal/openstack"
)

var (
ErrUnknownAddressFamily = errors.New("cannot determine the correct IP address family")
)

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 +52,61 @@ func NewNodePortLoadBalancerModelGenerator(
}
}

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

func isIPv4Address(ipString string) (isIPv4 bool, err error) {
return strings.Count(ipString, ":") < 2, isValidIp(ipString)
}

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

func (g *NodePortLoadBalancerModelGenerator) getDestinationAddresses() (addresses_v4 []string, addresses_v6 []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)

nodeAddressIsIPv4, err := isIPv4Address(addr.Address)
if err != nil {
return nil, nil, err
}
if nodeAddressIsIPv4 {
addresses_v4 = append(addresses_v4, addr.Address)
}

nodeAddressIsIPv6, err := isIPv6Address(addr.Address)
if err != nil {
return nil, nil, err
}
if nodeAddressIsIPv6 {
addresses_v6 = append(addresses_v6, addr.Address)
}

if !nodeAddressIsIPv4 && !nodeAddressIsIPv6 {
return nil, nil, ErrUnknownAddressFamily
}
}
}

return result, nil
return addresses_v4, addresses_v6, nil
}

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

ingressIpIsIpv4, err := isIPv4Address(ingress.Address)
if err != nil {
return nil, err
}
ingressIpIsIpv6, err := isIPv6Address(ingress.Address)
if err != nil {
return nil, err
}
if !ingressIpIsIpv4 && !ingressIpIsIpv6 {
return nil, ErrUnknownAddressFamily
}

var destAddresses []string

if ingressIpIsIpv4 {
destAddresses = append(destAddresses, addresses_v4...)
} else if ingressIpIsIpv6 {
destAddresses = append(destAddresses, addresses_v6...)
}

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 f927006

Please sign in to comment.