terraform-aws-ecs
terraform-aws-ecs copied to clipboard
terraform-aws-modules
container-definition module not usable on its own
Description
This is kinda a reopening of #122. I also hit the same issues and will try to explain as best as I can.
First thing first, the module is working perfectly fine. For example (a very simplified example)
variable "subnet_ids" {
type = list(string)
}
locals {
name = "test-ecs-module"
tags = {
Env = "test"
Project = "ecs-module"
}
}
module "cluster" {
source = "terraform-aws-modules/ecs/aws//modules/cluster"
cluster_name = local.name
fargate_capacity_providers = {
FARGATE = {
default_capacity_provider_strategy = {
weight = 100
}
}
}
tags = local.tags
}
module "nginx" {
source = "terraform-aws-modules/ecs/aws//modules/container-definition"
version = "5.7.3"
name = local.name
service = local.name
essential = true
readonly_root_filesystem = false
image = "public.ecr.aws/nginx/nginx:1.25.3"
mount_points = [
{
containerPath = "/conf/"
sourceVolume = "conf"
readOnly = true
}
]
port_mappings = [
{
containerPort = 80
hostPort = 80
protocol = "tcp"
}
]
enable_cloudwatch_logging = true
create_cloudwatch_log_group = false
}
module "service" {
source = "terraform-aws-modules/ecs/aws//modules/service"
version = "5.7.3"
name = local.name
cluster_arn = module.cluster.arn
cpu = 256
memory = 512
desired_count = 1
launch_type = "FARGATE"
create_task_exec_iam_role = true
create_tasks_iam_role = true
create_security_group = true
security_group_rules = [
{
description = "Allow egress"
type = "egress"
protocol = "all"
from_port = 0
to_port = 65535
cidr_blocks = ["0.0.0.0/0"]
}
]
subnet_ids = var.subnet_ids
network_mode = "awsvpc"
assign_public_ip = false
container_definitions = {
(local.name) = {
essential = true
readonly_root_filesystem = false
image = "public.ecr.aws/nginx/nginx:1.25.3"
mount_points = [
{
containerPath = "/conf/"
sourceVolume = "conf"
readOnly = true
}
]
port_mappings = [
{
containerPort = 80
hostPort = 80
protocol = "tcp"
}
]
enable_cloudwatch_logging = true
}
}
volume = [
{
name : "conf"
}
]
enable_autoscaling = false
ignore_task_definition_changes = false
tags = local.tags
propagate_tags = "TASK_DEFINITION"
}
This works. However, in some of our services, we have up to 5 containers. Each one of them may have its own port mappings, own volumes, own commands and so. Having that within only one resource is extremely hard to read and maintain.
On our current stack, we split each container definition in its own module, and the service only refers to all container definitions. This way, one can better identify the resource, its properties and so on.
Using the current module, it would give something like this :
variable "subnet_ids" {
type = list(string)
}
locals {
name = "test-ecs-module"
tags = {
Env = "test"
Project = "ecs-module"
}
}
module "cluster" {
source = "terraform-aws-modules/ecs/aws//modules/cluster"
cluster_name = local.name
fargate_capacity_providers = {
FARGATE = {
default_capacity_provider_strategy = {
weight = 100
}
}
}
tags = local.tags
}
module "nginx" {
source = "terraform-aws-modules/ecs/aws//modules/container-definition"
version = "5.7.3"
name = local.name
service = local.name
essential = true
readonly_root_filesystem = false
image = "public.ecr.aws/nginx/nginx:1.25.3"
mount_points = [
{
containerPath = "/conf/"
sourceVolume = "conf"
readOnly = true
}
]
port_mappings = [
{
containerPort = 80
hostPort = 80
protocol = "tcp"
}
]
enable_cloudwatch_logging = true
create_cloudwatch_log_group = false
}
module "service" {
source = "terraform-aws-modules/ecs/aws//modules/service"
version = "5.7.3"
name = local.name
cluster_arn = module.cluster.arn
cpu = 256
memory = 512
desired_count = 1
launch_type = "FARGATE"
create_task_exec_iam_role = true
create_tasks_iam_role = true
create_security_group = true
security_group_rules = [
{
description = "Allow egress"
type = "egress"
protocol = "all"
from_port = 0
to_port = 65535
cidr_blocks = ["0.0.0.0/0"]
}
]
subnet_ids = var.subnet_ids
network_mode = "awsvpc"
assign_public_ip = false
container_definitions = {
(local.name) = module.nginx.container_definition
}
volume = [
{
name : "conf"
}
]
enable_autoscaling = false
ignore_task_definition_changes = false
tags = local.tags
propagate_tags = "TASK_DEFINITION"
}
However, this does not work because of the case used in container-definition outputs for fields composed by multiple words.
Hence, instead of having portMappings, it should return the proper property name port_mappings
I made it work by modifying the local module from
locals {
is_not_windows = contains(["LINUX"], var.operating_system_family)
log_group_name = "/aws/ecs/${var.service}/${var.name}"
log_configuration = merge(
{ for k, v in {
logDriver = "awslogs",
options = {
awslogs-region = data.aws_region.current.name,
awslogs-group = try(aws_cloudwatch_log_group.this[0].name, ""),
awslogs-stream-prefix = "ecs"
},
} : k => v if var.enable_cloudwatch_logging },
var.log_configuration
)
linux_parameters = var.enable_execute_command ? merge({ "initProcessEnabled" : true }, var.linux_parameters) : merge({ "initProcessEnabled" : false }, var.linux_parameters)
definition = {
command = length(var.command) > 0 ? var.command : null
cpu = var.cpu
dependsOn = length(var.dependencies) > 0 ? var.dependencies : null # depends_on is a reserved word
disableNetworking = local.is_not_windows ? var.disable_networking : null
dnsSearchDomains = local.is_not_windows && length(var.dns_search_domains) > 0 ? var.dns_search_domains : null
dnsServers = local.is_not_windows && length(var.dns_servers) > 0 ? var.dns_servers : null
dockerLabels = length(var.docker_labels) > 0 ? var.docker_labels : null
dockerSecurityOptions = length(var.docker_security_options) > 0 ? var.docker_security_options : null
entrypoint = length(var.entrypoint) > 0 ? var.entrypoint : null
environment = var.environment
environmentFiles = length(var.environment_files) > 0 ? var.environment_files : null
essential = var.essential
extraHosts = local.is_not_windows && length(var.extra_hosts) > 0 ? var.extra_hosts : null
firelensConfiguration = length(var.firelens_configuration) > 0 ? var.firelens_configuration : null
healthCheck = length(var.health_check) > 0 ? var.health_check : null
hostname = var.hostname
image = var.image
interactive = var.interactive
links = local.is_not_windows && length(var.links) > 0 ? var.links : null
linuxParameters = local.is_not_windows && length(local.linux_parameters) > 0 ? local.linux_parameters : null
logConfiguration = length(local.log_configuration) > 0 ? local.log_configuration : null
memory = var.memory
memoryReservation = var.memory_reservation
mountPoints = var.mount_points
name = var.name
portMappings = var.port_mappings
privileged = local.is_not_windows ? var.privileged : null
pseudoTerminal = var.pseudo_terminal
readonlyRootFilesystem = local.is_not_windows ? var.readonly_root_filesystem : null
repositoryCredentials = length(var.repository_credentials) > 0 ? var.repository_credentials : null
resourceRequirements = length(var.resource_requirements) > 0 ? var.resource_requirements : null
secrets = length(var.secrets) > 0 ? var.secrets : null
startTimeout = var.start_timeout
stopTimeout = var.stop_timeout
systemControls = length(var.system_controls) > 0 ? var.system_controls : null
ulimits = local.is_not_windows && length(var.ulimits) > 0 ? var.ulimits : null
user = local.is_not_windows ? var.user : null
volumesFrom = var.volumes_from
workingDirectory = var.working_directory
}
# Strip out all null values, ECS API will provide defaults in place of null/empty values
container_definition = { for k, v in local.definition : k => v if v != null }
}
to
locals {
is_not_windows = contains(["LINUX"], var.operating_system_family)
log_group_name = "/aws/ecs/${var.service}/${var.name}"
log_configuration = merge(
{ for k, v in {
logDriver = "awslogs",
options = {
awslogs-region = data.aws_region.current.name,
awslogs-group = try(aws_cloudwatch_log_group.this[0].name, ""),
awslogs-stream-prefix = "ecs"
},
} : k => v if var.enable_cloudwatch_logging },
var.log_configuration
)
linux_parameters = var.enable_execute_command ? merge({ "initProcessEnabled" : true }, var.linux_parameters) : merge({ "initProcessEnabled" : false }, var.linux_parameters)
definition = {
command = length(var.command) > 0 ? var.command : null
cpu = var.cpu
depends_on = length(var.dependencies) > 0 ? var.dependencies : null # depends_on is a reserved word
disable_networking = local.is_not_windows ? var.disable_networking : null
dns_search_domains = local.is_not_windows && length(var.dns_search_domains) > 0 ? var.dns_search_domains : null
dns_servers = local.is_not_windows && length(var.dns_servers) > 0 ? var.dns_servers : null
docker_labels = length(var.docker_labels) > 0 ? var.docker_labels : null
docker_security_options = length(var.docker_security_options) > 0 ? var.docker_security_options : null
entrypoint = length(var.entrypoint) > 0 ? var.entrypoint : null
environment = var.environment
environment_diles = length(var.environment_files) > 0 ? var.environment_files : null
essential = var.essential
extra_hosts = local.is_not_windows && length(var.extra_hosts) > 0 ? var.extra_hosts : null
firelens_configuration = length(var.firelens_configuration) > 0 ? var.firelens_configuration : null
health_check = length(var.health_check) > 0 ? var.health_check : null
hostname = var.hostname
image = var.image
interactive = var.interactive
links = local.is_not_windows && length(var.links) > 0 ? var.links : null
linux_parameters = local.is_not_windows && length(local.linux_parameters) > 0 ? local.linux_parameters : null
log_configuration = length(local.log_configuration) > 0 ? local.log_configuration : null
memory = var.memory
memory_reservation = var.memory_reservation
mount_points = var.mount_points
name = var.name
port_mappings = var.port_mappings
privileged = local.is_not_windows ? var.privileged : null
pseudo_terminal = var.pseudo_terminal
readonly_root_filesystem = local.is_not_windows ? var.readonly_root_filesystem : null
repository_credentials = length(var.repository_credentials) > 0 ? var.repository_credentials : null
resource_requirements = length(var.resource_requirements) > 0 ? var.resource_requirements : null
secrets = length(var.secrets) > 0 ? var.secrets : null
start_timeout = var.start_timeout
stop_timeout = var.stop_timeout
system_controls = length(var.system_controls) > 0 ? var.system_controls : null
ulimits = local.is_not_windows && length(var.ulimits) > 0 ? var.ulimits : null
user = local.is_not_windows ? var.user : null
volumes_from = var.volumes_from
working_directory = var.working_directory
}
# Strip out all null values, ECS API will provide defaults in place of null/empty values
container_definition = { for k, v in local.definition : k => v if v != null }
}
Versions
- Module version [Required]:
5.7.3 - Terraform version:
v1.6.5 - Provider version(s):
provider registry.terraform.io/hashicorp/aws v5.30.0
Reproduction Code [Required]
Provided above.
Steps to reproduce the behavior:
Just run
terraform init && terraform apply -subnet_ids='[<your own list>]'
Expected behavior
It would be better for the container-definition module to be usable on its own.
Actual behavior
We can only use the service module with all container definition inside it. No split available.
Hence, instead of having portMappings, it should return the proper property name port_mappings
No, I beg to disagree. The container definition sub-module should render a JSON document that meets the container definition API https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html
The API expects portMappings not port_mappings - the snake case is a Terraform convention, but it will fail if you try to pass this to the container definition API. You can even see this reflected in the examples on the AWS provider https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ecs_task_definition#basic-example
I thought about something like this but haven't the possibility to test it right now.
However, would it be possible to make the output compatible, without touching the module itself ?
This would be the best of both world (unless I miss something else)
I'd like to know more about whats working on not working. we tend to jump into solutioning which muddies the water a bit.
Are you stating that this does not work?
variable "subnet_ids" {
type = list(string)
}
locals {
name = "test-ecs-module"
tags = {
Env = "test"
Project = "ecs-module"
}
}
module "cluster" {
source = "terraform-aws-modules/ecs/aws//modules/cluster"
cluster_name = local.name
fargate_capacity_providers = {
FARGATE = {
default_capacity_provider_strategy = {
weight = 100
}
}
}
tags = local.tags
}
module "nginx" {
source = "terraform-aws-modules/ecs/aws//modules/container-definition"
version = "5.7.3"
name = local.name
service = local.name
essential = true
readonly_root_filesystem = false
image = "public.ecr.aws/nginx/nginx:1.25.3"
mount_points = [
{
containerPath = "/conf/"
sourceVolume = "conf"
readOnly = true
}
]
port_mappings = [
{
containerPort = 80
hostPort = 80
protocol = "tcp"
}
]
enable_cloudwatch_logging = true
create_cloudwatch_log_group = false
}
module "service" {
source = "terraform-aws-modules/ecs/aws//modules/service"
version = "5.7.3"
name = local.name
cluster_arn = module.cluster.arn
cpu = 256
memory = 512
desired_count = 1
launch_type = "FARGATE"
create_task_exec_iam_role = true
create_tasks_iam_role = true
create_security_group = true
security_group_rules = [
{
description = "Allow egress"
type = "egress"
protocol = "all"
from_port = 0
to_port = 65535
cidr_blocks = ["0.0.0.0/0"]
}
]
subnet_ids = var.subnet_ids
network_mode = "awsvpc"
assign_public_ip = false
container_definitions = {
(local.name) = jsonencode(module.nginx.container_definition)
}
volume = [
{
name : "conf"
}
]
enable_autoscaling = false
ignore_task_definition_changes = false
tags = local.tags
propagate_tags = "TASK_DEFINITION"
}
Yes, exactly. In fact, it is working in terraform sense, since we have no errors on apply. However, the task definition is not complete
In the plan, we can see that some fields are missing (all that are composed by multiple words) see mountPoints, portMappings
# module.service.aws_ecs_task_definition.this[0] will be created
+ resource "aws_ecs_task_definition" "this" {
+ arn = (known after apply)
+ arn_without_revision = (known after apply)
+ container_definitions = jsonencode(
[
+ {
+ environment = []
+ essential = true
+ image = "public.ecr.aws/nginx/nginx:1.25.3"
+ interactive = false
+ linuxParameters = {
+ initProcessEnabled = false
}
+ logConfiguration = {
+ logDriver = "awslogs"
+ options = {
+ awslogs-group = "/aws/ecs/test-ecs-module/test-ecs-module"
+ awslogs-region = "eu-west-1"
+ awslogs-stream-prefix = "ecs"
}
}
+ mountPoints = []
+ name = "test-ecs-module"
+ portMappings = []
+ privileged = false
+ pseudoTerminal = false
+ readonlyRootFilesystem = true
+ startTimeout = 30
+ stopTimeout = 120
+ user = "0"
+ volumesFrom = []
},
]
)
+ cpu = "256"
+ execution_role_arn = (known after apply)
+ family = "test-ecs-module"
+ id = (known after apply)
+ memory = "512"
+ network_mode = "awsvpc"
+ requires_compatibilities = [
+ "FARGATE",
]
+ revision = (known after apply)
+ skip_destroy = false
+ tags = {
+ "Env" = "test"
+ "Project" = "ecs-module"
}
+ tags_all = {
+ "Env" = "test"
+ "Project" = "ecs-module"
}
+ task_role_arn = (known after apply)
+ runtime_platform {
+ cpu_architecture = "X86_64"
+ operating_system_family = "LINUX"
}
+ volume {
+ name = "conf"
}
}
So we have no errors, but the service may not be functioning normally.
When I use this:
provider "aws" {
region = "us-east-1"
}
locals {
name = "nginx"
}
module "nginx" {
source = "terraform-aws-modules/ecs/aws//modules/container-definition"
version = "~> 5.0"
name = local.name
service = local.name
essential = true
readonly_root_filesystem = false
image = "public.ecr.aws/nginx/nginx:1.25.3"
mount_points = [
{
containerPath = "/conf/"
sourceVolume = "conf"
readOnly = true
}
]
port_mappings = [
{
containerPort = 80
hostPort = 80
protocol = "tcp"
}
]
enable_cloudwatch_logging = true
create_cloudwatch_log_group = false
}
resource "local_file" "out" {
content = jsonencode(module.nginx.container_definition)
filename = "${path.module}/container_definition.json"
}
I get a container definition that has all of the expected values based on the inputs:
{
"environment": [],
"essential": true,
"image": "public.ecr.aws/nginx/nginx:1.25.3",
"interactive": false,
"linuxParameters": {
"initProcessEnabled": false
},
"logConfiguration": {
"logDriver": "awslogs",
"options": {
"awslogs-group": "",
"awslogs-region": "us-east-1",
"awslogs-stream-prefix": "ecs"
}
},
"mountPoints": [
{
"containerPath": "/conf/",
"readOnly": true,
"sourceVolume": "conf"
}
],
"name": "nginx",
"portMappings": [
{
"containerPort": 80,
"hostPort": 80,
"protocol": "tcp"
}
],
"privileged": false,
"pseudoTerminal": false,
"readonlyRootFilesystem": false,
"startTimeout": 30,
"stopTimeout": 120,
"volumesFrom": []
}
I do agree and am able to reproduce.
But in this file, we have eg mountPoints where in the service module, it looks for mount_points
mount_points = try(each.value.mount_points, var.container_definition_defaults.mount_points, [])
And the same goes for portMappings
You're welcome. If I can be of only help (for PR or anything else) please let me know (I just don't see at the moment what would be the best way to make it work without breaking everything)
This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Yes it appears the output of the container_definiton sub-module container_definition outputs in a different format than what the service submodule container_definiton expects.
container_definition output has a camel casing of portMapping whereas the service is looking for snake_case port_mappings
container_definiton submodule - portMappings = var.port_mappings
service submodule - port_mappings = try(each.value.port_mappings, var.container_definition_defaults.port_mappings, [])
This means this trying to use the output module.container_definition.container_definition inside the service module results in empty [] for portMappings (and any other variable that is camel case)
This is also happening with environment_files. So I am assuming that all the snakeCasing ones are not working because of this same issue, right?
A workaround to still be able to create standalone task definitions is modify line 594 of services module from
locals {
create_task_definition = var.create && var.create_task_definition
to
locals {
create_task_definition = var.create_task_definition
This way, when calling the service module, setting var.create to false but var.create_task_definition to true will create a correctly formatted task definition.
I did create a fork with that change here, but as it's not a solution to the container-definition module but only a workaround, I'm not sure this will pass a review if I open a PR. Hope this helps someone!
