amadan

carlos / platform Public

console: a seeded instance's derived environment must be unused (final fix F6)

seedOwnsStore accepted an absent or declared-only store without asking
who already named the environment. A source created with --environment
canary before its app took private storage, then seeded as
canary.<domain>, gave both instances the same environment and the
seed's new store, which the seed's finish could later tear down.

Both seed doors now refuse when any record of the app other than the
seed's own names the derived environment: in seedStoreRefusal before
the record is reserved, and again in instanceStoreProvisionFor under the
store walk lock an environment binding takes (F3). The listing is
ListRecordsStrict; a listing that fails, is truncated or holds a record
that will not decode refuses with the door's retry sentence. The
refusal is SD-N34 when the environment's store has a bucket, and SD-N33's
rule sentence when it has none (a copy question for Paul).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude e89c897fa2b9255f62763c1ad9e9a77afb4d06de parent 579c158
4 files changed, +187 −12
  • internal/console/apiinstanceimport.go +7 −3
  • internal/console/apiinstanceseed.go +58 −7
  • internal/console/instanceprivatestore.go +20 −2
  • internal/console/seedenvuse_test.go +102 −0
diff --git a/internal/console/apiinstanceimport.go b/internal/console/apiinstanceimport.go
index b97b7d381..00fd90bc5 100644
--- a/internal/console/apiinstanceimport.go
+++ b/internal/console/apiinstanceimport.go
@@ -183,13 +183,17 @@ func (s *Server) beginImport(w http.ResponseWriter, r *http.Request, c seedCreat
// The request's own context, as the ordinary create's walk: the
// walk takes storeGrantTimeout, and a client that leaves cancels a
// begin whose token it can no longer receive. A refusal is held on
- // w already, except errSeedStoreTaken, which is spoken here.
+ // w already, except errSeedStoreTaken and errSeedEnvironmentUnread,
+ // which are spoken here.
storeRec, ok, err := s.instanceStoreProvisionFor(ctx, w, acct, app, rec.Environment, c.principal, id)
if !ok {
s.unbeginImport(ctx, c, id, "store provisioning")
- if errors.Is(err, errSeedStoreTaken) {
- ref := seedStoreTakenRefusal(rec.Environment, storeBucketName(storeRec))
+ switch {
+ case errors.Is(err, errSeedStoreTaken):
+ ref := seedStoreTakenRefusal(rec.Environment, storeRec)
jsonError(w, ref.Status, ref.Msg)
+ case errors.Is(err, errSeedEnvironmentUnread):
+ jsonError(w, http.StatusBadGateway, importFailed)
}
return
}
diff --git a/internal/console/apiinstanceseed.go b/internal/console/apiinstanceseed.go
index d9a79f7bd..e221a246d 100644
--- a/internal/console/apiinstanceseed.go
+++ b/internal/console/apiinstanceseed.go
@@ -101,15 +101,54 @@ func seedPlacementRefusal(c seedCreate) *doorRefusal {
return nil
}
+// seedOwnEnvironment is SD-N33, the rule a seeded instance's environment
+// keeps (spec §4 Storage).
+const seedOwnEnvironment = "a seeded instance gets its own environment and an empty store"
+
// refuseSeededEnvironment is both seed doors' answer to an explicit
// --environment on an app with private storage (spec §4 Storage).
func (s *Server) refuseSeededEnvironment(w http.ResponseWriter) {
- jsonError(w, http.StatusConflict, "a seeded instance gets its own environment and an empty store")
+ jsonError(w, http.StatusConflict, seedOwnEnvironment)
}
-// seedStoreTakenRefusal names the store a seeded create may not use.
-func seedStoreTakenRefusal(env, bucket string) *doorRefusal {
- return &doorRefusal{http.StatusConflict, fmt.Sprintf("environment %s already has a store (%s). A seeded instance needs a new, empty one.", env, bucket)}
+// seedStoreTakenRefusal is the refusal of a seeded create whose environment
+// is not new: SD-N34 naming the store's bucket, or, when the environment
+// has no store with a bucket and is only named by another instance (final
+// review F6), SD-N33's rule.
+func seedStoreTakenRefusal(env string, store *objstore.Record) *doorRefusal {
+ if bucket := storeBucketName(store); bucket != "" {
+ return &doorRefusal{http.StatusConflict, fmt.Sprintf("environment %s already has a store (%s). A seeded instance needs a new, empty one.", env, bucket)}
+ }
+ return &doorRefusal{http.StatusConflict, seedOwnEnvironment}
+}
+
+// errSeedEnvironmentUnread is instanceStoreProvisionFor's answer when it
+// cannot tell whether another instance names a seed's environment. It
+// answers nothing on w: the seed door speaks its own retry sentence.
+var errSeedEnvironmentUnread = errors.New("console: the instances naming the environment could not be read")
+
+// seedEnvironmentUser names an instance of app, other than seed seedID's
+// own, whose record names env, or "" when none does (final review F6). A
+// seeded instance's derived environment must be unused: its store is new
+// and empty and its seed's finish may tear it down, which another
+// instance's environment must never be. It reads strictly
+// (provision.ListRecordsStrict): a listing it cannot complete, too many
+// records, or a record that will not decode is an error, since any of them
+// may name env.
+func (s *Server) seedEnvironmentUser(ctx context.Context, sqid, app, env, seedID string) (string, error) {
+ recs, unreadable, err := provision.ListRecordsStrict(ctx, s.cfg.Releases, sqid, app, instanceListMax)
+ if err != nil {
+ return "", err
+ }
+ if len(unreadable) > 0 {
+ return "", fmt.Errorf("%d records of %s/%s could not be read, so one may name environment %q", len(unreadable), sqid, app, env)
+ }
+ for _, r := range recs {
+ if r.Environment == env && (r.Seed == nil || r.Seed.ID != seedID) {
+ return r.Host, nil
+ }
+ }
+ return "", nil
}
// seedStoreRefusal is spec §4 Storage's existing-store rule for both doors,
@@ -126,7 +165,16 @@ func (s *Server) seedStoreRefusal(ctx context.Context, c seedCreate, seedID, fai
return &doorRefusal{http.StatusBadGateway, failed}
}
if !seedOwnsStore(rec, seedID) {
- return seedStoreTakenRefusal(c.rec.Environment, storeBucketName(rec))
+ return seedStoreTakenRefusal(c.rec.Environment, rec)
+ }
+ user, err := s.seedEnvironmentUser(ctx, c.acct.Sqid, c.app, c.rec.Environment, seedID)
+ if err != nil {
+ log.Printf("console: seed store check %s/%s/%s: who names the environment: %v", c.acct.Sqid, c.app, c.rec.Environment, err)
+ return &doorRefusal{http.StatusBadGateway, failed}
+ }
+ if user != "" {
+ log.Printf("console: seed store check %s/%s/%s: instance %s already names the environment", c.acct.Sqid, c.app, c.rec.Environment, user)
+ return seedStoreTakenRefusal(c.rec.Environment, rec)
}
return nil
}
@@ -340,8 +388,11 @@ func (s *Server) runFrom(j *fromCopy) {
storeRec, ok, err := s.instanceStoreProvisionFor(ctx, held, acct, app, c.rec.Environment, c.principal, j.id)
if !ok {
reason := held.message(j.failed)
- if errors.Is(err, errSeedStoreTaken) {
- reason = seedStoreTakenRefusal(c.rec.Environment, storeBucketName(storeRec)).Msg
+ switch {
+ case errors.Is(err, errSeedStoreTaken):
+ reason = seedStoreTakenRefusal(c.rec.Environment, storeRec).Msg
+ case errors.Is(err, errSeedEnvironmentUnread):
+ reason = j.failed
}
abort(reason, "store provisioning")
return
diff --git a/internal/console/instanceprivatestore.go b/internal/console/instanceprivatestore.go
index 982e1a7c5..a2b1029aa 100644
--- a/internal/console/instanceprivatestore.go
+++ b/internal/console/instanceprivatestore.go
@@ -195,8 +195,11 @@ var errSeedStoreTaken = errors.New("console: the environment's store is not this
// the lock (the seed door's own) can be overtaken by a concurrent `carlos
// store create --environment`, and GrantIn would then reuse that grant.
// On that refusal it answers nothing on w and returns the record it found,
-// false and errSeedStoreTaken. Every other refusal is answered on w and
-// returns a nil error, as instanceStoreProvision's do.
+// false and errSeedStoreTaken. The environment must also be one no other
+// instance names (final review F6): errSeedStoreTaken again, or
+// errSeedEnvironmentUnread when that cannot be read, also unanswered.
+// Every other refusal is answered on w and returns a nil error, as
+// instanceStoreProvision's do.
//
// A seed's declaration carries its id (objstore.Declared.Seed). A record
// that is declared and holds no estate is declared again for this seed.
@@ -243,6 +246,21 @@ func (s *Server) instanceStoreProvisionFor(ctx context.Context, w http.ResponseW
if seedID != "" && !seedOwnsStore(rec, seedID) {
return rec, false, errSeedStoreTaken
}
+ // A seed's environment must also be unused (final review F6), checked
+ // here under the lock an environment binding takes (seedenvbind.go): a
+ // binding either landed before this read, or finds the store marked
+ // and takes it from the seed.
+ if seedID != "" {
+ user, err := s.seedEnvironmentUser(walkCtx, acct.Sqid, app, env, seedID)
+ if err != nil {
+ log.Printf("console: instance store %s/%s/%s: seed %s: who names the environment: %v", acct.Sqid, app, env, seedID, err)
+ return nil, false, errSeedEnvironmentUnread
+ }
+ if user != "" {
+ log.Printf("console: instance store %s/%s/%s: seed %s: instance %s already names the environment", acct.Sqid, app, env, seedID, user)
+ return rec, false, errSeedStoreTaken
+ }
+ }
existed := rec != nil
if rec == nil {
rec = &objstore.Record{}
diff --git a/internal/console/seedenvuse_test.go b/internal/console/seedenvuse_test.go
new file mode 100644
index 000000000..393d909df
--- /dev/null
+++ b/internal/console/seedenvuse_test.go
@@ -0,0 +1,102 @@
+package console
+
+import (
+ "context"
+ "net/http"
+ "sync"
+ "testing"
+ "time"
+
+ "github.com/carlosframework/platform/internal/objstore"
+ "github.com/carlosframework/platform/internal/provision"
+ "github.com/carlosframework/platform/internal/release"
+)
+
+// sdN33 is the rule sentence a seeded create answers when its derived
+// environment is named by another instance that has no store yet.
+const sdN33 = "a seeded instance gets its own environment and an empty store"
+
+// Final review F6: a source created with --environment canary before its
+// app took private storage, then a --from seed of canary.<domain>, whose
+// derived environment is canary. Both would have shared the environment
+// and the seed's new store. The seed is refused before its record is
+// reserved, and no store is made.
+func TestSeededCreateRefusesAnEnvironmentAnotherInstanceNames(t *testing.T) {
+ f := newFromFixture(t, 3)
+ f.addSource(t, provision.Record{Host: "src.shop.example.com", Slug: "src", Environment: "canary"}, 3)
+ enableInstances(t, f.fx, fromApp, fromManifest(true))
+ code, out := f.create(t, `{"host":"`+fromHost+`","from":{"host":"src.shop.example.com"}}`)
+ if code != http.StatusConflict || str(t, out, "error") != sdN33 {
+ t.Fatalf("--from into a named environment = %d %v, want 409 SD-N33", code, out)
+ }
+ f.requireNoRecord(t, fromHost)
+ if store := f.canaryStore(t); store != nil {
+ t.Fatalf("a store was made for the refused seed: %+v", store)
+ }
+
+ // The import door answers the same, for an instance of any shape.
+ code, out = f.create(t, importBody(fromHost, importSHA, 10, importAddr))
+ if code != http.StatusConflict || str(t, out, "error") != sdN33 {
+ t.Fatalf("an import into a named environment = %d %v, want 409 SD-N33", code, out)
+ }
+ f.requireNoRecord(t, fromHost)
+}
+
+// recordOnRecordWrite writes another instance's record naming env right
+// after the seed's own record lands: the binding made between the door's
+// first check and its walk under the store lock.
+type recordOnRecordWrite struct {
+ release.Store
+ key, env string
+ once sync.Once
+}
+
+func (s *recordOnRecordWrite) Put(ctx context.Context, key string, body []byte, ifMatch string) (string, error) {
+ tag, err := s.Store.Put(ctx, key, body, ifMatch)
+ if err == nil && key == s.key && ifMatch == "" {
+ s.once.Do(func() {
+ if cerr := provision.CreateRecord(ctx, s.Store, provision.Record{Schema: 1, Host: bindHost, Slug: "other", App: fromApp,
+ Account: "bab", Channel: "stable", Environment: s.env, CreatedAt: time.Now().UTC()}); cerr != nil {
+ panic(cerr)
+ }
+ })
+ }
+ return tag, err
+}
+
+func (s *recordOnRecordWrite) DeleteIfMatch(ctx context.Context, key, etag string) error {
+ return release.DeleteIfMatch(ctx, s.Store, key, etag)
+}
+
+// The walk's own check under the store lock refuses an environment another
+// instance came to name after the door's first check: SD-N33, the import
+// rolled back and its host free, and no store declared for it.
+func TestImportBeginEnvironmentNamedUnderTheLock(t *testing.T) {
+ f := newImportFixture(t)
+ enableInstances(t, f.fx, fromApp, fromManifest(true))
+ f.fx.srv.cfg.Releases = &recordOnRecordWrite{Store: f.fx.rel, key: provision.RecordKey("bab", fromApp, importHost), env: "old"}
+ code, out := beginImport(t, f, importHost, importSHA, 10)
+ if code != http.StatusConflict || str(t, out, "error") != sdN33 {
+ t.Fatalf("= %d %v, want 409 SD-N33", code, out)
+ }
+ f.requireNoRecord(t, importHost)
+ if store, _, err := objstore.GetIn(context.Background(), f.fx.rel, "bab", fromApp, "old"); err != nil || store != nil {
+ t.Fatalf("old store = %+v, %v; want none declared", store, err)
+ }
+}
+
+// A record of the app that will not decode may name the environment: the
+// seeded create cannot tell, so it refuses with the door's retry sentence
+// and writes nothing.
+func TestSeededCreateRefusesWhenARecordCannotBeRead(t *testing.T) {
+ f := newImportFixture(t)
+ enableInstances(t, f.fx, fromApp, fromManifest(true))
+ if _, err := f.fx.rel.Put(context.Background(), provision.RecordKey("bab", fromApp, bindHost), []byte("{not json"), ""); err != nil {
+ t.Fatal(err)
+ }
+ code, out := beginImport(t, f, importHost, importSHA, 10)
+ if code != http.StatusBadGateway || str(t, out, "error") != importFailed {
+ t.Fatalf("= %d %v, want 502 %q", code, out, importFailed)
+ }
+ f.requireNoRecord(t, importHost)
+}