-
Notifications
You must be signed in to change notification settings - Fork 0
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
double instance #24
base: main
Are you sure you want to change the base?
double instance #24
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about explaining the logs and queries options in the README ?
bucket = coalesce(var.logs_bucket_name_override, "aws-waf-logs-${var.waf_name}-${data.aws_caller_identity.current.account_id}") | ||
force_destroy = true | ||
} | ||
|
||
# See issue <https://github.com/hashicorp/terraform-provider-aws/issues/28353> | ||
resource "aws_s3_bucket_ownership_controls" "logs" { | ||
bucket = aws_s3_bucket.logs.id | ||
bucket = var.deploy_logs ? aws_s3_bucket.logs[0].id : var.alternative_logs_bucket_name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the feeling it's not the right way to do. If the bucket is not managed (because var.deploy_logs = false), then all the resource "aws_s3_bucket_*" should not be managed either, so you have to put a count. Example:
bucket = var.deploy_logs ? aws_s3_bucket.logs[0].id : var.alternative_logs_bucket_name | |
count = var.deploy_logs ? 1 : 0 | |
bucket = aws_s3_bucket.logs[0].id |
and this for:
- aws_s3_bucket_ownership_controls
- aws_s3_bucket_acl
- aws_s3_bucket_lifecycle_configuration
Instead of having athena in a submodule, and since athema queries are tied to the logs bucket, I simplified a bit the logic and deploy the athena whenever we deploy the logs bucket. I think that kind of makes sense.
I tested in sandbox a successful parallel deployment with shared bucket/athena queries:
https://github.com/tx-pts-dai/sandbox-tests/pull/175