From eb3106e0e9a18a0e1e55d34aca2a5de1c59ceb82 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 30 Mar 2021 09:38:42 -0400 Subject: [PATCH 01/31] Tidy up 'RouteTableStatus'. --- aws/internal/service/ec2/waiter/status.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/aws/internal/service/ec2/waiter/status.go b/aws/internal/service/ec2/waiter/status.go index c4ebc3d6226..6e444caa179 100644 --- a/aws/internal/service/ec2/waiter/status.go +++ b/aws/internal/service/ec2/waiter/status.go @@ -247,8 +247,6 @@ func InstanceIamInstanceProfile(conn *ec2.EC2, id string) resource.StateRefreshF } const ( - ErrCodeInvalidRouteTableIDNotFound = "InvalidRouteTableID.NotFound" - RouteTableStatusReady = "ready" ) @@ -264,10 +262,6 @@ func RouteTableStatus(conn *ec2.EC2, id string) resource.StateRefreshFunc { return nil, "", err } - if output == nil { - return nil, "", nil - } - return output, RouteTableStatusReady, nil } } From 21838955b4b1af2dcaad4f4f675950c09669b468 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 1 Apr 2021 10:34:11 -0400 Subject: [PATCH 02/31] Use 'finder.RouteTableByID' in acceptance test CheckExists and CheckDestroy functions. Acceptance test output: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSRoute_basic\|TestAccAWSRoute_disappears\|TestAccAWSDefaultRouteTable_basic\|TestAccAWSDefaultRouteTable_disappears\|TestAccAWSRouteTableAssociation_Subnet_basic\|TestAccAWSRouteTableAssociation_disappears' ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=TestAccAWSRoute_basic\|TestAccAWSRoute_disappears\|TestAccAWSDefaultRouteTable_basic\|TestAccAWSDefaultRouteTable_disappears\|TestAccAWSRouteTableAssociation_Subnet_basic\|TestAccAWSRouteTableAssociation_disappears -timeout 180m === RUN TestAccAWSDefaultRouteTable_basic === PAUSE TestAccAWSDefaultRouteTable_basic === RUN TestAccAWSDefaultRouteTable_disappears_Vpc === PAUSE TestAccAWSDefaultRouteTable_disappears_Vpc === RUN TestAccAWSRouteTableAssociation_Subnet_basic === PAUSE TestAccAWSRouteTableAssociation_Subnet_basic === RUN TestAccAWSRouteTableAssociation_disappears === PAUSE TestAccAWSRouteTableAssociation_disappears === RUN TestAccAWSRoute_basic === PAUSE TestAccAWSRoute_basic === RUN TestAccAWSRoute_disappears === PAUSE TestAccAWSRoute_disappears === RUN TestAccAWSRoute_disappears_RouteTable === PAUSE TestAccAWSRoute_disappears_RouteTable === CONT TestAccAWSDefaultRouteTable_basic === CONT TestAccAWSRoute_basic --- PASS: TestAccAWSDefaultRouteTable_basic (28.58s) === CONT TestAccAWSRoute_disappears_RouteTable --- PASS: TestAccAWSRoute_basic (36.75s) === CONT TestAccAWSRoute_disappears --- PASS: TestAccAWSRoute_disappears_RouteTable (33.95s) === CONT TestAccAWSRouteTableAssociation_Subnet_basic --- PASS: TestAccAWSRoute_disappears (33.06s) === CONT TestAccAWSRouteTableAssociation_disappears --- PASS: TestAccAWSRouteTableAssociation_Subnet_basic (39.37s) === CONT TestAccAWSDefaultRouteTable_disappears_Vpc --- PASS: TestAccAWSRouteTableAssociation_disappears (36.32s) --- PASS: TestAccAWSDefaultRouteTable_disappears_Vpc (14.72s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 116.707s --- aws/resource_aws_default_route_table_test.go | 20 +++---- ...source_aws_route_table_association_test.go | 56 +++++++------------ aws/resource_aws_route_table_test.go | 30 ++++------ 3 files changed, 39 insertions(+), 67 deletions(-) diff --git a/aws/resource_aws_default_route_table_test.go b/aws/resource_aws_default_route_table_test.go index a35e3e36d6f..d40b73823da 100644 --- a/aws/resource_aws_default_route_table_test.go +++ b/aws/resource_aws_default_route_table_test.go @@ -5,11 +5,12 @@ import ( "regexp" "testing" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func TestAccAWSDefaultRouteTable_basic(t *testing.T) { @@ -511,22 +512,17 @@ func testAccCheckDefaultRouteTableDestroy(s *terraform.State) error { continue } - // Try to find the resource - resp, err := conn.DescribeRouteTables(&ec2.DescribeRouteTablesInput{ - RouteTableIds: []*string{aws.String(rs.Primary.ID)}, - }) - if err == nil { - if len(resp.RouteTables) > 0 { - return fmt.Errorf("still exist.") - } + _, err := finder.RouteTableByID(conn, rs.Primary.ID) - return nil + if tfresource.NotFound(err) { + continue } - // Verify the error is what we want - if !isAWSErr(err, "InvalidRouteTableID.NotFound", "") { + if err != nil { return err } + + return fmt.Errorf("Route table %s still exists", rs.Primary.ID) } return nil diff --git a/aws/resource_aws_route_table_association_test.go b/aws/resource_aws_route_table_association_test.go index 9f865809374..f33aa25d0ae 100644 --- a/aws/resource_aws_route_table_association_test.go +++ b/aws/resource_aws_route_table_association_test.go @@ -5,10 +5,11 @@ import ( "testing" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func TestAccAWSRouteTableAssociation_Subnet_basic(t *testing.T) { @@ -240,26 +241,18 @@ func testAccCheckRouteTableAssociationDestroy(s *terraform.State) error { continue } - // Try to find the resource - resp, err := conn.DescribeRouteTables(&ec2.DescribeRouteTablesInput{ - RouteTableIds: []*string{aws.String(rs.Primary.Attributes["route_table_id"])}, - }) + routeTable, err := finder.RouteTableByID(conn, rs.Primary.ID) + + if tfresource.NotFound(err) { + continue + } + if err != nil { - // Verify the error is what we want - ec2err, ok := err.(awserr.Error) - if !ok { - return err - } - if ec2err.Code() != "InvalidRouteTableID.NotFound" { - return err - } - return nil + return err } - rt := resp.RouteTables[0] - if len(rt.Associations) > 0 { - return fmt.Errorf( - "route table %s has associations", *rt.RouteTableId) + if len(routeTable.Associations) > 0 { + return fmt.Errorf("Route table %s has associations", aws.StringValue(routeTable.RouteTableId)) } } @@ -278,33 +271,22 @@ func testAccCheckRouteTableAssociationExists(n string, rta *ec2.RouteTableAssoci } conn := testAccProvider.Meta().(*AWSClient).ec2conn - resp, err := conn.DescribeRouteTables(&ec2.DescribeRouteTablesInput{ - RouteTableIds: []*string{aws.String(rs.Primary.Attributes["route_table_id"])}, - }) + + routeTable, err := finder.RouteTableByID(conn, rs.Primary.Attributes["route_table_id"]) + if err != nil { return err } - if len(resp.RouteTables) == 0 { - return fmt.Errorf("Route Table not found") - } - if len(resp.RouteTables[0].Associations) == 0 { - return fmt.Errorf("no associations found for Route Table %q", rs.Primary.Attributes["route_table_id"]) - } - - found := false - for _, association := range resp.RouteTables[0].Associations { - if rs.Primary.ID == *association.RouteTableAssociationId { - found = true + for _, association := range routeTable.Associations { + if rs.Primary.ID == aws.StringValue(association.RouteTableAssociationId) { *rta = *association - break + + return nil } } - if !found { - return fmt.Errorf("Association %q not found on Route Table %q", rs.Primary.ID, rs.Primary.Attributes["route_table_id"]) - } - return nil + return fmt.Errorf("Association %q not found on Route Table %q", rs.Primary.ID, rs.Primary.Attributes["route_table_id"]) } } diff --git a/aws/resource_aws_route_table_test.go b/aws/resource_aws_route_table_test.go index e331af27ef8..fe046a1837f 100644 --- a/aws/resource_aws_route_table_test.go +++ b/aws/resource_aws_route_table_test.go @@ -14,6 +14,8 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func init() { @@ -1083,17 +1085,14 @@ func testAccCheckRouteTableExists(n string, v *ec2.RouteTable) resource.TestChec } conn := testAccProvider.Meta().(*AWSClient).ec2conn - resp, err := conn.DescribeRouteTables(&ec2.DescribeRouteTablesInput{ - RouteTableIds: []*string{aws.String(rs.Primary.ID)}, - }) + + routeTable, err := finder.RouteTableByID(conn, rs.Primary.ID) + if err != nil { return err } - if len(resp.RouteTables) == 0 { - return fmt.Errorf("RouteTable not found") - } - *v = *resp.RouteTables[0] + *v = *routeTable return nil } @@ -1107,22 +1106,17 @@ func testAccCheckRouteTableDestroy(s *terraform.State) error { continue } - // Try to find the resource - resp, err := conn.DescribeRouteTables(&ec2.DescribeRouteTablesInput{ - RouteTableIds: []*string{aws.String(rs.Primary.ID)}, - }) - if err == nil { - if len(resp.RouteTables) > 0 { - return fmt.Errorf("still exist.") - } + _, err := finder.RouteTableByID(conn, rs.Primary.ID) - return nil + if tfresource.NotFound(err) { + continue } - // Verify the error is what we want - if !isAWSErr(err, "InvalidRouteTableID.NotFound", "") { + if err != nil { return err } + + return fmt.Errorf("Route table %s still exists", rs.Primary.ID) } return nil From 999fac869a732abb5b43af9e9cdf717165be4a8b Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 4 May 2021 17:02:45 -0400 Subject: [PATCH 03/31] Add 'createRoute'. --- aws/resource_aws_route.go | 51 +++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/aws/resource_aws_route.go b/aws/resource_aws_route.go index 9465f48fce2..6ec5001ce26 100644 --- a/aws/resource_aws_route.go +++ b/aws/resource_aws_route.go @@ -230,27 +230,7 @@ func resourceAwsRouteCreate(d *schema.ResourceData, meta interface{}) error { } log.Printf("[DEBUG] Creating Route: %s", input) - err = resource.Retry(d.Timeout(schema.TimeoutCreate), func() *resource.RetryError { - _, err = conn.CreateRoute(input) - - if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidParameterException) { - return resource.RetryableError(err) - } - - if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidTransitGatewayIDNotFound) { - return resource.RetryableError(err) - } - - if err != nil { - return resource.NonRetryableError(err) - } - - return nil - }) - - if tfresource.TimedOut(err) { - _, err = conn.CreateRoute(input) - } + err = createRoute(conn, input, d.Timeout(schema.TimeoutCreate)) if err != nil { return fmt.Errorf("error creating Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) @@ -523,3 +503,32 @@ func routeTargetAttribute(d *schema.ResourceData) (string, string, error) { return "", "", fmt.Errorf("route target attribute not specified") } + +// createRoute attempts to create a route. +// The specified eventual consistency timeout is respected. +// Any error is returned. +func createRoute(conn *ec2.EC2, input *ec2.CreateRouteInput, timeout time.Duration) error { + err := resource.Retry(timeout, func() *resource.RetryError { + _, err := conn.CreateRoute(input) + + if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidParameterException) { + return resource.RetryableError(err) + } + + if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidTransitGatewayIDNotFound) { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + + if tfresource.TimedOut(err) { + _, err = conn.CreateRoute(input) + } + + return nil +} From f962e2b5a9ed4c4cbca1e27e1e84ce79664a85e1 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 4 May 2021 17:05:43 -0400 Subject: [PATCH 04/31] Add 'CanonicalCIDRBlock' to 'internal/net' package and remove "local" 'canonicalCidrBlock' and 'cidrBlocksEqual'. --- aws/diff_suppress_funcs.go | 3 +- aws/ec2_transit_gateway.go | 5 +-- aws/internal/net/cidr.go | 11 ++++++ aws/internal/net/cidr_test.go | 17 ++++++++++ aws/resource_aws_route_table.go | 3 +- aws/resource_aws_wafv2_ip_set.go | 3 +- aws/validators.go | 37 +++------------------ aws/validators_test.go | 57 -------------------------------- 8 files changed, 41 insertions(+), 95 deletions(-) diff --git a/aws/diff_suppress_funcs.go b/aws/diff_suppress_funcs.go index bfa7146c7f5..911559ea5a7 100644 --- a/aws/diff_suppress_funcs.go +++ b/aws/diff_suppress_funcs.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" awspolicy "github.com/jen20/awspolicyequivalence" + tfnet "github.com/terraform-providers/terraform-provider-aws/aws/internal/net" ) func suppressEquivalentAwsPolicyDiffs(k, old, new string, d *schema.ResourceData) bool { @@ -131,7 +132,7 @@ func suppressEquivalentJsonOrYamlDiffs(k, old, new string, d *schema.ResourceDat // suppressEqualCIDRBlockDiffs provides custom difference suppression for CIDR blocks // that have different string values but represent the same CIDR. func suppressEqualCIDRBlockDiffs(k, old, new string, d *schema.ResourceData) bool { - return cidrBlocksEqual(old, new) + return tfnet.CIDRBlocksEqual(old, new) } // suppressEquivalentTime suppresses differences for time values that represent the same diff --git a/aws/ec2_transit_gateway.go b/aws/ec2_transit_gateway.go index aa8bb57714c..b8d211e4f24 100644 --- a/aws/ec2_transit_gateway.go +++ b/aws/ec2_transit_gateway.go @@ -9,6 +9,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + tfnet "github.com/terraform-providers/terraform-provider-aws/aws/internal/net" ) func decodeEc2TransitGatewayRouteID(id string) (string, string, error) { @@ -109,8 +110,8 @@ func ec2DescribeTransitGatewayRoute(conn *ec2.EC2, transitGatewayRouteTableID, d if route == nil { continue } - if cidrBlocksEqual(aws.StringValue(route.DestinationCidrBlock), destination) { - cidrString := canonicalCidrBlock(aws.StringValue(route.DestinationCidrBlock)) + if tfnet.CIDRBlocksEqual(aws.StringValue(route.DestinationCidrBlock), destination) { + cidrString := tfnet.CanonicalCIDRBlock(aws.StringValue(route.DestinationCidrBlock)) route.DestinationCidrBlock = aws.String(cidrString) return route, nil } diff --git a/aws/internal/net/cidr.go b/aws/internal/net/cidr.go index 5d4e8db0525..77ac94522f2 100644 --- a/aws/internal/net/cidr.go +++ b/aws/internal/net/cidr.go @@ -21,3 +21,14 @@ func CIDRBlocksEqual(cidr1, cidr2 string) bool { return ip2.String() == ip1.String() && ipnet2.String() == ipnet1.String() } + +// CanonicalCIDRBlock returns the canonical representation of a CIDR block. +// This function is especially useful for hash functions for sets which include IPv6 CIDR blocks. +func CanonicalCIDRBlock(cidr string) string { + _, ipnet, err := net.ParseCIDR(cidr) + if err != nil { + return cidr + } + + return ipnet.String() +} diff --git a/aws/internal/net/cidr_test.go b/aws/internal/net/cidr_test.go index a557b64a579..ce1ccefa324 100644 --- a/aws/internal/net/cidr_test.go +++ b/aws/internal/net/cidr_test.go @@ -24,3 +24,20 @@ func Test_CIDRBlocksEqual(t *testing.T) { } } } + +func Test_CanonicalCIDRBlock(t *testing.T) { + for _, ts := range []struct { + cidr string + expected string + }{ + {"10.2.2.0/24", "10.2.2.0/24"}, + {"::/0", "::/0"}, + {"::0/0", "::/0"}, + {"", ""}, + } { + got := CanonicalCIDRBlock(ts.cidr) + if ts.expected != got { + t.Fatalf("CanonicalCIDRBlock(%q) should be: %q, got: %q", ts.cidr, ts.expected, got) + } + } +} diff --git a/aws/resource_aws_route_table.go b/aws/resource_aws_route_table.go index 3469147c836..1bc78fe4353 100644 --- a/aws/resource_aws_route_table.go +++ b/aws/resource_aws_route_table.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" + tfnet "github.com/terraform-providers/terraform-provider-aws/aws/internal/net" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter" "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" @@ -574,7 +575,7 @@ func resourceAwsRouteTableHash(v interface{}) int { } if v, ok := m["ipv6_cidr_block"]; ok { - buf.WriteString(fmt.Sprintf("%s-", canonicalCidrBlock(v.(string)))) + buf.WriteString(fmt.Sprintf("%s-", tfnet.CanonicalCIDRBlock(v.(string)))) } if v, ok := m["cidr_block"]; ok { diff --git a/aws/resource_aws_wafv2_ip_set.go b/aws/resource_aws_wafv2_ip_set.go index 77d653cb6b9..adc2ee53e42 100644 --- a/aws/resource_aws_wafv2_ip_set.go +++ b/aws/resource_aws_wafv2_ip_set.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" + tfnet "github.com/terraform-providers/terraform-provider-aws/aws/internal/net" ) func resourceAwsWafv2IPSet() *schema.Resource { @@ -50,7 +51,7 @@ func resourceAwsWafv2IPSet() *schema.Resource { for _, ov := range oldAddresses { hasAddress := false for _, nv := range newAddresses { - if cidrBlocksEqual(ov.(string), nv.(string)) { + if tfnet.CIDRBlocksEqual(ov.(string), nv.(string)) { hasAddress = true break } diff --git a/aws/validators.go b/aws/validators.go index f2b09daad56..f0b4b2a5aee 100644 --- a/aws/validators.go +++ b/aws/validators.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + tfnet "github.com/terraform-providers/terraform-provider-aws/aws/internal/net" ) const ( @@ -833,7 +834,7 @@ func validateCIDRBlock(cidr string) error { return fmt.Errorf("%q is not a valid CIDR block: %w", cidr, err) } - if !cidrBlocksEqual(cidr, ipnet.String()) { + if !tfnet.CIDRBlocksEqual(cidr, ipnet.String()) { return fmt.Errorf("%q is not a valid CIDR block; did you mean %q?", cidr, ipnet) } @@ -855,7 +856,7 @@ func validateIpv4CIDRBlock(cidr string) error { return fmt.Errorf("%q is not a valid IPv4 CIDR block", cidr) } - if !cidrBlocksEqual(cidr, ipnet.String()) { + if !tfnet.CIDRBlocksEqual(cidr, ipnet.String()) { return fmt.Errorf("%q is not a valid IPv4 CIDR block; did you mean %q?", cidr, ipnet) } @@ -877,43 +878,13 @@ func validateIpv6CIDRBlock(cidr string) error { return fmt.Errorf("%q is not a valid IPv6 CIDR block", cidr) } - if !cidrBlocksEqual(cidr, ipnet.String()) { + if !tfnet.CIDRBlocksEqual(cidr, ipnet.String()) { return fmt.Errorf("%q is not a valid IPv6 CIDR block; did you mean %q?", cidr, ipnet) } return nil } -// TODO Replace with tfnet.CIDRBlocksEqual. -// cidrBlocksEqual returns whether or not two CIDR blocks are equal: -// - Both CIDR blocks parse to an IP address and network -// - The string representation of the IP addresses are equal -// - The string representation of the networks are equal -// This function is especially useful for IPv6 CIDR blocks which have multiple valid representations. -func cidrBlocksEqual(cidr1, cidr2 string) bool { - ip1, ipnet1, err := net.ParseCIDR(cidr1) - if err != nil { - return false - } - ip2, ipnet2, err := net.ParseCIDR(cidr2) - if err != nil { - return false - } - - return ip2.String() == ip1.String() && ipnet2.String() == ipnet1.String() -} - -// canonicalCidrBlock returns the canonical representation of a CIDR block. -// This function is especially useful for hash functions for sets which include IPv6 CIDR blocks. -func canonicalCidrBlock(cidr string) string { - _, ipnet, err := net.ParseCIDR(cidr) - if err != nil { - return cidr - } - - return ipnet.String() -} - func validateHTTPMethod() schema.SchemaValidateFunc { return validation.StringInSlice([]string{ "ANY", diff --git a/aws/validators_test.go b/aws/validators_test.go index 835ad78bc0b..8ac81ed7b26 100644 --- a/aws/validators_test.go +++ b/aws/validators_test.go @@ -653,63 +653,6 @@ func TestValidateIpv6CIDRBlock(t *testing.T) { } } -func TestCidrBlocksEqual(t *testing.T) { - for _, ts := range []struct { - cidr1 string - cidr2 string - equal bool - }{ - {"10.2.2.0/24", "10.2.2.0/24", true}, - {"10.2.2.0/1234", "10.2.2.0/24", false}, - {"10.2.2.0/24", "10.2.2.0/1234", false}, - {"2001::/15", "2001::/15", true}, - {"::/0", "2001::/15", false}, - {"::/0", "::0/0", true}, - {"", "", false}, - } { - equal := cidrBlocksEqual(ts.cidr1, ts.cidr2) - if ts.equal != equal { - t.Fatalf("cidrBlocksEqual(%q, %q) should be: %t", ts.cidr1, ts.cidr2, ts.equal) - } - } -} -func TestCanonicalCidrBlock(t *testing.T) { - for _, ts := range []struct { - cidr string - expected string - }{ - {"10.2.2.0/24", "10.2.2.0/24"}, - {"10.2.2.5/24", "10.2.2.0/24"}, - {"::/0", "::/0"}, - {"::0/0", "::/0"}, - {"2001::/15", "2000::/15"}, - {"2001:db8::1/120", "2001:db8::/120"}, - {"", ""}, - } { - got := canonicalCidrBlock(ts.cidr) - if ts.expected != got { - t.Fatalf("canonicalCidrBlock(%q) should be: %q, got: %q", ts.cidr, ts.expected, got) - } - } -} - -func Test_canonicalCidrBlock(t *testing.T) { - for _, ts := range []struct { - cidr string - expected string - }{ - {"10.2.2.0/24", "10.2.2.0/24"}, - {"::/0", "::/0"}, - {"::0/0", "::/0"}, - {"", ""}, - } { - got := canonicalCidrBlock(ts.cidr) - if ts.expected != got { - t.Fatalf("canonicalCidrBlock(%q) should be: %q, got: %q", ts.cidr, ts.expected, got) - } - } -} - func TestValidateLogMetricFilterName(t *testing.T) { validNames := []string{ "YadaHereAndThere", From cfdcac206fa7d51c46047c234ae38dcb2c1e36d9 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 11 May 2021 16:52:04 -0400 Subject: [PATCH 05/31] r/aws_route_table: Tidy up Create, Read and Delete. --- aws/internal/service/ec2/errors.go | 6 +- aws/resource_aws_route.go | 2 +- aws/resource_aws_route_table.go | 413 +++++++++++++++++++++-------- 3 files changed, 308 insertions(+), 113 deletions(-) diff --git a/aws/internal/service/ec2/errors.go b/aws/internal/service/ec2/errors.go index 169575c71ab..f4f93f28cd1 100644 --- a/aws/internal/service/ec2/errors.go +++ b/aws/internal/service/ec2/errors.go @@ -9,8 +9,10 @@ import ( ) const ( - ErrCodeInvalidParameterException = "InvalidParameterException" - ErrCodeInvalidParameterValue = "InvalidParameterValue" + ErrCodeGatewayNotAttached = "Gateway.NotAttached" + ErrCodeInvalidAssociationIDNotFound = "InvalidAssociationID.NotFound" + ErrCodeInvalidParameterException = "InvalidParameterException" + ErrCodeInvalidParameterValue = "InvalidParameterValue" ) const ( diff --git a/aws/resource_aws_route.go b/aws/resource_aws_route.go index 6ec5001ce26..3542ba99180 100644 --- a/aws/resource_aws_route.go +++ b/aws/resource_aws_route.go @@ -530,5 +530,5 @@ func createRoute(conn *ec2.EC2, input *ec2.CreateRouteInput, timeout time.Durati _, err = conn.CreateRoute(input) } - return nil + return err } diff --git a/aws/resource_aws_route_table.go b/aws/resource_aws_route_table.go index 1bc78fe4353..bdd398df497 100644 --- a/aws/resource_aws_route_table.go +++ b/aws/resource_aws_route_table.go @@ -10,12 +10,14 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" tfnet "github.com/terraform-providers/terraform-provider-aws/aws/internal/net" + tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter" "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" @@ -32,12 +34,12 @@ var routeTableValidTargets = []string{ "egress_only_gateway_id", "gateway_id", "instance_id", - "nat_gateway_id", "local_gateway_id", + "nat_gateway_id", + "network_interface_id", "transit_gateway_id", "vpc_endpoint_id", "vpc_peering_connection_id", - "network_interface_id", } func resourceAwsRouteTable() *schema.Resource { @@ -55,10 +57,12 @@ func resourceAwsRouteTable() *schema.Resource { Type: schema.TypeString, Computed: true, }, + "owner_id": { Type: schema.TypeString, Computed: true, }, + "propagating_vgws": { Type: schema.TypeSet, Optional: true, @@ -66,6 +70,7 @@ func resourceAwsRouteTable() *schema.Resource { Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, }, + "route": { Type: schema.TypeSet, Computed: true, @@ -144,8 +149,10 @@ func resourceAwsRouteTable() *schema.Resource { }, Set: resourceAwsRouteTableHash, }, + "tags": tagsSchema(), "tags_all": tagsSchemaComputed(), + "vpc_id": { Type: schema.TypeString, Required: true, @@ -162,30 +169,68 @@ func resourceAwsRouteTableCreate(d *schema.ResourceData, meta interface{}) error defaultTagsConfig := meta.(*AWSClient).DefaultTagsConfig tags := defaultTagsConfig.MergeTags(keyvaluetags.New(d.Get("tags").(map[string]interface{}))) - // Create the routing table - createOpts := &ec2.CreateRouteTableInput{ + input := &ec2.CreateRouteTableInput{ VpcId: aws.String(d.Get("vpc_id").(string)), TagSpecifications: ec2TagSpecificationsFromKeyValueTags(tags, ec2.ResourceTypeRouteTable), } - log.Printf("[DEBUG] RouteTable create config: %#v", createOpts) - resp, err := conn.CreateRouteTable(createOpts) + log.Printf("[DEBUG] Creating Route Table: %s", input) + output, err := conn.CreateRouteTable(input) + if err != nil { - return fmt.Errorf("error creating route table: %w", err) + return fmt.Errorf("error creating Route Table: %w", err) } - // Get the ID and store it - rt := resp.RouteTable - d.SetId(aws.StringValue(rt.RouteTableId)) - log.Printf("[INFO] Route Table ID: %s", d.Id()) + d.SetId(aws.StringValue(output.RouteTable.RouteTableId)) - // Wait for the route table to become available - log.Printf("[DEBUG] Waiting for route table (%s) to become available", d.Id()) if _, err := waiter.RouteTableReady(conn, d.Id()); err != nil { - return fmt.Errorf("error waiting for route table (%s) to become available: %w", d.Id(), err) + return fmt.Errorf("error waiting for Route Table (%s) to become available: %w", d.Id(), err) } - return resourceAwsRouteTableUpdate(d, meta) + if v, ok := d.GetOk("propagating_vgws"); ok && v.(*schema.Set).Len() > 0 { + for _, v := range v.(*schema.Set).List() { + v := v.(string) + + log.Printf("[DEBUG] Enabling Route Table (%s) VPN Gateway (%s) route propagation", d.Id(), v) + err = enableVgwRoutePropagation(conn, d.Id(), v, waiter.PropagationTimeout) + + if err != nil { + return fmt.Errorf("error enabling Route Table (%s) VPN Gateway (%s) route propagation: %w", d.Id(), v, err) + } + } + } + + if v, ok := d.GetOk("route"); ok && v.(*schema.Set).Len() > 0 { + for _, v := range v.(*schema.Set).List() { + v := v.(map[string]interface{}) + + if err := validateNestedExactlyOneOf(v, routeTableValidDestinations); err != nil { + return fmt.Errorf("error creating route: %w", err) + } + if err := validateNestedExactlyOneOf(v, routeTableValidTargets); err != nil { + return fmt.Errorf("error creating route: %w", err) + } + + _, destination := routeTableRouteDestinationAttribute(v) + + input := expandEc2CreateRouteInput(v) + + if input == nil { + continue + } + + input.RouteTableId = aws.String(d.Id()) + + log.Printf("[DEBUG] Creating Route: %s", input) + err = createRoute(conn, input, waiter.PropagationTimeout) + + if err != nil { + return fmt.Errorf("error creating Route for Route Table (%s) with destination (%s): %w", d.Id(), destination, err) + } + } + } + + return resourceAwsRouteTableRead(d, meta) } func resourceAwsRouteTableRead(d *schema.ResourceData, meta interface{}) error { @@ -196,88 +241,28 @@ func resourceAwsRouteTableRead(d *schema.ResourceData, meta interface{}) error { routeTable, err := finder.RouteTableByID(conn, d.Id()) if !d.IsNewResource() && tfresource.NotFound(err) { - log.Printf("[WARN] Route table (%s) not found, removing from state", d.Id()) + log.Printf("[WARN] Route Table (%s) not found, removing from state", d.Id()) d.SetId("") return nil } if err != nil { - return fmt.Errorf("error reading route table (%s): %w", d.Id(), err) + return fmt.Errorf("error reading Route Table (%s): %w", d.Id(), err) } d.Set("vpc_id", routeTable.VpcId) propagatingVGWs := make([]string, 0, len(routeTable.PropagatingVgws)) - for _, vgw := range routeTable.PropagatingVgws { - propagatingVGWs = append(propagatingVGWs, aws.StringValue(vgw.GatewayId)) + for _, v := range routeTable.PropagatingVgws { + propagatingVGWs = append(propagatingVGWs, aws.StringValue(v.GatewayId)) + } + if err := d.Set("propagating_vgws", propagatingVGWs); err != nil { + return fmt.Errorf("error setting propagating_vgws: %w", err) } - d.Set("propagating_vgws", propagatingVGWs) - - // Create an empty schema.Set to hold all routes - route := &schema.Set{F: resourceAwsRouteTableHash} - - // Loop through the routes and add them to the set - for _, r := range routeTable.Routes { - if aws.StringValue(r.GatewayId) == "local" { - continue - } - - if aws.StringValue(r.Origin) == ec2.RouteOriginEnableVgwRoutePropagation { - continue - } - - if r.DestinationPrefixListId != nil && strings.HasPrefix(aws.StringValue(r.GatewayId), "vpce-") { - // Skipping because VPC endpoint routes are handled separately - // See aws_vpc_endpoint - continue - } - - m := make(map[string]interface{}) - - if r.DestinationCidrBlock != nil { - m["cidr_block"] = aws.StringValue(r.DestinationCidrBlock) - } - if r.DestinationIpv6CidrBlock != nil { - m["ipv6_cidr_block"] = aws.StringValue(r.DestinationIpv6CidrBlock) - } - if r.DestinationPrefixListId != nil { - m["destination_prefix_list_id"] = aws.StringValue(r.DestinationPrefixListId) - } - if r.CarrierGatewayId != nil { - m["carrier_gateway_id"] = aws.StringValue(r.CarrierGatewayId) - } - if r.EgressOnlyInternetGatewayId != nil { - m["egress_only_gateway_id"] = aws.StringValue(r.EgressOnlyInternetGatewayId) - } - if r.GatewayId != nil { - if strings.HasPrefix(aws.StringValue(r.GatewayId), "vpce-") { - m["vpc_endpoint_id"] = aws.StringValue(r.GatewayId) - } else { - m["gateway_id"] = aws.StringValue(r.GatewayId) - } - } - if r.NatGatewayId != nil { - m["nat_gateway_id"] = aws.StringValue(r.NatGatewayId) - } - if r.LocalGatewayId != nil { - m["local_gateway_id"] = aws.StringValue(r.LocalGatewayId) - } - if r.InstanceId != nil { - m["instance_id"] = aws.StringValue(r.InstanceId) - } - if r.TransitGatewayId != nil { - m["transit_gateway_id"] = aws.StringValue(r.TransitGatewayId) - } - if r.VpcPeeringConnectionId != nil { - m["vpc_peering_connection_id"] = aws.StringValue(r.VpcPeeringConnectionId) - } - if r.NetworkInterfaceId != nil { - m["network_interface_id"] = aws.StringValue(r.NetworkInterfaceId) - } - route.Add(m) + if err := d.Set("route", flattenEc2Routes(routeTable.Routes)); err != nil { + return fmt.Errorf("error setting route: %w", err) } - d.Set("route", route) // Tags tags := keyvaluetags.Ec2KeyValueTags(routeTable.Tags).IgnoreAws().IgnoreConfig(ignoreTagsConfig) @@ -518,50 +503,38 @@ func resourceAwsRouteTableUpdate(d *schema.ResourceData, meta interface{}) error func resourceAwsRouteTableDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - // First request the routing table since we'll have to disassociate - // all the subnets first. - rt, err := waiter.RouteTableReady(conn, d.Id()) + routeTable, err := finder.RouteTableByID(conn, d.Id()) if err != nil { - return fmt.Errorf("error getting route table (%s) prior to disassociating associations: %w", d.Id(), err) + return fmt.Errorf("error reading Route Table (%s): %w", d.Id(), err) } // Do all the disassociations - for _, a := range rt.Associations { - log.Printf("[INFO] Disassociating association: %s", aws.StringValue(a.RouteTableAssociationId)) - _, err := conn.DisassociateRouteTable(&ec2.DisassociateRouteTableInput{ - AssociationId: a.RouteTableAssociationId, - }) - if err != nil { - // First check if the association ID is not found. If this - // is the case, then it was already disassociated somehow, - // and that is okay. - if isAWSErr(err, "InvalidAssociationID.NotFound", "") { - err = nil - } - } - if err != nil { - return err + for _, v := range routeTable.Associations { + v := aws.StringValue(v.RouteTableAssociationId) + + if err := disassociateRouteTable(conn, v); err != nil { + return fmt.Errorf("error disassociating Route Table (%s) %s: %w", d.Id(), v, err) } } - // Delete the route table log.Printf("[INFO] Deleting Route Table: %s", d.Id()) _, err = conn.DeleteRouteTable(&ec2.DeleteRouteTableInput{ RouteTableId: aws.String(d.Id()), }) - if err != nil { - if isAWSErr(err, "InvalidRouteTableID.NotFound", "") { - return nil - } - return fmt.Errorf("error deleting route table: %w", err) + if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidRouteTableIDNotFound) { + return nil + } + + if err != nil { + return fmt.Errorf("error deleting Route Table (%s): %w", d.Id(), err) } // Wait for the route table to really destroy log.Printf("[DEBUG] Waiting for route table (%s) deletion", d.Id()) if _, err := waiter.RouteTableDeleted(conn, d.Id()); err != nil { - return fmt.Errorf("error waiting for route table (%s) deletion: %w", d.Id(), err) + return fmt.Errorf("error waiting for Route Table (%s) deletion: %w", d.Id(), err) } return nil @@ -632,3 +605,223 @@ func resourceAwsRouteTableHash(v interface{}) int { return hashcode.String(buf.String()) } + +func disassociateRouteTable(conn *ec2.EC2, associationID string) error { + _, err := conn.DisassociateRouteTable(&ec2.DisassociateRouteTableInput{ + AssociationId: aws.String(associationID), + }) + + if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidAssociationIDNotFound) { + return nil + } + + return nil +} + +// enableVgwRoutePropagation attempts to enable VGW route propagation. +// The specified eventual consistency timeout is respected. +// Any error is returned. +func enableVgwRoutePropagation(conn *ec2.EC2, routeTableID, gatewayID string, timeout time.Duration) error { + input := &ec2.EnableVgwRoutePropagationInput{ + GatewayId: aws.String(gatewayID), + RouteTableId: aws.String(routeTableID), + } + + err := resource.Retry(timeout, func() *resource.RetryError { + _, err := conn.EnableVgwRoutePropagation(input) + + if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeGatewayNotAttached) { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + + if tfresource.TimedOut(err) { + _, err = conn.EnableVgwRoutePropagation(input) + } + + return err +} + +func expandEc2CreateRouteInput(tfMap map[string]interface{}) *ec2.CreateRouteInput { + if tfMap == nil { + return nil + } + + apiObject := &ec2.CreateRouteInput{} + + if v, ok := tfMap["cidr_block"].(string); ok && v != "" { + apiObject.DestinationCidrBlock = aws.String(v) + } + + if v, ok := tfMap["ipv6_cidr_block"].(string); ok && v != "" { + apiObject.DestinationIpv6CidrBlock = aws.String(v) + } + + if v, ok := tfMap["destination_prefix_list_id"].(string); ok && v != "" { + apiObject.DestinationPrefixListId = aws.String(v) + } + + if v, ok := tfMap["carrier_gateway_id"].(string); ok && v != "" { + apiObject.CarrierGatewayId = aws.String(v) + } + + if v, ok := tfMap["egress_only_gateway_id"].(string); ok && v != "" { + apiObject.EgressOnlyInternetGatewayId = aws.String(v) + } + + if v, ok := tfMap["gateway_id"].(string); ok && v != "" { + apiObject.GatewayId = aws.String(v) + } + + if v, ok := tfMap["instance_id"].(string); ok && v != "" { + apiObject.InstanceId = aws.String(v) + } + + if v, ok := tfMap["local_gateway_id"].(string); ok && v != "" { + apiObject.LocalGatewayId = aws.String(v) + } + + if v, ok := tfMap["nat_gateway_id"].(string); ok && v != "" { + apiObject.NatGatewayId = aws.String(v) + } + + if v, ok := tfMap["network_interface_id"].(string); ok && v != "" { + apiObject.NetworkInterfaceId = aws.String(v) + } + + if v, ok := tfMap["transit_gateway_id"].(string); ok && v != "" { + apiObject.TransitGatewayId = aws.String(v) + } + + if v, ok := tfMap["vpc_endpoint_id"].(string); ok && v != "" { + apiObject.VpcEndpointId = aws.String(v) + } + + if v, ok := tfMap["vpc_peering_connection_id"].(string); ok && v != "" { + apiObject.VpcPeeringConnectionId = aws.String(v) + } + + return apiObject +} + +func flattenEc2Route(apiObject *ec2.Route) map[string]interface{} { + if apiObject == nil { + return nil + } + + tfMap := map[string]interface{}{} + + if v := apiObject.DestinationCidrBlock; v != nil { + tfMap["cidr_block"] = aws.StringValue(v) + } + + if v := apiObject.DestinationIpv6CidrBlock; v != nil { + tfMap["ipv6_cidr_block"] = aws.StringValue(v) + } + + if v := apiObject.DestinationPrefixListId; v != nil { + tfMap["destination_prefix_list_id"] = aws.StringValue(v) + } + + if v := apiObject.CarrierGatewayId; v != nil { + tfMap["carrier_gateway_id"] = aws.StringValue(v) + } + + if v := apiObject.EgressOnlyInternetGatewayId; v != nil { + tfMap["egress_only_gateway_id"] = aws.StringValue(v) + } + + if v := apiObject.GatewayId; v != nil { + if strings.HasPrefix(aws.StringValue(v), "vpce-") { + tfMap["vpc_endpoint_id"] = aws.StringValue(v) + } else { + tfMap["gateway_id"] = aws.StringValue(v) + } + } + + if v := apiObject.InstanceId; v != nil { + tfMap["instance_id"] = aws.StringValue(v) + } + + if v := apiObject.LocalGatewayId; v != nil { + tfMap["local_gateway_id"] = aws.StringValue(v) + } + + if v := apiObject.NetworkInterfaceId; v != nil { + tfMap["nat_gateway_id"] = aws.StringValue(v) + } + + if v := apiObject.TransitGatewayId; v != nil { + tfMap["network_interface_id"] = aws.StringValue(v) + } + + if v := apiObject.TransitGatewayId; v != nil { + tfMap["transit_gateway_id"] = aws.StringValue(v) + } + + if v := apiObject.VpcPeeringConnectionId; v != nil { + tfMap["vpc_peering_connection_id"] = aws.StringValue(v) + } + + return tfMap +} + +func flattenEc2Routes(apiObjects []*ec2.Route) []interface{} { + if len(apiObjects) == 0 { + return nil + } + + var tfList []interface{} + + for _, apiObject := range apiObjects { + if apiObject == nil { + continue + } + + if aws.StringValue(apiObject.GatewayId) == "local" { + continue + } + + if aws.StringValue(apiObject.Origin) == ec2.RouteOriginEnableVgwRoutePropagation { + continue + } + + if apiObject.DestinationPrefixListId != nil && strings.HasPrefix(aws.StringValue(apiObject.GatewayId), "vpce-") { + // Skipping because VPC endpoint routes are handled separately + // See aws_vpc_endpoint + continue + } + + tfList = append(tfList, flattenEc2Route(apiObject)) + } + + return tfList +} + +// routeTableRouteDestinationAttribute returns the attribute key and value of the route table route's destination. +func routeTableRouteDestinationAttribute(m map[string]interface{}) (string, string) { + for _, key := range routeTableValidDestinations { + if v, ok := m[key].(string); ok && v != "" { + return key, v + } + } + + return "", "" +} + +// routeTableRouteTargetAttribute returns the attribute key and value of the route table route's target. +func routeTableRouteTargetAttribute(m map[string]interface{}) (string, string) { + for _, key := range routeTableValidTargets { + if v, ok := m[key].(string); ok && v != "" { + return key, v + } + } + + return "", "" +} From 87464d3b155efa55e5061981ac739cb19458c112 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 12 May 2021 09:35:35 -0400 Subject: [PATCH 06/31] r/aws_route_table: Simplify route table disassociation. --- aws/resource_aws_route_table.go | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/aws/resource_aws_route_table.go b/aws/resource_aws_route_table.go index bdd398df497..09ac9bcadbe 100644 --- a/aws/resource_aws_route_table.go +++ b/aws/resource_aws_route_table.go @@ -513,8 +513,12 @@ func resourceAwsRouteTableDelete(d *schema.ResourceData, meta interface{}) error for _, v := range routeTable.Associations { v := aws.StringValue(v.RouteTableAssociationId) - if err := disassociateRouteTable(conn, v); err != nil { - return fmt.Errorf("error disassociating Route Table (%s) %s: %w", d.Id(), v, err) + r := resourceAwsRouteTableAssociation() + d := r.Data(nil) + d.SetId(v) + + if err := r.Delete(d, meta); err != nil { + return err } } @@ -606,18 +610,6 @@ func resourceAwsRouteTableHash(v interface{}) int { return hashcode.String(buf.String()) } -func disassociateRouteTable(conn *ec2.EC2, associationID string) error { - _, err := conn.DisassociateRouteTable(&ec2.DisassociateRouteTableInput{ - AssociationId: aws.String(associationID), - }) - - if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidAssociationIDNotFound) { - return nil - } - - return nil -} - // enableVgwRoutePropagation attempts to enable VGW route propagation. // The specified eventual consistency timeout is respected. // Any error is returned. From c1d877de77353f1f655d5301447c11cca19e5aee Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 7 Jun 2021 17:22:49 -0400 Subject: [PATCH 07/31] Add and use 'tfresource.Delete'. --- aws/aws_sweeper_test.go | 3 +- aws/internal/tfresource/errors_test.go | 7 +- aws/internal/tfresource/resource.go | 34 +++++++++ aws/internal/tfresource/resource_test.go | 88 ++++++++++++++++++++++++ aws/provider_test.go | 26 +------ aws/resource_aws_route_table.go | 4 +- 6 files changed, 132 insertions(+), 30 deletions(-) create mode 100644 aws/internal/tfresource/resource.go create mode 100644 aws/internal/tfresource/resource_test.go diff --git a/aws/aws_sweeper_test.go b/aws/aws_sweeper_test.go index a16da090e85..4fda020dd9e 100644 --- a/aws/aws_sweeper_test.go +++ b/aws/aws_sweeper_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/terraform-providers/terraform-provider-aws/aws/internal/envvar" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) // sweeperAwsClients is a shared cache of regional AWSClient @@ -77,7 +78,7 @@ func testSweepResourceOrchestrator(sweepResources []*testSweepResource) error { sweepResource := sweepResource g.Go(func() error { - return testAccDeleteResource(sweepResource.resource, sweepResource.d, sweepResource.meta) + return tfresource.Delete(sweepResource.resource, sweepResource.d, sweepResource.meta) }) } diff --git a/aws/internal/tfresource/errors_test.go b/aws/internal/tfresource/errors_test.go index 175ef5150cd..da75f2cec9e 100644 --- a/aws/internal/tfresource/errors_test.go +++ b/aws/internal/tfresource/errors_test.go @@ -1,4 +1,4 @@ -package tfresource +package tfresource_test import ( "errors" @@ -6,6 +6,7 @@ import ( "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func TestNotFound(t *testing.T) { @@ -40,7 +41,7 @@ func TestNotFound(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { - got := NotFound(testCase.Err) + got := tfresource.NotFound(testCase.Err) if got != testCase.Expected { t.Errorf("got %t, expected %t", got, testCase.Expected) @@ -88,7 +89,7 @@ func TestTimedOut(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { - got := TimedOut(testCase.Err) + got := tfresource.TimedOut(testCase.Err) if got != testCase.Expected { t.Errorf("got %t, expected %t", got, testCase.Expected) diff --git a/aws/internal/tfresource/resource.go b/aws/internal/tfresource/resource.go new file mode 100644 index 00000000000..8b8a765c05e --- /dev/null +++ b/aws/internal/tfresource/resource.go @@ -0,0 +1,34 @@ +package tfresource + +import ( + "context" + "errors" + + multierror "github.com/hashicorp/go-multierror" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +// Delete calls the specified resource's delete handler. +func Delete(resource *schema.Resource, d *schema.ResourceData, meta interface{}) error { + if resource.DeleteContext != nil || resource.DeleteWithoutTimeout != nil { + var diags diag.Diagnostics + var err *multierror.Error + + if resource.DeleteContext != nil { + diags = resource.DeleteContext(context.Background(), d, meta) + } else { + diags = resource.DeleteWithoutTimeout(context.Background(), d, meta) + } + + for i := range diags { + if diags[i].Severity == diag.Error { + err = multierror.Append(err, errors.New(diags[i].Summary)) + } + } + + return err.ErrorOrNil() + } + + return resource.Delete(d, meta) +} diff --git a/aws/internal/tfresource/resource_test.go b/aws/internal/tfresource/resource_test.go new file mode 100644 index 00000000000..266691e618f --- /dev/null +++ b/aws/internal/tfresource/resource_test.go @@ -0,0 +1,88 @@ +package tfresource_test + +import ( + "context" + "errors" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" +) + +func TestDeleteNoError(t *testing.T) { + theError := errors.New("fail") + + testCases := []struct { + Name string + Resource *schema.Resource + ExpectError bool + }{ + { + Name: "no error Delete function", + Resource: &schema.Resource{ + Delete: func(rd *schema.ResourceData, i interface{}) error { + return nil + }, + }, + }, + { + Name: "no error DeleteContext function", + Resource: &schema.Resource{ + DeleteContext: func(c context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { + return nil + }, + }, + }, + { + Name: "no error DeleteWithoutTimeout function", + Resource: &schema.Resource{ + DeleteWithoutTimeout: func(c context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { + return nil + }, + }, + }, + { + Name: "error Delete function", + Resource: &schema.Resource{ + Delete: func(rd *schema.ResourceData, i interface{}) error { + return theError + }, + }, + ExpectError: true, + }, + { + Name: "error DeleteContext function", + Resource: &schema.Resource{ + DeleteContext: func(c context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { + return diag.FromErr(theError) + }, + }, + ExpectError: true, + }, + { + Name: "error DeleteWithoutTimeout function", + Resource: &schema.Resource{ + DeleteWithoutTimeout: func(c context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { + return diag.FromErr(theError) + }, + }, + ExpectError: true, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Name, func(t *testing.T) { + r := testCase.Resource + d := r.Data(nil) + + err := tfresource.Delete(r, d, nil) + + if testCase.ExpectError && err == nil { + t.Fatal("expected error") + } else if !testCase.ExpectError && err != nil { + t.Fatalf("unexpected error: %s", err) + } + }) + } +} diff --git a/aws/provider_test.go b/aws/provider_test.go index de8c5142252..38998ba6ce9 100644 --- a/aws/provider_test.go +++ b/aws/provider_test.go @@ -18,7 +18,6 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/iam" "github.com/aws/aws-sdk-go/service/organizations" - "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" @@ -26,6 +25,7 @@ import ( "github.com/terraform-providers/terraform-provider-aws/aws/internal/envvar" organizationsfinder "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/organizations/finder" stsfinder "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/sts/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) const ( @@ -1035,28 +1035,6 @@ func testAccAwsRegionProviderFunc(region string, providers *[]*schema.Provider) } } -func testAccDeleteResource(resource *schema.Resource, d *schema.ResourceData, meta interface{}) error { - if resource.DeleteContext != nil || resource.DeleteWithoutTimeout != nil { - var diags diag.Diagnostics - - if resource.DeleteContext != nil { - diags = resource.DeleteContext(context.Background(), d, meta) - } else { - diags = resource.DeleteWithoutTimeout(context.Background(), d, meta) - } - - for i := range diags { - if diags[i].Severity == diag.Error { - return fmt.Errorf("error deleting resource: %s", diags[i].Summary) - } - } - - return nil - } - - return resource.Delete(d, meta) -} - func testAccCheckResourceDisappears(provider *schema.Provider, resource *schema.Resource, resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { resourceState, ok := s.RootModule().Resources[resourceName] @@ -1069,7 +1047,7 @@ func testAccCheckResourceDisappears(provider *schema.Provider, resource *schema. return fmt.Errorf("resource ID missing: %s", resourceName) } - return testAccDeleteResource(resource, resource.Data(resourceState.Primary), provider.Meta()) + return tfresource.Delete(resource, resource.Data(resourceState.Primary), provider.Meta()) } } diff --git a/aws/resource_aws_route_table.go b/aws/resource_aws_route_table.go index 09ac9bcadbe..f11f816fb7b 100644 --- a/aws/resource_aws_route_table.go +++ b/aws/resource_aws_route_table.go @@ -517,7 +517,7 @@ func resourceAwsRouteTableDelete(d *schema.ResourceData, meta interface{}) error d := r.Data(nil) d.SetId(v) - if err := r.Delete(d, meta); err != nil { + if err := tfresource.Delete(r, d, meta); err != nil { return err } } @@ -538,7 +538,7 @@ func resourceAwsRouteTableDelete(d *schema.ResourceData, meta interface{}) error // Wait for the route table to really destroy log.Printf("[DEBUG] Waiting for route table (%s) deletion", d.Id()) if _, err := waiter.RouteTableDeleted(conn, d.Id()); err != nil { - return fmt.Errorf("error waiting for Route Table (%s) deletion: %w", d.Id(), err) + return fmt.Errorf("error waiting for Route Table (%s) to delete: %w", d.Id(), err) } return nil From 34659d066389114e2f31e19a242bb17f4cb0f789 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 11 Jun 2021 14:43:16 -0400 Subject: [PATCH 08/31] Add 'tfresource.RetryWhenAwsErrCodeEquals'. --- aws/internal/tfresource/retry.go | 41 +++++++++++++++ aws/internal/tfresource/retry_test.go | 73 +++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 aws/internal/tfresource/retry.go create mode 100644 aws/internal/tfresource/retry_test.go diff --git a/aws/internal/tfresource/retry.go b/aws/internal/tfresource/retry.go new file mode 100644 index 00000000000..517a6b21933 --- /dev/null +++ b/aws/internal/tfresource/retry.go @@ -0,0 +1,41 @@ +package tfresource + +import ( + "time" + + "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +// RetryWhenAwsErrCodeEquals retries the specified function when it returns one of the specified AWS error code. +func RetryWhenAwsErrCodeEquals(timeout time.Duration, f func() (interface{}, error), codes ...string) (interface{}, error) { + var output interface{} + + err := resource.Retry(timeout, func() *resource.RetryError { + var err error + + output, err = f() + + for _, code := range codes { + if tfawserr.ErrCodeEquals(err, code) { + return resource.RetryableError(err) + } + } + + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + + if TimedOut(err) { + output, err = f() + } + + if err != nil { + return nil, err + } + + return output, nil +} diff --git a/aws/internal/tfresource/retry_test.go b/aws/internal/tfresource/retry_test.go new file mode 100644 index 00000000000..8a396b9e3de --- /dev/null +++ b/aws/internal/tfresource/retry_test.go @@ -0,0 +1,73 @@ +package tfresource_test + +import ( + "errors" + "sync/atomic" + "testing" + "time" + + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" +) + +func TestRetryWhenAwsErrCodeEquals(t *testing.T) { + var retryCount int32 + + testCases := []struct { + Name string + F func() (interface{}, error) + ExpectError bool + }{ + { + Name: "no error", + F: func() (interface{}, error) { + return nil, nil + }, + }, + { + Name: "non-retryable other error", + F: func() (interface{}, error) { + return nil, errors.New("TestCode") + }, + ExpectError: true, + }, + { + Name: "non-retryable AWS error", + F: func() (interface{}, error) { + return nil, awserr.New("Testing", "Testing", nil) + }, + ExpectError: true, + }, + { + Name: "retryable AWS error timeout", + F: func() (interface{}, error) { + return nil, awserr.New("TestCode1", "TestMessage", nil) + }, + ExpectError: true, + }, + { + Name: "retryable AWS error success", + F: func() (interface{}, error) { + if atomic.CompareAndSwapInt32(&retryCount, 0, 1) { + return nil, awserr.New("TestCode2", "TestMessage", nil) + } + + return nil, nil + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Name, func(t *testing.T) { + retryCount = 0 + + _, err := tfresource.RetryWhenAwsErrCodeEquals(5*time.Second, testCase.F, "TestCode1", "TestCode2") + + if testCase.ExpectError && err == nil { + t.Fatal("expected error") + } else if !testCase.ExpectError && err != nil { + t.Fatalf("unexpected error: %s", err) + } + }) + } +} From 39a1614d8a3a541ef7efcfd5b81ca1def0b685a1 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 11 Jun 2021 15:57:43 -0400 Subject: [PATCH 09/31] Add and use 'waiter.RouteReady'. --- aws/internal/service/ec2/waiter/status.go | 20 ++++++ aws/internal/service/ec2/waiter/waiter.go | 21 +++++- aws/resource_aws_route.go | 78 +++++++---------------- aws/resource_aws_route_table.go | 33 ++++++++-- 4 files changed, 93 insertions(+), 59 deletions(-) diff --git a/aws/internal/service/ec2/waiter/status.go b/aws/internal/service/ec2/waiter/status.go index 6e444caa179..70f884f1ad5 100644 --- a/aws/internal/service/ec2/waiter/status.go +++ b/aws/internal/service/ec2/waiter/status.go @@ -246,6 +246,26 @@ func InstanceIamInstanceProfile(conn *ec2.EC2, id string) resource.StateRefreshF } } +const ( + RouteStatusReady = "ready" +) + +func RouteStatus(conn *ec2.EC2, routeFinder finder.RouteFinder, routeTableID, destination string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + output, err := routeFinder(conn, routeTableID, destination) + + if tfresource.NotFound(err) { + return nil, "", nil + } + + if err != nil { + return nil, "", err + } + + return output, RouteStatusReady, nil + } +} + const ( RouteTableStatusReady = "ready" ) diff --git a/aws/internal/service/ec2/waiter/waiter.go b/aws/internal/service/ec2/waiter/waiter.go index 985f0a89194..3151cb8a4c1 100644 --- a/aws/internal/service/ec2/waiter/waiter.go +++ b/aws/internal/service/ec2/waiter/waiter.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" ) const ( @@ -257,10 +258,28 @@ const ( NetworkAclEntryPropagationTimeout = 5 * time.Minute ) +func RouteReady(conn *ec2.EC2, routeFinder finder.RouteFinder, routeTableID, destination string) (*ec2.Route, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{}, + Target: []string{RouteStatusReady}, + Refresh: RouteStatus(conn, routeFinder, routeTableID, destination), + Timeout: PropagationTimeout, + ContinuousTargetOccurence: 2, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*ec2.Route); ok { + return output, err + } + + return nil, err +} + const ( RouteTableReadyTimeout = 10 * time.Minute RouteTableDeletedTimeout = 5 * time.Minute - RouteTableUpdateTimeout = 5 * time.Minute + RouteTableUpdatedTimeout = 5 * time.Minute RouteTableNotFoundChecks = 40 ) diff --git a/aws/resource_aws_route.go b/aws/resource_aws_route.go index 3542ba99180..e6740315dcb 100644 --- a/aws/resource_aws_route.go +++ b/aws/resource_aws_route.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter" "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) @@ -230,36 +231,23 @@ func resourceAwsRouteCreate(d *schema.ResourceData, meta interface{}) error { } log.Printf("[DEBUG] Creating Route: %s", input) - err = createRoute(conn, input, d.Timeout(schema.TimeoutCreate)) + _, err = tfresource.RetryWhenAwsErrCodeEquals( + d.Timeout(schema.TimeoutCreate), + func() (interface{}, error) { + return conn.CreateRoute(input) + }, + tfec2.ErrCodeInvalidParameterException, + tfec2.ErrCodeInvalidTransitGatewayIDNotFound, + ) if err != nil { return fmt.Errorf("error creating Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) } - err = resource.Retry(d.Timeout(schema.TimeoutCreate), func() *resource.RetryError { - _, err = routeFinder(conn, routeTableID, destination) - - if err != nil { - return resource.RetryableError(err) - } - - if tfresource.NotFound(err) { - return resource.RetryableError(fmt.Errorf("route not found")) - } - - return nil - }) - - if tfresource.TimedOut(err) { - _, err = routeFinder(conn, routeTableID, destination) - } + _, err = waiter.RouteReady(conn, routeFinder, routeTableID, destination) if err != nil { - return fmt.Errorf("error reading Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) - } - - if tfresource.NotFound(err) { - return fmt.Errorf("route in Route Table (%s) with destination (%s) not found", routeTableID, destination) + return fmt.Errorf("error waiting for Route in Route Table (%s) with destination (%s) to become available: %w", routeTableID, destination, err) } d.SetId(tfec2.RouteCreateID(routeTableID, destination)) @@ -349,13 +337,18 @@ func resourceAwsRouteUpdate(d *schema.ResourceData, meta interface{}) error { RouteTableId: aws.String(routeTableID), } + var routeFinder finder.RouteFinder + switch destination := aws.String(destination); destinationAttributeKey { case "destination_cidr_block": input.DestinationCidrBlock = destination + routeFinder = finder.RouteByIPv4Destination case "destination_ipv6_cidr_block": input.DestinationIpv6CidrBlock = destination + routeFinder = finder.RouteByIPv6Destination case "destination_prefix_list_id": input.DestinationPrefixListId = destination + routeFinder = finder.RouteByPrefixListIDDestination default: return fmt.Errorf("error updating Route: unexpected route destination attribute: %q", destinationAttributeKey) } @@ -392,6 +385,12 @@ func resourceAwsRouteUpdate(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error updating Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) } + _, err = waiter.RouteReady(conn, routeFinder, routeTableID, destination) + + if err != nil { + return fmt.Errorf("error waiting for Route in Route Table (%s) with destination (%s) to become available: %w", routeTableID, destination, err) + } + return resourceAwsRouteRead(d, meta) } @@ -420,9 +419,10 @@ func resourceAwsRouteDelete(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error deleting Route: unexpected route destination attribute: %q", destinationAttributeKey) } + log.Printf("[DEBUG] Deleting Route (%s)", d.Id()) err = resource.Retry(d.Timeout(schema.TimeoutDelete), func() *resource.RetryError { - log.Printf("[DEBUG] Deleting Route (%s)", d.Id()) _, err = conn.DeleteRoute(input) + if err == nil { return nil } @@ -444,7 +444,6 @@ func resourceAwsRouteDelete(d *schema.ResourceData, meta interface{}) error { }) if tfresource.TimedOut(err) { - log.Printf("[DEBUG] Deleting Route (%s)", d.Id()) _, err = conn.DeleteRoute(input) } @@ -503,32 +502,3 @@ func routeTargetAttribute(d *schema.ResourceData) (string, string, error) { return "", "", fmt.Errorf("route target attribute not specified") } - -// createRoute attempts to create a route. -// The specified eventual consistency timeout is respected. -// Any error is returned. -func createRoute(conn *ec2.EC2, input *ec2.CreateRouteInput, timeout time.Duration) error { - err := resource.Retry(timeout, func() *resource.RetryError { - _, err := conn.CreateRoute(input) - - if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidParameterException) { - return resource.RetryableError(err) - } - - if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidTransitGatewayIDNotFound) { - return resource.RetryableError(err) - } - - if err != nil { - return resource.NonRetryableError(err) - } - - return nil - }) - - if tfresource.TimedOut(err) { - _, err = conn.CreateRoute(input) - } - - return err -} diff --git a/aws/resource_aws_route_table.go b/aws/resource_aws_route_table.go index f11f816fb7b..35190c082d9 100644 --- a/aws/resource_aws_route_table.go +++ b/aws/resource_aws_route_table.go @@ -211,7 +211,20 @@ func resourceAwsRouteTableCreate(d *schema.ResourceData, meta interface{}) error return fmt.Errorf("error creating route: %w", err) } - _, destination := routeTableRouteDestinationAttribute(v) + destinationAttributeKey, destination := routeTableRouteDestinationAttribute(v) + + var routeFinder finder.RouteFinder + + switch destinationAttributeKey { + case "cidr_block": + routeFinder = finder.RouteByIPv4Destination + case "ipv6_cidr_block": + routeFinder = finder.RouteByIPv6Destination + case "destination_prefix_list_id": + routeFinder = finder.RouteByPrefixListIDDestination + default: + return fmt.Errorf("error creating Route: unexpected route destination attribute: %q", destinationAttributeKey) + } input := expandEc2CreateRouteInput(v) @@ -222,11 +235,24 @@ func resourceAwsRouteTableCreate(d *schema.ResourceData, meta interface{}) error input.RouteTableId = aws.String(d.Id()) log.Printf("[DEBUG] Creating Route: %s", input) - err = createRoute(conn, input, waiter.PropagationTimeout) + _, err = tfresource.RetryWhenAwsErrCodeEquals( + waiter.PropagationTimeout, + func() (interface{}, error) { + return conn.CreateRoute(input) + }, + tfec2.ErrCodeInvalidParameterException, + tfec2.ErrCodeInvalidTransitGatewayIDNotFound, + ) if err != nil { return fmt.Errorf("error creating Route for Route Table (%s) with destination (%s): %w", d.Id(), destination, err) } + + _, err = waiter.RouteReady(conn, routeFinder, d.Id(), destination) + + if err != nil { + return fmt.Errorf("error waiting for Route in Route Table (%s) with destination (%s) to become available: %w", d.Id(), destination, err) + } } } @@ -264,7 +290,6 @@ func resourceAwsRouteTableRead(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error setting route: %w", err) } - // Tags tags := keyvaluetags.Ec2KeyValueTags(routeTable.Tags).IgnoreAws().IgnoreConfig(ignoreTagsConfig) //lintignore:AWSR002 @@ -461,7 +486,7 @@ func resourceAwsRouteTableUpdate(d *schema.ResourceData, meta interface{}) error } log.Printf("[INFO] Creating route for %s: %#v", d.Id(), opts) - err := resource.Retry(waiter.RouteTableUpdateTimeout, func() *resource.RetryError { + err := resource.Retry(waiter.RouteTableUpdatedTimeout, func() *resource.RetryError { _, err := conn.CreateRoute(&opts) if isAWSErr(err, "InvalidRouteTableID.NotFound", "") { From 576a9f5a841959f7587e2918d19316cfe91797c5 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 11 Jun 2021 16:22:44 -0400 Subject: [PATCH 10/31] Simplify VGW route propagation. --- aws/resource_aws_route_table.go | 94 ++++++++++++--------------------- 1 file changed, 35 insertions(+), 59 deletions(-) diff --git a/aws/resource_aws_route_table.go b/aws/resource_aws_route_table.go index 35190c082d9..0db9e2c8143 100644 --- a/aws/resource_aws_route_table.go +++ b/aws/resource_aws_route_table.go @@ -5,7 +5,6 @@ import ( "fmt" "log" "strings" - "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" @@ -192,7 +191,7 @@ func resourceAwsRouteTableCreate(d *schema.ResourceData, meta interface{}) error v := v.(string) log.Printf("[DEBUG] Enabling Route Table (%s) VPN Gateway (%s) route propagation", d.Id(), v) - err = enableVgwRoutePropagation(conn, d.Id(), v, waiter.PropagationTimeout) + err = enableVgwRoutePropagation(conn, d.Id(), v) if err != nil { return fmt.Errorf("error enabling Route Table (%s) VPN Gateway (%s) route propagation: %w", d.Id(), v, err) @@ -322,57 +321,31 @@ func resourceAwsRouteTableUpdate(d *schema.ResourceData, meta interface{}) error o, n := d.GetChange("propagating_vgws") os := o.(*schema.Set) ns := n.(*schema.Set) - remove := os.Difference(ns).List() + del := os.Difference(ns).List() add := ns.Difference(os).List() // Now first loop through all the old propagations and disable any obsolete ones - for _, vgw := range remove { - id := vgw.(string) + for _, v := range del { + v := v.(string) + + log.Printf("[DEBUG] Disabling Route Table (%s) VPN Gateway (%s) route propagation", d.Id(), v) + err := disableVgwRoutePropagation(conn, d.Id(), v) - // Disable the propagation as it no longer exists in the config - log.Printf("[INFO] Deleting VGW propagation from %s: %s", d.Id(), id) - _, err := conn.DisableVgwRoutePropagation(&ec2.DisableVgwRoutePropagationInput{ - RouteTableId: aws.String(d.Id()), - GatewayId: aws.String(id), - }) if err != nil { - return err + return fmt.Errorf("error disabling Route Table (%s) VPN Gateway (%s) route propagation: %w", d.Id(), v, err) } } - // Make sure we save the state of the currently configured rules - propagatingVGWs := os.Intersection(ns) - d.Set("propagating_vgws", propagatingVGWs) - // Then loop through all the newly configured propagations and enable them - for _, vgw := range add { - id := vgw.(string) - - var err error - for i := 0; i < 5; i++ { - log.Printf("[INFO] Enabling VGW propagation for %s: %s", d.Id(), id) - _, err = conn.EnableVgwRoutePropagation(&ec2.EnableVgwRoutePropagationInput{ - RouteTableId: aws.String(d.Id()), - GatewayId: aws.String(id), - }) - if err == nil { - break - } + for _, v := range add { + v := v.(string) + + log.Printf("[DEBUG] Enabling Route Table (%s) VPN Gateway (%s) route propagation", d.Id(), v) + err := enableVgwRoutePropagation(conn, d.Id(), v) - // If we get a Gateway.NotAttached, it is usually some - // eventually consistency stuff. So we have to just wait a - // bit... - if isAWSErr(err, "Gateway.NotAttached", "") { - time.Sleep(20 * time.Second) - continue - } - } if err != nil { - return err + return fmt.Errorf("error enabling Route Table (%s) VPN Gateway (%s) route propagation: %w", d.Id(), v, err) } - - propagatingVGWs.Add(vgw) - d.Set("propagating_vgws", propagatingVGWs) } } @@ -635,33 +608,36 @@ func resourceAwsRouteTableHash(v interface{}) int { return hashcode.String(buf.String()) } -// enableVgwRoutePropagation attempts to enable VGW route propagation. -// The specified eventual consistency timeout is respected. +// disableVgwRoutePropagation attempts to disable VGW route propagation. // Any error is returned. -func enableVgwRoutePropagation(conn *ec2.EC2, routeTableID, gatewayID string, timeout time.Duration) error { - input := &ec2.EnableVgwRoutePropagationInput{ +func disableVgwRoutePropagation(conn *ec2.EC2, routeTableID, gatewayID string) error { + input := &ec2.DisableVgwRoutePropagationInput{ GatewayId: aws.String(gatewayID), RouteTableId: aws.String(routeTableID), } - err := resource.Retry(timeout, func() *resource.RetryError { - _, err := conn.EnableVgwRoutePropagation(input) - - if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeGatewayNotAttached) { - return resource.RetryableError(err) - } - - if err != nil { - return resource.NonRetryableError(err) - } + _, err := conn.DisableVgwRoutePropagation(input) - return nil - }) + return err +} - if tfresource.TimedOut(err) { - _, err = conn.EnableVgwRoutePropagation(input) +// enableVgwRoutePropagation attempts to enable VGW route propagation. +// The specified eventual consistency timeout is respected. +// Any error is returned. +func enableVgwRoutePropagation(conn *ec2.EC2, routeTableID, gatewayID string) error { + input := &ec2.EnableVgwRoutePropagationInput{ + GatewayId: aws.String(gatewayID), + RouteTableId: aws.String(routeTableID), } + _, err := tfresource.RetryWhenAwsErrCodeEquals( + waiter.PropagationTimeout, + func() (interface{}, error) { + return conn.EnableVgwRoutePropagation(input) + }, + tfec2.ErrCodeGatewayNotAttached, + ) + return err } From ca6a234647802531581b70bd9ca3394dfb304684 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 11 Jun 2021 16:31:49 -0400 Subject: [PATCH 11/31] Add and use 'waiter.RouteDeleted'. --- aws/internal/service/ec2/waiter/waiter.go | 18 ++++++++++++++++++ aws/resource_aws_route.go | 11 +++++++++++ 2 files changed, 29 insertions(+) diff --git a/aws/internal/service/ec2/waiter/waiter.go b/aws/internal/service/ec2/waiter/waiter.go index 3151cb8a4c1..7330192f77f 100644 --- a/aws/internal/service/ec2/waiter/waiter.go +++ b/aws/internal/service/ec2/waiter/waiter.go @@ -258,6 +258,24 @@ const ( NetworkAclEntryPropagationTimeout = 5 * time.Minute ) +func RouteDeleted(conn *ec2.EC2, routeFinder finder.RouteFinder, routeTableID, destination string) (*ec2.Route, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{RouteStatusReady}, + Target: []string{}, + Refresh: RouteStatus(conn, routeFinder, routeTableID, destination), + Timeout: PropagationTimeout, + ContinuousTargetOccurence: 2, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*ec2.Route); ok { + return output, err + } + + return nil, err +} + func RouteReady(conn *ec2.EC2, routeFinder finder.RouteFinder, routeTableID, destination string) (*ec2.Route, error) { stateConf := &resource.StateChangeConf{ Pending: []string{}, diff --git a/aws/resource_aws_route.go b/aws/resource_aws_route.go index e6740315dcb..9cb44186f65 100644 --- a/aws/resource_aws_route.go +++ b/aws/resource_aws_route.go @@ -408,13 +408,18 @@ func resourceAwsRouteDelete(d *schema.ResourceData, meta interface{}) error { RouteTableId: aws.String(routeTableID), } + var routeFinder finder.RouteFinder + switch destination := aws.String(destination); destinationAttributeKey { case "destination_cidr_block": input.DestinationCidrBlock = destination + routeFinder = finder.RouteByIPv4Destination case "destination_ipv6_cidr_block": input.DestinationIpv6CidrBlock = destination + routeFinder = finder.RouteByIPv6Destination case "destination_prefix_list_id": input.DestinationPrefixListId = destination + routeFinder = finder.RouteByPrefixListIDDestination default: return fmt.Errorf("error deleting Route: unexpected route destination attribute: %q", destinationAttributeKey) } @@ -455,6 +460,12 @@ func resourceAwsRouteDelete(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error deleting Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) } + _, err = waiter.RouteDeleted(conn, routeFinder, routeTableID, destination) + + if err != nil { + return fmt.Errorf("error waiting for Route in Route Table (%s) with destination (%s) to delete: %w", routeTableID, destination, err) + } + return nil } From 3e638dd6f8acc4a1a62a6af43d939b937fbfb3b0 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Sun, 13 Jun 2021 17:14:07 -0400 Subject: [PATCH 12/31] r/aws_route_table: Simplify route update. --- aws/resource_aws_route.go | 2 +- aws/resource_aws_route_table.go | 451 ++++++++++++++++----------- aws/resource_aws_route_table_test.go | 4 +- 3 files changed, 270 insertions(+), 187 deletions(-) diff --git a/aws/resource_aws_route.go b/aws/resource_aws_route.go index 9cb44186f65..7568e953d9f 100644 --- a/aws/resource_aws_route.go +++ b/aws/resource_aws_route.go @@ -424,7 +424,7 @@ func resourceAwsRouteDelete(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error deleting Route: unexpected route destination attribute: %q", destinationAttributeKey) } - log.Printf("[DEBUG] Deleting Route (%s)", d.Id()) + log.Printf("[DEBUG] Deleting Route: %s", input) err = resource.Retry(d.Timeout(schema.TimeoutDelete), func() *resource.RetryError { _, err = conn.DeleteRoute(input) diff --git a/aws/resource_aws_route_table.go b/aws/resource_aws_route_table.go index 0db9e2c8143..3259e703381 100644 --- a/aws/resource_aws_route_table.go +++ b/aws/resource_aws_route_table.go @@ -10,7 +10,6 @@ import ( "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/aws-sdk-go-base/tfawserr" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode" @@ -190,11 +189,8 @@ func resourceAwsRouteTableCreate(d *schema.ResourceData, meta interface{}) error for _, v := range v.(*schema.Set).List() { v := v.(string) - log.Printf("[DEBUG] Enabling Route Table (%s) VPN Gateway (%s) route propagation", d.Id(), v) - err = enableVgwRoutePropagation(conn, d.Id(), v) - - if err != nil { - return fmt.Errorf("error enabling Route Table (%s) VPN Gateway (%s) route propagation: %w", d.Id(), v, err) + if err := ec2RouteTableEnableVgwRoutePropagation(conn, d.Id(), v); err != nil { + return err } } } @@ -203,54 +199,8 @@ func resourceAwsRouteTableCreate(d *schema.ResourceData, meta interface{}) error for _, v := range v.(*schema.Set).List() { v := v.(map[string]interface{}) - if err := validateNestedExactlyOneOf(v, routeTableValidDestinations); err != nil { - return fmt.Errorf("error creating route: %w", err) - } - if err := validateNestedExactlyOneOf(v, routeTableValidTargets); err != nil { - return fmt.Errorf("error creating route: %w", err) - } - - destinationAttributeKey, destination := routeTableRouteDestinationAttribute(v) - - var routeFinder finder.RouteFinder - - switch destinationAttributeKey { - case "cidr_block": - routeFinder = finder.RouteByIPv4Destination - case "ipv6_cidr_block": - routeFinder = finder.RouteByIPv6Destination - case "destination_prefix_list_id": - routeFinder = finder.RouteByPrefixListIDDestination - default: - return fmt.Errorf("error creating Route: unexpected route destination attribute: %q", destinationAttributeKey) - } - - input := expandEc2CreateRouteInput(v) - - if input == nil { - continue - } - - input.RouteTableId = aws.String(d.Id()) - - log.Printf("[DEBUG] Creating Route: %s", input) - _, err = tfresource.RetryWhenAwsErrCodeEquals( - waiter.PropagationTimeout, - func() (interface{}, error) { - return conn.CreateRoute(input) - }, - tfec2.ErrCodeInvalidParameterException, - tfec2.ErrCodeInvalidTransitGatewayIDNotFound, - ) - - if err != nil { - return fmt.Errorf("error creating Route for Route Table (%s) with destination (%s): %w", d.Id(), destination, err) - } - - _, err = waiter.RouteReady(conn, routeFinder, d.Id(), destination) - - if err != nil { - return fmt.Errorf("error waiting for Route in Route Table (%s) with destination (%s) to become available: %w", d.Id(), destination, err) + if err := ec2RouteTableAddRoute(conn, d.Id(), v); err != nil { + return err } } } @@ -324,166 +274,80 @@ func resourceAwsRouteTableUpdate(d *schema.ResourceData, meta interface{}) error del := os.Difference(ns).List() add := ns.Difference(os).List() - // Now first loop through all the old propagations and disable any obsolete ones for _, v := range del { v := v.(string) - log.Printf("[DEBUG] Disabling Route Table (%s) VPN Gateway (%s) route propagation", d.Id(), v) - err := disableVgwRoutePropagation(conn, d.Id(), v) - - if err != nil { - return fmt.Errorf("error disabling Route Table (%s) VPN Gateway (%s) route propagation: %w", d.Id(), v, err) + if err := ec2RouteTableDisableVgwRoutePropagation(conn, d.Id(), v); err != nil { + return err } } - // Then loop through all the newly configured propagations and enable them for _, v := range add { v := v.(string) - log.Printf("[DEBUG] Enabling Route Table (%s) VPN Gateway (%s) route propagation", d.Id(), v) - err := enableVgwRoutePropagation(conn, d.Id(), v) - - if err != nil { - return fmt.Errorf("error enabling Route Table (%s) VPN Gateway (%s) route propagation: %w", d.Id(), v, err) + if err := ec2RouteTableEnableVgwRoutePropagation(conn, d.Id(), v); err != nil { + return err } } } - // Check if the route set as a whole has changed if d.HasChange("route") { o, n := d.GetChange("route") - ors := o.(*schema.Set).Difference(n.(*schema.Set)) - nrs := n.(*schema.Set).Difference(o.(*schema.Set)) - // Now first loop through all the old routes and delete any obsolete ones - for _, route := range ors.List() { - m := route.(map[string]interface{}) + for _, new := range n.(*schema.Set).List() { + vNew := new.(map[string]interface{}) - deleteOpts := &ec2.DeleteRouteInput{ - RouteTableId: aws.String(d.Id()), - } + _, newDestination := routeTableRouteDestinationAttribute(vNew) + _, newTarget := routeTableRouteTargetAttribute(vNew) - if s, ok := m["ipv6_cidr_block"].(string); ok && s != "" { - deleteOpts.DestinationIpv6CidrBlock = aws.String(s) + addRoute := true - log.Printf("[INFO] Deleting route from %s: %s", d.Id(), m["ipv6_cidr_block"].(string)) - } + for _, old := range o.(*schema.Set).List() { + vOld := old.(map[string]interface{}) - if s, ok := m["cidr_block"].(string); ok && s != "" { - deleteOpts.DestinationCidrBlock = aws.String(s) + _, oldDestination := routeTableRouteDestinationAttribute(vOld) + _, oldTarget := routeTableRouteTargetAttribute(vOld) - log.Printf("[INFO] Deleting route from %s: %s", d.Id(), m["cidr_block"].(string)) - } + if oldDestination == newDestination { + addRoute = false - if s, ok := m["destination_prefix_list_id"].(string); ok && s != "" { - deleteOpts.DestinationPrefixListId = aws.String(s) - - log.Printf("[INFO] Deleting route from %s: %s", d.Id(), m["destination_prefix_list_id"].(string)) + if oldTarget != newTarget { + if err := ec2RouteTableUpdateRoute(conn, d.Id(), vNew); err != nil { + return err + } + } + } } - _, err := conn.DeleteRoute(deleteOpts) - if err != nil { - return err + if addRoute { + if err := ec2RouteTableAddRoute(conn, d.Id(), vNew); err != nil { + return err + } } } - // Make sure we save the state of the currently configured rules - routes := o.(*schema.Set).Intersection(n.(*schema.Set)) - d.Set("route", routes) - - // Then loop through all the newly configured routes and create them - for _, route := range nrs.List() { - m := route.(map[string]interface{}) - - if err := validateNestedExactlyOneOf(m, routeTableValidDestinations); err != nil { - return fmt.Errorf("error creating route: %w", err) - } - if err := validateNestedExactlyOneOf(m, routeTableValidTargets); err != nil { - return fmt.Errorf("error creating route: %w", err) - } - - opts := ec2.CreateRouteInput{ - RouteTableId: aws.String(d.Id()), - } - - if s, ok := m["transit_gateway_id"].(string); ok && s != "" { - opts.TransitGatewayId = aws.String(s) - } - - if s, ok := m["vpc_endpoint_id"].(string); ok && s != "" { - opts.VpcEndpointId = aws.String(s) - } - - if s, ok := m["vpc_peering_connection_id"].(string); ok && s != "" { - opts.VpcPeeringConnectionId = aws.String(s) - } - - if s, ok := m["network_interface_id"].(string); ok && s != "" { - opts.NetworkInterfaceId = aws.String(s) - } - - if s, ok := m["instance_id"].(string); ok && s != "" { - opts.InstanceId = aws.String(s) - } - - if s, ok := m["ipv6_cidr_block"].(string); ok && s != "" { - opts.DestinationIpv6CidrBlock = aws.String(s) - } - - if s, ok := m["cidr_block"].(string); ok && s != "" { - opts.DestinationCidrBlock = aws.String(s) - } - - if s, ok := m["destination_prefix_list_id"].(string); ok && s != "" { - opts.DestinationPrefixListId = aws.String(s) - } + for _, old := range o.(*schema.Set).List() { + vOld := old.(map[string]interface{}) - if s, ok := m["gateway_id"].(string); ok && s != "" { - opts.GatewayId = aws.String(s) - } + _, oldDestination := routeTableRouteDestinationAttribute(vOld) - if s, ok := m["carrier_gateway_id"].(string); ok && s != "" { - opts.CarrierGatewayId = aws.String(s) - } + delRoute := true - if s, ok := m["egress_only_gateway_id"].(string); ok && s != "" { - opts.EgressOnlyInternetGatewayId = aws.String(s) - } + for _, new := range n.(*schema.Set).List() { + vNew := new.(map[string]interface{}) - if s, ok := m["nat_gateway_id"].(string); ok && s != "" { - opts.NatGatewayId = aws.String(s) - } - - if s, ok := m["local_gateway_id"].(string); ok && s != "" { - opts.LocalGatewayId = aws.String(s) - } + _, newDestination := routeTableRouteDestinationAttribute(vNew) - log.Printf("[INFO] Creating route for %s: %#v", d.Id(), opts) - err := resource.Retry(waiter.RouteTableUpdatedTimeout, func() *resource.RetryError { - _, err := conn.CreateRoute(&opts) - - if isAWSErr(err, "InvalidRouteTableID.NotFound", "") { - return resource.RetryableError(err) - } - - if isAWSErr(err, "InvalidTransitGatewayID.NotFound", "") { - return resource.RetryableError(err) + if newDestination == oldDestination { + delRoute = false } + } - if err != nil { - return resource.NonRetryableError(err) + if delRoute { + if err := ec2RouteTableDeleteRoute(conn, d.Id(), vOld); err != nil { + return err } - return nil - }) - if isResourceTimeoutError(err) { - _, err = conn.CreateRoute(&opts) - } - if err != nil { - return fmt.Errorf("error creating route: %w", err) } - - routes.Add(route) - d.Set("route", routes) } } @@ -608,28 +472,181 @@ func resourceAwsRouteTableHash(v interface{}) int { return hashcode.String(buf.String()) } -// disableVgwRoutePropagation attempts to disable VGW route propagation. +// ec2RouteTableAddRoute adds a route to the specified route table. +func ec2RouteTableAddRoute(conn *ec2.EC2, routeTableID string, tfMap map[string]interface{}) error { + if err := validateNestedExactlyOneOf(tfMap, routeTableValidDestinations); err != nil { + return fmt.Errorf("error creating route: %w", err) + } + if err := validateNestedExactlyOneOf(tfMap, routeTableValidTargets); err != nil { + return fmt.Errorf("error creating route: %w", err) + } + + destinationAttributeKey, destination := routeTableRouteDestinationAttribute(tfMap) + + var routeFinder finder.RouteFinder + + switch destinationAttributeKey { + case "cidr_block": + routeFinder = finder.RouteByIPv4Destination + case "ipv6_cidr_block": + routeFinder = finder.RouteByIPv6Destination + case "destination_prefix_list_id": + routeFinder = finder.RouteByPrefixListIDDestination + default: + return fmt.Errorf("error creating Route: unexpected route destination attribute: %q", destinationAttributeKey) + } + + input := expandEc2CreateRouteInput(tfMap) + + if input == nil { + return nil + } + + input.RouteTableId = aws.String(routeTableID) + + log.Printf("[DEBUG] Creating Route: %s", input) + _, err := tfresource.RetryWhenAwsErrCodeEquals( + waiter.PropagationTimeout, + func() (interface{}, error) { + return conn.CreateRoute(input) + }, + tfec2.ErrCodeInvalidParameterException, + tfec2.ErrCodeInvalidTransitGatewayIDNotFound, + ) + + if err != nil { + return fmt.Errorf("error creating Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) + } + + _, err = waiter.RouteReady(conn, routeFinder, routeTableID, destination) + + if err != nil { + return fmt.Errorf("error waiting for Route in Route Table (%s) with destination (%s) to become available: %w", routeTableID, destination, err) + } + + return nil +} + +// ec2RouteTableDeleteRoute deletes a route from the specified route table. +func ec2RouteTableDeleteRoute(conn *ec2.EC2, routeTableID string, tfMap map[string]interface{}) error { + destinationAttributeKey, destination := routeTableRouteDestinationAttribute(tfMap) + + input := &ec2.DeleteRouteInput{ + RouteTableId: aws.String(routeTableID), + } + + var routeFinder finder.RouteFinder + + switch destination := aws.String(destination); destinationAttributeKey { + case "cidr_block": + input.DestinationCidrBlock = destination + routeFinder = finder.RouteByIPv4Destination + case "ipv6_cidr_block": + input.DestinationIpv6CidrBlock = destination + routeFinder = finder.RouteByIPv6Destination + case "destination_prefix_list_id": + input.DestinationPrefixListId = destination + routeFinder = finder.RouteByPrefixListIDDestination + default: + return fmt.Errorf("error deleting Route: unexpected route destination attribute: %q", destinationAttributeKey) + } + + log.Printf("[DEBUG] Deleting Route: %s", input) + _, err := conn.DeleteRoute(input) + + if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidRouteNotFound) { + return nil + } + + if err != nil { + return fmt.Errorf("error deleting Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) + } + + _, err = waiter.RouteDeleted(conn, routeFinder, routeTableID, destination) + + if err != nil { + return fmt.Errorf("error waiting for Route in Route Table (%s) with destination (%s) to delete: %w", routeTableID, destination, err) + } + + return nil +} + +// ec2RouteTableUpdateRoute updates a route in the specified route table. +func ec2RouteTableUpdateRoute(conn *ec2.EC2, routeTableID string, tfMap map[string]interface{}) error { + if err := validateNestedExactlyOneOf(tfMap, routeTableValidDestinations); err != nil { + return fmt.Errorf("error updating route: %w", err) + } + if err := validateNestedExactlyOneOf(tfMap, routeTableValidTargets); err != nil { + return fmt.Errorf("error updating route: %w", err) + } + + destinationAttributeKey, destination := routeTableRouteDestinationAttribute(tfMap) + + var routeFinder finder.RouteFinder + + switch destinationAttributeKey { + case "cidr_block": + routeFinder = finder.RouteByIPv4Destination + case "ipv6_cidr_block": + routeFinder = finder.RouteByIPv6Destination + case "destination_prefix_list_id": + routeFinder = finder.RouteByPrefixListIDDestination + default: + return fmt.Errorf("error creating Route: unexpected route destination attribute: %q", destinationAttributeKey) + } + + input := expandEc2ReplaceRouteInput(tfMap) + + if input == nil { + return nil + } + + input.RouteTableId = aws.String(routeTableID) + + log.Printf("[DEBUG] Updating Route: %s", input) + _, err := conn.ReplaceRoute(input) + + if err != nil { + return fmt.Errorf("error updating Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) + } + + _, err = waiter.RouteReady(conn, routeFinder, routeTableID, destination) + + if err != nil { + return fmt.Errorf("error waiting for Route in Route Table (%s) with destination (%s) to become available: %w", routeTableID, destination, err) + } + + return nil +} + +// ec2RouteTableDisableVgwRoutePropagation attempts to disable VGW route propagation. // Any error is returned. -func disableVgwRoutePropagation(conn *ec2.EC2, routeTableID, gatewayID string) error { +func ec2RouteTableDisableVgwRoutePropagation(conn *ec2.EC2, routeTableID, gatewayID string) error { input := &ec2.DisableVgwRoutePropagationInput{ GatewayId: aws.String(gatewayID), RouteTableId: aws.String(routeTableID), } + log.Printf("[DEBUG] Disabling Route Table (%s) VPN Gateway (%s) route propagation", routeTableID, gatewayID) _, err := conn.DisableVgwRoutePropagation(input) - return err + if err != nil { + return fmt.Errorf("error disabling Route Table (%s) VPN Gateway (%s) route propagation: %w", routeTableID, gatewayID, err) + } + + return nil } -// enableVgwRoutePropagation attempts to enable VGW route propagation. +// ec2RouteTableEnableVgwRoutePropagation attempts to enable VGW route propagation. // The specified eventual consistency timeout is respected. // Any error is returned. -func enableVgwRoutePropagation(conn *ec2.EC2, routeTableID, gatewayID string) error { +func ec2RouteTableEnableVgwRoutePropagation(conn *ec2.EC2, routeTableID, gatewayID string) error { input := &ec2.EnableVgwRoutePropagationInput{ GatewayId: aws.String(gatewayID), RouteTableId: aws.String(routeTableID), } + log.Printf("[DEBUG] Enabling Route Table (%s) VPN Gateway (%s) route propagation", routeTableID, gatewayID) _, err := tfresource.RetryWhenAwsErrCodeEquals( waiter.PropagationTimeout, func() (interface{}, error) { @@ -638,7 +655,11 @@ func enableVgwRoutePropagation(conn *ec2.EC2, routeTableID, gatewayID string) er tfec2.ErrCodeGatewayNotAttached, ) - return err + if err != nil { + return fmt.Errorf("error enabling Route Table (%s) VPN Gateway (%s) route propagation: %w", routeTableID, gatewayID, err) + } + + return nil } func expandEc2CreateRouteInput(tfMap map[string]interface{}) *ec2.CreateRouteInput { @@ -703,6 +724,68 @@ func expandEc2CreateRouteInput(tfMap map[string]interface{}) *ec2.CreateRouteInp return apiObject } +func expandEc2ReplaceRouteInput(tfMap map[string]interface{}) *ec2.ReplaceRouteInput { + if tfMap == nil { + return nil + } + + apiObject := &ec2.ReplaceRouteInput{} + + if v, ok := tfMap["cidr_block"].(string); ok && v != "" { + apiObject.DestinationCidrBlock = aws.String(v) + } + + if v, ok := tfMap["ipv6_cidr_block"].(string); ok && v != "" { + apiObject.DestinationIpv6CidrBlock = aws.String(v) + } + + if v, ok := tfMap["destination_prefix_list_id"].(string); ok && v != "" { + apiObject.DestinationPrefixListId = aws.String(v) + } + + if v, ok := tfMap["carrier_gateway_id"].(string); ok && v != "" { + apiObject.CarrierGatewayId = aws.String(v) + } + + if v, ok := tfMap["egress_only_gateway_id"].(string); ok && v != "" { + apiObject.EgressOnlyInternetGatewayId = aws.String(v) + } + + if v, ok := tfMap["gateway_id"].(string); ok && v != "" { + apiObject.GatewayId = aws.String(v) + } + + if v, ok := tfMap["instance_id"].(string); ok && v != "" { + apiObject.InstanceId = aws.String(v) + } + + if v, ok := tfMap["local_gateway_id"].(string); ok && v != "" { + apiObject.LocalGatewayId = aws.String(v) + } + + if v, ok := tfMap["nat_gateway_id"].(string); ok && v != "" { + apiObject.NatGatewayId = aws.String(v) + } + + if v, ok := tfMap["network_interface_id"].(string); ok && v != "" { + apiObject.NetworkInterfaceId = aws.String(v) + } + + if v, ok := tfMap["transit_gateway_id"].(string); ok && v != "" { + apiObject.TransitGatewayId = aws.String(v) + } + + if v, ok := tfMap["vpc_endpoint_id"].(string); ok && v != "" { + apiObject.VpcEndpointId = aws.String(v) + } + + if v, ok := tfMap["vpc_peering_connection_id"].(string); ok && v != "" { + apiObject.VpcPeeringConnectionId = aws.String(v) + } + + return apiObject +} + func flattenEc2Route(apiObject *ec2.Route) map[string]interface{} { if apiObject == nil { return nil diff --git a/aws/resource_aws_route_table_test.go b/aws/resource_aws_route_table_test.go index fe046a1837f..f0c7742a486 100644 --- a/aws/resource_aws_route_table_test.go +++ b/aws/resource_aws_route_table_test.go @@ -353,7 +353,7 @@ func TestAccAWSRouteTable_IPv6_To_EgressOnlyInternetGateway(t *testing.T) { }) } -func TestAccAWSRouteTable_tags(t *testing.T) { +func TestAccAWSRouteTable_Tags(t *testing.T) { var routeTable ec2.RouteTable resourceName := "aws_route_table.test" rName := acctest.RandomWithPrefix("tf-acc-test") @@ -688,7 +688,7 @@ func TestAccAWSRouteTable_IPv4_To_VpcPeeringConnection(t *testing.T) { }) } -func TestAccAWSRouteTable_vgwRoutePropagation(t *testing.T) { +func TestAccAWSRouteTable_VgwRoutePropagation(t *testing.T) { var routeTable ec2.RouteTable resourceName := "aws_route_table.test" vgwResourceName1 := "aws_vpn_gateway.test1" From ccd9fd6149b5f1354a3a9e1aadbb2446eec26403 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Sun, 13 Jun 2021 17:53:28 -0400 Subject: [PATCH 13/31] Add 'TestAccAWSRouteTable_IPv4_To_NetworkInterfaces_Unattached'. --- aws/resource_aws_route_table.go | 4 +- aws/resource_aws_route_table_test.go | 134 +++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_route_table.go b/aws/resource_aws_route_table.go index 3259e703381..838477e1d05 100644 --- a/aws/resource_aws_route_table.go +++ b/aws/resource_aws_route_table.go @@ -829,11 +829,11 @@ func flattenEc2Route(apiObject *ec2.Route) map[string]interface{} { tfMap["local_gateway_id"] = aws.StringValue(v) } - if v := apiObject.NetworkInterfaceId; v != nil { + if v := apiObject.NatGatewayId; v != nil { tfMap["nat_gateway_id"] = aws.StringValue(v) } - if v := apiObject.TransitGatewayId; v != nil { + if v := apiObject.NetworkInterfaceId; v != nil { tfMap["network_interface_id"] = aws.StringValue(v) } diff --git a/aws/resource_aws_route_table_test.go b/aws/resource_aws_route_table_test.go index f0c7742a486..663fee82be6 100644 --- a/aws/resource_aws_route_table_test.go +++ b/aws/resource_aws_route_table_test.go @@ -847,6 +847,85 @@ func TestAccAWSRouteTable_IPv6_To_NetworkInterface_Unattached(t *testing.T) { }) } +func TestAccAWSRouteTable_IPv4_To_NetworkInterfaces_Unattached(t *testing.T) { + var routeTable ec2.RouteTable + resourceName := "aws_route_table.test" + eni1ResourceName := "aws_network_interface.test1" + eni2ResourceName := "aws_network_interface.test2" + rName := acctest.RandomWithPrefix("tf-acc-test") + destinationCidr1 := "10.2.0.0/16" + destinationCidr2 := "10.3.0.0/16" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, ec2.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckRouteTableDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSRouteTableConfigBasic(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckRouteTableExists(resourceName, &routeTable), + testAccCheckAWSRouteTableNumberOfRoutes(&routeTable, 1), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`route-table/.+$`)), + testAccCheckResourceAttrAccountID(resourceName, "owner_id"), + resource.TestCheckResourceAttr(resourceName, "propagating_vgws.#", "0"), + resource.TestCheckResourceAttr(resourceName, "route.#", "0"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + ), + }, + { + Config: testAccAWSRouteTableConfigIpv4TwoNetworkInterfacesUnattached(rName, destinationCidr1, destinationCidr2), + Check: resource.ComposeTestCheckFunc( + testAccCheckRouteTableExists(resourceName, &routeTable), + testAccCheckAWSRouteTableNumberOfRoutes(&routeTable, 3), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`route-table/.+$`)), + testAccCheckResourceAttrAccountID(resourceName, "owner_id"), + resource.TestCheckResourceAttr(resourceName, "propagating_vgws.#", "0"), + resource.TestCheckResourceAttr(resourceName, "route.#", "2"), + testAccCheckAWSRouteTableRoute(resourceName, "cidr_block", destinationCidr1, "network_interface_id", eni1ResourceName, "id"), + testAccCheckAWSRouteTableRoute(resourceName, "cidr_block", destinationCidr2, "network_interface_id", eni2ResourceName, "id"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.Name", rName), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAWSRouteTableConfigIpv4TwoNetworkInterfacesUnattached(rName, destinationCidr2, destinationCidr1), + Check: resource.ComposeTestCheckFunc( + testAccCheckRouteTableExists(resourceName, &routeTable), + testAccCheckAWSRouteTableNumberOfRoutes(&routeTable, 3), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`route-table/.+$`)), + testAccCheckResourceAttrAccountID(resourceName, "owner_id"), + resource.TestCheckResourceAttr(resourceName, "propagating_vgws.#", "0"), + resource.TestCheckResourceAttr(resourceName, "route.#", "2"), + testAccCheckAWSRouteTableRoute(resourceName, "cidr_block", destinationCidr2, "network_interface_id", eni1ResourceName, "id"), + testAccCheckAWSRouteTableRoute(resourceName, "cidr_block", destinationCidr1, "network_interface_id", eni2ResourceName, "id"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.Name", rName), + ), + }, + { + Config: testAccAWSRouteTableConfigRouteConfigModeZeroed(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckRouteTableExists(resourceName, &routeTable), + testAccCheckAWSRouteTableNumberOfRoutes(&routeTable, 1), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`route-table/.+$`)), + testAccCheckResourceAttrAccountID(resourceName, "owner_id"), + resource.TestCheckResourceAttr(resourceName, "propagating_vgws.#", "0"), + resource.TestCheckResourceAttr(resourceName, "route.#", "0"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.Name", rName), + ), + }, + }, + }) +} + func TestAccAWSRouteTable_VpcMultipleCidrs(t *testing.T) { var routeTable ec2.RouteTable resourceName := "aws_route_table.test" @@ -2009,6 +2088,61 @@ resource "aws_route_table" "test" { `, rName, destinationCidr) } +func testAccAWSRouteTableConfigIpv4TwoNetworkInterfacesUnattached(rName, destinationCidr1, destinationCidr2 string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { + cidr_block = "10.1.0.0/16" + + tags = { + Name = %[1]q + } +} + +resource "aws_subnet" "test" { + cidr_block = "10.1.1.0/24" + vpc_id = aws_vpc.test.id + + tags = { + Name = %[1]q + } +} + +resource "aws_network_interface" "test1" { + subnet_id = aws_subnet.test.id + + tags = { + Name = %[1]q + } +} + +resource "aws_network_interface" "test2" { + subnet_id = aws_subnet.test.id + + tags = { + Name = %[1]q + } + } + +resource "aws_route_table" "test" { + vpc_id = aws_vpc.test.id + + route { + cidr_block = %[2]q + network_interface_id = aws_network_interface.test1.id + } + + route { + cidr_block = %[3]q + network_interface_id = aws_network_interface.test2.id + } + + tags = { + Name = %[1]q + } +} +`, rName, destinationCidr1, destinationCidr2) +} + func testAccAWSRouteTableConfigVpcMultipleCidrs(rName string) string { return fmt.Sprintf(` resource "aws_vpc" "test" { From e0c7144a7e3d4602659197bc39d3ca1626051713 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Sun, 13 Jun 2021 18:22:33 -0400 Subject: [PATCH 14/31] Tweak error message. --- aws/resource_aws_route.go | 8 ++++---- aws/resource_aws_route_table.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/aws/resource_aws_route.go b/aws/resource_aws_route.go index 7568e953d9f..3ec120e060b 100644 --- a/aws/resource_aws_route.go +++ b/aws/resource_aws_route.go @@ -241,7 +241,7 @@ func resourceAwsRouteCreate(d *schema.ResourceData, meta interface{}) error { ) if err != nil { - return fmt.Errorf("error creating Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) + return fmt.Errorf("error creating Route in Route Table (%s) with destination (%s): %w", routeTableID, destination, err) } _, err = waiter.RouteReady(conn, routeFinder, routeTableID, destination) @@ -288,7 +288,7 @@ func resourceAwsRouteRead(d *schema.ResourceData, meta interface{}) error { } if err != nil { - return fmt.Errorf("error reading Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) + return fmt.Errorf("error reading Route in Route Table (%s) with destination (%s): %w", routeTableID, destination, err) } d.Set("carrier_gateway_id", route.CarrierGatewayId) @@ -382,7 +382,7 @@ func resourceAwsRouteUpdate(d *schema.ResourceData, meta interface{}) error { _, err = conn.ReplaceRoute(input) if err != nil { - return fmt.Errorf("error updating Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) + return fmt.Errorf("error updating Route in Route Table (%s) with destination (%s): %w", routeTableID, destination, err) } _, err = waiter.RouteReady(conn, routeFinder, routeTableID, destination) @@ -457,7 +457,7 @@ func resourceAwsRouteDelete(d *schema.ResourceData, meta interface{}) error { } if err != nil { - return fmt.Errorf("error deleting Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) + return fmt.Errorf("error deleting Route in Route Table (%s) with destination (%s): %w", routeTableID, destination, err) } _, err = waiter.RouteDeleted(conn, routeFinder, routeTableID, destination) diff --git a/aws/resource_aws_route_table.go b/aws/resource_aws_route_table.go index 838477e1d05..e5b32d2e2a7 100644 --- a/aws/resource_aws_route_table.go +++ b/aws/resource_aws_route_table.go @@ -515,7 +515,7 @@ func ec2RouteTableAddRoute(conn *ec2.EC2, routeTableID string, tfMap map[string] ) if err != nil { - return fmt.Errorf("error creating Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) + return fmt.Errorf("error creating Route in Route Table (%s) with destination (%s): %w", routeTableID, destination, err) } _, err = waiter.RouteReady(conn, routeFinder, routeTableID, destination) @@ -559,7 +559,7 @@ func ec2RouteTableDeleteRoute(conn *ec2.EC2, routeTableID string, tfMap map[stri } if err != nil { - return fmt.Errorf("error deleting Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) + return fmt.Errorf("error deleting Route in Route Table (%s) with destination (%s): %w", routeTableID, destination, err) } _, err = waiter.RouteDeleted(conn, routeFinder, routeTableID, destination) @@ -607,7 +607,7 @@ func ec2RouteTableUpdateRoute(conn *ec2.EC2, routeTableID string, tfMap map[stri _, err := conn.ReplaceRoute(input) if err != nil { - return fmt.Errorf("error updating Route for Route Table (%s) with destination (%s): %w", routeTableID, destination, err) + return fmt.Errorf("error updating Route in Route Table (%s) with destination (%s): %w", routeTableID, destination, err) } _, err = waiter.RouteReady(conn, routeFinder, routeTableID, destination) From e6bd0b5f1f87c2eeef03fedb8da19650d908aef0 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Sun, 13 Jun 2021 18:23:32 -0400 Subject: [PATCH 15/31] r/aws-default_route_table: Simplify read. --- aws/internal/service/ec2/finder/finder.go | 13 ++ aws/resource_aws_default_route_table.go | 231 +++++++++---------- aws/resource_aws_default_route_table_test.go | 2 +- 3 files changed, 125 insertions(+), 121 deletions(-) diff --git a/aws/internal/service/ec2/finder/finder.go b/aws/internal/service/ec2/finder/finder.go index 6aaab05915f..f2e11b8c6f4 100644 --- a/aws/internal/service/ec2/finder/finder.go +++ b/aws/internal/service/ec2/finder/finder.go @@ -235,6 +235,19 @@ func NetworkInterfaceSecurityGroup(conn *ec2.EC2, networkInterfaceID string, sec return result, err } +// MainRouteTableByVpcID returns the main route table for the specified VPC. +// Returns NotFoundError if no route table is found. +func MainRouteTableByVpcID(conn *ec2.EC2, vpcID string) (*ec2.RouteTable, error) { + input := &ec2.DescribeRouteTablesInput{ + Filters: tfec2.BuildAttributeFilterList(map[string]string{ + "association.main": "true", + "vpc-id": vpcID, + }), + } + + return RouteTable(conn, input) +} + // RouteTableByID returns the route table corresponding to the specified identifier. // Returns NotFoundError if no route table is found. func RouteTableByID(conn *ec2.EC2, routeTableID string) (*ec2.RouteTable, error) { diff --git a/aws/resource_aws_default_route_table.go b/aws/resource_aws_default_route_table.go index c61c9dfb954..0d400f4c200 100644 --- a/aws/resource_aws_default_route_table.go +++ b/aws/resource_aws_default_route_table.go @@ -7,10 +7,13 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" + tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter" ) func resourceAwsDefaultRouteTable() *schema.Resource { @@ -19,21 +22,27 @@ func resourceAwsDefaultRouteTable() *schema.Resource { Read: resourceAwsDefaultRouteTableRead, Update: resourceAwsRouteTableUpdate, Delete: resourceAwsDefaultRouteTableDelete, + Importer: &schema.ResourceImporter{ - State: func(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { - d.Set("vpc_id", d.Id()) - return []*schema.ResourceData{d}, nil - }, + State: resourceAwsDefaultRouteTableImport, }, + // + // The top-level attributes must be a superset of the aws_route_table resource's attributes as common CRUD handlers are used. + // Schema: map[string]*schema.Schema{ + "arn": { + Type: schema.TypeString, + Computed: true, + }, + "default_route_table_id": { Type: schema.TypeString, Required: true, ForceNew: true, }, - "vpc_id": { + "owner_id": { Type: schema.TypeString, Computed: true, }, @@ -63,7 +72,10 @@ func resourceAwsDefaultRouteTable() *schema.Resource { validateIpv4CIDRNetworkAddress, ), }, - + "destination_prefix_list_id": { + Type: schema.TypeString, + Optional: true, + }, "ipv6_cidr_block": { Type: schema.TypeString, Optional: true, @@ -73,49 +85,39 @@ func resourceAwsDefaultRouteTable() *schema.Resource { ), }, - "destination_prefix_list_id": { - Type: schema.TypeString, - Optional: true, - }, - // // Targets. + // These target attributes are a subset of the aws_route_table resource's target attributes + // as there are some targets that are not allowed in the default route table for a VPC. // "egress_only_gateway_id": { Type: schema.TypeString, Optional: true, }, - "gateway_id": { Type: schema.TypeString, Optional: true, }, - "instance_id": { Type: schema.TypeString, Optional: true, }, - "nat_gateway_id": { Type: schema.TypeString, Optional: true, }, - "network_interface_id": { Type: schema.TypeString, Optional: true, }, - "transit_gateway_id": { Type: schema.TypeString, Optional: true, }, - "vpc_endpoint_id": { Type: schema.TypeString, Optional: true, }, - "vpc_peering_connection_id": { Type: schema.TypeString, Optional: true, @@ -128,12 +130,7 @@ func resourceAwsDefaultRouteTable() *schema.Resource { "tags": tagsSchema(), "tags_all": tagsSchemaComputed(), - "arn": { - Type: schema.TypeString, - Computed: true, - }, - - "owner_id": { + "vpc_id": { Type: schema.TypeString, Computed: true, }, @@ -147,6 +144,7 @@ func resourceAwsDefaultRouteTableCreate(d *schema.ResourceData, meta interface{} conn := meta.(*AWSClient).ec2conn defaultTagsConfig := meta.(*AWSClient).DefaultTagsConfig tags := defaultTagsConfig.MergeTags(keyvaluetags.New(d.Get("tags").(map[string]interface{}))) + routeTableID := d.Get("default_route_table_id").(string) routeTable, err := finder.RouteTableByID(conn, routeTableID) @@ -155,132 +153,125 @@ func resourceAwsDefaultRouteTableCreate(d *schema.ResourceData, meta interface{} return fmt.Errorf("error reading EC2 Default Route Table (%s): %w", routeTableID, err) } - d.SetId(routeTableID) - d.Set("vpc_id", routeTable.VpcId) - - // revoke all default and pre-existing routes on the default route table. - // In the UPDATE method, we'll apply only the rules in the configuration. - log.Printf("[DEBUG] Revoking default routes for Default Route Table for %s", d.Id()) - if err := revokeAllRouteTableRules(conn, routeTable); err != nil { - return err - } - - if len(tags) > 0 { - if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), tags); err != nil { - return fmt.Errorf("error adding tags: %w", err) - } - } - - return resourceAwsRouteTableUpdate(d, meta) -} - -func resourceAwsDefaultRouteTableRead(d *schema.ResourceData, meta interface{}) error { - conn := meta.(*AWSClient).ec2conn - // look up default route table for VPC - filter1 := &ec2.Filter{ - Name: aws.String("association.main"), - Values: []*string{aws.String("true")}, - } - filter2 := &ec2.Filter{ - Name: aws.String("vpc-id"), - Values: []*string{aws.String(d.Get("vpc_id").(string))}, - } - - findOpts := &ec2.DescribeRouteTablesInput{ - Filters: []*ec2.Filter{filter1, filter2}, - } - - resp, err := conn.DescribeRouteTables(findOpts) - if err != nil { - return err - } - - if len(resp.RouteTables) < 1 || resp.RouteTables[0] == nil { - log.Printf("[WARN] EC2 Default Route Table (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } - - rt := resp.RouteTables[0] - - d.Set("default_route_table_id", rt.RouteTableId) - d.SetId(aws.StringValue(rt.RouteTableId)) - - // re-use regular AWS Route Table READ. This is an extra API call but saves us - // from trying to manually keep parity - return resourceAwsRouteTableRead(d, meta) -} - -func resourceAwsDefaultRouteTableDelete(d *schema.ResourceData, meta interface{}) error { - log.Printf("[WARN] Cannot destroy Default Route Table. Terraform will remove this resource from the state file, however resources may remain.") - return nil -} - -// revokeAllRouteTableRules revoke all routes on the Default Route Table -// This should only be ran once at creation time of this resource -func revokeAllRouteTableRules(conn *ec2.EC2, routeTable *ec2.RouteTable) error { - // Remove all Gateway association - for _, r := range routeTable.PropagatingVgws { - _, err := conn.DisableVgwRoutePropagation(&ec2.DisableVgwRoutePropagationInput{ - RouteTableId: routeTable.RouteTableId, - GatewayId: r.GatewayId, - }) + d.SetId(aws.StringValue(routeTable.RouteTableId)) - if err != nil { + // Remove all existing VGW associations. + for _, v := range routeTable.PropagatingVgws { + if err := ec2RouteTableDisableVgwRoutePropagation(conn, d.Id(), aws.StringValue(v.GatewayId)); err != nil { return err } } - // Delete all routes - for _, r := range routeTable.Routes { + // Delete all existing routes. + for _, v := range routeTable.Routes { // you cannot delete the local route - if aws.StringValue(r.GatewayId) == "local" { + if aws.StringValue(v.GatewayId) == "local" { continue } - if aws.StringValue(r.Origin) == ec2.RouteOriginEnableVgwRoutePropagation { + if aws.StringValue(v.Origin) == ec2.RouteOriginEnableVgwRoutePropagation { continue } - if r.DestinationPrefixListId != nil && strings.HasPrefix(aws.StringValue(r.GatewayId), "vpce-") { + if v.DestinationPrefixListId != nil && strings.HasPrefix(aws.StringValue(v.GatewayId), "vpce-") { // Skipping because VPC endpoint routes are handled separately // See aws_vpc_endpoint continue } - if r.DestinationCidrBlock != nil { - _, err := conn.DeleteRoute(&ec2.DeleteRouteInput{ - RouteTableId: routeTable.RouteTableId, - DestinationCidrBlock: r.DestinationCidrBlock, - }) + input := &ec2.DeleteRouteInput{ + RouteTableId: aws.String(d.Id()), + } - if err != nil { - return err - } + var destination string + var routeFinder finder.RouteFinder + + if v.DestinationCidrBlock != nil { + input.DestinationCidrBlock = v.DestinationCidrBlock + destination = aws.StringValue(v.DestinationCidrBlock) + routeFinder = finder.RouteByIPv4Destination + } else if v.DestinationIpv6CidrBlock != nil { + input.DestinationIpv6CidrBlock = v.DestinationIpv6CidrBlock + destination = aws.StringValue(v.DestinationIpv6CidrBlock) + routeFinder = finder.RouteByIPv6Destination + } else if v.DestinationPrefixListId != nil { + input.DestinationPrefixListId = v.DestinationPrefixListId + destination = aws.StringValue(v.DestinationPrefixListId) + routeFinder = finder.RouteByPrefixListIDDestination + } + + log.Printf("[DEBUG] Deleting Route: %s", input) + _, err := conn.DeleteRoute(input) + + if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidRouteNotFound) { + continue + } + + if err != nil { + return fmt.Errorf("error deleting Route in EC2 Default Route Table (%s) with destination (%s): %w", d.Id(), destination, err) + } + + _, err = waiter.RouteDeleted(conn, routeFinder, routeTableID, destination) + + if err != nil { + return fmt.Errorf("error waiting for Route in EC2 Default Route Table (%s) with destination (%s) to delete: %w", d.Id(), destination, err) } + } - if r.DestinationIpv6CidrBlock != nil { - _, err := conn.DeleteRoute(&ec2.DeleteRouteInput{ - RouteTableId: routeTable.RouteTableId, - DestinationIpv6CidrBlock: r.DestinationIpv6CidrBlock, - }) + // Add new VGW associations. + if v, ok := d.GetOk("propagating_vgws"); ok && v.(*schema.Set).Len() > 0 { + for _, v := range v.(*schema.Set).List() { + v := v.(string) - if err != nil { + if err := ec2RouteTableEnableVgwRoutePropagation(conn, d.Id(), v); err != nil { return err } } + } - if r.DestinationPrefixListId != nil { - _, err := conn.DeleteRoute(&ec2.DeleteRouteInput{ - RouteTableId: routeTable.RouteTableId, - DestinationPrefixListId: r.DestinationPrefixListId, - }) + // Add new routes. + if v, ok := d.GetOk("route"); ok && v.(*schema.Set).Len() > 0 { + for _, v := range v.(*schema.Set).List() { + v := v.(map[string]interface{}) - if err != nil { + if err := ec2RouteTableAddRoute(conn, d.Id(), v); err != nil { return err } } } + if len(tags) > 0 { + if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), tags); err != nil { + return fmt.Errorf("error adding tags: %w", err) + } + } + + return resourceAwsDefaultRouteTableRead(d, meta) +} + +func resourceAwsDefaultRouteTableRead(d *schema.ResourceData, meta interface{}) error { + d.Set("default_route_table_id", d.Id()) + + // re-use regular AWS Route Table READ. This is an extra API call but saves us + // from trying to manually keep parity + return resourceAwsRouteTableRead(d, meta) +} + +func resourceAwsDefaultRouteTableDelete(d *schema.ResourceData, meta interface{}) error { + log.Printf("[WARN] Cannot destroy Default Route Table. Terraform will remove this resource from the state file, however resources may remain.") return nil } + +func resourceAwsDefaultRouteTableImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { + conn := meta.(*AWSClient).ec2conn + + routeTable, err := finder.MainRouteTableByVpcID(conn, d.Id()) + + if err != nil { + return nil, err + } + + d.SetId(aws.StringValue(routeTable.RouteTableId)) + + return []*schema.ResourceData{d}, nil +} diff --git a/aws/resource_aws_default_route_table_test.go b/aws/resource_aws_default_route_table_test.go index d40b73823da..d055aab53a4 100644 --- a/aws/resource_aws_default_route_table_test.go +++ b/aws/resource_aws_default_route_table_test.go @@ -324,7 +324,7 @@ func TestAccAWSDefaultRouteTable_VpcEndpointAssociation(t *testing.T) { }) } -func TestAccAWSDefaultRouteTable_tags(t *testing.T) { +func TestAccAWSDefaultRouteTable_Tags(t *testing.T) { var routeTable ec2.RouteTable resourceName := "aws_default_route_table.test" rName := acctest.RandomWithPrefix("tf-acc-test") From 82e54764c199b06f55b0eff3330bff285baafdbc Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 14 Jun 2021 09:27:29 -0400 Subject: [PATCH 16/31] Revert "Add and use 'tfresource.Delete'." This reverts commit 3ce6f12091c6f96a723fa735e38963324a579be4. --- aws/aws_sweeper_test.go | 3 +- aws/internal/tfresource/errors_test.go | 7 +- aws/internal/tfresource/resource.go | 34 --------- aws/internal/tfresource/resource_test.go | 88 ------------------------ aws/provider_test.go | 26 ++++++- aws/resource_aws_route_table.go | 4 +- 6 files changed, 30 insertions(+), 132 deletions(-) delete mode 100644 aws/internal/tfresource/resource.go delete mode 100644 aws/internal/tfresource/resource_test.go diff --git a/aws/aws_sweeper_test.go b/aws/aws_sweeper_test.go index 4fda020dd9e..a16da090e85 100644 --- a/aws/aws_sweeper_test.go +++ b/aws/aws_sweeper_test.go @@ -10,7 +10,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/terraform-providers/terraform-provider-aws/aws/internal/envvar" - "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) // sweeperAwsClients is a shared cache of regional AWSClient @@ -78,7 +77,7 @@ func testSweepResourceOrchestrator(sweepResources []*testSweepResource) error { sweepResource := sweepResource g.Go(func() error { - return tfresource.Delete(sweepResource.resource, sweepResource.d, sweepResource.meta) + return testAccDeleteResource(sweepResource.resource, sweepResource.d, sweepResource.meta) }) } diff --git a/aws/internal/tfresource/errors_test.go b/aws/internal/tfresource/errors_test.go index da75f2cec9e..175ef5150cd 100644 --- a/aws/internal/tfresource/errors_test.go +++ b/aws/internal/tfresource/errors_test.go @@ -1,4 +1,4 @@ -package tfresource_test +package tfresource import ( "errors" @@ -6,7 +6,6 @@ import ( "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" - "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func TestNotFound(t *testing.T) { @@ -41,7 +40,7 @@ func TestNotFound(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { - got := tfresource.NotFound(testCase.Err) + got := NotFound(testCase.Err) if got != testCase.Expected { t.Errorf("got %t, expected %t", got, testCase.Expected) @@ -89,7 +88,7 @@ func TestTimedOut(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { - got := tfresource.TimedOut(testCase.Err) + got := TimedOut(testCase.Err) if got != testCase.Expected { t.Errorf("got %t, expected %t", got, testCase.Expected) diff --git a/aws/internal/tfresource/resource.go b/aws/internal/tfresource/resource.go deleted file mode 100644 index 8b8a765c05e..00000000000 --- a/aws/internal/tfresource/resource.go +++ /dev/null @@ -1,34 +0,0 @@ -package tfresource - -import ( - "context" - "errors" - - multierror "github.com/hashicorp/go-multierror" - "github.com/hashicorp/terraform-plugin-sdk/v2/diag" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" -) - -// Delete calls the specified resource's delete handler. -func Delete(resource *schema.Resource, d *schema.ResourceData, meta interface{}) error { - if resource.DeleteContext != nil || resource.DeleteWithoutTimeout != nil { - var diags diag.Diagnostics - var err *multierror.Error - - if resource.DeleteContext != nil { - diags = resource.DeleteContext(context.Background(), d, meta) - } else { - diags = resource.DeleteWithoutTimeout(context.Background(), d, meta) - } - - for i := range diags { - if diags[i].Severity == diag.Error { - err = multierror.Append(err, errors.New(diags[i].Summary)) - } - } - - return err.ErrorOrNil() - } - - return resource.Delete(d, meta) -} diff --git a/aws/internal/tfresource/resource_test.go b/aws/internal/tfresource/resource_test.go deleted file mode 100644 index 266691e618f..00000000000 --- a/aws/internal/tfresource/resource_test.go +++ /dev/null @@ -1,88 +0,0 @@ -package tfresource_test - -import ( - "context" - "errors" - "testing" - - "github.com/hashicorp/terraform-plugin-sdk/v2/diag" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" -) - -func TestDeleteNoError(t *testing.T) { - theError := errors.New("fail") - - testCases := []struct { - Name string - Resource *schema.Resource - ExpectError bool - }{ - { - Name: "no error Delete function", - Resource: &schema.Resource{ - Delete: func(rd *schema.ResourceData, i interface{}) error { - return nil - }, - }, - }, - { - Name: "no error DeleteContext function", - Resource: &schema.Resource{ - DeleteContext: func(c context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { - return nil - }, - }, - }, - { - Name: "no error DeleteWithoutTimeout function", - Resource: &schema.Resource{ - DeleteWithoutTimeout: func(c context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { - return nil - }, - }, - }, - { - Name: "error Delete function", - Resource: &schema.Resource{ - Delete: func(rd *schema.ResourceData, i interface{}) error { - return theError - }, - }, - ExpectError: true, - }, - { - Name: "error DeleteContext function", - Resource: &schema.Resource{ - DeleteContext: func(c context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { - return diag.FromErr(theError) - }, - }, - ExpectError: true, - }, - { - Name: "error DeleteWithoutTimeout function", - Resource: &schema.Resource{ - DeleteWithoutTimeout: func(c context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { - return diag.FromErr(theError) - }, - }, - ExpectError: true, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.Name, func(t *testing.T) { - r := testCase.Resource - d := r.Data(nil) - - err := tfresource.Delete(r, d, nil) - - if testCase.ExpectError && err == nil { - t.Fatal("expected error") - } else if !testCase.ExpectError && err != nil { - t.Fatalf("unexpected error: %s", err) - } - }) - } -} diff --git a/aws/provider_test.go b/aws/provider_test.go index 38998ba6ce9..de8c5142252 100644 --- a/aws/provider_test.go +++ b/aws/provider_test.go @@ -18,6 +18,7 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/iam" "github.com/aws/aws-sdk-go/service/organizations" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" @@ -25,7 +26,6 @@ import ( "github.com/terraform-providers/terraform-provider-aws/aws/internal/envvar" organizationsfinder "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/organizations/finder" stsfinder "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/sts/finder" - "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) const ( @@ -1035,6 +1035,28 @@ func testAccAwsRegionProviderFunc(region string, providers *[]*schema.Provider) } } +func testAccDeleteResource(resource *schema.Resource, d *schema.ResourceData, meta interface{}) error { + if resource.DeleteContext != nil || resource.DeleteWithoutTimeout != nil { + var diags diag.Diagnostics + + if resource.DeleteContext != nil { + diags = resource.DeleteContext(context.Background(), d, meta) + } else { + diags = resource.DeleteWithoutTimeout(context.Background(), d, meta) + } + + for i := range diags { + if diags[i].Severity == diag.Error { + return fmt.Errorf("error deleting resource: %s", diags[i].Summary) + } + } + + return nil + } + + return resource.Delete(d, meta) +} + func testAccCheckResourceDisappears(provider *schema.Provider, resource *schema.Resource, resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { resourceState, ok := s.RootModule().Resources[resourceName] @@ -1047,7 +1069,7 @@ func testAccCheckResourceDisappears(provider *schema.Provider, resource *schema. return fmt.Errorf("resource ID missing: %s", resourceName) } - return tfresource.Delete(resource, resource.Data(resourceState.Primary), provider.Meta()) + return testAccDeleteResource(resource, resource.Data(resourceState.Primary), provider.Meta()) } } diff --git a/aws/resource_aws_route_table.go b/aws/resource_aws_route_table.go index e5b32d2e2a7..18b93d0a913 100644 --- a/aws/resource_aws_route_table.go +++ b/aws/resource_aws_route_table.go @@ -379,7 +379,7 @@ func resourceAwsRouteTableDelete(d *schema.ResourceData, meta interface{}) error d := r.Data(nil) d.SetId(v) - if err := tfresource.Delete(r, d, meta); err != nil { + if err := r.Delete(d, meta); err != nil { return err } } @@ -400,7 +400,7 @@ func resourceAwsRouteTableDelete(d *schema.ResourceData, meta interface{}) error // Wait for the route table to really destroy log.Printf("[DEBUG] Waiting for route table (%s) deletion", d.Id()) if _, err := waiter.RouteTableDeleted(conn, d.Id()); err != nil { - return fmt.Errorf("error waiting for Route Table (%s) to delete: %w", d.Id(), err) + return fmt.Errorf("error waiting for Route Table (%s) deletion: %w", d.Id(), err) } return nil From baccf09bfc0583287106c743559e25411212502b Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 14 Jun 2021 09:30:06 -0400 Subject: [PATCH 17/31] Move tests to '_test' package. --- aws/internal/tfresource/errors_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/aws/internal/tfresource/errors_test.go b/aws/internal/tfresource/errors_test.go index 175ef5150cd..da75f2cec9e 100644 --- a/aws/internal/tfresource/errors_test.go +++ b/aws/internal/tfresource/errors_test.go @@ -1,4 +1,4 @@ -package tfresource +package tfresource_test import ( "errors" @@ -6,6 +6,7 @@ import ( "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func TestNotFound(t *testing.T) { @@ -40,7 +41,7 @@ func TestNotFound(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { - got := NotFound(testCase.Err) + got := tfresource.NotFound(testCase.Err) if got != testCase.Expected { t.Errorf("got %t, expected %t", got, testCase.Expected) @@ -88,7 +89,7 @@ func TestTimedOut(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { - got := TimedOut(testCase.Err) + got := tfresource.TimedOut(testCase.Err) if got != testCase.Expected { t.Errorf("got %t, expected %t", got, testCase.Expected) From 36aa2a54b893699ab686bb8b4da670b90eb9c424 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 14 Jun 2021 12:43:29 -0400 Subject: [PATCH 18/31] r/aws_route_table_association: Add waiters. Acceptance test output: % make testacc TEST=./aws TESTARGS='-run=TestAccAWSRouteTableAssociation_' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSRouteTableAssociation_ -timeout 180m === RUN TestAccAWSRouteTableAssociation_Subnet_basic === PAUSE TestAccAWSRouteTableAssociation_Subnet_basic === RUN TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable === PAUSE TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable === RUN TestAccAWSRouteTableAssociation_Gateway_basic === PAUSE TestAccAWSRouteTableAssociation_Gateway_basic === RUN TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable === PAUSE TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable === RUN TestAccAWSRouteTableAssociation_disappears === PAUSE TestAccAWSRouteTableAssociation_disappears === CONT TestAccAWSRouteTableAssociation_Subnet_basic === CONT TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable === CONT TestAccAWSRouteTableAssociation_disappears === CONT TestAccAWSRouteTableAssociation_Gateway_basic === CONT TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable --- PASS: TestAccAWSRouteTableAssociation_disappears (41.23s) --- PASS: TestAccAWSRouteTableAssociation_Subnet_basic (43.44s) --- PASS: TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable (64.12s) --- PASS: TestAccAWSRouteTableAssociation_Gateway_basic (74.27s) --- PASS: TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable (94.98s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 98.235s --- aws/internal/service/ec2/finder/finder.go | 28 ++ aws/internal/service/ec2/waiter/status.go | 16 + aws/internal/service/ec2/waiter/waiter.go | 72 ++++ aws/resource_aws_route_table.go | 6 +- aws/resource_aws_route_table_association.go | 233 +++++------ ...source_aws_route_table_association_test.go | 396 ++++++------------ 6 files changed, 346 insertions(+), 405 deletions(-) diff --git a/aws/internal/service/ec2/finder/finder.go b/aws/internal/service/ec2/finder/finder.go index f2e11b8c6f4..4bf913ffca1 100644 --- a/aws/internal/service/ec2/finder/finder.go +++ b/aws/internal/service/ec2/finder/finder.go @@ -235,6 +235,34 @@ func NetworkInterfaceSecurityGroup(conn *ec2.EC2, networkInterfaceID string, sec return result, err } +// RouteTableAssociationByID returns the route table association corresponding to the specified identifier. +// Returns NotFoundError if no route table association is found. +func RouteTableAssociationByID(conn *ec2.EC2, associationID string) (*ec2.RouteTableAssociation, error) { + input := &ec2.DescribeRouteTablesInput{ + Filters: tfec2.BuildAttributeFilterList(map[string]string{ + "association.route-table-association-id": associationID, + }), + } + + routeTable, err := RouteTable(conn, input) + + if err != nil { + return nil, err + } + + for _, association := range routeTable.Associations { + if aws.StringValue(association.RouteTableAssociationId) == associationID { + if state := aws.StringValue(association.AssociationState.State); state == ec2.RouteTableAssociationStateCodeDisassociated { + return nil, &resource.NotFoundError{Message: state} + } + + return association, nil + } + } + + return nil, &resource.NotFoundError{} +} + // MainRouteTableByVpcID returns the main route table for the specified VPC. // Returns NotFoundError if no route table is found. func MainRouteTableByVpcID(conn *ec2.EC2, vpcID string) (*ec2.RouteTable, error) { diff --git a/aws/internal/service/ec2/waiter/status.go b/aws/internal/service/ec2/waiter/status.go index 70f884f1ad5..8b8ffde410c 100644 --- a/aws/internal/service/ec2/waiter/status.go +++ b/aws/internal/service/ec2/waiter/status.go @@ -286,6 +286,22 @@ func RouteTableStatus(conn *ec2.EC2, id string) resource.StateRefreshFunc { } } +func RouteTableAssociationState(conn *ec2.EC2, id string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + output, err := finder.RouteTableAssociationByID(conn, id) + + if tfresource.NotFound(err) { + return nil, "", nil + } + + if err != nil { + return nil, "", err + } + + return output.AssociationState, aws.StringValue(output.AssociationState.State), nil + } +} + const ( SecurityGroupStatusCreated = "Created" diff --git a/aws/internal/service/ec2/waiter/waiter.go b/aws/internal/service/ec2/waiter/waiter.go index 7330192f77f..b119c2fdb77 100644 --- a/aws/internal/service/ec2/waiter/waiter.go +++ b/aws/internal/service/ec2/waiter/waiter.go @@ -1,14 +1,17 @@ package waiter import ( + "errors" "strconv" "time" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) const ( @@ -295,6 +298,12 @@ func RouteReady(conn *ec2.EC2, routeFinder finder.RouteFinder, routeTableID, des } const ( + RouteTableAssociationPropagationTimeout = 5 * time.Minute + + RouteTableAssociationCreatedTimeout = 5 * time.Minute + RouteTableAssociationUpdatedTimeout = 5 * time.Minute + RouteTableAssociationDeletedTimeout = 5 * time.Minute + RouteTableReadyTimeout = 10 * time.Minute RouteTableDeletedTimeout = 5 * time.Minute RouteTableUpdatedTimeout = 5 * time.Minute @@ -337,6 +346,69 @@ func RouteTableDeleted(conn *ec2.EC2, id string) (*ec2.RouteTable, error) { return nil, err } +func RouteTableAssociationCreated(conn *ec2.EC2, id string) (*ec2.RouteTableAssociationState, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{ec2.RouteTableAssociationStateCodeAssociating}, + Target: []string{ec2.RouteTableAssociationStateCodeAssociated}, + Refresh: RouteTableAssociationState(conn, id), + Timeout: RouteTableAssociationCreatedTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*ec2.RouteTableAssociationState); ok { + if output != nil && aws.StringValue(output.State) == ec2.RouteTableAssociationStateCodeFailed { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.StatusMessage))) + } + + return output, err + } + + return nil, err +} + +func RouteTableAssociationDeleted(conn *ec2.EC2, id string) (*ec2.RouteTableAssociationState, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{ec2.RouteTableAssociationStateCodeDisassociating}, + Target: []string{}, + Refresh: RouteTableAssociationState(conn, id), + Timeout: RouteTableAssociationDeletedTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*ec2.RouteTableAssociationState); ok { + if output != nil && aws.StringValue(output.State) == ec2.RouteTableAssociationStateCodeFailed { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.StatusMessage))) + } + + return output, err + } + + return nil, err +} + +func RouteTableAssociationUpdated(conn *ec2.EC2, id string) (*ec2.RouteTableAssociationState, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{ec2.RouteTableAssociationStateCodeAssociating}, + Target: []string{ec2.RouteTableAssociationStateCodeAssociated}, + Refresh: RouteTableAssociationState(conn, id), + Timeout: RouteTableAssociationUpdatedTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*ec2.RouteTableAssociationState); ok { + if output != nil && aws.StringValue(output.State) == ec2.RouteTableAssociationStateCodeFailed { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.StatusMessage))) + } + + return output, err + } + + return nil, err +} + func SecurityGroupCreated(conn *ec2.EC2, id string, timeout time.Duration) (*ec2.SecurityGroup, error) { stateConf := &resource.StateChangeConf{ Pending: []string{SecurityGroupStatusNotFound}, diff --git a/aws/resource_aws_route_table.go b/aws/resource_aws_route_table.go index 18b93d0a913..7844a32a403 100644 --- a/aws/resource_aws_route_table.go +++ b/aws/resource_aws_route_table.go @@ -375,11 +375,7 @@ func resourceAwsRouteTableDelete(d *schema.ResourceData, meta interface{}) error for _, v := range routeTable.Associations { v := aws.StringValue(v.RouteTableAssociationId) - r := resourceAwsRouteTableAssociation() - d := r.Data(nil) - d.SetId(v) - - if err := r.Delete(d, meta); err != nil { + if err := ec2RouteTableAssociationDelete(conn, v); err != nil { return err } } diff --git a/aws/resource_aws_route_table_association.go b/aws/resource_aws_route_table_association.go index 3b970da09dd..552dad00db1 100644 --- a/aws/resource_aws_route_table_association.go +++ b/aws/resource_aws_route_table_association.go @@ -4,13 +4,13 @@ import ( "fmt" "log" "strings" - "time" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter" "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) @@ -26,23 +26,24 @@ func resourceAwsRouteTableAssociation() *schema.Resource { }, Schema: map[string]*schema.Schema{ - "subnet_id": { - Type: schema.TypeString, - Optional: true, - ExactlyOneOf: []string{"subnet_id", "gateway_id"}, - ForceNew: true, - }, "gateway_id": { Type: schema.TypeString, Optional: true, - ExactlyOneOf: []string{"subnet_id", "gateway_id"}, ForceNew: true, + ExactlyOneOf: []string{"subnet_id", "gateway_id"}, }, "route_table_id": { Type: schema.TypeString, Required: true, }, + + "subnet_id": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ExactlyOneOf: []string{"subnet_id", "gateway_id"}, + }, }, } } @@ -50,89 +51,60 @@ func resourceAwsRouteTableAssociation() *schema.Resource { func resourceAwsRouteTableAssociationCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - associationOpts := ec2.AssociateRouteTableInput{ - RouteTableId: aws.String(d.Get("route_table_id").(string)), + routeTableID := d.Get("route_table_id").(string) + input := &ec2.AssociateRouteTableInput{ + RouteTableId: aws.String(routeTableID), } - if len(d.Get("subnet_id").(string)) > 0 { - log.Printf( - "[INFO] Creating route table association: %s => %s", - d.Get("subnet_id").(string), - d.Get("route_table_id").(string)) - associationOpts.SubnetId = aws.String(d.Get("subnet_id").(string)) - } else if len(d.Get("gateway_id").(string)) > 0 { - log.Printf( - "[INFO] Creating route table association: %s => %s", - d.Get("gateway_id").(string), - d.Get("route_table_id").(string)) - associationOpts.GatewayId = aws.String(d.Get("gateway_id").(string)) + + if v, ok := d.GetOk("gateway_id"); ok { + input.GatewayId = aws.String(v.(string)) } - var associationID string - var resp *ec2.AssociateRouteTableOutput - err := resource.Retry(5*time.Minute, func() *resource.RetryError { - var err error - resp, err = conn.AssociateRouteTable(&associationOpts) - if err != nil { - if awsErr, ok := err.(awserr.Error); ok { - if awsErr.Code() == "InvalidRouteTableID.NotFound" { - return resource.RetryableError(awsErr) - } - } - return resource.NonRetryableError(err) - } - associationID = *resp.AssociationId - return nil - }) - if isResourceTimeoutError(err) { - resp, err = conn.AssociateRouteTable(&associationOpts) + if v, ok := d.GetOk("subnet_id"); ok { + input.SubnetId = aws.String(v.(string)) } + + log.Printf("[DEBUG] Creating Route Table Association: %s", input) + output, err := tfresource.RetryWhenAwsErrCodeEquals( + waiter.RouteTableAssociationPropagationTimeout, + func() (interface{}, error) { + return conn.AssociateRouteTable(input) + }, + tfec2.ErrCodeInvalidRouteTableIDNotFound, + ) + if err != nil { - return fmt.Errorf("Error creating route table association: %s", err) + return fmt.Errorf("error creating Route Table (%s) Association: %w", routeTableID, err) } - // Set the ID and return - d.SetId(associationID) - log.Printf("[INFO] Association ID: %s", d.Id()) + d.SetId(aws.StringValue(output.(*ec2.AssociateRouteTableOutput).AssociationId)) - return nil + log.Printf("[DEBUG] Waiting for Route Table Association (%s) creation", d.Id()) + if _, err := waiter.RouteTableAssociationCreated(conn, d.Id()); err != nil { + return fmt.Errorf("error waiting for Route Table Association (%s) create: %w", d.Id(), err) + } + + return resourceAwsRouteTableAssociationRead(d, meta) } func resourceAwsRouteTableAssociationRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - // Get the routing table that this association belongs to - rtID := d.Get("route_table_id").(string) - rt, err := waiter.RouteTableReady(conn, rtID) + association, err := finder.RouteTableAssociationByID(conn, d.Id()) if !d.IsNewResource() && tfresource.NotFound(err) { - log.Printf("[WARN] Route table (%s) not found, removing route table association (%s) from state", rtID, d.Id()) + log.Printf("[WARN] Route Table Association (%s) not found, removing from state", d.Id()) d.SetId("") return nil } if err != nil { - return fmt.Errorf("error getting route table (%s) status while reading route table association: %w", rtID, err) - } - - // Inspect that the association exists - found := false - for _, a := range rt.Associations { - if *a.RouteTableAssociationId == d.Id() { - found = true - if a.SubnetId != nil { - d.Set("subnet_id", a.SubnetId) - } - if a.GatewayId != nil { - d.Set("gateway_id", a.GatewayId) - } - break - } + return fmt.Errorf("error reading Route Table Association (%s): %w", d.Id(), err) } - if !found { - // It seems it doesn't exist anymore, so clear the ID - d.SetId("") - } + d.Set("gateway_id", association.GatewayId) + d.Set("route_table_id", association.RouteTableId) + d.Set("subnet_id", association.SubnetId) return nil } @@ -140,51 +112,43 @@ func resourceAwsRouteTableAssociationRead(d *schema.ResourceData, meta interface func resourceAwsRouteTableAssociationUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - log.Printf( - "[INFO] Creating route table association: %s => %s", - d.Get("subnet_id").(string), - d.Get("route_table_id").(string)) - - req := &ec2.ReplaceRouteTableAssociationInput{ + input := &ec2.ReplaceRouteTableAssociationInput{ AssociationId: aws.String(d.Id()), RouteTableId: aws.String(d.Get("route_table_id").(string)), } - resp, err := conn.ReplaceRouteTableAssociation(req) - if err != nil { - ec2err, ok := err.(awserr.Error) - if ok && ec2err.Code() == "InvalidAssociationID.NotFound" { - // Not found, so just create a new one - return resourceAwsRouteTableAssociationCreate(d, meta) - } + log.Printf("[DEBUG] Updating Route Table Association: %s", input) + output, err := conn.ReplaceRouteTableAssociation(input) + + // This whole thing with the resource ID being changed on update seems unsustainable. + // Keeping it here for backwards compatibilty... + + if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidAssociationIDNotFound) { + // Not found, so just create a new one + return resourceAwsRouteTableAssociationCreate(d, meta) + } - return err + if err != nil { + return fmt.Errorf("error updating Route Table Association (%s): %w", d.Id(), err) } - // Update the ID - d.SetId(aws.StringValue(resp.NewAssociationId)) - log.Printf("[INFO] Association ID: %s", d.Id()) + // I don't think we'll ever reach this code for a subnet/gateway route table association. + // It would only come in to play for a VPC main route table association. - return nil + d.SetId(aws.StringValue(output.NewAssociationId)) + + log.Printf("[DEBUG] Waiting for Route Table Association (%s) update", d.Id()) + if _, err := waiter.RouteTableAssociationUpdated(conn, d.Id()); err != nil { + return fmt.Errorf("error waiting for Route Table Association (%s) update: %w", d.Id(), err) + } + + return resourceAwsRouteTableAssociationRead(d, meta) } func resourceAwsRouteTableAssociationDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - log.Printf("[INFO] Deleting route table association: %s", d.Id()) - _, err := conn.DisassociateRouteTable(&ec2.DisassociateRouteTableInput{ - AssociationId: aws.String(d.Id()), - }) - if err != nil { - ec2err, ok := err.(awserr.Error) - if ok && ec2err.Code() == "InvalidAssociationID.NotFound" { - return nil - } - - return fmt.Errorf("Error deleting route table association: %s", err) - } - - return nil + return ec2RouteTableAssociationDelete(conn, d.Id()) } func resourceAwsRouteTableAssociationImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { @@ -200,48 +164,59 @@ func resourceAwsRouteTableAssociationImport(d *schema.ResourceData, meta interfa conn := meta.(*AWSClient).ec2conn - input := &ec2.DescribeRouteTablesInput{} - input.Filters = buildEC2AttributeFilterList( - map[string]string{ - "association.route-table-id": routeTableID, - }, - ) + routeTable, err := finder.RouteTableByID(conn, routeTableID) - output, err := conn.DescribeRouteTables(input) if err != nil { - return nil, fmt.Errorf("Error finding route table: %s", err) - } - if len(output.RouteTables) == 0 { - return nil, fmt.Errorf("No route table found with ID %s", routeTableID) + return nil, err } - rt := output.RouteTables[0] - - var targetType string var associationID string - for _, a := range rt.Associations { - if aws.StringValue(a.SubnetId) == targetID { - targetType = "subnet" - associationID = aws.StringValue(a.RouteTableAssociationId) + + for _, association := range routeTable.Associations { + if aws.StringValue(association.SubnetId) == targetID { + d.Set("subnet_id", targetID) + associationID = aws.StringValue(association.RouteTableAssociationId) + break } - if aws.StringValue(a.SubnetId) == targetID || aws.StringValue(a.GatewayId) == targetID { - targetType = "gateway" - associationID = aws.StringValue(a.RouteTableAssociationId) + + if aws.StringValue(association.GatewayId) == targetID { + d.Set("gateway_id", targetID) + associationID = aws.StringValue(association.RouteTableAssociationId) + break } } + if associationID == "" { return nil, fmt.Errorf("No association found between route table ID %s and target ID %s", routeTableID, targetID) } d.SetId(associationID) - if targetType == "subnet" { - d.Set("subnet_id", targetID) - } else if targetType == "gateway" { - d.Set("gateway_id", targetID) - } d.Set("route_table_id", routeTableID) return []*schema.ResourceData{d}, nil } + +// ec2RouteTableAssociationDelete attempts to delete a route table association. +func ec2RouteTableAssociationDelete(conn *ec2.EC2, associationID string) error { + log.Printf("[INFO] Deleting Route Table Association: %s", associationID) + _, err := conn.DisassociateRouteTable(&ec2.DisassociateRouteTableInput{ + AssociationId: aws.String(associationID), + }) + + if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidAssociationIDNotFound) { + return nil + } + + if err != nil { + return fmt.Errorf("error deleting Route Table Association (%s): %w", associationID, err) + } + + log.Printf("[DEBUG] Waiting for Route Table Association (%s) deletion", associationID) + if _, err := waiter.RouteTableAssociationDeleted(conn, associationID); err != nil { + return fmt.Errorf("error waiting for Route Table Association (%s) delete: %w", associationID, err) + } + + return nil +} diff --git a/aws/resource_aws_route_table_association_test.go b/aws/resource_aws_route_table_association_test.go index f33aa25d0ae..e0017635d4e 100644 --- a/aws/resource_aws_route_table_association_test.go +++ b/aws/resource_aws_route_table_association_test.go @@ -4,8 +4,8 @@ import ( "fmt" "testing" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" @@ -14,10 +14,10 @@ import ( func TestAccAWSRouteTableAssociation_Subnet_basic(t *testing.T) { var rta ec2.RouteTableAssociation - - resourceNameAssoc := "aws_route_table_association.foo" - resourceNameRouteTable := "aws_route_table.foo" - resourceNameSubnet := "aws_subnet.foo" + resourceName := "aws_route_table_association.test" + resourceNameRouteTable := "aws_route_table.test" + resourceNameSubnet := "aws_subnet.test" + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -26,17 +26,17 @@ func TestAccAWSRouteTableAssociation_Subnet_basic(t *testing.T) { CheckDestroy: testAccCheckRouteTableAssociationDestroy, Steps: []resource.TestStep{ { - Config: testAccRouteTableAssociationSubnetConfig, + Config: testAccRouteTableAssociationConfigSubnet(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckRouteTableAssociationExists(resourceNameAssoc, &rta), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "route_table_id", resourceNameRouteTable, "id"), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "subnet_id", resourceNameSubnet, "id"), + testAccCheckRouteTableAssociationExists(resourceName, &rta), + resource.TestCheckResourceAttrPair(resourceName, "route_table_id", resourceNameRouteTable, "id"), + resource.TestCheckResourceAttrPair(resourceName, "subnet_id", resourceNameSubnet, "id"), ), }, { - ResourceName: resourceNameAssoc, + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccAWSRouteTabAssocImportStateIdFunc(resourceNameAssoc), + ImportStateIdFunc: testAccAWSRouteTabAssocImportStateIdFunc(resourceName), ImportStateVerify: true, }, }, @@ -44,46 +44,12 @@ func TestAccAWSRouteTableAssociation_Subnet_basic(t *testing.T) { } func TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable(t *testing.T) { - var rta1, rta2 ec2.RouteTableAssociation - - resourceNameAssoc := "aws_route_table_association.foo" - resourceNameRouteTable1 := "aws_route_table.foo" - resourceNameRouteTable2 := "aws_route_table.bar" - resourceNameSubnet := "aws_subnet.foo" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ErrorCheck: testAccErrorCheck(t, ec2.EndpointsID), - Providers: testAccProviders, - CheckDestroy: testAccCheckRouteTableAssociationDestroy, - Steps: []resource.TestStep{ - { - Config: testAccRouteTableAssociationSubnetConfig, - Check: resource.ComposeTestCheckFunc( - testAccCheckRouteTableAssociationExists(resourceNameAssoc, &rta1), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "route_table_id", resourceNameRouteTable1, "id"), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "subnet_id", resourceNameSubnet, "id"), - ), - }, - { - Config: testAccRouteTableAssociationSubnetConfig_ChangeRouteTable, - Check: resource.ComposeTestCheckFunc( - testAccCheckRouteTableAssociationExists(resourceNameAssoc, &rta2), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "route_table_id", resourceNameRouteTable2, "id"), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "subnet_id", resourceNameSubnet, "id"), - ), - }, - }, - }) -} - -func TestAccAWSRouteTableAssociation_Subnet_ChangeSubnet(t *testing.T) { - var rta1, rta2 ec2.RouteTableAssociation - - resourceNameAssoc := "aws_route_table_association.foo" - resourceNameRouteTable := "aws_route_table.foo" - resourceNameSubnet1 := "aws_subnet.foo" - resourceNameSubnet2 := "aws_subnet.bar" + var rta ec2.RouteTableAssociation + resourceName := "aws_route_table_association.test" + resourceNameRouteTable1 := "aws_route_table.test" + resourceNameRouteTable2 := "aws_route_table.test2" + resourceNameSubnet := "aws_subnet.test" + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -92,19 +58,19 @@ func TestAccAWSRouteTableAssociation_Subnet_ChangeSubnet(t *testing.T) { CheckDestroy: testAccCheckRouteTableAssociationDestroy, Steps: []resource.TestStep{ { - Config: testAccRouteTableAssociationSubnetConfig, + Config: testAccRouteTableAssociationConfigSubnet(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckRouteTableAssociationExists(resourceNameAssoc, &rta1), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "route_table_id", resourceNameRouteTable, "id"), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "subnet_id", resourceNameSubnet1, "id"), + testAccCheckRouteTableAssociationExists(resourceName, &rta), + resource.TestCheckResourceAttrPair(resourceName, "route_table_id", resourceNameRouteTable1, "id"), + resource.TestCheckResourceAttrPair(resourceName, "subnet_id", resourceNameSubnet, "id"), ), }, { - Config: testAccRouteTableAssociationSubnetConfig_ChangeSubnet, + Config: testAccRouteTableAssociationConfigSubnetChangeRouteTable(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckRouteTableAssociationExists(resourceNameAssoc, &rta2), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "route_table_id", resourceNameRouteTable, "id"), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "subnet_id", resourceNameSubnet2, "id"), + testAccCheckRouteTableAssociationExists(resourceName, &rta), + resource.TestCheckResourceAttrPair(resourceName, "route_table_id", resourceNameRouteTable2, "id"), + resource.TestCheckResourceAttrPair(resourceName, "subnet_id", resourceNameSubnet, "id"), ), }, }, @@ -113,10 +79,10 @@ func TestAccAWSRouteTableAssociation_Subnet_ChangeSubnet(t *testing.T) { func TestAccAWSRouteTableAssociation_Gateway_basic(t *testing.T) { var rta ec2.RouteTableAssociation - - resourceNameAssoc := "aws_route_table_association.foo" - resourceNameRouteTable := "aws_route_table.foo" - resourceNameGateway := "aws_internet_gateway.foo" + resourceName := "aws_route_table_association.test" + resourceNameRouteTable := "aws_route_table.test" + resourceNameGateway := "aws_internet_gateway.test" + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -125,17 +91,17 @@ func TestAccAWSRouteTableAssociation_Gateway_basic(t *testing.T) { CheckDestroy: testAccCheckRouteTableAssociationDestroy, Steps: []resource.TestStep{ { - Config: testAccRouteTableAssociationGatewayConfig, + Config: testAccRouteTableAssociationConfigGateway(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckRouteTableAssociationExists(resourceNameAssoc, &rta), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "route_table_id", resourceNameRouteTable, "id"), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "gateway_id", resourceNameGateway, "id"), + testAccCheckRouteTableAssociationExists(resourceName, &rta), + resource.TestCheckResourceAttrPair(resourceName, "route_table_id", resourceNameRouteTable, "id"), + resource.TestCheckResourceAttrPair(resourceName, "gateway_id", resourceNameGateway, "id"), ), }, { - ResourceName: resourceNameAssoc, + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccAWSRouteTabAssocImportStateIdFunc(resourceNameAssoc), + ImportStateIdFunc: testAccAWSRouteTabAssocImportStateIdFunc(resourceName), ImportStateVerify: true, }, }, @@ -143,46 +109,12 @@ func TestAccAWSRouteTableAssociation_Gateway_basic(t *testing.T) { } func TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable(t *testing.T) { - var rta1, rta2 ec2.RouteTableAssociation - - resourceNameAssoc := "aws_route_table_association.foo" - resourceNameRouteTable1 := "aws_route_table.foo" - resourceNameRouteTable2 := "aws_route_table.bar" - resourceNameGateway := "aws_internet_gateway.foo" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ErrorCheck: testAccErrorCheck(t, ec2.EndpointsID), - Providers: testAccProviders, - CheckDestroy: testAccCheckRouteTableAssociationDestroy, - Steps: []resource.TestStep{ - { - Config: testAccRouteTableAssociationGatewayConfig, - Check: resource.ComposeTestCheckFunc( - testAccCheckRouteTableAssociationExists(resourceNameAssoc, &rta1), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "route_table_id", resourceNameRouteTable1, "id"), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "gateway_id", resourceNameGateway, "id"), - ), - }, - { - Config: testAccRouteTableAssociationGatewayConfig_ChangeRouteTable, - Check: resource.ComposeTestCheckFunc( - testAccCheckRouteTableAssociationExists(resourceNameAssoc, &rta2), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "route_table_id", resourceNameRouteTable2, "id"), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "gateway_id", resourceNameGateway, "id"), - ), - }, - }, - }) -} - -func TestAccAWSRouteTableAssociation_Gateway_ChangeGateway(t *testing.T) { - var rta1, rta2 ec2.RouteTableAssociation - - resourceNameAssoc := "aws_route_table_association.foo" - resourceNameRouteTable := "aws_route_table.foo" - resourceNameGateway1 := "aws_internet_gateway.foo" - resourceNameGateway2 := "aws_vpn_gateway.bar" + var rta ec2.RouteTableAssociation + resourceName := "aws_route_table_association.test" + resourceNameRouteTable1 := "aws_route_table.test" + resourceNameRouteTable2 := "aws_route_table.test2" + resourceNameGateway := "aws_internet_gateway.test" + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -191,19 +123,19 @@ func TestAccAWSRouteTableAssociation_Gateway_ChangeGateway(t *testing.T) { CheckDestroy: testAccCheckRouteTableAssociationDestroy, Steps: []resource.TestStep{ { - Config: testAccRouteTableAssociationGatewayConfig, + Config: testAccRouteTableAssociationConfigGateway(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckRouteTableAssociationExists(resourceNameAssoc, &rta1), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "route_table_id", resourceNameRouteTable, "id"), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "gateway_id", resourceNameGateway1, "id"), + testAccCheckRouteTableAssociationExists(resourceName, &rta), + resource.TestCheckResourceAttrPair(resourceName, "route_table_id", resourceNameRouteTable1, "id"), + resource.TestCheckResourceAttrPair(resourceName, "gateway_id", resourceNameGateway, "id"), ), }, { - Config: testAccRouteTableAssociationGatewayConfig_ChangeGateway, + Config: testAccRouteTableAssociationConfigGatewayChangeRouteTable(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckRouteTableAssociationExists(resourceNameAssoc, &rta2), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "route_table_id", resourceNameRouteTable, "id"), - resource.TestCheckResourceAttrPair(resourceNameAssoc, "gateway_id", resourceNameGateway2, "id"), + testAccCheckRouteTableAssociationExists(resourceName, &rta), + resource.TestCheckResourceAttrPair(resourceName, "route_table_id", resourceNameRouteTable2, "id"), + resource.TestCheckResourceAttrPair(resourceName, "gateway_id", resourceNameGateway, "id"), ), }, }, @@ -212,8 +144,8 @@ func TestAccAWSRouteTableAssociation_Gateway_ChangeGateway(t *testing.T) { func TestAccAWSRouteTableAssociation_disappears(t *testing.T) { var rta ec2.RouteTableAssociation - - resourceNameAssoc := "aws_route_table_association.foo" + resourceName := "aws_route_table_association.test" + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -222,10 +154,10 @@ func TestAccAWSRouteTableAssociation_disappears(t *testing.T) { CheckDestroy: testAccCheckRouteTableAssociationDestroy, Steps: []resource.TestStep{ { - Config: testAccRouteTableAssociationSubnetConfig, + Config: testAccRouteTableAssociationConfigSubnet(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckRouteTableAssociationExists(resourceNameAssoc, &rta), - testAccCheckRouteTableAssociationDisappears(&rta), + testAccCheckRouteTableAssociationExists(resourceName, &rta), + testAccCheckResourceDisappears(testAccProvider, resourceAwsRouteTableAssociation(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -241,7 +173,7 @@ func testAccCheckRouteTableAssociationDestroy(s *terraform.State) error { continue } - routeTable, err := finder.RouteTableByID(conn, rs.Primary.ID) + _, err := finder.RouteTableAssociationByID(conn, rs.Primary.ID) if tfresource.NotFound(err) { continue @@ -251,15 +183,13 @@ func testAccCheckRouteTableAssociationDestroy(s *terraform.State) error { return err } - if len(routeTable.Associations) > 0 { - return fmt.Errorf("Route table %s has associations", aws.StringValue(routeTable.RouteTableId)) - } + return fmt.Errorf("Route table association %s still exists", rs.Primary.ID) } return nil } -func testAccCheckRouteTableAssociationExists(n string, rta *ec2.RouteTableAssociation) resource.TestCheckFunc { +func testAccCheckRouteTableAssociationExists(n string, v *ec2.RouteTableAssociation) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -272,21 +202,15 @@ func testAccCheckRouteTableAssociationExists(n string, rta *ec2.RouteTableAssoci conn := testAccProvider.Meta().(*AWSClient).ec2conn - routeTable, err := finder.RouteTableByID(conn, rs.Primary.Attributes["route_table_id"]) + association, err := finder.RouteTableAssociationByID(conn, rs.Primary.ID) if err != nil { return err } - for _, association := range routeTable.Associations { - if rs.Primary.ID == aws.StringValue(association.RouteTableAssociationId) { - *rta = *association - - return nil - } - } + *v = *association - return fmt.Errorf("Association %q not found on Route Table %q", rs.Primary.ID, rs.Primary.Attributes["route_table_id"]) + return nil } } @@ -318,205 +242,135 @@ func testAccAWSRouteTabAssocImportStateIdFunc(resourceName string) resource.Impo } } -const testAccRouteTableAssociationSubnetConfig = testAccRouteTableAssociatonCommonVpcConfig + ` -resource "aws_route_table" "foo" { - vpc_id = aws_vpc.foo.id - - route { - cidr_block = "10.0.0.0/8" - gateway_id = aws_internet_gateway.foo.id - } +func testAccRouteTableAssociationConfigBaseVPC(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { + cidr_block = "10.1.0.0/16" tags = { - Name = "tf-acc-route-table-assoc" + Name = %[1]q } } -resource "aws_route_table_association" "foo" { - route_table_id = aws_route_table.foo.id - subnet_id = aws_subnet.foo.id -} -` - -const testAccRouteTableAssociationSubnetConfig_ChangeRouteTable = testAccRouteTableAssociatonCommonVpcConfig + ` -resource "aws_route_table" "bar" { - vpc_id = aws_vpc.foo.id +resource "aws_subnet" "test" { + vpc_id = aws_vpc.test.id + cidr_block = "10.1.1.0/24" - route { - cidr_block = "10.0.0.0/8" - gateway_id = aws_internet_gateway.foo.id + tags = { + Name = %[1]q } +} + +resource "aws_internet_gateway" "test" { + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-route-update-table-assoc" + Name = %[1]q } } - -resource "aws_route_table_association" "foo" { - route_table_id = aws_route_table.bar.id - subnet_id = aws_subnet.foo.id +`, rName) } -` -const testAccRouteTableAssociationSubnetConfig_ChangeSubnet = testAccRouteTableAssociatonCommonVpcConfig + ` -resource "aws_route_table" "foo" { - vpc_id = aws_vpc.foo.id +func testAccRouteTableAssociationConfigSubnet(rName string) string { + return composeConfig(testAccRouteTableAssociationConfigBaseVPC(rName), fmt.Sprintf(` +resource "aws_route_table" "test" { + vpc_id = aws_vpc.test.id route { cidr_block = "10.0.0.0/8" - gateway_id = aws_internet_gateway.foo.id + gateway_id = aws_internet_gateway.test.id } tags = { - Name = "tf-acc-route-table-assoc" + Name = %[1]q } } -resource "aws_subnet" "bar" { - vpc_id = aws_vpc.foo.id - cidr_block = "10.1.2.0/24" - - tags = { - Name = "tf-acc-route-table-assoc" - } +resource "aws_route_table_association" "test" { + route_table_id = aws_route_table.test.id + subnet_id = aws_subnet.test.id } - -resource "aws_route_table_association" "foo" { - route_table_id = aws_route_table.foo.id - subnet_id = aws_subnet.bar.id +`, rName)) } -` -const testAccRouteTableAssociationGatewayConfig = testAccRouteTableAssociatonCommonVpcConfig + ` -resource "aws_route_table" "foo" { - vpc_id = aws_vpc.foo.id +func testAccRouteTableAssociationConfigSubnetChangeRouteTable(rName string) string { + return composeConfig(testAccRouteTableAssociationConfigBaseVPC(rName), fmt.Sprintf(` +resource "aws_route_table" "test2" { + vpc_id = aws_vpc.test.id route { - cidr_block = aws_subnet.foo.cidr_block - network_interface_id = aws_network_interface.appliance.id - } - - tags = { - Name = "tf-acc-route-table-assoc" + cidr_block = "10.0.0.0/8" + gateway_id = aws_internet_gateway.test.id } -} - -resource "aws_subnet" "appliance" { - vpc_id = aws_vpc.foo.id - cidr_block = "10.1.2.0/24" tags = { - Name = "tf-acc-route-table-assoc" + Name = %[1]q } } -resource "aws_network_interface" "appliance" { - subnet_id = aws_subnet.appliance.id +resource "aws_route_table_association" "test" { + route_table_id = aws_route_table.test2.id + subnet_id = aws_subnet.test.id } - -resource "aws_route_table_association" "foo" { - route_table_id = aws_route_table.foo.id - gateway_id = aws_internet_gateway.foo.id +`, rName)) } -` -const testAccRouteTableAssociationGatewayConfig_ChangeRouteTable = testAccRouteTableAssociatonCommonVpcConfig + ` -resource "aws_route_table" "bar" { - vpc_id = aws_vpc.foo.id +func testAccRouteTableAssociationConfigGateway(rName string) string { + return composeConfig(testAccRouteTableAssociationConfigBaseVPC(rName), fmt.Sprintf(` +resource "aws_route_table" "test" { + vpc_id = aws_vpc.test.id route { - cidr_block = aws_subnet.foo.cidr_block - network_interface_id = aws_network_interface.appliance.id + cidr_block = aws_subnet.test.cidr_block + network_interface_id = aws_network_interface.test.id } tags = { - Name = "tf-acc-route-table-assoc" + Name = %[1]q } } -resource "aws_subnet" "appliance" { - vpc_id = aws_vpc.foo.id - cidr_block = "10.1.2.0/24" +resource "aws_network_interface" "test" { + subnet_id = aws_subnet.test.id tags = { - Name = "tf-acc-route-table-assoc" + Name = %[1]q } } -resource "aws_network_interface" "appliance" { - subnet_id = aws_subnet.appliance.id +resource "aws_route_table_association" "test" { + route_table_id = aws_route_table.test.id + gateway_id = aws_internet_gateway.test.id } - -resource "aws_route_table_association" "foo" { - route_table_id = aws_route_table.bar.id - gateway_id = aws_internet_gateway.foo.id +`, rName)) } -` -const testAccRouteTableAssociationGatewayConfig_ChangeGateway = testAccRouteTableAssociatonCommonVpcConfig + ` -resource "aws_route_table" "foo" { - vpc_id = aws_vpc.foo.id +func testAccRouteTableAssociationConfigGatewayChangeRouteTable(rName string) string { + return composeConfig(testAccRouteTableAssociationConfigBaseVPC(rName), fmt.Sprintf(` +resource "aws_route_table" "test2" { + vpc_id = aws_vpc.test.id route { - cidr_block = aws_subnet.foo.cidr_block - network_interface_id = aws_network_interface.appliance.id - } - - tags = { - Name = "tf-acc-route-table-assoc" - } -} - -resource "aws_subnet" "appliance" { - vpc_id = aws_vpc.foo.id - cidr_block = "10.1.2.0/24" - - tags = { - Name = "tf-acc-route-table-assoc" + cidr_block = aws_subnet.test.cidr_block + network_interface_id = aws_network_interface.test.id } -} - -resource "aws_network_interface" "appliance" { - subnet_id = aws_subnet.appliance.id -} - -resource "aws_vpn_gateway" "bar" { - vpc_id = aws_vpc.foo.id tags = { - Name = "tf-acc-route-table-assoc" + Name = %[1]q } } -resource "aws_route_table_association" "foo" { - route_table_id = aws_route_table.foo.id - gateway_id = aws_vpn_gateway.bar.id -} -` - -const testAccRouteTableAssociatonCommonVpcConfig = ` -resource "aws_vpc" "foo" { - cidr_block = "10.1.0.0/16" +resource "aws_network_interface" "test" { + subnet_id = aws_subnet.test.id tags = { - Name = "tf-acc-route-table-assoc" + Name = %[1]q } } -resource "aws_subnet" "foo" { - vpc_id = aws_vpc.foo.id - cidr_block = "10.1.1.0/24" - - tags = { - Name = "tf-acc-route-table-assoc" - } +resource "aws_route_table_association" "test" { + route_table_id = aws_route_table.test2.id + gateway_id = aws_internet_gateway.test.id } - -resource "aws_internet_gateway" "foo" { - vpc_id = aws_vpc.foo.id - - tags = { - Name = "tf-acc-route-table-assoc" - } +`, rName)) } -` From 67c069f1d1b56cd63807c2c5aa05d2edf2cd4eca Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 14 Jun 2021 17:47:49 -0400 Subject: [PATCH 19/31] r/aws_main_route_table_association: Add waiters. Acceptance test output: % make testacc TEST=./aws TESTARGS='-run=TestAccAWSMainRouteTableAssociation_' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSMainRouteTableAssociation_ -timeout 180m === RUN TestAccAWSMainRouteTableAssociation_basic === PAUSE TestAccAWSMainRouteTableAssociation_basic === CONT TestAccAWSMainRouteTableAssociation_basic --- PASS: TestAccAWSMainRouteTableAssociation_basic (61.64s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 65.154s --- aws/internal/service/ec2/finder/finder.go | 40 +++++ ...source_aws_main_route_table_association.go | 158 ++++++++---------- ...e_aws_main_route_table_association_test.go | 143 ++++++++-------- ...source_aws_route_table_association_test.go | 12 -- 4 files changed, 177 insertions(+), 176 deletions(-) diff --git a/aws/internal/service/ec2/finder/finder.go b/aws/internal/service/ec2/finder/finder.go index 4bf913ffca1..19c803d940f 100644 --- a/aws/internal/service/ec2/finder/finder.go +++ b/aws/internal/service/ec2/finder/finder.go @@ -235,6 +235,46 @@ func NetworkInterfaceSecurityGroup(conn *ec2.EC2, networkInterfaceID string, sec return result, err } +// MainRouteTableAssociationByID returns the main route table association corresponding to the specified identifier. +// Returns NotFoundError if no route table association is found. +func MainRouteTableAssociationByID(conn *ec2.EC2, associationID string) (*ec2.RouteTableAssociation, error) { + association, err := RouteTableAssociationByID(conn, associationID) + + if err != nil { + return nil, err + } + + if !aws.BoolValue(association.Main) { + return nil, &resource.NotFoundError{ + Message: fmt.Sprintf("%s is not the association with the main route table", associationID), + } + } + + return association, err +} + +// MainRouteTableAssociationByVpcID returns the main route table association for the specified VPC. +// Returns NotFoundError if no route table association is found. +func MainRouteTableAssociationByVpcID(conn *ec2.EC2, vpcID string) (*ec2.RouteTableAssociation, error) { + routeTable, err := MainRouteTableByVpcID(conn, vpcID) + + if err != nil { + return nil, err + } + + for _, association := range routeTable.Associations { + if aws.BoolValue(association.Main) { + if state := aws.StringValue(association.AssociationState.State); state == ec2.RouteTableAssociationStateCodeDisassociated { + continue + } + + return association, nil + } + } + + return nil, &resource.NotFoundError{} +} + // RouteTableAssociationByID returns the route table association corresponding to the specified identifier. // Returns NotFoundError if no route table association is found. func RouteTableAssociationByID(conn *ec2.EC2, associationID string) (*ec2.RouteTableAssociation, error) { diff --git a/aws/resource_aws_main_route_table_association.go b/aws/resource_aws_main_route_table_association.go index a3f08b082e7..e8a3b955fd7 100644 --- a/aws/resource_aws_main_route_table_association.go +++ b/aws/resource_aws_main_route_table_association.go @@ -7,6 +7,9 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func resourceAwsMainRouteTableAssociation() *schema.Resource { @@ -17,9 +20,13 @@ func resourceAwsMainRouteTableAssociation() *schema.Resource { Delete: resourceAwsMainRouteTableAssociationDelete, Schema: map[string]*schema.Schema{ - "vpc_id": { + // We use this field to record the main route table that is automatically + // created when the VPC is created. We need this to be able to "destroy" + // our main route table association, which we do by returning this route + // table to its original place as the Main Route Table for the VPC. + "original_route_table_id": { Type: schema.TypeString, - Required: true, + Computed: true, }, "route_table_id": { @@ -27,13 +34,9 @@ func resourceAwsMainRouteTableAssociation() *schema.Resource { Required: true, }, - // We use this field to record the main route table that is automatically - // created when the VPC is created. We need this to be able to "destroy" - // our main route table association, which we do by returning this route - // table to its original place as the Main Route Table for the VPC. - "original_route_table_id": { + "vpc_id": { Type: schema.TypeString, - Computed: true, + Required: true, }, }, } @@ -41,134 +44,105 @@ func resourceAwsMainRouteTableAssociation() *schema.Resource { func resourceAwsMainRouteTableAssociationCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - vpcId := d.Get("vpc_id").(string) - routeTableId := d.Get("route_table_id").(string) - log.Printf("[INFO] Creating main route table association: %s => %s", vpcId, routeTableId) + vpcID := d.Get("vpc_id").(string) - mainAssociation, err := findMainRouteTableAssociation(conn, vpcId) + association, err := finder.MainRouteTableAssociationByVpcID(conn, vpcID) if err != nil { - return fmt.Errorf("error finding EC2 VPC (%s) main route table association for replacement: %w", vpcId, err) + return fmt.Errorf("error reading Main Route Table Association (%s): %w", vpcID, err) } - if mainAssociation == nil { - return fmt.Errorf("error finding EC2 VPC (%s) main route table association for replacement: association not found", vpcId) + routeTableID := d.Get("route_table_id").(string) + input := &ec2.ReplaceRouteTableAssociationInput{ + AssociationId: association.RouteTableAssociationId, + RouteTableId: aws.String(routeTableID), } - resp, err := conn.ReplaceRouteTableAssociation(&ec2.ReplaceRouteTableAssociationInput{ - AssociationId: mainAssociation.RouteTableAssociationId, - RouteTableId: aws.String(routeTableId), - }) + log.Printf("[DEBUG] Creating Main Route Table Association: %s", input) + output, err := conn.ReplaceRouteTableAssociation(input) + if err != nil { - return err + return fmt.Errorf("error creating Main Route Table Association (%s): %w", routeTableID, err) } - d.Set("original_route_table_id", mainAssociation.RouteTableId) - d.SetId(aws.StringValue(resp.NewAssociationId)) - log.Printf("[INFO] New main route table association ID: %s", d.Id()) + d.SetId(aws.StringValue(output.NewAssociationId)) - return nil + log.Printf("[DEBUG] Waiting for Main Route Table Association (%s) creation", d.Id()) + if _, err := waiter.RouteTableAssociationUpdated(conn, d.Id()); err != nil { + return fmt.Errorf("error waiting for Main Route Table Association (%s) create: %w", d.Id(), err) + } + + d.Set("original_route_table_id", association.RouteTableId) + + return resourceAwsMainRouteTableAssociationRead(d, meta) } func resourceAwsMainRouteTableAssociationRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - mainAssociation, err := findMainRouteTableAssociation( - conn, - d.Get("vpc_id").(string)) - if err != nil { - return err - } + _, err := finder.MainRouteTableAssociationByID(conn, d.Id()) - if mainAssociation == nil || *mainAssociation.RouteTableAssociationId != d.Id() { - // It seems it doesn't exist anymore, so clear the ID + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] Main Route Table Association (%s) not found, removing from state", d.Id()) d.SetId("") + return nil + } + + if err != nil { + return fmt.Errorf("error reading Main Route Table Association (%s): %w", d.Id(), err) } return nil } -// Update is almost exactly like Create, except we want to retain the -// original_route_table_id - this needs to stay recorded as the AWS-created -// table from VPC creation. func resourceAwsMainRouteTableAssociationUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - vpcId := d.Get("vpc_id").(string) - routeTableId := d.Get("route_table_id").(string) - log.Printf("[INFO] Updating main route table association: %s => %s", vpcId, routeTableId) - - resp, err := conn.ReplaceRouteTableAssociation(&ec2.ReplaceRouteTableAssociationInput{ + routeTableID := d.Get("route_table_id").(string) + input := &ec2.ReplaceRouteTableAssociationInput{ AssociationId: aws.String(d.Id()), - RouteTableId: aws.String(routeTableId), - }) + RouteTableId: aws.String(routeTableID), + } + + log.Printf("[DEBUG] Updating Main Route Table Association: %s", input) + output, err := conn.ReplaceRouteTableAssociation(input) + if err != nil { - return err + return fmt.Errorf("error updating Main Route Table Association (%s): %w", routeTableID, err) } - d.SetId(aws.StringValue(resp.NewAssociationId)) - log.Printf("[INFO] New main route table association ID: %s", d.Id()) + // This whole thing with the resource ID being changed on update seems unsustainable. + // Keeping it here for backwards compatibilty... + d.SetId(aws.StringValue(output.NewAssociationId)) - return nil + log.Printf("[DEBUG] Waiting for Main Route Table Association (%s) update", d.Id()) + if _, err := waiter.RouteTableAssociationUpdated(conn, d.Id()); err != nil { + return fmt.Errorf("error waiting for Main Route Table Association (%s) update: %w", d.Id(), err) + } + + return resourceAwsMainRouteTableAssociationRead(d, meta) } func resourceAwsMainRouteTableAssociationDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - vpcId := d.Get("vpc_id").(string) - originalRouteTableId := d.Get("original_route_table_id").(string) - - log.Printf("[INFO] Deleting main route table association by resetting Main Route Table for VPC: %s to its original Route Table: %s", - vpcId, - originalRouteTableId) - resp, err := conn.ReplaceRouteTableAssociation(&ec2.ReplaceRouteTableAssociationInput{ + input := &ec2.ReplaceRouteTableAssociationInput{ AssociationId: aws.String(d.Id()), - RouteTableId: aws.String(originalRouteTableId), - }) - if err != nil { - return err + RouteTableId: aws.String(d.Get("original_route_table_id").(string)), } - log.Printf("[INFO] Resulting Association ID: %s", *resp.NewAssociationId) + log.Printf("[DEBUG] Deleting Main Route Table Association: %s", input) + output, err := conn.ReplaceRouteTableAssociation(input) - return nil -} - -func findMainRouteTableAssociation(conn *ec2.EC2, vpcId string) (*ec2.RouteTableAssociation, error) { - mainRouteTable, err := findMainRouteTable(conn, vpcId) if err != nil { - return nil, err - } - if mainRouteTable == nil { - return nil, nil + return fmt.Errorf("error deleting Main Route Table Association (%s): %w", d.Get("route_table_id").(string), err) } - for _, a := range mainRouteTable.Associations { - if *a.Main { - return a, nil - } + log.Printf("[DEBUG] Waiting for Main Route Table Association (%s) deletion", d.Id()) + if _, err := waiter.RouteTableAssociationUpdated(conn, aws.StringValue(output.NewAssociationId)); err != nil { + return fmt.Errorf("error waiting for Main Route Table Association (%s) delete: %w", d.Id(), err) } - return nil, fmt.Errorf("Could not find main routing table association for VPC: %s", vpcId) -} -func findMainRouteTable(conn *ec2.EC2, vpcId string) (*ec2.RouteTable, error) { - mainFilter := &ec2.Filter{ - Name: aws.String("association.main"), - Values: []*string{aws.String("true")}, - } - vpcFilter := &ec2.Filter{ - Name: aws.String("vpc-id"), - Values: []*string{aws.String(vpcId)}, - } - routeResp, err := conn.DescribeRouteTables(&ec2.DescribeRouteTablesInput{ - Filters: []*ec2.Filter{mainFilter, vpcFilter}, - }) - if err != nil { - return nil, err - } else if len(routeResp.RouteTables) != 1 { - return nil, nil - } - - return routeResp.RouteTables[0], nil + return nil } diff --git a/aws/resource_aws_main_route_table_association_test.go b/aws/resource_aws_main_route_table_association_test.go index c511349fb00..83bbb135e53 100644 --- a/aws/resource_aws_main_route_table_association_test.go +++ b/aws/resource_aws_main_route_table_association_test.go @@ -4,13 +4,19 @@ import ( "fmt" "testing" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func TestAccAWSMainRouteTableAssociation_basic(t *testing.T) { + var rta ec2.RouteTableAssociation + resourceName := "aws_main_route_table_association.test" + rName := acctest.RandomWithPrefix("tf-acc-test") + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, ErrorCheck: testAccErrorCheck(t, ec2.EndpointsID), @@ -18,15 +24,15 @@ func TestAccAWSMainRouteTableAssociation_basic(t *testing.T) { CheckDestroy: testAccCheckMainRouteTableAssociationDestroy, Steps: []resource.TestStep{ { - Config: testAccMainRouteTableAssociationConfig, + Config: testAccMainRouteTableAssociationConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckMainRouteTableAssociation("aws_main_route_table_association.foo", "aws_vpc.foo"), + testAccCheckMainRouteTableAssociationExists(resourceName, &rta), ), }, { - Config: testAccMainRouteTableAssociationConfigUpdate, + Config: testAccMainRouteTableAssociationConfigUpdated(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckMainRouteTableAssociation("aws_main_route_table_association.foo", "aws_vpc.foo"), + testAccCheckMainRouteTableAssociationExists(resourceName, &rta), ), }, }, @@ -41,139 +47,132 @@ func testAccCheckMainRouteTableAssociationDestroy(s *terraform.State) error { continue } - mainAssociation, err := findMainRouteTableAssociation( - conn, - rs.Primary.Attributes["vpc_id"], - ) + _, err := finder.MainRouteTableAssociationByID(conn, rs.Primary.ID) + + if tfresource.NotFound(err) { + continue + } + if err != nil { - // Verify the error is what we want - if ae, ok := err.(awserr.Error); ok && ae.Code() == "ApplicationDoesNotExistException" { - continue - } return err } - if mainAssociation != nil { - return fmt.Errorf("still exists") - } + return fmt.Errorf("Main route table association %s still exists", rs.Primary.ID) } return nil } -func testAccCheckMainRouteTableAssociation(mainRouteTableAssociationResource string, vpcResource string) resource.TestCheckFunc { +func testAccCheckMainRouteTableAssociationExists(n string, v *ec2.RouteTableAssociation) resource.TestCheckFunc { return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[mainRouteTableAssociationResource] + rs, ok := s.RootModule().Resources[n] if !ok { - return fmt.Errorf("Not found: %s", mainRouteTableAssociationResource) + return fmt.Errorf("Not found: %s", n) } if rs.Primary.ID == "" { return fmt.Errorf("No ID is set") } - vpc, ok := s.RootModule().Resources[vpcResource] - if !ok { - return fmt.Errorf("Not found: %s", vpcResource) - } - conn := testAccProvider.Meta().(*AWSClient).ec2conn - mainAssociation, err := findMainRouteTableAssociation(conn, vpc.Primary.ID) + + association, err := finder.MainRouteTableAssociationByID(conn, rs.Primary.ID) + if err != nil { return err } - if *mainAssociation.RouteTableAssociationId != rs.Primary.ID { - return fmt.Errorf("Found wrong main association: %s", - *mainAssociation.RouteTableAssociationId) - } + *v = *association return nil } } -const testAccMainRouteTableAssociationConfig = ` -resource "aws_vpc" "foo" { +func testAccMainRouteTableAssociationConfigBaseVPC(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-main-route-table-association" + Name = %[1]q } } -resource "aws_subnet" "foo" { - vpc_id = aws_vpc.foo.id +resource "aws_subnet" "test" { + vpc_id = aws_vpc.test.id cidr_block = "10.1.1.0/24" tags = { - Name = "tf-acc-main-route-table-association" + Name = %[1]q } } -resource "aws_internet_gateway" "foo" { - vpc_id = aws_vpc.foo.id -} - -resource "aws_route_table" "foo" { - vpc_id = aws_vpc.foo.id +resource "aws_internet_gateway" "test" { + vpc_id = aws_vpc.test.id - route { - cidr_block = "10.0.0.0/8" - gateway_id = aws_internet_gateway.foo.id + tags = { + Name = %[1]q } } - -resource "aws_main_route_table_association" "foo" { - vpc_id = aws_vpc.foo.id - route_table_id = aws_route_table.foo.id +`, rName) } -` -const testAccMainRouteTableAssociationConfigUpdate = ` -resource "aws_vpc" "foo" { - cidr_block = "10.1.0.0/16" +func testAccMainRouteTableAssociationConfig(rName string) string { + return composeConfig(testAccMainRouteTableAssociationConfigBaseVPC(rName), fmt.Sprintf(` +resource "aws_route_table" "test" { + vpc_id = aws_vpc.test.id - tags = { - Name = "terraform-testacc-main-route-table-association-update" + route { + cidr_block = "10.0.0.0/8" + gateway_id = aws_internet_gateway.test.id } -} - -resource "aws_subnet" "foo" { - vpc_id = aws_vpc.foo.id - cidr_block = "10.1.1.0/24" tags = { - Name = "tf-acc-main-route-table-association-update" + Name = %[1]q } } -resource "aws_internet_gateway" "foo" { - vpc_id = aws_vpc.foo.id +resource "aws_main_route_table_association" "test" { + vpc_id = aws_vpc.test.id + route_table_id = aws_route_table.test.id +} +`, rName)) } +func testAccMainRouteTableAssociationConfigUpdated(rName string) string { + return composeConfig(testAccMainRouteTableAssociationConfigBaseVPC(rName), fmt.Sprintf(` # Need to keep the old route table around when we update the # main_route_table_association, otherwise Terraform will try to destroy the # route table too early, and will fail because it's still the main one -resource "aws_route_table" "foo" { - vpc_id = aws_vpc.foo.id +resource "aws_route_table" "test" { + vpc_id = aws_vpc.test.id route { cidr_block = "10.0.0.0/8" - gateway_id = aws_internet_gateway.foo.id + gateway_id = aws_internet_gateway.test.id + } + + tags = { + Name = %[1]q } } -resource "aws_route_table" "bar" { - vpc_id = aws_vpc.foo.id +resource "aws_route_table" "test2" { + vpc_id = aws_vpc.test.id route { cidr_block = "10.0.0.0/8" - gateway_id = aws_internet_gateway.foo.id + gateway_id = aws_internet_gateway.test.id + } + + tags = { + Name = %[1]q } } -resource "aws_main_route_table_association" "foo" { - vpc_id = aws_vpc.foo.id - route_table_id = aws_route_table.bar.id +resource "aws_main_route_table_association" "test" { + vpc_id = aws_vpc.test.id + route_table_id = aws_route_table.test2.id +} +`, rName)) } -` diff --git a/aws/resource_aws_route_table_association_test.go b/aws/resource_aws_route_table_association_test.go index e0017635d4e..44693350e37 100644 --- a/aws/resource_aws_route_table_association_test.go +++ b/aws/resource_aws_route_table_association_test.go @@ -214,18 +214,6 @@ func testAccCheckRouteTableAssociationExists(n string, v *ec2.RouteTableAssociat } } -func testAccCheckRouteTableAssociationDisappears(rta *ec2.RouteTableAssociation) resource.TestCheckFunc { - return func(s *terraform.State) error { - conn := testAccProvider.Meta().(*AWSClient).ec2conn - - _, err := conn.DisassociateRouteTable(&ec2.DisassociateRouteTableInput{ - AssociationId: rta.RouteTableAssociationId, - }) - - return err - } -} - func testAccAWSRouteTabAssocImportStateIdFunc(resourceName string) resource.ImportStateIdFunc { return func(s *terraform.State) (string, error) { rs, ok := s.RootModule().Resources[resourceName] From af7c0267f41c013e9b52482023912b147afea18a Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 15 Jun 2021 10:15:49 -0400 Subject: [PATCH 20/31] Add 'tfresource.RetryUntilFound'. Test 'tfresource.RetryUntilFound'. --- aws/internal/tfresource/retry.go | 33 ++++++++++++++++ aws/internal/tfresource/retry_test.go | 56 +++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/aws/internal/tfresource/retry.go b/aws/internal/tfresource/retry.go index 517a6b21933..544b2d68879 100644 --- a/aws/internal/tfresource/retry.go +++ b/aws/internal/tfresource/retry.go @@ -7,6 +7,39 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) +// RetryUntilFound retries the specified function until the underlying resource is found. +// The function returns a resource.NotFoundError to indicate that the underlying resource does not exist. +// If the retries time out, the function is called one last time. +func RetryUntilFound(timeout time.Duration, f func() (interface{}, error)) (interface{}, error) { + var output interface{} + + err := resource.Retry(timeout, func() *resource.RetryError { + var err error + + output, err = f() + + if NotFound(err) { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + + if TimedOut(err) { + output, err = f() + } + + if err != nil { + return nil, err + } + + return output, err +} + // RetryWhenAwsErrCodeEquals retries the specified function when it returns one of the specified AWS error code. func RetryWhenAwsErrCodeEquals(timeout time.Duration, f func() (interface{}, error), codes ...string) (interface{}, error) { var output interface{} diff --git a/aws/internal/tfresource/retry_test.go b/aws/internal/tfresource/retry_test.go index 8a396b9e3de..ecb8b14907f 100644 --- a/aws/internal/tfresource/retry_test.go +++ b/aws/internal/tfresource/retry_test.go @@ -7,9 +7,65 @@ import ( "time" "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) +func TestRetryUntilFound(t *testing.T) { + var retryCount int32 + + testCases := []struct { + Name string + F func() (interface{}, error) + ExpectError bool + }{ + { + Name: "no error", + F: func() (interface{}, error) { + return nil, nil + }, + }, + { + Name: "non-retryable other error", + F: func() (interface{}, error) { + return nil, errors.New("TestCode") + }, + ExpectError: true, + }, + { + Name: "retryable not-found error timeout", + F: func() (interface{}, error) { + return nil, &resource.NotFoundError{} + }, + ExpectError: true, + }, + { + Name: "retryable AWS error success", + F: func() (interface{}, error) { + if atomic.CompareAndSwapInt32(&retryCount, 0, 1) { + return nil, &resource.NotFoundError{} + } + + return nil, nil + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Name, func(t *testing.T) { + retryCount = 0 + + _, err := tfresource.RetryUntilFound(5*time.Second, testCase.F) + + if testCase.ExpectError && err == nil { + t.Fatal("expected error") + } else if !testCase.ExpectError && err != nil { + t.Fatalf("unexpected error: %s", err) + } + }) + } +} + func TestRetryWhenAwsErrCodeEquals(t *testing.T) { var retryCount int32 From 081aebdba912a7d9cb623fd0c8ad5cee3e1b1a46 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 15 Jun 2021 10:24:23 -0400 Subject: [PATCH 21/31] Revert "Add 'tfresource.RetryUntilFound'." This reverts commit af7c0267f41c013e9b52482023912b147afea18a. --- aws/internal/tfresource/retry.go | 33 ---------------- aws/internal/tfresource/retry_test.go | 56 --------------------------- 2 files changed, 89 deletions(-) diff --git a/aws/internal/tfresource/retry.go b/aws/internal/tfresource/retry.go index 544b2d68879..517a6b21933 100644 --- a/aws/internal/tfresource/retry.go +++ b/aws/internal/tfresource/retry.go @@ -7,39 +7,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) -// RetryUntilFound retries the specified function until the underlying resource is found. -// The function returns a resource.NotFoundError to indicate that the underlying resource does not exist. -// If the retries time out, the function is called one last time. -func RetryUntilFound(timeout time.Duration, f func() (interface{}, error)) (interface{}, error) { - var output interface{} - - err := resource.Retry(timeout, func() *resource.RetryError { - var err error - - output, err = f() - - if NotFound(err) { - return resource.RetryableError(err) - } - - if err != nil { - return resource.NonRetryableError(err) - } - - return nil - }) - - if TimedOut(err) { - output, err = f() - } - - if err != nil { - return nil, err - } - - return output, err -} - // RetryWhenAwsErrCodeEquals retries the specified function when it returns one of the specified AWS error code. func RetryWhenAwsErrCodeEquals(timeout time.Duration, f func() (interface{}, error), codes ...string) (interface{}, error) { var output interface{} diff --git a/aws/internal/tfresource/retry_test.go b/aws/internal/tfresource/retry_test.go index ecb8b14907f..8a396b9e3de 100644 --- a/aws/internal/tfresource/retry_test.go +++ b/aws/internal/tfresource/retry_test.go @@ -7,65 +7,9 @@ import ( "time" "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) -func TestRetryUntilFound(t *testing.T) { - var retryCount int32 - - testCases := []struct { - Name string - F func() (interface{}, error) - ExpectError bool - }{ - { - Name: "no error", - F: func() (interface{}, error) { - return nil, nil - }, - }, - { - Name: "non-retryable other error", - F: func() (interface{}, error) { - return nil, errors.New("TestCode") - }, - ExpectError: true, - }, - { - Name: "retryable not-found error timeout", - F: func() (interface{}, error) { - return nil, &resource.NotFoundError{} - }, - ExpectError: true, - }, - { - Name: "retryable AWS error success", - F: func() (interface{}, error) { - if atomic.CompareAndSwapInt32(&retryCount, 0, 1) { - return nil, &resource.NotFoundError{} - } - - return nil, nil - }, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.Name, func(t *testing.T) { - retryCount = 0 - - _, err := tfresource.RetryUntilFound(5*time.Second, testCase.F) - - if testCase.ExpectError && err == nil { - t.Fatal("expected error") - } else if !testCase.ExpectError && err != nil { - t.Fatalf("unexpected error: %s", err) - } - }) - } -} - func TestRetryWhenAwsErrCodeEquals(t *testing.T) { var retryCount int32 From 3a96fb1d3ea2b5cbcc8644d9394c156337fcf85e Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 15 Jun 2021 11:19:50 -0400 Subject: [PATCH 22/31] VPC Endpoint changes from #13737. --- aws/internal/service/ec2/enum.go | 12 + aws/internal/service/ec2/errors.go | 5 +- aws/internal/service/ec2/finder/finder.go | 87 ++++-- aws/internal/service/ec2/id.go | 8 + aws/internal/service/ec2/waiter/status.go | 16 ++ aws/internal/service/ec2/waiter/waiter.go | 62 +++++ aws/resource_aws_vpc_endpoint.go | 116 ++------ ...ws_vpc_endpoint_route_table_association.go | 115 +++----- ...c_endpoint_route_table_association_test.go | 86 +++--- ...rce_aws_vpc_endpoint_subnet_association.go | 105 ++++---- ...ws_vpc_endpoint_subnet_association_test.go | 202 ++++++-------- aws/resource_aws_vpc_endpoint_test.go | 255 ++++-------------- 12 files changed, 454 insertions(+), 615 deletions(-) create mode 100644 aws/internal/service/ec2/enum.go diff --git a/aws/internal/service/ec2/enum.go b/aws/internal/service/ec2/enum.go new file mode 100644 index 00000000000..e7bfd126952 --- /dev/null +++ b/aws/internal/service/ec2/enum.go @@ -0,0 +1,12 @@ +package ec2 + +const ( + // https://docs.aws.amazon.com/vpc/latest/privatelink/vpce-interface.html#vpce-interface-lifecycle + VpcEndpointStateAvailable = "available" + VpcEndpointStateDeleted = "deleted" + VpcEndpointStateDeleting = "deleting" + VpcEndpointStateFailed = "failed" + VpcEndpointStatePending = "pending" + VpcEndpointStatePendingAcceptance = "pendingAcceptance" + VpcEndpointStateRejected = "rejected" +) diff --git a/aws/internal/service/ec2/errors.go b/aws/internal/service/ec2/errors.go index f4f93f28cd1..cbcac12e097 100644 --- a/aws/internal/service/ec2/errors.go +++ b/aws/internal/service/ec2/errors.go @@ -5,12 +5,13 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" - "github.com/hashicorp/go-multierror" + multierror "github.com/hashicorp/go-multierror" ) const ( ErrCodeGatewayNotAttached = "Gateway.NotAttached" ErrCodeInvalidAssociationIDNotFound = "InvalidAssociationID.NotFound" + ErrCodeInvalidParameter = "InvalidParameter" ErrCodeInvalidParameterException = "InvalidParameterException" ErrCodeInvalidParameterValue = "InvalidParameterValue" ) @@ -29,6 +30,7 @@ const ( const ( ErrCodeInvalidRouteNotFound = "InvalidRoute.NotFound" + ErrCodeInvalidRouteTableIdNotFound = "InvalidRouteTableId.NotFound" ErrCodeInvalidRouteTableIDNotFound = "InvalidRouteTableID.NotFound" ) @@ -57,6 +59,7 @@ const ( ) const ( + ErrCodeInvalidSubnetIdNotFound = "InvalidSubnetId.NotFound" ErrCodeInvalidSubnetIDNotFound = "InvalidSubnetID.NotFound" ) diff --git a/aws/internal/service/ec2/finder/finder.go b/aws/internal/service/ec2/finder/finder.go index 19c803d940f..81d99aff35b 100644 --- a/aws/internal/service/ec2/finder/finder.go +++ b/aws/internal/service/ec2/finder/finder.go @@ -619,59 +619,96 @@ func VpcByID(conn *ec2.EC2, id string) (*ec2.Vpc, error) { return nil, nil } -// VpcEndpointByID looks up a VpcEndpoint by ID. When not found, returns nil and potentially an API error. -func VpcEndpointByID(conn *ec2.EC2, id string) (*ec2.VpcEndpoint, error) { +// VpcEndpointByID returns the VPC endpoint corresponding to the specified identifier. +// Returns NotFoundError if no VPC endpoint is found. +func VpcEndpointByID(conn *ec2.EC2, vpcEndpointID string) (*ec2.VpcEndpoint, error) { input := &ec2.DescribeVpcEndpointsInput{ - VpcEndpointIds: aws.StringSlice([]string{id}), + VpcEndpointIds: aws.StringSlice([]string{vpcEndpointID}), } - output, err := conn.DescribeVpcEndpoints(input) + vpcEndpoint, err := VpcEndpoint(conn, input) if err != nil { return nil, err } - if output == nil { - return nil, nil - } - - for _, vpcEndpoint := range output.VpcEndpoints { - if vpcEndpoint == nil { - continue + if state := aws.StringValue(vpcEndpoint.State); state == tfec2.VpcEndpointStateDeleted { + return nil, &resource.NotFoundError{ + Message: state, + LastRequest: input, } + } - if aws.StringValue(vpcEndpoint.VpcEndpointId) != id { - continue + // Eventual consistency check. + if aws.StringValue(vpcEndpoint.VpcEndpointId) != vpcEndpointID { + return nil, &resource.NotFoundError{ + LastRequest: input, } - - return vpcEndpoint, nil } - return nil, nil + return vpcEndpoint, nil } -// VpcEndpointRouteTableAssociation returns the associated Route Table ID if found -func VpcEndpointRouteTableAssociation(conn *ec2.EC2, vpcEndpointID string, routeTableID string) (*string, error) { - var result *string +func VpcEndpoint(conn *ec2.EC2, input *ec2.DescribeVpcEndpointsInput) (*ec2.VpcEndpoint, error) { + output, err := conn.DescribeVpcEndpoints(input) - vpcEndpoint, err := VpcEndpointByID(conn, vpcEndpointID) + if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidVpcEndpointIdNotFound) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: input, + } + } if err != nil { return nil, err } - if vpcEndpoint == nil { - return nil, nil + if output == nil || len(output.VpcEndpoints) == 0 || output.VpcEndpoints[0] == nil { + return nil, &resource.NotFoundError{ + Message: "Empty result", + LastRequest: input, + } + } + + return output.VpcEndpoints[0], nil +} + +// VpcEndpointRouteTableAssociationExists returns NotFoundError if no association for the specified VPC endpoint and route table IDs is found. +func VpcEndpointRouteTableAssociationExists(conn *ec2.EC2, vpcEndpointID string, routeTableID string) error { + vpcEndpoint, err := VpcEndpointByID(conn, vpcEndpointID) + + if err != nil { + return err } for _, vpcEndpointRouteTableID := range vpcEndpoint.RouteTableIds { if aws.StringValue(vpcEndpointRouteTableID) == routeTableID { - result = vpcEndpointRouteTableID - break + return nil } } - return result, err + return &resource.NotFoundError{ + LastError: fmt.Errorf("VPC Endpoint Route Table Association (%s/%s) not found", vpcEndpointID, routeTableID), + } +} + +// VpcEndpointSubnetAssociationExists returns NotFoundError if no association for the specified VPC endpoint and subnet IDs is found. +func VpcEndpointSubnetAssociationExists(conn *ec2.EC2, vpcEndpointID string, subnetID string) error { + vpcEndpoint, err := VpcEndpointByID(conn, vpcEndpointID) + + if err != nil { + return err + } + + for _, vpcEndpointSubnetID := range vpcEndpoint.SubnetIds { + if aws.StringValue(vpcEndpointSubnetID) == subnetID { + return nil + } + } + + return &resource.NotFoundError{ + LastError: fmt.Errorf("VPC Endpoint Subnet Association (%s/%s) not found", vpcEndpointID, subnetID), + } } // VpcPeeringConnectionByID returns the VPC peering connection corresponding to the specified identifier. diff --git a/aws/internal/service/ec2/id.go b/aws/internal/service/ec2/id.go index c7fa5040e0f..cf5cb8a0cb3 100644 --- a/aws/internal/service/ec2/id.go +++ b/aws/internal/service/ec2/id.go @@ -95,6 +95,14 @@ func TransitGatewayPrefixListReferenceParseID(id string) (string, string, error) return "", "", fmt.Errorf("unexpected format for ID (%[1]s), expected transit-gateway-route-table-id%[2]sprefix-list-id", id, transitGatewayPrefixListReferenceSeparator) } +func VpcEndpointRouteTableAssociationCreateID(vpcEndpointID, routeTableID string) string { + return fmt.Sprintf("a-%s%d", vpcEndpointID, hashcode.String(routeTableID)) +} + +func VpcEndpointSubnetAssociationCreateID(vpcEndpointID, subnetID string) string { + return fmt.Sprintf("a-%s%d", vpcEndpointID, hashcode.String(subnetID)) +} + func VpnGatewayVpcAttachmentCreateID(vpnGatewayID, vpcID string) string { return fmt.Sprintf("vpn-attachment-%x", hashcode.String(fmt.Sprintf("%s-%s", vpcID, vpnGatewayID))) } diff --git a/aws/internal/service/ec2/waiter/status.go b/aws/internal/service/ec2/waiter/status.go index 8b8ffde410c..3fa0a512c85 100644 --- a/aws/internal/service/ec2/waiter/status.go +++ b/aws/internal/service/ec2/waiter/status.go @@ -504,3 +504,19 @@ func ManagedPrefixListState(conn *ec2.EC2, prefixListId string) resource.StateRe return managedPrefixList, aws.StringValue(managedPrefixList.State), nil } } + +func VpcEndpointState(conn *ec2.EC2, id string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + vpcEndpoint, err := finder.VpcEndpointByID(conn, id) + + if tfresource.NotFound(err) { + return nil, "", nil + } + + if err != nil { + return nil, "", err + } + + return vpcEndpoint, aws.StringValue(vpcEndpoint.State), nil + } +} diff --git a/aws/internal/service/ec2/waiter/waiter.go b/aws/internal/service/ec2/waiter/waiter.go index b119c2fdb77..6a55619627a 100644 --- a/aws/internal/service/ec2/waiter/waiter.go +++ b/aws/internal/service/ec2/waiter/waiter.go @@ -2,6 +2,7 @@ package waiter import ( "errors" + "fmt" "strconv" "time" @@ -689,3 +690,64 @@ func ManagedPrefixListDeleted(conn *ec2.EC2, prefixListId string) error { return nil } + +func VpcEndpointAccepted(conn *ec2.EC2, vpcEndpointID string, timeout time.Duration) (*ec2.VpcEndpoint, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{tfec2.VpcEndpointStatePendingAcceptance}, + Target: []string{tfec2.VpcEndpointStateAvailable}, + Timeout: timeout, + Refresh: VpcEndpointState(conn, vpcEndpointID), + Delay: 5 * time.Second, + MinTimeout: 5 * time.Second, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*ec2.VpcEndpoint); ok { + if output != nil && aws.StringValue(output.State) == tfec2.VpcEndpointStateFailed && output.LastError != nil { + tfresource.SetLastError(err, fmt.Errorf("%s: %s", aws.StringValue(output.LastError.Code), aws.StringValue(output.LastError.Message))) + } + + return output, err + } + + return nil, err +} + +func VpcEndpointAvailable(conn *ec2.EC2, vpcEndpointID string, timeout time.Duration) (*ec2.VpcEndpoint, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{tfec2.VpcEndpointStatePending}, + Target: []string{tfec2.VpcEndpointStateAvailable, tfec2.VpcEndpointStatePendingAcceptance}, + Timeout: timeout, + Refresh: VpcEndpointState(conn, vpcEndpointID), + Delay: 5 * time.Second, + MinTimeout: 5 * time.Second, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*ec2.VpcEndpoint); ok { + return output, err + } + + return nil, err +} + +func VpcEndpointDeleted(conn *ec2.EC2, vpcEndpointID string, timeout time.Duration) (*ec2.VpcEndpoint, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{tfec2.VpcEndpointStateDeleting}, + Target: []string{}, + Timeout: timeout, + Refresh: VpcEndpointState(conn, vpcEndpointID), + Delay: 5 * time.Second, + MinTimeout: 5 * time.Second, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*ec2.VpcEndpoint); ok { + return output, err + } + + return nil, err +} diff --git a/aws/resource_aws_vpc_endpoint.go b/aws/resource_aws_vpc_endpoint.go index 8c4a0a08f40..2a2c17deb0f 100644 --- a/aws/resource_aws_vpc_endpoint.go +++ b/aws/resource_aws_vpc_endpoint.go @@ -10,12 +10,14 @@ import ( "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/aws-sdk-go-base/tfawserr" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) const ( @@ -198,8 +200,10 @@ func resourceAwsVpcEndpointCreate(d *schema.ResourceData, meta interface{}) erro } } - if err := vpcEndpointWaitUntilAvailable(conn, d.Id(), d.Timeout(schema.TimeoutCreate)); err != nil { - return fmt.Errorf("error waiting for VPC Endpoint (%s) to become available: %s", d.Id(), err) + _, err = waiter.VpcEndpointAvailable(conn, d.Id(), d.Timeout(schema.TimeoutCreate)) + + if err != nil { + return fmt.Errorf("error waiting for VPC Endpoint (%s) to become available: %w", d.Id(), err) } return resourceAwsVpcEndpointRead(d, meta) @@ -210,25 +214,17 @@ func resourceAwsVpcEndpointRead(d *schema.ResourceData, meta interface{}) error defaultTagsConfig := meta.(*AWSClient).DefaultTagsConfig ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig - vpceRaw, state, err := vpcEndpointStateRefresh(conn, d.Id())() - if err != nil && state != "failed" { - return fmt.Errorf("error reading VPC Endpoint (%s): %s", d.Id(), err) - } + vpce, err := finder.VpcEndpointByID(conn, d.Id()) - terminalStates := map[string]bool{ - "deleted": true, - "deleting": true, - "failed": true, - "expired": true, - "rejected": true, - } - if _, ok := terminalStates[state]; ok { - log.Printf("[WARN] VPC Endpoint (%s) in state (%s), removing from state", d.Id(), state) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] VPC Endpoint (%s) not found, removing from state", d.Id()) d.SetId("") return nil } - vpce := vpceRaw.(*ec2.VpcEndpoint) + if err != nil { + return fmt.Errorf("error reading VPC Endpoint (%s): %w", d.Id(), err) + } arn := arn.ARN{ Partition: meta.(*AWSClient).partition, @@ -355,8 +351,10 @@ func resourceAwsVpcEndpointUpdate(d *schema.ResourceData, meta interface{}) erro return fmt.Errorf("Error updating VPC Endpoint: %s", err) } - if err := vpcEndpointWaitUntilAvailable(conn, d.Id(), d.Timeout(schema.TimeoutUpdate)); err != nil { - return err + _, err := waiter.VpcEndpointAvailable(conn, d.Id(), d.Timeout(schema.TimeoutUpdate)) + + if err != nil { + return fmt.Errorf("error waiting for VPC Endpoint (%s) to become available: %w", d.Id(), err) } } @@ -395,7 +393,9 @@ func resourceAwsVpcEndpointDelete(d *schema.ResourceData, meta interface{}) erro } } - if err := vpcEndpointWaitUntilDeleted(conn, d.Id(), d.Timeout(schema.TimeoutDelete)); err != nil { + _, err = waiter.VpcEndpointDeleted(conn, d.Id(), d.Timeout(schema.TimeoutDelete)) + + if err != nil { return fmt.Errorf("error waiting for EC2 VPC Endpoint (%s) to delete: %w", d.Id(), err) } @@ -429,87 +429,15 @@ func vpcEndpointAccept(conn *ec2.EC2, vpceId, svcName string, timeout time.Durat return fmt.Errorf("error accepting VPC Endpoint (%s) connection: %s", vpceId, err) } - stateConf := &resource.StateChangeConf{ - Pending: []string{"pendingAcceptance", "pending"}, - Target: []string{"available"}, - Refresh: vpcEndpointStateRefresh(conn, vpceId), - Timeout: timeout, - Delay: 5 * time.Second, - MinTimeout: 5 * time.Second, - } + _, err = waiter.VpcEndpointAccepted(conn, vpceId, timeout) - _, err = stateConf.WaitForState() if err != nil { - return fmt.Errorf("error waiting for VPC Endpoint (%s) to be accepted: %s", vpceId, err) + return fmt.Errorf("error waiting for VPC Endpoint (%s) to be accepted: %w", vpceId, err) } return nil } -func vpcEndpointStateRefresh(conn *ec2.EC2, vpceId string) resource.StateRefreshFunc { - return func() (interface{}, string, error) { - log.Printf("[DEBUG] Reading VPC Endpoint: %s", vpceId) - resp, err := conn.DescribeVpcEndpoints(&ec2.DescribeVpcEndpointsInput{ - VpcEndpointIds: aws.StringSlice([]string{vpceId}), - }) - if err != nil { - if isAWSErr(err, "InvalidVpcEndpointId.NotFound", "") { - return "", "deleted", nil - } - - return nil, "", err - } - - n := len(resp.VpcEndpoints) - switch n { - case 0: - return "", "deleted", nil - - case 1: - vpce := resp.VpcEndpoints[0] - state := aws.StringValue(vpce.State) - // No use in retrying if the endpoint is in a failed state. - if state == "failed" { - return nil, state, errors.New("VPC Endpoint is in a failed state") - } - return vpce, state, nil - - default: - return nil, "", fmt.Errorf("Found %d VPC Endpoints for %s, expected 1", n, vpceId) - } - } -} - -func vpcEndpointWaitUntilAvailable(conn *ec2.EC2, vpceId string, timeout time.Duration) error { - stateConf := &resource.StateChangeConf{ - Pending: []string{"pending"}, - Target: []string{"available", "pendingAcceptance"}, - Refresh: vpcEndpointStateRefresh(conn, vpceId), - Timeout: timeout, - Delay: 5 * time.Second, - MinTimeout: 5 * time.Second, - } - - _, err := stateConf.WaitForState() - - return err -} - -func vpcEndpointWaitUntilDeleted(conn *ec2.EC2, vpceID string, timeout time.Duration) error { - stateConf := &resource.StateChangeConf{ - Pending: []string{"available", "pending", "deleting"}, - Target: []string{"deleted"}, - Refresh: vpcEndpointStateRefresh(conn, vpceID), - Timeout: timeout, - Delay: 5 * time.Second, - MinTimeout: 5 * time.Second, - } - - _, err := stateConf.WaitForState() - - return err -} - func setVpcEndpointCreateList(d *schema.ResourceData, key string, c *[]*string) { if v, ok := d.GetOk(key); ok { list := v.(*schema.Set) diff --git a/aws/resource_aws_vpc_endpoint_route_table_association.go b/aws/resource_aws_vpc_endpoint_route_table_association.go index 5c88d02be7b..0e56c8669b2 100644 --- a/aws/resource_aws_vpc_endpoint_route_table_association.go +++ b/aws/resource_aws_vpc_endpoint_route_table_association.go @@ -6,12 +6,9 @@ import ( "strings" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/aws-sdk-go-base/tfawserr" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode" tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter" @@ -47,21 +44,23 @@ func resourceAwsVpcEndpointRouteTableAssociationCreate(d *schema.ResourceData, m endpointId := d.Get("vpc_endpoint_id").(string) rtId := d.Get("route_table_id").(string) + // Human friendly ID for error messages since d.Id() is non-descriptive + id := fmt.Sprintf("%s/%s", endpointId, rtId) - _, err := findResourceVpcEndpoint(conn, endpointId) - if err != nil { - return err - } - - _, err = conn.ModifyVpcEndpoint(&ec2.ModifyVpcEndpointInput{ + input := &ec2.ModifyVpcEndpointInput{ VpcEndpointId: aws.String(endpointId), AddRouteTableIds: aws.StringSlice([]string{rtId}), - }) + } + + log.Printf("[DEBUG] Creating VPC Endpoint Route Table Association: %s", input) + + _, err := conn.ModifyVpcEndpoint(input) + if err != nil { - return fmt.Errorf("Error creating VPC Endpoint/Route Table association: %s", err.Error()) + return fmt.Errorf("error creating VPC Endpoint Route Table Association (%s): %w", id, err) } - d.SetId(vpcEndpointIdRouteTableIdHash(endpointId, rtId)) + d.SetId(tfec2.VpcEndpointRouteTableAssociationCreateID(endpointId, rtId)) return resourceAwsVpcEndpointRouteTableAssociationRead(d, meta) } @@ -74,35 +73,17 @@ func resourceAwsVpcEndpointRouteTableAssociationRead(d *schema.ResourceData, met // Human friendly ID for error messages since d.Id() is non-descriptive id := fmt.Sprintf("%s/%s", endpointId, rtId) - var routeTableID *string - - err := resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError { - var err error - - routeTableID, err = finder.VpcEndpointRouteTableAssociation(conn, endpointId, rtId) - - if d.IsNewResource() && tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidVpcEndpointIdNotFound) { - return resource.RetryableError(err) - } + _, err := tfresource.RetryUntilFound(waiter.PropagationTimeout, d.IsNewResource(), func() (interface{}, error) { + err := finder.VpcEndpointRouteTableAssociationExists(conn, endpointId, rtId) if err != nil { - return resource.NonRetryableError(err) - } - - if d.IsNewResource() && routeTableID == nil { - return resource.RetryableError(&resource.NotFoundError{ - LastError: fmt.Errorf("VPC Endpoint Route Table Association (%s) not found", id), - }) + return nil, err } - return nil + return struct{}{}, nil }) - if tfresource.TimedOut(err) { - routeTableID, err = finder.VpcEndpointRouteTableAssociation(conn, endpointId, rtId) - } - - if d.IsNewResource() && tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidVpcEndpointIdNotFound) { + if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] VPC Endpoint Route Table Association (%s) not found, removing from state", id) d.SetId("") return nil @@ -112,16 +93,6 @@ func resourceAwsVpcEndpointRouteTableAssociationRead(d *schema.ResourceData, met return fmt.Errorf("error reading VPC Endpoint Route Table Association (%s): %w", id, err) } - if routeTableID == nil { - if d.IsNewResource() { - return fmt.Errorf("error reading VPC Endpoint Route Table Association (%s): not found after creation", id) - } - - log.Printf("[WARN] VPC Endpoint Route Table Association (%s) not found, removing from state", id) - d.SetId("") - return nil - } - return nil } @@ -130,27 +101,24 @@ func resourceAwsVpcEndpointRouteTableAssociationDelete(d *schema.ResourceData, m endpointId := d.Get("vpc_endpoint_id").(string) rtId := d.Get("route_table_id").(string) + // Human friendly ID for error messages since d.Id() is non-descriptive + id := fmt.Sprintf("%s/%s", endpointId, rtId) - _, err := conn.ModifyVpcEndpoint(&ec2.ModifyVpcEndpointInput{ + input := &ec2.ModifyVpcEndpointInput{ VpcEndpointId: aws.String(endpointId), RemoveRouteTableIds: aws.StringSlice([]string{rtId}), - }) - if err != nil { - ec2err, ok := err.(awserr.Error) - if !ok { - return fmt.Errorf("Error deleting VPC Endpoint/Route Table association: %s", err.Error()) - } + } - switch ec2err.Code() { - case "InvalidVpcEndpointId.NotFound": - fallthrough - case "InvalidRouteTableId.NotFound": - fallthrough - case "InvalidParameter": - log.Printf("[DEBUG] VPC Endpoint/Route Table association is already gone") - default: - return fmt.Errorf("Error deleting VPC Endpoint/Route Table association: %s", err.Error()) - } + log.Printf("[DEBUG] Deleting VPC Endpoint Route Table Association: %s", input) + + _, err := conn.ModifyVpcEndpoint(input) + + if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidVpcEndpointIdNotFound) || tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidRouteTableIdNotFound) || tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidParameter) { + return nil + } + + if err != nil { + return fmt.Errorf("error deleting VPC Endpoint Route Table Association (%s): %w", id, err) } return nil @@ -164,30 +132,11 @@ func resourceAwsVpcEndpointRouteTableAssociationImport(d *schema.ResourceData, m vpceId := parts[0] rtId := parts[1] - log.Printf("[DEBUG] Importing VPC Endpoint (%s) Route Table (%s) association", vpceId, rtId) + log.Printf("[DEBUG] Importing VPC Endpoint (%s) Route Table (%s) Association", vpceId, rtId) - d.SetId(vpcEndpointIdRouteTableIdHash(vpceId, rtId)) + d.SetId(tfec2.VpcEndpointRouteTableAssociationCreateID(vpceId, rtId)) d.Set("vpc_endpoint_id", vpceId) d.Set("route_table_id", rtId) return []*schema.ResourceData{d}, nil } - -func findResourceVpcEndpoint(conn *ec2.EC2, id string) (*ec2.VpcEndpoint, error) { - resp, err := conn.DescribeVpcEndpoints(&ec2.DescribeVpcEndpointsInput{ - VpcEndpointIds: aws.StringSlice([]string{id}), - }) - if err != nil { - return nil, err - } - - if resp.VpcEndpoints == nil || len(resp.VpcEndpoints) == 0 { - return nil, fmt.Errorf("No VPC Endpoints were found for %s", id) - } - - return resp.VpcEndpoints[0], nil -} - -func vpcEndpointIdRouteTableIdHash(endpointId, rtId string) string { - return fmt.Sprintf("a-%s%d", endpointId, hashcode.String(rtId)) -} diff --git a/aws/resource_aws_vpc_endpoint_route_table_association_test.go b/aws/resource_aws_vpc_endpoint_route_table_association_test.go index 4b5df488365..fcbe71b0a7d 100644 --- a/aws/resource_aws_vpc_endpoint_route_table_association_test.go +++ b/aws/resource_aws_vpc_endpoint_route_table_association_test.go @@ -4,18 +4,17 @@ import ( "fmt" "testing" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func TestAccAWSVpcEndpointRouteTableAssociation_basic(t *testing.T) { - var vpce ec2.VpcEndpoint resourceName := "aws_vpc_endpoint_route_table_association.test" - rName := fmt.Sprintf("tf-testacc-vpce-%s", acctest.RandString(16)) + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -26,7 +25,7 @@ func TestAccAWSVpcEndpointRouteTableAssociation_basic(t *testing.T) { { Config: testAccVpcEndpointRouteTableAssociationConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckVpcEndpointRouteTableAssociationExists(resourceName, &vpce), + testAccCheckVpcEndpointRouteTableAssociationExists(resourceName), ), }, { @@ -39,6 +38,28 @@ func TestAccAWSVpcEndpointRouteTableAssociation_basic(t *testing.T) { }) } +func TestAccAWSVpcEndpointRouteTableAssociation_disappears(t *testing.T) { + resourceName := "aws_vpc_endpoint_route_table_association.test" + rName := acctest.RandomWithPrefix("tf-acc-test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, ec2.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckVpcEndpointRouteTableAssociationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccVpcEndpointRouteTableAssociationConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckVpcEndpointRouteTableAssociationExists(resourceName), + testAccCheckResourceDisappears(testAccProvider, resourceAwsVpcEndpointRouteTableAssociation(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func testAccCheckVpcEndpointRouteTableAssociationDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).ec2conn @@ -47,33 +68,23 @@ func testAccCheckVpcEndpointRouteTableAssociationDestroy(s *terraform.State) err continue } - // Try to find the resource - resp, err := conn.DescribeVpcEndpoints(&ec2.DescribeVpcEndpointsInput{ - VpcEndpointIds: aws.StringSlice([]string{rs.Primary.Attributes["vpc_endpoint_id"]}), - }) - if err != nil { - // Verify the error is what we want - ec2err, ok := err.(awserr.Error) - if !ok { - return err - } - if ec2err.Code() != "InvalidVpcEndpointId.NotFound" { - return err - } - return nil + err := finder.VpcEndpointRouteTableAssociationExists(conn, rs.Primary.Attributes["vpc_endpoint_id"], rs.Primary.Attributes["route_table_id"]) + + if tfresource.NotFound(err) { + continue } - vpce := resp.VpcEndpoints[0] - if len(vpce.RouteTableIds) > 0 { - return fmt.Errorf( - "VPC endpoint %s has route tables", *vpce.VpcEndpointId) + if err != nil { + return err } + + return fmt.Errorf("VPC Endpoint Route Table Association %s still exists", rs.Primary.ID) } return nil } -func testAccCheckVpcEndpointRouteTableAssociationExists(n string, vpce *ec2.VpcEndpoint) resource.TestCheckFunc { +func testAccCheckVpcEndpointRouteTableAssociationExists(n string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -85,29 +96,8 @@ func testAccCheckVpcEndpointRouteTableAssociationExists(n string, vpce *ec2.VpcE } conn := testAccProvider.Meta().(*AWSClient).ec2conn - resp, err := conn.DescribeVpcEndpoints(&ec2.DescribeVpcEndpointsInput{ - VpcEndpointIds: aws.StringSlice([]string{rs.Primary.Attributes["vpc_endpoint_id"]}), - }) - if err != nil { - return err - } - if len(resp.VpcEndpoints) == 0 { - return fmt.Errorf("VPC Endpoint not found") - } - *vpce = *resp.VpcEndpoints[0] - - if len(vpce.RouteTableIds) == 0 { - return fmt.Errorf("No VPC Endpoint Route Table Associations") - } - - for _, rtId := range vpce.RouteTableIds { - if aws.StringValue(rtId) == rs.Primary.Attributes["route_table_id"] { - return nil - } - } - - return fmt.Errorf("VPC Endpoint Route Table Association not found") + return finder.VpcEndpointRouteTableAssociationExists(conn, rs.Primary.Attributes["vpc_endpoint_id"], rs.Primary.Attributes["route_table_id"]) } } @@ -138,6 +128,10 @@ data "aws_region" "current" {} resource "aws_vpc_endpoint" "test" { vpc_id = aws_vpc.test.id service_name = "com.amazonaws.${data.aws_region.current.name}.s3" + + tags = { + Name = %[1]q + } } resource "aws_route_table" "test" { diff --git a/aws/resource_aws_vpc_endpoint_subnet_association.go b/aws/resource_aws_vpc_endpoint_subnet_association.go index 477fe03ebc9..11b92a26c92 100644 --- a/aws/resource_aws_vpc_endpoint_subnet_association.go +++ b/aws/resource_aws_vpc_endpoint_subnet_association.go @@ -6,11 +6,14 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode" + tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func resourceAwsVpcEndpointSubnetAssociation() *schema.Resource { @@ -47,12 +50,16 @@ func resourceAwsVpcEndpointSubnetAssociationCreate(d *schema.ResourceData, meta endpointId := d.Get("vpc_endpoint_id").(string) snId := d.Get("subnet_id").(string) + // Human friendly ID for error messages since d.Id() is non-descriptive + id := fmt.Sprintf("%s/%s", endpointId, snId) - _, err := findResourceVpcEndpoint(conn, endpointId) - if err != nil { - return err + input := &ec2.ModifyVpcEndpointInput{ + VpcEndpointId: aws.String(endpointId), + AddSubnetIds: aws.StringSlice([]string{snId}), } + log.Printf("[DEBUG] Creating VPC Endpoint Subnet Association: %s", input) + // See https://github.com/hashicorp/terraform-provider-aws/issues/3382. // Prevent concurrent subnet association requests and delay between requests. mk := "vpc_endpoint_subnet_association_" + endpointId @@ -64,22 +71,23 @@ func resourceAwsVpcEndpointSubnetAssociationCreate(d *schema.ResourceData, meta Timeout: 3 * time.Minute, Target: []string{"ok"}, Refresh: func() (interface{}, string, error) { - res, err := conn.ModifyVpcEndpoint(&ec2.ModifyVpcEndpointInput{ - VpcEndpointId: aws.String(endpointId), - AddSubnetIds: aws.StringSlice([]string{snId}), - }) - return res, "ok", err + output, err := conn.ModifyVpcEndpoint(input) + + return output, "ok", err }, } - _, err = c.WaitForState() + _, err := c.WaitForState() + if err != nil { - return fmt.Errorf("Error creating Vpc Endpoint/Subnet association: %s", err) + return fmt.Errorf("error creating VPC Endpoint Subnet Association (%s): %w", id, err) } - d.SetId(vpcEndpointSubnetAssociationId(endpointId, snId)) + d.SetId(tfec2.VpcEndpointSubnetAssociationCreateID(endpointId, snId)) + + _, err = waiter.VpcEndpointAvailable(conn, endpointId, d.Timeout(schema.TimeoutCreate)) - if err := vpcEndpointWaitUntilAvailable(conn, endpointId, d.Timeout(schema.TimeoutCreate)); err != nil { - return err + if err != nil { + return fmt.Errorf("error waiting for VPC Endpoint (%s) to become available: %w", endpointId, err) } return resourceAwsVpcEndpointSubnetAssociationRead(d, meta) @@ -90,31 +98,21 @@ func resourceAwsVpcEndpointSubnetAssociationRead(d *schema.ResourceData, meta in endpointId := d.Get("vpc_endpoint_id").(string) snId := d.Get("subnet_id").(string) + // Human friendly ID for error messages since d.Id() is non-descriptive + id := fmt.Sprintf("%s/%s", endpointId, snId) - vpce, err := findResourceVpcEndpoint(conn, endpointId) - if err != nil { - if isAWSErr(err, "InvalidVpcEndpointId.NotFound", "") { - log.Printf("[WARN] Vpc Endpoint (%s) not found, removing Vpc Endpoint/Subnet association (%s) from state", endpointId, d.Id()) - d.SetId("") - return nil - } - - return err - } + err := finder.VpcEndpointSubnetAssociationExists(conn, endpointId, snId) - found := false - for _, id := range vpce.SubnetIds { - if aws.StringValue(id) == snId { - found = true - break - } - } - if !found { - log.Printf("[WARN] Vpc Endpoint/Subnet association (%s) not found, removing from state", d.Id()) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] VPC Endpoint Subnet Association (%s) not found, removing from state", id) d.SetId("") return nil } + if err != nil { + return fmt.Errorf("error reading VPC Endpoint Subnet Association (%s): %w", id, err) + } + return nil } @@ -123,34 +121,31 @@ func resourceAwsVpcEndpointSubnetAssociationDelete(d *schema.ResourceData, meta endpointId := d.Get("vpc_endpoint_id").(string) snId := d.Get("subnet_id").(string) + // Human friendly ID for error messages since d.Id() is non-descriptive + id := fmt.Sprintf("%s/%s", endpointId, snId) - _, err := conn.ModifyVpcEndpoint(&ec2.ModifyVpcEndpointInput{ + input := &ec2.ModifyVpcEndpointInput{ VpcEndpointId: aws.String(endpointId), RemoveSubnetIds: aws.StringSlice([]string{snId}), - }) + } + + log.Printf("[DEBUG] Deleting VPC Endpoint Subnet Association: %s", input) + + _, err := conn.ModifyVpcEndpoint(input) + + if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidVpcEndpointIdNotFound) || tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidSubnetIdNotFound) || tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidParameter) { + return nil + } + if err != nil { - ec2err, ok := err.(awserr.Error) - if !ok { - return fmt.Errorf("Error deleting Vpc Endpoint/Subnet association: %s", err) - } - - switch ec2err.Code() { - case "InvalidVpcEndpointId.NotFound": - fallthrough - case "InvalidParameter": - log.Printf("[DEBUG] Vpc Endpoint/Subnet association is already gone") - default: - return fmt.Errorf("Error deleting Vpc Endpoint/Subnet association: %s", err) - } + return fmt.Errorf("error deleting VPC Endpoint Subnet Association (%s): %w", id, err) } - if err := vpcEndpointWaitUntilAvailable(conn, endpointId, d.Timeout(schema.TimeoutDelete)); err != nil { - return err + _, err = waiter.VpcEndpointAvailable(conn, endpointId, d.Timeout(schema.TimeoutDelete)) + + if err != nil { + return fmt.Errorf("error waiting for VPC Endpoint (%s) to become available: %w", endpointId, err) } return nil } - -func vpcEndpointSubnetAssociationId(endpointId, snId string) string { - return fmt.Sprintf("a-%s%d", endpointId, hashcode.String(snId)) -} diff --git a/aws/resource_aws_vpc_endpoint_subnet_association_test.go b/aws/resource_aws_vpc_endpoint_subnet_association_test.go index 8e6f80d1674..758426bc2a3 100644 --- a/aws/resource_aws_vpc_endpoint_subnet_association_test.go +++ b/aws/resource_aws_vpc_endpoint_subnet_association_test.go @@ -4,15 +4,18 @@ import ( "fmt" "testing" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func TestAccAWSVpcEndpointSubnetAssociation_basic(t *testing.T) { var vpce ec2.VpcEndpoint + resourceName := "aws_vpc_endpoint_subnet_association.test" + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -21,18 +24,44 @@ func TestAccAWSVpcEndpointSubnetAssociation_basic(t *testing.T) { CheckDestroy: testAccCheckVpcEndpointSubnetAssociationDestroy, Steps: []resource.TestStep{ { - Config: testAccVpcEndpointSubnetAssociationConfig_basic, + Config: testAccVpcEndpointSubnetAssociationConfigBasic(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckVpcEndpointSubnetAssociationExists( - "aws_vpc_endpoint_subnet_association.a", &vpce), + testAccCheckVpcEndpointSubnetAssociationExists(resourceName, &vpce), ), }, }, }) } +func TestAccAWSVpcEndpointSubnetAssociation_disappears(t *testing.T) { + var vpce ec2.VpcEndpoint + resourceName := "aws_vpc_endpoint_subnet_association.test" + rName := acctest.RandomWithPrefix("tf-acc-test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, ec2.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckVpcEndpointSubnetAssociationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccVpcEndpointSubnetAssociationConfigBasic(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckVpcEndpointSubnetAssociationExists(resourceName, &vpce), + testAccCheckResourceDisappears(testAccProvider, resourceAwsVpcEndpointSubnetAssociation(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func TestAccAWSVpcEndpointSubnetAssociation_multiple(t *testing.T) { var vpce ec2.VpcEndpoint + resourceName0 := "aws_vpc_endpoint_subnet_association.test.0" + resourceName1 := "aws_vpc_endpoint_subnet_association.test.1" + resourceName2 := "aws_vpc_endpoint_subnet_association.test.2" + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -41,14 +70,11 @@ func TestAccAWSVpcEndpointSubnetAssociation_multiple(t *testing.T) { CheckDestroy: testAccCheckVpcEndpointSubnetAssociationDestroy, Steps: []resource.TestStep{ { - Config: testAccVpcEndpointSubnetAssociationConfig_multiple, + Config: testAccVpcEndpointSubnetAssociationConfigMultiple(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckVpcEndpointSubnetAssociationExists( - "aws_vpc_endpoint_subnet_association.a.0", &vpce), - testAccCheckVpcEndpointSubnetAssociationExists( - "aws_vpc_endpoint_subnet_association.a.1", &vpce), - testAccCheckVpcEndpointSubnetAssociationExists( - "aws_vpc_endpoint_subnet_association.a.2", &vpce), + testAccCheckVpcEndpointSubnetAssociationExists(resourceName0, &vpce), + testAccCheckVpcEndpointSubnetAssociationExists(resourceName1, &vpce), + testAccCheckVpcEndpointSubnetAssociationExists(resourceName2, &vpce), ), }, }, @@ -63,27 +89,17 @@ func testAccCheckVpcEndpointSubnetAssociationDestroy(s *terraform.State) error { continue } - // Try to find the resource - resp, err := conn.DescribeVpcEndpoints(&ec2.DescribeVpcEndpointsInput{ - VpcEndpointIds: aws.StringSlice([]string{rs.Primary.Attributes["vpc_endpoint_id"]}), - }) - if err != nil { - // Verify the error is what we want - ec2err, ok := err.(awserr.Error) - if !ok { - return err - } - if ec2err.Code() != "InvalidVpcEndpointId.NotFound" { - return err - } - return nil + err := finder.VpcEndpointSubnetAssociationExists(conn, rs.Primary.Attributes["vpc_endpoint_id"], rs.Primary.Attributes["subnet_id"]) + + if tfresource.NotFound(err) { + continue } - vpce := resp.VpcEndpoints[0] - if len(vpce.SubnetIds) > 0 { - return fmt.Errorf( - "Vpc endpoint %s has subnets", *vpce.VpcEndpointId) + if err != nil { + return err } + + return fmt.Errorf("VPC Endpoint Subnet Association %s still exists", rs.Primary.ID) } return nil @@ -101,130 +117,90 @@ func testAccCheckVpcEndpointSubnetAssociationExists(n string, vpce *ec2.VpcEndpo } conn := testAccProvider.Meta().(*AWSClient).ec2conn - resp, err := conn.DescribeVpcEndpoints(&ec2.DescribeVpcEndpointsInput{ - VpcEndpointIds: aws.StringSlice([]string{rs.Primary.Attributes["vpc_endpoint_id"]}), - }) + + out, err := finder.VpcEndpointByID(conn, rs.Primary.Attributes["vpc_endpoint_id"]) + if err != nil { return err } - if len(resp.VpcEndpoints) == 0 { - return fmt.Errorf("Vpc endpoint not found") - } - *vpce = *resp.VpcEndpoints[0] + err = finder.VpcEndpointSubnetAssociationExists(conn, rs.Primary.Attributes["vpc_endpoint_id"], rs.Primary.Attributes["subnet_id"]) - if len(vpce.SubnetIds) == 0 { - return fmt.Errorf("no subnet associations") + if err != nil { + return err } - for _, id := range vpce.SubnetIds { - if aws.StringValue(id) == rs.Primary.Attributes["subnet_id"] { - return nil - } - } + *vpce = *out - return fmt.Errorf("subnet association not found") + return nil } } -const testAccVpcEndpointSubnetAssociationConfig_basic = ` -resource "aws_vpc" "foo" { +func testAccVpcEndpointSubnetAssociationConfigBase(rName string) string { + return composeConfig( + testAccAvailableAZsNoOptInConfig(), + fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.0.0.0/16" tags = { - Name = "terraform-testacc-vpc-endpoint-subnet-association" + Name = %[1]q } } -data "aws_security_group" "default" { - vpc_id = aws_vpc.foo.id +data "aws_security_group" "test" { + vpc_id = aws_vpc.test.id name = "default" } data "aws_region" "current" {} -data "aws_availability_zones" "available" { - state = "available" - - filter { - name = "opt-in-status" - values = ["opt-in-not-required"] - } -} - -resource "aws_vpc_endpoint" "ec2" { - vpc_id = aws_vpc.foo.id +resource "aws_vpc_endpoint" "test" { + vpc_id = aws_vpc.test.id vpc_endpoint_type = "Interface" service_name = "com.amazonaws.${data.aws_region.current.name}.ec2" - security_group_ids = [data.aws_security_group.default.id] + security_group_ids = [data.aws_security_group.test.id] private_dns_enabled = false -} - -resource "aws_subnet" "sn" { - vpc_id = aws_vpc.foo.id - availability_zone = data.aws_availability_zones.available.names[0] - cidr_block = "10.0.0.0/17" tags = { - Name = "tf-acc-vpc-endpoint-subnet-association" + Name = %[1]q } } -resource "aws_vpc_endpoint_subnet_association" "a" { - vpc_endpoint_id = aws_vpc_endpoint.ec2.id - subnet_id = aws_subnet.sn.id -} -` +resource "aws_subnet" "test" { + count = 3 -const testAccVpcEndpointSubnetAssociationConfig_multiple = ` -resource "aws_vpc" "foo" { - cidr_block = "10.0.0.0/16" + vpc_id = aws_vpc.test.id + availability_zone = data.aws_availability_zones.available.names[count.index] + cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 2, count.index) tags = { - Name = "terraform-testacc-vpc-endpoint-subnet-association" + Name = %[1]q } } - -data "aws_security_group" "default" { - vpc_id = aws_vpc.foo.id - name = "default" +`, rName)) } -data "aws_region" "current" {} - -data "aws_availability_zones" "available" { - state = "available" - - filter { - name = "opt-in-status" - values = ["opt-in-not-required"] - } +func testAccVpcEndpointSubnetAssociationConfigBasic(rName string) string { + return composeConfig( + testAccVpcEndpointSubnetAssociationConfigBase(rName), + ` +resource "aws_vpc_endpoint_subnet_association" "test" { + vpc_endpoint_id = aws_vpc_endpoint.test.id + subnet_id = aws_subnet.test[0].id } - -resource "aws_vpc_endpoint" "ec2" { - vpc_id = aws_vpc.foo.id - vpc_endpoint_type = "Interface" - service_name = "com.amazonaws.${data.aws_region.current.name}.ec2" - security_group_ids = [data.aws_security_group.default.id] - private_dns_enabled = false +`) } -resource "aws_subnet" "sn" { +func testAccVpcEndpointSubnetAssociationConfigMultiple(rName string) string { + return composeConfig( + testAccVpcEndpointSubnetAssociationConfigBase(rName), + ` +resource "aws_vpc_endpoint_subnet_association" "test" { count = 3 - vpc_id = aws_vpc.foo.id - availability_zone = data.aws_availability_zones.available.names[count.index] - cidr_block = cidrsubnet(aws_vpc.foo.cidr_block, 2, count.index) - - tags = { - Name = format("tf-acc-vpc-endpoint-subnet-association-%d", count.index + 1) - } + vpc_endpoint_id = aws_vpc_endpoint.test.id + subnet_id = aws_subnet.test[count.index].id } - -resource "aws_vpc_endpoint_subnet_association" "a" { - count = 3 - - vpc_endpoint_id = aws_vpc_endpoint.ec2.id - subnet_id = aws_subnet.sn.*.id[count.index] +`) } -` diff --git a/aws/resource_aws_vpc_endpoint_test.go b/aws/resource_aws_vpc_endpoint_test.go index f90a78c087c..55883b2fc4e 100644 --- a/aws/resource_aws_vpc_endpoint_test.go +++ b/aws/resource_aws_vpc_endpoint_test.go @@ -9,12 +9,13 @@ import ( "testing" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func init() { @@ -554,23 +555,15 @@ func testAccCheckVpcEndpointDestroy(s *terraform.State) error { continue } - // Try to find the VPC - input := &ec2.DescribeVpcEndpointsInput{ - VpcEndpointIds: []*string{aws.String(rs.Primary.ID)}, + _, err := finder.VpcEndpointByID(conn, rs.Primary.ID) + + if tfresource.NotFound(err) { + continue } - resp, err := conn.DescribeVpcEndpoints(input) + if err != nil { - // Verify the error is what we want - if ae, ok := err.(awserr.Error); ok && ae.Code() == "InvalidVpcEndpointId.NotFound" { - continue - } return err } - if len(resp.VpcEndpoints) > 0 && aws.StringValue(resp.VpcEndpoints[0].State) != "deleted" { - return fmt.Errorf("VPC Endpoints still exist.") - } - - return err } return nil @@ -588,18 +581,14 @@ func testAccCheckVpcEndpointExists(n string, endpoint *ec2.VpcEndpoint) resource } conn := testAccProvider.Meta().(*AWSClient).ec2conn - input := &ec2.DescribeVpcEndpointsInput{ - VpcEndpointIds: []*string{aws.String(rs.Primary.ID)}, - } - resp, err := conn.DescribeVpcEndpoints(input) + + out, err := finder.VpcEndpointByID(conn, rs.Primary.ID) + if err != nil { return err } - if len(resp.VpcEndpoints) == 0 { - return fmt.Errorf("VPC Endpoint not found") - } - *endpoint = *resp.VpcEndpoints[0] + *endpoint = *out return nil } @@ -793,21 +782,12 @@ resource "aws_vpc" "test" { } } -data "aws_security_group" "test" { - vpc_id = aws_vpc.test.id - name = "default" -} - data "aws_region" "current" {} resource "aws_vpc_endpoint" "test" { vpc_id = aws_vpc.test.id service_name = "com.amazonaws.${data.aws_region.current.name}.ec2" vpc_endpoint_type = "Interface" - - security_group_ids = [ - data.aws_security_group.test.id, - ] } `, rName) } @@ -840,8 +820,8 @@ POLICY `, rName, policy) } -func testAccVpcEndpointConfig_interfaceWithSubnet(rName string) string { - return fmt.Sprintf(` +func testAccVpcEndpointConfig_vpcBase(rName string) string { + return composeConfig(testAccAvailableAZsNoOptInConfig(), fmt.Sprintf(` resource "aws_vpc" "test" { cidr_block = "10.0.0.0/16" enable_dns_support = true @@ -854,61 +834,34 @@ resource "aws_vpc" "test" { data "aws_region" "current" {} -data "aws_availability_zones" "available" { - state = "available" - - filter { - name = "opt-in-status" - values = ["opt-in-not-required"] - } -} - -resource "aws_subnet" "test1" { - vpc_id = aws_vpc.test.id - cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 2, 0) - availability_zone = data.aws_availability_zones.available.names[0] - - tags = { - Name = %[1]q - } -} +resource "aws_subnet" "test" { + count = 3 -resource "aws_subnet" "test2" { vpc_id = aws_vpc.test.id - cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 2, 1) - availability_zone = data.aws_availability_zones.available.names[1] + cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 2, count.index) + availability_zone = data.aws_availability_zones.available.names[count.index] tags = { Name = %[1]q } } -resource "aws_subnet" "test3" { - vpc_id = aws_vpc.test.id - cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 2, 2) - availability_zone = data.aws_availability_zones.available.names[2] - - tags = { - Name = %[1]q - } -} +resource "aws_security_group" "test" { + count = 2 -resource "aws_security_group" "test1" { vpc_id = aws_vpc.test.id tags = { Name = %[1]q } } - -resource "aws_security_group" "test2" { - vpc_id = aws_vpc.test.id - - tags = { - Name = %[1]q - } +`, rName)) } +func testAccVpcEndpointConfig_interfaceWithSubnet(rName string) string { + return composeConfig( + testAccVpcEndpointConfig_vpcBase(rName), + fmt.Sprintf(` resource "aws_vpc_endpoint" "test" { vpc_id = aws_vpc.test.id service_name = "com.amazonaws.${data.aws_region.current.name}.ec2" @@ -916,90 +869,25 @@ resource "aws_vpc_endpoint" "test" { private_dns_enabled = false subnet_ids = [ - aws_subnet.test1.id, + aws_subnet.test[0].id, ] security_group_ids = [ - aws_security_group.test1.id, - aws_security_group.test2.id, + aws_security_group.test[0].id, + aws_security_group.test[1].id, ] tags = { Name = %[1]q } } -`, rName) +`, rName)) } func testAccVpcEndpointConfig_interfaceWithSubnetModified(rName string) string { - return fmt.Sprintf(` -resource "aws_vpc" "test" { - cidr_block = "10.0.0.0/16" - enable_dns_support = true - enable_dns_hostnames = true - - tags = { - Name = %[1]q - } -} - -data "aws_region" "current" {} - -data "aws_availability_zones" "available" { - state = "available" - - filter { - name = "opt-in-status" - values = ["opt-in-not-required"] - } -} - -resource "aws_subnet" "test1" { - vpc_id = aws_vpc.test.id - cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 2, 0) - availability_zone = data.aws_availability_zones.available.names[0] - - tags = { - Name = %[1]q - } -} - -resource "aws_subnet" "test2" { - vpc_id = aws_vpc.test.id - cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 2, 1) - availability_zone = data.aws_availability_zones.available.names[1] - - tags = { - Name = %[1]q - } -} - -resource "aws_subnet" "test3" { - vpc_id = aws_vpc.test.id - cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 2, 2) - availability_zone = data.aws_availability_zones.available.names[2] - - tags = { - Name = %[1]q - } -} - -resource "aws_security_group" "test1" { - vpc_id = aws_vpc.test.id - - tags = { - Name = %[1]q - } -} - -resource "aws_security_group" "test2" { - vpc_id = aws_vpc.test.id - - tags = { - Name = %[1]q - } -} - + return composeConfig( + testAccVpcEndpointConfig_vpcBase(rName), + fmt.Sprintf(` resource "aws_vpc_endpoint" "test" { vpc_id = aws_vpc.test.id service_name = "com.amazonaws.${data.aws_region.current.name}.ec2" @@ -1007,38 +895,32 @@ resource "aws_vpc_endpoint" "test" { private_dns_enabled = true subnet_ids = [ - aws_subnet.test1.id, - aws_subnet.test2.id, - aws_subnet.test3.id, + aws_subnet.test[2].id, + aws_subnet.test[1].id, + aws_subnet.test[0].id, ] security_group_ids = [ - aws_security_group.test1.id, + aws_security_group.test[1].id, ] tags = { Name = %[1]q } } -`, rName) +`, rName)) } func testAccVpcEndpointConfig_interfaceNonAWSService(rName string, autoAccept bool) string { - return fmt.Sprintf(` -resource "aws_vpc" "test" { - cidr_block = "10.0.0.0/16" - - tags = { - Name = %[1]q - } -} - + return composeConfig( + testAccVpcEndpointConfig_vpcBase(rName), + fmt.Sprintf(` resource "aws_lb" "test" { name = %[1]q subnets = [ - aws_subnet.test1.id, - aws_subnet.test2.id, + aws_subnet.test[0].id, + aws_subnet.test[1].id, ] load_balancer_type = "network" @@ -1051,47 +933,12 @@ resource "aws_lb" "test" { } } -data "aws_region" "current" {} - -data "aws_availability_zones" "available" { - state = "available" - - filter { - name = "opt-in-status" - values = ["opt-in-not-required"] - } -} - -resource "aws_subnet" "test1" { - vpc_id = aws_vpc.test.id - cidr_block = "10.0.1.0/24" - availability_zone = data.aws_availability_zones.available.names[0] - - tags = { - Name = %[1]q - } -} - -resource "aws_subnet" "test2" { - vpc_id = aws_vpc.test.id - cidr_block = "10.0.2.0/24" - availability_zone = data.aws_availability_zones.available.names[1] - - tags = { - Name = %[1]q - } -} - resource "aws_vpc_endpoint_service" "test" { acceptance_required = true network_load_balancer_arns = [ aws_lb.test.id, ] -} - -resource "aws_security_group" "test" { - vpc_id = aws_vpc.test.id tags = { Name = %[1]q @@ -1106,14 +953,14 @@ resource "aws_vpc_endpoint" "test" { auto_accept = %[2]t security_group_ids = [ - aws_security_group.test.id, + aws_security_group.test[0].id, ] tags = { Name = %[1]q } } -`, rName, autoAccept) +`, rName, autoAccept)) } func testAccVpcEndpointConfigTags1(rName, tagKey1, tagValue1 string) string { @@ -1173,7 +1020,7 @@ resource "aws_vpc" "test" { cidr_block = "10.10.10.0/25" tags = { - Name = "tf-acc-test-load-balancer" + Name = %[1]q } } @@ -1183,7 +1030,7 @@ resource "aws_subnet" "test" { vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-test-load-balancer" + Name = %[1]q } } @@ -1194,12 +1041,20 @@ resource "aws_lb" "test" { subnet_mapping { subnet_id = aws_subnet.test.id } + + tags = { + Name = %[1]q + } } resource "aws_vpc_endpoint_service" "test" { acceptance_required = false allowed_principals = [data.aws_caller_identity.current.arn] gateway_load_balancer_arns = [aws_lb.test.arn] + + tags = { + Name = %[1]q + } } resource "aws_vpc_endpoint" "test" { @@ -1207,6 +1062,10 @@ resource "aws_vpc_endpoint" "test" { subnet_ids = [aws_subnet.test.id] vpc_endpoint_type = aws_vpc_endpoint_service.test.service_type vpc_id = aws_vpc.test.id + + tags = { + Name = %[1]q + } } `, rName)) } From df29eba274596f085067023f129e99acfd47c3bd Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 15 Jun 2021 12:13:36 -0400 Subject: [PATCH 23/31] r/aws_vpc_endpoint_route_table_association: Use waiters. Acceptance test output: % make testacc TEST=./aws TESTARGS='-run=TestAccAWSVpcEndpointRouteTableAssociation_' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSVpcEndpointRouteTableAssociation_ -timeout 180m === RUN TestAccAWSVpcEndpointRouteTableAssociation_basic === PAUSE TestAccAWSVpcEndpointRouteTableAssociation_basic === RUN TestAccAWSVpcEndpointRouteTableAssociation_disappears === PAUSE TestAccAWSVpcEndpointRouteTableAssociation_disappears === CONT TestAccAWSVpcEndpointRouteTableAssociation_basic === CONT TestAccAWSVpcEndpointRouteTableAssociation_disappears --- PASS: TestAccAWSVpcEndpointRouteTableAssociation_disappears (39.13s) --- PASS: TestAccAWSVpcEndpointRouteTableAssociation_basic (41.03s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 44.118s --- aws/internal/service/ec2/waiter/status.go | 20 ++++++ aws/internal/service/ec2/waiter/waiter.go | 28 ++++++++ ...ws_vpc_endpoint_route_table_association.go | 70 ++++++++++--------- ...rce_aws_vpc_endpoint_subnet_association.go | 47 ++++++------- 4 files changed, 107 insertions(+), 58 deletions(-) diff --git a/aws/internal/service/ec2/waiter/status.go b/aws/internal/service/ec2/waiter/status.go index 3fa0a512c85..68265ef0f40 100644 --- a/aws/internal/service/ec2/waiter/status.go +++ b/aws/internal/service/ec2/waiter/status.go @@ -520,3 +520,23 @@ func VpcEndpointState(conn *ec2.EC2, id string) resource.StateRefreshFunc { return vpcEndpoint, aws.StringValue(vpcEndpoint.State), nil } } + +const ( + VpcEndpointRouteTableAssociationStatusReady = "ready" +) + +func VpcEndpointRouteTableAssociationStatus(conn *ec2.EC2, vpcEndpointID, routeTableID string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + err := finder.VpcEndpointRouteTableAssociationExists(conn, vpcEndpointID, routeTableID) + + if tfresource.NotFound(err) { + return nil, "", nil + } + + if err != nil { + return nil, "", err + } + + return "", VpcEndpointRouteTableAssociationStatusReady, nil + } +} diff --git a/aws/internal/service/ec2/waiter/waiter.go b/aws/internal/service/ec2/waiter/waiter.go index 6a55619627a..73ff47bfbd7 100644 --- a/aws/internal/service/ec2/waiter/waiter.go +++ b/aws/internal/service/ec2/waiter/waiter.go @@ -751,3 +751,31 @@ func VpcEndpointDeleted(conn *ec2.EC2, vpcEndpointID string, timeout time.Durati return nil, err } + +func VpcEndpointRouteTableAssociationDeleted(conn *ec2.EC2, vpcEndpointID, routeTableID string) error { + stateConf := &resource.StateChangeConf{ + Pending: []string{VpcEndpointRouteTableAssociationStatusReady}, + Target: []string{}, + Refresh: VpcEndpointRouteTableAssociationStatus(conn, vpcEndpointID, routeTableID), + Timeout: PropagationTimeout, + ContinuousTargetOccurence: 2, + } + + _, err := stateConf.WaitForState() + + return err +} + +func VpcEndpointRouteTableAssociationReady(conn *ec2.EC2, vpcEndpointID, routeTableID string) error { + stateConf := &resource.StateChangeConf{ + Pending: []string{}, + Target: []string{VpcEndpointRouteTableAssociationStatusReady}, + Refresh: VpcEndpointRouteTableAssociationStatus(conn, vpcEndpointID, routeTableID), + Timeout: PropagationTimeout, + ContinuousTargetOccurence: 2, + } + + _, err := stateConf.WaitForState() + + return err +} diff --git a/aws/resource_aws_vpc_endpoint_route_table_association.go b/aws/resource_aws_vpc_endpoint_route_table_association.go index 0e56c8669b2..0ef447a13e3 100644 --- a/aws/resource_aws_vpc_endpoint_route_table_association.go +++ b/aws/resource_aws_vpc_endpoint_route_table_association.go @@ -25,12 +25,12 @@ func resourceAwsVpcEndpointRouteTableAssociation() *schema.Resource { }, Schema: map[string]*schema.Schema{ - "vpc_endpoint_id": { + "route_table_id": { Type: schema.TypeString, Required: true, ForceNew: true, }, - "route_table_id": { + "vpc_endpoint_id": { Type: schema.TypeString, Required: true, ForceNew: true, @@ -42,25 +42,30 @@ func resourceAwsVpcEndpointRouteTableAssociation() *schema.Resource { func resourceAwsVpcEndpointRouteTableAssociationCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - endpointId := d.Get("vpc_endpoint_id").(string) - rtId := d.Get("route_table_id").(string) + endpointID := d.Get("vpc_endpoint_id").(string) + routeTableID := d.Get("route_table_id").(string) // Human friendly ID for error messages since d.Id() is non-descriptive - id := fmt.Sprintf("%s/%s", endpointId, rtId) + id := fmt.Sprintf("%s/%s", endpointID, routeTableID) input := &ec2.ModifyVpcEndpointInput{ - VpcEndpointId: aws.String(endpointId), - AddRouteTableIds: aws.StringSlice([]string{rtId}), + VpcEndpointId: aws.String(endpointID), + AddRouteTableIds: aws.StringSlice([]string{routeTableID}), } log.Printf("[DEBUG] Creating VPC Endpoint Route Table Association: %s", input) - _, err := conn.ModifyVpcEndpoint(input) if err != nil { return fmt.Errorf("error creating VPC Endpoint Route Table Association (%s): %w", id, err) } - d.SetId(tfec2.VpcEndpointRouteTableAssociationCreateID(endpointId, rtId)) + d.SetId(tfec2.VpcEndpointRouteTableAssociationCreateID(endpointID, routeTableID)) + + err = waiter.VpcEndpointRouteTableAssociationReady(conn, endpointID, routeTableID) + + if err != nil { + return fmt.Errorf("error waiting for VPC Endpoint Route Table Association (%s) to become available: %w", id, err) + } return resourceAwsVpcEndpointRouteTableAssociationRead(d, meta) } @@ -68,20 +73,12 @@ func resourceAwsVpcEndpointRouteTableAssociationCreate(d *schema.ResourceData, m func resourceAwsVpcEndpointRouteTableAssociationRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - endpointId := d.Get("vpc_endpoint_id").(string) - rtId := d.Get("route_table_id").(string) + endpointID := d.Get("vpc_endpoint_id").(string) + routeTableID := d.Get("route_table_id").(string) // Human friendly ID for error messages since d.Id() is non-descriptive - id := fmt.Sprintf("%s/%s", endpointId, rtId) - - _, err := tfresource.RetryUntilFound(waiter.PropagationTimeout, d.IsNewResource(), func() (interface{}, error) { - err := finder.VpcEndpointRouteTableAssociationExists(conn, endpointId, rtId) + id := fmt.Sprintf("%s/%s", endpointID, routeTableID) - if err != nil { - return nil, err - } - - return struct{}{}, nil - }) + err := finder.VpcEndpointRouteTableAssociationExists(conn, endpointID, routeTableID) if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] VPC Endpoint Route Table Association (%s) not found, removing from state", id) @@ -99,18 +96,17 @@ func resourceAwsVpcEndpointRouteTableAssociationRead(d *schema.ResourceData, met func resourceAwsVpcEndpointRouteTableAssociationDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - endpointId := d.Get("vpc_endpoint_id").(string) - rtId := d.Get("route_table_id").(string) + endpointID := d.Get("vpc_endpoint_id").(string) + routeTableID := d.Get("route_table_id").(string) // Human friendly ID for error messages since d.Id() is non-descriptive - id := fmt.Sprintf("%s/%s", endpointId, rtId) + id := fmt.Sprintf("%s/%s", endpointID, routeTableID) input := &ec2.ModifyVpcEndpointInput{ - VpcEndpointId: aws.String(endpointId), - RemoveRouteTableIds: aws.StringSlice([]string{rtId}), + VpcEndpointId: aws.String(endpointID), + RemoveRouteTableIds: aws.StringSlice([]string{routeTableID}), } - log.Printf("[DEBUG] Deleting VPC Endpoint Route Table Association: %s", input) - + log.Printf("[DEBUG] Deleting VPC Endpoint Route Table Association: %s", id) _, err := conn.ModifyVpcEndpoint(input) if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidVpcEndpointIdNotFound) || tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidRouteTableIdNotFound) || tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidParameter) { @@ -121,6 +117,12 @@ func resourceAwsVpcEndpointRouteTableAssociationDelete(d *schema.ResourceData, m return fmt.Errorf("error deleting VPC Endpoint Route Table Association (%s): %w", id, err) } + err = waiter.VpcEndpointRouteTableAssociationDeleted(conn, endpointID, routeTableID) + + if err != nil { + return fmt.Errorf("error waiting for VPC Endpoint Route Table Association (%s) to delete: %w", id, err) + } + return nil } @@ -130,13 +132,13 @@ func resourceAwsVpcEndpointRouteTableAssociationImport(d *schema.ResourceData, m return nil, fmt.Errorf("Wrong format of resource: %s. Please follow 'vpc-endpoint-id/route-table-id'", d.Id()) } - vpceId := parts[0] - rtId := parts[1] - log.Printf("[DEBUG] Importing VPC Endpoint (%s) Route Table (%s) Association", vpceId, rtId) + endpointID := parts[0] + routeTableID := parts[1] + log.Printf("[DEBUG] Importing VPC Endpoint (%s) Route Table (%s) Association", endpointID, routeTableID) - d.SetId(tfec2.VpcEndpointRouteTableAssociationCreateID(vpceId, rtId)) - d.Set("vpc_endpoint_id", vpceId) - d.Set("route_table_id", rtId) + d.SetId(tfec2.VpcEndpointRouteTableAssociationCreateID(endpointID, routeTableID)) + d.Set("vpc_endpoint_id", endpointID) + d.Set("route_table_id", routeTableID) return []*schema.ResourceData{d}, nil } diff --git a/aws/resource_aws_vpc_endpoint_subnet_association.go b/aws/resource_aws_vpc_endpoint_subnet_association.go index 11b92a26c92..9a2ada6ae59 100644 --- a/aws/resource_aws_vpc_endpoint_subnet_association.go +++ b/aws/resource_aws_vpc_endpoint_subnet_association.go @@ -26,12 +26,12 @@ func resourceAwsVpcEndpointSubnetAssociation() *schema.Resource { }, Schema: map[string]*schema.Schema{ - "vpc_endpoint_id": { + "subnet_id": { Type: schema.TypeString, Required: true, ForceNew: true, }, - "subnet_id": { + "vpc_endpoint_id": { Type: schema.TypeString, Required: true, ForceNew: true, @@ -48,21 +48,21 @@ func resourceAwsVpcEndpointSubnetAssociation() *schema.Resource { func resourceAwsVpcEndpointSubnetAssociationCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - endpointId := d.Get("vpc_endpoint_id").(string) - snId := d.Get("subnet_id").(string) + endpointID := d.Get("vpc_endpoint_id").(string) + subnetID := d.Get("subnet_id").(string) // Human friendly ID for error messages since d.Id() is non-descriptive - id := fmt.Sprintf("%s/%s", endpointId, snId) + id := fmt.Sprintf("%s/%s", endpointID, subnetID) input := &ec2.ModifyVpcEndpointInput{ - VpcEndpointId: aws.String(endpointId), - AddSubnetIds: aws.StringSlice([]string{snId}), + VpcEndpointId: aws.String(endpointID), + AddSubnetIds: aws.StringSlice([]string{subnetID}), } log.Printf("[DEBUG] Creating VPC Endpoint Subnet Association: %s", input) // See https://github.com/hashicorp/terraform-provider-aws/issues/3382. // Prevent concurrent subnet association requests and delay between requests. - mk := "vpc_endpoint_subnet_association_" + endpointId + mk := "vpc_endpoint_subnet_association_" + endpointID awsMutexKV.Lock(mk) defer awsMutexKV.Unlock(mk) @@ -82,12 +82,12 @@ func resourceAwsVpcEndpointSubnetAssociationCreate(d *schema.ResourceData, meta return fmt.Errorf("error creating VPC Endpoint Subnet Association (%s): %w", id, err) } - d.SetId(tfec2.VpcEndpointSubnetAssociationCreateID(endpointId, snId)) + d.SetId(tfec2.VpcEndpointSubnetAssociationCreateID(endpointID, subnetID)) - _, err = waiter.VpcEndpointAvailable(conn, endpointId, d.Timeout(schema.TimeoutCreate)) + _, err = waiter.VpcEndpointAvailable(conn, endpointID, d.Timeout(schema.TimeoutCreate)) if err != nil { - return fmt.Errorf("error waiting for VPC Endpoint (%s) to become available: %w", endpointId, err) + return fmt.Errorf("error waiting for VPC Endpoint (%s) to become available: %w", endpointID, err) } return resourceAwsVpcEndpointSubnetAssociationRead(d, meta) @@ -96,12 +96,12 @@ func resourceAwsVpcEndpointSubnetAssociationCreate(d *schema.ResourceData, meta func resourceAwsVpcEndpointSubnetAssociationRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - endpointId := d.Get("vpc_endpoint_id").(string) - snId := d.Get("subnet_id").(string) + endpointID := d.Get("vpc_endpoint_id").(string) + subnetID := d.Get("subnet_id").(string) // Human friendly ID for error messages since d.Id() is non-descriptive - id := fmt.Sprintf("%s/%s", endpointId, snId) + id := fmt.Sprintf("%s/%s", endpointID, subnetID) - err := finder.VpcEndpointSubnetAssociationExists(conn, endpointId, snId) + err := finder.VpcEndpointSubnetAssociationExists(conn, endpointID, subnetID) if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] VPC Endpoint Subnet Association (%s) not found, removing from state", id) @@ -119,18 +119,17 @@ func resourceAwsVpcEndpointSubnetAssociationRead(d *schema.ResourceData, meta in func resourceAwsVpcEndpointSubnetAssociationDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - endpointId := d.Get("vpc_endpoint_id").(string) - snId := d.Get("subnet_id").(string) + endpointID := d.Get("vpc_endpoint_id").(string) + subnetID := d.Get("subnet_id").(string) // Human friendly ID for error messages since d.Id() is non-descriptive - id := fmt.Sprintf("%s/%s", endpointId, snId) + id := fmt.Sprintf("%s/%s", endpointID, subnetID) input := &ec2.ModifyVpcEndpointInput{ - VpcEndpointId: aws.String(endpointId), - RemoveSubnetIds: aws.StringSlice([]string{snId}), + VpcEndpointId: aws.String(endpointID), + RemoveSubnetIds: aws.StringSlice([]string{subnetID}), } - log.Printf("[DEBUG] Deleting VPC Endpoint Subnet Association: %s", input) - + log.Printf("[DEBUG] Deleting VPC Endpoint Subnet Association: %s", id) _, err := conn.ModifyVpcEndpoint(input) if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidVpcEndpointIdNotFound) || tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidSubnetIdNotFound) || tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidParameter) { @@ -141,10 +140,10 @@ func resourceAwsVpcEndpointSubnetAssociationDelete(d *schema.ResourceData, meta return fmt.Errorf("error deleting VPC Endpoint Subnet Association (%s): %w", id, err) } - _, err = waiter.VpcEndpointAvailable(conn, endpointId, d.Timeout(schema.TimeoutDelete)) + _, err = waiter.VpcEndpointAvailable(conn, endpointID, d.Timeout(schema.TimeoutDelete)) if err != nil { - return fmt.Errorf("error waiting for VPC Endpoint (%s) to become available: %w", endpointId, err) + return fmt.Errorf("error waiting for VPC Endpoint (%s) to become available: %w", endpointID, err) } return nil From b6e6fb94e53038aeeafe7dc99a67e1c0d36c5ba2 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 15 Jun 2021 12:36:12 -0400 Subject: [PATCH 24/31] r/aws_vpc_endpoint: Fix 'TestAccAWSVpcEndpoint_interfaceBasic'. Acceptance test output: % make testacc TEST=./aws TESTARGS='-run=TestAccAWSVpcEndpoint_interfaceBasic' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSVpcEndpoint_interfaceBasic -timeout 180m === RUN TestAccAWSVpcEndpoint_interfaceBasic === PAUSE TestAccAWSVpcEndpoint_interfaceBasic === CONT TestAccAWSVpcEndpoint_interfaceBasic --- PASS: TestAccAWSVpcEndpoint_interfaceBasic (141.62s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 145.881s --- aws/resource_aws_vpc_endpoint_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/aws/resource_aws_vpc_endpoint_test.go b/aws/resource_aws_vpc_endpoint_test.go index 55883b2fc4e..40c22ec21d8 100644 --- a/aws/resource_aws_vpc_endpoint_test.go +++ b/aws/resource_aws_vpc_endpoint_test.go @@ -782,12 +782,21 @@ resource "aws_vpc" "test" { } } +data "aws_security_group" "test" { + vpc_id = aws_vpc.test.id + name = "default" +} + data "aws_region" "current" {} resource "aws_vpc_endpoint" "test" { vpc_id = aws_vpc.test.id service_name = "com.amazonaws.${data.aws_region.current.name}.ec2" vpc_endpoint_type = "Interface" + + security_group_ids = [ + data.aws_security_group.test.id, + ] } `, rName) } From f2af9b4defb216f413b1b65e88e2d4ad180b4b07 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 15 Jun 2021 15:39:35 -0400 Subject: [PATCH 25/31] Don't semgrep for 'tfresource.*' inside the 'tfresource' package. --- .semgrep.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.semgrep.yml b/.semgrep.yml index 7bc84a369ad..c9a41d64c4a 100644 --- a/.semgrep.yml +++ b/.semgrep.yml @@ -356,6 +356,7 @@ rules: paths: exclude: - "*_test.go" + - aws/internal/tfresource/*.go include: - aws/ patterns: From c41c7840ea717df084bafcf7de89857e9f1ead9d Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 15 Jun 2021 15:40:10 -0400 Subject: [PATCH 26/31] Fix awsproviderlint misspell errors. --- aws/resource_aws_main_route_table_association.go | 2 +- aws/resource_aws_route_table_association.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_main_route_table_association.go b/aws/resource_aws_main_route_table_association.go index e8a3b955fd7..b97508d2eef 100644 --- a/aws/resource_aws_main_route_table_association.go +++ b/aws/resource_aws_main_route_table_association.go @@ -113,7 +113,7 @@ func resourceAwsMainRouteTableAssociationUpdate(d *schema.ResourceData, meta int } // This whole thing with the resource ID being changed on update seems unsustainable. - // Keeping it here for backwards compatibilty... + // Keeping it here for backwards compatibility... d.SetId(aws.StringValue(output.NewAssociationId)) log.Printf("[DEBUG] Waiting for Main Route Table Association (%s) update", d.Id()) diff --git a/aws/resource_aws_route_table_association.go b/aws/resource_aws_route_table_association.go index 552dad00db1..8cc2a39bb91 100644 --- a/aws/resource_aws_route_table_association.go +++ b/aws/resource_aws_route_table_association.go @@ -121,7 +121,7 @@ func resourceAwsRouteTableAssociationUpdate(d *schema.ResourceData, meta interfa output, err := conn.ReplaceRouteTableAssociation(input) // This whole thing with the resource ID being changed on update seems unsustainable. - // Keeping it here for backwards compatibilty... + // Keeping it here for backwards compatibility... if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidAssociationIDNotFound) { // Not found, so just create a new one From f6a0203a4d7b4a97411494cbc0fadb177052cd85 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 15 Jun 2021 16:24:49 -0400 Subject: [PATCH 27/31] Ignore golanci-lint 'unparam' error. --- aws/resource_aws_route_table.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_route_table.go b/aws/resource_aws_route_table.go index 7844a32a403..4095f291c3a 100644 --- a/aws/resource_aws_route_table.go +++ b/aws/resource_aws_route_table.go @@ -888,7 +888,7 @@ func routeTableRouteDestinationAttribute(m map[string]interface{}) (string, stri } // routeTableRouteTargetAttribute returns the attribute key and value of the route table route's target. -func routeTableRouteTargetAttribute(m map[string]interface{}) (string, string) { +func routeTableRouteTargetAttribute(m map[string]interface{}) (string, string) { //nolint:unparam for _, key := range routeTableValidTargets { if v, ok := m[key].(string); ok && v != "" { return key, v From 8f8a267e9a7a1dfe2bb3187045111ac8f66b6988 Mon Sep 17 00:00:00 2001 From: Adam Lewandowski Date: Tue, 18 May 2021 13:55:27 -0400 Subject: [PATCH 28/31] Add changelog entry --- .changelog/19426.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/19426.txt diff --git a/.changelog/19426.txt b/.changelog/19426.txt new file mode 100644 index 00000000000..75097c3548e --- /dev/null +++ b/.changelog/19426.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_route_table: Add retries when reading route table +``` From cc31e7d7aab251e13819e34fec75da1612c6a5a7 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 15 Jun 2021 16:48:49 -0400 Subject: [PATCH 29/31] Add CHANGELOG entries. --- .changelog/19426.txt | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/.changelog/19426.txt b/.changelog/19426.txt index 75097c3548e..912bdafd6b0 100644 --- a/.changelog/19426.txt +++ b/.changelog/19426.txt @@ -1,3 +1,23 @@ ```release-note:enhancement -resource/aws_route_table: Add retries when reading route table +resource/aws_route_table: Add retries when creating, deleting and replacing routes ``` + +```release-note:enhancement +resource/aws_route: Add retries when creating, deleting and replacing routes +``` + +```release-note:enhancement +resource/aws_default_route_table: Add retries when creating, deleting and replacing routes +``` + +```release-note:enhancement +resource/aws_default_route_table: Add retries when creating, deleting and replacing routes +``` + +```release-note:enhancement +resource/aws_main_route_table_association: Wait for association to reach the required state +``` + +```release-note:enhancement +resource/aws_route_table_association: Wait for association to reach the required state +``` \ No newline at end of file From 4579cff2d603a322b07dad898a8490487a684c33 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 15 Jun 2021 16:50:35 -0400 Subject: [PATCH 30/31] Fix terrafmt errors. --- aws/resource_aws_route_table_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/aws/resource_aws_route_table_test.go b/aws/resource_aws_route_table_test.go index 663fee82be6..f377447270a 100644 --- a/aws/resource_aws_route_table_test.go +++ b/aws/resource_aws_route_table_test.go @@ -2116,12 +2116,12 @@ resource "aws_network_interface" "test1" { } resource "aws_network_interface" "test2" { - subnet_id = aws_subnet.test.id - - tags = { - Name = %[1]q - } + subnet_id = aws_subnet.test.id + + tags = { + Name = %[1]q } +} resource "aws_route_table" "test" { vpc_id = aws_vpc.test.id From c2953e4bacf1c46d8f3ef27433865909d4054535 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 15 Jun 2021 16:57:22 -0400 Subject: [PATCH 31/31] Rename tests to help tctest. Acceptance test output: % go test -v ./aws/internal/net === RUN TestCIDRBlocksEqual --- PASS: TestCIDRBlocksEqual (0.00s) === RUN TestCanonicalCIDRBlock --- PASS: TestCanonicalCIDRBlock (0.00s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws/internal/net 0.687s --- aws/internal/net/cidr_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/aws/internal/net/cidr_test.go b/aws/internal/net/cidr_test.go index ce1ccefa324..308f76a5079 100644 --- a/aws/internal/net/cidr_test.go +++ b/aws/internal/net/cidr_test.go @@ -1,10 +1,12 @@ -package net +package net_test import ( "testing" + + tfnet "github.com/terraform-providers/terraform-provider-aws/aws/internal/net" ) -func Test_CIDRBlocksEqual(t *testing.T) { +func TestCIDRBlocksEqual(t *testing.T) { for _, ts := range []struct { cidr1 string cidr2 string @@ -18,14 +20,14 @@ func Test_CIDRBlocksEqual(t *testing.T) { {"::/0", "::0/0", true}, {"", "", false}, } { - equal := CIDRBlocksEqual(ts.cidr1, ts.cidr2) + equal := tfnet.CIDRBlocksEqual(ts.cidr1, ts.cidr2) if ts.equal != equal { t.Fatalf("CIDRBlocksEqual(%q, %q) should be: %t", ts.cidr1, ts.cidr2, ts.equal) } } } -func Test_CanonicalCIDRBlock(t *testing.T) { +func TestCanonicalCIDRBlock(t *testing.T) { for _, ts := range []struct { cidr string expected string @@ -35,7 +37,7 @@ func Test_CanonicalCIDRBlock(t *testing.T) { {"::0/0", "::/0"}, {"", ""}, } { - got := CanonicalCIDRBlock(ts.cidr) + got := tfnet.CanonicalCIDRBlock(ts.cidr) if ts.expected != got { t.Fatalf("CanonicalCIDRBlock(%q) should be: %q, got: %q", ts.cidr, ts.expected, got) }