From 69e236c2a49f989a4661e1f8f9b67be30f50193d Mon Sep 17 00:00:00 2001 From: bleachbyte Date: Tue, 18 Apr 2023 18:58:32 -0500 Subject: [PATCH 1/3] add BucketOwnerPreferred ownership controls to buckets with aws_s3_bucket_acl resources --- elb_access_logs_bucket/main.tf | 10 ++++++++++ git2s3_artifacts/main.tf | 11 +++++++++++ guardduty/main.tf | 15 +++++++++++++++ s3_bucket_block/main.tf | 11 +++++++++++ slo_lambda/main.tf | 8 ++++---- ssm/main.tf | 10 ++++++++++ state_bucket/main.tf | 25 +++++++++++++++++++++++-- 7 files changed, 84 insertions(+), 6 deletions(-) diff --git a/elb_access_logs_bucket/main.tf b/elb_access_logs_bucket/main.tf index 96e6041a..737a0950 100644 --- a/elb_access_logs_bucket/main.tf +++ b/elb_access_logs_bucket/main.tf @@ -62,9 +62,19 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "logs" { } } +resource "aws_s3_bucket_ownership_controls" "logs" { + bucket = aws_s3_bucket.logs.id + + rule { + object_ownership = "BucketOwnerPreferred" + } +} + resource "aws_s3_bucket_acl" "logs" { bucket = aws_s3_bucket.logs.id acl = "log-delivery-write" + + depends_on = [aws_s3_bucket_ownership_controls.logs] } resource "aws_s3_bucket_policy" "logs" { diff --git a/git2s3_artifacts/main.tf b/git2s3_artifacts/main.tf index d1106257..32f506d1 100644 --- a/git2s3_artifacts/main.tf +++ b/git2s3_artifacts/main.tf @@ -91,11 +91,22 @@ resource "aws_s3_bucket" "artifact_bucket" { } } +resource "aws_s3_bucket_ownership_controls" "artifact_bucket" { + bucket = aws_s3_bucket.artifact_bucket.id + + rule { + object_ownership = "BucketOwnerPreferred" + } +} + resource "aws_s3_bucket_acl" "artifact_bucket" { bucket = aws_s3_bucket.artifact_bucket.id acl = "private" + + depends_on = [aws_s3_bucket_ownership_controls.artifact_bucket] } + resource "aws_s3_bucket_server_side_encryption_configuration" "artifact_bucket" { bucket = aws_s3_bucket.artifact_bucket.id diff --git a/guardduty/main.tf b/guardduty/main.tf index 74e7fa15..280a3d80 100644 --- a/guardduty/main.tf +++ b/guardduty/main.tf @@ -211,6 +211,21 @@ resource "aws_s3_bucket_acl" "guardduty" { acl = "private" } +resource "aws_s3_bucket_ownership_controls" "guardduty" { + bucket = aws_s3_bucket.guardduty.id + + rule { + object_ownership = "BucketOwnerPreferred" + } +} + +resource "aws_s3_bucket_acl" "guardduty" { + bucket = aws_s3_bucket.guardduty.id + acl = "private" + + depends_on = [aws_s3_bucket_ownership_controls.guardduty] +} + resource "aws_s3_bucket_policy" "guardduty" { bucket = aws_s3_bucket.guardduty.id policy = data.aws_iam_policy_document.guardduty_s3.json diff --git a/s3_bucket_block/main.tf b/s3_bucket_block/main.tf index 2a37ab74..8e38a249 100644 --- a/s3_bucket_block/main.tf +++ b/s3_bucket_block/main.tf @@ -44,10 +44,21 @@ resource "aws_s3_bucket" "bucket" { force_destroy = lookup(each.value, "force_destroy", true) } +resource "aws_s3_bucket_ownership_controls" "bucket" { + for_each = var.bucket_data + bucket = aws_s3_bucket.bucket[each.key] + + rule { + object_ownership = "BucketOwnerPreferred" + } +} + resource "aws_s3_bucket_acl" "bucket" { for_each = var.bucket_data bucket = aws_s3_bucket.bucket[each.key] acl = lookup(each.value, "acl", "private") + + depends_on = [aws_s3_bucket_ownership_controls.bucket[each.key]] } resource "aws_s3_bucket_policy" "bucket" { diff --git a/slo_lambda/main.tf b/slo_lambda/main.tf index 546ca20e..c8bdbf57 100644 --- a/slo_lambda/main.tf +++ b/slo_lambda/main.tf @@ -141,11 +141,11 @@ resource "aws_cloudwatch_dashboard" "sli" { ] "region" : data.aws_region.current.name "title" : "${v.description != null ? v.description : k} over last ${var.window_days} days" - "stat": "Average" + "stat" : "Average" "period" : 24 * 60 * 60 - "yAxis": { - "left": { - "showUnits": false + "yAxis" : { + "left" : { + "showUnits" : false } } } diff --git a/ssm/main.tf b/ssm/main.tf index 691b045a..92be8deb 100644 --- a/ssm/main.tf +++ b/ssm/main.tf @@ -102,9 +102,19 @@ resource "aws_s3_bucket_versioning" "ssm_logs" { } } +resource "aws_s3_bucket_ownership_controls" "ssm_logs" { + bucket = aws_s3_bucket.ssm_logs.id + + rule { + object_ownership = "BucketOwnerPreferred" + } +} + resource "aws_s3_bucket_acl" "ssm_logs" { bucket = aws_s3_bucket.ssm_logs.id acl = "private" + + depends_on = [aws_s3_bucket_ownership_controls.ssm_logs] } resource "aws_s3_bucket_lifecycle_configuration" "ssm_logs" { diff --git a/state_bucket/main.tf b/state_bucket/main.tf index f0ea8783..9de970b7 100644 --- a/state_bucket/main.tf +++ b/state_bucket/main.tf @@ -97,9 +97,19 @@ resource "aws_s3_bucket_versioning" "s3-access-logs" { } } +resource "aws_s3_bucket_ownership_controls" "s3-access-logs" { + bucket = aws_s3_bucket.s3-access-logs.id + + rule { + object_ownership = "BucketOwnerPreferred" + } +} + resource "aws_s3_bucket_acl" "s3-access-logs" { bucket = aws_s3_bucket.s3-access-logs.id acl = "log-delivery-write" + + depends_on = [aws_s3_bucket_ownership_controls.s3-access-logs] } resource "aws_s3_bucket_lifecycle_configuration" "s3-access-logs" { @@ -154,10 +164,21 @@ resource "aws_s3_bucket_versioning" "tf-state" { } } +resource "aws_s3_bucket_ownership_controls" "tf-state" { + count = var.remote_state_enabled + bucket = data.aws_s3_bucket.tf-state[count.index].id + + rule { + object_ownership = "BucketOwnerPreferred" + } +} + resource "aws_s3_bucket_acl" "tf-state" { count = var.remote_state_enabled bucket = data.aws_s3_bucket.tf-state[count.index].id acl = "private" + + depends_on = [aws_s3_bucket_ownership_controls.tf-state[count.index]] } resource "aws_s3_bucket_logging" "tf-state" { @@ -214,8 +235,8 @@ resource "aws_s3_bucket_public_access_block" "inventory" { } module "s3_config" { - for_each = var.remote_state_enabled == 1 ? toset(["s3-access-logs", "tf-state"]) : toset(["s3-access-logs"]) - source = "github.com/18F/identity-terraform//s3_config?ref=91f5c8a84c664fc5116ef970a5896c2edadff2b1" + for_each = var.remote_state_enabled == 1 ? toset(["s3-access-logs", "tf-state"]) : toset(["s3-access-logs"]) + source = "github.com/18F/identity-terraform//s3_config?ref=91f5c8a84c664fc5116ef970a5896c2edadff2b1" #source = "../s3_config" depends_on = [aws_s3_bucket.s3-access-logs] From c1d2729d18dd16e18a4fc12405e1692bfb5022b0 Mon Sep 17 00:00:00 2001 From: bleachbyte Date: Tue, 18 Apr 2023 19:16:35 -0500 Subject: [PATCH 2/3] remove duplicate acl entry --- guardduty/main.tf | 5 ----- 1 file changed, 5 deletions(-) diff --git a/guardduty/main.tf b/guardduty/main.tf index 280a3d80..8173efca 100644 --- a/guardduty/main.tf +++ b/guardduty/main.tf @@ -206,11 +206,6 @@ resource "aws_s3_bucket" "guardduty" { force_destroy = true } -resource "aws_s3_bucket_acl" "guardduty" { - bucket = aws_s3_bucket.guardduty.id - acl = "private" -} - resource "aws_s3_bucket_ownership_controls" "guardduty" { bucket = aws_s3_bucket.guardduty.id From 86342a4cc1f62b4e493c171017315c75efe42509 Mon Sep 17 00:00:00 2001 From: bleachbyte Date: Tue, 18 Apr 2023 19:26:53 -0500 Subject: [PATCH 3/3] don't use iterables for depends_on aws_s3_bucket_ownership_controls --- s3_bucket_block/main.tf | 2 +- state_bucket/main.tf | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/s3_bucket_block/main.tf b/s3_bucket_block/main.tf index 8e38a249..4b8145a4 100644 --- a/s3_bucket_block/main.tf +++ b/s3_bucket_block/main.tf @@ -58,7 +58,7 @@ resource "aws_s3_bucket_acl" "bucket" { bucket = aws_s3_bucket.bucket[each.key] acl = lookup(each.value, "acl", "private") - depends_on = [aws_s3_bucket_ownership_controls.bucket[each.key]] + depends_on = [aws_s3_bucket_ownership_controls.bucket] } resource "aws_s3_bucket_policy" "bucket" { diff --git a/state_bucket/main.tf b/state_bucket/main.tf index 9de970b7..06a2aef5 100644 --- a/state_bucket/main.tf +++ b/state_bucket/main.tf @@ -178,7 +178,7 @@ resource "aws_s3_bucket_acl" "tf-state" { bucket = data.aws_s3_bucket.tf-state[count.index].id acl = "private" - depends_on = [aws_s3_bucket_ownership_controls.tf-state[count.index]] + depends_on = [aws_s3_bucket_ownership_controls.tf-state] } resource "aws_s3_bucket_logging" "tf-state" {