amadan
appconfig: exec_delivery can be set on a named environment bundle (#195)
SetExecDeliveryAt generalizes the app-level setter to any one bundle. Clearing it in a named bundle means INHERIT the app default, never "off": a bundle that says nothing has not asked for delivery to stop. Narrows the 2026-08-03 issue #18 ruling, whose premise (one supervisor program per app) is false for titogo, which supervises its relay and its router fleet through different units. Route scope is still refused.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gyhxeoXGBW4WCFRnMqBue
3 files changed,
+188
−36
internal/appconfig/appconfig.go+33 −13internal/appconfig/environments_test.go+115 −0internal/appconfig/store.go+40 −23
diff --git a/internal/appconfig/appconfig.go b/internal/appconfig/appconfig.go| index 846396278..7d2521c2a 100644 |
| --- a/internal/appconfig/appconfig.go |
| +++ b/internal/appconfig/appconfig.go |
| @@ -33,20 +33,40 @@ type Manifest struct { |
| // does not own the process (see syncexec.go). |
| // |
| // Additive to schema 1 (schema is not bumped for this field), so any |
| - // other value is refused at SET time (SetExecDelivery, ValidExecDelivery) |
| - // and, since an older binary must still be able to parse a manifest a |
| - // newer one wrote, ignored — loudly, once per pass — at SYNC time |
| - // (see SyncExec's checkOptedIn) rather than causing a hard failure. |
| + // other value is refused at SET time (SetExecDeliveryAt, |
| + // ValidExecDelivery) and, since an older binary must still be able to |
| + // parse a manifest a newer one wrote, ignored — loudly, once per pass — |
| + // at SYNC time (see SyncExec's checkOptedIn) rather than causing a hard |
| + // failure. An older binary reading a NAMED bundle this one wrote the |
| + // field into ignores it entirely (it consults the default bundle |
| + // alone), which is the pre-#195 behaviour, not a fault. |
| // |
| - // APP-LEVEL ONLY: read from the default bundle (config.json) alone. |
| - // SetExecDelivery has no environment-scoped variant, unlike apply's |
| - // applyAt generalization — a named environment's manifest is never a |
| - // valid place for this field, and LoadMerged's merged result never |
| - // carries it forward. The reader is the app's own supervisor, one |
| - // program per app, not per-route or per-environment: per-route or |
| - // per-environment opt-in would let half a fleet read config the other |
| - // half silently ignores, the "config-that-lies" failure mode (issue |
| - // #18 design comment, 2026-08-03). |
| + // BUNDLE-LEVEL, resolved as an overlay (issue #195, ruled 2026-08-16). |
| + // SyncExec reads this field from the bundle a route's config actually |
| + // comes from: a route bound to environment e takes e's value when e |
| + // STATES one, and inherits the default bundle's when it does not. An |
| + // unbound route reads the default bundle alone, exactly as before. |
| + // LoadMerged still never carries this field into its merged result — |
| + // the merged manifest is what gets rendered into a file, and a delivery |
| + // flag has no business appearing there; checkOptedIn reads the two |
| + // bundles' fields directly instead. |
| + // |
| + // The 2026-08-03 ruling this narrows said a named environment's |
| + // manifest is never a valid place for the field, because "the reader is |
| + // the app's own supervisor, one program per app". That premise is false |
| + // in the deployment the feature was built for: the titogo app |
| + // supervises its relay and its router fleet through different units, so |
| + // the app-level flag thrown to deliver one sealed credential to the |
| + // relay wrote a file for every router too. |
| + // |
| + // STILL NOT PER-ROUTE. A per-route flag would need a registry column |
| + // and — since a Provisioned route's Channel and Environment belong to |
| + // its instance record — a record field and reconcile convergence too: |
| + // three doors onto one bit. The "config-that-lies" hazard the original |
| + // ruling named is real at that granularity, where two routes resolving |
| + // through the SAME bundle could disagree about whether it is delivered. |
| + // The environment binding is one door onto a scope that already exists, |
| + // and is the same axis the secrets themselves are scoped on. |
| ExecDelivery string `json:"exec_delivery,omitempty"` |
| } |
diff --git a/internal/appconfig/environments_test.go b/internal/appconfig/environments_test.go| index 24ef81079..d82b85e56 100644 |
| --- a/internal/appconfig/environments_test.go |
| +++ b/internal/appconfig/environments_test.go |
| @@ -331,3 +331,118 @@ func TestUnsetEnvironmentReportsActualRemovals(t *testing.T) { |
| t.Fatalf("B = %+v, want it untouched", m.Vars["B"]) |
| } |
| } |
| + |
| +// --- issue #195: exec_delivery at environment scope --- |
| + |
| +func TestSetExecDeliveryAtEnvironmentRoundTrips(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + // A bundle must exist before a flag can be written into it, exactly as |
| + // the default bundle must. |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"A": {Kind: "env", Value: "1"}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil { |
| + t.Fatalf("set: %v", err) |
| + } |
| + m, err := LoadEnvironment(ctx, s, "bab", "titogo", "relay") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if m.ExecDelivery != ExecDeliveryFile { |
| + t.Fatalf("environment bundle exec_delivery = %q, want %q", m.ExecDelivery, ExecDeliveryFile) |
| + } |
| + // The vars already in the bundle survive the flag write — read-modify- |
| + // write, not replace. |
| + if m.Vars["A"].Value != "1" { |
| + t.Fatalf("A = %q, want it preserved at \"1\"", m.Vars["A"].Value) |
| + } |
| + // And the DEFAULT bundle is untouched: an environment-scoped write that |
| + // leaked into config.json would opt the whole fleet in, which is the |
| + // bug this closes. |
| + def, found, err := getIfExists(ctx, s, "bab", "titogo") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if found && def.ExecDelivery != "" { |
| + t.Fatalf("default bundle exec_delivery = %q, want it untouched", def.ExecDelivery) |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtEnvironmentClearsToInherit(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"A": {Kind: "env", Value: "1"}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ""); err != nil { |
| + t.Fatalf("clear: %v", err) |
| + } |
| + m, err := LoadEnvironment(ctx, s, "bab", "titogo", "relay") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if m.ExecDelivery != "" { |
| + t.Fatalf("exec_delivery = %q, want it cleared to inherit", m.ExecDelivery) |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtRefusesAbsentBundle(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", "") |
| + if err == nil { |
| + t.Fatal("clearing the flag on a bundle that does not exist must be refused, not silently create one") |
| + } |
| + if !strings.Contains(err.Error(), "relay") { |
| + t.Fatalf("refusal %q must name the bundle it could not find", err) |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtRefusesInvalidValue(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"A": {Kind: "env", Value: "1"}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", "socket"); err == nil { |
| + t.Fatal("an unknown exec_delivery value must be refused before it reaches the bucket") |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtRefusesInvalidEnvironmentName(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "Relay", ExecDeliveryFile); err == nil { |
| + t.Fatal("an invalid environment name must be refused before any bucket write") |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtBumpsGeneration(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"A": {Kind: "env", Value: "1"}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, before, err := s.Get(ctx, GenKey("bab", "titogo")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, after, err := s.Get(ctx, GenKey("bab", "titogo")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if before == after { |
| + t.Fatal("an environment-scoped exec_delivery write must bump the generation object — the fast loop watches nothing else for it") |
| + } |
| +} |
diff --git a/internal/appconfig/store.go b/internal/appconfig/store.go| index 34be45bb4..de97f2217 100644 |
| --- a/internal/appconfig/store.go |
| +++ b/internal/appconfig/store.go |
| @@ -226,28 +226,36 @@ func Unset(ctx context.Context, s release.Store, acct, app string, keys []string |
| return apply(ctx, s, acct, app, DefaultBundle, nil, keys) |
| } |
| -// SetExecDelivery writes (`file`) or clears (`""`) the default bundle's |
| +// SetExecDelivery writes (`file`) or clears (`""`) the DEFAULT bundle's |
| // exec_delivery field — issue #18's TCP-addr exec route opt-in (see |
| -// Manifest.ExecDelivery) — via the same CAS read-modify-write path |
| -// SetEnv/SetSecret/Unset use. value must be ValidExecDelivery; anything |
| -// else is refused here, before it ever reaches the bucket, mirroring |
| -// applyAt's var-name/var-value validation. |
| +// Manifest.ExecDelivery). The app-level door, unchanged in behaviour and |
| +// signature; SetExecDeliveryAt is the general one. |
| +func SetExecDelivery(ctx context.Context, s release.Store, acct, app, value string) error { |
| + return SetExecDeliveryAt(ctx, s, acct, app, "", value) |
| +} |
| + |
| +// SetExecDeliveryAt writes or clears exec_delivery in ONE bundle: the |
| +// default bundle (envName == "") or a named environment's, via the same |
| +// CAS read-modify-write path SetEnv/SetSecret/Unset use. value must be |
| +// ValidExecDelivery; anything else is refused here, before it ever reaches |
| +// the bucket, mirroring applyAt's var-name/var-value validation. |
| // |
| -// APP-LEVEL ONLY, structurally: unlike apply's applyAt generalization, |
| -// there is no environment-scoped variant of this function — it always |
| -// writes Key(acct, app), never EnvironmentKey(acct, app, name). A named |
| -// environment's manifest is never a valid place for this field (Manifest. |
| -// ExecDelivery's doc comment has the "config-that-lies" rationale); the |
| -// CLI's -environment flag is refused before it can reach here at all |
| -// (appconfig_cli.go's runEnvExecDelivery). |
| +// Environment scope is issue #195 (ruled 2026-08-16). The app-level flag |
| +// answered "does this APP get files", so opting one relay route in wrote a |
| +// file for every router route of the same app too — a fleet-wide switch |
| +// thrown to deliver one service's credential. The flag now lives wherever |
| +// the route's config comes from, and clearing it in a named bundle means |
| +// INHERIT the app default, never "off": a bundle that says nothing has not |
| +// asked for delivery to stop. Route scope is still refused — see |
| +// Manifest.ExecDelivery. |
| // |
| -// Turning the field off (value == "") against an app with no default |
| -// bundle at all is refused, not silently turned into a create — the same |
| -// "unset-only against a nonexistent object" refusal applyAt documents, for |
| -// the same reason: minting an empty bundle just to hold an absent flag |
| -// would satisfy `carlos route -environment`'s existence rail and Sync's |
| -// "configured, then fully unset" state for nothing the operator asked for. |
| -func SetExecDelivery(ctx context.Context, s release.Store, acct, app, value string) error { |
| +// Turning the field off against a bundle that does not exist is refused, |
| +// not silently turned into a create — the same "unset-only against a |
| +// nonexistent object" refusal applyAt documents, for the same reason: |
| +// minting an empty bundle just to hold an absent flag would satisfy |
| +// `carlos route -environment`'s existence rail and Sync's "configured, |
| +// then fully unset" state for nothing the operator asked for. |
| +func SetExecDeliveryAt(ctx context.Context, s release.Store, acct, app, envName, value string) error { |
| if !release.ValidAccount(acct) { |
| return fmt.Errorf("config: invalid account %q", acct) |
| } |
| @@ -255,15 +263,21 @@ func SetExecDelivery(ctx context.Context, s release.Store, acct, app, value stri |
| return fmt.Errorf("config: invalid app %q", app) |
| } |
| if !ValidExecDelivery(value) { |
| - return fmt.Errorf("config: invalid exec_delivery %q — want %q or \"\" (off)", value, ExecDeliveryFile) |
| + return fmt.Errorf("config: invalid exec_delivery %q — want %q or \"\" (off/inherit)", value, ExecDeliveryFile) |
| + } |
| + key, label := Key(acct, app), "default" |
| + if envName != "" { |
| + if !ValidEnvironmentName(envName) { |
| + return fmt.Errorf("config: invalid environment name %q", envName) |
| + } |
| + key, label = EnvironmentKey(acct, app, envName), envName |
| } |
| - key := Key(acct, app) |
| m, etag, found, err := getForWriteAt(ctx, s, key, app) |
| if err != nil { |
| return err |
| } |
| if !found && value == "" { |
| - return fmt.Errorf("config: %s has no default bundle — nothing to turn off", app) |
| + return fmt.Errorf("config: %s has no %s bundle — nothing to turn off", app, label) |
| } |
| m.ExecDelivery = value |
| m.Schema, m.App = 1, app |
| @@ -277,7 +291,10 @@ func SetExecDelivery(ctx context.Context, s release.Store, acct, app, value stri |
| // The opt-in flag changes what SyncExec delivers for this app's routes, |
| // so it is a config change like any other and earns the same fast |
| // convergence — see apply's comment for why the touch cannot fail this |
| - // call. |
| + // call. The generation object is per-APP, not per-bundle |
| + // (generation.go), so an environment-scoped write signals through the |
| + // same key an app-level one does — which is why the fast loop needs no |
| + // new bundle awareness for this half. |
| touchLogged(ctx, s, acct, app) |
| return nil |
| } |