From 78b8b6e2560f7304180f431a204ac77fdb04fced Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 6 Sep 2026 14:08:51 +0200 Subject: [PATCH] catalogue: carry and validate a container schedule (ADR 0053) A container may declare schedule: "", the recurring twin of run-once. The resolver already carries a resource's keys through untouched, so schedule reaches the rendered host declaration on its own; what belongs here is refusing, near its author, what the host would otherwise refuse far away. The manifest parser refuses a schedule that is not a string, one that is not a well-formed five-field cron (cron.go: fields, ranges, *, comma, dash, slash), and the contradictory pair run-once + schedule -- a container runs once and gates, or on a cadence, or stays up, never two. Claude-Session: https://claude.ai/code/session_01LrgweAeERJYBg88c5cKDzF --- internal/catalogue/cron.go | 92 +++++++++++++++++++++++++++++ internal/catalogue/manifest.go | 59 +++++++++++++----- internal/catalogue/schedule_test.go | 89 ++++++++++++++++++++++++++++ 3 files changed, 224 insertions(+), 16 deletions(-) create mode 100644 internal/catalogue/cron.go create mode 100644 internal/catalogue/schedule_test.go diff --git a/internal/catalogue/cron.go b/internal/catalogue/cron.go new file mode 100644 index 0000000..fbb6d29 --- /dev/null +++ b/internal/catalogue/cron.go @@ -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 +} diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index fdee7a5..7a767d6 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -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"])) + } } } } diff --git a/internal/catalogue/schedule_test.go b/internal/catalogue/schedule_test.go new file mode 100644 index 0000000..289fb23 --- /dev/null +++ b/internal/catalogue/schedule_test.go @@ -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) + } +}