From e25eb053ee731dc698fd35af0ecc6630e84d87cf Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 22 Jul 2024 17:41:11 -0400 Subject: [PATCH 1/5] Add 'TestAccElastiCacheReplicationGroup_stateUpgrade5590'. --- .../elasticache/replication_group_test.go | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/internal/service/elasticache/replication_group_test.go b/internal/service/elasticache/replication_group_test.go index a1d179e2fac..cb722360e0b 100644 --- a/internal/service/elasticache/replication_group_test.go +++ b/internal/service/elasticache/replication_group_test.go @@ -643,6 +643,45 @@ func TestAccElastiCacheReplicationGroup_stateUpgrade5270(t *testing.T) { }) } +// https://github.com/hashicorp/terraform-provider-aws/issues/38464. +func TestAccElastiCacheReplicationGroup_stateUpgrade5590(t *testing.T) { + ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var rg awstypes.ReplicationGroup + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_elasticache_replication_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.ElastiCacheServiceID), + CheckDestroy: testAccCheckReplicationGroupDestroy(ctx), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "aws": { + Source: "hashicorp/aws", + VersionConstraint: "4.67.0", + }, + }, + Config: testAccReplicationGroupConfig_basic(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckReplicationGroupExists(ctx, resourceName, &rg), + ), + }, + { + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + Config: testAccReplicationGroupConfig_basic(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckReplicationGroupExists(ctx, resourceName, &rg), + ), + }, + }, + }) +} + func TestAccElastiCacheReplicationGroup_vpc(t *testing.T) { ctx := acctest.Context(t) if testing.Short() { From 0afa6adf89a3c9f91c20b37c564a76744482a574 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 22 Jul 2024 17:42:46 -0400 Subject: [PATCH 2/5] r/aws_elasticache_replication_group: Fix problem with reintroduction of a 'cluster_mode' attribute when upgrading from v4.67.0 to v5.59.0. --- .../elasticache/replication_group_migrate.go | 171 +++++++----------- 1 file changed, 62 insertions(+), 109 deletions(-) diff --git a/internal/service/elasticache/replication_group_migrate.go b/internal/service/elasticache/replication_group_migrate.go index f94db85fcc3..2de8473c7eb 100644 --- a/internal/service/elasticache/replication_group_migrate.go +++ b/internal/service/elasticache/replication_group_migrate.go @@ -9,11 +9,10 @@ import ( awstypes "github.com/aws/aws-sdk-go-v2/service/elasticache/types" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" - "github.com/hashicorp/terraform-provider-aws/internal/enum" + + tfmaps "github.com/hashicorp/terraform-provider-aws/internal/maps" "github.com/hashicorp/terraform-provider-aws/internal/sdkv2/types/nullable" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" - "github.com/hashicorp/terraform-provider-aws/internal/verify" "github.com/hashicorp/terraform-provider-aws/names" ) @@ -22,9 +21,20 @@ func replicationGroupStateUpgradeV1(ctx context.Context, rawState map[string]int rawState = map[string]interface{}{} } - // Set auth_token_update_strategy to new default value + // Set auth_token_update_strategy to new default value. rawState["auth_token_update_strategy"] = awstypes.AuthTokenUpdateStrategyTypeRotate + // The v4.67.0 schema contained block attribute named 'cluster_mode'. + // It was removed at v5.0.0. + // The v5.59.0 schema introduced a new string attribute named 'cluster_mode'. + // Remove any trace of the old cluster_mode block. + for _, k := range tfmaps.Keys(rawState) { + if strings.HasPrefix(k, "cluster_mode.") { + delete(rawState, k) + } + } + delete(rawState, "cluster_mode") + return rawState, nil } @@ -48,17 +58,14 @@ func resourceReplicationGroupConfigV1() *schema.Resource { Computed: true, }, "auth_token": { - Type: schema.TypeString, - Optional: true, - Sensitive: true, - ValidateFunc: validReplicationGroupAuthToken, - ConflictsWith: []string{"user_group_ids"}, + Type: schema.TypeString, + Optional: true, + Sensitive: true, }, names.AttrAutoMinorVersionUpgrade: { - Type: nullable.TypeNullableBool, - Optional: true, - Computed: true, - ValidateFunc: nullable.ValidateTypeStringNullableBool, + Type: nullable.TypeNullableBool, + Optional: true, + Computed: true, }, "automatic_failover_enabled": { Type: schema.TypeBool, @@ -80,23 +87,20 @@ func resourceReplicationGroupConfigV1() *schema.Resource { ForceNew: true, }, names.AttrDescription: { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validation.StringIsNotEmpty, + Type: schema.TypeString, + Optional: true, + Computed: true, }, names.AttrEngine: { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - Default: engineRedis, - ValidateFunc: validation.StringInSlice([]string{engineRedis}, true), + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Default: engineRedis, }, names.AttrEngineVersion: { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validRedisVersionString, + Type: schema.TypeString, + Optional: true, + Computed: true, }, "engine_version_actual": { Type: schema.TypeString, @@ -107,24 +111,11 @@ func resourceReplicationGroupConfigV1() *schema.Resource { Optional: true, ForceNew: true, Computed: true, - ConflictsWith: []string{ - "num_node_groups", - names.AttrParameterGroupName, - names.AttrEngine, - names.AttrEngineVersion, - "node_type", - "security_group_names", - "transit_encryption_enabled", - "at_rest_encryption_enabled", - "snapshot_arns", - "snapshot_name", - }, }, "ip_discovery": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateDiagFunc: enum.Validate[awstypes.IpDiscovery](), + Type: schema.TypeString, + Optional: true, + Computed: true, }, "log_delivery_configuration": { Type: schema.TypeSet, @@ -133,23 +124,20 @@ func resourceReplicationGroupConfigV1() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "destination_type": { - Type: schema.TypeString, - Required: true, - ValidateDiagFunc: enum.Validate[awstypes.DestinationType](), + Type: schema.TypeString, + Required: true, }, names.AttrDestination: { Type: schema.TypeString, Required: true, }, "log_format": { - Type: schema.TypeString, - Required: true, - ValidateDiagFunc: enum.Validate[awstypes.LogFormat](), + Type: schema.TypeString, + Required: true, }, "log_type": { - Type: schema.TypeString, - Required: true, - ValidateDiagFunc: enum.Validate[awstypes.LogType](), + Type: schema.TypeString, + Required: true, }, }, }, @@ -158,17 +146,11 @@ func resourceReplicationGroupConfigV1() *schema.Resource { Type: schema.TypeString, Optional: true, Computed: true, - StateFunc: func(val interface{}) string { - // ElastiCache always changes the maintenance to lowercase - return strings.ToLower(val.(string)) - }, - ValidateFunc: verify.ValidOnceAWeekWindowFormat, }, "member_clusters": { Type: schema.TypeSet, Computed: true, Elem: &schema.Schema{Type: schema.TypeString}, - Set: schema.HashString, }, "multi_az_enabled": { Type: schema.TypeBool, @@ -176,11 +158,10 @@ func resourceReplicationGroupConfigV1() *schema.Resource { Default: false, }, "network_type": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, - ValidateDiagFunc: enum.Validate[awstypes.NetworkType](), + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, }, "node_type": { Type: schema.TypeString, @@ -188,41 +169,28 @@ func resourceReplicationGroupConfigV1() *schema.Resource { Computed: true, }, "notification_topic_arn": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: verify.ValidARN, + Type: schema.TypeString, + Optional: true, }, "num_cache_clusters": { - Type: schema.TypeInt, - Computed: true, - Optional: true, - ConflictsWith: []string{"num_node_groups"}, + Type: schema.TypeInt, + Computed: true, + Optional: true, }, "num_node_groups": { - Type: schema.TypeInt, - Optional: true, - Computed: true, - ConflictsWith: []string{"num_cache_clusters", "global_replication_group_id"}, + Type: schema.TypeInt, + Optional: true, + Computed: true, }, names.AttrParameterGroupName: { Type: schema.TypeString, Optional: true, Computed: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - return strings.HasPrefix(old, "global-datastore-") - }, }, names.AttrPort: { Type: schema.TypeInt, Optional: true, ForceNew: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - // Suppress default Redis ports when not defined - if !d.IsNewResource() && new == "0" && old == defaultRedisPort { - return true - } - return false - }, }, "preferred_cache_cluster_azs": { Type: schema.TypeList, @@ -243,13 +211,9 @@ func resourceReplicationGroupConfigV1() *schema.Resource { Computed: true, }, "replication_group_id": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - ValidateFunc: validateReplicationGroupID, - StateFunc: func(val interface{}) string { - return strings.ToLower(val.(string)) - }, + Type: schema.TypeString, + Required: true, + ForceNew: true, }, "security_group_names": { Type: schema.TypeSet, @@ -257,14 +221,12 @@ func resourceReplicationGroupConfigV1() *schema.Resource { Computed: true, ForceNew: true, Elem: &schema.Schema{Type: schema.TypeString}, - Set: schema.HashString, }, names.AttrSecurityGroupIDs: { Type: schema.TypeSet, Optional: true, Computed: true, Elem: &schema.Schema{Type: schema.TypeString}, - Set: schema.HashString, }, "snapshot_arns": { Type: schema.TypeSet, @@ -273,23 +235,16 @@ func resourceReplicationGroupConfigV1() *schema.Resource { // Note: Unlike aws_elasticache_cluster, this does not have a limit of 1 item. Elem: &schema.Schema{ Type: schema.TypeString, - ValidateFunc: validation.All( - verify.ValidARN, - validation.StringDoesNotContainAny(","), - ), }, - Set: schema.HashString, }, "snapshot_retention_limit": { - Type: schema.TypeInt, - Optional: true, - ValidateFunc: validation.IntAtMost(35), + Type: schema.TypeInt, + Optional: true, }, "snapshot_window": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: verify.ValidOnceADayWindowFormat, + Type: schema.TypeString, + Optional: true, + Computed: true, }, "snapshot_name": { Type: schema.TypeString, @@ -311,11 +266,9 @@ func resourceReplicationGroupConfigV1() *schema.Resource { Computed: true, }, "user_group_ids": { - Type: schema.TypeSet, - Optional: true, - Elem: &schema.Schema{Type: schema.TypeString}, - Set: schema.HashString, - ConflictsWith: []string{"auth_token"}, + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, }, names.AttrKMSKeyID: { Type: schema.TypeString, From 4943c3d321cbc7399496ca1c6accf6e3d14bd5a5 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 22 Jul 2024 17:46:58 -0400 Subject: [PATCH 3/5] Add CHANGELOG entry. --- .changelog/38476.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/38476.txt diff --git a/.changelog/38476.txt b/.changelog/38476.txt new file mode 100644 index 00000000000..f3b573dc6e5 --- /dev/null +++ b/.changelog/38476.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_elasticache_replication_group: Fix `error marshaling prior state` when upgrading from v4.67.0 to v5.59.0 +``` \ No newline at end of file From 139dbf43592f8fe561fad020fbe89c55ea302d1c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 23 Jul 2024 08:27:05 -0400 Subject: [PATCH 4/5] Fix importlint 'Import groups are not in the proper order: ["Std" "Third party" "Third party"]'. --- internal/service/elasticache/replication_group_migrate.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/service/elasticache/replication_group_migrate.go b/internal/service/elasticache/replication_group_migrate.go index 2de8473c7eb..188262962fc 100644 --- a/internal/service/elasticache/replication_group_migrate.go +++ b/internal/service/elasticache/replication_group_migrate.go @@ -9,7 +9,6 @@ import ( awstypes "github.com/aws/aws-sdk-go-v2/service/elasticache/types" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - tfmaps "github.com/hashicorp/terraform-provider-aws/internal/maps" "github.com/hashicorp/terraform-provider-aws/internal/sdkv2/types/nullable" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" From 4cdea3fd1fd0732d6fee6836b4ca9e9222b3c3a4 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 23 Jul 2024 08:31:36 -0400 Subject: [PATCH 5/5] Fix semgrep 'ci.semgrep.types.valid-nullable-bool'. --- internal/service/elasticache/replication_group_migrate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/elasticache/replication_group_migrate.go b/internal/service/elasticache/replication_group_migrate.go index 188262962fc..bf4b6526629 100644 --- a/internal/service/elasticache/replication_group_migrate.go +++ b/internal/service/elasticache/replication_group_migrate.go @@ -61,7 +61,7 @@ func resourceReplicationGroupConfigV1() *schema.Resource { Optional: true, Sensitive: true, }, - names.AttrAutoMinorVersionUpgrade: { + names.AttrAutoMinorVersionUpgrade: { // nosemgrep:ci.semgrep.types.valid-nullable-bool Type: nullable.TypeNullableBool, Optional: true, Computed: true,