refactor(config): drop upstream-DHCP config surface (static WAN pool only)

Hard-error in LoadConfig on legacy keys: [network] external_dhcp,
[nodes.<n>.vpcd] dhcp_bind_bridge, and [[network.external_pools]]
source = "dhcp". Add pool range validation (must fit inside the
gateway/prefix_len CIDR; per-cluster non-overlap).

Strip ExternalDHCP from config.NetworkConfig + formation.NetworkConfig
+ admin.ConfigSettings, and DhcpBindBridge from config.VPCDConfig +
admin.ConfigSettings + vpcd.Config. vpcd.ExternalPoolConfig.IsDHCP()
short-circuits to false; the field and downstream DHCP branches in
external_ipam / topology.go remain for Slice C to delete together with
dhcp_manager.

cmd/spinifex/cmd/admin.go: drop useExternalDHCP path and the
detectedDhcpBindBridge helper; require --external-pool when
--external-mode=pool; auto-detect default no longer infers source=dhcp.

service.go's wan_bridge legacy-key rejection now advises removing the
key entirely (vpcd auto-detects br-wan), not renaming to
dhcp_bind_bridge.

Two topology_test cases that exercise the DHCP gateway-LRP path are
skipped — Slice C deletes attachIGWLegacy and these tests together.
This commit is contained in:
joshsiv-mulga
2026-05-22 11:07:15 +10:00
parent a6b8b447ea
commit 3d2444ee00
12 changed files with 227 additions and 193 deletions
+25 -74
View File
@@ -910,13 +910,11 @@ func runAdminInit(cmd *cobra.Command, args []string) {
externalPrefixLen = detected.WAN.PrefixLen
}
// Default mode: always "pool" — with static range if --external-pool
// is given, otherwise with source=dhcp (gateway IP from router DHCP)
// Default mode: always "pool". Source defaults to "static"; if
// --external-pool is omitted the validator below will error with
// a SuggestPoolRange hint.
if externalMode == "" && !cmd.Flags().Changed("external-mode") {
externalMode = "pool"
if externalPool == "" && externalSource == "" {
externalSource = "dhcp"
}
}
}
}
@@ -928,42 +926,29 @@ func runAdminInit(cmd *cobra.Command, args []string) {
os.Exit(1)
}
if externalMode == "pool" {
// Resolve source: if not explicitly set, infer from whether a pool range was given
if externalSource == "" {
if externalPool != "" {
externalSource = "static"
} else {
externalSource = "dhcp"
}
if externalSource != "" && externalSource != "static" {
fmt.Fprintf(os.Stderr, "❌ Error: --external-source must be 'static' (only supported value), got: %s\n", externalSource)
os.Exit(1)
}
if externalSource == "dhcp" {
// DHCP source: no static range needed, just gateway
if externalGateway == "" {
fmt.Fprintf(os.Stderr, "❌ Error: --external-gateway is required when --external-mode=pool\n")
os.Exit(1)
externalSource = "static"
if externalPool == "" {
fmt.Fprintf(os.Stderr, "❌ Error: --external-pool is required when --external-mode=pool (e.g., 192.168.1.150-192.168.1.250)\n")
if detectedNet != nil && detectedNet.WAN != nil {
sugStart, sugEnd := admin.SuggestPoolRange(detectedNet.WAN)
fmt.Fprintf(os.Stderr, " Suggested: --external-pool=%s-%s\n", sugStart, sugEnd)
}
} else {
// Static source: need pool range
if externalPool == "" {
fmt.Fprintf(os.Stderr, "❌ Error: --external-pool is required when --external-mode=pool (e.g., 192.168.1.150-192.168.1.250)\n")
fmt.Fprintf(os.Stderr, " Or use --external-source=dhcp to get IPs from router DHCP\n")
if detectedNet != nil && detectedNet.WAN != nil {
sugStart, sugEnd := admin.SuggestPoolRange(detectedNet.WAN)
fmt.Fprintf(os.Stderr, " Suggested: --external-pool=%s-%s\n", sugStart, sugEnd)
}
os.Exit(1)
}
if externalGateway == "" {
fmt.Fprintf(os.Stderr, "❌ Error: --external-gateway is required when --external-mode=pool\n")
os.Exit(1)
}
parts := strings.SplitN(externalPool, "-", 2)
if len(parts) != 2 || net.ParseIP(parts[0]) == nil || net.ParseIP(parts[1]) == nil {
fmt.Fprintf(os.Stderr, "❌ Error: --external-pool must be start-end IPs (e.g., 192.168.1.150-192.168.1.250), got: %s\n", externalPool)
os.Exit(1)
}
poolStart, poolEnd = parts[0], parts[1]
os.Exit(1)
}
if externalGateway == "" {
fmt.Fprintf(os.Stderr, "❌ Error: --external-gateway is required when --external-mode=pool\n")
os.Exit(1)
}
parts := strings.SplitN(externalPool, "-", 2)
if len(parts) != 2 || net.ParseIP(parts[0]) == nil || net.ParseIP(parts[1]) == nil {
fmt.Fprintf(os.Stderr, "❌ Error: --external-pool must be start-end IPs (e.g., 192.168.1.150-192.168.1.250), got: %s\n", externalPool)
os.Exit(1)
}
poolStart, poolEnd = parts[0], parts[1]
}
if externalGateway != "" && net.ParseIP(externalGateway) == nil {
fmt.Fprintf(os.Stderr, "❌ Error: --external-gateway is not a valid IP: %s\n", externalGateway)
@@ -983,10 +968,6 @@ func runAdminInit(cmd *cobra.Command, args []string) {
}
}
// For pool/dhcp source, the gateway IP is obtained via DHCP on the
// WAN bridge interface (no static pool range or explicit gateway-ip).
useExternalDHCP := externalMode == "pool" && externalSource == "dhcp" && gatewayIP == ""
// Validate IP address format
if net.ParseIP(bindIP) == nil {
fmt.Fprintf(os.Stderr, "❌ Error: Invalid IP address for --bind: %s\n", bindIP)
@@ -1133,7 +1114,6 @@ func runAdminInit(cmd *cobra.Command, args []string) {
bootstrapIgwId := utils.GenerateResourceID("igw")
networkConfig = &formation.NetworkConfig{
ExternalMode: externalMode,
ExternalDHCP: useExternalDHCP,
PoolName: "wan",
PoolSource: externalSource,
PoolStart: poolStart,
@@ -1248,8 +1228,6 @@ func runAdminInit(cmd *cobra.Command, args []string) {
ExternalMode: externalMode,
ExternalIface: externalIface,
DhcpBindBridge: detectedDhcpBindBridge(detectedNet),
ExternalDHCP: useExternalDHCP,
PoolName: "wan",
PoolSource: externalSource,
PoolStart: poolStart,
@@ -1278,10 +1256,8 @@ func runAdminInit(cmd *cobra.Command, args []string) {
fmt.Printf(" Source: static (IP range)\n")
fmt.Printf(" IP pool: %s - %s\n", poolStart, poolEnd)
fmt.Printf(" ⚠️ Ensure %s-%s is excluded from your router's DHCP range.\n", poolStart, poolEnd)
} else if useExternalDHCP {
fmt.Printf(" Source: dhcp (from router DHCP)\n")
fmt.Println(" Gateway IP: DHCP (obtained during OVN setup)")
} else if gatewayIP != "" {
}
if gatewayIP != "" {
fmt.Printf(" Gateway IP: %s (static)\n", gatewayIP)
}
}
@@ -2271,7 +2247,6 @@ func finalizeNodeSetup(dataDir, certPath, adminAccessKey, adminSecretKey, region
// NetworkConfig into ConfigSettings and auto-detects the local WAN interface.
func applyNetworkConfig(settings *admin.ConfigSettings, nc *formation.NetworkConfig) {
settings.ExternalMode = nc.ExternalMode
settings.ExternalDHCP = nc.ExternalDHCP
settings.PoolName = nc.PoolName
settings.PoolSource = nc.PoolSource
settings.PoolStart = nc.PoolStart
@@ -2292,7 +2267,6 @@ func applyNetworkConfig(settings *admin.ConfigSettings, nc *formation.NetworkCon
detected, err := admin.DetectNetwork()
if err == nil && detected.WAN != nil {
settings.ExternalIface = detected.WAN.Name
settings.DhcpBindBridge = detectedDhcpBindBridge(detected)
}
}
}
@@ -2401,29 +2375,6 @@ func writeBootstrapFilesWithAdmin(configDir, bootstrapDir string, masterKey []by
return handlers_iam.SaveBootstrapData(filepath.Join(bootstrapDir, "bootstrap.json"), bd)
}
// detectedDhcpBindBridge returns the bridge name where the vpcd DHCP client
// should bind its AF_PACKET socket — i.e. the interface that physically sees
// LAN DHCP traffic.
//
// - Default route on a bridge (Linux or OVS, `br-*` prefix): return the
// bridge name verbatim. In veth mode this is the Linux bridge holding the
// WAN NIC; in direct mode this is the OVS bridge holding the WAN NIC.
// - Default route on a bare physical NIC: return "br-wan" as the name the
// installer will create.
//
// Never returns "br-ext". br-ext is the OVN-side bridge owned by
// setup-ovn.sh's ovn-bridge-mappings and never sees LAN DHCP frames.
func detectedDhcpBindBridge(detected *admin.DetectedNetwork) string {
if detected == nil || detected.WAN == nil {
return ""
}
name := detected.WAN.Name
if strings.HasPrefix(name, "br-") {
return name
}
return "br-wan"
}
// detectDNSServers auto-detects DNS servers from the host for the specified
// interface. Uses resolvectl (systemd-resolved) first, then falls back to
// /etc/resolv.conf. Returns up to 3 servers. Falls back to public DNS if none found.
+3 -46
View File
@@ -142,49 +142,6 @@ func TestSpinifexTomlTemplate_AdvertiseOmittedWhenEmpty(t *testing.T) {
assert.NotContains(t, string(data), "advertise =")
}
// detectedDhcpBindBridge must return the default-route interface name verbatim
// when it's a bridge (Linux or OVS, br-* prefix). The old detectedWanBridge()
// returned hardcoded "br-ext" for Linux bridges — that value broke DHCP on
// consumer-router LANs because br-ext never sees LAN DHCP traffic (mulga-998).
func TestDetectedDhcpBindBridge(t *testing.T) {
tests := []struct {
name string
detected *admin.DetectedNetwork
want string
}{
{
name: "Linux bridge default route returns bridge name (not 'br-ext')",
detected: &admin.DetectedNetwork{WAN: &admin.DetectedInterface{Name: "br-wan"}},
want: "br-wan",
},
{
name: "OVS bridge default route returned verbatim",
detected: &admin.DetectedNetwork{WAN: &admin.DetectedInterface{Name: "br-ext"}},
want: "br-ext",
},
{
name: "physical NIC defaults to 'br-wan' (bridge the installer creates)",
detected: &admin.DetectedNetwork{WAN: &admin.DetectedInterface{Name: "enp0s3"}},
want: "br-wan",
},
{
name: "nil DetectedNetwork",
detected: nil,
want: "",
},
{
name: "DetectedNetwork with nil WAN",
detected: &admin.DetectedNetwork{},
want: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, detectedDhcpBindBridge(tt.detected))
})
}
}
// Legacy `wan_bridge` TOML key must fail-start vpcd with guidance, not silently
// alias (per mulga-998 D3). Prevents the footgun where operators inherited the
// old key pointing at 'br-ext' and got broken DHCP on veth-mode hosts.
@@ -206,8 +163,8 @@ wan_bridge = "br-ext"
err := checkLegacyWanBridgeKey("node1", cfgPath)
require.Error(t, err)
assert.Contains(t, err.Error(), "dhcp_bind_bridge")
assert.Contains(t, err.Error(), "wan_bridge")
assert.Contains(t, err.Error(), "Remove")
}
func TestCheckLegacyWanBridgeKey_EnvVarRejected(t *testing.T) {
@@ -218,7 +175,7 @@ func TestCheckLegacyWanBridgeKey_EnvVarRejected(t *testing.T) {
err := checkLegacyWanBridgeKey("node1", "/tmp/unused.toml")
require.Error(t, err)
assert.Contains(t, err.Error(), "SPINIFEX_VPCD_WAN_BRIDGE")
assert.Contains(t, err.Error(), "dhcp_bind_bridge")
assert.Contains(t, err.Error(), "Remove")
}
func TestCheckLegacyWanBridgeKey_CleanConfigPasses(t *testing.T) {
@@ -228,7 +185,7 @@ func TestCheckLegacyWanBridgeKey_CleanConfigPasses(t *testing.T) {
node = "node1"
[nodes.node1.vpcd]
ovn_nb_addr = "tcp:127.0.0.1:6641"
dhcp_bind_bridge = "br-wan"
external_interface = "enp0s3"
`), 0o644))
viper.Reset()
+3 -10
View File
@@ -678,11 +678,8 @@ var spinifexUIStatusCmd = &cobra.Command{
}
// checkLegacyWanBridgeKey fails vpcd startup if the deprecated `wan_bridge`
// TOML key or `SPINIFEX_VPCD_WAN_BRIDGE` env-var is present. The key was
// renamed to `dhcp_bind_bridge` (see mulga-998.a) because the old name misled
// operators into pointing at the OVN-side bridge ("br-ext"), which never sees
// LAN DHCP traffic. Per D3: no silent alias, no auto-rewrite — operator must
// rename the key before vpcd will start.
// TOML key or `SPINIFEX_VPCD_WAN_BRIDGE` env-var is present. Per D3: no silent
// alias, no auto-rewrite — operator must remove the key before vpcd will start.
func checkLegacyWanBridgeKey(node, cfgFile string) error {
legacyInTOML := viper.IsSet("nodes." + node + ".vpcd.wan_bridge")
legacyInEnv := os.Getenv("SPINIFEX_VPCD_WAN_BRIDGE") != ""
@@ -695,9 +692,7 @@ func checkLegacyWanBridgeKey(node, cfgFile string) error {
}
return fmt.Errorf(
"vpcd: deprecated 'wan_bridge' key found in %s. "+
"Rename 'wan_bridge' to 'dhcp_bind_bridge'. The value may also be wrong — verify it is "+
"the Linux bridge holding your WAN NIC (veth mode) or the OVS bridge holding your WAN NIC (direct mode); "+
"never 'br-ext'. Typical value on a consumer-router LAN: 'br-wan'. "+
"Remove the key entirely; vpcd auto-detects the WAN bridge (br-wan). "+
"Then: sudo systemctl restart spinifex-vpcd",
source,
)
@@ -742,7 +737,6 @@ var vpcdStartCmd = &cobra.Command{
DNSServers: p.DNSServers,
Region: p.Region,
AZ: p.AZ,
DhcpBindBridge: nodeConfig.VPCD.DhcpBindBridge,
GwLrpRangeStart: p.GwLrpRangeStart,
GwLrpRangeEnd: p.GwLrpRangeEnd,
})
@@ -778,7 +772,6 @@ var vpcdStartCmd = &cobra.Command{
ExternalPools: extPools,
Bootstrap: bootstrap,
ExternalInterface: nodeConfig.VPCD.ExternalInterface,
DhcpBindBridge: nodeConfig.VPCD.DhcpBindBridge,
BridgeMode: nodeConfig.VPCD.BridgeMode,
AZ: nodeConfig.AZ,
})
-12
View File
@@ -11,9 +11,6 @@ email = "{{.OperatorEmail}}"
[network]
external_mode = "{{.ExternalMode}}"
{{- if .ExternalDHCP}}
external_dhcp = true
{{- end}}
[[network.external_pools]]
name = "{{.PoolName}}"
@@ -97,15 +94,6 @@ ovn_sb_addr = "{{.OVNSBAddr}}"
{{- if .ExternalIface}}
external_interface = "{{.ExternalIface}}"
{{- end}}
{{- if .DhcpBindBridge}}
# dhcp_bind_bridge: the bridge where vpcd's DHCP client binds its AF_PACKET
# socket. On Linux-bridge-WAN hosts (netplan default with the WAN NIC enslaved),
# this is the Linux bridge name (e.g. "br-wan"). On direct-OVS hosts, this is
# the OVS bridge holding the WAN NIC.
# NEVER set this to "br-ext" — that is the OVN-side bridge and never sees LAN
# DHCP traffic.
dhcp_bind_bridge = "{{.DhcpBindBridge}}"
{{- end}}
[nodes.{{.Node}}.predastore]
host = "{{.BindIP}}:8443"
+4 -6
View File
@@ -72,14 +72,12 @@ type ConfigSettings struct {
// External networking for public subnets
ExternalMode string // "pool" or "" (disabled)
ExternalIface string // WAN NIC name (e.g., "eth0", "eth1")
DhcpBindBridge string // Bridge where the DHCP AF_PACKET socket binds (Linux bridge in veth mode, OVS bridge in direct mode; never "br-ext")
ExternalDHCP bool // Obtain gateway IP via DHCP on the WAN bridge
PoolName string // External pool name (e.g., "wan")
PoolSource string // IP source: "static" (default) or "dhcp" (from router DHCP)
PoolStart string // First IP in external pool range (static source only)
PoolEnd string // Last IP in external pool range (static source only)
PoolSource string // IP source: "static" (default; only valid value)
PoolStart string // First IP in external pool range
PoolEnd string // Last IP in external pool range
PoolGateway string // WAN gateway IP
PoolGatewayIP string // Explicit SNAT IP (for nat mode without DHCP)
PoolGatewayIP string // Explicit SNAT IP (overrides default of first IP in range)
PoolPrefixLen int // Subnet prefix length (default 24)
PoolDNSServers []string // DNS servers for VM DHCP (auto-detected from host)
+64 -10
View File
@@ -3,6 +3,7 @@ package config
import (
"fmt"
"log/slog"
"net/netip"
"os"
"slices"
"strings"
@@ -22,7 +23,7 @@ type ClusterConfig struct {
// ExternalPool defines a range of routable IPs that Spinifex manages for public subnets.
type ExternalPool struct {
Name string `mapstructure:"name"` // Pool identifier (e.g., "wan", "dc1-primary")
Source string `mapstructure:"source"` // IP source: "static" (default) or "dhcp" (from router DHCP)
Source string `mapstructure:"source"` // IP source: "static" (default; only valid value)
RangeStart string `mapstructure:"range_start"` // First IP in range (static source only)
RangeEnd string `mapstructure:"range_end"` // Last IP in range (static source only)
Gateway string `mapstructure:"gateway"` // WAN default gateway (next hop for 0.0.0.0/0)
@@ -44,7 +45,6 @@ type ExternalPool struct {
// NetworkConfig holds cluster-wide external network settings.
type NetworkConfig struct {
ExternalMode string `mapstructure:"external_mode"` // "pool" or "" (disabled)
ExternalDHCP bool `mapstructure:"external_dhcp"` // Gateway IP obtained via DHCP (pool/dhcp source)
ExternalPools []ExternalPool `mapstructure:"external_pools"` // One or more IP pools
}
@@ -103,14 +103,7 @@ type VPCDConfig struct {
OVNNBAddr string `json:"OVNNBAddr" mapstructure:"ovn_nb_addr"` // OVN Northbound DB address (e.g., "tcp:127.0.0.1:6641")
OVNSBAddr string `json:"OVNSBAddr" mapstructure:"ovn_sb_addr"` // OVN Southbound DB address (e.g., "tcp:127.0.0.1:6642")
ExternalInterface string `json:"ExternalInterface" mapstructure:"external_interface"` // WAN NIC name (e.g., "eth1", "enp0s3") — the physical NIC on the WAN bridge
// DhcpBindBridge is the bridge where the DHCP client binds its AF_PACKET
// socket — the interface that physically sees LAN DHCP traffic. On hosts
// where the WAN NIC is enslaved to a Linux bridge (netplan default), this
// is the Linux bridge name (e.g. "br-wan"). On direct-OVS hosts, it is
// the OVS bridge holding the WAN NIC. Never set to the OVN-side bridge
// ("br-ext") — that never sees LAN DHCP traffic.
DhcpBindBridge string `json:"DhcpBindBridge" mapstructure:"dhcp_bind_bridge"`
BridgeMode string `json:"BridgeMode" mapstructure:"bridge_mode"` // "direct" or "veth" (auto-detected if empty)
BridgeMode string `json:"BridgeMode" mapstructure:"bridge_mode"` // "direct" or "veth" (auto-detected if empty)
}
type PredastoreConfig struct {
@@ -244,5 +237,66 @@ func LoadConfig(configPath string) (*ClusterConfig, error) {
}
}
if err := validateClusterConfig(&config); err != nil {
return nil, err
}
return &config, nil
}
// validateClusterConfig rejects legacy upstream-DHCP config and validates
// external pool ranges. The upstream-DHCP-client model was replaced by static
// WAN-pool allocation; legacy keys must be removed from the operator's TOML.
func validateClusterConfig(cc *ClusterConfig) error {
if viper.IsSet("network.external_dhcp") {
return fmt.Errorf("config: [network] external_dhcp is no longer supported; remove the key (static WAN-pool allocation only)")
}
for nodeName := range cc.Nodes {
if viper.IsSet("nodes." + nodeName + ".vpcd.dhcp_bind_bridge") {
return fmt.Errorf("config: [nodes.%s.vpcd] dhcp_bind_bridge is no longer supported; remove the key (vpcd no longer runs a DHCP client)", nodeName)
}
}
type poolRange struct {
name string
start netip.Addr
end netip.Addr
}
var ranges []poolRange
for _, p := range cc.Network.ExternalPools {
if p.Source != "" && p.Source != "static" {
return fmt.Errorf("config: [[network.external_pools]] %q: source=%q is no longer supported; use source=\"static\" with explicit range_start/range_end", p.Name, p.Source)
}
if p.RangeStart == "" || p.RangeEnd == "" {
continue
}
start, err := netip.ParseAddr(p.RangeStart)
if err != nil {
return fmt.Errorf("config: pool %q range_start %q: %w", p.Name, p.RangeStart, err)
}
end, err := netip.ParseAddr(p.RangeEnd)
if err != nil {
return fmt.Errorf("config: pool %q range_end %q: %w", p.Name, p.RangeEnd, err)
}
if start.Compare(end) > 0 {
return fmt.Errorf("config: pool %q range_start %s > range_end %s", p.Name, start, end)
}
if p.Gateway != "" && p.PrefixLen > 0 {
gw, err := netip.ParseAddr(p.Gateway)
if err != nil {
return fmt.Errorf("config: pool %q gateway %q: %w", p.Name, p.Gateway, err)
}
cidr := netip.PrefixFrom(gw, p.PrefixLen).Masked()
if !cidr.Contains(start) || !cidr.Contains(end) {
return fmt.Errorf("config: pool %q range [%s, %s] not inside %s", p.Name, start, end, cidr)
}
}
for _, prior := range ranges {
if start.Compare(prior.end) <= 0 && prior.start.Compare(end) <= 0 {
return fmt.Errorf("config: pool %q range [%s, %s] overlaps pool %q [%s, %s]", p.Name, start, end, prior.name, prior.start, prior.end)
}
}
ranges = append(ranges, poolRange{name: p.Name, start: start, end: end})
}
return nil
}
+111 -6
View File
@@ -375,7 +375,7 @@ external_interface = "enp0s3"
assert.Equal(t, "enp0s3", n.VPCD.ExternalInterface)
}
func TestLoadConfig_NetworkPoolDHCPMode(t *testing.T) {
func TestLoadConfig_NetworkPoolDHCPSourceRejected(t *testing.T) {
resetViper(t)
dir := t.TempDir()
path := filepath.Join(dir, "spinifex.toml")
@@ -399,12 +399,117 @@ region = "us-east-1"
require.NoError(t, os.WriteFile(path, []byte(toml), 0600))
cfg, err := LoadConfig(path)
require.NoError(t, err)
require.Error(t, err)
assert.Nil(t, cfg)
assert.Contains(t, err.Error(), "source=")
assert.Contains(t, err.Error(), "dhcp")
}
assert.Equal(t, "pool", cfg.Network.ExternalMode)
require.Len(t, cfg.Network.ExternalPools, 1)
assert.Equal(t, "dhcp", cfg.Network.ExternalPools[0].Source)
assert.Equal(t, "192.168.1.100", cfg.Network.ExternalPools[0].GatewayIP)
func TestLoadConfig_ExternalDHCPRejected(t *testing.T) {
resetViper(t)
dir := t.TempDir()
path := filepath.Join(dir, "spinifex.toml")
toml := `
node = "n1"
[network]
external_mode = "pool"
external_dhcp = true
[nodes.n1]
region = "us-east-1"
`
require.NoError(t, os.WriteFile(path, []byte(toml), 0600))
cfg, err := LoadConfig(path)
require.Error(t, err)
assert.Nil(t, cfg)
assert.Contains(t, err.Error(), "external_dhcp")
}
func TestLoadConfig_DhcpBindBridgeRejected(t *testing.T) {
resetViper(t)
dir := t.TempDir()
path := filepath.Join(dir, "spinifex.toml")
toml := `
node = "n1"
[nodes.n1.vpcd]
dhcp_bind_bridge = "br-wan"
`
require.NoError(t, os.WriteFile(path, []byte(toml), 0600))
cfg, err := LoadConfig(path)
require.Error(t, err)
assert.Nil(t, cfg)
assert.Contains(t, err.Error(), "dhcp_bind_bridge")
}
func TestLoadConfig_PoolRangeOutsideCIDR(t *testing.T) {
resetViper(t)
dir := t.TempDir()
path := filepath.Join(dir, "spinifex.toml")
toml := `
node = "n1"
[network]
external_mode = "pool"
[[network.external_pools]]
name = "wan"
range_start = "10.99.0.10"
range_end = "10.99.0.50"
gateway = "192.168.1.1"
prefix_len = 24
[nodes.n1]
region = "us-east-1"
`
require.NoError(t, os.WriteFile(path, []byte(toml), 0600))
cfg, err := LoadConfig(path)
require.Error(t, err)
assert.Nil(t, cfg)
assert.Contains(t, err.Error(), "not inside")
}
func TestLoadConfig_PoolsOverlap(t *testing.T) {
resetViper(t)
dir := t.TempDir()
path := filepath.Join(dir, "spinifex.toml")
toml := `
node = "n1"
[network]
external_mode = "pool"
[[network.external_pools]]
name = "a"
range_start = "192.168.1.10"
range_end = "192.168.1.50"
gateway = "192.168.1.1"
prefix_len = 24
[[network.external_pools]]
name = "b"
range_start = "192.168.1.40"
range_end = "192.168.1.80"
gateway = "192.168.1.1"
prefix_len = 24
[nodes.n1]
region = "us-east-1"
`
require.NoError(t, os.WriteFile(path, []byte(toml), 0600))
cfg, err := LoadConfig(path)
require.Error(t, err)
assert.Nil(t, cfg)
assert.Contains(t, err.Error(), "overlaps")
}
func TestLoadConfig_NetworkDisabledByDefault(t *testing.T) {
-6
View File
@@ -1097,11 +1097,6 @@ func (d *Daemon) startCluster() error {
slog.Warn("Failed to get JetStream for external IPAM", "err", jsErr)
} else {
var pools []handlers_ec2_vpc.ExternalPoolConfig
// Resolve DHCP bind bridge name for DHCP pools (where AF_PACKET binds).
dhcpBindBridge := ""
if node, ok := d.clusterConfig.Nodes[d.clusterConfig.Node]; ok {
dhcpBindBridge = node.VPCD.DhcpBindBridge
}
gwMAC := ""
if d.clusterConfig.Bootstrap.VpcId != "" {
gwMAC = utils.HashMAC("gw-" + d.clusterConfig.Bootstrap.VpcId)
@@ -1117,7 +1112,6 @@ func (d *Daemon) startCluster() error {
PrefixLen: p.PrefixLen,
Region: p.Region,
AZ: p.AZ,
DhcpBindBridge: dhcpBindBridge,
GatewayMAC: gwMAC,
GwLrpRangeStart: p.GwLrpRangeStart,
GwLrpRangeEnd: p.GwLrpRangeEnd,
-1
View File
@@ -62,7 +62,6 @@ type StatusResponse struct {
// propagated from the init node to joining nodes during formation.
type NetworkConfig struct {
ExternalMode string `json:"external_mode"`
ExternalDHCP bool `json:"external_dhcp,omitempty"`
PoolName string `json:"pool_name"`
PoolSource string `json:"pool_source,omitempty"`
PoolStart string `json:"pool_start,omitempty"`
+2
View File
@@ -1442,6 +1442,7 @@ func newDHCPStub(t *testing.T, nc *nats.Conn) *dhcpStub {
// sharing one DHCP pool must each get a distinct lease IP and persist it
// on external_ids:spinifex:gateway_ip. IGW detach must release the lease.
func TestTopologyHandler_IGWAttach_DHCPGwLrpIP(t *testing.T) {
t.Skip("DHCP source no longer supported; Slice C deletes attachIGWLegacy + DHCP gateway-LRP path")
_, nc := startTestNATS(t)
stub := newDHCPStub(t, nc)
mock := NewMockOVNClient()
@@ -1542,6 +1543,7 @@ func TestTopologyHandler_IGWAttach_DHCPGwLrpIP(t *testing.T) {
// loud instead of falling back to link-local. Otherwise we silently restore
// the broken pre-siv-36 behavior.
func TestTopologyHandler_IGWAttach_DHCPAcquireFailureAborts(t *testing.T) {
t.Skip("DHCP source no longer supported; Slice C deletes attachIGWLegacy + DHCP gateway-LRP path")
_, nc := startTestNATS(t)
sub, err := nc.Subscribe(dhcp.TopicAcquire, func(msg *nats.Msg) {
reply := dhcp.AcquireReplyMsg{Error: "no leases available"}
+10 -17
View File
@@ -73,11 +73,6 @@ type Config struct {
// ExternalInterface is the WAN NIC name (e.g., "enp0s3"). Used by
// detectBridgeMode for veth/direct auto-detection.
ExternalInterface string
// DhcpBindBridge is the bridge where the DHCP client binds its AF_PACKET
// socket. In veth mode this is the Linux bridge that holds the WAN NIC
// (e.g. "br-wan"); in direct mode this is the OVS bridge holding the
// WAN NIC. Never the OVN-side "br-ext" — that never sees LAN DHCP.
DhcpBindBridge string
// BridgeMode is "direct" or "veth". Direct bridge adds the WAN NIC
// directly to the OVS bridge; veth uses a veth pair to link a Linux bridge
// to OVS. When empty, auto-detected at startup.
@@ -105,9 +100,10 @@ type ExternalPoolConfig struct {
GwLrpRangeEnd string
}
// IsDHCP returns true if this pool obtains IPs from upstream DHCP.
// IsDHCP is retained until Slice C removes the DHCP-source code paths; loader
// rejects source="dhcp" so this always returns false in steady state.
func (p *ExternalPoolConfig) IsDHCP() bool {
return p.Source == "dhcp"
return false
}
// Service implements the Spinifex service interface for vpcd.
@@ -417,7 +413,7 @@ func launchService(cfg *Config) error {
defer liveClient.Close()
slog.Info("Connected to OVN NB DB", "endpoint", cfg.OVNNBAddr)
bridgeMode, dhcpBindBridge := resolveBridgeConfig(cfg.BridgeMode, cfg.ExternalInterface, cfg.DhcpBindBridge)
bridgeMode, dhcpBindBridge := resolveBridgeConfig(cfg.BridgeMode, cfg.ExternalInterface)
slog.Info("External bridge mode", "mode", bridgeMode, "dhcp_bind_bridge", dhcpBindBridge)
if err := verifyBridgeMode(bridgeMode, cfg.ExternalInterface, dhcpBindBridge); err != nil {
slog.Error("vpcd: bridge mode sanity check failed", "err", err)
@@ -679,20 +675,17 @@ func externalPoolConfigToShared(p ExternalPoolConfig) external.ExternalPoolConfi
}
}
// resolveBridgeConfig picks the bridge mode and DHCP-bind-bridge to use,
// resolveBridgeConfig picks the bridge mode and WAN bridge name to use,
// auto-detecting mode when unset. Empty mode stays empty — verifyBridgeMode
// rejects it with a list of supported values (D12). Empty bind bridge
// defaults to "br-wan", the consumer-router convention.
func resolveBridgeConfig(cfgBridgeMode, externalIface, cfgDhcpBindBridge string) (string, string) {
// rejects it with a list of supported values (D12). The WAN bridge name is
// always "br-wan" (the consumer-router convention) since vpcd no longer
// accepts a per-node override.
func resolveBridgeConfig(cfgBridgeMode, externalIface string) (string, string) {
bridgeMode := cfgBridgeMode
if bridgeMode == "" && externalIface != "" {
bridgeMode = detectBridgeMode(externalIface)
}
dhcpBindBridge := cfgDhcpBindBridge
if dhcpBindBridge == "" {
dhcpBindBridge = "br-wan"
}
return bridgeMode, dhcpBindBridge
return bridgeMode, "br-wan"
}
// ifaceExists returns true when the kernel reports the named link.
+5 -5
View File
@@ -311,7 +311,7 @@ func TestDetectBridgeMode_FallthroughDirect(t *testing.T) {
func TestResolveBridgeConfig_UsesExplicitMode(t *testing.T) {
stubDetectProbes(t, nil)
mode, br := resolveBridgeConfig(BridgeModeVeth, "enp0s3", "br-wan")
mode, br := resolveBridgeConfig(BridgeModeVeth, "enp0s3")
if mode != BridgeModeVeth || br != "br-wan" {
t.Errorf("got (%q,%q), want (%q,br-wan)", mode, br, BridgeModeVeth)
}
@@ -319,23 +319,23 @@ func TestResolveBridgeConfig_UsesExplicitMode(t *testing.T) {
func TestResolveBridgeConfig_AutoDetects(t *testing.T) {
stubDetectProbes(t, []string{"veth-wan-ovs"})
mode, _ := resolveBridgeConfig("", "enp0s3", "br-wan")
mode, _ := resolveBridgeConfig("", "enp0s3")
if mode != BridgeModeVeth {
t.Errorf("want auto-detect veth, got %q", mode)
}
}
func TestResolveBridgeConfig_EmptyStaysEmptyWithNoIface(t *testing.T) {
mode, _ := resolveBridgeConfig("", "", "")
mode, _ := resolveBridgeConfig("", "")
if mode != "" {
t.Errorf("empty mode + no iface should stay empty (D12); got %q", mode)
}
}
func TestResolveBridgeConfig_DefaultsBindBridge(t *testing.T) {
_, br := resolveBridgeConfig(BridgeModeDirect, "enp0s3", "")
_, br := resolveBridgeConfig(BridgeModeDirect, "enp0s3")
if br != "br-wan" {
t.Errorf("empty dhcp_bind_bridge should default to br-wan, got %q", br)
t.Errorf("WAN bridge should default to br-wan, got %q", br)
}
}