Merge pull request #3668 from TimeIncOSS/b-aws-ecs-cluster-arn

provider/aws: Allow cluster name, not only ARN for aws_ecs_service
hashicorp · Oct 28, 2015 · 91f4be2 · 91f4be2
2 parents af04321 + 784aadd
commit 91f4be2
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 5 deletions.
diff --git a/builtin/providers/aws/resource_aws_ecs_service.go b/builtin/providers/aws/resource_aws_ecs_service.go
@@ -137,7 +137,6 @@ func resourceAwsEcsServiceCreate(d *schema.ResourceData, meta interface{}) error
 
 	log.Printf("[DEBUG] ECS service created: %s", *service.ServiceArn)
 	d.SetId(*service.ServiceArn)
-	d.Set("cluster", *service.ClusterArn)
 
 	return resourceAwsEcsServiceUpdate(d, meta)
 }
@@ -175,14 +174,21 @@ func resourceAwsEcsServiceRead(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	d.Set("desired_count", *service.DesiredCount)
-	d.Set("cluster", *service.ClusterArn)
+
+	// Save cluster in the same format
+	if strings.HasPrefix(d.Get("cluster").(string), "arn:aws:ecs:") {
+		d.Set("cluster", *service.ClusterArn)
+	} else {
+		clusterARN := getNameFromARN(*service.ClusterArn)
+		d.Set("cluster", clusterARN)
+	}
 
 	// Save IAM role in the same format
 	if service.RoleArn != nil {
 		if strings.HasPrefix(d.Get("iam_role").(string), "arn:aws:iam:") {
 			d.Set("iam_role", *service.RoleArn)
 		} else {
-			roleARN := buildIamRoleNameFromARN(*service.RoleArn)
+			roleARN := getNameFromARN(*service.RoleArn)
 			d.Set("iam_role", roleARN)
 		}
 	}
@@ -306,8 +312,10 @@ func buildFamilyAndRevisionFromARN(arn string) string {
 	return strings.Split(arn, "/")[1]
 }
 
-func buildIamRoleNameFromARN(arn string) string {
-	// arn:aws:iam::0123456789:role/EcsService
+// Expects the following ARNs:
+// arn:aws:iam::0123456789:role/EcsService
+// arn:aws:ecs:us-west-2:0123456789:cluster/radek-cluster
+func getNameFromARN(arn string) string {
 	return strings.Split(arn, "/")[1]
 }
 

diff --git a/builtin/providers/aws/resource_aws_ecs_service_test.go b/builtin/providers/aws/resource_aws_ecs_service_test.go
@@ -178,6 +178,26 @@ func TestAccAWSEcsService_withIamRole(t *testing.T) {
 	})
 }
 
+// Regression for https://github.com/hashicorp/terraform/issues/3361
+func TestAccAWSEcsService_withEcsClusterName(t *testing.T) {
+	clusterName := regexp.MustCompile("^terraformecstestcluster$")
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSEcsServiceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccAWSEcsServiceWithEcsClusterName,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSEcsServiceExists("aws_ecs_service.jenkins"),
+					resource.TestMatchResourceAttr(
+						"aws_ecs_service.jenkins", "cluster", clusterName),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckAWSEcsServiceDestroy(s *terraform.State) error {
 	conn := testAccProvider.Meta().(*AWSClient).ecsconn
 
@@ -471,3 +491,31 @@ resource "aws_ecs_service" "ghost" {
   desired_count = 1
 }
 `
+
+var testAccAWSEcsServiceWithEcsClusterName = `
+resource "aws_ecs_cluster" "default" {
+	name = "terraformecstestcluster"
+}
+
+resource "aws_ecs_task_definition" "jenkins" {
+  family = "jenkins"
+  container_definitions = <<DEFINITION
+[
+  {
+    "cpu": 128,
+    "essential": true,
+    "image": "jenkins:latest",
+    "memory": 128,
+    "name": "jenkins"
+  }
+]
+DEFINITION
+}
+
+resource "aws_ecs_service" "jenkins" {
+  name = "jenkins"
+  cluster = "${aws_ecs_cluster.default.name}"
+  task_definition = "${aws_ecs_task_definition.jenkins.arn}"
+  desired_count = 1
+}
+`