fact(bgp): separate bgp functionality into new package

Author: aauren

pkg/bgp/id.go

@@ -0,0 +1,67 @@
+package bgp
+
+import (
+	"encoding/binary"
+	"errors"
+	"fmt"
+	"hash/fnv"
+	"net"
+	"regexp"
+	"strconv"
+
+	"github.com/cloudnativelabs/kube-router/v2/pkg/utils"
+	gobgp "github.com/osrg/gobgp/v3/pkg/packet/bgp"
+)
+
+const (
+	CommunityMaxSize     = 32
+	CommunityMaxPartSize = 16
+)
+
+// GenerateRouterID will generate a router ID based upon the user's configuration (or lack there of) and the node's
+// primary IP address if the user has not specified. If the user has configured the router ID as "generate" then we
+// will generate a router ID based upon fnv hashing the node's primary IP address.
+func GenerateRouterID(nodeIPAware utils.NodeIPAware, configRouterID string) (string, error) {
+	switch {
+	case configRouterID == "generate":
+		h := fnv.New32a()
+		h.Write(nodeIPAware.GetPrimaryNodeIP())
+		hs := h.Sum32()
+		gip := make(net.IP, 4)
+		binary.BigEndian.PutUint32(gip, hs)
+		return gip.String(), nil
+	case configRouterID != "":
+		return configRouterID, nil
+	}
+
+	if nodeIPAware.GetPrimaryNodeIP().To4() == nil {
+		return "", errors.New("router-id must be specified when primary node IP is an IPv6 address")
+	}
+	return configRouterID, nil
+}
+
+// ValidateCommunity takes in a string and attempts to parse a BGP community out of it in a way that is similar to
+// gobgp (internal/pkg/table/policy.go:ParseCommunity()). If it is not able to parse the community information it
+// returns an error.
+func ValidateCommunity(arg string) error {
+	_, err := strconv.ParseUint(arg, 10, CommunityMaxSize)
+	if err == nil {
+		return nil
+	}
+
+	_regexpCommunity := regexp.MustCompile(`(\d+):(\d+)`)
+	elems := _regexpCommunity.FindStringSubmatch(arg)
+	if len(elems) == 3 {
+		if _, err := strconv.ParseUint(elems[1], 10, CommunityMaxPartSize); err == nil {
+			if _, err = strconv.ParseUint(elems[2], 10, CommunityMaxPartSize); err == nil {
+				return nil
+			}
+		}
+	}
+	for _, v := range gobgp.WellKnownCommunityNameMap {
+		if arg == v {
+			return nil
+		}
+	}
+	return fmt.Errorf("failed to parse %s as community", arg)
+}

pkg/bgp/id_test.go

@@ -0,0 +1,39 @@
+package bgp
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_ValidateCommunity(t *testing.T) {
+	t.Run("BGP community specified as a 32-bit integer should pass validation", func(t *testing.T) {
+		assert.Nil(t, ValidateCommunity("4294967041"))
+		assert.Nil(t, ValidateCommunity("4294967295"))
+	})
+	t.Run("BGP community specified as 2 16-bit integers should pass validation", func(t *testing.T) {
+		assert.Nil(t, ValidateCommunity("65535:65281"))
+		assert.Nil(t, ValidateCommunity("65535:65535"))
+	})
+	t.Run("Well known BGP communities passed as a string should pass validation", func(t *testing.T) {
+		assert.Nil(t, ValidateCommunity("no-export"))
+		assert.Nil(t, ValidateCommunity("internet"))
+		assert.Nil(t, ValidateCommunity("planned-shut"))
+		assert.Nil(t, ValidateCommunity("accept-own"))
+		assert.Nil(t, ValidateCommunity("blackhole"))
+		assert.Nil(t, ValidateCommunity("no-advertise"))
+		assert.Nil(t, ValidateCommunity("no-peer"))
+	})
+	t.Run("BGP community that is greater than 32-bit integer should fail validation", func(t *testing.T) {
+		assert.Error(t, ValidateCommunity("4294967296"))
+	})
+	t.Run("BGP community that is greater than 2 16-bit integers should fail validation", func(t *testing.T) {
+		assert.Error(t, ValidateCommunity("65536:65535"))
+		assert.Error(t, ValidateCommunity("65535:65536"))
+		assert.Error(t, ValidateCommunity("65536:65536"))
+	})
+	t.Run("BGP community that is not a number should fail validation", func(t *testing.T) {
+		assert.Error(t, ValidateCommunity("0xFFFFFFFF"))
+		assert.Error(t, ValidateCommunity("community"))
+	})
+}

pkg/bgp/parse.go

@@ -0,0 +1,63 @@
+package bgp
+
+import (
+	"fmt"
+	"net"
+
+	gobgpapi "github.com/osrg/gobgp/v3/api"
+	"github.com/vishvananda/netlink"
+)
+
+// ParseNextHop takes in a GoBGP Path and parses out the destination's next hop from its attributes. If it
+// can't parse a next hop IP from the GoBGP Path, it returns an error.
+func ParseNextHop(path *gobgpapi.Path) (net.IP, error) {
+	for _, pAttr := range path.GetPattrs() {
+		unmarshalNew, err := pAttr.UnmarshalNew()
+		if err != nil {
+			return nil, fmt.Errorf("failed to unmarshal path attribute: %s", err)
+		}
+		switch t := unmarshalNew.(type) {
+		case *gobgpapi.NextHopAttribute:
+			// This is the primary way that we receive NextHops and happens when both the client and the server exchange
+			// next hops on the same IP family that they negotiated BGP on
+			nextHopIP := net.ParseIP(t.NextHop)
+			if nextHopIP != nil && (nextHopIP.To4() != nil || nextHopIP.To16() != nil) {
+				return nextHopIP, nil
+			}
+			return nil, fmt.Errorf("invalid nextHop address: %s", t.NextHop)
+		case *gobgpapi.MpReachNLRIAttribute:
+			// in the case where the server and the client are exchanging next-hops that don't relate to their primary
+			// IP family, we get MpReachNLRIAttribute instead of NextHopAttributes
+			// TODO: here we only take the first next hop, at some point in the future it would probably be best to
+			// consider multiple next hops
+			nextHopIP := net.ParseIP(t.NextHops[0])
+			if nextHopIP != nil && (nextHopIP.To4() != nil || nextHopIP.To16() != nil) {
+				return nextHopIP, nil
+			}
+			return nil, fmt.Errorf("invalid nextHop address: %s", t.NextHops[0])
+		}
+	}
+	return nil, fmt.Errorf("could not parse next hop received from GoBGP for path: %s", path)
+}
+
+// ParsePath takes in a GoBGP Path and parses out the destination subnet and the next hop from its attributes.
+// If successful, it will return the destination of the BGP path as a subnet form and the next hop. If it
+// can't parse the destination or the next hop IP, it returns an error.
+func ParsePath(path *gobgpapi.Path) (*net.IPNet, net.IP, error) {
+	nextHop, err := ParseNextHop(path)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	nlri := path.GetNlri()
+	var prefix gobgpapi.IPAddressPrefix
+	err = nlri.UnmarshalTo(&prefix)
+	if err != nil {
+		return nil, nil, fmt.Errorf("invalid nlri in advertised path")
+	}
+	dstSubnet, err := netlink.ParseIPNet(prefix.Prefix + "/" + fmt.Sprint(prefix.PrefixLen))
+	if err != nil {
+		return nil, nil, fmt.Errorf("couldn't parse IP subnet from nlri advertised path")
+	}
+	return dstSubnet, nextHop, nil
+}

pkg/controllers/routing/network_routes_controller.go

@@ -14,6 +14,7 @@ import (
 
 	"google.golang.org/protobuf/types/known/anypb"
 
+	"github.com/cloudnativelabs/kube-router/v2/pkg/bgp"
 	"github.com/cloudnativelabs/kube-router/v2/pkg/healthcheck"
 	"github.com/cloudnativelabs/kube-router/v2/pkg/metrics"
 	"github.com/cloudnativelabs/kube-router/v2/pkg/options"
@@ -62,12 +63,10 @@ const (
 	ClusterIPST    = "ClusterIP"
 	NodePortST     = "NodePort"
 
-	prependPathMaxBits      = 8
-	asnMaxBitSize           = 32
-	bgpCommunityMaxSize     = 32
-	bgpCommunityMaxPartSize = 16
-	routeReflectorMaxID     = 32
-	ipv4MaskMinBits         = 32
+	prependPathMaxBits  = 8
+	asnMaxBitSize       = 32
+	routeReflectorMaxID = 32
+	ipv4MaskMinBits     = 32
 )
 
 // NetworkRoutingController is struct to hold necessary information required by controller
@@ -567,7 +566,7 @@ func (nrc *NetworkRoutingController) injectRoute(path *gobgpapi.Path) error {
 	var route *netlink.Route
 	var link netlink.Link
 
-	dst, nextHop, err := parseBGPPath(path)
+	dst, nextHop, err := bgp.ParsePath(path)
 	if err != nil {
 		return err
 	}
@@ -965,7 +964,7 @@ func (nrc *NetworkRoutingController) startBgpServer(grpcServer bool) error {
 	} else {
 		nodeCommunities = stringToSlice(nodeBGPCommunitiesAnnotation, ",")
 		for _, nodeCommunity := range nodeCommunities {
-			if err = validateCommunity(nodeCommunity); err != nil {
+			if err = bgp.ValidateCommunity(nodeCommunity); err != nil {
 				klog.Warningf("cannot add BGP community '%s' from node annotation as it does not appear "+
 					"to be a valid community identifier", nodeCommunity)
 				continue
@@ -1302,7 +1301,7 @@ func NewNetworkRoutingController(clientset kubernetes.Interface,
 		}
 	}
 
-	nrc.routerID, err = generateRouterID(nrc.krNode, kubeRouterConfig.RouterID)
+	nrc.routerID, err = bgp.GenerateRouterID(nrc.krNode, kubeRouterConfig.RouterID)
 	if err != nil {
 		return nil, err
 	}

pkg/controllers/routing/utils.go

@@ -2,22 +2,15 @@ package routing
 
 import (
 	"encoding/base64"
-	"encoding/binary"
-	"errors"
 	"fmt"
-	"hash/fnv"
 	"net"
-	"regexp"
 	"strconv"
 	"strings"
 
-	"github.com/cloudnativelabs/kube-router/v2/pkg/utils"
 	gobgpapi "github.com/osrg/gobgp/v3/api"
-	"github.com/osrg/gobgp/v3/pkg/packet/bgp"
+
 	v1core "k8s.io/api/core/v1"
 	"k8s.io/klog/v2"
-
-	"github.com/vishvananda/netlink"
 )
 
 // Used for processing Annotations that may contain multiple items
@@ -117,108 +110,6 @@ func statementsEqualByName(a, b []*gobgpapi.Statement) bool {
 	return true
 }
 
-// generateRouterID will generate a router ID based upon the user's configuration (or lack there of) and the node's
-// primary IP address if the user has not specified. If the user has configured the router ID as "generate" then we
-// will generate a router ID based upon fnv hashing the node's primary IP address.
-func generateRouterID(nodeIPAware utils.NodeIPAware, configRouterID string) (string, error) {
-	switch {
-	case configRouterID == "generate":
-		h := fnv.New32a()
-		h.Write(nodeIPAware.GetPrimaryNodeIP())
-		hs := h.Sum32()
-		gip := make(net.IP, 4)
-		binary.BigEndian.PutUint32(gip, hs)
-		return gip.String(), nil
-	case configRouterID != "":
-		return configRouterID, nil
-	}
-
-	if nodeIPAware.GetPrimaryNodeIP().To4() == nil {
-		return "", errors.New("router-id must be specified when primary node IP is an IPv6 address")
-	}
-	return configRouterID, nil
-}
-
-// validateCommunity takes in a string and attempts to parse a BGP community out of it in a way that is similar to
-// gobgp (internal/pkg/table/policy.go:ParseCommunity()). If it is not able to parse the community information it
-// returns an error.
-func validateCommunity(arg string) error {
-	_, err := strconv.ParseUint(arg, 10, bgpCommunityMaxSize)
-	if err == nil {
-		return nil
-	}
-
-	_regexpCommunity := regexp.MustCompile(`(\d+):(\d+)`)
-	elems := _regexpCommunity.FindStringSubmatch(arg)
-	if len(elems) == 3 {
-		if _, err := strconv.ParseUint(elems[1], 10, bgpCommunityMaxPartSize); err == nil {
-			if _, err = strconv.ParseUint(elems[2], 10, bgpCommunityMaxPartSize); err == nil {
-				return nil
-			}
-		}
-	}
-	for _, v := range bgp.WellKnownCommunityNameMap {
-		if arg == v {
-			return nil
-		}
-	}
-	return fmt.Errorf("failed to parse %s as community", arg)
-}
-
-// parseBGPNextHop takes in a GoBGP Path and parses out the destination's next hop from its attributes. If it
-// can't parse a next hop IP from the GoBGP Path, it returns an error.
-func parseBGPNextHop(path *gobgpapi.Path) (net.IP, error) {
-	for _, pAttr := range path.GetPattrs() {
-		unmarshalNew, err := pAttr.UnmarshalNew()
-		if err != nil {
-			return nil, fmt.Errorf("failed to unmarshal path attribute: %s", err)
-		}
-		switch t := unmarshalNew.(type) {
-		case *gobgpapi.NextHopAttribute:
-			// This is the primary way that we receive NextHops and happens when both the client and the server exchange
-			// next hops on the same IP family that they negotiated BGP on
-			nextHopIP := net.ParseIP(t.NextHop)
-			if nextHopIP != nil && (nextHopIP.To4() != nil || nextHopIP.To16() != nil) {
-				return nextHopIP, nil
-			}
-			return nil, fmt.Errorf("invalid nextHop address: %s", t.NextHop)
-		case *gobgpapi.MpReachNLRIAttribute:
-			// in the case where the server and the client are exchanging next-hops that don't relate to their primary
-			// IP family, we get MpReachNLRIAttribute instead of NextHopAttributes
-			// TODO: here we only take the first next hop, at some point in the future it would probably be best to
-			// consider multiple next hops
-			nextHopIP := net.ParseIP(t.NextHops[0])
-			if nextHopIP != nil && (nextHopIP.To4() != nil || nextHopIP.To16() != nil) {
-				return nextHopIP, nil
-			}
-			return nil, fmt.Errorf("invalid nextHop address: %s", t.NextHops[0])
-		}
-	}
-	return nil, fmt.Errorf("could not parse next hop received from GoBGP for path: %s", path)
-}
-
-// parseBGPPath takes in a GoBGP Path and parses out the destination subnet and the next hop from its attributes.
-// If successful, it will return the destination of the BGP path as a subnet form and the next hop. If it
-// can't parse the destination or the next hop IP, it returns an error.
-func parseBGPPath(path *gobgpapi.Path) (*net.IPNet, net.IP, error) {
-	nextHop, err := parseBGPNextHop(path)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	nlri := path.GetNlri()
-	var prefix gobgpapi.IPAddressPrefix
-	err = nlri.UnmarshalTo(&prefix)
-	if err != nil {
-		return nil, nil, fmt.Errorf("invalid nlri in advertised path")
-	}
-	dstSubnet, err := netlink.ParseIPNet(prefix.Prefix + "/" + fmt.Sprint(prefix.PrefixLen))
-	if err != nil {
-		return nil, nil, fmt.Errorf("couldn't parse IP subnet from nlri advertised path")
-	}
-	return dstSubnet, nextHop, nil
-}
-
 // getPodCIDRsFromAllNodeSources gets the pod CIDRs for all available sources on a given node in a specific order. The
 // order of preference is:
 //  1. From the kube-router.io/pod-cidr annotation (preserves backwards compatibility)

pkg/controllers/routing/utils_test.go

@@ -50,35 +50,3 @@ func Test_stringSliceToIPNets(t *testing.T) {
 		assert.Nil(t, ips)
 	})
 }
-
-func Test_validateCommunity(t *testing.T) {
-	t.Run("BGP community specified as a 32-bit integer should pass validation", func(t *testing.T) {
-		assert.Nil(t, validateCommunity("4294967041"))
-		assert.Nil(t, validateCommunity("4294967295"))
-	})
-	t.Run("BGP community specified as 2 16-bit integers should pass validation", func(t *testing.T) {
-		assert.Nil(t, validateCommunity("65535:65281"))
-		assert.Nil(t, validateCommunity("65535:65535"))
-	})
-	t.Run("Well known BGP communities passed as a string should pass validation", func(t *testing.T) {
-		assert.Nil(t, validateCommunity("no-export"))
-		assert.Nil(t, validateCommunity("internet"))
-		assert.Nil(t, validateCommunity("planned-shut"))
-		assert.Nil(t, validateCommunity("accept-own"))
-		assert.Nil(t, validateCommunity("blackhole"))
-		assert.Nil(t, validateCommunity("no-advertise"))
-		assert.Nil(t, validateCommunity("no-peer"))
-	})
-	t.Run("BGP community that is greater than 32-bit integer should fail validation", func(t *testing.T) {
-		assert.Error(t, validateCommunity("4294967296"))
-	})
-	t.Run("BGP community that is greater than 2 16-bit integers should fail validation", func(t *testing.T) {
-		assert.Error(t, validateCommunity("65536:65535"))
-		assert.Error(t, validateCommunity("65535:65536"))
-		assert.Error(t, validateCommunity("65536:65536"))
-	})
-	t.Run("BGP community that is not a number should fail validation", func(t *testing.T) {
-		assert.Error(t, validateCommunity("0xFFFFFFFF"))
-		assert.Error(t, validateCommunity("community"))
-	})
-}