-
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
r/aws_(default)_route_table: 'destination_prefix_list_id' attribute can be specified for managed prefix list destinations #17319
Conversation
This comment has been minimized.
This comment has been minimized.
169f6b6
to
5d8de4b
Compare
c2feb2f
to
22b1a10
Compare
This reverts commit a32cff5e705ae5c7f841c1743d8f7f84500d0294.
Acceptance test output: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSDefaultRouteTable_' ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=TestAccAWSDefaultRouteTable_ -timeout 120m === RUN TestAccAWSDefaultRouteTable_basic === PAUSE TestAccAWSDefaultRouteTable_basic === RUN TestAccAWSDefaultRouteTable_disappears_Vpc === PAUSE TestAccAWSDefaultRouteTable_disappears_Vpc === RUN TestAccAWSDefaultRouteTable_Route_ConfigMode === PAUSE TestAccAWSDefaultRouteTable_Route_ConfigMode === RUN TestAccAWSDefaultRouteTable_swap === PAUSE TestAccAWSDefaultRouteTable_swap === RUN TestAccAWSDefaultRouteTable_IPv4_To_TransitGateway === PAUSE TestAccAWSDefaultRouteTable_IPv4_To_TransitGateway === RUN TestAccAWSDefaultRouteTable_IPv4_To_VpcEndpoint === PAUSE TestAccAWSDefaultRouteTable_IPv4_To_VpcEndpoint === RUN TestAccAWSDefaultRouteTable_VpcEndpointAssociation === PAUSE TestAccAWSDefaultRouteTable_VpcEndpointAssociation === RUN TestAccAWSDefaultRouteTable_tags === PAUSE TestAccAWSDefaultRouteTable_tags === RUN TestAccAWSDefaultRouteTable_ConditionalCidrBlock === PAUSE TestAccAWSDefaultRouteTable_ConditionalCidrBlock === CONT TestAccAWSDefaultRouteTable_basic === CONT TestAccAWSDefaultRouteTable_IPv4_To_VpcEndpoint --- PASS: TestAccAWSDefaultRouteTable_basic (29.97s) === CONT TestAccAWSDefaultRouteTable_ConditionalCidrBlock --- PASS: TestAccAWSDefaultRouteTable_ConditionalCidrBlock (54.15s) === CONT TestAccAWSDefaultRouteTable_tags --- PASS: TestAccAWSDefaultRouteTable_tags (52.24s) === CONT TestAccAWSDefaultRouteTable_VpcEndpointAssociation --- PASS: TestAccAWSDefaultRouteTable_VpcEndpointAssociation (44.51s) === CONT TestAccAWSDefaultRouteTable_swap --- PASS: TestAccAWSDefaultRouteTable_swap (91.07s) === CONT TestAccAWSDefaultRouteTable_IPv4_To_TransitGateway === CONT TestAccAWSDefaultRouteTable_Route_ConfigMode --- PASS: TestAccAWSDefaultRouteTable_IPv4_To_VpcEndpoint (323.68s) --- PASS: TestAccAWSDefaultRouteTable_Route_ConfigMode (84.01s) === CONT TestAccAWSDefaultRouteTable_disappears_Vpc --- PASS: TestAccAWSDefaultRouteTable_disappears_Vpc (14.13s) --- PASS: TestAccAWSDefaultRouteTable_IPv4_To_TransitGateway (400.11s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 672.134s
…ified for managed prefix list destinations. Acceptance test output: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSRouteTable_PrefixList_To_InternetGateway' ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=TestAccAWSRouteTable_PrefixList_To_InternetGateway -timeout 120m === RUN TestAccAWSRouteTable_PrefixList_To_InternetGateway === PAUSE TestAccAWSRouteTable_PrefixList_To_InternetGateway === CONT TestAccAWSRouteTable_PrefixList_To_InternetGateway --- PASS: TestAccAWSRouteTable_PrefixList_To_InternetGateway (41.36s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 41.448s
… be specified for managed prefix list destinations. Acceptance test output: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway -timeout 120m === RUN TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway === PAUSE TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway === CONT TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway --- PASS: TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway (54.14s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 54.216s
Acceptance test output: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSRouteTable_\|TestAccAWSDefaultRouteTable_' ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=TestAccAWSRouteTable_\|TestAccAWSDefaultRouteTable_ -timeout 180m === RUN TestAccAWSDefaultRouteTable_basic === PAUSE TestAccAWSDefaultRouteTable_basic === RUN TestAccAWSDefaultRouteTable_disappears_Vpc === PAUSE TestAccAWSDefaultRouteTable_disappears_Vpc === RUN TestAccAWSDefaultRouteTable_Route_ConfigMode === PAUSE TestAccAWSDefaultRouteTable_Route_ConfigMode === RUN TestAccAWSDefaultRouteTable_swap === PAUSE TestAccAWSDefaultRouteTable_swap === RUN TestAccAWSDefaultRouteTable_IPv4_To_TransitGateway === PAUSE TestAccAWSDefaultRouteTable_IPv4_To_TransitGateway === RUN TestAccAWSDefaultRouteTable_IPv4_To_VpcEndpoint === PAUSE TestAccAWSDefaultRouteTable_IPv4_To_VpcEndpoint === RUN TestAccAWSDefaultRouteTable_VpcEndpointAssociation === PAUSE TestAccAWSDefaultRouteTable_VpcEndpointAssociation === RUN TestAccAWSDefaultRouteTable_tags === PAUSE TestAccAWSDefaultRouteTable_tags === RUN TestAccAWSDefaultRouteTable_ConditionalCidrBlock === PAUSE TestAccAWSDefaultRouteTable_ConditionalCidrBlock === RUN TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway === PAUSE TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway === RUN TestAccAWSRouteTable_basic === PAUSE TestAccAWSRouteTable_basic === RUN TestAccAWSRouteTable_disappears === PAUSE TestAccAWSRouteTable_disappears === RUN TestAccAWSRouteTable_disappears_SubnetAssociation === PAUSE TestAccAWSRouteTable_disappears_SubnetAssociation === RUN TestAccAWSRouteTable_IPv4_To_InternetGateway === PAUSE TestAccAWSRouteTable_IPv4_To_InternetGateway === RUN TestAccAWSRouteTable_IPv4_To_Instance === PAUSE TestAccAWSRouteTable_IPv4_To_Instance === RUN TestAccAWSRouteTable_IPv6_To_EgressOnlyInternetGateway === PAUSE TestAccAWSRouteTable_IPv6_To_EgressOnlyInternetGateway === RUN TestAccAWSRouteTable_tags === PAUSE TestAccAWSRouteTable_tags === RUN TestAccAWSRouteTable_RequireRouteDestination === PAUSE TestAccAWSRouteTable_RequireRouteDestination === RUN TestAccAWSRouteTable_RequireRouteTarget === PAUSE TestAccAWSRouteTable_RequireRouteTarget === RUN TestAccAWSRouteTable_Route_ConfigMode === PAUSE TestAccAWSRouteTable_Route_ConfigMode === RUN TestAccAWSRouteTable_IPv4_To_TransitGateway === PAUSE TestAccAWSRouteTable_IPv4_To_TransitGateway === RUN TestAccAWSRouteTable_IPv4_To_VpcEndpoint === PAUSE TestAccAWSRouteTable_IPv4_To_VpcEndpoint === RUN TestAccAWSRouteTable_IPv4_To_CarrierGateway === PAUSE TestAccAWSRouteTable_IPv4_To_CarrierGateway === RUN TestAccAWSRouteTable_IPv4_To_LocalGateway === PAUSE TestAccAWSRouteTable_IPv4_To_LocalGateway === RUN TestAccAWSRouteTable_IPv4_To_VpcPeeringConnection === PAUSE TestAccAWSRouteTable_IPv4_To_VpcPeeringConnection === RUN TestAccAWSRouteTable_vgwRoutePropagation === PAUSE TestAccAWSRouteTable_vgwRoutePropagation === RUN TestAccAWSRouteTable_ConditionalCidrBlock === PAUSE TestAccAWSRouteTable_ConditionalCidrBlock === RUN TestAccAWSRouteTable_IPv4_To_NatGateway === PAUSE TestAccAWSRouteTable_IPv4_To_NatGateway === RUN TestAccAWSRouteTable_IPv6_To_NetworkInterface_Unattached === PAUSE TestAccAWSRouteTable_IPv6_To_NetworkInterface_Unattached === RUN TestAccAWSRouteTable_VpcMultipleCidrs === PAUSE TestAccAWSRouteTable_VpcMultipleCidrs === RUN TestAccAWSRouteTable_VpcClassicLink === PAUSE TestAccAWSRouteTable_VpcClassicLink === RUN TestAccAWSRouteTable_GatewayVpcEndpoint === PAUSE TestAccAWSRouteTable_GatewayVpcEndpoint === RUN TestAccAWSRouteTable_MultipleRoutes === PAUSE TestAccAWSRouteTable_MultipleRoutes === RUN TestAccAWSRouteTable_PrefixList_To_InternetGateway === PAUSE TestAccAWSRouteTable_PrefixList_To_InternetGateway === CONT TestAccAWSDefaultRouteTable_basic === CONT TestAccAWSRouteTable_RequireRouteTarget --- PASS: TestAccAWSRouteTable_RequireRouteTarget (13.47s) === CONT TestAccAWSRouteTable_PrefixList_To_InternetGateway --- PASS: TestAccAWSDefaultRouteTable_basic (33.14s) === CONT TestAccAWSRouteTable_MultipleRoutes --- PASS: TestAccAWSRouteTable_PrefixList_To_InternetGateway (42.18s) === CONT TestAccAWSRouteTable_GatewayVpcEndpoint --- PASS: TestAccAWSRouteTable_GatewayVpcEndpoint (55.61s) === CONT TestAccAWSRouteTable_VpcClassicLink --- PASS: TestAccAWSRouteTable_VpcClassicLink (23.84s) === CONT TestAccAWSRouteTable_VpcMultipleCidrs --- PASS: TestAccAWSRouteTable_VpcMultipleCidrs (44.73s) === CONT TestAccAWSRouteTable_IPv6_To_NetworkInterface_Unattached --- PASS: TestAccAWSRouteTable_MultipleRoutes (153.22s) === CONT TestAccAWSRouteTable_IPv4_To_NatGateway --- PASS: TestAccAWSRouteTable_IPv6_To_NetworkInterface_Unattached (64.51s) === CONT TestAccAWSRouteTable_ConditionalCidrBlock --- PASS: TestAccAWSRouteTable_ConditionalCidrBlock (53.93s) === CONT TestAccAWSRouteTable_vgwRoutePropagation --- PASS: TestAccAWSRouteTable_vgwRoutePropagation (86.30s) === CONT TestAccAWSRouteTable_IPv4_To_VpcPeeringConnection --- PASS: TestAccAWSRouteTable_IPv4_To_VpcPeeringConnection (26.74s) === CONT TestAccAWSRouteTable_IPv4_To_LocalGateway data_source_aws_outposts_outposts_test.go:67: skipping since no Outposts found --- SKIP: TestAccAWSRouteTable_IPv4_To_LocalGateway (0.67s) === CONT TestAccAWSRouteTable_IPv4_To_CarrierGateway --- PASS: TestAccAWSRouteTable_IPv4_To_NatGateway (235.45s) === CONT TestAccAWSRouteTable_IPv4_To_VpcEndpoint --- PASS: TestAccAWSRouteTable_IPv4_To_CarrierGateway (26.19s) === CONT TestAccAWSRouteTable_IPv4_To_TransitGateway === CONT TestAccAWSRouteTable_Route_ConfigMode --- PASS: TestAccAWSRouteTable_IPv4_To_VpcEndpoint (261.01s) --- PASS: TestAccAWSRouteTable_Route_ConfigMode (71.61s) === CONT TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway --- PASS: TestAccAWSRouteTable_IPv4_To_TransitGateway (359.96s) === CONT TestAccAWSRouteTable_RequireRouteDestination --- PASS: TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway (54.98s) === CONT TestAccAWSRouteTable_tags --- PASS: TestAccAWSRouteTable_tags (68.33s) === CONT TestAccAWSRouteTable_IPv6_To_EgressOnlyInternetGateway --- PASS: TestAccAWSRouteTable_IPv6_To_EgressOnlyInternetGateway (48.66s) === CONT TestAccAWSRouteTable_IPv4_To_Instance --- PASS: TestAccAWSRouteTable_IPv4_To_Instance (81.54s) === CONT TestAccAWSRouteTable_IPv4_To_InternetGateway --- PASS: TestAccAWSRouteTable_IPv4_To_InternetGateway (63.41s) === CONT TestAccAWSRouteTable_disappears_SubnetAssociation --- PASS: TestAccAWSRouteTable_disappears_SubnetAssociation (24.67s) === CONT TestAccAWSRouteTable_disappears --- PASS: TestAccAWSRouteTable_RequireRouteDestination (304.50s) === CONT TestAccAWSRouteTable_basic === CONT TestAccAWSDefaultRouteTable_IPv4_To_VpcEndpoint --- PASS: TestAccAWSRouteTable_disappears (20.62s) --- PASS: TestAccAWSRouteTable_basic (22.67s) === CONT TestAccAWSDefaultRouteTable_ConditionalCidrBlock --- PASS: TestAccAWSDefaultRouteTable_ConditionalCidrBlock (55.10s) === CONT TestAccAWSDefaultRouteTable_tags --- PASS: TestAccAWSDefaultRouteTable_tags (53.67s) === CONT TestAccAWSDefaultRouteTable_VpcEndpointAssociation --- PASS: TestAccAWSDefaultRouteTable_VpcEndpointAssociation (44.59s) === CONT TestAccAWSDefaultRouteTable_swap --- PASS: TestAccAWSDefaultRouteTable_swap (102.85s) === CONT TestAccAWSDefaultRouteTable_IPv4_To_TransitGateway --- PASS: TestAccAWSDefaultRouteTable_IPv4_To_VpcEndpoint (278.89s) === CONT TestAccAWSDefaultRouteTable_Route_ConfigMode --- PASS: TestAccAWSDefaultRouteTable_Route_ConfigMode (94.74s) === CONT TestAccAWSDefaultRouteTable_disappears_Vpc --- PASS: TestAccAWSDefaultRouteTable_disappears_Vpc (15.28s) --- PASS: TestAccAWSDefaultRouteTable_IPv4_To_TransitGateway (369.81s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 1751.431s
…stingRules'. Acceptance test output: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSDefaultRouteTable_' ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=TestAccAWSDefaultRouteTable_ -timeout 180m === RUN TestAccAWSDefaultRouteTable_basic === PAUSE TestAccAWSDefaultRouteTable_basic === RUN TestAccAWSDefaultRouteTable_disappears_Vpc === PAUSE TestAccAWSDefaultRouteTable_disappears_Vpc === RUN TestAccAWSDefaultRouteTable_Route_ConfigMode === PAUSE TestAccAWSDefaultRouteTable_Route_ConfigMode === RUN TestAccAWSDefaultRouteTable_swap === PAUSE TestAccAWSDefaultRouteTable_swap === RUN TestAccAWSDefaultRouteTable_IPv4_To_TransitGateway === PAUSE TestAccAWSDefaultRouteTable_IPv4_To_TransitGateway === RUN TestAccAWSDefaultRouteTable_IPv4_To_VpcEndpoint === PAUSE TestAccAWSDefaultRouteTable_IPv4_To_VpcEndpoint === RUN TestAccAWSDefaultRouteTable_VpcEndpointAssociation === PAUSE TestAccAWSDefaultRouteTable_VpcEndpointAssociation === RUN TestAccAWSDefaultRouteTable_tags === PAUSE TestAccAWSDefaultRouteTable_tags === RUN TestAccAWSDefaultRouteTable_ConditionalCidrBlock === PAUSE TestAccAWSDefaultRouteTable_ConditionalCidrBlock === RUN TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway === PAUSE TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway === RUN TestAccAWSDefaultRouteTable_RevokeExistingRules === PAUSE TestAccAWSDefaultRouteTable_RevokeExistingRules === CONT TestAccAWSDefaultRouteTable_basic === CONT TestAccAWSDefaultRouteTable_VpcEndpointAssociation --- PASS: TestAccAWSDefaultRouteTable_basic (27.81s) === CONT TestAccAWSDefaultRouteTable_RevokeExistingRules --- PASS: TestAccAWSDefaultRouteTable_VpcEndpointAssociation (47.64s) === CONT TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway --- PASS: TestAccAWSDefaultRouteTable_PrefixList_To_InternetGateway (54.34s) === CONT TestAccAWSDefaultRouteTable_ConditionalCidrBlock --- PASS: TestAccAWSDefaultRouteTable_RevokeExistingRules (104.87s) === CONT TestAccAWSDefaultRouteTable_tags --- PASS: TestAccAWSDefaultRouteTable_ConditionalCidrBlock (55.49s) === CONT TestAccAWSDefaultRouteTable_swap --- PASS: TestAccAWSDefaultRouteTable_tags (55.05s) === CONT TestAccAWSDefaultRouteTable_IPv4_To_VpcEndpoint --- PASS: TestAccAWSDefaultRouteTable_swap (101.23s) === CONT TestAccAWSDefaultRouteTable_IPv4_To_TransitGateway --- PASS: TestAccAWSDefaultRouteTable_IPv4_To_VpcEndpoint (318.61s) === CONT TestAccAWSDefaultRouteTable_Route_ConfigMode --- PASS: TestAccAWSDefaultRouteTable_Route_ConfigMode (93.98s) === CONT TestAccAWSDefaultRouteTable_disappears_Vpc --- PASS: TestAccAWSDefaultRouteTable_disappears_Vpc (14.50s) --- PASS: TestAccAWSDefaultRouteTable_IPv4_To_TransitGateway (400.86s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 659.684s
The typical route table creation steps involve making an ec2/CreateRouteTable request, an ec2/DescribeRouteTables request, some other requests to update the route table, followed by a final ec2.DescribeRouteTables request. If the response to the final ec2/DescribeRouteTables request is InvalidRouteTableID.NotFound, then the ID of the route table resource is cleared so that the route table resource is not retained. However, the creation is still considered a success. This results in either (1) failures from other resources that depend upon the route table resource or (2) a report of a successful apply even though one of the resources needed does not actually exist. These changes adjust the result of the creation in the case where the route table cannot be found on the final ec2/DescribeRouteTables request. Instead of reporting success for the creation, report failure. Fixes hashicorp#7644
…shicorp#16796). Acceptance test output: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSRouteTable_' ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=TestAccAWSRouteTable_ -timeout 180m === RUN TestAccAWSRouteTable_basic === PAUSE TestAccAWSRouteTable_basic === RUN TestAccAWSRouteTable_disappears === PAUSE TestAccAWSRouteTable_disappears === RUN TestAccAWSRouteTable_disappears_SubnetAssociation === PAUSE TestAccAWSRouteTable_disappears_SubnetAssociation === RUN TestAccAWSRouteTable_IPv4_To_InternetGateway === PAUSE TestAccAWSRouteTable_IPv4_To_InternetGateway === RUN TestAccAWSRouteTable_IPv4_To_Instance === PAUSE TestAccAWSRouteTable_IPv4_To_Instance === RUN TestAccAWSRouteTable_IPv6_To_EgressOnlyInternetGateway === PAUSE TestAccAWSRouteTable_IPv6_To_EgressOnlyInternetGateway === RUN TestAccAWSRouteTable_tags === PAUSE TestAccAWSRouteTable_tags === RUN TestAccAWSRouteTable_RequireRouteDestination === PAUSE TestAccAWSRouteTable_RequireRouteDestination === RUN TestAccAWSRouteTable_RequireRouteTarget === PAUSE TestAccAWSRouteTable_RequireRouteTarget === RUN TestAccAWSRouteTable_Route_ConfigMode === PAUSE TestAccAWSRouteTable_Route_ConfigMode === RUN TestAccAWSRouteTable_IPv4_To_TransitGateway === PAUSE TestAccAWSRouteTable_IPv4_To_TransitGateway === RUN TestAccAWSRouteTable_IPv4_To_VpcEndpoint === PAUSE TestAccAWSRouteTable_IPv4_To_VpcEndpoint === RUN TestAccAWSRouteTable_IPv4_To_CarrierGateway === PAUSE TestAccAWSRouteTable_IPv4_To_CarrierGateway === RUN TestAccAWSRouteTable_IPv4_To_LocalGateway === PAUSE TestAccAWSRouteTable_IPv4_To_LocalGateway === RUN TestAccAWSRouteTable_IPv4_To_VpcPeeringConnection === PAUSE TestAccAWSRouteTable_IPv4_To_VpcPeeringConnection === RUN TestAccAWSRouteTable_vgwRoutePropagation === PAUSE TestAccAWSRouteTable_vgwRoutePropagation === RUN TestAccAWSRouteTable_ConditionalCidrBlock === PAUSE TestAccAWSRouteTable_ConditionalCidrBlock === RUN TestAccAWSRouteTable_IPv4_To_NatGateway === PAUSE TestAccAWSRouteTable_IPv4_To_NatGateway === RUN TestAccAWSRouteTable_IPv6_To_NetworkInterface_Unattached === PAUSE TestAccAWSRouteTable_IPv6_To_NetworkInterface_Unattached === RUN TestAccAWSRouteTable_VpcMultipleCidrs === PAUSE TestAccAWSRouteTable_VpcMultipleCidrs === RUN TestAccAWSRouteTable_VpcClassicLink === PAUSE TestAccAWSRouteTable_VpcClassicLink === RUN TestAccAWSRouteTable_GatewayVpcEndpoint === PAUSE TestAccAWSRouteTable_GatewayVpcEndpoint === RUN TestAccAWSRouteTable_MultipleRoutes === PAUSE TestAccAWSRouteTable_MultipleRoutes === RUN TestAccAWSRouteTable_PrefixList_To_InternetGateway === PAUSE TestAccAWSRouteTable_PrefixList_To_InternetGateway === CONT TestAccAWSRouteTable_basic === CONT TestAccAWSRouteTable_IPv4_To_LocalGateway data_source_aws_outposts_outposts_test.go:67: skipping since no Outposts found --- SKIP: TestAccAWSRouteTable_IPv4_To_LocalGateway (1.40s) === CONT TestAccAWSRouteTable_PrefixList_To_InternetGateway --- PASS: TestAccAWSRouteTable_basic (22.84s) === CONT TestAccAWSRouteTable_MultipleRoutes --- PASS: TestAccAWSRouteTable_PrefixList_To_InternetGateway (42.61s) === CONT TestAccAWSRouteTable_GatewayVpcEndpoint --- PASS: TestAccAWSRouteTable_GatewayVpcEndpoint (65.20s) === CONT TestAccAWSRouteTable_VpcClassicLink --- PASS: TestAccAWSRouteTable_VpcClassicLink (23.99s) === CONT TestAccAWSRouteTable_VpcMultipleCidrs --- PASS: TestAccAWSRouteTable_VpcMultipleCidrs (44.49s) === CONT TestAccAWSRouteTable_IPv6_To_NetworkInterface_Unattached --- PASS: TestAccAWSRouteTable_MultipleRoutes (174.34s) === CONT TestAccAWSRouteTable_IPv4_To_NatGateway --- PASS: TestAccAWSRouteTable_IPv6_To_NetworkInterface_Unattached (64.12s) === CONT TestAccAWSRouteTable_ConditionalCidrBlock --- PASS: TestAccAWSRouteTable_ConditionalCidrBlock (54.20s) === CONT TestAccAWSRouteTable_vgwRoutePropagation --- PASS: TestAccAWSRouteTable_vgwRoutePropagation (86.26s) === CONT TestAccAWSRouteTable_IPv4_To_VpcPeeringConnection --- PASS: TestAccAWSRouteTable_IPv4_To_VpcPeeringConnection (26.81s) === CONT TestAccAWSRouteTable_RequireRouteDestination --- PASS: TestAccAWSRouteTable_IPv4_To_NatGateway (246.64s) === CONT TestAccAWSRouteTable_IPv4_To_CarrierGateway --- PASS: TestAccAWSRouteTable_IPv4_To_CarrierGateway (26.55s) === CONT TestAccAWSRouteTable_IPv4_To_VpcEndpoint --- PASS: TestAccAWSRouteTable_RequireRouteDestination (324.00s) === CONT TestAccAWSRouteTable_IPv4_To_TransitGateway --- PASS: TestAccAWSRouteTable_IPv4_To_VpcEndpoint (263.18s) === CONT TestAccAWSRouteTable_Route_ConfigMode --- PASS: TestAccAWSRouteTable_Route_ConfigMode (70.84s) === CONT TestAccAWSRouteTable_RequireRouteTarget --- PASS: TestAccAWSRouteTable_RequireRouteTarget (11.86s) === CONT TestAccAWSRouteTable_IPv4_To_Instance --- PASS: TestAccAWSRouteTable_IPv4_To_Instance (113.23s) === CONT TestAccAWSRouteTable_tags --- PASS: TestAccAWSRouteTable_tags (68.59s) === CONT TestAccAWSRouteTable_IPv6_To_EgressOnlyInternetGateway --- PASS: TestAccAWSRouteTable_IPv6_To_EgressOnlyInternetGateway (47.38s) === CONT TestAccAWSRouteTable_disappears_SubnetAssociation --- PASS: TestAccAWSRouteTable_disappears_SubnetAssociation (24.28s) === CONT TestAccAWSRouteTable_IPv4_To_InternetGateway --- PASS: TestAccAWSRouteTable_IPv4_To_TransitGateway (339.66s) === CONT TestAccAWSRouteTable_disappears --- PASS: TestAccAWSRouteTable_disappears (20.05s) --- PASS: TestAccAWSRouteTable_IPv4_To_InternetGateway (64.71s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 1134.554s
eb87721
to
2fef042
Compare
return func() (interface{}, string, error) { | ||
output, err := finder.RouteTableByID(conn, id) | ||
|
||
if tfawserr.ErrCodeEquals(err, ErrCodeInvalidRouteTableIDNotFound) { |
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.
Should be if tfresource.NotFound(err)
as RouteTableByID
returns a resource.NotFoundError
if no route is found.
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.
AWS has some consistency problems with the ec2 package. However, I'm uncomfortable changing the API's response in the finder to resource.NotFoundError
. At some point, the API will/should add the error const. Between the options here, 1) changing the response to NotFoundError
and 2) returning the API response and using our own const, the tail on latter option is less problematic. We'd simply swap our const for their const. However, when changing the API response, we're not always sure future will code will use the finder leading to the problem of handling separate errors from the supposedly same API call. Also, it means the route table code is inconsistent using tfresource.NotFound()
instead of tfawserr.ErrCodeEquals()
like most other places we check d.IsNewResource()
. Thoughts?
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.
- Agree that consistency is important
- Right now most finders returns
nil
,nil
when the resource is not found although some are now returningnil
,resource.NotFoundError
(see Proposal: Add more fine-grained errors for finder functions #17613, Simplify single security group searches and error handling #18433) RouteTableByID
is already called in a number of replaces and is returningnil
,resource.NotFoundError
- As a totally personal opinion, I have come round from thinking that
nil
,nil
is better to preferringnil
,resource.NotFoundError
(better error propagation, less possibility fornil
pointer crashes) - GOTO 1 😄
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.
I think this needs more discussion and particularly to get everyone on board one way or the other. Since each approach takes such a drastically different path (error vs. no error), a lot of code would need to be changed to conform to a new approach. It will be confusing and error prone to have some code take one path and other take another. Casual contributors will randomly grab one or the other.
Should the pattern be 1) for finders to return an error for not found, and 2) for the waiter to eat the error when appropriate (Deleted
waiters)? Then you've got code that uses a finder handling an error while code using the waiter doesn't.
It's all doable and not necessarily right/wrong. But, in my mind, the key is for us to together figure out what we'll do so that we're on the same page and get the community on that page. To continue the metaphor, how helpful and correct a new page is should be weighed against the challenge of getting everyone on that page.
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.
When I first started creating the finder
functions, I went with nil, nil
mostly because it was simple, and I wasn't really thinking of the standard errors that the Plugin SDK provided. However, after some more thought, I figured that returning a resource.NotFoundError
would be more semantic, allow the function to be more self-contained, and give us consistent error handling.
If you need to access the underlying AWS error, it is available in the LastError
field of the resource.NotFoundError
.
One additional benefit of using a resource.NotFoundError
is avoiding checking for multiple not found cases, e.g.
terraform-provider-aws/aws/resource_aws_backup_vault.go
Lines 83 to 96 in 19b8055
if isAWSErr(err, backup.ErrCodeResourceNotFoundException, "") { | |
log.Printf("[WARN] Backup Vault %s not found, removing from state", d.Id()) | |
d.SetId("") | |
return nil | |
} | |
if isAWSErr(err, "AccessDeniedException", "") { | |
log.Printf("[WARN] Backup Vault %s not found, removing from state", d.Id()) | |
d.SetId("") | |
return nil | |
} | |
if err != nil { | |
return fmt.Errorf("error reading Backup Vault (%s): %s", d.Id(), err) | |
} |
terraform-provider-aws/aws/resource_aws_budgets_budget.go
Lines 311 to 326 in 19b8055
if isAWSErr(err, budgets.ErrCodeNotFoundException, "") { | |
log.Printf("[WARN] Budget %s not found, removing from state", d.Id()) | |
d.SetId("") | |
return nil | |
} | |
if err != nil { | |
return fmt.Errorf("describe budget failed: %v", err) | |
} | |
budget := describeBudgetOutput.Budget | |
if budget == nil { | |
log.Printf("[WARN] Budget %s not found, removing from state", d.Id()) | |
d.SetId("") | |
return nil | |
} |
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.
In order to reduce creep scope in this PR, we may want to consider rewinding back to 1961ca1 and moving all the subsequent commits into a new PR or multiple new PRs.
@@ -245,6 +245,32 @@ func InstanceIamInstanceProfile(conn *ec2.EC2, id string) resource.StateRefreshF | |||
} | |||
} | |||
|
|||
const ( | |||
ErrCodeInvalidRouteTableIDNotFound = "InvalidRouteTableID.NotFound" |
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.
Can remove this as it is should not be referenced below.
func RouteTableReady(conn *ec2.EC2, id string) (*ec2.RouteTable, error) { | ||
stateConf := &resource.StateChangeConf{ | ||
Pending: []string{"pending"}, | ||
Target: []string{"ready"}, |
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.
Target: []string{RouteTableStatusReady},
|
||
func RouteTableReady(conn *ec2.EC2, id string) (*ec2.RouteTable, error) { | ||
stateConf := &resource.StateChangeConf{ | ||
Pending: []string{"pending"}, |
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.
I think Pending: []string{},
should be OK as no code actually sets the status to "pending".
|
||
outputRaw, err := stateConf.WaitForState() | ||
|
||
if tfresource.NotFound(err) { |
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.
This can go since the status eats NotFound
.
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! 🎉
Acceptance tests in GovCloud:
--- PASS: TestAccAWSRouteDataSource_basic (122.02s)
--- PASS: TestAccAWSRouteDataSource_IPv6DestinationCidr (32.59s)
--- PASS: TestAccAWSRouteDataSource_TransitGatewayID (354.09s)
--- SKIP: TestAccAWSRouteDataSource_LocalGatewayID (1.91s)
--- PASS: TestAccAWSRouteTable_basic (34.76s)
--- PASS: TestAccAWSRouteTable_ConditionalCidrBlock (72.46s)
--- PASS: TestAccAWSRouteTable_disappears (38.83s)
--- PASS: TestAccAWSRouteTable_disappears_SubnetAssociation (406.62s)
--- PASS: TestAccAWSRouteTable_GatewayVpcEndpoint (53.28s)
--- PASS: TestAccAWSRouteTable_IPv4_To_Instance (138.58s)
--- PASS: TestAccAWSRouteTable_IPv4_To_InternetGateway (69.07s)
--- PASS: TestAccAWSRouteTable_IPv4_To_NatGateway (204.53s)
--- PASS: TestAccAWSRouteTable_IPv4_To_TransitGateway (337.90s)
--- PASS: TestAccAWSRouteTable_IPv4_To_VpcPeeringConnection (41.59s)
--- PASS: TestAccAWSRouteTable_IPv6_To_EgressOnlyInternetGateway (53.08s)
--- PASS: TestAccAWSRouteTable_IPv6_To_NetworkInterface_Unattached (80.50s)
--- PASS: TestAccAWSRouteTable_RequireRouteDestination (324.71s)
--- PASS: TestAccAWSRouteTable_RequireRouteTarget (14.42s)
--- PASS: TestAccAWSRouteTable_Route_ConfigMode (79.16s)
--- PASS: TestAccAWSRouteTable_tags (82.11s)
--- PASS: TestAccAWSRouteTable_vgwRoutePropagation (101.23s)
--- PASS: TestAccAWSRouteTable_VpcMultipleCidrs (57.43s)
--- SKIP: TestAccAWSRouteTable_IPv4_To_CarrierGateway (1.61s)
--- SKIP: TestAccAWSRouteTable_IPv4_To_LocalGateway (1.87s)
--- SKIP: TestAccAWSRouteTable_IPv4_To_VpcEndpoint (1.68s)
--- SKIP: TestAccAWSRouteTable_PrefixList_To_InternetGateway (1.63s)
--- SKIP: TestAccAWSRouteTable_VpcClassicLink (15.39s)
--- PASS: TestAccAWSRouteTableAssociation_disappears (51.80s)
--- PASS: TestAccAWSRouteTableAssociation_Gateway_basic (81.02s)
--- PASS: TestAccAWSRouteTableAssociation_Gateway_ChangeGateway (116.12s)
--- PASS: TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable (98.85s)
--- PASS: TestAccAWSRouteTableAssociation_Subnet_basic (53.34s)
--- PASS: TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable (60.24s)
--- PASS: TestAccAWSRouteTableAssociation_Subnet_ChangeSubnet (70.98s)
--- PASS: TestAccAWSVPNGatewayRoutePropagation_basic (57.80s)
--- PASS: TestAccDataSourceAwsRouteTable_basic (39.61s)
--- PASS: TestAccDataSourceAwsRouteTable_main (24.31s)
Acceptance tests in commercial:
--- PASS: TestAccAWSRouteDataSource_basic (131.73s)
--- PASS: TestAccAWSRouteDataSource_CarrierGatewayID (25.49s)
--- PASS: TestAccAWSRouteDataSource_IPv6DestinationCidr (24.89s)
--- PASS: TestAccAWSRouteDataSource_TransitGatewayID (392.82s)
--- SKIP: TestAccAWSRouteDataSource_LocalGatewayID (1.30s)
--- PASS: TestAccAWSRouteTable_basic (25.01s)
--- PASS: TestAccAWSRouteTable_ConditionalCidrBlock (69.21s)
--- PASS: TestAccAWSRouteTable_disappears (26.14s)
--- PASS: TestAccAWSRouteTable_disappears_SubnetAssociation (402.26s)
--- PASS: TestAccAWSRouteTable_GatewayVpcEndpoint (49.55s)
--- PASS: TestAccAWSRouteTable_IPv4_To_CarrierGateway (34.91s)
--- PASS: TestAccAWSRouteTable_IPv4_To_Instance (102.12s)
--- PASS: TestAccAWSRouteTable_IPv4_To_InternetGateway (67.94s)
--- PASS: TestAccAWSRouteTable_IPv4_To_NatGateway (210.97s)
--- PASS: TestAccAWSRouteTable_IPv4_To_TransitGateway (376.66s)
--- PASS: TestAccAWSRouteTable_IPv4_To_VpcEndpoint (252.59s)
--- PASS: TestAccAWSRouteTable_IPv4_To_VpcPeeringConnection (38.88s)
--- PASS: TestAccAWSRouteTable_IPv6_To_EgressOnlyInternetGateway (51.71s)
--- PASS: TestAccAWSRouteTable_IPv6_To_NetworkInterface_Unattached (75.47s)
--- PASS: TestAccAWSRouteTable_MultipleRoutes (190.15s)
--- PASS: TestAccAWSRouteTable_PrefixList_To_InternetGateway (51.57s)
--- PASS: TestAccAWSRouteTable_RequireRouteDestination (310.96s)
--- PASS: TestAccAWSRouteTable_RequireRouteTarget (18.34s)
--- PASS: TestAccAWSRouteTable_Route_ConfigMode (87.60s)
--- PASS: TestAccAWSRouteTable_tags (84.28s)
--- PASS: TestAccAWSRouteTable_vgwRoutePropagation (92.07s)
--- PASS: TestAccAWSRouteTable_VpcClassicLink (33.15s)
--- PASS: TestAccAWSRouteTable_VpcMultipleCidrs (52.73s)
--- SKIP: TestAccAWSRouteTable_IPv4_To_LocalGateway (1.51s)
--- PASS: TestAccDataSourceAwsRouteTable_main (20.46s)
--- PASS: TestAccDataSourceAwsRouteTable_basic (35.97s)
--- PASS: TestAccAWSRouteTableAssociation_disappears (39.67s)
--- PASS: TestAccAWSRouteTableAssociation_Gateway_basic (70.65s)
--- PASS: TestAccAWSRouteTableAssociation_Gateway_ChangeGateway (94.58s)
--- PASS: TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable (87.68s)
--- PASS: TestAccAWSRouteTableAssociation_Subnet_basic (47.38s)
--- PASS: TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable (56.70s)
--- PASS: TestAccAWSRouteTableAssociation_Subnet_ChangeSubnet (58.95s)
--- PASS: TestAccAWSVPNGatewayRoutePropagation_basic (57.70s)
This has been released in version 3.35.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: #15273.
Closes: #7644.
Builds on #16979.
Output from acceptance testing: