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

Add extra managed policy dedicated to S3 backups #37

Closed
wants to merge 2 commits into from

Conversation

GregoryVds
Copy link

what

AWS Backup for S3 buckets has reached GA recently (Feb 18 2022).

It seems AWS created a separate dedicated managed policy with the
permissions required to backup S3.

why

I am not sure if my fix is the correct way to address this, but for some reason that I ignore the managed policy of the backup service-linked role does not include the necessary S3 permissions and my backup fail (note that I did enable S3 for Backup in the Backup options via the Web Console).

references

AWS Backup for S3 buckets has reached GA recently (Feb 18 2022).

It seems AWS created a separate dedicated managed policy with the
permissions required to backup S3.

Signed-off-by: Gregory Vander Schueren <gregory.vanderschueren@gmail.com>
@GregoryVds GregoryVds requested review from a team as code owners April 20, 2022 12:06
@GregoryVds GregoryVds requested review from a team as code owners April 20, 2022 12:06
@GregoryVds GregoryVds requested review from srhopkins and brcnblc and removed request for a team April 20, 2022 12:06
@GregoryVds
Copy link
Author

Hello everyone,
Anyone to have a look and hopefully help me merge this patch? 🙏

@nitrocode
Copy link
Member

nitrocode commented Jun 18, 2022

Thanks for the PR @GregoryVds . I'm unsure if this is the correct way to fix this.

You can use the existing iam role functionality and attach this policy from outside the module. See iam_role_enabled = false and then give the module the role name created outside of the module using iam_role_name.

https://github.com/cloudposse/terraform-aws-iam-role#input_managed_policy_arns

module "iam_role" {
  source = "cloudposse/iam-role/aws"
  # Cloud Posse recommends pinning every module to a specific version
  # version     = "x.x.x"

  # ... your inputs

  managed_policy_arns = [
    "arn:aws:iam::aws:policy/AWSBackupServiceRolePolicyForS3Backup",
  ]
}

module "backup" {
  source = "cloudposse/backup/aws"
  # Cloud Posse recommends pinning every module to a specific version
  # version     = "x.x.x"

  iam_role_enabled = false
  iam_role_name    = module.iam_role.name
}

It may be better to expose the iam role name as an output and then you can create the attachment from outside of the module

module "backup" {
  source = "cloudposse/backup/aws"
  # Cloud Posse recommends pinning every module to a specific version
  # version     = "x.x.x"

  # ... your inputs
}

resource "aws_iam_role_policy_attachment" "s3" {
  policy_arn = "arn:aws:iam::aws:policy/AWSBackupServiceRolePolicyForS3Backup"
  role       = module.backup.iam_role_name
}

See PR #40

What do you think ?

@nitrocode nitrocode mentioned this pull request Jun 18, 2022
@GregoryVds
Copy link
Author

@nitrocode Thanks for reviewing and answering.

Yes, I could definitely create the attachment from outside of the module like you suggest.

Do you have a specific reason to treat the attachment of the generic AWSBackupServiceRolePolicyForBackup policy differently from the attachment of the S3 policy AWSBackupServiceRolePolicyForS3Backup?

Right now the module encapsulates nicely the policy attachment if you ask it to create the IAM role. This would be convenient for the user as I don't even need to know about this obscure new S3 policy.

@hans-d hans-d added the stale This PR has gone stale label Mar 2, 2024
@hans-d hans-d closed this Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale This PR has gone stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants