Make it optional to override the origin s3 policy (#67)

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. Co-authored-by: Maxim Mironenko <[email protected]>
cloudposse · Feb 25, 2020 · 493d99d · 493d99d
1 parent 99a794d
commit 493d99d
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 4 deletions.
diff --git a/README.md b/README.md
@@ -190,8 +190,9 @@ Available targets:
 | namespace | Namespace (e.g. `eg` or `cp`) | string | `` | no |
 | ordered_cache | An ordered list of cache behaviors resource for this distribution. List from top to bottom in order of precedence. The topmost cache behavior will have precedence 0. The fields can be described by the other variables in this file. For example, the field 'lambda_function_association' in this object has a description in var.lambda_function_association variable earlier in this file. The only difference is that fields on this object are in ordered caches, whereas the rest of the vars in this file apply only to the default cache. | object | `<list>` | 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 |

diff --git a/docs/terraform.md b/docs/terraform.md
@@ -46,8 +46,9 @@
 | namespace | Namespace (e.g. `eg` or `cp`) | string | `` | no |
 | ordered_cache | An ordered list of cache behaviors resource for this distribution. List from top to bottom in order of precedence. The topmost cache behavior will have precedence 0. The fields can be described by the other variables in this file. For example, the field 'lambda_function_association' in this object has a description in var.lambda_function_association variable earlier in this file. The only difference is that fields on this object are in ordered caches, whereas the rest of the vars in this file apply only to the default cache. | object | `<list>` | 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 |

diff --git a/main.tf b/main.tf
@@ -94,6 +94,7 @@ data "template_file" "default" {
 }
 
 resource "aws_s3_bucket_policy" "default" {
+  count  = ! local.using_existing_origin || var.override_origin_bucket_policy ? 1 : 0
   bucket = local.bucket
   policy = data.template_file.default.rendered
 }
@@ -102,7 +103,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
@@ -173,6 +174,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))

diff --git a/variables.tf b/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" {