Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

r/aws_elasticache_cluster: fix transit_encryption_enabled for redis #33451

Merged
merged 2 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}
Loading