From 317b54d64580072bc614ccd59a93808451972d71 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Wed, 4 Nov 2020 16:17:40 -0500 Subject: [PATCH 1/2] tests/resource/aws_default_security_group: Remove hardcoded us-east-1 handling Reference: https://github.com/hashicorp/terraform-provider-aws/issues/8316 Reference: https://github.com/hashicorp/terraform-provider-aws/issues/15737 Reference: https://github.com/hashicorp/terraform-provider-aws/issues/15791 Previously in AWS GovCloud (US): ``` === RUN TestAccAWSDefaultSecurityGroup_Classic_basic TestAccAWSDefaultSecurityGroup_Classic_basic: provider_test.go:196: [{0 error configuring Terraform AWS Provider: error validating provider credentials: error calling sts:GetCallerIdentity: InvalidClientTokenId: The security token included in the request is invalid. status code: 403, request id: fc6cf64f-8c40-4e8b-a37c-a82d8c6a69c9 []}] --- FAIL: TestAccAWSDefaultSecurityGroup_Classic_basic (0.40s) ``` Output from acceptance testing in AWS Commercial: ``` --- PASS: TestAccAWSDefaultSecurityGroup_Classic_basic (16.47s) --- SKIP: TestAccAWSDefaultSecurityGroup_Classic_empty (0.00s) ``` Output from acceptance testing in AWS GovCloud (US): ``` --- SKIP: TestAccAWSDefaultSecurityGroup_Classic_basic (2.90s) --- SKIP: TestAccAWSDefaultSecurityGroup_Classic_empty (0.00s) ``` --- aws/ec2_classic_test.go | 16 ++++ ...esource_aws_default_security_group_test.go | 89 +++++++++++++------ 2 files changed, 76 insertions(+), 29 deletions(-) diff --git a/aws/ec2_classic_test.go b/aws/ec2_classic_test.go index ffa241c430f..68b31ebcfdb 100644 --- a/aws/ec2_classic_test.go +++ b/aws/ec2_classic_test.go @@ -6,7 +6,9 @@ import ( "sync" "testing" + "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/aws/endpoints" + "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/terraform" ) @@ -75,3 +77,17 @@ func testAccGetEc2ClassicRegion() string { return testAccGetRegion() } + +// testAccCheckResourceAttrRegionalARNEc2Classic ensures the Terraform state exactly matches a formatted ARN with EC2-Classic region +func testAccCheckResourceAttrRegionalARNEc2Classic(resourceName, attributeName, arnService, arnResource string) resource.TestCheckFunc { + return func(s *terraform.State) error { + attributeValue := arn.ARN{ + AccountID: testAccGetAccountID(), + Partition: testAccGetPartition(), + Region: testAccGetEc2ClassicRegion(), + Resource: arnResource, + Service: arnService, + }.String() + return resource.TestCheckResourceAttr(resourceName, attributeName, attributeValue)(s) + } +} diff --git a/aws/resource_aws_default_security_group_test.go b/aws/resource_aws_default_security_group_test.go index b3c612f9442..58e0b858b9e 100644 --- a/aws/resource_aws_default_security_group_test.go +++ b/aws/resource_aws_default_security_group_test.go @@ -2,7 +2,6 @@ package aws import ( "fmt" - "os" "testing" "github.com/aws/aws-sdk-go/aws" @@ -95,23 +94,18 @@ func TestAccAWSDefaultSecurityGroup_Vpc_empty(t *testing.T) { } func TestAccAWSDefaultSecurityGroup_Classic_basic(t *testing.T) { - oldvar := os.Getenv("AWS_DEFAULT_REGION") - os.Setenv("AWS_DEFAULT_REGION", "us-east-1") - defer os.Setenv("AWS_DEFAULT_REGION", oldvar) - var group ec2.SecurityGroup resourceName := "aws_default_security_group.test" - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) }, - IDRefreshName: resourceName, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSDefaultSecurityGroupDestroy, + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) }, + ProviderFactories: testAccProviderFactories, + CheckDestroy: testAccCheckAWSDefaultSecurityGroupDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSDefaultSecurityGroupConfig_Classic, + Config: testAccAWSDefaultSecurityGroupConfig_Classic(), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSDefaultSecurityGroupExists(resourceName, &group), + testAccCheckAWSDefaultSecurityGroupEc2ClassicExists(resourceName, &group), resource.TestCheckResourceAttr(resourceName, "name", "default"), resource.TestCheckResourceAttr(resourceName, "description", "default group"), resource.TestCheckResourceAttr(resourceName, "vpc_id", ""), @@ -131,10 +125,11 @@ func TestAccAWSDefaultSecurityGroup_Classic_basic(t *testing.T) { ), }, { - Config: testAccAWSDefaultSecurityGroupConfig_Classic, + Config: testAccAWSDefaultSecurityGroupConfig_Classic(), PlanOnly: true, }, { + Config: testAccAWSDefaultSecurityGroupConfig_Classic(), ResourceName: resourceName, ImportState: true, ImportStateVerify: true, @@ -150,23 +145,18 @@ func TestAccAWSDefaultSecurityGroup_Classic_empty(t *testing.T) { // Additional references: // * https://github.com/hashicorp/terraform-provider-aws/issues/14631 - oldvar := os.Getenv("AWS_DEFAULT_REGION") - os.Setenv("AWS_DEFAULT_REGION", "us-east-1") - defer os.Setenv("AWS_DEFAULT_REGION", oldvar) - var group ec2.SecurityGroup resourceName := "aws_default_security_group.test" - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) }, - IDRefreshName: resourceName, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSDefaultSecurityGroupDestroy, + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) }, + ProviderFactories: testAccProviderFactories, + CheckDestroy: testAccCheckAWSDefaultSecurityGroupDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSDefaultSecurityGroupConfig_Classic_empty, + Config: testAccAWSDefaultSecurityGroupConfig_Classic_empty(), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSDefaultSecurityGroupExists(resourceName, &group), + testAccCheckAWSDefaultSecurityGroupEc2ClassicExists(resourceName, &group), resource.TestCheckResourceAttr(resourceName, "ingress.#", "0"), resource.TestCheckResourceAttr(resourceName, "egress.#", "0"), ), @@ -209,9 +199,42 @@ func testAccCheckAWSDefaultSecurityGroupExists(n string, group *ec2.SecurityGrou } } +func testAccCheckAWSDefaultSecurityGroupEc2ClassicExists(n string, group *ec2.SecurityGroup) 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 EC2 Default Security Group ID is set") + } + + conn := testAccProviderEc2Classic.Meta().(*AWSClient).ec2conn + + input := &ec2.DescribeSecurityGroupsInput{ + GroupIds: []*string{aws.String(rs.Primary.ID)}, + } + + resp, err := conn.DescribeSecurityGroups(input) + + if err != nil { + return fmt.Errorf("error describing EC2 Default Security Group (%s): %w", rs.Primary.ID, err) + } + + if len(resp.SecurityGroups) == 0 || aws.StringValue(resp.SecurityGroups[0].GroupId) != rs.Primary.ID { + return fmt.Errorf("EC2 Default Security Group (%s) not found", rs.Primary.ID) + } + + *group = *resp.SecurityGroups[0] + + return nil + } +} + func testAccCheckAWSDefaultSecurityGroupARN(resourceName string, group *ec2.SecurityGroup) resource.TestCheckFunc { return func(s *terraform.State) error { - return testAccCheckResourceAttrRegionalARN(resourceName, "arn", "ec2", fmt.Sprintf("security-group/%s", aws.StringValue(group.GroupId)))(s) + return testAccCheckResourceAttrRegionalARNEc2Classic(resourceName, "arn", "ec2", fmt.Sprintf("security-group/%s", aws.StringValue(group.GroupId)))(s) } } @@ -261,7 +284,10 @@ resource "aws_default_security_group" "test" { } ` -const testAccAWSDefaultSecurityGroupConfig_Classic = ` +func testAccAWSDefaultSecurityGroupConfig_Classic() string { + return composeConfig( + testAccEc2ClassicRegionProviderConfig(), + ` resource "aws_default_security_group" "test" { ingress { protocol = "6" @@ -274,13 +300,18 @@ resource "aws_default_security_group" "test" { Name = "tf-acc-test" } } -` +`) +} -const testAccAWSDefaultSecurityGroupConfig_Classic_empty = ` +func testAccAWSDefaultSecurityGroupConfig_Classic_empty() string { + return composeConfig( + testAccEc2ClassicRegionProviderConfig(), + ` resource "aws_default_security_group" "test" { # No attributes set. } -` +`) +} func TestAWSDefaultSecurityGroupMigrateState(t *testing.T) { cases := map[string]struct { From d7d6ca6a387ae571311108c249090bf82fb22b8f Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 10 Nov 2020 16:52:16 -0500 Subject: [PATCH 2/2] tests/resource/aws_default_security_group: Ensure EC2-Classic ARN checking is separate from regular ARN checking --- aws/resource_aws_default_security_group_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/aws/resource_aws_default_security_group_test.go b/aws/resource_aws_default_security_group_test.go index 58e0b858b9e..1e1e7857ebe 100644 --- a/aws/resource_aws_default_security_group_test.go +++ b/aws/resource_aws_default_security_group_test.go @@ -118,7 +118,7 @@ func TestAccAWSDefaultSecurityGroup_Classic_basic(t *testing.T) { "cidr_blocks.0": "10.0.0.0/8", }), resource.TestCheckResourceAttr(resourceName, "egress.#", "0"), - testAccCheckAWSDefaultSecurityGroupARN(resourceName, &group), + testAccCheckAWSDefaultSecurityGroupARNEc2Classic(resourceName, &group), testAccCheckResourceAttrAccountID(resourceName, "owner_id"), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), resource.TestCheckResourceAttr(resourceName, "tags.Name", "tf-acc-test"), @@ -233,6 +233,12 @@ func testAccCheckAWSDefaultSecurityGroupEc2ClassicExists(n string, group *ec2.Se } func testAccCheckAWSDefaultSecurityGroupARN(resourceName string, group *ec2.SecurityGroup) resource.TestCheckFunc { + return func(s *terraform.State) error { + return testAccCheckResourceAttrRegionalARN(resourceName, "arn", "ec2", fmt.Sprintf("security-group/%s", aws.StringValue(group.GroupId)))(s) + } +} + +func testAccCheckAWSDefaultSecurityGroupARNEc2Classic(resourceName string, group *ec2.SecurityGroup) resource.TestCheckFunc { return func(s *terraform.State) error { return testAccCheckResourceAttrRegionalARNEc2Classic(resourceName, "arn", "ec2", fmt.Sprintf("security-group/%s", aws.StringValue(group.GroupId)))(s) }