diff --git a/cmd/spinifex/cmd/admin.go b/cmd/spinifex/cmd/admin.go index 10b0dff31..d17aed25a 100644 --- a/cmd/spinifex/cmd/admin.go +++ b/cmd/spinifex/cmd/admin.go @@ -34,6 +34,7 @@ import ( "github.com/mulgadc/spinifex/spinifex/gpu" handlers_ec2_vpc "github.com/mulgadc/spinifex/spinifex/handlers/ec2/vpc" handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" + "github.com/mulgadc/spinifex/spinifex/hostdns" "github.com/mulgadc/spinifex/spinifex/network/host" "github.com/mulgadc/spinifex/spinifex/objectstore" "github.com/mulgadc/spinifex/spinifex/utils" @@ -339,6 +340,7 @@ func init() { adminInitCmd.Flags().Int("external-prefix-len", 24, "External pool subnet prefix length (auto-detected)") adminInitCmd.Flags().Bool("gpu-passthrough", false, "Enable VFIO GPU passthrough (sets gpu_passthrough = true in daemon config)") adminInitCmd.Flags().Bool("ipsec", true, "Encrypt intra-AZ Geneve via OVN native IPsec (cluster-wide); disable only for trusted single-rack lab") + adminInitCmd.Flags().Bool("skip-host-dns", false, "Do not point this node's host resolver at its local northstar (LB/EKS names then won't resolve from the node)") // Flags for admin join adminJoinCmd.Flags().String("region", "ap-southeast-2", "Region for this node") @@ -355,6 +357,7 @@ func init() { adminJoinCmd.Flags().StringSlice("services", nil, "Services this node runs (default: all)") adminJoinCmd.Flags().Bool("no-telemetry", false, "Disable telemetry metrics sent during join (default: enabled)") adminJoinCmd.Flags().String("email", "", "Operator email address (used for update and security notifications)") + adminJoinCmd.Flags().Bool("skip-host-dns", false, "Do not point this node's host resolver at its local northstar (LB/EKS names then won't resolve from the node)") adminJoinCmd.Flags().Int("predastore-compaction-interval", 0, "Predastore compactor interval in seconds (0 = unset, uses built-in default). Test clusters set a short interval.") adminJoinCmd.MarkFlagRequired("node") adminJoinCmd.MarkFlagRequired("host") @@ -1208,10 +1211,31 @@ func runAdminInit(cmd *cobra.Command, args []string) { os.Exit(1) } - // Detect DNS servers from the host for VM DHCP + // Validate IP address format + if net.ParseIP(bindIP) == nil { + fmt.Fprintf(os.Stderr, "❌ Error: Invalid IP address for --bind: %s\n", bindIP) + os.Exit(1) + } + + // Resolve the off-host advertise IP before detecting DNS. DetectNetwork may + // have failed earlier, so retry lazily when the listener still needs a WAN IP. + if advertiseFlag == "" && (bindIP == "0.0.0.0" || bindIP == "127.0.0.1") && detectedNet == nil { + if d, derr := admin.DetectNetwork(); derr == nil { + detectedNet = d + } + } + advertiseIP, err := resolveAdvertiseIP(bindIP, advertiseFlag, detectedNet) + if err != nil { + fmt.Fprintf(os.Stderr, "❌ Error: %v\n", err) + os.Exit(1) + } + + // Exclude the node-local Northstar listener from its own upstreams. On a + // forced re-init, resolvconf may still list this address first; forwarding it + // back to Northstar creates a recursive DNS loop. var dnsServers []string if externalMode != "" { - dnsServers = detectDNSServers(externalIface) + dnsServers = detectDNSServers(externalIface, advertiseIP) if len(dnsServers) > 0 { fmt.Printf(" DNS servers: %s\n", strings.Join(dnsServers, ", ")) } @@ -1247,25 +1271,6 @@ func runAdminInit(cmd *cobra.Command, args []string) { }) } - // Validate IP address format - if net.ParseIP(bindIP) == nil { - fmt.Fprintf(os.Stderr, "❌ Error: Invalid IP address for --bind: %s\n", bindIP) - os.Exit(1) - } - - // Resolve the off-host advertise IP. DetectNetwork may have failed earlier; - // detect lazily if we still need the WAN IP. - if advertiseFlag == "" && (bindIP == "0.0.0.0" || bindIP == "127.0.0.1") && detectedNet == nil { - if d, derr := admin.DetectNetwork(); derr == nil { - detectedNet = d - } - } - advertiseIP, err := resolveAdvertiseIP(bindIP, advertiseFlag, detectedNet) - if err != nil { - fmt.Fprintf(os.Stderr, "❌ Error: %v\n", err) - os.Exit(1) - } - // Validate port range if port < 1 || port > 65535 { fmt.Fprintf(os.Stderr, "❌ Error: Port must be between 1 and 65535, got: %d\n", port) @@ -1651,6 +1656,9 @@ func runAdminInit(cmd *cobra.Command, args []string) { finalizeNodeSetup(spxRoot, certPath, adminAccessKey, adminSecretKey, region, bindIP) + skipHostDNS, _ := cmd.Flags().GetBool("skip-host-dns") + configureHostDNS(configSettings, skipHostDNS) + // Write node.conf so spx admin banner works on source installs (not just ISO). nodeHostname, _ := os.Hostname() if nodeHostname == "" { @@ -1895,6 +1903,9 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, finalizeNodeSetup(spxRoot, certPath, adminAccessKey, adminSecretKey, region, bindIP) + skipHostDNS, _ := cmd.Flags().GetBool("skip-host-dns") + configureHostDNS(configSettings, skipHostDNS) + // Keep formation server running briefly so joining nodes can fetch complete status fmt.Println("\n⏳ Waiting for joining nodes to fetch cluster data...") time.Sleep(15 * time.Second) @@ -2362,6 +2373,9 @@ func runAdminJoin(cmd *cobra.Command, args []string) { finalizeNodeSetup(dataDir, caCertPath, creds.AdminAccessKey, creds.AdminSecretKey, creds.Region, bindIP) + skipHostDNS, _ := cmd.Flags().GetBool("skip-host-dns") + configureHostDNS(configSettings, skipHostDNS) + // Print cluster summary fmt.Println("\n🎉 Node successfully joined cluster!") fmt.Printf(" Cluster: %s (%d nodes)\n", creds.ClusterName, len(statusResp.Nodes)) @@ -2722,6 +2736,45 @@ func generateAndWriteConfigs(dirs configDirs, spinifexTomlPath string, settings return admin.GenerateConfigFiles(configs, settings) } +// hostDNSParams derives the resolver target from the settings just rendered into +// northstar.toml and reports whether host DNS should be configured at all. Host +// DNS is pointed at the node's own northstar only when a northstar config was +// rendered (scoped DNS credentials present) and the operator did not opt out. +// The resolver is the node AdvertiseIP, falling back to BindIP. +func hostDNSParams(settings admin.ConfigSettings, skip bool) (hostdns.Params, bool) { + northstarEnabled := settings.NorthstarAccessKey != "" && settings.NorthstarSecretKey != "" + if !northstarEnabled || skip { + return hostdns.Params{}, false + } + resolverIP := settings.AdvertiseIP + if resolverIP == "" { + resolverIP = settings.BindIP + } + return hostdns.Params{ + ResolverIP: resolverIP, + BaseDomain: settings.NorthstarDefaultDomain, + InternalDomain: settings.NorthstarInternalDomain, + }, true +} + +// configureHostDNS points this node's host resolver at its own northstar so the +// Spinifex authoritative zones (ELB/EKS names) resolve from the node itself on +// every install path. Failures are surfaced loudly but never abort a formed +// cluster, mirroring installCACertificate. +func configureHostDNS(settings admin.ConfigSettings, skip bool) { + params, ok := hostDNSParams(settings, skip) + if !ok { + return + } + fmt.Println("\n🔧 Configuring host DNS for Spinifex zones...") + if err := hostdns.Configure(params); err != nil { + fmt.Fprintf(os.Stderr, "⚠️ Warning: could not configure host DNS: %v\n", err) + fmt.Fprintf(os.Stderr, " LB/EKS names may not resolve from this node until the host resolver points at %s:53.\n", params.ResolverIP) + return + } + fmt.Printf("✅ Host DNS: %s + %s -> %s:53 (northstar)\n", params.BaseDomain, params.InternalDomain, params.ResolverIP) +} + // finalizeNodeSetup configures AWS credentials, creates service directories, // and sets ownership when running as root. func finalizeNodeSetup(dataDir, certPath, adminAccessKey, adminSecretKey, region, bindIP string) { @@ -3027,15 +3080,36 @@ func bridgeModeFor(externalMode string) string { return "" } -// 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. -func detectDNSServers(iface string) []string { +// dnsDetectionCommandTimeout prevents a wedged resolver manager from blocking init. +const dnsDetectionCommandTimeout = 5 * time.Second + +// dnsDetectionSources isolates host resolver discovery for deterministic tests. +type dnsDetectionSources struct { + queryResolvectl func(args ...string) (string, error) + readResolvConf func() (string, error) +} + +// detectDNSServers auto-detects up to three host DNS servers, preferring +// systemd-resolved before resolv.conf and public fallbacks. +func detectDNSServers(iface string, excludedIPs ...string) []string { + return detectDNSServersWithSources(iface, excludedIPs, dnsDetectionSources{ + queryResolvectl: func(args ...string) (string, error) { + return utils.RunCommandWithTimeout(dnsDetectionCommandTimeout, "resolvectl", args...) + }, + readResolvConf: func() (string, error) { + data, err := os.ReadFile("/etc/resolv.conf") + return string(data), err + }, + }) +} + +// detectDNSServersWithSources finds usable upstreams in resolver-manager order. +func detectDNSServersWithSources(iface string, excludedIPs []string, sources dnsDetectionSources) []string { // Try resolvectl for the specific link first (most reliable on modern systems) if iface != "" { - out, err := exec.Command("resolvectl", "dns", iface).CombinedOutput() + out, err := sources.queryResolvectl("dns", iface) if err == nil { - servers := parseDNSFromResolvectl(string(out)) + servers := filterDNSServers(parseDNSFromResolvectl(out), excludedIPs...) if len(servers) > 0 { return servers } @@ -3043,38 +3117,66 @@ func detectDNSServers(iface string) []string { } // Try resolvectl global - out, err := exec.Command("resolvectl", "dns").CombinedOutput() + out, err := sources.queryResolvectl("dns") if err == nil { - servers := parseDNSFromResolvectl(string(out)) + servers := filterDNSServers(parseDNSFromResolvectl(out), excludedIPs...) if len(servers) > 0 { return servers } } // Fall back to /etc/resolv.conf - data, err := os.ReadFile("/etc/resolv.conf") + resolvConf, err := sources.readResolvConf() if err == nil { var servers []string - for line := range strings.SplitSeq(string(data), "\n") { + for line := range strings.SplitSeq(resolvConf, "\n") { line = strings.TrimSpace(line) if strings.HasPrefix(line, "nameserver ") { - ip := strings.TrimSpace(strings.TrimPrefix(line, "nameserver")) - // Skip localhost (systemd-resolved stub) - if ip != "127.0.0.53" && ip != "127.0.0.1" && net.ParseIP(ip) != nil { - servers = append(servers, ip) - } + servers = append(servers, strings.TrimSpace(strings.TrimPrefix(line, "nameserver"))) } } - if len(servers) > 0 { - if len(servers) > 3 { - servers = servers[:3] - } + if servers = filterDNSServers(servers, excludedIPs...); len(servers) > 0 { return servers } } // Fallback to well-known public DNS - return []string{"8.8.8.8", "1.1.1.1"} + return filterDNSServers([]string{"8.8.8.8", "1.1.1.1"}, excludedIPs...) +} + +// filterDNSServers removes local, duplicate, and invalid resolver addresses and +// caps the upstream list at the three entries supported by generated configs. +func filterDNSServers(servers []string, excludedIPs ...string) []string { + excluded := make(map[string]struct{}, len(excludedIPs)+2) + excluded["127.0.0.1"] = struct{}{} + excluded["127.0.0.53"] = struct{}{} + for _, value := range excludedIPs { + if ip := net.ParseIP(strings.TrimSpace(value)); ip != nil { + excluded[ip.String()] = struct{}{} + } + } + + filtered := make([]string, 0, min(len(servers), 3)) + seen := make(map[string]struct{}, len(servers)) + for _, value := range servers { + ip := net.ParseIP(strings.TrimSpace(value)) + if ip == nil { + continue + } + normalized := ip.String() + if _, skip := excluded[normalized]; skip { + continue + } + if _, duplicate := seen[normalized]; duplicate { + continue + } + seen[normalized] = struct{}{} + filtered = append(filtered, normalized) + if len(filtered) == 3 { + break + } + } + return filtered } // parseDNSFromResolvectl extracts IP addresses from resolvectl dns output. @@ -3089,14 +3191,11 @@ func parseDNSFromResolvectl(output string) []string { } fields := strings.FieldsSeq(after) for f := range fields { - if net.ParseIP(f) != nil && f != "127.0.0.53" && f != "127.0.0.1" { + if net.ParseIP(f) != nil { servers = append(servers, f) } } } - if len(servers) > 3 { - servers = servers[:3] - } return servers } diff --git a/cmd/spinifex/cmd/admin_test.go b/cmd/spinifex/cmd/admin_test.go index 29406afca..42ae7614a 100644 --- a/cmd/spinifex/cmd/admin_test.go +++ b/cmd/spinifex/cmd/admin_test.go @@ -14,6 +14,7 @@ import ( "github.com/mulgadc/spinifex/spinifex/formation" handlers_dns "github.com/mulgadc/spinifex/spinifex/handlers/dns" handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" + "github.com/mulgadc/spinifex/spinifex/hostdns" "github.com/mulgadc/spinifex/spinifex/utils" toml "github.com/pelletier/go-toml/v2" "github.com/spf13/cobra" @@ -1083,6 +1084,135 @@ func TestAdminInitFlag_CompactionIntervalReachesRenderedConfig(t *testing.T) { assert.Contains(t, out, "interval_seconds = 45") } +func TestFilterDNSServersExcludesLocalNorthstar(t *testing.T) { + servers := []string{ + "10.0.0.5", + "127.0.0.53", + "1.1.1.1", + "1.1.1.1", + "not-an-ip", + "2001:0db8::1", + "8.8.8.8", + "9.9.9.9", + "4.4.4.4", + } + + got := filterDNSServers(servers, "10.0.0.5", "2001:db8::1") + assert.Equal(t, []string{"1.1.1.1", "8.8.8.8", "9.9.9.9"}, got) +} + +func TestDetectDNSServersContinuesAfterExcludedSource(t *testing.T) { + var queries []string + sources := dnsDetectionSources{ + queryResolvectl: func(args ...string) (string, error) { + query := strings.Join(args, " ") + queries = append(queries, query) + switch query { + case "dns br-wan": + return "Link 2 (br-wan): 10.0.0.5 127.0.0.53\n", nil + case "dns": + return "Global: 10.0.0.5 10.0.0.5 127.0.0.53 10.20.0.53 8.8.8.8\n", nil + default: + return "", fmt.Errorf("unexpected query %q", query) + } + }, + readResolvConf: func() (string, error) { + t.Fatal("resolv.conf read despite usable global resolvers") + return "", nil + }, + } + + got := detectDNSServersWithSources("br-wan", []string{"10.0.0.5"}, sources) + assert.Equal(t, []string{"10.20.0.53", "8.8.8.8"}, got) + assert.Equal(t, []string{"dns br-wan", "dns"}, queries) +} + +func TestDetectDNSServersFallsBackToResolvConf(t *testing.T) { + sources := dnsDetectionSources{ + queryResolvectl: func(_ ...string) (string, error) { + return "", errors.New("resolver manager unavailable") + }, + readResolvConf: func() (string, error) { + return "nameserver 10.0.0.5\nnameserver 10.20.0.53\n", nil + }, + } + + got := detectDNSServersWithSources("br-wan", []string{"10.0.0.5"}, sources) + assert.Equal(t, []string{"10.20.0.53"}, got) +} + +func TestHostDNSParams(t *testing.T) { + // northstarEnabled requires both credentials; the zones and IPs mirror what + // admin init/join hold in memory after rendering northstar.toml. + enabled := admin.ConfigSettings{ + NorthstarAccessKey: "AK", + NorthstarSecretKey: "SK", + NorthstarDefaultDomain: "spx3.net", + NorthstarInternalDomain: "compute.internal", + AdvertiseIP: "10.0.0.5", + BindIP: "0.0.0.0", + } + + tests := []struct { + name string + settings admin.ConfigSettings + skip bool + wantOK bool + wantParams hostdns.Params + }{ + { + name: "enabled selects AdvertiseIP over BindIP", + settings: enabled, + wantOK: true, + wantParams: hostdns.Params{ + ResolverIP: "10.0.0.5", + BaseDomain: "spx3.net", + InternalDomain: "compute.internal", + }, + }, + { + name: "falls back to BindIP when AdvertiseIP is unset", + settings: func() admin.ConfigSettings { + s := enabled + s.AdvertiseIP = "" + s.BindIP = "10.0.0.9" + return s + }(), + wantOK: true, + wantParams: hostdns.Params{ + ResolverIP: "10.0.0.9", + BaseDomain: "spx3.net", + InternalDomain: "compute.internal", + }, + }, + { + name: "opt-out skips even when enabled", + settings: enabled, + skip: true, + wantOK: false, + }, + { + name: "northstar disabled skips", + settings: func() admin.ConfigSettings { + s := enabled + s.NorthstarSecretKey = "" + return s + }(), + wantOK: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := hostDNSParams(tt.settings, tt.skip) + assert.Equal(t, tt.wantOK, ok) + if tt.wantOK { + assert.Equal(t, tt.wantParams, got) + } + }) + } +} + // The image is written into a root volume of exactly the size this returns, so // anything less than the image size truncates the image and the guest never // finds its root. The old flooring conversion undersized every non-round image. diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index f68fe5741..5666f0cf6 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -57,8 +57,8 @@ Operational commands for inspecting cluster state. These fan out NATS requests t | Command | Flags | Description | |---------|-------|-------------| -| `spx admin init` | `--nodes`, `--node`, `--bind`, `--port`, `--region`, `--az`, `--cluster-name`, `--cluster-bind`, `--cluster-routes`, `--predastore-nodes`, `--services`, `--formation-timeout`, `--token-ttl`, `--force`, `--external-mode` (`pool` \| `nat` routed NAT for non-bridgeable uplinks, single-node; pair with `setup-ovn.sh --nat-uplink`; add `--external-pool start-end --external-gateway ` or `--external-source=dhcp [--external-bind-bridge ]` for a public pool with full EIP support; note: system instances (ECS/EKS/load-balancer agents) are unsupported in nat v1 — they require `pool` mode) | Generates root IAM credentials (AKIA-prefixed access key + secret) → creates master.key (AES-256, 32 bytes, 0600) → writes bootstrap.json (consumed on first start) → generates CA + server TLS certificates → generates join token (written to `join-token` file, displayed in join command) → creates NATS config with auth token → writes spinifex.toml, awsgw.toml, predastore.toml → configures AWS CLI `spx` profile → creates directory structure under `~/spinifex/`. `--force` on an already-initialized node is idempotent for crypto: it preserves the existing keys (master.key, predastore/viperblock encryption keys), system + admin credentials, and CA, and refreshes only the config files and the CA-signed server certificate (so a changed bind IP / SANs is picked up without breaking already-joined nodes or CA-baked AMIs). A genuine clean slate requires removing the data dirs first. | -| `spx admin join` | `--host` (required), `--node` (required), `--token` (required), `--bind`, `--port`, `--region`, `--az`, `--cluster-bind`, `--cluster-routes`, `--data-dir`, `--services` | Connects to leader node with join token (Authorization: Bearer header) → retrieves cluster configuration → configures local node to join cluster and participate in distributed operations | +| `spx admin init` | `--nodes`, `--node`, `--bind`, `--port`, `--region`, `--az`, `--cluster-name`, `--cluster-bind`, `--cluster-routes`, `--predastore-nodes`, `--services`, `--formation-timeout`, `--token-ttl`, `--force`, `--skip-host-dns`, `--external-mode` (`pool` \| `nat` routed NAT for non-bridgeable uplinks, single-node; pair with `setup-ovn.sh --nat-uplink`; add `--external-pool start-end --external-gateway ` or `--external-source=dhcp [--external-bind-bridge ]` for a public pool with full EIP support; note: system instances (ECS/EKS/load-balancer agents) are unsupported in nat v1 — they require `pool` mode) | Generates root IAM credentials (AKIA-prefixed access key + secret) → creates master.key (AES-256, 32 bytes, 0600) → writes bootstrap.json (consumed on first start) → generates CA + server TLS certificates → generates join token (written to `join-token` file, displayed in join command) → creates NATS config with auth token → writes spinifex.toml, awsgw.toml, predastore.toml → configures AWS CLI `spx` profile → creates directory structure under `~/spinifex/` → points the host resolver at this node's local northstar so the Spinifex zones (ELB/EKS names) resolve from the node itself (opt out with `--skip-host-dns`). `--force` on an already-initialized node is idempotent for crypto: it preserves the existing keys (master.key, predastore/viperblock encryption keys), system + admin credentials, and CA, and refreshes only the config files and the CA-signed server certificate (so a changed bind IP / SANs is picked up without breaking already-joined nodes or CA-baked AMIs). A genuine clean slate requires removing the data dirs first. | +| `spx admin join` | `--host` (required), `--node` (required), `--token` (required), `--bind`, `--port`, `--region`, `--az`, `--cluster-bind`, `--cluster-routes`, `--data-dir`, `--services`, `--skip-host-dns` | Connects to leader node with join token (Authorization: Bearer header) → retrieves cluster configuration → configures local node to join cluster and participate in distributed operations → points the host resolver at this node's local northstar so the Spinifex zones resolve from the node itself (opt out with `--skip-host-dns`) | ### Version diff --git a/scripts/setup.sh b/scripts/setup.sh index a8a958ca0..1c0cc446e 100755 --- a/scripts/setup.sh +++ b/scripts/setup.sh @@ -15,7 +15,7 @@ # VERBOSE Set to 1 to echo "[setup] " before each top-level step. # SETUP_STAGES Comma-separated subset of stages to run: # deps, aws, users, sudoers, files, directories, -# env, systemd, logrotate, udev, fixown, resolved, +# env, systemd, logrotate, udev, fixown, # migrations # Unset = run every stage appropriate for the current mode. @@ -794,79 +794,6 @@ EOF info "Swap enabled: $swapfile (${size_mb}MiB), vm.swappiness=10" } -# --- Configure host split DNS for Spinifex zones (systemd-resolved) --- -# When systemd-resolved manages the host resolver, route the Spinifex -# authoritative zones (default_domain + compute.internal) to the local northstar -# resolver on the node WAN IP, while every other name keeps using the existing -# link/DHCP DNS. The "~" routing-domain prefix restricts the DNS= server to -# exactly those zones (resolved.conf(5)). northstar binds :53 (not -# 127.0.0.1), so DNS= must be the WAN IP. Idempotent; no-op without resolved. -setup_resolved_dns() { - stage "configuring systemd-resolved split DNS for Spinifex zones" - - # ISO chroot has no live resolver to reconfigure; firstboot handles it. - if [ "${ISO_BUILD:-0}" = "1" ]; then - info "Resolved split DNS skipped (ISO_BUILD=1; firstboot configures it)" - return - fi - - # Only act when systemd-resolved is the active resolver; otherwise the - # operator must point their resolver at the node :53 manually. - if ! $SUDO systemctl is-active --quiet systemd-resolved 2>/dev/null; then - info "systemd-resolved not active — skipping host split DNS" - info " To resolve the Spinifex zones, forward them to the node WAN IP on :53" - return - fi - - # northstar binds :53 on the node WAN IP. Detect the default-route source. - local wan_ip - wan_ip=$(ip -4 route get 1.1.1.1 2>/dev/null | grep -oE 'src [0-9.]+' | awk '{print $2}') - [ -z "$wan_ip" ] && wan_ip=$(ip -4 route show default 2>/dev/null | grep -oE 'src [0-9.]+' | awk '{print $2}') - if [ -z "$wan_ip" ]; then - warn "Could not detect a WAN IP — skipping resolved split DNS" - warn " Re-run once the WAN is up: sudo SETUP_STAGES=resolved $0" - return - fi - - # Default zones match `spx admin init` defaults; honour a custom - # default_domain if northstar.toml already exists (post-init re-run). - local base_domain="spx3.net" internal_domain="compute.internal" - local ns_toml=/etc/spinifex/northstar/northstar.toml - if $SUDO test -f "$ns_toml"; then - local cfg_domain - cfg_domain=$($SUDO grep -E '^[[:space:]]*default_domain[[:space:]]*=' "$ns_toml" 2>/dev/null \ - | head -1 | sed -E 's/.*=[[:space:]]*"?([^"#]*[^"# ])"?.*/\1/') - [ -n "$cfg_domain" ] && base_domain="$cfg_domain" - fi - - local dropin_dir=/etc/systemd/resolved.conf.d - local dropin="$dropin_dir/spinifex-dns.conf" - local tmp - tmp=$(mktemp) - cat > "$tmp" << EOF -# Generated by setup.sh — route the Spinifex authoritative zones to the local -# northstar resolver on the node WAN IP. The "~" routing-domain prefix restricts -# this DNS= server to exactly these zones; all other names use the link DNS. -# northstar binds :53 (not 127.0.0.1), so DNS= is the WAN IP. -[Resolve] -DNS=${wan_ip} -Domains=~${base_domain} ~${internal_domain} -EOF - - # Skip the resolved restart when nothing changed (idempotent re-runs). - if $SUDO test -f "$dropin" && $SUDO cmp -s "$tmp" "$dropin"; then - rm -f "$tmp" - info "Host split DNS already current: ${base_domain} + ${internal_domain} -> ${wan_ip}:53" - return - fi - - $SUDO mkdir -p "$dropin_dir" - $SUDO install -m 0644 "$tmp" "$dropin" - rm -f "$tmp" - $SUDO systemctl restart systemd-resolved 2>/dev/null || true - info "Host split DNS: ${base_domain} + ${internal_domain} -> ${wan_ip}:53 (northstar)" -} - # --- Main --- main() { info "Spinifex installer" @@ -903,7 +830,6 @@ main() { stage_enabled logrotate && install_logrotate stage_enabled udev && install_udev stage_enabled swap && setup_swap - stage_enabled resolved && setup_resolved_dns # Migrations are only safe on a live system (need a running NATS and a # persisted config file). Skip under ISO_BUILD and under any explicit diff --git a/spinifex/hostdns/hostdns.go b/spinifex/hostdns/hostdns.go new file mode 100644 index 000000000..9020b6e8b --- /dev/null +++ b/spinifex/hostdns/hostdns.go @@ -0,0 +1,277 @@ +// Package hostdns points the node's own host resolver at the local northstar +// listener so the Spinifex authoritative zones (ELB/EKS names in the base zone +// and the AWS-parity private zone) resolve from the node itself, not just from +// guest VMs handed northstar over DHCP. +// +// It is called from the universal chokepoints that write the node's +// northstar.toml — spx admin init and spx admin join — fed straight from the +// in-memory settings that render that file, so nothing is re-parsed off disk. It +// configures whichever resolver manager the host runs: an active +// systemd-resolved gets a route-only drop-in that sends only the Spinifex zones +// to northstar while every other name keeps the link DNS; otherwise resolvconf +// gets northstar as the first nameserver ahead of the retained upstream servers. +// A host with neither manager is an error naming the manual remediation, never a +// silent skip. +package hostdns + +import ( + "bufio" + "bytes" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/mulgadc/spinifex/spinifex/utils" +) + +// Host paths the resolver managers read. Joined onto configurator.root, which is +// empty in production and a temp dir under test. +const ( + resolvedDropinPath = "/etc/systemd/resolved.conf.d/spinifex-dns.conf" + resolvconfHeadPath = "/etc/resolvconf/resolv.conf.d/head" + resolvConfPath = "/etc/resolv.conf" + resolverCommandTimeout = 30 * time.Second +) + +// Params carries the node-local resolver target and the two Spinifex zones that +// must resolve through it. All three come straight from the admin.ConfigSettings +// that also render northstar.toml, so the listener is never re-parsed off disk. +type Params struct { + // ResolverIP is the node-local northstar listener (the node AdvertiseIP, + // falling back to BindIP). It becomes DNS= / the first nameserver directly. + ResolverIP string + // BaseDomain is the authoritative base zone (e.g. "spx3.net"). + BaseDomain string + // InternalDomain is the AWS-parity private zone (e.g. "compute.internal"). + InternalDomain string +} + +// validate rejects params that would produce a broken resolver — an unset or +// wildcard IP cannot be dialled, and the domains anchor the routing rules. +func (p Params) validate() error { + switch { + case p.ResolverIP == "" || p.ResolverIP == "0.0.0.0": + return fmt.Errorf("resolver IP is unset (advertise/bind IP was %q); cannot point the host resolver at northstar", p.ResolverIP) + case p.BaseDomain == "": + return fmt.Errorf("base domain is empty") + case p.InternalDomain == "": + return fmt.Errorf("internal domain is empty") + } + return nil +} + +// Configure points this host's resolver at northstar for the Spinifex zones. +// It is idempotent (no resolver restart when the fragment and the running +// resolver already reflect the target) and verifies the change against the live +// resolver before returning nil. +func Configure(p Params) error { + return newConfigurator().configure(p) +} + +// configurator holds the resolver-manager side effects behind function fields so +// the render, idempotency, and verification logic runs under go test with +// stubbed commands and a rooted temp filesystem. +type configurator struct { + // root is prefixed onto every host path; "" in production. + root string + + // resolvedActive reports whether systemd-resolved is the active resolver. + resolvedActive func() bool + // hasResolvconf reports whether the resolvconf binary is on PATH. + hasResolvconf func() bool + + // restartResolved reloads systemd-resolved after a drop-in change. + restartResolved func() error + // updateResolvconf regenerates /etc/resolv.conf from its fragments. + updateResolvconf func() error + + // resolvedStatus returns `resolvectl status` output for verification. + resolvedStatus func() (string, error) +} + +// newConfigurator wires the production side effects. admin init/join run as root, +// so the writes to /etc and the systemctl/resolvconf calls have privileges. +func newConfigurator() *configurator { + return &configurator{ + resolvedActive: func() bool { + _, err := utils.RunCommandWithTimeout(resolverCommandTimeout, "systemctl", "is-active", "--quiet", "systemd-resolved") + return err == nil + }, + hasResolvconf: func() bool { + _, err := exec.LookPath("resolvconf") + return err == nil + }, + restartResolved: func() error { + _, err := utils.RunCommandWithTimeout(resolverCommandTimeout, "systemctl", "restart", "systemd-resolved") + return err + }, + updateResolvconf: func() error { + _, err := utils.RunCommandWithTimeout(resolverCommandTimeout, "resolvconf", "-u") + return err + }, + resolvedStatus: func() (string, error) { + return utils.RunCommandWithTimeout(resolverCommandTimeout, "resolvectl", "status") + }, + } +} + +// configure selects the host's resolver manager and applies the northstar route. +// systemd-resolved wins when active because its route-only drop-in is the least +// disruptive; otherwise resolvconf's head fragment prepends northstar. +func (c *configurator) configure(p Params) error { + if err := p.validate(); err != nil { + return err + } + switch { + case c.resolvedActive(): + return c.configureResolved(p) + case c.hasResolvconf(): + return c.configureResolvconf(p) + default: + return fmt.Errorf( + "no supported resolver manager (systemd-resolved or resolvconf) is present; "+ + "manually forward the Spinifex zones %s and %s to %s:53", + p.BaseDomain, p.InternalDomain, p.ResolverIP) + } +} + +// configureResolved writes the route-only drop-in and restarts systemd-resolved. +// The restart is skipped when the file is unchanged and the running resolver +// already routes the zones to northstar; otherwise the change is verified. +func (c *configurator) configureResolved(p Params) error { + dropin := renderResolvedDropin(p) + dropinPath := c.path(resolvedDropinPath) + + if existing, err := os.ReadFile(dropinPath); err == nil && bytes.Equal(existing, dropin) { + if c.verifyResolved(p) == nil { + return nil + } + } + + if err := writeFragment(dropinPath, dropin); err != nil { + return err + } + if err := c.restartResolved(); err != nil { + return fmt.Errorf("restart systemd-resolved: %w", err) + } + return c.verifyResolved(p) +} + +// verifyResolved confirms systemd-resolved reflects the drop-in: northstar is a +// DNS server and the base zone is one of its routing domains. +func (c *configurator) verifyResolved(p Params) error { + out, err := c.resolvedStatus() + if err != nil { + return fmt.Errorf("query resolvectl status: %w", err) + } + if !strings.Contains(out, p.ResolverIP) { + return fmt.Errorf("systemd-resolved does not list %s as a DNS server after configuration", p.ResolverIP) + } + // Domains render as "~spx3.net"; a bare-domain substring match tolerates the + // prefix and resolvectl's per-version formatting. + if !strings.Contains(out, p.BaseDomain) { + return fmt.Errorf("systemd-resolved does not route %s to %s after configuration", p.BaseDomain, p.ResolverIP) + } + return nil +} + +// configureResolvconf writes the head fragment and regenerates resolv.conf. The +// regeneration is skipped when the fragment is unchanged and northstar is +// already the first nameserver; otherwise the result is verified. +func (c *configurator) configureResolvconf(p Params) error { + head := renderResolvconfHead(p) + headPath := c.path(resolvconfHeadPath) + + if existing, err := os.ReadFile(headPath); err == nil && bytes.Equal(existing, head) { + if c.verifyResolvconf(p) == nil { + return nil + } + } + + if err := writeFragment(headPath, head); err != nil { + return err + } + if err := c.updateResolvconf(); err != nil { + return fmt.Errorf("resolvconf -u: %w", err) + } + return c.verifyResolvconf(p) +} + +// verifyResolvconf confirms the regenerated resolv.conf queries northstar first, +// ahead of the upstream servers resolvconf keeps as fallbacks. +func (c *configurator) verifyResolvconf(p Params) error { + first, err := firstNameserver(c.path(resolvConfPath)) + if err != nil { + return fmt.Errorf("read %s: %w", resolvConfPath, err) + } + if first != p.ResolverIP { + return fmt.Errorf("%s: first nameserver is %q, expected node resolver %s", resolvConfPath, first, p.ResolverIP) + } + return nil +} + +// path roots a host path under c.root (a no-op in production, a temp dir in tests). +func (c *configurator) path(p string) string { + return filepath.Join(c.root, p) +} + +// renderResolvedDropin builds the resolved.conf.d drop-in. The "~" routing-domain +// prefix scopes DNS= to exactly the Spinifex zones, so every other name keeps +// using the link/DHCP DNS. +func renderResolvedDropin(p Params) []byte { + return fmt.Appendf(nil, `# Generated by spx admin init/join — route the Spinifex authoritative zones to +# the node-local northstar resolver. The "~" routing-domain prefix restricts +# this DNS= server to exactly these zones; all other names use the link DNS. +[Resolve] +DNS=%s +Domains=~%s ~%s +`, p.ResolverIP, p.BaseDomain, p.InternalDomain) +} + +// renderResolvconfHead builds the resolvconf head fragment. It is prepended +// verbatim to the generated resolv.conf, ahead of the upstream servers, so +// northstar is queried first while those upstreams remain as fallbacks. +func renderResolvconfHead(p Params) []byte { + return fmt.Appendf(nil, `# Generated by spx admin init/join — query the node-local northstar resolver +# first so the Spinifex authoritative zones (ELB/EKS names) resolve from this +# node. The upstream servers below remain as fallbacks. +nameserver %s +`, p.ResolverIP) +} + +// writeFragment creates the parent directory and writes a resolver fragment. +// Resolver config is public: the drop-in dir must be world-traversable (0755) +// and the fragment world-readable (0644) so the resolver daemon and every +// name-resolving process can read them, matching distro defaults for +// /etc/systemd/resolved.conf.d and /etc/resolvconf. +func writeFragment(path string, content []byte) error { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { //nolint:gosec // resolver config dir is intentionally world-traversable + return fmt.Errorf("create %s: %w", filepath.Dir(path), err) + } + if err := os.WriteFile(path, content, 0o644); err != nil { //nolint:gosec // resolver fragment is intentionally world-readable + return fmt.Errorf("write %s: %w", path, err) + } + return nil +} + +// firstNameserver returns the first "nameserver" entry in a resolv.conf, or "" +// when the file has none. +func firstNameserver(path string) (string, error) { + f, err := os.Open(path) + if err != nil { + return "", err + } + defer f.Close() + + sc := bufio.NewScanner(f) + for sc.Scan() { + fields := strings.Fields(sc.Text()) + if len(fields) >= 2 && fields[0] == "nameserver" { + return fields[1], nil + } + } + return "", sc.Err() +} diff --git a/spinifex/hostdns/hostdns_test.go b/spinifex/hostdns/hostdns_test.go new file mode 100644 index 000000000..bf2747600 --- /dev/null +++ b/spinifex/hostdns/hostdns_test.go @@ -0,0 +1,205 @@ +package hostdns + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// testParams are the values admin init/join would pass: the node-local northstar +// listener and the two default Spinifex zones. +var testParams = Params{ + ResolverIP: "10.0.0.5", + BaseDomain: "spx3.net", + InternalDomain: "compute.internal", +} + +// newTestConfigurator returns a configurator rooted at a temp dir with every +// side effect stubbed to a no-op. Individual tests override the fields they +// exercise. +func newTestConfigurator(t *testing.T) *configurator { + t.Helper() + return &configurator{ + root: t.TempDir(), + resolvedActive: func() bool { return false }, + hasResolvconf: func() bool { return false }, + restartResolved: func() error { return nil }, + updateResolvconf: func() error { return nil }, + resolvedStatus: func() (string, error) { return "", nil }, + } +} + +// reflectingResolvedStatus returns resolvectl output that reflects the drop-in, +// so verification passes. +func reflectingResolvedStatus(p Params) string { + return "Global\n" + + " DNS Servers: " + p.ResolverIP + "\n" + + " DNS Domain: ~" + p.BaseDomain + " ~" + p.InternalDomain + "\n" +} + +func TestConfigureResolvedWritesRouteOnlyDropin(t *testing.T) { + c := newTestConfigurator(t) + c.resolvedActive = func() bool { return true } + restarts := 0 + c.restartResolved = func() error { restarts++; return nil } + c.resolvedStatus = func() (string, error) { return reflectingResolvedStatus(testParams), nil } + + if err := c.configure(testParams); err != nil { + t.Fatalf("configure: %v", err) + } + if restarts != 1 { + t.Errorf("restartResolved called %d times, want 1", restarts) + } + + got := readFile(t, c.path(resolvedDropinPath)) + // DNS= is the node listener and the "~" prefix scopes it to the Spinifex + // zones — all other names keep the link DNS. + for _, want := range []string{ + "[Resolve]\n", + "DNS=10.0.0.5\n", + "Domains=~spx3.net ~compute.internal\n", + } { + if !strings.Contains(got, want) { + t.Errorf("drop-in missing %q\ngot:\n%s", want, got) + } + } + // A resolvconf host is not touched when resolved owns the resolver. + if _, err := os.Stat(c.path(resolvconfHeadPath)); !os.IsNotExist(err) { + t.Errorf("resolvconf head written on a resolved host") + } +} + +func TestConfigureResolvedIdempotentSkipsRestart(t *testing.T) { + c := newTestConfigurator(t) + c.resolvedActive = func() bool { return true } + c.restartResolved = func() error { t.Fatalf("restart on an already-current host"); return nil } + c.resolvedStatus = func() (string, error) { return reflectingResolvedStatus(testParams), nil } + + // Pre-seed the exact drop-in the render would produce. + writeFile(t, c.path(resolvedDropinPath), renderResolvedDropin(testParams)) + + if err := c.configure(testParams); err != nil { + t.Fatalf("configure: %v", err) + } +} + +func TestConfigureResolvedVerificationFailureSurfaced(t *testing.T) { + c := newTestConfigurator(t) + c.resolvedActive = func() bool { return true } + restarts := 0 + c.restartResolved = func() error { restarts++; return nil } + // resolved comes back without the node as a DNS server — the change did not + // take, so success must not be reported. + c.resolvedStatus = func() (string, error) { return "Global\n DNS Servers: 127.0.0.53\n", nil } + + err := c.configure(testParams) + if err == nil { + t.Fatal("expected verification error, got nil") + } + if !strings.Contains(err.Error(), "does not list") { + t.Errorf("error = %v, want a DNS-server verification failure", err) + } + if restarts != 1 { + t.Errorf("restartResolved called %d times, want 1", restarts) + } +} + +func TestConfigureResolvconfNorthstarFirst(t *testing.T) { + c := newTestConfigurator(t) + c.hasResolvconf = func() bool { return true } + updates := 0 + c.updateResolvconf = func() error { + updates++ + // resolvconf -u prepends the head fragment ahead of the upstream servers. + regen := "nameserver 10.0.0.5\nnameserver 8.8.8.8\n" + return os.WriteFile(c.path(resolvConfPath), []byte(regen), 0o644) + } + + if err := c.configure(testParams); err != nil { + t.Fatalf("configure: %v", err) + } + if updates != 1 { + t.Errorf("updateResolvconf called %d times, want 1", updates) + } + + head := readFile(t, c.path(resolvconfHeadPath)) + if !strings.Contains(head, "nameserver 10.0.0.5\n") { + t.Errorf("head fragment missing northstar nameserver\ngot:\n%s", head) + } +} + +func TestConfigureResolvconfIdempotentSkipsUpdate(t *testing.T) { + c := newTestConfigurator(t) + c.hasResolvconf = func() bool { return true } + c.updateResolvconf = func() error { t.Fatalf("resolvconf -u on an already-current host"); return nil } + + writeFile(t, c.path(resolvconfHeadPath), renderResolvconfHead(testParams)) + writeFile(t, c.path(resolvConfPath), []byte("nameserver 10.0.0.5\nnameserver 8.8.8.8\n")) + + if err := c.configure(testParams); err != nil { + t.Fatalf("configure: %v", err) + } +} + +func TestConfigurePrefersResolvedOverResolvconf(t *testing.T) { + c := newTestConfigurator(t) + c.resolvedActive = func() bool { return true } + c.hasResolvconf = func() bool { return true } + c.updateResolvconf = func() error { t.Fatalf("resolvconf touched while resolved is active"); return nil } + c.resolvedStatus = func() (string, error) { return reflectingResolvedStatus(testParams), nil } + + if err := c.configure(testParams); err != nil { + t.Fatalf("configure: %v", err) + } + if _, err := os.Stat(c.path(resolvedDropinPath)); err != nil { + t.Errorf("resolved drop-in not written: %v", err) + } +} + +func TestConfigureNoSupportedManagerErrors(t *testing.T) { + c := newTestConfigurator(t) // both managers absent by default + + err := c.configure(testParams) + if err == nil { + t.Fatal("expected an error when no resolver manager is present") + } + // The error must name the manual remediation rather than skip silently. + for _, want := range []string{"no supported resolver manager", "10.0.0.5:53"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error = %v, want it to contain %q", err, want) + } + } +} + +func TestConfigureRejectsUnusableResolverIP(t *testing.T) { + for _, ip := range []string{"", "0.0.0.0"} { + c := newTestConfigurator(t) + c.resolvedActive = func() bool { t.Fatalf("manager probed before validation"); return false } + + p := testParams + p.ResolverIP = ip + if err := c.configure(p); err == nil { + t.Errorf("ResolverIP %q: expected validation error, got nil", ip) + } + } +} + +func readFile(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + return string(data) +} + +func writeFile(t *testing.T, path string, content []byte) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", filepath.Dir(path), err) + } + if err := os.WriteFile(path, content, 0o644); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} diff --git a/spinifex/utils/command.go b/spinifex/utils/command.go new file mode 100644 index 000000000..dd6bffa24 --- /dev/null +++ b/spinifex/utils/command.go @@ -0,0 +1,112 @@ +package utils + +import ( + "bytes" + "context" + "errors" + "fmt" + "os/exec" + "strings" + "syscall" + "time" +) + +const ( + commandOutputLimit = 64 * 1024 + commandWaitDelay = time.Second +) + +// RunCommandWithTimeout runs a host command with bounded output and execution. +// The command gets its own process group so timeout cleanup also kills child +// hooks rather than leaving them running after their parent exits. +func RunCommandWithTimeout(timeout time.Duration, name string, args ...string) (string, error) { + cmd := exec.Command(name, args...) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + cmd.WaitDelay = commandWaitDelay + + var output limitedCommandOutput + cmd.Stdout = &output + cmd.Stderr = &output + if err := cmd.Start(); err != nil { + return "", fmt.Errorf("%s: %w", commandName(name, args), err) + } + + waitDone := make(chan error, 1) + go func() { + waitDone <- cmd.Wait() + }() + + timer := time.NewTimer(timeout) + defer timer.Stop() + + var err error + select { + case err = <-waitDone: + // ErrWaitDelay means a descendant retained the output pipes after the + // direct process exited. Kill its process group before returning. + if errors.Is(err, exec.ErrWaitDelay) { + _ = killProcessGroup(cmd) + } + case <-timer.C: + if killErr := killProcessGroup(cmd); killErr != nil { + _ = cmd.Process.Kill() + } + <-waitDone + return commandOutputError(output.String(), fmt.Sprintf("%s timed out after %s", commandName(name, args), timeout), context.DeadlineExceeded) + } + + if err != nil { + return commandOutputError(output.String(), commandName(name, args), err) + } + return output.String(), nil +} + +func killProcessGroup(cmd *exec.Cmd) error { + if cmd.Process == nil { + return nil + } + if err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL); err != nil && !errors.Is(err, syscall.ESRCH) { + return err + } + return nil +} + +func commandName(name string, args []string) string { + return strings.Join(append([]string{name}, args...), " ") +} + +func commandOutputError(output, operation string, err error) (string, error) { + trimmed := strings.TrimSpace(output) + if trimmed != "" { + return output, fmt.Errorf("%s: %w: %s", operation, err, trimmed) + } + return output, fmt.Errorf("%s: %w", operation, err) +} + +// limitedCommandOutput prevents a noisy resolver hook from growing the admin +// process without bound while retaining enough output for diagnostics. +type limitedCommandOutput struct { + bytes.Buffer + + truncated bool +} + +func (b *limitedCommandOutput) Write(p []byte) (int, error) { + originalLen := len(p) + remaining := commandOutputLimit - b.Len() + if remaining > 0 { + _, _ = b.Buffer.Write(p[:min(len(p), remaining)]) + } + if len(p) > remaining { + b.truncated = true + } + return originalLen, nil +} + +func (b *limitedCommandOutput) String() string { + output := b.Buffer.String() + if b.truncated { + return output + "\n[command output truncated]" + } + return output +} diff --git a/spinifex/utils/command_test.go b/spinifex/utils/command_test.go new file mode 100644 index 000000000..219de90cb --- /dev/null +++ b/spinifex/utils/command_test.go @@ -0,0 +1,99 @@ +package utils + +import ( + "context" + "errors" + "os" + "os/exec" + "strconv" + "strings" + "syscall" + "testing" + "time" +) + +func TestRunCommandWithTimeoutKillsProcessGroup(t *testing.T) { + output, err := RunCommandWithTimeout(100*time.Millisecond, "sh", "-c", "echo waiting-for-lock >&2; sleep 10 & echo child=$!; wait") + if err == nil { + t.Fatal("expected timeout error, got nil") + } + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("error = %v, want context deadline exceeded", err) + } + for _, want := range []string{"timed out after 100ms", "waiting-for-lock", "child="} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error = %v, want it to contain %q", err, want) + } + } + + childPID := childPIDFromOutput(t, output) + deadline := time.Now().Add(time.Second) + for processRunning(childPID) && time.Now().Before(deadline) { + time.Sleep(10 * time.Millisecond) + } + if processRunning(childPID) { + t.Errorf("child process %d survived command timeout", childPID) + } +} + +func TestRunCommandWithTimeoutKillsDetachedOutputHolder(t *testing.T) { + started := time.Now() + output, err := RunCommandWithTimeout(5*time.Second, "sh", "-c", "sleep 10 & echo child=$!") + if !errors.Is(err, exec.ErrWaitDelay) { + t.Fatalf("error = %v, want exec.ErrWaitDelay", err) + } + if elapsed := time.Since(started); elapsed > 2*time.Second { + t.Fatalf("detached output holder returned after %s", elapsed) + } + + childPID := childPIDFromOutput(t, output) + deadline := time.Now().Add(time.Second) + for processRunning(childPID) && time.Now().Before(deadline) { + time.Sleep(10 * time.Millisecond) + } + if processRunning(childPID) { + t.Errorf("detached child process %d survived command cleanup", childPID) + } +} + +func TestRunCommandWithTimeoutIncludesDiagnostics(t *testing.T) { + _, err := RunCommandWithTimeout(time.Second, "sh", "-c", "echo resolver-broke >&2; exit 7") + if err == nil { + t.Fatal("expected command error, got nil") + } + for _, want := range []string{"sh -c", "resolver-broke", "exit status 7"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error = %v, want it to contain %q", err, want) + } + } +} + +func childPIDFromOutput(t *testing.T, output string) int { + t.Helper() + for field := range strings.FieldsSeq(output) { + value, ok := strings.CutPrefix(field, "child=") + if !ok { + continue + } + pid, err := strconv.Atoi(value) + if err != nil { + t.Fatalf("parse child PID %q: %v", value, err) + } + return pid + } + t.Fatalf("child PID missing from output %q", output) + return 0 +} + +func processRunning(pid int) bool { + data, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat") + if err == nil { + fields := strings.Fields(string(data)) + return len(fields) < 3 || fields[2] != "Z" + } + if os.IsNotExist(err) { + return false + } + err = syscall.Kill(pid, 0) + return err == nil || !errors.Is(err, syscall.ESRCH) +}