default to creating bucket policy instead of ACL (#16)

* default to creating bucket policy instead of ACL

* Update README.md

Author: sblack4

README.md

@@ -1,22 +1,47 @@
 # terraform-aws-s3logging-bucket
-[![tflint](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/workflows/tflint/badge.svg?branch=master&event=push)](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions?query=workflow%3Atflint+event%3Apush+branch%3Amaster)
-[![tfsec](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/workflows/tfsec/badge.svg?branch=master&event=push)](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions?query=workflow%3Atfsec+event%3Apush+branch%3Amaster)
-[![yamllint](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/workflows/yamllint/badge.svg?branch=master&event=push)](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions?query=workflow%3Ayamllint+event%3Apush+branch%3Amaster)
-[![misspell](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/workflows/misspell/badge.svg?branch=master&event=push)](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions?query=workflow%3Amisspell+event%3Apush+branch%3Amaster)
-[![pre-commit-check](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/workflows/pre-commit-check/badge.svg?branch=master&event=push)](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions?query=workflow%3Apre-commit-check+event%3Apush+branch%3Amaster)
-<a href="https://twitter.com/intent/follow?screen_name=RhythmicTech"><img src="https://img.shields.io/twitter/follow/RhythmicTech?style=social&logo=twitter" alt="follow on Twitter"></a>
+[![tflint](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions/workflows/tflint.yaml/badge.svg?branch=master&event=push)](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions?query=workflow%3Atflint+event%3Apush+branch%3Amaster)
+[![tfsec](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions/workflows/tfsec.yaml/badge.svg?branch=master&event=push)](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions?query=workflow%3Atfsec+event%3Apush+branch%3Amaster)
+[![yamllint](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions/workflows/yamllint.yaml/badge.svg?branch=master&event=push)](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions?query=workflow%3Ayamllint+event%3Apush+branch%3Amaster)
+[![misspell](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions/workflows/misspell.yaml/badge.svg?branch=master&event=push)](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions?query=workflow%3Amisspell+event%3Apush+branch%3Amaster)
+[![pre-commit-check](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions/workflows/pre-commit.yaml/badge.svg?branch=master&event=push)](https://github.com/rhythmictech/terraform-aws-s3logging-bucket/actions?query=workflow%3Apre-commit-check+event%3Apush+branch%3Amaster)
+![X (formerly Twitter) Follow](https://img.shields.io/twitter/follow/RhythmicTech)
 
 
 Create and manage a bucket suitable for access logging for other S3 buckets.
 
 
 ## Usage
+Basic usage:
 ```
 module "s3logging-bucket" {
   source = "rhythmictech/s3logging-bucket/aws"
 }
 ```
 
+Combine with other S3-based modules, like our cloudtrail bucket module:
+```
+module "s3logging-bucket" {
+  source = "rhythmictech/s3logging-bucket/aws"
+  version = "3.3.0"
+}
+
+module "cloudtrail-bucket" {
+  source         = "git::https://github.com/rhythmictech/terraform-aws-cloudtrail-bucket?ref=v4.0.0"
+
+  logging_bucket      = module.s3logging-bucket.s3_bucket_name
+  region              = var.region
+}
+
+module "cloudtrail-logging" {
+  source            = "git::https://github.com/rhythmictech/terraform-aws-cloudtrail-logging?ref=v1.3.0"
+
+  cloudtrail_bucket = module.cloudtrail-bucket.s3_bucket_name
+  kms_key_id        = module.cloudtrail-bucket.kms_key_id
+  region            = var.region
+}
+```
+
+
 <!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
 ## Requirements
 
@@ -42,6 +67,8 @@ No modules.
 | [aws_s3_bucket.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket) | resource |
 | [aws_s3_bucket_acl.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_acl) | resource |
 | [aws_s3_bucket_lifecycle_configuration.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_lifecycle_configuration) | resource |
+| [aws_s3_bucket_ownership_controls.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_ownership_controls) | resource |
+| [aws_s3_bucket_policy.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_policy) | resource |
 | [aws_s3_bucket_public_access_block.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block) | resource |
 | [aws_s3_bucket_server_side_encryption_configuration.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_server_side_encryption_configuration) | resource |
 | [aws_s3_bucket_versioning.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_versioning) | resource |
@@ -56,6 +83,7 @@ No modules.
 | <a name="input_bucket_suffix"></a> [bucket\_suffix](#input\_bucket\_suffix) | Suffix to apply to the bucket (use `bucket_name` or `bucket_suffix`). When using `bucket_suffix`, the bucket name will be `[account_id]-[region]-s3logging-[bucket_suffix].` | `string` | `"default"` | no |
 | <a name="input_kms_key_id"></a> [kms\_key\_id](#input\_kms\_key\_id) | KMS key to encrypt bucket with. | `string` | `null` | no |
 | <a name="input_lifecycle_rules"></a> [lifecycle\_rules](#input\_lifecycle\_rules) | lifecycle rules to apply to the bucket | <pre>list(object(<br>    {<br>      id                            = string<br>      enabled                       = optional(bool, true)<br>      expiration                    = optional(number)<br>      prefix                        = optional(number)<br>      noncurrent_version_expiration = optional(number)<br>      transition = optional(list(object({<br>        days          = number<br>        storage_class = string<br>      })))<br>  }))</pre> | <pre>[<br>  {<br>    "id": "expire-noncurrent-objects-after-ninety-days",<br>    "noncurrent_version_expiration": 90<br>  },<br>  {<br>    "id": "transition-to-IA-after-30-days",<br>    "transition": [<br>      {<br>        "days": 30,<br>        "storage_class": "STANDARD_IA"<br>      }<br>    ]<br>  },<br>  {<br>    "expiration": 2557,<br>    "id": "delete-after-seven-years"<br>  }<br>]</pre> | no |
+| <a name="input_object_ownership"></a> [object\_ownership](#input\_object\_ownership) | Specifies S3 object ownership control. Defaults to BucketOwnerPreferred for backwards-compatibility. Recommended value is BucketOwnerEnforced. | `string` | `"BucketOwnerEnforced"` | no |
 | <a name="input_tags"></a> [tags](#input\_tags) | Tags to add to supported resources | `map(string)` | `{}` | no |
 | <a name="input_versioning_enabled"></a> [versioning\_enabled](#input\_versioning\_enabled) | Whether or not to use versioning on the bucket. This can be useful for audit purposes since objects in a logging bucket should not be updated. | `bool` | `true` | no |
 

main.tf

@@ -103,3 +103,33 @@ resource "aws_s3_bucket_versioning" "this" {
     status = "Enabled"
   }
 }
+
+resource "aws_s3_bucket_policy" "this" {
+  count = var.object_ownership == "BucketOwnerEnforced" ? 1 : 0
+
+  bucket = aws_s3_bucket.this.id
+
+  policy = <<EOF
+{
+    "Version": "2012-10-17",
+    "Statement": [
+        {
+            "Sid": "S3ServerAccessLogsPolicy",
+            "Effect": "Allow",
+            "Principal": {
+                "Service": "logging.s3.amazonaws.com"
+            },
+            "Action": [
+                "s3:PutObject"
+            ],
+            "Resource": "arn:aws:s3:::${aws_s3_bucket.this.bucket}/*",
+            "Condition": {
+                "StringEquals": {
+                    "aws:SourceAccount": "${local.account_id}"
+                }
+            }
+        }
+    ]
+}
+EOF
+}

variables.tf

@@ -52,7 +52,7 @@ variable "lifecycle_rules" {
 }
 
 variable "object_ownership" {
-  default     = "BucketOwnerPreferred"
+  default     = "BucketOwnerEnforced"
   description = "Specifies S3 object ownership control. Defaults to BucketOwnerPreferred for backwards-compatibility. Recommended value is BucketOwnerEnforced."
 }