diff --git a/aws/provider_test.go b/aws/provider_test.go index 23fd6daf776..aa4b0da283d 100644 --- a/aws/provider_test.go +++ b/aws/provider_test.go @@ -382,6 +382,14 @@ func testAccGetAlternateRegion() string { return v } +func testAccGetThirdRegion() string { + v := os.Getenv("AWS_THIRD_REGION") + if v == "" { + return "us-east-2" + } + return v +} + func testAccGetPartition() string { if partition, ok := endpoints.PartitionForRegion(endpoints.DefaultPartitions(), testAccGetRegion()); ok { return partition.ID() @@ -403,6 +411,13 @@ func testAccGetAlternateRegionPartition() string { return "aws" } +func testAccGetThirdRegionPartition() string { + if partition, ok := endpoints.PartitionForRegion(endpoints.DefaultPartitions(), testAccGetThirdRegion()); ok { + return partition.ID() + } + return "aws" +} + func testAccAlternateAccountPreCheck(t *testing.T) { if os.Getenv("AWS_ALTERNATE_PROFILE") == "" && os.Getenv("AWS_ALTERNATE_ACCESS_KEY_ID") == "" { t.Fatal("AWS_ALTERNATE_ACCESS_KEY_ID or AWS_ALTERNATE_PROFILE must be set for acceptance tests") @@ -423,6 +438,18 @@ func testAccAlternateRegionPreCheck(t *testing.T) { } } +func testAccThirdRegionPreCheck(t *testing.T) { + testAccAlternateRegionPreCheck(t) + + if testAccGetRegion() == testAccGetThirdRegion() { + t.Fatal("AWS_DEFAULT_REGION and AWS_THIRD_REGION must be set to different values for acceptance tests") + } + + if testAccGetPartition() != testAccGetThirdRegionPartition() { + t.Fatalf("AWS_THIRD_REGION partition (%s) does not match AWS_DEFAULT_REGION partition (%s)", testAccGetThirdRegionPartition(), testAccGetPartition()) + } +} + func testAccEC2ClassicPreCheck(t *testing.T) { client := testAccProvider.Meta().(*AWSClient) platforms := client.supportedplatforms @@ -451,6 +478,24 @@ func testAccPartitionHasServicePreCheck(serviceId string, t *testing.T) { } } +func testAccMultipleRegionPreCheck(t *testing.T, expected int) { + switch expected { + case 2: + testAccAlternateRegionPreCheck(t) + case 3: + testAccThirdRegionPreCheck(t) + default: + t.Fatalf("unknown expected region: %d", expected) + } + + if partition, ok := endpoints.PartitionForRegion(endpoints.DefaultPartitions(), testAccGetRegion()); ok { + if len(partition.Regions()) < expected { + t.Skipf("skipping tests; partition includes %d regions, %d expected", len(partition.Regions()), expected) + } + } +} + +// Deprecated: Use testAccMultipleRegionPreCheck instead. func testAccMultipleRegionsPreCheck(t *testing.T) { if partition, ok := endpoints.PartitionForRegion(endpoints.DefaultPartitions(), testAccGetRegion()); ok { if len(partition.Regions()) < 2 { @@ -536,6 +581,7 @@ provider "aws" { `, os.Getenv("AWS_ALTERNATE_ACCESS_KEY_ID"), os.Getenv("AWS_ALTERNATE_PROFILE"), testAccGetAlternateRegion(), os.Getenv("AWS_ALTERNATE_SECRET_ACCESS_KEY")) } +// Deprecated: Use testAccMultipleRegionProviderConfig instead func testAccAlternateRegionProviderConfig() string { //lintignore:AT004 return fmt.Sprintf(` @@ -546,6 +592,30 @@ provider "aws" { `, testAccGetAlternateRegion()) } +func testAccMultipleRegionProviderConfig(regions int) string { + var config strings.Builder + + //lintignore:AT004 + fmt.Fprintf(&config, ` +provider "aws" { + alias = "alternate" + region = %[1]q +} +`, testAccGetAlternateRegion()) + + if regions >= 3 { + //lintignore:AT004 + fmt.Fprintf(&config, ` +provider "aws" { + alias = "third" + region = %[1]q +} +`, testAccGetThirdRegion()) + } + + return config.String() +} + func testAccProviderConfigIgnoreTagsKeyPrefixes1(keyPrefix1 string) string { //lintignore:AT004 return fmt.Sprintf(` diff --git a/aws/resource_aws_dynamodb_table.go b/aws/resource_aws_dynamodb_table.go index 8e52f9621ef..c2a65e2bb82 100644 --- a/aws/resource_aws_dynamodb_table.go +++ b/aws/resource_aws_dynamodb_table.go @@ -760,9 +760,6 @@ func deleteAwsDynamoDbTable(tableName string, conn *dynamodb.DynamoDB) error { } func deleteDynamoDbReplicas(tableName string, tfList []interface{}, timeout time.Duration, conn *dynamodb.DynamoDB) error { - var ops []*dynamodb.ReplicationGroupUpdate - var regionNames []string - for _, tfMapRaw := range tfList { tfMap, ok := tfMapRaw.(map[string]interface{}) @@ -780,47 +777,43 @@ func deleteDynamoDbReplicas(tableName string, tfList []interface{}, timeout time continue } - ops = append(ops, &dynamodb.ReplicationGroupUpdate{ - Delete: &dynamodb.DeleteReplicationGroupMemberAction{ - RegionName: aws.String(regionName), + input := &dynamodb.UpdateTableInput{ + TableName: aws.String(tableName), + ReplicaUpdates: []*dynamodb.ReplicationGroupUpdate{ + { + Delete: &dynamodb.DeleteReplicationGroupMemberAction{ + RegionName: aws.String(regionName), + }, + }, }, - }) - regionNames = append(regionNames, regionName) - } - - if len(ops) == 0 { - return nil - } + } - input := &dynamodb.UpdateTableInput{ - TableName: aws.String(tableName), - ReplicaUpdates: ops, - } + err := resource.Retry(20*time.Minute, func() *resource.RetryError { + _, err := conn.UpdateTable(input) + if err != nil { + if isAWSErr(err, "ThrottlingException", "") { + return resource.RetryableError(err) + } + if isAWSErr(err, dynamodb.ErrCodeLimitExceededException, "can be created, updated, or deleted simultaneously") { + return resource.RetryableError(err) + } + if isAWSErr(err, dynamodb.ErrCodeResourceInUseException, "") { + return resource.RetryableError(err) + } - err := resource.Retry(20*time.Minute, func() *resource.RetryError { - _, err := conn.UpdateTable(input) - if err != nil { - if isAWSErr(err, "ThrottlingException", "") { - return resource.RetryableError(err) - } - if isAWSErr(err, dynamodb.ErrCodeLimitExceededException, "can be created, updated, or deleted simultaneously") { - return resource.RetryableError(err) + return resource.NonRetryableError(err) } + return nil + }) - return resource.NonRetryableError(err) + if isResourceTimeoutError(err) { + _, err = conn.UpdateTable(input) } - return nil - }) - - if isResourceTimeoutError(err) { - _, err = conn.UpdateTable(input) - } - if err != nil { - return fmt.Errorf("error deleting DynamoDB Table (%s) replicas: %s", tableName, err) - } + if err != nil { + return fmt.Errorf("error deleting DynamoDB Table (%s) replica (%s): %s", tableName, regionName, err) + } - for _, regionName := range regionNames { if err := waitForDynamoDbReplicaDeleteToBeCompleted(tableName, regionName, timeout, conn); err != nil { return fmt.Errorf("error waiting for DynamoDB Table (%s) replica (%s) deletion: %s", tableName, regionName, err) } @@ -947,9 +940,6 @@ func waitForDynamoDbReplicaDeleteToBeCompleted(tableName string, region string, } func createDynamoDbReplicas(tableName string, tfList []interface{}, timeout time.Duration, conn *dynamodb.DynamoDB) error { - var ops []*dynamodb.ReplicationGroupUpdate - var regionNames []string - for _, tfMapRaw := range tfList { tfMap, ok := tfMapRaw.(map[string]interface{}) @@ -967,47 +957,43 @@ func createDynamoDbReplicas(tableName string, tfList []interface{}, timeout time continue } - ops = append(ops, &dynamodb.ReplicationGroupUpdate{ - Create: &dynamodb.CreateReplicationGroupMemberAction{ - RegionName: aws.String(regionName), + input := &dynamodb.UpdateTableInput{ + TableName: aws.String(tableName), + ReplicaUpdates: []*dynamodb.ReplicationGroupUpdate{ + { + Create: &dynamodb.CreateReplicationGroupMemberAction{ + RegionName: aws.String(regionName), + }, + }, }, - }) - regionNames = append(regionNames, regionName) - } - - if len(ops) == 0 { - return nil - } + } - input := &dynamodb.UpdateTableInput{ - TableName: aws.String(tableName), - ReplicaUpdates: ops, - } + err := resource.Retry(20*time.Minute, func() *resource.RetryError { + _, err := conn.UpdateTable(input) + if err != nil { + if isAWSErr(err, "ThrottlingException", "") { + return resource.RetryableError(err) + } + if isAWSErr(err, dynamodb.ErrCodeLimitExceededException, "can be created, updated, or deleted simultaneously") { + return resource.RetryableError(err) + } + if isAWSErr(err, dynamodb.ErrCodeResourceInUseException, "") { + return resource.RetryableError(err) + } - err := resource.Retry(20*time.Minute, func() *resource.RetryError { - _, err := conn.UpdateTable(input) - if err != nil { - if isAWSErr(err, "ThrottlingException", "") { - return resource.RetryableError(err) - } - if isAWSErr(err, dynamodb.ErrCodeLimitExceededException, "can be created, updated, or deleted simultaneously") { - return resource.RetryableError(err) + return resource.NonRetryableError(err) } + return nil + }) - return resource.NonRetryableError(err) + if isResourceTimeoutError(err) { + _, err = conn.UpdateTable(input) } - return nil - }) - - if isResourceTimeoutError(err) { - _, err = conn.UpdateTable(input) - } - if err != nil { - return fmt.Errorf("error creating DynamoDB Table (%s) replicas: %s", tableName, err) - } + if err != nil { + return fmt.Errorf("error creating DynamoDB Table (%s) replica (%s): %s", tableName, regionName, err) + } - for _, regionName := range regionNames { if err := waitForDynamoDbReplicaUpdateToBeCompleted(tableName, regionName, timeout, conn); err != nil { return fmt.Errorf("error waiting for DynamoDB Table (%s) replica (%s) creation: %s", tableName, regionName, err) } diff --git a/aws/resource_aws_dynamodb_table_test.go b/aws/resource_aws_dynamodb_table_test.go index 77e0694e276..dd3060b22e5 100644 --- a/aws/resource_aws_dynamodb_table_test.go +++ b/aws/resource_aws_dynamodb_table_test.go @@ -1387,7 +1387,52 @@ func testAccCheckAWSDynamoDbTableDisappears(table *dynamodb.DescribeTableOutput) } } -func TestAccAWSDynamoDbTable_Replica(t *testing.T) { +func TestAccAWSDynamoDbTable_Replica_Multiple(t *testing.T) { + var table dynamodb.DescribeTableOutput + var providers []*schema.Provider + resourceName := "aws_dynamodb_table.test" + tableName := acctest.RandomWithPrefix("TerraformTestTable-") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccMultipleRegionPreCheck(t, 3) + }, + ProviderFactories: testAccProviderFactories(&providers), + CheckDestroy: testAccCheckAWSDynamoDbTableDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSDynamoDbTableConfigReplica2(tableName), + Check: resource.ComposeTestCheckFunc( + testAccCheckInitialAWSDynamoDbTableExists(resourceName, &table), + resource.TestCheckResourceAttr(resourceName, "replica.#", "2"), + ), + }, + { + Config: testAccAWSDynamoDbTableConfigReplica2(tableName), + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAWSDynamoDbTableConfigReplica0(tableName), + Check: resource.ComposeTestCheckFunc( + testAccCheckInitialAWSDynamoDbTableExists(resourceName, &table), + resource.TestCheckResourceAttr(resourceName, "replica.#", "0"), + ), + }, + { + Config: testAccAWSDynamoDbTableConfigReplica2(tableName), + Check: resource.ComposeTestCheckFunc( + testAccCheckInitialAWSDynamoDbTableExists(resourceName, &table), + resource.TestCheckResourceAttr(resourceName, "replica.#", "2"), + ), + }, + }, + }) +} + +func TestAccAWSDynamoDbTable_Replica_Single(t *testing.T) { var conf dynamodb.DescribeTableOutput var providers []*schema.Provider resourceName := "aws_dynamodb_table.test" @@ -1396,50 +1441,35 @@ func TestAccAWSDynamoDbTable_Replica(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) - testAccMultipleRegionsPreCheck(t) - testAccAlternateRegionPreCheck(t) + testAccMultipleRegionPreCheck(t, 2) }, - ProviderFactories: testAccProviderFactories(&providers), - CheckDestroy: testAccCheckAWSDynamoDbTableDestroy, - DisableBinaryDriver: true, + ProviderFactories: testAccProviderFactories(&providers), + CheckDestroy: testAccCheckAWSDynamoDbTableDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSDynamoDbReplicaUpdates(tableName), + Config: testAccAWSDynamoDbTableConfigReplica1(tableName), Check: resource.ComposeTestCheckFunc( testAccCheckInitialAWSDynamoDbTableExists(resourceName, &conf), - resource.TestCheckResourceAttr(resourceName, "name", tableName), - resource.TestCheckResourceAttr(resourceName, "hash_key", "TestTableHashKey"), - resource.TestCheckResourceAttr(resourceName, "attribute.2990477658.name", "TestTableHashKey"), - resource.TestCheckResourceAttr(resourceName, "attribute.2990477658.type", "S"), resource.TestCheckResourceAttr(resourceName, "replica.#", "1"), ), }, { - Config: testAccAWSDynamoDbReplicaUpdates(tableName), + Config: testAccAWSDynamoDbTableConfigReplica1(tableName), ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, { - Config: testAccAWSDynamoDbReplicaDeletes(tableName), + Config: testAccAWSDynamoDbTableConfigReplica0(tableName), Check: resource.ComposeTestCheckFunc( testAccCheckInitialAWSDynamoDbTableExists(resourceName, &conf), - resource.TestCheckResourceAttr(resourceName, "name", tableName), - resource.TestCheckResourceAttr(resourceName, "hash_key", "TestTableHashKey"), - resource.TestCheckResourceAttr(resourceName, "attribute.2990477658.name", "TestTableHashKey"), - resource.TestCheckResourceAttr(resourceName, "attribute.2990477658.type", "S"), - resource.TestCheckResourceAttr(resourceName, "hash_key", "TestTableHashKey"), resource.TestCheckResourceAttr(resourceName, "replica.#", "0"), ), }, { - Config: testAccAWSDynamoDbReplicaUpdates(tableName), + Config: testAccAWSDynamoDbTableConfigReplica1(tableName), Check: resource.ComposeTestCheckFunc( testAccCheckInitialAWSDynamoDbTableExists(resourceName, &conf), - resource.TestCheckResourceAttr(resourceName, "name", tableName), - resource.TestCheckResourceAttr(resourceName, "hash_key", "TestTableHashKey"), - resource.TestCheckResourceAttr(resourceName, "attribute.2990477658.name", "TestTableHashKey"), - resource.TestCheckResourceAttr(resourceName, "attribute.2990477658.type", "S"), resource.TestCheckResourceAttr(resourceName, "replica.#", "1"), ), }, @@ -1447,48 +1477,6 @@ func TestAccAWSDynamoDbTable_Replica(t *testing.T) { }) } -func testAccAWSDynamoDbReplicaUpdates(rName string) string { - return testAccAlternateRegionProviderConfig() + fmt.Sprintf(` -data "aws_region" "alternate" { - provider = "aws.alternate" -} - -resource "aws_dynamodb_table" "test" { - name = %[1]q - hash_key = "TestTableHashKey" - billing_mode = "PAY_PER_REQUEST" - stream_enabled = true - stream_view_type = "NEW_AND_OLD_IMAGES" - - attribute { - name = "TestTableHashKey" - type = "S" - } - - replica { - region_name = %[2]q - } -} -`, rName, testAccGetAlternateRegion()) -} - -func testAccAWSDynamoDbReplicaDeletes(rName string) string { - return testAccAlternateRegionProviderConfig() + fmt.Sprintf(` -resource "aws_dynamodb_table" "test" { - name = "%s" - hash_key = "TestTableHashKey" - billing_mode = "PAY_PER_REQUEST" - stream_enabled = true - stream_view_type = "NEW_AND_OLD_IMAGES" - - attribute { - name = "TestTableHashKey" - type = "S" - } -} -`, rName) -} - func dynamoDbGetGSIIndex(gsiList *[]*dynamodb.GlobalSecondaryIndexDescription, target string) int { for idx, gsiObject := range *gsiList { if *gsiObject.IndexName == target { @@ -2190,3 +2178,84 @@ resource "aws_dynamodb_table" "test" { } `, rName, attrName1, attrType1, attrName2, attrType2, hashKey, rangeKey) } + +func testAccAWSDynamoDbTableConfigReplica0(rName string) string { + return composeConfig( + testAccMultipleRegionProviderConfig(3), // Prevent "Provider configuration not present" errors + fmt.Sprintf(` +resource "aws_dynamodb_table" "test" { + name = %[1]q + hash_key = "TestTableHashKey" + billing_mode = "PAY_PER_REQUEST" + stream_enabled = true + stream_view_type = "NEW_AND_OLD_IMAGES" + + attribute { + name = "TestTableHashKey" + type = "S" + } +} +`, rName)) +} + +func testAccAWSDynamoDbTableConfigReplica1(rName string) string { + return composeConfig( + testAccMultipleRegionProviderConfig(3), // Prevent "Provider configuration not present" errors + fmt.Sprintf(` +data "aws_region" "alternate" { + provider = "aws.alternate" +} + +resource "aws_dynamodb_table" "test" { + name = %[1]q + hash_key = "TestTableHashKey" + billing_mode = "PAY_PER_REQUEST" + stream_enabled = true + stream_view_type = "NEW_AND_OLD_IMAGES" + + attribute { + name = "TestTableHashKey" + type = "S" + } + + replica { + region_name = data.aws_region.alternate.name + } +} +`, rName)) +} + +func testAccAWSDynamoDbTableConfigReplica2(rName string) string { + return composeConfig( + testAccMultipleRegionProviderConfig(3), + fmt.Sprintf(` +data "aws_region" "alternate" { + provider = "aws.alternate" +} + +data "aws_region" "third" { + provider = "aws.third" +} + +resource "aws_dynamodb_table" "test" { + name = %[1]q + hash_key = "TestTableHashKey" + billing_mode = "PAY_PER_REQUEST" + stream_enabled = true + stream_view_type = "NEW_AND_OLD_IMAGES" + + attribute { + name = "TestTableHashKey" + type = "S" + } + + replica { + region_name = data.aws_region.alternate.name + } + + replica { + region_name = data.aws_region.third.name + } +} +`, rName)) +} diff --git a/docs/contributing/running-and-writing-acceptance-tests.md b/docs/contributing/running-and-writing-acceptance-tests.md index 25cde2dbaed..eb727ee9cb6 100644 --- a/docs/contributing/running-and-writing-acceptance-tests.md +++ b/docs/contributing/running-and-writing-acceptance-tests.md @@ -126,12 +126,13 @@ export AWS_ALTERNATE_SECRET_ACCESS_KEY=... ### Running Cross-Region Tests -Certain testing requires multiple AWS regions. Additional setup is not typically required because the testing defaults the alternate AWS region to `us-east-1`. +Certain testing requires multiple AWS regions. Additional setup is not typically required because the testing defaults the second AWS region to `us-east-1` and the third AWS region to `us-east-2`. -Running these acceptance tests is the same as before, but if you wish to override the alternate region: +Running these acceptance tests is the same as before, but if you wish to override the second and third regions: ```sh export AWS_ALTERNATE_REGION=... +export AWS_THIRD_REGION=... ``` ## Writing an Acceptance Test @@ -659,12 +660,12 @@ Searching for usage of `testAccAlternateAccountPreCheck` in the codebase will yi #### Cross-Region Acceptance Tests -When testing requires AWS infrastructure in a second AWS region, the below changes to the normal setup will allow the management or reference of resources and data sources across regions: +When testing requires AWS infrastructure in a second or third AWS region, the below changes to the normal setup will allow the management or reference of resources and data sources across regions: -- In the `PreCheck` function, include `testAccMultipleRegionsPreCheck(t)` and `testAccAlternateRegionPreCheck(t)` to ensure a standardized set of information is required for cross-region testing configuration. If the infrastructure in the second AWS region is also in a second AWS account also include `testAccAlternateAccountPreCheck(t)` +- In the `PreCheck` function, include `testAccMultipleRegionPreCheck(t, ###)` to ensure a standardized set of information is required for cross-region testing configuration. If the infrastructure in the second AWS region is also in a second AWS account also include `testAccAlternateAccountPreCheck(t)` - Declare a `providers` variable at the top of the test function: `var providers []*schema.Provider` - Switch usage of `Providers: testAccProviders` to `ProviderFactories: testAccProviderFactories(&providers)` -- Add `testAccAlternateRegionProviderConfig()` to the test configuration and use `provider = "aws.alternate"` for cross-region resources. The resource that is the focus of the acceptance test should _not_ use the provider alias to simplify the testing setup. If the infrastructure in the second AWS region is also in a second AWS account use `testAccAlternateAccountAlternateRegionProviderConfig()` instead +- Add `testAccMultipleRegionProviderConfig(###)` to the test configuration and use `provider = "aws.alternate"` (and/or `provider = "aws.third"`) for cross-region resources. The resource that is the focus of the acceptance test should _not_ use the provider alias to simplify the testing setup. If the infrastructure in the second AWS region is also in a second AWS account use `testAccAlternateAccountAlternateRegionProviderConfig()` instead - For any `TestStep` that includes `ImportState: true`, add the `Config` that matches the previous `TestStep` `Config` An example acceptance test implementation can be seen below: @@ -677,8 +678,7 @@ func TestAccAwsExample_basic(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) - testAccMultipleRegionsPreCheck(t) - testAccAlternateRegionPreCheck(t) + testAccMultipleRegionPreCheck(t, 2) }, ProviderFactories: testAccProviderFactories(&providers), CheckDestroy: testAccCheckAwsExampleDestroy, @@ -701,7 +701,7 @@ func TestAccAwsExample_basic(t *testing.T) { } func testAccAwsExampleConfig() string { - return testAccAlternateRegionProviderConfig() + fmt.Sprintf(` + return testAccMultipleRegionProviderConfig(2) + fmt.Sprintf(` # Cross region resources should be handled by the cross region provider. # The standardized provider alias is aws.alternate as seen below. resource "aws_cross_region_example" "test" { @@ -719,7 +719,7 @@ resource "aws_example" "test" { } ``` -Searching for usage of `testAccAlternateRegionPreCheck` in the codebase will yield real world examples of this setup in action. +Searching for usage of `testAccMultipleRegionPreCheck` in the codebase will yield real world examples of this setup in action. ### Data Source Acceptance Testing