From 72ec5607846219628a082cd3baa805f0fd0d96f7 Mon Sep 17 00:00:00 2001 From: Nuru Date: Wed, 20 Apr 2022 16:21:38 -0700 Subject: [PATCH] Bug fixes (#147) --- .github/workflows/auto-readme.yml | 22 +++- README.md | 4 +- docs/terraform.md | 4 +- examples/complete/lifecycle.us-east-2.tfvars | 109 ++++++++++++++++--- lifecycle.tf | 46 ++++++-- main.tf | 2 +- versions.tf | 2 +- 7 files changed, 151 insertions(+), 38 deletions(-) diff --git a/.github/workflows/auto-readme.yml b/.github/workflows/auto-readme.yml index 03232cd6..6f25b8dd 100644 --- a/.github/workflows/auto-readme.yml +++ b/.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 diff --git a/README.md b/README.md index 11623c01..882a7aed 100644 --- a/README.md +++ b/README.md @@ -254,14 +254,14 @@ Available targets: | Name | Version | |------|---------| | [terraform](#requirement\_terraform) | >= 1.0 | -| [aws](#requirement\_aws) | >= 4.2.0 | +| [aws](#requirement\_aws) | >= 4.9.0 | | [time](#requirement\_time) | >= 0.7 | ## Providers | Name | Version | |------|---------| -| [aws](#provider\_aws) | >= 4.2.0 | +| [aws](#provider\_aws) | >= 4.9.0 | | [time](#provider\_time) | >= 0.7 | ## Modules diff --git a/docs/terraform.md b/docs/terraform.md index b7bac227..1bdd4b94 100644 --- a/docs/terraform.md +++ b/docs/terraform.md @@ -4,14 +4,14 @@ | Name | Version | |------|---------| | [terraform](#requirement\_terraform) | >= 1.0 | -| [aws](#requirement\_aws) | >= 4.2.0 | +| [aws](#requirement\_aws) | >= 4.9.0 | | [time](#requirement\_time) | >= 0.7 | ## Providers | Name | Version | |------|---------| -| [aws](#provider\_aws) | >= 4.2.0 | +| [aws](#provider\_aws) | >= 4.9.0 | | [time](#provider\_time) | >= 0.7 | ## Modules diff --git a/examples/complete/lifecycle.us-east-2.tfvars b/examples/complete/lifecycle.us-east-2.tfvars index 488d52e9..870d17d3 100644 --- a/examples/complete/lifecycle.us-east-2.tfvars +++ b/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 diff --git a/lifecycle.tf b/lifecycle.tf index 5fcee2b7..a71de14d 100644 --- a/lifecycle.tf +++ b/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 } } } diff --git a/main.tf b/main.tf index 72a067b6..57c960ff 100644 --- a/main.tf +++ b/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] diff --git a/versions.tf b/versions.tf index 87da5770..97b6fef4 100644 --- a/versions.tf +++ b/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"