diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 592d7cc16..176ddf44b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1003,6 +1003,17 @@ jobs: terraform -chdir="$d" test done + - name: Test Azure compute modules + if: matrix.cloud == 'azure' + run: | + for t in terraform/modules/compute/azure/*/tests; do + [ -d "$t" ] || continue + d=$(dirname "$t") + terraform -chdir="$d" init -backend=false -lockfile=readonly -input=false + terraform -chdir="$d" fmt -check -recursive + terraform -chdir="$d" test + done + # With CloudFront in front of a Fargate ALB, the ALB must admit only # CloudFront's origin-facing prefix list (#412). - name: Test AWS ALB ingress guard diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index a009f9319..a331a1d04 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -601,8 +601,9 @@ The server's `cleanup` task deletes expired sessions and, via `CleanupOldExecuti - **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 (opt-in):** set `enable_cleanup_schedule = true` (default false; `cleanup_schedule` defaults to `0 3 * * *`) on the AKS module. Like the module's other scheduled tasks, it runs as a Kubernetes CronJob (`terraform/modules/compute/azure/aks/scheduled-tasks.tf`) only when `deploy_kubernetes_resources` is true. -AKS has no scheduler (#893). Scheduling for AWS Fargate, CloudFormation and GCP is tracked by #712. +Scheduling for AWS Fargate, CloudFormation and GCP is tracked by #712. Alternatively, use pg_cron for database-native scheduling. diff --git a/known_issues/33_executions_retention_cleanup.md b/known_issues/33_executions_retention_cleanup.md index eedaec38d..caa2d4f02 100644 --- a/known_issues/33_executions_retention_cleanup.md +++ b/known_issues/33_executions_retention_cleanup.md @@ -26,7 +26,10 @@ 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.) + is not consumed by any environment.) +- **AKS (opt-in):** `enable_cleanup_schedule` / `cleanup_schedule` on the AKS + module add a `-task-cleanup` CronJob that runs the server cleanup + task (#893), when `deploy_kubernetes_resources` is true. The `CleanupOldExecutions` Postgres implementation in `internal/config/store_postgres.go` runs the expected diff --git a/terraform/environments/azure/compute.tf b/terraform/environments/azure/compute.tf index 28d6c6b74..92ea06b8a 100644 --- a/terraform/environments/azure/compute.tf +++ b/terraform/environments/azure/compute.tf @@ -200,6 +200,12 @@ module "compute_aks" { local_account_disabled = var.aks_local_account_disabled admin_group_object_ids = var.aks_admin_group_object_ids + # Scheduled tasks (CronJobs, #893). The other tasks use the module defaults. + enable_scheduled_tasks = var.enable_scheduled_tasks + recommendation_schedule = var.recommendation_schedule + enable_ri_exchange_schedule = var.enable_ri_exchange_schedule + ri_exchange_schedule = var.ri_exchange_schedule + tags = local.common_tags depends_on = [module.networking, module.database, module.secrets] diff --git a/terraform/environments/azure/variables.tf b/terraform/environments/azure/variables.tf index 1f199cd6d..90772e1a7 100644 --- a/terraform/environments/azure/variables.tf +++ b/terraform/environments/azure/variables.tf @@ -397,7 +397,7 @@ variable "additional_env_vars" { } variable "enable_scheduled_tasks" { - description = "Enable scheduled tasks via Logic Apps" + description = "Enable scheduled tasks (Logic Apps on Container Apps; on AKS, the collect_recommendations CronJob)" type = bool default = true } @@ -408,14 +408,15 @@ variable "recommendation_schedule" { hour field is parsed (the Azure module converts it to Logic Apps frequency=Day + interval=1 + startTime=:00). Sub-daily, day-of-week, and day-of-month patterns are silently ignored; - use daily-at-hour-N values only. + use daily-at-hour-N values only. On AKS the full expression is + the collect_recommendations CronJob schedule. EOT type = string default = "0 2 * * *" # Daily at 02:00 UTC } variable "enable_ri_exchange_schedule" { - description = "Enable scheduled RI exchange automation via Logic Apps" + description = "Enable scheduled RI exchange automation (a Logic App on Container Apps, a CronJob on AKS)" type = bool default = false } diff --git a/terraform/modules/compute/azure/aks/.terraform.lock.hcl b/terraform/modules/compute/azure/aks/.terraform.lock.hcl index 2ea85c8d1..4c43bed82 100644 --- a/terraform/modules/compute/azure/aks/.terraform.lock.hcl +++ b/terraform/modules/compute/azure/aks/.terraform.lock.hcl @@ -5,6 +5,8 @@ provider "registry.terraform.io/hashicorp/azurerm" { version = "3.117.1" constraints = "~> 3.0" hashes = [ + "h1:3c9iOEtBMnHrpJLlhbQ0sCZPWhE/2dvEPcL8KkXAh7w=", + "h1:MufLWHQWuFt+iX9bthLooPP3dDZloujkzaIZPW5A/p4=", "h1:j6wnjpHfBcQC4xd3ZYquaIPIIR46xJQs7rxwPdSOZos=", "zh:0c513676836e3c50d004ece7d2624a8aff6faac14b833b96feeac2e4bc2c1c12", "zh:50ea01ada95bae2f187db9e926e463f45d860767a85ebc59160414e00e76c35d", @@ -25,6 +27,8 @@ provider "registry.terraform.io/hashicorp/helm" { version = "2.17.0" constraints = "~> 2.11" hashes = [ + "h1:0LSHBFqJvHTzQesUwagpDLsrzVliY+t2c26nDJizHFM=", + "h1:K5FEjxvDnxb1JF1kG1xr8J3pNGxoaR3Z0IBG9Csm/Is=", "h1:kQMkcPVvHOguOqnxoEU2sm1ND9vCHiT8TvZ2x6v/Rsw=", "zh:06fb4e9932f0afc1904d2279e6e99353c2ddac0d765305ce90519af410706bd4", "zh:104eccfc781fc868da3c7fec4385ad14ed183eb985c96331a1a937ac79c2d1a7", @@ -45,6 +49,8 @@ provider "registry.terraform.io/hashicorp/kubernetes" { version = "2.38.0" constraints = "~> 2.23" hashes = [ + "h1:5CkveFo5ynsLdzKk+Kv+r7+U9rMrNjfZPT3a0N/fhgE=", + "h1:7nJdsd1RMPBtOjDXidB37+KSDN5VcOWkbkow69qJVGc=", "h1:soK8Lt0SZ6dB+HsypFRDzuX/npqlMU6M0fvyaR1yW0k=", "zh:0af928d776eb269b192dc0ea0f8a3f0f5ec117224cd644bdacdc682300f84ba0", "zh:1be998e67206f7cfc4ffe77c01a09ac91ce725de0abaec9030b22c0a832af44f", @@ -65,7 +71,9 @@ provider "registry.terraform.io/hashicorp/time" { version = "0.13.1" constraints = "~> 0.11" hashes = [ + "h1:+W+DMrVoVnoXo3f3M4W+OpZbkCrUn6PnqDF33D2Cuf0=", "h1:ZT5ppCNIModqk3iOkVt5my8b8yBHmDpl663JtXAIRqM=", + "h1:eSX+RgYfeumojmeJ9Y5CbnkH2NkBC+9vUEolqdQVtGw=", "zh:02cb9aab1002f0f2a94a4f85acec8893297dc75915f7404c165983f720a54b74", "zh:04429b2b31a492d19e5ecf999b116d396dac0b24bba0d0fb19ecaefe193fdb8f", "zh:26f8e51bb7c275c404ba6028c1b530312066009194db721a8427a7bc5cdbc83a", diff --git a/terraform/modules/compute/azure/aks/main.tf b/terraform/modules/compute/azure/aks/main.tf index 07b051e67..fddb66986 100644 --- a/terraform/modules/compute/azure/aks/main.tf +++ b/terraform/modules/compute/azure/aks/main.tf @@ -343,138 +343,23 @@ resource "kubernetes_deployment" "app" { protocol = "TCP" } - env { - name = "PORT" - value = "8080" - } - - env { - name = "ENVIRONMENT" - value = var.environment - } - - env { - name = "RUNTIME_MODE" - value = "http" - } - - env { - name = "DB_HOST" - value_from { - secret_key_ref { - name = kubernetes_secret.database[0].metadata[0].name - key = "host" - } - } - } - - env { - name = "DB_PORT" - value = "5432" - } - - env { - name = "DB_NAME" - value_from { - secret_key_ref { - name = kubernetes_secret.database[0].metadata[0].name - key = "name" - } - } - } - - env { - name = "DB_USER" - value_from { - secret_key_ref { - name = kubernetes_secret.database[0].metadata[0].name - key = "username" - } - } - } - - env { - name = "DB_PASSWORD_SECRET" - value_from { - secret_key_ref { - name = kubernetes_secret.database[0].metadata[0].name - key = "password_secret_name" - } - } - } - - env { - name = "DB_SSL_MODE" - value = "require" - } - - env { - name = "DB_CONNECT_TIMEOUT" - value = "8s" - } - - env { - name = "DB_AUTO_MIGRATE" - value = tostring(var.auto_migrate) - } - - env { - name = "DB_MIGRATIONS_PATH" - value = "/app/migrations" - } - - env { - name = "ADMIN_EMAIL" - value = var.admin_email - } - - env { - name = "ADMIN_PASSWORD_SECRET" - value = var.admin_password_secret_name - } - - env { - name = "SECRET_PROVIDER" - value = "azure" - } - - env { - name = "AZURE_KEY_VAULT_URL" - value = var.key_vault_uri - } - - env { - name = "AZURE_REGION" - value = var.location - } - - env { - name = "ALLOWED_ORIGINS" - value = join(",", var.allowed_origins) - } - - # Scheduled-task auth — Azure stays on bearer mode (Logic Apps - # fetch the shared secret from Key Vault at invocation time; - # Logic Apps' HTTP Connector does not emit Entra OIDC tokens). - # Validated app-side via internal/server/scheduledauth. - env { - name = "SCHEDULED_TASK_AUTH_MODE" - value = "bearer" - } - + # The same list feeds the scheduled-task CronJobs in + # scheduled-tasks.tf, so a task pod sees exactly this environment. dynamic "env" { - for_each = local.archera_env + for_each = local.app_env content { - name = env.key - value = env.value - } - } - - dynamic "env" { - for_each = var.additional_env_vars - content { - name = env.key - value = env.value + name = env.value.name + value = env.value.secret_ref == null ? env.value.value : null + + dynamic "value_from" { + for_each = env.value.secret_ref == null ? [] : [env.value.secret_ref] + content { + secret_key_ref { + name = kubernetes_secret.database[0].metadata[0].name + key = value_from.value + } + } + } } } @@ -843,4 +728,38 @@ locals { var.archera_plan_id != "" ? { ARCHERA_PLAN_ID = var.archera_plan_id } : {}, var.archera_api_key_secret_name != "" ? { ARCHERA_API_KEY_SECRET = var.archera_api_key_secret_name } : {}, ) + + # Container environment of the API Deployment, shared with the + # scheduled-task CronJobs (#893). The order is the one the Deployment has + # always used, so moving it here changes nothing in the Deployment's spec. + # An entry with secret_ref reads that key of the database-credentials Secret. + app_env = concat( + [ + { name = "PORT", value = "8080", secret_ref = null }, + { name = "ENVIRONMENT", value = var.environment, secret_ref = null }, + { name = "RUNTIME_MODE", value = "http", secret_ref = null }, + { name = "DB_HOST", value = null, secret_ref = "host" }, + { name = "DB_PORT", value = "5432", secret_ref = null }, + { name = "DB_NAME", value = null, secret_ref = "name" }, + { name = "DB_USER", value = null, secret_ref = "username" }, + { name = "DB_PASSWORD_SECRET", value = null, secret_ref = "password_secret_name" }, + { name = "DB_SSL_MODE", value = "require", secret_ref = null }, + { name = "DB_CONNECT_TIMEOUT", value = "8s", secret_ref = null }, + { name = "DB_AUTO_MIGRATE", value = tostring(var.auto_migrate), secret_ref = null }, + { name = "DB_MIGRATIONS_PATH", value = "/app/migrations", secret_ref = null }, + { name = "ADMIN_EMAIL", value = var.admin_email, secret_ref = null }, + { name = "ADMIN_PASSWORD_SECRET", value = var.admin_password_secret_name, secret_ref = null }, + { name = "SECRET_PROVIDER", value = "azure", secret_ref = null }, + { name = "AZURE_KEY_VAULT_URL", value = var.key_vault_uri, secret_ref = null }, + { name = "AZURE_REGION", value = var.location, secret_ref = null }, + { name = "ALLOWED_ORIGINS", value = join(",", var.allowed_origins), secret_ref = null }, + # Scheduled-task auth: Azure stays on bearer mode (Logic Apps fetch the + # shared secret from Key Vault at invocation time; Logic Apps' HTTP + # Connector does not emit Entra OIDC tokens). Validated app-side via + # internal/server/scheduledauth. + { name = "SCHEDULED_TASK_AUTH_MODE", value = "bearer", secret_ref = null }, + ], + [for k, v in local.archera_env : { name = k, value = v, secret_ref = null }], + [for k, v in var.additional_env_vars : { name = k, value = v, secret_ref = null }], + ) } diff --git a/terraform/modules/compute/azure/aks/scheduled-tasks.tf b/terraform/modules/compute/azure/aks/scheduled-tasks.tf new file mode 100644 index 000000000..01cb54803 --- /dev/null +++ b/terraform/modules/compute/azure/aks/scheduled-tasks.tf @@ -0,0 +1,171 @@ +# Scheduled server tasks on AKS (#893). +# +# Each enabled task is a Kubernetes CronJob that starts a one-off pod from the +# API image and runs `/app/cudly --task `, the same entry point the +# Fargate EventBridge targets use. The task runs in-process against the +# database, so no HTTP call, ingress route or scheduled-task secret is +# involved. The pod uses the API's service account (the only one the workload +# identity's federated credential accepts) and the API's environment +# (local.app_env in main.tf), so it reaches the database and Key Vault the +# same way the API does. +# +# Like the Deployment, the CronJobs exist only when +# var.deploy_kubernetes_resources is true. Schedules are standard 5-field cron +# expressions, evaluated by the cluster in UTC. + +locals { + # Keyed by server task name. `job` is the CronJob name suffix. Default-on + # tasks match the other platforms; fire_scheduled_purchases, + # process_scheduled_purchases, send_notifications and cleanup are opt-in + # (default off, as decided for #712). ri_exchange_reshape (as on Container + # Apps, Cloud Run and Fargate) and ladder_run (as on Fargate and Lambda) are + # opt-in features, off by default. + sweep_tasks = { + collect_recommendations = { job = "collect", enabled = var.enable_scheduled_tasks, schedule = var.recommendation_schedule } + reap_stuck_purchases = { job = "reap", enabled = var.enable_reap_stuck_purchases_schedule, schedule = var.reap_stuck_purchases_schedule } + analytics_collect = { job = "analytics", enabled = var.enable_analytics_collect_schedule, schedule = var.analytics_collect_schedule } + fire_scheduled_purchases = { job = "fire", enabled = var.enable_fire_scheduled_purchases_schedule, schedule = var.fire_scheduled_purchases_schedule } + process_scheduled_purchases = { job = "process", enabled = var.enable_process_scheduled_purchases_schedule, schedule = var.process_scheduled_purchases_schedule } + send_notifications = { job = "notify", enabled = var.enable_send_notifications_schedule, schedule = var.send_notifications_schedule } + finalize_revocations = { job = "finalize", enabled = var.enable_finalize_revocations_schedule, schedule = var.finalize_revocations_schedule } + cleanup = { job = "cleanup", enabled = var.enable_cleanup_schedule, schedule = var.cleanup_schedule } + ri_exchange_reshape = { job = "ri-exchange", enabled = var.enable_ri_exchange_schedule, schedule = var.ri_exchange_schedule } + ladder_run = { job = "ladder", enabled = var.enable_ladder_run_schedule, schedule = var.ladder_run_schedule } + } + active_sweeps = var.deploy_kubernetes_resources ? { for k, v in local.sweep_tasks : k => v if v.enabled } : {} + + # The server stops a task at TASK_TIMEOUT; the Job deadline adds headroom + # for startup (database connection, migrations, secret resolution) so a pod + # that hangs before or after the task is still killed. + task_job_deadline_seconds = var.task_timeout + 300 + + # TASK_TIMEOUT is owned by var.task_timeout, so an additional_env_vars entry + # of that name is dropped rather than duplicated. + task_env = concat( + [for e in local.app_env : e if e.name != "TASK_TIMEOUT"], + [{ name = "TASK_TIMEOUT", value = tostring(var.task_timeout), secret_ref = null }], + ) +} + +resource "kubernetes_cron_job_v1" "sweep" { + for_each = local.active_sweeps + + metadata { + name = "${var.project_name}-task-${each.value.job}" + namespace = kubernetes_namespace.app[0].metadata[0].name + + labels = { + app = "${var.project_name}-task" + task = each.value.job + environment = var.environment + } + } + + spec { + schedule = each.value.schedule + # A run still going when the next one is due makes the next one skip, + # rather than stacking pods. The server's per-task advisory lock already + # makes an overlapping run a no-op; this keeps the pods from piling up. + concurrency_policy = "Forbid" + starting_deadline_seconds = 300 + successful_jobs_history_limit = 1 + failed_jobs_history_limit = 3 + + job_template { + metadata { + labels = { + app = "${var.project_name}-task" + task = each.value.job + } + } + + spec { + # No in-Job retry: a failed run is retried by the next schedule, the + # same as the EventBridge-triggered Fargate tasks. + backoff_limit = 0 + active_deadline_seconds = local.task_job_deadline_seconds + + template { + metadata { + # Not the API's `app` label: the Service and the NGINX network + # policy select on that, and must never pick up a task pod. + labels = { + app = "${var.project_name}-task" + task = each.value.job + environment = var.environment + # Opts the pod into the workload identity webhook (#198), as on + # the Deployment. + "azure.workload.identity/use" = "true" + } + } + + spec { + service_account_name = kubernetes_service_account.app[0].metadata[0].name + restart_policy = "Never" + + container { + name = "task" + image = "${var.image_name}:${var.image_tag}" + # Bypasses /entrypoint.sh, which would append --mode/--port and + # start the HTTP server path; --task runs one task and exits. + command = ["/app/cudly", "--task", each.key] + + dynamic "env" { + for_each = local.task_env + content { + name = env.value.name + value = env.value.secret_ref == null ? env.value.value : null + + dynamic "value_from" { + for_each = env.value.secret_ref == null ? [] : [env.value.secret_ref] + content { + secret_key_ref { + name = kubernetes_secret.database[0].metadata[0].name + key = value_from.value + } + } + } + } + } + + # Requests and limits are required: the namespace ResourceQuota + # rejects pods without them. Same sizing as an API replica. + resources { + requests = { + cpu = "250m" + memory = "512Mi" + } + limits = { + cpu = "1000m" + memory = "1Gi" + } + } + } + } + } + } + } + } + + lifecycle { + precondition { + # Kubernetes appends an 11-character suffix to a CronJob's name for each + # Job, so the CronJob name itself is limited to 52 characters. + condition = length("${var.project_name}-task-${each.value.job}") <= 52 + error_message = "project_name is too long for the scheduled-task CronJob names (at most 52 characters including \"-task-\")." + } + precondition { + condition = can(regex("^(@(yearly|annually|monthly|weekly|daily|midnight|hourly)|\\S+(\\s+\\S+){4})$", trimspace(each.value.schedule))) + error_message = "Scheduled-task schedules on AKS must be 5-field cron expressions (for example \"*/5 * * * *\"), not EventBridge rate() or 6-field cron()." + } + } + + # Same ordering reasons as the Deployment: the pod reads Key Vault through + # the workload identity, so the grant and the federated credential should + # exist before the first run. + depends_on = [ + kubernetes_secret.database, + azurerm_role_assignment.workload_kv_secrets_user, + azurerm_federated_identity_credential.workload, + ] +} diff --git a/terraform/modules/compute/azure/aks/tests/scheduled_tasks.tftest.hcl b/terraform/modules/compute/azure/aks/tests/scheduled_tasks.tftest.hcl new file mode 100644 index 000000000..0aaf890ed --- /dev/null +++ b/terraform/modules/compute/azure/aks/tests/scheduled_tasks.tftest.hcl @@ -0,0 +1,287 @@ +# Module wiring for the scheduled-task CronJobs (#893). Plans are targeted: +# the mock azurerm provider plans kubelet_identity as an empty list, which +# azurerm_role_assignment.aks_acr indexes, and that resource is not under test. +# The Go test in terraform/modules/compute checks the static wiring. +mock_provider "azurerm" {} +mock_provider "kubernetes" {} +mock_provider "helm" {} +mock_provider "time" {} + +variables { + project_name = "cudly" + environment = "dev" + resource_group_name = "rg" + location = "eastus" + vnet_subnet_id = "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/vnet/subnets/aks" + image_name = "cudlyacr.azurecr.io/cudly" + image_tag = "abc123" + database_host = "db.example.com" + database_name = "cudly" + database_username = "cudly" + database_password_secret_name = "db-password" + key_vault_id = "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.KeyVault/vaults/kv" + key_vault_uri = "https://kv.vault.azure.net/" + admin_email = "admin@example.com" + admin_password_secret_name = "admin-password" + allowed_origins = ["https://app.example.com"] + archera_org_id = "org" + additional_env_vars = { SCHEDULED_TASK_SECRET_NAME = "scheduled-task-secret", CUDLY_SOURCE_CLOUD = "azure" } + deploy_kubernetes_resources = true +} + +run "cluster_only" { + command = plan + + variables { + deploy_kubernetes_resources = false + } + + plan_options { + target = [kubernetes_cron_job_v1.sweep] + } + + assert { + condition = length(kubernetes_cron_job_v1.sweep) == 0 + error_message = "No CronJob without deploy_kubernetes_resources (there is no namespace or service account to run in)." + } +} + +run "defaults" { + command = plan + + plan_options { + target = [kubernetes_deployment.app, kubernetes_service.app, kubernetes_secret.database, kubernetes_cron_job_v1.sweep] + } + + assert { + condition = toset(keys(kubernetes_cron_job_v1.sweep)) == toset(["analytics_collect", "collect_recommendations", "finalize_revocations", "reap_stuck_purchases"]) + error_message = "Default-on tasks are collect_recommendations, reap_stuck_purchases, analytics_collect and finalize_revocations; fire, process, send_notifications and cleanup are opt-in." + } + assert { + condition = alltrue([ + for k, j in kubernetes_cron_job_v1.sweep : + j.spec[0].job_template[0].spec[0].template[0].spec[0].container[0].command == tolist(["/app/cudly", "--task", k]) + ]) + error_message = "Each CronJob must run /app/cudly --task ." + } + assert { + condition = alltrue([ + for k, j in kubernetes_cron_job_v1.sweep : + j.spec[0].schedule == { collect_recommendations = var.recommendation_schedule, reap_stuck_purchases = var.reap_stuck_purchases_schedule, analytics_collect = var.analytics_collect_schedule, finalize_revocations = var.finalize_revocations_schedule }[k] + ]) + error_message = "Each schedule must come from its variable." + } + assert { + condition = toset([for j in kubernetes_cron_job_v1.sweep : j.metadata[0].name]) == toset(["cudly-task-collect", "cudly-task-reap", "cudly-task-analytics", "cudly-task-finalize"]) + error_message = "CronJob names must be distinct -task- names." + } + assert { + condition = alltrue([ + for j in kubernetes_cron_job_v1.sweep : + j.metadata[0].namespace == "dev" && j.spec[0].concurrency_policy == "Forbid" && j.spec[0].job_template[0].spec[0].backoff_limit == 0 && j.spec[0].job_template[0].spec[0].active_deadline_seconds == 1200 && j.spec[0].job_template[0].spec[0].template[0].spec[0].restart_policy == "Never" + ]) + error_message = "CronJobs run in the app namespace, never overlap, do not retry in-Job, are killed after task_timeout + 300s and do not restart." + } + assert { + condition = alltrue([ + for j in kubernetes_cron_job_v1.sweep : + j.spec[0].job_template[0].spec[0].template[0].spec[0].service_account_name == "cudly-api" && j.spec[0].job_template[0].spec[0].template[0].metadata[0].labels["azure.workload.identity/use"] == "true" + ]) + error_message = "Task pods must use the API service account (the federated credential's subject) and opt into workload identity." + } + assert { + condition = alltrue([ + for j in kubernetes_cron_job_v1.sweep : + j.spec[0].job_template[0].spec[0].template[0].metadata[0].labels["app"] != kubernetes_service.app[0].spec[0].selector["app"] + ]) + error_message = "Task pods must not match the API Service selector, or the Service would route traffic to them." + } + assert { + condition = alltrue([ + for j in kubernetes_cron_job_v1.sweep : + j.spec[0].job_template[0].spec[0].template[0].spec[0].container[0].image == "cudlyacr.azurecr.io/cudly:abc123" + ]) + error_message = "Task pods must run the API image." + } + assert { + condition = alltrue([ + for j in kubernetes_cron_job_v1.sweep : alltrue([ + for e in kubernetes_deployment.app[0].spec[0].template[0].spec[0].container[0].env : + contains(j.spec[0].job_template[0].spec[0].template[0].spec[0].container[0].env, e) + ]) + ]) + error_message = "Task pods must carry every environment variable of the API container, secret references included." + } + assert { + condition = [ + for e in kubernetes_deployment.app[0].spec[0].template[0].spec[0].container[0].env : + e.value_from[0].secret_key_ref[0].key if e.name == "DB_PASSWORD_SECRET" + ] == ["password_secret_name"] + error_message = "DB_PASSWORD_SECRET must read the database Secret's password_secret_name key (#1115); a key the Secret lacks fails the API and task pods with CreateContainerConfigError." + } + assert { + condition = alltrue(flatten([ + for env in concat( + [kubernetes_deployment.app[0].spec[0].template[0].spec[0].container[0].env], + [for j in kubernetes_cron_job_v1.sweep : j.spec[0].job_template[0].spec[0].template[0].spec[0].container[0].env], + ) : [ + for e in env : contains(keys(kubernetes_secret.database[0].data), e.value_from[0].secret_key_ref[0].key) if length(e.value_from) > 0 + ] + ])) + error_message = "Every secret_key_ref of the API and task pods must name a key the database-credentials Secret defines." + } + assert { + condition = alltrue([ + for j in kubernetes_cron_job_v1.sweep : + length(j.spec[0].job_template[0].spec[0].template[0].spec[0].container[0].env) == length(kubernetes_deployment.app[0].spec[0].template[0].spec[0].container[0].env) + 1 && + [for e in j.spec[0].job_template[0].spec[0].template[0].spec[0].container[0].env : e.value if e.name == "TASK_TIMEOUT"] == ["900"] + ]) + error_message = "Task pods add only TASK_TIMEOUT, set from task_timeout." + } + assert { + condition = alltrue([ + for j in kubernetes_cron_job_v1.sweep : + j.spec[0].job_template[0].spec[0].template[0].spec[0].container[0].resources[0].requests["cpu"] == "250m" && j.spec[0].job_template[0].spec[0].template[0].spec[0].container[0].resources[0].limits["memory"] == "1Gi" + ]) + error_message = "Task pods need requests and limits, or the namespace ResourceQuota rejects them." + } +} + +run "opt_in" { + command = plan + + variables { + enable_fire_scheduled_purchases_schedule = true + enable_process_scheduled_purchases_schedule = true + enable_send_notifications_schedule = true + enable_cleanup_schedule = true + enable_ri_exchange_schedule = true + enable_ladder_run_schedule = true + } + + plan_options { + target = [kubernetes_cron_job_v1.sweep] + } + + assert { + condition = toset(keys(kubernetes_cron_job_v1.sweep)) == toset(["analytics_collect", "cleanup", "collect_recommendations", "finalize_revocations", "fire_scheduled_purchases", "ladder_run", "process_scheduled_purchases", "reap_stuck_purchases", "ri_exchange_reshape", "send_notifications"]) + error_message = "Opting in must schedule the eight required tasks plus the opt-in ri_exchange_reshape and ladder_run." + } + assert { + condition = alltrue([ + for k, j in kubernetes_cron_job_v1.sweep : + j.spec[0].job_template[0].spec[0].template[0].spec[0].container[0].command == tolist(["/app/cudly", "--task", k]) + ]) + error_message = "Each CronJob must run /app/cudly --task ." + } + assert { + condition = alltrue([ + for k, j in kubernetes_cron_job_v1.sweep : + j.spec[0].schedule == { collect_recommendations = var.recommendation_schedule, reap_stuck_purchases = var.reap_stuck_purchases_schedule, analytics_collect = var.analytics_collect_schedule, fire_scheduled_purchases = var.fire_scheduled_purchases_schedule, process_scheduled_purchases = var.process_scheduled_purchases_schedule, send_notifications = var.send_notifications_schedule, finalize_revocations = var.finalize_revocations_schedule, cleanup = var.cleanup_schedule, ri_exchange_reshape = var.ri_exchange_schedule, ladder_run = var.ladder_run_schedule }[k] + ]) + error_message = "Each schedule must come from its variable." + } + assert { + condition = length(distinct([for j in kubernetes_cron_job_v1.sweep : j.metadata[0].name])) == 10 + error_message = "CronJob names must be distinct." + } +} + +run "all_off" { + command = plan + + variables { + enable_scheduled_tasks = false + enable_reap_stuck_purchases_schedule = false + enable_analytics_collect_schedule = false + enable_finalize_revocations_schedule = false + } + + plan_options { + target = [kubernetes_cron_job_v1.sweep] + } + + assert { + condition = length(kubernetes_cron_job_v1.sweep) == 0 + error_message = "Each enable variable must switch its CronJob off." + } +} + +run "task_timeout_owned_by_variable" { + command = plan + + variables { + task_timeout = 600 + additional_env_vars = { TASK_TIMEOUT = "5" } + } + + plan_options { + target = [kubernetes_cron_job_v1.sweep] + } + + assert { + condition = alltrue([ + for j in kubernetes_cron_job_v1.sweep : + [for e in j.spec[0].job_template[0].spec[0].template[0].spec[0].container[0].env : e.value if e.name == "TASK_TIMEOUT"] == ["600"] && j.spec[0].job_template[0].spec[0].active_deadline_seconds == 900 + ]) + error_message = "TASK_TIMEOUT must appear once, from task_timeout, with the Job deadline 300s past it." + } +} + +run "rejects_eventbridge_schedule" { + command = plan + + variables { + recommendation_schedule = "rate(1 day)" + } + + plan_options { + target = [kubernetes_cron_job_v1.sweep] + } + + expect_failures = [kubernetes_cron_job_v1.sweep] +} + +run "rejects_six_field_cron" { + command = plan + + variables { + reap_stuck_purchases_schedule = "0 */5 * * ? *" + } + + plan_options { + target = [kubernetes_cron_job_v1.sweep] + } + + expect_failures = [kubernetes_cron_job_v1.sweep] +} + +run "rejects_long_project_name" { + command = plan + + variables { + # 40 characters: within the cluster's 54-character dns_prefix, but + # "-task-analytics" takes the CronJob name past 52. + project_name = "cudly-a-long-project-name-for-the-limits" + } + + plan_options { + target = [kubernetes_cron_job_v1.sweep] + } + + expect_failures = [kubernetes_cron_job_v1.sweep] +} + +run "rejects_short_task_timeout" { + command = plan + + variables { + task_timeout = 30 + } + + plan_options { + target = [kubernetes_cron_job_v1.sweep] + } + + expect_failures = [var.task_timeout] +} diff --git a/terraform/modules/compute/azure/aks/variables.tf b/terraform/modules/compute/azure/aks/variables.tf index 435e523a8..dc9de06f7 100644 --- a/terraform/modules/compute/azure/aks/variables.tf +++ b/terraform/modules/compute/azure/aks/variables.tf @@ -229,3 +229,137 @@ variable "archera_api_key_secret_name" { type = string default = "" } + +# Scheduled server tasks (#893), run as CronJobs when +# deploy_kubernetes_resources is true. Schedules are 5-field cron in UTC. +variable "enable_scheduled_tasks" { + description = "Schedule the collect_recommendations task." + type = bool + default = true +} + +variable "recommendation_schedule" { + description = "Cron schedule for collect_recommendations." + type = string + default = "0 2 * * *" +} + +variable "enable_reap_stuck_purchases_schedule" { + description = "Schedule the reap_stuck_purchases sweep (backstop for purchases stuck in approved/running, #678)." + type = bool + default = true +} + +variable "reap_stuck_purchases_schedule" { + description = "Cron schedule for reap_stuck_purchases. Keep it more frequent than PURCHASE_APPROVED_REAP_AFTER (default 10m)." + type = string + default = "*/5 * * * *" +} + +variable "enable_analytics_collect_schedule" { + description = "Schedule the analytics_collect task (savings snapshot, partition maintenance, retention, view refresh)." + type = bool + default = true +} + +variable "analytics_collect_schedule" { + description = "Cron schedule for analytics_collect." + type = string + default = "0 4 * * *" +} + +variable "enable_fire_scheduled_purchases_schedule" { + description = "Schedule the fire_scheduled_purchases sweep, which fires delayed auto-approved purchases. Opt-in: default false." + type = bool + default = false +} + +variable "fire_scheduled_purchases_schedule" { + description = "Cron schedule for fire_scheduled_purchases." + type = string + default = "*/15 * * * *" +} + +variable "enable_process_scheduled_purchases_schedule" { + description = "Schedule the process_scheduled_purchases sweep (proposes purchases that are due). Opt-in: default false." + type = bool + default = false +} + +variable "process_scheduled_purchases_schedule" { + description = "Cron schedule for process_scheduled_purchases." + type = string + default = "0 * * * *" +} + +variable "enable_send_notifications_schedule" { + description = "Schedule the send_notifications sweep. Opt-in: default false; enable it together with process_scheduled_purchases." + type = bool + default = false +} + +variable "send_notifications_schedule" { + description = "Cron schedule for send_notifications." + type = string + default = "0 8 * * *" +} + +variable "enable_finalize_revocations_schedule" { + description = "Schedule the finalize_revocations sweep (idempotent audit repair of revoked purchases)." + type = bool + default = true +} + +variable "finalize_revocations_schedule" { + description = "Cron schedule for finalize_revocations." + type = string + default = "30 * * * *" +} + +variable "enable_cleanup_schedule" { + description = "Schedule the server cleanup sweep. Opt-in: default false. 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." + type = bool + default = false +} + +variable "cleanup_schedule" { + description = "Cron schedule for cleanup." + type = string + default = "0 3 * * *" +} + +variable "enable_ri_exchange_schedule" { + description = "Schedule the ri_exchange_reshape task (RI exchange automation). Opt-in: default false, as on the other platforms." + type = bool + default = false +} + +variable "ri_exchange_schedule" { + # Standard 5-field cron: 0 */6 * * * runs at 00:00, 06:00, 12:00, 18:00 UTC. + description = "Cron schedule for ri_exchange_reshape." + type = string + default = "0 */6 * * *" +} + +variable "enable_ladder_run_schedule" { + description = "Schedule the ladder_run task (commitment-ladder planning run). Opt-in: default false until laddering is promoted to GA, as on AWS." + type = bool + default = false +} + +variable "ladder_run_schedule" { + description = "Cron schedule for ladder_run." + type = string + default = "0 2 * * *" +} + +variable "task_timeout" { + description = "Seconds a scheduled task may run (TASK_TIMEOUT). The Job is killed 300 seconds after that." + type = number + default = 900 + + validation { + condition = var.task_timeout >= 60 && floor(var.task_timeout) == var.task_timeout + error_message = "task_timeout must be a whole number of seconds, at least 60." + } +} diff --git a/terraform/modules/compute/azure/aks_hardening_test.go b/terraform/modules/compute/azure/aks_hardening_test.go index 6621b4cb2..f31beb0da 100644 --- a/terraform/modules/compute/azure/aks_hardening_test.go +++ b/terraform/modules/compute/azure/aks_hardening_test.go @@ -74,9 +74,17 @@ func TestAKSDatabaseSecretKeyRefsExistInSecretData(t *testing.T) { keys[m[1]] = true } + // A key is either a literal on the secret_key_ref itself or, since the + // Deployment and the scheduled-task CronJobs (#893) render their env from + // local.app_env, the secret_ref of an app_env entry. refs := regexp.MustCompile(`secret_key_ref\s*\{\s*name\s*=\s*kubernetes_secret\.database\[0\]\.metadata\[0\]\.name\s*key\s*=\s*"([^"]+)"`).FindAllStringSubmatch(content, -1) + refs = append(refs, regexp.MustCompile(`\bsecret_ref\s*=\s*"([^"]+)"`).FindAllStringSubmatch(content, -1)...) if len(refs) == 0 { - t.Fatal(`found no secret_key_ref pointing at kubernetes_secret.database`) + t.Fatal(`found no secret_key_ref or app_env secret_ref naming a kubernetes_secret.database key`) + } + pw := regexp.MustCompile(`\{\s*name\s*=\s*"DB_PASSWORD_SECRET"\s*,\s*value\s*=\s*null\s*,\s*secret_ref\s*=\s*"([^"]+)"\s*\}`).FindAllStringSubmatch(content, -1) + if len(pw) != 1 || pw[0][1] != "password_secret_name" { + t.Error(`local.app_env must read DB_PASSWORD_SECRET from the database Secret's "password_secret_name" key (#1115)`) } for _, r := range refs { if !keys[r[1]] { diff --git a/terraform/modules/compute/scheduled_task_contract_test.go b/terraform/modules/compute/scheduled_task_contract_test.go index 26bf4073a..7a074a5b6 100644 --- a/terraform/modules/compute/scheduled_task_contract_test.go +++ b/terraform/modules/compute/scheduled_task_contract_test.go @@ -28,6 +28,9 @@ var schedulerRefs = []struct { // (the old "event") is not recognized by the Lambda event router. {"aws/lambda/main.tf", regexp.MustCompile(`jsonencode\(\{\s*action\s*=\s*"([^"]+)"`), 6}, {"aws/fargate/main.tf", regexp.MustCompile(`"--task",\s*"([^"]+)"`), 4}, + // AKS CronJobs run `--task each.key` over the sweep_tasks map, so the + // task names are the map keys (#893). + {"azure/aks/scheduled-tasks.tf", regexp.MustCompile(`(?m)^\s*(\w+)\s*=\s*\{\s*job\s*=`), 10}, } func TestSchedulerTargetsNameKnownServerTasks(t *testing.T) { diff --git a/terraform/modules/compute/scheduled_task_coverage_test.go b/terraform/modules/compute/scheduled_task_coverage_test.go index 189dfd1b3..ce30b57dc 100644 --- a/terraform/modules/compute/scheduled_task_coverage_test.go +++ b/terraform/modules/compute/scheduled_task_coverage_test.go @@ -41,9 +41,7 @@ const ( var ( validDims = []string{dimTask, dimPass, dimDefault} - allEight = []string{"analytics_collect", "cleanup", "collect_recommendations", "finalize_revocations", - "fire_scheduled_purchases", "process_scheduled_purchases", "reap_stuck_purchases", "send_notifications"} - sevenGCP = []string{"analytics_collect", "cleanup", "finalize_revocations", "fire_scheduled_purchases", + sevenGCP = []string{"analytics_collect", "cleanup", "finalize_revocations", "fire_scheduled_purchases", "process_scheduled_purchases", "reap_stuck_purchases", "send_notifications"} ) @@ -58,15 +56,15 @@ var knownGaps = map[string]map[string][]string{ "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 } // aws platforms are also checked against terraform/environments/aws: envBlock // is the module block in compute.tf, needsPermission whether sweeps.tf must -// hold an aws_lambda_permission (Lambda only). +// hold an aws_lambda_permission (Lambda only). cronJob platforms schedule +// their sweep_tasks through a Kubernetes CronJob in scheduled-tasks.tf. type platform struct { - name, dir, envBlock string - aws, needsPermission bool + name, dir, envBlock string + aws, needsPermission, cronJob bool } var platforms = []platform{ @@ -76,7 +74,7 @@ var platforms = []platform{ {name: "gcp-cloud-run", dir: "gcp/cloud-run"}, {name: "gcp-gke", dir: "gcp/gke"}, {name: "azure-container-apps", dir: "azure/container-apps"}, - {name: "azure-aks", dir: "azure/aks"}, + {name: "azure-aks", dir: "azure/aks", cronJob: true}, } // Copied from aws/cross_account_sts_discovery_test.go (commentLinePattern): @@ -332,6 +330,15 @@ func TestRequiredTasksWiredPerPlatform(t *testing.T) { t.Errorf("%s: sweeps.tf: %s", p.name, prob) } } + if p.cronJob { + probs, err := cronJobProblems(p.dir) + if err != nil { + t.Errorf("%s: %v", p.name, err) + } + for _, prob := range probs { + t.Errorf("%s: %s: %s", p.name, cronJobFile, prob) + } + } } } diff --git a/terraform/modules/compute/scheduled_task_cronjob_test.go b/terraform/modules/compute/scheduled_task_cronjob_test.go new file mode 100644 index 000000000..0af26c54b --- /dev/null +++ b/terraform/modules/compute/scheduled_task_cronjob_test.go @@ -0,0 +1,81 @@ +// AKS checks for issue #893: the AKS module schedules its sweep_tasks through a +// single for_each Kubernetes CronJob. wiredFrom counts every sweep_tasks key as +// wired; cronJobProblems checks that a CronJob actually runs each enabled key +// through the server's --task entry point. +package compute_test + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +const cronJobFile = "scheduled-tasks.tf" + +var ( + cronForEachRe = regexp.MustCompile(`(?m)^\s*for_each\s*=\s*local\.active_sweeps\s*$`) + cronCommandRe = regexp.MustCompile(`(?m)^\s*command\s*=\s*\[\s*"/app/cudly"\s*,\s*"--task"\s*,\s*each\.key\s*\]\s*$`) + // active_sweeps must keep the enabled entries of sweep_tasks, keyed by the + // task name (any outer gate, such as deploy_kubernetes_resources, is fine). + activeSweepsRe = regexp.MustCompile(`(?m)^\s*active_sweeps\s*=.*\{\s*for\s+(\w+)\s*,\s*(\w+)\s+in\s+local\.sweep_tasks\s*:\s*(\w+)\s*=>\s*(\w+)\s+if\s+(\w+)\.enabled\s*\}`) +) + +// cronJobProblems cross-checks dir/scheduled-tasks.tf: one CronJob iterating +// local.active_sweeps, running `/app/cudly --task `, with active_sweeps +// derived from the enabled sweep_tasks entries. +func cronJobProblems(dir string) ([]string, error) { + raw, err := os.ReadFile(filepath.Join(dir, cronJobFile)) + if err != nil { + return nil, err + } + src := stripComments(string(raw)) + body, ok := blockOnce(src, `resource\s+"kubernetes_cron_job_v1"\s+"sweep"\s*\{`) + if !ok { + return []string{"kubernetes_cron_job_v1.sweep is missing or duplicated"}, nil + } + var problems []string + if !cronForEachRe.MatchString(body) { + problems = append(problems, "kubernetes_cron_job_v1.sweep lacks for_each = local.active_sweeps") + } + if !cronCommandRe.MatchString(body) { + problems = append(problems, `kubernetes_cron_job_v1.sweep lacks command = ["/app/cudly", "--task", each.key]`) + } + if m := activeSweepsRe.FindStringSubmatch(src); m == nil || m[1] != m[3] || m[2] != m[4] || m[2] != m[5] { + problems = append(problems, "active_sweeps is not { for k, v in local.sweep_tasks : k => v if v.enabled }") + } + return problems, nil +} + +func TestCronJobCrossWiring(t *testing.T) { + const active = "locals {\n active_sweeps = var.deploy_kubernetes_resources ? { for k, v in local.sweep_tasks : k => v if v.enabled } : {}\n}\n" + const job = "resource \"kubernetes_cron_job_v1\" \"sweep\" {\n for_each = local.active_sweeps\n spec {\n container {\n command = [\"/app/cudly\", \"--task\", each.key]\n }\n }\n}\n" + for name, c := range map[string]struct { + src string + wantProb bool + }{ + "wired": {active + job, false}, + "ungated active_sweeps": {strings.Replace(active, "var.deploy_kubernetes_resources ? ", "", 1) + strings.Replace(job, " : {}", "", 1), false}, + "missing CronJob": {active, true}, + "duplicated CronJob": {active + job + job, true}, + "CronJob under another name": {active + strings.Replace(job, `"sweep"`, `"tasks"`, 1), true}, + "not for_each active_sweeps": {active + strings.Replace(job, "local.active_sweeps", "local.sweep_tasks", 1), true}, + "literal task name": {active + strings.Replace(job, "each.key", `"cleanup"`, 1), true}, + "task from each.value": {active + strings.Replace(job, "each.key", "each.value.job", 1), true}, + "wrong binary": {active + strings.Replace(job, "/app/cudly", "/entrypoint.sh", 1), true}, + "commented-out command": {active + strings.Replace(job, " command", " # command", 1), true}, + "active_sweeps ignores enabled": {strings.Replace(active, " if v.enabled", "", 1) + job, true}, + "active_sweeps filters a field": {strings.Replace(active, "if v.enabled", "if v.job", 1) + job, true}, + "active_sweeps from other map": {strings.Replace(active, "local.sweep_tasks", "local.other", 1) + job, true}, + "active_sweeps renames the key": {strings.Replace(active, "k => v", "v.job => v", 1) + job, true}, + } { + got, err := cronJobProblems(writeFiles(t, map[string]string{cronJobFile: c.src})) + if err != nil || (len(got) > 0) != c.wantProb { + t.Errorf("%s: problems = %v, err = %v", name, got, err) + } + } + if _, err := cronJobProblems(t.TempDir()); err == nil { + t.Error("a missing scheduled-tasks.tf must be an error") + } +}