mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-22 12:16:52 +00:00
feat(network/policy): L3 SecurityGroup/NAT/Route managers
Introduce network/policy as the data-plane policy layer (ADR-0006 L3).
Three managers, all backed by network/ovn:
- SecurityGroupManager: clear+add ACL replacement on an existing port
group; port group lifecycle stays with L2 (topology.Manager).
- NATManager: bound to one NATMode at construction (derived from
host.UplinkMode). Distributed sets logical_port/external_mac on EIP
dnat_and_snat rules; centralised leaves them unset. Per-EIP, NAT GW,
and IGW-default SNAT entrypoints; all deletes idempotent.
- RouteManager: AddStaticRoute is read-then-decide so retries no longer
duplicate rows (closing the long-standing non-idempotency bug in
vpcd/topology.go's IGW default-route path).
ACL priority constants and infrastructure ACL builders move from
vpcd/{security,acl}.go into policy/acl.go with their semantics preserved
(default-deny 900/800 logged, DHCP-allow 1050, tenant 1000). No vpcd
wiring changes here — additive only; the reconciler (bead .3.2) and
network/external (bead .3.4) will consume these managers.
bead: mulga-siv-125.3.1
This commit is contained in:
joshsiv-mulga
@@ -0,0 +1,226 @@
|
||||
package policy
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
"github.com/mulgadc/spinifex/spinifex/network/ovn"
|
||||
"github.com/mulgadc/spinifex/spinifex/network/topology"
|
||||
)
|
||||
|
||||
// ACL priority table. Higher priority wins. Tenants only ever populate
|
||||
// priority 1000; the 1100/1050/900/800 bands are platform-reserved.
|
||||
//
|
||||
// See docs/development/feature/spinifex-network-redesign.md §8.1 for the
|
||||
// authoritative table.
|
||||
const (
|
||||
// ACLPriorityAllowIntraSG allows unconditional traffic between members
|
||||
// of the same port group. Reserved for future use; not yet emitted by
|
||||
// InfrastructureACLs.
|
||||
ACLPriorityAllowIntraSG = 1100
|
||||
|
||||
// ACLPriorityAllowDHCP sits above tenant rules so a narrow-egress SG
|
||||
// (e.g. only-VPC-CIDR) doesn't silently drop the guest's DHCPDISCOVER
|
||||
// against the priority-900 default-deny. AWS users can't write a rule
|
||||
// for 255.255.255.255, so the platform must.
|
||||
ACLPriorityAllowDHCP = 1050
|
||||
|
||||
// ACLPriorityTenantAllow is the band tenant ingress/egress allow rules
|
||||
// emit at. Allows are not logged — high volume, low signal on a private
|
||||
// network.
|
||||
ACLPriorityTenantAllow = 1000
|
||||
|
||||
// ACLPriorityDefaultDenyIngress is the logged drop for unmatched
|
||||
// ingress. Severity = denyACLSeverity. CMMC SC.L1-3.13.1.
|
||||
ACLPriorityDefaultDenyIngress = 900
|
||||
|
||||
// ACLPriorityDefaultDenyEgress mirrors the ingress default-deny on the
|
||||
// egress direction.
|
||||
ACLPriorityDefaultDenyEgress = 800
|
||||
)
|
||||
|
||||
// denyACLSeverity is the syslog severity OVN logs against a default-deny ACL
|
||||
// hit. "info" is loud enough for a syslog forwarder to capture, quiet enough
|
||||
// that a port scan doesn't page anyone. Operators can promote it at the log
|
||||
// collector if they want higher-priority alerts.
|
||||
const denyACLSeverity = "info"
|
||||
|
||||
// Rule is the policy-layer representation of an AWS-style security group
|
||||
// rule. The handler layer constructs Rules from the AWS API payload after
|
||||
// validating CidrIp / SourceSG — this builder interpolates the values
|
||||
// verbatim, so untrusted input must not reach here.
|
||||
type Rule struct {
|
||||
IPProtocol string // "tcp", "udp", "icmp", or "-1" (all)
|
||||
FromPort int64
|
||||
ToPort int64
|
||||
CIDR string
|
||||
SourceSG string
|
||||
}
|
||||
|
||||
// InfrastructureACLs returns the platform ACLs that every port group carries
|
||||
// regardless of tenant rules: the priority-900/800 default-deny pair (logged
|
||||
// for CMMC SC.L1-3.13.1 boundary monitoring) and the priority-1050 DHCPv4
|
||||
// client/server allows.
|
||||
//
|
||||
// OVN's built-in DHCP responder runs inside the LS pipeline after the
|
||||
// port-group ACL stage, so without these allows a narrow-egress tenant SG
|
||||
// silently drops the guest's DHCPDISCOVER (dst=255.255.255.255) against the
|
||||
// default-deny and the VM never gets a lease. Matches AWS's implicit allow.
|
||||
func InfrastructureACLs(portGroupName string) []ovn.ACLSpec {
|
||||
return []ovn.ACLSpec{
|
||||
denyIngressACL(portGroupName),
|
||||
denyEgressACL(portGroupName),
|
||||
dhcpEgressACL(portGroupName),
|
||||
dhcpIngressACL(portGroupName),
|
||||
}
|
||||
}
|
||||
|
||||
// RuleACLSpecs builds the priority-1000 allow ACL specs for the given
|
||||
// tenant rules. Action is "allow-related" so reply traffic for established
|
||||
// flows passes without requiring an explicit reverse-direction rule.
|
||||
func RuleACLSpecs(portGroupName string, ingress, egress []Rule) []ovn.ACLSpec {
|
||||
specs := make([]ovn.ACLSpec, 0, len(ingress)+len(egress))
|
||||
for _, rule := range ingress {
|
||||
specs = append(specs, ovn.ACLSpec{
|
||||
Direction: "to-lport",
|
||||
Priority: ACLPriorityTenantAllow,
|
||||
Match: BuildIngressACLMatch(portGroupName, rule),
|
||||
Action: "allow-related",
|
||||
})
|
||||
}
|
||||
for _, rule := range egress {
|
||||
specs = append(specs, ovn.ACLSpec{
|
||||
Direction: "from-lport",
|
||||
Priority: ACLPriorityTenantAllow,
|
||||
Match: BuildEgressACLMatch(portGroupName, rule),
|
||||
Action: "allow-related",
|
||||
})
|
||||
}
|
||||
return specs
|
||||
}
|
||||
|
||||
// BuildIngressACLMatch generates an OVN ACL match expression for an ingress
|
||||
// rule. Ingress rules use "outport == @{pgName}" because OVN ACLs with
|
||||
// direction "to-lport" match on the destination port (outport from the
|
||||
// pipeline's perspective).
|
||||
//
|
||||
// Tenant-supplied CIDR/SourceSG values MUST be validated by the handler
|
||||
// (see handlers/ec2/vpc/security_group.go) before reaching this builder;
|
||||
// values are interpolated verbatim.
|
||||
func BuildIngressACLMatch(portGroupName string, rule Rule) string {
|
||||
parts := []string{fmt.Sprintf("outport == @%s", portGroupName), "ip4"}
|
||||
parts = appendProtocolMatch(parts, rule)
|
||||
if rule.CIDR != "" && rule.CIDR != "0.0.0.0/0" {
|
||||
parts = append(parts, fmt.Sprintf("ip4.src == %s", rule.CIDR))
|
||||
}
|
||||
if rule.SourceSG != "" {
|
||||
parts = append(parts, fmt.Sprintf("ip4.src == $%s", addressSetName(topology.SecurityGroupPortGroup(rule.SourceSG))))
|
||||
}
|
||||
return strings.Join(parts, " && ")
|
||||
}
|
||||
|
||||
// BuildEgressACLMatch generates an OVN ACL match expression for an egress
|
||||
// rule. Egress rules use "inport == @{pgName}" because OVN ACLs with
|
||||
// direction "from-lport" match on the source port (inport from the
|
||||
// pipeline's perspective).
|
||||
func BuildEgressACLMatch(portGroupName string, rule Rule) string {
|
||||
parts := []string{fmt.Sprintf("inport == @%s", portGroupName), "ip4"}
|
||||
parts = appendProtocolMatch(parts, rule)
|
||||
if rule.CIDR != "" && rule.CIDR != "0.0.0.0/0" {
|
||||
parts = append(parts, fmt.Sprintf("ip4.dst == %s", rule.CIDR))
|
||||
}
|
||||
if rule.SourceSG != "" {
|
||||
parts = append(parts, fmt.Sprintf("ip4.dst == $%s", addressSetName(topology.SecurityGroupPortGroup(rule.SourceSG))))
|
||||
}
|
||||
return strings.Join(parts, " && ")
|
||||
}
|
||||
|
||||
// addressSetName returns the SB Address_Set name that ovn-northd
|
||||
// auto-derives for the given port group's IPv4 addresses. SG-to-SG ACL
|
||||
// matches reference it via "ip4.src == $<name>" / "ip4.dst == $<name>".
|
||||
// Lifecycle is owned by ovn-northd — Spinifex must NOT create explicit NB
|
||||
// Address_Set rows with these names; a duplicate sync to SB wedges northd
|
||||
// in an "OVNSB commit failed" retry loop.
|
||||
func addressSetName(portGroupName string) string {
|
||||
return portGroupName + "_ip4"
|
||||
}
|
||||
|
||||
func appendProtocolMatch(parts []string, rule Rule) []string {
|
||||
switch rule.IPProtocol {
|
||||
case "tcp":
|
||||
parts = appendPortMatch(parts, "tcp", rule.FromPort, rule.ToPort)
|
||||
case "udp":
|
||||
parts = appendPortMatch(parts, "udp", rule.FromPort, rule.ToPort)
|
||||
case "icmp":
|
||||
parts = append(parts, "icmp4")
|
||||
case "-1", "":
|
||||
// All protocols — no additional filter beyond "ip4".
|
||||
default:
|
||||
// Unknown protocol — treat as all traffic.
|
||||
}
|
||||
return parts
|
||||
}
|
||||
|
||||
func appendPortMatch(parts []string, proto string, fromPort, toPort int64) []string {
|
||||
if fromPort == 0 && toPort == 0 {
|
||||
return append(parts, proto)
|
||||
}
|
||||
if fromPort == toPort {
|
||||
return append(parts, fmt.Sprintf("%s.dst == %d", proto, fromPort))
|
||||
}
|
||||
parts = append(parts, fmt.Sprintf("%s.dst >= %d", proto, fromPort))
|
||||
parts = append(parts, fmt.Sprintf("%s.dst <= %d", proto, toPort))
|
||||
return parts
|
||||
}
|
||||
|
||||
func denyIngressACL(portGroupName string) ovn.ACLSpec {
|
||||
return ovn.ACLSpec{
|
||||
Direction: "to-lport",
|
||||
Priority: ACLPriorityDefaultDenyIngress,
|
||||
Match: fmt.Sprintf("outport == @%s && ip4", portGroupName),
|
||||
Action: "drop",
|
||||
Name: portGroupName + "-deny-ingress",
|
||||
Log: true,
|
||||
Severity: denyACLSeverity,
|
||||
}
|
||||
}
|
||||
|
||||
func denyEgressACL(portGroupName string) ovn.ACLSpec {
|
||||
return ovn.ACLSpec{
|
||||
Direction: "from-lport",
|
||||
Priority: ACLPriorityDefaultDenyEgress,
|
||||
Match: fmt.Sprintf("inport == @%s && ip4", portGroupName),
|
||||
Action: "drop",
|
||||
Name: portGroupName + "-deny-egress",
|
||||
Log: true,
|
||||
Severity: denyACLSeverity,
|
||||
}
|
||||
}
|
||||
|
||||
// dhcpEgressACL allows the guest's DHCP client traffic (DHCPDISCOVER,
|
||||
// DHCPREQUEST: udp.src=68, udp.dst=67) out of the port. Action is "allow"
|
||||
// rather than "allow-related" — DHCP is single-shot request/reply and
|
||||
// allow-related on broadcast UDP interacts oddly with OVN's CT zones.
|
||||
func dhcpEgressACL(portGroupName string) ovn.ACLSpec {
|
||||
return ovn.ACLSpec{
|
||||
Direction: "from-lport",
|
||||
Priority: ACLPriorityAllowDHCP,
|
||||
Match: fmt.Sprintf("inport == @%s && udp && udp.src == 68 && udp.dst == 67", portGroupName),
|
||||
Action: "allow",
|
||||
Name: portGroupName + "-allow-dhcp-egress",
|
||||
}
|
||||
}
|
||||
|
||||
// dhcpIngressACL allows the OVN-generated DHCP server reply (DHCPOFFER,
|
||||
// DHCPACK: udp.src=67, udp.dst=68) back to the guest. OVN's DHCP responder
|
||||
// emits the reply from inside the LS pipeline, so without this matching
|
||||
// to-lport allow the reply hits the priority-900 ingress default-deny.
|
||||
func dhcpIngressACL(portGroupName string) ovn.ACLSpec {
|
||||
return ovn.ACLSpec{
|
||||
Direction: "to-lport",
|
||||
Priority: ACLPriorityAllowDHCP,
|
||||
Match: fmt.Sprintf("outport == @%s && udp && udp.src == 67 && udp.dst == 68", portGroupName),
|
||||
Action: "allow",
|
||||
Name: portGroupName + "-allow-dhcp-ingress",
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,136 @@
|
||||
package policy
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestACL_TCPPortFromCIDR(t *testing.T) {
|
||||
match := BuildIngressACLMatch("sg_test", Rule{
|
||||
IPProtocol: "tcp",
|
||||
FromPort: 22,
|
||||
ToPort: 22,
|
||||
CIDR: "10.0.0.0/8",
|
||||
})
|
||||
assert.Contains(t, match, "tcp.dst == 22")
|
||||
assert.Contains(t, match, "ip4.src == 10.0.0.0/8")
|
||||
assert.Contains(t, match, "outport == @sg_test")
|
||||
assert.Contains(t, match, "ip4")
|
||||
}
|
||||
|
||||
func TestACL_AllTrafficFromSG(t *testing.T) {
|
||||
match := BuildIngressACLMatch("sg_test", Rule{
|
||||
IPProtocol: "-1",
|
||||
SourceSG: "sg-abc123",
|
||||
})
|
||||
assert.Contains(t, match, "ip4.src == $sg_abc123_ip4")
|
||||
assert.Contains(t, match, "outport == @sg_test")
|
||||
assert.Contains(t, match, "ip4")
|
||||
}
|
||||
|
||||
func TestACL_PortRange(t *testing.T) {
|
||||
match := BuildIngressACLMatch("sg_test", Rule{
|
||||
IPProtocol: "udp",
|
||||
FromPort: 1024,
|
||||
ToPort: 65535,
|
||||
})
|
||||
assert.Contains(t, match, "udp.dst >= 1024")
|
||||
assert.Contains(t, match, "udp.dst <= 65535")
|
||||
}
|
||||
|
||||
func TestACL_ICMP(t *testing.T) {
|
||||
match := BuildIngressACLMatch("sg_test", Rule{
|
||||
IPProtocol: "icmp",
|
||||
CIDR: "0.0.0.0/0",
|
||||
})
|
||||
assert.Contains(t, match, "icmp4")
|
||||
assert.NotContains(t, match, "tcp.dst")
|
||||
assert.NotContains(t, match, "udp.dst")
|
||||
}
|
||||
|
||||
func TestACL_AllProtocols(t *testing.T) {
|
||||
match := BuildIngressACLMatch("sg_test", Rule{
|
||||
IPProtocol: "-1",
|
||||
CIDR: "10.0.0.0/16",
|
||||
})
|
||||
assert.Contains(t, match, "ip4")
|
||||
assert.Contains(t, match, "ip4.src == 10.0.0.0/16")
|
||||
assert.NotContains(t, match, "tcp")
|
||||
assert.NotContains(t, match, "udp")
|
||||
assert.NotContains(t, match, "icmp")
|
||||
}
|
||||
|
||||
func TestACL_EgressAll(t *testing.T) {
|
||||
match := BuildEgressACLMatch("sg_test", Rule{
|
||||
IPProtocol: "-1",
|
||||
CIDR: "0.0.0.0/0",
|
||||
})
|
||||
assert.Contains(t, match, "inport == @sg_test")
|
||||
assert.NotContains(t, match, "outport")
|
||||
assert.Contains(t, match, "ip4")
|
||||
}
|
||||
|
||||
func TestACL_TCPSinglePort(t *testing.T) {
|
||||
match := BuildIngressACLMatch("sg_test", Rule{
|
||||
IPProtocol: "tcp",
|
||||
FromPort: 443,
|
||||
ToPort: 443,
|
||||
CIDR: "10.0.0.0/8",
|
||||
})
|
||||
assert.Contains(t, match, "tcp.dst == 443")
|
||||
assert.NotContains(t, match, "tcp.dst >=")
|
||||
assert.NotContains(t, match, "tcp.dst <=")
|
||||
}
|
||||
|
||||
func TestACL_NoSource(t *testing.T) {
|
||||
match := BuildIngressACLMatch("sg_test", Rule{
|
||||
IPProtocol: "tcp",
|
||||
FromPort: 80,
|
||||
ToPort: 80,
|
||||
CIDR: "0.0.0.0/0",
|
||||
})
|
||||
assert.Contains(t, match, "tcp.dst == 80")
|
||||
assert.NotContains(t, match, "ip4.src")
|
||||
}
|
||||
|
||||
func TestACL_EgressFromSGToSG(t *testing.T) {
|
||||
match := BuildEgressACLMatch("sg_test", Rule{
|
||||
IPProtocol: "tcp",
|
||||
FromPort: 3306,
|
||||
ToPort: 3306,
|
||||
SourceSG: "sg-db-tier",
|
||||
})
|
||||
assert.Contains(t, match, "inport == @sg_test")
|
||||
assert.Contains(t, match, "tcp.dst == 3306")
|
||||
assert.Contains(t, match, "ip4.dst == $sg_db_tier_ip4")
|
||||
}
|
||||
|
||||
func TestInfrastructureACLs_Shape(t *testing.T) {
|
||||
specs := InfrastructureACLs("sg_test")
|
||||
if assert.Len(t, specs, 4) {
|
||||
// Priorities: deny-ingress 900, deny-egress 800, dhcp-egress 1050, dhcp-ingress 1050
|
||||
assert.Equal(t, ACLPriorityDefaultDenyIngress, specs[0].Priority)
|
||||
assert.Equal(t, "drop", specs[0].Action)
|
||||
assert.True(t, specs[0].Log)
|
||||
assert.Equal(t, ACLPriorityDefaultDenyEgress, specs[1].Priority)
|
||||
assert.True(t, specs[1].Log)
|
||||
assert.Equal(t, ACLPriorityAllowDHCP, specs[2].Priority)
|
||||
assert.Equal(t, "allow", specs[2].Action)
|
||||
assert.Equal(t, ACLPriorityAllowDHCP, specs[3].Priority)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRuleACLSpecs_PriorityAndAction(t *testing.T) {
|
||||
specs := RuleACLSpecs("sg_test",
|
||||
[]Rule{{IPProtocol: "tcp", FromPort: 80, ToPort: 80, CIDR: "0.0.0.0/0"}},
|
||||
[]Rule{{IPProtocol: "-1", CIDR: "0.0.0.0/0"}},
|
||||
)
|
||||
if assert.Len(t, specs, 2) {
|
||||
assert.Equal(t, ACLPriorityTenantAllow, specs[0].Priority)
|
||||
assert.Equal(t, "to-lport", specs[0].Direction)
|
||||
assert.Equal(t, "allow-related", specs[0].Action)
|
||||
assert.Equal(t, "from-lport", specs[1].Direction)
|
||||
assert.Equal(t, "allow-related", specs[1].Action)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,196 @@
|
||||
package policy
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
|
||||
"github.com/mulgadc/spinifex/spinifex/network/ovn"
|
||||
"github.com/mulgadc/spinifex/spinifex/network/topology"
|
||||
"github.com/mulgadc/spinifex/spinifex/services/vpcd/nbdb"
|
||||
)
|
||||
|
||||
// EIPSpec is a 1:1 Elastic IP NAT (dnat_and_snat) attaching ExternalIP to
|
||||
// LogicalIP on the VPC's router. In NATModeDistributed, PortName and MAC
|
||||
// MUST be set so OVN can install per-chassis flows; in NATModeCentralized
|
||||
// they are ignored. Callers that target distributed mode without supplying
|
||||
// both fields fall back to the centralised shape — the NAT still works but
|
||||
// hairpins through the gateway chassis.
|
||||
type EIPSpec struct {
|
||||
VPCID string
|
||||
ExternalIP string
|
||||
LogicalIP string
|
||||
PortName string // OVN LSP name backing the VM
|
||||
MAC string // external MAC for distributed NAT
|
||||
}
|
||||
|
||||
// NATGWSpec is a NAT Gateway SNAT rule: every packet leaving the private
|
||||
// subnet whose source is SubnetCIDR is rewritten to PublicIP. The OVN rule
|
||||
// keys on (snat, SubnetCIDR) on the VPC's router.
|
||||
type NATGWSpec struct {
|
||||
VPCID string
|
||||
NATGatewayID string
|
||||
PublicIP string
|
||||
SubnetCIDR string
|
||||
}
|
||||
|
||||
// NATManager owns the OVN NAT rule lifecycle for one cluster. It does not
|
||||
// create the LogicalRouter — L2 (topology.Manager) owns that — and does
|
||||
// not allocate public IPs (handlers/ec2/eip owns pool allocation).
|
||||
//
|
||||
// NAT mode is fixed at construction time from the host's UplinkMode and
|
||||
// never changes at runtime.
|
||||
type NATManager interface {
|
||||
// AddEIP installs a dnat_and_snat rule. In NATModeDistributed, PortName
|
||||
// and MAC are written to logical_port / external_mac so the rule fires
|
||||
// on the VM's own chassis. Stale dnat_and_snat rules referencing the
|
||||
// same ExternalIP on any router are removed first — an EIP can change
|
||||
// VPC association in our pool model and a leftover rule on the old
|
||||
// router would silently steal the public IP.
|
||||
AddEIP(ctx context.Context, eip EIPSpec) error
|
||||
|
||||
// DeleteEIP removes the dnat_and_snat rule with the given LogicalIP on
|
||||
// the VPC's router. Returns nil when the rule is already absent
|
||||
// (matching the idempotent semantics required by the multiple cleanup
|
||||
// paths that publish vpc.delete-nat).
|
||||
DeleteEIP(ctx context.Context, vpcID, logicalIP string) error
|
||||
|
||||
// AddNATGateway installs the snat rule for a NAT Gateway's private
|
||||
// subnet. The OVN key is (snat, SubnetCIDR); AddNATGateway therefore
|
||||
// rejects two NAT GWs on overlapping subnet CIDRs.
|
||||
AddNATGateway(ctx context.Context, gw NATGWSpec) error
|
||||
|
||||
// DeleteNATGateway removes the snat rule keyed by SubnetCIDR on the
|
||||
// VPC's router. Idempotent.
|
||||
DeleteNATGateway(ctx context.Context, vpcID, subnetCIDR string) error
|
||||
|
||||
// AddSNAT installs the IGW default-outbound snat rule rewriting the
|
||||
// VPC CIDR to ExternalIP. AWS-parity behaviour (only EIP-backed VMs
|
||||
// can egress) means callers in the IGW attach path typically skip
|
||||
// this — it exists for future deployments that opt into blanket SNAT.
|
||||
AddSNAT(ctx context.Context, vpcID, vpcCIDR, externalIP string) error
|
||||
|
||||
// DeleteSNAT removes the IGW default-outbound snat rule for vpcCIDR.
|
||||
// Idempotent.
|
||||
DeleteSNAT(ctx context.Context, vpcID, vpcCIDR string) error
|
||||
}
|
||||
|
||||
type natManager struct {
|
||||
ovn ovn.Client
|
||||
mode NATMode
|
||||
}
|
||||
|
||||
var _ NATManager = (*natManager)(nil)
|
||||
|
||||
// NewNATManager constructs a NATManager bound to one NAT mode. mode is
|
||||
// resolved at startup from host.Wiring.UplinkMode() via NATModeFromUplinkMode
|
||||
// and must not be NATModeUnknown.
|
||||
func NewNATManager(client ovn.Client, mode NATMode) (NATManager, error) {
|
||||
if mode == NATModeUnknown {
|
||||
return nil, fmt.Errorf("NAT mode is unknown; resolve from host.Wiring.UplinkMode()")
|
||||
}
|
||||
return &natManager{ovn: client, mode: mode}, nil
|
||||
}
|
||||
|
||||
func (m *natManager) AddEIP(ctx context.Context, eip EIPSpec) error {
|
||||
router := topology.VPCRouter(eip.VPCID)
|
||||
|
||||
natRule := &nbdb.NAT{
|
||||
Type: "dnat_and_snat",
|
||||
ExternalIP: eip.ExternalIP,
|
||||
LogicalIP: eip.LogicalIP,
|
||||
ExternalIDs: map[string]string{
|
||||
"spinifex:vpc_id": eip.VPCID,
|
||||
"spinifex:public_ip": eip.ExternalIP,
|
||||
},
|
||||
}
|
||||
if m.mode == NATModeDistributed && eip.PortName != "" && eip.MAC != "" {
|
||||
mac := eip.MAC
|
||||
port := eip.PortName
|
||||
natRule.ExternalMAC = &mac
|
||||
natRule.LogicalPort = &port
|
||||
}
|
||||
|
||||
// Search every router, not just the target — stale rules may exist on
|
||||
// a different VPC's router when vpc.delete-nat (fire-and-forget) hasn't
|
||||
// been processed before the IP was reused by a new VPC.
|
||||
if removed, err := m.ovn.DeleteAllNATsByExternalIP(ctx, "dnat_and_snat", eip.ExternalIP); err != nil {
|
||||
slog.Warn("policy: stale NAT cleanup failed before AddEIP", "external_ip", eip.ExternalIP, "err", err)
|
||||
} else if removed > 0 {
|
||||
slog.Info("policy: cleaned stale dnat_and_snat rules before AddEIP", "external_ip", eip.ExternalIP, "removed", removed)
|
||||
}
|
||||
|
||||
if err := m.ovn.AddNAT(ctx, router, natRule); err != nil {
|
||||
return fmt.Errorf("add dnat_and_snat %s -> %s on %s: %w", eip.LogicalIP, eip.ExternalIP, router, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *natManager) DeleteEIP(ctx context.Context, vpcID, logicalIP string) error {
|
||||
router := topology.VPCRouter(vpcID)
|
||||
if err := m.ovn.DeleteNAT(ctx, router, "dnat_and_snat", logicalIP); err != nil {
|
||||
if errors.Is(err, ovn.ErrNATNotFound) {
|
||||
return nil
|
||||
}
|
||||
return fmt.Errorf("delete dnat_and_snat %s on %s: %w", logicalIP, router, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *natManager) AddNATGateway(ctx context.Context, gw NATGWSpec) error {
|
||||
router := topology.VPCRouter(gw.VPCID)
|
||||
|
||||
snatRule := &nbdb.NAT{
|
||||
Type: "snat",
|
||||
ExternalIP: gw.PublicIP,
|
||||
LogicalIP: gw.SubnetCIDR,
|
||||
ExternalIDs: map[string]string{
|
||||
"spinifex:vpc_id": gw.VPCID,
|
||||
"spinifex:nat_gateway_id": gw.NATGatewayID,
|
||||
},
|
||||
}
|
||||
if err := m.ovn.AddNAT(ctx, router, snatRule); err != nil {
|
||||
return fmt.Errorf("add NAT GW snat %s -> %s on %s: %w", gw.SubnetCIDR, gw.PublicIP, router, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *natManager) DeleteNATGateway(ctx context.Context, vpcID, subnetCIDR string) error {
|
||||
router := topology.VPCRouter(vpcID)
|
||||
if err := m.ovn.DeleteNAT(ctx, router, "snat", subnetCIDR); err != nil {
|
||||
if errors.Is(err, ovn.ErrNATNotFound) {
|
||||
return nil
|
||||
}
|
||||
return fmt.Errorf("delete NAT GW snat %s on %s: %w", subnetCIDR, router, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *natManager) AddSNAT(ctx context.Context, vpcID, vpcCIDR, externalIP string) error {
|
||||
router := topology.VPCRouter(vpcID)
|
||||
snatRule := &nbdb.NAT{
|
||||
Type: "snat",
|
||||
ExternalIP: externalIP,
|
||||
LogicalIP: vpcCIDR,
|
||||
ExternalIDs: map[string]string{
|
||||
"spinifex:vpc_id": vpcID,
|
||||
"spinifex:role": "igw-default-snat",
|
||||
},
|
||||
}
|
||||
if err := m.ovn.AddNAT(ctx, router, snatRule); err != nil {
|
||||
return fmt.Errorf("add IGW snat %s -> %s on %s: %w", vpcCIDR, externalIP, router, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *natManager) DeleteSNAT(ctx context.Context, vpcID, vpcCIDR string) error {
|
||||
router := topology.VPCRouter(vpcID)
|
||||
if err := m.ovn.DeleteNAT(ctx, router, "snat", vpcCIDR); err != nil {
|
||||
if errors.Is(err, ovn.ErrNATNotFound) {
|
||||
return nil
|
||||
}
|
||||
return fmt.Errorf("delete IGW snat %s on %s: %w", vpcCIDR, router, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -0,0 +1,170 @@
|
||||
package policy
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/mulgadc/spinifex/spinifex/network/host"
|
||||
"github.com/mulgadc/spinifex/spinifex/network/ovn/mock"
|
||||
"github.com/mulgadc/spinifex/spinifex/network/topology"
|
||||
"github.com/mulgadc/spinifex/spinifex/services/vpcd/nbdb"
|
||||
)
|
||||
|
||||
func seedRouter(t *testing.T, m *mock.Client, vpcID string) string {
|
||||
t.Helper()
|
||||
name := topology.VPCRouter(vpcID)
|
||||
require.NoError(t, m.CreateLogicalRouter(context.Background(), &nbdb.LogicalRouter{Name: name}))
|
||||
return name
|
||||
}
|
||||
|
||||
func findNAT(m *mock.Client, natType, logicalIP string) *nbdb.NAT {
|
||||
for _, n := range m.NATs {
|
||||
if n.Type == natType && n.LogicalIP == logicalIP {
|
||||
return n
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func TestNATModeFromUplinkMode(t *testing.T) {
|
||||
assert.Equal(t, NATModeDistributed, NATModeFromUplinkMode(host.UplinkModePhysical))
|
||||
assert.Equal(t, NATModeCentralized, NATModeFromUplinkMode(host.UplinkModeVeth))
|
||||
assert.Equal(t, NATModeUnknown, NATModeFromUplinkMode(host.UplinkModeUnknown))
|
||||
}
|
||||
|
||||
func TestNewNATManager_RejectsUnknownMode(t *testing.T) {
|
||||
_, err := NewNATManager(mock.New(), NATModeUnknown)
|
||||
require.Error(t, err)
|
||||
}
|
||||
|
||||
func TestNATManager_AddEIP_Distributed_SetsPortAndMAC(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := mock.New()
|
||||
seedRouter(t, m, "vpc-1")
|
||||
nm, err := NewNATManager(m, NATModeDistributed)
|
||||
require.NoError(t, err)
|
||||
|
||||
require.NoError(t, nm.AddEIP(ctx, EIPSpec{
|
||||
VPCID: "vpc-1",
|
||||
ExternalIP: "1.2.3.4",
|
||||
LogicalIP: "10.0.0.5",
|
||||
PortName: "port-eni-abc",
|
||||
MAC: "aa:bb:cc:dd:ee:ff",
|
||||
}))
|
||||
|
||||
got := findNAT(m, "dnat_and_snat", "10.0.0.5")
|
||||
require.NotNil(t, got)
|
||||
require.NotNil(t, got.LogicalPort)
|
||||
require.NotNil(t, got.ExternalMAC)
|
||||
assert.Equal(t, "port-eni-abc", *got.LogicalPort)
|
||||
assert.Equal(t, "aa:bb:cc:dd:ee:ff", *got.ExternalMAC)
|
||||
}
|
||||
|
||||
func TestNATManager_AddEIP_Centralized_LeavesPortAndMACUnset(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := mock.New()
|
||||
seedRouter(t, m, "vpc-1")
|
||||
nm, err := NewNATManager(m, NATModeCentralized)
|
||||
require.NoError(t, err)
|
||||
|
||||
require.NoError(t, nm.AddEIP(ctx, EIPSpec{
|
||||
VPCID: "vpc-1",
|
||||
ExternalIP: "1.2.3.4",
|
||||
LogicalIP: "10.0.0.5",
|
||||
PortName: "port-eni-abc",
|
||||
MAC: "aa:bb:cc:dd:ee:ff",
|
||||
}))
|
||||
|
||||
got := findNAT(m, "dnat_and_snat", "10.0.0.5")
|
||||
require.NotNil(t, got)
|
||||
assert.Nil(t, got.LogicalPort)
|
||||
assert.Nil(t, got.ExternalMAC)
|
||||
}
|
||||
|
||||
func TestNATManager_AddEIP_RemovesStaleRuleOnOtherRouter(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := mock.New()
|
||||
seedRouter(t, m, "vpc-old")
|
||||
seedRouter(t, m, "vpc-new")
|
||||
nm, err := NewNATManager(m, NATModeDistributed)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Stale rule still attached to vpc-old's router for the same external IP.
|
||||
require.NoError(t, nm.AddEIP(ctx, EIPSpec{
|
||||
VPCID: "vpc-old",
|
||||
ExternalIP: "1.2.3.4",
|
||||
LogicalIP: "10.0.0.5",
|
||||
PortName: "port-a", MAC: "aa:aa:aa:aa:aa:aa",
|
||||
}))
|
||||
// Reuse the EIP in vpc-new.
|
||||
require.NoError(t, nm.AddEIP(ctx, EIPSpec{
|
||||
VPCID: "vpc-new",
|
||||
ExternalIP: "1.2.3.4",
|
||||
LogicalIP: "10.1.0.7",
|
||||
PortName: "port-b", MAC: "bb:bb:bb:bb:bb:bb",
|
||||
}))
|
||||
|
||||
// Only the new rule survives.
|
||||
var matching []*nbdb.NAT
|
||||
for _, n := range m.NATs {
|
||||
if n.Type == "dnat_and_snat" && n.ExternalIP == "1.2.3.4" {
|
||||
matching = append(matching, n)
|
||||
}
|
||||
}
|
||||
require.Len(t, matching, 1)
|
||||
assert.Equal(t, "10.1.0.7", matching[0].LogicalIP)
|
||||
}
|
||||
|
||||
func TestNATManager_DeleteEIP_IdempotentOnMissing(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := mock.New()
|
||||
seedRouter(t, m, "vpc-1")
|
||||
nm, _ := NewNATManager(m, NATModeDistributed)
|
||||
|
||||
// No prior AddEIP — delete should still succeed.
|
||||
require.NoError(t, nm.DeleteEIP(ctx, "vpc-1", "10.0.0.5"))
|
||||
}
|
||||
|
||||
func TestNATManager_AddNATGateway_AndDelete(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := mock.New()
|
||||
seedRouter(t, m, "vpc-1")
|
||||
nm, _ := NewNATManager(m, NATModeCentralized)
|
||||
|
||||
require.NoError(t, nm.AddNATGateway(ctx, NATGWSpec{
|
||||
VPCID: "vpc-1",
|
||||
NATGatewayID: "nat-xyz",
|
||||
PublicIP: "9.9.9.9",
|
||||
SubnetCIDR: "10.0.1.0/24",
|
||||
}))
|
||||
got := findNAT(m, "snat", "10.0.1.0/24")
|
||||
require.NotNil(t, got)
|
||||
assert.Equal(t, "9.9.9.9", got.ExternalIP)
|
||||
assert.Equal(t, "nat-xyz", got.ExternalIDs["spinifex:nat_gateway_id"])
|
||||
|
||||
require.NoError(t, nm.DeleteNATGateway(ctx, "vpc-1", "10.0.1.0/24"))
|
||||
assert.Nil(t, findNAT(m, "snat", "10.0.1.0/24"))
|
||||
|
||||
// Idempotent re-delete.
|
||||
require.NoError(t, nm.DeleteNATGateway(ctx, "vpc-1", "10.0.1.0/24"))
|
||||
}
|
||||
|
||||
func TestNATManager_AddSNAT_AndDelete(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := mock.New()
|
||||
seedRouter(t, m, "vpc-1")
|
||||
nm, _ := NewNATManager(m, NATModeDistributed)
|
||||
|
||||
require.NoError(t, nm.AddSNAT(ctx, "vpc-1", "10.0.0.0/16", "1.2.3.4"))
|
||||
got := findNAT(m, "snat", "10.0.0.0/16")
|
||||
require.NotNil(t, got)
|
||||
assert.Equal(t, "1.2.3.4", got.ExternalIP)
|
||||
assert.Equal(t, "igw-default-snat", got.ExternalIDs["spinifex:role"])
|
||||
|
||||
require.NoError(t, nm.DeleteSNAT(ctx, "vpc-1", "10.0.0.0/16"))
|
||||
assert.Nil(t, findNAT(m, "snat", "10.0.0.0/16"))
|
||||
require.NoError(t, nm.DeleteSNAT(ctx, "vpc-1", "10.0.0.0/16"))
|
||||
}
|
||||
@@ -0,0 +1,69 @@
|
||||
// Package policy is L3 of the spinifex network stack: data-plane policy
|
||||
// attached to the logical objects owned by L2 (network/topology). It exposes
|
||||
// three managers — SecurityGroupManager (ACLs against SG port groups),
|
||||
// NATManager (dnat_and_snat for EIPs, snat for NAT gateways and IGW default
|
||||
// outbound), and RouteManager (static + default routes) — and calls L1
|
||||
// (network/ovn) for the OVSDB transactions.
|
||||
//
|
||||
// L3 does not create or delete L2 objects. Port groups, logical routers, and
|
||||
// logical switches must already exist before the L3 managers can attach
|
||||
// policy to them; layering breaks are loud (L1 returns "not found" and the
|
||||
// L3 method propagates the error).
|
||||
//
|
||||
// See docs/development/feature/spinifex-network-redesign.md §8 and
|
||||
// docs/development/feature/spinifex-network-redesign-phase2.md §2.1 for the
|
||||
// full contract.
|
||||
package policy
|
||||
|
||||
import (
|
||||
"github.com/mulgadc/spinifex/spinifex/network/host"
|
||||
)
|
||||
|
||||
// NATMode tells NATManager whether the cluster runs distributed or
|
||||
// centralised NAT. The choice is fixed at L0 by the host's uplink mode and
|
||||
// is passed in at NATManager construction time — there is no runtime
|
||||
// reselection. ADR-0006 S3.
|
||||
type NATMode int
|
||||
|
||||
const (
|
||||
// NATModeUnknown is the zero value; never returned by a configured
|
||||
// NATManager.
|
||||
NATModeUnknown NATMode = iota
|
||||
|
||||
// NATModeDistributed sets ExternalMAC + LogicalPort on every dnat_and_snat
|
||||
// rule so OVN processes DNAT on the VM's own chassis without hairpinning
|
||||
// through the gateway chassis. Required by UplinkModePhysical.
|
||||
NATModeDistributed
|
||||
|
||||
// NATModeCentralized leaves ExternalMAC + LogicalPort unset so the
|
||||
// gateway chassis owns SNAT/DNAT. Required by UplinkModeVeth because the
|
||||
// Linux bridge intermediary breaks distributed-NAT hairpin routing.
|
||||
NATModeCentralized
|
||||
)
|
||||
|
||||
// String returns the canonical name used in logs.
|
||||
func (m NATMode) String() string {
|
||||
switch m {
|
||||
case NATModeDistributed:
|
||||
return "distributed"
|
||||
case NATModeCentralized:
|
||||
return "centralized"
|
||||
default:
|
||||
return "unknown"
|
||||
}
|
||||
}
|
||||
|
||||
// NATModeFromUplinkMode resolves the L0 uplink mode into the L3 NAT mode.
|
||||
// The mapping is fixed: physical uplinks support per-chassis NAT, veth
|
||||
// uplinks do not. UplinkModeUnknown returns NATModeUnknown so misconfiguration
|
||||
// surfaces loudly at NATManager construction.
|
||||
func NATModeFromUplinkMode(m host.UplinkMode) NATMode {
|
||||
switch m {
|
||||
case host.UplinkModePhysical:
|
||||
return NATModeDistributed
|
||||
case host.UplinkModeVeth:
|
||||
return NATModeCentralized
|
||||
default:
|
||||
return NATModeUnknown
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,138 @@
|
||||
package policy
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/netip"
|
||||
|
||||
"github.com/mulgadc/spinifex/spinifex/network/ovn"
|
||||
"github.com/mulgadc/spinifex/spinifex/network/topology"
|
||||
"github.com/mulgadc/spinifex/spinifex/services/vpcd/nbdb"
|
||||
)
|
||||
|
||||
// defaultRoutePrefix is the IPv4 default-route prefix used by AddDefaultRoute
|
||||
// / DeleteDefaultRoute. Held as a constant so the magic string isn't
|
||||
// scattered across the package.
|
||||
const defaultRoutePrefix = "0.0.0.0/0"
|
||||
|
||||
// RouteSpec is a single static route on a VPC's LogicalRouter. OutputPort is
|
||||
// only required when Nexthop is not on a directly-connected subnet — e.g. the
|
||||
// IGW default route uses 169.254.0.1/30 on the gateway LRP but the WAN
|
||||
// nexthop sits on the uplink subnet, so OVN northd silently drops the route
|
||||
// from southbound unless OutputPort pins it to the gateway LRP. The IGW
|
||||
// attach path always sets it.
|
||||
type RouteSpec struct {
|
||||
Prefix netip.Prefix
|
||||
Nexthop string
|
||||
OutputPort string
|
||||
}
|
||||
|
||||
// RouteManager owns static routes on VPC LogicalRouters. The router itself
|
||||
// is L2's responsibility; RouteManager only adds/removes route rows.
|
||||
//
|
||||
// Add operations are idempotent — a re-add with identical Nexthop+OutputPort
|
||||
// is a no-op, a re-add with drifted fields deletes-then-adds. The historical
|
||||
// vpcd path was non-idempotent (every retry left a duplicate row); the new
|
||||
// API closes that bug at the contract level so the reconciler can replay
|
||||
// freely.
|
||||
type RouteManager interface {
|
||||
AddDefaultRoute(ctx context.Context, vpcID, nexthop, outputPort string) error
|
||||
DeleteDefaultRoute(ctx context.Context, vpcID string) error
|
||||
AddStaticRoute(ctx context.Context, vpcID string, route RouteSpec) error
|
||||
DeleteStaticRoute(ctx context.Context, vpcID string, prefix netip.Prefix) error
|
||||
}
|
||||
|
||||
type routeManager struct {
|
||||
ovn ovn.Client
|
||||
}
|
||||
|
||||
var _ RouteManager = (*routeManager)(nil)
|
||||
|
||||
// NewRouteManager constructs a RouteManager backed by the given OVN client.
|
||||
func NewRouteManager(client ovn.Client) RouteManager {
|
||||
return &routeManager{ovn: client}
|
||||
}
|
||||
|
||||
func (m *routeManager) AddDefaultRoute(ctx context.Context, vpcID, nexthop, outputPort string) error {
|
||||
prefix, err := netip.ParsePrefix(defaultRoutePrefix)
|
||||
if err != nil {
|
||||
return fmt.Errorf("parse default route prefix: %w", err)
|
||||
}
|
||||
return m.AddStaticRoute(ctx, vpcID, RouteSpec{
|
||||
Prefix: prefix,
|
||||
Nexthop: nexthop,
|
||||
OutputPort: outputPort,
|
||||
})
|
||||
}
|
||||
|
||||
func (m *routeManager) DeleteDefaultRoute(ctx context.Context, vpcID string) error {
|
||||
return m.deleteByPrefixString(ctx, vpcID, defaultRoutePrefix)
|
||||
}
|
||||
|
||||
func (m *routeManager) AddStaticRoute(ctx context.Context, vpcID string, route RouteSpec) error {
|
||||
router := topology.VPCRouter(vpcID)
|
||||
prefixStr := route.Prefix.String()
|
||||
|
||||
existing, err := m.ovn.FindStaticRoute(ctx, router, prefixStr)
|
||||
if err != nil {
|
||||
return fmt.Errorf("find static route %s on %s: %w", prefixStr, router, err)
|
||||
}
|
||||
if existing != nil {
|
||||
if routeMatches(existing, route) {
|
||||
return nil
|
||||
}
|
||||
if err := m.ovn.DeleteStaticRoute(ctx, router, prefixStr); err != nil {
|
||||
return fmt.Errorf("delete drifted static route %s on %s: %w", prefixStr, router, err)
|
||||
}
|
||||
}
|
||||
|
||||
row := &nbdb.LogicalRouterStaticRoute{
|
||||
IPPrefix: prefixStr,
|
||||
Nexthop: route.Nexthop,
|
||||
}
|
||||
if route.OutputPort != "" {
|
||||
op := route.OutputPort
|
||||
row.OutputPort = &op
|
||||
}
|
||||
if err := m.ovn.AddStaticRoute(ctx, router, row); err != nil {
|
||||
return fmt.Errorf("add static route %s -> %s on %s: %w", prefixStr, route.Nexthop, router, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *routeManager) DeleteStaticRoute(ctx context.Context, vpcID string, prefix netip.Prefix) error {
|
||||
return m.deleteByPrefixString(ctx, vpcID, prefix.String())
|
||||
}
|
||||
|
||||
// deleteByPrefixString centralises the "delete and treat not-found as
|
||||
// success" idempotency required by every Delete* entrypoint. Returns nil
|
||||
// when the route is already absent — the only stable contract for callers
|
||||
// driven by fire-and-forget events (IGW detach, NAT GW delete).
|
||||
func (m *routeManager) deleteByPrefixString(ctx context.Context, vpcID, prefix string) error {
|
||||
router := topology.VPCRouter(vpcID)
|
||||
existing, err := m.ovn.FindStaticRoute(ctx, router, prefix)
|
||||
if err != nil {
|
||||
return fmt.Errorf("find static route %s on %s: %w", prefix, router, err)
|
||||
}
|
||||
if existing == nil {
|
||||
return nil
|
||||
}
|
||||
if err := m.ovn.DeleteStaticRoute(ctx, router, prefix); err != nil {
|
||||
return fmt.Errorf("delete static route %s on %s: %w", prefix, router, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func routeMatches(existing *nbdb.LogicalRouterStaticRoute, want RouteSpec) bool {
|
||||
if existing.Nexthop != want.Nexthop {
|
||||
return false
|
||||
}
|
||||
switch {
|
||||
case existing.OutputPort == nil && want.OutputPort == "":
|
||||
return true
|
||||
case existing.OutputPort == nil || want.OutputPort == "":
|
||||
return false
|
||||
default:
|
||||
return *existing.OutputPort == want.OutputPort
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,104 @@
|
||||
package policy
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/netip"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/mulgadc/spinifex/spinifex/network/ovn/mock"
|
||||
"github.com/mulgadc/spinifex/spinifex/network/topology"
|
||||
"github.com/mulgadc/spinifex/spinifex/services/vpcd/nbdb"
|
||||
)
|
||||
|
||||
func findRoute(m *mock.Client, prefix string) *nbdb.LogicalRouterStaticRoute {
|
||||
for _, r := range m.StaticRoutes {
|
||||
if r.IPPrefix == prefix {
|
||||
return r
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func countRoutes(m *mock.Client, prefix string) int {
|
||||
n := 0
|
||||
for _, r := range m.StaticRoutes {
|
||||
if r.IPPrefix == prefix {
|
||||
n++
|
||||
}
|
||||
}
|
||||
return n
|
||||
}
|
||||
|
||||
func TestRouteManager_AddDefaultRoute_WritesOutputPort(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := mock.New()
|
||||
seedRouter(t, m, "vpc-1")
|
||||
rm := NewRouteManager(m)
|
||||
|
||||
require.NoError(t, rm.AddDefaultRoute(ctx, "vpc-1", "192.168.1.1", topology.GatewayRouterPort("vpc-1")))
|
||||
|
||||
got := findRoute(m, "0.0.0.0/0")
|
||||
require.NotNil(t, got)
|
||||
assert.Equal(t, "192.168.1.1", got.Nexthop)
|
||||
require.NotNil(t, got.OutputPort)
|
||||
assert.Equal(t, "gw-vpc-1", *got.OutputPort)
|
||||
}
|
||||
|
||||
func TestRouteManager_AddStaticRoute_IsIdempotent(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := mock.New()
|
||||
seedRouter(t, m, "vpc-1")
|
||||
rm := NewRouteManager(m)
|
||||
|
||||
spec := RouteSpec{
|
||||
Prefix: netip.MustParsePrefix("10.42.0.0/24"),
|
||||
Nexthop: "10.0.0.254",
|
||||
}
|
||||
require.NoError(t, rm.AddStaticRoute(ctx, "vpc-1", spec))
|
||||
require.NoError(t, rm.AddStaticRoute(ctx, "vpc-1", spec))
|
||||
require.NoError(t, rm.AddStaticRoute(ctx, "vpc-1", spec))
|
||||
|
||||
assert.Equal(t, 1, countRoutes(m, "10.42.0.0/24"))
|
||||
}
|
||||
|
||||
func TestRouteManager_AddStaticRoute_DriftReplacesRow(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := mock.New()
|
||||
seedRouter(t, m, "vpc-1")
|
||||
rm := NewRouteManager(m)
|
||||
|
||||
require.NoError(t, rm.AddDefaultRoute(ctx, "vpc-1", "192.168.1.1", "gw-vpc-1"))
|
||||
// Drift the nexthop.
|
||||
require.NoError(t, rm.AddDefaultRoute(ctx, "vpc-1", "192.168.1.254", "gw-vpc-1"))
|
||||
|
||||
assert.Equal(t, 1, countRoutes(m, "0.0.0.0/0"))
|
||||
got := findRoute(m, "0.0.0.0/0")
|
||||
require.NotNil(t, got)
|
||||
assert.Equal(t, "192.168.1.254", got.Nexthop)
|
||||
}
|
||||
|
||||
func TestRouteManager_DeleteDefaultRoute_IdempotentOnMissing(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := mock.New()
|
||||
seedRouter(t, m, "vpc-1")
|
||||
rm := NewRouteManager(m)
|
||||
|
||||
require.NoError(t, rm.DeleteDefaultRoute(ctx, "vpc-1"))
|
||||
}
|
||||
|
||||
func TestRouteManager_DeleteStaticRoute_RemovesRow(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := mock.New()
|
||||
seedRouter(t, m, "vpc-1")
|
||||
rm := NewRouteManager(m)
|
||||
|
||||
prefix := netip.MustParsePrefix("10.42.0.0/24")
|
||||
require.NoError(t, rm.AddStaticRoute(ctx, "vpc-1", RouteSpec{Prefix: prefix, Nexthop: "10.0.0.254"}))
|
||||
require.NotNil(t, findRoute(m, "10.42.0.0/24"))
|
||||
|
||||
require.NoError(t, rm.DeleteStaticRoute(ctx, "vpc-1", prefix))
|
||||
assert.Nil(t, findRoute(m, "10.42.0.0/24"))
|
||||
}
|
||||
@@ -0,0 +1,91 @@
|
||||
package policy
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
|
||||
"github.com/mulgadc/spinifex/spinifex/network/ovn"
|
||||
"github.com/mulgadc/spinifex/spinifex/network/topology"
|
||||
)
|
||||
|
||||
// SGSpec is the policy-layer view of a security group: identifier plus
|
||||
// ingress/egress rule sets. The OVN port group named by
|
||||
// topology.SecurityGroupPortGroup(GroupID) must already exist (L2 owns
|
||||
// its lifecycle); SecurityGroupManager only attaches and clears ACLs.
|
||||
type SGSpec struct {
|
||||
GroupID string
|
||||
VPCID string
|
||||
IngressRules []Rule
|
||||
EgressRules []Rule
|
||||
}
|
||||
|
||||
// SecurityGroupManager attaches ACL policy to a security group's OVN port
|
||||
// group. EnsureSG and UpdateSG share semantics — both atomically clear and
|
||||
// re-add the full ACL set (infrastructure + tenant). The replace-all model
|
||||
// avoids per-rule diff complexity: OVN treats the AddACLs transaction as
|
||||
// atomic, so callers never observe a partial rule set on the wire.
|
||||
//
|
||||
// Port group lifecycle (create + delete) is owned by topology.Manager. If
|
||||
// the port group is absent, EnsureSG / UpdateSG / DeleteSG return the
|
||||
// underlying L1 "port group not found" error.
|
||||
type SecurityGroupManager interface {
|
||||
// EnsureSG sets the full ACL set on the SG's port group. Idempotent:
|
||||
// repeated calls with the same spec converge to the same OVN state.
|
||||
EnsureSG(ctx context.Context, sg SGSpec) error
|
||||
|
||||
// UpdateSG replaces the SG's ACL set. Identical to EnsureSG; kept as a
|
||||
// distinct method to mirror the parent plan's interface (§8.1) and
|
||||
// document caller intent.
|
||||
UpdateSG(ctx context.Context, sg SGSpec) error
|
||||
|
||||
// DeleteSG clears every ACL on the SG's port group. The port group
|
||||
// itself is not deleted — topology.Manager owns that.
|
||||
DeleteSG(ctx context.Context, groupID string) error
|
||||
}
|
||||
|
||||
type sgManager struct {
|
||||
ovn ovn.Client
|
||||
}
|
||||
|
||||
var _ SecurityGroupManager = (*sgManager)(nil)
|
||||
|
||||
// NewSecurityGroupManager constructs a SecurityGroupManager backed by the
|
||||
// given OVN client.
|
||||
func NewSecurityGroupManager(client ovn.Client) SecurityGroupManager {
|
||||
return &sgManager{ovn: client}
|
||||
}
|
||||
|
||||
func (m *sgManager) EnsureSG(ctx context.Context, sg SGSpec) error {
|
||||
return m.applyACLs(ctx, sg)
|
||||
}
|
||||
|
||||
func (m *sgManager) UpdateSG(ctx context.Context, sg SGSpec) error {
|
||||
return m.applyACLs(ctx, sg)
|
||||
}
|
||||
|
||||
func (m *sgManager) DeleteSG(ctx context.Context, groupID string) error {
|
||||
pg := topology.SecurityGroupPortGroup(groupID)
|
||||
if err := m.ovn.ClearACLs(ctx, pg); err != nil {
|
||||
return fmt.Errorf("clear ACLs on %s: %w", pg, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// applyACLs clears the port group's current ACL set and re-adds the
|
||||
// infrastructure + tenant ACLs. ClearACLs followed by AddACLs is not
|
||||
// transactional at the OVSDB level, but every chassis re-derives flows
|
||||
// from the new ACL set in one ovn-northd compile cycle — the observable
|
||||
// gap is bounded by southbound flow-install latency, the same window that
|
||||
// applies to any ACL change. Callers requiring stronger atomicity must
|
||||
// gate updates via leader election.
|
||||
func (m *sgManager) applyACLs(ctx context.Context, sg SGSpec) error {
|
||||
pg := topology.SecurityGroupPortGroup(sg.GroupID)
|
||||
if err := m.ovn.ClearACLs(ctx, pg); err != nil {
|
||||
return fmt.Errorf("clear ACLs on %s: %w", pg, err)
|
||||
}
|
||||
specs := append(InfrastructureACLs(pg), RuleACLSpecs(pg, sg.IngressRules, sg.EgressRules)...)
|
||||
if err := m.ovn.AddACLs(ctx, pg, specs); err != nil {
|
||||
return fmt.Errorf("add ACLs on %s: %w", pg, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -0,0 +1,111 @@
|
||||
package policy
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/mulgadc/spinifex/spinifex/network/ovn/mock"
|
||||
"github.com/mulgadc/spinifex/spinifex/network/topology"
|
||||
)
|
||||
|
||||
func newSGMockWithPG(t *testing.T, sgID string) (*mock.Client, string) {
|
||||
t.Helper()
|
||||
m := mock.New()
|
||||
pg := topology.SecurityGroupPortGroup(sgID)
|
||||
require.NoError(t, m.CreatePortGroup(context.Background(), pg, nil))
|
||||
return m, pg
|
||||
}
|
||||
|
||||
func TestSGManager_EnsureSG_AppliesInfraPlusTenantACLs(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m, pg := newSGMockWithPG(t, "sg-web")
|
||||
sg := NewSecurityGroupManager(m)
|
||||
|
||||
spec := SGSpec{
|
||||
GroupID: "sg-web",
|
||||
VPCID: "vpc-1",
|
||||
IngressRules: []Rule{
|
||||
{IPProtocol: "tcp", FromPort: 80, ToPort: 80, CIDR: "0.0.0.0/0"},
|
||||
{IPProtocol: "tcp", FromPort: 443, ToPort: 443, CIDR: "0.0.0.0/0"},
|
||||
},
|
||||
EgressRules: []Rule{
|
||||
{IPProtocol: "-1", CIDR: "0.0.0.0/0"},
|
||||
},
|
||||
}
|
||||
require.NoError(t, sg.EnsureSG(ctx, spec))
|
||||
|
||||
storedPG, ok := m.PortGroups[pg]
|
||||
require.True(t, ok)
|
||||
// 4 infra + 2 ingress + 1 egress = 7 ACLs.
|
||||
assert.Len(t, storedPG.ACLs, 7)
|
||||
}
|
||||
|
||||
func TestSGManager_UpdateSG_ReplacesACLSet(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m, pg := newSGMockWithPG(t, "sg-web")
|
||||
sg := NewSecurityGroupManager(m)
|
||||
|
||||
require.NoError(t, sg.EnsureSG(ctx, SGSpec{
|
||||
GroupID: "sg-web",
|
||||
IngressRules: []Rule{{IPProtocol: "tcp", FromPort: 22, ToPort: 22, CIDR: "10.0.0.0/8"}},
|
||||
}))
|
||||
firstCount := len(m.PortGroups[pg].ACLs)
|
||||
assert.Equal(t, 5, firstCount) // 4 infra + 1 tenant
|
||||
|
||||
// Replace with a different set — wider ingress, no egress.
|
||||
require.NoError(t, sg.UpdateSG(ctx, SGSpec{
|
||||
GroupID: "sg-web",
|
||||
IngressRules: []Rule{
|
||||
{IPProtocol: "tcp", FromPort: 80, ToPort: 80, CIDR: "0.0.0.0/0"},
|
||||
{IPProtocol: "tcp", FromPort: 443, ToPort: 443, CIDR: "0.0.0.0/0"},
|
||||
{IPProtocol: "icmp", CIDR: "0.0.0.0/0"},
|
||||
},
|
||||
}))
|
||||
assert.Len(t, m.PortGroups[pg].ACLs, 7) // 4 infra + 3 tenant
|
||||
}
|
||||
|
||||
func TestSGManager_DeleteSG_ClearsACLsKeepsPortGroup(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m, pg := newSGMockWithPG(t, "sg-web")
|
||||
sg := NewSecurityGroupManager(m)
|
||||
|
||||
require.NoError(t, sg.EnsureSG(ctx, SGSpec{
|
||||
GroupID: "sg-web",
|
||||
IngressRules: []Rule{{IPProtocol: "-1", CIDR: "0.0.0.0/0"}},
|
||||
}))
|
||||
assert.NotEmpty(t, m.PortGroups[pg].ACLs)
|
||||
|
||||
require.NoError(t, sg.DeleteSG(ctx, "sg-web"))
|
||||
assert.Empty(t, m.PortGroups[pg].ACLs)
|
||||
// Port group still present — L2 owns lifecycle.
|
||||
_, exists := m.PortGroups[pg]
|
||||
assert.True(t, exists)
|
||||
}
|
||||
|
||||
func TestSGManager_EnsureSG_MissingPortGroup_ReturnsError(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := mock.New()
|
||||
sg := NewSecurityGroupManager(m)
|
||||
|
||||
err := sg.EnsureSG(ctx, SGSpec{GroupID: "sg-missing"})
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "port group")
|
||||
}
|
||||
|
||||
func TestSGManager_EnsureSG_Idempotent(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m, pg := newSGMockWithPG(t, "sg-web")
|
||||
sg := NewSecurityGroupManager(m)
|
||||
|
||||
spec := SGSpec{
|
||||
GroupID: "sg-web",
|
||||
IngressRules: []Rule{{IPProtocol: "tcp", FromPort: 22, ToPort: 22, CIDR: "10.0.0.0/8"}},
|
||||
}
|
||||
require.NoError(t, sg.EnsureSG(ctx, spec))
|
||||
first := len(m.PortGroups[pg].ACLs)
|
||||
require.NoError(t, sg.EnsureSG(ctx, spec))
|
||||
assert.Equal(t, first, len(m.PortGroups[pg].ACLs))
|
||||
}
|
||||
Reference in New Issue
Block a user