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

fix: correct bucket name to fix broken -replication role #250

Merged
merged 5 commits into from
Nov 13, 2024

Conversation

amila-ku
Copy link
Contributor

@amila-ku amila-ku commented Sep 18, 2024

what

  • Fixes replication IAM role name that gets created as '-replication'

why

  • Replication IAM role name gets created as '-replication'. This made the replication role unsusable.
  • Used Terraform version: 1.8.0

references

@amila-ku amila-ku requested review from a team as code owners September 18, 2024 09:41
@mergify mergify bot added the triage Needs triage label Sep 18, 2024
@amila-ku amila-ku changed the title fix: fixes broken replication name fix: fixes broken replication iam role name Sep 18, 2024
replication.tf Outdated Show resolved Hide resolved
replication.tf Outdated Show resolved Hide resolved
amila-ku and others added 2 commits September 28, 2024 02:52
@vbrinza
Copy link

vbrinza commented Nov 11, 2024

I am having the same issue.
Using the module directly from the Terragrunt source, so no way to set the "module" name as expected by the https://github.com/cloudposse/terraform-aws-s3-bucket/blob/main/replication.tf#L4 construction.
Normally we should append "replication" at the end of the "bucket_name" which is going to be good enough for all the cases.
Also in our local fork we have something like
role = var.s3_replication_role == null ? aws_iam_role.replication[0].arn : var.s3_replication_role
for when we want to setup a separate role for it.
@nitrocode can we make this change go further?

@nitrocode
Copy link
Member

/terratest

@nitrocode
Copy link
Member

/terratest

@mergify mergify bot removed the triage Needs triage label Nov 13, 2024
@nitrocode nitrocode added the patch A minor, backward compatible change label Nov 13, 2024
@nitrocode nitrocode changed the title fix: fixes broken replication iam role name fix: correct bucket name to fix broken -replication role Nov 13, 2024
@nitrocode
Copy link
Member

I added changes, ran tests, and approved. I cannot merge because I was the last person to push changes. I reached out to sweetops in the #pr-reviews slack channel for visibility

https://sweetops.slack.com/archives/CUJPCP1K6/p1731506086562779

@gberenice gberenice merged commit 424de84 into cloudposse:main Nov 13, 2024
38 checks passed
Copy link

These changes were released in v4.7.2.

@nitrocode
Copy link
Member

Thanks for the contribution @amila-ku 👍 and for the ping @vbrinza

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch A minor, backward compatible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replication role is created as '-replication'
4 participants