amadan

carlos / platform Public

appconfig: write and delete an instance overlay, bumping the app's generation (plan 4, task 2)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul Campbell pushed by claude 7f0aa23c320dc1ce0e25bcda9eeb7408e88828c0 parent 11e20e4
4 files changed, +204 −11
  • internal/appconfig/environments.go +1 −1
  • internal/appconfig/instances.go +70 −0
  • internal/appconfig/instances_test.go +119 −0
  • internal/appconfig/store.go +14 −10
diff --git a/internal/appconfig/environments.go b/internal/appconfig/environments.go
index c5d376921..b3b4a4719 100644
--- a/internal/appconfig/environments.go
+++ b/internal/appconfig/environments.go
@@ -115,7 +115,7 @@ func applyEnvironment(ctx context.Context, s release.Store, acct, app, envName s
if !ValidEnvironmentName(envName) {
return 0, fmt.Errorf("config: invalid environment name %q", envName)
}
- removed, err := applyAt(ctx, s, EnvironmentKey(acct, app, envName), app, envName, sets, unsets)
+ removed, _, err := applyAt(ctx, s, EnvironmentKey(acct, app, envName), app, envName, sets, unsets)
if err != nil {
return 0, err
}
diff --git a/internal/appconfig/instances.go b/internal/appconfig/instances.go
index e6a43d332..429a7e830 100644
--- a/internal/appconfig/instances.go
+++ b/internal/appconfig/instances.go
@@ -100,3 +100,73 @@ func LoadInstanceConfig(ctx context.Context, s release.Store, acct, app, host st
}
return getIfExistsAtETag(ctx, s, InstanceConfigKey(acct, app, host))
}
+
+// ApplyInstance writes sets and removes unsets from ONE instance's overlay
+// in one CAS write, returning how many of unsets were actually present and
+// THE ETAG IT WROTE AT.
+//
+// That etag is the contract with the box. The console keeps it, the agent
+// reports the etag it rendered in its config ack, and the two are compared
+// (design §6.5, InstanceConfigDelivery). A map of delivered key names would
+// not do: it cannot tell an agent too old to render this layer apart from a
+// new agent rendering a route whose overlay sets nothing it recognises.
+//
+// There is no CLI verb and no console door onto this function, by design
+// (§6.2, "only the platform writes this layer"). It exists so a credential
+// can be scoped to one instance, and the only caller that scopes one is the
+// email walk in step 5.
+//
+// Unset-only against an overlay that does not exist is refused, not turned
+// into a create, for the reason applyAt's own doc comment gives: an empty
+// manifest that EXISTS is a state the sweeps converge a file to, and
+// minting one to hold nothing would deliver an empty file to a live
+// instance.
+func ApplyInstance(ctx context.Context, s release.Store, acct, app, host string, sets map[string]Var, unsets []string) (int, string, error) {
+ if !release.ValidAccount(acct) {
+ return 0, "", fmt.Errorf("config: invalid account %q", acct)
+ }
+ if !release.ValidApp(app) {
+ return 0, "", fmt.Errorf("config: invalid app %q", app)
+ }
+ if !ValidInstanceHost(host) {
+ return 0, "", fmt.Errorf("config: invalid instance host %q", host)
+ }
+ removed, etag, err := applyAt(ctx, s, InstanceConfigKey(acct, app, host), app, "instance "+host, sets, unsets)
+ if err != nil {
+ return 0, "", err
+ }
+ // The generation object is the ONLY signal this layer has.
+ // ConfigRouteFingerprints (routefingerprint.go) digests registry columns
+ // and cannot see a bucket-side change, so an overlay written without
+ // this touch would wait out configSyncLoop's hourly floor. Same
+ // best-effort demotion every other write here makes: the manifest has
+ // landed, so a failed touch is logged, never returned.
+ touchLogged(ctx, s, acct, app)
+ return removed, etag, nil
+}
+
+// DeleteInstanceConfig removes one instance's overlay outright, which
+// converges that route back to its group's merge on the next pass. Absent
+// is success: a retried teardown must converge rather than fail, and
+// release.Store.Delete is already idempotent.
+//
+// The generation is touched even when nothing was removed. Branching on it
+// would mean a Get before every delete to find out, and one redundant touch
+// costs a tiny object write where a missed one costs an hour of a live
+// instance holding config nothing meant it to have.
+func DeleteInstanceConfig(ctx context.Context, s release.Store, acct, app, host string) error {
+ if !release.ValidAccount(acct) {
+ return fmt.Errorf("config: invalid account %q", acct)
+ }
+ if !release.ValidApp(app) {
+ return fmt.Errorf("config: invalid app %q", app)
+ }
+ if !ValidInstanceHost(host) {
+ return fmt.Errorf("config: invalid instance host %q", host)
+ }
+ if err := s.Delete(ctx, InstanceConfigKey(acct, app, host)); err != nil {
+ return err
+ }
+ touchLogged(ctx, s, acct, app)
+ return nil
+}
diff --git a/internal/appconfig/instances_test.go b/internal/appconfig/instances_test.go
index 3175460e9..e6e51a3e9 100644
--- a/internal/appconfig/instances_test.go
+++ b/internal/appconfig/instances_test.go
@@ -103,3 +103,122 @@ func TestLoadInstanceConfigReadsTheLayerAndItsETag(t *testing.T) {
t.Fatal("LoadInstanceConfig accepted a traversal host")
}
}
+
+// The write door. It returns the etag it wrote at, which is the whole
+// mechanism behind the delivery check (design §6.5): the caller keeps that
+// string and compares it with what an agent's ack says it rendered.
+func TestApplyInstanceWritesTheLayerAndReturnsItsETag(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+
+ removed, etag, err := ApplyInstance(ctx, s, "bab", "titogo", "book.customer.example.com",
+ map[string]Var{"TENANT_SMTP_USER": {Kind: "env", Value: "book"}}, nil)
+ if err != nil {
+ t.Fatalf("ApplyInstance: %v", err)
+ }
+ if removed != 0 {
+ t.Fatalf("removed = %d on a create, want 0", removed)
+ }
+ if etag == "" {
+ t.Fatal("ApplyInstance returned an empty etag, so nothing can check delivery against it")
+ }
+ m, readETag, found, err := LoadInstanceConfig(ctx, s, "bab", "titogo", "book.customer.example.com")
+ if err != nil || !found {
+ t.Fatalf("LoadInstanceConfig after the write = %+v %v %v", m, found, err)
+ }
+ if readETag != etag {
+ t.Fatalf("the reader saw etag %q, the writer returned %q — the delivery check compares these two", readETag, etag)
+ }
+ if m.Schema != 1 || m.App != "titogo" || m.Vars["TENANT_SMTP_USER"].Value != "book" {
+ t.Fatalf("the written overlay is not a schema-1 manifest of this app: %+v", m)
+ }
+
+ // A second write moves the etag, which is what makes a stale ack
+ // detectable at all.
+ _, etag2, err := ApplyInstance(ctx, s, "bab", "titogo", "book.customer.example.com",
+ map[string]Var{"TENANT_SMTP_USER": {Kind: "env", Value: "book2"}}, nil)
+ if err != nil {
+ t.Fatalf("ApplyInstance (second): %v", err)
+ }
+ if etag2 == etag {
+ t.Fatalf("etag did not move on a changed write: %q", etag2)
+ }
+
+ // Unset reports how many of the keys were actually there, the same
+ // contract every other bundle's unset has.
+ removed, _, err = ApplyInstance(ctx, s, "bab", "titogo", "book.customer.example.com", nil,
+ []string{"TENANT_SMTP_USER", "NEVER_SET"})
+ if err != nil || removed != 1 {
+ t.Fatalf("unset = %d %v, want 1 and no error", removed, err)
+ }
+
+ // The var-name and var-value rules are the bundle's, not this layer's:
+ // applyAt refuses before anything reaches the bucket.
+ if _, _, err := ApplyInstance(ctx, s, "bab", "titogo", "book.customer.example.com",
+ map[string]Var{"not a name": {Kind: "env", Value: "x"}}, nil); err == nil {
+ t.Fatal("an invalid var name was accepted into the instance layer")
+ }
+ if _, _, err := ApplyInstance(ctx, s, "bab", "titogo", "../config", nil, nil); err == nil {
+ t.Fatal("ApplyInstance accepted a traversal host")
+ }
+}
+
+// An unset against an overlay that does not exist is refused rather than
+// minting an empty one, the same refusal applyAt already makes for the
+// other two bundles and for the same reason: an empty manifest that EXISTS
+// means "configured, then fully unset" by this package's contract, which is
+// a state the sweeps converge a file to.
+func TestApplyInstanceRefusesUnsetOnlyAgainstAMissingOverlay(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ _, _, err := ApplyInstance(ctx, s, "bab", "titogo", "book.customer.example.com", nil, []string{"K"})
+ if err == nil {
+ t.Fatal("unsetting from a nonexistent overlay was turned into a create")
+ }
+ if !strings.Contains(err.Error(), "book.customer.example.com") {
+ t.Fatalf("the refusal does not name the instance: %v", err)
+ }
+}
+
+// Writing the layer bumps the app's generation object, the way
+// SetExecDeliveryAt does. It has to: ConfigRouteFingerprints digests
+// registry columns only (routefingerprint.go:58) and cannot see a
+// bucket-side change, so without the bump an overlay would wait out the
+// agent's hourly floor with nothing to signal it.
+func TestWritingTheInstanceLayerBumpsTheGeneration(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"LOG_LEVEL": "debug"}); err != nil {
+ t.Fatalf("SetEnv: %v", err)
+ }
+ genETag := func() string {
+ t.Helper()
+ _, etag, err := s.Get(ctx, GenKey("bab", "titogo"))
+ if err != nil {
+ t.Fatalf("read generation: %v", err)
+ }
+ return etag
+ }
+ before := genETag()
+ if _, _, err := ApplyInstance(ctx, s, "bab", "titogo", "book.customer.example.com",
+ map[string]Var{"TENANT_SMTP_USER": {Kind: "env", Value: "book"}}, nil); err != nil {
+ t.Fatalf("ApplyInstance: %v", err)
+ }
+ afterWrite := genETag()
+ if afterWrite == before {
+ t.Fatal("writing the instance layer did not bump the generation, so the fast loop would never see it")
+ }
+ if err := DeleteInstanceConfig(ctx, s, "bab", "titogo", "book.customer.example.com"); err != nil {
+ t.Fatalf("DeleteInstanceConfig: %v", err)
+ }
+ if genETag() == afterWrite {
+ t.Fatal("removing the instance layer did not bump the generation, so the route would keep its last merge")
+ }
+ if _, _, found, err := LoadInstanceConfig(ctx, s, "bab", "titogo", "book.customer.example.com"); err != nil || found {
+ t.Fatalf("the overlay survived its delete: found=%v err=%v", found, err)
+ }
+ // Deleting again is a no-op success, so a retried teardown converges.
+ if err := DeleteInstanceConfig(ctx, s, "bab", "titogo", "book.customer.example.com"); err != nil {
+ t.Fatalf("second DeleteInstanceConfig: %v", err)
+ }
+}
diff --git a/internal/appconfig/store.go b/internal/appconfig/store.go
index 815c99506..b78f5597d 100644
--- a/internal/appconfig/store.go
+++ b/internal/appconfig/store.go
@@ -130,7 +130,7 @@ func apply(ctx context.Context, s release.Store, acct, app, bundle string, sets
if !release.ValidApp(app) {
return 0, fmt.Errorf("config: invalid app %q", app)
}
- removed, err := applyAt(ctx, s, Key(acct, app), app, bundle, sets, unsets)
+ removed, _, err := applyAt(ctx, s, Key(acct, app), app, bundle, sets, unsets)
if err != nil {
return 0, err
}
@@ -154,10 +154,10 @@ func apply(ctx context.Context, s release.Store, acct, app, bundle string, sets
// environment bundles that then satisfied `carlos route -environment`'s
// existence rail. Sets still create, which is how a bundle is meant to come
// into being.
-func applyAt(ctx context.Context, s release.Store, key, app, bundle string, sets map[string]Var, unsets []string) (int, error) {
+func applyAt(ctx context.Context, s release.Store, key, app, bundle string, sets map[string]Var, unsets []string) (int, string, error) {
for k, v := range sets {
if !ValidVarName(k) {
- return 0, fmt.Errorf("config: invalid var name %q", k)
+ return 0, "", fmt.Errorf("config: invalid var name %q", k)
}
// Env values are validated here, while the operator can still see
// the plaintext — a bad value that only failed at render time would
@@ -166,16 +166,16 @@ func applyAt(ctx context.Context, s release.Store, key, app, bundle string, sets
// CLI, before sealing; by this point they're opaque envelopes.
if v.Kind == "env" {
if err := ValidVarValue(v.Value); err != nil {
- return 0, fmt.Errorf("config: var %s: %w", k, err)
+ return 0, "", fmt.Errorf("config: var %s: %w", k, err)
}
}
}
m, etag, found, err := getForWriteAt(ctx, s, key, app)
if err != nil {
- return 0, err
+ return 0, "", err
}
if !found && len(sets) == 0 {
- return 0, fmt.Errorf("config: %s has no %s bundle — nothing to unset", app, bundle)
+ return 0, "", fmt.Errorf("config: %s has no %s bundle — nothing to unset", app, bundle)
}
if m.Vars == nil {
m.Vars = map[string]Var{}
@@ -193,12 +193,16 @@ func applyAt(ctx context.Context, s release.Store, key, app, bundle string, sets
m.Schema, m.App = 1, app
raw, err := json.MarshalIndent(m, "", " ")
if err != nil {
- return 0, err
+ return 0, "", err
}
- if _, err := s.Put(ctx, key, raw, etag); err != nil {
- return 0, err
+ // The new etag is returned because the instance layer's writer (design
+ // §6.5) has to hand it to whoever will later compare it with an agent's
+ // delivery ack. The other two bundles' callers ignore it.
+ newETag, err := s.Put(ctx, key, raw, etag)
+ if err != nil {
+ return 0, "", err
}
- return removed, nil
+ return removed, newETag, nil
}
// DefaultBundle is how the default bundle (config.json) is named to a human