From 3ebdec49605a0133ae23a1218f78cfd7f1f2425d Mon Sep 17 00:00:00 2001 From: sebastianc Date: Sat, 25 May 2019 17:35:28 -0700 Subject: [PATCH 1/7] adding proxy configuration to ecs_task_definition. resolves #8253 --- aws/resource_aws_ecs_task_definition.go | 95 +++++++++++ aws/resource_aws_ecs_task_definition_test.go | 151 +++++++++++++++++- .../docs/r/ecs_task_definition.html.markdown | 47 ++++++ 3 files changed, 292 insertions(+), 1 deletion(-) diff --git a/aws/resource_aws_ecs_task_definition.go b/aws/resource_aws_ecs_task_definition.go index d07f6dafe46..ade12d0a599 100644 --- a/aws/resource_aws_ecs_task_definition.go +++ b/aws/resource_aws_ecs_task_definition.go @@ -230,6 +230,70 @@ func resourceAwsEcsTaskDefinition() *schema.Resource { }, false), }, + "proxy_configuration": { + Type: schema.TypeSet, + Optional: true, + ForceNew: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "container_name": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + "properties": { + Type: schema.TypeSet, + Optional: true, + ForceNew: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + "value": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + }, + }, + Set: func(v interface{}) int { + var buf bytes.Buffer + m := v.(map[string]interface{}) + buf.WriteString(fmt.Sprintf("%s-", m["name"].(string))) + buf.WriteString(fmt.Sprintf("%s-", m["value"].(string))) + return hashcode.String(buf.String()) + }, + }, + "type": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ValidateFunc: validation.StringInSlice([]string{ + ecs.ProxyConfigurationTypeAppmesh, + }, false), + }, + }, + }, + Set: func(v interface{}) int { + var buf bytes.Buffer + m := v.(map[string]interface{}) + buf.WriteString(fmt.Sprintf("%s-", m["container_name"].(string))) + buf.WriteString(fmt.Sprintf("%s-", m["type"].(string))) + + rawProps := m["properties"].(*schema.Set).List() + for _, rawProp := range rawProps { + property := rawProp.(map[string]interface{}) + buf.WriteString(fmt.Sprintf("%s-", property["name"].(string))) + buf.WriteString(fmt.Sprintf("%s-", property["value"].(string))) + } + + return hashcode.String(buf.String()) + }, + }, + "tags": tagsSchema(), }, } @@ -321,6 +385,37 @@ func resourceAwsEcsTaskDefinitionCreate(d *schema.ResourceData, meta interface{} input.RequiresCompatibilities = expandStringList(v.(*schema.Set).List()) } + proxyConfigs := d.Get("proxy_configuration").(*schema.Set).List() + if len(proxyConfigs) > 0 { + proxyConfig := proxyConfigs[0] + configMap := proxyConfig.(map[string]interface{}) + + containerName := configMap["container_name"].(string) + proxyType := configMap["type"].(string) + + rawProperties := configMap["properties"].(*schema.Set).List() + + properties := make([]*ecs.KeyValuePair, len(rawProperties)) + + for i, rawProperty := range rawProperties { + propertyMap := rawProperty.(map[string]interface{}) + propertyName := propertyMap["name"].(string) + propertyValue := propertyMap["value"].(string) + + properties[i] = &ecs.KeyValuePair{ + Name: aws.String(propertyName), + Value: aws.String(propertyValue), + } + } + + var ecsProxyConfig ecs.ProxyConfiguration + ecsProxyConfig.ContainerName = aws.String(containerName) + ecsProxyConfig.Type = aws.String(proxyType) + ecsProxyConfig.Properties = properties + + input.ProxyConfiguration = &ecsProxyConfig + } + log.Printf("[DEBUG] Registering ECS task definition: %s", input) out, err := conn.RegisterTaskDefinition(&input) if err != nil { diff --git a/aws/resource_aws_ecs_task_definition_test.go b/aws/resource_aws_ecs_task_definition_test.go index 95401854c51..7a8b6c4af6b 100644 --- a/aws/resource_aws_ecs_task_definition_test.go +++ b/aws/resource_aws_ecs_task_definition_test.go @@ -504,6 +504,156 @@ func TestAccAWSEcsTaskDefinition_Tags(t *testing.T) { }) } +func TestAccAWSEcsTaskDefinition_ProxyConfiguration(t *testing.T){ + var taskDefinition ecs.TaskDefinition + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_ecs_task_definition.test" + + containerName := "web" + proxyType := "APPMESH" + ignoredUid := "1337" + ignoredGid := "999" + appPorts := "80" + proxyIngressPort := "15000" + proxyEgressPort := "15001" + egressIgnoredPorts := "5500" + egressIgnoredIPs := "169.254.170.2,169.254.169.254" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSEcsTaskDefinitionDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSEcsTaskDefinitionConfigProxyConfiguration(rName, containerName, proxyType, ignoredUid, ignoredGid, appPorts, proxyIngressPort, proxyEgressPort, egressIgnoredPorts, egressIgnoredIPs), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEcsTaskDefinitionExists(resourceName, &taskDefinition), + testAccCheckAWSEcsTaskDefinitionProxyConfiguration(&taskDefinition, containerName, proxyType, ignoredUid, ignoredGid, appPorts, proxyIngressPort, proxyEgressPort, egressIgnoredPorts, egressIgnoredIPs), + ), + }, + }, + }) +} + +func testAccAWSEcsTaskDefinitionConfigProxyConfiguration(rName string, containerName string, proxyType string, + ignoredUid string, ignoredGid string, appPorts string, proxyIngressPort string, proxyEgressPort string, + egressIgnoredPorts string, egressIgnoredIPs string) string { + + return fmt.Sprintf(` +resource "aws_ecs_cluster" "test" { + name = %q +} + +resource "aws_ecs_task_definition" "test" { + family = %q + network_mode = "awsvpc" + + proxy_configuration = { + type = %q + container_name = %q + properties = [ + { + name = "IgnoredUID" + value = %q + }, + { + name = "IgnoredGID" + value = %q + }, + { + name = "AppPorts" + value = %q + }, + { + name = "ProxyIngressPort", + value = %q + }, + { + name = "ProxyEgressPort", + value = %q + }, + { + name = "EgressIgnoredPorts" + value = %q + }, + { + name = "EgressIgnoredIPs", + value = %q + } + ] + } + + container_definitions = < Date: Sat, 25 May 2019 17:43:40 -0700 Subject: [PATCH 2/7] gofmt. resolves #8253 --- aws/resource_aws_ecs_task_definition_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/aws/resource_aws_ecs_task_definition_test.go b/aws/resource_aws_ecs_task_definition_test.go index 7a8b6c4af6b..736104cb005 100644 --- a/aws/resource_aws_ecs_task_definition_test.go +++ b/aws/resource_aws_ecs_task_definition_test.go @@ -504,7 +504,7 @@ func TestAccAWSEcsTaskDefinition_Tags(t *testing.T) { }) } -func TestAccAWSEcsTaskDefinition_ProxyConfiguration(t *testing.T){ +func TestAccAWSEcsTaskDefinition_ProxyConfiguration(t *testing.T) { var taskDefinition ecs.TaskDefinition rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_ecs_task_definition.test" @@ -537,7 +537,7 @@ func TestAccAWSEcsTaskDefinition_ProxyConfiguration(t *testing.T){ func testAccAWSEcsTaskDefinitionConfigProxyConfiguration(rName string, containerName string, proxyType string, ignoredUid string, ignoredGid string, appPorts string, proxyIngressPort string, proxyEgressPort string, - egressIgnoredPorts string, egressIgnoredIPs string) string { + egressIgnoredPorts string, egressIgnoredIPs string) string { return fmt.Sprintf(` resource "aws_ecs_cluster" "test" { @@ -618,7 +618,7 @@ func testAccCheckAWSEcsTaskDefinitionProxyConfiguration(after *ecs.TaskDefinitio } propertyLookups := make(map[string]string) - for _, property := range properties { + for _, property := range properties { propertyLookups[*property.Name] = *property.Value } From 26e6797b3dde798a8082df1360aab0eebcc227e1 Mon Sep 17 00:00:00 2001 From: sebastianc Date: Fri, 31 May 2019 10:48:14 -0700 Subject: [PATCH 3/7] changing use of schema.TypeSet to schema.TypeList per contribution guidelines. resolves #8253 --- aws/resource_aws_ecs_task_definition.go | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/aws/resource_aws_ecs_task_definition.go b/aws/resource_aws_ecs_task_definition.go index ade12d0a599..1e2a52ef291 100644 --- a/aws/resource_aws_ecs_task_definition.go +++ b/aws/resource_aws_ecs_task_definition.go @@ -231,7 +231,8 @@ func resourceAwsEcsTaskDefinition() *schema.Resource { }, "proxy_configuration": { - Type: schema.TypeSet, + Type: schema.TypeList, + MaxItems: 1, Optional: true, ForceNew: true, Elem: &schema.Resource{ @@ -277,21 +278,6 @@ func resourceAwsEcsTaskDefinition() *schema.Resource { }, }, }, - Set: func(v interface{}) int { - var buf bytes.Buffer - m := v.(map[string]interface{}) - buf.WriteString(fmt.Sprintf("%s-", m["container_name"].(string))) - buf.WriteString(fmt.Sprintf("%s-", m["type"].(string))) - - rawProps := m["properties"].(*schema.Set).List() - for _, rawProp := range rawProps { - property := rawProp.(map[string]interface{}) - buf.WriteString(fmt.Sprintf("%s-", property["name"].(string))) - buf.WriteString(fmt.Sprintf("%s-", property["value"].(string))) - } - - return hashcode.String(buf.String()) - }, }, "tags": tagsSchema(), @@ -385,7 +371,7 @@ func resourceAwsEcsTaskDefinitionCreate(d *schema.ResourceData, meta interface{} input.RequiresCompatibilities = expandStringList(v.(*schema.Set).List()) } - proxyConfigs := d.Get("proxy_configuration").(*schema.Set).List() + proxyConfigs := d.Get("proxy_configuration").([]interface{}) if len(proxyConfigs) > 0 { proxyConfig := proxyConfigs[0] configMap := proxyConfig.(map[string]interface{}) From d005634a360e4889b23e5c2e4c86f09694057bac Mon Sep 17 00:00:00 2001 From: sebastianc Date: Wed, 12 Jun 2019 16:31:04 -0700 Subject: [PATCH 4/7] first pass of code review items #8253 --- aws/resource_aws_ecs_task_definition.go | 40 +++--------- aws/resource_aws_ecs_task_definition_test.go | 43 ++++--------- .../docs/r/ecs_task_definition.html.markdown | 64 +++++++------------ 3 files changed, 42 insertions(+), 105 deletions(-) diff --git a/aws/resource_aws_ecs_task_definition.go b/aws/resource_aws_ecs_task_definition.go index 1e2a52ef291..1c8bec69454 100644 --- a/aws/resource_aws_ecs_task_definition.go +++ b/aws/resource_aws_ecs_task_definition.go @@ -243,33 +243,14 @@ func resourceAwsEcsTaskDefinition() *schema.Resource { ForceNew: true, }, "properties": { - Type: schema.TypeSet, + Type: schema.TypeMap, + Elem: &schema.Schema{Type: schema.TypeString}, Optional: true, ForceNew: true, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "name": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - }, - "value": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - }, - }, - }, - Set: func(v interface{}) int { - var buf bytes.Buffer - m := v.(map[string]interface{}) - buf.WriteString(fmt.Sprintf("%s-", m["name"].(string))) - buf.WriteString(fmt.Sprintf("%s-", m["value"].(string))) - return hashcode.String(buf.String()) - }, }, "type": { Type: schema.TypeString, + Default: ecs.ProxyConfigurationTypeAppmesh, Optional: true, ForceNew: true, ValidateFunc: validation.StringInSlice([]string{ @@ -379,19 +360,16 @@ func resourceAwsEcsTaskDefinitionCreate(d *schema.ResourceData, meta interface{} containerName := configMap["container_name"].(string) proxyType := configMap["type"].(string) - rawProperties := configMap["properties"].(*schema.Set).List() + rawProperties := configMap["properties"].(map[string]interface{}) properties := make([]*ecs.KeyValuePair, len(rawProperties)) - - for i, rawProperty := range rawProperties { - propertyMap := rawProperty.(map[string]interface{}) - propertyName := propertyMap["name"].(string) - propertyValue := propertyMap["value"].(string) - + i := 0 + for name, value := range rawProperties { properties[i] = &ecs.KeyValuePair{ - Name: aws.String(propertyName), - Value: aws.String(propertyValue), + Name: aws.String(name), + Value: aws.String(value.(string)), } + i++ } var ecsProxyConfig ecs.ProxyConfiguration diff --git a/aws/resource_aws_ecs_task_definition_test.go b/aws/resource_aws_ecs_task_definition_test.go index 736104cb005..fcef09bcd19 100644 --- a/aws/resource_aws_ecs_task_definition_test.go +++ b/aws/resource_aws_ecs_task_definition_test.go @@ -548,39 +548,18 @@ resource "aws_ecs_task_definition" "test" { family = %q network_mode = "awsvpc" - proxy_configuration = { + proxy_configuration { type = %q container_name = %q - properties = [ - { - name = "IgnoredUID" - value = %q - }, - { - name = "IgnoredGID" - value = %q - }, - { - name = "AppPorts" - value = %q - }, - { - name = "ProxyIngressPort", - value = %q - }, - { - name = "ProxyEgressPort", - value = %q - }, - { - name = "EgressIgnoredPorts" - value = %q - }, - { - name = "EgressIgnoredIPs", - value = %q - } - ] + properties = { + IgnoredUID = %q + IgnoredGID = %q + AppPorts = %q + ProxyIngressPort = %q + ProxyEgressPort = %q + EgressIgnoredPorts = %q + EgressIgnoredIPs = %q + } } container_definitions = < Date: Wed, 12 Jun 2019 16:59:14 -0700 Subject: [PATCH 5/7] converting sdk ProxyConfiguration to tf configuration --- aws/resource_aws_ecs_task_definition.go | 26 +++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/aws/resource_aws_ecs_task_definition.go b/aws/resource_aws_ecs_task_definition.go index 1c8bec69454..405bd5e8566 100644 --- a/aws/resource_aws_ecs_task_definition.go +++ b/aws/resource_aws_ecs_task_definition.go @@ -455,6 +455,10 @@ func resourceAwsEcsTaskDefinitionRead(d *schema.ResourceData, meta interface{}) return err } + if err := d.Set("proxy_configuration", flattenProxyConfiguration(taskDefinition.ProxyConfiguration)); err != nil { + return err + } + return nil } @@ -472,6 +476,28 @@ func flattenPlacementConstraints(pcs []*ecs.TaskDefinitionPlacementConstraint) [ return results } +func flattenProxyConfiguration(pc *ecs.ProxyConfiguration) []map[string]interface{} { + if pc == nil { + return nil + } + + meshProperties := make(map[string]string) + if pc.Properties != nil { + for _, prop := range pc.Properties { + meshProperties[*prop.Name] = *prop.Value + } + } + + config := make(map[string]interface{}) + config["container_name"] = *pc.ContainerName + config["type"] = *pc.Type + config["properties"] = meshProperties + + return []map[string]interface{} { + config, + } +} + func resourceAwsEcsTaskDefinitionUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ecsconn From 67ccad6ce3c79aab469e930008529c363808589b Mon Sep 17 00:00:00 2001 From: sebastianc Date: Wed, 12 Jun 2019 17:02:23 -0700 Subject: [PATCH 6/7] go fmt --- aws/resource_aws_ecs_task_definition.go | 2 +- aws/resource_aws_ecs_task_definition_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_ecs_task_definition.go b/aws/resource_aws_ecs_task_definition.go index 405bd5e8566..4e2cf2aa6e2 100644 --- a/aws/resource_aws_ecs_task_definition.go +++ b/aws/resource_aws_ecs_task_definition.go @@ -493,7 +493,7 @@ func flattenProxyConfiguration(pc *ecs.ProxyConfiguration) []map[string]interfac config["type"] = *pc.Type config["properties"] = meshProperties - return []map[string]interface{} { + return []map[string]interface{}{ config, } } diff --git a/aws/resource_aws_ecs_task_definition_test.go b/aws/resource_aws_ecs_task_definition_test.go index fcef09bcd19..f0e03126330 100644 --- a/aws/resource_aws_ecs_task_definition_test.go +++ b/aws/resource_aws_ecs_task_definition_test.go @@ -575,7 +575,7 @@ resource "aws_ecs_task_definition" "test" { DEFINITION } -`, rName, rName, proxyType, containerName, ignoredUid, ignoredGid, appPorts, proxyIngressPort, proxyEgressPort, egressIgnoredPorts, egressIgnoredIPs, containerName) +`, rName, rName, proxyType, containerName, ignoredUid, ignoredGid, appPorts, proxyIngressPort, proxyEgressPort, egressIgnoredPorts, egressIgnoredIPs, containerName) } func testAccCheckAWSEcsTaskDefinitionProxyConfiguration(after *ecs.TaskDefinition, containerName string, proxyType string, From fdaadee675c03325a125ec34b43dae3082735115 Mon Sep 17 00:00:00 2001 From: sebastianc Date: Wed, 19 Jun 2019 15:31:12 -0700 Subject: [PATCH 7/7] updates from code review --- aws/resource_aws_ecs_task_definition.go | 10 +++++----- aws/resource_aws_ecs_task_definition_test.go | 6 ++++++ website/docs/r/ecs_task_definition.html.markdown | 12 ++++++------ 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/aws/resource_aws_ecs_task_definition.go b/aws/resource_aws_ecs_task_definition.go index 4e2cf2aa6e2..f0b12b63c25 100644 --- a/aws/resource_aws_ecs_task_definition.go +++ b/aws/resource_aws_ecs_task_definition.go @@ -452,11 +452,11 @@ func resourceAwsEcsTaskDefinitionRead(d *schema.ResourceData, meta interface{}) } if err := d.Set("requires_compatibilities", flattenStringList(taskDefinition.RequiresCompatibilities)); err != nil { - return err + return fmt.Errorf("error setting requires_compatibilities: %s", err) } if err := d.Set("proxy_configuration", flattenProxyConfiguration(taskDefinition.ProxyConfiguration)); err != nil { - return err + return fmt.Errorf("error setting proxy_configuration: %s", err) } return nil @@ -484,13 +484,13 @@ func flattenProxyConfiguration(pc *ecs.ProxyConfiguration) []map[string]interfac meshProperties := make(map[string]string) if pc.Properties != nil { for _, prop := range pc.Properties { - meshProperties[*prop.Name] = *prop.Value + meshProperties[aws.StringValue(prop.Name)] = aws.StringValue(prop.Value) } } config := make(map[string]interface{}) - config["container_name"] = *pc.ContainerName - config["type"] = *pc.Type + config["container_name"] = aws.StringValue(pc.ContainerName) + config["type"] = aws.StringValue(pc.Type) config["properties"] = meshProperties return []map[string]interface{}{ diff --git a/aws/resource_aws_ecs_task_definition_test.go b/aws/resource_aws_ecs_task_definition_test.go index f0e03126330..d4f4a1bc325 100644 --- a/aws/resource_aws_ecs_task_definition_test.go +++ b/aws/resource_aws_ecs_task_definition_test.go @@ -531,6 +531,12 @@ func TestAccAWSEcsTaskDefinition_ProxyConfiguration(t *testing.T) { testAccCheckAWSEcsTaskDefinitionProxyConfiguration(&taskDefinition, containerName, proxyType, ignoredUid, ignoredGid, appPorts, proxyIngressPort, proxyEgressPort, egressIgnoredPorts, egressIgnoredIPs), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSEcsTaskDefinitionImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, }, }) } diff --git a/website/docs/r/ecs_task_definition.html.markdown b/website/docs/r/ecs_task_definition.html.markdown index 10549e9d994..f2f1ab407a4 100644 --- a/website/docs/r/ecs_task_definition.html.markdown +++ b/website/docs/r/ecs_task_definition.html.markdown @@ -73,14 +73,14 @@ resource "aws_ecs_task_definition" "service" { container_definitions = "${file("task-definitions/service.json")}" proxy_configuration { - type = "APPMESH" + type = "APPMESH" container_name = "applicationContainerName" properties = { - IgnoredUID = "1337" - AppPorts = "8080" - ProxyIngressPort = 15000 - ProxyEgressPort = 15001 + AppPorts = "8080" EgressIgnoredIPs = "169.254.170.2,169.254.169.254" + IgnoredUID = "1337" + ProxyEgressPort = 15001 + ProxyIngressPort = 15000 } } } @@ -158,9 +158,9 @@ Guide](http://docs.aws.amazon.com/AmazonECS/latest/developerguide/cluster-query- #### Proxy Configuration Arguments -* `type` - (Optional) The proxy type. The only supported value is `APPMESH`. * `container_name` - (Required) The name of the container that will serve as the App Mesh proxy. * `properties` - (Required) The set of network configuration parameters to provide the Container Network Interface (CNI) plugin, specified a key-value mapping. +* `type` - (Optional) The proxy type. The default value is `APPMESH`. The only supported value is `APPMESH`. ## Attributes Reference