amadan
host: fix edges-file review findings — EIP hold, ready-gate, wildcard doc, wire pin
Four fixes from Task 1 review: - Critical: readPublicIP's transient "" no longer blanks a known-good EIP — edgesFilePass falls back to the prior published file's EIP (read from *last) whenever this pass's probe comes back empty and a file has already been published. "" only ever ships for a box that has never had a discoverable IP. - Important: rows are filtered to Status == "ready" before entering Hosts, mirroring edge.Reload's own definition of "serves" (internal/edge/edge.go) — a pending/creating/failed row is never dialed by any edge and must not claim role:"home". - Important: documented (writer doc-comment + spec sentence) that Hosts keys are registry row hosts verbatim, including *.parent catch-all rows, and that an exact-match reader must apply lookupUpstream's one-label wildcard rule itself. - Important: added a raw-wire-body test pinned against a hand-written literal of the brief's exact schema (independent of this package's struct tags), so a json-tag rename fails loudly instead of staying green under the existing round-trip tests. Four new tests, each verified to fail against the pre-fix code by temporarily reverting the corresponding fix and re-running. Covering tests: go test ./internal/host/ -run EdgesFile -v (10/10 pass), go test ./internal/host/ -race -count=1, go test ./... -count=1, go build/vet (+GOOS=darwin), gofmt -l — all clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 files changed,
+179
−5
docs/superpowers/specs/2026-08-20-multi-region-ingress-design.md+6 −1internal/host/edgesfile.go+36 −4internal/host/edgesfile_test.go+137 −0
diff --git a/docs/superpowers/specs/2026-08-20-multi-region-ingress-design.md b/docs/superpowers/specs/2026-08-20-multi-region-ingress-design.md| index 95ba18281..8ce135c34 100644 |
| --- a/docs/superpowers/specs/2026-08-20-multi-region-ingress-design.md |
| +++ b/docs/superpowers/specs/2026-08-20-multi-region-ingress-design.md |
| @@ -192,7 +192,12 @@ certs ruling; edges mirror the `control/edges/` prefix on their own |
| 30-second poll, the same cadence the certs mirror uses. Every edge |
| folds these files into a deployment-wide host→home map. The file |
| doubles as the DR runbook's explicit statement of what a dead box was |
| -serving. |
| +serving. **Keys are registry row hosts verbatim, including `*.parent` |
| +catch-all rows** — this writer performs no wildcard expansion, so a |
| +reader that needs exact-host answers must apply `lookupUpstream`'s |
| +one-label wildcard rule (`internal/edge/edge.go`) itself: an exact |
| +match wins, and on a miss the first label's leading dot is swapped for |
| +`*` before trying again. |
| **The hop.** A remote edge dials `home-EIP:443` with ordinary SNI — the |
| home edge presents the same bucket-distributed cert. Two headers make it |
diff --git a/internal/host/edgesfile.go b/internal/host/edgesfile.go| index 47a9b7c56..4fa38a39d 100644 |
| --- a/internal/host/edgesfile.go |
| +++ b/internal/host/edgesfile.go |
| @@ -41,7 +41,7 @@ type edgesFile struct { |
| } |
| // edgesFilePass builds this box's edges file from the routes currently in |
| -// reg (every row this box serves, ALL kinds — the serving-class split |
| +// reg (every SERVED row on this box, ALL kinds — the serving-class split |
| // belongs to the reader, Task 3, not to this writer) and publishes it at |
| // edgesFileKey(box) when it changed. |
| // |
| @@ -53,9 +53,16 @@ type edgesFile struct { |
| // cleared) and is updated only after a successful write, mirroring |
| // registrySnapshotPass's *lastFP contract. |
| // |
| -// eip may be "" (readPublicIP's "unknown" case, e.g. not on EC2 or IMDS |
| -// disabled) — the file is still published, with `"eip":""`, so the DR |
| -// runbook's hosts list never depends on IMDS working. |
| +// eip may be "" (readPublicIP's "unknown" case, e.g. not on EC2, IMDS |
| +// disabled, or a transient metadata-endpoint failure). A blank reading is |
| +// NOT taken at face value: this box's public IP does not change tick to |
| +// tick, so "" almost always means the probe hiccuped, not that the address |
| +// vanished — publishing it verbatim would blank a known-good EIP for |
| +// every remote edge's dial address for up to one loop cadence. When eip is |
| +// "" and *last already holds a published file, this pass reuses that |
| +// file's EIP instead. "" only ever reaches the wire for a box that has |
| +// NEVER had a discoverable IP — the DR runbook's hosts list still gets |
| +// published in that case, just without a dial address. |
| func edgesFilePass(ctx context.Context, s release.Store, reg *registry.Registry, box, eip string, last *[]byte) error { |
| routes, err := reg.List() |
| if err != nil { |
| @@ -63,9 +70,34 @@ func edgesFilePass(ctx context.Context, s release.Store, reg *registry.Registry, |
| } |
| hosts := make(map[string]edgeHost, len(routes)) |
| for _, r := range routes { |
| + // edge.Reload (internal/edge/edge.go) is the deployment's own |
| + // definition of "serves": it skips any row whose Status != "ready" |
| + // because a pending/creating/failed row has no working backend and |
| + // is never dialed by any edge, home or remote. Mirroring that gate |
| + // here means this file never claims role:"home" for a host nothing |
| + // actually serves. Hibernating routes stay Status "ready" (that is |
| + // what makes wake-on-request possible), so nothing reachable is |
| + // excluded by this filter. |
| + if r.Status != "ready" { |
| + continue |
| + } |
| + // Keys are registry row Host values verbatim, INCLUDING *.parent |
| + // catch-all wildcard rows (Task 3's reconcile pass derives these |
| + // from a claim's catch-all row, same as edge.go's upstream map) — |
| + // this writer performs no expansion. A reader that needs exact-host |
| + // answers must apply edge.go's lookupUpstream one-label wildcard |
| + // rule itself: an exact match wins, and on a miss "*.<parent>" |
| + // (the leading dot of the first label only) is tried next. |
| hosts[r.Host] = edgeHost{Role: "home"} |
| } |
| + if eip == "" && *last != nil { |
| + var prior edgesFile |
| + if err := json.Unmarshal(*last, &prior); err == nil { |
| + eip = prior.EIP |
| + } |
| + } |
| + |
| f := edgesFile{V: 1, Box: box, EIP: eip, Hosts: hosts} |
| gate, err := json.Marshal(f) |
| if err != nil { |
diff --git a/internal/host/edgesfile_test.go b/internal/host/edgesfile_test.go| index 172261c80..162db2870 100644 |
| --- a/internal/host/edgesfile_test.go |
| +++ b/internal/host/edgesfile_test.go |
| @@ -5,6 +5,7 @@ import ( |
| "encoding/json" |
| "path/filepath" |
| "testing" |
| + "time" |
| "github.com/carlosframework/platform/internal/registry" |
| "github.com/carlosframework/platform/internal/release" |
| @@ -131,3 +132,139 @@ func TestEdgesFileSkipsWriteWhenUnchanged(t *testing.T) { |
| t.Fatalf("changed third pass puts = %d, want 2", s.puts) |
| } |
| } |
| + |
| +// FINDING 1 (review, 2026-08-22): readPublicIP is a single 2s best-effort |
| +// IMDS attempt, re-run every tick — a transient throttle or blip returns "" |
| +// exactly like a box that never had a public IP at all. Without a hold, a |
| +// single hiccuped tick would blank a known-good EIP in the published file, |
| +// costing every remote edge its dial address for this box's home-boxed |
| +// hosts until the next successful tick (up to 5min + jitter). A pass |
| +// reporting "" after a pass that reported "1.2.3.4" must keep "1.2.3.4". |
| +func TestEdgesFileHoldsLastKnownGoodEIPOnTransientBlank(t *testing.T) { |
| + s, reg := release.NewDirStore(t.TempDir()), edgesTestRegistry(t) |
| + seedRoute(t, reg, "app.example.com") |
| + |
| + var last []byte |
| + if err := edgesFilePass(context.Background(), s, reg, "annie", "1.2.3.4", &last); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + // Simulate a transient IMDS blip: this tick's probe returns "". |
| + if err := edgesFilePass(context.Background(), s, reg, "annie", "", &last); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + got, _, err := s.Get(context.Background(), "control/edges/annie.json") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + var f edgesFile |
| + if err := json.Unmarshal(got, &f); err != nil { |
| + t.Fatal(err) |
| + } |
| + if f.EIP != "1.2.3.4" { |
| + t.Fatalf("eip after a blank probe = %q, want held value %q", f.EIP, "1.2.3.4") |
| + } |
| +} |
| + |
| +// FINDING 2 (review, 2026-08-22, ruling): edge.Reload (internal/edge/edge.go) |
| +// skips any route whose Status != "ready" — that gate is the deployment's |
| +// own definition of "serves", so a pending/creating/failed row must not |
| +// claim role:"home" here either. Hibernating routes stay Status "ready", |
| +// so this filter excludes nothing a remote backhaul could actually reach. |
| +func TestEdgesFileExcludesNonReadyRows(t *testing.T) { |
| + s, reg := release.NewDirStore(t.TempDir()), edgesTestRegistry(t) |
| + seedRoute(t, reg, "ready.example.com") |
| + if err := reg.Upsert(registry.Route{Host: "creating.example.com", Kind: "service", Addr: "10.0.0.2:3000", Status: "creating"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + var last []byte |
| + if err := edgesFilePass(context.Background(), s, reg, "annie", "1.2.3.4", &last); err != nil { |
| + t.Fatal(err) |
| + } |
| + got, _, err := s.Get(context.Background(), "control/edges/annie.json") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + var f edgesFile |
| + if err := json.Unmarshal(got, &f); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok := f.Hosts["ready.example.com"]; !ok { |
| + t.Fatalf("hosts = %+v, want ready.example.com present", f.Hosts) |
| + } |
| + if _, ok := f.Hosts["creating.example.com"]; ok { |
| + t.Fatalf("hosts = %+v, want creating.example.com ABSENT (not ready, nothing serves it yet)", f.Hosts) |
| + } |
| +} |
| + |
| +// FINDING 3 (review, 2026-08-22): a catch-all claim's registry row is a real |
| +// "*.parent" host (Task 3's reconcile pass derives it the same way |
| +// edge.go's upstream map does). This writer performs no wildcard expansion |
| +// — the row's Host lands in Hosts verbatim, exactly like every other row — |
| +// so an exact-match reader must apply edge.go's lookupUpstream one-label |
| +// rule itself; that contract is documented at the call site and in the |
| +// spec, not enforced here. |
| +func TestEdgesFileWildcardRowPublishesVerbatim(t *testing.T) { |
| + s, reg := release.NewDirStore(t.TempDir()), edgesTestRegistry(t) |
| + if err := reg.Upsert(registry.Route{Host: "*.example.com", Kind: "service", Addr: "10.0.0.3:3000", Status: "ready"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + var last []byte |
| + if err := edgesFilePass(context.Background(), s, reg, "annie", "1.2.3.4", &last); err != nil { |
| + t.Fatal(err) |
| + } |
| + got, _, err := s.Get(context.Background(), "control/edges/annie.json") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + var f edgesFile |
| + if err := json.Unmarshal(got, &f); err != nil { |
| + t.Fatal(err) |
| + } |
| + if r, ok := f.Hosts["*.example.com"]; !ok || r.Role != "home" { |
| + t.Fatalf("hosts = %+v, want \"*.example.com\" present verbatim with role home", f.Hosts) |
| + } |
| +} |
| + |
| +// FINDING 4 (review, 2026-08-22): every other test in this file round-trips |
| +// through the edgesFile struct on both the write and the assert side, so a |
| +// json-tag rename (e.g. "eip" -> "publicIP") would stay green everywhere |
| +// while breaking Task 3's parser, which reads the wire bytes independently |
| +// of this package's Go types. This test pins the RAW body against a |
| +// hand-written literal matching the brief's exact schema string — every |
| +// byte is fixed except the generation timestamp, which this fixed |
| +// signature has no clock seam to inject (edgesFilePass takes no `now` |
| +// parameter, per the brief's pinned interface); that one field is read |
| +// back and re-inserted into the literal rather than relaxed by a regex. |
| +func TestEdgesFileRawWireBodyMatchesSchema(t *testing.T) { |
| + s, reg := release.NewDirStore(t.TempDir()), edgesTestRegistry(t) |
| + seedRoute(t, reg, "app.example.com") |
| + |
| + var last []byte |
| + if err := edgesFilePass(context.Background(), s, reg, "annie-welding", "99.81.104.219", &last); err != nil { |
| + t.Fatal(err) |
| + } |
| + got, _, err := s.Get(context.Background(), "control/edges/annie-welding.json") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + var probe struct { |
| + Generation string `json:"generation"` |
| + } |
| + if err := json.Unmarshal(got, &probe); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := time.Parse(time.RFC3339, probe.Generation); err != nil { |
| + t.Fatalf("generation %q is not RFC3339: %v", probe.Generation, err) |
| + } |
| + |
| + want := `{"v":1,"box":"annie-welding","eip":"99.81.104.219","generation":"` + |
| + probe.Generation + `","hosts":{"app.example.com":{"role":"home"}}}` |
| + if string(got) != want { |
| + t.Fatalf("raw wire body =\n%s\nwant\n%s", got, want) |
| + } |
| +} |