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

feat: Added Replication Time Control for Bucket Replication #114

Merged
merged 9 commits into from
Nov 7, 2021
Merged

Conversation

schniber
Copy link
Contributor

@schniber schniber commented Nov 5, 2021

Description

This change is about taking into account the recent enhancement of the AWS Terraform Provider which now allows to set S3 Replication Time Control (RTC)

Motivation and Context

Thanks to this enhancement, we will be able to provision S3 Bucket Replication Configurations with SLAs and Metrics on the Replication itself from this terraform module.
It's the reflection on the module of the closure of the following issue : hashicorp/terraform-provider-aws#10974

Breaking Changes

There is no breaking change since I have just added the right attributes on the Replication configuration which are read from user input in the destination block of the replication_configuration attribute.

How Has This Been Tested?

  • [X ] I have tested and validated these changes using one or more of the provided examples/* projects

I have adapted the s3-replication example by adding the right input and I was able to successfully see metrics on the AWS S3 Bucket Console.

There was no need to add any control over user input since AWS API provides meaningful error messages in case user input is wrong :

│ Error: expected replication_configuration.0.rules.0.destination.0.replication_time.0.minutes to be in the range (15 - 15), got 16

│ with module.s3_bucket.aws_s3_bucket.this[0],
│ on ../../main.tf line 5, in resource "aws_s3_bucket" "this":
│ 5: resource "aws_s3_bucket" "this" {

│ Error: expected replication_configuration.0.rules.0.destination.0.replication_time.0.status to be one of [Enabled Disabled], got yes

│ with module.s3_bucket.aws_s3_bucket.this[0],
│ on ../../main.tf line 5, in resource "aws_s3_bucket" "this":
│ 5: resource "aws_s3_bucket" "this" {

│ Error: expected replication_configuration.0.rules.0.destination.0.metrics.0.minutes to be in the range (10 - 15), got 16

│ with module.s3_bucket.aws_s3_bucket.this[0],
│ on ../../main.tf line 5, in resource "aws_s3_bucket" "this":
│ 5: resource "aws_s3_bucket" "this" {

│ Error: expected replication_configuration.0.rules.0.destination.0.metrics.0.status to be one of [Enabled Disabled], got true

│ with module.s3_bucket.aws_s3_bucket.this[0],
│ on ../../main.tf line 5, in resource "aws_s3_bucket" "this":
│ 5: resource "aws_s3_bucket" "this" {

In this case the module users, will be easily able to correct their input to feed with the right values.

I have also ran pre-commit hooks as per the original pre-commit config file.

Thanks again for your review and feedback.

Bests.

Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

Please use terraform-docs version 0.13.0 and update the minimum version of AWS provider to be 3.64.0.

@antonbabenko antonbabenko changed the title enhancement:Addition of Replication Time Control for Bucket Replication feat: Added Replication Time Control for Bucket Replication Nov 5, 2021
@schniber
Copy link
Contributor Author

schniber commented Nov 5, 2021

Please use terraform-docs version 0.13.0 and update the minimum version of AWS provider to be 3.64.0.

Done in the last push.

Thanks again.

Bests.

versions.tf Outdated
@@ -2,6 +2,6 @@ terraform {
required_version = ">= 0.13.1"

required_providers {
aws = ">= 3.50"
aws = ">= 3.64.0"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
aws = ">= 3.64.0"
aws = ">= 3.64"

examples/s3-replication/README.md Outdated Show resolved Hide resolved
README.md Outdated
@@ -1,120 +1,15 @@
# AWS S3 bucket Terraform module
Copy link
Member

Choose a reason for hiding this comment

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

Please revert all these docs changes in README, I will fix the rest of this PR (it is too confusing, I know).

Copy link
Member

Choose a reason for hiding this comment

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

This has not been done yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it possible to explain what's expected this way I can make sure I generate the right documentation ?
I am running terraform-docs 0.13.0 from a docker container locally before pushing.
Sorry for the back and forth.

Copy link
Member

Choose a reason for hiding this comment

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

In this PR you deleted most of the content of the README. Please revert that one, and then running pre-commit run -a will fix the required part related to Terraform versions (if not, I can fix it for you).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh got it now. Normally this should be fixed in the last push.

README.md Outdated Show resolved Hide resolved
@antonbabenko antonbabenko merged commit fa1defc into terraform-aws-modules:master Nov 7, 2021
@antonbabenko
Copy link
Member

Thanks, @schniber !

v2.11.0 has been just released.

@schniber
Copy link
Contributor Author

schniber commented Nov 7, 2021

Thanks a lot @antonbabenko !!!

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants