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

@@ -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:

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.

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
+// cronTemplateData builds the data the cron template is executed with. A task
-// their own
+// that sets its own MAILTO is written between a MAILTO line of its own and one
-type cronMailtoGroup struct {
+// resetting it, so every task after it goes back to the global MAILTO. Without
-	// Mailto is the MAILTO value the tasks are written under
+// a global MAILTO it is reset to the crontab's owner, which is who cron mails
-	Mailto string
+// when MAILTO is unset - an empty MAILTO would stop cron mailing anyone.
-
-	// 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{}
+	resetMailto := mailto
-	groups := map[string][]CronTask{}
+	if resetMailto == "" {
-	for _, task := range tasks {
+		resetMailto = crontabUser
-		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,
+		"Tasks":       tasks,
-		"MailtoGroups": mailtoGroups,
+		"Mailfrom":    mailfrom,
-		"Mailfrom":     mailfrom,
+		"Mailto":      mailto,
-		"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)

plugins/cron/crontab_test.go

@@ -153,16 +153,10 @@ func TestParseInjectedCronEntriesKeepsTextIDs(t *testing.T) {
 	}
 }
 
-// TestCronTemplateGroupsTasksByMailto pins that tasks with their own MAILTO are
+// renderCronTemplate renders the cron template for the given tasks and global
-// written after every task that uses the global MAILTO, grouped under a MAILTO
+// MAILTO
-// line per recipient.
+func renderCronTemplate(t *testing.T, tasks []CronTask, mailto string) string {
-func TestCronTemplateGroupsTasksByMailto(t *testing.T) {
+	t.Helper()
-	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 {
@@ -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 {
+	if got := renderCronTemplate(t, tasks, "global@example.com"); got != want {
-		t.Errorf("rendered crontab =\n%s\nwant\n%s", got.String(), 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()
+	got := renderCronTemplate(t, tasks, "")
-	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",
@@ -218,7 +247,7 @@ func TestCronTemplateWithoutMailtoGroups(t *testing.T) {
 		"@daily /bin/true &>> /var/log/dokku/log.log",
 		"",
 	}, "\n")
-	if got.String() != want {
+	if got != want {
-		t.Errorf("rendered crontab =\n%s\nwant\n%s", got.String(), want)
+		t.Errorf("rendered crontab =\n%s\nwant\n%s", got, want)
 	}
 }

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 -}}
+{{ if $task.Mailto -}}
-{{ range $group := .MailtoGroups }}
+MAILTO={{ $.ResetMailto }}
-MAILTO={{ $group.Mailto }}
-{{ range $task := $group.Tasks -}}
-{{ $task.Schedule }} {{ $task.DokkuRunCommand }}
 {{ end -}}
 {{ end -}}

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"