Terraform-provider-aws: Add aws_ecs_container_definitions_document data source

[jfirebaugh]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Currently, the container_definitions argument of aws_ecs_task_definition must be provided as a "single valid JSON document", with examples given of using file to load from an external document, and some tricky escaping rules.

However, there is precedent for a safer and more convenient mechanism in the aws_iam_policy_document data source, which provides a mechanism to define a JSON document using familiar Terraform syntax, with all its benefits. I'd like to request an analogous mechanism for container definitions, via a new aws_ecs_container_definitions_document data source.

New or Affected Resource(s)

• aws_ecs_container_definitions_document

Potential Terraform Configuration

data "aws_ecs_container_definitions_document" "containers" {
  container {
    name = "first"
    ...
  }

  container {
    name = "second"
    ...
  }
}

resource "aws_ecs_task_definition" "task" {
  family                = "service"
  container_definitions = "${data.aws_ecs_container_definitions_document.containers.json}"
}

References

3153

[bflad]

For some historical context, there was an older pull request that started this conversation here: https://github.com/terraform-providers/terraform-provider-aws/pull/5862#issuecomment-425446332

That note provides some additional talking points before we jump to a specific implementation, such as can we design this in a way where we can take a shared container definition and apply it across multiple container definitions? This was a use case in my previous work, where there were times we needed to install a monitoring agent in all applications and copy-pasting the definition was less than ideal. This may not require two data sources as the note suggests ("source" and "override" arguments similar to the aws_iam_policy_document data source may be better), but I think it would be good to potentially discuss the handling of this scenario to really make this data source more powerful for operators.

[jfirebaugh]

Thank you for the context! I like the design you sketched out in https://github.com/terraform-providers/terraform-provider-aws/pull/5862#issuecomment-425446332, making reuse of container definitions easier. The shared monitoring container is a very common use case.

Also agree with migrating the existing aws_ecs_container_definition to a container_definitions map in the aws_ecs_task_definition data source.

I'm not sure I understand the alternative of "source" and "override" arguments. Could you elaborate on that?

As another alternative, would it be possible to support an array of JSON documents directly in the task definition resource?

resource "aws_ecs_task_definition" "service1" {
  ...
  container_definitions = [
    "${data.aws_ecs_container_definition.monitoring.json}",
    "${data.aws_ecs_container_definition.application1.json}",
  ]
}

resource "aws_ecs_task_definition" "service2" {
  ...
  container_definitions = [
    "${data.aws_ecs_container_definition.monitoring.json}",
    "${data.aws_ecs_container_definition.application2.json}",
  ]
}

[gschaffer-cxn]

I think the main difference regarding source and mergeing and iam_policy_document, it that there are SIDs in an iam_policy_document that indicate clear blocks that require changing, here there are not. The main question I think is whether a container definitions file should generate 1 or more container definitions and the merging is within or the outside the task as @jfirebaugh suggested.

we did solve a couple of similar implementations:

• using a locals block that has the structure of the container definition and then calling jsonencode(). We normally use cloudposse/terraform-aws-ecs-container-definition module, however it was unable serve for a batch task definition so we had to improvise.

locals {
  container_properties = jsonencode({
  locals {
  container_properties = jsonencode({
    image      = var.batch_image_arn
    vcpus      = 2
    memory     = 2048
    command    = []
    jobRoleArn = aws_iam_role.batch_job.arn
    volumes    = []  image      = var.batch_image_arn
    vcpus      = 2
    memory     = 2048
    command    = []
    jobRoleArn = aws_iam_role.batch_job.arn
    volumes    = []
  })
}

• second isse we had to solve has using the same container def for minor differences, basically an incremental count variable. In this case a json template file was used, with common variables between all versions being in locals, and the count variable merged in.

resource "aws_ecs_task_definition" "taskdef" {
  count = var.number_of_shards
 
  family = "application_${var.env_name}_${count.index}"
  container_definitions = templatefile("${path.module}/templates/container_def.json",
    merge(local.container_template_common_variables,
    map("shard_id", "${count.index}")
    )

  ...
)

[tburow]

this feature request would really make things easier and less mistake prone. Especially in more complex containers. right now I have json templates - template resource then pass that as the container definition - requires me to rewrite vars, manually control ARN regions ect. kinda a mess.