feat: Add default Access Log Delivery Policy (same as ALB/NLB) to wor…

…k since April 2023 (terraform-aws-modules#230)

README.md

@@ -163,6 +163,7 @@ No modules.
 | [aws_s3_bucket_website_configuration.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_website_configuration) | resource |
 | [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source |
 | [aws_canonical_user_id.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/canonical_user_id) | data source |
+| [aws_iam_policy_document.access_log_delivery](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.combined](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.deny_insecure_transport](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.elb_log_delivery](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
@@ -177,11 +178,14 @@ No modules.
 | Name | Description | Type | Default | Required |
 |------|-------------|------|---------|:--------:|
 | <a name="input_acceleration_status"></a> [acceleration\_status](#input\_acceleration\_status) | (Optional) Sets the accelerate configuration of an existing bucket. Can be Enabled or Suspended. | `string` | `null` | no |
+| <a name="input_access_log_delivery_policy_source_accounts"></a> [access\_log\_delivery\_policy\_source\_accounts](#input\_access\_log\_delivery\_policy\_source\_accounts) | (Optional) List of AWS Account IDs should be allowed to deliver access logs to this bucket. | `list(string)` | `[]` | no |
+| <a name="input_access_log_delivery_policy_source_buckets"></a> [access\_log\_delivery\_policy\_source\_buckets](#input\_access\_log\_delivery\_policy\_source\_buckets) | (Optional) List of S3 bucket ARNs wich should be allowed to deliver access logs to this bucket. | `list(string)` | `[]` | no |
 | <a name="input_acl"></a> [acl](#input\_acl) | (Optional) The canned ACL to apply. Conflicts with `grant` | `string` | `null` | no |
 | <a name="input_analytics_configuration"></a> [analytics\_configuration](#input\_analytics\_configuration) | Map containing bucket analytics configuration. | `any` | `{}` | no |
 | <a name="input_analytics_self_source_destination"></a> [analytics\_self\_source\_destination](#input\_analytics\_self\_source\_destination) | Whether or not the analytics source bucket is also the destination bucket. | `bool` | `false` | no |
 | <a name="input_analytics_source_account_id"></a> [analytics\_source\_account\_id](#input\_analytics\_source\_account\_id) | The analytics source account id. | `string` | `null` | no |
 | <a name="input_analytics_source_bucket_arn"></a> [analytics\_source\_bucket\_arn](#input\_analytics\_source\_bucket\_arn) | The analytics source bucket ARN. | `string` | `null` | no |
+| <a name="input_attach_access_log_delivery_policy"></a> [attach\_access\_log\_delivery\_policy](#input\_attach\_access\_log\_delivery\_policy) | Controls if S3 bucket should have S3 access log delivery policy attached | `bool` | `false` | no |
 | <a name="input_attach_analytics_destination_policy"></a> [attach\_analytics\_destination\_policy](#input\_attach\_analytics\_destination\_policy) | Controls if S3 bucket should have bucket analytics destination policy attached. | `bool` | `false` | no |
 | <a name="input_attach_deny_insecure_transport_policy"></a> [attach\_deny\_insecure\_transport\_policy](#input\_attach\_deny\_insecure\_transport\_policy) | Controls if S3 bucket should have deny non-SSL transport policy attached | `bool` | `false` | no |
 | <a name="input_attach_elb_log_delivery_policy"></a> [attach\_elb\_log\_delivery\_policy](#input\_attach\_elb\_log\_delivery\_policy) | Controls if S3 bucket should have ELB log delivery policy attached | `bool` | `false` | no |

examples/complete/main.tf

@@ -67,16 +67,18 @@ module "log_bucket" {
   source = "../../"
 
   bucket        = "logs-${random_pet.this.id}"
-  acl           = "log-delivery-write"
   force_destroy = true
 
   control_object_ownership = true
-  object_ownership         = "ObjectWriter"
 
   attach_elb_log_delivery_policy        = true
   attach_lb_log_delivery_policy         = true
+  attach_access_log_delivery_policy     = true
   attach_deny_insecure_transport_policy = true
   attach_require_latest_tls_policy      = true
+
+  access_log_delivery_policy_source_accounts = [data.aws_caller_identity.current.account_id]
+  access_log_delivery_policy_source_buckets  = ["arn:aws:s3:::${local.bucket_name}"]
 }
 
 module "cloudfront_log_bucket" {

main.tf

@@ -530,6 +530,7 @@ data "aws_iam_policy_document" "combined" {
   source_policy_documents = compact([
     var.attach_elb_log_delivery_policy ? data.aws_iam_policy_document.elb_log_delivery[0].json : "",
     var.attach_lb_log_delivery_policy ? data.aws_iam_policy_document.lb_log_delivery[0].json : "",
+    var.attach_access_log_delivery_policy ? data.aws_iam_policy_document.access_log_delivery[0].json : "",
     var.attach_require_latest_tls_policy ? data.aws_iam_policy_document.require_latest_tls[0].json : "",
     var.attach_deny_insecure_transport_policy ? data.aws_iam_policy_document.deny_insecure_transport[0].json : "",
     var.attach_inventory_destination_policy || var.attach_analytics_destination_policy ? data.aws_iam_policy_document.inventory_and_analytics_destination_policy[0].json : "",
@@ -666,6 +667,71 @@ data "aws_iam_policy_document" "lb_log_delivery" {
   }
 }
 
+# Grant access to S3 log delivery group for server access logging
+# https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-ownership-migrating-acls-prerequisites.html#object-ownership-server-access-logs
+# https://docs.aws.amazon.com/AmazonS3/latest/userguide/enable-server-access-logging.html#grant-log-delivery-permissions-general
+data "aws_iam_policy_document" "access_log_delivery" {
+  count = local.create_bucket && var.attach_access_log_delivery_policy ? 1 : 0
+
+  statement {
+    sid = "AWSAccessLogDeliveryWrite"
+
+    principals {
+      type        = "Service"
+      identifiers = ["logging.s3.amazonaws.com"]
+    }
+
+    effect = "Allow"
+
+    actions = [
+      "s3:PutObject",
+    ]
+
+    resources = [
+      "${aws_s3_bucket.this[0].arn}/*",
+    ]
+
+    dynamic "condition" {
+      for_each = length(var.access_log_delivery_policy_source_buckets) != 0 ? [true] : []
+      content {
+        test     = "ForAnyValue:ArnLike"
+        variable = "aws:SourceArn"
+        values   = var.access_log_delivery_policy_source_buckets
+      }
+    }
+
+    dynamic "condition" {
+      for_each = length(var.access_log_delivery_policy_source_accounts) != 0 ? [true] : []
+      content {
+        test     = "ForAnyValue:StringEquals"
+        variable = "aws:SourceAccount"
+        values   = var.access_log_delivery_policy_source_accounts
+      }
+    }
+
+  }
+
+  statement {
+    sid = "AWSAccessLogDeliveryAclCheck"
+
+    effect = "Allow"
+
+    principals {
+      type        = "Service"
+      identifiers = ["logging.s3.amazonaws.com"]
+    }
+
+    actions = [
+      "s3:GetBucketAcl",
+    ]
+
+    resources = [
+      aws_s3_bucket.this[0].arn,
+    ]
+
+  }
+}
+
 data "aws_iam_policy_document" "deny_insecure_transport" {
   count = local.create_bucket && var.attach_deny_insecure_transport_policy ? 1 : 0
 

variables.tf

@@ -16,6 +16,12 @@ variable "attach_lb_log_delivery_policy" {
   default     = false
 }
 
+variable "attach_access_log_delivery_policy" {
+  description = "Controls if S3 bucket should have S3 access log delivery policy attached"
+  type        = bool
+  default     = false
+}
+
 variable "attach_deny_insecure_transport_policy" {
   description = "Controls if S3 bucket should have deny non-SSL transport policy attached"
   type        = bool
@@ -124,6 +130,18 @@ variable "logging" {
   default     = {}
 }
 
+variable "access_log_delivery_policy_source_buckets" {
+  description = "(Optional) List of S3 bucket ARNs wich should be allowed to deliver access logs to this bucket."
+  type        = list(string)
+  default     = []
+}
+
+variable "access_log_delivery_policy_source_accounts" {
+  description = "(Optional) List of AWS Account IDs should be allowed to deliver access logs to this bucket."
+  type        = list(string)
+  default     = []
+}
+
 variable "grant" {
   description = "An ACL policy grant. Conflicts with `acl`"
   type        = any

wrappers/main.tf

@@ -3,50 +3,53 @@ module "wrapper" {
 
   for_each = var.items
 
-  create_bucket                         = try(each.value.create_bucket, var.defaults.create_bucket, true)
-  attach_elb_log_delivery_policy        = try(each.value.attach_elb_log_delivery_policy, var.defaults.attach_elb_log_delivery_policy, false)
-  attach_lb_log_delivery_policy         = try(each.value.attach_lb_log_delivery_policy, var.defaults.attach_lb_log_delivery_policy, false)
-  attach_deny_insecure_transport_policy = try(each.value.attach_deny_insecure_transport_policy, var.defaults.attach_deny_insecure_transport_policy, false)
-  attach_require_latest_tls_policy      = try(each.value.attach_require_latest_tls_policy, var.defaults.attach_require_latest_tls_policy, false)
-  attach_policy                         = try(each.value.attach_policy, var.defaults.attach_policy, false)
-  attach_public_policy                  = try(each.value.attach_public_policy, var.defaults.attach_public_policy, true)
-  attach_inventory_destination_policy   = try(each.value.attach_inventory_destination_policy, var.defaults.attach_inventory_destination_policy, false)
-  attach_analytics_destination_policy   = try(each.value.attach_analytics_destination_policy, var.defaults.attach_analytics_destination_policy, false)
-  bucket                                = try(each.value.bucket, var.defaults.bucket, null)
-  bucket_prefix                         = try(each.value.bucket_prefix, var.defaults.bucket_prefix, null)
-  acl                                   = try(each.value.acl, var.defaults.acl, null)
-  policy                                = try(each.value.policy, var.defaults.policy, null)
-  tags                                  = try(each.value.tags, var.defaults.tags, {})
-  force_destroy                         = try(each.value.force_destroy, var.defaults.force_destroy, false)
-  acceleration_status                   = try(each.value.acceleration_status, var.defaults.acceleration_status, null)
-  request_payer                         = try(each.value.request_payer, var.defaults.request_payer, null)
-  website                               = try(each.value.website, var.defaults.website, {})
-  cors_rule                             = try(each.value.cors_rule, var.defaults.cors_rule, [])
-  versioning                            = try(each.value.versioning, var.defaults.versioning, {})
-  logging                               = try(each.value.logging, var.defaults.logging, {})
-  grant                                 = try(each.value.grant, var.defaults.grant, [])
-  owner                                 = try(each.value.owner, var.defaults.owner, {})
-  expected_bucket_owner                 = try(each.value.expected_bucket_owner, var.defaults.expected_bucket_owner, null)
-  lifecycle_rule                        = try(each.value.lifecycle_rule, var.defaults.lifecycle_rule, [])
-  replication_configuration             = try(each.value.replication_configuration, var.defaults.replication_configuration, {})
-  server_side_encryption_configuration  = try(each.value.server_side_encryption_configuration, var.defaults.server_side_encryption_configuration, {})
-  intelligent_tiering                   = try(each.value.intelligent_tiering, var.defaults.intelligent_tiering, {})
-  object_lock_configuration             = try(each.value.object_lock_configuration, var.defaults.object_lock_configuration, {})
-  metric_configuration                  = try(each.value.metric_configuration, var.defaults.metric_configuration, [])
-  inventory_configuration               = try(each.value.inventory_configuration, var.defaults.inventory_configuration, {})
-  inventory_source_account_id           = try(each.value.inventory_source_account_id, var.defaults.inventory_source_account_id, null)
-  inventory_source_bucket_arn           = try(each.value.inventory_source_bucket_arn, var.defaults.inventory_source_bucket_arn, null)
-  inventory_self_source_destination     = try(each.value.inventory_self_source_destination, var.defaults.inventory_self_source_destination, false)
-  analytics_configuration               = try(each.value.analytics_configuration, var.defaults.analytics_configuration, {})
-  analytics_source_account_id           = try(each.value.analytics_source_account_id, var.defaults.analytics_source_account_id, null)
-  analytics_source_bucket_arn           = try(each.value.analytics_source_bucket_arn, var.defaults.analytics_source_bucket_arn, null)
-  analytics_self_source_destination     = try(each.value.analytics_self_source_destination, var.defaults.analytics_self_source_destination, false)
-  object_lock_enabled                   = try(each.value.object_lock_enabled, var.defaults.object_lock_enabled, false)
-  block_public_acls                     = try(each.value.block_public_acls, var.defaults.block_public_acls, true)
-  block_public_policy                   = try(each.value.block_public_policy, var.defaults.block_public_policy, true)
-  ignore_public_acls                    = try(each.value.ignore_public_acls, var.defaults.ignore_public_acls, true)
-  restrict_public_buckets               = try(each.value.restrict_public_buckets, var.defaults.restrict_public_buckets, true)
-  control_object_ownership              = try(each.value.control_object_ownership, var.defaults.control_object_ownership, false)
-  object_ownership                      = try(each.value.object_ownership, var.defaults.object_ownership, "BucketOwnerEnforced")
-  putin_khuylo                          = try(each.value.putin_khuylo, var.defaults.putin_khuylo, true)
+  create_bucket                              = try(each.value.create_bucket, var.defaults.create_bucket, true)
+  attach_elb_log_delivery_policy             = try(each.value.attach_elb_log_delivery_policy, var.defaults.attach_elb_log_delivery_policy, false)
+  attach_lb_log_delivery_policy              = try(each.value.attach_lb_log_delivery_policy, var.defaults.attach_lb_log_delivery_policy, false)
+  attach_access_log_delivery_policy          = try(each.value.attach_access_log_delivery_policy, var.defaults.attach_access_log_delivery_policy, false)
+  attach_deny_insecure_transport_policy      = try(each.value.attach_deny_insecure_transport_policy, var.defaults.attach_deny_insecure_transport_policy, false)
+  attach_require_latest_tls_policy           = try(each.value.attach_require_latest_tls_policy, var.defaults.attach_require_latest_tls_policy, false)
+  attach_policy                              = try(each.value.attach_policy, var.defaults.attach_policy, false)
+  attach_public_policy                       = try(each.value.attach_public_policy, var.defaults.attach_public_policy, true)
+  attach_inventory_destination_policy        = try(each.value.attach_inventory_destination_policy, var.defaults.attach_inventory_destination_policy, false)
+  attach_analytics_destination_policy        = try(each.value.attach_analytics_destination_policy, var.defaults.attach_analytics_destination_policy, false)
+  bucket                                     = try(each.value.bucket, var.defaults.bucket, null)
+  bucket_prefix                              = try(each.value.bucket_prefix, var.defaults.bucket_prefix, null)
+  acl                                        = try(each.value.acl, var.defaults.acl, null)
+  policy                                     = try(each.value.policy, var.defaults.policy, null)
+  tags                                       = try(each.value.tags, var.defaults.tags, {})
+  force_destroy                              = try(each.value.force_destroy, var.defaults.force_destroy, false)
+  acceleration_status                        = try(each.value.acceleration_status, var.defaults.acceleration_status, null)
+  request_payer                              = try(each.value.request_payer, var.defaults.request_payer, null)
+  website                                    = try(each.value.website, var.defaults.website, {})
+  cors_rule                                  = try(each.value.cors_rule, var.defaults.cors_rule, [])
+  versioning                                 = try(each.value.versioning, var.defaults.versioning, {})
+  logging                                    = try(each.value.logging, var.defaults.logging, {})
+  access_log_delivery_policy_source_buckets  = try(each.value.access_log_delivery_policy_source_buckets, var.defaults.access_log_delivery_policy_source_buckets, [])
+  access_log_delivery_policy_source_accounts = try(each.value.access_log_delivery_policy_source_accounts, var.defaults.access_log_delivery_policy_source_accounts, [])
+  grant                                      = try(each.value.grant, var.defaults.grant, [])
+  owner                                      = try(each.value.owner, var.defaults.owner, {})
+  expected_bucket_owner                      = try(each.value.expected_bucket_owner, var.defaults.expected_bucket_owner, null)
+  lifecycle_rule                             = try(each.value.lifecycle_rule, var.defaults.lifecycle_rule, [])
+  replication_configuration                  = try(each.value.replication_configuration, var.defaults.replication_configuration, {})
+  server_side_encryption_configuration       = try(each.value.server_side_encryption_configuration, var.defaults.server_side_encryption_configuration, {})
+  intelligent_tiering                        = try(each.value.intelligent_tiering, var.defaults.intelligent_tiering, {})
+  object_lock_configuration                  = try(each.value.object_lock_configuration, var.defaults.object_lock_configuration, {})
+  metric_configuration                       = try(each.value.metric_configuration, var.defaults.metric_configuration, [])
+  inventory_configuration                    = try(each.value.inventory_configuration, var.defaults.inventory_configuration, {})
+  inventory_source_account_id                = try(each.value.inventory_source_account_id, var.defaults.inventory_source_account_id, null)
+  inventory_source_bucket_arn                = try(each.value.inventory_source_bucket_arn, var.defaults.inventory_source_bucket_arn, null)
+  inventory_self_source_destination          = try(each.value.inventory_self_source_destination, var.defaults.inventory_self_source_destination, false)
+  analytics_configuration                    = try(each.value.analytics_configuration, var.defaults.analytics_configuration, {})
+  analytics_source_account_id                = try(each.value.analytics_source_account_id, var.defaults.analytics_source_account_id, null)
+  analytics_source_bucket_arn                = try(each.value.analytics_source_bucket_arn, var.defaults.analytics_source_bucket_arn, null)
+  analytics_self_source_destination          = try(each.value.analytics_self_source_destination, var.defaults.analytics_self_source_destination, false)
+  object_lock_enabled                        = try(each.value.object_lock_enabled, var.defaults.object_lock_enabled, false)
+  block_public_acls                          = try(each.value.block_public_acls, var.defaults.block_public_acls, true)
+  block_public_policy                        = try(each.value.block_public_policy, var.defaults.block_public_policy, true)
+  ignore_public_acls                         = try(each.value.ignore_public_acls, var.defaults.ignore_public_acls, true)
+  restrict_public_buckets                    = try(each.value.restrict_public_buckets, var.defaults.restrict_public_buckets, true)
+  control_object_ownership                   = try(each.value.control_object_ownership, var.defaults.control_object_ownership, false)
+  object_ownership                           = try(each.value.object_ownership, var.defaults.object_ownership, "BucketOwnerEnforced")
+  putin_khuylo                               = try(each.value.putin_khuylo, var.defaults.putin_khuylo, true)
 }