diff --git a/internal/httpapi/pct_create_test.go b/internal/httpapi/pct_create_test.go index 9e41d67..5381f99 100644 --- a/internal/httpapi/pct_create_test.go +++ b/internal/httpapi/pct_create_test.go @@ -88,6 +88,32 @@ func TestJSONErrValidForNastyOutput(t *testing.T) { } } +// TestGatewayPreflightPassed guards the exact bug found live: "UNREACHABLE" +// contains "REACHABLE" as a substring, so a strings.Contains(out,"REACHABLE") +// check is true for BOTH outcomes and can never fail. Exact-match only. +func TestGatewayPreflightPassed(t *testing.T) { + cases := []struct { + out string + want bool + }{ + {"PREFLIGHT_OK", true}, + {"PREFLIGHT_OK\n", true}, + {" PREFLIGHT_OK ", true}, + {"PREFLIGHT_FAIL", false}, + {"PREFLIGHT_FAIL\n", false}, + {"", false}, + {"some garbage output", false}, + // the specific historical bug: a naive substring check on the old + // REACHABLE/UNREACHABLE markers would have called this true. + {"UNREACHABLE", false}, + } + for _, c := range cases { + if got := gatewayPreflightPassed(c.out); got != c.want { + t.Errorf("gatewayPreflightPassed(%q) = %v, want %v", c.out, got, c.want) + } + } +} + func TestProvisionScript(t *testing.T) { s := provisionScript([]string{"docker.io", "git"}, "echo hi > /root/x") // Network/DNS gate must come before apt. diff --git a/internal/httpapi/phase3.go b/internal/httpapi/phase3.go index 499e677..be323a4 100644 --- a/internal/httpapi/phase3.go +++ b/internal/httpapi/phase3.go @@ -439,8 +439,8 @@ func executeApprovedAction(ctx context.Context, pool *db.Pool, execID uuid.UUID, // actually experience, not what the host's broader routing table can // reach. if isStatic && cfg.GW != "" { - pingOut, pingErr := sshExec(ctx, host, user, fmt.Sprintf("ping -I %s -c1 -W2 %s >/dev/null 2>&1 && echo REACHABLE || echo UNREACHABLE", cfg.Bridge, cfg.GW)) - if pingErr != nil || !strings.Contains(pingOut, "REACHABLE") { + pingOut, pingErr := sshExec(ctx, host, user, fmt.Sprintf("ping -I %s -c1 -W2 %s >/dev/null 2>&1 && echo PREFLIGHT_OK || echo PREFLIGHT_FAIL", cfg.Bridge, cfg.GW)) + if pingErr != nil || !gatewayPreflightPassed(pingOut) { msg := fmt.Sprintf( "gateway %s is not reachable from %s on bridge %s — this almost always means the bridge doesn't carry that subnet on this host (each bridge only reaches the network it's physically wired to). "+ "Do not retry with a different gateway guess in the same subnet: find an existing LXC on this host with an IP in the same /28 and copy its exact bridge+gateway, or use DHCP instead.", @@ -637,6 +637,17 @@ func jsonErr(format string, args ...any) []byte { // the host's template cache. Exact match wins; a bare distro hint (e.g. // "debian-13" or "debian") matches by prefix; empty picks the newest debian // (falling back to any) template available. Returns "" when nothing fits. +// gatewayPreflightPassed interprets the PREFLIGHT_OK/PREFLIGHT_FAIL markers +// from the pct_create gateway pre-flight check. Pulled out as its own +// function (rather than an inline strings.Contains at the call site) so it's +// unit-testable: a prior version checked for "REACHABLE", which is a +// substring of "UNREACHABLE" — the check could never actually fail, and it +// took a live deployment to notice. Exact-match markers plus a test make +// that specific bug class structurally unable to recur silently. +func gatewayPreflightPassed(out string) bool { + return strings.TrimSpace(out) == "PREFLIGHT_OK" +} + func resolveTemplate(requested string, available []string) string { if len(available) == 0 { return ""