Make it optional to override the origin s3 policy

When specifying var.origin_bucket, it can be nice to leave the existing bucket's policy as is.

As an example, I manage an s3 bucket that multiple CloudFront dists use as their origin (without paths, they just use the same code).

In this case, I do not want to restrict the bucket to only talk to a single CF dist, as this module does by default.

README.md

@@ -188,8 +188,9 @@ Available targets:
 | name | Name  (e.g. `bastion` or `app`) | string | - | yes |
 | namespace | Namespace (e.g. `eg` or `cp`) | string | `` | no |
 | origin_bucket | Origin S3 bucket name | string | `` | no |
-| origin_force_destroy | Delete all objects from the bucket  so that the bucket can be destroyed without error (e.g. `true` or `false`) | bool | `false` | no |
+| origin_force_destroy | Delete all objects from the bucket so that the bucket can be destroyed without error (e.g. `true` or `false`) | bool | `false` | no |
 | origin_path | An optional element that causes CloudFront to request your content from a directory in your Amazon S3 bucket or your custom origin. It must begin with a /. Do not add a / at the end of the path. | string | `` | no |
+| override_origin_bucket_policy | When using an existing origin bucket (through var.origin_bucket), setting this to 'false' will make it so the existing bucket policy will not be overriden | bool | `true` | no |
 | parent_zone_id | ID of the hosted zone to contain this record  (or specify `parent_zone_name`) | string | `` | no |
 | parent_zone_name | Name of the hosted zone to contain this record (or specify `parent_zone_id`) | string | `` | no |
 | price_class | Price class for this distribution: `PriceClass_All`, `PriceClass_200`, `PriceClass_100` | string | `PriceClass_100` | no |

docs/terraform.md

@@ -44,8 +44,9 @@
 | name | Name  (e.g. `bastion` or `app`) | string | - | yes |
 | namespace | Namespace (e.g. `eg` or `cp`) | string | `` | no |
 | origin_bucket | Origin S3 bucket name | string | `` | no |
-| origin_force_destroy | Delete all objects from the bucket  so that the bucket can be destroyed without error (e.g. `true` or `false`) | bool | `false` | no |
+| origin_force_destroy | Delete all objects from the bucket so that the bucket can be destroyed without error (e.g. `true` or `false`) | bool | `false` | no |
 | origin_path | An optional element that causes CloudFront to request your content from a directory in your Amazon S3 bucket or your custom origin. It must begin with a /. Do not add a / at the end of the path. | string | `` | no |
+| override_origin_bucket_policy | When using an existing origin bucket (through var.origin_bucket), setting this to 'false' will make it so the existing bucket policy will not be overriden | bool | `true` | no |
 | parent_zone_id | ID of the hosted zone to contain this record  (or specify `parent_zone_name`) | string | `` | no |
 | parent_zone_name | Name of the hosted zone to contain this record (or specify `parent_zone_id`) | string | `` | no |
 | price_class | Price class for this distribution: `PriceClass_All`, `PriceClass_200`, `PriceClass_100` | string | `PriceClass_100` | no |

main.tf

@@ -69,6 +69,7 @@ data "template_file" "default" {
 }
 
 resource "aws_s3_bucket_policy" "default" {
+  count  = local.using_existing_origin && ! var.override_origin_bucket_policy ? 0 : 1
   bucket = local.bucket
   policy = data.template_file.default.rendered
 }
@@ -77,7 +78,7 @@ data "aws_region" "current" {
 }
 
 resource "aws_s3_bucket" "origin" {
-  count         = signum(length(var.origin_bucket)) == 1 ? 0 : 1
+  count         = local.using_existing_origin ? 0 : 1
   bucket        = module.origin_label.id
   acl           = "private"
   tags          = module.origin_label.tags
@@ -147,6 +148,8 @@ data "aws_s3_bucket" "selected" {
 }
 
 locals {
+  using_existing_origin = signum(length(var.origin_bucket)) == 1
+
   bucket = join("",
     compact(
       concat([var.origin_bucket], concat([""], aws_s3_bucket.origin.*.id))

variables.tf

@@ -81,6 +81,12 @@ variable "additional_bucket_policy" {
   description = "Additional policies for the bucket. If included in the policies, the variables `$${bucket_name}`, `$${origin_path}` and `$${cloudfront_origin_access_identity_iam_arn}` will be substituted. It is also possible to override the default policy statements by providing statements with `S3GetObjectForCloudFront` and `S3ListBucketForCloudFront` sid."
 }
 
+variable "override_origin_bucket_policy" {
+  type        = bool
+  default     = true
+  description = "When using an existing origin bucket (through var.origin_bucket), setting this to 'false' will make it so the existing bucket policy will not be overriden"
+}
+
 variable "origin_bucket" {
   type        = string
   default     = ""
@@ -97,7 +103,7 @@ variable "origin_path" {
 variable "origin_force_destroy" {
   type        = bool
   default     = false
-  description = "Delete all objects from the bucket  so that the bucket can be destroyed without error (e.g. `true` or `false`)"
+  description = "Delete all objects from the bucket so that the bucket can be destroyed without error (e.g. `true` or `false`)"
 }
 
 variable "bucket_domain_format" {