amadan

carlos / platform Public

appconfig: the notice says when a write reached nothing or a stamp is blocked

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev efbfb1d786ae68d19a74805d448024c81faade72 parent 9748035
2 files changed, +167 −24
  • internal/appconfig/awaitingrestart_test.go +90 −5
  • internal/appconfig/syncexec.go +77 −19
diff --git a/internal/appconfig/awaitingrestart_test.go b/internal/appconfig/awaitingrestart_test.go
index 53e4aaf0f..91e782d6b 100644
--- a/internal/appconfig/awaitingrestart_test.go
+++ b/internal/appconfig/awaitingrestart_test.go
@@ -3,6 +3,7 @@ package appconfig
import (
"encoding/json"
"errors"
+ "strings"
"testing"
"github.com/carlosframework/platform/internal/registry"
@@ -62,12 +63,13 @@ func TestAwaitingRestartSkipsUnoptedAndSocketRoutes(t *testing.T) {
}
// The same addr route, with the app never opted in: no file was ever
- // written for it, so there is nothing to restart over.
+ // written for it, so there is nothing to restart over — but the write
+ // still reached nothing here, which Undelivered now says.
ctx2, s2, _, _, _, _ := execKit(t)
got = AwaitingRestart(ctx2, s2, []registry.Route{addrRoute("connect.example.com", "relay", "bab", "")},
"bab", "relay", "", []string{"K"})
- if !got.Empty() {
- t.Fatalf("got %+v, want nothing for an app that never opted in", got)
+ if len(got.Hosts) != 0 || len(got.Undelivered) != 1 || got.Undelivered[0] != "connect.example.com" {
+ t.Fatalf("got %+v, want no Hosts and connect.example.com in Undelivered for an app that never opted in", got)
}
}
@@ -232,8 +234,9 @@ func TestAwaitingRestartUnknownExecDeliveryIsOff(t *testing.T) {
t.Fatalf("Put: %v", err)
}
routes := []registry.Route{addrRoute("connect.example.com", "relay", "bab", "")}
- if got := AwaitingRestart(ctx, s, routes, "bab", "relay", "", nil); !got.Empty() {
- t.Fatalf("got %+v, want nothing for an exec_delivery value this binary does not understand", got)
+ got := AwaitingRestart(ctx, s, routes, "bab", "relay", "", nil)
+ if len(got.Hosts) != 0 || len(got.Undelivered) != 1 || got.Undelivered[0] != "connect.example.com" {
+ t.Fatalf("got %+v, want no Hosts and connect.example.com in Undelivered for an exec_delivery value this binary does not understand", got)
}
}
@@ -261,3 +264,85 @@ func TestAwaitingRestartSorted(t *testing.T) {
}
}
}
+
+func TestAwaitingRestartListsUnoptedAddrRouteAsUndelivered(t *testing.T) {
+ ctx, s, _, _, _, _ := execKit(t)
+ if err := SetEnv(ctx, s, "bab", "relay", map[string]string{"K": "v"}); err != nil {
+ t.Fatal(err)
+ }
+ routes := []registry.Route{addrRoute("plain.example.com", "relay", "bab", "")}
+ got := AwaitingRestart(ctx, s, routes, "bab", "relay", "", []string{"K"})
+ if len(got.Undelivered) != 1 || got.Undelivered[0] != "plain.example.com" {
+ t.Fatalf("Undelivered = %v, want [plain.example.com]", got.Undelivered)
+ }
+ if len(got.Hosts) != 0 || got.Empty() {
+ t.Fatalf("Hosts = %v, Empty = %v", got.Hosts, got.Empty())
+ }
+}
+
+func TestAwaitingRestartStampedRouteWithMissingBundleIsUnresolved(t *testing.T) {
+ ctx, s, _, _, _, _ := execKit(t)
+ if err := SetEnv(ctx, s, "bab", "relay", map[string]string{"K": "v"}); err != nil {
+ t.Fatal(err)
+ }
+ rt := addrRoute("relay.example.com", "relay", "bab", "missing")
+ rt.Unit = "relay.service"
+ got := AwaitingRestart(ctx, s, []registry.Route{rt}, "bab", "relay", "", []string{"K"})
+ if len(got.Unresolved) != 1 || got.Unresolved[0] != "relay.example.com" {
+ t.Fatalf("Unresolved = %v, want the stamped route with a missing bound bundle", got.Unresolved)
+ }
+ if len(got.Hosts) != 0 || len(got.Undelivered) != 0 || len(got.Blocked) != 0 {
+ t.Fatalf("stamped route leaked into another set: %+v", got)
+ }
+}
+
+func TestAwaitingRestartHealthyStampedRouteIsListedNowhere(t *testing.T) {
+ ctx, s, _, _, _, _ := execKit(t)
+ if err := SetEnv(ctx, s, "bab", "relay", map[string]string{"K": "v"}); err != nil {
+ t.Fatal(err)
+ }
+ rt := addrRoute("relay.example.com", "relay", "bab", "")
+ rt.Unit = "relay.service"
+ got := AwaitingRestart(ctx, s, []registry.Route{rt}, "bab", "relay", "", []string{"K"})
+ if !got.Empty() {
+ t.Fatalf("healthy stamped route must be silent, got %+v", got)
+ }
+}
+
+func TestAwaitingRestartBlockedStamps(t *testing.T) {
+ ctx, s, _, _, _, _ := execKit(t)
+ if err := SetEnv(ctx, s, "bab", "relay", map[string]string{"K": "v"}); err != nil {
+ t.Fatal(err)
+ }
+ sock := addrRoute("sock.example.com", "relay", "bab", "")
+ sock.Unit, sock.Addr, sock.Socket = "sock.service", "", "/run/x.sock"
+ tmpl := addrRoute("tmpl.example.com", "relay", "bab", "")
+ tmpl.Unit = "carlos-app@.service"
+ a := addrRoute("a.example.com", "relay", "bab", "")
+ a.Unit = "shared.service"
+ // The duplicate is another ACCOUNT's route: it blocks a but is not disclosed.
+ other := addrRoute("other.example.com", "relay", "bae", "")
+ other.Unit = "shared.service"
+ got := AwaitingRestart(ctx, s, []registry.Route{sock, tmpl, a, other}, "bab", "relay", "", []string{"K"})
+ byHost := map[string]string{}
+ for _, b := range got.Blocked {
+ byHost[b.Host] = b.Reason
+ }
+ if len(byHost) != 3 {
+ t.Fatalf("Blocked = %+v, want sock, tmpl and a", got.Blocked)
+ }
+ if !strings.Contains(byHost["sock.example.com"], "socket") ||
+ !strings.Contains(byHost["tmpl.example.com"], "template") ||
+ !strings.Contains(byHost["a.example.com"], "shared.service") {
+ t.Fatalf("reasons = %v", byHost)
+ }
+ if _, leaked := byHost["other.example.com"]; leaked {
+ t.Fatal("another account's route must not be disclosed")
+ }
+ if strings.Contains(byHost["a.example.com"], "other.example.com") {
+ t.Fatal("the reason must not name another account's host")
+ }
+ if got.Empty() {
+ t.Fatal("Blocked must count as something to say")
+ }
+}
diff --git a/internal/appconfig/syncexec.go b/internal/appconfig/syncexec.go
index 4591b89c4..fe5b56ec5 100644
--- a/internal/appconfig/syncexec.go
+++ b/internal/appconfig/syncexec.go
@@ -639,25 +639,67 @@ type RestartHost struct {
Unit string
}
-// RestartPending is what a config write left behind on ONE box: files that
-// landed for processes carlos does not own, and therefore does not restart.
+// BlockedHost is a stamped route the root pass will refuse for a reason
+// knowable from the registry alone (spec §3.4, codex R2-6): the shape
+// rail, the name rail, the hostname rail or the one-route-per-unit rule.
+// The systemd-side refusals (alias, load state, the EnvironmentFiles
+// verification) are not knowable at the door and are not claimed here.
+type BlockedHost struct {
+ Host string
+ Reason string
+}
+
+// RestartPending is what a config write left behind on ONE box.
//
-// Unresolved is the honest half and the reason this is not a plain
-// []string. A route whose exec_delivery could not be resolved this call —
-// its bundle unreadable, or the bundle it is bound to missing outright —
-// is a route nothing can say "delivered" or "not delivered" about. Dropping
-// it silently would commit the very conflation this notice exists to end:
-// "nothing needs a restart" and "I could not tell" rendered identically is
-// how discussion #16 got filed in the first place.
+// Hosts: files that landed for processes carlos does not own, and
+// therefore does not restart. Unresolved: routes nothing can say
+// "delivered" or "not delivered" about. Undelivered: routes this write
+// reached that no pass delivers a file to (an un-opted addr route with no
+// stamp). Blocked: stamped routes the root pass will refuse. Each is its
+// own list because the honest sentence differs, and "could not tell" is
+// never rendered as any of the others.
type RestartPending struct {
- Hosts []RestartHost
- Unresolved []string
+ Hosts []RestartHost
+ Unresolved []string
+ Undelivered []string
+ Blocked []BlockedHost
}
-// Empty reports whether there is nothing to say — no host delivered to and
-// none unresolved. The renderers' one test, so "say nothing" is one
-// judgement rather than four.
-func (r RestartPending) Empty() bool { return len(r.Hosts) == 0 && len(r.Unresolved) == 0 }
+// Empty reports whether there is nothing to say. The renderers' one test.
+func (r RestartPending) Empty() bool {
+ return len(r.Hosts) == 0 && len(r.Unresolved) == 0 && len(r.Undelivered) == 0 && len(r.Blocked) == 0
+}
+
+// BlockedStamps applies the door-side rails to the stamped routes of
+// (acct, app) in routes. Uniqueness is checked against the FULL list, so
+// a route of another account blocks this one, but only this account's
+// hosts are returned and no other account's host is named in a reason.
+func BlockedStamps(routes []registry.Route, acct, app string) []BlockedHost {
+ byUnit := map[string]int{}
+ for _, rt := range routes {
+ if Stamped(rt) && StampShape(rt) == nil && DeliverableUnitName(rt.Unit) == nil && BareHostname(rt.Host) {
+ byUnit[rt.Unit]++
+ }
+ }
+ var out []BlockedHost
+ for _, rt := range routes {
+ if !Stamped(rt) || rt.App != app || release.RouteAccount(rt) != acct {
+ continue
+ }
+ switch {
+ case StampShape(rt) != nil:
+ out = append(out, BlockedHost{rt.Host, StampShape(rt).Error()})
+ case DeliverableUnitName(rt.Unit) != nil:
+ out = append(out, BlockedHost{rt.Host, DeliverableUnitName(rt.Unit).Error()})
+ case !BareHostname(rt.Host):
+ out = append(out, BlockedHost{rt.Host, fmt.Sprintf("%q is not a bare hostname", rt.Host)})
+ case byUnit[rt.Unit] > 1:
+ out = append(out, BlockedHost{rt.Host, fmt.Sprintf("two or more routes on this box name %s; one unit takes one route's config", rt.Unit)})
+ }
+ }
+ sort.Slice(out, func(i, j int) bool { return out[i].Host < out[j].Host })
+ return out
+}
// AwaitingRestart answers, for a config write to app's envName bundle
// ("" = the default), which of this box's routes were handed a file that
@@ -686,6 +728,11 @@ func (r RestartPending) Empty() bool { return len(r.Hosts) == 0 && len(r.Unresol
// short list read as a complete one.
func AwaitingRestart(ctx context.Context, s release.Store, routes []registry.Route, acct, app, envName string, keys []string) RestartPending {
var out RestartPending
+ out.Blocked = BlockedStamps(routes, acct, app)
+ blocked := map[string]bool{}
+ for _, b := range out.Blocked {
+ blocked[b.Host] = true
+ }
// Both caches exist for the same reason SyncExec's do: a box with many
// TCP-addr routes on one bundle must not cost more than one read for
// it. Unlike SyncExec's, they live for one call.
@@ -718,7 +765,8 @@ func AwaitingRestart(ctx context.Context, s release.Store, routes []registry.Rou
return e
}
for _, rt := range routes {
- if !execAddrCandidate(rt) || rt.App != app || release.RouteAccount(rt) != acct {
+ stamped := Stamped(rt) && !blocked[rt.Host]
+ if (!execAddrCandidate(rt) && !stamped) || rt.App != app || release.RouteAccount(rt) != acct {
continue
}
// Which routes a write REACHES. A named bundle is read by the
@@ -751,10 +799,19 @@ func AwaitingRestart(ctx context.Context, s release.Store, routes []registry.Rou
out.Unresolved = append(out.Unresolved, rt.Host)
continue
}
- // Not opted in — including the Unknown case, which decides "off"
- // — means no file was ever written for this route, by this write
- // or any other. Nothing to say.
+ // A stamped route is delivered and restarted by the root pass within
+ // seconds of this write; it has nothing to be told about unless its
+ // bundle would not resolve, which the checks above have already
+ // reported (spec §3.4, codex R1-8).
+ if stamped {
+ continue
+ }
+ // Not opted in — including the Unknown case, which decides "off" —
+ // means no pass delivers a file to this route: this write reached
+ // nothing here. Say so (spec §3.4, codex R1-9: "this write", not
+ // "no file" — an opted-out route may hold a file from before).
if !v.On {
+ out.Undelivered = append(out.Undelivered, rt.Host)
continue
}
// The shadowing test, and the reason keys is a parameter: a write
@@ -778,6 +835,7 @@ func AwaitingRestart(ctx context.Context, s release.Store, routes []registry.Rou
// ExecConfigApps'.
sort.Slice(out.Hosts, func(i, j int) bool { return out.Hosts[i].Host < out.Hosts[j].Host })
sort.Strings(out.Unresolved)
+ sort.Strings(out.Undelivered)
return out
}