fix: Correct variable optional argument defaults and container definition conditional logic (#332)

* fix: Correct variable defaults and container definition conditional logic

* chore: Correct comment

* chore: Add variable to enable/disable writing container definition to file

Author: bryantbiggs

README.md

@@ -23,13 +23,13 @@ Note that this example may create resources which will incur monetary charges on
 |------|---------|
 | <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.5.7 |
 | <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 6.4 |
-| <a name="requirement_local"></a> [local](#requirement\_local) | >= 2.5 |
+| <a name="requirement_null"></a> [null](#requirement\_null) | >= 3.2 |
 
 ## Providers
 
 | Name | Version |
 |------|---------|
-| <a name="provider_local"></a> [local](#provider\_local) | >= 2.5 |
+| <a name="provider_null"></a> [null](#provider\_null) | >= 3.2 |
 
 ## Modules
 
@@ -41,11 +41,13 @@ Note that this example may create resources which will incur monetary charges on
 
 | Name | Type |
 |------|------|
-| [local_file.container_definition_json](https://registry.terraform.io/providers/hashicorp/local/latest/docs/resources/file) | resource |
+| [null_resource.container_definition_json](https://registry.terraform.io/providers/hashicorp/null/latest/docs/resources/resource) | resource |
 
 ## Inputs
 
-No inputs.
+| Name | Description | Type | Default | Required |
+|------|-------------|------|---------|:--------:|
+| <a name="input_write_container_definition_to_file"></a> [write\_container\_definition\_to\_file](#input\_write\_container\_definition\_to\_file) | Determines whether the container definition JSON should be written to a file. Used for debugging and checking diffs | `bool` | `true` | no |
 
 ## Outputs
 

examples/container-definition/README.md

@@ -12,9 +12,17 @@ output "container_definition_json" {
   value       = module.ecs_container_definition.container_definition_json
 }
 
-resource "local_file" "container_definition_json" {
-  content  = module.ecs_container_definition.container_definition_json
-  filename = "${path.module}/definition.json"
+resource "null_resource" "container_definition_json" {
+  count = var.write_container_definition_to_file ? 1 : 0
+
+  triggers = {
+    container_definition_json = timestamp()
+  }
+
+  provisioner "local-exec" {
+    # Need the output pretty-printed and sorted for comparison
+    command = "echo '${module.ecs_container_definition.container_definition_json}' | jq -S > ./definition.json"
+  }
 }
 
 ################################################################################

examples/container-definition/outputs.tf

@@ -0,0 +1,5 @@
+variable "write_container_definition_to_file" {
+  description = "Determines whether the container definition JSON should be written to a file. Used for debugging and checking diffs"
+  type        = bool
+  default     = true
+}

examples/container-definition/variables.tf

@@ -6,9 +6,9 @@ terraform {
       source  = "hashicorp/aws"
       version = ">= 6.4"
     }
-    local = {
-      source  = "hashicorp/local"
-      version = ">= 2.5"
+    null = {
+      source  = "hashicorp/null"
+      version = ">= 3.2"
     }
   }
 }

examples/container-definition/versions.tf

@@ -65,18 +65,18 @@ module "ecs_service" {
     strategy             = "BLUE_GREEN"
     bake_time_in_minutes = 2
 
-    # example config using lifecycle hooks
+    # # Example config using lifecycle hooks
     # lifecycle_hook = {
-    #  success = {
-    #    hook_target_arn  = aws_lambda_function.hook_success.arn
-    #    role_arn         = aws_iam_role.global.arn
-    #    lifecycle_stages = ["POST_SCALE_UP", "POST_TEST_TRAFFIC_SHIFT"]
-    #  }
-    #  failure = {
-    #    hook_target_arn  = aws_lambda_function.hook_failure.arn
-    #    role_arn         = aws_iam_role.global.arn
-    #    lifecycle_stages = ["TEST_TRAFFIC_SHIFT", "POST_PRODUCTION_TRAFFIC_SHIFT"]
-    #  }
+    #   success = {
+    #     hook_target_arn  = aws_lambda_function.hook_success.arn
+    #     role_arn         = aws_iam_role.global.arn
+    #     lifecycle_stages = ["POST_SCALE_UP", "POST_TEST_TRAFFIC_SHIFT"]
+    #   }
+    #   failure = {
+    #     hook_target_arn  = aws_lambda_function.hook_failure.arn
+    #     role_arn         = aws_iam_role.global.arn
+    #     lifecycle_stages = ["TEST_TRAFFIC_SHIFT", "POST_PRODUCTION_TRAFFIC_SHIFT"]
+    #   }
     # }
   }
 

examples/fargate/main.tf

@@ -166,7 +166,7 @@ No modules.
 | <a name="input_image"></a> [image](#input\_image) | The image used to start a container. This string is passed directly to the Docker daemon. By default, images in the Docker Hub registry are available. Other repositories are specified with either `repository-url/image:tag` or `repository-url/image@digest` | `string` | `null` | no |
 | <a name="input_interactive"></a> [interactive](#input\_interactive) | When this parameter is `true`, you can deploy containerized applications that require `stdin` or a `tty` to be allocated | `bool` | `false` | no |
 | <a name="input_links"></a> [links](#input\_links) | The links parameter allows containers to communicate with each other without the need for port mappings. This parameter is only supported if the network mode of a task definition is `bridge` | `list(string)` | `null` | no |
-| <a name="input_linuxParameters"></a> [linuxParameters](#input\_linuxParameters) | Linux-specific modifications that are applied to the container, such as Linux kernel capabilities. For more information see [KernelCapabilities](https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_KernelCapabilities.html) | <pre>object({<br/>    capabilities = optional(object({<br/>      add  = optional(list(string))<br/>      drop = optional(list(string))<br/>    }))<br/>    devices = optional(list(object({<br/>      containerPath = optional(string)<br/>      hostPath      = optional(string)<br/>      permissions   = optional(list(string))<br/>    })))<br/>    initProcessEnabled = optional(bool, false)<br/>    maxSwap            = optional(number)<br/>    sharedMemorySize   = optional(number)<br/>    swappiness         = optional(number)<br/>    tmpfs = optional(list(object({<br/>      containerPath = string<br/>      mountOptions  = optional(list(string))<br/>      size          = number<br/>    })))<br/>  })</pre> | <pre>{<br/>  "initProcessEnabled": false<br/>}</pre> | no |
+| <a name="input_linuxParameters"></a> [linuxParameters](#input\_linuxParameters) | Linux-specific modifications that are applied to the container, such as Linux kernel capabilities. For more information see [KernelCapabilities](https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_KernelCapabilities.html) | <pre>object({<br/>    capabilities = optional(object({<br/>      add  = optional(list(string))<br/>      drop = optional(list(string))<br/>    }))<br/>    devices = optional(list(object({<br/>      containerPath = optional(string)<br/>      hostPath      = optional(string)<br/>      permissions   = optional(list(string))<br/>    })))<br/>    initProcessEnabled = optional(bool)<br/>    maxSwap            = optional(number)<br/>    sharedMemorySize   = optional(number)<br/>    swappiness         = optional(number)<br/>    tmpfs = optional(list(object({<br/>      containerPath = string<br/>      mountOptions  = optional(list(string))<br/>      size          = number<br/>    })))<br/>  })</pre> | `{}` | no |
 | <a name="input_logConfiguration"></a> [logConfiguration](#input\_logConfiguration) | The log configuration for the container. For more information see [LogConfiguration](https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_LogConfiguration.html) | <pre>object({<br/>    logDriver = optional(string)<br/>    options   = optional(map(string))<br/>    secretOptions = optional(list(object({<br/>      name      = string<br/>      valueFrom = string<br/>    })))<br/>  })</pre> | `{}` | no |
 | <a name="input_memory"></a> [memory](#input\_memory) | The amount (in MiB) of memory to present to the container. If your container attempts to exceed the memory specified here, the container is killed. The total amount of memory reserved for all containers within a task must be lower than the task `memory` value, if one is specified | `number` | `null` | no |
 | <a name="input_memoryReservation"></a> [memoryReservation](#input\_memoryReservation) | The soft limit (in MiB) of memory to reserve for the container. When system memory is under heavy contention, Docker attempts to keep the container memory to this soft limit. However, your container can consume more memory when it needs to, up to either the hard limit specified with the `memory` parameter (if applicable), or all of the available memory on the container instance | `number` | `null` | no |
@@ -180,7 +180,7 @@ No modules.
 | <a name="input_region"></a> [region](#input\_region) | Region where the resource(s) will be managed. Defaults to the Region set in the provider configuration | `string` | `null` | no |
 | <a name="input_repositoryCredentials"></a> [repositoryCredentials](#input\_repositoryCredentials) | Container repository credentials; required when using a private repo.  This map currently supports a single key; "credentialsParameter", which should be the ARN of a Secrets Manager's secret holding the credentials | <pre>object({<br/>    credentialsParameter = optional(string)<br/>  })</pre> | `null` | no |
 | <a name="input_resourceRequirements"></a> [resourceRequirements](#input\_resourceRequirements) | The type and amount of a resource to assign to a container. The only supported resource is a GPU | <pre>list(object({<br/>    type  = string<br/>    value = string<br/>  }))</pre> | `null` | no |
-| <a name="input_restartPolicy"></a> [restartPolicy](#input\_restartPolicy) | Container restart policy; helps overcome transient failures faster and maintain task availability | <pre>object({<br/>    enabled              = optional(bool, true)<br/>    ignoredExitCodes     = optional(list(number), [])<br/>    restartAttemptPeriod = optional(number)<br/>  })</pre> | <pre>{<br/>  "enabled": true<br/>}</pre> | no |
+| <a name="input_restartPolicy"></a> [restartPolicy](#input\_restartPolicy) | Container restart policy; helps overcome transient failures faster and maintain task availability | <pre>object({<br/>    enabled              = optional(bool)<br/>    ignoredExitCodes     = optional(list(number))<br/>    restartAttemptPeriod = optional(number)<br/>  })</pre> | <pre>{<br/>  "enabled": true<br/>}</pre> | no |
 | <a name="input_secrets"></a> [secrets](#input\_secrets) | The secrets to pass to the container. For more information, see [Specifying Sensitive Data](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/specifying-sensitive-data.html) in the Amazon Elastic Container Service Developer Guide | <pre>list(object({<br/>    name      = string<br/>    valueFrom = string<br/>  }))</pre> | `null` | no |
 | <a name="input_service"></a> [service](#input\_service) | The name of the service that the container definition is associated with. Used in CloudWatch log group default name (if one is not provided) | `string` | `null` | no |
 | <a name="input_startTimeout"></a> [startTimeout](#input\_startTimeout) | Time duration (in seconds) to wait before giving up on resolving dependencies for a container | `number` | `30` | no |
@@ -200,7 +200,7 @@ No modules.
 | <a name="output_cloudwatch_log_group_arn"></a> [cloudwatch\_log\_group\_arn](#output\_cloudwatch\_log\_group\_arn) | ARN of CloudWatch log group created |
 | <a name="output_cloudwatch_log_group_name"></a> [cloudwatch\_log\_group\_name](#output\_cloudwatch\_log\_group\_name) | Name of CloudWatch log group created |
 | <a name="output_container_definition"></a> [container\_definition](#output\_container\_definition) | Container definition |
-| <a name="output_container_definition_json"></a> [container\_definition\_json](#output\_container\_definition\_json) | Container definition |
+| <a name="output_container_definition_json"></a> [container\_definition\_json](#output\_container\_definition\_json) | Container definition. NOTE: use `jsonencode([module.ecs_container_definition.container_definition])` instead of this output when passing into a Task Definition |
 <!-- END_TF_DOCS -->
 
 ## License

modules/container-definition/README.md

@@ -2,7 +2,6 @@ data "aws_region" "current" {
   region = var.region
 }
 
-
 locals {
   is_not_windows = contains(["LINUX"], var.operating_system_family)
 
@@ -23,8 +22,16 @@ locals {
     { for k, v in var.logConfiguration : k => v if v != null }
   )
 
+  # 1. We remove any attributes that are set to `null` by default from the variable optional attributes
+  # tflint-ignore: terraform_naming_convention
+  trimedLinuxParameters = { for k, v in var.linuxParameters : k => v if v != null }
+  # 2. We then merge in the `initProcessEnabled` attribute based on whether `enable_execute_command` is true or false
+  # This also means we will always have something in `linuxParameters` (it will never be `null` or `{}`)
+  # Terraform doesn't allow us to set `initProcessEnabled` to `true` on one side only of the conditional, so we have to merge it in on both sides
+  # However, in the `true` case, we set it last to ensure `initProcessEnabled` is always `true` when `enable_execute_command` is true
+  # and the "psuedo-default" is `false` when `enable_execute_command` is false (but can still be overridden by the user)
   # tflint-ignore: terraform_naming_convention
-  linuxParameters = var.enable_execute_command ? merge({ "initProcessEnabled" : true }, var.linuxParameters) : merge({ "initProcessEnabled" : false }, var.linuxParameters)
+  linuxParameters = var.enable_execute_command ? merge(local.trimedLinuxParameters, { "initProcessEnabled" : true }) : merge({ "initProcessEnabled" : false }, local.trimedLinuxParameters)
 
   definition = {
     command                = var.command
@@ -46,7 +53,7 @@ locals {
     image                  = var.image
     interactive            = var.interactive
     links                  = local.is_not_windows ? var.links : null
-    linuxParameters        = local.is_not_windows ? { for k, v in local.linuxParameters : k => v if v != null } : null
+    linuxParameters        = local.is_not_windows ? local.linuxParameters : null
     logConfiguration       = length(local.logConfiguration) > 0 ? local.logConfiguration : null
     memory                 = var.memory
     memoryReservation      = var.memoryReservation

modules/container-definition/main.tf

@@ -7,8 +7,9 @@ output "container_definition" {
   value       = local.container_definition
 }
 
+# ToDo - remove at next breaking change. Not worth it
 output "container_definition_json" {
-  description = "Container definition"
+  description = "Container definition. NOTE: use `jsonencode([module.ecs_container_definition.container_definition])` instead of this output when passing into a Task Definition"
   value       = jsonencode(local.container_definition)
 }
 

modules/container-definition/outputs.tf

@@ -8,12 +8,14 @@ variable "operating_system_family" {
   description = "The OS family for task"
   type        = string
   default     = "LINUX"
+  nullable    = false
 }
 
 variable "tags" {
   description = "A map of tags to add to all resources"
   type        = map(string)
   default     = {}
+  nullable    = false
 }
 
 ################################################################################
@@ -81,12 +83,14 @@ variable "enable_execute_command" {
   description = "Specifies whether to enable Amazon ECS Exec for the tasks within the service"
   type        = bool
   default     = false
+  nullable    = false
 }
 
 variable "entrypoint" {
   description = "The entry point that is passed to the container"
   type        = list(string)
   default     = []
+  nullable    = false
 }
 
 variable "environment" {
@@ -95,7 +99,8 @@ variable "environment" {
     name  = string
     value = string
   }))
-  default = []
+  default  = []
+  nullable = false
 }
 
 # tflint-ignore: terraform_naming_convention
@@ -105,7 +110,8 @@ variable "environmentFiles" {
     value = string
     type  = string
   }))
-  default = []
+  default  = []
+  nullable = false
 }
 
 variable "essential" {
@@ -163,6 +169,7 @@ variable "interactive" {
   description = "When this parameter is `true`, you can deploy containerized applications that require `stdin` or a `tty` to be allocated"
   type        = bool
   default     = false
+  nullable    = false
 }
 
 variable "links" {
@@ -184,7 +191,7 @@ variable "linuxParameters" {
       hostPath      = optional(string)
       permissions   = optional(list(string))
     })))
-    initProcessEnabled = optional(bool, false)
+    initProcessEnabled = optional(bool)
     maxSwap            = optional(number)
     sharedMemorySize   = optional(number)
     swappiness         = optional(number)
@@ -194,9 +201,8 @@ variable "linuxParameters" {
       size          = number
     })))
   })
-  default = {
-    initProcessEnabled = false
-  }
+  default  = {}
+  nullable = false
 }
 
 # tflint-ignore: terraform_naming_convention
@@ -210,7 +216,8 @@ variable "logConfiguration" {
       valueFrom = string
     })))
   })
-  default = {}
+  default  = {}
+  nullable = false
 }
 
 variable "memory" {
@@ -234,7 +241,8 @@ variable "mountPoints" {
     readOnly      = optional(bool)
     sourceVolume  = optional(string)
   }))
-  default = []
+  default  = []
+  nullable = false
 }
 
 variable "name" {
@@ -261,20 +269,23 @@ variable "privileged" {
   description = "When this parameter is true, the container is given elevated privileges on the host container instance (similar to the root user)"
   type        = bool
   default     = false
+  nullable    = false
 }
 
 # tflint-ignore: terraform_naming_convention
 variable "pseudoTerminal" {
   description = "When this parameter is true, a `TTY` is allocated"
   type        = bool
   default     = false
+  nullable    = false
 }
 
 # tflint-ignore: terraform_naming_convention
 variable "readonlyRootFilesystem" {
   description = "When this parameter is true, the container is given read-only access to its root file system"
   type        = bool
   default     = true
+  nullable    = false
 }
 
 # tflint-ignore: terraform_naming_convention
@@ -300,8 +311,8 @@ variable "resourceRequirements" {
 variable "restartPolicy" {
   description = "Container restart policy; helps overcome transient failures faster and maintain task availability"
   type = object({
-    enabled              = optional(bool, true)
-    ignoredExitCodes     = optional(list(number), [])
+    enabled              = optional(bool)
+    ignoredExitCodes     = optional(list(number))
     restartAttemptPeriod = optional(number)
   })
   default = {
@@ -339,7 +350,8 @@ variable "systemControls" {
     namespace = optional(string)
     value     = optional(string)
   }))
-  default = []
+  default  = []
+  nullable = false
 }
 
 variable "ulimits" {
@@ -363,6 +375,7 @@ variable "versionConsistency" {
   description = "Specifies whether Amazon ECS will resolve the container image tag provided in the container definition to an image digest"
   type        = string
   default     = "disabled"
+  nullable    = false
 }
 
 # tflint-ignore: terraform_naming_convention
@@ -372,7 +385,8 @@ variable "volumesFrom" {
     readOnly        = optional(bool)
     sourceContainer = optional(string)
   }))
-  default = []
+  default  = []
+  nullable = false
 }
 
 # tflint-ignore: terraform_naming_convention
@@ -396,12 +410,14 @@ variable "enable_cloudwatch_logging" {
   description = "Determines whether CloudWatch logging is configured for this container definition. Set to `false` to use other logging drivers"
   type        = bool
   default     = true
+  nullable    = false
 }
 
 variable "create_cloudwatch_log_group" {
   description = "Determines whether a log group is created by this module. If not, AWS will automatically create one if logging is enabled"
   type        = bool
   default     = true
+  nullable    = false
 }
 
 variable "cloudwatch_log_group_name" {
@@ -414,6 +430,7 @@ variable "cloudwatch_log_group_use_name_prefix" {
   description = "Determines whether the log group name should be used as a prefix"
   type        = bool
   default     = false
+  nullable    = false
 }
 
 variable "cloudwatch_log_group_class" {
@@ -426,6 +443,7 @@ variable "cloudwatch_log_group_retention_in_days" {
   description = "Number of days to retain log events. Set to `0` to keep logs indefinitely"
   type        = number
   default     = 14
+  nullable    = false
 }
 
 variable "cloudwatch_log_group_kms_key_id" {