From 8bf83b03cbcfbebdb1cd025a2dc95dad69ab6af1 Mon Sep 17 00:00:00 2001
From: Brandon Cruz <54559337+brandoncruz3@users.noreply.github.com>
Date: Mon, 6 Nov 2023 15:41:53 -0500
Subject: [PATCH] BFD-2869: Removal of ACLs on S3 terraservices (#2035)

Co-authored-by: Michael J Burling <michael.burling@cms.hhs.gov>
---
 ops/terraform/services/common/s3.tf   | 41 ++++++---------------------
 ops/terraform/services/pipeline/s3.tf | 20 ++-----------
 2 files changed, 11 insertions(+), 50 deletions(-)

diff --git a/ops/terraform/services/common/s3.tf b/ops/terraform/services/common/s3.tf
index bee18d0df1..cf845f6520 100644
--- a/ops/terraform/services/common/s3.tf
+++ b/ops/terraform/services/common/s3.tf
@@ -10,8 +10,7 @@ resource "aws_s3_bucket" "this" {
 
 # block public access to the bucket
 resource "aws_s3_bucket_public_access_block" "this" {
-  bucket = aws_s3_bucket.this.id
-
+  bucket                  = aws_s3_bucket.this.id
   block_public_acls       = true
   block_public_policy     = true
   ignore_public_acls      = true
@@ -29,27 +28,16 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "this" {
 }
 
 resource "aws_s3_bucket_logging" "this" {
-  # TODO: Make this work better for ephemeral environments, etc
   count = local.is_ephemeral_env ? 0 : 1
 
   bucket = aws_s3_bucket.this.id
 
-  # TODO: consider adding this...
-  # expected_bucket_owner = local.account_id
+  expected_bucket_owner = local.account_id
 
   target_bucket = local.logging_bucket
   target_prefix = "${local.legacy_service}_s3_access_logs/"
 }
 
-resource "aws_s3_bucket_acl" "this" {
-  # After April 2023, new S3 Buckets have public access disabled along with ACLs disabled. This
-  # resource will fail to apply for ephemeral environments (new buckets)
-  # FIXME: Replace/resolve this before accepting BFD-2554
-  count = local.is_ephemeral_env ? 0 : 1
-
-  bucket = aws_s3_bucket.this.id
-  acl    = "private"
-}
 
 resource "aws_s3_bucket" "logging" {
   bucket = local.logging_bucket
@@ -59,32 +47,21 @@ resource "aws_s3_bucket" "logging" {
   }
 }
 
-resource "aws_s3_bucket_server_side_encryption_configuration" "logging" {
-  bucket = aws_s3_bucket.logging.id
-  rule {
-    apply_server_side_encryption_by_default {
-      sse_algorithm = "AES256"
-    }
-  }
-}
-
 resource "aws_s3_bucket_public_access_block" "logging" {
-  bucket = aws_s3_bucket.logging.id
-
+  bucket                  = aws_s3_bucket.logging.id
   block_public_acls       = true
   block_public_policy     = true
   ignore_public_acls      = true
   restrict_public_buckets = true
 }
 
-resource "aws_s3_bucket_acl" "logging" {
-  # After April 2023, new S3 Buckets have public access disabled along with ACLs disabled. This
-  # resource will fail to apply for ephemeral environments
-  # TODO: Replace/resolve this in BFD-2554
-  count = local.is_ephemeral_env ? 0 : 1
-
-  acl    = "log-delivery-write"
+resource "aws_s3_bucket_server_side_encryption_configuration" "logging" {
   bucket = aws_s3_bucket.logging.id
+  rule {
+    apply_server_side_encryption_by_default {
+      sse_algorithm = "AES256"
+    }
+  }
 }
 
 resource "aws_s3_bucket_policy" "logging" {
diff --git a/ops/terraform/services/pipeline/s3.tf b/ops/terraform/services/pipeline/s3.tf
index 2999d744ca..f563a72992 100644
--- a/ops/terraform/services/pipeline/s3.tf
+++ b/ops/terraform/services/pipeline/s3.tf
@@ -12,8 +12,7 @@ resource "aws_s3_bucket" "this" {
 
 # block public access to the bucket
 resource "aws_s3_bucket_public_access_block" "this" {
-  bucket = aws_s3_bucket.this.id
-
+  bucket                  = aws_s3_bucket.this.id
   block_public_acls       = true
   block_public_policy     = true
   ignore_public_acls      = true
@@ -31,26 +30,11 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "this" {
 }
 
 resource "aws_s3_bucket_logging" "this" {
-  # After April 2023, new S3 Buckets have public access disabled along with ACLs disabled. This
-  # resource will fail to apply for ephemeral environments (new buckets)
-  # FIXME: Replace/resolve this before accepting BFD-2554
   count = local.is_ephemeral_env ? 0 : 1
 
   bucket        = aws_s3_bucket.this.id
   target_bucket = local.logging_bucket
-
-  # TODO: correct the target prefix by adding a trailing '/'
-  target_prefix = "${local.legacy_service}_s3_access_logs"
-}
-
-resource "aws_s3_bucket_acl" "this" {
-  # After April 2023, new S3 Buckets have public access disabled along with ACLs disabled. This
-  # resource will fail to apply for ephemeral environments
-  # FIXME: Replace/resolve this after accepting BFD-2554
-  count = local.is_ephemeral_env ? 0 : 1
-
-  bucket = aws_s3_bucket.this.id
-  acl    = "private"
+  target_prefix = "${local.legacy_service}_s3_access_logs/"
 }
 
 resource "aws_s3_bucket_notification" "etl_bucket_notifications" {