Skip to content

EBS Disk Encryption + Fix an extra log group #47

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

EBS Disk Encryption + Fix an extra log group #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5c12db9
Add EBS Disk Encryption
bblnasch Aug 25, 2022
f477345
Add tfenv terraform version file
bblnasch Aug 25, 2022
8f48a2f
Fix duplicate log group error for multi-executor demo
bblnasch Aug 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
.terraform
plan
terraform.tfstate
terraform.tfstate.backup
.terraform.lock.hcl
tfplan
1 change: 1 addition & 0 deletions .terraform-version
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1.1.9
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this is annoying? it makes my life nicer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we usually use asdf at sourcegraph which uses .tool-versions, could that help you, too?

53 changes: 28 additions & 25 deletions examples/multiple-executors/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
locals {
region = "us-west-2"
availability_zone = "us-west-2a"
docker_mirror_static_ip = "10.0.1.4"
region = "us-west-2"
availability_zone = "us-west-2a"
docker_mirror_static_ip = "10.0.1.4"
sourcegraph_external_url = "https://sourcegraph.acme.com"
executor_sourcegraph_executor_proxy_password = "hunter2"
}

module "networking" {
Expand All @@ -25,32 +27,33 @@ module "executors-codeintel" {
source = "sourcegraph/executors/aws//modules/executors"
version = "3.43.0" # LATEST

vpc_id = module.networking.vpc_id
subnet_id = module.networking.subnet_id
resource_prefix = "codeintel-prod"
instance_tag = "codeintel-prod"
sourcegraph_external_url = "https://sourcegraph.acme.com"
sourcegraph_executor_proxy_password = "hunter2"
queue_name = "codeintel"
metrics_environment_label = "prod"
docker_registry_mirror = "http://${local.docker_mirror_static_ip}:5000"
# docker_registry_mirror_node_exporter_url = "http://${local.docker_mirror_static_ip}:9999"
use_firecracker = true
vpc_id = module.networking.vpc_id
subnet_id = module.networking.subnet_id
resource_prefix = "codeintel-prod"
instance_tag = "codeintel-prod"
sourcegraph_external_url = local.sourcegraph_external_url
sourcegraph_executor_proxy_password = local.executor_sourcegraph_executor_proxy_password
queue_name = "codeintel"
metrics_environment_label = "prod"
docker_registry_mirror = "http://${local.docker_mirror_static_ip}:5000"
# docker_registry_mirror_node_exporter_url = "http://${local.docker_mirror_static_ip}:9999"
use_firecracker = true
}

module "executors-batches" {
source = "sourcegraph/executors/aws//modules/executors"
version = "3.43.0" # LATEST

vpc_id = module.networking.vpc_id
subnet_id = module.networking.subnet_id
resource_prefix = "batches-prod"
instance_tag = "batches-prod"
sourcegraph_external_url = "https://sourcegraph.acme.com"
sourcegraph_executor_proxy_password = "hunter2"
queue_name = "batches"
metrics_environment_label = "prod"
docker_registry_mirror = "http://${local.docker_mirror_static_ip}:5000"
# docker_registry_mirror_node_exporter_url = "http://${local.docker_mirror_static_ip}:9999"
use_firecracker = true
vpc_id = module.networking.vpc_id
subnet_id = module.networking.subnet_id
resource_prefix = "batches-prod"
instance_tag = "batches-prod"
sourcegraph_external_url = local.sourcegraph_external_url
sourcegraph_executor_proxy_password = local.executor_sourcegraph_executor_proxy_password
queue_name = "batches"
metrics_environment_label = "prod"
docker_registry_mirror = "http://${local.docker_mirror_static_ip}:5000"
# docker_registry_mirror_node_exporter_url = "http://${local.docker_mirror_static_ip}:9999"
use_firecracker = true
need_syslogs = false
}
3 changes: 3 additions & 0 deletions main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ module "aws-docker-mirror" {
machine_ami = var.docker_mirror_machine_ami
machine_type = var.docker_mirror_machine_type
boot_disk_size = var.docker_mirror_boot_disk_size
boot_disk_kms_key_id = var.docker_mirror_boot_disk_kms_key_id
disk_kms_key_id = var.docker_mirror_disk_kms_key_id
static_ip = var.docker_mirror_static_ip
ssh_access_cidr_range = var.docker_mirror_ssh_access_cidr_range
instance_tag_prefix = var.executor_instance_tag
Expand All @@ -29,6 +31,7 @@ module "aws-executor" {
machine_image = var.executor_machine_image
machine_type = var.executor_machine_type
boot_disk_size = var.executor_boot_disk_size
boot_disk_kms_key_id = var.executor_boot_disk_kms_key_id
preemptible_machines = var.executor_preemptible_machines
instance_tag = var.executor_instance_tag
ssh_access_cidr_range = var.executor_ssh_access_cidr_range
Expand Down
2 changes: 2 additions & 0 deletions modules/docker-mirror/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ resource "aws_instance" "default" {
volume_size = var.boot_disk_size
volume_type = "gp3"
encrypted = true
kms_key_id = var.boot_disk_kms_key_id
}

tags = {
Expand All @@ -65,6 +66,7 @@ resource "aws_ebs_volume" "docker-storage" {
availability_zone = data.aws_subnet.main.availability_zone
size = var.disk_size
encrypted = true
kms_key_id = var.disk_kms_key_id
type = "gp3"
iops = var.disk_iops
throughput = var.disk_throughput
Expand Down
12 changes: 12 additions & 0 deletions modules/docker-mirror/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ variable "boot_disk_size" {
description = "Docker registry mirror node disk size in GB."
}

variable "boot_disk_kms_key_id" {
type = string
default = null
description = "[Optional] The KMS Key ID for EBS volume encryption."
}

variable "disk_size" {
type = number
default = 64
Expand All @@ -44,6 +50,12 @@ variable "disk_throughput" {
description = "Persistent Docker registry mirror disk throughput in MiB/s."
}

variable "disk_kms_key_id" {
type = string
default = null
description = "[Optional] The KMS Key ID for mirror disk EBS volume encryption."
}

variable "static_ip" {
type = string
description = "The IP to statically assign to the instance. Should be internal."
Expand Down
3 changes: 3 additions & 0 deletions modules/executors/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ resource "aws_security_group" "metrics_access" {

resource "aws_cloudwatch_log_group" "syslogs" {
# TODO: This is hardcoded in the executor image.
count = var.need_syslogs ? 1 : 0
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally i think we'd pull some of this stuff out into a different module altogether

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if using a data source to check for existence might work. We don't actually need to create it, that happens automatically when the first log comes in, we just want to configure retention, really.

name = "executors"
retention_in_days = 7
}
Expand Down Expand Up @@ -113,6 +114,8 @@ resource "aws_launch_template" "executor" {
volume_type = "gp3"
iops = var.boot_disk_iops
throughput = var.boot_disk_throughput
encrypted = true
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is 💥 if people use this code, so maybe we want to shim this with a var for "false" for awhile?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean it would depend on the boot_disk_kms_key_id variable being set correctly? Could we say

Suggested change
encrypted = true
encrypted = var.boot_disk_kms_key_id != null ? true : false

to mitigate it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, no that can't be it.. we set encryption to true for the docker thing as well 🤔 I'm lost, what do you mean? 😬

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅 aws_launch_template

Nevermind, new instances will just start to get encrypted. I thought this part of an aws_instance definition, which would cause the instance to be recreated.

kms_key_id = var.boot_disk_kms_key_id
}
}

Expand Down
12 changes: 12 additions & 0 deletions modules/executors/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ variable "boot_disk_throughput" {
description = "Persistent Docker registry mirror disk throughput in MiB/s."
}

variable "boot_disk_kms_key_id" {
type = string
default = null
description = "[Optional] The KMS Key ID for EBS volume encryption."
}

variable "preemptible_machines" {
type = bool
default = false
Expand Down Expand Up @@ -182,3 +188,9 @@ variable "assign_public_ip" {
default = true
description = "If false, no public IP will be associated with the executors."
}

variable "need_syslogs" {
type = bool
default = true
description = "For multi-executor only 1 of these needs to be made."
}
18 changes: 18 additions & 0 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ variable "docker_mirror_boot_disk_size" {
description = "Docker registry mirror node disk size in GB."
}

variable "docker_mirror_boot_disk_kms_key_id" {
type = string
default = null
description = "[Optional] KMS Key ID for EBS boot disk encryption"
}

variable "docker_mirror_disk_kms_key_id" {
type = string
default = null
description = "[Optional] KMS Key ID for EBS disk encryption"
}

variable "docker_mirror_static_ip" {
type = string
default = "10.0.1.4"
Expand Down Expand Up @@ -63,6 +75,12 @@ variable "executor_boot_disk_size" {
description = "Executor node disk size in GB"
}

variable "executor_boot_disk_kms_key_id" {
type = string
default = null
description = "[Optional] KMS Key ID for EBS boot disk encryption"
}

variable "executor_preemptible_machines" {
type = bool
default = false
Expand Down