-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
Add local gateway routes and route table entries #14864
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks well implemented, @johnbarney 👍 Just a few suggestions below. We will need to sort out getting access again to a test Outpost to double check things here or alternatively if other contributors can run the acceptance testing and show it passing, we can approve it that way as well.
Co-authored-by: Brian Flad <[email protected]>
Suggestions implemented and tests verified. Ran go test directly due to the Outpost account I'm on hitting the VPC limit during the tests.
Working on getting a colleague to run verification tests. |
Reproduced the test results
|
@bflad I have full time RW access to an Outpost now, so can run tests for you basically on demand. |
Awesome! Running the other resource tests to double check for regressions then we can likely merge this. 🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks @johnbarney and @FireballDWF 🚀
Output from acceptance testing (non-Outposts):
--- PASS: TestAccAWSRoute_basic (65.55s)
--- PASS: TestAccAWSRoute_changeCidr (57.34s)
--- PASS: TestAccAWSRoute_changeRouteTable (61.43s)
--- PASS: TestAccAWSRoute_ConditionalCidrBlock (49.63s)
--- PASS: TestAccAWSRoute_disappears (58.94s)
--- PASS: TestAccAWSRoute_doesNotCrashWithVPCEndpoint (44.20s)
--- PASS: TestAccAWSRoute_ipv6Support (72.32s)
--- PASS: TestAccAWSRoute_ipv6ToInstance (105.34s)
--- PASS: TestAccAWSRoute_ipv6ToInternetGateway (58.14s)
--- PASS: TestAccAWSRoute_ipv6ToNetworkInterface (133.57s)
--- PASS: TestAccAWSRoute_ipv6ToPeeringConnection (34.70s)
--- PASS: TestAccAWSRoute_noopdiff (85.23s)
--- PASS: TestAccAWSRoute_TransitGatewayID_DestinationCidrBlock (300.27s)
--- SKIP: TestAccAWSRoute_LocalGatewayID (1.88s)
--- PASS: TestAccAWSRouteDataSource_basic (109.88s)
--- PASS: TestAccAWSRouteDataSource_TransitGatewayID (270.00s)
--- SKIP: TestAccAWSRouteDataSource_LocalGatewayID (1.93s)
--- FAIL: TestAccAWSRouteTable_panicEmptyRoute (17.66s) # unrelated failure
--- PASS: TestAccAWSRouteTable_basic (109.01s)
--- PASS: TestAccAWSRouteTable_ConditionalCidrBlock (92.16s)
--- PASS: TestAccAWSRouteTable_instance (123.30s)
--- PASS: TestAccAWSRouteTable_ipv6 (49.89s)
--- PASS: TestAccAWSRouteTable_Route_ConfigMode (131.80s)
--- PASS: TestAccAWSRouteTable_Route_TransitGatewayID (270.19s)
--- PASS: TestAccAWSRouteTable_tags (127.25s)
--- PASS: TestAccAWSRouteTable_vgwRoutePropagation (73.73s)
--- PASS: TestAccAWSRouteTable_vpcPeering (63.66s)
--- SKIP: TestAccAWSRouteTable_Route_LocalGatewayID (1.85s)
--- PASS: TestAccDataSourceAwsRouteTable_basic (24.38s)
--- PASS: TestAccDataSourceAwsRouteTable_main (51.23s)
…e_table attribute changes Reference: #14864 Decoupling of the two resources can occur separately, but opting for this small fix as we cannot verify whether the `aws_default_route_table` resource can support `local_gateway_id` or not ourselves before tomorrow's release. Previously: ``` === CONT TestAccAWSDefaultRouteTable_Route ------- Stderr: ------- panic: interface conversion: interface {} is nil, not string goroutine 492 [running]: github.com/terraform-providers/terraform-provider-aws/aws.resourceAwsRouteTableUpdate(0xc00179f200, 0x5bf4d20, 0xc0017ccf00, 0x5a5f0c0, 0xc00144ab40) /opt/teamcity-agent/work/2e10e023da0c7520/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_route_table.go:397 +0x2cbd github.com/terraform-providers/terraform-provider-aws/aws.resourceAwsDefaultRouteTableCreate(0xc00179f200, 0x5bf4d20, 0xc0017ccf00, 0x3, 0xffffffffffffffff) /opt/teamcity-agent/work/2e10e023da0c7520/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_default_route_table.go:149 +0x5b2 github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).create(0xc0008c7760, 0x7763460, 0xc001ec8580, 0xc00179f200, 0x5bf4d20, 0xc0017ccf00, 0x0, 0x0, 0x0) /opt/teamcity-agent/work/2e10e023da0c7520/src/github.com/terraform-providers/terraform-provider-aws/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource.go:269 +0x88 ``` Output from acceptance testing: ``` --- PASS: TestAccAWSDefaultRouteTable_disappears_Vpc (20.85s) --- PASS: TestAccAWSDefaultRouteTable_basic (39.08s) --- PASS: TestAccAWSDefaultRouteTable_vpc_endpoint (54.68s) --- PASS: TestAccAWSDefaultRouteTable_ConditionalCidrBlock (69.75s) --- PASS: TestAccAWSDefaultRouteTable_tags (70.91s) --- PASS: TestAccAWSDefaultRouteTable_swap (83.45s) --- PASS: TestAccAWSDefaultRouteTable_Route (107.71s) --- PASS: TestAccAWSDefaultRouteTable_Route_TransitGatewayID (331.13s) --- FAIL: TestAccAWSRouteTable_panicEmptyRoute (17.11s) # See also: #14383 --- PASS: TestAccAWSRouteTable_ipv6 (38.51s) --- PASS: TestAccAWSRouteTable_vpcPeering (44.24s) --- PASS: TestAccAWSRouteTable_vgwRoutePropagation (45.36s) --- PASS: TestAccAWSRouteTable_ConditionalCidrBlock (70.65s) --- PASS: TestAccAWSRouteTable_basic (80.26s) --- PASS: TestAccAWSRouteTable_tags (90.73s) --- PASS: TestAccAWSRouteTable_Route_ConfigMode (92.20s) --- PASS: TestAccAWSRouteTable_instance (116.15s) --- PASS: TestAccAWSRouteTable_Route_TransitGatewayID (321.82s) ```
…e_table attribute changes (#14988) Reference: #14864 Decoupling of the two resources can occur separately, but opting for this small fix as we cannot verify whether the `aws_default_route_table` resource can support `local_gateway_id` or not ourselves before tomorrow's release. Previously: ``` === CONT TestAccAWSDefaultRouteTable_Route ------- Stderr: ------- panic: interface conversion: interface {} is nil, not string goroutine 492 [running]: github.com/terraform-providers/terraform-provider-aws/aws.resourceAwsRouteTableUpdate(0xc00179f200, 0x5bf4d20, 0xc0017ccf00, 0x5a5f0c0, 0xc00144ab40) /opt/teamcity-agent/work/2e10e023da0c7520/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_route_table.go:397 +0x2cbd github.com/terraform-providers/terraform-provider-aws/aws.resourceAwsDefaultRouteTableCreate(0xc00179f200, 0x5bf4d20, 0xc0017ccf00, 0x3, 0xffffffffffffffff) /opt/teamcity-agent/work/2e10e023da0c7520/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_default_route_table.go:149 +0x5b2 github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).create(0xc0008c7760, 0x7763460, 0xc001ec8580, 0xc00179f200, 0x5bf4d20, 0xc0017ccf00, 0x0, 0x0, 0x0) /opt/teamcity-agent/work/2e10e023da0c7520/src/github.com/terraform-providers/terraform-provider-aws/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource.go:269 +0x88 ``` Output from acceptance testing: ``` --- PASS: TestAccAWSDefaultRouteTable_disappears_Vpc (20.85s) --- PASS: TestAccAWSDefaultRouteTable_basic (39.08s) --- PASS: TestAccAWSDefaultRouteTable_vpc_endpoint (54.68s) --- PASS: TestAccAWSDefaultRouteTable_ConditionalCidrBlock (69.75s) --- PASS: TestAccAWSDefaultRouteTable_tags (70.91s) --- PASS: TestAccAWSDefaultRouteTable_swap (83.45s) --- PASS: TestAccAWSDefaultRouteTable_Route (107.71s) --- PASS: TestAccAWSDefaultRouteTable_Route_TransitGatewayID (331.13s) --- FAIL: TestAccAWSRouteTable_panicEmptyRoute (17.11s) # See also: #14383 --- PASS: TestAccAWSRouteTable_ipv6 (38.51s) --- PASS: TestAccAWSRouteTable_vpcPeering (44.24s) --- PASS: TestAccAWSRouteTable_vgwRoutePropagation (45.36s) --- PASS: TestAccAWSRouteTable_ConditionalCidrBlock (70.65s) --- PASS: TestAccAWSRouteTable_basic (80.26s) --- PASS: TestAccAWSRouteTable_tags (90.73s) --- PASS: TestAccAWSRouteTable_Route_ConfigMode (92.20s) --- PASS: TestAccAWSRouteTable_instance (116.15s) --- PASS: TestAccAWSRouteTable_Route_TransitGatewayID (321.82s) ```
This has been released in version 3.5.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
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! |
Community Note
Relates OR Closes #13888
Release note for CHANGELOG:
Output from acceptance testing:
Additional note: Default route table resource and datasource is not modified. This is because AWS does not allow Local Gateways to be associated with the default vpc route table.