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