Merge pull request #675 from GoogleCloudPlatform/fast/fix-pubsub-audi…

…t-logs

Fix FAST audit logs when using pubsub as destination

fast/stages/00-bootstrap/log-export.tf

@@ -59,15 +59,15 @@ module "log-export-gcs" {
 
 module "log-export-logbucket" {
   source      = "../../../modules/logging-bucket"
-  count       = contains(local.log_types, "logging") ? 1 : 0
+  for_each    = toset([for k, v in var.log_sinks : k if v.type == "logging"])
   parent_type = "project"
   parent      = module.log-export-project.project_id
-  id          = "audit-logs-0"
+  id          = "audit-logs-${each.key}"
 }
 
 module "log-export-pubsub" {
   source     = "../../../modules/pubsub"
-  for_each   = toset([for k, v in var.log_sinks : k if v == "pubsub"])
+  for_each   = toset([for k, v in var.log_sinks : k if v.type == "pubsub"])
   project_id = module.log-export-project.project_id
   name       = "audit-logs-${each.key}"
 }

fast/stages/00-bootstrap/organization.tf

@@ -85,11 +85,6 @@ locals {
   _iam_bootstrap_user = (
     var.bootstrap_user == null ? [] : ["user:${var.bootstrap_user}"]
   )
-  _log_sink_destinations = {
-    bigquery = try(module.log-export-dataset.0.id, null),
-    logging  = try(module.log-export-logbucket.0.id, null),
-    storage  = try(module.log-export-gcs.0.name, null)
-  }
   iam = {
     for role in local.iam_roles : role => distinct(concat(
       try(sort(local._iam[role]), []),
@@ -108,13 +103,16 @@ locals {
   iam_roles_additive = distinct(concat(
     keys(local._iam_additive), keys(var.iam_additive)
   ))
-  log_sink_destinations = {
-    for k, v in var.log_sinks : k => (
-      v.type == "pubsub"
-      ? module.log-export-pubsub[k]
-      : local._log_sink_destinations[v.type]
-    )
-  }
+  log_sink_destinations = merge(
+    # use the same dataset for all sinks with `bigquery` as  destination
+    { for k, v in var.log_sinks : k => module.log-export-dataset.0 if v.type == "bigquery" },
+    # use the same gcs bucket for all sinks with `storage` as destination
+    { for k, v in var.log_sinks : k => module.log-export-gcs.0 if v.type == "storage" },
+    # use separate pubsub topics and logging buckets for sinks with
+    # destination `pubsub` and `logging`
+    module.log-export-pubsub,
+    module.log-export-logbucket
+  )
 }
 
 module "organization" {
@@ -177,7 +175,7 @@ module "organization" {
   logging_sinks = {
     for name, attrs in var.log_sinks : name => {
       bq_partitioned_table = attrs.type == "bigquery"
-      destination          = local.log_sink_destinations[name]
+      destination          = local.log_sink_destinations[name].id
       exclusions           = {}
       filter               = attrs.filter
       iam                  = true

modules/folder/README.md

@@ -141,7 +141,7 @@ module "folder-sink" {
   logging_sinks = {
     warnings = {
       type             = "storage"
-      destination      = module.gcs.name
+      destination      = module.gcs.id
       filter           = "severity=WARNING"
       include_children = true
       exclusions       = {}

modules/gcs/README.md

@@ -135,9 +135,10 @@ module "bucket-gcs-notification" {
 | name | description | sensitive |
 |---|---|:---:|
 | [bucket](outputs.tf#L17) | Bucket resource. |  |
-| [name](outputs.tf#L22) | Bucket name. |  |
-| [notification](outputs.tf#L30) | GCS Notification self link. |  |
-| [topic](outputs.tf#L34) | Topic ID used by GCS. |  |
-| [url](outputs.tf#L38) | Bucket URL. |  |
+| [id](outputs.tf#L28) | Bucket ID (same as name). |  |
+| [name](outputs.tf#L37) | Bucket name. |  |
+| [notification](outputs.tf#L46) | GCS Notification self link. |  |
+| [topic](outputs.tf#L51) | Topic ID used by GCS. |  |
+| [url](outputs.tf#L56) | Bucket URL. |  |
 
 <!-- END TFDOC -->

modules/gcs/outputs.tf

@@ -19,6 +19,21 @@ output "bucket" {
   value       = google_storage_bucket.bucket
 }
 
+# We add `id` as an alias to `name` to simplify log sink handling.
+# Since all other log destinations (pubsub, logging-bucket, bigquery)
+# have an id output, it is convenient to have in this module too to
+# handle all log destination as homogeneous objects (i.e. you can
+# assume any valid log destination has an `id` output).
+
+output "id" {
+  description = "Bucket ID (same as name)."
+  value       = "${local.prefix}${lower(var.name)}"
+  depends_on = [
+    google_storage_bucket.bucket,
+    google_storage_bucket_iam_binding.bindings
+  ]
+}
+
 output "name" {
   description = "Bucket name."
   value       = "${local.prefix}${lower(var.name)}"
@@ -27,14 +42,17 @@ output "name" {
     google_storage_bucket_iam_binding.bindings
   ]
 }
+
 output "notification" {
   description = "GCS Notification self link."
   value       = local.notification ? google_storage_notification.notification[0].self_link : null
 }
+
 output "topic" {
   description = "Topic ID used by GCS."
   value       = local.notification ? google_pubsub_topic.topic[0].id : null
 }
+
 output "url" {
   description = "Bucket URL."
   value       = google_storage_bucket.bucket.url

modules/organization/README.md

@@ -179,7 +179,7 @@ module "org" {
   logging_sinks = {
     warnings = {
       type                 = "storage"
-      destination          = module.gcs.name
+      destination          = module.gcs.id
       filter               = "severity=WARNING"
       include_children     = true
       bq_partitioned_table = null

modules/project/README.md

@@ -221,7 +221,7 @@ module "project-host" {
   logging_sinks = {
     warnings = {
       type          = "storage"
-      destination   = module.gcs.name
+      destination   = module.gcs.id
       filter        = "severity=WARNING"
       iam           = false
       unique_writer = false