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

s3 module terraform warnings refactor #302

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
139 changes: 82 additions & 57 deletions s3/main.tf
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
resource "aws_s3_bucket" "log_bucket" {
count = var.access_logs == true && var.apply_resource == true ? 1 : 0
acl = "log-delivery-write"
bucket = "${local.bucket_name}-logs"
force_destroy = var.force_destroy

versioning {
enabled = true
}

tags = merge(
var.common_tags,
tomap(
Expand All @@ -16,6 +11,23 @@ resource "aws_s3_bucket" "log_bucket" {
)
}

resource "aws_s3_bucket_versioning" "log_bucket_versioning" {
count = var.access_logs == true && var.apply_resource == true ? 1 : 0

bucket = aws_s3_bucket.log_bucket[0].id

versioning_configuration {
status = "Enabled"
}
}

resource "aws_s3_bucket_acl" "log_bucket_acl" {
count = var.access_logs == true && var.apply_resource == true ? 1 : 0

bucket = aws_s3_bucket.log_bucket[count.index].id
acl = "log-delivery-write"
}

resource "aws_s3_bucket_server_side_encryption_configuration" "bucket_encryption" {
bucket = local.s3_bucket_id

Expand All @@ -39,9 +51,13 @@ resource "aws_s3_bucket_public_access_block" "log_bucket" {
}

resource "aws_s3_bucket_policy" "log_bucket" {
count = var.access_logs == true && var.apply_resource == true ? 1 : 0
bucket = aws_s3_bucket.log_bucket.*.id[0]
policy = templatefile("./tdr-terraform-modules/s3/templates/secure_transport.json.tpl", { bucket_name = aws_s3_bucket.log_bucket.*.id[0] })
count = var.access_logs == true && var.apply_resource == true ? 1 : 0
bucket = aws_s3_bucket.log_bucket.*.id[0]
policy = templatefile("./tdr-terraform-modules/s3/templates/secure_transport.json.tpl",
{
bucket_name = aws_s3_bucket.log_bucket.*.id[0],
canonical_user_grants = jsonencode(var.canonical_user_grants)
})
depends_on = [aws_s3_bucket_public_access_block.log_bucket]
}

Expand All @@ -55,76 +71,84 @@ resource "aws_s3_bucket_notification" "log_bucket_notification" {
}
depends_on = [aws_s3_bucket_policy.log_bucket]
}

# This module is to be deprecated and caused many terraform warnings.
# Attributes have been removed and added as resources.
# The grants are now passed as a variable to the bucket policy template.
resource "aws_s3_bucket" "bucket" {
count = var.apply_resource == true ? 1 : 0
bucket = local.bucket_name
acl = length(var.canonical_user_grants) == 0 ? var.acl : null
force_destroy = var.force_destroy
tags = merge(
var.common_tags,
tomap(
{ "Name" = local.bucket_name }
)
)
}

dynamic "grant" {
for_each = var.canonical_user_grants
content {
permissions = grant.value.permissions
type = "CanonicalUser"
id = grant.value.id
}
}
resource "aws_s3_bucket_versioning" "bucket_versioning" {
count = var.apply_resource == true && length(var.canonical_user_grants) == 0 ? 1 : 0

versioning {
enabled = var.versioning
}
bucket = aws_s3_bucket.bucket[0].id

dynamic "lifecycle_rule" {
for_each = var.abort_incomplete_uploads == true ? ["include_block"] : []
content {
id = "abort-incomplete-uploads"
enabled = true
abort_incomplete_multipart_upload_days = 7
expiration {
days = 0
expired_object_delete_marker = false
}
}
versioning_configuration {
status = "Enabled" # Use "Suspended" to disable versioning
}
}

resource "aws_s3_bucket_logging" "bucket_logging" {
count = var.access_logs == true && var.apply_resource == true ? 1 : 0

bucket = aws_s3_bucket.bucket[0].id
target_bucket = aws_s3_bucket.log_bucket[0].id
target_prefix = "${local.bucket_name}/${data.aws_caller_identity.current.account_id}/"
}

resource "aws_s3_bucket_lifecycle_configuration" "bucket_lifecycle" {
count = var.apply_resource == true && var.abort_incomplete_uploads == true ? 1 : 0

bucket = aws_s3_bucket.bucket[0].id

rule {
id = "abort-incomplete-uploads"
status = "Enabled"

dynamic "logging" {
for_each = var.access_logs == true ? ["include_block"] : []
content {
target_bucket = aws_s3_bucket.log_bucket.*.id[0]
target_prefix = "${local.bucket_name}/${data.aws_caller_identity.current.account_id}/"
abort_incomplete_multipart_upload {
days_after_initiation = 7
}
}

dynamic "cors_rule" {
for_each = length(var.cors_urls) > 0 ? ["include-cors"] : []
content {
allowed_headers = ["*"]
allowed_methods = ["PUT", "POST", "GET"]
allowed_origins = var.cors_urls
expose_headers = ["ETag", "x-amz-server-side-encryption", "x-amz-request-id", "x-amz-id-2"]
max_age_seconds = 3000
expiration {
days = 0
expired_object_delete_marker = false
}
}
}

tags = merge(
var.common_tags,
tomap(
{ "Name" = local.bucket_name }
)
)
resource "aws_s3_bucket_cors_configuration" "bucket_cors" {
count = var.apply_resource == true && length(var.cors_urls) > 0 ? 1 : 0

bucket = aws_s3_bucket.bucket[0].id

cors_rule {
allowed_headers = ["*"]
allowed_methods = ["PUT", "POST", "GET"]
allowed_origins = var.cors_urls
expose_headers = ["ETag", "x-amz-server-side-encryption", "x-amz-request-id", "x-amz-id-2"]
max_age_seconds = 3000
}
}

resource "aws_s3_bucket_policy" "bucket" {
count = var.apply_resource == true ? 1 : 0
bucket = aws_s3_bucket.bucket.*.id[0]
policy = local.environment == "mgmt" && contains(["log-data", "lambda_update"], var.bucket_policy) ? templatefile("./tdr-terraform-modules/s3/templates/${var.bucket_policy}.json.tpl",
{
bucket_name = aws_s3_bucket.bucket.*.id[0],
account_id = data.aws_caller_identity.current.account_id,
external_account_1 = data.aws_ssm_parameter.intg_account_number.*.value[0],
external_account_2 = data.aws_ssm_parameter.staging_account_number.*.value[0],
external_account_3 = data.aws_ssm_parameter.prod_account_number.*.value[0]
bucket_name = aws_s3_bucket.bucket.*.id[0],
account_id = data.aws_caller_identity.current.account_id,
external_account_1 = data.aws_ssm_parameter.intg_account_number.*.value[0],
external_account_2 = data.aws_ssm_parameter.staging_account_number.*.value[0],
external_account_3 = data.aws_ssm_parameter.prod_account_number.*.value[0]
canonical_user_grants = jsonencode(var.canonical_user_grants)
}) : templatefile("./tdr-terraform-modules/s3/templates/${var.bucket_policy}.json.tpl",
{
bucket_name = aws_s3_bucket.bucket.*.id[0],
Expand All @@ -134,6 +158,7 @@ resource "aws_s3_bucket_policy" "bucket" {
environment = local.environment, title_environment = title(local.environment),
read_access_roles = var.read_access_role_arns,
cloudfront_distribution_arns = jsonencode(var.cloudfront_distribution_arns)
canonical_user_grants = jsonencode(var.canonical_user_grants)
})
depends_on = [aws_s3_bucket_public_access_block.bucket]
}
Expand Down
2 changes: 1 addition & 1 deletion s3/templates/cloudtrail.json.tpl
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"Version": "2012-10-17",
"Statement": [
{
{
"Sid": "AWSCloudTrailAclCheck",
"Effect": "Allow",
"Principal": {
Expand Down
25 changes: 24 additions & 1 deletion s3/templates/secure_transport.json.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,30 @@
"Id": "secure-transport-${bucket_name}",
"Version": "2012-10-17",
"Statement": [
%{ for grant in jsondecode(canonical_user_grants) ~}
{
"Sid": "GrantPermissions-${grant.id}",
"Effect": "Allow",
"Principal": {
"CanonicalUser": "${grant.id}"
},
"Action": [
"s3:GetObject",
"s3:PutObject",
"s3:DeleteObject",
"s3:ListBucket",
"s3:GetBucketLocation",
"s3:GetBucketAcl",
"s3:PutBucketAcl",
"s3:DeleteBucket"
],
"Resource": [
"arn:aws:s3:::${bucket_name}",
"arn:aws:s3:::${bucket_name}/*"
]
},
%{ endfor ~}
{
"Sid": "AllowSSLRequestsOnly",
"Action": "s3:*",
"Effect": "Deny",
Expand All @@ -16,6 +39,6 @@
}
},
"Principal": "*"
}
}
]
}
3 changes: 2 additions & 1 deletion s3/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ variable "environment_suffix" {
variable "acl" {
default = "private"
}

# If this is set, the id will be used to set bucket policies for the user equivalent to 'FULL_CONTROL'.Permissions value ignored
# Only specified for the log bucket and used in the se
variable "canonical_user_grants" {
description = "A list of canonical user IDs and their permissions. If this is set, you cannot use a canned ACL"
type = list(object({ id = string, permissions = list(string) }))
Expand Down
2 changes: 1 addition & 1 deletion vpc/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ resource "aws_route" "internet_access" {

resource "aws_eip" "gw" {
count = var.az_count
vpc = true
domain = "vpc"
depends_on = [aws_internet_gateway.gw]

tags = merge(
Expand Down
Loading