From 86a50f52513daa5ce6b6f2d9cf119dcf00c4c58e Mon Sep 17 00:00:00 2001
From: Anton Babenko <anton@antonbabenko.com>
Date: Fri, 22 Nov 2019 13:06:16 +0100
Subject: [PATCH] Fix for bucket policy count when value is not computed (#12)

---
 README.md                 |  2 ++
 examples/complete/main.tf | 37 +++++++++++++++++++++++++++++--------
 main.tf                   |  2 +-
 variables.tf              |  6 ++++++
 4 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/README.md b/README.md
index 4da53ff8..0c94e019 100644
--- a/README.md
+++ b/README.md
@@ -17,6 +17,7 @@ These features of S3 bucket configurations are supported:
 - server-side encryption
 - object locking
 - Cross-Region Replication (CRR)
+- ELB log delivery bucket policy
 
 ## Terraform versions
 
@@ -83,6 +84,7 @@ module "s3_bucket" {
 | acceleration\_status | (Optional) Sets the accelerate configuration of an existing bucket. Can be Enabled or Suspended. | string | `"null"` | no |
 | acl | (Optional) The canned ACL to apply. Defaults to 'private'. | string | `"private"` | no |
 | attach\_elb\_log\_delivery\_policy | Controls if S3 bucket should have ELB log delivery policy attached | bool | `"false"` | no |
+| attach\_policy | Controls if S3 bucket should have bucket policy attached (set to `true` to use value of `policy` as bucket policy) | bool | `"false"` | no |
 | bucket | (Optional, Forces new resource) The name of the bucket. If omitted, Terraform will assign a random, unique name. | string | `"null"` | no |
 | bucket\_prefix | (Optional, Forces new resource) Creates a unique bucket name beginning with the specified prefix. Conflicts with bucket. | string | `"null"` | no |
 | cors\_rule | Map containing a rule of Cross-Origin Resource Sharing. | any | `{}` | no |
diff --git a/examples/complete/main.tf b/examples/complete/main.tf
index c7410a8d..4af31468 100644
--- a/examples/complete/main.tf
+++ b/examples/complete/main.tf
@@ -11,20 +11,29 @@ resource "aws_kms_key" "objects" {
   deletion_window_in_days = 7
 }
 
-module "log_bucket" {
-  source = "../../"
-
-  bucket                         = "logs-${random_pet.this.id}"
-  acl                            = "log-delivery-write"
-  force_destroy                  = true
-  attach_elb_log_delivery_policy = true
+resource "aws_iam_role" "this" {
+  assume_role_policy = <<EOF
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Action": "sts:AssumeRole",
+      "Principal": {
+        "Service": "ec2.amazonaws.com"
+      },
+      "Effect": "Allow",
+      "Sid": ""
+    }
+  ]
+}
+EOF
 }
 
 data "aws_iam_policy_document" "bucket_policy" {
   statement {
     principals {
       type        = "AWS"
-      identifiers = ["*"]
+      identifiers = [aws_iam_role.this.arn]
     }
 
     actions = [
@@ -36,12 +45,24 @@ data "aws_iam_policy_document" "bucket_policy" {
     ]
   }
 }
+
+module "log_bucket" {
+  source = "../../"
+
+  bucket                         = "logs-${random_pet.this.id}"
+  acl                            = "log-delivery-write"
+  force_destroy                  = true
+  attach_elb_log_delivery_policy = true
+}
+
 module "s3_bucket" {
   source = "../../"
 
   bucket        = local.bucket_name
   acl           = "private"
   force_destroy = true
+
+  attach_policy = true
   policy        = data.aws_iam_policy_document.bucket_policy.json
 
   tags = {
diff --git a/main.tf b/main.tf
index 4930c626..8e973d61 100644
--- a/main.tf
+++ b/main.tf
@@ -218,7 +218,7 @@ resource "aws_s3_bucket" "this" {
 }
 
 resource "aws_s3_bucket_policy" "this" {
-  count = var.create_bucket && (var.attach_elb_log_delivery_policy || var.policy != null) ? 1 : 0
+  count = var.create_bucket && (var.attach_elb_log_delivery_policy || var.attach_policy) ? 1 : 0
 
   bucket = aws_s3_bucket.this[0].id
   policy = var.attach_elb_log_delivery_policy ? data.aws_iam_policy_document.elb_log_delivery[0].json : var.policy
diff --git a/variables.tf b/variables.tf
index c261a8cf..f659aa68 100644
--- a/variables.tf
+++ b/variables.tf
@@ -10,6 +10,12 @@ variable "attach_elb_log_delivery_policy" {
   default     = false
 }
 
+variable "attach_policy" {
+  description = "Controls if S3 bucket should have bucket policy attached (set to `true` to use value of `policy` as bucket policy)"
+  type        = bool
+  default     = false
+}
+
 variable "bucket" {
   description = "(Optional, Forces new resource) The name of the bucket. If omitted, Terraform will assign a random, unique name."
   type        = string