Add additional variable enable_noncurrent_version_expiration

[wszychta]

what

• Enable possibility to disable noncurrent_version_expiration

why

• Sometimes there is no need to have that lifecycle option
• The rest of options are also configurable

[nitrocode]

The auto format cannot run due to the bot not having access to your repo. Is your fork private ?

[nitrocode]

/test all

[wszychta]

@nitrocode It is not possible to fork public repository to private repository. Our fork is public as it should be. You can check that with browser private window, or by just cloning it.

[adamantike]

I think it could be that the PR is not configured to allow modifications by project's maintainers? The Auto Format CI step is the only one that submits commits to the PR branch, instead of only reading from it.

[wszychta]

@Nuru @nitrocode - any updates on this PR? If you provide me details what type of permission @cloudpossebot needs then we can consider adding them to our public fork. Personally I don't know what we can do to get rid of Auto Fornat issue.

[adamantike]

@wszychta, as mentioned before, could you enable the "Allow edits by maintainers" checkbox visible in the right sidebar of this PR? I think that's the issue making the Auto Format CI step fail.

By checking the GitHub API, you can see that it replies with that option disabled:

$ curl -s -H "Accept: application/vnd.github.v3+json" https://api.github.com/repos/cloudposse/terraform-aws-s3-bucket/pulls/90 | jq ".maintainer_can_modify"
false

Compared to another open PR on this repo where the Auto Format step passed:

$ curl -s -H "Accept: application/vnd.github.v3+json" https://api.github.com/repos/cloudposse/terraform-aws-s3-bucket/pulls/94 | jq ".maintainer_can_modify"
true

[bmtoml]

sorry, what menu?

[adamantike]

@bmtoml, I think that option is only present when you're the PR author. This is an example from an open PR of mine:

[bmtoml]

@adamantike was checking on @wszychta 's behalf, as he figured it might be an organisational permission issue. he doesn't seem to have it(but I assume he's going to verify now), and nor do I.

@wszychta I figure you should probably doublecheck given the feedback you've gotten. alternatively I can try and recreate the PR from another branch myself and see if we have this option. as we have no actions/secrets in the repo, it should be fine (and apparantly, possibly be a slightly different setting/message: https://docs.github.com/en/github/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork)

[edit]
sorry @woz5999 - wrong user flag, just pretend I didn't flag you.

[adamantike]

Based on https://docs.github.com/en/github/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

People with push access to the upstream repository of a fork owned by a user account can commit to the forked branches.

So I wonder if this works at all when the fork is owned by an organization instead.

However, to simplify things, @wszychta could apply the changes that the Auto-Format bot is trying to make to this PR, and that should make it pass without the bot trying to push to the fork. I didn't find docs about it, but I could reproduce what the bot is running, by running the following command in the repo's root path:

docker run --rm -it -v "$(pwd)":/src -w /src cloudposse/build-harness pr/auto-format/host

[wszychta]

@adamantike By your suggestion I made auto format changes to this PR. You were right. Now this step passes.
Also I don't see any options mentioned earlier:

[nitrocode]

/test all

[mergify]

This pull request is now in conflict. Could you fix it @wszychta? 🙏

[wszychta]

@nitrocode @joe-niland I have fixed all conflicts and I see that auto format failed.
Like suggested before I did below check:
docker run --rm -it -v "$(pwd)":/src -w /src cloudposse/build-harness pr/auto-format/host
There were no code changes other than what was already pushed to the repository.
Can anybody look at this PR? This simple change was created 3 months ago and should be merged or dismissed a long time ago.

[adamantike]

@wszychta, I was able to get the changes the auto-format bot is trying to make. Probably you're not using the latest cloudposse/build-harness image (which could be solved by running docker pull cloudposse/build-harness).

--- a/README.md
+++ b/README.md
@@ -273,7 +273,7 @@ Available targets:
 | <a name="input_label_order"></a> [label\_order](#input\_label\_order) | The order in which the labels (ID elements) appear in the `id`.<br>Defaults to ["namespace", "environment", "stage", "name", "attributes"].<br>You can omit any of the 6 labels ("tenant" is the 6th), but at least one must be present. | `list(string)` | `null` | no |
 | <a name="input_label_value_case"></a> [label\_value\_case](#input\_label\_value\_case) | Controls the letter case of ID elements (labels) as included in `id`,<br>set as tag values, and output by this module individually.<br>Does not affect values of tags passed in via the `tags` input.<br>Possible values: `lower`, `title`, `upper` and `none` (no transformation).<br>Set this to `title` and set `delimiter` to `""` to yield Pascal Case IDs.<br>Default value: `lower`. | `string` | `null` | no |
 | <a name="input_labels_as_tags"></a> [labels\_as\_tags](#input\_labels\_as\_tags) | Set of labels (ID elements) to include as tags in the `tags` output.<br>Default is to include all labels.<br>Tags with empty values will not be included in the `tags` output.<br>Set to `[]` to suppress all generated tags.<br>**Notes:**<br>  The value of the `name` tag, if included, will be the `id`, not the `name`.<br>  Unlike other `null-label` inputs, the initial setting of `labels_as_tags` cannot be<br>  changed in later chained modules. Attempts to change it will be silently ignored. | `set(string)` | <pre>[<br>  "default"<br>]</pre> | no |
-| <a name="input_lifecycle_rules"></a> [lifecycle\_rules](#input\_lifecycle\_rules) | A list of lifecycle rules | <pre>list(object({<br>    prefix  = string<br>    enabled = bool<br>    tags    = map(string)<br><br>    enable_glacier_transition        = bool<br>    enable_deeparchive_transition    = bool<br>    enable_standard_ia_transition    = bool<br>    enable_current_object_expiration = bool<br><br>    abort_incomplete_multipart_upload_days         = number<br>    noncurrent_version_glacier_transition_days     = number<br>    noncurrent_version_deeparchive_transition_days = number<br>    noncurrent_version_expiration_days             = number<br><br>    standard_transition_days    = number<br>    glacier_transition_days     = number<br>    deeparchive_transition_days = number<br>    expiration_days             = number<br>  }))</pre> | <pre>[<br>  {<br>    "abort_incomplete_multipart_upload_days": 90,<br>    "deeparchive_transition_days": 90,<br>    "enable_current_object_expiration": true,<br>    "enable_deeparchive_transition": false,<br>    "enable_glacier_transition": true,<br>    "enable_standard_ia_transition": false,<br>    "enabled": false,<br>    "expiration_days": 90,<br>    "glacier_transition_days": 60,<br>    "noncurrent_version_deeparchive_transition_days": 60,<br>    "noncurrent_version_expiration_days": 90,<br>    "noncurrent_version_glacier_transition_days": 30,<br>    "prefix": "",<br>    "standard_transition_days": 30,<br>    "tags": {}<br>  }<br>]</pre> | no |
+| <a name="input_lifecycle_rules"></a> [lifecycle\_rules](#input\_lifecycle\_rules) | A list of lifecycle rules | <pre>list(object({<br>    prefix  = string<br>    enabled = bool<br>    tags    = map(string)<br><br>    enable_glacier_transition            = bool<br>    enable_deeparchive_transition        = bool<br>    enable_standard_ia_transition        = bool<br>    enable_current_object_expiration     = bool<br>    enable_noncurrent_version_expiration = bool<br><br>    abort_incomplete_multipart_upload_days         = number<br>    noncurrent_version_glacier_transition_days     = number<br>    noncurrent_version_deeparchive_transition_days = number<br>    noncurrent_version_expiration_days             = number<br><br>    standard_transition_days    = number<br>    glacier_transition_days     = number<br>    deeparchive_transition_days = number<br>    expiration_days             = number<br>  }))</pre> | <pre>[<br>  {<br>    "abort_incomplete_multipart_upload_days": 90,<br>    "deeparchive_transition_days": 90,<br>    "enable_current_object_expiration": true,<br>    "enable_deeparchive_transition": false,<br>    "enable_glacier_transition": true,<br>    "enable_noncurrent_version_expiration": true,<br>    "enable_standard_ia_transition": false,<br>    "enabled": false,<br>    "expiration_days": 90,<br>    "glacier_transition_days": 60,<br>    "noncurrent_version_deeparchive_transition_days": 60,<br>    "noncurrent_version_expiration_days": 90,<br>    "noncurrent_version_glacier_transition_days": 30,<br>    "prefix": "",<br>    "standard_transition_days": 30,<br>    "tags": {}<br>  }<br>]</pre> | no |
 | <a name="input_logging"></a> [logging](#input\_logging) | Bucket access logging configuration. | <pre>object({<br>    bucket_name = string<br>    prefix      = string<br>  })</pre> | `null` | no |
 | <a name="input_name"></a> [name](#input\_name) | ID element. Usually the component or solution name, e.g. 'app' or 'jenkins'.<br>This is the only ID element not also included as a `tag`.<br>The "name" tag is set to the full `id` string. There is no tag with the value of the `name` input. | `string` | `null` | no |
 | <a name="input_namespace"></a> [namespace](#input\_namespace) | ID element. Usually an abbreviation of your organization name, e.g. 'eg' or 'cp', to help ensure generated IDs are globally unique | `string` | `null` | no |
diff --git a/docs/terraform.md b/docs/terraform.md
index 0df8358..c7df24b 100644
--- a/docs/terraform.md
+++ b/docs/terraform.md
@@ -64,7 +64,7 @@
 | <a name="input_label_order"></a> [label\_order](#input\_label\_order) | The order in which the labels (ID elements) appear in the `id`.<br>Defaults to ["namespace", "environment", "stage", "name", "attributes"].<br>You can omit any of the 6 labels ("tenant" is the 6th), but at least one must be present. | `list(string)` | `null` | no |
 | <a name="input_label_value_case"></a> [label\_value\_case](#input\_label\_value\_case) | Controls the letter case of ID elements (labels) as included in `id`,<br>set as tag values, and output by this module individually.<br>Does not affect values of tags passed in via the `tags` input.<br>Possible values: `lower`, `title`, `upper` and `none` (no transformation).<br>Set this to `title` and set `delimiter` to `""` to yield Pascal Case IDs.<br>Default value: `lower`. | `string` | `null` | no |
 | <a name="input_labels_as_tags"></a> [labels\_as\_tags](#input\_labels\_as\_tags) | Set of labels (ID elements) to include as tags in the `tags` output.<br>Default is to include all labels.<br>Tags with empty values will not be included in the `tags` output.<br>Set to `[]` to suppress all generated tags.<br>**Notes:**<br>  The value of the `name` tag, if included, will be the `id`, not the `name`.<br>  Unlike other `null-label` inputs, the initial setting of `labels_as_tags` cannot be<br>  changed in later chained modules. Attempts to change it will be silently ignored. | `set(string)` | <pre>[<br>  "default"<br>]</pre> | no |
-| <a name="input_lifecycle_rules"></a> [lifecycle\_rules](#input\_lifecycle\_rules) | A list of lifecycle rules | <pre>list(object({<br>    prefix  = string<br>    enabled = bool<br>    tags    = map(string)<br><br>    enable_glacier_transition        = bool<br>    enable_deeparchive_transition    = bool<br>    enable_standard_ia_transition    = bool<br>    enable_current_object_expiration = bool<br><br>    abort_incomplete_multipart_upload_days         = number<br>    noncurrent_version_glacier_transition_days     = number<br>    noncurrent_version_deeparchive_transition_days = number<br>    noncurrent_version_expiration_days             = number<br><br>    standard_transition_days    = number<br>    glacier_transition_days     = number<br>    deeparchive_transition_days = number<br>    expiration_days             = number<br>  }))</pre> | <pre>[<br>  {<br>    "abort_incomplete_multipart_upload_days": 90,<br>    "deeparchive_transition_days": 90,<br>    "enable_current_object_expiration": true,<br>    "enable_deeparchive_transition": false,<br>    "enable_glacier_transition": true,<br>    "enable_standard_ia_transition": false,<br>    "enabled": false,<br>    "expiration_days": 90,<br>    "glacier_transition_days": 60,<br>    "noncurrent_version_deeparchive_transition_days": 60,<br>    "noncurrent_version_expiration_days": 90,<br>    "noncurrent_version_glacier_transition_days": 30,<br>    "prefix": "",<br>    "standard_transition_days": 30,<br>    "tags": {}<br>  }<br>]</pre> | no |
+| <a name="input_lifecycle_rules"></a> [lifecycle\_rules](#input\_lifecycle\_rules) | A list of lifecycle rules | <pre>list(object({<br>    prefix  = string<br>    enabled = bool<br>    tags    = map(string)<br><br>    enable_glacier_transition            = bool<br>    enable_deeparchive_transition        = bool<br>    enable_standard_ia_transition        = bool<br>    enable_current_object_expiration     = bool<br>    enable_noncurrent_version_expiration = bool<br><br>    abort_incomplete_multipart_upload_days         = number<br>    noncurrent_version_glacier_transition_days     = number<br>    noncurrent_version_deeparchive_transition_days = number<br>    noncurrent_version_expiration_days             = number<br><br>    standard_transition_days    = number<br>    glacier_transition_days     = number<br>    deeparchive_transition_days = number<br>    expiration_days             = number<br>  }))</pre> | <pre>[<br>  {<br>    "abort_incomplete_multipart_upload_days": 90,<br>    "deeparchive_transition_days": 90,<br>    "enable_current_object_expiration": true,<br>    "enable_deeparchive_transition": false,<br>    "enable_glacier_transition": true,<br>    "enable_noncurrent_version_expiration": true,<br>    "enable_standard_ia_transition": false,<br>    "enabled": false,<br>    "expiration_days": 90,<br>    "glacier_transition_days": 60,<br>    "noncurrent_version_deeparchive_transition_days": 60,<br>    "noncurrent_version_expiration_days": 90,<br>    "noncurrent_version_glacier_transition_days": 30,<br>    "prefix": "",<br>    "standard_transition_days": 30,<br>    "tags": {}<br>  }<br>]</pre> | no |
 | <a name="input_logging"></a> [logging](#input\_logging) | Bucket access logging configuration. | <pre>object({<br>    bucket_name = string<br>    prefix      = string<br>  })</pre> | `null` | no |
 | <a name="input_name"></a> [name](#input\_name) | ID element. Usually the component or solution name, e.g. 'app' or 'jenkins'.<br>This is the only ID element not also included as a `tag`.<br>The "name" tag is set to the full `id` string. There is no tag with the value of the `name` input. | `string` | `null` | no |
 | <a name="input_namespace"></a> [namespace](#input\_namespace) | ID element. Usually an abbreviation of your organization name, e.g. 'eg' or 'cp', to help ensure generated IDs are globally unique | `string` | `null` | no |

[wszychta]

@adamantike @nitrocode @joe-niland
I have made changes and now auto-format passes.

[mergify]

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

[nitrocode]

/test all

[nitrocode]

@wszychta nice work so far and so close. You just need to update the test so the lifecycle rules have the additional property.

https://github.com/cloudposse/terraform-aws-s3-bucket/blob/master/examples/complete/lifecycle.us-east-2.tfvars

[wszychta]

@nitrocode - I have updated test in the way that should work :)

[nitrocode]

/test all

[wszychta]

/test all

[nitrocode]

/test all

[nitrocode]

Thanks @wszychta for the contribution!

[wszychta]

Thank you very much for help and merge :) Mayby I will make other changes in the future.