Merge branch 'relay-review-followups' into main
16 files changed,
+452
−21
apimode.go+3 −1docs/site/cli.md+4 −2docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md+9 −1internal/console/apirelay.go+19 −0internal/console/apirelay_test.go+43 −1internal/console/relayops.go+8 −1internal/relay/authority.go+5 −0internal/relay/masters.go+19 −0internal/relay/masters_test.go+23 −0internal/relay/race_off_test.go+6 −0internal/relay/race_on_test.go+12 −0internal/relay/serve.go+35 −14internal/relay/serve_test.go+192 −1internal/relay/server_test.go+37 −0relay.go+18 −0relay_test.go+19 −0
CI passed — run details
diff --git a/apimode.go b/apimode.go| index 16adcf4c7..7e8d0cd6f 100644 |
| --- a/apimode.go |
| +++ b/apimode.go |
| @@ -2109,7 +2109,9 @@ type apiRelayRegion struct { |
| LeaseLive bool `json:"lease_live"` |
| Listeners []string `json:"listeners"` |
| Kids []string `json:"kids"` |
| - Stale bool `json:"stale"` |
| + // kid -> RFC3339 retire_at, for the loaded masters that carry one. |
| + Retiring map[string]string `json:"retiring,omitempty"` |
| + Stale bool `json:"stale"` |
| // The relay's own last two errors: why it stopped reading. Printed on a |
| // stale row, because the state without the reason is not actionable. |
| MastersError string `json:"masters_error,omitempty"` |
diff --git a/docs/site/cli.md b/docs/site/cli.md| index 1f638d8fc..fc75af093 100644 |
| --- a/docs/site/cli.md |
| +++ b/docs/site/cli.md |
| @@ -909,8 +909,10 @@ app holds ends on the relays' next authority read, within a minute. |
| TCP-address route it restarted by hand now runs the new key — `status` names |
| the routes waiting on that, since they are the one shape this platform |
| cannot restart for you. `rotate --region <r>` is the operator's verb after |
| -staging a new master; `keygen` prints one. `serve` is the unit's entry point |
| -and not a thing to run by hand. |
| +staging a new master; `keygen` prints one. After a rotation, `status` names |
| +any master that is retiring and the time it may be dropped, box by box, since |
| +a region is done with a key only once every relay there has stopped loading |
| +it. `serve` is the unit's entry point and not a thing to run by hand. |
| `keygen` prints one line — a key id, a key, and `-` for "no retirement date" |
| — for a region's master key. Stage that line in `/etc/carlos/relay-masters` |
diff --git a/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md b/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md| index bdc935a46..087801786 100644 |
| --- a/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md |
| +++ b/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md |
| @@ -393,7 +393,15 @@ ours. **(codex R1-20)** |
| - `apps/<acct>/<app>/relay.json`, CAS-guarded like `store.json`, is the |
| **source of truth** for the app: `{enabled, incarnation, regions, |
| - op, phase, config_generation, enabled_at, disabled_at}`. `op` is a |
| + op, phase, issued_digest, issued_suffixes, cutovers, enabled_at, |
| + disabled_at}`. `issued_digest` is the sha256 of the sealed envelope |
| + that was delivered, which is what `Record.IssuedDigest` records and |
| + what reconcile compares; an earlier draft of this spec called it |
| + `config_generation` and no implementation ever carried that name. |
| + `issued_suffixes` (region to the username suffix last issued) and |
| + `cutovers` (the per-route member acknowledgements) are the other two |
| + fields `relay.Record` has always carried and this listing has always |
| + omitted. **(discussion #120)** `op` is a |
| counter bumped by every operation; `phase` is one of `config`, |
| `aggregate`, `done` and records how far the operation got. The |
| incarnation is sixteen hex characters drawn at random at first |
diff --git a/internal/console/apirelay.go b/internal/console/apirelay.go| index 18d8c8fda..b2d40fbb0 100644 |
| --- a/internal/console/apirelay.go |
| +++ b/internal/console/apirelay.go |
| @@ -171,6 +171,13 @@ type apiRelayRegion struct { |
| LeaseLive bool `json:"lease_live"` // relay.Lease still armed to serve (15-minute ceiling) |
| Listeners []string `json:"listeners"` |
| Kids []string `json:"kids"` |
| + // Retiring is kid -> RFC3339 retire_at for the masters this box loaded |
| + // that carry one. It answers the operator's mid-rotation question — |
| + // when may the old key leave /etc/carlos/relay-masters — from the CLI, |
| + // per box, which is the only place the answer is true: a region is only |
| + // done with a key once EVERY relay there has stopped loading it |
| + // (discussion #120). |
| + Retiring map[string]string `json:"retiring,omitempty"` |
| // Stale mirrors relayRotate's own gate: a heartbeat whose last successful |
| // authority read is older than relayHeartbeatFresh (five minutes) cannot |
| // be read as current, so a dead relay cannot look alive by silence. |
| @@ -214,6 +221,12 @@ type apiRelayReply struct { |
| // retirement date on an app's status would mean either inventing state the |
| // console does not keep or recomputing it from a clock, which would tell a |
| // member a retirement deadline that drifts every time they ask. |
| + // |
| + // apiRelayRegion.Retiring is NOT that value and is not a softening of this |
| + // rule (discussion #120). It is the retire_at a relay actually loaded out |
| + // of its own masters file, reported per box and echoed verbatim: state the |
| + // operator wrote, not state this console computed, so it cannot drift |
| + // between two reads. |
| } |
| // apiRelayRotateReply is what `carlos relay rotate` answers with — a region |
| @@ -461,6 +474,12 @@ func (s *Server) relayReply(ctx context.Context, acct *Account, app string, rec |
| MastersError: hb.MastersError, |
| AuthorityError: hb.AuthorityError, |
| } |
| + for kid, at := range hb.Retiring { |
| + if row.Retiring == nil { |
| + row.Retiring = map[string]string{} |
| + } |
| + row.Retiring[kid] = at.UTC().Format(time.RFC3339) |
| + } |
| if hb.ReadAt.IsZero() { |
| row.HeartbeatAge = "never" |
| } else { |
diff --git a/internal/console/apirelay_test.go b/internal/console/apirelay_test.go| index f54ab92c5..b5762f374 100644 |
| --- a/internal/console/apirelay_test.go |
| +++ b/internal/console/apirelay_test.go |
| @@ -36,11 +36,18 @@ func relayURL(fx *consoleFixture, app string) string { |
| // with its last SUCCESSFUL authority read `age` in the past — the clock |
| // status reads, and the one a dead relay cannot move. |
| func seedHeartbeat(t *testing.T, fx *consoleFixture, box, region string, age time.Duration, kids ...string) { |
| + t.Helper() |
| + seedHeartbeatRetiring(t, fx, box, region, age, nil, kids...) |
| +} |
| + |
| +// seedHeartbeatRetiring is seedHeartbeat with the retiring map set, for the |
| +// one test that covers the hop from a relay's heartbeat to the status reply. |
| +func seedHeartbeatRetiring(t *testing.T, fx *consoleFixture, box, region string, age time.Duration, retiring map[string]time.Time, kids ...string) { |
| t.Helper() |
| now := time.Now() |
| hb := relay.Heartbeat{ |
| Box: box, Region: region, ReadAt: now.Add(-age), WrittenAt: now.Add(-age), |
| - Listeners: []string{"udp:3478", "tcp:3478"}, Kids: kids, |
| + Listeners: []string{"udp:3478", "tcp:3478"}, Kids: kids, Retiring: retiring, |
| } |
| if err := relay.WriteHeartbeat(context.Background(), fx.rel, hb); err != nil { |
| t.Fatalf("seed heartbeat: %v", err) |
| @@ -741,3 +748,38 @@ func TestAPIRelayStatusNeverAdoptsOnEmptyIssuedSuffixes(t *testing.T) { |
| t.Fatalf("delivery = %q, an empty-IssuedSuffixes record must never read as adopted", got) |
| } |
| } |
| + |
| +// TestAPIRelayStatusCarriesRetiringMasters covers the hop from a relay's |
| +// heartbeat to the status reply (discussion #120). Without it the console |
| +// half of the retiring display can be deleted with the whole suite green, |
| +// and an operator mid-rotation silently reads "nothing is retiring" — which |
| +// is indistinguishable from the true answer and is the wrong one to act on. |
| +func TestAPIRelayStatusCarriesRetiringMasters(t *testing.T) { |
| + fx, _ := relayFixture(t, "code-relay-retiring") |
| + token := cliToken(t, fx, "alice@keymail.dev", "code-relay-retiring-2") |
| + if status, out := postJSON(t, relayURL(fx, "titogo"), map[string]any{}, token); status != http.StatusOK { |
| + t.Fatalf("enable = %d %v, want 200", status, out) |
| + } |
| + |
| + retire := time.Date(2026, 9, 9, 12, 0, 0, 0, time.UTC) |
| + seedHeartbeatRetiring(t, fx, "box1", "eu-west-1", time.Minute, |
| + map[string]time.Time{"k1": retire}, "k1", "k2") |
| + |
| + status, out := getJSON(t, relayURL(fx, "titogo"), token) |
| + if status != http.StatusOK { |
| + t.Fatalf("status = %d %v, want 200", status, out) |
| + } |
| + row := regionRow(t, out, 0) |
| + got, ok := row["retiring"].(map[string]any) |
| + if !ok { |
| + t.Fatalf("region row carries no retiring map: %#v", row) |
| + } |
| + if got["k1"] != retire.Format(time.RFC3339) { |
| + t.Errorf("retiring[k1] = %v, want %s", got["k1"], retire.Format(time.RFC3339)) |
| + } |
| + // k2 is loaded but carries no retirement date, so it must be absent |
| + // rather than present with a zero time. |
| + if _, ok := got["k2"]; ok { |
| + t.Errorf("k2 has no retirement date and must not appear: %#v", got) |
| + } |
| +} |
diff --git a/internal/console/relayops.go b/internal/console/relayops.go| index 3c26d297f..719e196df 100644 |
| --- a/internal/console/relayops.go |
| +++ b/internal/console/relayops.go |
| @@ -123,8 +123,15 @@ var ( |
| "this deployment runs no relay (CARLOS_RELAY_REGIONS is empty) — ask the operator to arm a region before enabling it for an app") |
| errRelayNoSealKey = errors.New( |
| "this console has no secrets key (CARLOS_SECRETS_PUBKEY) — it cannot deliver a relay key, so it will not issue one") |
| + // The commonest cause is not another console at all: it is THIS console |
| + // having just restarted, leaving its own previous process's lease to |
| + // expire. That takes up to relayLeaseTTL, so the message says to retry |
| + // and names the wait rather than leaving an operator to guess whether |
| + // the deployment is wedged (discussion #120). |
| errRelayNotHeld = errors.New( |
| - "this console does not hold the relay writer lease — another console is driving the relay; the operation was not started") |
| + "this console does not hold the relay writer lease. another console is driving the relay, " + |
| + "or this one restarted and its previous lease has not expired yet. the operation was not started, " + |
| + "so retry in up to a minute") |
| errRelayNotEnabled = errors.New("the relay is not enabled for this app") |
| errRelayUndelivered = errors.New("a route has no config delivery") |
| // errRelayUnknownHost is `carlos relay adopted --host` naming something |
diff --git a/internal/relay/authority.go b/internal/relay/authority.go| index f827bce06..beae93c24 100644 |
| --- a/internal/relay/authority.go |
| +++ b/internal/relay/authority.go |
| @@ -80,6 +80,11 @@ type Heartbeat struct { |
| ReadAt time.Time `json:"read_at"` |
| Listeners []string `json:"listeners"` |
| Kids []string `json:"kids"` |
| + // Retiring is kid -> retire_at for the loaded masters that carry one, |
| + // so `carlos relay status` can tell an operator mid-rotation when the |
| + // old key may leave the box's masters file (discussion #120). Additive: |
| + // a relay older than this field simply reports none. |
| + Retiring map[string]time.Time `json:"retiring,omitempty"` |
| // The two "why is this record stale" fields: the last masters-file |
| // reload error and the last authority read error, either of which |
| // explains a ReadAt that has stopped moving. |
diff --git a/internal/relay/masters.go b/internal/relay/masters.go| index c8c1f7431..12886dbd8 100644 |
| --- a/internal/relay/masters.go |
| +++ b/internal/relay/masters.go |
| @@ -67,6 +67,25 @@ func (m *Masters) Lookup(kid string, now time.Time) (Master, bool) { |
| return mk, true |
| } |
| +// Retiring is the kids that carry a retire_at, and when it falls. An |
| +// operator mid-rotation reads it through `carlos relay status` to learn when |
| +// the old key may be dropped from /etc/carlos/relay-masters — the one fact |
| +// the CLI could not answer before (discussion #120). Kids with no retirement |
| +// date are absent rather than zero-valued, so "nothing is retiring" is an |
| +// empty map and not a map of zero times. |
| +func (m *Masters) Retiring() map[string]time.Time { |
| + if m == nil { |
| + return nil |
| + } |
| + out := map[string]time.Time{} |
| + for k, mk := range m.byKid { |
| + if !mk.RetireAt.IsZero() { |
| + out[k] = mk.RetireAt |
| + } |
| + } |
| + return out |
| +} |
| + |
| func (m *Masters) Kids() []string { |
| out := make([]string, 0, len(m.byKid)) |
| for k := range m.byKid { |
diff --git a/internal/relay/masters_test.go b/internal/relay/masters_test.go| index 1213e2fe5..2de1d203b 100644 |
| --- a/internal/relay/masters_test.go |
| +++ b/internal/relay/masters_test.go |
| @@ -100,3 +100,26 @@ func TestWatchMastersReloadsOnChange(t *testing.T) { |
| t.Fatal("no reload within three poll ticks") |
| } |
| } |
| + |
| +// Retiring is what `carlos relay status` turns into "when may this key leave |
| +// the box" (discussion #120). A master with no retirement date must be absent |
| +// rather than present with a zero time, or the CLI prints a retirement for |
| +// every key a box holds. |
| +func TestMastersRetiring(t *testing.T) { |
| + m, err := ParseMasters(strings.NewReader( |
| + "k0 " + base64.StdEncoding.EncodeToString(testMaster()) + " -\n" + |
| + "k1 " + base64.StdEncoding.EncodeToString(testMaster()) + " 2026-09-09T12:00:00Z\n")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + r := m.Retiring() |
| + if len(r) != 1 { |
| + t.Fatalf("Retiring() = %v, want just k1", r) |
| + } |
| + if got, want := r["k1"].UTC().Format(time.RFC3339), "2026-09-09T12:00:00Z"; got != want { |
| + t.Errorf("k1 retire_at = %s, want %s", got, want) |
| + } |
| + if _, ok := r["k0"]; ok { |
| + t.Error("k0 has no retirement date and must not appear") |
| + } |
| +} |
diff --git a/internal/relay/race_off_test.go b/internal/relay/race_off_test.go| new file mode 100644 |
| index 000000000..472c943f7 |
| --- /dev/null |
| +++ b/internal/relay/race_off_test.go |
| @@ -0,0 +1,6 @@ |
| +//go:build !race |
| + |
| +package relay |
| + |
| +// raceDetector is false in an ordinary build. See race_on_test.go. |
| +const raceDetector = false |
diff --git a/internal/relay/race_on_test.go b/internal/relay/race_on_test.go| new file mode 100644 |
| index 000000000..5ce1e39f7 |
| --- /dev/null |
| +++ b/internal/relay/race_on_test.go |
| @@ -0,0 +1,12 @@ |
| +//go:build race |
| + |
| +package relay |
| + |
| +// raceDetector lets one test skip itself under -race. The race it would |
| +// report is upstream, in pion/turn: Allocation.AddPermission publishes a |
| +// permission under permissionsLock and then calls perms.start(), which |
| +// writes p.lifetimeTimer with no lock, while Allocation.Close reads that |
| +// field from the packet-handler goroutine. Nothing in this package can fix |
| +// it, and leaving it unskipped costs the whole package its -race coverage, |
| +// which is worth more than this one test (discussion #120). |
| +const raceDetector = true |
diff --git a/internal/relay/serve.go b/internal/relay/serve.go| index f855f51a5..9c3aaae33 100644 |
| --- a/internal/relay/serve.go |
| +++ b/internal/relay/serve.go |
| @@ -124,6 +124,13 @@ type Config struct { |
| // test seam and nothing else: production is :3478 on both, per spec §3, |
| // and no env may move them. Empty means the constants above. |
| udp, tcp string |
| + |
| + // extraAllow is the same kind of seam, for the same reason. A relay on a |
| + // real box never permits a loopback peer, and an end-to-end test that |
| + // carries traffic between two sockets on this machine has nowhere else |
| + // to put them. Unexported so no env and no operator can widen the peer |
| + // policy; ServerConfig.ExtraAllow is the reviewed, production-facing one. |
| + extraAllow []*net.IPNet |
| } |
| // ConfigFromEnv builds a Config from the unit's environment. imdsIP and |
| @@ -242,20 +249,21 @@ func Serve(ctx context.Context, cfg Config, logf func(string, ...any)) error { |
| tcp = relayTCPAddr |
| } |
| srv, err := NewServer(ServerConfig{ |
| - Region: cfg.Region, |
| - RelayIP: cfg.PublicIP, |
| - UDP: udp, |
| - TCP: tcp, |
| - MinPort: 49152, |
| - MaxPort: 65535, |
| - DenyCIDRs: cfg.DenyCIDRs, |
| - Limits: cfg.Limits, |
| - Now: now, |
| - Log: logf, |
| - Meter: meter, |
| - Masters: func() *Masters { mu.Lock(); defer mu.Unlock(); return masters }, |
| - Authority: func() *Authority { mu.Lock(); defer mu.Unlock(); return auth }, |
| - Lease: func(t time.Time) bool { mu.Lock(); defer mu.Unlock(); return lease.Live(t) }, |
| + Region: cfg.Region, |
| + RelayIP: cfg.PublicIP, |
| + UDP: udp, |
| + TCP: tcp, |
| + MinPort: 49152, |
| + MaxPort: 65535, |
| + DenyCIDRs: cfg.DenyCIDRs, |
| + ExtraAllow: cfg.extraAllow, |
| + Limits: cfg.Limits, |
| + Now: now, |
| + Log: logf, |
| + Meter: meter, |
| + Masters: func() *Masters { mu.Lock(); defer mu.Unlock(); return masters }, |
| + Authority: func() *Authority { mu.Lock(); defer mu.Unlock(); return auth }, |
| + Lease: func(t time.Time) bool { mu.Lock(); defer mu.Unlock(); return lease.Live(t) }, |
| }) |
| if err != nil { |
| return err |
| @@ -309,6 +317,7 @@ func Serve(ctx context.Context, cfg Config, logf func(string, ...any)) error { |
| ReadAt: lease.Last(), // the last SUCCESSFUL read, never the wall clock |
| Listeners: listeners, |
| Kids: kidsOf(masters), |
| + Retiring: retiringOf(masters), |
| MastersError: mastersE, |
| } |
| if err != nil { |
| @@ -437,6 +446,18 @@ func kidsOf(m *Masters) []string { |
| return m.Kids() |
| } |
| +// retiringOf is Masters.Retiring with the same nil guard kidsOf carries. |
| +func retiringOf(m *Masters) map[string]time.Time { |
| + if m == nil { |
| + return nil |
| + } |
| + r := m.Retiring() |
| + if len(r) == 0 { |
| + return nil |
| + } |
| + return r |
| +} |
| + |
| // readPSI reads the `some avg10` percentage out of a pressure-stall file |
| // (/proc/pressure/memory): |
| // |
diff --git a/internal/relay/serve_test.go b/internal/relay/serve_test.go| index 565b42328..6008e1ed1 100644 |
| --- a/internal/relay/serve_test.go |
| +++ b/internal/relay/serve_test.go |
| @@ -7,6 +7,7 @@ import ( |
| "net" |
| "os" |
| "path/filepath" |
| + "strconv" |
| "strings" |
| "sync" |
| "testing" |
| @@ -188,7 +189,11 @@ func TestServeWritesHeartbeatAndStops(t *testing.T) { |
| store := newMemStore(t) |
| dir := t.TempDir() |
| masters := filepath.Join(dir, "relay-masters") |
| - if err := os.WriteFile(masters, []byte("k1 "+b64(testMaster())+" -\n"), 0o640); err != nil { |
| + // k2 carries a retirement date, so this fixture also covers the hop that |
| + // puts it on the heartbeat for `carlos relay status` (discussion #120). |
| + if err := os.WriteFile(masters, []byte( |
| + "k1 "+b64(testMaster())+" -\n"+ |
| + "k2 "+b64(testMaster())+" 2026-09-09T12:00:00Z\n"), 0o640); err != nil { |
| t.Fatal(err) |
| } |
| cfg := Config{ |
| @@ -227,6 +232,28 @@ func TestServeWritesHeartbeatAndStops(t *testing.T) { |
| if hb.ReadAt.IsZero() { |
| t.Error("heartbeat carries no read time, so the lease clock is invisible") |
| } |
| + // Without this the relay half of the retiring display can be deleted with |
| + // the whole suite green, and an operator mid-rotation silently reads |
| + // "nothing is retiring". Polled rather than asserted on the heartbeat |
| + // above: the masters watcher is a goroutine, so the FIRST heartbeat can |
| + // legitimately be written before the file has been read. |
| + deadline := time.Now().Add(10 * time.Second) |
| + for { |
| + hb := waitHeartbeat(t, store, "box-serve") |
| + if got, ok := hb.Retiring["k2"]; ok { |
| + if want := "2026-09-09T12:00:00Z"; got.UTC().Format(time.RFC3339) != want { |
| + t.Errorf("k2 retire_at = %s, want %s", got.UTC().Format(time.RFC3339), want) |
| + } |
| + if _, ok := hb.Retiring["k1"]; ok { |
| + t.Error("k1 has no retirement date and must not be reported as retiring") |
| + } |
| + break |
| + } |
| + if time.Now().After(deadline) { |
| + t.Fatalf("no heartbeat ever carried k2's retirement (last: %+v)", hb.Retiring) |
| + } |
| + time.Sleep(50 * time.Millisecond) |
| + } |
| cancel() |
| select { |
| @@ -425,3 +452,167 @@ func waitHeartbeat(t *testing.T, store release.Store, box string) Heartbeat { |
| time.Sleep(20 * time.Millisecond) |
| } |
| } |
| + |
| +// heartbeatUDPPort pulls the udp listener's port out of a heartbeat, which is |
| +// the only place a test outside Serve can learn the port it bound. |
| +func heartbeatUDPPort(t *testing.T, hb Heartbeat) int { |
| + t.Helper() |
| + for _, l := range hb.Listeners { |
| + if p, ok := strings.CutPrefix(l, "udp:"); ok { |
| + n, err := strconv.Atoi(p) |
| + if err != nil { |
| + t.Fatalf("udp listener %q has no port: %v", l, err) |
| + } |
| + return n |
| + } |
| + } |
| + t.Fatalf("heartbeat lists no udp listener: %v", hb.Listeners) |
| + return 0 |
| +} |
| + |
| +// TestServeEndsAllocationsOnDisable is the end-to-end disable path through |
| +// the real Serve loop, which spec §7 asks for and the branch never carried |
| +// (discussion #120): a pion client holding a live allocation, an app removed |
| +// from the authority record, and the relay acting on it within one authority |
| +// read rather than on a direct EndWhere call. |
| +// |
| +// The serial is what makes it a test and not a minute-long wait: the relay |
| +// polls control/serial every three seconds and re-reads the authority record |
| +// when it moves, which is the same path a console-mediated disable takes on |
| +// a real box. |
| +func TestServeEndsAllocationsOnDisable(t *testing.T) { |
| + store := newMemStore(t) |
| + dir := t.TempDir() |
| + mastersPath := filepath.Join(dir, "relay-masters") |
| + if err := os.WriteFile(mastersPath, []byte("k1 "+b64(testMaster())+" -\n"), 0o640); err != nil { |
| + t.Fatal(err) |
| + } |
| + ctx := context.Background() |
| + putAuthority := func(apps []AuthorityApp) time.Time { |
| + t.Helper() |
| + writtenAt := time.Now().UTC() |
| + body, err := json.Marshal(Authority{Apps: apps, WrittenAt: writtenAt}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + _, etag, _ := store.Get(ctx, AuthorityKey) |
| + if _, err := store.Put(ctx, AuthorityKey, body, etag); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := release.BumpSerial(ctx, store, time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + return writtenAt |
| + } |
| + putAuthority([]AuthorityApp{{Account: "abc", App: "meet", Incarnation: "0123456789abcdef", Regions: []string{"eu-west-1"}}}) |
| + |
| + cfg := Config{ |
| + Box: "box-disable", Region: "eu-west-1", MastersPath: mastersPath, |
| + // The advertised relay address is deliberately NOT loopback, as on a |
| + // real box: the peers are on loopback and a relay IP equal to them |
| + // does not forward (TestServerRelaysBothLegs makes the same choice). |
| + PublicIP: net.ParseIP("203.0.113.7"), Store: store, Limits: DefaultLimits(), |
| + PressurePath: filepath.Join(dir, "no-pressure-here"), |
| + udp: "127.0.0.1:0", |
| + tcp: "127.0.0.1:0", |
| + extraAllow: mustCIDRs("127.0.0.0/8"), |
| + } |
| + var logMu sync.Mutex |
| + quiet := false |
| + logf := func(format string, args ...any) { |
| + logMu.Lock() |
| + defer logMu.Unlock() |
| + if !quiet { |
| + t.Logf(format, args...) |
| + } |
| + } |
| + t.Cleanup(func() { logMu.Lock(); quiet = true; logMu.Unlock() }) |
| + |
| + runCtx, cancel := context.WithCancel(ctx) |
| + defer cancel() |
| + done := make(chan error, 1) |
| + go func() { done <- Serve(runCtx, cfg, logf) }() |
| + |
| + port := heartbeatUDPPort(t, waitHeartbeat(t, store, "box-disable")) |
| + username, password := creds(time.Now(), "abc", "meet", "0123456789abcdef") |
| + client := turnClient(t, &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: port}, username, password) |
| + relayConn, err := client.Allocate() |
| + if err != nil { |
| + t.Fatalf("allocate against a relay that has read an enabling record: %v", err) |
| + } |
| + defer relayConn.Close() |
| + |
| + // A peer on loopback, permitted through the extraAllow seam, and one |
| + // packet through the relay to prove the allocation carries traffic while |
| + // the app IS enabled. Without this the "no traffic after" check below |
| + // would pass against a relay that never forwarded anything at all. |
| + peer, err := net.ListenPacket("udp4", "127.0.0.1:0") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer peer.Close() |
| + peerAddr := peer.LocalAddr().(*net.UDPAddr) |
| + if err := client.CreatePermission(peerAddr); err != nil { |
| + t.Fatalf("permission for a loopback peer: %v", err) |
| + } |
| + if _, err := relayConn.WriteTo([]byte("before"), peerAddr); err != nil { |
| + t.Fatalf("write through the relay: %v", err) |
| + } |
| + buf := make([]byte, 64) |
| + peer.SetReadDeadline(time.Now().Add(5 * time.Second)) |
| + if n, _, err := peer.ReadFrom(buf); err != nil || string(buf[:n]) != "before" { |
| + t.Fatalf("the relay did not carry traffic while enabled: n=%d err=%v", n, err) |
| + } |
| + |
| + // The disable: the same record without the app, and a serial bump so the |
| + // relay re-reads inside three seconds rather than at the next minute. |
| + disabledAt := putAuthority(nil) |
| + |
| + // Wait on the relay's OWN report that it is holding the new record — |
| + // the heartbeat carries the WrittenAt of the authority copy it serves — |
| + // rather than on an allocation failing. Retrying Allocate until any |
| + // error appeared was the first shape of this test and it was not a test |
| + // at all: a refusal for an unrelated reason (a per-app allocation cap, |
| + // say) reads exactly like the disable, so the assertion passed with the |
| + // app still enabled. |
| + deadline := time.Now().Add(20 * time.Second) |
| + for { |
| + hb := waitHeartbeat(t, store, "box-disable") |
| + if !hb.WrittenAt.Before(disabledAt) { |
| + break |
| + } |
| + if time.Now().After(deadline) { |
| + t.Fatalf("the relay never picked up the record written at %s (last heartbeat holds %s)", disabledAt, hb.WrittenAt) |
| + } |
| + time.Sleep(100 * time.Millisecond) |
| + } |
| + |
| + // The allocation taken before the disable must stop carrying traffic. |
| + // This is the assertion the test is named for, and the first version of |
| + // it did not make: it only re-Allocated, which proves admission refuses |
| + // a NEW allocation and says nothing about the live one. Deleting Serve's |
| + // EndWhere(..., "authority") call left that version green. |
| + if _, err := relayConn.WriteTo([]byte("after"), peerAddr); err == nil { |
| + peer.SetReadDeadline(time.Now().Add(time.Second)) |
| + if n, _, err := peer.ReadFrom(buf); err == nil { |
| + t.Errorf("the relay forwarded %q after the app was disabled: the allocation did not end", buf[:n]) |
| + } |
| + } |
| + |
| + // And a credential that was good a moment ago must be refused, which is |
| + // what a member sees after `relay disable`. |
| + probe := turnClient(t, &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: port}, username, password) |
| + if _, err := probe.Allocate(); err == nil { |
| + t.Error("the relay allocated for an app the authority record no longer carries") |
| + } |
| + |
| + cancel() |
| + select { |
| + case err := <-done: |
| + if err != nil { |
| + t.Fatalf("Serve returned %v, want nil on a cancelled context", err) |
| + } |
| + case <-time.After(10 * time.Second): |
| + t.Fatal("Serve did not return after its context was cancelled") |
| + } |
| +} |
diff --git a/internal/relay/server_test.go b/internal/relay/server_test.go| index 043d53bee..0b69675e4 100644 |
| --- a/internal/relay/server_test.go |
| +++ b/internal/relay/server_test.go |
| @@ -657,3 +657,40 @@ func TestServerCloseKeepsReplacementBinding(t *testing.T) { |
| t.Fatal("the closed socket is still owned, so its admission never came back") |
| } |
| } |
| + |
| +// TestServerRefusesPermissionIntoConfiguredDenyCIDR covers the operator's |
| +// half of the peer policy, which the built-in private ranges do not reach: |
| +// CARLOS_RELAY_DENY_CIDRS carries the box's own VPC CIDR, and that CIDR is |
| +// ordinary public-looking space as far as the built-in list is concerned. |
| +// It is the one thing standing between a TURN allocation and the deployment's |
| +// private network, and until now only the built-in ranges had a test |
| +// (discussion #120). |
| +func TestServerRefusesPermissionIntoConfiguredDenyCIDR(t *testing.T) { |
| + if raceDetector { |
| + t.Skip("pion/turn races on Permission.lifetimeTimer between AddPermission and Close; see race_on_test.go") |
| + } |
| + // 198.51.100.0/24 stands in for the VPC: globally routable as far as |
| + // the built-in private list is concerned, denied only because the |
| + // operator said so. |
| + _, vpc, err := net.ParseCIDR("198.51.100.0/24") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + s, _, now, _, _ := newTestServer(t, "127.0.0.1", nil, func(cfg *ServerConfig) { |
| + cfg.DenyCIDRs = append(cfg.DenyCIDRs, vpc) |
| + }) |
| + username, password := creds(now(), "abc", "meet", "0123456789abcdef") |
| + client := turnClient(t, s.UDPAddr(), username, password) |
| + if _, err := client.Allocate(); err != nil { |
| + t.Fatalf("allocate: %v", err) |
| + } |
| + if err := client.CreatePermission(&net.UDPAddr{IP: net.ParseIP("198.51.100.9"), Port: 1}); err == nil { |
| + t.Error("permission into the configured deny CIDR was granted — a TURN client could reach the VPC") |
| + } |
| + // The control: an address outside both the built-in list and the |
| + // configured range is still permitted, so the test is not passing |
| + // because permissions are broken outright. |
| + if err := client.CreatePermission(&net.UDPAddr{IP: net.ParseIP("203.0.113.9"), Port: 1}); err != nil { |
| + t.Errorf("permission to an ordinary public peer was refused: %v", err) |
| + } |
| +} |
diff --git a/relay.go b/relay.go| index f678ea8ca..b9c8492fb 100644 |
| --- a/relay.go |
| +++ b/relay.go |
| @@ -33,6 +33,7 @@ import ( |
| "log" |
| "os" |
| "os/signal" |
| + "sort" |
| "strings" |
| "syscall" |
| @@ -402,6 +403,23 @@ func printRelayStatus(w io.Writer, res apiRelayReply) { |
| if len(rg.Kids) > 0 { |
| line += ", masters " + strings.Join(rg.Kids, ",") |
| } |
| + // A retiring master is the operator's cue, not the member's: it is |
| + // the only thing that says when the old key may leave this box's |
| + // /etc/carlos/relay-masters. Printed per box, because the region is |
| + // done with a key only once every relay there has stopped loading |
| + // it (discussion #120). |
| + if len(rg.Retiring) > 0 { |
| + kids := make([]string, 0, len(rg.Retiring)) |
| + for kid := range rg.Retiring { |
| + kids = append(kids, kid) |
| + } |
| + sort.Strings(kids) |
| + parts := make([]string, 0, len(kids)) |
| + for _, kid := range kids { |
| + parts = append(parts, kid+" after "+rg.Retiring[kid]) |
| + } |
| + line += ", retiring " + strings.Join(parts, ",") |
| + } |
| switch { |
| case rg.Stale: |
| // STALE is the important word and it gets the warning colour: a |
diff --git a/relay_test.go b/relay_test.go| index 8fc848383..3d98f753f 100644 |
| --- a/relay_test.go |
| +++ b/relay_test.go |
| @@ -170,6 +170,25 @@ func TestPrintRelayStatus(t *testing.T) { |
| // an old record — the two words a member acts on. |
| wantNot: []string{"STALE", "older record", "disable pending"}, |
| }, |
| + { |
| + // The operator's mid-rotation question, and the only place the |
| + // CLI can answer it: when may the old key leave this box's |
| + // /etc/carlos/relay-masters (discussion #120). |
| + name: "a retiring master names when it may be dropped", |
| + res: healthy, |
| + modifier: func(r *apiRelayReply) { |
| + r.RegionStatus = []apiRelayRegion{{ |
| + Region: "eu-west-1", Box: "relay-1", HeartbeatAge: "12s", Current: true, |
| + LeaseLive: true, Listeners: []string{"udp:3478"}, Kids: []string{"k1", "k2"}, |
| + Retiring: map[string]string{"k2": "2026-09-10T12:00:00Z", "k1": "2026-09-09T12:00:00Z"}, |
| + }} |
| + }, |
| + // Two of them, deliberately seeded out of order: the kids are |
| + // sorted, and the retiring clause uses the same tight separator |
| + // the masters list does, so its items cannot be misread as |
| + // further clauses of the line. |
| + want: []string{"masters k1,k2", "retiring k1 after 2026-09-09T12:00:00Z,k2 after 2026-09-10T12:00:00Z"}, |
| + }, |
| { |
| name: "stale names the relay's own reason", |
| res: healthy, |