Adding Support for updating Availability Zones

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
   }

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" {

private_subnets.tf

@@ -1,38 +1,42 @@
 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"
   }
 }
 
 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"
   }
 }
 
 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
 }

public_subnets.tf

@@ -1,38 +1,42 @@
 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"
   }
 }
 
 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"
   }
 }
 
 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
 }

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