replicas: the Fable review's three blocking findings and eight follow-ups
Review verdict was CHANGES REQUESTED; every finding verified against the code, then fixed. The three blockers: 1. STEERING REGRESSION (the serious one). My first fold gave per-host latency records only to boxes whose edges file named the host — but edges files are built from each box's LOCAL rows, so a non-home edge never names a home-boxed host. On rollout, every non-home region's per-host record would have been deleted. Steering is any-edge-any-app (far edges terminate and BACKHAUL); the fold now keeps the pre-#162 rule — every armed edge earns a record — and removes ONLY entries carrying the replica mark. The test that hid this is rewritten, plus a new case pinning that an unpublished host still earns records from every armed edge. 2. CLI UNREACHABLE. replicaReqFromFlags refused every combination the console accepts: --role replica + --region was refused (role needs --replicas), and --replicas alone was refused. Now the refusals mirror the console door exactly: role-without-region refused, role-with-set refused, --replicas alone rides a plain create as the home's opt-in. 3. RESIDENCE SECOND DOOR. A replica with a public addr would pass everywhereClass's dial test and mint on EVERY non-fleet box — the exact rule-stated-as-an-absence failure the AGENTS.md warns about (there are two doors, and the second names no action). ValidateShape now refuses role replica with any addr; everywhereClass returns false for a replica FIRST, before any kind or addr test; everywhereRoute agrees off the row. TestReplicaNeverEverywhereClass pins all three. Follow-ups: the edges file's account-apex key was inert as spelled (<app>.<sqid>, no domain) and would have been an apex-capture hazard spelled correctly (foldHomes' first-sorted-box rule) — deleted; the mark rides the row's own host only. Record.Replica bool (duplicate of Role, read by nobody) deleted. replica.go's stale header and unvalidated constructor fixed (ValidateShape now runs; named sentinels). The unused ReplicasOrDefault deleted. TestUpsertRoundTripsAllColumns extended with distinct values for both new columns, and the replica mark's own round trip (Upsert blanks the set on a replica row, so the cases are separate). carlos add's re-add path preserves Replicas beside Steering (the window between a re-add and the next converge tick would otherwise publish the account without its replica placements). Fleet-placed app + role replica refused at the create door — the armed/hosting gate is skipped under a fleet placement, so a replica would land unchecked. The dead top-level Replica field on steering's edgesFileView removed (the mark rides the entry); the anonymous Hosts struct is now a named type (edgeHostView). internal/edgesfile's consumer-list doc updated. The best-effort normalization in replicaSet now logs, so a hand-edited body is visible.
14 files changed,
+231
−222
cli.go+12 −0instances_cli.go+9 −8internal/console/apiinstances.go+12 −0internal/edgesfile/edgesfile.go+3 −2internal/host/edgesfile.go+9 −37internal/host/edgesfile_test.go+8 −1internal/provision/provision.go+17 −35internal/provision/provision_test.go+2 −10internal/provision/reconcile.go+16 −0internal/provision/reconcile_test.go+26 −0internal/provision/replica.go+44 −45internal/registry/registry_test.go+25 −0internal/steering/steering.go+30 −62internal/steering/steering_test.go+18 −22
CI passed — run details
diff --git a/cli.go b/cli.go| index f6e7dceb3..ad82f5462 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -239,6 +239,18 @@ func runAdd(args []string) { |
| // steering opt-in the moment the steering housekeeper duty (Task 6) |
| // starts converging DNS from it. Preserve it the same way. |
| route.Steering = existing.Steering |
| + // Replicas (#162) has the same shape and a different preserve rule: |
| + // the column CONVERGES from the record in both directions (the one |
| + // deliberate widening), so the next converge tick repairs a re-add's |
| + // blank anyway — but between this write and that tick the edges file |
| + // would publish the account without its replica placements, a window |
| + // of residence lying for seconds-to-minutes. Preserve it here so the |
| + // window does not exist. The replica MARK is deliberately NOT |
| + // preserved: it rides the record's Role, which this verb cannot |
| + // express, and a re-add minting an unmarked row over a replica's is |
| + // a shape change the next converge repairs — the mark is a routing |
| + // annotation, not a loss of state. |
| + route.Replicas = existing.Replicas |
| } |
| if err := r.Upsert(route); err != nil { |
| log.Fatal(err) |
diff --git a/instances_cli.go b/instances_cli.go| index 693a303bd..9ad840f5b 100644 |
| --- a/instances_cli.go |
| +++ b/instances_cli.go |
| @@ -308,23 +308,24 @@ func runInstancesCreate(args []string) { |
| // replicaReqFromFlags builds the create body's replica slice from the CLI's |
| // --role/--replicas pair, or nil when neither flag was given — a nil request |
| // sends neither key, byte-identical to every create before #162 existed. |
| -// Half a decision is refused HERE rather than echoed to the console's door: |
| -// the error names what is missing, the same fail-loud shape parseProbeFlags |
| -// uses for a half-stated probe. |
| +// The refusals mirror the console door's (checkReplicaCreate): a replica's |
| +// placement IS its --region, the placement set is the HOME record's opt-in |
| +// and rides a PLAIN create, and a replica never carries a set of its own. |
| +// Refusing here names what is missing without a console round trip. |
| func replicaReqFromFlags(role, replicas string, regionGiven bool) *instanceReplicaReq { |
| if role == "" && replicas == "" { |
| return nil |
| } |
| - if role == "" { |
| - log.Fatal("instances create: --replicas names where the account's replicas live — it is the HOME record's opt-in; provision a replica with --role replica --region <placement>") |
| + if role != "" && replicas != "" { |
| + log.Fatal("instances create: --role replica with --replicas: the placement set is the HOME record's opt-in — a replica does not carry one (drop --replicas, or drop --role)") |
| } |
| - if replicas == "" { |
| - log.Fatal("instances create: --role replica needs --region naming the replica's placement") |
| + if role == "" { |
| + return &instanceReplicaReq{Set: parseAddrList(replicas, true)} |
| } |
| if !regionGiven { |
| log.Fatal("instances create: --role replica needs --region naming the replica's placement") |
| } |
| - return &instanceReplicaReq{Role: role, Set: parseAddrList(replicas, true)} |
| + return &instanceReplicaReq{Role: role} |
| } |
| // parseAddrList splits a comma-separated --addr/--addrs flag into the wire's |
diff --git a/internal/console/apiinstances.go b/internal/console/apiinstances.go| index d93aa18af..2f6f8112f 100644 |
| --- a/internal/console/apiinstances.go |
| +++ b/internal/console/apiinstances.go |
| @@ -777,6 +777,18 @@ func (s *Server) handleAPIInstanceCreate(w http.ResponseWriter, r *http.Request) |
| return |
| } |
| if placed != "" { |
| + // A fleet-placed app governs every instance's Region uniformly, and |
| + // the armed/hosting gate below is skipped for it — the fleet is the |
| + // placement, validated by its own rail. A replica under that regime |
| + // would land wherever the fleet says, unchecked by anything that |
| + // knows what a replica placement means (#162): refused, not guessed. |
| + // Delete the app's placement first, or place the replica's account |
| + // on a fleet that spans the placement you want. |
| + if probeRec.Role == "replica" { |
| + jsonError(w, http.StatusBadRequest, |
| + "role replica on a fleet-placed app: the app's fleet placement ("+placed+") governs every instance's region, and a replica's placement must be an armed, hosting platform region — remove the app's placement first") |
| + return |
| + } |
| region = placed |
| } else if !provision.ValidRegion(region) || !instanceRegionAllowed(m.Regions, region) { |
| msg := "invalid region" |
diff --git a/internal/edgesfile/edgesfile.go b/internal/edgesfile/edgesfile.go| index 15623ecbd..10346696d 100644 |
| --- a/internal/edgesfile/edgesfile.go |
| +++ b/internal/edgesfile/edgesfile.go |
| @@ -17,8 +17,9 @@ |
| // Two readers deliberately keep their own narrow decoders and are NOT |
| // migrated: internal/edge/homemap.go (the serving hot path, with its own |
| // polling and staleness machinery) and internal/steering (which reads no |
| -// Hosts at all — steering is any-edge-any-app). Both are named here so the |
| -// consumer list stays complete rather than merely short. |
| +// role values — since #162 it does read the per-host replica mark, but its |
| +// decode stays its own for the same isolation reason). Both are named here |
| +// so the consumer list stays complete rather than merely short. |
| package edgesfile |
| import ( |
diff --git a/internal/host/edgesfile.go b/internal/host/edgesfile.go| index e7767cfc3..a6d00f09e 100644 |
| --- a/internal/host/edgesfile.go |
| +++ b/internal/host/edgesfile.go |
| @@ -50,8 +50,6 @@ type edgeHost struct { |
| // to a registry route. Additive (omitempty). The reader's fold lets a |
| // route beat an alias on a name collision across boxes: a tenant |
| // attaching an operator's hostname must never capture it fleet-wide. |
| - // route beat an alias on a name collision across boxes: a tenant |
| - // attaching an operator's hostname must never capture it fleet-wide. |
| Alias bool `json:"alias,omitempty"` |
| // Replica marks the account's checkout replica for this host (#162): |
| // this box serves the account from a one-way mirror of it — same app, |
| @@ -292,41 +290,15 @@ func edgesFilePass(ctx context.Context, s release.Store, reg *registry.Registry, |
| // Replica placements (#162): a row minted from a role=replica record — |
| // this box holds it BECAUSE its placement is this box's (the reconciler's |
| - // scope test is the only mint gate) — publishes its served names with the |
| - // replica mark set. The mark is how every reader of this file tells the |
| - // account's homes from its checkout replicas: steering's per-host fold |
| - // gives latency records to homes only, while the placement vocabulary |
| - // (this file's Placement field) is what ties a replica placement to the |
| - // hosts it serves. |
| - // |
| - // Names are the replica row's OWN host and the account's platform apex |
| - // (<app>.<sqid>) — the two names the replica box can vouch for from its |
| - // own registry. The home's host and the attached aliases live on the |
| - // home box's rows and never publish here: a name this box cannot serve |
| - // from a local row must not be claimed, and custom domains reach the |
| - // edges by their owner's CNAME to edge.<apps-domain> anyway (the |
| - // steering fold's own documented path). |
| - // |
| - // Never platform: like an alias, a replica entry derives no wildcard |
| - // parent. NOT the wildcard catch-all either: a replica serves the |
| - // account's exact names, and an invented label under a parent is a name |
| - // the home's own wildcard row already answers through backhaul — a |
| - // replica claiming the catch-all would widen where order data lives |
| - // with no record authorizing it. |
| - for _, r := range routes { |
| - if !r.Replica || r.Status != "ready" || r.IsSidecar() { |
| - continue |
| - } |
| - eh := edgeHost{Role: "home", Replica: true} |
| - // The row's own host was stamped by the main loop above; the extra |
| - // name a replica adds is the account's platform apex. |
| - if r.Account != "" && r.App != "" { |
| - apex := r.App + "." + r.Account |
| - if _, taken := hosts[apex]; !taken { |
| - hosts[apex] = eh |
| - } |
| - } |
| - } |
| + // scope test is the only mint gate) — publishes with the replica mark |
| + // set, which the main loop above already stamps onto its own host (the |
| + // mark rides the ROW). Nothing beyond that host is claimed: the home's |
| + // hostname and the platform apex live on the home box's rows, and a name |
| + // this box cannot serve from a local row must not be claimed (a |
| + // replica-claimed apex would let foldHomes' first-sorted-box rule hand |
| + // the apex DIAL target to a replica box fleet-wide). Custom domains |
| + // reach the edges by their owner's CNAME to edge.<apps-domain> anyway |
| + // (the steering fold's own documented path). |
| // Delegated customer wildcards (#305): the wildcard_domains rows are the |
| // parents this box obtains "*.<parent>" for (a claim with the Wildcard |
diff --git a/internal/host/edgesfile_test.go b/internal/host/edgesfile_test.go| index f02f0b933..ad3eaec5a 100644 |
| --- a/internal/host/edgesfile_test.go |
| +++ b/internal/host/edgesfile_test.go |
| @@ -1032,7 +1032,7 @@ func TestEdgesFilePublishesReplicaEntries(t *testing.T) { |
| if err := json.Unmarshal(got, &raw); err != nil { |
| t.Fatal(err) |
| } |
| - for _, name := range []string{"acme-uk.go.tito.io", "tito.bab"} { |
| + for _, name := range []string{"acme-uk.go.tito.io"} { |
| h, ok := raw.Hosts[name] |
| if !ok { |
| t.Errorf("replica row published nothing at %s (hosts: %+v)", name, raw.Hosts) |
| @@ -1045,6 +1045,13 @@ func TestEdgesFilePublishesReplicaEntries(t *testing.T) { |
| if h := raw.Hosts["acme.go.tito.io"]; h.Replica { |
| t.Errorf("the home row's entry carries the replica mark: %+v — the mark is the ROW's, and the home row is not a replica", h) |
| } |
| + // The account apex is deliberately NOT published: a name the replica box |
| + // has no local row for must not be claimed (an apex key here would let |
| + // foldHomes' first-sorted-box rule hand the apex dial target to a |
| + // replica box fleet-wide). |
| + if _, ok := raw.Hosts["tito.bab"]; ok { |
| + t.Errorf("replica row published the account apex tito.bab — a name this box has no local row for must not be claimed (hosts: %+v)", raw.Hosts) |
| + } |
| } |
| // TestEdgesFileReplicaMarkFollowsTheRow pins the fail-closed default: a |
diff --git a/internal/provision/provision.go b/internal/provision/provision.go| index ec7a92413..a432c5fa8 100644 |
| --- a/internal/provision/provision.go |
| +++ b/internal/provision/provision.go |
| @@ -14,6 +14,7 @@ import ( |
| "errors" |
| "fmt" |
| "github.com/carlosframework/platform/internal/controlname" |
| + "log" |
| "net" |
| "os" |
| "path/filepath" |
| @@ -130,18 +131,11 @@ type Record struct { |
| // even then only once a deployment arms CARLOS_STEERING_ZONE_ID. |
| Steering string `json:"steering,omitempty"` |
| - // Replica is an account's checkout replica for this instance (#162): |
| - // true names a second instance of the same app/account holding a |
| - // one-way mirror of it, placed near the buyers, selling against a |
| - // capacity lease from home (edge-checkout design record 2026-09-21). |
| - // Absent (the default every record written before the field existed |
| - // decodes to) is the serving home — byte-identical behaviour to |
| - // before the field existed. Written only beside a create; never a |
| - // PATCH axis in either direction, because a role flip is a |
| - // data-plane event (which database is authoritative), not a shape |
| - // edit. ValidRole is the door. |
| - Replica bool `json:"replica,omitempty"` |
| - |
| + // Replica is NOT a separate record field: Role carries the role |
| + // vocabulary alone (ValidRole's door), and every reader derives the |
| + // boolean from it. A second spelling of the same fact is two chances |
| + // to disagree. |
| + // |
| // Gate is the bcrypt hash of the host's shared password ("" = open, |
| // the value every record written before this field existed decodes |
| // to). The ONLY place the secret material lives in the bucket; the |
| @@ -492,12 +486,18 @@ func ValidateShape(rec Record) error { |
| return fmt.Errorf("steering %q: want \"\" or \"latency\"", rec.Steering) |
| } |
| // A replica is a process holding a mirror of live data — never a file |
| - // tree and never a foreign upstream the platform did not spawn. Pinning |
| + // tree, never a foreign upstream the platform did not spawn. Pinning |
| // the class here (not only at the reconciler's mint gate) keeps the rail |
| - // holding no matter which writer produced the record. |
| + // holding no matter which writer produced the record — and the ADDR half |
| + // matters as much as the kind half: a replica with a public addr would |
| + // pass everywhereClass's dial test and mint on EVERY non-fleet box, the |
| + // residence rule's second door. |
| if rec.Role == "replica" && rec.KindOrDefault() != "instance" { |
| return errors.New("role replica with kind " + rec.KindOrDefault() + ": a replica is an instance of the account — a process the activator spawns holding a mirror of its data — drop kind or drop role") |
| } |
| + if rec.Role == "replica" && len(rec.AddrsOrDefault()) > 0 { |
| + return errors.New("role replica with addr: a replica's process is the platform's own, spawned by the activator on the box its placement names — an upstream would make it an everywhere-class route and mint it on every box — drop the addr or drop role") |
| + } |
| for _, r := range rec.Replicas { |
| r = strings.TrimSpace(r) |
| if r != "" && !ValidRegion(r) { |
| @@ -669,26 +669,6 @@ var regionPoolRe = regexp.MustCompile(`^[a-z0-9-]{0,16}$`) |
| func ValidRegion(region string) bool { return regionPoolRe.MatchString(region) } |
| -// ReplicasOrDefault answers "where do this account's replicas live" from |
| -// the record's own two placement vocabularies: the explicit replica set |
| -// when the record names one, else the record's own single Region (the |
| -// pre-#162 vocabulary, folded so a record written before the set existed |
| -// still answers "one replica, in my own placement"), else none. A record |
| -// with an explicit set IGNORES its own Region: the set is the newer |
| -// vocabulary and the two are written together at the same door, never |
| -// independently. Callers that need the distinction the fold erases ("set |
| -// present but empty") read Replicas directly — no supported writer |
| -// produces that shape. |
| -func (rec Record) ReplicasOrDefault() []string { |
| - if len(rec.Replicas) > 0 { |
| - return rec.Replicas |
| - } |
| - if rec.Region != "" { |
| - return []string{rec.Region} |
| - } |
| - return nil |
| -} |
| - |
| // replicaSet renders a record's replica placement set for the wire column |
| // that carries it (registry.Route.Replicas): comma-joined placement codes in |
| // canonical (sorted, deduplicated, ValidRegion-checked at the door) order, |
| @@ -702,7 +682,9 @@ func replicaSet(regions []string) string { |
| // The door has already checked the set; a record that slipped past |
| // it (a hand-written bucket body — the reconcile pass reads those) |
| // still renders DETERMINISTICALLY, never as two different columns |
| - // on two boxes. Sorted, deduped, best-effort. |
| + // on two boxes. Sorted, deduped, best-effort — and LOGGED, so a |
| + // hand-edited body is visible rather than silently normalized. |
| + log.Printf("provision: replicas set failed validation (%v) — rendering the sorted, deduplicated remainder", err) |
| seen := map[string]bool{} |
| var keep []string |
| for _, r := range regions { |
diff --git a/internal/provision/provision_test.go b/internal/provision/provision_test.go| index 54145247e..849885ac4 100644 |
| --- a/internal/provision/provision_test.go |
| +++ b/internal/provision/provision_test.go |
| @@ -1130,22 +1130,14 @@ func TestRecordReplicaFieldsAreAdditive(t *testing.T) { |
| if err := json.Unmarshal([]byte(legacy), &rec); err != nil { |
| t.Fatal(err) |
| } |
| - if rec.Replica { |
| - t.Error("Replica = true on a legacy record, want false (absent is the home)") |
| - } |
| if rec.Role != "" { |
| t.Errorf("Role = %q, want empty", rec.Role) |
| } |
| if len(rec.Replicas) != 0 { |
| t.Errorf("Replicas = %v, want none", rec.Replicas) |
| } |
| - rec.Region = "ie" |
| - if got := rec.ReplicasOrDefault(); len(got) != 1 || got[0] != "ie" { |
| - t.Errorf("ReplicasOrDefault() = %v, want [ie] — the pre-set vocabulary folds to one replica, its own placement", got) |
| - } |
| - rec.Region = "" |
| - if got := rec.ReplicasOrDefault(); got != nil { |
| - t.Errorf("ReplicasOrDefault() = %v on a region-less record, want nil", got) |
| + if got := rec.Replicas; got != nil { |
| + t.Errorf("Replicas = %v on a legacy record, want nil", got) |
| } |
| } |
diff --git a/internal/provision/reconcile.go b/internal/provision/reconcile.go| index 308b25186..8b3d2d9e8 100644 |
| --- a/internal/provision/reconcile.go |
| +++ b/internal/provision/reconcile.go |
| @@ -200,6 +200,16 @@ func productionClear(ctx context.Context, s release.Store, pub ed25519.PublicKey |
| // elsewhere), while one wrongly promoted to everywhere is a box proxying to an |
| // upstream it cannot reach. |
| func everywhereClass(rec Record) bool { |
| + // A replica is the residence rule made structural (#162): it belongs to |
| + // the box its placement names and nowhere else, and this widening is the |
| + // mint gate for every non-fleet box. ValidateShape refuses a replica |
| + // with an upstream, so the addr arm below could never fire on one — but |
| + // "could never through THIS door" is not "cannot", and a replica of a |
| + // static kind would pass the kind arm above if the kind rail ever |
| + // moved. The role check is FIRST and absolute. |
| + if rec.Role == "replica" { |
| + return false |
| + } |
| // Unit names the systemd unit that owns this route's process (issue |
| // #110); adoption CYCLES it. That unit exists on the home box, so a far |
| // box adopting the row would be stamping authority over a unit name that |
| @@ -256,6 +266,12 @@ func everywhereClass(rec Record) bool { |
| // documents as human-gated, which is worse. The conservative gate stands; the |
| // gap is tracked in the bookkeeping issue, not papered over here. |
| func everywhereRoute(rt registry.Route) bool { |
| + // The replica mark is everywhereClass's role check, read off the row |
| + // (the record whose class it was may be exactly what is gone) — the |
| + // same FIRST-and-absolute position, for the same reason. |
| + if rt.Replica { |
| + return false |
| + } |
| if rt.Unit != "" || rt.Hibernate || rt.Backing != "" || rt.Tunnel != "" || rt.Caps != "" { |
| return false |
| } |
diff --git a/internal/provision/reconcile_test.go b/internal/provision/reconcile_test.go| index fd1f22aa4..e877c33dc 100644 |
| --- a/internal/provision/reconcile_test.go |
| +++ b/internal/provision/reconcile_test.go |
| @@ -2874,3 +2874,29 @@ func TestReplicaMarkMintsAndConverges(t *testing.T) { |
| t.Fatal("row keeps a replica mark its record no longer carries — the edges file would go on telling the fleet this box replicas an account it homes") |
| } |
| } |
| + |
| +// TestReplicaNeverEverywhereClass pins the residence rule's second door |
| +// (Fable review, finding 3): a replica is a process on ONE box — the one its |
| +// placement names — and everywhereClass's widening is the mint gate for |
| +// every non-fleet box. The class check refuses a replica FIRST (before any |
| +// kind or addr test), and everywhereRoute agrees, because the two must |
| +// never disagree about a row whose record is gone. |
| +func TestReplicaNeverEverywhereClass(t *testing.T) { |
| + // Static would pass the kind arm on its own — the role check must be |
| + // first and absolute. |
| + if everywhereClass(Record{Role: "replica", Kind: "static"}) { |
| + t.Error("everywhereClass(replica static) = true — a replica must never widen past its placement box") |
| + } |
| + if everywhereClass(Record{Role: "replica", Kind: "instance"}) { |
| + t.Error("everywhereClass(replica instance) = true") |
| + } |
| + // A public-addr replica would pass the dial test on its own; the shape |
| + // door refuses it, and the class check must not depend on that door |
| + // holding. |
| + if err := ValidateShape(Record{Role: "replica", Addr: "203.0.113.5:8090"}); err == nil { |
| + t.Error("ValidateShape(replica with a public addr) accepted — the everywhere-class second door") |
| + } |
| + if everywhereRoute(registry.Route{Replica: true, Kind: "static"}) { |
| + t.Error("everywhereRoute(replica row) = true — the removal-phase half of the same question") |
| + } |
| +} |
diff --git a/internal/provision/replica.go b/internal/provision/replica.go| index e9cf1a134..e2283bf86 100644 |
| --- a/internal/provision/replica.go |
| +++ b/internal/provision/replica.go |
| @@ -1,65 +1,52 @@ |
| package provision |
| -// The replica reconciliation pass (#162, edge checkout). The instance record |
| -// family's own verb, exactly as the discussion's Ask 2 frames it: a replica |
| -// is the sandbox verb with a different role and a chosen placement — same |
| -// app, same account, role=replica, placed on the box that carries the |
| -// placement code. This file adds only what the RECORD needs that the |
| -// instance pass cannot express: |
| +// The replica provisioning surface (#162, edge checkout). The instance |
| +// record family's own verb, exactly as the discussion's Ask 2 frames it: a |
| +// replica is the sandbox verb with a different role and a chosen placement — |
| +// same app, same account, role=replica, placed on the box that carries the |
| +// placement code. |
| // |
| -// - SetRecordReplica provisions the record: same create-only CAS shape |
| -// CreateRecord uses (a lost race is a conflict, never a silent |
| -// overwrite), with the replica's placement riding the record's own |
| -// Region column and the role naming what it is. |
| +// The record the console's create door writes (apiinstances.go) and the |
| +// record a home's own code would write share ONE constructor, Replica, so |
| +// "what a replica record looks like" has exactly one answer: everything |
| +// copied from the home record's policy half (app, account, pool, channel), |
| +// Region = the replica's placement, Role = "replica", and ValidateShape run |
| +// over the result — the same rails the door applies, because neither side |
| +// trusts the other about what is in the bucket. |
| // |
| -// - The replica's HOME link. The routing half of #162 publishes replica |
| -// placements from the home's own registry row (registry.Route.Replicas), |
| -// and the edges file on the REPLICA's box needs to know which account |
| -// (and therefore which home record) this replica serves — the replica's |
| -// box holds no home route row to read the set from. Home is written at |
| -// provisioning time and never converged afterwards: repointing a |
| -// replica at a different home is a delete-and-recreate, because what it |
| -// serves is a mirror OF something. |
| +// The routing half of #162 lives in the reconciler and the edges file; the |
| +// lease/drain protocol that runs BETWEEN home and replica is titogo's own. |
| import ( |
| "context" |
| - "fmt" |
| + "errors" |
| "time" |
| "github.com/carlosframework/platform/internal/release" |
| ) |
| -// ReplicaRecord is a Record that has passed the replica rails: ValidateShape |
| -// (which refuses a replica role on anything but an instance) has seen this |
| -// exact value. Constructors return it; nothing else may build one, so a |
| -// writer that wants to stamp the role onto a record must go through the |
| -// same validation the door applies. |
| -type ReplicaRecord struct { |
| - Record |
| -} |
| +// ErrReplicaOfReplica and ErrReplicaIncompleteHome are Replica's refusals: |
| +// a replica of a replica is not a shape the platform carries, and a home |
| +// link to an incomplete record would mint a replica whose account or app is |
| +// nothing the home ever was. |
| +var ( |
| + ErrReplicaOfReplica = errors.New("provision: record is itself a replica — a replica of a replica is not a shape the platform carries") |
| + ErrReplicaIncompleteHome = errors.New("provision: home record is not a complete instance record (host/app/account)") |
| +) |
| -// Replica turns a home-shaped record into the replica record SetRecordReplica |
| -// writes, validating the placement as it goes. host is the replica's own |
| -// host (one label + the app's instance domain — the same shape a home |
| -// claims), placement is the replica's placement code (ValidRegion's |
| -// vocabulary), home is the home record's host. Refusals mirror the console |
| -// create door's: an in-use placement vocabulary, a replica of a replica, |
| -// and a home link to a different account or app. |
| +// Replica turns a home-shaped record into the replica record |
| +// CreateRecordReplica writes, validating the result as it goes. host is the |
| +// replica's own host (one label + the app's instance domain — the same shape |
| +// a home claims), placement is the replica's placement code (ValidRegion's |
| +// vocabulary, the same one the create door validates as armed), home is the |
| +// home record as read from the bucket. The returned record has passed |
| +// ValidateShape — the proof ReplicaRecord's type carries. |
| func Replica(home Record, host, placement string) (ReplicaRecord, error) { |
| - if !ValidRole("replica") { // unreachable; pins the vocabulary's shape for the compiler-checked switch below |
| - return ReplicaRecord{}, fmt.Errorf("replica: role vocabulary is misconfigured") |
| - } |
| if home.Role == "replica" { |
| - return ReplicaRecord{}, fmt.Errorf("replica: record %s is itself a replica — a replica of a replica is not a shape the platform carries", home.Host) |
| + return ReplicaRecord{}, ErrReplicaOfReplica |
| } |
| if home.Host == "" || home.App == "" || home.Account == "" { |
| - return ReplicaRecord{}, fmt.Errorf("replica: home record %s is not a complete instance record (host/app/account)", home.Host) |
| - } |
| - if !ValidRegion(placement) { |
| - return ReplicaRecord{}, fmt.Errorf("replica: placement %q is not a placement code", placement) |
| - } |
| - if !ValidHostShape(host) { |
| - return ReplicaRecord{}, fmt.Errorf("replica: host %q is not a hostname", host) |
| + return ReplicaRecord{}, ErrReplicaIncompleteHome |
| } |
| rec := Record{ |
| Schema: 1, |
| @@ -73,9 +60,21 @@ func Replica(home Record, host, placement string) (ReplicaRecord, error) { |
| CreatedAt: time.Now().UTC(), |
| Role: "replica", |
| } |
| + if err := ValidateShape(rec); err != nil { |
| + return ReplicaRecord{}, err |
| + } |
| return ReplicaRecord{rec}, nil |
| } |
| +// ReplicaRecord is a Record that has passed the replica rails: ValidateShape |
| +// (which refuses a replica role on anything but a socket-backed instance) |
| +// has seen this exact value. Constructors return it; nothing else may build |
| +// one, so a writer that wants to stamp the role onto a record must go |
| +// through the same validation the door applies. |
| +type ReplicaRecord struct { |
| + Record |
| +} |
| + |
| // CreateRecordReplica writes the replica record — create-only, the |
| // CreateRecord contract (a lost race is release.ErrCASConflict, never a |
| // silent overwrite). The record MUST have come from Replica: the type is |
diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go| index 3bdb2fee1..43353b324 100644 |
| --- a/internal/registry/registry_test.go |
| +++ b/internal/registry/registry_test.go |
| @@ -77,6 +77,7 @@ func TestUpsertRoundTripsAllColumns(t *testing.T) { |
| Hibernate: true, Canary: true, Replication: "batched", |
| NextDue: 4200, Backing: BackingUnit, Unit: "console.service", |
| Steering: "latency", |
| + Replicas: "ie,uk,us", |
| Gate: "$2a$10$distinct", |
| CreatedAt: 1700000000, |
| } |
| @@ -93,6 +94,30 @@ func TestUpsertRoundTripsAllColumns(t *testing.T) { |
| if *got != want { |
| t.Fatalf("round trip mismatch:\n got %+v\n want %+v", *got, want) |
| } |
| + |
| + // The replica mark and the replica set are the #162 pair, and Upsert's |
| + // invariant blanks the SET on a row that IS a replica (a replica of a |
| + // replica is not a shape the platform carries) — so the mark needs its |
| + // own round trip, on a row whose set is empty. |
| + wantReplica := Route{ |
| + Host: "r.example.com", Slug: "r", Kind: "instance", |
| + Addr: "127.0.0.1:8081", Status: "ready", |
| + App: "kass", Pool: "shared", Region: "uk", Channel: "stable", |
| + Replica: true, CreatedAt: 1700000001, |
| + } |
| + if err := reg.Upsert(wantReplica); err != nil { |
| + t.Fatal(err) |
| + } |
| + gotReplica, err := reg.Get("r.example.com") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if gotReplica == nil { |
| + t.Fatal("Get returned nil for the replica row") |
| + } |
| + if *gotReplica != wantReplica { |
| + t.Fatalf("replica row round trip mismatch:\n got %+v\n want %+v", *gotReplica, wantReplica) |
| + } |
| } |
| // An addrs or health change must move the edge's Fingerprint — the edge only |
diff --git a/internal/steering/steering.go b/internal/steering/steering.go| index 0cf792c33..2e8dc7708 100644 |
| --- a/internal/steering/steering.go |
| +++ b/internal/steering/steering.go |
| @@ -56,31 +56,31 @@ type desiredRecord struct { |
| // Region field this fold needs (it is a Task 1 addition homemap.go has no |
| // consumer for). Hosts is deliberately absent: steering is any-edge- |
| // any-app (the spec's home-boxed-kinds-steer-via-backhaul rule). That |
| -// was true until #162: the replica-aware fold in desiredState DOES need |
| -// to know which hosts an edge serves, because the replicas' existence is |
| -// exactly what turns "who serves this host" into a question with more |
| -// than one answer — so Hosts is now carried, and the replica mark on |
| -// each entry is what lets the fold pick the home out of the serving set. |
| +// was true until #162: the replica-aware skip in desiredState DOES need to |
| +// know which hosts an edge serves, because the replica mark is exactly what |
| +// removes a record — a box whose file names a steered host with |
| +// replica:true is serving it from a read-only checkout shard, not as a |
| +// home. |
| + |
| +// edgeHostView is one entry of an edges file's hosts map, as the steering |
| +// fold reads it. A named type rather than an anonymous struct so tests and |
| +// readers spell it once; the only field this fold consults is the replica |
| +// mark — role values stay the writer's and homemap's business. |
| +type edgeHostView struct { |
| + Replica bool `json:"replica,omitempty"` |
| +} |
| + |
| type edgesFileView struct { |
| V int `json:"v"` |
| Box string `json:"box"` |
| EIP string `json:"eip"` |
| Region string `json:"region,omitempty"` |
| - // Replica marks an entry the publishing box serves as the account's |
| - // checkout replica (#162) rather than as the account's home. The |
| - // steering fold reads it because the edges file does not say WHICH box |
| - // is a home — only who serves what — and a home map answer of "several |
| - // boxes serve acme.go.tito.io" needs the replica marks to become "one |
| - // home plus replicas in ie and uk". Absent (every pre-#162 file) is |
| - // home: the only role v1 ever published. |
| - Replica bool `json:"replica,omitempty"` |
| // Hosts is the per-host map the file publishes (the writer's |
| - // internal/host/edgeHost shape, read only for the replica mark — the |
| - // ROLE values themselves stay this reader's business exactly as |
| - // before). |
| - Hosts map[string]struct { |
| - Replica bool `json:"replica,omitempty"` |
| - } `json:"hosts"` |
| + // internal/host/edgeHost shape; see edgeHostView for what survives the |
| + // decode). The mark rides the ENTRY (edgeHost.Replica on the writer |
| + // side, #162); a file-level flag would have no meaning — a box can |
| + // serve one account as a home and another as a replica. |
| + Hosts map[string]edgeHostView `json:"hosts"` |
| } |
| // parseEdgesFileView decodes one box's edges file body into the fields |
| @@ -352,22 +352,6 @@ func desiredState(routes []registry.Route, edges []edgesFileView, appsDomain str |
| return nil |
| } |
| - // servedByHost folds the ARMED edges' published hosts into one |
| - // host → serving-boxes index for the replica-aware fold below. Built |
| - // from `armed` rather than raw edges deliberately: an unarmed edge's |
| - // answer is unusable in a record anyway (no EIP, or a region Route53 |
| - // would reject batch-wide), so excluding it here keeps the fold's |
| - // candidate set and the record set from disagreeing. |
| - servedByHost := map[string][]edgesFileView{} |
| - for _, e := range armed { |
| - for host, h := range e.Hosts { |
| - if h.Replica { |
| - continue // replica entries never answer the HOME question |
| - } |
| - servedByHost[fqdn(host)] = append(servedByHost[fqdn(host)], e) |
| - } |
| - } |
| - |
| var out []desiredRecord |
| // TrimSpace OUTSIDE TrimSuffix, deliberately: TrimSuffix only strips a |
| // dot that is the very last byte, so the other order (TrimSuffix then |
| @@ -390,33 +374,17 @@ func desiredState(routes []registry.Route, edges []edgesFileView, appsDomain str |
| for _, r := range steeredHosts(routes, domain) { |
| name := fqdn(r.Host) |
| - // Replica-aware fold (#162): the edges files tell us which boxes |
| - // serve the host, not which box is its home — a replica-carrying |
| - // account is SERVED in several regions by design, and writing the |
| - // naive loop would publish latency records for the replicas too, |
| - // sending buyers' whole sessions to a read-only shard. The answer |
| - // folds each host's entries down to its HOME box first; a host no |
| - // box names as home (all replicas, mid-teardown) earns nothing — |
| - // "not known to be homed anywhere" is the safe direction for DNS. |
| - // |
| - // A single home (the overwhelmingly common shape) and a 2-home |
| - // mid-move window keep exactly today's records: the replica mark is |
| - // absent on every pre-#162 file, so homedEdge's filter passes |
| - // everything, and homedEdge returns the FIRST home-shaped entry |
| - // armed order gives it — same records the fold without replicas |
| - // would have written, whether or not a replica entry ever joined |
| - // the candidate set. |
| - cands := servedByHost[name] |
| - if len(cands) == 0 { |
| - continue |
| - } |
| - // EVERY home earns a record (the two-home mid-move window keeps |
| - // both — a replica mark is what removed entries, not a first-wins |
| - // pick); a host no box names as home (all replicas, mid-teardown) |
| - // earns nothing — "not known to be homed anywhere" is the safe |
| - // direction for DNS. |
| - for _, e := range cands { |
| - if e.Replica { |
| + // Replica-aware skip (#162): steering is any-edge-any-app (a far edge |
| + // terminates and BACKHAULS to the home — that is the whole design), |
| + // so every armed edge earns a record for a steered host exactly as it |
| + // did before #162, and the ONLY entries removed are the replica mark's: |
| + // a box whose file names the host WITH replica:true is serving it from |
| + // a read-only checkout shard, and steering buyers' whole sessions there |
| + // would be wrong in a way backhaul is not. Entries with no replica |
| + // mark — every pre-#162 file, every home — keep their records |
| + // byte-identically. |
| + for _, e := range armed { |
| + if h, named := e.Hosts[r.Host]; named && h.Replica { |
| continue |
| } |
| out = append(out, desiredRecord{Name: name, EIP: e.EIP, Box: e.Box, Region: e.Region}) |
diff --git a/internal/steering/steering_test.go b/internal/steering/steering_test.go| index 16f8da458..71f19edd9 100644 |
| --- a/internal/steering/steering_test.go |
| +++ b/internal/steering/steering_test.go |
| @@ -26,18 +26,15 @@ func steeredRoute(host string) registry.Route { |
| } |
| // TestDesiredStateReplicasDoNotSteer pins the #162 half of the fold: an |
| -// account whose edges files show a home (dublin) AND a replica (oregon, the |
| -// same host marked replica) gets latency records for its HOME only — never |
| -// for the replica — because steering a replica would send every buyer's |
| -// whole session to a read-only shard. The synthetic edge.<apps-domain> |
| -// records keep ALL armed edges regardless (every edge is a cache front); |
| -// only the per-host records are replica-aware. |
| +// account whose replica box publishes the host marked replica gets records |
| +// from EVERY armed edge EXCEPT that one — the pre-#162 rule is any-edge- |
| +// any-app (far edges backhaul), so the ONLY entries a replica mark removes |
| +// are its own. The synthetic edge.<apps-domain> records keep ALL armed |
| +// edges regardless (every edge is a cache front). |
| func TestDesiredStateReplicasDoNotSteer(t *testing.T) { |
| dublin := edgeHomes("dublin", "1.2.3.4", "eu-west-1", "app.example.com") |
| oregon := edge("oregon", "5.6.7.8", "us-west-2") |
| - oregon.Hosts = map[string]struct { |
| - Replica bool `json:"replica,omitempty"` |
| - }{ |
| + oregon.Hosts = map[string]edgeHostView{ |
| "app.example.com": {Replica: true}, |
| } |
| @@ -53,16 +50,19 @@ func TestDesiredStateReplicasDoNotSteer(t *testing.T) { |
| t.Fatalf("desiredState() =\n%#v\nwant\n%#v", got, want) |
| } |
| - // And the all-replicas case: a host no box names as home earns nothing |
| - // ("not known to be homed anywhere" is the safe direction for DNS), |
| - // while the synthetic edge hostname still lands on every armed edge. |
| - got = desiredState([]registry.Route{steeredRoute("app.example.com")}, |
| - []edgesFileView{oregon, oregon}, "example.com") |
| + // A host named by NO box still earns records from every armed edge: |
| + // steering is any-edge-any-app, and the far edge backhauls to the home |
| + // whether or not this box holds a row for the host. |
| + got = desiredState([]registry.Route{steeredRoute("never-published.example.com")}, |
| + []edgesFileView{dublin, oregon}, "example.com") |
| want = []desiredRecord{ |
| + {Name: "edge.example.com.", EIP: "1.2.3.4", Box: "dublin", Region: "eu-west-1"}, |
| {Name: "edge.example.com.", EIP: "5.6.7.8", Box: "oregon", Region: "us-west-2"}, |
| + {Name: "never-published.example.com.", EIP: "1.2.3.4", Box: "dublin", Region: "eu-west-1"}, |
| + {Name: "never-published.example.com.", EIP: "5.6.7.8", Box: "oregon", Region: "us-west-2"}, |
| } |
| if !reflect.DeepEqual(got, want) { |
| - t.Fatalf("all-replicas desiredState() =\n%#v\nwant\n%#v", got, want) |
| + t.Fatalf("unpublished-host desiredState() =\n%#v\nwant\n%#v", got, want) |
| } |
| } |
| @@ -77,13 +77,9 @@ func edge(box, eip, region string) edgesFileView { |
| // every entry is home-marked (Replica clear) unless a test says otherwise. |
| func edgeHomes(box, eip, region string, homes ...string) edgesFileView { |
| e := edge(box, eip, region) |
| - e.Hosts = map[string]struct { |
| - Replica bool `json:"replica,omitempty"` |
| - }{} |
| + e.Hosts = map[string]edgeHostView{} |
| for _, h := range homes { |
| - e.Hosts[h] = struct { |
| - Replica bool `json:"replica,omitempty"` |
| - }{} |
| + e.Hosts[h] = edgeHostView{} |
| } |
| return e |
| } |
| @@ -471,7 +467,7 @@ func TestParseEdgesFileView(t *testing.T) { |
| // scalar fields are the ones this test pins; Hosts is read |
| // only for the replica mark in desiredState, pinned there. |
| if got.V != tt.want.V || got.Box != tt.want.Box || got.EIP != tt.want.EIP || |
| - got.Region != tt.want.Region || got.Replica != tt.want.Replica { |
| + got.Region != tt.want.Region { |
| t.Fatalf("parseEdgesFileView(%q) = %+v, want %+v", tt.body, got, tt.want) |
| } |
| }) |