From 3d2444ee00f25a81276ebbcd18b968c0e094e165 Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Fri, 22 May 2026 11:07:15 +1000 Subject: [PATCH] refactor(config): drop upstream-DHCP config surface (static WAN pool only) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hard-error in LoadConfig on legacy keys: [network] external_dhcp, [nodes..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. --- cmd/spinifex/cmd/admin.go | 99 +++++-------------- cmd/spinifex/cmd/admin_test.go | 49 +--------- cmd/spinifex/cmd/service.go | 13 +-- cmd/spinifex/cmd/templates/spinifex.toml | 12 --- spinifex/admin/admin.go | 10 +- spinifex/config/config.go | 74 ++++++++++++-- spinifex/config/config_test.go | 117 +++++++++++++++++++++-- spinifex/daemon/daemon.go | 6 -- spinifex/formation/formation.go | 1 - spinifex/services/vpcd/topology_test.go | 2 + spinifex/services/vpcd/vpcd.go | 27 ++---- spinifex/services/vpcd/vpcd_test.go | 10 +- 12 files changed, 227 insertions(+), 193 deletions(-) diff --git a/cmd/spinifex/cmd/admin.go b/cmd/spinifex/cmd/admin.go index 8780fb873..6c2cf457e 100644 --- a/cmd/spinifex/cmd/admin.go +++ b/cmd/spinifex/cmd/admin.go @@ -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. diff --git a/cmd/spinifex/cmd/admin_test.go b/cmd/spinifex/cmd/admin_test.go index 9a5e9f0da..b5b9dd80a 100644 --- a/cmd/spinifex/cmd/admin_test.go +++ b/cmd/spinifex/cmd/admin_test.go @@ -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() diff --git a/cmd/spinifex/cmd/service.go b/cmd/spinifex/cmd/service.go index 103d8314b..b07f44bdc 100644 --- a/cmd/spinifex/cmd/service.go +++ b/cmd/spinifex/cmd/service.go @@ -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, }) diff --git a/cmd/spinifex/cmd/templates/spinifex.toml b/cmd/spinifex/cmd/templates/spinifex.toml index f341002f1..a93d402d5 100644 --- a/cmd/spinifex/cmd/templates/spinifex.toml +++ b/cmd/spinifex/cmd/templates/spinifex.toml @@ -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" diff --git a/spinifex/admin/admin.go b/spinifex/admin/admin.go index fea3fd251..400d981ea 100644 --- a/spinifex/admin/admin.go +++ b/spinifex/admin/admin.go @@ -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) diff --git a/spinifex/config/config.go b/spinifex/config/config.go index 3b5a5960c..ab8d766e3 100644 --- a/spinifex/config/config.go +++ b/spinifex/config/config.go @@ -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 +} diff --git a/spinifex/config/config_test.go b/spinifex/config/config_test.go index 6c2a9844d..002058371 100644 --- a/spinifex/config/config_test.go +++ b/spinifex/config/config_test.go @@ -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) { diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index b875d173f..546eb4a61 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -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, diff --git a/spinifex/formation/formation.go b/spinifex/formation/formation.go index d0e72ced2..54000bd41 100644 --- a/spinifex/formation/formation.go +++ b/spinifex/formation/formation.go @@ -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"` diff --git a/spinifex/services/vpcd/topology_test.go b/spinifex/services/vpcd/topology_test.go index 29b65428a..7b649cd39 100644 --- a/spinifex/services/vpcd/topology_test.go +++ b/spinifex/services/vpcd/topology_test.go @@ -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"} diff --git a/spinifex/services/vpcd/vpcd.go b/spinifex/services/vpcd/vpcd.go index c4139e4c7..cdabb73f4 100644 --- a/spinifex/services/vpcd/vpcd.go +++ b/spinifex/services/vpcd/vpcd.go @@ -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. diff --git a/spinifex/services/vpcd/vpcd_test.go b/spinifex/services/vpcd/vpcd_test.go index d5f80c1a7..4bf974e84 100644 --- a/spinifex/services/vpcd/vpcd_test.go +++ b/spinifex/services/vpcd/vpcd_test.go @@ -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) } }