Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix #147

Merged
merged 3 commits into from
Apr 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions .github/workflows/auto-readme.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
name: "auto-readme"
on:
workflow_dispatch:

schedule:
# Example of job definition:
# .---------------- minute (0 - 59)
Expand All @@ -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.
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions docs/terraform.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
109 changes: 91 additions & 18 deletions examples/complete/lifecycle.us-east-2.tfvars
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
]
}
]

Expand Down Expand Up @@ -84,6 +156,7 @@ lifecycle_rules = [
}
]


force_destroy = true

versioning_enabled = false
Expand Down
46 changes: 35 additions & 11 deletions lifecycle.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
}],
Expand All @@ -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" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this empty filter?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we need it, maybe add a comment why (not easy to understand by looking at the code)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we need it, maybe add a comment why (not easy to understand by looking at the code)

Did you read line 168 (the line above this one)?

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
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion versions.tf
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ terraform {
required_providers {
aws = {
source = "hashicorp/aws"
version = ">= 4.2.0"
version = ">= 4.9.0"
}
time = {
source = "hashicorp/time"
Expand Down