amadan

carlos / platform Public

objstore: teardown walk, record deleted last

EmptyBucket -> DeleteBucket -> DeleteUser -> unset the delivered vars ->
delete store.json LAST (spec §9): the record is the only name for the
bucket and principal, so deleting it first would strand a billing bucket
with a live credential nothing can find. Any failure returns with the
record intact and provisioned, so the caller's purge aborts rather than
sweeping the prefix over a standing estate.

The unset is guarded by "is any delivered name present" rather than run
blind: appconfig refuses an unset-only apply against a config object that
does not exist, and an app that was never configured must not have that
refusal strand its teardown.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell e14ff8c9726b2b3db650c96dd2778de4d0cf9667 parent 36263d6
2 files changed, +245 −0
  • internal/objstore/walk.go +89 −0
  • internal/objstore/walk_test.go +156 −0
diff --git a/internal/objstore/walk.go b/internal/objstore/walk.go
index 6032e3321..ad7b58929 100644
--- a/internal/objstore/walk.go
+++ b/internal/objstore/walk.go
@@ -362,6 +362,95 @@ func RotateFinish(ctx context.Context, s release.Store, p Provisioner, acct, app
return rec, nil
}
+// Teardown dismantles a store: bucket contents, bucket, IAM principal (keys
+// and inline policy with it), the delivered config vars, and finally the
+// record. It is the shared engine under the console's "Delete now" purge and
+// its store-destroy door (§9), and it is idempotent at every step, so a
+// half-failed run is repaired by running it again.
+//
+// The ordering rule §9 states once lives here: the record is READ FIRST and
+// DELETED LAST. Everything this walk destroys is named only by the record —
+// the bucket an app's data is in, the principal holding its keys — so a
+// record deleted before the backend is a bucket nobody can find again, still
+// billing, with a live credential attached. Any failure therefore returns with
+// the record intact and provisioned; the caller (a purge) aborts on that error
+// rather than sweeping the prefix over an estate that is still standing.
+//
+// An absent record is nil, not an error: "no store" is the state teardown is
+// trying to reach, and both callers race other operators to it. A record with
+// no Provisioned block — declared, or granted and never walked — has no
+// backend to touch, so it dies trivially.
+func Teardown(ctx context.Context, s release.Store, p Provisioner, acct, app string) error {
+ rec, _, err := Get(ctx, s, acct, app)
+ if err != nil {
+ return err
+ }
+ if rec == nil {
+ return nil
+ }
+ if rec.Provisioned == nil {
+ return s.Delete(ctx, Key(acct, app))
+ }
+ pr := rec.Provisioned
+ // Empty before delete: a real backend refuses to delete a bucket with
+ // objects in it, and EmptyBucket is paginated and resumable for exactly
+ // the bucket too big to clear in one call.
+ if err := p.EmptyBucket(ctx, pr.Bucket); err != nil {
+ return fmt.Errorf("objstore: empty bucket %s: %w", pr.Bucket, err)
+ }
+ if err := p.DeleteBucket(ctx, pr.Bucket); err != nil {
+ return fmt.Errorf("objstore: delete bucket %s: %w", pr.Bucket, err)
+ }
+ // DeleteUser reaps the keys and the inline policy, so the credential dies
+ // with the estate it was scoped to — no orphan principal keeps a key that
+ // names a bucket that no longer exists.
+ if err := p.DeleteUser(ctx, pr.IAMUser); err != nil {
+ return fmt.Errorf("objstore: delete user %s: %w", pr.IAMUser, err)
+ }
+ // The delivered set goes even though the credentials behind it are already
+ // dead: leaving them would hand the app's next render a bucket name and a
+ // key that authenticate against nothing, which is harder to debug than an
+ // unset variable. The prefix is the member's, read back off the record;
+ // Declared should never be nil on a provisioned store (Grant refuses
+ // without it), but this is bucket data an operator can hand-edit, so the
+ // fallback is the default set rather than a panic or a stranded estate.
+ prefix := DefaultEnvPrefix
+ if rec.Declared != nil {
+ prefix = rec.Declared.EnvPrefix
+ }
+ bucketName, regionName, endpointName, accessKeyName, secretKeyName := EnvNames(prefix)
+ unsets := []string{bucketName, regionName, endpointName, accessKeyName, secretKeyName}
+ // _ENDPOINT is unset unconditionally: whether it was delivered depends on
+ // what the deployment looked like at grant time, and unsetting a var that
+ // is not there is a no-op.
+ //
+ // The guard is existence, not the unset itself: appconfig REFUSES an
+ // unset-only apply against a config object that does not exist (creating
+ // one would mint an empty manifest, which means "configured, then fully
+ // unset" to Sync, which converges every route's env file to empty as
+ // root). An app that was never configured has nothing of ours to remove
+ // and must not have that refusal strand its backend teardown — so if none
+ // of the names are present, this step is already converged.
+ m, err := appconfig.Get(ctx, s, acct, app)
+ if err != nil {
+ return err
+ }
+ present := false
+ for _, name := range unsets {
+ if _, ok := m.Vars[name]; ok {
+ present = true
+ break
+ }
+ }
+ if present {
+ if _, err := appconfig.Apply(ctx, s, acct, app, nil, unsets); err != nil {
+ return fmt.Errorf("objstore: remove store config for %s/%s: %w", acct, app, err)
+ }
+ }
+ // Last (§9). Everything it named is gone.
+ return s.Delete(ctx, Key(acct, app))
+}
+
// writeEnv delivers the whole coordinate set in ONE appconfig.Apply — one CAS
// write, so an app never reads a bucket name paired with the previous key.
// Both key halves are sealed for the deployment's secrets key; the plaintext
diff --git a/internal/objstore/walk_test.go b/internal/objstore/walk_test.go
index d41463f7d..24f91218d 100644
--- a/internal/objstore/walk_test.go
+++ b/internal/objstore/walk_test.go
@@ -4,6 +4,7 @@ import (
"context"
"crypto/ecdh"
"errors"
+ "os"
"strings"
"testing"
"time"
@@ -992,3 +993,158 @@ func TestRotateRefusesRetryAgainstAnotherRotation(t *testing.T) {
t.Fatalf("the competing rotation was clobbered: %+v, %v", stored.Provisioned, err)
}
}
+
+// teardownFixture grants a fresh store and returns its record — the estate a
+// teardown has to dismantle.
+func teardownFixture(t *testing.T, s release.Store, pub *ecdh.PublicKey, p *MemProvisioner) *Record {
+ t.Helper()
+ declare(t, s, "bab", "titogo", "TITO_S3")
+ rec, err := Grant(context.Background(), s, p, testDeployment(), pub, "bab", "titogo", "op", "", time.Now().UTC())
+ if err != nil {
+ t.Fatal(err)
+ }
+ return rec
+}
+
+func TestTeardownFullSweep(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ _, pub := testSealKeypair(t)
+ p := NewMemProvisioner("eu-west-1")
+ rec := teardownFixture(t, s, pub, p)
+ // A bucket with data in it is the case that matters: DeleteBucket alone
+ // would fail on a real backend, so the walk must empty it first.
+ p.Buckets[rec.Provisioned.Bucket]["uploads/one.txt"] = []byte("payload")
+
+ if err := Teardown(ctx, s, p, "bab", "titogo"); err != nil {
+ t.Fatal(err)
+ }
+ if len(p.Buckets) != 0 {
+ t.Fatalf("buckets after teardown = %v", p.Buckets)
+ }
+ if len(p.Users) != 0 {
+ t.Fatalf("users after teardown = %v", p.Users)
+ }
+ for name := range mustVars(t, s, "bab", "titogo") {
+ if strings.HasPrefix(name, "TITO_S3_") {
+ t.Fatalf("delivered var %q survived teardown", name)
+ }
+ }
+ stored, _, err := Get(ctx, s, "bab", "titogo")
+ if err != nil || stored != nil {
+ t.Fatalf("record after teardown = %+v, %v", stored, err)
+ }
+}
+
+func TestTeardownResumable(t *testing.T) {
+ for _, method := range []string{"EmptyBucket", "DeleteBucket", "DeleteUser"} {
+ t.Run(method, func(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ _, pub := testSealKeypair(t)
+ p := NewMemProvisioner("eu-west-1")
+ rec := teardownFixture(t, s, pub, p)
+ p.Buckets[rec.Provisioned.Bucket]["uploads/one.txt"] = []byte("payload")
+
+ p.FailNext[method] = errors.New("injected: " + method + " failed")
+ if err := Teardown(ctx, s, p, "bab", "titogo"); err == nil {
+ t.Fatalf("%s failure: Teardown returned no error", method)
+ }
+ // The record is the ONLY name for what is left standing (§9), so a
+ // half-failed teardown must leave it intact and still provisioned.
+ half, _, err := Get(ctx, s, "bab", "titogo")
+ if err != nil || half == nil || half.Provisioned == nil {
+ t.Fatalf("record after %s failure = %+v, %v", method, half, err)
+ }
+ if half.Provisioned.Bucket != rec.Provisioned.Bucket || half.Provisioned.IAMUser != rec.Provisioned.IAMUser {
+ t.Fatalf("record coordinates changed after %s failure: %+v", method, half.Provisioned)
+ }
+
+ if err := Teardown(ctx, s, p, "bab", "titogo"); err != nil {
+ t.Fatalf("resume after %s failure: %v", method, err)
+ }
+ if len(p.Buckets) != 0 || len(p.Users) != 0 {
+ t.Fatalf("estate after resume: buckets %v users %v", p.Buckets, p.Users)
+ }
+ for name := range mustVars(t, s, "bab", "titogo") {
+ if strings.HasPrefix(name, "TITO_S3_") {
+ t.Fatalf("delivered var %q survived teardown", name)
+ }
+ }
+ stored, _, err := Get(ctx, s, "bab", "titogo")
+ if err != nil || stored != nil {
+ t.Fatalf("record after resume = %+v, %v", stored, err)
+ }
+ })
+ }
+}
+
+func TestTeardownDeclaredOnly(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ p := NewMemProvisioner("eu-west-1")
+ declare(t, s, "bab", "titogo", "TITO_S3")
+
+ if err := Teardown(ctx, s, p, "bab", "titogo"); err != nil {
+ t.Fatal(err)
+ }
+ stored, _, err := Get(ctx, s, "bab", "titogo")
+ if err != nil || stored != nil {
+ t.Fatalf("declared-only record after teardown = %+v, %v", stored, err)
+ }
+ if len(p.Buckets) != 0 || len(p.Users) != 0 {
+ t.Fatalf("declared-only teardown touched the backend: %v %v", p.Buckets, p.Users)
+ }
+}
+
+func TestTeardownIdempotentOnAbsent(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ p := NewMemProvisioner("eu-west-1")
+ if err := Teardown(ctx, s, p, "bab", "titogo"); err != nil {
+ t.Fatalf("teardown with no record: %v", err)
+ }
+ if len(p.Buckets) != 0 || len(p.Users) != 0 {
+ t.Fatalf("teardown with no record touched the backend: %v %v", p.Buckets, p.Users)
+ }
+}
+
+func TestTeardownConfigObjectAbsent(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ p := NewMemProvisioner("eu-west-1")
+ // A provisioned record whose app was never configured — hand-crafted, so
+ // no config.json exists at all. appconfig REFUSES an unset-only apply on a
+ // nonexistent object, and that refusal must not strand the estate.
+ if _, err := p.EnsureBucket(ctx, "carlos-store-tito-bab-titogo"); err != nil {
+ t.Fatal(err)
+ }
+ arn, err := p.EnsureUser(ctx, "carlos-store-bab-titogo", UserPath, "arn:aws:s3:::carlos-store-tito-bab-titogo")
+ if err != nil {
+ t.Fatal(err)
+ }
+ rec := &Record{
+ Declared: &Declared{EnvPrefix: "TITO_S3", At: time.Now().UTC(), By: "member@example.com"},
+ Grant: &GrantBlock{Backend: "aws", Bucket: "carlos-store-tito-bab-titogo", At: time.Now().UTC(), By: "op"},
+ Provisioned: &Provisioned{Bucket: "carlos-store-tito-bab-titogo", Region: "eu-west-1",
+ IAMUser: "carlos-store-bab-titogo", UserARN: arn, KeyID: "MEMKEY1", At: time.Now().UTC()},
+ }
+ if _, err := Put(ctx, s, "bab", "titogo", rec, ""); err != nil {
+ t.Fatal(err)
+ }
+
+ if err := Teardown(ctx, s, p, "bab", "titogo"); err != nil {
+ t.Fatalf("teardown with no config object: %v", err)
+ }
+ if len(p.Buckets) != 0 || len(p.Users) != 0 {
+ t.Fatalf("estate after teardown: buckets %v users %v", p.Buckets, p.Users)
+ }
+ stored, _, err := Get(ctx, s, "bab", "titogo")
+ if err != nil || stored != nil {
+ t.Fatalf("record after teardown = %+v, %v", stored, err)
+ }
+ // And no phantom config object was minted on the way through.
+ if _, _, err := s.Get(ctx, appconfig.Key("bab", "titogo")); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("teardown minted a config object: %v", err)
+ }
+}