Dynamic Block Inside for_each Resource Does Not Behave as Expected

Terraform Providers AWS
1

Sorry for the somewhat vague title, but the issue can’t be summarised without an example.

I have the following resource definition:

resource "aws_security_group" "these" {
  for_each = var.vpc_config.open_ports

  name   = "${local.name}-${each.key}"
  vpc_id = module.vpc[0].vpc_attributes.id
  tags   = local.tags

  dynamic "ingress" {
    for_each = toset(each.value)

    content {
      from_port        = ingress.value
      to_port          = ingress.value
      protocol         = each.key
      cidr_blocks      = ["0.0.0.0/0"]
      ipv6_cidr_blocks = ["::/0"]
    }
  }

Given the following variable definition:

variable "vpc_config" {
  description = "VPC will be created for this application if supplied"

  type = object({
    vpc_cidr = string,
    az_count = optional(number, 3),
    open_ports = optional(object({
      tcp = optional(list(number), [])
      udp = optional(list(number), [])
      }), {
      tcp = [],
      udp = []
    })
  })

  default = null
}

I am able to successfully supply port values for tcp and/or udp. The resulting security groups are what I expect, with the ports specified being in the ingress.

However, if I then remove values I supply for vpc_config.open_ports.tcp/udp, Terraform tells me there are no changes to apply. The only way I can get the values to change are if I supply different values, but the empty list does not empty the ingress rules.

I guess this is to do with the way the dynamic block is being activated, i.e. it’s not, since the for_each has nothing to run on when the list is empty. It still seems surprising Terraform doesn’t see this as a difference though.

2

Hi @afrazo,

I think what you’ve observed here is a behavior of the AWS provider rather than of Terraform itself.

To allow the option of using either inline rule blocks or defining rules as separate resources, the provider treats the total absence of any rule blocks as “keep whatever rules are currently present”. Unfortunately that means that you activate that special exception whenever your dynamic block generates zero blocks.

One way to avoid that quirk would be to declare your rules using a separate aws_security_group_rule resource instead – which can still use for_each – and then it will be Terraform itself dealing with the situation where one or more rules are removed and so it won’t be subject to this design quirk of the AWS provider.

3

Thanks @apparentlymart, helpful as always :slight_smile:

I didn’t know about the new (?) aws_vpc_security_group_ingress_rule whilst I was looking too. It does mean I have to have different resources for the different protocols which makes it less dynamic, though:

Whereas before I had

resource "aws_security_group" "these" {
  for_each = var.vpc_config.open_ports

...

    dynamic "ingress" {
      for_each = toset(each.value)

...

I now have

resource "aws_vpc_security_group_ingress_rule" "tcp" {
  for_each = var.vpc_config.open_ports.tcp

...

Which means open_ports can no longer be dynamic. This isn’t a big problem, since my variable definition made it non-dynamic anyway, but I wasn’t wondering if the only way to achieve the same thing as before (looping through open_ports, then looping through each.value inside the dynamic block) is to make a module and loop through that?

4

Never mind, I was being a dummy. I just switched up my variable definition and it’s now entirely dynamic — actually better than before:

variable "vpc_config" {
  description = "VPC will be created for this application if supplied"

  type = object({
    vpc_cidr = string,
    az_count = optional(number, 3),
    egress_rules = optional(map(object({
      protocol  = optional(string, "-1"),
      cidr      = optional(string, "0.0.0.0/0"),
      from_port = optional(number, 0),
      to_port   = optional(number, 0)
      })
    ), {}),
    ingress_rules = optional(map(object({
      protocol  = string,
      cidr      = string,
      from_port = number,
      to_port   = number
      })
    ), {})
  })

  default = null
}