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: Add support for existing IAM role for enhanced monitoring #79

Merged
merged 2 commits into from
Sep 22, 2020

Conversation

atrepca
Copy link
Contributor

@atrepca atrepca commented Nov 9, 2019

Description

Add support for using an existing IAM role to send RDS enhanced monitoring metrics to CloudWatch Logs.

Background:

If the module is used for provisioning multiple clusters in the same account, it will create one monitoring role for each, which isn't really necessary as they're identical - attaching the same AWS managed policy - AmazonRDSEnhancedMonitoringRole.

Also, when enabling via the console, from the docs:

The first time that you enable Enhanced Monitoring in the console, you can select the Default option for the Monitoring Role property to have RDS create the required IAM role. RDS then automatically creates a role named rds-monitoring-role for you, and uses it for the specified DB instance or Read Replica.

Testing:

Added the below to an existing cluster:

data "aws_iam_role" "default_aws_rds_monitoring_role" {
  name = "rds-monitoring-role"
}

module "ods2" {
  source                 = "terraform-aws-modules/rds-aurora/aws"
  (...)
  create_monitoring_role = false
  monitoring_interval    = 60
  monitoring_role_arn    = data.aws_iam_role.default_aws_rds_monitoring_role.arn
}

Generated these changes:

      ~ monitoring_interval = 0 -> 60
      + monitoring_role_arn = "arn:aws:iam::<REMOVED>:role/rds-monitoring-role"

@atrepca
Copy link
Contributor Author

atrepca commented Feb 10, 2020

@antonbabenko I rebased this using the latest master. Does this PR make sense?

Thanks,

@jecnua
Copy link

jecnua commented Feb 19, 2020

Sorry to comment on this PR but we need this in place for also another reason as explained in #93 . Running enhanced monitoring in AWS CHINA with this module is impossible because the role policy is hardcoded with a specific partition and the role is not injectable. This PR would be a good workaround while we wait for a partition implementation.

@atrepca atrepca changed the title Add support for existing IAM role for enhanced monitoring feat: Add support for existing IAM role for enhanced monitoring May 29, 2020
@atrepca
Copy link
Contributor Author

atrepca commented May 29, 2020

Rebased this again, would you be able to give this an eye @antonbabenko? 🙏 and 🙇.

@DWSR
Copy link

DWSR commented Jul 22, 2020

We need this as well because we have restrictions on IAM roles in our org that this module doesn't satisfy. Being able to supply our own IAM role for things would be great.

@atrepca
Copy link
Contributor Author

atrepca commented Jul 22, 2020

Rebased again, and updated commit title to pass semantic check.

main.tf Outdated Show resolved Hide resolved
@bryantbiggs
Copy link
Member

@antonbabenko 👍

@antonbabenko antonbabenko merged commit 358ab8a into terraform-aws-modules:master Sep 22, 2020
@antonbabenko
Copy link
Member

Great!

v2.24.0 has been just released.

tomasbackman pushed a commit to qapital/terraform-aws-rds-aurora that referenced this pull request Nov 16, 2020
@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 Apr 15, 2023
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.

None yet

5 participants