diff --git a/.changelog/23734.txt b/.changelog/23734.txt new file mode 100644 index 000000000000..461af7663aed --- /dev/null +++ b/.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 +``` diff --git a/internal/service/elasticache/cluster.go b/internal/service/elasticache/cluster.go index 013f28f7b425..b256404410d6 100644 --- a/internal/service/elasticache/cluster.go +++ b/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()) diff --git a/internal/service/elasticache/cluster_test.go b/internal/service/elasticache/cluster_test.go index e4e855c439e4..fc1d7c775510 100644 --- a/internal/service/elasticache/cluster_test.go +++ b/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 diff --git a/internal/service/elasticache/diff.go b/internal/service/elasticache/diff.go index 53bd010cb95d..979ea1da74b6 100644 --- a/internal/service/elasticache/diff.go +++ b/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 .. - // Redis: Starting with version 6, must match .x, prior to version 6, .. - 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 { diff --git a/internal/service/elasticache/engine_version.go b/internal/service/elasticache/engine_version.go new file mode 100644 index 000000000000..841c6d256eea --- /dev/null +++ b/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 ..", 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 . when using version 6 or higher, or ..", 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 .. + // Redis: Starting with version 6, must match ., prior to version 6, .. + 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. +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) +} diff --git a/internal/service/elasticache/engine_version_test.go b/internal/service/elasticache/engine_version_test.go new file mode 100644 index 000000000000..740031622d85 --- /dev/null +++ b/internal/service/elasticache/engine_version_test.go @@ -0,0 +1,421 @@ +package elasticache + +import ( + "fmt" + "math" + "testing" +) + +func TestValidMemcachedVersionString(t *testing.T) { + testcases := []struct { + version string + valid bool + }{ + { + version: "1.2.3", + valid: true, + }, + { + version: "10.20.30", + valid: true, + }, + { + version: "1.2.", + valid: false, + }, + { + version: "1.2", + valid: false, + }, + { + version: "1.", + valid: false, + }, + { + version: "1", + valid: false, + }, + { + version: "1.2.x", + valid: false, + }, + { + version: "1.x", + valid: false, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.version, func(t *testing.T) { + warnings, errors := validMemcachedVersionString(testcase.version, "key") + + if l := len(warnings); l != 0 { + t.Errorf("expected no warnings, got %d", l) + } + + if testcase.valid { + if l := len(errors); l != 0 { + t.Errorf("expected no errors, got %d: %v", l, errors) + } + } else { + if l := len(errors); l == 0 { + t.Error("expected one error, got none") + } else if l > 1 { + t.Errorf("expected one error, got %d: %v", l, errors) + } + } + }) + } +} + +func TestValidRedisVersionString(t *testing.T) { + testcases := []struct { + version string + valid bool + }{ + { + version: "5.4.3", + valid: true, + }, + { + version: "5.4.", + valid: false, + }, + { + version: "5.4", + valid: false, + }, + { + version: "5.", + valid: false, + }, + { + version: "5", + valid: false, + }, + { + version: "5.4.x", + valid: false, + }, + { + version: "5.x", + valid: false, + }, + { + version: "6.x", + valid: true, + }, + { + version: "6.2", + valid: true, + }, + { + version: "6.5.0", + valid: false, + }, + { + version: "6.5.", + valid: false, + }, + { + version: "6.", + valid: false, + }, + { + version: "6", + valid: false, + }, + { + version: "6.y", + valid: false, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.version, func(t *testing.T) { + warnings, errors := validRedisVersionString(testcase.version, "key") + + if l := len(warnings); l != 0 { + t.Errorf("expected no warnings, got %d", l) + } + + if testcase.valid { + if l := len(errors); l != 0 { + t.Errorf("expected no errors, got %d: %v", l, errors) + } + } else { + if l := len(errors); l == 0 { + t.Error("expected one error, got none") + } else if l > 1 { + t.Errorf("expected one error, got %d: %v", l, errors) + } + } + }) + } +} + +func TestValidateClusterEngineVersion(t *testing.T) { + testcases := []struct { + engine string + version string + valid bool + }{ + // Empty engine value is Memcached + { + engine: "", + version: "1.2.3", + valid: true, + }, + { + engine: "", + version: "6.x", + valid: false, + }, + { + engine: "", + version: "6.0", + valid: false, + }, + + { + engine: engineMemcached, + version: "1.2.3", + valid: true, + }, + { + engine: engineMemcached, + version: "6.x", + valid: false, + }, + { + engine: engineMemcached, + version: "6.0", + valid: false, + }, + + { + engine: engineRedis, + version: "1.2.3", + valid: true, + }, + { + engine: engineRedis, + version: "6.x", + valid: true, + }, + { + engine: engineRedis, + version: "6.0", + valid: true, + }, + } + + for _, testcase := range testcases { + t.Run(fmt.Sprintf("%s %s", testcase.engine, testcase.version), func(t *testing.T) { + err := validateClusterEngineVersion(testcase.engine, testcase.version) + + if testcase.valid { + if err != nil { + t.Errorf("expected no error, got %s", err) + } + } else { + if err == nil { + t.Error("expected an error, got none") + } + } + }) + } +} + +type differ struct { + id string + old, new string + hasChange bool // force HasChange() to return true + forceNew bool +} + +func (d *differ) Id() string { + return d.id +} + +func (d *differ) HasChange(key string) bool { + return d.hasChange || d.old != d.new +} + +func (d *differ) GetChange(key string) (interface{}, interface{}) { + return d.old, d.new +} + +func (d *differ) ForceNew(key string) error { + d.forceNew = true + + return nil +} + +func TestCustomizeDiffEngineVersionForceNewOnDowngrade(t *testing.T) { + testcases := map[string]struct { + isNew bool + old, new string + hasChange bool // force HasChange() to return true + expectForceNew bool + }{ + "new resource": { + isNew: true, + expectForceNew: false, + }, + + "no change": { + old: "1.2.3", + new: "1.2.3", + expectForceNew: false, + }, + + "spurious change": { + old: "1.2.3", + new: "1.2.3", + hasChange: true, + expectForceNew: false, + }, + + "upgrade minor versions": { + old: "1.2.3", + new: "1.3.5", + expectForceNew: false, + }, + + "upgrade major versions": { + old: "1.2.3", + new: "2.4.6", + expectForceNew: false, + }, + + // "upgrade major 6.x": { + // old: "5.0.6", + // new: "6.x", + // expectForceNew: false, + // }, + + // "upgrade major 6.digit": { + // old: "5.0.6", + // new: "6.0", + // expectForceNew: false, + // }, + + "downgrade minor versions": { + old: "1.3.5", + new: "1.2.3", + expectForceNew: true, + }, + + "downgrade major versions": { + old: "2.4.6", + new: "1.2.3", + expectForceNew: true, + }, + + "downgrade from major 6.x": { + old: "6.x", + new: "5.0.6", + expectForceNew: true, + }, + + "downgrade major 6.digit": { + old: "6.2", + new: "6.0", + expectForceNew: true, + }, + + "switch major 6.digit to 6.x": { + old: "6.2", + new: "6.x", + expectForceNew: false, + }, + + "downgrade from major 7.x to 6.x": { + old: "7.x", + new: "6.x", + expectForceNew: true, + }, + + "downgrade from major 7.digit to 6.x": { + old: "7.2", + new: "6.x", + expectForceNew: true, + }, + } + + for name, testcase := range testcases { + t.Run(name, func(t *testing.T) { + diff := &differ{} + if !testcase.isNew { + diff.id = "some id" + diff.old = testcase.old + diff.new = testcase.new + } + diff.hasChange = testcase.hasChange + + err := engineVersionForceNewOnDowngrade(diff) + + if err != nil { + t.Fatalf("no error expected, got %s", err) + } + + if testcase.expectForceNew { + if !diff.forceNew { + t.Error("expected ForceNew") + } + } else { + if diff.forceNew { + t.Error("unexpected ForceNew") + } + } + }) + } +} + +func TestNormalizeEngineVersion(t *testing.T) { + testcases := []struct { + version string + normalized string + valid bool + }{ + { + version: "5.4.3", + normalized: "5.4.3", + valid: true, + }, + { + version: "6.2", + normalized: "6.2.0", + valid: true, + }, + { + version: "6.x", + normalized: fmt.Sprintf("6.%d.0", math.MaxInt), + valid: true, + }, + { + version: "5.x", + valid: false, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.version, func(t *testing.T) { + version, err := normalizeEngineVersion(testcase.version) + + if testcase.valid { + if err != nil { + t.Fatalf("expected no error, got %s", err) + } + if a, e := version.String(), testcase.normalized; a != e { + t.Errorf("expected %q, got %q", e, a) + } + } else { + if err == nil { + t.Error("expected an error, got none") + } + } + }) + } +} diff --git a/internal/service/elasticache/global_replication_group.go b/internal/service/elasticache/global_replication_group.go index b245b9bf7479..aab6e8acf3de 100644 --- a/internal/service/elasticache/global_replication_group.go +++ b/internal/service/elasticache/global_replication_group.go @@ -70,8 +70,8 @@ func ResourceGlobalReplicationGroup() *schema.Resource { }, // Leaving space for `engine_version` for creation and updating. // `engine_version` cannot be used for returning the version because, starting with Redis 6, - // version configuration is major-version-only: `engine_version = "6.x"`, while `engine_version_actual` - // will be e.g. `6.0.5` + // version configuration is major-version-only: `engine_version = "6.x"` or major-minor-version-only: `engine_version = "6.2"`, + // while `engine_version_actual` will be the full version e.g. `6.0.5` // See also https://github.com/hashicorp/terraform-provider-aws/issues/15625 "engine_version_actual": { Type: schema.TypeString, @@ -163,7 +163,7 @@ func resourceGlobalReplicationGroupCreate(d *schema.ResourceData, meta interface d.SetId(aws.StringValue(output.GlobalReplicationGroup.GlobalReplicationGroupId)) if _, err := WaitGlobalReplicationGroupAvailable(conn, d.Id(), GlobalReplicationGroupDefaultCreatedTimeout); err != nil { - return fmt.Errorf("error waiting for ElastiCache Global Replication Group (%s) availability: %w", d.Id(), err) + return fmt.Errorf("error waiting for ElastiCache Global Replication Group (%s) creation: %w", d.Id(), err) } return resourceGlobalReplicationGroupRead(d, meta) diff --git a/internal/service/elasticache/global_replication_group_test.go b/internal/service/elasticache/global_replication_group_test.go index 2bcdb8570c16..9b7e4f453715 100644 --- a/internal/service/elasticache/global_replication_group_test.go +++ b/internal/service/elasticache/global_replication_group_test.go @@ -492,7 +492,7 @@ resource "aws_elasticache_replication_group" "test" { replication_group_description = "test" engine = "redis" - engine_version = "6.x" + engine_version = "6.2" node_type = "cache.m5.large" parameter_group_name = "default.redis6.x.cluster.on" diff --git a/internal/service/elasticache/replication_group.go b/internal/service/elasticache/replication_group.go index 295a04a73d71..79b1a8e58377 100644 --- a/internal/service/elasticache/replication_group.go +++ b/internal/service/elasticache/replication_group.go @@ -136,7 +136,7 @@ func ResourceReplicationGroup() *schema.Resource { Type: schema.TypeString, Optional: true, Computed: true, - ValidateFunc: ValidRedisVersionString, + ValidateFunc: validRedisVersionString, }, "engine_version_actual": { Type: schema.TypeString, @@ -389,7 +389,7 @@ func ResourceReplicationGroup() *schema.Resource { CustomizeDiff: customdiff.Sequence( CustomizeDiffValidateReplicationGroupAutomaticFailover, - CustomizeDiffEngineVersion, + customizeDiffEngineVersionForceNewOnDowngrade, customdiff.ComputedIf("member_clusters", func(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) bool { return diff.HasChange("number_cache_clusters") || diff.HasChange("num_cache_clusters") || @@ -589,7 +589,7 @@ func resourceReplicationGroupCreate(d *schema.ResourceData, meta interface{}) er // to be fully added to the global replication group. // API calls to the global replication group can be made in any region. if _, err := WaitGlobalReplicationGroupAvailable(conn, v.(string), GlobalReplicationGroupDefaultCreatedTimeout); err != nil { - return fmt.Errorf("error waiting for ElastiCache Global Replication Group (%s) availability: %w", v, err) + return fmt.Errorf("error waiting for ElastiCache Global Replication Group (%s) to be available: %w", v, err) } } diff --git a/internal/service/elasticache/replication_group_test.go b/internal/service/elasticache/replication_group_test.go index b8f42f54c7c6..174c2e08ea81 100644 --- a/internal/service/elasticache/replication_group_test.go +++ b/internal/service/elasticache/replication_group_test.go @@ -51,7 +51,7 @@ func TestAccElastiCacheReplicationGroup_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "cluster_mode.0.num_node_groups", "1"), resource.TestCheckResourceAttr(resourceName, "cluster_mode.0.replicas_per_node_group", "0"), resource.TestCheckResourceAttr(resourceName, "cluster_enabled", "false"), - resource.TestCheckResourceAttr(resourceName, "engine_version", "6.x"), + resource.TestCheckResourceAttr(resourceName, "engine_version", "6.2"), resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexp.MustCompile(`^6\.[[:digit:]]+\.[[:digit:]]+$`)), resource.TestCheckResourceAttr(resourceName, "auto_minor_version_upgrade", "true"), resource.TestCheckResourceAttr(resourceName, "data_tiering_enabled", "false"), @@ -140,7 +140,7 @@ func TestAccElastiCacheReplicationGroup_EngineVersion_update(t *testing.T) { t.Skip("skipping long-running test in short mode") } - var v1, v2, v3, v4, v5 elasticache.ReplicationGroup + var v1, v2, v3, v4, v5, v6, v7, v8 elasticache.ReplicationGroup rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_elasticache_replication_group.test" @@ -177,23 +177,50 @@ func TestAccElastiCacheReplicationGroup_EngineVersion_update(t *testing.T) { ), }, { - Config: testAccReplicationGroupConfig_EngineVersion(rName, "6.x"), + Config: testAccReplicationGroupConfig_EngineVersion(rName, "6.0"), Check: resource.ComposeTestCheckFunc( testAccCheckReplicationGroupExists(resourceName, &v4), testAccCheckReplicationGroupNotRecreated(&v3, &v4), - resource.TestCheckResourceAttr(resourceName, "engine_version", "6.x"), + resource.TestCheckResourceAttr(resourceName, "engine_version", "6.0"), resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexp.MustCompile(`^6\.[[:digit:]]+\.[[:digit:]]+$`)), ), }, { - Config: testAccReplicationGroupConfig_EngineVersion(rName, "5.0.6"), + Config: testAccReplicationGroupConfig_EngineVersion(rName, "6.2"), Check: resource.ComposeTestCheckFunc( testAccCheckReplicationGroupExists(resourceName, &v5), - testAccCheckReplicationGroupRecreated(&v4, &v5), + testAccCheckReplicationGroupNotRecreated(&v4, &v5), + resource.TestCheckResourceAttr(resourceName, "engine_version", "6.2"), + resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexp.MustCompile(`^6\.[[:digit:]]+\.[[:digit:]]+$`)), + ), + }, + { + Config: testAccReplicationGroupConfig_EngineVersion(rName, "5.0.6"), + Check: resource.ComposeTestCheckFunc( + testAccCheckReplicationGroupExists(resourceName, &v6), + testAccCheckReplicationGroupRecreated(&v5, &v6), resource.TestCheckResourceAttr(resourceName, "engine_version", "5.0.6"), resource.TestCheckResourceAttr(resourceName, "engine_version_actual", "5.0.6"), ), }, + { + Config: testAccReplicationGroupConfig_EngineVersion(rName, "6.x"), + Check: resource.ComposeTestCheckFunc( + testAccCheckReplicationGroupExists(resourceName, &v7), + testAccCheckReplicationGroupNotRecreated(&v6, &v7), + resource.TestCheckResourceAttr(resourceName, "engine_version", "6.x"), + resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexp.MustCompile(`^6\.[[:digit:]]+\.[[:digit:]]+$`)), + ), + }, + { + Config: testAccReplicationGroupConfig_EngineVersion(rName, "6.0"), + Check: resource.ComposeTestCheckFunc( + testAccCheckReplicationGroupExists(resourceName, &v8), + testAccCheckReplicationGroupRecreated(&v7, &v8), + resource.TestCheckResourceAttr(resourceName, "engine_version", "6.0"), + resource.TestMatchResourceAttr(resourceName, "engine_version_actual", regexp.MustCompile(`^6\.0\.[[:digit:]]+$`)), + ), + }, }, }) } @@ -2374,7 +2401,7 @@ func TestAccElastiCacheReplicationGroup_dataTiering(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckReplicationGroupExists(resourceName, &rg), resource.TestCheckResourceAttr(resourceName, "engine", "redis"), - resource.TestCheckResourceAttr(resourceName, "engine_version", "6.x"), + resource.TestCheckResourceAttr(resourceName, "engine_version", "6.2"), resource.TestCheckResourceAttr(resourceName, "data_tiering_enabled", "true"), ), }, @@ -3480,7 +3507,7 @@ resource "aws_elasticache_replication_group" "primary" { subnet_group_name = aws_elasticache_subnet_group.primary.name engine = "redis" - engine_version = "6.x" + engine_version = "6.2" node_type = "cache.m5.large" parameter_group_name = "default.redis6.x.cluster.on" @@ -3531,7 +3558,7 @@ resource "aws_elasticache_replication_group" "primary" { subnet_group_name = aws_elasticache_subnet_group.primary.name engine = "redis" - engine_version = "6.x" + engine_version = "6.2" node_type = "cache.m5.large" parameter_group_name = "default.redis6.x.cluster.on" diff --git a/internal/service/elasticache/validate.go b/internal/service/elasticache/validate.go index 16e912d3f0d7..6eeb223d0906 100644 --- a/internal/service/elasticache/validate.go +++ b/internal/service/elasticache/validate.go @@ -5,13 +5,6 @@ import ( "regexp" ) -const ( - versionStringRegexpInternalPattern = `[[:digit:]]+(.[[:digit:]]+){2}` - versionStringRegexpPattern = "^" + versionStringRegexpInternalPattern + "$" -) - -var versionStringRegexp = regexp.MustCompile(versionStringRegexpPattern) - func validReplicationGroupAuthToken(v interface{}, k string) (ws []string, errors []error) { value := v.(string) if (len(value) < 16) || (len(value) > 128) { @@ -24,40 +17,3 @@ func validReplicationGroupAuthToken(v interface{}, k string) (ws []string, error } return } - -func validVersionString(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 x.y.z", k)) - } - - return -} - -const ( - redisVersionPreV6RegexpRaw = `[1-5](\.[[:digit:]]+){2}` - redisVersionPostV6RegexpRaw = `([6-9]|[[:digit:]]{2})\.x` - - 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 .x when using version 6 or higher, or ..", k)) - } - - return -} diff --git a/internal/service/elasticache/wait.go b/internal/service/elasticache/wait.go index 6eae06744bde..52e98023a5e5 100644 --- a/internal/service/elasticache/wait.go +++ b/internal/service/elasticache/wait.go @@ -150,7 +150,7 @@ func WaitCacheClusterDeleted(conn *elasticache.ElastiCache, cacheClusterID strin } const ( - GlobalReplicationGroupDefaultCreatedTimeout = 20 * time.Minute + GlobalReplicationGroupDefaultCreatedTimeout = 25 * time.Minute GlobalReplicationGroupDefaultUpdatedTimeout = 40 * time.Minute GlobalReplicationGroupDefaultDeletedTimeout = 20 * time.Minute diff --git a/website/docs/r/elasticache_cluster.html.markdown b/website/docs/r/elasticache_cluster.html.markdown index ce951dd9bdbb..9b0e9e1592c7 100644 --- a/website/docs/r/elasticache_cluster.html.markdown +++ b/website/docs/r/elasticache_cluster.html.markdown @@ -113,8 +113,11 @@ The following arguments are optional: * `az_mode` - (Optional, Memcached only) Whether the nodes in this Memcached node group are created in a single Availability Zone or created across multiple Availability Zones in the cluster's region. Valid values for this parameter are `single-az` or `cross-az`, default is `single-az`. If you want to choose `cross-az`, `num_cache_nodes` must be greater than `1`. * `engine_version` – (Optional) Version number of the cache engine to be used. If not set, defaults to the latest version. - See [Describe Cache Engine Versions](https://docs.aws.amazon.com/cli/latest/reference/elasticache/describe-cache-engine-versions.html) - in the AWS Documentation for supported versions. When `engine` is `redis` and the version is 6 or higher, only the major version can be set, e.g., `6.x`, otherwise, specify the full version desired, e.g., `5.0.6`. The actual engine version used is returned in the attribute `engine_version_actual`, , see [Attributes Reference](#attributes-reference) below. + See [Describe Cache Engine Versions](https://docs.aws.amazon.com/cli/latest/reference/elasticache/describe-cache-engine-versions.html) in the AWS Documentation for supported versions. + When `engine` is `redis` and the version is 6 or higher, the major and minor version can be set, e.g., `6.2`, + or the minor version can be unspecified which will use the latest version at creation time, e.g., `6.x`. + Otherwise, specify the full version desired, e.g., `5.0.6`. + The actual engine version used is returned in the attribute `engine_version_actual`, see [Attributes Reference](#attributes-reference) below. * `final_snapshot_identifier` - (Optional, Redis only) Name of your final cluster snapshot. If omitted, no final snapshot will be made. * `log_delivery_configuration` - (Optional, Redis only) Specifies the destination and format of Redis [SLOWLOG](https://redis.io/commands/slowlog) or Redis [Engine Log](https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/Log_Delivery.html#Log_contents-engine-log). See the documentation on [Amazon ElastiCache](https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/Log_Delivery.html). See [Log Delivery Configuration](#log-delivery-configuration) below for more details. * `maintenance_window` – (Optional) Specifies the weekly time range for when maintenance @@ -138,7 +141,7 @@ The minimum maintenance window is a 60 minute period. Example: `sun:05:00-sun:09 In addition to all arguments above, the following attributes are exported: * `arn` - The ARN of the created ElastiCache Cluster. -* `engine_version_actual` - The running version of the cache engine. +* `engine_version_actual` - Because ElastiCache pulls the latest minor or patch for a version, this attribute returns the running version of the cache engine. * `cache_nodes` - List of node objects including `id`, `address`, `port` and `availability_zone`. * `cluster_address` - (Memcached only) DNS name of the cache cluster without the port appended. * `configuration_endpoint` - (Memcached only) Configuration endpoint to allow host discovery. diff --git a/website/docs/r/elasticache_replication_group.html.markdown b/website/docs/r/elasticache_replication_group.html.markdown index 44f5e6456bea..fefa86f1ccd4 100644 --- a/website/docs/r/elasticache_replication_group.html.markdown +++ b/website/docs/r/elasticache_replication_group.html.markdown @@ -183,7 +183,11 @@ The following arguments are optional: * `cluster_mode` - (Optional, **Deprecated** use root-level `num_node_groups` and `replicas_per_node_group` instead) Create a native Redis cluster. `automatic_failover_enabled` must be set to true. Cluster Mode documented below. Only 1 `cluster_mode` block is allowed. Note that configuring this block does not enable cluster mode, i.e., data sharding, this requires using a parameter group that has the parameter `cluster-enabled` set to true. * `data_tiering_enabled` - (Optional) Enables data tiering. Data tiering is only supported for replication groups using the r6gd node type. This parameter must be set to `true` when using r6gd nodes. * `engine` - (Optional) Name of the cache engine to be used for the clusters in this replication group. The only valid value is `redis`. -* `engine_version` - (Optional) Version number of the cache engine to be used for the cache clusters in this replication group. If the version is 6 or higher, only the major version can be set, e.g., `6.x`, otherwise, specify the full version desired, e.g., `5.0.6`. The actual engine version used is returned in the attribute `engine_version_actual`, see [Attributes Reference](#attributes-reference) below. +* `engine_version` - (Optional) Version number of the cache engine to be used for the cache clusters in this replication group. + If the version is 6 or higher, the major and minor version can be set, e.g., `6.2`, + or the minor version can be unspecified which will use the latest version at creation time, e.g., `6.x`. + Otherwise, specify the full version desired, e.g., `5.0.6`. + The actual engine version used is returned in the attribute `engine_version_actual`, see [Attributes Reference](#attributes-reference) below. * `final_snapshot_identifier` - (Optional) The name of your final node group (shard) snapshot. ElastiCache creates the snapshot from the primary node in the cluster. If omitted, no final snapshot will be made. * `global_replication_group_id` - (Optional) The ID of the global replication group to which this replication group should belong. If this parameter is specified, the replication group is added to the specified global replication group as a secondary replication group; otherwise, the replication group is not part of any global replication group. If `global_replication_group_id` is set, the `num_node_groups` parameter (or the `num_node_groups` parameter of the deprecated `cluster_mode` block) cannot be set. * `kms_key_id` - (Optional) The ARN of the key that you wish to use if encrypting at rest. If not supplied, uses service managed encryption. Can be specified only if `at_rest_encryption_enabled = true`. @@ -227,7 +231,7 @@ The `log_delivery_configuration` block allows the streaming of Redis [SLOWLOG](h In addition to all arguments above, the following attributes are exported: * `arn` - ARN of the created ElastiCache Replication Group. -* `engine_version_actual` - Running version of the cache engine. +* `engine_version_actual` - Because ElastiCache pulls the latest minor or patch for a version, this attribute returns the running version of the cache engine. * `cluster_enabled` - Indicates if cluster mode is enabled. * `configuration_endpoint_address` - Address of the replication group configuration endpoint when cluster mode is enabled. * `id` - ID of the ElastiCache Replication Group.