amadan

carlos / platform Public

appconfig: mirror the instance-overlay dedup fix into syncexec (plan 4, task 6)

Task 4's said dedup was copied into syncexec.go before 82df5301 fixed the
same two defects in sync.go: a per-host Get/parse failure was folding into
the per-app listing-failure dedup (hiding a second corrupt overlay's
failure behind the first route's line), and said was keyed on groupKey
instead of appKey, over-scoping the dedup to (account, app, environment).
Mirrors 82df5301's errors.As(instanceOverlayListErr) branch and the appKey
key exactly, with syncexec's own file-not-env wording. instances.go needed
no change: instanceOverlayLoader already wraps only its listing failure,
usable by both sweeps as-is.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul Campbell pushed by claude e273fd57860677e2773933499797680db1f2a1b5 parent 82df530
2 files changed, +198 −11
  • internal/appconfig/instances_test.go +148 −0
  • internal/appconfig/syncexec.go +50 −11
diff --git a/internal/appconfig/instances_test.go b/internal/appconfig/instances_test.go
index 94f488a19..3708c290f 100644
--- a/internal/appconfig/instances_test.go
+++ b/internal/appconfig/instances_test.go
@@ -817,3 +817,151 @@ func TestSyncExecNeverReadsExecDeliveryFromTheInstanceLayer(t *testing.T) {
t.Fatalf("the overlay's vars did not land once the app opted in:\n%s", cur)
}
}
+
+// The exec mirror of TestSyncReportsEachRoutesOverlayGetFailureSeparately:
+// two independently corrupt overlays are two independent failures, not one.
+// The listing itself succeeds (both hosts are named), so the "once per app"
+// dedup for a failed LISTING must not swallow the second route's Get
+// failure — that would leave one of two stale config files invisible,
+// which is exactly what this layer exists to surface.
+func TestSyncExecReportsEachRoutesOverlayGetFailureSeparately(t *testing.T) {
+ ctx, s, reg, dd, privB64, _ := execKit(t)
+ priv, err := ParsePrivateKey(privB64, "test")
+ if err != nil {
+ t.Fatalf("ParsePrivateKey: %v", err)
+ }
+ if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"APP_WIDE": "yes"}); err != nil {
+ t.Fatalf("SetEnv: %v", err)
+ }
+ // Schema 2 is refused by getIfExistsAtETag ("newer than this binary
+ // understands"), which is a Get-time failure specific to the object
+ // read, not a listing failure — instanceOverlays only reads keys, never
+ // their bodies, so the listing itself succeeds for both hosts.
+ putInstanceRaw(t, ctx, s, "bab", "titogo", "book.example.com", &Manifest{Schema: 2, App: "titogo"})
+ putInstanceRaw(t, ctx, s, "bab", "titogo", "cart.example.com", &Manifest{Schema: 2, App: "titogo"})
+ for _, host := range []string{"book.example.com", "cart.example.com"} {
+ if err := reg.Upsert(registry.Route{
+ Host: host, Kind: "instance", App: "titogo", Account: "bab", Status: "ready",
+ Socket: "/run/carlos/" + host + ".sock",
+ }); err != nil {
+ t.Fatalf("Upsert %s: %v", host, err)
+ }
+ }
+
+ out := captureStdout(t, func() {
+ if err := SyncExec(ctx, reg, s, priv, dd, func(string) error { return nil }, "", nil); err != nil {
+ t.Fatalf("SyncExec: %v", err)
+ }
+ })
+ for _, host := range []string{"book.example.com", "cart.example.com"} {
+ if !strings.Contains(out, host) {
+ t.Fatalf("no line for %s's overlay failure, one route's stale file went unreported:\n%s", host, out)
+ }
+ }
+ if n := strings.Count(out, "instance config unresolved"); n != 2 {
+ t.Fatalf("instance config unresolved lines = %d, want 2 (one per corrupt overlay), got:\n%s", n, out)
+ }
+ // Neither route had a file before this pass, and a Get failure must
+ // leave that exactly as it is: no config file materializes from a
+ // resolved group merge that an unresolved overlay was supposed to sit
+ // on top of.
+ for _, host := range []string{"book.example.com", "cart.example.com"} {
+ if _, err := os.Stat(filepath.Join(dd, host+".config.env")); !os.IsNotExist(err) {
+ t.Fatalf("%s.config.env exists after an unresolved overlay: %v", host, err)
+ }
+ }
+}
+
+// An app with no default bundle at all, and one instance with an overlay.
+// The overlay is the whole configuration of that route, so its file is
+// written, not removed — while its sibling, which has nothing anywhere, is
+// reaped exactly as it is today.
+func TestSyncExecConvergesAnOverlayAloneInsteadOfRemoving(t *testing.T) {
+ ctx, s, reg, dd, privB64, _ := execKit(t)
+ priv, err := ParsePrivateKey(privB64, "test")
+ if err != nil {
+ t.Fatalf("ParsePrivateKey: %v", err)
+ }
+ if _, _, err := ApplyInstance(ctx, s, "bab", "titogo", "book.example.com",
+ map[string]Var{"TENANT": {Kind: "env", Value: "book"}}, nil); err != nil {
+ t.Fatalf("ApplyInstance: %v", err)
+ }
+ for _, host := range []string{"book.example.com", "cart.example.com"} {
+ if err := reg.Upsert(registry.Route{
+ Host: host, Kind: "instance", App: "titogo", Account: "bab", Status: "ready",
+ Socket: "/run/carlos/" + host + ".sock",
+ }); err != nil {
+ t.Fatalf("Upsert %s: %v", host, err)
+ }
+ if err := os.WriteFile(filepath.Join(dd, host+".config.env"), []byte("OLD=1\n"), 0o600); err != nil {
+ t.Fatalf("stage %s: %v", host, err)
+ }
+ }
+ if err := SyncExec(ctx, reg, s, priv, dd, func(string) error { return nil }, "", nil); err != nil {
+ t.Fatalf("SyncExec: %v", err)
+ }
+ book, err := os.ReadFile(filepath.Join(dd, "book.example.com.config.env"))
+ if err != nil {
+ t.Fatalf("the overlaid route's file was removed: %v", err)
+ }
+ if !bytes.Contains(book, []byte("TENANT=book\n")) || bytes.Contains(book, []byte("OLD=1")) {
+ t.Fatalf("the overlay-alone render did not converge:\n%s", book)
+ }
+ if _, err := os.Stat(filepath.Join(dd, "cart.example.com.config.env")); !os.IsNotExist(err) {
+ t.Fatalf("a route with no config anywhere kept its file: %v", err)
+ }
+}
+
+// The mass-removal guard, with an overlay in the middle of it. Two apps
+// whose manifests all read absent still freeze every removal on the box,
+// and the one route that has an overlay still converges: writing a file a
+// bundle asked for is not a removal, and the guard exists to stop
+// deletions, not delivery.
+func TestSyncExecMassRemovalGuardHoldsRemovalsButNotOverlays(t *testing.T) {
+ ctx, s, reg, dd, privB64, _ := execKit(t)
+ priv, err := ParsePrivateKey(privB64, "test")
+ if err != nil {
+ t.Fatalf("ParsePrivateKey: %v", err)
+ }
+ if _, _, err := ApplyInstance(ctx, s, "bab", "titogo", "book.example.com",
+ map[string]Var{"TENANT": {Kind: "env", Value: "book"}}, nil); err != nil {
+ t.Fatalf("ApplyInstance: %v", err)
+ }
+ routes := map[string]string{
+ "book.example.com": "titogo",
+ "cart.example.com": "titogo",
+ "desk.example.com": "eleven",
+ }
+ for host, app := range routes {
+ if err := reg.Upsert(registry.Route{
+ Host: host, Kind: "instance", App: app, Account: "bab", Status: "ready",
+ Socket: "/run/carlos/" + host + ".sock",
+ }); err != nil {
+ t.Fatalf("Upsert %s: %v", host, err)
+ }
+ if err := os.WriteFile(filepath.Join(dd, host+".config.env"), []byte("OLD=1\n"), 0o600); err != nil {
+ t.Fatalf("stage %s: %v", host, err)
+ }
+ }
+ out := captureStdout(t, func() {
+ if err := SyncExec(ctx, reg, s, priv, dd, func(string) error { return nil }, "", nil); err != nil {
+ t.Fatalf("SyncExec: %v", err)
+ }
+ })
+ if !strings.Contains(out, "refusing to remove any config files this pass") {
+ t.Fatalf("the mass-removal guard did not fire:\n%s", out)
+ }
+ for _, host := range []string{"cart.example.com", "desk.example.com"} {
+ cur, err := os.ReadFile(filepath.Join(dd, host+".config.env"))
+ if err != nil || string(cur) != "OLD=1\n" {
+ t.Fatalf("%s lost its file while the guard was up: %q %v", host, cur, err)
+ }
+ }
+ book, err := os.ReadFile(filepath.Join(dd, "book.example.com.config.env"))
+ if err != nil {
+ t.Fatalf("the overlaid route was frozen by a guard about removals: %v", err)
+ }
+ if !bytes.Contains(book, []byte("TENANT=book\n")) {
+ t.Fatalf("the overlay did not converge under the guard:\n%s", book)
+ }
+}
diff --git a/internal/appconfig/syncexec.go b/internal/appconfig/syncexec.go
index f9180957d..826d983b3 100644
--- a/internal/appconfig/syncexec.go
+++ b/internal/appconfig/syncexec.go
@@ -37,6 +37,7 @@ import (
"bytes"
"context"
"crypto/ecdh"
+ "errors"
"fmt"
"os"
"path/filepath"
@@ -362,6 +363,14 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
// beats an unbounded silent freeze; routes bound to an environment are
// protected by the error path regardless, since an unresolved group
// never reaches a removal at all.
+ //
+ // The instance layer does not enter this count, and does not need to.
+ // The fault this guard exists for is registry-wide — a mangled account
+ // column makes EVERY key of every app 404 at once, the overlay listing
+ // included, since it hangs off the same mangled prefix — so an overlay
+ // could not survive the fault it is counting. A route that does have an
+ // overlay never reaches a removal branch at all (see below), so it is
+ // neither protected nor endangered by this number.
eligible, absent := 0, 0
for _, r := range resolved {
if r.err != nil {
@@ -384,9 +393,15 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
// pass's: one listing per app, a Get only for a host it named, and an
// unresolved overlay leaves the file exactly as it is.
overlayFor := instanceOverlayLoader(ctx, s)
- // One line per app when its listing fails, not one per route: every
- // route of that app fails identically and for the same reason.
- said := map[groupKey]bool{}
+ // One line per app when the LISTING fails, not one per route: every
+ // route of that app fails identically and for the same reason, since
+ // instanceOverlayLoader caches that failure per (account, app) —
+ // independent of Environment, which is why this is keyed on appKey and
+ // not groupKey. A per-host Get or parse failure is a DIFFERENT claim —
+ // two routes can have two separately corrupt overlay objects, each
+ // wrong for its own reason — so that case is never folded in here; see
+ // the errors.As branch below.
+ said := map[appKey]bool{}
for _, rt := range routes {
in, addrShape, sidecarShape := inScope(rt)
@@ -420,13 +435,36 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
// delivered secrets on a listing blip.
inst, _, instErr := overlayFor(k.acct, k.app, rt.Host)
if instErr != nil {
- if !said[k] {
- said[k] = true
- fmt.Printf(" %s/%s: instance config unresolved, config files left as-is: %v\n", k.acct, k.app, instErr)
+ var listErr *instanceOverlayListErr
+ if errors.As(instErr, &listErr) {
+ // Said ONCE per (account, app), not once per route: the
+ // listing this failed on is cached per app inside the
+ // loader, so every route of the app, across every
+ // environment it is bound to, fails identically and for
+ // the same reason; a line each would bury the cause under
+ // its own repetitions on a box with many routes.
+ ak := appKey{k.acct, k.app}
+ if !said[ak] {
+ said[ak] = true
+ fmt.Printf(" %s/%s: instance config unresolved, config files left as-is: %v\n", k.acct, k.app, instErr)
+ }
+ } else {
+ // A per-host Get or parse failure, unlike the listing
+ // above, is specific to THIS route: the listing already
+ // succeeded (it named this host), so another route of
+ // the same app can have a perfectly good overlay, or a
+ // failure of its own for a different reason. Folding this
+ // into the app-level line would leave that route's stale
+ // config file invisible, so it is reported every time.
+ fmt.Printf(" %s: instance config unresolved, config file left as-is: %v\n", rt.Host, instErr)
}
continue
}
- if r.m == nil && skipRemovals {
+ // An overlay is configuration, so a route that has one is never
+ // "absent" however its group base resolved. Both branches below decide
+ // REMOVAL, and neither may fire for a route whose own layer is
+ // standing right there.
+ if r.m == nil && inst == nil && skipRemovals {
continue
}
// The #344 guard: an absent manifest removes the config file ONLY
@@ -436,7 +474,7 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
// stamps it from bucket listings, and a mis-stamp deleted a live
// tenant's delivered secrets on 2026-08-28). Held, loudly, like an
// unresolved group — the fix is the row's account, not this file.
- if r.m == nil {
+ if r.m == nil && inst == nil {
if sq := release.HostAccountLabel(rt.Host, appsDomain); sq != "" && sq != k.acct {
fmt.Printf(" %s: no config manifest under account %q, but the host names account %q — config file left as-is; fix the route row's account column (#344)\n",
rt.Host, k.acct, sq)
@@ -488,10 +526,11 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
hostBounce = nil
}
// The group's merge, overlaid by this instance's own layer. A nil
- // base stays nil: an absent manifest is a REMOVAL on this path, and
- // task 7 is the one case where an overlay changes that.
+ // base with an overlay is the overlay ALONE, which is a real
+ // configuration and not the absence that drives a removal; nil and
+ // nil together stay nil, which still removes the file.
m := r.m
- if m != nil {
+ if m != nil || inst != nil {
m = applyInstanceOverlay(k.app, m, inst)
}
syncExecHost(rt, m, Key(k.acct, k.app), priv, dataDir, hostBounce, header, ack)