Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Race Condition between the Creation of the S3 Bucket Policy and the CloudTrail Trail #91

Merged
merged 3 commits into from Apr 15, 2024

Conversation

X-Guardian
Copy link
Contributor

what

Fix the race condition between the creation of the S3 Bucket policy and the CloudTrail trail by adding a depends_on argument to the bucket_id output which is used as input to the CloudTrail module. This ensures that all the resources in the CloudTrail S3 Bucket module, including the S3 Bucket Policy have been created before the CloudTrail trail is created.

The example used for the tests has also been updated to include the creation of the CloudTrail Trail to verify that this is working.

why

@X-Guardian X-Guardian requested review from a team as code owners April 12, 2024 10:34
@mergify mergify bot added the triage Needs triage label Apr 12, 2024
@jamengual
Copy link
Contributor

/terratest

@jamengual
Copy link
Contributor

/terratest

@jamengual
Copy link
Contributor

What version of terraform are you using?
Usually, depends_on in outputs is not used, so I just want to make sure it is ok with other CP reviewers.

@X-Guardian
Copy link
Contributor Author

The Terraform version is not relevant. It is the AWS CloudTrail CreateTrail API that is checking the S3 Bucket Policy when creating the trail. Without the depends_on on the output this check happens before the S3 Bucket Policy has been applied.

outputs.tf Outdated Show resolved Hide resolved
examples/complete/main.tf Show resolved Hide resolved
@Nuru
Copy link
Sponsor Contributor

Nuru commented Apr 15, 2024

/terratest

@Nuru Nuru added bugfix Change that restores intended behavior and removed triage Needs triage labels Apr 15, 2024
Copy link
Sponsor Contributor

@Nuru Nuru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am uncomfortable allowing the use of an undocumented feature, and this should probably be fixed upstream in terraform-aws-s3-bucket, but it works with Terraform 1.3, 1.5, and 1.8 and I don't see a better way to achieve the desired result, so I will allow this and we can see how it goes.

@Nuru Nuru merged commit 91595d9 into cloudposse:main Apr 15, 2024
27 checks passed
@X-Guardian
Copy link
Contributor Author

X-Guardian commented Apr 15, 2024

Thank for merging this @Nuru. If you are talking about the depends_on feature for an entire module being undocumented, it is documented here: https://developer.hashicorp.com/terraform/language/meta-arguments/depends_on.

When the dependency object is an entire module, depends_on affects the order in which Terraform processes all of the resources and data sources associated with that module.

And depends_on for outputs is documented here: https://developer.hashicorp.com/terraform/language/values/outputs#depends_on-explicit-output-dependencies

@X-Guardian X-Guardian deleted the bucket-policy-race-condition-fix branch April 15, 2024 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Change that restores intended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Race Condition between the Creation of the S3 Bucket Policy and the CloudTrail Trail
3 participants