fix: strengthen types, simplify logic (#154) (#164)

* fix: strengthen types, simplify logic (#154)

* fix: strengthen types, simplify logic

* enable & use optional attrs
* strengthen types for log_configuration, repository_credentials, system_controls, container_definition
* rm redundant lookups
* simplify secret & environment var sorting

* Auto Format

* fix: address missing optional; update examples/complete

* Auto Format

---------

Co-authored-by: cloudpossebot <[email protected]>
Co-authored-by: Igor Rodionov <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Andriy Knysh <[email protected]>
Co-authored-by: obataku <[email protected]>
Co-authored-by: cloudpossebot <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Andriy Knysh <[email protected]>

Author: 5 people

README.md

@@ -49,17 +49,18 @@ port_mappings = [
 log_configuration = {
   logDriver = "json-file"
   options = {
-    "max-size" = "10m"
-    "max-file" = "3"
+    max-size = "10m"
+    max-file = "3"
   }
   secretOptions = null
 }
 
 privileged = false
 
-extra_hosts = [{
-  ipAddress = "127.0.0.1"
-  hostname  = "app.local"
+extra_hosts = [
+  {
+    ipAddress = "127.0.0.1"
+    hostname  = "app.local"
   },
 ]
 

docs/terraform.md

@@ -24,34 +24,14 @@ variable "container_memory_reservation" {
   default     = null
 }
 
-variable "container_definition" {
-  type        = map(any)
-  description = "Container definition overrides which allows for extra keys or overriding existing keys."
-  default     = {}
-}
-
+# https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_PortMapping.html
 variable "port_mappings" {
   type = list(object({
     containerPort = number
-    hostPort      = number
-    protocol      = string
+    hostPort      = optional(number)
+    protocol      = optional(string)
   }))
-
   description = "The port mappings to configure for the container. This is a list of maps. Each map should contain \"containerPort\", \"hostPort\", and \"protocol\", where \"protocol\" is one of \"tcp\" or \"udp\". If using containers in a task with the awsvpc or host network mode, the hostPort can either be left blank or set to the same value as the containerPort"
-
-  default = []
-}
-
-# https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_HealthCheck.html
-variable "healthcheck" {
-  type = object({
-    command     = list(string)
-    retries     = number
-    timeout     = number
-    interval    = number
-    startPeriod = number
-  })
-  description = "A map containing command (string), timeout, interval (duration in seconds), retries (1-10, number of times to retry before marking container unhealthy), and startPeriod (0-300, optional grace period to wait, in seconds, before failed healthchecks count toward retries)"
   default     = null
 }
 
@@ -91,13 +71,14 @@ variable "container_environment" {
     value = string
   }))
   description = "The environment variables to pass to the container. This is a list of maps. map_environment overrides environment"
-  default     = []
+  default     = null
 }
 
+# https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_HostEntry.html
 variable "extra_hosts" {
   type = list(object({
-    ipAddress = string
     hostname  = string
+    ipAddress = string
   }))
   description = "A list of hostnames and IP address mappings to append to the /etc/hosts file on the container. This is a list of maps"
   default     = null
@@ -109,154 +90,23 @@ variable "map_environment" {
   default     = null
 }
 
-# https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_EnvironmentFile.html
-variable "environment_files" {
-  type = list(object({
-    value = string
-    type  = string
-  }))
-  description = "One or more files containing the environment variables to pass to the container. This maps to the --env-file option to docker run. The file must be hosted in Amazon S3. This option is only available to tasks using the EC2 launch type. This is a list of maps"
-  default     = null
-}
-
-variable "secrets" {
-  type = list(object({
-    name      = string
-    valueFrom = string
-  }))
-  description = "The secrets to pass to the container. This is a list of maps"
-  default     = null
-}
-
 variable "readonly_root_filesystem" {
   type        = bool
   description = "Determines whether a container is given read-only access to its root filesystem. Due to how Terraform type casts booleans in json it is required to double quote this value"
   default     = false
 }
 
-# https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_LinuxParameters.html
-variable "linux_parameters" {
-  type = object({
-    capabilities = object({
-      add  = list(string)
-      drop = list(string)
-    })
-    devices = list(object({
-      containerPath = string
-      hostPath      = string
-      permissions   = list(string)
-    }))
-    initProcessEnabled = bool
-    maxSwap            = number
-    sharedMemorySize   = number
-    swappiness         = number
-    tmpfs = list(object({
-      containerPath = string
-      mountOptions  = list(string)
-      size          = number
-    }))
-  })
-  description = "Linux-specific modifications that are applied to the container, such as Linux kernel capabilities. For more details, see https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_LinuxParameters.html"
-  default     = null
-}
-
 # https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_LogConfiguration.html
 variable "log_configuration" {
-  type        = any
-  description = "Log configuration options to send to a custom log driver for the container. For more details, see https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_LogConfiguration.html"
-  default     = null
-}
-
-# https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_FirelensConfiguration.html
-variable "firelens_configuration" {
   type = object({
-    type    = string
-    options = map(string)
+    logDriver = string
+    options   = optional(map(string))
+    secretOptions = optional(list(object({
+      name      = string
+      valueFrom = string
+    })))
   })
-  description = "The FireLens configuration for the container. This is used to specify and configure a log router for container logs. For more details, see https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_FirelensConfiguration.html"
-  default     = null
-}
-
-variable "mount_points" {
-  type = list(any)
-
-  description = "Container mount points. This is a list of maps, where each map should contain a `containerPath` and `sourceVolume`. The `readOnly` key is optional."
-  default     = []
-}
-
-variable "dns_servers" {
-  type        = list(string)
-  description = "Container DNS servers. This is a list of strings specifying the IP addresses of the DNS servers"
-  default     = null
-}
-
-variable "dns_search_domains" {
-  type        = list(string)
-  description = "Container DNS search domains. A list of DNS search domains that are presented to the container"
-  default     = null
-}
-
-variable "ulimits" {
-  type = list(object({
-    name      = string
-    hardLimit = number
-    softLimit = number
-  }))
-  description = "Container ulimit settings. This is a list of maps, where each map should contain \"name\", \"hardLimit\" and \"softLimit\""
-  default     = null
-}
-
-variable "repository_credentials" {
-  type        = map(string)
-  description = "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"
-  default     = null
-}
-
-variable "volumes_from" {
-  type = list(object({
-    sourceContainer = string
-    readOnly        = bool
-  }))
-  description = "A list of VolumesFrom maps which contain \"sourceContainer\" (name of the container that has the volumes to mount) and \"readOnly\" (whether the container can write to the volume)"
-  default     = []
-}
-
-variable "links" {
-  type        = list(string)
-  description = "List of container names this container can communicate with without port mappings"
-  default     = null
-}
-
-variable "user" {
-  type        = string
-  description = "The user to run as inside the container. Can be any of these formats: user, user:group, uid, uid:gid, user:gid, uid:group. The default (null) will use the container's configured `USER` directive or root if not set."
-  default     = null
-}
-
-variable "container_depends_on" {
-  type = list(object({
-    containerName = string
-    condition     = string
-  }))
-  description = "The dependencies defined for container startup and shutdown. A container can contain multiple dependencies. When a dependency is defined for container startup, for container shutdown it is reversed. The condition can be one of START, COMPLETE, SUCCESS or HEALTHY"
-  default     = null
-}
-
-variable "docker_labels" {
-  type        = map(string)
-  description = "The configuration options to send to the `docker_labels`"
-  default     = null
-}
-
-variable "start_timeout" {
-  type        = number
-  description = "Time duration (in seconds) to wait before giving up on resolving dependencies for a container"
-  default     = null
-}
-
-variable "stop_timeout" {
-  type        = number
-  description = "Time duration (in seconds) to wait before the container is forcefully killed if it doesn't exit normally on its own"
+  description = "Log configuration options to send to a custom log driver for the container. For more details, see https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_LogConfiguration.html"
   default     = null
 }
 
@@ -266,12 +116,6 @@ variable "privileged" {
   default     = null
 }
 
-variable "system_controls" {
-  type        = list(map(string))
-  description = "A list of namespaced kernel parameters to set in the container, mapping to the --sysctl option to docker run. This is a list of maps: { namespace = \"\", value = \"\"}"
-  default     = null
-}
-
 variable "hostname" {
   type        = string
   description = "The hostname to use for your container."

examples/complete/fixtures.us-east-2.tfvars

@@ -1,5 +1,5 @@
 terraform {
-  required_version = ">= 0.13.0"
+  required_version = ">= 1.3.0"
 
   required_providers {
     local = {

examples/complete/variables.tf

@@ -1,57 +1,26 @@
 locals {
-  # Sort environment variables so terraform will not try to recreate on each plan/apply
-  env_vars_keys        = var.map_environment != null ? keys(var.map_environment) : var.environment != null ? [for m in var.environment : lookup(m, "name")] : []
-  env_vars_values      = var.map_environment != null ? values(var.map_environment) : var.environment != null ? [for m in var.environment : lookup(m, "value")] : []
-  env_vars_as_map      = zipmap(local.env_vars_keys, local.env_vars_values)
-  sorted_env_vars_keys = sort(local.env_vars_keys)
+  # Sort environment variables & secrets so terraform will not try to recreate on each plan/apply
+  env_as_map     = var.map_environment != null ? var.map_environment : var.environment != null ? { for m in var.environment : m.name => m.value } : null
+  secrets_as_map = var.map_secrets != null ? var.map_secrets : var.secrets != null ? { for m in var.secrets : m.name => m.valueFrom } : null
 
-  sorted_environment_vars = [
-    for key in local.sorted_env_vars_keys :
+  # https://www.terraform.io/docs/configuration/expressions.html#null
+  final_environment_vars = local.env_as_map != null ? [
+    for k, v in local.env_as_map :
     {
-      name  = key
-      value = lookup(local.env_vars_as_map, key)
+      name  = k
+      value = v
     }
-  ]
-
-  # Sort secrets so terraform will not try to recreate on each plan/apply
-  secrets_keys        = var.map_secrets != null ? keys(var.map_secrets) : var.secrets != null ? [for m in var.secrets : lookup(m, "name")] : []
-  secrets_values      = var.map_secrets != null ? values(var.map_secrets) : var.secrets != null ? [for m in var.secrets : lookup(m, "valueFrom")] : []
-  secrets_as_map      = zipmap(local.secrets_keys, local.secrets_values)
-  sorted_secrets_keys = sort(local.secrets_keys)
-
-  sorted_secrets_vars = [
-    for key in local.sorted_secrets_keys :
+  ] : null
+  final_secrets_vars = local.secrets_as_map != null ? [
+    for k, v in local.secrets_as_map :
     {
-      name      = key
-      valueFrom = lookup(local.secrets_as_map, key)
+      name      = k
+      valueFrom = v
     }
-  ]
-
-  mount_points = length(var.mount_points) > 0 ? [
-    for mount_point in var.mount_points : {
-      containerPath = lookup(mount_point, "containerPath")
-      sourceVolume  = lookup(mount_point, "sourceVolume")
-      readOnly      = tobool(lookup(mount_point, "readOnly", false))
-    }
-  ] : var.mount_points
-
-  # https://www.terraform.io/docs/configuration/expressions.html#null
-  final_environment_vars = length(local.sorted_environment_vars) > 0 ? local.sorted_environment_vars : []
-  final_secrets_vars     = length(local.sorted_secrets_vars) > 0 ? local.sorted_secrets_vars : null
+  ] : null
 
-  log_configuration_secret_options = var.log_configuration != null ? lookup(var.log_configuration, "secretOptions", null) : null
-  log_configuration_with_null = var.log_configuration == null ? null : {
-    logDriver = tostring(lookup(var.log_configuration, "logDriver"))
-    options   = tomap(lookup(var.log_configuration, "options"))
-    secretOptions = local.log_configuration_secret_options == null ? null : [
-      for secret_option in tolist(local.log_configuration_secret_options) : {
-        name      = tostring(lookup(secret_option, "name"))
-        valueFrom = tostring(lookup(secret_option, "valueFrom"))
-      }
-    ]
-  }
-  log_configuration_without_null = local.log_configuration_with_null == null ? null : {
-    for k, v in local.log_configuration_with_null :
+  log_configuration_without_null = var.log_configuration == null ? null : {
+    for k, v in var.log_configuration :
     k => v
     if v != null
   }
@@ -65,7 +34,7 @@ locals {
     command                = var.command
     workingDirectory       = var.working_directory
     readonlyRootFilesystem = var.readonly_root_filesystem
-    mountPoints            = local.mount_points
+    mountPoints            = var.mount_points
     dnsServers             = var.dns_servers
     dnsSearchDomains       = var.dns_search_domains
     ulimits                = var.ulimits
@@ -104,5 +73,13 @@ locals {
     k => v
     if v != null
   }
-  json_map = jsonencode(merge(local.container_definition_without_null, var.container_definition))
+
+  container_definition_override_without_null = {
+    for k, v in var.container_definition :
+    k => v
+    if v != null
+  }
+
+  final_container_definition = merge(local.container_definition_without_null, local.container_definition_override_without_null)
+  json_map                   = jsonencode(local.final_container_definition)
 }

examples/complete/versions.tf

@@ -10,7 +10,7 @@ output "json_map_encoded" {
 
 output "json_map_object" {
   description = "JSON map encoded container definition"
-  value       = jsondecode(local.json_map)
+  value       = local.final_container_definition
 }
 
 output "sensitive_json_map_encoded_list" {
@@ -27,6 +27,6 @@ output "sensitive_json_map_encoded" {
 
 output "sensitive_json_map_object" {
   description = "JSON map encoded container definition (sensitive)"
-  value       = jsondecode(local.json_map)
+  value       = local.final_container_definition
   sensitive   = true
 }

main.tf

@@ -1,8 +1,8 @@
 module github.com/cloudposse/terraform-aws-ecs-container-definition
 
-go 1.14
+go 1.15
 
 require (
-	github.com/gruntwork-io/terratest v0.30.23
-	github.com/stretchr/testify v1.6.1
+	github.com/gruntwork-io/terratest v0.43.0
+	github.com/stretchr/testify v1.8.1
 )