Repository navigation
feat(terraform): default-off EventBridge ticks for ladder_run + fire_scheduled_purchases (L12) - #1369
Merged
Merged
Conversation
…purchases (G9) Add two default-off EventBridge rules cloned from the ri_exchange pattern: - aws_cloudwatch_event_rule.ladder_run / target / permission, gated count = var.enable_ladder_run_schedule ? 1 : 0, fires action "ladder_run" on var.ladder_run_schedule (default rate(1 day)). - aws_cloudwatch_event_rule.fire_scheduled_purchases / target / permission, gated count = var.enable_fire_scheduled_purchases_schedule ? 1 : 0, fires action "fire_scheduled_purchases" on var.fire_scheduled_purchases_schedule (default rate(15 minutes)). Four new module vars (enable_ladder_run_schedule, ladder_run_schedule, enable_fire_scheduled_purchases_schedule, fire_scheduled_purchases_schedule) threaded through environments/aws, azure, gcp (variables.tf + compute.tf). All four default to off/false; no existing resource is modified. No new IAM: each block reuses the EventBridge->Lambda invoke-permission pattern already used by ri_exchange, reap_stuck_purchases, and analytics_collect. No bootstrap PR needed. Verified action strings against internal/server/handler.go scheduledEventActions map: "fire_scheduled_purchases" maps to TaskFireScheduledPurchases (line 71). "ladder_run" is not yet in the map (pending PR #1362); the rule is default-off and safe to add ahead of the dispatcher wiring. Gates: terraform fmt -check -recursive (exit 0), terraform validate in terraform/modules/compute/aws/lambda with -backend=false init (exit 0).
cristim
added
triaged
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughTerraform adds optional EventBridge schedules for commitment-ladder planning and scheduled-purchase sweeps in both Lambda and Fargate compute paths, with configurable enablement and rate expressions. Lambda Function URL authorization is derived from CDN enablement. ChangesScheduled automation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EventBridge
participant ComputeTarget
participant CUDly
EventBridge->>ComputeTarget: Trigger configured schedule
ComputeTarget->>CUDly: Run ladder_run or fire_scheduled_purchases
CUDly-->>ComputeTarget: Complete scheduled workflow
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@terraform/modules/compute/aws/fargate/main.tf`:
- Around line 1054-1074: The EventBridge IAM policies are overly broad. In both
policy blocks at terraform/modules/compute/aws/fargate/main.tf lines 1054-1074
and 1153-1173, constrain each ecs:RunTask statement with an ecs:cluster
condition referencing aws_ecs_cluster.main.arn, and each iam:PassRole statement
with a StringEquals iam:PassedToService condition set to
ecs-tasks.amazonaws.com.
In `@terraform/modules/compute/aws/lambda/main.tf`:
- Around line 673-675: Update the scheduled-action dispatch configuration by
adding "ladder_run" to the scheduledEventActions collection used by the handler,
while preserving the existing jsonencoded action payload and other registered
actions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eb3eff2c-57f9-4144-b5bd-3b5e64eb091e
📒 Files selected for processing (6)
terraform/environments/aws/compute.tfterraform/environments/aws/variables.tfterraform/modules/compute/aws/fargate/main.tfterraform/modules/compute/aws/fargate/variables.tfterraform/modules/compute/aws/lambda/main.tfterraform/modules/compute/aws/lambda/variables.tf
…on ladder ticks Address CodeRabbit findings on the L12 EventBridge ticks: - Tighten the two new Fargate EventBridge->ECS roles (ladder_run, fire_scheduled_purchases). Add ArnEquals ecs:cluster = aws_ecs_cluster.main.arn on the ecs:RunTask statement and StringEquals iam:PassedToService = "ecs-tasks.amazonaws.com" on the iam:PassRole statement, matching AWS best practice for EventBridge-triggered ECS tasks. Only the two new blocks are touched; the existing ri_exchange block is unchanged. - Document the intentional dispatch state above the ladder_run rule in both lambda/main.tf and fargate/main.tf: the "ladder_run" scheduled action is registered by the ladder_run task (PR #1362); the rule is default-off (count-gated) and enabling it before that task ships causes a loud dispatch error, never a silent no-op. No handler.go change here (that is #1362 scope; this PR stays terraform-only + default-off).
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Member
Author
|
Merged to main after: Fable review (SHIP - IAM verified byte-identical scope to the existing ri_exchange pattern), CodeRabbit IAM hardening applied (ecs:cluster ArnEquals on RunTask + iam:PassedToService on PassRole for both new roles), and the ladder_run dispatch-registration thread resolved by #1362 landing on main (the action is now registered; the rule stays default-off). Both EventBridge ticks (ladder_run daily, fire_scheduled_purchases 15-min) exist for Lambda and Fargate, count-gated off. Enabling is an explicit operator step via enable_*_schedule vars. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Work item L12 of the auto-laddering full-automation plan (closes gap G9). Adds two default-off EventBridge scheduled ticks, for both the Lambda and Fargate compute platforms:
ladder_run(daily) — drives the scheduled laddering task added in feat(ladder): add ladder_run scheduled task (plan-only milestone) #1362.fire_scheduled_purchases(every 15 min) — the gap analysis found this action had no EventBridge rule anywhere, yet the auto-approve delayed-fire path reuses it verbatim; without a tick, delayed/scheduled purchases would never fire.Both are cloned exactly from the existing
ri_exchange/reap_stuck_purchasesscheduled-rule pattern.Safety
count = var.enable_* ? 1 : 0, and both enable vars defaultfalsein every module and inenvironments/aws. Enabling is an explicit operator step.eventbridge_ri_exchangeblock (ecs:RunTaskon the one task definition,iam:PassRoleon exactly the two task roles, no wildcards); the Lambda permission grants onlyevents.amazonaws.cominvoke scoped to the specific rule ARN. The deploy SA already holds theevents:*/iam:CreateRolebootstrap perms for thecudly-*prefixed names, so no bootstrap-permission change is required.ladder_runisn't in the Go dispatcher until feat(ladder): add ladder_run scheduled task (plan-only milestone) #1362 merges, but a disabled rule creates zero resources, and premature enablement would error loudly (dispatchTaskrejects unknown task types), never silently no-op.Variables (all default-off)
enable_ladder_run_schedule(bool,false),ladder_run_schedule(rate(1 day))enable_fire_scheduled_purchases_schedule(bool,false),fire_scheduled_purchases_schedule(rate(15 minutes))Threaded through
modules/compute/aws/{lambda,fargate}andenvironments/aws. Azure/GCP are intentionally untouched (no EventBridge; their ladder scheduling is Phase-4, L19-21).Verification (exit 0)
terraform fmt -check -recursiveclean on all touched modules.terraform init -backend=false && terraform validatein the lambda and fargate modules: valid.Terraform validate+tflintpass (no unused/undeclared vars).internal/server/handler.goscheduledEventActions/ dispatch map.Part of LeanerCloud/cloud-commitments-platform#70. Tracker LeanerCloud/cloud-commitments-go#27.
Summary by CodeRabbit