From 35bc760211acbb8b4b3188ca6c5584a0c70558c3 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 11 Aug 2022 17:12:23 -0400 Subject: [PATCH 1/4] dynamodb_table: Fix replica propagate_tags bug --- internal/service/dynamodb/table.go | 82 +++- internal/service/dynamodb/table_test.go | 461 +++++++++++++++++++- website/docs/r/dynamodb_table.html.markdown | 2 +- 3 files changed, 508 insertions(+), 37 deletions(-) diff --git a/internal/service/dynamodb/table.go b/internal/service/dynamodb/table.go index fda5ea30ba7..59606660c5f 100644 --- a/internal/service/dynamodb/table.go +++ b/internal/service/dynamodb/table.go @@ -595,7 +595,7 @@ func resourceTableCreate(d *schema.ResourceData, meta interface{}) error { return create.Error(names.DynamoDB, create.ErrActionCreating, ResNameTable, d.Id(), fmt.Errorf("replicas: %w", err)) } - if err := updateReplicaTags(conn, aws.StringValue(output.TableArn), v.List(), nil, tags, meta.(*conns.AWSClient).TerraformVersion); err != nil { + if err := updateReplicaTags(conn, aws.StringValue(output.TableArn), v.List(), tags, meta.(*conns.AWSClient).TerraformVersion); err != nil { return create.Error(names.DynamoDB, create.ErrActionCreating, ResNameTable, d.Id(), fmt.Errorf("replica tags: %w", err)) } } @@ -912,20 +912,27 @@ func resourceTableUpdate(d *schema.ResourceData, meta interface{}) error { } } + replicaTagsChange := false if d.HasChange("replica") { + replicaTagsChange = true + if err := updateReplica(d, conn, meta.(*conns.AWSClient).TerraformVersion); err != nil { return create.Error(names.DynamoDB, create.ErrActionUpdating, ResNameTable, d.Id(), err) } } if d.HasChange("tags_all") { + replicaTagsChange = true + o, n := d.GetChange("tags_all") if err := UpdateTags(conn, d.Get("arn").(string), o, n); err != nil { return create.Error(names.DynamoDB, create.ErrActionUpdating, ResNameTable, d.Id(), err) } + } + if replicaTagsChange { if v, ok := d.Get("replica").(*schema.Set); ok && v.Len() > 0 { - if err := updateReplicaTags(conn, d.Get("arn").(string), v.List(), o, n, meta.(*conns.AWSClient).TerraformVersion); err != nil { + if err := updateReplicaTags(conn, d.Get("arn").(string), v.List(), d.Get("tags_all"), meta.(*conns.AWSClient).TerraformVersion); err != nil { return create.Error(names.DynamoDB, create.ErrActionUpdating, ResNameTable, d.Id(), err) } } @@ -967,6 +974,54 @@ func resourceTableDelete(d *schema.ResourceData, meta interface{}) error { } // custom diff +/* +func replicaTagsDiff(_ context.Context, diff *schema.ResourceDiff, meta interface{}) error { + + defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig + ignoreTagsConfig := meta.(*conns.AWSClient).IgnoreTagsConfig + + if v, ok := tfMap["propagate_tags"].(bool); ok && v { + if aws.StringValue(conn.Config.Region) != region { + session, err := conns.NewSessionForRegion(&conn.Config, region, terraformVersion) + if err != nil { + return fmt.Errorf("updating replica (%s) tags: %w", region, err) + } + + conn = dynamodb.New(session) + } + } + + resourceTags := tftags.New(diff.Get("tags").(map[string]interface{})) + + if defaultTagsConfig.TagsEqual(resourceTags) { + return fmt.Errorf(`"tags" are identical to those in the "default_tags" configuration block of the provider: please de-duplicate and try again`) + } + + allTags := defaultTagsConfig.MergeTags(resourceTags).IgnoreConfig(ignoreTagsConfig) + + // To ensure "tags_all" is correctly computed, we explicitly set the attribute diff + // when the merger of resource-level tags onto provider-level tags results in n > 0 tags, + // otherwise we mark the attribute as "Computed" only when their is a known diff (excluding an empty map) + // or a change for "tags_all". + // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/18366 + // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/19005 + if len(allTags) > 0 { + if err := diff.SetNew("tags_all", allTags.Map()); err != nil { + return fmt.Errorf("error setting new tags_all diff: %w", err) + } + } else if len(diff.Get("tags_all").(map[string]interface{})) > 0 { + if err := diff.SetNewComputed("tags_all"); err != nil { + return fmt.Errorf("error setting tags_all to computed: %w", err) + } + } else if diff.HasChange("tags_all") { + if err := diff.SetNewComputed("tags_all"); err != nil { + return fmt.Errorf("error setting tags_all to computed: %w", err) + } + } + + return nil +} +*/ func isTableOptionDisabled(v interface{}) bool { options := v.([]interface{}) @@ -1070,7 +1125,7 @@ func createReplicas(conn *dynamodb.DynamoDB, tableName string, tfList []interfac return nil } -func updateReplicaTags(conn *dynamodb.DynamoDB, rn string, replicas []interface{}, oldTags interface{}, newTags interface{}, terraformVersion string) error { +func updateReplicaTags(conn *dynamodb.DynamoDB, rn string, replicas []interface{}, newTags interface{}, terraformVersion string) error { for _, tfMapRaw := range replicas { tfMap, ok := tfMapRaw.(map[string]interface{}) @@ -1085,21 +1140,24 @@ func updateReplicaTags(conn *dynamodb.DynamoDB, rn string, replicas []interface{ } if v, ok := tfMap["propagate_tags"].(bool); ok && v { - if aws.StringValue(conn.Config.Region) != region { - session, err := conns.NewSessionForRegion(&conn.Config, region, terraformVersion) - if err != nil { - return fmt.Errorf("updating replica (%s) tags: %w", region, err) - } - - conn = dynamodb.New(session) + session, err := conns.NewSessionForRegion(&conn.Config, region, terraformVersion) + if err != nil { + return fmt.Errorf("updating replica (%s) tags: %w", region, err) } - newARN, err := ARNForNewRegion(rn, region) + conn = dynamodb.New(session) + + repARN, err := ARNForNewRegion(rn, region) if err != nil { return fmt.Errorf("per region ARN for replica (%s): %w", region, err) } - if err := UpdateTags(conn, newARN, oldTags, newTags); err != nil { + oldTags, err := ListTags(conn, repARN) + if err != nil { + return fmt.Errorf("listing tags (%s): %w", repARN, err) + } + + if err := UpdateTags(conn, repARN, oldTags, newTags); err != nil { return fmt.Errorf("updating tags: %w", err) } } diff --git a/internal/service/dynamodb/table_test.go b/internal/service/dynamodb/table_test.go index 68d7497aabc..cf81eb0db80 100644 --- a/internal/service/dynamodb/table_test.go +++ b/internal/service/dynamodb/table_test.go @@ -1,7 +1,6 @@ package dynamodb_test import ( - "errors" "fmt" "log" "regexp" @@ -1622,8 +1621,8 @@ func TestAccDynamoDBTable_Replica_tagsOneOfTwo(t *testing.T) { Config: testAccTableConfig_replicaTags(rName, "benny", "smiles", true, false), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInitialTableExists(resourceName, &conf), - testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), true), - testAccCheckReplicaHasTags(resourceName, acctest.ThirdRegion(), false), + testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), 3), + testAccCheckReplicaHasTags(resourceName, acctest.ThirdRegion(), 0), resource.TestCheckResourceAttr(resourceName, "replica.#", "2"), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ "region_name": acctest.AlternateRegion(), @@ -1661,8 +1660,196 @@ func TestAccDynamoDBTable_Replica_tagsTwoOfTwo(t *testing.T) { Config: testAccTableConfig_replicaTags(rName, "Structure", "Adobe", true, true), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInitialTableExists(resourceName, &conf), - testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), true), - testAccCheckReplicaHasTags(resourceName, acctest.ThirdRegion(), true), + testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), 3), + testAccCheckReplicaHasTags(resourceName, acctest.ThirdRegion(), 3), + resource.TestCheckResourceAttr(resourceName, "replica.#", "2"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.AlternateRegion(), + "propagate_tags": "true", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.ThirdRegion(), + "propagate_tags": "true", + }), + ), + }, + }, + }) +} + +func TestAccDynamoDBTable_Replica_tagsNext(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var conf dynamodb.DescribeTableOutput + resourceName := "aws_dynamodb_table.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckMultipleRegion(t, 2) }, + ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(t, 3), + CheckDestroy: testAccCheckTableDestroy, + Steps: []resource.TestStep{ + { + Config: testAccTableConfig_replicaTagsNext1(rName, acctest.AlternateRegion(), true), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInitialTableExists(resourceName, &conf), + testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), 2), + resource.TestCheckResourceAttr(resourceName, "replica.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.AlternateRegion(), + "propagate_tags": "true", + }), + ), + }, + { + Config: testAccTableConfig_replicaTagsNext2(rName, acctest.AlternateRegion(), true, acctest.ThirdRegion(), true), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInitialTableExists(resourceName, &conf), + testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), 2), + testAccCheckReplicaHasTags(resourceName, acctest.ThirdRegion(), 2), + resource.TestCheckResourceAttr(resourceName, "replica.#", "2"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.AlternateRegion(), + "propagate_tags": "true", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.ThirdRegion(), + "propagate_tags": "true", + }), + ), + }, + { + Config: testAccTableConfig_replicaTagsNext1(rName, acctest.AlternateRegion(), true), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInitialTableExists(resourceName, &conf), + testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), 2), + resource.TestCheckResourceAttr(resourceName, "replica.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.AlternateRegion(), + "propagate_tags": "true", + }), + ), + }, + { + Config: testAccTableConfig_replicaTagsNext2(rName, acctest.AlternateRegion(), true, acctest.ThirdRegion(), false), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInitialTableExists(resourceName, &conf), + testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), 2), + testAccCheckReplicaHasTags(resourceName, acctest.ThirdRegion(), 0), + resource.TestCheckResourceAttr(resourceName, "replica.#", "2"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.AlternateRegion(), + "propagate_tags": "true", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.ThirdRegion(), + "propagate_tags": "false", + }), + ), + }, + { + Config: testAccTableConfig_replicaTagsNext2(rName, acctest.AlternateRegion(), false, acctest.ThirdRegion(), false), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInitialTableExists(resourceName, &conf), + testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), 2), + testAccCheckReplicaHasTags(resourceName, acctest.ThirdRegion(), 0), + resource.TestCheckResourceAttr(resourceName, "replica.#", "2"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.AlternateRegion(), + "propagate_tags": "false", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.ThirdRegion(), + "propagate_tags": "false", + }), + ), + }, + }, + }) +} + +func TestAccDynamoDBTable_Replica_tagsUpdate(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var conf dynamodb.DescribeTableOutput + resourceName := "aws_dynamodb_table.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckMultipleRegion(t, 2) }, + ErrorCheck: acctest.ErrorCheck(t, dynamodb.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(t, 3), + CheckDestroy: testAccCheckTableDestroy, + Steps: []resource.TestStep{ + { + Config: testAccTableConfig_replicaTagsUpdate1(rName, acctest.AlternateRegion()), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInitialTableExists(resourceName, &conf), + testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), 2), + resource.TestCheckResourceAttr(resourceName, "replica.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.AlternateRegion(), + "propagate_tags": "true", + }), + ), + }, + { + Config: testAccTableConfig_replicaTagsUpdate2(rName, acctest.AlternateRegion()), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInitialTableExists(resourceName, &conf), + testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), 4), + resource.TestCheckResourceAttr(resourceName, "replica.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.AlternateRegion(), + "propagate_tags": "true", + }), + ), + }, + { + Config: testAccTableConfig_replicaTagsUpdate3(rName, acctest.AlternateRegion(), acctest.ThirdRegion()), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInitialTableExists(resourceName, &conf), + testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), 4), + testAccCheckReplicaHasTags(resourceName, acctest.ThirdRegion(), 4), + resource.TestCheckResourceAttr(resourceName, "replica.#", "2"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.AlternateRegion(), + "propagate_tags": "true", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.ThirdRegion(), + "propagate_tags": "true", + }), + ), + }, + { + Config: testAccTableConfig_replicaTagsUpdate4(rName, acctest.AlternateRegion(), acctest.ThirdRegion()), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInitialTableExists(resourceName, &conf), + testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), 6), + testAccCheckReplicaHasTags(resourceName, acctest.ThirdRegion(), 6), + resource.TestCheckResourceAttr(resourceName, "replica.#", "2"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.AlternateRegion(), + "propagate_tags": "true", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ + "region_name": acctest.ThirdRegion(), + "propagate_tags": "true", + }), + ), + }, + { + Config: testAccTableConfig_replicaTagsUpdate5(rName, acctest.AlternateRegion(), acctest.ThirdRegion()), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInitialTableExists(resourceName, &conf), + testAccCheckReplicaHasTags(resourceName, acctest.AlternateRegion(), 1), + testAccCheckReplicaHasTags(resourceName, acctest.ThirdRegion(), 1), resource.TestCheckResourceAttr(resourceName, "replica.#", "2"), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "replica.*", map[string]string{ "region_name": acctest.AlternateRegion(), @@ -1854,15 +2041,15 @@ func testAccCheckInitialTableExists(n string, table *dynamodb.DescribeTableOutpu } } -func testAccCheckReplicaHasTags(n string, region string, should bool) resource.TestCheckFunc { +func testAccCheckReplicaHasTags(n string, region string, count int) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { - return fmt.Errorf("Not found: %s", n) + return fmt.Errorf("not found: %s", n) } if rs.Primary.ID == "" { - return fmt.Errorf("No DynamoDB table name specified!") + return fmt.Errorf("no DynamoDB table name specified!") } conn := acctest.Provider.Meta().(*conns.AWSClient).DynamoDBConn @@ -1889,12 +2076,8 @@ func testAccCheckReplicaHasTags(n string, region string, should bool) resource.T return create.Error(names.DynamoDB, create.ErrActionChecking, tfdynamodb.ResNameTable, rs.Primary.Attributes["arn"], err) } - if len(tags.Keys()) > 0 && !should { - return create.Error(names.DynamoDB, create.ErrActionChecking, tfdynamodb.ResNameTable, rs.Primary.Attributes["arn"], errors.New("replica should not have tags but does")) - } - - if len(tags.Keys()) == 0 && should { - return create.Error(names.DynamoDB, create.ErrActionChecking, tfdynamodb.ResNameTable, rs.Primary.Attributes["arn"], errors.New("replica should have tags but does not")) + if len(tags.Keys()) != count { + return create.Error(names.DynamoDB, create.ErrActionChecking, tfdynamodb.ResNameTable, rs.Primary.Attributes["arn"], fmt.Errorf("expected %d tags on replica, found %d", count, len(tags.Keys()))) } return nil @@ -2899,7 +3082,7 @@ resource "aws_dynamodb_table" "test" { `, rName, mainPITR, replica1, replica2)) } -func testAccTableConfig_replica2(rName string) string { +func testAccTableConfig_replicaTags(rName, key, value string, propagate1, propagate2 bool) string { return acctest.ConfigCompose( acctest.ConfigMultipleRegionProvider(3), fmt.Sprintf(` @@ -2924,17 +3107,25 @@ resource "aws_dynamodb_table" "test" { } replica { - region_name = data.aws_region.alternate.name + region_name = data.aws_region.alternate.name + propagate_tags = %[4]t } replica { - region_name = data.aws_region.third.name + region_name = data.aws_region.third.name + propagate_tags = %[5]t + } + + tags = { + Name = %[1]q + Pozo = "Amargo" + %[2]s = %[3]q } } -`, rName)) +`, rName, key, value, propagate1, propagate2)) } -func testAccTableConfig_replicaTags(rName, key, value string, propagate1, propagate2 bool) string { +func testAccTableConfig_replica2(rName string) string { return acctest.ConfigCompose( acctest.ConfigMultipleRegionProvider(3), fmt.Sprintf(` @@ -2959,22 +3150,244 @@ resource "aws_dynamodb_table" "test" { } replica { - region_name = data.aws_region.alternate.name - propagate_tags = %[4]t + region_name = data.aws_region.alternate.name } replica { - region_name = data.aws_region.third.name + region_name = data.aws_region.third.name + } +} +`, rName)) +} + +func testAccTableConfig_replicaTagsNext1(rName string, region1 string, propagate1 bool) string { + return acctest.ConfigCompose( + acctest.ConfigMultipleRegionProvider(3), + 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" + } + + replica { + region_name = %[2]q + propagate_tags = %[3]t + } + + tags = { + Name = %[1]q + Pozo = "Amargo" + } +} +`, rName, region1, propagate1)) +} + +func testAccTableConfig_replicaTagsNext2(rName, region1 string, propagate1 bool, region2 string, propagate2 bool) string { + return acctest.ConfigCompose( + acctest.ConfigMultipleRegionProvider(3), + 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" + } + + replica { + region_name = %[2]q + propagate_tags = %[3]t + } + + replica { + region_name = %[4]q propagate_tags = %[5]t } tags = { Name = %[1]q Pozo = "Amargo" - %[2]s = %[3]q } } -`, rName, key, value, propagate1, propagate2)) +`, rName, region1, propagate1, region2, propagate2)) +} + +func testAccTableConfig_replicaTagsUpdate1(rName, region1 string) string { + return acctest.ConfigCompose( + acctest.ConfigMultipleRegionProvider(3), + 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" + } + + replica { + region_name = %[2]q + propagate_tags = true + } + + tags = { + Name = %[1]q + Pozo = "Amargo" + } +} +`, rName, region1)) +} + +func testAccTableConfig_replicaTagsUpdate2(rName, region1 string) string { + return acctest.ConfigCompose( + acctest.ConfigMultipleRegionProvider(3), + 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" + } + + replica { + region_name = %[2]q + propagate_tags = true + } + + tags = { + Name = %[1]q + Pozo = "Amargo" + tyDi = "Lullaby" + Thrill = "Seekers" + } +} +`, rName, region1)) +} + +func testAccTableConfig_replicaTagsUpdate3(rName, region1 string, region2 string) string { + return acctest.ConfigCompose( + acctest.ConfigMultipleRegionProvider(3), + 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" + } + + replica { + region_name = %[2]q + propagate_tags = true + } + + replica { + region_name = %[3]q + propagate_tags = true + } + + tags = { + Name = %[1]q + Pozo = "Amargo" + tyDi = "Lullaby" + Thrill = "Seekers" + } +} +`, rName, region1, region2)) +} + +func testAccTableConfig_replicaTagsUpdate4(rName, region1 string, region2 string) string { + return acctest.ConfigCompose( + acctest.ConfigMultipleRegionProvider(3), + 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" + } + + replica { + region_name = %[2]q + propagate_tags = true + } + + replica { + region_name = %[3]q + propagate_tags = true + } + + tags = { + Name = %[1]q + Pozo = "Amargo" + tyDi = "Lullaby" + Thrill = "Seekers" + Tristan = "Joe" + Humming = "bird" + } +} +`, rName, region1, region2)) +} + +func testAccTableConfig_replicaTagsUpdate5(rName, region1 string, region2 string) string { + return acctest.ConfigCompose( + acctest.ConfigMultipleRegionProvider(3), + 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" + } + + replica { + region_name = %[2]q + propagate_tags = true + } + + replica { + region_name = %[3]q + propagate_tags = true + } + + tags = { + Name = %[1]q + } +} +`, rName, region1, region2)) } func testAccTableConfig_lsi(rName, lsiName string) string { diff --git a/website/docs/r/dynamodb_table.html.markdown b/website/docs/r/dynamodb_table.html.markdown index 4ed774f1413..a1969085b23 100644 --- a/website/docs/r/dynamodb_table.html.markdown +++ b/website/docs/r/dynamodb_table.html.markdown @@ -219,7 +219,7 @@ Optional arguments: * `kms_key_arn` - (Optional) ARN of the CMK that should be used for the AWS KMS encryption. * `point_in_time_recovery` - (Optional) Whether to enable Point In Time Recovery for the replica. Default is `false`. -* `propagate_tags` - (Optional) Whether to propagate the main table's tags to a replica. Default is `false`. Changes to tags only move in one direction: from main to replica. In other words, tag drift on a replica will not trigger an update. Tag changes on the main table, whether from drift or configuration changes, are propagated to replicas. +* `propagate_tags` - (Optional) Whether to propagate the global table's tags to a replica. Default is `false`. Changes to tags only move in one direction: from global (source) to replica. In other words, tag drift on a replica will not trigger an update. Tag or replica changes on the global table, whether from drift or configuration changes, are propagated to replicas. Changing from `true` to `false` on a subsequent `apply` means replica tags are left as they were, unmanaged, not deleted. * `region_name` - (Required) Region name of the replica. ### `server_side_encryption` From a05c5a66c14c220a46006f0aa6f98ca40b960026 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 11 Aug 2022 17:14:53 -0400 Subject: [PATCH 2/4] Add changelog --- .changelog/26257.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/26257.txt diff --git a/.changelog/26257.txt b/.changelog/26257.txt new file mode 100644 index 00000000000..3b328c2d122 --- /dev/null +++ b/.changelog/26257.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_dynamodb_table: Fix `replica.*.propagate_tags` not propagating tags to newly added replicas +``` \ No newline at end of file From 83d38c4b298091bb443b685623d9e2410f2dcd4c Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 11 Aug 2022 17:16:48 -0400 Subject: [PATCH 3/4] terrafmt fixes --- internal/service/dynamodb/table_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/service/dynamodb/table_test.go b/internal/service/dynamodb/table_test.go index cf81eb0db80..dee13e1a285 100644 --- a/internal/service/dynamodb/table_test.go +++ b/internal/service/dynamodb/table_test.go @@ -3182,8 +3182,8 @@ resource "aws_dynamodb_table" "test" { } tags = { - Name = %[1]q - Pozo = "Amargo" + Name = %[1]q + Pozo = "Amargo" } } `, rName, region1, propagate1)) @@ -3216,8 +3216,8 @@ resource "aws_dynamodb_table" "test" { } tags = { - Name = %[1]q - Pozo = "Amargo" + Name = %[1]q + Pozo = "Amargo" } } `, rName, region1, propagate1, region2, propagate2)) @@ -3245,8 +3245,8 @@ resource "aws_dynamodb_table" "test" { } tags = { - Name = %[1]q - Pozo = "Amargo" + Name = %[1]q + Pozo = "Amargo" } } `, rName, region1)) @@ -3384,7 +3384,7 @@ resource "aws_dynamodb_table" "test" { } tags = { - Name = %[1]q + Name = %[1]q } } `, rName, region1, region2)) From 32e9af761a911628fea90e4026f586a892c17b6b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 11 Aug 2022 17:21:27 -0400 Subject: [PATCH 4/4] Remove commented out code --- internal/service/dynamodb/table.go | 48 ------------------------------ 1 file changed, 48 deletions(-) diff --git a/internal/service/dynamodb/table.go b/internal/service/dynamodb/table.go index 59606660c5f..ca5b27ba12f 100644 --- a/internal/service/dynamodb/table.go +++ b/internal/service/dynamodb/table.go @@ -974,54 +974,6 @@ func resourceTableDelete(d *schema.ResourceData, meta interface{}) error { } // custom diff -/* -func replicaTagsDiff(_ context.Context, diff *schema.ResourceDiff, meta interface{}) error { - - defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig - ignoreTagsConfig := meta.(*conns.AWSClient).IgnoreTagsConfig - - if v, ok := tfMap["propagate_tags"].(bool); ok && v { - if aws.StringValue(conn.Config.Region) != region { - session, err := conns.NewSessionForRegion(&conn.Config, region, terraformVersion) - if err != nil { - return fmt.Errorf("updating replica (%s) tags: %w", region, err) - } - - conn = dynamodb.New(session) - } - } - - resourceTags := tftags.New(diff.Get("tags").(map[string]interface{})) - - if defaultTagsConfig.TagsEqual(resourceTags) { - return fmt.Errorf(`"tags" are identical to those in the "default_tags" configuration block of the provider: please de-duplicate and try again`) - } - - allTags := defaultTagsConfig.MergeTags(resourceTags).IgnoreConfig(ignoreTagsConfig) - - // To ensure "tags_all" is correctly computed, we explicitly set the attribute diff - // when the merger of resource-level tags onto provider-level tags results in n > 0 tags, - // otherwise we mark the attribute as "Computed" only when their is a known diff (excluding an empty map) - // or a change for "tags_all". - // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/18366 - // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/19005 - if len(allTags) > 0 { - if err := diff.SetNew("tags_all", allTags.Map()); err != nil { - return fmt.Errorf("error setting new tags_all diff: %w", err) - } - } else if len(diff.Get("tags_all").(map[string]interface{})) > 0 { - if err := diff.SetNewComputed("tags_all"); err != nil { - return fmt.Errorf("error setting tags_all to computed: %w", err) - } - } else if diff.HasChange("tags_all") { - if err := diff.SetNewComputed("tags_all"); err != nil { - return fmt.Errorf("error setting tags_all to computed: %w", err) - } - } - - return nil -} -*/ func isTableOptionDisabled(v interface{}) bool { options := v.([]interface{})