From c473365f1a8121167f52a9e5bbbd0b30735e33c5 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:54:58 +1000 Subject: [PATCH] refactor: move new test files to external test packages Tests added since main used in-package test packages, failing the test-package check. Convert them to _test with export_test.go hooks; the ecs-agent main package keeps an in-package marker. --- cmd/ecs-agent/runtimegate_test.go | 2 + internal/awsmodel/coverage_json_test.go | 24 +-- spinifex/gateway/ec2_describeregions_test.go | 15 +- spinifex/gateway/export_test.go | 21 +++ .../ec2/instance/data_volumes_test.go | 45 +++-- spinifex/handlers/ec2/instance/export_test.go | 53 ++++++ .../handlers/ec2/vpc/central_tags_test.go | 157 ++++++++---------- spinifex/handlers/ec2/vpc/export_test.go | 36 ++++ .../ec2/vpc/security_group_modify_test.go | 6 +- spinifex/handlers/ecs/export_test.go | 35 ++++ .../handlers/ecs/instance_list_status_test.go | 57 +++---- spinifex/handlers/ecs/task_accounting_test.go | 105 ++++++------ 12 files changed, 341 insertions(+), 215 deletions(-) create mode 100644 spinifex/gateway/export_test.go create mode 100644 spinifex/handlers/ec2/instance/export_test.go create mode 100644 spinifex/handlers/ec2/vpc/export_test.go create mode 100644 spinifex/handlers/ecs/export_test.go diff --git a/cmd/ecs-agent/runtimegate_test.go b/cmd/ecs-agent/runtimegate_test.go index 6f639061b..80c5278a9 100644 --- a/cmd/ecs-agent/runtimegate_test.go +++ b/cmd/ecs-agent/runtimegate_test.go @@ -1,3 +1,5 @@ +//test:in-package — package main cannot be imported by an external test package. + package main import ( diff --git a/internal/awsmodel/coverage_json_test.go b/internal/awsmodel/coverage_json_test.go index aeaf6eacb..19a69328f 100644 --- a/internal/awsmodel/coverage_json_test.go +++ b/internal/awsmodel/coverage_json_test.go @@ -1,4 +1,4 @@ -package awsmodel +package awsmodel_test import ( "encoding/json" @@ -6,10 +6,12 @@ import ( "path/filepath" "strings" "testing" + + "github.com/mulgadc/spinifex/internal/awsmodel" ) func TestRenderCoverageJSONIsDeterministicAndPreservesDispatchStates(t *testing.T) { - sts, err := CompareOperations(STS, DispatchInventory{ + sts, err := awsmodel.CompareOperations(awsmodel.STS, awsmodel.DispatchInventory{ Registered: []string{"AssumeRole", "GetCallerIdentity", "GetSessionToken"}, Stubbed: []string{"GetSessionToken"}, Unsupported: []string{"GetCallerIdentity"}, @@ -17,12 +19,12 @@ func TestRenderCoverageJSONIsDeterministicAndPreservesDispatchStates(t *testing. if err != nil { t.Fatal(err) } - iam, err := CompareOperations(IAM, DispatchInventory{}) + iam, err := awsmodel.CompareOperations(awsmodel.IAM, awsmodel.DispatchInventory{}) if err != nil { t.Fatal(err) } - contents, err := RenderCoverageJSON([]OperationCoverage{sts, iam}) + contents, err := awsmodel.RenderCoverageJSON([]awsmodel.OperationCoverage{sts, iam}) if err != nil { t.Fatal(err) } @@ -44,11 +46,11 @@ func TestRenderCoverageJSONIsDeterministicAndPreservesDispatchStates(t *testing. if err := json.Unmarshal([]byte(contents), &document); err != nil { t.Fatalf("generated JSON is invalid: %v\n%s", err, contents) } - if document.SchemaVersion != CoverageJSONSchemaVersion { - t.Errorf("schema version = %d, want %d", document.SchemaVersion, CoverageJSONSchemaVersion) + if document.SchemaVersion != awsmodel.CoverageJSONSchemaVersion { + t.Errorf("schema version = %d, want %d", document.SchemaVersion, awsmodel.CoverageJSONSchemaVersion) } - if document.ModelSource.AWSSDKGoVersion != SourceSDKVersion { - t.Errorf("source version = %q, want %q", document.ModelSource.AWSSDKGoVersion, SourceSDKVersion) + if document.ModelSource.AWSSDKGoVersion != awsmodel.SourceSDKVersion { + t.Errorf("source version = %q, want %q", document.ModelSource.AWSSDKGoVersion, awsmodel.SourceSDKVersion) } if !strings.Contains(contents, `"registered": []`) { t.Errorf("an empty operation set must be an array, not null:\n%s", contents) @@ -72,16 +74,16 @@ func TestRenderCoverageJSONIsDeterministicAndPreservesDispatchStates(t *testing. } func TestWriteCoverageJSONWritesTheSameDocument(t *testing.T) { - coverage, err := CompareOperations(STS, DispatchInventory{Registered: []string{"AssumeRole"}}) + coverage, err := awsmodel.CompareOperations(awsmodel.STS, awsmodel.DispatchInventory{Registered: []string{"AssumeRole"}}) if err != nil { t.Fatal(err) } - want, err := RenderCoverageJSON([]OperationCoverage{coverage}) + want, err := awsmodel.RenderCoverageJSON([]awsmodel.OperationCoverage{coverage}) if err != nil { t.Fatal(err) } path := filepath.Join(t.TempDir(), "coverage.json") - if err := WriteCoverageJSON(path, []OperationCoverage{coverage}); err != nil { + if err := awsmodel.WriteCoverageJSON(path, []awsmodel.OperationCoverage{coverage}); err != nil { t.Fatal(err) } got, err := os.ReadFile(path) diff --git a/spinifex/gateway/ec2_describeregions_test.go b/spinifex/gateway/ec2_describeregions_test.go index ea77af6cb..12e6c2698 100644 --- a/spinifex/gateway/ec2_describeregions_test.go +++ b/spinifex/gateway/ec2_describeregions_test.go @@ -1,8 +1,9 @@ -package gateway +package gateway_test import ( "testing" + "github.com/mulgadc/spinifex/spinifex/gateway" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -26,8 +27,8 @@ func TestAdvertisedEndpoint_ResolvesFromRegistryHost(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - gw := &GatewayConfig{RegistryHost: tc.registryHost, RegistryPort: tc.port} - assert.Equal(t, tc.want, gw.advertisedEndpoint()) + gw := &gateway.GatewayConfig{RegistryHost: tc.registryHost, RegistryPort: tc.port} + assert.Equal(t, tc.want, gw.AdvertisedEndpoint()) }) } } @@ -37,12 +38,8 @@ func TestAdvertisedEndpoint_ResolvesFromRegistryHost(t *testing.T) { // response. A caller on a concrete-host deployment must never be told to // dial its own loopback, which is the defect this guards against. func TestDescribeRegions_UsesAdvertisedEndpoint(t *testing.T) { - h := ec2Actions["DescribeRegions"] - input, err := h.parse(map[string]string{}) - require.NoError(t, err) - - gw := &GatewayConfig{Region: "ap-southeast-2", RegistryHost: "10.0.0.5", RegistryPort: "9999"} - xmlOutput, err := h.dispatch("DescribeRegions", input, gw, "acct-123", nil) + gw := &gateway.GatewayConfig{Region: "ap-southeast-2", RegistryHost: "10.0.0.5", RegistryPort: "9999"} + xmlOutput, err := gateway.DispatchEC2Action(gw, "DescribeRegions", map[string]string{}, "acct-123") require.NoError(t, err) body := string(xmlOutput) diff --git a/spinifex/gateway/export_test.go b/spinifex/gateway/export_test.go new file mode 100644 index 000000000..73472bdac --- /dev/null +++ b/spinifex/gateway/export_test.go @@ -0,0 +1,21 @@ +package gateway + +import "fmt" + +// AdvertisedEndpoint exposes the gateway's own dialable base URL to the +// external test package. +func (gw *GatewayConfig) AdvertisedEndpoint() string { return gw.advertisedEndpoint() } + +// DispatchEC2Action runs an action through the parse-then-dispatch entry +// EC2_Request uses, without an *http.Request. +func DispatchEC2Action(gw *GatewayConfig, action string, query map[string]string, accountID string) ([]byte, error) { + h, ok := ec2Actions[action] + if !ok { + return nil, fmt.Errorf("no EC2 action %q", action) + } + input, err := h.parse(query) + if err != nil { + return nil, err + } + return h.dispatch(action, input, gw, accountID, nil) +} diff --git a/spinifex/handlers/ec2/instance/data_volumes_test.go b/spinifex/handlers/ec2/instance/data_volumes_test.go index f0826191a..e0fff0052 100644 --- a/spinifex/handlers/ec2/instance/data_volumes_test.go +++ b/spinifex/handlers/ec2/instance/data_volumes_test.go @@ -1,4 +1,4 @@ -package handlers_ec2_instance +package handlers_ec2_instance_test import ( "context" @@ -10,7 +10,7 @@ import ( "github.com/mulgadc/bluebottle/pkg/safecast" "github.com/mulgadc/spinifex/spinifex/ebsmetadata" "github.com/mulgadc/spinifex/spinifex/ebsprovider" - "github.com/mulgadc/spinifex/spinifex/objectstore" + handlers_ec2_instance "github.com/mulgadc/spinifex/spinifex/handlers/ec2/instance" "github.com/mulgadc/spinifex/spinifex/vm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -51,12 +51,11 @@ func (f *fakeVolumeCreator) DeleteVolume(_ context.Context, input *ec2.DeleteVol // dataVolumeService builds an instance service with a metadata store and the // fake creator wired in, which is all the data-volume path touches. -func dataVolumeService(t *testing.T, creator *fakeVolumeCreator) *InstanceServiceImpl { +func dataVolumeService(t *testing.T, creator *fakeVolumeCreator) *handlers_ec2_instance.InstanceServiceImpl { t.Helper() - svc := providerRootVolumeService(t, ebsprovider.NewMemoryProvider(ebsprovider.Capabilities{}), - rootVolumeAMILoader(), objectstore.NewMemoryObjectStore()) - creator.metadata = svc.metadata - svc.volumeCreator = creator + svc := handlers_ec2_instance.NewProviderRootVolumeTestService(t) + creator.metadata = svc.MetadataStore() + svc.SetVolumeCreator(creator) return svc } @@ -81,9 +80,9 @@ func dataMappingInput(devices ...string) *ec2.RunInstancesInput { func TestPrepareDataVolumes_CreatesOnePerMapping(t *testing.T) { creator := &fakeVolumeCreator{} svc := dataVolumeService(t, creator) - instance := &vm.VM{AccountID: testRootAccount} + instance := &vm.VM{AccountID: handlers_ec2_instance.TestRootAccount} - infos, err := svc.prepareDataVolumes(context.Background(), dataMappingInput("/dev/sdf", "/dev/sdg"), instance) + infos, err := svc.PrepareDataVolumes(context.Background(), dataMappingInput("/dev/sdf", "/dev/sdg"), instance) require.NoError(t, err) require.Len(t, infos, 2) assert.Equal(t, "/dev/sdf", infos[0].DeviceName) @@ -92,7 +91,7 @@ func TestPrepareDataVolumes_CreatesOnePerMapping(t *testing.T) { require.Len(t, creator.created, 2) assert.Equal(t, int64(4), aws.Int64Value(creator.created[0].Size)) assert.Equal(t, int64(5), aws.Int64Value(creator.created[1].Size)) - assert.Equal(t, testRootAZ, aws.StringValue(creator.created[0].AvailabilityZone)) + assert.Equal(t, handlers_ec2_instance.TestRootAZ, aws.StringValue(creator.created[0].AvailabilityZone)) require.Len(t, instance.EBSRequests.Requests, 2) for i, req := range instance.EBSRequests.Requests { @@ -106,9 +105,9 @@ func TestPrepareDataVolumes_CreatesOnePerMapping(t *testing.T) { func TestPrepareDataVolumes_NoMappingsCreatesNothing(t *testing.T) { creator := &fakeVolumeCreator{} svc := dataVolumeService(t, creator) - instance := &vm.VM{AccountID: testRootAccount} + instance := &vm.VM{AccountID: handlers_ec2_instance.TestRootAccount} - infos, err := svc.prepareDataVolumes(context.Background(), dataMappingInput(), instance) + infos, err := svc.PrepareDataVolumes(context.Background(), dataMappingInput(), instance) require.NoError(t, err) assert.Empty(t, infos) assert.Empty(t, creator.created) @@ -120,9 +119,9 @@ func TestPrepareDataVolumes_NoMappingsCreatesNothing(t *testing.T) { func TestPrepareDataVolumes_UnwindsWhatItCreated(t *testing.T) { creator := &fakeVolumeCreator{failFrom: 1} svc := dataVolumeService(t, creator) - instance := &vm.VM{AccountID: testRootAccount} + instance := &vm.VM{AccountID: handlers_ec2_instance.TestRootAccount} - _, err := svc.prepareDataVolumes(context.Background(), dataMappingInput("/dev/sdf", "/dev/sdg"), instance) + _, err := svc.PrepareDataVolumes(context.Background(), dataMappingInput("/dev/sdf", "/dev/sdg"), instance) require.Error(t, err) assert.Len(t, creator.created, 1) assert.Equal(t, []string{"vol-data-a"}, creator.deleted) @@ -133,27 +132,27 @@ func TestPrepareDataVolumes_UnwindsWhatItCreated(t *testing.T) { func TestPrepareDataVolumes_RecordsDeleteOnTermination(t *testing.T) { creator := &fakeVolumeCreator{} svc := dataVolumeService(t, creator) - instance := &vm.VM{AccountID: testRootAccount} + instance := &vm.VM{AccountID: handlers_ec2_instance.TestRootAccount} input := dataMappingInput("/dev/sdf") input.BlockDeviceMappings[1].Ebs.DeleteOnTermination = aws.Bool(false) - infos, err := svc.prepareDataVolumes(context.Background(), input, instance) + infos, err := svc.PrepareDataVolumes(context.Background(), input, instance) require.NoError(t, err) require.Len(t, infos, 1) assert.False(t, infos[0].DeleteOnTermination) require.Len(t, instance.EBSRequests.Requests, 1) assert.False(t, instance.EBSRequests.Requests[0].DeleteOnTermination) - doc, err := svc.metadata.GetVolume(context.Background(), testRootAccount, infos[0].VolumeId) + doc, err := svc.MetadataStore().GetVolume(context.Background(), handlers_ec2_instance.TestRootAccount, infos[0].VolumeId) require.NoError(t, err) assert.False(t, doc.DeleteOnTermination) kept := &fakeVolumeCreator{} keptSvc := dataVolumeService(t, kept) - keptInfos, err := keptSvc.prepareDataVolumes(context.Background(), dataMappingInput("/dev/sdf"), &vm.VM{AccountID: testRootAccount}) + keptInfos, err := keptSvc.PrepareDataVolumes(context.Background(), dataMappingInput("/dev/sdf"), &vm.VM{AccountID: handlers_ec2_instance.TestRootAccount}) require.NoError(t, err) require.Len(t, keptInfos, 1) - doc, err = keptSvc.metadata.GetVolume(context.Background(), testRootAccount, keptInfos[0].VolumeId) + doc, err = keptSvc.MetadataStore().GetVolume(context.Background(), handlers_ec2_instance.TestRootAccount, keptInfos[0].VolumeId) require.NoError(t, err) assert.True(t, doc.DeleteOnTermination) } @@ -167,9 +166,9 @@ func TestParseDataVolumeParams_SkipsRootAndEphemeral(t *testing.T) { {DeviceName: aws.String("/dev/sdb"), VirtualName: aws.String("ephemeral0")}, }, } - params := parseDataVolumeParams(input) + params := handlers_ec2_instance.ParseDataVolumeParams(input) require.Len(t, params, 1) - assert.Equal(t, "/dev/sdf", params[0].deviceName) - assert.Equal(t, int64(4), params[0].sizeGiB) - assert.True(t, params[0].deleteOnTermination) + assert.Equal(t, "/dev/sdf", params[0].DeviceName) + assert.Equal(t, int64(4), params[0].SizeGiB) + assert.True(t, params[0].DeleteOnTermination) } diff --git a/spinifex/handlers/ec2/instance/export_test.go b/spinifex/handlers/ec2/instance/export_test.go new file mode 100644 index 000000000..b2db3bbfd --- /dev/null +++ b/spinifex/handlers/ec2/instance/export_test.go @@ -0,0 +1,53 @@ +package handlers_ec2_instance + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/ebsmetadata" + "github.com/mulgadc/spinifex/spinifex/ebsprovider" + "github.com/mulgadc/spinifex/spinifex/objectstore" + "github.com/mulgadc/spinifex/spinifex/vm" +) + +// Unexported hooks and in-package fixtures the external test package needs. +const ( + TestRootAccount = testRootAccount + TestRootAZ = testRootAZ +) + +// NewProviderRootVolumeTestService is the in-package provider fixture over an +// in-memory provider and object store, with the one-AMI loader. +func NewProviderRootVolumeTestService(t *testing.T) *InstanceServiceImpl { + t.Helper() + return providerRootVolumeService(t, ebsprovider.NewMemoryProvider(ebsprovider.Capabilities{}), + rootVolumeAMILoader(), objectstore.NewMemoryObjectStore()) +} + +func (s *InstanceServiceImpl) MetadataStore() *ebsmetadata.Store { return s.metadata } + +func (s *InstanceServiceImpl) PrepareDataVolumes(ctx context.Context, input *ec2.RunInstancesInput, instance *vm.VM) ([]VolumeInfo, error) { + return s.prepareDataVolumes(ctx, input, instance) +} + +// DataVolumeParams mirrors dataVolumeParams with its fields exported. +type DataVolumeParams struct { + DeviceName string + SizeGiB int64 + Iops int64 + DeleteOnTermination bool +} + +func ParseDataVolumeParams(input *ec2.RunInstancesInput) []DataVolumeParams { + var out []DataVolumeParams + for _, p := range parseDataVolumeParams(input) { + out = append(out, DataVolumeParams{ + DeviceName: p.deviceName, + SizeGiB: p.sizeGiB, + Iops: p.iops, + DeleteOnTermination: p.deleteOnTermination, + }) + } + return out +} diff --git a/spinifex/handlers/ec2/vpc/central_tags_test.go b/spinifex/handlers/ec2/vpc/central_tags_test.go index 823fe232f..267f4d0ac 100644 --- a/spinifex/handlers/ec2/vpc/central_tags_test.go +++ b/spinifex/handlers/ec2/vpc/central_tags_test.go @@ -1,4 +1,4 @@ -package handlers_ec2_vpc +package handlers_ec2_vpc_test import ( "context" @@ -7,36 +7,15 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + handlers_ec2_vpc "github.com/mulgadc/spinifex/spinifex/handlers/ec2/vpc" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// fakeCentralTagStore records the tag map passed to PutResourceTags per -// resource and the ids passed to DeleteAllTags, and can be made to fail so -// tests can prove a central-store error never fails the operation it tracks. -type fakeCentralTagStore struct { - calls map[string]map[string]string - deleted []string - err error -} - -func (f *fakeCentralTagStore) PutResourceTags(_ context.Context, _, resourceID string, tags map[string]string) error { - if f.calls == nil { - f.calls = map[string]map[string]string{} - } - f.calls[resourceID] = tags - return f.err -} - -func (f *fakeCentralTagStore) DeleteAllTags(_ context.Context, _, resourceID string) error { - f.deleted = append(f.deleted, resourceID) - return f.err -} - func TestCreateVpc_ProjectsTagsToCentralStore(t *testing.T) { t.Parallel() - svc := setupTestVPCService(t) - writer := &fakeCentralTagStore{} + svc := handlers_ec2_vpc.SetupTestVPCService(t) + writer := &handlers_ec2_vpc.FakeCentralTagStore{} svc.SetCentralTagStore(writer) out, err := svc.CreateVpc(context.Background(), &ec2.CreateVpcInput{ @@ -49,20 +28,20 @@ func TestCreateVpc_ProjectsTagsToCentralStore(t *testing.T) { }, }, }, - }, testAccountID) + }, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) vpcID := *out.Vpc.VpcId - require.Contains(t, writer.calls, vpcID) - assert.Equal(t, map[string]string{"Name": "my-vpc"}, writer.calls[vpcID]) + require.Contains(t, writer.Calls, vpcID) + assert.Equal(t, map[string]string{"Name": "my-vpc"}, writer.Calls[vpcID]) } func TestCreateSubnet_ProjectsTagsToCentralStore(t *testing.T) { t.Parallel() - svc := setupTestVPCService(t) - writer := &fakeCentralTagStore{} + svc := handlers_ec2_vpc.SetupTestVPCService(t) + writer := &handlers_ec2_vpc.FakeCentralTagStore{} svc.SetCentralTagStore(writer) - vpcID := createTestVPC(t, svc, "10.0.0.0/16") + vpcID := handlers_ec2_vpc.CreateTestVPC(t, svc, "10.0.0.0/16") out, err := svc.CreateSubnet(context.Background(), &ec2.CreateSubnetInput{ VpcId: aws.String(vpcID), @@ -75,20 +54,20 @@ func TestCreateSubnet_ProjectsTagsToCentralStore(t *testing.T) { }, }, }, - }, testAccountID) + }, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) subnetID := *out.Subnet.SubnetId - require.Contains(t, writer.calls, subnetID) - assert.Equal(t, map[string]string{"Name": "my-subnet"}, writer.calls[subnetID]) + require.Contains(t, writer.Calls, subnetID) + assert.Equal(t, map[string]string{"Name": "my-subnet"}, writer.Calls[subnetID]) } func TestCreateSecurityGroup_ProjectsTagsToCentralStore(t *testing.T) { t.Parallel() - svc := setupTestVPCService(t) - writer := &fakeCentralTagStore{} + svc := handlers_ec2_vpc.SetupTestVPCService(t) + writer := &handlers_ec2_vpc.FakeCentralTagStore{} svc.SetCentralTagStore(writer) - vpcID := createTestVPC(t, svc, "10.0.0.0/16") + vpcID := handlers_ec2_vpc.CreateTestVPC(t, svc, "10.0.0.0/16") out, err := svc.CreateSecurityGroup(context.Background(), &ec2.CreateSecurityGroupInput{ GroupName: aws.String("web-sg"), @@ -102,31 +81,31 @@ func TestCreateSecurityGroup_ProjectsTagsToCentralStore(t *testing.T) { }, }, }, - }, testAccountID) + }, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) groupID := *out.GroupId - require.Contains(t, writer.calls, groupID) - assert.Equal(t, map[string]string{"Name": "my-sg"}, writer.calls[groupID]) + require.Contains(t, writer.Calls, groupID) + assert.Equal(t, map[string]string{"Name": "my-sg"}, writer.Calls[groupID]) } func TestCreateVpc_NoTags_SkipsCentralStoreWrite(t *testing.T) { t.Parallel() - svc := setupTestVPCService(t) - writer := &fakeCentralTagStore{} + svc := handlers_ec2_vpc.SetupTestVPCService(t) + writer := &handlers_ec2_vpc.FakeCentralTagStore{} svc.SetCentralTagStore(writer) out, err := svc.CreateVpc(context.Background(), &ec2.CreateVpcInput{ CidrBlock: aws.String("10.0.0.0/16"), - }, testAccountID) + }, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) - assert.NotContains(t, writer.calls, *out.Vpc.VpcId) + assert.NotContains(t, writer.Calls, *out.Vpc.VpcId) } func TestCreateVpc_NilCentralTagStore_CreateSucceeds(t *testing.T) { t.Parallel() - svc := setupTestVPCService(t) + svc := handlers_ec2_vpc.SetupTestVPCService(t) // No SetCentralTagStore call: centralTags stays nil. out, err := svc.CreateVpc(context.Background(), &ec2.CreateVpcInput{ @@ -139,15 +118,15 @@ func TestCreateVpc_NilCentralTagStore_CreateSucceeds(t *testing.T) { }, }, }, - }, testAccountID) + }, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) require.Len(t, out.Vpc.Tags, 1) } func TestCreateVpc_CentralTagStoreError_DoesNotFailCreate(t *testing.T) { t.Parallel() - svc := setupTestVPCService(t) - writer := &fakeCentralTagStore{err: errors.New("central store unavailable")} + svc := handlers_ec2_vpc.SetupTestVPCService(t) + writer := &handlers_ec2_vpc.FakeCentralTagStore{Err: errors.New("central store unavailable")} svc.SetCentralTagStore(writer) out, err := svc.CreateVpc(context.Background(), &ec2.CreateVpcInput{ @@ -160,7 +139,7 @@ func TestCreateVpc_CentralTagStoreError_DoesNotFailCreate(t *testing.T) { }, }, }, - }, testAccountID) + }, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) require.Len(t, out.Vpc.Tags, 1) } @@ -175,13 +154,13 @@ func TestDelete_ClearsCentralTagStore(t *testing.T) { cases := []struct { name string // create returns the id of a resource of this type, already tagged. - create func(t *testing.T, svc *VPCServiceImpl) string - del func(t *testing.T, svc *VPCServiceImpl, id string) + create func(t *testing.T, svc *handlers_ec2_vpc.VPCServiceImpl) string + del func(t *testing.T, svc *handlers_ec2_vpc.VPCServiceImpl, id string) }{ { name: "security group", - create: func(t *testing.T, svc *VPCServiceImpl) string { - vpcID := createTestVPC(t, svc, "10.1.0.0/16") + create: func(t *testing.T, svc *handlers_ec2_vpc.VPCServiceImpl) string { + vpcID := handlers_ec2_vpc.CreateTestVPC(t, svc, "10.1.0.0/16") out, err := svc.CreateSecurityGroup(context.Background(), &ec2.CreateSecurityGroupInput{ GroupName: aws.String("doomed-sg"), Description: aws.String("doomed"), @@ -191,20 +170,20 @@ func TestDelete_ClearsCentralTagStore(t *testing.T) { {Key: aws.String("Name"), Value: aws.String("doomed")}, }}, }, - }, testAccountID) + }, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) return *out.GroupId }, - del: func(t *testing.T, svc *VPCServiceImpl, id string) { + del: func(t *testing.T, svc *handlers_ec2_vpc.VPCServiceImpl, id string) { _, err := svc.DeleteSecurityGroup(context.Background(), - &ec2.DeleteSecurityGroupInput{GroupId: aws.String(id)}, testAccountID) + &ec2.DeleteSecurityGroupInput{GroupId: aws.String(id)}, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) }, }, { name: "subnet", - create: func(t *testing.T, svc *VPCServiceImpl) string { - vpcID := createTestVPC(t, svc, "10.2.0.0/16") + create: func(t *testing.T, svc *handlers_ec2_vpc.VPCServiceImpl) string { + vpcID := handlers_ec2_vpc.CreateTestVPC(t, svc, "10.2.0.0/16") out, err := svc.CreateSubnet(context.Background(), &ec2.CreateSubnetInput{ VpcId: aws.String(vpcID), CidrBlock: aws.String("10.2.1.0/24"), @@ -213,13 +192,13 @@ func TestDelete_ClearsCentralTagStore(t *testing.T) { {Key: aws.String("Name"), Value: aws.String("doomed")}, }}, }, - }, testAccountID) + }, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) return *out.Subnet.SubnetId }, - del: func(t *testing.T, svc *VPCServiceImpl, id string) { + del: func(t *testing.T, svc *handlers_ec2_vpc.VPCServiceImpl, id string) { _, err := svc.DeleteSubnet(context.Background(), - &ec2.DeleteSubnetInput{SubnetId: aws.String(id)}, testAccountID) + &ec2.DeleteSubnetInput{SubnetId: aws.String(id)}, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) }, }, @@ -228,9 +207,9 @@ func TestDelete_ClearsCentralTagStore(t *testing.T) { // and the forced instance teardown run the same path, so covering // one covers the ENI reclaimed by a task going away. name: "network interface", - create: func(t *testing.T, svc *VPCServiceImpl) string { - vpcID := createTestVPC(t, svc, "10.7.0.0/16") - subnetID := createTestSubnet(t, svc, vpcID, "10.7.1.0/24") + create: func(t *testing.T, svc *handlers_ec2_vpc.VPCServiceImpl) string { + vpcID := handlers_ec2_vpc.CreateTestVPC(t, svc, "10.7.0.0/16") + subnetID := handlers_ec2_vpc.CreateTestSubnet(t, svc, vpcID, "10.7.1.0/24") out, err := svc.CreateNetworkInterface(context.Background(), &ec2.CreateNetworkInterfaceInput{ SubnetId: aws.String(subnetID), TagSpecifications: []*ec2.TagSpecification{ @@ -238,19 +217,19 @@ func TestDelete_ClearsCentralTagStore(t *testing.T) { {Key: aws.String("Name"), Value: aws.String("doomed")}, }}, }, - }, testAccountID) + }, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) return *out.NetworkInterface.NetworkInterfaceId }, - del: func(t *testing.T, svc *VPCServiceImpl, id string) { + del: func(t *testing.T, svc *handlers_ec2_vpc.VPCServiceImpl, id string) { _, err := svc.DeleteNetworkInterface(context.Background(), - &ec2.DeleteNetworkInterfaceInput{NetworkInterfaceId: aws.String(id)}, testAccountID) + &ec2.DeleteNetworkInterfaceInput{NetworkInterfaceId: aws.String(id)}, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) }, }, { name: "vpc", - create: func(t *testing.T, svc *VPCServiceImpl) string { + create: func(t *testing.T, svc *handlers_ec2_vpc.VPCServiceImpl) string { out, err := svc.CreateVpc(context.Background(), &ec2.CreateVpcInput{ CidrBlock: aws.String("10.3.0.0/16"), TagSpecifications: []*ec2.TagSpecification{ @@ -258,13 +237,13 @@ func TestDelete_ClearsCentralTagStore(t *testing.T) { {Key: aws.String("Name"), Value: aws.String("doomed")}, }}, }, - }, testAccountID) + }, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) return *out.Vpc.VpcId }, - del: func(t *testing.T, svc *VPCServiceImpl, id string) { + del: func(t *testing.T, svc *handlers_ec2_vpc.VPCServiceImpl, id string) { _, err := svc.DeleteVpc(context.Background(), - &ec2.DeleteVpcInput{VpcId: aws.String(id)}, testAccountID) + &ec2.DeleteVpcInput{VpcId: aws.String(id)}, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) }, }, @@ -273,15 +252,15 @@ func TestDelete_ClearsCentralTagStore(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - svc := setupTestVPCService(t) - store := &fakeCentralTagStore{} + svc := handlers_ec2_vpc.SetupTestVPCService(t) + store := &handlers_ec2_vpc.FakeCentralTagStore{} svc.SetCentralTagStore(store) id := tc.create(t, svc) - require.Contains(t, store.calls, id, "precondition: the create must have tagged it") + require.Contains(t, store.Calls, id, "precondition: the create must have tagged it") tc.del(t, svc, id) - assert.Contains(t, store.deleted, id, "the deleted resource's tags must be retired") + assert.Contains(t, store.Deleted, id, "the deleted resource's tags must be retired") }) } } @@ -290,28 +269,28 @@ func TestDelete_ClearsCentralTagStore(t *testing.T) { // by hand. Asserted separately because the id is not the one under test. func TestDeleteVpc_ClearsTheCascadedDefaultSecurityGroup(t *testing.T) { t.Parallel() - svc := setupTestVPCService(t) - store := &fakeCentralTagStore{} + svc := handlers_ec2_vpc.SetupTestVPCService(t) + store := &handlers_ec2_vpc.FakeCentralTagStore{} svc.SetCentralTagStore(store) - vpcID := createTestVPC(t, svc, "10.4.0.0/16") - defaultSG, err := svc.FindDefaultSGForVPC(testAccountID, vpcID) + vpcID := handlers_ec2_vpc.CreateTestVPC(t, svc, "10.4.0.0/16") + defaultSG, err := svc.FindDefaultSGForVPC(handlers_ec2_vpc.TestAccountID, vpcID) require.NoError(t, err) require.NotEmpty(t, defaultSG, "precondition: CreateVpc makes a default SG") - _, err = svc.DeleteVpc(context.Background(), &ec2.DeleteVpcInput{VpcId: aws.String(vpcID)}, testAccountID) + _, err = svc.DeleteVpc(context.Background(), &ec2.DeleteVpcInput{VpcId: aws.String(vpcID)}, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) - assert.Contains(t, store.deleted, defaultSG, "the cascaded default SG must not keep its tags") + assert.Contains(t, store.Deleted, defaultSG, "the cascaded default SG must not keep its tags") } func TestDelete_NilCentralTagStore_DeleteSucceeds(t *testing.T) { t.Parallel() - svc := setupTestVPCService(t) + svc := handlers_ec2_vpc.SetupTestVPCService(t) // No SetCentralTagStore call: centralTags stays nil. - vpcID := createTestVPC(t, svc, "10.5.0.0/16") - _, err := svc.DeleteVpc(context.Background(), &ec2.DeleteVpcInput{VpcId: aws.String(vpcID)}, testAccountID) + vpcID := handlers_ec2_vpc.CreateTestVPC(t, svc, "10.5.0.0/16") + _, err := svc.DeleteVpc(context.Background(), &ec2.DeleteVpcInput{VpcId: aws.String(vpcID)}, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) } @@ -320,12 +299,12 @@ func TestDelete_NilCentralTagStore_DeleteSucceeds(t *testing.T) { // happened, and would be left believing the resource survived. func TestDelete_CentralTagStoreError_DoesNotFailDelete(t *testing.T) { t.Parallel() - svc := setupTestVPCService(t) - store := &fakeCentralTagStore{err: errors.New("central store unavailable")} + svc := handlers_ec2_vpc.SetupTestVPCService(t) + store := &handlers_ec2_vpc.FakeCentralTagStore{Err: errors.New("central store unavailable")} svc.SetCentralTagStore(store) - vpcID := createTestVPC(t, svc, "10.6.0.0/16") - _, err := svc.DeleteVpc(context.Background(), &ec2.DeleteVpcInput{VpcId: aws.String(vpcID)}, testAccountID) + vpcID := handlers_ec2_vpc.CreateTestVPC(t, svc, "10.6.0.0/16") + _, err := svc.DeleteVpc(context.Background(), &ec2.DeleteVpcInput{VpcId: aws.String(vpcID)}, handlers_ec2_vpc.TestAccountID) require.NoError(t, err) - assert.Contains(t, store.deleted, vpcID, "the clear must still have been attempted") + assert.Contains(t, store.Deleted, vpcID, "the clear must still have been attempted") } diff --git a/spinifex/handlers/ec2/vpc/export_test.go b/spinifex/handlers/ec2/vpc/export_test.go new file mode 100644 index 000000000..af899b208 --- /dev/null +++ b/spinifex/handlers/ec2/vpc/export_test.go @@ -0,0 +1,36 @@ +package handlers_ec2_vpc + +import "context" + +// In-package fixtures the external test package needs. +const TestAccountID = testAccountID + +var ( + SetupTestVPCService = setupTestVPCService + CreateTestVPC = createTestVPC + CreateTestSubnet = createTestSubnet +) + +// FakeCentralTagStore records the tag map passed to PutResourceTags per +// resource and the ids passed to DeleteAllTags, and can be made to fail so +// tests can prove a central-store error never fails the operation it tracks. +type FakeCentralTagStore struct { + Calls map[string]map[string]string + Deleted []string + Err error +} + +var _ CentralTagStore = (*FakeCentralTagStore)(nil) + +func (f *FakeCentralTagStore) PutResourceTags(_ context.Context, _, resourceID string, tags map[string]string) error { + if f.Calls == nil { + f.Calls = map[string]map[string]string{} + } + f.Calls[resourceID] = tags + return f.Err +} + +func (f *FakeCentralTagStore) DeleteAllTags(_ context.Context, _, resourceID string) error { + f.Deleted = append(f.Deleted, resourceID) + return f.Err +} diff --git a/spinifex/handlers/ec2/vpc/security_group_modify_test.go b/spinifex/handlers/ec2/vpc/security_group_modify_test.go index 8d6d454b6..b04bcb223 100644 --- a/spinifex/handlers/ec2/vpc/security_group_modify_test.go +++ b/spinifex/handlers/ec2/vpc/security_group_modify_test.go @@ -189,7 +189,7 @@ func TestModifySecurityGroupRules_RevocableByNewContent(t *testing.T) { func TestModifySecurityGroupRules_PreservesTags(t *testing.T) { t.Parallel() svc := setupTestVPCService(t) - writer := &fakeCentralTagStore{} + writer := &FakeCentralTagStore{} svc.SetCentralTagStore(writer) vpcID := createTestVPC(t, svc, "10.0.0.0/16") sgID := createTestSG(t, svc, vpcID, "mod-tags") @@ -212,8 +212,8 @@ func TestModifySecurityGroupRules_PreservesTags(t *testing.T) { want := map[string]string{"Name": "web"} assert.Equal(t, want, filterutil.EC2TagsToMap(sgRuleByID(t, svc, testAccountID, ruleID).Tags)) - assert.Equal(t, want, writer.calls[ruleID], "the central tag store must still hold the rule's tags") - assert.NotContains(t, writer.deleted, ruleID, "a modify must not clear the rule's central tags") + assert.Equal(t, want, writer.Calls[ruleID], "the central tag store must still hold the rule's tags") + assert.NotContains(t, writer.Deleted, ruleID, "a modify must not clear the rule's central tags") } // TestModifySecurityGroupRules_DescriptionIsReplaced pins AWS's whole-rule diff --git a/spinifex/handlers/ecs/export_test.go b/spinifex/handlers/ecs/export_test.go new file mode 100644 index 000000000..ca64cb2e9 --- /dev/null +++ b/spinifex/handlers/ecs/export_test.go @@ -0,0 +1,35 @@ +package handlers_ecs + +import ( + "context" + + "github.com/mulgadc/spinifex/spinifex/handlers/ecs/bus" + "github.com/nats-io/nats.go/jetstream" +) + +// Unexported hooks and in-package fixtures the external test package needs. +// Task status is reported by the agent, so these tests seed KV directly +// rather than drive a live agent through RunTask. +const TestAccountID = testAccountID + +var ( + NewTestService = newTestService + RegisterInstance = registerInstance + PutJSON = putJSON + GetJSON = getJSON + ServiceTaskGroup = serviceTaskGroup +) + +func (s *Service) Region() string { return s.region } + +func (s *Service) Bucket(ctx context.Context, accountID string) (jetstream.KeyValue, error) { + return s.bucket(ctx, accountID) +} + +func (s *Service) UpsertInstance(ctx context.Context, kv jetstream.KeyValue, accountID, cluster, instanceID string, mutate func(*InstanceRecord)) (*InstanceRecord, error) { + return s.upsertInstance(ctx, kv, accountID, cluster, instanceID, mutate) +} + +func (s *Service) RecordTaskState(ctx context.Context, msg *bus.TaskState) error { + return s.recordTaskState(ctx, msg) +} diff --git a/spinifex/handlers/ecs/instance_list_status_test.go b/spinifex/handlers/ecs/instance_list_status_test.go index a0b9e84b4..af2f2c616 100644 --- a/spinifex/handlers/ecs/instance_list_status_test.go +++ b/spinifex/handlers/ecs/instance_list_status_test.go @@ -1,23 +1,24 @@ -package handlers_ecs +package handlers_ecs_test import ( "testing" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ecs" + handlers_ecs "github.com/mulgadc/spinifex/spinifex/handlers/ecs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) // listInstanceARNs lists a cluster's container instances at one status, or at // every status when the filter is empty. -func listInstanceARNs(t *testing.T, svc *Service, cluster, status string) []string { +func listInstanceARNs(t *testing.T, svc *handlers_ecs.Service, cluster, status string) []string { t.Helper() in := &ecs.ListContainerInstancesInput{Cluster: aws.String(cluster)} if status != "" { in.Status = aws.String(status) } - out, err := svc.ListContainerInstances(t.Context(), in, testAccountID) + out, err := svc.ListContainerInstances(t.Context(), in, handlers_ecs.TestAccountID) require.NoError(t, err) return aws.StringValueSlice(out.ContainerInstanceArns) } @@ -26,24 +27,24 @@ func listInstanceARNs(t *testing.T, svc *Service, cluster, status string) []stri // find one that can take work, and a cluster carrying a drained node reads as // if every node were live. func TestListContainerInstances_StatusSelectsOnlyThatStatus(t *testing.T) { - svc, _ := newTestService(t) - _, err := svc.CreateCluster(t.Context(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID) + svc, _ := handlers_ecs.NewTestService(t) + _, err := svc.CreateCluster(t.Context(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, handlers_ecs.TestAccountID) require.NoError(t, err) - registerInstance(t, svc, "web", "i-live", 1024, 2048) - registerInstance(t, svc, "web", "i-drained", 1024, 2048) + handlers_ecs.RegisterInstance(t, svc, "web", "i-live", 1024, 2048) + handlers_ecs.RegisterInstance(t, svc, "web", "i-drained", 1024, 2048) - kv, err := svc.bucket(t.Context(), testAccountID) + kv, err := svc.Bucket(t.Context(), handlers_ecs.TestAccountID) require.NoError(t, err) - drained, err := svc.upsertInstance(t.Context(), kv, testAccountID, "web", "i-drained", func(r *InstanceRecord) { - r.Status = InstanceStatusDraining + drained, err := svc.UpsertInstance(t.Context(), kv, handlers_ecs.TestAccountID, "web", "i-drained", func(r *handlers_ecs.InstanceRecord) { + r.Status = handlers_ecs.InstanceStatusDraining }) require.NoError(t, err) - active := listInstanceARNs(t, svc, "web", InstanceStatusActive) + active := listInstanceARNs(t, svc, "web", handlers_ecs.InstanceStatusActive) assert.Len(t, active, 1) assert.NotContains(t, active, drained.ARN) - assert.Equal(t, []string{drained.ARN}, listInstanceARNs(t, svc, "web", InstanceStatusDraining)) + assert.Equal(t, []string{drained.ARN}, listInstanceARNs(t, svc, "web", handlers_ecs.InstanceStatusDraining)) assert.Len(t, listInstanceARNs(t, svc, "web", ""), 2) } @@ -51,10 +52,10 @@ func TestListContainerInstances_StatusSelectsOnlyThatStatus(t *testing.T) { // so asking for one is a valid question with an empty answer rather than an // error. func TestListContainerInstances_TransitionalStatusMatchesNothing(t *testing.T) { - svc, _ := newTestService(t) - _, err := svc.CreateCluster(t.Context(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID) + svc, _ := handlers_ecs.NewTestService(t) + _, err := svc.CreateCluster(t.Context(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, handlers_ecs.TestAccountID) require.NoError(t, err) - registerInstance(t, svc, "web", "i-live", 1024, 2048) + handlers_ecs.RegisterInstance(t, svc, "web", "i-live", 1024, 2048) for _, status := range []string{"REGISTERING", "DEREGISTERING", "REGISTRATION_FAILED"} { assert.Empty(t, listInstanceARNs(t, svc, "web", status), status) @@ -62,42 +63,42 @@ func TestListContainerInstances_TransitionalStatusMatchesNothing(t *testing.T) { } func TestListContainerInstances_UnknownStatusIsRefused(t *testing.T) { - svc, _ := newTestService(t) - _, err := svc.CreateCluster(t.Context(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID) + svc, _ := handlers_ecs.NewTestService(t) + _, err := svc.CreateCluster(t.Context(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, handlers_ecs.TestAccountID) require.NoError(t, err) _, err = svc.ListContainerInstances(t.Context(), &ecs.ListContainerInstancesInput{ Cluster: aws.String("web"), Status: aws.String("INACTIVE"), - }, testAccountID) + }, handlers_ecs.TestAccountID) require.Error(t, err) } // A filtered list and a describe of what it returned have to agree, since the // status the filter selects on is the one describe reports. func TestListContainerInstances_FilterAgreesWithDescribe(t *testing.T) { - svc, _ := newTestService(t) - _, err := svc.CreateCluster(t.Context(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID) + svc, _ := handlers_ecs.NewTestService(t) + _, err := svc.CreateCluster(t.Context(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, handlers_ecs.TestAccountID) require.NoError(t, err) - registerInstance(t, svc, "web", "i-live", 1024, 2048) - registerInstance(t, svc, "web", "i-drained", 1024, 2048) + handlers_ecs.RegisterInstance(t, svc, "web", "i-live", 1024, 2048) + handlers_ecs.RegisterInstance(t, svc, "web", "i-drained", 1024, 2048) - kv, err := svc.bucket(t.Context(), testAccountID) + kv, err := svc.Bucket(t.Context(), handlers_ecs.TestAccountID) require.NoError(t, err) - _, err = svc.upsertInstance(t.Context(), kv, testAccountID, "web", "i-drained", func(r *InstanceRecord) { - r.Status = InstanceStatusDraining + _, err = svc.UpsertInstance(t.Context(), kv, handlers_ecs.TestAccountID, "web", "i-drained", func(r *handlers_ecs.InstanceRecord) { + r.Status = handlers_ecs.InstanceStatusDraining }) require.NoError(t, err) - arns := listInstanceARNs(t, svc, "web", InstanceStatusActive) + arns := listInstanceARNs(t, svc, "web", handlers_ecs.InstanceStatusActive) require.NotEmpty(t, arns) out, err := svc.DescribeContainerInstances(t.Context(), &ecs.DescribeContainerInstancesInput{ Cluster: aws.String("web"), ContainerInstances: aws.StringSlice(arns), - }, testAccountID) + }, handlers_ecs.TestAccountID) require.NoError(t, err) require.Len(t, out.ContainerInstances, len(arns)) for _, ci := range out.ContainerInstances { - assert.Equal(t, InstanceStatusActive, aws.StringValue(ci.Status)) + assert.Equal(t, handlers_ecs.InstanceStatusActive, aws.StringValue(ci.Status)) } } diff --git a/spinifex/handlers/ecs/task_accounting_test.go b/spinifex/handlers/ecs/task_accounting_test.go index c84abe536..3b3fabfba 100644 --- a/spinifex/handlers/ecs/task_accounting_test.go +++ b/spinifex/handlers/ecs/task_accounting_test.go @@ -1,10 +1,11 @@ -package handlers_ecs +package handlers_ecs_test import ( "testing" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ecs" + handlers_ecs "github.com/mulgadc/spinifex/spinifex/handlers/ecs" "github.com/mulgadc/spinifex/spinifex/handlers/ecs/bus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -13,49 +14,49 @@ import ( // seedTasks writes task records straight to KV. The statuses under test are // reported by the agent, so driving them through RunTask would require a live // agent to reach anything other than PENDING. -func seedTasks(t *testing.T, svc *Service, cluster string, recs ...*TaskRecord) { +func seedTasks(t *testing.T, svc *handlers_ecs.Service, cluster string, recs ...*handlers_ecs.TaskRecord) { t.Helper() - kv, err := svc.bucket(t.Context(), testAccountID) + kv, err := svc.Bucket(t.Context(), handlers_ecs.TestAccountID) require.NoError(t, err) for _, r := range recs { r.Cluster = cluster - r.ARN = TaskARN(svc.region, testAccountID, cluster, r.TaskID) - require.NoError(t, putJSON(t.Context(), kv, TaskKey(cluster, r.TaskID), r)) + r.ARN = handlers_ecs.TaskARN(svc.Region(), handlers_ecs.TestAccountID, cluster, r.TaskID) + require.NoError(t, handlers_ecs.PutJSON(t.Context(), kv, handlers_ecs.TaskKey(cluster, r.TaskID), r)) } } -func listTaskIDs(t *testing.T, svc *Service, input *ecs.ListTasksInput) []string { +func listTaskIDs(t *testing.T, svc *handlers_ecs.Service, input *ecs.ListTasksInput) []string { t.Helper() - out, err := svc.ListTasks(t.Context(), input, testAccountID) + out, err := svc.ListTasks(t.Context(), input, handlers_ecs.TestAccountID) require.NoError(t, err) ids := make([]string, 0, len(out.TaskArns)) for _, a := range out.TaskArns { - ids = append(ids, ContainerInstanceShortID(aws.StringValue(a))) + ids = append(ids, handlers_ecs.ContainerInstanceShortID(aws.StringValue(a))) } return ids } func TestListTasks_DesiredStatusFilters(t *testing.T) { - svc, _ := newTestService(t) + svc, _ := handlers_ecs.NewTestService(t) seedTasks(t, svc, "web", - &TaskRecord{TaskID: "run-1", DesiredStatus: TaskStatusRunning, LastStatus: TaskStatusRunning}, - &TaskRecord{TaskID: "stop-1", DesiredStatus: TaskStatusStopped, LastStatus: TaskStatusStopped}, - &TaskRecord{TaskID: "stop-2", DesiredStatus: TaskStatusStopped, LastStatus: TaskStatusStopped}, + &handlers_ecs.TaskRecord{TaskID: "run-1", DesiredStatus: handlers_ecs.TaskStatusRunning, LastStatus: handlers_ecs.TaskStatusRunning}, + &handlers_ecs.TaskRecord{TaskID: "stop-1", DesiredStatus: handlers_ecs.TaskStatusStopped, LastStatus: handlers_ecs.TaskStatusStopped}, + &handlers_ecs.TaskRecord{TaskID: "stop-2", DesiredStatus: handlers_ecs.TaskStatusStopped, LastStatus: handlers_ecs.TaskStatusStopped}, ) assert.ElementsMatch(t, []string{"run-1"}, - listTaskIDs(t, svc, &ecs.ListTasksInput{Cluster: aws.String("web"), DesiredStatus: aws.String(TaskStatusRunning)})) + listTaskIDs(t, svc, &ecs.ListTasksInput{Cluster: aws.String("web"), DesiredStatus: aws.String(handlers_ecs.TaskStatusRunning)})) assert.ElementsMatch(t, []string{"stop-1", "stop-2"}, - listTaskIDs(t, svc, &ecs.ListTasksInput{Cluster: aws.String("web"), DesiredStatus: aws.String(TaskStatusStopped)})) + listTaskIDs(t, svc, &ecs.ListTasksInput{Cluster: aws.String("web"), DesiredStatus: aws.String(handlers_ecs.TaskStatusStopped)})) } // The default is the case that broke a caller: asking for nothing used to hand // back every task the cluster had ever run. func TestListTasks_OmittedDesiredStatusMeansRunning(t *testing.T) { - svc, _ := newTestService(t) + svc, _ := handlers_ecs.NewTestService(t) seedTasks(t, svc, "web", - &TaskRecord{TaskID: "run-1", DesiredStatus: TaskStatusRunning, LastStatus: TaskStatusRunning}, - &TaskRecord{TaskID: "stop-1", DesiredStatus: TaskStatusStopped, LastStatus: TaskStatusStopped}, + &handlers_ecs.TaskRecord{TaskID: "run-1", DesiredStatus: handlers_ecs.TaskStatusRunning, LastStatus: handlers_ecs.TaskStatusRunning}, + &handlers_ecs.TaskRecord{TaskID: "stop-1", DesiredStatus: handlers_ecs.TaskStatusStopped, LastStatus: handlers_ecs.TaskStatusStopped}, ) assert.ElementsMatch(t, []string{"run-1"}, @@ -63,15 +64,15 @@ func TestListTasks_OmittedDesiredStatusMeansRunning(t *testing.T) { } func TestListTasks_FamilyStartedByServiceAndInstanceFilter(t *testing.T) { - svc, _ := newTestService(t) + svc, _ := handlers_ecs.NewTestService(t) seedTasks(t, svc, "web", - &TaskRecord{ - TaskID: "a", DesiredStatus: TaskStatusRunning, LastStatus: TaskStatusRunning, - TaskDefFamily: "api", StartedBy: "ci", Group: serviceTaskGroup("web"), ContainerInstanceID: "i-1", + &handlers_ecs.TaskRecord{ + TaskID: "a", DesiredStatus: handlers_ecs.TaskStatusRunning, LastStatus: handlers_ecs.TaskStatusRunning, + TaskDefFamily: "api", StartedBy: "ci", Group: handlers_ecs.ServiceTaskGroup("web"), ContainerInstanceID: "i-1", }, - &TaskRecord{ - TaskID: "b", DesiredStatus: TaskStatusRunning, LastStatus: TaskStatusRunning, - TaskDefFamily: "worker", StartedBy: "human", Group: serviceTaskGroup("batch"), ContainerInstanceID: "i-2", + &handlers_ecs.TaskRecord{ + TaskID: "b", DesiredStatus: handlers_ecs.TaskStatusRunning, LastStatus: handlers_ecs.TaskStatusRunning, + TaskDefFamily: "worker", StartedBy: "human", Group: handlers_ecs.ServiceTaskGroup("batch"), ContainerInstanceID: "i-2", }, ) @@ -88,14 +89,14 @@ func TestListTasks_FamilyStartedByServiceAndInstanceFilter(t *testing.T) { // Filters combine, so a task has to satisfy all of them rather than any. func TestListTasks_FiltersCombine(t *testing.T) { - svc, _ := newTestService(t) + svc, _ := handlers_ecs.NewTestService(t) seedTasks(t, svc, "web", - &TaskRecord{ - TaskID: "a", DesiredStatus: TaskStatusRunning, LastStatus: TaskStatusRunning, + &handlers_ecs.TaskRecord{ + TaskID: "a", DesiredStatus: handlers_ecs.TaskStatusRunning, LastStatus: handlers_ecs.TaskStatusRunning, TaskDefFamily: "api", ContainerInstanceID: "i-1", }, - &TaskRecord{ - TaskID: "b", DesiredStatus: TaskStatusRunning, LastStatus: TaskStatusRunning, + &handlers_ecs.TaskRecord{ + TaskID: "b", DesiredStatus: handlers_ecs.TaskStatusRunning, LastStatus: handlers_ecs.TaskStatusRunning, TaskDefFamily: "api", ContainerInstanceID: "i-2", }, ) @@ -108,49 +109,49 @@ func TestListTasks_FiltersCombine(t *testing.T) { // A task nobody asked to stop still has to leave the RUNNING filter when it // exits, or a caller polling for live tasks never stops seeing it. func TestRecordTaskState_SelfExitMovesDesiredStatusToStopped(t *testing.T) { - svc, _ := newTestService(t) + svc, _ := handlers_ecs.NewTestService(t) seedTasks(t, svc, "web", - &TaskRecord{TaskID: "t-1", DesiredStatus: TaskStatusRunning, LastStatus: TaskStatusRunning}, + &handlers_ecs.TaskRecord{TaskID: "t-1", DesiredStatus: handlers_ecs.TaskStatusRunning, LastStatus: handlers_ecs.TaskStatusRunning}, ) - require.NoError(t, svc.recordTaskState(t.Context(), &bus.TaskState{ - AccountID: testAccountID, ClusterName: "web", TaskID: "t-1", - LastStatus: TaskStatusStopped, Reason: "container exited (1)", + require.NoError(t, svc.RecordTaskState(t.Context(), &bus.TaskState{ + AccountID: handlers_ecs.TestAccountID, ClusterName: "web", TaskID: "t-1", + LastStatus: handlers_ecs.TaskStatusStopped, Reason: "container exited (1)", })) - kv, err := svc.bucket(t.Context(), testAccountID) + kv, err := svc.Bucket(t.Context(), handlers_ecs.TestAccountID) require.NoError(t, err) - var rec TaskRecord - found, err := getJSON(t.Context(), kv, TaskKey("web", "t-1"), &rec) + var rec handlers_ecs.TaskRecord + found, err := handlers_ecs.GetJSON(t.Context(), kv, handlers_ecs.TaskKey("web", "t-1"), &rec) require.NoError(t, err) require.True(t, found) - assert.Equal(t, TaskStatusStopped, rec.DesiredStatus) + assert.Equal(t, handlers_ecs.TaskStatusStopped, rec.DesiredStatus) assert.Empty(t, listTaskIDs(t, svc, &ecs.ListTasksInput{Cluster: aws.String("web")})) assert.ElementsMatch(t, []string{"t-1"}, listTaskIDs(t, svc, &ecs.ListTasksInput{ - Cluster: aws.String("web"), DesiredStatus: aws.String(TaskStatusStopped), + Cluster: aws.String("web"), DesiredStatus: aws.String(handlers_ecs.TaskStatusStopped), })) } func TestDescribeContainerInstances_CountsComeFromTaskRecords(t *testing.T) { - svc, _ := newTestService(t) + svc, _ := handlers_ecs.NewTestService(t) _, err := svc.RegisterContainerInstance(t.Context(), &ecs.RegisterContainerInstanceInput{ Cluster: aws.String("web"), InstanceIdentityDocument: aws.String("i-1"), - }, testAccountID) + }, handlers_ecs.TestAccountID) require.NoError(t, err) seedTasks(t, svc, "web", - &TaskRecord{TaskID: "r1", LastStatus: TaskStatusRunning, ContainerInstanceID: "i-1"}, - &TaskRecord{TaskID: "p1", LastStatus: TaskStatusPending, ContainerInstanceID: "i-1"}, - &TaskRecord{TaskID: "s1", LastStatus: TaskStatusStopped, ContainerInstanceID: "i-1"}, - &TaskRecord{TaskID: "s2", LastStatus: TaskStatusStopped, ContainerInstanceID: "i-1"}, - &TaskRecord{TaskID: "r2", LastStatus: TaskStatusRunning, ContainerInstanceID: "i-2"}, + &handlers_ecs.TaskRecord{TaskID: "r1", LastStatus: handlers_ecs.TaskStatusRunning, ContainerInstanceID: "i-1"}, + &handlers_ecs.TaskRecord{TaskID: "p1", LastStatus: handlers_ecs.TaskStatusPending, ContainerInstanceID: "i-1"}, + &handlers_ecs.TaskRecord{TaskID: "s1", LastStatus: handlers_ecs.TaskStatusStopped, ContainerInstanceID: "i-1"}, + &handlers_ecs.TaskRecord{TaskID: "s2", LastStatus: handlers_ecs.TaskStatusStopped, ContainerInstanceID: "i-1"}, + &handlers_ecs.TaskRecord{TaskID: "r2", LastStatus: handlers_ecs.TaskStatusRunning, ContainerInstanceID: "i-2"}, ) out, err := svc.DescribeContainerInstances(t.Context(), &ecs.DescribeContainerInstancesInput{ Cluster: aws.String("web"), ContainerInstances: []*string{aws.String("i-1")}, - }, testAccountID) + }, handlers_ecs.TestAccountID) require.NoError(t, err) require.Len(t, out.ContainerInstances, 1) assert.Equal(t, int64(1), aws.Int64Value(out.ContainerInstances[0].RunningTasksCount)) @@ -161,21 +162,21 @@ func TestDescribeContainerInstances_CountsComeFromTaskRecords(t *testing.T) { // names a task that stopped, so a count taken from it reports a task nobody is // running. func TestDescribeContainerInstances_StalePlacedTasksDoNotInflateCount(t *testing.T) { - svc, _ := newTestService(t) - kv, err := svc.bucket(t.Context(), testAccountID) + svc, _ := handlers_ecs.NewTestService(t) + kv, err := svc.Bucket(t.Context(), handlers_ecs.TestAccountID) require.NoError(t, err) - _, err = svc.upsertInstance(t.Context(), kv, testAccountID, "web", "i-1", func(r *InstanceRecord) { + _, err = svc.UpsertInstance(t.Context(), kv, handlers_ecs.TestAccountID, "web", "i-1", func(r *handlers_ecs.InstanceRecord) { r.PlacedTasks = []string{"gone-1", "gone-2", "r1"} }) require.NoError(t, err) seedTasks(t, svc, "web", - &TaskRecord{TaskID: "r1", LastStatus: TaskStatusRunning, ContainerInstanceID: "i-1"}, + &handlers_ecs.TaskRecord{TaskID: "r1", LastStatus: handlers_ecs.TaskStatusRunning, ContainerInstanceID: "i-1"}, ) out, err := svc.DescribeContainerInstances(t.Context(), &ecs.DescribeContainerInstancesInput{ Cluster: aws.String("web"), ContainerInstances: []*string{aws.String("i-1")}, - }, testAccountID) + }, handlers_ecs.TestAccountID) require.NoError(t, err) require.Len(t, out.ContainerInstances, 1) assert.Equal(t, int64(1), aws.Int64Value(out.ContainerInstances[0].RunningTasksCount))