Terraform loses track of dependency

[Nuru]

Not positive if this is a Terraform bug or a provider bug, but seems more like a core Terraform bug to me.

In the code below, when a new security group replaces the old one via create_before_destroy, Terraform does not update the VPC endpoint in the same plan/apply cycle. With other resources (such as aws_elasticache_replication_group) it is even more problematic because without updating the resource, Terraform will not remove the resource from the security group, and then AWS will refuse to destroy it, leading to an apply-time failure.

Workaround seems to be to remove compact(). Please provide another workaround if possible.

Terraform Version

Terraform v1.0.9
on darwin_amd64
+ provider registry.terraform.io/hashicorp/aws v3.63.0

Terraform Configuration Files

Click to reveal

variable "region" {
  type    = string
  default = "us-east-2"
}

provider "aws" {
  region = var.region
}

variable "sg_name" {
  type    = string
  default = ""
}

resource "aws_vpc" "main" {
  cidr_block = "10.111.0.0/16"
}

variable "associated_security_group_ids" {
  type    = list(string)
  default = []
}
variable "existing_security_groups" {
  type    = list(string)
  default = []
}
locals {
  associated_security_group_ids = concat(var.existing_security_groups, var.associated_security_group_ids)
}



module "ingress" {
  source  = "cloudposse/security-group/aws"
  version = "0.4.2"

  create_before_destroy = true
  security_group_name   = length(var.sg_name) > 0 ? [var.sg_name] : []

  allow_all_egress = false
  vpc_id           = aws_vpc.main.id
  rules = [{
    key              = "allow_all_ingress"
    type             = "ingress"
    from_port        = 0
    to_port          = 0
    protocol         = "-1"
    cidr_blocks      = ["0.0.0.0/0"]
    ipv6_cidr_blocks = ["::/0"]
    description      = "Allow all inbound traffic"
  }]
}

module "egress" {
  source  = "cloudposse/security-group/aws"
  version = "0.4.2"

  create_before_destroy = true

  allow_all_egress = true
  vpc_id           = aws_vpc.main.id
}

resource "aws_vpc_endpoint" "ec2" {
  vpc_id            = aws_vpc.main.id
  service_name      = "com.amazonaws.${var.region}.ec2"
  vpc_endpoint_type = "Interface"

  security_group_ids = compact(concat(local.associated_security_group_ids, [
    module.ingress.id,
    module.egress.id,
  ]))

  private_dns_enabled = false
}

Steps to Reproduce

1. terraform init
2. terraform apply
3. tf1 apply -var sg_name="first_replacement" -parallelism=1
4. tf1 apply -var sg_name="first_replacement" -parallelism=1

Expected Behavior

The first tf1 apply -var sg_name="first_replacement" -parallelism=1 should create a new security group, update the aws_vpc_endpoint resource, replacing the old security group with the new one, then delete the old security group.

The second tf1 apply -var sg_name="first_replacement" -parallelism=1 should do nothing

Actual Behavior

The first tf1 apply -var sg_name="first_replacement" -parallelism=1 creates a new security group and deletes the old security group.

The second tf1 apply -var sg_name="first_replacement" -parallelism=1 updates the aws_vpc_endpoint resource, replacing the old security group with the new one.

Additional Context

If you replace

  security_group_ids = compact(concat(local.associated_security_group_ids, [
    module.ingress.id,
    module.egress.id,
  ]))

with

  security_group_ids = concat(local.associated_security_group_ids, [
    module.ingress.id,
    module.egress.id,
  ])

removing the compact() operation, then Terraform works as expected.

[jbardin]

Hi @Nuru,

Thanks for filing the issue, and the complete example! There's no dependency problem here, the behavior you are seeing is solely the result of how aws_vpc_endpoint is planing its changes, which itself is a result of the legacy SDK behavior. Unfortunately there's likely nothing that be done about this edge case right now, until the provider is free from the limitations of legacy SDK.

I did not carefully review the modules themselves, but the optimal case here is that the outputs can be assured to never be empty strings so that the compact function is not needed when assigning these values. It of course won't avoid the problem in all possible cases, but reduces the possibility of the resource not being able to plan the changes correctly.

A few details for those interested:

When the ingress id is planned to be replaced, it becomes unknown in the security_group_ids attribute expression during planning. Because the compact function sees an unknown value which it cannot compare to an empty string, it must return an entirely unknown list as the result. The resource then sees this unknown value, and because the legacy SDK conflated unknown and computed values, and also cannot detect when an optional+computed value becomes unset, it determines there's no change to be made.

Without the compact function, a set of 2 values is passed to the provider for security_group_ids, one known string and one unknown string. This time the provider (or SDK) can now detect there is a change to be made, and it plans the update more or less correctly (though there will probably still be some warnings about incorrectly planned values in the logs).

[jbardin]

And found the original SDK issue: hashicorp/terraform-plugin-sdk#174

[Nuru]

Thank you, @jbardin, for the quick and detailed reply. I see the linked SDK issue is still open. Please tell me: what is the fix for the AWS provider and how can I track that fix getting into the provider?

[jbardin]

While the behavior shown here is one of many things that cannot be fixed with the legacy SDK, the latest release may have some new features to work around the issue. I would have to defer to the AWS provider developers as to whether updating the SDK and working around the problem is possible.

[github-actions]

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.