From 83c7687dfa3e12898fd945663b5bacc5be880896 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 14 Aug 2026 02:48:49 +0800 Subject: [PATCH] fix: close preview and idle hibernate gaps --- docs/deploy.md | 3 +-- sandboxd/config/config.go | 7 +++++-- sandboxd/config/config_test.go | 1 + sandboxd/pool/claim.go | 22 +++------------------- sandboxd/pool/telemetry_test.go | 6 +++--- sandboxd/server/preview.go | 16 +--------------- sandboxd/server/preview_test.go | 29 +++++++---------------------- 7 files changed, 21 insertions(+), 63 deletions(-) diff --git a/docs/deploy.md b/docs/deploy.md index fb67312..29c9567 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -104,7 +104,7 @@ sandboxd reads one JSON file (`-config`, default | `warm_max` (pool entry) | 0 (static) | turns on the demand-adaptive watermark for that pool: the warm target rises from `warm` toward `warm_max` while claims arrive faster than the measured provision lead covers, and decays back over ~a minute of silence | | `max_claims` | 0 (unlimited) | node-wide cap on live claims; claim/fork/branch requests beyond it answer 429 with the pool state unharmed (on a cluster, normal warm-candidate placement applies, with volume claims limited to candidates holding every requested volume) | | `audit_log` | false | append every relayed request frame's op + addressing fields (never payloads) to `/audit.jsonl`, size-rotated with one `.1` backup. Records are `{t, id, op}` plus whichever addressing fields the op carries (`argv`, `path`, `dest`, `from`, `to`, `url`, `session`, `port`); preview accesses record as op `preview`, one per request. A request frame whose first line exceeds 4 KiB is skipped, never truncated | -| `idle_hibernate_seconds` | 0 (off) | node-wide idle policy for unpooled claims (template/checkpoint claims): a claim with no data-plane connection for this long is hibernated; the next call wakes it transparently. Per-pool `idle_hibernate_seconds` (in a pool entry) does the same for that pool's claims — pooled keys ignore the node-wide value. Opt-in deliberately: a wake costs latency and the snapshot, so callers with their own idle logic must not pay twice | +| `idle_hibernate_seconds` | 0 (off) | node-wide idle policy for unpooled claims (template/checkpoint claims): a none-lane claim with no data-plane connection for this long is hibernated; the next call wakes it transparently. Per-pool `idle_hibernate_seconds` does the same for that pool's claims; pooled keys ignore the node-wide value, and egress pools reject it because they cannot resume safely. Opt in deliberately: a wake costs latency and the snapshot, so callers with their own idle logic must not pay twice | | `archive_after_seconds` | 0 (off) | tier below hibernation: a hibernated claim idle this long is checkpointed to the store and its local VM dropped, freeing the node entirely; the next call restores it transparently (a checkpoint restore's latency). Requires `idle_hibernate_seconds > 0` and must exceed it. Node-wide for unpooled keys; per-pool overrides for that pool | | `archive_delete_after_seconds` | 0 (keep) | purge an archived claim's store checkpoint this long after it was archived, reclaiming storage; the claim is then gone for good. Same node-wide/per-pool split | | `mesh` | unset | join a cluster ([Clusters](cluster.md)); unset = single node | @@ -295,7 +295,6 @@ here validates on load: "pools": [ {"template": "rt:24.04", "net": "none", "size": "small", "warm": 4, "warm_max": 12}, {"template": "rt:24.04", "net": "egress", "size": "medium", "warm": 2, - "idle_hibernate_seconds": 120, "archive_after_seconds": 900, "egress": {"allow": [ {"host": "api.github.com", "methods": ["GET", "POST"], "secret": "gh", "intercept": true}, {"host": "*.googleapis.com"} diff --git a/sandboxd/config/config.go b/sandboxd/config/config.go index bf4d6ad..ec4973c 100644 --- a/sandboxd/config/config.go +++ b/sandboxd/config/config.go @@ -49,7 +49,7 @@ type PoolSpec struct { // name a secret, the pool's is injected. Nil denies all egress. Egress *egress.Policy `json:"egress,omitempty"` - // IdleHibernateSeconds, when >0, hibernates this pool's idle claims + // IdleHibernateSeconds, when >0, hibernates this none-lane pool's idle claims // after that many seconds without a data-plane connection; the next // call wakes them transparently. Zero disables. IdleHibernateSeconds int `json:"idle_hibernate_seconds,omitempty"` @@ -76,6 +76,9 @@ func (s PoolSpec) ValidateLimits() error { if s.IdleHibernateSeconds < 0 { return fmt.Errorf("idle_hibernate_seconds must not be negative") } + if s.Net == types.NetEgress && s.IdleHibernateSeconds > 0 { + return fmt.Errorf("idle_hibernate_seconds is not supported for egress pools") + } return validateArchiveWindow(s.IdleHibernateSeconds, s.ArchiveAfterSeconds, s.ArchiveDeleteAfterSeconds) } @@ -210,7 +213,7 @@ type Config struct { // name; values come from the environment (value_env), never this file. Secrets []egress.SecretSpec `json:"secrets,omitempty"` - // IdleHibernateSeconds is the idle policy for claims of unpooled keys + // IdleHibernateSeconds is the idle policy for unpooled none-lane claims // (template and checkpoint claims); per-pool settings override it for // pooled keys. Zero disables. IdleHibernateSeconds int `json:"idle_hibernate_seconds,omitempty"` diff --git a/sandboxd/config/config_test.go b/sandboxd/config/config_test.go index 6324239..ac37e34 100644 --- a/sandboxd/config/config_test.go +++ b/sandboxd/config/config_test.go @@ -96,6 +96,7 @@ func TestLoadRejectsInvalid(t *testing.T) { {"bad restore mode", `{"restore_mode":"Mmap","pools":[]}`, "restore_mode"}, {"bad pool key", `{"pools":[{"template":"","net":"none","size":"small"}]}`, "pool"}, {"egress without attachment", `{"pools":[{"template":"rt:24.04","net":"egress","size":"small"}]}`, "egress lane needs"}, + {"egress idle hibernate", `{"bridges":["br0"],"pools":[{"template":"rt:24.04","net":"egress","size":"small","idle_hibernate_seconds":1}]}`, "not supported for egress"}, {"negative warm", `{"pools":[{"template":"rt:24.04","net":"none","size":"small","warm":-2}]}`, "negative"}, {"tenants without api_token", `{"pools":[],"tenants":[{"name":"acme","token":"t1"}]}`, "require api_token"}, {"empty tenant name", `{"api_token":"root","pools":[],"tenants":[{"name":"","token":"t1"}]}`, "tenant name"}, diff --git a/sandboxd/pool/claim.go b/sandboxd/pool/claim.go index a3a2a26..f34f25e 100644 --- a/sandboxd/pool/claim.go +++ b/sandboxd/pool/claim.go @@ -103,25 +103,7 @@ func (m *Manager) ClaimDeadline(id, token string) (time.Time, error) { return sb.Deadline, nil } -// PreviewTouch authorizes one preview request: the token is already verified, -// so the live-claim lookup is the whole check — a released sandbox is absent -// and its URL stops resolving. Stamps data-plane activity and writes the -// audit record (preview bypasses the relay's tap; this is the only trace). -func (m *Manager) PreviewTouch(ctx context.Context, id string, port uint16) error { - m.mu.Lock() - sb, ok := m.claimed[id] - m.mu.Unlock() - if !ok { - return ErrUnknownSandbox - } - sb.Touch() - m.recordAudit(ctx, id, auditFrame{Op: "preview", Port: port}) - return nil -} - -// PreviewDial opens a byte stream to a guest port for the preview proxy's -// connection pool; PreviewTouch authorizes each request separately. A -// hibernated sandbox wakes. +// PreviewDial authorizes one preview request and opens its guest connection. func (m *Manager) PreviewDial(ctx context.Context, id string, port uint16) (net.Conn, error) { m.mu.Lock() sb, ok := m.claimed[id] @@ -129,6 +111,8 @@ func (m *Manager) PreviewDial(ctx context.Context, id string, port uint16) (net. if !ok { return nil, ErrUnknownSandbox } + sb.Touch() + m.recordAudit(ctx, id, auditFrame{Op: "preview", Port: port}) sock, err := m.wakeResolved(ctx, sb) if err != nil { return nil, err diff --git a/sandboxd/pool/telemetry_test.go b/sandboxd/pool/telemetry_test.go index d8b5d87..f37bd7f 100644 --- a/sandboxd/pool/telemetry_test.go +++ b/sandboxd/pool/telemetry_test.go @@ -189,7 +189,7 @@ func TestSandboxesIndexOmitsTokens(t *testing.T) { } } -func TestPreviewTouchWritesAuditEvent(t *testing.T) { +func TestPreviewDialWritesAuditEvent(t *testing.T) { eng := newFakeEngine() dir := t.TempDir() m, err := NewManager(t.Context(), &config.Config{DataDir: dir, AuditLog: true, Pools: []config.PoolSpec{}}, eng, testSecrets(t)) @@ -197,8 +197,8 @@ func TestPreviewTouchWritesAuditEvent(t *testing.T) { t.Fatalf("setup manager: %v", err) } sb := mustClaim(t, m, testKey) - if touchErr := m.PreviewTouch(t.Context(), sb.ID, 8080); touchErr != nil { - t.Fatalf("preview touch: %v", touchErr) + if _, dialErr := m.PreviewDial(t.Context(), sb.ID, 8080); dialErr == nil { + t.Fatal("fake engine dial unexpectedly succeeded") } raw, err := os.ReadFile(filepath.Join(dir, "audit.jsonl")) diff --git a/sandboxd/server/preview.go b/sandboxd/server/preview.go index 1a01b4f..a5ba913 100644 --- a/sandboxd/server/preview.go +++ b/sandboxd/server/preview.go @@ -20,7 +20,6 @@ import ( "github.com/cocoonstack/sandbox/sandboxd/types" ) -// previewClaims is the signed guest target and owner route carried by a preview URL. type previewClaims struct { ID string `json:"id"` Port uint16 `json:"port"` @@ -30,7 +29,6 @@ type previewClaims struct { // PreviewManager is the slice of the pool manager the preview path needs. type PreviewManager interface { - PreviewTouch(ctx context.Context, id string, port uint16) error PreviewDial(ctx context.Context, id string, port uint16) (net.Conn, error) } @@ -49,12 +47,8 @@ func NewPreviewServer(secret, base, owner string, mgr PreviewManager) *PreviewSe return nil } p := &PreviewServer{secret: []byte(secret), base: base, owner: owner, mgr: mgr} - // One shared transport so a page's sub-resource fan-out reuses kept-alive - // guest conns; the Director keys each request's host to sandbox:port so - // the idle pool never mixes claims. Revocation rides PreviewTouch in - // serve — pooled conns skip this dial. p.transport = &http.Transport{ - IdleConnTimeout: 90 * time.Second, + DisableKeepAlives: true, DialContext: func(ctx context.Context, _, addr string) (net.Conn, error) { id, portStr, err := net.SplitHostPort(addr) if err != nil { @@ -101,15 +95,9 @@ func (p *PreviewServer) serve(w http.ResponseWriter, r *http.Request) { p.forward(w, r, claims.Owner) return } - if err := p.mgr.PreviewTouch(r.Context(), claims.ID, claims.Port); err != nil { - http.Error(w, "preview target unreachable", http.StatusBadGateway) - return - } p.proxyLocal(w, r, claims) } -// proxyLocal reverse-proxies to the guest port over the pooled relay -// transport; serve's PreviewTouch has already authorized the request. func (p *PreviewServer) proxyLocal(w http.ResponseWriter, r *http.Request, claims previewClaims) { rp := &httputil.ReverseProxy{ Director: func(req *http.Request) { @@ -130,7 +118,6 @@ func (p *PreviewServer) proxyLocal(w http.ResponseWriter, r *http.Request, claim rp.ServeHTTP(w, r) //nolint:gosec // target derived from an HMAC-signed token, not client input } -// forward relays the signed request to the owner node's main listener. func (p *PreviewServer) forward(w http.ResponseWriter, r *http.Request, owner string) { target := &url.URL{Scheme: "http", Host: owner} rp := httputil.NewSingleHostReverseProxy(target) @@ -163,7 +150,6 @@ func (p *PreviewServer) verify(token string) (previewClaims, bool) { return claims, true } -// handlePreview mints a preview URL bounded by the claim's remaining lease. func (s *Server) handlePreview(w http.ResponseWriter, r *http.Request) { if s.preview == nil { writeErr(w, http.StatusNotImplemented, "preview not configured") diff --git a/sandboxd/server/preview_test.go b/sandboxd/server/preview_test.go index 0b350da..19755c5 100644 --- a/sandboxd/server/preview_test.go +++ b/sandboxd/server/preview_test.go @@ -90,18 +90,14 @@ func TestPreviewRechecksClaimForEveryRequest(t *testing.T) { guestAddr := newGuestServer(t, func(*http.Request) string { return "guest" }) var live atomic.Bool - var touches, dials atomic.Int32 + var dials atomic.Int32 live.Store(true) ps := NewPreviewServer("secret", "node:9000", "node:7777", &fakePreviewMgr{ - touch: func(string, uint16) error { - touches.Add(1) - if !live.Load() { - return net.ErrClosed - } - return nil - }, dial: func(string, uint16) (net.Conn, error) { dials.Add(1) + if !live.Load() { + return nil, net.ErrClosed + } return net.Dial("tcp", guestAddr) }, }) @@ -128,11 +124,8 @@ func TestPreviewRechecksClaimForEveryRequest(t *testing.T) { if resp.StatusCode != http.StatusBadGateway { t.Errorf("status after release = %d, want 502", resp.StatusCode) } - if got := touches.Load(); got != 2 { - t.Errorf("PreviewTouch calls = %d, want one per request", got) - } - if got := dials.Load(); got != 1 { - t.Errorf("PreviewDial calls = %d, want the pooled conn reused", got) + if got := dials.Load(); got != 2 { + t.Errorf("PreviewDial calls = %d, want one per request", got) } } @@ -166,15 +159,7 @@ func TestPreviewForwardsToOwner(t *testing.T) { } type fakePreviewMgr struct { - touch func(id string, port uint16) error - dial func(id string, port uint16) (net.Conn, error) -} - -func (f *fakePreviewMgr) PreviewTouch(_ context.Context, id string, port uint16) error { - if f.touch == nil { - return nil - } - return f.touch(id, port) + dial func(id string, port uint16) (net.Conn, error) } func (f *fakePreviewMgr) PreviewDial(_ context.Context, id string, port uint16) (net.Conn, error) {