From 678df2e88cc78c26d8d088f81dddec52484f5a23 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 08:10:36 +0000 Subject: [PATCH 1/2] fix(cfn): schedule finalize, analytics, reap, fire and notification tasks Add an EventBridge rule and invoke permission to the CUDly CloudFormation stack for each scheduled server task it was missing, each switched by an Enable*Schedule parameter with a matching *Schedule expression: - finalize_revocations: on, rate(1 hour) - analytics_collect: on, rate(1 day) - reap_stuck_purchases: off (owner decision on #712: the PurchaseCheck rule runs process only daily, so the reaper's 1h20m approved floor would still fail stranded approvals before process re-drives them) - fire_scheduled_purchases: off (opt-in), rate(15 minutes) - send_notifications: off (opt-in), rate(1 day) Cadences match the Terraform Lambda module defaults. The existing collection and PurchaseCheck rules are unchanged. A new Go test parses the template and checks, for every required server task, one rule targeting the Lambda with {"action": task}, one permission gated by the rule's condition, an enable default that follows the opt-in policy (with listed exceptions), a schedule parameter matching the Lambda module default, and a name suffix no longer than the oldest rule's. cleanup stays a documented CloudFormation gap until the next commit. Refs #712 Co-Authored-By: Claude Opus 5.5 --- cloudformation/stacks/CUDly/template.yaml | 233 ++++++++++ .../compute/scheduled_task_cfn_test.go | 400 ++++++++++++++++++ .../compute/scheduled_task_coverage_test.go | 7 +- 3 files changed, 636 insertions(+), 4 deletions(-) create mode 100644 terraform/modules/compute/scheduled_task_cfn_test.go diff --git a/cloudformation/stacks/CUDly/template.yaml b/cloudformation/stacks/CUDly/template.yaml index b801ef6aa..0a7471902 100644 --- a/cloudformation/stacks/CUDly/template.yaml +++ b/cloudformation/stacks/CUDly/template.yaml @@ -37,6 +37,20 @@ Metadata: - LogRetentionPeriod - ExecutionFrequency + - Label: + default: Scheduled Tasks + Parameters: + - EnableFinalizeRevocationsSchedule + - FinalizeRevocationsSchedule + - EnableAnalyticsCollectSchedule + - AnalyticsCollectSchedule + - EnableReapStuckPurchasesSchedule + - ReapStuckPurchasesSchedule + - EnableFireScheduledPurchasesSchedule + - FireScheduledPurchasesSchedule + - EnableSendNotificationsSchedule + - SendNotificationsSchedule + ParameterLabels: EmailAddress: default: Notification email address @@ -140,6 +154,84 @@ Parameters: Default: "rate(1 day)" Description: How often to check for new recommendations + # Scheduled server tasks (#712). Defaults and cadences match the Terraform + # Lambda module (terraform/modules/compute/aws/lambda/variables.tf) except + # where noted. Opt-in tasks start purchases or retry them, so each stack + # enables them deliberately. + EnableFinalizeRevocationsSchedule: + Type: String + Default: "true" + AllowedValues: + - "true" + - "false" + Description: Run the finalize_revocations task on a schedule (idempotent audit repair of revoked purchases) + + FinalizeRevocationsSchedule: + Type: String + Default: "rate(1 hour)" + Description: EventBridge schedule expression for finalize_revocations + + EnableAnalyticsCollectSchedule: + Type: String + Default: "true" + AllowedValues: + - "true" + - "false" + Description: Run the analytics_collect task on a schedule (savings snapshot, partition maintenance, retention and view refresh) + + AnalyticsCollectSchedule: + Type: String + Default: "rate(1 day)" + Description: EventBridge schedule expression for analytics_collect + + EnableReapStuckPurchasesSchedule: + Type: String + Default: "false" + AllowedValues: + - "true" + - "false" + Description: >- + Run the reap_stuck_purchases task on a schedule. It marks executions stuck + in running (after 10 minutes) or approved (after 1h20m) as failed. Off by + default on this stack, unlike the Terraform Lambda module: the + PurchaseCheck rule runs process_scheduled_purchases only daily, so an + approved execution stranded by a crash would be failed before the next + process run re-drives it (#894). + + ReapStuckPurchasesSchedule: + Type: String + Default: "rate(5 minutes)" + Description: EventBridge schedule expression for reap_stuck_purchases + + EnableFireScheduledPurchasesSchedule: + Type: String + Default: "false" + AllowedValues: + - "true" + - "false" + Description: >- + Run the fire_scheduled_purchases task on a schedule. It executes + purchases in status scheduled whose scheduled time has passed (the + delayed auto-approve path), so it makes real purchases. Opt-in. + + FireScheduledPurchasesSchedule: + Type: String + Default: "rate(15 minutes)" + Description: EventBridge schedule expression for fire_scheduled_purchases + + EnableSendNotificationsSchedule: + Type: String + Default: "false" + AllowedValues: + - "true" + - "false" + Description: Run the send_notifications task on a schedule (upcoming-purchase emails). Opt-in. + + SendNotificationsSchedule: + Type: String + Default: "rate(1 day)" + Description: EventBridge schedule expression for send_notifications + CrossAccountTargetAccountIds: Type: CommaDelimitedList Default: "" @@ -190,6 +282,31 @@ Conditions: - Ref: CrossAccountTargetAccountIds - "" + ScheduleFinalizeRevocations: + Fn::Equals: + - Ref: EnableFinalizeRevocationsSchedule + - "true" + + ScheduleAnalyticsCollect: + Fn::Equals: + - Ref: EnableAnalyticsCollectSchedule + - "true" + + ScheduleReapStuckPurchases: + Fn::Equals: + - Ref: EnableReapStuckPurchasesSchedule + - "true" + + ScheduleFireScheduledPurchases: + Fn::Equals: + - Ref: EnableFireScheduledPurchasesSchedule + - "true" + + ScheduleSendNotifications: + Fn::Equals: + - Ref: EnableSendNotificationsSchedule + - "true" + Resources: # ============================================================================= # DynamoDB Tables @@ -733,6 +850,122 @@ Resources: Principal: events.amazonaws.com SourceArn: !GetAtt PurchaseCheckRule.Arn + # Remaining scheduled server tasks (#712), one rule and invoke permission + # each, switched by the Enable*Schedule parameters. Name suffixes are at most + # as long as "-ScheduledExecution", so they fit EventBridge's 64-character + # limit for any stack name the existing rules accept. Retries stay at the + # EventBridge default: a re-run is safe under the server's per-task advisory + # lock and row compare-and-swap. + FinalizeRevocationsRule: + Type: AWS::Events::Rule + Condition: ScheduleFinalizeRevocations + Properties: + Name: !Sub "${AWS::StackName}-Finalize" + Description: Scheduled finalize_revocations task + ScheduleExpression: !Ref FinalizeRevocationsSchedule + State: ENABLED + Targets: + - Id: LambdaTarget + Arn: !GetAtt LambdaFunction.Arn + Input: '{"action": "finalize_revocations"}' + + FinalizeRevocationsPermission: + Type: AWS::Lambda::Permission + Condition: ScheduleFinalizeRevocations + Properties: + Action: lambda:InvokeFunction + FunctionName: !Ref LambdaFunction + Principal: events.amazonaws.com + SourceArn: !GetAtt FinalizeRevocationsRule.Arn + + AnalyticsCollectRule: + Type: AWS::Events::Rule + Condition: ScheduleAnalyticsCollect + Properties: + Name: !Sub "${AWS::StackName}-AnalyticsCollect" + Description: Scheduled analytics_collect task + ScheduleExpression: !Ref AnalyticsCollectSchedule + State: ENABLED + Targets: + - Id: LambdaTarget + Arn: !GetAtt LambdaFunction.Arn + Input: '{"action": "analytics_collect"}' + + AnalyticsCollectPermission: + Type: AWS::Lambda::Permission + Condition: ScheduleAnalyticsCollect + Properties: + Action: lambda:InvokeFunction + FunctionName: !Ref LambdaFunction + Principal: events.amazonaws.com + SourceArn: !GetAtt AnalyticsCollectRule.Arn + + ReapStuckPurchasesRule: + Type: AWS::Events::Rule + Condition: ScheduleReapStuckPurchases + Properties: + Name: !Sub "${AWS::StackName}-ReapStuck" + Description: Scheduled reap_stuck_purchases task + ScheduleExpression: !Ref ReapStuckPurchasesSchedule + State: ENABLED + Targets: + - Id: LambdaTarget + Arn: !GetAtt LambdaFunction.Arn + Input: '{"action": "reap_stuck_purchases"}' + + ReapStuckPurchasesPermission: + Type: AWS::Lambda::Permission + Condition: ScheduleReapStuckPurchases + Properties: + Action: lambda:InvokeFunction + FunctionName: !Ref LambdaFunction + Principal: events.amazonaws.com + SourceArn: !GetAtt ReapStuckPurchasesRule.Arn + + FireScheduledPurchasesRule: + Type: AWS::Events::Rule + Condition: ScheduleFireScheduledPurchases + Properties: + Name: !Sub "${AWS::StackName}-FireScheduled" + Description: Scheduled fire_scheduled_purchases task + ScheduleExpression: !Ref FireScheduledPurchasesSchedule + State: ENABLED + Targets: + - Id: LambdaTarget + Arn: !GetAtt LambdaFunction.Arn + Input: '{"action": "fire_scheduled_purchases"}' + + FireScheduledPurchasesPermission: + Type: AWS::Lambda::Permission + Condition: ScheduleFireScheduledPurchases + Properties: + Action: lambda:InvokeFunction + FunctionName: !Ref LambdaFunction + Principal: events.amazonaws.com + SourceArn: !GetAtt FireScheduledPurchasesRule.Arn + + SendNotificationsRule: + Type: AWS::Events::Rule + Condition: ScheduleSendNotifications + Properties: + Name: !Sub "${AWS::StackName}-SendNotifications" + Description: Scheduled send_notifications task + ScheduleExpression: !Ref SendNotificationsSchedule + State: ENABLED + Targets: + - Id: LambdaTarget + Arn: !GetAtt LambdaFunction.Arn + Input: '{"action": "send_notifications"}' + + SendNotificationsPermission: + Type: AWS::Lambda::Permission + Condition: ScheduleSendNotifications + Properties: + Action: lambda:InvokeFunction + FunctionName: !Ref LambdaFunction + Principal: events.amazonaws.com + SourceArn: !GetAtt SendNotificationsRule.Arn + # ============================================================================= # SNS Topic for Notifications # ============================================================================= diff --git a/terraform/modules/compute/scheduled_task_cfn_test.go b/terraform/modules/compute/scheduled_task_cfn_test.go new file mode 100644 index 000000000..ac61a9683 --- /dev/null +++ b/terraform/modules/compute/scheduled_task_cfn_test.go @@ -0,0 +1,400 @@ +// CloudFormation checks for issue #712: the CUDly stack must schedule every +// required server task with one EventBridge rule that invokes the stack's +// Lambda with {"action": ""}, an invoke permission gated exactly like +// its rule, and an enable parameter whose default follows the opt-in policy +// of the coverage test. The coverage test only finds the task names; this +// one parses the template. +package compute_test + +import ( + "encoding/json" + "fmt" + "os" + "regexp" + "slices" + "strings" + "testing" + + "gopkg.in/yaml.v3" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/server" +) + +const ( + cfnTemplatePath = "../../../cloudformation/stacks/CUDly/template.yaml" + lambdaModuleVars = "aws/lambda/variables.tf" + // cfnLongestSuffix is the name suffix of the oldest rule. New rules must not + // be longer, so they fit the 64-char EventBridge limit for every stack name + // that rule already accepts. + cfnLongestSuffix = "-ScheduledExecution" +) + +// cfnAlwaysOn are the rules that predate #712 and have no enable parameter. +// Turning them into opt-in switches would change running stacks, so they stay. +var cfnAlwaysOn = map[string]bool{ + "collect_recommendations": true, + "process_scheduled_purchases": true, +} + +// cfnDefaultExceptions are tasks whose CloudFormation default is the opposite +// of the opt-in policy, with the reason. +var cfnDefaultExceptions = map[string]string{ + "process_scheduled_purchases": "the daily PurchaseCheck rule predates the opt-in policy; removing it would stop purchases on existing stacks", + "reap_stuck_purchases": "PurchaseCheck runs process only daily, so the reaper's 1h20m approved floor (#894) would still fail " + + "stranded approvals before process re-drives them", +} + +// cfnStack is a parsed template: Resources, Parameters and Conditions by +// logical id. Short-form tags are rewritten to their long form (!Ref to Ref, +// !X to Fn::X, !GetAtt "A.B" to Fn::GetAtt [A, B]). +type cfnStack struct { + Resources, Parameters, Conditions map[string]map[string]any +} + +func cfnValue(n *yaml.Node) any { + var v any + switch n.Kind { + case yaml.DocumentNode: + return cfnValue(n.Content[0]) + case yaml.AliasNode: + return cfnValue(n.Alias) + case yaml.MappingNode: + m := map[string]any{} + for i := 0; i+1 < len(n.Content); i += 2 { + m[n.Content[i].Value] = cfnValue(n.Content[i+1]) + } + v = m + case yaml.SequenceNode: + s := make([]any, 0, len(n.Content)) + for _, c := range n.Content { + s = append(s, cfnValue(c)) + } + v = s + default: + v = n.Value + } + if !strings.HasPrefix(n.Tag, "!") || strings.HasPrefix(n.Tag, "!!") { + return v + } + if n.Tag == "!Ref" { + return map[string]any{"Ref": v} + } + return map[string]any{"Fn::" + n.Tag[1:]: v} +} + +func parseCFN(src []byte) (*cfnStack, error) { + var root yaml.Node + if err := yaml.Unmarshal(src, &root); err != nil { + return nil, err + } + top, ok := cfnValue(&root).(map[string]any) + if !ok { + return nil, fmt.Errorf("template is not a mapping") + } + section := func(name string) map[string]map[string]any { + out := map[string]map[string]any{} + raw, _ := top[name].(map[string]any) + for id, v := range raw { + out[id], _ = v.(map[string]any) + } + return out + } + return &cfnStack{section("Resources"), section("Parameters"), section("Conditions")}, nil +} + +func props(res map[string]any) map[string]any { + p, _ := res["Properties"].(map[string]any) + return p +} + +// ref returns the logical id of {Ref: id}. +func ref(v any) string { + m, _ := v.(map[string]any) + s, _ := m["Ref"].(string) + return s +} + +// getAtt returns "Id.Attr" for both GetAtt forms. +func getAtt(v any) string { + m, _ := v.(map[string]any) + switch a := m["Fn::GetAtt"].(type) { + case string: + return a + case []any: + parts := make([]string, 0, len(a)) + for _, p := range a { + s, _ := p.(string) + parts = append(parts, s) + } + return strings.Join(parts, ".") + } + return "" +} + +func str(v any) string { + s, _ := v.(string) + return s +} + +// cfnRuleTask returns the task a rule's single target sends, or a problem. +func cfnRuleTask(id string, p map[string]any) (string, string) { + targets, _ := p["Targets"].([]any) + if len(targets) != 1 { + return "", fmt.Sprintf("%s: want exactly one target, got %d", id, len(targets)) + } + t, _ := targets[0].(map[string]any) + if getAtt(t["Arn"]) != "LambdaFunction.Arn" { + return "", id + ": target is not LambdaFunction.Arn" + } + var input map[string]any + if err := json.Unmarshal([]byte(str(t["Input"])), &input); err != nil || len(input) != 1 || str(input["action"]) == "" { + return "", fmt.Sprintf("%s: target Input must be exactly {\"action\": }, got %q", id, str(t["Input"])) + } + return str(input["action"]), "" +} + +var cfnNameRe = regexp.MustCompile(`^\$\{AWS::StackName\}(-[A-Za-z]+)$`) + +// cfnRules maps each scheduled task to the logical ids of its rules and +// reports malformed rules. +func cfnRules(s *cfnStack) (map[string][]string, []string) { + tasks := map[string][]string{} + var problems []string + names := map[string]string{} + for id, res := range s.Resources { + if res["Type"] != "AWS::Events::Rule" { + continue + } + p := props(res) + task, prob := cfnRuleTask(id, p) + if prob != "" { + problems = append(problems, prob) + continue + } + tasks[task] = append(tasks[task], id) + name, _ := p["Name"].(map[string]any) + m := cfnNameRe.FindStringSubmatch(str(name["Fn::Sub"])) + switch { + case m == nil: + problems = append(problems, id+`: Name must be !Sub "${AWS::StackName}-"`) + case len(m[1]) > len(cfnLongestSuffix): + problems = append(problems, fmt.Sprintf("%s: name suffix %s is longer than %s", id, m[1], cfnLongestSuffix)) + case names[m[1]] != "": + problems = append(problems, id+": name suffix "+m[1]+" also used by "+names[m[1]]) + default: + names[m[1]] = id + } + if p["State"] != "ENABLED" { + problems = append(problems, id+": State must be ENABLED; the enable parameter switches it") + } + } + return tasks, problems +} + +// cfnPermissionProblems checks that exactly one invoke permission points at +// the rule and that it carries the rule's condition. +func cfnPermissionProblems(s *cfnStack, ruleID string) []string { + var found []string + for id, res := range s.Resources { + if res["Type"] == "AWS::Lambda::Permission" && getAtt(props(res)["SourceArn"]) == ruleID+".Arn" { + found = append(found, id) + } + } + if len(found) != 1 { + return []string{fmt.Sprintf("%s: want exactly one Lambda permission with SourceArn %s.Arn, got %v", ruleID, ruleID, found)} + } + perm := s.Resources[found[0]] + p := props(perm) + var problems []string + if p["Action"] != "lambda:InvokeFunction" || p["Principal"] != "events.amazonaws.com" || ref(p["FunctionName"]) != "LambdaFunction" { + problems = append(problems, found[0]+": must grant lambda:InvokeFunction on LambdaFunction to events.amazonaws.com") + } + if perm["Condition"] != s.Resources[ruleID]["Condition"] { + problems = append(problems, fmt.Sprintf("%s: Condition %v differs from its rule's %v", found[0], perm["Condition"], s.Resources[ruleID]["Condition"])) + } + return problems +} + +// cfnEnableDefault resolves a rule's condition to its enable parameter's +// default: Fn::Equals [Ref: , "true"] on a "true"/"false" parameter. +func cfnEnableDefault(s *cfnStack, ruleID, cond string) (bool, string) { + eq, _ := s.Conditions[cond]["Fn::Equals"].([]any) + if len(eq) != 2 || eq[1] != "true" || ref(eq[0]) == "" { + return false, fmt.Sprintf("%s: condition %s must be Fn::Equals [Ref: , \"true\"]", ruleID, cond) + } + param := s.Parameters[ref(eq[0])] + allowed, _ := param["AllowedValues"].([]any) + if param == nil || !slices.Equal(allowed, []any{"true", "false"}) { + return false, fmt.Sprintf("%s: parameter %s must allow exactly \"true\" and \"false\"", ruleID, ref(eq[0])) + } + def := str(param["Default"]) + if def != "true" && def != "false" { + return false, fmt.Sprintf("%s: parameter %s default %q is not \"true\" or \"false\"", ruleID, ref(eq[0]), def) + } + return def == "true", "" +} + +var tfStringDefaultRe = regexp.MustCompile(`(?m)^\s*default\s*=\s*"([^"]*)"`) + +// cfnScheduleProblem checks that a switchable rule's schedule is a parameter +// whose default matches the Lambda module's default for the same task. +func cfnScheduleProblem(s *cfnStack, ruleID, task, lambdaVars string) string { + param := ref(props(s.Resources[ruleID])["ScheduleExpression"]) + if param == "" || s.Parameters[param] == nil { + return ruleID + ": ScheduleExpression must be a Ref to a stack parameter" + } + body, ok := blockOnce(stripComments(lambdaVars), `variable\s+"`+taskVars[task].schedule+`"\s*\{`) + m := tfStringDefaultRe.FindStringSubmatch(body) + if !ok || m == nil { + return ruleID + ": Lambda module has no string default for " + taskVars[task].schedule + } + if got := str(s.Parameters[param]["Default"]); got != m[1] { + return fmt.Sprintf("%s: %s defaults to %q, the Lambda module's %s to %q", ruleID, param, got, taskVars[task].schedule, m[1]) + } + return "" +} + +// cfnTaskProblems checks one required task's rule, permission, default and +// schedule. +func cfnTaskProblems(s *cfnStack, task string, ruleIDs []string, lambdaVars string) []string { + if len(ruleIDs) != 1 { + return []string{fmt.Sprintf("%s: want exactly one rule, got %v", task, ruleIDs)} + } + id := ruleIDs[0] + problems := cfnPermissionProblems(s, id) + cond := str(s.Resources[id]["Condition"]) + if (cond == "") != cfnAlwaysOn[task] { + return append(problems, fmt.Sprintf("%s: has condition %q; only the pre-existing rules %v may be unconditional", id, cond, cfnAlwaysOn)) + } + on := true + if cond != "" { + var prob string + if on, prob = cfnEnableDefault(s, id, cond); prob != "" { + return append(problems, prob) + } + if prob := cfnScheduleProblem(s, id, task, lambdaVars); prob != "" { + problems = append(problems, prob) + } + } + _, exception := cfnDefaultExceptions[task] + if want := defaultOn(task) != exception; on != want { + problems = append(problems, fmt.Sprintf("%s: on by default = %v, want %v (opt-in policy, exception %v)", task, on, want, exception)) + } + return problems +} + +// cfnProblems runs every check on a parsed template. +func cfnProblems(s *cfnStack, lambdaVars string) []string { + tasks, problems := cfnRules(s) + for task := range tasks { + if _, err := server.ParseScheduledTaskType(task); err != nil { + problems = append(problems, "rule schedules unknown task "+task) + } + } + for _, task := range requiredTasks() { + if slices.Contains(knownGaps["cloudformation"][dimTask], task) { + continue // the coverage test holds it to the documented gap + } + problems = append(problems, cfnTaskProblems(s, task, tasks[task], lambdaVars)...) + } + return problems +} + +func readCFN(t *testing.T) (tpl, lambdaVars string) { + t.Helper() + a, err := os.ReadFile(cfnTemplatePath) + if err != nil { + t.Fatal(err) + } + b, err := os.ReadFile(lambdaModuleVars) + if err != nil { + t.Fatal(err) + } + return string(a), string(b) +} + +func TestCloudFormationSchedulesEveryRequiredTask(t *testing.T) { + for task, reason := range cfnDefaultExceptions { + if !slices.Contains(requiredTasks(), task) || reason == "" { + t.Errorf("cfnDefaultExceptions[%s] must name a required task and give a reason", task) + } + } + tpl, lambdaVars := readCFN(t) + s, err := parseCFN([]byte(tpl)) + if err != nil { + t.Fatal(err) + } + for _, prob := range cfnProblems(s, lambdaVars) { + t.Error(prob) + } +} + +func TestCloudFormationScheduleCheckNegatives(t *testing.T) { + tpl, lambdaVars := readCFN(t) + for name, edit := range map[string]func(string) string{ + "rule removed": func(s string) string { + return strings.Replace(s, `Input: '{"action": "finalize_revocations"}'`, `Input: '{"action": "analytics_collect"}'`, 1) + }, + "opt-in task on by default": func(s string) string { + return regexp.MustCompile(`(EnableFireScheduledPurchasesSchedule:\n Type: String\n Default: )"false"`).ReplaceAllString(s, `$1"true"`) + }, + "default-on task off by default": func(s string) string { + return regexp.MustCompile(`(EnableFinalizeRevocationsSchedule:\n Type: String\n Default: )"true"`).ReplaceAllString(s, `$1"false"`) + }, + "exception flipped to policy": func(s string) string { + return regexp.MustCompile(`(EnableReapStuckPurchasesSchedule:\n Type: String\n Default: )"false"`).ReplaceAllString(s, `$1"true"`) + }, + "permission points at another rule": func(s string) string { + return strings.Replace(s, "SourceArn: !GetAtt SendNotificationsRule.Arn", "SourceArn: !GetAtt FinalizeRevocationsRule.Arn", 1) + }, + "permission ungated": func(s string) string { + return strings.Replace(s, "Type: AWS::Lambda::Permission\n Condition: ScheduleAnalyticsCollect\n", "Type: AWS::Lambda::Permission\n", 1) + }, + "rule ungated": func(s string) string { + return strings.Replace(s, " Condition: ScheduleReapStuckPurchases\n Properties:\n Name", " Properties:\n Name", 1) + }, + "name too long": func(s string) string { return strings.Replace(s, `-ReapStuck"`, `-ReapStuckPurchasesNow"`, 1) }, + "name reused": func(s string) string { return strings.Replace(s, `-ReapStuck"`, `-Finalize"`, 1) }, + "literal schedule": func(s string) string { + return strings.Replace(s, "!Ref FinalizeRevocationsSchedule", `"rate(1 hour)"`, 1) + }, + "schedule drift": func(s string) string { + return strings.Replace(s, `Default: "rate(15 minutes)"`, `Default: "rate(1 minute)"`, 1) + }, + "wrong target": func(s string) string { + return strings.Replace(s, "Arn: !GetAtt LambdaFunction.Arn\n Input: '{\"action\": \"send_", "Arn: !Ref NotificationTopic\n Input: '{\"action\": \"send_", 1) + }, + "extra input key": func(s string) string { + return strings.Replace(s, `{"action": "analytics_collect"}`, `{"action": "analytics_collect", "x": 1}`, 1) + }, + "rule disabled": func(s string) string { + return strings.Replace(s, "ScheduleExpression: !Ref SendNotificationsSchedule\n State: ENABLED", "ScheduleExpression: !Ref SendNotificationsSchedule\n State: DISABLED", 1) + }, + "unknown task": func(s string) string { + return strings.Replace(s, `{"action": "fire_scheduled_purchases"}`, `{"action": "fire_purchases"}`, 1) + }, + "loose enable param": func(s string) string { + return strings.Replace(s, " - Ref: EnableSendNotificationsSchedule\n - \"true\"", " - Ref: EnableSendNotificationsSchedule\n - \"yes\"", 1) + }, + "legacy rule switched": func(s string) string { + return strings.Replace(s, " PurchaseCheckRule:\n Type: AWS::Events::Rule\n", " PurchaseCheckRule:\n Type: AWS::Events::Rule\n Condition: ScheduleSendNotifications\n", 1) + }, + } { + mutated := edit(tpl) + if mutated == tpl { + t.Errorf("%s: edit did not apply; update the fixture", name) + continue + } + s, err := parseCFN([]byte(mutated)) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + if len(cfnProblems(s, lambdaVars)) == 0 { + t.Errorf("%s: no problem reported", name) + } + } + long, err := parseCFN([]byte("Resources:\n P:\n Type: AWS::Lambda::Permission\n Properties:\n SourceArn:\n Fn::GetAtt: [R, Arn]\n")) + if err != nil || len(cfnPermissionProblems(long, "R")) == 0 || getAtt(props(long.Resources["P"])["SourceArn"]) != "R.Arn" { + t.Errorf("long-form Fn::GetAtt must resolve to R.Arn and the bare permission must be flagged: err = %v", err) + } +} diff --git a/terraform/modules/compute/scheduled_task_coverage_test.go b/terraform/modules/compute/scheduled_task_coverage_test.go index 722602c5f..fb36ea46c 100644 --- a/terraform/modules/compute/scheduled_task_coverage_test.go +++ b/terraform/modules/compute/scheduled_task_coverage_test.go @@ -54,10 +54,9 @@ var ( var knownGaps = map[string]map[string][]string{ "aws-fargate": {dimTask: {"analytics_collect", "cleanup", "finalize_revocations", "process_scheduled_purchases", "reap_stuck_purchases", "send_notifications"}}, - "cloudformation": {dimTask: {"analytics_collect", "cleanup", "finalize_revocations", - "fire_scheduled_purchases", "reap_stuck_purchases", "send_notifications"}}, - "gcp-cloud-run": {dimTask: sevenGCP}, - "gcp-gke": {dimTask: sevenGCP}, + "cloudformation": {dimTask: {"cleanup"}}, + "gcp-cloud-run": {dimTask: sevenGCP}, + "gcp-gke": {dimTask: sevenGCP}, "azure-container-apps": {dimTask: {"analytics_collect", "finalize_revocations", "fire_scheduled_purchases", "process_scheduled_purchases", "reap_stuck_purchases", "send_notifications"}}, "azure-aks": {dimTask: allEight}, // no scheduler resources at all: #893 From e5bcda0aa8728f3150251ff78113c968c1089d80 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 08:22:08 +0000 Subject: [PATCH 2/2] fix(cfn): add an opt-in cleanup schedule Add EnableCleanupSchedule (default "false") and CleanupSchedule (default rate(1 day)) to the CUDly CloudFormation stack, with a -Cleanup rule and invoke permission that run the server cleanup task. It stays opt-in, as on the Lambda module: the first run deletes every execution past the 30-day retention. Drop the last CloudFormation entry from knownGaps and document the parameter in DEPLOYMENT.md and known_issues/33. Refs #712 Co-Authored-By: Claude Opus 5.5 --- cloudformation/stacks/CUDly/template.yaml | 48 +++++++++++++++++++ docs/DEPLOYMENT.md | 5 +- .../33_executions_retention_cleanup.md | 7 ++- .../compute/scheduled_task_coverage_test.go | 5 +- 4 files changed, 58 insertions(+), 7 deletions(-) diff --git a/cloudformation/stacks/CUDly/template.yaml b/cloudformation/stacks/CUDly/template.yaml index 0a7471902..b74cf9913 100644 --- a/cloudformation/stacks/CUDly/template.yaml +++ b/cloudformation/stacks/CUDly/template.yaml @@ -50,6 +50,8 @@ Metadata: - FireScheduledPurchasesSchedule - EnableSendNotificationsSchedule - SendNotificationsSchedule + - EnableCleanupSchedule + - CleanupSchedule ParameterLabels: EmailAddress: @@ -232,6 +234,25 @@ Parameters: Default: "rate(1 day)" Description: EventBridge schedule expression for send_notifications + EnableCleanupSchedule: + Type: String + Default: "false" + AllowedValues: + - "true" + - "false" + Description: >- + Run the server cleanup task on a schedule. Opt-in. It deletes expired + sessions and, via CleanupOldExecutions(30), purchase executions that are + completed with scheduled_date older than 30 days, canceled with + updated_at older than 30 days, or in a status outside failed/canceled + with expires_at older than 30 days. The first run deletes the whole + backlog, so export any history you want to keep first. + + CleanupSchedule: + Type: String + Default: "rate(1 day)" + Description: EventBridge schedule expression for cleanup + CrossAccountTargetAccountIds: Type: CommaDelimitedList Default: "" @@ -307,6 +328,11 @@ Conditions: - Ref: EnableSendNotificationsSchedule - "true" + ScheduleCleanup: + Fn::Equals: + - Ref: EnableCleanupSchedule + - "true" + Resources: # ============================================================================= # DynamoDB Tables @@ -966,6 +992,28 @@ Resources: Principal: events.amazonaws.com SourceArn: !GetAtt SendNotificationsRule.Arn + CleanupRule: + Type: AWS::Events::Rule + Condition: ScheduleCleanup + Properties: + Name: !Sub "${AWS::StackName}-Cleanup" + Description: Scheduled cleanup task + ScheduleExpression: !Ref CleanupSchedule + State: ENABLED + Targets: + - Id: LambdaTarget + Arn: !GetAtt LambdaFunction.Arn + Input: '{"action": "cleanup"}' + + CleanupPermission: + Type: AWS::Lambda::Permission + Condition: ScheduleCleanup + Properties: + Action: lambda:InvokeFunction + FunctionName: !Ref LambdaFunction + Principal: events.amazonaws.com + SourceArn: !GetAtt CleanupRule.Arn + # ============================================================================= # SNS Topic for Notifications # ============================================================================= diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 446733524..34f917480 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -592,11 +592,12 @@ Each cloud provider has lifecycle policies configured via Terraform (`terraform/ ### Database Cleanup Jobs -The server's `cleanup` task deletes expired sessions and, via `CleanupOldExecutions(30)`, purchase executions that are completed with `scheduled_date` older than 30 days, canceled with `updated_at` older than 30 days, or in any status other than failed or canceled with `expires_at` older than 30 days. Failed executions are kept. Azure Container Apps schedules it by default; AWS Lambda can schedule it on request: +The server's `cleanup` task deletes expired sessions and, via `CleanupOldExecutions(30)`, purchase executions that are completed with `scheduled_date` older than 30 days, canceled with `updated_at` older than 30 days, or in any status other than failed or canceled with `expires_at` older than 30 days. Failed executions are kept. Azure Container Apps schedules it by default; AWS Lambda and CloudFormation can schedule it on request: - **Azure Container Apps (when `enable_scheduled_tasks` is true):** a Logic App calls `/api/scheduled/cleanup` daily (`terraform/modules/compute/azure/container-apps/scheduled-tasks.tf`) - **AWS Lambda (opt-in):** set `enable_cleanup_schedule = true` (default false; `cleanup_schedule` defaults to `rate(1 day)`) in `terraform/environments/aws`. The first run deletes the whole backlog older than 30 days, so export any history you want to keep first. -- **AWS Fargate and CloudFormation:** nothing schedules it yet. The `terraform/modules/compute/aws/cleanup-lambda/` module is not consumed by any environment. +- **CloudFormation (opt-in):** set the `EnableCleanupSchedule` stack parameter to `true` (default `false`; `CleanupSchedule` defaults to `rate(1 day)`) on `cloudformation/stacks/CUDly`. The same backlog warning applies. +- **AWS Fargate:** nothing schedules it yet. The `terraform/modules/compute/aws/cleanup-lambda/` module is not consumed by any environment. - **GCP:** nothing schedules it. The `terraform/modules/compute/gcp/cleanup-function/` module is not consumed by any environment. AKS has no scheduler (#893). Scheduling for AWS Fargate, CloudFormation and GCP is tracked by #712. diff --git a/known_issues/33_executions_retention_cleanup.md b/known_issues/33_executions_retention_cleanup.md index e5a0cfdac..eedaec38d 100644 --- a/known_issues/33_executions_retention_cleanup.md +++ b/known_issues/33_executions_retention_cleanup.md @@ -9,7 +9,7 @@ > schedules cleanup on AWS. The same holds for the Terraform behind the > "matches what the cleanup-lambda Terraform actually configures" sentence > under "Original problem". Only Azure Container Apps schedules cleanup today. Retention -> scheduling is tracked by #712; retention is scheduled via the server `cleanup` task on AWS Lambda when opted in. +> scheduling is tracked by #712; retention is scheduled via the server `cleanup` task on AWS Lambda and CloudFormation when opted in. ## Resolution @@ -21,6 +21,9 @@ `terraform/modules/compute/aws/cleanup-lambda/main.tf:98`~~ (unconsumed module, see amendment) - **AWS Lambda (opt-in):** the `enable_cleanup_schedule` / `cleanup_schedule` sweep `-sweep-cleanup` runs the server cleanup task. +- **CloudFormation (opt-in):** the `EnableCleanupSchedule` / + `CleanupSchedule` stack parameters add a `-Cleanup` rule that runs + the server cleanup task. - **Azure Container Apps:** a Logic App calls `/api/scheduled/cleanup` daily when `enable_scheduled_tasks` is true. (The `azure/cleanup-function` module is not consumed by any environment, and AKS has no scheduler, #893.) @@ -28,7 +31,7 @@ The `CleanupOldExecutions` Postgres implementation in `internal/config/store_postgres.go` runs the expected `DELETE FROM purchase_executions WHERE …`, so the table is bounded -only where that task is scheduled (Azure Container Apps by default, AWS Lambda when opted in, see amendment). +only where that task is scheduled (Azure Container Apps by default, AWS Lambda and CloudFormation when opted in, see amendment). ## Original problem (historical context) diff --git a/terraform/modules/compute/scheduled_task_coverage_test.go b/terraform/modules/compute/scheduled_task_coverage_test.go index fb36ea46c..189dfd1b3 100644 --- a/terraform/modules/compute/scheduled_task_coverage_test.go +++ b/terraform/modules/compute/scheduled_task_coverage_test.go @@ -54,9 +54,8 @@ var ( var knownGaps = map[string]map[string][]string{ "aws-fargate": {dimTask: {"analytics_collect", "cleanup", "finalize_revocations", "process_scheduled_purchases", "reap_stuck_purchases", "send_notifications"}}, - "cloudformation": {dimTask: {"cleanup"}}, - "gcp-cloud-run": {dimTask: sevenGCP}, - "gcp-gke": {dimTask: sevenGCP}, + "gcp-cloud-run": {dimTask: sevenGCP}, + "gcp-gke": {dimTask: sevenGCP}, "azure-container-apps": {dimTask: {"analytics_collect", "finalize_revocations", "fire_scheduled_purchases", "process_scheduled_purchases", "reap_stuck_purchases", "send_notifications"}}, "azure-aks": {dimTask: allEight}, // no scheduler resources at all: #893