From 0d88fd14b45f2569c094a99187e1f430eccac00b Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Mon, 14 Sep 2026 16:13:46 +1000 Subject: [PATCH] feat: implement EC2 GetSecurityGroupsForVpc (#1026) --- docs/coverage/README.md | 2 +- docs/coverage/ec2/README.md | 4 +- spinifex/daemon/daemon.go | 1 + spinifex/gateway/ec2.go | 3 + spinifex/gateway/ec2/authz.go | 4 + .../ec2/vpc/GetSecurityGroupsForVpc.go | 41 +++ spinifex/gateway/ec2_authz_test.go | 13 + spinifex/gateway/ec2_emptyresult_test.go | 3 + .../ec2/vpc/GetSecurityGroupsForVpc_test.go | 234 ++++++++++++++++++ spinifex/handlers/ec2/vpc/security_group.go | 165 ++++++++++++ spinifex/handlers/ec2/vpc/service.go | 1 + spinifex/handlers/ec2/vpc/service_nats.go | 4 + 12 files changed, 472 insertions(+), 3 deletions(-) create mode 100644 spinifex/gateway/ec2/vpc/GetSecurityGroupsForVpc.go create mode 100644 spinifex/handlers/ec2/vpc/GetSecurityGroupsForVpc_test.go diff --git a/docs/coverage/README.md b/docs/coverage/README.md index b4f79acc6..1104cff9c 100644 --- a/docs/coverage/README.md +++ b/docs/coverage/README.md @@ -22,7 +22,7 @@ Spinifex serves the AWS APIs below. Every page counts the operations in the pinn | Service | Implemented | Modelled | Coverage | |---|---:|---:|---:| | [ACM](/coverage/acm) | 9 | 15 | 60.0% | -| [EC2](/coverage/ec2) | 122 | 625 | 19.5% | +| [EC2](/coverage/ec2) | 123 | 625 | 19.7% | | [ECR](/coverage/ecr) | 19 | 47 | 40.4% | | [ECS](/coverage/ecs) | 31 | 56 | 55.4% | | [EKS](/coverage/eks) | 34 | 56 | 60.7% | diff --git a/docs/coverage/ec2/README.md b/docs/coverage/ec2/README.md index 4b3363aa8..5aef1e7cc 100644 --- a/docs/coverage/ec2/README.md +++ b/docs/coverage/ec2/README.md @@ -18,7 +18,7 @@ tags: ## Overview -Spinifex implements **122 of the 625** operations (**19.5%**) in the EC2 `2016-11-15` API model. +Spinifex implements **123 of the 625** operations (**19.7%**) in the EC2 `2016-11-15` API model. ### Spot Instances Are a Mock @@ -492,7 +492,7 @@ Spot Instance Requests are a mock over the on-demand `RunInstances` path. A requ | `GetNetworkInsightsAccessScopeContent` | ❌ Not implemented | | `GetPasswordData` | ✅ Implemented | | `GetReservedInstancesExchangeQuote` | ❌ Not implemented | -| `GetSecurityGroupsForVpc` | ❌ Not implemented | +| `GetSecurityGroupsForVpc` | ✅ Implemented | | `GetSerialConsoleAccessStatus` | ✅ Implemented | | `GetSnapshotBlockPublicAccessState` | ❌ Not implemented | | `GetSpotPlacementScores` | ❌ Not implemented | diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index 18ac0fe7f..0877859f6 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -970,6 +970,7 @@ func (d *Daemon) subscribeAll() error { {"ec2.DeleteSecurityGroup", handleNATSRequest(d.node, d.vpcService.DeleteSecurityGroup), "spinifex-workers"}, {"ec2.DescribeSecurityGroups", handleNATSRequest(d.node, d.vpcService.DescribeSecurityGroups), "spinifex-workers"}, {"ec2.DescribeSecurityGroupRules", handleNATSRequest(d.node, d.vpcService.DescribeSecurityGroupRules), "spinifex-workers"}, + {"ec2.GetSecurityGroupsForVpc", handleNATSRequest(d.node, d.vpcService.GetSecurityGroupsForVpc), "spinifex-workers"}, {"ec2.AuthorizeSecurityGroupIngress", handleNATSRequest(d.node, d.vpcService.AuthorizeSecurityGroupIngress), "spinifex-workers"}, {"ec2.AuthorizeSecurityGroupEgress", handleNATSRequest(d.node, d.vpcService.AuthorizeSecurityGroupEgress), "spinifex-workers"}, {"ec2.RevokeSecurityGroupIngress", handleNATSRequest(d.node, d.vpcService.RevokeSecurityGroupIngress), "spinifex-workers"}, diff --git a/spinifex/gateway/ec2.go b/spinifex/gateway/ec2.go index 201206bad..5cbd92791 100644 --- a/spinifex/gateway/ec2.go +++ b/spinifex/gateway/ec2.go @@ -535,6 +535,9 @@ var ec2Actions = map[string]ec2Action{ "DescribeSecurityGroupRules": ec2Handler(func(ctx context.Context, input *ec2.DescribeSecurityGroupRulesInput, gw *GatewayConfig, accountID string) (any, error) { return gateway_ec2_vpc.DescribeSecurityGroupRules(ctx, input, gw.NATSConn, accountID) }), + "GetSecurityGroupsForVpc": ec2Handler(func(ctx context.Context, input *ec2.GetSecurityGroupsForVpcInput, gw *GatewayConfig, accountID string) (any, error) { + return gateway_ec2_vpc.GetSecurityGroupsForVpc(ctx, input, gw.NATSConn, accountID) + }), "AuthorizeSecurityGroupIngress": ec2Handler(func(ctx context.Context, input *ec2.AuthorizeSecurityGroupIngressInput, gw *GatewayConfig, accountID string) (any, error) { return gateway_ec2_vpc.AuthorizeSecurityGroupIngress(ctx, input, gw.NATSConn, accountID) }), diff --git a/spinifex/gateway/ec2/authz.go b/spinifex/gateway/ec2/authz.go index 06d3903d1..cc0f23403 100644 --- a/spinifex/gateway/ec2/authz.go +++ b/spinifex/gateway/ec2/authz.go @@ -214,6 +214,10 @@ var ec2Scopes = map[string][]*resourceScope{ "RevokeSecurityGroupIngress": {securityGroupScope}, "RevokeSecurityGroupEgress": {securityGroupScope}, + // Unlike the account-wide describes, this one names the VPC it reports on, + // so a policy scoped to one VPC has to apply. + "GetSecurityGroupsForVpc": {vpcScope}, + // Addresses. "AllocateAddress": {addressNewScope}, "ReleaseAddress": {addressScope}, diff --git a/spinifex/gateway/ec2/vpc/GetSecurityGroupsForVpc.go b/spinifex/gateway/ec2/vpc/GetSecurityGroupsForVpc.go new file mode 100644 index 000000000..685b55486 --- /dev/null +++ b/spinifex/gateway/ec2/vpc/GetSecurityGroupsForVpc.go @@ -0,0 +1,41 @@ +package gateway_ec2_vpc + +import ( + "context" + "errors" + + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_ec2_vpc "github.com/mulgadc/spinifex/spinifex/handlers/ec2/vpc" + "github.com/nats-io/nats.go" +) + +// MaxResults bounds from the EC2 model. A raw query request bypasses the +// client-side validation an SDK caller gets, so the range is enforced here +// rather than silently clamped into a page size the caller did not ask for. +const ( + minSGForVpcResults = 5 + maxSGForVpcResults = 1000 +) + +// VpcId is required by the model, so an absent one is MissingParameter before +// the request crosses NATS. Whether it names a VPC is the handler's call: only +// the store knows. +func GetSecurityGroupsForVpc(ctx context.Context, input *ec2.GetSecurityGroupsForVpcInput, natsConn *nats.Conn, accountID string) (ec2.GetSecurityGroupsForVpcOutput, error) { + var output ec2.GetSecurityGroupsForVpcOutput + if input == nil { + return output, errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.VpcId == nil || *input.VpcId == "" { + return output, errors.New(awserrors.ErrorMissingParameter) + } + if input.MaxResults != nil && (*input.MaxResults < minSGForVpcResults || *input.MaxResults > maxSGForVpcResults) { + return output, errors.New(awserrors.ErrorInvalidParameterValue) + } + svc := handlers_ec2_vpc.NewNATSVPCService(natsConn) + result, err := svc.GetSecurityGroupsForVpc(ctx, input, accountID) + if err != nil { + return output, err + } + return *result, nil +} diff --git a/spinifex/gateway/ec2_authz_test.go b/spinifex/gateway/ec2_authz_test.go index 1e2dd6299..f28eb9f6b 100644 --- a/spinifex/gateway/ec2_authz_test.go +++ b/spinifex/gateway/ec2_authz_test.go @@ -177,6 +177,19 @@ func TestEC2Request_RequestSpotInstancesChecksLaunchResources(t *testing.T) { assertDenied(t, dispatchEC2(t, gw, body)) } +// TestEC2Request_GetSecurityGroupsForVpcIsVpcScoped pins the one read action +// that names a resource: registered unscoped, a policy fencing one VPC would be +// bypassed by asking about another. +func TestEC2Request_GetSecurityGroupsForVpcIsVpcScoped(t *testing.T) { + gw := scopedPolicyGateway( + statement("Allow", "ec2:*", "*"), + statement("Deny", "ec2:GetSecurityGroupsForVpc", "arn:aws:ec2:*:*:vpc/vpc-prod"), + ) + + assertDenied(t, dispatchEC2(t, gw, "Action=GetSecurityGroupsForVpc&VpcId=vpc-prod")) + assertPermitted(t, dispatchEC2(t, gw, "Action=GetSecurityGroupsForVpc&VpcId=vpc-dev")) +} + func TestEC2Request_RejectsOversizedResourceList(t *testing.T) { var body strings.Builder body.WriteString("Action=TerminateInstances") diff --git a/spinifex/gateway/ec2_emptyresult_test.go b/spinifex/gateway/ec2_emptyresult_test.go index e4097b062..ad7a37146 100644 --- a/spinifex/gateway/ec2_emptyresult_test.go +++ b/spinifex/gateway/ec2_emptyresult_test.go @@ -63,6 +63,9 @@ func TestEC2Describe_EmptyResult_EmitsContainer(t *testing.T) { {"DescribeInstanceTypeOfferings", "DescribeInstanceTypeOfferings", "instanceTypeOfferingSet", func(t *testing.T) string { return render[ec2.DescribeInstanceTypeOfferingsInput](t, "DescribeInstanceTypeOfferings", ec2.DescribeInstanceTypeOfferingsOutput{}) }}, + {"GetSecurityGroupsForVpc", "GetSecurityGroupsForVpc", "securityGroupForVpcSet", func(t *testing.T) string { + return render[ec2.GetSecurityGroupsForVpcInput](t, "GetSecurityGroupsForVpc", ec2.GetSecurityGroupsForVpcOutput{}) + }}, } for _, tc := range cases { diff --git a/spinifex/handlers/ec2/vpc/GetSecurityGroupsForVpc_test.go b/spinifex/handlers/ec2/vpc/GetSecurityGroupsForVpc_test.go new file mode 100644 index 000000000..c3e376858 --- /dev/null +++ b/spinifex/handlers/ec2/vpc/GetSecurityGroupsForVpc_test.go @@ -0,0 +1,234 @@ +package handlers_ec2_vpc + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// groupIDsForVpc collects the ids one GetSecurityGroupsForVpc page reports. +func groupIDsForVpc(out *ec2.GetSecurityGroupsForVpcOutput) []string { + ids := make([]string, 0, len(out.SecurityGroupForVpcs)) + for _, g := range out.SecurityGroupForVpcs { + ids = append(ids, aws.StringValue(g.GroupId)) + } + return ids +} + +// describedGroupIDsForVpc is what DescribeSecurityGroups reports for the same +// VPC, so the narrower call can be held against the wider one rather than +// against a list the test itself built. +func describedGroupIDsForVpc(t *testing.T, svc *VPCServiceImpl, vpcID string) []string { + t.Helper() + out, err := svc.DescribeSecurityGroups(context.Background(), &ec2.DescribeSecurityGroupsInput{ + Filters: []*ec2.Filter{{Name: aws.String("vpc-id"), Values: []*string{aws.String(vpcID)}}}, + }, testAccountID) + require.NoError(t, err) + ids := make([]string, 0, len(out.SecurityGroups)) + for _, sg := range out.SecurityGroups { + ids = append(ids, aws.StringValue(sg.GroupId)) + } + return ids +} + +func TestGetSecurityGroupsForVpc_MatchesDescribeForSameVpc(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + vpcID := createTestVPC(t, svc, "10.0.0.0/16") + web := createTestSG(t, svc, vpcID, "web-sg") + db := createTestSG(t, svc, vpcID, "db-sg") + + out, err := svc.GetSecurityGroupsForVpc(context.Background(), &ec2.GetSecurityGroupsForVpcInput{ + VpcId: aws.String(vpcID), + }, testAccountID) + require.NoError(t, err) + + ids := groupIDsForVpc(out) + assert.ElementsMatch(t, describedGroupIDsForVpc(t, svc, vpcID), ids) + assert.Contains(t, ids, web) + assert.Contains(t, ids, db) + assert.Len(t, ids, 3, "the VPC's default group must be reported alongside the two created ones") + assert.Nil(t, out.NextToken) + + for _, g := range out.SecurityGroupForVpcs { + assert.Equal(t, vpcID, aws.StringValue(g.PrimaryVpcId)) + assert.Equal(t, testAccountID, aws.StringValue(g.OwnerId)) + assert.NotEmpty(t, aws.StringValue(g.GroupName)) + } +} + +func TestGetSecurityGroupsForVpc_ExcludesOtherVpcInSameAccount(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + vpcID := createTestVPC(t, svc, "10.0.0.0/16") + otherVpcID := createTestVPC(t, svc, "10.1.0.0/16") + mine := createTestSG(t, svc, vpcID, "mine-sg") + theirs := createTestSG(t, svc, otherVpcID, "theirs-sg") + + out, err := svc.GetSecurityGroupsForVpc(context.Background(), &ec2.GetSecurityGroupsForVpcInput{ + VpcId: aws.String(vpcID), + }, testAccountID) + require.NoError(t, err) + + ids := groupIDsForVpc(out) + assert.Contains(t, ids, mine) + assert.NotContains(t, ids, theirs) +} + +func TestGetSecurityGroupsForVpc_ExcludesOtherAccount(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + const otherAccountID = "210987654321" + + vpcID := createTestVPC(t, svc, "10.0.0.0/16") + mine := createTestSG(t, svc, vpcID, "mine-sg") + + otherVpc, err := svc.CreateVpc(context.Background(), &ec2.CreateVpcInput{ + CidrBlock: aws.String("10.2.0.0/16"), + }, otherAccountID) + require.NoError(t, err) + otherSG, err := svc.CreateSecurityGroup(context.Background(), &ec2.CreateSecurityGroupInput{ + GroupName: aws.String("other-account-sg"), + Description: aws.String("test sg"), + VpcId: otherVpc.Vpc.VpcId, + }, otherAccountID) + require.NoError(t, err) + + out, err := svc.GetSecurityGroupsForVpc(context.Background(), &ec2.GetSecurityGroupsForVpcInput{ + VpcId: aws.String(vpcID), + }, testAccountID) + require.NoError(t, err) + + ids := groupIDsForVpc(out) + assert.Contains(t, ids, mine) + assert.NotContains(t, ids, aws.StringValue(otherSG.GroupId)) + + // The other account's own VPC is invisible to this caller too, rather than + // returning that account's groups. + _, err = svc.GetSecurityGroupsForVpc(context.Background(), &ec2.GetSecurityGroupsForVpcInput{ + VpcId: otherVpc.Vpc.VpcId, + }, testAccountID) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorInvalidVpcIDNotFound, err.Error()) +} + +func TestGetSecurityGroupsForVpc_MissingVpcId(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + + _, err := svc.GetSecurityGroupsForVpc(context.Background(), &ec2.GetSecurityGroupsForVpcInput{}, testAccountID) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorMissingParameter, err.Error()) +} + +func TestGetSecurityGroupsForVpc_UnknownVpcIsNotFound(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + createTestVPC(t, svc, "10.0.0.0/16") + + out, err := svc.GetSecurityGroupsForVpc(context.Background(), &ec2.GetSecurityGroupsForVpcInput{ + VpcId: aws.String("vpc-00000000000000000"), + }, testAccountID) + require.Error(t, err, "an unknown VPC must not be reported as a VPC with no groups") + assert.Nil(t, out) + assert.Equal(t, awserrors.ErrorInvalidVpcIDNotFound, err.Error()) +} + +// A VPC whose only group is the default one still answers, so the not-found +// path above cannot be firing on an empty scan result. +func TestGetSecurityGroupsForVpc_DefaultGroupOnly(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + vpcID := createTestVPC(t, svc, "10.0.0.0/16") + + out, err := svc.GetSecurityGroupsForVpc(context.Background(), &ec2.GetSecurityGroupsForVpcInput{ + VpcId: aws.String(vpcID), + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.SecurityGroupForVpcs, 1) + assert.Equal(t, "default", aws.StringValue(out.SecurityGroupForVpcs[0].GroupName)) + assert.Nil(t, out.NextToken) +} + +func TestGetSecurityGroupsForVpc_PagesEveryGroupOnce(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + vpcID := createTestVPC(t, svc, "10.0.0.0/16") + + expected := describedGroupIDsForVpc(t, svc, vpcID) + for _, name := range []string{"sg-a", "sg-b", "sg-c", "sg-d", "sg-e", "sg-f", "sg-g"} { + expected = append(expected, createTestSG(t, svc, vpcID, name)) + } + + var seen []string + var token *string + for page := 0; ; page++ { + require.Less(t, page, 10, "paging did not terminate") + + out, err := svc.GetSecurityGroupsForVpc(context.Background(), &ec2.GetSecurityGroupsForVpcInput{ + VpcId: aws.String(vpcID), + MaxResults: aws.Int64(5), + NextToken: token, + }, testAccountID) + require.NoError(t, err) + assert.LessOrEqual(t, len(out.SecurityGroupForVpcs), 5) + + seen = append(seen, groupIDsForVpc(out)...) + token = out.NextToken + if token == nil { + break + } + } + + assert.ElementsMatch(t, expected, seen) + assert.Len(t, seen, len(expected), "a group was reported on more than one page") +} + +func TestGetSecurityGroupsForVpc_MalformedNextToken(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + vpcID := createTestVPC(t, svc, "10.0.0.0/16") + + _, err := svc.GetSecurityGroupsForVpc(context.Background(), &ec2.GetSecurityGroupsForVpcInput{ + VpcId: aws.String(vpcID), + NextToken: aws.String("not-a-token"), + }, testAccountID) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error()) +} + +func TestGetSecurityGroupsForVpc_Filters(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + vpcID := createTestVPC(t, svc, "10.0.0.0/16") + web := createTestSG(t, svc, vpcID, "web-sg") + createTestSG(t, svc, vpcID, "db-sg") + + out, err := svc.GetSecurityGroupsForVpc(context.Background(), &ec2.GetSecurityGroupsForVpcInput{ + VpcId: aws.String(vpcID), + Filters: []*ec2.Filter{{Name: aws.String("group-name"), Values: []*string{aws.String("web-sg")}}}, + }, testAccountID) + require.NoError(t, err) + assert.Equal(t, []string{web}, groupIDsForVpc(out)) + + // vpc-id is DescribeSecurityGroups' name for it; this model says + // primary-vpc-id, and an unsupported filter must not be ignored. + _, err = svc.GetSecurityGroupsForVpc(context.Background(), &ec2.GetSecurityGroupsForVpcInput{ + VpcId: aws.String(vpcID), + Filters: []*ec2.Filter{{Name: aws.String("vpc-id"), Values: []*string{aws.String(vpcID)}}}, + }, testAccountID) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error()) + + out, err = svc.GetSecurityGroupsForVpc(context.Background(), &ec2.GetSecurityGroupsForVpcInput{ + VpcId: aws.String(vpcID), + Filters: []*ec2.Filter{{Name: aws.String("primary-vpc-id"), Values: []*string{aws.String(vpcID)}}}, + }, testAccountID) + require.NoError(t, err) + assert.Len(t, out.SecurityGroupForVpcs, 3) +} diff --git a/spinifex/handlers/ec2/vpc/security_group.go b/spinifex/handlers/ec2/vpc/security_group.go index 729d57a05..c46f5a5a4 100644 --- a/spinifex/handlers/ec2/vpc/security_group.go +++ b/spinifex/handlers/ec2/vpc/security_group.go @@ -9,6 +9,7 @@ import ( "net" "regexp" "slices" + "strconv" "strings" "time" @@ -533,6 +534,170 @@ func sgIngressCIDRMatchesAny(rules []SGRule, values []string) bool { return false } +// getSecurityGroupsForVpcValidFilters defines the set of filter names accepted +// by GetSecurityGroupsForVpc. The model names the VPC primary-vpc-id here, and +// does not accept the vpc-id or ip-permission.cidr DescribeSecurityGroups takes. +var getSecurityGroupsForVpcValidFilters = map[string]bool{ + "group-id": true, + "group-name": true, + "description": true, + "owner-id": true, + "primary-vpc-id": true, +} + +// defaultSGForVpcPerPage caps a request that names no MaxResults, matching the +// model's ceiling. +const defaultSGForVpcPerPage = 1000 + +// GetSecurityGroupsForVpc lists the security groups an interface in the named +// VPC can be associated with. It is the same account-prefixed scan +// DescribeSecurityGroups runs, projected to the fields the model returns: the +// rule set is deliberately absent, since a caller wanting rules has +// DescribeSecurityGroupRules. +func (s *VPCServiceImpl) GetSecurityGroupsForVpc(ctx context.Context, input *ec2.GetSecurityGroupsForVpcInput, accountID string) (*ec2.GetSecurityGroupsForVpcOutput, error) { + if input == nil || input.VpcId == nil || *input.VpcId == "" { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + vpcID := *input.VpcId + + // An unknown VPC is not-found rather than an empty list: an empty list is + // indistinguishable from a VPC that genuinely holds no groups, and would + // send the caller on to create an interface in a VPC that does not exist. + if err := s.requireVPCExists(ctx, accountID, vpcID); err != nil { + return nil, err + } + + parsedFilters, err := filterutil.ParseFilters(input.Filters, getSecurityGroupsForVpcValidFilters) + if err != nil { + slog.WarnContext(ctx, "GetSecurityGroupsForVpc: invalid filter", "err", err) + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + + prefix := accountID + "." + keys, err := s.sgKV.Keys(ctx) + if err != nil && !errors.Is(err, jetstream.ErrNoKeysFound) { + return nil, errors.New(awserrors.ErrorServerInternal) + } + + groups := []*ec2.SecurityGroupForVpc{} + for _, key := range keys { + if key == utils.VersionKey || !strings.HasPrefix(key, prefix) { + continue + } + entry, err := s.sgKV.Get(ctx, key) + if err != nil { + slog.ErrorContext(ctx, "GetSecurityGroupsForVpc: SG read failed", "key", key, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + var record SecurityGroupRecord + if err := json.Unmarshal(entry.Value(), &record); err != nil { + slog.ErrorContext(ctx, "GetSecurityGroupsForVpc: SG unmarshal failed", "key", key, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + + if record.VpcId != vpcID { + continue + } + if len(parsedFilters) > 0 && !sgForVpcMatchesFilters(&record, accountID, parsedFilters) { + continue + } + + groups = append(groups, &ec2.SecurityGroupForVpc{ + GroupId: aws.String(record.GroupId), + GroupName: aws.String(record.GroupName), + Description: aws.String(record.Description), + OwnerId: aws.String(accountID), + PrimaryVpcId: aws.String(record.VpcId), + Tags: utils.MapToEC2Tags(record.Tags), + }) + } + + // The KV key order is not guaranteed stable across calls, so a token + // addressing an offset into an unsorted set could skip or repeat a group. + slices.SortFunc(groups, func(a, b *ec2.SecurityGroupForVpc) int { + return strings.Compare(aws.StringValue(a.GroupId), aws.StringValue(b.GroupId)) + }) + + page, nextToken, err := pageSecurityGroupsForVpc(groups, input.NextToken, input.MaxResults) + if err != nil { + slog.WarnContext(ctx, "GetSecurityGroupsForVpc: invalid NextToken", "next_token", aws.StringValue(input.NextToken)) + return nil, err + } + + slog.InfoContext(ctx, "GetSecurityGroupsForVpc completed", + "count", len(page), "total", len(groups), "vpcId", vpcID, "accountID", accountID) + + return &ec2.GetSecurityGroupsForVpcOutput{ + SecurityGroupForVpcs: page, + NextToken: nextToken, + }, nil +} + +// sgForVpcMatchesFilters applies the GetSecurityGroupsForVpc filter set. The +// owner is the account the scan is prefixed by, so owner-id can only match the +// caller's own account. +func sgForVpcMatchesFilters(record *SecurityGroupRecord, accountID string, filters map[string][]string) bool { + for name, values := range filters { + if strings.HasPrefix(name, "tag:") { + continue + } + + switch name { + case "group-id": + if !filterutil.MatchesAny(values, record.GroupId) { + return false + } + case "group-name": + if !filterutil.MatchesAny(values, record.GroupName) { + return false + } + case "description": + if !filterutil.MatchesAny(values, record.Description) { + return false + } + case "owner-id": + if !filterutil.MatchesAny(values, accountID) { + return false + } + case "primary-vpc-id": + if !filterutil.MatchesAny(values, record.VpcId) { + return false + } + default: + return false + } + } + + return filterutil.MatchesTags(filters, record.Tags) +} + +// pageSecurityGroupsForVpc slices one page out of a sorted group set, using an +// opaque integer offset as the token. The last page carries a nil token. +func pageSecurityGroupsForVpc(groups []*ec2.SecurityGroupForVpc, token *string, maxResults *int64) ([]*ec2.SecurityGroupForVpc, *string, error) { + start := 0 + if aws.StringValue(token) != "" { + n, err := strconv.Atoi(*token) + if err != nil || n < 0 { + return nil, nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + start = n + } + if start > len(groups) { + start = len(groups) + } + + size := defaultSGForVpcPerPage + if maxResults != nil && *maxResults > 0 { + size = int(*maxResults) + } + + end := start + size + if end >= len(groups) { + return groups[start:], nil, nil + } + return groups[start:end], aws.String(strconv.Itoa(end)), nil +} + // describeSecurityGroupRulesValidFilters defines the set of filter names accepted by DescribeSecurityGroupRules. var describeSecurityGroupRulesValidFilters = map[string]bool{ "group-id": true, diff --git a/spinifex/handlers/ec2/vpc/service.go b/spinifex/handlers/ec2/vpc/service.go index 59477c4cf..81d981906 100644 --- a/spinifex/handlers/ec2/vpc/service.go +++ b/spinifex/handlers/ec2/vpc/service.go @@ -25,6 +25,7 @@ type VPCService interface { DeleteSecurityGroup(ctx context.Context, input *ec2.DeleteSecurityGroupInput, accountID string) (*ec2.DeleteSecurityGroupOutput, error) DescribeSecurityGroups(ctx context.Context, input *ec2.DescribeSecurityGroupsInput, accountID string) (*ec2.DescribeSecurityGroupsOutput, error) DescribeSecurityGroupRules(ctx context.Context, input *ec2.DescribeSecurityGroupRulesInput, accountID string) (*ec2.DescribeSecurityGroupRulesOutput, error) + GetSecurityGroupsForVpc(ctx context.Context, input *ec2.GetSecurityGroupsForVpcInput, accountID string) (*ec2.GetSecurityGroupsForVpcOutput, error) AuthorizeSecurityGroupIngress(ctx context.Context, input *ec2.AuthorizeSecurityGroupIngressInput, accountID string) (*ec2.AuthorizeSecurityGroupIngressOutput, error) AuthorizeSecurityGroupEgress(ctx context.Context, input *ec2.AuthorizeSecurityGroupEgressInput, accountID string) (*ec2.AuthorizeSecurityGroupEgressOutput, error) RevokeSecurityGroupIngress(ctx context.Context, input *ec2.RevokeSecurityGroupIngressInput, accountID string) (*ec2.RevokeSecurityGroupIngressOutput, error) diff --git a/spinifex/handlers/ec2/vpc/service_nats.go b/spinifex/handlers/ec2/vpc/service_nats.go index e5f864e9f..a89b9d905 100644 --- a/spinifex/handlers/ec2/vpc/service_nats.go +++ b/spinifex/handlers/ec2/vpc/service_nats.go @@ -89,6 +89,10 @@ func (s *NATSVPCService) DescribeSecurityGroupRules(ctx context.Context, input * return utils.NATSRequest[ec2.DescribeSecurityGroupRulesOutput](ctx, s.natsConn, "ec2.DescribeSecurityGroupRules", input, 30*time.Second, accountID) } +func (s *NATSVPCService) GetSecurityGroupsForVpc(ctx context.Context, input *ec2.GetSecurityGroupsForVpcInput, accountID string) (*ec2.GetSecurityGroupsForVpcOutput, error) { + return utils.NATSRequest[ec2.GetSecurityGroupsForVpcOutput](ctx, s.natsConn, "ec2.GetSecurityGroupsForVpc", input, 30*time.Second, accountID) +} + func (s *NATSVPCService) AuthorizeSecurityGroupIngress(ctx context.Context, input *ec2.AuthorizeSecurityGroupIngressInput, accountID string) (*ec2.AuthorizeSecurityGroupIngressOutput, error) { return utils.NATSRequest[ec2.AuthorizeSecurityGroupIngressOutput](ctx, s.natsConn, "ec2.AuthorizeSecurityGroupIngress", input, 30*time.Second, accountID) }