diff --git a/pkg/bgp/id.go b/pkg/bgp/id.go index 3d3a0a3d7..3396fe714 100644 --- a/pkg/bgp/id.go +++ b/pkg/bgp/id.go @@ -6,8 +6,8 @@ import ( "fmt" "hash/fnv" "net" - "regexp" "strconv" + "strings" "github.com/cloudnativelabs/kube-router/v2/pkg/utils" gobgp "github.com/osrg/gobgp/v3/pkg/packet/bgp" @@ -49,11 +49,10 @@ func ValidateCommunity(arg string) error { 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 { + elem1, elem2, found := strings.Cut(arg, ":") + if found { + if _, err := strconv.ParseUint(elem1, 10, CommunityMaxPartSize); err == nil { + if _, err = strconv.ParseUint(elem2, 10, CommunityMaxPartSize); err == nil { return nil } } diff --git a/pkg/controllers/routing/network_routes_controller.go b/pkg/controllers/routing/network_routes_controller.go index db2478898..9bb69a670 100644 --- a/pkg/controllers/routing/network_routes_controller.go +++ b/pkg/controllers/routing/network_routes_controller.go @@ -69,6 +69,20 @@ const ( ipv4MaskMinBits = 32 ) +// RouteSyncer is an interface that defines the methods needed to sync routes to the kernel's routing table +type RouteSyncer interface { + AddInjectedRoute(dst *net.IPNet, route *netlink.Route) + DelInjectedRoute(dst *net.IPNet) + Run(stopCh <-chan struct{}, wg *sync.WaitGroup) + SyncLocalRouteTable() +} + +// PolicyBasedRouting is an interface that defines the methods needed to enable/disable policy based routing +type PolicyBasedRouter interface { + Enable() error + Disable() error +} + // NetworkRoutingController is struct to hold necessary information required by controller type NetworkRoutingController struct { krNode utils.NodeAware @@ -124,8 +138,8 @@ type NetworkRoutingController struct { podIPv6CIDRs []string CNIFirewallSetup *sync.Cond ipsetMutex *sync.Mutex - routeSyncer routes.RouteSyncer - pbr routes.PBRer + routeSyncer RouteSyncer + pbr PolicyBasedRouter tunneler tunnels.Tunneler nodeLister cache.Indexer @@ -165,13 +179,13 @@ func (nrc *NetworkRoutingController) Run(healthChan chan<- *healthcheck.Controll nrc.CNIFirewallSetup.Broadcast() - nrc.pbr = routes.NewPBR(nrc.krNode, nrc.podIPv4CIDRs, nrc.podIPv6CIDRs) + nrc.pbr = routes.NewPolicyBasedRules(nrc.krNode, nrc.podIPv4CIDRs, nrc.podIPv6CIDRs) // Handle ipip tunnel overlay if nrc.enableOverlays { klog.V(1).Info("Tunnel Overlay enabled in configuration.") klog.V(1).Info("Setting up overlay networking.") - err = nrc.pbr.EnablePolicyBasedRouting() + err = nrc.pbr.Enable() if err != nil { klog.Errorf("Failed to enable required policy based routing: %s", err.Error()) } @@ -187,7 +201,7 @@ func (nrc *NetworkRoutingController) Run(healthChan chan<- *healthcheck.Controll } else { klog.V(1).Info("Tunnel Overlay disabled in configuration.") klog.V(1).Info("Cleaning up old overlay networking if needed.") - err = nrc.pbr.DisablePolicyBasedRouting() + err = nrc.pbr.Disable() if err != nil { klog.Errorf("Failed to disable policy based routing: %s", err.Error()) } @@ -1365,8 +1379,8 @@ func NewNetworkRoutingController(clientset kubernetes.Interface, nrc.autoMTU = kubeRouterConfig.AutoMTU nrc.enableOverlays = kubeRouterConfig.EnableOverlay nrc.overlayType = kubeRouterConfig.OverlayType - overlayEncap, err := tunnels.ParseEncapType(kubeRouterConfig.OverlayEncap) - if err != nil { + overlayEncap, ok := tunnels.ParseEncapType(kubeRouterConfig.OverlayEncap) + if !ok { return nil, fmt.Errorf("unknown --overlay-encap option '%s' selected, unable to continue", overlayEncap) } overlayEncapPort, err := tunnels.ParseEncapPort(kubeRouterConfig.OverlayEncapPort) diff --git a/pkg/routes/pbr.go b/pkg/routes/pbr.go index 2abf9429f..50b913b64 100644 --- a/pkg/routes/pbr.go +++ b/pkg/routes/pbr.go @@ -15,20 +15,16 @@ const ( CustomTableName = "kube-router" ) -type PBR struct { +// PolicyBasedRules is a struct that holds all of the information needed for manipulating policy based routing rules +type PolicyBasedRules struct { nfa utils.NodeFamilyAware podIPv4CIDRs []string podIPv6CIDRs []string } -type PBRer interface { - EnablePolicyBasedRouting() error - DisablePolicyBasedRouting() error -} - -// NewPBR creates a new PBR object which will be used to manipulate policy based routing rules -func NewPBR(nfa utils.NodeFamilyAware, podIPv4CIDRs, podIPv6CIDRs []string) *PBR { - return &PBR{ +// NewPolicyBasedRules creates a new PBR object which will be used to manipulate policy based routing rules +func NewPolicyBasedRules(nfa utils.NodeFamilyAware, podIPv4CIDRs, podIPv6CIDRs []string) *PolicyBasedRules { + return &PolicyBasedRules{ nfa: nfa, podIPv4CIDRs: podIPv4CIDRs, podIPv6CIDRs: podIPv6CIDRs, @@ -60,9 +56,9 @@ func ipRuleAbstraction(ipProtocol, ipOp, cidr string) error { return nil } -// setup a custom routing table that will be used for policy based routing to ensure traffic originating -// on tunnel interface only leaves through tunnel interface irrespective rp_filter enabled/disabled -func (pbr *PBR) EnablePolicyBasedRouting() error { +// Enable setup a custom routing table that will be used for policy based routing to ensure traffic +// originating on tunnel interface only leaves through tunnel interface irrespective rp_filter enabled/disabled +func (pbr *PolicyBasedRules) Enable() error { err := utils.RouteTableAdd(CustomTableID, CustomTableName) if err != nil { return fmt.Errorf("failed to update rt_tables file: %s", err) @@ -86,7 +82,8 @@ func (pbr *PBR) EnablePolicyBasedRouting() error { return nil } -func (pbr *PBR) DisablePolicyBasedRouting() error { +// Disable removes the custom routing table that was used for policy based routing +func (pbr *PolicyBasedRules) Disable() error { err := utils.RouteTableAdd(CustomTableID, CustomTableName) if err != nil { return fmt.Errorf("failed to update rt_tables file: %s", err) diff --git a/pkg/routes/route_sync.go b/pkg/routes/route_sync.go index 2da0061a3..6a5ded040 100644 --- a/pkg/routes/route_sync.go +++ b/pkg/routes/route_sync.go @@ -9,14 +9,6 @@ import ( "k8s.io/klog/v2" ) -// RouteSyncer is an interface that defines the methods needed to sync routes to the kernel's routing table -type RouteSyncer interface { - AddInjectedRoute(dst *net.IPNet, route *netlink.Route) - DelInjectedRoute(dst *net.IPNet) - Run(stopCh <-chan struct{}, wg *sync.WaitGroup) - SyncLocalRouteTable() -} - // RouteSync is a struct that holds all of the information needed for syncing routes to the kernel's routing table type RouteSync struct { routeTableStateMap map[string]*netlink.Route diff --git a/pkg/tunnels/linux_tunnels.go b/pkg/tunnels/linux_tunnels.go index 241644ece..1da4f3d58 100644 --- a/pkg/tunnels/linux_tunnels.go +++ b/pkg/tunnels/linux_tunnels.go @@ -6,6 +6,7 @@ import ( "fmt" "net" "os/exec" + "slices" "strconv" "strings" @@ -39,17 +40,6 @@ var ( // EncapType represents the type of encapsulation used for an overlay tunnel in kube-router. type EncapType string -// IsValid checks if the encapsulation type is valid by comparing it against a list of valid types. -// It returns true if the encapsulation type is valid, otherwise it returns false. -func (e EncapType) IsValid() bool { - for _, validType := range validEncapTypes { - if e == validType { - return true - } - } - return false -} - // ParseEncapType parses the given string and returns an Encap type if valid. // It returns an error if the encapsulation type is invalid. // @@ -58,13 +48,13 @@ func (e EncapType) IsValid() bool { // // Returns: // - Encap: The parsed encapsulation type. -// - error: An error if the encapsulation type is invalid. -func ParseEncapType(encapType string) (EncapType, error) { +// - bool: A boolean indicating whether the encapsulation type is valid. +func ParseEncapType(encapType string) (EncapType, bool) { encap := EncapType(encapType) - if !encap.IsValid() { - return "", fmt.Errorf("invalid encapsulation type: %s", encapType) + if !slices.Contains(validEncapTypes, encap) { + return "", false } - return encap, nil + return encap, true } type EncapPort uint16 @@ -92,26 +82,25 @@ type Tunneler interface { } type OverlayTunnel struct { - krNode utils.NodeIPAndFamilyAware - overlayEncapPort EncapPort - overlayEncap EncapType + krNode utils.NodeIPAware + encapPort EncapPort + encapType EncapType } -func NewOverlayTunnel(krNode utils.NodeIPAndFamilyAware, overlayEncap EncapType, - overlayEncapPort EncapPort) *OverlayTunnel { +func NewOverlayTunnel(krNode utils.NodeIPAware, encapType EncapType, encapPort EncapPort) *OverlayTunnel { return &OverlayTunnel{ - krNode: krNode, - overlayEncapPort: overlayEncapPort, - overlayEncap: overlayEncap, + krNode: krNode, + encapPort: encapPort, + encapType: encapType, } } func (o *OverlayTunnel) EncapType() EncapType { - return o.overlayEncap + return o.encapType } func (o *OverlayTunnel) EncapPort() EncapPort { - return o.overlayEncapPort + return o.encapPort } // setupOverlayTunnel attempts to create a tunnel link and corresponding routes for IPIP based overlay networks @@ -124,7 +113,7 @@ func (o *OverlayTunnel) SetupOverlayTunnel(tunnelName string, nextHop net.IP, var ipipMode, fouLinkType string isIPv6 := false ipBase := make([]string, 0) - strFormattedEncapPort := strconv.FormatInt(int64(o.overlayEncapPort), 10) + strFormattedEncapPort := strconv.FormatInt(int64(o.encapPort), 10) if nextHop.To4() != nil { bestIPForFamily = o.krNode.FindBestIPv4NodeAddress() @@ -151,7 +140,7 @@ func (o *OverlayTunnel) SetupOverlayTunnel(tunnelName string, nextHop net.IP, klog.V(1).Infof("Tunnel interface: %s with encap type %s for the node %s already exists.", tunnelName, link.Attrs().EncapType, nextHop.String()) - switch o.overlayEncap { + switch o.encapType { case EncapTypeIPIP: if linkFOUEnabled(tunnelName) { klog.Infof("Was configured to use ipip tunnels, but found existing fou tunnels in place, cleaning up") @@ -162,7 +151,7 @@ func (o *OverlayTunnel) SetupOverlayTunnel(tunnelName string, nextHop net.IP, CleanupTunnel(nextHopSubnet, tunnelName) // If we are transitioning from FoU to IPIP we also need to clean up the old FoU port if it exists - if fouPortAndProtoExist(o.overlayEncapPort, isIPv6) { + if fouPortAndProtoExist(o.encapPort, isIPv6) { fouArgs := ipBase fouArgs = append(fouArgs, "fou", "del", "port", strFormattedEncapPort) out, err := exec.Command("ip", fouArgs...).CombinedOutput() @@ -188,9 +177,9 @@ func (o *OverlayTunnel) SetupOverlayTunnel(tunnelName string, nextHop net.IP, // nothing to do here if err != nil || recreate { klog.Infof("Creating tunnel %s of type %s with encap %s for destination %s", - tunnelName, fouLinkType, o.overlayEncap, nextHop.String()) + tunnelName, fouLinkType, o.encapType, nextHop.String()) cmdArgs := ipBase - switch o.overlayEncap { + switch o.encapType { case EncapTypeIPIP: // Plain IPIP tunnel without any encapsulation cmdArgs = append(cmdArgs, "tunnel", "add", tunnelName, "mode", ipipMode, "local", bestIPForFamily.String(), @@ -198,7 +187,7 @@ func (o *OverlayTunnel) SetupOverlayTunnel(tunnelName string, nextHop net.IP, case EncapTypeFOU: // Ensure that the FOU tunnel port is set correctly - if !fouPortAndProtoExist(o.overlayEncapPort, isIPv6) { + if !fouPortAndProtoExist(o.encapPort, isIPv6) { fouArgs := ipBase fouArgs = append(fouArgs, "fou", "add", "port", strFormattedEncapPort, "gue") out, err := exec.Command("ip", fouArgs...).CombinedOutput() @@ -216,7 +205,7 @@ func (o *OverlayTunnel) SetupOverlayTunnel(tunnelName string, nextHop net.IP, default: return nil, fmt.Errorf("unknown tunnel encapsulation was passed: %s, unable to continue with overlay "+ - "setup", o.overlayEncap) + "setup", o.encapType) } klog.V(2).Infof("Executing the following command to create tunnel: ip %s", cmdArgs) @@ -276,6 +265,10 @@ func CleanupTunnel(destinationSubnet *net.IPNet, tunnelName string) { // Since linux restricts interface names to 15 characters, we take the sha-256 of the node IP after removing // non-entropic characters like '.' and ':', and then use the first 12 bytes of it. This allows us to cater to both // long IPv4 addresses and much longer IPv6 addresses. +// +// TODO: In the future, we should consider using the hexadecimal byte representation of IPv4 addresses and using a the +// SHA256 of the hash. Additionally, we should not remove non-entropic characters as it can cause hash collisions as +// "21.3.0.4" would has the same as "2.13.0.4" without "."'s. func GenerateTunnelName(nodeIP string) string { // remove dots from an IPv4 address strippedIP := strings.ReplaceAll(nodeIP, ".", "")