Skip to content

Commit

Permalink
Merge pull request #23734 from bschaatsbergen/fix-redit-versioning
Browse files Browse the repository at this point in the history
Fix ElastiCache Redis versioning
  • Loading branch information
gdavison authored Apr 23, 2022
2 parents 1636563 + 63d345e commit 3370515
Show file tree
Hide file tree
Showing 14 changed files with 660 additions and 136 deletions.
7 changes: 7 additions & 0 deletions .changelog/23734.txt
Original file line number Diff line number Diff line change
@@ -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
```
10 changes: 8 additions & 2 deletions internal/service/elasticache/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ func ResourceCluster() *schema.Resource {
CustomizeDiff: customdiff.Sequence(
CustomizeDiffValidateClusterAZMode,
CustomizeDiffValidateClusterEngineVersion,
CustomizeDiffEngineVersion,
customizeDiffEngineVersionForceNewOnDowngrade,
CustomizeDiffValidateClusterNumCacheNodes,
CustomizeDiffClusterMemcachedNodeType,
CustomizeDiffValidateClusterMemcachedSnapshotIdentifier,
Expand Down Expand Up @@ -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())

Expand Down
45 changes: 36 additions & 9 deletions internal/service/elasticache/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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:]]+$`)),
),
},
},
})
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
59 changes: 0 additions & 59 deletions internal/service/elasticache/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
132 changes: 132 additions & 0 deletions internal/service/elasticache/engine_version.go
Original file line number Diff line number Diff line change
@@ -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)
}
Loading

0 comments on commit 3370515

Please sign in to comment.