Skip to content
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

Merged
merged 3 commits into from
Sep 1, 2020
Merged

Add local gateway routes and route table entries #14864

merged 3 commits into from
Sep 1, 2020

Conversation

johnbarney
Copy link
Contributor

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #13888

Release note for CHANGELOG:

* Add Local Gateway Routes
* Add support for Local Gateway routes in Route Tables

Output from acceptance testing:

$ make testacc TESTARGS='-run=LocalGatewayID$'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=LocalGatewayID -timeout 120m
=== RUN   TestAccAWSRouteDataSource_LocalGatewayID
=== PAUSE TestAccAWSRouteDataSource_LocalGatewayID
=== RUN   TestAccAWSRouteTable_Route_LocalGatewayID
=== PAUSE TestAccAWSRouteTable_Route_LocalGatewayID
=== RUN   TestAccAWSRoute_LocalGatewayID
=== PAUSE TestAccAWSRoute_LocalGatewayID
=== CONT  TestAccAWSRouteDataSource_LocalGatewayID
=== CONT  TestAccAWSRouteTable_Route_LocalGatewayID
=== CONT  TestAccAWSRoute_LocalGatewayID
--- PASS: TestAccAWSRouteDataSource_LocalGatewayID (70.98s)
--- PASS: TestAccAWSRouteTable_Route_LocalGatewayID (74.51s)
--- PASS: TestAccAWSRoute_LocalGatewayID (78.50s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       80.880s

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.

@johnbarney johnbarney requested a review from a team August 27, 2020 06:03
@ghost ghost added size/L Managed by automation to categorize the size of a PR. service/ec2 Issues and PRs that pertain to the ec2 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. documentation Introduces or discusses updates to documentation. needs-triage Waiting for first response or review from a maintainer. labels Aug 27, 2020
@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Aug 28, 2020
Copy link
Contributor

@bflad bflad left a 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.

aws/data_source_aws_route_test.go Outdated Show resolved Hide resolved
aws/resource_aws_route_test.go Outdated Show resolved Hide resolved
aws/data_source_aws_route_test.go Outdated Show resolved Hide resolved
aws/resource_aws_route_table_test.go Outdated Show resolved Hide resolved
aws/resource_aws_route_test.go Outdated Show resolved Hide resolved
@johnbarney
Copy link
Contributor Author

Suggestions implemented and tests verified. Ran go test directly due to the Outpost account I'm on hitting the VPC limit during the tests.

$ TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=LocalGatewayID -timeout 120m
=== RUN   TestAccAWSRouteDataSource_LocalGatewayID
=== PAUSE TestAccAWSRouteDataSource_LocalGatewayID
=== RUN   TestAccAWSRouteTable_Route_LocalGatewayID
=== PAUSE TestAccAWSRouteTable_Route_LocalGatewayID
=== RUN   TestAccAWSRoute_LocalGatewayID
=== PAUSE TestAccAWSRoute_LocalGatewayID
=== CONT  TestAccAWSRouteDataSource_LocalGatewayID
=== CONT  TestAccAWSRoute_LocalGatewayID
=== CONT  TestAccAWSRouteTable_Route_LocalGatewayID
--- PASS: TestAccAWSRouteDataSource_LocalGatewayID (71.98s)
--- PASS: TestAccAWSRoute_LocalGatewayID (81.73s)
--- PASS: TestAccAWSRouteTable_Route_LocalGatewayID (76.21s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       149.386s

Working on getting a colleague to run verification tests.

@FireballDWF
Copy link

Reproduced the test results

$ TF_ACC=1 go test ./aws -v -count 1 -parallel 1 -run=LocalGatewayID -timeout 120m
=== RUN   TestAccAWSRouteDataSource_LocalGatewayID
=== PAUSE TestAccAWSRouteDataSource_LocalGatewayID
=== RUN   TestAccAWSRouteTable_Route_LocalGatewayID
=== PAUSE TestAccAWSRouteTable_Route_LocalGatewayID
=== RUN   TestAccAWSRoute_LocalGatewayID
=== PAUSE TestAccAWSRoute_LocalGatewayID
=== CONT  TestAccAWSRouteDataSource_LocalGatewayID
--- PASS: TestAccAWSRouteDataSource_LocalGatewayID (28.58s)
=== CONT  TestAccAWSRoute_LocalGatewayID
--- PASS: TestAccAWSRoute_LocalGatewayID (32.56s)
=== CONT  TestAccAWSRouteTable_Route_LocalGatewayID
--- PASS: TestAccAWSRouteTable_Route_LocalGatewayID (31.07s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	94.929s

@FireballDWF
Copy link

@bflad I have full time RW access to an Outpost now, so can run tests for you basically on demand.

@bflad
Copy link
Contributor

bflad commented Sep 1, 2020

Awesome! Running the other resource tests to double check for regressions then we can likely merge this. 🚀

@bflad bflad added this to the v3.5.0 milestone Sep 1, 2020
@bflad bflad self-assigned this Sep 1, 2020
Copy link
Contributor

@bflad bflad left a 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)

@bflad bflad merged commit 1ce5d42 into hashicorp:master Sep 1, 2020
bflad added a commit that referenced this pull request Sep 1, 2020
@johnbarney johnbarney deleted the LocalGatewayRoutes branch September 2, 2020 01:06
bflad added a commit that referenced this pull request Sep 2, 2020
…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)
```
bflad added a commit that referenced this pull request Sep 2, 2020
…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)
```
@ghost
Copy link

ghost commented Sep 3, 2020

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!

@ghost
Copy link

ghost commented Oct 2, 2020

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 ghost locked as resolved and limited conversation to collaborators Oct 2, 2020
@justinretzolk justinretzolk added the partner Contribution from a partner. label May 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. partner Contribution from a partner. service/ec2 Issues and PRs that pertain to the ec2 service. size/L Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

resource/aws_route: Add LocalGatewayId attribute support
4 participants