diff --git a/cloudformation/stacks/CUDly/template.yaml b/cloudformation/stacks/CUDly/template.yaml index b801ef6aa..b74cf9913 100644 --- a/cloudformation/stacks/CUDly/template.yaml +++ b/cloudformation/stacks/CUDly/template.yaml @@ -37,6 +37,22 @@ Metadata: - LogRetentionPeriod - ExecutionFrequency + - Label: + default: Scheduled Tasks + Parameters: + - EnableFinalizeRevocationsSchedule + - FinalizeRevocationsSchedule + - EnableAnalyticsCollectSchedule + - AnalyticsCollectSchedule + - EnableReapStuckPurchasesSchedule + - ReapStuckPurchasesSchedule + - EnableFireScheduledPurchasesSchedule + - FireScheduledPurchasesSchedule + - EnableSendNotificationsSchedule + - SendNotificationsSchedule + - EnableCleanupSchedule + - CleanupSchedule + ParameterLabels: EmailAddress: default: Notification email address @@ -140,6 +156,103 @@ 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 + + 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: "" @@ -190,6 +303,36 @@ 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" + + ScheduleCleanup: + Fn::Equals: + - Ref: EnableCleanupSchedule + - "true" + Resources: # ============================================================================= # DynamoDB Tables @@ -733,6 +876,144 @@ 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 + + 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 7b4e23976..f150e9be3 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_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..189dfd1b3 100644 --- a/terraform/modules/compute/scheduled_task_coverage_test.go +++ b/terraform/modules/compute/scheduled_task_coverage_test.go @@ -54,8 +54,6 @@ 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}, "azure-container-apps": {dimTask: {"analytics_collect", "finalize_revocations", "fire_scheduled_purchases",