Fix s3 lifecycle issues (#190)

* fix S3 lifecycle issues * fix syntax * test syntax * test syntax * test syntax * test syntax * test syntax * test syntax * test syntax * test syntax * test syntax * test syntax * test syntax * syntax refactor * syntax refactor * syntax refactor * syntax refactor * syntax refactor * syntax refactor * syntax refactor * syntax refactor * add comments * update changelog Co-authored-by: Scott Barnhart <[email protected]>
ExpediaGroup · Apr 28, 2021 · be9f087 · be9f087
1 parent 55fd73f
commit be9f087
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,14 @@ 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.7.9] - 2021-04-28
+### Fixed
+- If the S3 bucket specifies an expiration TTL in days that is <= the Intelligent-Tiering transition days, don't create
+  a lifecycle `transition` policy. This will prevent errors like:
+  ```
+  Error: Error putting S3 lifecycle: InvalidArgument: 'Days' in the Expiration action for filter '(prefix=)' must be greater than 'Days' in the Transition action
+  ``` 
+
 ## [6.7.8] - 2021-04-01
 ### Changed
 - Added `DenyUnsecureCommunication` policy for `s3-other.tf` buckets.

diff --git a/common.tf b/common.tf
@@ -19,6 +19,12 @@ locals {
         resource_suffix : replace(schema["schema_name"], "_", "-"),
         data_bucket : "${local.apiary_bucket_prefix}-${replace(schema["schema_name"], "_", "-")}"
         customer_accounts : lookup(schema, "customer_accounts", join(",", var.apiary_customer_accounts))
+        s3_lifecycle_policy_transition_period: lookup(schema, "s3_lifecycle_policy_transition_period", var.s3_lifecycle_policy_transition_period)
+        # Need to change the default "null" value of s3_object_expiration_days to a number so we can compare it
+        # later to s3_lifecycle_policy_transition_period without getting a TF error.  However, TF is doing weird things
+        # when comparing them as actual "number" type (-1), so use a string type ("-1"), which works as expected.
+        s3_object_expiration_days_num: coalesce(lookup(schema, "s3_object_expiration_days", "-1"), "-1")
+        s3_storage_class = lookup(schema, "s3_storage_class", var.s3_storage_class)
       },
     schema)
   ]

diff --git a/s3.tf b/s3.tf
@@ -27,6 +27,7 @@ data "template_file" "bucket_policy" {
   }
 }
 
+
 ##
 ### Apiary S3 data buckets
 ##
@@ -53,15 +54,18 @@ resource "aws_s3_bucket" "apiary_data_bucket" {
 
     abort_incomplete_multipart_upload_days = var.s3_lifecycle_abort_incomplete_multipart_upload_days
 
-    transition {
-      days          = lookup(each.value, "s3_lifecycle_policy_transition_period", var.s3_lifecycle_policy_transition_period)
-      storage_class = lookup(each.value, "s3_storage_class", var.s3_storage_class)
+    dynamic "transition" {
+      for_each = each.value["s3_object_expiration_days_num"] == "-1" || each.value["s3_lifecycle_policy_transition_period"] < each.value["s3_object_expiration_days_num"] ? [1] : []
+      content {
+        days          = each.value["s3_lifecycle_policy_transition_period"]
+        storage_class = each.value["s3_storage_class"]
+      }
     }
 
     dynamic "expiration" {
-      for_each = lookup(each.value, "s3_object_expiration_days", null) != null ? [1] : []
+      for_each = each.value["s3_object_expiration_days_num"] != "-1" ? [1] : []
       content {
-        days = lookup(each.value, "s3_object_expiration_days", null)
+        days = each.value["s3_object_expiration_days_num"]
       }
     }
   }