Fix errors in EC2 Transit Gateway VPC Attachment when TGW is shared from another account #7513

andrewsuperbrilliant · 2019-02-12T06:13:51Z

Fix EC2 Transit Gateway VPC Attachment "Transit gateway route table was deleted or no longer exists" when attaching to Shared Transit Gateway from another AWS Account

See: #6670

Managing transit gateway route tables does not seem to make sense for a shared TGW

In transit gateway attachment resource, add checks to see if the VPC has the same owner as the TGW. If not, skip the route table sections.

Output from acceptance testing:

 (shared transit gateway in a different account)
terraform apply
module.workspaces.aws_ec2_transit_gateway_vpc_attachment.tgw: Creating...
  dns_support:                                     "" => "enable"
  ipv6_support:                                    "" => "disable"
  subnet_ids.#:                                    "" => "3"
  subnet_ids.1648947649:                           "" => "subnet-016c0198d75a99fd3"
  subnet_ids.1894115500:                           "" => "subnet-08bfed79fac5b768d"
  subnet_ids.2219410493:                           "" => "subnet-0b26ed0912874649a"
  transit_gateway_default_route_table_association: "" => "true"
  transit_gateway_default_route_table_propagation: "" => "true"
  transit_gateway_id:                              "" => "tgw-0394354acd5bf4dd4"
  vpc_id:                                          "" => "vpc-0898c9d5a0ced2a64"
  vpc_owner_id:                                    "" => "<computed>"
module.workspaces.aws_ec2_transit_gateway_vpc_attachment.tgw: Still creating... (10s elapsed)
module.workspaces.aws_ec2_transit_gateway_vpc_attachment.tgw: Still creating... (20s elapsed)
module.workspaces.aws_ec2_transit_gateway_vpc_attachment.tgw: Still creating... (30s elapsed)
module.workspaces.aws_ec2_transit_gateway_vpc_attachment.tgw: Still creating... (40s elapsed)
module.workspaces.aws_ec2_transit_gateway_vpc_attachment.tgw: Still creating... (50s elapsed)
module.workspaces.aws_ec2_transit_gateway_vpc_attachment.tgw: Creation complete after 1m0s (ID: tgw-attach-0baaa44889180d3d2)
...

…as deleted or no longer exists" when attaching to Shared Transit Gateway from another AWS Account hashicorp#6670 Managing transit gateway route tables does not seem to make sense for a shared TGW In transit gateway attachement resource, add checks to see if the VPC has the same owner as the TGW. If not, skip the route table sections.

Puneeth-n · 2019-02-21T17:16:41Z

🙏 Please review this. It is blocking us 😢 @bflad

mcardinalupgrade · 2019-02-21T21:42:06Z

This is also impacting us, some movement on this would be amazing!

Puneeth-n · 2019-02-23T11:21:52Z

@andrewsuperbrilliant The acceptance tests seem to fail

AWS_REGION=us-east-1 TF_ACC=1 go test ./... -v -run=TestAccAWSEc2TransitGatewayVpcAttachment_basic -timeout 120m -parallel=1
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSEc2TransitGatewayVpcAttachment_basic
=== PAUSE TestAccAWSEc2TransitGatewayVpcAttachment_basic
=== CONT  TestAccAWSEc2TransitGatewayVpcAttachment_basic
--- FAIL: TestAccAWSEc2TransitGatewayVpcAttachment_basic (317.18s)
    testing.go:538: Step 1 error: 1 error occurred:
        	* aws_ec2_transit_gateway_vpc_attachment.test (import id: tgw-attach-0c611a54d50027c19): 1 error occurred:
        	* import aws_ec2_transit_gateway_vpc_attachment.test result: tgw-attach-0c611a54d50027c19: aws_ec2_transit_gateway_vpc_attachment.test: InvalidVpcID.NotFound: The vpc ID '' does not exist
        	status code: 400, request id: 3077eb15-a2d0-4024-8322-d0326542f8ad




FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	317.261s

Puneeth-n · 2019-02-26T14:19:27Z

@andrewsuperbrilliant any updates?

andrewsuperbrilliant · 2019-02-27T22:46:05Z

@Puneeth-n, that test should pass now.

88kami88 · 2019-03-01T17:16:45Z

👍

mfortedoitt · 2019-03-05T18:12:19Z

🥇

bflad

Hi @andrewsuperbrilliant 👋 Thanks so much for submitting this. I was able to provide some review items below by experimenting with our acceptance testing framework and multiple account testing.

Given the complexity and requirements associated with implementing the acceptance testing in this case, we would prefer just resource and resource documentation updates in this pull request and we will add the testing afterwards once we have finalized our desired testing implementation. Once this pull request is updated we will run the acceptance testing again before merge just to confirm everything.

One item not noted below that we would also like addressed is updating the resource documentation for the two arguments in website/docs/r/ec2_transit_gateway_vpc_attachment.html.markdown:

* `transit_gateway_default_route_table_association` - (Optional) Boolean whether the VPC Attachment should be associated with the EC2 Transit Gateway association default route table. This cannot be configured or perform drift detection with Resource Access Manager shared EC2 Transit Gateways. Default value: `true`.
* `transit_gateway_default_route_table_propagation` - (Optional) Boolean whether the VPC Attachment should propagate routes with the EC2 Transit Gateway propagation default route table. This cannot be configured or perform drift detection with Resource Access Manager shared EC2 Transit Gateways. Default value: `true`.

Output from the acceptance testing with below changes (including our experimental new test):

--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_SharedTransitGateway (233.35s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_TransitGatewayDefaultRouteTableAssociationAndPropagationDisabled (270.65s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_disappears (350.59s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_TransitGatewayDefaultRouteTablePropagation (381.75s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_Tags (387.63s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_Ipv6Support (388.37s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_basic (389.22s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_DnsSupport (449.51s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_TransitGatewayDefaultRouteTableAssociation (511.77s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_SubnetIds (514.14s)

Experimental test configuration just for reference, does not need to included:

resource "aws_ec2_transit_gateway" "test" {
  provider = "aws.alternate"
}

resource "aws_ram_resource_share" "test" {
  provider = "aws.alternate"

  name = %[1]q
}

resource "aws_ram_resource_association" "test" {
  provider = "aws.alternate"

  resource_arn       = "${aws_ec2_transit_gateway.test.arn}"
  resource_share_arn = "${aws_ram_resource_share.test.id}"
}

resource "aws_ram_principal_association" "test" {
  provider = "aws.alternate"

  principal          = "--OMITTED--"
  resource_share_arn = "${aws_ram_resource_share.test.id}"
}

resource "aws_vpc" "test" {
  cidr_block = "10.0.0.0/16"

  tags = {
    Name = "tf-acc-test-ec2-transit-gateway-vpc-attachment"
  }
}

resource "aws_subnet" "test" {
  cidr_block = "10.0.0.0/24"
  vpc_id     = "${aws_vpc.test.id}"

  tags = {
    Name = "tf-acc-test-ec2-transit-gateway-vpc-attachment"
  }
}

resource "aws_ec2_transit_gateway_vpc_attachment" "test" {
  depends_on = ["aws_ram_principal_association.test", "aws_ram_resource_association.test"]

  subnet_ids         = ["${aws_subnet.test.id}"]
  transit_gateway_id = "${aws_ec2_transit_gateway.test.id}"
  vpc_id             = "${aws_vpc.test.id}"
}

bflad · 2019-03-08T14:06:10Z

aws/resource_aws_ec2_transit_gateway_vpc_attachment.go

-	if err := ec2TransitGatewayRouteTableAssociationUpdate(conn, aws.StringValue(transitGateway.Options.AssociationDefaultRouteTableId), d.Id(), d.Get("transit_gateway_default_route_table_association").(bool)); err != nil {
-		return fmt.Errorf("error updating EC2 Transit Gateway Attachment (%s) Route Table (%s) association: %s", d.Id(), aws.StringValue(transitGateway.Options.AssociationDefaultRouteTableId), err)
-	}
+	if *transitGateway.OwnerId == vpcOwnerID {


We can remove the above DescribeVpcs API call by checking the VPC owner ID available from creating the VPC attachment here. We should also leave an inline comment for future code maintainers about why this check is here.

Suggested change

if *transitGateway.OwnerId == vpcOwnerID {

// We cannot modify Transit Gateway Route Tables for Resource Access Manager shared Transit Gateways

if aws.StringValue(transitGateway.OwnerId) == aws.StringValue(output.TransitGatewayVpcAttachment.VpcOwnerId) {

bflad · 2019-03-08T14:09:29Z

aws/resource_aws_ec2_transit_gateway_vpc_attachment.go

-	}
+	var transitGatewayDefaultRouteTableAssociation *ec2.TransitGatewayRouteTableAssociation
+	var transitGatewayDefaultRouteTablePropagation *ec2.TransitGatewayRouteTablePropagation
+	if *transitGateway.OwnerId == vpcOwnerID {


We can remove the above DescribeVpcs API call by using the Transit Gateway VPC Attachment information we already have available. We should also leave an inline comment here for future code maintainers about this conditional. Please note this whole comment details about the defaults should be included to go along with a change below.

// We cannot read Transit Gateway Route Tables for Resource Access Manager shared Transit Gateways // Default these to a non-nil value so we can match the existing schema of Default: true transitGatewayDefaultRouteTableAssociation := &ec2.TransitGatewayRouteTableAssociation{} transitGatewayDefaultRouteTablePropagation := &ec2.TransitGatewayRouteTablePropagation{} if aws.StringValue(transitGateway.OwnerId) == aws.StringValue(transitGatewayVpcAttachment.VpcOwnerId) {

bflad · 2019-03-08T14:14:14Z

aws/resource_aws_ec2_transit_gateway_vpc_attachment.go

@@ -192,8 +224,10 @@ func resourceAwsEc2TransitGatewayVpcAttachmentRead(d *schema.ResourceData, meta
 		return fmt.Errorf("error setting tags: %s", err)
 	}

-	d.Set("transit_gateway_default_route_table_association", (transitGatewayDefaultRouteTableAssociation != nil))
-	d.Set("transit_gateway_default_route_table_propagation", (transitGatewayDefaultRouteTablePropagation != nil))
+	if *transitGateway.OwnerId == vpcOwnerID {


This conditional should be removed to match with the above change. 👍 Without calling d.Set() for the below attributes in the RAM shared Transit Gateway scenario, operators will be presented a plan immediately after import similar to:

"transit_gateway_default_route_table_association": "" => "true" "transit_gateway_default_route_table_propagation": "" => "true"

bflad · 2019-03-08T14:21:59Z

aws/resource_aws_ec2_transit_gateway_vpc_attachment.go

-		if d.HasChange("transit_gateway_default_route_table_association") {
-			if err := ec2TransitGatewayRouteTableAssociationUpdate(conn, aws.StringValue(transitGateway.Options.AssociationDefaultRouteTableId), d.Id(), d.Get("transit_gateway_default_route_table_association").(bool)); err != nil {
-				return fmt.Errorf("error updating EC2 Transit Gateway Attachment (%s) Route Table (%s) association: %s", d.Id(), aws.StringValue(transitGateway.Options.AssociationDefaultRouteTableId), err)
+		if *transitGateway.OwnerId == vpcOwnerID {


Given that it would be invalid to attempt adjusting these configuration arguments since we cannot manage the EC2 Transit Gateway Default Route Table for a RAM shared Transit Gateway, we can either remove all these proposed resourceAwsEc2TransitGatewayVpcAttachmentUpdate changes and allow the API to return any errors (I personally think this is preferable in this situation, paired with updates to the resource documentation) or continue with these adjustments where Terraform will instead just return a perpetual plan difference, e.g. "true" => "false".

If we do keep these changes, we can remove the above DescribeVpcs API call above by using information present in the Terraform state here. 👍 We should also provide an inline comment for future code maintainers about this conditional.

Suggested change

if *transitGateway.OwnerId == vpcOwnerID {

// We cannot modify Transit Gateway Route Tables for Resource Access Manager shared Transit Gateways

if aws.StringValue(transitGateway.OwnerId) == d.Get("vpc_owner_id").(string) {

Puneeth-n · 2019-03-11T20:11:01Z

@andrewsuperbrilliant any updates on this? would be nice if we could get it merged.

zenvdeluca · 2019-03-13T20:15:00Z

also being blocked by this issue

bflad · 2019-03-14T21:51:57Z

Hi again @andrewsuperbrilliant 👋 since we haven't heard back from you, we have addressed the feedback in a followup commit (920b498) so we can get this released today. Thanks for your contribution.

--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_basic (333.79s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_disappears (306.32s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_DnsSupport (334.40s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_Ipv6Support (335.74s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_SharedTransitGateway (250.72s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_SubnetIds (485.69s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_Tags (361.38s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_TransitGatewayDefaultRouteTableAssociation (481.03s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_TransitGatewayDefaultRouteTableAssociationAndPropagationDisabled (246.08s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_TransitGatewayDefaultRouteTablePropagation (364.23s)

…dback Output from acceptance testing: ``` --- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_basic (333.79s) --- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_disappears (306.32s) --- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_DnsSupport (334.40s) --- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_Ipv6Support (335.74s) --- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_SharedTransitGateway (250.72s) --- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_SubnetIds (485.69s) --- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_Tags (361.38s) --- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_TransitGatewayDefaultRouteTableAssociation (481.03s) --- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_TransitGatewayDefaultRouteTableAssociationAndPropagationDisabled (246.08s) --- PASS: TestAccAWSEc2TransitGatewayVpcAttachment_TransitGatewayDefaultRouteTablePropagation (364.23s) ```

andrewsuperbrilliant · 2019-03-14T22:27:04Z

@bflad being more familiar with the aws-provider code base (this was my first peak into terraform source code), your comments all look great to me. I've been traveling this week and haven't had a chance to look at the comments that were provided. Just glad my initial work around code was a starting point for a better fix. Happy to see a fix moving to master! Thanks

bflad · 2019-03-15T05:39:27Z

This has been released in version 2.2.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

BrunoCarrier · 2019-07-02T14:45:57Z

Terraform will perform the following actions:

  # aws_ec2_transit_gateway_vpc_attachment.backup_dmc_prod will be updated in-place
  ~ resource "aws_ec2_transit_gateway_vpc_attachment" "backup_dmc_prod" {
        dns_support                                     = "enable"
        id                                              = "tgw-attach-xxxx"
        ipv6_support                                    = "disable"
        subnet_ids                                      = [
            "subnet-43b36e0b",
            "subnet-918c30f7",
        ]
        tags                                            = {
            "Name"    = "backup-dmc-prod 10.xxxx.0.0/16"
            "product" = "cd"
            "service" = "network"
        }
      ~ transit_gateway_default_route_table_association = true -> false
      ~ transit_gateway_default_route_table_propagation = true -> false
        transit_gateway_id                              = "tgw-xxxx"
        vpc_id                                          = "vpc-xxxx"
        vpc_owner_id                                    = "123123123123"
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

aws_ec2_transit_gateway_vpc_attachment.backup_dmc_prod: Modifying... [id=tgw-attach-xxxxxx]

Error: error updating EC2 Transit Gateway Attachment (tgw-attach-xxxxxx) Route Table (tgw-rtb-xxxxxx) association: error determining EC2 Transit Gateway Attachment Route Table (tgw-rtb-xxxxxx) association (tgw-attach-xxxxxxx): InvalidRouteTableID.NotFound: Transit Gateway Route Table tgw-rtb-xxxxxxxxxx was deleted or does not exist.
	status code: 400, request id: c34b75cc-c45b-4a05-9261-6fa0a2666f72

  on vpc-backup-dmc-prod-10.98.tf line 16, in resource "aws_ec2_transit_gateway_vpc_attachment" "backup_dmc_prod":
  16: resource "aws_ec2_transit_gateway_vpc_attachment" "backup_dmc_prod" {

provider "aws" {
  region = "us-west-2"
  assume_role {
    role_arn = "arn:aws:iam::123123123123:role/terraform-network-only"
  }
  version = "2.17.0"
}

Since version 2.17.0 is higher than 2.2, I expected it to be fixed in version 2.17.0. Could you advise what could be going wrong here?

The VPC lives in a different account than the TGW because of RAM

bflad · 2019-07-02T18:23:53Z

@BrunoCarrier 👋 Please note this in the aws_ec2_transit_gateway_vpc_attachment resource documentation for those two arguments:

transit_gateway_default_route_table_association - (Optional) Boolean whether the VPC Attachment should be associated with the EC2 Transit Gateway association default route table. This cannot be configured or perform drift detection with Resource Access Manager shared EC2 Transit Gateways. Default value: true.
transit_gateway_default_route_table_propagation - (Optional) Boolean whether the VPC Attachment should propagate routes with the EC2 Transit Gateway propagation default route table. This cannot be configured or perform drift detection with Resource Access Manager shared EC2 Transit Gateways. Default value: true.

Since you mention using RAM, you'll want to ensure they are either omitted from the configuration or set to match the default. Hope this helps.

BrunoCarrier · 2019-07-03T13:29:06Z

The issue is actually with the aws_ec2_transit_gateway_vpc_attachment_accepter resource, not aws_ec2_transit_gateway_vpc_attachment @bflad
It seems that whoever reported the initial issue was not using terraform to accept VPC attachments, and thus this bug went undetected/unhandled

ghost · 2019-11-03T14:57:56Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

ghost added size/M Managed by automation to categorize the size of a PR. service/ec2 Issues and PRs that pertain to the ec2 service. labels Feb 12, 2019

Fix failing unit test

fb3a1b3

bflad added the bug Addresses a defect in current functionality. label Mar 7, 2019

bflad added this to the v2.3.0 milestone Mar 8, 2019

bflad mentioned this pull request Mar 8, 2019

EC2 Transit Gateway VPC Attachment "Transit gateway route table was deleted or no longer exists" when attaching to Shared Transit Gateway from another AWS Account #6670

Closed

bflad reviewed Mar 8, 2019

View reviewed changes

bflad modified the milestones: v2.3.0, v2.2.0 Mar 14, 2019

bflad merged commit fb3a1b3 into hashicorp:master Mar 14, 2019

bflad added a commit that referenced this pull request Mar 14, 2019

Update CHANGELOG for #7513

33bb245

ghost locked and limited conversation to collaborators Nov 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix errors in EC2 Transit Gateway VPC Attachment when TGW is shared from another account #7513

Fix errors in EC2 Transit Gateway VPC Attachment when TGW is shared from another account #7513

andrewsuperbrilliant commented Feb 12, 2019

Puneeth-n commented Feb 21, 2019 •

edited

Loading

mcardinalupgrade commented Feb 21, 2019 •

edited

Loading

Puneeth-n commented Feb 23, 2019

Puneeth-n commented Feb 26, 2019

andrewsuperbrilliant commented Feb 27, 2019

88kami88 commented Mar 1, 2019

mfortedoitt commented Mar 5, 2019

bflad left a comment

bflad Mar 8, 2019

bflad Mar 8, 2019

bflad Mar 8, 2019

bflad Mar 8, 2019

Puneeth-n commented Mar 11, 2019

zenvdeluca commented Mar 13, 2019

bflad commented Mar 14, 2019

andrewsuperbrilliant commented Mar 14, 2019

bflad commented Mar 15, 2019

BrunoCarrier commented Jul 2, 2019

bflad commented Jul 2, 2019

BrunoCarrier commented Jul 3, 2019

ghost commented Nov 3, 2019

	if *transitGateway.OwnerId == vpcOwnerID {
	// We cannot modify Transit Gateway Route Tables for Resource Access Manager shared Transit Gateways
	if aws.StringValue(transitGateway.OwnerId) == aws.StringValue(output.TransitGatewayVpcAttachment.VpcOwnerId) {

Fix errors in EC2 Transit Gateway VPC Attachment when TGW is shared from another account #7513

Fix errors in EC2 Transit Gateway VPC Attachment when TGW is shared from another account #7513

Conversation

andrewsuperbrilliant commented Feb 12, 2019

Puneeth-n commented Feb 21, 2019 • edited Loading

mcardinalupgrade commented Feb 21, 2019 • edited Loading

Puneeth-n commented Feb 23, 2019

Puneeth-n commented Feb 26, 2019

andrewsuperbrilliant commented Feb 27, 2019

88kami88 commented Mar 1, 2019

mfortedoitt commented Mar 5, 2019

bflad left a comment

Choose a reason for hiding this comment

bflad Mar 8, 2019

Choose a reason for hiding this comment

bflad Mar 8, 2019

Choose a reason for hiding this comment

bflad Mar 8, 2019

Choose a reason for hiding this comment

bflad Mar 8, 2019

Choose a reason for hiding this comment

Puneeth-n commented Mar 11, 2019

zenvdeluca commented Mar 13, 2019

bflad commented Mar 14, 2019

andrewsuperbrilliant commented Mar 14, 2019

bflad commented Mar 15, 2019

BrunoCarrier commented Jul 2, 2019

bflad commented Jul 2, 2019

BrunoCarrier commented Jul 3, 2019

ghost commented Nov 3, 2019

Puneeth-n commented Feb 21, 2019 •

edited

Loading

mcardinalupgrade commented Feb 21, 2019 •

edited

Loading