Skip to content

Commit

Permalink
Fix bug in availability zone mapping (#49)
Browse files Browse the repository at this point in the history
Previously we had a bug in the netblock arithmetics for allocating the
private vs the public subnets based on availability zones. Due to this
bug we had instances being deployed into private networks which had an
availability zone that wasn't in our list of supported zones and
therefore we were never deploying tasks onto it.

In order to address this, since it requires changing the networking
stack, we had to also implement a lot of changes to how security groups
are being created and switched, which was anyways a good things to do.

The key point is that when you would like to modify a security group,
because for example you need to change it's subnet, you need to create
the new security group before deleting the old one.
In order to do this, you need to make use of `name_prefix` instead of
static `name` attributes.

This PR does that.
  • Loading branch information
hellais authored Apr 23, 2024
1 parent 960f8fb commit 104d148
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 19 deletions.
4 changes: 2 additions & 2 deletions tf/environments/dev/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ module "ooniapi_cluster" {
asg_max = 6
asg_desired = 2

instance_type = "t2.small"
instance_type = "t3.small"

tags = merge(
local.tags,
Expand All @@ -296,7 +296,7 @@ module "oonith_cluster" {
asg_max = 4
asg_desired = 1

instance_type = "t2.small"
instance_type = "t3.small"

tags = merge(
local.tags,
Expand Down
10 changes: 7 additions & 3 deletions tf/modules/clickhouse/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ resource "aws_instance" "clickhouse_server_prod_tier1" {
}

user_data = templatefile("${path.module}/templates/clickhouse-setup.sh", {
hostname = local.clickhouse_hostname,
device_name = local.clickhouse_device_name
hostname = local.clickhouse_hostname,
device_name = local.clickhouse_device_name
})

tags = merge(
Expand Down Expand Up @@ -110,7 +110,7 @@ resource "aws_eip" "clickhouse_ip" {
}

resource "aws_security_group" "clickhouse_sg" {
name = "clickhouse_sg"
name_prefix = "clickhouse"
description = "Allow Clickhouse traffic"

vpc_id = var.aws_vpc_id
Expand Down Expand Up @@ -140,6 +140,10 @@ resource "aws_security_group" "clickhouse_sg" {
cidr_blocks = ["0.0.0.0/0"]
}

lifecycle {
create_before_destroy = true
}

tags = local.tags
}

Expand Down
14 changes: 11 additions & 3 deletions tf/modules/ecs_cluster/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ EOF
resource "aws_security_group" "web" {
description = "controls access to the applications ELB web endpoint"

vpc_id = var.vpc_id
name = "${var.name}-web-sg"
vpc_id = var.vpc_id
name_prefix = "ooni-ecs-web"

ingress {
protocol = "tcp"
Expand All @@ -75,6 +75,10 @@ resource "aws_security_group" "web" {
ipv6_cidr_blocks = ["::/0"]
}

lifecycle {
create_before_destroy = true
}

tags = var.tags
}

Expand All @@ -94,7 +98,7 @@ resource "aws_iam_role_policy" "container_host" {
resource "aws_security_group" "container_host" {
description = "controls direct access to application instances"
vpc_id = var.vpc_id
name = "${var.name}-container-host-sg"
name_prefix = "ooni-ecs-container"

ingress {
protocol = "tcp"
Expand Down Expand Up @@ -124,6 +128,10 @@ resource "aws_security_group" "container_host" {
ipv6_cidr_blocks = ["::/0"]
}

lifecycle {
create_before_destroy = true
}

tags = var.tags
}

Expand Down
21 changes: 18 additions & 3 deletions tf/modules/network/main.tf
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
locals {
private_net_offset = 100
}

resource "aws_vpc" "main" {
cidr_block = var.vpc_main_cidr_block
Expand All @@ -23,6 +26,10 @@ resource "aws_subnet" "public" {

depends_on = [aws_internet_gateway.gw]

lifecycle {
create_before_destroy = true
}

tags = {
Name = "ooni-public-subnet-${count.index}"
}
Expand All @@ -31,17 +38,21 @@ resource "aws_subnet" "public" {
resource "aws_subnet" "private" {
count = var.az_count

cidr_block = cidrsubnet(aws_vpc.main.cidr_block, 8, var.az_count + count.index)
cidr_block = cidrsubnet(aws_vpc.main.cidr_block, 8, local.private_net_offset + count.index)

ipv6_cidr_block = cidrsubnet(aws_vpc.main.ipv6_cidr_block, 8, var.az_count + count.index)
ipv6_cidr_block = cidrsubnet(aws_vpc.main.ipv6_cidr_block, 8, local.private_net_offset + count.index)
assign_ipv6_address_on_creation = true

availability_zone = element(var.aws_availability_zones_available.names, var.az_count + count.index)
availability_zone = element(var.aws_availability_zones_available.names, count.index)
vpc_id = aws_vpc.main.id
map_public_ip_on_launch = false

depends_on = [aws_internet_gateway.gw]

lifecycle {
create_before_destroy = true
}

tags = {
Name = "ooni-private-subnet-${count.index}"
}
Expand Down Expand Up @@ -128,4 +139,8 @@ resource "aws_route_table_association" "private" {
count = var.az_count
subnet_id = element(aws_subnet.private[*].id, count.index)
route_table_id = element(aws_route_table.private[*].id, count.index)

lifecycle {
create_before_destroy = true
}
}
17 changes: 13 additions & 4 deletions tf/modules/ooni_backendproxy/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ data "aws_ssm_parameter" "ubuntu_22_ami" {
# https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group#recreating-a-security-group
resource "aws_security_group" "nginx_sg" {
description = "security group for nginx"
name_prefix = "ooni-bckprx"

vpc_id = var.vpc_id

Expand Down Expand Up @@ -33,6 +34,10 @@ resource "aws_security_group" "nginx_sg" {
]
}

lifecycle {
create_before_destroy = true
}

tags = var.tags
}

Expand Down Expand Up @@ -88,10 +93,14 @@ resource "aws_autoscaling_group" "oonibackend_proxy" {
}

resource "aws_alb_target_group" "oonibackend_proxy" {
name = var.name
port = 80
protocol = "HTTP"
vpc_id = var.vpc_id
name_prefix = "oobpx"
port = 80
protocol = "HTTP"
vpc_id = var.vpc_id

lifecycle {
create_before_destroy = true
}

tags = var.tags
}
Expand Down
5 changes: 4 additions & 1 deletion tf/modules/ooniapi_service/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ resource "aws_ecs_task_definition" "ooniapi_service" {
}

resource "aws_security_group" "ooniapi_service_ecs" {
name = "${local.name}_ecs-sg"
name_prefix = "ooniapi-service"
description = "Allow all traffic"
vpc_id = var.vpc_id

Expand Down Expand Up @@ -130,6 +130,9 @@ resource "aws_security_group" "ooniapi_service_ecs" {
ipv6_cidr_blocks = ["::/0"]
}

lifecycle {
create_before_destroy = true
}
}

resource "aws_ecs_service" "ooniapi_service" {
Expand Down
6 changes: 5 additions & 1 deletion tf/modules/oonith_service/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ resource "aws_ecs_task_definition" "oonith_service" {
}

resource "aws_security_group" "oonith_service_ecs" {
name = "${local.name}_ecs-sg"
name_prefix = "oonith-service"
description = "Allow all traffic"
vpc_id = var.vpc_id

Expand All @@ -118,6 +118,10 @@ resource "aws_security_group" "oonith_service_ecs" {
cidr_blocks = ["0.0.0.0/0"]
ipv6_cidr_blocks = ["::/0"]
}

lifecycle {
create_before_destroy = true
}
}

resource "aws_ecs_service" "oonith_service" {
Expand Down
8 changes: 6 additions & 2 deletions tf/modules/postgresql/main.tf
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
resource "aws_security_group" "pg" {
description = "controls access to postgresql database"

vpc_id = var.vpc_id
name = "${var.name}-sg"
vpc_id = var.vpc_id
name_prefix = "oonipg"

ingress {
protocol = "tcp"
Expand All @@ -21,6 +21,10 @@ resource "aws_security_group" "pg" {
]
}

lifecycle {
create_before_destroy = true
}

tags = var.tags
}

Expand Down

0 comments on commit 104d148

Please sign in to comment.