Add EC2 support for GetSecurityGroupsForVpc API operation (#12602) #12615

iamramtin · 2025-05-13T15:59:04Z

Motivation

This PR adds support for the EC2 API's GetSecurityGroupsForVpc operation, which retrieves security groups associated with a specified VPC. This operation provides a convenient way to filter security groups by VPC ID, without having to use the more generic filters syntax of DescribeSecurityGroups.

Changes

Added implementation for the GetSecurityGroupsForVpc EC2 API operation
Built the implementation to leverage the existing DescribeSecurityGroups functionality with a VPC filter
Added integration tests to verify both normal operation and edge cases
Ensured proper validation for empty VPC IDs
Made sure the response format matches AWS API specifications

Testing

You can test this new functionality in the following way:

# Create a VPC and capture its ID
VPC_ID=$(awslocal ec2 create-vpc --cidr-block 10.0.0.0/16 --query Vpc.VpcId --output text)

# Create a security group in the VPC
awslocal ec2 create-security-group --group-name my-sg --description "My security group" --vpc-id $VPC_ID

# Filter security groups by VPC ID using describe-security-groups
awslocal ec2 describe-security-groups --filters Name=vpc-id,Values=$VPC_ID

# Use the new get-security-groups-for-vpc operation (should do the same as above)
awslocal ec2 get-security-groups-for-vpc --vpc-id $VPC_ID

Additionally, the PR includes integration tests that verify regular functionality and edge cases.

localstack-bot

Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.

localstack-bot · 2025-05-13T15:59:15Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

iamramtin · 2025-05-13T16:44:04Z

I have read the CLA Document and I hereby sign the CLA

iamramtin · 2025-05-14T10:37:40Z

How can I add label semver: minor?

viren-nadkarni

Hi @iamramtin, thanks for raising the PR! I have added my review comments below. I will also add the semver label.

localstack-core/localstack/services/ec2/provider.py

tests/aws/services/ec2/test_ec2.py

…#12602) Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>

…oupName when adding ingress rules This avoids issues with security groups in different VPCs Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>

Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>

tests/aws/services/ec2/test_ec2.py

…p fixture (localstack#12602) Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>

…stack#12602) - Structure tests to validate core filtering functionality - Add SortingTransformer to normalize security group ordering in responses - Verify both initial VPC state and post-security-group addition state Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>

iamramtin · 2025-05-18T18:39:13Z

@viren-nadkarni thanks for your review. I took a closer look at the other snapshot tests in the codebase and reworked my approach. I've removed the assertions and now rely on the snapshot framework for validation similar to other parity tests.

I've also added some FIXME/TODOs to highlight potential issues / inconsistencies I came across. Let me know if this looks good, or if you'd prefer any further adjustments to the testing approach.

viren-nadkarni

Hi @iamramtin, the implementation currently breaks when a security group has tags. Could you please fix it and include the necessary test cases?

$ awslocal ec2 create-vpc --cidr-block 10.1.0.0/16 --query Vpc.VpcId
"vpc-596131631cff9cea1"

$ awslocal ec2 create-security-group --group-name hello --description world --vpc vpc-596131631cff9cea1 --tag-specifications "ResourceType=security-group,Tags=[{Key=colour,Value=blue}]"{
    "GroupId": "sg-cbf52384c65ddc0b6",
    "Tags": [
        {
            "Key": "colour",
            "Value": "blue"
        }
    ],
    "SecurityGroupArn": "arn:aws:ec2:eu-central-1:000000000000:security-group/sg-cbf52384c65ddc0b6"
}

$ awslocal ec2 get-security-groups-for-vpc --vpc-id vpc-596131631cff9cea1

An error occurred (InternalError) when calling the GetSecurityGroupsForVpc operation (reached max retries: 4): exception while calling ec2.GetSecurityGroupsForVpc: Traceback (most recent call last):
  File "/home/viren/.virtualenvs/ls_venv/lib/python3.11/site-packages/rolo/gateway/chain.py", line 166, in handle
    handler(self, self.context, response)
  File "/home/viren/repo/localstack/localstack-core/localstack/aws/handlers/service.py", line 113, in __call__
    handler(chain, context, response)
  File "/home/viren/repo/localstack/localstack-core/localstack/aws/handlers/service.py", line 83, in __call__
    skeleton_response = self.skeleton.invoke(context)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/viren/repo/localstack/localstack-core/localstack/aws/skeleton.py", line 154, in invoke
    return self.dispatch_request(serializer, context, instance)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/viren/repo/localstack/localstack-core/localstack/aws/skeleton.py", line 168, in dispatch_request
    result = handler(context, instance) or {}
             ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/viren/repo/localstack/localstack-core/localstack/aws/forwarder.py", line 133, in _call
    return handler(context, req)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/viren/repo/localstack/localstack-core/localstack/aws/skeleton.py", line 118, in __call__
    return self.fn(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/viren/repo/localstack/localstack-core/localstack/aws/api/core.py", line 163, in operation_marker
    return fn(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^
  File "/home/viren/repo/localstack/localstack-core/localstack/services/ec2/provider.py", line 556, in get_security_groups_for_vpc
    sgs = [
          ^
  File "/home/viren/repo/localstack/localstack-core/localstack/services/ec2/provider.py", line 563, in <listcomp>
    Tags=[{"Key": k, "Value": v} for k, v in (sg.get_tags() or {}).items()],
                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'list' object has no attribute 'items'

viren-nadkarni

Thanks for bearing with me. If you could clarify these last concerns, we can merge this.

tests/aws/services/ec2/test_ec2.py

localstack-core/localstack/services/ec2/provider.py

…12602) Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>

viren-nadkarni

Looks good, thanks for the contribution @iamramtin!

localstack-bot reviewed May 13, 2025

View reviewed changes

iamramtin force-pushed the feature/ec2-get-security-groups-for-vpc branch from bf228f3 to b7cc833 Compare May 13, 2025 15:59

localstack-bot added a commit that referenced this pull request May 13, 2025

@iamramtin has signed the CLA in #12615

d47c14f

iamramtin marked this pull request as ready for review May 14, 2025 07:10

iamramtin requested review from viren-nadkarni and macnev2013 as code owners May 14, 2025 07:10

viren-nadkarni reviewed May 14, 2025

View reviewed changes

viren-nadkarni added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label May 14, 2025

Add EC2 support for GetSecurityGroupsForVpc API operation (localstack…

cf63323

…#12602) Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>

iamramtin force-pushed the feature/ec2-get-security-groups-for-vpc branch from b7cc833 to cf63323 Compare May 14, 2025 20:10

iamramtin added 2 commits May 14, 2025 22:11

Update ec2_create_security_group fixture to use GroupId instead of Gr…

37330c5

…oupName when adding ingress rules This avoids issues with security groups in different VPCs Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>

Add GetSecurityGroupsForVpc API parity tests (localstack#12602)

629f280

Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>

iamramtin requested a review from dominikschubert as a code owner May 14, 2025 20:13

Merge branch 'master' into feature/ec2-get-security-groups-for-vpc

eb6b6c6

Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>

iamramtin force-pushed the feature/ec2-get-security-groups-for-vpc branch 2 times, most recently from ae55ba2 to facdeec Compare May 14, 2025 21:38

iamramtin requested a review from viren-nadkarni May 15, 2025 05:40

viren-nadkarni removed the request for review from dominikschubert May 15, 2025 06:44

viren-nadkarni requested changes May 15, 2025

View reviewed changes

tests/aws/services/ec2/test_ec2.py Outdated Show resolved Hide resolved

tests/aws/services/ec2/test_ec2.py Outdated Show resolved Hide resolved

tests/aws/services/ec2/test_ec2.py Outdated Show resolved Hide resolved

Ensure tests work correctly with the updated ec2_create_security_grou…

fd20316

…p fixture (localstack#12602) Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>

iamramtin force-pushed the feature/ec2-get-security-groups-for-vpc branch from facdeec to fd20316 Compare May 17, 2025 12:00

iamramtin force-pushed the feature/ec2-get-security-groups-for-vpc branch from 5332c4d to 05ecace Compare May 18, 2025 18:16

iamramtin requested a review from viren-nadkarni May 18, 2025 18:39

viren-nadkarni requested changes May 19, 2025

View reviewed changes

iamramtin requested a review from viren-nadkarni May 19, 2025 13:15

viren-nadkarni requested changes May 19, 2025

View reviewed changes

iamramtin force-pushed the feature/ec2-get-security-groups-for-vpc branch from 359355d to 48a445d Compare May 19, 2025 13:50

Normalize tag format in GetSecurityGroupsForVpc response (localstack#…

6bac29d

…12602) Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>

iamramtin force-pushed the feature/ec2-get-security-groups-for-vpc branch from 48a445d to 6bac29d Compare May 19, 2025 13:54

iamramtin requested a review from viren-nadkarni May 19, 2025 16:48

viren-nadkarni approved these changes May 20, 2025

View reviewed changes

viren-nadkarni merged commit 4925ef7 into localstack:master May 20, 2025
30 checks passed

viren-nadkarni mentioned this pull request May 20, 2025

feature request: Add EC2 support for GetSecurityGroupsForVpc API operation #12602

Closed

1 task

Uh oh!

Add EC2 support for GetSecurityGroupsForVpc API operation (#12602) #12615

Add EC2 support for GetSecurityGroupsForVpc API operation (#12602) #12615

Uh oh!

Conversation

iamramtin commented May 13, 2025

Motivation

Changes

Testing

Uh oh!

localstack-bot left a comment

Choose a reason for hiding this comment

Uh oh!

localstack-bot commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iamramtin commented May 13, 2025

Uh oh!

iamramtin commented May 14, 2025

Uh oh!

viren-nadkarni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iamramtin commented May 18, 2025

Uh oh!

viren-nadkarni left a comment

Choose a reason for hiding this comment

Uh oh!

viren-nadkarni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

viren-nadkarni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

localstack-bot commented May 13, 2025 •

edited

Loading