Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EC2: Security Groups lookup by Owner #30331

Closed
1 of 2 tasks
eschirle opened this issue May 24, 2024 · 6 comments · Fixed by #30625 or mannjaro/serverless-bedrock-proxy#2
Closed
1 of 2 tasks

EC2: Security Groups lookup by Owner #30331

eschirle opened this issue May 24, 2024 · 6 comments · Fixed by #30625 or mannjaro/serverless-bedrock-proxy#2
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@eschirle
Copy link

eschirle commented May 24, 2024

Describe the feature

SecurityGroup.fromLookupByName() provides an option for looking up a security group by name - but this fails if multiple SGs are found with the provided name.

This feature is for the ability to filter security groups by Owner as well.

Use Case

I have a use case where I'd like to use a Baseline SecurityGroup for Lambda Functions that are a part of a custom construct to avoid creating a new SG and using up more Hyperplane ENIs. I can lookup by name "BaselineSecurityGroup", but my VPC is shared across multiple micro accounts, and so multiple SecurityGroups are returned.

This feature would include the option to filter SecurityGroups by SecurityGroupName and Owner, so that I can grab the SG when there are multiple with the same name in a single VPC.

Proposed Solution

Either adding an optional owner input to fromLookupByName or add a new method fromLookupByNameAndOwner in security-group.ts

public static fromLookupByName(scope: Construct, id: string, securityGroupName: string, vpc: IVpc, owner?: string) { return this.fromLookupAttributes(scope, id, { securityGroupName, vpc, owner }); }

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.142.0

Environment details (OS name and version, etc.)

N/A

@eschirle eschirle added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels May 24, 2024
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label May 24, 2024
@eschirle
Copy link
Author

eschirle commented May 24, 2024

I created a PR with a possible solution for this #30334

@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels May 24, 2024
@khushail
Copy link
Contributor

@eschirle , this makes sense to me. Thanks for reporting and volunteering for PR contribution !

@khushail khushail added effort/small Small work item – less than a day of effort p2 and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels May 24, 2024
@eschirle
Copy link
Author

@eschirle , this makes sense to me. Thanks for reporting and volunteering for PR contribution !

Hi @khushail - I made some updates to the PR today and marked it as ready for review. I was able to run unit tests and integration tests in my development environment but I think it needs some additional changes to pass validation.
Could you please take a look?

@khushail
Copy link
Contributor

khushail commented May 30, 2024

@eschirle , thanks for your contribution. Usually community PRs ar first reviewed by approved community reviewers and then by Core CDK Team and you could also ask for help in the mentioned slack channel in this doc. Here are the approved guidelines.

Copy link

github-actions bot commented Aug 7, 2024

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

1 similar comment
Copy link

github-actions bot commented Aug 7, 2024

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2024
duranbe pushed a commit to duranbe/aws-cdk that referenced this issue Aug 25, 2024
Closes aws#30331.

This will improve the security group lookup functionality for importing existing security groups into a CDK stack.

I added the ability to lookup existing security groups via more filters. Filters are supported by the [DescribeSecurityGroups API](https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_DescribeSecurityGroups.html), and using these filters can be immensely useful for looking up existing security groups, especially if your account or organization follows predictable rules regarding things like security group tags.

I added unit tests similar to the ones that test the normal lookup by ID or name.

- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
2 participants