Skip to content

Commit

Permalink
r/aws_elasticache_cluster: fix transit_encryption_enabled for redis (#…
Browse files Browse the repository at this point in the history
…33451)

* r/aws_elasticache_cluster: fix transit_encryption_enabled for redis

- Removes the default `false` value and makes the argument optional and computed. For `redis` engine types the value will effectively always be computed.
- Removes the `CustomizeDiff` function focused on the `transit_encryption_enabled` attribute, instead relying on the native AWS errors to provide the same feedback.

1. AWS error for minimum supported memcached version check:
```
Error: creating ElastiCache Cache Cluster (jb-test-memcached): InvalidParameterCombination: Encryption features are not supported for engine version 1.6.6. Please use engine version 1.6.12
```
2. AWS error when attempting to create a `redis` cluster with `transit_encryption_enabled` set (must be set on replication group instead):
```
Error: creating ElastiCache Cache Cluster (jb-test-redis): InvalidParameterCombination: Encryption feature is not supported for engine REDIS.
```

* chore: changelog
  • Loading branch information
jar-b committed Sep 14, 2023
1 parent cb8c8ea commit cc0edc9
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 35 deletions.
3 changes: 3 additions & 0 deletions .changelog/33451.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_elasticache_cluster: Fix regression for `redis` engine types caused by the new `transit_encryption_enabled` argument
```
3 changes: 1 addition & 2 deletions internal/service/elasticache/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ func ResourceCluster() *schema.Resource {
Type: schema.TypeBool,
ForceNew: true,
Optional: true,
Default: false,
Computed: true,
},
names.AttrTags: tftags.TagsSchema(),
names.AttrTagsAll: tftags.TagsSchemaComputed(),
Expand All @@ -343,7 +343,6 @@ func ResourceCluster() *schema.Resource {
CustomizeDiffValidateClusterNumCacheNodes,
CustomizeDiffClusterMemcachedNodeType,
CustomizeDiffValidateClusterMemcachedSnapshotIdentifier,
CustomizeDiffValidateTransitEncryptionEnabled,
verify.SetTagsDiff,
),
}
Expand Down
64 changes: 59 additions & 5 deletions internal/service/elasticache/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,37 @@ func TestAccElastiCacheCluster_ReplicationGroupID_availabilityZone(t *testing.T)
})
}

func TestAccElastiCacheCluster_ReplicationGroupID_transitEncryption(t *testing.T) {
ctx := acctest.Context(t)
if testing.Short() {
t.Skip("skipping long-running test in short mode")
}

var cluster elasticache.CacheCluster
var replicationGroup elasticache.ReplicationGroup
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
clusterResourceName := "aws_elasticache_cluster.test"
replicationGroupResourceName := "aws_elasticache_replication_group.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, elasticache.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckClusterDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccClusterConfig_replicationGroupIDTransitEncryption(rName, true),
Check: resource.ComposeTestCheckFunc(
testAccCheckReplicationGroupExists(ctx, replicationGroupResourceName, &replicationGroup),
testAccCheckClusterExists(ctx, clusterResourceName, &cluster),
testAccCheckClusterReplicationGroupIDAttribute(&cluster, &replicationGroup),
resource.TestCheckResourceAttr(clusterResourceName, "transit_encryption_enabled", "true"),
),
},
},
})
}

func TestAccElastiCacheCluster_ReplicationGroupID_singleReplica(t *testing.T) {
ctx := acctest.Context(t)
if testing.Short() {
Expand Down Expand Up @@ -1244,12 +1275,12 @@ func TestAccElastiCacheCluster_TransitEncryption(t *testing.T) {
CheckDestroy: testAccCheckClusterDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccClusterConfig_transitEncryption(rName, "memcached", "1.6.11"),
ExpectError: regexache.MustCompile(`Transit encryption is not supported for memcached version 1.6.11`),
Config: testAccClusterConfig_transitEncryption(rName, "memcached", "1.6.6"),
ExpectError: regexache.MustCompile(`InvalidParameterCombination: Encryption features are not supported for engine version 1.6.6. Please use engine version 1.6.12`),
},
{
Config: testAccClusterConfig_transitEncryption(rName, "redis", "6.2"),
ExpectError: regexache.MustCompile(`aws_elasticache_cluster does not support transit encryption using the redis engine, use aws_elasticache_replication_group instead`),
ExpectError: regexache.MustCompile(`InvalidParameterCombination: Encryption feature is not supported for engine REDIS.`),
},
{
Config: testAccClusterConfig_transitEncryption(rName, "memcached", "1.6.12"),
Expand Down Expand Up @@ -1955,12 +1986,35 @@ resource "aws_elasticache_replication_group" "test" {
resource "aws_elasticache_cluster" "test" {
availability_zone = data.aws_availability_zones.available.names[0]
cluster_id = "%[1]s1"
cluster_id = "%[1]s-1"
replication_group_id = aws_elasticache_replication_group.test.id
}
`, rName))
}

func testAccClusterConfig_replicationGroupIDTransitEncryption(rName string, enabled bool) string {
return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptIn(), fmt.Sprintf(`
resource "aws_elasticache_replication_group" "test" {
description = "Terraform Acceptance Testing"
replication_group_id = %[1]q
node_type = "cache.t3.medium"
num_cache_clusters = 1
port = 6379
transit_encryption_enabled = %[2]t
lifecycle {
ignore_changes = [num_cache_clusters]
}
}
resource "aws_elasticache_cluster" "test" {
availability_zone = data.aws_availability_zones.available.names[0]
cluster_id = "%[1]s-1"
replication_group_id = aws_elasticache_replication_group.test.id
}
`, rName, enabled))
}

func testAccClusterConfig_replicationGroupIDReplica(rName string, count int) string {
return fmt.Sprintf(`
resource "aws_elasticache_replication_group" "test" {
Expand All @@ -1977,7 +2031,7 @@ resource "aws_elasticache_replication_group" "test" {
resource "aws_elasticache_cluster" "test" {
count = %[2]d
cluster_id = "%[1]s${count.index}"
cluster_id = "%[1]s-${count.index}"
replication_group_id = aws_elasticache_replication_group.test.id
}
`, rName, count)
Expand Down
28 changes: 0 additions & 28 deletions internal/service/elasticache/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,11 @@ package elasticache
import (
"context"
"errors"
"fmt"

"github.com/aws/aws-sdk-go/service/elasticache"
gversion "github.com/hashicorp/go-version"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

var minMemcachedTransitEncryptionVersion = gversion.Must(gversion.NewVersion("1.6.12"))

// 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 Down Expand Up @@ -73,27 +69,3 @@ func CustomizeDiffValidateReplicationGroupAutomaticFailover(_ context.Context, d
}
return nil
}

// CustomizeDiffValidateTransitEncryptionEnabled validates that an appropriate engine type and version
// are utilized when in-transit encryption is enabled
func CustomizeDiffValidateTransitEncryptionEnabled(_ context.Context, diff *schema.ResourceDiff, _ interface{}) error {
if v, ok := diff.GetOk("transit_encryption_enabled"); ok && v.(bool) {
if engine := diff.Get("engine").(string); engine == engineRedis {
return errors.New("aws_elasticache_cluster does not support transit encryption using the redis engine, use aws_elasticache_replication_group instead")
}

engineVersion, ok := diff.GetOk("engine_version")
if !ok {
return nil
}
version, err := normalizeEngineVersion(engineVersion.(string))
if err != nil {
return err
}
if version.LessThan(minMemcachedTransitEncryptionVersion) {
return fmt.Errorf("Transit encryption is not supported for memcached version %v", version)
}
}

return nil
}

0 comments on commit cc0edc9

Please sign in to comment.