From 54338ca47ffeac1c4106661d5058f3fd3b579291 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Sun, 14 Feb 2021 22:42:44 -0800 Subject: [PATCH 1/8] Adds ElastiCache Replication Group ID validator --- ...ource_aws_elasticache_replication_group.go | 5 +++-- aws/resource_aws_elasticache_cluster.go | 7 +++--- ...ws_elasticache_global_replication_group.go | 7 +++--- ...ource_aws_elasticache_replication_group.go | 22 ++++++++++--------- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/aws/data_source_aws_elasticache_replication_group.go b/aws/data_source_aws_elasticache_replication_group.go index f748325ca57..a8a2d4a8fe0 100644 --- a/aws/data_source_aws_elasticache_replication_group.go +++ b/aws/data_source_aws_elasticache_replication_group.go @@ -16,8 +16,9 @@ func dataSourceAwsElasticacheReplicationGroup() *schema.Resource { Read: dataSourceAwsElasticacheReplicationGroupRead, Schema: map[string]*schema.Schema{ "replication_group_id": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Required: true, + ValidateFunc: validateReplicationGroupID, }, "replication_group_description": { Type: schema.TypeString, diff --git a/aws/resource_aws_elasticache_cluster.go b/aws/resource_aws_elasticache_cluster.go index c9347a0d038..2c21551107e 100644 --- a/aws/resource_aws_elasticache_cluster.go +++ b/aws/resource_aws_elasticache_cluster.go @@ -174,9 +174,10 @@ func resourceAwsElasticacheCluster() *schema.Resource { Elem: &schema.Schema{Type: schema.TypeString}, }, "replication_group_id": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ValidateFunc: validateReplicationGroupID, ConflictsWith: []string{ "az_mode", "engine_version", diff --git a/aws/resource_aws_elasticache_global_replication_group.go b/aws/resource_aws_elasticache_global_replication_group.go index 1d8e731f857..d7816b3d2a5 100644 --- a/aws/resource_aws_elasticache_global_replication_group.go +++ b/aws/resource_aws_elasticache_global_replication_group.go @@ -115,9 +115,10 @@ func resourceAwsElasticacheGlobalReplicationGroup() *schema.Resource { // }, // }, "primary_replication_group_id": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validateReplicationGroupID, }, "transit_encryption_enabled": { Type: schema.TypeBool, diff --git a/aws/resource_aws_elasticache_replication_group.go b/aws/resource_aws_elasticache_replication_group.go index e6474ac569b..4885acae1a1 100644 --- a/aws/resource_aws_elasticache_replication_group.go +++ b/aws/resource_aws_elasticache_replication_group.go @@ -180,16 +180,10 @@ func resourceAwsElasticacheReplicationGroup() *schema.Resource { Required: true, }, "replication_group_id": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - ValidateFunc: validation.All( - validation.StringLenBetween(1, 40), - validation.StringMatch(regexp.MustCompile(`^[0-9a-zA-Z-]+$`), "must contain only alphanumeric characters and hyphens"), - validation.StringMatch(regexp.MustCompile(`^[a-zA-Z]`), "must begin with a letter"), - validation.StringDoesNotMatch(regexp.MustCompile(`--`), "cannot contain two consecutive hyphens"), - validation.StringDoesNotMatch(regexp.MustCompile(`-$`), "cannot end with a hyphen"), - ), + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validateReplicationGroupID, StateFunc: func(val interface{}) string { return strings.ToLower(val.(string)) }, @@ -913,3 +907,11 @@ func resourceAwsElasticacheReplicationGroupModify(conn *elasticache.ElastiCache, } return nil } + +var validateReplicationGroupID schema.SchemaValidateFunc = validation.All( + validation.StringLenBetween(1, 40), + validation.StringMatch(regexp.MustCompile(`^[0-9a-zA-Z-]+$`), "must contain only alphanumeric characters and hyphens"), + validation.StringMatch(regexp.MustCompile(`^[a-zA-Z]`), "must begin with a letter"), + validation.StringDoesNotMatch(regexp.MustCompile(`--`), "cannot contain two consecutive hyphens"), + validation.StringDoesNotMatch(regexp.MustCompile(`-$`), "cannot end with a hyphen"), +) From f8ce42df607d3b95b1ed67a9b660bf4b208323c7 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 16 Feb 2021 17:58:18 -0800 Subject: [PATCH 2/8] Allows creating replication group as member of global replication group --- .../service/elasticache/waiter/status.go | 19 ++ .../service/elasticache/waiter/waiter.go | 28 +++ ...ource_aws_elasticache_replication_group.go | 94 +++++++- ..._aws_elasticache_replication_group_test.go | 215 +++++++++++++++++- ...lasticache_replication_group.html.markdown | 3 +- 5 files changed, 347 insertions(+), 12 deletions(-) diff --git a/aws/internal/service/elasticache/waiter/status.go b/aws/internal/service/elasticache/waiter/status.go index 73af9a4d94b..9fb4fbd58dd 100644 --- a/aws/internal/service/elasticache/waiter/status.go +++ b/aws/internal/service/elasticache/waiter/status.go @@ -106,3 +106,22 @@ func GlobalReplicationGroupStatus(conn *elasticache.ElastiCache, globalReplicati return grg, aws.StringValue(grg.Status), nil } } + +const ( + GlobalReplicationGroupMemberStatusAssociated = "associated" +) + +// GlobalReplicationGroupStatus fetches the Global Replication Group and its Status +func GlobalReplicationGroupMemberStatus(conn *elasticache.ElastiCache, globalReplicationGroupID, id string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + member, err := finder.GlobalReplicationGroupMemberByID(conn, globalReplicationGroupID, id) + if tfresource.NotFound(err) { + return nil, "", nil + } + if err != nil { + return nil, "", err + } + + return member, aws.StringValue(member.Status), nil + } +} diff --git a/aws/internal/service/elasticache/waiter/waiter.go b/aws/internal/service/elasticache/waiter/waiter.go index b1ab13ced6e..29e9e4db448 100644 --- a/aws/internal/service/elasticache/waiter/waiter.go +++ b/aws/internal/service/elasticache/waiter/waiter.go @@ -199,3 +199,31 @@ func GlobalReplicationGroupDeleted(conn *elasticache.ElastiCache, globalReplicat } return nil, err } + +const ( + GlobalReplicationGroupDisassociationRetryTimeout = 30 * time.Minute + + globalReplicationGroupDisassociationTimeout = 10 * time.Minute + + globalReplicationGroupDisassociationMinTimeout = 10 * time.Second + globalReplicationGroupDisassociationDelay = 30 * time.Second +) + +func GlobalReplicationGroupMemberDetached(conn *elasticache.ElastiCache, globalReplicationGroupID, id string) (*elasticache.GlobalReplicationGroupMember, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{ + GlobalReplicationGroupMemberStatusAssociated, + }, + Target: []string{}, + Refresh: GlobalReplicationGroupMemberStatus(conn, globalReplicationGroupID, id), + Timeout: globalReplicationGroupDisassociationTimeout, + MinTimeout: globalReplicationGroupDisassociationMinTimeout, + Delay: globalReplicationGroupDisassociationDelay, + } + + outputRaw, err := stateConf.WaitForState() + if v, ok := outputRaw.(*elasticache.GlobalReplicationGroupMember); ok { + return v, err + } + return nil, err +} diff --git a/aws/resource_aws_elasticache_replication_group.go b/aws/resource_aws_elasticache_replication_group.go index 4885acae1a1..2167a738ba3 100644 --- a/aws/resource_aws_elasticache_replication_group.go +++ b/aws/resource_aws_elasticache_replication_group.go @@ -12,6 +12,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/elasticache" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -113,6 +114,25 @@ func resourceAwsElasticacheReplicationGroup() *schema.Resource { Optional: true, Computed: true, }, + "global_replication_group_id": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Computed: true, + ConflictsWith: []string{ + "automatic_failover_enabled", + "cluster_mode", // should/will be "num_node_groups" + "parameter_group_name", + "engine", + "engine_version", + "node_type", + "security_group_names", + "transit_encryption_enabled", + "at_rest_encryption_enabled", + "snapshot_arns", + "snapshot_name", + }, + }, "maintenance_window": { Type: schema.TypeString, Optional: true, @@ -318,13 +338,23 @@ func resourceAwsElasticacheReplicationGroupCreate(d *schema.ResourceData, meta i params := &elasticache.CreateReplicationGroupInput{ ReplicationGroupId: aws.String(d.Get("replication_group_id").(string)), ReplicationGroupDescription: aws.String(d.Get("replication_group_description").(string)), - AutomaticFailoverEnabled: aws.Bool(d.Get("automatic_failover_enabled").(bool)), AutoMinorVersionUpgrade: aws.Bool(d.Get("auto_minor_version_upgrade").(bool)), - CacheNodeType: aws.String(d.Get("node_type").(string)), - Engine: aws.String(d.Get("engine").(string)), Tags: tags, } + if v, ok := d.GetOk("global_replication_group_id"); ok { + params.GlobalReplicationGroupId = aws.String(v.(string)) + } else { + // This cannot be handled at plan-time + nodeType := d.Get("node_type").(string) + if nodeType == "" { + return errors.New(`"node_type" is required unless "global_replication_group_id" is set.`) + } + params.AutomaticFailoverEnabled = aws.Bool(d.Get("automatic_failover_enabled").(bool)) + params.CacheNodeType = aws.String(nodeType) + params.Engine = aws.String(d.Get("engine").(string)) + } + if v, ok := d.GetOk("engine_version"); ok { params.EngineVersion = aws.String(v.(string)) } @@ -416,7 +446,7 @@ func resourceAwsElasticacheReplicationGroupCreate(d *schema.ResourceData, meta i resp, err := conn.CreateReplicationGroup(params) if err != nil { - return fmt.Errorf("Error creating ElastiCache Replication Group: %w", err) + return fmt.Errorf("Error creating ElastiCache Replication Group (%s): %w", d.Get("replication_group_id").(string), err) } d.SetId(aws.StringValue(resp.ReplicationGroup.ReplicationGroupId)) @@ -443,12 +473,16 @@ func resourceAwsElasticacheReplicationGroupRead(d *schema.ResourceData, meta int return err } - if aws.StringValue(rgp.Status) == "deleting" { + if aws.StringValue(rgp.Status) == waiter.ReplicationGroupStatusDeleting { log.Printf("[WARN] ElastiCache Replication Group (%s) is currently in the `deleting` status, removing from state", d.Id()) d.SetId("") return nil } + if rgp.GlobalReplicationGroupInfo != nil && rgp.GlobalReplicationGroupInfo.GlobalReplicationGroupId != nil { + d.Set("global_replication_group_id", rgp.GlobalReplicationGroupInfo.GlobalReplicationGroupId) + } + if rgp.AutomaticFailover != nil { switch strings.ToLower(aws.StringValue(rgp.AutomaticFailover)) { case elasticache.AutomaticFailoverStatusDisabled, elasticache.AutomaticFailoverStatusDisabling: @@ -687,6 +721,13 @@ func resourceAwsElasticacheReplicationGroupUpdate(d *schema.ResourceData, meta i func resourceAwsElasticacheReplicationGroupDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).elasticacheconn + if globalReplicationGroupID, ok := d.GetOk("global_replication_group_id"); ok { + err := disassociateElasticacheReplicationGroup(conn, globalReplicationGroupID.(string), d.Id(), meta.(*AWSClient).region) + if err != nil { + return fmt.Errorf("error disassociating ElastiCache Replication Group (%s) from Global Replication Group (%s): %w", d.Id(), globalReplicationGroupID, err) + } + } + var finalSnapshotID = d.Get("final_snapshot_identifier").(string) err := deleteElasticacheReplicationGroup(d.Id(), conn, finalSnapshotID, d.Timeout(schema.TimeoutDelete)) if err != nil { @@ -696,6 +737,49 @@ func resourceAwsElasticacheReplicationGroupDelete(d *schema.ResourceData, meta i return nil } +func disassociateElasticacheReplicationGroup(conn *elasticache.ElastiCache, globalReplicationGroupID, id, region string) error { + input := &elasticache.DisassociateGlobalReplicationGroupInput{ + GlobalReplicationGroupId: aws.String(globalReplicationGroupID), + ReplicationGroupId: aws.String(id), + ReplicationGroupRegion: aws.String(region), + } + err := resource.Retry(waiter.GlobalReplicationGroupDisassociationRetryTimeout, func() *resource.RetryError { + _, err := conn.DisassociateGlobalReplicationGroup(input) + if tfawserr.ErrCodeEquals(err, elasticache.ErrCodeGlobalReplicationGroupNotFoundFault) { + return nil + } + if tfawserr.ErrCodeEquals(err, elasticache.ErrCodeInvalidGlobalReplicationGroupStateFault) { + return resource.RetryableError(err) + } + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + if isResourceTimeoutError(err) { + _, err = conn.DisassociateGlobalReplicationGroup(input) + } + if tfawserr.ErrMessageContains(err, elasticache.ErrCodeInvalidParameterValueException, "is not associated with Global Replication Group") { + return nil + } + if tfawserr.ErrCodeEquals(err, elasticache.ErrCodeInvalidGlobalReplicationGroupStateFault) { + return fmt.Errorf("tried for %s: %w", waiter.GlobalReplicationGroupDisassociationRetryTimeout.String(), err) + } + + if err != nil { + return err + } + + _, err = waiter.GlobalReplicationGroupMemberDetached(conn, globalReplicationGroupID, id) + if err != nil { + return fmt.Errorf("waiting for completion: %w", err) + } + + return nil + +} + func deleteElasticacheReplicationGroup(replicationGroupID string, conn *elasticache.ElastiCache, finalSnapshotID string, timeout time.Duration) error { input := &elasticache.DeleteReplicationGroupInput{ ReplicationGroupId: aws.String(replicationGroupID), diff --git a/aws/resource_aws_elasticache_replication_group_test.go b/aws/resource_aws_elasticache_replication_group_test.go index 3817ce359f1..a1be632e890 100644 --- a/aws/resource_aws_elasticache_replication_group_test.go +++ b/aws/resource_aws_elasticache_replication_group_test.go @@ -13,6 +13,7 @@ import ( "github.com/aws/aws-sdk-go/service/elasticache" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/elasticache/finder" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/elasticache/waiter" @@ -129,6 +130,28 @@ func TestAccAWSElasticacheReplicationGroup_Uppercase(t *testing.T) { }) } +func TestAccAWSElasticacheReplicationGroup_disappears(t *testing.T) { + var rg elasticache.ReplicationGroup + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_elasticache_replication_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSElasticacheReplicationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSElasticacheReplicationGroupConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheReplicationGroupExists(resourceName, &rg), + testAccCheckResourceDisappears(testAccProvider, resourceAwsElasticacheReplicationGroup(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func TestAccAWSElasticacheReplicationGroup_updateDescription(t *testing.T) { var rg elasticache.ReplicationGroup rName := acctest.RandomWithPrefix("tf-acc-test") @@ -785,8 +808,7 @@ func TestAccAWSElasticacheReplicationGroup_enableSnapshotting(t *testing.T) { Config: testAccAWSElasticacheReplicationGroupConfig(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSElasticacheReplicationGroupExists(resourceName, &rg), - resource.TestCheckResourceAttr( - resourceName, "snapshot_retention_limit", "0"), + resource.TestCheckResourceAttr(resourceName, "snapshot_retention_limit", "0"), ), }, { @@ -799,8 +821,7 @@ func TestAccAWSElasticacheReplicationGroup_enableSnapshotting(t *testing.T) { Config: testAccAWSElasticacheReplicationGroupConfigEnableSnapshotting(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSElasticacheReplicationGroupExists(resourceName, &rg), - resource.TestCheckResourceAttr( - resourceName, "snapshot_retention_limit", "2"), + resource.TestCheckResourceAttr(resourceName, "snapshot_retention_limit", "2"), ), }, }, @@ -847,8 +868,7 @@ func TestAccAWSElasticacheReplicationGroup_enableAtRestEncryption(t *testing.T) Config: testAccAWSElasticacheReplicationGroup_EnableAtRestEncryptionConfig(acctest.RandInt(), acctest.RandString(10)), Check: resource.ComposeTestCheckFunc( testAccCheckAWSElasticacheReplicationGroupExists(resourceName, &rg), - resource.TestCheckResourceAttr( - resourceName, "at_rest_encryption_enabled", "true"), + resource.TestCheckResourceAttr(resourceName, "at_rest_encryption_enabled", "true"), ), }, { @@ -1341,6 +1361,109 @@ func TestAccAWSElasticacheReplicationGroup_FinalSnapshot(t *testing.T) { }) } +func TestAccAWSElasticacheReplicationGroup_Validation_NoNodeType(t *testing.T) { + var providers []*schema.Provider + rName := acctest.RandomWithPrefix("tf-acc-test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccMultipleRegionPreCheck(t, 2) + }, + ProviderFactories: testAccProviderFactoriesMultipleRegion(&providers, 2), + CheckDestroy: testAccCheckAWSElasticacheReplicationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSElasticacheReplicationGroupConfig_Validation_NoNodeType(rName), + ExpectError: regexp.MustCompile(`"node_type" is required unless "global_replication_group_id" is set.`), + }, + }, + }) +} + +func TestAccAWSElasticacheReplicationGroup_Validation_GlobalReplicationGroupIdAndNodeType(t *testing.T) { + var providers []*schema.Provider + rName := acctest.RandomWithPrefix("tf-acc-test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccMultipleRegionPreCheck(t, 2) + }, + ProviderFactories: testAccProviderFactoriesMultipleRegion(&providers, 2), + CheckDestroy: testAccCheckAWSElasticacheReplicationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSElasticacheReplicationGroupConfig_Validation_GlobalReplicationGroupIdAndNodeType(rName), + ExpectError: regexp.MustCompile(`"global_replication_group_id": conflicts with node_type`), + }, + }, + }) +} + +func TestAccAWSElasticacheReplicationGroup_GlobalReplicationGroupId_Basic(t *testing.T) { + var providers []*schema.Provider + var rg elasticache.ReplicationGroup + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_elasticache_replication_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccMultipleRegionPreCheck(t, 2) + }, + ProviderFactories: testAccProviderFactoriesMultipleRegion(&providers, 2), + CheckDestroy: testAccCheckAWSElasticacheReplicationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSElasticacheReplicationGroupConfig_GlobalReplicationGroupId_Basic(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheReplicationGroupExists(resourceName, &rg), + resource.TestCheckResourceAttrPair(resourceName, "global_replication_group_id", "aws_elasticache_global_replication_group.test", "global_replication_group_id"), + resource.TestCheckResourceAttrPair(resourceName, "node_type", "aws_elasticache_replication_group.primary", "node_type"), + resource.TestCheckResourceAttrPair(resourceName, "engine", "aws_elasticache_replication_group.primary", "engine"), + resource.TestCheckResourceAttrPair(resourceName, "engine_version", "aws_elasticache_replication_group.primary", "engine_version"), + resource.TestCheckResourceAttrPair(resourceName, "parameter_group_name", "aws_elasticache_replication_group.primary", "parameter_group_name"), + ), + }, + { + Config: testAccAWSElasticacheReplicationGroupConfig_GlobalReplicationGroupId_Basic(rName), + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"apply_immediately"}, + }, + }, + }) +} + +// Test for out-of-band deletion +func TestAccAWSElasticacheReplicationGroup_disappears_GlobalReplicationGroupId(t *testing.T) { + var providers []*schema.Provider + var rg elasticache.ReplicationGroup + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_elasticache_replication_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccMultipleRegionPreCheck(t, 2) + }, + ProviderFactories: testAccProviderFactoriesMultipleRegion(&providers, 2), + CheckDestroy: testAccCheckAWSElasticacheReplicationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSElasticacheReplicationGroupConfig_GlobalReplicationGroupId_Basic(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheReplicationGroupExists(resourceName, &rg), + testAccCheckResourceDisappears(testAccProvider, resourceAwsElasticacheReplicationGroup(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func testAccCheckAWSElasticacheReplicationGroupExists(n string, v *elasticache.ReplicationGroup) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -2364,6 +2487,86 @@ resource "aws_elasticache_replication_group" "test" { `, rName) } +func testAccAWSElasticacheReplicationGroupConfig_Validation_NoNodeType(rName string) string { + return fmt.Sprintf(` +resource "aws_elasticache_replication_group" "test" { + replication_group_id = %[1]q + replication_group_description = "test description" + number_cache_clusters = 1 +} +`, rName) +} + +func testAccAWSElasticacheReplicationGroupConfig_Validation_GlobalReplicationGroupIdAndNodeType(rName string) string { + return composeConfig( + testAccMultipleRegionProviderConfig(2), + fmt.Sprintf(` +resource "aws_elasticache_replication_group" "test" { + replication_group_id = "%[1]s-s" + replication_group_description = "secondary" + global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + + node_type = "cache.m5.large" + + number_cache_clusters = 1 +} + +resource "aws_elasticache_global_replication_group" "test" { + provider = awsalternate + + global_replication_group_id_suffix = %[1]q + primary_replication_group_id = aws_elasticache_replication_group.primary.id +} + +resource "aws_elasticache_replication_group" "primary" { + provider = awsalternate + + replication_group_id = "%[1]s-p" + replication_group_description = "primary" + + node_type = "cache.m5.large" + + engine = "redis" + engine_version = "5.0.6" + number_cache_clusters = 1 +} +`, rName)) +} + +func testAccAWSElasticacheReplicationGroupConfig_GlobalReplicationGroupId_Basic(rName string) string { + return composeConfig( + testAccMultipleRegionProviderConfig(2), + fmt.Sprintf(` +resource "aws_elasticache_replication_group" "test" { + replication_group_id = "%[1]s-s" + replication_group_description = "secondary" + global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + + number_cache_clusters = 1 +} + +resource "aws_elasticache_global_replication_group" "test" { + provider = awsalternate + + global_replication_group_id_suffix = %[1]q + primary_replication_group_id = aws_elasticache_replication_group.primary.id +} + +resource "aws_elasticache_replication_group" "primary" { + provider = awsalternate + + replication_group_id = "%[1]s-p" + replication_group_description = "primary" + + node_type = "cache.m5.large" + + engine = "redis" + engine_version = "5.0.6" + number_cache_clusters = 1 +} +`, rName)) +} + func resourceAwsElasticacheReplicationGroupDisableAutomaticFailover(conn *elasticache.ElastiCache, replicationGroupID string, timeout time.Duration) error { return resourceAwsElasticacheReplicationGroupModify(conn, timeout, &elasticache.ModifyReplicationGroupInput{ ReplicationGroupId: aws.String(replicationGroupID), diff --git a/website/docs/r/elasticache_replication_group.html.markdown b/website/docs/r/elasticache_replication_group.html.markdown index 996fd6541aa..c2e04723c3e 100644 --- a/website/docs/r/elasticache_replication_group.html.markdown +++ b/website/docs/r/elasticache_replication_group.html.markdown @@ -109,8 +109,9 @@ The following arguments are supported: * `replication_group_id` – (Required) The replication group identifier. This parameter is stored as a lowercase string. * `replication_group_description` – (Required) A user-created description for the replication group. +* ``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. * `number_cache_clusters` - (Optional) The number of cache clusters (primary and replicas) this replication group will have. If Multi-AZ is enabled, the value of this parameter must be at least 2. Updates will occur before other modifications. One of `number_cache_clusters` or `cluster_mode` is required. -* `node_type` - (Required) The instance class to be used. See AWS documentation for information on [supported node types](https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/CacheNodes.SupportedTypes.html) and [guidance on selecting node types](https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/nodes-select-size.html). +* `node_type` - (Optional) The instance class to be used. See AWS documentation for information on [supported node types](https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/CacheNodes.SupportedTypes.html) and [guidance on selecting node types](https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/nodes-select-size.html). Required unless `global_replication_group_id` is set. Cannot be set if `global_replication_group_id` is set. * `automatic_failover_enabled` - (Optional) Specifies whether a read-only replica will be automatically promoted to read/write primary if the existing primary fails. If true, Multi-AZ is enabled for this replication group. If false, Multi-AZ is disabled for this replication group. Must be enabled for Redis (cluster mode enabled) replication groups. Defaults to `false`. * `multi_az_enabled` - (Optional) Specifies whether to enable Multi-AZ Support for the replication group. If `true`, `automatic_failover_enabled` must also be enabled. Defaults to `false`. * `auto_minor_version_upgrade` - (Optional) Specifies whether a minor engine upgrades will be applied automatically to the underlying Cache Cluster instances during the maintenance window. This parameter is currently not supported by the AWS API. Defaults to `true`. From ba0eda6ed61461ffc5740081cca9b35963c11ee4 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 17 Feb 2021 12:09:08 -0800 Subject: [PATCH 3/8] Updates documentation --- ...che_global_replication_group.html.markdown | 29 +++++++++++----- ...lasticache_replication_group.html.markdown | 34 +++++++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/website/docs/r/elasticache_global_replication_group.html.markdown b/website/docs/r/elasticache_global_replication_group.html.markdown index f578d36f94f..8ad8af5866d 100644 --- a/website/docs/r/elasticache_global_replication_group.html.markdown +++ b/website/docs/r/elasticache_global_replication_group.html.markdown @@ -8,27 +8,38 @@ description: |- # Resource: aws_elasticache_global_replication_group -Provides an ElastiCache Global Replication Group resource, which manage a replication between 2 or more redis replication group in different regions. +Provides an ElastiCache Global Replication Group resource, which manage a replication between two or more Replication Groups in different regions. ## Example Usage -### Global replication group with a single instance redis replication group +### Global replication group with one secondary replication group -To create a single shard primary with single read replica: +The global replication group depends on the primary group existing. Secondary replication groups depend on the global replication group. Terraform dependency management will handle this transparently using resource value references. ```hcl -resource "aws_elasticache_global_replication_group" "replication_group" { +resource "aws_elasticache_global_replication_group" "example" { global_replication_group_id_suffix = "example" primary_replication_group_id = aws_elasticache_replication_group.primary.id } resource "aws_elasticache_replication_group" "primary" { - replication_group_id = "example" - replication_group_description = "test example" + replication_group_id = "example-primary" + replication_group_description = "primary replication group" + + engine = "redis" + engine_version = "5.0.6" + node_type = "cache.m5.large" + + number_cache_clusters = 1 +} + +resource "aws_elasticache_replication_group" "secondary" { + provider = aws.other_region + + replication_group_id = "example-secondary" + replication_group_description = "secondary replication group" + global_replication_group_id = aws_elasticache_global_replication_group.example.global_replication_group_id - engine = "redis" - engine_version = "5.0.6" - node_type = "cache.m5.large" number_cache_clusters = 1 } ``` diff --git a/website/docs/r/elasticache_replication_group.html.markdown b/website/docs/r/elasticache_replication_group.html.markdown index c2e04723c3e..334557e5b3d 100644 --- a/website/docs/r/elasticache_replication_group.html.markdown +++ b/website/docs/r/elasticache_replication_group.html.markdown @@ -103,6 +103,40 @@ resource "aws_elasticache_replication_group" "baz" { and unavailable on T1 node types. For T2 node types, it is only available on Redis version 3.2.4 or later with cluster mode enabled. See the [High Availability Using Replication Groups](https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/Replication.html) guide for full details on using Replication Groups. +### Creating a secondary replication group for a global replication group + +A Global Replication Group can have one one two secondary Replication Groups in different regions. These are added to an existing Global Replication Group. + +```hcl +resource "aws_elasticache_replication_group" "secondary" { + replication_group_id = "example-secondary" + replication_group_description = "secondary replication group" + global_replication_group_id = aws_elasticache_global_replication_group.example.global_replication_group_id + + number_cache_clusters = 1 +} + +resource "aws_elasticache_global_replication_group" "example" { + provider = aws.other_region + + global_replication_group_id_suffix = "example" + primary_replication_group_id = aws_elasticache_replication_group.primary.id +} + +resource "aws_elasticache_replication_group" "primary" { + provider = aws.other_region + + replication_group_id = "example-primary" + replication_group_description = "primary replication group" + + engine = "redis" + engine_version = "5.0.6" + node_type = "cache.m5.large" + + number_cache_clusters = 1 +} +``` + ## Argument Reference The following arguments are supported: From b9fa4c419b6995304f54d8ae292fa0aeec8b0999 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 19 Feb 2021 09:46:29 -0800 Subject: [PATCH 4/8] Adds tests for adding members to global replication groups --- .semgrep.yml | 8 + .../service/elasticache/finder/finder.go | 8 +- .../service/elasticache/waiter/waiter.go | 4 +- ...asticache_global_replication_group_test.go | 247 +++++++++++++++++- ...ource_aws_elasticache_replication_group.go | 3 +- ..._aws_elasticache_replication_group_test.go | 2 +- 6 files changed, 262 insertions(+), 10 deletions(-) diff --git a/.semgrep.yml b/.semgrep.yml index 8637bcd5012..21afe3264d5 100644 --- a/.semgrep.yml +++ b/.semgrep.yml @@ -10,6 +10,8 @@ rules: - metavariable-regex: metavariable: "$FUNCNAME" regex: "^TestAcc[^_]+_([a-zA-Z]+[dD]isappears|[^_]+_disappears)$" + # To allow grouping all TestAccAWSElasticacheReplicationGroup_GlobalReplicationGroupId_* tests + - pattern-not: func TestAccAWSElasticacheReplicationGroup_GlobalReplicationGroupId_disappears(t *testing.T) { ... } severity: WARNING - id: aws-sdk-go-multiple-service-imports @@ -160,6 +162,12 @@ rules: ... errors.As($ERR, &$CAST) - pattern-not-inside: func NotFound(err error) bool { ... } + - pattern-not-inside: | + var $CAST *resource.NotFoundError + ... + errors.As($ERR, &$CAST) + ... + $CAST.$FIELD - patterns: - pattern: | $X, $Y := $ERR.(*resource.NotFoundError) diff --git a/aws/internal/service/elasticache/finder/finder.go b/aws/internal/service/elasticache/finder/finder.go index c4d37907090..a0171bc90ea 100644 --- a/aws/internal/service/elasticache/finder/finder.go +++ b/aws/internal/service/elasticache/finder/finder.go @@ -27,7 +27,7 @@ func ReplicationGroupByID(conn *elasticache.ElastiCache, id string) (*elasticach if output == nil || len(output.ReplicationGroups) == 0 || output.ReplicationGroups[0] == nil { return nil, &resource.NotFoundError{ - Message: "Empty result", + Message: "empty result", LastRequest: input, } } @@ -48,7 +48,7 @@ func ReplicationGroupMemberClustersByID(conn *elasticache.ElastiCache, id string } if len(clusters) == 0 { return clusters, &resource.NotFoundError{ - Message: "No Member Clusters found", + Message: fmt.Sprintf("No Member Clusters found in Replication Group (%s)", id), } } @@ -87,7 +87,7 @@ func CacheCluster(conn *elasticache.ElastiCache, input *elasticache.DescribeCach if result == nil || len(result.CacheClusters) == 0 || result.CacheClusters[0] == nil { return nil, &resource.NotFoundError{ - Message: "Empty result", + Message: "empty result", LastRequest: input, } } @@ -176,6 +176,6 @@ func GlobalReplicationGroupMemberByID(conn *elasticache.ElastiCache, globalRepli } return nil, &resource.NotFoundError{ - Message: fmt.Sprintf("Replication Group %q not found in Global Replication Group %q", id, globalReplicationGroupID), + Message: fmt.Sprintf("Replication Group (%s) not found in Global Replication Group (%s)", id, globalReplicationGroupID), } } diff --git a/aws/internal/service/elasticache/waiter/waiter.go b/aws/internal/service/elasticache/waiter/waiter.go index 29e9e4db448..4012e5c77d8 100644 --- a/aws/internal/service/elasticache/waiter/waiter.go +++ b/aws/internal/service/elasticache/waiter/waiter.go @@ -201,9 +201,9 @@ func GlobalReplicationGroupDeleted(conn *elasticache.ElastiCache, globalReplicat } const ( - GlobalReplicationGroupDisassociationRetryTimeout = 30 * time.Minute + GlobalReplicationGroupDisassociationRetryTimeout = 45 * time.Minute - globalReplicationGroupDisassociationTimeout = 10 * time.Minute + globalReplicationGroupDisassociationTimeout = 20 * time.Minute globalReplicationGroupDisassociationMinTimeout = 10 * time.Second globalReplicationGroupDisassociationDelay = 30 * time.Second diff --git a/aws/resource_aws_elasticache_global_replication_group_test.go b/aws/resource_aws_elasticache_global_replication_group_test.go index 8baf3136fb8..26008b619ad 100644 --- a/aws/resource_aws_elasticache_global_replication_group_test.go +++ b/aws/resource_aws_elasticache_global_replication_group_test.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/elasticache/finder" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/elasticache/waiter" @@ -170,6 +171,60 @@ func TestAccAWSElasticacheGlobalReplicationGroup_disappears(t *testing.T) { }) } +func TestAccAWSElasticacheGlobalReplicationGroup_MultipleSecondaries(t *testing.T) { + var providers []*schema.Provider + var globalReplcationGroup elasticache.GlobalReplicationGroup + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_elasticache_global_replication_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccMultipleRegionPreCheck(t, 3) + }, + ProviderFactories: testAccProviderFactoriesMultipleRegion(&providers, 3), + CheckDestroy: testAccCheckAWSElasticacheReplicationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSElasticacheGlobalReplicationGroupConfig_MultipleSecondaries(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheGlobalReplicationGroupExists(resourceName, &globalReplcationGroup), + ), + }, + }, + }) +} + +func TestAccAWSElasticacheGlobalReplicationGroup_ReplaceSecondary_DifferentRegion(t *testing.T) { + var providers []*schema.Provider + var globalReplcationGroup elasticache.GlobalReplicationGroup + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_elasticache_global_replication_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccMultipleRegionPreCheck(t, 3) + }, + ProviderFactories: testAccProviderFactoriesMultipleRegion(&providers, 3), + CheckDestroy: testAccCheckAWSElasticacheReplicationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_DifferentRegion_Setup(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheGlobalReplicationGroupExists(resourceName, &globalReplcationGroup), + ), + }, + { + Config: testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_DifferentRegion_Move(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSElasticacheGlobalReplicationGroupExists(resourceName, &globalReplcationGroup), + ), + }, + }, + }) +} + func testAccCheckAWSElasticacheGlobalReplicationGroupExists(resourceName string, v *elasticache.GlobalReplicationGroup) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resourceName] @@ -187,7 +242,7 @@ func testAccCheckAWSElasticacheGlobalReplicationGroupExists(resourceName string, return fmt.Errorf("error retrieving ElastiCache Global Replication Group (%s): %w", rs.Primary.ID, err) } - if aws.StringValue(grg.Status) != waiter.GlobalReplicationGroupStatusAvailable && aws.StringValue(grg.Status) != waiter.GlobalReplicationGroupStatusPrimaryOnly { + if aws.StringValue(grg.Status) == waiter.GlobalReplicationGroupStatusDeleting || aws.StringValue(grg.Status) == waiter.GlobalReplicationGroupStatusDeleted { return fmt.Errorf("ElastiCache Global Replication Group (%s) exists, but is in a non-available state: %s", rs.Primary.ID, aws.StringValue(grg.Status)) } @@ -273,3 +328,193 @@ resource "aws_elasticache_replication_group" "test" { } `, rName, primaryReplicationGroupId, description) } + +func testAccAWSElasticacheGlobalReplicationGroupConfig_MultipleSecondaries(rName string) string { + return composeConfig( + testAccMultipleRegionProviderConfig(3), + fmt.Sprintf(` +resource "aws_elasticache_global_replication_group" "test" { + provider = aws + + global_replication_group_id_suffix = %[1]q + primary_replication_group_id = aws_elasticache_replication_group.primary.id +} + +resource "aws_elasticache_replication_group" "primary" { + provider = aws + + replication_group_id = "%[1]s-p" + replication_group_description = "primary" + + node_type = "cache.m5.large" + + engine = "redis" + engine_version = "5.0.6" + number_cache_clusters = 1 +} + +resource "aws_elasticache_replication_group" "alternate" { + provider = awsalternate + + replication_group_id = "%[1]s-a" + replication_group_description = "alternate" + global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + + number_cache_clusters = 1 +} + +resource "aws_elasticache_replication_group" "third" { + provider = awsthird + + replication_group_id = "%[1]s-t" + replication_group_description = "third" + global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + + number_cache_clusters = 1 +} +`, rName)) +} + +func testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_SameRegion_Setup(rName string) string { + return composeConfig( + testAccMultipleRegionProviderConfig(2), + fmt.Sprintf(` +resource "aws_elasticache_global_replication_group" "test" { + provider = aws + + global_replication_group_id_suffix = %[1]q + primary_replication_group_id = aws_elasticache_replication_group.primary.id +} + +resource "aws_elasticache_replication_group" "primary" { + provider = aws + + replication_group_id = "%[1]s-p" + replication_group_description = "primary" + + node_type = "cache.m5.large" + + engine = "redis" + engine_version = "5.0.6" + number_cache_clusters = 1 +} + +resource "aws_elasticache_replication_group" "original" { + provider = awsalternate + + replication_group_id = "%[1]s-o" + replication_group_description = "original" + global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + + number_cache_clusters = 1 +} +`, rName)) +} + +func testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_SameRegion_Move(rName string) string { + return composeConfig( + testAccMultipleRegionProviderConfig(2), + fmt.Sprintf(` +resource "aws_elasticache_global_replication_group" "test" { + provider = aws + + global_replication_group_id_suffix = %[1]q + primary_replication_group_id = aws_elasticache_replication_group.primary.id +} + +resource "aws_elasticache_replication_group" "primary" { + provider = aws + + replication_group_id = "%[1]s-p" + replication_group_description = "primary" + + node_type = "cache.m5.large" + + engine = "redis" + engine_version = "5.0.6" + number_cache_clusters = 1 +} + +resource "aws_elasticache_replication_group" "new" { + provider = awsalternate + + replication_group_id = "%[1]s-t" + replication_group_description = "new" + global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + + number_cache_clusters = 1 +} +`, rName)) +} + +func testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_DifferentRegion_Setup(rName string) string { + return composeConfig( + testAccMultipleRegionProviderConfig(3), + fmt.Sprintf(` +resource "aws_elasticache_global_replication_group" "test" { + provider = aws + + global_replication_group_id_suffix = %[1]q + primary_replication_group_id = aws_elasticache_replication_group.primary.id +} + +resource "aws_elasticache_replication_group" "primary" { + provider = aws + + replication_group_id = "%[1]s-p" + replication_group_description = "primary" + + node_type = "cache.m5.large" + + engine = "redis" + engine_version = "5.0.6" + number_cache_clusters = 1 +} + +resource "aws_elasticache_replication_group" "secondary" { + provider = awsalternate + + replication_group_id = "%[1]s-a" + replication_group_description = "alternate" + global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + + number_cache_clusters = 1 +} +`, rName)) +} + +func testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_DifferentRegion_Move(rName string) string { + return composeConfig( + testAccMultipleRegionProviderConfig(3), + fmt.Sprintf(` +resource "aws_elasticache_global_replication_group" "test" { + provider = aws + + global_replication_group_id_suffix = %[1]q + primary_replication_group_id = aws_elasticache_replication_group.primary.id +} + +resource "aws_elasticache_replication_group" "primary" { + provider = aws + + replication_group_id = "%[1]s-p" + replication_group_description = "primary" + + node_type = "cache.m5.large" + + engine = "redis" + engine_version = "5.0.6" + number_cache_clusters = 1 +} + +resource "aws_elasticache_replication_group" "third" { + provider = awsthird + + replication_group_id = "%[1]s-t" + replication_group_description = "third" + global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + + number_cache_clusters = 1 +} +`, rName)) +} diff --git a/aws/resource_aws_elasticache_replication_group.go b/aws/resource_aws_elasticache_replication_group.go index 2167a738ba3..79d3d7b23ce 100644 --- a/aws/resource_aws_elasticache_replication_group.go +++ b/aws/resource_aws_elasticache_replication_group.go @@ -443,7 +443,6 @@ func resourceAwsElasticacheReplicationGroupCreate(d *schema.ResourceData, meta i if cacheClusters, ok := d.GetOk("number_cache_clusters"); ok { params.NumCacheClusters = aws.Int64(int64(cacheClusters.(int))) } - resp, err := conn.CreateReplicationGroup(params) if err != nil { return fmt.Errorf("Error creating ElastiCache Replication Group (%s): %w", d.Get("replication_group_id").(string), err) @@ -453,7 +452,7 @@ func resourceAwsElasticacheReplicationGroupCreate(d *schema.ResourceData, meta i _, err = waiter.ReplicationGroupAvailable(conn, d.Id(), d.Timeout(schema.TimeoutCreate)) if err != nil { - return fmt.Errorf("error waiting for ElastiCache Replication Group (%s) to be created: %w", d.Id(), err) + return fmt.Errorf("error creating ElastiCache Replication Group (%s): waiting for completion: %w", d.Id(), err) } return resourceAwsElasticacheReplicationGroupRead(d, meta) diff --git a/aws/resource_aws_elasticache_replication_group_test.go b/aws/resource_aws_elasticache_replication_group_test.go index a1be632e890..97f287b1641 100644 --- a/aws/resource_aws_elasticache_replication_group_test.go +++ b/aws/resource_aws_elasticache_replication_group_test.go @@ -1438,7 +1438,7 @@ func TestAccAWSElasticacheReplicationGroup_GlobalReplicationGroupId_Basic(t *tes } // Test for out-of-band deletion -func TestAccAWSElasticacheReplicationGroup_disappears_GlobalReplicationGroupId(t *testing.T) { +func TestAccAWSElasticacheReplicationGroup_GlobalReplicationGroupId_disappears(t *testing.T) { var providers []*schema.Provider var rg elasticache.ReplicationGroup rName := acctest.RandomWithPrefix("tf-acc-test") From 9234996dc2b0be642d05dd393c9d230af7439eb5 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 19 Feb 2021 14:47:31 -0800 Subject: [PATCH 5/8] Removes unused code --- ...asticache_global_replication_group_test.go | 72 ------------------- 1 file changed, 72 deletions(-) diff --git a/aws/resource_aws_elasticache_global_replication_group_test.go b/aws/resource_aws_elasticache_global_replication_group_test.go index 26008b619ad..e8ab8e7fb29 100644 --- a/aws/resource_aws_elasticache_global_replication_group_test.go +++ b/aws/resource_aws_elasticache_global_replication_group_test.go @@ -375,78 +375,6 @@ resource "aws_elasticache_replication_group" "third" { `, rName)) } -func testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_SameRegion_Setup(rName string) string { - return composeConfig( - testAccMultipleRegionProviderConfig(2), - fmt.Sprintf(` -resource "aws_elasticache_global_replication_group" "test" { - provider = aws - - global_replication_group_id_suffix = %[1]q - primary_replication_group_id = aws_elasticache_replication_group.primary.id -} - -resource "aws_elasticache_replication_group" "primary" { - provider = aws - - replication_group_id = "%[1]s-p" - replication_group_description = "primary" - - node_type = "cache.m5.large" - - engine = "redis" - engine_version = "5.0.6" - number_cache_clusters = 1 -} - -resource "aws_elasticache_replication_group" "original" { - provider = awsalternate - - replication_group_id = "%[1]s-o" - replication_group_description = "original" - global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id - - number_cache_clusters = 1 -} -`, rName)) -} - -func testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_SameRegion_Move(rName string) string { - return composeConfig( - testAccMultipleRegionProviderConfig(2), - fmt.Sprintf(` -resource "aws_elasticache_global_replication_group" "test" { - provider = aws - - global_replication_group_id_suffix = %[1]q - primary_replication_group_id = aws_elasticache_replication_group.primary.id -} - -resource "aws_elasticache_replication_group" "primary" { - provider = aws - - replication_group_id = "%[1]s-p" - replication_group_description = "primary" - - node_type = "cache.m5.large" - - engine = "redis" - engine_version = "5.0.6" - number_cache_clusters = 1 -} - -resource "aws_elasticache_replication_group" "new" { - provider = awsalternate - - replication_group_id = "%[1]s-t" - replication_group_description = "new" - global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id - - number_cache_clusters = 1 -} -`, rName)) -} - func testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_DifferentRegion_Setup(rName string) string { return composeConfig( testAccMultipleRegionProviderConfig(3), From 6ff6cd008ed01b93adbb349f3f5ce611f5786593 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 9 Mar 2021 12:20:10 -0800 Subject: [PATCH 6/8] Uses `nosemgrep` to exclude matches --- .semgrep.yml | 2 -- aws/resource_aws_elasticache_replication_group_test.go | 3 ++- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.semgrep.yml b/.semgrep.yml index 9b53116d71d..98b570a7b11 100644 --- a/.semgrep.yml +++ b/.semgrep.yml @@ -10,8 +10,6 @@ rules: - metavariable-regex: metavariable: "$FUNCNAME" regex: "^TestAcc[^_]+_([a-zA-Z]+[dD]isappears|[^_]+_disappears)$" - # To allow grouping all TestAccAWSElasticacheReplicationGroup_GlobalReplicationGroupId_* tests - - pattern-not: func TestAccAWSElasticacheReplicationGroup_GlobalReplicationGroupId_disappears(t *testing.T) { ... } severity: WARNING - id: aws-sdk-go-multiple-service-imports diff --git a/aws/resource_aws_elasticache_replication_group_test.go b/aws/resource_aws_elasticache_replication_group_test.go index 97f287b1641..329605b8658 100644 --- a/aws/resource_aws_elasticache_replication_group_test.go +++ b/aws/resource_aws_elasticache_replication_group_test.go @@ -1438,7 +1438,8 @@ func TestAccAWSElasticacheReplicationGroup_GlobalReplicationGroupId_Basic(t *tes } // Test for out-of-band deletion -func TestAccAWSElasticacheReplicationGroup_GlobalReplicationGroupId_disappears(t *testing.T) { +// Naming to allow grouping all TestAccAWSElasticacheReplicationGroup_GlobalReplicationGroupId_* tests +func TestAccAWSElasticacheReplicationGroup_GlobalReplicationGroupId_disappears(t *testing.T) { // nosemgrep: acceptance-test-naming-parent-disappears var providers []*schema.Provider var rg elasticache.ReplicationGroup rName := acctest.RandomWithPrefix("tf-acc-test") From f5dc2d0eb5e336501e98c365379ec42999232f65 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 11 Mar 2021 12:30:03 -0800 Subject: [PATCH 7/8] Adds VPC to Global Replication Group tests --- GNUmakefile | 2 +- ...asticache_global_replication_group_test.go | 74 +++++++++++++++++++ ..._aws_elasticache_replication_group_test.go | 14 ++++ 3 files changed, 89 insertions(+), 1 deletion(-) diff --git a/GNUmakefile b/GNUmakefile index 6dc5be1472e..61a0304aa75 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -3,7 +3,7 @@ TEST?=./... SWEEP_DIR?=./aws PKG_NAME=aws TEST_COUNT?=1 -ACCTEST_TIMEOUT?=120m +ACCTEST_TIMEOUT?=180m ACCTEST_PARALLELISM?=20 default: build diff --git a/aws/resource_aws_elasticache_global_replication_group_test.go b/aws/resource_aws_elasticache_global_replication_group_test.go index e8ab8e7fb29..304343cd981 100644 --- a/aws/resource_aws_elasticache_global_replication_group_test.go +++ b/aws/resource_aws_elasticache_global_replication_group_test.go @@ -332,6 +332,9 @@ resource "aws_elasticache_replication_group" "test" { func testAccAWSElasticacheGlobalReplicationGroupConfig_MultipleSecondaries(rName string) string { return composeConfig( testAccMultipleRegionProviderConfig(3), + testAccElasticacheVpcBaseWithProvider(rName, "primary", ProviderNameAws), + testAccElasticacheVpcBaseWithProvider(rName, "alternate", ProviderNameAwsAlternate), + testAccElasticacheVpcBaseWithProvider(rName, "third", ProviderNameAwsThird), fmt.Sprintf(` resource "aws_elasticache_global_replication_group" "test" { provider = aws @@ -346,6 +349,8 @@ resource "aws_elasticache_replication_group" "primary" { replication_group_id = "%[1]s-p" replication_group_description = "primary" + subnet_group_name = aws_elasticache_subnet_group.primary.name + node_type = "cache.m5.large" engine = "redis" @@ -360,6 +365,8 @@ resource "aws_elasticache_replication_group" "alternate" { replication_group_description = "alternate" global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + subnet_group_name = aws_elasticache_subnet_group.alternate.name + number_cache_clusters = 1 } @@ -370,6 +377,8 @@ resource "aws_elasticache_replication_group" "third" { replication_group_description = "third" global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + subnet_group_name = aws_elasticache_subnet_group.third.name + number_cache_clusters = 1 } `, rName)) @@ -378,6 +387,9 @@ resource "aws_elasticache_replication_group" "third" { func testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_DifferentRegion_Setup(rName string) string { return composeConfig( testAccMultipleRegionProviderConfig(3), + testAccElasticacheVpcBaseWithProvider(rName, "primary", ProviderNameAws), + testAccElasticacheVpcBaseWithProvider(rName, "secondary", ProviderNameAwsAlternate), + testAccElasticacheVpcBaseWithProvider(rName, "third", ProviderNameAwsThird), fmt.Sprintf(` resource "aws_elasticache_global_replication_group" "test" { provider = aws @@ -392,6 +404,8 @@ resource "aws_elasticache_replication_group" "primary" { replication_group_id = "%[1]s-p" replication_group_description = "primary" + subnet_group_name = aws_elasticache_subnet_group.primary.name + node_type = "cache.m5.large" engine = "redis" @@ -406,6 +420,8 @@ resource "aws_elasticache_replication_group" "secondary" { replication_group_description = "alternate" global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + subnet_group_name = aws_elasticache_subnet_group.secondary.name + number_cache_clusters = 1 } `, rName)) @@ -414,6 +430,9 @@ resource "aws_elasticache_replication_group" "secondary" { func testAccAWSElasticacheReplicationGroupConfig_ReplaceSecondary_DifferentRegion_Move(rName string) string { return composeConfig( testAccMultipleRegionProviderConfig(3), + testAccElasticacheVpcBaseWithProvider(rName, "primary", ProviderNameAws), + testAccElasticacheVpcBaseWithProvider(rName, "secondary", ProviderNameAwsAlternate), + testAccElasticacheVpcBaseWithProvider(rName, "third", ProviderNameAwsThird), fmt.Sprintf(` resource "aws_elasticache_global_replication_group" "test" { provider = aws @@ -428,6 +447,8 @@ resource "aws_elasticache_replication_group" "primary" { replication_group_id = "%[1]s-p" replication_group_description = "primary" + subnet_group_name = aws_elasticache_subnet_group.primary.name + node_type = "cache.m5.large" engine = "redis" @@ -442,7 +463,60 @@ resource "aws_elasticache_replication_group" "third" { replication_group_description = "third" global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + subnet_group_name = aws_elasticache_subnet_group.third.name + number_cache_clusters = 1 } `, rName)) } + +func testAccElasticacheVpcBaseWithProvider(rName, name, provider string) string { + return composeConfig( + testAccAvailableAZsNoOptInConfigWithProvider(name, provider), + fmt.Sprintf(` +resource "aws_vpc" "%[1]s" { + provider = %[2]s + + cidr_block = "192.168.0.0/16" +} + +resource "aws_subnet" "%[1]s" { + provider = %[2]s + + vpc_id = aws_vpc.%[1]s.id + cidr_block = "192.168.0.0/20" + availability_zone = data.aws_availability_zones.%[1]s.names[0] + + tags = { + Name = "tf-acc-elasticache-replication-group-at-rest-encryption" + } +} + +resource "aws_elasticache_subnet_group" "%[1]s" { + provider = %[2]s + + name = %[3]q + description = "tf-test-cache-subnet-group-descr" + + subnet_ids = [ + aws_subnet.%[1]s.id, + ] +} +`, name, provider, rName), + ) +} + +func testAccAvailableAZsNoOptInConfigWithProvider(name, provider string) string { + return fmt.Sprintf(` +data "aws_availability_zones" "%[1]s" { + provider = %[2]s + + state = "available" + + filter { + name = "opt-in-status" + values = ["opt-in-not-required"] + } +} +`, name, provider) +} diff --git a/aws/resource_aws_elasticache_replication_group_test.go b/aws/resource_aws_elasticache_replication_group_test.go index 329605b8658..b855ae7dd62 100644 --- a/aws/resource_aws_elasticache_replication_group_test.go +++ b/aws/resource_aws_elasticache_replication_group_test.go @@ -2501,12 +2501,18 @@ resource "aws_elasticache_replication_group" "test" { func testAccAWSElasticacheReplicationGroupConfig_Validation_GlobalReplicationGroupIdAndNodeType(rName string) string { return composeConfig( testAccMultipleRegionProviderConfig(2), + testAccElasticacheVpcBaseWithProvider(rName, "test", ProviderNameAws), + testAccElasticacheVpcBaseWithProvider(rName, "primary", ProviderNameAwsAlternate), fmt.Sprintf(` resource "aws_elasticache_replication_group" "test" { + provider = aws + replication_group_id = "%[1]s-s" replication_group_description = "secondary" global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + subnet_group_name = aws_elasticache_subnet_group.test.name + node_type = "cache.m5.large" number_cache_clusters = 1 @@ -2525,6 +2531,8 @@ resource "aws_elasticache_replication_group" "primary" { replication_group_id = "%[1]s-p" replication_group_description = "primary" + subnet_group_name = aws_elasticache_subnet_group.primary.name + node_type = "cache.m5.large" engine = "redis" @@ -2537,12 +2545,16 @@ resource "aws_elasticache_replication_group" "primary" { func testAccAWSElasticacheReplicationGroupConfig_GlobalReplicationGroupId_Basic(rName string) string { return composeConfig( testAccMultipleRegionProviderConfig(2), + testAccElasticacheVpcBaseWithProvider(rName, "test", ProviderNameAws), + testAccElasticacheVpcBaseWithProvider(rName, "primary", ProviderNameAwsAlternate), fmt.Sprintf(` resource "aws_elasticache_replication_group" "test" { replication_group_id = "%[1]s-s" replication_group_description = "secondary" global_replication_group_id = aws_elasticache_global_replication_group.test.global_replication_group_id + subnet_group_name = aws_elasticache_subnet_group.test.name + number_cache_clusters = 1 } @@ -2559,6 +2571,8 @@ resource "aws_elasticache_replication_group" "primary" { replication_group_id = "%[1]s-p" replication_group_description = "primary" + subnet_group_name = aws_elasticache_subnet_group.primary.name + node_type = "cache.m5.large" engine = "redis" From 611926893c10019bcbb08edc525a34d0994c9cb6 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 11 Mar 2021 15:43:01 -0800 Subject: [PATCH 8/8] Adds CHANGELOG --- .changelog/17725.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/17725.txt diff --git a/.changelog/17725.txt b/.changelog/17725.txt new file mode 100644 index 00000000000..3256fcaa2bd --- /dev/null +++ b/.changelog/17725.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_elasticache_replication_group: Allows creating a Replication Group as part of a Global Replication Group +```