From cd2618bebf424b779b0a329de80821998a078055 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Tue, 3 Nov 2026 10:50:28 +1000 Subject: [PATCH] feat(admin): non-finalizing DeleteAccount exercises the quiesced EC2 platform kinds (#1458) * feat(admin): non-finalizing DeleteAccount exercises the quiesced EC2 platform kinds A non-finalizing DeleteAccount quiesces the account, records the S9 evaluation, cleans only the key-pair, placement-group and launch-template kinds it recorded as quiesced, then re-reads the owner inventory and accounts for what remains. The job ends EXERCISED with the account still TERMINATING; a later ordinary delete takes the same operation over. Only an exercise records a quiesced scope, so ordinary deletes still record none. The cleanup capability gains a kinds selector, awsgw binds the in-process owners, and the instance reference reader fails closed. * refactor(admin): name the takeover of a finished exercise exercise_finished --- contracts/operator/v1/admin.go | 76 ++++- contracts/operator/v1/admin_test.go | 21 ++ .../domains/ec2/accountcleanup/cleanup.go | 72 ++++- .../ec2/accountcleanup/cleanup_test.go | 35 +++ spinifex/domains/ec2/accountcleanup/kinds.go | 8 +- spinifex/domains/ec2/accountcleanup/types.go | 3 + spinifex/operator/accountlifecycle/engine.go | 12 +- .../operator/accountlifecycle/exercise.go | 207 +++++++++++++ .../accountlifecycle/exercise_test.go | 288 ++++++++++++++++++ .../operator/accountlifecycle/quiescence.go | 15 +- .../accountlifecycle/quiescence_test.go | 17 +- spinifex/operator/admin/account_delete.go | 60 +++- .../admin/account_delete_exercise_test.go | 133 ++++++++ .../operator/admin/account_delete_resumer.go | 2 + .../admin/account_delete_resumer_test.go | 1 + spinifex/operator/admin/registration.go | 5 +- spinifex/operator/cli/admin_account_delete.go | 2 + .../cli/admin_account_delete_remote.go | 5 + spinifex/runtime/roles/awsgw/awsgw.go | 3 + spinifex/runtime/roles/awsgw/compose_test.go | 41 +++ .../runtime/roles/awsgw/platform_cleanup.go | 66 ++++ 21 files changed, 1024 insertions(+), 48 deletions(-) create mode 100644 spinifex/operator/accountlifecycle/exercise.go create mode 100644 spinifex/operator/accountlifecycle/exercise_test.go create mode 100644 spinifex/operator/admin/account_delete_exercise_test.go create mode 100644 spinifex/runtime/roles/awsgw/platform_cleanup.go diff --git a/contracts/operator/v1/admin.go b/contracts/operator/v1/admin.go index a5fe0e0ad..babbe1aac 100644 --- a/contracts/operator/v1/admin.go +++ b/contracts/operator/v1/admin.go @@ -25,6 +25,10 @@ const ( DeletionStateRunning = "RUNNING" DeletionStateCompleted = "COMPLETED" DeletionStateFailed = "FAILED" + + // DeletionStateExercised ends a non-finalizing run: the account is left + // TERMINATING and a later DeleteAccount takes the same operation over. + DeletionStateExercised = "EXERCISED" ) // CreateAccountRequest is the /admin/CreateAccount request body. @@ -52,6 +56,10 @@ type DeleteAccountRequest struct { ClientToken string `json:"clientToken"` Force bool `json:"force,omitempty"` DryRun bool `json:"dryRun,omitempty"` + + // NonFinalizing quiesces the account, cleans the quiesced EC2 platform + // kinds and stops. No legacy stage runs and the account record stays. + NonFinalizing bool `json:"nonFinalizing,omitempty"` } // DescribeAccountDeletionRequest is the /admin/DescribeAccountDeletion body. @@ -76,8 +84,8 @@ type ListAccountsResponse struct { } // IAMQuiescence is the coordinator's recorded view of IAM quiescence for one -// fence. Recording only: it admits nothing, and QuiescedScope stays empty until -// a final inventory re-read exists to catch a late write. +// fence. Recording only: it admits nothing. QuiescedScope is filled only for a +// non-finalizing run, and holds as of EvaluatedAt: a write after that is not caught. type IAMQuiescence struct { OperationID string `json:"operationId"` Fence uint64 `json:"fence"` @@ -136,3 +144,67 @@ const ( ReasonOwnerInventoryIncomplete = "owner-inventory-incomplete" ReasonFinalInventoryRereadNotWired = "final-inventory-reread-not-wired" ) + +// ExerciseResult is what a non-finalizing run did after IAM quiescence. +type ExerciseResult struct { + // Skipped is the fixed reason no cleanup ran; Cleanup is then absent. + Skipped string `json:"skipped,omitempty"` + Cleanup *PlatformCleanup `json:"cleanup,omitempty"` + Accounting []KindAccounting `json:"accounting,omitempty"` +} + +// PlatformCleanup is the EC2 platform cleanup result. It carries resource IDs, +// fixed reasons and counts only, never names, tags, key material or data. +type PlatformCleanup struct { + Kinds []CleanupKind `json:"kinds"` + Items []CleanupItem `json:"items,omitempty"` + Counts []CleanupCount `json:"counts,omitempty"` +} + +// CleanupKind is one kind's scan status after the cleanup's own re-scan. +type CleanupKind struct { + Kind string `json:"kind"` + Status string `json:"status"` + IncompleteReasons []string `json:"incompleteReasons,omitempty"` +} + +// CleanupItem is one resource by its ID with what the cleanup did to it. +type CleanupItem struct { + ResourceID string `json:"resourceId"` + Kind string `json:"kind"` + Disposition string `json:"disposition"` + Reason string `json:"reason,omitempty"` + MemberCount int `json:"memberCount,omitempty"` +} + +// CleanupCount is the number of records of one class that have no public ID. +type CleanupCount struct { + Kind string `json:"kind"` + Class string `json:"class"` + Disposition string `json:"disposition"` + Reason string `json:"reason,omitempty"` + Count int `json:"count"` +} + +// KindAccounting is the final inventory re-read held against the cleanup result +// for one kind. Quiescence is not rewritten from it. +type KindAccounting struct { + Kind string `json:"kind"` + Status string `json:"status"` + Reason string `json:"reason,omitempty"` + Remaining int `json:"remaining"` +} + +// Accounting statuses and fixed reasons of a KindAccounting. +const ( + AccountingAccounted = "accounted" + AccountingUnaccounted = "unaccounted" + + AccountingReasonRereadIncomplete = "reread-incomplete" + AccountingReasonUnnamedRemainder = "unnamed-remainder" +) + +// Fixed reasons an exercise ran no cleanup. +const ( + ExerciseSkippedNoQuiescedKind = "no-quiesced-kind" +) diff --git a/contracts/operator/v1/admin_test.go b/contracts/operator/v1/admin_test.go index d4172eee2..5a9a629f8 100644 --- a/contracts/operator/v1/admin_test.go +++ b/contracts/operator/v1/admin_test.go @@ -43,6 +43,27 @@ func TestAdminWire_DeletionStates(t *testing.T) { require.Equal(t, "RUNNING", DeletionStateRunning) require.Equal(t, "COMPLETED", DeletionStateCompleted) require.Equal(t, "FAILED", DeletionStateFailed) + require.Equal(t, "EXERCISED", DeletionStateExercised) +} + +func TestAdminWire_NonFinalizingDeleteRequest(t *testing.T) { + goldenJSON(t, DeleteAccountRequest{AccountID: "000000000042", AccountName: "acme", ClientToken: "tok-3", NonFinalizing: true}, + `{"accountId":"000000000042","accountName":"acme","clientToken":"tok-3","nonFinalizing":true}`) +} + +func TestAdminWire_ExerciseResult(t *testing.T) { + goldenJSON(t, ExerciseResult{ + Cleanup: &PlatformCleanup{ + Kinds: []CleanupKind{{Kind: "key-pair", Status: "observed-clean-at-scan-time"}}, + Items: []CleanupItem{{ResourceID: "pg-1", Kind: "placement-group", Disposition: "blocked", Reason: "instance-members", MemberCount: 1}}, + Counts: []CleanupCount{{Kind: "key-pair", Class: "public-key-object-without-metadata", Disposition: "deleted", Count: 2}}, + }, + Accounting: []KindAccounting{{Kind: "placement-group", Status: AccountingAccounted, Remaining: 1}}, + }, `{"cleanup":{"kinds":[{"kind":"key-pair","status":"observed-clean-at-scan-time"}], + "items":[{"resourceId":"pg-1","kind":"placement-group","disposition":"blocked","reason":"instance-members","memberCount":1}], + "counts":[{"kind":"key-pair","class":"public-key-object-without-metadata","disposition":"deleted","count":2}]}, + "accounting":[{"kind":"placement-group","status":"accounted","remaining":1}]}`) + goldenJSON(t, ExerciseResult{Skipped: ExerciseSkippedNoQuiescedKind}, `{"skipped":"no-quiesced-kind"}`) } func TestAdminWire_ListAccountsResponse(t *testing.T) { diff --git a/spinifex/domains/ec2/accountcleanup/cleanup.go b/spinifex/domains/ec2/accountcleanup/cleanup.go index 96c6d8003..5005b4381 100644 --- a/spinifex/domains/ec2/accountcleanup/cleanup.go +++ b/spinifex/domains/ec2/accountcleanup/cleanup.go @@ -69,6 +69,28 @@ var ( _ TagIndexOwner = (*ec2tags.TagsServiceImpl)(nil) ) +// kindPrefixes maps each cleanable kind to its resource-ID prefix in the tag index. +var kindPrefixes = map[string]string{ + ec2v1.PlatformKindKeyPair: keyPairIDPrefix, + ec2v1.PlatformKindPlacementGroup: placementGroupIDPrefix, + ec2v1.PlatformKindLaunchTemplate: launchTemplateIDPrefix, +} + +// selectKinds returns the kinds a request cleans, in report order. No kinds +// means all three; a kind that is not cleanable, such as the tag index, is refused. +func selectKinds(requested []string) ([]string, error) { + all := []string{ec2v1.PlatformKindKeyPair, ec2v1.PlatformKindPlacementGroup, ec2v1.PlatformKindLaunchTemplate} + if len(requested) == 0 { + return all, nil + } + for _, kind := range requested { + if !slices.Contains(all, kind) { + return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "The kind is not one of key-pair, placement-group or launch-template.") + } + } + return slices.DeleteFunc(all, func(kind string) bool { return !slices.Contains(requested, kind) }), nil +} + // DefaultSettleWindow is how old headerless residue must be before it is // removed; anything younger may be a create still in flight. const DefaultSettleWindow = 10 * time.Minute @@ -102,6 +124,7 @@ const ( // CleanupPlatform deletes the account's key pairs, empty placement groups and // launch templates, removes settled residue and orphaned tag-index entries, // then re-scans every owner store to build the result. It is safe to re-run. +// Request.Kinds limits the run to those kinds and their tag-index entries. func (c *Cleanup) CleanupPlatform(ctx context.Context, req *Request) (*Result, error) { if req == nil || req.AccountID == "" { return nil, awserrors.Errorf(awserrors.ErrorMissingParameter, "The request must specify an account ID.") @@ -118,27 +141,54 @@ func (c *Cleanup) CleanupPlatform(ctx context.Context, req *Request) (*Result, e acct := req.AccountID res := &Result{AccountID: acct, CorrelationRef: req.CorrelationRef} - refs := c.readReferences(ctx, acct) - keyRun := c.cleanKeyPairs(ctx, acct, settledBefore, refs) - groupRun := c.cleanPlacementGroups(ctx, acct, refs) - templateRun := c.cleanLaunchTemplates(ctx, acct, settledBefore) + kinds, err := selectKinds(req.Kinds) + if err != nil { + return nil, err + } + var prefixes []string + for _, kind := range kinds { + prefixes = append(prefixes, kindPrefixes[kind]) + } + + var keyRun *keyPairRun + var groupRun *placementGroupRun + var templateRun *launchTemplateRun + if slices.Contains(kinds, ec2v1.PlatformKindKeyPair) || slices.Contains(kinds, ec2v1.PlatformKindPlacementGroup) { + refs := c.readReferences(ctx, acct) + if slices.Contains(kinds, ec2v1.PlatformKindKeyPair) { + keyRun = c.cleanKeyPairs(ctx, acct, settledBefore, refs) + } + if slices.Contains(kinds, ec2v1.PlatformKindPlacementGroup) { + groupRun = c.cleanPlacementGroups(ctx, acct, refs) + } + } + if slices.Contains(kinds, ec2v1.PlatformKindLaunchTemplate) { + templateRun = c.cleanLaunchTemplates(ctx, acct, settledBefore) + } // Tags are listed before the owner re-scans, so a resource created in // between is seen as owned and keeps its tags. tagKind := c.startKind(ec2v1.PlatformKindTagIndex) - tagged, tagErr := c.tags.ScanAccountResourceIDs(ctx, acct, keyPairIDPrefix, placementGroupIDPrefix, launchTemplateIDPrefix) + tagged, tagErr := c.tags.ScanAccountResourceIDs(ctx, acct, prefixes...) if tagErr != nil { slog.WarnContext(ctx, "Platform cleanup: tag index listing failed", "err", tagErr) tagKind.incomplete(ec2v1.PlatformReasonStoreListFailed) } - owners := map[string]ownedIDs{ - keyPairIDPrefix: c.finishKeyPairs(ctx, acct, keyRun, res), - placementGroupIDPrefix: c.finishPlacementGroups(ctx, acct, groupRun, res), - launchTemplateIDPrefix: c.finishLaunchTemplates(ctx, acct, templateRun, res), + owners := map[string]ownedIDs{} + if keyRun != nil { + owners[keyPairIDPrefix] = c.finishKeyPairs(ctx, acct, keyRun, res) + res.Kinds = append(res.Kinds, keyRun.kind.report) } - res.Kinds = []KindReport{keyRun.kind.report, groupRun.kind.report, templateRun.kind.report} - res.Kinds = append(res.Kinds, c.cleanTagIndex(ctx, acct, tagKind, tagged, tagErr == nil, owners, res)) + if groupRun != nil { + owners[placementGroupIDPrefix] = c.finishPlacementGroups(ctx, acct, groupRun, res) + res.Kinds = append(res.Kinds, groupRun.kind.report) + } + if templateRun != nil { + owners[launchTemplateIDPrefix] = c.finishLaunchTemplates(ctx, acct, templateRun, res) + res.Kinds = append(res.Kinds, templateRun.kind.report) + } + res.Kinds = append(res.Kinds, c.cleanTagIndex(ctx, acct, tagKind, tagged, prefixes, tagErr == nil, owners, res)) slog.InfoContext(ctx, "Platform cleanup completed", "accountID", acct, "correlationRef", req.CorrelationRef, "items", len(res.Items), "duration_ms", otelsetup.Millis(time.Since(start))) diff --git a/spinifex/domains/ec2/accountcleanup/cleanup_test.go b/spinifex/domains/ec2/accountcleanup/cleanup_test.go index dcb4c4da4..1a3583cb8 100644 --- a/spinifex/domains/ec2/accountcleanup/cleanup_test.go +++ b/spinifex/domains/ec2/accountcleanup/cleanup_test.go @@ -775,3 +775,38 @@ func (f failingResidue) CleanupAccountResidue(ctx context.Context, acct string, } return &ec2launchtemplate.AccountCleanup{KeptTemplateIDs: scan.LiveTemplateIDs, OrphanBodiesFailed: scan.OrphanVersionBodies}, nil } + +// A run limited to one kind cleans that kind and its tag-index entries only: +// the other kinds keep their resources and their tags, and are not reported. +func TestCleanupPlatformLimitedToKindsLeavesTheRestAlone(t *testing.T) { + o := newOwners(t) + keyID := o.importKey(t, testAccount, "k1") + groupID := o.createGroup(t, testAccount, "g1") + o.tag(t, testAccount, keyID) + + c := o.newCleanup() + c.now = func() time.Time { return time.Now().Add(time.Hour) } + res, err := c.CleanupPlatform(t.Context(), &Request{AccountID: testAccount, Kinds: []string{ec2v1.PlatformKindPlacementGroup}}) + require.NoError(t, err) + + assert.Equal(t, []Item{item(groupID, ec2v1.PlatformKindPlacementGroup, DispositionDeleted, "")}, res.Items) + assert.Equal(t, map[string]string{ + ec2v1.PlatformKindPlacementGroup: ec2v1.PlatformStatusObservedClean, + ec2v1.PlatformKindTagIndex: ec2v1.PlatformStatusObservedClean, + }, statuses(res)) + + keys, err := o.keys.ScanAccount(t.Context(), testAccount) + require.NoError(t, err) + assert.Equal(t, []string{keyID}, keys.LiveKeyPairIDs, "an unselected kind is not touched") + tagged, err := o.tags.ScanAccountResourceIDs(t.Context(), testAccount, "key-") + require.NoError(t, err) + assert.Equal(t, []string{keyID}, tagged, "an unselected kind keeps its tags") +} + +func TestCleanupPlatformRefusesAKindItCannotClean(t *testing.T) { + o := newOwners(t) + for _, kinds := range [][]string{{ec2v1.PlatformKindTagIndex}, {"volume"}} { + _, err := o.newCleanup().CleanupPlatform(t.Context(), &Request{AccountID: testAccount, Kinds: kinds}) + require.Error(t, err, "%v", kinds) + } +} diff --git a/spinifex/domains/ec2/accountcleanup/kinds.go b/spinifex/domains/ec2/accountcleanup/kinds.go index c1e0dff2e..3637be3de 100644 --- a/spinifex/domains/ec2/accountcleanup/kinds.go +++ b/spinifex/domains/ec2/accountcleanup/kinds.go @@ -282,15 +282,15 @@ func (c *Cleanup) finishLaunchTemplates(ctx context.Context, acct string, run *l // cleanTagIndex clears entries whose owner re-scan resolved and holds no // record for the ID, then re-lists the index to prove each clear. Entries of // resources that still exist are theirs, not residue, and are left alone. -func (c *Cleanup) cleanTagIndex(ctx context.Context, acct string, kind *kindRun, tagged []string, listed bool, owners map[string]ownedIDs, res *Result) KindReport { +func (c *Cleanup) cleanTagIndex(ctx context.Context, acct string, kind *kindRun, tagged, prefixes []string, listed bool, owners map[string]ownedIDs, res *Result) KindReport { if listed { - c.clearOrphanTags(ctx, acct, kind, tagged, owners, res) + c.clearOrphanTags(ctx, acct, kind, tagged, prefixes, owners, res) } c.finishKind(kind) return kind.report } -func (c *Cleanup) clearOrphanTags(ctx context.Context, acct string, kind *kindRun, tagged []string, owners map[string]ownedIDs, res *Result) { +func (c *Cleanup) clearOrphanTags(ctx context.Context, acct string, kind *kindRun, tagged, prefixes []string, owners map[string]ownedIDs, res *Result) { // A resource this run reports as not deleted owns its tags even when its // kind's re-scan failed. reported := map[string]bool{} @@ -317,7 +317,7 @@ func (c *Cleanup) clearOrphanTags(ctx context.Context, acct string, kind *kindRu cleared = append(cleared, id) } } - after, err := c.tags.ScanAccountResourceIDs(ctx, acct, keyPairIDPrefix, placementGroupIDPrefix, launchTemplateIDPrefix) + after, err := c.tags.ScanAccountResourceIDs(ctx, acct, prefixes...) if err != nil { slog.WarnContext(ctx, "Platform cleanup: tag index re-scan failed", "err", err) kind.incomplete(ec2v1.PlatformReasonStoreListFailed) diff --git a/spinifex/domains/ec2/accountcleanup/types.go b/spinifex/domains/ec2/accountcleanup/types.go index 4ed6b7fab..8a3cf1349 100644 --- a/spinifex/domains/ec2/accountcleanup/types.go +++ b/spinifex/domains/ec2/accountcleanup/types.go @@ -7,6 +7,9 @@ import "time" type Request struct { AccountID string `json:"account_id"` CorrelationRef string `json:"correlation_ref,omitempty"` + + // Kinds limits the run to these platform kinds. Empty means all three. + Kinds []string `json:"kinds,omitempty"` } // Result is what the owner stores hold after one run. Kinds reuse the ec2.v1 diff --git a/spinifex/operator/accountlifecycle/engine.go b/spinifex/operator/accountlifecycle/engine.go index c53c37f24..a89919dff 100644 --- a/spinifex/operator/accountlifecycle/engine.go +++ b/spinifex/operator/accountlifecycle/engine.go @@ -35,6 +35,12 @@ type Engine struct { // TERMINATING and before the first stage. It records evidence and gates nothing. OnQuiesced func(ctx context.Context, quiescence operatorv1.IAMQuiescence) + // Cleanup runs the EC2 platform cleanup. Only Exercise uses it. + Cleanup PlatformCleaner + + // OnExercised receives what a non-finalizing run did after quiescence. + OnExercised func(ctx context.Context, result operatorv1.ExerciseResult) + // ec2Inventory is EC2's read-only platform inventory. Only DryRun consults // it, and only to log a comparison; no teardown or Inventory call does. ec2Inventory ec2PlatformInventory @@ -129,7 +135,7 @@ func (e *Engine) Teardown(ctx context.Context, req Request) (*Result, error) { return nil, fmt.Errorf("quiesce account: %w", err) } if e.OnQuiesced != nil { - e.OnQuiesced(ctx, e.evaluateQuiescence(ctx, req, generation)) + e.OnQuiesced(ctx, e.evaluateQuiescence(ctx, req, generation, false)) } e.logger().InfoContext(ctx, "Account teardown started", "accountID", account.AccountID, "accountName", account.AccountName, "force", req.Force) @@ -338,9 +344,9 @@ func SortReapers(reapers []Reaper) { // evaluateQuiescence reads back what the quiesce wrote and, only if the fence // is confirmed, asks EC2 for its platform inventory within a short budget. -func (e *Engine) evaluateQuiescence(ctx context.Context, req Request, generation uint64) operatorv1.IAMQuiescence { +func (e *Engine) evaluateQuiescence(ctx context.Context, req Request, generation uint64, exercise bool) operatorv1.IAMQuiescence { ev := QuiesceEvidence{ - OperationID: req.OperationID, Fence: req.Fence, Generation: generation, EvaluatedAt: time.Now(), + OperationID: req.OperationID, Fence: req.Fence, Generation: generation, EvaluatedAt: time.Now(), Exercise: exercise, } ev.Account, ev.ReadbackErr = e.Accounts.GetAccount(req.AccountID) if _, confirmed := fenceReadback(ev); confirmed && e.ec2Inventory != nil { diff --git a/spinifex/operator/accountlifecycle/exercise.go b/spinifex/operator/accountlifecycle/exercise.go new file mode 100644 index 000000000..265830687 --- /dev/null +++ b/spinifex/operator/accountlifecycle/exercise.go @@ -0,0 +1,207 @@ +package accountlifecycle + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/aws/aws-sdk-go/service/ec2" + ec2v1 "github.com/mulgadc/spinifex/contracts/ec2/v1" + operatorv1 "github.com/mulgadc/spinifex/contracts/operator/v1" + "github.com/mulgadc/spinifex/spinifex/domains/ec2/accountcleanup" + ec2instanceapi "github.com/mulgadc/spinifex/spinifex/domains/ec2/awsapi/instance" + "github.com/nats-io/nats.go" +) + +// PlatformCleaner is the EC2 platform cleanup the exercise runs. +type PlatformCleaner = accountcleanup.PlatformCleaner + +// Exercise quiesces the account, cleans the EC2 platform kinds the evaluation +// recorded as quiesced, then re-reads the owner inventory and holds it against +// the cleanup. No legacy stage runs and the account record is never removed. +func (e *Engine) Exercise(ctx context.Context, req Request) (*operatorv1.ExerciseResult, error) { + if _, err := e.checkDeletable(req.AccountID, req.AccountName); err != nil { + return nil, err + } + if req.Force { + return nil, errors.New("a non-finalizing run refuses force") + } + if e.Cleanup == nil || e.ec2Inventory == nil { + return nil, errors.New("exercise needs the platform cleanup and inventory") + } + generation, err := e.Accounts.QuiesceAccount(req.AccountID, req.OperationID, req.Fence) + if err != nil { + return nil, fmt.Errorf("quiesce account: %w", err) + } + quiescence := e.evaluateQuiescence(ctx, req, generation, true) + if e.OnQuiesced != nil { + e.OnQuiesced(ctx, quiescence) + } + + result := &operatorv1.ExerciseResult{} + if len(quiescence.QuiescedScope) == 0 { + result.Skipped = operatorv1.ExerciseSkippedNoQuiescedKind + } else { + cleaned, err := e.Cleanup.CleanupPlatform(ctx, &accountcleanup.Request{ + AccountID: req.AccountID, CorrelationRef: req.OperationID, Kinds: quiescence.QuiescedScope, + }) + if err != nil { + return nil, fmt.Errorf("platform cleanup: %w", err) + } + result.Cleanup = platformCleanup(cleaned) + result.Accounting = e.accountForRemainder(ctx, req, quiescence.QuiescedScope, result.Cleanup) + } + if e.OnExercised != nil { + e.OnExercised(ctx, *result) + } + return result, nil +} + +// accountForRemainder re-reads the owner inventory after cleanup and holds it +// against the cleanup result, per quiesced kind. +func (e *Engine) accountForRemainder(ctx context.Context, req Request, kinds []string, cleanup *operatorv1.PlatformCleanup) []operatorv1.KindAccounting { + budget := ec2PlatformShadowBudget + if e.ec2PlatformShadowBudget > 0 { + budget = e.ec2PlatformShadowBudget + } + rereadCtx, cancel := context.WithTimeout(ctx, budget) + defer cancel() + reply := e.ec2Inventory.PlatformInventory(rereadCtx, req.AccountID, req.OperationID) + return AccountForRemainder(kinds, &reply, cleanup) +} + +// AccountForRemainder marks a kind accounted only when its re-read was complete +// and every record still there is named in the cleanup result with a reason. +// A record the cleanup reported deleted that is back is unnamed. +func AccountForRemainder(kinds []string, reread *ec2v1.PlatformInventoryReply, cleanup *operatorv1.PlatformCleanup) []operatorv1.KindAccounting { + out := make([]operatorv1.KindAccounting, 0, len(kinds)) + for _, kind := range kinds { + entry := operatorv1.KindAccounting{Kind: kind, Status: operatorv1.AccountingAccounted} + status, complete := kindInventory(reread, kind) + if !complete { + entry.Status, entry.Reason = operatorv1.AccountingUnaccounted, operatorv1.AccountingReasonRereadIncomplete + out = append(out, entry) + continue + } + if status == ec2v1.PlatformStatusRecordsFound { + unnamed := false + for _, item := range reread.Items { + if item.Kind != kind { + continue + } + entry.Remaining++ + unnamed = unnamed || !namedItem(cleanup, item.ResourceID, kind) + } + for _, count := range reread.Counts { + if count.Kind != kind { + continue + } + entry.Remaining += count.Count + unnamed = unnamed || !namedCount(cleanup, count) + } + if unnamed { + entry.Status, entry.Reason = operatorv1.AccountingUnaccounted, operatorv1.AccountingReasonUnnamedRemainder + } + } + out = append(out, entry) + } + return out +} + +func namedItem(cleanup *operatorv1.PlatformCleanup, id, kind string) bool { + if cleanup == nil { + return false + } + for _, item := range cleanup.Items { + if item.ResourceID == id && item.Kind == kind && item.Disposition != accountcleanup.DispositionDeleted && item.Reason != "" { + return true + } + } + return false +} + +func namedCount(cleanup *operatorv1.PlatformCleanup, want ec2v1.PlatformRecordCount) bool { + if cleanup == nil { + return false + } + for _, count := range cleanup.Counts { + if count.Kind == want.Kind && count.Class == want.Class && count.Disposition != accountcleanup.DispositionDeleted && + count.Reason != "" && count.Count >= want.Count { + return true + } + } + return false +} + +func platformCleanup(res *accountcleanup.Result) *operatorv1.PlatformCleanup { + out := &operatorv1.PlatformCleanup{Kinds: []operatorv1.CleanupKind{}} + for _, kind := range res.Kinds { + out.Kinds = append(out.Kinds, operatorv1.CleanupKind{Kind: kind.Kind, Status: kind.Status, IncompleteReasons: kind.IncompleteReasons}) + } + for _, item := range res.Items { + out.Items = append(out.Items, operatorv1.CleanupItem{ + ResourceID: item.ResourceID, Kind: item.Kind, Disposition: item.Disposition, Reason: item.Reason, MemberCount: item.MemberCount, + }) + } + for _, count := range res.Counts { + out.Counts = append(out.Counts, operatorv1.CleanupCount{ + Kind: count.Kind, Class: count.Class, Disposition: count.Disposition, Reason: count.Reason, Count: count.Count, + }) + } + return out +} + +// InstanceReferenceReader reads the key-pair and placement-group names the +// account's non-terminated instances carry. It fails closed: a listing that is +// not complete from every node is an error, never a partial answer. +type InstanceReferenceReader struct { + nc *nats.Conn + nodeIDs []string + budget time.Duration +} + +var _ accountcleanup.InstanceReferenceReader = (*InstanceReferenceReader)(nil) + +// NewInstanceReferenceReader builds the reader over the configured node set. +func NewInstanceReferenceReader(nc *nats.Conn, nodeIDs []string) *InstanceReferenceReader { + return &InstanceReferenceReader{nc: nc, nodeIDs: nodeIDs, budget: ec2PlatformShadowBudget} +} + +func (r *InstanceReferenceReader) AccountInstanceReferences(ctx context.Context, accountID string) (*accountcleanup.References, error) { + ctx, cancel := context.WithTimeout(ctx, r.budget) + defer cancel() + out, err := ec2instanceapi.DescribeAccountInstancesComplete(ctx, r.nc, r.nodeIDs, accountID) + if err != nil { + return nil, err + } + return instanceReferences(out), nil +} + +func instanceReferences(out *ec2.DescribeInstancesOutput) *accountcleanup.References { + refs := &accountcleanup.References{ + KeyNames: map[string]bool{}, PlacementGroupNames: map[string]bool{}, PlacementGroupIDs: map[string]bool{}, + } + for _, reservation := range out.Reservations { + for _, instance := range reservation.Instances { + if instance == nil { + continue + } + if instance.State != nil && instance.State.Name != nil && terminalInstanceStates[*instance.State.Name] { + continue + } + if instance.KeyName != nil && *instance.KeyName != "" { + refs.KeyNames[*instance.KeyName] = true + } + if instance.Placement != nil { + if instance.Placement.GroupName != nil && *instance.Placement.GroupName != "" { + refs.PlacementGroupNames[*instance.Placement.GroupName] = true + } + if instance.Placement.GroupId != nil && *instance.Placement.GroupId != "" { + refs.PlacementGroupIDs[*instance.Placement.GroupId] = true + } + } + } + } + return refs +} diff --git a/spinifex/operator/accountlifecycle/exercise_test.go b/spinifex/operator/accountlifecycle/exercise_test.go new file mode 100644 index 000000000..10c139342 --- /dev/null +++ b/spinifex/operator/accountlifecycle/exercise_test.go @@ -0,0 +1,288 @@ +package accountlifecycle + +//test:in-package — the exercise's inventory and cleanup collaborators are +// unexported seams, and the S7 invariants drive the engine directly. + +import ( + "context" + "errors" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + ec2v1 "github.com/mulgadc/spinifex/contracts/ec2/v1" + operatorv1 "github.com/mulgadc/spinifex/contracts/operator/v1" + "github.com/mulgadc/spinifex/spinifex/domains/ec2/accountcleanup" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type stubCleaner struct { + calls []accountcleanup.Request + result *accountcleanup.Result + err error + // onCall runs inside the cleanup, so a test can observe what was recorded first. + onCall func() +} + +func (s *stubCleaner) CleanupPlatform(_ context.Context, req *accountcleanup.Request) (*accountcleanup.Result, error) { + s.calls = append(s.calls, *req) + if s.onCall != nil { + s.onCall() + } + return s.result, s.err +} + +// sequencedInventory answers the quiescence evaluation first, then the re-read. +type sequencedInventory struct { + replies []ec2v1.PlatformInventoryReply + calls int +} + +func (s *sequencedInventory) PlatformInventory(context.Context, string, string) ec2v1.PlatformInventoryReply { + reply := s.replies[min(s.calls, len(s.replies)-1)] + s.calls++ + return reply +} + +func exerciseEngine(accounts AccountStore, cleaner PlatformCleaner, inventory ec2PlatformInventory) *Engine { + engine := quietEngine(accounts) + engine.Cleanup = cleaner + engine.ec2Inventory = inventory + return engine +} + +func exerciseRequest() Request { + req := request() + req.OperationID, req.Fence = "op-1", 1 + return req +} + +func cleanResult() *accountcleanup.Result { + res := &accountcleanup.Result{AccountID: testAccountID} + for _, kind := range ec2v1.PlatformKinds() { + res.Kinds = append(res.Kinds, accountcleanup.KindReport{Kind: kind, Status: ec2v1.PlatformStatusObservedClean}) + } + return res +} + +func accountingFor(result *operatorv1.ExerciseResult, kind string) operatorv1.KindAccounting { + for _, entry := range result.Accounting { + if entry.Kind == kind { + return entry + } + } + return operatorv1.KindAccounting{} +} + +// The exercise records the evaluation before it cleans, cleans only the kinds that +// evaluation recorded as quiesced, and never removes the account record. +func TestExerciseCleansTheQuiescedKindsAfterRecordingAndKeepsTheAccount(t *testing.T) { + accounts := newFakeAccounts() + var recorded *operatorv1.IAMQuiescence + cleaner := &stubCleaner{result: cleanResult()} + cleaner.onCall = func() { assert.NotNil(t, recorded, "quiescence must be recorded before any cleanup") } + inventory := &sequencedInventory{replies: []ec2v1.PlatformInventoryReply{*completeInventory()}} + engine := exerciseEngine(accounts, cleaner, inventory) + engine.OnQuiesced = func(_ context.Context, q operatorv1.IAMQuiescence) { recorded = &q } + + result, err := engine.Exercise(context.Background(), exerciseRequest()) + + require.NoError(t, err) + require.NotNil(t, recorded) + assert.ElementsMatch(t, ec2PlatformKinds, recorded.QuiescedScope) + require.Len(t, cleaner.calls, 1) + assert.ElementsMatch(t, recorded.QuiescedScope, cleaner.calls[0].Kinds) + assert.Equal(t, "op-1", cleaner.calls[0].CorrelationRef) + assert.False(t, accounts.deleted, "an exercise never removes the account record") + assert.Equal(t, AccountStatusTerminating, accounts.account.Status) + assert.Equal(t, 2, inventory.calls, "one evaluation read and one final re-read") + for _, kind := range recorded.QuiescedScope { + assert.Equal(t, operatorv1.AccountingAccounted, accountingFor(result, kind).Status, kind) + } +} + +// The final re-read is held against the cleanup result and the quiescence record +// is not rewritten from it. +func TestExerciseAccountsForARemainderOnlyWhenTheCleanupNamedIt(t *testing.T) { + cases := map[string]struct { + cleanup *accountcleanup.Result + reread ec2v1.PlatformInventoryReply + want string + reason string + }{ + "late write the cleanup never saw": { + cleanup: cleanResult(), + reread: rereadWithKeyPair("key-late"), + want: operatorv1.AccountingUnaccounted, reason: operatorv1.AccountingReasonUnnamedRemainder, + }, + "cleanup deleted it but it is back": { + cleanup: withItem(cleanResult(), accountcleanup.Item{ResourceID: "key-1", Kind: ec2v1.PlatformKindKeyPair, Disposition: accountcleanup.DispositionDeleted}), + reread: rereadWithKeyPair("key-1"), + want: operatorv1.AccountingUnaccounted, reason: operatorv1.AccountingReasonUnnamedRemainder, + }, + "cleanup blocked it with a reason": { + cleanup: withItem(cleanResult(), accountcleanup.Item{ResourceID: "key-1", Kind: ec2v1.PlatformKindKeyPair, Disposition: accountcleanup.DispositionBlocked, Reason: accountcleanup.ReasonInstanceReference}), + reread: rereadWithKeyPair("key-1"), + want: operatorv1.AccountingAccounted, + }, + "re-read incomplete": { + cleanup: cleanResult(), + reread: *incompleteKeyPairs(), + want: operatorv1.AccountingUnaccounted, reason: operatorv1.AccountingReasonRereadIncomplete, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + accounts := newFakeAccounts() + inventory := &sequencedInventory{replies: []ec2v1.PlatformInventoryReply{*completeInventory(), tc.reread}} + engine := exerciseEngine(accounts, &stubCleaner{result: tc.cleanup}, inventory) + var recorded operatorv1.IAMQuiescence + engine.OnQuiesced = func(_ context.Context, q operatorv1.IAMQuiescence) { recorded = q } + + result, err := engine.Exercise(context.Background(), exerciseRequest()) + + require.NoError(t, err) + entry := accountingFor(result, ec2v1.PlatformKindKeyPair) + assert.Equal(t, tc.want, entry.Status) + assert.Equal(t, tc.reason, entry.Reason) + assert.Contains(t, recorded.QuiescedScope, ec2v1.PlatformKindKeyPair, "quiescence is not rewritten from the re-read") + }) + } +} + +func rereadWithKeyPair(id string) ec2v1.PlatformInventoryReply { + reply := *completeInventory() + reply.Kinds[0].Status = ec2v1.PlatformStatusRecordsFound + reply.Items = []ec2v1.PlatformItem{{ResourceID: id, Kind: ec2v1.PlatformKindKeyPair, Disposition: accountcleanup.DispositionBlocked}} + return reply +} + +func incompleteKeyPairs() *ec2v1.PlatformInventoryReply { + reply := completeInventory() + reply.Kinds[0].Status = ec2v1.PlatformStatusScanIncomplete + return reply +} + +func withItem(res *accountcleanup.Result, item accountcleanup.Item) *accountcleanup.Result { + res.Items = append(res.Items, item) + return res +} + +// With no kind quiesced the exercise runs no cleanup and records why. +func TestExerciseWithNoQuiescedKindRunsNoCleanup(t *testing.T) { + accounts := newFakeAccounts() + cleaner := &stubCleaner{result: cleanResult()} + incomplete := ec2v1.IncompletePlatformInventory(testAccountID, "op-1", ec2v1.PlatformReasonTimeout) + engine := exerciseEngine(accounts, cleaner, &sequencedInventory{replies: []ec2v1.PlatformInventoryReply{incomplete}}) + + result, err := engine.Exercise(context.Background(), exerciseRequest()) + + require.NoError(t, err) + assert.Empty(t, cleaner.calls) + assert.Equal(t, operatorv1.ExerciseSkippedNoQuiescedKind, result.Skipped) + assert.Nil(t, result.Cleanup) +} + +func TestExerciseRefusesForceAndAMissingCollaborator(t *testing.T) { + accounts := newFakeAccounts() + req := exerciseRequest() + req.Force = true + _, err := exerciseEngine(accounts, &stubCleaner{}, &stubInventory{}).Exercise(context.Background(), req) + require.Error(t, err) + + _, err = exerciseEngine(accounts, nil, &stubInventory{}).Exercise(context.Background(), exerciseRequest()) + require.Error(t, err) + assert.Empty(t, accounts.quiesces, "a refused exercise quiesces nothing") +} + +// ADR-0003:S7: the exercise is an automated actor that deletes customer-visible +// records, so it acts only on kinds whose quiescence was recorded under a confirmed +// fence, and a failed cleanup leaves the account where it is. +func TestS7_ExerciseCleansNothingUnlessItsQuiescenceWasRecordedUnderAConfirmedFence(t *testing.T) { + cases := map[string]func(*Engine, *fakeAccounts) *sequencedInventory{ + "read-back fails": func(e *Engine, a *fakeAccounts) *sequencedInventory { + e.Accounts = &readbackFails{fakeAccounts: a} + return &sequencedInventory{replies: []ec2v1.PlatformInventoryReply{*completeInventory()}} + }, + "inventory incomplete": func(*Engine, *fakeAccounts) *sequencedInventory { + return &sequencedInventory{replies: []ec2v1.PlatformInventoryReply{ + ec2v1.IncompletePlatformInventory(testAccountID, "op-1", ec2v1.PlatformReasonNoResponder), + }} + }, + } + for name, setup := range cases { + t.Run(name, func(t *testing.T) { + accounts := newFakeAccounts() + cleaner := &stubCleaner{result: cleanResult()} + engine := exerciseEngine(accounts, cleaner, nil) + engine.ec2Inventory = setup(engine, accounts) + + _, err := engine.Exercise(context.Background(), exerciseRequest()) + + require.NoError(t, err) + assert.Empty(t, cleaner.calls, "no kind may be cleaned without a recorded quiescence") + assert.False(t, accounts.deleted) + }) + } + + t.Run("only the kinds with a complete inventory are cleaned", func(t *testing.T) { + accounts := newFakeAccounts() + cleaner := &stubCleaner{result: cleanResult()} + partial := completeInventory() + partial.Kinds[1].Status = ec2v1.PlatformStatusScanIncomplete + engine := exerciseEngine(accounts, cleaner, &sequencedInventory{replies: []ec2v1.PlatformInventoryReply{*partial}}) + + _, err := engine.Exercise(context.Background(), exerciseRequest()) + + require.NoError(t, err) + require.Len(t, cleaner.calls, 1) + assert.ElementsMatch(t, []string{ec2v1.PlatformKindKeyPair, ec2v1.PlatformKindLaunchTemplate}, cleaner.calls[0].Kinds) + }) + + t.Run("a failed cleanup returns an error and removes nothing", func(t *testing.T) { + accounts := newFakeAccounts() + engine := exerciseEngine(accounts, &stubCleaner{err: errors.New("owner down")}, &sequencedInventory{replies: []ec2v1.PlatformInventoryReply{*completeInventory()}}) + + _, err := engine.Exercise(context.Background(), exerciseRequest()) + + require.Error(t, err) + assert.False(t, accounts.deleted) + }) +} + +// ADR-0003:S7: an ordinary teardown never runs the platform cleanup and never +// records a quiesced scope, whatever its inventory says. +func TestS7_OrdinaryTeardownNeverCleansOrRecordsAScope(t *testing.T) { + accounts := newFakeAccounts() + cleaner := &stubCleaner{result: cleanResult()} + engine := exerciseEngine(accounts, cleaner, &sequencedInventory{replies: []ec2v1.PlatformInventoryReply{*completeInventory()}}) + var recorded operatorv1.IAMQuiescence + engine.OnQuiesced = func(_ context.Context, q operatorv1.IAMQuiescence) { recorded = q } + + _, err := engine.Teardown(context.Background(), exerciseRequest()) + + require.NoError(t, err) + assert.Empty(t, cleaner.calls) + assert.Empty(t, recorded.QuiescedScope) +} + +func TestInstanceReferencesSkipTerminalInstancesAndNameKeysAndGroups(t *testing.T) { + refs := instanceReferences(&ec2.DescribeInstancesOutput{Reservations: []*ec2.Reservation{{Instances: []*ec2.Instance{ + instanceWith("running", "kp-a", "pg-a", "pg-1"), + instanceWith("stopped", "kp-b", "", ""), + instanceWith("terminated", "kp-gone", "pg-gone", "pg-9"), + }}}}) + + assert.Equal(t, map[string]bool{"kp-a": true, "kp-b": true}, refs.KeyNames) + assert.Equal(t, map[string]bool{"pg-a": true}, refs.PlacementGroupNames) + assert.Equal(t, map[string]bool{"pg-1": true}, refs.PlacementGroupIDs) +} + +func instanceWith(state, key, groupName, groupID string) *ec2.Instance { + return &ec2.Instance{ + State: &ec2.InstanceState{Name: aws.String(state)}, + KeyName: aws.String(key), + Placement: &ec2.Placement{GroupName: aws.String(groupName), GroupId: aws.String(groupID)}, + } +} diff --git a/spinifex/operator/accountlifecycle/quiescence.go b/spinifex/operator/accountlifecycle/quiescence.go index 833b473b6..88cefa246 100644 --- a/spinifex/operator/accountlifecycle/quiescence.go +++ b/spinifex/operator/accountlifecycle/quiescence.go @@ -7,17 +7,17 @@ import ( operatorv1 "github.com/mulgadc/spinifex/contracts/operator/v1" ) -// finalInventoryRereadWired says whether anything re-reads the owner inventory -// before the account record goes. Until it does, a write landing after the -// reaper stage is orphaned, so no kind may be recorded as quiesced. -var finalInventoryRereadWired = false - // QuiesceEvidence is what the engine read after QuiesceAccount returned. type QuiesceEvidence struct { OperationID string Fence uint64 Generation uint64 + // Exercise is set only for a non-finalizing run, which re-reads the owner + // inventory after its cleanup and removes no account record. An ordinary + // delete re-reads nothing before the record goes, so it records no scope. + Exercise bool + // Account is the IAM record read back after the quiesce. Account *Account ReadbackErr error @@ -79,7 +79,8 @@ var otherScopes = []unresolvedEntry{ // EvaluateQuiescence applies ADR-0008:S9's three conditions to the evidence and // records what holds. A fence the read-back does not confirm stops everything -// else; otherwise every scope is listed, and none is reported quiesced yet. +// else; otherwise every scope is listed. A kind is quiesced only for an +// exercise whose owner inventory for it was complete. func EvaluateQuiescence(ev QuiesceEvidence) operatorv1.IAMQuiescence { out := operatorv1.IAMQuiescence{ OperationID: ev.OperationID, @@ -109,7 +110,7 @@ func EvaluateQuiescence(ev QuiesceEvidence) operatorv1.IAMQuiescence { switch { case !complete: out.Unresolved = append(out.Unresolved, writerEntry("ec2-"+kind, operatorv1.ReasonOwnerInventoryIncomplete)) - case !finalInventoryRereadWired: + case !ev.Exercise: out.Unresolved = append(out.Unresolved, writerEntry("ec2-"+kind, operatorv1.ReasonFinalInventoryRereadNotWired)) default: out.QuiescedScope = append(out.QuiescedScope, kind) diff --git a/spinifex/operator/accountlifecycle/quiescence_test.go b/spinifex/operator/accountlifecycle/quiescence_test.go index 55be2b79f..cd676cf97 100644 --- a/spinifex/operator/accountlifecycle/quiescence_test.go +++ b/spinifex/operator/accountlifecycle/quiescence_test.go @@ -72,12 +72,10 @@ func TestEvaluateQuiescenceRecordsEvidenceButQuiescesNothingWithoutAFinalReread( assert.Equal(t, "2026-11-02T00:00:00Z", q.EvaluatedAt) } -// The wiring step flips this switch; the complete kinds are then, and only then, named. +// Only an exercise, which re-reads the inventory after cleanup, names the complete kinds. func TestEvaluateQuiescenceNamesCompleteKindsOnceTheRereadIsWired(t *testing.T) { - finalInventoryRereadWired = true - t.Cleanup(func() { finalInventoryRereadWired = false }) - ev := confirmedEvidence() + ev.Exercise = true ev.Inventory.Kinds[1].Status = ec2v1.PlatformStatusScanIncomplete q := EvaluateQuiescence(ev) @@ -133,9 +131,8 @@ func TestEvaluateQuiescenceRecordsNothingWhenTheFenceIsNotConfirmed(t *testing.T } for name, tc := range cases { t.Run(name, func(t *testing.T) { - finalInventoryRereadWired = true - t.Cleanup(func() { finalInventoryRereadWired = false }) ev := confirmedEvidence() + ev.Exercise = true tc.mutate(&ev) q := EvaluateQuiescence(ev) @@ -172,11 +169,11 @@ func TestEveryOtherScopeIsRecordedUnresolvedByName(t *testing.T) { // Predastore has no accepted enforcement contract, so the scopes it admits writes // for stay unresolved and it is never cited as a consumer, even once the -// re-read is wired and every L09 kind is complete. +// run is an exercise and every L09 kind is complete. func TestPredastoreIsNeverCitedAndItsScopesNeverQuiesce(t *testing.T) { - finalInventoryRereadWired = true - t.Cleanup(func() { finalInventoryRereadWired = false }) - q := EvaluateQuiescence(confirmedEvidence()) + ev := confirmedEvidence() + ev.Exercise = true + q := EvaluateQuiescence(ev) for _, scope := range []string{"s3", "ecr"} { got := unresolvedFor(q, scope) diff --git a/spinifex/operator/admin/account_delete.go b/spinifex/operator/admin/account_delete.go index ae2248baa..5b53499e6 100644 --- a/spinifex/operator/admin/account_delete.go +++ b/spinifex/operator/admin/account_delete.go @@ -66,7 +66,10 @@ type accountDeletionJob struct { ClientToken string `json:"clientToken"` State string `json:"state"` Force bool `json:"force"` - StartedAt string `json:"startedAt"` + // NonFinalizing marks an exercise: it quiesces and cleans the quiesced EC2 + // platform kinds, then ends EXERCISED with the account still TERMINATING. + NonFinalizing bool `json:"nonFinalizing,omitempty"` + StartedAt string `json:"startedAt"` // UpdatedAt is the running job's heartbeat. Its absence for long enough is // what lets another gateway take over after a restart. UpdatedAt string `json:"updatedAt"` @@ -88,6 +91,8 @@ type accountDeletionJob struct { // IAM is the quiescence evaluation made under the current fence. A takeover // starts without one and the new holder records its own. IAM *operatorv1.IAMQuiescence `json:"iam,omitempty"` + // Exercise is what a non-finalizing run did after quiescence. + Exercise *operatorv1.ExerciseResult `json:"exercise,omitempty"` } // deletionTakeover records one change of holder. @@ -99,8 +104,9 @@ type deletionTakeover struct { } const ( - takeoverReasonLeaseExpired = "lease_expired" - takeoverReasonFailed = "failed" + takeoverReasonLeaseExpired = "lease_expired" + takeoverReasonFailed = "failed" + takeoverReasonExerciseFinished = "exercise_finished" ) // deleteAccount starts a teardown, or answers a dry run inline. @@ -118,6 +124,11 @@ func (d Deps) deleteAccount(ctx context.Context, body []byte) (any, error) { return nil, errors.New(awserrors.ErrorInternalError) } + if req.NonFinalizing && d.PlatformCleaner == nil { + slog.Error("DeleteAccount: platform cleanup not available") + return nil, errors.New(awserrors.ErrorServiceUnavailable) + } + engine, err := accountteardown.NewClusterEngine(ctx, d.NATS, accountteardown.ClusterNodes{Expected: d.ExpectedNodes, DaemonIDs: d.NodeIDs, GatewayIDs: d.GatewayNodeIDs}, d.Termination, d.BucketStore) if err != nil { slog.Error("DeleteAccount: failed to build teardown engine", "err", err) @@ -262,6 +273,10 @@ func (d Deps) runAccountDeletion( tracker := &deletionTracker{jobs: jobs, job: job, revision: revision, cancel: cancel} engine.OnStage = tracker.stageFinished engine.OnQuiesced = tracker.quiesced + if job.NonFinalizing { + engine.Cleanup = d.PlatformCleaner + engine.OnExercised = tracker.exercised + } go func() { defer cancel() @@ -269,13 +284,19 @@ func (d Deps) runAccountDeletion( stop := tracker.startHeartbeat(ctx) defer stop() - result, err := engine.Teardown(ctx, accountteardown.Request{ + request := accountteardown.Request{ AccountID: job.AccountID, AccountName: job.AccountName, Force: job.Force, OperationID: job.OperationID, Fence: job.Fence, - }) + } + if job.NonFinalizing { + _, err := engine.Exercise(ctx, request) + tracker.finished(ctx, nil, err) + return + } + result, err := engine.Teardown(ctx, request) tracker.finished(ctx, result, err) }() } @@ -308,6 +329,13 @@ func (t *deletionTracker) quiesced(ctx context.Context, quiescence operatorv1.IA t.persist(ctx) } +func (t *deletionTracker) exercised(ctx context.Context, result operatorv1.ExerciseResult) { + t.mu.Lock() + t.job.Exercise = &result + t.mu.Unlock() + t.persist(ctx) +} + func (t *deletionTracker) finished(ctx context.Context, result *accountteardown.Result, err error) { t.mu.Lock() if result != nil { @@ -316,6 +344,8 @@ func (t *deletionTracker) finished(ctx context.Context, result *accountteardown. if err != nil { t.job.State = operatorv1.DeletionStateFailed t.job.Error = err.Error() + } else if t.job.NonFinalizing { + t.job.State = operatorv1.DeletionStateExercised } else { t.job.State = operatorv1.DeletionStateCompleted } @@ -445,8 +475,10 @@ func claimDeletionJob( StartedAt: now.Format(time.RFC3339), UpdatedAt: now.Format(time.RFC3339), Fence: 1, - Holder: holder, - LeaseUntil: now.Add(accountDeletionStale).Format(time.RFC3339), + + NonFinalizing: req.NonFinalizing, + Holder: holder, + LeaseUntil: now.Add(accountDeletionStale).Format(time.RFC3339), } record, err := json.Marshal(job) if err != nil { @@ -478,6 +510,9 @@ func claimDeletionJob( } reason := takeoverReasonFailed + if existing.State == operatorv1.DeletionStateExercised { + reason = takeoverReasonExerciseFinished + } if existing.State == operatorv1.DeletionStateRunning { if !deletionLeaseExpired(existing, now) { return nil, 0, nil, errors.New(awserrors.ErrorOperationInProgress) @@ -512,8 +547,10 @@ func takeOverDeletionJob( Force: req.Force, StartedAt: now.Format(time.RFC3339), UpdatedAt: now.Format(time.RFC3339), - Holder: holder, - LeaseUntil: now.Add(accountDeletionStale).Format(time.RFC3339), + + NonFinalizing: req.NonFinalizing, + Holder: holder, + LeaseUntil: now.Add(accountDeletionStale).Format(time.RFC3339), } if existing.OperationID != "" { job.OperationID = existing.OperationID @@ -557,7 +594,7 @@ func replayDeletionJob( return nil, err } if existing.ClientToken == req.ClientToken { - if existing.AccountName != req.AccountName { + if existing.AccountName != req.AccountName || existing.NonFinalizing != req.NonFinalizing { return nil, errors.New(awserrors.ErrorIdempotentParameterMismatch) } // A RUNNING job whose lease has lapsed has no holder to follow, so the @@ -703,6 +740,9 @@ func validateDeleteAccountRequest(req *operatorv1.DeleteAccountRequest) error { if !accountIDRE.MatchString(req.AccountID) { return errors.New(awserrors.ErrorInvalidParameterValue) } + if req.NonFinalizing && (req.Force || req.DryRun) { + return errors.New(awserrors.ErrorInvalidParameterCombination) + } if req.DryRun { // A dry run deletes nothing, so it needs neither the name confirmation // nor a token to make it idempotent. diff --git a/spinifex/operator/admin/account_delete_exercise_test.go b/spinifex/operator/admin/account_delete_exercise_test.go new file mode 100644 index 000000000..c33cc8383 --- /dev/null +++ b/spinifex/operator/admin/account_delete_exercise_test.go @@ -0,0 +1,133 @@ +package admin + +//test:in-package — the exercise runs through the job record, the takeover and +// the resumer, all of which are unexported. + +import ( + "context" + "encoding/json" + "testing" + "time" + + operatorv1 "github.com/mulgadc/spinifex/contracts/operator/v1" + "github.com/mulgadc/spinifex/spinifex/domains/ec2/accountcleanup" + awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const exerciseToken = "1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d" + +type panicCleaner struct{ calls int } + +func (p *panicCleaner) CleanupPlatform(context.Context, *accountcleanup.Request) (*accountcleanup.Result, error) { + p.calls++ + return &accountcleanup.Result{}, nil +} + +func exerciseBody(t *testing.T, accountID, name, token string, mutate func(*operatorv1.DeleteAccountRequest)) []byte { + t.Helper() + req := operatorv1.DeleteAccountRequest{AccountID: accountID, AccountName: name, ClientToken: token, NonFinalizing: true} + if mutate != nil { + mutate(&req) + } + body, err := json.Marshal(req) + require.NoError(t, err) + return body +} + +func awaitState(t *testing.T, gw Deps, accountID, state string) *accountDeletionJob { + t.Helper() + require.Eventually(t, func() bool { + return describeDeletion(t, gw, accountID).State == state + }, 30*time.Second, 100*time.Millisecond) + return describeDeletion(t, gw, accountID) +} + +// An exercise ends EXERCISED with the account still TERMINATING, and a later +// ordinary delete takes the same operation over to finish it. +func TestNonFinalizingDeleteEndsExercisedAndALaterDeleteTakesItOver(t *testing.T) { + gw := newDeleteAccountGateway(t) + gw.PlatformCleaner = &panicCleaner{} + account := newTenantAccount(t, gw, "tenant@example.com") + + _, err := gw.deleteAccount(t.Context(), exerciseBody(t, account.AccountID, "tenant@example.com", exerciseToken, nil)) + require.NoError(t, err) + + exercised := awaitState(t, gw, account.AccountID, operatorv1.DeletionStateExercised) + require.NotNil(t, exercised.IAM) + require.NotNil(t, exercised.Exercise) + assert.True(t, exercised.NonFinalizing) + assert.NotEmpty(t, exercised.FinishedAt) + assert.Empty(t, exercised.Stages, "no legacy stage runs") + stored, err := gw.IAM.GetAccount(account.AccountID) + require.NoError(t, err) + assert.Equal(t, "TERMINATING", stored.Status) + + _, err = gw.deleteAccount(t.Context(), deleteAccountBody(t, account.AccountID, "tenant@example.com", deleteClientToken)) + require.NoError(t, err) + + done := awaitState(t, gw, account.AccountID, operatorv1.DeletionStateCompleted) + assert.Equal(t, exercised.OperationID, done.OperationID) + assert.Equal(t, uint64(2), done.Fence) + require.Len(t, done.Takeovers, 1) + assert.Equal(t, takeoverReasonExerciseFinished, done.Takeovers[0].Reason) + assert.Nil(t, done.Exercise, "the takeover starts a fresh run") + assert.Equal(t, uint64(2), done.IAM.Fence) + assert.Empty(t, done.IAM.QuiescedScope, "an ordinary delete records no scope") +} + +func TestNonFinalizingIsPartOfTheIdempotencyKeyAndRefusesForce(t *testing.T) { + gw := newDeleteAccountGateway(t) + gw.PlatformCleaner = &panicCleaner{} + account := newTenantAccount(t, gw, "tenant@example.com") + + _, err := gw.deleteAccount(t.Context(), exerciseBody(t, account.AccountID, "tenant@example.com", exerciseToken, + func(r *operatorv1.DeleteAccountRequest) { r.Force = true })) + require.EqualError(t, err, awserrors.ErrorInvalidParameterCombination) + + _, err = gw.deleteAccount(t.Context(), exerciseBody(t, account.AccountID, "tenant@example.com", exerciseToken, nil)) + require.NoError(t, err) + awaitState(t, gw, account.AccountID, operatorv1.DeletionStateExercised) + + _, err = gw.deleteAccount(t.Context(), deleteAccountBody(t, account.AccountID, "tenant@example.com", exerciseToken)) + require.EqualError(t, err, awserrors.ErrorIdempotentParameterMismatch, "the same token cannot turn an exercise into a delete") + + again, err := gw.deleteAccount(t.Context(), exerciseBody(t, account.AccountID, "tenant@example.com", exerciseToken, nil)) + require.NoError(t, err) + assert.Equal(t, operatorv1.DeletionStateExercised, again.(*DeleteAccountResponse).State, "a repeated exercise is answered from the record") +} + +func TestNonFinalizingWithoutACleanupIsRefusedBeforeAnyJobExists(t *testing.T) { + gw := newDeleteAccountGateway(t) + account := newTenantAccount(t, gw, "tenant@example.com") + + _, err := gw.deleteAccount(t.Context(), exerciseBody(t, account.AccountID, "tenant@example.com", exerciseToken, nil)) + + require.EqualError(t, err, awserrors.ErrorServiceUnavailable) + jobs, err := gw.accountDeletionStore(t.Context()) + require.NoError(t, err) + stored, err := readDeletionJob(t.Context(), jobs, account.AccountID) + require.NoError(t, err) + assert.Nil(t, stored) +} + +// A resumed exercise stays an exercise: the resumer must not turn a recorded +// non-finalizing decision into a deletion. +func TestResumerKeepsAnAbandonedExerciseNonFinalizing(t *testing.T) { + gw := newDeleteAccountGateway(t) + gw.PlatformCleaner = &panicCleaner{} + account := newTenantAccount(t, gw, "tenant@example.com") + job := abandonedJob(account.AccountID, "tenant@example.com") + job.NonFinalizing = true + storeDeletionJob(t, gw, job) + + require.Equal(t, 1, gw.resumeDeletions(t.Context(), time.Now().UTC())) + + done := awaitState(t, gw, account.AccountID, operatorv1.DeletionStateExercised) + assert.Equal(t, uint64(2), done.Fence) + assert.True(t, done.NonFinalizing) + stored, err := gw.IAM.GetAccount(account.AccountID) + require.NoError(t, err) + assert.Equal(t, "TERMINATING", stored.Status, "the account record is never removed") +} diff --git a/spinifex/operator/admin/account_delete_resumer.go b/spinifex/operator/admin/account_delete_resumer.go index ab600f62f..92b1b0615 100644 --- a/spinifex/operator/admin/account_delete_resumer.go +++ b/spinifex/operator/admin/account_delete_resumer.go @@ -110,6 +110,8 @@ func (d Deps) resumeDeletion( AccountName: existing.AccountName, ClientToken: existing.ClientToken, Force: existing.Force, + + NonFinalizing: existing.NonFinalizing, } job, newRevision, _, err := takeOverDeletionJob(ctx, jobs, req, d.holderID(), existing, revision, takeoverReasonLeaseExpired, now) if err != nil { diff --git a/spinifex/operator/admin/account_delete_resumer_test.go b/spinifex/operator/admin/account_delete_resumer_test.go index e77363485..94c92c01f 100644 --- a/spinifex/operator/admin/account_delete_resumer_test.go +++ b/spinifex/operator/admin/account_delete_resumer_test.go @@ -45,6 +45,7 @@ func TestS7_DeletionResumerActsOnlyOnAnAcceptedOperationWithoutALiveLease(t *tes }, "failed job": func(j *accountDeletionJob) { j.State = operatorv1.DeletionStateFailed }, "completed job": func(j *accountDeletionJob) { j.State = operatorv1.DeletionStateCompleted }, + "exercised job": func(j *accountDeletionJob) { j.State = operatorv1.DeletionStateExercised }, "legacy record": func(j *accountDeletionJob) { j.OperationID, j.Fence, j.LeaseUntil = "", 0, "" }, } for name, mutate := range cases { diff --git a/spinifex/operator/admin/registration.go b/spinifex/operator/admin/registration.go index 6ffc8fab6..ab112477f 100644 --- a/spinifex/operator/admin/registration.go +++ b/spinifex/operator/admin/registration.go @@ -58,7 +58,10 @@ type Deps struct { IAM iamdomain.IAMService // Termination is the concrete IAM service, the only holder of the capability // to quiesce and remove an account. DeleteAccount needs it. - Termination accountteardown.IAMBackend + Termination accountteardown.IAMBackend + // PlatformCleaner is the in-process EC2 platform cleanup a non-finalizing + // DeleteAccount runs. Nil refuses that request. + PlatformCleaner accountteardown.PlatformCleaner Quota *quota.Service ExpectedNodes int NodeIDs []string diff --git a/spinifex/operator/cli/admin_account_delete.go b/spinifex/operator/cli/admin_account_delete.go index a2c44ffba..c8c67d5fa 100644 --- a/spinifex/operator/cli/admin_account_delete.go +++ b/spinifex/operator/cli/admin_account_delete.go @@ -61,6 +61,8 @@ func init() { accountCmd.AddCommand(accountDeleteCmd) accountDeleteCmd.Flags().Bool("dry-run", false, "Report what would be deleted and change nothing") + accountDeleteCmd.Flags().Bool("non-finalizing", false, "Quiesce and clean the EC2 platform kinds, then stop with the account still TERMINATING") + _ = accountDeleteCmd.Flags().MarkHidden("non-finalizing") accountDeleteCmd.Flags().Bool("force", false, "Escalate past state guards for resources that will not delete") accountDeleteCmd.Flags().Bool("yes", false, "Skip the interactive confirmation (requires --name)") accountDeleteCmd.Flags().String("name", "", "Account name, which must match the stored record") diff --git a/spinifex/operator/cli/admin_account_delete_remote.go b/spinifex/operator/cli/admin_account_delete_remote.go index 67df14a88..efc359ad3 100644 --- a/spinifex/operator/cli/admin_account_delete_remote.go +++ b/spinifex/operator/cli/admin_account_delete_remote.go @@ -24,6 +24,7 @@ var accountDeletePollInterval = 10 * time.Second func runAccountDeleteRemote(cmd *cobra.Command, accountID string) error { dryRun, _ := cmd.Flags().GetBool("dry-run") force, _ := cmd.Flags().GetBool("force") + nonFinalizing, _ := cmd.Flags().GetBool("non-finalizing") assumeYes, _ := cmd.Flags().GetBool("yes") name, _ := cmd.Flags().GetString("name") clientToken, _ := cmd.Flags().GetString("client-token") @@ -66,6 +67,7 @@ func runAccountDeleteRemote(cmd *cobra.Command, accountID string) error { started, err := deleteAccountRemote(ctx, target, operatorv1.DeleteAccountRequest{ AccountID: accountID, AccountName: name, ClientToken: clientToken, Force: force, + NonFinalizing: nonFinalizing, }) if err != nil { var adminErr *adminError @@ -101,6 +103,9 @@ func followAccountDeletion(ctx context.Context, target adminTarget, accountID st case operatorv1.DeletionStateCompleted: fmt.Printf("\nAccount %s deleted.\n", accountID) return nil + case operatorv1.DeletionStateExercised: + fmt.Printf("\nAccount %s exercised and left TERMINATING; a normal delete finishes it.\n", accountID) + return nil case operatorv1.DeletionStateFailed: fmt.Fprintf(os.Stderr, "\nThe account is still TERMINATING so its leftovers keep an owner.\n") return fmt.Errorf("teardown failed: %s", job.Error) diff --git a/spinifex/runtime/roles/awsgw/awsgw.go b/spinifex/runtime/roles/awsgw/awsgw.go index 47957932f..6ac407bc0 100644 --- a/spinifex/runtime/roles/awsgw/awsgw.go +++ b/spinifex/runtime/roles/awsgw/awsgw.go @@ -725,6 +725,9 @@ func launchService(config *config.ClusterConfig) error { SQSWorkerPolling: sqsRelay.WorkerPresent, SQSTenantAPIRegistered: func() bool { return sqsServed(composedServices) }, }) + operatorAdminDeps = withPlatformCleaner(operatorAdminDeps, func() (accountlifecycle.PlatformCleaner, error) { + return platformCleaner(janitorCtx, &nodeConfig, natsConn, js, operatorAdminDeps.NodeIDs) + }) operatorControlPlane := operatoradmin.NewRegistration(operatorAdminDeps) // Every gateway scans for account deletions whose holder died; the fenced // takeover makes any number of them safe. diff --git a/spinifex/runtime/roles/awsgw/compose_test.go b/spinifex/runtime/roles/awsgw/compose_test.go index 0898235b1..8e31d9905 100644 --- a/spinifex/runtime/roles/awsgw/compose_test.go +++ b/spinifex/runtime/roles/awsgw/compose_test.go @@ -1,12 +1,16 @@ package awsgw import ( + "context" + "errors" "go/ast" "go/parser" "go/token" "testing" "github.com/mulgadc/spinifex/spinifex/bootstrap/config" + "github.com/mulgadc/spinifex/spinifex/domains/ec2/accountcleanup" + "github.com/mulgadc/spinifex/spinifex/operator/accountlifecycle" operatoradmin "github.com/mulgadc/spinifex/spinifex/operator/admin" awsapi "github.com/mulgadc/spinifex/spinifex/domains/ecr/awsapi" @@ -88,3 +92,40 @@ func TestAwsgwStartsTheDeletionResumer(t *testing.T) { }) assert.True(t, started, "awsgw.go must call Deps.StartDeletionResumer") } + +type fixedCleaner struct{} + +func (fixedCleaner) CleanupPlatform(context.Context, *accountcleanup.Request) (*accountcleanup.Result, error) { + return &accountcleanup.Result{}, nil +} + +// A non-finalizing DeleteAccount cleans through the in-process owners awsgw +// binds. A cleanup that cannot be built must leave the field nil, which refuses +// the request, rather than bind something half-working. +func TestWithPlatformCleaner_BindsTheCleanupOrLeavesItNil(t *testing.T) { + bound := withPlatformCleaner(operatoradmin.Deps{}, func() (accountlifecycle.PlatformCleaner, error) { return fixedCleaner{}, nil }) + assert.NotNil(t, bound.PlatformCleaner) + + unbound := withPlatformCleaner(operatoradmin.Deps{}, func() (accountlifecycle.PlatformCleaner, error) { + return nil, errors.New("owner store unavailable") + }) + assert.Nil(t, unbound.PlatformCleaner) +} + +// The cleanup is bound where the operator deps are composed, or the exercise is +// refused on every gateway. +func TestAwsgwBindsThePlatformCleanup(t *testing.T) { + file, err := parser.ParseFile(token.NewFileSet(), "awsgw.go", nil, 0) + require.NoError(t, err) + + var bound bool + ast.Inspect(file, func(n ast.Node) bool { + if call, ok := n.(*ast.CallExpr); ok { + if ident, ok := call.Fun.(*ast.Ident); ok && ident.Name == "withPlatformCleaner" { + bound = true + } + } + return true + }) + assert.True(t, bound, "awsgw.go must call withPlatformCleaner") +} diff --git a/spinifex/runtime/roles/awsgw/platform_cleanup.go b/spinifex/runtime/roles/awsgw/platform_cleanup.go new file mode 100644 index 000000000..ef0a8006d --- /dev/null +++ b/spinifex/runtime/roles/awsgw/platform_cleanup.go @@ -0,0 +1,66 @@ +package awsgw + +import ( + "context" + "fmt" + "log/slog" + + "github.com/mulgadc/spinifex/spinifex/bootstrap/config" + "github.com/mulgadc/spinifex/spinifex/domains/ec2/accountcleanup" + ec2key "github.com/mulgadc/spinifex/spinifex/domains/ec2/key" + ec2launchtemplate "github.com/mulgadc/spinifex/spinifex/domains/ec2/launchtemplate" + ec2placementgroup "github.com/mulgadc/spinifex/spinifex/domains/ec2/placementgroup" + ec2spotinstance "github.com/mulgadc/spinifex/spinifex/domains/ec2/spotinstance" + ec2tags "github.com/mulgadc/spinifex/spinifex/domains/ec2/tags" + "github.com/mulgadc/spinifex/spinifex/operator/accountlifecycle" + operatoradmin "github.com/mulgadc/spinifex/spinifex/operator/admin" + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" +) + +// platformCleaner builds the EC2 platform cleanup over the owners' own services +// in this process. It has no NATS subject, so the only caller is the operator +// admin DeleteAccount that holds it. Deleting a key pair, placement group or +// launch template through these owners clears its tag-index entry too. +func platformCleaner(ctx context.Context, node *config.Config, nc *nats.Conn, js jetstream.JetStream, nodeIDs []string) (accountlifecycle.PlatformCleaner, error) { + tagsKV, err := ec2tags.GetOrCreateTagsBucket(ctx, js) + if err != nil { + return nil, fmt.Errorf("tags bucket: %w", err) + } + tags := ec2tags.NewTagsServiceImpl(node, tagsKV) + keys := ec2key.NewKeyServiceImpl(node) + groups, err := ec2placementgroup.NewPlacementGroupServiceImplWithNATS(ctx, node, nc) + if err != nil { + return nil, fmt.Errorf("placement group service: %w", err) + } + templates, err := ec2launchtemplate.NewLaunchTemplateServiceImplWithNATS(ctx, node, nc) + if err != nil { + return nil, fmt.Errorf("launch template service: %w", err) + } + spot, err := ec2spotinstance.NewSpotInstanceServiceImplWithNATS(ctx, node, nc) + if err != nil { + return nil, fmt.Errorf("spot instance service: %w", err) + } + keys.SetCentralTagStore(tags) + groups.SetCentralTagStore(tags) + templates.SetCentralTagStore(tags) + + return accountcleanup.New(keys, groups, templates, tags, accountcleanup.ReferenceReaders{ + Instances: accountlifecycle.NewInstanceReferenceReader(nc, nodeIDs), + Spot: spot, + Templates: templates, + }), nil +} + +// withPlatformCleaner binds the cleanup a non-finalizing DeleteAccount runs. When +// it cannot be built the field stays nil, which refuses that request instead of +// half-running it. +func withPlatformCleaner(d operatoradmin.Deps, build func() (accountlifecycle.PlatformCleaner, error)) operatoradmin.Deps { + cleaner, err := build() + if err != nil { + slog.Error("awsgw: EC2 platform cleanup unavailable", "err", err) + return d + } + d.PlatformCleaner = cleaner + return d +}