Variable to deny IAM roles access to Apiary managed S3 buckets (#158)

* deny_iamroles variable * fix template variable * fix policy * changelog * PR comment update Co-authored-by: Raj Poluri <[email protected]>
ExpediaGroup · May 11, 2020 · f4f8ae9 · f4f8ae9
1 parent 8e67c7a
commit f4f8ae9
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,10 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [6.2.0] - 2020-05-11
+### Added
+- Variable to deny IAM roles access to Apiary managed S3 buckets.
+
 ## [6.1.3] - 2020-05-11
 ### Changed
 - Set min/max size of HMS thread pool based on memory.  Max will be set to 1 connection for every 2MB RAM.  Min will be 0.25% of max.  This will prevent large HMS instances from not having enough threads/connections available.

diff --git a/VARIABLES.md b/VARIABLES.md
@@ -7,6 +7,7 @@
 | apiary_assume_roles | List of maps - each map describes an IAM role that can be assumed in this account to write data into the configured list of schemas. See section [`apiary_assume_roles`](#apiary_assume_roles) for more info. | list(map) | - | no |
 | apiary_customer_accounts | AWS account IDs for clients of this Metastore. | list | - | yes |
 | apiary_database_name | Database name to create in RDS for Apiary. | string | `apiary` | no |
+| apiary_deny_roles | AWS IAM roles denied access to Apiary managed S3 buckets. | list | - | yes |
 | apiary_domain_name | Apiary domain name for Route 53. | string | `` | no |
 | apiary_log_bucket | Bucket for Apiary logs. | string | - | yes |
 | apiary_log_prefix | Prefix for Apiary logs. | string | `` | no |

diff --git a/s3.tf b/s3.tf
@@ -18,10 +18,11 @@ data "template_file" "bucket_policy" {
     #if apiary_shared_schemas is empty or contains current schema, allow customer accounts to access this bucket.
     customer_principal = "${length(var.apiary_shared_schemas) == 0 || contains(var.apiary_shared_schemas, each.key) ?
       join("\",\"", formatlist("arn:aws:iam::%s:root", var.apiary_customer_accounts)) :
-      "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"}"
+    "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"}"
 
     bucket_name       = each.value["data_bucket"]
     producer_iamroles = "${replace(lookup(var.apiary_producer_iamroles, each.key, "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"), ",", "\",\"")}"
+    deny_iamroles     = join("\",\"", var.apiary_deny_iamroles)
   }
 }
 
@@ -36,9 +37,9 @@ resource "aws_s3_bucket" "apiary_data_bucket" {
   acl           = "private"
   request_payer = "BucketOwner"
   policy        = data.template_file.bucket_policy[each.key].rendered
-  tags          = merge(map("Name", each.value["data_bucket"]),
-                        var.apiary_tags,
-                        jsondecode(lookup(each.value, "tags", "{}")))
+  tags = merge(map("Name", each.value["data_bucket"]),
+    var.apiary_tags,
+  jsondecode(lookup(each.value, "tags", "{}")))
 
   logging {
     target_bucket = local.enable_apiary_s3_log_management ? aws_s3_bucket.apiary_managed_logs_bucket[0].id : var.apiary_log_bucket
@@ -120,8 +121,8 @@ resource "aws_s3_bucket_notification" "data_queue_events" {
   bucket   = aws_s3_bucket.apiary_data_bucket[each.key].id
 
   queue {
-    queue_arn     = aws_sqs_queue.apiary_data_event_queue[0].arn
-    events        = ["s3:ObjectCreated:*", "s3:ObjectRemoved:*"]
+    queue_arn = aws_sqs_queue.apiary_data_event_queue[0].arn
+    events    = ["s3:ObjectCreated:*", "s3:ObjectRemoved:*"]
   }
 }
 

diff --git a/templates/apiary_bucket_policy.json b/templates/apiary_bucket_policy.json
@@ -18,6 +18,20 @@
                 "arn:aws:s3:::${bucket_name}/*"
             ]
         },
+%{if deny_iamroles != ""}
+        {
+            "Sid": "Local role deny permissions",
+            "Effect": "Deny",
+            "Principal": "*",
+            "Action": "s3:*",
+            "Resource": "arn:aws:s3:::${bucket_name}/*",
+            "Condition": {
+              "StringLike": {
+                "aws:PrincipalArn": [ "${deny_iamroles}" ]
+              }
+            }
+        },
+%{endif}
         {
             "Sid": "Apiary producer iamrole permissions",
             "Effect": "Allow",

diff --git a/variables.tf b/variables.tf
@@ -95,6 +95,12 @@ variable "apiary_customer_accounts" {
   type        = list(any)
 }
 
+variable "apiary_deny_iamroles" {
+  description = "AWS IAM roles denied access to Apiary managed S3 buckets."
+  type        = list(string)
+  default     = []
+}
+
 variable "apiary_assume_roles" {
   description = "Cross account AWS IAM roles allowed write access to managed Apiary S3 buckets using assume policy."
   type        = list(any)