Bug fixes (#147)

Author: Nuru

.github/workflows/auto-readme.yml

@@ -1,5 +1,7 @@
 name: "auto-readme"
 on:
+  workflow_dispatch:
+
   schedule:
   # Example of job definition:
   # .---------------- minute (0 - 59)
@@ -15,21 +17,35 @@ on:
 
 jobs:
   update:
-    if: github.event_name == 'schedule'
+    if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v2
 
+    - name: Find default branch name
+      id: defaultBranch
+      shell: bash
+      env:
+        GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
+      run: |
+        default_branch=$(gh repo view --json defaultBranchRef --jq .defaultBranchRef.name)
+        printf "::set-output name=defaultBranch::%s\n" "${default_branch}"
+        printf "defaultBranchRef.name=%s\n" "${default_branch}"
+
     - name: Update readme
       shell: bash
       id: update
       env:
         GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
+        DEF: "${{ steps.defaultBranch.outputs.defaultBranch }}"
       run: |
         make init
         make readme/build
         # Ignore changes if they are only whitespace
-        git diff --ignore-all-space --ignore-blank-lines --quiet README.md && { git restore README.md; echo Ignoring whitespace-only changes in README; }
+        if ! git diff --quiet README.md && git diff --ignore-all-space --ignore-blank-lines --quiet README.md; then
+          git restore README.md
+          echo Ignoring whitespace-only changes in README
+        fi
 
     - name: Create Pull Request
       # This action will not create or change a pull request if there are no changes to make.
@@ -47,7 +63,7 @@ jobs:
           To have most recent changes of README.md and doc from origin templates
 
         branch: auto-update/readme
-        base: main
+        base: ${{ steps.defaultBranch.outputs.defaultBranch }}
         delete-branch: true
         labels: |
           auto-update

README.md

@@ -254,14 +254,14 @@ Available targets:
 | Name | Version |
 |------|---------|
 | <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.0 |
-| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 4.2.0 |
+| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 4.9.0 |
 | <a name="requirement_time"></a> [time](#requirement\_time) | >= 0.7 |
 
 ## Providers
 
 | Name | Version |
 |------|---------|
-| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 4.2.0 |
+| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 4.9.0 |
 | <a name="provider_time"></a> [time](#provider\_time) | >= 0.7 |
 
 ## Modules

docs/terraform.md

@@ -4,14 +4,14 @@
 | Name | Version |
 |------|---------|
 | <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.0 |
-| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 4.2.0 |
+| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 4.9.0 |
 | <a name="requirement_time"></a> [time](#requirement\_time) | >= 0.7 |
 
 ## Providers
 
 | Name | Version |
 |------|---------|
-| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 4.2.0 |
+| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 4.9.0 |
 | <a name="provider_time"></a> [time](#provider\_time) | >= 0.7 |
 
 ## Modules

examples/complete/lifecycle.us-east-2.tfvars

@@ -9,33 +9,105 @@ name = "s3-lifecycle-test"
 acl = "private"
 
 lifecycle_configuration_rules = [
+  # Be sure to cover https://github.com/cloudposse/terraform-aws-s3-bucket/issues/137
   {
-    enabled = true # bool
-    id      = "v2rule"
+    abort_incomplete_multipart_upload_days = 1
+    enabled                                = true
+    expiration = {
+      days                         = null
+      expired_object_delete_marker = null
+    }
+
+    # test no filter
+    filter_and = {}
+    id         = "nofilter"
+    noncurrent_version_expiration = {
+      newer_noncurrent_versions = 2
+      noncurrent_days           = 30
+    }
+    noncurrent_version_transition = []
+    transition = [
+      {
+        days          = 7
+        storage_class = "GLACIER"
+      },
+    ]
 
-    abort_incomplete_multipart_upload_days = 1 # number
+  },
+  {
+    abort_incomplete_multipart_upload_days = 1
+    enabled                                = true
+    expiration = {
+      days                         = null
+      expired_object_delete_marker = null
+    }
+
+    # test prefix only
+    filter_and = {
+      prefix = "prefix1"
+    }
+    id = "prefix1"
+    noncurrent_version_expiration = {
+      newer_noncurrent_versions = 2
+      noncurrent_days           = 30
+    }
+    noncurrent_version_transition = []
+    transition = [
+      {
+        days          = 7
+        storage_class = "GLACIER"
+      },
+    ]
 
-    filter_and = null
+  },
+  {
+    abort_incomplete_multipart_upload_days = null
+    enabled                                = true
     expiration = {
-      days = 120 # integer > 0
+      days                         = 1461
+      expired_object_delete_marker = false
     }
+    # test prefix with other filter
+    filter_and = {
+      prefix                   = "prefix2"
+      object_size_greater_than = 128 * 1024
+    }
+    id = "prefix2"
     noncurrent_version_expiration = {
-      newer_noncurrent_versions = 3  # integer > 0
-      noncurrent_days           = 60 # integer >= 0
+      newer_noncurrent_versions = 2
+      noncurrent_days           = 14
     }
-    transition = [{
-      days          = 30            # integer >= 0
-      storage_class = "STANDARD_IA" # string/enum, one of GLACIER, STANDARD_IA, ONEZONE_IA, INTELLIGENT_TIERING, DEEP_ARCHIVE, GLACIER_IR.
+    noncurrent_version_transition = []
+    transition = [
+      {
+        days          = 366
+        storage_class = "GLACIER"
       },
+    ]
+  },
+  {
+    abort_incomplete_multipart_upload_days = null
+    enabled                                = true
+    expiration = {
+      days                         = 93
+      expired_object_delete_marker = false
+    }
+    # test filter without prefix
+    filter_and = {
+      object_size_greater_than = 256 * 1024
+    }
+    id = "big"
+    noncurrent_version_expiration = {
+      newer_noncurrent_versions = 2
+      noncurrent_days           = 14
+    }
+    noncurrent_version_transition = []
+    transition = [
       {
-        days          = 60           # integer >= 0
-        storage_class = "ONEZONE_IA" # string/enum, one of GLACIER, STANDARD_IA, ONEZONE_IA, INTELLIGENT_TIERING, DEEP_ARCHIVE, GLACIER_IR.
-    }]
-    noncurrent_version_transition = [{
-      newer_noncurrent_versions = 3            # integer >= 0
-      noncurrent_days           = 30           # integer >= 0
-      storage_class             = "ONEZONE_IA" # string/enum, one of GLACIER, STANDARD_IA, ONEZONE_IA, INTELLIGENT_TIERING, DEEP_ARCHIVE, GLACIER_IR.
-    }]
+        days          = 90
+        storage_class = "GLACIER"
+      },
+    ]
   }
 ]
 
@@ -84,6 +156,7 @@ lifecycle_rules = [
   }
 ]
 
+
 force_destroy = true
 
 versioning_enabled = false

lifecycle.tf

@@ -5,6 +5,7 @@ locals {
     id      = null # string, must be specified and unique
 
     abort_incomplete_multipart_upload_days = null # number
+    filter_prefix_only                     = null # string See https://github.com/hashicorp/terraform-provider-aws/issues/23882
     filter_and = {
       object_size_greater_than = null # integer >= 0
       object_size_less_than    = null # integer >= 1
@@ -39,9 +40,16 @@ locals {
     id      = rule.id
 
     abort_incomplete_multipart_upload_days = rule.abort_incomplete_multipart_upload_days # number
+
+    # Due to https://github.com/hashicorp/terraform-provider-aws/issues/23882
+    # we have to treat having only the `prefix` set differently than having any other setting.
+    filter_prefix_only = (try(rule.filter_and.object_size_greater_than, null) == null &&
+      try(rule.filter_and.object_size_less_than, null) == null &&
+      try(length(rule.filter_and.tags), 0) == 0 &&
+    try(length(rule.filter_and.prefix), 0) > 0) ? rule.filter_and.prefix : null
+
     filter_and = (try(rule.filter_and.object_size_greater_than, null) == null &&
       try(rule.filter_and.object_size_less_than, null) == null &&
-      try(length(rule.filter_and.prefix), 0) == 0 &&
       try(length(rule.filter_and.tags), 0) == 0) ? null : {
       object_size_greater_than = try(rule.filter_and.object_size_greater_than, null)
       object_size_less_than    = try(rule.filter_and.object_size_less_than, null)
@@ -84,7 +92,9 @@ locals {
     id      = try(var.lifecycle_rule_ids[i], "rule-${i + 1}")
 
     abort_incomplete_multipart_upload_days = rule.abort_incomplete_multipart_upload_days # number
-    filter_and = try(length(rule.prefix), 0) == 0 && try(length(rule.tags), 0) == 0 ? null : {
+
+    filter_prefix_only = try(length(rule.prefix), 0) > 0 && try(length(rule.tags), 0) == 0 ? rule.prefix : null
+    filter_and = try(length(rule.tags), 0) == 0 ? null : {
       object_size_greater_than = null                                   # integer >= 0
       object_size_less_than    = null                                   # integer >= 1
       prefix                   = rule.prefix == "" ? null : rule.prefix # string
@@ -133,7 +143,7 @@ locals {
       # enabled the transition for both current and non-current version.
       rule.enable_deeparchive_transition != true ? [] :
       [{
-        newer_noncurrent_versions = null                                                # string
+        newer_noncurrent_versions = null                                                # integer >= 0
         noncurrent_days           = rule.noncurrent_version_deeparchive_transition_days # integer >= 0
         storage_class             = "DEEP_ARCHIVE"
       }],
@@ -156,14 +166,28 @@ resource "aws_s3_bucket_lifecycle_configuration" "default" {
       status = rule.value.enabled == true ? "Enabled" : "Disabled"
 
       # Filter is always required due to https://github.com/hashicorp/terraform-provider-aws/issues/23299
-      filter {
-        dynamic "and" {
-          for_each = rule.value.filter_and == null ? [] : [rule.value.filter_and]
-          content {
-            object_size_greater_than = and.value.object_size_greater_than
-            object_size_less_than    = and.value.object_size_less_than
-            prefix                   = and.value.prefix
-            tags                     = and.value.tags
+      dynamic "filter" {
+        for_each = rule.value.filter_prefix_only == null && rule.value.filter_and == null ? ["empty"] : []
+        content {}
+      }
+
+      # When only specifying `prefix`, do not use `and` due to https://github.com/hashicorp/terraform-provider-aws/issues/23882
+      dynamic "filter" {
+        for_each = rule.value.filter_prefix_only == null ? [] : ["prefix"]
+        content {
+          prefix = rule.value.filter_prefix_only
+        }
+      }
+
+      # When specifying more than 1 filter criterion, use `and`
+      dynamic "filter" {
+        for_each = rule.value.filter_and == null ? [] : ["and"]
+        content {
+          and {
+            object_size_greater_than = rule.value.filter_and.object_size_greater_than
+            object_size_less_than    = rule.value.filter_and.object_size_less_than
+            prefix                   = rule.value.filter_and.prefix
+            tags                     = rule.value.filter_and.tags
           }
         }
       }

main.tf

@@ -444,7 +444,7 @@ data "aws_iam_policy_document" "aggregated_policy" {
 }
 
 resource "aws_s3_bucket_policy" "default" {
-  count      = local.enabled && (var.allow_ssl_requests_only || var.allow_encrypted_uploads_only || length(var.s3_replication_source_roles) > 0 || length(var.privileged_principal_arns) > 0 || var.policy != "") ? 1 : 0
+  count      = local.enabled && (var.allow_ssl_requests_only || var.allow_encrypted_uploads_only || length(var.s3_replication_source_roles) > 0 || length(var.privileged_principal_arns) > 0 || length(var.source_policy_documents) > 0) ? 1 : 0
   bucket     = join("", aws_s3_bucket.default.*.id)
   policy     = join("", data.aws_iam_policy_document.aggregated_policy.*.json)
   depends_on = [aws_s3_bucket_public_access_block.default]

versions.tf

@@ -4,7 +4,7 @@ terraform {
   required_providers {
     aws = {
       source  = "hashicorp/aws"
-      version = ">= 4.2.0"
+      version = ">= 4.9.0"
     }
     time = {
       source  = "hashicorp/time"