feat: Add the inteligent tiering configuration #166

bamaralf · 2022-06-16T17:38:25Z

Description

This pull request adds the option of managing the bucket intelligent tiering configuration by passing a map of objects to this module. This map of objects encapsulates the configuration options from this resource: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_intelligent_tiering_configuration

Motivation and Context

Intelligent tiering configuration can optimize the costs of the S3 buckets infrastructure and the changes in this pull request make these configurations available from the module.

Breaking Changes

None

How Has This Been Tested?

I have updated at least one of the examples/* to demonstrate and validate my change(s)
I have tested and validated these changes using one or more of the provided examples/* projects

I have executed pre-commit run -a on my pull request

Tf 0.12

antonbabenko

Thanks for the contribution. Could you please try to implement such a feature as close to the rest of the code in this module (no need for a new module, var.intelligent_tiering_config should be map(any) or any, do not use module experiments, defaults(), optional())?

antonbabenko · 2022-06-16T19:51:21Z

variables.tf

@@ -4,6 +4,24 @@ variable "create_bucket" {
  default     = true
 }

+variable "intelligent_tiering" {


Move this variable to the end of this file.

Hi @antonbabenko. I pushed the suggested changes

antonbabenko · 2022-06-17T07:25:24Z

main.tf

@@ -707,3 +712,25 @@ resource "aws_s3_bucket_ownership_controls" "this" {
    aws_s3_bucket.this
  ]
 }
+
+resource "aws_s3_bucket_intelligent_tiering_configuration" "tiering_conf" {


Suggested change

resource "aws_s3_bucket_intelligent_tiering_configuration" "tiering_conf" {

resource "aws_s3_bucket_intelligent_tiering_configuration" "this" {

The resource was renamed

antonbabenko · 2022-06-17T07:25:57Z

main.tf

@@ -9,6 +9,11 @@ locals {
  grants          = try(jsondecode(var.grant), var.grant)
  cors_rules      = try(jsondecode(var.cors_rule), var.cors_rule)
  lifecycle_rules = try(jsondecode(var.lifecycle_rule), var.lifecycle_rule)
+  intelligent_tiering_config = var.intelligent_tiering_config == null ? null : defaults(var.intelligent_tiering_config, {


Suggested change

intelligent_tiering_config = var.intelligent_tiering_config == null ? null : defaults(var.intelligent_tiering_config, {

intelligent_tiering_config = try(jsondecode(var.intelligent_tiering_config), var.intelligent_tiering_config)

Drop the magic with defaults()

antonbabenko · 2022-06-17T07:26:50Z

main.tf

@@ -707,3 +712,25 @@ resource "aws_s3_bucket_ownership_controls" "this" {
    aws_s3_bucket.this
  ]
 }
+
+resource "aws_s3_bucket_intelligent_tiering_configuration" "tiering_conf" {
+  for_each = local.intelligent_tiering_config


Add new line after for_each

Such for_each should respect the value of local.create_bucket as in other resources.

Ok. I updated the for_each loop

antonbabenko · 2022-06-17T07:28:13Z

main.tf

+  bucket   = aws_s3_bucket.this[0].id
+  status   = each.value.status
+
+  filter {


filter is optional according to the docs. Same as prefix and tags in it. Update the code to use dynamic. See other resources for examples.

Moved the config to a dynamic block

Can filter be one or multiple blocks? Now it looks like multiple, but according to the docs it should be zero-or-one? Please update as in other resource (... ? [true] : [])

The change is in place

antonbabenko · 2022-06-17T07:33:12Z

main.tf

+    for_each = each.value.tiering
+
+    content {
+      access_tier = try(tiering.key, null)


access_tier and days are required, so there is no need to wrap values with try()

Yes, I removed the try()

antonbabenko · 2022-06-17T07:35:07Z

variables.tf

@@ -201,3 +201,22 @@ variable "putin_khuylo" {
  type        = bool
  default     = true
 }
+
+variable "intelligent_tiering_config" {
+  description = <<EOS


Please make the description as a string (not heredoc), no link to the documentation, and no comments for the type. See line 146, for example.

Ok, the description was updated

antonbabenko · 2022-06-17T07:35:16Z

variables.tf

+  #     days = number
+  #   }))
+  # }))
+  default = null


Suggested change

default = null

default = {}

The change is in place

antonbabenko · 2022-06-17T07:36:34Z

main.tf

+  for_each = local.intelligent_tiering_config
+  name     = each.key
+  bucket   = aws_s3_bucket.this[0].id
+  status   = each.value.status


Please make status to accept boolean true / false also. See other resources in this module.

The change is in place

antonbabenko · 2022-06-17T19:35:08Z

This issue has been resolved in version 3.3.0 🎉

github-actions · 2022-10-28T02:41:46Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

bamaralf added 11 commits February 5, 2019 16:03

Add .gitignore file.

249183a

Add module files and examples. Update README.md

da2ec62

Add outputs to README.md

e0fda0d

Merge pull request #1 from bamaralf/tf_0.12

3c4c857

Tf 0.12

Update README.md

e347cbd

Update README.md

fe38549

Sync the repo with the upstream.

d203464

Merge remote-tracking branch 'upstream/master'

1ca55b2

Merge remote-tracking branch 'upstream/master'

15fa311

Merge remote-tracking branch 'upstream/master'

4e06233

Merge remote-tracking branch 'upstream/master'

1abb6c4

bamaralf changed the title ~~Add intelligent tiering configuration~~ feat/intelligent-tiering-configuration Jun 16, 2022

bamaralf changed the title ~~feat/intelligent-tiering-configuration~~ feat: add the inteligent tiering configuration Jun 16, 2022

bamaralf changed the title ~~feat: add the inteligent tiering configuration~~ feat: Add the inteligent tiering configuration Jun 16, 2022

bamaralf force-pushed the master branch 2 times, most recently from f442a08 to 8a673bf Compare June 16, 2022 18:48

bamaralf marked this pull request as ready for review June 16, 2022 19:02

antonbabenko requested changes Jun 16, 2022

View reviewed changes

bamaralf force-pushed the master branch from 8a673bf to e961f97 Compare June 16, 2022 19:56

bamaralf marked this pull request as draft June 16, 2022 19:59

bamaralf force-pushed the master branch 2 times, most recently from e6b4ed2 to 6a1087f Compare June 17, 2022 02:41

bamaralf marked this pull request as ready for review June 17, 2022 02:44

bamaralf requested a review from antonbabenko June 17, 2022 02:44

antonbabenko requested changes Jun 17, 2022

View reviewed changes

bamaralf marked this pull request as draft June 17, 2022 12:26

bamaralf force-pushed the master branch 4 times, most recently from 914dca8 to 0c7d472 Compare June 17, 2022 12:39

bamaralf requested a review from antonbabenko June 17, 2022 12:43

bamaralf marked this pull request as ready for review June 17, 2022 12:44

bamaralf force-pushed the master branch from 0c7d472 to 0c94776 Compare June 17, 2022 14:56

Add the inteligent tiering configuration to the module

425ba2f

bamaralf force-pushed the master branch from 0c94776 to 425ba2f Compare June 17, 2022 14:58

antonbabenko mentioned this pull request Jun 17, 2022

feat: Add the intelligent tiering configuration #167

Merged

antonbabenko closed this in #167 Jun 17, 2022

github-actions bot locked as resolved and limited conversation to collaborators Oct 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add the inteligent tiering configuration #166

feat: Add the inteligent tiering configuration #166

bamaralf commented Jun 16, 2022 •

edited

Loading

antonbabenko left a comment

antonbabenko Jun 16, 2022

bamaralf Jun 17, 2022

antonbabenko Jun 17, 2022

bamaralf Jun 17, 2022

antonbabenko Jun 17, 2022

antonbabenko Jun 17, 2022

bamaralf Jun 17, 2022

antonbabenko Jun 17, 2022

antonbabenko Jun 17, 2022

bamaralf Jun 17, 2022

antonbabenko Jun 17, 2022

bamaralf Jun 17, 2022

antonbabenko Jun 17, 2022

bamaralf Jun 17, 2022

antonbabenko Jun 17, 2022

bamaralf Jun 17, 2022

antonbabenko Jun 17, 2022

bamaralf Jun 17, 2022

antonbabenko Jun 17, 2022

bamaralf Jun 17, 2022

antonbabenko Jun 17, 2022

bamaralf Jun 17, 2022

antonbabenko commented Jun 17, 2022

github-actions bot commented Oct 28, 2022

	resource "aws_s3_bucket_intelligent_tiering_configuration" "tiering_conf" {
	resource "aws_s3_bucket_intelligent_tiering_configuration" "this" {

	intelligent_tiering_config = var.intelligent_tiering_config == null ? null : defaults(var.intelligent_tiering_config, {
	intelligent_tiering_config = try(jsondecode(var.intelligent_tiering_config), var.intelligent_tiering_config)

feat: Add the inteligent tiering configuration #166

feat: Add the inteligent tiering configuration #166

Conversation

bamaralf commented Jun 16, 2022 • edited Loading

Description

Motivation and Context

Breaking Changes

How Has This Been Tested?

antonbabenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonbabenko commented Jun 17, 2022

github-actions bot commented Oct 28, 2022

bamaralf commented Jun 16, 2022 •

edited

Loading