resource/aws_elasticache_replication_group: Validation fixes

.changelog/38396.txt

@@ -0,0 +1,15 @@
+```release-note:bug
+resource/aws_elasticache_replication_group: Requires `description`.
+```
+
+```release-note:bug
+resource/aws_elasticache_replication_group: `num_cache_clusters` must be at least 2 when `automatic_failover_enabled` is `true`.
+```
+
+```release-note:bug
+resource/aws_elasticache_replication_group: When `num_cache_clusters` is set, prevents setting `replicas_per_node_group`.
+```
+
+```release-note:bug
+resource/aws_elasticache_replication_group: Allows setting `replicas_per_node_group` to `0` and sets the maximum to `5`.
+```

internal/service/elasticache/cluster.go

@@ -335,12 +335,12 @@ func resourceCluster() *schema.Resource {
 		},
 
 		CustomizeDiff: customdiff.Sequence(
-			customizeDiffValidateClusterAZMode,
+			clusterValidateAZMode,
 			customizeDiffValidateClusterEngineVersion,
 			customizeDiffEngineVersionForceNewOnDowngrade,
-			customizeDiffValidateClusterNumCacheNodes,
-			customizeDiffClusterMemcachedNodeType,
-			customizeDiffValidateClusterMemcachedSnapshotIdentifier,
+			clusterValidateNumCacheNodes,
+			clusterForceNewOnMemcachedNodeTypeChange,
+			clusterValidateMemcachedSnapshotIdentifier,
 			verify.SetTagsDiff,
 		),
 	}
@@ -996,3 +996,49 @@ func setFromCacheCluster(d *schema.ResourceData, c *awstypes.CacheCluster) error
 
 	return nil
 }
+
+// clusterValidateAZMode validates that `num_cache_nodes` is greater than 1 when `az_mode` is "cross-az"
+func clusterValidateAZMode(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
+	if v, ok := diff.GetOk("az_mode"); !ok || awstypes.AZMode(v.(string)) != awstypes.AZModeCrossAz {
+		return nil
+	}
+	if v, ok := diff.GetOk("num_cache_nodes"); !ok || v.(int) != 1 {
+		return nil
+	}
+	return errors.New(`az_mode "cross-az" is not supported with num_cache_nodes = 1`)
+}
+
+// clusterValidateNumCacheNodes validates that `num_cache_nodes` is 1 when `engine` is "redis"
+func clusterValidateNumCacheNodes(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
+	if v, ok := diff.GetOk(names.AttrEngine); !ok || v.(string) == engineMemcached {
+		return nil
+	}
+	if v, ok := diff.GetOk("num_cache_nodes"); !ok || v.(int) == 1 {
+		return nil
+	}
+	return errors.New(`engine "redis" does not support num_cache_nodes > 1`)
+}
+
+// clusterForceNewOnMemcachedNodeTypeChange causes re-creation when `node_type` is changed and `engine` is "memcached"
+func clusterForceNewOnMemcachedNodeTypeChange(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
+	// Engine memcached does not currently support vertical scaling
+	// https://docs.aws.amazon.com/AmazonElastiCache/latest/mem-ug/Scaling.html#Scaling.Memcached.Vertically
+	if diff.Id() == "" || !diff.HasChange("node_type") {
+		return nil
+	}
+	if v, ok := diff.GetOk(names.AttrEngine); !ok || v.(string) == engineRedis {
+		return nil
+	}
+	return diff.ForceNew("node_type")
+}
+
+// clusterValidateMemcachedSnapshotIdentifier validates that `final_snapshot_identifier` is not set when `engine` is "memcached"
+func clusterValidateMemcachedSnapshotIdentifier(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
+	if v, ok := diff.GetOk(names.AttrEngine); !ok || v.(string) == engineRedis {
+		return nil
+	}
+	if _, ok := diff.GetOk(names.AttrFinalSnapshotIdentifier); !ok {
+		return nil
+	}
+	return errors.New(`engine "memcached" does not support final_snapshot_identifier`)
+}

internal/service/elasticache/replication_group.go

@@ -5,6 +5,7 @@ package elasticache
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log"
 	"slices"
@@ -15,6 +16,8 @@ import (
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/elasticache"
 	awstypes "github.com/aws/aws-sdk-go-v2/service/elasticache/types"
+	"github.com/hashicorp/go-cty/cty"
+	"github.com/hashicorp/go-cty/cty/gocty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
@@ -35,6 +38,10 @@ import (
 	"github.com/hashicorp/terraform-provider-aws/names"
 )
 
+const (
+	failoverMinNumCacheClusters = 2
+)
+
 // @SDKResource("aws_elasticache_replication_group", name="Replication Group")
 // @Tags(identifierAttribute="arn")
 func resourceReplicationGroup() *schema.Resource {
@@ -111,8 +118,7 @@ func resourceReplicationGroup() *schema.Resource {
 			},
 			names.AttrDescription: {
 				Type:         schema.TypeString,
-				Optional:     true,
-				Computed:     true,
+				Required:     true,
 				ValidateFunc: validation.StringIsNotEmpty,
 			},
 			names.AttrEngine: {
@@ -235,7 +241,7 @@ func resourceReplicationGroup() *schema.Resource {
 				Type:          schema.TypeInt,
 				Computed:      true,
 				Optional:      true,
-				ConflictsWith: []string{"num_node_groups"},
+				ConflictsWith: []string{"num_node_groups", "replicas_per_node_group"},
 			},
 			"num_node_groups": {
 				Type:          schema.TypeInt,
@@ -277,9 +283,11 @@ func resourceReplicationGroup() *schema.Resource {
 				Computed: true,
 			},
 			"replicas_per_node_group": {
-				Type:     schema.TypeInt,
-				Optional: true,
-				Computed: true,
+				Type:          schema.TypeInt,
+				Optional:      true,
+				Computed:      true,
+				ConflictsWith: []string{"num_cache_clusters"},
+				ValidateFunc:  validation.IntBetween(0, 5),
 			},
 			"replication_group_id": {
 				Type:         schema.TypeString,
@@ -385,8 +393,8 @@ func resourceReplicationGroup() *schema.Resource {
 			Delete: schema.DefaultTimeout(45 * time.Minute),
 		},
 
-		CustomizeDiff: customdiff.Sequence(
-			customizeDiffValidateReplicationGroupAutomaticFailover,
+		CustomizeDiff: customdiff.All(
+			replicationGroupValidateMultiAZAutomaticFailover,
 			customizeDiffEngineVersionForceNewOnDowngrade,
 			customdiff.ComputedIf("member_clusters", func(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) bool {
 				return diff.HasChange("num_cache_clusters") ||
@@ -398,6 +406,7 @@ func resourceReplicationGroup() *schema.Resource {
 				// be configured during creation of the cluster.
 				return semver.LessThan(d.Get("engine_version_actual").(string), "7.0.5")
 			}),
+			replicationGroupValidateAutomaticFailoverNumCacheClusters,
 			verify.SetTagsDiff,
 		),
 	}
@@ -513,8 +522,23 @@ func resourceReplicationGroupCreate(ctx context.Context, d *schema.ResourceData,
 		input.PreferredCacheClusterAZs = flex.ExpandStringValueList(v.([]interface{}))
 	}
 
-	if v, ok := d.GetOk("replicas_per_node_group"); ok {
-		input.ReplicasPerNodeGroup = aws.Int32(int32(v.(int)))
+	rawConfig := d.GetRawConfig()
+	rawReplicasPerNodeGroup := rawConfig.GetAttr("replicas_per_node_group")
+	if rawReplicasPerNodeGroup.IsKnown() && !rawReplicasPerNodeGroup.IsNull() {
+		var v int32
+		err := gocty.FromCtyValue(rawReplicasPerNodeGroup, &v)
+		if err != nil {
+			path := cty.GetAttrPath("replicas_per_node_group")
+			diags = append(diags, errs.NewAttributeErrorDiagnostic(
+				path,
+				"Invalid Value",
+				"An unexpected error occurred while reading configuration values. "+
+					"This is always an error in the provider. "+
+					"Please report the following to the provider developer:\n\n"+
+					fmt.Sprintf(`Reading "%s": %s`, errs.PathString(path), err),
+			))
+		}
+		input.ReplicasPerNodeGroup = aws.Int32(v)
 	}
 
 	if v, ok := d.GetOk("subnet_group_name"); ok {
@@ -1387,3 +1411,29 @@ var validateReplicationGroupID schema.SchemaValidateFunc = validation.All(
 	validation.StringDoesNotMatch(regexache.MustCompile(`--`), "cannot contain two consecutive hyphens"),
 	validation.StringDoesNotMatch(regexache.MustCompile(`-$`), "cannot end with a hyphen"),
 )
+
+// replicationGroupValidateMultiAZAutomaticFailover validates that `automatic_failover_enabled` is set when `multi_az_enabled` is true
+func replicationGroupValidateMultiAZAutomaticFailover(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
+	if v := diff.Get("multi_az_enabled").(bool); !v {
+		return nil
+	}
+	if v := diff.Get("automatic_failover_enabled").(bool); !v {
+		return errors.New(`automatic_failover_enabled must be true if multi_az_enabled is true`)
+	}
+	return nil
+}
+
+// replicationGroupValidateAutomaticFailoverNumCacheClusters validates that `automatic_failover_enabled` is set when `multi_az_enabled` is true
+func replicationGroupValidateAutomaticFailoverNumCacheClusters(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
+	if v := diff.Get("automatic_failover_enabled").(bool); !v {
+		return nil
+	}
+	raw := diff.GetRawConfig().GetAttr("num_cache_clusters")
+	if !raw.IsKnown() || raw.IsNull() {
+		return nil
+	}
+	if raw.GreaterThanOrEqualTo(cty.NumberIntVal(failoverMinNumCacheClusters)).True() {
+		return nil
+	}
+	return errors.New(`"num_cache_clusters": must be at least 2 if automatic_failover_enabled is true`)
+}