From c3e2d57209eb5cb02a0a767e30b282f635b5c829 Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Mon, 25 May 2026 14:44:22 +1000 Subject: [PATCH] refactor(network): move NATS VPC subscribers to network/subscribers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4 Slice C (mulga-siv-125.6). Promotes the NATS event handlers out of services/vpcd into a new spinifex/network/subscribers package. Subscriber holds the topology/policy/external managers directly — the lazy *Once adapter shim (topology_manager.go) is deleted along with vpcd's local event-struct copies. launchService now constructs topology.LiveManager eagerly with a DNS callback derived from external pools (preserving the unscoped-pool lookup old TopologyHandler.lmDNSServer did), builds every manager up front, then hands them to subscribers.New + Subscribe. reconcile.New receives topology.Manager directly instead of through the adapter. Bridge-mode constants and waitForFlowsHV move into vpcd.go alongside the launcher; they were left behind as topology.go's last responsibilities. Handler-layer tests (topology_test.go, security_test.go, integration_test.go) are dropped: substance is covered by network/topology, network/policy, network/external manager tests. subscribers/subscribers_test.go adds focused coverage: missing-dep rejection, topic registration smoke, and bad-JSON envelope per topic. services/vpcd is now down to the launcher + the ovn/mock compat aliases (removed in Slice D when the launcher promotes to cmd/vpcd/). --- spinifex/handlers/ec2/vpc/eni.go | 4 +- spinifex/network/subscribers/events.go | 95 + spinifex/network/subscribers/respond.go | 34 + spinifex/network/subscribers/security.go | 104 + spinifex/network/subscribers/subscribers.go | 100 + .../network/subscribers/subscribers_test.go | 144 + spinifex/network/subscribers/topology.go | 311 ++ spinifex/services/vpcd/integration_test.go | 603 ---- spinifex/services/vpcd/security.go | 143 - spinifex/services/vpcd/security_test.go | 261 -- spinifex/services/vpcd/topology.go | 719 ---- spinifex/services/vpcd/topology_manager.go | 157 - spinifex/services/vpcd/topology_test.go | 3052 ----------------- spinifex/services/vpcd/vpcd.go | 143 +- 14 files changed, 896 insertions(+), 4974 deletions(-) create mode 100644 spinifex/network/subscribers/events.go create mode 100644 spinifex/network/subscribers/respond.go create mode 100644 spinifex/network/subscribers/security.go create mode 100644 spinifex/network/subscribers/subscribers.go create mode 100644 spinifex/network/subscribers/subscribers_test.go create mode 100644 spinifex/network/subscribers/topology.go delete mode 100644 spinifex/services/vpcd/integration_test.go delete mode 100644 spinifex/services/vpcd/security.go delete mode 100644 spinifex/services/vpcd/security_test.go delete mode 100644 spinifex/services/vpcd/topology.go delete mode 100644 spinifex/services/vpcd/topology_manager.go delete mode 100644 spinifex/services/vpcd/topology_test.go diff --git a/spinifex/handlers/ec2/vpc/eni.go b/spinifex/handlers/ec2/vpc/eni.go index ae2d3b728..4bc6d145d 100644 --- a/spinifex/handlers/ec2/vpc/eni.go +++ b/spinifex/handlers/ec2/vpc/eni.go @@ -588,8 +588,8 @@ func generateENIMac(eniId string) string { } // portEventPayload is the wire shape for vpc.create-port / vpc.delete-port. -// Mirrors vpcd.PortEvent — duplicated here to avoid a vpcd → handlers import -// cycle. +// Mirrors network/subscribers.PortEvent — duplicated here to avoid a +// subscribers → handlers import cycle. type portEventPayload struct { NetworkInterfaceId string `json:"network_interface_id"` SubnetId string `json:"subnet_id"` diff --git a/spinifex/network/subscribers/events.go b/spinifex/network/subscribers/events.go new file mode 100644 index 000000000..2d8cb4966 --- /dev/null +++ b/spinifex/network/subscribers/events.go @@ -0,0 +1,95 @@ +package subscribers + +// NATS topic names for VPC lifecycle events published by daemon. +const ( + TopicVPCCreate = "vpc.create" + TopicVPCDelete = "vpc.delete" + TopicSubnetCreate = "vpc.create-subnet" + TopicSubnetDelete = "vpc.delete-subnet" + TopicCreatePort = "vpc.create-port" + TopicDeletePort = "vpc.delete-port" + TopicUpdatePortSGs = "vpc.update-port-sgs" + TopicIGWAttach = "vpc.igw-attach" + TopicIGWDetach = "vpc.igw-detach" + TopicAddNAT = "vpc.add-nat" + TopicDeleteNAT = "vpc.delete-nat" + TopicAddNATGateway = "vpc.add-nat-gateway" + TopicDeleteNATGateway = "vpc.delete-nat-gateway" + TopicCreateSG = "vpc.create-sg" + TopicDeleteSG = "vpc.delete-sg" + TopicUpdateSG = "vpc.update-sg" +) + +// VPCEvent is published on vpc.create / vpc.delete. +type VPCEvent struct { + VpcId string `json:"vpc_id"` + CidrBlock string `json:"cidr_block"` + VNI int64 `json:"vni"` +} + +// SubnetEvent is published on vpc.create-subnet / vpc.delete-subnet. +type SubnetEvent struct { + SubnetId string `json:"subnet_id"` + VpcId string `json:"vpc_id"` + CidrBlock string `json:"cidr_block"` +} + +// PortEvent is published on vpc.create-port / vpc.delete-port. +// +// SecurityGroupIds carries the SG membership the port should join at create +// time so vpcd can wire OVN port-group membership atomically with the LSP +// create. Empty on delete-port (handleDeletePort discovers current +// memberships from the libovsdb cache). +type PortEvent struct { + NetworkInterfaceId string `json:"network_interface_id"` + SubnetId string `json:"subnet_id"` + VpcId string `json:"vpc_id"` + PrivateIpAddress string `json:"private_ip_address"` + MacAddress string `json:"mac_address"` + SecurityGroupIds []string `json:"security_group_ids,omitempty"` +} + +// UpdatePortSGsEvent is published on vpc.update-port-sgs after +// ModifyNetworkInterfaceAttribute changes an ENI's SG membership. The payload +// is declarative — vpcd reads its libovsdb cache to discover current +// memberships and computes the diff against SecurityGroupIds. +type UpdatePortSGsEvent struct { + NetworkInterfaceId string `json:"network_interface_id"` + PrivateIpAddress string `json:"private_ip_address"` + SecurityGroupIds []string `json:"security_group_ids"` +} + +// NATEvent is published on vpc.add-nat / vpc.delete-nat for 1:1 public IP NAT. +type NATEvent struct { + VpcId string `json:"vpc_id"` + ExternalIP string `json:"external_ip"` + LogicalIP string `json:"logical_ip"` + PortName string `json:"port_name"` // logical port for distributed NAT + MAC string `json:"mac"` // external MAC for distributed NAT +} + +// NATGatewayEvent is published on vpc.add-nat-gateway / vpc.delete-nat-gateway. +type NATGatewayEvent struct { + VpcId string `json:"vpc_id"` + NatGatewayId string `json:"nat_gateway_id"` + PublicIp string `json:"public_ip"` + SubnetCidr string `json:"subnet_cidr"` // private subnet CIDR for SNAT rule +} + +// SGRule mirrors the on-wire payload from handlers/ec2/vpc.SGRule. Kept +// local so subscribers do not import the handlers package. +type SGRule struct { + IpProtocol string `json:"ip_protocol"` + FromPort int64 `json:"from_port"` + ToPort int64 `json:"to_port"` + CidrIp string `json:"cidr_ip,omitempty"` + SourceSG string `json:"source_sg,omitempty"` +} + +// SGEvent mirrors handlers/ec2/vpc.SGEvent. +type SGEvent struct { + GroupId string `json:"group_id"` + VpcId string `json:"vpc_id"` + IngressRules []SGRule `json:"ingress_rules,omitempty"` + EgressRules []SGRule `json:"egress_rules,omitempty"` +} diff --git a/spinifex/network/subscribers/respond.go b/spinifex/network/subscribers/respond.go new file mode 100644 index 000000000..21839b43d --- /dev/null +++ b/spinifex/network/subscribers/respond.go @@ -0,0 +1,34 @@ +package subscribers + +import ( + "encoding/json" + "log/slog" + + "github.com/nats-io/nats.go" +) + +type respondResponse struct { + Success bool `json:"success"` + Error string `json:"error,omitempty"` +} + +// respond sends a JSON success/error envelope to a NATS request. Fire-and- +// forget when no reply is set on msg. +func respond(msg *nats.Msg, err error) { + if msg.Reply == "" { + return + } + resp := respondResponse{Success: true} + if err != nil { + resp.Success = false + resp.Error = err.Error() + } + data, marshalErr := json.Marshal(resp) + if marshalErr != nil { + slog.Error("subscribers: failed to marshal NATS response", "err", marshalErr) + data = []byte(`{"success":false,"error":"internal marshal failure"}`) + } + if err := msg.Respond(data); err != nil { + slog.Error("subscribers: failed to respond to NATS request", "err", err) + } +} diff --git a/spinifex/network/subscribers/security.go b/spinifex/network/subscribers/security.go new file mode 100644 index 000000000..112f5d37b --- /dev/null +++ b/spinifex/network/subscribers/security.go @@ -0,0 +1,104 @@ +package subscribers + +import ( + "context" + "encoding/json" + "log/slog" + + "github.com/mulgadc/spinifex/spinifex/network/policy" + "github.com/nats-io/nats.go" +) + +func (e SGEvent) toSpec() policy.SGSpec { + return policy.SGSpec{ + GroupID: e.GroupId, + VPCID: e.VpcId, + IngressRules: toPolicyRules(e.IngressRules), + EgressRules: toPolicyRules(e.EgressRules), + } +} + +func toPolicyRules(in []SGRule) []policy.Rule { + out := make([]policy.Rule, len(in)) + for i, r := range in { + out[i] = policy.Rule{ + IPProtocol: r.IpProtocol, + FromPort: r.FromPort, + ToPort: r.ToPort, + CIDR: r.CidrIp, + SourceSG: r.SourceSG, + } + } + return out +} + +// handleCreateSG ensures the OVN port group exists then applies the ACL set +// for a new security group. Two-step (topology + policy) because port-group +// lifecycle is L2 and ACL programming is L3 — see plan §8.1. +func (s *Subscriber) handleCreateSG(msg *nats.Msg) { + var evt SGEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.create-sg event", "err", err) + respond(msg, err) + return + } + ctx := context.Background() + if err := s.topology.EnsureSGPortGroup(ctx, evt.GroupId); err != nil { + slog.Error("subscribers: EnsureSGPortGroup failed", "group_id", evt.GroupId, "err", err) + respond(msg, err) + return + } + if err := s.sg.EnsureSG(ctx, evt.toSpec()); err != nil { + slog.Error("subscribers: EnsureSG failed", "group_id", evt.GroupId, "err", err) + respond(msg, err) + return + } + slog.Info("subscribers: created security group", + "group_id", evt.GroupId, + "vpc_id", evt.VpcId, + "ingress_rules", len(evt.IngressRules), + "egress_rules", len(evt.EgressRules), + ) + respond(msg, nil) +} + +// handleDeleteSG removes the SG's port group (and its ACLs in one OVN +// transaction). Idempotent on already-absent. +func (s *Subscriber) handleDeleteSG(msg *nats.Msg) { + var evt SGEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.delete-sg event", "err", err) + respond(msg, err) + return + } + if err := s.topology.DeleteSGPortGroup(context.Background(), evt.GroupId); err != nil { + slog.Error("subscribers: DeleteSGPortGroup failed", "group_id", evt.GroupId, "err", err) + respond(msg, err) + return + } + slog.Info("subscribers: deleted security group", "group_id", evt.GroupId, "vpc_id", evt.VpcId) + respond(msg, nil) +} + +// handleUpdateSG replaces the SG's ACL set with the rules from the event. +// Port-group lifecycle is unaffected — UpdateSG only touches ACLs. +func (s *Subscriber) handleUpdateSG(msg *nats.Msg) { + var evt SGEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.update-sg event", "err", err) + respond(msg, err) + return + } + if err := s.sg.UpdateSG(context.Background(), evt.toSpec()); err != nil { + slog.Error("subscribers: UpdateSG failed", "group_id", evt.GroupId, "err", err) + respond(msg, err) + return + } + slog.Info("subscribers: updated security group", + "group_id", evt.GroupId, + "vpc_id", evt.VpcId, + "ingress_rules", len(evt.IngressRules), + "egress_rules", len(evt.EgressRules), + ) + respond(msg, nil) +} diff --git a/spinifex/network/subscribers/subscribers.go b/spinifex/network/subscribers/subscribers.go new file mode 100644 index 000000000..067051a96 --- /dev/null +++ b/spinifex/network/subscribers/subscribers.go @@ -0,0 +1,100 @@ +// Package subscribers translates VPC lifecycle NATS events into calls +// against the network/{topology,policy,external} managers. Each handler is +// a thin JSON-decode + manager-call adapter; ACL semantics, OVN object +// lifecycle, and convergence live in the managers themselves. Constructed +// eagerly with every dependency injected — no lazy *Once builders. +package subscribers + +import ( + "errors" + "fmt" + "log/slog" + + "github.com/mulgadc/spinifex/spinifex/network/external" + "github.com/mulgadc/spinifex/spinifex/network/policy" + "github.com/mulgadc/spinifex/spinifex/network/topology" + "github.com/nats-io/nats.go" +) + +// QueueGroup is the NATS queue group used for every VPC lifecycle topic. +// Membership is exclusive within the cluster: only one vpcd processes any +// given event, since all vpcds share the same OVN NB DB. +const QueueGroup = "vpcd-workers" + +// Subscriber wires VPC lifecycle NATS topics to the network managers. +type Subscriber struct { + topology topology.Manager + sg policy.SecurityGroupManager + nat policy.NATManager + igw external.IGWManager +} + +// Config is the construction-time bag. Every field is required. +type Config struct { + Topology topology.Manager + SG policy.SecurityGroupManager + NAT policy.NATManager + IGW external.IGWManager +} + +// New constructs a Subscriber, returning an error when any manager is nil. +func New(cfg Config) (*Subscriber, error) { + switch { + case cfg.Topology == nil: + return nil, errors.New("subscribers: Topology manager required") + case cfg.SG == nil: + return nil, errors.New("subscribers: SecurityGroupManager required") + case cfg.NAT == nil: + return nil, errors.New("subscribers: NATManager required") + case cfg.IGW == nil: + return nil, errors.New("subscribers: IGWManager required") + } + return &Subscriber{ + topology: cfg.Topology, + sg: cfg.SG, + nat: cfg.NAT, + igw: cfg.IGW, + }, nil +} + +// Subscribe registers NATS queue subscriptions for every VPC lifecycle +// topic. Cleanup on partial failure: previously-registered subscriptions +// are unsubscribed before the error is returned. +func (s *Subscriber) Subscribe(nc *nats.Conn) ([]*nats.Subscription, error) { + type sub struct { + topic string + handler nats.MsgHandler + } + subs := []sub{ + {TopicVPCCreate, s.handleVPCCreate}, + {TopicVPCDelete, s.handleVPCDelete}, + {TopicSubnetCreate, s.handleSubnetCreate}, + {TopicSubnetDelete, s.handleSubnetDelete}, + {TopicCreatePort, s.handleCreatePort}, + {TopicDeletePort, s.handleDeletePort}, + {TopicUpdatePortSGs, s.handleUpdatePortSGs}, + {TopicIGWAttach, s.handleIGWAttach}, + {TopicIGWDetach, s.handleIGWDetach}, + {TopicAddNAT, s.handleAddNAT}, + {TopicDeleteNAT, s.handleDeleteNAT}, + {TopicAddNATGateway, s.handleAddNATGateway}, + {TopicDeleteNATGateway, s.handleDeleteNATGateway}, + {TopicCreateSG, s.handleCreateSG}, + {TopicDeleteSG, s.handleDeleteSG}, + {TopicUpdateSG, s.handleUpdateSG}, + } + + var result []*nats.Subscription + for _, item := range subs { + natsSub, err := nc.QueueSubscribe(item.topic, QueueGroup, item.handler) + if err != nil { + for _, r := range result { + _ = r.Unsubscribe() + } + return nil, fmt.Errorf("subscribe %s: %w", item.topic, err) + } + result = append(result, natsSub) + slog.Info("Subscribed to VPC topic", "topic", item.topic) + } + return result, nil +} diff --git a/spinifex/network/subscribers/subscribers_test.go b/spinifex/network/subscribers/subscribers_test.go new file mode 100644 index 000000000..35418759f --- /dev/null +++ b/spinifex/network/subscribers/subscribers_test.go @@ -0,0 +1,144 @@ +package subscribers + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/mulgadc/spinifex/spinifex/network/external" + "github.com/mulgadc/spinifex/spinifex/network/ovn/mock" + "github.com/mulgadc/spinifex/spinifex/network/policy" + "github.com/mulgadc/spinifex/spinifex/network/topology" + "github.com/mulgadc/spinifex/spinifex/testutil" +) + +const requestTimeout = 2 * time.Second + +// newTestSubscriber wires every manager against one MockOVNClient so the +// subscriber acts as a real NATS → OVN bridge in tests. The mock OVN client +// satisfies every manager constructor's nil-check at zero cost. +func newTestSubscriber(t *testing.T) (*Subscriber, *mock.Client) { + t.Helper() + m := mock.New() + _ = m.Connect(context.Background()) + + topo := topology.NewLiveManager(m) + sg := policy.NewSecurityGroupManager(m) + nat, err := policy.NewNATManager(m, policy.NATModeDistributed) + if err != nil { + t.Fatalf("NewNATManager: %v", err) + } + routes := policy.NewRouteManager(m) + igw, err := external.NewIGWManager(external.IGWManagerConfig{ + OVN: m, + Routes: routes, + NAT: nat, + Allocator: external.NewStaticRangeAllocator(m), + Chassis: []string{"hv1"}, + NATMode: policy.NATModeDistributed, + }) + if err != nil { + t.Fatalf("NewIGWManager: %v", err) + } + s, err := New(Config{Topology: topo, SG: sg, NAT: nat, IGW: igw}) + if err != nil { + t.Fatalf("New: %v", err) + } + return s, m +} + +func TestNew_RejectsMissingDeps(t *testing.T) { + cases := []struct { + name string + cfg Config + }{ + {"no topology", Config{SG: policy.NewSecurityGroupManager(mock.New())}}, + {"no sg", Config{Topology: topology.NewLiveManager(mock.New())}}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if _, err := New(c.cfg); err == nil { + t.Fatal("expected error, got nil") + } + }) + } +} + +func TestSubscribe_RegistersAllTopics(t *testing.T) { + _, nc := testutil.StartTestNATS(t) + sub, _ := newTestSubscriber(t) + + subs, err := sub.Subscribe(nc) + if err != nil { + t.Fatalf("Subscribe: %v", err) + } + defer func() { + for _, s := range subs { + _ = s.Unsubscribe() + } + }() + + wantTopics := []string{ + TopicVPCCreate, TopicVPCDelete, + TopicSubnetCreate, TopicSubnetDelete, + TopicCreatePort, TopicDeletePort, TopicUpdatePortSGs, + TopicIGWAttach, TopicIGWDetach, + TopicAddNAT, TopicDeleteNAT, + TopicAddNATGateway, TopicDeleteNATGateway, + TopicCreateSG, TopicDeleteSG, TopicUpdateSG, + } + if len(subs) != len(wantTopics) { + t.Fatalf("subscription count: got %d, want %d", len(subs), len(wantTopics)) + } + got := map[string]bool{} + for _, s := range subs { + got[s.Subject] = true + } + for _, topic := range wantTopics { + if !got[topic] { + t.Errorf("missing subscription for topic %q", topic) + } + } +} + +// TestBadJSON_AllRequestReplies verifies every request/reply topic returns a +// structured error envelope (not a NATS timeout) when given malformed JSON. +// Fire-and-forget topics (NAT gateway) just have to not panic. +func TestBadJSON_AllRequestReplies(t *testing.T) { + _, nc := testutil.StartTestNATS(t) + sub, _ := newTestSubscriber(t) + subs, err := sub.Subscribe(nc) + if err != nil { + t.Fatalf("Subscribe: %v", err) + } + defer func() { + for _, s := range subs { + _ = s.Unsubscribe() + } + }() + + topics := []string{ + TopicVPCCreate, TopicVPCDelete, + TopicSubnetCreate, TopicSubnetDelete, + TopicCreatePort, TopicDeletePort, TopicUpdatePortSGs, + TopicIGWAttach, TopicIGWDetach, + TopicAddNAT, TopicDeleteNAT, + TopicCreateSG, TopicDeleteSG, TopicUpdateSG, + } + for _, topic := range topics { + t.Run(topic, func(t *testing.T) { + resp, err := nc.Request(topic, []byte("not json"), requestTimeout) + if err != nil { + t.Fatalf("request %s: %v", topic, err) + } + var env respondResponse + if err := json.Unmarshal(resp.Data, &env); err != nil { + t.Fatalf("unmarshal envelope: %v", err) + } + if env.Success { + t.Errorf("topic %s: expected success=false on bad JSON, got success=true", topic) + } + }) + } +} diff --git a/spinifex/network/subscribers/topology.go b/spinifex/network/subscribers/topology.go new file mode 100644 index 000000000..3cc7b8834 --- /dev/null +++ b/spinifex/network/subscribers/topology.go @@ -0,0 +1,311 @@ +package subscribers + +import ( + "context" + "encoding/json" + "log/slog" + "net" + "net/netip" + + "github.com/mulgadc/spinifex/spinifex/network/external" + "github.com/mulgadc/spinifex/spinifex/network/policy" + "github.com/mulgadc/spinifex/spinifex/network/topology" + "github.com/mulgadc/spinifex/spinifex/types" + "github.com/nats-io/nats.go" +) + +// --- VPC (LogicalRouter) --- + +func (s *Subscriber) handleVPCCreate(msg *nats.Msg) { + var evt VPCEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.create event", "err", err) + respond(msg, err) + return + } + spec := topology.VPCSpec{VPCID: evt.VpcId, VNI: evt.VNI} + if evt.CidrBlock != "" { + cidr, err := netip.ParsePrefix(evt.CidrBlock) + if err != nil { + slog.Error("subscribers: invalid CIDR in vpc.create event", "cidr", evt.CidrBlock, "err", err) + respond(msg, err) + return + } + spec.CIDR = cidr + } + if err := s.topology.EnsureVPC(context.Background(), spec); err != nil { + slog.Error("subscribers: EnsureVPC failed", "vpc_id", evt.VpcId, "err", err) + respond(msg, err) + return + } + respond(msg, nil) +} + +func (s *Subscriber) handleVPCDelete(msg *nats.Msg) { + var evt VPCEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.delete event", "err", err) + respond(msg, err) + return + } + if err := s.topology.DeleteVPC(context.Background(), evt.VpcId); err != nil { + slog.Error("subscribers: DeleteVPC failed", "vpc_id", evt.VpcId, "err", err) + respond(msg, err) + return + } + respond(msg, nil) +} + +// --- Subnet (LogicalSwitch + RouterPort + DHCP) --- + +func (s *Subscriber) handleSubnetCreate(msg *nats.Msg) { + var evt SubnetEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.create-subnet event", "err", err) + respond(msg, err) + return + } + cidr, err := netip.ParsePrefix(evt.CidrBlock) + if err != nil { + slog.Error("subscribers: invalid CIDR in vpc.create-subnet event", "cidr", evt.CidrBlock, "err", err) + respond(msg, err) + return + } + ctx := context.Background() + // vpc.create and vpc.create-subnet are independent NATS events with no + // inter-event ordering guarantee. Tenant bootstrap publishes both + // within <1ms, so EnsureSubnet can race the parent VPC's + // CreateLogicalRouter and fail with "logical router not found". + // EnsureVPC is idempotent on the router name (mulga-siv-133). + if err := s.topology.EnsureVPC(ctx, topology.VPCSpec{VPCID: evt.VpcId}); err != nil { + slog.Error("subscribers: EnsureVPC (subnet pre-create) failed", "vpc_id", evt.VpcId, "err", err) + respond(msg, err) + return + } + if err := s.topology.EnsureSubnet(ctx, topology.SubnetSpec{ + SubnetID: evt.SubnetId, + VPCID: evt.VpcId, + CIDR: cidr, + }); err != nil { + slog.Error("subscribers: EnsureSubnet failed", "subnet_id", evt.SubnetId, "err", err) + respond(msg, err) + return + } + respond(msg, nil) +} + +func (s *Subscriber) handleSubnetDelete(msg *nats.Msg) { + var evt SubnetEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.delete-subnet event", "err", err) + respond(msg, err) + return + } + spec := topology.SubnetSpec{SubnetID: evt.SubnetId, VPCID: evt.VpcId} + if evt.CidrBlock != "" { + if cidr, perr := netip.ParsePrefix(evt.CidrBlock); perr == nil { + spec.CIDR = cidr + } + } + if err := s.topology.DeleteSubnet(context.Background(), spec); err != nil { + slog.Error("subscribers: DeleteSubnet failed", "subnet_id", evt.SubnetId, "err", err) + respond(msg, err) + return + } + respond(msg, nil) +} + +// --- Port (LogicalSwitchPort for VM/ENI) --- + +func (s *Subscriber) handleCreatePort(msg *nats.Msg) { + var evt PortEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.create-port event", "err", err) + respond(msg, err) + return + } + ip, err := netip.ParseAddr(evt.PrivateIpAddress) + if err != nil { + slog.Error("subscribers: invalid private IP in vpc.create-port event", "ip", evt.PrivateIpAddress, "err", err) + respond(msg, err) + return + } + mac, err := net.ParseMAC(evt.MacAddress) + if err != nil { + slog.Error("subscribers: invalid MAC in vpc.create-port event", "mac", evt.MacAddress, "err", err) + respond(msg, err) + return + } + if err := s.topology.EnsurePort(context.Background(), topology.PortSpec{ + PortID: evt.NetworkInterfaceId, + SubnetID: evt.SubnetId, + VPCID: evt.VpcId, + PrivateIP: ip, + MAC: mac, + SGIDs: evt.SecurityGroupIds, + }); err != nil { + slog.Error("subscribers: EnsurePort failed", "eni_id", evt.NetworkInterfaceId, "err", err) + respond(msg, err) + return + } + respond(msg, nil) +} + +func (s *Subscriber) handleDeletePort(msg *nats.Msg) { + var evt PortEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.delete-port event", "err", err) + respond(msg, err) + return + } + if err := s.topology.DeletePort(context.Background(), topology.PortSpec{ + PortID: evt.NetworkInterfaceId, + SubnetID: evt.SubnetId, + VPCID: evt.VpcId, + }); err != nil { + slog.Error("subscribers: DeletePort failed", "eni_id", evt.NetworkInterfaceId, "err", err) + respond(msg, err) + return + } + respond(msg, nil) +} + +// handleUpdatePortSGs reconciles the port group membership for an ENI's LSP +// against the desired SG list in the event. The payload is declarative — the +// manager computes the add/remove diff from current OVN state. +func (s *Subscriber) handleUpdatePortSGs(msg *nats.Msg) { + var evt UpdatePortSGsEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.update-port-sgs event", "err", err) + respond(msg, err) + return + } + if err := s.topology.SetPortSecurityGroups(context.Background(), evt.NetworkInterfaceId, evt.SecurityGroupIds); err != nil { + slog.Error("subscribers: SetPortSecurityGroups failed", + "eni_id", evt.NetworkInterfaceId, "sgs", evt.SecurityGroupIds, "err", err) + respond(msg, err) + return + } + respond(msg, nil) +} + +// --- Internet Gateway (external connectivity + NAT) --- + +func (s *Subscriber) handleIGWAttach(msg *nats.Msg) { + var evt types.IGWEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.igw-attach event", "err", err) + respond(msg, err) + return + } + if err := s.igw.AttachIGW(context.Background(), external.IGWSpec{ + VPCID: evt.VpcId, + InternetGatewayID: evt.InternetGatewayId, + }); err != nil { + slog.Error("subscribers: AttachIGW failed", + "vpc_id", evt.VpcId, "igw_id", evt.InternetGatewayId, "err", err) + respond(msg, err) + return + } + respond(msg, nil) +} + +func (s *Subscriber) handleIGWDetach(msg *nats.Msg) { + var evt types.IGWEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.igw-detach event", "err", err) + respond(msg, err) + return + } + if err := s.igw.DetachIGW(context.Background(), evt.VpcId); err != nil { + slog.Error("subscribers: DetachIGW failed", "vpc_id", evt.VpcId, "err", err) + respond(msg, err) + return + } + slog.Info("subscribers: detached internet gateway from VPC", + "igw_id", evt.InternetGatewayId, "vpc_id", evt.VpcId) + respond(msg, nil) +} + +// --- NAT (dnat_and_snat for public IPs) --- + +func (s *Subscriber) handleAddNAT(msg *nats.Msg) { + var evt NATEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.add-nat event", "err", err) + respond(msg, err) + return + } + if err := s.nat.AddEIP(context.Background(), policy.EIPSpec{ + VPCID: evt.VpcId, + ExternalIP: evt.ExternalIP, + LogicalIP: evt.LogicalIP, + PortName: evt.PortName, + MAC: evt.MAC, + }); err != nil { + slog.Error("subscribers: AddEIP failed", + "vpc_id", evt.VpcId, "external_ip", evt.ExternalIP, + "logical_ip", evt.LogicalIP, "err", err) + respond(msg, err) + return + } + slog.Info("subscribers: added dnat_and_snat rule", + "vpc_id", evt.VpcId, "external_ip", evt.ExternalIP, + "logical_ip", evt.LogicalIP, "port", evt.PortName) + respond(msg, nil) +} + +func (s *Subscriber) handleDeleteNAT(msg *nats.Msg) { + var evt NATEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.delete-nat event", "err", err) + respond(msg, err) + return + } + if err := s.nat.DeleteEIP(context.Background(), evt.VpcId, evt.LogicalIP); err != nil { + slog.Error("subscribers: DeleteEIP failed", + "vpc_id", evt.VpcId, "logical_ip", evt.LogicalIP, "err", err) + respond(msg, err) + return + } + slog.Info("subscribers: deleted dnat_and_snat rule", + "vpc_id", evt.VpcId, "logical_ip", evt.LogicalIP) + respond(msg, nil) +} + +func (s *Subscriber) handleAddNATGateway(msg *nats.Msg) { + var evt NATGatewayEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.add-nat-gateway event", "err", err) + return + } + if err := s.nat.AddNATGateway(context.Background(), policy.NATGWSpec{ + VPCID: evt.VpcId, + NATGatewayID: evt.NatGatewayId, + PublicIP: evt.PublicIp, + SubnetCIDR: evt.SubnetCidr, + }); err != nil { + slog.Error("subscribers: AddNATGateway failed", + "vpc_id", evt.VpcId, "natgw_id", evt.NatGatewayId, + "public_ip", evt.PublicIp, "subnet_cidr", evt.SubnetCidr, "err", err) + return + } + slog.Info("subscribers: NAT Gateway SNAT rule added", + "vpc_id", evt.VpcId, "natgw_id", evt.NatGatewayId, + "public_ip", evt.PublicIp, "subnet_cidr", evt.SubnetCidr) +} + +func (s *Subscriber) handleDeleteNATGateway(msg *nats.Msg) { + var evt NATGatewayEvent + if err := json.Unmarshal(msg.Data, &evt); err != nil { + slog.Error("subscribers: failed to unmarshal vpc.delete-nat-gateway event", "err", err) + return + } + if err := s.nat.DeleteNATGateway(context.Background(), evt.VpcId, evt.SubnetCidr); err != nil { + slog.Warn("subscribers: DeleteNATGateway failed", + "vpc_id", evt.VpcId, "subnet_cidr", evt.SubnetCidr, "err", err) + return + } + slog.Info("subscribers: NAT Gateway SNAT rule removed", + "vpc_id", evt.VpcId, "natgw_id", evt.NatGatewayId, "subnet_cidr", evt.SubnetCidr) +} diff --git a/spinifex/services/vpcd/integration_test.go b/spinifex/services/vpcd/integration_test.go deleted file mode 100644 index 93ef961d2..000000000 --- a/spinifex/services/vpcd/integration_test.go +++ /dev/null @@ -1,603 +0,0 @@ -package vpcd - -import ( - "context" - "encoding/json" - "testing" - "time" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ec2" - handlers_ec2_igw "github.com/mulgadc/spinifex/spinifex/handlers/ec2/igw" - handlers_ec2_vpc "github.com/mulgadc/spinifex/spinifex/handlers/ec2/vpc" - "github.com/mulgadc/spinifex/spinifex/testutil" - "github.com/mulgadc/spinifex/spinifex/utils" - "github.com/nats-io/nats-server/v2/server" - "github.com/nats-io/nats.go" -) - -const testIGWAccountID = "123456789012" - -// startTestJetStreamNATS starts an embedded NATS server with JetStream for integration tests. -func startTestJetStreamNATS(t *testing.T) (*server.Server, *nats.Conn) { - t.Helper() - ns, nc, _ := testutil.StartTestJetStream(t) - return ns, nc -} - -// TestIntegration_VPCLifecycle tests the full VPC lifecycle: -// VPC create → subnet create → ENI create → IGW attach → IGW detach → cleanup. -// This validates the NATS event flow between the daemon and vpcd topology handler. -func TestIntegration_VPCLifecycle(t *testing.T) { - _, nc := startTestJetStreamNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - // Set up topology handler subscriptions (simulates vpcd) - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe topology handler: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // === Phase 1: Create VPC === - vpcEvt := VPCEvent{VpcId: "vpc-integ1", CidrBlock: "10.0.0.0/16", VNI: 1000} - vpcData, _ := json.Marshal(vpcEvt) - resp, err := nc.Request(TopicVPCCreate, vpcData, 5*time.Second) - if err != nil { - t.Fatalf("vpc.create: %v", err) - } - assertSuccess(t, resp, "create VPC") - - // Verify OVN: logical router exists - router, err := mock.GetLogicalRouter(ctx, "vpc-vpc-integ1") - if err != nil { - t.Fatalf("expected VPC router: %v", err) - } - if router.ExternalIDs["spinifex:vpc_id"] != "vpc-integ1" { - t.Errorf("router vpc_id = %s, want vpc-integ1", router.ExternalIDs["spinifex:vpc_id"]) - } - - // === Phase 2: Create Subnet === - subEvt := SubnetEvent{SubnetId: "subnet-integ1", VpcId: "vpc-integ1", CidrBlock: "10.0.1.0/24"} - subData, _ := json.Marshal(subEvt) - resp, err = nc.Request(TopicSubnetCreate, subData, 5*time.Second) - if err != nil { - t.Fatalf("vpc.create-subnet: %v", err) - } - assertSuccess(t, resp, "create subnet") - - // Verify OVN: logical switch, router port, DHCP - ls, err := mock.GetLogicalSwitch(ctx, "subnet-subnet-integ1") - if err != nil { - t.Fatalf("expected logical switch: %v", err) - } - if ls.ExternalIDs["spinifex:subnet_id"] != "subnet-integ1" { - t.Errorf("switch subnet_id = %s, want subnet-integ1", ls.ExternalIDs["spinifex:subnet_id"]) - } - dhcp, err := mock.FindDHCPOptionsByCIDR(ctx, "10.0.1.0/24") - if err != nil { - t.Fatalf("expected DHCP options: %v", err) - } - if dhcp.Options["router"] != "10.0.1.1" { - t.Errorf("DHCP router = %s, want 10.0.1.1", dhcp.Options["router"]) - } - - // Verify router has 1 port (subnet port) - router, _ = mock.GetLogicalRouter(ctx, "vpc-vpc-integ1") - if len(router.Ports) != 1 { - t.Errorf("router ports = %d, want 1", len(router.Ports)) - } - - // === Phase 3: Create ENI port === - portEvt := PortEvent{ - NetworkInterfaceId: "eni-integ1", - SubnetId: "subnet-integ1", - VpcId: "vpc-integ1", - PrivateIpAddress: "10.0.1.10", - MacAddress: "02:00:00:aa:bb:01", - } - portData, _ := json.Marshal(portEvt) - resp, err = nc.Request(TopicCreatePort, portData, 5*time.Second) - if err != nil { - t.Fatalf("vpc.create-port: %v", err) - } - assertSuccess(t, resp, "create ENI port") - - // Verify OVN: logical switch port with correct addresses and DHCP - lsp, err := mock.GetLogicalSwitchPort(ctx, "port-eni-integ1") - if err != nil { - t.Fatalf("expected logical switch port: %v", err) - } - if lsp.Addresses[0] != "02:00:00:aa:bb:01 10.0.1.10" { - t.Errorf("port addresses = %v, want [02:00:00:aa:bb:01 10.0.1.10]", lsp.Addresses) - } - if lsp.DHCPv4Options == nil || *lsp.DHCPv4Options != dhcp.UUID { - t.Error("port should have DHCPv4Options set to subnet DHCP UUID") - } - - // Verify switch has 2 ports (router port + ENI port) - ls, _ = mock.GetLogicalSwitch(ctx, "subnet-subnet-integ1") - if len(ls.Ports) != 2 { - t.Errorf("switch ports = %d, want 2 (router + ENI)", len(ls.Ports)) - } - - // === Phase 4: Create and Attach Internet Gateway === - // Create VPC KV bucket and register test VPC so IGW ownership checks pass - js, err := nc.JetStream() - if err != nil { - t.Fatalf("JetStream: %v", err) - } - vpcKV, err := js.CreateKeyValue(&nats.KeyValueConfig{Bucket: handlers_ec2_vpc.KVBucketVPCs, History: 1}) - if err != nil { - t.Fatalf("create VPC KV: %v", err) - } - if _, err := vpcKV.Put(utils.AccountKey(testIGWAccountID, "vpc-integ1"), []byte(`{"vpc_id":"vpc-integ1","state":"available"}`)); err != nil { - t.Fatalf("register test VPC: %v", err) - } - - igwSvc, err := handlers_ec2_igw.NewIGWServiceImplWithNATS(nil, nc) - if err != nil { - t.Fatalf("create IGW service: %v", err) - } - - igwOut, err := igwSvc.CreateInternetGateway(&ec2.CreateInternetGatewayInput{ - TagSpecifications: []*ec2.TagSpecification{ - { - ResourceType: aws.String("internet-gateway"), - Tags: []*ec2.Tag{ - {Key: aws.String("Name"), Value: aws.String("integ-igw")}, - }, - }, - }, - }, testIGWAccountID) - if err != nil { - t.Fatalf("CreateInternetGateway: %v", err) - } - igwID := *igwOut.InternetGateway.InternetGatewayId - - // Attach IGW to VPC - _, err = igwSvc.AttachInternetGateway(&ec2.AttachInternetGatewayInput{ - InternetGatewayId: aws.String(igwID), - VpcId: aws.String("vpc-integ1"), - }, testIGWAccountID) - if err != nil { - t.Fatalf("AttachInternetGateway: %v", err) - } - - // Wait briefly for NATS event to be processed - time.Sleep(100 * time.Millisecond) - - // Verify OVN: external switch, SNAT, default route - _, err = mock.GetLogicalSwitch(ctx, "ext-vpc-integ1") - if err != nil { - t.Fatalf("expected external switch after IGW attach: %v", err) - } - - router, _ = mock.GetLogicalRouter(ctx, "vpc-vpc-integ1") - // Router should have 2 ports (subnet + gateway) - if len(router.Ports) != 2 { - t.Errorf("router ports = %d, want 2 (subnet + gateway)", len(router.Ports)) - } - // No blanket SNAT — only per-VM dnat_and_snat rules provide NAT (AWS parity) - if len(router.NAT) != 0 { - t.Errorf("router NAT rules = %d, want 0 (no blanket SNAT)", len(router.NAT)) - } - // Default route should exist - if len(router.StaticRoutes) != 1 { - t.Errorf("router static routes = %d, want 1", len(router.StaticRoutes)) - } - - // Verify IGW state via Describe - descOut, err := igwSvc.DescribeInternetGateways(&ec2.DescribeInternetGatewaysInput{ - InternetGatewayIds: []*string{aws.String(igwID)}, - }, testIGWAccountID) - if err != nil { - t.Fatalf("DescribeInternetGateways: %v", err) - } - if len(descOut.InternetGateways) != 1 { - t.Fatalf("expected 1 IGW, got %d", len(descOut.InternetGateways)) - } - if len(descOut.InternetGateways[0].Attachments) != 1 { - t.Fatal("expected IGW to have 1 attachment") - } - if *descOut.InternetGateways[0].Attachments[0].VpcId != "vpc-integ1" { - t.Errorf("IGW attachment VpcId = %s, want vpc-integ1", *descOut.InternetGateways[0].Attachments[0].VpcId) - } - - // === Phase 5: Verify Full Topology Summary === - switches, _ := mock.ListLogicalSwitches(ctx) - if len(switches) != 2 { - t.Errorf("total switches = %d, want 2 (subnet + external)", len(switches)) - } - routers, _ := mock.ListLogicalRouters(ctx) - if len(routers) != 1 { - t.Errorf("total routers = %d, want 1", len(routers)) - } - dhcpList, _ := mock.ListDHCPOptions(ctx) - if len(dhcpList) != 1 { - t.Errorf("total DHCP options = %d, want 1", len(dhcpList)) - } - - // === Phase 6: Detach IGW === - _, err = igwSvc.DetachInternetGateway(&ec2.DetachInternetGatewayInput{ - InternetGatewayId: aws.String(igwID), - VpcId: aws.String("vpc-integ1"), - }, testIGWAccountID) - if err != nil { - t.Fatalf("DetachInternetGateway: %v", err) - } - time.Sleep(100 * time.Millisecond) - - // External switch should be cleaned up - _, err = mock.GetLogicalSwitch(ctx, "ext-vpc-integ1") - if err == nil { - t.Error("expected external switch to be deleted after IGW detach") - } - - // Router should have 1 port (subnet only), no NAT, no routes - router, _ = mock.GetLogicalRouter(ctx, "vpc-vpc-integ1") - if len(router.Ports) != 1 { - t.Errorf("router ports after detach = %d, want 1", len(router.Ports)) - } - if len(router.NAT) != 0 { - t.Errorf("router NAT after detach = %d, want 0", len(router.NAT)) - } - if len(router.StaticRoutes) != 0 { - t.Errorf("router routes after detach = %d, want 0", len(router.StaticRoutes)) - } - - // === Phase 7: Delete IGW === - _, err = igwSvc.DeleteInternetGateway(&ec2.DeleteInternetGatewayInput{ - InternetGatewayId: aws.String(igwID), - }, testIGWAccountID) - if err != nil { - t.Fatalf("DeleteInternetGateway: %v", err) - } - - // === Phase 8: Clean up — delete ENI, subnet, VPC === - resp, _ = nc.Request(TopicDeletePort, portData, 5*time.Second) - assertSuccess(t, resp, "delete ENI port") - - resp, _ = nc.Request(TopicSubnetDelete, subData, 5*time.Second) - assertSuccess(t, resp, "delete subnet") - - resp, _ = nc.Request(TopicVPCDelete, vpcData, 5*time.Second) - assertSuccess(t, resp, "delete VPC") - - // === Phase 9: Verify Complete Cleanup === - switches, _ = mock.ListLogicalSwitches(ctx) - if len(switches) != 0 { - t.Errorf("expected 0 switches after cleanup, got %d", len(switches)) - } - routers, _ = mock.ListLogicalRouters(ctx) - if len(routers) != 0 { - t.Errorf("expected 0 routers after cleanup, got %d", len(routers)) - } - dhcpList, _ = mock.ListDHCPOptions(ctx) - if len(dhcpList) != 0 { - t.Errorf("expected 0 DHCP options after cleanup, got %d", len(dhcpList)) - } - - descOut, err = igwSvc.DescribeInternetGateways(&ec2.DescribeInternetGatewaysInput{}, testIGWAccountID) - if err != nil { - t.Fatalf("DescribeInternetGateways: %v", err) - } - if len(descOut.InternetGateways) != 0 { - t.Errorf("expected 0 IGWs after cleanup, got %d", len(descOut.InternetGateways)) - } -} - -// TestIntegration_MultiSubnetWithIGW tests a VPC with multiple subnets and IGW. -func TestIntegration_MultiSubnetWithIGW(t *testing.T) { - _, nc := startTestJetStreamNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Create VPC - vpcEvt := VPCEvent{VpcId: "vpc-multi", CidrBlock: "10.0.0.0/16", VNI: 2000} - vpcData, _ := json.Marshal(vpcEvt) - resp, _ := nc.Request(TopicVPCCreate, vpcData, 5*time.Second) - assertSuccess(t, resp, "create VPC") - - // Create 3 subnets in the same VPC - subnets := []SubnetEvent{ - {SubnetId: "subnet-a", VpcId: "vpc-multi", CidrBlock: "10.0.1.0/24"}, - {SubnetId: "subnet-b", VpcId: "vpc-multi", CidrBlock: "10.0.2.0/24"}, - {SubnetId: "subnet-c", VpcId: "vpc-multi", CidrBlock: "10.0.3.0/24"}, - } - for _, sub := range subnets { - data, _ := json.Marshal(sub) - resp, _ = nc.Request(TopicSubnetCreate, data, 5*time.Second) - assertSuccess(t, resp, "create subnet "+sub.SubnetId) - } - - // Create ENI in each subnet - ports := []PortEvent{ - {NetworkInterfaceId: "eni-a1", SubnetId: "subnet-a", VpcId: "vpc-multi", PrivateIpAddress: "10.0.1.10", MacAddress: "02:00:00:01:01:01"}, - {NetworkInterfaceId: "eni-b1", SubnetId: "subnet-b", VpcId: "vpc-multi", PrivateIpAddress: "10.0.2.10", MacAddress: "02:00:00:02:02:02"}, - {NetworkInterfaceId: "eni-c1", SubnetId: "subnet-c", VpcId: "vpc-multi", PrivateIpAddress: "10.0.3.10", MacAddress: "02:00:00:03:03:03"}, - } - for _, port := range ports { - data, _ := json.Marshal(port) - resp, _ = nc.Request(TopicCreatePort, data, 5*time.Second) - assertSuccess(t, resp, "create port "+port.NetworkInterfaceId) - } - - // Verify: 1 router, 3 switches, 3 DHCP options - routerList, _ := mock.ListLogicalRouters(ctx) - if len(routerList) != 1 { - t.Errorf("routers = %d, want 1", len(routerList)) - } - switchList, _ := mock.ListLogicalSwitches(ctx) - if len(switchList) != 3 { - t.Errorf("switches = %d, want 3", len(switchList)) - } - dhcpList, _ := mock.ListDHCPOptions(ctx) - if len(dhcpList) != 3 { - t.Errorf("DHCP options = %d, want 3", len(dhcpList)) - } - - // Router should have 3 ports (one per subnet) - router, _ := mock.GetLogicalRouter(ctx, "vpc-vpc-multi") - if len(router.Ports) != 3 { - t.Errorf("router ports = %d, want 3", len(router.Ports)) - } - - // Create VPC KV bucket and register test VPC so IGW ownership checks pass - js, err := nc.JetStream() - if err != nil { - t.Fatalf("JetStream: %v", err) - } - vpcKV, err := js.CreateKeyValue(&nats.KeyValueConfig{Bucket: handlers_ec2_vpc.KVBucketVPCs, History: 1}) - if err != nil { - t.Fatalf("create VPC KV: %v", err) - } - if _, err := vpcKV.Put(utils.AccountKey(testIGWAccountID, "vpc-multi"), []byte(`{"vpc_id":"vpc-multi","state":"available"}`)); err != nil { - t.Fatalf("register test VPC: %v", err) - } - - // Attach IGW - igwSvc, _ := handlers_ec2_igw.NewIGWServiceImplWithNATS(nil, nc) - igwOut, _ := igwSvc.CreateInternetGateway(&ec2.CreateInternetGatewayInput{}, testIGWAccountID) - igwID := *igwOut.InternetGateway.InternetGatewayId - - _, err = igwSvc.AttachInternetGateway(&ec2.AttachInternetGatewayInput{ - InternetGatewayId: aws.String(igwID), - VpcId: aws.String("vpc-multi"), - }, testIGWAccountID) - if err != nil { - t.Fatalf("AttachInternetGateway: %v", err) - } - time.Sleep(100 * time.Millisecond) - - // Now: 4 switches (3 subnets + 1 external), router has 4 ports + NAT + route - switchList, _ = mock.ListLogicalSwitches(ctx) - if len(switchList) != 4 { - t.Errorf("switches after IGW = %d, want 4", len(switchList)) - } - router, _ = mock.GetLogicalRouter(ctx, "vpc-vpc-multi") - if len(router.Ports) != 4 { - t.Errorf("router ports after IGW = %d, want 4", len(router.Ports)) - } - if len(router.NAT) != 0 { - t.Errorf("router NAT = %d, want 0 (no blanket SNAT)", len(router.NAT)) - } - if len(router.StaticRoutes) != 1 { - t.Errorf("router routes = %d, want 1", len(router.StaticRoutes)) - } - - // === Cleanup: detach IGW, delete everything === - _, _ = igwSvc.DetachInternetGateway(&ec2.DetachInternetGatewayInput{ - InternetGatewayId: aws.String(igwID), - VpcId: aws.String("vpc-multi"), - }, testIGWAccountID) - time.Sleep(100 * time.Millisecond) - _, _ = igwSvc.DeleteInternetGateway(&ec2.DeleteInternetGatewayInput{ - InternetGatewayId: aws.String(igwID), - }, testIGWAccountID) - - // Delete ports - for _, port := range ports { - data, _ := json.Marshal(port) - resp, _ = nc.Request(TopicDeletePort, data, 5*time.Second) - assertSuccess(t, resp, "delete port "+port.NetworkInterfaceId) - } - - // Delete subnets - for _, sub := range subnets { - data, _ := json.Marshal(sub) - resp, _ = nc.Request(TopicSubnetDelete, data, 5*time.Second) - assertSuccess(t, resp, "delete subnet "+sub.SubnetId) - } - - // Delete VPC - resp, _ = nc.Request(TopicVPCDelete, vpcData, 5*time.Second) - assertSuccess(t, resp, "delete VPC") - - // Verify complete cleanup - routerList, _ = mock.ListLogicalRouters(ctx) - if len(routerList) != 0 { - t.Errorf("routers after cleanup = %d, want 0", len(routerList)) - } - switchList, _ = mock.ListLogicalSwitches(ctx) - if len(switchList) != 0 { - t.Errorf("switches after cleanup = %d, want 0", len(switchList)) - } -} - -// TestIntegration_IGWErrorPaths tests IGW error handling across the integration boundary. -func TestIntegration_IGWErrorPaths(t *testing.T) { - _, nc := startTestJetStreamNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Create VPC KV bucket and register test VPCs so IGW ownership checks pass - js, err := nc.JetStream() - if err != nil { - t.Fatalf("JetStream: %v", err) - } - vpcKV, err := js.CreateKeyValue(&nats.KeyValueConfig{Bucket: handlers_ec2_vpc.KVBucketVPCs, History: 1}) - if err != nil { - t.Fatalf("create VPC KV: %v", err) - } - for _, vpcID := range []string{"vpc-err1", "vpc-err2"} { - if _, err := vpcKV.Put(utils.AccountKey(testIGWAccountID, vpcID), []byte(`{"vpc_id":"`+vpcID+`","state":"available"}`)); err != nil { - t.Fatalf("register test VPC: %v", err) - } - } - - igwSvc, err := handlers_ec2_igw.NewIGWServiceImplWithNATS(nil, nc) - if err != nil { - t.Fatalf("create IGW service: %v", err) - } - - // Create and attach IGW - igwOut, _ := igwSvc.CreateInternetGateway(&ec2.CreateInternetGatewayInput{}, testIGWAccountID) - igwID := *igwOut.InternetGateway.InternetGatewayId - - // Can't delete while attached - _, _ = igwSvc.AttachInternetGateway(&ec2.AttachInternetGatewayInput{ - InternetGatewayId: aws.String(igwID), - VpcId: aws.String("vpc-err1"), - }, testIGWAccountID) - - _, err = igwSvc.DeleteInternetGateway(&ec2.DeleteInternetGatewayInput{ - InternetGatewayId: aws.String(igwID), - }, testIGWAccountID) - if err == nil { - t.Error("expected DependencyViolation error") - } - - // Can't attach twice - _, err = igwSvc.AttachInternetGateway(&ec2.AttachInternetGatewayInput{ - InternetGatewayId: aws.String(igwID), - VpcId: aws.String("vpc-err2"), - }, testIGWAccountID) - if err == nil { - t.Error("expected ResourceAlreadyAssociated error") - } - - // Can't detach from wrong VPC - _, err = igwSvc.DetachInternetGateway(&ec2.DetachInternetGatewayInput{ - InternetGatewayId: aws.String(igwID), - VpcId: aws.String("vpc-wrong"), - }, testIGWAccountID) - if err == nil { - t.Error("expected GatewayNotAttached error") - } - - // Operations on nonexistent IGW - _, err = igwSvc.AttachInternetGateway(&ec2.AttachInternetGatewayInput{ - InternetGatewayId: aws.String("igw-doesnotexist"), - VpcId: aws.String("vpc-err1"), - }, testIGWAccountID) - if err == nil { - t.Error("expected NotFound error for attach") - } - - _, err = igwSvc.DetachInternetGateway(&ec2.DetachInternetGatewayInput{ - InternetGatewayId: aws.String("igw-doesnotexist"), - VpcId: aws.String("vpc-err1"), - }, testIGWAccountID) - if err == nil { - t.Error("expected NotFound error for detach") - } - - // Cleanup - _, _ = igwSvc.DetachInternetGateway(&ec2.DetachInternetGatewayInput{ - InternetGatewayId: aws.String(igwID), - VpcId: aws.String("vpc-err1"), - }, testIGWAccountID) - _, _ = igwSvc.DeleteInternetGateway(&ec2.DeleteInternetGatewayInput{ - InternetGatewayId: aws.String(igwID), - }, testIGWAccountID) -} - -// TestIntegration_SubnetBeforeVPC asserts that vpc.create-subnet does not -// fail when it arrives before the matching vpc.create-vpc (mulga-siv-133). -// The subnet handler must idempotently EnsureVPC so the parent logical -// router is in place by the time CreateLogicalRouterPort runs. -func TestIntegration_SubnetBeforeVPC(t *testing.T) { - _, nc := startTestJetStreamNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Subnet event arrives first — no prior vpc.create-vpc. - subEvt := SubnetEvent{SubnetId: "subnet-race", VpcId: "vpc-race", CidrBlock: "10.0.1.0/24"} - subData, _ := json.Marshal(subEvt) - resp, err := nc.Request(TopicSubnetCreate, subData, 5*time.Second) - if err != nil { - t.Fatalf("vpc.create-subnet (race): %v", err) - } - assertSuccess(t, resp, "subnet-before-vpc create") - - // Router was created defensively. - router, err := mock.GetLogicalRouter(ctx, "vpc-vpc-race") - if err != nil { - t.Fatalf("expected VPC router after subnet pre-create: %v", err) - } - // Subnet topology landed. - if _, err := mock.GetLogicalSwitch(ctx, "subnet-subnet-race"); err != nil { - t.Fatalf("expected subnet switch: %v", err) - } - if len(router.Ports) != 1 { - t.Errorf("router ports = %d, want 1", len(router.Ports)) - } - - // Now publish the (delayed) vpc.create-vpc with full metadata. EnsureVPC - // must short-circuit on the existing LR — no duplicate, no error. - vpcEvt := VPCEvent{VpcId: "vpc-race", CidrBlock: "10.0.0.0/16", VNI: 4242} - vpcData, _ := json.Marshal(vpcEvt) - resp, err = nc.Request(TopicVPCCreate, vpcData, 5*time.Second) - if err != nil { - t.Fatalf("vpc.create (late): %v", err) - } - assertSuccess(t, resp, "late VPC create") - - routerList, _ := mock.ListLogicalRouters(ctx) - if len(routerList) != 1 { - t.Errorf("routers = %d, want 1 (duplicate after race-loser create)", len(routerList)) - } -} diff --git a/spinifex/services/vpcd/security.go b/spinifex/services/vpcd/security.go deleted file mode 100644 index 72903a608..000000000 --- a/spinifex/services/vpcd/security.go +++ /dev/null @@ -1,143 +0,0 @@ -package vpcd - -// Phase 2.6 (mulga-siv-129) thin NATS wrappers for SG lifecycle. Each handler -// deserialises the event, translates to the policy/topology layer's spec -// types, and calls the manager. ACL semantics, OVN port-group lifecycle and -// the full all-or-nothing convergence model live in network/topology and -// network/policy; this file owns NATS plumbing only. - -import ( - "context" - "encoding/json" - "fmt" - "log/slog" - - "github.com/mulgadc/spinifex/spinifex/network/policy" - "github.com/nats-io/nats.go" -) - -// sgRule mirrors the on-wire payload from handlers/ec2/vpc.SGRule. Kept local -// so vpcd does not import the handlers package. -type sgRule struct { - IpProtocol string `json:"ip_protocol"` - FromPort int64 `json:"from_port"` - ToPort int64 `json:"to_port"` - CidrIp string `json:"cidr_ip,omitempty"` - SourceSG string `json:"source_sg,omitempty"` -} - -// sgEvent mirrors handlers/ec2/vpc.SGEvent. -type sgEvent struct { - GroupId string `json:"group_id"` - VpcId string `json:"vpc_id"` - IngressRules []sgRule `json:"ingress_rules,omitempty"` - EgressRules []sgRule `json:"egress_rules,omitempty"` -} - -func (e sgEvent) toSpec() policy.SGSpec { - return policy.SGSpec{ - GroupID: e.GroupId, - VPCID: e.VpcId, - IngressRules: toPolicyRules(e.IngressRules), - EgressRules: toPolicyRules(e.EgressRules), - } -} - -func toPolicyRules(in []sgRule) []policy.Rule { - out := make([]policy.Rule, len(in)) - for i, r := range in { - out[i] = policy.Rule{ - IPProtocol: r.IpProtocol, - FromPort: r.FromPort, - ToPort: r.ToPort, - CIDR: r.CidrIp, - SourceSG: r.SourceSG, - } - } - return out -} - -// handleCreateSG ensures the OVN port group exists then applies the ACL set -// for a new security group. Two-step (topology + policy) because port-group -// lifecycle is L2 and ACL programming is L3 — see plan §8.1. -func (h *TopologyHandler) handleCreateSG(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt sgEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.create-sg event", "err", err) - respond(msg, err) - return - } - ctx := context.Background() - if err := h.EnsureSGPortGroup(ctx, evt.GroupId); err != nil { - slog.Error("vpcd: EnsureSGPortGroup failed", "group_id", evt.GroupId, "err", err) - respond(msg, err) - return - } - if err := h.securityGroupManager().EnsureSG(ctx, evt.toSpec()); err != nil { - slog.Error("vpcd: EnsureSG failed", "group_id", evt.GroupId, "err", err) - respond(msg, err) - return - } - slog.Info("vpcd: created security group", - "group_id", evt.GroupId, - "vpc_id", evt.VpcId, - "ingress_rules", len(evt.IngressRules), - "egress_rules", len(evt.EgressRules), - ) - respond(msg, nil) -} - -// handleDeleteSG removes the SG's port group (and its ACLs in one OVN -// transaction). Idempotent on already-absent. -func (h *TopologyHandler) handleDeleteSG(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt sgEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.delete-sg event", "err", err) - respond(msg, err) - return - } - ctx := context.Background() - if err := h.DeleteSGPortGroup(ctx, evt.GroupId); err != nil { - slog.Error("vpcd: DeleteSGPortGroup failed", "group_id", evt.GroupId, "err", err) - respond(msg, err) - return - } - slog.Info("vpcd: deleted security group", "group_id", evt.GroupId, "vpc_id", evt.VpcId) - respond(msg, nil) -} - -// handleUpdateSG replaces the SG's ACL set with the rules from the event. -// Port-group lifecycle is unaffected — UpdateSG only touches ACLs. -func (h *TopologyHandler) handleUpdateSG(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt sgEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.update-sg event", "err", err) - respond(msg, err) - return - } - ctx := context.Background() - if err := h.securityGroupManager().UpdateSG(ctx, evt.toSpec()); err != nil { - slog.Error("vpcd: UpdateSG failed", "group_id", evt.GroupId, "err", err) - respond(msg, err) - return - } - slog.Info("vpcd: updated security group", - "group_id", evt.GroupId, - "vpc_id", evt.VpcId, - "ingress_rules", len(evt.IngressRules), - "egress_rules", len(evt.EgressRules), - ) - respond(msg, nil) -} diff --git a/spinifex/services/vpcd/security_test.go b/spinifex/services/vpcd/security_test.go deleted file mode 100644 index 142713769..000000000 --- a/spinifex/services/vpcd/security_test.go +++ /dev/null @@ -1,261 +0,0 @@ -package vpcd - -// Phase 2.6 (mulga-siv-129) tests for the SG NATS wrappers in security.go. -// ACL emission semantics live in network/policy/sg_test.go + -// network/policy/acl_test.go; tests here cover the wrapper layer: -// event-shape errors, OVN-client wiring, and end-to-end NATS → OVN happy -// paths through the new managers. - -import ( - "context" - "encoding/json" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func sgTopoFixture(t *testing.T) (*TopologyHandler, *MockOVNClient) { - t.Helper() - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - return NewTopologyHandler(mock), mock -} - -func TestSG_CreateSG_HappyPath(t *testing.T) { - _, nc := startTestNATS(t) - topo, mock := sgTopoFixture(t) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - evt := sgEvent{GroupId: "sg-create1", VpcId: "vpc-test"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicCreateSG, data, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "create SG") - - mock.Mu.Lock() - _, pgExists := mock.PortGroups["sg_create1"] - aclCount := len(mock.ACLs) - mock.Mu.Unlock() - assert.True(t, pgExists, "port group sg_create1 must exist after create-sg") - assert.Equal(t, 4, aclCount, "infra ACL set (2 deny + 2 DHCP) must be attached") -} - -func TestSG_CreateSG_Idempotent(t *testing.T) { - _, nc := startTestNATS(t) - topo, mock := sgTopoFixture(t) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - data, _ := json.Marshal(sgEvent{GroupId: "sg-idem", VpcId: "vpc-test"}) - for range 2 { - resp, err := nc.Request(TopicCreateSG, data, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "create SG retry") - } - - mock.Mu.Lock() - pgCount := len(mock.PortGroups) - mock.Mu.Unlock() - assert.Equal(t, 1, pgCount, "duplicate create-sg must not create a second port group") -} - -func TestSG_DeleteSG_HappyPath(t *testing.T) { - _, nc := startTestNATS(t) - topo, mock := sgTopoFixture(t) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - data, _ := json.Marshal(sgEvent{GroupId: "sg-del1", VpcId: "vpc-test"}) - resp, err := nc.Request(TopicCreateSG, data, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "create SG before delete") - - resp, err = nc.Request(TopicDeleteSG, data, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "delete SG") - - mock.Mu.Lock() - _, pgExists := mock.PortGroups["sg_del1"] - mock.Mu.Unlock() - assert.False(t, pgExists, "port group must be gone after delete-sg") -} - -// Delete of a never-created SG must succeed: the new wrapper delegates to -// topology.Manager.DeleteSGPortGroup which is idempotent on absent port -// groups (the reconciler retries delete events that race with vm-cleanup). -func TestSG_DeleteSG_IdempotentOnAbsent(t *testing.T) { - _, nc := startTestNATS(t) - topo, _ := sgTopoFixture(t) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - data, _ := json.Marshal(sgEvent{GroupId: "sg-ghost", VpcId: "vpc-test"}) - resp, err := nc.Request(TopicDeleteSG, data, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "delete absent SG must be a no-op") -} - -func TestSG_UpdateSG_ReplacesACLSet(t *testing.T) { - _, nc := startTestNATS(t) - topo, mock := sgTopoFixture(t) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - createData, _ := json.Marshal(sgEvent{GroupId: "sg-upd", VpcId: "vpc-test"}) - resp, err := nc.Request(TopicCreateSG, createData, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "create SG before update") - - updateData, _ := json.Marshal(sgEvent{ - GroupId: "sg-upd", - VpcId: "vpc-test", - IngressRules: []sgRule{ - {IpProtocol: "tcp", FromPort: 22, ToPort: 22, CidrIp: "10.0.0.0/8"}, - }, - }) - resp, err = nc.Request(TopicUpdateSG, updateData, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "update SG") - - mock.Mu.Lock() - pg := mock.PortGroups["sg_upd"] - aclCount := 0 - hasTenantRule := false - if pg != nil { - aclCount = len(pg.ACLs) - for _, uuid := range pg.ACLs { - if acl := mock.ACLs[uuid]; acl != nil && acl.Priority == 1000 { - hasTenantRule = true - } - } - } - mock.Mu.Unlock() - assert.Equal(t, 5, aclCount, "update must leave 4 infra + 1 tenant ACL on the port group") - assert.True(t, hasTenantRule, "tenant priority-1000 ACL must be present after update") -} - -func TestSG_HandleCreateSG_OVNNotConnected(t *testing.T) { - _, nc := startTestNATS(t) - topo := NewTopologyHandler(nil) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - data, _ := json.Marshal(sgEvent{GroupId: "sg-noovn"}) - resp, err := nc.Request(TopicCreateSG, data, 5_000_000_000) - require.NoError(t, err) - assertFailure(t, resp, "create SG must fail when ovn client is nil") -} - -func TestSG_HandleDeleteSG_OVNNotConnected(t *testing.T) { - _, nc := startTestNATS(t) - topo := NewTopologyHandler(nil) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - data, _ := json.Marshal(sgEvent{GroupId: "sg-noovn"}) - resp, err := nc.Request(TopicDeleteSG, data, 5_000_000_000) - require.NoError(t, err) - assertFailure(t, resp, "delete SG must fail when ovn client is nil") -} - -func TestSG_HandleUpdateSG_OVNNotConnected(t *testing.T) { - _, nc := startTestNATS(t) - topo := NewTopologyHandler(nil) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - data, _ := json.Marshal(sgEvent{GroupId: "sg-noovn"}) - resp, err := nc.Request(TopicUpdateSG, data, 5_000_000_000) - require.NoError(t, err) - assertFailure(t, resp, "update SG must fail when ovn client is nil") -} - -func TestSG_HandleCreateSG_BadJSON(t *testing.T) { - _, nc := startTestNATS(t) - topo, _ := sgTopoFixture(t) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - resp, err := nc.Request(TopicCreateSG, []byte("not json"), 5_000_000_000) - require.NoError(t, err) - assertFailure(t, resp, "create SG with malformed payload must fail") -} - -func TestSG_HandleDeleteSG_BadJSON(t *testing.T) { - _, nc := startTestNATS(t) - topo, _ := sgTopoFixture(t) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - resp, err := nc.Request(TopicDeleteSG, []byte("not json"), 5_000_000_000) - require.NoError(t, err) - assertFailure(t, resp, "delete SG with malformed payload must fail") -} - -func TestSG_HandleUpdateSG_BadJSON(t *testing.T) { - _, nc := startTestNATS(t) - topo, _ := sgTopoFixture(t) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - resp, err := nc.Request(TopicUpdateSG, []byte("not json"), 5_000_000_000) - require.NoError(t, err) - assertFailure(t, resp, "update SG with malformed payload must fail") -} diff --git a/spinifex/services/vpcd/topology.go b/spinifex/services/vpcd/topology.go deleted file mode 100644 index 721cc5151..000000000 --- a/spinifex/services/vpcd/topology.go +++ /dev/null @@ -1,719 +0,0 @@ -package vpcd - -import ( - "context" - "encoding/json" - "fmt" - "log/slog" - "net" - "net/netip" - "strings" - "sync" - "time" - - "github.com/mulgadc/spinifex/spinifex/network/external" - "github.com/mulgadc/spinifex/spinifex/network/policy" - "github.com/mulgadc/spinifex/spinifex/network/topology" - "github.com/mulgadc/spinifex/spinifex/types" - "github.com/nats-io/nats.go" -) - -// waitForFlowsHV shells out to `ovn-nbctl --wait=hv sync`, which bumps -// NB_Global.nb_cfg and blocks until every connected chassis has acknowledged -// the new sequence number — i.e. ovn-northd has compiled NB -> SB and -// ovn-controller has installed the resulting flows. Used after IGW attach -// so newly-launched VMs aren't unreachable while their gateway chassis is -// still catching up (mulga-siv-105). -// -// Bounded by ovn-nbctl --timeout=30 (seconds). On overrun we log a WARN -// and return nil — the caller continues. In practice flows converge within -// seconds; a 30s overrun means something OVN-side is wedged enough that -// failing VPC create wouldn't improve things. -// -// Declared as a var so tests can stub it. -var waitForFlowsHV = func() error { - start := time.Now() - cmd := sudoCommand("ovn-nbctl", - "--no-leader-only", - "--timeout=30", - "--wait=hv", - "sync", - ) - if out, err := cmd.CombinedOutput(); err != nil { - slog.Warn("vpcd: OVN flows-ready barrier overran; continuing without confirmation", - "elapsed", time.Since(start), - "err", err, - "output", strings.TrimSpace(string(out)), - ) - return nil - } - slog.Debug("vpcd: OVN flows-ready barrier complete", "elapsed", time.Since(start)) - return nil -} - -// NATS topics for VPC lifecycle events published by the daemon. -const ( - TopicVPCCreate = "vpc.create" - TopicVPCDelete = "vpc.delete" - TopicSubnetCreate = "vpc.create-subnet" - TopicSubnetDelete = "vpc.delete-subnet" - TopicCreatePort = "vpc.create-port" - TopicDeletePort = "vpc.delete-port" - TopicUpdatePortSGs = "vpc.update-port-sgs" - TopicPortStatus = "vpc.port-status" - TopicIGWAttach = "vpc.igw-attach" - TopicIGWDetach = "vpc.igw-detach" - TopicAddNAT = "vpc.add-nat" - TopicDeleteNAT = "vpc.delete-nat" - TopicAddNATGateway = "vpc.add-nat-gateway" - TopicDeleteNATGateway = "vpc.delete-nat-gateway" - TopicCreateSG = "vpc.create-sg" - TopicDeleteSG = "vpc.delete-sg" - TopicUpdateSG = "vpc.update-sg" -) - -// VPCEvent is published on vpc.create after a VPC is persisted. -type VPCEvent struct { - VpcId string `json:"vpc_id"` - CidrBlock string `json:"cidr_block"` - VNI int64 `json:"vni"` -} - -// SubnetEvent is published on vpc.create-subnet / vpc.delete-subnet. -type SubnetEvent struct { - SubnetId string `json:"subnet_id"` - VpcId string `json:"vpc_id"` - CidrBlock string `json:"cidr_block"` -} - -// PortEvent is published on vpc.create-port / vpc.delete-port. -// -// SecurityGroupIds carries the SG membership the port should join at create -// time so vpcd can wire OVN port-group membership atomically with the LSP -// create. Empty on delete-port (handleDeletePort discovers current -// memberships from the libovsdb cache). -type PortEvent struct { - NetworkInterfaceId string `json:"network_interface_id"` - SubnetId string `json:"subnet_id"` - VpcId string `json:"vpc_id"` - PrivateIpAddress string `json:"private_ip_address"` - MacAddress string `json:"mac_address"` - SecurityGroupIds []string `json:"security_group_ids,omitempty"` -} - -// UpdatePortSGsEvent is published on vpc.update-port-sgs after -// ModifyNetworkInterfaceAttribute changes an ENI's SG membership. The payload -// is declarative — vpcd reads its libovsdb cache to discover current -// memberships and computes the diff against SecurityGroupIds. -type UpdatePortSGsEvent struct { - NetworkInterfaceId string `json:"network_interface_id"` - PrivateIpAddress string `json:"private_ip_address"` - SecurityGroupIds []string `json:"security_group_ids"` -} - -// NATEvent is published on vpc.add-nat / vpc.delete-nat for 1:1 public IP NAT. -type NATEvent struct { - VpcId string `json:"vpc_id"` - ExternalIP string `json:"external_ip"` - LogicalIP string `json:"logical_ip"` - PortName string `json:"port_name"` // logical port for distributed NAT - MAC string `json:"mac"` // external MAC for distributed NAT -} - -// NATGatewayEvent is published on vpc.add-nat-gateway / vpc.delete-nat-gateway. -type NATGatewayEvent struct { - VpcId string `json:"vpc_id"` - NatGatewayId string `json:"nat_gateway_id"` - PublicIp string `json:"public_ip"` - SubnetCidr string `json:"subnet_cidr"` // private subnet CIDR for SNAT rule -} - -// Bridge mode constants for external connectivity. -const ( - // BridgeModeDirect adds the WAN NIC directly to br-external as an OVS port. - // Enables distributed NAT. Only safe when the WAN NIC is NOT the SSH/ - // management NIC. - BridgeModeDirect = "direct" - // BridgeModeVeth uses a veth pair to link a Linux bridge (br-wan) to an - // OVS bridge (br-ext). Requires centralized NAT because the Linux bridge - // intermediary breaks distributed NAT hairpin routing. - BridgeModeVeth = "veth" - // OvnExternalBridge is the OVS bridge that ovn-bridge-mappings targets - // for the "external" localnet. Owned by setup-ovn.sh's ovn-bridge-mappings - // setup and independent of the WAN bridge (which is Linux-side in veth - // mode). Named as a constant so the vpcd sanity check refers to the - // contract, not a hardcoded string (D18). - OvnExternalBridge = "br-ext" -) - -// TopologyHandler translates VPC lifecycle NATS events into OVN NB DB operations. -type TopologyHandler struct { - ovn OVNClient - externalMode string - externalPools []ExternalPoolConfig - chassisNames []string // OVN chassis names for gateway HA scheduling - natMode policy.NATMode // distributed (direct uplink) or centralized (veth uplink) - - // lm + lmOnce cache the topology.Manager produced from h.ovn so each - // adapter call doesn't re-construct (Phase 2.6, mulga-siv-129). - lmOnce sync.Once - lm topology.Manager - - // sgm + sgmOnce cache the policy.SecurityGroupManager backed by h.ovn so - // SG subscribers reuse one manager instance across events. - sgmOnce sync.Once - sgm policy.SecurityGroupManager - - // natm + natmOnce cache the policy.NATManager backed by h.ovn. NAT mode is - // the value injected via WithNATMode; the FlowsBarrier closure shells out - // to ovn-nbctl --wait=hv sync via waitForFlowsHV. - natmOnce sync.Once - natm policy.NATManager - natmErr error - - // igwm + igwmOnce cache the external.IGWManager. All IGW attach/detach - // paths route through it. - igwmOnce sync.Once - igwm external.IGWManager - igwmErr error -} - -// NewTopologyHandler creates a new TopologyHandler with optional external network config. -// NAT mode defaults to Distributed (direct uplink) when WithNATMode is omitted; -// matches the pre-Slice-B behaviour where unset bridgeMode also implied -// distributed via useCentralizedNAT(). -func NewTopologyHandler(ovn OVNClient, opts ...TopologyOption) *TopologyHandler { - h := &TopologyHandler{ovn: ovn, natMode: policy.NATModeDistributed} - for _, opt := range opts { - opt(h) - } - return h -} - -// TopologyOption configures a TopologyHandler. -type TopologyOption func(*TopologyHandler) - -// WithExternalNetwork configures external connectivity for public subnets. -func WithExternalNetwork(mode string, pools []ExternalPoolConfig) TopologyOption { - return func(h *TopologyHandler) { - h.externalMode = mode - h.externalPools = pools - } -} - -// WithChassisNames sets the OVN chassis names for gateway HA scheduling. -func WithChassisNames(names []string) TopologyOption { - return func(h *TopologyHandler) { - h.chassisNames = names - } -} - -// WithNATMode sets the NAT mode used by lazily-constructed managers. Derived -// upstream from the uplink/bridge mode (direct→distributed, veth→centralized). -func WithNATMode(mode policy.NATMode) TopologyOption { - return func(h *TopologyHandler) { - h.natMode = mode - } -} - -// findExternalPool returns the first pool matching the given region/AZ, -// using the fallback order: AZ-scoped → region-scoped → unscoped. -func (h *TopologyHandler) findExternalPool(region, az string) *ExternalPoolConfig { - // 1. AZ-scoped match - for i := range h.externalPools { - p := &h.externalPools[i] - if p.AZ != "" && p.AZ == az && p.Region == region { - return p - } - } - // 2. Region-scoped (no AZ) - for i := range h.externalPools { - p := &h.externalPools[i] - if p.AZ == "" && p.Region != "" && p.Region == region { - return p - } - } - // 3. Unscoped (no region, no AZ — global pool) - for i := range h.externalPools { - p := &h.externalPools[i] - if p.Region == "" && p.AZ == "" { - return p - } - } - return nil -} - -// Subscribe registers NATS subscriptions for VPC lifecycle topics. -// Global topology events (VPC/subnet/IGW) use a queue group so exactly one vpcd -// instance handles each event — all instances connect to the same OVN NB DB, so -// any one can process these. Per-node port events use regular subscriptions so -// all instances see them (future: route to the specific node). -func (h *TopologyHandler) Subscribe(nc *nats.Conn) ([]*nats.Subscription, error) { - type sub struct { - topic string - handler nats.MsgHandler - queue bool // true = use queue group (one handler), false = fan-out (all handlers) - } - - subs := []sub{ - {TopicVPCCreate, h.handleVPCCreate, true}, - {TopicVPCDelete, h.handleVPCDelete, true}, - {TopicSubnetCreate, h.handleSubnetCreate, true}, - {TopicSubnetDelete, h.handleSubnetDelete, true}, - {TopicCreatePort, h.handleCreatePort, true}, - {TopicDeletePort, h.handleDeletePort, true}, - {TopicUpdatePortSGs, h.handleUpdatePortSGs, true}, - {TopicIGWAttach, h.handleIGWAttach, true}, - {TopicIGWDetach, h.handleIGWDetach, true}, - {TopicAddNAT, h.handleAddNAT, true}, - {TopicDeleteNAT, h.handleDeleteNAT, true}, - {TopicAddNATGateway, h.handleAddNATGateway, true}, - {TopicDeleteNATGateway, h.handleDeleteNATGateway, true}, - {TopicCreateSG, h.handleCreateSG, true}, - {TopicDeleteSG, h.handleDeleteSG, true}, - {TopicUpdateSG, h.handleUpdateSG, true}, - } - - var result []*nats.Subscription - for _, s := range subs { - var natsSub *nats.Subscription - var err error - if s.queue { - natsSub, err = nc.QueueSubscribe(s.topic, "vpcd-workers", s.handler) - } else { - natsSub, err = nc.Subscribe(s.topic, s.handler) - } - if err != nil { - for _, r := range result { - _ = r.Unsubscribe() - } - return nil, fmt.Errorf("subscribe %s: %w", s.topic, err) - } - result = append(result, natsSub) - slog.Info("Subscribed to VPC topic", "topic", s.topic) - } - - return result, nil -} - -// --- VPC (LogicalRouter) --- - -func (h *TopologyHandler) handleVPCCreate(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt VPCEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.create event", "err", err) - respond(msg, err) - return - } - spec := topology.VPCSpec{VPCID: evt.VpcId, VNI: evt.VNI} - if evt.CidrBlock != "" { - cidr, err := netip.ParsePrefix(evt.CidrBlock) - if err != nil { - slog.Error("vpcd: invalid CIDR in vpc.create event", "cidr", evt.CidrBlock, "err", err) - respond(msg, err) - return - } - spec.CIDR = cidr - } - if err := h.EnsureVPC(context.Background(), spec); err != nil { - slog.Error("vpcd: EnsureVPC failed", "vpc_id", evt.VpcId, "err", err) - respond(msg, err) - return - } - respond(msg, nil) -} - -func (h *TopologyHandler) handleVPCDelete(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt VPCEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.delete event", "err", err) - respond(msg, err) - return - } - if err := h.DeleteVPC(context.Background(), evt.VpcId); err != nil { - slog.Error("vpcd: DeleteVPC failed", "vpc_id", evt.VpcId, "err", err) - respond(msg, err) - return - } - respond(msg, nil) -} - -// --- Subnet (LogicalSwitch + RouterPort + DHCP) --- - -func (h *TopologyHandler) handleSubnetCreate(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt SubnetEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.create-subnet event", "err", err) - respond(msg, err) - return - } - cidr, err := netip.ParsePrefix(evt.CidrBlock) - if err != nil { - slog.Error("vpcd: invalid CIDR in vpc.create-subnet event", "cidr", evt.CidrBlock, "err", err) - respond(msg, err) - return - } - ctx := context.Background() - // vpc.create-vpc and vpc.create-subnet are independent NATS events with no - // inter-event ordering guarantee. Tenant bootstrap publishes both within - // <1ms, so EnsureSubnet can race the parent VPC's CreateLogicalRouter and - // fail with "logical router not found". EnsureVPC is idempotent on the - // router name (mulga-siv-133). - if err := h.EnsureVPC(ctx, topology.VPCSpec{VPCID: evt.VpcId}); err != nil { - slog.Error("vpcd: EnsureVPC (subnet pre-create) failed", "vpc_id", evt.VpcId, "err", err) - respond(msg, err) - return - } - if err := h.EnsureSubnet(ctx, topology.SubnetSpec{ - SubnetID: evt.SubnetId, - VPCID: evt.VpcId, - CIDR: cidr, - }); err != nil { - slog.Error("vpcd: EnsureSubnet failed", "subnet_id", evt.SubnetId, "err", err) - respond(msg, err) - return - } - respond(msg, nil) -} - -func (h *TopologyHandler) handleSubnetDelete(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt SubnetEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.delete-subnet event", "err", err) - respond(msg, err) - return - } - spec := topology.SubnetSpec{SubnetID: evt.SubnetId, VPCID: evt.VpcId} - if evt.CidrBlock != "" { - if cidr, perr := netip.ParsePrefix(evt.CidrBlock); perr == nil { - spec.CIDR = cidr - } - } - if err := h.DeleteSubnet(context.Background(), spec); err != nil { - slog.Error("vpcd: DeleteSubnet failed", "subnet_id", evt.SubnetId, "err", err) - respond(msg, err) - return - } - respond(msg, nil) -} - -// --- Port (LogicalSwitchPort for VM/ENI) --- - -func (h *TopologyHandler) handleCreatePort(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt PortEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.create-port event", "err", err) - respond(msg, err) - return - } - ip, err := netip.ParseAddr(evt.PrivateIpAddress) - if err != nil { - slog.Error("vpcd: invalid private IP in vpc.create-port event", "ip", evt.PrivateIpAddress, "err", err) - respond(msg, err) - return - } - mac, err := net.ParseMAC(evt.MacAddress) - if err != nil { - slog.Error("vpcd: invalid MAC in vpc.create-port event", "mac", evt.MacAddress, "err", err) - respond(msg, err) - return - } - if err := h.EnsurePort(context.Background(), topology.PortSpec{ - PortID: evt.NetworkInterfaceId, - SubnetID: evt.SubnetId, - VPCID: evt.VpcId, - PrivateIP: ip, - MAC: mac, - SGIDs: evt.SecurityGroupIds, - }); err != nil { - slog.Error("vpcd: EnsurePort failed", "eni_id", evt.NetworkInterfaceId, "err", err) - respond(msg, err) - return - } - respond(msg, nil) -} - -func (h *TopologyHandler) handleDeletePort(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt PortEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.delete-port event", "err", err) - respond(msg, err) - return - } - if err := h.DeletePort(context.Background(), topology.PortSpec{ - PortID: evt.NetworkInterfaceId, - SubnetID: evt.SubnetId, - VPCID: evt.VpcId, - }); err != nil { - slog.Error("vpcd: DeletePort failed", "eni_id", evt.NetworkInterfaceId, "err", err) - respond(msg, err) - return - } - respond(msg, nil) -} - -// handleUpdatePortSGs reconciles the port group membership for an ENI's LSP -// against the desired SG list in the event. The payload is declarative — the -// manager computes the add/remove diff from current OVN state. -func (h *TopologyHandler) handleUpdatePortSGs(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt UpdatePortSGsEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.update-port-sgs event", "err", err) - respond(msg, err) - return - } - if err := h.SetPortSecurityGroups(context.Background(), evt.NetworkInterfaceId, evt.SecurityGroupIds); err != nil { - slog.Error("vpcd: SetPortSecurityGroups failed", - "eni_id", evt.NetworkInterfaceId, "sgs", evt.SecurityGroupIds, "err", err) - respond(msg, err) - return - } - respond(msg, nil) -} - -// --- Internet Gateway (external connectivity + NAT) --- - -// handleIGWAttach decodes the event and delegates to -// external.IGWManager.AttachIGW. -func (h *TopologyHandler) handleIGWAttach(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt types.IGWEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.igw-attach event", "err", err) - respond(msg, err) - return - } - ctx := context.Background() - - igw, err := h.igwManager() - if err != nil { - slog.Error("vpcd: IGW manager init failed", "err", err) - respond(msg, err) - return - } - if err := igw.AttachIGW(ctx, external.IGWSpec{ - VPCID: evt.VpcId, - InternetGatewayID: evt.InternetGatewayId, - }); err != nil { - slog.Error("vpcd: AttachIGW failed", - "vpc_id", evt.VpcId, "igw_id", evt.InternetGatewayId, "err", err) - respond(msg, err) - return - } - respond(msg, nil) -} - -// handleIGWDetach decodes the event and delegates the OVN teardown to -// external.IGWManager.DetachIGW. Static gateway LRP IPs live in OVN -// external_ids and are cleared when the LRP itself is deleted. -func (h *TopologyHandler) handleIGWDetach(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt types.IGWEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.igw-detach event", "err", err) - respond(msg, err) - return - } - ctx := context.Background() - - igw, err := h.igwManager() - if err != nil { - slog.Error("vpcd: IGW manager init failed", "err", err) - respond(msg, err) - return - } - if err := igw.DetachIGW(ctx, evt.VpcId); err != nil { - slog.Error("vpcd: DetachIGW failed", "vpc_id", evt.VpcId, "err", err) - respond(msg, err) - return - } - slog.Info("vpcd: detached internet gateway from VPC", - "igw_id", evt.InternetGatewayId, "vpc_id", evt.VpcId) - respond(msg, nil) -} - -// --- NAT (dnat_and_snat for public IPs) --- - -// handleAddNAT decodes the EIP event and delegates to -// policy.NATManager.AddEIP. The manager owns the idempotency-skip, -// stale-rule cleanup, distributed-NAT mode selection, and post-write -// flow-install barrier. -func (h *TopologyHandler) handleAddNAT(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt NATEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.add-nat event", "err", err) - respond(msg, err) - return - } - nm, err := h.natManager() - if err != nil { - slog.Error("vpcd: NAT manager init failed", "err", err) - respond(msg, err) - return - } - if err := nm.AddEIP(context.Background(), policy.EIPSpec{ - VPCID: evt.VpcId, - ExternalIP: evt.ExternalIP, - LogicalIP: evt.LogicalIP, - PortName: evt.PortName, - MAC: evt.MAC, - }); err != nil { - slog.Error("vpcd: AddEIP failed", - "vpc_id", evt.VpcId, "external_ip", evt.ExternalIP, - "logical_ip", evt.LogicalIP, "err", err) - respond(msg, err) - return - } - slog.Info("vpcd: added dnat_and_snat rule", - "vpc_id", evt.VpcId, "external_ip", evt.ExternalIP, - "logical_ip", evt.LogicalIP, "port", evt.PortName) - respond(msg, nil) -} - -// handleDeleteNAT decodes the event and delegates to -// policy.NATManager.DeleteEIP. Idempotency on already-absent rules is -// handled by the manager. -func (h *TopologyHandler) handleDeleteNAT(msg *nats.Msg) { - if h.ovn == nil { - respond(msg, fmt.Errorf("OVN client not connected")) - return - } - var evt NATEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.delete-nat event", "err", err) - respond(msg, err) - return - } - nm, err := h.natManager() - if err != nil { - slog.Error("vpcd: NAT manager init failed", "err", err) - respond(msg, err) - return - } - if err := nm.DeleteEIP(context.Background(), evt.VpcId, evt.LogicalIP); err != nil { - slog.Error("vpcd: DeleteEIP failed", - "vpc_id", evt.VpcId, "logical_ip", evt.LogicalIP, "err", err) - respond(msg, err) - return - } - slog.Info("vpcd: deleted dnat_and_snat rule", - "vpc_id", evt.VpcId, "logical_ip", evt.LogicalIP) - respond(msg, nil) -} - -// --- Helpers --- - -// respond sends a simple JSON response to a NATS request. -func respond(msg *nats.Msg, err error) { - if msg.Reply == "" { - return // fire-and-forget, no reply expected - } - - type response struct { - Success bool `json:"success"` - Error string `json:"error,omitempty"` - } - - resp := response{Success: true} - if err != nil { - resp.Success = false - resp.Error = err.Error() - } - - data, marshalErr := json.Marshal(resp) - if marshalErr != nil { - slog.Error("vpcd: failed to marshal NATS response", "err", marshalErr) - data = []byte(`{"success":false,"error":"internal marshal failure"}`) - } - if err := msg.Respond(data); err != nil { - slog.Error("vpcd: failed to respond to NATS request", "err", err) - } -} - -// handleAddNATGateway is a thin wrapper that decodes the NATS event and -// delegates the SNAT installation to policy.NATManager.AddNATGateway. -func (h *TopologyHandler) handleAddNATGateway(msg *nats.Msg) { - var evt NATGatewayEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.add-nat-gateway event", "err", err) - return - } - nm, err := h.natManager() - if err != nil { - slog.Error("vpcd: NAT manager init failed", "err", err) - return - } - if err := nm.AddNATGateway(context.Background(), policy.NATGWSpec{ - VPCID: evt.VpcId, - NATGatewayID: evt.NatGatewayId, - PublicIP: evt.PublicIp, - SubnetCIDR: evt.SubnetCidr, - }); err != nil { - slog.Error("vpcd: AddNATGateway failed", - "vpc_id", evt.VpcId, "natgw_id", evt.NatGatewayId, - "public_ip", evt.PublicIp, "subnet_cidr", evt.SubnetCidr, "err", err) - return - } - slog.Info("vpcd: NAT Gateway SNAT rule added", - "vpc_id", evt.VpcId, "natgw_id", evt.NatGatewayId, - "public_ip", evt.PublicIp, "subnet_cidr", evt.SubnetCidr) -} - -// handleDeleteNATGateway delegates SNAT teardown to policy.NATManager. -func (h *TopologyHandler) handleDeleteNATGateway(msg *nats.Msg) { - var evt NATGatewayEvent - if err := json.Unmarshal(msg.Data, &evt); err != nil { - slog.Error("vpcd: failed to unmarshal vpc.delete-nat-gateway event", "err", err) - return - } - nm, err := h.natManager() - if err != nil { - slog.Error("vpcd: NAT manager init failed", "err", err) - return - } - if err := nm.DeleteNATGateway(context.Background(), evt.VpcId, evt.SubnetCidr); err != nil { - slog.Warn("vpcd: DeleteNATGateway failed", - "vpc_id", evt.VpcId, "subnet_cidr", evt.SubnetCidr, "err", err) - return - } - slog.Info("vpcd: NAT Gateway SNAT rule removed", - "vpc_id", evt.VpcId, "natgw_id", evt.NatGatewayId, "subnet_cidr", evt.SubnetCidr) -} diff --git a/spinifex/services/vpcd/topology_manager.go b/spinifex/services/vpcd/topology_manager.go deleted file mode 100644 index d0ebc9d38..000000000 --- a/spinifex/services/vpcd/topology_manager.go +++ /dev/null @@ -1,157 +0,0 @@ -package vpcd - -// Phase 2.6 (mulga-siv-129) adapter: TopologyHandler still implements -// topology.Manager so existing NATS subscribers and tests stay green, but the -// concrete logic now lives in network/topology.NewLiveManager. The shim here -// constructs the live manager lazily on first use, passing TopologyHandler's -// per-VPC external-pool resolver as the DHCP dns_server callback. -// -// Once all NATS subscribers route through the live manager directly (the -// remainder of mulga-siv-129), this adapter — and TopologyHandler's -// topology.Manager conformance — can be deleted along with topology.go. - -import ( - "context" - "strings" - - "github.com/mulgadc/spinifex/spinifex/network/external" - "github.com/mulgadc/spinifex/spinifex/network/policy" - "github.com/mulgadc/spinifex/spinifex/network/topology" -) - -var _ topology.Manager = (*TopologyHandler)(nil) - -func (h *TopologyHandler) topologyManager() topology.Manager { - h.lmOnce.Do(func() { - h.lm = topology.NewLiveManager(h.ovn, topology.WithDNSServer(h.lmDNSServer)) - }) - return h.lm -} - -// lmDNSServer is the callback handed to topology.NewLiveManager so per-subnet -// DHCPOptions inherit the external pool's configured DNS list. Mirrors the -// legacy dnsServer() method without depending on TopologyHandler.findExternalPool's -// region/AZ keys (Phase 2.6 leaves multi-AZ routing to Phase 3). -func (h *TopologyHandler) lmDNSServer() string { - pool := h.findExternalPool("", "") - if pool != nil && len(pool.DNSServers) > 0 { - return "{" + strings.Join(pool.DNSServers, ", ") + "}" - } - return "{8.8.8.8, 1.1.1.1}" -} - -// EnsureVPC creates the OVN logical router for the VPC, idempotently. -func (h *TopologyHandler) EnsureVPC(ctx context.Context, spec topology.VPCSpec) error { - return h.topologyManager().EnsureVPC(ctx, spec) -} - -// DeleteVPC removes the OVN logical router for the VPC and cascades through -// any subnets that belong to it. -func (h *TopologyHandler) DeleteVPC(ctx context.Context, vpcID string) error { - return h.topologyManager().DeleteVPC(ctx, vpcID) -} - -// EnsureSubnet creates the OVN logical switch, subnet→VPC router port pair, -// and DHCP options for the subnet. -func (h *TopologyHandler) EnsureSubnet(ctx context.Context, spec topology.SubnetSpec) error { - return h.topologyManager().EnsureSubnet(ctx, spec) -} - -// DeleteSubnet tears down the subnet's logical switch, router port, switch -// router port, and DHCP options. -func (h *TopologyHandler) DeleteSubnet(ctx context.Context, spec topology.SubnetSpec) error { - return h.topologyManager().DeleteSubnet(ctx, spec) -} - -// EnsurePort creates the ENI's LogicalSwitchPort and joins it to its initial -// SG port groups in one OVSDB transaction. -func (h *TopologyHandler) EnsurePort(ctx context.Context, spec topology.PortSpec) error { - return h.topologyManager().EnsurePort(ctx, spec) -} - -// DeletePort clears the ENI's port-group memberships and removes the LSP. -func (h *TopologyHandler) DeletePort(ctx context.Context, spec topology.PortSpec) error { - return h.topologyManager().DeletePort(ctx, spec) -} - -// SetPortSecurityGroups reconciles the port's port-group memberships against -// the declared list. -func (h *TopologyHandler) SetPortSecurityGroups(ctx context.Context, portID string, sgIDs []string) error { - return h.topologyManager().SetPortSecurityGroups(ctx, portID, sgIDs) -} - -// EnsureSGPortGroup creates the OVN port group for a security group. -func (h *TopologyHandler) EnsureSGPortGroup(ctx context.Context, groupID string) error { - return h.topologyManager().EnsureSGPortGroup(ctx, groupID) -} - -// DeleteSGPortGroup removes the OVN port group for a security group. -func (h *TopologyHandler) DeleteSGPortGroup(ctx context.Context, groupID string) error { - return h.topologyManager().DeleteSGPortGroup(ctx, groupID) -} - -// DeleteSGPortGroupByName removes the OVN port group given its raw name. -func (h *TopologyHandler) DeleteSGPortGroupByName(ctx context.Context, pgName string) error { - return h.topologyManager().DeleteSGPortGroupByName(ctx, pgName) -} - -// securityGroupManager returns the lazily-constructed policy SG manager -// backed by the same OVN client as the live topology manager. Subscribers -// use it to apply ACL sets against the SG's port group. -func (h *TopologyHandler) securityGroupManager() policy.SecurityGroupManager { - h.sgmOnce.Do(func() { - h.sgm = policy.NewSecurityGroupManager(h.ovn) - }) - return h.sgm -} - -// natManager returns the lazily-constructed policy.NATManager. NAT mode is -// the value injected via WithNATMode; the FlowsBarrier is bound to -// waitForFlowsHV so the EIP subscriber blocks on hypervisor flow-install -// before responding. -func (h *TopologyHandler) natManager() (policy.NATManager, error) { - h.natmOnce.Do(func() { - h.natm, h.natmErr = policy.NewNATManager(h.ovn, h.natMode, policy.WithFlowsBarrier(waitForFlowsHV)) - }) - return h.natm, h.natmErr -} - -// igwManager returns the lazily-constructed external.IGWManager. All -// IGW attach/detach paths route through it. -func (h *TopologyHandler) igwManager() (external.IGWManager, error) { - h.igwmOnce.Do(func() { - nm, err := h.natManager() - if err != nil { - h.igwmErr = err - return - } - var poolCfg *external.ExternalPoolConfig - if p := h.findExternalPool("", ""); p != nil { - shared := external.ExternalPoolConfig{ - Name: p.Name, - RangeStart: p.RangeStart, - RangeEnd: p.RangeEnd, - Gateway: p.Gateway, - GatewayIP: p.GatewayIP, - PrefixLen: p.PrefixLen, - DNSServers: p.DNSServers, - Region: p.Region, - AZ: p.AZ, - GwLrpRangeStart: p.GwLrpRangeStart, - GwLrpRangeEnd: p.GwLrpRangeEnd, - } - poolCfg = &shared - } - h.igwm, h.igwmErr = external.NewIGWManager(external.IGWManagerConfig{ - OVN: h.ovn, - Routes: policy.NewRouteManager(h.ovn), - NAT: nm, - Pool: poolCfg, - Allocator: external.NewStaticRangeAllocator(h.ovn), - Chassis: h.chassisNames, - NATMode: h.natMode, - FlowsBarrier: waitForFlowsHV, - }) - }) - return h.igwm, h.igwmErr -} diff --git a/spinifex/services/vpcd/topology_test.go b/spinifex/services/vpcd/topology_test.go deleted file mode 100644 index 054941762..000000000 --- a/spinifex/services/vpcd/topology_test.go +++ /dev/null @@ -1,3052 +0,0 @@ -package vpcd - -import ( - "context" - "encoding/json" - "slices" - "strings" - "testing" - "time" - - "github.com/mulgadc/spinifex/spinifex/network/ovn/nbdb" - "github.com/mulgadc/spinifex/spinifex/network/policy" - "github.com/mulgadc/spinifex/spinifex/network/topology" - "github.com/mulgadc/spinifex/spinifex/testutil" - "github.com/mulgadc/spinifex/spinifex/types" - "github.com/nats-io/nats-server/v2/server" - "github.com/nats-io/nats.go" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// startTestNATS starts an embedded NATS server for testing. -func startTestNATS(t *testing.T) (*server.Server, *nats.Conn) { - t.Helper() - return testutil.StartTestNATS(t) -} - -func TestTopologyHandler_VPCCreate(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - evt := VPCEvent{VpcId: "vpc-abc123", CidrBlock: "10.0.0.0/16", VNI: 100} - data, _ := json.Marshal(evt) - - resp, err := nc.Request(TopicVPCCreate, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.create: %v", err) - } - - var result struct { - Success bool `json:"success"` - Error string `json:"error,omitempty"` - } - if err := json.Unmarshal(resp.Data, &result); err != nil { - t.Fatalf("unmarshal response: %v", err) - } - if !result.Success { - t.Fatalf("vpc.create failed: %s", result.Error) - } - - // Verify router was created in OVN - ctx := context.Background() - lr, err := mock.GetLogicalRouter(ctx, "vpc-vpc-abc123") - if err != nil { - t.Fatalf("expected logical router: %v", err) - } - if lr.ExternalIDs["spinifex:vpc_id"] != "vpc-abc123" { - t.Errorf("expected vpc_id external_id, got %v", lr.ExternalIDs) - } - if lr.ExternalIDs["spinifex:vni"] != "100" { - t.Errorf("expected vni external_id=100, got %v", lr.ExternalIDs["spinifex:vni"]) - } - if lr.ExternalIDs["spinifex:cidr"] != "10.0.0.0/16" { - t.Errorf("expected cidr external_id=10.0.0.0/16, got %v", lr.ExternalIDs["spinifex:cidr"]) - } -} - -func TestTopologyHandler_VPCDelete(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Pre-create a router - _ = mock.CreateLogicalRouter(ctx, nbdbLogicalRouter("vpc-vpc-xyz", "vpc-xyz")) - - // Delete via NATS - evt := VPCEvent{VpcId: "vpc-xyz"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicVPCDelete, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.delete: %v", err) - } - - var result struct { - Success bool `json:"Success"` - } - _ = json.Unmarshal(resp.Data, &result) - if !result.Success { - t.Fatal("vpc.delete failed") - } - - // Verify router is gone - _, err = mock.GetLogicalRouter(ctx, "vpc-vpc-xyz") - if err == nil { - t.Error("expected router to be deleted") - } -} - -func TestTopologyHandler_SubnetCreate(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Pre-create the VPC router - _ = mock.CreateLogicalRouter(ctx, nbdbLogicalRouter("vpc-vpc-sub1", "vpc-sub1")) - - // Create subnet - evt := SubnetEvent{SubnetId: "subnet-aaa", VpcId: "vpc-sub1", CidrBlock: "10.0.1.0/24"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicSubnetCreate, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.create-subnet: %v", err) - } - - var result struct { - Success bool `json:"success"` - Error string `json:"error,omitempty"` - } - _ = json.Unmarshal(resp.Data, &result) - if !result.Success { - t.Fatalf("vpc.create-subnet failed: %s", result.Error) - } - - // Verify logical switch created - ls, err := mock.GetLogicalSwitch(ctx, "subnet-subnet-aaa") - if err != nil { - t.Fatalf("expected logical switch: %v", err) - } - if ls.ExternalIDs["spinifex:subnet_id"] != "subnet-aaa" { - t.Errorf("expected subnet_id external_id, got %v", ls.ExternalIDs) - } - - // Verify router port created - lr, err := mock.GetLogicalRouter(ctx, "vpc-vpc-sub1") - if err != nil { - t.Fatalf("expected router: %v", err) - } - if len(lr.Ports) != 1 { - t.Errorf("expected 1 router port, got %d", len(lr.Ports)) - } - - // Verify switch has 1 port (the router port) - if len(ls.Ports) != 1 { - t.Errorf("expected 1 switch port, got %d", len(ls.Ports)) - } - - // Verify DHCP options created - dhcpOpts, err := mock.FindDHCPOptionsByCIDR(ctx, "10.0.1.0/24") - if err != nil { - t.Fatalf("expected DHCP options: %v", err) - } - if dhcpOpts.Options["router"] != "10.0.1.1" { - t.Errorf("expected router=10.0.1.1, got %s", dhcpOpts.Options["router"]) - } - if dhcpOpts.Options["lease_time"] != "3600" { - t.Errorf("expected lease_time=3600, got %s", dhcpOpts.Options["lease_time"]) - } - if dhcpOpts.Options["mtu"] != "1442" { - t.Errorf("expected mtu=1442, got %s", dhcpOpts.Options["mtu"]) - } -} - -func TestTopologyHandler_SubnetDelete(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Setup: create VPC router + subnet topology manually - _ = mock.CreateLogicalRouter(ctx, nbdbLogicalRouter("vpc-vpc-del", "vpc-del")) - _ = mock.CreateLogicalSwitch(ctx, nbdbLogicalSwitch("subnet-subnet-del", "subnet-del", "vpc-del")) - _ = mock.CreateLogicalRouterPort(ctx, "vpc-vpc-del", nbdbLogicalRouterPort("rtr-subnet-del", "subnet-del", "vpc-del")) - _ = mock.CreateLogicalSwitchPort(ctx, "subnet-subnet-del", nbdbLogicalSwitchPortRouter("rtr-port-subnet-del", "rtr-subnet-del", "subnet-del", "vpc-del")) - _, _ = mock.CreateDHCPOptions(ctx, nbdbDHCPOptions("10.0.2.0/24", "subnet-del", "vpc-del")) - - // Delete subnet via NATS - evt := SubnetEvent{SubnetId: "subnet-del", VpcId: "vpc-del", CidrBlock: "10.0.2.0/24"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicSubnetDelete, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.delete-subnet: %v", err) - } - - var result struct { - Success bool `json:"success"` - Error string `json:"error,omitempty"` - } - _ = json.Unmarshal(resp.Data, &result) - if !result.Success { - t.Fatalf("vpc.delete-subnet failed: %s", result.Error) - } - - // Verify switch is deleted - _, err = mock.GetLogicalSwitch(ctx, "subnet-subnet-del") - if err == nil { - t.Error("expected switch to be deleted") - } - - // Verify DHCP options are deleted - _, err = mock.FindDHCPOptionsByCIDR(ctx, "10.0.2.0/24") - if err == nil { - t.Error("expected DHCP options to be deleted") - } - - // Verify router still exists (only subnet topology deleted, not VPC) - _, err = mock.GetLogicalRouter(ctx, "vpc-vpc-del") - if err != nil { - t.Error("expected VPC router to still exist") - } -} - -func TestTopologyHandler_FullLifecycle(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // 1. Create VPC - vpcEvt := VPCEvent{VpcId: "vpc-full", CidrBlock: "10.0.0.0/16", VNI: 200} - data, _ := json.Marshal(vpcEvt) - resp, _ := nc.Request(TopicVPCCreate, data, 5_000_000_000) - assertSuccess(t, resp, "create VPC") - - // 2. Create Subnet 1 - subEvt1 := SubnetEvent{SubnetId: "subnet-a", VpcId: "vpc-full", CidrBlock: "10.0.1.0/24"} - data, _ = json.Marshal(subEvt1) - resp, _ = nc.Request(TopicSubnetCreate, data, 5_000_000_000) - assertSuccess(t, resp, "create subnet-a") - - // 3. Create Subnet 2 - subEvt2 := SubnetEvent{SubnetId: "subnet-b", VpcId: "vpc-full", CidrBlock: "10.0.2.0/24"} - data, _ = json.Marshal(subEvt2) - resp, _ = nc.Request(TopicSubnetCreate, data, 5_000_000_000) - assertSuccess(t, resp, "create subnet-b") - - // Verify: 1 router with 2 ports, 2 switches, 2 DHCP option sets - routers, _ := mock.ListLogicalRouters(ctx) - if len(routers) != 1 { - t.Errorf("expected 1 router, got %d", len(routers)) - } - switches, _ := mock.ListLogicalSwitches(ctx) - if len(switches) != 2 { - t.Errorf("expected 2 switches, got %d", len(switches)) - } - dhcpList, _ := mock.ListDHCPOptions(ctx) - if len(dhcpList) != 2 { - t.Errorf("expected 2 DHCP option sets, got %d", len(dhcpList)) - } - - // 4. Delete Subnet 1 - data, _ = json.Marshal(subEvt1) - resp, _ = nc.Request(TopicSubnetDelete, data, 5_000_000_000) - assertSuccess(t, resp, "delete subnet-a") - - switches, _ = mock.ListLogicalSwitches(ctx) - if len(switches) != 1 { - t.Errorf("expected 1 switch after delete, got %d", len(switches)) - } - - // 5. Delete Subnet 2 - data, _ = json.Marshal(subEvt2) - resp, _ = nc.Request(TopicSubnetDelete, data, 5_000_000_000) - assertSuccess(t, resp, "delete subnet-b") - - // 6. Delete VPC - delEvt := VPCEvent{VpcId: "vpc-full"} - data, _ = json.Marshal(delEvt) - resp, _ = nc.Request(TopicVPCDelete, data, 5_000_000_000) - assertSuccess(t, resp, "delete VPC") - - // Verify everything is gone - routers, _ = mock.ListLogicalRouters(ctx) - if len(routers) != 0 { - t.Errorf("expected 0 routers, got %d", len(routers)) - } - switches, _ = mock.ListLogicalSwitches(ctx) - if len(switches) != 0 { - t.Errorf("expected 0 switches, got %d", len(switches)) - } - dhcpList, _ = mock.ListDHCPOptions(ctx) - if len(dhcpList) != 0 { - t.Errorf("expected 0 DHCP options, got %d", len(dhcpList)) - } -} - -func TestTopologyHandler_VPCDeleteCascade(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Create VPC + subnet directly in mock (simulating pre-existing state) - _ = mock.CreateLogicalRouter(ctx, nbdbLogicalRouter("vpc-vpc-cas", "vpc-cas")) - _ = mock.CreateLogicalSwitch(ctx, nbdbLogicalSwitch("subnet-sub-cas", "sub-cas", "vpc-cas")) - _, _ = mock.CreateDHCPOptions(ctx, nbdbDHCPOptions("10.0.3.0/24", "sub-cas", "vpc-cas")) - - // Delete VPC should cascade-delete switches and DHCP - evt := VPCEvent{VpcId: "vpc-cas"} - data, _ := json.Marshal(evt) - resp, _ := nc.Request(TopicVPCDelete, data, 5_000_000_000) - assertSuccess(t, resp, "cascade delete VPC") - - switches, _ := mock.ListLogicalSwitches(ctx) - if len(switches) != 0 { - t.Errorf("expected 0 switches after cascade delete, got %d", len(switches)) - } - dhcpList, _ := mock.ListDHCPOptions(ctx) - if len(dhcpList) != 0 { - t.Errorf("expected 0 DHCP options after cascade delete, got %d", len(dhcpList)) - } -} - -func TestTopologyHandler_CreatePort(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Setup: create VPC router, subnet switch, and DHCP options - _ = mock.CreateLogicalRouter(ctx, nbdbLogicalRouter("vpc-vpc-port1", "vpc-port1")) - _ = mock.CreateLogicalSwitch(ctx, nbdbLogicalSwitch("subnet-subnet-port1", "subnet-port1", "vpc-port1")) - dhcpUUID, _ := mock.CreateDHCPOptions(ctx, nbdbDHCPOptions("10.0.1.0/24", "subnet-port1", "vpc-port1")) - - // Create port via NATS - evt := PortEvent{ - NetworkInterfaceId: "eni-aaa111", - SubnetId: "subnet-port1", - VpcId: "vpc-port1", - PrivateIpAddress: "10.0.1.4", - MacAddress: "02:00:00:11:22:33", - } - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicCreatePort, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.create-port: %v", err) - } - assertSuccess(t, resp, "create port") - - // Verify logical switch port created - lsp, err := mock.GetLogicalSwitchPort(ctx, "port-eni-aaa111") - if err != nil { - t.Fatalf("expected logical switch port: %v", err) - } - - // Verify addresses - expectedAddr := "02:00:00:11:22:33 10.0.1.4" - if len(lsp.Addresses) != 1 || lsp.Addresses[0] != expectedAddr { - t.Errorf("expected addresses [%s], got %v", expectedAddr, lsp.Addresses) - } - - // Verify port security - if len(lsp.PortSecurity) != 1 || lsp.PortSecurity[0] != expectedAddr { - t.Errorf("expected port_security [%s], got %v", expectedAddr, lsp.PortSecurity) - } - - // Verify DHCPv4 options - if lsp.DHCPv4Options == nil { - t.Fatal("expected DHCPv4Options to be set") - } - if *lsp.DHCPv4Options != dhcpUUID { - t.Errorf("expected DHCPv4Options UUID %s, got %s", dhcpUUID, *lsp.DHCPv4Options) - } - - // Verify external IDs - if lsp.ExternalIDs["spinifex:eni_id"] != "eni-aaa111" { - t.Errorf("expected eni_id=eni-aaa111, got %s", lsp.ExternalIDs["spinifex:eni_id"]) - } - if lsp.ExternalIDs["spinifex:subnet_id"] != "subnet-port1" { - t.Errorf("expected subnet_id=subnet-port1, got %s", lsp.ExternalIDs["spinifex:subnet_id"]) - } - if lsp.ExternalIDs["spinifex:vpc_id"] != "vpc-port1" { - t.Errorf("expected vpc_id=vpc-port1, got %s", lsp.ExternalIDs["spinifex:vpc_id"]) - } - - // Verify port was added to the switch - ls, err := mock.GetLogicalSwitch(ctx, "subnet-subnet-port1") - if err != nil { - t.Fatalf("get switch: %v", err) - } - if len(ls.Ports) != 1 { - t.Errorf("expected 1 port on switch, got %d", len(ls.Ports)) - } -} - -func TestTopologyHandler_DeletePort(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Setup: create switch and port - _ = mock.CreateLogicalSwitch(ctx, nbdbLogicalSwitch("subnet-subnet-del2", "subnet-del2", "vpc-del2")) - _ = mock.CreateLogicalSwitchPort(ctx, "subnet-subnet-del2", &nbdb.LogicalSwitchPort{ - Name: "port-eni-bbb222", - Addresses: []string{"02:00:00:44:55:66 10.0.2.4"}, - PortSecurity: []string{"02:00:00:44:55:66 10.0.2.4"}, - ExternalIDs: map[string]string{ - "spinifex:eni_id": "eni-bbb222", - "spinifex:subnet_id": "subnet-del2", - "spinifex:vpc_id": "vpc-del2", - }, - }) - - // Verify port exists before delete - _, err = mock.GetLogicalSwitchPort(ctx, "port-eni-bbb222") - if err != nil { - t.Fatalf("expected port to exist before delete: %v", err) - } - - // Delete port via NATS - evt := PortEvent{ - NetworkInterfaceId: "eni-bbb222", - SubnetId: "subnet-del2", - VpcId: "vpc-del2", - PrivateIpAddress: "10.0.2.4", - MacAddress: "02:00:00:44:55:66", - } - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicDeletePort, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.delete-port: %v", err) - } - assertSuccess(t, resp, "delete port") - - // Verify port is gone - _, err = mock.GetLogicalSwitchPort(ctx, "port-eni-bbb222") - if err == nil { - t.Error("expected port to be deleted") - } - - // Verify switch still exists but has no ports - ls, err := mock.GetLogicalSwitch(ctx, "subnet-subnet-del2") - if err != nil { - t.Fatalf("expected switch to still exist: %v", err) - } - if len(ls.Ports) != 0 { - t.Errorf("expected 0 ports on switch after delete, got %d", len(ls.Ports)) - } -} - -func TestTopologyHandler_CreatePortNoDHCP(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Setup: create switch but NO DHCP options - _ = mock.CreateLogicalSwitch(ctx, nbdbLogicalSwitch("subnet-subnet-nodhcp", "subnet-nodhcp", "vpc-nodhcp")) - - // Create port — should succeed but without DHCPv4Options - evt := PortEvent{ - NetworkInterfaceId: "eni-nodhcp", - SubnetId: "subnet-nodhcp", - VpcId: "vpc-nodhcp", - PrivateIpAddress: "10.0.3.4", - MacAddress: "02:00:00:77:88:99", - } - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicCreatePort, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.create-port: %v", err) - } - assertSuccess(t, resp, "create port without DHCP") - - // Port should exist but without DHCPv4Options - lsp, err := mock.GetLogicalSwitchPort(ctx, "port-eni-nodhcp") - if err != nil { - t.Fatalf("expected port: %v", err) - } - if lsp.DHCPv4Options != nil { - t.Errorf("expected nil DHCPv4Options when no DHCP configured, got %s", *lsp.DHCPv4Options) - } -} - -func TestTopologyHandler_PortLifecycle(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // 1. Create VPC - vpcEvt := VPCEvent{VpcId: "vpc-plc", CidrBlock: "10.0.0.0/16", VNI: 300} - data, _ := json.Marshal(vpcEvt) - resp, _ := nc.Request(TopicVPCCreate, data, 5_000_000_000) - assertSuccess(t, resp, "create VPC") - - // 2. Create Subnet - subEvt := SubnetEvent{SubnetId: "subnet-plc", VpcId: "vpc-plc", CidrBlock: "10.0.1.0/24"} - data, _ = json.Marshal(subEvt) - resp, _ = nc.Request(TopicSubnetCreate, data, 5_000_000_000) - assertSuccess(t, resp, "create subnet") - - // 3. Create two ports - portEvt1 := PortEvent{ - NetworkInterfaceId: "eni-plc-1", - SubnetId: "subnet-plc", - VpcId: "vpc-plc", - PrivateIpAddress: "10.0.1.4", - MacAddress: "02:00:00:aa:bb:01", - } - data, _ = json.Marshal(portEvt1) - resp, _ = nc.Request(TopicCreatePort, data, 5_000_000_000) - assertSuccess(t, resp, "create port 1") - - portEvt2 := PortEvent{ - NetworkInterfaceId: "eni-plc-2", - SubnetId: "subnet-plc", - VpcId: "vpc-plc", - PrivateIpAddress: "10.0.1.5", - MacAddress: "02:00:00:aa:bb:02", - } - data, _ = json.Marshal(portEvt2) - resp, _ = nc.Request(TopicCreatePort, data, 5_000_000_000) - assertSuccess(t, resp, "create port 2") - - // Verify switch has 3 ports (router port + 2 ENI ports) - ls, err := mock.GetLogicalSwitch(ctx, "subnet-subnet-plc") - if err != nil { - t.Fatalf("get switch: %v", err) - } - if len(ls.Ports) != 3 { - t.Errorf("expected 3 ports (1 router + 2 ENI), got %d", len(ls.Ports)) - } - - // 4. Delete port 1 - data, _ = json.Marshal(portEvt1) - resp, _ = nc.Request(TopicDeletePort, data, 5_000_000_000) - assertSuccess(t, resp, "delete port 1") - - // Verify switch has 2 ports now - ls, err = mock.GetLogicalSwitch(ctx, "subnet-subnet-plc") - if err != nil { - t.Fatalf("get switch after port delete: %v", err) - } - if len(ls.Ports) != 2 { - t.Errorf("expected 2 ports after delete, got %d", len(ls.Ports)) - } - - // Port 2 should still exist - _, err = mock.GetLogicalSwitchPort(ctx, "port-eni-plc-2") - if err != nil { - t.Error("expected port 2 to still exist") - } - - // Port 1 should be gone - _, err = mock.GetLogicalSwitchPort(ctx, "port-eni-plc-1") - if err == nil { - t.Error("expected port 1 to be deleted") - } -} - -func TestTopologyHandler_NilOVN(t *testing.T) { - _, nc := startTestNATS(t) - - // nil OVN client (OVN not connected) - topo := NewTopologyHandler(nil) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Should fail gracefully when OVN is nil - evt := VPCEvent{VpcId: "vpc-nil", CidrBlock: "10.0.0.0/16", VNI: 100} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicVPCCreate, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - - var result struct { - Success bool `json:"success"` - Error string `json:"error,omitempty"` - } - _ = json.Unmarshal(resp.Data, &result) - if result.Success { - t.Error("expected failure when OVN is nil") - } - - // Port create should also fail gracefully - portEvt := PortEvent{ - NetworkInterfaceId: "eni-nil", - SubnetId: "subnet-nil", - VpcId: "vpc-nil", - PrivateIpAddress: "10.0.1.4", - MacAddress: "02:00:00:00:00:01", - } - data, _ = json.Marshal(portEvt) - resp, err = nc.Request(TopicCreatePort, data, 5_000_000_000) - if err != nil { - t.Fatalf("request create port: %v", err) - } - _ = json.Unmarshal(resp.Data, &result) - if result.Success { - t.Error("expected create-port failure when OVN is nil") - } - - // Port delete should also fail gracefully - resp, err = nc.Request(TopicDeletePort, data, 5_000_000_000) - if err != nil { - t.Fatalf("request delete port: %v", err) - } - _ = json.Unmarshal(resp.Data, &result) - if result.Success { - t.Error("expected delete-port failure when OVN is nil") - } -} - -func TestTopologyHandler_IGWAttach(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Pre-create VPC router - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-igw1", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-igw1", - "spinifex:cidr": "10.0.0.0/16", - }, - }) - - // Attach IGW - evt := types.IGWEvent{InternetGatewayId: "igw-test1", VpcId: "vpc-igw1"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.igw-attach: %v", err) - } - assertSuccess(t, resp, "attach IGW") - - // Verify external switch created - extSwitch, err := mock.GetLogicalSwitch(ctx, "ext-vpc-igw1") - if err != nil { - t.Fatalf("expected external switch: %v", err) - } - if extSwitch.ExternalIDs["spinifex:role"] != "external" { - t.Errorf("expected role=external, got %s", extSwitch.ExternalIDs["spinifex:role"]) - } - if extSwitch.ExternalIDs["spinifex:igw_id"] != "igw-test1" { - t.Errorf("expected igw_id=igw-test1, got %s", extSwitch.ExternalIDs["spinifex:igw_id"]) - } - - // Verify localnet port created on external switch - _, err = mock.GetLogicalSwitchPort(ctx, "ext-port-vpc-igw1") - if err != nil { - t.Fatalf("expected localnet port: %v", err) - } - - // Verify gateway router port created - router, err := mock.GetLogicalRouter(ctx, "vpc-vpc-igw1") - if err != nil { - t.Fatalf("expected router: %v", err) - } - if len(router.Ports) < 1 { - t.Error("expected at least 1 router port (gateway)") - } - - // Verify switch gateway port created - _, err = mock.GetLogicalSwitchPort(ctx, "gw-port-vpc-igw1") - if err != nil { - t.Fatalf("expected switch gateway port: %v", err) - } - - // Verify NO blanket SNAT rule — only per-VM dnat_and_snat rules should - // provide NAT (AWS parity: private subnet instances cannot route via IGW) - if len(router.NAT) != 0 { - t.Errorf("expected 0 NAT rules (no blanket SNAT), got %d", len(router.NAT)) - } - - // Verify default route added - if len(router.StaticRoutes) != 1 { - t.Errorf("expected 1 static route, got %d", len(router.StaticRoutes)) - } -} - -// TestTopologyHandler_IGWAttach_InvokesFlowBarrier asserts handleIGWAttach -// calls waitForFlowsHV after the OVN writes complete (mulga-siv-105). The -// barrier itself shells out to ovn-nbctl --wait=hv sync; stub it here so -// the test doesn't depend on a live OVN. -func TestTopologyHandler_IGWAttach_InvokesFlowBarrier(t *testing.T) { - orig := waitForFlowsHV - var called int - waitForFlowsHV = func() error { called++; return nil } - defer func() { waitForFlowsHV = orig }() - - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - require.NoError(t, mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-barrier1", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-barrier1", - "spinifex:cidr": "10.0.0.0/16", - }, - })) - - evt := types.IGWEvent{InternetGatewayId: "igw-barrier1", VpcId: "vpc-barrier1"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "attach IGW") - - if called != 1 { - t.Errorf("expected waitForFlowsHV to run exactly once after IGW attach, got %d", called) - } -} - -// TestTopologyHandler_AddNAT_InvokesFlowBarrier asserts handleAddNAT -// calls waitForFlowsHV after writing the dnat_and_snat row to NB. -// Without this barrier, RunInstances returns success while OF flows for -// the new public IP are still being installed on the gateway chassis, -// and the freshly launched VM is unreachable on its public IP for the -// flow-install latency. Reproduced in CI run 26072432957 Phase8b. -func TestTopologyHandler_AddNAT_InvokesFlowBarrier(t *testing.T) { - orig := waitForFlowsHV - var called int - waitForFlowsHV = func() error { called++; return nil } - defer func() { waitForFlowsHV = orig }() - - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - require.NoError(t, mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-natbarrier", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-natbarrier", - "spinifex:cidr": "10.0.0.0/16", - }, - })) - - evt := NATEvent{ - VpcId: "vpc-natbarrier", - ExternalIP: "192.168.0.250", - LogicalIP: "10.0.1.4", - } - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicAddNAT, data, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "add NAT") - - if called != 1 { - t.Errorf("expected waitForFlowsHV to run exactly once after add-nat, got %d", called) - } -} - -// TestTopologyHandler_AddNAT_Idempotent asserts handleAddNAT does NOT -// delete-then-re-add the dnat_and_snat rule when the second invocation -// targets a rule whose state already matches the event (mulga-siv-124). -// Concurrent duplicate publishes for the same EIP would otherwise tear -// down the rule the previous invocation just installed, creating a -// flow-install gap during which the EIP is unreachable. -// -// Witness: the row's UUID. If the handler re-ran delete+add the row would -// be recreated with a fresh UUID; if the handler short-circuits the UUID -// is preserved. We also confirm the second invocation skips the flow -// barrier (waitForFlowsHV) — the flow already exists. -func TestTopologyHandler_AddNAT_Idempotent(t *testing.T) { - orig := waitForFlowsHV - var barrier int - waitForFlowsHV = func() error { barrier++; return nil } - defer func() { waitForFlowsHV = orig }() - - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock, WithNATMode(policy.NATModeCentralized)) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - require.NoError(t, mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-idem", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-idem", - "spinifex:cidr": "10.0.0.0/16", - }, - })) - - evt := NATEvent{ - VpcId: "vpc-idem", - ExternalIP: "192.168.1.244", - LogicalIP: "10.0.1.7", - PortName: "port-eni-idem", - MAC: "02:00:00:cc:cc:cc", - } - data, _ := json.Marshal(evt) - - // First invocation: rule does not exist → installs + waits for flows. - resp, err := nc.Request(TopicAddNAT, data, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "first add-nat") - - router, err := mock.GetLogicalRouter(ctx, "vpc-vpc-idem") - require.NoError(t, err) - if len(router.NAT) != 1 { - t.Fatalf("after first add-nat: expected 1 NAT rule, got %d", len(router.NAT)) - } - firstUUID := router.NAT[0] - if barrier != 1 { - t.Fatalf("after first add-nat: waitForFlowsHV called %d times, want 1", barrier) - } - - // Second invocation with identical event: must short-circuit. The - // existing row must survive untouched (same UUID), and the flow - // barrier must NOT run again. - resp, err = nc.Request(TopicAddNAT, data, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "second add-nat (idempotent)") - - router, err = mock.GetLogicalRouter(ctx, "vpc-vpc-idem") - require.NoError(t, err) - if len(router.NAT) != 1 { - t.Fatalf("after second add-nat: expected 1 NAT rule, got %d", len(router.NAT)) - } - if router.NAT[0] != firstUUID { - t.Errorf("second add-nat recreated the NAT row (uuid %q -> %q); delete-then-add was NOT skipped", firstUUID, router.NAT[0]) - } - if barrier != 1 { - t.Errorf("second add-nat ran flow barrier: waitForFlowsHV called %d times, want 1", barrier) - } - - nat := mock.NATs[router.NAT[0]] - if nat.LogicalIP != evt.LogicalIP { - t.Errorf("NAT logical_ip=%q want %q", nat.LogicalIP, evt.LogicalIP) - } -} - -func TestTopologyHandler_IGWAttach_WithExternalPool(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - pools := []ExternalPoolConfig{ - { - Name: "wan", - RangeStart: "192.168.1.150", - RangeEnd: "192.168.1.250", - Gateway: "192.168.1.1", - PrefixLen: 24, - }, - } - topo := NewTopologyHandler(mock, WithExternalNetwork("pool", pools), WithNATMode(policy.NATModeCentralized)) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Pre-create VPC router - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-extpool", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-extpool", - "spinifex:cidr": "10.0.0.0/16", - }, - }) - - // Attach IGW - evt := types.IGWEvent{InternetGatewayId: "igw-ext1", VpcId: "vpc-extpool"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.igw-attach: %v", err) - } - assertSuccess(t, resp, "attach IGW with external pool") - - // Verify gateway router port uses real external IP, not link-local - router, err := mock.GetLogicalRouter(ctx, "vpc-vpc-extpool") - if err != nil { - t.Fatalf("expected router: %v", err) - } - if len(router.Ports) < 1 { - t.Fatal("expected at least 1 router port") - } - - gwPort, err := mock.GetLogicalRouterPort(ctx, "gw-vpc-extpool") - if err != nil { - t.Fatalf("expected gateway router port: %v", err) - } - // Centralized NAT: LRP gets a WAN-subnet IP auto-derived below the - // per-VM EIP range (192.168.1.150-250), so somewhere in - // 192.168.1.134-149 with /24. Pool range_start - 16 = .134, range_start - 1 = .149. - if len(gwPort.Networks) != 1 { - t.Fatalf("expected 1 LRP network, got %v", gwPort.Networks) - } - gotNet := gwPort.Networks[0] - if !strings.HasPrefix(gotNet, "192.168.1.1") || !strings.HasSuffix(gotNet, "/24") { - t.Errorf("expected gw LRP network in 192.168.1.134-149/24, got %s", gotNet) - } - if gwPort.ExternalIDs["spinifex:gateway_ip"] == "" { - t.Errorf("expected spinifex:gateway_ip on LRP, got empty") - } - - // Verify NO blanket SNAT rule (AWS parity) - if len(router.NAT) != 0 { - t.Errorf("expected 0 NAT rules (no blanket SNAT), got %d", len(router.NAT)) - } - - // Verify default route points to WAN gateway, not link-local - if len(router.StaticRoutes) != 1 { - t.Fatalf("expected 1 static route, got %d", len(router.StaticRoutes)) - } - route := mock.StaticRoutes[router.StaticRoutes[0]] - if route == nil { - t.Fatal("static route not found in mock") - } - if route.Nexthop != "192.168.1.1" { - t.Errorf("expected nexthop 192.168.1.1, got %s", route.Nexthop) - } - gwPortName := "gw-vpc-extpool" - if route.OutputPort == nil || *route.OutputPort != gwPortName { - t.Errorf("expected OutputPort=%s, got %v", gwPortName, route.OutputPort) - } -} - -func TestTopologyHandler_IGWAttach_PoolWithGatewayIP(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - pools := []ExternalPoolConfig{ - { - Name: "dc1", - RangeStart: "203.0.113.8", - RangeEnd: "203.0.113.14", - Gateway: "203.0.113.1", - GatewayIP: "203.0.113.8", // Explicit gateway IP - PrefixLen: 28, - }, - } - topo := NewTopologyHandler(mock, WithExternalNetwork("pool", pools), WithNATMode(policy.NATModeCentralized)) - subs, _ := topo.Subscribe(nc) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-gwip", - ExternalIDs: map[string]string{"spinifex:vpc_id": "vpc-gwip", "spinifex:cidr": "10.0.0.0/16"}, - }) - - evt := types.IGWEvent{InternetGatewayId: "igw-gwip", VpcId: "vpc-gwip"} - data, _ := json.Marshal(evt) - resp, _ := nc.Request(TopicIGWAttach, data, 5_000_000_000) - assertSuccess(t, resp, "attach IGW with explicit gateway IP") - - router, _ := mock.GetLogicalRouter(ctx, "vpc-vpc-gwip") - gwPort, _ := mock.GetLogicalRouterPort(ctx, "gw-vpc-gwip") - // /28 with full pool .2-.14 + gateway .1 leaves no room in-subnet — - // auto-derive shifts below RangeStart and clamps; whatever the result, - // it must be a /28 IP carrying spinifex:gateway_ip. - if !strings.HasSuffix(gwPort.Networks[0], "/28") { - t.Errorf("expected /28 LRP network, got %s", gwPort.Networks[0]) - } - if gwPort.ExternalIDs["spinifex:gateway_ip"] == "" { - t.Errorf("expected spinifex:gateway_ip on LRP, got empty") - } - // No blanket SNAT rule (AWS parity) - if len(router.NAT) != 0 { - t.Errorf("expected 0 NAT rules (no blanket SNAT), got %d", len(router.NAT)) - } - route := mock.StaticRoutes[router.StaticRoutes[0]] - if route.Nexthop != "203.0.113.1" { - t.Errorf("expected nexthop 203.0.113.1, got %s", route.Nexthop) - } - gwPortName := "gw-vpc-gwip" - if route.OutputPort == nil || *route.OutputPort != gwPortName { - t.Errorf("expected OutputPort=%s, got %v", gwPortName, route.OutputPort) - } -} - -// TestTopologyHandler_IGWAttach_MultiVPC_NoIPCollision is the regression -// guard for mulga-siv-26: three VPCs sharing one external pool must produce -// three gateway LRPs with distinct WAN-subnet IPs, distinct MACs and distinct -// default routes. Pre-fix every LRP claimed pool.RangeStart and the VPCs -// ARP-collided on br-ext. -func TestTopologyHandler_IGWAttach_MultiVPC_NoIPCollision(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - pools := []ExternalPoolConfig{ - { - Name: "ci-multi-2", - RangeStart: "192.168.0.160", - RangeEnd: "192.168.0.169", - Gateway: "192.168.0.1", - PrefixLen: 24, - }, - } - topo := NewTopologyHandler(mock, WithExternalNetwork("pool", pools), WithNATMode(policy.NATModeCentralized)) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - vpcIds := []string{"vpc-multi-a", "vpc-multi-b", "vpc-multi-c"} - for _, vpcId := range vpcIds { - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-" + vpcId, - ExternalIDs: map[string]string{ - "spinifex:vpc_id": vpcId, - "spinifex:cidr": "10.0.0.0/16", - }, - }) - evt := types.IGWEvent{InternetGatewayId: "igw-" + vpcId, VpcId: vpcId} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.igw-attach %s: %v", vpcId, err) - } - assertSuccess(t, resp, "attach IGW "+vpcId) - } - - macs := make(map[string]string, len(vpcIds)) - ips := make(map[string]string, len(vpcIds)) - for _, vpcId := range vpcIds { - gwPortName := "gw-" + vpcId - gwPort, err := mock.GetLogicalRouterPort(ctx, gwPortName) - if err != nil { - t.Fatalf("get %s: %v", gwPortName, err) - } - if len(gwPort.Networks) != 1 { - t.Fatalf("%s: expected 1 network, got %v", gwPortName, gwPort.Networks) - } - ip, cidr, _ := strings.Cut(gwPort.Networks[0], "/") - if cidr != "24" { - t.Errorf("%s: expected /24, got /%s", gwPortName, cidr) - } - if !strings.HasPrefix(ip, "192.168.0.") { - t.Errorf("%s: expected 192.168.0.x, got %s", gwPortName, ip) - } - gwIP := gwPort.ExternalIDs["spinifex:gateway_ip"] - if gwIP != ip { - t.Errorf("%s: external_ids gateway_ip=%q, expected %q", gwPortName, gwIP, ip) - } - if other, dup := ips[ip]; dup { - t.Errorf("IP collision: %s and %s share %s", gwPortName, other, ip) - } - ips[ip] = gwPortName - if gwPort.MAC == "" { - t.Errorf("%s: empty MAC", gwPortName) - } - if other, dup := macs[gwPort.MAC]; dup { - t.Errorf("MAC collision: %s and %s share %s", gwPortName, other, gwPort.MAC) - } - macs[gwPort.MAC] = gwPortName - - router, err := mock.GetLogicalRouter(ctx, "vpc-"+vpcId) - if err != nil { - t.Fatalf("get router for %s: %v", vpcId, err) - } - if len(router.StaticRoutes) != 1 { - t.Fatalf("%s: expected 1 static route, got %d", vpcId, len(router.StaticRoutes)) - } - route := mock.StaticRoutes[router.StaticRoutes[0]] - if route.Nexthop != "192.168.0.1" { - t.Errorf("%s: expected nexthop 192.168.0.1, got %s", vpcId, route.Nexthop) - } - if route.OutputPort == nil || *route.OutputPort != gwPortName { - t.Errorf("%s: expected OutputPort=%s, got %v", vpcId, gwPortName, route.OutputPort) - } - } - if len(macs) != len(vpcIds) { - t.Errorf("expected %d distinct MACs, got %d", len(vpcIds), len(macs)) - } - if len(ips) != len(vpcIds) { - t.Errorf("expected %d distinct IPs, got %d", len(vpcIds), len(ips)) - } -} - -// TestTopologyHandler_IGWAttach_CentralizedNATAllocatesGwLrpIP guards -// mulga-siv-36: in centralized NAT (veth) the gateway LRP must -// hold a WAN-subnet IP from pool.GwLrpRange so it can ARP the upstream -// nexthop. Three VPCs sharing one pool must each get a distinct IP and -// persist it on external_ids:spinifex:gateway_ip. -func TestTopologyHandler_IGWAttach_CentralizedNATAllocatesGwLrpIP(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - pools := []ExternalPoolConfig{ - { - Name: "lan", - RangeStart: "192.168.3.100", - RangeEnd: "192.168.3.150", - Gateway: "192.168.3.1", - PrefixLen: 24, - GwLrpRangeStart: "192.168.3.20", - GwLrpRangeEnd: "192.168.3.29", - }, - } - topo := NewTopologyHandler(mock, - WithExternalNetwork("pool", pools), - WithNATMode(policy.NATModeCentralized), - ) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - vpcIds := []string{"vpc-cn-a", "vpc-cn-b", "vpc-cn-c"} - for _, vpcId := range vpcIds { - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-" + vpcId, - ExternalIDs: map[string]string{ - "spinifex:vpc_id": vpcId, - "spinifex:cidr": "10.0.0.0/16", - }, - }) - evt := types.IGWEvent{InternetGatewayId: "igw-" + vpcId, VpcId: vpcId} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.igw-attach %s: %v", vpcId, err) - } - assertSuccess(t, resp, "attach IGW "+vpcId) - } - - seen := make(map[string]string, len(vpcIds)) - for _, vpcId := range vpcIds { - gwPortName := "gw-" + vpcId - gwPort, err := mock.GetLogicalRouterPort(ctx, gwPortName) - if err != nil { - t.Fatalf("get %s: %v", gwPortName, err) - } - if len(gwPort.Networks) != 1 { - t.Fatalf("%s: expected 1 Network, got %v", gwPortName, gwPort.Networks) - } - net := gwPort.Networks[0] - if !strings.HasPrefix(net, "192.168.3.") || !strings.HasSuffix(net, "/24") { - t.Errorf("%s: expected 192.168.3.X/24 from gw_lrp_range, got %s", gwPortName, net) - } - gwIP := gwPort.ExternalIDs["spinifex:gateway_ip"] - if gwIP == "" { - t.Errorf("%s: missing spinifex:gateway_ip external_id", gwPortName) - } - if other, dup := seen[gwIP]; dup { - t.Errorf("gw IP collision: %s and %s share %s", gwPortName, other, gwIP) - } - seen[gwIP] = gwPortName - - // Idempotent: re-attach must not change the assignment. - evt := types.IGWEvent{InternetGatewayId: "igw-" + vpcId, VpcId: vpcId} - data, _ := json.Marshal(evt) - _, _ = nc.Request(TopicIGWAttach, data, 5_000_000_000) - gwPort2, _ := mock.GetLogicalRouterPort(ctx, gwPortName) - if gwPort2.ExternalIDs["spinifex:gateway_ip"] != gwIP { - t.Errorf("%s: re-attach changed gateway_ip from %s to %s", - gwPortName, gwIP, gwPort2.ExternalIDs["spinifex:gateway_ip"]) - } - } -} - -func TestTopologyHandler_FindExternalPool(t *testing.T) { - pools := []ExternalPoolConfig{ - {Name: "az-a", Region: "us-east-1", AZ: "us-east-1a", RangeStart: "1.1.1.1"}, - {Name: "region", Region: "us-east-1", RangeStart: "2.2.2.2"}, - {Name: "global", RangeStart: "3.3.3.3"}, - } - topo := NewTopologyHandler(nil, WithExternalNetwork("pool", pools)) - - // AZ match - p := topo.findExternalPool("us-east-1", "us-east-1a") - if p == nil || p.Name != "az-a" { - t.Errorf("expected az-a pool, got %v", p) - } - - // Region fallback (no matching AZ) - p = topo.findExternalPool("us-east-1", "us-east-1b") - if p == nil || p.Name != "region" { - t.Errorf("expected region pool, got %v", p) - } - - // Global fallback - p = topo.findExternalPool("eu-west-1", "eu-west-1a") - if p == nil || p.Name != "global" { - t.Errorf("expected global pool, got %v", p) - } - - // No pools - topo2 := NewTopologyHandler(nil) - p = topo2.findExternalPool("us-east-1", "us-east-1a") - if p != nil { - t.Errorf("expected nil pool, got %v", p) - } -} - -func TestTopologyHandler_IGWDetach(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Pre-create VPC router - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-igw2", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-igw2", - "spinifex:cidr": "10.0.0.0/16", - }, - }) - - // Attach IGW first - attachEvt := types.IGWEvent{InternetGatewayId: "igw-test2", VpcId: "vpc-igw2"} - data, _ := json.Marshal(attachEvt) - resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.igw-attach: %v", err) - } - assertSuccess(t, resp, "attach IGW") - - // Verify resources exist before detach - _, err = mock.GetLogicalSwitch(ctx, "ext-vpc-igw2") - if err != nil { - t.Fatal("expected external switch before detach") - } - - // Detach IGW - detachEvt := types.IGWEvent{InternetGatewayId: "igw-test2", VpcId: "vpc-igw2"} - data, _ = json.Marshal(detachEvt) - resp, err = nc.Request(TopicIGWDetach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.igw-detach: %v", err) - } - assertSuccess(t, resp, "detach IGW") - - // Verify external switch deleted - _, err = mock.GetLogicalSwitch(ctx, "ext-vpc-igw2") - if err == nil { - t.Error("expected external switch to be deleted") - } - - // Verify router still exists but NAT and routes are cleaned up - router, err := mock.GetLogicalRouter(ctx, "vpc-vpc-igw2") - if err != nil { - t.Fatal("expected VPC router to still exist") - } - if len(router.NAT) != 0 { - t.Errorf("expected 0 NAT rules after detach, got %d", len(router.NAT)) - } - if len(router.StaticRoutes) != 0 { - t.Errorf("expected 0 static routes after detach, got %d", len(router.StaticRoutes)) - } - if len(router.Ports) != 0 { - t.Errorf("expected 0 router ports after detach, got %d", len(router.Ports)) - } -} - -func TestTopologyHandler_IGWAttachDetachLifecycle(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // 1. Create VPC - vpcEvt := VPCEvent{VpcId: "vpc-igwlc", CidrBlock: "10.0.0.0/16", VNI: 400} - data, _ := json.Marshal(vpcEvt) - resp, _ := nc.Request(TopicVPCCreate, data, 5_000_000_000) - assertSuccess(t, resp, "create VPC") - - // 2. Create subnet - subEvt := SubnetEvent{SubnetId: "subnet-igwlc", VpcId: "vpc-igwlc", CidrBlock: "10.0.1.0/24"} - data, _ = json.Marshal(subEvt) - resp, _ = nc.Request(TopicSubnetCreate, data, 5_000_000_000) - assertSuccess(t, resp, "create subnet") - - // 3. Attach IGW - igwEvt := types.IGWEvent{InternetGatewayId: "igw-lc1", VpcId: "vpc-igwlc"} - data, _ = json.Marshal(igwEvt) - resp, _ = nc.Request(TopicIGWAttach, data, 5_000_000_000) - assertSuccess(t, resp, "attach IGW") - - // Verify full topology: 2 switches (subnet + external), 1 router with ports+NAT+routes - switches, _ := mock.ListLogicalSwitches(ctx) - if len(switches) != 2 { - t.Errorf("expected 2 switches (subnet + external), got %d", len(switches)) - } - - router, _ := mock.GetLogicalRouter(ctx, "vpc-vpc-igwlc") - if len(router.Ports) != 2 { - t.Errorf("expected 2 router ports (subnet + gateway), got %d", len(router.Ports)) - } - - // 4. Detach IGW - data, _ = json.Marshal(igwEvt) - resp, _ = nc.Request(TopicIGWDetach, data, 5_000_000_000) - assertSuccess(t, resp, "detach IGW") - - // Only subnet switch should remain - switches, _ = mock.ListLogicalSwitches(ctx) - if len(switches) != 1 { - t.Errorf("expected 1 switch after IGW detach, got %d", len(switches)) - } - - // Router should still have subnet port but no gateway port - router, _ = mock.GetLogicalRouter(ctx, "vpc-vpc-igwlc") - if len(router.Ports) != 1 { - t.Errorf("expected 1 router port after IGW detach, got %d", len(router.Ports)) - } - - // 5. Delete subnet and VPC - data, _ = json.Marshal(subEvt) - resp, _ = nc.Request(TopicSubnetDelete, data, 5_000_000_000) - assertSuccess(t, resp, "delete subnet") - - data, _ = json.Marshal(VPCEvent{VpcId: "vpc-igwlc"}) - resp, _ = nc.Request(TopicVPCDelete, data, 5_000_000_000) - assertSuccess(t, resp, "delete VPC") - - // Everything should be gone - routers, _ := mock.ListLogicalRouters(ctx) - if len(routers) != 0 { - t.Errorf("expected 0 routers, got %d", len(routers)) - } - switches, _ = mock.ListLogicalSwitches(ctx) - if len(switches) != 0 { - t.Errorf("expected 0 switches, got %d", len(switches)) - } -} - -func TestTopologyHandler_IGWNilOVN(t *testing.T) { - _, nc := startTestNATS(t) - - topo := NewTopologyHandler(nil) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Should fail gracefully when OVN is nil - evt := types.IGWEvent{InternetGatewayId: "igw-nil", VpcId: "vpc-nil"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - - var result struct { - Success bool `json:"success"` - Error string `json:"error,omitempty"` - } - _ = json.Unmarshal(resp.Data, &result) - if result.Success { - t.Error("expected failure when OVN is nil") - } - - // Detach should also fail gracefully - resp, err = nc.Request(TopicIGWDetach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - _ = json.Unmarshal(resp.Data, &result) - if result.Success { - t.Error("expected detach failure when OVN is nil") - } -} - -// --- Error path tests (Phase 3) --- - -func assertFailure(t *testing.T, msg *nats.Msg, label string) { - t.Helper() - var result struct { - Success bool `json:"success"` - Error string `json:"error,omitempty"` - } - if err := json.Unmarshal(msg.Data, &result); err != nil { - t.Fatalf("%s: unmarshal: %v", label, err) - } - if result.Success { - t.Fatalf("%s: expected failure, got success", label) - } -} - -func TestTopologyHandler_BadJSON(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - badData := []byte("{invalid json") - - // Each topic should return an error response on bad JSON - topics := []string{ - TopicVPCCreate, TopicVPCDelete, - TopicSubnetCreate, TopicSubnetDelete, - TopicCreatePort, TopicDeletePort, TopicUpdatePortSGs, - TopicIGWAttach, TopicIGWDetach, - } - for _, topic := range topics { - resp, err := nc.Request(topic, badData, 5_000_000_000) - if err != nil { - t.Fatalf("request %s: %v", topic, err) - } - assertFailure(t, resp, "bad JSON on "+topic) - } -} - -func TestTopologyHandler_VPCCreate_DuplicateRouter(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Pre-create router to trigger idempotent path - _ = mock.CreateLogicalRouter(ctx, nbdbLogicalRouter("vpc-vpc-dup", "vpc-dup")) - - evt := VPCEvent{VpcId: "vpc-dup", CidrBlock: "10.0.0.0/16", VNI: 100} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicVPCCreate, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertSuccess(t, resp, "idempotent VPC create") - - // Verify exactly 1 router (no duplicate created) - routers, _ := mock.ListLogicalRouters(ctx) - if len(routers) != 1 { - t.Errorf("expected 1 router (idempotent), got %d", len(routers)) - } -} - -func TestTopologyHandler_VPCDelete_RouterNotFound(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Delete VPC that doesn't exist — DeleteLogicalRouter will fail - evt := VPCEvent{VpcId: "vpc-nonexistent"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicVPCDelete, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertFailure(t, resp, "delete nonexistent VPC") -} - -func TestTopologyHandler_SubnetCreate_SwitchAlreadyExists(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Pre-create switch to trigger idempotent path - _ = mock.CreateLogicalSwitch(ctx, nbdbLogicalSwitch("subnet-subnet-exists", "subnet-exists", "vpc-exists")) - - evt := SubnetEvent{SubnetId: "subnet-exists", VpcId: "vpc-exists", CidrBlock: "10.0.1.0/24"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicSubnetCreate, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertSuccess(t, resp, "idempotent subnet create") - - // Verify exactly 1 switch (no duplicate) - switches, _ := mock.ListLogicalSwitches(ctx) - if len(switches) != 1 { - t.Errorf("expected 1 switch (idempotent), got %d", len(switches)) - } -} - -func TestTopologyHandler_SubnetCreate_InvalidCIDR(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - evt := SubnetEvent{SubnetId: "subnet-badcidr", VpcId: "vpc-badcidr", CidrBlock: "not-a-cidr"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicSubnetCreate, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertFailure(t, resp, "subnet create with invalid CIDR") -} - -func TestTopologyHandler_SubnetDelete_SwitchNotFound(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Delete subnet where nothing exists — switch delete fails at step 4 - evt := SubnetEvent{SubnetId: "subnet-ghost", VpcId: "vpc-ghost", CidrBlock: "10.0.99.0/24"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicSubnetDelete, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertFailure(t, resp, "delete nonexistent subnet") -} - -func TestTopologyHandler_CreatePort_SwitchNotFound(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Create port on non-existent switch - evt := PortEvent{ - NetworkInterfaceId: "eni-orphan", - SubnetId: "subnet-missing", - VpcId: "vpc-missing", - PrivateIpAddress: "10.0.1.4", - MacAddress: "02:00:00:00:00:01", - } - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicCreatePort, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertFailure(t, resp, "create port on missing switch") -} - -func TestTopologyHandler_CreatePort_Idempotent(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Setup: create switch and port directly - _ = mock.CreateLogicalSwitch(ctx, nbdbLogicalSwitch("subnet-subnet-idem", "subnet-idem", "vpc-idem")) - _ = mock.CreateLogicalSwitchPort(ctx, "subnet-subnet-idem", &nbdb.LogicalSwitchPort{ - Name: "port-eni-idem", - Addresses: []string{"02:00:00:11:22:33 10.0.1.4"}, - ExternalIDs: map[string]string{ - "spinifex:eni_id": "eni-idem", - }, - }) - - // Send create for same port — should succeed (idempotent skip) - evt := PortEvent{ - NetworkInterfaceId: "eni-idem", - SubnetId: "subnet-idem", - VpcId: "vpc-idem", - PrivateIpAddress: "10.0.1.4", - MacAddress: "02:00:00:11:22:33", - } - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicCreatePort, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertSuccess(t, resp, "idempotent port create") -} - -func TestTopologyHandler_DeletePort_PortNotFound(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Delete port that doesn't exist - evt := PortEvent{ - NetworkInterfaceId: "eni-ghost", - SubnetId: "subnet-ghost", - VpcId: "vpc-ghost", - PrivateIpAddress: "10.0.1.99", - MacAddress: "02:00:00:ff:ff:ff", - } - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicDeletePort, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertFailure(t, resp, "delete nonexistent port") -} - -func TestTopologyHandler_IGWAttach_RouterNotFound(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Attach IGW without VPC router — CreateLogicalRouterPort fails - evt := types.IGWEvent{InternetGatewayId: "igw-orphan", VpcId: "vpc-norouter"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertFailure(t, resp, "IGW attach without router") - - // Verify external switch was cleaned up - _, err = mock.GetLogicalSwitch(ctx, "ext-vpc-norouter") - if err == nil { - t.Error("expected external switch to be cleaned up after router port failure") - } -} - -func TestTopologyHandler_IGWAttach_Idempotent(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Pre-create external switch to trigger idempotent path - _ = mock.CreateLogicalSwitch(ctx, &nbdb.LogicalSwitch{ - Name: "ext-vpc-igw-idem", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-igw-idem", - "spinifex:role": "external", - }, - }) - - evt := types.IGWEvent{InternetGatewayId: "igw-idem", VpcId: "vpc-igw-idem"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertSuccess(t, resp, "idempotent IGW attach") -} - -func TestTopologyHandler_IGWDetach_PartialCleanup(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Only create the external switch (not the full IGW topology). - // All intermediate deletes will warn but switch delete should succeed. - _ = mock.CreateLogicalSwitch(ctx, &nbdb.LogicalSwitch{ - Name: "ext-vpc-partial", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-partial", - "spinifex:role": "external", - }, - }) - // Create router so NAT cleanup path is exercised (but NAT won't be found) - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-partial", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-partial", - "spinifex:cidr": "10.0.0.0/16", - }, - }) - - evt := types.IGWEvent{InternetGatewayId: "igw-partial", VpcId: "vpc-partial"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWDetach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - // Should succeed — intermediate deletes warn but final switch delete succeeds - assertSuccess(t, resp, "partial IGW detach") - - // Verify external switch was deleted - _, err = mock.GetLogicalSwitch(ctx, "ext-vpc-partial") - if err == nil { - t.Error("expected external switch to be deleted") - } -} - -func TestTopologyHandler_IGWDetach_NoRouter(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Create external switch but no router — exercises the GetLogicalRouter warn path - _ = mock.CreateLogicalSwitch(ctx, &nbdb.LogicalSwitch{ - Name: "ext-vpc-nortr", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-nortr", - "spinifex:role": "external", - }, - }) - - evt := types.IGWEvent{InternetGatewayId: "igw-nortr", VpcId: "vpc-nortr"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWDetach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - // Should succeed — router not found is a warn path, switch delete succeeds - assertSuccess(t, resp, "IGW detach without router") - - _, err = mock.GetLogicalSwitch(ctx, "ext-vpc-nortr") - if err == nil { - t.Error("expected external switch to be deleted") - } -} - -func TestTopologyHandler_IGWDetach_SwitchNotFound(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // No external switch — final DeleteLogicalSwitch fails - evt := types.IGWEvent{InternetGatewayId: "igw-nosw", VpcId: "vpc-nosw"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWDetach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertFailure(t, resp, "IGW detach without external switch") -} - -func TestTopologyHandler_VPCDeleteCascade_WithPorts(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Create VPC router + subnet switch with ports (simulating pre-existing state) - _ = mock.CreateLogicalRouter(ctx, nbdbLogicalRouter("vpc-vpc-casp", "vpc-casp")) - _ = mock.CreateLogicalSwitch(ctx, nbdbLogicalSwitch("subnet-sub-casp", "sub-casp", "vpc-casp")) - _ = mock.CreateLogicalSwitchPort(ctx, "subnet-sub-casp", &nbdb.LogicalSwitchPort{ - Name: "port-eni-casp1", - Addresses: []string{"02:00:00:11:22:33 10.0.1.4"}, - ExternalIDs: map[string]string{ - "spinifex:eni_id": "eni-casp1", - "spinifex:subnet_id": "sub-casp", - "spinifex:vpc_id": "vpc-casp", - }, - }) - _ = mock.CreateLogicalSwitchPort(ctx, "subnet-sub-casp", &nbdb.LogicalSwitchPort{ - Name: "port-eni-casp2", - Addresses: []string{"02:00:00:44:55:66 10.0.1.5"}, - ExternalIDs: map[string]string{ - "spinifex:eni_id": "eni-casp2", - "spinifex:subnet_id": "sub-casp", - "spinifex:vpc_id": "vpc-casp", - }, - }) - _, _ = mock.CreateDHCPOptions(ctx, nbdbDHCPOptions("10.0.1.0/24", "sub-casp", "vpc-casp")) - - // Verify switch has 2 ports before delete - ls, err := mock.GetLogicalSwitch(ctx, "subnet-sub-casp") - if err != nil { - t.Fatalf("expected switch: %v", err) - } - if len(ls.Ports) != 2 { - t.Errorf("expected 2 ports before cascade, got %d", len(ls.Ports)) - } - - // Delete VPC should cascade-delete switches (with ports) and DHCP - evt := VPCEvent{VpcId: "vpc-casp"} - data, _ := json.Marshal(evt) - resp, _ := nc.Request(TopicVPCDelete, data, 5_000_000_000) - assertSuccess(t, resp, "cascade delete VPC with ports") - - // Verify switch is gone - _, err = mock.GetLogicalSwitch(ctx, "subnet-sub-casp") - if err == nil { - t.Error("expected switch to be deleted after cascade") - } - // Verify DHCP options gone - dhcpList, _ := mock.ListDHCPOptions(ctx) - if len(dhcpList) != 0 { - t.Errorf("expected 0 DHCP options after cascade, got %d", len(dhcpList)) - } - // Verify router gone - _, err = mock.GetLogicalRouter(ctx, "vpc-vpc-casp") - if err == nil { - t.Error("expected router to be deleted after cascade") - } -} - -func TestTopologyHandler_SubnetDelete_NilOVN(t *testing.T) { - _, nc := startTestNATS(t) - - topo := NewTopologyHandler(nil) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - evt := SubnetEvent{SubnetId: "subnet-nil", VpcId: "vpc-nil", CidrBlock: "10.0.0.0/24"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicSubnetDelete, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - - var result struct { - Success bool `json:"success"` - Error string `json:"error,omitempty"` - } - _ = json.Unmarshal(resp.Data, &result) - if result.Success { - t.Error("expected failure when OVN is nil") - } -} - -// --- Test helpers --- - -func assertSuccess(t *testing.T, msg *nats.Msg, label string) { - t.Helper() - var result struct { - Success bool `json:"success"` - Error string `json:"error,omitempty"` - } - if err := json.Unmarshal(msg.Data, &result); err != nil { - t.Fatalf("%s: unmarshal: %v", label, err) - } - if !result.Success { - t.Fatalf("%s: failed: %s", label, result.Error) - } -} - -// nbdb helper factories for tests - -func nbdbLogicalRouter(name, vpcId string) *nbdb.LogicalRouter { - return &nbdb.LogicalRouter{ - Name: name, - ExternalIDs: map[string]string{ - "spinifex:vpc_id": vpcId, - }, - } -} - -func nbdbLogicalSwitch(name, subnetId, vpcId string) *nbdb.LogicalSwitch { - return &nbdb.LogicalSwitch{ - Name: name, - ExternalIDs: map[string]string{ - "spinifex:subnet_id": subnetId, - "spinifex:vpc_id": vpcId, - }, - } -} - -func nbdbLogicalRouterPort(name, subnetId, vpcId string) *nbdb.LogicalRouterPort { - return &nbdb.LogicalRouterPort{ - Name: name, - MAC: "02:00:00:aa:bb:cc", - Networks: []string{"10.0.2.1/24"}, - ExternalIDs: map[string]string{ - "spinifex:subnet_id": subnetId, - "spinifex:vpc_id": vpcId, - }, - } -} - -func nbdbLogicalSwitchPortRouter(name, routerPort, subnetId, vpcId string) *nbdb.LogicalSwitchPort { - return &nbdb.LogicalSwitchPort{ - Name: name, - Type: "router", - Addresses: []string{"router"}, - Options: map[string]string{ - "router-port": routerPort, - }, - ExternalIDs: map[string]string{ - "spinifex:subnet_id": subnetId, - "spinifex:vpc_id": vpcId, - }, - } -} - -func nbdbDHCPOptions(cidr, subnetId, vpcId string) *nbdb.DHCPOptions { - return &nbdb.DHCPOptions{ - CIDR: cidr, - Options: map[string]string{ - "router": "10.0.2.1", - "lease_time": "3600", - }, - ExternalIDs: map[string]string{ - "spinifex:subnet_id": subnetId, - "spinifex:vpc_id": vpcId, - }, - } -} - -// --- Bridge mode tests --- - -func TestTopologyHandler_IGWAttach_DirectBridge_NoNatAddresses(t *testing.T) { - // In direct bridge mode, the localnet port should NOT have nat-addresses=router - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock, WithNATMode(policy.NATModeDistributed)) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Pre-create VPC router - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-direct1", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-direct1", - "spinifex:cidr": "10.0.0.0/16", - }, - }) - - // Attach IGW - evt := types.IGWEvent{InternetGatewayId: "igw-d1", VpcId: "vpc-direct1"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.igw-attach: %v", err) - } - assertSuccess(t, resp, "attach IGW direct bridge") - - // Verify localnet port does NOT have nat-addresses - port, err := mock.GetLogicalSwitchPort(ctx, "ext-port-vpc-direct1") - if err != nil { - t.Fatalf("expected localnet port: %v", err) - } - if _, ok := port.Options["nat-addresses"]; ok { - t.Errorf("direct bridge mode should NOT have nat-addresses option, got %q", port.Options["nat-addresses"]) - } -} - -func TestTopologyHandler_AddNAT_DirectBridge_DistributedNAT(t *testing.T) { - // In direct bridge mode, DNAT rules should have ExternalMAC and LogicalPort set - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock, WithNATMode(policy.NATModeDistributed)) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Pre-create VPC router - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-dnat1", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-dnat1", - "spinifex:cidr": "10.0.0.0/16", - }, - }) - - // Add NAT with MAC and port (simulating a VM with public IP) - evt := NATEvent{ - VpcId: "vpc-dnat1", - ExternalIP: "192.168.1.201", - LogicalIP: "10.0.1.5", - PortName: "port-eni-1234", - MAC: "02:00:00:ab:cd:ef", - } - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicAddNAT, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.add-nat: %v", err) - } - assertSuccess(t, resp, "add NAT direct bridge") - - // Verify the NAT rule has ExternalMAC and LogicalPort (distributed NAT) - router, err := mock.GetLogicalRouter(ctx, "vpc-vpc-dnat1") - if err != nil { - t.Fatalf("expected router: %v", err) - } - if len(router.NAT) != 1 { - t.Fatalf("expected 1 NAT rule, got %d", len(router.NAT)) - } - nat := mock.NATs[router.NAT[0]] - if nat.ExternalMAC == nil || *nat.ExternalMAC != "02:00:00:ab:cd:ef" { - t.Errorf("expected ExternalMAC=02:00:00:ab:cd:ef for distributed NAT, got %v", nat.ExternalMAC) - } - if nat.LogicalPort == nil || *nat.LogicalPort != "port-eni-1234" { - t.Errorf("expected LogicalPort=port-eni-1234 for distributed NAT, got %v", nat.LogicalPort) - } -} - -func TestTopologyHandler_IGWAttach_VethMode_HasNatAddresses(t *testing.T) { - // In veth mode, the localnet port SHOULD have nat-addresses=router (centralized NAT) - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock, WithNATMode(policy.NATModeCentralized)) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-veth1", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-veth1", - "spinifex:cidr": "10.0.0.0/16", - }, - }) - - evt := types.IGWEvent{InternetGatewayId: "igw-veth1", VpcId: "vpc-veth1"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.igw-attach: %v", err) - } - assertSuccess(t, resp, "attach IGW veth") - - port, err := mock.GetLogicalSwitchPort(ctx, "ext-port-vpc-veth1") - if err != nil { - t.Fatalf("expected localnet port: %v", err) - } - if port.Options["nat-addresses"] != "router" { - t.Errorf("veth mode should have nat-addresses=router, got %q", port.Options["nat-addresses"]) - } -} - -func TestTopologyHandler_AddNAT_VethMode_CentralizedNAT(t *testing.T) { - // In veth mode, DNAT rules should NOT have ExternalMAC/LogicalPort (centralized NAT) - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock, WithNATMode(policy.NATModeCentralized)) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-vnat1", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-vnat1", - "spinifex:cidr": "10.0.0.0/16", - }, - }) - - evt := NATEvent{ - VpcId: "vpc-vnat1", - ExternalIP: "192.168.1.201", - LogicalIP: "10.0.1.5", - PortName: "port-eni-veth1", - MAC: "02:00:00:aa:bb:cc", - } - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicAddNAT, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.add-nat: %v", err) - } - assertSuccess(t, resp, "add NAT veth") - - router, err := mock.GetLogicalRouter(ctx, "vpc-vpc-vnat1") - if err != nil { - t.Fatalf("expected router: %v", err) - } - if len(router.NAT) != 1 { - t.Fatalf("expected 1 NAT rule, got %d", len(router.NAT)) - } - nat := mock.NATs[router.NAT[0]] - if nat.ExternalMAC != nil { - t.Errorf("veth mode should NOT have ExternalMAC, got %v", *nat.ExternalMAC) - } - if nat.LogicalPort != nil { - t.Errorf("veth mode should NOT have LogicalPort, got %v", *nat.LogicalPort) - } -} - -func TestTopologyHandler_AddNAT_CleansStaleRulesFromOtherVPCs(t *testing.T) { - // When a public IP is reused across VPCs (e.g. instance terminated in the - // default VPC, IP returned to pool, then LB allocates it in a new VPC), - // the fire-and-forget vpc.delete-nat for the old instance may not have been - // processed. handleAddNAT must clean up stale NAT rules from ALL routers, - // not just the target VPC's router. - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock, WithNATMode(policy.NATModeCentralized)) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Create two VPC routers (simulating default VPC and a new test VPC). - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-default", - ExternalIDs: map[string]string{"spinifex:vpc_id": "vpc-default", "spinifex:cidr": "10.0.0.0/16"}, - }) - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-new", - ExternalIDs: map[string]string{"spinifex:vpc_id": "vpc-new", "spinifex:cidr": "10.200.0.0/16"}, - }) - - // Simulate: instance in default VPC got public IP 192.168.1.243. - // Add NAT rule to default VPC's router (as if vpc.add-nat was processed). - oldEvt := NATEvent{ - VpcId: "vpc-default", - ExternalIP: "192.168.1.243", - LogicalIP: "10.0.1.5", - PortName: "port-eni-old", - MAC: "02:00:00:aa:aa:aa", - } - data, _ := json.Marshal(oldEvt) - resp, err := nc.Request(TopicAddNAT, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.add-nat (old): %v", err) - } - assertSuccess(t, resp, "add old NAT") - - // Verify the stale NAT exists on the default VPC's router. - defaultRouter, _ := mock.GetLogicalRouter(ctx, "vpc-vpc-default") - if len(defaultRouter.NAT) != 1 { - t.Fatalf("expected 1 NAT on default router, got %d", len(defaultRouter.NAT)) - } - - // Simulate: instance terminated, IP returned to pool, and a new LB in - // the new VPC re-allocates the same IP. vpc.delete-nat was NOT processed - // (fire-and-forget race). Now vpc.add-nat fires for the new VPC. - newEvt := NATEvent{ - VpcId: "vpc-new", - ExternalIP: "192.168.1.243", - LogicalIP: "10.200.1.10", - PortName: "port-eni-new", - MAC: "02:00:00:bb:bb:bb", - } - data, _ = json.Marshal(newEvt) - resp, err = nc.Request(TopicAddNAT, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.add-nat (new): %v", err) - } - assertSuccess(t, resp, "add new NAT (should clean stale)") - - // The stale NAT rule on the default VPC's router must be gone. - defaultRouter, _ = mock.GetLogicalRouter(ctx, "vpc-vpc-default") - if len(defaultRouter.NAT) != 0 { - t.Errorf("stale NAT rule was NOT cleaned from default VPC's router; got %d NAT rules", len(defaultRouter.NAT)) - } - - // The new VPC's router must have exactly 1 NAT rule with the new logical IP. - newRouter, _ := mock.GetLogicalRouter(ctx, "vpc-vpc-new") - if len(newRouter.NAT) != 1 { - t.Fatalf("expected 1 NAT on new router, got %d", len(newRouter.NAT)) - } - newNAT := mock.NATs[newRouter.NAT[0]] - if newNAT.LogicalIP != "10.200.1.10" { - t.Errorf("new NAT logical IP = %q, want 10.200.1.10", newNAT.LogicalIP) - } -} - -// --- Phase 4: SG enforcement port-group membership --- - -// setupPortSGFixture stands up a switch + DHCP options + the port groups for -// the named SGs so a vpc.create-port event can join them. -func setupPortSGFixture(t *testing.T, mock *MockOVNClient, ctx context.Context, vpcId, subnetId, cidr string, sgIds []string) { - t.Helper() - if err := mock.CreateLogicalRouter(ctx, nbdbLogicalRouter("vpc-"+vpcId, vpcId)); err != nil { - t.Fatalf("CreateLogicalRouter: %v", err) - } - if err := mock.CreateLogicalSwitch(ctx, nbdbLogicalSwitch("subnet-"+subnetId, subnetId, vpcId)); err != nil { - t.Fatalf("CreateLogicalSwitch: %v", err) - } - if _, err := mock.CreateDHCPOptions(ctx, nbdbDHCPOptions(cidr, subnetId, vpcId)); err != nil { - t.Fatalf("CreateDHCPOptions: %v", err) - } - for _, sgId := range sgIds { - pg := topology.SecurityGroupPortGroup(sgId) - if err := mock.CreatePortGroup(ctx, pg, nil); err != nil { - t.Fatalf("CreatePortGroup %s: %v", sgId, err) - } - } -} - -func TestTopologyHandler_CreatePort_JoinsPortGroups(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - setupPortSGFixture(t, mock, ctx, "vpc-sg1", "subnet-sg1", "10.0.1.0/24", []string{"sg-1111", "sg-2222"}) - - evt := PortEvent{ - NetworkInterfaceId: "eni-sg1", - SubnetId: "subnet-sg1", - VpcId: "vpc-sg1", - PrivateIpAddress: "10.0.1.10", - MacAddress: "02:00:00:00:00:01", - SecurityGroupIds: []string{"sg-1111", "sg-2222"}, - } - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicCreatePort, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertSuccess(t, resp, "create port with SGs") - - lsp, err := mock.GetLogicalSwitchPort(ctx, "port-eni-sg1") - if err != nil { - t.Fatalf("GetLogicalSwitchPort: %v", err) - } - - for _, sgId := range []string{"sg-1111", "sg-2222"} { - pg := mock.PortGroups[topology.SecurityGroupPortGroup(sgId)] - if !slices.Contains(pg.Ports, lsp.UUID) { - t.Errorf("expected %s membership in port group %s, got %v", lsp.UUID, sgId, pg.Ports) - } - } -} - -func TestTopologyHandler_CreatePort_MissingPortGroup_NoLSPCreated(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Switch + DHCP, but the SG's port group is intentionally absent. - setupPortSGFixture(t, mock, ctx, "vpc-sg2", "subnet-sg2", "10.0.2.0/24", nil) - - evt := PortEvent{ - NetworkInterfaceId: "eni-sg2", - SubnetId: "subnet-sg2", - VpcId: "vpc-sg2", - PrivateIpAddress: "10.0.2.10", - MacAddress: "02:00:00:00:00:02", - SecurityGroupIds: []string{"sg-missing"}, - } - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicCreatePort, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertFailure(t, resp, "create port with missing SG port group") - - if _, err := mock.GetLogicalSwitchPort(ctx, "port-eni-sg2"); err == nil { - t.Error("expected LSP not to be created when atomic transaction fails") - } -} - -func TestTopologyHandler_DeletePort_ClearsPortGroupMembership(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - setupPortSGFixture(t, mock, ctx, "vpc-del", "subnet-del", "10.0.3.0/24", []string{"sg-del"}) - - createEvt := PortEvent{ - NetworkInterfaceId: "eni-del", - SubnetId: "subnet-del", - VpcId: "vpc-del", - PrivateIpAddress: "10.0.3.10", - MacAddress: "02:00:00:00:00:03", - SecurityGroupIds: []string{"sg-del"}, - } - data, _ := json.Marshal(createEvt) - resp, _ := nc.Request(TopicCreatePort, data, 5_000_000_000) - assertSuccess(t, resp, "create port") - - if got := len(mock.PortGroups[topology.SecurityGroupPortGroup("sg-del")].Ports); got != 1 { - t.Fatalf("setup: expected 1 member in port group, got %d", got) - } - - delEvt := PortEvent{ - NetworkInterfaceId: "eni-del", - SubnetId: "subnet-del", - VpcId: "vpc-del", - PrivateIpAddress: "10.0.3.10", - MacAddress: "02:00:00:00:00:03", - } - data, _ = json.Marshal(delEvt) - resp, _ = nc.Request(TopicDeletePort, data, 5_000_000_000) - assertSuccess(t, resp, "delete port") - - if got := len(mock.PortGroups[topology.SecurityGroupPortGroup("sg-del")].Ports); got != 0 { - t.Errorf("expected 0 members after delete-port, got %d", got) - } -} - -func TestTopologyHandler_UpdatePortSGs_Transitions(t *testing.T) { - cases := []struct { - name string - initial []string - desired []string - expected []string - }{ - {name: "empty_to_A", initial: nil, desired: []string{"sg-A"}, expected: []string{"sg-A"}}, - {name: "A_to_B", initial: []string{"sg-A"}, desired: []string{"sg-B"}, expected: []string{"sg-B"}}, - {name: "A_to_AB", initial: []string{"sg-A"}, desired: []string{"sg-A", "sg-B"}, expected: []string{"sg-A", "sg-B"}}, - {name: "AB_to_B", initial: []string{"sg-A", "sg-B"}, desired: []string{"sg-B"}, expected: []string{"sg-B"}}, - {name: "AB_to_empty", initial: []string{"sg-A", "sg-B"}, desired: nil, expected: nil}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // All possible SGs need their port groups in place — desired \ initial - // has to find them on the way in. - allSGs := make(map[string]struct{}) - for _, s := range tc.initial { - allSGs[s] = struct{}{} - } - for _, s := range tc.desired { - allSGs[s] = struct{}{} - } - sgList := make([]string, 0, len(allSGs)) - for s := range allSGs { - sgList = append(sgList, s) - } - setupPortSGFixture(t, mock, ctx, "vpc-tr", "subnet-tr", "10.0.4.0/24", sgList) - - createEvt := PortEvent{ - NetworkInterfaceId: "eni-tr", - SubnetId: "subnet-tr", - VpcId: "vpc-tr", - PrivateIpAddress: "10.0.4.10", - MacAddress: "02:00:00:00:00:04", - SecurityGroupIds: tc.initial, - } - data, _ := json.Marshal(createEvt) - resp, _ := nc.Request(TopicCreatePort, data, 5_000_000_000) - assertSuccess(t, resp, "create port") - - updEvt := UpdatePortSGsEvent{ - NetworkInterfaceId: "eni-tr", - PrivateIpAddress: "10.0.4.10", - SecurityGroupIds: tc.desired, - } - data, _ = json.Marshal(updEvt) - resp, err = nc.Request(TopicUpdatePortSGs, data, 5_000_000_000) - if err != nil { - t.Fatalf("request: %v", err) - } - assertSuccess(t, resp, "update port SGs") - - lsp, err := mock.GetLogicalSwitchPort(ctx, "port-eni-tr") - if err != nil { - t.Fatalf("GetLogicalSwitchPort: %v", err) - } - expected := make(map[string]struct{}, len(tc.expected)) - for _, s := range tc.expected { - expected[topology.SecurityGroupPortGroup(s)] = struct{}{} - } - for name, pg := range mock.PortGroups { - inGroup := slices.Contains(pg.Ports, lsp.UUID) - _, want := expected[name] - if inGroup && !want { - t.Errorf("port unexpectedly in %s", name) - } - if !inGroup && want { - t.Errorf("port missing from expected %s", name) - } - } - }) - } -} - -// handleCreatePort must converge SG memberships even when the LSP already -// exists from a previous failed attempt — otherwise a port stranded outside -// its SG port groups would have unrestricted traffic until the next -// reconciler pass (~30s of zero-ACL exposure on the recovery path). -func TestTopologyHandler_CreatePort_ExistingLSP_ReconcilesSGMemberships(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - setupPortSGFixture(t, mock, ctx, "vpc-recover", "subnet-recover", "10.0.6.0/24", []string{"sg-recover"}) - - // Pre-create the LSP without joining any port group, simulating a - // previous create that crashed mid-transaction in some hypothetical - // non-atomic world. - require.NoError(t, mock.CreateLogicalSwitchPort(ctx, "subnet-subnet-recover", &nbdb.LogicalSwitchPort{ - Name: "port-eni-recover", - Addresses: []string{"02:00:00:00:00:99 10.0.6.50"}, - })) - pgName := topology.SecurityGroupPortGroup("sg-recover") - require.NotContains(t, mock.PortGroups[pgName].Ports, mock.Ports["port-eni-recover"].UUID) - - evt := PortEvent{ - NetworkInterfaceId: "eni-recover", - SubnetId: "subnet-recover", - VpcId: "vpc-recover", - PrivateIpAddress: "10.0.6.50", - MacAddress: "02:00:00:00:00:99", - SecurityGroupIds: []string{"sg-recover"}, - } - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicCreatePort, data, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "create port with pre-existing LSP") - - // The port is now in the SG port group. The matching `_ip4` - // Address_Set is auto-derived by ovn-northd in production from port - // group port addresses and intentionally not modelled in the mock. - lsp, err := mock.GetLogicalSwitchPort(ctx, "port-eni-recover") - require.NoError(t, err) - assert.Contains(t, mock.PortGroups[pgName].Ports, lsp.UUID, "existing-LSP path must reconcile SG memberships") -} - -// --- handleAddNATGateway / handleDeleteNATGateway --- -// -// These handlers are fire-and-forget (no respond()), so tests publish + flush -// + poll OVN state instead of awaiting a NATS reply. - -// natGatewayProcessed waits for the SNAT rule for subnetCidr to (dis)appear on -// router routerName. Polls because handleAdd/DeleteNATGateway is fire-and- -// forget; without a wait the test races the dispatch goroutine. -func natGatewayProcessed(t *testing.T, mock *MockOVNClient, routerName, subnetCidr string, wantPresent bool) { - t.Helper() - deadline := time.Now().Add(2 * time.Second) - for time.Now().Before(deadline) { - mock.Mu.Lock() - lr := mock.Routers[routerName] - present := false - if lr != nil { - for _, uuid := range lr.NAT { - if n := mock.NATs[uuid]; n != nil && n.Type == "snat" && n.LogicalIP == subnetCidr { - present = true - break - } - } - } - mock.Mu.Unlock() - if present == wantPresent { - return - } - time.Sleep(20 * time.Millisecond) - } - t.Fatalf("NAT-Gateway SNAT (router=%s, cidr=%s) wantPresent=%v never converged", routerName, subnetCidr, wantPresent) -} - -func TestTopologyHandler_AddNATGateway_AddsSnat(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - require.NoError(t, mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-natgw", - ExternalIDs: map[string]string{"spinifex:vpc_id": "vpc-natgw"}, - })) - - evt := NATGatewayEvent{ - VpcId: "vpc-natgw", - NatGatewayId: "nat-001", - PublicIp: "203.0.113.10", - SubnetCidr: "10.0.50.0/24", - } - data, _ := json.Marshal(evt) - require.NoError(t, nc.Publish(TopicAddNATGateway, data)) - require.NoError(t, nc.Flush()) - - natGatewayProcessed(t, mock, "vpc-vpc-natgw", "10.0.50.0/24", true) - - mock.Mu.Lock() - defer mock.Mu.Unlock() - lr := mock.Routers["vpc-vpc-natgw"] - require.Len(t, lr.NAT, 1) - nat := mock.NATs[lr.NAT[0]] - assert.Equal(t, "snat", nat.Type) - assert.Equal(t, "203.0.113.10", nat.ExternalIP) - assert.Equal(t, "10.0.50.0/24", nat.LogicalIP) - assert.Equal(t, "nat-001", nat.ExternalIDs["spinifex:nat_gateway_id"]) - _ = ctx -} - -func TestTopologyHandler_DeleteNATGateway_RemovesSnat(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - require.NoError(t, mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{Name: "vpc-vpc-rm"})) - - evt := NATGatewayEvent{ - VpcId: "vpc-rm", - NatGatewayId: "nat-rm", - PublicIp: "203.0.113.20", - SubnetCidr: "10.0.51.0/24", - } - data, _ := json.Marshal(evt) - require.NoError(t, nc.Publish(TopicAddNATGateway, data)) - require.NoError(t, nc.Flush()) - natGatewayProcessed(t, mock, "vpc-vpc-rm", "10.0.51.0/24", true) - - require.NoError(t, nc.Publish(TopicDeleteNATGateway, data)) - require.NoError(t, nc.Flush()) - natGatewayProcessed(t, mock, "vpc-vpc-rm", "10.0.51.0/24", false) - - mock.Mu.Lock() - natCount := len(mock.Routers["vpc-vpc-rm"].NAT) - mock.Mu.Unlock() - assert.Zero(t, natCount, "delete must remove the SNAT rule") - _ = ctx -} - -// --- handleDeleteNAT (1:1 dnat_and_snat removal) --- - -func TestTopologyHandler_DeleteNAT_OvnNotConnected(t *testing.T) { - _, nc := startTestNATS(t) - topo := NewTopologyHandler(nil) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - data, _ := json.Marshal(NATEvent{VpcId: "v", LogicalIP: "10.0.0.1"}) - resp, err := nc.Request(TopicDeleteNAT, data, 5_000_000_000) - require.NoError(t, err) - assertFailure(t, resp, "delete-nat must fail when ovn is nil") -} - -func TestTopologyHandler_DeleteNAT_BadJSON(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - resp, err := nc.Request(TopicDeleteNAT, []byte("not json"), 5_000_000_000) - require.NoError(t, err) - assertFailure(t, resp, "delete-nat with malformed payload must fail") -} - -func TestTopologyHandler_DeleteNAT_Success(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - require.NoError(t, mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{Name: "vpc-vpc-dnatrm"})) - - addEvt := NATEvent{VpcId: "vpc-dnatrm", ExternalIP: "203.0.113.30", LogicalIP: "10.0.60.5"} - addData, _ := json.Marshal(addEvt) - resp, err := nc.Request(TopicAddNAT, addData, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "add NAT") - - delEvt := NATEvent{VpcId: "vpc-dnatrm", LogicalIP: "10.0.60.5"} - delData, _ := json.Marshal(delEvt) - resp, err = nc.Request(TopicDeleteNAT, delData, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "delete NAT") - - lr, _ := mock.GetLogicalRouter(ctx, "vpc-vpc-dnatrm") - assert.Empty(t, lr.NAT, "NAT rule must be removed") -} - -// TestTopologyHandler_DeleteNAT_NotFound_Idempotent: instance termination -// fans out through multiple cleanup paths (ReleasePublicIP via -// instanceCleanerAdapter AND DeleteNetworkInterface in the VPC service), and -// both publish vpc.delete-nat for the same (vpc, logical_ip). The publish is -// fire-and-forget (utils.PublishNATEvent for vpc.delete-nat), so callers -// never branch on the response — a hard error on second delete just -// pollutes triage logs without surfacing any actionable failure. Treat -// "rule already absent" as success so the second publisher doesn't emit a -// spurious ERROR. The handler still returns an error for other failure -// modes (router missing, OVN unreachable, transact failure). -func TestTopologyHandler_DeleteNAT_NotFound_Idempotent(t *testing.T) { - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock) - subs, err := topo.Subscribe(nc) - require.NoError(t, err) - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - require.NoError(t, mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{Name: "vpc-vpc-empty"})) - - delEvt := NATEvent{VpcId: "vpc-empty", LogicalIP: "10.99.99.99"} - data, _ := json.Marshal(delEvt) - resp, err := nc.Request(TopicDeleteNAT, data, 5_000_000_000) - require.NoError(t, err) - assertSuccess(t, resp, "delete-nat for non-existent rule must be idempotent (mulga-siv-107)") -} diff --git a/spinifex/services/vpcd/vpcd.go b/spinifex/services/vpcd/vpcd.go index 94fea2efe..6f38fa0ae 100644 --- a/spinifex/services/vpcd/vpcd.go +++ b/spinifex/services/vpcd/vpcd.go @@ -19,9 +19,57 @@ import ( "github.com/mulgadc/spinifex/spinifex/network/host" "github.com/mulgadc/spinifex/spinifex/network/policy" "github.com/mulgadc/spinifex/spinifex/network/reconcile" + "github.com/mulgadc/spinifex/spinifex/network/subscribers" + "github.com/mulgadc/spinifex/spinifex/network/topology" "github.com/mulgadc/spinifex/spinifex/utils" ) +// Bridge mode constants for external connectivity. Bridge mode selects how +// the WAN NIC reaches the OVS integration bridge: +// - Direct: WAN NIC is added directly to br-external as an OVS port. +// Enables distributed NAT. Only safe when the WAN NIC is NOT the SSH/ +// management NIC. +// - Veth: a veth pair links a Linux bridge (br-wan) to the OVS bridge +// (br-ext). Requires centralized NAT — the Linux bridge intermediary +// breaks distributed NAT hairpin routing. +const ( + BridgeModeDirect = "direct" + BridgeModeVeth = "veth" + // OvnExternalBridge is the OVS bridge that ovn-bridge-mappings targets + // for the "external" localnet. Owned by setup-ovn.sh and independent of + // the WAN bridge (which is Linux-side in veth mode). + OvnExternalBridge = "br-ext" +) + +// waitForFlowsHV shells out to `ovn-nbctl --wait=hv sync`, bumping +// NB_Global.nb_cfg and blocking until every connected chassis has acknowledged +// the new sequence — i.e. ovn-northd has compiled NB→SB and ovn-controller +// has installed the resulting flows. Used as the NAT manager's flows barrier +// so newly-launched VMs aren't unreachable while their gateway chassis is +// still catching up (mulga-siv-105). Bounded by ovn-nbctl --timeout=30; on +// overrun we log a Warn and return nil so the caller continues. +// +// Declared as a var so tests can stub it. +var waitForFlowsHV = func() error { + start := time.Now() + cmd := sudoCommand("ovn-nbctl", + "--no-leader-only", + "--timeout=30", + "--wait=hv", + "sync", + ) + if out, err := cmd.CombinedOutput(); err != nil { + slog.Warn("vpcd: OVN flows-ready barrier overran; continuing without confirmation", + "elapsed", time.Since(start), + "err", err, + "output", strings.TrimSpace(string(out)), + ) + return nil + } + slog.Debug("vpcd: OVN flows-ready barrier complete", "elapsed", time.Since(start)) + return nil +} + // sudoCommand wraps exec.Command with sudo when running as non-root. // OVS/OVN commands require elevated privileges; when running as root // (Docker, production) no wrapper is needed. @@ -416,12 +464,7 @@ func launchService(cfg *Config) error { return err } - // Reconcile OVN topology from bootstrap config before subscribing. - // This ensures the default VPC topology exists even if admin init ran - // before services were started (first-install case). - var topoOpts []TopologyOption if cfg.ExternalMode != "" { - topoOpts = append(topoOpts, WithExternalNetwork(cfg.ExternalMode, cfg.ExternalPools)) slog.Info("External network enabled", "mode", cfg.ExternalMode, "pools", len(cfg.ExternalPools)) } // Discover chassis from OVN SBDB. The OVS-managed system-id (persisted at @@ -436,46 +479,24 @@ func launchService(cfg *Config) error { if len(chassisNames) == 0 { return fmt.Errorf("vpcd: no OVN chassis registered in SBDB — is ovn-controller running and connected?") } - topoOpts = append(topoOpts, WithChassisNames(chassisNames)) slog.Info("vpcd: gateway chassis discovered", "chassis", chassisNames) + uplinkMode := host.UplinkModePhysical if bridgeMode == BridgeModeVeth { uplinkMode = host.UplinkModeVeth } natMode := policy.NATModeFromUplinkMode(uplinkMode) - topoOpts = append(topoOpts, WithNATMode(natMode)) - topo := NewTopologyHandler(liveClient, topoOpts...) - // Elect a single vpcd to run startup reconcile. Without this, N vpcds in a - // multi-node cluster all hit Get-then-Create on Logical_Router with no - // atomicity, producing duplicate rows that ovn-nbctl rejects with - // "Multiple logical routers named '...'. Use a UUID." (mulga-siv-29). - // Runtime VPC events still fan out via the vpcd-workers queue group, so - // non-leaders remain functional after Subscribe below. - holder, _ := os.Hostname() - releaseLeader, isLeader := reconcile.AcquireLeader(nc, holder) - - subs, err := topo.Subscribe(nc) - if err != nil { - slog.Error("Failed to subscribe to VPC topics", "err", err) - return err + // Construct the network manager stack. The reconciler is the single + // intent-driven pass that runs once at startup (leader-gated) and then + // on a 5-minute drift ticker; together they replace the five separate + // passes the old vpcd ran on boot (mulga-siv-125, Phase 2). + var topoOpts []topology.Option + if dns := pickDNSServer(cfg.ExternalPools); dns != "" { + topoOpts = append(topoOpts, topology.WithDNSServer(func() string { return dns })) } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() + topoMgr := topology.NewLiveManager(liveClient, topoOpts...) - js, err := nc.JetStream() - if err != nil { - slog.Error("Failed to get JetStream context", "err", err) - return err - } - - // Construct the network/reconcile manager stack. The reconciler is the - // single intent-driven pass that runs once at startup (leader-gated) and - // then on a 5-minute drift ticker; together they replace the five - // separate passes the old vpcd ran on boot (mulga-siv-125, Phase 2). sgMgr := policy.NewSecurityGroupManager(liveClient) natMgr, err := policy.NewNATManager(liveClient, natMode, policy.WithFlowsBarrier(waitForFlowsHV)) if err != nil { @@ -502,13 +523,48 @@ func launchService(cfg *Config) error { return fmt.Errorf("construct IGW manager: %w", err) } + // Elect a single vpcd to run startup reconcile. Without this, N vpcds in + // a multi-node cluster all hit Get-then-Create on Logical_Router with no + // atomicity, producing duplicate rows that ovn-nbctl rejects with + // "Multiple logical routers named '...'. Use a UUID." (mulga-siv-29). + // Runtime VPC events still fan out via the vpcd-workers queue group, so + // non-leaders remain functional after Subscribe below. + holder, _ := os.Hostname() + releaseLeader, isLeader := reconcile.AcquireLeader(nc, holder) + + subscriber, err := subscribers.New(subscribers.Config{ + Topology: topoMgr, + SG: sgMgr, + NAT: natMgr, + IGW: igwMgr, + }) + if err != nil { + return fmt.Errorf("construct subscriber: %w", err) + } + subs, err := subscriber.Subscribe(nc) + if err != nil { + slog.Error("Failed to subscribe to VPC topics", "err", err) + return err + } + defer func() { + for _, s := range subs { + _ = s.Unsubscribe() + } + }() + + js, err := nc.JetStream() + if err != nil { + slog.Error("Failed to get JetStream context", "err", err) + return err + } + rec, err := reconcile.New(reconcile.Config{ OVN: liveClient, SG: sgMgr, NAT: natMgr, Routes: routeMgr, IGW: igwMgr, - Topology: topo, + Topology: topoMgr, LocalAZ: cfg.AZ, NodeHostname: holder, Chassis: chassisNames, @@ -558,6 +614,19 @@ func launchService(cfg *Config) error { return nil } +// pickDNSServer returns the OVN dhcp_options dns_server string ("{a, b}") +// from the first unscoped (no region, no AZ) pool with DNS servers set. +// Empty string falls back to topology.NewLiveManager's default +// ("{8.8.8.8, 1.1.1.1}"). +func pickDNSServer(pools []ExternalPoolConfig) string { + for _, p := range pools { + if p.Region == "" && p.AZ == "" && len(p.DNSServers) > 0 { + return "{" + strings.Join(p.DNSServers, ", ") + "}" + } + } + return "" +} + // externalPoolConfigToShared translates the vpcd-local ExternalPoolConfig // into the network/external package's shared shape. The two will merge // once topology.go is split (bead mulga-siv-125.3.4 — see external.go's