amadan

carlos / platform Public

appconfig: LoadMergedHost resolves default < environment < instance (plan 4, task 3)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul Campbell pushed by claude e3c571371700ec9b27963e4ffedba906e1aa14e4 parent 7f0aa23
2 files changed, +243 −0
  • internal/appconfig/instances.go +91 −0
  • internal/appconfig/instances_test.go +152 −0
diff --git a/internal/appconfig/instances.go b/internal/appconfig/instances.go
index 429a7e830..4c04ed307 100644
--- a/internal/appconfig/instances.go
+++ b/internal/appconfig/instances.go
@@ -170,3 +170,94 @@ func DeleteInstanceConfig(ctx context.Context, s release.Store, acct, app, host
touchLogged(ctx, s, acct, app)
return nil
}
+
+// LoadMergedHost is LoadMerged plus the instance layer: default <
+// environment < instance, the instance winning per key, and the overlay's
+// etag returned alongside so a caller can check delivery against it later
+// (design §6.2, §6.5). An empty etag means this host has no overlay, which
+// is a different statement from an agent that never rendered one.
+//
+// LoadMerged keeps its signature and its meaning. Everything that asks
+// "what does this APP's environment resolve to" still gets two layers, and
+// only a caller that names a host gets three.
+//
+// The two sync sweeps do NOT call this function, deliberately: they resolve
+// one group base for every route that shares it and would re-fetch
+// config.json per route if they came through here. They reach the same
+// answer through instanceOverlays and applyInstanceOverlay instead, and the
+// equivalence is pinned by a test rather than by hope.
+func LoadMergedHost(ctx context.Context, s release.Store, acct, app, envName, host string) (*Manifest, string, error) {
+ if !ValidInstanceHost(host) {
+ return nil, "", fmt.Errorf("config: invalid instance host %q", host)
+ }
+ base, err := LoadMerged(ctx, s, acct, app, envName)
+ if err != nil {
+ return nil, "", err
+ }
+ inst, etag, found, err := LoadInstanceConfig(ctx, s, acct, app, host)
+ if err != nil {
+ return nil, "", err
+ }
+ if !found {
+ return base, "", nil
+ }
+ return applyInstanceOverlay(app, base, inst), etag, nil
+}
+
+// applyInstanceOverlay lays inst over base, per key. base may be nil (the
+// app has no default bundle and the route is unbound), in which case the
+// merged view is the overlay ALONE — which the callers treat as the
+// wholesale replacement it is, see sync.go's defaultMissing guard.
+//
+// A nil overlay returns base untouched, pointer and all: that is the path
+// every instance without an overlay takes, and rebuilding the manifest
+// there would strip exec_delivery off the default bundle object LoadMerged
+// hands back for an unbound route.
+//
+// The merged view never carries exec_delivery FROM the overlay. See this
+// file's own doc comment: a per-host delivery flag is the granularity the
+// #195 ruling refused, and the merged manifest is what gets rendered into a
+// file, where a delivery flag has no business appearing anyway.
+func applyInstanceOverlay(app string, base, inst *Manifest) *Manifest {
+ if inst == nil {
+ return base
+ }
+ merged := &Manifest{Schema: 1, App: app, Vars: map[string]Var{}}
+ if base != nil {
+ for k, v := range base.Vars {
+ merged.Vars[k] = v
+ }
+ }
+ for k, v := range inst.Vars {
+ merged.Vars[k] = v
+ }
+ return merged
+}
+
+// instanceOverlays lists which of this app's instances have an overlay at
+// all: one listing per (account, app), cached by the caller for a whole
+// pass, so a sweep pays one List per app and then one Get per instance that
+// actually has one. Reading every route's key blind would cost a bucket
+// round trip per instance on a box where nothing has an overlay, which is
+// every box in the fleet the day this ships.
+//
+// A key that does not end in .json, or whose host does not survive
+// ValidInstanceHost, is skipped rather than reported: nothing can have
+// written it through this package, so it is not this app's overlay
+// whatever it is.
+func instanceOverlays(ctx context.Context, s release.Store, acct, app string) (map[string]bool, error) {
+ prefix := InstancesConfigPrefix(acct, app)
+ keys, err := s.List(ctx, prefix)
+ if err != nil {
+ return nil, err
+ }
+ out := make(map[string]bool, len(keys))
+ for _, k := range keys {
+ host := strings.TrimSuffix(strings.TrimPrefix(k, prefix), ".json")
+ if host == strings.TrimPrefix(k, prefix) || !ValidInstanceHost(host) {
+ continue
+ }
+ out[host] = true
+ }
+ return out, nil
+}
diff --git a/internal/appconfig/instances_test.go b/internal/appconfig/instances_test.go
index e6e51a3e9..8cbaf7fed 100644
--- a/internal/appconfig/instances_test.go
+++ b/internal/appconfig/instances_test.go
@@ -222,3 +222,155 @@ func TestWritingTheInstanceLayerBumpsTheGeneration(t *testing.T) {
t.Fatalf("second DeleteInstanceConfig: %v", err)
}
}
+
+// The precedence the design states: default < environment < instance
+// (§6.2). Three layers, each winning per key over the one below it, and the
+// instance layer's etag handed back so the caller can later check what was
+// delivered.
+func TestLoadMergedHostResolvesDefaultThenEnvironmentThenInstance(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{
+ "FROM_DEFAULT": "default", "FROM_ENV": "default", "FROM_INSTANCE": "default"}); err != nil {
+ t.Fatalf("SetEnv: %v", err)
+ }
+ if _, err := ApplyEnvironment(ctx, s, "bab", "titogo", "prod", map[string]Var{
+ "FROM_ENV": {Kind: "env", Value: "environment"},
+ "FROM_INSTANCE": {Kind: "env", Value: "environment"}}, nil); err != nil {
+ t.Fatalf("ApplyEnvironment: %v", err)
+ }
+ _, want, err := ApplyInstance(ctx, s, "bab", "titogo", "book.customer.example.com", map[string]Var{
+ "FROM_INSTANCE": {Kind: "env", Value: "instance"}}, nil)
+ if err != nil {
+ t.Fatalf("ApplyInstance: %v", err)
+ }
+
+ m, etag, err := LoadMergedHost(ctx, s, "bab", "titogo", "prod", "book.customer.example.com")
+ if err != nil {
+ t.Fatalf("LoadMergedHost: %v", err)
+ }
+ if etag != want {
+ t.Fatalf("etag = %q, want the overlay's own %q", etag, want)
+ }
+ for k, wantVal := range map[string]string{
+ "FROM_DEFAULT": "default", "FROM_ENV": "environment", "FROM_INSTANCE": "instance"} {
+ if got := m.Vars[k].Value; got != wantVal {
+ t.Errorf("%s = %q, want %q", k, got, wantVal)
+ }
+ }
+
+ // A sibling with no overlay of its own gets the two-layer merge and an
+ // empty etag, which is what says "this host has no overlay" rather than
+ // "this agent did not render one".
+ m2, etag2, err := LoadMergedHost(ctx, s, "bab", "titogo", "prod", "other.customer.example.com")
+ if err != nil {
+ t.Fatalf("LoadMergedHost (sibling): %v", err)
+ }
+ if etag2 != "" {
+ t.Fatalf("a host with no overlay reported etag %q", etag2)
+ }
+ if m2.Vars["FROM_INSTANCE"].Value != "environment" {
+ t.Fatalf("the sibling saw another instance's value: %+v", m2.Vars)
+ }
+}
+
+// With no overlay, LoadMergedHost is LoadMerged, to the field. That matters
+// for the unbound case in particular: LoadMerged returns the default bundle
+// OBJECT there, exec_delivery and all, and a merge that quietly rebuilt it
+// would strip a flag other code reads off the same manifest.
+func TestLoadMergedHostWithNoOverlayIsExactlyLoadMerged(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"K": "v"}); err != nil {
+ t.Fatalf("SetEnv: %v", err)
+ }
+ if err := SetExecDelivery(ctx, s, "bab", "titogo", ExecDeliveryFile); err != nil {
+ t.Fatalf("SetExecDelivery: %v", err)
+ }
+ want, err := LoadMerged(ctx, s, "bab", "titogo", "")
+ if err != nil {
+ t.Fatalf("LoadMerged: %v", err)
+ }
+ got, etag, err := LoadMergedHost(ctx, s, "bab", "titogo", "", "book.customer.example.com")
+ if err != nil || etag != "" {
+ t.Fatalf("LoadMergedHost = %q %v", etag, err)
+ }
+ if got.ExecDelivery != want.ExecDelivery || got.Vars["K"] != want.Vars["K"] || len(got.Vars) != len(want.Vars) {
+ t.Fatalf("LoadMergedHost diverged from LoadMerged with no overlay: %+v vs %+v", got, want)
+ }
+}
+
+// exec_delivery is never read from this layer (the ruling at
+// appconfig.go:66-71). An overlay that carries the field does not put it in
+// the merged view, so nothing downstream can act on it.
+func TestInstanceLayerNeverCarriesExecDelivery(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"K": "v"}); err != nil {
+ t.Fatalf("SetEnv: %v", err)
+ }
+ putInstanceRaw(t, ctx, s, "bab", "titogo", "book.customer.example.com", &Manifest{
+ Schema: 1, App: "titogo", Vars: map[string]Var{"OWN": {Kind: "env", Value: "1"}},
+ ExecDelivery: ExecDeliveryFile})
+ m, _, err := LoadMergedHost(ctx, s, "bab", "titogo", "", "book.customer.example.com")
+ if err != nil {
+ t.Fatalf("LoadMergedHost: %v", err)
+ }
+ if m.ExecDelivery != "" {
+ t.Fatalf("exec_delivery = %q: the instance layer must never decide delivery", m.ExecDelivery)
+ }
+ if m.Vars["OWN"].Value != "1" {
+ t.Fatalf("the overlay's vars were dropped along with its flag: %+v", m.Vars)
+ }
+}
+
+// A route bound to a bundle that does not exist is still an ERROR, overlay
+// or no overlay. The binding asserted the bundle exists; an overlay does not
+// make that assertion true, and answering with a partial merge would hand a
+// live route a file built from a binding nobody honoured.
+func TestLoadMergedHostStillRefusesAMissingBoundBundle(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"K": "v"}); err != nil {
+ t.Fatalf("SetEnv: %v", err)
+ }
+ if _, _, err := ApplyInstance(ctx, s, "bab", "titogo", "book.customer.example.com",
+ map[string]Var{"OWN": {Kind: "env", Value: "1"}}, nil); err != nil {
+ t.Fatalf("ApplyInstance: %v", err)
+ }
+ if _, _, err := LoadMergedHost(ctx, s, "bab", "titogo", "gone", "book.customer.example.com"); err == nil {
+ t.Fatal("a missing bound bundle resolved anyway because the host had an overlay")
+ }
+}
+
+// The index the sweeps use: one listing per app answers "which of this
+// app's instances have an overlay at all", so a pass costs a Get only for
+// the instances that do.
+func TestInstanceOverlaysListsOnlyThisAppsHosts(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ have, err := instanceOverlays(ctx, s, "bab", "titogo")
+ if err != nil || len(have) != 0 {
+ t.Fatalf("empty app = %v %v, want an empty set and no error", have, err)
+ }
+ for _, host := range []string{"book.customer.example.com", "cart.customer.example.com"} {
+ if _, _, err := ApplyInstance(ctx, s, "bab", "titogo", host,
+ map[string]Var{"OWN": {Kind: "env", Value: host}}, nil); err != nil {
+ t.Fatalf("ApplyInstance %s: %v", host, err)
+ }
+ }
+ if _, _, err := ApplyInstance(ctx, s, "bab", "eleven", "other.example.com",
+ map[string]Var{"OWN": {Kind: "env", Value: "x"}}, nil); err != nil {
+ t.Fatalf("ApplyInstance (other app): %v", err)
+ }
+ have, err = instanceOverlays(ctx, s, "bab", "titogo")
+ if err != nil {
+ t.Fatalf("instanceOverlays: %v", err)
+ }
+ if len(have) != 2 || !have["book.customer.example.com"] || !have["cart.customer.example.com"] {
+ t.Fatalf("instanceOverlays = %v, want just this app's two hosts", have)
+ }
+ if have["other.example.com"] {
+ t.Fatal("another app's overlay leaked into this app's index")
+ }
+}