-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
feat: Add elb_log_delivery_policy_source_organizations
variable
#330
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
base: master
Are you sure you want to change the base?
Conversation
e411b02
to
adb163d
Compare
* New `elb_log_delivery_policy_source_organizations` variable allows scoping access to the S3 bucket to AWS organization. * Improve security by scoping ALB/NLB/VPC flow logs access to the caller's AWS account. * Improve variable description. According to AWS documentation, the same S3 permissions can be used for both NLB access logs and VPC flow logs. References: * https://docs.aws.amazon.com/vpc/latest/userguide/flow-logs-s3-permissions.html * https://docs.aws.amazon.com/elasticloadbalancing/latest/network/enable-access-logs.html * https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-access-logging.html Fix terraform-aws-modules#324 Signed-off-by: szubersk <[email protected]>
adb163d
to
4570fd3
Compare
@@ -622,7 +622,7 @@ data "aws_iam_policy_document" "elb_log_delivery" { | |||
for_each = { for k, v in local.elb_service_accounts : k => v if k == data.aws_region.current.name } | |||
|
|||
content { | |||
sid = format("ELBRegion%s", title(statement.key)) |
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.
this is a breaking change and should not be modified at this time
@@ -5,13 +5,13 @@ variable "create_bucket" { | |||
} | |||
|
|||
variable "attach_elb_log_delivery_policy" { | |||
description = "Controls if S3 bucket should have ELB log delivery policy attached" |
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.
why are there all of these changes? ELB is correct - ALB and NLB are in fact ELB v2 (classic load balancer was ELB v1)
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.
it looks like we are trying to do too much in this PR and I'm not quite following, nor do I agree with the name changes (i.e. ELB
to ALB
, etc.)
Description
New
elb_log_delivery_policy_source_organizations
variable allows scoping access to the S3 bucket to AWS organization.Improve security by scoping ALB/NLB/VPC flow logs access to the caller's AWS account.
Improve variable description. According to AWS documentation, the same S3 permissions can be used for both NLB access logs and VPC flow logs.
References:
https://docs.aws.amazon.com/vpc/latest/userguide/flow-logs-s3-permissions.html
https://docs.aws.amazon.com/elasticloadbalancing/latest/network/enable-access-logs.html
https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-access-logging.html
Motivation and Context
Fix #324
Breaking Changes
N/A
How Has This Been Tested?
examples/*
to demonstrate and validate my change(s)examples/*
projectspre-commit run -a
on my pull request