From ceb58d9495d8c47c918e684045c8fa87f190570b Mon Sep 17 00:00:00 2001
From: John Cowie Del Corral <john@go-atomic.io>
Date: Fri, 10 Oct 2025 11:07:19 +0200
Subject: [PATCH 1/7] Revert "Merge pull request #144 from
 infrablocks/revert-support-az-updates"

This reverts commit 12e1b050360f9830dbd26c4dcdce7d7fe4759d11, reversing
changes made to 0e05f060a9c949db1de90a3a8f9fce7ea4593d03.
---
 nat.tf                                        |  12 +-
 outputs.tf                                    |  14 +-
 private_subnets.tf                            |  28 ++--
 public_subnets.tf                             |  26 +--
 .../availability_zone_addition_spec.rb        | 150 ++++++++++++++++++
 5 files changed, 194 insertions(+), 36 deletions(-)
 create mode 100644 spec/integration/availability_zone_addition_spec.rb

diff --git a/nat.tf b/nat.tf
index 318b8df..9554eaf 100644
--- a/nat.tf
+++ b/nat.tf
@@ -1,27 +1,27 @@
 resource "aws_eip" "nat" {
-  count = local.include_nat_gateways == "yes" ? length(var.availability_zones) : 0
+  for_each = local.include_nat_gateways == "yes" ? toset(var.availability_zones) : toset([])
 
   vpc = true
 
   tags = {
-    Name = "eip-nat-${var.component}-${var.deployment_identifier}-${element(var.availability_zones, count.index)}"
+    Name = "eip-nat-${var.component}-${var.deployment_identifier}-${each.value}"
     Component = var.component
     DeploymentIdentifier = var.deployment_identifier
   }
 }
 
 resource "aws_nat_gateway" "base" {
-  count = local.include_nat_gateways == "yes" ? length(var.availability_zones) : 0
+  for_each = local.include_nat_gateways == "yes" ? toset(var.availability_zones) : toset([])
 
-  allocation_id = element(aws_eip.nat.*.id, count.index)
-  subnet_id = element(aws_subnet.public.*.id, count.index)
+  allocation_id = aws_eip.nat[each.value].id
+  subnet_id = aws_subnet.public[each.value].id
 
   depends_on = [
     aws_internet_gateway.base_igw
   ]
 
   tags = {
-    Name = "nat-${var.component}-${var.deployment_identifier}-${element(var.availability_zones, count.index)}"
+    Name = "nat-${var.component}-${var.deployment_identifier}-${each.value}"
     Component = var.component
     DeploymentIdentifier = var.deployment_identifier
   }
diff --git a/outputs.tf b/outputs.tf
index 191f666..0facc49 100644
--- a/outputs.tf
+++ b/outputs.tf
@@ -20,37 +20,37 @@ output "number_of_availability_zones" {
 
 output "public_subnet_ids" {
   description = "The IDs of the public subnets."
-  value = aws_subnet.public.*.id
+  value = [for az in var.availability_zones : aws_subnet.public[az].id]
 }
 
 output "public_subnet_cidr_blocks" {
   description = "The CIDRs of the public subnets."
-  value = aws_subnet.public.*.cidr_block
+  value = [for az in var.availability_zones : aws_subnet.public[az].cidr_block]
 }
 
 output "public_route_table_ids" {
   description = "The IDs of the public route tables."
-  value = aws_route_table.public.*.id
+  value = [for az in var.availability_zones : aws_route_table.public[az].id]
 }
 
 output "private_subnet_ids" {
   description = "The IDs of the private subnets."
-  value = aws_subnet.private.*.id
+  value = [for az in var.availability_zones : aws_subnet.private[az].id]
 }
 
 output "private_subnet_cidr_blocks" {
   description = "The CIDRs of the private subnets."
-  value = aws_subnet.private.*.cidr_block
+  value = [for az in var.availability_zones : aws_subnet.private[az].cidr_block]
 }
 
 output "private_route_table_ids" {
   description = "The IDs of the private route tables."
-  value = aws_route_table.private.*.id
+  value = [for az in var.availability_zones : aws_route_table.private[az].id]
 }
 
 output "nat_public_ips" {
   description = "The EIPs attached to the NAT gateways."
-  value = aws_eip.nat.*.public_ip
+  value = local.include_nat_gateways == "yes" ? [for az in var.availability_zones : aws_eip.nat[az].public_ip] : []
 }
 
 output "internet_gateway_id" {
diff --git a/private_subnets.tf b/private_subnets.tf
index 6156a97..b850dca 100644
--- a/private_subnets.tf
+++ b/private_subnets.tf
@@ -1,11 +1,12 @@
 resource "aws_subnet" "private" {
+  for_each = toset(var.availability_zones)
+
   vpc_id = aws_vpc.base.id
-  count = length(var.availability_zones)
-  cidr_block = cidrsubnet(var.vpc_cidr, 8, count.index + length(var.availability_zones) + local.private_subnets_offset)
-  availability_zone = element(var.availability_zones, count.index)
+  cidr_block = cidrsubnet(var.vpc_cidr, 8, index(var.availability_zones, each.value) + length(var.availability_zones) + local.private_subnets_offset)
+  availability_zone = each.value
 
   tags = {
-    Name = "private-subnet-${var.component}-${var.deployment_identifier}-${element(var.availability_zones, count.index)}"
+    Name = "private-subnet-${var.component}-${var.deployment_identifier}-${each.value}"
     Component = var.component
     DeploymentIdentifier = var.deployment_identifier
     Tier = "private"
@@ -13,11 +14,12 @@ resource "aws_subnet" "private" {
 }
 
 resource "aws_route_table" "private" {
+  for_each = toset(var.availability_zones)
+
   vpc_id = aws_vpc.base.id
-  count = length(var.availability_zones)
 
   tags = {
-    Name = "private-routetable-${var.component}-${var.deployment_identifier}-${element(var.availability_zones, count.index)}"
+    Name = "private-routetable-${var.component}-${var.deployment_identifier}-${each.value}"
     Component = var.component
     DeploymentIdentifier = var.deployment_identifier
     Tier = "private"
@@ -25,14 +27,16 @@ resource "aws_route_table" "private" {
 }
 
 resource "aws_route" "private_internet" {
-  count = local.include_nat_gateways == "yes" ? length(var.availability_zones) : 0
-  route_table_id = element(aws_route_table.private.*.id, count.index)
-  nat_gateway_id = element(aws_nat_gateway.base.*.id, count.index)
+  for_each = local.include_nat_gateways == "yes" ? toset(var.availability_zones) : toset([])
+
+  route_table_id = aws_route_table.private[each.value].id
+  nat_gateway_id = aws_nat_gateway.base[each.value].id
   destination_cidr_block = "0.0.0.0/0"
 }
 
 resource "aws_route_table_association" "private" {
-  count = length(var.availability_zones)
-  subnet_id = element(aws_subnet.private.*.id, count.index)
-  route_table_id = element(aws_route_table.private.*.id, count.index)
+  for_each = toset(var.availability_zones)
+
+  subnet_id = aws_subnet.private[each.value].id
+  route_table_id = aws_route_table.private[each.value].id
 }
diff --git a/public_subnets.tf b/public_subnets.tf
index e989be0..af68e00 100644
--- a/public_subnets.tf
+++ b/public_subnets.tf
@@ -1,11 +1,12 @@
 resource "aws_subnet" "public" {
+  for_each = toset(var.availability_zones)
+
   vpc_id = aws_vpc.base.id
-  count = length(var.availability_zones)
-  cidr_block = cidrsubnet(var.vpc_cidr, 8, count.index + local.public_subnets_offset)
-  availability_zone = element(var.availability_zones, count.index)
+  cidr_block = cidrsubnet(var.vpc_cidr, 8, index(var.availability_zones, each.value) + local.public_subnets_offset)
+  availability_zone = each.value
 
   tags = {
-    Name = "public-subnet-${var.component}-${var.deployment_identifier}-${element(var.availability_zones, count.index)}"
+    Name = "public-subnet-${var.component}-${var.deployment_identifier}-${each.value}"
     Component = var.component
     DeploymentIdentifier = var.deployment_identifier
     Tier = "public"
@@ -13,11 +14,12 @@ resource "aws_subnet" "public" {
 }
 
 resource "aws_route_table" "public" {
+  for_each = toset(var.availability_zones)
+
   vpc_id = aws_vpc.base.id
-  count = length(var.availability_zones)
 
   tags = {
-    Name = "public-routetable-${var.component}-${var.deployment_identifier}-${element(var.availability_zones, count.index)}"
+    Name = "public-routetable-${var.component}-${var.deployment_identifier}-${each.value}"
     Component = var.component
     DeploymentIdentifier = var.deployment_identifier
     Tier = "public"
@@ -25,14 +27,16 @@ resource "aws_route_table" "public" {
 }
 
 resource "aws_route" "public_internet" {
-  count = length(var.availability_zones)
-  route_table_id = element(aws_route_table.public.*.id, count.index)
+  for_each = toset(var.availability_zones)
+
+  route_table_id = aws_route_table.public[each.value].id
   gateway_id = aws_internet_gateway.base_igw.id
   destination_cidr_block = "0.0.0.0/0"
 }
 
 resource "aws_route_table_association" "public" {
-  count = length(var.availability_zones)
-  subnet_id = element(aws_subnet.public.*.id, count.index)
-  route_table_id = element(aws_route_table.public.*.id, count.index)
+  for_each = toset(var.availability_zones)
+
+  subnet_id = aws_subnet.public[each.value].id
+  route_table_id = aws_route_table.public[each.value].id
 }
diff --git a/spec/integration/availability_zone_addition_spec.rb b/spec/integration/availability_zone_addition_spec.rb
new file mode 100644
index 0000000..af7b598
--- /dev/null
+++ b/spec/integration/availability_zone_addition_spec.rb
@@ -0,0 +1,150 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'json'
+
+describe 'availability zone addition' do
+  let(:initial_availability_zones) do
+    %w[eu-west-1a eu-west-1b]
+  end
+
+  let(:updated_availability_zones) do
+    %w[eu-west-1a eu-west-1b eu-west-1c]
+  end
+
+  let(:component) { 'test-component' }
+  let(:deployment_identifier) { 'test-deployment' }
+  let(:vpc_cidr) { '10.0.0.0/16' }
+  let(:region) { 'eu-west-1' }
+
+  describe 'adding a new availability zone' do
+    it 'does not destroy existing subnets when adding a new availability zone' do
+      # Step 1: Apply with initial set of availability zones
+      initial_state = apply_and_get_state(initial_availability_zones)
+
+      # Get the initial subnet IDs
+      initial_public_subnet_ids = get_resource_ids(initial_state, 'aws_subnet', 'public')
+      initial_private_subnet_ids = get_resource_ids(initial_state, 'aws_subnet', 'private')
+      initial_nat_gateway_ids = get_resource_ids(initial_state, 'aws_nat_gateway', 'base')
+      initial_eip_ids = get_resource_ids(initial_state, 'aws_eip', 'nat')
+
+      # Step 2: Plan with additional availability zone
+      plan_output = plan_with_azs(updated_availability_zones)
+
+      # Parse the plan output to check for destructions
+      plan_json = JSON.parse(plan_output)
+
+      # Check that no existing resources are being destroyed
+      resource_changes = plan_json['resource_changes'] || []
+
+      # Find any destroy actions for our existing resources
+      destroyed_resources = resource_changes.select do |change|
+        change['change']['actions'].include?('delete') &&
+          (initial_public_subnet_ids.values.include?(change['change']['before']['id']) ||
+           initial_private_subnet_ids.values.include?(change['change']['before']['id']) ||
+           initial_nat_gateway_ids.values.include?(change['change']['before']['id']) ||
+           initial_eip_ids.values.include?(change['change']['before']['id']))
+      end
+
+      # Assert no existing resources are being destroyed
+      expect(destroyed_resources).to be_empty,
+        "Expected no resources to be destroyed, but found: #{destroyed_resources.map { |r| "#{r['type']}.#{r['name']}" }.join(', ')}"
+
+      # Check that only new resources are being created
+      created_resources = resource_changes.select do |change|
+        change['change']['actions'].include?('create') &&
+          %w[aws_subnet aws_route_table aws_route_table_association aws_nat_gateway aws_eip].include?(change['type'])
+      end
+
+      # We expect exactly 1 new public subnet, 1 new private subnet,
+      # 2 new route tables, 2 new route table associations,
+      # 1 new NAT gateway, and 1 new EIP for the new AZ
+      expected_new_resources = {
+        'aws_subnet' => 2, # 1 public + 1 private
+        'aws_route_table' => 2, # 1 for public + 1 for private
+        'aws_route_table_association' => 2, # 1 for public + 1 for private
+        'aws_route' => 2, # 1 for public internet route + 1 for private NAT route
+        'aws_nat_gateway' => 1,
+        'aws_eip' => 1
+      }
+
+      actual_new_resources = created_resources.group_by { |r| r['type'] }
+        .transform_values(&:count)
+
+      expected_new_resources.each do |resource_type, expected_count|
+        actual_count = actual_new_resources[resource_type] || 0
+        expect(actual_count).to eq(expected_count),
+          "Expected #{expected_count} new #{resource_type} resources, but found #{actual_count}"
+      end
+    end
+  end
+
+  private
+
+  def apply_and_get_state(availability_zones)
+    # Create a temporary directory for this test run
+    test_dir = "spec/integration/test_runs/#{Time.now.to_i}"
+    FileUtils.mkdir_p(test_dir)
+
+    # Write the terraform configuration
+    File.write("#{test_dir}/main.tf", generate_terraform_config(availability_zones))
+
+    # Initialize and apply
+    Dir.chdir(test_dir) do
+      system('terraform init', out: File::NULL, err: File::NULL)
+      system('terraform apply -auto-approve', out: File::NULL, err: File::NULL)
+
+      # Get the state
+      state_output = `terraform show -json`
+      JSON.parse(state_output)
+    end
+  ensure
+    # Cleanup is handled by the test framework
+  end
+
+  def plan_with_azs(availability_zones)
+    # Update the configuration with new AZs
+    test_dir = Dir.glob('spec/integration/test_runs/*').last
+    File.write("#{test_dir}/main.tf", generate_terraform_config(availability_zones))
+
+    # Run plan and capture output
+    Dir.chdir(test_dir) do
+      `terraform plan -out=tfplan -json`
+      `terraform show -json tfplan`
+    end
+  end
+
+  def generate_terraform_config(availability_zones)
+    <<~HCL
+      module "base_networking" {
+        source = "../../../"
+
+        vpc_cidr              = "#{vpc_cidr}"
+        region                = "#{region}"
+        availability_zones    = #{availability_zones.inspect}
+        component             = "#{component}"
+        deployment_identifier = "#{deployment_identifier}"
+      }
+
+      provider "aws" {
+        region = "#{region}"
+      }
+    HCL
+  end
+
+  def get_resource_ids(state, resource_type, resource_name)
+    resources = state['values']['root_module']['child_modules']
+      &.first['resources'] || []
+
+    resources.select do |r|
+      r['type'] == resource_type && r['name'] == resource_name
+    end.map do |r|
+      # For for_each resources, use the index key (AZ name) as the key
+      if r['index'].is_a?(String)
+        [r['index'], r['values']['id']]
+      else
+        [r['index'].to_s, r['values']['id']]
+      end
+    end.to_h
+  end
+end
\ No newline at end of file

From 4120b23ed8d1e897a5c232e60e774f69d64d0aab Mon Sep 17 00:00:00 2001
From: John Cowie Del Corral <john@go-atomic.io>
Date: Fri, 10 Oct 2025 11:29:28 +0200
Subject: [PATCH 2/7] Experiment with passing cidr blocks explicitly

---
 .gitignore                                    |  3 +
 defaults.tf                                   | 23 ++++++
 examples/full/base_networking.tf              |  4 +-
 examples/full/prerequisites.tf                |  8 +-
 examples/full/terraform.tf                    |  2 +-
 igw.tf                                        |  6 +-
 nat.tf                                        | 16 ++--
 outputs.tf                                    | 24 +++---
 private_subnets.tf                            | 34 ++++----
 public_subnets.tf                             | 34 ++++----
 .../availability_zone_addition_spec.rb        | 78 +++++++++++++------
 spec/unit/infra/prerequisites/main.tf         |  8 +-
 spec/unit/infra/prerequisites/provider.tf     |  2 +-
 spec/unit/infra/prerequisites/terraform.tf    |  2 +-
 spec/unit/infra/root/main.tf                  | 12 +--
 spec/unit/infra/root/terraform.tf             |  2 +-
 spec/unit/infra/root/variables.tf             |  6 +-
 variables.tf                                  | 55 ++++++++-----
 vpc.tf                                        | 12 +--
 19 files changed, 204 insertions(+), 127 deletions(-)

diff --git a/.gitignore b/.gitignore
index 1a27768..863ca63 100644
--- a/.gitignore
+++ b/.gitignore
@@ -34,3 +34,6 @@ state/
 *.tfplan
 *.tfstate
 *.tfstate.backup
+
+# Test run files
+spec/integration/test_runs/**/*
diff --git a/defaults.tf b/defaults.tf
index e25c0bb..0fd4546 100644
--- a/defaults.tf
+++ b/defaults.tf
@@ -5,4 +5,27 @@ locals {
   private_subnets_offset           = var.private_subnets_offset == null ? 0 : var.private_subnets_offset
   include_route53_zone_association = var.include_route53_zone_association == null ? "yes" : var.include_route53_zone_association
   include_nat_gateways             = var.include_nat_gateways == null ? "yes" : var.include_nat_gateways
+
+  # Validation: ensure only one of availability_zones or availability_zone_configurations is provided
+  _validate_az_input = (
+    (var.availability_zones == null && var.availability_zone_configurations == null) ||
+    (var.availability_zones != null && var.availability_zone_configurations != null)
+  ) ? tobool("ERROR: Exactly one of availability_zones or availability_zone_configurations must be specified.") : true
+
+  # Normalize AZ configuration to a consistent format
+  # If availability_zone_configurations is provided, use it directly
+  # Otherwise, generate configurations from availability_zones list
+  az_configurations = var.availability_zone_configurations != null ? var.availability_zone_configurations : [
+    for idx, az in var.availability_zones : {
+      zone                = az
+      public_subnet_cidr  = cidrsubnet(var.vpc_cidr, 8, idx + local.public_subnets_offset)
+      private_subnet_cidr = cidrsubnet(var.vpc_cidr, 8, idx + 128 + local.private_subnets_offset)
+    }
+  ]
+
+  # Create a map keyed by AZ name for easy lookup in resources
+  az_map = { for config in local.az_configurations : config.zone => config }
+
+  # List of AZ names for outputs and other references
+  availability_zones = [for config in local.az_configurations : config.zone]
 }
diff --git a/examples/full/base_networking.tf b/examples/full/base_networking.tf
index af4921a..e9dfc1a 100644
--- a/examples/full/base_networking.tf
+++ b/examples/full/base_networking.tf
@@ -4,9 +4,9 @@ module "base_networking" {
   component             = var.component
   deployment_identifier = var.deployment_identifier
 
-  region = var.region
+  region             = var.region
   availability_zones = var.availability_zones
-  vpc_cidr = var.vpc_cidr
+  vpc_cidr           = var.vpc_cidr
 
   dependencies = ["other_vpc_1", "other_vpc_2"]
 
diff --git a/examples/full/prerequisites.tf b/examples/full/prerequisites.tf
index 2358614..0acf268 100644
--- a/examples/full/prerequisites.tf
+++ b/examples/full/prerequisites.tf
@@ -1,11 +1,11 @@
 resource "aws_default_vpc" "default" {}
 
 module "dns_zones" {
-  source = "infrablocks/dns-zones/aws"
+  source  = "infrablocks/dns-zones/aws"
   version = "2.0.0"
 
-  domain_name = var.domain_name
-  private_domain_name = var.domain_name
-  private_zone_vpc_id = aws_default_vpc.default.id
+  domain_name             = var.domain_name
+  private_domain_name     = var.domain_name
+  private_zone_vpc_id     = aws_default_vpc.default.id
   private_zone_vpc_region = var.region
 }
diff --git a/examples/full/terraform.tf b/examples/full/terraform.tf
index 9631039..98aa83e 100644
--- a/examples/full/terraform.tf
+++ b/examples/full/terraform.tf
@@ -3,7 +3,7 @@ terraform {
 
   required_providers {
     aws = {
-      source = "hashicorp/aws"
+      source  = "hashicorp/aws"
       version = "4.33"
     }
   }
diff --git a/igw.tf b/igw.tf
index 4bb0f9b..bcbb896 100644
--- a/igw.tf
+++ b/igw.tf
@@ -2,9 +2,9 @@ resource "aws_internet_gateway" "base_igw" {
   vpc_id = aws_vpc.base.id
 
   tags = {
-    Name = "igw-${var.component}-${var.deployment_identifier}"
-    Component = var.component
+    Name                 = "igw-${var.component}-${var.deployment_identifier}"
+    Component            = var.component
     DeploymentIdentifier = var.deployment_identifier
-    Tier = "public"
+    Tier                 = "public"
   }
 }
diff --git a/nat.tf b/nat.tf
index 9554eaf..65048cd 100644
--- a/nat.tf
+++ b/nat.tf
@@ -1,28 +1,28 @@
 resource "aws_eip" "nat" {
-  for_each = local.include_nat_gateways == "yes" ? toset(var.availability_zones) : toset([])
+  for_each = local.include_nat_gateways == "yes" ? local.az_map : {}
 
   vpc = true
 
   tags = {
-    Name = "eip-nat-${var.component}-${var.deployment_identifier}-${each.value}"
-    Component = var.component
+    Name                 = "eip-nat-${var.component}-${var.deployment_identifier}-${each.value.zone}"
+    Component            = var.component
     DeploymentIdentifier = var.deployment_identifier
   }
 }
 
 resource "aws_nat_gateway" "base" {
-  for_each = local.include_nat_gateways == "yes" ? toset(var.availability_zones) : toset([])
+  for_each = local.include_nat_gateways == "yes" ? local.az_map : {}
 
-  allocation_id = aws_eip.nat[each.value].id
-  subnet_id = aws_subnet.public[each.value].id
+  allocation_id = aws_eip.nat[each.key].id
+  subnet_id     = aws_subnet.public[each.key].id
 
   depends_on = [
     aws_internet_gateway.base_igw
   ]
 
   tags = {
-    Name = "nat-${var.component}-${var.deployment_identifier}-${each.value}"
-    Component = var.component
+    Name                 = "nat-${var.component}-${var.deployment_identifier}-${each.value.zone}"
+    Component            = var.component
     DeploymentIdentifier = var.deployment_identifier
   }
 }
diff --git a/outputs.tf b/outputs.tf
index 0facc49..5ee445f 100644
--- a/outputs.tf
+++ b/outputs.tf
@@ -1,59 +1,59 @@
 output "vpc_id" {
   description = "The ID of the created VPC."
-  value = aws_vpc.base.id
+  value       = aws_vpc.base.id
 }
 
 output "vpc_cidr" {
   description = "The CIDR of the created VPC."
-  value = aws_vpc.base.cidr_block
+  value       = aws_vpc.base.cidr_block
 }
 
 output "availability_zones" {
   description = "The availability zones in which subnets were created."
-  value = var.availability_zones
+  value       = local.availability_zones
 }
 
 output "number_of_availability_zones" {
   description = "The number of populated availability zones available."
-  value = length(var.availability_zones)
+  value       = length(local.availability_zones)
 }
 
 output "public_subnet_ids" {
   description = "The IDs of the public subnets."
-  value = [for az in var.availability_zones : aws_subnet.public[az].id]
+  value       = [for az in local.availability_zones : aws_subnet.public[az].id]
 }
 
 output "public_subnet_cidr_blocks" {
   description = "The CIDRs of the public subnets."
-  value = [for az in var.availability_zones : aws_subnet.public[az].cidr_block]
+  value       = [for az in local.availability_zones : aws_subnet.public[az].cidr_block]
 }
 
 output "public_route_table_ids" {
   description = "The IDs of the public route tables."
-  value = [for az in var.availability_zones : aws_route_table.public[az].id]
+  value       = [for az in local.availability_zones : aws_route_table.public[az].id]
 }
 
 output "private_subnet_ids" {
   description = "The IDs of the private subnets."
-  value = [for az in var.availability_zones : aws_subnet.private[az].id]
+  value       = [for az in local.availability_zones : aws_subnet.private[az].id]
 }
 
 output "private_subnet_cidr_blocks" {
   description = "The CIDRs of the private subnets."
-  value = [for az in var.availability_zones : aws_subnet.private[az].cidr_block]
+  value       = [for az in local.availability_zones : aws_subnet.private[az].cidr_block]
 }
 
 output "private_route_table_ids" {
   description = "The IDs of the private route tables."
-  value = [for az in var.availability_zones : aws_route_table.private[az].id]
+  value       = [for az in local.availability_zones : aws_route_table.private[az].id]
 }
 
 output "nat_public_ips" {
   description = "The EIPs attached to the NAT gateways."
-  value = local.include_nat_gateways == "yes" ? [for az in var.availability_zones : aws_eip.nat[az].public_ip] : []
+  value       = local.include_nat_gateways == "yes" ? [for az in local.availability_zones : aws_eip.nat[az].public_ip] : []
 }
 
 output "internet_gateway_id" {
   description = "The ID of IGW attached to the VPC."
-  value = aws_internet_gateway.base_igw.id
+  value       = aws_internet_gateway.base_igw.id
 }
diff --git a/private_subnets.tf b/private_subnets.tf
index b850dca..9ce9f5c 100644
--- a/private_subnets.tf
+++ b/private_subnets.tf
@@ -1,42 +1,42 @@
 resource "aws_subnet" "private" {
-  for_each = toset(var.availability_zones)
+  for_each = local.az_map
 
-  vpc_id = aws_vpc.base.id
-  cidr_block = cidrsubnet(var.vpc_cidr, 8, index(var.availability_zones, each.value) + length(var.availability_zones) + local.private_subnets_offset)
-  availability_zone = each.value
+  vpc_id            = aws_vpc.base.id
+  cidr_block        = each.value.private_subnet_cidr
+  availability_zone = each.value.zone
 
   tags = {
-    Name = "private-subnet-${var.component}-${var.deployment_identifier}-${each.value}"
-    Component = var.component
+    Name                 = "private-subnet-${var.component}-${var.deployment_identifier}-${each.value.zone}"
+    Component            = var.component
     DeploymentIdentifier = var.deployment_identifier
-    Tier = "private"
+    Tier                 = "private"
   }
 }
 
 resource "aws_route_table" "private" {
-  for_each = toset(var.availability_zones)
+  for_each = local.az_map
 
   vpc_id = aws_vpc.base.id
 
   tags = {
-    Name = "private-routetable-${var.component}-${var.deployment_identifier}-${each.value}"
-    Component = var.component
+    Name                 = "private-routetable-${var.component}-${var.deployment_identifier}-${each.value.zone}"
+    Component            = var.component
     DeploymentIdentifier = var.deployment_identifier
-    Tier = "private"
+    Tier                 = "private"
   }
 }
 
 resource "aws_route" "private_internet" {
-  for_each = local.include_nat_gateways == "yes" ? toset(var.availability_zones) : toset([])
+  for_each = local.include_nat_gateways == "yes" ? local.az_map : {}
 
-  route_table_id = aws_route_table.private[each.value].id
-  nat_gateway_id = aws_nat_gateway.base[each.value].id
+  route_table_id         = aws_route_table.private[each.key].id
+  nat_gateway_id         = aws_nat_gateway.base[each.key].id
   destination_cidr_block = "0.0.0.0/0"
 }
 
 resource "aws_route_table_association" "private" {
-  for_each = toset(var.availability_zones)
+  for_each = local.az_map
 
-  subnet_id = aws_subnet.private[each.value].id
-  route_table_id = aws_route_table.private[each.value].id
+  subnet_id      = aws_subnet.private[each.key].id
+  route_table_id = aws_route_table.private[each.key].id
 }
diff --git a/public_subnets.tf b/public_subnets.tf
index af68e00..f1500a5 100644
--- a/public_subnets.tf
+++ b/public_subnets.tf
@@ -1,42 +1,42 @@
 resource "aws_subnet" "public" {
-  for_each = toset(var.availability_zones)
+  for_each = local.az_map
 
-  vpc_id = aws_vpc.base.id
-  cidr_block = cidrsubnet(var.vpc_cidr, 8, index(var.availability_zones, each.value) + local.public_subnets_offset)
-  availability_zone = each.value
+  vpc_id            = aws_vpc.base.id
+  cidr_block        = each.value.public_subnet_cidr
+  availability_zone = each.value.zone
 
   tags = {
-    Name = "public-subnet-${var.component}-${var.deployment_identifier}-${each.value}"
-    Component = var.component
+    Name                 = "public-subnet-${var.component}-${var.deployment_identifier}-${each.value.zone}"
+    Component            = var.component
     DeploymentIdentifier = var.deployment_identifier
-    Tier = "public"
+    Tier                 = "public"
   }
 }
 
 resource "aws_route_table" "public" {
-  for_each = toset(var.availability_zones)
+  for_each = local.az_map
 
   vpc_id = aws_vpc.base.id
 
   tags = {
-    Name = "public-routetable-${var.component}-${var.deployment_identifier}-${each.value}"
-    Component = var.component
+    Name                 = "public-routetable-${var.component}-${var.deployment_identifier}-${each.value.zone}"
+    Component            = var.component
     DeploymentIdentifier = var.deployment_identifier
-    Tier = "public"
+    Tier                 = "public"
   }
 }
 
 resource "aws_route" "public_internet" {
-  for_each = toset(var.availability_zones)
+  for_each = local.az_map
 
-  route_table_id = aws_route_table.public[each.value].id
-  gateway_id = aws_internet_gateway.base_igw.id
+  route_table_id         = aws_route_table.public[each.key].id
+  gateway_id             = aws_internet_gateway.base_igw.id
   destination_cidr_block = "0.0.0.0/0"
 }
 
 resource "aws_route_table_association" "public" {
-  for_each = toset(var.availability_zones)
+  for_each = local.az_map
 
-  subnet_id = aws_subnet.public[each.value].id
-  route_table_id = aws_route_table.public[each.value].id
+  subnet_id      = aws_subnet.public[each.key].id
+  route_table_id = aws_route_table.public[each.key].id
 }
diff --git a/spec/integration/availability_zone_addition_spec.rb b/spec/integration/availability_zone_addition_spec.rb
index af7b598..5d04899 100644
--- a/spec/integration/availability_zone_addition_spec.rb
+++ b/spec/integration/availability_zone_addition_spec.rb
@@ -18,6 +18,18 @@
   let(:region) { 'eu-west-1' }
 
   describe 'adding a new availability zone' do
+    let(:test_dir) { @test_dir }
+
+    after(:each) do
+      # Cleanup: destroy all resources created during the test
+      if @test_dir && Dir.exist?(@test_dir)
+        terraform_exec = from_root_directory("vendor/terraform/bin/terraform")
+        Dir.chdir(@test_dir) do
+          system("#{terraform_exec} destroy -auto-approve")
+        end
+      end
+    end
+
     it 'does not destroy existing subnets when adding a new availability zone' do
       # Step 1: Apply with initial set of availability zones
       initial_state = apply_and_get_state(initial_availability_zones)
@@ -53,7 +65,7 @@
       # Check that only new resources are being created
       created_resources = resource_changes.select do |change|
         change['change']['actions'].include?('create') &&
-          %w[aws_subnet aws_route_table aws_route_table_association aws_nat_gateway aws_eip].include?(change['type'])
+          %w[aws_subnet aws_route_table aws_route_table_association aws_route aws_nat_gateway aws_eip].include?(change['type'])
       end
 
       # We expect exactly 1 new public subnet, 1 new private subnet,
@@ -82,24 +94,27 @@
   private
 
   def apply_and_get_state(availability_zones)
+    terraform_exec = from_root_directory("vendor/terraform/bin/terraform")
     # Create a temporary directory for this test run
-    test_dir = "spec/integration/test_runs/#{Time.now.to_i}"
-    FileUtils.mkdir_p(test_dir)
+    @test_dir = "spec/integration/test_runs/#{Time.now.to_i}"
+    FileUtils.mkdir_p(@test_dir)
 
     # Write the terraform configuration
-    File.write("#{test_dir}/main.tf", generate_terraform_config(availability_zones))
+    File.write("#{@test_dir}/main.tf", generate_terraform_config(availability_zones))
 
     # Initialize and apply
-    Dir.chdir(test_dir) do
-      system('terraform init', out: File::NULL, err: File::NULL)
-      system('terraform apply -auto-approve', out: File::NULL, err: File::NULL)
+    Dir.chdir(@test_dir) do
+      init_result = system("#{terraform_exec} init")
+      raise "Terraform init failed" unless init_result
+
+      apply_result = system("#{terraform_exec} apply -auto-approve")
+      raise "Terraform apply failed" unless apply_result
 
-      # Get the state
-      state_output = `terraform show -json`
-      JSON.parse(state_output)
+      # Get the state - properly write to file
+      state_json = `#{terraform_exec} show -json`
+      File.write("initialstate.json", state_json)
+      JSON.parse(state_json)
     end
-  ensure
-    # Cleanup is handled by the test framework
   end
 
   def plan_with_azs(availability_zones)
@@ -107,31 +122,48 @@ def plan_with_azs(availability_zones)
     test_dir = Dir.glob('spec/integration/test_runs/*').last
     File.write("#{test_dir}/main.tf", generate_terraform_config(availability_zones))
 
+    terraform_exec = from_root_directory("vendor/terraform/bin/terraform")
+
     # Run plan and capture output
     Dir.chdir(test_dir) do
-      `terraform plan -out=tfplan -json`
-      `terraform show -json tfplan`
+      `#{terraform_exec} plan -out=tfplan -json`
+      `#{terraform_exec} show -json tfplan > tfplan.json`
+      `cat tfplan.json`
     end
   end
 
   def generate_terraform_config(availability_zones)
     <<~HCL
-      module "base_networking" {
-        source = "../../../"
-
-        vpc_cidr              = "#{vpc_cidr}"
-        region                = "#{region}"
-        availability_zones    = #{availability_zones.inspect}
-        component             = "#{component}"
-        deployment_identifier = "#{deployment_identifier}"
+      terraform {
+        required_providers {
+          aws = {
+            source  = "hashicorp/aws"
+            version = "~> 4.0"
+          }
+        }
       }
 
       provider "aws" {
         region = "#{region}"
       }
+
+      module "base_networking" {
+        source = "#{from_root_directory("")}"
+
+        vpc_cidr                         = "#{vpc_cidr}"
+        region                           = "#{region}"
+        availability_zones               = #{availability_zones.inspect}
+        component                        = "#{component}"
+        deployment_identifier            = "#{deployment_identifier}"
+        include_route53_zone_association = "no"
+      }
     HCL
   end
 
+  def from_root_directory(dir)
+    return "../../../../#{dir}"
+  end
+
   def get_resource_ids(state, resource_type, resource_name)
     resources = state['values']['root_module']['child_modules']
       &.first['resources'] || []
@@ -147,4 +179,4 @@ def get_resource_ids(state, resource_type, resource_name)
       end
     end.to_h
   end
-end
\ No newline at end of file
+end
diff --git a/spec/unit/infra/prerequisites/main.tf b/spec/unit/infra/prerequisites/main.tf
index 2358614..0acf268 100644
--- a/spec/unit/infra/prerequisites/main.tf
+++ b/spec/unit/infra/prerequisites/main.tf
@@ -1,11 +1,11 @@
 resource "aws_default_vpc" "default" {}
 
 module "dns_zones" {
-  source = "infrablocks/dns-zones/aws"
+  source  = "infrablocks/dns-zones/aws"
   version = "2.0.0"
 
-  domain_name = var.domain_name
-  private_domain_name = var.domain_name
-  private_zone_vpc_id = aws_default_vpc.default.id
+  domain_name             = var.domain_name
+  private_domain_name     = var.domain_name
+  private_zone_vpc_id     = aws_default_vpc.default.id
   private_zone_vpc_region = var.region
 }
diff --git a/spec/unit/infra/prerequisites/provider.tf b/spec/unit/infra/prerequisites/provider.tf
index 66428ae..dc58d9a 100644
--- a/spec/unit/infra/prerequisites/provider.tf
+++ b/spec/unit/infra/prerequisites/provider.tf
@@ -1,3 +1,3 @@
 provider "aws" {
-  region  = var.region
+  region = var.region
 }
diff --git a/spec/unit/infra/prerequisites/terraform.tf b/spec/unit/infra/prerequisites/terraform.tf
index 9631039..98aa83e 100644
--- a/spec/unit/infra/prerequisites/terraform.tf
+++ b/spec/unit/infra/prerequisites/terraform.tf
@@ -3,7 +3,7 @@ terraform {
 
   required_providers {
     aws = {
-      source = "hashicorp/aws"
+      source  = "hashicorp/aws"
       version = "4.33"
     }
   }
diff --git a/spec/unit/infra/root/main.tf b/spec/unit/infra/root/main.tf
index e35a7a2..25e3d5c 100644
--- a/spec/unit/infra/root/main.tf
+++ b/spec/unit/infra/root/main.tf
@@ -9,19 +9,19 @@ data "terraform_remote_state" "prerequisites" {
 module "base_network" {
   source = "../../../.."
 
-  vpc_cidr = var.vpc_cidr
-  region = var.region
+  vpc_cidr           = var.vpc_cidr
+  region             = var.region
   availability_zones = var.availability_zones
 
-  public_subnets_offset = var.public_subnets_offset
+  public_subnets_offset  = var.public_subnets_offset
   private_subnets_offset = var.private_subnets_offset
 
-  component = var.component
+  component             = var.component
   deployment_identifier = var.deployment_identifier
-  dependencies = var.dependencies
+  dependencies          = var.dependencies
 
   private_zone_id = data.terraform_remote_state.prerequisites.outputs.private_zone_id
 
   include_route53_zone_association = var.include_route53_zone_association
-  include_nat_gateways = var.include_nat_gateways
+  include_nat_gateways             = var.include_nat_gateways
 }
diff --git a/spec/unit/infra/root/terraform.tf b/spec/unit/infra/root/terraform.tf
index 9631039..98aa83e 100644
--- a/spec/unit/infra/root/terraform.tf
+++ b/spec/unit/infra/root/terraform.tf
@@ -3,7 +3,7 @@ terraform {
 
   required_providers {
     aws = {
-      source = "hashicorp/aws"
+      source  = "hashicorp/aws"
       version = "4.33"
     }
   }
diff --git a/spec/unit/infra/root/variables.tf b/spec/unit/infra/root/variables.tf
index f732ca5..7cf0058 100644
--- a/spec/unit/infra/root/variables.tf
+++ b/spec/unit/infra/root/variables.tf
@@ -8,16 +8,16 @@ variable "component" {}
 variable "deployment_identifier" {}
 
 variable "public_subnets_offset" {
-  type = number
+  type    = number
   default = null
 }
 variable "private_subnets_offset" {
-  type = number
+  type    = number
   default = null
 }
 
 variable "dependencies" {
-  type = list(string)
+  type    = list(string)
   default = null
 }
 
diff --git a/variables.tf b/variables.tf
index e48a48a..c045b72 100644
--- a/variables.tf
+++ b/variables.tf
@@ -1,55 +1,74 @@
 variable "vpc_cidr" {
-  type = string
+  type        = string
   description = "The CIDR to use for the VPC."
 }
 variable "region" {
-  type = string
+  type        = string
   description = "The region into which to deploy the VPC."
 }
 variable "availability_zones" {
-  type = list(string)
-  description = "The availability zones for which to add subnets."
+  type        = list(string)
+  description = "The availability zones for which to add subnets. Mutually exclusive with availability_zone_configurations."
+  default     = null
+}
+
+variable "availability_zone_configurations" {
+  type = list(object({
+    zone                = string
+    public_subnet_cidr  = string
+    private_subnet_cidr = string
+  }))
+  description = <<-DESC
+    Explicit availability zone configurations with CIDR blocks. Mutually exclusive with availability_zones.
+    Use this to specify exact CIDR blocks for each subnet, allowing you to add new AZs without
+    changing existing subnet CIDR allocations. Example: [{
+      zone = "us-east-1a"
+      public_subnet_cidr = "10.0.0.0/24"
+      private_subnet_cidr = "10.0.10.0/24"
+    }]
+  DESC
+  default     = null
 }
 
 variable "component" {
-  type = string
+  type        = string
   description = "The component this network will contain."
 }
 variable "deployment_identifier" {
-  type = string
+  type        = string
   description = "An identifier for this instantiation."
 }
 variable "dependencies" {
   description = "A comma separated list of components depended on my this component."
-  type = list(string)
-  default = []
+  type        = list(string)
+  default     = []
 }
 
 variable "public_subnets_offset" {
   description = "The number of /24s to offset the public subnets in the VPC CIDR."
-  type = number
-  default = 0
+  type        = number
+  default     = 0
 }
 variable "private_subnets_offset" {
   description = "The number of /24s to offset the private subnets in the VPC CIDR."
-  type = number
-  default = 0
+  type        = number
+  default     = 0
 }
 
 variable "include_route53_zone_association" {
   description = "Whether or not to associate the VPC with a zone id (\"yes\" or \"no\")."
-  type = string
-  default = "yes"
+  type        = string
+  default     = "yes"
 }
 
 variable "private_zone_id" {
   description = "The ID of the private Route 53 zone."
-  type = string
-  default = null
+  type        = string
+  default     = null
 }
 
 variable "include_nat_gateways" {
   description = "Whether or not to deploy NAT gateways in each availability zone for outbound Internet connectivity (\"yes\" or \"no\")."
-  type = string
-  default = "yes"
+  type        = string
+  default     = "yes"
 }
diff --git a/vpc.tf b/vpc.tf
index a87c287..ac289eb 100644
--- a/vpc.tf
+++ b/vpc.tf
@@ -1,17 +1,17 @@
 resource "aws_vpc" "base" {
-  cidr_block = var.vpc_cidr
+  cidr_block           = var.vpc_cidr
   enable_dns_hostnames = true
 
   tags = {
-    Name = "vpc-${var.component}-${var.deployment_identifier}"
-    Component = var.component
+    Name                 = "vpc-${var.component}-${var.deployment_identifier}"
+    Component            = var.component
     DeploymentIdentifier = var.deployment_identifier
-    Dependencies = join(",", local.dependencies)
+    Dependencies         = join(",", local.dependencies)
   }
 }
 
 resource "aws_route53_zone_association" "base" {
-  count = local.include_route53_zone_association == "yes" ? 1 : 0
+  count   = local.include_route53_zone_association == "yes" ? 1 : 0
   zone_id = var.private_zone_id
-  vpc_id = aws_vpc.base.id
+  vpc_id  = aws_vpc.base.id
 }

From 3091fae85a80b434bdc6411f5bca3aca0a6e464c Mon Sep 17 00:00:00 2001
From: John Cowie Del Corral <john@go-atomic.io>
Date: Mon, 13 Oct 2025 10:24:12 +0200
Subject: [PATCH 3/7] Auto-fix some formatting issues

---
 .../availability_zone_addition_spec.rb        | 50 +++++++++++--------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/spec/integration/availability_zone_addition_spec.rb b/spec/integration/availability_zone_addition_spec.rb
index 5d04899..9ee943c 100644
--- a/spec/integration/availability_zone_addition_spec.rb
+++ b/spec/integration/availability_zone_addition_spec.rb
@@ -20,10 +20,10 @@
   describe 'adding a new availability zone' do
     let(:test_dir) { @test_dir }
 
-    after(:each) do
+    after do
       # Cleanup: destroy all resources created during the test
       if @test_dir && Dir.exist?(@test_dir)
-        terraform_exec = from_root_directory("vendor/terraform/bin/terraform")
+        terraform_exec = from_root_directory('vendor/terraform/bin/terraform')
         Dir.chdir(@test_dir) do
           system("#{terraform_exec} destroy -auto-approve")
         end
@@ -35,9 +35,12 @@
       initial_state = apply_and_get_state(initial_availability_zones)
 
       # Get the initial subnet IDs
-      initial_public_subnet_ids = get_resource_ids(initial_state, 'aws_subnet', 'public')
-      initial_private_subnet_ids = get_resource_ids(initial_state, 'aws_subnet', 'private')
-      initial_nat_gateway_ids = get_resource_ids(initial_state, 'aws_nat_gateway', 'base')
+      initial_public_subnet_ids = get_resource_ids(initial_state, 'aws_subnet',
+                                                   'public')
+      initial_private_subnet_ids = get_resource_ids(initial_state,
+                                                    'aws_subnet', 'private')
+      initial_nat_gateway_ids = get_resource_ids(initial_state,
+                                                 'aws_nat_gateway', 'base')
       initial_eip_ids = get_resource_ids(initial_state, 'aws_eip', 'nat')
 
       # Step 2: Plan with additional availability zone
@@ -60,12 +63,15 @@
 
       # Assert no existing resources are being destroyed
       expect(destroyed_resources).to be_empty,
-        "Expected no resources to be destroyed, but found: #{destroyed_resources.map { |r| "#{r['type']}.#{r['name']}" }.join(', ')}"
+                                     "Expected no resources to be destroyed, but found: #{destroyed_resources.map do |r|
+                                                                                            "#{r['type']}.#{r['name']}"
+                                                                                          end.join(', ')}"
 
       # Check that only new resources are being created
       created_resources = resource_changes.select do |change|
         change['change']['actions'].include?('create') &&
-          %w[aws_subnet aws_route_table aws_route_table_association aws_route aws_nat_gateway aws_eip].include?(change['type'])
+          %w[aws_subnet aws_route_table aws_route_table_association aws_route
+             aws_nat_gateway aws_eip].include?(change['type'])
       end
 
       # We expect exactly 1 new public subnet, 1 new private subnet,
@@ -81,12 +87,12 @@
       }
 
       actual_new_resources = created_resources.group_by { |r| r['type'] }
-        .transform_values(&:count)
+                                              .transform_values(&:count)
 
       expected_new_resources.each do |resource_type, expected_count|
         actual_count = actual_new_resources[resource_type] || 0
         expect(actual_count).to eq(expected_count),
-          "Expected #{expected_count} new #{resource_type} resources, but found #{actual_count}"
+                                "Expected #{expected_count} new #{resource_type} resources, but found #{actual_count}"
       end
     end
   end
@@ -94,25 +100,26 @@
   private
 
   def apply_and_get_state(availability_zones)
-    terraform_exec = from_root_directory("vendor/terraform/bin/terraform")
+    terraform_exec = from_root_directory('vendor/terraform/bin/terraform')
     # Create a temporary directory for this test run
     @test_dir = "spec/integration/test_runs/#{Time.now.to_i}"
     FileUtils.mkdir_p(@test_dir)
 
     # Write the terraform configuration
-    File.write("#{@test_dir}/main.tf", generate_terraform_config(availability_zones))
+    File.write("#{@test_dir}/main.tf",
+               generate_terraform_config(availability_zones))
 
     # Initialize and apply
     Dir.chdir(@test_dir) do
       init_result = system("#{terraform_exec} init")
-      raise "Terraform init failed" unless init_result
+      raise 'Terraform init failed' unless init_result
 
       apply_result = system("#{terraform_exec} apply -auto-approve")
-      raise "Terraform apply failed" unless apply_result
+      raise 'Terraform apply failed' unless apply_result
 
       # Get the state - properly write to file
       state_json = `#{terraform_exec} show -json`
-      File.write("initialstate.json", state_json)
+      File.write('initialstate.json', state_json)
       JSON.parse(state_json)
     end
   end
@@ -120,9 +127,10 @@ def apply_and_get_state(availability_zones)
   def plan_with_azs(availability_zones)
     # Update the configuration with new AZs
     test_dir = Dir.glob('spec/integration/test_runs/*').last
-    File.write("#{test_dir}/main.tf", generate_terraform_config(availability_zones))
+    File.write("#{test_dir}/main.tf",
+               generate_terraform_config(availability_zones))
 
-    terraform_exec = from_root_directory("vendor/terraform/bin/terraform")
+    terraform_exec = from_root_directory('vendor/terraform/bin/terraform')
 
     # Run plan and capture output
     Dir.chdir(test_dir) do
@@ -148,7 +156,7 @@ def generate_terraform_config(availability_zones)
       }
 
       module "base_networking" {
-        source = "#{from_root_directory("")}"
+        source = "#{from_root_directory('')}"
 
         vpc_cidr                         = "#{vpc_cidr}"
         region                           = "#{region}"
@@ -161,22 +169,22 @@ module "base_networking" {
   end
 
   def from_root_directory(dir)
-    return "../../../../#{dir}"
+    "../../../../#{dir}"
   end
 
   def get_resource_ids(state, resource_type, resource_name)
     resources = state['values']['root_module']['child_modules']
-      &.first['resources'] || []
+                &.first&.[]('resources') || []
 
     resources.select do |r|
       r['type'] == resource_type && r['name'] == resource_name
-    end.map do |r|
+    end.to_h do |r|
       # For for_each resources, use the index key (AZ name) as the key
       if r['index'].is_a?(String)
         [r['index'], r['values']['id']]
       else
         [r['index'].to_s, r['values']['id']]
       end
-    end.to_h
+    end
   end
 end

From 06904e1e4dd568ce6eecbf80db04eb3c43bde454 Mon Sep 17 00:00:00 2001
From: John Cowie Del Corral <john@go-atomic.io>
Date: Mon, 13 Oct 2025 11:08:51 +0200
Subject: [PATCH 4/7] Refactor spec

---
 .../availability_zone_addition_spec.rb        | 165 ++++++++++--------
 1 file changed, 91 insertions(+), 74 deletions(-)

diff --git a/spec/integration/availability_zone_addition_spec.rb b/spec/integration/availability_zone_addition_spec.rb
index 9ee943c..a98200f 100644
--- a/spec/integration/availability_zone_addition_spec.rb
+++ b/spec/integration/availability_zone_addition_spec.rb
@@ -3,70 +3,73 @@
 require 'spec_helper'
 require 'json'
 
-describe 'availability zone addition' do
-  let(:initial_availability_zones) do
-    %w[eu-west-1a eu-west-1b]
-  end
+# TODO
+# extract function for executing terraform
 
-  let(:updated_availability_zones) do
-    %w[eu-west-1a eu-west-1b eu-west-1c]
-  end
+INITIAL_AVAILABILITY_ZONES = %w[eu-west-1a eu-west-1b].freeze
+UPDATED_AVAILABILITY_ZONES = %w[eu-west-1a eu-west-1b eu-west-1c].freeze
 
-  let(:component) { 'test-component' }
-  let(:deployment_identifier) { 'test-deployment' }
-  let(:vpc_cidr) { '10.0.0.0/16' }
-  let(:region) { 'eu-west-1' }
+COMPONENT = 'test-component'
+DEPLOYMENT_IDENTIFIER = 'test-deployment'
+VPC_CIDR = '10.0.0.0/16'
+REGION = 'eu-west-1'
 
+describe 'availability zone addition' do
   describe 'adding a new availability zone' do
-    let(:test_dir) { @test_dir }
+    let(:initial_state) { @initial_state }
+    let(:resource_changes) { @resource_changes }
+
+    before(:all) do
+      @test_dir = make_test_run_dir
+
+      # Apply initial availability zones
+      apply_availability_zones(@test_dir, INITIAL_AVAILABILITY_ZONES)
+      Dir.chdir(@test_dir) do
+        @initial_state = get_terraform_state('applied.json')
+      end
 
-    after do
+      # Run plan with additional availability zone
+      plan_output = plan_with_azs(@test_dir, UPDATED_AVAILABILITY_ZONES)
+      plan_json = JSON.parse(plan_output)
+      @resource_changes = plan_json['resource_changes'] || []
+    end
+
+    after(:all) do
       # Cleanup: destroy all resources created during the test
       if @test_dir && Dir.exist?(@test_dir)
-        terraform_exec = from_root_directory('vendor/terraform/bin/terraform')
         Dir.chdir(@test_dir) do
+          terraform_exec = from_root_directory('vendor/terraform/bin/terraform')
           system("#{terraform_exec} destroy -auto-approve")
         end
       end
     end
 
-    it 'does not destroy existing subnets when adding a new availability zone' do
-      # Step 1: Apply with initial set of availability zones
-      initial_state = apply_and_get_state(initial_availability_zones)
-
+    it 'does not destroy existing subnets when adding new availability zone' do
       # Get the initial subnet IDs
-      initial_public_subnet_ids = get_resource_ids(initial_state, 'aws_subnet',
-                                                   'public')
-      initial_private_subnet_ids = get_resource_ids(initial_state,
-                                                    'aws_subnet', 'private')
-      initial_nat_gateway_ids = get_resource_ids(initial_state,
-                                                 'aws_nat_gateway', 'base')
-      initial_eip_ids = get_resource_ids(initial_state, 'aws_eip', 'nat')
-
-      # Step 2: Plan with additional availability zone
-      plan_output = plan_with_azs(updated_availability_zones)
-
-      # Parse the plan output to check for destructions
-      plan_json = JSON.parse(plan_output)
-
-      # Check that no existing resources are being destroyed
-      resource_changes = plan_json['resource_changes'] || []
+      resource_ids = gather_resource_ids(initial_state)
 
       # Find any destroy actions for our existing resources
       destroyed_resources = resource_changes.select do |change|
+        before_id = change.dig('change', 'before', 'id')
         change['change']['actions'].include?('delete') &&
-          (initial_public_subnet_ids.values.include?(change['change']['before']['id']) ||
-           initial_private_subnet_ids.values.include?(change['change']['before']['id']) ||
-           initial_nat_gateway_ids.values.include?(change['change']['before']['id']) ||
-           initial_eip_ids.values.include?(change['change']['before']['id']))
+          (resource_ids['public_subnets'].values.include?(before_id) ||
+            resource_ids['private_subnets'].values.include?(before_id) ||
+            resource_ids['nat_gateways'].values.include?(before_id) ||
+            resource_ids['eips'].values.include?(before_id))
       end
 
       # Assert no existing resources are being destroyed
-      expect(destroyed_resources).to be_empty,
-                                     "Expected no resources to be destroyed, but found: #{destroyed_resources.map do |r|
-                                                                                            "#{r['type']}.#{r['name']}"
-                                                                                          end.join(', ')}"
+      destroyed_resource_str = destroyed_resources.map do |r|
+        "#{r['type']}.#{r['name']}"
+      end.join(', ')
+      expect(destroyed_resources).to(
+        be_empty,
+        'Expected no resources to be destroyed, ' \
+        "but found: #{destroyed_resource_str}"
+      )
+    end
 
+    it 'creates resources for new availability zone' do
       # Check that only new resources are being created
       created_resources = resource_changes.select do |change|
         change['change']['actions'].include?('create') &&
@@ -81,7 +84,8 @@
         'aws_subnet' => 2, # 1 public + 1 private
         'aws_route_table' => 2, # 1 for public + 1 for private
         'aws_route_table_association' => 2, # 1 for public + 1 for private
-        'aws_route' => 2, # 1 for public internet route + 1 for private NAT route
+        # 1 for public internet route + 1 for private NAT route
+        'aws_route' => 2,
         'aws_nat_gateway' => 1,
         'aws_eip' => 1
       }
@@ -91,49 +95,55 @@
 
       expected_new_resources.each do |resource_type, expected_count|
         actual_count = actual_new_resources[resource_type] || 0
-        expect(actual_count).to eq(expected_count),
-                                "Expected #{expected_count} new #{resource_type} resources, but found #{actual_count}"
+        expect(actual_count).to(
+          eq(expected_count),
+          "Expected #{expected_count} new #{resource_type} resources, " \
+          "but found #{actual_count}"
+        )
       end
     end
   end
 
   private
 
-  def apply_and_get_state(availability_zones)
+  def make_test_run_dir
+    dir = "spec/integration/test_runs/#{Time.now.to_i}"
+    FileUtils.mkdir_p(dir)
+    dir
+  end
+
+  def apply_availability_zones(terraform_dir, availability_zones)
     terraform_exec = from_root_directory('vendor/terraform/bin/terraform')
-    # Create a temporary directory for this test run
-    @test_dir = "spec/integration/test_runs/#{Time.now.to_i}"
-    FileUtils.mkdir_p(@test_dir)
 
     # Write the terraform configuration
-    File.write("#{@test_dir}/main.tf",
+    File.write("#{terraform_dir}/main.tf",
                generate_terraform_config(availability_zones))
 
     # Initialize and apply
-    Dir.chdir(@test_dir) do
+    Dir.chdir(terraform_dir) do
       init_result = system("#{terraform_exec} init")
       raise 'Terraform init failed' unless init_result
 
       apply_result = system("#{terraform_exec} apply -auto-approve")
       raise 'Terraform apply failed' unless apply_result
-
-      # Get the state - properly write to file
-      state_json = `#{terraform_exec} show -json`
-      File.write('initialstate.json', state_json)
-      JSON.parse(state_json)
     end
   end
 
-  def plan_with_azs(availability_zones)
-    # Update the configuration with new AZs
-    test_dir = Dir.glob('spec/integration/test_runs/*').last
-    File.write("#{test_dir}/main.tf",
-               generate_terraform_config(availability_zones))
+  def get_terraform_state(state_file)
+    terraform_exec = from_root_directory('vendor/terraform/bin/terraform')
+    state_json = `#{terraform_exec} show -json`
+    File.write(state_file, state_json)
+    JSON.parse(state_json)
+  end
 
+  def plan_with_azs(terraform_dir, availability_zones)
     terraform_exec = from_root_directory('vendor/terraform/bin/terraform')
+    # Update the configuration with new AZs
+    File.write("#{terraform_dir}/main.tf",
+               generate_terraform_config(availability_zones))
 
     # Run plan and capture output
-    Dir.chdir(test_dir) do
+    Dir.chdir(terraform_dir) do
       `#{terraform_exec} plan -out=tfplan -json`
       `#{terraform_exec} show -json tfplan > tfplan.json`
       `cat tfplan.json`
@@ -152,17 +162,17 @@ def generate_terraform_config(availability_zones)
       }
 
       provider "aws" {
-        region = "#{region}"
+        region = "#{REGION}"
       }
 
       module "base_networking" {
         source = "#{from_root_directory('')}"
 
-        vpc_cidr                         = "#{vpc_cidr}"
-        region                           = "#{region}"
+        vpc_cidr                         = "#{VPC_CIDR}"
+        region                           = "#{REGION}"
         availability_zones               = #{availability_zones.inspect}
-        component                        = "#{component}"
-        deployment_identifier            = "#{deployment_identifier}"
+        component                        = "#{COMPONENT}"
+        deployment_identifier            = "#{DEPLOYMENT_IDENTIFIER}"
         include_route53_zone_association = "no"
       }
     HCL
@@ -172,19 +182,26 @@ def from_root_directory(dir)
     "../../../../#{dir}"
   end
 
+  def gather_resource_ids(state)
+    {
+      public_subnets: get_resource_ids(state, 'aws_subnet', 'public'),
+      private_subnets: get_resource_ids(state, 'aws_subnet', 'private'),
+      nat_gateways: get_resource_ids(state, 'aws_nat_gateway', 'base'),
+      eips: get_resource_ids(state, 'aws_eip', 'nat')
+    }
+  end
+
   def get_resource_ids(state, resource_type, resource_name)
     resources = state['values']['root_module']['child_modules']
                 &.first&.[]('resources') || []
 
-    resources.select do |r|
+    matching_resources = resources.select do |r|
       r['type'] == resource_type && r['name'] == resource_name
-    end.to_h do |r|
+    end
+
+    matching_resources.to_h do |r|
       # For for_each resources, use the index key (AZ name) as the key
-      if r['index'].is_a?(String)
-        [r['index'], r['values']['id']]
-      else
-        [r['index'].to_s, r['values']['id']]
-      end
+      [r['index'].to_s, r['values']['id']]
     end
   end
 end

From 0d74a35df7a37a0e177b72aff46dc8e4ae6aa0eb Mon Sep 17 00:00:00 2001
From: John Cowie Del Corral <john@go-atomic.io>
Date: Mon, 13 Oct 2025 16:35:29 +0200
Subject: [PATCH 5/7] Default private_subnets_offset to 128

---
 defaults.tf  | 2 +-
 variables.tf | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/defaults.tf b/defaults.tf
index 0fd4546..fc8cef5 100644
--- a/defaults.tf
+++ b/defaults.tf
@@ -19,7 +19,7 @@ locals {
     for idx, az in var.availability_zones : {
       zone                = az
       public_subnet_cidr  = cidrsubnet(var.vpc_cidr, 8, idx + local.public_subnets_offset)
-      private_subnet_cidr = cidrsubnet(var.vpc_cidr, 8, idx + 128 + local.private_subnets_offset)
+      private_subnet_cidr = cidrsubnet(var.vpc_cidr, 8, idx + local.private_subnets_offset)
     }
   ]
 
diff --git a/variables.tf b/variables.tf
index c045b72..8187ea4 100644
--- a/variables.tf
+++ b/variables.tf
@@ -52,7 +52,7 @@ variable "public_subnets_offset" {
 variable "private_subnets_offset" {
   description = "The number of /24s to offset the private subnets in the VPC CIDR."
   type        = number
-  default     = 0
+  default     = 128
 }
 
 variable "include_route53_zone_association" {

From 73cdf6e02986e41de6fcacf2f1e87fb1dc06953b Mon Sep 17 00:00:00 2001
From: John Cowie Del Corral <john@go-atomic.io>
Date: Mon, 13 Oct 2025 16:54:55 +0200
Subject: [PATCH 6/7] Add notes to CHANGELOG

---
 CHANGELOG.md | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2540dac..a0a8daa 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,48 @@
 ## Unreleased
 
+BACKWARDS INCOMPATIBILITIES / NOTES:
+
+* `for_each` is used instead of `count` for creating resources for each 
+  availability zone, so the availability zone will be used as the resource key, 
+  not an index. 
+
+  As a consequence, if resources have been created with a 
+  previous version of this module, they will need to be `moved` to avoid them
+  being destroyed and recreated.
+
+  e.g.
+
+  ```terraform
+     moved {
+       from = module.base-network.aws_subnet.public[0]
+       to   = module.base-network.aws_subnet.public["eu-west-1a"]
+     }
+  
+     moved {
+       from = module.base-network.aws_subnet.private[0]
+       to   = module.base-network.aws_subnet.private["eu-west-1a"]
+     }
+  
+     # etc..
+  ```
+
+
+* The default value for the `private_subnets_offset` variable has been changed
+  from 0 to 128.  This means that if the offsets are not provided, there will
+  be sufficient space between the private and public CIDR blocks such that new 
+  availability_zones can be added without needing to destroy existing private 
+  subnets.
+
+ENHANCEMENTS
+
+* As an alternative to the `availability_zones` variable, an
+  `availability_zone_configuration` variable is also supported, which takes a 
+  list of objects with the keys `zone`, `public_subnet_cidr` and
+  `private_subnet_cidr`. This can be useful in cases where a new availability
+  zone is being added, but inference of the CIDR blocks could result in existing
+  subnets being destroyed and recreated, in which case the existing public and 
+  private subnet CIDRs can be explicitly supplied to prevent this. 
+
 ## 5.1.0 (13th Feb 2023)
 
 ENHANCEMENTS

From a4ff4405186892847a965e1c91415526646bbcbb Mon Sep 17 00:00:00 2001
From: John Cowie Del Corral <john@go-atomic.io>
Date: Tue, 14 Oct 2025 11:52:40 +0200
Subject: [PATCH 7/7] Add some additional notes in CHANGELOG around breaking
 private_subnets_offset change

---
 CHANGELOG.md | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a0a8daa..aee2c72 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -33,6 +33,11 @@ BACKWARDS INCOMPATIBILITIES / NOTES:
   availability_zones can be added without needing to destroy existing private 
   subnets.
 
+  **NB**. If resources were created with a previous module version and the 
+  `private_subnets_offset` variable wasn't previously supplied, then the 
+  variable will need to be explicitly set to `0` to ensure that the private 
+  subnet CIDR blocks are calculated to the same values that they previously had.
+
 ENHANCEMENTS
 
 * As an alternative to the `availability_zones` variable, an