Fix ElastiCache Redis versioning

.changelog/23734.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+resource/aws_elasticache_replication_group: Update regex pattern to target specific Redis V6 versions through the `engine_version` attribute
+```
+
+```release-note:bug
+resource/aws_elasticache_cluster: Update regex pattern to target specific Redis V6 versions through the `engine_version` attribute
+```

internal/service/elasticache/cluster.go

@@ -305,7 +305,7 @@ func ResourceCluster() *schema.Resource {
 		CustomizeDiff: customdiff.Sequence(
 			CustomizeDiffValidateClusterAZMode,
 			CustomizeDiffValidateClusterEngineVersion,
-			CustomizeDiffEngineVersion,
+			customizeDiffEngineVersionForceNewOnDowngrade,
 			CustomizeDiffValidateClusterNumCacheNodes,
 			CustomizeDiffClusterMemcachedNodeType,
 			CustomizeDiffValidateClusterMemcachedSnapshotIdentifier,
@@ -565,7 +565,13 @@ func setEngineVersionFromCacheCluster(d *schema.ResourceData, c *elasticache.Cac
 	if engineVersion.Segments()[0] < 6 {
 		d.Set("engine_version", engineVersion.String())
 	} else {
-		d.Set("engine_version", fmt.Sprintf("%d.x", engineVersion.Segments()[0]))
+		// Handle major-only version number
+		configVersion := d.Get("engine_version").(string)
+		if t, _ := regexp.MatchString(`[6-9]\.x`, configVersion); t {
+			d.Set("engine_version", fmt.Sprintf("%d.x", engineVersion.Segments()[0]))
+		} else {
+			d.Set("engine_version", fmt.Sprintf("%d.%d", engineVersion.Segments()[0], engineVersion.Segments()[1]))
+		}
 	}
 	d.Set("engine_version_actual", engineVersion.String())
 

internal/service/elasticache/cluster_test.go

@@ -599,7 +599,7 @@ func TestAccElastiCacheCluster_EngineVersion_redis(t *testing.T) {
 		t.Skip("skipping long-running test in short mode")
 	}
 
-	var v1, v2, v3, v4, v5 elasticache.CacheCluster
+	var v1, v2, v3, v4, v5, v6, v7, v8 elasticache.CacheCluster
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	resourceName := "aws_elasticache_cluster.test"
 
@@ -636,23 +636,50 @@ func TestAccElastiCacheCluster_EngineVersion_redis(t *testing.T) {
 				),
 			},
 			{
-				Config: testAccClusterConfig_EngineVersion_Redis(rName, "6.x"),
+				Config: testAccClusterConfig_EngineVersion_Redis(rName, "6.0"),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckClusterExists(resourceName, &v4),
 					testAccCheckClusterNotRecreated(&v3, &v4),
-					resource.TestCheckResourceAttr(resourceName, "engine_version", "6.x"),
-					resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexp.MustCompile(`^6\.[[:digit:]]+\.[[:digit:]]+$`)),
+					resource.TestCheckResourceAttr(resourceName, "engine_version", "6.0"),
+					resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexp.MustCompile(`^6\.0\.[[:digit:]]+$`)),
 				),
 			},
 			{
-				Config: testAccClusterConfig_EngineVersion_Redis(rName, "5.0.6"),
+				Config: testAccClusterConfig_EngineVersion_Redis(rName, "6.2"),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckClusterExists(resourceName, &v5),
-					testAccCheckClusterRecreated(&v4, &v5),
+					testAccCheckClusterNotRecreated(&v4, &v5),
+					resource.TestCheckResourceAttr(resourceName, "engine_version", "6.2"),
+					resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexp.MustCompile(`^6\.2\.[[:digit:]]+$`)),
+				),
+			},
+			{
+				Config: testAccClusterConfig_EngineVersion_Redis(rName, "5.0.6"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckClusterExists(resourceName, &v6),
+					testAccCheckClusterRecreated(&v5, &v6),
 					resource.TestCheckResourceAttr(resourceName, "engine_version", "5.0.6"),
 					resource.TestCheckResourceAttr(resourceName, "engine_version_actual", "5.0.6"),
 				),
 			},
+			{
+				Config: testAccClusterConfig_EngineVersion_Redis(rName, "6.x"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckClusterExists(resourceName, &v7),
+					testAccCheckClusterNotRecreated(&v6, &v7),
+					resource.TestCheckResourceAttr(resourceName, "engine_version", "6.x"),
+					resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexp.MustCompile(`^6\.[[:digit:]]+\.[[:digit:]]+$`)),
+				),
+			},
+			{
+				Config: testAccClusterConfig_EngineVersion_Redis(rName, "6.0"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckClusterExists(resourceName, &v8),
+					testAccCheckClusterRecreated(&v7, &v8),
+					resource.TestCheckResourceAttr(resourceName, "engine_version", "6.0"),
+					resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexp.MustCompile(`^6\.0\.[[:digit:]]+$`)),
+				),
+			},
 		},
 	})
 }
@@ -1394,7 +1421,7 @@ resource "aws_elasticache_cluster" "test" {
 func testAccClusterConfig_snapshots(rName string) string {
 	return fmt.Sprintf(`
 resource "aws_elasticache_cluster" "test" {
-  cluster_id               = "tf-%s"
+  cluster_id               = %[1]q
   engine                   = "redis"
   node_type                = "cache.t3.small"
   num_cache_nodes          = 1
@@ -1408,7 +1435,7 @@ resource "aws_elasticache_cluster" "test" {
 func testAccClusterConfig_snapshotsUpdated(rName string) string {
 	return fmt.Sprintf(`
 resource "aws_elasticache_cluster" "test" {
-  cluster_id               = "tf-%s"
+  cluster_id               = %[1]q
   engine                   = "redis"
   node_type                = "cache.t3.small"
   num_cache_nodes          = 1
@@ -1812,7 +1839,7 @@ func testAccClusterConfig_Redis_AutoMinorVersionUpgrade(rName string, enable boo
 resource "aws_elasticache_cluster" "test" {
   cluster_id      = %[1]q
   engine          = "redis"
-  engine_version  = "6.x"
+  engine_version  = "6.0"
   node_type       = "cache.t3.small"
   num_cache_nodes = 1
 

internal/service/elasticache/diff.go

@@ -3,47 +3,11 @@ package elasticache
 import (
 	"context"
 	"errors"
-	"fmt"
 
 	"github.com/aws/aws-sdk-go/service/elasticache"
-	multierror "github.com/hashicorp/go-multierror"
-	gversion "github.com/hashicorp/go-version"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
-// NormalizeEngineVersion returns a github.com/hashicorp/go-version Version
-// that can handle a regular 1.2.3 version number or a 6.x version number used for
-// ElastiCache Redis version 6 and higher
-func NormalizeEngineVersion(version string) (*gversion.Version, error) {
-	if matches := redisVersionPostV6Regexp.FindStringSubmatch(version); matches != nil {
-		version = matches[1]
-	}
-	return gversion.NewVersion(version)
-}
-
-// CustomizeDiffEngineVersion causes re-creation of the resource if the version is being downgraded
-func CustomizeDiffEngineVersion(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
-	if diff.Id() == "" || !diff.HasChange("engine_version") {
-		return nil
-	}
-
-	o, n := diff.GetChange("engine_version")
-	oVersion, err := NormalizeEngineVersion(o.(string))
-	if err != nil {
-		return fmt.Errorf("error parsing old engine_version: %w", err)
-	}
-	nVersion, err := NormalizeEngineVersion(n.(string))
-	if err != nil {
-		return fmt.Errorf("error parsing new engine_version: %w", err)
-	}
-
-	if nVersion.GreaterThan(oVersion) {
-		return nil
-	}
-
-	return diff.ForceNew("engine_version")
-}
-
 // CustomizeDiffValidateClusterAZMode validates that `num_cache_nodes` is greater than 1 when `az_mode` is "cross-az"
 func CustomizeDiffValidateClusterAZMode(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
 	if v, ok := diff.GetOk("az_mode"); !ok || v.(string) != elasticache.AZModeCrossAz {
@@ -56,29 +20,6 @@ func CustomizeDiffValidateClusterAZMode(_ context.Context, diff *schema.Resource
 	return errors.New(`az_mode "cross-az" is not supported with num_cache_nodes = 1`)
 }
 
-// CustomizeDiffValidateClusterEngineVersion validates the correct format for `engine_version`, based on `engine`
-func CustomizeDiffValidateClusterEngineVersion(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
-	// Memcached: Versions in format <major>.<minor>.<bug fix>
-	// Redis: Starting with version 6, must match <major>.x, prior to version 6, <major>.<minor>.<bug fix>
-	engineVersion, ok := diff.GetOk("engine_version")
-	if !ok {
-		return nil
-	}
-
-	var validator schema.SchemaValidateFunc
-	if v, ok := diff.GetOk("engine"); !ok || v.(string) == engineMemcached {
-		validator = validVersionString
-	} else {
-		validator = ValidRedisVersionString
-	}
-
-	_, errs := validator(engineVersion, "engine_version")
-
-	var err *multierror.Error
-	err = multierror.Append(err, errs...)
-	return err.ErrorOrNil()
-}
-
 // CustomizeDiffValidateClusterNumCacheNodes validates that `num_cache_nodes` is 1 when `engine` is "redis"
 func CustomizeDiffValidateClusterNumCacheNodes(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
 	if v, ok := diff.GetOk("engine"); !ok || v.(string) == engineMemcached {

internal/service/elasticache/engine_version.go

@@ -0,0 +1,132 @@
+package elasticache
+
+import (
+	"context"
+	"fmt"
+	"math"
+	"regexp"
+
+	multierror "github.com/hashicorp/go-multierror"
+	gversion "github.com/hashicorp/go-version"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+)
+
+const (
+	versionStringRegexpInternalPattern = `[[:digit:]]+(\.[[:digit:]]+){2}`
+	versionStringRegexpPattern         = "^" + versionStringRegexpInternalPattern + "$"
+)
+
+var versionStringRegexp = regexp.MustCompile(versionStringRegexpPattern)
+
+func validMemcachedVersionString(v interface{}, k string) (ws []string, errors []error) {
+	value := v.(string)
+
+	if !versionStringRegexp.MatchString(value) {
+		errors = append(errors, fmt.Errorf("%s: must be a version string matching <major>.<minor>.<patch>", k))
+	}
+
+	return
+}
+
+const (
+	redisVersionPreV6RegexpRaw  = `[1-5](\.[[:digit:]]+){2}`
+	redisVersionPostV6RegexpRaw = `(([6-9])\.x)|([6-9]\.[[:digit:]]+)`
+
+	redisVersionRegexpRaw = redisVersionPreV6RegexpRaw + "|" + redisVersionPostV6RegexpRaw
+)
+
+const (
+	redisVersionRegexpPattern       = "^" + redisVersionRegexpRaw + "$"
+	redisVersionPostV6RegexpPattern = "^" + redisVersionPostV6RegexpRaw + "$"
+)
+
+var (
+	redisVersionRegexp       = regexp.MustCompile(redisVersionRegexpPattern)
+	redisVersionPostV6Regexp = regexp.MustCompile(redisVersionPostV6RegexpPattern)
+)
+
+func validRedisVersionString(v interface{}, k string) (ws []string, errors []error) {
+	value := v.(string)
+
+	if !redisVersionRegexp.MatchString(value) {
+		errors = append(errors, fmt.Errorf("%s: Redis versions must match <major>.<minor> when using version 6 or higher, or <major>.<minor>.<patch>", k))
+	}
+
+	return
+}
+
+// CustomizeDiffValidateClusterEngineVersion validates the correct format for `engine_version`, based on `engine`
+func CustomizeDiffValidateClusterEngineVersion(_ context.Context, diff *schema.ResourceDiff, _ interface{}) error {
+	engineVersion, ok := diff.GetOk("engine_version")
+	if !ok {
+		return nil
+	}
+
+	return validateClusterEngineVersion(diff.Get("engine").(string), engineVersion.(string))
+}
+
+// validateClusterEngineVersion validates the correct format for `engine_version`, based on `engine`
+func validateClusterEngineVersion(engine, engineVersion string) error {
+	// Memcached: Versions in format <major>.<minor>.<patch>
+	// Redis: Starting with version 6, must match <major>.<minor>, prior to version 6, <major>.<minor>.<patch>
+	var validator schema.SchemaValidateFunc
+	if engine == "" || engine == engineMemcached {
+		validator = validMemcachedVersionString
+	} else {
+		validator = validRedisVersionString
+	}
+
+	_, errs := validator(engineVersion, "engine_version")
+
+	var err *multierror.Error
+	err = multierror.Append(err, errs...)
+	return err.ErrorOrNil()
+}
+
+// customizeDiffEngineVersionForceNewOnDowngrade causes re-creation of the resource if the version is being downgraded
+func customizeDiffEngineVersionForceNewOnDowngrade(_ context.Context, diff *schema.ResourceDiff, _ interface{}) error {
+	return engineVersionForceNewOnDowngrade(diff)
+}
+
+type forceNewDiffer interface {
+	Id() string
+	HasChange(key string) bool
+	GetChange(key string) (interface{}, interface{})
+	ForceNew(key string) error
+}
+
+func engineVersionForceNewOnDowngrade(diff forceNewDiffer) error {
+	if diff.Id() == "" || !diff.HasChange("engine_version") {
+		return nil
+	}
+
+	o, n := diff.GetChange("engine_version")
+	oVersion, err := normalizeEngineVersion(o.(string))
+	if err != nil {
+		return fmt.Errorf("error parsing old engine_version: %w", err)
+	}
+	nVersion, err := normalizeEngineVersion(n.(string))
+	if err != nil {
+		return fmt.Errorf("error parsing new engine_version: %w", err)
+	}
+
+	if nVersion.GreaterThan(oVersion) || nVersion.Equal(oVersion) {
+		return nil
+	}
+
+	return diff.ForceNew("engine_version")
+}
+
+// normalizeEngineVersion returns a github.com/hashicorp/go-version Version
+// that can handle a regular 1.2.3 version number or either the  6.x or 6.0 version number used for
+// ElastiCache Redis version 6 and higher. 6.x will sort to 6.<maxint>
+func normalizeEngineVersion(version string) (*gversion.Version, error) {
+	if matches := redisVersionPostV6Regexp.FindStringSubmatch(version); matches != nil {
+		if matches[1] != "" {
+			version = fmt.Sprintf("%s.%d", matches[2], math.MaxInt)
+		} else if matches[3] != "" {
+			version = matches[3]
+		}
+	}
+	return gversion.NewVersion(version)
+}