From 5ac9d4241da907a38c75dbad0e33f4eebe013e05 Mon Sep 17 00:00:00 2001 From: dmattia Date: Wed, 15 Jan 2020 14:17:48 -0600 Subject: [PATCH] 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 | 3 ++- docs/terraform.md | 3 ++- main.tf | 5 ++++- variables.tf | 8 +++++++- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 13d6250a..e5688d88 100644 --- a/README.md +++ b/README.md @@ -189,8 +189,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 | diff --git a/docs/terraform.md b/docs/terraform.md index 7e814862..1433f7fa 100644 --- a/docs/terraform.md +++ b/docs/terraform.md @@ -45,8 +45,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 | diff --git a/main.tf b/main.tf index ce52552c..b4b1ebc5 100644 --- a/main.tf +++ b/main.tf @@ -69,6 +69,7 @@ data "template_file" "default" { } resource "aws_s3_bucket_policy" "default" { + count = ! local.use_existing_origin || var.override_origin_bucket_policy ? 1 : 0 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 @@ -148,6 +149,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 index fc4da713..57ed69f1 100644 --- 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" {