diff --git a/Taskfile.yaml b/Taskfile.yaml index 8f45d766..597f77f9 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -133,6 +133,7 @@ tasks: -X go.datum.net/galactic/internal/metadata.GitURL={{.GIT_URL}} cmds: - 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-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-tap-cni/main.go b/cmd/galactic-tap-cni/main.go new file mode 100644 index 00000000..1fa4a35c --- /dev/null +++ b/cmd/galactic-tap-cni/main.go @@ -0,0 +1,103 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package main + +import ( + "fmt" + "io" + "log" + "os" + "strings" + + "github.com/containernetworking/cni/pkg/version" + "github.com/spf13/cobra" + "golang.org/x/term" + + "go.datum.net/galactic/internal/cnitap" + "go.datum.net/galactic/internal/metadata" +) + +const ( + appName = "galactic-tap-cni" + + appDesc = `Galactic tap CNI Plugin + + The tap master plugin in the galactic CNI chain, for VM-based workloads + (Kata, Firecracker, kraftlet/Unikraft) attaching directly to a galactic VPC + network. Unrelated to vmtap-cni, which is chained after Cilium's own CNI + plugin for a different purpose entirely (see its own doc comment). + + 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 { + cnitap.InitCNIConfig() + confFile, _ := cmd.Flags().GetString("conf-file") + if confFile != "" { + cnitap.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 + } + + // Tap mode never enters a network namespace — all operations + // are host-side. Set the override so the CNI library skips its + // same-netns rejection check, which would otherwise reject + // kraftlet workloads that pass the host netns. + 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 }() + + _ = os.Setenv("CNI_NETNS_OVERRIDE", "true") + + cnitap.RunPlugin() + return nil + }, + } + + cmd.PersistentFlags().String("conf-file", cnitap.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 c949f093..ec09b1ab 100644 --- a/containers/galactic-cni/Dockerfile +++ b/containers/galactic-cni/Dockerfile @@ -48,6 +48,20 @@ RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build \ -X go.datum.net/galactic/internal/metadata.GitURL=${GIT_URL}" \ -o galactic-cni cmd/galactic-cni/main.go +# Build galactic-tap-cni, the tap master plugin in the galactic CNI chain +# (VM workloads: Kata, Firecracker, kraftlet/Unikraft). Ships in this same +# image/binary set since every plugin in the chain is staged onto the host +# by the same galactic-cni init container (installer.Bootstrap). +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-tap-cni cmd/galactic-tap-cni/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 @@ -82,6 +96,7 @@ FROM gcr.io/distroless/static:nonroot AS production # Copy binaries from builder into the distroless production image COPY --from=builder /workspace/galactic-cni /galactic-cni +COPY --from=builder /workspace/galactic-tap-cni /galactic-tap-cni 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 @@ -94,6 +109,7 @@ FROM docker.io/library/alpine:latest # Copy binaries from the production (distroless) image COPY --from=production /galactic-cni /galactic-cni +COPY --from=production /galactic-tap-cni /galactic-tap-cni 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/bgp.go b/internal/cni/bgp.go deleted file mode 100644 index 1e43af95..00000000 --- a/internal/cni/bgp.go +++ /dev/null @@ -1,890 +0,0 @@ -// Copyright 2025 Datum Cloud, Inc. -// -// SPDX-License-Identifier: AGPL-3.0-or-later - -package cni - -import ( - "context" - "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" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - - "go.datum.net/galactic/internal/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" -) - -// 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 - -// isTransientError reports whether err is a transient failure that may -// resolve itself on retry (API server unavailable, timeout, network blip). -// Returns false for validation errors, not-found, and other permanent -// failures that should not be retried. -func isTransientError(err error) bool { - if err == nil { - return false - } - // Context-level failures (deadline exceeded, cancelled) are transient - // because they usually indicate the API server was slow/unavailable. - if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) { - return true - } - // Unwrap to handle wrapped errors (e.g. from controllerutil.CreateOrUpdate). - unwrapped := errors.Unwrap(err) - if unwrapped != nil { - if errors.Is(unwrapped, context.DeadlineExceeded) || errors.Is(unwrapped, context.Canceled) { - return true - } - } - // Kubernetes API errors: 503 Service Unavailable, 500 Internal Server - // Error, 504 Server Timeout, and 429 Too Many Requests. - if apierrors.IsServiceUnavailable(err) || - apierrors.IsInternalError(err) || - apierrors.IsServerTimeout(err) || - apierrors.IsTooManyRequests(err) { - return true - } - // Network-level transient errors (connection refused/reset, unreachable). - if netErr, ok := unwrapped.(interface{ Temporary() bool }); ok && netErr.Temporary() { - return true - } - return false -} - -// retryK8sOps runs fn with up to maxRetries+1 attempts, retrying on transient -// k8s API errors with exponential backoff. The context passed to fn has a -// timeout derived from timeout (respecting the original ctx deadline when set). -// Non-transient errors are returned immediately without retry. -func retryK8sOps(timeout time.Duration, fn func(ctx context.Context) error) error { - var lastErr error - for attempt := 0; attempt <= maxRetries; attempt++ { - if attempt > 0 { - backoff := time.Duration(1< create -> checkArgumentCollision runs -// sequentially within one call, so if A's create happens before B's create, -// then either B's own check (which always runs after B's own create) sees -// A's already-committed CRD and B reports a collision, or it doesn't only if -// A's check already ran (and hence already reported the collision itself) -// before B created its CRD. At least one side always detects it this way; a -// tie-breaker that lets exactly one side "win" without that guarantee (as a -// prior version of this function did) can let both sides pass when the two -// creates and checks interleave, leaving two BGPVRFInstances -- and, -// consequently, two vrf_table registrations -- permanently sharing the same -// VRFID. Both sides erroring out is harmless: the caller's non-transient -// error triggers the failed-ADD rollback (resourceTracker.cleanup), which -// deletes each side's own BGPVRFInstance, and the CNI runtime retries ADD. -func checkArgumentCollision( - ctx context.Context, k8s client.Client, namespace, routerName, vrfName string, vrfID int32, -) error { - list := &bgpv1alpha1.BGPVRFInstanceList{} - if err := k8s.List(ctx, list, client.InNamespace(namespace)); err != nil { - return fmt.Errorf("list BGPVRFInstances to verify argument uniqueness: %w", err) - } - for _, inst := range list.Items { - if inst.Spec.RouterRef == nil || inst.Spec.RouterRef.Name != routerName { - continue - } - if inst.Name != vrfName && inst.Spec.VRFID == vrfID { - return fmt.Errorf("argument collision: VRFID %d claimed by both %s and %s, retrying", vrfID, inst.Name, vrfName) - } - } - return nil -} - -// lookupBGPRouter finds the BGPRouter targeting this node in the given namespace. -// Returns an error if none is found or if multiple are found (ambiguous). -func lookupBGPRouter(ctx context.Context, k8s client.Client, nodeName, namespace string) (bgpConfig, error) { - routerList := &bgpv1alpha1.BGPRouterList{} - if err := k8s.List(ctx, routerList, client.InNamespace(namespace)); err != nil { - return bgpConfig{}, fmt.Errorf("list BGPRouters in namespace %s: %w", namespace, err) - } - - var matches []bgpv1alpha1.BGPRouter - for _, r := range routerList.Items { - if r.Spec.TargetRef.Name == nodeName { - matches = append(matches, r) - } - } - - switch len(matches) { - case 0: - return bgpConfig{}, fmt.Errorf("no BGPRouter found for node %s in namespace %s", nodeName, namespace) - case 1: - // expected - default: - return bgpConfig{}, fmt.Errorf("ambiguous BGP config: %d BGPRouters target node %s in namespace %s", - len(matches), nodeName, namespace) - } - - slog.Debug("BGP: router matched", "nodeName", nodeName, "router", matches[0].Name, - "asNumber", matches[0].Spec.LocalASN, "srv6Locator", matches[0].Spec.SRv6Locator, "nodeID", matches[0].Spec.NodeID) - - return bgpConfig{ - asNumber: uint32(matches[0].Spec.LocalASN), - routerName: matches[0].Name, - srv6Locator: matches[0].Spec.SRv6Locator, - nodeID: matches[0].Spec.NodeID, - }, nil -} - -// buildVRFInstanceSpec constructs the BGPVRFInstanceSpec for a VPC attachment. -// The route distinguisher is no longer stored on the CRD; it's derived -// downstream from the router's ID and vrfID. -func buildVRFInstanceSpec(routerName, rtValue string, vrfID int32) bgpv1alpha1.BGPVRFInstanceSpec { - return bgpv1alpha1.BGPVRFInstanceSpec{ - RouterTarget: bgpv1alpha1.RouterTarget{ - RouterRef: &bgpv1alpha1.RouterRef{Name: routerName}, - }, - VRFID: vrfID, - ImportRouteTargets: []bgpv1alpha1.RouteTarget{{Value: rtValue}}, - ExportRouteTargets: []bgpv1alpha1.RouteTarget{{Value: rtValue}}, - } -} - -// buildAdvertisementSpec constructs the BGPAdvertisementSpec for a VPC -// attachment's pod subnet(s) — one IPv6 prefix, plus an IPv4 prefix when the -// attachment is dual-stack. RFC 9136's Type-5 route is self-describing per -// NLRI, so a single BGPAdvertisement carrying both families is valid; see -// galactic-router's buildEVPNPaths for the corresponding per-family gateway -// handling. VRFID and Function record structurally what used to live in the -// legacy galactic.datum.net/srv6-sid annotation: which VRF this advertisement -// belongs to, and which SRv6 endpoint behavior the eBPF uSID datapath -// resolves (always End.DT46, regardless of pod-subnet address family — see -// registerEBPFDatapath). -func buildAdvertisementSpec( - routerName, rtValue string, prefixes []string, vrfID int32, -) bgpv1alpha1.BGPAdvertisementSpec { - function := bgpv1alpha1.SRv6FunctionEndDT46 - bgpPrefixes := make([]bgpv1alpha1.Prefix, len(prefixes)) - for i, p := range prefixes { - bgpPrefixes[i] = bgpv1alpha1.Prefix(p) - } - return bgpv1alpha1.BGPAdvertisementSpec{ - RouterRef: bgpv1alpha1.RouterRef{Name: routerName}, - AddressFamily: bgpv1alpha1.AddressFamily{AFI: bgpv1alpha1.AFIL2VPN, SAFI: bgpv1alpha1.SAFIEVPN}, - Prefixes: bgpPrefixes, - Communities: []bgpv1alpha1.Community{bgpv1alpha1.Community(rtValue)}, - VRFID: &vrfID, - Function: &function, - } -} - -// newK8sClient creates a new Kubernetes client using the in-cluster config. -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 -} - -// 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). -// -// K8s API operations are retried with exponential backoff on transient errors -// (503, timeout, network blip). Non-k8s operations (kernel networking) run -// once before the retry loop. Non-transient errors (validation, not-found) -// fail immediately without retry. -func publishBGPState( - args *skel.CmdArgs, pluginConf *PluginConf, nodeName, namespace string, ipamResult *ipamResult, - guestHWAddr net.HardwareAddr, tracker *resourceTracker, -) error { - // ---- non-k8s operations (run once) ---- - if err := configureHostGateway(pluginConf.VPC, pluginConf.VPCAttachment, ipamResult, guestHWAddr); err != nil { - return 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") - } - - // ---- k8s operations (retry on transient errors) ---- - // The SID Argument is allocated inside publishBGPStateK8s's retry - // closure, not here: it depends on this node's BGPRouter (looked up - // there) and must itself be a k8s-retried operation, since it lists - // BGPVRFInstance CRDs. - return publishBGPStateK8s(args, pluginConf, nodeName, namespace, ipamResult, vpcHex, tracker.k8s, tracker) -} - -// 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. Either family alone yields a single-entry prefixes -// slice; ipv6Subnet/ipv4Addr are empty when that family wasn't allocated. -func ipamAdvertisementPrefixes(ipamResult *ipamResult) (prefixes []string, ipv6Subnet, ipv4Addr string) { - if ipamResult == nil { - return nil, "", "" - } - if ipamResult.ipv6Subnet != nil { - ipv6Subnet = ipamResult.ipv6Subnet.String() - prefixes = append(prefixes, ipv6Subnet) - } - if ipamResult.ipv4Address != nil { - // The annotation stores the bare address (matching - // IPv4PoolAllocator's marker-file naming, so cmdDel's Deallocate - // call finds it — see ipam_ops.go); the advertised prefix needs the - // explicit /32 CIDR form. - ipv4Addr = ipamResult.ipv4Address.String() - prefixes = append(prefixes, ipv4Addr+"/32") - } - return prefixes, ipv6Subnet, ipv4Addr -} - -// allAdvertisedPrefixes derives the full set of BGP-advertised prefixes for -// a BGPAdvertisement CRD from every subnet annotation currently present on -// it, rather than from just the container currently being processed. -// -// A single BGPAdvertisement is keyed by (vpc, vpcAttachment) alone -// (bgpAdvertisementName), so multiple containers attaching under the same -// VPCAttachment on this node — a second pod, or a second interface with its -// own vpcattachment reusing this one — all share one CRD. Each one's own -// CNI ADD must not clobber another still-live container's already-published -// prefix: annotations is the durable per-container record (subnetAnnotationKeyIPv6/IPv4), -// so recomputing Spec.Prefixes from all of them on every ADD keeps every -// live container's prefix present regardless of ADD order. cmdDel -// deliberately leaves this annotation (and thus this prefix) in place even -// after that container exits — see ops_del.go's "skipping shared resource -// cleanup (handled by GC)" — so a stale entry for an exited container can -// briefly outlive it until gc.CollectOrphanedCRDs removes the whole CRD -// once every container sharing it is gone; that's a pre-existing tradeoff -// this function doesn't change. -func allAdvertisedPrefixes(annotations map[string]string) []string { - var prefixes []string - for key, value := range annotations { - switch { - case strings.HasPrefix(key, annotationAllocatedSubnetIPv6+"."): - prefixes = append(prefixes, value) - case strings.HasPrefix(key, annotationAllocatedSubnetIPv4+"."): - // Annotation stores the bare address (see ipamAdvertisementPrefixes); - // the advertised prefix needs the explicit /32 CIDR form. - prefixes = append(prefixes, value+"/32") - } - } - // Deterministic ordering: map iteration is randomized, and an - // unstable Spec.Prefixes order across otherwise-identical ADDs would - // look like a spurious spec change to anything diffing this CRD. - sort.Strings(prefixes) - return prefixes -} - -// publishBGPStateK8s creates the BGPVRFInstance and BGPAdvertisement CRDs with -// retry on transient k8s API errors. The host gateway must be configured before -// calling this (via configureHostGateway). This is interface-agnostic and can be -// used by both veth and tap code paths. -func publishBGPStateK8s( - args *skel.CmdArgs, pluginConf *PluginConf, nodeName, namespace string, ipamResult *ipamResult, - vpcHex string, k8s client.Client, tracker *resourceTracker, -) error { - return retryK8sOps(cniTimeout, func(ctx context.Context) error { - bgp, err := lookupBGPRouter(ctx, k8s, nodeName, namespace) - if err != nil { - return err - } - - vrfID, err := allocateArgument( - ctx, k8s, namespace, bgp.routerName, bgpVRFInstanceName(pluginConf.VPC, pluginConf.VPCAttachment)) - if err != nil { - return err - } - - rtValue, err := routeTarget(int64(bgp.asNumber), vpcHex) - if err != nil { - return fmt.Errorf("compute route target: %w", err) - } - - // Create the BGPVRFInstance to configure the VRF with its VRFID and - // import/export route targets. This must be created before advertisements - // so the BGP runtime has the VRF context when originating EVPN paths. - vrfName := bgpVRFInstanceName(pluginConf.VPC, pluginConf.VPCAttachment) - vrfInst := &bgpv1alpha1.BGPVRFInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: vrfName, - Namespace: namespace, - }, - } - _, err = controllerutil.CreateOrUpdate(ctx, k8s, vrfInst, func() error { - vrfInst.Spec = buildVRFInstanceSpec(bgp.routerName, rtValue, vrfID) - return nil - }) - if err != nil { - return fmt.Errorf("apply BGPVRFInstance: %w", err) - } - tracker.vrfInstanceCreated = true - slog.Debug("BGP: BGPVRFInstance applied", "name", vrfName, "namespace", namespace, - "vrfID", vrfID, "routeTarget", rtValue, "router", bgp.routerName) - - if err := checkArgumentCollision(ctx, k8s, namespace, bgp.routerName, vrfName, vrfID); err != nil { - return err - } - - // eBPF uSID datapath registration -- the only forwarding path - // (the legacy seg6local static-route path was removed once this - // datapath covered both veth and tap attachments). registered is - // false, with no error, only when the router has no - // srv6Locator/nodeID configured at all -- SRv6 is intentionally - // not set up for this attachment. Any other failure is fatal: - // with no legacy path to fall back to, an attachment with no - // registered datapath entry has no forwarding path at all. - // registerEBPFDatapath is itself idempotent (Register - // overwrites), so re-running it on a k8s-op retry is safe. - registered, ebpfBlock, err := registerEBPFDatapath( - bgp, pluginConf.VPC, pluginConf.VPCAttachment, pluginConf.InterfaceType, uint16(vrfID), attach.PinDir) - if err != nil { - return fmt.Errorf("register eBPF uSID datapath: %w", err) - } - if registered { - // Recorded so a failed-ADD rollback (resourceTracker.cleanup) - // can unregister this exact (block, argument) pair -- see - // Milestone 7.2. - tracker.ebpfRegistered = true - tracker.ebpfBlock = ebpfBlock - tracker.ebpfArgument = uint16(vrfID) - } - - // Create the BGPAdvertisement to originate the pod's subnet prefix(es). - adv := &bgpv1alpha1.BGPAdvertisement{ - ObjectMeta: metav1.ObjectMeta{ - Name: bgpAdvertisementName(pluginConf.VPC, pluginConf.VPCAttachment), - Namespace: namespace, - }, - } - prefixes, ipv6Subnet, ipv4Addr := ipamAdvertisementPrefixes(ipamResult) - var mergedPrefixes []string - _, err = controllerutil.CreateOrUpdate(ctx, k8s, adv, func() error { - if adv.Annotations == nil { - adv.Annotations = make(map[string]string) - } - // Record the netns path this container attached with, so the GC - // controller can check whether it still exists rather than - // guessing a name from the container ID (see - // gc.ContainerNetNSExistsByPath). - adv.Annotations[netnsAnnotationKey(args.ContainerID)] = args.Netns - // Store the allocated addresses keyed by container ID so cmdDel can - // look them up, one annotation per family so DEL can deallocate - // each independently. - if ipv6Subnet != "" { - adv.Annotations[subnetAnnotationKeyIPv6(args.ContainerID)] = ipv6Subnet - } - if ipv4Addr != "" { - adv.Annotations[subnetAnnotationKeyIPv4(args.ContainerID)] = ipv4Addr - } - // Recompute Spec.Prefixes from every container's annotations, not - // just this one's own — see allAdvertisedPrefixes. Must run after - // this container's own annotations are set above, and be read - // back into mergedPrefixes for the log line below since this - // closure may run more than once (RetryOnConflict). - mergedPrefixes = allAdvertisedPrefixes(adv.Annotations) - adv.Spec = buildAdvertisementSpec(bgp.routerName, rtValue, mergedPrefixes, vrfID) - return nil - }) - if err != nil { - return fmt.Errorf("apply BGPAdvertisement: %w", err) - } - tracker.advCreated = 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", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment) - 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 -} - -// 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 -- tap's -// neighbor resolution, if it turns out to need the same fix, is out of -// scope here since this fix targets the veth-only bug it was found from. -func configureHostGateway(vpc, vpcAttachment string, res *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 (that -// resolution happens as a side effect of the slow-path forwarding this -// datapath deliberately bypasses), so without a pre-existing neighbor table -// entry it fails with BPF_FIB_LKUP_RET_NO_NEIGH and the datapath counts and -// drops the packet (DROP_REASON_FIB_LOOKUP_FAILED) -- confirmed live: every -// cross-region packet to a pod that had never otherwise triggered NDP for -// its own address was silently and permanently blackholed, since nothing -// else in this attach path ever resolves it. 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 — see the anycast-avoidance note on -// configureHostGateway for why that route must not exist. 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. -// addrFlags is passed through to the netlink address (e.g. IFA_F_NOPREFIXROUTE -// to suppress the kernel's auto-created connected route for a wider mask). -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, - } - - // Check for existing routes with the same destination before installing. - 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, - ) - } - // Route already exists with matching attributes — idempotent, skip. - return nil - } - - if err := netlink.RouteAdd(desiredRoute); err != nil { - if errors.Is(err, syscall.EEXIST) { - return nil // already installed by a concurrent caller - } - 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 (design plan §5.1) -- the only forwarding path -// (the legacy seg6local static-route path was removed once this covered -// both veth and tap attachments, Milestone 6.1's tap-mode redirect fix). -// -// Design plan §4.4 assigns locator_table/function_table population to "the -// control daemon, at startup + on locator change." The actual control -// daemon (galactic-cni's "run" subcommand) does not read BGPRouter/watch -// for locator changes -- it only loads/attaches/pins the program -- so -// those two maps would otherwise sit permanently empty and every packet -// would locator_table-miss and pass through unchanged. This function -// registers all three tables (locator_table, function_table, vrf_table) -// from here instead, since the CNI ADD path already independently -// resolves bgp.srv6Locator/bgp.nodeID via lookupBGPRouter on every -// invocation -- an intentional deviation from the design plan's literal -// placement, not an oversight, tracked for revisiting once a real -// control-daemon-side CRD watch exists. -// -// argument is the same real, allocated 12-bit value (Milestone 6.1's -// allocateArgument) the router independently recomputes the BGP-advertised -// SID from (internal/reconcile) -- both must agree on the same value or a -// remote node's encapsulated traffic decodes into the wrong VRF. -// -// registerEBPFDatapath's return values let the caller record exactly what -// (if anything) was registered, so a later failed-ADD rollback -// (resourceTracker.cleanup, Milestone 7.2) can unregister the same -// (block, argument) pair without having to recompute or guess it. -// registered is false, with a nil error, only when this router has no -// srv6Locator/nodeID configured at all -- SRv6 is intentionally not set up -// for this attachment. Any other failure is returned as an error: with no -// legacy path to fall back to, the caller must treat that as fatal. -func registerEBPFDatapath( - bgp bgpConfig, vpc, vpcAttachment, ifaceType string, argument uint16, pinDir string, -) (registered bool, block uint64, err error) { - if bgp.srv6Locator == "" || bgp.nodeID == 0 { - return false, 0, nil - } - - // Validate the raw int32 nodeID against uformat's actual encode-time - // range *before* the uint16 narrowing below, mirroring srv6.ComputeSID's - // own bounds check on this exact value (internal/plumbing/srv6/usid.go). - // registry.Locator.Register below validates its uint16 argument via - // uformat.ValidateNodeID too, but only after this narrowing has already - // happened -- an out-of-[uint16] nodeID (e.g. 65537) wraps to some - // other, often perfectly in-range uint16 (1, here) that check can't - // tell apart from a legitimately-registered node's real Node-ID. Left - // unchecked here, that silently registers a locator_table entry for a - // Node-ID this router was never actually assigned, while ComputeSID - // (used independently by the router to build the SID it advertises) - // rejects the same raw value outright -- so nothing ever advertises - // reachability for the bogus entry this node just committed to - // forwarding, and it may collide with a different node's genuine one. - if bgp.nodeID < uformat.NodeIDMin || bgp.nodeID > uformat.NodeIDMax { - return false, 0, fmt.Errorf("eBPF registration: nodeID %d out of range [%#x,%#x]", - bgp.nodeID, uint16(uformat.NodeIDMin), uint16(uformat.NodeIDMax)) - } - - egressKind, err := egressKindForInterfaceType(ifaceType) - if err != nil { - return false, 0, fmt.Errorf("determine eBPF egress kind: %w", err) - } - - prefix, err := netip.ParsePrefix(bgp.srv6Locator) - if err != nil { - return false, 0, fmt.Errorf("parse SRv6 locator %q for eBPF registration: %w", bgp.srv6Locator, err) - } - block, err = uformat.Block(prefix.Addr()) - if err != nil { - return false, 0, fmt.Errorf("derive eBPF uSID Block from locator %q: %w", bgp.srv6Locator, err) - } - - vrfTableID, err := vrf.TableID(vpc, vpcAttachment) - if err != nil { - return false, 0, fmt.Errorf("look up VRF table id for eBPF registration: %w", err) - } - - registry, closer, err := usidmap.OpenPinnedRegistry(pinDir) - if err != nil { - return false, 0, fmt.Errorf("open pinned eBPF uSID maps: %w", err) - } - defer func() { _ = closer.Close() }() - - if err := registry.Locator.Register(block, uint16(bgp.nodeID)); err != nil { - return false, 0, fmt.Errorf("register eBPF locator_table entry: %w", err) - } - if err := registry.Function.Register(block, uformat.FunctionEndDT46); err != nil { - return false, 0, fmt.Errorf("register eBPF function_table entry: %w", err) - } - - if err := registry.VRF.Register(block, argument, vrfTableID, egressKind); err != nil { - return false, 0, fmt.Errorf("register eBPF vrf_table entry: %w", err) - } - return true, block, nil -} - -// egressKindForInterfaceType maps the CNI's InterfaceType field 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 -- internal/cni/tap -// creates it here and never moves it). This is what closes the tap-mode -// redirect_failed gap (Milestone 6.1's fix, design plan §4.2 step 9). -func egressKindForInterfaceType(ifaceType string) (uint32, error) { - switch ifaceType { - case interfaceTypeVeth, "": - // Empty matches config.go's own default-to-veth behavior for an - // omitted interface_type field. - return usidmap.EgressKindVeth, nil - case interfaceTypeTap: - return usidmap.EgressKindTap, nil - default: - return 0, fmt.Errorf("unknown interface type %q", ifaceType) - } -} - -// unregisterEBPFDatapath removes the vrf_table entry registerEBPFDatapath -// wrote for this (block, argument) pair, from the failed-ADD rollback path -// (resourceTracker.cleanup, Milestone 7.2). Unlike registerEBPFDatapath, -// this has no flag/config short-circuit of its own -- callers only invoke -// it when resourceTracker recorded a real registration -// (resourceTracker.ebpfRegistered), so by construction the flag was on and -// the maps were reachable at Register time. Idempotent: not an error if -// the entry is already gone (VRFTable.Unregister's own documented -// behavior). -// -// expectedVRFTableID must be this attachment's own VRF table id (recomputed -// by the caller via vrf.TableID, not read back from the tracker, since it's -// cheap and deterministic to recompute and the whole point here is not to -// trust stale state). A retried k8s-op attempt (retryK8sOps) can re-run the -// same publishBGPStateK8s closure without re-registering the eBPF entry -// (registerEBPFDatapath only runs again if that attempt gets far enough), -// 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 (vrf_table's key is just (block, -// argument); Register always overwrites). Unregistering unconditionally in -// that case would delete a live attachment's forwarding entry instead of -// this rolled-back one's own -- so this only deletes the entry when it -// still resolves to expectedVRFTableID, and leaves it alone otherwise. -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) - } - defer func() { _ = closer.Close() }() - - entry, ok, err := registry.VRF.Get(block, argument) - if err != nil { - return fmt.Errorf("read eBPF vrf_table entry before unregister: %w", err) - } - if !ok { - return nil // already gone - } - if entry.VRFTableID != expectedVRFTableID { - slog.Warn("Rollback: eBPF vrf_table entry no longer belongs to this attachment, leaving it in place", - "block", block, "argument", argument, - "expectedVRFTableID", expectedVRFTableID, "currentVRFTableID", entry.VRFTableID) - return nil - } - - if err := registry.VRF.Unregister(block, argument); err != nil { - return fmt.Errorf("unregister eBPF vrf_table entry: %w", err) - } - return nil -} diff --git a/internal/cni/cni.go b/internal/cni/cni.go index 33171596..762f27ad 100644 --- a/internal/cni/cni.go +++ b/internal/cni/cni.go @@ -10,59 +10,18 @@ import ( "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/version" + "go.datum.net/galactic/internal/cniipam" "go.datum.net/galactic/internal/metadata" ) const cniTimeout = 10 * time.Second -// ipamTypeStatic is the ipam type for a single pre-assigned static address. -// Any other (or empty) IPAM.Type value takes the pool-based dual-stack path -// — see wantsIPAM/allocateIPAM in ipam_ops.go. -const ipamTypeStatic = "static" - -// localIPAMDefaultPool is the IPv6 CIDR pool used when local IPAM is enabled -// but IPv6Subnet is unset in the CNI config. Allocations from it use -// ipam.DefaultSubnetLen (/96). -const localIPAMDefaultPool = "fd00:10:ff01::/64" - -const ( - // annotationAllocatedSubnetIPv6 is the BGPAdvertisement annotation key - // prefix holding the allocated IPv6 pod subnet CIDR (the /96) for a - // container ID. The full key appends a truncated container ID; see - // subnetAnnotationKeyIPv6. - annotationAllocatedSubnetIPv6 = "galactic.datum.net/allocated-subnet-ipv6" - - // annotationAllocatedSubnetIPv4 is the BGPAdvertisement annotation key - // prefix holding the allocated IPv4 pod address (the /32) for a - // container ID, when the attachment is dual-stack. The full key appends - // a truncated container ID; see subnetAnnotationKeyIPv4. - annotationAllocatedSubnetIPv4 = "galactic.datum.net/allocated-subnet-ipv4" - - // annotationNetNS is the BGPAdvertisement annotation key prefix holding - // the CNI-provided network namespace path for a container ID. The GC - // controller checks whether this exact path still exists to decide if - // the container is still live — it cannot reconstruct the path from the - // container ID alone, since netns bind-mounts are named by the - // runtime's own convention (e.g. containerd's "cni-"), which is - // unrelated to the container ID. The full key appends a truncated - // container ID; see netnsAnnotationKey. - annotationNetNS = "galactic.datum.net/netns" - - // annotationContainerIDLen is the number of characters used from a - // container ID in annotation keys. Kubernetes limits the name part of an - // annotation key to 63 bytes. The longest prefix sharing this constant is - // "allocated-subnet-ipv6." (or "-ipv4."), both 22 bytes, leaving 41 bytes - // for the container ID suffix — shorter prefixes ("netns.") just leave - // more room than they need. - annotationContainerIDLen = 41 -) - -const ( - // interfaceTypeVeth is the default interface type: veth pair for containers. - interfaceTypeVeth = "veth" - // interfaceTypeTap is the tap interface type: L2 fd for VMs (Kata, Firecracker). - interfaceTypeTap = "tap" -) +// SetEnableLocalIPAM sets the local IPAM flag from the CLI. Kept here as a +// thin forwarder so cmd/galactic-cni/main.go doesn't need to know that IPAM +// allocation itself now lives in internal/cniipam. +func SetEnableLocalIPAM(v bool) { + cniipam.SetEnableLocalIPAM(v) +} // RunPlugin starts the CNI plugin, handling ADD, DEL, CHECK, and STATUS operations. func RunPlugin() { diff --git a/internal/cni/cni_test.go b/internal/cni/cni_test.go index a4eeb9dc..39bd9360 100644 --- a/internal/cni/cni_test.go +++ b/internal/cni/cni_test.go @@ -15,19 +15,13 @@ import ( "reflect" "strings" "testing" - "time" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" type100 "github.com/containernetworking/cni/pkg/types/100" - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" + "go.datum.net/galactic/internal/cniipam" "go.datum.net/galactic/internal/config" - bgpv1alpha1 "go.datum.net/network/api/v1alpha1" ) func TestMain(m *testing.M) { @@ -39,17 +33,12 @@ func TestMain(m *testing.M) { const ( testVPC = "abc" testAttachment = "def" - testVPCHex1234 = "0000000004d2" // decimal 1234 - testRD65000_1 = "65000:1" // RD/RT for ASN 65000, NN 1 testContainerID = "test-container" testInvalidBase62 = "abc-def" // shared invalid base62 string for tests testNetns = "/proc/1/ns/net" testMac = "aa:bb:cc:dd:ee:ff" testIfName = "eth0" - testRouterName = "overlay-router" - testSID128 = "2001:db8::1/128" testCNIVersion = "1.0.0" - testIPv4Subnet = "10.128.0.0/20" // testPrevResult is a valid CNI v1.0.0 result used in prevResult tests. testPrevResult = `{"cniVersion":"1.0.0",` + @@ -58,32 +47,6 @@ const ( `"ips":[{"version":"6","address":"fd00:1::1/64"}]}` ) -func fakeClient(objs ...client.Object) client.Client { - return fake.NewClientBuilder().WithScheme(cniScheme).WithObjects(objs...).Build() -} - -// routerForNode builds a BGPRouter with spec.targetRef.name set to nodeName. -func routerForNode(name, nodeName, namespace string, asn int64) *bgpv1alpha1.BGPRouter { - return &bgpv1alpha1.BGPRouter{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - }, - Spec: bgpv1alpha1.BGPRouterSpec{ - TargetRef: bgpv1alpha1.TargetRef{ - Kind: "Node", - Name: nodeName, - }, - LocalASN: asn, - RouterID: "10.0.0.1", - Roles: []bgpv1alpha1.RouterRole{bgpv1alpha1.RouterRoleTenant}, - AddressFamilies: []bgpv1alpha1.AddressFamily{ - {AFI: bgpv1alpha1.AFIL2VPN, SAFI: bgpv1alpha1.SAFIEVPN}, - }, - }, - } -} - // 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. @@ -108,7 +71,6 @@ func TestParseConf(t *testing.T) { name string input string wantVPC string - wantIfType string wantAddressFamilies []string // nil means "don't check" wantErr string wantCode uint // CNI error code; 0 means "don't check" @@ -118,11 +80,10 @@ func TestParseConf(t *testing.T) { input: fmt.Sprintf( `{"cniVersion":"1.0.0","name":"test",`+ `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s","srv6_sid":"2001:db8::1/128"}`, + `"vpcattachment":"%s"}`, testVPC, testAttachment, ), - wantVPC: testVPC, - wantIfType: interfaceTypeVeth, + wantVPC: testVPC, }, { name: "invalid JSON", @@ -136,50 +97,6 @@ func TestParseConf(t *testing.T) { wantErr: "invalid CNI config", wantCode: 7, }, - { - name: "interface_type=veth", - input: fmt.Sprintf( - `{"cniVersion":"1.0.0","name":"test",`+ - `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s","interface_type":"veth"}`, - testVPC, testAttachment, - ), - wantVPC: testVPC, - wantIfType: interfaceTypeVeth, - }, - { - name: "interface_type=tap", - input: fmt.Sprintf( - `{"cniVersion":"1.0.0","name":"test",`+ - `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s","interface_type":"tap"}`, - testVPC, testAttachment, - ), - wantVPC: testVPC, - wantIfType: interfaceTypeTap, - }, - { - name: "interface_type empty defaults to veth", - input: fmt.Sprintf( - `{"cniVersion":"1.0.0","name":"test",`+ - `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s","interface_type":""}`, - testVPC, testAttachment, - ), - wantVPC: testVPC, - wantIfType: interfaceTypeVeth, - }, - { - name: "interface_type=unknown", - input: fmt.Sprintf( - `{"cniVersion":"1.0.0","name":"test",`+ - `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s","interface_type":"unknown"}`, - testVPC, testAttachment, - ), - wantErr: `invalid interface_type "unknown": must be "veth" or "tap"`, - wantCode: 7, - }, { name: "missing vpc", input: fmt.Sprintf( @@ -260,54 +177,8 @@ func TestParseConf(t *testing.T) { input: `{"cniVersion":"1.0.0","name":"test",` + `"type":"galactic-cni","vpc":"Abc123XYZ",` + `"vpcattachment":"DeF456"}`, - wantVPC: "Abc123XYZ", - wantIfType: interfaceTypeVeth, - }, - { - name: "valid srv6_sid with /128", - input: fmt.Sprintf( - `{"cniVersion":"1.0.0","name":"test",`+ - `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s","srv6_sid":"2001:db8::1/128"}`, - testVPC, testAttachment, - ), - wantVPC: testVPC, - wantIfType: interfaceTypeVeth, - }, - { - name: "valid srv6_sid bare IPv6 address", - input: fmt.Sprintf( - `{"cniVersion":"1.0.0","name":"test",`+ - `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s","srv6_sid":"2001:db8::1"}`, - testVPC, testAttachment, - ), - wantVPC: testVPC, - wantIfType: interfaceTypeVeth, + wantVPC: "Abc123XYZ", }, - { - name: "srv6_sid empty is allowed", - input: fmt.Sprintf( - `{"cniVersion":"1.0.0","name":"test",`+ - `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s","srv6_sid":""}`, - testVPC, testAttachment, - ), - wantVPC: testVPC, - wantIfType: interfaceTypeVeth, - }, - { - name: "srv6_sid missing is allowed", - input: fmt.Sprintf( - `{"cniVersion":"1.0.0","name":"test",`+ - `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s"}`, - testVPC, testAttachment, - ), - wantVPC: testVPC, - wantIfType: interfaceTypeVeth, - }, - { name: "prevResult valid JSON result is accepted", input: fmt.Sprintf( @@ -317,8 +188,7 @@ func TestParseConf(t *testing.T) { `"prevResult":%s}`, testVPC, testAttachment, testPrevResult, ), - wantVPC: testVPC, - wantIfType: interfaceTypeVeth, + wantVPC: testVPC, }, // ---- dual-stack addressing fields (ipv6_subnet, ipv4_subnet, address_families) ---- @@ -332,7 +202,6 @@ func TestParseConf(t *testing.T) { testVPC, testAttachment, ), wantVPC: testVPC, - wantIfType: interfaceTypeVeth, wantAddressFamilies: []string{addressFamilyIPv6}, }, { @@ -343,8 +212,7 @@ func TestParseConf(t *testing.T) { `"vpcattachment":"%s","ipv6_subnet":"fd00:10:ff01::/48"}`, testVPC, testAttachment, ), - wantVPC: testVPC, - wantIfType: interfaceTypeVeth, + wantVPC: testVPC, }, { name: "invalid ipv6_subnet CIDR rejected", @@ -387,8 +255,7 @@ func TestParseConf(t *testing.T) { `"vpcattachment":"%s","ipv4_subnet":"10.0.0.0/20"}`, testVPC, testAttachment, ), - wantVPC: testVPC, - wantIfType: interfaceTypeVeth, + wantVPC: testVPC, }, { // A standard dotted-decimal CIDR can never carry a mask longer @@ -425,7 +292,6 @@ func TestParseConf(t *testing.T) { testVPC, testAttachment, ), wantVPC: testVPC, - wantIfType: interfaceTypeVeth, wantAddressFamilies: []string{addressFamilyIPv6}, }, { @@ -437,7 +303,6 @@ func TestParseConf(t *testing.T) { testVPC, testAttachment, ), wantVPC: testVPC, - wantIfType: interfaceTypeVeth, wantAddressFamilies: []string{addressFamilyIPv6, addressFamilyIPv4}, }, { @@ -474,9 +339,6 @@ func TestParseConf(t *testing.T) { if conf.VPC != tt.wantVPC { t.Errorf("VPC = %q, want %q", conf.VPC, tt.wantVPC) } - if conf.InterfaceType != tt.wantIfType { - t.Errorf("InterfaceType = %q, want %q", conf.InterfaceType, tt.wantIfType) - } if tt.wantAddressFamilies != nil && !reflect.DeepEqual(conf.AddressFamilies, tt.wantAddressFamilies) { t.Errorf("AddressFamilies = %v, want %v", conf.AddressFamilies, tt.wantAddressFamilies) } @@ -631,242 +493,12 @@ func TestValidatePrevResultAdd(t *testing.T) { } } -// ---- bgpVRFInstanceName -------------------------------------------------- - -func TestBGPVRFInstanceName(t *testing.T) { - tests := []struct{ vpc, attachment, want string }{ - {testVPC, testAttachment, testVPC + "-" + testAttachment}, - {"0000000jU", "00G", "0000000jU-00G"}, - } - for _, tt := range tests { - got := bgpVRFInstanceName(tt.vpc, tt.attachment) - if got != tt.want { - t.Errorf("bgpVRFInstanceName(%q, %q) = %q, want %q", tt.vpc, tt.attachment, got, tt.want) - } - } -} - -// ---- bgpAdvertisementName ------------------------------------------------ - -func TestBGPAdvertisementName(t *testing.T) { - tests := []struct{ vpc, attachment, want string }{ - {testVPC, testAttachment, testVPC + "-" + testAttachment}, - {"0000000jU", "00G", "0000000jU-00G"}, - } - for _, tt := range tests { - got := bgpAdvertisementName(tt.vpc, tt.attachment) - if got != tt.want { - t.Errorf("bgpAdvertisementName(%q, %q) = %q, want %q", tt.vpc, tt.attachment, got, tt.want) - } - } -} - -// ---- routeTarget --------------------------------------------------------- - -func TestRouteTarget(t *testing.T) { - tests := []struct { - name string - asNumber int64 - vpcHex string - want string - wantErr bool - }{ - { - name: "VPC value fits in 32 bits", - asNumber: 65000, - vpcHex: testVPCHex1234, - want: "65000:1234", - }, - { - name: "upper bits beyond 32 stripped", - asNumber: 65000, - vpcHex: "000100000001", // 0x000100000001; low32 = 1 - want: testRD65000_1, - }, - { - name: "low 32 bits all set", - asNumber: 65000, - vpcHex: "0000ffffffff", - want: "65000:4294967295", - }, - { - name: "different ASN", - asNumber: 4200000000, - vpcHex: testVPCHex1234, - want: "4200000000:1234", - }, - { - name: "invalid hex string", - vpcHex: "zzzzzz", - wantErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := routeTarget(tt.asNumber, tt.vpcHex) - if tt.wantErr { - if err == nil { - t.Fatal("expected error, got nil") - } - return - } - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got != tt.want { - t.Errorf("routeTarget(%d, %q) = %q, want %q", tt.asNumber, tt.vpcHex, got, tt.want) - } - }) - } -} - -// ---- SetEnableLocalIPAM -------------------------------------------------- - -func TestSetEnableLocalIPAM(t *testing.T) { - // Save and restore original state. - original := enableLocalIPAM - defer func() { enableLocalIPAM = original }() - - // Default should be false. - if enableLocalIPAM { - t.Error("enableLocalIPAM default = true, want false") - } - - // Setting to true should work. - SetEnableLocalIPAM(true) - if !enableLocalIPAM { - t.Error("enableLocalIPAM after SetEnableLocalIPAM(true) = false, want true") - } - - // Setting back to false should work. - SetEnableLocalIPAM(false) - if enableLocalIPAM { - t.Error("enableLocalIPAM after SetEnableLocalIPAM(false) = true, want false") - } -} - -// ---- lookupBGPRouter ----------------------------------------------------- - -func TestLookupBGPRouter(t *testing.T) { - ctx := context.Background() - const ( - nodeName = "node1" - namespace = "default" - ) - - matchingRouter := routerForNode(testRouterName, nodeName, namespace, 65000) - - tests := []struct { - name string - objects []client.Object - wantErr string - check func(t *testing.T, cfg bgpConfig) - }{ - { - name: "no router for node", - objects: nil, - wantErr: "no BGPRouter found", - }, - { - name: "single matching router returns correct config", - objects: []client.Object{matchingRouter}, - check: func(t *testing.T, cfg bgpConfig) { - t.Helper() - if cfg.asNumber != 65000 { - t.Errorf("asNumber = %d, want 65000", cfg.asNumber) - } - if cfg.routerName != testRouterName { - t.Errorf("routerName = %q, want %q", cfg.routerName, testRouterName) - } - if cfg.srv6Locator != "" { - t.Errorf("srv6Locator = %q, want empty (not configured on fixture)", cfg.srv6Locator) - } - if cfg.nodeID != 0 { - t.Errorf("nodeID = %d, want 0 (not configured on fixture)", cfg.nodeID) - } - }, - }, - { - name: "router with SRv6Locator and NodeID configured", - objects: []client.Object{ - func() *bgpv1alpha1.BGPRouter { - r := routerForNode("srv6-router", nodeName, namespace, 65000) - r.Spec.SRv6Locator = "fd00:10::/48" - r.Spec.NodeID = 7 - return r - }(), - }, - check: func(t *testing.T, cfg bgpConfig) { - t.Helper() - if cfg.srv6Locator != "fd00:10::/48" { - t.Errorf("srv6Locator = %q, want %q", cfg.srv6Locator, "fd00:10::/48") - } - if cfg.nodeID != 7 { - t.Errorf("nodeID = %d, want 7", cfg.nodeID) - } - }, - }, - { - name: "router in different namespace is ignored", - objects: []client.Object{ - routerForNode("other-ns-router", nodeName, "other-ns", 65001), - }, - wantErr: "no BGPRouter found", - }, - { - name: "non-matching node router is ignored", - objects: []client.Object{ - routerForNode("other-node-router", "node2", namespace, 65001), - matchingRouter, - }, - check: func(t *testing.T, cfg bgpConfig) { - t.Helper() - if cfg.routerName != testRouterName { - t.Errorf("routerName = %q, want %q", cfg.routerName, testRouterName) - } - }, - }, - { - name: "ambiguous: two routers target same node", - objects: []client.Object{ - routerForNode("router-a", nodeName, namespace, 65000), - routerForNode("router-b", nodeName, namespace, 65001), - }, - wantErr: "ambiguous", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - k8s := fakeClient(tt.objects...) - - cfg, err := lookupBGPRouter(ctx, k8s, nodeName, namespace) - if tt.wantErr != "" { - if err == nil { - t.Fatalf("expected error containing %q, got nil", tt.wantErr) - } - if !strings.Contains(err.Error(), tt.wantErr) { - t.Fatalf("error %q does not contain %q", err, tt.wantErr) - } - return - } - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if tt.check != nil { - tt.check(t, cfg) - } - }) - } -} - // ---- buildResult --------------------------------------------------------- func TestBuildResult(t *testing.T) { subnet := mustParseCIDR(t, "fd00:10:ff01::1234/80") gateway := net.ParseIP("fd00:10:ff01::1") - route := mustParseCIDR(t, "::/0") + defaultRoute := mustParseCIDR(t, "::/0") netns := "/proc/1234/ns/net" conf := &PluginConf{ @@ -877,7 +509,7 @@ func TestBuildResult(t *testing.T) { tests := []struct { name string - ipRes *ipamResult + ipRes *cniipam.IPAMResult wantInts int wantIPs int wantRoutes int @@ -885,7 +517,7 @@ func TestBuildResult(t *testing.T) { }{ { name: "with IPAM config", - ipRes: &ipamResult{ipv6Subnet: subnet, ipv6Gateway: gateway, routes: []*net.IPNet{route}}, + ipRes: &cniipam.IPAMResult{IPv6Subnet: subnet, IPv6Gateway: gateway, Routes: []*net.IPNet{defaultRoute}}, wantInts: 2, wantIPs: 1, wantRoutes: 1, @@ -995,12 +627,12 @@ func TestBuildResultDualStack(t *testing.T) { VPC: testVPC, VPCAttachment: testAttachment, } - ipRes := &ipamResult{ - ipv6Subnet: ipv6Subnet, - ipv6Gateway: ipv6Gateway, - ipv4Address: ipv4Address, - ipv4Gateway: ipv4Gateway, - routes: []*net.IPNet{ipv6Route, ipv4Route}, + ipRes := &cniipam.IPAMResult{ + IPv6Subnet: ipv6Subnet, + IPv6Gateway: ipv6Gateway, + IPv4Address: ipv4Address, + IPv4Gateway: ipv4Gateway, + Routes: []*net.IPNet{ipv6Route, ipv4Route}, } result := buildResult(conf, ipRes, "G09-vpc03-vpcAttH", "eth0", @@ -1046,10 +678,10 @@ func TestBuildResultIPv4Only(t *testing.T) { VPC: testVPC, VPCAttachment: testAttachment, } - ipRes := &ipamResult{ - ipv4Address: ipv4Address, - ipv4Gateway: ipv4Gateway, - routes: []*net.IPNet{ipv4Route}, + ipRes := &cniipam.IPAMResult{ + IPv4Address: ipv4Address, + IPv4Gateway: ipv4Gateway, + Routes: []*net.IPNet{ipv4Route}, } result := buildResult(conf, ipRes, "G09-vpc03-vpcAttH", "eth0", @@ -1073,162 +705,6 @@ func TestBuildResultIPv4Only(t *testing.T) { } } -// ---- buildTapResult ------------------------------------------------------ - -func TestBuildTapResult(t *testing.T) { - subnet := mustParseCIDR(t, "fd00:10:ff01::1234/80") - gateway := net.ParseIP("fd00:10:ff01::1") - route := mustParseCIDR(t, "::/0") - - conf := &PluginConf{ - PluginConf: types.PluginConf{CNIVersion: testCNIVersion}, - VPC: testVPC, - VPCAttachment: testAttachment, - } - - tests := []struct { - name string - ipRes *ipamResult - wantIPs int - wantRoutes int - }{ - { - name: "with IPAM config", - ipRes: &ipamResult{ipv6Subnet: subnet, ipv6Gateway: gateway, routes: []*net.IPNet{route}}, - wantIPs: 1, - wantRoutes: 1, - }, - { - name: "without IPAM config", - ipRes: nil, - wantIPs: 0, - wantRoutes: 0, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := buildTapResult(conf, tt.ipRes, "H0abc123", "aa:bb:cc:dd:ee:ff", 1500) - - if result.CNIVersion != testCNIVersion { - t.Errorf("CNIVersion = %q, want %q", result.CNIVersion, testCNIVersion) - } - - if len(result.Interfaces) != 1 { - t.Fatalf("Interfaces count = %d, want 1", len(result.Interfaces)) - } - - if result.Interfaces[0].Name != "H0abc123" { - t.Errorf("Interfaces[0].Name = %q, want %q", result.Interfaces[0].Name, "H0abc123") - } - if result.Interfaces[0].Mac != "aa:bb:cc:dd:ee:ff" { - t.Errorf("Interfaces[0].Mac = %q, want %q", result.Interfaces[0].Mac, "aa:bb:cc:dd:ee:ff") - } - if result.Interfaces[0].Mtu != 1500 { - t.Errorf("Interfaces[0].Mtu = %d, want 1500", result.Interfaces[0].Mtu) - } - if result.Interfaces[0].Sandbox != "" { - t.Errorf("Interfaces[0].Sandbox = %q, want empty", result.Interfaces[0].Sandbox) - } - - if len(result.IPs) != tt.wantIPs { - t.Errorf("IPs count = %d, want %d", len(result.IPs), tt.wantIPs) - } - if tt.wantIPs > 0 { - if result.IPs[0].Address.String() != subnet.String() { - t.Errorf("IPs[0].Address = %q, want %q", result.IPs[0].Address, subnet) - } - if !result.IPs[0].Gateway.Equal(gateway) { - t.Errorf("IPs[0].Gateway = %v, want %v", result.IPs[0].Gateway, gateway) - } - if result.IPs[0].Interface == nil || *result.IPs[0].Interface != 0 { - t.Errorf("IPs[0].Interface = %v, want 0", result.IPs[0].Interface) - } - } - - if len(result.Routes) != tt.wantRoutes { - t.Errorf("Routes count = %d, want %d", len(result.Routes), tt.wantRoutes) - } - }) - } -} - -// TestBuildTapResultIPv4Mask verifies that buildTapResult reports the IPv4 -// address with a /25 mask (matching the host gateway mask -// ipv4GatewayAddrParams installs on the tap interface), not the /32 used for -// veth. -func TestBuildTapResultIPv4Mask(t *testing.T) { - ipv4Address := net.ParseIP("172.20.1.5") - ipv4Gateway := net.ParseIP("172.20.1.1") - ipv4Route := mustParseCIDR(t, "0.0.0.0/0") - - conf := &PluginConf{ - PluginConf: types.PluginConf{CNIVersion: testCNIVersion}, - VPC: testVPC, - VPCAttachment: testAttachment, - } - ipRes := &ipamResult{ - ipv4Address: ipv4Address, - ipv4Gateway: ipv4Gateway, - routes: []*net.IPNet{ipv4Route}, - } - - result := buildTapResult(conf, ipRes, "H0abc123", "aa:bb:cc:dd:ee:ff", 1500) - - if len(result.IPs) != 1 { - t.Fatalf("IPs count = %d, want 1", len(result.IPs)) - } - wantIPv4Mask := net.CIDRMask(25, 32).String() - if result.IPs[0].Address.IP.String() != ipv4Address.String() || result.IPs[0].Address.Mask.String() != wantIPv4Mask { - t.Errorf("IPs[0].Address = %v, want %s/25", result.IPs[0].Address, ipv4Address) - } - if !result.IPs[0].Gateway.Equal(ipv4Gateway) { - t.Errorf("IPs[0].Gateway = %v, want %v", result.IPs[0].Gateway, ipv4Gateway) - } - if result.IPs[0].Interface == nil || *result.IPs[0].Interface != 0 { - t.Errorf("IPs[0].Interface = %v, want 0 (host tap)", result.IPs[0].Interface) - } -} - -// TestBuildTapResultHostNetns verifies that the tap path produces a valid -// CNI result when args.Netns is the host network namespace. Kraftlet/unikraft -// workloads pass the host netns because they don't have a Linux network -// namespace. The main.go entry point detects interface_type=tap and sets -// CNI_NETNS_OVERRIDE to bypass the CNI library's same-netns rejection check. -// The tap result must not reference a sandbox. -func TestBuildTapResultHostNetns(t *testing.T) { - subnet := mustParseCIDR(t, "fd00:10:ff01::1234/80") - gateway := net.ParseIP("fd00:10:ff01::1") - route := mustParseCIDR(t, "::/0") - - conf := &PluginConf{ - PluginConf: types.PluginConf{CNIVersion: testCNIVersion}, - VPC: testVPC, - VPCAttachment: testAttachment, - } - ipRes := &ipamResult{ipv6Subnet: subnet, ipv6Gateway: gateway, routes: []*net.IPNet{route}} - - result := buildTapResult(conf, ipRes, "H0abc123", "aa:bb:cc:dd:ee:ff", 1500) - - // Result should be structurally valid for kraftlet (host netns) workloads. - if result.CNIVersion != testCNIVersion { - t.Errorf("CNIVersion = %q, want %q", result.CNIVersion, testCNIVersion) - } - if len(result.Interfaces) != 1 { - t.Fatalf("Interfaces count = %d, want 1", len(result.Interfaces)) - } - // Host tap interface must not reference a sandbox (kraftlet has no netns). - if result.Interfaces[0].Sandbox != "" { - t.Errorf("Interfaces[0].Sandbox = %q, want empty (host netns, no sandbox)", result.Interfaces[0].Sandbox) - } - if len(result.IPs) != 1 { - t.Fatalf("IPs count = %d, want 1", len(result.IPs)) - } - if len(result.Routes) != 1 { - t.Fatalf("Routes count = %d, want 1", len(result.Routes)) - } -} - // ---- cmdDel idempotency -------------------------------------------------- // TestCmdDelIdempotent returns nil even when the CNI config is invalid. @@ -1248,14 +724,10 @@ func TestCmdDelIdempotent(t *testing.T) { // TestCmdDelIdempotentMissingResources returns nil even when the config is // valid but all resources are missing (k8s client creation fails in tests). func TestCmdDelIdempotentMissingResources(t *testing.T) { - // Save and restore the original enableLocalIPAM state. - original := enableLocalIPAM - defer func() { enableLocalIPAM = original }() - conf := fmt.Sprintf( `{"cniVersion":"1.0.0","name":"test",`+ `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s","interface_type":"veth"}`, + `"vpcattachment":"%s"}`, testVPC, testAttachment, ) args := &skel.CmdArgs{ @@ -1292,7 +764,7 @@ func TestCmdDelFlushesGuestNetnsConfig(t *testing.T) { conf := fmt.Sprintf( `{"cniVersion":"1.0.0","name":"test",`+ `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s","interface_type":"veth"}`, + `"vpcattachment":"%s"}`, testVPC, testAttachment, ) args := &skel.CmdArgs{ @@ -1340,27 +812,6 @@ func TestCmdCheckInvalidConfig(t *testing.T) { } } -func TestCmdCheckInvalidInterfaceType(t *testing.T) { - conf := fmt.Sprintf( - `{"cniVersion":"1.0.0","name":"test",`+ - `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s","interface_type":"bogus"}`, - testVPC, testAttachment, - ) - args := &skel.CmdArgs{ - ContainerID: testContainerID, - StdinData: []byte(conf), - } - - err := cmdCheck(args) - if err == nil { - t.Fatalf("expected error for invalid interface_type, got nil") - } - if !strings.Contains(err.Error(), `invalid interface_type "bogus"`) { - t.Fatalf("error %q does not contain expected message", err.Error()) - } -} - func TestCmdCheckValidConfigMissingResources(t *testing.T) { conf := fmt.Sprintf( `{"cniVersion":"1.0.0","name":"test",`+ @@ -1384,27 +835,6 @@ func TestCmdCheckValidConfigMissingResources(t *testing.T) { } } -func TestCmdCheckTapModeValidConfigMissingResources(t *testing.T) { - conf := fmt.Sprintf( - `{"cniVersion":"1.0.0","name":"test",`+ - `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s","interface_type":"tap"}`, - testVPC, testAttachment, - ) - args := &skel.CmdArgs{ - ContainerID: testContainerID, - StdinData: []byte(conf), - } - - err := cmdCheck(args) - if err == nil { - t.Fatalf("expected CHECK failure for missing resources, got nil") - } - if !strings.Contains(err.Error(), "CHECK failed") { - t.Fatalf("error %q does not contain 'CHECK failed'", err.Error()) - } -} - func TestCmdCheckMissingVPC(t *testing.T) { conf := `{"cniVersion":"1.0.0","name":"test","type":"galactic-cni"}` args := &skel.CmdArgs{ @@ -1498,7 +928,6 @@ func TestResourceTrackerCleanupPartialState(t *testing.T) { tracker := &resourceTracker{ vpc: testVPC, vpcAttachment: testAttachment, - ifaceType: interfaceTypeVeth, namespace: "default", } ctx := context.Background() @@ -1509,7 +938,6 @@ func TestResourceTrackerFieldsSet(t *testing.T) { tracker := &resourceTracker{ vpc: testVPC, vpcAttachment: testAttachment, - ifaceType: interfaceTypeTap, namespace: "test-ns", } @@ -1519,9 +947,6 @@ func TestResourceTrackerFieldsSet(t *testing.T) { if tracker.vpcAttachment != testAttachment { t.Errorf("vpcAttachment = %q, want %q", tracker.vpcAttachment, testAttachment) } - if tracker.ifaceType != interfaceTypeTap { - t.Errorf("ifaceType = %q, want %q", tracker.ifaceType, interfaceTypeTap) - } if tracker.namespace != "test-ns" { t.Errorf("namespace = %q, want %q", tracker.namespace, "test-ns") } @@ -1545,22 +970,6 @@ func TestCmdStatusInvalidConfig(t *testing.T) { assertCNIError(t, err, 7, "invalid CNI config") } -func TestCmdStatusInvalidInterfaceType(t *testing.T) { - conf := fmt.Sprintf( - `{"cniVersion":"1.0.0","name":"test",`+ - `"type":"galactic-cni","vpc":"%s",`+ - `"vpcattachment":"%s","interface_type":"bogus"}`, - testVPC, testAttachment, - ) - args := &skel.CmdArgs{ - ContainerID: testContainerID, - StdinData: []byte(conf), - } - - err := cmdStatus(args) - assertCNIError(t, err, 7, `invalid interface_type "bogus"`) -} - func TestCmdStatusValidConfigMissingResources(t *testing.T) { // STATUS should succeed with valid config even when VRF/interface // resources don't exist — STATUS answers "is the plugin ready to ADD?" @@ -1653,163 +1062,6 @@ func TestCmdStatusAPIProbeFailure(t *testing.T) { assertCNIError(t, err, 50, "API server health check failed") } -// ---- isTransientError ---------------------------------------------------- - -func TestIsTransientError(t *testing.T) { - tests := []struct { - name string - err error - wantTrans bool - }{ - { - name: "nil error is not transient", - err: nil, - wantTrans: false, - }, - { - name: "context deadline exceeded is transient", - err: context.DeadlineExceeded, - wantTrans: true, - }, - { - name: "context canceled is transient", - err: context.Canceled, - wantTrans: true, - }, - { - name: "wrapped context deadline exceeded is transient", - err: fmt.Errorf("k8s: %w", context.DeadlineExceeded), - wantTrans: true, - }, - { - name: "wrapped context canceled is transient", - err: fmt.Errorf("k8s: %w", context.Canceled), - wantTrans: true, - }, - { - name: "generic error is not transient", - err: errors.New("some error"), - wantTrans: false, - }, - { - name: "validation error is not transient", - err: apierrors.NewBadRequest("bad request"), - wantTrans: false, - }, - { - name: "not found error is not transient", - err: apierrors.NewNotFound( - schema.GroupResource{Group: "network.datumapis.com", Resource: "bgpadvertisements"}, "test"), - wantTrans: false, - }, - { - name: "503 service unavailable is transient", - err: apierrors.NewServiceUnavailable("service unavailable"), - // apierrors.IsServiceUnavailable catches 503. - wantTrans: true, - }, - { - name: "429 too many requests is transient", - err: apierrors.NewTooManyRequests("too many requests", 0), - // apierrors.IsTooManyRequests catches 429. - wantTrans: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := isTransientError(tt.err) - if got != tt.wantTrans { - t.Errorf("isTransientError(%v) = %v, want %v", tt.err, got, tt.wantTrans) - } - }) - } -} - -// ---- retryK8sOps --------------------------------------------------------- - -func TestRetryK8sOpsSucceedsImmediately(t *testing.T) { - calls := 0 - err := retryK8sOps(100*time.Millisecond, func(ctx context.Context) error { - calls++ - return nil - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if calls != 1 { - t.Errorf("expected 1 call, got %d", calls) - } -} - -func TestRetryK8sOpsRetriesOnTransientError(t *testing.T) { - calls := 0 - err := retryK8sOps(2*time.Second, func(ctx context.Context) error { - calls++ - if calls < 3 { - return context.DeadlineExceeded - } - return nil - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if calls != 3 { - t.Errorf("expected 3 calls (initial + 2 retries), got %d", calls) - } -} - -func TestRetryK8sOpsFailsAfterMaxRetries(t *testing.T) { - calls := 0 - err := retryK8sOps(2*time.Second, func(ctx context.Context) error { - calls++ - return context.DeadlineExceeded - }) - if err == nil { - t.Fatal("expected error, got nil") - } - if calls != maxRetries+1 { - t.Errorf("expected %d calls (initial + maxRetries), got %d", maxRetries+1, calls) - } -} - -func TestRetryK8sOpsNoRetryOnNonTransientError(t *testing.T) { - calls := 0 - permanentErr := errors.New("validation failed") - err := retryK8sOps(2*time.Second, func(ctx context.Context) error { - calls++ - return permanentErr - }) - if !errors.Is(err, permanentErr) { - t.Fatalf("expected %v, got %v", permanentErr, err) - } - if calls != 1 { - t.Errorf("expected 1 call (no retry), got %d", calls) - } -} - -func TestRetryK8sOpsExhaustsDeadline(t *testing.T) { - // When the timeout is very short, retries still happen but the fn - // completes instantly — so we exhaust maxRetries and get the last - // transient error back (not a context timeout, since fn is fast). - calls := 0 - err := retryK8sOps(1*time.Millisecond, func(ctx context.Context) error { - calls++ - return apierrors.NewServiceUnavailable("unavailable") - }) - if err == nil { - t.Fatal("expected error, got nil") - } - // Should have made maxRetries+1 attempts (initial + 2 retries). - if calls != maxRetries+1 { - t.Errorf("expected %d calls, got %d", maxRetries+1, calls) - } - // Final error is the last transient error returned by fn. - if !strings.Contains(err.Error(), "unavailable") { - t.Errorf("expected 'unavailable' in error, got %v", err) - } -} - // ---- probeAPIServer ------------------------------------------------------ func TestProbeAPIServerErrNotInCluster(t *testing.T) { @@ -1851,7 +1103,6 @@ func TestCmdAddPrevResultValid(t *testing.T) { t.Setenv("GALACTIC_CNI_NODE_NAME", "") t.Setenv("NODE_NAME", "") // prevResult that is a valid CNI result. cmdAdd should pass prevResult - // validation and fail later due to missing node name. conf := fmt.Sprintf( `{"cniVersion":"1.0.0","name":"test",`+ @@ -1969,45 +1220,6 @@ func TestEnableLocalIPAMRequired(t *testing.T) { } } -// ---- annotation key length ------------------------------------------------- - -// TestAnnotationKeyNameLength verifies that every annotation key builder stays -// within Kubernetes' 63-byte limit on the "name" part of an annotation key -// (the segment after the last "/"), using a realistic 64-character container -// ID (containerd/Docker use full SHA256 hex digests). This guards against a -// real production incident: annotationContainerIDLen was sized for the old -// "allocated-subnet." prefix (17 bytes) and wasn't updated when the prefix -// grew by 5 bytes to "allocated-subnet-ipv6."/"-ipv4." — every BGPAdvertisement -// apply failed with "name part must be no more than 63 bytes" until fixed. -func TestAnnotationKeyNameLength(t *testing.T) { - const maxAnnotationNameLen = 63 - // A realistic full-length container ID (64 hex chars, as containerd/Docker use). - fullContainerID := strings.Repeat("a", 64) - - tests := []struct { - name string - key string - }{ - {"subnetAnnotationKeyIPv6", subnetAnnotationKeyIPv6(fullContainerID)}, - {"subnetAnnotationKeyIPv4", subnetAnnotationKeyIPv4(fullContainerID)}, - {"netnsAnnotationKey", netnsAnnotationKey(fullContainerID)}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - slash := strings.LastIndex(tt.key, "/") - namePart := tt.key - if slash != -1 { - namePart = tt.key[slash+1:] - } - if len(namePart) > maxAnnotationNameLen { - t.Errorf("%s(%d-char containerID) name part %q is %d bytes, want <= %d", - tt.name, len(fullContainerID), namePart, len(namePart), maxAnnotationNameLen) - } - }) - } -} - // ---- logging setup ---------------------------------------------------------- func TestLoggingSetup(t *testing.T) { diff --git a/internal/cni/config.go b/internal/cni/config.go index 3ff10a04..3eda19e7 100644 --- a/internal/cni/config.go +++ b/internal/cni/config.go @@ -5,7 +5,6 @@ package cni import ( - "context" "encoding/json" "errors" "fmt" @@ -17,12 +16,9 @@ import ( "github.com/containernetworking/cni/pkg/types" type100 "github.com/containernetworking/cni/pkg/types/100" - "github.com/vishvananda/netlink" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" + "go.datum.net/galactic/internal/cni/hostconf" + "go.datum.net/galactic/internal/cniipam" "go.datum.net/galactic/internal/config" ) @@ -88,113 +84,38 @@ func isValidBase62(s string) bool { return true } -// conflistEnvelope matches standard CNI conflist JSON structure. -type conflistEnvelope struct { - CNIVersion string `json:"cniVersion"` - Name string `json:"name"` - Plugins []json.RawMessage `json:"plugins"` -} - -// loadHostConf loads node-local settings from the CNI conflist. -// If the file is missing, it returns a zero-value HostConf (tolerating local test runs) -// but still defaulting Namespace to config.DefaultNamespace. +// 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 } - data, err := os.ReadFile(filePath) + conf, err := hostconf.Load(filePath, hostconf.PluginType) if err != nil { - if os.IsNotExist(err) { - // Tolerated, return defaulted config. - return &HostConf{ - Namespace: config.DefaultNamespace, - }, nil + if os.IsNotExist(unwrapPathError(err)) { + return &HostConf{Namespace: config.DefaultNamespace}, nil } - return nil, fmt.Errorf("read conflist file %q: %w", filePath, err) - } - - var env conflistEnvelope - if err := json.Unmarshal(data, &env); err != nil { - return nil, fmt.Errorf("parse conflist envelope: %w", err) + return nil, err } - - for _, raw := range env.Plugins { - var meta struct { - Type string `json:"type"` - } - if err := json.Unmarshal(raw, &meta); err != nil { - continue - } - if meta.Type == "galactic-cni" { - var conf HostConf - if err := json.Unmarshal(raw, &conf); err != nil { - return nil, fmt.Errorf("parse host CNI config: %w", err) - } - if conf.Namespace == "" { - conf.Namespace = config.DefaultNamespace - } - return &conf, nil - } + if conf.Namespace == "" { + conf.Namespace = config.DefaultNamespace } - - return nil, fmt.Errorf("conflist at %q does not contain a plugin with type \"galactic-cni\"", filePath) + return conf, nil } -// detectNodeNameFromAPI queries the Kubernetes API and matches the node's -// InternalIP addresses against local interface addresses. Returns the first -// matching node name, or empty string with no error if detection fails -// (allowing callers to fall through to other resolution methods). -func detectNodeNameFromAPI() (string, error) { - restCfg, err := ctrl.GetConfig() - if err != nil { - return "", fmt.Errorf("get kubeconfig: %w", err) - } - - k8sClient, err := client.New(restCfg, client.Options{ - Scheme: buildDetectScheme(), - }) - if err != nil { - return "", fmt.Errorf("create k8s client: %w", err) - } - - var nodeList corev1.NodeList - if err := k8sClient.List(context.Background(), &nodeList, &client.ListOptions{ - Limit: 1000, - }); err != nil { - return "", fmt.Errorf("list nodes: %w", err) - } - - // Collect all local interface addresses - addrs, err := netlink.AddrList(nil, netlink.FAMILY_ALL) - if err != nil { - return "", fmt.Errorf("list local addresses: %w", err) - } - - localIPs := make(map[string]bool, len(addrs)) - for _, addr := range addrs { - localIPs[addr.IP.String()] = true - } - - // Match against node InternalIPs - for _, node := range nodeList.Items { - for _, addr := range node.Status.Addresses { - if addr.Type == corev1.NodeInternalIP && localIPs[addr.Address] { - slog.Info("Auto-detected node name from Kubernetes API", - "nodeName", node.Name, "matchedIP", addr.Address) - return node.Name, 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 } - - return "", errors.New("no local interface address matched any node InternalIP") -} - -// buildDetectScheme returns a minimal scheme containing only corev1 types -// needed for node name detection. -func buildDetectScheme() *runtime.Scheme { - scheme := runtime.NewScheme() - _ = corev1.AddToScheme(scheme) - return scheme } // parseLogLevel maps a config-supplied level name to a slog.Level. Matching is @@ -249,22 +170,19 @@ func setupLogging(logPath, logLevel string) { } // statusConf holds the minimal CNI config fields needed for STATUS validation. - -// STATUS only checks that the config is parseable and the API server is reachable; -// it does not validate attachment-specific fields (VPC, VPCAttachment) because -// STATUS must succeed before any ADD has ever run. +// +// STATUS only checks that the config is parseable and the API server is +// reachable; it does not validate attachment-specific fields (VPC, +// VPCAttachment) because STATUS must succeed before any ADD has ever run. type statusConf struct { - CNIVersion string `json:"cniVersion"` - Type string `json:"type"` - InterfaceType string `json:"interface_type"` + CNIVersion string `json:"cniVersion"` + Type string `json:"type"` } // parseStatusConf validates that the CNI config is parseable and contains the // required top-level fields (cniVersion, type). Unlike parseConf, it does not // validate VPC or VPCAttachment because STATUS must succeed on a freshly -// started node before any ADD has run. However, interface_type is validated -// if present because it is a structural config field, not an attachment -// identifier. +// started node before any ADD has run. func parseStatusConf(data []byte) error { var sc statusConf if err := json.Unmarshal(data, &sc); err != nil { @@ -276,17 +194,6 @@ func parseStatusConf(data []byte) error { if sc.Type == "" { return &types.Error{Code: 7, Msg: "type is required"} } - // Validate interface_type if present. - if sc.InterfaceType != "" { - switch sc.InterfaceType { - case interfaceTypeVeth, interfaceTypeTap: - default: - return &types.Error{Code: 7, Msg: fmt.Sprintf( - "invalid interface_type %q: must be %q or %q", - sc.InterfaceType, interfaceTypeVeth, interfaceTypeTap, - )} - } - } return nil } @@ -339,8 +246,8 @@ func validatePrevResultAdd(res types.Result) error { } // parseConf unmarshals the CNI configuration from stdin data and validates -// the interface type and base62-encoded identifier fields. It resolves the -// host configuration and sets up process environment variables and logging. +// the base62-encoded identifier fields. It resolves the host configuration +// and sets up process environment variables and logging. func parseConf(data []byte) (*PluginConf, error) { conf := &PluginConf{} if err := json.Unmarshal(data, &conf); err != nil { @@ -385,7 +292,7 @@ func parseConf(data []byte) (*PluginConf, error) { // the conflist file is missing (e.g. hostPath mount issues in container- // based environments like Kind). if cniConfig.NodeName == "" { - detected, detectErr := detectNodeNameFromAPI() + detected, detectErr := hostconf.DetectNodeNameFromAPI() if detectErr != nil { slog.Warn("Node name auto-detection failed", "err", detectErr) } @@ -410,11 +317,15 @@ func parseConf(data []byte) (*PluginConf, error) { setupLogging(cniConfig.LogFile, cniConfig.LogLevel) slog.Debug("CNI config received", "stdin", string(data)) - // Resolve local IPAM flag - enableLocalIPAM = config.CNIGetEnableLocalIPAM() + // Resolve local IPAM flag and propagate it to internal/cniipam, which + // owns wantsIPAM/allocateIPAM. (This env-var-as-trigger shape is + // slated to be replaced by an explicit ipam-block-presence contract + // once internal/cniipam becomes its own delegated plugin.) + localIPAM := config.CNIGetEnableLocalIPAM() + cniipam.SetEnableLocalIPAM(localIPAM) // Enforce required IPAM block if local IPAM is enabled - if enableLocalIPAM && conf.IPAM == nil { + if localIPAM && conf.IPAM == nil { return nil, &types.Error{Code: 7, Msg: "local IPAM is enabled, but no 'ipam' block is present in the configuration"} } @@ -422,7 +333,7 @@ func parseConf(data []byte) (*PluginConf, error) { // address_families). Both subnet fields stay optional at the parseConf // level: whether one is actually required depends on which IPAM path a // given ADD takes (static, local-IPAM fallback, or pool), which is - // resolved in allocateIPAM — see wantsIPAM/allocateIPAM in ipam_ops.go. + // resolved in internal/cniipam.Allocate — see WantsIPAM/Allocate there. // When present, both are validated for CIDR shape so misconfigurations // are caught early regardless of which path runs. if conf.IPv6Subnet != "" { @@ -483,18 +394,6 @@ func parseConf(data []byte) (*PluginConf, error) { return nil, &types.Error{Code: 6, Msg: fmt.Sprintf("invalid prevResult: %v", err)} } } - if conf.InterfaceType == "" { - conf.InterfaceType = interfaceTypeVeth - } - switch conf.InterfaceType { - case interfaceTypeVeth, interfaceTypeTap: - default: - return nil, &types.Error{Code: 7, Msg: fmt.Sprintf( - "invalid interface_type %q: must be %q or %q", - conf.InterfaceType, interfaceTypeVeth, interfaceTypeTap, - )} - - } return conf, nil } @@ -508,36 +407,3 @@ func sanitizeForError(s string) string { } return s } - -// subnetAnnotationKeyIPv6 returns the annotation key for storing the -// allocated IPv6 subnet for the given container ID. Kubernetes limits the -// name part of an annotation key to 63 bytes; "allocated-subnet-ipv6." is 22 -// bytes, leaving 41 bytes for the container ID prefix. -func subnetAnnotationKeyIPv6(containerID string) string { - id := containerID - if len(id) > annotationContainerIDLen { - id = id[:annotationContainerIDLen] - } - return fmt.Sprintf("%s.%s", annotationAllocatedSubnetIPv6, id) -} - -// subnetAnnotationKeyIPv4 returns the annotation key for storing the -// allocated IPv4 address for the given container ID. Mirrors -// subnetAnnotationKeyIPv6. -func subnetAnnotationKeyIPv4(containerID string) string { - id := containerID - if len(id) > annotationContainerIDLen { - id = id[:annotationContainerIDLen] - } - return fmt.Sprintf("%s.%s", annotationAllocatedSubnetIPv4, id) -} - -// netnsAnnotationKey returns the annotation key for storing the network -// namespace path used by the given container ID. Mirrors subnetAnnotationKeyIPv6. -func netnsAnnotationKey(containerID string) string { - id := containerID - if len(id) > annotationContainerIDLen { - id = id[:annotationContainerIDLen] - } - return fmt.Sprintf("%s.%s", annotationNetNS, id) -} diff --git a/internal/cni/crdnames/crdnames.go b/internal/cni/crdnames/crdnames.go new file mode 100644 index 00000000..a39902b7 --- /dev/null +++ b/internal/cni/crdnames/crdnames.go @@ -0,0 +1,83 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +// Package crdnames is the shared vocabulary for naming and annotating the +// BGPVRFInstance/BGPAdvertisement CRDs a VPC attachment's chain of plugins +// cooperate on: galactic-bgp writes them, galactic-ipam's deallocation path +// (until its own local marker-file persistence lands) reads the subnet +// annotations back, and galactic-router's GC controller reads the netns +// annotation to decide liveness. Kept as one small leaf package — imported +// by internal/cni, internal/cnitap, internal/cniipam, and internal/cnibgp — +// so none of those need to import each other just to agree on a name. +package crdnames + +import "fmt" + +// AnnotationAllocatedSubnetIPv6 is the BGPAdvertisement annotation key prefix +// holding the allocated IPv6 pod subnet CIDR (the /96) for a container ID. +// The full key appends a truncated container ID; see SubnetKeyIPv6. +const AnnotationAllocatedSubnetIPv6 = "galactic.datum.net/allocated-subnet-ipv6" + +// AnnotationAllocatedSubnetIPv4 is the BGPAdvertisement annotation key prefix +// holding the allocated IPv4 pod address (the /32) for a container ID, when +// the attachment is dual-stack. The full key appends a truncated container +// ID; see SubnetKeyIPv4. +const AnnotationAllocatedSubnetIPv4 = "galactic.datum.net/allocated-subnet-ipv4" + +// AnnotationNetNS is the BGPAdvertisement annotation key prefix holding the +// CNI-provided network namespace path for a container ID. The GC controller +// checks whether this exact path still exists to decide if the container is +// still live — it cannot reconstruct the path from the container ID alone, +// since netns bind-mounts are named by the runtime's own convention (e.g. +// containerd's "cni-"), which is unrelated to the container ID. The +// full key appends a truncated container ID; see NetNSKey. +const AnnotationNetNS = "galactic.datum.net/netns" + +// containerIDLen is the number of characters used from a container ID in +// annotation keys. Kubernetes limits the name part of an annotation key to +// 63 bytes. The longest prefix sharing this constant is +// "allocated-subnet-ipv6." (or "-ipv4."), both 22 bytes, leaving 41 bytes for +// the container ID suffix — shorter prefixes ("netns.") just leave more room +// than they need. +const containerIDLen = 41 + +// truncate returns id, shortened to containerIDLen characters if longer. +func truncate(id string) string { + if len(id) > containerIDLen { + return id[:containerIDLen] + } + return id +} + +// SubnetKeyIPv6 returns the annotation key for storing the allocated IPv6 +// subnet for the given container ID. +func SubnetKeyIPv6(containerID string) string { + return fmt.Sprintf("%s.%s", AnnotationAllocatedSubnetIPv6, truncate(containerID)) +} + +// SubnetKeyIPv4 returns the annotation key for storing the allocated IPv4 +// address for the given container ID. +func SubnetKeyIPv4(containerID string) string { + return fmt.Sprintf("%s.%s", AnnotationAllocatedSubnetIPv4, truncate(containerID)) +} + +// NetNSKey returns the annotation key for storing the network namespace path +// used by the given container ID. +func NetNSKey(containerID string) string { + return fmt.Sprintf("%s.%s", AnnotationNetNS, truncate(containerID)) +} + +// BGPVRFInstanceName returns the deterministic name for a BGPVRFInstance. +// Each VPCAttachment is unique per interface across the cluster, so the +// (vpc, vpcAttachment) pair is a reliable 1:1 key. +func BGPVRFInstanceName(vpc, vpcAttachment string) string { + return fmt.Sprintf("%s-%s", vpc, vpcAttachment) +} + +// BGPAdvertisementName returns the deterministic name for a +// BGPAdvertisement. Each VPCAttachment is unique per interface across the +// cluster, so the (vpc, vpcAttachment) pair is a reliable 1:1 key. +func BGPAdvertisementName(vpc, vpcAttachment string) string { + return fmt.Sprintf("%s-%s", vpc, vpcAttachment) +} diff --git a/internal/cni/crdnames/crdnames_test.go b/internal/cni/crdnames/crdnames_test.go new file mode 100644 index 00000000..81db39ce --- /dev/null +++ b/internal/cni/crdnames/crdnames_test.go @@ -0,0 +1,74 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package crdnames + +import ( + "strings" + "testing" +) + +func TestBGPVRFInstanceName(t *testing.T) { + tests := []struct{ vpc, attachment, want string }{ + {"abc", "def", "abc-def"}, + {"0000000jU", "00G", "0000000jU-00G"}, + } + for _, tt := range tests { + got := BGPVRFInstanceName(tt.vpc, tt.attachment) + if got != tt.want { + t.Errorf("BGPVRFInstanceName(%q, %q) = %q, want %q", tt.vpc, tt.attachment, got, tt.want) + } + } +} + +func TestBGPAdvertisementName(t *testing.T) { + tests := []struct{ vpc, attachment, want string }{ + {"abc", "def", "abc-def"}, + {"0000000jU", "00G", "0000000jU-00G"}, + } + for _, tt := range tests { + got := BGPAdvertisementName(tt.vpc, tt.attachment) + if got != tt.want { + t.Errorf("BGPAdvertisementName(%q, %q) = %q, want %q", tt.vpc, tt.attachment, got, tt.want) + } + } +} + +// TestAnnotationKeyNameLength verifies that every annotation key builder +// stays within Kubernetes' 63-byte limit on the "name" part of an +// annotation key (the segment after the last "/"), using a realistic +// 64-character container ID (containerd/Docker use full SHA256 hex +// digests). This guards against a real production incident: +// containerIDLen was sized for the old "allocated-subnet." prefix (17 +// bytes) and wasn't updated when the prefix grew by 5 bytes to +// "allocated-subnet-ipv6."/"-ipv4." — every BGPAdvertisement apply failed +// with "name part must be no more than 63 bytes" until fixed. +func TestAnnotationKeyNameLength(t *testing.T) { + const maxAnnotationNameLen = 63 + // A realistic full-length container ID (64 hex chars, as containerd/Docker use). + fullContainerID := strings.Repeat("a", 64) + + tests := []struct { + name string + key string + }{ + {"SubnetKeyIPv6", SubnetKeyIPv6(fullContainerID)}, + {"SubnetKeyIPv4", SubnetKeyIPv4(fullContainerID)}, + {"NetNSKey", NetNSKey(fullContainerID)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + slash := strings.LastIndex(tt.key, "/") + namePart := tt.key + if slash != -1 { + namePart = tt.key[slash+1:] + } + if len(namePart) > maxAnnotationNameLen { + t.Errorf("%s(%d-char containerID) name part %q is %d bytes, want <= %d", + tt.name, len(fullContainerID), namePart, len(namePart), maxAnnotationNameLen) + } + }) + } +} diff --git a/internal/cni/hostconf/hostconf.go b/internal/cni/hostconf/hostconf.go new file mode 100644 index 00000000..59eaa1c8 --- /dev/null +++ b/internal/cni/hostconf/hostconf.go @@ -0,0 +1,154 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +// Package hostconf reads node-local settings (node name, kubeconfig, +// namespace, log file/level) from the static per-node CNI conflist written +// once by internal/installer.Bootstrap. Every binary in the galactic CNI +// plugin chain needs this same lookup, so it lives here rather than being +// duplicated per binary — internal/cni and internal/installer both used to +// carry their own near-identical copy, hardcoded to match a single plugin +// type ("galactic-cni"). +// +// The static conflist is not the per-attachment chain conflist the CNI +// runtime execs each plugin with (that one carries vpc/vpcattachment and is +// templated per VPCAttachment by the external companion operator) — it +// exists solely so any binary in the chain can find node-level settings by +// reading a well-known path off disk, independent of how it was actually +// invoked. Bootstrap only ever writes one entry, typed PluginType, so every +// caller in this repo passes that same constant. +package hostconf + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "os" + + "github.com/vishvananda/netlink" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// PluginType is the "type" value Bootstrap always writes into the static +// conflist's single plugin entry, regardless of which binary is actually +// reading it. Every caller in the chain passes this to Load. +const PluginType = "galactic-cni" + +// HostConf holds node-local settings read from the static per-node conflist +// (default /etc/cni/net.d/10-galactic.conflist). +type HostConf struct { + NodeName string `json:"node_name"` + Kubeconfig string `json:"kubeconfig"` + Namespace string `json:"namespace"` + LogFile string `json:"log_file"` + LogLevel string `json:"log_level,omitempty"` +} + +// conflistEnvelope matches standard CNI conflist JSON structure. +type conflistEnvelope struct { + CNIVersion string `json:"cniVersion"` + Name string `json:"name"` + Plugins []json.RawMessage `json:"plugins"` +} + +// Load reads and parses the conflist at filePath and returns the HostConf +// carried by whichever plugin entry's "type" matches one of acceptedTypes. +// Returns an error wrapping fs.ErrNotExist (checkable via errors.Is) when +// filePath does not exist, so tolerant callers can fall back to defaults. +func Load(filePath string, acceptedTypes ...string) (*HostConf, error) { + data, err := os.ReadFile(filePath) + if err != nil { + return nil, fmt.Errorf("read conflist file %q: %w", filePath, err) + } + + var env conflistEnvelope + if err := json.Unmarshal(data, &env); err != nil { + return nil, fmt.Errorf("parse conflist envelope: %w", err) + } + + accepted := make(map[string]bool, len(acceptedTypes)) + for _, t := range acceptedTypes { + accepted[t] = true + } + + for _, raw := range env.Plugins { + var meta struct { + Type string `json:"type"` + } + if err := json.Unmarshal(raw, &meta); err != nil { + continue + } + if accepted[meta.Type] { + var conf HostConf + if err := json.Unmarshal(raw, &conf); err != nil { + return nil, fmt.Errorf("parse host CNI config: %w", err) + } + return &conf, nil + } + } + + return nil, fmt.Errorf("conflist at %q does not contain a plugin with type in %v", filePath, acceptedTypes) +} + +// detectScheme returns a minimal scheme containing only corev1 types needed +// for node name detection. +func detectScheme() *runtime.Scheme { + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + return scheme +} + +// DetectNodeNameFromAPI queries the Kubernetes API and matches the node's +// InternalIP addresses against local interface addresses. Returns the first +// matching node name, or empty string with no error if detection fails +// (allowing callers to fall through to other resolution methods). Used as a +// fallback by any binary's config resolution when the static conflist is +// missing or doesn't carry a node name (e.g. hostPath mount issues in +// container-based test environments like Kind). +func DetectNodeNameFromAPI() (string, error) { + restCfg, err := ctrl.GetConfig() + if err != nil { + return "", fmt.Errorf("get kubeconfig: %w", err) + } + + k8sClient, err := client.New(restCfg, client.Options{ + Scheme: detectScheme(), + }) + if err != nil { + return "", fmt.Errorf("create k8s client: %w", err) + } + + var nodeList corev1.NodeList + if err := k8sClient.List(context.Background(), &nodeList, &client.ListOptions{ + Limit: 1000, + }); err != nil { + return "", fmt.Errorf("list nodes: %w", err) + } + + addrs, err := netlink.AddrList(nil, netlink.FAMILY_ALL) + if err != nil { + return "", fmt.Errorf("list local addresses: %w", err) + } + + localIPs := make(map[string]bool, len(addrs)) + for _, addr := range addrs { + localIPs[addr.IP.String()] = true + } + + for _, node := range nodeList.Items { + for _, addr := range node.Status.Addresses { + if addr.Type == corev1.NodeInternalIP && localIPs[addr.Address] { + slog.Info("Auto-detected node name from Kubernetes API", + "nodeName", node.Name, "matchedIP", addr.Address) + return node.Name, nil + } + } + } + + return "", errors.New("no local interface address matched any node InternalIP") +} diff --git a/internal/cni/hostconf/hostconf_test.go b/internal/cni/hostconf/hostconf_test.go new file mode 100644 index 00000000..d2b59b04 --- /dev/null +++ b/internal/cni/hostconf/hostconf_test.go @@ -0,0 +1,95 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package hostconf + +import ( + "errors" + "os" + "path/filepath" + "testing" +) + +func TestLoadMissingFile(t *testing.T) { + tmpDir := t.TempDir() + _, err := Load(filepath.Join(tmpDir, "does-not-exist.conflist"), PluginType) + if err == nil { + t.Fatal("expected error for missing file, got nil") + } + if !errors.Is(err, os.ErrNotExist) { + t.Errorf("expected error wrapping os.ErrNotExist, got %v", err) + } +} + +func TestLoadNoMatchingPluginType(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "10-galactic.conflist") + content := `{"cniVersion":"1.0.0","name":"test","plugins":[{"type":"some-other-plugin"}]}` + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("os.WriteFile: %v", err) + } + + if _, err := Load(path, PluginType); err == nil { + t.Fatal("expected error for missing plugin type, got nil") + } +} + +func TestLoadMatchingPluginType(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "10-galactic.conflist") + content := `{ + "cniVersion": "1.0.0", + "name": "galactic", + "plugins": [ + { + "type": "galactic-cni", + "node_name": "test-worker", + "kubeconfig": "/etc/custom-kubeconfig", + "namespace": "custom-namespace", + "log_file": "/var/log/custom.log", + "log_level": "debug" + } + ] + }` + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("os.WriteFile: %v", err) + } + + conf, err := Load(path, PluginType) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if conf.NodeName != "test-worker" { + t.Errorf("NodeName = %q, want %q", conf.NodeName, "test-worker") + } + if conf.Kubeconfig != "/etc/custom-kubeconfig" { + t.Errorf("Kubeconfig = %q, want %q", conf.Kubeconfig, "/etc/custom-kubeconfig") + } + if conf.Namespace != "custom-namespace" { + t.Errorf("Namespace = %q, want %q", conf.Namespace, "custom-namespace") + } + if conf.LogFile != "/var/log/custom.log" { + t.Errorf("LogFile = %q, want %q", conf.LogFile, "/var/log/custom.log") + } + if conf.LogLevel != "debug" { + t.Errorf("LogLevel = %q, want %q", conf.LogLevel, "debug") + } +} + +func TestLoadAcceptsAnyOfMultipleTypes(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "10-galactic.conflist") + content := `{"cniVersion":"1.0.0","name":"test","plugins":[{"type":"galactic-tap-cni","node_name":"tap-node"}]}` + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("os.WriteFile: %v", err) + } + + conf, err := Load(path, "galactic-cni", "galactic-tap-cni") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if conf.NodeName != "tap-node" { + t.Errorf("NodeName = %q, want %q", conf.NodeName, "tap-node") + } +} diff --git a/internal/cni/ipam_ops.go b/internal/cni/ipam_ops.go deleted file mode 100644 index ac76e05d..00000000 --- a/internal/cni/ipam_ops.go +++ /dev/null @@ -1,220 +0,0 @@ -// Copyright 2025 Datum Cloud, Inc. -// -// SPDX-License-Identifier: AGPL-3.0-or-later - -package cni - -import ( - "context" - "errors" - "fmt" - "log/slog" - "net" - - "github.com/containernetworking/cni/pkg/skel" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - - "go.datum.net/galactic/internal/cni/ipam" - bgpv1alpha1 "go.datum.net/network/api/v1alpha1" -) - -// ipv4LockDir is the IPv4PoolAllocator lock/state directory used by -// allocatePoolIPAM/deallocateIPAM. Overridable in tests (mirrors the -// ConfFile pattern in config.go) so unit tests never touch the real -// production path. -var ipv4LockDir = ipam.DefaultIPv4LockDir - -// wantsIPAM reports whether the given config should trigger IPAM allocation -// at all. Four independent signals opt in: an explicit "static" IPAM type, a -// configured IPv6Subnet or IPv4Subnet (the NAD-driven pool-IPAM path, either -// family alone or both), or the --enable-local-ipam dev fallback. A config -// with none of these (e.g. a tap workload that manages its own addressing) -// allocates nothing, matching today's behavior of skipping IPAM entirely -// rather than erroring. -func wantsIPAM(pluginConf *PluginConf) bool { - if pluginConf.IPAM != nil && pluginConf.IPAM.Type == ipamTypeStatic { - return true - } - return pluginConf.IPv6Subnet != "" || pluginConf.IPv4Subnet != "" || enableLocalIPAM -} - -// allocateIPAM allocates addresses for the given container. This is -// interface-agnostic — it does not touch any kernel state or network -// namespaces. Returns (nil, nil) when wantsIPAM reports no allocation is -// requested. When enableLocalIPAM is true and IPv6Subnet is unset, falls back -// to a built-in default IPv6 pool CIDR. -func allocateIPAM(args *skel.CmdArgs, pluginConf *PluginConf) (*ipamResult, error) { - if !wantsIPAM(pluginConf) { - return nil, nil - } - - if pluginConf.IPAM != nil && pluginConf.IPAM.Type == ipamTypeStatic { - return allocateStaticIPAM(args, pluginConf.IPAM) - } - - return allocatePoolIPAM(args, pluginConf) -} - -// allocateStaticIPAM validates and returns the pre-assigned static IPv6 -// address from the "static" IPAM block. No IPv4 address is ever allocated -// for static IPAM — it is a single fixed address, not a dual-stack pool. -func allocateStaticIPAM(args *skel.CmdArgs, ipamConf *IPAM) (*ipamResult, error) { - alloc := ipam.NewStaticAllocator() - allocIP, err := alloc.Allocate(args.ContainerID, ipamConf.StaticIP) - if err != nil { - return nil, fmt.Errorf("allocate static IP: %w", err) - } - subnet := &net.IPNet{ - IP: allocIP, - Mask: net.CIDRMask(64, 128), - } - slog.Debug("IPAM: allocated static", "containerID", args.ContainerID, "subnet", subnet) - return &ipamResult{ipv6Subnet: subnet}, nil -} - -// allocatePoolIPAM allocates a dual-stack, IPv6-only, or IPv4-only pool-based -// endpoint address for the given container, via ipam.DualStackAllocator. -// IPv6Subnet and IPv4Subnet each independently supply a pool CIDR for their -// family; at least one must be set (falling back to localIPAMDefaultPool for -// IPv6 when enableLocalIPAM and both are unset). -func allocatePoolIPAM(args *skel.CmdArgs, pluginConf *PluginConf) (*ipamResult, error) { - ipv6Pool := pluginConf.IPv6Subnet - if ipv6Pool == "" && pluginConf.IPv4Subnet == "" { - if !enableLocalIPAM { - return nil, errors.New("ipv6_subnet or ipv4_subnet is required (or enable local IPAM)") - } - ipv6Pool = localIPAMDefaultPool - } - - alloc, err := ipam.NewDualStackAllocator(ipv6Pool, "", pluginConf.IPv4Subnet, "", ipv4LockDir) - if err != nil { - return nil, fmt.Errorf("create dual-stack allocator: %w", err) - } - - res, err := alloc.Allocate(args.ContainerID) - if err != nil { - return nil, fmt.Errorf("allocate dual-stack addresses: %w", err) - } - - var routes []*net.IPNet - if res.IPv6Subnet != nil { - routes = append(routes, &net.IPNet{IP: net.IPv6zero, Mask: net.CIDRMask(0, 128)}) - } - if res.IPv4Address != nil { - routes = append(routes, &net.IPNet{IP: net.IPv4zero, Mask: net.CIDRMask(0, 32)}) - } - - slog.Debug("IPAM: allocated", "containerID", args.ContainerID, - "ipv6Subnet", res.IPv6Subnet, "ipv6Gateway", res.IPv6Gateway, - "ipv4Address", res.IPv4Address, "ipv4Gateway", res.IPv4Gateway) - - return &ipamResult{ - ipv6Subnet: res.IPv6Subnet, - ipv6Gateway: res.IPv6Gateway, - ipv4Address: res.IPv4Address, - ipv4Gateway: res.IPv4Gateway, - routes: routes, - }, nil -} - -// configureIPAM allocates addresses and configures the guest interface inside -// the container network namespace with both families (when dual-stack). This -// is veth-only; for tap mode, use allocateIPAM directly (the VM manages its -// own guest interface). -func configureIPAM(args *skel.CmdArgs, pluginConf *PluginConf, guestName string) (*ipamResult, error) { - ipamResult, err := allocateIPAM(args, pluginConf) - if err != nil { - return nil, err - } - if ipamResult == nil { - return nil, nil - } - - var ipv4Net *net.IPNet - if ipamResult.ipv4Address != nil { - ipv4Net = &net.IPNet{IP: ipamResult.ipv4Address, Mask: net.CIDRMask(32, 32)} - } - if err := configureInterfaceInNetns( - args.Netns, guestName, - ipamResult.ipv6Subnet, ipamResult.ipv6Gateway, - ipv4Net, ipamResult.ipv4Gateway, - ); err != nil { - return nil, err - } - - return ipamResult, nil -} - -// deallocateIPAM releases the IPAM allocation for the given container. -// Reads the allocated IPv6 subnet and (if present) IPv4 address from the -// BGPAdvertisement CRD annotations, then deallocates each independently and -// non-fatally: a missing annotation for one family (e.g. a pre-existing -// v6-only pod, or a partial ADD failure that never reached IPv4 allocation) -// must not prevent cleanup of the other. -func deallocateIPAM(args *skel.CmdArgs, pluginConf *PluginConf, k8s client.Client) { - if pluginConf.IPAM != nil && pluginConf.IPAM.Type == ipamTypeStatic { - // Static allocations don't need deallocation. - return - } - - ipv6Subnet, ipv4Addr := getAllocatedSubnetsFromCRD(args.ContainerID, pluginConf, k8s) - if ipv6Subnet == "" && ipv4Addr == "" { - // No allocation found — either allocation was never completed, - // or the advertisement was already deleted. Nothing to clean up. - slog.Debug("IPAM: no allocation found to deallocate", "containerID", args.ContainerID) - return - } - - if ipv6Subnet != "" { - ipv6Pool := pluginConf.IPv6Subnet - if ipv6Pool == "" && enableLocalIPAM { - ipv6Pool = localIPAMDefaultPool - } - pa, err := ipam.NewPoolAllocator(ipv6Pool, "", 0) - if err != nil { - slog.Warn("IPAM: failed to build IPv6 pool allocator for deallocation, skipping", "err", err, - "containerID", args.ContainerID, "subnet", ipv6Subnet) - } else { - pa.Deallocate(ipv6Subnet) - slog.Debug("IPAM: deallocated IPv6", "containerID", args.ContainerID, "subnet", ipv6Subnet) - } - } - - if ipv4Addr != "" { - if pluginConf.IPv4Subnet == "" { - slog.Warn("IPAM: found allocated IPv4 address but no ipv4_subnet in config, skipping deallocation", - "containerID", args.ContainerID, "address", ipv4Addr) - } else if pa, err := ipam.NewIPv4PoolAllocator(pluginConf.IPv4Subnet, "", ipv4LockDir); err != nil { - slog.Warn("IPAM: failed to build IPv4 pool allocator for deallocation, skipping", "err", err, - "containerID", args.ContainerID, "address", ipv4Addr) - } else { - pa.Deallocate(ipv4Addr) - slog.Debug("IPAM: deallocated IPv4", "containerID", args.ContainerID, "address", ipv4Addr) - } - } -} - -// getAllocatedSubnetsFromCRD reads the allocated IPv6 subnet and (if present) -// IPv4 address for the given container from the BGPAdvertisement CRD -// annotations. Either return value is empty when not found. -func getAllocatedSubnetsFromCRD( - containerID string, pluginConf *PluginConf, k8s client.Client, -) (ipv6Subnet, ipv4Addr string) { - namespace := pluginConf.Namespace - - ctx, cancel := context.WithTimeout(context.Background(), cniTimeout) - defer cancel() - - adv := &bgpv1alpha1.BGPAdvertisement{ - ObjectMeta: metav1.ObjectMeta{ - Name: bgpAdvertisementName(pluginConf.VPC, pluginConf.VPCAttachment), - Namespace: namespace, - }, - } - if err := k8s.Get(ctx, client.ObjectKeyFromObject(adv), adv); err != nil { - return "", "" - } - - return adv.Annotations[subnetAnnotationKeyIPv6(containerID)], adv.Annotations[subnetAnnotationKeyIPv4(containerID)] -} diff --git a/internal/cni/ipam_ops_test.go b/internal/cni/ipam_ops_test.go deleted file mode 100644 index fc31bcda..00000000 --- a/internal/cni/ipam_ops_test.go +++ /dev/null @@ -1,361 +0,0 @@ -// Copyright 2025 Datum Cloud, Inc. -// -// SPDX-License-Identifier: AGPL-3.0-or-later - -package cni - -import ( - "net" - "testing" - - "github.com/containernetworking/cni/pkg/skel" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "go.datum.net/galactic/internal/cni/ipam" - "go.datum.net/galactic/internal/config" - bgpv1alpha1 "go.datum.net/network/api/v1alpha1" -) - -// ---- wantsIPAM ------------------------------------------------------------ - -func TestWantsIPAM(t *testing.T) { - original := enableLocalIPAM - defer func() { enableLocalIPAM = original }() - - tests := []struct { - name string - pluginConf *PluginConf - enableLocalIPA bool - want bool - }{ - { - name: "no ipam block, no ipv6_subnet, local IPAM disabled", - pluginConf: &PluginConf{}, - want: false, - }, - { - name: "static ipam type opts in regardless of other fields", - pluginConf: &PluginConf{IPAM: &IPAM{Type: ipamTypeStatic}}, - want: true, - }, - { - name: "ipv6_subnet set opts in", - pluginConf: &PluginConf{IPv6Subnet: localIPAMDefaultPool}, - want: true, - }, - { - name: "ipv4_subnet set opts in", - pluginConf: &PluginConf{IPv4Subnet: testIPv4Subnet}, - want: true, - }, - { - name: "local IPAM enabled opts in even without ipv6_subnet", - pluginConf: &PluginConf{}, - enableLocalIPA: true, - want: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - enableLocalIPAM = tt.enableLocalIPA - if got := wantsIPAM(tt.pluginConf); got != tt.want { - t.Errorf("wantsIPAM(%+v) = %v, want %v", tt.pluginConf, got, tt.want) - } - }) - } -} - -// ---- allocateIPAM ---------------------------------------------------------- - -func TestAllocateIPAMNoAllocation(t *testing.T) { - original := enableLocalIPAM - defer func() { enableLocalIPAM = original }() - enableLocalIPAM = false - - args := &skel.CmdArgs{ContainerID: testContainerID} - res, err := allocateIPAM(args, &PluginConf{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if res != nil { - t.Errorf("allocateIPAM() = %+v, want nil (wantsIPAM should have been false)", res) - } -} - -func TestAllocateIPAMStatic(t *testing.T) { - args := &skel.CmdArgs{ContainerID: testContainerID} - pluginConf := &PluginConf{ - IPAM: &IPAM{Type: ipamTypeStatic, StaticIP: "fd00:10:ff01::1234"}, - } - - res, err := allocateIPAM(args, pluginConf) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if res == nil { - t.Fatal("allocateIPAM() = nil, want a result") - } - if res.ipv6Subnet == nil || !res.ipv6Subnet.IP.Equal(net.ParseIP("fd00:10:ff01::1234")) { - t.Errorf("ipv6Subnet = %v, want fd00:10:ff01::1234", res.ipv6Subnet) - } - if res.ipv4Address != nil { - t.Errorf("ipv4Address = %v, want nil for static IPAM", res.ipv4Address) - } -} - -func TestAllocateIPAMPoolIPv6Only(t *testing.T) { - args := &skel.CmdArgs{ContainerID: testContainerID} - pluginConf := &PluginConf{IPv6Subnet: localIPAMDefaultPool} - - res, err := allocateIPAM(args, pluginConf) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if res == nil { - t.Fatal("allocateIPAM() = nil, want a result") - } - if res.ipv6Subnet == nil { - t.Fatal("ipv6Subnet = nil, want an allocated /96") - } - if ones, bits := res.ipv6Subnet.Mask.Size(); ones != 96 || bits != 128 { - t.Errorf("ipv6Subnet mask = /%d, want /96", ones) - } - if res.ipv6Gateway == nil { - t.Error("ipv6Gateway = nil, want the pool's default gateway (::1 of the /64)") - } - if res.ipv4Address != nil { - t.Errorf("ipv4Address = %v, want nil (no ipv4_subnet configured)", res.ipv4Address) - } - if len(res.routes) != 1 { - t.Errorf("routes = %v, want exactly one default IPv6 route", res.routes) - } -} - -func TestAllocateIPAMPoolIPv4Only(t *testing.T) { - origLockDir := ipv4LockDir - ipv4LockDir = t.TempDir() - defer func() { ipv4LockDir = origLockDir }() - - args := &skel.CmdArgs{ContainerID: testContainerID} - pluginConf := &PluginConf{IPv4Subnet: testIPv4Subnet} - - res, err := allocateIPAM(args, pluginConf) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if res == nil { - t.Fatal("allocateIPAM() = nil, want a result") - } - if res.ipv6Subnet != nil { - t.Errorf("ipv6Subnet = %v, want nil (no ipv6_subnet configured)", res.ipv6Subnet) - } - if res.ipv4Address == nil { - t.Fatal("ipv4Address = nil, want an allocated /32") - } - if res.ipv4Gateway == nil { - t.Error("ipv4Gateway = nil, want the pool's default gateway") - } - if len(res.routes) != 1 { - t.Errorf("routes = %v, want exactly one default IPv4 route", res.routes) - } -} - -func TestAllocateIPAMPoolDualStack(t *testing.T) { - origLockDir := ipv4LockDir - ipv4LockDir = t.TempDir() - defer func() { ipv4LockDir = origLockDir }() - - args := &skel.CmdArgs{ContainerID: testContainerID} - pluginConf := &PluginConf{ - IPv6Subnet: localIPAMDefaultPool, - IPv4Subnet: testIPv4Subnet, - } - - res, err := allocateIPAM(args, pluginConf) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if res == nil { - t.Fatal("allocateIPAM() = nil, want a result") - } - if res.ipv6Subnet == nil { - t.Error("ipv6Subnet = nil, want an allocated /96") - } - if res.ipv4Address == nil { - t.Fatal("ipv4Address = nil, want an allocated /32") - } - if res.ipv4Gateway == nil { - t.Error("ipv4Gateway = nil, want the pool's default gateway") - } - if len(res.routes) != 2 { - t.Errorf("routes = %v, want one default route per family", res.routes) - } -} - -func TestAllocateIPAMPoolMissingBothSubnetsErrors(t *testing.T) { - original := enableLocalIPAM - defer func() { enableLocalIPAM = original }() - enableLocalIPAM = false - - // wantsIPAM only opts in here via the static/ipv6_subnet/ipv4_subnet/ - // local-IPAM signals, so force the pool path directly to exercise the - // "no source for any pool CIDR" error without relying on wantsIPAM's - // gating. - args := &skel.CmdArgs{ContainerID: testContainerID} - _, err := allocatePoolIPAM(args, &PluginConf{}) - if err == nil { - t.Fatal("expected error when both ipv6_subnet and ipv4_subnet are unset and local IPAM is disabled, got nil") - } -} - -// ---- deallocateIPAM --------------------------------------------------------- - -func TestDeallocateIPAMStaticNoop(t *testing.T) { - // Static IPAM never wrote a CRD annotation for cmdDel to look up, and - // deallocateIPAM must return immediately without attempting a k8s lookup. - pluginConf := &PluginConf{ - VPC: testVPC, VPCAttachment: testAttachment, - IPAM: &IPAM{Type: ipamTypeStatic}, - } - args := &skel.CmdArgs{ContainerID: testContainerID} - // A nil client would panic if deallocateIPAM tried to use it; passing nil - // here asserts the static-type early return happens first. - deallocateIPAM(args, pluginConf, nil) -} - -func TestDeallocateIPAMDualStack(t *testing.T) { - origLockDir := ipv4LockDir - ipv4LockDir = t.TempDir() - defer func() { ipv4LockDir = origLockDir }() - - pluginConf := &PluginConf{ - VPC: testVPC, VPCAttachment: testAttachment, - IPv6Subnet: localIPAMDefaultPool, - IPv4Subnet: testIPv4Subnet, - } - args := &skel.CmdArgs{ContainerID: testContainerID} - - // Allocate an IPv4 address the same way ADD would, so there's a real - // marker file for Deallocate to remove. - alloc, err := ipam.NewDualStackAllocator(pluginConf.IPv6Subnet, "", pluginConf.IPv4Subnet, "", ipv4LockDir) - if err != nil { - t.Fatalf("NewDualStackAllocator: %v", err) - } - dsRes, err := alloc.Allocate(args.ContainerID) - if err != nil { - t.Fatalf("Allocate: %v", err) - } - - ipv4Pool, err := ipam.NewIPv4PoolAllocator(pluginConf.IPv4Subnet, "", ipv4LockDir) - if err != nil { - t.Fatalf("NewIPv4PoolAllocator: %v", err) - } - if !ipv4Pool.IsAllocated(dsRes.IPv4Address.String()) { - t.Fatalf("setup: IPv4 address %s not marked allocated", dsRes.IPv4Address) - } - - adv := &bgpv1alpha1.BGPAdvertisement{ - ObjectMeta: metav1.ObjectMeta{ - Name: bgpAdvertisementName(pluginConf.VPC, pluginConf.VPCAttachment), - Namespace: config.DefaultNamespace, - Annotations: map[string]string{ - subnetAnnotationKeyIPv6(args.ContainerID): dsRes.IPv6Subnet.String(), - subnetAnnotationKeyIPv4(args.ContainerID): dsRes.IPv4Address.String(), - }, - }, - } - pluginConf.Namespace = config.DefaultNamespace - k8s := fakeClient(adv) - - deallocateIPAM(args, pluginConf, k8s) - - if ipv4Pool.IsAllocated(dsRes.IPv4Address.String()) { - t.Errorf("IPv4 address %s still marked allocated after deallocateIPAM", dsRes.IPv4Address) - } -} - -func TestDeallocateIPAMIPv4Only(t *testing.T) { - origLockDir := ipv4LockDir - ipv4LockDir = t.TempDir() - defer func() { ipv4LockDir = origLockDir }() - - pluginConf := &PluginConf{ - VPC: testVPC, VPCAttachment: testAttachment, - Namespace: config.DefaultNamespace, - IPv4Subnet: testIPv4Subnet, - } - args := &skel.CmdArgs{ContainerID: testContainerID} - - ipv4Pool, err := ipam.NewIPv4PoolAllocator(pluginConf.IPv4Subnet, "", ipv4LockDir) - if err != nil { - t.Fatalf("NewIPv4PoolAllocator: %v", err) - } - ipv4Addr, err := ipv4Pool.Allocate(args.ContainerID) - if err != nil { - t.Fatalf("Allocate: %v", err) - } - if !ipv4Pool.IsAllocated(ipv4Addr.String()) { - t.Fatalf("setup: IPv4 address %s not marked allocated", ipv4Addr) - } - - adv := &bgpv1alpha1.BGPAdvertisement{ - ObjectMeta: metav1.ObjectMeta{ - Name: bgpAdvertisementName(pluginConf.VPC, pluginConf.VPCAttachment), - Namespace: config.DefaultNamespace, - Annotations: map[string]string{ - // No IPv6 annotation — this is an IPv4-only allocation. - subnetAnnotationKeyIPv4(args.ContainerID): ipv4Addr.String(), - }, - }, - } - k8s := fakeClient(adv) - - // Must not panic despite no ipv6_subnet in config, and must deallocate - // the IPv4 address. - deallocateIPAM(args, pluginConf, k8s) - - if ipv4Pool.IsAllocated(ipv4Addr.String()) { - t.Errorf("IPv4 address %s still marked allocated after deallocateIPAM", ipv4Addr) - } -} - -func TestDeallocateIPAMPartialAllocationNonFatal(t *testing.T) { - // A v6-only pod (no IPv4 annotation, e.g. pre-existing or a partial ADD - // failure) must still have its IPv6 side cleaned up without erroring, - // and must not attempt to touch a nonexistent IPv4 pool. - pluginConf := &PluginConf{ - VPC: testVPC, VPCAttachment: testAttachment, - Namespace: config.DefaultNamespace, - IPv6Subnet: localIPAMDefaultPool, - // IPv4Subnet intentionally unset. - } - args := &skel.CmdArgs{ContainerID: testContainerID} - - adv := &bgpv1alpha1.BGPAdvertisement{ - ObjectMeta: metav1.ObjectMeta{ - Name: bgpAdvertisementName(pluginConf.VPC, pluginConf.VPCAttachment), - Namespace: config.DefaultNamespace, - Annotations: map[string]string{ - subnetAnnotationKeyIPv6(args.ContainerID): "fd00:10:ff01::1234/96", - }, - }, - } - k8s := fakeClient(adv) - - // Must not panic despite no ipv4_subnet in config. - deallocateIPAM(args, pluginConf, k8s) -} - -func TestDeallocateIPAMNoAllocationFound(t *testing.T) { - pluginConf := &PluginConf{ - VPC: testVPC, VPCAttachment: testAttachment, - Namespace: config.DefaultNamespace, - } - args := &skel.CmdArgs{ContainerID: testContainerID} - // No BGPAdvertisement exists at all. - k8s := fakeClient() - - // Must return cleanly with nothing to deallocate. - deallocateIPAM(args, pluginConf, k8s) -} diff --git a/internal/cni/nad.go b/internal/cni/nadpatch/nadpatch.go similarity index 62% rename from internal/cni/nad.go rename to internal/cni/nadpatch/nadpatch.go index bdf4ee8a..8a10699a 100644 --- a/internal/cni/nad.go +++ b/internal/cni/nadpatch/nadpatch.go @@ -2,7 +2,11 @@ // // SPDX-License-Identifier: AGPL-3.0-or-later -package cni +// Package nadpatch patches the NetworkAttachmentDefinition with the +// deterministic host-side interface name a master plugin (galactic-cni, +// galactic-tap-cni) just created — shared since NAD annotation is identical +// regardless of interface type. +package nadpatch import ( "context" @@ -17,10 +21,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// annotationHostInterface is the NAD annotation key that records the +// AnnotationHostInterface is the NAD annotation key that records the // deterministic host-side interface name created by the CNI plugin for // this VPC+VPCAttachment pair. -const annotationHostInterface = "k8s.v1.cni.cncf.io/host-interface" +const AnnotationHostInterface = "k8s.v1.cni.cncf.io/host-interface" // nadGVK is the GroupVersionKind for NetworkAttachmentDefinition. var nadGVK = schema.GroupVersionKind{ @@ -29,10 +33,11 @@ var nadGVK = schema.GroupVersionKind{ Kind: "NetworkAttachmentDefinition", } -// parsePodNamespace extracts the K8S_POD_NAMESPACE value from the CNI_ARGS -// environment variable string passed as args.Args by Multus. Returns an empty -// string when the value is not present (e.g. standalone CNI invocation). -func parsePodNamespace(cniArgs string) string { +// ParsePodNamespace extracts the K8S_POD_NAMESPACE value from the CNI_ARGS +// environment variable string passed as args.Args by Multus. Returns an +// empty string when the value is not present (e.g. standalone CNI +// invocation). +func ParsePodNamespace(cniArgs string) string { for _, part := range strings.Split(cniArgs, ";") { key, value, ok := strings.Cut(part, "=") if ok && key == "K8S_POD_NAMESPACE" { @@ -42,13 +47,13 @@ func parsePodNamespace(cniArgs string) string { return "" } -// annotateNAD patches the NetworkAttachmentDefinition with the host interface -// name. The NAD is expected to already exist (created by the external VPC -// operator before the CNI is invoked), so a not-found response is a hard -// failure rather than something to tolerate. A conflict response is the one -// case treated as non-fatal: it means the annotation was already applied by a -// previous invocation. -func annotateNAD(ctx context.Context, k8s client.Client, nadName, nadNamespace, hostInterface string) error { +// AnnotateNAD patches the NetworkAttachmentDefinition with the host +// interface name. The NAD is expected to already exist (created by the +// external VPC operator before the CNI is invoked), so a not-found response +// is a hard failure rather than something to tolerate. A conflict response +// is the one case treated as non-fatal: it means the annotation was already +// applied by a previous invocation. +func AnnotateNAD(ctx context.Context, k8s client.Client, nadName, nadNamespace, hostInterface string) error { if nadNamespace == "" { return nil } @@ -59,7 +64,7 @@ func annotateNAD(ctx context.Context, k8s client.Client, nadName, nadNamespace, nad.SetNamespace(nadNamespace) patch := fmt.Sprintf(`[{"op":"add","path":"/metadata/annotations","value":{"%s":"%s"}}]`, - annotationHostInterface, hostInterface) + AnnotationHostInterface, hostInterface) err := k8s.Patch(ctx, nad, client.RawPatch(types.JSONPatchType, []byte(patch))) if err != nil { diff --git a/internal/cni/nad_test.go b/internal/cni/nadpatch/nadpatch_test.go similarity index 65% rename from internal/cni/nad_test.go rename to internal/cni/nadpatch/nadpatch_test.go index 7bff7566..461f9207 100644 --- a/internal/cni/nad_test.go +++ b/internal/cni/nadpatch/nadpatch_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0-or-later -package cni +package nadpatch import ( "context" @@ -10,25 +10,23 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +func fakeClient(objs ...client.Object) client.Client { + return fake.NewClientBuilder().WithScheme(runtime.NewScheme()).WithObjects(objs...).Build() +} + func TestParsePodNamespace(t *testing.T) { tests := []struct { name string cniArgs string expected string }{ - { - name: "empty string", - cniArgs: "", - expected: "", - }, - { - name: "namespace only", - cniArgs: "K8S_POD_NAMESPACE=default", - expected: "default", - }, + {name: "empty string", cniArgs: "", expected: ""}, + {name: "namespace only", cniArgs: "K8S_POD_NAMESPACE=default", expected: "default"}, { name: "full multus args", cniArgs: "K8S_POD_NAME=my-pod;K8S_POD_NAMESPACE=galactic-system;K8S_POD_INFRA_CONTAINER_ID=abc123", @@ -39,18 +37,14 @@ func TestParsePodNamespace(t *testing.T) { cniArgs: "K8S_POD_NAME=my-pod;K8S_POD_INFRA_CONTAINER_ID=abc123", expected: "", }, - { - name: "namespace with hyphens", - cniArgs: "K8S_POD_NAMESPACE=my-custom-namespace", - expected: "my-custom-namespace", - }, + {name: "namespace with hyphens", cniArgs: "K8S_POD_NAMESPACE=my-custom-namespace", expected: "my-custom-namespace"}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got := parsePodNamespace(tc.cniArgs) + got := ParsePodNamespace(tc.cniArgs) if got != tc.expected { - t.Errorf("parsePodNamespace(%q) = %q, want %q", tc.cniArgs, got, tc.expected) + t.Errorf("ParsePodNamespace(%q) = %q, want %q", tc.cniArgs, got, tc.expected) } }) } @@ -66,7 +60,7 @@ func TestAnnotateNAD(t *testing.T) { t.Run("NAD does not exist is a hard failure", func(t *testing.T) { k8s := fakeClient() - err := annotateNAD(context.Background(), k8s, nadName, nadNamespace, hostIface) + err := AnnotateNAD(context.Background(), k8s, nadName, nadNamespace, hostIface) if err == nil { t.Fatal("expected error when NAD does not exist, got nil") } @@ -82,8 +76,8 @@ func TestAnnotateNAD(t *testing.T) { nad.SetNamespace(nadNamespace) k8s := fakeClient(nad) - if err := annotateNAD(context.Background(), k8s, nadName, nadNamespace, hostIface); err != nil { - t.Fatalf("annotateNAD() = %v, want nil", err) + if err := AnnotateNAD(context.Background(), k8s, nadName, nadNamespace, hostIface); err != nil { + t.Fatalf("AnnotateNAD() = %v, want nil", err) } got := &unstructured.Unstructured{} @@ -91,16 +85,16 @@ func TestAnnotateNAD(t *testing.T) { if err := k8s.Get(context.Background(), client.ObjectKey{Name: nadName, Namespace: nadNamespace}, got); err != nil { t.Fatalf("get NAD after annotate: %v", err) } - if gotAnnotation := got.GetAnnotations()[annotationHostInterface]; gotAnnotation != hostIface { - t.Errorf("annotation %s = %q, want %q", annotationHostInterface, gotAnnotation, hostIface) + if gotAnnotation := got.GetAnnotations()[AnnotationHostInterface]; gotAnnotation != hostIface { + t.Errorf("annotation %s = %q, want %q", AnnotationHostInterface, gotAnnotation, hostIface) } }) t.Run("empty pod namespace is a no-op", func(t *testing.T) { k8s := fakeClient() - if err := annotateNAD(context.Background(), k8s, nadName, "", hostIface); err != nil { - t.Fatalf("annotateNAD() with empty namespace = %v, want nil", err) + if err := AnnotateNAD(context.Background(), k8s, nadName, "", hostIface); err != nil { + t.Fatalf("AnnotateNAD() with empty namespace = %v, want nil", err) } }) } diff --git a/internal/cni/ops_add.go b/internal/cni/ops_add.go index f1b6aa1f..50d8a6ce 100644 --- a/internal/cni/ops_add.go +++ b/internal/cni/ops_add.go @@ -6,19 +6,18 @@ package cni import ( "context" - "errors" "fmt" "log/slog" - "net" "os" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" "github.com/vishvananda/netlink" + "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/cni/veth" + "go.datum.net/galactic/internal/cnibgp" "go.datum.net/galactic/internal/plumbing/intf" "go.datum.net/galactic/internal/plumbing/vrf" ) @@ -57,19 +56,18 @@ func cmdAdd(args *skel.CmdArgs) (err error) { slog.Info("ADD: starting", "containerID", args.ContainerID, "netns", args.Netns, "ifName", args.IfName, "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment, - "interfaceType", pluginConf.InterfaceType, "namespace", namespace, "nodeName", nodeName) + "namespace", namespace, "nodeName", nodeName) // Track resources for selective rollback on failure. tracker := &resourceTracker{ vpc: pluginConf.VPC, vpcAttachment: pluginConf.VPCAttachment, - ifaceType: pluginConf.InterfaceType, namespace: namespace, } // Selective rollback: clean up only resources that were created. // We need a context for k8s operations in rollback; the k8s client - // will be populated by publishBGPState before it's needed. + // will be populated below before it's needed. rollbackCtx, rollbackCancel := context.WithTimeout(context.Background(), cniTimeout) defer func() { if err != nil { @@ -86,16 +84,8 @@ func cmdAdd(args *skel.CmdArgs) (err error) { tracker.vrfCreated = true slog.Debug("ADD: VRF ready", "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment) - // Create the appropriate interface type (veth or tap). - switch pluginConf.InterfaceType { - case interfaceTypeVeth: - if err := veth.Add(pluginConf.VPC, pluginConf.VPCAttachment, pluginConf.MTU); err != nil { - return fmt.Errorf("add veth: %w", err) - } - case interfaceTypeTap: - if err := tap.Add(pluginConf.VPC, pluginConf.VPCAttachment, pluginConf.MTU); err != nil { - return fmt.Errorf("add tap: %w", err) - } + if err := veth.Add(pluginConf.VPC, pluginConf.VPCAttachment, pluginConf.MTU); err != nil { + return fmt.Errorf("add veth: %w", err) } hostName := intf.GenerateInterfaceNameHost(pluginConf.VPC, pluginConf.VPCAttachment) @@ -115,8 +105,8 @@ func cmdAdd(args *skel.CmdArgs) (err error) { return fmt.Errorf("create k8s client: %w", err) } tracker.k8s = k8sClient - podNamespace := parsePodNamespace(args.Args) - if err := annotateNAD(rollbackCtx, k8sClient, pluginConf.Name, podNamespace, hostName); err != nil { + podNamespace := nadpatch.ParsePodNamespace(args.Args) + if err := nadpatch.AnnotateNAD(rollbackCtx, k8sClient, pluginConf.Name, podNamespace, hostName); err != nil { return fmt.Errorf("annotate NAD: %w", err) } @@ -131,63 +121,25 @@ func cmdAdd(args *skel.CmdArgs) (err error) { slog.Debug("ADD: termination routes installed", "count", tracker.routesCreated, "dev", dev) } - // Host-device delegation and IPAM are veth-only. - // In tap mode the guest VM manages its own networking. - var ipamResult *ipamResult - var guestHWAddr net.HardwareAddr - switch pluginConf.InterfaceType { - case interfaceTypeVeth: - 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) - } - case interfaceTypeTap: - // Allocate IPAM for the tap interface (same as veth). - // The VM manages its own guest interface; the CNI only configures the host side. - ipamResult, err = allocateIPAM(args, pluginConf) - if err != nil { - return fmt.Errorf("allocate IPAM: %w", err) - } - if ipamResult != nil { - slog.Debug("ADD: IPAM allocated", "containerID", args.ContainerID, - "ipv6Subnet", ipamResult.ipv6Subnet, "ipv6Gateway", ipamResult.ipv6Gateway, - "ipv4Address", ipamResult.ipv4Address, "ipv4Gateway", ipamResult.ipv4Gateway) - } - - // Configure the gateway address on the host tap and install the VRF route. - if err := configureHostGateway(pluginConf.VPC, pluginConf.VPCAttachment, ipamResult, nil); err != nil { - return err - } - if ipamResult != nil && ipamResult.ipv6Gateway != nil { - slog.Debug("ADD: host gateway configured", "name", hostName, "gateway", ipamResult.ipv6Gateway) - } - - // Print the CNI result with IP info. - result := buildTapResult(pluginConf, ipamResult, hostName, hostMac, hostMTU) - if err := types.PrintResult(result, pluginConf.CNIVersion); err != nil { - return fmt.Errorf("print CNI result: %w", err) - } - - // Decode VPC for BGP state publish. - vpcHex, err := intf.Base62ToHex(pluginConf.VPC) - if err != nil { - return fmt.Errorf("decode VPC: %w", err) - } - - // Publish BGP state (SRv6 ingress + BGP CRDs). - if tracker.k8s == nil { - return errors.New("k8s client not set in tracker") - } - slog.Debug("ADD: publishing BGP state", "containerID", args.ContainerID, "interfaceType", interfaceTypeTap) - return publishBGPStateK8s(args, pluginConf, nodeName, namespace, ipamResult, vpcHex, tracker.k8s, tracker) + 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, "interfaceType", pluginConf.InterfaceType) - return publishBGPState(args, pluginConf, nodeName, namespace, ipamResult, guestHWAddr, tracker) + 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 } diff --git a/internal/cni/ops_check.go b/internal/cni/ops_check.go index 866b27be..adebc672 100644 --- a/internal/cni/ops_check.go +++ b/internal/cni/ops_check.go @@ -38,7 +38,7 @@ func cmdCheck(args *skel.CmdArgs) error { return err } slog.Info("CHECK: starting", "containerID", args.ContainerID, - "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment, "interfaceType", pluginConf.InterfaceType) + "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment) var errs []error @@ -46,17 +46,15 @@ func cmdCheck(args *skel.CmdArgs) error { hostName, nodeErrs := checkNodeLevelState(pluginConf.VPC, pluginConf.VPCAttachment) errs = append(errs, nodeErrs...) - // For veth mode, verify the guest interface is in the container netns. - if pluginConf.InterfaceType == interfaceTypeVeth { - guestName := intf.GenerateInterfaceNameGuest(pluginConf.VPC, pluginConf.VPCAttachment) - if err := checkGuestInterface(args.Netns, guestName); err != nil { - errs = append(errs, fmt.Errorf("guest interface %q: %w", guestName, err)) - } + // Verify the guest interface is in the container netns. + guestName := intf.GenerateInterfaceNameGuest(pluginConf.VPC, pluginConf.VPCAttachment) + if err := checkGuestInterface(args.Netns, guestName); err != nil { + 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)) - } + // 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)) } // Validate kernel state against prevResult (CNI spec §4.3). diff --git a/internal/cni/ops_del.go b/internal/cni/ops_del.go index 8539f9c2..17d719c0 100644 --- a/internal/cni/ops_del.go +++ b/internal/cni/ops_del.go @@ -10,6 +10,8 @@ import ( "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/cniipam" ) func cmdDel(args *skel.CmdArgs) error { @@ -30,10 +32,11 @@ func cmdDel(args *skel.CmdArgs) error { vpc, vpcAtt := pluginConf.VPC, pluginConf.VPCAttachment // Deallocate the pod's IPAM subnet. This is pod-specific and safe to - // release immediately. Applies to both veth and tap modes. - if wantsIPAM(pluginConf) { + // release immediately. + cfg := allocConfig(pluginConf) + if cniipam.WantsIPAM(cfg) { if k8s, err := newK8sClient(); err == nil { - deallocateIPAM(args, pluginConf, k8s) + cniipam.Deallocate(args, cfg, k8s) } else { slog.Warn("DEL: failed to create k8s client, skipping IPAM deallocation", "err", err, "containerID", args.ContainerID) @@ -49,31 +52,25 @@ func cmdDel(args *skel.CmdArgs) error { // Multus secondary attachment) — the move is then a no-op, and the // leftover route survives indefinitely since there's no ephemeral // sandbox netns to reclaim it, wedging the next ADD with "file exists". - // Only applies to veth mode; tap mode has no guest-side netns config. - if pluginConf.InterfaceType == interfaceTypeVeth { - if err := flushGuestNetnsConfig(args.Netns, args.IfName); err != nil { - slog.Warn("DEL: failed to flush guest interface address/route, may still be in the netns", - "err", err, "containerID", args.ContainerID, "netns", args.Netns) - } + if err := flushGuestNetnsConfig(args.Netns, args.IfName); err != nil { + slog.Warn("DEL: failed to flush guest interface address/route, may still be in the netns", + "err", err, "containerID", args.ContainerID, "netns", args.Netns) } // Forward DEL to host-device delegated plugin (CNI spec §4). This moves // the guest veth end back out of the container netns and restores its - // original (host-side) name. Only applies to veth mode; tap mode has no - // host-device delegation. + // original (host-side) name. // // DEL must always return success per the CNI spec, so an error here // (e.g. the device was never moved into the netns because ADD failed // before reaching that step, or the netns is already gone) is logged // rather than propagated. - if pluginConf.InterfaceType == interfaceTypeVeth { - if err := hostDevice("DEL", args, pluginConf); err != nil { - slog.Warn("DEL: host-device DEL failed, guest interface may still be in the netns", - "err", err, "containerID", args.ContainerID, "netns", args.Netns) - } + if err := hostDevice("DEL", args, pluginConf); err != nil { + slog.Warn("DEL: host-device DEL failed, guest interface may still be in the netns", + "err", err, "containerID", args.ContainerID, "netns", args.Netns) } - // Shared resources (VRF, veth/tap, routes, SRv6 ingress, BGPAdvertisement, + // Shared resources (VRF, veth, routes, SRv6 ingress, BGPAdvertisement, // BGPVRFInstance) are keyed by (vpc, vpcAttachment) and may still be in use // by another pod. Deleting them here races with cmdAdd during pod restarts — // the old pod's DEL can destroy resources the new pod just created. diff --git a/internal/cni/resource.go b/internal/cni/resource.go index 5810ecf6..57cc430c 100644 --- a/internal/cni/resource.go +++ b/internal/cni/resource.go @@ -6,16 +6,19 @@ package cni 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/tap" + "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" @@ -23,24 +26,34 @@ import ( var cniScheme = runtime.NewScheme() -// enableLocalIPAM controls whether the plugin performs IP allocation when -// no explicit ipam block is present in the CNI config. Defaults to false. -var enableLocalIPAM bool - -// SetEnableLocalIPAM sets the local IPAM flag from the CLI. -func SetEnableLocalIPAM(v bool) { - enableLocalIPAM = v -} - func init() { utilruntime.Must(clientgoscheme.AddToScheme(cniScheme)) utilruntime.Must(bgpv1alpha1.AddToScheme(cniScheme)) } -// resourceTracker tracks resources created during cmdAdd for selective rollback. +// 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). +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-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. type resourceTracker struct { vpc, vpcAttachment string - ifaceType string vrfCreated bool routesCreated int vrfInstanceCreated bool @@ -48,16 +61,9 @@ type resourceTracker struct { k8s client.Client namespace string - // ebpfRegistered, ebpfBlock, and ebpfArgument record the eBPF uSID - // datapath's vrf_table registration (registerEBPFDatapath, Milestone - // 7.1), if one actually happened (the BGPRouter may not be - // configured, in which case ebpfRegistered stays false and cleanup - // has nothing to unregister). Only vrf_table - // is rolled back here -- locator_table/function_table entries are - // keyed by Block, not by this specific (vpc, vpcAttachment), and - // typically shared across many attachments on the same node, so they - // are never this attachment's rollback's responsibility to remove - // (Milestone 7.2). + // 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 @@ -73,7 +79,7 @@ func (rt *resourceTracker) cleanup(ctx context.Context) { if rt.advCreated && rt.k8s != nil { adv := &bgpv1alpha1.BGPAdvertisement{ ObjectMeta: metav1.ObjectMeta{ - Name: bgpAdvertisementName(rt.vpc, rt.vpcAttachment), + Name: crdnames.BGPAdvertisementName(rt.vpc, rt.vpcAttachment), Namespace: rt.namespace, }, } @@ -89,7 +95,7 @@ func (rt *resourceTracker) cleanup(ctx context.Context) { if rt.vrfInstanceCreated && rt.k8s != nil { vrfInst := &bgpv1alpha1.BGPVRFInstance{ ObjectMeta: metav1.ObjectMeta{ - Name: bgpVRFInstanceName(rt.vpc, rt.vpcAttachment), + Name: crdnames.BGPVRFInstanceName(rt.vpc, rt.vpcAttachment), Namespace: rt.namespace, }, } @@ -102,20 +108,14 @@ func (rt *resourceTracker) cleanup(ctx context.Context) { } // 3. Unregister the eBPF uSID datapath's vrf_table entry (only if - // registerEBPFDatapath actually wrote one, Milestone 7.2). A pinned - // BPF map entry has no implicit teardown when the VRF/interfaces are - // deleted below, so it must be removed explicitly here and nowhere - // else in the normal cmdDel path is expected to (design plan §5.1). + // 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 { - // Recomputed fresh rather than cached at registration time: this is - // exactly the value unregisterEBPFDatapath needs to confirm the - // vrf_table slot still belongs to this attachment before deleting it - // (see its doc comment). The VRF interface itself isn't deleted - // until step 6 below, so it's still resolvable here. 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, attach.PinDir); err != nil { + } 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 { @@ -124,27 +124,15 @@ func (rt *resourceTracker) cleanup(ctx context.Context) { } } - // 4. Delete host veth (veth mode only) - if rt.ifaceType == interfaceTypeVeth { - 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) - } else { - slog.Debug("Rollback: deleted veth", "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) - } - } - - // 5. Delete tap (tap mode only) - if rt.ifaceType == interfaceTypeTap { - if err := tap.Delete(rt.vpc, rt.vpcAttachment); err != nil { - slog.Error("Rollback: failed to delete tap", "err", err, - "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) - } else { - slog.Debug("Rollback: deleted tap", "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) - } + // 4. 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) + } else { + slog.Debug("Rollback: deleted veth", "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) } - // 6. Delete VRF (flushes all routes, removes VRF interface) + // 5. 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 new file mode 100644 index 00000000..1482f7e6 --- /dev/null +++ b/internal/cni/resource_ebpf_test.go @@ -0,0 +1,140 @@ +// 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 3c2ee3a1..23a80968 100644 --- a/internal/cni/result.go +++ b/internal/cni/result.go @@ -12,12 +12,14 @@ import ( "github.com/containernetworking/cni/pkg/types" type100 "github.com/containernetworking/cni/pkg/types/100" "github.com/vishvananda/netlink" + + "go.datum.net/galactic/internal/cniipam" ) // buildResult constructs the CNI result, including IPAM data if configured. func buildResult( pluginConf *PluginConf, - ipRes *ipamResult, + ipRes *cniipam.IPAMResult, hostName, guestName string, hostMac, guestMac string, hostMTU, guestMTU int, @@ -46,32 +48,28 @@ func buildResult( // appendIPConfigs adds one IPConfig per allocated address family in ipRes // (IPv6, and IPv4 when present) plus any default routes, all pointing at the -// given Interfaces index. ipv4Mask sets the prefix length reported for the -// IPv4 address — /32 for veth, /25 for tap (matching the host gateway mask -// installed by ipv4GatewayAddrParams, so downstream consumers such as -// kraftlet configure the guest with the same real subnet the host side -// advertises). No-op when ipRes is nil. -func appendIPConfigs(result *type100.Result, ipRes *ipamResult, ifaceIndex int, ipv4Mask net.IPMask) { +// given Interfaces index. No-op when ipRes is nil. +func appendIPConfigs(result *type100.Result, ipRes *cniipam.IPAMResult, ifaceIndex int, ipv4Mask net.IPMask) { if ipRes == nil { return } - if ipRes.ipv6Subnet != nil { + if ipRes.IPv6Subnet != nil { result.IPs = append(result.IPs, &type100.IPConfig{ - Address: *ipRes.ipv6Subnet, - Gateway: ipRes.ipv6Gateway, + Address: *ipRes.IPv6Subnet, + Gateway: ipRes.IPv6Gateway, Interface: type100.Int(ifaceIndex), }) } - if ipRes.ipv4Address != nil { + if ipRes.IPv4Address != nil { result.IPs = append(result.IPs, &type100.IPConfig{ - Address: net.IPNet{IP: ipRes.ipv4Address, Mask: ipv4Mask}, - Gateway: ipRes.ipv4Gateway, + Address: net.IPNet{IP: ipRes.IPv4Address, Mask: ipv4Mask}, + Gateway: ipRes.IPv4Gateway, Interface: type100.Int(ifaceIndex), }) } - if len(ipRes.routes) > 0 { - result.Routes = make([]*types.Route, 0, len(ipRes.routes)) - for _, dst := range ipRes.routes { + if len(ipRes.Routes) > 0 { + result.Routes = make([]*types.Route, 0, len(ipRes.Routes)) + for _, dst := range ipRes.Routes { result.Routes = append(result.Routes, &types.Route{ Dst: *dst, }) @@ -88,7 +86,7 @@ func buildVethResult( hostName, guestName string, hostMac string, hostMTU int, -) (*ipamResult, net.HardwareAddr, error) { +) (*cniipam.IPAMResult, net.HardwareAddr, 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. @@ -105,9 +103,10 @@ func buildVethResult( } // Configure IP address on the guest interface inside the container netns. - var ipamResult *ipamResult - if wantsIPAM(pluginConf) { - result, err := configureIPAM(args, pluginConf, args.IfName) + cfg := allocConfig(pluginConf) + var ipamResult *cniipam.IPAMResult + if cniipam.WantsIPAM(cfg) { + result, err := configureIPAM(args, cfg, args.IfName) if err != nil { return nil, nil, fmt.Errorf("configure IPAM: %w", err) } @@ -131,28 +130,28 @@ func buildVethResult( return ipamResult, guestHWAddr, nil } -// 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 -// ipv4GatewayAddrParams installs on the host side of the tap (see bgp.go). -func buildTapResult( - pluginConf *PluginConf, - ipRes *ipamResult, - hostName, hostMac string, - hostMTU int, -) *type100.Result { - result := &type100.Result{ - CNIVersion: pluginConf.CNIVersion, - Interfaces: []*type100.Interface{ - { - Name: hostName, - Mac: hostMac, - Mtu: hostMTU, - Sandbox: "", - }, - }, +// configureIPAM allocates addresses and configures the guest interface inside +// the container network namespace with both families (when dual-stack). +func configureIPAM(args *skel.CmdArgs, cfg cniipam.AllocConfig, guestName string) (*cniipam.IPAMResult, error) { + ipamResult, err := cniipam.Allocate(args, cfg) + if err != nil { + return nil, err } - appendIPConfigs(result, ipRes, 0, net.CIDRMask(25, 32)) // index into Interfaces (host tap) - return result + if ipamResult == nil { + return nil, nil + } + + var ipv4Net *net.IPNet + if ipamResult.IPv4Address != nil { + ipv4Net = &net.IPNet{IP: ipamResult.IPv4Address, Mask: net.CIDRMask(32, 32)} + } + if err := configureInterfaceInNetns( + args.Netns, guestName, + ipamResult.IPv6Subnet, ipamResult.IPv6Gateway, + ipv4Net, ipamResult.IPv4Gateway, + ); err != nil { + return nil, err + } + + return ipamResult, nil } diff --git a/internal/cni/types.go b/internal/cni/types.go index 6398f6aa..9f33cf6f 100644 --- a/internal/cni/types.go +++ b/internal/cni/types.go @@ -5,9 +5,10 @@ package cni import ( - "net" - "github.com/containernetworking/cni/pkg/types" + + "go.datum.net/galactic/internal/cni/hostconf" + "go.datum.net/galactic/internal/cniipam" ) // Termination represents a network termination point with a destination @@ -17,28 +18,8 @@ type Termination struct { Via string `json:"via,omitempty"` } -// IPAM holds IP address management configuration passed in the CNI config. -// Pool CIDR fields (formerly Pool/Gateway/SubnetLen) have been retired in -// favor of PluginConf.IPv6Subnet/IPv4Subnet — see allocateIPAM. -type IPAM struct { - Type string `json:"type"` // "pool" (default) or "static" - StaticIP string `json:"static_ip,omitempty"` // used when type="static" - Routes []Route `json:"routes,omitempty"` - Addresses []Address `json:"addresses,omitempty"` -} - -// Route describes a static route to install. -type Route struct { - Dst string `json:"dst"` - GW string `json:"gw,omitempty"` -} - -// Address describes a static IP address assignment. -type Address struct { - Address string `json:"address"` -} - -// PluginConf is the CNI plugin configuration passed via stdin on each invocation. +// PluginConf is the CNI plugin configuration passed via stdin on each +// invocation of galactic-cni, the veth master plugin. // // IPv6Subnet, IPv4Subnet, and AddressFamilies feed the dual-stack IPAM // allocators (internal/cni/ipam, IPv4PoolAllocator/DualStackAllocator); as of @@ -49,9 +30,8 @@ type PluginConf struct { VPC string `json:"vpc"` VPCAttachment string `json:"vpcattachment"` MTU int `json:"mtu,omitempty"` - InterfaceType string `json:"interface_type,omitempty"` // interfaceTypeVeth or interfaceTypeTap Terminations []Termination `json:"terminations,omitempty"` - IPAM *IPAM `json:"ipam"` + IPAM *cniipam.IPAM `json:"ipam"` Namespace string `json:"namespace,omitempty"` IPv6Subnet string `json:"ipv6_subnet,omitempty"` // region IPv6 pool CIDR; endpoints alloc /96 IPv4Subnet string `json:"ipv4_subnet,omitempty"` // optional site IPv4 pool CIDR; endpoints alloc /32 @@ -59,23 +39,7 @@ type PluginConf struct { } // HostConf holds node-local settings read from /etc/cni/net.d/10-galactic.conflist. -type HostConf struct { - NodeName string `json:"node_name"` - Kubeconfig string `json:"kubeconfig"` - Namespace string `json:"namespace"` - LogFile string `json:"log_file"` - LogLevel string `json:"log_level,omitempty"` -} - -// ipamResult holds the IPAM allocation details for building the CNI result. -// ipv4Address/ipv4Gateway are nil when the attachment is IPv6-only. -type ipamResult struct { - ipv6Subnet *net.IPNet - ipv6Gateway net.IP - ipv4Address net.IP - ipv4Gateway net.IP - routes []*net.IPNet -} +type HostConf = hostconf.HostConf // HostDevicePluginConf is the configuration for the host-device CNI plugin // delegation used to move the guest veth endpoint into the container netns. @@ -83,3 +47,19 @@ type HostDevicePluginConf struct { types.PluginConf Device string `json:"device"` } + +// allocConfig adapts pluginConf's fields into cniipam.AllocConfig, the shape +// internal/cniipam actually needs — see that package's doc comment for why +// it takes its own minimal config shape rather than this package's full +// PluginConf. +func allocConfig(pluginConf *PluginConf) cniipam.AllocConfig { + return cniipam.AllocConfig{ + VPC: pluginConf.VPC, + VPCAttachment: pluginConf.VPCAttachment, + Namespace: pluginConf.Namespace, + IPAM: pluginConf.IPAM, + IPv6Subnet: pluginConf.IPv6Subnet, + IPv4Subnet: pluginConf.IPv4Subnet, + AddressFamilies: pluginConf.AddressFamilies, + } +} diff --git a/internal/cnibgp/bgp.go b/internal/cnibgp/bgp.go new file mode 100644 index 00000000..477ccc0f --- /dev/null +++ b/internal/cnibgp/bgp.go @@ -0,0 +1,703 @@ +// Copyright 2025 Datum Cloud, Inc. +// +// 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 + +import ( + "context" + "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" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + "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. +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 +} + +// 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 + // 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 +} + +// isTransientError reports whether err is a transient failure that may +// resolve itself on retry (API server unavailable, timeout, network blip). +// Returns false for validation errors, not-found, and other permanent +// failures that should not be retried. +func isTransientError(err error) bool { + if err == nil { + return false + } + if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) { + return true + } + unwrapped := errors.Unwrap(err) + if unwrapped != nil { + if errors.Is(unwrapped, context.DeadlineExceeded) || errors.Is(unwrapped, context.Canceled) { + return true + } + } + if apierrors.IsServiceUnavailable(err) || + apierrors.IsInternalError(err) || + apierrors.IsServerTimeout(err) || + apierrors.IsTooManyRequests(err) { + return true + } + if netErr, ok := unwrapped.(interface{ Temporary() bool }); ok && netErr.Temporary() { + return true + } + return false +} + +// retryK8sOps runs fn with up to maxRetries+1 attempts, retrying on +// transient k8s API errors with exponential backoff. The context passed to +// fn has a timeout derived from timeout. Non-transient errors are returned +// immediately without retry. +func retryK8sOps(timeout time.Duration, fn func(ctx context.Context) error) error { + var lastErr error + for attempt := 0; attempt <= maxRetries; attempt++ { + if attempt > 0 { + backoff := time.Duration(1< %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 +// intentionally not set up for this attachment. Any other failure is +// returned as an error. +func registerEBPFDatapath( + bgp bgpConfig, vpc, vpcAttachment, ifaceType string, argument uint16, pinDir string, +) (registered bool, block uint64, err error) { + if bgp.srv6Locator == "" || bgp.nodeID == 0 { + return false, 0, nil + } + + if bgp.nodeID < uformat.NodeIDMin || bgp.nodeID > uformat.NodeIDMax { + return false, 0, fmt.Errorf("eBPF registration: nodeID %d out of range [%#x,%#x]", + bgp.nodeID, uint16(uformat.NodeIDMin), uint16(uformat.NodeIDMax)) + } + + egressKind, err := EgressKindForInterfaceType(ifaceType) + if err != nil { + return false, 0, fmt.Errorf("determine eBPF egress kind: %w", err) + } + + prefix, err := netip.ParsePrefix(bgp.srv6Locator) + if err != nil { + return false, 0, fmt.Errorf("parse SRv6 locator %q for eBPF registration: %w", bgp.srv6Locator, err) + } + block, err = uformat.Block(prefix.Addr()) + if err != nil { + return false, 0, fmt.Errorf("derive eBPF uSID Block from locator %q: %w", bgp.srv6Locator, err) + } + + vrfTableID, err := vrf.TableID(vpc, vpcAttachment) + if err != nil { + return false, 0, fmt.Errorf("look up VRF table id for eBPF registration: %w", err) + } + + registry, closer, err := usidmap.OpenPinnedRegistry(pinDir) + if err != nil { + return false, 0, fmt.Errorf("open pinned eBPF uSID maps: %w", err) + } + defer func() { _ = closer.Close() }() + + if err := registry.Locator.Register(block, uint16(bgp.nodeID)); err != nil { + return false, 0, fmt.Errorf("register eBPF locator_table entry: %w", err) + } + if err := registry.Function.Register(block, uformat.FunctionEndDT46); err != nil { + return false, 0, fmt.Errorf("register eBPF function_table entry: %w", err) + } + + if err := registry.VRF.Register(block, argument, vrfTableID, egressKind); err != nil { + return false, 0, fmt.Errorf("register eBPF vrf_table entry: %w", err) + } + return true, block, nil +} + +// 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) { + switch ifaceType { + case ifaceTypeVeth, "": + return usidmap.EgressKindVeth, nil + case ifaceTypeTap: + return usidmap.EgressKindTap, nil + default: + return 0, fmt.Errorf("unknown interface type %q", ifaceType) + } +} + +// 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. +// +// 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 { + registry, closer, err := usidmap.OpenPinnedRegistry(pinDir) + if err != nil { + return fmt.Errorf("open pinned eBPF uSID maps: %w", err) + } + defer func() { _ = closer.Close() }() + + entry, ok, err := registry.VRF.Get(block, argument) + if err != nil { + return fmt.Errorf("read eBPF vrf_table entry before unregister: %w", err) + } + if !ok { + return nil + } + if entry.VRFTableID != expectedVRFTableID { + slog.Warn("Rollback: eBPF vrf_table entry no longer belongs to this attachment, leaving it in place", + "block", block, "argument", argument, + "expectedVRFTableID", expectedVRFTableID, "currentVRFTableID", entry.VRFTableID) + return nil + } + + if err := registry.VRF.Unregister(block, argument); err != nil { + return fmt.Errorf("unregister eBPF vrf_table entry: %w", err) + } + return nil +} diff --git a/internal/cni/bgp_ebpf_test.go b/internal/cnibgp/bgp_ebpf_test.go similarity index 54% rename from internal/cni/bgp_ebpf_test.go rename to internal/cnibgp/bgp_ebpf_test.go index 49e810ff..7d97f1e4 100644 --- a/internal/cni/bgp_ebpf_test.go +++ b/internal/cnibgp/bgp_ebpf_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0-or-later -package cni +package cnibgp import ( "fmt" @@ -15,6 +15,16 @@ import ( "go.datum.net/galactic/internal/plumbing/vrf" ) +// requireRoot skips the test unless running as root — pinned eBPF maps and +// real VRF/netlink state need CAP_NET_ADMIN/CAP_BPF and a real kernel. See +// internal/cni's own requireRoot for the project-wide pattern. +func requireRoot(t *testing.T) { + t.Helper() + if os.Geteuid() != 0 { + t.Skip("requires root (CAP_NET_ADMIN/CAP_BPF); run under scripts/ci.sh unittest-root") + } +} + // TestRegisterEBPFDatapath_NotConfiguredIsNoOp covers the short-circuit for // a node whose BGPRouter has no SRv6Locator/NodeID configured at all: SRv6 // is intentionally not set up for this attachment, so registerEBPFDatapath @@ -22,7 +32,7 @@ import ( func TestRegisterEBPFDatapath_NotConfiguredIsNoOp(t *testing.T) { cfg := bgpConfig{srv6Locator: "", nodeID: 0} registered, _, err := registerEBPFDatapath( - cfg, testVPC, testAttachment, interfaceTypeVeth, 42, "/sys/fs/bpf/galactic-does-not-exist") + cfg, testVPC, testAttachment, ifaceTypeVeth, 42, "/sys/fs/bpf/galactic-does-not-exist") if err != nil { t.Errorf("registerEBPFDatapath with unconfigured BGPRouter = %v, want nil (no-op)", err) } @@ -35,18 +45,11 @@ func TestRegisterEBPFDatapath_NotConfiguredIsNoOp(t *testing.T) { // bounds check on the raw nodeID *before* it narrows to uint16 for // registration: an out-of-[uformat.NodeIDMin,NodeIDMax] value (here, one // that wraps to an in-range-looking uint16 if narrowed unchecked -- 0x10001 -// wraps to 1, which alone would otherwise pass uformat.ValidateNodeID inside -// registry.Locator.Register) must be rejected here, before any pinned map -// is even opened -- proven the same way -// TestRegisterEBPFDatapath_NotConfiguredIsNoOp proves its own no-op, via a -// pinDir that doesn't exist: if this check ran after the narrowing (or not -// at all), the call would instead fail later with an "open pinned eBPF uSID -// maps" error against that bogus path, not the bounds-check error this test -// asserts. +// wraps to 1) must be rejected here, before any pinned map is even opened. func TestRegisterEBPFDatapath_RejectsOutOfRangeNodeID(t *testing.T) { cfg := bgpConfig{srv6Locator: "2001:db8:1::/48", nodeID: 0x10001} // wraps to uint16(1) if narrowed unchecked registered, _, err := registerEBPFDatapath( - cfg, testVPC, testAttachment, interfaceTypeVeth, 42, "/sys/fs/bpf/galactic-does-not-exist") + cfg, testVPC, testAttachment, ifaceTypeVeth, 42, "/sys/fs/bpf/galactic-does-not-exist") if err == nil { t.Fatal("registerEBPFDatapath with nodeID=0x10001 = nil error, want an out-of-range rejection") } @@ -58,11 +61,11 @@ func TestRegisterEBPFDatapath_RejectsOutOfRangeNodeID(t *testing.T) { } } -// TestRegisterEBPFDatapath_RegistersAllThreeTables is Milestone 7.1's exit -// criterion: a single registerEBPFDatapath call populates locator_table, -// function_table, and vrf_table consistently for the same (vpc, -// vpcAttachment), against real pinned eBPF maps under a throwaway pin -// directory -- not the production attach.PinDir. +// TestRegisterEBPFDatapath_RegistersAllThreeTables: a single +// registerEBPFDatapath call populates locator_table, function_table, and +// vrf_table consistently for the same (vpc, vpcAttachment), against real +// pinned eBPF maps under a throwaway pin directory — not the production +// attach.PinDir. func TestRegisterEBPFDatapath_RegistersAllThreeTables(t *testing.T) { requireRoot(t) @@ -88,7 +91,7 @@ func TestRegisterEBPFDatapath_RegistersAllThreeTables(t *testing.T) { t.Cleanup(func() { _ = loaderObjs.Close() }) cfg := bgpConfig{srv6Locator: locator, nodeID: nodeID} - registered, _, err := registerEBPFDatapath(cfg, vpc, vpcAttachment, interfaceTypeVeth, uint16(vrfID), pinDir) + registered, _, err := registerEBPFDatapath(cfg, vpc, vpcAttachment, ifaceTypeVeth, uint16(vrfID), pinDir) if err != nil { t.Fatalf("registerEBPFDatapath: %v", err) } @@ -119,8 +122,8 @@ func TestRegisterEBPFDatapath_RegistersAllThreeTables(t *testing.T) { entries[0].VRFTableID, vrfTableID) } if entries[0].EgressKind != usidmap.EgressKindVeth { - t.Errorf("vrf_table entry EgressKind = %d, want %d (EgressKindVeth, from InterfaceType %q)", - entries[0].EgressKind, usidmap.EgressKindVeth, interfaceTypeVeth) + t.Errorf("vrf_table entry EgressKind = %d, want %d (EgressKindVeth, from interfaceType %q)", + entries[0].EgressKind, usidmap.EgressKindVeth, ifaceTypeVeth) } locEntries, err := reg.Locator.List() @@ -140,24 +143,10 @@ func TestRegisterEBPFDatapath_RegistersAllThreeTables(t *testing.T) { } } -// TestResourceTrackerCleanup_UnregistersEBPFVRFEntry is Milestone 7.2's -// exit criterion: a failed ADD's rollback (resourceTracker.cleanup) cleans -// up both the kernel route (existing behavior, already covered by -// TestResourceTrackerCleanupPartialState) and the new 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, unlike registerEBPFDatapath -- see resource.go), so this -// test loads/pins the real datapath there for the duration of the test, -// cleaning it up fully afterward; this mirrors the same "real global -// state" pattern this file's other resourceTracker tests already use for -// vrf.Delete/veth.Delete. -// -// cleanup's unregister step now 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 -- unlike before this fix, -// where the seeded entry's VRFTableID was an arbitrary, unrelated value. -func TestResourceTrackerCleanup_UnregistersEBPFVRFEntry(t *testing.T) { +// TestUnregisterEBPFDatapath_RemovesOwnEntry covers UnregisterEBPFDatapath's +// normal path: an entry this attachment registered gets removed when its +// VRFTableID still matches. +func TestUnregisterEBPFDatapath_RemovesOwnEntry(t *testing.T) { requireRoot(t) if err := vrf.Add(testVPC, testAttachment); err != nil { @@ -169,16 +158,17 @@ func TestResourceTrackerCleanup_UnregistersEBPFVRFEntry(t *testing.T) { t.Fatalf("vrf.TableID: %v", err) } - loaderObjs, err := attach.Load(attach.PinDir) + pinDir := fmt.Sprintf("/sys/fs/bpf/galactic-bgp-test-%d", os.Getpid()) + t.Cleanup(func() { _ = os.RemoveAll(pinDir) }) + loaderObjs, err := attach.Load(pinDir) if err != nil { - t.Fatalf("attach.Load(attach.PinDir): %v", err) + t.Fatalf("attach.Load: %v", err) } t.Cleanup(func() { _ = loaderObjs.Close() }) - t.Cleanup(func() { _ = os.RemoveAll(attach.PinDir) }) - reg, closer, err := usidmap.OpenPinnedRegistry(attach.PinDir) + reg, closer, err := usidmap.OpenPinnedRegistry(pinDir) if err != nil { - t.Fatalf("OpenPinnedRegistry(attach.PinDir): %v", err) + t.Fatalf("OpenPinnedRegistry: %v", err) } defer func() { _ = closer.Close() }() @@ -188,62 +178,46 @@ func TestResourceTrackerCleanup_UnregistersEBPFVRFEntry(t *testing.T) { 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 err := UnregisterEBPFDatapath(testBlock, testArgument, vrfTableID, pinDir); err != nil { + t.Fatalf("UnregisterEBPFDatapath: %v", err) + } 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) + t.Errorf("vrf_table entry after unregister: ok=%v err=%v, want ok=false", 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 (registerEBPFDatapath only runs again if -// that attempt gets that far), so by the time a later attempt's -// checkArgumentCollision failure triggers this rollback, the (block, +// TestUnregisterEBPFDatapath_LeavesEntryOwnedByAnotherAttachment covers the +// race UnregisterEBPFDatapath guards against: 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 a caller's 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) { +// against. Unregistering unconditionally would delete a live attachment's +// forwarding entry instead of this rolled-back one's own. +func TestUnregisterEBPFDatapath_LeavesEntryOwnedByAnotherAttachment(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) + pinDir := fmt.Sprintf("/sys/fs/bpf/galactic-bgp-test-%d", os.Getpid()) + t.Cleanup(func() { _ = os.RemoveAll(pinDir) }) + loaderObjs, err := attach.Load(pinDir) if err != nil { - t.Fatalf("attach.Load(attach.PinDir): %v", err) + t.Fatalf("attach.Load: %v", err) } t.Cleanup(func() { _ = loaderObjs.Close() }) - t.Cleanup(func() { _ = os.RemoveAll(attach.PinDir) }) - reg, closer, err := usidmap.OpenPinnedRegistry(attach.PinDir) + reg, closer, err := usidmap.OpenPinnedRegistry(pinDir) if err != nil { - t.Fatalf("OpenPinnedRegistry(attach.PinDir): %v", err) + t.Fatalf("OpenPinnedRegistry: %v", err) } defer func() { _ = closer.Close() }() const testBlock uint64 = 0x0102030405 const testArgument uint16 = 0x042 const anotherAttachmentsVRFTableID uint32 = 0x9999 + const thisAttachmentsVRFTableID uint32 = 0x1111 // Simulate the colliding attachment having since overwritten this same // (block, argument) slot with its own, different VRF table id. @@ -251,23 +225,17 @@ func TestResourceTrackerCleanup_LeavesEBPFVRFEntryOwnedByAnotherAttachment(t *te 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()) + if err := UnregisterEBPFDatapath(testBlock, testArgument, thisAttachmentsVRFTableID, pinDir); err != nil { + t.Fatalf("UnregisterEBPFDatapath: %v", err) + } 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)", + t.Fatalf("vrf_table entry after unregister: 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", + t.Errorf("vrf_table entry VRFTableID after unregister = %#x, want unchanged %#x", entry.VRFTableID, anotherAttachmentsVRFTableID) } } diff --git a/internal/cni/bgp_test.go b/internal/cnibgp/bgp_test.go similarity index 54% rename from internal/cni/bgp_test.go rename to internal/cnibgp/bgp_test.go index 91cd70e3..1ab67fc7 100644 --- a/internal/cni/bgp_test.go +++ b/internal/cnibgp/bgp_test.go @@ -2,26 +2,99 @@ // // SPDX-License-Identifier: AGPL-3.0-or-later -package cni +package cnibgp import ( "context" + "errors" "fmt" "net" "reflect" "strings" "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" + "k8s.io/apimachinery/pkg/runtime/schema" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "go.datum.net/galactic/internal/cni/crdnames" + "go.datum.net/galactic/internal/cniipam" "go.datum.net/galactic/internal/plumbing/ebpf/uformat" "go.datum.net/galactic/internal/plumbing/ebpf/usidmap" bgpv1alpha1 "go.datum.net/network/api/v1alpha1" ) +const ( + testVPC = "abc" + testAttachment = "def" + testRouterName = "overlay-router" + testRD65000_1 = "65000:1" + testVPCHex1234 = "0000000004d2" // decimal 1234 + testNetns = "/proc/1/ns/net" +) + +var testScheme = func() *runtime.Scheme { + s := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(s)) + utilruntime.Must(bgpv1alpha1.AddToScheme(s)) + return s +}() + +func fakeClient(objs ...client.Object) client.Client { + return fake.NewClientBuilder().WithScheme(testScheme).WithObjects(objs...).Build() +} + +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 +} + +// routerForNode builds a BGPRouter with spec.targetRef.name set to nodeName. +func routerForNode(name, nodeName, namespace string, asn int64) *bgpv1alpha1.BGPRouter { + return &bgpv1alpha1.BGPRouter{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: bgpv1alpha1.BGPRouterSpec{ + TargetRef: bgpv1alpha1.TargetRef{ + Kind: "Node", + Name: nodeName, + }, + LocalASN: asn, + RouterID: "10.0.0.1", + Roles: []bgpv1alpha1.RouterRole{bgpv1alpha1.RouterRoleTenant}, + AddressFamilies: []bgpv1alpha1.AddressFamily{ + {AFI: bgpv1alpha1.AFIL2VPN, SAFI: bgpv1alpha1.SAFIEVPN}, + }, + }, + } +} + +// vrfInstanceForRouter builds a BGPVRFInstance targeting routerName with the +// given VRFID (the allocated Argument), for allocateArgument's test fixtures. +func vrfInstanceForRouter(name, namespace, routerName string, vrfID int32) *bgpv1alpha1.BGPVRFInstance { + return &bgpv1alpha1.BGPVRFInstance{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + Spec: bgpv1alpha1.BGPVRFInstanceSpec{ + RouterTarget: bgpv1alpha1.RouterTarget{RouterRef: &bgpv1alpha1.RouterRef{Name: routerName}}, + VRFID: vrfID, + }, + } +} + // ---- ipv4GatewayAddrParams ------------------------------------------------ func TestIPv4GatewayAddrParams(t *testing.T) { @@ -72,71 +145,40 @@ func TestRouteConflicts(t *testing.T) { 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}, { - name: "nil existing destination — no conflict", - existing: &netlink.Route{Dst: nil}, - desired: &netlink.Route{Dst: dst}, - want: false, - }, - { - name: "nil desired destination — no conflict", - existing: &netlink.Route{Dst: dst}, - desired: &netlink.Route{Dst: nil}, - want: false, - }, - { - name: "different destinations — no conflict", - existing: &netlink.Route{Dst: otherDst}, - desired: &netlink.Route{Dst: dst}, - want: false, - }, - { - name: "same destination, no gateway on either — no conflict", - existing: &netlink.Route{Dst: dst, LinkIndex: 5}, - desired: &netlink.Route{Dst: dst, LinkIndex: 5}, - want: false, + "same destination, no gateway on either — no conflict", + &netlink.Route{Dst: dst, LinkIndex: 5}, &netlink.Route{Dst: dst, LinkIndex: 5}, false, }, { - name: "same destination, same gateway — no conflict", - existing: &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, - desired: &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, - want: false, + "same destination, same gateway — no conflict", + &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, false, }, { - name: "same destination, different gateway — conflict", - existing: &netlink.Route{Dst: dst, Gw: gw1}, - desired: &netlink.Route{Dst: dst, Gw: gw2}, - want: true, + "same destination, different gateway — conflict", + &netlink.Route{Dst: dst, Gw: gw1}, &netlink.Route{Dst: dst, Gw: gw2}, true, }, { - name: "existing has gateway, desired does not — conflict", - existing: &netlink.Route{Dst: dst, Gw: gw1}, - desired: &netlink.Route{Dst: dst}, - want: true, + "existing has gateway, desired does not — conflict", + &netlink.Route{Dst: dst, Gw: gw1}, &netlink.Route{Dst: dst}, true, }, { - name: "desired has gateway, existing does not — conflict", - existing: &netlink.Route{Dst: dst}, - desired: &netlink.Route{Dst: dst, Gw: gw1}, - want: true, + "desired has gateway, existing does not — conflict", + &netlink.Route{Dst: dst}, &netlink.Route{Dst: dst, Gw: gw1}, true, }, { - name: "same destination, same gateway, different link index — conflict", - existing: &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, - desired: &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 7}, - want: 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, }, { - name: "same destination, gateway set, link index zero on existing — no conflict", - existing: &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 0}, - desired: &netlink.Route{Dst: dst, Gw: gw1, LinkIndex: 5}, - want: false, + "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, }, { - name: "same destination, no gateway, different link index — conflict", - existing: &netlink.Route{Dst: dst, LinkIndex: 5}, - desired: &netlink.Route{Dst: dst, LinkIndex: 7}, - want: true, + "same destination, no gateway, different link index — conflict", + &netlink.Route{Dst: dst, LinkIndex: 5}, &netlink.Route{Dst: dst, LinkIndex: 7}, true, }, } @@ -152,18 +194,6 @@ func TestRouteConflicts(t *testing.T) { // ---- allocateArgument ------------------------------------------------------ -// vrfInstanceForRouter builds a BGPVRFInstance targeting routerName with the -// given VRFID (the allocated Argument), for allocateArgument's test fixtures. -func vrfInstanceForRouter(name, namespace, routerName string, vrfID int32) *bgpv1alpha1.BGPVRFInstance { - return &bgpv1alpha1.BGPVRFInstance{ - ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, - Spec: bgpv1alpha1.BGPVRFInstanceSpec{ - RouterTarget: bgpv1alpha1.RouterTarget{RouterRef: &bgpv1alpha1.RouterRef{Name: routerName}}, - VRFID: vrfID, - }, - } -} - func TestAllocateArgument(t *testing.T) { const ( namespace = "default" @@ -196,9 +226,6 @@ func TestAllocateArgument(t *testing.T) { t.Run("skips values used by this router and ignores other routers", func(t *testing.T) { used1 := vrfInstanceForRouter("other-att-1", namespace, routerName, 1) used2 := vrfInstanceForRouter("other-att-2", namespace, routerName, 2) - // Same VRFID (1) under a different router -- must not count toward - // this router's used set, since Argument allocation is per node - // (i.e. per BGPRouter), not platform-wide. differentRouter := vrfInstanceForRouter("different-router-att", namespace, "other-router", 1) k8s := fakeClient(used1, used2, differentRouter) got, err := allocateArgument(context.Background(), k8s, namespace, routerName, "new-att") @@ -241,12 +268,6 @@ func TestAllocateArgument(t *testing.T) { // ---- checkArgumentCollision ------------------------------------------------- -// TestCheckArgumentCollision guards against a regression of the fix where a -// lexicographic-name tie-break let exactly one of two colliding instances -// "win" without ever proving the other side's check would run after this -// one's create -- concurrent create+check interleaving could let both sides -// pass. Detection must not depend on name ordering: it must fire regardless -// of whether the other instance's name sorts before or after this one's. func TestCheckArgumentCollision(t *testing.T) { const ( namespace = "default" @@ -295,7 +316,7 @@ func TestCheckArgumentCollision(t *testing.T) { }) } -// ---- egressKindForInterfaceType -------------------------------------------- +// ---- EgressKindForInterfaceType -------------------------------------------- func TestEgressKindForInterfaceType(t *testing.T) { tests := []struct { @@ -304,26 +325,26 @@ func TestEgressKindForInterfaceType(t *testing.T) { want uint32 wantErr bool }{ - {name: "veth maps to EgressKindVeth", iface: interfaceTypeVeth, want: usidmap.EgressKindVeth}, + {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: interfaceTypeTap, want: usidmap.EgressKindTap}, + {name: "tap maps to EgressKindTap", iface: ifaceTypeTap, want: usidmap.EgressKindTap}, {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) } }) } @@ -337,9 +358,6 @@ func TestBuildVRFInstanceSpec(t *testing.T) { if spec.RouterRef == nil || spec.RouterRef.Name != testRouterName { t.Errorf("RouterRef = %+v, want Name %q", spec.RouterRef, testRouterName) } - if spec.RouterSelector != nil { - t.Errorf("RouterSelector = %+v, want nil", spec.RouterSelector) - } if spec.VRFID != 1234 { t.Errorf("VRFID = %d, want 1234", spec.VRFID) } @@ -364,7 +382,7 @@ func TestIPAMAdvertisementPrefixesNil(t *testing.T) { } func TestIPAMAdvertisementPrefixesIPv4Only(t *testing.T) { - res := &ipamResult{ipv4Address: net.ParseIP("10.128.0.5")} + res := &cniipam.IPAMResult{IPv4Address: net.ParseIP("10.128.0.5")} prefixes, ipv6Subnet, ipv4Addr := ipamAdvertisementPrefixes(res) @@ -381,7 +399,7 @@ func TestIPAMAdvertisementPrefixesIPv4Only(t *testing.T) { func TestIPAMAdvertisementPrefixesDualStack(t *testing.T) { ipv6Subnet := mustParseCIDR(t, "fd00:10:ff01::1234/96") - res := &ipamResult{ipv6Subnet: ipv6Subnet, ipv4Address: net.ParseIP("10.128.0.5")} + res := &cniipam.IPAMResult{IPv6Subnet: ipv6Subnet, IPv4Address: net.ParseIP("10.128.0.5")} prefixes, gotIPv6Subnet, gotIPv4Addr := ipamAdvertisementPrefixes(res) @@ -410,9 +428,9 @@ func TestAllAdvertisedPrefixesEmpty(t *testing.T) { func TestAllAdvertisedPrefixesSingleContainer(t *testing.T) { const v6, v4 = "fd00:20:ff01::1234/96", "172.20.1.5" annotations := map[string]string{ - netnsAnnotationKey("cid-a"): testNetns, - subnetAnnotationKeyIPv6("cid-a"): v6, - subnetAnnotationKeyIPv4("cid-a"): v4, + crdnames.NetNSKey("cid-a"): testNetns, + crdnames.SubnetKeyIPv6("cid-a"): v6, + crdnames.SubnetKeyIPv4("cid-a"): v4, } got := allAdvertisedPrefixes(annotations) @@ -431,11 +449,11 @@ func TestAllAdvertisedPrefixesSingleContainer(t *testing.T) { func TestAllAdvertisedPrefixesMultipleContainers(t *testing.T) { const aV4, bV6, bV4 = "172.20.1.5", "fd00:20:ff01::1234/96", "172.21.1.2" annotations := map[string]string{ - netnsAnnotationKey("cid-a"): testNetns, - subnetAnnotationKeyIPv4("cid-a"): aV4, - netnsAnnotationKey("cid-b"): testNetns, - subnetAnnotationKeyIPv6("cid-b"): bV6, - subnetAnnotationKeyIPv4(("cid-b")): bV4, + crdnames.NetNSKey("cid-a"): testNetns, + crdnames.SubnetKeyIPv4("cid-a"): aV4, + crdnames.NetNSKey("cid-b"): testNetns, + crdnames.SubnetKeyIPv6("cid-b"): bV6, + crdnames.SubnetKeyIPv4("cid-b"): bV4, } got := allAdvertisedPrefixes(annotations) @@ -449,9 +467,9 @@ func TestAllAdvertisedPrefixesMultipleContainers(t *testing.T) { func TestAllAdvertisedPrefixesIgnoresOtherAnnotations(t *testing.T) { const v4 = "172.20.1.5" annotations := map[string]string{ - netnsAnnotationKey("cid-a"): testNetns, - subnetAnnotationKeyIPv4("cid-a"): v4, - "some.other/annotation": "should be ignored", + crdnames.NetNSKey("cid-a"): testNetns, + crdnames.SubnetKeyIPv4("cid-a"): v4, + "some.other/annotation": "should be ignored", } got := allAdvertisedPrefixes(annotations) @@ -503,3 +521,255 @@ func TestBuildAdvertisementSpecDualStack(t *testing.T) { t.Errorf("Prefixes[1] = %q, want %q", spec.Prefixes[1], ipv4Prefix) } } + +// ---- routeTarget --------------------------------------------------------- + +func TestRouteTarget(t *testing.T) { + tests := []struct { + name string + asNumber int64 + vpcHex string + want string + wantErr bool + }{ + {name: "VPC value fits in 32 bits", asNumber: 65000, vpcHex: testVPCHex1234, want: "65000:1234"}, + {name: "upper bits beyond 32 stripped", asNumber: 65000, vpcHex: "000100000001", want: testRD65000_1}, + {name: "low 32 bits all set", asNumber: 65000, vpcHex: "0000ffffffff", want: "65000:4294967295"}, + {name: "different ASN", asNumber: 4200000000, vpcHex: testVPCHex1234, want: "4200000000:1234"}, + {name: "invalid hex string", vpcHex: "zzzzzz", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := routeTarget(tt.asNumber, tt.vpcHex) + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Errorf("routeTarget(%d, %q) = %q, want %q", tt.asNumber, tt.vpcHex, got, tt.want) + } + }) + } +} + +// ---- lookupBGPRouter ----------------------------------------------------- + +func TestLookupBGPRouter(t *testing.T) { + ctx := context.Background() + const ( + nodeName = "node1" + namespace = "default" + ) + + matchingRouter := routerForNode(testRouterName, nodeName, namespace, 65000) + + tests := []struct { + name string + objects []client.Object + wantErr string + check func(t *testing.T, cfg bgpConfig) + }{ + {name: "no router for node", objects: nil, wantErr: "no BGPRouter found"}, + { + name: "single matching router returns correct config", + objects: []client.Object{matchingRouter}, + check: func(t *testing.T, cfg bgpConfig) { + t.Helper() + if cfg.asNumber != 65000 { + t.Errorf("asNumber = %d, want 65000", cfg.asNumber) + } + if cfg.routerName != testRouterName { + t.Errorf("routerName = %q, want %q", cfg.routerName, testRouterName) + } + }, + }, + { + name: "router with SRv6Locator and NodeID configured", + objects: []client.Object{ + func() *bgpv1alpha1.BGPRouter { + r := routerForNode("srv6-router", nodeName, namespace, 65000) + r.Spec.SRv6Locator = "fd00:10::/48" + r.Spec.NodeID = 7 + return r + }(), + }, + check: func(t *testing.T, cfg bgpConfig) { + t.Helper() + if cfg.srv6Locator != "fd00:10::/48" { + t.Errorf("srv6Locator = %q, want %q", cfg.srv6Locator, "fd00:10::/48") + } + if cfg.nodeID != 7 { + t.Errorf("nodeID = %d, want 7", cfg.nodeID) + } + }, + }, + { + name: "router in different namespace is ignored", + objects: []client.Object{routerForNode("other-ns-router", nodeName, "other-ns", 65001)}, + wantErr: "no BGPRouter found", + }, + { + name: "non-matching node router is ignored", + objects: []client.Object{ + routerForNode("other-node-router", "node2", namespace, 65001), + matchingRouter, + }, + check: func(t *testing.T, cfg bgpConfig) { + t.Helper() + if cfg.routerName != testRouterName { + t.Errorf("routerName = %q, want %q", cfg.routerName, testRouterName) + } + }, + }, + { + name: "ambiguous: two routers target same node", + objects: []client.Object{ + routerForNode("router-a", nodeName, namespace, 65000), + routerForNode("router-b", nodeName, namespace, 65001), + }, + wantErr: "ambiguous", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k8s := fakeClient(tt.objects...) + + cfg, err := lookupBGPRouter(ctx, k8s, nodeName, namespace) + if tt.wantErr != "" { + if err == nil { + t.Fatalf("expected error containing %q, got nil", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("error %q does not contain %q", err, tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tt.check != nil { + tt.check(t, cfg) + } + }) + } +} + +// ---- isTransientError ---------------------------------------------------- + +func TestIsTransientError(t *testing.T) { + tests := []struct { + name string + err error + wantTrans bool + }{ + {"nil error is not transient", nil, false}, + {"context deadline exceeded is transient", context.DeadlineExceeded, true}, + {"context canceled is transient", context.Canceled, true}, + {"wrapped context deadline exceeded is transient", fmt.Errorf("k8s: %w", context.DeadlineExceeded), true}, + {"wrapped context canceled is transient", fmt.Errorf("k8s: %w", context.Canceled), true}, + {"generic error is not transient", errors.New("some error"), false}, + {"validation error is not transient", apierrors.NewBadRequest("bad request"), false}, + { + "not found error is not transient", + apierrors.NewNotFound(schema.GroupResource{Group: "network.datumapis.com", Resource: "bgpadvertisements"}, "test"), + false, + }, + {"503 service unavailable is transient", apierrors.NewServiceUnavailable("service unavailable"), true}, + {"429 too many requests is transient", apierrors.NewTooManyRequests("too many requests", 0), true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isTransientError(tt.err) + if got != tt.wantTrans { + t.Errorf("isTransientError(%v) = %v, want %v", tt.err, got, tt.wantTrans) + } + }) + } +} + +// ---- retryK8sOps --------------------------------------------------------- + +func TestRetryK8sOpsSucceedsImmediately(t *testing.T) { + calls := 0 + err := retryK8sOps(100*time.Millisecond, func(ctx context.Context) error { + calls++ + return nil + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if calls != 1 { + t.Errorf("expected 1 call, got %d", calls) + } +} + +func TestRetryK8sOpsRetriesOnTransientError(t *testing.T) { + calls := 0 + err := retryK8sOps(2*time.Second, func(ctx context.Context) error { + calls++ + if calls < 3 { + return context.DeadlineExceeded + } + return nil + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if calls != 3 { + t.Errorf("expected 3 calls (initial + 2 retries), got %d", calls) + } +} + +func TestRetryK8sOpsFailsAfterMaxRetries(t *testing.T) { + calls := 0 + err := retryK8sOps(2*time.Second, func(ctx context.Context) error { + calls++ + return context.DeadlineExceeded + }) + if err == nil { + t.Fatal("expected error, got nil") + } + if calls != maxRetries+1 { + t.Errorf("expected %d calls (initial + maxRetries), got %d", maxRetries+1, calls) + } +} + +func TestRetryK8sOpsNoRetryOnNonTransientError(t *testing.T) { + calls := 0 + permanentErr := errors.New("validation failed") + err := retryK8sOps(2*time.Second, func(ctx context.Context) error { + calls++ + return permanentErr + }) + if !errors.Is(err, permanentErr) { + t.Fatalf("expected %v, got %v", permanentErr, err) + } + if calls != 1 { + t.Errorf("expected 1 call (no retry), got %d", calls) + } +} + +func TestRetryK8sOpsExhaustsDeadline(t *testing.T) { + calls := 0 + err := retryK8sOps(1*time.Millisecond, func(ctx context.Context) error { + calls++ + return apierrors.NewServiceUnavailable("unavailable") + }) + if err == nil { + t.Fatal("expected error, got nil") + } + if calls != maxRetries+1 { + t.Errorf("expected %d calls, got %d", maxRetries+1, calls) + } + if !strings.Contains(err.Error(), "unavailable") { + t.Errorf("expected 'unavailable' in error, got %v", err) + } +} diff --git a/internal/cniipam/ipam.go b/internal/cniipam/ipam.go new file mode 100644 index 00000000..46310173 --- /dev/null +++ b/internal/cniipam/ipam.go @@ -0,0 +1,276 @@ +// Copyright 2025 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +// Package cniipam holds the IPAM allocation/deallocation logic shared by +// every master plugin in the galactic CNI chain (galactic-cni, veth; +// galactic-tap-cni, tap) — interface-agnostic, since neither kernel state +// nor network namespaces are touched here (galactic-cni's own +// configureInterfaceInNetns applies the result to the guest veth; tap mode +// applies nothing, the VM manages its own interface). +// +// This package is a plain library today, imported directly by the master +// plugins, not yet a delegated CNI IPAM plugin of its own — that lands in a +// follow-up step (cmd/galactic-ipam, the CNI IPAM delegation protocol, and +// dropping the getAllocatedSubnetsFromCRD dependency below in favor of local +// marker-file persistence for the IPv6 pool allocator, mirroring +// IPv4PoolAllocator's). Landing this as its own package now — instead of +// duplicating it between galactic-cni and galactic-tap-cni — means that +// follow-up step only has to add the delegation wiring, not first +// de-duplicate two drifted copies. +package cniipam + +import ( + "context" + "errors" + "fmt" + "log/slog" + "net" + "time" + + "github.com/containernetworking/cni/pkg/skel" + 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/cni/ipam" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +// cniTimeout bounds the k8s API call getAllocatedSubnetsFromCRD makes while +// deallocating. Mirrors internal/cni's own cniTimeout — small enough that a +// duplicated constant beats a shared package for this alone. +const cniTimeout = 10 * time.Second + +// TypeStatic is the ipam.type value for a single pre-assigned static +// address. Any other (or empty) value takes the pool-based dual-stack path +// — see WantsIPAM/Allocate. +const TypeStatic = "static" + +// localIPAMDefaultPool is the IPv6 CIDR pool used when local IPAM is enabled +// but IPv6Subnet is unset in the CNI config. Allocations from it use +// ipam.DefaultSubnetLen (/96). +const localIPAMDefaultPool = "fd00:10:ff01::/64" + +// IPAM holds IP address management configuration passed in the CNI config's +// "ipam" block. +type IPAM struct { + Type string `json:"type"` // "pool" (default) or "static" + StaticIP string `json:"static_ip,omitempty"` // used when type="static" + Routes []Route `json:"routes,omitempty"` + Addresses []Address `json:"addresses,omitempty"` +} + +// Route describes a static route to install. +type Route struct { + Dst string `json:"dst"` + GW string `json:"gw,omitempty"` +} + +// Address describes a static IP address assignment. +type Address struct { + Address string `json:"address"` +} + +// IPAMResult holds the IPAM allocation details a caller uses to build its +// own CNI result and, for veth, to configure the guest interface. +// IPv4Address/IPv4Gateway are nil when the attachment is IPv6-only. +type IPAMResult struct { + IPv6Subnet *net.IPNet + IPv6Gateway net.IP + IPv4Address net.IP + IPv4Gateway net.IP + Routes []*net.IPNet +} + +// AllocConfig carries the subset of a caller's own CNI config that IPAM +// allocation needs. Each master plugin passes its own values in — this +// package doesn't know, or need to know, about the rest of that config's +// shape. +type AllocConfig struct { + VPC string + VPCAttachment string + Namespace string + IPAM *IPAM + IPv6Subnet string + IPv4Subnet string + AddressFamilies []string +} + +// ipv4LockDir is the IPv4PoolAllocator lock/state directory used by +// allocatePoolIPAM/Deallocate. Overridable in tests so unit tests never +// touch the real production path. +var ipv4LockDir = ipam.DefaultIPv4LockDir + +// enableLocalIPAM controls whether allocation proceeds when no explicit +// "ipam" block is present in the CNI config. Defaults to false. +var enableLocalIPAM bool + +// SetEnableLocalIPAM sets the local IPAM flag from the CLI/env. +func SetEnableLocalIPAM(v bool) { + enableLocalIPAM = v +} + +// WantsIPAM reports whether cfg should trigger IPAM allocation at all. Four +// independent signals opt in: an explicit "static" IPAM type, a configured +// IPv6Subnet or IPv4Subnet (the NAD-driven pool-IPAM path, either family +// alone or both), or the enable-local-ipam dev fallback. A config with none +// of these (e.g. a tap workload that manages its own addressing) allocates +// nothing, matching the CNI plugin's own longstanding behavior of skipping +// IPAM entirely rather than erroring. +func WantsIPAM(cfg AllocConfig) bool { + if cfg.IPAM != nil && cfg.IPAM.Type == TypeStatic { + return true + } + return cfg.IPv6Subnet != "" || cfg.IPv4Subnet != "" || enableLocalIPAM +} + +// Allocate allocates addresses for the given container. This is +// interface-agnostic — it does not touch any kernel state or network +// namespaces. Returns (nil, nil) when WantsIPAM reports no allocation is +// requested. When enableLocalIPAM is true and IPv6Subnet is unset, falls +// back to a built-in default IPv6 pool CIDR. +func Allocate(args *skel.CmdArgs, cfg AllocConfig) (*IPAMResult, error) { + if !WantsIPAM(cfg) { + return nil, nil + } + + if cfg.IPAM != nil && cfg.IPAM.Type == TypeStatic { + return allocateStatic(args, cfg.IPAM) + } + + return allocatePool(args, cfg) +} + +// allocateStatic validates and returns the pre-assigned static IPv6 address +// from the "static" IPAM block. No IPv4 address is ever allocated for +// static IPAM — it is a single fixed address, not a dual-stack pool. +func allocateStatic(args *skel.CmdArgs, ipamConf *IPAM) (*IPAMResult, error) { + alloc := ipam.NewStaticAllocator() + allocIP, err := alloc.Allocate(args.ContainerID, ipamConf.StaticIP) + if err != nil { + return nil, fmt.Errorf("allocate static IP: %w", err) + } + subnet := &net.IPNet{ + IP: allocIP, + Mask: net.CIDRMask(64, 128), + } + slog.Debug("IPAM: allocated static", "containerID", args.ContainerID, "subnet", subnet) + return &IPAMResult{IPv6Subnet: subnet}, nil +} + +// allocatePool allocates a dual-stack, IPv6-only, or IPv4-only pool-based +// endpoint address for the given container, via ipam.DualStackAllocator. +// IPv6Subnet and IPv4Subnet each independently supply a pool CIDR for their +// family; at least one must be set (falling back to localIPAMDefaultPool for +// IPv6 when enableLocalIPAM and both are unset). +func allocatePool(args *skel.CmdArgs, cfg AllocConfig) (*IPAMResult, error) { + ipv6Pool := cfg.IPv6Subnet + if ipv6Pool == "" && cfg.IPv4Subnet == "" { + if !enableLocalIPAM { + return nil, errors.New("ipv6_subnet or ipv4_subnet is required (or enable local IPAM)") + } + ipv6Pool = localIPAMDefaultPool + } + + alloc, err := ipam.NewDualStackAllocator(ipv6Pool, "", cfg.IPv4Subnet, "", ipv4LockDir) + if err != nil { + return nil, fmt.Errorf("create dual-stack allocator: %w", err) + } + + res, err := alloc.Allocate(args.ContainerID) + if err != nil { + return nil, fmt.Errorf("allocate dual-stack addresses: %w", err) + } + + var routes []*net.IPNet + if res.IPv6Subnet != nil { + routes = append(routes, &net.IPNet{IP: net.IPv6zero, Mask: net.CIDRMask(0, 128)}) + } + if res.IPv4Address != nil { + routes = append(routes, &net.IPNet{IP: net.IPv4zero, Mask: net.CIDRMask(0, 32)}) + } + + slog.Debug("IPAM: allocated", "containerID", args.ContainerID, + "ipv6Subnet", res.IPv6Subnet, "ipv6Gateway", res.IPv6Gateway, + "ipv4Address", res.IPv4Address, "ipv4Gateway", res.IPv4Gateway) + + return &IPAMResult{ + IPv6Subnet: res.IPv6Subnet, + IPv6Gateway: res.IPv6Gateway, + IPv4Address: res.IPv4Address, + IPv4Gateway: res.IPv4Gateway, + Routes: routes, + }, nil +} + +// Deallocate releases the IPAM allocation for the given container. Reads the +// allocated IPv6 subnet and (if present) IPv4 address from the +// BGPAdvertisement CRD annotations galactic-bgp wrote, then deallocates each +// independently and non-fatally: a missing annotation for one family (e.g. a +// pre-existing v6-only pod, or a partial ADD failure that never reached IPv4 +// allocation) must not prevent cleanup of the other. +func Deallocate(args *skel.CmdArgs, cfg AllocConfig, k8s client.Client) { + if cfg.IPAM != nil && cfg.IPAM.Type == TypeStatic { + // Static allocations don't need deallocation. + return + } + + ipv6Subnet, ipv4Addr := getAllocatedSubnetsFromCRD(args.ContainerID, cfg, k8s) + if ipv6Subnet == "" && ipv4Addr == "" { + // No allocation found — either allocation was never completed, + // or the advertisement was already deleted. Nothing to clean up. + slog.Debug("IPAM: no allocation found to deallocate", "containerID", args.ContainerID) + return + } + + if ipv6Subnet != "" { + ipv6Pool := cfg.IPv6Subnet + if ipv6Pool == "" && enableLocalIPAM { + ipv6Pool = localIPAMDefaultPool + } + pa, err := ipam.NewPoolAllocator(ipv6Pool, "", 0) + if err != nil { + slog.Warn("IPAM: failed to build IPv6 pool allocator for deallocation, skipping", "err", err, + "containerID", args.ContainerID, "subnet", ipv6Subnet) + } else { + pa.Deallocate(ipv6Subnet) + slog.Debug("IPAM: deallocated IPv6", "containerID", args.ContainerID, "subnet", ipv6Subnet) + } + } + + if ipv4Addr != "" { + if cfg.IPv4Subnet == "" { + slog.Warn("IPAM: found allocated IPv4 address but no ipv4_subnet in config, skipping deallocation", + "containerID", args.ContainerID, "address", ipv4Addr) + } else if pa, err := ipam.NewIPv4PoolAllocator(cfg.IPv4Subnet, "", ipv4LockDir); err != nil { + slog.Warn("IPAM: failed to build IPv4 pool allocator for deallocation, skipping", "err", err, + "containerID", args.ContainerID, "address", ipv4Addr) + } else { + pa.Deallocate(ipv4Addr) + slog.Debug("IPAM: deallocated IPv4", "containerID", args.ContainerID, "address", ipv4Addr) + } + } +} + +// getAllocatedSubnetsFromCRD reads the allocated IPv6 subnet and (if +// present) IPv4 address for the given container from the BGPAdvertisement +// CRD annotations. Either return value is empty when not found. +func getAllocatedSubnetsFromCRD( + containerID string, cfg AllocConfig, k8s client.Client, +) (ipv6Subnet, ipv4Addr string) { + ctx, cancel := context.WithTimeout(context.Background(), cniTimeout) + defer cancel() + + adv := &bgpv1alpha1.BGPAdvertisement{ + ObjectMeta: metav1.ObjectMeta{ + Name: crdnames.BGPAdvertisementName(cfg.VPC, cfg.VPCAttachment), + Namespace: cfg.Namespace, + }, + } + if err := k8s.Get(ctx, client.ObjectKeyFromObject(adv), adv); err != nil { + return "", "" + } + + return adv.Annotations[crdnames.SubnetKeyIPv6(containerID)], adv.Annotations[crdnames.SubnetKeyIPv4(containerID)] +} diff --git a/internal/cniipam/ipam_test.go b/internal/cniipam/ipam_test.go new file mode 100644 index 00000000..c8d2ee22 --- /dev/null +++ b/internal/cniipam/ipam_test.go @@ -0,0 +1,397 @@ +// Copyright 2025 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cniipam + +import ( + "net" + "testing" + + "github.com/containernetworking/cni/pkg/skel" + 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" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "go.datum.net/galactic/internal/cni/crdnames" + "go.datum.net/galactic/internal/cni/ipam" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +const ( + testVPC = "abc" + testAttachment = "def" + testContainerID = "test-container" + testNamespace = "galactic-system" + testIPv4Subnet = "10.128.0.0/20" +) + +var testScheme = func() *runtime.Scheme { + s := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(s)) + utilruntime.Must(bgpv1alpha1.AddToScheme(s)) + return s +}() + +func fakeClient(objs ...client.Object) client.Client { + return fake.NewClientBuilder().WithScheme(testScheme).WithObjects(objs...).Build() +} + +// ---- WantsIPAM ------------------------------------------------------------- + +func TestWantsIPAM(t *testing.T) { + original := enableLocalIPAM + defer func() { enableLocalIPAM = original }() + + tests := []struct { + name string + cfg AllocConfig + enableLocalIPA bool + want bool + }{ + { + name: "no ipam block, no ipv6_subnet, local IPAM disabled", + cfg: AllocConfig{}, + want: false, + }, + { + name: "static ipam type opts in regardless of other fields", + cfg: AllocConfig{IPAM: &IPAM{Type: TypeStatic}}, + want: true, + }, + { + name: "ipv6_subnet set opts in", + cfg: AllocConfig{IPv6Subnet: localIPAMDefaultPool}, + want: true, + }, + { + name: "ipv4_subnet set opts in", + cfg: AllocConfig{IPv4Subnet: testIPv4Subnet}, + want: true, + }, + { + name: "local IPAM enabled opts in even without ipv6_subnet", + cfg: AllocConfig{}, + enableLocalIPA: true, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + enableLocalIPAM = tt.enableLocalIPA + if got := WantsIPAM(tt.cfg); got != tt.want { + t.Errorf("WantsIPAM(%+v) = %v, want %v", tt.cfg, got, tt.want) + } + }) + } +} + +func TestSetEnableLocalIPAM(t *testing.T) { + original := enableLocalIPAM + defer func() { enableLocalIPAM = original }() + + enableLocalIPAM = false + if enableLocalIPAM { + t.Error("enableLocalIPAM default = true, want false") + } + + SetEnableLocalIPAM(true) + if !enableLocalIPAM { + t.Error("enableLocalIPAM after SetEnableLocalIPAM(true) = false, want true") + } + + SetEnableLocalIPAM(false) + if enableLocalIPAM { + t.Error("enableLocalIPAM after SetEnableLocalIPAM(false) = true, want false") + } +} + +// ---- Allocate --------------------------------------------------------- + +func TestAllocateNoAllocation(t *testing.T) { + original := enableLocalIPAM + defer func() { enableLocalIPAM = original }() + enableLocalIPAM = false + + args := &skel.CmdArgs{ContainerID: testContainerID} + res, err := Allocate(args, AllocConfig{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res != nil { + t.Errorf("Allocate() = %+v, want nil (WantsIPAM should have been false)", res) + } +} + +func TestAllocateStatic(t *testing.T) { + args := &skel.CmdArgs{ContainerID: testContainerID} + cfg := AllocConfig{IPAM: &IPAM{Type: TypeStatic, StaticIP: "fd00:10:ff01::1234"}} + + res, err := Allocate(args, cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res == nil { + t.Fatal("Allocate() = nil, want a result") + } + if res.IPv6Subnet == nil || !res.IPv6Subnet.IP.Equal(net.ParseIP("fd00:10:ff01::1234")) { + t.Errorf("IPv6Subnet = %v, want fd00:10:ff01::1234", res.IPv6Subnet) + } + if res.IPv4Address != nil { + t.Errorf("IPv4Address = %v, want nil for static IPAM", res.IPv4Address) + } +} + +func TestAllocatePoolIPv6Only(t *testing.T) { + args := &skel.CmdArgs{ContainerID: testContainerID} + cfg := AllocConfig{IPv6Subnet: localIPAMDefaultPool} + + res, err := Allocate(args, cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res == nil { + t.Fatal("Allocate() = nil, want a result") + } + if res.IPv6Subnet == nil { + t.Fatal("IPv6Subnet = nil, want an allocated /96") + } + if ones, bits := res.IPv6Subnet.Mask.Size(); ones != 96 || bits != 128 { + t.Errorf("IPv6Subnet mask = /%d, want /96", ones) + } + if res.IPv6Gateway == nil { + t.Error("IPv6Gateway = nil, want the pool's default gateway (::1 of the /64)") + } + if res.IPv4Address != nil { + t.Errorf("IPv4Address = %v, want nil (no ipv4_subnet configured)", res.IPv4Address) + } + if len(res.Routes) != 1 { + t.Errorf("Routes = %v, want exactly one default IPv6 route", res.Routes) + } +} + +func TestAllocatePoolIPv4Only(t *testing.T) { + origLockDir := ipv4LockDir + ipv4LockDir = t.TempDir() + defer func() { ipv4LockDir = origLockDir }() + + args := &skel.CmdArgs{ContainerID: testContainerID} + cfg := AllocConfig{IPv4Subnet: testIPv4Subnet} + + res, err := Allocate(args, cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res == nil { + t.Fatal("Allocate() = nil, want a result") + } + if res.IPv6Subnet != nil { + t.Errorf("IPv6Subnet = %v, want nil (no ipv6_subnet configured)", res.IPv6Subnet) + } + if res.IPv4Address == nil { + t.Fatal("IPv4Address = nil, want an allocated /32") + } + if res.IPv4Gateway == nil { + t.Error("IPv4Gateway = nil, want the pool's default gateway") + } + if len(res.Routes) != 1 { + t.Errorf("Routes = %v, want exactly one default IPv4 route", res.Routes) + } +} + +func TestAllocatePoolDualStack(t *testing.T) { + origLockDir := ipv4LockDir + ipv4LockDir = t.TempDir() + defer func() { ipv4LockDir = origLockDir }() + + args := &skel.CmdArgs{ContainerID: testContainerID} + cfg := AllocConfig{ + IPv6Subnet: localIPAMDefaultPool, + IPv4Subnet: testIPv4Subnet, + } + + res, err := Allocate(args, cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res == nil { + t.Fatal("Allocate() = nil, want a result") + } + if res.IPv6Subnet == nil { + t.Error("IPv6Subnet = nil, want an allocated /96") + } + if res.IPv4Address == nil { + t.Fatal("IPv4Address = nil, want an allocated /32") + } + if res.IPv4Gateway == nil { + t.Error("IPv4Gateway = nil, want the pool's default gateway") + } + if len(res.Routes) != 2 { + t.Errorf("Routes = %v, want one default route per family", res.Routes) + } +} + +func TestAllocatePoolMissingBothSubnetsErrors(t *testing.T) { + original := enableLocalIPAM + defer func() { enableLocalIPAM = original }() + enableLocalIPAM = false + + args := &skel.CmdArgs{ContainerID: testContainerID} + _, err := allocatePool(args, AllocConfig{}) + if err == nil { + t.Fatal("expected error when both ipv6_subnet and ipv4_subnet are unset and local IPAM is disabled, got nil") + } +} + +// ---- Deallocate --------------------------------------------------------- + +func TestDeallocateStaticNoop(t *testing.T) { + // Static IPAM never wrote a CRD annotation for DEL to look up, and + // Deallocate must return immediately without attempting a k8s lookup. + cfg := AllocConfig{ + VPC: testVPC, VPCAttachment: testAttachment, + IPAM: &IPAM{Type: TypeStatic}, + } + args := &skel.CmdArgs{ContainerID: testContainerID} + // A nil client would panic if Deallocate tried to use it; passing nil + // here asserts the static-type early return happens first. + Deallocate(args, cfg, nil) +} + +func TestDeallocateDualStack(t *testing.T) { + origLockDir := ipv4LockDir + ipv4LockDir = t.TempDir() + defer func() { ipv4LockDir = origLockDir }() + + cfg := AllocConfig{ + VPC: testVPC, VPCAttachment: testAttachment, + Namespace: testNamespace, + IPv6Subnet: localIPAMDefaultPool, + IPv4Subnet: testIPv4Subnet, + } + args := &skel.CmdArgs{ContainerID: testContainerID} + + alloc, err := ipam.NewDualStackAllocator(cfg.IPv6Subnet, "", cfg.IPv4Subnet, "", ipv4LockDir) + if err != nil { + t.Fatalf("NewDualStackAllocator: %v", err) + } + dsRes, err := alloc.Allocate(args.ContainerID) + if err != nil { + t.Fatalf("Allocate: %v", err) + } + + ipv4Pool, err := ipam.NewIPv4PoolAllocator(cfg.IPv4Subnet, "", ipv4LockDir) + if err != nil { + t.Fatalf("NewIPv4PoolAllocator: %v", err) + } + if !ipv4Pool.IsAllocated(dsRes.IPv4Address.String()) { + t.Fatalf("setup: IPv4 address %s not marked allocated", dsRes.IPv4Address) + } + + adv := &bgpv1alpha1.BGPAdvertisement{ + ObjectMeta: metav1.ObjectMeta{ + Name: crdnames.BGPAdvertisementName(cfg.VPC, cfg.VPCAttachment), + Namespace: testNamespace, + Annotations: map[string]string{ + crdnames.SubnetKeyIPv6(args.ContainerID): dsRes.IPv6Subnet.String(), + crdnames.SubnetKeyIPv4(args.ContainerID): dsRes.IPv4Address.String(), + }, + }, + } + k8s := fakeClient(adv) + + Deallocate(args, cfg, k8s) + + if ipv4Pool.IsAllocated(dsRes.IPv4Address.String()) { + t.Errorf("IPv4 address %s still marked allocated after Deallocate", dsRes.IPv4Address) + } +} + +func TestDeallocateIPv4Only(t *testing.T) { + origLockDir := ipv4LockDir + ipv4LockDir = t.TempDir() + defer func() { ipv4LockDir = origLockDir }() + + cfg := AllocConfig{ + VPC: testVPC, VPCAttachment: testAttachment, + Namespace: testNamespace, + IPv4Subnet: testIPv4Subnet, + } + args := &skel.CmdArgs{ContainerID: testContainerID} + + ipv4Pool, err := ipam.NewIPv4PoolAllocator(cfg.IPv4Subnet, "", ipv4LockDir) + if err != nil { + t.Fatalf("NewIPv4PoolAllocator: %v", err) + } + ipv4Addr, err := ipv4Pool.Allocate(args.ContainerID) + if err != nil { + t.Fatalf("Allocate: %v", err) + } + if !ipv4Pool.IsAllocated(ipv4Addr.String()) { + t.Fatalf("setup: IPv4 address %s not marked allocated", ipv4Addr) + } + + adv := &bgpv1alpha1.BGPAdvertisement{ + ObjectMeta: metav1.ObjectMeta{ + Name: crdnames.BGPAdvertisementName(cfg.VPC, cfg.VPCAttachment), + Namespace: testNamespace, + Annotations: map[string]string{ + // No IPv6 annotation — this is an IPv4-only allocation. + crdnames.SubnetKeyIPv4(args.ContainerID): ipv4Addr.String(), + }, + }, + } + k8s := fakeClient(adv) + + // Must not panic despite no ipv6_subnet in config, and must deallocate + // the IPv4 address. + Deallocate(args, cfg, k8s) + + if ipv4Pool.IsAllocated(ipv4Addr.String()) { + t.Errorf("IPv4 address %s still marked allocated after Deallocate", ipv4Addr) + } +} + +func TestDeallocatePartialAllocationNonFatal(t *testing.T) { + // A v6-only pod (no IPv4 annotation, e.g. pre-existing or a partial ADD + // failure) must still have its IPv6 side cleaned up without erroring, + // and must not attempt to touch a nonexistent IPv4 pool. + cfg := AllocConfig{ + VPC: testVPC, VPCAttachment: testAttachment, + Namespace: testNamespace, + IPv6Subnet: localIPAMDefaultPool, + // IPv4Subnet intentionally unset. + } + args := &skel.CmdArgs{ContainerID: testContainerID} + + adv := &bgpv1alpha1.BGPAdvertisement{ + ObjectMeta: metav1.ObjectMeta{ + Name: crdnames.BGPAdvertisementName(cfg.VPC, cfg.VPCAttachment), + Namespace: testNamespace, + Annotations: map[string]string{ + crdnames.SubnetKeyIPv6(args.ContainerID): "fd00:10:ff01::1234/96", + }, + }, + } + k8s := fakeClient(adv) + + // Must not panic despite no ipv4_subnet in config. + Deallocate(args, cfg, k8s) +} + +func TestDeallocateNoAllocationFound(t *testing.T) { + cfg := AllocConfig{ + VPC: testVPC, VPCAttachment: testAttachment, + Namespace: testNamespace, + } + args := &skel.CmdArgs{ContainerID: testContainerID} + // No BGPAdvertisement exists at all. + k8s := fakeClient() + + // Must return cleanly with nothing to deallocate. + Deallocate(args, cfg, k8s) +} diff --git a/internal/cnitap/cnitap.go b/internal/cnitap/cnitap.go new file mode 100644 index 00000000..a8c2ccd4 --- /dev/null +++ b/internal/cnitap/cnitap.go @@ -0,0 +1,30 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnitap + +import ( + "time" + + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/version" + + "go.datum.net/galactic/internal/metadata" +) + +const cniTimeout = 10 * time.Second + +// RunPlugin starts the CNI plugin, handling ADD, DEL, CHECK, and STATUS operations. +func RunPlugin() { + skel.PluginMainFuncs( + skel.CNIFuncs{ + Add: cmdAdd, + Check: cmdCheck, + Del: cmdDel, + Status: cmdStatus, + }, + version.All, + "CNI galactic-tap plugin "+metadata.Version, + ) +} diff --git a/internal/cnitap/cnitap_test.go b/internal/cnitap/cnitap_test.go new file mode 100644 index 00000000..c8a98a63 --- /dev/null +++ b/internal/cnitap/cnitap_test.go @@ -0,0 +1,300 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnitap + +import ( + "errors" + "fmt" + "net" + "os" + "strings" + "testing" + + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/types" + + "go.datum.net/galactic/internal/cniipam" +) + +const ( + testVPC = "abc" + testAttachment = "def" + testContainerID = "test-container" + testInvalidBase62 = "abc-def" + testCNIVersion = "1.0.0" +) + +func TestMain(m *testing.M) { + _ = os.Setenv("GALACTIC_CNI_NODE_NAME", "test-node") + InitCNIConfig() + os.Exit(m.Run()) +} + +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) + } +} + +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 +} + +// ---- parseConf ----------------------------------------------------------- + +func TestParseConf(t *testing.T) { + tests := []struct { + name string + input string + wantVPC string + wantErr string + wantCode uint + }{ + { + name: "valid config", + input: fmt.Sprintf( + `{"cniVersion":"1.0.0","name":"test","type":"galactic-tap-cni","vpc":"%s","vpcattachment":"%s"}`, + testVPC, testAttachment, + ), + wantVPC: testVPC, + }, + {name: "invalid JSON", input: "not json", wantErr: "invalid CNI config", wantCode: 7}, + { + name: "missing vpc", + input: fmt.Sprintf(`{"cniVersion":"1.0.0","name":"test","type":"galactic-tap-cni","vpcattachment":"%s"}`, + testAttachment), + wantErr: "vpc is required and must be a non-empty base62 string", + wantCode: 7, + }, + { + name: "vpc with invalid char", + input: fmt.Sprintf(`{"cniVersion":"1.0.0","name":"test","type":"galactic-tap-cni","vpc":"%s","vpcattachment":"%s"}`, + testInvalidBase62, testAttachment), + wantErr: fmt.Sprintf("invalid base62 value for field 'vpc': %q", testInvalidBase62), + wantCode: 7, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + conf, err := parseConf([]byte(tt.input)) + if tt.wantErr != "" { + if err == nil { + t.Fatalf("expected error containing %q, got nil", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("error %q does not contain %q", err, tt.wantErr) + } + if tt.wantCode > 0 { + assertCNIError(t, err, tt.wantCode, tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if conf.VPC != tt.wantVPC { + t.Errorf("VPC = %q, want %q", conf.VPC, tt.wantVPC) + } + }) + } +} + +// ---- buildTapResult ------------------------------------------------------ + +func TestBuildTapResult(t *testing.T) { + subnet := mustParseCIDR(t, "fd00:10:ff01::1234/80") + gateway := net.ParseIP("fd00:10:ff01::1") + defaultRoute := mustParseCIDR(t, "::/0") + + conf := &PluginConf{ + PluginConf: types.PluginConf{CNIVersion: testCNIVersion}, + VPC: testVPC, + VPCAttachment: testAttachment, + } + + tests := []struct { + name string + ipRes *cniipam.IPAMResult + wantIPs int + wantRoutes int + }{ + { + name: "with IPAM config", + ipRes: &cniipam.IPAMResult{IPv6Subnet: subnet, IPv6Gateway: gateway, Routes: []*net.IPNet{defaultRoute}}, + wantIPs: 1, + wantRoutes: 1, + }, + {name: "without IPAM config", ipRes: nil, wantIPs: 0, wantRoutes: 0}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := buildTapResult(conf, tt.ipRes, "H0abc123", "aa:bb:cc:dd:ee:ff", 1500) + + if result.CNIVersion != testCNIVersion { + t.Errorf("CNIVersion = %q, want %q", result.CNIVersion, testCNIVersion) + } + if len(result.Interfaces) != 1 { + t.Fatalf("Interfaces count = %d, want 1", len(result.Interfaces)) + } + if result.Interfaces[0].Name != "H0abc123" { + t.Errorf("Interfaces[0].Name = %q, want %q", result.Interfaces[0].Name, "H0abc123") + } + if result.Interfaces[0].Sandbox != "" { + t.Errorf("Interfaces[0].Sandbox = %q, want empty", result.Interfaces[0].Sandbox) + } + if len(result.IPs) != tt.wantIPs { + t.Errorf("IPs count = %d, want %d", len(result.IPs), tt.wantIPs) + } + if tt.wantIPs > 0 { + if result.IPs[0].Interface == nil || *result.IPs[0].Interface != 0 { + t.Errorf("IPs[0].Interface = %v, want 0", result.IPs[0].Interface) + } + } + if len(result.Routes) != tt.wantRoutes { + t.Errorf("Routes count = %d, want %d", len(result.Routes), tt.wantRoutes) + } + }) + } +} + +// TestBuildTapResultIPv4Mask verifies that buildTapResult reports the IPv4 +// address with a /25 mask (matching the host gateway mask +// cnibgp.ConfigureHostGateway installs on the tap interface), not the /32 +// used for veth. +func TestBuildTapResultIPv4Mask(t *testing.T) { + ipv4Address := net.ParseIP("172.20.1.5") + ipv4Gateway := net.ParseIP("172.20.1.1") + ipv4Route := mustParseCIDR(t, "0.0.0.0/0") + + conf := &PluginConf{ + PluginConf: types.PluginConf{CNIVersion: testCNIVersion}, + VPC: testVPC, + VPCAttachment: testAttachment, + } + ipRes := &cniipam.IPAMResult{IPv4Address: ipv4Address, IPv4Gateway: ipv4Gateway, Routes: []*net.IPNet{ipv4Route}} + + result := buildTapResult(conf, ipRes, "H0abc123", "aa:bb:cc:dd:ee:ff", 1500) + + if len(result.IPs) != 1 { + t.Fatalf("IPs count = %d, want 1", len(result.IPs)) + } + wantIPv4Mask := net.CIDRMask(25, 32).String() + if result.IPs[0].Address.IP.String() != ipv4Address.String() || result.IPs[0].Address.Mask.String() != wantIPv4Mask { + t.Errorf("IPs[0].Address = %v, want %s/25", result.IPs[0].Address, ipv4Address) + } +} + +// TestBuildTapResultHostNetns verifies that the tap path produces a valid +// CNI result when args.Netns is the host network namespace. Kraftlet/ +// unikraft workloads pass the host netns because they don't have a Linux +// network namespace. main.go's own CNI_NETNS_OVERRIDE handles bypassing the +// CNI library's same-netns rejection check. The tap result must not +// reference a sandbox. +func TestBuildTapResultHostNetns(t *testing.T) { + subnet := mustParseCIDR(t, "fd00:10:ff01::1234/80") + gateway := net.ParseIP("fd00:10:ff01::1") + defaultRoute := mustParseCIDR(t, "::/0") + + conf := &PluginConf{ + PluginConf: types.PluginConf{CNIVersion: testCNIVersion}, + VPC: testVPC, + VPCAttachment: testAttachment, + } + ipRes := &cniipam.IPAMResult{IPv6Subnet: subnet, IPv6Gateway: gateway, Routes: []*net.IPNet{defaultRoute}} + + result := buildTapResult(conf, ipRes, "H0abc123", "aa:bb:cc:dd:ee:ff", 1500) + + if result.Interfaces[0].Sandbox != "" { + t.Errorf("Interfaces[0].Sandbox = %q, want empty (host netns, no sandbox)", result.Interfaces[0].Sandbox) + } +} + +// ---- cmdDel / cmdCheck / cmdStatus ---------------------------------------- + +func TestCmdDelIdempotent(t *testing.T) { + args := &skel.CmdArgs{ContainerID: testContainerID, StdinData: []byte("not valid json")} + if err := cmdDel(args); err != nil { + t.Fatalf("cmdDel with invalid config returned error = %v, want nil", err) + } +} + +func TestCmdDelIdempotentMissingResources(t *testing.T) { + conf := fmt.Sprintf(`{"cniVersion":"1.0.0","name":"test","type":"galactic-tap-cni","vpc":"%s","vpcattachment":"%s"}`, + testVPC, testAttachment) + args := &skel.CmdArgs{ContainerID: testContainerID, StdinData: []byte(conf)} + if err := cmdDel(args); err != nil { + t.Fatalf("cmdDel with missing resources returned error = %v, want nil", err) + } +} + +func TestCmdCheckInvalidConfig(t *testing.T) { + args := &skel.CmdArgs{ContainerID: testContainerID, StdinData: []byte("not valid json")} + err := cmdCheck(args) + if err == nil || !strings.Contains(err.Error(), "invalid CNI config") { + t.Fatalf("expected 'invalid CNI config' error, got: %v", err) + } +} + +func TestCmdCheckValidConfigMissingResources(t *testing.T) { + conf := fmt.Sprintf(`{"cniVersion":"1.0.0","name":"test","type":"galactic-tap-cni","vpc":"%s","vpcattachment":"%s"}`, + testVPC, testAttachment) + args := &skel.CmdArgs{ContainerID: testContainerID, StdinData: []byte(conf)} + err := cmdCheck(args) + if err == nil || !strings.Contains(err.Error(), "CHECK failed") { + t.Fatalf("expected 'CHECK failed', got: %v", err) + } +} + +func TestCmdStatusInvalidConfig(t *testing.T) { + args := &skel.CmdArgs{ContainerID: testContainerID, StdinData: []byte("not valid json")} + err := cmdStatus(args) + assertCNIError(t, err, 7, "invalid CNI config") +} + +func TestCmdStatusAPIProbeFailure(t *testing.T) { + original := probeAPIServer + probeAPIServer = func() error { return errors.New("connection refused") } + defer func() { probeAPIServer = original }() + + conf := fmt.Sprintf(`{"cniVersion":"1.0.0","name":"test","type":"galactic-tap-cni","vpc":"%s","vpcattachment":"%s"}`, + testVPC, testAttachment) + args := &skel.CmdArgs{ContainerID: testContainerID, StdinData: []byte(conf)} + err := cmdStatus(args) + assertCNIError(t, err, 50, "API server health check failed") +} + +// ---- resourceTracker ------------------------------------------------------ + +func TestResourceTrackerCleanupZeroValue(t *testing.T) { + tracker := &resourceTracker{} + tracker.cleanup(t.Context()) // should not panic +} + +// ---- loadHostConf / logging ----------------------------------------------- + +func TestLoadHostConfMissingFile(t *testing.T) { + conf, err := loadHostConf("/nonexistent/path/10-galactic.conflist") + if err != nil { + t.Fatalf("unexpected error for missing conflist: %v", err) + } + if conf.Namespace == "" { + t.Error("Namespace = empty, want default namespace") + } +} diff --git a/internal/cnitap/config.go b/internal/cnitap/config.go new file mode 100644 index 00000000..e399a502 --- /dev/null +++ b/internal/cnitap/config.go @@ -0,0 +1,345 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnitap + +import ( + "encoding/json" + "errors" + "fmt" + "log/slog" + "net" + "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/cniipam" + "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-tap-cni/main.go). +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 sanitizeForErrorBinary = "" + +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 ( + maxIPv6SubnetPrefixLen = 96 + maxIPv4SubnetPrefixLen = 32 +) + +const ( + addressFamilyIPv6 = "ipv6" + addressFamilyIPv4 = "ipv4" +) + +// 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. Matching is +// case-insensitive. An empty string resolves to config.DefaultLogLevel. +// Unrecognized values return an error alongside the info-level fallback, so +// callers can warn without failing the CNI operation over a typo'd setting. +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. If opening the file fails, it logs a +// warning to os.Stderr and falls back. An unrecognized logLevel also logs a +// warning and falls back to config.DefaultLogLevel rather than failing the +// operation. +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 +} + +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 validatePrevResultAdd(res types.Result) error { + if res == nil { + return nil + } + jsonBytes, err := json.Marshal(res) + if err != nil { + return fmt.Errorf("marshal prevResult: %w", err) + } + result, err := type100.NewResult(jsonBytes) + if err != nil { + return fmt.Errorf("parse prevResult: %w", err) + } + versioned, err := type100.GetResult(result) + if err != nil { + return fmt.Errorf("get prevResult version: %w", err) + } + if len(versioned.Interfaces) == 0 && len(versioned.IPs) == 0 { + return errors.New("prevResult declares no interfaces or IP assignments") + } + return nil +} + +// parseConf unmarshals the CNI configuration from stdin data and validates +// the base62-encoded identifier fields. It resolves the host configuration +// and sets up process environment variables 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("NODE_NAME", cniConfig.NodeName) + _ = 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)) + + localIPAM := config.CNIGetEnableLocalIPAM() + cniipam.SetEnableLocalIPAM(localIPAM) + if localIPAM && conf.IPAM == nil { + return nil, &types.Error{Code: 7, Msg: "local IPAM is enabled, but no 'ipam' block is present in the configuration"} + } + + if conf.IPv6Subnet != "" { + ip, mask, err := net.ParseCIDR(conf.IPv6Subnet) + if err != nil { + return nil, &types.Error{Code: 7, Msg: fmt.Sprintf( + "invalid CIDR value for field 'ipv6_subnet': %q", sanitizeForError(conf.IPv6Subnet)), + } + } + if ip.To4() != nil { + return nil, &types.Error{Code: 7, Msg: fmt.Sprintf( + "ipv6_subnet must be an IPv6 CIDR, got IPv4: %q", sanitizeForError(conf.IPv6Subnet)), + } + } + if prefixLen, _ := mask.Mask.Size(); prefixLen > maxIPv6SubnetPrefixLen { + return nil, &types.Error{Code: 7, Msg: fmt.Sprintf( + "ipv6_subnet prefix length %d exceeds maximum of %d: %q", + prefixLen, maxIPv6SubnetPrefixLen, sanitizeForError(conf.IPv6Subnet)), + } + } + } + if conf.IPv4Subnet != "" { + ip, mask, err := net.ParseCIDR(conf.IPv4Subnet) + if err != nil { + return nil, &types.Error{Code: 7, Msg: fmt.Sprintf( + "invalid CIDR value for field 'ipv4_subnet': %q", sanitizeForError(conf.IPv4Subnet)), + } + } + if ip.To4() == nil { + return nil, &types.Error{Code: 7, Msg: fmt.Sprintf( + "ipv4_subnet must be an IPv4 CIDR, got IPv6: %q", sanitizeForError(conf.IPv4Subnet)), + } + } + if prefixLen, _ := mask.Mask.Size(); prefixLen > maxIPv4SubnetPrefixLen { + return nil, &types.Error{Code: 7, Msg: fmt.Sprintf( + "ipv4_subnet prefix length %d exceeds maximum of %d: %q", + prefixLen, maxIPv4SubnetPrefixLen, sanitizeForError(conf.IPv4Subnet)), + } + } + } + if len(conf.AddressFamilies) == 0 { + conf.AddressFamilies = []string{addressFamilyIPv6} + } else { + for _, af := range conf.AddressFamilies { + switch af { + case addressFamilyIPv6, addressFamilyIPv4: + default: + return nil, &types.Error{Code: 7, Msg: fmt.Sprintf( + "invalid address_families entry %q: must be %q or %q", + sanitizeForError(af), addressFamilyIPv6, addressFamilyIPv4), + } + } + } + } + + 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 sanitizeForError(s string) string { + for _, c := range s { + if c < 0x20 || c > 0x7e { + return sanitizeForErrorBinary + } + } + return s +} diff --git a/internal/cnitap/ops_add.go b/internal/cnitap/ops_add.go new file mode 100644 index 00000000..b8af4533 --- /dev/null +++ b/internal/cnitap/ops_add.go @@ -0,0 +1,155 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnitap + +import ( + "context" + "errors" + "fmt" + "log/slog" + "os" + + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/types" + "github.com/vishvananda/netlink" + + "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. +func cmdAdd(args *skel.CmdArgs) (err error) { + pluginConf, err := parseConf(args.StdinData) + if err != nil { + return err + } + + if pluginConf.PrevResult != nil { + if err := validatePrevResultAdd(pluginConf.PrevResult); err != nil { + return &types.Error{Code: 6, Msg: fmt.Sprintf("prevResult validation in ADD: %v", err)} + } + } + + nodeName := os.Getenv("NODE_NAME") + if nodeName == "" { + return &types.Error{Code: 4, Msg: "NODE_NAME environment variable is not set"} + } + + namespace := pluginConf.Namespace + + slog.Info("ADD: starting", + "containerID", args.ContainerID, "netns", args.Netns, "ifName", args.IfName, + "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment, + "namespace", namespace, "nodeName", nodeName) + + tracker := &resourceTracker{ + vpc: pluginConf.VPC, + vpcAttachment: pluginConf.VPCAttachment, + namespace: namespace, + } + + 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() + } + }() + + if err := vrf.Add(pluginConf.VPC, pluginConf.VPCAttachment); err != nil { + return fmt.Errorf("add VRF: %w", err) + } + tracker.vrfCreated = true + slog.Debug("ADD: VRF ready", "vpc", pluginConf.VPC, "vpcAttachment", pluginConf.VPCAttachment) + + if err := tap.Add(pluginConf.VPC, pluginConf.VPCAttachment, pluginConf.MTU); err != nil { + return fmt.Errorf("add tap: %w", err) + } + + hostName := intf.GenerateInterfaceNameHost(pluginConf.VPC, pluginConf.VPCAttachment) + hostLink, err := netlink.LinkByName(hostName) + if err != nil { + return fmt.Errorf("get host interface %q: %w", hostName, err) + } + hostMac := hostLink.Attrs().HardwareAddr.String() + hostMTU := hostLink.Attrs().MTU + slog.Debug("ADD: host interface ready", "name", hostName, "mac", hostMac, "mtu", hostMTU) + + k8sClient, err := newK8sClient() + 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 { + 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) + } + + // Allocate IPAM for the tap interface. The VM manages its own guest + // interface; the CNI only configures the host side. + ipamResult, err := cniipam.Allocate(args, allocConfig(pluginConf)) + if err != nil { + return fmt.Errorf("allocate IPAM: %w", err) + } + if ipamResult != nil { + slog.Debug("ADD: IPAM allocated", "containerID", args.ContainerID, + "ipv6Subnet", ipamResult.IPv6Subnet, "ipv6Gateway", ipamResult.IPv6Gateway, + "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 { + return err + } + if ipamResult != nil && ipamResult.IPv6Gateway != nil { + slog.Debug("ADD: host gateway configured", "name", hostName, "gateway", ipamResult.IPv6Gateway) + } + + 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 +} diff --git a/internal/cnitap/ops_check.go b/internal/cnitap/ops_check.go new file mode 100644 index 00000000..7e89229a --- /dev/null +++ b/internal/cnitap/ops_check.go @@ -0,0 +1,242 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnitap + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "net" + "net/http" + "os" + "time" + + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/types" + type100 "github.com/containernetworking/cni/pkg/types/100" + "github.com/vishvananda/netlink" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + + "go.datum.net/galactic/internal/config" + "go.datum.net/galactic/internal/plumbing/intf" + "go.datum.net/galactic/internal/plumbing/vrf" +) + +// cmdCheck validates that the node's tap-side networking state matches what +// was established during cmdAdd. Unlike internal/cni's own cmdCheck, there +// is no guest interface to verify — tap mode never enters a container +// netns. +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) + + var errs []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)) + } + + if pluginConf.RawPrevResult != nil { + if err := checkPrevResult(pluginConf.RawPrevResult, hostName); err != nil { + errs = append(errs, fmt.Errorf("prevResult validation: %w", 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 +} + +// cmdStatus implements the CNI spec STATUS operation — see internal/cni's +// own cmdStatus for the full reasoning; identical here. +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 + +// checkNodeLevelState verifies that node-level networking resources exist: +// the VRF interface and the host-side tap interface. +func checkNodeLevelState(vpc, vpcAttachment string) (string, []error) { + var errs []error + + if err := vrf.Exists(vpc, vpcAttachment); err != nil { + errs = append(errs, fmt.Errorf("vrf %s-%s: %w", vpc, vpcAttachment, err)) + } + + hostName := intf.GenerateInterfaceNameHost(vpc, vpcAttachment) + if _, err := netlink.LinkByName(hostName); err != nil { + errs = append(errs, fmt.Errorf("host interface %q: %w", hostName, err)) + } + + 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. +func checkPrevResult(rawPrevResult map[string]interface{}, _ string) error { + jsonBytes, err := json.Marshal(rawPrevResult) + if err != nil { + return fmt.Errorf("marshal prevResult: %w", err) + } + res, err := type100.NewResult(jsonBytes) + if err != nil { + return fmt.Errorf("parse prevResult: %w", err) + } + result, err := type100.GetResult(res) + if err != nil { + return fmt.Errorf("get prevResult: %w", err) + } + + for _, iface := range result.Interfaces { + if iface.Name == "" || iface.Sandbox != "" { + continue + } + if err := validateHostInterface(iface.Name, iface.Mac, iface.Mtu); err != nil { + return fmt.Errorf("interface %q (host): %w", iface.Name, err) + } + } + return nil +} + +// validateHostInterface checks that a host-side interface's MAC and MTU match +// the values recorded in prevResult. +func validateHostInterface(name, wantMac string, wantMtu int) error { + link, err := netlink.LinkByName(name) + if err != nil { + return fmt.Errorf("find link: %w", err) + } + if wantMac != "" && link.Attrs().HardwareAddr.String() != wantMac { + return fmt.Errorf("MAC mismatch: expected %q, got %q", wantMac, link.Attrs().HardwareAddr.String()) + } + if wantMtu > 0 && link.Attrs().MTU != wantMtu { + return fmt.Errorf("MTU mismatch: expected %d, got %d", wantMtu, link.Attrs().MTU) + } + return nil +} diff --git a/internal/cnitap/ops_del.go b/internal/cnitap/ops_del.go new file mode 100644 index 00000000..d9e05cfb --- /dev/null +++ b/internal/cnitap/ops_del.go @@ -0,0 +1,56 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnitap + +import ( + "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/cniipam" +) + +// cmdDel mirrors internal/cni's own cmdDel, minus everything guest-netns +// specific (no flushGuestNetnsConfig, no host-device DEL delegation — tap +// mode never touches a container netns at all). +func cmdDel(args *skel.CmdArgs) error { + // DEL is idempotent per the CNI spec: always return success. + slog.Info("DEL: starting", "containerID", args.ContainerID, "netns", args.Netns) + + pluginConf, parseErr := parseConf(args.StdinData) + if parseErr != nil { + slog.Error("DEL: failed to parse CNI config, skipping cleanup", "err", parseErr, + "containerID", args.ContainerID) + result := &type100.Result{} + _ = types.PrintResult(result, "1.0.0") + return nil + } + vpc, vpcAtt := pluginConf.VPC, pluginConf.VPCAttachment + + cfg := allocConfig(pluginConf) + if cniipam.WantsIPAM(cfg) { + if k8s, err := newK8sClient(); err == nil { + cniipam.Deallocate(args, cfg, k8s) + } else { + slog.Warn("DEL: failed to create k8s client, skipping IPAM deallocation", "err", err, + "containerID", args.ContainerID) + } + } + + // Shared resources (VRF, tap, routes, SRv6 ingress, BGPAdvertisement, + // BGPVRFInstance) are keyed by (vpc, vpcAttachment) and may still be in + // use by another VM. Deleting them here races with cmdAdd during + // restarts, so cleanup is left to galactic-router's GC controller — see + // internal/cni's own cmdDel for the full reasoning. + slog.Info("DEL: skipping shared resource cleanup (handled by GC)", + "containerID", args.ContainerID, "vpc", vpc, "vpcAttachment", vpcAtt) + + result := &type100.Result{} + _ = types.PrintResult(result, pluginConf.CNIVersion) + + return nil +} diff --git a/internal/cnitap/resource.go b/internal/cnitap/resource.go new file mode 100644 index 00000000..28bcae04 --- /dev/null +++ b/internal/cnitap/resource.go @@ -0,0 +1,128 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnitap + +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/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)) +} + +// newK8sClient creates a new Kubernetes client using the in-cluster config, +// scoped to cniScheme. +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-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. +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 +} + +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.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 err := tap.Delete(rt.vpc, rt.vpcAttachment); err != nil { + slog.Error("Rollback: failed to delete tap", "err", err, + "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) + } else { + slog.Debug("Rollback: deleted tap", "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) + } + + 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) + } else { + slog.Debug("Rollback: deleted VRF", "vpc", rt.vpc, "vpcAttachment", rt.vpcAttachment) + } +} diff --git a/internal/cnitap/result.go b/internal/cnitap/result.go new file mode 100644 index 00000000..abe09b45 --- /dev/null +++ b/internal/cnitap/result.go @@ -0,0 +1,71 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnitap + +import ( + "net" + + "github.com/containernetworking/cni/pkg/types" + type100 "github.com/containernetworking/cni/pkg/types/100" + + "go.datum.net/galactic/internal/cniipam" +) + +// 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. +func buildTapResult( + pluginConf *PluginConf, + ipRes *cniipam.IPAMResult, + hostName, hostMac string, + hostMTU int, +) *type100.Result { + result := &type100.Result{ + CNIVersion: pluginConf.CNIVersion, + Interfaces: []*type100.Interface{ + { + Name: hostName, + Mac: hostMac, + Mtu: hostMTU, + Sandbox: "", + }, + }, + } + appendIPConfigs(result, ipRes, 0, net.CIDRMask(25, 32)) // index into Interfaces (host tap) + return result +} + +// appendIPConfigs adds one IPConfig per allocated address family in ipRes +// (IPv6, and IPv4 when present) plus any default routes, all pointing at +// the given Interfaces index. No-op when ipRes is nil. +func appendIPConfigs(result *type100.Result, ipRes *cniipam.IPAMResult, ifaceIndex int, ipv4Mask net.IPMask) { + if ipRes == nil { + return + } + if ipRes.IPv6Subnet != nil { + result.IPs = append(result.IPs, &type100.IPConfig{ + Address: *ipRes.IPv6Subnet, + Gateway: ipRes.IPv6Gateway, + Interface: type100.Int(ifaceIndex), + }) + } + if ipRes.IPv4Address != nil { + result.IPs = append(result.IPs, &type100.IPConfig{ + Address: net.IPNet{IP: ipRes.IPv4Address, Mask: ipv4Mask}, + Gateway: ipRes.IPv4Gateway, + Interface: type100.Int(ifaceIndex), + }) + } + if len(ipRes.Routes) > 0 { + result.Routes = make([]*types.Route, 0, len(ipRes.Routes)) + for _, dst := range ipRes.Routes { + result.Routes = append(result.Routes, &types.Route{ + Dst: *dst, + }) + } + } +} diff --git a/internal/cnitap/types.go b/internal/cnitap/types.go new file mode 100644 index 00000000..17d72fb8 --- /dev/null +++ b/internal/cnitap/types.go @@ -0,0 +1,55 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +// Package cnitap implements galactic-tap-cni, the tap master plugin for +// VM-based workloads (Kata, Firecracker, kraftlet/Unikraft). It mirrors +// internal/cni (the veth master, galactic-cni) but never delegates to +// host-device (no container netns to move anything into — the VM manages +// its own guest interface) and never configures a guest-side netns. +package cnitap + +import ( + "github.com/containernetworking/cni/pkg/types" + + "go.datum.net/galactic/internal/cni/hostconf" + "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. +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"` + IPv6Subnet string `json:"ipv6_subnet,omitempty"` + IPv4Subnet string `json:"ipv4_subnet,omitempty"` + AddressFamilies []string `json:"address_families,omitempty"` +} + +// HostConf holds node-local settings read from /etc/cni/net.d/10-galactic.conflist. +type HostConf = hostconf.HostConf + +// allocConfig adapts pluginConf's fields into cniipam.AllocConfig. +func allocConfig(pluginConf *PluginConf) cniipam.AllocConfig { + return cniipam.AllocConfig{ + VPC: pluginConf.VPC, + VPCAttachment: pluginConf.VPCAttachment, + Namespace: pluginConf.Namespace, + IPAM: pluginConf.IPAM, + IPv6Subnet: pluginConf.IPv6Subnet, + IPv4Subnet: pluginConf.IPv4Subnet, + AddressFamilies: pluginConf.AddressFamilies, + } +} diff --git a/internal/installer/installer.go b/internal/installer/installer.go index 894a1194..4a9c7d4b 100644 --- a/internal/installer/installer.go +++ b/internal/installer/installer.go @@ -66,6 +66,7 @@ var ( HostEtcDir = "/host/var/lib/galactic" SADir = "/var/run/secrets/kubernetes.io/serviceaccount" SourceCNIBinary = "/galactic-cni" + SourceTapCNIBinary = "/galactic-tap-cni" SourceHostDeviceBinary = "/host-device" ) @@ -254,13 +255,19 @@ func Bootstrap(ctx context.Context, nodeName string) error { slog.Info("Starting CNI installer bootstrap", "nodeName", nodeName) - // 1. Copy CNI and host-device binaries to the host + // 1. Copy the CNI plugin chain's binaries to the host. Every binary in + // the chain ships in this same image and is staged here by this one + // init container, regardless of which master plugin(s) a given node's + // workloads actually use. if err := os.MkdirAll(HostBinDir, 0755); err != nil { return fmt.Errorf("create host CNI bin dir: %w", err) } if err := atomicCopyFile(SourceCNIBinary, filepath.Join(HostBinDir, "galactic-cni"), 0755); err != nil { return fmt.Errorf("copy galactic-cni binary: %w", err) } + if err := atomicCopyFile(SourceTapCNIBinary, filepath.Join(HostBinDir, "galactic-tap-cni"), 0755); err != nil { + return fmt.Errorf("copy galactic-tap-cni 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 6801cc44..cd3ef050 100644 --- a/internal/installer/installer_test.go +++ b/internal/installer/installer_test.go @@ -71,6 +71,17 @@ func TestResolveLogLevel(t *testing.T) { } } +// assertBinaryCopied verifies that the binary at path exists and contains +// wantContent, factored out of TestBootstrap to keep its own cyclomatic +// complexity within golangci-lint's gocyclo budget. +func assertBinaryCopied(t *testing.T, path, wantContent string) { + t.Helper() + got, err := os.ReadFile(path) + if err != nil || string(got) != wantContent { + t.Fatalf("binary copy verification failed for %q: err=%v content=%q", path, err, got) + } +} + func TestBootstrap(t *testing.T) { // Set up temporary directories for testing overrides tmpDir := t.TempDir() @@ -92,10 +103,14 @@ func TestBootstrap(t *testing.T) { // Create mock CNI source binary files SourceCNIBinary = filepath.Join(tmpDir, "source-galactic-cni") + SourceTapCNIBinary = filepath.Join(tmpDir, "source-galactic-tap-cni") 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(SourceHostDeviceBinary, []byte("host-device-content"), 0755); err != nil { t.Fatalf("write SourceHostDeviceBinary: %v", err) } @@ -151,10 +166,8 @@ func TestBootstrap(t *testing.T) { } // Verify binaries copied - cniContent, err := os.ReadFile(filepath.Join(HostBinDir, "galactic-cni")) - if err != nil || string(cniContent) != "cni-content" { - t.Fatalf("galactic-cni binary copy verification failed") - } + assertBinaryCopied(t, filepath.Join(HostBinDir, "galactic-cni"), "cni-content") + assertBinaryCopied(t, filepath.Join(HostBinDir, "galactic-tap-cni"), "tap-cni-content") // Verify conflist written conflist, err := loadHostConf(HostConflist) diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index c72ded88..9a9ec417 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -158,8 +158,8 @@ func TestKernelCapabilities(t *testing.T) { } } -// TestCNITapInterface exercises the galactic CNI plugin in tap interface mode. -// It creates a pod that invokes the CNI plugin with CNI_COMMAND=ADD and a tap +// TestCNITapInterface exercises galactic-tap-cni, the tap master plugin. +// It creates a pod that invokes the plugin with CNI_COMMAND=ADD and a tap // config, then validates the CNI result JSON: a single host interface with an // empty sandbox and the host-side gateway/subnet IPAM allocated for it. // @@ -212,13 +212,15 @@ func TestCNITapInterface(t *testing.T) { } // The eBPF uSID datapath is now the only forwarding path (see - // internal/cni/bgp.go's registerEBPFDatapath), 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. + // 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/ + // 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. startEBPFControlDaemon(t, name) // Write the CNI config to a file inside the pod, then run the plugin @@ -227,11 +229,9 @@ func TestCNITapInterface(t *testing.T) { cniConf := `{ "cniVersion": "1.0.0", "name": "galactic", - "type": "galactic-cni", + "type": "galactic-tap-cni", "vpc": "1", "vpcattachment": "1", - "interface_type": "tap", - "srv6_locator": "2001:db8:ff01::/48", "ipam": { "type": "pool" } @@ -250,7 +250,7 @@ CNI_IFNAME=eth0 \ CNI_PATH=/opt/cni/bin \ NODE_NAME=` + nodeName() + ` \ GALACTIC_CNI_ENABLE_LOCAL_IPAM=true \ - /galactic-cni < /tmp/cni.json + /galactic-tap-cni < /tmp/cni.json ` _, err = kubectl(t.Context(), "exec", name, "--", "sh", "-c",