amadan

carlos / platform Public

appconfig: SyncExec resolves exec_delivery from the route's bound bundle (#195)

checkOptedIn is keyed on (account, app, environment). A named bundle that
STATES a value decides for the routes bound to it; one that is silent, or
absent, inherits the app default — so every deployment on an app-level
"file" today delivers to exactly the same routes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gyhxeoXGBW4WCFRnMqBue
Paul Campbell 8a33585259daafa8fdde5ec4d060881674fca026 parent c6ab994
2 files changed, +291 −24
  • internal/appconfig/syncexec.go +90 −24
  • internal/appconfig/syncexec_test.go +201 −0
diff --git a/internal/appconfig/syncexec.go b/internal/appconfig/syncexec.go
index 306f6d2f9..2c7f9a381 100644
--- a/internal/appconfig/syncexec.go
+++ b/internal/appconfig/syncexec.go
@@ -9,16 +9,21 @@
// 2026-08-02-exec-config-delivery-design.md.
//
// A third eligibility case (issue #18): a TCP-addr exec route (Backing ==
-// "", Socket == "", Addr != "") whose app has opted in via its default
-// bundle's additive "exec_delivery": "file" manifest field
-// (Manifest.ExecDelivery) gets the identical file, materialized with the
-// same layering guarantees, environments-awareness, reap discipline, and
-// mass-removal/path-traversal guards as a socket-exec route — but is
-// NEVER bounced (see syncExecHost and execAddrCandidate): carlos routes
-// to these processes without spawning or owning them, so the restart half
-// belongs to the app's own supervisor. An un-opted TCP-addr route is
-// skipped exactly as before this case existed — see execAddrCandidate and
-// SyncExec's checkOptedIn closure.
+// "", Socket == "", Addr != "") whose BOUND BUNDLE has opted in via the
+// additive "exec_delivery": "file" manifest field (Manifest.ExecDelivery)
+// gets the identical file, materialized with the same layering guarantees,
+// environments-awareness, reap discipline, and mass-removal/path-traversal
+// guards as a socket-exec route — but is NEVER bounced (see syncExecHost
+// and execAddrCandidate): carlos routes to these processes without
+// spawning or owning them, so the restart half belongs to the app's own
+// supervisor. An un-opted TCP-addr route is skipped exactly as before this
+// case existed — see execAddrCandidate and SyncExec's checkOptedIn closure.
+//
+// "Bound bundle" is issue #195: the flag is read from the route's named
+// environment when that bundle STATES a value, and from the app's default
+// bundle otherwise. Before that it was app-level only, so opting one
+// relay route in wrote a file for every other TCP-addr route of the same
+// app — a fleet-wide switch thrown to deliver one service's credential.
//
// Resolution mirrors sync.go's exactly — the group is (Account, App,
// Environment) and a route's bound environment is an overlay on the app's
@@ -119,20 +124,81 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
v bool
known bool
}
- // optedIn caches the app-level exec_delivery decision so a box with
- // many TCP-addr routes on one app doesn't cost more than one manifest
- // read for it (loadDefault's own cache already dedupes the fetch; this
- // dedupes the value-interpretation, including the two logs below,
- // which must fire at most once per (acct, app) per pass, not once per
- // route).
- optedIn := map[appKey]optedInRes{}
- checkOptedIn := func(acct, app string) optedInRes {
- ak := appKey{acct, app}
- if c, done := optedIn[ak]; done {
+ // optKey keys the opt-in decision on the BUNDLE a route's config
+ // actually resolves through, not on the app alone (issue #195). Two
+ // routes of one app bound to different environments can now differ, so
+ // the cache — and the once-per-pass logs below — must too.
+ type optKey struct{ acct, app, env string }
+ // optedIn caches the exec_delivery decision so a box with many TCP-addr
+ // routes on one bundle doesn't cost more than one manifest read for it
+ // (loadDefault/loadEnvBundle already dedupe the fetches; this dedupes
+ // the value-interpretation, including the logs below, which must fire
+ // at most once per (acct, app, env) per pass, not once per route).
+ optedIn := map[optKey]optedInRes{}
+ // envBundles caches the named-bundle read the same way loadDefault
+ // caches the default's, and for the same reason.
+ type envRes struct {
+ m *Manifest
+ found bool
+ err error
+ }
+ envBundles := map[optKey]envRes{}
+ loadEnvBundle := func(acct, app, env string) envRes {
+ k := optKey{acct, app, env}
+ if e, done := envBundles[k]; done {
+ return e
+ }
+ m, found, err := getIfExistsAt(ctx, s, EnvironmentKey(acct, app, env))
+ if err != nil {
+ err = fmt.Errorf("%s/%s (environment %q): config fetch failed: %w", acct, app, env, err)
+ }
+ e := envRes{m: m, found: found, err: err}
+ envBundles[k] = e
+ return e
+ }
+ checkOptedIn := func(acct, app, env string) optedInRes {
+ k := optKey{acct, app, env}
+ if c, done := optedIn[k]; done {
return c
}
- def := loadDefault(acct, app)
var c optedInRes
+ // The route's OWN bundle first, and only when it STATES something:
+ // a named bundle silent on the field inherits the app default,
+ // exactly as a key it does not set inherits the default bundle's
+ // value. Absence is inheritance, never "off" — an operator who has
+ // never mentioned exec_delivery in a bundle has not asked for
+ // delivery to stop.
+ decided := false
+ if env != "" {
+ eb := loadEnvBundle(acct, app, env)
+ switch {
+ case eb.err != nil:
+ // NOT a determination — the same contract the default
+ // bundle's fetch-failure branch below gives: say so loudly,
+ // leave every file alone, retry next pass.
+ fmt.Printf(" %s/%s: exec_delivery unresolved for environment %q (config fetch failed), leaving this bundle's TCP-addr route file(s) as-is: %v\n", acct, app, env, eb.err)
+ c, decided = optedInRes{v: false, known: false}, true
+ case eb.found && eb.m.ExecDelivery == ExecDeliveryFile:
+ c, decided = optedInRes{v: true, known: true}, true
+ case eb.found && eb.m.ExecDelivery != "":
+ // A value THIS binary doesn't understand, in a bundle that
+ // does state one: a statement, so it decides — treated as
+ // not opted in, loudly, once per bundle per pass, never a
+ // hard failure (additive discipline, CLAUDE.md).
+ fmt.Printf(" %s/%s: config: exec_delivery %q in environment %q is not a value this binary understands — ignoring it (not opted in); upgrade carlos if this is expected\n", acct, app, eb.m.ExecDelivery, env)
+ c, decided = optedInRes{v: false, known: true}, true
+ }
+ // found-but-silent, and absent, both fall through to the app
+ // default. A bundle bound but MISSING is a defect the route's
+ // own resolution reports (LoadMerged refuses it) and its file is
+ // left as-is there — the verdict reached here never gets to a
+ // write in that case.
+ }
+ if decided {
+ optedIn[k] = c
+ return c
+ }
+ def := loadDefault(acct, app)
switch {
case def.err != nil:
// NOT a determination: this app may well be opted in and this
@@ -172,7 +238,7 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
fmt.Printf(" %s/%s: config: exec_delivery %q in the default bundle is not a value this binary understands — ignoring it (not opted in); upgrade carlos if this is expected\n", acct, app, def.m.ExecDelivery)
c = optedInRes{v: false, known: true}
}
- optedIn[ak] = c
+ optedIn[k] = c
return c
}
// inScope is the single door both loops below use to decide whether a
@@ -185,7 +251,7 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
if execSyncable(rt) {
return true, false
}
- if execAddrCandidate(rt) && checkOptedIn(release.RouteAccount(rt), rt.App).v {
+ if execAddrCandidate(rt) && checkOptedIn(release.RouteAccount(rt), rt.App, rt.Environment).v {
return true, true
}
return false, false
@@ -375,7 +441,7 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
if !execAddrCandidate(rt) {
continue
}
- c := checkOptedIn(release.RouteAccount(rt), rt.App)
+ c := checkOptedIn(release.RouteAccount(rt), rt.App, rt.Environment)
if c.v || !c.known {
continue
}
diff --git a/internal/appconfig/syncexec_test.go b/internal/appconfig/syncexec_test.go
index 53d5519d9..9daa680cf 100644
--- a/internal/appconfig/syncexec_test.go
+++ b/internal/appconfig/syncexec_test.go
@@ -1628,3 +1628,204 @@ func TestUnitConfigAppsNoRoutes(t *testing.T) {
t.Fatalf("UnitConfigApps(nil) = %v, want empty", got)
}
}
+
+// --- issue #195: the opt-in resolves from the route's bound bundle ---
+
+// TestSyncExecOptInResolvesPerEnvironment is issue #195's matrix: which
+// hosts get a <host>.config.env, for each combination of the app-level
+// flag and the bound bundle's own. Two TCP-addr exec routes of ONE app,
+// one bound to "relay" and one unbound — titogo's exact shape, where
+// turning the app-level flag on to deliver the relay's sealed Apple Wallet
+// material wrote a file for every router route too.
+func TestSyncExecOptInResolvesPerEnvironment(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ appLevel string // "" (never set) | ExecDeliveryFile
+ envLevel string // absent | silent | file | bogus
+ wantRelayFile bool
+ wantUnboundFile bool
+ }{
+ {"both off", "", "silent", false, false},
+ {"app on, env silent — inherit on", ExecDeliveryFile, "silent", true, true},
+ {"app off, env on — the narrow case #195 exists for", "", "file", true, false},
+ {"app on, env on", ExecDeliveryFile, "file", true, true},
+ {"app off, env bundle absent", "", "absent", false, false},
+ // Bound to a bundle that does not exist: the opt-in inherits "on"
+ // from the app default, but the route never reaches a write anyway
+ // — LoadMerged refuses a bound-but-absent bundle, so the group
+ // resolves to an ERROR and the file is left exactly as it is. The
+ // unbound router is untouched by that and still gets its file.
+ {"app on, env bundle absent — resolution errors before delivery", ExecDeliveryFile, "absent", false, true},
+ {"app on, env states a value this binary doesn't know — that statement decides", ExecDeliveryFile, "bogus", false, true},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ ctx, s, reg, dd, priv, _ := execKit(t)
+ pk, err := ParsePrivateKey(priv, "test")
+ if err != nil {
+ t.Fatalf("ParsePrivateKey: %v", err)
+ }
+ // The default bundle always exists and always has a var, so the
+ // only thing under test is WHETHER a file is written.
+ if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"SHARED": "1"}); err != nil {
+ t.Fatalf("SetEnv: %v", err)
+ }
+ if tc.appLevel != "" {
+ if err := SetExecDelivery(ctx, s, "bab", "titogo", tc.appLevel); err != nil {
+ t.Fatalf("SetExecDelivery: %v", err)
+ }
+ }
+ switch tc.envLevel {
+ case "absent":
+ // Nothing written: the route is bound to a bundle that does
+ // not exist.
+ case "bogus":
+ // Written past the setter's validation on purpose, and
+ // without SaveEnvironment first (a create, not an update):
+ // this is what an OLDER binary sees when a NEWER one
+ // invents a value, and it must not brick the pass.
+ raw := []byte(`{"schema":1,"app":"titogo","vars":{"SEALED":{"kind":"env","value":"2"}},"exec_delivery":"grpc"}`)
+ if _, err := s.Put(ctx, EnvironmentKey("bab", "titogo", "relay"), raw, ""); err != nil {
+ t.Fatalf("Put: %v", err)
+ }
+ default:
+ if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay",
+ map[string]Var{"SEALED": {Kind: "env", Value: "2"}}, nil); err != nil {
+ t.Fatalf("SaveEnvironment: %v", err)
+ }
+ if tc.envLevel == "file" {
+ if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil {
+ t.Fatalf("SetExecDeliveryAt: %v", err)
+ }
+ }
+ }
+ for _, rt := range []registry.Route{
+ {Host: "connect.go.tito.io", Kind: "instance", App: "titogo", Account: "bab",
+ Status: "ready", Addr: "127.0.0.1:9001", Environment: "relay"},
+ {Host: "router-a.go.tito.io", Kind: "instance", App: "titogo", Account: "bab",
+ Status: "ready", Addr: "127.0.0.1:9002"},
+ } {
+ if err := reg.Upsert(rt); err != nil {
+ t.Fatalf("Upsert %s: %v", rt.Host, err)
+ }
+ }
+ if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil {
+ t.Fatalf("SyncExec: %v", err)
+ }
+ assertConfigFile(t, dd, "connect.go.tito.io", tc.wantRelayFile)
+ assertConfigFile(t, dd, "router-a.go.tito.io", tc.wantUnboundFile)
+ })
+ }
+}
+
+// assertConfigFile asserts a route's <host>.config.env exists (or not).
+func assertConfigFile(t *testing.T, dir, host string, want bool) {
+ t.Helper()
+ _, err := os.Stat(filepath.Join(dir, host+configEnvSuffix))
+ switch {
+ case want && err != nil:
+ t.Fatalf("%s: want a config file, got %v", host, err)
+ case !want && err == nil:
+ t.Fatalf("%s: want NO config file, one was written", host)
+ }
+}
+
+// TestSyncExecEnvScopedOptInDeliversTheMergedBundle: the flag decides
+// WHETHER a file is written, never what is in it. A route opted in through
+// its own bundle still gets the default bundle overlaid by that bundle —
+// the same merge LoadMerged has always produced.
+func TestSyncExecEnvScopedOptInDeliversTheMergedBundle(t *testing.T) {
+ ctx, s, reg, dd, priv, _ := execKit(t)
+ pk, err := ParsePrivateKey(priv, "test")
+ if err != nil {
+ t.Fatalf("ParsePrivateKey: %v", err)
+ }
+ if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"SHARED": "1", "OVERRIDDEN": "default"}); err != nil {
+ t.Fatalf("SetEnv: %v", err)
+ }
+ if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", map[string]Var{
+ "OVERRIDDEN": {Kind: "env", Value: "relay"},
+ "WALLET": {Kind: "env", Value: "sealed"},
+ }, nil); err != nil {
+ t.Fatalf("SaveEnvironment: %v", err)
+ }
+ if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil {
+ t.Fatalf("SetExecDeliveryAt: %v", err)
+ }
+ if err := reg.Upsert(registry.Route{
+ Host: "connect.go.tito.io", Kind: "instance", App: "titogo", Account: "bab",
+ Status: "ready", Addr: "127.0.0.1:9001", Environment: "relay",
+ }); err != nil {
+ t.Fatalf("Upsert: %v", err)
+ }
+ if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil {
+ t.Fatalf("SyncExec: %v", err)
+ }
+ got, err := os.ReadFile(filepath.Join(dd, "connect.go.tito.io"+configEnvSuffix))
+ if err != nil {
+ t.Fatalf("read config file: %v", err)
+ }
+ want := execAddrConfigHeader + "OVERRIDDEN=relay\nSHARED=1\nWALLET=sealed\n"
+ if string(got) != want {
+ t.Fatalf("config env = %q, want %q (the merged bundle, unchanged by where the flag lives)", got, want)
+ }
+ // exec_delivery itself must never be rendered into the file: it is a
+ // delivery flag, not a config var.
+ if strings.Contains(string(got), "exec_delivery") || strings.Contains(string(got), "EXEC_DELIVERY") {
+ t.Fatalf("the delivery flag leaked into the rendered file: %q", got)
+ }
+}
+
+// TestSyncExecEnvBundleFetchErrorLeavesFileAlone: an unreadable
+// environment bundle is NOT a determination that the route is un-opted.
+// The route's existing file must survive it byte-identical — the same
+// contract a default-bundle fetch failure already has, extended to the
+// second bundle the opt-in now consults.
+func TestSyncExecEnvBundleFetchErrorLeavesFileAlone(t *testing.T) {
+ ctx, s, reg, dd, priv, _ := execKit(t)
+ pk, err := ParsePrivateKey(priv, "test")
+ if err != nil {
+ t.Fatalf("ParsePrivateKey: %v", err)
+ }
+ if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"SHARED": "1"}); err != nil {
+ t.Fatalf("SetEnv: %v", err)
+ }
+ if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay",
+ map[string]Var{"WALLET": {Kind: "env", Value: "sealed"}}, nil); err != nil {
+ t.Fatalf("SaveEnvironment: %v", err)
+ }
+ if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil {
+ t.Fatalf("SetExecDeliveryAt: %v", err)
+ }
+ if err := reg.Upsert(registry.Route{
+ Host: "connect.go.tito.io", Kind: "instance", App: "titogo", Account: "bab",
+ Status: "ready", Addr: "127.0.0.1:9001", Environment: "relay",
+ }); err != nil {
+ t.Fatalf("Upsert: %v", err)
+ }
+ if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil {
+ t.Fatalf("SyncExec: %v", err)
+ }
+ path := filepath.Join(dd, "connect.go.tito.io"+configEnvSuffix)
+ before, err := os.ReadFile(path)
+ if err != nil {
+ t.Fatalf("first pass wrote no file: %v", err)
+ }
+
+ blip := &failingKeyStore{Store: s, failKey: EnvironmentKey("bab", "titogo", "relay"),
+ err: errors.New("503 SlowDown")}
+ out := captureStdout(t, func() {
+ if err := SyncExec(ctx, reg, blip, pk, dd, func(string) error { return nil }); err != nil {
+ t.Fatalf("SyncExec (blip): %v", err)
+ }
+ })
+ after, err := os.ReadFile(path)
+ if err != nil {
+ t.Fatalf("a bundle fetch blip removed a live route's config file: %v", err)
+ }
+ if !bytes.Equal(before, after) {
+ t.Fatalf("config file changed on a fetch blip: %q -> %q", before, after)
+ }
+ if !strings.Contains(out, "unresolved") || !strings.Contains(out, "relay") {
+ t.Fatalf("a fetch blip against the bound bundle must say so loudly and name the environment; got %q", out)
+ }
+}