Merge pull request #900 from alphagov/acls-adjusted

Update S3 Bucket ACLs To Match AWS's New Requirements
GovWifi · Sep 6, 2024 · acd44f7 · acd44f7
2 parents 3798259 + 2e89c7c
commit acd44f7
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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" {