Terraform: Terraform crash when using for_each with a list of maps

Created on 19 Aug 2019  ยท  2Comments  ยท  Source: hashicorp/terraform

Terraform Version

Terraform v0.12.6
+ provider.null v2.1.2

Terraform Configuration Files

locals {
  dynamic_self_sg = [
    {
      type      = "ingress"
      from_port = 22
      to_port   = 22
      protocol  = "tcp"
    },
    {
      type      = "ingress"
      from_port = 80
      to_port   = 80
      protocol  = "tcp"
    },
  ]
}

resource "null_resource" "dynamic_self" {
  for_each = [
    for s in local.dynamic_self_sg : {
      type      = s.type
      from_port = s.from_port
      to_port   = s.to_port
      protocol  = s.protocol
    }
    if length(local.dynamic_self_sg) != 0
  ]

  provisioner "local-exec" {
    command = "echo ${each.value}"
  }
}

Debug Output

https://gist.github.com/hmrbarros/ed14ab3b027d1d6f3703e37e3ae3a3fe#file-trace-log

Crash Output

https://gist.github.com/hmrbarros/ed14ab3b027d1d6f3703e37e3ae3a3fe#file-crash-log

Expected Behavior

This is a quick example using null_resource, the same crash happened when using `aws_security_group_rule.
In this exampled I expected an echo containing each map defined in the locals.

Actual Behavior

Terraform crashed

Steps to Reproduce

  1. terraform init
  2. terraform apply
bug config crash

Most helpful comment

Hi @hmrbarros! Sorry for this crash and thanks for reporting it.

It looks like you've hit the same crash that was fixed in #22279 here; that fix is merged into master ready to be included in the forthcoming 0.12.7 release, which should be coming soon.

The crash here is coming from a missing validation check that should instead have returned the following error for this configuration:

Error: Invalid for_each argument

The given "for_each" argument value is unsuitable: the "for_each" argument
must be a map, or set of strings, and you have provided a value of type tuple.

The purpose of for_each is to identify each instance by a string key, so the expression given to for_each must produce a map or a set of strings. In your case there isn't a single attribute to serve a unique key for a security group rule, so you'll need to construct a compound key out of each of the attributes like this:

resource "null_resource" "dynamic_self" {
  for_each = {
    for s in local.dynamic_self_sg : "${s.type} ${s.protocol}:${s.from_port}-${s.to_port}" => s
  }

  provisioner "local-exec" {
    command = "echo ${each.value}"
  }
}

This will produce instances with addresses like null_resource.dynamic_self["ingress tcp:22-22"], so in effect changing any of the values within an object will be interpreted as removing the old object and creating a new one.

If you want to be able to edit certain rules in-place then you can instead change your dynamic_self_sg value to be a map itself, using meaningful keys to identify each rule:

locals {
  dynamic_self_sg = {
    ssh = {
      type      = "ingress"
      from_port = 22
      to_port   = 22
      protocol  = "tcp"
    }
    http = {
      type      = "ingress"
      from_port = 80
      to_port   = 80
      protocol  = "tcp"
    },
  }
}

resource "null_resource" "dynamic_self" {
  for_each = local.dynamic_self_sg

  provisioner "local-exec" {
    command = "echo ${each.value}"
  }
}

In this case, the instances will have addresses like null_resource.dynamic_self["ssh"], so if you want to change the definition of the "ssh" rule later you can do so as an update. In practice for security groups this won't make any significant functional difference because the AWS API itself identifies them by all of their constituent parts anyway, but using meaningful keys may be helpful to humans when reading a Terraform plan.

Some others things to note:

  • You don't need a filter expression like if length(local.dynamic_self_sg) != 0 in your for_each since the for expression will just immediately evaluate to an empty result if the input is empty, without evaluating that expression at all.
  • If the result of your for expression is the same as the input then you don't need to write out the whole object structure again. Instead, you can just refer directly to the iterator variable to get the whole value as-is.

Since the crashing bug was already fixed in #22279 we're going to close this out now, but hopefully the above will help to avoid the crash and also the proper error that will replace it in the forthcoming 0.12.7 release. If you have any follow-up questions, please feel free to ask in the community forum. Thanks!

All 2 comments

Hi @hmrbarros! Sorry for this crash and thanks for reporting it.

It looks like you've hit the same crash that was fixed in #22279 here; that fix is merged into master ready to be included in the forthcoming 0.12.7 release, which should be coming soon.

The crash here is coming from a missing validation check that should instead have returned the following error for this configuration:

Error: Invalid for_each argument

The given "for_each" argument value is unsuitable: the "for_each" argument
must be a map, or set of strings, and you have provided a value of type tuple.

The purpose of for_each is to identify each instance by a string key, so the expression given to for_each must produce a map or a set of strings. In your case there isn't a single attribute to serve a unique key for a security group rule, so you'll need to construct a compound key out of each of the attributes like this:

resource "null_resource" "dynamic_self" {
  for_each = {
    for s in local.dynamic_self_sg : "${s.type} ${s.protocol}:${s.from_port}-${s.to_port}" => s
  }

  provisioner "local-exec" {
    command = "echo ${each.value}"
  }
}

This will produce instances with addresses like null_resource.dynamic_self["ingress tcp:22-22"], so in effect changing any of the values within an object will be interpreted as removing the old object and creating a new one.

If you want to be able to edit certain rules in-place then you can instead change your dynamic_self_sg value to be a map itself, using meaningful keys to identify each rule:

locals {
  dynamic_self_sg = {
    ssh = {
      type      = "ingress"
      from_port = 22
      to_port   = 22
      protocol  = "tcp"
    }
    http = {
      type      = "ingress"
      from_port = 80
      to_port   = 80
      protocol  = "tcp"
    },
  }
}

resource "null_resource" "dynamic_self" {
  for_each = local.dynamic_self_sg

  provisioner "local-exec" {
    command = "echo ${each.value}"
  }
}

In this case, the instances will have addresses like null_resource.dynamic_self["ssh"], so if you want to change the definition of the "ssh" rule later you can do so as an update. In practice for security groups this won't make any significant functional difference because the AWS API itself identifies them by all of their constituent parts anyway, but using meaningful keys may be helpful to humans when reading a Terraform plan.

Some others things to note:

  • You don't need a filter expression like if length(local.dynamic_self_sg) != 0 in your for_each since the for expression will just immediately evaluate to an empty result if the input is empty, without evaluating that expression at all.
  • If the result of your for expression is the same as the input then you don't need to write out the whole object structure again. Instead, you can just refer directly to the iterator variable to get the whole value as-is.

Since the crashing bug was already fixed in #22279 we're going to close this out now, but hopefully the above will help to avoid the crash and also the proper error that will replace it in the forthcoming 0.12.7 release. If you have any follow-up questions, please feel free to ask in the community forum. Thanks!

I'm going to lock this issue because it has been closed for _30 days_ โณ. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

Was this page helpful?
0 / 5 - 0 ratings