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.

Author: timothy-welsh_ramp

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
 	}

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
 }

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

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,15 +359,15 @@ 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 {
 			if p != "" {
 				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)
 	}