GoogleCloudPlatform · ludoo · Dec 22, 2022 · Dec 22, 2022
diff --git a/modules/gcs/README.md b/modules/gcs/README.md
@@ -1,4 +1,5 @@
 # Google Cloud Storage Module
+
 ## Example
 
 ```hcl
@@ -41,12 +42,10 @@ module "bucket" {
   iam = {
     "roles/storage.admin" = ["group:[email protected]"]
   }
-
   retention_policy = {
     retention_period = 100
     is_locked        = true
   }
-
   logging_config = {
     log_bucket        = var.bucket
     log_object_prefix = null
@@ -63,32 +62,26 @@ module "bucket" {
   project_id = "myproject"
   prefix     = "test"
   name       = "my-bucket"
-
   iam = {
     "roles/storage.admin" = ["group:[email protected]"]
   }
-
-  lifecycle_rule = {
-    action = {
-      type          = "SetStorageClass"
-      storage_class = "STANDARD"
-    }
-    condition = {
-      age                        = 30
-      created_before             = null
-      with_state                 = null
-      matches_storage_class      = null
-      num_newer_versions         = null
-      custom_time_before         = null
-      days_since_custom_time     = null
-      days_since_noncurrent_time = null
-      noncurrent_time_before     = null
+  lifecycle_rules = {
+    lr-0 = {
+      action = {
+        type          = "SetStorageClass"
+        storage_class = "STANDARD"
+      }
+      condition = {
+        age = 30
+      }
     }
   }
 }
 # tftest modules=1 resources=2
 ```
+
 ### Minimal example with GCS notifications
+
 ```hcl
 module "bucket-gcs-notification" {
   source     = "./fabric/modules/gcs"
@@ -112,23 +105,23 @@ module "bucket-gcs-notification" {
 
 | name | description | type | required | default |
 |---|---|:---:|:---:|:---:|
-| [name](variables.tf#L89) | Bucket name suffix. | <code>string</code> | ✓ |  |
-| [project_id](variables.tf#L117) | Bucket project id. | <code>string</code> | ✓ |  |
-| [cors](variables.tf#L17) | CORS configuration for the bucket. Defaults to null. | <code title="object&#40;&#123;&#10;  origin          &#61; list&#40;string&#41;&#10;  method          &#61; list&#40;string&#41;&#10;  response_header &#61; list&#40;string&#41;&#10;  max_age_seconds &#61; number&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
+| [name](variables.tf#L116) | Bucket name suffix. | <code>string</code> | ✓ |  |
+| [project_id](variables.tf#L145) | Bucket project id. | <code>string</code> | ✓ |  |
+| [cors](variables.tf#L17) | CORS configuration for the bucket. Defaults to null. | <code title="object&#40;&#123;&#10;  origin          &#61; optional&#40;list&#40;string&#41;&#41;&#10;  method          &#61; optional&#40;list&#40;string&#41;&#41;&#10;  response_header &#61; optional&#40;list&#40;string&#41;&#41;&#10;  max_age_seconds &#61; optional&#40;number&#41;&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
 | [encryption_key](variables.tf#L28) | KMS key that will be used for encryption. | <code>string</code> |  | <code>null</code> |
 | [force_destroy](variables.tf#L34) | Optional map to set force destroy keyed by name, defaults to false. | <code>bool</code> |  | <code>false</code> |
 | [iam](variables.tf#L40) | IAM bindings in {ROLE => [MEMBERS]} format. | <code>map&#40;list&#40;string&#41;&#41;</code> |  | <code>&#123;&#125;</code> |
 | [labels](variables.tf#L46) | Labels to be attached to all buckets. | <code>map&#40;string&#41;</code> |  | <code>&#123;&#125;</code> |
-| [lifecycle_rule](variables.tf#L52) | Bucket lifecycle rule. | <code title="object&#40;&#123;&#10;  action &#61; object&#40;&#123;&#10;    type          &#61; string&#10;    storage_class &#61; string&#10;  &#125;&#41;&#10;  condition &#61; object&#40;&#123;&#10;    age                        &#61; number&#10;    created_before             &#61; string&#10;    with_state                 &#61; string&#10;    matches_storage_class      &#61; list&#40;string&#41;&#10;    num_newer_versions         &#61; string&#10;    custom_time_before         &#61; string&#10;    days_since_custom_time     &#61; string&#10;    days_since_noncurrent_time &#61; string&#10;    noncurrent_time_before     &#61; string&#10;  &#125;&#41;&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
-| [location](variables.tf#L74) | Bucket location. | <code>string</code> |  | <code>&#34;EU&#34;</code> |
-| [logging_config](variables.tf#L80) | Bucket logging configuration. | <code title="object&#40;&#123;&#10;  log_bucket        &#61; string&#10;  log_object_prefix &#61; string&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
-| [notification_config](variables.tf#L94) | GCS Notification configuration. | <code title="object&#40;&#123;&#10;  enabled           &#61; bool&#10;  payload_format    &#61; string&#10;  topic_name        &#61; string&#10;  sa_email          &#61; string&#10;  event_types       &#61; list&#40;string&#41;&#10;  custom_attributes &#61; map&#40;string&#41;&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
-| [prefix](variables.tf#L107) | Optional prefix used to generate the bucket name. | <code>string</code> |  | <code>null</code> |
-| [retention_policy](variables.tf#L122) | Bucket retention policy. | <code title="object&#40;&#123;&#10;  retention_period &#61; number&#10;  is_locked        &#61; bool&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
-| [storage_class](variables.tf#L131) | Bucket storage class. | <code>string</code> |  | <code>&#34;MULTI_REGIONAL&#34;</code> |
-| [uniform_bucket_level_access](variables.tf#L141) | Allow using object ACLs (false) or not (true, this is the recommended behavior) , defaults to true (which is the recommended practice, but not the behavior of storage API). | <code>bool</code> |  | <code>true</code> |
-| [versioning](variables.tf#L147) | Enable versioning, defaults to false. | <code>bool</code> |  | <code>false</code> |
-| [website](variables.tf#L153) | Bucket website. | <code title="object&#40;&#123;&#10;  main_page_suffix &#61; string&#10;  not_found_page   &#61; string&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
+| [lifecycle_rules](variables.tf#L52) | Bucket lifecycle rule. | <code title="map&#40;object&#40;&#123;&#10;  action &#61; object&#40;&#123;&#10;    type          &#61; string&#10;    storage_class &#61; optional&#40;string&#41;&#10;  &#125;&#41;&#10;  condition &#61; object&#40;&#123;&#10;    age                        &#61; optional&#40;number&#41;&#10;    created_before             &#61; optional&#40;string&#41;&#10;    custom_time_before         &#61; optional&#40;string&#41;&#10;    days_since_custom_time     &#61; optional&#40;number&#41;&#10;    days_since_noncurrent_time &#61; optional&#40;number&#41;&#10;    matches_prefix             &#61; optional&#40;list&#40;string&#41;&#41;&#10;    matches_storage_class      &#61; optional&#40;list&#40;string&#41;&#41; &#35; STANDARD, MULTI_REGIONAL, REGIONAL, NEARLINE, COLDLINE, ARCHIVE, DURABLE_REDUCED_AVAILABILITY&#10;    matches_suffix             &#61; optional&#40;list&#40;string&#41;&#41;&#10;    noncurrent_time_before     &#61; optional&#40;string&#41;&#10;    num_newer_versions         &#61; optional&#40;number&#41;&#10;    with_state                 &#61; optional&#40;string&#41; &#35; &#34;LIVE&#34;, &#34;ARCHIVED&#34;, &#34;ANY&#34;&#10;  &#125;&#41;&#10;&#125;&#41;&#41;">map&#40;object&#40;&#123;&#8230;&#125;&#41;&#41;</code> |  | <code>&#123;&#125;</code> |
+| [location](variables.tf#L101) | Bucket location. | <code>string</code> |  | <code>&#34;EU&#34;</code> |
+| [logging_config](variables.tf#L107) | Bucket logging configuration. | <code title="object&#40;&#123;&#10;  log_bucket        &#61; string&#10;  log_object_prefix &#61; optional&#40;string&#41;&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
+| [notification_config](variables.tf#L121) | GCS Notification configuration. | <code title="object&#40;&#123;&#10;  enabled            &#61; bool&#10;  payload_format     &#61; string&#10;  topic_name         &#61; string&#10;  sa_email           &#61; string&#10;  event_types        &#61; optional&#40;list&#40;string&#41;&#41;&#10;  custom_attributes  &#61; optional&#40;map&#40;string&#41;&#41;&#10;  object_name_prefix &#61; optional&#40;string&#41;&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
+| [prefix](variables.tf#L135) | Optional prefix used to generate the bucket name. | <code>string</code> |  | <code>null</code> |
+| [retention_policy](variables.tf#L150) | Bucket retention policy. | <code title="object&#40;&#123;&#10;  retention_period &#61; number&#10;  is_locked        &#61; optional&#40;bool&#41;&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
+| [storage_class](variables.tf#L159) | Bucket storage class. | <code>string</code> |  | <code>&#34;MULTI_REGIONAL&#34;</code> |
+| [uniform_bucket_level_access](variables.tf#L169) | Allow using object ACLs (false) or not (true, this is the recommended behavior) , defaults to true (which is the recommended practice, but not the behavior of storage API). | <code>bool</code> |  | <code>true</code> |
+| [versioning](variables.tf#L175) | Enable versioning, defaults to false. | <code>bool</code> |  | <code>false</code> |
+| [website](variables.tf#L181) | Bucket website. | <code title="object&#40;&#123;&#10;  main_page_suffix &#61; optional&#40;string&#41;&#10;  not_found_page   &#61; optional&#40;string&#41;&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
 
 ## Outputs
 

diff --git a/modules/gcs/main.tf b/modules/gcs/main.tf
@@ -75,22 +75,25 @@ resource "google_storage_bucket" "bucket" {
   }
 
   dynamic "lifecycle_rule" {
-    for_each = var.lifecycle_rule == null ? [] : [""]
+    for_each = var.lifecycle_rules
+    iterator = rule
     content {
       action {
-        type          = var.lifecycle_rule.action["type"]
-        storage_class = var.lifecycle_rule.action["storage_class"]
+        type          = rule.value.action.type
+        storage_class = rule.value.action.storage_class
       }
       condition {
-        age                        = var.lifecycle_rule.condition["age"]
-        created_before             = var.lifecycle_rule.condition["created_before"]
-        with_state                 = var.lifecycle_rule.condition["with_state"]
-        matches_storage_class      = var.lifecycle_rule.condition["matches_storage_class"]
-        num_newer_versions         = var.lifecycle_rule.condition["num_newer_versions"]
-        custom_time_before         = var.lifecycle_rule.condition["custom_time_before"]
-        days_since_custom_time     = var.lifecycle_rule.condition["days_since_custom_time"]
-        days_since_noncurrent_time = var.lifecycle_rule.condition["days_since_noncurrent_time"]
-        noncurrent_time_before     = var.lifecycle_rule.condition["noncurrent_time_before"]
+        age                        = rule.value.condition.age
+        created_before             = rule.value.condition.created_before
+        custom_time_before         = rule.value.condition.custom_time_before
+        days_since_custom_time     = rule.value.condition.days_since_custom_time
+        days_since_noncurrent_time = rule.value.condition.days_since_noncurrent_time
+        matches_prefix             = rule.value.condition.matches_prefix
+        matches_storage_class      = rule.value.condition.matches_storage_class
+        matches_suffix             = rule.value.condition.matches_suffix
+        noncurrent_time_before     = rule.value.condition.noncurrent_time_before
+        num_newer_versions         = rule.value.condition.num_newer_versions
+        with_state                 = rule.value.condition.with_state
       }
     }
   }
@@ -104,15 +107,14 @@ resource "google_storage_bucket_iam_binding" "bindings" {
 }
 
 resource "google_storage_notification" "notification" {
-  count             = local.notification ? 1 : 0
-  bucket            = google_storage_bucket.bucket.name
-  payload_format    = var.notification_config.payload_format
-  topic             = google_pubsub_topic.topic[0].id
-  event_types       = var.notification_config.event_types
-  custom_attributes = var.notification_config.custom_attributes
-
-  depends_on = [google_pubsub_topic_iam_binding.binding]
-
+  count              = local.notification ? 1 : 0
+  bucket             = google_storage_bucket.bucket.name
+  payload_format     = var.notification_config.payload_format
+  topic              = google_pubsub_topic.topic[0].id
+  custom_attributes  = var.notification_config.custom_attributes
+  event_types        = var.notification_config.event_types
+  object_name_prefix = var.notification_config.object_name_prefix
+  depends_on         = [google_pubsub_topic_iam_binding.binding]
 }
 resource "google_pubsub_topic_iam_binding" "binding" {
   count   = local.notification ? 1 : 0

diff --git a/modules/gcs/variables.tf b/modules/gcs/variables.tf
@@ -17,10 +17,10 @@
 variable "cors" {
   description = "CORS configuration for the bucket. Defaults to null."
   type = object({
-    origin          = list(string)
-    method          = list(string)
-    response_header = list(string)
-    max_age_seconds = number
+    origin          = optional(list(string))
+    method          = optional(list(string))
+    response_header = optional(list(string))
+    max_age_seconds = optional(number)
   })
   default = null
 }
@@ -49,26 +49,53 @@ variable "labels" {
   default     = {}
 }
 
-variable "lifecycle_rule" {
+variable "lifecycle_rules" {
   description = "Bucket lifecycle rule."
-  type = object({
+  type = map(object({
     action = object({
       type          = string
-      storage_class = string
+      storage_class = optional(string)
     })
     condition = object({
-      age                        = number
-      created_before             = string
-      with_state                 = string
-      matches_storage_class      = list(string)
-      num_newer_versions         = string
-      custom_time_before         = string
-      days_since_custom_time     = string
-      days_since_noncurrent_time = string
-      noncurrent_time_before     = string
+      age                        = optional(number)
+      created_before             = optional(string)
+      custom_time_before         = optional(string)
+      days_since_custom_time     = optional(number)
+      days_since_noncurrent_time = optional(number)
+      matches_prefix             = optional(list(string))
+      matches_storage_class      = optional(list(string)) # STANDARD, MULTI_REGIONAL, REGIONAL, NEARLINE, COLDLINE, ARCHIVE, DURABLE_REDUCED_AVAILABILITY
+      matches_suffix             = optional(list(string))
+      noncurrent_time_before     = optional(string)
+      num_newer_versions         = optional(number)
+      with_state                 = optional(string) # "LIVE", "ARCHIVED", "ANY"
     })
-  })
-  default = null
+  }))
+  default  = {}
+  nullable = false
+  validation {
+    condition = alltrue([
+      for k, v in var.lifecycle_rules : v.action != null && v.condition != null
+    ])
+    error_message = "Lifecycle rules action and condition cannot be null."
+  }
+  validation {
+    condition = alltrue([
+      for k, v in var.lifecycle_rules : contains(
+        ["Delete", "SetStorageClass", "AbortIncompleteMultipartUpload"],
+        v.action.type
+      )
+    ])
+    error_message = "Lifecycle rules action type has unsupported value."
+  }
+  validation {
+    condition = alltrue([
+      for k, v in var.lifecycle_rules :
+      v.action.type != "SetStorageClass"
+      ||
+      v.action.storage_class != null
+    ])
+    error_message = "Lifecycle rules with action type SetStorageClass require a storage class."
+  }
 }
 
 variable "location" {
@@ -81,7 +108,7 @@ variable "logging_config" {
   description = "Bucket logging configuration."
   type = object({
     log_bucket        = string
-    log_object_prefix = string
+    log_object_prefix = optional(string)
   })
   default = null
 }
@@ -94,12 +121,13 @@ variable "name" {
 variable "notification_config" {
   description = "GCS Notification configuration."
   type = object({
-    enabled           = bool
-    payload_format    = string
-    topic_name        = string
-    sa_email          = string
-    event_types       = list(string)
-    custom_attributes = map(string)
+    enabled            = bool
+    payload_format     = string
+    topic_name         = string
+    sa_email           = string
+    event_types        = optional(list(string))
+    custom_attributes  = optional(map(string))
+    object_name_prefix = optional(string)
   })
   default = null
 }
@@ -123,7 +151,7 @@ variable "retention_policy" {
   description = "Bucket retention policy."
   type = object({
     retention_period = number
-    is_locked        = bool
+    is_locked        = optional(bool)
   })
   default = null
 }
@@ -153,8 +181,8 @@ variable "versioning" {
 variable "website" {
   description = "Bucket website."
   type = object({
-    main_page_suffix = string
-    not_found_page   = string
+    main_page_suffix = optional(string)
+    not_found_page   = optional(string)
   })
   default = null
 }
diff --git a/tests/modules/gcs/common.tfvars b/tests/modules/gcs/common.tfvars
@@ -0,0 +1,13 @@
+force_destroy = true
+labels        = { environment = "test" }
+logging_config = {
+  log_bucket = "foo"
+}
+name       = "test"
+project_id = "test-project"
+retention_policy = {
+  retention_period = 5
+  is_locked        = false
+}
+storage_class = "MULTI_REGIONAL"
+versioning    = true
diff --git a/tests/modules/gcs/fixture/main.tf b/tests/modules/gcs/fixture/main.tf