From e93f46765d2dd9bc8eef1cb52c8fef029e8baf6c Mon Sep 17 00:00:00 2001
From: Jake Champlin <jake@gnu.space>
Date: Sun, 29 Jan 2017 11:42:50 -0500
Subject: [PATCH] provider/aws: ECS Placement constraints fix (#11475)

* fixing AWS ECS placement constraints

* correcting AWS ECS task definition doc

* reverting unnecessary change to resource_aws_ecs_task_definition

* provider/aws: ECS Placement constraints fix

Expands upon #11446 from @bgetsug

Adds:
 - Acceptance Test
 - Improves `nil` check on constraint expression

 Fixes: #10968
---
 .../providers/aws/resource_aws_ecs_service.go | 17 +++++--
 .../aws/resource_aws_ecs_service_test.go      | 46 +++++++++++++++++++
 .../aws/r/ecs_task_definition.html.markdown   |  6 +--
 3 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/builtin/providers/aws/resource_aws_ecs_service.go b/builtin/providers/aws/resource_aws_ecs_service.go
index a177eacf4deb..842deb4daf82 100644
--- a/builtin/providers/aws/resource_aws_ecs_service.go
+++ b/builtin/providers/aws/resource_aws_ecs_service.go
@@ -202,10 +202,14 @@ func resourceAwsEcsServiceCreate(d *schema.ResourceData, meta interface{}) error
 			if err := validateAwsEcsPlacementConstraint(t, e); err != nil {
 				return err
 			}
-			pc = append(pc, &ecs.PlacementConstraint{
-				Type:       aws.String(t),
-				Expression: aws.String(e),
-			})
+			constraint := &ecs.PlacementConstraint{
+				Type: aws.String(t),
+			}
+			if e != "" {
+				constraint.Expression = aws.String(e)
+			}
+
+			pc = append(pc, constraint)
 		}
 		input.PlacementConstraints = pc
 	}
@@ -336,7 +340,10 @@ func flattenServicePlacementConstraints(pcs []*ecs.PlacementConstraint) []map[st
 	for _, pc := range pcs {
 		c := make(map[string]interface{})
 		c["type"] = *pc.Type
-		c["expression"] = *pc.Expression
+		if pc.Expression != nil {
+			c["expression"] = *pc.Expression
+		}
+
 		results = append(results, c)
 	}
 	return results
diff --git a/builtin/providers/aws/resource_aws_ecs_service_test.go b/builtin/providers/aws/resource_aws_ecs_service_test.go
index 2f4cfd320661..3c1bd2b2968d 100644
--- a/builtin/providers/aws/resource_aws_ecs_service_test.go
+++ b/builtin/providers/aws/resource_aws_ecs_service_test.go
@@ -298,6 +298,23 @@ func TestAccAWSEcsServiceWithPlacementConstraints(t *testing.T) {
 	})
 }
 
+func TestAccAWSEcsServiceWithPlacementConstraints_emptyExpression(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSEcsServiceDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSEcsServiceWithPlacementConstraintEmptyExpression,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSEcsServiceExists("aws_ecs_service.mongo"),
+					resource.TestCheckResourceAttr("aws_ecs_service.mongo", "placement_constraints.#", "1"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckAWSEcsServiceDestroy(s *terraform.State) error {
 	conn := testAccProvider.Meta().(*AWSClient).ecsconn
 
@@ -465,6 +482,35 @@ resource "aws_ecs_service" "mongo" {
 }
 `
 
+var testAccAWSEcsServiceWithPlacementConstraintEmptyExpression = `
+resource "aws_ecs_cluster" "default" {
+	name = "terraformecstest212"
+}
+resource "aws_ecs_task_definition" "mongo" {
+  family = "mongodb"
+  container_definitions = <<DEFINITION
+[
+  {
+    "cpu": 128,
+    "essential": true,
+    "image": "mongo:latest",
+    "memory": 128,
+    "name": "mongodb"
+  }
+]
+DEFINITION
+}
+resource "aws_ecs_service" "mongo" {
+  name = "mongodb"
+  cluster = "${aws_ecs_cluster.default.id}"
+  task_definition = "${aws_ecs_task_definition.mongo.arn}"
+  desired_count = 1
+  placement_constraints {
+	  type = "distinctInstance"
+  }
+}
+`
+
 var testAccAWSEcsService_withIamRole = `
 resource "aws_ecs_cluster" "main" {
 	name = "terraformecstest11"
diff --git a/website/source/docs/providers/aws/r/ecs_task_definition.html.markdown b/website/source/docs/providers/aws/r/ecs_task_definition.html.markdown
index d8669d57d3a9..b70660d7afe3 100644
--- a/website/source/docs/providers/aws/r/ecs_task_definition.html.markdown
+++ b/website/source/docs/providers/aws/r/ecs_task_definition.html.markdown
@@ -92,9 +92,9 @@ parameter of container definition in the `mountPoints` section.
 
 `placement_constraints` support the following:
 
-* `type` - (Required) The type of constraint. The only valid values at this time are `memberOf` or `distinctInstance`.
-* `expression` -  (Optional) Cluster Query Language expression to apply to the constraint.  Does not need to be specified
-for the `distinctInstance` type.
+* `type` - (Required) The type of constraint. Use `memberOf` to restrict selection to a group of valid candidates.
+Note that `distinctInstance` is not supported in task definitions.
+* `expression` -  (Optional) Cluster Query Language expression to apply to the constraint.
 For more information, see [Cluster Query Language in the Amazon EC2 Container
 Service Developer
 Guide](http://docs.aws.amazon.com/AmazonECS/latest/developerguide/cluster-query-language.html).