feat(vpcv2): vpc peering connection construct #31645
Conversation
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
…/1kaileychen/aws-cdk into vpc_peering_connection_construct
shikha372
force-pushed
the
vpc_peering_connection_construct
branch
from
a85eca7 to
7cd8009
Compare
aws-cdk-automation
added
the
pr/needs-maintainer-review
shikha372
force-pushed
the
vpc_peering_connection_construct
branch
from
102491d to
14d789a
Compare
shikha372
force-pushed
the
vpc_peering_connection_construct
branch
from
14d789a to
c0949b5
Compare
shikha372
force-pushed
the
vpc_peering_connection_construct
branch
from
9aac380 to
a6c29e4
Compare
shikha372
force-pushed
the
vpc_peering_connection_construct
branch
from
a6c29e4 to
882a0b5
Compare
|
|
||
| **Case 2: Same Account and Cross Region Peering Connection** | ||
|
|
||
| There is no difference from Case 1 when calling `createPeeringConnection`. The only change is that one of the VPCs are created in another stack with a different region. |
There was a problem hiding this comment.
can you also mention that customers need to create a VPC object using the fromVpcAttributes function.
| primaryAddressBlock: IpAddresses.ipv4('10.1.0.0/16'), | ||
| }); | ||
|
|
||
| const vpcB = VpcV2.fromVpcV2Attributes(stackA, 'ImportedVpcB', { |
There was a problem hiding this comment.
why do you added V2 name in the function name, I believe this abstract method exists only in the VpcV2 class.
There was a problem hiding this comment.
this function exists in VpcV2 but there is another function with name fromVpcAttributes that resides in main module currently, imo it is better to name it VpcV2 as both the VPCs have difference in terms of required attributes and fields needed to import, eg. in VPC we don't need to provide AZ details but in VPCv2 it is required.
| const stackA = new Stack(app, 'VpcStackA', { env: { account: '000000000000', region: 'us-east-1' } }); | ||
| const stackB = new Stack(app, 'VpcStackB', { env: { account: '111111111111', region: 'us-west-2' } }); |
There was a problem hiding this comment.
it should be same account
| primaryAddressBlock: IpAddresses.ipv4('10.0.0.0/16'), | ||
| }); | ||
|
|
||
| const acceptorRoleArn = acceptorVpc.createAcceptorVpcRole('000000000000') // Requestor account ID |
| ``` | ||
|
|
||
| For more information, see [Delete a VPC peering connection](https://docs.aws.amazon.com/vpc/latest/peering/create-vpc-peering-connection.html#delete-vpc-peering-connection). | ||
|
|
There was a problem hiding this comment.
I think this section is not required .. or the question is, if the customer removed the peering connection from the CDK application, I am assuming that the peer connection will be deleted is it correct?
There was a problem hiding this comment.
I think it was added to address this comment #31645 (comment), but I agree we can remove it as peering connection gets deleted if a corresponding CFN stack is deleted.
aws-cdk-automation
removed
the
pr/needs-maintainer-review
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
@Mergifyio update |
❌ Mergify doesn't have permission to updateFor security reasons, Mergify can't update this pull request. Try updating locally. |
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 main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Contributes to closing RFC#507
Tracking: #30762
Reason for this change
This PR implements a new L2 construct to setup VPC Peering Connection.
Description of changes
validateVpcCidrOverlapto ensure IPv4 CIDR blocks don't overlap for subnets in the VPCsDescription of how you validated changes
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license