fix: Dns_options for vpc-endpoints

[taylorsilva]

Description

While trying to setup an S3 VPC endpoint I found I could not set dns_options.private_dns_only_for_inbound_resolver_endpoint even though this was apparently resolved by #1029

To see what was going on under the hood, I removed the try wrapping private_dns_only_for_inbound_resolver_endpoint and got this error:

Planning failed. Terraform encountered an error while generating this plan.

╷
│ Error: Unsupported attribute
│
│   on .terraform/modules/endpoints/modules/vpc-endpoints/main.tf line 42, in resource "aws_vpc_endpoint" "this":
│   42:       private_dns_only_for_inbound_resolver_endpoint = dns_options.value.private_dns_only_for_inbound_resolver_endpoint
│     ├────────────────
│     │ dns_options.value is false
│
│ Can't access attributes on a primitive-typed value (bool).

So this whole thing is already off. Idk why dns_options.value is a bool, all I know is it has something to do with the dynamic block being used here.

I noticed the dynamic block is not needed. You're only allowed one dns_options block for the aws_vpc_endpoint resource: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/vpc_endpoint#dns_options

So I removed the dynamic block and updated the value lookups to each.value.dns_options[] which now works.

For an S3 service, with
dns_options.private_dns_only_for_inbound_resolver_endpoint set to false, this is the plan it generated:

+ resource "aws_vpc_endpoint" "this" {
   + arn                   = (known after apply)
   + cidr_blocks           = (known after apply)
   + dns_entry             = (known after apply)
   + id                    = (known after apply)
   + ip_address_type       = (known after apply)
   + network_interface_ids = (known after apply)
   + owner_id              = (known after apply)
   + policy                = jsonencode(
         {
           + Statement = [
               + {
                   + Action    = "*"
                   + Condition = {
                       + StringNotEquals = {
                           + "aws:SourceVpc" = "vpc-0518b6428b706516d"
                         }
                     }
                   + Effect    = "Deny"
                   + Principal = "*"
                   + Resource  = "*"
                 },
             ]
           + Version   = "2012-10-17"
         }
     )
   + prefix_list_id        = (known after apply)
   + private_dns_enabled   = true
   + requester_managed     = (known after apply)
   + route_table_ids       = (known after apply)
   + security_group_ids    = [
       + "sg-050ed3bb7b46fd681",
     ]
   + service_name          = "com.amazonaws.us-east-1.s3"
   + state                 = (known after apply)
   + subnet_ids            = [
       + "subnet-0af61b529ffb7940b",
       + "subnet-0dd27b92ffe0d02cd",
     ]
   + tags                  = {
       + "Name" = "s3"
     }
   + tags_all              = {
       + "Name" = "s3"
     }
   + vpc_endpoint_type     = "Interface"
   + vpc_id                = "vpc-0518b6428b706516d"

   + dns_options {
       + dns_record_ip_type                             = (known after apply)
       + private_dns_only_for_inbound_resolver_endpoint = false
     }

   + timeouts {
       + create = "10m"
       + delete = "10m"
       + update = "10m"
     }
 }

For a resource that did not pass in a dns_options, like this:

...
ecr_api = {
      service             = "ecr.api"
      private_dns_enabled = true
      subnet_ids          = module.vpc.private_subnets
      policy              = data.aws_iam_policy_document.generic_endpoint_policy.json
    },
...

I got this plan:

+ resource "aws_vpc_endpoint" "this" {
   + arn                   = (known after apply)
   + cidr_blocks           = (known after apply)
   + dns_entry             = (known after apply)
   + id                    = (known after apply)
   + ip_address_type       = (known after apply)
   + network_interface_ids = (known after apply)
   + owner_id              = (known after apply)
   + policy                = jsonencode(
         {
           + Statement = [
               + {
                   + Action    = "*"
                   + Condition = {
                       + StringNotEquals = {
                           + "aws:SourceVpc" = "vpc-0518b6428b706516d"
                         }
                     }
                   + Effect    = "Deny"
                   + Principal = "*"
                   + Resource  = "*"
                 },
             ]
           + Version   = "2012-10-17"
         }
     )
   + prefix_list_id        = (known after apply)
   + private_dns_enabled   = true
   + requester_managed     = (known after apply)
   + route_table_ids       = (known after apply)
   + security_group_ids    = [
       + "sg-050ed3bb7b46fd681",
     ]
   + service_name          = "com.amazonaws.us-east-1.ecr.api"
   + state                 = (known after apply)
   + subnet_ids            = [
       + "subnet-0af61b529ffb7940b",
       + "subnet-0dd27b92ffe0d02cd",
     ]
   + tags                  = {
       + "Name" = "ecr.api"
     }
   + tags_all              = {
       + "Name" = "ecr.api"
     }
   + vpc_endpoint_type     = "Interface"
   + vpc_id                = "vpc-0518b6428b706516d"

   + dns_options {
       + dns_record_ip_type = (known after apply)
     }

   + timeouts {
       + create = "10m"
       + delete = "10m"
       + update = "10m"
     }
 }

I think the removal of the dynamic block simplifies things now. It's easier to see what's going on. There should be no change for anyone that isn't setting dns_options as shown by my plan output above.

Breaking Changes

None. For VPCE's that I had beforehand, nothing changed for them. Existing setup was respected.

How Has This Been Tested?

• I have updated at least one of the examples/* to demonstrate and validate my change(s)
• I have tested and validated these changes using one or more of the provided examples/* projects

• I have executed pre-commit run -a on my pull request

[taylorsilva]

I looked at the pre-commit stuff and it looked like a lot to setup. If you really need me to do it to get the PR in then let me know and I'll make the time. Not feeling it right now because I've already spent a bunch of time troubleshooting this and it's late now 😭

[bryantbiggs]

its dynamic because its an optional argument, so removing the dynamic block is not correct.

lets start with an issue and a reproduction

[erezo9]

@taylorsilva
it should be like this
private_dns_enabled = true
dns_options = {
private_dns_only_for_inbound_resolver_endpoint = true
}

like @bryantbiggs said, its optional that is why i put dynamic in the original PR
it is also mentioned in the examples
https://github.com/terraform-aws-modules/terraform-aws-vpc/blob/master/examples/complete/main.tf#L107

[github-actions]

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.