Skip to content

Commit

Permalink
resource/aws_default_route_table: Fixes for creation and recreation b…
Browse files Browse the repository at this point in the history
…ehavior when Default Route Table is missing (#10981)

Reference: #398
Reference: #3551
Reference: #9009

Previously on creation, if the Default Route Table was incorrectly configured or non-existent, the resource would unexpectedly attempt to remove itself from the Terraform state and propose recreation immediately. Prior to Terraform 0.12, this behavior was errantly acceptable. In Terraform 0.12, resources are required to return Terraform state about themselves during creation or throw an error explaining why the creation failed.

Previously on read, if the Default Route Table was missing (e.g. due to the VPC being deleted outside Terraform), the resource would return an error and require operators to manually perform a `terraform state rm` command instead of proposing resource recreation.

Output from new acceptance testing before code updates:

```
--- FAIL: TestAccAWSDefaultRouteTable_basic (20.02s)
    testing.go:628: Step 0, expected error:

        errors during apply: Provider produced inconsistent result after apply: When applying changes to aws_default_route_table.foo, provider "aws" produced an unexpected new value for was present, but now absent.

        This is a bug in the provider, which should be reported in the provider's own issue tracker.

        To match:

        TBD

--- FAIL: TestAccAWSDefaultRouteTable_disappears_Vpc (20.27s)
    testing.go:635: Step 0 error: errors during follow-up refresh:

        Error: Default Route table not found

    testing.go:696: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: errors during refresh: Default Route table not found

        State: <nil>
```

Output from acceptance testing after code updates:

```
--- PASS: TestAccAWSDefaultRouteTable_disappears_Vpc (20.46s)
--- PASS: TestAccAWSDefaultRouteTable_basic (40.99s)
--- PASS: TestAccAWSDefaultRouteTable_vpc_endpoint (49.71s)
--- PASS: TestAccAWSDefaultRouteTable_swap (73.89s)
--- PASS: TestAccAWSDefaultRouteTable_Route (87.09s)
--- PASS: TestAccAWSDefaultRouteTable_Route_TransitGatewayID (374.87s)
```
  • Loading branch information
bflad authored Nov 25, 2019
1 parent a5d02ee commit e60923b
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 5 deletions.
10 changes: 5 additions & 5 deletions aws/resource_aws_default_route_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,10 @@ func resourceAwsDefaultRouteTableCreate(d *schema.ResourceData, meta interface{}
conn := meta.(*AWSClient).ec2conn
rtRaw, _, err := resourceAwsRouteTableStateRefreshFunc(conn, d.Id())()
if err != nil {
return err
return fmt.Errorf("error reading EC2 Default Route Table (%s): %s", d.Id(), err)
}
if rtRaw == nil {
log.Printf("[WARN] Default Route Table not found")
d.SetId("")
return nil
return fmt.Errorf("error reading EC2 Default Route Table (%s): not found", d.Id())
}

rt := rtRaw.(*ec2.RouteTable)
Expand Down Expand Up @@ -151,7 +149,9 @@ func resourceAwsDefaultRouteTableRead(d *schema.ResourceData, meta interface{})
}

if len(resp.RouteTables) < 1 || resp.RouteTables[0] == nil {
return fmt.Errorf("Default Route table not found")
log.Printf("[WARN] EC2 Default Route Table (%s) not found, removing from state", d.Id())
d.SetId("")
return nil
}

rt := resp.RouteTables[0]
Expand Down
80 changes: 80 additions & 0 deletions aws/resource_aws_default_route_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package aws

import (
"fmt"
"regexp"
"testing"

"github.com/aws/aws-sdk-go/aws"
Expand All @@ -12,6 +13,65 @@ import (
)

func TestAccAWSDefaultRouteTable_basic(t *testing.T) {
var routeTable1 ec2.RouteTable
resourceName := "aws_default_route_table.test"
vpcResourceName := "aws_vpc.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckRouteTableDestroy,
Steps: []resource.TestStep{
// Verify non-existent Route Table ID behavior
{
Config: testAccDefaultRouteTableConfigDefaultRouteTableId("rtb-00000000"),
ExpectError: regexp.MustCompile(`EC2 Default Route Table \(rtb-00000000\): not found`),
},
// Verify invalid Route Table ID behavior
{
Config: testAccDefaultRouteTableConfigDefaultRouteTableId("vpc-00000000"),
ExpectError: regexp.MustCompile(`EC2 Default Route Table \(vpc-00000000\): not found`),
},
{
Config: testAccDefaultRouteTableConfigRequired(),
Check: resource.ComposeTestCheckFunc(
testAccCheckRouteTableExists(resourceName, &routeTable1),
testAccCheckResourceAttrAccountID(resourceName, "owner_id"),
resource.TestCheckResourceAttr(resourceName, "propagating_vgws.#", "0"),
resource.TestCheckResourceAttr(resourceName, "route.#", "0"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
resource.TestCheckResourceAttrPair(resourceName, "vpc_id", vpcResourceName, "id"),
),
},
},
})
}

func TestAccAWSDefaultRouteTable_disappears_Vpc(t *testing.T) {
var routeTable1 ec2.RouteTable
var vpc1 ec2.Vpc
resourceName := "aws_default_route_table.test"
vpcResourceName := "aws_vpc.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckRouteTableDestroy,
Steps: []resource.TestStep{
{
Config: testAccDefaultRouteTableConfigRequired(),
Check: resource.ComposeTestCheckFunc(
testAccCheckRouteTableExists(resourceName, &routeTable1),
testAccCheckVpcExists(vpcResourceName, &vpc1),
testAccCheckVpcDisappears(&vpc1),
),
ExpectNonEmptyPlan: true,
},
},
})
}

func TestAccAWSDefaultRouteTable_Route(t *testing.T) {
var v ec2.RouteTable

resource.ParallelTest(t, resource.TestCase{
Expand Down Expand Up @@ -153,6 +213,26 @@ func testAccCheckDefaultRouteTableDestroy(s *terraform.State) error {
return nil
}

func testAccDefaultRouteTableConfigDefaultRouteTableId(defaultRouteTableId string) string {
return fmt.Sprintf(`
resource "aws_default_route_table" "test" {
default_route_table_id = %[1]q
}
`, defaultRouteTableId)
}

func testAccDefaultRouteTableConfigRequired() string {
return fmt.Sprintf(`
resource "aws_vpc" "test" {
cidr_block = "10.1.0.0/16"
}
resource "aws_default_route_table" "test" {
default_route_table_id = aws_vpc.test.default_route_table_id
}
`)
}

const testAccDefaultRouteTableConfig = `
resource "aws_vpc" "foo" {
cidr_block = "10.1.0.0/16"
Expand Down

0 comments on commit e60923b

Please sign in to comment.