Accept map or list for policy arns, fix race condition (#198)

Author: Nuru

README.md

@@ -367,13 +367,13 @@ Available targets:
 | <a name="input_tags"></a> [tags](#input\_tags) | Additional tags (e.g. `{'BusinessUnit': 'XYZ'}`).<br>Neither the tag keys nor the tag values will be modified by this module. | `map(string)` | `{}` | no |
 | <a name="input_task_cpu"></a> [task\_cpu](#input\_task\_cpu) | The number of CPU units used by the task. If using `FARGATE` launch type `task_cpu` must match [supported memory values](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task_definition_parameters.html#task_size) | `number` | `256` | no |
 | <a name="input_task_definition"></a> [task\_definition](#input\_task\_definition) | Reuse an existing task definition family and revision for the ecs service instead of creating one | `string` | `null` | no |
-| <a name="input_task_exec_policy_arns"></a> [task\_exec\_policy\_arns](#input\_task\_exec\_policy\_arns) | A list of IAM Policy ARNs to attach to the generated task execution role. | `list(string)` | `[]` | no |
-| <a name="input_task_exec_policy_arns_map"></a> [task\_exec\_policy\_arns\_map](#input\_task\_exec\_policy\_arns\_map) | A map of name to IAM Policy ARNs to attach to the generated task execution role. | `map(string)` | `{}` | no |
+| <a name="input_task_exec_policy_arns"></a> [task\_exec\_policy\_arns](#input\_task\_exec\_policy\_arns) | A list of IAM Policy ARNs to attach to the generated task execution role.<br>Changes to the list will have ripple effects, so use `task_exec_policy_arns_map` if possible. | `list(string)` | `[]` | no |
+| <a name="input_task_exec_policy_arns_map"></a> [task\_exec\_policy\_arns\_map](#input\_task\_exec\_policy\_arns\_map) | A map of name to IAM Policy ARNs to attach to the generated task execution role.<br>The names are arbitrary, but must be known at plan time. The purpose of the name<br>is so that changes to one ARN do not cause a ripple effect on the other ARNs.<br>If you cannot provide unique names known at plan time, use `task_exec_policy_arns` instead. | `map(string)` | `{}` | no |
 | <a name="input_task_exec_role_arn"></a> [task\_exec\_role\_arn](#input\_task\_exec\_role\_arn) | A `list(string)` of zero or one ARNs of IAM roles that allows the<br>ECS/Fargate agent to make calls to the ECS API on your behalf.<br>If the list is empty, a role will be created for you.<br>DEPRECATED: you can also pass a `string` with the ARN, but that<br>string must be known a "plan" time. | `any` | `[]` | no |
 | <a name="input_task_memory"></a> [task\_memory](#input\_task\_memory) | The amount of memory (in MiB) used by the task. If using Fargate launch type `task_memory` must match [supported cpu value](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task_definition_parameters.html#task_size) | `number` | `512` | no |
 | <a name="input_task_placement_constraints"></a> [task\_placement\_constraints](#input\_task\_placement\_constraints) | A set of placement constraints rules that are taken into consideration during task placement.<br>Maximum number of placement\_constraints is 10. See [`placement_constraints`](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ecs_task_definition#placement-constraints-arguments) | <pre>list(object({<br>    type       = string<br>    expression = string<br>  }))</pre> | `[]` | no |
-| <a name="input_task_policy_arns"></a> [task\_policy\_arns](#input\_task\_policy\_arns) | A list of IAM Policy ARNs to attach to the generated task role. | `list(string)` | `[]` | no |
-| <a name="input_task_policy_arns_map"></a> [task\_policy\_arns\_map](#input\_task\_policy\_arns\_map) | A map of name to IAM Policy ARNs to attach to the generated task role. | `map(string)` | `{}` | no |
+| <a name="input_task_policy_arns"></a> [task\_policy\_arns](#input\_task\_policy\_arns) | A list of IAM Policy ARNs to attach to the generated task role.<br>Changes to the list will have ripple effects, so use `task_policy_arns_map` if possible. | `list(string)` | `[]` | no |
+| <a name="input_task_policy_arns_map"></a> [task\_policy\_arns\_map](#input\_task\_policy\_arns\_map) | A map of name to IAM Policy ARNs to attach to the generated task role.<br>The names are arbitrary, but must be known at plan time. The purpose of the name<br>is so that changes to one ARN do not cause a ripple effect on the other ARNs.<br>If you cannot provide unique names known at plan time, use `task_policy_arns` instead. | `map(string)` | `{}` | no |
 | <a name="input_task_role_arn"></a> [task\_role\_arn](#input\_task\_role\_arn) | A `list(string)` of zero or one ARNs of IAM roles that allows<br>your Amazon ECS container task to make calls to other AWS services.<br>If the list is empty, a role will be created for you.<br>DEPRECATED: you can also pass a `string` with the ARN, but that<br>string must be known a "plan" time. | `any` | `[]` | no |
 | <a name="input_tenant"></a> [tenant](#input\_tenant) | ID element \_(Rarely used, not included by default)\_. A customer identifier, indicating who this instance of a resource is for | `string` | `null` | no |
 | <a name="input_use_alb_security_group"></a> [use\_alb\_security\_group](#input\_use\_alb\_security\_group) | A flag to enable/disable allowing traffic from the ALB security group to the service security group | `bool` | `false` | no |

docs/terraform.md

@@ -120,13 +120,13 @@
 | <a name="input_tags"></a> [tags](#input\_tags) | Additional tags (e.g. `{'BusinessUnit': 'XYZ'}`).<br>Neither the tag keys nor the tag values will be modified by this module. | `map(string)` | `{}` | no |
 | <a name="input_task_cpu"></a> [task\_cpu](#input\_task\_cpu) | The number of CPU units used by the task. If using `FARGATE` launch type `task_cpu` must match [supported memory values](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task_definition_parameters.html#task_size) | `number` | `256` | no |
 | <a name="input_task_definition"></a> [task\_definition](#input\_task\_definition) | Reuse an existing task definition family and revision for the ecs service instead of creating one | `string` | `null` | no |
-| <a name="input_task_exec_policy_arns"></a> [task\_exec\_policy\_arns](#input\_task\_exec\_policy\_arns) | A list of IAM Policy ARNs to attach to the generated task execution role. | `list(string)` | `[]` | no |
-| <a name="input_task_exec_policy_arns_map"></a> [task\_exec\_policy\_arns\_map](#input\_task\_exec\_policy\_arns\_map) | A map of name to IAM Policy ARNs to attach to the generated task execution role. | `map(string)` | `{}` | no |
+| <a name="input_task_exec_policy_arns"></a> [task\_exec\_policy\_arns](#input\_task\_exec\_policy\_arns) | A list of IAM Policy ARNs to attach to the generated task execution role.<br>Changes to the list will have ripple effects, so use `task_exec_policy_arns_map` if possible. | `list(string)` | `[]` | no |
+| <a name="input_task_exec_policy_arns_map"></a> [task\_exec\_policy\_arns\_map](#input\_task\_exec\_policy\_arns\_map) | A map of name to IAM Policy ARNs to attach to the generated task execution role.<br>The names are arbitrary, but must be known at plan time. The purpose of the name<br>is so that changes to one ARN do not cause a ripple effect on the other ARNs.<br>If you cannot provide unique names known at plan time, use `task_exec_policy_arns` instead. | `map(string)` | `{}` | no |
 | <a name="input_task_exec_role_arn"></a> [task\_exec\_role\_arn](#input\_task\_exec\_role\_arn) | A `list(string)` of zero or one ARNs of IAM roles that allows the<br>ECS/Fargate agent to make calls to the ECS API on your behalf.<br>If the list is empty, a role will be created for you.<br>DEPRECATED: you can also pass a `string` with the ARN, but that<br>string must be known a "plan" time. | `any` | `[]` | no |
 | <a name="input_task_memory"></a> [task\_memory](#input\_task\_memory) | The amount of memory (in MiB) used by the task. If using Fargate launch type `task_memory` must match [supported cpu value](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task_definition_parameters.html#task_size) | `number` | `512` | no |
 | <a name="input_task_placement_constraints"></a> [task\_placement\_constraints](#input\_task\_placement\_constraints) | A set of placement constraints rules that are taken into consideration during task placement.<br>Maximum number of placement\_constraints is 10. See [`placement_constraints`](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ecs_task_definition#placement-constraints-arguments) | <pre>list(object({<br>    type       = string<br>    expression = string<br>  }))</pre> | `[]` | no |
-| <a name="input_task_policy_arns"></a> [task\_policy\_arns](#input\_task\_policy\_arns) | A list of IAM Policy ARNs to attach to the generated task role. | `list(string)` | `[]` | no |
-| <a name="input_task_policy_arns_map"></a> [task\_policy\_arns\_map](#input\_task\_policy\_arns\_map) | A map of name to IAM Policy ARNs to attach to the generated task role. | `map(string)` | `{}` | no |
+| <a name="input_task_policy_arns"></a> [task\_policy\_arns](#input\_task\_policy\_arns) | A list of IAM Policy ARNs to attach to the generated task role.<br>Changes to the list will have ripple effects, so use `task_policy_arns_map` if possible. | `list(string)` | `[]` | no |
+| <a name="input_task_policy_arns_map"></a> [task\_policy\_arns\_map](#input\_task\_policy\_arns\_map) | A map of name to IAM Policy ARNs to attach to the generated task role.<br>The names are arbitrary, but must be known at plan time. The purpose of the name<br>is so that changes to one ARN do not cause a ripple effect on the other ARNs.<br>If you cannot provide unique names known at plan time, use `task_policy_arns` instead. | `map(string)` | `{}` | no |
 | <a name="input_task_role_arn"></a> [task\_role\_arn](#input\_task\_role\_arn) | A `list(string)` of zero or one ARNs of IAM roles that allows<br>your Amazon ECS container task to make calls to other AWS services.<br>If the list is empty, a role will be created for you.<br>DEPRECATED: you can also pass a `string` with the ARN, but that<br>string must be known a "plan" time. | `any` | `[]` | no |
 | <a name="input_tenant"></a> [tenant](#input\_tenant) | ID element \_(Rarely used, not included by default)\_. A customer identifier, indicating who this instance of a resource is for | `string` | `null` | no |
 | <a name="input_use_alb_security_group"></a> [use\_alb\_security\_group](#input\_use\_alb\_security\_group) | A flag to enable/disable allowing traffic from the ALB security group to the service security group | `bool` | `false` | no |

examples/complete/main.tf

@@ -2,37 +2,45 @@ provider "aws" {
   region = var.region
 }
 
+locals {
+  enabled = module.this.enabled
+}
+
 module "vpc" {
   source  = "cloudposse/vpc/aws"
-  version = "0.28.1"
+  version = "2.0.0"
 
-  cidr_block = var.vpc_cidr_block
+  ipv4_primary_cidr_block = var.vpc_cidr_block
 
   context = module.this.context
 }
 
 module "subnets" {
   source  = "cloudposse/dynamic-subnets/aws"
-  version = "0.39.8"
+  version = "2.1.0"
 
   availability_zones   = var.availability_zones
   vpc_id               = module.vpc.vpc_id
-  igw_id               = module.vpc.igw_id
-  cidr_block           = module.vpc.vpc_cidr_block
-  nat_gateway_enabled  = true
+  igw_id               = [module.vpc.igw_id]
+  ipv4_cidr_block      = [module.vpc.vpc_cidr_block]
+  nat_gateway_enabled  = false
   nat_instance_enabled = false
 
   context = module.this.context
 }
 
 resource "aws_ecs_cluster" "default" {
-  name = module.this.id
-  tags = module.this.tags
+  #bridgecrew:skip=BC_AWS_LOGGING_11: not required for testing
+  count = local.enabled ? 1 : 0
+  name  = module.this.id
+  tags  = module.this.tags
 }
 
 module "container_definition" {
+  count = local.enabled ? 1 : 0
+
   source  = "cloudposse/ecs-container-definition/aws"
-  version = "0.58.1"
+  version = "0.58.2"
 
   container_name               = var.container_name
   container_image              = var.container_image
@@ -71,8 +79,8 @@ module "test_policy" {
 module "ecs_alb_service_task" {
   source                             = "../.."
   alb_security_group                 = module.vpc.vpc_default_security_group_id
-  container_definition_json          = module.container_definition.json_map_encoded_list
-  ecs_cluster_arn                    = aws_ecs_cluster.default.arn
+  container_definition_json          = one(module.container_definition.*.json_map_encoded_list)
+  ecs_cluster_arn                    = one(aws_ecs_cluster.default.*.id)
   launch_type                        = var.ecs_launch_type
   vpc_id                             = module.vpc.vpc_id
   security_group_ids                 = [module.vpc.vpc_default_security_group_id]
@@ -91,7 +99,7 @@ module "ecs_alb_service_task" {
   force_new_deployment               = var.force_new_deployment
   redeploy_on_apply                  = var.redeploy_on_apply
   task_policy_arns                   = [module.test_policy.policy_arn]
-  # task_policy_arns_map = { test = module.test_policy.policy_arn }
+  task_exec_policy_arns_map          = { test = module.test_policy.policy_arn }
 
   context = module.this.context
 }

examples/complete/outputs.tf

@@ -14,22 +14,22 @@ output "vpc_cidr" {
 }
 
 output "container_definition_json" {
-  value       = module.container_definition.json_map_encoded_list
+  value       = one(module.container_definition.*.json_map_encoded_list)
   description = "JSON encoded list of container definitions for use with other terraform resources such as aws_ecs_task_definition"
 }
 
 output "container_definition_json_map" {
-  value       = module.container_definition.json_map_encoded
+  value       = one(module.container_definition.*.json_map_encoded)
   description = "JSON encoded container definitions for use with other terraform resources such as aws_ecs_task_definition"
 }
 
 output "ecs_cluster_id" {
-  value       = aws_ecs_cluster.default.id
+  value       = one(aws_ecs_cluster.default.*.id)
   description = "ECS cluster ID"
 }
 
 output "ecs_cluster_arn" {
-  value       = aws_ecs_cluster.default.arn
+  value       = one(aws_ecs_cluster.default.*.arn)
   description = "ECS cluster ARN"
 }
 

main.tf

@@ -14,9 +14,9 @@ locals {
     redeployment = timestamp()
   } : {}
 
-  task_policy_arns_map = length(var.task_policy_arns) > 0 ? { for i, a in var.task_policy_arns : i => a } : var.task_policy_arns_map
+  task_policy_arns_map = merge({ for i, a in var.task_policy_arns : format("_#%v_", i) => a }, var.task_policy_arns_map)
 
-  task_exec_policy_arns_map = length(var.task_exec_policy_arns) > 0 ? { for i, a in var.task_exec_policy_arns : i => a } : var.task_exec_policy_arns_map
+  task_exec_policy_arns_map = merge({ for i, a in var.task_exec_policy_arns : format("_#%v_", i) => a }, var.task_exec_policy_arns_map)
 }
 
 module "task_label" {
@@ -450,6 +450,10 @@ resource "aws_ecs_service" "ignore_changes_task_definition" {
   lifecycle {
     ignore_changes = [task_definition]
   }
+
+  # Avoid race condition on destroy.
+  # See https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ecs_service
+  depends_on = [aws_iam_role.ecs_service, aws_iam_role_policy.ecs_service]
 }
 
 resource "aws_ecs_service" "ignore_changes_task_definition_and_desired_count" {
@@ -545,6 +549,10 @@ resource "aws_ecs_service" "ignore_changes_task_definition_and_desired_count" {
   lifecycle {
     ignore_changes = [task_definition, desired_count]
   }
+
+  # Avoid race condition on destroy.
+  # See https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ecs_service
+  depends_on = [aws_iam_role.ecs_service, aws_iam_role_policy.ecs_service]
 }
 
 resource "aws_ecs_service" "ignore_changes_desired_count" {
@@ -640,6 +648,10 @@ resource "aws_ecs_service" "ignore_changes_desired_count" {
   lifecycle {
     ignore_changes = [desired_count]
   }
+
+  # Avoid race condition on destroy.
+  # See https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ecs_service
+  depends_on = [aws_iam_role.ecs_service, aws_iam_role_policy.ecs_service]
 }
 
 resource "aws_ecs_service" "default" {
@@ -731,4 +743,9 @@ resource "aws_ecs_service" "default" {
   }
 
   triggers = local.redeployment_trigger
+
+  # Avoid race condition on destroy.
+  # See https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ecs_service
+  depends_on = [aws_iam_role.ecs_service, aws_iam_role_policy.ecs_service]
+
 }

test/src/examples_complete_test.go

@@ -2,39 +2,53 @@ package test
 
 import (
 	"encoding/json"
+	"os"
+	"regexp"
+	"strings"
+	"testing"
+
 	"github.com/gruntwork-io/terratest/modules/random"
 	"github.com/gruntwork-io/terratest/modules/terraform"
+	testStructure "github.com/gruntwork-io/terratest/modules/test-structure"
 	"github.com/stretchr/testify/assert"
-	"strings"
-	"testing"
 )
 
+func cleanup(t *testing.T, terraformOptions *terraform.Options, tempTestFolder string) {
+	terraform.Destroy(t, terraformOptions)
+	os.RemoveAll(tempTestFolder)
+}
+
 // Test the Terraform module in examples/complete using Terratest.
 func TestExamplesComplete(t *testing.T) {
-	// This test is not configured to run in parallel (due to shared Terraform workspace)
-	// t.Parallel()
-
+	t.Parallel()
 	randID := strings.ToLower(random.UniqueId())
 	attributes := []string{randID}
-	basename := "eg-test-ecs-alb-service-task-" + randID
+
+	rootFolder := "../../"
+	terraformFolderRelativeToRoot := "examples/complete"
+	varFiles := []string{"fixtures.us-east-2.tfvars"}
+
+	tempTestFolder := testStructure.CopyTerraformFolderToTemp(t, rootFolder, terraformFolderRelativeToRoot)
 
 	terraformOptions := &terraform.Options{
 		// The path to where our Terraform code is located
-		TerraformDir: "../../examples/complete",
+		TerraformDir: tempTestFolder,
 		Upgrade:      true,
 		// Variables to pass to our Terraform code using -var-file options
-		VarFiles: []string{"fixtures.us-east-2.tfvars"},
+		VarFiles: varFiles,
 		Vars: map[string]interface{}{
 			"attributes": attributes,
 		},
 	}
 
 	// At the end of the test, run `terraform destroy` to clean up any resources that were created
-	defer terraform.Destroy(t, terraformOptions)
+	defer cleanup(t, terraformOptions, tempTestFolder)
 
 	// This will run `terraform init` and `terraform apply` and fail the test if there are any errors
 	terraform.InitAndApply(t, terraformOptions)
 
+	basename := "eg-test-ecs-alb-service-task-" + randID
+
 	// Run `terraform output` to get the value of an output variable
 	jsonMap := terraform.OutputRequired(t, terraformOptions, "container_definition_json_map")
 	// Verify we're getting back the outputs we expect
@@ -109,3 +123,39 @@ func TestExamplesComplete(t *testing.T) {
 	// Verify we're getting back the outputs we expect
 	assert.Equal(t, "arn:aws:iam::126450723953:role/"+basename+"-task", taskRoleArn)
 }
+
+func TestExamplesCompleteDisabled(t *testing.T) {
+	t.Parallel()
+	randID := strings.ToLower(random.UniqueId())
+	attributes := []string{randID}
+
+	rootFolder := "../../"
+	terraformFolderRelativeToRoot := "examples/complete"
+	varFiles := []string{"fixtures.us-east-2.tfvars"}
+
+	tempTestFolder := testStructure.CopyTerraformFolderToTemp(t, rootFolder, terraformFolderRelativeToRoot)
+
+	terraformOptions := &terraform.Options{
+		// The path to where our Terraform code is located
+		TerraformDir: tempTestFolder,
+		Upgrade:      true,
+		// Variables to pass to our Terraform code using -var-file options
+		VarFiles: varFiles,
+		Vars: map[string]interface{}{
+			"attributes": attributes,
+			"enabled":    "false",
+		},
+	}
+
+	// At the end of the test, run `terraform destroy` to clean up any resources that were created
+	defer cleanup(t, terraformOptions, tempTestFolder)
+
+	// This will run `terraform init` and `terraform apply` and fail the test if there are any errors
+	results := terraform.InitAndApply(t, terraformOptions)
+
+	// Should complete successfully without creating or changing any resources.
+	// Extract the "Resources:" section of the output to make the error message more readable.
+	re := regexp.MustCompile(`Resources: [^.]+\.`)
+	match := re.FindString(results)
+	assert.Equal(t, "Resources: 0 added, 0 changed, 0 destroyed.", match, "Re-applying the same configuration should not change any resources")
+}