Merge pull request #1272 from gruntwork-io/s3-tests-fix

Update S3 bucket creation to pass ObjectOwnership
gruntwork-io · Apr 25, 2023 · 6c49de6 · 6c49de6
2 parents e0c790c + 926e08b
commit 6c49de6
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 5 deletions.
diff --git a/examples/terraform-aws-s3-example/main.tf b/examples/terraform-aws-s3-example/main.tf
@@ -41,9 +41,18 @@ resource "aws_s3_bucket_versioning" "test_bucket" {
   }
 }
 
-resource "aws_s3_bucket_acl" "test_bucket" {
+resource "aws_s3_bucket_ownership_controls" "test_bucket" {
   bucket = aws_s3_bucket.test_bucket.id
-  acl    = "private"
+  rule {
+    object_ownership = "ObjectWriter"
+  }
+  depends_on = [aws_s3_bucket.test_bucket]
+}
+
+resource "aws_s3_bucket_acl" "test_bucket" {
+  bucket     = aws_s3_bucket.test_bucket.id
+  acl        = "private"
+  depends_on = [aws_s3_bucket_ownership_controls.test_bucket]
 }
 
 
@@ -59,9 +68,18 @@ resource "aws_s3_bucket" "test_bucket_logs" {
   force_destroy = true
 }
 
-resource "aws_s3_bucket_acl" "test_bucket_logs" {
+resource "aws_s3_bucket_ownership_controls" "test_bucket_logs" {
   bucket = aws_s3_bucket.test_bucket_logs.id
-  acl    = "log-delivery-write"
+  rule {
+    object_ownership = "ObjectWriter"
+  }
+  depends_on = [aws_s3_bucket.test_bucket_logs]
+}
+
+resource "aws_s3_bucket_acl" "test_bucket_logs" {
+  bucket     = aws_s3_bucket.test_bucket_logs.id
+  acl        = "log-delivery-write"
+  depends_on = [aws_s3_bucket_ownership_controls.test_bucket_logs]
 }
 
 # Configure bucket access policies

diff --git a/modules/aws/s3.go b/modules/aws/s3.go
@@ -145,6 +145,8 @@ func CreateS3BucketE(t testing.TestingT, region string, name string) error {
 
 	params := &s3.CreateBucketInput{
 		Bucket: aws.String(name),
+		// https://github.com/aws/aws-sdk-go/blob/v1.44.122/service/s3/api.go#L41646
+		ObjectOwnership: aws.String(s3.ObjectOwnershipObjectWriter),
 	}
 	_, err = s3Client.CreateBucket(params)
 	return err

diff --git a/modules/aws/s3_test.go b/modules/aws/s3_test.go
@@ -137,7 +137,7 @@ func TestAssertS3BucketPolicyExists(t *testing.T) {
 	logger.Logf(t, "Random values selected. Region = %s, Id = %s\n", region, id)
 
 	s3BucketName := "gruntwork-terratest-" + strings.ToLower(id)
-	exampleBucketPolicy := fmt.Sprintf(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"AWS":["*"]},"Action":"s3:Get*","Resource":"arn:aws:s3:::%s/*","Condition":{"Bool":{"aws:SecureTransport":"false"}}}]}`, s3BucketName)
+	exampleBucketPolicy := fmt.Sprintf(`{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":{"AWS":["*"]},"Action":"s3:Get*","Resource":"arn:aws:s3:::%s/*","Condition":{"Bool":{"aws:SecureTransport":"false"}}}]}`, s3BucketName)
 
 	CreateS3Bucket(t, region, s3BucketName)
 	defer DeleteS3Bucket(t, region, s3BucketName)