amadan
appconfig: Sync renders each route's merged environment
Sync resolved apps/<acct>/<app>/config.json per (Account, App) and rendered it into every route of that app. With Route.Environment bound, two routes of one app resolve to different manifests, so the resolution cache key gains Environment and the bound case goes through LoadMerged (default overlaid by the named bundle) instead of the bare default. Keying on (acct, app) alone would have rendered whichever route was seen first into both files. A bound environment asserts the bundle exists, so LoadMerged errors when it is gone. That error is contained per route and, critically, never folded into an empty manifest: an empty render differs from the file on disk, so it would truncate a live route's env file as root and restart its unit — the class of bug PR #9's review caught on the empty-route-list path. The cache entry now distinguishes three states that are not interchangeable: a manifest to converge to, "never configured" (silent skip, the hand-staged-file promise), and "resolution failed" (loud per-route skip, file left as-is). Store fetch failures move to that same per-route line, replacing the per-app one — on a fetch failure the operator's question is which routes are now stale. Reaping is untouched and stays keyed on route rows only; a deleted environment bundle is not a reap trigger, and there is now a test pinning that. Restart-on-change semantics unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 files changed,
+356
−32
internal/appconfig/sync.go+95 −32internal/appconfig/sync_test.go+261 −0
diff --git a/internal/appconfig/sync.go b/internal/appconfig/sync.go| index 3bc7d0112..e019c75c3 100644 |
| --- a/internal/appconfig/sync.go |
| +++ b/internal/appconfig/sync.go |
| @@ -1,8 +1,11 @@ |
| -// Sync is the host-side convergence sweep: for every app the box's |
| -// registry runs, fetch its config manifest once and materialize the |
| -// result into every route's EnvironmentFile. Shaped after release.Pass |
| -// (resolve once per group, converge once per route) but with none of that |
| -// function's staging/symlink/bake machinery — config isn't versioned. |
| +// Sync is the host-side convergence sweep: for every (app, environment) the |
| +// box's registry runs, resolve the merged config manifest once and |
| +// materialize the result into every route's EnvironmentFile. Shaped after |
| +// release.Pass (resolve once per group, converge once per route) but with |
| +// none of that function's staging/symlink/bake machinery — config isn't |
| +// versioned. The group is (Account, App, Environment): a route's bound |
| +// environment (environments.go, pipelines design §4a) is an overlay on the |
| +// app's default bundle, so two routes of one app can resolve differently. |
| package appconfig |
| import ( |
| @@ -23,50 +26,102 @@ import ( |
| // (EnvironmentFile=-/etc/carlos/apps/%i.env, %i = route Host). |
| const DefaultEnvDir = "/etc/carlos/apps" |
| -// Sync fetches apps/<acct>/<app>/config.json once per distinct |
| -// (Account, App) among reg's routes, then for every route running that |
| -// app renders <envDir>/<route.Host>.env and, if it changed, writes it and |
| +// Sync resolves each route's merged config once per distinct |
| +// (Account, App, Environment) among reg's routes, then for every route |
| +// renders <envDir>/<route.Host>.env and, if it changed, writes it and |
| // restarts that route's unit via restart. Die-free by design, like |
| -// release.Pass: a fetch or render failure for one app logs and continues |
| -// rather than aborting the whole sweep. |
| +// release.Pass: a fetch or resolve failure for one app+environment logs and |
| +// continues rather than aborting the whole sweep. |
| +// |
| +// The cache key carries Environment as well as (Account, App) because two |
| +// routes of ONE app bound to different environments resolve to different |
| +// manifests (pipelines design §4a): keying on (acct, app) alone would render |
| +// whichever route was seen first into both files. Distinct environments cost |
| +// one extra resolve each, still one pass over the routes. |
| func Sync(ctx context.Context, reg *registry.Registry, s release.Store, priv *ecdh.PrivateKey, envDir string, restart func(host string) error) error { |
| routes, err := reg.List() |
| if err != nil { |
| return err |
| } |
| - type groupKey struct{ acct, app string } |
| - manifests := map[groupKey]*Manifest{} |
| + type groupKey struct{ acct, app, env string } |
| + // A resolution is one of three states, and they are NOT interchangeable: |
| + // a manifest to converge to; "this app was never configured" (nil, nil), |
| + // which leaves the file alone; and "resolution failed" (nil, err), which |
| + // also leaves the file alone but says so, loudly, per route. Never fold a |
| + // failure into an empty manifest: an empty render differs from whatever |
| + // is on disk, so it would truncate a live route's env file as root and |
| + // restart the unit on it. |
| + type resolution struct { |
| + m *Manifest |
| + err error |
| + } |
| + resolved := map[groupKey]resolution{} |
| for _, rt := range routes { |
| if !syncable(rt) { |
| continue |
| } |
| - k := groupKey{release.RouteAccount(rt), rt.App} |
| - if _, done := manifests[k]; done { |
| + k := groupKey{release.RouteAccount(rt), rt.App, rt.Environment} |
| + if _, done := resolved[k]; done { |
| continue |
| } |
| - // getIfExists, not Get: an app with no manifest object in the bucket |
| - // at all is left entirely alone (nil manifest => skipped below), so |
| - // turning this feature on does not truncate env files that are still |
| - // hand-staged on the host. Get folds not-found into an empty |
| - // manifest, which would be indistinguishable from a real, fully |
| - // unset one — and those must converge. |
| - m, found, err := getIfExists(ctx, s, k.acct, k.app) |
| - if err != nil { |
| - fmt.Printf(" %s/%s: config fetch failed: %v\n", k.acct, k.app, err) |
| - manifests[k] = nil |
| + if k.env == "" { |
| + // getIfExists, not Get: an app with no manifest object in the |
| + // bucket at all is left entirely alone (nil manifest => skipped |
| + // below), so turning this feature on does not truncate env files |
| + // that are still hand-staged on the host. Get folds not-found |
| + // into an empty manifest, which would be indistinguishable from a |
| + // real, fully unset one — and those must converge. LoadMerged |
| + // makes the same fold for envName == "", which is why the |
| + // unbound case still goes through getIfExists here. |
| + m, found, err := getIfExists(ctx, s, k.acct, k.app) |
| + if err != nil { |
| + // Reported per route below rather than here (where PR #9 |
| + // printed one line per app): the operator's question on a |
| + // fetch failure is which routes are now running on a stale |
| + // file, and the answer is per route either way. |
| + resolved[k] = resolution{err: fmt.Errorf("%s/%s: config fetch failed: %w", k.acct, k.app, err)} |
| + continue |
| + } |
| + if !found { |
| + resolved[k] = resolution{} |
| + continue |
| + } |
| + resolved[k] = resolution{m: m} |
| continue |
| } |
| - if !found { |
| - manifests[k] = nil |
| + // A bound environment asserts the bundle exists, so LoadMerged treats |
| + // its absence as an error rather than an empty overlay — and that |
| + // error must stay contained to the routes bound to it. No |
| + // never-configured case here: an app can lack a default bundle and |
| + // still resolve (environment-only), and if the environment itself is |
| + // missing that is the error above, not silence. |
| + m, err := LoadMerged(ctx, s, k.acct, k.app, k.env) |
| + if err != nil || m == nil { |
| + if err == nil { |
| + err = fmt.Errorf("config: resolved a nil manifest for environment %q", k.env) |
| + } |
| + resolved[k] = resolution{err: err} |
| continue |
| } |
| - manifests[k] = m |
| + resolved[k] = resolution{m: m} |
| } |
| for _, rt := range routes { |
| if !syncable(rt) { |
| continue |
| } |
| - m := manifests[groupKey{release.RouteAccount(rt), rt.App}] |
| + r := resolved[groupKey{release.RouteAccount(rt), rt.App, rt.Environment}] |
| + if r.err != nil { |
| + // Loud, per route, every pass: this route is running on a stale |
| + // env file until an operator fixes it. Its file is left exactly |
| + // as it is — nothing below this line runs for it. |
| + env := rt.Environment |
| + if env == "" { |
| + env = "default" |
| + } |
| + fmt.Printf(" %s: config unresolved (environment %q), env file left as-is: %v\n", rt.Host, env, r.err) |
| + continue |
| + } |
| + m := r.m |
| if m == nil { |
| continue |
| } |
| @@ -102,6 +157,12 @@ func Sync(ctx context.Context, reg *registry.Registry, s release.Store, priv *ec |
| // assumption release adoption's symlink repointing already makes), so a |
| // host with zero rows is reaped immediately, with no grace period. A |
| // missing or unreadable env dir just means there is nothing to reap. |
| +// |
| +// Config state is deliberately not an input here: whether an app has a |
| +// manifest, or whether a route's bound environment still exists in the |
| +// bucket, has no bearing on whether its file is orphaned. Deleting an |
| +// environment bundle must never delete a live route's env file — that route |
| +// keeps its last-good file and Sync logs the unresolved binding instead. |
| func reap(routes []registry.Route, envDir string) { |
| ents, err := os.ReadDir(envDir) |
| if err != nil { |
| @@ -150,10 +211,12 @@ func syncable(rt registry.Route) bool { |
| (rt.Status == "" || rt.Status == "ready") |
| } |
| -// syncHost converges one route's env file. It is called only for apps whose |
| -// manifest object really exists in the bucket, so an empty manifest here |
| -// means "every var was unset" and converges the file to empty — the |
| -// "never configured at all" case is filtered out by Sync, before this. |
| +// syncHost converges one route's env file. It is called only with a manifest |
| +// that really resolved — the object (default bundle, named environment, or |
| +// both) exists in the bucket — so an empty manifest here means "every var |
| +// was unset" and converges the file to empty. The "never configured at all" |
| +// and "resolution failed" cases are both filtered out by Sync, before this: |
| +// either one reaching here would truncate a live env file. |
| func syncHost(rt registry.Route, m *Manifest, priv *ecdh.PrivateKey, envDir string, restart func(host string) error) { |
| // The host is joined into a root-written path; a registry row carrying |
| // a separator must never place a file outside envDir. Same trust level |
diff --git a/internal/appconfig/sync_test.go b/internal/appconfig/sync_test.go| index 0a0c4aced..d2849cb88 100644 |
| --- a/internal/appconfig/sync_test.go |
| +++ b/internal/appconfig/sync_test.go |
| @@ -435,6 +435,267 @@ func TestSyncSurvivesAMissingEnvDir(t *testing.T) { |
| } |
| } |
| +func TestSyncRendersEachRoutesBoundEnvironment(t *testing.T) { |
| + // One app, three routes of it, two of them bound to different named |
| + // environments. Each route's file must be the default bundle overlaid by |
| + // ITS OWN environment — no leakage of one route's environment into |
| + // another's render (the per-(acct, app) fetch cache must not collapse |
| + // routes that resolve differently). |
| + s := release.NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + reg := testRegistry(t) |
| + envDir := t.TempDir() |
| + priv, _, err := GenerateKey() |
| + if err != nil { |
| + t.Fatalf("GenerateKey: %v", err) |
| + } |
| + privKey, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "keymail", map[string]string{"LOG_LEVEL": "info", "SHARED": "yes"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if err := SaveEnvironment(ctx, s, "bab", "keymail", "demo", map[string]Var{ |
| + "LOG_LEVEL": {Kind: "env", Value: "debug"}, |
| + "DEMO_ONLY": {Kind: "env", Value: "1"}, |
| + }, nil); err != nil { |
| + t.Fatalf("SaveEnvironment demo: %v", err) |
| + } |
| + if err := SaveEnvironment(ctx, s, "bab", "keymail", "staging", map[string]Var{ |
| + "LOG_LEVEL": {Kind: "env", Value: "trace"}, |
| + "STAGING_ONLY": {Kind: "env", Value: "1"}, |
| + }, nil); err != nil { |
| + t.Fatalf("SaveEnvironment staging: %v", err) |
| + } |
| + routes := map[string]string{ // host -> bound environment |
| + "plain.example.com": "", |
| + "demo.example.com": "demo", |
| + "staging.example.com": "staging", |
| + } |
| + for host, env := range routes { |
| + if err := reg.Upsert(registry.Route{ |
| + Host: host, Kind: "instance", App: "keymail", Account: "bab", Status: "ready", |
| + Backing: registry.BackingUnit, Environment: env, |
| + }); err != nil { |
| + t.Fatalf("Upsert %s: %v", host, err) |
| + } |
| + } |
| + |
| + var restarted []string |
| + restart := func(h string) error { restarted = append(restarted, h); return nil } |
| + if err := Sync(ctx, reg, s, privKey, envDir, restart); err != nil { |
| + t.Fatalf("Sync: %v", err) |
| + } |
| + |
| + want := map[string]string{ |
| + "plain.example.com": "LOG_LEVEL=info\nSHARED=yes\n", |
| + "demo.example.com": "DEMO_ONLY=1\nLOG_LEVEL=debug\nSHARED=yes\n", |
| + "staging.example.com": "LOG_LEVEL=trace\nSHARED=yes\nSTAGING_ONLY=1\n", |
| + } |
| + for host, body := range want { |
| + got, err := os.ReadFile(filepath.Join(envDir, host+".env")) |
| + if err != nil { |
| + t.Fatalf("%s: read rendered env: %v", host, err) |
| + } |
| + if string(got) != body { |
| + t.Fatalf("%s: rendered env = %q, want %q", host, got, body) |
| + } |
| + } |
| + if len(restarted) != 3 { |
| + t.Fatalf("restarted = %v, want all three routes", restarted) |
| + } |
| + |
| + // Unchanged merged output on a second pass: no rewrite, no restart — |
| + // including for the environment-bound routes, whose merge must be stable. |
| + before := map[string]os.FileInfo{} |
| + for host := range want { |
| + fi, err := os.Stat(filepath.Join(envDir, host+".env")) |
| + if err != nil { |
| + t.Fatalf("%s: stat: %v", host, err) |
| + } |
| + before[host] = fi |
| + } |
| + restarted = nil |
| + if err := Sync(ctx, reg, s, privKey, envDir, restart); err != nil { |
| + t.Fatalf("Sync (second pass): %v", err) |
| + } |
| + if len(restarted) != 0 { |
| + t.Fatalf("second pass restarted %v, want none", restarted) |
| + } |
| + for host, fi := range before { |
| + now, err := os.Stat(filepath.Join(envDir, host+".env")) |
| + if err != nil { |
| + t.Fatalf("%s: stat after second pass: %v", host, err) |
| + } |
| + if !now.ModTime().Equal(fi.ModTime()) { |
| + t.Fatalf("%s: env file was rewritten on an unchanged pass", host) |
| + } |
| + } |
| +} |
| + |
| +func TestSyncLeavesEnvFileUntouchedWhenBoundEnvironmentIsMissing(t *testing.T) { |
| + // The bundle was deleted out from under a bound route. That route's |
| + // resolution FAILS — and a failed resolution must never reach the |
| + // render-and-diff path, because an empty render differs from the file on |
| + // disk and would truncate it as root and restart the unit. The last-good |
| + // file stays byte-identical and untouched; other routes of the same app |
| + // still converge. |
| + s := release.NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + reg := testRegistry(t) |
| + envDir := t.TempDir() |
| + priv, _, err := GenerateKey() |
| + if err != nil { |
| + t.Fatalf("GenerateKey: %v", err) |
| + } |
| + privKey, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "keymail", map[string]string{"LOG_LEVEL": "info"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + for host, env := range map[string]string{ |
| + "gone.example.com": "vanished", |
| + "ok.example.com": "", |
| + } { |
| + if err := reg.Upsert(registry.Route{ |
| + Host: host, Kind: "instance", App: "keymail", Account: "bab", Status: "ready", |
| + Backing: registry.BackingUnit, Environment: env, |
| + }); err != nil { |
| + t.Fatalf("Upsert %s: %v", host, err) |
| + } |
| + } |
| + lastGood := "LOG_LEVEL=debug\nDB_URL=postgres://last-good\n" |
| + path := filepath.Join(envDir, "gone.example.com.env") |
| + if err := os.WriteFile(path, []byte(lastGood), 0o600); err != nil { |
| + t.Fatalf("stage last-good env file: %v", err) |
| + } |
| + fi, err := os.Stat(path) |
| + if err != nil { |
| + t.Fatalf("stat staged file: %v", err) |
| + } |
| + |
| + var restarted []string |
| + if err := Sync(ctx, reg, s, privKey, envDir, func(h string) error { |
| + restarted = append(restarted, h) |
| + return nil |
| + }); err != nil { |
| + t.Fatalf("Sync: %v", err) |
| + } |
| + |
| + got, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("last-good env file gone after Sync: %v", err) |
| + } |
| + if string(got) != lastGood { |
| + t.Fatalf("env file = %q, want it untouched (%q)", got, lastGood) |
| + } |
| + now, err := os.Stat(path) |
| + if err != nil { |
| + t.Fatalf("stat after Sync: %v", err) |
| + } |
| + if !now.ModTime().Equal(fi.ModTime()) { |
| + t.Fatalf("env file of a route with a missing environment was rewritten") |
| + } |
| + if _, err := os.Stat(path + ".tmp"); !os.IsNotExist(err) { |
| + t.Fatalf("a temp file was written for a route whose environment is missing, stat err = %v", err) |
| + } |
| + for _, h := range restarted { |
| + if h == "gone.example.com" { |
| + t.Fatalf("restarted a route whose bound environment is missing: %v", restarted) |
| + } |
| + } |
| + // The sibling route must be entirely unaffected by the other's failure. |
| + sibling, err := os.ReadFile(filepath.Join(envDir, "ok.example.com.env")) |
| + if err != nil { |
| + t.Fatalf("sibling route not materialized: %v", err) |
| + } |
| + if string(sibling) != "LOG_LEVEL=info\n" { |
| + t.Fatalf("sibling env = %q, want LOG_LEVEL=info", sibling) |
| + } |
| + if len(restarted) != 1 || restarted[0] != "ok.example.com" { |
| + t.Fatalf("restarted = %v, want exactly [ok.example.com]", restarted) |
| + } |
| +} |
| + |
| +func TestSyncDoesNotReapEnvFileWhenEnvironmentIsDeleted(t *testing.T) { |
| + // Reaping is keyed on ROUTES, never on environments: deleting a bundle |
| + // must not make a live route's env file look orphaned. |
| + s := release.NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + reg := testRegistry(t) |
| + envDir := t.TempDir() |
| + priv, _, err := GenerateKey() |
| + if err != nil { |
| + t.Fatalf("GenerateKey: %v", err) |
| + } |
| + privKey, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "bound.example.com", Kind: "instance", App: "keymail", Account: "bab", Status: "ready", |
| + Backing: registry.BackingUnit, Environment: "deleted-env", |
| + }); err != nil { |
| + t.Fatalf("Upsert: %v", err) |
| + } |
| + path := filepath.Join(envDir, "bound.example.com.env") |
| + if err := os.WriteFile(path, []byte("KEEP=1\n"), 0o600); err != nil { |
| + t.Fatalf("stage env file: %v", err) |
| + } |
| + if err := Sync(ctx, reg, s, privKey, envDir, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Sync: %v", err) |
| + } |
| + got, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("env file reaped after its environment was deleted: %v", err) |
| + } |
| + if string(got) != "KEEP=1\n" { |
| + t.Fatalf("env file = %q, want it untouched", got) |
| + } |
| +} |
| + |
| +func TestSyncEnvironmentOnlyAppWithNoDefaultBundle(t *testing.T) { |
| + // A named environment does not require a default bundle to exist: |
| + // LoadMerged resolves to the environment alone, and Sync must converge it |
| + // (the object exists, so this is a configured app, not an unconfigured one). |
| + s := release.NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + reg := testRegistry(t) |
| + envDir := t.TempDir() |
| + priv, _, err := GenerateKey() |
| + if err != nil { |
| + t.Fatalf("GenerateKey: %v", err) |
| + } |
| + privKey, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SaveEnvironment(ctx, s, "bab", "keymail", "demo", map[string]Var{ |
| + "ONLY": {Kind: "env", Value: "here"}, |
| + }, nil); err != nil { |
| + t.Fatalf("SaveEnvironment: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "demo.example.com", Kind: "instance", App: "keymail", Account: "bab", Status: "ready", |
| + Backing: registry.BackingUnit, Environment: "demo", |
| + }); err != nil { |
| + t.Fatalf("Upsert: %v", err) |
| + } |
| + if err := Sync(ctx, reg, s, privKey, envDir, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Sync: %v", err) |
| + } |
| + got, err := os.ReadFile(filepath.Join(envDir, "demo.example.com.env")) |
| + if err != nil { |
| + t.Fatalf("read rendered env: %v", err) |
| + } |
| + if string(got) != "ONLY=here\n" { |
| + t.Fatalf("rendered env = %q, want ONLY=here", got) |
| + } |
| +} |
| + |
| func TestSyncMaterializesEveryRouteOfAMultiRouteApp(t *testing.T) { |
| // One app, two simultaneous routes (e.g. a stable route and a canary |
| // route are two different Hosts) — the shared config must land in |