From 0f55dfbec9ed82d47323a8fc6c23842f9d687085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Mon, 22 Jun 2026 11:12:36 +0200 Subject: [PATCH] fix(snapshot ecs): handle clusters with zero services kosli snapshot ecs aborted when any scanned cluster had no services, because getFilteredECSServicesInCluster called DescribeServices with an empty Services list, which AWS rejects with InvalidParameterException: Services cannot be empty. Guard the empty case so an empty cluster contributes zero services instead of erroring. Introduce an ECSServicesAPI interface and a FakeECSClient (mirroring the existing Lambda pattern) so the behaviour is covered by unit tests without live AWS. Fixes #965 --- internal/aws/aws.go | 18 +++++++++- internal/aws/ecs_services_test.go | 58 +++++++++++++++++++++++++++++++ internal/aws/fake_ecs.go | 32 +++++++++++++++++ 3 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 internal/aws/ecs_services_test.go create mode 100644 internal/aws/fake_ecs.go diff --git a/internal/aws/aws.go b/internal/aws/aws.go index c549a78e5..8b0954405 100644 --- a/internal/aws/aws.go +++ b/internal/aws/aws.go @@ -161,6 +161,14 @@ func ResetLambdaClientFactory() { NewLambdaClientFunc = defaultNewLambdaClient } +// ECSServicesAPI is the subset of AWS ECS operations used when listing and +// describing services in a cluster. The real *ecs.Client satisfies this +// implicitly. +type ECSServicesAPI interface { + ListServices(ctx context.Context, params *ecs.ListServicesInput, optFns ...func(*ecs.Options)) (*ecs.ListServicesOutput, error) + DescribeServices(ctx context.Context, params *ecs.DescribeServicesInput, optFns ...func(*ecs.Options)) (*ecs.DescribeServicesOutput, error) +} + // NewECSClient returns a new ECS API client func (staticCreds *AWSStaticCreds) NewECSClient() (*ecs.Client, error) { cfg, err := staticCreds.NewAWSConfigFromEnvOrFlags() @@ -623,7 +631,7 @@ func (staticCreds *AWSStaticCreds) GetEcsTasksData(clusterFilter, serviceFilter } // getFilteredECSServicesInCluster fetches a filtered set of ECS services recursively (10 at a time) and returns a list of ecs Services -func getFilteredECSServicesInCluster(client *ecs.Client, cluster string, allServices *[]ecsTypes.Service, serviceFilter *filters.ResourceFilterOptions, +func getFilteredECSServicesInCluster(client ECSServicesAPI, cluster string, allServices *[]ecsTypes.Service, serviceFilter *filters.ResourceFilterOptions, nextToken *string, logger *logger.Logger) (*[]ecsTypes.Service, error) { listInput := &ecs.ListServicesInput{ Cluster: aws.String(cluster), @@ -636,6 +644,14 @@ func getFilteredECSServicesInCluster(client *ecs.Client, cluster string, allServ return allServices, err } + // A cluster with no services yields an empty ServiceArns list. The AWS ECS + // DescribeServices API rejects an empty Services list with + // "InvalidParameterException: Services cannot be empty", so skip the call + // and let the cluster contribute zero services. + if len(listServicesOutput.ServiceArns) == 0 { + return allServices, nil + } + describeServicesOutput, err := client.DescribeServices(context.TODO(), &ecs.DescribeServicesInput{Cluster: aws.String(cluster), Services: listServicesOutput.ServiceArns}) if err != nil { return allServices, err diff --git a/internal/aws/ecs_services_test.go b/internal/aws/ecs_services_test.go new file mode 100644 index 000000000..50807c354 --- /dev/null +++ b/internal/aws/ecs_services_test.go @@ -0,0 +1,58 @@ +package aws + +import ( + "testing" + + ecsTypes "github.com/aws/aws-sdk-go-v2/service/ecs/types" + "github.com/kosli-dev/cli/internal/filters" + "github.com/kosli-dev/cli/internal/logger" + "github.com/stretchr/testify/require" +) + +// TestGetFilteredECSServicesInCluster_EmptyCluster reproduces the regression +// where a cluster with no services caused DescribeServices to be called with an +// empty Services list, which the real AWS API rejects with +// "InvalidParameterException: Services cannot be empty". +// +// An empty cluster should simply contribute zero services without error. +func TestGetFilteredECSServicesInCluster_EmptyCluster(t *testing.T) { + client := &FakeECSClient{ + ServiceArns: []string{}, // cluster has no services + Services: []ecsTypes.Service{}, + } + + allServices, err := getFilteredECSServicesInCluster( + client, + "empty-cluster", + &[]ecsTypes.Service{}, + &filters.ResourceFilterOptions{}, + nil, + logger.NewStandardLogger(), + ) + + require.NoError(t, err) + require.Empty(t, *allServices) +} + +// TestGetFilteredECSServicesInCluster_WithServices verifies the happy path is +// unchanged by the empty-cluster guard: a cluster with services returns them. +func TestGetFilteredECSServicesInCluster_WithServices(t *testing.T) { + svcName := "my-service" + client := &FakeECSClient{ + ServiceArns: []string{"arn:aws:ecs:eu-central-1:123:service/cluster/my-service"}, + Services: []ecsTypes.Service{{ServiceName: &svcName}}, + } + + allServices, err := getFilteredECSServicesInCluster( + client, + "cluster", + &[]ecsTypes.Service{}, + &filters.ResourceFilterOptions{}, + nil, + logger.NewStandardLogger(), + ) + + require.NoError(t, err) + require.Len(t, *allServices, 1) + require.Equal(t, svcName, *(*allServices)[0].ServiceName) +} diff --git a/internal/aws/fake_ecs.go b/internal/aws/fake_ecs.go new file mode 100644 index 000000000..987455017 --- /dev/null +++ b/internal/aws/fake_ecs.go @@ -0,0 +1,32 @@ +package aws + +import ( + "context" + "fmt" + + "github.com/aws/aws-sdk-go-v2/service/ecs" + ecsTypes "github.com/aws/aws-sdk-go-v2/service/ecs/types" +) + +// FakeECSClient is an in-memory implementation of ECSServicesAPI for testing. +// It returns the configured services for a cluster and reproduces the real AWS +// contract where DescribeServices rejects an empty Services list. +type FakeECSClient struct { + // ServiceArns is the list of service ARNs returned by ListServices. + ServiceArns []string + // Services is the list of services returned by DescribeServices. + Services []ecsTypes.Service +} + +func (f *FakeECSClient) ListServices(_ context.Context, _ *ecs.ListServicesInput, _ ...func(*ecs.Options)) (*ecs.ListServicesOutput, error) { + return &ecs.ListServicesOutput{ServiceArns: f.ServiceArns}, nil +} + +func (f *FakeECSClient) DescribeServices(_ context.Context, params *ecs.DescribeServicesInput, _ ...func(*ecs.Options)) (*ecs.DescribeServicesOutput, error) { + // Mirror the real AWS ECS API, which rejects an empty Services list with + // InvalidParameterException: Services cannot be empty. + if len(params.Services) == 0 { + return nil, fmt.Errorf("InvalidParameterException: Services cannot be empty") + } + return &ecs.DescribeServicesOutput{Services: f.Services}, nil +}