carlos / platform Public

host: refuse a sidecar on the wake publish and on the binding reader too

Paul Campbell pushed by paul@keymail.dev 089eb0eb9be94642a43ddbc0b5277efab2c8b577 parent 04f3fd9
3 files changed, +132 −6
  • internal/host/agent.go +1 −1
  • internal/host/fleetbind.go +41 −1
  • internal/host/fleetbind_test.go +90 −4
diff --git a/internal/host/agent.go b/internal/host/agent.go
index b7498b778..0c610dbd8 100644
--- a/internal/host/agent.go
+++ b/internal/host/agent.go
@@ -268,7 +268,7 @@ func RunAgent(args []string) {
// MUST be assigned before Start() below spawns the sweeper
// goroutine, which reads svc.OnWake on every successful wake from
// then on — setting it any later is a data race.
- svc.OnWake = fleetBindingHook(fleetStore, identity.Fleet, identity.Label)
+ svc.OnWake = fleetBindingHook(fleetStore, identity.Fleet, identity.Label, reg)
}
if logSink != nil {
svc.Logs = logSink
diff --git a/internal/host/fleetbind.go b/internal/host/fleetbind.go
index e75fdb298..015a3b7de 100644
--- a/internal/host/fleetbind.go
+++ b/internal/host/fleetbind.go
@@ -237,6 +237,21 @@ func ReconcileFleetBindings(ctx context.Context, s release.Store, reg *registry.
if rt == nil {
continue // no matching route on this platform box
}
+ // Never stamp a sidecar. SetFleetBox on a fleet-placed route is
+ // precisely what turns the edge's fleet dispatch arm into a live
+ // proxy, so this reader is the last gate on the chain and it
+ // refuses on kind rather than trusting its input.
+ //
+ // Redundant with residentBindingHosts on the publishing side, and
+ // deliberately so: the two halves run on DIFFERENT machines and
+ // upgrade independently. A fleet box still on an older binary
+ // keeps publishing sidecar bindings into the bucket for the whole
+ // of a mixed-version roll, and this platform box is what reads
+ // them. A binding is an untrusted claim written by another host;
+ // the route row is local truth.
+ if rt.Kind == "sidecar" {
+ continue
+ }
if fleet, ok := registry.FleetPlacement(rt.Region); !ok || fleet != b.Fleet {
// The route isn't placed on THIS binding's fleet at all —
// v1 trust boundary: refuse to let it stamp (see doc
@@ -267,8 +282,33 @@ func ReconcileFleetBindings(ctx context.Context, s release.Store, reg *registry.
// goes dark, heartbeat ages, lease expires, the next request wakes a
// surviving box, which restamps — is exactly the fallback that covers a
// dropped write here).
-func fleetBindingHook(store release.Store, fleet, box string) func(host string) {
+//
+// reg is consulted for one thing only: a sidecar host must never have a
+// binding published for it, and this is the write path that carries TAKEOVER
+// semantics, so an errant one here does not merely appear — it claims the host
+// away from whoever legitimately held it. The filter lives here rather than
+// resting on "the activator never wakes a sidecar": that is true of the
+// activator TODAY, and it is a fact a later task in this same branch removes
+// (the sidecar worker goes through the same wake() that fires OnWake). A guard
+// that depends on a caller's current behaviour is not a guard.
+//
+// A route the registry cannot answer for — read error, or no row at all — is
+// published, exactly as it was before this filter existed: only a POSITIVE
+// "this is a sidecar" refuses. That keeps every non-sidecar host's behaviour
+// byte-identical through a transient registry hiccup (a dropped binding is a
+// routing outage; the fallback that covers it is slow), and the case is
+// covered anyway — ReconcileFleetBindings refuses to stamp a sidecar on the
+// reading side, and the edge refuses to bind one at all.
+func fleetBindingHook(store release.Store, fleet, box string, reg *registry.Registry) func(host string) {
return func(host string) {
+ if reg != nil {
+ switch rt, err := reg.Get(host); {
+ case err != nil:
+ log.Printf("agent: fleet binding for %s: route lookup: %v (publishing anyway)", host, err)
+ case rt != nil && rt.Kind == "sidecar":
+ return
+ }
+ }
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
b := FleetBinding{Host: host, Fleet: fleet, Box: box, BoundAt: time.Now().UnixMilli()}
diff --git a/internal/host/fleetbind_test.go b/internal/host/fleetbind_test.go
index 7f24c1031..0602e5654 100644
--- a/internal/host/fleetbind_test.go
+++ b/internal/host/fleetbind_test.go
@@ -49,7 +49,7 @@ func TestPublishedBindingBoxMatchesTunnelChannelName(t *testing.T) {
ctx := context.Background()
s := release.NewDirStore(t.TempDir())
- hook := fleetBindingHook(s, id.Fleet, id.Label)
+ hook := fleetBindingHook(s, id.Fleet, id.Label, testRegistry(t))
hook("one.example.com")
body, _, err := s.Get(ctx, fleetBindingKey(id.Fleet, "one.example.com"))
@@ -533,7 +533,7 @@ func TestFleetBindingHookLogsErrorWithoutPanicking(t *testing.T) {
defer log.SetOutput(prev)
store := errPutStore{Store: release.NewDirStore(t.TempDir())}
- hook := fleetBindingHook(store, "pi", "amber-gliding")
+ hook := fleetBindingHook(store, "pi", "amber-gliding", testRegistry(t))
hook("one.example.com") // must not panic even though every Put fails
@@ -545,7 +545,7 @@ func TestFleetBindingHookLogsErrorWithoutPanicking(t *testing.T) {
func TestFleetBindingHookWritesBinding(t *testing.T) {
ctx := context.Background()
s := release.NewDirStore(t.TempDir())
- hook := fleetBindingHook(s, "pi", "amber-gliding")
+ hook := fleetBindingHook(s, "pi", "amber-gliding", testRegistry(t))
hook("woken.example.com")
@@ -576,7 +576,7 @@ func TestFleetBindingHookTakesOverFromAnotherBox(t *testing.T) {
t.Fatalf("seed old-box's binding: %v", err)
}
- hook := fleetBindingHook(s, "pi", "new-box")
+ hook := fleetBindingHook(s, "pi", "new-box", testRegistry(t))
hook("one.example.com")
body, _, err := s.Get(ctx, fleetBindingKey("pi", "one.example.com"))
@@ -631,3 +631,89 @@ func TestPublishResidentFleetBindingsSkipsSidecars(t *testing.T) {
t.Fatalf("a sidecar binding was published — a box must never advertise a sidecar as served")
}
}
+
+// --- Fix round 1: the other two ends of the same chain ------------------------
+
+// TestFleetBindingHookNeverPublishesSidecar covers the WAKE publish path,
+// which the startup-pass filter does not reach. It matters more than the
+// startup pass, not less: this hook writes with takeover=true, so an errant
+// sidecar binding here does not merely appear — it claims the host away from
+// whichever box legitimately held it.
+//
+// Deliberately not resting on "the activator never wakes a sidecar": that is
+// true of the activator today and it is a fact a later task on this same
+// branch removes, at which point the hole would open silently. An ordinary
+// instance host must still be published, so this asserts both directions.
+func TestFleetBindingHookNeverPublishesSidecar(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ reg := testRegistry(t)
+ if err := reg.Upsert(registry.Route{Host: "app.example.com", Kind: "instance", Region: "fleet/pi"}); err != nil {
+ t.Fatalf("upsert instance: %v", err)
+ }
+ if err := reg.Upsert(registry.Route{Host: "bot.eleven.bdm.sidecar.internal", Kind: "sidecar", Region: "fleet/pi"}); err != nil {
+ t.Fatalf("upsert sidecar: %v", err)
+ }
+
+ hook := fleetBindingHook(s, "pi", "box1", reg)
+ hook("app.example.com")
+ hook("bot.eleven.bdm.sidecar.internal")
+
+ if _, _, err := s.Get(ctx, fleetBindingKey("pi", "app.example.com")); err != nil {
+ t.Fatalf("instance binding missing from the wake publish: %v", err)
+ }
+ if _, _, err := s.Get(ctx, fleetBindingKey("pi", "bot.eleven.bdm.sidecar.internal")); err == nil {
+ t.Fatalf("the wake hook published a sidecar binding — with takeover semantics, no less")
+ }
+}
+
+// TestReconcileFleetBindingsNeverStampsSidecar covers the READING side. A
+// binding is an untrusted claim written by another machine that upgrades on
+// its own schedule: through a mixed-version roll, a fleet box still on an
+// older binary keeps publishing sidecar bindings, and this platform box is
+// what acts on them. SetFleetBox is the act that hands a fleet-placed route to
+// the edge's fleet dispatch arm, so it must refuse on kind independently of
+// whoever wrote the object.
+//
+// Both routes here are placed on the SAME fleet as their binding, so the
+// pre-existing placement check passes for both — the kind guard is the only
+// thing that can distinguish them.
+func TestReconcileFleetBindingsNeverStampsSidecar(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ reg := testRegistry(t)
+ const sidecarHost = "bot.eleven.bdm.sidecar.internal"
+ if err := reg.Upsert(registry.Route{Host: "app.example.com", Kind: "instance", Region: "fleet/pi"}); err != nil {
+ t.Fatalf("upsert instance: %v", err)
+ }
+ if err := reg.Upsert(registry.Route{Host: sidecarHost, Kind: "sidecar", Region: "fleet/pi"}); err != nil {
+ t.Fatalf("upsert sidecar: %v", err)
+ }
+ // Both bindings written directly, as an older box's publisher would.
+ for _, host := range []string{"app.example.com", sidecarHost} {
+ if err := WriteFleetBinding(ctx, s, FleetBinding{
+ Host: host, Fleet: "pi", Box: "box1", BoundAt: 100,
+ }, true); err != nil {
+ t.Fatalf("seed binding for %s: %v", host, err)
+ }
+ }
+
+ if err := ReconcileFleetBindings(ctx, s, reg); err != nil {
+ t.Fatalf("ReconcileFleetBindings: %v", err)
+ }
+
+ rt, err := reg.Get("app.example.com")
+ if err != nil || rt == nil {
+ t.Fatalf("get instance route: %v", err)
+ }
+ if rt.FleetBox != "box1" {
+ t.Fatalf("instance FleetBox = %q, want box1 — ordinary routes must still be stamped", rt.FleetBox)
+ }
+ sc, err := reg.Get(sidecarHost)
+ if err != nil || sc == nil {
+ t.Fatalf("get sidecar route: %v", err)
+ }
+ if sc.FleetBox != "" {
+ t.Fatalf("sidecar FleetBox = %q, want empty — stamping it hands the row to the edge's fleet arm", sc.FleetBox)
+ }
+}