-
Notifications
You must be signed in to change notification settings - Fork 0
Make currently active region a terraform parameter #359
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
Conversation
💰 Infracost reportMonthly estimate increased by $6 📈
*Usage costs were estimated using infracost-usage.yml, see docs for other options. Estimate details |
WalkthroughAdds a new Makefile variable Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CF as CloudFront
participant FrontendTF as Frontend (Terraform config)
participant Lambda_E as Lambda us-east-2
participant Lambda_W as Lambda us-west-2
Note over FrontendTF,CF: Provision time — dynamic origins created per-region
FrontendTF->>CF: register origins for "us-east-2" and "us-west-2" (origin IDs include region)
CF-->>FrontendTF: origins created
Note over User,CF: Runtime request routing
User->>CF: HTTP request
alt CurrentActiveRegion = "us-east-2"
CF->>Lambda_E: route to origin "LambdaFunction-us-east-2"
else CurrentActiveRegion = "us-west-2"
CF->>Lambda_W: route to origin "LambdaFunction-us-west-2"
end
Lambda_E-->>CF: response
Lambda_W-->>CF: response
CF-->>User: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📒 Files selected for processing (2)
🚧 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). (2)
🔇 Additional comments (6)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
terraform/envs/qa/variables.tf (1)
53-61: Provide more descriptive validation error message and verify default value intent.The validation error message is generic and doesn't inform users which regions are allowed. Additionally, this variable lacks a default value, which means deployments will fail if not explicitly provided via CLI or a
.tfvarsfile. Verify if this is intentional for safety or an oversight.For better UX, include the allowed regions in the error message:
- validation { - condition = contains(["us-east-2", "us-west-2"], var.current_active_region) - error_message = "Invalid value for current_active_region" - } + validation { + condition = contains(["us-east-2", "us-west-2"], var.current_active_region) + error_message = "current_active_region must be either 'us-east-2' or 'us-west-2'" + }If a default value is appropriate for this environment, add:
default = "us-east-2".terraform/envs/prod/variables.tf (1)
52-60: Improve validation error message and clarify default value strategy.Same issue as the QA environment: the error message is not user-friendly. Update to include the allowed regions:
- validation { - condition = contains(["us-east-2", "us-west-2"], var.current_active_region) - error_message = "Invalid value for current_active_region" - } + validation { + condition = contains(["us-east-2", "us-west-2"], var.current_active_region) + error_message = "current_active_region must be either 'us-east-2' or 'us-west-2'" + }Consider adding a default value if appropriate for this environment.
Makefile (1)
4-4: Consider whether hardcoded region aligns with PR objective of "easily switching" regions.The PR description states the goal is to "allow for easily switching the region currently serving traffic." However, the
current_active_regionis hardcoded to "us-east-2" in the Makefile (line 4). To enable true runtime flexibility, consider allowing the region to be overridden at invocation time:-current_active_region = "us-east-2" +current_active_region ?= "us-east-2"This allows operators to switch regions via:
make deploy_prod current_active_region=us-west-2Alternatively, if hardcoding is intentional to prevent accidental region switches, document this decision and clarify the intended workflow for changing regions (e.g., via code review and merge).
Also applies to: 52-52, 59-59
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
Makefile(2 hunks)terraform/envs/prod/variables.tf(1 hunks)terraform/envs/qa/variables.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). (2)
- GitHub Check: Build Application
- GitHub Check: Run Unit Tests
Allows for easily switching the region currently serving traffic
Summary by CodeRabbit
New Features
Chores