fix bucket acl error (#160)

defenseunicorns · Apr 11, 2023 · 902ea3a · 902ea3a
1 parent caf4b0b
commit 902ea3a
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 9 deletions.
diff --git a/modules/bastion/README.md b/modules/bastion/README.md
@@ -67,6 +67,7 @@ No modules.
 | [aws_s3_bucket_logging.access_logging_on_session_logs_bucket](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_logging) | resource |
 | [aws_s3_bucket_notification.access_log_bucket_notification](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_notification) | resource |
 | [aws_s3_bucket_notification.session_logs_bucket_notification](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_notification) | resource |
+| [aws_s3_bucket_ownership_controls.session_logs_bucket](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_ownership_controls) | resource |
 | [aws_s3_bucket_policy.cloudwatch-s3-policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_policy) | resource |
 | [aws_s3_bucket_public_access_block.access_log_bucket](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block) | resource |
 | [aws_s3_bucket_public_access_block.session_logs_bucket](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block) | resource |

diff --git a/modules/bastion/s3-buckets.tf b/modules/bastion/s3-buckets.tf
@@ -125,6 +125,9 @@ resource "aws_s3_bucket_lifecycle_configuration" "access_log_bucket" {
       days = var.access_log_expire_days
     }
   }
+  depends_on = [
+    aws_s3_bucket_versioning.access_log_bucket
+  ]
 }
 
 resource "aws_s3_bucket_notification" "access_log_bucket_notification" {
@@ -146,10 +149,24 @@ resource "aws_s3_bucket" "session_logs_bucket" {
 
 }
 
+resource "aws_s3_bucket_ownership_controls" "session_logs_bucket" {
+  bucket = aws_s3_bucket.session_logs_bucket.id
+  rule {
+    object_ownership = "BucketOwnerPreferred"
+  }
+  depends_on = [
+    aws_s3_bucket.session_logs_bucket,
+    aws_s3_bucket_public_access_block.session_logs_bucket
+  ]
+}
+
 resource "aws_s3_bucket_acl" "session_logs_bucket" {
   bucket = aws_s3_bucket.session_logs_bucket.id
+  acl    = "private"
 
-  acl = "private"
+  depends_on = [
+    aws_s3_bucket_ownership_controls.session_logs_bucket
+  ]
 }
 
 resource "aws_s3_bucket_versioning" "session_logs_bucket" {
@@ -177,10 +194,6 @@ resource "aws_s3_bucket_public_access_block" "session_logs_bucket" {
   block_public_policy     = true
   ignore_public_acls      = true
   restrict_public_buckets = true
-
-  # The ACL needs to be added before this public access block can be applied. If the public access block gets applied first, then the ACL gets is not able to be added, leading to a race condition.
-  # https://stackoverflow.com/questions/71080354/getting-the-bucket-does-not-allow-acls-error
-  depends_on = [aws_s3_bucket_acl.session_logs_bucket]
 }
 
 resource "aws_s3_bucket_lifecycle_configuration" "session_logs_bucket" {
@@ -199,6 +212,9 @@ resource "aws_s3_bucket_lifecycle_configuration" "session_logs_bucket" {
       days = var.log_expire_days
     }
   }
+  depends_on = [
+    aws_s3_bucket_versioning.session_logs_bucket
+  ]
 }
 
 resource "aws_s3_bucket_notification" "session_logs_bucket_notification" {

diff --git a/test/e2e/examples_complete_secure_test.go b/test/e2e/examples_complete_secure_test.go
@@ -2,14 +2,15 @@ package e2e_test
 
 import (
 	"fmt"
+	"os/exec"
+	"testing"
+	"time"
+
 	"github.com/gruntwork-io/terratest/modules/logger"
 	"github.com/gruntwork-io/terratest/modules/terraform"
 	teststructure "github.com/gruntwork-io/terratest/modules/test-structure"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"os/exec"
-	"testing"
-	"time"
 )
 
 // This test deploys the complete example in "secure mode". Secure mode is:
@@ -26,7 +27,7 @@ func TestExamplesCompleteSecure(t *testing.T) {
 	tempFolder := teststructure.CopyTerraformFolderToTemp(t, "../..", "examples/complete")
 	terraformOptionsNoTargets := &terraform.Options{
 		TerraformDir: tempFolder,
-		Upgrade:      false,
+		Upgrade:      true,
 		VarFiles: []string{
 			"fixtures.common.tfvars",
 			"fixtures.secure.tfvars",