From aa5224c5ba3e1b54f2cb2a1de035845a77274c9d Mon Sep 17 00:00:00 2001 From: Simon Heather Date: Fri, 12 Apr 2024 11:27:54 +0100 Subject: [PATCH 1/3] Fix S3 Bucket Policy Race Condition --- examples/complete/fixtures.us-east-2.tfvars | 10 ++++++++++ examples/complete/main.tf | 14 ++++++++++++++ outputs.tf | 6 +++++- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/examples/complete/fixtures.us-east-2.tfvars b/examples/complete/fixtures.us-east-2.tfvars index ee0e5ce..63a90fd 100644 --- a/examples/complete/fixtures.us-east-2.tfvars +++ b/examples/complete/fixtures.us-east-2.tfvars @@ -5,3 +5,13 @@ namespace = "eg" stage = "test" name = "cloudtrail-s3-bucket" + +enable_log_file_validation = true + +is_multi_region_trail = false + +include_global_service_events = false + +enable_logging = true + +is_organization_trail = false diff --git a/examples/complete/main.tf b/examples/complete/main.tf index c8a3517..9a7c215 100644 --- a/examples/complete/main.tf +++ b/examples/complete/main.tf @@ -10,3 +10,17 @@ module "cloudtrail_s3_bucket" { context = module.this.context } + +module "cloudtrail" { + source = "cloudposse/cloudtrail/aws" + version = "0.23.0" + + enable_logging = var.enable_logging + enable_log_file_validation = var.enable_log_file_validation + include_global_service_events = var.include_global_service_events + is_multi_region_trail = var.is_multi_region_trail + is_organization_trail = var.is_organization_trail + s3_bucket_name = module.cloudtrail_s3_bucket.bucket_id + + context = module.this.context +} diff --git a/outputs.tf b/outputs.tf index dad425c..a359ff0 100644 --- a/outputs.tf +++ b/outputs.tf @@ -6,6 +6,10 @@ output "bucket_domain_name" { output "bucket_id" { value = module.s3_bucket.bucket_id description = "Bucket ID" + depends_on = [ + # The S3 bucket policy resource must be created before the bucket is referenced by CloudTrail + module.s3_bucket + ] } output "bucket_arn" { @@ -21,4 +25,4 @@ output "prefix" { output "bucket_notifications_sqs_queue_arn" { value = module.s3_bucket.bucket_notifications_sqs_queue_arn description = "Notifications SQS queue ARN" -} \ No newline at end of file +} From 4905c5c8287d3156213f57e676083f5665c50c29 Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Fri, 12 Apr 2024 17:05:25 +0100 Subject: [PATCH 2/3] refine tests --- examples/complete/fixtures.us-east-2.tfvars | 10 ---------- examples/complete/main.tf | 8 ++------ 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/examples/complete/fixtures.us-east-2.tfvars b/examples/complete/fixtures.us-east-2.tfvars index 63a90fd..ee0e5ce 100644 --- a/examples/complete/fixtures.us-east-2.tfvars +++ b/examples/complete/fixtures.us-east-2.tfvars @@ -5,13 +5,3 @@ namespace = "eg" stage = "test" name = "cloudtrail-s3-bucket" - -enable_log_file_validation = true - -is_multi_region_trail = false - -include_global_service_events = false - -enable_logging = true - -is_organization_trail = false diff --git a/examples/complete/main.tf b/examples/complete/main.tf index 9a7c215..b66e619 100644 --- a/examples/complete/main.tf +++ b/examples/complete/main.tf @@ -15,12 +15,8 @@ module "cloudtrail" { source = "cloudposse/cloudtrail/aws" version = "0.23.0" - enable_logging = var.enable_logging - enable_log_file_validation = var.enable_log_file_validation - include_global_service_events = var.include_global_service_events - is_multi_region_trail = var.is_multi_region_trail - is_organization_trail = var.is_organization_trail - s3_bucket_name = module.cloudtrail_s3_bucket.bucket_id + is_multi_region_trail = false + s3_bucket_name = module.cloudtrail_s3_bucket.bucket_id context = module.this.context } From 23178f34820ccdf50ac1873b02765fa5945c8c97 Mon Sep 17 00:00:00 2001 From: Nuru Date: Mon, 15 Apr 2024 11:17:41 -0700 Subject: [PATCH 3/3] Provide explanation of use of undocumented feature --- outputs.tf | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/outputs.tf b/outputs.tf index a359ff0..8087836 100644 --- a/outputs.tf +++ b/outputs.tf @@ -6,8 +6,19 @@ output "bucket_domain_name" { output "bucket_id" { value = module.s3_bucket.bucket_id description = "Bucket ID" + # + # Ensure the bucket is fully configured before allowing any use of `bucket_id`. + # + # Although undocumented, `depends_on` is allowed in an output block. + # The `bucket_id` is available before the bucket is fully configured + # with policies, versioning, lifecycle, etc. However, all that + # needs to happen before the bucket can be used as a destination for + # a CloudTrail. While the documented way to ensure this would be + # for the user of this module to add a `depends_on` that depends + # on this module, since this is such a common need, and since + # we can depend on a submodule here rather than this entire module, + # we add the `depends_on` block here. depends_on = [ - # The S3 bucket policy resource must be created before the bucket is referenced by CloudTrail module.s3_bucket ] }