amadan

carlos / platform Public

edge: purge-host list review fixes (#173)

From the Opus review of 6fca5900 (nothing blocking):

- Resolve each listed host with lookupIn, not an exact map read, so a
  host served through a wildcard alias is purged instead of skipped as
  unserved. The tenancy check runs on the route it resolves to.
- Refuse "*" in a Carlos-Purge-Host entry: a wildcard key is a routing
  pattern, and nothing is cached under it.
- Log skipped hosts as override-unserved, apart from the refusals.
- Pin the staleness re-read in list form, with the served sibling not
  first in the list.
- Rename the stale resolvePurgeHost references, and the test that
  purges one served host out of two.
- Say that the socket's "*" covers only the listener's own host.
Human

Prompt

Can we build a fix, review and deploy for carlos/platform#173 ?

From this commit's own message. The prompt ledger has no entries for it — which is what a squashed, rebased or amended commit looks like, and what every commit looks like in a repo without a ledger.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 985c05c770c629f59905f7b8d94d6bb50737f277 parent 6fca590
6 files changed, +177 −11
  • internal/edge/edge.go +3 −3
  • internal/edge/purge.go +5 −0
  • internal/edge/purge_header.go +21 −5
  • internal/edge/purge_header_test.go +2 −2
  • internal/edge/purge_hosts_test.go +145 −0
  • internal/edge/purge_path_test.go +1 −1

CI passed — run details

diff --git a/internal/edge/edge.go b/internal/edge/edge.go
index 2045b8eeb..063435f7a 100644
--- a/internal/edge/edge.go
+++ b/internal/edge/edge.go
@@ -1065,7 +1065,7 @@ func (e *Edge) lookupUpstream(host string) (upstream, bool) {
// 3. Purge tenancy is the TARGET's app and account (purgeModifier is built
// from up.route), while the response CACHE keys on the REQUESTED host.
// Those two disagree, and closing that is what the path-routing branch
-// in resolvePurgeHost exists for — see below.
+// in purgeHostInTenancy exists for — see below.
// 4. Activator keying and hibernation are the TARGET's (up.hibernating and
// up.route come from the same entry), so a request for a flipped prefix
// wakes the target instance, not the requested hostname's. Correct: the
@@ -1076,7 +1076,7 @@ func (e *Edge) lookupUpstream(host string) (upstream, bool) {
// the target's route, so a path-routed app emitting a bare Carlos-Purge still
// purges under go.tito.io while its entry is stored under ti.to, and drops
// nothing. The fix is the documented escape hatch: name the hostname the
-// request arrived on, Carlos-Purge-Host: ti.to. resolvePurgeHost used to
+// request arrived on, Carlos-Purge-Host: ti.to. The override check used to
// refuse that — it required the override to name a host in the SAME app, and
// path routing exists precisely to cross an app boundary — and now allows it
// when a prefix on the named host selects this route. Account is still
@@ -1085,7 +1085,7 @@ func (e *Edge) lookupUpstream(host string) (upstream, bool) {
//
// So an app can emit Carlos-Purge-Host set to the Host header it received,
// unconditionally, from shared code. The inbound Host is preserved end to end,
-// so on an ordinary request that equals rt.Host and takes resolvePurgeHost's
+// so on an ordinary request that equals rt.Host and takes resolvePurgeHosts'
// own-host branch; on a path-routed one it names the front hostname and takes
// the path-routing branch. Nothing in the app has to know which it is.
func (e *Edge) lookupUpstreamPath(host, urlPath string) (upstream, bool) {
diff --git a/internal/edge/purge.go b/internal/edge/purge.go
index 766a00044..4e2f61466 100644
--- a/internal/edge/purge.go
+++ b/internal/edge/purge.go
@@ -35,6 +35,11 @@ type PurgeListener = ControlListener
// instance had to enumerate its own paths to do what the header channel does
// in one token.
//
+// "Whole host" means the one host the listener was built for. Entries cached
+// under that route's aliases or custom domains are keyed by those names, and
+// this channel cannot name them; an app that needs them uses the header
+// channel's Carlos-Purge-Host list.
+//
// "*" is a whole-host purge only on its own. With a query beside it the body
// names nothing a request could be stored under, so it is refused rather than
// guessed at: widening a malformed exact purge to the whole host is not a
diff --git a/internal/edge/purge_header.go b/internal/edge/purge_header.go
index c2ffb20be..9b590e2a0 100644
--- a/internal/edge/purge_header.go
+++ b/internal/edge/purge_header.go
@@ -114,7 +114,8 @@ const PurgeHeader = "Carlos-Purge"
// There is deliberately no fallback to the serving host, and no "honour
// the good entries": a half-honoured purge is worse than a refused one,
// because it clears something and so looks like it worked.
-// - An entry this edge does not SERVE — no ready route under that key — is
+// - An entry this edge does not SERVE — no ready route, exact or wildcard,
+// that it would route the name to — is
// skipped (and logged), and the rest of the list is honoured. That is the
// one exception, and it is not a half-honoured purge: an unserved host has
// nothing this edge would serve from cache, and it cannot be checked
@@ -293,7 +294,7 @@ func scrubInboundPurgeHeader(pr *httputil.ProxyRequest) {
// time the response comes back.
//
// The OVERRIDE side is deliberately the other way round: the named host is
-// looked up in e.current() at response time (see resolvePurgeHost), because
+// looked up in e.current() at response time (see resolvePurgeHosts), because
// the route table is live and a sibling added since this proxy was built must
// still validate. e.current() is an atomic pointer load, so reading it from
// this hook needs no lock and cannot tear against a concurrent Reload.
@@ -413,7 +414,13 @@ func (e *Edge) resolvePurgeHosts(rt registry.Route, declared []string) ([]string
hosts = append(hosts, host)
continue
}
- up, ok := cur[host]
+ // lookupIn, not cur[host]: a host served through a wildcard key
+ // ("*.acme.example") has no exact entry, but the edge routes it and
+ // caches its pages under the name the browser asked for, which is
+ // the name listed here. An exact key, another tenant's included,
+ // still wins over the wildcard, and either way the route found is the
+ // one judged below.
+ up, ok := lookupIn(cur, host)
if !ok {
unserved = append(unserved, host)
continue
@@ -453,7 +460,10 @@ func (e *Edge) resolvePurgeHosts(rt registry.Route, declared []string) ([]string
if len(unserved) > 0 {
// One line per response, not one per entry, and only after the whole
// list has passed: a refused declaration logs its refusal instead.
- e.logPurgeIssue("override-unknown", rt.Host, fmt.Sprintf(
+ // Its own kind, apart from the refusals: a domain waiting on DNS makes
+ // this line on every write, and sharing a throttle with the refusals
+ // would bury the rare one an operator needs to see.
+ e.logPurgeIssue("override-unserved", rt.Host, fmt.Sprintf(
"%s declared %d Carlos-Purge-Host value(s) this edge does not serve, first %q — skipped, the rest of the declaration is honoured",
rt.Host, len(unserved), truncateForLog(unserved[0])))
}
@@ -557,6 +567,12 @@ func pathRoutesTo(up upstream, target string) bool {
// out, and "["/"]" are refused with it for consistency. Route keys are the
// hosts ServeHTTP matches r.Host against — DNS names, in every deployment this
// platform has — so the exclusion costs nothing real.
+//
+// "*" is refused too. A wildcard route key ("*.acme.example") is a routing
+// pattern, not a name anything is cached under: the cache keys on the host
+// the browser asked for. Naming the pattern would pass the tenancy check and
+// purge nothing, which looks like it worked. An app names the real hosts,
+// which resolvePurgeHosts routes through the wildcard.
func validPurgeOverrideHost(h string) bool {
if h == "" {
return false
@@ -570,7 +586,7 @@ func validPurgeOverrideHost(h string) bool {
return false
}
switch c {
- case ':', '/', '\\', '@', '?', '#', '%', '[', ']':
+ case ':', '/', '\\', '@', '?', '#', '%', '[', ']', '*':
return false
}
}
diff --git a/internal/edge/purge_header_test.go b/internal/edge/purge_header_test.go
index 1be682edb..ba9fbc4e5 100644
--- a/internal/edge/purge_header_test.go
+++ b/internal/edge/purge_header_test.go
@@ -1316,13 +1316,13 @@ func TestPurgeHostOverrideUnknownHostIsRefused(t *testing.T) {
}
}
-// TestPurgeHostOverrideTwoValuesPurgeBoth — this used to pin "at most ONE
+// TestPurgeHostOverrideTwoValuesPurgeTheServedOne — this used to pin "at most ONE
// host", with two names refused as malformed. #173 made the header a list, so
// two names now mean "purge both" — on separate lines or comma-joined on one
// (RFC 9110 §5.3). c.example.com has no route here, so it is skipped rather
// than purged (see TestPurgeHostListSkipsUnservedHosts), and the serving host
// a.example.com is untouched because neither spelling names it.
-func TestPurgeHostOverrideTwoValuesPurgeBoth(t *testing.T) {
+func TestPurgeHostOverrideTwoValuesPurgeTheServedOne(t *testing.T) {
for _, tc := range []struct {
name string
set func(http.Header)
diff --git a/internal/edge/purge_hosts_test.go b/internal/edge/purge_hosts_test.go
index ba58ab57d..405f7b603 100644
--- a/internal/edge/purge_hosts_test.go
+++ b/internal/edge/purge_hosts_test.go
@@ -337,3 +337,148 @@ func TestPurgeHostListEveryEntryUnservedPurgesNothing(t *testing.T) {
t.Fatal("a list of unserved hosts fell back to the serving host")
}
}
+
+// TestPurgeHostListReachesAHostServedThroughAWildcard — a catch-all alias
+// ("*.acme.example") routes every subdomain to one route, and the cache keys
+// each page on the name the browser asked for. Listing that name must purge
+// it: the edge serves it, so skipping it as "unserved" would leave a stale
+// page behind a purge that cleared the rest of the list.
+func TestPurgeHostListReachesAHostServedThroughAWildcard(t *testing.T) {
+ e := purgeHostsEdge(t, titoAccount(), func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Carlos-Purge-Host", "tickets.acme.example, foo.wild.example")
+ w.Header().Set("Carlos-Purge", "/x")
+ w.WriteHeader(http.StatusOK)
+ })
+ upsertAlias(t, e.reg, "*.wild.example", "acme.go.tito.example")
+ if err := e.Reload(); err != nil {
+ t.Fatalf("Reload: %v", err)
+ }
+ if _, ok := e.current()["*.wild.example"]; !ok {
+ t.Fatal("fixture: the wildcard alias is not in the served table, so this proves nothing")
+ }
+ seed(t, e.cache, "foo.wild.example", "/x", "", "wild")
+ seed(t, e.cache, "tickets.acme.example", "/x", "", "custom")
+
+ servePOST(e, "http://go.tito.example/write")
+
+ if cached(e.cache, "foo.wild.example", "/x", "") {
+ t.Fatal("a host served through a wildcard alias was skipped as unserved")
+ }
+ if cached(e.cache, "tickets.acme.example", "/x", "") {
+ t.Fatal("the rest of the list was not honoured")
+ }
+}
+
+// TestPurgeHostListWildcardOfAnotherTenantIsRefused — resolving through the
+// wildcard is still a tenancy check on the route it lands on: a name under
+// another account's catch-all refuses the whole list.
+func TestPurgeHostListWildcardOfAnotherTenantIsRefused(t *testing.T) {
+ routes := append(titoAccount(), route("other-account.example", "tito", "tenantb"))
+ e := purgeHostsEdge(t, routes, func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Carlos-Purge-Host", "tickets.acme.example, foo.theirs.example")
+ w.Header().Set("Carlos-Purge", "/x")
+ w.WriteHeader(http.StatusOK)
+ })
+ upsertAlias(t, e.reg, "*.theirs.example", "other-account.example")
+ if err := e.Reload(); err != nil {
+ t.Fatalf("Reload: %v", err)
+ }
+ if _, ok := e.current()["*.theirs.example"]; !ok {
+ t.Fatal("fixture: the wildcard alias is not in the served table, so this proves nothing")
+ }
+ seed(t, e.cache, "foo.theirs.example", "/x", "", "theirs")
+ seed(t, e.cache, "tickets.acme.example", "/x", "", "custom")
+
+ servePOST(e, "http://go.tito.example/write")
+
+ if !cached(e.cache, "foo.theirs.example", "/x", "") {
+ t.Fatal("a host under ANOTHER ACCOUNT's wildcard was purged")
+ }
+ if !cached(e.cache, "tickets.acme.example", "/x", "") {
+ t.Fatal("a list carrying a cross-tenant entry was partly honoured")
+ }
+}
+
+// TestPurgeHostListNamingAWildcardKeyIsRefused — "*.acme.example" is a
+// routing pattern, not a name anything is cached under, so naming it is a
+// malformed entry and refuses the list rather than purging nothing quietly.
+func TestPurgeHostListNamingAWildcardKeyIsRefused(t *testing.T) {
+ e := purgeHostsEdge(t, titoAccount(), func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Carlos-Purge-Host", "tickets.acme.example, *.wild.example")
+ w.Header().Set("Carlos-Purge", "/x")
+ w.WriteHeader(http.StatusOK)
+ })
+ upsertAlias(t, e.reg, "*.wild.example", "acme.go.tito.example")
+ if err := e.Reload(); err != nil {
+ t.Fatalf("Reload: %v", err)
+ }
+ seed(t, e.cache, "tickets.acme.example", "/x", "", "custom")
+
+ servePOST(e, "http://go.tito.example/write")
+
+ if !cached(e.cache, "tickets.acme.example", "/x", "") {
+ t.Fatal("a list naming a wildcard key was honoured")
+ }
+}
+
+// TestPurgeHostListRefusedWhenTheServingRouteChangedSinceCapture — the
+// staleness re-read runs for the first SERVED cross-host entry wherever it
+// sits, not only when it leads the list. Own host first and an unserved host
+// first are the two orders that reach a served sibling without it being
+// first.
+func TestPurgeHostListRefusedWhenTheServingRouteChangedSinceCapture(t *testing.T) {
+ for _, list := range []string{
+ "a.example.com, b.example.com",
+ "ghost.example.com, b.example.com",
+ } {
+ t.Run(list, func(t *testing.T) {
+ var (
+ e *Edge
+ reg *registry.Registry
+ addr string
+ )
+ backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ // The route table moves under the in-flight request.
+ if err := reg.Upsert(registry.Route{
+ Host: "a.example.com", App: "tito", Account: "tenantb",
+ Kind: "instance", Addr: addr, Status: "ready",
+ }); err != nil {
+ t.Errorf("upsert during flight: %v", err)
+ }
+ if err := e.Reload(); err != nil {
+ t.Errorf("reload during flight: %v", err)
+ }
+ w.Header().Set("Carlos-Purge-Host", list)
+ w.Header().Set("Carlos-Purge", "/x")
+ w.WriteHeader(http.StatusOK)
+ }))
+ t.Cleanup(backend.Close)
+ addr = backend.Listener.Addr().String()
+ reg = openReg(t)
+ for _, h := range []string{"a.example.com", "b.example.com"} {
+ if err := reg.Upsert(registry.Route{
+ Host: h, App: "tito", Account: "tenanta",
+ Kind: "instance", Addr: addr, Status: "ready",
+ }); err != nil {
+ t.Fatalf("upsert %s: %v", h, err)
+ }
+ }
+ var err error
+ if e, err = New(reg, nil); err != nil {
+ t.Fatalf("New: %v", err)
+ }
+ e.EnableCache(NewCache(DefaultCacheMaxEntries, DefaultCacheMaxBodyBytes))
+ seed(t, e.cache, "a.example.com", "/x", "", "A")
+ seed(t, e.cache, "b.example.com", "/x", "", "B")
+
+ servePOST(e, "http://a.example.com/write")
+
+ if !cached(e.cache, "b.example.com", "/x", "") {
+ t.Fatal("the sibling was purged on the strength of a STALE captured identity")
+ }
+ if !cached(e.cache, "a.example.com", "/x", "") {
+ t.Fatal("a refused list was partly honoured on the serving host")
+ }
+ })
+ }
+}
diff --git a/internal/edge/purge_path_test.go b/internal/edge/purge_path_test.go
index 29d4d6b4d..698bafdfd 100644
--- a/internal/edge/purge_path_test.go
+++ b/internal/edge/purge_path_test.go
@@ -48,7 +48,7 @@ func purgePathEdge(t *testing.T, front, target registry.Route, prefix string, h
//
// The request really is path-routed here rather than simulated: it arrives on
// ti.to/paulca and reaches the target through the prefix map, so this pins the
-// composition and not just resolvePurgeHost in isolation.
+// composition and not just resolvePurgeHosts in isolation.
func TestPurgeHostOverrideAllowedFromAPathRoutedHost(t *testing.T) {
e := purgePathEdge(t,
route("ti.to", "classic", "tenanta"),