feat: expose additional DNS outputs #50
Conversation
WalkthroughAdded a new input Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Root as Root Module
participant Child as aurora_postgres_cluster (v2.1.1)
participant Outputs as Module Outputs
rect rgb(240,248,255)
Root->>Child: pass inputs (including promotion_tier)
Note right of Child: Child manages cluster/instances
Child-->>Root: exposes attributes (endpoint, reader_endpoint, instance_endpoints)
Root->>Outputs: map child attributes to outputs
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Important Do not edit the Please update the Could you fix it @Benbentwo? 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/outputs.tf (2)
22-30: Good additions; clarify overlap with existing host outputs.
cluster_endpoint/reader_endpointmirrormaster_hostname/replicas_hostnamevalues from the same module. Consider documenting the aliasing/deprecation plan to avoid API confusion for consumers.Would you like me to add a short note to README.yaml indicating:
- cluster_endpoint ≈ master_hostname (alias)
- reader_endpoint ≈ replicas_hostname (alias)?
22-35: Optional: declare explicit output types for stability.Not required, but adding types makes interface clearer.
output "cluster_endpoint" { + type = string value = module.aurora_postgres_cluster.endpoint description = "Postgres cluster endpoint" } output "reader_endpoint" { + type = string value = module.aurora_postgres_cluster.reader_endpoint description = "Postgres reader endpoint" } output "instance_endpoints" { + type = list(string) value = module.aurora_postgres_cluster.instance_endpoints description = "List of Postgres instance endpoints" }README.md (1)
424-435: Note potential duplication between new and legacy outputs.Consider a short note that:
cluster_endpointis equivalent tomaster_hostnamereader_endpointis equivalent toreplicas_hostname
to reduce confusion for consumers.If acceptable, I’ll add these notes to README.yaml so the generated docs stay consistent.
src/README.md (1)
370-383: Add aliasing note for newly exposed endpoints vs legacy hostnames.Recommend noting the equivalence to help users choose the preferred outputs.
I can add this to README.yaml and regenerate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
README.md(1 hunks)src/README.md(1 hunks)src/outputs.tf(1 hunks)
⏰ 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 (8)
README.md (4)
321-330: Docs build pre-commit failure:iam_rolesmodule path likely causing lookup issues.
../account-map/modules/iam-rolesappears in the Modules table and may trigger “module is not found” duringpre-commit run. Either ensure the relative path exists in this repo during CI/docs generation or exclude it from docs generation.I can propose README.yaml adjustments to annotate this module as external or omit unresolved local modules from the generated table. Want me to open a follow-up?
426-426: LGTM: Addedcluster_endpointto Outputs.
430-430: LGTM: Addedinstance_endpointsto Outputs.
433-433: LGTM: Addedreader_endpointto Outputs.src/README.md (4)
268-276: Docs build pre-commit failure:iam_rolesmodule path likely causing lookup issues.Same as root README: the relative path module can break docs checks. Align handling here as well (ensure path exists in CI context or omit from generated table).
I can update README.yaml to avoid listing non-resolvable local modules in generated docs.
374-374: LGTM: Addedcluster_endpointto Outputs.
378-378: LGTM: Addedinstance_endpointsto Outputs.
381-381: LGTM: Addedreader_endpointto Outputs.
|
/terratest |
Benbentwo
added
the
do not merge
|
Need to expose endpoints, but they only exist in latest version of module. Need to see what it takes to update module. Adding Do not merge until then. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/README.md (1)
351-351: Tighten grammar and scope note for “promotion_tier”.Clarify failover wording and explicitly scope the scaling behavior to Serverless v2; convert bare URL to a markdown link.
-<a name="input_promotion_tier"></a> [promotion_tier](#input_promotion_tier) | Failover Priority setting on instance level. The reader who has lower tier has higher priority to get promoted to writer.<br/><br/>Readers in promotion tiers 0 and 1 scale at the same time as the writer. Readers in promotion tiers 2–15 scale independently from the writer. For more information, see: https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/aurora-serverless-v2.how-it-works.html#aurora-serverless-v2.how-it-works.scaling | `number` | `0` | no | +<a name="input_promotion_tier"></a> [promotion_tier](#input_promotion_tier) | Instance-level failover priority. Lower tiers have higher priority for promotion to writer.<br/><br/>Serverless v2 note: readers in tiers 0–1 scale with the writer; readers in tiers 2–15 scale independently. See the [Aurora Serverless v2 scaling docs](https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/aurora-serverless-v2.how-it-works.html#aurora-serverless-v2.how-it-works.scaling). | `number` | `0` | no |README.md (1)
403-403: Polish “promotion_tier” description (grammar + link).Mirror the clearer wording and Serverless v2 scope note used in src/README.md.
-<a name="input_promotion_tier"></a> [promotion_tier](#input_promotion_tier) | Failover Priority setting on instance level. The reader who has lower tier has higher priority to get promoted to writer.<br/><br/>Readers in promotion tiers 0 and 1 scale at the same time as the writer. Readers in promotion tiers 2–15 scale independently from the writer. For more information, see: https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/aurora-serverless-v2.how-it-works.html#aurora-serverless-v2.how-it-works.scaling | `number` | `0` | no | +<a name="input_promotion_tier"></a> [promotion_tier](#input_promotion_tier) | Instance-level failover priority. Lower tiers have higher priority for promotion to writer.<br/><br/>Serverless v2 note: readers in tiers 0–1 scale with the writer; readers in tiers 2–15 scale independently. See the [Aurora Serverless v2 scaling docs](https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/aurora-serverless-v2.how-it-works.html#aurora-serverless-v2.how-it-works.scaling). | `number` | `0` | no |src/cluster-regional.tf (1)
17-17: Pass-through of promotion_tier is correct; consider documenting valid range.AWS accepts 0–15. Add a validation in variables.tf to enforce bounds.
variable "promotion_tier" { type = number default = 0 description = <<-EOT @@ EOT } +# +# Enforce AWS bounds (0–15) +validation { + condition = var.promotion_tier >= 0 && var.promotion_tier <= 15 + error_message = "promotion_tier must be between 0 and 15." +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
README.md(2 hunks)src/README.md(2 hunks)src/cluster-regional.tf(2 hunks)src/variables.tf(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~403-~403: There might be a mistake here.
Context: ...tting on instance level. The reader who has lower tier has higher priority to get p...
(QB_NEW_EN)
[grammar] ~403-~403: There might be a mistake here.
Context: ...it-works.scaling | number | 0 | no | | <a name="input_rds_monitoring_interval...
(QB_NEW_EN)
src/README.md
[grammar] ~351-~351: There might be a mistake here.
Context: ...tting on instance level. The reader who has lower tier has higher priority to get p...
(QB_NEW_EN)
[grammar] ~351-~351: There might be a mistake here.
Context: ...it-works.scaling | number | 0 | no | | <a name="input_rds_monitoring_interval...
(QB_NEW_EN)
⏰ 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 (3)
src/README.md (1)
375-383: Outputs look good; ensure docs reflect module version that provides them.cluster_endpoint, instance_endpoints, reader_endpoint additions are correct. Verify the “Modules” table shows the rds-cluster version that actually exposes these (your code pins 2.1.1). If the table still lists 1.18.0, update README.yaml so generated docs display 2.1.1.
README.md (1)
427-435: Outputs LGTM; sync module version in docs.The three new endpoints are valuable. Ensure the “Modules” table reflects cloudposse/rds-cluster/aws v2.1.1 (the code pin) so readers know where these outputs originate. Update README.yaml and regenerate.
src/cluster-regional.tf (1)
8-8: Version bump: verify compatibility and that outputs exist in v2.1.1.Confirm rds-cluster v2.1.1 exposes endpoint, reader_endpoint, and instance_endpoints and accepts promotion_tier. Run a plan against a fixture to catch any breaking changes from 1.18.0→2.1.1 before lifting “do not merge”.
|
/terratest |
|
Removing |
Benbentwo
removed
the
do not merge
|
@RoseSecurity PTAL if you get a moment |
RoseSecurity
deleted the
codex/expose-instance_endpoints-and-dns-outputs
branch
|
These changes were released in v1.538.4. |
Summary
Testing
atmos docs generate readmeatmos docs generate readme-simplepre-commit run --files README.md src/README.md src/outputs.tf(fails: "iam_roles" module is not found)https://chatgpt.com/codex/tasks/task_b_68b049e0d0fc832b9f6a2872e4f16a56
Summary by CodeRabbit
New Features
Documentation