amadan

carlos / platform Public

appconfig: the unit sync pass applies each route's instance overlay (plan 4, task 4)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul Campbell pushed by claude 77cd1f1ee216f6a9e409236467e4c949760f6d2e parent e3c5713
3 files changed, +286 −1
  • internal/appconfig/instances.go +50 −0
  • internal/appconfig/instances_test.go +202 −0
  • internal/appconfig/sync.go +34 −1
diff --git a/internal/appconfig/instances.go b/internal/appconfig/instances.go
index 4c04ed307..88ac66fab 100644
--- a/internal/appconfig/instances.go
+++ b/internal/appconfig/instances.go
@@ -261,3 +261,53 @@ func instanceOverlays(ctx context.Context, s release.Store, acct, app string) (m
}
return out, nil
}
+
+// instanceOverlayLoader is one pass's instance-layer reader, shared by both
+// sweeps so neither can drift from the other about what a host gets or what
+// it costs. One List per (account, app), cached for the pass including its
+// failure, then a Get only for a host that listing named.
+//
+// A listing FAILURE is returned to the caller, never folded into "no
+// overlay". The two sweeps treat it the way they treat a bundle fetch
+// failure: leave the file exactly as it is, say so per route, retry next
+// pass. Folding it away would silently deliver the app-wide merge to an
+// instance that is meant to have its own credential, which is the one
+// outcome this layer exists to prevent.
+//
+// A host the listing named but whose object is gone by the time of the Get
+// is simply "no overlay": a delete racing the pass converges the route back
+// to its group's merge, which is what the delete asked for.
+func instanceOverlayLoader(ctx context.Context, s release.Store) func(acct, app, host string) (*Manifest, string, error) {
+ type appKey struct{ acct, app string }
+ type indexRes struct {
+ hosts map[string]bool
+ err error
+ }
+ index := map[appKey]indexRes{}
+ return func(acct, app, host string) (*Manifest, string, error) {
+ ak := appKey{acct, app}
+ idx, done := index[ak]
+ if !done {
+ hosts, err := instanceOverlays(ctx, s, acct, app)
+ if err != nil {
+ err = fmt.Errorf("%s/%s: instance config listing failed: %w", acct, app, err)
+ }
+ idx = indexRes{hosts: hosts, err: err}
+ index[ak] = idx
+ }
+ if idx.err != nil {
+ return nil, "", idx.err
+ }
+ if !idx.hosts[host] {
+ return nil, "", nil
+ }
+ m, etag, found, err := LoadInstanceConfig(ctx, s, acct, app, host)
+ if err != nil {
+ return nil, "", fmt.Errorf("%s/%s: instance config for %s: %w", acct, app, host, err)
+ }
+ if !found {
+ return nil, "", nil
+ }
+ return m, etag, nil
+ }
+}
diff --git a/internal/appconfig/instances_test.go b/internal/appconfig/instances_test.go
index 8cbaf7fed..9e4420e51 100644
--- a/internal/appconfig/instances_test.go
+++ b/internal/appconfig/instances_test.go
@@ -1,11 +1,18 @@
package appconfig
import (
+ "bytes"
"context"
+ "crypto/ecdh"
"encoding/json"
+ "errors"
+ "os"
+ "path/filepath"
"strings"
+ "sync"
"testing"
+ "github.com/carlosframework/platform/internal/registry"
"github.com/carlosframework/platform/internal/release"
)
@@ -374,3 +381,198 @@ func TestInstanceOverlaysListsOnlyThisAppsHosts(t *testing.T) {
t.Fatal("another app's overlay leaked into this app's index")
}
}
+
+// countingStore counts the reads and writes a pass makes, per key, so a
+// test can pin the COST of a feature and not just its output. The instance
+// layer's promise is one extra listing per app and one extra Get per
+// instance that actually has an overlay, which is a claim only a counter
+// can keep honest.
+type countingStore struct {
+ release.Store
+ mu sync.Mutex
+ gets map[string]int
+ lists map[string]int
+ puts map[string]int
+}
+
+func newCountingStore(s release.Store) *countingStore {
+ return &countingStore{Store: s, gets: map[string]int{}, lists: map[string]int{}, puts: map[string]int{}}
+}
+
+func (c *countingStore) Get(ctx context.Context, key string) ([]byte, string, error) {
+ c.mu.Lock()
+ c.gets[key]++
+ c.mu.Unlock()
+ return c.Store.Get(ctx, key)
+}
+
+func (c *countingStore) List(ctx context.Context, prefix string) ([]string, error) {
+ c.mu.Lock()
+ c.lists[prefix]++
+ c.mu.Unlock()
+ return c.Store.List(ctx, prefix)
+}
+
+func (c *countingStore) Put(ctx context.Context, key string, body []byte, ifMatch string) (string, error) {
+ c.mu.Lock()
+ c.puts[key]++
+ c.mu.Unlock()
+ return c.Store.Put(ctx, key, body, ifMatch)
+}
+
+func (c *countingStore) n(m map[string]int, k string) int {
+ c.mu.Lock()
+ defer c.mu.Unlock()
+ return m[k]
+}
+
+// syncInstanceKit is the unit-path fixture these tests share: a store, a
+// registry, an env dir and a parsed key pair.
+func syncInstanceKit(t *testing.T) (context.Context, release.Store, *registry.Registry, string, *ecdh.PrivateKey) {
+ t.Helper()
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ reg := testRegistry(t)
+ priv, _, err := GenerateKey()
+ if err != nil {
+ t.Fatalf("GenerateKey: %v", err)
+ }
+ privKey, err := ParsePrivateKey(priv, "test")
+ if err != nil {
+ t.Fatalf("ParsePrivateKey: %v", err)
+ }
+ return ctx, s, reg, t.TempDir(), privKey
+}
+
+func unitRoute(t *testing.T, reg *registry.Registry, host, app, acct, env string) {
+ t.Helper()
+ if err := reg.Upsert(registry.Route{
+ Host: host, Kind: "instance", App: app, Account: acct, Status: "ready",
+ Environment: env, Backing: registry.BackingUnit,
+ }); err != nil {
+ t.Fatalf("Upsert %s: %v", host, err)
+ }
+}
+
+// The delivery this layer exists for: one instance of an app gets a value
+// its siblings never see. The sibling's file is the app merge, untouched.
+func TestSyncRendersTheInstanceOverlayForItsOwnRouteOnly(t *testing.T) {
+ ctx, s, reg, envDir, priv := syncInstanceKit(t)
+ if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"APP_WIDE": "yes", "TENANT": "none"}); err != nil {
+ t.Fatalf("SetEnv: %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)
+ }
+ unitRoute(t, reg, "book.example.com", "titogo", "bab", "")
+ unitRoute(t, reg, "cart.example.com", "titogo", "bab", "")
+
+ if err := Sync(ctx, reg, s, priv, envDir, func(string) error { return nil }, nil); err != nil {
+ t.Fatalf("Sync: %v", err)
+ }
+ book, err := os.ReadFile(filepath.Join(envDir, "book.example.com.env"))
+ if err != nil {
+ t.Fatalf("read book env: %v", err)
+ }
+ if !bytes.Contains(book, []byte("TENANT=book\n")) || !bytes.Contains(book, []byte("APP_WIDE=yes\n")) {
+ t.Fatalf("the overlaid route's file is wrong:\n%s", book)
+ }
+ cart, err := os.ReadFile(filepath.Join(envDir, "cart.example.com.env"))
+ if err != nil {
+ t.Fatalf("read cart env: %v", err)
+ }
+ if bytes.Contains(cart, []byte("book")) || !bytes.Contains(cart, []byte("TENANT=none\n")) {
+ t.Fatalf("a sibling saw another instance's config:\n%s", cart)
+ }
+
+ // The sweep and the single-shot API must agree about what a host gets.
+ // The sweeps resolve through the group cache and never call
+ // LoadMergedHost, so this is the only thing holding the two together.
+ want, _, err := LoadMergedHost(ctx, s, "bab", "titogo", "", "book.example.com")
+ if err != nil {
+ t.Fatalf("LoadMergedHost: %v", err)
+ }
+ rendered, err := RenderEnv(want, priv)
+ if err != nil {
+ t.Fatalf("RenderEnv: %v", err)
+ }
+ if !bytes.Contains(book, rendered) {
+ t.Fatalf("the pass and LoadMergedHost disagree:\npass:\n%s\nLoadMergedHost:\n%s", book, rendered)
+ }
+}
+
+// The cost. One listing per app, a Get for the instance that has an
+// overlay, and NOTHING for the instance that does not.
+func TestSyncCostsOneListingPerAppAndOneGetPerOverlay(t *testing.T) {
+ ctx, base, reg, envDir, priv := syncInstanceKit(t)
+ if err := SetEnv(ctx, base, "bab", "titogo", map[string]string{"APP_WIDE": "yes"}); err != nil {
+ t.Fatalf("SetEnv: %v", err)
+ }
+ if _, _, err := ApplyInstance(ctx, base, "bab", "titogo", "book.example.com",
+ map[string]Var{"TENANT": {Kind: "env", Value: "book"}}, nil); err != nil {
+ t.Fatalf("ApplyInstance: %v", err)
+ }
+ unitRoute(t, reg, "book.example.com", "titogo", "bab", "")
+ unitRoute(t, reg, "cart.example.com", "titogo", "bab", "")
+ unitRoute(t, reg, "desk.example.com", "titogo", "bab", "")
+
+ s := newCountingStore(base)
+ if err := Sync(ctx, reg, s, priv, envDir, func(string) error { return nil }, nil); err != nil {
+ t.Fatalf("Sync: %v", err)
+ }
+ if got := s.n(s.lists, InstancesConfigPrefix("bab", "titogo")); got != 1 {
+ t.Fatalf("instance overlay listings = %d, want exactly 1 for three routes of one app", got)
+ }
+ if got := s.n(s.gets, InstanceConfigKey("bab", "titogo", "book.example.com")); got != 1 {
+ t.Fatalf("overlay Gets for book = %d, want 1", got)
+ }
+ for _, host := range []string{"cart.example.com", "desk.example.com"} {
+ if got := s.n(s.gets, InstanceConfigKey("bab", "titogo", host)); got != 0 {
+ t.Fatalf("overlay Gets for %s = %d, want none: the listing already said it has no overlay", host, got)
+ }
+ }
+}
+
+// A listing that fails is not a determination. The route keeps its file and
+// the pass says so, the same contract a bundle fetch failure already has.
+func TestSyncLeavesTheFileAloneWhenTheOverlayListingFails(t *testing.T) {
+ ctx, base, reg, envDir, priv := syncInstanceKit(t)
+ if err := SetEnv(ctx, base, "bab", "titogo", map[string]string{"APP_WIDE": "yes"}); err != nil {
+ t.Fatalf("SetEnv: %v", err)
+ }
+ unitRoute(t, reg, "book.example.com", "titogo", "bab", "")
+ path := filepath.Join(envDir, "book.example.com.env")
+ if err := os.WriteFile(path, []byte("APP_WIDE=stale\n"), 0o600); err != nil {
+ t.Fatalf("stage file: %v", err)
+ }
+ failing := &failingListStore{Store: base, failPrefix: InstancesConfigPrefix("bab", "titogo"),
+ err: errors.New("s3: transient 503 SlowDown")}
+ out := captureStdout(t, func() {
+ if err := Sync(ctx, reg, failing, priv, envDir, func(string) error { return nil }, nil); err != nil {
+ t.Fatalf("Sync: %v", err)
+ }
+ })
+ cur, err := os.ReadFile(path)
+ if err != nil || string(cur) != "APP_WIDE=stale\n" {
+ t.Fatalf("the file moved on a listing failure: %q %v", cur, err)
+ }
+ if !strings.Contains(out, "book.example.com") || !strings.Contains(out, "left as-is") {
+ t.Fatalf("the pass was silent about an unresolved instance layer:\n%s", out)
+ }
+}
+
+// failingListStore makes List fail for exactly one prefix, the listing
+// shape of failingKeyStore (syncexec_test.go).
+type failingListStore struct {
+ release.Store
+ failPrefix string
+ err error
+}
+
+func (f *failingListStore) List(ctx context.Context, prefix string) ([]string, error) {
+ if prefix == f.failPrefix {
+ return nil, f.err
+ }
+ return f.Store.List(ctx, prefix)
+}
diff --git a/internal/appconfig/sync.go b/internal/appconfig/sync.go
index 0fe6d4d6c..830a71566 100644
--- a/internal/appconfig/sync.go
+++ b/internal/appconfig/sync.go
@@ -140,11 +140,21 @@ func Sync(ctx context.Context, reg *registry.Registry, s release.Store, priv *ec
}
resolved[k] = resolution{m: m, defaultMissing: !def.found}
}
+ // The instance layer (senders design §6.2), resolved per route AFTER the
+ // group base above. The group is still (Account, App, Environment) and
+ // the default bundle is still read once per app: only the third layer is
+ // per host, and it costs one listing per app plus a Get for each
+ // instance that actually has an overlay.
+ 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{}
for _, rt := range routes {
if !syncable(rt) {
continue
}
- r := resolved[groupKey{release.RouteAccount(rt), rt.App, rt.Environment}]
+ k := groupKey{release.RouteAccount(rt), rt.App, rt.Environment}
+ r := resolved[k]
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
@@ -160,6 +170,29 @@ func Sync(ctx context.Context, reg *registry.Registry, s release.Store, priv *ec
if m == nil {
continue
}
+ // The instance overlay wins per key over the group's merge. An
+ // unresolved overlay is NOT an absent one: this route keeps its file
+ // and the pass says which route and why, exactly as it does for a
+ // group whose bundle could not be read.
+ inst, _, err := overlayFor(k.acct, k.app, rt.Host)
+ if err != nil {
+ // Said ONCE per (account, app), not once per route. The listing
+ // this failed on is per app, so every route of that app fails
+ // identically and for the same reason; a line each would bury the
+ // cause under its own repetitions on a box with many routes. The
+ // loader already holds the error, so said is only about output.
+ if !said[k] {
+ said[k] = true
+ // rt.Host anchors the line to a real route (the first one
+ // this pass happened to reach), so an operator scanning the
+ // log has something to grep for; every other route of this
+ // (account, app, environment) failed for the identical
+ // reason and is silently covered by the same line.
+ fmt.Printf(" %s/%s: instance config unresolved (seen at %s), env files left as-is: %v\n", k.acct, k.app, rt.Host, err)
+ }
+ continue
+ }
+ m = applyInstanceOverlay(k.app, m, inst)
// Only unit-backed routes read /etc/carlos/apps — an exec-backed
// route's env comes from <DataDir>/<host>.env, which the activator
// owns (it also carries CARLOS_ADMIN_TOKEN, so it can't be blindly