diff --git a/docs/bgp.md b/docs/bgp.md index a68b444c1f..96f1a88ab4 100644 --- a/docs/bgp.md +++ b/docs/bgp.md @@ -10,7 +10,7 @@ pod IPs, service IPs, etc.). This is the default mode. All nodes in the clusters form iBGP peering relationships with rest of the nodes forming a full node-to-node mesh. Each node advertise the pod CIDR allocated to the nodes with its peers (the rest of the nodes in -the cluster). There is no configuration required in this mode. All the nodes in the cluster are associated with the +the cluster). There is no configuration required in this mode. All the nodes in the cluster are associated with the private ASN 64512 implicitly (which can be configured with `--cluster-asn` flag) and users are transparent to use of iBGP. This mode is suitable in public cloud environments or small cluster deployments. @@ -30,7 +30,7 @@ kubectl annotate node "kube-router.io/node.asn=64512" Only nodes within same ASN form full mesh. Two nodes with different ASNs never get peered. -### Route-Reflector setup Without Full Mesh +### Route-Reflector setup Without Full Mesh This model supports the common scheme of using a Route Reflector Server node to concentrate peering from client peers. This has the big advantage of not needing full mesh, and will scale better. In this mode kube-router expects each node @@ -75,11 +75,45 @@ For example: ### Node Specific External BGP Peers -Alternatively, each node can be configured with one or more node specific BGP peers. Information regarding node specific -BGP peer is read from node API object annotations: +Each node can be configured with one or more node specific BGP peers using the `kube-router.io/peers` node annotation. +Previously, these settings were configured using individual `kube-router.io/peer.*` annotations. +While these individual annotations are still supported, they're now deprecated and +will be removed in a future release. + +#### Using Consolidated Annotation + +The `kube-router.io/peers` annotation accepts peer configurations in YAML format with the following fields: + +- `remoteip` (required): The IP address of the peer +- `remoteasn` (required): The ASN of the peer +- `localip` (optional): Local IP address to use for this peer connection +- `password` (optional): Base64 encoded password for BGP authentication +- `port` (optional): BGP port (defaults to 179 if not specified) + +```shell +kubectl annotate node \ +kube-router.io/peers="$(cat <<'EOF' +- remoteip: 192.168.1.99 + remoteasn: 65000 + password: U2VjdXJlUGFzc3dvcmQK +- remoteip: 192.168.1.100 + remoteasn: 65000 + password: U2VjdXJlUGFzc3dvcmQK +EOF +)" +``` + +#### Using Individual Annotations (Deprecated) + +> **NOTE:** The individual peer annotations listed below are deprecated in favor of the consolidated `kube-router.io/peers` +> annotation. They are maintained for backward compatibility but will be removed in a future release. + +Node-specific BGP peer configs can also be set via individual node API object annotations: - `kube-router.io/peer.ips` - `kube-router.io/peer.asns` +- `kube-router.io/peer.passwords` +- `kube-router.io/peer.localips` For example, users can annotate node object with below commands: @@ -106,26 +140,23 @@ kubectl annotate node "kube-router.io/path-prepend.repeat-n=5" ### BGP Peer Local IP configuration -In some setups it might be desirable to set a local IP address used for connecting external BGP peers. This can be -accomplished on nodes with annotations: +In some setups it might be desirable to set a local IP address used for connecting external BGP peers. -- `kube-router.io/peer.localips` - -If set, this must be a list with a local IP address for each peer, or left empty to use nodeIP. +When using the `kube-router.io/peers` annotation, specify the `localip` field for each peer as shown in the +[Node Specific External BGP Peers](#node-specific-external-bgp-peers) section above. -Example: +When using individual annotations, you can specify the local IP address using `kube-router.io/peer.localips`: ```shell kubectl annotate node "kube-router.io/peer.localips=10.1.1.1,10.1.1.2" ``` -This will instruct kube-router to use IP `10.1.1.1` for first BGP peer as a local address, and use `10.1.1.2`for the -second. +If set, this must be a list with a local IP address for each peer, or left empty to use nodeIP. ### BGP Peer Password Authentication -The examples above have assumed there is no password authentication with BGP peer routers. If you need to use a password -for peering, you can use the `--peer-router-passwords` command-line option, the `kube-router.io/peer.passwords` node +If you need to use a password for peering with BGP peer routers, you can configure it using the `kube-router.io/peers` +annotation, the `--peer-router-passwords` command-line option, the deprecated `kube-router.io/peer.passwords` node annotation, or the `--peer-router-passwords-file` command-line option. #### Base64 Encoding Passwords @@ -142,7 +173,15 @@ U2VjdXJlUGFzc3dvcmQ= #### Password Configuration Examples -In this CLI flag example the first router (192.168.1.99) uses a password, while the second (192.168.1.100) does not. +**Using the consolidated annotation (recommended):** + +When using the `kube-router.io/peers` annotation, specify the `password` field with a base64 encoded password for each +peer that requires authentication. See the +[Node Specific External BGP Peers](#node-specific-external-bgp-peers) section for an example. + +**Using CLI flags:** + +In this example the first router (192.168.1.99) uses a password, while the second (192.168.1.100) does not: ```sh --peer-router-ips="192.168.1.99,192.168.1.100" @@ -152,7 +191,9 @@ In this CLI flag example the first router (192.168.1.99) uses a password, while Note the comma indicating the end of the first password. -Here's the same example but configured as node annotations: +**Using individual annotations (deprecated):** + +Here's the same example but configured with individual node annotations: ```shell kubectl annotate node "kube-router.io/peer.ips=192.168.1.99,192.168.1.100" @@ -160,6 +201,8 @@ kubectl annotate node "kube-router.io/peer.asns=65000,65000" kubectl annotate node "kube-router.io/peer.passwords=U2VjdXJlUGFzc3dvcmQK," ``` +**Using a password file:** + Finally, to include peer passwords as a file you would run kube-router with the following option: ```shell @@ -168,8 +211,8 @@ Finally, to include peer passwords as a file you would run kube-router with the --peer-router-passwords-file="/etc/kube-router/bgp-passwords.conf" ``` -The password file, closely follows the syntax of the command-line and node annotation options. -Here, the first peer IP (192.168.1.99) would be configured with a password, while the second would not. +The password file closely follows the syntax of the command-line and node annotation options. +Here, the first peer IP (192.168.1.99) would be configured with a password, while the second would not: ```sh U2VjdXJlUGFzc3dvcmQK, diff --git a/go.mod b/go.mod index a59bb82ea6..42b102e629 100644 --- a/go.mod +++ b/go.mod @@ -2,14 +2,16 @@ module github.com/cloudnativelabs/kube-router/v2 require ( github.com/aws/aws-sdk-go-v2 v1.40.0 - github.com/aws/aws-sdk-go-v2/config v1.31.17 + github.com/aws/aws-sdk-go-v2/config v1.32.2 github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.14 - github.com/aws/aws-sdk-go-v2/service/ec2 v1.274.0 + github.com/aws/aws-sdk-go-v2/service/ec2 v1.275.0 github.com/aws/smithy-go v1.23.2 github.com/ccoveille/go-safecast/v2 v2.0.0 github.com/coreos/go-iptables v0.8.0 github.com/docker/docker v28.5.2+incompatible - github.com/hashicorp/go-version v1.7.0 + github.com/goccy/go-yaml v1.19.0 + github.com/google/go-cmp v0.7.0 + github.com/hashicorp/go-version v1.8.0 github.com/moby/ipvs v1.1.0 github.com/onsi/ginkgo v1.16.5 github.com/onsi/gomega v1.38.2 @@ -34,15 +36,16 @@ require ( require ( github.com/Microsoft/go-winio v0.6.2 // indirect - github.com/aws/aws-sdk-go-v2/credentials v1.18.21 // indirect + github.com/aws/aws-sdk-go-v2/credentials v1.19.2 // indirect github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.14 // indirect github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.14 // indirect github.com/aws/aws-sdk-go-v2/internal/ini v1.8.4 // indirect github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.3 // indirect github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.14 // indirect - github.com/aws/aws-sdk-go-v2/service/sso v1.30.1 // indirect - github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.5 // indirect - github.com/aws/aws-sdk-go-v2/service/sts v1.39.1 // indirect + github.com/aws/aws-sdk-go-v2/service/signin v1.0.2 // indirect + github.com/aws/aws-sdk-go-v2/service/sso v1.30.5 // indirect + github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.10 // indirect + github.com/aws/aws-sdk-go-v2/service/sts v1.41.2 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/containerd/errdefs v1.0.0 // indirect @@ -78,7 +81,6 @@ require ( github.com/go-viper/mapstructure/v2 v2.4.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/google/gnostic-models v0.7.0 // indirect - github.com/google/go-cmp v0.7.0 // indirect github.com/google/uuid v1.6.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect diff --git a/go.sum b/go.sum index c9f59ff023..c4fe4df3eb 100644 --- a/go.sum +++ b/go.sum @@ -6,10 +6,10 @@ github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERo github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/aws/aws-sdk-go-v2 v1.40.0 h1:/WMUA0kjhZExjOQN2z3oLALDREea1A7TobfuiBrKlwc= github.com/aws/aws-sdk-go-v2 v1.40.0/go.mod h1:c9pm7VwuW0UPxAEYGyTmyurVcNrbF6Rt/wixFqDhcjE= -github.com/aws/aws-sdk-go-v2/config v1.31.17 h1:QFl8lL6RgakNK86vusim14P2k8BFSxjvUkcWLDjgz9Y= -github.com/aws/aws-sdk-go-v2/config v1.31.17/go.mod h1:V8P7ILjp/Uef/aX8TjGk6OHZN6IKPM5YW6S78QnRD5c= -github.com/aws/aws-sdk-go-v2/credentials v1.18.21 h1:56HGpsgnmD+2/KpG0ikvvR8+3v3COCwaF4r+oWwOeNA= -github.com/aws/aws-sdk-go-v2/credentials v1.18.21/go.mod h1:3YELwedmQbw7cXNaII2Wywd+YY58AmLPwX4LzARgmmA= +github.com/aws/aws-sdk-go-v2/config v1.32.2 h1:4liUsdEpUUPZs5WVapsJLx5NPmQhQdez7nYFcovrytk= +github.com/aws/aws-sdk-go-v2/config v1.32.2/go.mod h1:l0hs06IFz1eCT+jTacU/qZtC33nvcnLADAPL/XyrkZI= +github.com/aws/aws-sdk-go-v2/credentials v1.19.2 h1:qZry8VUyTK4VIo5aEdUcBjPZHL2v4FyQ3QEOaWcFLu4= +github.com/aws/aws-sdk-go-v2/credentials v1.19.2/go.mod h1:YUqm5a1/kBnoK+/NY5WEiMocZihKSo15/tJdmdXnM5g= github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.14 h1:WZVR5DbDgxzA0BJeudId89Kmgy6DIU4ORpxwsVHz0qA= github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.14/go.mod h1:Dadl9QO0kHgbrH1GRqGiZdYtW5w+IXXaBNCHTIaheM4= github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.14 h1:PZHqQACxYb8mYgms4RZbhZG0a7dPW06xOjmaH0EJC/I= @@ -18,18 +18,20 @@ github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.14 h1:bOS19y6zlJwagBfHxs github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.14/go.mod h1:1ipeGBMAxZ0xcTm6y6paC2C/J6f6OO7LBODV9afuAyM= github.com/aws/aws-sdk-go-v2/internal/ini v1.8.4 h1:WKuaxf++XKWlHWu9ECbMlha8WOEGm0OUEZqm4K/Gcfk= github.com/aws/aws-sdk-go-v2/internal/ini v1.8.4/go.mod h1:ZWy7j6v1vWGmPReu0iSGvRiise4YI5SkR3OHKTZ6Wuc= -github.com/aws/aws-sdk-go-v2/service/ec2 v1.274.0 h1:Q2+WD4KSVRkd27QxD9I30nM3O7B4WYwE+ua5dm2NJY0= -github.com/aws/aws-sdk-go-v2/service/ec2 v1.274.0/go.mod h1:QrV+/GjhSrJh6MRRuTO6ZEg4M2I0nwPakf0lZHSrE1o= +github.com/aws/aws-sdk-go-v2/service/ec2 v1.275.0 h1:ymusjrsOjrcVBQNQXYFIQEHJIJ17/m+VoDSmWIMjGe0= +github.com/aws/aws-sdk-go-v2/service/ec2 v1.275.0/go.mod h1:QrV+/GjhSrJh6MRRuTO6ZEg4M2I0nwPakf0lZHSrE1o= github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.3 h1:x2Ibm/Af8Fi+BH+Hsn9TXGdT+hKbDd5XOTZxTMxDk7o= github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.3/go.mod h1:IW1jwyrQgMdhisceG8fQLmQIydcT/jWY21rFhzgaKwo= github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.14 h1:FIouAnCE46kyYqyhs0XEBDFFSREtdnr8HQuLPQPLCrY= github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.14/go.mod h1:UTwDc5COa5+guonQU8qBikJo1ZJ4ln2r1MkF7Dqag1E= -github.com/aws/aws-sdk-go-v2/service/sso v1.30.1 h1:0JPwLz1J+5lEOfy/g0SURC9cxhbQ1lIMHMa+AHZSzz0= -github.com/aws/aws-sdk-go-v2/service/sso v1.30.1/go.mod h1:fKvyjJcz63iL/ftA6RaM8sRCtN4r4zl4tjL3qw5ec7k= -github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.5 h1:OWs0/j2UYR5LOGi88sD5/lhN6TDLG6SfA7CqsQO9zF0= -github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.5/go.mod h1:klO+ejMvYsB4QATfEOIXk8WAEwN4N0aBfJpvC+5SZBo= -github.com/aws/aws-sdk-go-v2/service/sts v1.39.1 h1:mLlUgHn02ue8whiR4BmxxGJLR2gwU6s6ZzJ5wDamBUs= -github.com/aws/aws-sdk-go-v2/service/sts v1.39.1/go.mod h1:E19xDjpzPZC7LS2knI9E6BaRFDK43Eul7vd6rSq2HWk= +github.com/aws/aws-sdk-go-v2/service/signin v1.0.2 h1:MxMBdKTYBjPQChlJhi4qlEueqB1p1KcbTEa7tD5aqPs= +github.com/aws/aws-sdk-go-v2/service/signin v1.0.2/go.mod h1:iS6EPmNeqCsGo+xQmXv0jIMjyYtQfnwg36zl2FwEouk= +github.com/aws/aws-sdk-go-v2/service/sso v1.30.5 h1:ksUT5KtgpZd3SAiFJNJ0AFEJVva3gjBmN7eXUZjzUwQ= +github.com/aws/aws-sdk-go-v2/service/sso v1.30.5/go.mod h1:av+ArJpoYf3pgyrj6tcehSFW+y9/QvAY8kMooR9bZCw= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.10 h1:GtsxyiF3Nd3JahRBJbxLCCdYW9ltGQYrFWg8XdkGDd8= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.10/go.mod h1:/j67Z5XBVDx8nZVp9EuFM9/BS5dvBznbqILGuu73hug= +github.com/aws/aws-sdk-go-v2/service/sts v1.41.2 h1:a5UTtD4mHBU3t0o6aHQZFJTNKVfxFWfPX7J0Lr7G+uY= +github.com/aws/aws-sdk-go-v2/service/sts v1.41.2/go.mod h1:6TxbXoDSgBQ225Qd8Q+MbxUxUh6TtNKwbRt/EPS9xso= github.com/aws/smithy-go v1.23.2 h1:Crv0eatJUQhaManss33hS5r40CG3ZFH+21XSkqMrIUM= github.com/aws/smithy-go v1.23.2/go.mod h1:LEj2LM3rBRQJxPZTB4KuzZkaZYnZPnvgIhb4pu07mx0= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= @@ -119,6 +121,8 @@ github.com/go-test/deep v1.1.0 h1:WOcxcdHcvdgThNXjw0t76K42FXTU7HpNQWHpA2HHNlg= github.com/go-test/deep v1.1.0/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE= github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= +github.com/goccy/go-yaml v1.19.0 h1:EmkZ9RIsX+Uq4DYFowegAuJo8+xdX3T/2dwNPXbxEYE= +github.com/goccy/go-yaml v1.19.0/go.mod h1:XBurs7gK8ATbW4ZPGKgcbrY1Br56PdM69F7LkFRi1kA= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= @@ -145,8 +149,8 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.0 h1:Wqo399gCIufwto+VfwCSvsnfGpF/w5E9CNxSwbpD6No= github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.0/go.mod h1:qmOFXW2epJhM0qSnUUYpldc7gVz2KMQwJ/QYCDIa7XU= -github.com/hashicorp/go-version v1.7.0 h1:5tqGy27NaOTB8yJKUZELlFAS/LTKJkrmONwQKeRZfjY= -github.com/hashicorp/go-version v1.7.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= +github.com/hashicorp/go-version v1.8.0 h1:KAkNb1HAiZd1ukkxDFGmokVZe1Xy9HG6NUp+bPle2i4= +github.com/hashicorp/go-version v1.8.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= diff --git a/internal/testutils/pointers.go b/internal/testutils/pointers.go new file mode 100644 index 0000000000..232dd0a3ea --- /dev/null +++ b/internal/testutils/pointers.go @@ -0,0 +1,22 @@ +package testutils + +import ( + "net" + + "github.com/cloudnativelabs/kube-router/v2/pkg/utils" +) + +type TestValue interface { + string | uint32 | net.IP | utils.Base64String +} + +func ValToPtr[V TestValue](v V) *V { + return &v +} + +func PtrToVal[V TestValue](v *V) V { + if v == nil { + return *new(V) + } + return *v +} diff --git a/pkg/bgp/peer_config.go b/pkg/bgp/peer_config.go new file mode 100644 index 0000000000..c104e3ddef --- /dev/null +++ b/pkg/bgp/peer_config.go @@ -0,0 +1,197 @@ +package bgp + +import ( + "errors" + "fmt" + "net" + "strconv" + "strings" + + "github.com/cloudnativelabs/kube-router/v2/pkg/options" + "github.com/cloudnativelabs/kube-router/v2/pkg/utils" + "github.com/goccy/go-yaml" +) + +type PeerConfig struct { + remoteASN uint32 `yaml:"remoteasn"` + remoteIP net.IP `yaml:"remoteip"` + localIP string `yaml:"localip"` + password utils.Base64String `yaml:"password"` + port *uint32 `yaml:"port"` +} + +func NewPeerConfig(remoteIPStr string, remoteASN uint32, port *uint32, b64EncodedPassword utils.Base64String, + localIP string, +) (PeerConfig, error) { + remoteIP := net.ParseIP(remoteIPStr) + if remoteIP == nil { + return PeerConfig{}, fmt.Errorf("invalid IP address: %s", remoteIPStr) + } + if err := validateASN(remoteASN); err != nil { + return PeerConfig{}, err + } + + return PeerConfig{ + remoteIP: remoteIP, + remoteASN: remoteASN, + localIP: localIP, + password: b64EncodedPassword, + port: port, + }, nil +} + +func (p PeerConfig) RemoteASN() uint32 { + return p.remoteASN +} + +func (p PeerConfig) RemoteIP() net.IP { + return p.remoteIP +} + +func (p PeerConfig) LocalIP() string { + return p.localIP +} + +func (p PeerConfig) Password() string { + return string(p.password) +} + +func (p PeerConfig) Port() *uint32 { + return p.port +} + +// Custom Stringer to prevent leaking passwords when printed +func (p PeerConfig) String() string { + var fields []string + if p.localIP != "" { + fields = append(fields, fmt.Sprintf("LocalIP: %s", p.localIP)) + } + if p.port != nil { + fields = append(fields, fmt.Sprintf("Port: %d", *p.port)) + } + if p.remoteASN != uint32(0) { + fields = append(fields, fmt.Sprintf("RemoteASN: %d", p.remoteASN)) + } + if p.remoteIP != nil { + fields = append(fields, fmt.Sprintf("RemoteIP: %v", p.remoteIP)) + } + return fmt.Sprintf("PeerConfig{%s}", strings.Join(fields, ", ")) +} + +func (p *PeerConfig) UnmarshalYAML(raw []byte) error { + tmp := struct { + LocalIP *string `yaml:"localip"` + Password *utils.Base64String `yaml:"password"` + Port *uint32 `yaml:"port"` + RemoteASN *uint32 `yaml:"remoteasn"` + RemoteIP *string `yaml:"remoteip"` + }{} + + if err := yaml.Unmarshal(raw, &tmp); err != nil { + return fmt.Errorf("failed to unmarshal peer config: %w", err) + } + + if tmp.RemoteIP == nil { + return errors.New("remoteip cannot be empty") + } + if tmp.RemoteASN == nil { + return errors.New("remoteasn cannot be empty") + } + if err := validateASN(*tmp.RemoteASN); err != nil { + return err + } + if tmp.LocalIP != nil { + p.localIP = *tmp.LocalIP + } + if tmp.Password != nil { + p.password = *tmp.Password + } + p.port = tmp.Port + p.remoteASN = *tmp.RemoteASN + ip := net.ParseIP(*tmp.RemoteIP) + if ip == nil { + return fmt.Errorf("%s is not a valid IP address", *tmp.RemoteIP) + } + p.remoteIP = ip + return nil +} + +type PeerConfigs []PeerConfig + +func (p PeerConfigs) RemoteIPStrings() []string { + remoteIPs := make([]string, 0) + for _, cfg := range p { + remoteIPs = append(remoteIPs, cfg.RemoteIP().String()) + } + return remoteIPs +} + +// Prints the PeerConfigs without the passwords leaking +func (p PeerConfigs) String() string { + pcs := make([]string, len(p)) + for i, pc := range p { + pcs[i] = pc.String() + } + return fmt.Sprintf("PeerConfigs[%s]", strings.Join(pcs, ",")) +} + +func NewPeerConfigs( + remoteIPs []string, + remoteASNs []uint32, + ports []uint32, + b64EncodedPasswords []string, + localIPs []string, + localAddress string, +) (PeerConfigs, error) { + if len(remoteIPs) != len(remoteASNs) { + return nil, errors.New("invalid peer router config, the number of IPs and ASN numbers must be equal") + } + if len(remoteIPs) != len(b64EncodedPasswords) && len(b64EncodedPasswords) != 0 { + return nil, errors.New("invalid peer router config. The number of passwords should either be zero, or " + + "one per peer router. Use blank items if a router doesn't expect a password. Example: \"pass,,pass\" " + + "OR [\"pass\",\"\",\"pass\"]") + } + if len(remoteIPs) != len(ports) && len(ports) != 0 { + return nil, fmt.Errorf("invalid peer router config. The number of ports should either be zero, or "+ + "one per peer router. If blank items are used, it will default to standard BGP port, %s. ", + strconv.Itoa(options.DefaultBgpPort)) + } + if len(remoteIPs) != len(localIPs) && len(localIPs) != 0 { + return nil, fmt.Errorf("invalid peer router config. The number of localIPs should either be zero, or "+ + "one per peer router. If blank items are used, it will default to nodeIP, %s. ", localAddress) + } + + peerCfgs := make(PeerConfigs, len(remoteIPs)) + for i, remoteIP := range remoteIPs { + var localIP string + var pw utils.Base64String + var port *uint32 + if len(ports) != 0 { + port = &ports[i] + } + if len(b64EncodedPasswords) != 0 { + pw = utils.Base64String(b64EncodedPasswords[i]) + } + if len(localIPs) != 0 { + localIP = localIPs[i] + } + peerCfg, err := NewPeerConfig(remoteIP, remoteASNs[i], port, pw, localIP) + if err != nil { + return nil, err + } + peerCfgs[i] = peerCfg + } + + return peerCfgs, nil +} + +func validateASN(asn uint32) error { + if (asn < 1 || asn > 23455) && + (asn < 23457 || asn > 63999) && + (asn < 64512 || asn > 65534) && + (asn < 131072 || asn > 4199999999) && + (asn < 4200000000 || asn > 4294967294) { + return fmt.Errorf("reserved ASN number \"%d\" for global BGP peer", asn) + } + return nil +} diff --git a/pkg/bgp/peer_config_test.go b/pkg/bgp/peer_config_test.go new file mode 100644 index 0000000000..9551b07283 --- /dev/null +++ b/pkg/bgp/peer_config_test.go @@ -0,0 +1,261 @@ +package bgp + +import ( + "net" + "testing" + + "github.com/cloudnativelabs/kube-router/v2/internal/testutils" + "github.com/cloudnativelabs/kube-router/v2/pkg/utils" + "github.com/goccy/go-yaml" + "github.com/stretchr/testify/assert" +) + +func TestPeerConfig_String(t *testing.T) { + tests := []struct { + name string + config PeerConfig + expected string + }{ + { + name: "LocalIP", + config: PeerConfig{ + localIP: "192.168.1.1", + }, + expected: "PeerConfig{LocalIP: 192.168.1.1}", + }, + { + name: "Port", + config: PeerConfig{ + port: testutils.ValToPtr(uint32(179)), + }, + expected: "PeerConfig{Port: 179}", + }, + { + name: "RemoteASN", + config: PeerConfig{ + remoteASN: uint32(65000), + }, + expected: "PeerConfig{RemoteASN: 65000}", + }, + { + name: "RemoteIP", + config: PeerConfig{ + remoteIP: net.ParseIP("10.0.0.1"), + }, + expected: "PeerConfig{RemoteIP: 10.0.0.1}", + }, + { + name: "RemoteIP with IPv6", + config: PeerConfig{ + remoteIP: net.ParseIP("2001:db8::1"), + }, + expected: "PeerConfig{RemoteIP: 2001:db8::1}", + }, + { + name: "Password should not be printed", + config: PeerConfig{ + password: utils.Base64String("password"), + }, + expected: "PeerConfig{}", + }, + { + name: "All fields - password should not be printed", + config: PeerConfig{ + localIP: "192.168.1.1", + password: utils.Base64String("password"), + port: testutils.ValToPtr(uint32(179)), + remoteASN: uint32(65000), + remoteIP: net.ParseIP("10.0.0.1"), + }, + expected: "PeerConfig{LocalIP: 192.168.1.1, Port: 179, RemoteASN: 65000, RemoteIP: 10.0.0.1}", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(st *testing.T) { + result := tt.config.String() + assert.Equal(st, tt.expected, result) + }) + } +} + +func TestPeerConfig_UnmarshalYAML(t *testing.T) { + tests := []struct { + name string + input []byte + expected PeerConfig + errorContains string + }{ + { + name: "empty YAML", + expected: PeerConfig{}, + }, + { + name: "Remote IP must be set", + input: []byte(`remoteasn: 1234`), + errorContains: "remoteip cannot be empty", + }, + { + name: "Remote ASN must be set", + input: []byte(`remoteip: 1.1.1.1`), + errorContains: "remoteasn cannot be empty", + }, + { + name: "Invalid ASN", + input: []byte(`remoteip: 1.1.1.1 +remoteasn: 0`), + errorContains: "reserved ASN number", + }, + { + name: "Invalid remote IPv4", + input: []byte(`remoteip: 1234.12 +remoteasn: 1234`), + errorContains: "is not a valid IP address", + }, + { + name: "Invalid remote IPv6", + input: []byte(`remoteip: 2001::db8::abcd +remoteasn: 1234`), + errorContains: "is not a valid IP address", + }, + { + name: "Valid peer config YAML", + input: []byte(`remoteip: 1.1.1.1 +remoteasn: 1234 +password: aGVsbG8= +`), + expected: PeerConfig{ + password: utils.Base64String("hello"), + remoteIP: net.ParseIP("1.1.1.1"), + remoteASN: uint32(1234), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(st *testing.T) { + var actual PeerConfig + err := yaml.Unmarshal(tt.input, &actual) + if tt.errorContains != "" { + assert.ErrorContains(st, err, tt.errorContains) + } else { + assert.NoError(st, err) + assert.Equal(st, tt.expected, actual) + } + }) + } +} + +func TestPeerConfigs_String(t *testing.T) { + tests := []struct { + name string + configs PeerConfigs + expected string + }{ + { + name: "empty PeerConfigs", + configs: PeerConfigs{}, + expected: "PeerConfigs[]", + }, + { + name: "PeerConfig - password should not be printed", + configs: PeerConfigs{ + { + localIP: "192.168.1.1", + password: utils.Base64String("secret"), + port: testutils.ValToPtr(uint32(179)), + remoteASN: uint32(65000), + remoteIP: net.ParseIP("10.0.0.1"), + }, + }, + expected: "PeerConfigs[PeerConfig{LocalIP: 192.168.1.1, Port: 179, RemoteASN: 65000, RemoteIP: 10.0.0.1}]", + }, + { + name: "multiple PeerConfigs - passwords should not be printed", + configs: PeerConfigs{ + { + localIP: "192.168.1.1", + password: utils.Base64String("secret"), + remoteASN: uint32(65000), + remoteIP: net.ParseIP("10.0.0.1"), + }, + { + port: testutils.ValToPtr(uint32(179)), + remoteASN: uint32(65001), + remoteIP: net.ParseIP("10.0.0.2"), + }, + { + remoteIP: net.ParseIP("10.0.0.3"), + }, + }, + expected: "PeerConfigs[PeerConfig{LocalIP: 192.168.1.1, RemoteASN: 65000, RemoteIP: 10.0.0.1},PeerConfig{Port: 179, RemoteASN: 65001, RemoteIP: 10.0.0.2},PeerConfig{RemoteIP: 10.0.0.3}]", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(st *testing.T) { + assert.Equal(st, tt.expected, tt.configs.String()) + }) + } +} + +func Test_NewPeerConfigs(t *testing.T) { + tests := []struct { + name string + remoteIPs []string + remoteASNs []uint32 + ports []uint32 + b64EncodedPasswords []string + localIPs []string + localAddress string + errorContains string + }{ + { + name: "all fields set to nil returns nothing", + }, + { + name: "number of remote IPs and remote ASNs don't match returns error", + remoteIPs: []string{"10.0.0.1", "10.0.0.2"}, + remoteASNs: []uint32{1234}, + errorContains: "the number of IPs and ASN numbers must be equal", + }, + { + name: "number of remote IPs and passwords don't match returns error", + remoteIPs: []string{"10.0.0.1", "10.0.0.2"}, + remoteASNs: []uint32{1234, 2345}, + b64EncodedPasswords: []string{"fakepassword"}, + errorContains: "number of passwords", + }, + { + name: "number of remote IPs and ports don't match returns error", + remoteIPs: []string{"10.0.0.1", "10.0.0.2"}, + remoteASNs: []uint32{1234, 2345}, + ports: []uint32{8080}, + errorContains: "number of ports", + }, + { + name: "number of remote IPs and local IPs don't match returns error", + remoteIPs: []string{"10.0.0.1", "10.0.0.2"}, + remoteASNs: []uint32{1234, 2345}, + localIPs: []string{"1.1.1.1"}, + errorContains: "number of localIPs", + }, + { + name: "remoteASN contains a reserved ASN number returns error", + remoteIPs: []string{"10.0.0.1", "10.0.0.2"}, + remoteASNs: []uint32{0, 2345}, + errorContains: "reserved ASN", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := NewPeerConfigs(tt.remoteIPs, tt.remoteASNs, tt.ports, tt.b64EncodedPasswords, tt.localIPs, tt.localAddress) + if tt.errorContains != "" { + assert.ErrorContains(t, err, tt.errorContains) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/pkg/controllers/routing/bgp_peers.go b/pkg/controllers/routing/bgp_peers.go index 3f290575f0..ea07011c5a 100644 --- a/pkg/controllers/routing/bgp_peers.go +++ b/pkg/controllers/routing/bgp_peers.go @@ -2,13 +2,13 @@ package routing import ( "context" - "errors" "fmt" "net" "strconv" "strings" "time" + "github.com/cloudnativelabs/kube-router/v2/pkg/bgp" "github.com/cloudnativelabs/kube-router/v2/pkg/metrics" "github.com/cloudnativelabs/kube-router/v2/pkg/options" "github.com/cloudnativelabs/kube-router/v2/pkg/utils" @@ -285,51 +285,23 @@ func (nrc *NetworkRoutingController) connectToExternalBGPPeers(server *gobgp.Bgp } // Does validation and returns neighbor configs -func newGlobalPeers(ips []net.IP, ports []uint32, asns []uint32, passwords []string, localips []string, - holdtime float64, localAddress string) ([]*gobgpapi.Peer, error) { - peers := make([]*gobgpapi.Peer, 0) - - // Validations - if len(ips) != len(asns) { - return nil, errors.New("invalid peer router config, the number of IPs and ASN numbers must be equal") - } - - if len(ips) != len(passwords) && len(passwords) != 0 { - return nil, errors.New("invalid peer router config. The number of passwords should either be zero, or " + - "one per peer router. Use blank items if a router doesn't expect a password. Example: \"pass,,pass\" " + - "OR [\"pass\",\"\",\"pass\"]") - } - - if len(ips) != len(ports) && len(ports) != 0 { - return nil, fmt.Errorf("invalid peer router config. The number of ports should either be zero, or "+ - "one per peer router. If blank items are used, it will default to standard BGP port, %s. "+ - "Example: \"port,,port\" OR [\"port\",\"\",\"port\"]", strconv.Itoa(options.DefaultBgpPort)) - } - - if len(ips) != len(localips) && len(localips) != 0 { - return nil, fmt.Errorf("invalid peer router config. The number of localIPs should either be zero, or "+ - "one per peer router. If blank items are used, it will default to nodeIP, %s. "+ - "Example: \"10.1.1.1,,10.1.1.2\" OR [\"10.1.1.1\",\"\",\"10.1.1.2\"]", localAddress) - } - - for i := 0; i < len(ips); i++ { - if (asns[i] < 1 || asns[i] > 23455) && - (asns[i] < 23457 || asns[i] > 63999) && - (asns[i] < 64512 || asns[i] > 65534) && - (asns[i] < 131072 || asns[i] > 4199999999) && - (asns[i] < 4200000000 || asns[i] > 4294967294) { - return nil, fmt.Errorf("reserved ASN number \"%d\" for global BGP peer", - asns[i]) - } - +func newGlobalPeers( + peerConfigs bgp.PeerConfigs, + holdtime float64, + localAddress string, +) []*gobgpapi.Peer { + peers := make([]*gobgpapi.Peer, len(peerConfigs)) + + for i, peerConfig := range peerConfigs { // explicitly set neighbors.transport.config.local-address with primaryIP which is configured // as their neighbor address at the remote peers. // this prevents the controller from initiating connection to its peers with a different IP address // when multiple L3 interfaces are active. peer := &gobgpapi.Peer{ Conf: &gobgpapi.PeerConf{ - NeighborAddress: ips[i].String(), - PeerAsn: asns[i], + NeighborAddress: peerConfig.RemoteIP().String(), + PeerAsn: peerConfig.RemoteASN(), + AuthPassword: peerConfig.Password(), }, Timers: &gobgpapi.Timers{Config: &gobgpapi.TimersConfig{HoldTime: uint64(holdtime)}}, Transport: &gobgpapi.Transport{ @@ -340,23 +312,18 @@ func newGlobalPeers(ips []net.IP, ports []uint32, asns []uint32, passwords []str }, } - if len(ports) != 0 { - peer.Transport.RemotePort = ports[i] - } - - if len(passwords) != 0 { - peer.Conf.AuthPassword = passwords[i] + if peerConfig.Port() != nil { + peer.Transport.RemotePort = *peerConfig.Port() } - // if localip is set and is non-blank for BGP configuration override primaryIP choice set for peer above - if len(localips) != 0 && localips[i] != "" { - peer.Transport.LocalAddress = localips[i] + if peerConfig.LocalIP() != "" { + peer.Transport.LocalAddress = peerConfig.LocalIP() } - peers = append(peers, peer) + peers[i] = peer } - return peers, nil + return peers } func (nrc *NetworkRoutingController) newNodeEventHandler() cache.ResourceEventHandler { diff --git a/pkg/controllers/routing/network_routes_controller.go b/pkg/controllers/routing/network_routes_controller.go index 235220089a..69b2c97aff 100644 --- a/pkg/controllers/routing/network_routes_controller.go +++ b/pkg/controllers/routing/network_routes_controller.go @@ -12,6 +12,7 @@ import ( "sync" "time" + "github.com/goccy/go-yaml" "google.golang.org/protobuf/types/known/anypb" "github.com/ccoveille/go-safecast/v2" @@ -43,9 +44,14 @@ const ( nodeCustomImportRejectAnnotation = "kube-router.io/node.bgp.customimportreject" pathPrependASNAnnotation = "kube-router.io/path-prepend.as" pathPrependRepeatNAnnotation = "kube-router.io/path-prepend.repeat-n" - peerASNAnnotation = "kube-router.io/peer.asns" - peerIPAnnotation = "kube-router.io/peer.ips" - peerLocalIPAnnotation = "kube-router.io/peer.localips" + + // Prefer using this consolidated BGP config annotation, as the + // individual annotation config options are deprecated. + peersAnnotation = "kube-router.io/peers" + + peerASNAnnotation = "kube-router.io/peer.asns" + peerIPAnnotation = "kube-router.io/peer.ips" + peerLocalIPAnnotation = "kube-router.io/peer.localips" //nolint:gosec // this is not a hardcoded password peerPasswordAnnotation = "kube-router.io/peer.passwords" peerPortAnnotation = "kube-router.io/peer.ports" @@ -76,7 +82,11 @@ const ( type RouteSyncer interface { AddInjectedRoute(dst *net.IPNet, route *netlink.Route) DelInjectedRoute(dst *net.IPNet) - Run(healthChan chan<- *healthcheck.ControllerHeartbeat, stopCh <-chan struct{}, wg *sync.WaitGroup) + Run( + healthChan chan<- *healthcheck.ControllerHeartbeat, + stopCh <-chan struct{}, + wg *sync.WaitGroup, + ) SyncLocalRouteTable() error } @@ -156,8 +166,11 @@ type NetworkRoutingController struct { } // Run runs forever until we are notified on stop channel -func (nrc *NetworkRoutingController) Run(healthChan chan<- *healthcheck.ControllerHeartbeat, stopCh <-chan struct{}, - wg *sync.WaitGroup) { +func (nrc *NetworkRoutingController) Run( + healthChan chan<- *healthcheck.ControllerHeartbeat, + stopCh <-chan struct{}, + wg *sync.WaitGroup, +) { var err error if nrc.enableCNI { nrc.updateCNIConfig() @@ -245,26 +258,39 @@ func (nrc *NetworkRoutingController) Run(healthChan chan<- *healthcheck.Controll linkAttrs.Name = "kube-bridge" bridge := &netlink.Bridge{LinkAttrs: linkAttrs} if err = netlink.LinkAdd(bridge); err != nil { - klog.Errorf("Failed to create `kube-router` bridge due to %s. Will be created by CNI bridge "+ - "plugin when pod is launched.", err.Error()) + klog.Errorf( + "Failed to create `kube-router` bridge due to %s. Will be created by CNI bridge "+ + "plugin when pod is launched.", + err.Error(), + ) } kubeBridgeIf, err = netlink.LinkByName("kube-bridge") if err != nil { - klog.Errorf("Failed to find created `kube-router` bridge due to %s. Will be created by CNI "+ - "bridge plugin when pod is launched.", err.Error()) + klog.Errorf( + "Failed to find created `kube-router` bridge due to %s. Will be created by CNI "+ + "bridge plugin when pod is launched.", + err.Error(), + ) } err = netlink.LinkSetUp(kubeBridgeIf) if err != nil { - klog.Errorf("Failed to bring `kube-router` bridge up due to %s. Will be created by CNI bridge "+ - "plugin at later point when pod is launched.", err.Error()) + klog.Errorf( + "Failed to bring `kube-router` bridge up due to %s. Will be created by CNI bridge "+ + "plugin at later point when pod is launched.", + err.Error(), + ) } } if nrc.autoMTU { mtu, err := nrc.krNode.GetNodeMTU() if err != nil { - klog.Errorf("Failed to find MTU for node IP: %s for intelligently setting the kube-bridge MTU "+ - "due to %s.", nrc.krNode.GetPrimaryNodeIP(), err.Error()) + klog.Errorf( + "Failed to find MTU for node IP: %s for intelligently setting the kube-bridge MTU "+ + "due to %s.", + nrc.krNode.GetPrimaryNodeIP(), + err.Error(), + ) } if mtu > 0 { klog.Infof("Setting MTU of kube-bridge interface to: %d", mtu) @@ -272,12 +298,17 @@ func (nrc *NetworkRoutingController) Run(healthChan chan<- *healthcheck.Controll if err != nil { klog.Errorf( "Failed to set MTU for kube-bridge interface due to: %s (kubeBridgeIf: %#v, mtu: %v)", - err.Error(), kubeBridgeIf, mtu, + err.Error(), + kubeBridgeIf, + mtu, ) // need to correct kuberouter.conf because autoConfigureMTU() may have set an invalid value! currentMTU := kubeBridgeIf.Attrs().MTU if currentMTU > 0 && currentMTU != mtu { - klog.Warningf("Updating config file with current MTU for kube-bridge: %d", currentMTU) + klog.Warningf( + "Updating config file with current MTU for kube-bridge: %d", + currentMTU, + ) cniNetConf, err := utils.NewCNINetworkConfig(nrc.cniConfFile) if err == nil { cniNetConf.SetMTU(currentMTU) @@ -470,7 +501,8 @@ func (nrc *NetworkRoutingController) watchBgpUpdates() { if path.NeighborIp == "" { return } - klog.V(2).Infof("Processing bgp route advertisement from peer: %s", path.NeighborIp) + klog.V(2). + Infof("Processing bgp route advertisement from peer: %s", path.NeighborIp) if err := nrc.injectRoute(path); err != nil { klog.Errorf("failed to inject routes due to: %v", err) } @@ -544,8 +576,10 @@ func (nrc *NetworkRoutingController) advertisePodRoute() error { if nrc.krNode.IsIPv6Capable() { nodePrimaryIPv6IP := nrc.krNode.FindBestIPv6NodeAddress() if nodePrimaryIPv6IP == nil { - return fmt.Errorf("previous logic marked this node as IPv6 capable, but we couldn't find any " + - "available IPv6 node IPs, this shouldn't happen") + return fmt.Errorf( + "previous logic marked this node as IPv6 capable, but we couldn't find any " + + "available IPv6 node IPs, this shouldn't happen", + ) } for _, cidr := range nrc.podIPv6CIDRs { @@ -1063,8 +1097,11 @@ func (nrc *NetworkRoutingController) startBgpServer(grpcServer bool) error { // Make sure that the address type matches what we're capable of before listening if ip.To4() != nil { if !nrc.krNode.IsIPv4Capable() { - klog.Warningf("was configured to listen on %s, but node is not enabled for IPv4 or does not "+ - "have any IPv4 addresses configured for it, skipping", addr) + klog.Warningf( + "was configured to listen on %s, but node is not enabled for IPv4 or does not "+ + "have any IPv4 addresses configured for it, skipping", + addr, + ) continue } } else { @@ -1098,107 +1135,30 @@ func (nrc *NetworkRoutingController) startBgpServer(grpcServer bool) error { // If the global routing peer is configured then peer with it // else attempt to get peers from node specific BGP annotations. if len(nrc.globalPeerRouters) == 0 { - // Get Global Peer Router ASN configs - nodeBgpPeerAsnsAnnotation, ok := node.Annotations[peerASNAnnotation] - if !ok { - klog.Infof("Could not find BGP peer info for the node in the node annotations so " + - "skipping configuring peer.") - return nil - } - - asnStrings := stringToSlice(nodeBgpPeerAsnsAnnotation, ",") - peerASNs, err := stringSliceToUInt32(asnStrings) + klog.V(2). + Infof("Attempting to construct peer configs from annotation: %+v", node.Annotations) + peerCfgs, err := bgpPeerConfigsFromAnnotations( + node.Annotations, + nrc.krNode.GetPrimaryNodeIP().String(), + ) if err != nil { err2 := nrc.bgpServer.StopBgp(context.Background(), &gobgpapi.StopBgpRequest{}) if err2 != nil { klog.Errorf("Failed to stop bgpServer: %s", err2) } - return fmt.Errorf("failed to parse node's Peer ASN Numbers Annotation: %s", err) + return err } - - // Get Global Peer Router IP Address configs - nodeBgpPeersAnnotation, ok := node.Annotations[peerIPAnnotation] - if !ok { - klog.Infof("Could not find BGP peer info for the node in the node annotations " + - "so skipping configuring peer.") + // Early exist because no BGP peer info was set in annotations for the node + if peerCfgs == nil { return nil } - ipStrings := stringToSlice(nodeBgpPeersAnnotation, ",") - peerIPs, err := stringSliceToIPs(ipStrings) - if err != nil { - err2 := nrc.bgpServer.StopBgp(context.Background(), &gobgpapi.StopBgpRequest{}) - if err2 != nil { - klog.Errorf("Failed to stop bgpServer: %s", err2) - } - - return fmt.Errorf("failed to parse node's Peer Addresses Annotation: %s", err) - } - - // Get Global Peer Router ASN configs - nodeBgpPeerPortsAnnotation, ok := node.Annotations[peerPortAnnotation] - // Default to default BGP port if port annotation is not found - var peerPorts = make([]uint32, 0) - if ok { - portStrings := stringToSlice(nodeBgpPeerPortsAnnotation, ",") - peerPorts, err = stringSliceToUInt32(portStrings) - if err != nil { - err2 := nrc.bgpServer.StopBgp(context.Background(), &gobgpapi.StopBgpRequest{}) - if err2 != nil { - klog.Errorf("Failed to stop bgpServer: %s", err2) - } - return fmt.Errorf("failed to parse node's Peer Port Numbers Annotation: %s", err) - } - } - - // Get Global Peer Router Password configs - var peerPasswords []string - nodeBGPPasswordsAnnotation, ok := node.Annotations[peerPasswordAnnotation] - if !ok { - klog.Infof("Could not find BGP peer password info in the node's annotations. Assuming no passwords.") - } else { - passStrings := stringToSlice(nodeBGPPasswordsAnnotation, ",") - peerPasswords, err = stringSliceB64Decode(passStrings) - if err != nil { - err2 := nrc.bgpServer.StopBgp(context.Background(), &gobgpapi.StopBgpRequest{}) - if err2 != nil { - klog.Errorf("Failed to stop bgpServer: %s", err2) - } - return fmt.Errorf("failed to parse node's Peer Passwords Annotation") - } - } - - // Get Global Peer Router LocalIP configs - var peerLocalIPs []string - nodeBGPPeerLocalIPs, ok := node.Annotations[peerLocalIPAnnotation] - if !ok { - klog.Infof("Could not find BGP peer local ip info in the node's annotations. Assuming node IP.") - } else { - peerLocalIPs = stringToSlice(nodeBGPPeerLocalIPs, ",") - err = func() error { - for _, s := range peerLocalIPs { - if s != "" { - ip := net.ParseIP(s) - if ip == nil { - return fmt.Errorf("could not parse \"%s\" as an IP", s) - } - } - } - - return nil - }() - if err != nil { - err2 := nrc.bgpServer.StopBgp(context.Background(), &gobgpapi.StopBgpRequest{}) - if err2 != nil { - klog.Errorf("Failed to stop bgpServer: %s", err2) - } - - return fmt.Errorf("failed to parse node's Peer Local Addresses Annotation: %s", err) - } - } // Create and set Global Peer Router complete configs - nrc.globalPeerRouters, err = newGlobalPeers(peerIPs, peerPorts, peerASNs, peerPasswords, peerLocalIPs, - nrc.bgpHoldtime, nrc.krNode.GetPrimaryNodeIP().String()) + nrc.globalPeerRouters = newGlobalPeers( + peerCfgs, + nrc.bgpHoldtime, + nrc.krNode.GetPrimaryNodeIP().String(), + ) if err != nil { err2 := nrc.bgpServer.StopBgp(context.Background(), &gobgpapi.StopBgpRequest{}) if err2 != nil { @@ -1208,12 +1168,18 @@ func (nrc *NetworkRoutingController) startBgpServer(grpcServer bool) error { return fmt.Errorf("failed to process Global Peer Router configs: %s", err) } - nrc.nodePeerRouters = ipStrings + nrc.nodePeerRouters = peerCfgs.RemoteIPStrings() } if len(nrc.globalPeerRouters) != 0 { - err := nrc.connectToExternalBGPPeers(nrc.bgpServer, nrc.globalPeerRouters, nrc.bgpGracefulRestart, - nrc.bgpGracefulRestartDeferralTime, nrc.bgpGracefulRestartTime, nrc.peerMultihopTTL) + err := nrc.connectToExternalBGPPeers( + nrc.bgpServer, + nrc.globalPeerRouters, + nrc.bgpGracefulRestart, + nrc.bgpGracefulRestartDeferralTime, + nrc.bgpGracefulRestartTime, + nrc.peerMultihopTTL, + ) if err != nil { err2 := nrc.bgpServer.StopBgp(context.Background(), &gobgpapi.StopBgpRequest{}) if err2 != nil { @@ -1284,8 +1250,8 @@ func (nrc *NetworkRoutingController) setupHandlers(node *v1core.Node) error { func NewNetworkRoutingController(clientset kubernetes.Interface, kubeRouterConfig *options.KubeRouterConfig, nodeInformer cache.SharedIndexInformer, svcInformer cache.SharedIndexInformer, - epSliceInformer cache.SharedIndexInformer, ipsetMutex *sync.Mutex) (*NetworkRoutingController, error) { - + epSliceInformer cache.SharedIndexInformer, ipsetMutex *sync.Mutex, +) (*NetworkRoutingController, error) { var err error nrc := NetworkRoutingController{ipsetMutex: ipsetMutex} @@ -1464,8 +1430,28 @@ func NewNetworkRoutingController(clientset kubernetes.Interface, } } - nrc.globalPeerRouters, err = newGlobalPeers(kubeRouterConfig.PeerRouters, peerPorts, - peerASNs, peerPasswords, nil, nrc.bgpHoldtime, nrc.krNode.GetPrimaryNodeIP().String()) + peerRouterIPs := make([]string, len(kubeRouterConfig.PeerRouters)) + for i, pr := range kubeRouterConfig.PeerRouters { + peerRouterIPs[i] = pr.String() + } + + peerCfgs, err := bgp.NewPeerConfigs( + peerRouterIPs, + peerASNs, + peerPorts, + peerPasswords, + nil, + nrc.krNode.GetPrimaryNodeIP().String(), + ) + if err != nil { + return nil, err + } + + nrc.globalPeerRouters = newGlobalPeers( + peerCfgs, + nrc.bgpHoldtime, + nrc.krNode.GetPrimaryNodeIP().String(), + ) if err != nil { return nil, fmt.Errorf("error processing Global Peer Router configs: %s", err) } @@ -1478,8 +1464,11 @@ func NewNetworkRoutingController(clientset kubernetes.Interface, if nrc.krNode.IsIPv6Capable() { nrc.localAddressList = append(nrc.localAddressList, nrc.krNode.FindBestIPv6NodeAddress().String()) } - klog.Infof("Could not find annotation `kube-router.io/bgp-local-addresses` on node object so BGP "+ - "will listen on node IP: %s addresses.", nrc.localAddressList) + klog.Infof( + "Could not find annotation `kube-router.io/bgp-local-addresses` on node object so BGP "+ + "will listen on node IP: %s addresses.", + nrc.localAddressList, + ) } else { klog.Infof("Found annotation `kube-router.io/bgp-local-addresses` on node object so BGP will listen "+ "on local IP's: %s", bgpLocalAddressListAnnotation) @@ -1509,3 +1498,104 @@ func NewNetworkRoutingController(clientset kubernetes.Interface, return &nrc, nil } + +func bgpPeerConfigsFromAnnotations( + nodeAnnotations map[string]string, + localAddress string, +) (bgp.PeerConfigs, error) { + nodeBgpPeersAnnotation, ok := nodeAnnotations[peersAnnotation] + if !ok { + klog.Infof( + "%s annotation not set, using individual node annotations to configure BGP peer info", + peersAnnotation, + ) + return bgpPeerConfigsFromIndividualAnnotations(nodeAnnotations, localAddress) + } + + var peerConfigs bgp.PeerConfigs + if err := yaml.Unmarshal([]byte(nodeBgpPeersAnnotation), &peerConfigs); err != nil { + return nil, fmt.Errorf("failed to parse %s annotation: %w", peersAnnotation, err) + } + klog.Infof("Peer config from %s annotation: %+v", peersAnnotation, peerConfigs) + return peerConfigs, nil +} + +func bgpPeerConfigsFromIndividualAnnotations( + nodeAnnotations map[string]string, + localAddress string, +) (bgp.PeerConfigs, error) { + // Get Global Peer Router ASN configs + nodeBgpPeerAsnsAnnotation, ok := nodeAnnotations[peerASNAnnotation] + if !ok { + klog.Infof("Could not find BGP peer info for the node in the node annotations so " + + "skipping configuring peer.") + return nil, nil + } + asnStrings := stringToSlice(nodeBgpPeerAsnsAnnotation, ",") + peerASNs, err := stringSliceToUInt32(asnStrings) + if err != nil { + return nil, fmt.Errorf("failed to parse node's Peer ASN Numbers Annotation: %w", err) + } + + // Get Global Peer Router IP Address configs + nodeBgpPeersAnnotation, ok := nodeAnnotations[peerIPAnnotation] + if !ok { + klog.Infof("Could not find BGP peer info for the node in the node annotations " + + "so skipping configuring peer.") + return nil, nil + } + ipStrings := stringToSlice(nodeBgpPeersAnnotation, ",") + + // Get Global Peer Router ASN configs + ports := make([]uint32, 0) + nodeBgpPeerPortsAnnotation, ok := nodeAnnotations[peerPortAnnotation] + if ok { + portStrings := stringToSlice(nodeBgpPeerPortsAnnotation, ",") + ports, err = stringSliceToUInt32(portStrings) + if err != nil { + return nil, fmt.Errorf("failed to parse node's Peer Port Numbers Annotation: %w", err) + } + } + + var passwords []string + // Get Global Peer Router Password configs + nodeBGPPasswordsAnnotation, ok := nodeAnnotations[peerPasswordAnnotation] + if !ok { + klog.Infof( + "Could not find BGP peer password info in the node's annotations. Assuming no passwords.", + ) + } else { + passStrings := stringToSlice(nodeBGPPasswordsAnnotation, ",") + passwords, err = stringSliceB64Decode(passStrings) + if err != nil { + return nil, fmt.Errorf("failed to parse node's Peer Passwords Annotation: %w", err) + } + } + + // Get Global Peer Router LocalIP configs + var localIPs []string + nodeBGPPeerLocalIPs, ok := nodeAnnotations[peerLocalIPAnnotation] + if !ok { + klog.Infof( + "Could not find BGP peer local ip info in the node's annotations. Assuming node IP.", + ) + } else { + localIPs = stringToSlice(nodeBGPPeerLocalIPs, ",") + err = func() error { + for _, s := range localIPs { + if s != "" { + ip := net.ParseIP(s) + if ip == nil { + return fmt.Errorf("could not parse \"%s\" as an IP", s) + } + } + } + return nil + }() + if err != nil { + return nil, fmt.Errorf("failed to parse node's Peer Local Addresses Annotation: %w", err) + } + } + + return bgp.NewPeerConfigs(ipStrings, peerASNs, ports, passwords, localIPs, localAddress) +} diff --git a/pkg/controllers/routing/network_routes_controller_test.go b/pkg/controllers/routing/network_routes_controller_test.go index 66ac257567..90f904e82a 100644 --- a/pkg/controllers/routing/network_routes_controller_test.go +++ b/pkg/controllers/routing/network_routes_controller_test.go @@ -10,9 +10,15 @@ import ( "testing" "time" + "github.com/cloudnativelabs/kube-router/v2/internal/testutils" + "github.com/cloudnativelabs/kube-router/v2/pkg/bgp" "github.com/cloudnativelabs/kube-router/v2/pkg/k8s/indexers" "github.com/cloudnativelabs/kube-router/v2/pkg/utils" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" . "github.com/onsi/ginkgo" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" v1core "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -34,7 +40,6 @@ func Test_advertiseClusterIPs(t *testing.T) { // the key is the subnet from the watch event watchEvents map[string]bool }{ - { "add bgp path for service with ClusterIP", &NetworkRoutingController{ @@ -1818,11 +1823,11 @@ func Test_nodeHasEndpointsForService(t *testing.T) { Endpoints: []discoveryv1.Endpoint{ { Addresses: []string{"172.20.1.1"}, - NodeName: ptrToString("node-1"), + NodeName: testutils.ValToPtr("node-1"), }, { Addresses: []string{"172.20.1.2"}, - NodeName: ptrToString("node-2"), + NodeName: testutils.ValToPtr("node-2"), }, }, }, @@ -1863,11 +1868,11 @@ func Test_nodeHasEndpointsForService(t *testing.T) { Endpoints: []discoveryv1.Endpoint{ { Addresses: []string{"172.20.1.1"}, - NodeName: ptrToString("node-2"), + NodeName: testutils.ValToPtr("node-2"), }, { Addresses: []string{"172.20.1.2"}, - NodeName: ptrToString("node-3"), + NodeName: testutils.ValToPtr("node-3"), }, }, }, @@ -1902,7 +1907,6 @@ func Test_nodeHasEndpointsForService(t *testing.T) { t.Logf("actual nodeHasEndpoints: %v", nodeHasEndpoints) t.Error("unexpected nodeHasEndpoints") } - }) } } @@ -2618,7 +2622,222 @@ func Test_routeReflectorConfiguration(t *testing.T) { } }) } +} + +func Test_bgpPeerConfigsFromAnnotations(t *testing.T) { + testCases := []struct { + name string + nodeAnnotations map[string]string + expected bgp.PeerConfigs + errorContains string + }{ + { + name: "node annotations are empty", + nodeAnnotations: map[string]string{}, + }, + { + name: "combined bgp peers config annotation", + nodeAnnotations: map[string]string{ + peersAnnotation: `- remoteip: 10.0.0.1 + remoteasn: 64640 + password: cGFzc3dvcmQ= + localip: 192.168.0.1 +- remoteip: 10.0.0.2 + remoteasn: 64641 + password: cGFzc3dvcmQ= + localip: 192.168.0.2`, + }, + expected: func() bgp.PeerConfigs { + peer1, _ := bgp.NewPeerConfig("10.0.0.1", 64640, nil, "cGFzc3dvcmQ=", "192.168.0.1") + peer2, _ := bgp.NewPeerConfig("10.0.0.2", 64641, nil, "cGFzc3dvcmQ=", "192.168.0.2") + return bgp.PeerConfigs{peer1, peer2} + }(), + }, + { + name: "combined bgp peers config annotation with mix of local IPs, passwords, and ports being set", + nodeAnnotations: map[string]string{ + peersAnnotation: `- remoteip: 10.0.0.1 + remoteasn: 64640 + password: cGFzc3dvcmQ= + localip: 192.168.0.1 +- remoteip: 10.0.0.2 + remoteasn: 64641 + localip: 192.168.0.2 + port: 180 +- remoteip: 10.0.0.3 + password: cGFzc3dvcmQ= + remoteasn: 64642 + port: 181`, + }, + expected: func() bgp.PeerConfigs { + peer1, err := bgp.NewPeerConfig("10.0.0.1", 64640, nil, "cGFzc3dvcmQ=", "192.168.0.1") + if err != nil { + t.Fatalf("should not have gotten error constructing PeerConfig but got one: %s", err) + } + port2 := uint32(180) + peer2, err := bgp.NewPeerConfig("10.0.0.2", 64641, &port2, "", "192.168.0.2") + if err != nil { + t.Fatalf("should not have gotten error constructing PeerConfig but got one: %s", err) + } + port3 := uint32(181) + peer3, err := bgp.NewPeerConfig("10.0.0.3", 64642, &port3, "cGFzc3dvcmQ=", "") + if err != nil { + t.Fatalf("should not have gotten error constructing PeerConfig but got one: %s", err) + } + return bgp.PeerConfigs{peer1, peer2, peer3} + }(), + }, + { + name: "combined bgp peers config annotation with one peer missing remote ip returns error", + nodeAnnotations: map[string]string{ + peersAnnotation: `- remoteip: 10.0.0.1 + remoteasn: 64640 + port: 180 +- remoteasn: 64641 + password: cGFzc3dvcmQ= + localip: 192.168.0.2`, + }, + errorContains: "remoteip cannot be empty", + }, + { + name: "combined bgp peers config annotation with one peer missing remote asn returns error", + nodeAnnotations: map[string]string{ + peersAnnotation: `- remoteip: 10.0.0.1 + remoteasn: 64640 + port: 180 +- remoteip: 10.0.0.2 + password: cGFzc3dvcmQ= + localip: 192.168.0.2`, + }, + errorContains: "remoteasn cannot be empty", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(st *testing.T) { + bgpPeerCfgs, err := bgpPeerConfigsFromAnnotations(tc.nodeAnnotations, "") + if tc.errorContains != "" { + assert.ErrorContains(st, err, tc.errorContains) + } else { + require.NoError(t, err) + if !cmp.Equal(tc.expected, bgpPeerCfgs, cmpopts.IgnoreUnexported(bgp.PeerConfig{})) { + diff := cmp.Diff(tc.expected, bgpPeerCfgs, cmpopts.IgnoreUnexported(bgp.PeerConfig{})) + t.Errorf("BGP peer config mismatch:\n%s", diff) + } + } + }) + } +} + +func Test_bgpPeerConfigsFromIndividualAnnotations(t *testing.T) { + testCases := []struct { + name string + nodeAnnotations map[string]string + expected bgp.PeerConfigs + errorContains string + }{ + { + name: "individual bgp peers config annotations", + nodeAnnotations: map[string]string{ + peerIPAnnotation: "10.0.0.1,10.0.0.2", + peerASNAnnotation: "64640,64641", + peerPasswordAnnotation: "cGFzc3dvcmQ=,cGFzc3dvcmQ=", + peerLocalIPAnnotation: "192.168.0.1,192.168.0.2", + peerPortAnnotation: "180,181", + }, + expected: func() bgp.PeerConfigs { + port1 := uint32(180) + port2 := uint32(181) + peer1, _ := bgp.NewPeerConfig("10.0.0.1", 64640, &port1, "cGFzc3dvcmQ=", "192.168.0.1") + peer2, _ := bgp.NewPeerConfig("10.0.0.2", 64641, &port2, "cGFzc3dvcmQ=", "192.168.0.2") + return bgp.PeerConfigs{peer1, peer2} + }(), + }, + { + name: "individual bgp peers config annotations with some blank passwords", + nodeAnnotations: map[string]string{ + peerIPAnnotation: "10.0.0.1,10.0.0.2", + peerASNAnnotation: "64640,64641", + peerPasswordAnnotation: ",cGFzc3dvcmQ=", + peerLocalIPAnnotation: "192.168.0.1,192.168.0.2", + peerPortAnnotation: "180,181", + }, + expected: func() bgp.PeerConfigs { + port1 := uint32(180) + port2 := uint32(181) + peer1, _ := bgp.NewPeerConfig("10.0.0.1", 64640, &port1, "", "192.168.0.1") + peer2, _ := bgp.NewPeerConfig("10.0.0.2", 64641, &port2, "cGFzc3dvcmQ=", "192.168.0.2") + return bgp.PeerConfigs{peer1, peer2} + }(), + }, + { + name: "individual bgp peers config annotations without matching number of passwords returns error", + nodeAnnotations: map[string]string{ + peerIPAnnotation: "10.0.0.1,10.0.0.2", + peerASNAnnotation: "64640,64641", + peerPasswordAnnotation: "cGFzc3dvcmQ=", + peerLocalIPAnnotation: "192.168.0.2,", + }, + errorContains: "The number of passwords should either be zero, or one per peer router.", + }, + { + name: "individual bgp peers config annotations without matching number of ports returns error", + nodeAnnotations: map[string]string{ + peerIPAnnotation: "10.0.0.1,10.0.0.2", + peerASNAnnotation: "64640,64641", + peerPasswordAnnotation: "cGFzc3dvcmQ=,cGFzc3dvcmQ=", + peerLocalIPAnnotation: "192.168.0.1,192.168.0.2", + peerPortAnnotation: "180", + }, + errorContains: "The number of ports should either be zero, or one per peer router", + }, + { + name: "individual bgp peers config annotations without matching number of local IPs returns error", + nodeAnnotations: map[string]string{ + peerIPAnnotation: "10.0.0.1,10.0.0.2", + peerASNAnnotation: "64640,64641", + peerPasswordAnnotation: "cGFzc3dvcmQ=,cGFzc3dvcmQ=", + peerLocalIPAnnotation: "192.168.0.2", + peerPortAnnotation: "180,181", + }, + errorContains: "The number of localIPs should either be zero, or one per peer router", + }, + { + name: "individual bgp peers config annotations without matching number of peer IPs returns error", + nodeAnnotations: map[string]string{ + peerIPAnnotation: "10.0.0.1", + peerASNAnnotation: "64640,64641", + peerPasswordAnnotation: "cGFzc3dvcmQ=,cGFzc3dvcmQ=", + peerLocalIPAnnotation: "192.168.0.1,192.168.0.2", + }, + errorContains: "the number of IPs and ASN numbers must be equal", + }, + { + name: "individual bgp peers config annotations without matching number of peer ASNs returns error", + nodeAnnotations: map[string]string{ + peerIPAnnotation: "10.0.0.1,10.0.0.2", + peerASNAnnotation: "64640", + peerPasswordAnnotation: "cGFzc3dvcmQ=,cGFzc3dvcmQ=", + peerLocalIPAnnotation: "192.168.0.1,192.168.0.2", + }, + errorContains: "the number of IPs and ASN numbers must be equal", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(st *testing.T) { + bgpPeerCfgs, err := bgpPeerConfigsFromAnnotations(tc.nodeAnnotations, "") + if tc.errorContains != "" { + assert.ErrorContains(st, err, tc.errorContains) + } else { + require.NoError(t, err) + if !cmp.Equal(tc.expected, bgpPeerCfgs, cmpopts.IgnoreUnexported(bgp.PeerConfig{})) { + diff := cmp.Diff(tc.expected, bgpPeerCfgs, cmpopts.IgnoreUnexported(bgp.PeerConfig{})) + t.Errorf("BGP peer config mismatch:\n%s", diff) + } + } + }) + } } /* Disabling test for now. OnNodeUpdate() behaviour is changed. test needs to be adopted. @@ -2865,7 +3084,3 @@ func waitForListerWithTimeout(lister cache.Indexer, timeout time.Duration, t *te } } } - -func ptrToString(str string) *string { - return &str -} diff --git a/pkg/utils/base64.go b/pkg/utils/base64.go new file mode 100644 index 0000000000..5b1a869c29 --- /dev/null +++ b/pkg/utils/base64.go @@ -0,0 +1,25 @@ +package utils + +import ( + "encoding/base64" + "fmt" + + "github.com/goccy/go-yaml" +) + +// Base64String is a wrapper type that handles automatic b64 decoding of a +// string when unmarshalling +type Base64String string + +func (b *Base64String) UnmarshalYAML(raw []byte) error { + var tmp string + if err := yaml.Unmarshal(raw, &tmp); err != nil { + return fmt.Errorf("failed to unmarshal string into base64string type: %w", err) + } + decoded, err := base64.StdEncoding.DecodeString(tmp) + if err != nil { + return fmt.Errorf("failed to base64 decode field: %w", err) + } + *b = Base64String(string(decoded)) + return nil +} diff --git a/pkg/utils/base64_test.go b/pkg/utils/base64_test.go new file mode 100644 index 0000000000..b4b3843103 --- /dev/null +++ b/pkg/utils/base64_test.go @@ -0,0 +1,48 @@ +package utils + +import ( + "fmt" + "testing" + + "github.com/goccy/go-yaml" + "github.com/stretchr/testify/assert" +) + +func TestBase64String(t *testing.T) { + type testStruct struct { + Password Base64String `yaml:"password"` + } + + tcs := []struct { + name string + input []byte + shouldError bool + errorContains string + }{ + { + name: "happy path", + // b64: hello world + input: []byte(`password: "aGVsbG8gd29ybGQ="`), + }, + { + name: "invalid base64 encoding", + input: []byte(`password: "notbase64"`), + shouldError: true, + errorContains: "failed to base64 decode", + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(tt *testing.T) { + var ts testStruct + err := yaml.Unmarshal(tc.input, &ts) + fmt.Printf("TS: %+v\n", ts) + if tc.shouldError { + assert.ErrorContains(tt, err, tc.errorContains) + } else { + assert.NoError(tt, err) + assert.NotEmpty(tt, ts.Password) + } + }) + } +}