Refactor route table module

[ronaldour]

• Decouple route table associations from propagations. An attachment can be associated to a single route table but propagated to multiple route tables. Having separate variables for both allows managing them separately. Reference here.

• Also changed the definition of the associations and propagations variable to list instead of map for a cleaner definition. The attachment-id is used as the key in the for_each to prevent recreation.

• Renamed routes to static_routes. AWS refers to these as Static Routes in their documentation so just trying to use the same terminology. Also changed the variable from a map to a list and used the destination_cidr_block as the key for the for_each since every route requires a destination_cidr_block and it can't be duplicated. I believe this makes it easier to define than trying to figure out a name for each route to use as a key.

[ronaldour]

@bryantbiggs I have many updates like these that I created over the significant-refactor branch that I would like to review, I'm trying to introduce them as small separate changes so it's easier to review but I don't know if that's the right path, if you could share some guidance around that I'd appreciate it 🙏

[bryantbiggs]

I don't have merge rights here - but regardless, I don't understand the changes or the motivation. most of the breaking change is moving from count (arrays) to for_each (maps) to reduce disruptive changes when modifying - this change appears to be going backwards. if you can instead describe the desired end solution, perhaps providing an example that shows how the current proposed PR terraform-aws-modules#113 doesn't work, then we can iterate on that

[ronaldour]

We still use for_each to prevent changes, it's just the parameter that it's a list instead of a map. We use the destination_cidr_block as the key (it's a good candidate because duplicated cidrs are not allowed in the routes).

I think this simplifies using the module but if that's not a good Terraform practice we can stick with maps

[ronaldour]

@bryantbiggs thanks for taking the time for reviewing this.

Current usage:

module "transit_gateway_route_table" {
  source  = "terraform-aws-modules/transit-gateway/aws//modules/route-table"

  name               = "example"
  transit_gateway_id = module.transit_gateway.id

  associations = {
    vpc1 = {
      transit_gateway_attachment_id = module.transit_gateway.vpc_attachments["vpc1"].id
      # ISSUE 1 - Can't enable propagation without enabling association, we should be able to add associations and propagations independently
      propagate_route_table         = true  
    }
  }

  routes = {
     # ISSUE 2 - (This is more of an improvement rather than an issue)
     # We have to come up with names for the keys for each route because we are using a map. Usually that would be either a 
     # meaningless name (e.g. route1, route2) or the destination_cidr block which then we're duplicating 
    "10.1.0.0/16" = {
      destination_cidr_block         = "10.1.0.0/16"
      transit_gateway_attachment_id  = module.transit_gateway.vpc_attachments["vpc2"].id
    }
    "10.1.0.0/16" = {
      destination_cidr_block         = "10.2.0.0/16"
      transit_gateway_attachment_id  = module.transit_gateway.vpc_attachments["vpc3"].id
    }
  }
}

Proposed Usage:

module "transit_gateway_route_table" {
  source  = "terraform-aws-modules/transit-gateway/aws//modules/route-table"

  name               = "example"
  transit_gateway_id = module.transit_gateway.id

  # FIX 1 - Use independent paramenters for associations and propagations. We are using lists for the parameter but we're
  # still using a for_each (not count) on the resource. 
  associations = [
    {
      transit_gateway_attachment_id = module.transit_gateway.vpc_attachments["vpc1"].id
      replace_existing_association  = true
    },
    {
      transit_gateway_attachment_id = module.transit_gateway.vpc_attachments["vpc2"].id
    },
  ]

  propagations = [ module.transit_gateway.vpc_attachments["vpc1"].id, module.transit_gateway.vpc_attachments["vpc2"].id]

  static_routes = [
     # FIX 2 - Use a list instead of a map, but we still use a for_each (not a count) on the resource definition, using the 
     # destination_cidr_block as the key for the for_each because each route can't have a duplicate cidr_block. 
    {
      destination_cidr_block         = "10.1.0.0/16"
      transit_gateway_attachment_id  = module.transit_gateway.vpc_attachments["vpc2"].id
    },
    {
      destination_cidr_block         = "10.2.0.0/16"
      transit_gateway_attachment_id  = module.transit_gateway.vpc_attachments["vpc3"].id
    }
  ]
}

The main change of this PR is decoupling Route Table Associations from Route Table Propagations since we should be able to enable them independently.
The other proposed changes are not really necessary but I thought they could improve the module since this release is going to be a major version with lot's of breaking changes anyways. But if you think those are not a good idea from a Terraform perspective I can remove them.

[ronaldour]

Actually I just realized there are issues with this approach, I'm going to close the PR and come up with a better proposal