terraform-aws-modules / terraform-aws-vpc

Terraform module to create AWS VPC resources πŸ‡ΊπŸ‡¦
https://registry.terraform.io/modules/terraform-aws-modules/vpc/aws
Apache License 2.0
2.97k stars 4.42k forks source link

VPC endpoints are being replaced sporadically due to `service_name` being unknown at plan time #1054

Closed zack-is-cool closed 6 months ago

zack-is-cool commented 6 months ago

Description

vpc endpoints flapping sporadically due to https://github.com/hashicorp/terraform-provider-aws/issues/25568.

Commented on problem here: https://github.com/hashicorp/terraform-provider-aws/issues/25568#issuecomment-1998004128

This is happening for us sporadically. It doesn't happen all the time. Sometimes we apply and suddenly all of our VPC endpoints need to be replaced. We aren't changing anything going into the VPC module or relating to the endpoints. Sidenote - we are deploying in govcloud.

We're using this module:

https://github.com/defenseunicorns/terraform-aws-vpc/blob/main/main.tf#L130-L256 which feeds into this -> https://github.com/terraform-aws-modules/terraform-aws-vpc/blob/master/modules/vpc-endpoints/main.tf#L11-L21

truncated output, but it does this for all of our endpoints that get fed from that aws_vpc_endpoint_service data that feed into the aws_vpc_endpoint resource

         ~ requester_managed     = false -> (known after apply)                                                                                                                  
           ~ route_table_ids       = [] -> (known after apply)                                                                                                                     
           ~ service_name          = "com.amazonaws.us-gov-west-1.ssm" # forces replacement -> (known after apply) # forces replacement                                            
           ~ state                 = "available" -> (known after apply)    

Versions

Reproduction Code [Required]

defenseunicorns/terraform-aws-vpc@main/main.tf#L130-L256 which feeds into this -> terraform-aws-modules/terraform-aws-vpc@master/modules/vpc-endpoints/main.tf#L11-L21

Steps to reproduce the behavior:

  1. Deploy vpc + endpoints using https://github.com/defenseunicorns/terraform-aws-vpc which uses this module
  2. Deploy again some time later. TF will detect that the service names are unknown and replace them for no reason.

Expected behavior

They should not be replaced if nothing is changing.

Actual behavior

The VPC endpoints are forced to be replace due to the use of the aws_vpc_endpoint_service data source, sometimes. This is due to them being known after apply.

Terminal Output Screenshot(s)

basically this, for all VPC endpoints, sometimes.

image

Additional context

possible solution

Ideally this module should allow the use of feeding in a service_name and doing a lookup() on that so that we can build it outside when passing in the map of endpoints, but defaulting to data.aws_vpc_endpoint_service.this[each.key].service_name if none provided.

jordanboston commented 6 months ago

We are also encountering this and it is forcing endpoint creation when we just want to update our Tags for the module. Would love to see a fix for this if it's a bug because we've had to do after hours deployments due to this.

bryantbiggs commented 6 months ago

@zack-is-cool your issue is most likely related to this https://github.com/defenseunicorns/terraform-aws-vpc/blob/f21519a579225c4f7a6e0efc05cf2cf96f0a8b89/main.tf#L255

@jordanboston do you have an explicit depends_on your module definition as well? If so, remove it

zack-is-cool commented 6 months ago

@zack-is-cool your issue is most likely related to this defenseunicorns/terraform-aws-vpc@f21519a/main.tf#L255

It's odd that it's not replacing them each time the TF is applied though? This code is being deployed to long lived environments and just seems to randomly decide it doesn't know the vpc endpoints - this is without messing with the aws_ec2_subnet_cidr_reservation.this object or locals that it uses in the referenced code. I've experienced that depends_on is sometimes finicky in general.

Assuming I can't get rid of this depends_on, do you see any issue with the proposed workaround in the OP^? Or is there another way I should be handling this?

possible solution Ideally this module should allow the use of feeding in a service_name and doing a lookup() on that so that we can build it outside when passing in the map of endpoints, but defaulting to data.aws_vpc_endpoint_service.this[each.key].service_name if none provided.

antonbabenko commented 6 months ago

This issue has been resolved in version 5.7.0 :tada:

github-actions[bot] commented 5 months ago

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.