From 91608b23585f49e667f591720ee21394afbdf058 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 16 Sep 2020 16:37:49 -0400 Subject: [PATCH 1/4] r/aws_vpn_gateway_attachment: Refactor using internal 'finder' and 'waiter' packages. Increase detachment timeout to 30m. Acceptance test output: $ make testacc TEST=./aws TESTARGS='-run=TestAccAWSVpnGatewayAttachment_' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSVpnGatewayAttachment_ -timeout 120m === RUN TestAccAWSVpnGatewayAttachment_basic === PAUSE TestAccAWSVpnGatewayAttachment_basic === RUN TestAccAWSVpnGatewayAttachment_deleted === PAUSE TestAccAWSVpnGatewayAttachment_deleted === CONT TestAccAWSVpnGatewayAttachment_basic === CONT TestAccAWSVpnGatewayAttachment_deleted resource_aws_vpn_gateway_attachment_test.go:40: [INFO] Got non-empty plan, as expected --- PASS: TestAccAWSVpnGatewayAttachment_deleted (33.77s) --- PASS: TestAccAWSVpnGatewayAttachment_basic (38.51s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 38.552s --- aws/internal/service/ec2/errors.go | 5 + aws/internal/service/ec2/finder/finder.go | 40 +++++ aws/internal/service/ec2/id.go | 6 + aws/internal/service/ec2/waiter/status.go | 24 +++ aws/internal/service/ec2/waiter/waiter.go | 40 +++++ aws/resource_aws_vpn_gateway.go | 40 +++++ aws/resource_aws_vpn_gateway_attachment.go | 146 ++++-------------- ...esource_aws_vpn_gateway_attachment_test.go | 129 ++++++---------- 8 files changed, 230 insertions(+), 200 deletions(-) diff --git a/aws/internal/service/ec2/errors.go b/aws/internal/service/ec2/errors.go index 51a142922de..1a2b2aa92cb 100644 --- a/aws/internal/service/ec2/errors.go +++ b/aws/internal/service/ec2/errors.go @@ -28,3 +28,8 @@ const ( InvalidSecurityGroupIDNotFound = "InvalidSecurityGroupID.NotFound" InvalidGroupNotFound = "InvalidGroup.NotFound" ) + +const ( + InvalidVpnGatewayAttachmentNotFound = "InvalidVpnGatewayAttachment.NotFound" + InvalidVpnGatewayIDNotFound = "InvalidVpnGatewayID.NotFound" +) diff --git a/aws/internal/service/ec2/finder/finder.go b/aws/internal/service/ec2/finder/finder.go index 03963246074..6a513edf6fd 100644 --- a/aws/internal/service/ec2/finder/finder.go +++ b/aws/internal/service/ec2/finder/finder.go @@ -71,3 +71,43 @@ func SecurityGroupByID(conn *ec2.EC2, id string) (*ec2.SecurityGroup, error) { return result.SecurityGroups[0], nil } + +// VpnGatewayVpcAttachment returns the attachment between the specified VPN gateway and VPC. +// Returns nil and potentially an error if no attachment is found. +func VpnGatewayVpcAttachment(conn *ec2.EC2, vpnGatewayID, vpcID string) (*ec2.VpcAttachment, error) { + vpnGateway, err := VpnGatewayByID(conn, vpnGatewayID) + if err != nil { + return nil, err + } + + if vpnGateway == nil { + return nil, nil + } + + for _, vpcAttachment := range vpnGateway.VpcAttachments { + if aws.StringValue(vpcAttachment.VpcId) == vpcID { + return vpcAttachment, nil + } + } + + return nil, nil +} + +// VpnGatewayByID returns the VPN gateway corresponding to the specified identifier. +// Returns nil and potentially an error if no VPN gateway is found. +func VpnGatewayByID(conn *ec2.EC2, id string) (*ec2.VpnGateway, error) { + input := &ec2.DescribeVpnGatewaysInput{ + VpnGatewayIds: aws.StringSlice([]string{id}), + } + + output, err := conn.DescribeVpnGateways(input) + if err != nil { + return nil, err + } + + if output == nil || len(output.VpnGateways) == 0 { + return nil, nil + } + + return output.VpnGateways[0], nil +} diff --git a/aws/internal/service/ec2/id.go b/aws/internal/service/ec2/id.go index a01addcae9a..c3797b4a2ed 100644 --- a/aws/internal/service/ec2/id.go +++ b/aws/internal/service/ec2/id.go @@ -3,6 +3,8 @@ package ec2 import ( "fmt" "strings" + + "github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode" ) const clientVpnAuthorizationRuleIDSeparator = "," @@ -68,3 +70,7 @@ func ClientVpnRouteParseID(id string) (string, string, string, error) { fmt.Errorf("unexpected format for ID (%q), expected endpoint-id"+clientVpnRouteIDSeparator+ "target-subnet-id"+clientVpnRouteIDSeparator+"destination-cidr-block", id) } + +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 47bf25d8557..8603eb0e892 100644 --- a/aws/internal/service/ec2/waiter/status.go +++ b/aws/internal/service/ec2/waiter/status.go @@ -204,3 +204,27 @@ func SecurityGroupStatus(conn *ec2.EC2, id string) resource.StateRefreshFunc { return group, SecurityGroupStatusCreated, nil } } + +const ( + attachmentStateNotFound = "NotFound" + attachmentStateUnknown = "Unknown" +) + +// VpnGatewayVpcAttachmentState fetches the attachment between the specified VPN gateway and VPC and its state +func VpnGatewayVpcAttachmentState(conn *ec2.EC2, vpnGatewayID, vpcID string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + vpcAttachment, err := finder.VpnGatewayVpcAttachment(conn, vpnGatewayID, vpcID) + if tfec2.ErrCodeEquals(err, tfec2.InvalidVpnGatewayIDNotFound) { + return nil, attachmentStateNotFound, nil + } + if err != nil { + return nil, attachmentStateUnknown, err + } + + if vpcAttachment == nil { + return nil, attachmentStateNotFound, nil + } + + return vpcAttachment, aws.StringValue(vpcAttachment.State), nil + } +} diff --git a/aws/internal/service/ec2/waiter/waiter.go b/aws/internal/service/ec2/waiter/waiter.go index d776dd2b85b..72bd331341a 100644 --- a/aws/internal/service/ec2/waiter/waiter.go +++ b/aws/internal/service/ec2/waiter/waiter.go @@ -199,3 +199,43 @@ func SecurityGroupCreated(conn *ec2.EC2, id string, timeout time.Duration) (*ec2 return nil, err } + +const ( + VpnGatewayVpcAttachmentAttachedTimeout = 15 * time.Minute + + VpnGatewayVpcAttachmentDetachedTimeout = 30 * time.Minute +) + +func VpnGatewayVpcAttachmentAttached(conn *ec2.EC2, vpnGatewayID, vpcID string) (*ec2.VpcAttachment, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{ec2.AttachmentStatusDetached, ec2.AttachmentStatusAttaching}, + Target: []string{ec2.AttachmentStatusAttached}, + Refresh: VpnGatewayVpcAttachmentState(conn, vpnGatewayID, vpcID), + Timeout: VpnGatewayVpcAttachmentAttachedTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*ec2.VpcAttachment); ok { + return output, err + } + + return nil, err +} + +func VpnGatewayVpcAttachmentDetached(conn *ec2.EC2, vpnGatewayID, vpcID string) (*ec2.VpcAttachment, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{ec2.AttachmentStatusAttached, ec2.AttachmentStatusDetaching}, + Target: []string{ec2.AttachmentStatusDetached}, + Refresh: VpnGatewayVpcAttachmentState(conn, vpnGatewayID, vpcID), + Timeout: VpnGatewayVpcAttachmentDetachedTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*ec2.VpcAttachment); ok { + return output, err + } + + return nil, err +} diff --git a/aws/resource_aws_vpn_gateway.go b/aws/resource_aws_vpn_gateway.go index eca95e670c0..551604b1cd5 100644 --- a/aws/resource_aws_vpn_gateway.go +++ b/aws/resource_aws_vpn_gateway.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" + "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/helper/schema" @@ -330,3 +331,42 @@ func vpnGatewayGetAttachment(vgw *ec2.VpnGateway) *ec2.VpcAttachment { } return nil } + +func vpnGatewayAttachmentStateRefresh(conn *ec2.EC2, vpcId, vgwId string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + resp, err := conn.DescribeVpnGateways(&ec2.DescribeVpnGatewaysInput{ + Filters: buildEC2AttributeFilterList(map[string]string{ + "attachment.vpc-id": vpcId, + }), + VpnGatewayIds: []*string{aws.String(vgwId)}, + }) + + if err != nil { + awsErr, ok := err.(awserr.Error) + if ok { + switch awsErr.Code() { + case "InvalidVpnGatewayID.NotFound": + fallthrough + case "InvalidVpnGatewayAttachment.NotFound": + return nil, "", nil + } + } + + return nil, "", err + } + + vgw := resp.VpnGateways[0] + + return vgw, vpnGatewayGetAttachmentState(vgw, vpcId), nil + } +} + +// vpnGatewayGetAttachmentState returns the state of any VGW attachment to the specified VPC or "detached". +func vpnGatewayGetAttachmentState(vgw *ec2.VpnGateway, vpcId string) string { + for _, vpcAttachment := range vgw.VpcAttachments { + if aws.StringValue(vpcAttachment.VpcId) == vpcId { + return aws.StringValue(vpcAttachment.State) + } + } + return ec2.AttachmentStatusDetached +} diff --git a/aws/resource_aws_vpn_gateway_attachment.go b/aws/resource_aws_vpn_gateway_attachment.go index 7d7fae06cd9..9eb10d12f01 100644 --- a/aws/resource_aws_vpn_gateway_attachment.go +++ b/aws/resource_aws_vpn_gateway_attachment.go @@ -3,14 +3,13 @@ package aws import ( "fmt" "log" - "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/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" ) func resourceAwsVpnGatewayAttachment() *schema.Resource { @@ -25,6 +24,7 @@ func resourceAwsVpnGatewayAttachment() *schema.Resource { Required: true, ForceNew: true, }, + "vpn_gateway_id": { Type: schema.TypeString, Required: true, @@ -40,36 +40,25 @@ func resourceAwsVpnGatewayAttachmentCreate(d *schema.ResourceData, meta interfac vpcId := d.Get("vpc_id").(string) vgwId := d.Get("vpn_gateway_id").(string) - createOpts := &ec2.AttachVpnGatewayInput{ + input := &ec2.AttachVpnGatewayInput{ VpcId: aws.String(vpcId), VpnGatewayId: aws.String(vgwId), } - log.Printf("[DEBUG] VPN Gateway attachment options: %#v", *createOpts) - _, err := conn.AttachVpnGateway(createOpts) + log.Printf("[DEBUG] Creating VPN Gateway Attachment: %s", input) + _, err := conn.AttachVpnGateway(input) + if err != nil { - return fmt.Errorf("Error attaching VPN Gateway %q to VPC %q: %s", - vgwId, vpcId, err) + return fmt.Errorf("error creating VPN Gateway (%s) Attachment (%s): %w", vgwId, vpcId, err) } - d.SetId(vpnGatewayAttachmentId(vpcId, vgwId)) - log.Printf("[INFO] VPN Gateway %q attachment ID: %s", vgwId, d.Id()) + d.SetId(tfec2.VpnGatewayVpcAttachmentCreateID(vgwId, vpcId)) - stateConf := &resource.StateChangeConf{ - Pending: []string{"detached", "attaching"}, - Target: []string{"attached"}, - Refresh: vpnGatewayAttachmentStateRefresh(conn, vpcId, vgwId), - Timeout: 15 * time.Minute, - Delay: 10 * time.Second, - MinTimeout: 5 * time.Second, - } + _, err = waiter.VpnGatewayVpcAttachmentAttached(conn, vgwId, vpcId) - _, err = stateConf.WaitForState() if err != nil { - return fmt.Errorf("Error waiting for VPN Gateway %q to attach to VPC %q: %s", - vgwId, vpcId, err) + return fmt.Errorf("error waiting for VPN Gateway (%s) Attachment (%s) to become attached: %w", vgwId, vpcId, err) } - log.Printf("[DEBUG] VPN Gateway %q attached to VPC %q.", vgwId, vpcId) return resourceAwsVpnGatewayAttachmentRead(d, meta) } @@ -77,36 +66,27 @@ func resourceAwsVpnGatewayAttachmentCreate(d *schema.ResourceData, meta interfac func resourceAwsVpnGatewayAttachmentRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn + vpcId := d.Get("vpc_id").(string) vgwId := d.Get("vpn_gateway_id").(string) - resp, err := conn.DescribeVpnGateways(&ec2.DescribeVpnGatewaysInput{ - VpnGatewayIds: []*string{aws.String(vgwId)}, - }) - - if err != nil { - awsErr, ok := err.(awserr.Error) - if ok && awsErr.Code() == "InvalidVpnGatewayID.NotFound" { - log.Printf("[WARN] VPN Gateway %q not found.", vgwId) - d.SetId("") - return nil - } - return err - } + vpcAttachment, err := finder.VpnGatewayVpcAttachment(conn, vgwId, vpcId) - vgw := resp.VpnGateways[0] - if *vgw.State == "deleted" { - log.Printf("[INFO] VPN Gateway %q appears to have been deleted.", vgwId) + if isAWSErr(err, tfec2.InvalidVpnGatewayIDNotFound, "") { + log.Printf("[WARN] VPN Gateway (%s) Attachment (%s) not found, removing from state", vgwId, vpcId) d.SetId("") return nil } - vga := vpnGatewayGetAttachment(vgw) - if vga == nil { - d.Set("vpc_id", "") + if err != nil { + return fmt.Errorf("error reading VPN Gateway (%s) Attachment (%s): %w", vgwId, vpcId, err) + } + + if vpcAttachment == nil || aws.StringValue(vpcAttachment.State) == ec2.AttachmentStatusDetached { + log.Printf("[WARN] VPN Gateway (%s) Attachment (%s) not found, removing from state", vgwId, vpcId) + d.SetId("") return nil } - d.Set("vpc_id", vga.VpcId) return nil } @@ -116,89 +96,25 @@ func resourceAwsVpnGatewayAttachmentDelete(d *schema.ResourceData, meta interfac vpcId := d.Get("vpc_id").(string) vgwId := d.Get("vpn_gateway_id").(string) - if vpcId == "" { - log.Printf("[DEBUG] Not detaching VPN Gateway %q as no VPC ID is set.", vgwId) - return nil - } - + log.Printf("[INFO] Deleting VPN Gateway (%s) Attachment (%s)", vgwId, vpcId) _, err := conn.DetachVpnGateway(&ec2.DetachVpnGatewayInput{ VpcId: aws.String(vpcId), VpnGatewayId: aws.String(vgwId), }) - if err != nil { - awsErr, ok := err.(awserr.Error) - if ok { - switch awsErr.Code() { - case "InvalidVpnGatewayID.NotFound": - return nil - case "InvalidVpnGatewayAttachment.NotFound": - return nil - } - } - - return fmt.Errorf("Error detaching VPN Gateway %q from VPC %q: %s", - vgwId, vpcId, err) - } - - stateConf := &resource.StateChangeConf{ - Pending: []string{"attached", "detaching"}, - Target: []string{"detached"}, - Refresh: vpnGatewayAttachmentStateRefresh(conn, vpcId, vgwId), - Timeout: 15 * time.Minute, - Delay: 10 * time.Second, - MinTimeout: 5 * time.Second, + if isAWSErr(err, tfec2.InvalidVpnGatewayAttachmentNotFound, "") || isAWSErr(err, tfec2.InvalidVpnGatewayIDNotFound, "") { + return nil } - _, err = stateConf.WaitForState() if err != nil { - return fmt.Errorf("Error waiting for VPN Gateway %q to detach from VPC %q: %s", - vgwId, vpcId, err) + return fmt.Errorf("error deleting VPN Gateway (%s) Attachment (%s): %w", vgwId, vpcId, err) } - log.Printf("[DEBUG] VPN Gateway %q detached from VPC %q.", vgwId, vpcId) - return nil -} + _, err = waiter.VpnGatewayVpcAttachmentDetached(conn, vgwId, vpcId) -func vpnGatewayAttachmentStateRefresh(conn *ec2.EC2, vpcId, vgwId string) resource.StateRefreshFunc { - return func() (interface{}, string, error) { - resp, err := conn.DescribeVpnGateways(&ec2.DescribeVpnGatewaysInput{ - Filters: buildEC2AttributeFilterList(map[string]string{ - "attachment.vpc-id": vpcId, - }), - VpnGatewayIds: []*string{aws.String(vgwId)}, - }) - - if err != nil { - awsErr, ok := err.(awserr.Error) - if ok { - switch awsErr.Code() { - case "InvalidVpnGatewayID.NotFound": - fallthrough - case "InvalidVpnGatewayAttachment.NotFound": - return nil, "", nil - } - } - - return nil, "", err - } - - vgw := resp.VpnGateways[0] - - return vgw, vpnGatewayGetAttachmentState(vgw, vpcId), nil - } -} - -// vpnGatewayGetAttachmentState returns the state of any VGW attachment to the specified VPC or "detached". -func vpnGatewayGetAttachmentState(vgw *ec2.VpnGateway, vpcId string) string { - for _, vpcAttachment := range vgw.VpcAttachments { - if aws.StringValue(vpcAttachment.VpcId) == vpcId { - return aws.StringValue(vpcAttachment.State) - } + if err != nil { + return fmt.Errorf("error waiting for VPN Gateway (%s) Attachment (%s) to become detached: %w", vgwId, vpcId, err) } - return ec2.AttachmentStatusDetached -} -func vpnGatewayAttachmentId(vpcId, vgwId string) string { - return fmt.Sprintf("vpn-attachment-%x", hashcode.String(fmt.Sprintf("%s-%s", vpcId, vgwId))) + return nil } diff --git a/aws/resource_aws_vpn_gateway_attachment_test.go b/aws/resource_aws_vpn_gateway_attachment_test.go index adf74aecc5d..d3a57423996 100644 --- a/aws/resource_aws_vpn_gateway_attachment_test.go +++ b/aws/resource_aws_vpn_gateway_attachment_test.go @@ -6,32 +6,26 @@ import ( "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" ) func TestAccAWSVpnGatewayAttachment_basic(t *testing.T) { - var vpc ec2.Vpc - var vgw ec2.VpnGateway + var v ec2.VpcAttachment + resourceName := "aws_vpn_gateway_attachment.test" + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - IDRefreshName: "aws_vpn_gateway_attachment.test", - Providers: testAccProviders, - CheckDestroy: testAccCheckVpnGatewayAttachmentDestroy, + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckVpnGatewayAttachmentDestroy, Steps: []resource.TestStep{ { - Config: testAccVpnGatewayAttachmentConfig, + Config: testAccVpnGatewayAttachmentConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckVpcExists( - "aws_vpc.test", - &vpc), - testAccCheckVpnGatewayExists( - "aws_vpn_gateway.test", - &vgw), - testAccCheckVpnGatewayAttachmentExists( - "aws_vpn_gateway_attachment.test", - &vpc, &vgw), + testAccCheckVpnGatewayAttachmentExists(resourceName, &v), ), }, }, @@ -39,50 +33,28 @@ func TestAccAWSVpnGatewayAttachment_basic(t *testing.T) { } func TestAccAWSVpnGatewayAttachment_deleted(t *testing.T) { - var vpc ec2.Vpc - var vgw ec2.VpnGateway - - testDeleted := func(n string) resource.TestCheckFunc { - return func(s *terraform.State) error { - _, ok := s.RootModule().Resources[n] - if ok { - return fmt.Errorf("Expected VPN Gateway attachment resource %q to be deleted.", n) - } - return nil - } - } + var v ec2.VpcAttachment + resourceName := "aws_vpn_gateway_attachment.test" + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - IDRefreshName: "aws_vpn_gateway_attachment.test", - Providers: testAccProviders, - CheckDestroy: testAccCheckVpnGatewayAttachmentDestroy, + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckVpnGatewayAttachmentDestroy, Steps: []resource.TestStep{ { - Config: testAccVpnGatewayAttachmentConfig, - Check: resource.ComposeTestCheckFunc( - testAccCheckVpcExists( - "aws_vpc.test", - &vpc), - testAccCheckVpnGatewayExists( - "aws_vpn_gateway.test", - &vgw), - testAccCheckVpnGatewayAttachmentExists( - "aws_vpn_gateway_attachment.test", - &vpc, &vgw), - ), - }, - { - Config: testAccNoVpnGatewayAttachmentConfig, + Config: testAccVpnGatewayAttachmentConfig(rName), Check: resource.ComposeTestCheckFunc( - testDeleted("aws_vpn_gateway_attachment.test"), + testAccCheckVpnGatewayAttachmentExists(resourceName, &v), + testAccCheckResourceDisappears(testAccProvider, resourceAwsVpnGatewayAttachment(), resourceName), ), + ExpectNonEmptyPlan: true, }, }, }) } -func testAccCheckVpnGatewayAttachmentExists(n string, vpc *ec2.Vpc, vgw *ec2.VpnGateway) resource.TestCheckFunc { +func testAccCheckVpnGatewayAttachmentExists(n string, v *ec2.VpcAttachment) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -93,23 +65,20 @@ func testAccCheckVpnGatewayAttachmentExists(n string, vpc *ec2.Vpc, vgw *ec2.Vpn return fmt.Errorf("No ID is set") } - vpcId := rs.Primary.Attributes["vpc_id"] - vgwId := rs.Primary.Attributes["vpn_gateway_id"] - - if len(vgw.VpcAttachments) == 0 { - return fmt.Errorf("VPN Gateway %q has no attachments.", vgwId) + conn := testAccProvider.Meta().(*AWSClient).ec2conn + out, err := finder.VpnGatewayVpcAttachment(conn, rs.Primary.Attributes["vpn_gateway_id"], rs.Primary.Attributes["vpc_id"]) + if err != nil { + return err } - - if *vgw.VpcAttachments[0].State != "attached" { - return fmt.Errorf("Expected VPN Gateway %q to be in attached state, but got: %q", - vgwId, *vgw.VpcAttachments[0].State) + if out == nil { + return fmt.Errorf("VPN Gateway Attachment not found") } - - if *vgw.VpcAttachments[0].VpcId != *vpc.VpcId { - return fmt.Errorf("Expected VPN Gateway %q to be attached to VPC %q, but got: %q", - vgwId, vpcId, *vgw.VpcAttachments[0].VpcId) + if state := aws.StringValue(out.State); state != ec2.AttachmentStatusAttached { + return fmt.Errorf("VPN Gateway Attachment in incorrect state. Expected: %s, got: %s", ec2.AttachmentStatusAttached, state) } + *v = *out + return nil } } @@ -122,50 +91,40 @@ func testAccCheckVpnGatewayAttachmentDestroy(s *terraform.State) error { continue } - vgwId := rs.Primary.Attributes["vpn_gateway_id"] - - resp, err := conn.DescribeVpnGateways(&ec2.DescribeVpnGatewaysInput{ - VpnGatewayIds: []*string{aws.String(vgwId)}, - }) + out, err := finder.VpnGatewayVpcAttachment(conn, rs.Primary.Attributes["vpn_gateway_id"], rs.Primary.Attributes["vpc_id"]) if err != nil { return err } - - vgw := resp.VpnGateways[0] - if *vgw.VpcAttachments[0].State != "detached" { - return fmt.Errorf("Expected VPN Gateway %q to be in detached state, but got: %q", - vgwId, *vgw.VpcAttachments[0].State) + if out == nil { + continue + } + if state := aws.StringValue(out.State); state != ec2.AttachmentStatusDetached { + return fmt.Errorf("VPN Gateway Attachment in incorrect state. Expected: %s, got: %s", ec2.AttachmentStatusDetached, state) } } return nil } -const testAccNoVpnGatewayAttachmentConfig = ` +func testAccVpnGatewayAttachmentConfig(rName string) string { + return fmt.Sprintf(` resource "aws_vpc" "test" { cidr_block = "10.0.0.0/16" tags = { - Name = "terraform-testacc-vpn-gateway-attachment-basic" + Name = %[1]q } } -resource "aws_vpn_gateway" "test" {} -` - -const testAccVpnGatewayAttachmentConfig = ` -resource "aws_vpc" "test" { - cidr_block = "10.0.0.0/16" - +resource "aws_vpn_gateway" "test" { tags = { - Name = "terraform-testacc-vpn-gateway-attachment-deleted" + Name = %[1]q } } -resource "aws_vpn_gateway" "test" {} - resource "aws_vpn_gateway_attachment" "test" { vpc_id = aws_vpc.test.id vpn_gateway_id = aws_vpn_gateway.test.id } -` +`, rName) +} From 73969d7475a26cf9809311d7e9b2466bbcf5927c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 17 Sep 2020 13:19:59 -0400 Subject: [PATCH 2/4] r/aws_vpn_gateway: Refactor test sweeper to call aws_vpn_gateway and aws_vpn_gateway_attachment Delete methods. Acceptance test output: $ TEST=./aws SWEEP=us-west-2,us-east-1 SWEEPARGS=-sweep-run=aws_vpn_gateway make sweep WARNING: This will destroy infrastructure. Use only in development accounts. go test ./aws -v -sweep=us-west-2,us-east-1 -sweep-run=aws_vpn_gateway -timeout 60m 2020/09/17 14:01:43 [DEBUG] Running Sweepers for region (us-west-2): 2020/09/17 14:01:43 [DEBUG] Sweeper (aws_vpn_gateway) has dependency (aws_dx_gateway_association), running.. 2020/09/17 14:01:43 [DEBUG] Sweeper (aws_dx_gateway_association) has dependency (aws_dx_gateway_association_proposal), running.. 2020/09/17 14:01:43 [DEBUG] Running Sweeper (aws_dx_gateway_association_proposal) in region (us-west-2) 2020/09/17 14:01:43 [INFO] AWS Auth provider used: "EnvProvider" 2020/09/17 14:01:43 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2020/09/17 14:01:44 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2020/09/17 14:01:45 [DEBUG] Running Sweeper (aws_dx_gateway_association) in region (us-west-2) 2020/09/17 14:01:46 [DEBUG] Running Sweeper (aws_vpn_gateway) in region (us-west-2) 2020/09/17 14:01:46 [INFO] Deleting VPN Gateway (vgw-0b8054188ab62b680) Attachment (vpc-f2e16e8b) 2020/09/17 14:01:47 [DEBUG] Waiting for state to become: [detached] 2020/09/17 14:01:47 [DEBUG] Not detaching VPN Gateway 'vgw-0b8054188ab62b680' as no VPC ID is set 2020/09/17 14:01:47 [INFO] Deleting VPN gateway: vgw-0b8054188ab62b680 2020/09/17 14:01:47 [DEBUG] Waiting for state to become: [success] 2020/09/17 14:01:48 [DEBUG] Sweeper (aws_dx_gateway_association) has dependency (aws_dx_gateway_association_proposal), running.. 2020/09/17 14:01:48 [DEBUG] Sweeper (aws_dx_gateway_association_proposal) already ran in region (us-west-2) 2020/09/17 14:01:48 [DEBUG] Sweeper (aws_dx_gateway_association) already ran in region (us-west-2) 2020/09/17 14:01:48 [DEBUG] Sweeper (aws_dx_gateway_association_proposal) already ran in region (us-west-2) 2020/09/17 14:01:48 Sweeper Tests ran successfully: - aws_vpn_gateway - aws_dx_gateway_association_proposal - aws_dx_gateway_association 2020/09/17 14:01:48 [DEBUG] Running Sweepers for region (us-east-1): 2020/09/17 14:01:48 [DEBUG] Sweeper (aws_vpn_gateway) has dependency (aws_dx_gateway_association), running.. 2020/09/17 14:01:48 [DEBUG] Sweeper (aws_dx_gateway_association) has dependency (aws_dx_gateway_association_proposal), running.. 2020/09/17 14:01:48 [DEBUG] Running Sweeper (aws_dx_gateway_association_proposal) in region (us-east-1) 2020/09/17 14:01:48 [INFO] AWS Auth provider used: "EnvProvider" 2020/09/17 14:01:48 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2020/09/17 14:01:48 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2020/09/17 14:01:48 [DEBUG] Running Sweeper (aws_dx_gateway_association) in region (us-east-1) 2020/09/17 14:01:49 [DEBUG] Running Sweeper (aws_vpn_gateway) in region (us-east-1) 2020/09/17 14:01:49 [DEBUG] No VPN Gateways to sweep 2020/09/17 14:01:49 [DEBUG] Sweeper (aws_dx_gateway_association) has dependency (aws_dx_gateway_association_proposal), running.. 2020/09/17 14:01:49 [DEBUG] Sweeper (aws_dx_gateway_association_proposal) already ran in region (us-east-1) 2020/09/17 14:01:49 [DEBUG] Sweeper (aws_dx_gateway_association) already ran in region (us-east-1) 2020/09/17 14:01:49 [DEBUG] Sweeper (aws_dx_gateway_association_proposal) already ran in region (us-east-1) 2020/09/17 14:01:49 Sweeper Tests ran successfully: - aws_dx_gateway_association_proposal - aws_dx_gateway_association - aws_vpn_gateway ok github.com/terraform-providers/terraform-provider-aws/aws 5.308s --- aws/resource_aws_vpn_gateway_test.go | 55 +++++++++------------------- 1 file changed, 18 insertions(+), 37 deletions(-) diff --git a/aws/resource_aws_vpn_gateway_test.go b/aws/resource_aws_vpn_gateway_test.go index 1c98cd36b7e..3bec0525882 100644 --- a/aws/resource_aws_vpn_gateway_test.go +++ b/aws/resource_aws_vpn_gateway_test.go @@ -5,11 +5,11 @@ import ( "log" "regexp" "testing" - "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/go-multierror" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) @@ -31,6 +31,7 @@ func testSweepVPNGateways(region string) error { return fmt.Errorf("error getting client: %s", err) } conn := client.(*AWSClient).ec2conn + var sweeperErrs *multierror.Error req := &ec2.DescribeVpnGatewaysInput{} resp, err := conn.DescribeVpnGateways(req) @@ -57,52 +58,32 @@ func testSweepVPNGateways(region string) error { continue } - input := &ec2.DetachVpnGatewayInput{ - VpcId: vpcAttachment.VpcId, - VpnGatewayId: vpng.VpnGatewayId, - } - - log.Printf("[DEBUG] Detaching VPN Gateway: %s", input) - _, err := conn.DetachVpnGateway(input) - - if isAWSErr(err, "InvalidVpnGatewayAttachment.NotFound", "") || isAWSErr(err, "InvalidVpnGatewayID.NotFound", "") { - continue - } + r := resourceAwsVpnGatewayAttachment() + d := r.Data(nil) + d.Set("vpc_id", vpcAttachment.VpcId) + d.Set("vpn_gateway_id", vpng.VpnGatewayId) + err := r.Delete(d, client) if err != nil { - return fmt.Errorf("error detaching VPN Gateway (%s) from VPC (%s): %s", aws.StringValue(vpng.VpnGatewayId), aws.StringValue(vpcAttachment.VpcId), err) - } - - stateConf := &resource.StateChangeConf{ - Pending: []string{ec2.AttachmentStatusAttached, ec2.AttachmentStatusDetaching}, - Target: []string{ec2.AttachmentStatusDetached}, - Refresh: vpnGatewayAttachmentStateRefresh(conn, aws.StringValue(vpcAttachment.VpcId), aws.StringValue(vpng.VpnGatewayId)), - Timeout: 10 * time.Minute, - } - - log.Printf("[DEBUG] Waiting for VPN Gateway (%s) to detach from VPC (%s)", aws.StringValue(vpng.VpnGatewayId), aws.StringValue(vpcAttachment.VpcId)) - if _, err = stateConf.WaitForState(); err != nil { - return fmt.Errorf("error waiting for VPN Gateway (%s) to detach from VPC (%s): %s", aws.StringValue(vpng.VpnGatewayId), aws.StringValue(vpcAttachment.VpcId), err) + log.Printf("[ERROR] %s", err) + sweeperErrs = multierror.Append(sweeperErrs, err) + continue } } - input := &ec2.DeleteVpnGatewayInput{ - VpnGatewayId: vpng.VpnGatewayId, - } - - log.Printf("[DEBUG] Deleting VPN Gateway: %s", input) - _, err := conn.DeleteVpnGateway(input) - - if isAWSErr(err, "InvalidVpnGatewayID.NotFound", "") { - continue - } + r := resourceAwsVpnGateway() + d := r.Data(nil) + d.SetId(aws.StringValue(vpng.VpnGatewayId)) + err := r.Delete(d, client) if err != nil { - return fmt.Errorf("error deleting VPN Gateway (%s): %s", aws.StringValue(vpng.VpnGatewayId), err) + log.Printf("[ERROR] %s", err) + sweeperErrs = multierror.Append(sweeperErrs, err) + continue } } - return nil + return sweeperErrs.ErrorOrNil() } func TestAccAWSVpnGateway_basic(t *testing.T) { From 1dbf6627f31a3dc68c2433c6ce61b84ad4a9c058 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 17 Sep 2020 14:12:51 -0400 Subject: [PATCH 3/4] r/aws_vpn_gateway: Refactor using internal 'waiter' package. Increase detachment timeout to 30m. Acceptance test output: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVpnGateway_' ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=TestAccAWSVpnGateway_ -timeout 120m === RUN TestAccAWSVpnGateway_basic === PAUSE TestAccAWSVpnGateway_basic === RUN TestAccAWSVpnGateway_withAvailabilityZoneSetToState === PAUSE TestAccAWSVpnGateway_withAvailabilityZoneSetToState === RUN TestAccAWSVpnGateway_withAmazonSideAsnSetToState === PAUSE TestAccAWSVpnGateway_withAmazonSideAsnSetToState === RUN TestAccAWSVpnGateway_disappears === PAUSE TestAccAWSVpnGateway_disappears === RUN TestAccAWSVpnGateway_reattach === PAUSE TestAccAWSVpnGateway_reattach === RUN TestAccAWSVpnGateway_delete === PAUSE TestAccAWSVpnGateway_delete === RUN TestAccAWSVpnGateway_tags === PAUSE TestAccAWSVpnGateway_tags === CONT TestAccAWSVpnGateway_basic === CONT TestAccAWSVpnGateway_reattach --- PASS: TestAccAWSVpnGateway_basic (87.01s) === CONT TestAccAWSVpnGateway_tags --- PASS: TestAccAWSVpnGateway_reattach (123.40s) === CONT TestAccAWSVpnGateway_delete --- PASS: TestAccAWSVpnGateway_delete (60.46s) === CONT TestAccAWSVpnGateway_withAmazonSideAsnSetToState --- PASS: TestAccAWSVpnGateway_tags (97.73s) === CONT TestAccAWSVpnGateway_disappears resource_aws_vpn_gateway_test.go:195: [INFO] Got non-empty plan, as expected --- PASS: TestAccAWSVpnGateway_disappears (40.79s) === CONT TestAccAWSVpnGateway_withAvailabilityZoneSetToState --- PASS: TestAccAWSVpnGateway_withAmazonSideAsnSetToState (52.12s) --- PASS: TestAccAWSVpnGateway_withAvailabilityZoneSetToState (47.02s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 272.597s --- aws/resource_aws_vpn_gateway.go | 88 +++++----------------------- aws/resource_aws_vpn_gateway_test.go | 8 +++ 2 files changed, 23 insertions(+), 73 deletions(-) diff --git a/aws/resource_aws_vpn_gateway.go b/aws/resource_aws_vpn_gateway.go index 551604b1cd5..37adcd08f8c 100644 --- a/aws/resource_aws_vpn_gateway.go +++ b/aws/resource_aws_vpn_gateway.go @@ -8,11 +8,12 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" - "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/helper/schema" "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/waiter" ) func resourceAwsVpnGateway() *schema.Resource { @@ -234,7 +235,7 @@ func resourceAwsVpnGatewayAttach(d *schema.ResourceData, meta interface{}) error err := resource.Retry(1*time.Minute, func() *resource.RetryError { _, err := conn.AttachVpnGateway(req) if err != nil { - if isAWSErr(err, "InvalidVpnGatewayID.NotFound", "") { + if isAWSErr(err, tfec2.InvalidVpnGatewayIDNotFound, "") { return resource.RetryableError(err) } return resource.NonRetryableError(err) @@ -251,14 +252,10 @@ func resourceAwsVpnGatewayAttach(d *schema.ResourceData, meta interface{}) error // Wait for it to be fully attached before continuing log.Printf("[DEBUG] Waiting for VPN gateway (%s) to attach", d.Id()) - stateConf := &resource.StateChangeConf{ - Pending: []string{ec2.AttachmentStatusDetached, ec2.AttachmentStatusAttaching}, - Target: []string{ec2.AttachmentStatusAttached}, - Refresh: vpnGatewayAttachmentStateRefresh(conn, vpcId, d.Id()), - Timeout: 15 * time.Minute, - } - if _, err := stateConf.WaitForState(); err != nil { - return fmt.Errorf("Error waiting for VPN gateway (%s) to attach: %s", d.Id(), err) + _, err = waiter.VpnGatewayVpcAttachmentAttached(conn, d.Id(), vpcId) + + if err != nil { + return fmt.Errorf("error waiting for VPN Gateway (%s) Attachment (%s) to become attached: %w", d.Id(), vpcId, err) } return nil @@ -283,40 +280,24 @@ func resourceAwsVpnGatewayDetach(d *schema.ResourceData, meta interface{}) error d.Id(), vpcId) - wait := true _, err := conn.DetachVpnGateway(&ec2.DetachVpnGatewayInput{ VpnGatewayId: aws.String(d.Id()), VpcId: aws.String(vpcId), }) - if err != nil { - if isAWSErr(err, "InvalidVpnGatewayID.NotFound", "") { - err = nil - wait = false - } - if isAWSErr(err, "InvalidVpnGatewayAttachment.NotFound", "") { - err = nil - wait = false - } - if err != nil { - return err - } + if isAWSErr(err, tfec2.InvalidVpnGatewayAttachmentNotFound, "") || isAWSErr(err, tfec2.InvalidVpnGatewayIDNotFound, "") { + return nil } - if !wait { - return nil + if err != nil { + return fmt.Errorf("error deleting VPN Gateway (%s) Attachment (%s): %w", d.Id(), vpcId, err) } // Wait for it to be fully detached before continuing - log.Printf("[DEBUG] Waiting for VPN gateway (%s) to detach", d.Id()) - stateConf := &resource.StateChangeConf{ - Pending: []string{ec2.AttachmentStatusAttached, ec2.AttachmentStatusDetaching, "available"}, - Target: []string{ec2.AttachmentStatusDetached}, - Refresh: vpnGatewayAttachmentStateRefresh(conn, vpcId, d.Id()), - Timeout: 10 * time.Minute, - } - if _, err := stateConf.WaitForState(); err != nil { - return fmt.Errorf("Error waiting for vpn gateway (%s) to detach: %s", d.Id(), err) + _, err = waiter.VpnGatewayVpcAttachmentDetached(conn, d.Id(), vpcId) + + if err != nil { + return fmt.Errorf("error waiting for VPN Gateway (%s) Attachment (%s) to become detached: %w", d.Id(), vpcId, err) } return nil @@ -331,42 +312,3 @@ func vpnGatewayGetAttachment(vgw *ec2.VpnGateway) *ec2.VpcAttachment { } return nil } - -func vpnGatewayAttachmentStateRefresh(conn *ec2.EC2, vpcId, vgwId string) resource.StateRefreshFunc { - return func() (interface{}, string, error) { - resp, err := conn.DescribeVpnGateways(&ec2.DescribeVpnGatewaysInput{ - Filters: buildEC2AttributeFilterList(map[string]string{ - "attachment.vpc-id": vpcId, - }), - VpnGatewayIds: []*string{aws.String(vgwId)}, - }) - - if err != nil { - awsErr, ok := err.(awserr.Error) - if ok { - switch awsErr.Code() { - case "InvalidVpnGatewayID.NotFound": - fallthrough - case "InvalidVpnGatewayAttachment.NotFound": - return nil, "", nil - } - } - - return nil, "", err - } - - vgw := resp.VpnGateways[0] - - return vgw, vpnGatewayGetAttachmentState(vgw, vpcId), nil - } -} - -// vpnGatewayGetAttachmentState returns the state of any VGW attachment to the specified VPC or "detached". -func vpnGatewayGetAttachmentState(vgw *ec2.VpnGateway, vpcId string) string { - for _, vpcAttachment := range vgw.VpcAttachments { - if aws.StringValue(vpcAttachment.VpcId) == vpcId { - return aws.StringValue(vpcAttachment.State) - } - } - return ec2.AttachmentStatusDetached -} diff --git a/aws/resource_aws_vpn_gateway_test.go b/aws/resource_aws_vpn_gateway_test.go index 3bec0525882..ebc40c6e764 100644 --- a/aws/resource_aws_vpn_gateway_test.go +++ b/aws/resource_aws_vpn_gateway_test.go @@ -478,6 +478,14 @@ resource "aws_vpn_gateway" "test" { ` const testAccVpnGatewayConfigChangeVPC = ` +resource "aws_vpc" "test" { + cidr_block = "10.1.0.0/16" + + tags = { + Name = "terraform-testacc-vpn-gateway" + } +} + resource "aws_vpc" "test2" { cidr_block = "10.2.0.0/16" From 2ce1dbb9a63100f202fd21f6742e2bfc19f39b28 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 17 Sep 2020 14:24:27 -0400 Subject: [PATCH 4/4] 'TestAccAWSVpnGatewayAttachment_deleted' -> 'TestAccAWSVpnGatewayAttachment_disappears' (#13826, #13527). --- aws/resource_aws_vpn_gateway_attachment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_vpn_gateway_attachment_test.go b/aws/resource_aws_vpn_gateway_attachment_test.go index d3a57423996..8bd7068ff30 100644 --- a/aws/resource_aws_vpn_gateway_attachment_test.go +++ b/aws/resource_aws_vpn_gateway_attachment_test.go @@ -32,7 +32,7 @@ func TestAccAWSVpnGatewayAttachment_basic(t *testing.T) { }) } -func TestAccAWSVpnGatewayAttachment_deleted(t *testing.T) { +func TestAccAWSVpnGatewayAttachment_disappears(t *testing.T) { var v ec2.VpcAttachment resourceName := "aws_vpn_gateway_attachment.test" rName := acctest.RandomWithPrefix("tf-acc-test")