From d2075273e2d51166ebc6d2b554362a6e704e313d Mon Sep 17 00:00:00 2001 From: Bill Beesley Date: Wed, 9 Aug 2023 13:02:24 +0100 Subject: [PATCH 1/2] feat: sort subnets to calculate by their netmask to efficiently use ip space The previous implementation calculates the ipv4 subnet cidr ranges in an arbitrary order (actually alphabetically based on the subnet type string). This means that if you have different netmasks for different subnets you end up trying to take your vpc cidr range, then cut out some small netmasks, then encounter a large netmask, at which point you have to skip a bunch of ips in order to get to the next start address for that larger netmask. In practice this causes really inefficient use of ip space. For example, with a vpc netmask of 22, 3 db subnets with a netmask of 27, 3 public subnets with a netmask of 28, 3 private subnets with a netmask of 24, and 3 transit gateway subnets with a netmask of 28, then the module is unable to calculate the cidr ranges, because first it creates the db subnets with their 27 netmask, then the public with their 28 netmask, then it gets to the private ones and has to skip a huge chunk to get to the start of a `/24` block. At this point it has skipped so many that its unable to create the remaining subnets. The fix for this is to first calculate the subnets where netmask is 24, then the ones where it is 27, then the ones where it is 28. If you do this, then the subnet calculator is then able to create all the required subnets, as its no longer skipping large chunks due to starting with small netmasks. BREAKING CHANGE: Since the subnets calculated after sorting wont be the same as those calculated without sorting, this change would cause a delete and recreate of existing subnets that were created with older versions. --- modules/calculate_subnets/main.tf | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/modules/calculate_subnets/main.tf b/modules/calculate_subnets/main.tf index 14d4604..f3c566e 100644 --- a/modules/calculate_subnets/main.tf +++ b/modules/calculate_subnets/main.tf @@ -16,6 +16,30 @@ locals { } ]]) + # map of subnet names to netmask values for looking up netmask by name + netmasks_for_subnets = { for subnet in local.calculated_subnet_objects : subnet.name => subnet.netmask } + + # sorted list of netmasks from largest to smallest so we can efficiently use the ip address space + sorted_subnet_netmasks = reverse(distinct(sort([ + for subnet in local.calculated_subnet_objects : format("%05d", subnet.netmask) + ]))) + + # list of subnet names sorted based on their netmask value (large to small) + sorted_subnet_names = compact(flatten([ + for netmask in local.sorted_subnet_netmasks : [ + for subnet in local.calculated_subnet_objects : + subnet.name if subnet.netmask == tonumber(netmask) + ] + ])) + + # list of subnet the original calculated subnet objects, but sorted based on their netmask value (large to small) + sorted_subnet_objects = [ + for name in local.sorted_subnet_names : { + name = name + netmask = local.netmasks_for_subnets[name] + } + ] + # map of explicit cidrs to az explicit_cidrs_grouped = { for _, type in local.types_with_explicit : type => zipmap(var.azs, var.subnets[type].cidrs[*]) } } @@ -27,6 +51,6 @@ module "subnet_calculator" { version = "1.0.2" base_cidr_block = var.cidr - networks = local.calculated_subnet_objects + networks = local.sorted_subnet_objects } From e214f024c048bce61b3792b18ce60050372053c0 Mon Sep 17 00:00:00 2001 From: Ivan Levchenko Date: Fri, 20 Dec 2024 15:29:01 +0100 Subject: [PATCH 2/2] adding feature flag and tests --- examples/optimized_ipv4_subnets/.header.md | 6 +++ .../.terraform-docs.yaml | 21 +++++++++ examples/optimized_ipv4_subnets/README.md | 46 +++++++++++++++++++ examples/optimized_ipv4_subnets/main.tf | 32 +++++++++++++ examples/optimized_ipv4_subnets/outputs.tf | 6 +++ examples/optimized_ipv4_subnets/providers.tf | 15 ++++++ examples/optimized_ipv4_subnets/variables.tf | 11 +++++ main.tf | 5 +- modules/calculate_subnets/main.tf | 3 +- modules/calculate_subnets/variables.tf | 6 +++ test/examples_optimized_ipv4_subnets_test.go | 21 +++++++++ variables.tf | 6 +++ 12 files changed, 174 insertions(+), 4 deletions(-) create mode 100644 examples/optimized_ipv4_subnets/.header.md create mode 100644 examples/optimized_ipv4_subnets/.terraform-docs.yaml create mode 100644 examples/optimized_ipv4_subnets/README.md create mode 100644 examples/optimized_ipv4_subnets/main.tf create mode 100644 examples/optimized_ipv4_subnets/outputs.tf create mode 100644 examples/optimized_ipv4_subnets/providers.tf create mode 100644 examples/optimized_ipv4_subnets/variables.tf create mode 100644 test/examples_optimized_ipv4_subnets_test.go diff --git a/examples/optimized_ipv4_subnets/.header.md b/examples/optimized_ipv4_subnets/.header.md new file mode 100644 index 0000000..6d38660 --- /dev/null +++ b/examples/optimized_ipv4_subnets/.header.md @@ -0,0 +1,6 @@ +# Creating subnets with sorted CIDR ranges + +This example shows how you can use this module to efficiently use the VPC CIDR range. Before calculating CIDR ranges, subnets are sorted from largest to smallest. + +* The VPC module creates the following: + * Four sets of subnets (*public*, *private*, *database*, and *infrastructure*) diff --git a/examples/optimized_ipv4_subnets/.terraform-docs.yaml b/examples/optimized_ipv4_subnets/.terraform-docs.yaml new file mode 100644 index 0000000..6dc99de --- /dev/null +++ b/examples/optimized_ipv4_subnets/.terraform-docs.yaml @@ -0,0 +1,21 @@ +formatter: markdown +header-from: .header.md +settings: + anchor: true + color: true + default: true + escape: true + html: true + indent: 2 + required: true + sensitive: true + type: true + lockfile: false + +sort: + enabled: true + by: required + +output: + file: README.md + mode: replace diff --git a/examples/optimized_ipv4_subnets/README.md b/examples/optimized_ipv4_subnets/README.md new file mode 100644 index 0000000..13c5458 --- /dev/null +++ b/examples/optimized_ipv4_subnets/README.md @@ -0,0 +1,46 @@ + +# Creating subnets with sorted CIDR ranges + +This example shows how you can use this module to efficiently use the VPC CIDR range. Before calculating CIDR ranges, subnets are sorted from largest to smallest. + +* The VPC module creates the following: + * Four sets of subnets (*public*, *private*, *database*, and *infrastructure*) + +## Requirements + +| Name | Version | +|------|---------| +| [terraform](#requirement\_terraform) | ~> 1.9 | +| [aws](#requirement\_aws) | ~> 5.0 | + +## Providers + +| Name | Version | +|------|---------| +| [aws](#provider\_aws) | ~> 5.0 | + +## Modules + +| Name | Source | Version | +|------|--------|---------| +| [vpc](#module\_vpc) | aws-ia/vpc/aws | ~> 4.4 | + +## Resources + +| Name | Type | +|------|------| +| [aws_availability_zones.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/availability_zones) | data source | + +## Inputs + +| Name | Description | Type | Default | Required | +|------|-------------|------|---------|:--------:| +| [aws\_region](#input\_aws\_region) | AWS Region to create resources in. | `string` | `"eu-west-2"` | no | +| [vpc\_cidr](#input\_vpc\_cidr) | VPC cidr range | `string` | `"10.0.0.0/22"` | no | + +## Outputs + +| Name | Description | +|------|-------------| +| [private\_subnets\_tags\_length](#output\_private\_subnets\_tags\_length) | Count of private subnet tags for a single az. | + \ No newline at end of file diff --git a/examples/optimized_ipv4_subnets/main.tf b/examples/optimized_ipv4_subnets/main.tf new file mode 100644 index 0000000..c68c3f4 --- /dev/null +++ b/examples/optimized_ipv4_subnets/main.tf @@ -0,0 +1,32 @@ +data "aws_availability_zones" "current" { + filter { + name = "opt-in-status" + values = ["opt-in-not-required"] + } +} + +module "vpc" { + source = "aws-ia/vpc/aws" + version = "~> 4.4" + + name = "sorted-subnets" + cidr_block = var.vpc_cidr + az_count = 3 + optimize_subnet_cidr_ranges = true + + subnets = { + private = { + netmask = 24 + } + database = { + netmask = 27 + } + public = { + netmask = 28 + nat_gateway_configuration = "single_az" + } + infrastructure = { + netmask = 28 + } + } +} diff --git a/examples/optimized_ipv4_subnets/outputs.tf b/examples/optimized_ipv4_subnets/outputs.tf new file mode 100644 index 0000000..6678e99 --- /dev/null +++ b/examples/optimized_ipv4_subnets/outputs.tf @@ -0,0 +1,6 @@ +## Used for Testing, do not delete + +output "private_subnets_tags_length" { + description = "Count of private subnet tags for a single az." + value = try(length(module.vpc.private_subnet_attributes_by_az["private/${data.aws_availability_zones.current.names[0]}"].tags), null) +} diff --git a/examples/optimized_ipv4_subnets/providers.tf b/examples/optimized_ipv4_subnets/providers.tf new file mode 100644 index 0000000..9bca1d9 --- /dev/null +++ b/examples/optimized_ipv4_subnets/providers.tf @@ -0,0 +1,15 @@ +terraform { + required_version = "~> 1.9" + required_providers { + aws = { + source = "hashicorp/aws" + version = "~> 5.0" + } + } +} + +# Provider definition +provider "aws" { + region = var.aws_region +} + diff --git a/examples/optimized_ipv4_subnets/variables.tf b/examples/optimized_ipv4_subnets/variables.tf new file mode 100644 index 0000000..4f792b6 --- /dev/null +++ b/examples/optimized_ipv4_subnets/variables.tf @@ -0,0 +1,11 @@ +variable "aws_region" { + description = "AWS Region to create resources in." + type = string + default = "eu-west-2" +} + +variable "vpc_cidr" { + type = string + description = "VPC cidr range" + default = "10.0.0.0/22" +} diff --git a/main.tf b/main.tf index 5bf91a7..528633a 100644 --- a/main.tf +++ b/main.tf @@ -7,7 +7,8 @@ module "calculate_subnets" { cidr = local.cidr_block azs = local.azs - subnets = var.subnets + subnets = var.subnets + optimize_subnet_cidr_ranges = var.optimize_subnet_cidr_ranges } module "calculate_subnets_ipv6" { @@ -521,4 +522,4 @@ resource "aws_vpclattice_service_network_vpc_association" "vpc_lattice_service_n module.tags.tags_aws, module.vpc_lattice_tags.tags_aws ) -} \ No newline at end of file +} diff --git a/modules/calculate_subnets/main.tf b/modules/calculate_subnets/main.tf index f3c566e..436173b 100644 --- a/modules/calculate_subnets/main.tf +++ b/modules/calculate_subnets/main.tf @@ -51,6 +51,5 @@ module "subnet_calculator" { version = "1.0.2" base_cidr_block = var.cidr - networks = local.sorted_subnet_objects + networks = var.optimize_subnet_cidr_ranges ? local.sorted_subnet_objects : local.calculated_subnet_objects } - diff --git a/modules/calculate_subnets/variables.tf b/modules/calculate_subnets/variables.tf index 9bba577..ddceae8 100644 --- a/modules/calculate_subnets/variables.tf +++ b/modules/calculate_subnets/variables.tf @@ -12,3 +12,9 @@ variable "cidr" { description = "CIDR value to use as base for calculating IP address prefixes." type = string } + +variable "optimize_subnet_cidr_ranges" { + description = "Sort subnets to calculate by their netmask to efficiently use IP space." + type = bool + default = false +} diff --git a/test/examples_optimized_ipv4_subnets_test.go b/test/examples_optimized_ipv4_subnets_test.go new file mode 100644 index 0000000..b39e368 --- /dev/null +++ b/test/examples_optimized_ipv4_subnets_test.go @@ -0,0 +1,21 @@ +package test + +import ( + "testing" + + "github.com/gruntwork-io/terratest/modules/terraform" + "github.com/likexian/gokit/assert" +) + +func TestExamplesOptimizedSubnets(t *testing.T) { + terraformOptions := &terraform.Options{ + TerraformDir: "../examples/optimized_ipv4_subnets", + } + + defer terraform.Destroy(t, terraformOptions) + terraform.InitAndApply(t, terraformOptions) + terraform.ApplyAndIdempotent(t, terraformOptions) + + privateTagsLength := terraform.Output(t, terraformOptions, "private_subnets_tags_length") + assert.Equal(t, "1", privateTagsLength) +} diff --git a/variables.tf b/variables.tf index 95c9936..ebe617a 100644 --- a/variables.tf +++ b/variables.tf @@ -250,6 +250,12 @@ EOF } } +variable "optimize_subnet_cidr_ranges" { + description = "Sort subnets to calculate by their netmask to efficiently use IP space." + type = bool + default = false +} + variable "tags" { description = "Tags to apply to all resources." type = map(string)