feat: Add custom subnet names

[andrewtcymmer]

Description

This PR adds the option to override the values used in resource tags with key "Name" (Name tags) with a list of strings, on public and private subnets. By default (if the optional variables are not specified, or if they are empty arrays), the module will generate the same Name tag values it did prior to this change.

Please reference issue #817 requesting this feature.

Motivation and Context

For VPCs which support multiple applications in multiple subnets, allowing the module to generate Name tags on subnets may cause multiple subnets to have the same value for their Name tag. Example VPC which generates such a condition, using two groups of private subnets:

module "vpc" {
  source = "terraform-aws-modules/vpc/aws"

  name = "problem-vpc"
  cidr = "10.0.0.0/16"

  azs = ["eu-west-1a", "eu-west-1b"]
  private_subnets = [
    # For an application named "Alpha"
    "10.0.1.0/24", "10.0.2.0/24",
    # For an application named "Beta"
    "10.0.3.0/24", "10.0.4.0/24"
  ]
  public_subnets  = ["10.0.101.0/24", "10.0.102.0/24"]
}

# Problem: Private subnets 10.0.1.0 and 10.0.3.0 both have the same value for Name, which can get confusing.
# 10.0.1.0 => "Name = problem-vpc-eu-west-1a"
# 10.0.3.0 => "Name = problem-vpc-eu-west-1a"
# 10.0.2.0 => "Name = problem-vpc-eu-west-1b"
# 10.0.4.0 => "Name = problem-vpc-eu-west-1b"
# Trying to add private_subnet_suffix does not help, because the same suffix is used for all names. Changing this would not be backward compatible.

It may be useful to specify the names of the subnets from the consumer side of the module, rather than let the module generate it. Proposed solution:

module "vpc" {
  source = "terraform-aws-modules/vpc/aws"

  name = "solution-vpc"
  cidr = "10.0.0.0/16"

  azs = ["eu-west-1a", "eu-west-1b"]
  private_subnets = [
    # For an application named "Alpha"
    "10.0.1.0/24", "10.0.2.0/24",
    # For an application named "Beta"
    "10.0.3.0/24", "10.0.4.0/24"
  ]
  private_subnet_names = [
    "Alpha Primary", "Alpha Backup",
    "Beta Primary", "Beta Backup"
  ]

  public_subnets  = ["10.0.101.0/24", "10.0.102.0/24"]
}

# Solution: Subnets 10.0.1.0 and 10.0.3.0 now have different names.
# 10.0.1.0 => "Name = Alpha Primary"
# 10.0.3.0 => "Name = Beta Primary"
# 10.0.2.0 => "Name = Alpha Backup"
# 10.0.4.0 => "Name = Beta Backup"

This solution also accommodates a case where a consumer would want to generate their own naming convention. As an example, consider the example below which adds in the availability zone to the names.

locals {
  private_app_names = ["Alpha private", "Beta private"]
  public_app_names = ["Alpha public", "Beta public"]
  azs  = ["eu-west-1a", "eu-west-1b"]

  private_subnet_names = split(";", trim(join("", flatten([for k, v in local.private_app_names : [for a, b in local.azs : "${v} ${b};"]])), ";"))
  public_subnet_names  = split(";", trim(join("", flatten([for k, v in local.public_app_names : [for a, b in local.azs : "${v} ${b};"]])), ";"))
}

module "vpc" {
  source = "terraform-aws-modules/vpc/aws"

  name = "solution-vpc"
  cidr = "10.0.0.0/16"

  azs = local.azs
  private_subnets = [
    # For an application named "Alpha"
    "10.0.1.0/24", "10.0.2.0/24",
    # For an application named "Beta"
    "10.0.3.0/24", "10.0.4.0/24"
  ]
  private_subnet_names = local.private_subnet_names

  public_subnets  = ["10.0.101.0/24", "10.0.102.0/24"]
  public_subnet_names = local.public_subnet_names
}

# Solution: Subnets 10.0.1.0 and 10.0.3.0 now have different names.
# 10.0.1.0 => "Name = Alpha private eu-west-1a"
# 10.0.3.0 => "Name = Beta private eu-west-1a"
# 10.0.2.0 => "Name = Alpha private eu-west-1b"
# 10.0.4.0 => "Name = Beta private eu-west-1b"

Breaking Changes

This change should be backwards compatible with the current major version, since the change adds an optional (opt-in) variable.

How Has This Been Tested?

• [x] I have updated at least one of the examples/* to demonstrate and validate my change(s)
• [x] ~I have tested and validated these changes using one or more of the provided examples/* projects~ I have run this change with a Terraform project in my organization. That project has the characteristics explained in the Motivation and Context section above (the example is the same concept, but reduced to simplest terms and scrubbed of company-specific information). PR includes updates to the examples/* which are exactly the same as I have tested on my project.

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

[NIXKnight]

Anyone still working on this PR?

[andrewtcymmer]

@NIXKnight Yes, I am still interested in bringing this PR out of draft status and getting it accepted. I plan to finish a minor detail and publish it today.

[rooty0]

I was about to make my PR to be able to provide custom tags per a subnet, tho just found your PR, and it actually looks fantastic! I like the tag name auto-generation compatibility part. I think forcefully inserting the availability zone at the end of a custom name is redundant. Why would you do it if you want to manually control the names of your subnets?

[andrewtcymmer]

@rooty0 Thanks for the support on this PR. Your question about forcing the availability zone has me thinking again, because my first pass at this did not include the AZ in the name for simplicity. Your question also made me realize that, as implemented at the current moment in this PR, this feature should have also considered the private_subnet_suffix and public_subnet_suffix in its convention to be complete. (An example name would be mysuper-custom-name-public-us-west-2a). With that addition, this feature would be getting closer to changing the way names are generated, rather than being custom. Which would not be a bad thing IMO, but it would be redundant and confusing to have two competing naming strategies in the same module. Changing the default naming strategy would be breaking change as well, which I want to avoid. So now I have arrived back at what you are suggesting, which is that a custom name which can be anything is actually a much simpler approach which can include anything it wants, and in any order.

I think it would be good for me to change this as you suggest, but also include an example Terraform snippet that could generate a naming convention based on a map(object) type, if desired. Let me try this out later today, and I will update the PR.

[andrewtcymmer]

I updated the PR description to reflect the change (which I rewrote over the old commit to try and keep the review neat and readable). The snippet I came up with to generate names is a bit messy. Any ways to neaten it up would be welcome. Put this into a new file in its own directory and run terraform plan to try it out.

locals {
  app_names = ["Alpha private", "Beta private"]
  azs  = ["eu-west-1a", "eu-west-1b"]
}

output "example" {
  value = split(";", trim(join("", flatten([for k, v in local.app_names : [for a, b in local.azs : "${v} ${b};"]])), ";"))
}

[dfradejas]

Hello there!

When is going to be merged this PR? I am really interested!

Thanks.

[tcharewicz]

I added example PR with code how external subnets can be created in VPC.

https://github.com/terraform-aws-modules/terraform-aws-vpc/pull/834

[rooty0]

@antonbabenko any chance you can review/merge this please?

[lordz-md]

This would be also usefull when you have private subnets and want to have separate EKS subnets.

[tcharewicz]

This would be also usefull when you have private subnets and want to have separate EKS subnets.

Yes I create my code to create additional subnets, when I need to create dedicated EKS subnets.

[antonbabenko]

@lordz-md Here we go!

Thank you, @andrewtcymmer , for the PR!

[antonbabenko]

This PR is included in version 3.17.0 :tada:

[lordz-md]

This would be also usefull when you have private subnets and want to have separate EKS subnets.

Yes I create my code to create additional subnets, when I need to create dedicated EKS subnets.

I recently opened a PR to have separated EKS subnets also.

[balq60]

This feature is not working.

Here is my network.tf file.

`provider "aws" { region = local.region }

locals { name = "ex-${replace(basename(path.cwd), "_", "-")}" region = "us-east-1"

tags = { Name = local.name Environment = "dev" Customer = "Blue Origin" } }

################################################################################

VPC Module -

################################################################################ module "vpc" { source = "terraform-aws-modules/vpc/aws" version = "3.18.1"

name = "blue-vpn-vpc" cidr = "10.0.0.0/16"

azs = ["${local.region}a", "${local.region}b", "${local.region}c"] private_subnets = ["10.0.1.0/24", "10.0.2.0/24", "10.0.3.0/24"] public_subnets = [] database_subnets = [] elasticache_subnets = [] redshift_subnets = [] intra_subnets = []

private_subnet_names = ["blue-subnet-us-gov-west-1a", "blue-subnet-us-gov-west-1b", "blue-subnet-us-gov-west-1c"] public_subnet_names = [] database_subnet_names = [] elasticache_subnet_names = [] redshift_subnet_names = [] intra_subnet_names = []

VPC Flow Logs (Cloudwatch log group and IAM role will be created)

enable_flow_log = true create_flow_log_cloudwatch_log_group = true create_flow_log_cloudwatch_iam_role = true flow_log_max_aggregation_interval = 60

create_vpc = true

create_database_internet_gateway_route = false create_database_nat_gateway_route = false create_database_subnet_group = false create_database_subnet_route_table = false create_egress_only_igw = true create_elasticache_subnet_group = false create_elasticache_subnet_route_table = false

create_igw = true

create_redshift_subnet_group = false create_redshift_subnet_route_table = false

manage_default_network_acl = true default_network_acl_tags = { Name = "${local.name}-default" }

manage_default_route_table = true default_route_table_tags = { Name = "${local.name}-default" }

manage_default_security_group = true default_security_group_tags = { Name = "${local.name}-default" }

enable_dns_hostnames = true enable_dns_support = true

enable_nat_gateway = false single_nat_gateway = false

customer_gateways = {

IP1 = {

bgp_asn = 65112

ip_address = "1.2.3.4"

device_name = "some_name"

},

IP2 = {

bgp_asn = 65112

ip_address = "5.6.7.8"

}

}

enable_vpn_gateway = false

enable_dhcp_options = true dhcp_options_domain_name = "service.consul" dhcp_options_domain_name_servers = ["127.0.0.1", "10.10.0.2"]

tags = local.tags } `

This results in subnets all created with the name name from locals.name

You can see the local.name repeated for the Name of the Subnets.

Name | Subnet ID | State | VPC | IPv4 CIDR ex-IaC | subnet-097d4221401bc0671 | Available | vpc-00abe999801f08aae | ex-IaC | 10.0.1.0/24 ex-IaC | subnet-05d94c33d42d1ae4c | Available | vpc-00abe999801f08aae | ex-IaC | 10.0.2.0/24 ex-IaC | subnet-00ea9d53c896eb254 | Available | vpc-00abe999801f08aae | ex-IaC | 10.0.3.0/24

[dduzgun-security]

Hey, this didn't work on my side too. Subnet names are not changed.

module "vpc" {
  source  = "terraform-aws-modules/vpc/aws"
  version = "3.18.1"
  name    = "${var.prefix}-vpc-default"
  cidr    = var.cidr

  azs             = var.azs
  private_subnets = local.private_subnets
  public_subnets  = local.public_subnets

  private_subnet_names = local.private_subnet_names
  public_subnet_names  = local.public_subnet_names
....

[rooty0]

@balq60 @dduzgun-security I just want to confirm it works like a charm for me. I'd recommend opening an Issue with detailed information rather than posting to this PR.

[balq60]

The problem for me was no support for Terraform files.

So if your file type is not supported it won’t work. Even if they are in the root folder

Bernie

On Wed, Nov 23, 2022 at 12:32 PM Deniz Onur Duzgun @.***> wrote:

Hey, this didn't work on my side too. Subnet names are not changed.

module "vpc" { source = "terraform-aws-modules/vpc/aws" version = "3.18.1" name = "${var.prefix}-vpc-default" cidr = var.cidr

azs = var.azs private_subnets = local.private_subnets public_subnets = local.public_subnets

private_subnet_names = local.private_subnet_names public_subnet_names = local.public_subnet_names....

— Reply to this email directly, view it on GitHub https://github.com/terraform-aws-modules/terraform-aws-vpc/pull/816#issuecomment-1325497743, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGNQUZSZLV547LXD6K6SJ3TWJZPL5ANCNFSM5354RR7Q . You are receiving this because you commented.Message ID: @.***>

[bryantbiggs]

The problem for me was no support for Terraform files.

So if your file type is not supported it won’t work. Even if they are in the root folder

What?!

[balq60]

Terraform files end in .tf

If you look at the code, there is no filter for .tf files.

So they get ignored.

-Bernie

On Wed, Nov 23, 2022 at 4:50 PM Bryant Biggs @.***> wrote:

The problem for me was no support for Terraform files.

So if your file type is not supported it won’t work. Even if they are in the root folder

What?!

— Reply to this email directly, view it on GitHub https://github.com/terraform-aws-modules/terraform-aws-vpc/pull/816#issuecomment-1325741685, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGNQUZWCZZSA4UOWUVAJN5LWJ2NSRANCNFSM5354RR7Q . You are receiving this because you were mentioned.Message ID: @.***>

[cooka]

@balq60 @dduzgun-security @rooty0 @andrewtcymmer it seems, if tags_all has 'Name' tag, the custom xxx_subnet_names would not work.

[github-actions[bot]]

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.