Skip to content
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.

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

[Issue #3162] Make s3 buckets an iterator, with generated sub-paths #3251

Merged
merged 8 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 infra/api/app-config/env-config/outputs.tf
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ output "scheduled_jobs" {
value = local.scheduled_jobs
}

output "s3_buckets" {
value = local.s3_buckets
}

output "incident_management_service_integration" {
value = var.has_incident_management_service ? {
integration_url_param_name = "/monitoring/${var.app_name}/${var.environment}/incident-management-integration-url"
Expand Down
36 changes: 36 additions & 0 deletions infra/api/app-config/env-config/s3_buckets.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
locals {
# Renaming something in here might make terraform try (and fail) to re-create the buckets.
# So just assuming you can't ever rename anything in here!
# (except the environment variables eg. `env_var` those are always safe)
s3_buckets = {
# s3_buckets[key].env_var must:
# - Start with the same prefix as the object key
# - End with BUCKET
public-files = {
env_var = "PUBLIC_FILES_BUCKET"
public = true # Handle with care!!! this makes your bucket public
# paths[index].env_var must:
# - Start with the same prefix as it's parent bucket, minus the "BUCKET" suffix
# - Include the name of the path in some way, doesn't have to be verbatim
# - End with PATH
#
# path must start with a forward slash
paths = [
{
path = "/opportunity-data-extracts"
env_var = "PUBLIC_FILES_OPPORTUNITY_DATA_EXTRACTS_PATH"
},
]
}
draft-files = {
env_var = "DRAFT_FILES_BUCKET"
public = false
paths = []
}
api-analytics-transfer = {
env_var = "API_ANALYTICS_BUCKET"
public = false
paths = []
}
}
}
1 change: 1 addition & 0 deletions infra/api/service/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ module "service" {
migrator_access_policy_arn = data.aws_iam_policy.migrator_db_access_policy[0].arn

scheduled_jobs = local.environment_config.scheduled_jobs
s3_buckets = local.environment_config.s3_buckets

db_vars = module.app_config.has_database ? {
security_group_ids = data.aws_rds_cluster.db_cluster[0].vpc_security_group_ids
Expand Down
15 changes: 13 additions & 2 deletions infra/modules/service/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ locals {
task_executor_role_name = "${var.service_name}-task-executor"
image_url = var.image_repository_url != null ? "${var.image_repository_url}:${var.image_tag}" : "${data.aws_ecr_repository.app[0].repository_url}:${var.image_tag}"
hostname = var.hostname != null ? [{ name = "HOSTNAME", value = var.hostname }] : []
drafts_s3_bucket_url = var.enable_drafts_bucket != null && length(aws_s3_bucket.draft_documents) > 0 ? [{ name : "DRAFTS_S3_BUCKET_URL", value : aws_s3_bucket.draft_documents[0].bucket_regional_domain_name }] : []

base_environment_variables = concat([
{ name : "PORT", value : tostring(var.container_port) },
Expand All @@ -38,7 +37,7 @@ locals {
# TODO: https://github.com/HHS/simpler-grants-gov/issues/3177
# { name : "DEPLOY_GITHUB_REF", value : data.external.deploy_github_ref.result.value },
{ name : "DEPLOY_WHOAMI", value : data.external.whoami.result.value }
], local.hostname, local.drafts_s3_bucket_url)
], local.hostname)
db_environment_variables = var.db_vars == null ? [] : [
{ name : "DB_HOST", value : var.db_vars.connection_info.host },
{ name : "DB_PORT", value : var.db_vars.connection_info.port },
Expand All @@ -53,6 +52,18 @@ locals {
for name, value in var.extra_environment_variables :
{ name : name, value : value }
],
[
for name, value in var.s3_buckets :
{ name : value.env_var, value : "s3://${aws_s3_bucket.s3_buckets[name].bucket_regional_domain_name}" }
],
flatten([
for name, s3_bucket in var.s3_buckets : [
for paths in s3_bucket.paths : {
name = paths.env_var,
value = "s3://${aws_s3_bucket.s3_buckets[name].bucket_regional_domain_name}${paths.path}"
}
]
])
Comment on lines +55 to +66
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😵

)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
resource "aws_s3_bucket" "draft_documents" {
count = var.enable_drafts_bucket ? 1 : 0
resource "aws_s3_bucket" "s3_buckets" {
for_each = var.s3_buckets

bucket_prefix = "${var.service_name}-documents-draft"
bucket_prefix = "${var.service_name}-${each.key}-"
force_destroy = false
# checkov:skip=CKV2_AWS_62:Event notification not necessary for this bucket especially due to likely use of lifecycle rules
# checkov:skip=CKV_AWS_18:Access logging was not considered necessary for this bucket
Expand All @@ -13,25 +13,30 @@ resource "aws_s3_bucket" "draft_documents" {
# checkov:skip=CKV2_AWS_61:False positive
}

resource "aws_s3_bucket_public_access_block" "draft_documents" {
count = var.enable_drafts_bucket ? 1 : 0
resource "aws_s3_bucket_public_access_block" "s3_buckets" {
for_each = var.s3_buckets

bucket = aws_s3_bucket.draft_documents[0].id
bucket = aws_s3_bucket.s3_buckets[each.key].id

block_public_acls = true
block_public_policy = true
ignore_public_acls = true
restrict_public_buckets = true
block_public_acls = !each.value.public
block_public_policy = !each.value.public
ignore_public_acls = !each.value.public
restrict_public_buckets = !each.value.public

#checkov:skip=CKV_AWS_56:Some buckets are public on purpose
#checkov:skip=CKV_AWS_55:Some buckets are public on purpose
#checkov:skip=CKV_AWS_54:Some buckets are public on purpose
#checkov:skip=CKV_AWS_53:Some buckets are public on purpose
}

data "aws_iam_policy_document" "draft_documents_put_access" {
count = var.enable_drafts_bucket ? 1 : 0
data "aws_iam_policy_document" "s3_buckets_put_access" {
for_each = var.s3_buckets

statement {
effect = "Allow"
resources = [
aws_s3_bucket.draft_documents[0].arn,
"${aws_s3_bucket.draft_documents[0].arn}/*"
aws_s3_bucket.s3_buckets[each.key].arn,
"${aws_s3_bucket.s3_buckets[each.key].arn}/*"
]
actions = ["s3:*"]

Expand All @@ -45,8 +50,8 @@ data "aws_iam_policy_document" "draft_documents_put_access" {
sid = "AllowSSLRequestsOnly"
effect = "Deny"
resources = [
aws_s3_bucket.draft_documents[0].arn,
"${aws_s3_bucket.draft_documents[0].arn}/*"
aws_s3_bucket.s3_buckets[each.key].arn,
"${aws_s3_bucket.s3_buckets[each.key].arn}/*"
]
actions = ["s3:*"]
condition {
Expand All @@ -61,10 +66,10 @@ data "aws_iam_policy_document" "draft_documents_put_access" {
}
}

resource "aws_s3_bucket_lifecycle_configuration" "draft_documents" {
count = var.enable_drafts_bucket ? 1 : 0
resource "aws_s3_bucket_lifecycle_configuration" "s3_buckets" {
for_each = var.s3_buckets

bucket = aws_s3_bucket.draft_documents[0].id
bucket = aws_s3_bucket.s3_buckets[each.key].id
rule {
id = "AbortIncompleteUpload"
status = "Enabled"
Expand All @@ -77,10 +82,10 @@ resource "aws_s3_bucket_lifecycle_configuration" "draft_documents" {
}


resource "aws_s3_bucket_server_side_encryption_configuration" "draft_documents_encryption" {
count = var.enable_drafts_bucket ? 1 : 0
resource "aws_s3_bucket_server_side_encryption_configuration" "s3_buckets" {
for_each = var.s3_buckets

bucket = aws_s3_bucket.draft_documents[0].id
bucket = aws_s3_bucket.s3_buckets[each.key].id
rule {
apply_server_side_encryption_by_default {
sse_algorithm = "aws:kms"
Expand All @@ -89,8 +94,8 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "draft_documents_e
}
}

resource "aws_s3_bucket_policy" "draft_documents" {
count = var.enable_drafts_bucket ? 1 : 0
bucket = aws_s3_bucket.draft_documents[0].id
policy = data.aws_iam_policy_document.draft_documents_put_access[0].json
resource "aws_s3_bucket_policy" "s3_buckets" {
for_each = var.s3_buckets
bucket = aws_s3_bucket.s3_buckets[each.key].id
policy = data.aws_iam_policy_document.s3_buckets_put_access[each.key].json
}
12 changes: 12 additions & 0 deletions infra/modules/service/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,18 @@ variable "scheduled_jobs" {
default = {}
}

variable "s3_buckets" {
type = map(object({
env_var = string
public = bool
paths = list(object({
path = string
env_var = string
}))
}))
default = {}
}

variable "enable_drafts_bucket" {
description = "does the service need a private S3 bucket for draft document storage"
type = bool
Expand Down