feat: support RDS-managed admin password via Secrets Manager #64
Conversation
Add `manage_admin_user_password` variable to allow AWS RDS to manage the master user password in Secrets Manager. Adjust logic to ensure `admin_password` and `manage_admin_user_password` are mutually exclusive, and update module and locals to support this new option. This enhances security and flexibility for password management.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughExposes a new boolean input Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant TF as Terraform module
participant RP as random_password
participant RDS as AWS RDS
participant SM as SecretsManager
User->>TF: provide inputs (manage_admin_user_password, admin_password, enabled)
alt manage_admin_user_password == true
Note right of TF #DDEEDF: admin_password set to null (RDS-managed)
TF->>RDS: create cluster (admin_password = null)
RDS->>SM: store/rotate secret
else admin_password provided
Note right of TF #FFF2CC: use provided admin_password
TF->>RDS: create cluster (admin_password = var.admin_password)
else neither set
Note right of TF #F2E6FF: generate random password locally
TF->>RP: create random_password
RP-->>TF: generated value
TF->>RDS: create cluster (admin_password = generated)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for RDS-managed admin passwords via AWS Secrets Manager while maintaining backward compatibility with existing password management approaches. It introduces a new variable to control AWS-managed passwords and updates the local logic to handle three distinct password management scenarios.
- Added
manage_admin_user_passwordvariable to enable AWS Secrets Manager password management - Refactored password logic to support AWS-managed, user-provided, or auto-generated passwords exclusively
- Updated cluster configuration to pass appropriate password values based on the selected management method
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/variables.tf | Adds new boolean variable for enabling AWS-managed password via Secrets Manager |
| src/main.tf | Updates local password logic to handle three mutually exclusive password management methods |
| src/cluster-regional.tf | Modifies cluster module calls to conditionally set admin_password and manage_admin_user_password |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main.tf (1)
79-80: Skip random_password when AWS manages the secret.Even when
manage_admin_user_passwordis true, this resource still gets created because it only checks for an emptyvar.admin_password. That leaves an unused password resource in state. Please gate the count on the new flag as well.-resource "random_password" "admin_password" { - count = local.enabled && length(var.admin_password) == 0 ? 1 : 0 +resource "random_password" "admin_password" { + count = local.enabled && length(var.admin_password) == 0 && !var.manage_admin_user_password ? 1 : 0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/cluster-regional.tf(1 hunks)src/main.tf(1 hunks)src/variables.tf(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/@(main|variables|outputs|providers|versions|context).tf
📄 CodeRabbit inference engine (AGENTS.md)
Keep the Terraform component source of truth under src/ with canonical files: main.tf, variables.tf, outputs.tf, providers.tf, versions.tf, context.tf
Files:
src/variables.tfsrc/main.tf
src/**/*.tf
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.tf: Terraform code must be formatted (terraform fmt -recursive) and free of lint violations (TFLint rules)
Use lower_snake_case for Terraform variables and locals; keep resource and data source names descriptive and aligned with Cloud Posse null-label patterns
Use 2-space indentation for Terraform files
Files:
src/variables.tfsrc/main.tfsrc/cluster-regional.tf
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
|
Apologies, the original documentation was vendored from the DocumentDB implementation of this code |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cluster-regional.tf (1)
6-21: Add a precondition to enforce exclusivity between manage flag and admin_password.Prevents accidental configuration drift and aligns with the variable’s contract.
module "aurora_postgres_cluster" { source = "cloudposse/rds-cluster/aws" version = "2.1.1" ... + precondition { + condition = !(var.manage_admin_user_password && var.admin_password != "") + error_message = "admin_password must be empty when manage_admin_user_password is true." + } ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/cluster-regional.tf(1 hunks)src/main.tf(1 hunks)src/variables.tf(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/@(main|variables|outputs|providers|versions|context).tf
📄 CodeRabbit inference engine (AGENTS.md)
Keep the Terraform component source of truth under src/ with canonical files: main.tf, variables.tf, outputs.tf, providers.tf, versions.tf, context.tf
Files:
src/variables.tfsrc/main.tf
src/**/*.tf
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.tf: Terraform code must be formatted (terraform fmt -recursive) and free of lint violations (TFLint rules)
Use lower_snake_case for Terraform variables and locals; keep resource and data source names descriptive and aligned with Cloud Posse null-label patterns
Use 2-space indentation for Terraform files
Files:
src/variables.tfsrc/cluster-regional.tfsrc/main.tf
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (1)
src/cluster-regional.tf (1)
18-21: admin_password wiring correct; no rename needed. The child module v2.1.1 expectsmanage_admin_user_password, so the existing argument is already correct.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/variables.tf (1)
125-130: Add plan-time validation to enforce mutual exclusivity.Configuration currently allows
manage_admin_user_password = truewhileadmin_passwordis non-empty, which leads to provider errors. Please add a validation block here so plans fail fast when both are set.variable "manage_admin_user_password" { type = bool default = false description = "Set to true to allow RDS to manage the master user password in Secrets Manager. Cannot be set if admin_password is provided" nullable = false + + validation { + condition = !(var.manage_admin_user_password && length(trimspace(var.admin_password)) > 0) + error_message = "manage_admin_user_password cannot be true when admin_password is provided. Leave admin_password empty to delegate password management to RDS." + } }src/main.tf (1)
17-22: Null out admin_password when AWS manages it.When
manage_admin_user_passwordis true and no password is supplied, this local still returns"", so the module sends an empty string to AWS instead ofnull, causing apply-time failures. Please returnnullwhen the manage flag is enabled.- create_password = local.enabled && var.admin_password == "" && !var.manage_admin_user_password - admin_password = local.create_password ? one(random_password.admin_password[*].result) : var.admin_password + create_password = local.enabled && var.admin_password == "" && !var.manage_admin_user_password + admin_password = var.manage_admin_user_password ? null : (local.create_password ? one(random_password.admin_password[*].result) : var.admin_password)
🧹 Nitpick comments (1)
src/main.tf (1)
78-80: Only allocate random_password when actually needed.With the current
count, Terraform still creates a random password even when RDS should manage it. Tie the resource tolocal.create_passwordso it is skipped in that case.-resource "random_password" "admin_password" { - count = local.enabled && length(var.admin_password) == 0 ? 1 : 0 +resource "random_password" "admin_password" { + count = local.create_password ? 1 : 0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main.tf(1 hunks)src/variables.tf(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/@(main|variables|outputs|providers|versions|context).tf
📄 CodeRabbit inference engine (AGENTS.md)
Keep the Terraform component source of truth under src/ with canonical files: main.tf, variables.tf, outputs.tf, providers.tf, versions.tf, context.tf
Files:
src/variables.tfsrc/main.tf
src/**/*.tf
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.tf: Terraform code must be formatted (terraform fmt -recursive) and free of lint violations (TFLint rules)
Use lower_snake_case for Terraform variables and locals; keep resource and data source names descriptive and aligned with Cloud Posse null-label patterns
Use 2-space indentation for Terraform files
Files:
src/variables.tfsrc/main.tf
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
/terratest |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main.tf (1)
78-91: Don’t create random password when RDS manages it.The random_password still gets created when manage_admin_user_password=true (admin_password empty). This leaks an unnecessary secret into state and diverges from the chosen management mode.
Apply:
resource "random_password" "admin_password" { - count = local.enabled && length(var.admin_password) == 0 ? 1 : 0 + # Only generate when this module is responsible for creating the password + count = local.create_password ? 1 : 0
🧹 Nitpick comments (1)
src/main.tf (1)
17-24: Minor consistency: use one style for “empty” checks.You mix
== ""andlength(...) == 0. Pick one for readability; suggest length(...) == 0 everywhere.- create_password = local.enabled && var.admin_password == "" && !var.manage_admin_user_password + create_password = local.enabled && length(var.admin_password) == 0 && !var.manage_admin_user_password
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main.tf(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/@(main|variables|outputs|providers|versions|context).tf
📄 CodeRabbit inference engine (AGENTS.md)
Keep the Terraform component source of truth under src/ with canonical files: main.tf, variables.tf, outputs.tf, providers.tf, versions.tf, context.tf
Files:
src/main.tf
src/**/*.tf
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.tf: Terraform code must be formatted (terraform fmt -recursive) and free of lint violations (TFLint rules)
Use lower_snake_case for Terraform variables and locals; keep resource and data source names descriptive and aligned with Cloud Posse null-label patterns
Use 2-space indentation for Terraform files
Files:
src/main.tf
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
Replaces join() with one() for random_pet admin_user and database_name IDs to ensure correct value selection. This prevents potential issues with unexpected list handling and aligns with Terraform's best practices for single value extraction.
|
/terratest |
Co-authored-by: Igor Rodionov <goruha@gmail.com>
|
Thanks! |
|
These changes were released in v1.540.3. |
2 participants
what
This pull request introduces support for allowing AWS RDS to manage the admin user password for Aurora PostgreSQL clusters via Secrets Manager, while maintaining backward compatibility with the existing manual password management. The changes clarify and enforce the logic for how admin passwords are set, ensuring only one method is used at a time, and update the relevant variables and local values accordingly.
Password management enhancements:
manage_admin_user_passwordto enable AWS-managed admin passwords via Secrets Manager, with validation to prevent conflicts ifadmin_passwordis also provided. (src/variables.tfsrc/variables.tfR125-R131)admin_passwordaccordingly. (src/main.tfsrc/main.tfR17-L18)admin_passwordand the newmanage_admin_user_passwordparameter, ensuring compatibility with the new logic. (src/cluster-regional.tfsrc/cluster-regional.tfL19-R20)why
manage_admin_user_passwordvariable to allow AWS RDS to manage the master user password in Secrets Manager. Adjust logic to ensureadmin_passwordandmanage_admin_user_passwordare mutually exclusive, and update module and locals to support this new option. This enhances security and flexibility for password management.references
Summary by CodeRabbit
New Features
Documentation