feat: transit_encryption_mode
#47
Conversation
WalkthroughAdds gitignore rule for Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Root as Root Module
participant RedisMod as Redis Cluster Module
Note over Root: Root stores var.transit_encryption_mode and local.enabled
User->>Root: request output transit_encryption_mode
alt local.enabled == true
Root->>RedisMod: read transit_encryption_mode from first redis cluster (try(...))
RedisMod-->>Root: returns transit_encryption_mode (or null)
Root-->>User: returns value
else local.enabled == false
Root-->>User: null
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches🧪 Generate unit tests
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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/modules/redis_cluster/main.tf (1)
11-47: Wire through transit_encryption_mode to actually support the PR rationale.Right now we only expose the mode as an output; there’s no way to set it. Please plumb an input into the child module so users can set preferred/required when transit encryption is enabled. AWS added this in provider v5.47.0 and requires a two-step migration (preferred then required). (github.com, docs.aws.amazon.com)
Add this line inside module "redis":
# inside module "redis" transit_encryption_mode = var.cluster_attributes.transit_encryption_modeAnd add the attribute to cluster_attributes (see variables.tf comment).
src/modules/redis_cluster/variables.tf (1)
53-74: Add optional transit_encryption_mode to cluster_attributes with value validation.Needed to actually configure the mode (preferred/required) when in-transit encryption is enabled. (github.com)
Add the attribute to the object type (keeping it optional to avoid breaking callers):
variable "cluster_attributes" { type = object({ availability_zones = list(string) vpc_id = string additional_security_group_rules = list(any) allowed_security_groups = list(string) allow_all_egress = bool subnets = list(string) family = string port = number zone_id = string multi_az_enabled = bool at_rest_encryption_enabled = bool transit_encryption_enabled = bool apply_immediately = bool automatic_failover_enabled = bool auto_minor_version_upgrade = bool auth_token_enabled = bool snapshot_retention_limit = number transit_encryption_mode = optional(string) # "preferred" | "required" }) description = "Cluster attributes" validation { condition = try(var.cluster_attributes.transit_encryption_mode == null || contains(["preferred","required"], lower(var.cluster_attributes.transit_encryption_mode)), true) error_message = "transit_encryption_mode must be null, \"preferred\", or \"required\"." } }
🧹 Nitpick comments (2)
.gitignore (1)
79-79: Scope the ignore rule to repo root (if intended).If you only mean to ignore the top-level directory, prefix with a slash; otherwise this also ignores any nested account-map/ dirs.
-account-map/ +/account-map/src/outputs.tf (1)
11-14: Expose modes for all clusters (optional).Current pattern returns only the first cluster’s mode. Consider returning a map keyed by cluster for multi-cluster users. (Keep existing output for backward compatibility if needed.)
# Alternative/additional output output "transit_encryption_modes" { description = "TLS in-transit encryption mode per Redis cluster" value = local.enabled ? { for name, mod in module.redis_clusters : name => try(mod.transit_encryption_mode, null) } : {} }Also ensure the child module actually exports transit_encryption_mode as an output to prevent plan errors. (github.com)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.gitignore(1 hunks)src/modules/redis_cluster/main.tf(1 hunks)src/modules/redis_cluster/outputs.tf(1 hunks)src/modules/redis_cluster/variables.tf(1 hunks)src/modules/redis_cluster/versions.tf(0 hunks)src/outputs.tf(1 hunks)
💤 Files with no reviewable changes (1)
- src/modules/redis_cluster/versions.tf
🧰 Additional context used
📓 Path-based instructions (2)
src/@(main|variables|outputs|providers|versions|context).tf
📄 CodeRabbit inference engine (AGENTS.md)
Keep the Terraform component as the source of truth under src/ with files: main.tf, variables.tf, outputs.tf, providers.tf, versions.tf, context.tf
Files:
src/outputs.tf
**/*.tf
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tf: Use 2-space indentation for Terraform files
In Terraform, use lower_snake_case for variables and locals; keep resource/data source names descriptive and aligned with Cloud Posse null-label patterns
Do not commit formatting violations; run terraform fmt -recursive
Adhere to TFLint rules defined in .tflint.hcl
Files:
src/outputs.tfsrc/modules/redis_cluster/variables.tfsrc/modules/redis_cluster/outputs.tfsrc/modules/redis_cluster/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 (4)
src/modules/redis_cluster/main.tf (2)
8-8: Direct indexing over splat/join is correct.This resolves the TFLint warning and avoids unnecessary allocations. LGTM.
41-44: Compatibility note: provider and engine constraints.Ensure the component enforces aws provider >= 5.47.0 and document that transit_encryption_mode only applies to Redis OSS 7+ for modify operations. Otherwise applies may fail on older engines. (github.com, docs.aws.amazon.com)
src/modules/redis_cluster/variables.tf (1)
91-91: Typing looks good.Explicit string type for kms_alias_name_ssm is fine; SSM supports alias names for KMS key_id.
src/modules/redis_cluster/outputs.tf (1)
31-34: Verify upstream output exists in v1.9.2.This forwards module.redis.transit_encryption_mode; confirm the Cloud Posse module exposes an output with that exact name in v1.9.2, or this will fail at plan time. The module added the input in v1.4.0; output availability isn’t clearly documented. (github.com)
If not present upstream, fallback to outputting the configured value:
-output "transit_encryption_mode" { - value = module.redis.transit_encryption_mode - description = "TLS in-transit encryption mode for Redis cluster" -} +output "transit_encryption_mode" { + value = var.cluster_attributes.transit_encryption_mode + description = "TLS in-transit encryption mode for Redis cluster" +}
|
/terratest |
|
/terratest |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/modules/redis_cluster/outputs.tf (1)
31-34: Good addition; consider guarding with try() for robustness and consistency.If
cluster_attributes.transit_encryption_modeis not always set, accessing it will fail whenlocal.enabledis true. Usingtry()mirrors the pattern used at the root output and avoids hard failures while still yieldingnullwhen absent.Apply this diff:
output "transit_encryption_mode" { - value = local.enabled ? var.cluster_attributes.transit_encryption_mode : null + value = local.enabled ? try(var.cluster_attributes.transit_encryption_mode, null) : null description = "TLS in-transit encryption mode for Redis cluster" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/modules/redis_cluster/main.tf(1 hunks)src/modules/redis_cluster/outputs.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/modules/redis_cluster/main.tf
🧰 Additional context used
📓 Path-based instructions (1)
**/*.tf
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tf: Use 2-space indentation for Terraform files
In Terraform, use lower_snake_case for variables and locals; keep resource/data source names descriptive and aligned with Cloud Posse null-label patterns
Do not commit formatting violations; run terraform fmt -recursive
Adhere to TFLint rules defined in .tflint.hcl
Files:
src/modules/redis_cluster/outputs.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
milldr
changed the title
transit_encryption_mode outputtransit_encryption_mode
|
/terratest |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/variables.tf (1)
53-57: Validate allowed values and couple to transit_encryption_enabledPrevent config drift and invalid plans by validating the mode and only allowing it when TLS is enabled.
Apply:
variable "transit_encryption_mode" { type = string default = null description = "Transit encryption mode. Valid values are 'preferred' and 'required'" + validation { + condition = var.transit_encryption_mode == null || contains(["preferred", "required"], lower(var.transit_encryption_mode)) + error_message = "transit_encryption_mode must be null, 'preferred', or 'required'." + } + validation { + condition = var.transit_encryption_enabled || var.transit_encryption_mode == null + error_message = "Set transit_encryption_mode only when transit_encryption_enabled = true." + } }src/modules/redis_cluster/variables.tf (1)
67-67: Make attribute optional to accept null cleanlyThis avoids type noise when the root passes null and keeps the schema future-proof.
- transit_encryption_mode = string + transit_encryption_mode = optional(string)If the repo pins Terraform < 0.15, optional() may not be available. Confirm TF version before applying.
src/main.tf (1)
49-51: Prefer explicit default to avoid provider/API default driftSet “preferred” when TLS is enabled and mode unset; otherwise pass null. This matches AWS defaults while being explicit.
- transit_encryption_enabled = var.transit_encryption_enabled - transit_encryption_mode = var.transit_encryption_mode + transit_encryption_enabled = var.transit_encryption_enabled + transit_encryption_mode = var.transit_encryption_enabled ? coalesce(var.transit_encryption_mode, "preferred") : nullsrc/modules/redis_cluster/outputs.tf (1)
31-34: Align output gating with existing outputsOther outputs aren’t gated by local.enabled; keep consistent unless there’s a specific reason.
-output "transit_encryption_mode" { - value = local.enabled ? module.redis.transit_encryption_mode : null - description = "TLS in-transit encryption mode for Redis cluster" -} +output "transit_encryption_mode" { + value = module.redis.transit_encryption_mode + description = "TLS in-transit encryption mode for Redis cluster" +}If module.redis is conditionally created elsewhere, keep the guard; otherwise, consider removing it here or adding guards to all outputs for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main.tf(1 hunks)src/modules/redis_cluster/main.tf(2 hunks)src/modules/redis_cluster/outputs.tf(1 hunks)src/modules/redis_cluster/variables.tf(2 hunks)src/variables.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/modules/redis_cluster/main.tf
🧰 Additional context used
📓 Path-based instructions (2)
src/@(main|variables|outputs|providers|versions|context).tf
📄 CodeRabbit inference engine (AGENTS.md)
Keep the Terraform component as the source of truth under src/ with files: main.tf, variables.tf, outputs.tf, providers.tf, versions.tf, context.tf
Files:
src/variables.tfsrc/main.tf
**/*.tf
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tf: Use 2-space indentation for Terraform files
In Terraform, use lower_snake_case for variables and locals; keep resource/data source names descriptive and aligned with Cloud Posse null-label patterns
Do not commit formatting violations; run terraform fmt -recursive
Adhere to TFLint rules defined in .tflint.hcl
Files:
src/variables.tfsrc/main.tfsrc/modules/redis_cluster/variables.tfsrc/modules/redis_cluster/outputs.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/modules/redis_cluster/variables.tf (1)
92-95: LGTM: explicit type for kms_alias_name_ssmExplicit typing improves validation without behavior change.
Co-authored-by: Igor Rodionov <goruha@gmail.com>
|
/terratest |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/modules/redis_cluster/outputs.tf (1)
31-34: LGTM: Output correctly wired to nested module.Exposes
transit_encryption_modeviamodule.redis.transit_encryption_modeas suggested previously. No issues.
🧹 Nitpick comments (1)
src/modules/redis_cluster/outputs.tf (1)
31-34: Consider consistent naming with existing outputs.Most outputs in this file use the
cluster_prefix. For consistency, consider renaming tocluster_transit_encryption_mode. If you do, update any downstream references accordingly.-output "transit_encryption_mode" { +output "cluster_transit_encryption_mode" { value = module.redis.transit_encryption_mode description = "TLS in-transit encryption mode for Redis cluster" }If you prefer the current name for external API stability, keep it as-is and optionally add an alias output to maintain both.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/modules/redis_cluster/outputs.tf(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.tf
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tf: Use 2-space indentation for Terraform files
In Terraform, use lower_snake_case for variables and locals; keep resource/data source names descriptive and aligned with Cloud Posse null-label patterns
Do not commit formatting violations; run terraform fmt -recursive
Adhere to TFLint rules defined in .tflint.hcl
Files:
src/modules/redis_cluster/outputs.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 (2)
src/modules/redis_cluster/outputs.tf (2)
30-30: No action: formatting-only change.Whitespace aligns with
terraform fmt.
31-34: Confirm upstream support for transit_encryption_mode: ensure the external modulecloudposse/elasticache-redis/awsv1.10.0 defines an output namedtransit_encryption_mode; if it doesn’t, add or update that output upstream or adjust your wrapper accordingly.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
These changes were released in v1.537.0. |
what
why
references
.
Summary by CodeRabbit
New Features
Refactor
Chores