cloudposse / terraform-aws-vpc-peering-multi-account

Terraform module to provision a VPC peering across multiple VPCs in different accounts by using multiple providers
https://cloudposse.com/accelerate
Apache License 2.0
129 stars 92 forks source link

Module fails if subnet uses default "main" routing table #45

Closed Nuru closed 2 years ago

Nuru commented 3 years ago

Describe the Bug

If one of the subnets in one of the VPCs is using the default "main" routing table, the module fails.

Steps to Reproduce

Steps to reproduce the behavior:

  1. Create a new VPC
  2. Create new subnets in the VPC without associating route tables with them
  3. Use the VPC as the "accepter"
  4. See error
Error: query returned no results. Please change your search criteria and try again

  on .terraform/modules/vpc_peering/accepter.tf line 64, in data "aws_route_table" "accepter":
  64: data "aws_route_table" "accepter" {

Additional Context

This bug was introduced by #44 which incorporated #31 but still did not really fix #30.

I propose eliminating the option to filter subnets because it never really worked. If someone wants it back, they can submit a PR to implement it properly.

Problems with filtering by subnet

Problem 1: Finding the route tables to modify

Sub-problem 1A: Looking up the route table for a subnet

You cannot find the route table for a subnet via

data "aws_route_table" "subnet" {
  subnet_id = local.subnet_id
}

because this will throw an error if the subnet does not have an associated route table and is instead using the default "main" table. That is the source of this bug.

Because there is no way to make this conditional, we simply cannot use data "aws_route_table".

Sub-problem 1B: Looking up the subnet for a route table

You could try looking up all the route tables and filtering them by associated subnets, but

Conclusion: you might as well add routes to all the route tables, because you cannot promise better restrictions

After all, the entire VPC might have only one, default route table.

Problem 2: Why are we looking up subnets by tag, anyway?

The only meaningful restriction we can make is to limit routing to some portion of the VPC's CIDR, so we might as well pass explicit CIDRs for each VPC and just route those in all routing tables. It is not at all clear that selecting the CIDRs by subnet tag is more convenient than explicitly specifying them (or having the root module look them up any way it wants to).

sebastianmacarescu commented 3 years ago

Any workaround for this?

opera-oplakida commented 2 years ago

Any workaround for this?

You can explicitly associate subnets with default route table in console. It worked for me.