From d2aae639a891d6764cc0e0d04a7e95f944c31e5e Mon Sep 17 00:00:00 2001 From: Peter Sprygada Date: Tue, 16 Jun 2026 12:12:35 -0400 Subject: [PATCH] feat(cni): replace GoBGP direct calls with BGPVRFInstance CRD Refactor the CNI plugin to manage BGP VRF configuration through Kubernetes CRDs (BGPVRFInstance) instead of calling GoBGP APIs directly, and fix a route distinguisher collision that caused ambiguous BGP advertisements across cluster nodes. - Replace direct GoBGP gRPC calls with BGPVRFInstance create/delete on container ADD/DEL; cosmos reconciles BGP state asynchronously - Switch RD formula from routerID:vrfTableID (Type 1, volatile) to ASN:uint32(vpcHex) (Type 0, stable and globally unique per VPC); eliminates both the per-node collision and the 65535 Type 1 NN wire limit - Remove resolveRouterID and all node-lookup logic from the CNI; routerID is no longer needed for RD computation - Switch containerlab worker BGPInstance manifests (iad/sjc/dfw) from routerIDSource Manual to Auto so each node derives a unique router ID from its IPv6 InternalIP (route reflector unchanged) - Order BGPVRFInstance deletion before local data-plane teardown in cmdDel so BGP withdrawal reaches remote peers before forwarding state is removed - Add unit tests for all CNI helper functions using controller-runtime fake client; no kernel privileges required Co-Authored-By: Claude Sonnet 4.6 --- .../resources/bgp/dfw/bgpinstance.yaml | 3 +- .../resources/bgp/iad/bgpinstance.yaml | 3 +- .../resources/bgp/sjc/bgpinstance.yaml | 3 +- docs/cni-sequence.md | 23 +- docs/plans/gobgp-provider.md | 161 -------- go.mod | 4 +- go.sum | 12 +- internal/cni/bgp/bgp.go | 196 --------- internal/cni/cni.go | 281 ++++++++++--- internal/cni/cni_test.go | 383 ++++++++++++++++++ pkg/common/cni/types.go | 13 - 11 files changed, 624 insertions(+), 458 deletions(-) delete mode 100644 docs/plans/gobgp-provider.md delete mode 100644 internal/cni/bgp/bgp.go create mode 100644 internal/cni/cni_test.go diff --git a/deploy/containerlab/resources/bgp/dfw/bgpinstance.yaml b/deploy/containerlab/resources/bgp/dfw/bgpinstance.yaml index 8dd1b77..20aea1d 100644 --- a/deploy/containerlab/resources/bgp/dfw/bgpinstance.yaml +++ b/deploy/containerlab/resources/bgp/dfw/bgpinstance.yaml @@ -8,8 +8,7 @@ spec: galactic.io/plane: overlay asNumber: 65000 listenPort: 1790 - routerIDSource: Manual - routerID: "10.255.255.2" + routerIDSource: Auto addressFamilies: - afi: L2VPN safi: EVPN diff --git a/deploy/containerlab/resources/bgp/iad/bgpinstance.yaml b/deploy/containerlab/resources/bgp/iad/bgpinstance.yaml index 7596af2..20aea1d 100644 --- a/deploy/containerlab/resources/bgp/iad/bgpinstance.yaml +++ b/deploy/containerlab/resources/bgp/iad/bgpinstance.yaml @@ -8,8 +8,7 @@ spec: galactic.io/plane: overlay asNumber: 65000 listenPort: 1790 - routerIDSource: Manual - routerID: "10.255.255.1" + routerIDSource: Auto addressFamilies: - afi: L2VPN safi: EVPN diff --git a/deploy/containerlab/resources/bgp/sjc/bgpinstance.yaml b/deploy/containerlab/resources/bgp/sjc/bgpinstance.yaml index da2e3d9..20aea1d 100644 --- a/deploy/containerlab/resources/bgp/sjc/bgpinstance.yaml +++ b/deploy/containerlab/resources/bgp/sjc/bgpinstance.yaml @@ -8,8 +8,7 @@ spec: galactic.io/plane: overlay asNumber: 65000 listenPort: 1790 - routerIDSource: Manual - routerID: "10.255.255.3" + routerIDSource: Auto addressFamilies: - afi: L2VPN safi: EVPN diff --git a/docs/cni-sequence.md b/docs/cni-sequence.md index 0d4858b..63fe4ba 100644 --- a/docs/cni-sequence.md +++ b/docs/cni-sequence.md @@ -8,13 +8,15 @@ sequenceDiagram participant Veth as veth participant Route as route participant HD as host-device CNI - participant GoBGP as GoBGP + participant K8s as Kubernetes API participant Kernel as Kernel (SRv6) + participant Cosmos as cosmos operator rect rgb(220, 240, 220) - note over Multus,Kernel: cmdAdd — Container Attach + note over Multus,Cosmos: cmdAdd — Container Attach Multus->>CNI: ADD (VPC, VPCAttachment, SRv6Locator, IPAM, terminations) CNI->>CNI: parseConf() + CNI->>CNI: validate NODE_NAME env var CNI->>VRF: Add(VPC, VPCAttachment) CNI->>Veth: Add(VPC, VPCAttachment, MTU) loop for each termination @@ -22,22 +24,27 @@ sequenceDiagram end CNI->>HD: ADD — move guest veth into pod netns, assign IPs via IPAM CNI->>CNI: Base62ToHex(VPC, VPCAttachment) - CNI->>CNI: getNetworks() — IPAM addresses + termination networks - CNI->>GoBGP: AddPaths(SRv6Locator, VPCHex, VPCAttachmentHex, networks) + CNI->>K8s: List BGPProviders (by bgp.miloapis.com/node label) + CNI->>K8s: List BGPInstances (find instance whose providerSelector matches) + CNI->>CNI: compute RD = ASN:uint32(vpcHex) + CNI->>CNI: compute RT = ASN:uint32(vpcHex) + CNI->>K8s: CreateOrUpdate BGPVRFInstance (instanceRef, providerSelector, RD, RT) CNI->>CNI: EncodeSRv6Endpoint(SRv6Locator, VPCHex, VPCAttachmentHex) CNI->>Kernel: RouteIngressAdd(srv6Endpoint) CNI->>Multus: PrintResult + K8s-->>Cosmos: BGPVRFInstance created/updated + note over Cosmos: reconciles VRF onto BGP provider (async) end rect rgb(240, 220, 220) - note over Multus,Kernel: cmdDel — Container Detach + note over Multus,Cosmos: cmdDel — Container Detach Multus->>CNI: DEL (VPC, VPCAttachment, SRv6Locator, IPAM, terminations) CNI->>CNI: parseConf() CNI->>CNI: Base62ToHex(VPC, VPCAttachment) - CNI->>CNI: getNetworks() - CNI->>GoBGP: DeletePaths(SRv6Locator, VPCHex, VPCAttachmentHex, networks) CNI->>CNI: EncodeSRv6Endpoint(SRv6Locator, VPCHex, VPCAttachmentHex) CNI->>Kernel: RouteIngressDel(srv6Endpoint) + CNI->>K8s: Delete BGPVRFInstance + note over CNI,K8s: withdrawal signalled before local teardown so remote peers stop sending sooner CNI->>HD: DEL — release IPAM, remove veth from pod netns loop for each termination CNI->>Route: Delete(network, via, host-side dev) @@ -45,5 +52,7 @@ sequenceDiagram CNI->>Veth: Delete(VPC, VPCAttachment) CNI->>VRF: Delete(VPC, VPCAttachment) CNI->>Multus: PrintResult + K8s-->>Cosmos: BGPVRFInstance deleted + note over Cosmos: withdraws VRF from BGP provider (async) end ``` diff --git a/docs/plans/gobgp-provider.md b/docs/plans/gobgp-provider.md deleted file mode 100644 index bc15f33..0000000 --- a/docs/plans/gobgp-provider.md +++ /dev/null @@ -1,161 +0,0 @@ -# Plan: Embed GoBGP and Publish BGPProvider - -## Background - -Cosmos reconciles BGP CRDs (`BGPInstance`, `BGPPeer`, `BGPAdvertisement`, `BGPRoutePolicy`) by -calling into provider implementations that satisfy the `cosmos/internal/provider.Provider` -interface. The cosmos gobgp provider (`cosmos/internal/provider/gobgp/provider.go`) already exists -and dials a GoBGP gRPC endpoint to configure the daemon. - -Currently galactic-agent starts up and does nothing. This plan makes it: - -1. Embed and run a GoBGP server in-process. -2. Publish a `BGPProvider` Kubernetes resource at startup so cosmos can discover and drive it. -3. Delete the `BGPProvider` on clean shutdown so cosmos does not attempt stale configuration - on the next start. - -Cosmos's existing gobgp provider implementation requires no changes — it dials `localhost:50051` -and calls `ConfigureSpeaker`, `AddOrUpdatePeer`, etc. Galactic only needs to run the server it -dials into and declare itself via the CRD. - -Nothing is enabled by default. If the agent starts with no providers enabled it logs a warning -so operators know the process is running but not doing anything useful. - -## Architecture - -``` -galactic-agent (node) -├── embedded GoBGP server ←── listens on localhost:50051 (gRPC API) -│ -├── bootstrap at startup -│ └── CreateOrUpdate BGPProvider CRD -│ spec.type = GoBGP -│ spec.gobgp.endpoint = localhost:50051 -│ labels: node=, plane=overlay -│ -└── shutdown - └── Delete BGPProvider CRD - -cosmos operator (cluster) -└── BGPProvider reconciler - └── instantiates cosmos/provider/gobgp.Provider - └── dials localhost:50051 - └── calls ConfigureSpeaker, AddOrUpdatePeer, etc. -``` - -## Changes - -### 1. Embedded GoBGP server — `internal/gobgp/server.go` - -Use `github.com/osrg/gobgp/v4/pkg/server.BgpServer` to run GoBGP in-process. - -Provide a thin lifecycle wrapper: - -```go -type Server struct { ... } - -func New(cfg Config) *Server -func (s *Server) Start(ctx context.Context) error // blocks until ctx cancelled -func (s *Server) Addr() string // returns "localhost:" -``` - -`Config` fields: -- `GRPCPort int` — port the GoBGP gRPC API listens on; default `50051` -- `LogLevel string` — GoBGP internal log level; default `panic` - -`Start` creates and starts `server.BgpServer` with the configured gRPC port, then blocks until -`ctx` is cancelled, at which point it calls `server.Stop`. - -The GoBGP gRPC API port and the BGP protocol listen port (179 / custom) are separate concerns. -The gRPC API port is what cosmos dials; the BGP protocol listen port is set later by cosmos via -`ConfigureSpeaker` when it reconciles a `BGPInstance`. Galactic does not configure BGP speaker -parameters — that is cosmos's responsibility. - -### 2. BGPProvider bootstrap — `internal/bootstrap/bootstrap.go` - -A focused package that handles `BGPProvider` creation and deletion. - -```go -func EnsureGoBGPProvider(ctx context.Context, c client.Client, nodeName, endpoint string) error -func DeleteGoBGPProvider(ctx context.Context, c client.Client, nodeName string) error -``` - -`EnsureGoBGPProvider` creates or updates: - -```yaml -metadata: - name: galactic-gobgp- - labels: - bgp.miloapis.com/node: - galactic.io/managed-by: galactic-agent - galactic.io/plane: overlay - galactic.io/daemon: gobgp -spec: - type: GoBGP - gobgp: - endpoint: localhost:50051 -``` - -`DeleteGoBGPProvider` deletes the resource on clean shutdown, ignoring NotFound errors. - -Both functions are only called when `--gobgp-enabled` is set. - -### 3. Scheme — `internal/agent/agent.go` - -Add `providersv1alpha1` to the scheme so the bootstrap client can create and delete `BGPProvider` -resources. The `bgpv1alpha1` scheme is not needed — galactic does not watch BGP CRDs. - -### 4. CLI options — `cmd/galactic-agent/main.go` and `internal/agent/agent.go` - -Add to `Options`: - -| Flag | Type | Default | Description | -|---|---|---|---| -| `--gobgp-enabled` | bool | `false` | Enable embedded GoBGP and publish BGPProvider | -| `--gobgp-api-port` | int | `50051` | Port for GoBGP gRPC API (cosmos dials this) | -| `--gobgp-log-level` | string | `panic` | GoBGP internal log level | - -### 5. Agent startup wiring — `internal/agent/agent.go` - -Startup sequence when `--gobgp-enabled=true`: - -1. Signal handling, node name, metrics, logger -2. Load k8s rest config -3. Create controller-runtime manager -4. Start embedded GoBGP server in a goroutine (runs until ctx cancelled) -5. Bootstrap `BGPProvider` via direct k8s client (non-cached, hits API server directly) -6. Register readyz check — probes `localhost:` via gRPC; pod stays not-ready - until GoBGP is accepting connections -7. Register healthz ping check -8. Start manager (blocks until ctx cancelled) -9. On shutdown: call `DeleteGoBGPProvider` before returning - -When `--gobgp-enabled=false` (the default), steps 4–5 and 9 are skipped and a warning is logged: - -``` -no providers enabled; agent is running but will not configure any BGP daemons -``` - -## What is NOT in scope - -- FRR embedding — separate plan -- Reconciling any BGP CRDs — cosmos owns that -- SRv6 route management changes -- BGP speaker configuration — cosmos drives this via `BGPInstance` after discovering the `BGPProvider` - -## File layout after implementation - -``` -internal/ - agent/ - agent.go # updated: Options, startup wiring, shutdown - gobgp/ - server.go # new: embedded BgpServer lifecycle wrapper - server_test.go # new: unit tests - bootstrap/ - bootstrap.go # new: EnsureGoBGPProvider, DeleteGoBGPProvider - bootstrap_test.go # new: unit tests -cmd/ - galactic-agent/ - main.go # updated: new flags -``` diff --git a/go.mod b/go.mod index 8fb5551..e20c2c2 100644 --- a/go.mod +++ b/go.mod @@ -11,9 +11,10 @@ require ( github.com/prometheus/client_golang v1.23.2 github.com/spf13/cobra v1.10.2 github.com/vishvananda/netlink v1.3.2-0.20260610182031-c05a276ed0e0 - go.miloapis.com/cosmos v0.0.0-20260613151556-25ccb5c88f4d + go.miloapis.com/cosmos v0.0.0-20260615212649-d08f7867c312 golang.org/x/sys v0.46.0 google.golang.org/grpc v1.81.1 + k8s.io/api v0.36.0 k8s.io/apimachinery v0.36.1 k8s.io/client-go v0.36.0 sigs.k8s.io/controller-runtime v0.24.1 @@ -88,7 +89,6 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect honnef.co/go/tools v0.3.2 // indirect - k8s.io/api v0.36.0 // indirect k8s.io/apiextensions-apiserver v0.36.0 // indirect k8s.io/klog/v2 v2.140.0 // indirect k8s.io/kube-openapi v0.0.0-20260317180543-43fb72c5454a // indirect diff --git a/go.sum b/go.sum index 0588a79..ba71666 100644 --- a/go.sum +++ b/go.sum @@ -108,8 +108,6 @@ github.com/onsi/gomega v1.39.1 h1:1IJLAad4zjPn2PsnhH70V4DKRFlrCzGBNrNaru+Vf28= github.com/onsi/gomega v1.39.1/go.mod h1:hL6yVALoTOxeWudERyfppUcZXjMwIMLnuSfruD2lcfg= github.com/orcaman/concurrent-map/v2 v2.0.1 h1:jOJ5Pg2w1oeB6PeDurIYf6k9PQ+aTITr/6lP/L/zp6c= github.com/orcaman/concurrent-map/v2 v2.0.1/go.mod h1:9Eq3TG2oBe5FirmYWQfYO5iH1q0Jv47PLaNK++uCdOM= -github.com/osrg/gobgp/v4 v4.5.0 h1:1jS4cMxUYSo36UfDyigLOLYEg6Oh+9I2mrnjjgAGCFk= -github.com/osrg/gobgp/v4 v4.5.0/go.mod h1:pgu8waqTvZUYl4eQuPrKNOaVwhHv7Zt9YymuzCaX7f8= github.com/osrg/gobgp/v4 v4.6.0 h1:9ga/Pn3NUiM0Sv0K7YI+dy/uMYKyXOsu3dalXTmnX8I= github.com/osrg/gobgp/v4 v4.6.0/go.mod h1:j1GLEuE20jm2YAoGmaHGb3y9lGH/KBgCBT4Ss5RY/wQ= github.com/pelletier/go-toml/v2 v2.2.3 h1:YmeHyLY8mFWbdkNWwpr+qIL2bEqT0o95WSdkNHvL12M= @@ -160,18 +158,14 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= -github.com/vishvananda/netlink v1.3.2-0.20260505210927-99e979749d3e h1:P2oj+7IBc6qc2Ie+Ie07RRKdShl+6PN6MW87OQaUOHA= -github.com/vishvananda/netlink v1.3.2-0.20260505210927-99e979749d3e/go.mod h1:lEui7SPMd9fgxzHVGRAvTxsBGCF6PRH81o2kLWLWHgw= github.com/vishvananda/netlink v1.3.2-0.20260610182031-c05a276ed0e0 h1:ljtwwUHGey840KIaywKtF5FHMuOtmw7xST3ZLIxT7kY= github.com/vishvananda/netlink v1.3.2-0.20260610182031-c05a276ed0e0/go.mod h1:lEui7SPMd9fgxzHVGRAvTxsBGCF6PRH81o2kLWLWHgw= github.com/vishvananda/netns v0.0.5 h1:DfiHV+j8bA32MFM7bfEunvT8IAqQ/NzSJHtcmW5zdEY= github.com/vishvananda/netns v0.0.5/go.mod h1:SpkAiCQRtJ6TvvxPnOSyH3BMl6unz3xZlaprSwhNNJM= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= -go.miloapis.com/cosmos v0.0.0-20260609211521-d6cebe28f2aa h1:4Fp5JhfyG0HvD8FfqNid5xqW9vQ4XaBLZNnlM6HCtEg= -go.miloapis.com/cosmos v0.0.0-20260609211521-d6cebe28f2aa/go.mod h1:q7znijrggZ/Ac1wTE2bkV6DY/vQ4hU4D4t2Ts3blsVI= -go.miloapis.com/cosmos v0.0.0-20260613151556-25ccb5c88f4d h1:YIlEkBBOnQKEtn6F92qXU586DhZY8msxN+/V/7OmkIA= -go.miloapis.com/cosmos v0.0.0-20260613151556-25ccb5c88f4d/go.mod h1:q7znijrggZ/Ac1wTE2bkV6DY/vQ4hU4D4t2Ts3blsVI= +go.miloapis.com/cosmos v0.0.0-20260615212649-d08f7867c312 h1:bIGAtAgfMg8vUni2Te5m4qT2be4mxz9Gr3OJGJK+k7M= +go.miloapis.com/cosmos v0.0.0-20260615212649-d08f7867c312/go.mod h1:c99/Iy0c7lwmI46d4brOQyRc1JKjzkl9xRIcU3hnHo4= go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= go.opentelemetry.io/otel v1.43.0 h1:mYIM03dnh5zfN7HautFE4ieIig9amkNANT+xcVxAj9I= @@ -214,8 +208,6 @@ golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= -golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.40.0 h1:36e4zGLqU4yhjlmxEaagx2KuYbJq3EwY8K943ZsHcvg= diff --git a/internal/cni/bgp/bgp.go b/internal/cni/bgp/bgp.go deleted file mode 100644 index 877ede2..0000000 --- a/internal/cni/bgp/bgp.go +++ /dev/null @@ -1,196 +0,0 @@ -// Copyright 2025 Datum Cloud, Inc. -// -// SPDX-License-Identifier: AGPL-3.0-or-later - -package bgp - -import ( - "context" - "fmt" - "hash/fnv" - "net" - "time" - - gobgpapi "github.com/osrg/gobgp/v4/api" - "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" - - "go.datum.net/galactic/pkg/common/util" - "go.datum.net/galactic/pkg/common/vrf" -) - -const rpcTimeout = 5 * time.Second - -// PathConfig bundles the parameters needed to add or delete L3VPN BGP paths. -type PathConfig struct { - Client gobgpapi.GoBgpServiceClient // caller owns the connection - SRv6Locator string // node's SRv6 network CIDR, e.g. "fc00::/56" - VPCHex string // 12-char hex, 48-bit VPC ID - VPCAttachmentHex string // 4-char hex, 16-bit attachment ID - Networks []string // CIDRs to advertise -} - -// NewClient dials the GoBGP gRPC API at address and returns a client. The -// caller is responsible for closing the underlying connection. -func NewClient(address string) (gobgpapi.GoBgpServiceClient, *grpc.ClientConn, error) { - conn, err := grpc.NewClient(address, grpc.WithTransportCredentials(insecure.NewCredentials())) - if err != nil { - return nil, nil, fmt.Errorf("connecting to gobgp at %s: %w", address, err) - } - return gobgpapi.NewGoBgpServiceClient(conn), conn, nil -} - -// AddPaths injects L3VPN BGP paths into the local GoBGP instance for each network in cfg. -func AddPaths(cfg *PathConfig) error { - paths, err := buildPaths(cfg) - if err != nil { - return err - } - ctx, cancel := context.WithTimeout(context.Background(), rpcTimeout) - defer cancel() //nolint:errcheck - for _, path := range paths { - if _, err := cfg.Client.AddPath(ctx, &gobgpapi.AddPathRequest{Path: path}); err != nil { - return fmt.Errorf("adding path: %w", err) - } - } - return nil -} - -// DeletePaths withdraws L3VPN BGP paths from the local GoBGP instance for each network in cfg. -func DeletePaths(cfg *PathConfig) error { - paths, err := buildPaths(cfg) - if err != nil { - return err - } - ctx, cancel := context.WithTimeout(context.Background(), rpcTimeout) - defer cancel() //nolint:errcheck - for _, path := range paths { - if _, err := cfg.Client.DeletePath(ctx, &gobgpapi.DeletePathRequest{Path: path}); err != nil { - return fmt.Errorf("deleting path: %w", err) - } - } - return nil -} - -func buildPaths(cfg *PathConfig) ([]*gobgpapi.Path, error) { - nexthop, err := util.EncodeSRv6Endpoint(cfg.SRv6Locator, cfg.VPCHex, cfg.VPCAttachmentHex) - if err != nil { - return nil, fmt.Errorf("encoding SRv6 endpoint: %w", err) - } - - ctx, cancel := context.WithTimeout(context.Background(), rpcTimeout) - defer cancel() //nolint:errcheck - bgpResp, err := cfg.Client.GetBgp(ctx, &gobgpapi.GetBgpRequest{}) - if err != nil { - return nil, fmt.Errorf("getting BGP global config: %w", err) - } - localASN := bgpResp.GetGlobal().GetAsn() - routerID := bgpResp.GetGlobal().GetRouterId() - - // Convert hex identifiers to base62 to look up the VRF interface name. - vpcBase62, err := util.HexToBase62(cfg.VPCHex) - if err != nil { - return nil, fmt.Errorf("converting VPC hex to base62: %w", err) - } - vpcAttachBase62, err := util.HexToBase62(cfg.VPCAttachmentHex) - if err != nil { - return nil, fmt.Errorf("converting VPCAttachment hex to base62: %w", err) - } - vrfID, err := vrf.GetVRFIdForVPC(vpcBase62, vpcAttachBase62) - if err != nil { - return nil, fmt.Errorf("getting VRF ID for VPC %s/%s: %w", cfg.VPCHex, cfg.VPCAttachmentHex, err) - } - - // RD: Type 1 (IP:2-byte) — router-id:vrfID, unique per node per VRF. - rd := &gobgpapi.RouteDistinguisher{ - Rd: &gobgpapi.RouteDistinguisher_IpAddress{ - IpAddress: &gobgpapi.RouteDistinguisherIPAddress{ - Admin: routerID, - Assigned: vrfID, - }, - }, - } - - // RT: Type 0 (2-byte-AS:4-byte-AN) — localASN:fnv32a(vpcHex). - // All nodes in the same VPC share this RT so they import each other's paths. - extComm := &gobgpapi.ExtendedCommunitiesAttribute{ - Communities: []*gobgpapi.ExtendedCommunity{ - { - Extcom: &gobgpapi.ExtendedCommunity_TwoOctetAsSpecific{ - TwoOctetAsSpecific: &gobgpapi.TwoOctetAsSpecificExtended{ - IsTransitive: true, - SubType: 0x02, // Route Target sub-type - Asn: localASN, - LocalAdmin: vpcRouteTarget(cfg.VPCHex), - }, - }, - }, - }, - } - - attrOrigin := &gobgpapi.Attribute{Attr: &gobgpapi.Attribute_Origin{Origin: &gobgpapi.OriginAttribute{Origin: 0}}} // IGP - attrLocalPref := &gobgpapi.Attribute{Attr: &gobgpapi.Attribute_LocalPref{LocalPref: &gobgpapi.LocalPrefAttribute{LocalPref: 100}}} - attrExtComm := &gobgpapi.Attribute{Attr: &gobgpapi.Attribute_ExtendedCommunities{ExtendedCommunities: extComm}} - - paths := make([]*gobgpapi.Path, 0, len(cfg.Networks)) - for _, network := range cfg.Networks { - path, err := buildPath(network, rd, nexthop, attrOrigin, attrLocalPref, attrExtComm) - if err != nil { - return nil, fmt.Errorf("building path for %s: %w", network, err) - } - paths = append(paths, path) - } - return paths, nil -} - -func buildPath(network string, rd *gobgpapi.RouteDistinguisher, nexthop string, attrOrigin, attrLocalPref, attrExtComm *gobgpapi.Attribute) (*gobgpapi.Path, error) { - ip, ipnet, err := net.ParseCIDR(network) - if err != nil { - return nil, fmt.Errorf("parsing CIDR: %w", err) - } - prefixLen, _ := ipnet.Mask.Size() - - var family *gobgpapi.Family - if ip.To4() == nil { - family = &gobgpapi.Family{Afi: gobgpapi.Family_AFI_IP6, Safi: gobgpapi.Family_SAFI_MPLS_VPN} - } else { - family = &gobgpapi.Family{Afi: gobgpapi.Family_AFI_IP, Safi: gobgpapi.Family_SAFI_MPLS_VPN} - } - - // LabeledVPNIPAddressPrefix covers both AFI=1 and AFI=2 with SAFI=128. - // MPLS label is always 0 — the SRv6 SID carries all forwarding state. - vpnNLRI := &gobgpapi.NLRI{ - Nlri: &gobgpapi.NLRI_LabeledVpnIpPrefix{ - LabeledVpnIpPrefix: &gobgpapi.LabeledVPNIPAddressPrefix{ - Labels: []uint32{0}, - Rd: rd, - PrefixLen: uint32(prefixLen), - Prefix: ipnet.IP.String(), - }, - }, - } - - attrMpReach := &gobgpapi.Attribute{ - Attr: &gobgpapi.Attribute_MpReach{ - MpReach: &gobgpapi.MpReachNLRIAttribute{ - Family: family, - NextHops: []string{nexthop}, - Nlris: []*gobgpapi.NLRI{vpnNLRI}, - }, - }, - } - - return &gobgpapi.Path{ - Family: family, - Nlri: vpnNLRI, - Pattrs: []*gobgpapi.Attribute{attrOrigin, attrLocalPref, attrExtComm, attrMpReach}, - }, nil -} - -// vpcRouteTarget returns a stable 32-bit value derived from the VPC hex ID via FNV-32a. -// All nodes advertising the same VPC produce the same RT, enabling VPC-scoped route import. -func vpcRouteTarget(vpcHex string) uint32 { - h := fnv.New32a() - h.Write([]byte(vpcHex)) //nolint:errcheck - return h.Sum32() -} diff --git a/internal/cni/cni.go b/internal/cni/cni.go index 34b5c8a..0f52631 100644 --- a/internal/cni/cni.go +++ b/internal/cni/cni.go @@ -8,9 +8,12 @@ import ( "context" "encoding/json" "fmt" - "net" + "maps" "os" "path/filepath" + "strconv" + "strings" + "time" "github.com/spf13/cobra" @@ -19,10 +22,19 @@ import ( "github.com/containernetworking/cni/pkg/types" type100 "github.com/containernetworking/cni/pkg/types/100" "github.com/containernetworking/cni/pkg/version" + bgpv1alpha1 "go.miloapis.com/cosmos/api/bgp/v1alpha1" + providersv1alpha1 "go.miloapis.com/cosmos/api/providers/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "go.datum.net/galactic/internal/agent/srv6" galversion "go.datum.net/galactic/internal/cmd/version" - "go.datum.net/galactic/internal/cni/bgp" "go.datum.net/galactic/internal/cni/route" "go.datum.net/galactic/internal/cni/veth" "go.datum.net/galactic/pkg/common/cni" @@ -30,6 +42,19 @@ import ( "go.datum.net/galactic/pkg/common/vrf" ) +const cniTimeout = 10 * time.Second + +const labelNode = "bgp.miloapis.com/node" + +var cniScheme = runtime.NewScheme() + +func init() { + utilruntime.Must(clientgoscheme.AddToScheme(cniScheme)) + utilruntime.Must(bgpv1alpha1.AddToScheme(cniScheme)) + utilruntime.Must(providersv1alpha1.AddToScheme(cniScheme)) +} + +// PluginConf is the CNI plugin configuration passed via stdin on each invocation. type PluginConf struct { types.PluginConf VPC string `json:"vpc"` @@ -37,7 +62,6 @@ type PluginConf struct { MTU int `json:"mtu,omitempty"` Terminations []cni.Termination `json:"terminations,omitempty"` IPAM cni.IPAM `json:"ipam,omitempty"` - GoBGP cni.GoBGPConfig `json:"gobgp,omitempty"` SRv6Locator string `json:"srv6_locator"` } @@ -61,144 +85,275 @@ func NewCommand() *cobra.Command { func parseConf(data []byte) (*PluginConf, error) { conf := &PluginConf{} if err := json.Unmarshal(data, &conf); err != nil { - return nil, err + return nil, fmt.Errorf("parse CNI config: %w", err) } return conf, nil } -func getNetworks(pluginConf *PluginConf) ([]string, error) { - addresses := pluginConf.IPAM.Addresses - terminations := pluginConf.Terminations +// bgpVRFInstanceName returns the deterministic cluster-scoped name for a +// BGPVRFInstance. Each VPCAttachment is unique per interface across the +// cluster, so the (vpc, vpcAttachment) pair is a reliable 1:1 key. +func bgpVRFInstanceName(vpc, vpcAttachment string) string { + return fmt.Sprintf("%s-%s", vpc, vpcAttachment) +} - networks := make([]string, 0, len(addresses)+len(terminations)) +// routeDistinguisher returns the RD in "ASN:NN" (Type 0) format using the +// low 32 bits of the VPC identifier as the NN field. Type 0 has a 4-byte NN +// field, so the full uint32 range is safe on the wire. The RD is VPC-scoped +// rather than node-scoped; EVPN Type 5 next-hop differentiates routes from +// different nodes, so per-node uniqueness is not required. +func routeDistinguisher(asNumber int64, vpcHex string) (string, error) { + v, err := strconv.ParseUint(vpcHex, 16, 64) + if err != nil { + return "", fmt.Errorf("parse VPC hex %q: %w", vpcHex, err) + } + return fmt.Sprintf("%d:%d", asNumber, uint32(v)), nil +} - for _, a := range addresses { - ip, _, err := net.ParseCIDR(a.Address) +// routeTarget returns the RT in "ASN:NN" format using the low 32 bits of the +// VPC identifier. All nodes in the same VPC produce the same value, enabling +// VPC-scoped route import/export. vpcHex is the 48-bit hex VPC identifier. +func routeTarget(asNumber int64, vpcHex string) (string, error) { + v, err := strconv.ParseUint(vpcHex, 16, 64) + if err != nil { + return "", fmt.Errorf("parse VPC hex %q: %w", vpcHex, err) + } + return fmt.Sprintf("%d:%d", asNumber, uint32(v)), nil +} + +// bgpConfig holds the BGP values the CNI needs to populate a BGPVRFInstance. +type bgpConfig struct { + asNumber int64 + instanceName string + providerSelector metav1.LabelSelector +} + +// lookupBGPConfig finds the BGPProvider(s) on this node and the unique +// BGPInstance whose providerSelector matches one of them. It errors if the +// match is ambiguous (multiple instances) so BGP config is always deterministic. +func lookupBGPConfig(ctx context.Context, k8s client.Client, nodeName string) (bgpConfig, error) { + providerList := &providersv1alpha1.BGPProviderList{} + if err := k8s.List(ctx, providerList, client.MatchingLabels{ + labelNode: nodeName, + }); err != nil { + return bgpConfig{}, fmt.Errorf("list BGPProviders for node %s: %w", nodeName, err) + } + if len(providerList.Items) == 0 { + return bgpConfig{}, fmt.Errorf("no BGPProvider found for node %s", nodeName) + } + + instanceList := &bgpv1alpha1.BGPInstanceList{} + if err := k8s.List(ctx, instanceList); err != nil { + return bgpConfig{}, fmt.Errorf("list BGPInstances: %w", err) + } + + // Collect all BGPInstances whose providerSelector matches any provider on + // this node. The selector encodes daemon type; we just narrow to this node. + var matches []*bgpv1alpha1.BGPInstance + for i := range instanceList.Items { + sel, err := metav1.LabelSelectorAsSelector(&instanceList.Items[i].Spec.ProviderSelector) if err != nil { - return nil, err + return bgpConfig{}, fmt.Errorf("BGPInstance %s has invalid providerSelector: %w", instanceList.Items[i].Name, err) } - if ip.To4() != nil { - networks = append(networks, fmt.Sprintf("%s/32", ip.String())) - } else { - networks = append(networks, fmt.Sprintf("%s/128", ip.String())) + for j := range providerList.Items { + if sel.Matches(labels.Set(providerList.Items[j].Labels)) { + matches = append(matches, &instanceList.Items[i]) + break // count this instance once even if it matches multiple providers + } } } - for _, t := range terminations { - if t.Via != "" { - networks = append(networks, t.Network) + switch len(matches) { + case 0: + return bgpConfig{}, fmt.Errorf("no BGPInstance found selecting a provider on node %s", nodeName) + case 1: + // expected + default: + names := make([]string, len(matches)) + for i, m := range matches { + names[i] = m.Name } + return bgpConfig{}, fmt.Errorf("ambiguous BGP config: %d BGPInstances select providers on node %s: [%s]", + len(matches), nodeName, strings.Join(names, ", ")) } + instance := matches[0] - return networks, nil + // Build a provider selector that is the BGPInstance's selector narrowed to + // this specific node. The instance selector already encodes daemon type and + // other constraints; adding the node label makes it target exactly one node. + matchLabels := map[string]string{ + labelNode: nodeName, + } + maps.Copy(matchLabels, instance.Spec.ProviderSelector.MatchLabels) + + return bgpConfig{ + asNumber: instance.Spec.ASNumber, + instanceName: instance.Name, + providerSelector: metav1.LabelSelector{ + MatchLabels: matchLabels, + MatchExpressions: instance.Spec.ProviderSelector.MatchExpressions, + }, + }, nil +} + +func newK8sClient() (client.Client, error) { + restCfg, err := ctrl.GetConfig() + if err != nil { + return nil, fmt.Errorf("get kubeconfig: %w", err) + } + c, err := client.New(restCfg, client.Options{Scheme: cniScheme}) + if err != nil { + return nil, fmt.Errorf("create k8s client: %w", err) + } + return c, nil } func cmdAdd(args *skel.CmdArgs) error { - pluginConf, _ := parseConf(args.StdinData) - if err := vrf.Add(pluginConf.VPC, pluginConf.VPCAttachment); err != nil { + pluginConf, err := parseConf(args.StdinData) + if err != nil { return err } + + nodeName := os.Getenv("NODE_NAME") + if nodeName == "" { + return fmt.Errorf("NODE_NAME environment variable is not set") + } + + if err := vrf.Add(pluginConf.VPC, pluginConf.VPCAttachment); err != nil { + return fmt.Errorf("add VRF: %w", err) + } if err := veth.Add(pluginConf.VPC, pluginConf.VPCAttachment, pluginConf.MTU); err != nil { - return err + return fmt.Errorf("add veth: %w", err) } dev := util.GenerateInterfaceNameHost(pluginConf.VPC, pluginConf.VPCAttachment) for _, termination := range pluginConf.Terminations { if err := route.Add(pluginConf.VPC, pluginConf.VPCAttachment, termination.Network, termination.Via, dev); err != nil { - return err + return fmt.Errorf("add route %s: %w", termination.Network, err) } } if err := hostDevice("ADD", args, pluginConf); err != nil { - return err + return fmt.Errorf("host-device ADD: %w", err) } + vpcHex, err := util.Base62ToHex(pluginConf.VPC) if err != nil { - return err + return fmt.Errorf("decode VPC: %w", err) } vpcAttachmentHex, err := util.Base62ToHex(pluginConf.VPCAttachment) if err != nil { - return err + return fmt.Errorf("decode VPCAttachment: %w", err) } - networks, err := getNetworks(pluginConf) + + k8s, err := newK8sClient() if err != nil { return err } - bgpClient, bgpConn, err := bgp.NewClient(pluginConf.GoBGP.AddressOrDefault()) + ctx, cancel := context.WithTimeout(context.Background(), cniTimeout) + defer cancel() + + bgp, err := lookupBGPConfig(ctx, k8s, nodeName) if err != nil { return err } - defer bgpConn.Close() //nolint:errcheck - err = bgp.AddPaths(&bgp.PathConfig{ - Client: bgpClient, - SRv6Locator: pluginConf.SRv6Locator, - VPCHex: vpcHex, - VPCAttachmentHex: vpcAttachmentHex, - Networks: networks, + + rdValue, err := routeDistinguisher(bgp.asNumber, vpcHex) + if err != nil { + return fmt.Errorf("compute route distinguisher: %w", err) + } + rtValue, err := routeTarget(bgp.asNumber, vpcHex) + if err != nil { + return fmt.Errorf("compute route target: %w", err) + } + rt := bgpv1alpha1.RouteTarget{Value: rtValue} + + inst := &bgpv1alpha1.BGPVRFInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: bgpVRFInstanceName(pluginConf.VPC, pluginConf.VPCAttachment), + }, + } + _, err = controllerutil.CreateOrUpdate(ctx, k8s, inst, func() error { + inst.Spec = bgpv1alpha1.BGPVRFInstanceSpec{ + InstanceRef: bgp.instanceName, + ProviderSelector: bgp.providerSelector, + RouteDistinguisher: rdValue, + ImportRouteTargets: []bgpv1alpha1.RouteTarget{rt}, + ExportRouteTargets: []bgpv1alpha1.RouteTarget{rt}, + } + return nil }) if err != nil { - return err + return fmt.Errorf("apply BGPVRFInstance: %w", err) } + srv6Endpoint, err := util.EncodeSRv6Endpoint(pluginConf.SRv6Locator, vpcHex, vpcAttachmentHex) if err != nil { - return err + return fmt.Errorf("encode SRv6 endpoint: %w", err) } if err := srv6.RouteIngressAdd(srv6Endpoint); err != nil { - return err + return fmt.Errorf("add SRv6 ingress route: %w", err) } + result := &type100.Result{} return types.PrintResult(result, pluginConf.CNIVersion) } func cmdDel(args *skel.CmdArgs) error { - pluginConf, _ := parseConf(args.StdinData) - vpcHex, err := util.Base62ToHex(pluginConf.VPC) + pluginConf, err := parseConf(args.StdinData) if err != nil { return err } - vpcAttachmentHex, err := util.Base62ToHex(pluginConf.VPCAttachment) + + vpcHex, err := util.Base62ToHex(pluginConf.VPC) if err != nil { - return err + return fmt.Errorf("decode VPC: %w", err) } - networks, err := getNetworks(pluginConf) + vpcAttachmentHex, err := util.Base62ToHex(pluginConf.VPCAttachment) if err != nil { - return err + return fmt.Errorf("decode VPCAttachment: %w", err) } - bgpClient, bgpConn, err := bgp.NewClient(pluginConf.GoBGP.AddressOrDefault()) + srv6Endpoint, err := util.EncodeSRv6Endpoint(pluginConf.SRv6Locator, vpcHex, vpcAttachmentHex) if err != nil { - return err + return fmt.Errorf("encode SRv6 endpoint: %w", err) } - defer bgpConn.Close() //nolint:errcheck - err = bgp.DeletePaths(&bgp.PathConfig{ - Client: bgpClient, - SRv6Locator: pluginConf.SRv6Locator, - VPCHex: vpcHex, - VPCAttachmentHex: vpcAttachmentHex, - Networks: networks, - }) - if err != nil { - return err + if err := srv6.RouteIngressDel(srv6Endpoint); err != nil { + return fmt.Errorf("delete SRv6 ingress route: %w", err) } - srv6Endpoint, err := util.EncodeSRv6Endpoint(pluginConf.SRv6Locator, vpcHex, vpcAttachmentHex) + + // Signal BGP route withdrawal immediately after stopping kernel ingress so + // remote peers are notified as soon as possible. cosmos reconciles async. + // IgnoreNotFound handles concurrent DEL races. + k8s, err := newK8sClient() if err != nil { return err } - if err := srv6.RouteIngressDel(srv6Endpoint); err != nil { - return err + ctx, cancel := context.WithTimeout(context.Background(), cniTimeout) + defer cancel() + + inst := &bgpv1alpha1.BGPVRFInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: bgpVRFInstanceName(pluginConf.VPC, pluginConf.VPCAttachment), + }, + } + if err := k8s.Delete(ctx, inst); client.IgnoreNotFound(err) != nil { + return fmt.Errorf("delete BGPVRFInstance: %w", err) } + dev := util.GenerateInterfaceNameHost(pluginConf.VPC, pluginConf.VPCAttachment) if err := hostDevice("DEL", args, pluginConf); err != nil { - return err + return fmt.Errorf("host-device DEL: %w", err) } for _, termination := range pluginConf.Terminations { if err := route.Delete(pluginConf.VPC, pluginConf.VPCAttachment, termination.Network, termination.Via, dev); err != nil { - return err + return fmt.Errorf("delete route %s: %w", termination.Network, err) } } if err := veth.Delete(pluginConf.VPC, pluginConf.VPCAttachment); err != nil { - return err + return fmt.Errorf("delete veth: %w", err) } if err := vrf.Delete(pluginConf.VPC, pluginConf.VPCAttachment); err != nil { - return err + return fmt.Errorf("delete VRF: %w", err) } + result := &type100.Result{} return types.PrintResult(result, pluginConf.CNIVersion) } diff --git a/internal/cni/cni_test.go b/internal/cni/cni_test.go new file mode 100644 index 0000000..d518040 --- /dev/null +++ b/internal/cni/cni_test.go @@ -0,0 +1,383 @@ +// Copyright 2025 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cni + +import ( + "context" + "maps" + "strings" + "testing" + + bgpv1alpha1 "go.miloapis.com/cosmos/api/bgp/v1alpha1" + providersv1alpha1 "go.miloapis.com/cosmos/api/providers/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +const ( + labelDaemon = "galactic.io/daemon" + daemonGoBGP = "gobgp" + testVPCHex1234 = "0000000004d2" // decimal 1234 + testRD65000_1 = "65000:1" // RD/RT for ASN 65000, NN 1 +) + +func fakeClient(objs ...client.Object) client.Client { + return fake.NewClientBuilder().WithScheme(cniScheme).WithObjects(objs...).Build() +} + +// providerForNode builds a BGPProvider with the label set galactic-agent applies. +func providerForNode(name, node string, extraLabels map[string]string) *providersv1alpha1.BGPProvider { + lbls := map[string]string{ + labelNode: node, + labelDaemon: daemonGoBGP, + "galactic.io/plane": "overlay", + "galactic.io/managed-by": "galactic-agent", + } + maps.Copy(lbls, extraLabels) + return &providersv1alpha1.BGPProvider{ + ObjectMeta: metav1.ObjectMeta{Name: name, Labels: lbls}, + Spec: providersv1alpha1.BGPProviderSpec{ + Type: "GoBGP", + GoBGP: &providersv1alpha1.GoBGPProviderConfig{Endpoint: "localhost:50051"}, + }, + } +} + +// manualInstance builds a BGPInstance with the given providerSelector labels. +func manualInstance(name string, asn int64, selectorLabels map[string]string) *bgpv1alpha1.BGPInstance { + return &bgpv1alpha1.BGPInstance{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: bgpv1alpha1.BGPInstanceSpec{ + ProviderSelector: metav1.LabelSelector{MatchLabels: selectorLabels}, + ASNumber: asn, + AddressFamilies: []bgpv1alpha1.AddressFamily{{AFI: bgpv1alpha1.AFIL2VPN, SAFI: bgpv1alpha1.SAFIEVPN}}, + }, + } +} + +// ---- parseConf ----------------------------------------------------------- + +func TestParseConf(t *testing.T) { + tests := []struct { + name string + input string + wantVPC string + wantErr string + }{ + { + name: "valid config", + input: `{"cniVersion":"1.0.0","name":"test","type":"galactic","vpc":"abc","vpcattachment":"def","srv6_locator":"2001:db8::/48"}`, + wantVPC: "abc", + }, + { + name: "invalid JSON", + input: "not json", + wantErr: "parse CNI config", + }, + { + name: "empty input", + input: "", + wantErr: "parse CNI config", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + conf, err := parseConf([]byte(tt.input)) + if tt.wantErr != "" { + if err == nil { + t.Fatalf("expected error containing %q, got nil", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("error %q does not contain %q", err, tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if conf.VPC != tt.wantVPC { + t.Errorf("VPC = %q, want %q", conf.VPC, tt.wantVPC) + } + }) + } +} + +// ---- bgpVRFInstanceName -------------------------------------------------- + +func TestBGPVRFInstanceName(t *testing.T) { + tests := []struct{ vpc, attachment, want string }{ + {"abc", "def", "abc-def"}, + {"0000000jU", "00G", "0000000jU-00G"}, + } + for _, tt := range tests { + got := bgpVRFInstanceName(tt.vpc, tt.attachment) + if got != tt.want { + t.Errorf("bgpVRFInstanceName(%q, %q) = %q, want %q", tt.vpc, tt.attachment, got, tt.want) + } + } +} + +// ---- routeDistinguisher -------------------------------------------------- + +func TestRouteDistinguisher(t *testing.T) { + tests := []struct { + name string + asNumber int64 + vpcHex string + want string + wantErr bool + }{ + { + name: "small VPC value", + asNumber: 65000, + vpcHex: "000000000064", // 100 decimal + want: "65000:100", + }, + { + name: "VPC value 1", + asNumber: 65000, + vpcHex: "000000000001", + want: testRD65000_1, + }, + { + name: "exceeds Type 1 limit — valid for Type 0", + asNumber: 65000, + vpcHex: "000000010000", // 65536 — would overflow Type 1 NN, safe in Type 0 + want: "65000:65536", + }, + { + name: "low 32 bits all set", + asNumber: 65000, + vpcHex: "0000ffffffff", // low32 = 4294967295 + want: "65000:4294967295", + }, + { + name: "upper 16 bits of 48-bit VPC are stripped", + asNumber: 65000, + vpcHex: "000100000001", // 0x000100000001; low32 = 1 + want: testRD65000_1, + }, + { + name: "four-byte ASN", + asNumber: 4200000000, + vpcHex: testVPCHex1234, + want: "4200000000:1234", + }, + { + name: "invalid hex string", + vpcHex: "zzzzzz", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := routeDistinguisher(tt.asNumber, tt.vpcHex) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got nil (result: %q)", got) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Errorf("routeDistinguisher(%d, %q) = %q, want %q", tt.asNumber, tt.vpcHex, got, tt.want) + } + }) + } +} + +// ---- routeTarget --------------------------------------------------------- + +func TestRouteTarget(t *testing.T) { + tests := []struct { + name string + asNumber int64 + vpcHex string + want string + wantErr bool + }{ + { + name: "VPC value fits in 32 bits", + asNumber: 65000, + vpcHex: testVPCHex1234, + want: "65000:1234", + }, + { + name: "upper 16 bits of 48-bit VPC stripped", + asNumber: 65000, + vpcHex: "000100000001", // 0x000100000001; low32 = 1 + want: testRD65000_1, + }, + { + name: "low 32 bits all set", + asNumber: 65000, + vpcHex: "0000ffffffff", + want: "65000:4294967295", + }, + { + name: "different ASN", + asNumber: 4200000000, + vpcHex: testVPCHex1234, + want: "4200000000:1234", + }, + { + name: "invalid hex string", + vpcHex: "zzzzzz", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := routeTarget(tt.asNumber, tt.vpcHex) + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Errorf("routeTarget(%d, %q) = %q, want %q", tt.asNumber, tt.vpcHex, got, tt.want) + } + }) + } +} + +// ---- lookupBGPConfig ----------------------------------------------------- + +func TestLookupBGPConfig(t *testing.T) { + ctx := context.Background() + const nodeName = "node1" + + gobgpProvider := providerForNode("galactic-gobgp-node1", nodeName, nil) + gobgpSelector := map[string]string{labelDaemon: daemonGoBGP} + matchingInstance := manualInstance("overlay-instance", 65000, gobgpSelector) + + tests := []struct { + name string + objects []client.Object + wantErr string + check func(t *testing.T, cfg bgpConfig) + }{ + { + name: "no providers for node", + objects: nil, + wantErr: "no BGPProvider found", + }, + { + name: "provider present but no matching instance", + objects: []client.Object{gobgpProvider}, + wantErr: "no BGPInstance found", + }, + { + name: "single matching instance returns correct config", + objects: []client.Object{gobgpProvider, matchingInstance}, + check: func(t *testing.T, cfg bgpConfig) { + t.Helper() + if cfg.asNumber != 65000 { + t.Errorf("asNumber = %d, want 65000", cfg.asNumber) + } + if cfg.instanceName != "overlay-instance" { + t.Errorf("instanceName = %q, want %q", cfg.instanceName, "overlay-instance") + } + }, + }, + { + name: "providerSelector merges node label with instance's selector labels", + objects: []client.Object{gobgpProvider, matchingInstance}, + check: func(t *testing.T, cfg bgpConfig) { + t.Helper() + ml := cfg.providerSelector.MatchLabels + if ml[labelNode] != nodeName { + t.Errorf("MatchLabels[node] = %q, want %q", ml[labelNode], nodeName) + } + if ml[labelDaemon] != "gobgp" { + t.Errorf("MatchLabels[daemon] = %q, want %q", ml[labelDaemon], "gobgp") + } + }, + }, + { + name: "non-matching instance selector is ignored", + objects: []client.Object{ + gobgpProvider, + matchingInstance, + manualInstance("frr-instance", 65001, map[string]string{labelDaemon: "frr"}), + }, + check: func(t *testing.T, cfg bgpConfig) { + t.Helper() + if cfg.instanceName != "overlay-instance" { + t.Errorf("instanceName = %q, want %q", cfg.instanceName, "overlay-instance") + } + }, + }, + { + name: "ambiguous: two instances both select the provider", + objects: []client.Object{ + gobgpProvider, + manualInstance("instance-a", 65000, gobgpSelector), + manualInstance("instance-b", 65001, gobgpSelector), + }, + wantErr: "ambiguous", + }, + { + name: "ambiguous error lists instance names", + objects: []client.Object{ + gobgpProvider, + manualInstance("alpha", 65000, gobgpSelector), + manualInstance("beta", 65001, gobgpSelector), + }, + wantErr: "alpha", + }, + { + name: "invalid providerSelector on instance surfaces error", + objects: []client.Object{ + gobgpProvider, + &bgpv1alpha1.BGPInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "bad-instance"}, + Spec: bgpv1alpha1.BGPInstanceSpec{ + ASNumber: 65000, + ProviderSelector: metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + {Key: "foo", Operator: "BadOperator", Values: []string{"bar"}}, + }, + }, + AddressFamilies: []bgpv1alpha1.AddressFamily{{AFI: bgpv1alpha1.AFIL2VPN, SAFI: bgpv1alpha1.SAFIEVPN}}, + }, + }, + }, + wantErr: "invalid providerSelector", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k8s := fakeClient(tt.objects...) + + cfg, err := lookupBGPConfig(ctx, k8s, nodeName) + if tt.wantErr != "" { + if err == nil { + t.Fatalf("expected error containing %q, got nil", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("error %q does not contain %q", err, tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tt.check != nil { + tt.check(t, cfg) + } + }) + } +} diff --git a/pkg/common/cni/types.go b/pkg/common/cni/types.go index 2220dd1..9b5f9ef 100644 --- a/pkg/common/cni/types.go +++ b/pkg/common/cni/types.go @@ -9,19 +9,6 @@ type Termination struct { Via string `json:"via,omitempty"` } -// GoBGPConfig holds the address of the local GoBGP gRPC API. -type GoBGPConfig struct { - Address string `json:"address,omitempty"` -} - -// AddressOrDefault returns the configured address or the node-local default. -func (c GoBGPConfig) AddressOrDefault() string { - if c.Address == "" { - return "127.0.0.1:50051" - } - return c.Address -} - type IPAM struct { Type string `json:"type"` Routes []Route `json:"routes,omitempty"`