Merge pull request #216 from terraform-google-modules/jccb/logging-iam-fixes

Fix IAM bindings for logging sinks

.gitignore

@@ -15,3 +15,4 @@ credentials.json
 key.json
 terraform-ls.tf
 bundle.zip
+.DS_Store

modules/folder/README.md

@@ -114,7 +114,7 @@ module "folder-sink" {
     no-gce-instances = "resource.type=gce_instance"
   }
 }
-# tftest:modules=5:resources=11
+# tftest:modules=5:resources=12
 ```
 
 ### Hierarchical firewall policies
@@ -186,5 +186,5 @@ module "folder2" {
 | folder | Folder resource. |  |
 | id | Folder id. |  |
 | name | Folder name. |  |
-| sink_writer_identities | None |  |
+| sink_writer_identities | Writer identities created for each sink. |  |
 <!-- END TFDOC -->

modules/folder/main.tf

@@ -202,27 +202,36 @@ resource "google_logging_folder_sink" "sink" {
   }
 }
 
-resource "google_storage_bucket_iam_binding" "gcs-sinks-binding" {
+resource "google_storage_bucket_iam_member" "gcs-sinks-binding" {
   for_each = local.sink_bindings["gcs"]
   bucket   = each.value.destination
   role     = "roles/storage.objectCreator"
-  members  = [google_logging_folder_sink.sink[each.key].writer_identity]
+  member   = google_logging_folder_sink.sink[each.key].writer_identity
 }
 
-resource "google_bigquery_dataset_iam_binding" "bq-sinks-binding" {
+resource "google_bigquery_dataset_iam_member" "bq-sinks-binding" {
   for_each   = local.sink_bindings["bigquery"]
   project    = split("/", each.value.destination)[1]
   dataset_id = split("/", each.value.destination)[3]
   role       = "roles/bigquery.dataEditor"
-  members    = [google_logging_folder_sink.sink[each.key].writer_identity]
+  member     = google_logging_folder_sink.sink[each.key].writer_identity
 }
 
-resource "google_pubsub_topic_iam_binding" "pubsub-sinks-binding" {
+resource "google_pubsub_topic_iam_member" "pubsub-sinks-binding" {
   for_each = local.sink_bindings["pubsub"]
   project  = split("/", each.value.destination)[1]
   topic    = split("/", each.value.destination)[3]
   role     = "roles/pubsub.publisher"
-  members  = [google_logging_folder_sink.sink[each.key].writer_identity]
+  member   = google_logging_folder_sink.sink[each.key].writer_identity
+}
+
+resource "google_project_iam_member" "bucket-sinks-binding" {
+  for_each = local.sink_bindings["logging"]
+  project  = split("/", each.value.destination)[1]
+  role     = "roles/logging.bucketWriter"
+  member   = google_logging_folder_sink.sink[each.key].writer_identity
+  # TODO(jccb): use a condition to limit writer-identity only to this
+  # bucket
 }
 
 resource "google_logging_folder_exclusion" "logging-exclusion" {

modules/folder/outputs.tf

@@ -51,7 +51,7 @@ output "firewall_policy_id" {
 }
 
 output "sink_writer_identities" {
-  description = ""
+  description = "Writer identities created for each sink."
   value = {
     for name, sink in google_logging_folder_sink.sink : name => sink.writer_identity
   }

modules/organization/README.md

@@ -131,7 +131,7 @@ module "org" {
     no-gce-instances = "resource.type=gce_instance"
   }
 }
-# tftest:modules=5:resources=10
+# tftest:modules=5:resources=11
 ```
 
 
@@ -163,5 +163,5 @@ module "org" {
 | firewall_policies | Map of firewall policy resources created in the organization. |  |
 | firewall_policy_id | Map of firewall policy ids created in the organization. |  |
 | organization_id | Organization id dependent on module resources. |  |
-| sink_writer_identities | None |  |
+| sink_writer_identities | Writer identities created for each sink. |  |
 <!-- END TFDOC -->

modules/organization/main.tf

@@ -286,27 +286,36 @@ resource "google_logging_organization_sink" "sink" {
   }
 }
 
-resource "google_storage_bucket_iam_binding" "gcs-sinks-binding" {
+resource "google_storage_bucket_iam_member" "gcs-sinks-binding" {
   for_each = local.sink_bindings["gcs"]
   bucket   = each.value.destination
   role     = "roles/storage.objectCreator"
-  members  = [google_logging_organization_sink.sink[each.key].writer_identity]
+  member   = google_logging_organization_sink.sink[each.key].writer_identity
 }
 
-resource "google_bigquery_dataset_iam_binding" "bq-sinks-binding" {
+resource "google_bigquery_dataset_iam_member" "bq-sinks-binding" {
   for_each   = local.sink_bindings["bigquery"]
   project    = split("/", each.value.destination)[1]
   dataset_id = split("/", each.value.destination)[3]
   role       = "roles/bigquery.dataEditor"
-  members    = [google_logging_organization_sink.sink[each.key].writer_identity]
+  member     = google_logging_organization_sink.sink[each.key].writer_identity
 }
 
-resource "google_pubsub_topic_iam_binding" "pubsub-sinks-binding" {
+resource "google_pubsub_topic_iam_member" "pubsub-sinks-binding" {
   for_each = local.sink_bindings["pubsub"]
   project  = split("/", each.value.destination)[1]
   topic    = split("/", each.value.destination)[3]
   role     = "roles/pubsub.publisher"
-  members  = [google_logging_organization_sink.sink[each.key].writer_identity]
+  member   = google_logging_organization_sink.sink[each.key].writer_identity
+}
+
+resource "google_project_iam_member" "bucket-sinks-binding" {
+  for_each = local.sink_bindings["logging"]
+  project  = split("/", each.value.destination)[1]
+  role     = "roles/logging.bucketWriter"
+  member   = google_logging_organization_sink.sink[each.key].writer_identity
+  # TODO(jccb): use a condition to limit writer-identity only to this
+  # bucket
 }
 
 resource "google_logging_organization_exclusion" "logging-exclusion" {

modules/organization/outputs.tf

@@ -45,7 +45,7 @@ output "firewall_policy_id" {
 }
 
 output "sink_writer_identities" {
-  description = ""
+  description = "Writer identities created for each sink."
   value = {
     for name, sink in google_logging_organization_sink.sink : name => sink.writer_identity
   }

modules/project/README.md

@@ -146,7 +146,7 @@ module "project-host" {
     no-gce-instances = "resource.type=gce_instance"
   }
 }
-# tftest:modules=5:resources=11
+# tftest:modules=5:resources=12
 ```
 
 
@@ -191,6 +191,6 @@ module "project-host" {
 | number | Project number. |  |
 | project_id | Project id. |  |
 | service_accounts | Product robot service accounts in project. |  |
-| sink_writer_identities | None |  |
+| sink_writer_identities | Writer identities created for each sink. |  |
 <!-- END TFDOC -->
 

modules/project/main.tf

@@ -277,27 +277,36 @@ resource "google_logging_project_sink" "sink" {
   }
 }
 
-resource "google_storage_bucket_iam_binding" "gcs-sinks-binding" {
+resource "google_storage_bucket_iam_member" "gcs-sinks-binding" {
   for_each = local.sink_bindings["gcs"]
   bucket   = each.value.destination
   role     = "roles/storage.objectCreator"
-  members  = [google_logging_project_sink.sink[each.key].writer_identity]
+  member   = google_logging_project_sink.sink[each.key].writer_identity
 }
 
-resource "google_bigquery_dataset_iam_binding" "bq-sinks-binding" {
+resource "google_bigquery_dataset_iam_member" "bq-sinks-binding" {
   for_each   = local.sink_bindings["bigquery"]
   project    = split("/", each.value.destination)[1]
   dataset_id = split("/", each.value.destination)[3]
   role       = "roles/bigquery.dataEditor"
-  members    = [google_logging_project_sink.sink[each.key].writer_identity]
+  member     = google_logging_project_sink.sink[each.key].writer_identity
 }
 
-resource "google_pubsub_topic_iam_binding" "pubsub-sinks-binding" {
+resource "google_pubsub_topic_iam_member" "pubsub-sinks-binding" {
   for_each = local.sink_bindings["pubsub"]
   project  = split("/", each.value.destination)[1]
   topic    = split("/", each.value.destination)[3]
   role     = "roles/pubsub.publisher"
-  members  = [google_logging_project_sink.sink[each.key].writer_identity]
+  member   = google_logging_project_sink.sink[each.key].writer_identity
+}
+
+resource "google_project_iam_member" "bucket-sinks-binding" {
+  for_each = local.sink_bindings["logging"]
+  project  = split("/", each.value.destination)[1]
+  role     = "roles/logging.bucketWriter"
+  member   = google_logging_project_sink.sink[each.key].writer_identity
+  # TODO(jccb): use a condition to limit writer-identity only to this
+  # bucket
 }
 
 resource "google_logging_project_exclusion" "logging-exclusion" {
@@ -318,7 +327,7 @@ resource "google_essential_contacts_contact" "contact" {
 }
 
 resource "google_access_context_manager_service_perimeter_resource" "service-perimeter-resource-standard" {
-  count          = var.service_perimeter_standard != null ? 1 : 0
+  count = var.service_perimeter_standard != null ? 1 : 0
 
   # If used, remember to uncomment 'lifecycle' block in the 
   # modules/vpc-sc/google_access_context_manager_service_perimeter resource.
@@ -327,7 +336,7 @@ resource "google_access_context_manager_service_perimeter_resource" "service-per
 }
 
 resource "google_access_context_manager_service_perimeter_resource" "service-perimeter-resource-bridges" {
-  for_each       = toset(var.service_perimeter_bridges != null ? var.service_perimeter_bridges : [])
+  for_each = toset(var.service_perimeter_bridges != null ? var.service_perimeter_bridges : [])
 
   # If used, remember to uncomment 'lifecycle' block in the 
   # modules/vpc-sc/google_access_context_manager_service_perimeter resource.

modules/project/outputs.tf

@@ -68,7 +68,7 @@ output "custom_roles" {
 }
 
 output "sink_writer_identities" {
-  description = ""
+  description = "Writer identities created for each sink."
   value = {
     for name, sink in google_logging_project_sink.sink : name => sink.writer_identity
   }

tests/modules/folder/test_plan_logging.py

@@ -62,15 +62,16 @@ def test_sinks(plan_runner):
   }
   """
     _, resources = plan_runner(FIXTURES_DIR, logging_sinks=logging_sinks)
-    assert len(resources) == 8
+    assert len(resources) == 9
 
     resource_types = Counter([r["type"] for r in resources])
     assert resource_types == {
-        "google_bigquery_dataset_iam_binding": 1,
-        "google_folder": 1,
         "google_logging_folder_sink": 4,
-        "google_pubsub_topic_iam_binding": 1,
-        "google_storage_bucket_iam_binding": 1,
+        "google_folder": 1,
+        "google_bigquery_dataset_iam_member": 1,
+        "google_project_iam_member": 1,
+        "google_pubsub_topic_iam_member": 1,
+        "google_storage_bucket_iam_member": 1,
     }
 
     sinks = [r for r in resources if r["type"] == "google_logging_folder_sink"]
@@ -111,12 +112,13 @@ def test_sinks(plan_runner):
         ("warning", "severity=WARNING", "storage.googleapis.com/mybucket", True),
     ]
 
-    bindings = [r for r in resources if "binding" in r["type"]]
+    bindings = [r for r in resources if "member" in r["type"]]
     values = [(r["index"], r["type"], r["values"]["role"]) for r in bindings]
     assert sorted(values) == [
-        ("info", "google_bigquery_dataset_iam_binding", "roles/bigquery.dataEditor"),
-        ("notice", "google_pubsub_topic_iam_binding", "roles/pubsub.publisher"),
-        ("warning", "google_storage_bucket_iam_binding", "roles/storage.objectCreator"),
+        ("debug", "google_project_iam_member", "roles/logging.bucketWriter"),
+        ("info", "google_bigquery_dataset_iam_member", "roles/bigquery.dataEditor"),
+        ("notice", "google_pubsub_topic_iam_member", "roles/pubsub.publisher"),
+        ("warning", "google_storage_bucket_iam_member", "roles/storage.objectCreator"),
     ]
 
     exclusions = [(r["index"], r["values"]["exclusions"]) for r in sinks]

tests/modules/organization/test_plan_logging.py

@@ -62,14 +62,15 @@ def test_sinks(plan_runner):
   }
   """
     _, resources = plan_runner(FIXTURES_DIR, logging_sinks=logging_sinks)
-    assert len(resources) == 7
+    assert len(resources) == 8
 
     resource_types = Counter([r["type"] for r in resources])
     assert resource_types == {
-        "google_bigquery_dataset_iam_binding": 1,
         "google_logging_organization_sink": 4,
-        "google_pubsub_topic_iam_binding": 1,
-        "google_storage_bucket_iam_binding": 1,
+        "google_bigquery_dataset_iam_member": 1,
+        "google_project_iam_member": 1,
+        "google_pubsub_topic_iam_member": 1,
+        "google_storage_bucket_iam_member": 1,
     }
 
     sinks = [r for r in resources if r["type"] == "google_logging_organization_sink"]
@@ -110,12 +111,13 @@ def test_sinks(plan_runner):
         ("warning", "severity=WARNING", "storage.googleapis.com/mybucket", True),
     ]
 
-    bindings = [r for r in resources if "binding" in r["type"]]
+    bindings = [r for r in resources if "member" in r["type"]]
     values = [(r["index"], r["type"], r["values"]["role"]) for r in bindings]
     assert sorted(values) == [
-        ("info", "google_bigquery_dataset_iam_binding", "roles/bigquery.dataEditor"),
-        ("notice", "google_pubsub_topic_iam_binding", "roles/pubsub.publisher"),
-        ("warning", "google_storage_bucket_iam_binding", "roles/storage.objectCreator"),
+        ("debug", "google_project_iam_member", "roles/logging.bucketWriter"),
+        ("info", "google_bigquery_dataset_iam_member", "roles/bigquery.dataEditor"),
+        ("notice", "google_pubsub_topic_iam_member", "roles/pubsub.publisher"),
+        ("warning", "google_storage_bucket_iam_member", "roles/storage.objectCreator"),
     ]
 
     exclusions = [(r["index"], r["values"]["exclusions"]) for r in sinks]

tests/modules/project/test_plan_logging.py

@@ -62,15 +62,16 @@ def test_sinks(plan_runner):
   }
   """
     _, resources = plan_runner(FIXTURES_DIR, logging_sinks=logging_sinks)
-    assert len(resources) == 8
+    assert len(resources) == 9
 
     resource_types = Counter([r["type"] for r in resources])
     assert resource_types == {
-        "google_bigquery_dataset_iam_binding": 1,
         "google_logging_project_sink": 4,
+        "google_bigquery_dataset_iam_member": 1,
         "google_project": 1,
-        "google_pubsub_topic_iam_binding": 1,
-        "google_storage_bucket_iam_binding": 1,
+        "google_project_iam_member": 1,
+        "google_pubsub_topic_iam_member": 1,
+        "google_storage_bucket_iam_member": 1,
     }
 
     sinks = [r for r in resources if r["type"] == "google_logging_project_sink"]
@@ -111,12 +112,13 @@ def test_sinks(plan_runner):
         ("warning", "severity=WARNING", "storage.googleapis.com/mybucket", False),
     ]
 
-    bindings = [r for r in resources if "binding" in r["type"]]
+    bindings = [r for r in resources if "member" in r["type"]]
     values = [(r["index"], r["type"], r["values"]["role"]) for r in bindings]
     assert sorted(values) == [
-        ("info", "google_bigquery_dataset_iam_binding", "roles/bigquery.dataEditor"),
-        ("notice", "google_pubsub_topic_iam_binding", "roles/pubsub.publisher"),
-        ("warning", "google_storage_bucket_iam_binding", "roles/storage.objectCreator"),
+        ("debug", "google_project_iam_member", "roles/logging.bucketWriter"),
+        ("info", "google_bigquery_dataset_iam_member", "roles/bigquery.dataEditor"),
+        ("notice", "google_pubsub_topic_iam_member", "roles/pubsub.publisher"),
+        ("warning", "google_storage_bucket_iam_member", "roles/storage.objectCreator"),
     ]
 
     exclusions = [(r["index"], r["values"]["exclusions"]) for r in sinks]