From 2e89c7cd7971362f7059aac1cd3586c757ae0fed Mon Sep 17 00:00:00 2001 From: Camille Descartes Date: Tue, 3 Sep 2024 12:13:22 +0100 Subject: [PATCH] Update S3 Bucket ACLs To Match AWS's New Requirements Since we coded our S3 buckets in terraform, AWS has made some changes to S3 ACLs. The way old way we had S3 ACLs configured caused errors when creating a new environment. The full summary of this change is available by Hashicorp on github: https://github.com/hashicorp/terraform-provider-aws/issues/28353 Here is a summary from Hashicorp: "The impending S3 security changes will not require breaking changes to v3 or v4 of the Terraform AWS provider. However, some previously valid S3 bucket ACL configurations will begin returning errors for net-new buckets. Existing buckets (and their corresponding terraform configuration) are not impacted." Jira: https://technologyprogramme.atlassian.net/jira/software/projects/GW/boards/251?assignee=5e45277d29e6d00c970616c6&selectedIssue=GW-1471 --- govwifi-admin/s3.tf | 10 ++++++++++ govwifi-dashboard/s3.tf | 15 +++------------ govwifi-deploy/s3.tf | 11 ++++++++++- govwifi-smoke-tests/s3.tf | 10 ++++++++++ new-terraform-state/accesslogs_bucket.tf | 10 ++++++++++ terraform-state/accesslogs.tf | 10 ++++++++++ 6 files changed, 53 insertions(+), 13 deletions(-) diff --git a/govwifi-admin/s3.tf b/govwifi-admin/s3.tf index dcec3d41..36f01a07 100644 --- a/govwifi-admin/s3.tf +++ b/govwifi-admin/s3.tf @@ -35,9 +35,19 @@ resource "aws_s3_bucket" "product_page_data_bucket" { } } +resource "aws_s3_bucket_ownership_controls" "product_page_data_bucket" { + bucket = aws_s3_bucket.product_page_data_bucket.id + rule { + object_ownership = "BucketOwnerPreferred" + } +} + resource "aws_s3_bucket_acl" "product_page_data_bucket" { bucket = aws_s3_bucket.product_page_data_bucket.id acl = "public-read" + + depends_on = [aws_s3_bucket_ownership_controls.product_page_data_bucket] + } resource "aws_s3_bucket_versioning" "product_page_data_bucket" { diff --git a/govwifi-dashboard/s3.tf b/govwifi-dashboard/s3.tf index 6543c215..9b9969c4 100644 --- a/govwifi-dashboard/s3.tf +++ b/govwifi-dashboard/s3.tf @@ -39,18 +39,9 @@ resource "aws_s3_bucket_versioning" "export_data_bucket" { } } -resource "aws_s3_bucket_policy" "export_data_bucket" { +resource "aws_s3_bucket_public_access_block" "export_data_bucket" { bucket = aws_s3_bucket.export_data_bucket.id - policy = jsonencode({ - "Version" : "2012-10-17", - "Id" : "ExportDataBucketToPublic", - "Statement" : [ - { - "Sid" : "AllowPublicAccessToExportDataBucket", - "Effect" : "Allow", - "Principal" : "*", - "Action" : "s3:GetObject", - "Resource" : "${aws_s3_bucket.export_data_bucket.arn}/*" - }] }) + block_public_acls = false + block_public_policy = false } diff --git a/govwifi-deploy/s3.tf b/govwifi-deploy/s3.tf index a5e3d94e..cf82cc4b 100644 --- a/govwifi-deploy/s3.tf +++ b/govwifi-deploy/s3.tf @@ -2,9 +2,18 @@ resource "aws_s3_bucket" "codepipeline_bucket" { bucket = "govwifi-codepipeline-bucket" } +resource "aws_s3_bucket_ownership_controls" "codepipeline_bucket" { + bucket = aws_s3_bucket.codepipeline_bucket.id + rule { + object_ownership = "BucketOwnerPreferred" + } +} + resource "aws_s3_bucket_acl" "codepipeline_bucket_acl" { bucket = aws_s3_bucket.codepipeline_bucket.id acl = "private" + + depends_on = [aws_s3_bucket_ownership_controls.codepipeline_bucket] } resource "aws_s3_bucket_public_access_block" "codepipeline_bucket" { @@ -25,7 +34,7 @@ resource "aws_s3_bucket_versioning" "source" { # Push S3 notifications to EventBridge resource "aws_s3_bucket_notification" "codepipeline_bucket" { - bucket = aws_s3_bucket.codepipeline_bucket.id + bucket = aws_s3_bucket.codepipeline_bucket.id eventbridge = true } diff --git a/govwifi-smoke-tests/s3.tf b/govwifi-smoke-tests/s3.tf index 1fa3d32e..2ce59ceb 100644 --- a/govwifi-smoke-tests/s3.tf +++ b/govwifi-smoke-tests/s3.tf @@ -70,7 +70,17 @@ POLICY } +# Resource to avoid error "AccessControlListNotSupported: The bucket does not allow ACLs" +resource "aws_s3_bucket_ownership_controls" "smoke_tests_bucket_acl" { + bucket = aws_s3_bucket.smoke_tests_bucket.id + rule { + object_ownership = "BucketOwnerPreferred" + } +} + resource "aws_s3_bucket_acl" "smoke_tests_bucket_acl" { bucket = aws_s3_bucket.smoke_tests_bucket.id acl = "private" + + depends_on = [aws_s3_bucket_ownership_controls.smoke_tests_bucket_acl] } diff --git a/new-terraform-state/accesslogs_bucket.tf b/new-terraform-state/accesslogs_bucket.tf index b41924b9..f75a5a79 100644 --- a/new-terraform-state/accesslogs_bucket.tf +++ b/new-terraform-state/accesslogs_bucket.tf @@ -118,9 +118,19 @@ resource "aws_s3_bucket_replication_configuration" "accesslogs_replication" { } } +resource "aws_s3_bucket_ownership_controls" "accesslogs_bucket" { + bucket = aws_s3_bucket.accesslogs_bucket.id + rule { + object_ownership = "BucketOwnerPreferred" + } +} + + resource "aws_s3_bucket_acl" "accesslogs_bucket" { bucket = aws_s3_bucket.accesslogs_bucket.id acl = "log-delivery-write" + + depends_on = [aws_s3_bucket_ownership_controls.accesslogs_bucket] } resource "aws_s3_bucket_versioning" "accesslogs_bucket" { diff --git a/terraform-state/accesslogs.tf b/terraform-state/accesslogs.tf index df11bc5e..b8f71bf9 100644 --- a/terraform-state/accesslogs.tf +++ b/terraform-state/accesslogs.tf @@ -118,9 +118,19 @@ resource "aws_s3_bucket_replication_configuration" "accesslogs_bucket" { } } +resource "aws_s3_bucket_ownership_controls" "accesslogs_bucket" { + bucket = aws_s3_bucket.accesslogs_bucket.id + rule { + object_ownership = "BucketOwnerPreferred" + } +} + resource "aws_s3_bucket_acl" "accesslogs_bucket" { bucket = aws_s3_bucket.accesslogs_bucket.id acl = "log-delivery-write" + + depends_on = [aws_s3_bucket_ownership_controls.accesslogs_bucket] + } resource "aws_s3_bucket_versioning" "accesslogs_bucket" {