From 6913625cbf0ba1bd5b309a7f6030599762c9dca4 Mon Sep 17 00:00:00 2001 From: timothy-welsh_ramp Date: Thu, 15 May 2025 09:04:22 -0400 Subject: [PATCH 1/5] Support Valkey upgrades on elasticache_global_replication_group Once a replication group is added to a global datastore, all of it's engine upgrades need to be controlled on the global datastore. This resource did not support setting an explicit engine value on global datastores, preventing upgrades (#41618). Further, if creating a Valkey global datastore by inheriting version from the primary (or manually upgrading the engine of the global datastore in AWS Console), then you could not continue manage the secondary replication groups in terraform. A Valkey engine would cause an incorrect destroy & re-create plan (if using defaults for engine) or a conflict (if trying to set the engine correctly). We remove the default engine value, mark it as computed and add DiffSuppressFunc to engine, engine_version and parmeter_group_name to ignore changes to these attributes in plans when the resource belongs to a global datastore (primary or secondary). Fixes #40786 and #41713. This also has the benefit of removing the need to add explicit ignore_changes lifecycle blocks to attempt to work around these plans. We update existing documentation on around this. --- .../service/elasticache/engine_version.go | 22 ++ .../elasticache/engine_version_test.go | 4 + internal/service/elasticache/exports_test.go | 2 +- .../elasticache/global_replication_group.go | 68 ++++- .../global_replication_group_test.go | 278 ++++++++++++++++-- .../service/elasticache/replication_group.go | 45 +-- ...che_global_replication_group.html.markdown | 21 +- 7 files changed, 362 insertions(+), 78 deletions(-) diff --git a/internal/service/elasticache/engine_version.go b/internal/service/elasticache/engine_version.go index 412413e6ccd0..4ce8e8cdb720 100644 --- a/internal/service/elasticache/engine_version.go +++ b/internal/service/elasticache/engine_version.go @@ -13,6 +13,7 @@ import ( "github.com/YakDriver/regexache" "github.com/aws/aws-sdk-go-v2/aws" gversion "github.com/hashicorp/go-version" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/names" ) @@ -109,6 +110,22 @@ func customizeDiffEngineVersionForceNewOnDowngrade(_ context.Context, diff *sche return engineVersionForceNewOnDowngrade(diff) } +func customizeDiffEngineForceNewOnDowngrade() schema.CustomizeDiffFunc { + return customdiff.ForceNewIf(names.AttrEngine, func(_ context.Context, diff *schema.ResourceDiff, meta any) bool { + if _, is_global := diff.GetOk("global_replication_group_id"); is_global { + return false + } + + if !diff.HasChange(names.AttrEngine) { + return false + } + if old, new := diff.GetChange(names.AttrEngine); old.(string) == engineRedis && new.(string) == engineValkey { + return false + } + return true + }) +} + type getChangeDiffer interface { Get(key string) any GetChange(key string) (any, any) @@ -151,12 +168,17 @@ func engineVersionIsDowngrade(diff getChangeDiffer) (bool, error) { type forceNewDiffer interface { Id() string Get(key string) any + GetOk(key string) (any, bool) GetChange(key string) (any, any) HasChange(key string) bool ForceNew(key string) error } func engineVersionForceNewOnDowngrade(diff forceNewDiffer) error { + if _, is_global := diff.GetOk("global_replication_group_id"); is_global { + return nil + } + if diff.Id() == "" || !diff.HasChange(names.AttrEngineVersion) { return nil } diff --git a/internal/service/elasticache/engine_version_test.go b/internal/service/elasticache/engine_version_test.go index b9101f353c09..929401a7917e 100644 --- a/internal/service/elasticache/engine_version_test.go +++ b/internal/service/elasticache/engine_version_test.go @@ -513,6 +513,10 @@ func (d *mockForceNewDiffer) Get(key string) any { return d.old } +func (d *mockForceNewDiffer) GetOk(key string) (any, bool) { + return d.old, d.old != "" +} + func (d *mockForceNewDiffer) HasChange(key string) bool { return d.hasChange || d.old != d.new } diff --git a/internal/service/elasticache/exports_test.go b/internal/service/elasticache/exports_test.go index 0a2778624848..4904a245956c 100644 --- a/internal/service/elasticache/exports_test.go +++ b/internal/service/elasticache/exports_test.go @@ -41,7 +41,7 @@ var ( EngineVersionIsDowngrade = engineVersionIsDowngrade GlobalReplicationGroupRegionPrefixFormat = globalReplicationGroupRegionPrefixFormat NormalizeEngineVersion = normalizeEngineVersion - ParamGroupNameRequiresMajorVersionUpgrade = paramGroupNameRequiresMajorVersionUpgrade + ParamGroupNameRequiresMajorVersionUpgrade = paramGroupNameRequiresEngineOrMajorVersionUpgrade ValidateClusterEngineVersion = validateClusterEngineVersion ValidMemcachedVersionString = validMemcachedVersionString ValidRedisVersionString = validRedisVersionString diff --git a/internal/service/elasticache/global_replication_group.go b/internal/service/elasticache/global_replication_group.go index abf04d1a2618..3f0546e44bbf 100644 --- a/internal/service/elasticache/global_replication_group.go +++ b/internal/service/elasticache/global_replication_group.go @@ -90,8 +90,10 @@ func resourceGlobalReplicationGroup() *schema.Resource { Computed: true, }, names.AttrEngine: { - Type: schema.TypeString, - Computed: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateFunc: validation.StringInSlice([]string{engineRedis, engineValkey}, true), }, names.AttrEngineVersion: { Type: schema.TypeString, @@ -210,7 +212,8 @@ func resourceGlobalReplicationGroup() *schema.Resource { CustomizeDiff: customdiff.All( customizeDiffGlobalReplicationGroupEngineVersionErrorOnDowngrade, - customizeDiffGlobalReplicationGroupParamGroupNameRequiresMajorVersionUpgrade, + customizeDiffEngineForceNewOnDowngrade(), + customizeDiffGlobalReplicationGroupParamGroupNameRequiresEngineOrMajorVersionUpgrade, customdiff.ComputedIf("global_node_groups", diffHasChange("num_node_groups")), ), } @@ -233,18 +236,23 @@ of the Global Replication Group and all Replication Group members. The AWS provi Please use the "-replace" option on the terraform plan and apply commands (see https://www.terraform.io/cli/commands/plan#replace-address).`, diff.Id()) } -func customizeDiffGlobalReplicationGroupParamGroupNameRequiresMajorVersionUpgrade(_ context.Context, diff *schema.ResourceDiff, _ any) error { - return paramGroupNameRequiresMajorVersionUpgrade(diff) +func customizeDiffGlobalReplicationGroupParamGroupNameRequiresEngineOrMajorVersionUpgrade(_ context.Context, diff *schema.ResourceDiff, _ any) error { + return paramGroupNameRequiresEngineOrMajorVersionUpgrade(diff) } // parameter_group_name can only be set when doing a major update, // but we also should allow it to stay set afterwards -func paramGroupNameRequiresMajorVersionUpgrade(diff sdkv2.ResourceDiffer) error { +func paramGroupNameRequiresEngineOrMajorVersionUpgrade(diff sdkv2.ResourceDiffer) error { o, n := diff.GetChange(names.AttrParameterGroupName) if o.(string) == n.(string) { return nil } + // param group must be able to change on Redis 7.1 to Valkey 7.2 upgrade + if diff.HasChange(names.AttrEngine) { + return nil + } + if diff.Id() == "" { if !diff.HasChange(names.AttrEngineVersion) { return errors.New("cannot change parameter group name without upgrading major engine version") @@ -262,7 +270,7 @@ func paramGroupNameRequiresMajorVersionUpgrade(diff sdkv2.ResourceDiffer) error if vDiff[0] == 0 && vDiff[1] == 0 { return errors.New("cannot change parameter group name without upgrading major engine version") } - if vDiff[0] != 1 { + if vDiff[0] == 0 { return fmt.Errorf("cannot change parameter group name on minor engine version upgrade, upgrading from %s to %s", oldVersion.String(), newVersion.String()) } } @@ -318,9 +326,25 @@ func resourceGlobalReplicationGroupCreate(ctx context.Context, d *schema.Resourc } } - if v, ok := d.GetOk(names.AttrEngineVersion); ok { + if e, ok := d.GetOk(names.AttrEngine); ok { + if e.(string) == aws.ToString(globalReplicationGroup.Engine) { + log.Printf("[DEBUG] Not updating ElastiCache Global Replication Group (%s) engine: no change from %q", d.Id(), e) + } else { + version := d.Get(names.AttrEngineVersion).(string) + p := d.Get(names.AttrParameterGroupName).(string) + if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(e.(string), version, p), "engine", d.Timeout(schema.TimeoutUpdate)); err != nil { + return sdkdiag.AppendFromErr(diags, err) + } + } + } else if v, ok := d.GetOk(names.AttrEngineVersion); ok { requestedVersion, _ := normalizeEngineVersion(v.(string)) + // backwards-compatibility; imply redis engine if just given engine version + engine, ok := d.GetOk(names.AttrEngine) + if !ok { + engine = engineRedis + } + engineVersion, err := gversion.NewVersion(aws.ToString(globalReplicationGroup.EngineVersion)) if err != nil { return sdkdiag.AppendErrorf(diags, "updating ElastiCache Global Replication Group (%s) engine version on creation: error reading engine version: %s", d.Id(), err) @@ -335,7 +359,7 @@ func resourceGlobalReplicationGroupCreate(ctx context.Context, d *schema.Resourc p := d.Get(names.AttrParameterGroupName).(string) if diff[0] == 1 { - if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(v.(string), p), "engine version (major)", d.Timeout(schema.TimeoutCreate)); err != nil { + if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(engine.(string), v.(string), p), "engine version (major)", d.Timeout(schema.TimeoutCreate)); err != nil { return sdkdiag.AppendFromErr(diags, err) } } else if diff[1] == 1 { @@ -343,7 +367,7 @@ func resourceGlobalReplicationGroupCreate(ctx context.Context, d *schema.Resourc return sdkdiag.AppendErrorf(diags, "cannot change parameter group name on minor engine version upgrade, upgrading from %s to %s", engineVersion.String(), requestedVersion.String()) } if t, _ := regexp.MatchString(`[6-9]\.x`, v.(string)); !t { - if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMinorUpdater(v.(string)), "engine version (minor)", d.Timeout(schema.TimeoutCreate)); err != nil { + if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMinorUpdater(engine.(string), v.(string)), "engine version (minor)", d.Timeout(schema.TimeoutCreate)); err != nil { return sdkdiag.AppendFromErr(diags, err) } } @@ -440,20 +464,32 @@ func resourceGlobalReplicationGroupUpdate(ctx context.Context, d *schema.Resourc } } - if d.HasChange(names.AttrEngineVersion) { + if d.HasChange(names.AttrEngine) { + engine := d.Get(names.AttrEngine).(string) + version := d.Get(names.AttrEngineVersion).(string) + p := d.Get(names.AttrParameterGroupName).(string) + if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(engine, version, p), "engine version (major)", d.Timeout(schema.TimeoutUpdate)); err != nil { + return sdkdiag.AppendFromErr(diags, err) + } + } else if d.HasChange(names.AttrEngineVersion) { o, n := d.GetChange(names.AttrEngineVersion) newVersion, _ := normalizeEngineVersion(n.(string)) oldVersion, _ := gversion.NewVersion(o.(string)) + // backwards-compatibility; imply redis engine if just given engine version + engine, ok := d.GetOk(names.AttrEngine) + if !ok { + engine = engineRedis + } diff := diffVersion(newVersion, oldVersion) if diff[0] == 1 { p := d.Get(names.AttrParameterGroupName).(string) - if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(n.(string), p), "engine version (major)", d.Timeout(schema.TimeoutUpdate)); err != nil { + if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(engine.(string), n.(string), p), "engine version (major)", d.Timeout(schema.TimeoutUpdate)); err != nil { return sdkdiag.AppendFromErr(diags, err) } } else if diff[1] == 1 { - if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMinorUpdater(n.(string)), "engine version (minor)", d.Timeout(schema.TimeoutUpdate)); err != nil { + if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMinorUpdater(engine.(string), n.(string)), "engine version (minor)", d.Timeout(schema.TimeoutUpdate)); err != nil { return sdkdiag.AppendFromErr(diags, err) } } @@ -509,14 +545,16 @@ func globalReplicationGroupDescriptionUpdater(description string) globalReplicat } } -func globalReplicationGroupEngineVersionMinorUpdater(version string) globalReplicationGroupUpdater { +func globalReplicationGroupEngineVersionMinorUpdater(engine, version string) globalReplicationGroupUpdater { return func(input *elasticache.ModifyGlobalReplicationGroupInput) { + input.Engine = aws.String(engine) input.EngineVersion = aws.String(version) } } -func globalReplicationGroupEngineVersionMajorUpdater(version, paramGroupName string) globalReplicationGroupUpdater { +func globalReplicationGroupEngineVersionMajorUpdater(engine, version, paramGroupName string) globalReplicationGroupUpdater { return func(input *elasticache.ModifyGlobalReplicationGroupInput) { + input.Engine = aws.String(engine) input.EngineVersion = aws.String(version) input.CacheParameterGroupName = aws.String(paramGroupName) } diff --git a/internal/service/elasticache/global_replication_group_test.go b/internal/service/elasticache/global_replication_group_test.go index 44e40491efcc..06eb8dcceaa6 100644 --- a/internal/service/elasticache/global_replication_group_test.go +++ b/internal/service/elasticache/global_replication_group_test.go @@ -926,10 +926,9 @@ func TestAccElastiCacheGlobalReplicationGroup_SetEngineVersionOnCreate_NoChange_ ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{names.AttrEngineVersion}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, }, }, }) @@ -1230,7 +1229,7 @@ func TestAccElastiCacheGlobalReplicationGroup_SetEngineVersionOnUpdate_MinorUpgr CheckDestroy: testAccCheckGlobalReplicationGroupDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccGlobalReplicationGroupConfig_engineVersionInherit(rName, primaryReplicationGroupId, "6.0"), + Config: testAccGlobalReplicationGroupConfig_Redis_engineVersionInherit(rName, primaryReplicationGroupId, "6.0"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckGlobalReplicationGroupExists(ctx, resourceName, &globalReplicationGroup), resource.TestCheckResourceAttrPair(resourceName, "engine_version_actual", primaryReplicationGroupResourceName, "engine_version_actual"), @@ -1267,7 +1266,7 @@ func TestAccElastiCacheGlobalReplicationGroup_SetEngineVersionOnUpdate_MinorUpgr CheckDestroy: testAccCheckGlobalReplicationGroupDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccGlobalReplicationGroupConfig_engineVersionInherit(rName, primaryReplicationGroupId, "6.0"), + Config: testAccGlobalReplicationGroupConfig_Redis_engineVersionInherit(rName, primaryReplicationGroupId, "6.0"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckGlobalReplicationGroupExists(ctx, resourceName, &globalReplicationGroup), resource.TestCheckResourceAttrPair(resourceName, "engine_version_actual", primaryReplicationGroupResourceName, "engine_version_actual"), @@ -1313,7 +1312,7 @@ func TestAccElastiCacheGlobalReplicationGroup_SetEngineVersionOnUpdate_MinorDown CheckDestroy: testAccCheckGlobalReplicationGroupDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccGlobalReplicationGroupConfig_engineVersionInherit(rName, primaryReplicationGroupId, "6.2"), + Config: testAccGlobalReplicationGroupConfig_Redis_engineVersionInherit(rName, primaryReplicationGroupId, "6.2"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckGlobalReplicationGroupExists(ctx, resourceName, &globalReplicationGroup), resource.TestCheckResourceAttrPair(resourceName, "engine_version_actual", primaryReplicationGroupResourceName, "engine_version_actual"), @@ -1360,7 +1359,7 @@ func TestAccElastiCacheGlobalReplicationGroup_SetEngineVersionOnUpdate_MajorUpgr CheckDestroy: testAccCheckGlobalReplicationGroupDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccGlobalReplicationGroupConfig_engineVersionInherit(rName, primaryReplicationGroupId, "5.0.6"), + Config: testAccGlobalReplicationGroupConfig_Redis_engineVersionInherit(rName, primaryReplicationGroupId, "5.0.6"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckGlobalReplicationGroupExists(ctx, resourceName, &globalReplicationGroup), resource.TestCheckResourceAttrPair(resourceName, "engine_version_actual", primaryReplicationGroupResourceName, "engine_version_actual"), @@ -1402,7 +1401,7 @@ func TestAccElastiCacheGlobalReplicationGroup_SetEngineVersionOnUpdate_MajorUpgr CheckDestroy: testAccCheckGlobalReplicationGroupDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccGlobalReplicationGroupConfig_engineVersionInherit(rName, primaryReplicationGroupId, "5.0.6"), + Config: testAccGlobalReplicationGroupConfig_Redis_engineVersionInherit(rName, primaryReplicationGroupId, "5.0.6"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckGlobalReplicationGroupExists(ctx, resourceName, &globalReplicationGroup), resource.TestCheckResourceAttrPair(resourceName, "engine_version_actual", primaryReplicationGroupResourceName, "engine_version_actual"), @@ -1445,7 +1444,7 @@ func TestAccElastiCacheGlobalReplicationGroup_SetParameterGroupOnUpdate_NoVersio CheckDestroy: testAccCheckGlobalReplicationGroupDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccGlobalReplicationGroupConfig_engineVersionInherit(rName, primaryReplicationGroupId, "6.2"), + Config: testAccGlobalReplicationGroupConfig_Redis_engineVersionInherit(rName, primaryReplicationGroupId, "6.2"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckGlobalReplicationGroupExists(ctx, resourceName, &globalReplicationGroup), resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexache.MustCompile(`^6\.2\.[[:digit:]]+$`)), @@ -1479,7 +1478,7 @@ func TestAccElastiCacheGlobalReplicationGroup_SetParameterGroupOnUpdate_MinorUpg CheckDestroy: testAccCheckGlobalReplicationGroupDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccGlobalReplicationGroupConfig_engineVersionInherit(rName, primaryReplicationGroupId, "6.0"), + Config: testAccGlobalReplicationGroupConfig_Redis_engineVersionInherit(rName, primaryReplicationGroupId, "6.0"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckGlobalReplicationGroupExists(ctx, resourceName, &globalReplicationGroup), resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexache.MustCompile(`^6\.0\.[[:digit:]]+$`)), @@ -1528,6 +1527,179 @@ func TestAccElastiCacheGlobalReplicationGroup_UpdateParameterGroupName(t *testin }) } +func TestAccElastiCacheGlobalReplicationGroup_SetEngineOnCreate_ValkeyUpgrade(t *testing.T) { + ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var globalReplicationGroup awstypes.GlobalReplicationGroup + + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + primaryReplicationGroupId := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_elasticache_global_replication_group.test" + primaryReplicationGroupResourceName := "aws_elasticache_replication_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheckGlobalReplicationGroup(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.ElastiCacheServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckGlobalReplicationGroupDestroy(ctx), + Steps: []resource.TestStep{ + { + // create global datastore with valkey 8.0 engine version from a redis 7.1 primary replication group + Config: testAccGlobalReplicationGroupConfig_engineParam(rName, primaryReplicationGroupId, "redis", "7.1", "valkey", "8.0", "default.valkey8"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckGlobalReplicationGroupExists(ctx, resourceName, &globalReplicationGroup), + resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexache.MustCompile(`^8\.0\.[[:digit:]]+$`)), + ), + }, + { + // check import of global datastore + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{names.AttrParameterGroupName}, + }, + { + // refresh primary replication group after being upgraded by the global datastore + RefreshState: true, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngine, "valkey"), + resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngineVersion, "8.0"), + resource.TestMatchResourceAttr(primaryReplicationGroupResourceName, "parameter_group_name", regexache.MustCompile(`^global-datastore-.+$`)), + resource.TestCheckResourceAttrPair(primaryReplicationGroupResourceName, "global_replication_group_id", resourceName, "global_replication_group_id"), + ), + }, + }, + }) +} + +func TestAccElastiCacheGlobalReplicationGroup_SetEngineOnUpdate_ValkeyUpgrade(t *testing.T) { + ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var globalReplicationGroup awstypes.GlobalReplicationGroup + + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + primaryReplicationGroupId := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_elasticache_global_replication_group.test" + primaryReplicationGroupResourceName := "aws_elasticache_replication_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheckGlobalReplicationGroup(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.ElastiCacheServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckGlobalReplicationGroupDestroy(ctx), + Steps: []resource.TestStep{ + { + // create global datastore using redis 7.1 primary replication group engine version + Config: testAccGlobalReplicationGroupConfig_Redis_engineVersionInherit(rName, primaryReplicationGroupId, "7.1"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckGlobalReplicationGroupExists(ctx, resourceName, &globalReplicationGroup), + resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexache.MustCompile(`^7\.1\.[[:digit:]]+$`)), + ), + }, + { + // check import of global datastore + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{names.AttrParameterGroupName}, + }, + { + // refresh primary replication group after being associated to global datastore + RefreshState: true, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngine, "redis"), + resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngineVersion, "7.1"), + resource.TestMatchResourceAttr(primaryReplicationGroupResourceName, "parameter_group_name", regexache.MustCompile(`^global-datastore-.+$`)), + resource.TestCheckResourceAttrPair(primaryReplicationGroupResourceName, "global_replication_group_id", resourceName, "global_replication_group_id"), + ), + }, + { + // upgrade engine and version on global datastore + Config: testAccGlobalReplicationGroupConfig_engineParam(rName, primaryReplicationGroupId, "redis", "7.1", "valkey", "7.2", "default.valkey7"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckGlobalReplicationGroupExists(ctx, resourceName, &globalReplicationGroup), + resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexache.MustCompile(`^7\.2\.[[:digit:]]+$`)), + ), + }, + { + // check import of global datastore + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{names.AttrParameterGroupName}, + }, + { + // refresh primary replication group after being upgraded by the global datastore + RefreshState: true, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngine, "valkey"), + resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngineVersion, "7.2"), + resource.TestMatchResourceAttr(primaryReplicationGroupResourceName, "parameter_group_name", regexache.MustCompile(`^global-datastore-.+$`)), + resource.TestCheckResourceAttrPair(primaryReplicationGroupResourceName, "global_replication_group_id", resourceName, "global_replication_group_id"), + ), + }, + }, + }) +} + +func TestAccElastiCacheGlobalReplicationGroup_InheritValkeyEngine_SecondaryReplicationGroup(t *testing.T) { + ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var globalReplicationGroup awstypes.GlobalReplicationGroup + + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + primaryReplicationGroupId := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + secondaryReplicationGroupId := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_elasticache_global_replication_group.test" + primaryReplicationGroupResourceName := "aws_elasticache_replication_group.test" + secondaryReplicationGroupResourceName := "aws_elasticache_replication_group.secondary" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckMultipleRegion(t, 2) + testAccPreCheckGlobalReplicationGroup(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, names.ElastiCacheServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(ctx, t, 2), + CheckDestroy: testAccCheckGlobalReplicationGroupDestroy(ctx), + Steps: []resource.TestStep{ + { + // create global datastore using Valkey 8.0 primary replication group and add secondary replication group + Config: testAccGlobalReplicationGroupConfig_Valkey_inheritEngine_secondaryReplicationGroup(rName, primaryReplicationGroupId, secondaryReplicationGroupId), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckGlobalReplicationGroupExists(ctx, resourceName, &globalReplicationGroup), + resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexache.MustCompile(`^8\.0\.[[:digit:]]+$`)), + ), + }, + { + // refresh replication groups to pick up all engine and version computed changes + RefreshState: true, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngine, "valkey"), + resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngineVersion, "8.0"), + resource.TestMatchResourceAttr(primaryReplicationGroupResourceName, "parameter_group_name", regexache.MustCompile(`^global-datastore-.+$`)), + resource.TestCheckResourceAttrPair(primaryReplicationGroupResourceName, "global_replication_group_id", resourceName, "global_replication_group_id"), + + resource.TestCheckResourceAttr(secondaryReplicationGroupResourceName, names.AttrEngine, "valkey"), + resource.TestCheckResourceAttr(secondaryReplicationGroupResourceName, names.AttrEngineVersion, "8.0"), + resource.TestMatchResourceAttr(secondaryReplicationGroupResourceName, "parameter_group_name", regexache.MustCompile(`^global-datastore-.+$`)), + resource.TestCheckResourceAttrPair(secondaryReplicationGroupResourceName, "global_replication_group_id", resourceName, "global_replication_group_id"), + ), + }, + }, + }) +} + func testAccCheckGlobalReplicationGroupExists(ctx context.Context, resourceName string, v *awstypes.GlobalReplicationGroup) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resourceName] @@ -2070,7 +2242,7 @@ resource "aws_elasticache_replication_group" "test" { `, rName, numNodeGroups, globalNumNodeGroups) } -func testAccGlobalReplicationGroupConfig_engineVersionInherit(rName, primaryReplicationGroupId, repGroupEngineVersion string) string { +func testAccGlobalReplicationGroupConfig_Redis_engineVersionInherit(rName, primaryReplicationGroupId, repGroupEngineVersion string) string { return fmt.Sprintf(` resource "aws_elasticache_global_replication_group" "test" { global_replication_group_id_suffix = %[1]q @@ -2106,10 +2278,6 @@ resource "aws_elasticache_replication_group" "test" { engine_version = %[3]q node_type = "cache.m5.large" num_cache_clusters = 1 - - lifecycle { - ignore_changes = [engine_version] - } } `, rName, primaryReplicationGroupId, repGroupEngineVersion, globalEngineVersion) } @@ -2131,10 +2299,6 @@ resource "aws_elasticache_replication_group" "test" { engine_version = %[3]q node_type = "cache.m5.large" num_cache_clusters = 1 - - lifecycle { - ignore_changes = [engine_version] - } } `, rName, primaryReplicationGroupId, repGroupEngineVersion, globalEngineVersion) } @@ -2157,14 +2321,72 @@ resource "aws_elasticache_replication_group" "test" { engine_version = %[3]q node_type = "cache.m5.large" num_cache_clusters = 1 - - lifecycle { - ignore_changes = [engine_version] - } } `, rName, primaryReplicationGroupId, repGroupEngineVersion, globalEngineVersion, parameterGroup) } +func testAccGlobalReplicationGroupConfig_engineParam( + rName, + primaryReplicationGroupId, + repGroupEngine, + repGroupEngineVersion, + globalEngine, + globalEngineVersion, + globalParamGroup string, +) string { + return fmt.Sprintf(` +resource "aws_elasticache_global_replication_group" "test" { + global_replication_group_id_suffix = %[1]q + primary_replication_group_id = aws_elasticache_replication_group.test.id + + engine = %[5]q + engine_version = %[6]q + parameter_group_name = %[7]q +} + +resource "aws_elasticache_replication_group" "test" { + replication_group_id = %[2]q + description = "test" + + engine = %[3]q + engine_version = %[4]q + node_type = "cache.m5.large" + num_cache_clusters = 1 +} +`, rName, primaryReplicationGroupId, repGroupEngine, repGroupEngineVersion, globalEngine, globalEngineVersion, globalParamGroup) +} + +func testAccGlobalReplicationGroupConfig_Valkey_inheritEngine_secondaryReplicationGroup( + rName, + primaryReplicationGroupId, + secondaryReplicationGroupId string, +) string { + return acctest.ConfigCompose(acctest.ConfigMultipleRegionProvider(2), fmt.Sprintf(` +resource "aws_elasticache_global_replication_group" "test" { + global_replication_group_id_suffix = %[1]q + primary_replication_group_id = aws_elasticache_replication_group.test.id +} + +resource "aws_elasticache_replication_group" "test" { + replication_group_id = %[2]q + description = "test" + + engine = "valkey" + engine_version = "8.0" + node_type = "cache.m5.large" + num_cache_clusters = 1 +} + +resource "aws_elasticache_replication_group" "secondary" { + provider = awsalternate + + replication_group_id = %[3]q + description = "test secondary" + global_replication_group_id = aws_elasticache_global_replication_group.test.id +} +`, rName, primaryReplicationGroupId, secondaryReplicationGroupId)) +} + func testAccGlobalReplicationGroupConfig_engineVersionCustomParam(rName, primaryReplicationGroupId, repGroupEngineVersion, globalEngineVersion, parameterGroupName, parameterGroupFamily string) string { return fmt.Sprintf(` resource "aws_elasticache_global_replication_group" "test" { @@ -2183,10 +2405,6 @@ resource "aws_elasticache_replication_group" "test" { engine_version = %[3]q node_type = "cache.m5.large" num_cache_clusters = 1 - - lifecycle { - ignore_changes = [engine_version] - } } resource "aws_elasticache_parameter_group" "test" { @@ -2214,10 +2432,6 @@ resource "aws_elasticache_replication_group" "test" { engine_version = %[3]q node_type = "cache.m5.large" num_cache_clusters = 1 - - lifecycle { - ignore_changes = [engine_version] - } } `, rName, primaryReplicationGroupId, repGroupEngineVersion, parameterGroup) } diff --git a/internal/service/elasticache/replication_group.go b/internal/service/elasticache/replication_group.go index 278d993dc594..3368cbc90c6f 100644 --- a/internal/service/elasticache/replication_group.go +++ b/internal/service/elasticache/replication_group.go @@ -124,9 +124,9 @@ func resourceReplicationGroup() *schema.Resource { ValidateFunc: validation.StringIsNotEmpty, }, names.AttrEngine: { - Type: schema.TypeString, - Optional: true, - Default: engineRedis, + Type: schema.TypeString, + Optional: true, + Computed: true, ValidateDiagFunc: validation.AllDiag( validation.ToDiagFunc(validation.StringInSlice([]string{engineRedis, engineValkey}, true)), // While the existing validator makes it technically possible to provide an @@ -136,6 +136,7 @@ func resourceReplicationGroup() *schema.Resource { // practitioners that stricter validation will be enforced in v7.0.0. verify.CaseInsensitiveMatchDeprecation([]string{engineRedis, engineValkey}), ), + DiffSuppressFunc: suppressDiffIfBelongsToGlobalReplicationGroup, }, names.AttrEngineVersion: { Type: schema.TypeString, @@ -145,6 +146,7 @@ func resourceReplicationGroup() *schema.Resource { validRedisVersionString, validValkeyVersionString, ), + DiffSuppressFunc: suppressDiffIfBelongsToGlobalReplicationGroup, }, "engine_version_actual": { Type: schema.TypeString, @@ -266,7 +268,7 @@ func resourceReplicationGroup() *schema.Resource { Optional: true, Computed: true, DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - return strings.HasPrefix(old, "global-datastore-") + return suppressDiffIfBelongsToGlobalReplicationGroup(k, old, new, d) }, }, names.AttrPort: { @@ -415,15 +417,7 @@ func resourceReplicationGroup() *schema.Resource { CustomizeDiff: customdiff.All( replicationGroupValidateMultiAZAutomaticFailover, customizeDiffEngineVersionForceNewOnDowngrade, - customdiff.ForceNewIf(names.AttrEngine, func(_ context.Context, diff *schema.ResourceDiff, meta any) bool { - if !diff.HasChange(names.AttrEngine) { - return false - } - if old, new := diff.GetChange(names.AttrEngine); old.(string) == engineRedis && new.(string) == engineValkey { - return false - } - return true - }), + customizeDiffEngineForceNewOnDowngrade(), customdiff.ComputedIf("member_clusters", func(ctx context.Context, diff *schema.ResourceDiff, meta any) bool { return diff.HasChange("num_cache_clusters") || diff.HasChange("num_node_groups") || @@ -492,8 +486,14 @@ func resourceReplicationGroupCreate(ctx context.Context, d *schema.ResourceData, } input.AutomaticFailoverEnabled = aws.Bool(d.Get("automatic_failover_enabled").(bool)) input.CacheNodeType = aws.String(nodeType) - input.Engine = aws.String(d.Get(names.AttrEngine).(string)) input.TransitEncryptionEnabled = aws.Bool(d.Get("transit_encryption_enabled").(bool)) + + // backwards-compatibility; imply redis engine if empty and not part of global replication group + if e, ok := d.GetOk(names.AttrEngine); ok { + input.Engine = aws.String(e.(string)) + } else { + input.Engine = aws.String(engineRedis) + } } if v, ok := d.GetOk("ip_discovery"); ok { @@ -682,8 +682,6 @@ func resourceReplicationGroupRead(ctx context.Context, d *schema.ResourceData, m d.Set("global_replication_group_id", rgp.GlobalReplicationGroupInfo.GlobalReplicationGroupId) } - d.Set(names.AttrEngine, rgp.Engine) - switch rgp.AutomaticFailover { case awstypes.AutomaticFailoverStatusDisabled, awstypes.AutomaticFailoverStatusDisabling: d.Set("automatic_failover_enabled", false) @@ -862,7 +860,7 @@ func resourceReplicationGroupUpdate(ctx context.Context, d *schema.ResourceData, requestUpdate = true } - if old, new := d.GetChange(names.AttrEngine); old.(string) == engineRedis && new.(string) == engineValkey { + if old, new := d.GetChange(names.AttrEngine); old.(string) != new.(string) && new.(string) == engineValkey { if !d.HasChange(names.AttrEngineVersion) { return sdkdiag.AppendErrorf(diags, "must explicitly set '%s' attribute for Replication Group (%s) when updating engine to 'valkey'", names.AttrEngineVersion, d.Id()) } @@ -872,6 +870,14 @@ func resourceReplicationGroupUpdate(ctx context.Context, d *schema.ResourceData, if d.HasChange(names.AttrEngineVersion) { input.EngineVersion = aws.String(d.Get(names.AttrEngineVersion).(string)) + if input.Engine == nil { + // backwards-compatibility; imply redis engine if just given engine version + if e, ok := d.GetOk(names.AttrEngine); ok { + input.Engine = aws.String(e.(string)) + } else { + input.Engine = aws.String(engineRedis) + } + } requestUpdate = true } @@ -1500,3 +1506,8 @@ func replicationGroupValidateAutomaticFailoverNumCacheClusters(_ context.Context } return errors.New(`"num_cache_clusters": must be at least 2 if automatic_failover_enabled is true`) } + +func suppressDiffIfBelongsToGlobalReplicationGroup(k, old, new string, d *schema.ResourceData) bool { + _, has_global_replication_group := d.GetOk("global_replication_group_id") + return has_global_replication_group && !d.IsNewResource() +} diff --git a/website/docs/r/elasticache_global_replication_group.html.markdown b/website/docs/r/elasticache_global_replication_group.html.markdown index 5e2ddaa606c4..7e77665e01e1 100644 --- a/website/docs/r/elasticache_global_replication_group.html.markdown +++ b/website/docs/r/elasticache_global_replication_group.html.markdown @@ -50,8 +50,7 @@ The initial Redis version is determined by the version set on the primary replic However, once it is part of a Global Replication Group, the Global Replication Group manages the version of all member replication groups. -The member replication groups must have [`lifecycle.ignore_changes[engine_version]`](https://www.terraform.io/language/meta-arguments/lifecycle) set, -or Terraform will always return a diff. +The provider is configured to ignore changes to `engine`, `engine_version` and `parameter_group_name` inside `aws_elasticache_replication_group` resources if they belong to a global replication group. In this example, the primary replication group will be created with Redis 6.0, @@ -75,10 +74,6 @@ resource "aws_elasticache_replication_group" "primary" { node_type = "cache.m5.large" num_cache_clusters = 1 - - lifecycle { - ignore_changes = [engine_version] - } } resource "aws_elasticache_replication_group" "secondary" { @@ -89,10 +84,6 @@ resource "aws_elasticache_replication_group" "secondary" { global_replication_group_id = aws_elasticache_global_replication_group.example.global_replication_group_id num_cache_clusters = 1 - - lifecycle { - ignore_changes = [engine_version] - } } ``` @@ -107,7 +98,12 @@ This resource supports the following arguments: See AWS documentation for information on [supported node types](https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/CacheNodes.SupportedTypes.html) and [guidance on selecting node types](https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/nodes-select-size.html). When creating, by default the Global Replication Group inherits the node type of the primary replication group. -* `engine_version` - (Optional) Redis version to use for the Global Replication Group. +* `engine` - (Optional) The name of the cache engine to be used for the clusters in this global replication group. + When creating, by default the Global Replication Group inherits the engine of the primary replication group. + If an engine is specified, the Global Replication Group and all member replication groups will be upgraded to this engine. + Valid values are `redis` or `valkey`. + Default is `redis` if `engine_version` is specified. +* `engine_version` - (Optional) Engine version to use for the Global Replication Group. When creating, by default the Global Replication Group inherits the version of the primary replication group. If a version is specified, the Global Replication Group and all member replication groups will be upgraded to this version. Cannot be downgraded without replacing the Global Replication Group and all member replication groups. @@ -120,7 +116,7 @@ This resource supports the following arguments: * `global_replication_group_description` - (Optional) A user-created description for the global replication group. * `num_node_groups` - (Optional) The number of node groups (shards) on the global replication group. * `parameter_group_name` - (Optional) An ElastiCache Parameter Group to use for the Global Replication Group. - Required when upgrading a major engine version, but will be ignored if left configured after the upgrade is complete. + Required when upgrading an engine or major engine version, but will be ignored if left configured after the upgrade is complete. Specifying without a major version upgrade will fail. Note that ElastiCache creates a copy of this parameter group for each member replication group. @@ -134,7 +130,6 @@ This resource exports the following attributes in addition to the arguments abov * `at_rest_encryption_enabled` - A flag that indicate whether the encryption at rest is enabled. * `auth_token_enabled` - A flag that indicate whether AuthToken (password) is enabled. * `cluster_enabled` - Indicates whether the Global Datastore is cluster enabled. -* `engine` - The name of the cache engine to be used for the clusters in this global replication group. * `global_replication_group_id` - The full ID of the global replication group. * `global_node_groups` - Set of node groups (shards) on the global replication group. Has the values: From 40bbf29c7e3838f122929c182488fa653edfbe19 Mon Sep 17 00:00:00 2001 From: twelsh-aw Date: Thu, 15 May 2025 15:59:02 -0400 Subject: [PATCH 2/5] fix units and semgrep --- internal/service/elasticache/engine_version_test.go | 2 +- .../service/elasticache/global_replication_group.go | 2 +- .../elasticache/global_replication_group_test.go | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/service/elasticache/engine_version_test.go b/internal/service/elasticache/engine_version_test.go index 929401a7917e..cb350f585d7b 100644 --- a/internal/service/elasticache/engine_version_test.go +++ b/internal/service/elasticache/engine_version_test.go @@ -514,7 +514,7 @@ func (d *mockForceNewDiffer) Get(key string) any { } func (d *mockForceNewDiffer) GetOk(key string) (any, bool) { - return d.old, d.old != "" + return "", false } func (d *mockForceNewDiffer) HasChange(key string) bool { diff --git a/internal/service/elasticache/global_replication_group.go b/internal/service/elasticache/global_replication_group.go index 3f0546e44bbf..1c6652ae8fb9 100644 --- a/internal/service/elasticache/global_replication_group.go +++ b/internal/service/elasticache/global_replication_group.go @@ -332,7 +332,7 @@ func resourceGlobalReplicationGroupCreate(ctx context.Context, d *schema.Resourc } else { version := d.Get(names.AttrEngineVersion).(string) p := d.Get(names.AttrParameterGroupName).(string) - if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(e.(string), version, p), "engine", d.Timeout(schema.TimeoutUpdate)); err != nil { + if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(e.(string), version, p), names.AttrEngine, d.Timeout(schema.TimeoutUpdate)); err != nil { return sdkdiag.AppendFromErr(diags, err) } } diff --git a/internal/service/elasticache/global_replication_group_test.go b/internal/service/elasticache/global_replication_group_test.go index 06eb8dcceaa6..56cd2fb6cca2 100644 --- a/internal/service/elasticache/global_replication_group_test.go +++ b/internal/service/elasticache/global_replication_group_test.go @@ -1567,7 +1567,7 @@ func TestAccElastiCacheGlobalReplicationGroup_SetEngineOnCreate_ValkeyUpgrade(t Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngine, "valkey"), resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngineVersion, "8.0"), - resource.TestMatchResourceAttr(primaryReplicationGroupResourceName, "parameter_group_name", regexache.MustCompile(`^global-datastore-.+$`)), + resource.TestMatchResourceAttr(primaryReplicationGroupResourceName, names.AttrParameterGroupName, regexache.MustCompile(`^global-datastore-.+$`)), resource.TestCheckResourceAttrPair(primaryReplicationGroupResourceName, "global_replication_group_id", resourceName, "global_replication_group_id"), ), }, @@ -1615,7 +1615,7 @@ func TestAccElastiCacheGlobalReplicationGroup_SetEngineOnUpdate_ValkeyUpgrade(t Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngine, "redis"), resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngineVersion, "7.1"), - resource.TestMatchResourceAttr(primaryReplicationGroupResourceName, "parameter_group_name", regexache.MustCompile(`^global-datastore-.+$`)), + resource.TestMatchResourceAttr(primaryReplicationGroupResourceName, names.AttrParameterGroupName, regexache.MustCompile(`^global-datastore-.+$`)), resource.TestCheckResourceAttrPair(primaryReplicationGroupResourceName, "global_replication_group_id", resourceName, "global_replication_group_id"), ), }, @@ -1640,7 +1640,7 @@ func TestAccElastiCacheGlobalReplicationGroup_SetEngineOnUpdate_ValkeyUpgrade(t Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngine, "valkey"), resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngineVersion, "7.2"), - resource.TestMatchResourceAttr(primaryReplicationGroupResourceName, "parameter_group_name", regexache.MustCompile(`^global-datastore-.+$`)), + resource.TestMatchResourceAttr(primaryReplicationGroupResourceName, names.AttrParameterGroupName, regexache.MustCompile(`^global-datastore-.+$`)), resource.TestCheckResourceAttrPair(primaryReplicationGroupResourceName, "global_replication_group_id", resourceName, "global_replication_group_id"), ), }, @@ -1687,12 +1687,12 @@ func TestAccElastiCacheGlobalReplicationGroup_InheritValkeyEngine_SecondaryRepli Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngine, "valkey"), resource.TestCheckResourceAttr(primaryReplicationGroupResourceName, names.AttrEngineVersion, "8.0"), - resource.TestMatchResourceAttr(primaryReplicationGroupResourceName, "parameter_group_name", regexache.MustCompile(`^global-datastore-.+$`)), + resource.TestMatchResourceAttr(primaryReplicationGroupResourceName, names.AttrParameterGroupName, regexache.MustCompile(`^global-datastore-.+$`)), resource.TestCheckResourceAttrPair(primaryReplicationGroupResourceName, "global_replication_group_id", resourceName, "global_replication_group_id"), resource.TestCheckResourceAttr(secondaryReplicationGroupResourceName, names.AttrEngine, "valkey"), resource.TestCheckResourceAttr(secondaryReplicationGroupResourceName, names.AttrEngineVersion, "8.0"), - resource.TestMatchResourceAttr(secondaryReplicationGroupResourceName, "parameter_group_name", regexache.MustCompile(`^global-datastore-.+$`)), + resource.TestMatchResourceAttr(secondaryReplicationGroupResourceName, names.AttrParameterGroupName, regexache.MustCompile(`^global-datastore-.+$`)), resource.TestCheckResourceAttrPair(secondaryReplicationGroupResourceName, "global_replication_group_id", resourceName, "global_replication_group_id"), ), }, From c8439372e4084b1b4412ceec969b2724a9f0dd4b Mon Sep 17 00:00:00 2001 From: twelsh-aw Date: Thu, 15 May 2025 18:04:58 -0400 Subject: [PATCH 3/5] Create 42636.txt --- .changelog/42636.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changelog/42636.txt diff --git a/.changelog/42636.txt b/.changelog/42636.txt new file mode 100644 index 000000000000..79410a4790ad --- /dev/null +++ b/.changelog/42636.txt @@ -0,0 +1,7 @@ +```release-note:enhancement +resource/aws_elasticache_global_replication_group: Support `engine` attribute for Valkey +``` + +```release-note:bug +resource/aws_elasticache_replication_group: Correctly set Valkey engine on secondary replication groups using global datastores +``` From edf46a18fd522e50d6fc0547aa706063f2e3b30e Mon Sep 17 00:00:00 2001 From: twelsh-aw Date: Thu, 15 May 2025 18:25:09 -0400 Subject: [PATCH 4/5] Add back ImportStateVerifyIgnore This is redis6.x specific. It has nothing to do with my DiffSuppressFunc changes or removing the lifecycle { ignore_changes } blocks. Still needed to pass tests :) ``` Step 2/2 error running import: ImportStateVerify attributes not equivalent. Difference is shown below. The - symbol indicates attributes missing after import. map[string]string{ - "engine_version": "6.x", + "engine_version": "6.2", } ``` --- .../service/elasticache/global_replication_group_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/service/elasticache/global_replication_group_test.go b/internal/service/elasticache/global_replication_group_test.go index 56cd2fb6cca2..9a557ae1c01e 100644 --- a/internal/service/elasticache/global_replication_group_test.go +++ b/internal/service/elasticache/global_replication_group_test.go @@ -926,9 +926,10 @@ func TestAccElastiCacheGlobalReplicationGroup_SetEngineVersionOnCreate_NoChange_ ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{names.AttrEngineVersion}, }, }, }) From 31697b95d4eedf8c1280eb83f2fe2d0019dad876 Mon Sep 17 00:00:00 2001 From: twelsh-aw Date: Fri, 4 Jul 2025 13:38:05 -0400 Subject: [PATCH 5/5] Fix lint --- internal/service/elasticache/replication_group.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/service/elasticache/replication_group.go b/internal/service/elasticache/replication_group.go index 3368cbc90c6f..274b23daaeb1 100644 --- a/internal/service/elasticache/replication_group.go +++ b/internal/service/elasticache/replication_group.go @@ -124,9 +124,9 @@ func resourceReplicationGroup() *schema.Resource { ValidateFunc: validation.StringIsNotEmpty, }, names.AttrEngine: { - Type: schema.TypeString, - Optional: true, - Computed: true, + Type: schema.TypeString, + Optional: true, + Computed: true, ValidateDiagFunc: validation.AllDiag( validation.ToDiagFunc(validation.StringInSlice([]string{engineRedis, engineValkey}, true)), // While the existing validator makes it technically possible to provide an