feat(httpapi): harden control plane and speed up prefix refresh

Очередь job получила lease и fencing, телеметрия и прогресс deploy сохраняются, ревизия пишет manifest входов. Обновление префиксов не переписывает неизменённый снимок, большие наборы идут через COPY, а устаревшая ревизия не попадает в deploy. Бандл ноды кэшируется, prune и постановка job больше не держат горячий путь. Co-authored-by: Cursor <cursoragent@cursor.com>
2026-11-02 17:23:59 +07:00
1 parent 924f111e48
commit f8e0520817
@@ -0,0 +1,31 @@
+package httpclient
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"net/http"
+)
+
+// ErrResponseTooLarge means the complete decoded HTTP response exceeded its budget.
+var ErrResponseTooLarge = errors.New("httpclient: response body exceeds limit")
+
+// ReadResponseBody reads a complete bounded body, including one extra byte to
+// distinguish an exact limit from truncation. Standard Transport gzip decoding
+// occurs before this reader, so the limit covers decompressed data as well.
+func ReadResponseBody(resp *http.Response, limit int64) ([]byte, error) {
+	if limit <= 0 || limit == int64(^uint64(0)>>1) {
+		return nil, fmt.Errorf("httpclient: invalid response limit")
+	}
+	if resp.ContentLength > limit {
+		return nil, ErrResponseTooLarge
+	}
+	body, err := io.ReadAll(io.LimitReader(resp.Body, limit+1))
+	if err != nil {
+		return nil, err
+	}
+	if int64(len(body)) > limit {
+		return nil, ErrResponseTooLarge
+	}
+	return body, nil
+}
@@ -0,0 +1,22 @@
+package httpclient
+
+import (
+	"errors"
+	"io"
+	"net/http"
+	"strings"
+	"testing"
+)
+
+func TestReadResponseBodyDoesNotAcceptValidPrefixTruncation(t *testing.T) {
+	for _, length := range []int64{-1, 9} {
+		resp := &http.Response{ContentLength: length, Body: io.NopCloser(strings.NewReader("{}       "))}
+		if _, err := ReadResponseBody(resp, 8); !errors.Is(err, ErrResponseTooLarge) {
+			t.Fatalf("length=%d got %v", length, err)
+		}
+	}
+	resp := &http.Response{ContentLength: -1, Body: io.NopCloser(strings.NewReader("{}      "))}
+	if b, err := ReadResponseBody(resp, 8); err != nil || len(b) != 8 {
+		t.Fatalf("exact limit failed: %q %v", b, err)
+	}
+}
@@ -1,8 +1,13 @@
 package httpclient

 import (
+	"errors"
+	"fmt"
+	"strings"
 	"sync"
 	"time"
+
+	"evobgp/internal/observability"
 )

 const (
@@ -16,14 +21,77 @@ type hostBreaker struct {
 	openUntil time.Time
 }

-var hostBreakers sync.Map // string -> *hostBreaker
+// ErrBreakerCapacity rejects new upstream identities when the bounded cache is full.
+var ErrBreakerCapacity = errors.New("httpclient: upstream circuit capacity exhausted")

-func breakerForHost(host string) *hostBreaker {
+type breakerEntry struct {
+	breaker  *hostBreaker
+	lastUsed time.Time
+	active   int
+}
+
+type breakerCache struct {
+	mu        sync.Mutex
+	entries   map[string]*breakerEntry
+	maximum   int
+	idle      time.Duration
+	nextSweep time.Time
+}
+
+var hostBreakers = breakerCache{entries: make(map[string]*breakerEntry), maximum: 1024, idle: 10 * time.Minute}
+
+func (c *breakerCache) sweep(now time.Time) {
+	for host, e := range c.entries {
+		if e.active != 0 || now.Before(e.lastUsed.Add(c.idle)) {
+			continue
+		}
+		e.breaker.mu.Lock()
+		open := now.Before(e.breaker.openUntil)
+		e.breaker.mu.Unlock()
+		if !open {
+			delete(c.entries, host)
+			observability.RemoveUpstreamBreaker(host)
+		}
+	}
+	c.nextSweep = now.Add(time.Second)
+}
+
+func (c *breakerCache) acquire(host string, now time.Time) (*hostBreaker, string, func(), error) {
+	if len(host) > 254 {
+		return nil, "", nil, fmt.Errorf("httpclient: upstream hostname too long")
+	}
+	host = strings.TrimSuffix(strings.ToLower(host), ".")
+	if len(host) > 253 {
+		return nil, "", nil, fmt.Errorf("httpclient: upstream hostname too long")
+	}
 	if host == "" {
 		host = "_"
 	}
-	v, _ := hostBreakers.LoadOrStore(host, &hostBreaker{})
-	return v.(*hostBreaker)
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	if !now.Before(c.nextSweep) {
+		c.sweep(now)
+	}
+	e := c.entries[host]
+	if e == nil {
+		if len(c.entries) >= c.maximum {
+			return nil, host, nil, ErrBreakerCapacity
+		}
+		e = &breakerEntry{breaker: &hostBreaker{}}
+		c.entries[host] = e
+	}
+	e.active++
+	e.lastUsed = now
+	var once sync.Once
+	release := func() {
+		once.Do(func() {
+			c.mu.Lock()
+			defer c.mu.Unlock()
+			e.active--
+			e.lastUsed = time.Now()
+		})
+	}
+	return e.breaker, host, release, nil
 }

 func (b *hostBreaker) allow() bool {
@@ -51,23 +119,31 @@ func (b *hostBreaker) recordFailure() {

 // SnapshotBreakers returns whether each known host breaker is currently open.
 func SnapshotBreakers() map[string]bool {
+	hostBreakers.mu.Lock()
+	defer hostBreakers.mu.Unlock()
+	now := time.Now()
+	hostBreakers.sweep(now)
 	out := map[string]bool{}
-	hostBreakers.Range(func(key, value any) bool {
-		host, _ := key.(string)
-		b, _ := value.(*hostBreaker)
-		if b == nil {
-			return true
-		}
+	for host, e := range hostBreakers.entries {
+		b := e.breaker
 		b.mu.Lock()
 		open := time.Now().Before(b.openUntil)
 		b.mu.Unlock()
 		out[host] = open
-		return true
-	})
+	}
 	return out
 }

-// ResetHostBreakers clears all circuit breakers (tests only).
+// ResetHostBreakers clears idle circuit breakers (tests only). In-flight
+// references are retained so their completions cannot recreate orphan metrics.
 func ResetHostBreakers() {
-	hostBreakers = sync.Map{}
+	hostBreakers.mu.Lock()
+	defer hostBreakers.mu.Unlock()
+	for host, e := range hostBreakers.entries {
+		if e.active == 0 {
+			delete(hostBreakers.entries, host)
+			observability.RemoveUpstreamBreaker(host)
+		}
+	}
+	hostBreakers.nextSweep = time.Time{}
 }
@@ -0,0 +1,118 @@
+package httpclient
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net/http"
+	"strings"
+	"testing"
+	"time"
+
+	"evobgp/internal/observability"
+	"github.com/prometheus/client_golang/prometheus"
+)
+
+func TestBreakerCanonicalHostAndActiveRetention(t *testing.T) {
+	c := breakerCache{entries: make(map[string]*breakerEntry), maximum: 1, idle: time.Minute}
+	now := time.Now()
+	first, key, release, err := c.acquire("Cache-Test.Example.", now)
+	if err != nil || key != "cache-test.example" {
+		t.Fatalf("canonical key=%s err=%v", key, err)
+	}
+	second, _, release2, err := c.acquire("cache-test.example", now)
+	if err != nil || first != second || len(c.entries) != 1 {
+		t.Fatal("alias bypassed shared breaker")
+	}
+	observability.SetUpstreamBreakerOpen(key, true)
+	first.openUntil = now.Add(3 * time.Minute)
+	if _, _, _, err := c.acquire("other.example", now.Add(2*time.Minute)); !errors.Is(err, ErrBreakerCapacity) {
+		t.Fatalf("active entry evicted: %v", err)
+	}
+	release()
+	release()
+	release2()
+	if _, _, _, err := c.acquire("other.example", now.Add(2*time.Minute+2*time.Second)); !errors.Is(err, ErrBreakerCapacity) {
+		t.Fatalf("open breaker evicted: %v", err)
+	}
+	_, _, done, err := c.acquire("other.example", now.Add(4*time.Minute))
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer done()
+	if len(c.entries) != 1 || c.entries[key] != nil {
+		t.Fatal("idle closed entry retained")
+	}
+	families, err := prometheus.DefaultGatherer.Gather()
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, family := range families {
+		if strings.HasSuffix(family.GetName(), "upstream_breaker_open") {
+			for _, metric := range family.Metric {
+				for _, label := range metric.Label {
+					if label.GetName() == "host" && label.GetValue() == key {
+						t.Fatal("evicted host metric retained")
+					}
+				}
+			}
+		}
+	}
+}
+
+type breakerTransport func(*http.Request) (*http.Response, error)
+
+func (f breakerTransport) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }
+
+func TestBreakerCapacityRejectsBeforeDial(t *testing.T) {
+	ResetHostBreakers()
+	var releases []func()
+	defer func() {
+		for _, release := range releases {
+			release()
+		}
+		ResetHostBreakers()
+	}()
+	for i := 0; i < hostBreakers.maximum; i++ {
+		_, _, release, err := hostBreakers.acquire(fmt.Sprintf("capacity-%d.example", i), time.Now())
+		if err != nil {
+			t.Fatal(err)
+		}
+		releases = append(releases, release)
+	}
+	called := false
+	hc := &http.Client{Transport: breakerTransport(func(*http.Request) (*http.Response, error) { called = true; return nil, errors.New("unexpected dial") })}
+	req, _ := http.NewRequest(http.MethodGet, "https://overflow.example/", nil)
+	_, err := DoWithBreaker(context.Background(), hc, req, 1)
+	if !errors.Is(err, ErrBreakerCapacity) || called {
+		t.Fatalf("capacity err=%v dial=%t", err, called)
+	}
+	if len(SnapshotBreakers()) != hostBreakers.maximum {
+		t.Fatal("cache capacity changed")
+	}
+	ResetHostBreakers()
+	if len(SnapshotBreakers()) != hostBreakers.maximum {
+		t.Fatal("reset discarded in-flight references")
+	}
+}
+
+func TestBreakerRejectsOversizedIdentity(t *testing.T) {
+	c := breakerCache{entries: make(map[string]*breakerEntry), maximum: 1, idle: time.Minute}
+	if _, _, _, err := c.acquire(strings.Repeat("x", 254), time.Now()); err == nil || len(c.entries) != 0 {
+		t.Fatal("oversized identity retained")
+	}
+	name := strings.Repeat("a", 253)
+	first, key, release, err := c.acquire(name, time.Now())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer release()
+	second, alias, release2, err := c.acquire(strings.ToUpper(name)+".", time.Now())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer release2()
+	if first != second || key != alias {
+		t.Fatal("maximum length trailing-dot alias differs")
+	}
+}
@@ -54,7 +54,9 @@ func DoWithRetry(ctx context.Context, hc *http.Client, req *http.Request, maxAtt
 			continue
 		}
 		if resp.StatusCode >= 500 {
-			_, _ = io.Copy(io.Discard, resp.Body)
+			// Draining is only an optimization for connection reuse, never an
+			// invitation to consume an unbounded error stream before retrying.
+			_, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 64<<10))
 			_ = resp.Body.Close()
 			lastErr = fmt.Errorf("httpclient: upstream %s", resp.Status)
 			continue
@@ -72,8 +74,11 @@ func DoWithBreaker(ctx context.Context, hc *http.Client, req *http.Request, maxA
 	if req == nil || req.URL == nil {
 		return nil, fmt.Errorf("httpclient: nil request")
 	}
-	host := req.URL.Hostname()
-	br := breakerForHost(host)
+	br, host, release, err := hostBreakers.acquire(req.URL.Hostname(), time.Now())
+	if err != nil {
+		return nil, err
+	}
+	defer release()
 	if !br.allow() {
 		observability.SetUpstreamBreakerOpen(host, true)
 		return nil, fmt.Errorf("httpclient: circuit open for %s", host)
@@ -0,0 +1,44 @@
+package httpclient
+
+import (
+	"context"
+	"io"
+	"net/http"
+	"testing"
+)
+
+type endlessRetryBody struct {
+	read   int
+	closed bool
+}
+
+func (b *endlessRetryBody) Read(p []byte) (int, error) {
+	for i := range p {
+		p[i] = 'x'
+	}
+	b.read += len(p)
+	return len(p), nil
+}
+func (b *endlessRetryBody) Close() error { b.closed = true; return nil }
+
+type retryTransport func(*http.Request) (*http.Response, error)
+
+func (f retryTransport) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }
+
+func TestRetryDrainBoundsEndlessErrorBody(t *testing.T) {
+	b := &endlessRetryBody{}
+	c := &http.Client{Transport: retryTransport(func(*http.Request) (*http.Response, error) {
+		return &http.Response{StatusCode: 503, Status: "503 Service Unavailable", Body: b, Header: make(http.Header)}, nil
+	})}
+	req, err := http.NewRequest(http.MethodGet, "http://agent.example/bundle", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err = DoWithRetry(context.Background(), c, req, 1); err == nil {
+		t.Fatal("upstream failure accepted")
+	}
+	if b.read != 64<<10 || !b.closed {
+		t.Fatalf("retry consumed %d bytes, closed=%v", b.read, b.closed)
+	}
+	var _ io.ReadCloser = b
+}