diff --git a/aws/resource_aws_eip.go b/aws/resource_aws_eip.go index 837f05a559e8..6bc3f6273275 100644 --- a/aws/resource_aws_eip.go +++ b/aws/resource_aws_eip.go @@ -10,9 +10,16 @@ import ( "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/keyvaluetags" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" +) + +const ( + // Maximum amount of time to wait for EIP association with EC2-Classic instances + ec2AddressAssociationClassicTimeout = 2 * time.Minute ) func resourceAwsEip() *schema.Resource { @@ -376,6 +383,12 @@ func resourceAwsEipUpdate(d *schema.ResourceData, meta interface{}) error { d.Set("network_interface", "") return fmt.Errorf("Failure associating EIP: %s", err) } + + if assocOpts.AllocationId == nil { + if err := waitForEc2AddressAssociationClassic(ec2conn, aws.StringValue(assocOpts.PublicIp), aws.StringValue(assocOpts.InstanceId)); err != nil { + return fmt.Errorf("error waiting for EC2 Address (%s) to associate with EC2-Classic Instance (%s): %w", aws.StringValue(assocOpts.PublicIp), aws.StringValue(assocOpts.InstanceId), err) + } + } } if d.HasChange("tags") && !d.IsNewResource() { @@ -489,3 +502,53 @@ func disassociateEip(d *schema.ResourceData, meta interface{}) error { } return err } + +// waitForEc2AddressAssociationClassic ensures the correct Instance is associated with an Address +// +// This can take a few seconds to appear correctly for EC2-Classic addresses. +func waitForEc2AddressAssociationClassic(conn *ec2.EC2, publicIP string, instanceID string) error { + input := &ec2.DescribeAddressesInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("public-ip"), + Values: []*string{aws.String(publicIP)}, + }, + { + Name: aws.String("domain"), + Values: []*string{aws.String(ec2.DomainTypeStandard)}, + }, + }, + } + + err := resource.Retry(ec2AddressAssociationClassicTimeout, func() *resource.RetryError { + output, err := conn.DescribeAddresses(input) + + if tfawserr.ErrCodeEquals(err, "InvalidAddress.NotFound") { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) + } + + if len(output.Addresses) == 0 || output.Addresses[0] == nil { + return resource.RetryableError(fmt.Errorf("not found")) + } + + if aws.StringValue(output.Addresses[0].InstanceId) != instanceID { + return resource.RetryableError(fmt.Errorf("not associated")) + } + + return nil + }) + + if tfresource.TimedOut(err) { + _, err = conn.DescribeAddresses(input) + } + + if err != nil { + return fmt.Errorf("error describing EC2 Address (%s) association: %w", publicIP, err) + } + + return nil +} diff --git a/aws/resource_aws_eip_association.go b/aws/resource_aws_eip_association.go index 40791b4e3475..01cc47aba138 100644 --- a/aws/resource_aws_eip_association.go +++ b/aws/resource_aws_eip_association.go @@ -8,6 +8,7 @@ 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/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -96,12 +97,25 @@ func resourceAwsEipAssociationCreate(d *schema.ResourceData, meta interface{}) e err := resource.Retry(2*time.Minute, func() *resource.RetryError { var err error resp, err = conn.AssociateAddress(request) + + // EC2-VPC error for new addresses + if tfawserr.ErrCodeEquals(err, "InvalidAllocationID.NotFound") { + return resource.RetryableError(err) + } + + // EC2-Classic error for new addresses + if tfawserr.ErrMessageContains(err, "AuthFailure", "does not belong to you") { + return resource.RetryableError(err) + } + + if tfawserr.ErrMessageContains(err, "InvalidInstanceID", "pending instance") { + return resource.RetryableError(err) + } + if err != nil { - if isAWSErr(err, "InvalidInstanceID", "pending instance") { - return resource.RetryableError(err) - } return resource.NonRetryableError(err) } + return nil }) if isResourceTimeoutError(err) { @@ -121,11 +135,15 @@ func resourceAwsEipAssociationCreate(d *schema.ResourceData, meta interface{}) e supportedPlatforms, resp) } - if resp.AssociationId == nil { - // This is required field for EC2 Classic per docs - d.SetId(*request.PublicIp) + if resp.AssociationId != nil { + d.SetId(aws.StringValue(resp.AssociationId)) } else { - d.SetId(*resp.AssociationId) + // EC2-Classic + d.SetId(aws.StringValue(request.PublicIp)) + + if err := waitForEc2AddressAssociationClassic(conn, aws.StringValue(request.PublicIp), aws.StringValue(request.InstanceId)); err != nil { + return fmt.Errorf("error waiting for EC2 Address (%s) to associate with EC2-Classic Instance (%s): %w", aws.StringValue(request.PublicIp), aws.StringValue(request.InstanceId), err) + } } return resourceAwsEipAssociationRead(d, meta) diff --git a/aws/resource_aws_eip_association_test.go b/aws/resource_aws_eip_association_test.go index b235541af851..7f6e01078b18 100644 --- a/aws/resource_aws_eip_association_test.go +++ b/aws/resource_aws_eip_association_test.go @@ -2,7 +2,6 @@ package aws import ( "fmt" - "os" "testing" "github.com/aws/aws-sdk-go/aws" @@ -17,9 +16,9 @@ func TestAccAWSEIPAssociation_instance(t *testing.T) { var a ec2.Address resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSEIPAssociationDestroy, + PreCheck: func() { testAccPreCheck(t) }, + ProviderFactories: testAccProviderFactories, + CheckDestroy: testAccCheckAWSEIPAssociationDestroy, Steps: []resource.TestStep{ { Config: testAccAWSEIPAssociationConfig_instance(), @@ -42,9 +41,9 @@ func TestAccAWSEIPAssociation_networkInterface(t *testing.T) { var a ec2.Address resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSEIPAssociationDestroy, + PreCheck: func() { testAccPreCheck(t) }, + ProviderFactories: testAccProviderFactories, + CheckDestroy: testAccCheckAWSEIPAssociationDestroy, Steps: []resource.TestStep{ { Config: testAccAWSEIPAssociationConfig_networkInterface, @@ -67,9 +66,9 @@ func TestAccAWSEIPAssociation_basic(t *testing.T) { resourceName := "aws_eip_association.by_allocation_id" resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t); testAccEC2VPCOnlyPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSEIPAssociationDestroy, + PreCheck: func() { testAccPreCheck(t); testAccEC2VPCOnlyPreCheck(t) }, + ProviderFactories: testAccProviderFactories, + CheckDestroy: testAccCheckAWSEIPAssociationDestroy, Steps: []resource.TestStep{ { Config: testAccAWSEIPAssociationConfig(), @@ -95,21 +94,16 @@ func TestAccAWSEIPAssociation_ec2Classic(t *testing.T) { var a ec2.Address resourceName := "aws_eip_association.test" - oldvar := os.Getenv("AWS_DEFAULT_REGION") - os.Setenv("AWS_DEFAULT_REGION", "us-east-1") - defer os.Setenv("AWS_DEFAULT_REGION", oldvar) - - // This test cannot run in parallel with the other EIP Association tests - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSEIPAssociationDestroy, + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) }, + ProviderFactories: testAccProviderFactories, + CheckDestroy: testAccCheckAWSEIPAssociationDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSEIPAssociationConfig_ec2Classic, + Config: testAccAWSEIPAssociationConfig_ec2Classic(), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSEIPExists("aws_eip.test", &a), - testAccCheckAWSEIPAssociationExists(resourceName, &a), + testAccCheckAWSEIPEc2ClassicExists("aws_eip.test", &a), + testAccCheckAWSEIPAssociationEc2ClassicExists(resourceName, &a), resource.TestCheckResourceAttrSet(resourceName, "public_ip"), resource.TestCheckResourceAttr(resourceName, "allocation_id", ""), testAccCheckAWSEIPAssociationHasIpBasedId(resourceName), @@ -130,9 +124,9 @@ func TestAccAWSEIPAssociation_spotInstance(t *testing.T) { resourceName := "aws_eip_association.test" resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSEIPAssociationDestroy, + PreCheck: func() { testAccPreCheck(t) }, + ProviderFactories: testAccProviderFactories, + CheckDestroy: testAccCheckAWSEIPAssociationDestroy, Steps: []resource.TestStep{ { Config: testAccAWSEIPAssociationConfig_spotInstance(rInt), @@ -158,9 +152,9 @@ func TestAccAWSEIPAssociation_disappears(t *testing.T) { resourceName := "aws_eip_association.test" resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSEIPAssociationDestroy, + PreCheck: func() { testAccPreCheck(t) }, + ProviderFactories: testAccProviderFactories, + CheckDestroy: testAccCheckAWSEIPAssociationDestroy, Steps: []resource.TestStep{ { Config: testAccAWSEIPAssociationConfigDisappears(), @@ -208,6 +202,42 @@ func testAccCheckAWSEIPAssociationExists(name string, res *ec2.Address) resource } } +func testAccCheckAWSEIPAssociationEc2ClassicExists(name string, res *ec2.Address) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[name] + if !ok { + return fmt.Errorf("Not found: %s", name) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No EIP Association ID is set") + } + + conn := testAccProviderEc2Classic.Meta().(*AWSClient).ec2conn + platforms := testAccProviderEc2Classic.Meta().(*AWSClient).supportedplatforms + + request, err := describeAddressesById(rs.Primary.ID, platforms) + + if err != nil { + return err + } + + describe, err := conn.DescribeAddresses(request) + + if err != nil { + return fmt.Errorf("error describing EC2 Address (%s): %w", rs.Primary.ID, err) + } + + if len(describe.Addresses) != 1 || aws.StringValue(describe.Addresses[0].PublicIp) != rs.Primary.ID { + return fmt.Errorf("EC2 Address (%s) not found", rs.Primary.ID) + } + + *res = *describe.Addresses[0] + + return nil + } +} + func testAccCheckAWSEIPAssociationHasIpBasedId(name string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[name] @@ -385,51 +415,25 @@ resource "aws_eip_association" "test" { `) } -const testAccAWSEIPAssociationConfig_ec2Classic = ` -provider "aws" { - region = "us-east-1" -} - +func testAccAWSEIPAssociationConfig_ec2Classic() string { + return composeConfig( + testAccEc2ClassicRegionProviderConfig(), + testAccLatestAmazonLinuxPvEbsAmiConfig(), + testAccAvailableEc2InstanceTypeForRegion("t1.micro", "m3.medium", "m3.large", "c3.large", "r3.large"), + ` resource "aws_eip" "test" {} -data "aws_availability_zones" "available" { - state = "available" - - filter { - name = "opt-in-status" - values = ["opt-in-not-required"] - } -} - -data "aws_ami" "ubuntu" { - most_recent = true - - filter { - name = "name" - values = ["ubuntu/images/ebs/ubuntu-trusty-14.04-i386-server-*"] - } - - filter { - name = "virtualization-type" - values = ["paravirtual"] - } - - owners = ["099720109477"] # Canonical -} - resource "aws_instance" "test" { - ami = data.aws_ami.ubuntu.id - availability_zone = data.aws_availability_zones.available.names[0] - - # tflint-ignore: aws_instance_previous_type - instance_type = "t1.micro" + ami = data.aws_ami.amzn-ami-minimal-pv-ebs.id + instance_type = data.aws_ec2_instance_type_offering.available.instance_type } resource "aws_eip_association" "test" { public_ip = aws_eip.test.public_ip instance_id = aws_instance.test.id } -` +`) +} func testAccAWSEIPAssociationConfig_spotInstance(rInt int) string { return composeConfig( diff --git a/aws/resource_aws_eip_test.go b/aws/resource_aws_eip_test.go index ef7a39fa7e60..858189b4918a 100644 --- a/aws/resource_aws_eip_test.go +++ b/aws/resource_aws_eip_test.go @@ -10,6 +10,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/aws/endpoints" "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" @@ -86,24 +87,20 @@ func testSweepEc2Eips(region string) error { } func TestAccAWSEIP_Ec2Classic(t *testing.T) { - oldvar := os.Getenv("AWS_DEFAULT_REGION") - os.Setenv("AWS_DEFAULT_REGION", "us-east-1") - defer os.Setenv("AWS_DEFAULT_REGION", oldvar) - resourceName := "aws_eip.test" var conf ec2.Address - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSEIPDestroy, + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) }, + ProviderFactories: testAccProviderFactories, + CheckDestroy: testAccCheckAWSEIPDestroy, Steps: []resource.TestStep{ { Config: testAccAWSEIPInstanceEc2Classic(), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSEIPExists(resourceName, &conf), + testAccCheckAWSEIPEc2ClassicExists(resourceName, &conf), testAccCheckAWSEIPAttributes(&conf), - testAccCheckAWSEIPPublicDNS(resourceName), + testAccCheckAWSEIPPublicDNSEc2Classic(resourceName), resource.TestCheckResourceAttr(resourceName, "domain", ec2.DomainTypeStandard), ), }, @@ -400,13 +397,9 @@ func TestAccAWSEIP_tags_Vpc(t *testing.T) { } func TestAccAWSEIP_tags_Ec2Classic(t *testing.T) { - oldvar := os.Getenv("AWS_DEFAULT_REGION") - os.Setenv("AWS_DEFAULT_REGION", "us-east-1") - defer os.Setenv("AWS_DEFAULT_REGION", oldvar) - rName1 := fmt.Sprintf("%s-%d", t.Name(), acctest.RandInt()) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSEIPDestroy, @@ -622,6 +615,39 @@ func testAccCheckAWSEIPExists(n string, res *ec2.Address) resource.TestCheckFunc } } +func testAccCheckAWSEIPEc2ClassicExists(n string, res *ec2.Address) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No EIP ID is set") + } + + conn := testAccProviderEc2Classic.Meta().(*AWSClient).ec2conn + + input := &ec2.DescribeAddressesInput{ + PublicIps: []*string{aws.String(rs.Primary.ID)}, + } + + describe, err := conn.DescribeAddresses(input) + + if err != nil { + return fmt.Errorf("error describing EC2 Address (%s): %w", rs.Primary.ID, err) + } + + if len(describe.Addresses) != 1 || aws.StringValue(describe.Addresses[0].PublicIp) != rs.Primary.ID { + return fmt.Errorf("EC2 Address (%s) not found", rs.Primary.ID) + } + + *res = *describe.Addresses[0] + + return nil + } +} + func testAccCheckAWSEIPPrivateDNS(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resourceName] @@ -668,6 +694,29 @@ func testAccCheckAWSEIPPublicDNS(resourceName string) resource.TestCheckFunc { } } +func testAccCheckAWSEIPPublicDNSEc2Classic(resourceName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[resourceName] + if !ok { + return fmt.Errorf("Not found: %s", resourceName) + } + + publicIPDashed := strings.Replace(rs.Primary.Attributes["public_ip"], ".", "-", -1) + publicDNS := rs.Primary.Attributes["public_dns"] + expectedPublicDNS := fmt.Sprintf("ec2-%s.%s.compute.%s", publicIPDashed, testAccGetEc2ClassicRegion(), testAccGetPartitionDNSSuffix()) + + if testAccGetEc2ClassicRegion() == endpoints.UsEast1RegionID { + expectedPublicDNS = fmt.Sprintf("ec2-%s.compute-1.%s", publicIPDashed, testAccGetPartitionDNSSuffix()) + } + + if publicDNS != expectedPublicDNS { + return fmt.Errorf("expected public_dns value (%s), received: %s", expectedPublicDNS, publicDNS) + } + + return nil + } +} + const testAccAWSEIPConfig = ` resource "aws_eip" "test" { } @@ -716,16 +765,13 @@ resource "aws_eip" "test" { func testAccAWSEIPInstanceEc2Classic() string { return composeConfig( - testAccLatestAmazonLinuxPvInstanceStoreAmiConfig(), ` -provider "aws" { - region = "us-east-1" -} - + testAccEc2ClassicRegionProviderConfig(), + testAccLatestAmazonLinuxPvEbsAmiConfig(), + testAccAvailableEc2InstanceTypeForRegion("t1.micro", "m3.medium", "m3.large", "c3.large", "r3.large"), + ` resource "aws_instance" "test" { - ami = data.aws_ami.amzn-ami-minimal-pv-instance-store.id - - # tflint-ignore: aws_instance_previous_type - instance_type = "m1.small" + ami = data.aws_ami.amzn-ami-minimal-pv-ebs.id + instance_type = data.aws_ec2_instance_type_offering.available.instance_type } resource "aws_eip" "test" {