From 93ba7d8c74eb3bdb21e650c2f552cbab4ca9097c Mon Sep 17 00:00:00 2001 From: Peter Sprygada Date: Sat, 8 Aug 2026 00:41:44 -0400 Subject: [PATCH 1/2] refactor(cni): extract galactic-route as its own CNI chain plugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 3 of the CNI plugin-chain split (galactic/plan-cni-plugin-chain): pulls termination-route installation out of the veth and tap master plugins into its own chained CNI binary, galactic-route, invoked between the master plugin and galactic-bgp per conflist order. Unlike every other binary in the chain, galactic-route has zero Kubernetes dependency — it neither reads nor writes any CRD, and never needs a namespace. internal/cniroute is the new plugin package, mirroring the shape established by cniipam/cnibgp: - cniroute.go: RunPlugin() entrypoint (skel.PluginMainFuncs, ADD/DEL/ CHECK/STATUS/VERSION). - types.go/config.go: PluginConf{VPC, VPCAttachment, Terminations}, parsed from stdin — the same document the master plugin itself received, since the CNI runtime passes every chain entry its own stanza plus prevResult. parseConf still reuses config.CNIConfig for LogFile/LogLevel's env-var > conflist > default precedence (so logging behaves identically to every other binary), but — unlike galactic-bgp — never resolves NodeName or Kubeconfig, since nothing here ever talks to the API server. - ops_add.go: cmdAdd installs each termination as a VRF route via the existing internal/cni/route package (route.Add), deriving the host device name from (vpc, vpcAttachment) alone via intf.GenerateInterfaceNameHost — identical for a veth master's host end and a tap master's tap device, so galactic-route needs no interface-kind inference the way galactic-bgp does. It then passes prevResult through unchanged, adding no interfaces or IPs of its own. Requires a non-nil prevResult (galactic-route must be chained after a master plugin) and reads it from RawPrevResult, not the never-populated typed PrevResult field. - ops_del.go: cmdDel is a no-op, same as every other binary in the chain — termination routes are keyed by (vpc, vpcAttachment) and may still be in use by another pod/VM sharing the same attachment, so cleanup is left entirely to galactic-router's GC controller. This matches the pre-split behavior too: the old monolithic plugin's own DEL never deleted termination routes either, for the same reason — extracting this into its own binary changes nothing about when routes actually get removed. - ops_check.go: cmdCheck is checkTerminationRoutes, moved unchanged from internal/cni/ops_check.go (also mirrored in internal/cnitap). cmdStatus is a trivial always-ready success — galactic-route has nothing external to probe, matching galactic-ipam's own STATUS, implemented for uniformity across the chain per the plan's decision rather than skipped. - resource.go: a resourceTracker scoped to exactly what galactic- route's own ADD creates — the termination routes it actually installed (route-delete only). Rollback deletes only the routes recorded as added, never routes a failed route.Add call never reached. internal/cni and internal/cnitap: dropped the Terminations field from each PluginConf (both packages had their own copy of a Termination type, now living only in cniroute since neither master plugin reads "terminations" out of its own stanza anymore), the route.Add loop and routesCreated tracker field from ops_add.go/resource.go, and the checkTerminationRoutes call from ops_check.go's CHECK path (the function itself moved to cniroute, verbatim). Taskfile.yaml, containers/galactic-cni/Dockerfile, and internal/installer/installer.go (SourceRouteBinary) gain galactic-route alongside the four other chain binaries, following the exact pattern established for those in steps 0-2. Verification: task lint (0 issues), task build (all 8 binaries, including galactic-route), task test:unit all green. internal/cniroute lands at 62.9% coverage — its first-ever test coverage, since internal/cni/route (the package it wraps) had none before this split either; backfilling that package's own tests is unrelated to this split's scope and left as-is. task test:e2e not run in this step, same caveat as steps 0-2 (requires sudo modprobe vrf plus a Kind cluster bring-up, deferred to the end of the full stack per the plan's verification approach). --- Taskfile.yaml | 1 + cmd/galactic-route/main.go | 92 +++++++ containers/galactic-cni/Dockerfile | 16 ++ internal/cni/cni_test.go | 3 - internal/cni/ops_add.go | 13 +- internal/cni/ops_check.go | 57 +---- internal/cni/resource.go | 6 +- internal/cni/types.go | 11 +- internal/cniroute/cniroute.go | 27 ++ internal/cniroute/cniroute_test.go | 369 +++++++++++++++++++++++++++ internal/cniroute/config.go | 237 +++++++++++++++++ internal/cniroute/ops_add.go | 88 +++++++ internal/cniroute/ops_check.go | 105 ++++++++ internal/cniroute/ops_del.go | 30 +++ internal/cniroute/resource.go | 37 +++ internal/cniroute/types.go | 45 ++++ internal/cnitap/ops_add.go | 13 +- internal/cnitap/ops_check.go | 56 +--- internal/cnitap/resource.go | 4 +- internal/cnitap/types.go | 12 +- internal/installer/installer.go | 4 + internal/installer/installer_test.go | 3 + 22 files changed, 1075 insertions(+), 154 deletions(-) create mode 100644 cmd/galactic-route/main.go create mode 100644 internal/cniroute/cniroute.go create mode 100644 internal/cniroute/cniroute_test.go create mode 100644 internal/cniroute/config.go create mode 100644 internal/cniroute/ops_add.go create mode 100644 internal/cniroute/ops_check.go create mode 100644 internal/cniroute/ops_del.go create mode 100644 internal/cniroute/resource.go create mode 100644 internal/cniroute/types.go diff --git a/Taskfile.yaml b/Taskfile.yaml index d2e5341d..0dbe9d88 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -136,6 +136,7 @@ tasks: - 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-route ./cmd/galactic-route - 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-route/main.go b/cmd/galactic-route/main.go new file mode 100644 index 00000000..d5a92784 --- /dev/null +++ b/cmd/galactic-route/main.go @@ -0,0 +1,92 @@ +// 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/cniroute" + "go.datum.net/galactic/internal/metadata" +) + +const ( + appName = "galactic-route" + + appDesc = `Galactic Route CNI Plugin + + The termination-route plugin in the galactic CNI chain — chained after + galactic-cni/galactic-tap-cni and before galactic-bgp per conflist order, + never run standalone, and optional (only present for attachments with + terminations to install). Has no Kubernetes dependency at all: it only + installs kernel routes into the VRF routing table the master plugin + already created. + + 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, _ []string) error { + cniroute.InitCNIConfig() + confFile, _ := cmd.Flags().GetString("conf-file") + if confFile != "" { + cniroute.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 or talks to the API server, 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. + cniroute.RunPlugin() + return nil + }, + } + + cmd.PersistentFlags().String("conf-file", cniroute.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 4c41d158..c8c9fe17 100644 --- a/containers/galactic-cni/Dockerfile +++ b/containers/galactic-cni/Dockerfile @@ -88,6 +88,20 @@ RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build \ -X go.datum.net/galactic/internal/metadata.GitURL=${GIT_URL}" \ -o galactic-bgp cmd/galactic-bgp/main.go +# Build galactic-route, the termination-route plugin in the galactic CNI +# chain. Ships in this same image/binary set for the same reason +# galactic-tap-cni/galactic-ipam/galactic-bgp 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-route cmd/galactic-route/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 @@ -125,6 +139,7 @@ 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/galactic-route /galactic-route 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 @@ -140,6 +155,7 @@ 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 /galactic-route /galactic-route 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/internal/cni/cni_test.go b/internal/cni/cni_test.go index a16ddf8e..98714025 100644 --- a/internal/cni/cni_test.go +++ b/internal/cni/cni_test.go @@ -825,9 +825,6 @@ func TestResourceTrackerFieldsSet(t *testing.T) { if tracker.vrfCreated { t.Error("vrfCreated should be false by default") } - if tracker.routesCreated != 0 { - t.Error("routesCreated should be zero by default") - } } // ---- cmdStatus --------------------------------------------------------- diff --git a/internal/cni/ops_add.go b/internal/cni/ops_add.go index 11c616c7..4ad3b830 100644 --- a/internal/cni/ops_add.go +++ b/internal/cni/ops_add.go @@ -15,7 +15,6 @@ import ( "github.com/vishvananda/netlink" "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/plumbing/intf" "go.datum.net/galactic/internal/plumbing/vrf" @@ -114,16 +113,8 @@ func cmdAdd(args *skel.CmdArgs) (err error) { return fmt.Errorf("annotate NAD: %w", err) } - dev := hostName - for _, termination := range pluginConf.Terminations { - if err := route.Add(pluginConf.VPC, pluginConf.VPCAttachment, termination.Network, termination.Via, dev); err != nil { - return fmt.Errorf("add route %s: %w", termination.Network, err) - } - tracker.routesCreated++ - } - if tracker.routesCreated > 0 { - slog.Debug("ADD: termination routes installed", "count", tracker.routesCreated, "dev", dev) - } + // Termination routes are galactic-route's job now — chained next after + // this plugin, when the attachment has any (see internal/cniroute). guestName := intf.GenerateInterfaceNameGuest(pluginConf.VPC, pluginConf.VPCAttachment) return buildVethResult(args, pluginConf, hostName, guestName, hostMac, hostMTU) diff --git a/internal/cni/ops_check.go b/internal/cni/ops_check.go index b325baec..a66ff384 100644 --- a/internal/cni/ops_check.go +++ b/internal/cni/ops_check.go @@ -53,10 +53,9 @@ func cmdCheck(args *skel.CmdArgs) error { errs = append(errs, fmt.Errorf("guest interface %q: %w", guestName, err)) } - // Verify termination routes exist in the VRF table. - if err := checkTerminationRoutes(pluginConf.VPC, pluginConf.VPCAttachment, pluginConf.Terminations); err != nil { - errs = append(errs, fmt.Errorf("termination routes: %w", err)) - } + // Termination routes are galactic-route's own CHECK now (see + // internal/cniroute's checkTerminationRoutes) — this plugin's CHECK no + // longer verifies them. // Validate kernel state against prevResult (CNI spec §4.3). if pluginConf.RawPrevResult != nil { @@ -210,56 +209,6 @@ func checkGuestInterface(netnsPath, ifName string) error { }) } -// checkTerminationRoutes verifies that all termination routes exist in the -// VRF table for the given VPC/VPCAttachment pair. -func checkTerminationRoutes(vpc, vpcAttachment string, terminations []Termination) error { - tableID, err := vrf.TableID(vpc, vpcAttachment) - if err != nil { - return fmt.Errorf("get VRF table ID: %w", err) - } - - handle, err := netlink.NewHandle() - if err != nil { - return fmt.Errorf("create netlink handle: %w", err) - } - defer handle.Close() //nolint:errcheck // netlink cleanup on teardown - - routes, err := handle.RouteListFiltered( - netlink.FAMILY_V6, - &netlink.Route{Table: int(tableID)}, - netlink.RT_FILTER_TABLE, - ) - if err != nil { - return fmt.Errorf("list routes: %w", err) - } - - dev := intf.GenerateInterfaceNameHost(vpc, vpcAttachment) - for _, term := range terminations { - viaIP := net.ParseIP(term.Via) - if viaIP == nil { - return fmt.Errorf("invalid termination gateway %q", term.Via) - } - found := false - for _, r := range routes { - if r.Dst != nil && - r.Dst.String() == term.Network && - r.Gw != nil && - r.Gw.Equal(viaIP) && - r.LinkIndex > 0 { - // Verify the link name matches (defers to the veth/tap device). - if link, linkErr := handle.LinkByIndex(r.LinkIndex); linkErr == nil && link.Attrs().Name == dev { - found = true - break - } - } - } - if !found { - return fmt.Errorf("missing route %s via %s in VRF table %d", term.Network, term.Via, tableID) - } - } - return nil -} - // checkPrevResult validates that kernel state matches the interfaces and IPs // recorded in the prevResult returned by the most recent ADD. Per the CNI spec // §4.3, CHECK must verify that managed resources have not drifted. diff --git a/internal/cni/resource.go b/internal/cni/resource.go index c986b05a..634217f3 100644 --- a/internal/cni/resource.go +++ b/internal/cni/resource.go @@ -49,12 +49,12 @@ func newK8sClient() (client.Client, error) { // 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. +// its own smaller tracker (internal/cnibgp); termination routes are +// galactic-route's own, with its own smaller tracker (internal/cniroute); +// so this one no longer needs to know anything about either. type resourceTracker struct { vpc, vpcAttachment string vrfCreated bool - routesCreated int // ipamDelegated, ipamType, and ipamStdin record enough to release the // IPAM allocation during rollback. Set as soon as pluginConf.IPAM != nil diff --git a/internal/cni/types.go b/internal/cni/types.go index 2857b017..3f6e6e65 100644 --- a/internal/cni/types.go +++ b/internal/cni/types.go @@ -11,13 +11,6 @@ import ( "go.datum.net/galactic/internal/cniipam" ) -// Termination represents a network termination point with a destination -// CIDR and next-hop gateway address. -type Termination struct { - Network string `json:"network"` - Via string `json:"via,omitempty"` -} - // PluginConf is the CNI plugin configuration passed via stdin on each // invocation of galactic-cni, the veth master plugin. // @@ -26,12 +19,14 @@ type Termination struct { // go.datum.net/galactic/internal/cniipam's doc comment for the explicit // delegation contract: this struct only decides *whether* to delegate // (IPAM != nil), never anything about how allocation itself works. +// Termination routes are galactic-route's own concern now (see +// internal/cniroute) — this plugin's own JSON stanza carries no +// "terminations" field of its own to read. type PluginConf struct { types.PluginConf VPC string `json:"vpc"` VPCAttachment string `json:"vpcattachment"` MTU int `json:"mtu,omitempty"` - Terminations []Termination `json:"terminations,omitempty"` IPAM *cniipam.IPAM `json:"ipam"` Namespace string `json:"namespace,omitempty"` } diff --git a/internal/cniroute/cniroute.go b/internal/cniroute/cniroute.go new file mode 100644 index 00000000..7784fa4c --- /dev/null +++ b/internal/cniroute/cniroute.go @@ -0,0 +1,27 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cniroute + +import ( + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/version" + + "go.datum.net/galactic/internal/metadata" +) + +// RunPlugin starts galactic-route, handling the CNI ADD, DEL, CHECK, and +// STATUS operations for the termination-route stage of the chain. +func RunPlugin() { + skel.PluginMainFuncs( + skel.CNIFuncs{ + Add: cmdAdd, + Check: cmdCheck, + Del: cmdDel, + Status: cmdStatus, + }, + version.All, + "CNI galactic-route plugin "+metadata.Version, + ) +} diff --git a/internal/cniroute/cniroute_test.go b/internal/cniroute/cniroute_test.go new file mode 100644 index 00000000..4cfe5d25 --- /dev/null +++ b/internal/cniroute/cniroute_test.go @@ -0,0 +1,369 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cniroute + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "strings" + "testing" + + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/types" +) + +func TestMain(m *testing.M) { + InitCNIConfig() + os.Exit(m.Run()) +} + +const ( + testVPC = "abc" + testAttachment = "def" + testContainerID = "test-container" + testInvalidBase62 = "abc-def" + testMac = "aa:bb:cc:dd:ee:ff" + testIfName = "eth0" + + // testPrevResult is a valid CNI v1.0.0 result used in prevResult tests. + testPrevResult = `{"cniVersion":"1.0.0",` + + `"interfaces":[{"name":"` + testIfName + `","mac":"` + testMac + `",` + + `"sandbox":"/proc/1/ns/net"}],` + + `"ips":[{"version":"6","address":"fd00:1::1/64"}]}` +) + +// confJSON builds a minimal galactic-route CNI config document for tests. +func confJSON(vpc, vpcAttachment, prevResult string) string { + prevResultField := "" + if prevResult != "" { + prevResultField = `,"prevResult":` + prevResult + } + return fmt.Sprintf( + `{"cniVersion":"1.0.0","name":"test","type":"galactic-route",`+ + `"vpc":%q,"vpcattachment":%q%s}`, + vpc, vpcAttachment, prevResultField, + ) +} + +// assertCNIError verifies that err is a *types.Error with the expected Code +// and that its Msg contains wantMsg (substring match). Pass wantMsg == "" to +// skip the message check. +func assertCNIError(t *testing.T, err error, wantCode uint, wantMsg string) { + t.Helper() + var cniErr *types.Error + if !errors.As(err, &cniErr) { + t.Fatalf("expected *types.Error, got %T: %v", err, err) + } + if cniErr.Code != wantCode { + t.Fatalf("expected code %d, got %d (Msg: %q)", wantCode, cniErr.Code, cniErr.Msg) + } + if wantMsg != "" && !strings.Contains(cniErr.Msg, wantMsg) { + t.Fatalf("expected Msg to contain %q, got %q", wantMsg, cniErr.Msg) + } +} + +// ---- parseConf ------------------------------------------------------------- + +func TestParseConfInvalidJSON(t *testing.T) { + _, err := parseConf([]byte("not valid json")) + assertCNIError(t, err, 7, errInvalidCNIConfig) +} + +func TestParseConfMissingVPC(t *testing.T) { + _, err := parseConf([]byte(confJSON("", testAttachment, ""))) + assertCNIError(t, err, 7, errVPCRequired) +} + +func TestParseConfInvalidVPC(t *testing.T) { + _, err := parseConf([]byte(confJSON(testInvalidBase62, testAttachment, ""))) + assertCNIError(t, err, 7, "invalid base62 value for field 'vpc'") +} + +func TestParseConfMissingVPCAttachment(t *testing.T) { + _, err := parseConf([]byte(confJSON(testVPC, "", ""))) + assertCNIError(t, err, 7, errVPCAttachmentRequired) +} + +func TestParseConfInvalidVPCAttachment(t *testing.T) { + _, err := parseConf([]byte(confJSON(testVPC, testInvalidBase62, ""))) + assertCNIError(t, err, 7, "invalid base62 value for field 'vpcattachment'") +} + +func TestParseConfValid(t *testing.T) { + conf, err := parseConf([]byte(confJSON(testVPC, testAttachment, ""))) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if conf.VPC != testVPC || conf.VPCAttachment != testAttachment { + t.Errorf("got vpc=%q vpcAttachment=%q, want %q/%q", conf.VPC, conf.VPCAttachment, testVPC, testAttachment) + } +} + +func TestParseConfWithTerminations(t *testing.T) { + conf := fmt.Sprintf( + `{"cniVersion":"1.0.0","name":"test","type":"galactic-route",`+ + `"vpc":%q,"vpcattachment":%q,`+ + `"terminations":[{"network":"fd00:2::/64","via":"fd00:1::1"}]}`, + testVPC, testAttachment, + ) + parsed, err := parseConf([]byte(conf)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(parsed.Terminations) != 1 { + t.Fatalf("got %d terminations, want 1", len(parsed.Terminations)) + } + if parsed.Terminations[0].Network != "fd00:2::/64" || parsed.Terminations[0].Via != "fd00:1::1" { + t.Errorf("got termination %+v, want network=fd00:2::/64 via=fd00:1::1", parsed.Terminations[0]) + } +} + +func TestParseConfPrevResultNeverValidatedAtParseTime(t *testing.T) { + // types.PluginConf.PrevResult has json tag "-" and is never populated by + // plain json.Unmarshal (a pre-existing quirk of that library — see + // internal/cnibgp/prevresult.go's own doc comment). parseConf's + // validatePrevResult check therefore never fires here regardless of + // what "prevResult" contains; cmdAdd's own parsePrevResult (reading + // RawPrevResult instead) is what actually validates prevResult content. + conf, err := parseConf([]byte(confJSON(testVPC, testAttachment, `{"cniVersion":"garbage"}`))) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if conf.PrevResult != nil { + t.Error("PrevResult should remain nil after plain json.Unmarshal (see RawPrevResult instead)") + } + if conf.RawPrevResult == nil { + t.Error("RawPrevResult should be populated") + } +} + +// ---- isValidBase62 --------------------------------------------------------- + +func TestIsValidBase62(t *testing.T) { + tests := []struct { + name string + in string + want bool + }{ + {"empty", "", false}, + {"valid alnum", "aB3", true}, + {"hyphen", "abc-def", false}, + {"unicode", "abc€", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isValidBase62(tt.in); got != tt.want { + t.Errorf("isValidBase62(%q) = %v, want %v", tt.in, got, tt.want) + } + }) + } +} + +// ---- sanitizeForError ------------------------------------------------------- + +func TestSanitizeForError(t *testing.T) { + if got := sanitizeForError("printable-value"); got != "printable-value" { + t.Errorf("got %q, want unchanged", got) + } + if got := sanitizeForError("bad\x00value"); got != sanitizeForErrorBinary { + t.Errorf("got %q, want %q", got, sanitizeForErrorBinary) + } +} + +// ---- parseStatusConf -------------------------------------------------------- + +func TestParseStatusConfInvalidJSON(t *testing.T) { + err := parseStatusConf([]byte("not valid json")) + assertCNIError(t, err, 7, errInvalidCNIConfig) +} + +func TestParseStatusConfMissingCNIVersion(t *testing.T) { + err := parseStatusConf([]byte(`{"type":"galactic-route"}`)) + assertCNIError(t, err, 7, "cniVersion is required") +} + +func TestParseStatusConfMissingType(t *testing.T) { + err := parseStatusConf([]byte(`{"cniVersion":"1.0.0"}`)) + assertCNIError(t, err, 7, "type is required") +} + +func TestParseStatusConfValid(t *testing.T) { + if err := parseStatusConf([]byte(`{"cniVersion":"1.0.0","type":"galactic-route"}`)); err != nil { + t.Errorf("unexpected error: %v", err) + } +} + +// ---- cmdAdd ----------------------------------------------------------------- + +func TestCmdAddInvalidConfig(t *testing.T) { + args := &skel.CmdArgs{ContainerID: testContainerID, StdinData: []byte("not valid json")} + err := cmdAdd(args) + assertCNIError(t, err, 7, errInvalidCNIConfig) +} + +func TestCmdAddNoPrevResult(t *testing.T) { + args := &skel.CmdArgs{ + ContainerID: testContainerID, + StdinData: []byte(confJSON(testVPC, testAttachment, "")), + } + err := cmdAdd(args) + assertCNIError(t, err, 6, "must be chained after a master plugin") +} + +func TestCmdAddInvalidPrevResult(t *testing.T) { + // A prevResult that unmarshals but isn't a parseable versioned CNI + // result. Caught by cmdAdd's own parsePrevResult, reading RawPrevResult + // — parseConf's validatePrevResult(conf.PrevResult) never sees this at + // all, since that field is never populated (see + // TestParseConfPrevResultNeverValidatedAtParseTime). + args := &skel.CmdArgs{ + ContainerID: testContainerID, + StdinData: []byte(confJSON(testVPC, testAttachment, `{"cniVersion":"garbage"}`)), + } + err := cmdAdd(args) + assertCNIError(t, err, 6, "parse prevResult") +} + +func TestCmdAddNoTerminationsPassesThroughPrevResult(t *testing.T) { + // With no terminations to install, cmdAdd never touches route.Add at + // all — it should succeed and simply echo prevResult back. + args := &skel.CmdArgs{ + ContainerID: testContainerID, + StdinData: []byte(confJSON(testVPC, testAttachment, testPrevResult)), + } + if err := cmdAdd(args); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// ---- parsePrevResult -------------------------------------------------------- + +func TestParsePrevResultNil(t *testing.T) { + _, err := parsePrevResult(nil) + if err == nil { + t.Fatal("expected error for nil prevResult") + } +} + +func TestParsePrevResultValid(t *testing.T) { + var raw map[string]interface{} + if err := json.Unmarshal([]byte(testPrevResult), &raw); err != nil { + t.Fatalf("unmarshal test fixture: %v", err) + } + result, err := parsePrevResult(raw) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result == nil { + t.Fatal("expected non-nil result") + } +} + +// ---- cmdDel ----------------------------------------------------------------- + +func TestCmdDelIdempotent(t *testing.T) { + args := &skel.CmdArgs{ + ContainerID: testContainerID, + StdinData: []byte(confJSON(testVPC, testAttachment, "")), + } + if err := cmdDel(args); err != nil { + t.Errorf("cmdDel should never return an error, got: %v", err) + } +} + +func TestCmdDelIdempotentInvalidConfig(t *testing.T) { + args := &skel.CmdArgs{ContainerID: testContainerID, StdinData: []byte("not valid json")} + if err := cmdDel(args); err != nil { + t.Errorf("cmdDel should never return an error even for unparseable config, got: %v", err) + } +} + +// ---- cmdCheck --------------------------------------------------------------- + +func TestCmdCheckInvalidConfig(t *testing.T) { + args := &skel.CmdArgs{ContainerID: testContainerID, StdinData: []byte("not valid json")} + err := cmdCheck(args) + assertCNIError(t, err, 7, errInvalidCNIConfig) +} + +func TestCmdCheckMissingRoute(t *testing.T) { + // The VRF this termination's route should live in doesn't exist on the + // test host, so checkTerminationRoutes fails at the vrf.TableID lookup — + // the same "missing resources" shape internal/cni's own CHECK tests use. + conf := fmt.Sprintf( + `{"cniVersion":"1.0.0","name":"test","type":"galactic-route",`+ + `"vpc":%q,"vpcattachment":%q,`+ + `"terminations":[{"network":"fd00:2::/64","via":"fd00:1::1"}]}`, + testVPC, testAttachment, + ) + args := &skel.CmdArgs{ContainerID: testContainerID, StdinData: []byte(conf)} + if err := cmdCheck(args); err == nil { + t.Fatal("expected error for missing VRF/route state") + } +} + +func TestCmdCheckNoTerminationsStillRequiresVRF(t *testing.T) { + // checkTerminationRoutes resolves the VRF table ID up front, before + // its per-termination loop — with zero terminations to check, an + // absent VRF still surfaces as an error rather than being skipped, the + // same order the original code (internal/cni/ops_check.go, before this + // split) already used. + args := &skel.CmdArgs{ + ContainerID: testContainerID, + StdinData: []byte(confJSON(testVPC, testAttachment, "")), + } + if err := cmdCheck(args); err == nil { + t.Fatal("expected error: VRF for this vpc/vpcAttachment does not exist on the test host") + } +} + +// ---- checkTerminationRoutes ------------------------------------------------- + +func TestCheckTerminationRoutesInvalidGateway(t *testing.T) { + err := checkTerminationRoutes(testVPC, testAttachment, []Termination{{Network: "fd00:2::/64", Via: "not-an-ip"}}) + if err == nil { + t.Fatal("expected error for invalid gateway") + } +} + +func TestCheckTerminationRoutesRequiresVRFEvenWithNone(t *testing.T) { + // vrf.TableID resolves before the (empty) per-termination loop runs, so + // a missing VRF still surfaces as an error here too. + if err := checkTerminationRoutes(testVPC, testAttachment, nil); err == nil { + t.Fatal("expected error: VRF for this vpc/vpcAttachment does not exist on the test host") + } +} + +// ---- cmdStatus -------------------------------------------------------------- + +func TestCmdStatusInvalidConfig(t *testing.T) { + args := &skel.CmdArgs{StdinData: []byte("not valid json")} + err := cmdStatus(args) + assertCNIError(t, err, 7, errInvalidCNIConfig) +} + +func TestCmdStatusReady(t *testing.T) { + args := &skel.CmdArgs{StdinData: []byte(`{"cniVersion":"1.0.0","type":"galactic-route"}`)} + if err := cmdStatus(args); err != nil { + t.Errorf("unexpected error: %v", err) + } +} + +// ---- resourceTracker -------------------------------------------------------- + +func TestResourceTrackerCleanupZeroValue(t *testing.T) { + tracker := &resourceTracker{} + tracker.cleanup() // should not panic; empty added slice, nothing to delete +} + +func TestResourceTrackerCleanupUnaddedRoute(t *testing.T) { + // A route that was never actually added (e.g. route.Add failed before + // tracker.added recorded it) should never be attempted here — this + // tracker only ever unwinds what it itself recorded as added. + tracker := &resourceTracker{vpc: testVPC, vpcAttachment: testAttachment, dev: testIfName} + tracker.cleanup() // no entries in added; should not panic or attempt anything +} diff --git a/internal/cniroute/config.go b/internal/cniroute/config.go new file mode 100644 index 00000000..86700e37 --- /dev/null +++ b/internal/cniroute/config.go @@ -0,0 +1,237 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cniroute + +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-route/main.go). +// +// galactic-route has no k8s dependency, so — unlike every other binary in +// the chain — it never resolves NodeName or Kubeconfig. It uses +// config.CNIConfig purely for LogFile/LogLevel's env-var > conflist > +// default precedence, so logging behaves the same way here as everywhere +// else in the chain (see internal/cni/hostconf's doc comment on the one +// static conflist file every binary shares). +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). +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{}, nil + } + return nil, err + } + return conf, nil +} + +// unwrapPathError returns the innermost *os.PathError-shaped error wrapped +// by err, if any, so os.IsNotExist (which does not itself traverse %w +// wrapping) can still recognize a missing conflist file wrapped by +// hostconf.Load's fmt.Errorf("read conflist file %q: %w", ...). +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. Matching +// is case-insensitive. An empty string resolves to config.DefaultLogLevel. +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"` +} + +// parseStatusConf validates that the CNI config is parseable and contains +// the required top-level fields. galactic-route has no attachment-specific +// or API-server state to check — STATUS must succeed on a freshly started +// node before any ADD has run. +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 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{ + LogFile: hostConf.LogFile, + LogLevel: hostConf.LogLevel, + }) + 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 +} + +// validatePrevResult checks that the prevResult (from a preceding plugin in +// the CNI chain) is a valid, parseable CNI result. +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 +} + +// sanitizeForError returns s unchanged if it contains only printable ASCII +// characters; otherwise returns "" to avoid corrupting log output. +func sanitizeForError(s string) string { + for _, c := range s { + if c < 0x20 || c > 0x7e { + return sanitizeForErrorBinary + } + } + return s +} diff --git a/internal/cniroute/ops_add.go b/internal/cniroute/ops_add.go new file mode 100644 index 00000000..a9750ab9 --- /dev/null +++ b/internal/cniroute/ops_add.go @@ -0,0 +1,88 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cniroute + +import ( + "encoding/json" + "errors" + "fmt" + "log/slog" + + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/types" + type100 "github.com/containernetworking/cni/pkg/types/100" + + "go.datum.net/galactic/internal/cni/route" + "go.datum.net/galactic/internal/plumbing/intf" +) + +// cmdAdd installs each of pluginConf's termination routes into the VRF +// routing table the preceding master plugin (galactic-cni/galactic-tap-cni) +// already created, then passes prevResult through unchanged — galactic- +// route adds no interfaces or IPs of its own, only kernel routes alongside +// whatever came before it in the chain. +func cmdAdd(args *skel.CmdArgs) (err error) { + pluginConf, err := parseConf(args.StdinData) + if err != nil { + return err + } + + prevResult, prevErr := parsePrevResult(pluginConf.RawPrevResult) + if prevErr != nil { + return &types.Error{Code: 6, Msg: fmt.Sprintf("parse prevResult: %v", prevErr)} + } + + slog.Info("ADD: starting", "containerID", args.ContainerID, + "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment, + "terminations", len(pluginConf.Terminations)) + + // The host interface name is derived from (vpc, vpcAttachment) alone — + // identical for a veth master's host end and a tap master's tap device + // (both call intf.GenerateInterfaceNameHost), so galactic-route needs no + // interface-kind inference the way galactic-bgp does. + dev := intf.GenerateInterfaceNameHost(pluginConf.VPC, pluginConf.VPCAttachment) + + tracker := &resourceTracker{vpc: pluginConf.VPC, vpcAttachment: pluginConf.VPCAttachment, dev: dev} + 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() + } + }() + + for _, termination := range pluginConf.Terminations { + if err := route.Add(pluginConf.VPC, pluginConf.VPCAttachment, termination.Network, termination.Via, dev); err != nil { + return fmt.Errorf("add route %s: %w", termination.Network, err) + } + tracker.added = append(tracker.added, termination) + } + if len(tracker.added) > 0 { + slog.Debug("ADD: termination routes installed", "count", len(tracker.added), "dev", dev) + } + + return types.PrintResult(prevResult, pluginConf.CNIVersion) +} + +// parsePrevResult parses rawPrevResult (PluginConf.RawPrevResult; the typed +// PluginConf.PrevResult field is never populated by plain JSON unmarshal, +// per its "json:\"-\"" tag) into a versioned CNI result galactic-route can +// pass straight back through as its own ADD result. galactic-route is +// optional in the chain, but when present it must be chained after a +// master plugin, which always produces a prevResult. +func parsePrevResult(rawPrevResult map[string]interface{}) (types.Result, error) { + if rawPrevResult == nil { + return nil, errors.New("no prevResult: galactic-route must be chained after a master plugin") + } + jsonBytes, err := json.Marshal(rawPrevResult) + if err != nil { + return nil, fmt.Errorf("marshal prevResult: %w", err) + } + parsed, err := type100.NewResult(jsonBytes) + if err != nil { + return nil, fmt.Errorf("parse prevResult: %w", err) + } + return parsed, nil +} diff --git a/internal/cniroute/ops_check.go b/internal/cniroute/ops_check.go new file mode 100644 index 00000000..01ffdf27 --- /dev/null +++ b/internal/cniroute/ops_check.go @@ -0,0 +1,105 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cniroute + +import ( + "fmt" + "log/slog" + "net" + + "github.com/containernetworking/cni/pkg/skel" + "github.com/vishvananda/netlink" + + "go.datum.net/galactic/internal/plumbing/intf" + "go.datum.net/galactic/internal/plumbing/vrf" +) + +// cmdCheck verifies that the termination routes cmdAdd installed are still +// present in the VRF routing table. Per the plan's CHECK/STATUS +// distribution, this is galactic-route's entire CHECK story — moved +// unchanged from internal/cni/ops_check.go (also mirrored in +// internal/cnitap), since the underlying kernel state it verifies didn't +// change shape by moving which process installs it. +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) + + if err := checkTerminationRoutes(pluginConf.VPC, pluginConf.VPCAttachment, pluginConf.Terminations); err != nil { + err = fmt.Errorf("CHECK failed: termination routes: %w", err) + 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 +} + +// cmdStatus implements the CNI spec STATUS operation. galactic-route has no +// API server or attachment-specific state to probe — it either parses a +// well-formed config or it doesn't, matching galactic-ipam's own trivial +// STATUS (see the plan's CHECK/STATUS distribution: implemented for +// uniformity across the chain, not skipped). +func cmdStatus(args *skel.CmdArgs) error { + if err := parseStatusConf(args.StdinData); err != nil { + return err + } + slog.Info("STATUS: ready") + return nil +} + +// checkTerminationRoutes verifies that all termination routes exist in the +// VRF table for the given VPC/VPCAttachment pair. +func checkTerminationRoutes(vpc, vpcAttachment string, terminations []Termination) error { + tableID, err := vrf.TableID(vpc, vpcAttachment) + if err != nil { + return fmt.Errorf("get VRF table ID: %w", err) + } + + handle, err := netlink.NewHandle() + if err != nil { + return fmt.Errorf("create netlink handle: %w", err) + } + defer handle.Close() //nolint:errcheck // netlink cleanup on teardown + + routes, err := handle.RouteListFiltered( + netlink.FAMILY_V6, + &netlink.Route{Table: int(tableID)}, + netlink.RT_FILTER_TABLE, + ) + if err != nil { + return fmt.Errorf("list routes: %w", err) + } + + dev := intf.GenerateInterfaceNameHost(vpc, vpcAttachment) + for _, term := range terminations { + viaIP := net.ParseIP(term.Via) + if viaIP == nil { + return fmt.Errorf("invalid termination gateway %q", term.Via) + } + found := false + for _, r := range routes { + if r.Dst != nil && + r.Dst.String() == term.Network && + r.Gw != nil && + r.Gw.Equal(viaIP) && + r.LinkIndex > 0 { + // Verify the link name matches (defers to the veth/tap device). + if link, linkErr := handle.LinkByIndex(r.LinkIndex); linkErr == nil && link.Attrs().Name == dev { + found = true + break + } + } + } + if !found { + return fmt.Errorf("missing route %s via %s in VRF table %d", term.Network, term.Via, tableID) + } + } + return nil +} diff --git a/internal/cniroute/ops_del.go b/internal/cniroute/ops_del.go new file mode 100644 index 00000000..9bed975c --- /dev/null +++ b/internal/cniroute/ops_del.go @@ -0,0 +1,30 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cniroute + +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 +// termination routes this plugin's own ADD installed 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. This also matches the pre-split behavior: the old +// monolithic plugin's own DEL never deleted termination routes either, +// for the same reason. +func cmdDel(args *skel.CmdArgs) error { + slog.Info("DEL: skipping shared resource cleanup (handled by GC)", "containerID", args.ContainerID) + + result := &type100.Result{} + _ = types.PrintResult(result, "1.0.0") + return nil +} diff --git a/internal/cniroute/resource.go b/internal/cniroute/resource.go new file mode 100644 index 00000000..f3fa7781 --- /dev/null +++ b/internal/cniroute/resource.go @@ -0,0 +1,37 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cniroute + +import ( + "log/slog" + + "go.datum.net/galactic/internal/cni/route" +) + +// resourceTracker tracks routes created during cmdAdd for selective +// rollback. galactic-route's own ADD only ever creates termination +// routes — route-delete only, per the plan's decision that each binary's +// tracker unwinds exactly what its own ADD created (the VRF/veth-or-tap +// it runs alongside belongs to the master plugin's own tracker instead). +type resourceTracker struct { + vpc, vpcAttachment, dev string + added []Termination +} + +// cleanup deletes every route this tracker recorded as added. Errors are +// logged but never returned — the caller already has a failure. +func (rt *resourceTracker) cleanup() { + slog.Info("Selective rollback: cleaning up routes created during failed ADD", + "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) + + for _, term := range rt.added { + if err := route.Delete(rt.vpc, rt.vpcAttachment, term.Network, term.Via, rt.dev); err != nil { + slog.Error("Rollback: failed to delete route", "err", err, + "network", term.Network, "via", term.Via, "dev", rt.dev) + } else { + slog.Debug("Rollback: deleted route", "network", term.Network, "via", term.Via, "dev", rt.dev) + } + } +} diff --git a/internal/cniroute/types.go b/internal/cniroute/types.go new file mode 100644 index 00000000..03ce538f --- /dev/null +++ b/internal/cniroute/types.go @@ -0,0 +1,45 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +// Package cniroute implements galactic-route, the termination-route plugin +// in the galactic CNI chain — chained after galactic-cni/galactic-tap-cni +// and before galactic-bgp per conflist order (optional: only present when +// an attachment has terminations to install). It installs kernel routes +// into the VRF routing table the preceding master plugin already created, +// then passes prevResult through unchanged, adding no interfaces or IPs of +// its own. +// +// Unlike every other binary in the chain, galactic-route has no Kubernetes +// dependency at all: it neither reads nor writes any CRD, and never needs +// a namespace. +package cniroute + +import ( + "github.com/containernetworking/cni/pkg/types" + + "go.datum.net/galactic/internal/cni/hostconf" +) + +// Termination represents a network termination point with a destination +// CIDR and next-hop gateway address. +type Termination struct { + Network string `json:"network"` + Via string `json:"via,omitempty"` +} + +// PluginConf is the CNI plugin configuration passed via stdin on each +// invocation of galactic-route — the same document the master plugin +// itself received, since the CNI runtime passes each chain entry its own +// stanza plus prevResult. galactic-route only reads vpc/vpcattachment/ +// terminations out of it (mtu, ipam, namespace are the master's/ +// galactic-ipam's/galactic-bgp's own concerns). +type PluginConf struct { + types.PluginConf + VPC string `json:"vpc"` + VPCAttachment string `json:"vpcattachment"` + Terminations []Termination `json:"terminations,omitempty"` +} + +// HostConf holds node-local settings read from /etc/cni/net.d/10-galactic.conflist. +type HostConf = hostconf.HostConf diff --git a/internal/cnitap/ops_add.go b/internal/cnitap/ops_add.go index fe442a4b..7a0fadf1 100644 --- a/internal/cnitap/ops_add.go +++ b/internal/cnitap/ops_add.go @@ -17,7 +17,6 @@ import ( "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/cniipam" "go.datum.net/galactic/internal/plumbing/intf" @@ -103,16 +102,8 @@ func cmdAdd(args *skel.CmdArgs) (err error) { return fmt.Errorf("annotate NAD: %w", err) } - dev := hostName - for _, termination := range pluginConf.Terminations { - if err := route.Add(pluginConf.VPC, pluginConf.VPCAttachment, termination.Network, termination.Via, dev); err != nil { - return fmt.Errorf("add route %s: %w", termination.Network, err) - } - tracker.routesCreated++ - } - if tracker.routesCreated > 0 { - slog.Debug("ADD: termination routes installed", "count", tracker.routesCreated, "dev", dev) - } + // Termination routes are galactic-route's job now — chained next after + // this plugin, when the attachment has any (see internal/cniroute). // Allocate IPAM for the tap interface via delegation (only if pluginConf // carries an "ipam" block at all — see internal/cniipam's doc comment diff --git a/internal/cnitap/ops_check.go b/internal/cnitap/ops_check.go index df5a874a..c8413db6 100644 --- a/internal/cnitap/ops_check.go +++ b/internal/cnitap/ops_check.go @@ -10,7 +10,6 @@ import ( "errors" "fmt" "log/slog" - "net" "net/http" "os" "time" @@ -45,9 +44,9 @@ func cmdCheck(args *skel.CmdArgs) error { hostName, nodeErrs := checkNodeLevelState(pluginConf.VPC, pluginConf.VPCAttachment) errs = append(errs, nodeErrs...) - if err := checkTerminationRoutes(pluginConf.VPC, pluginConf.VPCAttachment, pluginConf.Terminations); err != nil { - errs = append(errs, fmt.Errorf("termination routes: %w", err)) - } + // Termination routes are galactic-route's own CHECK now (see + // internal/cniroute's checkTerminationRoutes) — this plugin's CHECK no + // longer verifies them. if pluginConf.RawPrevResult != nil { if err := checkPrevResult(pluginConf.RawPrevResult, hostName); err != nil { @@ -157,55 +156,6 @@ func checkNodeLevelState(vpc, vpcAttachment string) (string, []error) { return hostName, errs } -// checkTerminationRoutes verifies that all termination routes exist in the -// VRF table for the given VPC/VPCAttachment pair. -func checkTerminationRoutes(vpc, vpcAttachment string, terminations []Termination) error { - tableID, err := vrf.TableID(vpc, vpcAttachment) - if err != nil { - return fmt.Errorf("get VRF table ID: %w", err) - } - - handle, err := netlink.NewHandle() - if err != nil { - return fmt.Errorf("create netlink handle: %w", err) - } - defer handle.Close() //nolint:errcheck // netlink cleanup on teardown - - routes, err := handle.RouteListFiltered( - netlink.FAMILY_V6, - &netlink.Route{Table: int(tableID)}, - netlink.RT_FILTER_TABLE, - ) - if err != nil { - return fmt.Errorf("list routes: %w", err) - } - - dev := intf.GenerateInterfaceNameHost(vpc, vpcAttachment) - for _, term := range terminations { - viaIP := net.ParseIP(term.Via) - if viaIP == nil { - return fmt.Errorf("invalid termination gateway %q", term.Via) - } - found := false - for _, r := range routes { - if r.Dst != nil && - r.Dst.String() == term.Network && - r.Gw != nil && - r.Gw.Equal(viaIP) && - r.LinkIndex > 0 { - if link, linkErr := handle.LinkByIndex(r.LinkIndex); linkErr == nil && link.Attrs().Name == dev { - found = true - break - } - } - } - if !found { - return fmt.Errorf("missing route %s via %s in VRF table %d", term.Network, term.Via, tableID) - } - } - return nil -} - // checkPrevResult validates that kernel state matches the host interface // recorded in the prevResult returned by the most recent ADD. Tap mode has // no guest-side interface or netns to validate against. diff --git a/internal/cnitap/resource.go b/internal/cnitap/resource.go index 87742425..fe9426bf 100644 --- a/internal/cnitap/resource.go +++ b/internal/cnitap/resource.go @@ -49,11 +49,11 @@ func newK8sClient() (client.Client, error) { // 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). +// smaller tracker (internal/cnibgp); termination routes are galactic- +// route's own, with its own smaller tracker (internal/cniroute). type resourceTracker struct { vpc, vpcAttachment string vrfCreated bool - routesCreated int // ipamDelegated, ipamType, and ipamStdin record enough to release the // IPAM allocation during rollback — see internal/cni's own diff --git a/internal/cnitap/types.go b/internal/cnitap/types.go index 189a7c8a..5cffc104 100644 --- a/internal/cnitap/types.go +++ b/internal/cnitap/types.go @@ -16,25 +16,19 @@ import ( "go.datum.net/galactic/internal/cniipam" ) -// Termination represents a network termination point with a destination -// CIDR and next-hop gateway address. -type Termination struct { - Network string `json:"network"` - Via string `json:"via,omitempty"` -} - // PluginConf is the CNI plugin configuration passed via stdin on each // invocation of galactic-tap-cni. // // IPAM addressing fields live entirely inside the "ipam" block — see // go.datum.net/galactic/internal/cniipam's doc comment for the explicit -// delegation contract. +// delegation contract. Termination routes are galactic-route's own +// concern now (see internal/cniroute) — this plugin's own JSON stanza +// carries no "terminations" field of its own to read. type PluginConf struct { types.PluginConf VPC string `json:"vpc"` VPCAttachment string `json:"vpcattachment"` MTU int `json:"mtu,omitempty"` - Terminations []Termination `json:"terminations,omitempty"` IPAM *cniipam.IPAM `json:"ipam"` Namespace string `json:"namespace,omitempty"` } diff --git a/internal/installer/installer.go b/internal/installer/installer.go index 3cc4c868..2807e600 100644 --- a/internal/installer/installer.go +++ b/internal/installer/installer.go @@ -69,6 +69,7 @@ var ( SourceTapCNIBinary = "/galactic-tap-cni" SourceIPAMBinary = "/galactic-ipam" SourceBGPBinary = "/galactic-bgp" + SourceRouteBinary = "/galactic-route" SourceHostDeviceBinary = "/host-device" ) @@ -276,6 +277,9 @@ func Bootstrap(ctx context.Context, nodeName string) error { if err := atomicCopyFile(SourceBGPBinary, filepath.Join(HostBinDir, "galactic-bgp"), 0755); err != nil { return fmt.Errorf("copy galactic-bgp binary: %w", err) } + if err := atomicCopyFile(SourceRouteBinary, filepath.Join(HostBinDir, "galactic-route"), 0755); err != nil { + return fmt.Errorf("copy galactic-route 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 b21befc7..ca164bac 100644 --- a/internal/installer/installer_test.go +++ b/internal/installer/installer_test.go @@ -117,11 +117,13 @@ func TestBootstrap(t *testing.T) { SourceTapCNIBinary = filepath.Join(tmpDir, "source-galactic-tap-cni") SourceIPAMBinary = filepath.Join(tmpDir, "source-galactic-ipam") SourceBGPBinary = filepath.Join(tmpDir, "source-galactic-bgp") + SourceRouteBinary = filepath.Join(tmpDir, "source-galactic-route") SourceHostDeviceBinary = filepath.Join(tmpDir, "source-host-device") writeSourceBinary(t, SourceCNIBinary, "cni-content") writeSourceBinary(t, SourceTapCNIBinary, "tap-cni-content") writeSourceBinary(t, SourceIPAMBinary, "ipam-content") writeSourceBinary(t, SourceBGPBinary, "bgp-content") + writeSourceBinary(t, SourceRouteBinary, "route-content") writeSourceBinary(t, SourceHostDeviceBinary, "host-device-content") // Mock node object @@ -179,6 +181,7 @@ func TestBootstrap(t *testing.T) { 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") + assertBinaryCopied(t, filepath.Join(HostBinDir, "galactic-route"), "route-content") // Verify conflist written conflist, err := loadHostConf(HostConflist) From dddf8182f52a60fd561b48a49587c63c9822e8fd Mon Sep 17 00:00:00 2001 From: Peter Sprygada Date: Sun, 9 Aug 2026 16:29:55 -0400 Subject: [PATCH 2/2] fix(cniroute): address PR #306 review feedback from mattdjenkinson CNI_NETNS_OVERRIDE (blocking): galactic-route never entered any netns, so cmd/galactic-route/main.go assumed it never needed the stdin peek-and-repipe dance or CNI_NETNS_OVERRIDE that galactic-cni/ galactic-tap-cni use. That's true for veth-mode attachments, where CNI_NETNS points at the container's netns and differs from this process's own ambient (host) netns. It's false for tap-mode attachments: CNI_NETNS is deliberately set to the host's own root netns there (no per-VM netns exists), which equals this process's ambient netns, so skel's post-Add/Del same-netns check rejected every tap-mode ADD/DEL with terminations even though the route was already installed correctly. Now peeks stdin for interface_type the same way galactic-cni does and sets CNI_NETNS_OVERRIDE=true only for tap mode. CHECK on-link routes: checkTerminationRoutes unconditionally called net.ParseIP on Via and errored on nil, but Via is omitempty and assembleRoute (route.go) has a real branch that installs a valid on-link route for an empty Via, which cmdAdd installs fine. CHECK always failed with "invalid termination gateway" for those regardless. Restructured the match loop to treat an empty Via as looking for a gateway-less, device-scoped route instead of erroring immediately. Carried over byte for byte from the pre-split internal/cni/ops_check.go (also reachable via internal/cnitap), so this fixes the same bug there too by virtue of the code having moved. Docs: docs/cni/configuration.md still listed terminations as a galactic-cni/galactic-tap-cni field and showed it inline in the master's own JSON, which this PR's PluginConf split made wrong -- the master's slimmer struct silently drops the field on unmarshal, so an operator following the doc gets a silent no-op. Moved the field out of the master's Top-Level Fields table, reworded Termination Fields to attribute it to galactic-route's own conflist stanza (including that cmdDel is a no-op, not "deleted in reverse order"), and rewrote the worked example as a chained plugins array. Deferred per the review: the internal/cniroute/config.go:205 dead `if conf.PrevResult != nil` branch is copy-pasted across internal/cni, internal/cnitap, and internal/cnibgp too, predating this PR -- left for #307, which already scopes "dead code" cleanup for the chain split. Verification: task lint, task build (all 8 binaries), task test:unit all pass. task test:e2e not run, same caveat as #303/#304/#305 -- this repo has no root/CAP_NET_ADMIN available, and the existing checkTerminationRoutes tests already can't get past the vrf.TableID lookup without a real kernel VRF, so the on-link CHECK fix has no new automated regression test beyond what task test:e2e's Kind cluster would exercise. Co-Authored-By: Claude Sonnet 5 --- cmd/galactic-route/main.go | 44 +++++++++++++++++++++--- docs/cni/configuration.md | 63 ++++++++++++++++++++++++---------- internal/cniroute/ops_check.go | 37 +++++++++++++------- 3 files changed, 108 insertions(+), 36 deletions(-) diff --git a/cmd/galactic-route/main.go b/cmd/galactic-route/main.go index d5a92784..cbd49068 100644 --- a/cmd/galactic-route/main.go +++ b/cmd/galactic-route/main.go @@ -5,7 +5,9 @@ package main import ( + "encoding/json" "fmt" + "io" "log" "os" "strings" @@ -69,11 +71,32 @@ func newRootCommand() *cobra.Command { return nil } - // Unlike galactic-cni/galactic-tap-cni, this plugin never enters - // any network namespace or talks to the API server, 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. + // Unlike galactic-cni/galactic-tap-cni, this plugin never talks + // to the API server. It does, however, run natively in whatever + // netns CNI_NETNS points at rather than entering it — for a + // veth-mode attachment CNI_NETNS is the container's netns, which + // differs from this process's own ambient (host) netns, so the + // CNI library's same-netns rejection check never fires. For a + // tap-mode attachment, though, CNI_NETNS is deliberately set to + // the host's own root netns (there's no per-VM netns to enter), + // which does equal this process's ambient netns — so the same + // peek-and-repipe dance and CNI_NETNS_OVERRIDE galactic-cni uses + // for tap mode are needed here too, or the library rejects every + // tap-mode ADD/DEL after the route is already installed. + stdinData, _ := io.ReadAll(os.Stdin) + r, w, _ := os.Pipe() + go func() { + _, _ = w.Write(stdinData) + _ = w.Close() + }() + oldStdin := os.Stdin + os.Stdin = r + defer func() { os.Stdin = oldStdin }() + + if isTapMode(stdinData) { + _ = os.Setenv("CNI_NETNS_OVERRIDE", "true") + } + cniroute.RunPlugin() return nil }, @@ -85,6 +108,17 @@ func newRootCommand() *cobra.Command { return cmd } +// isTapMode returns true when the CNI config requests tap interface type. +// Only a minimal JSON parse is needed — full validation happens later in +// parseConf inside cmdAdd/cmdDel. +func isTapMode(stdinData []byte) bool { + var cfg struct { + InterfaceType string `json:"interface_type"` + } + _ = json.Unmarshal(stdinData, &cfg) + return cfg.InterfaceType == "tap" +} + func main() { if err := newRootCommand().Execute(); err != nil { log.Fatalf("error: %v", err) diff --git a/docs/cni/configuration.md b/docs/cni/configuration.md index ebe34242..e29507aa 100644 --- a/docs/cni/configuration.md +++ b/docs/cni/configuration.md @@ -97,7 +97,6 @@ the standard CNI `PluginConf` with Galactic-specific fields. | `vpcattachment` | **Yes** | `string` | Base62-encoded VPC attachment identifier (16-bit value). Paired with `vpc` for deterministic VRF/BGP naming. | | `interface_type` | No | `string` | Interface mode: `"veth"` (default, for containers) or `"tap"` (for VMs such as Kata, Firecracker, QEMU). Both modes run IPAM and SRv6/BGP publish; `tap` mode only skips host-device delegation and guest-netns configuration (see the Tap mode section below). | | `mtu` | No | `int` | MTU for the host-side interface. For `veth` mode this applies to both veth endpoints; for `tap` mode it applies to the tap interface. | -| `terminations` | No | `[]Termination` | Array of static routes to add on the host side (see Termination sub-fields below). | | `namespace` | No | `string` | Kubernetes namespace used to look up the `BGPRouter` CRD. Resolution order: this field → `GALACTIC_CNI_NAMESPACE` env → `HostConf.Namespace` (conflist) → `galactic-system`. See [Runtime Configuration](#runtime-configuration) above. | | `ipam` | No* | `IPAM` | Legacy static-IP / local-IPAM configuration block (see IPAM sub-fields below). Only `type: "static"` still drives its own allocation path; `type: "pool"` is otherwise superseded by `ipv6_subnet`/`ipv4_subnet` below. *Required unless `GALACTIC_CNI_ENABLE_LOCAL_IPAM`, `ipv6_subnet`, or `ipv4_subnet` is set — applies identically in `veth` and `tap` mode. In `tap` mode `cmdAdd` (`internal/cni/ops_add.go`) calls `allocateIPAM` unconditionally (unlike `veth` mode, which checks first), so a config satisfying none of those currently produces a nil-pointer panic in `tap` mode rather than a clean validation error — always set one of them for tap. | | `ipv6_subnet` | No* | `string` | Region IPv6 pool CIDR for the NAD-driven pool-IPAM path; endpoints allocate a `/96` from it by default. Setting this field or `ipv4_subnet` (or both) opts a config into pool IPAM directly — no `ipam` block needed. See [Pool IPAM via `ipv6_subnet`/`ipv4_subnet`](#pool-ipam-via-ipv6_subnetipv4_subnet) below. | @@ -121,6 +120,13 @@ 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. +`terminations` (static routes to add on the host side) is **not** a +`galactic-cni`/`galactic-tap-cni` field — it belongs to `galactic-route`, the +chained plugin invoked after the master plugin per conflist order (see +[Termination Fields](#termination-fields) below). Putting a `terminations` +array in the master's own stanza does nothing: `galactic-cni`'s slimmer +`PluginConf` silently drops the unknown field on unmarshal. + ### Interface Types #### `veth` (default) @@ -201,15 +207,22 @@ this fallback is IPv6-only; there is no default IPv4 pool. ### Termination Fields -Each entry in the `terminations` array has the following fields: +`terminations` is a field of `galactic-route`'s own conflist stanza — the +chained CNI plugin invoked after `galactic-cni`/`galactic-tap-cni` and before +`galactic-bgp` per conflist order, present only for attachments that need +static routes installed on the host side. Each entry in the array has the +following fields: | Field | Required | Type | Description | | --------- | -------- | -------- | ------------------------------------------------------------------------------------------ | | `network` | **Yes** | `string` | CIDR prefix for a static route (e.g. `"fd00::/48"`). | | `via` | No | `string` | Next-hop gateway IP. If omitted, a link-local route is installed via the host-side device. | -Used in `cmdAdd` to install routes into the VRF table for each termination -entry. Deleted in `cmdDel` in reverse order. +`galactic-route`'s `cmdAdd` installs routes into the VRF table for each +termination entry. `cmdDel` is a no-op — routes are keyed by +`(vpc, vpcattachment)` and may still be in use by another pod/VM sharing the +same attachment, so cleanup is left entirely to `galactic-router`'s GC +controller. ## Example Configurations @@ -225,8 +238,10 @@ entry. Deleted in `cmdDel` in reverse order. } ``` -Omits `namespace` (defaults to `galactic-system`), `ipam`, and `terminations`. -Without `GALACTIC_CNI_ENABLE_LOCAL_IPAM` set, no IP address is assigned to the +Omits `namespace` (defaults to `galactic-system`) and `ipam`, and has no +`galactic-route` chain entry (see [Configuration with terminations](#configuration-with-terminations) +below) since there are no static routes to install. Without +`GALACTIC_CNI_ENABLE_LOCAL_IPAM` set, no IP address is assigned to the guest interface. With `GALACTIC_CNI_ENABLE_LOCAL_IPAM` set, a subnet is allocated from the built-in pool. @@ -304,26 +319,38 @@ carries only the IPv4 `/32` prefix. ### Configuration with terminations +`terminations` goes in `galactic-route`'s own stanza of the conflist's +`plugins` array, not the master plugin's: + ```json { "cniVersion": "1.0.0", "name": "galactic", - "type": "galactic-cni", - "vpc": "1", - "vpcattachment": "1", - "terminations": [ - { "network": "fd00::/48", "via": "fe80::1" }, - { "network": "fd01::/48" } - ], - "ipam": { - "type": "pool", - "pool": "fd00:1:ff01::/48" - } + "plugins": [ + { + "type": "galactic-cni", + "vpc": "1", + "vpcattachment": "1", + "ipam": { + "type": "pool", + "pool": "fd00:1:ff01::/48" + } + }, + { + "type": "galactic-route", + "vpc": "1", + "vpcattachment": "1", + "terminations": [ + { "network": "fd00::/48", "via": "fe80::1" }, + { "network": "fd01::/48" } + ] + } + ] } ``` The first termination installs a specific next-hop route; the second installs -a link-local route via the host-side device. +an on-link route via the host-side device. ### Tap interface configuration (VM workloads) diff --git a/internal/cniroute/ops_check.go b/internal/cniroute/ops_check.go index 01ffdf27..43edb7d8 100644 --- a/internal/cniroute/ops_check.go +++ b/internal/cniroute/ops_check.go @@ -79,26 +79,37 @@ func checkTerminationRoutes(vpc, vpcAttachment string, terminations []Terminatio dev := intf.GenerateInterfaceNameHost(vpc, vpcAttachment) for _, term := range terminations { - viaIP := net.ParseIP(term.Via) - if viaIP == nil { - return fmt.Errorf("invalid termination gateway %q", term.Via) + // An empty Via is not an error: assembleRoute (route.go) installs a + // valid on-link route for it, device-scoped with no gateway, and + // cmdAdd installs it fine. Only reject a non-empty Via that fails to + // parse. + var viaIP net.IP + if term.Via != "" { + viaIP = net.ParseIP(term.Via) + if viaIP == nil { + return fmt.Errorf("invalid termination gateway %q", term.Via) + } } found := false for _, r := range routes { - if r.Dst != nil && - r.Dst.String() == term.Network && - r.Gw != nil && - r.Gw.Equal(viaIP) && - r.LinkIndex > 0 { - // Verify the link name matches (defers to the veth/tap device). - if link, linkErr := handle.LinkByIndex(r.LinkIndex); linkErr == nil && link.Attrs().Name == dev { - found = true - break + if r.Dst == nil || r.Dst.String() != term.Network || r.LinkIndex <= 0 { + continue + } + if viaIP != nil { + if r.Gw == nil || !r.Gw.Equal(viaIP) { + continue } + } else if r.Gw != nil { + continue + } + // Verify the link name matches (defers to the veth/tap device). + if link, linkErr := handle.LinkByIndex(r.LinkIndex); linkErr == nil && link.Attrs().Name == dev { + found = true + break } } if !found { - return fmt.Errorf("missing route %s via %s in VRF table %d", term.Network, term.Via, tableID) + return fmt.Errorf("missing route %s via %q in VRF table %d", term.Network, term.Via, tableID) } } return nil