Skip to content
This repository has been archived by the owner on Apr 18, 2022. It is now read-only.

Logging module for s3 buckets #40

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions log_bucket/README.md
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
Copy link
Contributor

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?

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}"
}
}
```
43 changes: 43 additions & 0 deletions log_bucket/main.tf
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"]
Copy link
Contributor

Choose a reason for hiding this comment

The 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}"
}
27 changes: 27 additions & 0 deletions log_bucket/outputs.tf
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" {
Copy link
Contributor

Choose a reason for hiding this comment

The 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}"
}
18 changes: 18 additions & 0 deletions log_bucket/variables.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
variable "cdn_logs_path" {
Copy link
Contributor

Choose a reason for hiding this comment

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

for these can you a type = string that helps with Terraform 0.12+ to allow type checking of input variables. You get better error messages

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" {
Copy link
Contributor

Choose a reason for hiding this comment

The 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 adhoc.team-shared-logs or something. But the details are up to you

description = "Bucket name"
}