Merge pull request #3835 from terraform-providers/f-aws_elasticache-o…

…ptional-port resource/aws_elasticache_*: Allow port to be optional
hashicorp · Mar 26, 2018 · 2ac0b83 · 2ac0b83
2 parents 0b7a4ff + 456536d
commit 2ac0b83
Show file tree

Hide file tree

Showing 6 changed files with 155 additions and 56 deletions.
diff --git a/aws/import_aws_elasticache_cluster_test.go b/aws/import_aws_elasticache_cluster_test.go
diff --git a/aws/resource_aws_elasticache_cluster.go b/aws/resource_aws_elasticache_cluster.go
@@ -105,8 +105,15 @@ func resourceAwsElastiCacheCommonSchema() map[string]*schema.Schema {
 		},
 		"port": {
 			Type:     schema.TypeInt,
-			Required: true,
+			Optional: true,
 			ForceNew: true,
+			DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
+				// Supress default memcached/redis ports when not defined
+				if !d.IsNewResource() && new == "0" && (old == "6379" || old == "11211") {
+					return true
+				}
+				return false
+			},
 		},
 		"notification_topic_arn": {
 			Type:     schema.TypeString,
@@ -307,7 +314,6 @@ func resourceAwsElasticacheClusterCreate(d *schema.ResourceData, meta interface{
 	numNodes := int64(d.Get("num_cache_nodes").(int)) // 2
 	engine := d.Get("engine").(string)                // memcached
 	engineVersion := d.Get("engine_version").(string) // 1.4.14
-	port := int64(d.Get("port").(int))                // e.g) 11211
 	subnetGroupName := d.Get("subnet_group_name").(string)
 	securityNameSet := d.Get("security_group_names").(*schema.Set)
 	securityIdSet := d.Get("security_group_ids").(*schema.Set)
@@ -322,7 +328,6 @@ func resourceAwsElasticacheClusterCreate(d *schema.ResourceData, meta interface{
 		NumCacheNodes:           aws.Int64(numNodes),
 		Engine:                  aws.String(engine),
 		EngineVersion:           aws.String(engineVersion),
-		Port:                    aws.Int64(port),
 		CacheSubnetGroupName:    aws.String(subnetGroupName),
 		CacheSecurityGroupNames: securityNames,
 		SecurityGroupIds:        securityIds,
@@ -334,6 +339,10 @@ func resourceAwsElasticacheClusterCreate(d *schema.ResourceData, meta interface{
 		req.CacheParameterGroupName = aws.String(v.(string))
 	}
 
+	if v, ok := d.GetOk("port"); ok {
+		req.Port = aws.Int64(int64(v.(int)))
+	}
+
 	if v, ok := d.GetOk("snapshot_retention_limit"); ok {
 		req.SnapshotRetentionLimit = aws.Int64(int64(v.(int)))
 	}
@@ -438,6 +447,8 @@ func resourceAwsElasticacheClusterRead(d *schema.ResourceData, meta interface{})
 			d.Set("port", c.ConfigurationEndpoint.Port)
 			d.Set("configuration_endpoint", aws.String(fmt.Sprintf("%s:%d", *c.ConfigurationEndpoint.Address, *c.ConfigurationEndpoint.Port)))
 			d.Set("cluster_address", aws.String(fmt.Sprintf("%s", *c.ConfigurationEndpoint.Address)))
+		} else if len(c.CacheNodes) > 0 {
+			d.Set("port", int(aws.Int64Value(c.CacheNodes[0].Endpoint.Port)))
 		}
 
 		if c.ReplicationGroupId != nil {

diff --git a/aws/resource_aws_elasticache_cluster_test.go b/aws/resource_aws_elasticache_cluster_test.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"regexp"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -16,15 +17,116 @@ import (
 	"github.com/hashicorp/terraform/terraform"
 )
 
-func TestAccAWSElasticacheCluster_basic(t *testing.T) {
+func TestAccAWSElasticacheCluster_Engine_Memcached_Ec2Classic(t *testing.T) {
+	oldvar := os.Getenv("AWS_DEFAULT_REGION")
+	os.Setenv("AWS_DEFAULT_REGION", "us-east-1")
+	defer os.Setenv("AWS_DEFAULT_REGION", oldvar)
+
+	var ec elasticache.CacheCluster
+	rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(8))
+	resourceName := "aws_elasticache_cluster.bar"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSElasticacheClusterDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSElasticacheClusterConfig_Engine_Memcached(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSElasticacheClusterExists(resourceName, &ec),
+					resource.TestCheckResourceAttr(resourceName, "cache_nodes.0.id", "0001"),
+					resource.TestCheckResourceAttrSet(resourceName, "configuration_endpoint"),
+					resource.TestCheckResourceAttrSet(resourceName, "cluster_address"),
+					resource.TestCheckResourceAttr(resourceName, "engine", "memcached"),
+					resource.TestCheckResourceAttr(resourceName, "port", "11211"),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestAccAWSElasticacheCluster_Engine_Redis_Ec2Classic(t *testing.T) {
+	oldvar := os.Getenv("AWS_DEFAULT_REGION")
+	os.Setenv("AWS_DEFAULT_REGION", "us-east-1")
+	defer os.Setenv("AWS_DEFAULT_REGION", oldvar)
+
+	var ec elasticache.CacheCluster
+	rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(8))
+	resourceName := "aws_elasticache_cluster.bar"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSElasticacheClusterDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSElasticacheClusterConfig_Engine_Redis(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSElasticacheClusterExists(resourceName, &ec),
+					resource.TestCheckResourceAttr(resourceName, "cache_nodes.0.id", "0001"),
+					resource.TestCheckResourceAttr(resourceName, "engine", "redis"),
+					resource.TestCheckResourceAttr(resourceName, "port", "6379"),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestAccAWSElasticacheCluster_Port_Ec2Classic(t *testing.T) {
+	oldvar := os.Getenv("AWS_DEFAULT_REGION")
+	os.Setenv("AWS_DEFAULT_REGION", "us-east-1")
+	defer os.Setenv("AWS_DEFAULT_REGION", oldvar)
+
+	var ec elasticache.CacheCluster
+	port := 11212
+	rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(8))
+	resourceName := "aws_elasticache_cluster.bar"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t); testAccEC2ClassicPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSElasticacheClusterDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSElasticacheClusterConfig_Port(rName, port),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSElasticacheClusterExists(resourceName, &ec),
+					resource.TestCheckResourceAttr(resourceName, "cache_nodes.0.id", "0001"),
+					resource.TestCheckResourceAttrSet(resourceName, "configuration_endpoint"),
+					resource.TestCheckResourceAttrSet(resourceName, "cluster_address"),
+					resource.TestCheckResourceAttr(resourceName, "engine", "memcached"),
+					resource.TestCheckResourceAttr(resourceName, "port", strconv.Itoa(port)),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestAccAWSElasticacheCluster_SecurityGroup(t *testing.T) {
 	var ec elasticache.CacheCluster
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckAWSElasticacheClusterDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccAWSElasticacheClusterConfig,
+				Config: testAccAWSElasticacheClusterConfig_SecurityGroup,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSElasticacheSecurityGroupExists("aws_elasticache_security_group.bar"),
 					testAccCheckAWSElasticacheClusterExists("aws_elasticache_cluster.bar", &ec),
@@ -504,24 +606,44 @@ func testAccCheckAWSElasticacheClusterExists(n string, v *elasticache.CacheClust
 	}
 }
 
-func testAccAWSElasticacheClusterConfigBasic(clusterId string) string {
+func testAccAWSElasticacheClusterConfig_Engine_Memcached(rName string) string {
 	return fmt.Sprintf(`
-provider "aws" {
-	region = "us-east-1"
+resource "aws_elasticache_cluster" "bar" {
+  cluster_id           = "%s"
+  engine               = "memcached"
+  node_type            = "cache.m1.small"
+  num_cache_nodes      = 1
+  parameter_group_name = "default.memcached1.4"
+}
+`, rName)
 }
 
+func testAccAWSElasticacheClusterConfig_Engine_Redis(rName string) string {
+	return fmt.Sprintf(`
 resource "aws_elasticache_cluster" "bar" {
-    cluster_id = "tf-%s"
-    engine = "memcached"
-    node_type = "cache.m1.small"
-    num_cache_nodes = 1
-    port = 11211
-    parameter_group_name = "default.memcached1.4"
+  cluster_id           = "%s"
+  engine               = "redis"
+  node_type            = "cache.m1.small"
+  num_cache_nodes      = 1
+  parameter_group_name = "default.redis3.2"
+}
+`, rName)
+}
+
+func testAccAWSElasticacheClusterConfig_Port(rName string, port int) string {
+	return fmt.Sprintf(`
+resource "aws_elasticache_cluster" "bar" {
+  cluster_id           = "%s"
+  engine               = "memcached"
+  node_type            = "cache.m1.small"
+  num_cache_nodes      = 1
+  parameter_group_name = "default.memcached1.4"
+  port                 = %d
 }
-`, clusterId)
+`, rName, port)
 }
 
-var testAccAWSElasticacheClusterConfig = fmt.Sprintf(`
+var testAccAWSElasticacheClusterConfig_SecurityGroup = fmt.Sprintf(`
 provider "aws" {
 	region = "us-east-1"
 }

diff --git a/aws/resource_aws_elasticache_replication_group.go b/aws/resource_aws_elasticache_replication_group.go
@@ -133,7 +133,6 @@ func resourceAwsElasticacheReplicationGroupCreate(d *schema.ResourceData, meta i
 		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)),
-		Port:                        aws.Int64(int64(d.Get("port").(int))),
 		Tags:                        tags,
 	}
 
@@ -151,6 +150,10 @@ func resourceAwsElasticacheReplicationGroupCreate(d *schema.ResourceData, meta i
 		params.CacheParameterGroupName = aws.String(v.(string))
 	}
 
+	if v, ok := d.GetOk("port"); ok {
+		params.Port = aws.Int64(int64(v.(int)))
+	}
+
 	if v, ok := d.GetOk("subnet_group_name"); ok {
 		params.CacheSubnetGroupName = aws.String(v.(string))
 	}

diff --git a/website/docs/r/elasticache_cluster.html.markdown b/website/docs/r/elasticache_cluster.html.markdown
@@ -64,8 +64,7 @@ the highest numbered nodes will be removed.
 * `parameter_group_name` – (Required) Name of the parameter group to associate
 with this cache cluster
 
-* `port` – (Required) The port number on which each of the cache nodes will
-accept connections. For Memcache the default is 11211, and for Redis the default port is 6379.
+* `port` – (Optional) The port number on which each of the cache nodes will accept connections. For Memcache the default is 11211, and for Redis the default port is 6379.
 
 * `subnet_group_name` – (Optional, VPC only) Name of the subnet group to be used
 for the cache cluster.

diff --git a/website/docs/r/elasticache_replication_group.html.markdown b/website/docs/r/elasticache_replication_group.html.markdown
@@ -69,7 +69,7 @@ The following arguments are supported:
 * `auth_token` - (Optional) The password used to access a password protected server. Can be specified only if `transit_encryption_enabled = true`.
 * `engine_version` - (Optional) The version number of the cache engine to be used for the cache clusters in this replication group.
 * `parameter_group_name` - (Optional) The name of the parameter group to associate with this replication group. If this argument is omitted, the default cache parameter group for the specified engine is used.
-* `port` – (Required) The port number on which each of the cache nodes will accept connections. For Memcache the default is 11211, and for Redis the default port is 6379.
+* `port` – (Optional) The port number on which each of the cache nodes will accept connections. For Memcache the default is 11211, and for Redis the default port is 6379.
 * `subnet_group_name` - (Optional) The name of the cache subnet group to be used for the replication group.
 * `security_group_names` - (Optional) A list of cache security group names to associate with this replication group.
 * `security_group_ids` - (Optional) One or more Amazon VPC security groups associated with this replication group. Use this parameter only when you are creating a replication group in an Amazon Virtual Private Cloud