From f91b1f7fc7d02132746e663012363a4d69f3cf55 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 30 Sep 2026 17:01:25 -0400 Subject: [PATCH 1/3] feat: allow cron-entries to set a per-entry MAILTO The `cron-entries` trigger is now passed `json` as a second argument, and plugins may print each task as a json object on its own line. A task that sets a `mailto` is written to the crontab under a `MAILTO` of its own, and its output is mailed to that address as well as appended to its log file, so plugins such as the datastore plugins can have the output of a specific scheduled task mailed to a specific person. --- docs/development/plugin-triggers.md | 13 +- docs/processes/scheduled-cron-tasks.md | 1 + plugins/cron/cron.go | 42 ++--- plugins/cron/cron_test.go | 32 ++++ plugins/cron/crontab.go | 178 ++++++++++++++++---- plugins/cron/crontab_test.go | 224 +++++++++++++++++++++++++ plugins/cron/templates/cron.tmpl | 6 + tests/unit/cron-3.bats | 40 +++++ 8 files changed, 474 insertions(+), 62 deletions(-) create mode 100644 plugins/cron/crontab_test.go diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index b5cc0445f..d18833250 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -688,8 +688,10 @@ APP="$1"; PROPERTY="$2" ### `cron-entries` - Description: Allows injecting cron tasks into the written out scheduled cron task list. Each entry is newline delimited, and individual tasks come in the form `$SCHEDULE;$FULL_COMMAND;$ARBITRARY_DATA`. Individual implementations of cron writing can decide whether and how to include these cron tasks. The `ARBITRARY_DATA` includes the log file path for the basic `docker-local` cron implementation. -- Invoked by: -- Arguments: `$DOKKU_SCHEDULER` + + When `$CRON_ENTRY_FORMAT` is `json`, each entry may instead be printed as a json object on its own line, with the keys `schedule`, `command`, `log-file` and `mailto`. Only `schedule` and `command` are required. A task with a `mailto` value is written to the crontab under a `MAILTO` of its own rather than the global `MAILTO`, and its output is mailed to that address in addition to being appended to its `log-file`. Json entries and `;` delimited entries may be printed together. Older versions of Dokku do not pass `$CRON_ENTRY_FORMAT`, and fail to write the crontab if a json entry is printed, so a json entry should only be printed when the argument is `json`. +- Invoked by: `dokku cron:list --global`, and when the `dokku` user crontab is written +- Arguments: `$DOKKU_SCHEDULER $CRON_ENTRY_FORMAT` - Example: ```shell @@ -699,8 +701,13 @@ set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x source "$PLUGIN_CORE_AVAILABLE_PATH/common/functions" DOKKU_SCHEDULER="$1" +CRON_ENTRY_FORMAT="$2" -# TODO +if [[ "$CRON_ENTRY_FORMAT" == "json" ]]; then + echo '{"schedule":"@daily","command":"/usr/local/bin/some-task","log-file":"/var/log/dokku/some-task.log","mailto":"ops@example.com"}' +else + echo "@daily;/usr/local/bin/some-task;/var/log/dokku/some-task.log" +fi ``` ### `dependencies` diff --git a/docs/processes/scheduled-cron-tasks.md b/docs/processes/scheduled-cron-tasks.md index a54fcd24c..977d8dada 100644 --- a/docs/processes/scheduled-cron-tasks.md +++ b/docs/processes/scheduled-cron-tasks.md @@ -56,6 +56,7 @@ When running scheduled cron tasks, there are a few items to be aware of: - At this time, only the `PATH` and `SHELL` environment variables are specified in the cron template. - A `MAILTO` value can be set via the `cron:set` command. - A `MAILFROM` value can be set via the `cron:set` command. + - Tasks injected by plugins via the [`cron-entries` trigger](/docs/development/plugin-triggers.md#cron-entries) may set a `MAILTO` value of their own, which is used for that task instead of the global value. The output of such a task is mailed to that address as well as appended to the task's log file. - Each scheduled task is executed within a one-off `run` container, and thus inherit any docker-options specified for `run` containers. Resources are never shared between scheduled tasks. - Scheduled cron tasks are supported on a per-scheduler basis. Schedulers that use the host crontab - such as `docker-local` - have their `app.json` cron tasks written to the `dokku` user crontab, while schedulers that manage their own cron backend - such as `k3s` - schedule them natively. - Tasks for _all_ apps managed by a host-crontab scheduler such as `docker-local` are written to a single crontab file owned by the `dokku` user. The `dokku` user's crontab should be considered reserved for this purpose. diff --git a/plugins/cron/cron.go b/plugins/cron/cron.go index 845abfbff..ba23e4c78 100644 --- a/plugins/cron/cron.go +++ b/plugins/cron/cron.go @@ -3,7 +3,6 @@ package cron import ( "fmt" "strconv" - "strings" appjson "github.com/dokku/dokku/plugins/app-json" "github.com/dokku/dokku/plugins/common" @@ -85,6 +84,9 @@ type CronTask struct { // LogFile is the log file to write to LogFile string `json:"-"` + // Mailto is the MAILTO value cron uses for this task instead of the global one + Mailto string `json:"mailto,omitempty"` + // AppInMaintenance is whether the app's cron is in maintenance mode AppInMaintenance bool `json:"app-in-maintenance"` @@ -98,6 +100,10 @@ type CronTask struct { // DokkuRunCommand returns the dokku run command to execute for a given cron task func (t CronTask) DokkuRunCommand() string { if t.AltCommand != "" { + if t.LogFile != "" && t.Mailto != "" { + // keep the output on stdout so cron mails it to the task's MAILTO + return fmt.Sprintf("%s 2>&1 | tee -a %s", t.AltCommand, t.LogFile) + } if t.LogFile != "" { return fmt.Sprintf("%s &>> %s", t.AltCommand, t.LogFile) } @@ -210,37 +216,17 @@ func FetchCronTasks(input FetchCronTasksInput) ([]CronTask, error) { // This function should only be used for the cron:list --global command // and not internally by the cron plugin func FetchGlobalCronTasks() ([]CronTask, error) { - tasks := []CronTask{} response, _ := common.CallPlugnTrigger(common.PlugnTriggerInput{ Trigger: "cron-entries", - Args: []string{"docker-local"}, + Args: []string{"docker-local", CronEntryFormat}, }) - for _, line := range strings.Split(response.StdoutContents(), "\n") { - if strings.TrimSpace(line) == "" { - continue - } - parts := strings.Split(line, ";") - if len(parts) != 2 && len(parts) != 3 { - common.LogWarn(fmt.Sprintf("Invalid injected cron task: %v", line)) - continue - } - - id := base36.EncodeToStringLc([]byte(strings.Join(parts, ";;;"))) - task := CronTask{ - ID: id, - Schedule: parts[0], - Command: parts[1], - AltCommand: parts[1], - Global: true, - Maintenance: false, - TaskInMaintenance: false, - AppInMaintenance: false, - } - if len(parts) == 3 { - task.LogFile = parts[2] - } - tasks = append(tasks, task) + tasks, invalid := parseInjectedCronEntries(response.StdoutContents()) + for _, line := range invalid { + common.LogWarn(fmt.Sprintf("Invalid injected cron task: %v", line)) + } + for i := range tasks { + tasks[i].Global = true } return tasks, nil } diff --git a/plugins/cron/cron_test.go b/plugins/cron/cron_test.go index 2f9767415..31f9799a9 100644 --- a/plugins/cron/cron_test.go +++ b/plugins/cron/cron_test.go @@ -101,6 +101,38 @@ func TestDokkuRunCommandAltCommandWithLogFile(t *testing.T) { } } +// TestDokkuRunCommandAltCommandWithMailto pins that an injected task with its +// own MAILTO keeps its output on stdout, where cron mails it from, while still +// appending it to the log file. +func TestDokkuRunCommandAltCommandWithMailto(t *testing.T) { + task := CronTask{ + ID: "abc123", + AltCommand: "/usr/bin/some-internal-task", + LogFile: "/var/log/dokku/internal-task.log", + Mailto: "ops@example.com", + } + + got := task.DokkuRunCommand() + want := "/usr/bin/some-internal-task 2>&1 | tee -a /var/log/dokku/internal-task.log" + if got != want { + t.Errorf("DokkuRunCommand() = %q, want %q", got, want) + } +} + +func TestDokkuRunCommandAltCommandWithMailtoAndNoLogFile(t *testing.T) { + task := CronTask{ + ID: "abc123", + AltCommand: "/usr/bin/some-internal-task", + Mailto: "ops@example.com", + } + + got := task.DokkuRunCommand() + want := "/usr/bin/some-internal-task" + if got != want { + t.Errorf("DokkuRunCommand() = %q, want %q", got, want) + } +} + // TestDokkuRunCommandAppTaskIgnoresLogFile pins that LogFile is honored only // for internally injected tasks from the cron-entries trigger. App tasks never // interpolate a path into the crontab line - their output is shipped by the diff --git a/plugins/cron/crontab.go b/plugins/cron/crontab.go index a2b229f32..42cd1c0b2 100644 --- a/plugins/cron/crontab.go +++ b/plugins/cron/crontab.go @@ -2,8 +2,11 @@ package cron import ( _ "embed" + "encoding/json" "fmt" + "maps" "os" + "slices" "strings" "text/template" @@ -50,36 +53,114 @@ func hostCronSchedulers(appSchedulers []string) map[string]bool { return schedulers } -// injectedCronTasks parses the tasks injected via the cron-entries trigger for a -// given scheduler. Each entry is newline delimited in the form -// $SCHEDULE;$COMMAND[;$LOGFILE]. -func injectedCronTasks(scheduler string) ([]CronTask, error) { +// CronEntryFormat is passed to the cron-entries trigger to signal that each +// entry may be printed as a json object on its own line. Implementations that +// do not know it keep printing $SCHEDULE;$COMMAND[;$LOGFILE] lines, which are +// still read. +const CronEntryFormat = "json" + +// injectedCronEntry is a task printed as a json line by the cron-entries trigger +type injectedCronEntry struct { + // Schedule is the cron schedule + Schedule string `json:"schedule"` + + // Command is the command to run + Command string `json:"command"` + + // LogFile is the log file the command's output is appended to + LogFile string `json:"log-file"` + + // Mailto is the MAILTO value cron uses for the task instead of the global one + Mailto string `json:"mailto"` +} + +// parseInjectedCronEntry parses a single line printed by the cron-entries +// trigger, either a json object or in the form $SCHEDULE;$COMMAND[;$LOGFILE]. +// The fields the task id is generated from are returned alongside the task. +func parseInjectedCronEntry(line string) (injectedCronEntry, []string, bool) { + if strings.HasPrefix(strings.TrimSpace(line), "{") { + entry := injectedCronEntry{} + if err := json.Unmarshal([]byte(line), &entry); err != nil { + return injectedCronEntry{}, nil, false + } + if entry.Schedule == "" || entry.Command == "" { + return injectedCronEntry{}, nil, false + } + + // json can encode a newline, which would add a line to the crontab + fields := []string{entry.Schedule, entry.Command, entry.LogFile, entry.Mailto} + for _, field := range fields { + if strings.ContainsAny(field, "\r\n") { + return injectedCronEntry{}, nil, false + } + } + + fields = []string{entry.Schedule, entry.Command} + if entry.LogFile != "" { + fields = append(fields, entry.LogFile) + } + if entry.Mailto != "" { + fields = append(fields, entry.Mailto) + } + return entry, fields, true + } + + parts := strings.Split(line, ";") + if len(parts) != 2 && len(parts) != 3 { + return injectedCronEntry{}, nil, false + } + + entry := injectedCronEntry{ + Schedule: parts[0], + Command: parts[1], + } + if len(parts) == 3 { + entry.LogFile = parts[2] + } + return entry, parts, true +} + +// parseInjectedCronEntries parses the newline delimited output of the +// cron-entries trigger, returning the tasks and any lines that are not valid +// entries +func parseInjectedCronEntries(output string) ([]CronTask, []string) { tasks := []CronTask{} + invalid := []string{} + for _, line := range strings.Split(output, "\n") { + if strings.TrimSpace(line) == "" { + continue + } + + entry, fields, ok := parseInjectedCronEntry(line) + if !ok { + invalid = append(invalid, line) + continue + } + + tasks = append(tasks, CronTask{ + ID: base36.EncodeToStringLc([]byte(strings.Join(fields, ";;;"))), + Schedule: entry.Schedule, + Command: entry.Command, + AltCommand: entry.Command, + LogFile: entry.LogFile, + Mailto: entry.Mailto, + Maintenance: false, + }) + } + return tasks, invalid +} + +// injectedCronTasks parses the tasks injected via the cron-entries trigger for a +// given scheduler +func injectedCronTasks(scheduler string) ([]CronTask, error) { response, _ := common.CallPlugnTrigger(common.PlugnTriggerInput{ Trigger: "cron-entries", - Args: []string{scheduler}, + Args: []string{scheduler, CronEntryFormat}, }) - for _, line := range strings.Split(response.StdoutContents(), "\n") { - if strings.TrimSpace(line) == "" { - return []CronTask{}, nil - } - parts := strings.Split(line, ";") - if len(parts) != 2 && len(parts) != 3 { - return []CronTask{}, fmt.Errorf("Invalid injected cron task: %v", line) - } - - id := base36.EncodeToStringLc([]byte(strings.Join(parts, ";;;"))) - task := CronTask{ - ID: id, - Schedule: parts[0], - AltCommand: parts[1], - Maintenance: false, - } - if len(parts) == 3 { - task.LogFile = parts[2] - } - tasks = append(tasks, task) + tasks, invalid := parseInjectedCronEntries(response.StdoutContents()) + if len(invalid) > 0 { + return []CronTask{}, fmt.Errorf("Invalid injected cron task: %v", invalid[0]) } return tasks, nil } @@ -179,12 +260,7 @@ func writeCronTab() error { mailfrom := common.PropertyGetDefault("cron", "--global", "mailfrom", DefaultProperties["mailfrom"]) mailto := common.PropertyGetDefault("cron", "--global", "mailto", DefaultProperties["mailto"]) - - data := map[string]interface{}{ - "Tasks": tasks, - "Mailfrom": mailfrom, - "Mailto": mailto, - } + data := cronTemplateData(tasks, mailfrom, mailto) t, err := getCronTemplate() if err != nil { @@ -219,6 +295,46 @@ func writeCronTab() error { return nil } +// cronMailtoGroup is the set of tasks written to the crontab under a MAILTO of +// their own +type cronMailtoGroup struct { + // Mailto is the MAILTO value the tasks are written under + Mailto string + + // Tasks are the tasks written under the MAILTO value + Tasks []CronTask +} + +// cronTemplateData builds the data the cron template is executed with. Tasks +// that set their own MAILTO are grouped by it and written after every other +// task, so the global MAILTO applies to all of the tasks before them. +func cronTemplateData(tasks []CronTask, mailfrom string, mailto string) map[string]interface{} { + globalTasks := []CronTask{} + groups := map[string][]CronTask{} + for _, task := range tasks { + if task.Mailto == "" { + globalTasks = append(globalTasks, task) + continue + } + groups[task.Mailto] = append(groups[task.Mailto], task) + } + + mailtoGroups := []cronMailtoGroup{} + for _, groupMailto := range slices.Sorted(maps.Keys(groups)) { + mailtoGroups = append(mailtoGroups, cronMailtoGroup{ + Mailto: groupMailto, + Tasks: groups[groupMailto], + }) + } + + return map[string]interface{}{ + "Tasks": globalTasks, + "MailtoGroups": mailtoGroups, + "Mailfrom": mailfrom, + "Mailto": mailto, + } +} + // deleteCrontab removes the dokku user crontab func deleteCrontab() error { result, err := common.CallExecCommand(common.ExecCommandInput{ diff --git a/plugins/cron/crontab_test.go b/plugins/cron/crontab_test.go new file mode 100644 index 000000000..ec02286a2 --- /dev/null +++ b/plugins/cron/crontab_test.go @@ -0,0 +1,224 @@ +package cron + +import ( + "bytes" + "reflect" + "strings" + "testing" + + base36 "github.com/multiformats/go-base36" +) + +func TestParseInjectedCronEntries(t *testing.T) { + cases := []struct { + name string + output string + wantTasks []CronTask + wantInvalid []string + }{ + { + name: "text entry without a log file", + output: "@daily;/bin/true", + wantTasks: []CronTask{ + {Schedule: "@daily", Command: "/bin/true", AltCommand: "/bin/true"}, + }, + }, + { + name: "text entry with a log file", + output: "@daily;/bin/true;/var/log/dokku/log.log", + wantTasks: []CronTask{ + {Schedule: "@daily", Command: "/bin/true", AltCommand: "/bin/true", LogFile: "/var/log/dokku/log.log"}, + }, + }, + { + name: "json entry without a log file or mailto", + output: `{"schedule":"@daily","command":"/bin/true"}`, + wantTasks: []CronTask{ + {Schedule: "@daily", Command: "/bin/true", AltCommand: "/bin/true"}, + }, + }, + { + name: "json entry with a log file and mailto", + output: `{"schedule":"@daily","command":"/bin/true","log-file":"/var/log/dokku/log.log","mailto":"ops@example.com"}`, + wantTasks: []CronTask{ + {Schedule: "@daily", Command: "/bin/true", AltCommand: "/bin/true", LogFile: "/var/log/dokku/log.log", Mailto: "ops@example.com"}, + }, + }, + { + name: "json entry whose command holds a semicolon", + output: `{"schedule":"@daily","command":"/bin/true; /bin/false"}`, + wantTasks: []CronTask{ + {Schedule: "@daily", Command: "/bin/true; /bin/false", AltCommand: "/bin/true; /bin/false"}, + }, + }, + { + name: "json and text entries printed together, with blank lines", + output: "@daily;/bin/true\n\n" + `{"schedule":"@hourly","command":"/bin/false","mailto":"ops@example.com"}` + "\n", + wantTasks: []CronTask{ + {Schedule: "@daily", Command: "/bin/true", AltCommand: "/bin/true"}, + {Schedule: "@hourly", Command: "/bin/false", AltCommand: "/bin/false", Mailto: "ops@example.com"}, + }, + }, + { + name: "text entry with too many fields", + output: "@daily;/bin/true;/var/log/dokku/log.log;ops@example.com", + wantTasks: []CronTask{}, + wantInvalid: []string{"@daily;/bin/true;/var/log/dokku/log.log;ops@example.com"}, + }, + { + name: "text entry with too few fields", + output: "@daily", + wantTasks: []CronTask{}, + wantInvalid: []string{"@daily"}, + }, + { + name: "json entry that does not decode", + output: `{"schedule":"@daily",`, + wantTasks: []CronTask{}, + wantInvalid: []string{`{"schedule":"@daily",`}, + }, + { + name: "json entry without a schedule", + output: `{"command":"/bin/true"}`, + wantTasks: []CronTask{}, + wantInvalid: []string{`{"command":"/bin/true"}`}, + }, + { + name: "json entry without a command", + output: `{"schedule":"@daily"}`, + wantTasks: []CronTask{}, + wantInvalid: []string{`{"schedule":"@daily"}`}, + }, + { + name: "json entry whose mailto holds a newline", + output: `{"schedule":"@daily","command":"/bin/true","mailto":"ops@example.com\n* * * * * /bin/evil"}`, + wantTasks: []CronTask{}, + wantInvalid: []string{`{"schedule":"@daily","command":"/bin/true","mailto":"ops@example.com\n* * * * * /bin/evil"}`}, + }, + { + name: "json entry whose command holds a carriage return", + output: `{"schedule":"@daily","command":"/bin/true\r/bin/evil"}`, + wantTasks: []CronTask{}, + wantInvalid: []string{`{"schedule":"@daily","command":"/bin/true\r/bin/evil"}`}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tasks, invalid := parseInjectedCronEntries(tc.output) + for i := range tasks { + if tasks[i].ID == "" { + t.Errorf("task %d has no id", i) + } + tasks[i].ID = "" + } + + if !reflect.DeepEqual(tasks, tc.wantTasks) { + t.Errorf("parseInjectedCronEntries() tasks = %#v, want %#v", tasks, tc.wantTasks) + } + + wantInvalid := tc.wantInvalid + if wantInvalid == nil { + wantInvalid = []string{} + } + if !reflect.DeepEqual(invalid, wantInvalid) { + t.Errorf("parseInjectedCronEntries() invalid = %#v, want %#v", invalid, wantInvalid) + } + }) + } +} + +// TestParseInjectedCronEntriesKeepsTextIDs pins that the id of a text entry is +// generated as it was before json entries were read, and that a json entry +// without a mailto gets the same id as the text entry it replaces. +func TestParseInjectedCronEntriesKeepsTextIDs(t *testing.T) { + want := base36.EncodeToStringLc([]byte("@daily;;;/bin/true;;;/var/log/dokku/log.log")) + + for _, output := range []string{ + "@daily;/bin/true;/var/log/dokku/log.log", + `{"schedule":"@daily","command":"/bin/true","log-file":"/var/log/dokku/log.log"}`, + } { + tasks, _ := parseInjectedCronEntries(output) + if len(tasks) != 1 { + t.Fatalf("parseInjectedCronEntries(%q) returned %d tasks, want 1", output, len(tasks)) + } + if tasks[0].ID != want { + t.Errorf("parseInjectedCronEntries(%q) id = %q, want %q", output, tasks[0].ID, want) + } + } + + withMailto, _ := parseInjectedCronEntries(`{"schedule":"@daily","command":"/bin/true","log-file":"/var/log/dokku/log.log","mailto":"ops@example.com"}`) + if len(withMailto) != 1 || withMailto[0].ID == want { + t.Errorf("a json entry with a mailto should not share the id of the entry without one") + } +} + +// TestCronTemplateGroupsTasksByMailto pins that tasks with their own MAILTO are +// written after every task that uses the global MAILTO, grouped under a MAILTO +// line per recipient. +func TestCronTemplateGroupsTasksByMailto(t *testing.T) { + tasks := []CronTask{ + {Schedule: "@daily", AltCommand: "/bin/zeta", LogFile: "/var/log/dokku/zeta.log", Mailto: "zeta@example.com"}, + {Schedule: "@daily", AltCommand: "/bin/true", LogFile: "/var/log/dokku/log.log"}, + {Schedule: "@hourly", AltCommand: "/bin/alpha", LogFile: "/var/log/dokku/alpha.log", Mailto: "alpha@example.com"}, + {Schedule: "@weekly", AltCommand: "/bin/zeta-weekly", Mailto: "zeta@example.com"}, + } + + tmpl, err := getCronTemplate() + if err != nil { + t.Fatalf("getCronTemplate() returned an error: %v", err) + } + + var got bytes.Buffer + if err := tmpl.Execute(&got, cronTemplateData(tasks, "", "global@example.com")); err != nil { + t.Fatalf("Execute() returned an error: %v", err) + } + + want := strings.Join([]string{ + "MAILTO=global@example.com", + "PATH=/usr/local/bin:/usr/bin:/bin", + "SHELL=/bin/bash", + "", + "@daily /bin/true &>> /var/log/dokku/log.log", + "", + "MAILTO=alpha@example.com", + "@hourly /bin/alpha 2>&1 | tee -a /var/log/dokku/alpha.log", + "", + "MAILTO=zeta@example.com", + "@daily /bin/zeta 2>&1 | tee -a /var/log/dokku/zeta.log", + "@weekly /bin/zeta-weekly", + "", + }, "\n") + if got.String() != want { + t.Errorf("rendered crontab =\n%s\nwant\n%s", got.String(), want) + } +} + +// TestCronTemplateWithoutMailtoGroups pins that a crontab without tasks that set +// their own MAILTO renders as it did before per-task MAILTO values existed. +func TestCronTemplateWithoutMailtoGroups(t *testing.T) { + tasks := []CronTask{ + {Schedule: "@daily", AltCommand: "/bin/true", LogFile: "/var/log/dokku/log.log"}, + } + + tmpl, err := getCronTemplate() + if err != nil { + t.Fatalf("getCronTemplate() returned an error: %v", err) + } + + var got bytes.Buffer + if err := tmpl.Execute(&got, cronTemplateData(tasks, "", "")); err != nil { + t.Fatalf("Execute() returned an error: %v", err) + } + + want := strings.Join([]string{ + "PATH=/usr/local/bin:/usr/bin:/bin", + "SHELL=/bin/bash", + "", + "@daily /bin/true &>> /var/log/dokku/log.log", + "", + }, "\n") + if got.String() != want { + t.Errorf("rendered crontab =\n%s\nwant\n%s", got.String(), want) + } +} diff --git a/plugins/cron/templates/cron.tmpl b/plugins/cron/templates/cron.tmpl index 75577e6ee..8e6114fc3 100644 --- a/plugins/cron/templates/cron.tmpl +++ b/plugins/cron/templates/cron.tmpl @@ -10,3 +10,9 @@ SHELL=/bin/bash {{ range $task := .Tasks -}} {{ $task.Schedule }} {{ $task.DokkuRunCommand }} {{ end -}} +{{ range $group := .MailtoGroups }} +MAILTO={{ $group.Mailto }} +{{ range $task := $group.Tasks -}} +{{ $task.Schedule }} {{ $task.DokkuRunCommand }} +{{ end -}} +{{ end -}} diff --git a/tests/unit/cron-3.bats b/tests/unit/cron-3.bats index 4f830c620..e1c843940 100644 --- a/tests/unit/cron-3.bats +++ b/tests/unit/cron-3.bats @@ -104,6 +104,46 @@ teardown() { assert_failure } +@test "(cron) injected json entries" { + cat >/var/lib/dokku/plugins/enabled/cron-entries/cron-entries <<'EOF' +#!/usr/bin/env bash +[[ "$2" == "json" ]] || exit 0 +echo '@daily;/bin/true' +echo '{"schedule":"@hourly","command":"/bin/false","log-file":"/var/log/dokku/log.log","mailto":"ops@example.com"}' +EOF + chmod +x /var/lib/dokku/plugins/enabled/cron-entries/cron-entries + + run /bin/bash -c "dokku cron:set --global mailto global@example.com" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku plugin:trigger scheduler-cron-write" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "cat /var/spool/cron/crontabs/dokku" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "MAILTO=global@example.com" + assert_output_contains "@daily /bin/true" + assert_output_contains "MAILTO=ops@example.com" + assert_output_contains "@hourly /bin/false 2>&1 | tee -a /var/log/dokku/log.log" + + run /bin/bash -c "dokku cron:list --global --format json | jq -r '.[] | select(.schedule == \"@hourly\") | .mailto'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "ops@example.com" + + run /bin/bash -c "dokku cron:set --global mailto" + echo "output: $output" + echo "status: $status" + assert_success +} + @test "(cron) invalid [command]" { run deploy_app python dokku@$DOKKU_DOMAIN:$TEST_APP template_cron_file_injection echo "output: $output" From b158f482eff27867cf0d694dd2a5f74bca265877 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 30 Sep 2026 17:47:27 -0400 Subject: [PATCH 2/3] fix: reset MAILTO after a cron entry that sets its own A task with its own `MAILTO` is now written in place, followed by a line resetting `MAILTO` to the global value, or to the `dokku` user when none is set, rather than grouped at the end of the crontab, so no other task is mailed to its recipient whatever order the tasks are written in. --- docs/development/plugin-triggers.md | 2 +- docs/processes/scheduled-cron-tasks.md | 2 +- plugins/cron/crontab.go | 56 ++++++----------- plugins/cron/crontab_test.go | 85 +++++++++++++++++--------- plugins/cron/templates/cron.tmpl | 10 +-- tests/unit/cron-3.bats | 7 +++ 6 files changed, 89 insertions(+), 73 deletions(-) diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index d18833250..d101177e5 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -689,7 +689,7 @@ APP="$1"; PROPERTY="$2" - Description: Allows injecting cron tasks into the written out scheduled cron task list. Each entry is newline delimited, and individual tasks come in the form `$SCHEDULE;$FULL_COMMAND;$ARBITRARY_DATA`. Individual implementations of cron writing can decide whether and how to include these cron tasks. The `ARBITRARY_DATA` includes the log file path for the basic `docker-local` cron implementation. - When `$CRON_ENTRY_FORMAT` is `json`, each entry may instead be printed as a json object on its own line, with the keys `schedule`, `command`, `log-file` and `mailto`. Only `schedule` and `command` are required. A task with a `mailto` value is written to the crontab under a `MAILTO` of its own rather than the global `MAILTO`, and its output is mailed to that address in addition to being appended to its `log-file`. Json entries and `;` delimited entries may be printed together. Older versions of Dokku do not pass `$CRON_ENTRY_FORMAT`, and fail to write the crontab if a json entry is printed, so a json entry should only be printed when the argument is `json`. + When `$CRON_ENTRY_FORMAT` is `json`, each entry may instead be printed as a json object on its own line, with the keys `schedule`, `command`, `log-file` and `mailto`. Only `schedule` and `command` are required. A task with a `mailto` value is written to the crontab after a `MAILTO` line of its own and followed by one resetting `MAILTO` to the global value, or to the `dokku` user when no global value is set, so the value only applies to that task. Its output is mailed to that address in addition to being appended to its `log-file`. Json entries and `;` delimited entries may be printed together. Older versions of Dokku do not pass `$CRON_ENTRY_FORMAT`, and fail to write the crontab if a json entry is printed, so a json entry should only be printed when the argument is `json`. - Invoked by: `dokku cron:list --global`, and when the `dokku` user crontab is written - Arguments: `$DOKKU_SCHEDULER $CRON_ENTRY_FORMAT` - Example: diff --git a/docs/processes/scheduled-cron-tasks.md b/docs/processes/scheduled-cron-tasks.md index 977d8dada..86ab44d2d 100644 --- a/docs/processes/scheduled-cron-tasks.md +++ b/docs/processes/scheduled-cron-tasks.md @@ -56,7 +56,7 @@ When running scheduled cron tasks, there are a few items to be aware of: - At this time, only the `PATH` and `SHELL` environment variables are specified in the cron template. - A `MAILTO` value can be set via the `cron:set` command. - A `MAILFROM` value can be set via the `cron:set` command. - - Tasks injected by plugins via the [`cron-entries` trigger](/docs/development/plugin-triggers.md#cron-entries) may set a `MAILTO` value of their own, which is used for that task instead of the global value. The output of such a task is mailed to that address as well as appended to the task's log file. + - Tasks injected by plugins via the [`cron-entries` trigger](/docs/development/plugin-triggers.md#cron-entries) may set a `MAILTO` value of their own, which is used for that task instead of the global value. `MAILTO` is reset after the task to the global value, or to the `dokku` user when no global value is set, so no other task is mailed to that address. The output of such a task is mailed to that address as well as appended to the task's log file. - Each scheduled task is executed within a one-off `run` container, and thus inherit any docker-options specified for `run` containers. Resources are never shared between scheduled tasks. - Scheduled cron tasks are supported on a per-scheduler basis. Schedulers that use the host crontab - such as `docker-local` - have their `app.json` cron tasks written to the `dokku` user crontab, while schedulers that manage their own cron backend - such as `k3s` - schedule them natively. - Tasks for _all_ apps managed by a host-crontab scheduler such as `docker-local` are written to a single crontab file owned by the `dokku` user. The `dokku` user's crontab should be considered reserved for this purpose. diff --git a/plugins/cron/crontab.go b/plugins/cron/crontab.go index 42cd1c0b2..6d03e4b14 100644 --- a/plugins/cron/crontab.go +++ b/plugins/cron/crontab.go @@ -4,9 +4,7 @@ import ( _ "embed" "encoding/json" "fmt" - "maps" "os" - "slices" "strings" "text/template" @@ -19,6 +17,9 @@ import ( //go:embed templates/cron.tmpl var cronTemplate string +// crontabUser is the user whose crontab the host-cron tasks are written to +const crontabUser = "dokku" + // usesHostCron reports whether the given scheduler writes its cron tasks to the // host crontab (as opposed to managing its own cron backend). An empty scheduler // or an unimplemented scheduler-uses-host-cron trigger is treated as false. @@ -281,7 +282,7 @@ func writeCronTab() error { result, err := common.CallExecCommand(common.ExecCommandInput{ Command: "crontab", - Args: []string{"-u", "dokku", tmpFile.Name()}, + Args: []string{"-u", crontabUser, tmpFile.Name()}, }) if err != nil { return fmt.Errorf("Unable to update schedule file: %w", err) @@ -295,43 +296,22 @@ func writeCronTab() error { return nil } -// cronMailtoGroup is the set of tasks written to the crontab under a MAILTO of -// their own -type cronMailtoGroup struct { - // Mailto is the MAILTO value the tasks are written under - Mailto string - - // Tasks are the tasks written under the MAILTO value - Tasks []CronTask -} - -// cronTemplateData builds the data the cron template is executed with. Tasks -// that set their own MAILTO are grouped by it and written after every other -// task, so the global MAILTO applies to all of the tasks before them. +// cronTemplateData builds the data the cron template is executed with. A task +// that sets its own MAILTO is written between a MAILTO line of its own and one +// resetting it, so every task after it goes back to the global MAILTO. Without +// a global MAILTO it is reset to the crontab's owner, which is who cron mails +// when MAILTO is unset - an empty MAILTO would stop cron mailing anyone. func cronTemplateData(tasks []CronTask, mailfrom string, mailto string) map[string]interface{} { - globalTasks := []CronTask{} - groups := map[string][]CronTask{} - for _, task := range tasks { - if task.Mailto == "" { - globalTasks = append(globalTasks, task) - continue - } - groups[task.Mailto] = append(groups[task.Mailto], task) - } - - mailtoGroups := []cronMailtoGroup{} - for _, groupMailto := range slices.Sorted(maps.Keys(groups)) { - mailtoGroups = append(mailtoGroups, cronMailtoGroup{ - Mailto: groupMailto, - Tasks: groups[groupMailto], - }) + resetMailto := mailto + if resetMailto == "" { + resetMailto = crontabUser } return map[string]interface{}{ - "Tasks": globalTasks, - "MailtoGroups": mailtoGroups, - "Mailfrom": mailfrom, - "Mailto": mailto, + "Tasks": tasks, + "Mailfrom": mailfrom, + "Mailto": mailto, + "ResetMailto": resetMailto, } } @@ -339,7 +319,7 @@ func cronTemplateData(tasks []CronTask, mailfrom string, mailto string) map[stri func deleteCrontab() error { result, err := common.CallExecCommand(common.ExecCommandInput{ Command: "crontab", - Args: []string{"-l", "-u", "dokku"}, + Args: []string{"-l", "-u", crontabUser}, }) if err != nil || result.ExitCode != 0 { return nil @@ -347,7 +327,7 @@ func deleteCrontab() error { result, err = common.CallExecCommand(common.ExecCommandInput{ Command: "crontab", - Args: []string{"-r", "-u", "dokku"}, + Args: []string{"-r", "-u", crontabUser}, }) if err != nil { return fmt.Errorf("Unable to remove schedule file: %w", err) diff --git a/plugins/cron/crontab_test.go b/plugins/cron/crontab_test.go index ec02286a2..8949ec39c 100644 --- a/plugins/cron/crontab_test.go +++ b/plugins/cron/crontab_test.go @@ -153,16 +153,10 @@ func TestParseInjectedCronEntriesKeepsTextIDs(t *testing.T) { } } -// TestCronTemplateGroupsTasksByMailto pins that tasks with their own MAILTO are -// written after every task that uses the global MAILTO, grouped under a MAILTO -// line per recipient. -func TestCronTemplateGroupsTasksByMailto(t *testing.T) { - tasks := []CronTask{ - {Schedule: "@daily", AltCommand: "/bin/zeta", LogFile: "/var/log/dokku/zeta.log", Mailto: "zeta@example.com"}, - {Schedule: "@daily", AltCommand: "/bin/true", LogFile: "/var/log/dokku/log.log"}, - {Schedule: "@hourly", AltCommand: "/bin/alpha", LogFile: "/var/log/dokku/alpha.log", Mailto: "alpha@example.com"}, - {Schedule: "@weekly", AltCommand: "/bin/zeta-weekly", Mailto: "zeta@example.com"}, - } +// renderCronTemplate renders the cron template for the given tasks and global +// MAILTO +func renderCronTemplate(t *testing.T, tasks []CronTask, mailto string) string { + t.Helper() tmpl, err := getCronTemplate() if err != nil { @@ -170,46 +164,81 @@ func TestCronTemplateGroupsTasksByMailto(t *testing.T) { } var got bytes.Buffer - if err := tmpl.Execute(&got, cronTemplateData(tasks, "", "global@example.com")); err != nil { + if err := tmpl.Execute(&got, cronTemplateData(tasks, "", mailto)); err != nil { t.Fatalf("Execute() returned an error: %v", err) } + return got.String() +} + +// TestCronTemplateResetsMailtoAfterATaskWithItsOwn pins that a task with its own +// MAILTO is written in place between a MAILTO line of its own and one resetting +// it to the global MAILTO, so no task after it is mailed to its recipient. +func TestCronTemplateResetsMailtoAfterATaskWithItsOwn(t *testing.T) { + tasks := []CronTask{ + {Schedule: "@daily", AltCommand: "/bin/true", LogFile: "/var/log/dokku/log.log"}, + {Schedule: "@hourly", AltCommand: "/bin/alpha", LogFile: "/var/log/dokku/alpha.log", Mailto: "alpha@example.com"}, + {Schedule: "@daily", AltCommand: "/bin/false"}, + {Schedule: "@daily", AltCommand: "/bin/zeta", LogFile: "/var/log/dokku/zeta.log", Mailto: "zeta@example.com"}, + {Schedule: "@weekly", AltCommand: "/bin/zeta-weekly", Mailto: "zeta@example.com"}, + } + want := strings.Join([]string{ "MAILTO=global@example.com", "PATH=/usr/local/bin:/usr/bin:/bin", "SHELL=/bin/bash", "", "@daily /bin/true &>> /var/log/dokku/log.log", - "", "MAILTO=alpha@example.com", "@hourly /bin/alpha 2>&1 | tee -a /var/log/dokku/alpha.log", - "", + "MAILTO=global@example.com", + "@daily /bin/false", "MAILTO=zeta@example.com", "@daily /bin/zeta 2>&1 | tee -a /var/log/dokku/zeta.log", + "MAILTO=global@example.com", + "MAILTO=zeta@example.com", "@weekly /bin/zeta-weekly", + "MAILTO=global@example.com", "", }, "\n") - if got.String() != want { - t.Errorf("rendered crontab =\n%s\nwant\n%s", got.String(), want) + if got := renderCronTemplate(t, tasks, "global@example.com"); got != want { + t.Errorf("rendered crontab =\n%s\nwant\n%s", got, want) } } -// TestCronTemplateWithoutMailtoGroups pins that a crontab without tasks that set +// TestCronTemplateResetsMailtoToTheCrontabOwner pins that without a global +// MAILTO, a task with its own is followed by a MAILTO naming the crontab's +// owner, who cron mails when MAILTO is unset. An empty MAILTO would stop cron +// mailing anyone about the tasks after it. +func TestCronTemplateResetsMailtoToTheCrontabOwner(t *testing.T) { + tasks := []CronTask{ + {Schedule: "@hourly", AltCommand: "/bin/alpha", LogFile: "/var/log/dokku/alpha.log", Mailto: "alpha@example.com"}, + {Schedule: "@daily", AltCommand: "/bin/true", LogFile: "/var/log/dokku/log.log"}, + } + + want := strings.Join([]string{ + "PATH=/usr/local/bin:/usr/bin:/bin", + "SHELL=/bin/bash", + "", + "MAILTO=alpha@example.com", + "@hourly /bin/alpha 2>&1 | tee -a /var/log/dokku/alpha.log", + "MAILTO=dokku", + "@daily /bin/true &>> /var/log/dokku/log.log", + "", + }, "\n") + if got := renderCronTemplate(t, tasks, ""); got != want { + t.Errorf("rendered crontab =\n%s\nwant\n%s", got, want) + } +} + +// TestCronTemplateWithoutTaskMailto pins that a crontab without tasks that set // their own MAILTO renders as it did before per-task MAILTO values existed. -func TestCronTemplateWithoutMailtoGroups(t *testing.T) { +func TestCronTemplateWithoutTaskMailto(t *testing.T) { tasks := []CronTask{ {Schedule: "@daily", AltCommand: "/bin/true", LogFile: "/var/log/dokku/log.log"}, } - tmpl, err := getCronTemplate() - if err != nil { - t.Fatalf("getCronTemplate() returned an error: %v", err) - } - - var got bytes.Buffer - if err := tmpl.Execute(&got, cronTemplateData(tasks, "", "")); err != nil { - t.Fatalf("Execute() returned an error: %v", err) - } + got := renderCronTemplate(t, tasks, "") want := strings.Join([]string{ "PATH=/usr/local/bin:/usr/bin:/bin", @@ -218,7 +247,7 @@ func TestCronTemplateWithoutMailtoGroups(t *testing.T) { "@daily /bin/true &>> /var/log/dokku/log.log", "", }, "\n") - if got.String() != want { - t.Errorf("rendered crontab =\n%s\nwant\n%s", got.String(), want) + if got != want { + t.Errorf("rendered crontab =\n%s\nwant\n%s", got, want) } } diff --git a/plugins/cron/templates/cron.tmpl b/plugins/cron/templates/cron.tmpl index 8e6114fc3..34648f3e6 100644 --- a/plugins/cron/templates/cron.tmpl +++ b/plugins/cron/templates/cron.tmpl @@ -8,11 +8,11 @@ PATH=/usr/local/bin:/usr/bin:/bin SHELL=/bin/bash {{ range $task := .Tasks -}} +{{ if $task.Mailto -}} +MAILTO={{ $task.Mailto }} +{{ end -}} {{ $task.Schedule }} {{ $task.DokkuRunCommand }} -{{ end -}} -{{ range $group := .MailtoGroups }} -MAILTO={{ $group.Mailto }} -{{ range $task := $group.Tasks -}} -{{ $task.Schedule }} {{ $task.DokkuRunCommand }} +{{ if $task.Mailto -}} +MAILTO={{ $.ResetMailto }} {{ end -}} {{ end -}} diff --git a/tests/unit/cron-3.bats b/tests/unit/cron-3.bats index e1c843940..843f71af9 100644 --- a/tests/unit/cron-3.bats +++ b/tests/unit/cron-3.bats @@ -132,6 +132,13 @@ EOF assert_output_contains "MAILTO=ops@example.com" assert_output_contains "@hourly /bin/false 2>&1 | tee -a /var/log/dokku/log.log" + # the global MAILTO is restored for every task after the one with its own + run /bin/bash -c "grep -A1 -F 'tee -a /var/log/dokku/log.log' /var/spool/cron/crontabs/dokku | tail -n 1" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "MAILTO=global@example.com" + run /bin/bash -c "dokku cron:list --global --format json | jq -r '.[] | select(.schedule == \"@hourly\") | .mailto'" echo "output: $output" echo "status: $status" From c776fbc7a45c80c3d80609c92e37c04d98fda2e8 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 30 Sep 2026 21:40:31 -0400 Subject: [PATCH 3/3] test: expect the global MAILTO reset in cron-3 --- tests/unit/cron-3.bats | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/cron-3.bats b/tests/unit/cron-3.bats index 843f71af9..f20f2e72a 100644 --- a/tests/unit/cron-3.bats +++ b/tests/unit/cron-3.bats @@ -127,7 +127,8 @@ EOF echo "output: $output" echo "status: $status" assert_success - assert_output_contains "MAILTO=global@example.com" + # once in the header, and once to reset it after the task with its own + assert_output_contains "MAILTO=global@example.com" 2 assert_output_contains "@daily /bin/true" assert_output_contains "MAILTO=ops@example.com" assert_output_contains "@hourly /bin/false 2>&1 | tee -a /var/log/dokku/log.log"