schedule: parse + validate a cron on a container, refuse run-once+schedule (ADR 0053) #11

Merged
jschoubben merged 1 commits from feat/schedule-container into main 2026-09-06 12:27:42 +00:00
3 changed files with 224 additions and 16 deletions
Showing only changes of commit 78b8b6e256 - Show all commits
+92
View File
@@ -0,0 +1,92 @@
package catalogue
// A five-field cron validator, enough to refuse a malformed schedule near its author
// (novox/hq ADR 0053).
//
// **Validation only, and written rather than pulled in.** The control plane's part in a scheduled
// step is small and exact: carry the field to the host unchanged, and refuse a `schedule` that is
// not a well-formed cron here rather than on the machine — the same near-versus-far discipline the
// manifest keeps for an action a module may not declare and a run-once that is not a boolean. What
// *when to run it again* means is the host's to evaluate; the control plane only checks the string
// is one it could. A cron library would be a dependency carried for evaluation nobody does here.
//
// The ordinary five fields — minute, hour, day-of-month, month, day-of-week — with `*`, lists
// (`,`), ranges (`-`) and steps (`/`). No names (jan, mon): a numeric cron is what the host
// evaluates, and refusing a name here is refusing it near whoever wrote it.
import (
"fmt"
"strconv"
"strings"
)
// validateCron reports why a string is not a five-field cron expression, or nil if it is one.
func validateCron(expr string) error {
fields := strings.Fields(expr)
if len(fields) != 5 {
return fmt.Errorf(
"a schedule is a five-field cron expression (minute hour day-of-month month "+
"day-of-week), and this has %d field(s)", len(fields))
}
bounds := []struct {
name string
min, max int
}{
{"minute", 0, 59},
{"hour", 0, 23},
{"day-of-month", 1, 31},
{"month", 1, 12},
{"day-of-week", 0, 7}, // 0 and 7 are both Sunday, the convention every cron keeps
}
for i, b := range bounds {
if err := validateCronField(fields[i], b.min, b.max); err != nil {
return fmt.Errorf("%s field: %w", b.name, err)
}
}
return nil
}
// validateCronField checks one field's elements are within range and well-formed.
func validateCronField(spec string, min, max int) error {
if spec == "" {
return fmt.Errorf("is empty")
}
for _, part := range strings.Split(spec, ",") {
if part == "" {
return fmt.Errorf("%q has an empty element between commas", spec)
}
rangePart := part
if slash := strings.IndexByte(part, '/'); slash >= 0 {
rangePart = part[:slash]
n, err := strconv.Atoi(part[slash+1:])
if err != nil || n < 1 {
return fmt.Errorf("step in %q is not a positive number", part)
}
}
switch {
case rangePart == "*":
// Any value; nothing to bound.
case strings.IndexByte(rangePart, '-') >= 0:
dash := strings.IndexByte(rangePart, '-')
lo, errLo := strconv.Atoi(rangePart[:dash])
hi, errHi := strconv.Atoi(rangePart[dash+1:])
if errLo != nil || errHi != nil {
return fmt.Errorf("range %q is not two numbers", rangePart)
}
if lo < min || hi > max || lo > hi {
return fmt.Errorf("range %q is outside the allowed %d-%d", part, min, max)
}
default:
v, err := strconv.Atoi(rangePart)
if err != nil {
return fmt.Errorf("%q is not a number", rangePart)
}
if v < min || v > max {
return fmt.Errorf("%q is outside the allowed range %d-%d", part, min, max)
}
}
}
return nil
}
+43 -16
View File
@@ -734,23 +734,50 @@ func ParseManifest(raw []byte) (Manifest, error) {
if fmt.Sprint(r["type"]) != "container" {
continue
}
raw, present := r["run-once"]
if !present {
continue
}
once, ok := raw.(bool)
if !ok {
problems = append(problems, fmt.Sprintf(
"%s declares run-once on %v as a %T; run-once is true or false",
m.Module, r["id"], raw))
continue
}
if once {
if _, hasRestart := r["restart-on"]; hasRestart {
var runOnce bool
if raw, present := r["run-once"]; present {
once, ok := raw.(bool)
if !ok {
problems = append(problems, fmt.Sprintf(
"%s declares %v as run-once and with restart-on; a run-once step runs to "+
"completion rather than staying running to be restarted (novox/hq ADR 0052)",
m.Module, r["id"]))
"%s declares run-once on %v as a %T; run-once is true or false",
m.Module, r["id"], raw))
} else {
runOnce = once
if once {
if _, hasRestart := r["restart-on"]; hasRestart {
problems = append(problems, fmt.Sprintf(
"%s declares %v as run-once and with restart-on; a run-once step runs to "+
"completion rather than staying running to be restarted (novox/hq ADR 0052)",
m.Module, r["id"]))
}
}
}
}
// **A scheduled container runs on a recurring cadence** (novox/hq ADR 0053), the recurring
// twin of run-once. The control plane carries the field to the host unchanged (the resolver
// copies every key of a resource, so `schedule` reaches the rendered declaration on its own);
// what belongs here is refusing, near its author, a value the host would only refuse far away.
// Three things: a value that is not a string, a string that is not a valid cron, and the pair
// run-once + schedule — a container runs once, or on a cadence, or stays up, never two.
if raw, present := r["schedule"]; present {
cron, ok := raw.(string)
switch {
case !ok:
problems = append(problems, fmt.Sprintf(
"%s declares schedule on %v as a %T; a schedule is a five-field cron string",
m.Module, r["id"], raw))
default:
if err := validateCron(cron); err != nil {
problems = append(problems, fmt.Sprintf(
"%s declares schedule %q on %v, which is not a valid cron: %v",
m.Module, cron, r["id"], err))
}
if runOnce {
problems = append(problems, fmt.Sprintf(
"%s declares %v as both run-once and schedule; a container runs once and "+
"gates, or on a cadence, or stays up — never two (novox/hq ADR 0053)",
m.Module, r["id"]))
}
}
}
}
+89
View File
@@ -0,0 +1,89 @@
package catalogue
import (
"strings"
"testing"
)
// A scheduled container is the recurring twin of run-once (novox/hq ADR 0053). The control plane's
// part is small and exact: carry the field to the host unchanged, and refuse a malformed schedule —
// or the contradictory pair run-once + schedule — near its author rather than on the machine. These
// tests defend that.
func TestAScheduledContainerRendersWithTheCronField(t *testing.T) {
// The resolver carries `schedule` into the rendered host declaration untouched, exactly as it
// carries run-once — a container's keys are copied through, so the host receives the field as
// written.
digest := "@sha256:" + strings.Repeat("a", 64)
r := Resolution{Node: "laptop", Modules: []Manifest{{
Module: "kometa",
Resources: []map[string]any{
{"id": "sync", "type": "container", "name": "sync",
"image": "registry.example/runtime" + digest, "schedule": "0 3 * * *"},
},
}}}
out, err := r.Declaration(Rendering{})
if err != nil {
t.Fatal(err)
}
sync := indexOfID(out, "kometa.sync")
if sync == -1 {
t.Fatal("the scheduled container was lost in rendering")
}
if got, _ := out[sync]["schedule"].(string); got != "0 3 * * *" {
t.Errorf("schedule did not reach the host declaration: %+v", out[sync])
}
}
func TestAMalformedScheduleIsRefused(t *testing.T) {
digest := "@sha256:" + strings.Repeat("a", 64)
bad := []byte(`{"module":"m","resources":[
{"id":"sync","type":"container","name":"sync","image":"registry.example/x` + digest + `","schedule":"every night"}
]}`)
if _, err := ParseManifest(bad); err == nil {
t.Error("a malformed schedule was accepted")
} else if !strings.Contains(err.Error(), "cron") {
t.Errorf("refused for the wrong reason: %v", err)
}
// A schedule out of range is refused for the same reason, near its author.
outOfRange := []byte(`{"module":"m","resources":[
{"id":"sync","type":"container","name":"sync","image":"registry.example/x` + digest + `","schedule":"0 25 * * *"}
]}`)
if _, err := ParseManifest(outOfRange); err == nil {
t.Error("a schedule with an hour out of range was accepted")
}
good := []byte(`{"module":"m","resources":[
{"id":"sync","type":"container","name":"sync","image":"registry.example/x` + digest + `","schedule":"*/15 * * * *"}
]}`)
if _, err := ParseManifest(good); err != nil {
t.Errorf("a valid scheduled container was refused: %v", err)
}
}
func TestAScheduleThatIsNotAStringIsRefused(t *testing.T) {
digest := "@sha256:" + strings.Repeat("a", 64)
bad := []byte(`{"module":"m","resources":[
{"id":"sync","type":"container","name":"sync","image":"registry.example/x` + digest + `","schedule":true}
]}`)
if _, err := ParseManifest(bad); err == nil {
t.Error("a schedule that is not a string was accepted")
} else if !strings.Contains(err.Error(), "schedule") {
t.Errorf("refused for the wrong reason: %v", err)
}
}
func TestAContainerCannotBeBothRunOnceAndScheduled(t *testing.T) {
// A container runs once and gates, or on a cadence, or stays up — never two (novox/hq ADR 0053).
digest := "@sha256:" + strings.Repeat("a", 64)
bad := []byte(`{"module":"m","resources":[
{"id":"sync","type":"container","name":"sync","image":"registry.example/x` + digest + `","run-once":true,"schedule":"0 3 * * *"}
]}`)
if _, err := ParseManifest(bad); err == nil {
t.Error("a container that was both run-once and scheduled was accepted")
} else if !strings.Contains(err.Error(), "both run-once and schedule") {
t.Errorf("refused for the wrong reason: %v", err)
}
}