From e0bd7b4e33a3c22754d47dc339d11364a95361fc Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 21 Apr 2021 16:51:35 -0700 Subject: [PATCH] Centralizes `aws_elasticache_cluster` and `aws_elasticache_replication_group` validation functions --- aws/elasticache_validation.go | 86 +++++++++++++++++++ aws/resource_aws_elasticache_cluster.go | 81 ++--------------- ...ource_aws_elasticache_replication_group.go | 15 +--- 3 files changed, 98 insertions(+), 84 deletions(-) diff --git a/aws/elasticache_validation.go b/aws/elasticache_validation.go index 33bff2ff964..e5acc560310 100644 --- a/aws/elasticache_validation.go +++ b/aws/elasticache_validation.go @@ -2,11 +2,15 @@ package aws import ( "context" + "errors" "fmt" "regexp" + "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" + tfelasticache "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/elasticache" ) const ( @@ -68,3 +72,85 @@ func CustomizeDiffElastiCacheEngineVersion(_ context.Context, diff *schema.Resou 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 { + return nil + } + if v, ok := diff.GetOk("num_cache_nodes"); !ok || v.(int) != 1 { + return nil + } + + 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) == tfelasticache.EngineMemcached { + validator = validateVersionString + } else { + validator = ValidateElastiCacheRedisVersionString + } + + _, 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) == tfelasticache.EngineMemcached { + return nil + } + + if v, ok := diff.GetOk("num_cache_nodes"); !ok || v.(int) == 1 { + return nil + } + return errors.New(`engine "redis" does not support num_cache_nodes > 1`) +} + +// CustomizeDiffClusterMemcachedNodeType causes re-creation when `node_type` is changed and `engine` is "memcached" +func CustomizeDiffClusterMemcachedNodeType(_ context.Context, diff *schema.ResourceDiff, v interface{}) error { + // Engine memcached does not currently support vertical scaling + // https://docs.aws.amazon.com/AmazonElastiCache/latest/mem-ug/Scaling.html#Scaling.Memcached.Vertically + if diff.Id() == "" || !diff.HasChange("node_type") { + return nil + } + if v, ok := diff.GetOk("engine"); !ok || v.(string) == tfelasticache.EngineRedis { + return nil + } + return diff.ForceNew("node_type") +} + +// CustomizeDiffValidateClusterMemcachedSnapshotIdentifier validates that `final_snapshot_identifier` is not set when `engine` is "memcached" +func CustomizeDiffValidateClusterMemcachedSnapshotIdentifier(_ context.Context, diff *schema.ResourceDiff, v interface{}) error { + if v, ok := diff.GetOk("engine"); !ok || v.(string) == tfelasticache.EngineRedis { + return nil + } + if _, ok := diff.GetOk("final_snapshot_identifier"); !ok { + return nil + } + return errors.New(`engine "memcached" does not support final_snapshot_identifier`) +} + +// CustomizeDiffValidateReplicationGroupAutomaticFailover validates that `automatic_failover_enabled` is set when `multi_az_enabled` is true +func CustomizeDiffValidateReplicationGroupAutomaticFailover(_ context.Context, diff *schema.ResourceDiff, v interface{}) error { + if v := diff.Get("multi_az_enabled").(bool); !v { + return nil + } + if v := diff.Get("automatic_failover_enabled").(bool); !v { + return errors.New(`automatic_failover_enabled must be true if multi_az_enabled is true`) + } + return nil +} diff --git a/aws/resource_aws_elasticache_cluster.go b/aws/resource_aws_elasticache_cluster.go index 749f5892fd6..e64b927eeff 100644 --- a/aws/resource_aws_elasticache_cluster.go +++ b/aws/resource_aws_elasticache_cluster.go @@ -1,7 +1,6 @@ package aws import ( - "context" "errors" "fmt" "log" @@ -13,7 +12,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/elasticache" "github.com/hashicorp/aws-sdk-go-base/tfawserr" - multierror "github.com/hashicorp/go-multierror" gversion "github.com/hashicorp/go-version" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -263,75 +261,12 @@ func resourceAwsElasticacheCluster() *schema.Resource { }, CustomizeDiff: customdiff.Sequence( - func(_ context.Context, diff *schema.ResourceDiff, v interface{}) error { - // Plan time validation for az_mode - // InvalidParameterCombination: Must specify at least two cache nodes in order to specify AZ Mode of 'cross-az'. - if v, ok := diff.GetOk("az_mode"); !ok || v.(string) != elasticache.AZModeCrossAz { - return nil - } - if v, ok := diff.GetOk("num_cache_nodes"); !ok || v.(int) != 1 { - return nil - } - return errors.New(`az_mode "cross-az" is not supported with num_cache_nodes = 1`) - }, - func(_ context.Context, diff *schema.ResourceDiff, v interface{}) error { - // Plan time validation for engine_version values - // 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) == tfelasticache.EngineMemcached { - validator = validateVersionString - } else { - validator = ValidateElastiCacheRedisVersionString - } - - _, errs := validator(engineVersion, "engine_version") - - var err *multierror.Error - err = multierror.Append(err, errs...) - return err.ErrorOrNil() - }, - + CustomizeDiffValidateClusterAZMode, + CustomizeDiffValidateClusterEngineVersion, CustomizeDiffElastiCacheEngineVersion, - - func(_ context.Context, diff *schema.ResourceDiff, v interface{}) error { - // Plan time validation for num_cache_nodes - // InvalidParameterValue: Cannot create a Redis cluster with a NumCacheNodes parameter greater than 1. - if v, ok := diff.GetOk("engine"); !ok || v.(string) == tfelasticache.EngineMemcached { - return nil - } - - if v, ok := diff.GetOk("num_cache_nodes"); !ok || v.(int) == 1 { - return nil - } - return errors.New(`engine "redis" does not support num_cache_nodes > 1`) - }, - func(_ context.Context, diff *schema.ResourceDiff, v interface{}) error { - // Engine memcached does not currently support vertical scaling - // InvalidParameterCombination: Scaling is not supported for engine memcached - // https://docs.aws.amazon.com/AmazonElastiCache/latest/mem-ug/Scaling.html#Scaling.Memcached.Vertically - if diff.Id() == "" || !diff.HasChange("node_type") { - return nil - } - if v, ok := diff.GetOk("engine"); !ok || v.(string) == tfelasticache.EngineRedis { - return nil - } - return diff.ForceNew("node_type") - }, - func(_ context.Context, diff *schema.ResourceDiff, v interface{}) error { - if v, ok := diff.GetOk("engine"); !ok || v.(string) == tfelasticache.EngineRedis { - return nil - } - if _, ok := diff.GetOk("final_snapshot_identifier"); !ok { - return nil - } - return errors.New(`engine "memcached" does not support final_snapshot_identifier`) - }, + CustomizeDiffValidateClusterNumCacheNodes, + CustomizeDiffClusterMemcachedNodeType, + CustomizeDiffValidateClusterMemcachedSnapshotIdentifier, ), } } @@ -455,6 +390,9 @@ func resourceAwsElasticacheClusterRead(d *schema.ResourceData, meta interface{}) return err } + d.Set("snapshot_window", c.SnapshotWindow) + d.Set("snapshot_retention_limit", c.SnapshotRetentionLimit) + d.Set("num_cache_nodes", c.NumCacheNodes) if c.ConfigurationEndpoint != nil { @@ -522,9 +460,6 @@ func elasticacheSetResourceDataFromCacheCluster(d *schema.ResourceData, c *elast d.Set("maintenance_window", c.PreferredMaintenanceWindow) - d.Set("snapshot_window", c.SnapshotWindow) - d.Set("snapshot_retention_limit", c.SnapshotRetentionLimit) - return nil } diff --git a/aws/resource_aws_elasticache_replication_group.go b/aws/resource_aws_elasticache_replication_group.go index 545f692db84..cfd5293befd 100644 --- a/aws/resource_aws_elasticache_replication_group.go +++ b/aws/resource_aws_elasticache_replication_group.go @@ -298,18 +298,8 @@ func resourceAwsElasticacheReplicationGroup() *schema.Resource { }, CustomizeDiff: customdiff.Sequence( - func(_ context.Context, diff *schema.ResourceDiff, v interface{}) error { - if v := diff.Get("multi_az_enabled").(bool); !v { - return nil - } - if v := diff.Get("automatic_failover_enabled").(bool); !v { - return errors.New(`automatic_failover_enabled must be true if multi_az_enabled is true`) - } - return nil - }, - + CustomizeDiffValidateReplicationGroupAutomaticFailover, CustomizeDiffElastiCacheEngineVersion, - customdiff.ComputedIf("member_clusters", func(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) bool { return diff.HasChange("number_cache_clusters") || diff.HasChange("cluster_mode.0.num_node_groups") || @@ -530,6 +520,9 @@ func resourceAwsElasticacheReplicationGroupRead(d *schema.ResourceData, meta int return err } + d.Set("snapshot_window", rgp.SnapshotWindow) + d.Set("snapshot_retention_limit", rgp.SnapshotRetentionLimit) + if rgp.ConfigurationEndpoint != nil { d.Set("port", rgp.ConfigurationEndpoint.Port) d.Set("configuration_endpoint_address", rgp.ConfigurationEndpoint.Address)