diff --git a/Taskfile.yaml b/Taskfile.yaml index a368dec1..d2e5341d 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -135,6 +135,7 @@ tasks: - go build -ldflags "{{.LDFLAGS}}" -o bin/galactic-cni ./cmd/galactic-cni - go build -ldflags "{{.LDFLAGS}}" -o bin/galactic-tap-cni ./cmd/galactic-tap-cni - go build -ldflags "{{.LDFLAGS}}" -o bin/galactic-ipam ./cmd/galactic-ipam + - go build -ldflags "{{.LDFLAGS}}" -o bin/galactic-bgp ./cmd/galactic-bgp - go build -ldflags "{{.LDFLAGS}}" -o bin/galactic-router ./cmd/galactic-router - go build -ldflags "{{.LDFLAGS}}" -o bin/vmtap-cni ./cmd/vmtap-cni - GOBIN={{.LOCALBIN}} go install github.com/containernetworking/plugins/plugins/main/host-device@v1.9.1 diff --git a/cmd/galactic-bgp/main.go b/cmd/galactic-bgp/main.go new file mode 100644 index 00000000..c96d2c97 --- /dev/null +++ b/cmd/galactic-bgp/main.go @@ -0,0 +1,91 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package main + +import ( + "fmt" + "log" + "os" + "strings" + + "github.com/containernetworking/cni/pkg/version" + "github.com/spf13/cobra" + "golang.org/x/term" + + "go.datum.net/galactic/internal/cnibgp" + "go.datum.net/galactic/internal/metadata" +) + +const ( + appName = "galactic-bgp" + + appDesc = `Galactic BGP CNI Plugin + + The BGP/SRv6/eBPF publish plugin in the galactic CNI chain — chained after + galactic-cni/galactic-tap-cni (and, when present, galactic-route) per + conflist order, never run standalone. Has zero kernel-interface + dependency: every address it advertises comes from prevResult, not from a + runtime call into an interface it doesn't own. + + Find more information at: https://www.datum.net/docs` +) + +func newRootCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: appName, + Short: strings.Split(appDesc, "\n")[0], + Long: appDesc, + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + cnibgp.InitCNIConfig() + confFile, _ := cmd.Flags().GetString("conf-file") + if confFile != "" { + cnibgp.ConfFile = confFile + } + return nil + }, + RunE: func(cmd *cobra.Command, _ []string) error { + if ok, _ := cmd.Flags().GetBool("build-info"); ok { + fmt.Println(metadata.BuildInfo(appName)) + return nil + } + if ok, _ := cmd.Flags().GetBool("version"); ok { + fmt.Printf("%s version %s\n", appName, metadata.Version) + return nil + } + if os.Getenv("CNI_COMMAND") == "VERSION" { + return version.All.Encode(os.Stdout) + } + + // Real CNI runtimes always pipe the network config JSON on + // stdin and close it. If stdin is an interactive terminal + // instead, no config will ever arrive and skel's blocking + // stdin read would hang forever — print version info instead. + if term.IsTerminal(int(os.Stdin.Fd())) { + fmt.Printf("%s version %s\n", appName, metadata.Version) + fmt.Printf("CNI protocol versions supported: %s\n", strings.Join(version.All.SupportedVersions(), ", ")) + return nil + } + + // Unlike galactic-cni/galactic-tap-cni, this plugin never enters + // any network namespace at all (it only makes k8s API calls), + // so it needs neither the stdin peek-and-repipe dance nor + // CNI_NETNS_OVERRIDE those two use to detect and handle + // tap-mode's host-netns invocation. + cnibgp.RunPlugin() + return nil + }, + } + + cmd.PersistentFlags().String("conf-file", cnibgp.ConfFile, "Path to CNI conflist file") + cmd.Flags().Bool("build-info", false, "Print build information and exit") + cmd.Flags().BoolP("version", "V", false, "Print version and exit") + return cmd +} + +func main() { + if err := newRootCommand().Execute(); err != nil { + log.Fatalf("error: %v", err) + } +} diff --git a/containers/galactic-cni/Dockerfile b/containers/galactic-cni/Dockerfile index 74baa0b4..4c41d158 100644 --- a/containers/galactic-cni/Dockerfile +++ b/containers/galactic-cni/Dockerfile @@ -75,6 +75,19 @@ RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build \ -X go.datum.net/galactic/internal/metadata.GitURL=${GIT_URL}" \ -o galactic-ipam cmd/galactic-ipam/main.go +# Build galactic-bgp, the BGP/SRv6/eBPF publish plugin in the galactic CNI +# chain. Ships in this same image/binary set for the same reason +# galactic-tap-cni/galactic-ipam do — see their own comments above. +RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build \ + -ldflags "-s -w \ + -X go.datum.net/galactic/internal/metadata.Version=${VERSION} \ + -X go.datum.net/galactic/internal/metadata.GitCommit=${GIT_COMMIT} \ + -X go.datum.net/galactic/internal/metadata.GitTreeState=${GIT_TREE_STATE} \ + -X go.datum.net/galactic/internal/metadata.BuildDate=${BUILD_DATE} \ + -X go.datum.net/galactic/internal/metadata.SPDXLicense=${SPDX_LICENSE} \ + -X go.datum.net/galactic/internal/metadata.GitURL=${GIT_URL}" \ + -o galactic-bgp cmd/galactic-bgp/main.go + # Build vmtap-cni. It ships in this image rather than one of its own so the # vmtap DaemonSet (config/vmtap/) can reference the same published # ghcr.io/datum-cloud/galactic-cni image instead of a second, separately @@ -111,6 +124,7 @@ FROM gcr.io/distroless/static:nonroot AS production COPY --from=builder /workspace/galactic-cni /galactic-cni COPY --from=builder /workspace/galactic-tap-cni /galactic-tap-cni COPY --from=builder /workspace/galactic-ipam /galactic-ipam +COPY --from=builder /workspace/galactic-bgp /galactic-bgp COPY --from=builder /workspace/vmtap-cni /vmtap-cni COPY --from=builder /workspace/host-device /host-device COPY --from=builder /var/run/galactic-cni /var/run/galactic-cni @@ -125,6 +139,7 @@ FROM docker.io/library/alpine:latest COPY --from=production /galactic-cni /galactic-cni COPY --from=production /galactic-tap-cni /galactic-tap-cni COPY --from=production /galactic-ipam /galactic-ipam +COPY --from=production /galactic-bgp /galactic-bgp COPY --from=production /vmtap-cni /vmtap-cni COPY --from=production /host-device /host-device COPY --from=production /var/run/galactic-cni /var/run/galactic-cni diff --git a/docs/cni/configuration.md b/docs/cni/configuration.md index 7abd5e9c..ebe34242 100644 --- a/docs/cni/configuration.md +++ b/docs/cni/configuration.md @@ -111,6 +111,16 @@ v1.3.0 in `go.mod`) and returns CNI Result `1.0.0` (`type100`); generated configs (the installer's default conflist, Multus `NetworkAttachmentDefinition` manifests) use `"cniVersion": "1.0.0"`. +Despite that broad declared range, `cniVersion` must be `"1.0.0"` or `"1.1.0"` +in practice: `galactic-bgp`, chained after the master plugin, reconstructs +`prevResult` via `type100.NewResult` (`internal/cnibgp/prevresult.go`), which +only accepts a Result whose own `cniVersion` field is exactly one of those two +values — the master plugin echoes the conflist's `cniVersion` straight into +its printed Result, so an older value (e.g. `"0.4.0"`) makes `galactic-bgp`'s +ADD fail for every attachment in the chain. Every config in this doc already +uses `"1.0.0"`; keep it that way for any config authored outside these +examples. + ### Interface Types #### `veth` (default) diff --git a/internal/cni/cni_test.go b/internal/cni/cni_test.go index 3734151b..a16ddf8e 100644 --- a/internal/cni/cni_test.go +++ b/internal/cni/cni_test.go @@ -5,7 +5,6 @@ package cni import ( - "context" "errors" "fmt" "log/slog" @@ -798,8 +797,7 @@ func TestResourceTrackerCleanupZeroValue(t *testing.T) { // cleanup with a zero-value tracker must not panic — it's called in a // defer and the caller may have failed before setting any fields. tracker := &resourceTracker{} - ctx := context.Background() - tracker.cleanup(ctx) // should not panic + tracker.cleanup() // should not panic } func TestResourceTrackerCleanupPartialState(t *testing.T) { @@ -808,17 +806,14 @@ func TestResourceTrackerCleanupPartialState(t *testing.T) { tracker := &resourceTracker{ vpc: testVPC, vpcAttachment: testAttachment, - namespace: "default", } - ctx := context.Background() - tracker.cleanup(ctx) // should not panic; vrf.Delete will fail but is logged + tracker.cleanup() // should not panic; vrf.Delete will fail but is logged } func TestResourceTrackerFieldsSet(t *testing.T) { tracker := &resourceTracker{ vpc: testVPC, vpcAttachment: testAttachment, - namespace: "test-ns", } if tracker.vpc != testVPC { @@ -827,14 +822,11 @@ func TestResourceTrackerFieldsSet(t *testing.T) { if tracker.vpcAttachment != testAttachment { t.Errorf("vpcAttachment = %q, want %q", tracker.vpcAttachment, testAttachment) } - if tracker.namespace != "test-ns" { - t.Errorf("namespace = %q, want %q", tracker.namespace, "test-ns") - } if tracker.vrfCreated { t.Error("vrfCreated should be false by default") } - if tracker.advCreated { - t.Error("advCreated should be false by default") + if tracker.routesCreated != 0 { + t.Error("routesCreated should be zero by default") } } diff --git a/internal/cni/hostgw/hostgw.go b/internal/cni/hostgw/hostgw.go new file mode 100644 index 00000000..9d036e20 --- /dev/null +++ b/internal/cni/hostgw/hostgw.go @@ -0,0 +1,212 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +// Package hostgw configures the host-side gateway address and VRF-table +// pod-subnet route for a VPC attachment's allocated IPAM addresses. +// +// This is kernel-interface work (netlink address/route/neighbor +// manipulation on the interface a master plugin — galactic-cni, +// galactic-tap-cni — itself created), not BGP/SRv6/eBPF publish, so it +// lives here rather than in internal/cnibgp: once galactic-bgp became its +// own chain-invoked plugin (a separate process, invoked after the master +// has already printed its own result), it no longer has any interface to +// configure — "zero kernel-interface dependency" is the whole reason that +// split was worth doing in the first place. Both master plugins call this +// directly, before building their own CNI result; galactic-bgp reads +// whatever addresses ended up in prevResult and never touches the kernel +// interface at all. +package hostgw + +import ( + "errors" + "fmt" + "net" + "syscall" + + "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" + + "go.datum.net/galactic/internal/cniipam" + "go.datum.net/galactic/internal/plumbing/intf" + "go.datum.net/galactic/internal/plumbing/vrf" +) + +// ConfigureHostGateway assigns each configured family's gateway address as a +// host address (/128 for IPv6, /32 for IPv4 on veth) on the host-side +// interface (veth or tap) and installs an explicit pod-subnet route for that +// family into the VRF table. IPv4 is skipped entirely when the attachment is +// IPv6-only. +// +// Using a full-length host address (not the pod subnet mask) prevents the +// kernel from auto-creating a subnet-router anycast entry in the VRF local +// table. When the pod address equals the subnet network address the anycast +// absorbs seg6local-decapped inner packets before they reach the guest +// interface. The explicit subnet route replaces the one the kernel would +// have created from the wider mask. +// +// For tap interfaces, the IPv4 gateway is instead assigned as a /25 so the +// address reported on the interface reflects a real subnet (VM guests expect +// this). That reintroduces the wider-mask hazard described above, so the +// address is added with IFA_F_NOPREFIXROUTE: the kernel skips auto-creating +// the connected /25 route entirely, leaving the explicit pod-subnet route +// below as the only thing that governs delivery to this VM's address. +// +// guestHWAddr is the guest-side veth's MAC address, used to prime a +// permanent neighbor table entry for the pod's own address (see +// installGatewayNeighbor). It is nil for tap attachments, which have no +// separate guest-side link in this netns to resolve a MAC from. +func ConfigureHostGateway(vpc, vpcAttachment string, res *cniipam.IPAMResult, guestHWAddr net.HardwareAddr) error { + if res == nil { + return nil + } + hostName := intf.GenerateInterfaceNameHost(vpc, vpcAttachment) + hostLink, err := netlink.LinkByName(hostName) + if err != nil { + return fmt.Errorf("get host interface %q: %w", hostName, err) + } + tableID, err := vrf.TableID(vpc, vpcAttachment) + if err != nil { + return fmt.Errorf("get VRF table ID for pod subnet route: %w", err) + } + + if res.IPv6Gateway != nil { + gwNet := &net.IPNet{IP: res.IPv6Gateway, Mask: net.CIDRMask(128, 128)} + if err := installGatewayRoute(hostLink, gwNet, res.IPv6Subnet, netlink.FAMILY_V6, int(tableID), 0); err != nil { + return err + } + if guestHWAddr != nil { + if err := installGatewayNeighbor(hostLink, res.IPv6Subnet.IP, netlink.FAMILY_V6, guestHWAddr); err != nil { + return err + } + } + } + if res.IPv4Gateway != nil { + ipv4Mask, addrFlags := ipv4GatewayAddrParams(hostLink) + gwNet := &net.IPNet{IP: res.IPv4Gateway, Mask: ipv4Mask} + ipv4Subnet := &net.IPNet{IP: res.IPv4Address, Mask: net.CIDRMask(32, 32)} + if err := installGatewayRoute(hostLink, gwNet, ipv4Subnet, netlink.FAMILY_V4, int(tableID), addrFlags); err != nil { + return err + } + if guestHWAddr != nil { + if err := installGatewayNeighbor(hostLink, res.IPv4Address, netlink.FAMILY_V4, guestHWAddr); err != nil { + return err + } + } + } + return nil +} + +// installGatewayNeighbor installs a permanent neighbor table entry mapping +// podIP to guestHWAddr on hostLink. +// +// The eBPF uSID ingress datapath (internal/plumbing/ebpf/prog/usid.c) +// decapsulates SRv6 traffic and calls bpf_fib_lookup() to resolve the +// egress path for the inner packet, then redirects it straight to the +// resolved neighbor — entirely in-kernel, never touching the normal +// forwarding stack. bpf_fib_lookup() does not itself trigger ARP/NDP +// resolution the way ordinary kernel packet forwarding does, so without a +// pre-existing neighbor table entry it fails with BPF_FIB_LKUP_RET_NO_NEIGH +// and the datapath drops the packet. A permanent entry (installed once, at +// CNI ADD, using the guest veth's own known MAC) means this resolution +// never depends on dynamic ARP/NDP at all. +func installGatewayNeighbor(hostLink netlink.Link, podIP net.IP, family int, guestHWAddr net.HardwareAddr) error { + neigh := &netlink.Neigh{ + LinkIndex: hostLink.Attrs().Index, + Family: family, + State: netlink.NUD_PERMANENT, + IP: podIP, + HardwareAddr: guestHWAddr, + } + if err := netlink.NeighSet(neigh); err != nil { + return fmt.Errorf("add permanent neighbor %s -> %s on host interface %q: %w", + podIP, guestHWAddr, hostLink.Attrs().Name, err) + } + return nil +} + +// ipv4GatewayAddrParams returns the IPv4 gateway mask and netlink address +// flags to use for hostLink. Tap interfaces get a /25 (so the address +// reported on the interface reflects a real subnet) with +// IFA_F_NOPREFIXROUTE, which stops the kernel from auto-creating a connected +// route for the wider mask. Veth interfaces keep the plain /32 host address +// with no flags. +func ipv4GatewayAddrParams(hostLink netlink.Link) (net.IPMask, int) { + if _, isTap := hostLink.(*netlink.Tuntap); isTap { + return net.CIDRMask(25, 32), unix.IFA_F_NOPREFIXROUTE + } + return net.CIDRMask(32, 32), 0 +} + +// installGatewayRoute assigns gwNet as a host address on hostLink and +// installs an explicit route to subnet into the given VRF table, for one +// address family. Idempotent: existing matching routes/addresses are left +// alone, and conflicting ones return an error rather than being overwritten. +func installGatewayRoute(hostLink netlink.Link, gwNet, subnet *net.IPNet, family, tableID, addrFlags int) error { + hostName := hostLink.Attrs().Name + if err := netlink.AddrAdd(hostLink, &netlink.Addr{IPNet: gwNet, Flags: addrFlags}); err != nil { + if !errors.Is(err, syscall.EEXIST) { + return fmt.Errorf("add gateway address %s to host interface %q: %w", gwNet, hostName, err) + } + } + + desiredRoute := &netlink.Route{ + Dst: subnet, + LinkIndex: hostLink.Attrs().Index, + Table: tableID, + } + + existingRoutes, err := netlink.RouteListFiltered( + family, + &netlink.Route{Table: tableID}, + netlink.RT_FILTER_TABLE, + ) + if err != nil { + return fmt.Errorf("list routes in VRF table: %w", err) + } + for _, r := range existingRoutes { + if r.Dst == nil { + continue + } + if r.Dst.String() != desiredRoute.Dst.String() { + continue + } + if routeConflicts(&r, desiredRoute) { + return fmt.Errorf( + "existing route %v to %s conflicts with desired route %v", + r, desiredRoute.Dst, desiredRoute, + ) + } + return nil + } + + if err := netlink.RouteAdd(desiredRoute); err != nil { + if errors.Is(err, syscall.EEXIST) { + return nil + } + return fmt.Errorf("add pod subnet route to VRF table: %w", err) + } + return nil +} + +// routeConflicts reports whether an existing route conflicts with the desired +// pod-subnet route. A conflict occurs when the destination matches but the +// gateway or link index differs. +func routeConflicts(existing, desired *netlink.Route) bool { + if existing.Dst == nil || desired.Dst == nil { + return false + } + if existing.Dst.String() != desired.Dst.String() { + return false + } + if (existing.Gw != nil) != (desired.Gw != nil) { + return true + } + if existing.Gw != nil && !existing.Gw.Equal(desired.Gw) { + return true + } + if existing.LinkIndex != 0 && desired.LinkIndex != 0 && existing.LinkIndex != desired.LinkIndex { + return true + } + return false +} diff --git a/internal/cni/hostgw/hostgw_test.go b/internal/cni/hostgw/hostgw_test.go new file mode 100644 index 00000000..f251994a --- /dev/null +++ b/internal/cni/hostgw/hostgw_test.go @@ -0,0 +1,115 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package hostgw + +import ( + "net" + "testing" + + "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" +) + +func mustParseCIDR(t *testing.T, cidr string) *net.IPNet { + t.Helper() + _, ipnet, err := net.ParseCIDR(cidr) + if err != nil { + t.Fatalf("parse CIDR %q: %v", cidr, err) + } + return ipnet +} + +func TestIPv4GatewayAddrParams(t *testing.T) { + tests := []struct { + name string + hostLink netlink.Link + wantMask net.IPMask + wantFlags int + }{ + { + name: "tap gets /25 with NOPREFIXROUTE", + hostLink: &netlink.Tuntap{LinkAttrs: netlink.LinkAttrs{Name: "tap0"}}, + wantMask: net.CIDRMask(25, 32), + wantFlags: unix.IFA_F_NOPREFIXROUTE, + }, + { + name: "veth gets /32 with no flags", + hostLink: &netlink.Veth{LinkAttrs: netlink.LinkAttrs{Name: "veth0"}}, + wantMask: net.CIDRMask(32, 32), + wantFlags: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotMask, gotFlags := ipv4GatewayAddrParams(tt.hostLink) + if gotMask.String() != tt.wantMask.String() { + t.Errorf("mask = %v, want %v", gotMask, tt.wantMask) + } + if gotFlags != tt.wantFlags { + t.Errorf("flags = %v, want %v", gotFlags, tt.wantFlags) + } + }) + } +} + +func TestRouteConflicts(t *testing.T) { + dst := mustParseCIDR(t, "fd00:10:ff01::1234/80") + gw1 := net.ParseIP("fd00:10:ff01::1") + gw2 := net.ParseIP("fd00:10:ff01::2") + otherDst := mustParseCIDR(t, "fd00:10:ff02::1234/80") + + tests := []struct { + name string + existing *netlink.Route + desired *netlink.Route + want bool + }{ + {"nil existing destination — no conflict", &netlink.Route{Dst: nil}, &netlink.Route{Dst: dst}, false}, + {"nil desired destination — no conflict", &netlink.Route{Dst: dst}, &netlink.Route{Dst: nil}, false}, + {"different destinations — no conflict", &netlink.Route{Dst: otherDst}, &netlink.Route{Dst: dst}, false}, + { + "same destination, no gateway on either — no conflict", + &netlink.Route{Dst: dst, LinkIndex: 5}, &netlink.Route{Dst: dst, LinkIndex: 5}, false, + }, + { + "same destination, same gateway — no conflict", + &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, false, + }, + { + "same destination, different gateway — conflict", + &netlink.Route{Dst: dst, Gw: gw1}, &netlink.Route{Dst: dst, Gw: gw2}, true, + }, + { + "existing has gateway, desired does not — conflict", + &netlink.Route{Dst: dst, Gw: gw1}, &netlink.Route{Dst: dst}, true, + }, + { + "desired has gateway, existing does not — conflict", + &netlink.Route{Dst: dst}, &netlink.Route{Dst: dst, Gw: gw1}, true, + }, + { + "same destination, same gateway, different link index — conflict", + &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 7}, true, + }, + { + "same destination, gateway set, link index zero on existing — no conflict", + &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 0}, &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, false, + }, + { + "same destination, no gateway, different link index — conflict", + &netlink.Route{Dst: dst, LinkIndex: 5}, &netlink.Route{Dst: dst, LinkIndex: 7}, true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := routeConflicts(tt.existing, tt.desired) + if got != tt.want { + t.Errorf("routeConflicts() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/cni/ops_add.go b/internal/cni/ops_add.go index 7a5fc761..11c616c7 100644 --- a/internal/cni/ops_add.go +++ b/internal/cni/ops_add.go @@ -17,19 +17,17 @@ import ( "go.datum.net/galactic/internal/cni/nadpatch" "go.datum.net/galactic/internal/cni/route" "go.datum.net/galactic/internal/cni/veth" - "go.datum.net/galactic/internal/cnibgp" "go.datum.net/galactic/internal/plumbing/intf" "go.datum.net/galactic/internal/plumbing/vrf" ) // cmdAdd uses a named return (err) so that the deferred selective rollback -// below always observes the real failure: several branches check errors via -// "if err := f(); err != nil" inside nested if/switch blocks, which declares -// a block-scoped err that would otherwise shadow this function's err and -// leave the deferred rollback thinking the call succeeded. A plain -// "return expr" always assigns expr to a named result, regardless of that -// local shadowing, so naming the return here is what makes rollback fire on -// every failure path instead of just the ones using top-level "x, err := f()". +// below always observes the real failure — see the doc comment on the +// original version of this function for why a named return matters here; +// still true with fewer branches. galactic-cni's own ADD ends by printing +// its own result and returning: BGP/SRv6/eBPF publish is galactic-bgp's +// job, invoked next by the CNI runtime per conflist order, not by this +// process. func cmdAdd(args *skel.CmdArgs) (err error) { pluginConf, err := parseConf(args.StdinData) if err != nil { @@ -62,7 +60,6 @@ func cmdAdd(args *skel.CmdArgs) (err error) { tracker := &resourceTracker{ vpc: pluginConf.VPC, vpcAttachment: pluginConf.VPCAttachment, - namespace: namespace, } // Record IPAM delegation intent up front, before configureIPAM (called // from buildVethResult below) ever runs — see resourceTracker's @@ -76,15 +73,11 @@ func cmdAdd(args *skel.CmdArgs) (err error) { } // Selective rollback: clean up only resources that were created. - // We need a context for k8s operations in rollback; the k8s client - // will be populated below before it's needed. - rollbackCtx, rollbackCancel := context.WithTimeout(context.Background(), cniTimeout) defer func() { if err != nil { slog.Error("ADD: failed, rolling back created resources", "err", err, "containerID", args.ContainerID, "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment) - tracker.cleanup(rollbackCtx) - rollbackCancel() + tracker.cleanup() } }() @@ -114,9 +107,10 @@ func cmdAdd(args *skel.CmdArgs) (err error) { if err != nil { return fmt.Errorf("create k8s client: %w", err) } - tracker.k8s = k8sClient podNamespace := nadpatch.ParsePodNamespace(args.Args) - if err := nadpatch.AnnotateNAD(rollbackCtx, k8sClient, pluginConf.Name, podNamespace, hostName); err != nil { + nadCtx, nadCancel := context.WithTimeout(context.Background(), cniTimeout) + defer nadCancel() + if err := nadpatch.AnnotateNAD(nadCtx, k8sClient, pluginConf.Name, podNamespace, hostName); err != nil { return fmt.Errorf("annotate NAD: %w", err) } @@ -132,24 +126,5 @@ func cmdAdd(args *skel.CmdArgs) (err error) { } guestName := intf.GenerateInterfaceNameGuest(pluginConf.VPC, pluginConf.VPCAttachment) - ipamResult, guestHWAddr, err := buildVethResult(args, pluginConf, hostName, guestName, hostMac, hostMTU) - if err != nil { - return err - } - if ipamResult != nil { - slog.Debug("ADD: IPAM allocated", "containerID", args.ContainerID, - "ipv6Subnet", ipamResult.IPv6Subnet, "ipv6Gateway", ipamResult.IPv6Gateway, - "ipv4Address", ipamResult.IPv4Address, "ipv4Gateway", ipamResult.IPv4Gateway) - } - - slog.Debug("ADD: publishing BGP state", "containerID", args.ContainerID) - result, err := cnibgp.PublishBGPState( - args, cnibgp.PublishConfig{VPC: pluginConf.VPC, VPCAttachment: pluginConf.VPCAttachment, InterfaceType: "veth"}, - nodeName, namespace, ipamResult, guestHWAddr, tracker.k8s) - tracker.vrfInstanceCreated = result.VRFInstanceCreated - tracker.advCreated = result.AdvertisementCreated - tracker.ebpfRegistered = result.EBPFRegistered - tracker.ebpfBlock = result.EBPFBlock - tracker.ebpfArgument = result.EBPFArgument - return err + return buildVethResult(args, pluginConf, hostName, guestName, hostMac, hostMTU) } diff --git a/internal/cni/resource.go b/internal/cni/resource.go index 710bf1a0..c986b05a 100644 --- a/internal/cni/resource.go +++ b/internal/cni/resource.go @@ -5,37 +5,34 @@ package cni import ( - "context" "fmt" "log/slog" "github.com/containernetworking/plugins/pkg/ipam" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "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" - "go.datum.net/galactic/internal/cni/crdnames" "go.datum.net/galactic/internal/cni/veth" - "go.datum.net/galactic/internal/cnibgp" - "go.datum.net/galactic/internal/plumbing/ebpf/attach" "go.datum.net/galactic/internal/plumbing/vrf" - bgpv1alpha1 "go.datum.net/network/api/v1alpha1" ) var cniScheme = runtime.NewScheme() func init() { utilruntime.Must(clientgoscheme.AddToScheme(cniScheme)) - utilruntime.Must(bgpv1alpha1.AddToScheme(cniScheme)) + // No BGP CRD scheme registration: this package no longer reads or + // writes BGP CRDs itself (that's galactic-bgp's own, chain-invoked + // concern now — internal/cnibgp/resource.go registers bgpv1alpha1 for + // its own client), and NAD annotation's unstructured.Unstructured Patch + // call doesn't require any scheme registration at all. } // newK8sClient creates a new Kubernetes client using the in-cluster config, -// scoped to cniScheme (core types + BGP CRDs — the latter needed for both -// rollback's own CRD deletes and the cnibgp.PublishBGPState call this -// client gets passed into). +// scoped to cniScheme. The only k8s call this plugin makes directly is the +// NAD annotation patch. func newK8sClient() (client.Client, error) { restCfg, err := ctrl.GetConfig() if err != nil { @@ -49,25 +46,15 @@ func newK8sClient() (client.Client, error) { } // resourceTracker tracks resources created during cmdAdd for selective -// rollback. galactic-cni is veth-only, so this is scoped to exactly what its -// own ADD creates: the VRF, the veth pair, and — for now, until BGP publish -// becomes its own chain-invoked plugin — the BGP CRDs and eBPF vrf_table -// entry that internal/cnibgp.PublishBGPState wrote on its behalf. +// rollback. galactic-cni is veth-only, and its ADD only ever creates the +// VRF, the veth pair, and (if delegated) an IPAM allocation — BGP/SRv6/eBPF +// publish is galactic-bgp's own, separately chain-invoked plugin now, with +// its own smaller tracker (internal/cnibgp), so this one no longer needs to +// know anything about that state at all. type resourceTracker struct { vpc, vpcAttachment string vrfCreated bool routesCreated int - vrfInstanceCreated bool - advCreated bool - k8s client.Client - namespace string - - // ebpfRegistered, ebpfBlock, and ebpfArgument mirror - // cnibgp.PublishResult's fields, recorded here so cleanup can call - // cnibgp.UnregisterEBPFDatapath for the same (block, argument) pair. - ebpfRegistered bool - ebpfBlock uint64 - ebpfArgument uint16 // ipamDelegated, ipamType, and ipamStdin record enough to release the // IPAM allocation during rollback. Set as soon as pluginConf.IPAM != nil @@ -87,60 +74,15 @@ type resourceTracker struct { // cleanup rolls back all tracked resources in reverse creation order. // Errors are logged but never returned — the caller already has a failure. -func (rt *resourceTracker) cleanup(ctx context.Context) { +// Takes no context: unlike before this split, the only non-kernel call left +// here is ipam.ExecDel, which (like ExecAdd) shells out to the delegated +// plugin binary rather than making a k8s API call (BGP CRD/eBPF rollback is +// galactic-bgp's own tracker now). +func (rt *resourceTracker) cleanup() { slog.Info("Selective rollback: cleaning up resources created during failed ADD", "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) - // 1. Delete BGPAdvertisement (withdraws prefixes) - if rt.advCreated && rt.k8s != nil { - adv := &bgpv1alpha1.BGPAdvertisement{ - ObjectMeta: metav1.ObjectMeta{ - Name: crdnames.BGPAdvertisementName(rt.vpc, rt.vpcAttachment), - Namespace: rt.namespace, - }, - } - if err := rt.k8s.Delete(ctx, adv); client.IgnoreNotFound(err) != nil { - slog.Error("Rollback: failed to delete BGPAdvertisement", "err", err, - "name", adv.Name, "namespace", rt.namespace) - } else { - slog.Debug("Rollback: deleted BGPAdvertisement", "name", adv.Name, "namespace", rt.namespace) - } - } - - // 2. Delete BGPVRFInstance - if rt.vrfInstanceCreated && rt.k8s != nil { - vrfInst := &bgpv1alpha1.BGPVRFInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: crdnames.BGPVRFInstanceName(rt.vpc, rt.vpcAttachment), - Namespace: rt.namespace, - }, - } - if err := rt.k8s.Delete(ctx, vrfInst); client.IgnoreNotFound(err) != nil { - slog.Error("Rollback: failed to delete BGPVRFInstance", "err", err, - "name", vrfInst.Name, "namespace", rt.namespace) - } else { - slog.Debug("Rollback: deleted BGPVRFInstance", "name", vrfInst.Name, "namespace", rt.namespace) - } - } - - // 3. Unregister the eBPF uSID datapath's vrf_table entry (only if - // PublishBGPState actually wrote one). A pinned BPF map entry has no - // implicit teardown when the VRF/interfaces are deleted below, so it - // must be removed explicitly here. - if rt.ebpfRegistered { - if vrfTableID, err := vrf.TableID(rt.vpc, rt.vpcAttachment); err != nil { - slog.Error("Rollback: failed to resolve VRF table id, skipping eBPF vrf_table unregister", "err", err, - "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) - } else if err := cnibgp.UnregisterEBPFDatapath(rt.ebpfBlock, rt.ebpfArgument, vrfTableID, attach.PinDir); err != nil { - slog.Error("Rollback: failed to unregister eBPF vrf_table entry", "err", err, - "block", rt.ebpfBlock, "argument", rt.ebpfArgument) - } else { - slog.Debug("Rollback: unregistered eBPF vrf_table entry", - "block", rt.ebpfBlock, "argument", rt.ebpfArgument) - } - } - - // 4. Release the IPAM allocation (if pluginConf carried an "ipam" block + // 1. Release the IPAM allocation (if pluginConf carried an "ipam" block // at all — see the ipamDelegated field doc comment for why this fires // unconditionally on that alone, not just after a confirmed ExecAdd). if rt.ipamDelegated { @@ -151,7 +93,7 @@ func (rt *resourceTracker) cleanup(ctx context.Context) { } } - // 5. Delete host veth + // 2. Delete host veth if err := veth.Delete(rt.vpc, rt.vpcAttachment); err != nil { slog.Error("Rollback: failed to delete veth", "err", err, "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) @@ -159,7 +101,7 @@ func (rt *resourceTracker) cleanup(ctx context.Context) { slog.Debug("Rollback: deleted veth", "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) } - // 6. Delete VRF (flushes all routes, removes VRF interface) + // 3. Delete VRF (flushes all routes, removes VRF interface) if err := vrf.Delete(rt.vpc, rt.vpcAttachment); err != nil { slog.Error("Rollback: failed to delete VRF", "err", err, "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) diff --git a/internal/cni/resource_ebpf_test.go b/internal/cni/resource_ebpf_test.go deleted file mode 100644 index 1482f7e6..00000000 --- a/internal/cni/resource_ebpf_test.go +++ /dev/null @@ -1,140 +0,0 @@ -// Copyright 2026 Datum Cloud, Inc. -// -// SPDX-License-Identifier: AGPL-3.0-or-later - -package cni - -import ( - "os" - "testing" - - "go.datum.net/galactic/internal/plumbing/ebpf/attach" - "go.datum.net/galactic/internal/plumbing/ebpf/usidmap" - "go.datum.net/galactic/internal/plumbing/vrf" -) - -// TestResourceTrackerCleanup_UnregistersEBPFVRFEntry: a failed ADD's rollback -// (resourceTracker.cleanup) cleans up both the kernel route (existing -// behavior, already covered by TestResourceTrackerCleanupPartialState) and -// the eBPF vrf_table map entry, when one was actually registered. cleanup's -// own unregister step always targets the real, production attach.PinDir (it -// is not parameterized), so this test loads/pins the real datapath there for -// the duration of the test, cleaning it up fully afterward. -// -// cleanup's unregister step recomputes this attachment's own VRF table id -// (vrf.TableID) and only deletes the vrf_table entry if it still resolves -// there, so a real VRF interface for (testVPC, testAttachment) must exist -// for the duration of this test. -func TestResourceTrackerCleanup_UnregistersEBPFVRFEntry(t *testing.T) { - requireRoot(t) - - if err := vrf.Add(testVPC, testAttachment); err != nil { - t.Fatalf("vrf.Add: %v", err) - } - t.Cleanup(func() { _ = vrf.Delete(testVPC, testAttachment) }) - vrfTableID, err := vrf.TableID(testVPC, testAttachment) - if err != nil { - t.Fatalf("vrf.TableID: %v", err) - } - - loaderObjs, err := attach.Load(attach.PinDir) - if err != nil { - t.Fatalf("attach.Load(attach.PinDir): %v", err) - } - t.Cleanup(func() { _ = loaderObjs.Close() }) - t.Cleanup(func() { _ = os.RemoveAll(attach.PinDir) }) - - reg, closer, err := usidmap.OpenPinnedRegistry(attach.PinDir) - if err != nil { - t.Fatalf("OpenPinnedRegistry(attach.PinDir): %v", err) - } - defer func() { _ = closer.Close() }() - - const testBlock uint64 = 0x0102030405 - const testArgument uint16 = 0x042 - - if err := reg.VRF.Register(testBlock, testArgument, vrfTableID, usidmap.EgressKindVeth); err != nil { - t.Fatalf("seed vrf_table entry: %v", err) - } - if _, ok, err := reg.VRF.Get(testBlock, testArgument); err != nil || !ok { - t.Fatalf("seeded entry not visible before cleanup: ok=%v err=%v", ok, err) - } - - tracker := &resourceTracker{ - vpc: testVPC, - vpcAttachment: testAttachment, - namespace: "ebpf-cleanup-test", - ebpfRegistered: true, - ebpfBlock: testBlock, - ebpfArgument: testArgument, - } - tracker.cleanup(t.Context()) - - if _, ok, err := reg.VRF.Get(testBlock, testArgument); err != nil || ok { - t.Errorf("vrf_table entry after cleanup: ok=%v err=%v, want ok=false (unregistered)", ok, err) - } -} - -// TestResourceTrackerCleanup_LeavesEBPFVRFEntryOwnedByAnotherAttachment -// covers the race this fix closes: retryK8sOps can re-run -// PublishBGPStateK8s's whole closure on a later attempt without -// re-registering the eBPF entry, so by the time a later attempt's -// checkArgumentCollision failure triggers this rollback, the (block, -// argument) slot this attachment originally wrote may have since been -// overwritten by the very other attachment the collision was detected -// against -- unregistering unconditionally would delete a live -// attachment's forwarding entry instead of this rolled-back one's own. If -// the current entry's VRFTableID no longer matches this attachment's own -// (recomputed fresh, not read from the tracker), cleanup must leave it in -// place. -func TestResourceTrackerCleanup_LeavesEBPFVRFEntryOwnedByAnotherAttachment(t *testing.T) { - requireRoot(t) - - if err := vrf.Add(testVPC, testAttachment); err != nil { - t.Fatalf("vrf.Add: %v", err) - } - t.Cleanup(func() { _ = vrf.Delete(testVPC, testAttachment) }) - - loaderObjs, err := attach.Load(attach.PinDir) - if err != nil { - t.Fatalf("attach.Load(attach.PinDir): %v", err) - } - t.Cleanup(func() { _ = loaderObjs.Close() }) - t.Cleanup(func() { _ = os.RemoveAll(attach.PinDir) }) - - reg, closer, err := usidmap.OpenPinnedRegistry(attach.PinDir) - if err != nil { - t.Fatalf("OpenPinnedRegistry(attach.PinDir): %v", err) - } - defer func() { _ = closer.Close() }() - - const testBlock uint64 = 0x0102030405 - const testArgument uint16 = 0x042 - const anotherAttachmentsVRFTableID uint32 = 0x9999 - - // Simulate the colliding attachment having since overwritten this same - // (block, argument) slot with its own, different VRF table id. - if err := reg.VRF.Register(testBlock, testArgument, anotherAttachmentsVRFTableID, usidmap.EgressKindVeth); err != nil { - t.Fatalf("seed vrf_table entry: %v", err) - } - - tracker := &resourceTracker{ - vpc: testVPC, - vpcAttachment: testAttachment, - namespace: "ebpf-cleanup-test", - ebpfRegistered: true, - ebpfBlock: testBlock, - ebpfArgument: testArgument, - } - tracker.cleanup(t.Context()) - - entry, ok, err := reg.VRF.Get(testBlock, testArgument) - if err != nil || !ok { - t.Fatalf("vrf_table entry after cleanup: ok=%v err=%v, want ok=true (must survive, it's not this attachment's)", - ok, err) - } - if entry.VRFTableID != anotherAttachmentsVRFTableID { - t.Errorf("vrf_table entry VRFTableID after cleanup = %#x, want unchanged %#x", - entry.VRFTableID, anotherAttachmentsVRFTableID) - } -} diff --git a/internal/cni/result.go b/internal/cni/result.go index 46f5eb60..0238cbe7 100644 --- a/internal/cni/result.go +++ b/internal/cni/result.go @@ -6,6 +6,7 @@ package cni import ( "fmt" + "log/slog" "net" "github.com/containernetworking/cni/pkg/skel" @@ -14,6 +15,7 @@ import ( "github.com/containernetworking/plugins/pkg/ipam" "github.com/vishvananda/netlink" + "go.datum.net/galactic/internal/cni/hostgw" "go.datum.net/galactic/internal/cniipam" ) @@ -79,15 +81,18 @@ func appendIPConfigs(result *type100.Result, ipRes *cniipam.IPAMResult, ifaceInd } // buildVethResult handles veth-specific result building: host-device -// delegation, IPAM, guest interface reading, and result printing. -// Returns the IPAM result for BGP advertisement, or nil if no IPAM. +// delegation, IPAM, host gateway configuration, guest interface reading, +// and result printing. galactic-bgp (chained next by the runtime) picks up +// everything it needs — the allocated addresses, and that this was a veth +// attachment — from the result this prints, not from a Go-level return +// value. func buildVethResult( args *skel.CmdArgs, pluginConf *PluginConf, hostName, guestName string, hostMac string, hostMTU int, -) (*cniipam.IPAMResult, net.HardwareAddr, error) { +) error { // Only call host-device ADD if the guest interface is still in the host // namespace. If a prior attempt already moved it to the container netns but // failed at a later step, we must not try to move it again. @@ -96,10 +101,10 @@ func buildVethResult( // previous run. The host-device plugin renames the moved interface // to args.IfName, so a prior run may have left that name behind. if err := cleanupContainerNetns(args.Netns, args.IfName); err != nil { - return nil, nil, fmt.Errorf("cleanup container netns: %w", err) + return fmt.Errorf("cleanup container netns: %w", err) } if err := hostDevice("ADD", args, pluginConf); err != nil { - return nil, nil, fmt.Errorf("host-device ADD: %w", err) + return fmt.Errorf("host-device ADD: %w", err) } } @@ -111,26 +116,40 @@ func buildVethResult( if pluginConf.IPAM != nil { result, err := configureIPAM(args, pluginConf, args.IfName) if err != nil { - return nil, nil, fmt.Errorf("configure IPAM: %w", err) + return fmt.Errorf("configure IPAM: %w", err) } ipamResult = result } + if ipamResult != nil { + slog.Debug("ADD: IPAM allocated", "containerID", args.ContainerID, + "ipv6Subnet", ipamResult.IPv6Subnet, "ipv6Gateway", ipamResult.IPv6Gateway, + "ipv4Address", ipamResult.IPv4Address, "ipv4Gateway", ipamResult.IPv4Gateway) + } // Read guest veth attributes inside the container netns. guestMac, guestMTU, err := readGuestInterface(args.Netns, args.IfName) if err != nil { - return nil, nil, fmt.Errorf("read guest interface: %w", err) + return fmt.Errorf("read guest interface: %w", err) } guestHWAddr, err := net.ParseMAC(guestMac) if err != nil { - return nil, nil, fmt.Errorf("parse guest interface MAC %q: %w", guestMac, err) + return fmt.Errorf("parse guest interface MAC %q: %w", guestMac, err) + } + + // Configure the host-side gateway address and VRF route before printing + // the result — kernel-interface work this plugin owns (see + // internal/cni/hostgw's doc comment for why galactic-bgp no longer does + // this itself). + if err := hostgw.ConfigureHostGateway(pluginConf.VPC, pluginConf.VPCAttachment, ipamResult, guestHWAddr); err != nil { + return fmt.Errorf("configure host gateway: %w", err) } + result := buildResult(pluginConf, ipamResult, hostName, args.IfName, hostMac, guestMac, hostMTU, guestMTU, args.Netns) if err := types.PrintResult(result, pluginConf.CNIVersion); err != nil { - return nil, nil, fmt.Errorf("print CNI result: %w", err) + return fmt.Errorf("print CNI result: %w", err) } - return ipamResult, guestHWAddr, nil + return nil } // configureIPAM delegates IPAM allocation to whatever binary pluginConf's diff --git a/internal/cnibgp/bgp.go b/internal/cnibgp/bgp.go index 477ccc0f..2a01a02e 100644 --- a/internal/cnibgp/bgp.go +++ b/internal/cnibgp/bgp.go @@ -2,19 +2,14 @@ // // SPDX-License-Identifier: AGPL-3.0-or-later -// Package cnibgp holds the BGP/SRv6/eBPF publish logic shared by every -// master plugin in the galactic CNI chain (galactic-cni, veth; -// galactic-tap-cni, tap) — interface-agnostic aside from -// EgressKindForInterfaceType's eBPF egress_kind lookup. -// -// Like internal/cniipam, this is a plain library today, imported directly by -// the master plugins rather than a chain-invoked plugin of its own — that -// lands in a follow-up step (cmd/galactic-bgp, new CHECK logic for the CRDs/ -// eBPF state this package writes, and replacing PublishConfig.InterfaceType -// with an inference from prevResult.interfaces[] shape so no config field -// carries it at all). Callers pass in a k8s client already scoped to a -// scheme that includes go.datum.net/network/api/v1alpha1 — this package -// never builds its own client. +// Package cnibgp implements galactic-bgp, the SRv6/BGP/eBPF publish plugin +// in the galactic CNI chain. It is chain-invoked (per CNI conflist order) +// after the master plugin (galactic-cni or galactic-tap-cni), not called +// as a library — it has zero kernel-interface dependency: every address it +// advertises comes from prevResult (see prevresult.go), never from a +// runtime call into the interface it doesn't own. Host-interface gateway +// configuration lives in internal/cni/hostgw instead, called directly by +// the master plugins, for exactly this reason. package cnibgp import ( @@ -22,17 +17,13 @@ import ( "errors" "fmt" "log/slog" - "net" "net/netip" "sort" "strconv" "strings" - "syscall" "time" "github.com/containernetworking/cni/pkg/skel" - "github.com/vishvananda/netlink" - "golang.org/x/sys/unix" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -40,57 +31,47 @@ import ( "go.datum.net/galactic/internal/cni/crdnames" "go.datum.net/galactic/internal/cniipam" - "go.datum.net/galactic/internal/plumbing/ebpf/attach" "go.datum.net/galactic/internal/plumbing/ebpf/uformat" "go.datum.net/galactic/internal/plumbing/ebpf/usidmap" - "go.datum.net/galactic/internal/plumbing/intf" "go.datum.net/galactic/internal/plumbing/vrf" bgpv1alpha1 "go.datum.net/network/api/v1alpha1" ) -// cniTimeout bounds each individual k8s API retry attempt in -// PublishBGPStateK8s. -const cniTimeout = 10 * time.Second - // maxRetries is the maximum number of retry attempts for transient k8s API // errors during the BGP state publish phase. The total number of attempts // is maxRetries+1 (initial + retries). const maxRetries = 2 -// ifaceTypeVeth and ifaceTypeTap are the two values PublishConfig.InterfaceType -// accepts. Duplicated here (rather than imported) since they're plain -// protocol-level string literals every caller already knows independently. +// ifaceTypeVeth and ifaceTypeTap are the two values publishConfig.ifaceType +// accepts, inferred from prevResult (see prevresult.go) rather than a +// config field. const ( ifaceTypeVeth = "veth" ifaceTypeTap = "tap" ) -// PublishConfig carries the subset of a caller's own CNI config that BGP -// publish needs. Each master plugin passes its own values in. -type PublishConfig struct { - VPC string - VPCAttachment string - // InterfaceType selects the eBPF vrf_table egress_kind (veth vs tap) — - // see EgressKindForInterfaceType. A follow-up step replaces this with an - // inference from prevResult.interfaces[] shape instead of a config field. - InterfaceType string +// publishConfig carries the subset of this plugin's own config that +// publishing needs. +type publishConfig struct { + vpc, vpcAttachment string + // ifaceType selects the eBPF vrf_table egress_kind (veth vs tap) — see + // egressKindForInterfaceType. Inferred from prevResult, never a config + // field. + ifaceType string } -// PublishResult records what PublishBGPState/PublishBGPStateK8s actually -// created, so callers can fold it into their own rollback bookkeeping — the -// rollback for a partially-failed ADD still belongs to whichever master -// plugin's ADD is failing, even though the BGP publish logic itself lives -// here. -type PublishResult struct { - VRFInstanceCreated bool - AdvertisementCreated bool - // EBPFRegistered, EBPFBlock, and EBPFArgument record the eBPF uSID +// publishResult records what publishBGPState actually created, so cmdAdd +// can fold it into its own rollback tracker. +type publishResult struct { + vrfInstanceCreated bool + advertisementCreated bool + // ebpfRegistered, ebpfBlock, and ebpfArgument record the eBPF uSID // datapath's vrf_table registration, if one actually happened (the - // BGPRouter may not be configured, in which case EBPFRegistered stays - // false). See UnregisterEBPFDatapath for rolling this back. - EBPFRegistered bool - EBPFBlock uint64 - EBPFArgument uint16 + // BGPRouter may not be configured, in which case ebpfRegistered stays + // false). See unregisterEBPFDatapath for rolling this back. + ebpfRegistered bool + ebpfBlock uint64 + ebpfArgument uint16 } // isTransientError reports whether err is a transient failure that may @@ -290,34 +271,11 @@ func buildAdvertisementSpec( } } -// PublishBGPState configures the host gateway, sets up the SRv6 ingress -// route, and creates the BGPVRFInstance and BGPAdvertisement CRDs. The host -// gateway configuration is interface-agnostic (works for both veth and -// tap) — callers that already invoked ConfigureHostGateway themselves -// (tap mode, which needs the gateway configured before printing its own CNI -// result) should call PublishBGPStateK8s directly instead, to avoid -// configuring it twice. -func PublishBGPState( - args *skel.CmdArgs, cfg PublishConfig, nodeName, namespace string, ipamResult *cniipam.IPAMResult, - guestHWAddr net.HardwareAddr, k8s client.Client, -) (PublishResult, error) { - if err := ConfigureHostGateway(cfg.VPC, cfg.VPCAttachment, ipamResult, guestHWAddr); err != nil { - return PublishResult{}, err - } - - vpcHex, err := intf.Base62ToHex(cfg.VPC) - if err != nil { - return PublishResult{}, fmt.Errorf("decode VPC: %w", err) - } - - return PublishBGPStateK8s(args, cfg, nodeName, namespace, ipamResult, vpcHex, k8s) -} - // ipamAdvertisementPrefixes derives the BGPAdvertisement prefixes to // originate, plus the per-family values to record in the annotations, from -// ipamResult. ipamResult is nil when the attachment has no IPAM allocation -// (e.g. a tap workload that manages its own addressing), in which case -// prefixes is empty. +// ipamResult (reconstructed from prevResult — see prevresult.go). ipamResult +// is nil when the attachment has no IPAM allocation (e.g. a tap workload +// that manages its own addressing), in which case prefixes is empty. func ipamAdvertisementPrefixes(ipamResult *cniipam.IPAMResult) (prefixes []string, ipv6Subnet, ipv4Addr string) { if ipamResult == nil { return nil, "", "" @@ -352,15 +310,15 @@ func allAdvertisedPrefixes(annotations map[string]string) []string { return prefixes } -// PublishBGPStateK8s creates the BGPVRFInstance and BGPAdvertisement CRDs -// with retry on transient k8s API errors. The host gateway must already be -// configured before calling this (via ConfigureHostGateway, or PublishBGPState -// which calls it first). -func PublishBGPStateK8s( - args *skel.CmdArgs, cfg PublishConfig, nodeName, namespace string, ipamResult *cniipam.IPAMResult, +// publishBGPState creates the BGPVRFInstance and BGPAdvertisement CRDs and +// registers the eBPF uSID datapath entry, with retry on transient k8s API +// errors. Assumes the host gateway is already configured (internal/cni/ +// hostgw, called by the master plugin before this ever runs). +func publishBGPState( + args *skel.CmdArgs, cfg publishConfig, nodeName, namespace string, ipamResult *cniipam.IPAMResult, vpcHex string, k8s client.Client, -) (PublishResult, error) { - var result PublishResult +) (publishResult, error) { + var result publishResult err := retryK8sOps(cniTimeout, func(ctx context.Context) error { bgp, err := lookupBGPRouter(ctx, k8s, nodeName, namespace) if err != nil { @@ -368,7 +326,7 @@ func PublishBGPStateK8s( } vrfID, err := allocateArgument( - ctx, k8s, namespace, bgp.routerName, crdnames.BGPVRFInstanceName(cfg.VPC, cfg.VPCAttachment)) + ctx, k8s, namespace, bgp.routerName, crdnames.BGPVRFInstanceName(cfg.vpc, cfg.vpcAttachment)) if err != nil { return err } @@ -378,7 +336,7 @@ func PublishBGPStateK8s( return fmt.Errorf("compute route target: %w", err) } - vrfName := crdnames.BGPVRFInstanceName(cfg.VPC, cfg.VPCAttachment) + vrfName := crdnames.BGPVRFInstanceName(cfg.vpc, cfg.vpcAttachment) vrfInst := &bgpv1alpha1.BGPVRFInstance{ ObjectMeta: metav1.ObjectMeta{ Name: vrfName, @@ -392,7 +350,7 @@ func PublishBGPStateK8s( if err != nil { return fmt.Errorf("apply BGPVRFInstance: %w", err) } - result.VRFInstanceCreated = true + result.vrfInstanceCreated = true slog.Debug("BGP: BGPVRFInstance applied", "name", vrfName, "namespace", namespace, "vrfID", vrfID, "routeTarget", rtValue, "router", bgp.routerName) @@ -401,19 +359,19 @@ func PublishBGPStateK8s( } registered, ebpfBlock, err := registerEBPFDatapath( - bgp, cfg.VPC, cfg.VPCAttachment, cfg.InterfaceType, uint16(vrfID), attach.PinDir) + bgp, cfg.vpc, cfg.vpcAttachment, cfg.ifaceType, uint16(vrfID), ebpfPinDir) if err != nil { return fmt.Errorf("register eBPF uSID datapath: %w", err) } if registered { - result.EBPFRegistered = true - result.EBPFBlock = ebpfBlock - result.EBPFArgument = uint16(vrfID) + result.ebpfRegistered = true + result.ebpfBlock = ebpfBlock + result.ebpfArgument = uint16(vrfID) } adv := &bgpv1alpha1.BGPAdvertisement{ ObjectMeta: metav1.ObjectMeta{ - Name: crdnames.BGPAdvertisementName(cfg.VPC, cfg.VPCAttachment), + Name: crdnames.BGPAdvertisementName(cfg.vpc, cfg.vpcAttachment), Namespace: namespace, }, } @@ -437,165 +395,17 @@ func PublishBGPStateK8s( if err != nil { return fmt.Errorf("apply BGPAdvertisement: %w", err) } - result.AdvertisementCreated = true + result.advertisementCreated = true slog.Debug("BGP: BGPAdvertisement applied", "name", adv.Name, "namespace", namespace, "prefixes", mergedPrefixes, "addedPrefixes", prefixes, "containerID", args.ContainerID) slog.Info("ADD: BGP state published", "containerID", args.ContainerID, - "vpc", cfg.VPC, "vpcAttachment", cfg.VPCAttachment) + "vpc", cfg.vpc, "vpcAttachment", cfg.vpcAttachment) return nil }) return result, err } -// routeConflicts reports whether an existing route conflicts with the desired -// pod-subnet route. A conflict occurs when the destination matches but the -// gateway or link index differs. -func routeConflicts(existing, desired *netlink.Route) bool { - if existing.Dst == nil || desired.Dst == nil { - return false - } - if existing.Dst.String() != desired.Dst.String() { - return false - } - if (existing.Gw != nil) != (desired.Gw != nil) { - return true - } - if existing.Gw != nil && !existing.Gw.Equal(desired.Gw) { - return true - } - if existing.LinkIndex != 0 && desired.LinkIndex != 0 && existing.LinkIndex != desired.LinkIndex { - return true - } - return false -} - -// ConfigureHostGateway assigns each configured family's gateway address as a -// host address (/128 for IPv6, /32 for IPv4 on veth) on the host-side -// interface (veth or tap) and installs an explicit pod-subnet route for that -// family into the VRF table. IPv4 is skipped entirely when the attachment is -// IPv6-only. guestHWAddr is nil for tap attachments. -func ConfigureHostGateway(vpc, vpcAttachment string, res *cniipam.IPAMResult, guestHWAddr net.HardwareAddr) error { - if res == nil { - return nil - } - hostName := intf.GenerateInterfaceNameHost(vpc, vpcAttachment) - hostLink, err := netlink.LinkByName(hostName) - if err != nil { - return fmt.Errorf("get host interface %q: %w", hostName, err) - } - tableID, err := vrf.TableID(vpc, vpcAttachment) - if err != nil { - return fmt.Errorf("get VRF table ID for pod subnet route: %w", err) - } - - if res.IPv6Gateway != nil { - gwNet := &net.IPNet{IP: res.IPv6Gateway, Mask: net.CIDRMask(128, 128)} - if err := installGatewayRoute(hostLink, gwNet, res.IPv6Subnet, netlink.FAMILY_V6, int(tableID), 0); err != nil { - return err - } - if guestHWAddr != nil { - if err := installGatewayNeighbor(hostLink, res.IPv6Subnet.IP, netlink.FAMILY_V6, guestHWAddr); err != nil { - return err - } - } - } - if res.IPv4Gateway != nil { - ipv4Mask, addrFlags := ipv4GatewayAddrParams(hostLink) - gwNet := &net.IPNet{IP: res.IPv4Gateway, Mask: ipv4Mask} - ipv4Subnet := &net.IPNet{IP: res.IPv4Address, Mask: net.CIDRMask(32, 32)} - if err := installGatewayRoute(hostLink, gwNet, ipv4Subnet, netlink.FAMILY_V4, int(tableID), addrFlags); err != nil { - return err - } - if guestHWAddr != nil { - if err := installGatewayNeighbor(hostLink, res.IPv4Address, netlink.FAMILY_V4, guestHWAddr); err != nil { - return err - } - } - } - return nil -} - -// installGatewayNeighbor installs a permanent neighbor table entry mapping -// podIP to guestHWAddr on hostLink — see the design plan's note on why the -// eBPF ingress datapath needs this pre-installed rather than relying on -// dynamic ARP/NDP. -func installGatewayNeighbor(hostLink netlink.Link, podIP net.IP, family int, guestHWAddr net.HardwareAddr) error { - neigh := &netlink.Neigh{ - LinkIndex: hostLink.Attrs().Index, - Family: family, - State: netlink.NUD_PERMANENT, - IP: podIP, - HardwareAddr: guestHWAddr, - } - if err := netlink.NeighSet(neigh); err != nil { - return fmt.Errorf("add permanent neighbor %s -> %s on host interface %q: %w", - podIP, guestHWAddr, hostLink.Attrs().Name, err) - } - return nil -} - -// ipv4GatewayAddrParams returns the IPv4 gateway mask and netlink address -// flags to use for hostLink. Tap interfaces get a /25 with -// IFA_F_NOPREFIXROUTE; veth interfaces keep the plain /32 host address with -// no flags. -func ipv4GatewayAddrParams(hostLink netlink.Link) (net.IPMask, int) { - if _, isTap := hostLink.(*netlink.Tuntap); isTap { - return net.CIDRMask(25, 32), unix.IFA_F_NOPREFIXROUTE - } - return net.CIDRMask(32, 32), 0 -} - -// installGatewayRoute assigns gwNet as a host address on hostLink and -// installs an explicit route to subnet into the given VRF table, for one -// address family. Idempotent. -func installGatewayRoute(hostLink netlink.Link, gwNet, subnet *net.IPNet, family, tableID, addrFlags int) error { - hostName := hostLink.Attrs().Name - if err := netlink.AddrAdd(hostLink, &netlink.Addr{IPNet: gwNet, Flags: addrFlags}); err != nil { - if !errors.Is(err, syscall.EEXIST) { - return fmt.Errorf("add gateway address %s to host interface %q: %w", gwNet, hostName, err) - } - } - - desiredRoute := &netlink.Route{ - Dst: subnet, - LinkIndex: hostLink.Attrs().Index, - Table: tableID, - } - - existingRoutes, err := netlink.RouteListFiltered( - family, - &netlink.Route{Table: tableID}, - netlink.RT_FILTER_TABLE, - ) - if err != nil { - return fmt.Errorf("list routes in VRF table: %w", err) - } - for _, r := range existingRoutes { - if r.Dst == nil { - continue - } - if r.Dst.String() != desiredRoute.Dst.String() { - continue - } - if routeConflicts(&r, desiredRoute) { - return fmt.Errorf( - "existing route %v to %s conflicts with desired route %v", - r, desiredRoute.Dst, desiredRoute, - ) - } - return nil - } - - if err := netlink.RouteAdd(desiredRoute); err != nil { - if errors.Is(err, syscall.EEXIST) { - return nil - } - return fmt.Errorf("add pod subnet route to VRF table: %w", err) - } - return nil -} - // registerEBPFDatapath registers this attachment against the eBPF uSID // datapath's pinned maps. registered is false, with a nil error, only when // this router has no srv6Locator/nodeID configured at all — SRv6 is @@ -613,7 +423,7 @@ func registerEBPFDatapath( bgp.nodeID, uint16(uformat.NodeIDMin), uint16(uformat.NodeIDMax)) } - egressKind, err := EgressKindForInterfaceType(ifaceType) + egressKind, err := egressKindForInterfaceType(ifaceType) if err != nil { return false, 0, fmt.Errorf("determine eBPF egress kind: %w", err) } @@ -651,13 +461,13 @@ func registerEBPFDatapath( return true, block, nil } -// EgressKindForInterfaceType maps a "veth"/"tap" interface type string to the +// egressKindForInterfaceType maps a "veth"/"tap" interface type string to the // vrf_table egress_kind value usid.c's step 9 uses to pick between // bpf_redirect_peer (veth, crosses into the container's netns) and plain // bpf_redirect (tap, which never leaves this netns). -func EgressKindForInterfaceType(ifaceType string) (uint32, error) { +func egressKindForInterfaceType(ifaceType string) (uint32, error) { switch ifaceType { - case ifaceTypeVeth, "": + case ifaceTypeVeth: return usidmap.EgressKindVeth, nil case ifaceTypeTap: return usidmap.EgressKindTap, nil @@ -666,16 +476,15 @@ func EgressKindForInterfaceType(ifaceType string) (uint32, error) { } } -// UnregisterEBPFDatapath removes the vrf_table entry registerEBPFDatapath -// wrote for this (block, argument) pair, from a caller's failed-ADD -// rollback path. Idempotent: not an error if the entry is already gone. +// unregisterEBPFDatapath removes the vrf_table entry registerEBPFDatapath +// wrote for this (block, argument) pair, from cmdAdd's failed-ADD rollback +// path. Idempotent: not an error if the entry is already gone. // // expectedVRFTableID must be this attachment's own VRF table id (recomputed // by the caller via vrf.TableID). Only deletes the entry when it still -// resolves to expectedVRFTableID, and leaves it alone otherwise — see the -// original resourceTracker.cleanup's doc comment for the race this guards -// against. -func UnregisterEBPFDatapath(block uint64, argument uint16, expectedVRFTableID uint32, pinDir string) error { +// resolves to expectedVRFTableID, and leaves it alone otherwise — see +// resourceTracker.cleanup's doc comment for the race this guards against. +func unregisterEBPFDatapath(block uint64, argument uint16, expectedVRFTableID uint32, pinDir string) error { registry, closer, err := usidmap.OpenPinnedRegistry(pinDir) if err != nil { return fmt.Errorf("open pinned eBPF uSID maps: %w", err) diff --git a/internal/cnibgp/bgp_ebpf_test.go b/internal/cnibgp/bgp_ebpf_test.go index 7d97f1e4..4d34838e 100644 --- a/internal/cnibgp/bgp_ebpf_test.go +++ b/internal/cnibgp/bgp_ebpf_test.go @@ -179,7 +179,7 @@ func TestUnregisterEBPFDatapath_RemovesOwnEntry(t *testing.T) { t.Fatalf("seed vrf_table entry: %v", err) } - if err := UnregisterEBPFDatapath(testBlock, testArgument, vrfTableID, pinDir); err != nil { + if err := unregisterEBPFDatapath(testBlock, testArgument, vrfTableID, pinDir); err != nil { t.Fatalf("UnregisterEBPFDatapath: %v", err) } @@ -225,7 +225,7 @@ func TestUnregisterEBPFDatapath_LeavesEntryOwnedByAnotherAttachment(t *testing.T t.Fatalf("seed vrf_table entry: %v", err) } - if err := UnregisterEBPFDatapath(testBlock, testArgument, thisAttachmentsVRFTableID, pinDir); err != nil { + if err := unregisterEBPFDatapath(testBlock, testArgument, thisAttachmentsVRFTableID, pinDir); err != nil { t.Fatalf("UnregisterEBPFDatapath: %v", err) } diff --git a/internal/cnibgp/bgp_test.go b/internal/cnibgp/bgp_test.go index 1ab67fc7..322e47c9 100644 --- a/internal/cnibgp/bgp_test.go +++ b/internal/cnibgp/bgp_test.go @@ -14,8 +14,6 @@ import ( "testing" "time" - "github.com/vishvananda/netlink" - "golang.org/x/sys/unix" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -95,103 +93,6 @@ func vrfInstanceForRouter(name, namespace, routerName string, vrfID int32) *bgpv } } -// ---- ipv4GatewayAddrParams ------------------------------------------------ - -func TestIPv4GatewayAddrParams(t *testing.T) { - tests := []struct { - name string - hostLink netlink.Link - wantMask net.IPMask - wantFlags int - }{ - { - name: "tap gets /25 with NOPREFIXROUTE", - hostLink: &netlink.Tuntap{LinkAttrs: netlink.LinkAttrs{Name: "tap0"}}, - wantMask: net.CIDRMask(25, 32), - wantFlags: unix.IFA_F_NOPREFIXROUTE, - }, - { - name: "veth gets /32 with no flags", - hostLink: &netlink.Veth{LinkAttrs: netlink.LinkAttrs{Name: "veth0"}}, - wantMask: net.CIDRMask(32, 32), - wantFlags: 0, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotMask, gotFlags := ipv4GatewayAddrParams(tt.hostLink) - if gotMask.String() != tt.wantMask.String() { - t.Errorf("mask = %v, want %v", gotMask, tt.wantMask) - } - if gotFlags != tt.wantFlags { - t.Errorf("flags = %v, want %v", gotFlags, tt.wantFlags) - } - }) - } -} - -// ---- routeConflicts ------------------------------------------------------ - -func TestRouteConflicts(t *testing.T) { - dst := mustParseCIDR(t, "fd00:10:ff01::1234/80") - gw1 := net.ParseIP("fd00:10:ff01::1") - gw2 := net.ParseIP("fd00:10:ff01::2") - otherDst := mustParseCIDR(t, "fd00:10:ff02::1234/80") - - tests := []struct { - name string - existing *netlink.Route - desired *netlink.Route - want bool - }{ - {"nil existing destination — no conflict", &netlink.Route{Dst: nil}, &netlink.Route{Dst: dst}, false}, - {"nil desired destination — no conflict", &netlink.Route{Dst: dst}, &netlink.Route{Dst: nil}, false}, - {"different destinations — no conflict", &netlink.Route{Dst: otherDst}, &netlink.Route{Dst: dst}, false}, - { - "same destination, no gateway on either — no conflict", - &netlink.Route{Dst: dst, LinkIndex: 5}, &netlink.Route{Dst: dst, LinkIndex: 5}, false, - }, - { - "same destination, same gateway — no conflict", - &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, false, - }, - { - "same destination, different gateway — conflict", - &netlink.Route{Dst: dst, Gw: gw1}, &netlink.Route{Dst: dst, Gw: gw2}, true, - }, - { - "existing has gateway, desired does not — conflict", - &netlink.Route{Dst: dst, Gw: gw1}, &netlink.Route{Dst: dst}, true, - }, - { - "desired has gateway, existing does not — conflict", - &netlink.Route{Dst: dst}, &netlink.Route{Dst: dst, Gw: gw1}, true, - }, - { - "same destination, same gateway, different link index — conflict", - &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 7}, true, - }, - { - "same destination, gateway set, link index zero on existing — no conflict", - &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 0}, &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, false, - }, - { - "same destination, no gateway, different link index — conflict", - &netlink.Route{Dst: dst, LinkIndex: 5}, &netlink.Route{Dst: dst, LinkIndex: 7}, true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := routeConflicts(tt.existing, tt.desired) - if got != tt.want { - t.Errorf("routeConflicts() = %v, want %v", got, tt.want) - } - }) - } -} - // ---- allocateArgument ------------------------------------------------------ func TestAllocateArgument(t *testing.T) { @@ -316,7 +217,7 @@ func TestCheckArgumentCollision(t *testing.T) { }) } -// ---- EgressKindForInterfaceType -------------------------------------------- +// ---- egressKindForInterfaceType -------------------------------------------- func TestEgressKindForInterfaceType(t *testing.T) { tests := []struct { @@ -326,25 +227,25 @@ func TestEgressKindForInterfaceType(t *testing.T) { wantErr bool }{ {name: "veth maps to EgressKindVeth", iface: ifaceTypeVeth, want: usidmap.EgressKindVeth}, - {name: "empty defaults to EgressKindVeth", iface: "", want: usidmap.EgressKindVeth}, {name: "tap maps to EgressKindTap", iface: ifaceTypeTap, want: usidmap.EgressKindTap}, + {name: "empty type errors", iface: "", wantErr: true}, {name: "unknown type errors", iface: "bogus", wantErr: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := EgressKindForInterfaceType(tt.iface) + got, err := egressKindForInterfaceType(tt.iface) if tt.wantErr { if err == nil { - t.Fatalf("EgressKindForInterfaceType(%q) error = nil, want error", tt.iface) + t.Fatalf("egressKindForInterfaceType(%q) error = nil, want error", tt.iface) } return } if err != nil { - t.Fatalf("EgressKindForInterfaceType(%q) unexpected error: %v", tt.iface, err) + t.Fatalf("egressKindForInterfaceType(%q) unexpected error: %v", tt.iface, err) } if got != tt.want { - t.Errorf("EgressKindForInterfaceType(%q) = %d, want %d", tt.iface, got, tt.want) + t.Errorf("egressKindForInterfaceType(%q) = %d, want %d", tt.iface, got, tt.want) } }) } diff --git a/internal/cnibgp/cnibgp.go b/internal/cnibgp/cnibgp.go new file mode 100644 index 00000000..20384b4c --- /dev/null +++ b/internal/cnibgp/cnibgp.go @@ -0,0 +1,42 @@ +// Copyright 2025 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnibgp + +import ( + "time" + + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/version" + + "go.datum.net/galactic/internal/metadata" + "go.datum.net/galactic/internal/plumbing/ebpf/attach" +) + +const cniTimeout = 10 * time.Second + +// ebpfPinDir is the bpffs directory this package's own eBPF registration +// (registerEBPFDatapath, bgp.go), rollback (resourceTracker.cleanup, +// resource.go), and CHECK (checkEBPFEntry, ops_check.go) all read/write +// pinned uSID maps under. A package var defaulting to attach.PinDir, rather +// than every call site reading that constant directly, so tests can point +// it at a throwaway pin directory instead of the real production one — +// attach.PinDir being a const otherwise gives production callers no seam +// for that (see resource_test.go's resourceTracker.cleanup tests). +var ebpfPinDir = attach.PinDir + +// RunPlugin starts galactic-bgp, handling the CNI ADD, DEL, CHECK, and +// STATUS operations for the BGP/SRv6/eBPF publish stage of the chain. +func RunPlugin() { + skel.PluginMainFuncs( + skel.CNIFuncs{ + Add: cmdAdd, + Check: cmdCheck, + Del: cmdDel, + Status: cmdStatus, + }, + version.All, + "CNI galactic-bgp plugin "+metadata.Version, + ) +} diff --git a/internal/cnibgp/config.go b/internal/cnibgp/config.go new file mode 100644 index 00000000..f9e11e6d --- /dev/null +++ b/internal/cnibgp/config.go @@ -0,0 +1,252 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnibgp + +import ( + "encoding/json" + "errors" + "fmt" + "log/slog" + "os" + "path/filepath" + "strings" + + "github.com/containernetworking/cni/pkg/types" + type100 "github.com/containernetworking/cni/pkg/types/100" + + "go.datum.net/galactic/internal/cni/hostconf" + "go.datum.net/galactic/internal/config" +) + +var ConfFile = config.DefaultConfFile + +// cniConfig is the shared config resolver for env var resolution. +// Initialized by InitCNIConfig() (called from cmd/galactic-bgp/main.go). +// +// Reuses internal/config.CNIConfig (the GALACTIC_CNI_* env var names) as-is +// rather than defining a GALACTIC_BGP_* set: node_name/kubeconfig/namespace +// are shared node-level settings, not domain-specific behavior the way +// galactic-ipam's own enable-local-ipam flag is — every binary in the chain +// resolves them from the same static conflist file (see +// go.datum.net/galactic/internal/cni/hostconf's doc comment). +var cniConfig *config.CNIConfig + +// InitCNIConfig initializes the shared config resolver for CNI env var +// resolution. Callers should invoke this once at process startup before any +// config lookups. +func InitCNIConfig() { + cniConfig = config.NewCNIConfig() +} + +const errInvalidCNIConfig = "invalid CNI config" + +const ( + errVPCRequired = "vpc is required and must be a non-empty base62 string" + errVPCAttachmentRequired = "vpcattachment is required and must be a non-empty base62 string" +) + +const sanitizeForErrorBinary = "" + +// isValidBase62 reports whether s contains only valid base62 characters +// ([0-9a-zA-Z]) and is non-empty. +func isValidBase62(s string) bool { + if len(s) == 0 { + return false + } + for _, c := range s { + if (c < '0' || c > '9') && (c < 'a' || c > 'z') && (c < 'A' || c > 'Z') { + return false + } + } + return true +} + +// loadHostConf loads node-local settings from the static per-node conflist. +// If the file is missing, it returns a zero-value HostConf (tolerating local +// test runs) but still defaulting Namespace to config.DefaultNamespace. +func loadHostConf(filePath string) (*HostConf, error) { + if filePath == "" { + filePath = config.DefaultConfFile + } + conf, err := hostconf.Load(filePath, hostconf.PluginType) + if err != nil { + if os.IsNotExist(unwrapPathError(err)) { + return &HostConf{Namespace: config.DefaultNamespace}, nil + } + return nil, err + } + if conf.Namespace == "" { + conf.Namespace = config.DefaultNamespace + } + return conf, nil +} + +// unwrapPathError returns the innermost error wrapped by err, so +// os.IsNotExist (which does not itself traverse %w wrapping) can still +// recognize a missing conflist file wrapped by hostconf.Load. +func unwrapPathError(err error) error { + for { + unwrapped := errors.Unwrap(err) + if unwrapped == nil { + return err + } + err = unwrapped + } +} + +// parseLogLevel maps a config-supplied level name to a slog.Level. +func parseLogLevel(s string) (slog.Level, error) { + switch strings.ToLower(strings.TrimSpace(s)) { + case "": + return parseLogLevel(config.DefaultLogLevel) + case config.LogLevelDebug: + return slog.LevelDebug, nil + case config.DefaultLogLevel: + return slog.LevelInfo, nil + case config.LogLevelWarn, config.LogLevelWarning: + return slog.LevelWarn, nil + case config.LogLevelError: + return slog.LevelError, nil + default: + return slog.LevelInfo, fmt.Errorf("unknown log level %q (want %s, %s, %s, or %s)", + s, config.LogLevelDebug, config.DefaultLogLevel, config.LogLevelWarn, config.LogLevelError) + } +} + +// setupLogging configures the slog default logger to write to the specified +// path at the specified verbosity. +func setupLogging(logPath, logLevel string) { + if logPath == "" { + logPath = config.DefaultLogFile + } + level, err := parseLogLevel(logLevel) + if err != nil { + slog.Warn("Invalid log level, falling back to default", + "value", logLevel, "default", config.DefaultLogLevel, "err", err) + } + if err := os.MkdirAll(filepath.Dir(logPath), 0755); err != nil { + slog.Warn("Failed to create log directory", "path", filepath.Dir(logPath), "err", err) + return + } + file, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644) + if err != nil { + slog.Warn("Failed to open log file, falling back to Stderr", "path", logPath, "err", err) + return + } + handler := slog.NewJSONHandler(file, &slog.HandlerOptions{Level: level}) + slog.SetDefault(slog.New(handler)) +} + +// statusConf holds the minimal CNI config fields needed for STATUS validation. +type statusConf struct { + CNIVersion string `json:"cniVersion"` + Type string `json:"type"` +} + +func parseStatusConf(data []byte) error { + var sc statusConf + if err := json.Unmarshal(data, &sc); err != nil { + return &types.Error{Code: 7, Msg: errInvalidCNIConfig, Details: err.Error()} + } + if sc.CNIVersion == "" { + return &types.Error{Code: 7, Msg: "cniVersion is required"} + } + if sc.Type == "" { + return &types.Error{Code: 7, Msg: "type is required"} + } + return nil +} + +// parseConf unmarshals the CNI configuration from stdin data (the same +// document the master plugin received), validates the base62-encoded +// identifier fields, and resolves node-level settings and logging. +func parseConf(data []byte) (*PluginConf, error) { + conf := &PluginConf{} + if err := json.Unmarshal(data, &conf); err != nil { + return nil, &types.Error{Code: 7, Msg: errInvalidCNIConfig, Details: err.Error()} + } + if !isValidBase62(conf.VPC) { + if len(conf.VPC) == 0 { + return nil, &types.Error{Code: 7, Msg: errVPCRequired} + } + return nil, &types.Error{ + Code: 7, + Msg: fmt.Sprintf("invalid base62 value for field 'vpc': %q", sanitizeForError(conf.VPC)), + } + } + if !isValidBase62(conf.VPCAttachment) { + if len(conf.VPCAttachment) == 0 { + return nil, &types.Error{Code: 7, Msg: errVPCAttachmentRequired} + } + return nil, &types.Error{ + Code: 7, + Msg: fmt.Sprintf("invalid base62 value for field 'vpcattachment': %q", sanitizeForError(conf.VPCAttachment)), + } + } + + hostConf, err := loadHostConf(ConfFile) + if err != nil { + return nil, fmt.Errorf("load host CNI config: %w", err) + } + + cniConfig.Resolve(&config.ConflistValues{ + NodeName: hostConf.NodeName, + Kubeconfig: hostConf.Kubeconfig, + Namespace: hostConf.Namespace, + LogFile: hostConf.LogFile, + LogLevel: hostConf.LogLevel, + }) + + if cniConfig.NodeName == "" { + detected, detectErr := hostconf.DetectNodeNameFromAPI() + if detectErr != nil { + slog.Warn("Node name auto-detection failed", "err", detectErr) + } + cniConfig.NodeName = detected + } + if cniConfig.NodeName == "" { + return nil, &types.Error{Code: 4, Msg: "node name is required (or set GALACTIC_CNI_NODE_NAME)"} + } + _ = os.Setenv("KUBECONFIG", cniConfig.Kubeconfig) + + namespace := conf.Namespace + if namespace == "" { + namespace = cniConfig.Namespace + } + conf.Namespace = namespace + + setupLogging(cniConfig.LogFile, cniConfig.LogLevel) + slog.Debug("CNI config received", "stdin", string(data)) + + if conf.PrevResult != nil { + if err := validatePrevResult(conf.PrevResult); err != nil { + return nil, &types.Error{Code: 6, Msg: fmt.Sprintf("invalid prevResult: %v", err)} + } + } + return conf, nil +} + +func validatePrevResult(res types.Result) error { + if res == nil { + return nil + } + jsonBytes, err := json.Marshal(res) + if err != nil { + return fmt.Errorf("marshal prevResult: %w", err) + } + if _, err := type100.NewResult(jsonBytes); err != nil { + return fmt.Errorf("parse prevResult: %w", err) + } + return nil +} + +func sanitizeForError(s string) string { + for _, c := range s { + if c < 0x20 || c > 0x7e { + return sanitizeForErrorBinary + } + } + return s +} diff --git a/internal/cnibgp/ops_add.go b/internal/cnibgp/ops_add.go new file mode 100644 index 00000000..17fe9fc5 --- /dev/null +++ b/internal/cnibgp/ops_add.go @@ -0,0 +1,92 @@ +// Copyright 2025 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnibgp + +import ( + "context" + "fmt" + "log/slog" + + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/types" + + "go.datum.net/galactic/internal/plumbing/intf" +) + +// cmdAdd is galactic-bgp's own ADD: the last (per the design note's sample +// conflists) plugin in the chain, publishing SRv6/BGP/eBPF state for +// whatever the master plugin already created. It never touches a kernel +// interface — everything it needs (which interface kind, which addresses) +// comes from prevResult (see prevresult.go). +func cmdAdd(args *skel.CmdArgs) (err error) { + pluginConf, err := parseConf(args.StdinData) + if err != nil { + return err + } + + ifaceType, ipamResult, prevResult, err := inferFromPrevResult(pluginConf.RawPrevResult) + if err != nil { + return &types.Error{Code: 6, Msg: fmt.Sprintf("infer from prevResult: %v", err)} + } + + nodeName := cniConfig.NodeName + namespace := pluginConf.Namespace + + slog.Info("ADD: starting", "containerID", args.ContainerID, + "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment, + "ifaceType", ifaceType, "namespace", namespace, "nodeName", nodeName) + + tracker := &resourceTracker{ + vpc: pluginConf.VPC, + vpcAttachment: pluginConf.VPCAttachment, + namespace: namespace, + } + + defer func() { + if err != nil { + slog.Error("ADD: failed, rolling back created resources", "err", err, + "containerID", args.ContainerID, "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment) + // rollbackCtx is created here, fresh, rather than up front and + // shared with the work that just failed: publishBGPState's own + // retryK8sOps (bgp.go) can burn up to ~30s across its retries, + // each on its own fresh cniTimeout context. A context created + // before that call and reused here could already be expired by + // the time cleanup runs, so Delete would fail with "context + // deadline exceeded" instead of NotFound — client.IgnoreNotFound + // doesn't catch that, and the just-created CRDs would leak. + // Giving rollback its own full cniTimeout budget, on the failure + // path only, also means a successful ADD never allocates (and + // must remember to cancel) a context it doesn't use. + rollbackCtx, rollbackCancel := context.WithTimeout(context.Background(), cniTimeout) + tracker.cleanup(rollbackCtx) + rollbackCancel() + } + }() + + k8sClient, err := newK8sClient() + if err != nil { + return fmt.Errorf("create k8s client: %w", err) + } + tracker.k8s = k8sClient + + vpcHex, err := intf.Base62ToHex(pluginConf.VPC) + if err != nil { + return fmt.Errorf("decode VPC: %w", err) + } + + cfg := publishConfig{vpc: pluginConf.VPC, vpcAttachment: pluginConf.VPCAttachment, ifaceType: ifaceType} + result, err := publishBGPState(args, cfg, nodeName, namespace, ipamResult, vpcHex, k8sClient) + tracker.publishResult = result + if err != nil { + return err + } + + // Pass prevResult through unchanged: this plugin adds no new interfaces + // or IPs of its own, so its own CNI result is exactly what it received. + // Per the design note's sample conflists, galactic-bgp is the last + // plugin in the chain, so this becomes the runtime's authoritative + // result for the ADD. + return types.PrintResult(prevResult, pluginConf.CNIVersion) +} diff --git a/internal/cnibgp/ops_check.go b/internal/cnibgp/ops_check.go new file mode 100644 index 00000000..9bf7e710 --- /dev/null +++ b/internal/cnibgp/ops_check.go @@ -0,0 +1,231 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnibgp + +import ( + "context" + "errors" + "fmt" + "log/slog" + "net/http" + "net/netip" + "os" + "time" + + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/types" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + "go.datum.net/galactic/internal/cni/crdnames" + "go.datum.net/galactic/internal/config" + "go.datum.net/galactic/internal/plumbing/ebpf/uformat" + "go.datum.net/galactic/internal/plumbing/ebpf/usidmap" + "go.datum.net/galactic/internal/plumbing/vrf" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +// cmdCheck verifies that the BGP state cmdAdd published is still in place: +// the BGPVRFInstance and BGPAdvertisement CRDs exist, and — when this +// node's BGPRouter has SRv6 configured — the eBPF vrf_table entry for this +// attachment is still registered. None of this is a move from +// internal/cni's own CHECK; it's genuinely new, since nothing before this +// split ever verified CRD/eBPF state independently of kernel interface +// state. +func cmdCheck(args *skel.CmdArgs) error { + pluginConf, err := parseConf(args.StdinData) + if err != nil { + return err + } + slog.Info("CHECK: starting", "containerID", args.ContainerID, + "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment) + + k8s, err := newK8sClient() + if err != nil { + return fmt.Errorf("create k8s client: %w", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), cniTimeout) + defer cancel() + + var errs []error + + vrfInst := &bgpv1alpha1.BGPVRFInstance{} + vrfName := crdnames.BGPVRFInstanceName(pluginConf.VPC, pluginConf.VPCAttachment) + vrfErr := k8s.Get(ctx, client.ObjectKey{Name: vrfName, Namespace: pluginConf.Namespace}, vrfInst) + if vrfErr != nil { + errs = append(errs, fmt.Errorf("BGPVRFInstance %s: %w", vrfName, vrfErr)) + } + + adv := &bgpv1alpha1.BGPAdvertisement{} + advName := crdnames.BGPAdvertisementName(pluginConf.VPC, pluginConf.VPCAttachment) + if err := k8s.Get(ctx, client.ObjectKey{Name: advName, Namespace: pluginConf.Namespace}, adv); err != nil { + errs = append(errs, fmt.Errorf("BGPAdvertisement %s: %w", advName, err)) + } + + // The eBPF vrf_table entry is only checkable once the BGPVRFInstance + // lookup succeeded (it carries the Argument value the entry is keyed + // on) and this node's router actually has SRv6 configured — matches + // registerEBPFDatapath's own no-op case. + if vrfErr == nil { + if err := checkEBPFEntry(ctx, k8s, pluginConf, uint16(vrfInst.Spec.VRFID)); err != nil { + errs = append(errs, err) + } + } + + if len(errs) > 0 { + err := fmt.Errorf("CHECK failed: %w", errors.Join(errs...)) + slog.Error("CHECK: failed", "err", err, "containerID", args.ContainerID, + "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment) + return err + } + slog.Info("CHECK: passed", "containerID", args.ContainerID, + "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment) + return nil +} + +// checkEBPFEntry verifies every eBPF table registerEBPFDatapath wrote for +// this attachment still exists and is intact: the locator_table entry (this +// router's own node), the function_table entry (SRv6 End.DT46 behavior), +// and the vrf_table entry (still resolving to this attachment's own VRF +// table id) — plus the same nodeID range check ADD treats as a hard error. +// Checking vrf_table alone would miss a corrupted/missing locator or +// function entry, or a nodeID that drifted out of range after ADD, while +// still reporting the attachment healthy. Returns nil (not an error) when +// this node's router has no SRv6Locator/nodeID configured — SRv6 was +// intentionally never set up for this attachment, matching +// registerEBPFDatapath's own no-op case. +func checkEBPFEntry(ctx context.Context, k8s client.Client, pluginConf *PluginConf, argument uint16) error { + bgp, err := lookupBGPRouter(ctx, k8s, cniConfig.NodeName, pluginConf.Namespace) + if err != nil { + return fmt.Errorf("look up BGPRouter: %w", err) + } + if bgp.srv6Locator == "" || bgp.nodeID == 0 { + return nil + } + + if bgp.nodeID < uformat.NodeIDMin || bgp.nodeID > uformat.NodeIDMax { + return fmt.Errorf("eBPF check: nodeID %d out of range [%#x,%#x]", + bgp.nodeID, uint16(uformat.NodeIDMin), uint16(uformat.NodeIDMax)) + } + + prefix, err := netip.ParsePrefix(bgp.srv6Locator) + if err != nil { + return fmt.Errorf("parse SRv6 locator %q: %w", bgp.srv6Locator, err) + } + block, err := uformat.Block(prefix.Addr()) + if err != nil { + return fmt.Errorf("derive eBPF uSID Block from locator %q: %w", bgp.srv6Locator, err) + } + + vrfTableID, err := vrf.TableID(pluginConf.VPC, pluginConf.VPCAttachment) + if err != nil { + return fmt.Errorf("get VRF table ID: %w", err) + } + + registry, closer, err := usidmap.OpenPinnedRegistry(ebpfPinDir) + if err != nil { + return fmt.Errorf("open pinned eBPF uSID maps: %w", err) + } + defer func() { _ = closer.Close() }() + + var errs []error + + if _, ok, err := registry.Locator.Get(block, uint16(bgp.nodeID)); err != nil { + errs = append(errs, fmt.Errorf("read eBPF locator_table entry: %w", err)) + } else if !ok { + errs = append(errs, fmt.Errorf( + "eBPF locator_table entry for block %#x node-id %#x not found", block, uint16(bgp.nodeID))) + } + + if funcEntry, ok, err := registry.Function.Get(block, uformat.FunctionEndDT46); err != nil { + errs = append(errs, fmt.Errorf("read eBPF function_table entry: %w", err)) + } else if !ok { + errs = append(errs, fmt.Errorf( + "eBPF function_table entry for block %#x function %#x not found", block, uformat.FunctionEndDT46)) + } else if funcEntry.Behavior != usidmap.BehaviorEndDT46 { + errs = append(errs, fmt.Errorf( + "eBPF function_table entry Behavior = %#x, want %#x", funcEntry.Behavior, usidmap.BehaviorEndDT46)) + } + + entry, ok, err := registry.VRF.Get(block, argument) + if err != nil { + errs = append(errs, fmt.Errorf("read eBPF vrf_table entry: %w", err)) + } else if !ok { + errs = append(errs, fmt.Errorf("eBPF vrf_table entry for block %#x argument %#x not found", block, argument)) + } else if entry.VRFTableID != vrfTableID { + errs = append(errs, fmt.Errorf("eBPF vrf_table entry VRFTableID = %#x, want %#x", entry.VRFTableID, vrfTableID)) + } + + return errors.Join(errs...) +} + +// cmdStatus implements the CNI spec STATUS operation — galactic-bgp talks +// to the API server (BGP CRD reads/writes), so this probes it the same way +// internal/cni's own cmdStatus does. +func cmdStatus(args *skel.CmdArgs) error { + if err := parseStatusConf(args.StdinData); err != nil { + return err + } + + hostConf, err := loadHostConf(ConfFile) + if err != nil { + return &types.Error{Code: 7, Msg: fmt.Sprintf("load host CNI config: %v", err)} + } + + cniConfig.Resolve(&config.ConflistValues{ + Kubeconfig: hostConf.Kubeconfig, + Namespace: hostConf.Namespace, + LogFile: hostConf.LogFile, + LogLevel: hostConf.LogLevel, + }) + + _ = os.Setenv("KUBECONFIG", cniConfig.Kubeconfig) + + setupLogging(cniConfig.LogFile, cniConfig.LogLevel) + slog.Debug("CNI config received", "stdin", string(args.StdinData)) + + slog.Info("STATUS: probing API server reachability") + if err := probeAPIServer(); err != nil { + slog.Error("STATUS: API server probe failed", "err", err) + return &types.Error{Code: 50, Msg: fmt.Sprintf("API server health check failed: %v", err)} + } + slog.Info("STATUS: ready") + return nil +} + +// probeAPIServerFn is a variable so tests can override it. +var probeAPIServerFn = func() error { + kubeconfig, err := ctrl.GetConfig() + if err != nil { + if errors.Is(err, rest.ErrNotInCluster) { + return nil + } + return fmt.Errorf("load kubeconfig: %w", err) + } + kubeconfig.Timeout = 2 * time.Second + httpClient, err := rest.HTTPClientFor(kubeconfig) + if err != nil { + return fmt.Errorf("build http client: %w", err) + } + req, err := http.NewRequestWithContext( + context.Background(), + http.MethodGet, + kubeconfig.Host+"/healthz", + nil, + ) + if err != nil { + return fmt.Errorf("build healthz request: %w", err) + } + resp, err := httpClient.Do(req) + if err != nil { + return fmt.Errorf("healthz request failed: %w", err) + } + defer resp.Body.Close() //nolint:errcheck // best-effort probe + return nil +} + +var probeAPIServer = probeAPIServerFn diff --git a/internal/cnibgp/ops_del.go b/internal/cnibgp/ops_del.go new file mode 100644 index 00000000..5bd8abba --- /dev/null +++ b/internal/cnibgp/ops_del.go @@ -0,0 +1,42 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnibgp + +import ( + "log/slog" + + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/types" + type100 "github.com/containernetworking/cni/pkg/types/100" +) + +// cmdDel is a no-op, same as every other binary in the chain: the +// BGPVRFInstance/BGPAdvertisement CRDs and eBPF vrf_table entry this +// plugin's own ADD created are keyed by (vpc, vpcAttachment) and may still +// be in use by another pod/VM sharing the same attachment. Deleting them +// here would race with a concurrent ADD during restarts. Cleanup is left +// entirely to galactic-router's GC controller — see internal/cni's own +// cmdDel for the full reasoning, identical here. +func cmdDel(args *skel.CmdArgs) error { + // DEL is idempotent per the CNI spec: always return success, even if + // parsing the config fails — logging vpc/vpcAttachment (when parseable) + // is the only reason to parse at all here, since there's no cleanup to + // gate on it. + pluginConf, parseErr := parseConf(args.StdinData) + if parseErr != nil { + slog.Error("DEL: failed to parse CNI config, skipping cleanup logging", "err", parseErr, + "containerID", args.ContainerID) + result := &type100.Result{} + _ = types.PrintResult(result, "1.0.0") + return nil + } + + slog.Info("DEL: skipping shared resource cleanup (handled by GC)", "containerID", args.ContainerID, + "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment) + + result := &type100.Result{} + _ = types.PrintResult(result, pluginConf.CNIVersion) + return nil +} diff --git a/internal/cnibgp/ops_del_test.go b/internal/cnibgp/ops_del_test.go new file mode 100644 index 00000000..f12c363d --- /dev/null +++ b/internal/cnibgp/ops_del_test.go @@ -0,0 +1,58 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnibgp + +import ( + "fmt" + "testing" + + "github.com/containernetworking/cni/pkg/skel" + + "go.datum.net/galactic/internal/config" +) + +// TestCmdDelIdempotent returns nil even when the CNI config is invalid, per +// the CNI spec's DEL idempotency requirement. +func TestCmdDelIdempotent(t *testing.T) { + args := &skel.CmdArgs{ + ContainerID: "test-container", + StdinData: []byte("not valid json"), + } + + if err := cmdDel(args); err != nil { + t.Fatalf("cmdDel with invalid config returned error = %v, want nil", err) + } +} + +// TestCmdDelParsesConfigForLogging exercises the valid-config path: DEL has +// nothing to clean up itself (GC handles it), but it should still be able to +// parse the conflist it's given — vpc/vpcAttachment come from that parse, +// and CNIVersion for the printed result comes from pluginConf.CNIVersion, +// not a hardcoded "1.0.0", so a conflist authored with a different version +// still gets a matching result back on DEL the same way ADD/CHECK do. +func TestCmdDelParsesConfigForLogging(t *testing.T) { + // parseConf resolves cniConfig's node-level env vars, which is normally + // done once at process startup by InitCNIConfig() (cmd/galactic-bgp's + // main); tests that go through parseConf need the same setup. Setting + // GALACTIC_CNI_NODE_NAME explicitly also skips parseConf's fallback to + // hostconf.DetectNodeNameFromAPI, which would otherwise try (and hang + // retrying) to reach a real API server that doesn't exist in this test. + t.Setenv(config.EnvCNINodeName, "test-node") + cniConfig = config.NewCNIConfig() + t.Cleanup(func() { cniConfig = nil }) + + conf := fmt.Sprintf( + `{"cniVersion":"1.1.0","name":"test","type":"galactic-bgp","vpc":"%s","vpcattachment":"%s"}`, + testVPC, testAttachment, + ) + args := &skel.CmdArgs{ + ContainerID: "test-container", + StdinData: []byte(conf), + } + + if err := cmdDel(args); err != nil { + t.Fatalf("cmdDel with valid config returned error = %v, want nil", err) + } +} diff --git a/internal/cnibgp/prevresult.go b/internal/cnibgp/prevresult.go new file mode 100644 index 00000000..02756e36 --- /dev/null +++ b/internal/cnibgp/prevresult.go @@ -0,0 +1,106 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnibgp + +import ( + "encoding/json" + "errors" + "fmt" + + "github.com/containernetworking/cni/pkg/types" + type100 "github.com/containernetworking/cni/pkg/types/100" + + "go.datum.net/galactic/internal/cniipam" +) + +// inferFromPrevResult reconstructs the ipamResult and infers whether the +// preceding master plugin created a veth or tap interface, both from +// rawPrevResult — the raw JSON form of the CNI chain's accumulated result +// so far (PluginConf.RawPrevResult; the typed PluginConf.PrevResult field +// is never populated by plain JSON unmarshal, per its "json:\"-\"" tag — +// ops_check.go elsewhere in this repo already reads prevResult the same, +// correct way). +// +// type100.NewResult only accepts a Result whose own CNIVersion field is +// exactly "1.0.0" or "1.1.0" (github.com/containernetworking/cni's +// pkg/types/100 supportedVersions) — and the master plugin's printed Result +// carries the conflist's own cniVersion value verbatim (buildResult/ +// buildTapResult set CNIVersion: pluginConf.CNIVersion), not a value this +// plugin controls. A conflist authored with an older cniVersion (e.g. +// "0.4.0") therefore fails ADD here for every attachment in the chain. This +// is a real, hard requirement on every conflist in this repo's CNI chain — +// see docs/cni/configuration.md's cniVersion paragraph — not just an +// implementation detail of this function. +// +// galactic-bgp has no kernel-interface access or IPAM knowledge of its own +// — this is the only way it learns either. Interface-kind inference: a +// veth master's own result declares one interface with a non-empty Sandbox +// (the guest end, moved into the container netns — see internal/cni/ +// result.go's buildResult); a tap master's declares zero (the tap device +// stays on the host — internal/cnitap/result.go's buildTapResult). This +// counts Sandbox-carrying interfaces rather than len(Interfaces) itself, +// on purpose: #306 chains galactic-route into this same prevResult next, +// and total interface count is not this plugin's to own — if anything +// appended later adds a host-side interface entry of its own, a raw count +// would silently reclassify a tap master as veth (or vice versa) instead +// of failing loudly, because both 1 and 2 are valid switch cases. Whether +// an interface was moved into the container's netns is the actual property +// that distinguishes veth from tap, not how many entries happen to be in +// the slice — see the design note's "Split the veth/tap master into two +// binaries" section for why no interface_type field exists anywhere in the +// chain to make this explicit instead. +func inferFromPrevResult( + rawPrevResult map[string]interface{}, +) (ifaceType string, ipamResult *cniipam.IPAMResult, parsed types.Result, err error) { + if rawPrevResult == nil { + return "", nil, nil, errors.New("no prevResult: galactic-bgp must be chained after a master plugin") + } + + jsonBytes, err := json.Marshal(rawPrevResult) + if err != nil { + return "", nil, nil, fmt.Errorf("marshal prevResult: %w", err) + } + parsed, err = type100.NewResult(jsonBytes) + if err != nil { + return "", nil, nil, fmt.Errorf("parse prevResult: %w", err) + } + versioned, err := type100.GetResult(parsed) + if err != nil { + return "", nil, nil, fmt.Errorf("get prevResult: %w", err) + } + + if len(versioned.Interfaces) == 0 { + return "", nil, nil, errors.New("prevResult declares no interfaces") + } + + var sandboxed int + for _, iface := range versioned.Interfaces { + if iface.Sandbox != "" { + sandboxed++ + } + } + switch sandboxed { + case 0: + ifaceType = ifaceTypeTap + case 1: + ifaceType = ifaceTypeVeth + default: + return "", nil, nil, fmt.Errorf( + "prevResult declares %d interfaces with a non-empty Sandbox, want 0 (tap master) or 1 (veth master)", + sandboxed) + } + + ipamResult, err = cniipam.ResultToIPAMResult(parsed) + if err != nil { + return "", nil, nil, fmt.Errorf("convert prevResult IPs: %w", err) + } + if ipamResult.IPv6Subnet == nil && ipamResult.IPv4Address == nil { + // No IPAM allocation at all (e.g. a tap workload managing its own + // addressing) — nil, not a zero-value result, matches what every + // other caller in this chain already treats as "no IPAM." + ipamResult = nil + } + return ifaceType, ipamResult, parsed, nil +} diff --git a/internal/cnibgp/prevresult_test.go b/internal/cnibgp/prevresult_test.go new file mode 100644 index 00000000..fa8a7702 --- /dev/null +++ b/internal/cnibgp/prevresult_test.go @@ -0,0 +1,185 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnibgp + +import ( + "encoding/json" + "net" + "testing" + + type100 "github.com/containernetworking/cni/pkg/types/100" +) + +const ( + testCNIVersion100 = "1.0.0" + testTapMAC = "aa:bb:cc:dd:ee:ff" + testVethHostMAC = "aa:bb:cc:dd:ee:f0" + testVethGuestMAC = "aa:bb:cc:dd:ee:f1" + testTapIfaceName = "tap0" + testVethHostName = "veth0" + testVethGuestName = "eth0" +) + +// rawPrevResultFrom round-trips result through JSON, the same way the CNI +// runtime hands prevResult to a chained plugin's stdin (skel.CmdArgs.StdinData +// -> PluginConf.RawPrevResult), so tests exercise inferFromPrevResult exactly +// as it receives real input. +func rawPrevResultFrom(t *testing.T, result *type100.Result) map[string]interface{} { + t.Helper() + b, err := json.Marshal(result) + if err != nil { + t.Fatalf("marshal result: %v", err) + } + var raw map[string]interface{} + if err := json.Unmarshal(b, &raw); err != nil { + t.Fatalf("unmarshal result: %v", err) + } + return raw +} + +func TestInferFromPrevResult_TapMaster(t *testing.T) { + // A tap master's own result declares one host-only interface — no + // Sandbox — see internal/cnitap/result.go's buildTapResult. + raw := rawPrevResultFrom(t, &type100.Result{ + CNIVersion: testCNIVersion100, + Interfaces: []*type100.Interface{ + {Name: testTapIfaceName, Mac: testTapMAC, Sandbox: ""}, + }, + }) + + ifaceType, ipamResult, _, err := inferFromPrevResult(raw) + if err != nil { + t.Fatalf("inferFromPrevResult() error = %v", err) + } + if ifaceType != ifaceTypeTap { + t.Errorf("ifaceType = %q, want %q", ifaceType, ifaceTypeTap) + } + if ipamResult != nil { + t.Errorf("ipamResult = %+v, want nil (no IPAM in this result)", ipamResult) + } +} + +func TestInferFromPrevResult_VethMaster(t *testing.T) { + // A veth master's own result declares two interfaces: host (no Sandbox) + // and guest (Sandbox = the container netns it was moved into) — see + // internal/cni/result.go's buildResult. + raw := rawPrevResultFrom(t, &type100.Result{ + CNIVersion: testCNIVersion100, + Interfaces: []*type100.Interface{ + {Name: testVethHostName, Mac: testVethHostMAC, Sandbox: ""}, + {Name: testVethGuestName, Mac: testVethGuestMAC, Sandbox: testNetns}, + }, + }) + + ifaceType, _, _, err := inferFromPrevResult(raw) + if err != nil { + t.Fatalf("inferFromPrevResult() error = %v", err) + } + if ifaceType != ifaceTypeVeth { + t.Errorf("ifaceType = %q, want %q", ifaceType, ifaceTypeVeth) + } +} + +func TestInferFromPrevResult_ExtraHostInterfaceStillClassifiesCorrectly(t *testing.T) { + // A hypothetical extra host-side interface appended by a later + // chain-invoked plugin (e.g. #306's galactic-route) must not flip a tap + // master's classification to veth just because len(Interfaces) grew + // from 1 to 2 — only Sandbox-carrying interfaces count. + raw := rawPrevResultFrom(t, &type100.Result{ + CNIVersion: testCNIVersion100, + Interfaces: []*type100.Interface{ + {Name: testTapIfaceName, Mac: testTapMAC, Sandbox: ""}, + {Name: "route0", Mac: "aa:bb:cc:dd:ee:fe", Sandbox: ""}, + }, + }) + + ifaceType, _, _, err := inferFromPrevResult(raw) + if err != nil { + t.Fatalf("inferFromPrevResult() error = %v", err) + } + if ifaceType != ifaceTypeTap { + t.Errorf("ifaceType = %q, want %q", ifaceType, ifaceTypeTap) + } +} + +func TestInferFromPrevResult_MultipleSandboxedInterfacesIsHardError(t *testing.T) { + // More than one Sandbox-carrying interface is genuinely ambiguous — no + // known master plugin produces this — and must fail loudly rather than + // guess. + raw := rawPrevResultFrom(t, &type100.Result{ + CNIVersion: testCNIVersion100, + Interfaces: []*type100.Interface{ + {Name: testVethHostName, Mac: testVethHostMAC, Sandbox: ""}, + {Name: testVethGuestName, Mac: testVethGuestMAC, Sandbox: testNetns}, + {Name: "eth1", Mac: "aa:bb:cc:dd:ee:f2", Sandbox: testNetns}, + }, + }) + + if _, _, _, err := inferFromPrevResult(raw); err == nil { + t.Fatal("inferFromPrevResult() error = nil, want error for ambiguous interface shape") + } +} + +func TestInferFromPrevResult_NoInterfacesIsError(t *testing.T) { + raw := rawPrevResultFrom(t, &type100.Result{CNIVersion: testCNIVersion100}) + + if _, _, _, err := inferFromPrevResult(raw); err == nil { + t.Fatal("inferFromPrevResult() error = nil, want error for empty Interfaces") + } +} + +func TestInferFromPrevResult_NilRawPrevResult(t *testing.T) { + if _, _, _, err := inferFromPrevResult(nil); err == nil { + t.Fatal("inferFromPrevResult() error = nil, want error for nil rawPrevResult") + } +} + +// TestInferFromPrevResult_RejectsOlderCNIVersion documents (and locks in) +// the hard cniVersion requirement described in this function's doc comment +// and docs/cni/configuration.md: type100.NewResult only accepts a Result +// whose own CNIVersion is exactly "1.0.0" or "1.1.0", and the master +// plugin's printed Result carries the conflist's cniVersion straight +// through, so an older value must fail loudly here rather than proceed with +// stale/wrong assumptions about the Result's shape. +func TestInferFromPrevResult_RejectsOlderCNIVersion(t *testing.T) { + raw := rawPrevResultFrom(t, &type100.Result{ + CNIVersion: "0.4.0", + Interfaces: []*type100.Interface{ + {Name: testTapIfaceName, Mac: testTapMAC, Sandbox: ""}, + }, + }) + + if _, _, _, err := inferFromPrevResult(raw); err == nil { + t.Fatal("inferFromPrevResult() error = nil, want error for cniVersion \"0.4.0\"") + } +} + +func TestInferFromPrevResult_CarriesIPAMResult(t *testing.T) { + ipv6 := net.IPNet{IP: net.ParseIP("2001:db8::1"), Mask: net.CIDRMask(64, 128)} + raw := rawPrevResultFrom(t, &type100.Result{ + CNIVersion: testCNIVersion100, + Interfaces: []*type100.Interface{ + {Name: testVethHostName, Mac: testVethHostMAC, Sandbox: ""}, + {Name: testVethGuestName, Mac: testVethGuestMAC, Sandbox: testNetns}, + }, + IPs: []*type100.IPConfig{ + {Address: ipv6, Interface: type100.Int(1)}, + }, + }) + + ifaceType, ipamResult, _, err := inferFromPrevResult(raw) + if err != nil { + t.Fatalf("inferFromPrevResult() error = %v", err) + } + if ifaceType != ifaceTypeVeth { + t.Errorf("ifaceType = %q, want %q", ifaceType, ifaceTypeVeth) + } + if ipamResult == nil || ipamResult.IPv6Subnet == nil { + t.Fatalf("ipamResult = %+v, want a non-nil IPv6Subnet", ipamResult) + } + if ipamResult.IPv6Subnet.String() != "2001:db8::1/64" { + t.Errorf("ipamResult.IPv6Subnet = %s, want 2001:db8::1/64", ipamResult.IPv6Subnet.String()) + } +} diff --git a/internal/cnibgp/resource.go b/internal/cnibgp/resource.go new file mode 100644 index 00000000..227ee65a --- /dev/null +++ b/internal/cnibgp/resource.go @@ -0,0 +1,113 @@ +// Copyright 2025 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnibgp + +import ( + "context" + "fmt" + "log/slog" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "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" + + "go.datum.net/galactic/internal/cni/crdnames" + "go.datum.net/galactic/internal/plumbing/vrf" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +var cniScheme = runtime.NewScheme() + +func init() { + utilruntime.Must(clientgoscheme.AddToScheme(cniScheme)) + utilruntime.Must(bgpv1alpha1.AddToScheme(cniScheme)) +} + +// newK8sClient creates a new Kubernetes client using the in-cluster config, +// scoped to cniScheme (BGP CRDs). +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 +} + +// resourceTracker tracks resources created during cmdAdd for selective +// rollback. galactic-bgp's own ADD only ever creates BGP CRDs and an eBPF +// vrf_table entry — the kernel-interface/VRF cleanup that used to live +// alongside these in one process-wide tracker is now each master plugin's +// own, smaller tracker (internal/cni, internal/cnitap), scoped to exactly +// what its own ADD creates. +// +// publishResult is embedded, rather than its five fields being copied over +// field-by-field, so a future field added to one struct can't silently stop +// being tracked in the other with no compiler error to catch it — cmdAdd +// assigns the whole publishResult from publishBGPState in one shot +// (tracker.publishResult = result). +type resourceTracker struct { + vpc, vpcAttachment string + namespace string + k8s client.Client + + publishResult +} + +// cleanup rolls back all tracked resources. Errors are logged but never +// returned — the caller already has a failure. +func (rt *resourceTracker) cleanup(ctx context.Context) { + slog.Info("Selective rollback: cleaning up resources created during failed ADD", + "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) + + if rt.advertisementCreated && rt.k8s != nil { + adv := &bgpv1alpha1.BGPAdvertisement{ + ObjectMeta: metav1.ObjectMeta{ + Name: crdnames.BGPAdvertisementName(rt.vpc, rt.vpcAttachment), + Namespace: rt.namespace, + }, + } + if err := rt.k8s.Delete(ctx, adv); client.IgnoreNotFound(err) != nil { + slog.Error("Rollback: failed to delete BGPAdvertisement", "err", err, + "name", adv.Name, "namespace", rt.namespace) + } else { + slog.Debug("Rollback: deleted BGPAdvertisement", "name", adv.Name, "namespace", rt.namespace) + } + } + + if rt.vrfInstanceCreated && rt.k8s != nil { + vrfInst := &bgpv1alpha1.BGPVRFInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: crdnames.BGPVRFInstanceName(rt.vpc, rt.vpcAttachment), + Namespace: rt.namespace, + }, + } + if err := rt.k8s.Delete(ctx, vrfInst); client.IgnoreNotFound(err) != nil { + slog.Error("Rollback: failed to delete BGPVRFInstance", "err", err, + "name", vrfInst.Name, "namespace", rt.namespace) + } else { + slog.Debug("Rollback: deleted BGPVRFInstance", "name", vrfInst.Name, "namespace", rt.namespace) + } + } + + if rt.ebpfRegistered { + if vrfTableID, err := vrf.TableID(rt.vpc, rt.vpcAttachment); err != nil { + slog.Error("Rollback: failed to resolve VRF table id, skipping eBPF vrf_table unregister", "err", err, + "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) + } else if err := unregisterEBPFDatapath(rt.ebpfBlock, rt.ebpfArgument, vrfTableID, ebpfPinDir); err != nil { + slog.Error("Rollback: failed to unregister eBPF vrf_table entry", "err", err, + "block", rt.ebpfBlock, "argument", rt.ebpfArgument) + } else { + slog.Debug("Rollback: unregistered eBPF vrf_table entry", + "block", rt.ebpfBlock, "argument", rt.ebpfArgument) + } + } +} diff --git a/internal/cnibgp/resource_test.go b/internal/cnibgp/resource_test.go new file mode 100644 index 00000000..785c4c1a --- /dev/null +++ b/internal/cnibgp/resource_test.go @@ -0,0 +1,279 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnibgp + +import ( + "context" + "fmt" + "os" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "go.datum.net/galactic/internal/cni/crdnames" + "go.datum.net/galactic/internal/plumbing/ebpf/attach" + "go.datum.net/galactic/internal/plumbing/ebpf/usidmap" + "go.datum.net/galactic/internal/plumbing/vrf" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +const testDefaultNamespace = "default" + +// ---- resourceTracker.cleanup: zero-value / partial state ------------------- + +// TestResourceTrackerCleanup_ZeroValue verifies cleanup with a zero-value +// tracker doesn't panic — it's called from cmdAdd's defer, and the caller +// may have failed before setting any fields (e.g. newK8sClient itself +// failed, so tracker.k8s is nil). +func TestResourceTrackerCleanup_ZeroValue(t *testing.T) { + tracker := &resourceTracker{} + tracker.cleanup(context.Background()) // must not panic +} + +// TestResourceTrackerCleanup_NilK8sClientSkipsCRDDeletes verifies cleanup +// doesn't attempt a Delete through a nil client even when the created-flags +// say there's something to roll back — this shouldn't happen in practice +// (tracker.k8s is set right after newK8sClient succeeds, before anything +// else can set vrfInstanceCreated/advertisementCreated), but cleanup's own +// nil check is what actually prevents the panic if it ever does. +func TestResourceTrackerCleanup_NilK8sClientSkipsCRDDeletes(t *testing.T) { + tracker := &resourceTracker{ + vpc: testVPC, + vpcAttachment: testAttachment, + namespace: testDefaultNamespace, + publishResult: publishResult{ + vrfInstanceCreated: true, + advertisementCreated: true, + }, + } + tracker.cleanup(context.Background()) // must not panic +} + +// ---- resourceTracker.cleanup: CRD rollback --------------------------------- + +// TestResourceTrackerCleanup_DeletesOnlyWhatWasCreated exercises cleanup's +// actual wiring end to end: given a tracker whose publishResult says only +// the BGPVRFInstance was created (advertisementCreated stays false, as it +// would if publishBGPState failed between the two), cleanup must delete the +// BGPVRFInstance and leave any BGPAdvertisement alone. +func TestResourceTrackerCleanup_DeletesOnlyWhatWasCreated(t *testing.T) { + namespace := testDefaultNamespace + vrfName := crdnames.BGPVRFInstanceName(testVPC, testAttachment) + advName := crdnames.BGPAdvertisementName(testVPC, testAttachment) + + existingVRFInst := &bgpv1alpha1.BGPVRFInstance{ + ObjectMeta: metav1.ObjectMeta{Name: vrfName, Namespace: namespace}, + } + // A BGPAdvertisement that was NOT created by this ADD (e.g. left over + // from another container sharing the same attachment) — cleanup must + // not touch it, since advertisementCreated is false. + untouchedAdv := &bgpv1alpha1.BGPAdvertisement{ + ObjectMeta: metav1.ObjectMeta{Name: advName, Namespace: namespace}, + } + + k8s := fakeClient(existingVRFInst, untouchedAdv) + tracker := &resourceTracker{ + vpc: testVPC, + vpcAttachment: testAttachment, + namespace: namespace, + k8s: k8s, + publishResult: publishResult{vrfInstanceCreated: true}, + } + + tracker.cleanup(context.Background()) + + if err := k8s.Get(context.Background(), client.ObjectKey{Name: vrfName, Namespace: namespace}, + &bgpv1alpha1.BGPVRFInstance{}); err == nil { + t.Error("BGPVRFInstance still exists after cleanup, want deleted") + } + if err := k8s.Get(context.Background(), client.ObjectKey{Name: advName, Namespace: namespace}, + &bgpv1alpha1.BGPAdvertisement{}); err != nil { + t.Errorf("BGPAdvertisement Get after cleanup = %v, want it left untouched (advertisementCreated was false)", err) + } +} + +// TestResourceTrackerCleanup_DeletesBothCRDsWhenBothCreated covers the +// common failure path: both CRDs were created, then something later in +// publishBGPState (or types.PrintResult) failed, so both must roll back. +func TestResourceTrackerCleanup_DeletesBothCRDsWhenBothCreated(t *testing.T) { + namespace := testDefaultNamespace + vrfName := crdnames.BGPVRFInstanceName(testVPC, testAttachment) + advName := crdnames.BGPAdvertisementName(testVPC, testAttachment) + + k8s := fakeClient( + &bgpv1alpha1.BGPVRFInstance{ObjectMeta: metav1.ObjectMeta{Name: vrfName, Namespace: namespace}}, + &bgpv1alpha1.BGPAdvertisement{ObjectMeta: metav1.ObjectMeta{Name: advName, Namespace: namespace}}, + ) + tracker := &resourceTracker{ + vpc: testVPC, + vpcAttachment: testAttachment, + namespace: namespace, + k8s: k8s, + publishResult: publishResult{vrfInstanceCreated: true, advertisementCreated: true}, + } + + tracker.cleanup(context.Background()) + + if err := k8s.Get(context.Background(), client.ObjectKey{Name: vrfName, Namespace: namespace}, + &bgpv1alpha1.BGPVRFInstance{}); err == nil { + t.Error("BGPVRFInstance still exists after cleanup, want deleted") + } + if err := k8s.Get(context.Background(), client.ObjectKey{Name: advName, Namespace: namespace}, + &bgpv1alpha1.BGPAdvertisement{}); err == nil { + t.Error("BGPAdvertisement still exists after cleanup, want deleted") + } +} + +// TestResourceTrackerCleanup_MissingCRDsAreNotError covers cleanup's use of +// client.IgnoreNotFound: a CRD already gone (e.g. a retry after a partial +// prior rollback) must not surface as an error — cleanup never returns one. +func TestResourceTrackerCleanup_MissingCRDsAreNotError(t *testing.T) { + tracker := &resourceTracker{ + vpc: testVPC, + vpcAttachment: testAttachment, + namespace: testDefaultNamespace, + k8s: fakeClient(), + publishResult: publishResult{vrfInstanceCreated: true, advertisementCreated: true}, + } + + tracker.cleanup(context.Background()) // must not panic; nothing to delete +} + +// ---- resourceTracker.cleanup: eBPF rollback -------------------------------- + +// TestResourceTrackerCleanup_UnregistersOwnEBPFEntry covers the eBPF branch +// of cleanup's own wiring (not unregisterEBPFDatapath in isolation, which +// bgp_ebpf_test.go already covers): a tracker with ebpfRegistered=true must +// resolve this attachment's own VRF table id and remove exactly the +// vrf_table entry it registered. +func TestResourceTrackerCleanup_UnregistersOwnEBPFEntry(t *testing.T) { + requireRoot(t) + + if err := vrf.Add(testVPC, testAttachment); err != nil { + t.Fatalf("vrf.Add: %v", err) + } + t.Cleanup(func() { _ = vrf.Delete(testVPC, testAttachment) }) + vrfTableID, err := vrf.TableID(testVPC, testAttachment) + if err != nil { + t.Fatalf("vrf.TableID: %v", err) + } + + pinDir := fmt.Sprintf("/sys/fs/bpf/galactic-bgp-test-%d", os.Getpid()) + t.Cleanup(func() { _ = os.RemoveAll(pinDir) }) + // cleanup (resource.go) reads the pin directory from the package-level + // ebpfPinDir var, not attach.PinDir directly — point it at this test's + // own throwaway directory instead of the real production one. + origPinDir := ebpfPinDir + ebpfPinDir = pinDir + t.Cleanup(func() { ebpfPinDir = origPinDir }) + + loaderObjs, err := attach.Load(pinDir) + if err != nil { + t.Fatalf("attach.Load: %v", err) + } + t.Cleanup(func() { _ = loaderObjs.Close() }) + + reg, closer, err := usidmap.OpenPinnedRegistry(pinDir) + if err != nil { + t.Fatalf("OpenPinnedRegistry: %v", err) + } + defer func() { _ = closer.Close() }() + + const testBlock uint64 = 0x0102030405 + const testArgument uint16 = 0x042 + if err := reg.VRF.Register(testBlock, testArgument, vrfTableID, usidmap.EgressKindVeth); err != nil { + t.Fatalf("seed vrf_table entry: %v", err) + } + + tracker := &resourceTracker{ + vpc: testVPC, + vpcAttachment: testAttachment, + publishResult: publishResult{ + ebpfRegistered: true, + ebpfBlock: testBlock, + ebpfArgument: testArgument, + }, + } + + tracker.cleanup(context.Background()) + + if _, ok, err := reg.VRF.Get(testBlock, testArgument); err != nil || ok { + t.Errorf("vrf_table entry after cleanup: ok=%v err=%v, want ok=false", ok, err) + } +} + +// TestResourceTrackerCleanup_LeavesEBPFEntryOwnedByAnotherAttachment is the +// rollback-collision race exercised at resourceTracker.cleanup's own level +// (bgp_ebpf_test.go's TestUnregisterEBPFDatapath_LeavesEntryOwnedByAnotherAttachment +// covers the same guard one layer lower, calling unregisterEBPFDatapath +// directly). retryK8sOps can re-run publishBGPState's whole closure without +// re-registering the eBPF entry, so a later attempt's checkArgumentCollision +// failure can trigger cleanup after the (block, argument) slot this +// attachment originally wrote has already been overwritten by the very +// other attachment the collision was detected against. cleanup must resolve +// its own vrf.TableID and leave the slot alone when it no longer matches, +// rather than deleting a live attachment's forwarding entry. +func TestResourceTrackerCleanup_LeavesEBPFEntryOwnedByAnotherAttachment(t *testing.T) { + requireRoot(t) + + if err := vrf.Add(testVPC, testAttachment); err != nil { + t.Fatalf("vrf.Add: %v", err) + } + t.Cleanup(func() { _ = vrf.Delete(testVPC, testAttachment) }) + + pinDir := fmt.Sprintf("/sys/fs/bpf/galactic-bgp-test-%d", os.Getpid()) + t.Cleanup(func() { _ = os.RemoveAll(pinDir) }) + // cleanup (resource.go) reads the pin directory from the package-level + // ebpfPinDir var, not attach.PinDir directly — point it at this test's + // own throwaway directory instead of the real production one. + origPinDir := ebpfPinDir + ebpfPinDir = pinDir + t.Cleanup(func() { ebpfPinDir = origPinDir }) + + loaderObjs, err := attach.Load(pinDir) + if err != nil { + t.Fatalf("attach.Load: %v", err) + } + t.Cleanup(func() { _ = loaderObjs.Close() }) + + reg, closer, err := usidmap.OpenPinnedRegistry(pinDir) + if err != nil { + t.Fatalf("OpenPinnedRegistry: %v", err) + } + defer func() { _ = closer.Close() }() + + const testBlock uint64 = 0x0102030405 + const testArgument uint16 = 0x042 + const anotherAttachmentsVRFTableID uint32 = 0x9999 + + // Simulate the colliding (winning) attachment having since overwritten + // this same (block, argument) slot with its own, different VRF table id. + if err := reg.VRF.Register(testBlock, testArgument, anotherAttachmentsVRFTableID, usidmap.EgressKindVeth); err != nil { + t.Fatalf("seed vrf_table entry: %v", err) + } + + tracker := &resourceTracker{ + vpc: testVPC, + vpcAttachment: testAttachment, + publishResult: publishResult{ + ebpfRegistered: true, + ebpfBlock: testBlock, + ebpfArgument: testArgument, + }, + } + + tracker.cleanup(context.Background()) + + entry, ok, err := reg.VRF.Get(testBlock, testArgument) + if err != nil || !ok { + t.Fatalf("vrf_table entry after cleanup: ok=%v err=%v, want ok=true (must survive, it's not this attachment's)", + ok, err) + } + if entry.VRFTableID != anotherAttachmentsVRFTableID { + t.Errorf("vrf_table entry VRFTableID after cleanup = %#x, want unchanged %#x", + entry.VRFTableID, anotherAttachmentsVRFTableID) + } +} diff --git a/internal/cnibgp/types.go b/internal/cnibgp/types.go new file mode 100644 index 00000000..d92745eb --- /dev/null +++ b/internal/cnibgp/types.go @@ -0,0 +1,27 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnibgp + +import ( + "github.com/containernetworking/cni/pkg/types" + + "go.datum.net/galactic/internal/cni/hostconf" +) + +// PluginConf is the CNI plugin configuration passed via stdin on each +// invocation of galactic-bgp — the same document the master plugin itself +// received, since the CNI runtime passes each chain entry its own stanza +// plus prevResult; galactic-bgp only reads vpc/vpcattachment/namespace out +// of it (mtu, terminations, ipam are the master's/galactic-ipam's own +// concerns). +type PluginConf struct { + types.PluginConf + VPC string `json:"vpc"` + VPCAttachment string `json:"vpcattachment"` + Namespace string `json:"namespace,omitempty"` +} + +// HostConf holds node-local settings read from /etc/cni/net.d/10-galactic.conflist. +type HostConf = hostconf.HostConf diff --git a/internal/cnitap/cnitap_test.go b/internal/cnitap/cnitap_test.go index c8a98a63..51877c0f 100644 --- a/internal/cnitap/cnitap_test.go +++ b/internal/cnitap/cnitap_test.go @@ -284,7 +284,7 @@ func TestCmdStatusAPIProbeFailure(t *testing.T) { func TestResourceTrackerCleanupZeroValue(t *testing.T) { tracker := &resourceTracker{} - tracker.cleanup(t.Context()) // should not panic + tracker.cleanup() // should not panic } // ---- loadHostConf / logging ----------------------------------------------- diff --git a/internal/cnitap/ops_add.go b/internal/cnitap/ops_add.go index f352441d..fe442a4b 100644 --- a/internal/cnitap/ops_add.go +++ b/internal/cnitap/ops_add.go @@ -6,7 +6,6 @@ package cnitap import ( "context" - "errors" "fmt" "log/slog" "os" @@ -16,20 +15,20 @@ import ( "github.com/containernetworking/plugins/pkg/ipam" "github.com/vishvananda/netlink" + "go.datum.net/galactic/internal/cni/hostgw" "go.datum.net/galactic/internal/cni/nadpatch" "go.datum.net/galactic/internal/cni/route" "go.datum.net/galactic/internal/cni/tap" - "go.datum.net/galactic/internal/cnibgp" "go.datum.net/galactic/internal/cniipam" "go.datum.net/galactic/internal/plumbing/intf" "go.datum.net/galactic/internal/plumbing/vrf" ) -// cmdAdd mirrors internal/cni's own cmdAdd (see its doc comment for why the -// return is named), minus everything specific to a guest-side netns: no -// host-device delegation, no guest interface, no netns IP configuration. -// The VM manages its own interface entirely; galactic-tap-cni only -// configures the host side and publishes BGP state for it. +// cmdAdd mirrors internal/cni's own cmdAdd, minus everything specific to a +// guest-side netns: no host-device delegation, no guest interface, no +// netns IP configuration. The VM manages its own interface entirely. +// BGP/SRv6/eBPF publish is galactic-bgp's job, invoked next by the CNI +// runtime per conflist order, not by this process. func cmdAdd(args *skel.CmdArgs) (err error) { pluginConf, err := parseConf(args.StdinData) if err != nil { @@ -57,7 +56,6 @@ func cmdAdd(args *skel.CmdArgs) (err error) { tracker := &resourceTracker{ vpc: pluginConf.VPC, vpcAttachment: pluginConf.VPCAttachment, - namespace: namespace, } // Record IPAM delegation intent up front, before the ExecAdd call // below ever runs — see resourceTracker's ipamDelegated doc comment. @@ -67,13 +65,11 @@ func cmdAdd(args *skel.CmdArgs) (err error) { tracker.ipamStdin = args.StdinData } - rollbackCtx, rollbackCancel := context.WithTimeout(context.Background(), cniTimeout) defer func() { if err != nil { slog.Error("ADD: failed, rolling back created resources", "err", err, "containerID", args.ContainerID, "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment) - tracker.cleanup(rollbackCtx) - rollbackCancel() + tracker.cleanup() } }() @@ -100,9 +96,10 @@ func cmdAdd(args *skel.CmdArgs) (err error) { if err != nil { return fmt.Errorf("create k8s client: %w", err) } - tracker.k8s = k8sClient podNamespace := nadpatch.ParsePodNamespace(args.Args) - if err := nadpatch.AnnotateNAD(rollbackCtx, k8sClient, pluginConf.Name, podNamespace, hostName); err != nil { + nadCtx, nadCancel := context.WithTimeout(context.Background(), cniTimeout) + defer nadCancel() + if err := nadpatch.AnnotateNAD(nadCtx, k8sClient, pluginConf.Name, podNamespace, hostName); err != nil { return fmt.Errorf("annotate NAD: %w", err) } @@ -138,8 +135,10 @@ func cmdAdd(args *skel.CmdArgs) (err error) { "ipv4Address", ipamResult.IPv4Address, "ipv4Gateway", ipamResult.IPv4Gateway) } - // Configure the gateway address on the host tap and install the VRF route. - if err := cnibgp.ConfigureHostGateway(pluginConf.VPC, pluginConf.VPCAttachment, ipamResult, nil); err != nil { + // Configure the gateway address on the host tap and install the VRF + // route — kernel-interface work this plugin owns (see + // internal/cni/hostgw's doc comment). + if err := hostgw.ConfigureHostGateway(pluginConf.VPC, pluginConf.VPCAttachment, ipamResult, nil); err != nil { return err } if ipamResult != nil && ipamResult.IPv6Gateway != nil { @@ -147,26 +146,5 @@ func cmdAdd(args *skel.CmdArgs) (err error) { } result := buildTapResult(pluginConf, ipamResult, hostName, hostMac, hostMTU) - if err := types.PrintResult(result, pluginConf.CNIVersion); err != nil { - return fmt.Errorf("print CNI result: %w", err) - } - - vpcHex, err := intf.Base62ToHex(pluginConf.VPC) - if err != nil { - return fmt.Errorf("decode VPC: %w", err) - } - - if tracker.k8s == nil { - return errors.New("k8s client not set in tracker") - } - slog.Debug("ADD: publishing BGP state", "containerID", args.ContainerID) - pubResult, err := cnibgp.PublishBGPStateK8s( - args, cnibgp.PublishConfig{VPC: pluginConf.VPC, VPCAttachment: pluginConf.VPCAttachment, InterfaceType: "tap"}, - nodeName, namespace, ipamResult, vpcHex, tracker.k8s) - tracker.vrfInstanceCreated = pubResult.VRFInstanceCreated - tracker.advCreated = pubResult.AdvertisementCreated - tracker.ebpfRegistered = pubResult.EBPFRegistered - tracker.ebpfBlock = pubResult.EBPFBlock - tracker.ebpfArgument = pubResult.EBPFArgument - return err + return types.PrintResult(result, pluginConf.CNIVersion) } diff --git a/internal/cnitap/resource.go b/internal/cnitap/resource.go index 9e3d0790..87742425 100644 --- a/internal/cnitap/resource.go +++ b/internal/cnitap/resource.go @@ -5,35 +5,34 @@ package cnitap import ( - "context" "fmt" "log/slog" "github.com/containernetworking/plugins/pkg/ipam" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "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" - "go.datum.net/galactic/internal/cni/crdnames" "go.datum.net/galactic/internal/cni/tap" - "go.datum.net/galactic/internal/cnibgp" - "go.datum.net/galactic/internal/plumbing/ebpf/attach" "go.datum.net/galactic/internal/plumbing/vrf" - bgpv1alpha1 "go.datum.net/network/api/v1alpha1" ) var cniScheme = runtime.NewScheme() func init() { utilruntime.Must(clientgoscheme.AddToScheme(cniScheme)) - utilruntime.Must(bgpv1alpha1.AddToScheme(cniScheme)) + // No BGP CRD scheme registration: this package no longer reads or + // writes BGP CRDs itself (that's galactic-bgp's own, chain-invoked + // concern now — internal/cnibgp/resource.go registers bgpv1alpha1 for + // its own client), and NAD annotation's unstructured.Unstructured Patch + // call doesn't require any scheme registration at all. } // newK8sClient creates a new Kubernetes client using the in-cluster config, -// scoped to cniScheme. +// scoped to cniScheme. The only k8s call this plugin makes directly is the +// NAD annotation patch. func newK8sClient() (client.Client, error) { restCfg, err := ctrl.GetConfig() if err != nil { @@ -47,23 +46,14 @@ func newK8sClient() (client.Client, error) { } // resourceTracker tracks resources created during cmdAdd for selective -// rollback. galactic-tap-cni is tap-only, so this is scoped to exactly what -// its own ADD creates: the VRF, the tap device, and — for now, until BGP -// publish becomes its own chain-invoked plugin — the BGP CRDs and eBPF -// vrf_table entry that internal/cnibgp.PublishBGPStateK8s wrote on its -// behalf. Mirrors internal/cni's own resourceTracker. +// rollback. galactic-tap-cni's ADD only ever creates the VRF, the tap +// device, and (if delegated) an IPAM allocation — BGP/SRv6/eBPF publish is +// galactic-bgp's own, separately chain-invoked plugin now, with its own +// smaller tracker (internal/cnibgp). type resourceTracker struct { vpc, vpcAttachment string vrfCreated bool routesCreated int - vrfInstanceCreated bool - advCreated bool - k8s client.Client - namespace string - - ebpfRegistered bool - ebpfBlock uint64 - ebpfArgument uint16 // ipamDelegated, ipamType, and ipamStdin record enough to release the // IPAM allocation during rollback — see internal/cni's own @@ -75,53 +65,10 @@ type resourceTracker struct { ipamStdin []byte } -func (rt *resourceTracker) cleanup(ctx context.Context) { +func (rt *resourceTracker) cleanup() { slog.Info("Selective rollback: cleaning up resources created during failed ADD", "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) - if rt.advCreated && rt.k8s != nil { - adv := &bgpv1alpha1.BGPAdvertisement{ - ObjectMeta: metav1.ObjectMeta{ - Name: crdnames.BGPAdvertisementName(rt.vpc, rt.vpcAttachment), - Namespace: rt.namespace, - }, - } - if err := rt.k8s.Delete(ctx, adv); client.IgnoreNotFound(err) != nil { - slog.Error("Rollback: failed to delete BGPAdvertisement", "err", err, - "name", adv.Name, "namespace", rt.namespace) - } else { - slog.Debug("Rollback: deleted BGPAdvertisement", "name", adv.Name, "namespace", rt.namespace) - } - } - - if rt.vrfInstanceCreated && rt.k8s != nil { - vrfInst := &bgpv1alpha1.BGPVRFInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: crdnames.BGPVRFInstanceName(rt.vpc, rt.vpcAttachment), - Namespace: rt.namespace, - }, - } - if err := rt.k8s.Delete(ctx, vrfInst); client.IgnoreNotFound(err) != nil { - slog.Error("Rollback: failed to delete BGPVRFInstance", "err", err, - "name", vrfInst.Name, "namespace", rt.namespace) - } else { - slog.Debug("Rollback: deleted BGPVRFInstance", "name", vrfInst.Name, "namespace", rt.namespace) - } - } - - if rt.ebpfRegistered { - if vrfTableID, err := vrf.TableID(rt.vpc, rt.vpcAttachment); err != nil { - slog.Error("Rollback: failed to resolve VRF table id, skipping eBPF vrf_table unregister", "err", err, - "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) - } else if err := cnibgp.UnregisterEBPFDatapath(rt.ebpfBlock, rt.ebpfArgument, vrfTableID, attach.PinDir); err != nil { - slog.Error("Rollback: failed to unregister eBPF vrf_table entry", "err", err, - "block", rt.ebpfBlock, "argument", rt.ebpfArgument) - } else { - slog.Debug("Rollback: unregistered eBPF vrf_table entry", - "block", rt.ebpfBlock, "argument", rt.ebpfArgument) - } - } - if rt.ipamDelegated { if err := ipam.ExecDel(rt.ipamType, rt.ipamStdin); err != nil { slog.Error("Rollback: failed to release IPAM allocation", "err", err, "ipamType", rt.ipamType) diff --git a/internal/cnitap/result.go b/internal/cnitap/result.go index abe09b45..cb21b894 100644 --- a/internal/cnitap/result.go +++ b/internal/cnitap/result.go @@ -15,9 +15,11 @@ import ( // buildTapResult constructs the CNI result for tap mode: a single host // interface with optional IPAM data. The guest VM manages its own -// interface; the IP here describes the allocated subnet for BGP -// advertisement. The IPv4 address is reported with a /25 mask, matching the -// mask cnibgp.ConfigureHostGateway installs on the host side of the tap. +// interface; the IP here describes the allocated subnet, which +// galactic-bgp (chained next by the runtime) reads back out of this same +// result to know what to advertise — see internal/cnibgp's doc comment. +// The IPv4 address is reported with a /25 mask, matching the mask +// hostgw.ConfigureHostGateway installs on the host side of the tap. func buildTapResult( pluginConf *PluginConf, ipRes *cniipam.IPAMResult, diff --git a/internal/installer/installer.go b/internal/installer/installer.go index 011d7c11..3cc4c868 100644 --- a/internal/installer/installer.go +++ b/internal/installer/installer.go @@ -68,6 +68,7 @@ var ( SourceCNIBinary = "/galactic-cni" SourceTapCNIBinary = "/galactic-tap-cni" SourceIPAMBinary = "/galactic-ipam" + SourceBGPBinary = "/galactic-bgp" SourceHostDeviceBinary = "/host-device" ) @@ -272,6 +273,9 @@ func Bootstrap(ctx context.Context, nodeName string) error { if err := atomicCopyFile(SourceIPAMBinary, filepath.Join(HostBinDir, "galactic-ipam"), 0755); err != nil { return fmt.Errorf("copy galactic-ipam binary: %w", err) } + if err := atomicCopyFile(SourceBGPBinary, filepath.Join(HostBinDir, "galactic-bgp"), 0755); err != nil { + return fmt.Errorf("copy galactic-bgp binary: %w", err) + } if err := atomicCopyFile(SourceHostDeviceBinary, filepath.Join(HostBinDir, "host-device"), 0755); err != nil { return fmt.Errorf("copy host-device binary: %w", err) } diff --git a/internal/installer/installer_test.go b/internal/installer/installer_test.go index 2041da33..b21befc7 100644 --- a/internal/installer/installer_test.go +++ b/internal/installer/installer_test.go @@ -82,6 +82,17 @@ func assertBinaryCopied(t *testing.T, path, wantContent string) { } } +// writeSourceBinary writes content to path, failing the test on error. +// Factored out alongside assertBinaryCopied for the same reason — each +// binary this test seeds would otherwise add its own branch to +// TestBootstrap's own cyclomatic complexity. +func writeSourceBinary(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0755); err != nil { + t.Fatalf("write source binary %q: %v", path, err) + } +} + func TestBootstrap(t *testing.T) { // Set up temporary directories for testing overrides tmpDir := t.TempDir() @@ -105,19 +116,13 @@ func TestBootstrap(t *testing.T) { SourceCNIBinary = filepath.Join(tmpDir, "source-galactic-cni") SourceTapCNIBinary = filepath.Join(tmpDir, "source-galactic-tap-cni") SourceIPAMBinary = filepath.Join(tmpDir, "source-galactic-ipam") + SourceBGPBinary = filepath.Join(tmpDir, "source-galactic-bgp") SourceHostDeviceBinary = filepath.Join(tmpDir, "source-host-device") - if err := os.WriteFile(SourceCNIBinary, []byte("cni-content"), 0755); err != nil { - t.Fatalf("write SourceCNIBinary: %v", err) - } - if err := os.WriteFile(SourceTapCNIBinary, []byte("tap-cni-content"), 0755); err != nil { - t.Fatalf("write SourceTapCNIBinary: %v", err) - } - if err := os.WriteFile(SourceIPAMBinary, []byte("ipam-content"), 0755); err != nil { - t.Fatalf("write SourceIPAMBinary: %v", err) - } - if err := os.WriteFile(SourceHostDeviceBinary, []byte("host-device-content"), 0755); err != nil { - t.Fatalf("write SourceHostDeviceBinary: %v", err) - } + writeSourceBinary(t, SourceCNIBinary, "cni-content") + writeSourceBinary(t, SourceTapCNIBinary, "tap-cni-content") + writeSourceBinary(t, SourceIPAMBinary, "ipam-content") + writeSourceBinary(t, SourceBGPBinary, "bgp-content") + writeSourceBinary(t, SourceHostDeviceBinary, "host-device-content") // Mock node object node := &corev1.Node{ @@ -173,6 +178,7 @@ func TestBootstrap(t *testing.T) { assertBinaryCopied(t, filepath.Join(HostBinDir, "galactic-cni"), "cni-content") assertBinaryCopied(t, filepath.Join(HostBinDir, "galactic-tap-cni"), "tap-cni-content") assertBinaryCopied(t, filepath.Join(HostBinDir, "galactic-ipam"), "ipam-content") + assertBinaryCopied(t, filepath.Join(HostBinDir, "galactic-bgp"), "bgp-content") // Verify conflist written conflist, err := loadHostConf(HostConflist) diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index f1df4ead..9a4b807d 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -212,12 +212,13 @@ func TestCNITapInterface(t *testing.T) { } // The eBPF uSID datapath is now the only forwarding path (see - // internal/cnibgp/bgp.go's registerEBPFDatapath, called inline from - // galactic-tap-cni's own cmdAdd at this point in the CNI plugin-chain - // split), so CNI ADD requires this node's locator_table/function_table/ - // vrf_table maps to already be pinned under attach.PinDir. In - // production that's done ahead of time by the CNI DaemonSet's - // long-running "credential-refresh" container (config/cni/ + // internal/cnibgp/bgp.go's registerEBPFDatapath, called from + // galactic-bgp's own cmdAdd — a separately chain-invoked binary since + // the CNI plugin-chain split, not inline in galactic-tap-cni's cmdAdd + // any more), so CNI ADD requires this node's locator_table/ + // function_table/vrf_table maps to already be pinned under + // attach.PinDir. In production that's done ahead of time by the CNI + // DaemonSet's long-running "credential-refresh" container (config/cni/ // daemonset.yaml, `/galactic-cni run`); this test runs its own pod // instead of relying on that DaemonSet, so it must start the same // control daemon itself before exercising CNI ADD below. @@ -286,13 +287,13 @@ NODE_NAME=` + nodeName() + ` \ t.Fatalf("CNI ADD failed: %v", err) } - // The CNI result is JSON; find the first '{'. - jsonStart := strings.Index(out, "{") - if jsonStart == -1 { - t.Fatalf("no JSON found in CNI ADD output:\n%s", out) - } + // The CNI result is JSON on stdout, but kubectl exec interleaves + // stderr (slog log lines) into the captured output. Decode only + // the first JSON value so trailing log lines are ignored. var result map[string]any - if err := json.Unmarshal([]byte(out[jsonStart:]), &result); err != nil { + if jsonStart := strings.Index(out, "{"); jsonStart == -1 { + t.Fatalf("no JSON found in CNI ADD output:\n%s", out) + } else if err := json.NewDecoder(strings.NewReader(out[jsonStart:])).Decode(&result); err != nil { t.Fatalf("CNI ADD output is not valid JSON: %v\noutput:\n%s", err, out) } @@ -323,6 +324,109 @@ NODE_NAME=` + nodeName() + ` \ if len(ips) != 1 { t.Errorf("ips count = %d, want 1", len(ips)) } + + // Step 3: chain galactic-bgp — the CNI runtime's next plugin in the + // conflist — after galactic-tap-cni, feeding it the tap master's own + // CNI result as prevResult, exactly as the runtime would. Everything + // up to this point only exercises galactic-tap-cni's own cmdAdd; + // BGPVRFInstance/BGPAdvertisement CRD creation and eBPF + // locator_table/function_table/vrf_table registration all moved into + // galactic-bgp's own cmdAdd with the CNI plugin-chain split (see + // internal/cnibgp's doc comment), so without this step the whole BGP + // publish path has no e2e coverage on the ADD path at all. + testChainedGalacticBGP(t, name, result) +} + +// testChainedGalacticBGP runs galactic-bgp's own ADD, chained after the tap +// master's ADD (tapResult is exactly what that ADD printed, fed through as +// prevResult — see internal/cnibgp/prevresult.go's inferFromPrevResult), +// then CHECK. It asserts the BGPVRFInstance/BGPAdvertisement CRDs exist +// after ADD, and — since checkEBPFEntry (internal/cnibgp/ops_check.go) reads +// back the locator_table, function_table, and vrf_table entries +// registerEBPFDatapath wrote and fails if any are missing or inconsistent — +// that CHECK succeeding is itself the assertion that eBPF registration +// happened correctly on ADD; there is no separate bpftool-style dump here. +func testChainedGalacticBGP(t *testing.T, podName string, tapResult map[string]any) { + t.Helper() + + const vpc, vpcAttachment = "1", "1" + crdName := vpc + "-" + vpcAttachment + t.Cleanup(func() { + //nolint:errcheck // best-effort cleanup, mirrors deletePod + kubectl(context.Background(), "delete", "bgpvrfinstance", crdName, "--ignore-not-found") + //nolint:errcheck // best-effort cleanup, mirrors deletePod + kubectl(context.Background(), "delete", "bgpadvertisement", crdName, "--ignore-not-found") + }) + + prevResultJSON, err := json.Marshal(tapResult) + if err != nil { + t.Fatalf("marshal tap ADD result for prevResult: %v", err) + } + bgpConf := fmt.Sprintf(`{ + "cniVersion": "1.0.0", + "name": "galactic", + "type": "galactic-bgp", + "vpc": %q, + "vpcattachment": %q, + "prevResult": %s +}`, vpc, vpcAttachment, prevResultJSON) + + // Reuses the same netns/containerID/ifname galactic-tap-cni's own step + // (above) already set up: a real chained plugin sees the identical + // values across every plugin invoked for one CNI ADD. + bgpScript := `#!/bin/sh +CNI_NETNS=/var/run/netns/e2e-tap-ns \ +CNI_COMMAND=$1 \ +CNI_CONTAINERID=e2e-tap-001 \ +CNI_IFNAME=eth0 \ +CNI_PATH=/ \ +NODE_NAME=` + nodeName() + ` \ + /galactic-bgp < /tmp/cni-bgp.json +` + _, err = kubectl(t.Context(), "exec", podName, "--", + "sh", "-c", + "echo '"+bgpConf+"' > /tmp/cni-bgp.json && "+ + "echo '"+bgpScript+"' > /tmp/run-bgp.sh && "+ + "chmod +x /tmp/run-bgp.sh", + ) + if err != nil { + t.Fatalf("write galactic-bgp config and script: %v", err) + } + + addOut, err := kubectl(t.Context(), "exec", podName, "-i", "--", "/tmp/run-bgp.sh", "ADD") + if err != nil { + t.Fatalf("galactic-bgp ADD failed: %v\noutput: %s", err, addOut) + } + + // galactic-bgp is the last plugin in the chain: its own result is + // prevResult passed through unchanged, not a new one it builds itself. + jsonStart := strings.Index(addOut, "{") + if jsonStart == -1 { + t.Fatalf("no JSON found in galactic-bgp ADD output:\n%s", addOut) + } + var bgpResult map[string]any + if err := json.Unmarshal([]byte(addOut[jsonStart:]), &bgpResult); err != nil { + t.Fatalf("galactic-bgp ADD output is not valid JSON: %v\noutput:\n%s", err, addOut) + } + if bgpIfaces, _ := bgpResult["interfaces"].([]any); len(bgpIfaces) != 1 { + t.Errorf("galactic-bgp ADD result interfaces count = %d, want 1 (passed through from prevResult unchanged)", + len(bgpIfaces)) + } + + if out, err := kubectl(t.Context(), "get", "bgpvrfinstance", crdName); err != nil { + t.Errorf("BGPVRFInstance %s not found after galactic-bgp ADD: %v\n%s", crdName, err, out) + } + if out, err := kubectl(t.Context(), "get", "bgpadvertisement", crdName); err != nil { + t.Errorf("BGPAdvertisement %s not found after galactic-bgp ADD: %v\n%s", crdName, err, out) + } + + // CHECK reads back the locator_table/function_table/vrf_table entries + // registerEBPFDatapath wrote on ADD (see ops_check.go's checkEBPFEntry) + // — its success is this test's assertion that eBPF registration + // actually happened, not just that the CRDs exist. + if checkOut, err := kubectl(t.Context(), "exec", podName, "-i", "--", "/tmp/run-bgp.sh", "CHECK"); err != nil { + t.Errorf("galactic-bgp CHECK failed: %v\noutput: %s", err, checkOut) + } } // startEBPFControlDaemon runs `/galactic-cni run` inside the already-running