amadan

carlos / platform Public

edge: the watch re-bases local holds on an alias's DNS stamp, Reload publishes its table and holds as one snapshot, ExactHome refuses a literal catch-all (Task 2 final fix round)

Astra's plan 3 final review, F1 to F3.

- F1: the registry fingerprint leaves alias DNS status out, so a DNS-only
  stamp never reloaded and a held row's explicit alias kept its stale
  hold eligibility. Watch's tick is now watchTick; while the holds'
  rows hold a row it also reads a digest of the explicit alias rows'
  (Alias, Host, DNSStatus, Explicit), and when only that moved it
  re-lists the rows on the watch goroutine (outside the view's lock)
  and re-bases the holds (refreshHoldRows) without a Reload.
  TestHoldingAliasFollowsDNSStamp drives both transitions through the
  real tick.
- F2: Reload published the table before the holds built from it, so a
  request in between could see neither. holdSnapshot now carries its
  table; installTable publishes both under holdsMu and resolve and
  Allowed read them from one pointer (servingPair).
  TestReloadPublishesReadyAndHoldsAtomically pauses a Reload inside its
  rebuild, both ways.
- F3: ExactHome refuses a literal "*." name, as Allowed and KnownHost do.
- Task 7's harness: a comment that its unconditional per-round Reload
  masked F1 (behaviour unchanged).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
claude a0f6cf559e7e71122159f6e7cd731187ad62f58a parent d04ba6b
5 files changed, +329 −54
  • internal/e2e/selfserve/holding_world_test.go +3 −0
  • internal/edge/decision_test.go +122 −0
  • internal/edge/edge.go +103 −47
  • internal/edge/holding.go +98 −6
  • internal/edge/homemap.go +3 −1
diff --git a/internal/e2e/selfserve/holding_world_test.go b/internal/e2e/selfserve/holding_world_test.go
index 435de27e2..35d6e298a 100644
--- a/internal/e2e/selfserve/holding_world_test.go
+++ b/internal/e2e/selfserve/holding_world_test.go
@@ -397,6 +397,9 @@ func (w *holdWorld) newThirdEdge(t *testing.T) *boxEdge {
func (w *holdWorld) converge() error {
w.t.Helper()
for _, be := range []*boxEdge{w.src, w.tgt} {
+ // An unconditional Reload, not the watch's fingerprint decision: it
+ // masked final review F1 (a DNS-only alias stamp the fingerprint skips);
+ // edge's TestHoldingAliasFollowsDNSStamp drives the real watch tick.
if err := be.e.Reload(); err != nil {
w.t.Fatalf("%s: reload: %v", be.label, err)
}
diff --git a/internal/edge/decision_test.go b/internal/edge/decision_test.go
index ba484e933..3de0f4e26 100644
--- a/internal/edge/decision_test.go
+++ b/internal/edge/decision_test.go
@@ -248,6 +248,19 @@ func TestResolveExactVersusRequest(t *testing.T) {
t.Fatal("an expired hold with nothing behind it is no longer a member")
}
})
+ t.Run("ExactHome rejects a literal wildcard name", func(t *testing.T) {
+ sc := newScene(t, "carol")
+ sc.publish("betty", "2.2.2.2", `"*.example.com":{"role":"home"}`, "")
+ if _, _, ok := sc.m.ExactHome("*.example.com"); ok {
+ t.Fatal("the catch-all key itself is not an exact name")
+ }
+ if _, box, ok := sc.m.Home("shop.example.com"); !ok || box != "betty" {
+ t.Fatalf("a concrete request host still resolves through the catch-all: %q %v", box, ok)
+ }
+ if d := sc.e.resolve("shop.example.com", sc.now); d.kind != decRemote || d.box != "betty" {
+ t.Fatalf("resolve = %v %q, want betty's catch-all", d.kind, d.box)
+ }
+ })
t.Run("Allowed rejects the wildcard label and admits the tunnel host", func(t *testing.T) {
sc := newScene(t, "carol")
sc.route("*.example.com", "local wildcard", "bab", "wild")
@@ -675,3 +688,112 @@ func TestLocalHoldsFollowTheViewAndTheRegistry(t *testing.T) {
}
})
}
+
+// An explicit alias of a held row follows its DNS stamp through the real
+// registry watch (final review F1): the registry fingerprint leaves alias
+// DNS status out on purpose, so no Reload runs, and the watch's holds
+// digest is what re-lists the rows outside the view's lock.
+func TestHoldingAliasFollowsDNSStamp(t *testing.T) {
+ sc := newScene(t, "annie")
+ sc.heldRow("shop.example.com", "bab", "shop", "mv_r", 1, 1)
+ sc.alias("shop.example.net", "shop.example.com", "pending", true)
+ sc.view(map[string]HoldState{"shop.example.com": moveR})
+ var st watchState
+ sc.e.watchTick(&st) // the first tick baselines (and reloads)
+ var reloads int
+ sc.e.OnReload(func() { reloads++ })
+ aliasHeld := func() bool {
+ return sc.e.resolve("shop.example.net", sc.now).kind == decHolding && sc.e.Allowed("shop.example.net")
+ }
+ if aliasHeld() {
+ t.Fatal("a pending alias is not held")
+ }
+ fp, err := sc.reg.Fingerprint()
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, step := range []struct {
+ dns string
+ want bool
+ }{{"pointing", true}, {"pending", false}, {"pointing", true}} {
+ if err := sc.reg.SetAliasDNSStatus("shop.example.net", step.dns, time.Now().Unix()); err != nil {
+ t.Fatal(err)
+ }
+ if now, _ := sc.reg.Fingerprint(); now != fp {
+ t.Fatal("test bug: the fingerprint must not see a DNS stamp")
+ }
+ sc.e.watchTick(&st)
+ if aliasHeld() != step.want {
+ t.Fatalf("after the stamp turned %s: held %v, want %v", step.dns, aliasHeld(), step.want)
+ }
+ if step.want {
+ wantPage(t, sc.serve("shop.example.net", "/", ordinary), "the alias once pointing")
+ } else {
+ wantUnknown(t, sc.serve("shop.example.net", "/", ordinary), "the alias once pending")
+ }
+ }
+ if reloads != 0 {
+ t.Fatalf("the watch reloaded %d time(s) for a DNS stamp; only the holds are rebuilt", reloads)
+ }
+ // A quiet tick re-lists nothing and changes nothing.
+ sc.e.watchTick(&st)
+ if !aliasHeld() || reloads != 0 {
+ t.Fatal("a quiet tick changed the holds or reloaded")
+ }
+}
+
+// A Reload publishes its table and the holds built from it together (final
+// review F2): a request between the table swap and the holds rebuild sees
+// the earlier pair (the ready row, or the hold), never neither.
+func TestReloadPublishesReadyAndHoldsAtomically(t *testing.T) {
+ sc := newScene(t, "annie")
+ sc.route("shop.example.com", "shop", "bab", "shop", func(r *registry.Route) { r.Incarnation, r.ServingSeq = 1, 1 })
+ sc.view(map[string]HoldState{"shop.example.com": moveR})
+ // pauseReload runs a Reload on its own goroutine, holds it inside the
+ // holds rebuild (after its table has been built and swapped in), runs
+ // probe there, then lets it finish.
+ pauseReload := func(probe func()) {
+ t.Helper()
+ paused, release := make(chan struct{}), make(chan struct{})
+ var once sync.Once
+ sc.e.holdsRead = func(*HoldSnapshot) {
+ once.Do(func() {
+ close(paused)
+ <-release
+ })
+ }
+ done := make(chan error, 1)
+ go func() { done <- sc.e.Reload() }()
+ <-paused
+ probe()
+ close(release)
+ if err := <-done; err != nil {
+ t.Fatal(err)
+ }
+ sc.e.holdsRead = nil
+ }
+
+ // ready -> held: mid-reload the request still gets the ready row.
+ if err := sc.reg.SetHold("shop.example.com", "moving", "mv_r"); err != nil {
+ t.Fatal(err)
+ }
+ pauseReload(func() {
+ wantBody(t, sc.serve("shop.example.com", "/", ordinary), "shop", "ready to held, mid-reload")
+ })
+ wantPage(t, sc.serve("shop.example.com", "/", ordinary), "held, after the reload")
+
+ // held -> ready: mid-reload the request still gets the page.
+ if _, err := sc.reg.ReleaseHold("shop.example.com", "mv_r"); err != nil {
+ t.Fatal(err)
+ }
+ if err := sc.reg.SetStatus("shop.example.com", "ready"); err != nil {
+ t.Fatal(err)
+ }
+ pauseReload(func() {
+ wantPage(t, sc.serve("shop.example.com", "/", ordinary), "held to ready, mid-reload")
+ if !sc.e.Allowed("shop.example.com") {
+ t.Fatal("mid-reload the name must stay Allowed")
+ }
+ })
+ wantBody(t, sc.serve("shop.example.com", "/", ordinary), "shop", "ready, after the reload")
+}
diff --git a/internal/edge/edge.go b/internal/edge/edge.go
index 8f6057575..c8b740eed 100644
--- a/internal/edge/edge.go
+++ b/internal/edge/edge.go
@@ -1012,13 +1012,13 @@ func (e *Edge) Reload() error {
// host being deleted is a moment early, a 503 for a host being added is
// simply wrong.
e.probers.Sync(probeSpecs)
- e.upstreams.Store(&m)
- // The local holds (holding.go) are rebuilt from the rows this reload
- // read: a row turning held, an alias detached, a provenance or DNS
- // eligibility change all land here with the view unchanged. Before the
- // reload hook, so the wildcard manager's re-derivation sees them.
- e.holdInputs.Store(&holdInputs{routes: routes, aliases: aliases, table: m})
- e.rebuildHolds()
+ // The table and the local holds (holding.go) built from the rows this
+ // reload read are published together (installTable): a row turning
+ // held, an alias detached, a provenance or DNS eligibility change all
+ // land here with the view unchanged, and the request path never sees
+ // the new table without its holds. Before the reload hook, so the
+ // wildcard manager's re-derivation sees them.
+ e.installTable(routes, aliases, m)
// Reconcile the activator with the new route set: a host un-flagged (or
// removed) since the last reload may still have a live child holding its
// socket and lease — retire it so a systemd unit can take over cleanly.
@@ -1190,10 +1190,11 @@ func (e *Edge) Allowed(host string) bool {
// moves plan 3): a held row's host and its explicit publishable aliases
// are registry-backed exact names this box answers with the holding
// page, and need their certificate through the move.
- if _, ok := e.current()[host]; ok {
+ table, holds := e.servingPair()
+ if _, ok := table[host]; ok {
return true
}
- return len(e.localHoldsFor(host)) > 0
+ return len(holds.candsFor(host)) > 0
}
// deploymentHost reports whether host is served HERE (Allowed) or is exactly
@@ -1298,7 +1299,9 @@ func localReady(name string, up upstream, table map[string]upstream) candidate {
// losing local hold does not count) and it has the "*.<parent>" upstream,
// that upstream, from this same table, travels with the decision.
func (e *Edge) resolve(host string, now time.Time) decision {
- table := e.current()
+ // The table and the local holds from one read (servingPair), so a
+ // Reload is seen whole or not at all.
+ table, holds := e.servingPair()
var remote map[string][]candidate
if hm := e.homes.Load(); hm != nil && hm.self != "" {
remote = hm.snapshot().cands
@@ -1308,7 +1311,7 @@ func (e *Edge) resolve(host string, now time.Time) decision {
if up, ok := table[name]; ok {
cs = append(cs, localReady(name, up, table))
}
- cs = append(cs, e.localHoldsFor(name)...)
+ cs = append(cs, holds.candsFor(name)...)
return append(cs, remote[name]...)
}
d := rank(host, gather(host), now)
@@ -2449,9 +2452,7 @@ func (e *Edge) Watch(stop <-chan struct{}) {
// triggers a reload, so a route added between the edge's startup reload
// and this poll can't be silently baselined away. (Eleven's version has
// that gap; one redundant reload at startup is the cost of closing it.)
- var last string
- errs := 0
- reloadErrs := 0
+ var st watchState
tick := time.NewTicker(2 * time.Second)
defer tick.Stop()
for {
@@ -2460,43 +2461,98 @@ func (e *Edge) Watch(stop <-chan struct{}) {
return
case <-tick.C:
}
- fp, err := e.reg.Fingerprint()
- if err != nil {
- errs++
- if errs == 1 || errs%300 == 0 {
- log.Printf("edge: registry-watch poll error (#%d): %v", errs, err)
- }
- continue
- }
- errs = 0
- // The edge's own loop is alive, its routing table readable, and the
- // :443 path's shared lock free (region HA spec 2 §6, servingPathLive).
- // A wedged poll or a wedged request path stops beating.
- if e.lbHealth != nil && e.servingPathLive() {
- e.lbHealth.EdgeBeat(time.Now())
+ e.watchTick(&st)
+ }
+}
+
+// watchState is what the watch carries from one tick to the next.
+type watchState struct {
+ last string
+ errs, reloadErrs int
+ // lastHolds is the holds digest (holdAliasDigest) the holds were last
+ // built against; holdErrs counts its read failures.
+ lastHolds string
+ holdErrs int
+}
+
+// watchTick is one poll of the watch: the fingerprint, the serving-path
+// beat, and a reload when the fingerprint moved. While the box holds a row,
+// it also watches the one input of the local holds the fingerprint leaves
+// out on purpose (an explicit alias's DNS stamp, holdAliasDigest): when only
+// that moved, it re-lists the rows here, outside the view's lock, and
+// re-bases the holds on them (refreshHoldRows) without a Reload.
+func (e *Edge) watchTick(st *watchState) {
+ fp, err := e.reg.Fingerprint()
+ if err != nil {
+ st.errs++
+ if st.errs == 1 || st.errs%300 == 0 {
+ log.Printf("edge: registry-watch poll error (#%d): %v", st.errs, err)
}
- if fp == last {
- continue
+ return
+ }
+ st.errs = 0
+ // The edge's own loop is alive, its routing table readable, and the
+ // :443 path's shared lock free (region HA spec 2 §6, servingPathLive).
+ // A wedged poll or a wedged request path stops beating.
+ if e.lbHealth != nil && e.servingPathLive() {
+ e.lbHealth.EdgeBeat(time.Now())
+ }
+ // Read before any reload: a stamp landing after it is caught on the next
+ // tick, never baselined away.
+ holds, aliases, armed, herr := e.holdAliasDigest()
+ if fp == st.last {
+ e.watchHolds(st, holds, aliases, armed, herr)
+ return
+ }
+ // last is baselined only on a SUCCESSFUL reload: a fleet detach is a
+ // revocation, and if Reload fails on this tick, the fingerprint it
+ // was meant to apply must not be discarded — baselining it here
+ // (before Reload runs) would make the change invisible to every
+ // later tick until some UNRELATED write moves the fingerprint again,
+ // silently sitting on a revocation that never took effect. Leaving
+ // last unchanged means the very next tick sees the same fp != last
+ // and retries.
+ if err := e.Reload(); err != nil {
+ st.reloadErrs++
+ if st.reloadErrs == 1 || st.reloadErrs%300 == 0 {
+ log.Printf("edge: registry-watch reload failed (#%d): %v", st.reloadErrs, err)
}
- // last is baselined only on a SUCCESSFUL reload: a fleet detach is a
- // revocation, and if Reload fails on this tick, the fingerprint it
- // was meant to apply must not be discarded — baselining it here
- // (before Reload runs) would make the change invisible to every
- // later tick until some UNRELATED write moves the fingerprint again,
- // silently sitting on a revocation that never took effect. Leaving
- // last unchanged means the very next tick sees the same fp != last
- // and retries.
- if err := e.Reload(); err != nil {
- reloadErrs++
- if reloadErrs == 1 || reloadErrs%300 == 0 {
- log.Printf("edge: registry-watch reload failed (#%d): %v", reloadErrs, err)
- }
- continue
+ return
+ }
+ st.reloadErrs = 0
+ st.last = fp
+ st.lastHolds = ""
+ if armed && herr == nil {
+ st.lastHolds = holds
+ }
+ log.Printf("edge: registry changed — reloaded (%d route(s))", len(e.current()))
+}
+
+// watchHolds is watchTick's holds half, on a tick whose fingerprint did not
+// move: a changed digest re-lists the rows and re-bases the holds. A failed
+// read leaves the baseline, so the next tick retries.
+func (e *Edge) watchHolds(st *watchState, holds string, aliases []registry.RouteAlias, armed bool, herr error) {
+ if !armed {
+ st.lastHolds = ""
+ return
+ }
+ if herr == nil && holds == st.lastHolds {
+ return
+ }
+ var routes []registry.Route
+ if herr == nil {
+ routes, herr = e.reg.List()
+ }
+ if herr != nil {
+ st.holdErrs++
+ if st.holdErrs == 1 || st.holdErrs%300 == 0 {
+ log.Printf("edge: registry-watch holds read failed (#%d): %v", st.holdErrs, herr)
}
- reloadErrs = 0
- last = fp
- log.Printf("edge: registry changed — reloaded (%d route(s))", len(e.current()))
+ return
}
+ st.holdErrs = 0
+ e.refreshHoldRows(routes, aliases)
+ st.lastHolds = holds
}
// versionHeader wraps next so every response also carries
diff --git a/internal/edge/holding.go b/internal/edge/holding.go
index 977392a62..54c09ad0e 100644
--- a/internal/edge/holding.go
+++ b/internal/edge/holding.go
@@ -2,6 +2,8 @@ package edge
import (
"bytes"
+ "crypto/sha256"
+ "encoding/hex"
"fmt"
"html/template"
"net/http"
@@ -179,9 +181,16 @@ func (e *Edge) holdingAnswer(w http.ResponseWriter, r *http.Request, h *holdingV
// holdSnapshot is this box's own holding candidates, by name: one real
// candidate per held row the view knows the move of, and one alias
// candidate per explicit alias the edges-file writer would publish for it.
-// Immutable once installed; RefreshHolds is its only writer.
+//
+// table is the upstream table the holds were built against, published in
+// the same value (final review F2): the request path (resolve, Allowed)
+// reads the table and the holds from this one pointer, so a Reload that
+// turns a ready row held (or back) is seen whole or not at all, never as
+// the new table with the old holds. Immutable once installed; only
+// rebuildHoldsLocked writes it, under holdsMu.
type holdSnapshot struct {
cands map[string][]candidate
+ table map[string]upstream
}
// holdInputs is the registry as the latest Reload read it: every row, every
@@ -210,8 +219,9 @@ func (e *Edge) SetHoldView(s HoldSnapshot) {
e.RefreshHolds()
}
-// RefreshHolds rebuilds the local holds from the latest view and the latest
-// Reload's rows, and installs them. The inputs are read under holdsMu, the
+// RefreshHolds rebuilds the local holds from the latest view and the rows
+// the latest Reload (or the watch's holds refresh, refreshHoldRows) listed,
+// and installs them. The inputs are read under holdsMu, the
// lock that installs, so rebuilds install in the order they read and none
// installs older inputs than one before it; no revision is compared (a
// view's Rev is publication order, not freshness). When the hold names that
@@ -230,17 +240,94 @@ func (e *Edge) RefreshHolds() {
func (e *Edge) rebuildHolds() bool {
e.holdsMu.Lock()
defer e.holdsMu.Unlock()
+ return e.rebuildHoldsLocked()
+}
+
+// rebuildHoldsLocked is the one writer of e.holds; holdsMu is held. The
+// snapshot it installs carries the table its inputs name.
+func (e *Edge) rebuildHoldsLocked() bool {
view := e.holdView.Load()
in := e.holdInputs.Load()
if e.holdsRead != nil {
e.holdsRead(view)
}
next := localHolds(view, in)
+ if in != nil {
+ next.table = in.table
+ }
prev := e.holds.Load()
e.holds.Store(next)
return !slices.Equal(prev.platformNames(), next.platformNames())
}
+// installTable is Reload's publication: the new table and the holds built
+// from the rows that produced it, installed under holdsMu as one
+// holdSnapshot (the request path's pair), with the table also stored for
+// every other reader of current(). It reports whether the platform hold
+// names changed.
+func (e *Edge) installTable(routes []registry.Route, aliases []registry.RouteAlias, m map[string]upstream) bool {
+ e.holdsMu.Lock()
+ defer e.holdsMu.Unlock()
+ e.upstreams.Store(&m)
+ e.holdInputs.Store(&holdInputs{routes: routes, aliases: aliases, table: m})
+ return e.rebuildHoldsLocked()
+}
+
+// refreshHoldRows re-bases the holds on freshly listed rows and alias rows
+// against the table already published (final review F1): the watch calls it,
+// outside the view's lock, when the only registry change is one the
+// fingerprint leaves out (an alias's DNS stamp). The table is not rebuilt.
+// Before the first Reload there is nothing to re-base.
+func (e *Edge) refreshHoldRows(routes []registry.Route, aliases []registry.RouteAlias) {
+ e.holdsMu.Lock()
+ in := e.holdInputs.Load()
+ changed := false
+ if in != nil {
+ e.holdInputs.Store(&holdInputs{routes: routes, aliases: aliases, table: in.table})
+ changed = e.rebuildHoldsLocked()
+ }
+ e.holdsMu.Unlock()
+ if changed {
+ if fn := e.reloadHook.Load(); fn != nil {
+ go (*fn)()
+ }
+ }
+}
+
+// servingPair is the request path's table and local holds, from one read
+// (holdSnapshot). An edge that has never reloaded (a bare test edge) has no
+// snapshot and reads the table alone.
+func (e *Edge) servingPair() (map[string]upstream, *holdSnapshot) {
+ if h := e.holds.Load(); h != nil && h.table != nil {
+ return h.table, h
+ }
+ return e.current(), e.holds.Load()
+}
+
+// holdAliasDigest is the watch's second input (final review F1): a digest
+// of every explicit alias row's (Alias, Host, DNSStatus, Explicit), which a
+// held row's alias holds depend on and the registry fingerprint leaves out.
+// armed is false, and nothing is read, while the rows the holds were last
+// built from hold nothing: then no alias can be a local hold, and a row
+// turning held moves the fingerprint.
+func (e *Edge) holdAliasDigest() (digest string, aliases []registry.RouteAlias, armed bool, err error) {
+ in := e.holdInputs.Load()
+ if in == nil || !slices.ContainsFunc(in.routes, func(r registry.Route) bool { return registry.IsHold(r.Status) }) {
+ return "", nil, false, nil
+ }
+ aliases, err = e.reg.ListAliases()
+ if err != nil {
+ return "", nil, true, err
+ }
+ h := sha256.New()
+ for _, a := range aliases {
+ if a.Explicit {
+ fmt.Fprintf(h, "%s\x1f%s\x1f%s\x1f%t\x1e", a.Alias, a.Host, a.DNSStatus, a.Explicit)
+ }
+ }
+ return hex.EncodeToString(h.Sum(nil)), aliases, true, nil
+}
+
// localHolds builds the box's holding candidates: none without a view, or
// with a failed one. A held row (registry.IsHold, not a sidecar) whose hold
// op is the move the view knows for its host is one real candidate, owned
@@ -320,10 +407,15 @@ func (h *holdSnapshot) platformNames() []string {
// localHoldsFor is name's local holding candidates.
func (e *Edge) localHoldsFor(name string) []candidate {
- if h := e.holds.Load(); h != nil {
- return h.cands[name]
+ return e.holds.Load().candsFor(name)
+}
+
+// candsFor is name's candidates in h, nil-receiver-safe.
+func (h *holdSnapshot) candsFor(name string) []candidate {
+ if h == nil {
+ return nil
}
- return nil
+ return h.cands[name]
}
// certNames is every exact name this box must keep a certificate for
diff --git a/internal/edge/homemap.go b/internal/edge/homemap.go
index c2e80724c..9acdff4cc 100644
--- a/internal/edge/homemap.go
+++ b/internal/edge/homemap.go
@@ -1083,7 +1083,9 @@ func (m *homeMap) Home(host string) (eip, box string, ok bool) {
// exact name before the catch-all. Fail-closed without a self label,
// exactly as Home is.
func (m *homeMap) ExactHome(host string) (eip, box string, ok bool) {
- if m == nil || m.self == "" {
+ // A literal catch-all key is no exact name (Allowed's and KnownHost's
+ // rule).
+ if m == nil || m.self == "" || strings.HasPrefix(host, "*.") {
return "", "", false
}
if d := m.resolveExact(host, m.clock()); d.kind == decRemote {