aws-samples / aws-secure-environment-accelerator

The AWS Secure Environment Accelerator is a tool designed to help deploy and operate secure multi-account, multi-region AWS environments on an ongoing basis. The power of the solution is the configuration file which enables the completely automated deployment of customizable architectures within AWS without changing a single line of code.
Apache License 2.0
725 stars 233 forks source link

[BUG] [Functional] Routes - transit gateway route association failure in phase 3 with multi region. #855

Closed rteja91 closed 11 months ago

rteja91 commented 2 years ago

I am trying to setup multi region asea with transit gateway peering model. I have used multi-region deployment with transit gateway peering. config i kept as similar to what is given in example. Installation stops at phase 3. stating as below.

Error: There is already a Construct with name 'tgw_associate_core' in AccountStack [SharedNetworkPhase3]

65 | at Node.addChild (/app/node_modules/.pnpm/constructs@3.3.71/node_modules/constructs/src/construct.ts:534:13) 66 | at new Node (/app/node_modules/.pnpm/constructs@3.3.71/node_modules/constructs/src/construct.ts:75:22) 67 | at new ConstructNode (/app/node_modules/.pnpm/@aws-cdk+core@1.113.0/node_modules/@aws-cdk/core/lib/construct-compat.ts:184:24) 68 | at Object.createNode (/app/node_modules/.pnpm/@aws-cdk+core@1.113.0/node_modules/@aws-cdk/core/lib/construct-compat.ts:55:11) 69 | at new Construct (/app/node_modules/.pnpm/constructs@3.3.71/node_modules/constructs/src/construct.ts:575:26) 70 | at new Construct (/app/node_modules/.pnpm/@aws-cdk+core@1.113.0/node_modules/@aws-cdk/core/lib/construct-compat.ts:52:5) 71 | at new CfnElement (/app/node_modules/.pnpm/@aws-cdk+core@1.113.0/node_modules/@aws-cdk/core/lib/cfn-element.ts:31:5) 72 | at new CfnRefElement (/app/node_modules/.pnpm/@aws-cdk+core@1.113.0/node_modules/@aws-cdk/core/lib/cfn-element.ts:106:1) 73 | at new CfnResource (/app/node_modules/.pnpm/@aws-cdk+core@1.113.0/node_modules/@aws-cdk/core/lib/cfn-resource.ts:68:5) 74 | at new CfnTransitGatewayRouteTableAssociation (/app/node_modules/.pnpm/@aws-cdk+aws-ec2@1.113.0/node_modules/@aws-cdk/aws-ec2/lib/ec2.generated.ts:15750:9)

I attaching the mandatory accounts json where we have transitgateway config in shared network. and phase3 build logs. files are sanitized already

madatoryaccounts.txt phase3-build.txt

is there anything that I can try to fix this issue.?

Brian969 commented 2 years ago

Hi, I see the same route table names used on each TGW, can you rename the route tables to be unique and retry? (i.e. core-Main, core-USW, core-APSE1 instead of all using core) May only be an issue for the connected RTs, but they all may need to be unique (i.e. "core", "segregated", "shared", and "standalone"). You'll need to also fixup all associated references. Let us know!

rteja91 commented 2 years ago

@Brian969,

Thanks for the suggestion and it works. Here is how i resolved this.

I went through the below code file couple of times, I was able to figure out that the code is failing when we are trying to associate the transit gateway remote with the same route table name.

while i was running this second round, i had to remove the route 0.0.0.0/0 pointing to Main Transit gateway in tgw-routes[] in peered transit gateways. this route is already added by default when we do peering attachment between transit gateways, no need of adding this route block in new transit gateway deployments, unless we have specific routes to main transit gateway apart from 0.0.0.0/0.

"deployments": { "tgw": [ { "name": "Main", "asn": 65521, "region": "${HOME_REGION}", "features": { "DNS-support": true, "VPN-ECMP-support": true, "Default-route-table-association": false, "Default-route-table-propagation": false, "Auto-accept-sharing-attachments": true }, "route-tables": ["core", "segregated", "shared", "standalone", "west", "southeast"], "tgw-routes": [ { "name": "{TGW_ALL}", "routes": [ { "destination": "0.0.0.0/0", "target-vpc": "Perimeter", "target-account": "perimeter" } ] }, { "name": "west", "routes": [ { "destination": "10.40.0.0/13", "target-tgw": "USWest" } ] }, { "name": "southeast", "routes": [ { "destination": "10.96.0.0/13", "target-tgw": "APSE1" } ] } ] }, { "name": "USWest", "asn": 64526, "region": "us-west-2", "features": { "DNS-support": true, "VPN-ECMP-support": true, "Default-route-table-association": false, "Default-route-table-propagation": false, "Auto-accept-sharing-attachments": true }, "route-tables": ["core", "segregated", "shared", "standalone"], "tgw-attach": { "associate-to-tgw": "Main", "account": "shared-network", "region": "${HOME_REGION}", "tgw-rt-associate-local": ["core"], "tgw-rt-associate-remote": ["west"] }, "tgw-routes": [] }, { "name": "APSE1", "asn": 64530, "region": "ap-southeast-1", "features": { "DNS-support": true, "VPN-ECMP-support": true, "Default-route-table-association": false, "Default-route-table-propagation": false, "Auto-accept-sharing-attachments": true }, "route-tables": ["core", "segregated", "shared", "standalone"], "tgw-attach": { "associate-to-tgw": "Main", "account": "shared-network", "region": "${HOME_REGION}", "tgw-rt-associate-local": ["core"], "tgw-rt-associate-remote": ["southeast"] }, "tgw-routes": [] } ] `

https://github.com/aws-samples/aws-secure-environment-accelerator/blob/release/v1.5.0/src/deployments/cdk/src/deployments/transit-gateway/step-3.ts

we might have to correct mult-region example to help this issue not being raised again.

Thanks for your guidance, feel free to close this ticket.

Brian969 commented 2 years ago

Glad you were able to get your environment running and very much appreciate the feedback/follow-up. I still consider this a bug - multiple TGW's should be able to associate to the same route table.

Issue:

Fix:

Specific related error:

fredbonin commented 2 years ago

Fixed here : https://github.com/aws-samples/aws-secure-environment-accelerator/pull/878

As mentioned in the PR, I did not use the remote TGW name since it might lead to collision : name would be tgw_associate_Main_core and tgw_associate_Main_core if 2 TGWs want to target Main TGW for the save core route.

With the TGW local name, it works : tgw_associate_TGW2_core and tgw_associate_TGW3_core

Brian969 commented 2 years ago

PR sent back for rework.

archikierstead commented 11 months ago

Did not implement