feat(ec2): lookup security group by name #17246
Conversation
|
|
github-actions
bot
added
the
@aws-cdk/aws-ec2
|
The first build failed because property securityGroupId was changed to optional: I added this line to the breaking changes to allow this change. Please check if this change is allowed. |
|
@jumic sorry for asking this question but I am not that familiar with the approval/pipeline process of cdk. I just by coincident bumbed into this issue today when setting up a multi account pipeline. Therefore I would really appreciate if you could give me an indication when this feature hits GA. Would it be deployed to next release? Best, and thank you for your work! |
packages/@aws-cdk/aws-ec2/README.md
Outdated
| The result of the `SecurityGroup.fromLookupAttributes` operation will be written to a file called `cdk.context.json`. You must commit this file to source control so that the lookup values are available in non-privileged environments such as CI build steps, and to ensure your template builds are repeatable. | ||
|
|
||
| ```ts fixture=with-vpc | ||
| const sg = ec2.SecurityGroup.fromLookupAttributes( |
There was a problem hiding this comment.
What do you think about dedicated fromLookupById and fromLookupByName methods? They could both still use a (private) fromLookupAttributes method, but it may make the API intent a bit more clear.
There was a problem hiding this comment.
I tried to create an API similar to VpcLookupOptions. Now I compared it to fromLookupById and fromLookupByName again. For me, this seems to be a better option because the necessary parameters are defined by the API. I've implemented the following changes:
- Deprecated method
fromLookup, will be replaced byfromLookupById(same parameters). - New method
fromLookupByIdwith parametersecurityGroupId. Parametervpcnot defined because the security group id is sufficient to clearly identify the security group. - New method
fromLookupByNamewith parameterssecurityGroupNameandvpc.vpcis defined as required parameter because it avoids problems if the same security group name is used multiple times per account and region. A security group is used in context of a VPC. Therefore, the VPC should be available in the CDK code. If it should be defined as optional later, we could changed it easily (no breaking change).
mergify
bot
dismissed
njlynch’s stale review
Pull request has been modified.
mergify
bot
dismissed
njlynch’s stale review
Pull request has been modified.
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Support looking up a security group by name. Currently, looking up a security group is only possible by ID. This PR enhances the existing implementation to support lookup by security group name. `securityGroupName` or `securityGroupId` can be passed to the new method `SecurityGroup.fromLookupAttributes`. In addition, property `vpc` provides the option to restrict the lookup method to a specific VPC. If no or more than one security group is found, an error is thrown. Closes aws#4241. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Support looking up a security group by name. Currently, looking up a security group is only possible by ID. This PR enhances the existing implementation to support lookup by security group name. `securityGroupName` or `securityGroupId` can be passed to the new method `SecurityGroup.fromLookupAttributes`. In addition, property `vpc` provides the option to restrict the lookup method to a specific VPC. If no or more than one security group is found, an error is thrown. Closes aws#4241. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Support looking up a security group by name.
Currently, looking up a security group is only possible by ID. This PR enhances the existing implementation to support lookup by security group name.
securityGroupNameorsecurityGroupIdcan be passed to the new methodSecurityGroup.fromLookupAttributes. In addition, propertyvpcprovides the option to restrict the lookup method to a specific VPC.If no or more than one security group is found, an error is thrown.
Closes #4241.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license