feat: add kms key arn input for customer-managed keys #24
Conversation
WalkthroughA new variable, Changes
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Require terratestWonderful, this rule succeeded.This rule require terratest status
|
|
/terratest |
RoseSecurity
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/variables.tf (1)
50-54: Add validation for KMS Key ARN format
To ensure users provide a valid KMS key ARN (or leave it null to let the module generate one), consider adding avalidationblock that checks the ARN pattern. This will catch typos early and surface a clear error message.variable "kms_key_arn" { type = string description = "The ARN for the KMS encryption key." default = null validation { condition = var.kms_key_arn == null || can(regex("^arn:aws:kms:[^:]+:[0-9]{12}:key/[A-Za-z0-9\\-_/]+$", var.kms_key_arn)) error_message = "The value for kms_key_arn must be a valid AWS KMS Key ARN or null." } }
📜 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)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: component / terraform / Lint (./src/modules/postgresql-user)
- GitHub Check: component / terraform / Lint (./src)
- GitHub Check: component / terraform / Format HCL
- GitHub Check: component / terraform / bats
- GitHub Check: Mergify Merge Protections
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🔇 Additional comments (1)
src/main.tf (1)
11-11:❓ Verification inconclusive
Verify precedence of
coalescearguments
Currently,kms_key_arn = coalesce(module.aurora_postgres.outputs.kms_key_arn, var.kms_key_arn)will always select the module output if it’s non-null, which may mask a user-supplied
var.kms_key_arn. Please confirm whethermodule.aurora_postgres.outputs.kms_key_arnis set tonullwhen a custom key ARN is provided. If not, swap the arguments to give explicit override precedence:kms_key_arn = coalesce(var.kms_key_arn, module.aurora_postgres.outputs.kms_key_arn)
🏁 Script executed:
#!/bin/bash # Locate the kms_key_arn output in the Aurora Postgres module to confirm its behavior rg -R --include "*.tf" 'output "kms_key_arn"' -n .Length of output: 457
I’ll re-run the search without the invalid
-Rflag to locate anykms_key_arndefinitions in the repo:If these still return nothing (e.g. because the module is remote), please manually verify whether
module.aurora_postgres.outputs.kms_key_arnever returns null whenvar.kms_key_arnis set to ensure the correctcoalesceargument order.
🏁 Script executed:
#!/bin/bash # Look for output and variable definitions for kms_key_arn across all .tf files rg --with-filename --line-number --context=3 --include '*.tf' 'output "kms_key_arn"' rg --with-filename --line-number --context=3 --include '*.tf' 'variable "kms_key_arn"'Length of output: 1059
I’ll switch to a grep-based search to find any local definitions or references:
🏁 Script executed:
#!/bin/bash # Search for kms_key_arn outputs, variables, and the aurora_postgres module declaration grep -RIn --include='*.tf' 'output "kms_key_arn"' . grep -RIn --include='*.tf' 'variable "kms_key_arn"' . grep -RIn --include='*.tf' 'module "aurora_postgres"' .Length of output: 265
Confirm
coalesceargument precedenceIt looks like
module.aurora_postgres.outputs.kms_key_arncomes from a remote module (no localoutput "kms_key_arn"found), so we can’t verify from source whether it returnsnullwhen you supplyvar.kms_key_arn. Please double-check that behavior and, if the module always emits a non-null ARN, swap the arguments so a user-provided key wins:• File: src/main.tf
Line: 11- kms_key_arn = coalesce(module.aurora_postgres.outputs.kms_key_arn, var.kms_key_arn) + kms_key_arn = coalesce(var.kms_key_arn, module.aurora_postgres.outputs.kms_key_arn)Until you’ve confirmed the module output can be
null, leave a note or adjust accordingly.
|
These changes were released in v1.537.0. |
why
kms_key_arnvariable, allowing consumers to supply their own customer-managed KMS keywhat
Default behavior
kms_key_arnOverride behavior
kms_key_arn = "arn:aws:kms:…:key/EXAMPLE"references
Summary by CodeRabbit