diff --git a/cmd/obol/sell.go b/cmd/obol/sell.go index 2df0f18a6..84fcd7e1f 100644 --- a/cmd/obol/sell.go +++ b/cmd/obol/sell.go @@ -9,7 +9,6 @@ import ( "fmt" "io" "math/big" - "net" "net/http" "net/url" "os" @@ -25,6 +24,7 @@ import ( "github.com/ObolNetwork/obol-stack/internal/agentcrd" "github.com/ObolNetwork/obol-stack/internal/app" "github.com/ObolNetwork/obol-stack/internal/config" + stackdefaults "github.com/ObolNetwork/obol-stack/internal/defaults" "github.com/ObolNetwork/obol-stack/internal/erc8004" "github.com/ObolNetwork/obol-stack/internal/hermes" "github.com/ObolNetwork/obol-stack/internal/images" @@ -4872,33 +4872,12 @@ func createHostService(cfg *config.Config, name, ns, port string) error { // For k3s (bare-metal) the host is localhost; for k3d the host is // reachable via Docker networking. func resolveHostIP(cfg *config.Config) (string, error) { - // Check if this is a k3s (bare-metal) backend — host is localhost. - if backend := stack.DetectExistingBackend(cfg); backend == stack.BackendK3s { - return "127.0.0.1", nil - } - - // k3d / Docker: try DNS resolution of host.docker.internal or host.k3d.internal. - for _, host := range []string{"host.docker.internal", "host.k3d.internal"} { - if addrs, err := net.LookupHost(host); err == nil && len(addrs) > 0 { - return addrs[0], nil - } - } - // macOS Docker Desktop fallback: well-known VM gateway. - if runtime.GOOS == "darwin" { - return "192.168.65.254", nil - } - // Linux fallback: docker0 bridge IP. - if iface, err := net.InterfaceByName("docker0"); err == nil { - if addrs, err := iface.Addrs(); err == nil { - for _, addr := range addrs { - if ipNet, ok := addr.(*net.IPNet); ok && ipNet.IP.To4() != nil { - return ipNet.IP.String(), nil - } - } - } - } - - return "", errors.New("cannot determine host IP; ensure Docker is running or using k3s backend") + // Delegates to the same resolver `obol stack up` uses for the ollama + // Endpoints. This used to be a second copy of that strategy, which drifted: + // it returned 127.0.0.1 for the k3s backend, and Kubernetes rejects + // loopback addresses in Endpoints (enforced since v1.33) — which is exactly + // what this result is used to build. One resolver, one loopback guard. + return stackdefaults.OllamaHostIPForBackend(stack.DetectExistingBackend(cfg)) } // buildInferenceServiceOfferSpec builds a ServiceOffer spec for a host-side diff --git a/internal/defaults/defaults.go b/internal/defaults/defaults.go index 806c8c971..d47ae8277 100644 --- a/internal/defaults/defaults.go +++ b/internal/defaults/defaults.go @@ -212,7 +212,86 @@ func OllamaHostForBackend(backendName string) string { // containers). // 2. Fall back to the Docker Desktop magic gateway IP (192.168.65.254) so // Docker Desktop users without a working `docker` CLI still work. +// +// Whatever the backend resolves to, the result is never a loopback address: +// see the guard below. func OllamaHostIPForBackend(backendName string) (string, error) { + ip, err := resolveOllamaHostIP(backendName) + if err != nil { + return "", err + } + + // Kubernetes rejects loopback addresses in Endpoints (enforced since v1.33): + // inside a pod's network namespace 127.0.0.1 is the pod itself, not the host, + // so such an endpoint could never have worked. k3s hits this by design — it + // runs directly on the host, so OllamaHostForBackend returns 127.0.0.1 — and + // some Docker runtimes map host.docker.internal to loopback too. Substitute + // the host's primary routable address, which is what pods can actually reach. + if parsed := net.ParseIP(ip); parsed != nil && parsed.IsLoopback() { + hostIP, hostErr := hostPrimaryIP() + if hostErr != nil { + return "", fmt.Errorf( + "Ollama host resolved to loopback address %q, which Kubernetes rejects in Endpoints, "+ + "and no routable host address could be determined: %w", ip, hostErr) + } + + return hostIP, nil + } + + return ip, nil +} + +// hostPrimaryIP returns the host's primary routable IPv4 address — the address +// pods reach the host on when the cluster runs on that same host. +func hostPrimaryIP() (string, error) { + // A UDP "dial" sends no packets; it only asks the kernel which route it + // would take, which yields the address of the default-route interface. + // 192.0.2.1 is TEST-NET-1 (RFC 5737) and is never actually contacted. + if conn, dialErr := net.Dial("udp", "192.0.2.1:80"); dialErr == nil { + defer conn.Close() + + if addr, ok := conn.LocalAddr().(*net.UDPAddr); ok && !addr.IP.IsLoopback() { + if v4 := addr.IP.To4(); v4 != nil { + return v4.String(), nil + } + } + } + + // No default route (isolated or air-gapped host): take the first up, + // non-loopback IPv4 address instead. + ifaces, err := net.Interfaces() + if err != nil { + return "", fmt.Errorf("enumerate network interfaces: %w", err) + } + + for _, iface := range ifaces { + if iface.Flags&net.FlagUp == 0 || iface.Flags&net.FlagLoopback != 0 { + continue + } + + addrs, addrErr := iface.Addrs() + if addrErr != nil { + continue + } + + for _, addr := range addrs { + ipNet, ok := addr.(*net.IPNet) + if !ok || ipNet.IP.IsLoopback() { + continue + } + + if v4 := ipNet.IP.To4(); v4 != nil { + return v4.String(), nil + } + } + } + + return "", errors.New("no routable non-loopback IPv4 address found on any interface") +} + +// resolveOllamaHostIP performs the per-backend resolution. Callers should use +// OllamaHostIPForBackend, which additionally rejects loopback results. +func resolveOllamaHostIP(backendName string) (string, error) { host := OllamaHostForBackend(backendName) if net.ParseIP(host) != nil { diff --git a/internal/defaults/defaults_test.go b/internal/defaults/defaults_test.go index 0528f2077..4bfea7b35 100644 --- a/internal/defaults/defaults_test.go +++ b/internal/defaults/defaults_test.go @@ -1,6 +1,7 @@ package defaults import ( + "net" "os" "path/filepath" "regexp" @@ -133,8 +134,19 @@ func TestCopyInfrastructureRendersStackPlaceholders(t *testing.T) { } out := string(data) + + // The ollama Endpoints IP must be rendered and routable — never loopback, + // which Kubernetes rejects in Endpoints (enforced since v1.33). + endpointIP := regexp.MustCompile(`ip: "([^"]+)"`).FindStringSubmatch(out) + if endpointIP == nil { + t.Fatalf("rendered defaults contain no ollama endpoint IP:\n%s", out) + } + + if parsed := net.ParseIP(endpointIP[1]); parsed == nil || parsed.IsLoopback() { + t.Fatalf("ollama endpoint rendered as %q, want a routable non-loopback IP", endpointIP[1]) + } + for _, want := range []string{ - `ip: "127.0.0.1"`, `LITELLM_MASTER_KEY: "sk-obol-test-stack"`, } { if !strings.Contains(out, want) { @@ -207,3 +219,36 @@ func TestDetectedBackendNameDefaultsToK3d(t *testing.T) { t.Fatalf("DetectedBackendName() = %q, want %q", got, backendK3s) } } + +// Kubernetes rejects loopback addresses in Endpoints (enforced since v1.33), +// so the k3s backend — which runs on the host and therefore names 127.0.0.1 as +// the Ollama host — must still resolve to a routable address. Regression test +// for `UPGRADE FAILED: cannot patch "ollama" with kind Endpoints: ... may not +// be in the loopback range`. +func TestOllamaHostIPForBackendNeverReturnsLoopback(t *testing.T) { + ip, err := OllamaHostIPForBackend(backendK3s) + if err != nil { + t.Fatalf("resolve k3s ollama host IP: %v", err) + } + + parsed := net.ParseIP(ip) + if parsed == nil { + t.Fatalf("resolved %q is not a valid IP address", ip) + } + + if parsed.IsLoopback() { + t.Fatalf("resolved %q is a loopback address; Kubernetes rejects these in Endpoints", ip) + } +} + +func TestHostPrimaryIPIsRoutable(t *testing.T) { + ip, err := hostPrimaryIP() + if err != nil { + t.Skipf("no routable interface available in this environment: %v", err) + } + + parsed := net.ParseIP(ip) + if parsed == nil || parsed.IsLoopback() || parsed.To4() == nil { + t.Fatalf("hostPrimaryIP returned %q, want a non-loopback IPv4 address", ip) + } +} diff --git a/internal/dns/resolver.go b/internal/dns/resolver.go index f57acc3a5..3058289ee 100644 --- a/internal/dns/resolver.go +++ b/internal/dns/resolver.go @@ -28,6 +28,12 @@ const ( containerName = "obol-dns" dnsImage = "alpine:3.21" domain = "obol.stack" + // StorefrontPreviewHostname is the local-only operator storefront preview + // origin. Like domain it is a fixed property of every local stack, so + // EnsureHostsEntries always emits it — the managed block is replaced + // wholesale on each call, and callers that only know about agent + // hostnames would otherwise drop it (see internal/hermes, internal/openclaw). + StorefrontPreviewHostname = "storefront-preview.obol.stack" // OS name constants for runtime.GOOS comparisons. osDarwin = "darwin" @@ -134,14 +140,16 @@ func isValidHostname(h string) bool { return validHostnameRegex.MatchString(h) } -// EnsureHostsEntries adds /etc/hosts entries for the given hostnames. -// Always includes "obol.stack" plus any additional hostnames (e.g. openclaw subdomains). -// Entries are idempotent — existing managed block is replaced. -func EnsureHostsEntries(hostnames []string) error { - // Always include the base domain. - all := []string{domain} - - seen := map[string]bool{domain: true} +// buildHostsBlock renders the managed /etc/hosts block for the given hostnames. +// +// The base domain and the storefront preview origin are always emitted: this +// function replaces the managed block wholesale, and most callers only know +// about agent hostnames, so anything not unconditional here gets dropped by +// whichever caller writes last. +func buildHostsBlock(hostnames []string) string { + all := []string{domain, StorefrontPreviewHostname} + + seen := map[string]bool{domain: true, StorefrontPreviewHostname: true} for _, h := range hostnames { if h != "" && !seen[h] && isValidHostname(h) { all = append(all, h) @@ -149,7 +157,6 @@ func EnsureHostsEntries(hostnames []string) error { } } - // Build the managed block. var block strings.Builder block.WriteString(hostsMarkerBegin + "\n") @@ -159,6 +166,16 @@ func EnsureHostsEntries(hostnames []string) error { block.WriteString(hostsMarkerEnd + "\n") + return block.String() +} + +// EnsureHostsEntries adds /etc/hosts entries for the given hostnames. +// Always includes "obol.stack" plus any additional hostnames (e.g. openclaw subdomains). +// Entries are idempotent — existing managed block is replaced. +func EnsureHostsEntries(hostnames []string) error { + block := strings.Builder{} + block.WriteString(buildHostsBlock(hostnames)) + data, err := os.ReadFile(hostsFile) if err != nil { return fmt.Errorf("read %s: %w", hostsFile, err) diff --git a/internal/dns/resolver_test.go b/internal/dns/resolver_test.go index 16377d083..7ee6c5a74 100644 --- a/internal/dns/resolver_test.go +++ b/internal/dns/resolver_test.go @@ -3,6 +3,7 @@ package dns import ( "os" "path/filepath" + "strings" "testing" ) @@ -75,3 +76,34 @@ func TestHasNMDnsmasqConfig(t *testing.T) { t.Errorf("hasNMDnsmasqConfig() = %v, but file exists = %v", result, fileExists == nil) } } + +// EnsureHostsEntries replaces the managed block wholesale, and most callers +// (internal/hermes, internal/openclaw) only know about agent hostnames. If the +// storefront preview origin were merely appended by one caller, whichever call +// ran last would silently drop it — which is exactly what happened on a real +// stack: `obol stack up` added it, then the agent-resume path overwrote the +// block without it, leaving the /storefront iframe unable to resolve its +// preview origin. It must be emitted unconditionally, like the base domain. +func TestEnsureHostsBlockAlwaysCarriesPreviewOrigin(t *testing.T) { + for name, hostnames := range map[string][]string{ + "no caller hostnames": nil, + "agent hostnames only": {"hermes-obol-agent.obol.stack", "obol-agent.obol.stack"}, + "preview passed by hand": {StorefrontPreviewHostname}, + } { + t.Run(name, func(t *testing.T) { + block := buildHostsBlock(hostnames) + + if !strings.Contains(block, "127.0.0.1 "+StorefrontPreviewHostname) { + t.Fatalf("managed block is missing the preview origin:\n%s", block) + } + + if got := strings.Count(block, StorefrontPreviewHostname); got != 1 { + t.Fatalf("preview origin appears %d times, want exactly 1:\n%s", got, block) + } + + if !strings.Contains(block, "127.0.0.1 "+domain) { + t.Fatalf("managed block is missing the base domain:\n%s", block) + } + }) + } +} diff --git a/internal/stack/stack_test.go b/internal/stack/stack_test.go index cb78df559..d39354f78 100644 --- a/internal/stack/stack_test.go +++ b/internal/stack/stack_test.go @@ -481,14 +481,22 @@ func TestDestroyOldBackendIfSwitching_SkipConfirmStillDestroys(t *testing.T) { } func TestOllamaHostIPForBackend_K3s(t *testing.T) { - // k3s backend should return 127.0.0.1 (already an IP, no DNS resolution needed) + // k3s runs on the host, so the configured Ollama host is 127.0.0.1 — but + // the result feeds a Kubernetes Endpoints object, and Kubernetes rejects + // loopback addresses there (enforced since v1.33). The resolver must + // substitute the host's routable address instead. ip, err := ollamaHostIPForBackend(BackendK3s) if err != nil { t.Fatalf("unexpected error for k3s backend: %v", err) } - if ip != "127.0.0.1" { - t.Errorf("expected 127.0.0.1 for k3s backend, got %s", ip) + parsed := net.ParseIP(ip) + if parsed == nil { + t.Fatalf("expected a valid IP for k3s backend, got %q", ip) + } + + if parsed.IsLoopback() { + t.Errorf("k3s backend returned loopback %s; Kubernetes rejects loopback in Endpoints", ip) } } @@ -512,16 +520,18 @@ func TestOllamaHostIPForBackend_K3d(t *testing.T) { } func TestOllamaHostIPForBackend_AlreadyIP(t *testing.T) { - // Verify the function passes through an already-numeric IP unchanged. - // k3s returns "127.0.0.1" from ollamaHostForBackend, so it should - // short-circuit on net.ParseIP without attempting DNS. + // The resolver short-circuits on net.ParseIP rather than attempting DNS + // when the configured host is already numeric. k3s is the numeric case + // (127.0.0.1), so this exercises that path — the loopback guard then + // swaps it for a routable address, which must still be a valid IP and + // must not have gone through a DNS lookup failure. ip, err := ollamaHostIPForBackend(BackendK3s) if err != nil { t.Fatalf("unexpected error: %v", err) } - if ip != "127.0.0.1" { - t.Errorf("expected pass-through of 127.0.0.1, got %s", ip) + if net.ParseIP(ip) == nil { + t.Errorf("expected a valid IP address, got %q", ip) } } diff --git a/internal/tunnel/tunnel.go b/internal/tunnel/tunnel.go index 13b99fc61..0bf8810c3 100644 --- a/internal/tunnel/tunnel.go +++ b/internal/tunnel/tunnel.go @@ -19,6 +19,7 @@ import ( "github.com/ObolNetwork/obol-stack/internal/agentruntime" "github.com/ObolNetwork/obol-stack/internal/config" stackdefaults "github.com/ObolNetwork/obol-stack/internal/defaults" + "github.com/ObolNetwork/obol-stack/internal/dns" "github.com/ObolNetwork/obol-stack/internal/images" "github.com/ObolNetwork/obol-stack/internal/ui" "github.com/ObolNetwork/obol-stack/internal/version" @@ -1284,7 +1285,9 @@ const ( storefrontNamespace = "traefik" // StorefrontPreviewHostname is the local-only operator preview origin. // It is never written into cloudflared ingress configuration. - StorefrontPreviewHostname = "storefront-preview.obol.stack" + // Defined in internal/dns, which owns the /etc/hosts managed block that + // must always contain it — one source of truth for the two. + StorefrontPreviewHostname = dns.StorefrontPreviewHostname ) // storefrontHostnames returns the hostnames the public storefront should be