amadan

carlos / platform Public

activator: the refreshed-route check judges a worker by its own role (re-review finding 9)

wakeRouteChanged required ActivatorManaged, which admits only
socket-backed instance rows. WakeWorker brings standalone sidecars up on
the same wake path, and production always wires Routes, so every
sidecar wake (boot, schedule, operator) failed before its lease.

The check now takes the entry's role: a worker entry's refreshed row must
still be a sidecar, an instance entry's must still be ActivatorManaged.
The identity checks (account, app, address, import gate, seed id) hold
for both.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude d1c0c80df353c0d81705fc84f43ae4af5d7eb378 parent a2f33f6
3 files changed, +74 −4
  • internal/activator/activator.go +1 −1
  • internal/activator/wakeidentity.go +9 −2
  • internal/activator/wakeidentity_test.go +64 −1
diff --git a/internal/activator/activator.go b/internal/activator/activator.go
index 6d3f4b760..949e057e5 100644
--- a/internal/activator/activator.go
+++ b/internal/activator/activator.go
@@ -1150,7 +1150,7 @@ func (a *Service) doWake(ctx context.Context, e *actEntry) (*wokenInstance, erro
}
// The refreshed row must still be the route this wake began for,
// and the activator's to wake (final review F4, wakeidentity.go).
- if err := wakeRouteChanged(rt, *fresh); err != nil {
+ if err := wakeRouteChanged(rt, *fresh, e.worker); err != nil {
return w, err
}
rt = *fresh
diff --git a/internal/activator/wakeidentity.go b/internal/activator/wakeidentity.go
index 811b93b97..06f289686 100644
--- a/internal/activator/wakeidentity.go
+++ b/internal/activator/wakeidentity.go
@@ -17,9 +17,16 @@ import (
// manages, or that is now a different route (account, app, seed, import
// gate or address), is not woken: the answer is an ordinary failed wake,
// before any lease or restore. nil means the wake goes on.
-func wakeRouteChanged(began, now registry.Route) error {
+//
+// The shape is judged by the entry's own role (re-review finding 9): an
+// instance entry's row must still be ActivatorManaged, and a worker
+// entry's (WakeWorker, a standalone sidecar, which binds no socket) must
+// still be a sidecar. The identity checks hold for both.
+func wakeRouteChanged(began, now registry.Route, worker bool) error {
switch {
- case !ActivatorManaged(now):
+ case worker && !now.IsSidecar():
+ return fmt.Errorf("%s: the route changed before the wake: it is no longer a sidecar", now.Host)
+ case !worker && !ActivatorManaged(now):
return fmt.Errorf("%s: the route changed before the wake: the activator no longer manages it", now.Host)
case began.Account != now.Account || began.App != now.App:
return fmt.Errorf("%s: the route changed before the wake: it belongs to %s/%s now, not %s/%s", now.Host, now.Account, now.App, began.Account, began.App)
diff --git a/internal/activator/wakeidentity_test.go b/internal/activator/wakeidentity_test.go
index 09755699d..bf7cbda80 100644
--- a/internal/activator/wakeidentity_test.go
+++ b/internal/activator/wakeidentity_test.go
@@ -116,8 +116,71 @@ func TestWakeRouteChanged(t *testing.T) {
} {
now := began
c.edit(&now)
- if err := wakeRouteChanged(began, now); (err != nil) != c.refused {
+ if err := wakeRouteChanged(began, now, false); (err != nil) != c.refused {
t.Errorf("%s: %v, want refused %v", c.name, err, c.refused)
}
}
}
+
+// Re-review finding 9: a standalone sidecar wakes through WakeWorker on the
+// same path, with Routes wired to the registry as production has it. Its
+// row is no ActivatorManaged instance (no socket, kind sidecar), and it
+// must still wake; a sidecar whose identity changed under the wake is still
+// refused before any lease.
+func TestWakeWorkerWithTheRegistryWired(t *testing.T) {
+ a, b, _, reg := testActivator(t, 10*time.Second)
+ a.Routes = reg
+ rt := testWorkerRoute("bot.shop.example.com")
+ rt.Account, rt.App = "bab", "shop"
+ if err := reg.Upsert(rt); err != nil {
+ t.Fatal(err)
+ }
+ if err := a.WakeWorker(rt); err != nil {
+ t.Fatalf("WakeWorker of an unchanged sidecar: %v", err)
+ }
+ if got := b.events(); !contains(got, "startWorker:bot.shop.example.com") {
+ t.Fatalf("events %v; want the sidecar started", got)
+ }
+
+ moved := testWorkerRoute("bot2.shop.example.com")
+ moved.Account, moved.App = "bab", "shop"
+ if err := reg.Upsert(moved); err != nil {
+ t.Fatal(err)
+ }
+ stale := moved
+ stale.App = "blog" // the caller's copy names another app
+ b.mu.Lock()
+ before := b.acquires
+ b.mu.Unlock()
+ if err := a.WakeWorker(stale); err == nil {
+ t.Fatal("a sidecar whose identity changed under the wake woke")
+ }
+ b.mu.Lock()
+ after := b.acquires
+ b.mu.Unlock()
+ if after != before {
+ t.Fatalf("the refused wake took %d leases", after-before)
+ }
+}
+
+// The role decides the shape: a worker entry needs a sidecar row, an
+// instance entry an ActivatorManaged one; the identity checks hold for both.
+func TestWakeRouteChangedByRole(t *testing.T) {
+ sidecar := testWorkerRoute("bot.shop.example.com")
+ sidecar.Account, sidecar.App = "bab", "shop"
+ if err := wakeRouteChanged(sidecar, sidecar, true); err != nil {
+ t.Fatalf("an unchanged sidecar for a worker entry: %v", err)
+ }
+ if err := wakeRouteChanged(sidecar, sidecar, false); err == nil {
+ t.Fatal("a sidecar row passed for an instance entry")
+ }
+ inst := testRoute("h.example.com")
+ if err := wakeRouteChanged(inst, inst, true); err == nil {
+ t.Fatal("an instance row passed for a worker entry")
+ }
+ other := sidecar
+ other.Account = "cdef"
+ if err := wakeRouteChanged(sidecar, other, true); err == nil {
+ t.Fatal("a worker entry woke a sidecar of another account")
+ }
+}