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

Latest commit converting bucket_id variable from string to list incorrectly reversed the bucket creation check logic #44

Closed
gpapakyriakopoulos opened this issue Feb 26, 2024 · 0 comments · Fixed by #55
Labels
bug 🐛 An issue with the system

Comments

@gpapakyriakopoulos
Copy link
Contributor

Describe the Bug

As described in the title, the latest change in the bucket_id variable that converted it from string to list erroneously introduced reverse logic on the check of s3 bucket creation resulting in creating a new bucket it should not.

The offending line can be seen here

Specifically, the previous logic was written as follows :

create_log_bucket = local.enabled && var.bucket_id == null

Which correctly created a bucket only when the var.bucket_id variable was not populated with an existing bucket ID.

After the latest commit and release, the logic is now as follows :

create_log_bucket = local.enabled && length(var.bucket_id) > 0

This now incorrectly creates a bucket (evaluates the condition as true) when the var.bucket_id list is not empty instead of evaluating to true when the list IS actually empty.

Please amend the logic to avoid inconsistencies with exiting resources when plan/applying.

Expected Behavior

Only create a new s3 bucket when var.bucket_id list is empty

Steps to Reproduce

Attempt to create the module, passing it an existing s3 bucket id as variable

Screenshots

No response

Environment

No response

Additional Context

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 An issue with the system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant