-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
(@aws-cdk/aws-ec2-alpha): vpc.addInternetGateway cannot handle multiple subnets with shared routetable #33672
Comments
Thank you for the report. The issue is occurring when a VPC has multiple public subnets that share the same route table. When
Root CauseThe current implementation in vpc-v2-base.ts doesn't check if a route for a specific route table has already been created. It simply iterates over all subnets and adds routes to each subnet's route table, which causes duplication when multiple subnets share the same route table. Possible SolutionOff the top of my head, one of the possible solutions is:
This solution addresses the issue by identifying unique route tables among the subnets and only adding routes to each route table once, preventing the duplication that was causing the CloudFormation error. Making this a p1 and we welcome PRs as well. |
Comments on closed issues and PRs are hard for our team to see. |
1 similar comment
Comments on closed issues and PRs are hard for our team to see. |
…ws#33824) ### Issue # (if applicable) Closes aws#33672 ### Reason for this change Earlier same routes were being added to shared route table between subnets causing deployment failures. ### Description of changes - Added helper method to unique we don't process single route table twice. - Also added return value for IGW and EIGW so that customers can refer them to as target later, similar to other gateway and endpoints. ### Describe any new or updated permissions being added NA ### Description of how you validated changes Added unit test and integration test Limitation for adding integration test for IPv6, we don't know the `AmazonprovidedIPv6` before for which local route gets added and needs to be added in expected assertion. ``` Error: Resolution error: Resolution error: Resolution error: Found an encoded list token string in a scalar string context. Use 'Fn.select(0, list)' (not 'list[0]') to extract elements from token lists. { DestinationCidrBlock: vpc.ipv6CidrBlocks[0], GatewayId: 'local', }, ``` ### Checklist - [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*
Describe the bug
If a VPC has 2 public subnets that share a single routing table, calling
vpc.addInternetGateway
with no options will attempt to add a route to the IGW to the route table for each subnet. This causes the cloudformation update to fail with a message like:It is possible to work around this by using seperate route tables for each subnet and whilst it's generally good practice to use one route table per subnet, shared route tables are legal.
It'a also possible to manually supplying single subnet to
vpc.addInternetGateway
, but it looks confusing semantically.Regression Issue
Last Known Working CDK Version
No response
Expected Behavior
That the vpc.addInternetGateway should not try to add identical routes to the same routetable.
Current Behavior
That the vpc.addInternetGateway tries to add identical routes to the same routetable.
Reproduction Steps
Possible Solution
As
vpc.addInternetGateway
internally iterates over subnets, and the method it uses internally isvpc.addDefaultInternetRoute
, I'm not sure where the cleanest place to fix this is.Maybe
addInternetGateway
should iterate the subnets to produce a unique list of route tables, andaddDefaultInternetRoute
should operate on route tables not subnets, as semantically the route is added to the table, not the subnet. You would need some way of tellingaddDefaultInternetRoute
if it needed to added optional IPv6 routes as well.Here is very niave approach:
Additional Information/Context
No response
CDK CLI Version
2.1000.3
Framework Version
No response
Node.js Version
v20.13.1
OS
OSX 15.3.1
Language
TypeScript
Language Version
TypeScript 5.6.3
Other information
No response
The text was updated successfully, but these errors were encountered: