diff --git a/spinifex/accountteardown/reapers_ec2_platform_current_behaviour_test.go b/spinifex/accountteardown/reapers_ec2_platform_current_behaviour_test.go new file mode 100644 index 000000000..5a3df5a1a --- /dev/null +++ b/spinifex/accountteardown/reapers_ec2_platform_current_behaviour_test.go @@ -0,0 +1,219 @@ +package accountteardown + +//test:in-package — drives the unexported platform reapers and reuses the +// engine test doubles. + +import ( + "context" + "encoding/base64" + "errors" + "slices" + "strings" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/s3" + "github.com/mulgadc/spinifex/internal/testkit" + "github.com/mulgadc/spinifex/spinifex/bootstrap/config" + 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" + ec2tags "github.com/mulgadc/spinifex/spinifex/domains/ec2/tags" + "github.com/mulgadc/spinifex/spinifex/foundation/messaging/nats" + "github.com/mulgadc/spinifex/spinifex/providers/objectstore" + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The TestCurrentBehaviour_AccountTermination_ tests pin what the platform +// reapers report while owner stores still hold records. They describe current +// behaviour, not the target design; a failure means it changed. + +const platformBucket = "test-bucket" + +// deleteFailingStore fails object deletes its predicate selects. +type deleteFailingStore struct { + *objectstore.MemoryObjectStore + + failDelete func(key string) bool +} + +var _ objectstore.ObjectStore = (*deleteFailingStore)(nil) + +func (s *deleteFailingStore) DeleteObject(ctx context.Context, in *s3.DeleteObjectInput) (*s3.DeleteObjectOutput, error) { + if s.failDelete != nil && s.failDelete(aws.StringValue(in.Key)) { + return nil, errors.New("injected store failure") + } + return s.MemoryObjectStore.DeleteObject(ctx, in) +} + +// platformOwners runs the real key, placement-group, launch-template and tag +// services behind the subjects the platform reapers call. +type platformOwners struct { + nc *nats.Conn + store *deleteFailingStore + keys *ec2key.KeyServiceImpl + groups *ec2placementgroup.PlacementGroupServiceImpl + templates *ec2launchtemplate.LaunchTemplateServiceImpl + tags *ec2tags.TagsServiceImpl + pgKV jetstream.KeyValue + ltKV jetstream.KeyValue + tagsKV jetstream.KeyValue +} + +// serveOwner answers subject with fn, taking the account from X-Account-ID +// as the daemon's handlers do. +func serveOwner[I, O any](t *testing.T, nc *nats.Conn, subject string, fn func(context.Context, *I, string) (*O, error)) { + t.Helper() + sub, err := nc.QueueSubscribe(subject, "spinifex-workers", func(msg *nats.Msg) { + natsmsg.ServeNATSRequestCtx(msg, func(ctx context.Context, in *I) (*O, error) { + return fn(ctx, in, natsmsg.AccountIDFromMsg(msg)) + }) + }) + require.NoError(t, err) + t.Cleanup(func() { _ = sub.Unsubscribe() }) +} + +func newPlatformOwners(t *testing.T) *platformOwners { + t.Helper() + _, nc, js := testutil.StartTestJetStream(t) + ctx := t.Context() + o := &platformOwners{nc: nc, store: &deleteFailingStore{MemoryObjectStore: objectstore.NewMemoryObjectStore()}} + + var err error + o.tagsKV, err = ec2tags.GetOrCreateTagsBucket(ctx, js) + require.NoError(t, err) + cfg := &config.Config{Predastore: config.PredastoreConfig{Bucket: platformBucket}} + o.tags = ec2tags.NewTagsServiceImplWithStore(cfg, o.store, o.tagsKV) + o.keys = ec2key.NewKeyServiceImplWithStore(o.store, platformBucket) + o.keys.SetCentralTagStore(o.tags) + o.groups, err = ec2placementgroup.NewPlacementGroupServiceImplWithNATS(ctx, nil, nc) + require.NoError(t, err) + o.templates, err = ec2launchtemplate.NewLaunchTemplateServiceImplWithNATS(ctx, nil, nc) + require.NoError(t, err) + o.pgKV, err = js.KeyValue(ctx, ec2placementgroup.KVBucketPlacementGroups) + require.NoError(t, err) + o.ltKV, err = js.KeyValue(ctx, ec2launchtemplate.KVBucketLaunchTemplates) + require.NoError(t, err) + + serveOwner(t, nc, "ec2.DescribeKeyPairs", o.keys.DescribeKeyPairs) + serveOwner(t, nc, "ec2.DeleteKeyPair", o.keys.DeleteKeyPair) + serveOwner(t, nc, "ec2.DescribePlacementGroups", o.groups.DescribePlacementGroups) + serveOwner(t, nc, "ec2.DeletePlacementGroup", o.groups.DeletePlacementGroup) + serveOwner(t, nc, "ec2.DescribeLaunchTemplates", o.templates.DescribeLaunchTemplates) + serveOwner(t, nc, "ec2.DeleteLaunchTemplate", o.templates.DeleteLaunchTemplate) + require.NoError(t, nc.Flush()) + return o +} + +func (o *platformOwners) objectKeys(t *testing.T) []string { + t.Helper() + objects, _, err := objectstore.ListAll(t.Context(), o.store, &s3.ListObjectsV2Input{ + Bucket: aws.String(platformBucket), Prefix: aws.String("keys/" + testAccountID + "/"), + }) + require.NoError(t, err) + var keys []string + for _, obj := range objects { + keys = append(keys, aws.StringValue(obj.Key)) + } + return keys +} + +func accountKVKeys(t *testing.T, kv jetstream.KeyValue) []string { + t.Helper() + keys, err := kv.Keys(t.Context()) + if errors.Is(err, jetstream.ErrNoKeysFound) { + return nil + } + require.NoError(t, err) + var mine []string + for _, k := range keys { + if strings.HasPrefix(k, testAccountID+".") { + mine = append(mine, k) + } + } + slices.Sort(mine) + return mine +} + +func platformReapers(nc *nats.Conn) []Reaper { + return []Reaper{&keyPairReaper{nc: nc}, &placementGroupReaper{nc: nc}, &launchTemplateReaper{nc: nc}} +} + +// Pins current behaviour that differs from AWS: a teardown reports the +// platform stage drained and deletes the account record while the owner +// stores still hold key, placement-group, launch-template and tag records. +func TestCurrentBehaviour_AccountTermination_PlatformReapersReportDrainedOverHiddenResidue(t *testing.T) { + o := newPlatformOwners(t) + ctx := testCtx(t) + + // Live items: each is listed, and deleted through its owner's subject. + _, err := o.keys.ImportKeyPair(ctx, &ec2.ImportKeyPairInput{ + KeyName: aws.String("deploy"), + PublicKeyMaterial: []byte("ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl"), + }, testAccountID) + require.NoError(t, err) + pg, err := o.groups.CreatePlacementGroup(ctx, &ec2.CreatePlacementGroupInput{ + GroupName: aws.String("spread"), Strategy: aws.String(ec2.PlacementStrategySpread), + }, testAccountID) + require.NoError(t, err) + lt, err := o.templates.CreateLaunchTemplate(ctx, &ec2.CreateLaunchTemplateInput{ + LaunchTemplateName: aws.String("web"), + LaunchTemplateData: &ec2.RequestLaunchTemplateData{UserData: aws.String(base64.StdEncoding.EncodeToString([]byte("secret")))}, + }, testAccountID) + require.NoError(t, err) + pgID, ltID := aws.StringValue(pg.PlacementGroup.GroupId), aws.StringValue(lt.LaunchTemplate.LaunchTemplateId) + _, err = o.tags.CreateTags(ctx, &ec2.CreateTagsInput{ + Resources: []*string{aws.String(pgID), aws.String(ltID)}, + Tags: []*ec2.Tag{{Key: aws.String("team"), Value: aws.String("blue")}}, + }, testAccountID) + require.NoError(t, err) + + // Residue each owner's Describe skips. + publicKey := "keys/" + testAccountID + "/deploy" + o.store.failDelete = func(key string) bool { return key == publicKey } + _, err = o.store.PutObject(ctx, &s3.PutObjectInput{ + Bucket: aws.String(platformBucket), Key: aws.String("keys/" + testAccountID + "/key-0badbadbadbadbad0.json"), + Body: strings.NewReader("{not json"), + }) + require.NoError(t, err) + _, err = o.pgKV.Put(ctx, testAccountID+".broken", []byte("{not json")) + require.NoError(t, err) + orphanBody := testAccountID + ".lt-0orphan000000000.v1" + _, err = o.ltKV.Put(ctx, orphanBody, []byte(`{"version_number":1,"data":{"UserData":"c2VjcmV0"}}`)) + require.NoError(t, err) + + accounts := newFakeAccounts() + result, err := quietEngine(accounts, platformReapers(o.nc)...).Teardown(ctx, request()) + require.NoError(t, err) + assert.True(t, result.AccountDeleted) + assert.True(t, accounts.deleted) + assert.Zero(t, result.StuckCount()) + + var deleted []string + for _, stage := range result.Stages { + for _, r := range stage.Deleted { + deleted = append(deleted, r.Kind+"/"+r.ID) + } + } + assert.ElementsMatch(t, []string{"key-pair/deploy", "placement-group/spread", "launch-template/" + ltID}, deleted) + + for _, reaper := range platformReapers(o.nc) { + found, err := reaper.List(ctx, testAccountID) + require.NoError(t, err) + assert.Empty(t, found, "%s lists nothing", reaper.Kind()) + } + + assert.ElementsMatch(t, []string{publicKey, "keys/" + testAccountID + "/key-0badbadbadbadbad0.json"}, o.objectKeys(t)) + assert.Equal(t, []string{testAccountID + ".broken"}, accountKVKeys(t, o.pgKV)) + assert.Equal(t, []string{orphanBody}, accountKVKeys(t, o.ltKV)) + assert.Equal(t, sortedStrings(testAccountID+"."+pgID, testAccountID+"."+ltID), accountKVKeys(t, o.tagsKV)) +} + +func sortedStrings(s ...string) []string { + slices.Sort(s) + return s +} diff --git a/spinifex/daemon/platform_disclosure_current_behaviour_test.go b/spinifex/daemon/platform_disclosure_current_behaviour_test.go new file mode 100644 index 000000000..8ccb88b7d --- /dev/null +++ b/spinifex/daemon/platform_disclosure_current_behaviour_test.go @@ -0,0 +1,252 @@ +package daemon + +//test:in-package — the evidence is the daemon's own subscription table, +// registered by the unexported subscribeAll. + +import ( + "bytes" + "encoding/json" + "slices" + "strings" + "testing" + "time" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/s3" + "github.com/mulgadc/spinifex/internal/testkit" + "github.com/mulgadc/spinifex/spinifex/bootstrap/config" + 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" + ec2tags "github.com/mulgadc/spinifex/spinifex/domains/ec2/tags" + "github.com/mulgadc/spinifex/spinifex/providers/objectstore" + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The TestCurrentBehaviour_AccountTermination_ tests pin what any NATS client +// can read through the daemon's existing per-kind EC2 subjects today. They +// describe current behaviour, not the target design; a failure means it changed. + +const ( + disclosureAccount = "111122223333" + disclosureOther = "444455556666" + disclosureBucket = "test-bucket" +) + +// disclosureCluster is a daemon serving its real subscription table, the +// stores behind it, and a separate NATS client that holds no IAM credential. +type disclosureCluster struct { + caller *nats.Conn + store *objectstore.MemoryObjectStore + js jetstream.JetStream +} + +func newDisclosureCluster(t *testing.T) *disclosureCluster { + t.Helper() + ns, nc, js := testutil.StartTestJetStream(t) + d := newSubscribeTestDaemon(t, nc, "", "") + + store := objectstore.NewMemoryObjectStore() + cfg := &config.Config{Predastore: config.PredastoreConfig{Bucket: disclosureBucket}} + tagsKV, err := ec2tags.GetOrCreateTagsBucket(t.Context(), js) + require.NoError(t, err) + d.keyService = ec2key.NewKeyServiceImplWithStore(store, disclosureBucket) + d.tagsService = ec2tags.NewTagsServiceImplWithStore(cfg, store, tagsKV) + d.keyService.SetCentralTagStore(d.tagsService) + d.placementGroupService, err = ec2placementgroup.NewPlacementGroupServiceImplWithNATS(t.Context(), nil, nc) + require.NoError(t, err) + d.launchTemplateService, err = ec2launchtemplate.NewLaunchTemplateServiceImplWithNATS(t.Context(), nil, nc) + require.NoError(t, err) + require.NoError(t, d.subscribeAll()) + + caller, err := nats.Connect(ns.ClientURL()) + require.NoError(t, err) + t.Cleanup(caller.Close) + return &disclosureCluster{caller: caller, store: store, js: js} +} + +// call sends one request with X-Account-ID set to account, the only account +// selector the subjects read, and decodes a successful reply into out. +func (c *disclosureCluster) call(t *testing.T, subject, account string, in, out any) { + t.Helper() + data, err := json.Marshal(in) + require.NoError(t, err) + reply, err := natsRequestAs(c.caller, subject, data, account, 5*time.Second) + require.NoError(t, err) + require.Empty(t, replyErrCode(t, reply.Data), "%s answered %s", subject, reply.Data) + if out != nil { + require.NoError(t, json.Unmarshal(reply.Data, out)) + } +} + +// platformIDs is the account-scoped identity set each Describe subject returns. +type platformIDs struct { + keyPairs, placementGroups, launchTemplates, taggedResources []string +} + +// seededAccount records the identities created for one account. +type seededAccount struct { + keyName, keyID, groupName, groupID, templateName, templateID string +} + +func (c *disclosureCluster) seed(t *testing.T, account, suffix string) seededAccount { + t.Helper() + tag := []*ec2.Tag{{Key: aws.String("owner"), Value: aws.String(suffix)}} + + var key ec2.CreateKeyPairOutput + c.call(t, "ec2.CreateKeyPair", account, &ec2.CreateKeyPairInput{ + KeyName: aws.String("key-" + suffix), + TagSpecifications: []*ec2.TagSpecification{{ResourceType: aws.String("key-pair"), Tags: tag}}, + }, &key) + var pg ec2.CreatePlacementGroupOutput + c.call(t, "ec2.CreatePlacementGroup", account, &ec2.CreatePlacementGroupInput{ + GroupName: aws.String("pg-" + suffix), Strategy: aws.String(ec2.PlacementStrategySpread), + }, &pg) + var lt ec2.CreateLaunchTemplateOutput + c.call(t, "ec2.CreateLaunchTemplate", account, &ec2.CreateLaunchTemplateInput{ + LaunchTemplateName: aws.String("lt-" + suffix), + LaunchTemplateData: &ec2.RequestLaunchTemplateData{ImageId: aws.String("ami-123")}, + }, <) + s := seededAccount{ + keyName: aws.StringValue(key.KeyName), keyID: aws.StringValue(key.KeyPairId), + groupName: aws.StringValue(pg.PlacementGroup.GroupName), groupID: aws.StringValue(pg.PlacementGroup.GroupId), + templateName: aws.StringValue(lt.LaunchTemplate.LaunchTemplateName), templateID: aws.StringValue(lt.LaunchTemplate.LaunchTemplateId), + } + c.call(t, "ec2.CreateTags", account, &ec2.CreateTagsInput{ + Resources: []*string{aws.String(s.groupID), aws.String(s.templateID)}, Tags: tag, + }, nil) + return s +} + +// describe reads every platform identity the four Describe subjects return +// for account, as "/" pairs (tag rows as ""). +func (c *disclosureCluster) describe(t *testing.T, account string) platformIDs { + t.Helper() + var ids platformIDs + + var keys ec2.DescribeKeyPairsOutput + c.call(t, "ec2.DescribeKeyPairs", account, &ec2.DescribeKeyPairsInput{}, &keys) + for _, kp := range keys.KeyPairs { + ids.keyPairs = append(ids.keyPairs, aws.StringValue(kp.KeyPairId)+"/"+aws.StringValue(kp.KeyName)) + } + var groups ec2.DescribePlacementGroupsOutput + c.call(t, "ec2.DescribePlacementGroups", account, &ec2.DescribePlacementGroupsInput{}, &groups) + for _, g := range groups.PlacementGroups { + ids.placementGroups = append(ids.placementGroups, aws.StringValue(g.GroupId)+"/"+aws.StringValue(g.GroupName)) + } + var templates ec2.DescribeLaunchTemplatesOutput + c.call(t, "ec2.DescribeLaunchTemplates", account, &ec2.DescribeLaunchTemplatesInput{}, &templates) + for _, lt := range templates.LaunchTemplates { + ids.launchTemplates = append(ids.launchTemplates, aws.StringValue(lt.LaunchTemplateId)+"/"+aws.StringValue(lt.LaunchTemplateName)) + } + var tags ec2.DescribeTagsOutput + c.call(t, "ec2.DescribeTags", account, &ec2.DescribeTagsInput{}, &tags) + for _, row := range tags.Tags { + if id := aws.StringValue(row.ResourceId); !slices.Contains(ids.taggedResources, id) { + ids.taggedResources = append(ids.taggedResources, id) + } + } + for _, s := range [][]string{ids.keyPairs, ids.placementGroups, ids.launchTemplates, ids.taggedResources} { + slices.Sort(s) + } + return ids +} + +func sortedIDs(ids ...string) []string { + slices.Sort(ids) + return ids +} + +// A NATS client with no IAM credential chooses the account in X-Account-ID +// and receives that account's key pair, placement group and launch template +// IDs and names, and every tagged key-, pg- and lt- ID, scoped to it alone. +func TestCurrentBehaviour_AccountTermination_DirectNATSCallerReadsAnyAccountsPlatformIdentities(t *testing.T) { + c := newDisclosureCluster(t) + a := c.seed(t, disclosureAccount, "a") + b := c.seed(t, disclosureOther, "b") + + for _, tc := range []struct { + account string + s seededAccount + }{{disclosureAccount, a}, {disclosureOther, b}} { + got := c.describe(t, tc.account) + assert.Equal(t, []string{tc.s.keyID + "/" + tc.s.keyName}, got.keyPairs, "key pairs of %s", tc.account) + assert.Equal(t, []string{tc.s.groupID + "/" + tc.s.groupName}, got.placementGroups, "placement groups of %s", tc.account) + assert.Equal(t, []string{tc.s.templateID + "/" + tc.s.templateName}, got.launchTemplates, "launch templates of %s", tc.account) + assert.Equal(t, sortedIDs(tc.s.keyID, tc.s.groupID, tc.s.templateID), got.taggedResources, "tag index of %s", tc.account) + } + + // Without the header the subjects scope to the empty account and return nothing. + assert.Equal(t, platformIDs{}, c.describe(t, "")) +} + +// Records the owner stores hold but Describe skips are not reachable through +// the per-kind subjects: orphan and undecodable key-pair objects, undecodable +// placement-group records, and launch-template bodies or headers with no live template. +func TestCurrentBehaviour_AccountTermination_DirectNATSCallerCannotSeeHiddenResidue(t *testing.T) { + c := newDisclosureCluster(t) + c.seed(t, disclosureAccount, "a") + before := c.describe(t, disclosureAccount) + + prefix := "keys/" + disclosureAccount + "/" + for key, body := range map[string]string{ + prefix + "orphan-public-key": "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl\n", + prefix + "key-0badbadbadbadbad0.json": "{not json", + } { + _, err := c.store.PutObject(t.Context(), &s3.PutObjectInput{ + Bucket: aws.String(disclosureBucket), Key: aws.String(key), Body: bytes.NewReader([]byte(body)), + }) + require.NoError(t, err) + } + pgKV, err := c.js.KeyValue(t.Context(), ec2placementgroup.KVBucketPlacementGroups) + require.NoError(t, err) + _, err = pgKV.Put(t.Context(), disclosureAccount+".broken", []byte("{not json")) + require.NoError(t, err) + ltKV, err := c.js.KeyValue(t.Context(), ec2launchtemplate.KVBucketLaunchTemplates) + require.NoError(t, err) + for key, body := range map[string]string{ + disclosureAccount + ".lt-0orphan000000000.v1": `{"version_number":1,"data":{"UserData":"c2VjcmV0"}}`, + disclosureAccount + ".name.6f727068616e": "lt-0orphan000000000", + disclosureAccount + ".lt-0badbadbadbadbad0": "{not json", + disclosureAccount + ".lt-0badbadbadbadbad0.v1": `{"version_number":1}`, + } { + _, err := ltKV.Put(t.Context(), key, []byte(body)) + require.NoError(t, err) + } + + after := c.describe(t, disclosureAccount) + assert.Equal(t, before.keyPairs, after.keyPairs) + assert.Equal(t, before.placementGroups, after.placementGroups) + assert.Equal(t, before.launchTemplates, after.launchTemplates) + assert.Equal(t, before.taggedResources, after.taggedResources) +} + +// Pins current behaviour that differs from AWS: deleting a placement group or +// launch template leaves its tag-index entry, which DescribeTags keeps +// returning with its values. A key-pair delete clears its entry. +func TestCurrentBehaviour_AccountTermination_PlatformDeletesLeaveTagIndexEntries(t *testing.T) { + c := newDisclosureCluster(t) + s := c.seed(t, disclosureAccount, "a") + + c.call(t, "ec2.DeleteKeyPair", disclosureAccount, &ec2.DeleteKeyPairInput{KeyName: aws.String(s.keyName)}, nil) + c.call(t, "ec2.DeletePlacementGroup", disclosureAccount, &ec2.DeletePlacementGroupInput{GroupName: aws.String(s.groupName)}, nil) + c.call(t, "ec2.DeleteLaunchTemplate", disclosureAccount, &ec2.DeleteLaunchTemplateInput{LaunchTemplateId: aws.String(s.templateID)}, nil) + + got := c.describe(t, disclosureAccount) + assert.Empty(t, got.keyPairs) + assert.Empty(t, got.placementGroups) + assert.Empty(t, got.launchTemplates) + assert.Equal(t, sortedIDs(s.groupID, s.templateID), got.taggedResources) + + var tags ec2.DescribeTagsOutput + c.call(t, "ec2.DescribeTags", disclosureAccount, &ec2.DescribeTagsInput{}, &tags) + for _, row := range tags.Tags { + assert.Equal(t, "owner", aws.StringValue(row.Key)) + assert.Equal(t, "a", aws.StringValue(row.Value)) + assert.True(t, strings.HasPrefix(aws.StringValue(row.ResourceId), "pg-") || strings.HasPrefix(aws.StringValue(row.ResourceId), "lt-")) + } +} diff --git a/spinifex/domains/ec2/key/account_termination_current_behaviour_test.go b/spinifex/domains/ec2/key/account_termination_current_behaviour_test.go new file mode 100644 index 000000000..2421cc777 --- /dev/null +++ b/spinifex/domains/ec2/key/account_termination_current_behaviour_test.go @@ -0,0 +1,233 @@ +package key_test + +import ( + "context" + "errors" + "io" + "slices" + "strings" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/s3" + ec2key "github.com/mulgadc/spinifex/spinifex/domains/ec2/key" + awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" + "github.com/mulgadc/spinifex/spinifex/providers/objectstore" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The TestCurrentBehaviour_AccountTermination_ tests pin how key pairs are +// stored and what a delete or Describe leaves behind today. They describe +// current behaviour, not the target design; a failure means it changed. + +const ( + ctBucket = "test-bucket" + ctAccount = "111122223333" + ctPubKey = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl" +) + +var errInjected = errors.New("injected store failure") + +// faultyStore fails the operations its predicates select and passes the rest +// to an in-memory store, standing in for a partial object-store outage. +type faultyStore struct { + *objectstore.MemoryObjectStore + + failGet func(key string) bool + failPut func(key string) bool + failDelete func(key string) bool +} + +var _ objectstore.ObjectStore = (*faultyStore)(nil) + +func newFaultyStore() *faultyStore { + return &faultyStore{MemoryObjectStore: objectstore.NewMemoryObjectStore()} +} + +func (f *faultyStore) GetObject(ctx context.Context, in *s3.GetObjectInput) (*s3.GetObjectOutput, error) { + if f.failGet != nil && f.failGet(aws.StringValue(in.Key)) { + return nil, errInjected + } + return f.MemoryObjectStore.GetObject(ctx, in) +} + +func (f *faultyStore) PutObject(ctx context.Context, in *s3.PutObjectInput) (*s3.PutObjectOutput, error) { + if f.failPut != nil && f.failPut(aws.StringValue(in.Key)) { + return nil, errInjected + } + return f.MemoryObjectStore.PutObject(ctx, in) +} + +func (f *faultyStore) DeleteObject(ctx context.Context, in *s3.DeleteObjectInput) (*s3.DeleteObjectOutput, error) { + if f.failDelete != nil && f.failDelete(aws.StringValue(in.Key)) { + return nil, errInjected + } + return f.MemoryObjectStore.DeleteObject(ctx, in) +} + +// accountObjects lists every object key under the account's key prefix. +func accountObjects(t *testing.T, store objectstore.ObjectStore, account string) []string { + t.Helper() + objects, _, err := objectstore.ListAll(context.Background(), store, &s3.ListObjectsV2Input{ + Bucket: aws.String(ctBucket), + Prefix: aws.String("keys/" + account + "/"), + }) + require.NoError(t, err) + var keys []string + for _, obj := range objects { + keys = append(keys, aws.StringValue(obj.Key)) + } + slices.Sort(keys) + return keys +} + +func readObject(t *testing.T, store objectstore.ObjectStore, key string) string { + t.Helper() + out, err := store.GetObject(context.Background(), &s3.GetObjectInput{Bucket: aws.String(ctBucket), Key: aws.String(key)}) + require.NoError(t, err) + defer out.Body.Close() + body, err := io.ReadAll(out.Body) + require.NoError(t, err) + return string(body) +} + +func putObject(t *testing.T, store objectstore.ObjectStore, key, body string) { + t.Helper() + _, err := store.PutObject(context.Background(), &s3.PutObjectInput{ + Bucket: aws.String(ctBucket), Key: aws.String(key), Body: strings.NewReader(body), + }) + require.NoError(t, err) +} + +func importKey(t *testing.T, svc *ec2key.KeyServiceImpl, account, name string) *ec2.ImportKeyPairOutput { + t.Helper() + out, err := svc.ImportKeyPair(context.Background(), &ec2.ImportKeyPairInput{ + KeyName: aws.String(name), PublicKeyMaterial: []byte(ctPubKey), + }, account) + require.NoError(t, err) + return out +} + +func describedKeyNames(t *testing.T, svc *ec2key.KeyServiceImpl, account string) []string { + t.Helper() + out, err := svc.DescribeKeyPairs(context.Background(), &ec2.DescribeKeyPairsInput{}, account) + require.NoError(t, err) + var names []string + for _, kp := range out.KeyPairs { + names = append(names, aws.StringValue(kp.KeyName)) + } + slices.Sort(names) + return names +} + +// A key pair is two objects under keys//: the public key by name and +// the metadata by key pair ID. The generated private key is in neither. +func TestCurrentBehaviour_AccountTermination_KeyPairStorageLayout(t *testing.T) { + store := newFaultyStore() + svc := ec2key.NewKeyServiceImplWithStore(store, ctBucket) + + imported := importKey(t, svc, ctAccount, "imported") + created, err := svc.CreateKeyPair(context.Background(), &ec2.CreateKeyPairInput{KeyName: aws.String("created")}, ctAccount) + require.NoError(t, err) + importKey(t, svc, "444455556666", "other-account") + + prefix := "keys/" + ctAccount + "/" + want := []string{ + prefix + "created", + prefix + "imported", + prefix + aws.StringValue(created.KeyPairId) + ".json", + prefix + aws.StringValue(imported.KeyPairId) + ".json", + } + slices.Sort(want) + assert.Equal(t, want, accountObjects(t, store, ctAccount)) + + assert.Contains(t, readObject(t, store, prefix+"imported"), ctPubKey) + meta := readObject(t, store, prefix+aws.StringValue(created.KeyPairId)+".json") + assert.Contains(t, meta, `"KeyName":"created"`) + assert.NotContains(t, meta, "PRIVATE KEY") + for _, key := range accountObjects(t, store, ctAccount) { + assert.NotContains(t, readObject(t, store, key), aws.StringValue(created.KeyMaterial)) + } +} + +// Pins current behaviour that differs from AWS: a failed public-key delete is +// logged, the metadata delete proceeds and the call reports success. The +// leftover object still blocks the name and passes launch validation. +func TestCurrentBehaviour_AccountTermination_DeleteKeyPairLeavesPublicKeyWhenItsDeleteFails(t *testing.T) { + store := newFaultyStore() + svc := ec2key.NewKeyServiceImplWithStore(store, ctBucket) + imported := importKey(t, svc, ctAccount, "deploy") + pubKeyPath := "keys/" + ctAccount + "/deploy" + store.failDelete = func(key string) bool { return key == pubKeyPath } + + out, err := svc.DeleteKeyPair(context.Background(), &ec2.DeleteKeyPairInput{KeyName: aws.String("deploy")}, ctAccount) + require.NoError(t, err) + assert.True(t, aws.BoolValue(out.Return)) + assert.Equal(t, aws.StringValue(imported.KeyPairId), aws.StringValue(out.KeyPairId)) + + assert.Equal(t, []string{pubKeyPath}, accountObjects(t, store, ctAccount), "only the public key object remains") + assert.Empty(t, describedKeyNames(t, svc, ctAccount)) + assert.NoError(t, svc.ValidateKeyPairExists(context.Background(), ctAccount, "deploy")) + + // A retried delete finds no metadata, reports success and removes nothing. + store.failDelete = nil + _, err = svc.DeleteKeyPair(context.Background(), &ec2.DeleteKeyPairInput{KeyName: aws.String("deploy")}, ctAccount) + require.NoError(t, err) + assert.Equal(t, []string{pubKeyPath}, accountObjects(t, store, ctAccount)) + + _, err = svc.CreateKeyPair(context.Background(), &ec2.CreateKeyPairInput{KeyName: aws.String("deploy")}, ctAccount) + require.Error(t, err) + code, _ := awserrors.ResolveErrorCode(err) + assert.Equal(t, awserrors.ErrorInvalidKeyPairDuplicate, code) +} + +// A create whose metadata write fails and whose cleanup delete also fails +// returns an error and leaves the public key object with no metadata. +func TestCurrentBehaviour_AccountTermination_FailedCreateCleanupLeavesPublicKey(t *testing.T) { + store := newFaultyStore() + svc := ec2key.NewKeyServiceImplWithStore(store, ctBucket) + store.failPut = func(key string) bool { return strings.HasSuffix(key, ".json") } + store.failDelete = func(string) bool { return true } + + _, err := svc.CreateKeyPair(context.Background(), &ec2.CreateKeyPairInput{KeyName: aws.String("half")}, ctAccount) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorServerInternal, err.Error()) + + assert.Equal(t, []string{"keys/" + ctAccount + "/half"}, accountObjects(t, store, ctAccount)) + assert.Empty(t, describedKeyNames(t, svc, ctAccount)) +} + +// DescribeKeyPairs reports only metadata it can read and decode, and never a +// public key object on its own. Each record class below is skipped silently +// and the call still succeeds. +func TestCurrentBehaviour_AccountTermination_DescribeKeyPairsHidesUnreadableRecords(t *testing.T) { + prefix := "keys/" + ctAccount + "/" + tests := []struct { + name string + seed func(t *testing.T, store *faultyStore) + }{ + {"public key without metadata", func(t *testing.T, store *faultyStore) { + putObject(t, store, prefix+"orphan", ctPubKey+"\n") + }}, + {"undecodable metadata", func(t *testing.T, store *faultyStore) { + putObject(t, store, prefix+"key-0badbadbadbadbad0.json", "{not json") + }}, + {"metadata the store cannot read", func(t *testing.T, store *faultyStore) { + putObject(t, store, prefix+"key-0unreadable000000.json", `{"KeyName":"hidden","KeyPairId":"key-0unreadable000000"}`) + store.failGet = func(key string) bool { return key == prefix+"key-0unreadable000000.json" } + }}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + store := newFaultyStore() + svc := ec2key.NewKeyServiceImplWithStore(store, ctBucket) + importKey(t, svc, ctAccount, "live") + tc.seed(t, store) + + assert.Len(t, accountObjects(t, store, ctAccount), 3, "the seeded record is in the owner store") + assert.Equal(t, []string{"live"}, describedKeyNames(t, svc, ctAccount)) + }) + } +} diff --git a/spinifex/domains/ec2/launchtemplate/account_termination_current_behaviour_test.go b/spinifex/domains/ec2/launchtemplate/account_termination_current_behaviour_test.go new file mode 100644 index 000000000..19c1b8efd --- /dev/null +++ b/spinifex/domains/ec2/launchtemplate/account_termination_current_behaviour_test.go @@ -0,0 +1,273 @@ +package launchtemplate + +//test:in-package — the faults are injected by wrapping the service's KV +// handle, which is unexported, and the three record shapes under test are private. + +import ( + "context" + "encoding/base64" + "encoding/json" + "errors" + "slices" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" + "github.com/mulgadc/spinifex/spinifex/foundation/state/kvutil" + "github.com/nats-io/nats.go/jetstream" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The TestCurrentBehaviour_AccountTermination_ tests pin how launch templates +// are stored and what Describe and Delete leave behind today. They describe +// current behaviour, not the target design; a failure means it changed. + +var errInjected = errors.New("injected store failure") + +// faultyKV fails the operations its predicates select and passes the rest to +// a real bucket, standing in for a partial KV outage. +type faultyKV struct { + jetstream.KeyValue + + failGet func(key string) bool + failPut func(key string) bool + failDelete func(key string) bool + failKeys func() bool +} + +func (k *faultyKV) Get(ctx context.Context, key string) (jetstream.KeyValueEntry, error) { + if k.failGet != nil && k.failGet(key) { + return nil, errInjected + } + return k.KeyValue.Get(ctx, key) +} + +func (k *faultyKV) Put(ctx context.Context, key string, value []byte) (uint64, error) { + if k.failPut != nil && k.failPut(key) { + return 0, errInjected + } + return k.KeyValue.Put(ctx, key, value) +} + +func (k *faultyKV) Delete(ctx context.Context, key string, opts ...jetstream.KVDeleteOpt) error { + if k.failDelete != nil && k.failDelete(key) { + return errInjected + } + return k.KeyValue.Delete(ctx, key, opts...) +} + +func (k *faultyKV) Keys(ctx context.Context, opts ...jetstream.WatchOpt) ([]string, error) { + if k.failKeys != nil && k.failKeys() { + return nil, errInjected + } + return k.KeyValue.Keys(ctx, opts...) +} + +const ctUserData = "#!/bin/sh\necho tenant-secret\n" + +// createWithUserData creates a template whose version 1 carries user data, +// then adds version 2. +func createWithUserData(t *testing.T, svc *LaunchTemplateServiceImpl, name string) string { + t.Helper() + out, err := svc.CreateLaunchTemplate(context.Background(), &ec2.CreateLaunchTemplateInput{ + LaunchTemplateName: aws.String(name), + LaunchTemplateData: &ec2.RequestLaunchTemplateData{ + ImageId: aws.String("ami-123"), + UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(ctUserData))), + }, + }, testAccountID) + require.NoError(t, err) + id := aws.StringValue(out.LaunchTemplate.LaunchTemplateId) + addVersion(t, svc, id, "t3.large", "1") + return id +} + +func storedKeys(t *testing.T, kv jetstream.KeyValue) []string { + t.Helper() + keys, err := kv.Keys(t.Context()) + require.NoError(t, err) + slices.Sort(keys) + return keys +} + +func sortedKeys(keys ...string) []string { + slices.Sort(keys) + return keys +} + +func storedUserData(t *testing.T, kv jetstream.KeyValue, key string) string { + t.Helper() + entry, err := kv.Get(t.Context(), key) + require.NoError(t, err) + var rec LaunchTemplateVersionRec + require.NoError(t, json.Unmarshal(entry.Value(), &rec)) + require.NotNil(t, rec.Data) + decoded, err := base64.StdEncoding.DecodeString(aws.StringValue(rec.Data.UserData)) + require.NoError(t, err) + return string(decoded) +} + +func describedTemplateIDs(t *testing.T, svc *LaunchTemplateServiceImpl, account string) []string { + t.Helper() + out, err := svc.DescribeLaunchTemplates(context.Background(), &ec2.DescribeLaunchTemplatesInput{}, account) + require.NoError(t, err) + var ids []string + for _, lt := range out.LaunchTemplates { + ids = append(ids, aws.StringValue(lt.LaunchTemplateId)) + } + return ids +} + +// A template is a header .lt-, a name index .name. +// holding the ID, and one body .lt-.v per version. Bodies +// carry the full LaunchTemplateData, user data included. +func TestCurrentBehaviour_AccountTermination_LaunchTemplateStorageLayout(t *testing.T) { + svc := setupTestService(t) + id := createWithUserData(t, svc, "web(1)") + + header := testAccountID + "." + id + name := testAccountID + ".name.776562283129" // hex of "web(1)" + assert.Equal(t, sortedKeys(header, header+".v1", header+".v2", name, kvutil.VersionKey), storedKeys(t, svc.kv)) + + entry, err := svc.kv.Get(t.Context(), name) + require.NoError(t, err) + assert.Equal(t, id, string(entry.Value())) + assert.Equal(t, ctUserData, storedUserData(t, svc.kv, header+".v1")) + assert.Equal(t, ctUserData, storedUserData(t, svc.kv, header+".v2"), "a version copied from v1 keeps its user data") +} + +// Pins current behaviour that differs from AWS: the header delete alone +// decides success, and a failed name-index or version-body cleanup leaves +// those records, user data included, with no header to find them by. +func TestCurrentBehaviour_AccountTermination_DeleteLaunchTemplateLeavesRecordsWhenCleanupFails(t *testing.T) { + tests := []struct { + name string + inject func(kv *faultyKV, header, nameIdx string) + residual func(header, nameIdx string) []string + }{ + { + name: "version body deletes fail", + inject: func(kv *faultyKV, header, _ string) { + kv.failDelete = func(key string) bool { return key == header+".v1" || key == header+".v2" } + }, + residual: func(header, _ string) []string { return []string{header + ".v1", header + ".v2"} }, + }, + { + name: "name index delete fails", + inject: func(kv *faultyKV, _, nameIdx string) { + kv.failDelete = func(key string) bool { return key == nameIdx } + }, + residual: func(_, nameIdx string) []string { return []string{nameIdx} }, + }, + { + name: "version listing fails after the header delete", + inject: func(kv *faultyKV, header, _ string) { + headerGone := false + kv.failDelete = func(key string) bool { + headerGone = headerGone || key == header + return false + } + kv.failKeys = func() bool { return headerGone } + }, + residual: func(header, _ string) []string { return []string{header + ".v1", header + ".v2"} }, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + svc := setupTestService(t) + id := createWithUserData(t, svc, "web") + inner := svc.kv + header, nameIdx := testAccountID+"."+id, nameKey(testAccountID, "web") + faulty := &faultyKV{KeyValue: inner} + tc.inject(faulty, header, nameIdx) + svc.kv = faulty + + out, err := svc.DeleteLaunchTemplate(context.Background(), &ec2.DeleteLaunchTemplateInput{LaunchTemplateId: aws.String(id)}, testAccountID) + require.NoError(t, err) + assert.Equal(t, id, aws.StringValue(out.LaunchTemplate.LaunchTemplateId)) + + svc.kv = inner + want := append(tc.residual(header, nameIdx), kvutil.VersionKey) + assert.Equal(t, sortedKeys(want...), storedKeys(t, inner)) + assert.Empty(t, describedTemplateIDs(t, svc, testAccountID)) + if slices.Contains(want, header+".v1") { + assert.Equal(t, ctUserData, storedUserData(t, inner, header+".v1")) + } + }) + } +} + +// A create interrupted before its header write leaves the name claim and +// version 1 behind. A retry under the same name reclaims the name, and the +// first attempt's body is never reachable again. +func TestCurrentBehaviour_AccountTermination_InterruptedCreateLeavesAnOrphanBody(t *testing.T) { + svc := setupTestService(t) + inner := svc.kv + svc.kv = &faultyKV{KeyValue: inner, failPut: func(key string) bool { return isHeaderKey(key, testAccountID+".") }} + + _, err := svc.CreateLaunchTemplate(context.Background(), &ec2.CreateLaunchTemplateInput{ + LaunchTemplateName: aws.String("web"), + LaunchTemplateData: &ec2.RequestLaunchTemplateData{ + UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(ctUserData))), + }, + }, testAccountID) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorServerInternal, err.Error()) + + svc.kv = inner + keys := storedKeys(t, inner) + require.Len(t, keys, 3, "name index, version 1 and the bucket version key") + nameEntry, err := inner.Get(t.Context(), nameKey(testAccountID, "web")) + require.NoError(t, err) + orphanBody := testAccountID + "." + string(nameEntry.Value()) + ".v1" + assert.Contains(t, keys, orphanBody) + assert.Equal(t, ctUserData, storedUserData(t, inner, orphanBody)) + assert.Empty(t, describedTemplateIDs(t, svc, testAccountID)) + + retried := createTemplate(t, svc, "web", "t3.micro") + assert.NotEqual(t, string(nameEntry.Value()), aws.StringValue(retried.LaunchTemplateId)) + assert.Contains(t, storedKeys(t, inner), orphanBody) + assert.Equal(t, []string{aws.StringValue(retried.LaunchTemplateId)}, describedTemplateIDs(t, svc, testAccountID)) +} + +// Describe skips a header it cannot read or decode, and still succeeds. An +// undecodable header also cannot be removed through DeleteLaunchTemplate. +func TestCurrentBehaviour_AccountTermination_DescribeLaunchTemplatesHidesUnreadableHeaders(t *testing.T) { + const brokenID = "lt-0badbadbadbadbad0" + brokenHeader := kvutil.AccountKey(testAccountID, brokenID) + tests := []struct { + name string + seed func(t *testing.T, svc *LaunchTemplateServiceImpl) + deleteCode string + }{ + {"undecodable header", func(t *testing.T, svc *LaunchTemplateServiceImpl) { + _, err := svc.kv.Put(t.Context(), brokenHeader, []byte("{not json")) + require.NoError(t, err) + }, awserrors.ErrorServerInternal}, + {"header the store cannot read", func(t *testing.T, svc *LaunchTemplateServiceImpl) { + data, err := json.Marshal(LaunchTemplateHeader{LaunchTemplateId: brokenID, LaunchTemplateName: "hidden", AccountID: testAccountID}) + require.NoError(t, err) + _, err = svc.kv.Put(t.Context(), brokenHeader, data) + require.NoError(t, err) + svc.kv = &faultyKV{KeyValue: svc.kv, failGet: func(key string) bool { return key == brokenHeader }} + }, awserrors.ErrorInvalidLaunchTemplateIdNotFound}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + svc := setupTestService(t) + live := createTemplate(t, svc, "live", "t3.micro") + tc.seed(t, svc) + + assert.Contains(t, storedKeys(t, svc.kv), brokenHeader) + assert.Equal(t, []string{aws.StringValue(live.LaunchTemplateId)}, describedTemplateIDs(t, svc, testAccountID)) + + _, err := svc.DeleteLaunchTemplate(context.Background(), &ec2.DeleteLaunchTemplateInput{LaunchTemplateId: aws.String(brokenID)}, testAccountID) + require.Error(t, err) + code, _ := awserrors.ResolveErrorCode(err) + assert.Equal(t, tc.deleteCode, code) + assert.Contains(t, storedKeys(t, svc.kv), brokenHeader) + }) + } +} diff --git a/spinifex/domains/ec2/placementgroup/account_termination_current_behaviour_test.go b/spinifex/domains/ec2/placementgroup/account_termination_current_behaviour_test.go new file mode 100644 index 000000000..72ebb5b94 --- /dev/null +++ b/spinifex/domains/ec2/placementgroup/account_termination_current_behaviour_test.go @@ -0,0 +1,167 @@ +package placementgroup + +//test:in-package — the faults are injected by wrapping the service's KV +// handle, which is unexported, and the record layout under test is private. + +import ( + "context" + "encoding/json" + "errors" + "slices" + "sync" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" + "github.com/mulgadc/spinifex/spinifex/foundation/state/kvutil" + "github.com/nats-io/nats.go/jetstream" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The TestCurrentBehaviour_AccountTermination_ tests pin how placement groups +// are stored and what Describe and Delete do with them today. They describe +// current behaviour, not the target design; a failure means it changed. + +// hookedKV fails reads of failGet and runs afterGet once, after the first read +// of key returns, standing in for a writer on another node. +type hookedKV struct { + jetstream.KeyValue + + failGet string + key string + once sync.Once + afterGet func() +} + +func (k *hookedKV) Get(ctx context.Context, key string) (jetstream.KeyValueEntry, error) { + if key == k.failGet { + return nil, errors.New("injected read failure") + } + entry, err := k.KeyValue.Get(ctx, key) + if key == k.key && k.afterGet != nil { + k.once.Do(k.afterGet) + } + return entry, err +} + +func describedGroupNames(t *testing.T, svc *PlacementGroupServiceImpl, account string) []string { + t.Helper() + out, err := svc.DescribePlacementGroups(context.Background(), &ec2.DescribePlacementGroupsInput{}, account) + require.NoError(t, err) + var names []string + for _, g := range out.PlacementGroups { + names = append(names, aws.StringValue(g.GroupName)) + } + slices.Sort(names) + return names +} + +func bucketKeys(t *testing.T, kv jetstream.KeyValue) []string { + t.Helper() + keys, err := kv.Keys(t.Context()) + require.NoError(t, err) + slices.Sort(keys) + return keys +} + +// One KV entry per group, keyed ., holding the record with +// its tags and member map; the bucket also holds its schema version key. +func TestCurrentBehaviour_AccountTermination_PlacementGroupStorageLayout(t *testing.T) { + svc := setupTestService(t) + out, err := svc.CreatePlacementGroup(context.Background(), &ec2.CreatePlacementGroupInput{ + GroupName: aws.String("spread-a"), + Strategy: aws.String(ec2.PlacementStrategySpread), + TagSpecifications: []*ec2.TagSpecification{{ + ResourceType: aws.String("placement-group"), + Tags: []*ec2.Tag{{Key: aws.String("team"), Value: aws.String("blue")}}, + }}, + }, testAccountID) + require.NoError(t, err) + + assert.Equal(t, []string{testAccountID + ".spread-a", kvutil.VersionKey}, bucketKeys(t, svc.kv)) + + entry, err := svc.kv.Get(t.Context(), testAccountID+".spread-a") + require.NoError(t, err) + var record PlacementGroupRecord + require.NoError(t, json.Unmarshal(entry.Value(), &record)) + assert.Equal(t, aws.StringValue(out.PlacementGroup.GroupId), record.GroupId) + assert.Equal(t, testAccountID, record.AccountID) + assert.Equal(t, map[string]string{"team": "blue"}, record.Tags) + assert.Empty(t, record.NodeInstances) +} + +// Describe skips a record it cannot read or decode, and still succeeds. An +// undecodable record also cannot be removed through DeletePlacementGroup. +func TestCurrentBehaviour_AccountTermination_DescribePlacementGroupsHidesUnreadableRecords(t *testing.T) { + brokenKey := kvutil.AccountKey(testAccountID, "broken") + tests := []struct { + name string + seed func(t *testing.T, svc *PlacementGroupServiceImpl) + deleteCode string + }{ + {"undecodable record", func(t *testing.T, svc *PlacementGroupServiceImpl) { + _, err := svc.kv.Put(t.Context(), brokenKey, []byte("{not json")) + require.NoError(t, err) + }, awserrors.ErrorServerInternal}, + {"record the store cannot read", func(t *testing.T, svc *PlacementGroupServiceImpl) { + createTestGroup(t, svc, "broken", ec2.PlacementStrategySpread) + svc.kv = &hookedKV{KeyValue: svc.kv, failGet: brokenKey} + }, awserrors.ErrorInvalidPlacementGroupUnknown}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + svc := setupTestService(t) + createTestGroup(t, svc, "live", ec2.PlacementStrategySpread) + tc.seed(t, svc) + + assert.Contains(t, bucketKeys(t, svc.kv), brokenKey) + assert.Equal(t, []string{"live"}, describedGroupNames(t, svc, testAccountID)) + + _, err := svc.DeletePlacementGroup(context.Background(), &ec2.DeletePlacementGroupInput{GroupName: aws.String("broken")}, testAccountID) + require.Error(t, err) + code, _ := awserrors.ResolveErrorCode(err) + assert.Equal(t, tc.deleteCode, code) + assert.Contains(t, bucketKeys(t, svc.kv), brokenKey) + }) + } +} + +// Pins current behaviour that differs from AWS: the delete reads the record, +// checks membership, then deletes without a revision guard. A launch that +// records an instance between the two is deleted with the group. +func TestCurrentBehaviour_AccountTermination_DeletePlacementGroupDropsAConcurrentMember(t *testing.T) { + svc := setupTestService(t) + createTestGroup(t, svc, "cluster-a", ec2.PlacementStrategyCluster) + key := kvutil.AccountKey(testAccountID, "cluster-a") + + launcher := &PlacementGroupServiceImpl{kv: svc.kv, region: svc.region} + var reserveErr, finalizeErr error + svc.kv = &hookedKV{KeyValue: svc.kv, key: key, afterGet: func() { + var reserved *ReserveClusterNodeOutput + reserved, reserveErr = launcher.ReserveClusterNode(context.Background(), &ReserveClusterNodeInput{ + GroupName: "cluster-a", EligibleNodes: []string{"node-1"}, + }, testAccountID) + if reserveErr == nil { + _, finalizeErr = launcher.FinalizeClusterInstances(context.Background(), &FinalizeClusterInstancesInput{ + GroupName: "cluster-a", NodeInstances: map[string][]string{reserved.TargetNode: {"i-0123456789abcdef0"}}, + }, testAccountID) + } + }} + + _, err := svc.DeletePlacementGroup(context.Background(), &ec2.DeletePlacementGroupInput{GroupName: aws.String("cluster-a")}, testAccountID) + require.NoError(t, err, "the delete succeeds although the group gained a member") + require.NoError(t, reserveErr) + require.NoError(t, finalizeErr, "the interleaved launch committed its member") + + _, err = launcher.kv.Get(t.Context(), key) + assert.ErrorIs(t, err, jetstream.ErrKeyNotFound) + assert.Empty(t, describedGroupNames(t, launcher, testAccountID)) + + // The member's later shutdown finds no group and reports success. + _, err = launcher.RemoveInstance(context.Background(), &RemoveInstanceInput{ + GroupName: "cluster-a", NodeName: "node-1", InstanceID: "i-0123456789abcdef0", + }, testAccountID) + assert.NoError(t, err) +}