-
Notifications
You must be signed in to change notification settings - Fork 1
Logging module for s3 buckets #40
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
# log_bucket | ||
|
||
This module was inspired by https://github.com/jetbrains-infra/terraform-aws-s3-bucket-for-logs | ||
|
||
Note: be sure to use the desired branch/tagged release. | ||
|
||
Usage: | ||
|
||
```terraform | ||
module "log_storage" { | ||
source = "github.com/adhocteam/tf//log_bucket?ref=master" | ||
bucket = "unique-identifier-for-log-storage" | ||
} | ||
|
||
resource "aws_s3_bucket" "example" { | ||
bucket = "unique-identifier-for-s3-bucket" | ||
acl = "public-read" | ||
|
||
policy = <<POLICY | ||
{ | ||
"Version": "2012-10-17", | ||
"Statement": [ | ||
{ | ||
"Effect": "Allow", | ||
"Principal": "*", | ||
"Action": [ | ||
"s3:GetObject" | ||
], | ||
"Resource": [ | ||
"arn:aws:s3:::unique-identifier-for-s3-bucket/*" | ||
] | ||
} | ||
] | ||
} | ||
POLICY | ||
|
||
logging { | ||
target_bucket = "${module.log_storage.bucket_id}" | ||
target_prefix = "${module.log_storage.s3_logs_path}" | ||
} | ||
} | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
resource "aws_s3_bucket" "logs" { | ||
acl = "log-delivery-write" | ||
bucket = "${var.bucket}" | ||
} | ||
|
||
data "aws_iam_policy_document" "logs_bucket_policy" { | ||
|
||
statement { | ||
sid = "Allow LB to write logs" | ||
actions = ["s3:PutObject"] | ||
resources = ["arn:aws:s3:::${aws_s3_bucket.logs.bucket}/${var.alb_logs_path}*"] | ||
principals { | ||
type = "AWS" | ||
identifiers = ["arn:aws:iam::156460612806:root"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are intending to hard code these accounts? I didn't see them in our organization so I'm not sure what they are? |
||
} | ||
} | ||
|
||
statement { | ||
sid = "Permit access log delivery by AWS ID for Log Delivery service" | ||
actions = ["s3:PutObject"] | ||
resources = ["arn:aws:s3:::${aws_s3_bucket.logs.bucket}/${var.s3_logs_path}*"] | ||
principals { | ||
type = "AWS" | ||
identifiers = ["arn:aws:iam::858827067514:root"] | ||
} | ||
} | ||
|
||
statement { | ||
sid = "Permit access log delivery by AWS ID for CloudFront service" | ||
actions = ["s3:PutObject"] | ||
resources = ["arn:aws:s3:::${aws_s3_bucket.logs.bucket}/${var.cdn_logs_path}*"] | ||
principals { | ||
type = "AWS" | ||
identifiers = ["arn:aws:iam::162777425019:root"] | ||
} | ||
} | ||
|
||
} | ||
|
||
resource "aws_s3_bucket_policy" "logs" { | ||
bucket = "${aws_s3_bucket.logs.bucket}" | ||
policy = "${data.aws_iam_policy_document.logs_bucket_policy.json}" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
output "bucket" { | ||
value = "${aws_s3_bucket.logs.bucket_domain_name}" | ||
} | ||
|
||
output "bucket_arn" { | ||
value = "${aws_s3_bucket.logs.arn}" | ||
} | ||
|
||
output "bucket_id" { | ||
value = "${aws_s3_bucket.logs.id}" | ||
} | ||
|
||
output "bucket_domain_name" { | ||
value = "${aws_s3_bucket.logs.bucket_domain_name}" | ||
} | ||
|
||
output "s3_logs_path" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd suggest not passing out the vars as the same. Since the caller passed them in, it already has these values. the more important ones are the stuff you had above where we are passing out "internal' values |
||
value = "${var.s3_logs_path}" | ||
} | ||
|
||
output "alb_logs_path" { | ||
value = "${var.alb_logs_path}" | ||
} | ||
|
||
output "cdn_logs_path" { | ||
value = "${var.cdn_logs_path}" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
variable "cdn_logs_path" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for these can you a |
||
description = "Prefix for CloudFront logs" | ||
default = "cdn/" | ||
} | ||
|
||
variable "alb_logs_path" { | ||
description = "Prefix for ALB logs" | ||
default = "alb/" | ||
} | ||
|
||
variable "s3_logs_path" { | ||
description = "Prefix for S3 access logs" | ||
default = "s3/" | ||
} | ||
|
||
variable "bucket" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since this needs to be unique, i'd suggest you set it inside here instead of relying on the caller do it. In other situations we used domain name + environment name + some identifying suffix like |
||
description = "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.
Are you going to include in this PR updates to the different buckets, CloudFronts, and LBs to point to these?