From 4a9a0ff557f2a844513acac36372aa931360047e Mon Sep 17 00:00:00 2001 From: Peter Sprygada Date: Thu, 13 Aug 2026 16:43:07 -0400 Subject: [PATCH] feat(gateway): reconcile tenant VRF egress default routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase D of #865, per docs/plans/865-edge-gateway-nat66-egress.md §4.4/§4.5, resolved toward the new-controller candidate. - internal/egressroute: per-node reconcile logic -- enumerate local VRFs, resolve VPC, check NetworkEgressPolicy enablement + gateway assignment, resolve the destination uSID, install/remove the ::/0 SEG6 encap route. Mirrors internal/gc's split of real-logic-package plus thin-controller-wrapper. - internal/controller/egressroute_controller.go: ticker-driven wrapper (mirrors GCReconciler), registered in galactic-router -- runs on every compute node, not just gateway nodes, since that's where tenant VRFs actually live. - internal/controller/networkegresspolicy_controller.go: new NetworkEgressPolicyReconciler, one-time gateway-node assignment mirroring NetworkRuleReconciler.assignPrimaryNode. Registered in galactic-gateway alongside NetworkRuleReconciler. - internal/plumbing/vrf.ResolveVPC: new exported VPC-from-VRF-name resolver. - NetworkGatewayReconciler.publishEgressAddresses: extended to also publish EgressSID, refactored to share a new publishHostRouteAdvertisement helper across all three self-address advertisements (SRv6Address, EgressAddress, EgressSID) instead of triplicating the get-or-create-or-update block. - config: GALACTIC_ROUTER_EGRESS_ROUTE_INTERVAL, separate from GC's own interval. Two inferences the plan's own text doesn't spell out, made necessary by actually implementing this phase (see network repo's companion PR and internal/egressroute's package doc comment for the full reasoning): - tenant_arg reuses each VPC's own BGPVRFInstance.Spec.VRFID (already allocated per-node for ingress SRv6 decap, and already unique per-node -- exactly the isolation property egress needs). - NetworkGatewayStatus.EgressSID must be published into BGP the same way SRv6Address is, or no compute node has a kernel route to encapsulate toward at all. Bug found and fixed along the way: srv6.RouteEgressDel has been broken since it was written -- it set an empty SEG6Encap{} on the delete request, which netlink unconditionally rejects. Already called in production by internal/runtime/gobgp/monitor.go's BGP path-withdrawal handler, so every SEG6 route withdrawal in this codebase was likely failing, unrelated to egress. Fixed and added the first test coverage RouteEgressDel has ever had. --- cmd/galactic-gateway/root.go | 12 + cmd/galactic-router/root.go | 57 +++ internal/config/router.go | 35 +- internal/config/router_test.go | 7 + internal/controller/egressroute_controller.go | 90 ++++ .../networkegresspolicy_controller.go | 180 ++++++++ .../networkegresspolicy_controller_test.go | 126 ++++++ .../controller/networkgateway_controller.go | 131 +++--- .../networkgateway_controller_test.go | 36 +- internal/controller/status.go | 9 + internal/egressroute/egressroute.go | 291 +++++++++++++ internal/egressroute/egressroute_test.go | 395 ++++++++++++++++++ internal/plumbing/srv6/egress.go | 17 +- internal/plumbing/srv6/egress_test.go | 88 ++++ internal/plumbing/vrf/vrf.go | 34 ++ internal/plumbing/vrf/vrf_test.go | 28 ++ 16 files changed, 1454 insertions(+), 82 deletions(-) create mode 100644 internal/controller/egressroute_controller.go create mode 100644 internal/controller/networkegresspolicy_controller.go create mode 100644 internal/controller/networkegresspolicy_controller_test.go create mode 100644 internal/egressroute/egressroute.go create mode 100644 internal/egressroute/egressroute_test.go diff --git a/cmd/galactic-gateway/root.go b/cmd/galactic-gateway/root.go index 7d242ed0..b7a29db0 100644 --- a/cmd/galactic-gateway/root.go +++ b/cmd/galactic-gateway/root.go @@ -138,6 +138,7 @@ func runCmd(cfg *config.GatewayConfig) error { NodeName: nodeName, SRv6Address: cfg.SRv6Address, EgressAddress: cfg.EgressAddress, + EgressSID: cfg.EgressSID, }).SetupWithManager(mgr); err != nil { return fmt.Errorf("setup NetworkGateway controller: %w", err) } @@ -153,6 +154,17 @@ func runCmd(cfg *config.GatewayConfig) error { return fmt.Errorf("setup NetworkRule controller: %w", err) } + // Register NetworkEgressPolicy controller (one-time gateway-node + // assignment; see internal/controller/networkegresspolicy_controller.go, + // datum-cloud/enhancements#865). + if err := (&controller.NetworkEgressPolicyReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + NodeName: nodeName, + }).SetupWithManager(mgr); err != nil { + return fmt.Errorf("setup NetworkEgressPolicy controller: %w", err) + } + if err := mgr.Start(ctx); err != nil { return fmt.Errorf("manager exited: %w", err) } diff --git a/cmd/galactic-router/root.go b/cmd/galactic-router/root.go index b8edd418..effecc1c 100644 --- a/cmd/galactic-router/root.go +++ b/cmd/galactic-router/root.go @@ -282,6 +282,14 @@ func runCmd(cfg *config.RouterConfig) error { } }() + // Register EgressRoute controller and start its ticker goroutine + // (datum-cloud/enhancements#865, design plan §4.4/§7.1) -- extracted + // to its own function to keep this function's own cyclomatic + // complexity down (gocyclo), not for any reuse reason. + if err := startEgressRouteController(ctx, mgr, cfg, nodeName); err != nil { + return err + } + if err := mgr.Start(ctx); err != nil { return fmt.Errorf("manager exited: %w", err) } @@ -296,6 +304,52 @@ func runCmd(cfg *config.RouterConfig) error { return nil } +// startEgressRouteController registers controller.EgressRouteReconciler and +// starts its ticker goroutine, reconciling tenant VRF ::/0 default routes +// toward each VPC's assigned gateway egress_sid (datum-cloud/ +// enhancements#865, design plan §4.4/§7.1). Mirrors the GC ticker in +// runCmd exactly (including waiting for cache sync first, so the initial +// pass doesn't see an empty NetworkEgressPolicy list and remove every live +// egress route) -- reuses cfg.GCNamespace (both scan the same namespace's +// CRDs) but its own separate interval, see DefaultRouterEgressRouteInterval's +// doc comment for why. +func startEgressRouteController( + ctx context.Context, mgr ctrl.Manager, cfg *config.RouterConfig, nodeName string, +) error { + rec := &controller.EgressRouteReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Namespace: cfg.GCNamespace, + NodeName: nodeName, + Interval: cfg.EgressRouteInterval, + } + if err := rec.SetupWithManager(mgr); err != nil { + return fmt.Errorf("setup EgressRoute controller: %w", err) + } + + go func() { + ticker := time.NewTicker(cfg.EgressRouteInterval) + defer ticker.Stop() + + if !mgr.GetCache().WaitForCacheSync(ctx) { + log.Printf("EgressRoute: cache sync failed, skipping initial pass") + return + } + rec.RunOnce(ctx) + + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + rec.RunOnce(ctx) + } + } + }() + + return nil +} + // newRootCommand builds the root cobra command with all flags and the // application startup logic. func newRootCommand() *cobra.Command { @@ -344,6 +398,9 @@ func newRootCommand() *cobra.Command { cmd.Flags().DurationP("gc-interval", "", config.DefaultRouterGCInterval, "Cleanup interval") + cmd.Flags().DurationP("egress-route-interval", "", + config.DefaultRouterEgressRouteInterval, + "Egress default-route reconcile interval (datum-cloud/enhancements#865)") cmd.Flags().Bool("webhook-enabled", false, "Enable the NetworkRule admission webhook (requires TLS cert material; see config/webhook/)") cmd.Flags().IntP("webhook-port", "", diff --git a/internal/config/router.go b/internal/config/router.go index 6cda0ffe..118698ef 100644 --- a/internal/config/router.go +++ b/internal/config/router.go @@ -22,6 +22,14 @@ const ( DefaultRouterGCNamespace = "galactic-system" DefaultRouterGCInterval = 5 * time.Minute + // DefaultRouterEgressRouteInterval is the default reconcile period for + // EgressRouteReconciler (datum-cloud/enhancements#865, design plan + // §4.4/§7.1), a separate tunable from DefaultRouterGCInterval despite + // sharing a default value: GC cleans up stale kernel/CRD state, this + // installs/removes live egress default routes -- conceptually distinct + // concerns that operators may want to tune independently. + DefaultRouterEgressRouteInterval = 5 * time.Minute + // DefaultRouterWebhookPort matches sigs.k8s.io/controller-runtime/pkg/webhook's // own DefaultPort, named here so callers don't need that import just to // read the default. @@ -41,6 +49,11 @@ const ( EnvRouterGCNamespace = "GALACTIC_ROUTER_GC_NAMESPACE" EnvRouterGCInterval = "GALACTIC_ROUTER_GC_INTERVAL" + // EnvRouterEgressRouteInterval configures EgressRouteReconciler's + // reconcile period -- see DefaultRouterEgressRouteInterval's doc + // comment for why this is a separate knob from EnvRouterGCInterval. + EnvRouterEgressRouteInterval = "GALACTIC_ROUTER_EGRESS_ROUTE_INTERVAL" + // EnvRouterWebhookEnabled gates the NetworkRule admission webhook // (internal/webhook). Defaults to false: this is the first webhook in // this codebase, and enabling it requires TLS cert material @@ -72,15 +85,16 @@ type RouterConfig struct { prefix string // Resolved fields. - NodeName string - Mode string - Reflector bool - BGPListenPort int - BGPLocalAddr string - MetricsPort int - GRPCHealthPort int - GCNamespace string - GCInterval time.Duration + NodeName string + Mode string + Reflector bool + BGPListenPort int + BGPLocalAddr string + MetricsPort int + GRPCHealthPort int + GCNamespace string + GCInterval time.Duration + EgressRouteInterval time.Duration // WebhookEnabled/WebhookPort/WebhookCertDir configure the NetworkRule // admission webhook (internal/webhook) -- see @@ -107,6 +121,7 @@ func NewRouterConfig() *RouterConfig { v.SetDefault("grpc_health_port", DefaultRouterGRPCHealthPort) v.SetDefault("gc_namespace", DefaultRouterGCNamespace) v.SetDefault("gc_interval", DefaultRouterGCInterval.String()) + v.SetDefault("egress_route_interval", DefaultRouterEgressRouteInterval.String()) v.SetDefault("webhook_enabled", false) v.SetDefault("webhook_port", DefaultRouterWebhookPort) v.SetDefault("webhook_cert_dir", "") @@ -135,6 +150,7 @@ func (c *RouterConfig) BindFlags(flags *pflag.FlagSet) { {"grpc-health-port", "grpc_health_port"}, {"gc-namespace", "gc_namespace"}, {"gc-interval", "gc_interval"}, + {"egress-route-interval", "egress_route_interval"}, {"webhook-enabled", "webhook_enabled"}, {"webhook-port", "webhook_port"}, {"webhook-cert-dir", "webhook_cert_dir"}, @@ -161,6 +177,7 @@ func (c *RouterConfig) readFields() { c.GRPCHealthPort = c.v.GetInt("grpc_health_port") c.GCNamespace = c.v.GetString("gc_namespace") c.GCInterval = c.v.GetDuration("gc_interval") + c.EgressRouteInterval = c.v.GetDuration("egress_route_interval") c.WebhookEnabled = c.v.GetBool("webhook_enabled") c.WebhookPort = c.v.GetInt("webhook_port") c.WebhookCertDir = c.v.GetString("webhook_cert_dir") diff --git a/internal/config/router_test.go b/internal/config/router_test.go index 8d7fce7d..278f3d3f 100644 --- a/internal/config/router_test.go +++ b/internal/config/router_test.go @@ -38,6 +38,9 @@ func TestRouterConfigDefaults(t *testing.T) { if cfg.GCInterval != DefaultRouterGCInterval { t.Errorf("GCInterval = %v, want %v", cfg.GCInterval, DefaultRouterGCInterval) } + if cfg.EgressRouteInterval != DefaultRouterEgressRouteInterval { + t.Errorf("EgressRouteInterval = %v, want %v", cfg.EgressRouteInterval, DefaultRouterEgressRouteInterval) + } if cfg.Reflector { t.Error("Reflector = true, want false") } @@ -62,6 +65,7 @@ func TestRouterConfigEnvOverride(t *testing.T) { t.Setenv(EnvRouterGRPCHealthPort, "5179") t.Setenv(EnvRouterGCNamespace, "custom-ns") t.Setenv(EnvRouterGCInterval, "10m") + t.Setenv(EnvRouterEgressRouteInterval, "15m") t.Setenv(EnvRouterWebhookEnabled, testBoolTrue) t.Setenv(EnvRouterWebhookPort, "9444") t.Setenv(EnvRouterWebhookCertDir, "/tmp/certs") @@ -95,6 +99,9 @@ func TestRouterConfigEnvOverride(t *testing.T) { if cfg.GCInterval != 10*time.Minute { t.Errorf("GCInterval = %v, want 10m", cfg.GCInterval) } + if cfg.EgressRouteInterval != 15*time.Minute { + t.Errorf("EgressRouteInterval = %v, want 15m", cfg.EgressRouteInterval) + } if !cfg.WebhookEnabled { t.Error("WebhookEnabled = false, want true") } diff --git a/internal/controller/egressroute_controller.go b/internal/controller/egressroute_controller.go new file mode 100644 index 00000000..37f7d2d4 --- /dev/null +++ b/internal/controller/egressroute_controller.go @@ -0,0 +1,90 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package controller + +import ( + "context" + "log/slog" + "time" + + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + "go.datum.net/galactic/internal/egressroute" +) + +// EgressRouteReconciler runs periodic egress default-route reconciliation +// on this compute node (datum-cloud/enhancements#865, design plan +// §4.4/§7.1) — the ticker-driven wrapper around internal/egressroute.Run, +// mirroring GCReconciler's own split between a thin, time-driven +// controller here and the real logic in a sibling non-controller package +// (internal/gc). Unlike NetworkGatewayReconciler/NetworkEgressPolicyReconciler +// (gateway-node-scoped, registered from cmd/galactic-gateway), this runs +// from cmd/galactic-router's tenant-role process: it needs to see every +// compute node's own local VRF state, not just gateway nodes', and +// galactic-router (tenant role) already runs on every node that could have +// one. +// +// Time-driven, not object-driven, for the same reason GCReconciler is: a +// newly-created local VRF interface is pure kernel state with no +// corresponding Kubernetes watch event to react to, so this can only ever +// notice it on a periodic sweep, not a reactive one. +type EgressRouteReconciler struct { + client.Client + Scheme *runtime.Scheme + Namespace string + NodeName string + Interval time.Duration +} + +// slogAdapter adapts log/slog's package-level functions to +// egressroute.Logger, matching GCReconciler's own choice of slog over +// logr.Logger for this package's plain log lines (see gc_controller.go's +// identical use of slog.Info/slog.Error at its own call sites). +type slogAdapter struct{} + +func (slogAdapter) Info(msg string, keysAndValues ...any) { slog.Info(msg, keysAndValues...) } +func (slogAdapter) Error(err error, msg string, keysAndValues ...any) { + slog.Error(msg, append([]any{"err", err}, keysAndValues...)...) +} + +// Reconcile runs an egress-route pass at the configured interval. It does +// not watch any Kubernetes resources — it is purely time-driven, same as +// GCReconciler.Reconcile. +func (r *EgressRouteReconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result, error) { + if r.Namespace == "" { + slog.Debug("EgressRoute: namespace not configured, skipping") + return ctrl.Result{RequeueAfter: r.Interval}, nil + } + + result := r.RunOnce(ctx) + if result.Errors > 0 { + slog.Info("EgressRoute: completed with errors", + "routesInstalled", result.RoutesInstalled, "routesRemoved", result.RoutesRemoved, "errors", result.Errors) + } else if result.RoutesInstalled > 0 || result.RoutesRemoved > 0 { + slog.Info("EgressRoute: reconcile complete", + "routesInstalled", result.RoutesInstalled, "routesRemoved", result.RoutesRemoved) + } + + return ctrl.Result{RequeueAfter: r.Interval}, nil +} + +// SetupWithManager registers the EgressRouteReconciler with the manager. +// Like GCReconciler, it is started by a ticker goroutine launched from +// root.go where the manager's context is available. +func (r *EgressRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { + if r.Interval == 0 { + r.Interval = 5 * time.Minute + } + return nil +} + +// RunOnce runs a single egress-route reconcile pass in the given context. +// This is the public API for triggering it from outside the reconciler +// (e.g. root.go's ticker goroutine), mirroring GCReconciler.RunGC. +func (r *EgressRouteReconciler) RunOnce(ctx context.Context) egressroute.Result { + return egressroute.Run(ctx, r.Client, r.Namespace, r.NodeName, slogAdapter{}) +} diff --git a/internal/controller/networkegresspolicy_controller.go b/internal/controller/networkegresspolicy_controller.go new file mode 100644 index 00000000..dfe7a4ff --- /dev/null +++ b/internal/controller/networkegresspolicy_controller.go @@ -0,0 +1,180 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package controller + +import ( + "context" + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/log" + ctrlreconcile "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "go.datum.net/galactic/internal/gateway" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +// NetworkEgressPolicyReconciler owns NetworkEgressPolicy's one piece of +// per-object lifecycle state: status.assignedGatewayNode, assigned exactly +// once at creation (datum-cloud/enhancements#865, design plan §4.5), +// mirroring NetworkRuleReconciler.assignPrimaryNode almost exactly — +// gateway.AssignPrimaryNode's own doc comment explains why silently +// recomputing an existing assignment on a later reconcile would be a +// correctness bug (an avoidable traffic flap if the gateway-node pool +// later changes), not just a wasted computation. +// +// Unlike NetworkRuleReconciler, this reconciler carries no finalizer and no +// BGP-withdrawal teardown ordering: NetworkEgressPolicy owns no +// BGPAdvertisement of its own to withdraw (the gateway's own +// EgressAddress/EgressSID advertisements are independent of any single +// policy, and a tenant VRF's default route is a compute-node-local kernel +// resource internal/egressroute's route reconciler owns, not something +// this reconciler needs to order against). Deletion needs no special +// handling at all: internal/egressroute's route reconciler simply stops +// seeing an accepted policy for that VPC on its next periodic pass and +// removes the route then — no cross-node coordination to get right here. +// +// Safe to run from every gateway node's galactic-gateway process (see +// isGatewayNode) without leader election, for the identical reason +// NetworkRuleReconciler's own doc comment gives: assignGatewayNode is a +// pure function of inputs every gateway node observes identically +// (gateway.AssignPrimaryNode). +type NetworkEgressPolicyReconciler struct { + client.Client + Scheme *runtime.Scheme + NodeName string +} + +// Reconcile reconciles a single NetworkEgressPolicy's gateway-node +// assignment. +func (r *NetworkEgressPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + policy := &bgpv1alpha1.NetworkEgressPolicy{} + if err := r.Get(ctx, req.NamespacedName, policy); err != nil { + if apierrors.IsNotFound(err) { + return ctrl.Result{}, nil + } + return ctrl.Result{}, fmt.Errorf("get NetworkEgressPolicy %s: %w", req.NamespacedName, err) + } + + isGateway, err := isGatewayNode(ctx, r.Client, policy.Namespace, r.NodeName) + if err != nil { + return ctrl.Result{}, fmt.Errorf("determine gateway-node membership for %s: %w", r.NodeName, err) + } + if !isGateway { + return ctrl.Result{}, nil + } + + if !policy.DeletionTimestamp.IsZero() { + // No finalizer, nothing to tear down here — see this type's doc + // comment. + return ctrl.Result{}, nil + } + + if err := r.assignGatewayNode(ctx, policy); err != nil { + return ctrl.Result{}, fmt.Errorf("assign gateway node for NetworkEgressPolicy %s: %w", req.NamespacedName, err) + } + + return ctrl.Result{}, nil +} + +// assignGatewayNode sets policy.Status.AssignedGatewayNode exactly once — +// mirrors NetworkRuleReconciler.assignPrimaryNode's identical structure +// (immutability guard, Accepted condition gated on the gateway-node pool +// existing) — see that method's doc comment for why the webhook can't set +// Accepted itself. +func (r *NetworkEgressPolicyReconciler) assignGatewayNode( + ctx context.Context, policy *bgpv1alpha1.NetworkEgressPolicy, +) error { + nodes, err := gatewayNodeNames(ctx, r.Client, policy.Namespace) + if err != nil { + return err + } + + policyCopy := policy.DeepCopy() + if len(nodes) == 0 { + setEgressPolicyCondition(policyCopy, metav1.Condition{ + Type: bgpv1alpha1.ConditionTypeAccepted, + Status: metav1.ConditionFalse, + Reason: "NoGatewayNodes", + Message: "no NetworkGateway nodes are registered in this namespace yet", + }) + return r.Status().Update(ctx, policyCopy) + } + + changed := false + if policy.Status.AssignedGatewayNode == "" { + assigned, err := gateway.AssignPrimaryNode(policy.Spec.VPCRef, nodes) + if err != nil { + return fmt.Errorf("assign gateway node: %w", err) + } + policyCopy.Status.AssignedGatewayNode = assigned + policyCopy.Status.ObservedGeneration = policy.Generation + changed = true + } + + if !meta.IsStatusConditionTrue(policy.Status.Conditions, bgpv1alpha1.ConditionTypeAccepted) { + setEgressPolicyCondition(policyCopy, metav1.Condition{ + Type: bgpv1alpha1.ConditionTypeAccepted, + Status: metav1.ConditionTrue, + Reason: "GatewayNodesRegistered", + Message: "gateway nodes are registered for this namespace", + }) + changed = true + } + + if !changed { + return nil + } + return r.Status().Update(ctx, policyCopy) +} + +// SetupWithManager registers the NetworkEgressPolicyReconciler with the +// manager. The NetworkGateway watch mirrors NetworkRuleReconciler's own: +// assignGatewayNode is a function of the namespace's gateway-node pool, so +// a policy must be re-examined whenever that pool changes. +func (r *NetworkEgressPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&bgpv1alpha1.NetworkEgressPolicy{}). + Watches(&bgpv1alpha1.NetworkGateway{}, handler.EnqueueRequestsFromMapFunc( + func(ctx context.Context, obj client.Object) []ctrlreconcile.Request { + return gatewayToEgressPolicyRequests(ctx, r.Client, obj) + }), + ). + Named("networkegresspolicy"). + Complete(r) +} + +// gatewayToEgressPolicyRequests maps a NetworkGateway change to every +// NetworkEgressPolicy in its namespace — the same broadcast pattern +// gatewayToRuleRequests uses for NetworkRule, for the identical reason: a +// NetworkEgressPolicy carries no gatewayRef, and its assignment depends on +// the namespace's whole gateway-node pool. +func gatewayToEgressPolicyRequests(ctx context.Context, c client.Client, obj client.Object) []ctrlreconcile.Request { + gw, ok := obj.(*bgpv1alpha1.NetworkGateway) + if !ok { + return nil + } + logger := log.FromContext(ctx) + + policyList := &bgpv1alpha1.NetworkEgressPolicyList{} + if err := c.List(ctx, policyList, client.InNamespace(gw.Namespace)); err != nil { + logger.Error(err, "list NetworkEgressPolicies for NetworkGateway change", "networkGateway", gw.Name) + return nil + } + reqs := make([]ctrlreconcile.Request, 0, len(policyList.Items)) + for _, policy := range policyList.Items { + reqs = append(reqs, ctrlreconcile.Request{ + NamespacedName: types.NamespacedName{Namespace: policy.Namespace, Name: policy.Name}, + }) + } + return reqs +} diff --git a/internal/controller/networkegresspolicy_controller_test.go b/internal/controller/networkegresspolicy_controller_test.go new file mode 100644 index 00000000..aae25035 --- /dev/null +++ b/internal/controller/networkegresspolicy_controller_test.go @@ -0,0 +1,126 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package controller + +import ( + "context" + "testing" + + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +func newTestEgressPolicy(name, vpcRef string) *bgpv1alpha1.NetworkEgressPolicy { + return &bgpv1alpha1.NetworkEgressPolicy{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: name}, + Spec: bgpv1alpha1.NetworkEgressPolicySpec{VPCRef: vpcRef, VPCAttachmentRef: "attach-1"}, + } +} + +func TestNetworkEgressPolicyReconciler_AssignsGatewayNodeOnce(t *testing.T) { + scheme := newRuleTestScheme(t) + gwA := newTestGateway(testNodeGWA) + gwB := newTestGateway(testNodeGWB) + policy := newTestEgressPolicy("policy-1", "vpc-1") + + fakeClient := newIndexedClientBuilder(scheme). + WithStatusSubresource(&bgpv1alpha1.NetworkEgressPolicy{}). + WithObjects(gwA, gwB, policy). + Build() + + r := &NetworkEgressPolicyReconciler{Client: fakeClient, Scheme: scheme, NodeName: testNodeGWA} + req := ctrl.Request{NamespacedName: testRuleKey("policy-1")} + + if _, err := r.Reconcile(context.Background(), req); err != nil { + t.Fatalf("Reconcile: unexpected error: %v", err) + } + + got := &bgpv1alpha1.NetworkEgressPolicy{} + if err := fakeClient.Get(context.Background(), testRuleKey("policy-1"), got); err != nil { + t.Fatalf("get NetworkEgressPolicy: %v", err) + } + if got.Status.AssignedGatewayNode == "" { + t.Fatal("AssignedGatewayNode was not set") + } + if !meta.IsStatusConditionTrue(got.Status.Conditions, bgpv1alpha1.ConditionTypeAccepted) { + t.Error("Accepted condition was not set true") + } + + firstAssignment := got.Status.AssignedGatewayNode + + // A second reconcile (e.g. triggered by an unrelated NetworkGateway + // change) must not recompute the assignment -- gateway.AssignPrimaryNode's + // own doc comment explains why silently recomputing would be a + // correctness bug, not just a wasted computation. + if _, err := r.Reconcile(context.Background(), req); err != nil { + t.Fatalf("second Reconcile: unexpected error: %v", err) + } + got2 := &bgpv1alpha1.NetworkEgressPolicy{} + if err := fakeClient.Get(context.Background(), testRuleKey("policy-1"), got2); err != nil { + t.Fatalf("get NetworkEgressPolicy after second reconcile: %v", err) + } + if got2.Status.AssignedGatewayNode != firstAssignment { + t.Errorf("AssignedGatewayNode changed across reconciles: %q -> %q", firstAssignment, got2.Status.AssignedGatewayNode) + } +} + +func TestNetworkEgressPolicyReconciler_NoGatewayNodesSetsNotAccepted(t *testing.T) { + scheme := newRuleTestScheme(t) + policy := newTestEgressPolicy("policy-1", "vpc-1") + + fakeClient := newIndexedClientBuilder(scheme). + WithStatusSubresource(&bgpv1alpha1.NetworkEgressPolicy{}). + WithObjects(policy). + Build() + + r := &NetworkEgressPolicyReconciler{Client: fakeClient, Scheme: scheme, NodeName: testNodeGWA} + req := ctrl.Request{NamespacedName: testRuleKey("policy-1")} + + // No NetworkGateway registered for this node at all -- isGatewayNode + // gates the whole reconcile, so this must be a no-op, not an error. + if _, err := r.Reconcile(context.Background(), req); err != nil { + t.Fatalf("Reconcile: unexpected error: %v", err) + } + + got := &bgpv1alpha1.NetworkEgressPolicy{} + if err := fakeClient.Get(context.Background(), testRuleKey("policy-1"), got); err != nil { + t.Fatalf("get NetworkEgressPolicy: %v", err) + } + if got.Status.AssignedGatewayNode != "" { + t.Errorf("AssignedGatewayNode = %q, want empty (this node is not a gateway node)", got.Status.AssignedGatewayNode) + } +} + +func TestNetworkEgressPolicyReconciler_SkipsNonGatewayNode(t *testing.T) { + scheme := newRuleTestScheme(t) + gwA := newTestGateway(testNodeGWA) + policy := newTestEgressPolicy("policy-1", "vpc-1") + + fakeClient := newIndexedClientBuilder(scheme). + WithStatusSubresource(&bgpv1alpha1.NetworkEgressPolicy{}). + WithObjects(gwA, policy). + Build() + + // "compute-node-1" (testComputeNodeName) is not a registered gateway + // node -- assignment must not run from its process. + r := &NetworkEgressPolicyReconciler{Client: fakeClient, Scheme: scheme, NodeName: testComputeNodeName} + req := ctrl.Request{NamespacedName: testRuleKey("policy-1")} + + if _, err := r.Reconcile(context.Background(), req); err != nil { + t.Fatalf("Reconcile: unexpected error: %v", err) + } + + got := &bgpv1alpha1.NetworkEgressPolicy{} + if err := fakeClient.Get(context.Background(), testRuleKey("policy-1"), got); err != nil { + t.Fatalf("get NetworkEgressPolicy: %v", err) + } + if got.Status.AssignedGatewayNode != "" { + t.Errorf("AssignedGatewayNode = %q, want empty (reconciler running on a non-gateway node)", + got.Status.AssignedGatewayNode) + } +} diff --git a/internal/controller/networkgateway_controller.go b/internal/controller/networkgateway_controller.go index e387c949..4a98de92 100644 --- a/internal/controller/networkgateway_controller.go +++ b/internal/controller/networkgateway_controller.go @@ -108,9 +108,19 @@ type NetworkGatewayReconciler struct { // source for tenant egress (datum-cloud/enhancements#865, // config.GatewayConfig.EgressAddress), already the address the // running datapath was configured with. Empty on a gateway node not - // offering egress — see publishEgressAddress's doc comment, the + // offering egress — see publishEgressAddresses's doc comment, the // egress-specific sibling of publishSelfAddress. EgressAddress string + + // EgressSID is this node's own egress_sid uSID locator + // (config.GatewayConfig.EgressSID), already the value the running + // datapath's egress_config_table was configured with. Always set + // together with EgressAddress, or neither — see + // publishEgressAddresses's doc comment. Published so compute nodes + // (internal/egressroute's route reconciler) can resolve a real kernel + // route to it before encapsulating a tenant VRF's default route + // toward it. + EgressSID string } const ( @@ -193,9 +203,9 @@ func (r *NetworkGatewayReconciler) Reconcile(ctx context.Context, req ctrl.Reque logger.Error(err, "publish gateway self-address", "networkGateway", req.NamespacedName) advErrs = append(advErrs, fmt.Errorf("publish self-address: %w", err)) } - if err := r.publishEgressAddress(ctx, gw); err != nil { - logger.Error(err, "publish gateway egress address", "networkGateway", req.NamespacedName) - advErrs = append(advErrs, fmt.Errorf("publish egress address: %w", err)) + if err := r.publishEgressAddresses(ctx, gw); err != nil { + logger.Error(err, "publish gateway egress addresses", "networkGateway", req.NamespacedName) + advErrs = append(advErrs, fmt.Errorf("publish egress addresses: %w", err)) } // Crash-safety ordering contract (see GatewayEngine.ReconcileOrphans): @@ -371,20 +381,33 @@ func (r *NetworkGatewayReconciler) publishSelfAddress(ctx context.Context, gw *b return nil // nothing to advertise into yet } - addr, err := netip.ParseAddr(r.SRv6Address) + return r.publishHostRouteAdvertisement(ctx, gw.Namespace, gw.Name+"-selfaddr", routerName, r.SRv6Address) +} + +// publishHostRouteAdvertisement ensures a BGPAdvertisement exists at +// namespace/name distributing addrStr as a /128 (or /32 for IPv4) host +// route via routerName — the shared "self-address" advertisement shape +// publishSelfAddress and publishEgressAddresses (for both EgressAddress and +// EgressSID) all need identically, differing only in which status field +// and object name each caller uses. Extracted here once a third caller +// (EgressSID, design plan §4.3) would otherwise have triplicated this +// get-or-create-or-update block. +func (r *NetworkGatewayReconciler) publishHostRouteAdvertisement( + ctx context.Context, namespace, name, routerName, addrStr string, +) error { + addr, err := netip.ParseAddr(addrStr) if err != nil { - return fmt.Errorf("parse gateway SRv6 address %q: %w", r.SRv6Address, err) + return fmt.Errorf("parse address %q: %w", addrStr, err) } prefix := netip.PrefixFrom(addr, addr.BitLen()) - name := gw.Name + "-selfaddr" adv := &bgpv1alpha1.BGPAdvertisement{} - key := types.NamespacedName{Namespace: gw.Namespace, Name: name} + key := types.NamespacedName{Namespace: namespace, Name: name} getErr := r.Get(ctx, key, adv) switch { case apierrors.IsNotFound(getErr): adv = &bgpv1alpha1.BGPAdvertisement{ - ObjectMeta: metav1.ObjectMeta{Namespace: gw.Namespace, Name: name}, + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: name}, Spec: bgpv1alpha1.BGPAdvertisementSpec{ RouterRef: bgpv1alpha1.RouterRef{Name: routerName}, AddressFamily: bgpv1alpha1.AddressFamily{AFI: bgpv1alpha1.AFIL2VPN, SAFI: bgpv1alpha1.SAFIEVPN}, @@ -392,11 +415,11 @@ func (r *NetworkGatewayReconciler) publishSelfAddress(ctx context.Context, gw *b }, } if createErr := r.Create(ctx, adv); createErr != nil { - return fmt.Errorf("create self-address BGPAdvertisement %s: %w", name, createErr) + return fmt.Errorf("create BGPAdvertisement %s: %w", name, createErr) } return nil case getErr != nil: - return fmt.Errorf("get self-address BGPAdvertisement %s: %w", name, getErr) + return fmt.Errorf("get BGPAdvertisement %s: %w", name, getErr) } advCopy := adv.DeepCopy() @@ -404,28 +427,36 @@ func (r *NetworkGatewayReconciler) publishSelfAddress(ctx context.Context, gw *b advCopy.Spec.AddressFamily = bgpv1alpha1.AddressFamily{AFI: bgpv1alpha1.AFIL2VPN, SAFI: bgpv1alpha1.SAFIEVPN} advCopy.Spec.Prefixes = []bgpv1alpha1.Prefix{bgpv1alpha1.Prefix(prefix.String())} if updateErr := r.Update(ctx, advCopy); updateErr != nil { - return fmt.Errorf("update self-address BGPAdvertisement %s: %w", name, updateErr) + return fmt.Errorf("update BGPAdvertisement %s: %w", name, updateErr) } return nil } -// publishEgressAddress sets gw.Status.EgressAddress to r.EgressAddress and -// ensures a BGPAdvertisement exists distributing it as a /128 host route -- -// the egress-specific sibling of publishSelfAddress, mirroring its pattern -// exactly (design plan §4.3), kept as a separate function rather than -// folded into publishSelfAddress because the two fields are independently -// optional: a node can have SRv6Address set with EgressAddress empty (not -// offering egress), so combining them would tangle two unrelated -// early-return conditions into one function. +// publishEgressAddresses sets gw.Status.EgressAddress/EgressSID to +// r.EgressAddress/r.EgressSID and ensures a BGPAdvertisement exists for +// each, distributing it as a /128 host route -- the egress-specific +// sibling of publishSelfAddress, mirroring its pattern exactly (design +// plan §4.3), kept as a separate function rather than folded into +// publishSelfAddress because SRv6Address is unconditionally required while +// EgressAddress/EgressSID are optional as a pair (config.GatewayConfig. +// Validate) — combining them would tangle an always-true condition with a +// sometimes-true one. // -// Unlike SRv6Address (reachable only within the SRv6 fabric), EgressAddress -// must additionally be reachable from the public internet -- an -// eBGP/uplink-peering concern entirely outside this repo (likely -// config/fabric's FRR underlay, or a dedicated transit peer). This method -// only publishes the value into the CRD and the internal iBGP/EVPN mesh, the -// same way publishSelfAddress does for SRv6Address; it does not, and -// cannot, arrange the internet-facing side of that reachability. -func (r *NetworkGatewayReconciler) publishEgressAddress(ctx context.Context, gw *bgpv1alpha1.NetworkGateway) error { +// Both fields get published here, not two separate functions, because +// unlike EgressAddress-vs-SRv6Address they are never independently +// optional from *each other* — always both set or both empty. +// +// EgressAddress must additionally be reachable from the public internet -- +// an eBGP/uplink-peering concern entirely outside this repo (likely +// config/fabric's FRR underlay, or a dedicated transit peer). EgressSID, by +// contrast, is exactly like SRv6Address: a real uSID other (compute) nodes +// need a kernel route to before they can install a SEG6 encap route naming +// it as the destination (internal/egressroute's route reconciler) -- this +// method only publishes both values into the CRD and the internal +// iBGP/EVPN mesh, the same way publishSelfAddress does for SRv6Address; it +// does not, and cannot, arrange EgressAddress's internet-facing +// reachability. +func (r *NetworkGatewayReconciler) publishEgressAddresses(ctx context.Context, gw *bgpv1alpha1.NetworkGateway) error { if r.EgressAddress == "" { return nil // this gateway node does not offer egress } @@ -435,12 +466,13 @@ func (r *NetworkGatewayReconciler) publishEgressAddress(ctx context.Context, gw return fmt.Errorf("resolve BGPRouter for node %s: %w", r.NodeName, err) } - if gw.Status.EgressAddress != r.EgressAddress { + if gw.Status.EgressAddress != r.EgressAddress || gw.Status.EgressSID != r.EgressSID { // Updated in place, not through a copy -- same reasoning as // publishSelfAddress's identical in-place update (#365). gw.Status.EgressAddress = r.EgressAddress + gw.Status.EgressSID = r.EgressSID if err := r.Status().Update(ctx, gw); err != nil { - return fmt.Errorf("update NetworkGateway status.egressAddress: %w", err) + return fmt.Errorf("update NetworkGateway status.egressAddress/egressSID: %w", err) } } @@ -448,40 +480,13 @@ func (r *NetworkGatewayReconciler) publishEgressAddress(ctx context.Context, gw return nil // nothing to advertise into yet } - addr, err := netip.ParseAddr(r.EgressAddress) - if err != nil { - return fmt.Errorf("parse gateway egress address %q: %w", r.EgressAddress, err) - } - prefix := netip.PrefixFrom(addr, addr.BitLen()) - - name := gw.Name + "-egressaddr" - adv := &bgpv1alpha1.BGPAdvertisement{} - key := types.NamespacedName{Namespace: gw.Namespace, Name: name} - getErr := r.Get(ctx, key, adv) - switch { - case apierrors.IsNotFound(getErr): - adv = &bgpv1alpha1.BGPAdvertisement{ - ObjectMeta: metav1.ObjectMeta{Namespace: gw.Namespace, Name: name}, - Spec: bgpv1alpha1.BGPAdvertisementSpec{ - RouterRef: bgpv1alpha1.RouterRef{Name: routerName}, - AddressFamily: bgpv1alpha1.AddressFamily{AFI: bgpv1alpha1.AFIL2VPN, SAFI: bgpv1alpha1.SAFIEVPN}, - Prefixes: []bgpv1alpha1.Prefix{bgpv1alpha1.Prefix(prefix.String())}, - }, - } - if createErr := r.Create(ctx, adv); createErr != nil { - return fmt.Errorf("create egress-address BGPAdvertisement %s: %w", name, createErr) - } - return nil - case getErr != nil: - return fmt.Errorf("get egress-address BGPAdvertisement %s: %w", name, getErr) + if err := r.publishHostRouteAdvertisement( + ctx, gw.Namespace, gw.Name+"-egressaddr", routerName, r.EgressAddress); err != nil { + return fmt.Errorf("publish egress-address route: %w", err) } - - advCopy := adv.DeepCopy() - advCopy.Spec.RouterRef = bgpv1alpha1.RouterRef{Name: routerName} - advCopy.Spec.AddressFamily = bgpv1alpha1.AddressFamily{AFI: bgpv1alpha1.AFIL2VPN, SAFI: bgpv1alpha1.SAFIEVPN} - advCopy.Spec.Prefixes = []bgpv1alpha1.Prefix{bgpv1alpha1.Prefix(prefix.String())} - if updateErr := r.Update(ctx, advCopy); updateErr != nil { - return fmt.Errorf("update egress-address BGPAdvertisement %s: %w", name, updateErr) + if err := r.publishHostRouteAdvertisement( + ctx, gw.Namespace, gw.Name+"-egresssid", routerName, r.EgressSID); err != nil { + return fmt.Errorf("publish egress-sid route: %w", err) } return nil } diff --git a/internal/controller/networkgateway_controller_test.go b/internal/controller/networkgateway_controller_test.go index b6b2d0c6..a85838e5 100644 --- a/internal/controller/networkgateway_controller_test.go +++ b/internal/controller/networkgateway_controller_test.go @@ -438,15 +438,16 @@ func TestNetworkGatewayReconciler_PublishesSelfAddress(t *testing.T) { } } -// TestNetworkGatewayReconciler_PublishesEgressAddress verifies -// publishEgressAddress writes status.egressAddress and creates a plain (no -// VRFID/Function) BGPAdvertisement for it once a BGPRouter targets this -// node -- the egress-specific sibling of +// TestNetworkGatewayReconciler_PublishesEgressAddresses verifies +// publishEgressAddresses writes status.egressAddress/egressSID and creates +// a plain (no VRFID/Function) BGPAdvertisement for each once a BGPRouter +// targets this node -- the egress-specific sibling of // TestNetworkGatewayReconciler_PublishesSelfAddress // (datum-cloud/enhancements#865). -func TestNetworkGatewayReconciler_PublishesEgressAddress(t *testing.T) { +func TestNetworkGatewayReconciler_PublishesEgressAddresses(t *testing.T) { scheme := newRuleTestScheme(t) const egressAddr = "2001:db8:eeee::1" + const egressSID = "2001:db8:dddd::1" gw := newTestGateway(testNodeGWA) router := newTestRouter() @@ -458,6 +459,7 @@ func TestNetworkGatewayReconciler_PublishesEgressAddress(t *testing.T) { engine := newFakeGatewayEngine() r := newGatewayReconciler(fakeClient, scheme, engine, testNodeGWA) r.EgressAddress = egressAddr + r.EgressSID = egressSID req := ctrl.Request{NamespacedName: testRuleKey(testNodeGWA)} if _, err := r.Reconcile(context.Background(), req); err != nil { @@ -471,6 +473,9 @@ func TestNetworkGatewayReconciler_PublishesEgressAddress(t *testing.T) { if gotGW.Status.EgressAddress != egressAddr { t.Errorf("Status.EgressAddress = %q, want %q", gotGW.Status.EgressAddress, egressAddr) } + if gotGW.Status.EgressSID != egressSID { + t.Errorf("Status.EgressSID = %q, want %q", gotGW.Status.EgressSID, egressSID) + } adv := &bgpv1alpha1.BGPAdvertisement{} if err := fakeClient.Get(context.Background(), testRuleKey(testNodeGWA+"-egressaddr"), adv); err != nil { @@ -483,12 +488,20 @@ func TestNetworkGatewayReconciler_PublishesEgressAddress(t *testing.T) { t.Errorf("egress-address advertisement must carry no VRFID/Function, got VRFID=%v Function=%v", adv.Spec.VRFID, adv.Spec.Function) } + + sidAdv := &bgpv1alpha1.BGPAdvertisement{} + if err := fakeClient.Get(context.Background(), testRuleKey(testNodeGWA+"-egresssid"), sidAdv); err != nil { + t.Fatalf("get egress-sid BGPAdvertisement: %v", err) + } + if len(sidAdv.Spec.Prefixes) != 1 || sidAdv.Spec.Prefixes[0] != bgpv1alpha1.Prefix(egressSID+"/128") { + t.Errorf("Prefixes = %v, want [%s/128]", sidAdv.Spec.Prefixes, egressSID) + } } // TestNetworkGatewayReconciler_NoEgressAddressSkipsPublication verifies a -// gateway node with EgressAddress unset (the common case -- a node not -// offering egress) neither writes status.egressAddress nor creates an -// egress-address BGPAdvertisement. +// gateway node with EgressAddress/EgressSID unset (the common case -- a +// node not offering egress) neither writes status.egressAddress/egressSID +// nor creates either egress BGPAdvertisement. func TestNetworkGatewayReconciler_NoEgressAddressSkipsPublication(t *testing.T) { scheme := newRuleTestScheme(t) gw := newTestGateway(testNodeGWA) @@ -514,11 +527,18 @@ func TestNetworkGatewayReconciler_NoEgressAddressSkipsPublication(t *testing.T) if gotGW.Status.EgressAddress != "" { t.Errorf("Status.EgressAddress = %q, want empty (this node does not offer egress)", gotGW.Status.EgressAddress) } + if gotGW.Status.EgressSID != "" { + t.Errorf("Status.EgressSID = %q, want empty (this node does not offer egress)", gotGW.Status.EgressSID) + } err := fakeClient.Get(context.Background(), testRuleKey(testNodeGWA+"-egressaddr"), &bgpv1alpha1.BGPAdvertisement{}) if err == nil { t.Error("egress-address BGPAdvertisement was created for a node with no EgressAddress configured") } + err = fakeClient.Get(context.Background(), testRuleKey(testNodeGWA+"-egresssid"), &bgpv1alpha1.BGPAdvertisement{}) + if err == nil { + t.Error("egress-sid BGPAdvertisement was created for a node with no EgressSID configured") + } } // TestNetworkGatewayReconciler_AdvertisementFailureSurfaces is the diff --git a/internal/controller/status.go b/internal/controller/status.go index 6b915265..ade6f44b 100644 --- a/internal/controller/status.go +++ b/internal/controller/status.go @@ -119,3 +119,12 @@ func setRuleCondition(rule *bgpv1alpha1.NetworkRule, condition metav1.Condition) condition.ObservedGeneration = rule.Generation meta.SetStatusCondition(&rule.Status.Conditions, condition) } + +// setEgressPolicyCondition sets or updates a condition on NetworkEgressPolicy +// (datum-cloud/enhancements#865). ConditionTypeAccepted (reused from +// peer_types.go, same as NetworkRule) is the only condition type this +// reconciler sets — see NetworkEgressPolicyReconciler's doc comment. +func setEgressPolicyCondition(policy *bgpv1alpha1.NetworkEgressPolicy, condition metav1.Condition) { + condition.ObservedGeneration = policy.Generation + meta.SetStatusCondition(&policy.Status.Conditions, condition) +} diff --git a/internal/egressroute/egressroute.go b/internal/egressroute/egressroute.go new file mode 100644 index 00000000..85349538 --- /dev/null +++ b/internal/egressroute/egressroute.go @@ -0,0 +1,291 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +// Package egressroute reconciles a tenant VRF's ::/0 default route toward +// the assigned gateway node's egress_sid, on every compute node that has a +// local VRF for that tenant (datum-cloud/enhancements#865, design plan +// §4.4/§7.1). It is the per-node route-installing half of the plan's +// candidate-2 decision (a new controller, not galactic-cni's cmdAdd) — +// see internal/controller/egressroute_controller.go for the ticker-driven +// wrapper that calls Run periodically, mirroring internal/gc's own +// split between the real logic (this package) and a thin controller +// wrapper (internal/controller/gc_controller.go). +// +// Scope: this package only ever installs/removes a plain IPv6 ::/0 route +// (design plan §3.4 — IPv4 egress destinations are out of scope for +// Phase 1's datapath, so an IPv4 default route would be pointless work). +// +// Two significant inferences this package makes, neither spelled out +// explicitly in the design plan text, flagged here for review: +// +// 1. tenant_arg reuse: the Argument value embedded in the destination +// uSID this package builds is the tenant's own BGPVRFInstance.Spec. +// VRFID — the same Argument value already allocated for that VPC's +// *ingress* SRv6 decap on this same node (internal/cnibgp/bgp.go's +// allocateArgument). This is safe because VRFID uniqueness is scoped +// per-node (allocateArgument scans only the calling router's own +// BGPVRFInstances), which is exactly the isolation property egress's +// tenant_arg needs too (no two VPCs on the *same* node ever share a +// value) — nothing downstream compares one node's choice against +// another node's for the same VPC, so the value does not need to be +// globally consistent across every node hosting that VPC, only +// locally unique on each one. +// 2. NetworkGatewayStatus.EgressSID (this repo's own network API +// addition, not in the original design plan text) publishes the +// assigned gateway's egress_sid locator so a compute node can resolve +// it at all — necessary because RouteEgressAdd's underlying +// netlink.RouteGet requires a real kernel route to the destination +// SID to already exist, which only happens once it's been advertised +// into BGP/EVPN the same way SRv6Address already is. +package egressroute + +import ( + "context" + "fmt" + "net" + "net/netip" + + "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + "sigs.k8s.io/controller-runtime/pkg/client" + + "go.datum.net/galactic/internal/crdnames" + "go.datum.net/galactic/internal/plumbing/ebpf/uformat" + "go.datum.net/galactic/internal/plumbing/srv6" + "go.datum.net/galactic/internal/plumbing/vrf" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +// ipv6DefaultPrefix is ::/0 — the only prefix this package ever installs a +// route for (see this package's own doc comment on IPv4 scope). +var ipv6DefaultPrefix = &net.IPNet{IP: net.IPv6zero, Mask: net.CIDRMask(0, 128)} + +// Result summarizes one reconcile pass, mirroring gc.CleanupResult's shape +// and logging convention. +type Result struct { + RoutesInstalled int + RoutesRemoved int + Errors int +} + +// Run reconciles every local Galactic-managed VRF interface's ::/0 default +// route against NetworkEgressPolicy state in namespace, for this node +// (nodeName). For each local VRF: +// +// - If an accepted NetworkEgressPolicy names this VRF's VPC, and that +// policy has an AssignedGatewayNode whose NetworkGateway publishes a +// non-empty EgressSID, and this node has its own BGPVRFInstance for +// the VPC: ensure the ::/0 SEG6 encap route exists, encapsulating +// toward that gateway's egress_sid locator with this VPC's own VRFID +// as the uSID Argument (tenant_arg) — see this package's doc comment +// for why VRFID is safe to reuse this way. +// - Otherwise: ensure no ::/0 route remains in that VRF's table (removes +// one if enablement was withdrawn, the assigned gateway isn't ready +// yet, or was never installed at all). +// +// Best-effort: one VRF's failure is logged and counted, not fatal to the +// rest of the pass — same convention as gc.RunGC. +func Run(ctx context.Context, k8s client.Client, namespace, nodeName string, log Logger) Result { + var result Result + + links, err := vrf.ListVRFLinks() + if err != nil { + log.Error(err, "list local VRF links") + result.Errors++ + return result + } + if len(links) == 0 { + return result + } + + enabled, err := acceptedEgressPolicyByVPC(ctx, k8s, namespace) + if err != nil { + log.Error(err, "list NetworkEgressPolicies") + result.Errors++ + return result + } + + for _, link := range links { + vpc, ok := vrf.ResolveVPC(link.Name) + if !ok { + continue // not a Galactic VRF + } + + tableID, err := vrf.TableID(vpc) + if err != nil { + log.Error(err, "resolve table ID for local VRF", "vpc", vpc, "vrf", link.Name) + result.Errors++ + continue + } + + dest, err := resolveEgressDestination(ctx, k8s, namespace, nodeName, vpc, enabled) + if err != nil { + log.Error(err, "resolve egress destination for VPC", "vpc", vpc) + result.Errors++ + continue + } + + if dest == nil { + removed, err := removeDefaultRoute(tableID) + if err != nil { + log.Error(err, "remove egress default route", "vpc", vpc, "table", tableID) + result.Errors++ + continue + } + if removed { + log.Info("removed egress default route", "vpc", vpc, "table", tableID) + result.RoutesRemoved++ + } + continue + } + + if err := srv6.RouteEgressAdd(ipv6DefaultPrefix, *dest, tableID); err != nil { + log.Error(err, "install egress default route", "vpc", vpc, "table", tableID, "destination", dest.String()) + result.Errors++ + continue + } + result.RoutesInstalled++ + } + + return result +} + +// Logger is the minimal structured-logging interface Run needs — satisfied +// by both log/slog's *slog.Logger (via a thin adapter) and +// sigs.k8s.io/controller-runtime/pkg/log's logr.Logger, so this package +// depends on neither directly. +type Logger interface { + Info(msg string, keysAndValues ...any) + Error(err error, msg string, keysAndValues ...any) +} + +// acceptedEgressPolicyByVPC lists every NetworkEgressPolicy in namespace and +// returns the first Accepted one found per vpcRef. Taking the first is safe +// even if a VPC has multiple policies (one per vpcAttachmentRef, design +// plan §4.1): AssignedGatewayNode is a pure function of vpcRef alone +// (gateway.AssignPrimaryNode), so every accepted policy for the same VPC +// resolves to the identical value regardless of which one this picks. +func acceptedEgressPolicyByVPC( + ctx context.Context, k8s client.Client, namespace string, +) (map[string]*bgpv1alpha1.NetworkEgressPolicy, error) { + list := &bgpv1alpha1.NetworkEgressPolicyList{} + if err := k8s.List(ctx, list, client.InNamespace(namespace)); err != nil { + return nil, fmt.Errorf("list NetworkEgressPolicies: %w", err) + } + + byVPC := make(map[string]*bgpv1alpha1.NetworkEgressPolicy, len(list.Items)) + for i := range list.Items { + policy := &list.Items[i] + if !policy.DeletionTimestamp.IsZero() { + continue + } + if !meta.IsStatusConditionTrue(policy.Status.Conditions, bgpv1alpha1.ConditionTypeAccepted) { + continue + } + if _, exists := byVPC[policy.Spec.VPCRef]; exists { + continue + } + byVPC[policy.Spec.VPCRef] = policy + } + return byVPC, nil +} + +// resolveEgressDestination returns the full destination uSID this VPC's +// default route should encapsulate toward, or nil if egress is not +// (yet, or no longer) enabled for it — never an error for "not enabled", +// only for a real lookup failure. +func resolveEgressDestination( + ctx context.Context, k8s client.Client, namespace, nodeName, vpc string, + enabled map[string]*bgpv1alpha1.NetworkEgressPolicy, +) (*net.IP, error) { + policy, ok := enabled[vpc] + if !ok || policy.Status.AssignedGatewayNode == "" { + return nil, nil + } + + gw := &bgpv1alpha1.NetworkGateway{} + gwKey := client.ObjectKey{Namespace: namespace, Name: policy.Status.AssignedGatewayNode} + if err := k8s.Get(ctx, gwKey, gw); err != nil { + if errors.IsNotFound(err) { + return nil, nil // assigned node no longer exists; treat as not-yet-ready + } + return nil, fmt.Errorf("get NetworkGateway %s: %w", policy.Status.AssignedGatewayNode, err) + } + if gw.Status.EgressSID == "" { + return nil, nil // assigned gateway isn't offering egress yet + } + + locator, err := netip.ParseAddr(gw.Status.EgressSID) + if err != nil { + return nil, fmt.Errorf("parse egress_sid %q for gateway %s: %w", gw.Status.EgressSID, gw.Name, err) + } + block, err := uformat.Block(locator) + if err != nil { + return nil, fmt.Errorf("read Block from egress_sid %q: %w", gw.Status.EgressSID, err) + } + nodeID, err := uformat.NodeID(locator) + if err != nil { + return nil, fmt.Errorf("read Node-ID from egress_sid %q: %w", gw.Status.EgressSID, err) + } + + vrfInst := &bgpv1alpha1.BGPVRFInstance{} + vrfKey := client.ObjectKey{Namespace: namespace, Name: crdnames.BGPVRFInstanceName(vpc, nodeName)} + if err := k8s.Get(ctx, vrfKey, vrfInst); err != nil { + if errors.IsNotFound(err) { + return nil, nil // no ingress VRF instance yet for this VPC on this node + } + return nil, fmt.Errorf("get BGPVRFInstance %s: %w", vrfKey.Name, err) + } + // VRFID's own CRD range (1-65535) is wider than uformat's 12-bit + // Argument field (up to 0xFFF=4095) -- the same range mismatch + // internal/gc/gc.go's SweepEBPFVRFTable already guards against for + // the ingress path; mirrored here rather than silently truncating. + if vrfInst.Spec.VRFID < int32(uformat.ArgumentMin) || vrfInst.Spec.VRFID > int32(uformat.ArgumentMax) { + return nil, fmt.Errorf("BGPVRFInstance %s has out-of-range VRFID %d for uSID Argument use [%#x,%#x]", + vrfKey.Name, vrfInst.Spec.VRFID, uint16(uformat.ArgumentMin), uint16(uformat.ArgumentMax)) + } + //nolint:gosec // range-checked immediately above + tenantArg := uint16(vrfInst.Spec.VRFID) + + dest, err := uformat.Encode(uformat.Fields{Block: block, NodeID: nodeID, Function: 0, Argument: tenantArg}) + if err != nil { + return nil, fmt.Errorf("encode egress destination uSID: %w", err) + } + destIP := net.IP(dest.AsSlice()) + return &destIP, nil +} + +// removeDefaultRoute removes ipv6DefaultPrefix's SEG6 encap route from +// tableID if present, reporting whether anything was actually removed. +// Checks for existence first (rather than calling srv6.RouteEgressDel +// unconditionally and tolerating a "no such route" error) — the same +// list-then-delete convention internal/plumbing/vrf.flush already uses. +func removeDefaultRoute(tableID uint32) (bool, error) { + routes, err := netlink.RouteListFiltered( + unix.AF_INET6, + &netlink.Route{Table: int(tableID)}, + netlink.RT_FILTER_TABLE, + ) + if err != nil { + return false, fmt.Errorf("list routes in table %d: %w", tableID, err) + } + + found := false + for _, r := range routes { + if r.Dst != nil && r.Dst.String() == ipv6DefaultPrefix.String() { + found = true + break + } + } + if !found { + return false, nil + } + + if err := srv6.RouteEgressDel(ipv6DefaultPrefix, tableID); err != nil { + return false, err + } + return true, nil +} diff --git a/internal/egressroute/egressroute_test.go b/internal/egressroute/egressroute_test.go new file mode 100644 index 00000000..179ca1f5 --- /dev/null +++ b/internal/egressroute/egressroute_test.go @@ -0,0 +1,395 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package egressroute + +import ( + "context" + "fmt" + "net" + "os" + "testing" + + "github.com/containernetworking/plugins/pkg/ns" + "github.com/vishvananda/netlink" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "go.datum.net/galactic/internal/crdnames" + "go.datum.net/galactic/internal/plumbing/vrf" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +const ( + testNamespace = "ns" + testNodeName = "compute-1" + testGatewayNode = "gw-a" + testVPC = "vpc-a" + testAttachment = "attach-1" + testEgressSID = "2001:db8:eeee::1" + testVRFID = int32(42) +) + +// testLogger discards everything -- these tests assert on Result and +// fake-client state directly, not log output. +type testLogger struct{} + +func (testLogger) Info(string, ...any) {} +func (testLogger) Error(error, string, ...any) {} + +func newEgressTestScheme(t *testing.T) *runtime.Scheme { + t.Helper() + scheme := runtime.NewScheme() + if err := bgpv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme: %v", err) + } + return scheme +} + +// acceptedPolicy builds a NetworkEgressPolicy fixture with the Accepted +// condition already set true and assigned to testGatewayNode -- this +// package (unlike internal/controller) has no existing acceptRule-style +// fixture to reuse. +func acceptedPolicy(vpc string) *bgpv1alpha1.NetworkEgressPolicy { + return &bgpv1alpha1.NetworkEgressPolicy{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: vpc + "-" + testAttachment}, + Spec: bgpv1alpha1.NetworkEgressPolicySpec{VPCRef: vpc, VPCAttachmentRef: testAttachment}, + Status: bgpv1alpha1.NetworkEgressPolicyStatus{ + AssignedGatewayNode: testGatewayNode, + Conditions: []metav1.Condition{{ + Type: bgpv1alpha1.ConditionTypeAccepted, + Status: metav1.ConditionTrue, + Reason: bgpv1alpha1.AcceptedReasonOwnershipVerified, + }}, + }, + } +} + +func testGateway(egressSID string) *bgpv1alpha1.NetworkGateway { + return &bgpv1alpha1.NetworkGateway{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testGatewayNode}, + Status: bgpv1alpha1.NetworkGatewayStatus{EgressSID: egressSID}, + } +} + +func testVRFInstance(vpc, node string, vrfID int32) *bgpv1alpha1.BGPVRFInstance { + return &bgpv1alpha1.BGPVRFInstance{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: crdnames.BGPVRFInstanceName(vpc, node)}, + Spec: bgpv1alpha1.BGPVRFInstanceSpec{VRFID: vrfID}, + } +} + +// --- acceptedEgressPolicyByVPC ----------------------------------------- + +func TestAcceptedEgressPolicyByVPC_OnlyAcceptedIncluded(t *testing.T) { + scheme := newEgressTestScheme(t) + accepted := acceptedPolicy(testVPC) + notAccepted := &bgpv1alpha1.NetworkEgressPolicy{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "vpc-b-attach"}, + Spec: bgpv1alpha1.NetworkEgressPolicySpec{VPCRef: "vpc-b", VPCAttachmentRef: "attach"}, + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(accepted, notAccepted).Build() + + byVPC, err := acceptedEgressPolicyByVPC(context.Background(), c, testNamespace) + if err != nil { + t.Fatalf("acceptedEgressPolicyByVPC: %v", err) + } + if _, ok := byVPC[testVPC]; !ok { + t.Errorf("accepted policy for %q missing from result", testVPC) + } + if _, ok := byVPC["vpc-b"]; ok { + t.Errorf("non-accepted policy for vpc-b should not appear in result") + } +} + +func TestAcceptedEgressPolicyByVPC_DeletingExcluded(t *testing.T) { + scheme := newEgressTestScheme(t) + policy := acceptedPolicy(testVPC) + now := metav1.Now() + policy.DeletionTimestamp = &now + policy.Finalizers = []string{"keep-alive-for-test"} // fake client requires a finalizer to accept a deletion timestamp + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(policy).Build() + + byVPC, err := acceptedEgressPolicyByVPC(context.Background(), c, testNamespace) + if err != nil { + t.Fatalf("acceptedEgressPolicyByVPC: %v", err) + } + if _, ok := byVPC[testVPC]; ok { + t.Error("a policy being deleted must be excluded, even if still Accepted") + } +} + +// --- resolveEgressDestination ------------------------------------------- + +func TestResolveEgressDestination_NotEnabledReturnsNil(t *testing.T) { + scheme := newEgressTestScheme(t) + c := fake.NewClientBuilder().WithScheme(scheme).Build() + + dest, err := resolveEgressDestination(context.Background(), c, testNamespace, testNodeName, testVPC, nil) + if err != nil { + t.Fatalf("resolveEgressDestination: %v", err) + } + if dest != nil { + t.Errorf("dest = %v, want nil (no enabled policy for this VPC)", dest) + } +} + +func TestResolveEgressDestination_AssignedGatewayMissingIsNilNotError(t *testing.T) { + scheme := newEgressTestScheme(t) + c := fake.NewClientBuilder().WithScheme(scheme).Build() + + enabled := map[string]*bgpv1alpha1.NetworkEgressPolicy{ + testVPC: acceptedPolicy(testVPC), + } + dest, err := resolveEgressDestination(context.Background(), c, testNamespace, testNodeName, testVPC, enabled) + if err != nil { + t.Fatalf("resolveEgressDestination: %v", err) + } + if dest != nil { + t.Errorf("dest = %v, want nil (assigned NetworkGateway doesn't exist yet)", dest) + } +} + +func TestResolveEgressDestination_GatewayNotOfferingEgressIsNilNotError(t *testing.T) { + scheme := newEgressTestScheme(t) + gw := testGateway("") // EgressSID empty -- not offering egress + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(gw).Build() + + enabled := map[string]*bgpv1alpha1.NetworkEgressPolicy{ + testVPC: acceptedPolicy(testVPC), + } + dest, err := resolveEgressDestination(context.Background(), c, testNamespace, testNodeName, testVPC, enabled) + if err != nil { + t.Fatalf("resolveEgressDestination: %v", err) + } + if dest != nil { + t.Errorf("dest = %v, want nil (assigned gateway's EgressSID is empty)", dest) + } +} + +func TestResolveEgressDestination_NoLocalVRFInstanceIsNilNotError(t *testing.T) { + scheme := newEgressTestScheme(t) + gw := testGateway(testEgressSID) + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(gw).Build() + + enabled := map[string]*bgpv1alpha1.NetworkEgressPolicy{ + testVPC: acceptedPolicy(testVPC), + } + dest, err := resolveEgressDestination(context.Background(), c, testNamespace, testNodeName, testVPC, enabled) + if err != nil { + t.Fatalf("resolveEgressDestination: %v", err) + } + if dest != nil { + t.Errorf("dest = %v, want nil (no BGPVRFInstance for this VPC on this node yet)", dest) + } +} + +func TestResolveEgressDestination_ComposesFullUSID(t *testing.T) { + scheme := newEgressTestScheme(t) + gw := testGateway(testEgressSID) + vrfInst := testVRFInstance(testVPC, testNodeName, testVRFID) + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(gw, vrfInst).Build() + + enabled := map[string]*bgpv1alpha1.NetworkEgressPolicy{ + testVPC: acceptedPolicy(testVPC), + } + dest, err := resolveEgressDestination(context.Background(), c, testNamespace, testNodeName, testVPC, enabled) + if err != nil { + t.Fatalf("resolveEgressDestination: %v", err) + } + if dest == nil { + t.Fatal("dest = nil, want a resolved destination uSID") + } + // Locator (Block+Node-ID, top 8 bytes) must match egress_sid's own; + // the low 12 bits (Argument) must carry this VPC's own VRFID. + locator := net.ParseIP(testEgressSID).To16() + got := *dest + if got == nil || len(got) != 16 { + t.Fatalf("dest = %v, want a 16-byte IPv6 address", got) + } + for i := range 8 { + if got[i] != locator[i] { + t.Fatalf("dest = %v, locator bytes don't match egress_sid %s", got, testEgressSID) + } + } + arg := (uint16(got[8]&0x0F) << 8) | uint16(got[9]) + if arg != uint16(testVRFID) { + t.Errorf("dest Argument = %#x, want %#x (this VPC's own VRFID)", arg, testVRFID) + } +} + +func TestResolveEgressDestination_VRFIDOutOfArgumentRangeErrors(t *testing.T) { + scheme := newEgressTestScheme(t) + gw := testGateway(testEgressSID) + // 4096 (0x1000) overflows the 12-bit Argument field (max 0xFFF). + vrfInst := testVRFInstance(testVPC, testNodeName, 4096) + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(gw, vrfInst).Build() + + enabled := map[string]*bgpv1alpha1.NetworkEgressPolicy{ + testVPC: acceptedPolicy(testVPC), + } + _, err := resolveEgressDestination(context.Background(), c, testNamespace, testNodeName, testVPC, enabled) + if err == nil { + t.Error("resolveEgressDestination with an out-of-range VRFID: want an error, got nil") + } +} + +// --- Run, end-to-end against real kernel state -------------------------- + +func requireRoot(t *testing.T) { + t.Helper() + if os.Geteuid() != 0 { + t.Skip("test requires root (CAP_NET_ADMIN) to create a test network namespace, " + + "a real VRF, and install routes; re-run via sudo") + } +} + +// TestRun_InstallsThenRemovesDefaultRoute proves the full enable/disable +// lifecycle against a real kernel VRF: enabling egress installs a working +// ::/0 SEG6 encap route in the VRF's own table, and withdrawing enablement +// (deleting the NetworkEgressPolicy) removes it again on the next pass. +func TestRun_InstallsThenRemovesDefaultRoute(t *testing.T) { + requireRoot(t) + + const ( + vpc = "rtest" + ifaceName = "egrtest0" + ifaceAddr = "2001:db8:1::1/64" + sidRoute = "2001:db8:eeee::/64" // reachable via ifaceAddr's subnet + sidGw = "2001:db8:1::2" // on-link next-hop + ) + + nsObj, err := ns.TempNetNS() + if err != nil { + t.Fatalf("create test netns: %v", err) + } + defer func() { _ = nsObj.Close() }() + + var tableID uint32 + err = nsObj.Do(func(_ ns.NetNS) error { + if addErr := vrf.Add(vpc); addErr != nil { + return fmt.Errorf("vrf.Add: %w", addErr) + } + var tidErr error + tableID, tidErr = vrf.TableID(vpc) + if tidErr != nil { + return fmt.Errorf("vrf.TableID: %w", tidErr) + } + + dummy := &netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Name: ifaceName}} + if linkErr := netlink.LinkAdd(dummy); linkErr != nil { + return fmt.Errorf("add dummy link: %w", linkErr) + } + if upErr := netlink.LinkSetUp(dummy); upErr != nil { + return fmt.Errorf("set link up: %w", upErr) + } + addr, addrErr := netlink.ParseAddr(ifaceAddr) + if addrErr != nil { + return fmt.Errorf("parse addr: %w", addrErr) + } + if addrAddErr := netlink.AddrAdd(dummy, addr); addrAddErr != nil { + return fmt.Errorf("add addr: %w", addrAddErr) + } + _, sidDst, cidrErr := net.ParseCIDR(sidRoute) + if cidrErr != nil { + return fmt.Errorf("parse SID route: %w", cidrErr) + } + route := &netlink.Route{LinkIndex: dummy.Attrs().Index, Dst: sidDst, Gw: net.ParseIP(sidGw)} + if routeErr := netlink.RouteAdd(route); routeErr != nil { + return fmt.Errorf("add route to egress_sid subnet: %w", routeErr) + } + return nil + }) + if err != nil { + t.Fatalf("setup: %v", err) + } + t.Cleanup(func() { + _ = nsObj.Do(func(_ ns.NetNS) error { return vrf.Delete(vpc) }) + }) + + scheme := newEgressTestScheme(t) + gw := testGateway(testEgressSID) + vrfInst := testVRFInstance(vpc, testNodeName, testVRFID) + policy := acceptedPolicy(vpc) + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(gw, vrfInst, policy).Build() + + // Enable: Run must install the ::/0 route. + var result Result + err = nsObj.Do(func(_ ns.NetNS) error { + result = Run(context.Background(), c, testNamespace, testNodeName, testLogger{}) + return nil + }) + if err != nil { + t.Fatalf("Run (enable pass): %v", err) + } + if result.Errors != 0 { + t.Fatalf("Run (enable pass) result = %+v, want 0 errors", result) + } + if result.RoutesInstalled != 1 { + t.Fatalf("Run (enable pass) RoutesInstalled = %d, want 1", result.RoutesInstalled) + } + + var installed bool + err = nsObj.Do(func(_ ns.NetNS) error { + routes, listErr := netlink.RouteListFiltered( + netlink.FAMILY_V6, &netlink.Route{Table: int(tableID)}, netlink.RT_FILTER_TABLE) + if listErr != nil { + return listErr + } + for _, r := range routes { + if r.Dst != nil && r.Dst.String() == "::/0" { + installed = true + } + } + return nil + }) + if err != nil { + t.Fatalf("verify installed route: %v", err) + } + if !installed { + t.Fatal("::/0 route was not installed in the VRF's table after the enable pass") + } + + // Disable: delete the policy, then Run must remove the route. + if delErr := c.Delete(context.Background(), policy); delErr != nil { + t.Fatalf("delete NetworkEgressPolicy: %v", delErr) + } + err = nsObj.Do(func(_ ns.NetNS) error { + result = Run(context.Background(), c, testNamespace, testNodeName, testLogger{}) + return nil + }) + if err != nil { + t.Fatalf("Run (disable pass): %v", err) + } + if result.Errors != 0 { + t.Fatalf("Run (disable pass) result = %+v, want 0 errors", result) + } + if result.RoutesRemoved != 1 { + t.Fatalf("Run (disable pass) RoutesRemoved = %d, want 1", result.RoutesRemoved) + } + + var stillInstalled bool + err = nsObj.Do(func(_ ns.NetNS) error { + routes, listErr := netlink.RouteListFiltered( + netlink.FAMILY_V6, &netlink.Route{Table: int(tableID)}, netlink.RT_FILTER_TABLE) + if listErr != nil { + return listErr + } + for _, r := range routes { + if r.Dst != nil && r.Dst.String() == "::/0" { + stillInstalled = true + } + } + return nil + }) + if err != nil { + t.Fatalf("verify route removal: %v", err) + } + if stillInstalled { + t.Fatal("::/0 route is still present in the VRF's table after the disable pass") + } +} diff --git a/internal/plumbing/srv6/egress.go b/internal/plumbing/srv6/egress.go index 252e44b5..fe188f68 100644 --- a/internal/plumbing/srv6/egress.go +++ b/internal/plumbing/srv6/egress.go @@ -106,11 +106,24 @@ func RouteEgressAdd(prefix *net.IPNet, gateway net.IP, tableID uint32) error { return netlink.RouteReplace(route) } -// RouteEgressDel removes the SEG6 encap route for prefix from routing table tableID. +// RouteEgressDel removes the SEG6 encap route for prefix from routing table +// tableID. +// +// Deliberately does not set Encap on the delete request the way the +// analogous add/replace path does: RTM_DELROUTE only needs Dst+Table to +// identify which route to remove, but netlink.RouteDel's shared encoding +// path (routeHandle, used by every Route* function regardless of message +// type) still tries to encode whatever Encap is set to, and +// nl.EncodeSEG6Encap unconditionally rejects a SEG6Encap with zero +// Segments — exactly what an empty &netlink.SEG6Encap{} is. Every call to +// this function failed with "EncodeSEG6Encap: No Segment in srh" until +// this fix (datum-cloud/enhancements#865's internal/egressroute is the +// first caller with a test that actually exercises the delete path; +// existing production callers — internal/runtime/gobgp/monitor.go's BGP +// path-withdrawal handler — had none). func RouteEgressDel(prefix *net.IPNet, tableID uint32) error { return netlink.RouteDel(&netlink.Route{ Dst: prefix, Table: int(tableID), - Encap: &netlink.SEG6Encap{}, }) } diff --git a/internal/plumbing/srv6/egress_test.go b/internal/plumbing/srv6/egress_test.go index f7216f66..4c712675 100644 --- a/internal/plumbing/srv6/egress_test.go +++ b/internal/plumbing/srv6/egress_test.go @@ -175,3 +175,91 @@ func TestRouteEgressAdd_InstallsForBothPrefixFamilies(t *testing.T) { }) } } + +// TestRouteEgressDel_RemovesInstalledRoute is the regression test for a bug +// found while building datum-cloud/enhancements#865's egress route +// reconciler: every call to RouteEgressDel failed with "EncodeSEG6Encap: No +// Segment in srh", because it set Encap to an empty &netlink.SEG6Encap{} +// on the delete request, and netlink.RouteDel's shared encoding path +// (routeHandle, used by every Route* function regardless of message type) +// tries to encode whatever Encap is set to -- unconditionally rejecting a +// SEG6Encap with zero Segments. This was a real, previously-untested bug in +// already-shipped production code: internal/runtime/gobgp/monitor.go's BGP +// path-withdrawal handler has called this function since it was written, +// with nothing to notice every call was actually failing. +func TestRouteEgressDel_RemovesInstalledRoute(t *testing.T) { + requireRoot(t) + + const ( + ifaceName = "srv6del0" + ifaceAddr = "2001:db8:2::1/64" + sidRoute = "2001:db8:9::9/128" + sidGw = "2001:db8:2::2" + sid = "2001:db8:9::9" + prefix = "fd20:10:ff03::/96" + table = 101 + ) + + nsObj, err := ns.TempNetNS() + if err != nil { + t.Fatalf("create test netns: %v", err) + } + defer func() { _ = nsObj.Close() }() + + _, dst, err := net.ParseCIDR(prefix) + if err != nil { + t.Fatalf("ParseCIDR: %v", err) + } + + err = nsObj.Do(func(_ ns.NetNS) error { + dummy := &netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Name: ifaceName}} + if err := netlink.LinkAdd(dummy); err != nil { + return fmt.Errorf("add dummy link: %w", err) + } + if err := netlink.LinkSetUp(dummy); err != nil { + return fmt.Errorf("set link up: %w", err) + } + addr, err := netlink.ParseAddr(ifaceAddr) + if err != nil { + return fmt.Errorf("parse addr: %w", err) + } + if err := netlink.AddrAdd(dummy, addr); err != nil { + return fmt.Errorf("add addr: %w", err) + } + _, sidDst, err := net.ParseCIDR(sidRoute) + if err != nil { + return fmt.Errorf("parse SID route: %w", err) + } + route := &netlink.Route{LinkIndex: dummy.Attrs().Index, Dst: sidDst, Gw: net.ParseIP(sidGw)} + if err := netlink.RouteAdd(route); err != nil { + return fmt.Errorf("add route to SID: %w", err) + } + return RouteEgressAdd(dst, net.ParseIP(sid), table) + }) + if err != nil { + t.Fatalf("setup (install route to delete): %v", err) + } + + err = nsObj.Do(func(_ ns.NetNS) error { + return RouteEgressDel(dst, table) + }) + if err != nil { + t.Fatalf("RouteEgressDel: %v, want success", err) + } + + err = nsObj.Do(func(_ ns.NetNS) error { + routes, err := netlink.RouteListFiltered(netlink.FAMILY_V6, &netlink.Route{Table: table}, netlink.RT_FILTER_TABLE) + if err != nil { + return err + } + for _, r := range routes { + if r.Dst != nil && r.Dst.String() == dst.String() { + return fmt.Errorf("route to %s still present in table %d after RouteEgressDel", dst, table) + } + } + return nil + }) + if err != nil { + t.Error(err) + } +} diff --git a/internal/plumbing/vrf/vrf.go b/internal/plumbing/vrf/vrf.go index ebf75c39..767eabd1 100644 --- a/internal/plumbing/vrf/vrf.go +++ b/internal/plumbing/vrf/vrf.go @@ -12,6 +12,8 @@ import ( "errors" "fmt" "math" + "regexp" + "strings" "sync" "github.com/vishvananda/netlink" @@ -155,6 +157,38 @@ func flush(vrfID uint32) error { return nil } +// vrfNameRegex matches the deterministic VRF interface name pattern this +// package's own Add generates ("G%09sV" — see intf.GenerateInterfaceNameVRF) +// — one VRF per VPC, shared across every attachment on that VPC on this +// node. Base62 includes digits and letters. Mirrors internal/gc/gc.go's +// identical, older regex (kept separate rather than shared: this is a new, +// narrower need — reading a VPC back out of a name, not the GC sweep's own +// orphan-detection bookkeeping — datum-cloud/enhancements#865). +var vrfNameRegex = regexp.MustCompile(`^G([A-Za-z0-9]{9})V$`) + +// legacyVRFNameRegex matches the VRF interface name this package generated +// before the VRF became per-VPC: the template was "G%09s%03sV", carrying a +// VPCAttachment segment the current per-VPC name no longer does. A node +// upgraded in place keeps whatever VRFs it created under the old template +// — see internal/gc/gc.go's identical regex for the fuller history. +var legacyVRFNameRegex = regexp.MustCompile(`^G([A-Za-z0-9]{9})[A-Za-z0-9]{3}V$`) + +// ResolveVPC extracts the base62-encoded VPC a Galactic-managed VRF +// interface belongs to, accepting both the current per-VPC name and the +// legacy pre-rename name — both resolve to the same VPC, since only the +// leading base62 segment ever encodes it. Returns ok=false for a name that +// doesn't match either Galactic VRF shape at all (e.g. a non-Galactic VRF +// interface on the same host). +func ResolveVPC(name string) (vpc string, ok bool) { + if matches := vrfNameRegex.FindStringSubmatch(name); matches != nil { + return strings.TrimLeft(matches[1], "0"), true + } + if matches := legacyVRFNameRegex.FindStringSubmatch(name); matches != nil { + return strings.TrimLeft(matches[1], "0"), true + } + return "", false +} + // ListVRFLinks returns all VRF interfaces currently present on the host. func ListVRFLinks() ([]*netlink.Vrf, error) { links, err := netlink.LinkList() diff --git a/internal/plumbing/vrf/vrf_test.go b/internal/plumbing/vrf/vrf_test.go index 8eb0ebc7..0151389c 100644 --- a/internal/plumbing/vrf/vrf_test.go +++ b/internal/plumbing/vrf/vrf_test.go @@ -145,3 +145,31 @@ func TestDelete_ThenAddRecreates(t *testing.T) { t.Errorf("Exists after re-Add: %v", err) } } + +// TestResolveVPC covers both current and legacy Galactic VRF interface name +// shapes, plus non-Galactic names (datum-cloud/enhancements#865's +// internal/egressroute needs this to identify which local VRF interfaces +// are its own). +func TestResolveVPC(t *testing.T) { + tests := []struct { + name string + wantVPC string + wantOK bool + }{ + {name: "G000000123V", wantVPC: "123", wantOK: true}, // current per-VPC shape + {name: "G000000123abcV", wantVPC: "123", wantOK: true}, // legacy shape (VPCAttachment segment) + {name: "eth0", wantOK: false}, // not a Galactic VRF at all + {name: "Gzzz_nonexistent_vpczzz_nonexistentV", wantOK: false}, // wrong shape entirely + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + vpc, ok := vrf.ResolveVPC(tc.name) + if ok != tc.wantOK { + t.Fatalf("ResolveVPC(%q) ok = %v, want %v", tc.name, ok, tc.wantOK) + } + if ok && vpc != tc.wantVPC { + t.Errorf("ResolveVPC(%q) vpc = %q, want %q", tc.name, vpc, tc.wantVPC) + } + }) + } +}