coalfire-cf / terraform-aws-vpc-nfw Goto Github PK

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

I'd like to see the name from the hashicorp/subnets/cidr added to this module for processing.

module "app_subnet_addrs" {
  source  = "hashicorp/subnets/cidr"
  version = "v1.0.0"

  base_cidr_block = var.app_vpc_cidr
  networks = [
    {
      name     = "${var.resource_prefix}-firewall-1a"
      new_bits = 8
    },
    {
      name     = "${var.resource_prefix}-firewall-1b"
      new_bits = 8
    },
...

Describe the solution you'd like
A clear and concise description of what you want to happen.

The module would need updated to handle a map with a key/value pair to process.

The tagging would be easier, instead of

"Name" = format("%s-${lower(element(values(var.private_subnet_name_tag), count.index))}-%s", var.name, element(var.azs, count.index))

we can just do

"Name" = keys(var.private_subnets)[count.index]

It appears after you deploy a network firewall deployment, and decide to go set the "deploy nfw flag" it thus removes the NFW but leaves an artifact of the GWLBs which in turn breaks the DMZ subnet routing completely. This issue unfortunately wasn't visible during initial PR due to not having an env for us to test RA in.

To resolve:
The Firewall subnets need be removed when the firewall is destroyed and the IGWs must be migrated to the DMZ subnets, or this issue will persist.

Here's how I had solved it before:
https://github.com/Coalfire-CF/terraform-aws-vpc-nfw/blob/b2e656c5026a8d30ee009af633226134f3c55e9e/main.tf

Is your feature request related to a problem? Please describe.
Use of AWS Workspaces require very specific AZ ids to be used (use1-az2, use1-az4, use1-az6), we currently only use AZ names (us-east-1a, us-east-1b, us-east-1c). The implications are that 2 accounts can deploy the same AZ names, but Workspaces can't deploy because the IDs are different between accounts even if the AZ names are the same, and a Workspaces deployment will fail. Other things like AD connectors and Domain Controllers are similarly restricted if Workspaces uses that integration.

Describe the solution you'd like
Use the "availability_zone_id" parameter for "aws_subnet" resources instead of "availability_zone" that we currently do. May also need some validation against the valid AZ IDs that are valid for Workspaces:
https://docs.aws.amazon.com/workspaces/latest/adminguide/azs-workspaces.html

Describe alternatives you've considered
It's possible that this is optional if Workspaces never needs to be used, but it is probably simpler to make it mandatory since it would be difficult (impossible) to rework the networking after deployment if someone wants to use Workspaces down the road beyond the initial planning stages.

Functionally I'm not expecting problems to arise since there are sufficient AZs available for use even with said restrictions. I'm not aware of any other requirements beyond "AZ 1 must be different from AZ 2". Naming conventions for subnets would probably need to change though since those tend to be based off AZ names.

just noticed the naming is an issue on these - its using SM instead of nfw