Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/fargate support v2 #2559

Merged
merged 13 commits into from
Feb 9, 2018
22 changes: 19 additions & 3 deletions aws/resource_aws_ecs_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ func resourceAwsEcsService() *schema.Resource {
Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
},
"assign_public_ip": {
Type: schema.TypeBool,
Optional: true,
Default: false,
},
},
},
},
Expand Down Expand Up @@ -410,9 +415,14 @@ func flattenEcsNetworkConfigration(nc *ecs.NetworkConfiguration) []interface{} {
if nc == nil {
return nil
}

result := make(map[string]interface{})
result["security_groups"] = schema.NewSet(schema.HashString, flattenStringList(nc.AwsvpcConfiguration.SecurityGroups))
result["subnets"] = schema.NewSet(schema.HashString, flattenStringList(nc.AwsvpcConfiguration.Subnets))
result["assign_public_ip"] = "true"
if *nc.AwsvpcConfiguration.AssignPublicIp == ecs.AssignPublicIpDisabled {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If nc.AwsvpcConfiguration.AssignPublicIp is nil (its listed as optional in the ECS API documentation), this can cause Terraform to crash. I can fix this (and clean it up a little since it will automatically default to "false") prior to merge via:

if nc.AwsvpcConfiguration.AssignPublicIp != nil {
	result["assign_public_ip"] = fmt.Sprintf("%v", *nc.AwsvpcConfiguration.AssignPublicIp == ecs.AssignPublicIpEnabled)
}

result["assign_public_ip"] = "false"
}
return []interface{}{result}
}

Expand All @@ -426,6 +436,13 @@ func expandEcsNetworkConfigration(nc []interface{}) *ecs.NetworkConfiguration {
awsVpcConfig.SecurityGroups = expandStringSet(val.(*schema.Set))
}
awsVpcConfig.Subnets = expandStringSet(raw["subnets"].(*schema.Set))
if val, ok := raw["assign_public_ip"].(bool); ok {
awsVpcConfig.AssignPublicIp = aws.String(ecs.AssignPublicIpDisabled)
if val {
awsVpcConfig.AssignPublicIp = aws.String(ecs.AssignPublicIpEnabled)
}
}

return &ecs.NetworkConfiguration{AwsvpcConfiguration: awsVpcConfig}
}

Expand Down Expand Up @@ -495,9 +512,8 @@ func resourceAwsEcsServiceUpdate(d *schema.ResourceData, meta interface{}) error
}
}

if d.HasChange("network_configration") {
input.NetworkConfiguration = expandEcsNetworkConfigration(d.Get("network_configuration").([]interface{}))
}
//d.HasChange("network_configration") is not working, so explicity calling method.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why the the func HadChange is not detecting a change on "network_configuration". Maybe we need to implement equals on the structure?

input.NetworkConfiguration = expandEcsNetworkConfigration(d.Get("network_configuration").([]interface{}))

// Retry due to IAM & ECS eventual consistency
err := resource.Retry(2*time.Minute, func() *resource.RetryError {
Expand Down
33 changes: 29 additions & 4 deletions aws/resource_aws_ecs_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ func TestAccAWSEcsService_withLaunchTypeFargate(t *testing.T) {
})
}

func TestAccAWSEcsService_withNetworkConfiguration(t *testing.T) {
func TestAccAWSEcsService_withNetworkConfigurationAssignPublicIp(t *testing.T) {
rString := acctest.RandString(8)

sg1Name := fmt.Sprintf("tf-acc-sg-1-svc-w-nc-%s", rString)
Expand All @@ -501,7 +501,31 @@ func TestAccAWSEcsService_withNetworkConfiguration(t *testing.T) {
CheckDestroy: testAccCheckAWSEcsServiceDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName),
Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName, "true"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsServiceExists("aws_ecs_service.main"),
),
},
},
})
}

func TestAccAWSEcsService_withNetworkConfigurationDoNotAssignPublicIp(t *testing.T) {
rString := acctest.RandString(8)

sg1Name := fmt.Sprintf("tf-acc-sg-1-svc-w-nc-%s", rString)
sg2Name := fmt.Sprintf("tf-acc-sg-2-svc-w-nc-%s", rString)
clusterName := fmt.Sprintf("tf-acc-cluster-svc-w-nc-%s", rString)
tdName := fmt.Sprintf("tf-acc-td-svc-w-nc-%s", rString)
svcName := fmt.Sprintf("tf-acc-svc-w-nc-%s", rString)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEcsServiceDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName, "false"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsServiceExists("aws_ecs_service.main"),
),
Expand Down Expand Up @@ -1448,7 +1472,7 @@ resource "aws_ecs_service" "with_alb" {
`, clusterName, tdName, roleName, policyName, tgName, lbName, svcName)
}

func testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName string) string {
func testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName, assignPublicIp string) string {
return fmt.Sprintf(`
data "aws_availability_zones" "available" {}

Expand Down Expand Up @@ -1517,7 +1541,8 @@ resource "aws_ecs_service" "main" {
network_configuration {
security_groups = ["${aws_security_group.allow_all_a.id}", "${aws_security_group.allow_all_b.id}"]
subnets = ["${aws_subnet.main.*.id}"]
assign_public_ip = %s
}
}
`, sg1Name, sg2Name, clusterName, tdName, svcName)
`, sg1Name, sg2Name, clusterName, tdName, svcName, assignPublicIp)
}
1 change: 0 additions & 1 deletion aws/resource_aws_lb_listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ func resourceAwsLbListener() *schema.Resource {
"load_balancer_arn": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change - will fix prior to merge

},

"port": {
Expand Down
1 change: 0 additions & 1 deletion examples/networking/region/numbering.tf

This file was deleted.

1 change: 0 additions & 1 deletion examples/networking/subnet/numbering.tf

This file was deleted.

1 change: 1 addition & 0 deletions website/docs/r/ecs_service.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ Guide](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/cluster-query

* `subnets` - (Required) The subnets associated with the task or service.
* `security_groups` - (Optional) The security groups associated with the task or service. If you do not specify a security group, the default security group for the VPC is used.
* `assign_public_ip` - (Optional) Valid values are "true" or "false". Will assign a public IP address to the ENI. Default value is "false".
For more information, see [Task Networking](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-networking.html)

## Attributes Reference
Expand Down