terraform-aws-modules / terraform-aws-s3-bucket

Terraform module to create AWS S3 resources 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/s3-bucket/aws
Apache License 2.0
526 stars 3.62k forks source link

Default to bucket policies against legacy ACLs #284

Open atorrescogollo opened 3 months ago

atorrescogollo commented 3 months ago

Is your request related to a problem? Please describe.

AWS recommends to use control_object_ownership and stop using ACLs: https://aws.amazon.com/blogs/security/iam-policies-and-bucket-policies-and-acls-oh-my-controlling-access-to-s3-resources/

As a general rule, AWS recommends that you use S3 bucket policies or IAM policies for access control. S3 ACLs are a legacy access control mechanism that predates IAM. By default, object ownership is set to the bucket owner enforced setting, and all ACLs are disabled

However, the module sets var.control_object_ownership = false as the default: https://github.com/terraform-aws-modules/terraform-aws-s3-bucket/blob/v4.1.2/variables.tf#L301-L305

If I want to deploy a simple bucket, it should use bucket policies by default.

Describe the solution you'd like.

Default to bucket policies:

  variable "control_object_ownership" {
    description = "Whether to manage S3 Bucket Ownership Controls on this bucket."
    type        = bool
-   default     = false
+   default     = true
  }

I think the module should use what AWS recommends by default (also, that's what AWS console creates by default) and legacy behaviours should be stated explicitly by actively disabling the var.control_object_ownership.

module "s3" {
  ...

  control_object_ownership = false # Since we still use ACLs (legacy)
  acl                      = "private"
  ...
}

Describe alternatives you've considered.

Now, to use AWS defaults and recommendation, I have to explicitly set the var.control_object_ownership=true, which is anoying.

module "s3" {
  ...

  control_object_ownership = true # Disabling ACLs in favor of bucket policies is the recommended approach
  ...
}

Additional context

image

github-actions[bot] commented 2 months ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

atorrescogollo commented 2 months ago

Remove stale label

github-actions[bot] commented 1 month ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

atorrescogollo commented 4 weeks ago

Remove stale label