amadan
appconfig: the instance config layer's key, host rule and etag-returning reader (plan 4, task 1)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3 files changed,
+227
−9
internal/appconfig/instances.go+102 −0internal/appconfig/instances_test.go+105 −0internal/appconfig/store.go+20 −9
diff --git a/internal/appconfig/instances.go b/internal/appconfig/instances.go| new file mode 100644 |
| index 000000000..e6a43d332 |
| --- /dev/null |
| +++ b/internal/appconfig/instances.go |
| @@ -0,0 +1,102 @@ |
| +// The instance config layer (tenant-email senders design §6.2): a third |
| +// bundle, one object per route host, resolved after the named environment. |
| +// Same Manifest shape, same sealed envelopes, same name and value rules as |
| +// the other two — a per-instance bundle is not a new format, just a third |
| +// bucket key the same shape is written to. |
| +// |
| +// Bucket layout addition: |
| +// |
| +// apps/<acct>/<app>/instances-config/<host>.json CAS-guarded, schema-1 manifest |
| +// |
| +// It exists so a credential can be scoped to ONE instance. #148's shape is |
| +// one instance per customer under a single app, and everything the platform |
| +// delivers today lands in a bundle every instance of the app renders, so a |
| +// per-customer secret delivered through the default bundle reaches every |
| +// other customer's process. |
| +// |
| +// Manifest.ExecDelivery is NOT read from this layer. That is the ruling at |
| +// appconfig.go's ExecDelivery doc comment, unchanged: a per-route delivery |
| +// flag would need a registry column, a record field and reconcile |
| +// convergence, and two routes resolving through the same bundle could |
| +// disagree about whether they are delivered. A per-host bundle is exactly |
| +// the granularity that ruling refused, so resolveExecDelivery is never |
| +// handed one of these manifests and a field that somehow appears in one is |
| +// inert. |
| +package appconfig |
| + |
| +import ( |
| + "context" |
| + "fmt" |
| + "regexp" |
| + "strings" |
| + |
| + "github.com/carlosframework/platform/internal/release" |
| +) |
| + |
| +// InstancesConfigPrefix is the list prefix behind "which instances of this |
| +// app have an overlay" — every instances-config/<host>.json key shares it. |
| +// Inside apps/<acct>/<app>/ on purpose: release.PurgeApp's prefix sweep |
| +// reaches it, so a purged app cannot leave an object full of sealed |
| +// secrets behind. |
| +func InstancesConfigPrefix(acct, app string) string { |
| + return release.AppPrefix(acct, app) + "instances-config/" |
| +} |
| + |
| +// InstanceConfigKey is the bucket key of one instance's overlay. host is a |
| +// route's Host column verbatim, which is what makes the key derivable from |
| +// a registry row with nothing to look up. |
| +func InstanceConfigKey(acct, app, host string) string { |
| + return InstancesConfigPrefix(acct, app) + host + ".json" |
| +} |
| + |
| +// instanceHostRe is a bare lowercase hostname: labels of letters, digits |
| +// and hyphens, joined by dots, never starting or ending with a hyphen or a |
| +// dot. Case-restricted like ValidEnvironmentName and for the same reason, |
| +// with one more: this string is also the file name sync derives on the box |
| +// (sync.go's filepath.Base guard), so one convention keeps a bucket key and |
| +// a file name from disagreeing about which host they belong to. |
| +var instanceHostRe = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)*$`) |
| + |
| +// ValidInstanceHost reports whether host is usable as an instance overlay |
| +// key. A separator or a traversal is refused at the one gate every caller |
| +// passes through, because this string is concatenated into a bucket key: a |
| +// host of "../config" would otherwise name the app's default bundle. |
| +func ValidInstanceHost(host string) bool { |
| + if host == "" || len(host) > 253 || strings.Contains(host, "..") { |
| + return false |
| + } |
| + // Per-label bound as well as the whole-name one, matching DNS and |
| + // launchctx/controlname's label rule. Without it a single 200-character |
| + // label passes here and is refused everywhere else that handles the same |
| + // host, which is a disagreement between two gates rather than one rule. |
| + for _, label := range strings.Split(host, ".") { |
| + if len(label) > 63 { |
| + return false |
| + } |
| + } |
| + return instanceHostRe.MatchString(host) |
| +} |
| + |
| +// LoadInstanceConfig reads one instance's overlay, returning the manifest, |
| +// the etag it was read at, and whether the object exists at all. |
| +// |
| +// The etag is the point of this signature. The delivery ack (design §6.5) |
| +// carries the etag the agent actually rendered, and the console compares it |
| +// with the one its own Put returned: that comparison is decidable, where a |
| +// map of delivered key names cannot tell an old agent apart from a new one |
| +// rendering a route with no overlay vars. |
| +// |
| +// Not-found is (nil, "", false, nil): an instance with no overlay is the |
| +// normal case and the state every instance is in until something writes one. |
| +func LoadInstanceConfig(ctx context.Context, s release.Store, acct, app, host string) (*Manifest, string, bool, error) { |
| + if !release.ValidAccount(acct) { |
| + return nil, "", false, fmt.Errorf("config: invalid account %q", acct) |
| + } |
| + if !release.ValidApp(app) { |
| + return nil, "", false, fmt.Errorf("config: invalid app %q", app) |
| + } |
| + if !ValidInstanceHost(host) { |
| + return nil, "", false, fmt.Errorf("config: invalid instance host %q", host) |
| + } |
| + return getIfExistsAtETag(ctx, s, InstanceConfigKey(acct, app, host)) |
| +} |
diff --git a/internal/appconfig/instances_test.go b/internal/appconfig/instances_test.go| new file mode 100644 |
| index 000000000..3175460e9 |
| --- /dev/null |
| +++ b/internal/appconfig/instances_test.go |
| @@ -0,0 +1,105 @@ |
| +package appconfig |
| + |
| +import ( |
| + "context" |
| + "encoding/json" |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/release" |
| +) |
| + |
| +// putInstanceRaw writes an instance-layer object without going through the |
| +// package's own writer, which does not exist until task 2. Tests in this |
| +// file that only READ the layer use it, so task 1 can be verified on its |
| +// own. |
| +func putInstanceRaw(t *testing.T, ctx context.Context, s release.Store, acct, app, host string, m *Manifest) string { |
| + t.Helper() |
| + body, err := json.MarshalIndent(m, "", " ") |
| + if err != nil { |
| + t.Fatalf("marshal instance manifest: %v", err) |
| + } |
| + etag, err := s.Put(ctx, InstanceConfigKey(acct, app, host), body, "") |
| + if err != nil { |
| + t.Fatalf("put instance manifest: %v", err) |
| + } |
| + return etag |
| +} |
| + |
| +// The layer sits under the app's own prefix, beside config.json and |
| +// environments/, so release.PurgeApp's prefix sweep reaches it and a purged |
| +// app leaves no instance overlay behind holding sealed secrets. |
| +func TestInstanceConfigKeysSitUnderTheAppsOwnPrefix(t *testing.T) { |
| + if got, want := InstancesConfigPrefix("bab", "titogo"), "apps/bab/titogo/instances-config/"; got != want { |
| + t.Fatalf("InstancesConfigPrefix = %q, want %q", got, want) |
| + } |
| + if got, want := InstanceConfigKey("bab", "titogo", "book.customer.example.com"), |
| + "apps/bab/titogo/instances-config/book.customer.example.com.json"; got != want { |
| + t.Fatalf("InstanceConfigKey = %q, want %q", got, want) |
| + } |
| +} |
| + |
| +// A host becomes a bucket key segment here, so the one gate every caller |
| +// passes through is where a separator, a traversal or an empty name is |
| +// refused. The shape is a bare lowercase hostname, the same thing a route |
| +// row's Host column holds and the same thing sync.go's own path guard |
| +// (rt.Host != filepath.Base(rt.Host)) already insists on before it derives |
| +// a file name from one. |
| +func TestValidInstanceHostRefusesAnythingThatIsNotABareHost(t *testing.T) { |
| + for _, bad := range []string{ |
| + "", ".", "..", "a/b", "../etc", "Book.example.com", "book..example.com", |
| + "-lead.example.com", "trail-.example.com", "book.example.com/", |
| + strings.Repeat("a", 254), |
| + } { |
| + if ValidInstanceHost(bad) { |
| + t.Errorf("ValidInstanceHost(%q) = true", bad) |
| + } |
| + } |
| + for _, good := range []string{"book.customer.example.com", "inst1", "a-b.c-d.example.com", "127-0-0-1.example.com"} { |
| + if !ValidInstanceHost(good) { |
| + t.Errorf("ValidInstanceHost(%q) = false", good) |
| + } |
| + } |
| +} |
| + |
| +// The reader answers three ways, and the caller needs all three apart: a |
| +// manifest and the etag it was read at, "no object" without an error (an |
| +// instance with no overlay is the normal case, not a fault), and an error |
| +// for anything else. The etag is what the delivery ack carries (design §6.5), |
| +// so a read that dropped it would leave the agent unable to say which |
| +// version of the layer it rendered. |
| +func TestLoadInstanceConfigReadsTheLayerAndItsETag(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + |
| + m, etag, found, err := LoadInstanceConfig(ctx, s, "bab", "titogo", "book.customer.example.com") |
| + if err != nil || found || m != nil || etag != "" { |
| + t.Fatalf("absent overlay = %+v %q %v %v, want (nil, \"\", false, nil)", m, etag, found, err) |
| + } |
| + |
| + want := putInstanceRaw(t, ctx, s, "bab", "titogo", "book.customer.example.com", |
| + &Manifest{Schema: 1, App: "titogo", Vars: map[string]Var{"TENANT_SMTP_USER": {Kind: "env", Value: "book"}}}) |
| + m, etag, found, err = LoadInstanceConfig(ctx, s, "bab", "titogo", "book.customer.example.com") |
| + if err != nil || !found || m == nil { |
| + t.Fatalf("present overlay = %+v %v %v", m, found, err) |
| + } |
| + if etag == "" || etag != want { |
| + t.Fatalf("etag = %q, want the object's own %q", etag, want) |
| + } |
| + if m.Vars["TENANT_SMTP_USER"].Value != "book" { |
| + t.Fatalf("overlay lost its var: %+v", m.Vars) |
| + } |
| + |
| + // The same newer-schema refusal every other bundle read makes: acting on |
| + // a manifest whose unknown fields json.Unmarshal has already dropped |
| + // would silently discard whatever they meant. |
| + putInstanceRaw(t, ctx, s, "bab", "titogo", "newer.example.com", &Manifest{Schema: 2, App: "titogo"}) |
| + if _, _, _, err := LoadInstanceConfig(ctx, s, "bab", "titogo", "newer.example.com"); err == nil { |
| + t.Fatal("a schema-2 overlay was accepted, want a refusal naming the upgrade") |
| + } |
| + |
| + // A host that is not a bare hostname never reaches the bucket at all. |
| + if _, _, _, err := LoadInstanceConfig(ctx, s, "bab", "titogo", "../config"); err == nil { |
| + t.Fatal("LoadInstanceConfig accepted a traversal host") |
| + } |
| +} |
diff --git a/internal/appconfig/store.go b/internal/appconfig/store.go| index de97f2217..815c99506 100644 |
| --- a/internal/appconfig/store.go |
| +++ b/internal/appconfig/store.go |
| @@ -48,28 +48,39 @@ func getIfExists(ctx context.Context, s release.Store, acct, app string) (*Manif |
| // getIfExistsAt is getIfExists generalized to any manifest key — Key(acct, |
| // app) for the default bundle, EnvironmentKey(acct, app, name) for a named |
| -// one (see environments.go). Same shape either way: a config.json and a |
| -// named environment bundle are the identical manifest format at sibling |
| -// bucket paths, so one read path serves both. |
| +// one (see environments.go), InstanceConfigKey(acct, app, host) for an |
| +// instance overlay (instances.go). Same shape either way: all three are the |
| +// identical manifest format at sibling bucket paths, so one read path |
| +// serves them. |
| func getIfExistsAt(ctx context.Context, s release.Store, key string) (*Manifest, bool, error) { |
| - raw, _, err := s.Get(ctx, key) |
| + m, _, found, err := getIfExistsAtETag(ctx, s, key) |
| + return m, found, err |
| +} |
| + |
| +// getIfExistsAtETag is getIfExistsAt plus the object's etag, which the |
| +// instance layer's reader needs and the other two bundles' readers do not: |
| +// the delivery ack names the etag of the overlay the agent rendered |
| +// (senders design §6.5), and an etag dropped here could not be recovered |
| +// later without a second read racing the first. |
| +func getIfExistsAtETag(ctx context.Context, s release.Store, key string) (*Manifest, string, bool, error) { |
| + raw, etag, err := s.Get(ctx, key) |
| if err != nil { |
| if errors.Is(err, os.ErrNotExist) { |
| - return nil, false, nil |
| + return nil, "", false, nil |
| } |
| - return nil, false, err |
| + return nil, "", false, err |
| } |
| var m Manifest |
| if err := json.Unmarshal(raw, &m); err != nil { |
| - return nil, false, fmt.Errorf("config %s: %w", key, err) |
| + return nil, "", false, fmt.Errorf("config %s: %w", key, err) |
| } |
| // json.Unmarshal has already dropped any fields this binary doesn't |
| // know about, so acting on a newer-schema manifest would silently |
| // discard whatever those fields meant. Refuse instead. |
| if m.Schema > 1 { |
| - return nil, false, fmt.Errorf("config %s: schema %d is newer than this binary understands — upgrade carlos", key, m.Schema) |
| + return nil, "", false, fmt.Errorf("config %s: schema %d is newer than this binary understands — upgrade carlos", key, m.Schema) |
| } |
| - return &m, true, nil |
| + return &m, etag, true, nil |
| } |
| // getForWriteAt is Get plus the two bits a caller needs to write back: the |