diff --git a/docs/src/docs/operations.md b/docs/src/docs/operations.md index ae41a0db..eb15e75e 100644 --- a/docs/src/docs/operations.md +++ b/docs/src/docs/operations.md @@ -40,6 +40,25 @@ Backrest provides flexible scheduling options for all operations through policie | Interval Days | Run every N days | Regular daily+ intervals | | Interval Hours | Run every N hours | Regular sub-daily intervals | +### Cron Expression Syntax + +Cron schedules accept the standard five fields (minute, hour, day of month, month, day of week) with the usual `*`, `,`, `-`, and `/` operators. Day of week accepts `0`-`7` (both `0` and `7` mean Sunday) or the names `SUN`-`SAT`. + +The following extensions are also supported: + +| Syntax | Meaning | Example | +| ----------- | ------------------------------------------ | ------------------------------------------------- | +| `#` | Nth weekday of the month | `0 0 * * 1#1` runs on the first Monday | +| `L` | Last day of the month | `0 0 L * *` runs on the last day of each month | +| `L` | Last given weekday of the month | `0 0 * * 5L` runs on the last Friday | +| `W` | Nearest weekday to a day of the month | `0 0 15W * *` runs on the weekday nearest the 15th | +| `?` | Same as `*` | `0 0 * * ?` | +| `@` aliases | `@yearly`, `@monthly`, `@weekly`, `@daily`, `@hourly` | `@daily` runs at midnight | + +Longer expressions are also accepted. Six fields adds a trailing year (`minute hour day-of-month month day-of-week year`), and seven fields adds both a leading seconds field and a trailing year (`second minute hour day-of-month month day-of-week year`). + +Expressions that are syntactically valid but can never match a real date, such as `0 0 30 2 *`, are rejected when the configuration is saved rather than silently never running. + ### Schedule Clocks | Clock | Description | Best For | diff --git a/go.mod b/go.mod index ca093b22..192789c2 100644 --- a/go.mod +++ b/go.mod @@ -8,12 +8,12 @@ require ( fyne.io/systray v1.12.2 github.com/djherbis/buffer v1.2.0 github.com/djherbis/nio/v3 v3.0.1 - github.com/gitploy-io/cronexpr v0.2.2 github.com/gofrs/flock v0.13.0 github.com/golang-jwt/jwt/v5 v5.3.1 github.com/google/go-cmp v0.7.0 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/google/uuid v1.6.0 + github.com/hashicorp/cronexpr v1.1.3 github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/golang-lru/v2 v2.0.7 github.com/hectane/go-acl v1.0.0 diff --git a/go.sum b/go.sum index 7b8478a4..eab359fb 100644 --- a/go.sum +++ b/go.sum @@ -27,8 +27,6 @@ github.com/djherbis/nio/v3 v3.0.1 h1:6wxhnuppteMa6RHA4L81Dq7ThkZH8SwnDzXDYy95vB4 github.com/djherbis/nio/v3 v3.0.1/go.mod h1:Ng4h80pbZFMla1yKzm61cF0tqqilXZYrogmWgZxOcmg= github.com/eclipse/paho.golang v0.23.0 h1:KHgl2wz6EJo7cMBmkuhpt7C576vP+kpPv7jjvSyR6Mk= github.com/eclipse/paho.golang v0.23.0/go.mod h1:nQRhTkoZv8EAiNs5UU0/WdQIx2NrnWUpL9nsGJTQN04= -github.com/gitploy-io/cronexpr v0.2.2 h1:Au+wK6FqmOLAF7AkW6q4gnrNXTe3rEW97XFZ4chy0xs= -github.com/gitploy-io/cronexpr v0.2.2/go.mod h1:Uep5sbzUSocMZvJ1s0lNI9zi37s5iUI1llkw3vRGK9M= github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= @@ -53,6 +51,8 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= +github.com/hashicorp/cronexpr v1.1.3 h1:rl5IkxXN2m681EfivTlccqIryzYJSXRGRNa0xeG7NA4= +github.com/hashicorp/cronexpr v1.1.3/go.mod h1:P4wA0KBl9C5q2hABiMO7cp6jcIg96CDh1Efb3g1PWA4= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I= github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= diff --git a/internal/orchestrator/tasks/scheduling_test.go b/internal/orchestrator/tasks/scheduling_test.go index 743d782c..f78fb6de 100644 --- a/internal/orchestrator/tasks/scheduling_test.go +++ b/internal/orchestrator/tasks/scheduling_test.go @@ -255,7 +255,10 @@ func TestScheduling(t *testing.T) { UnixTimeEndMs: farFuture.UnixMilli(), }, }, - wantTime: mustParseTime(t, "1970-01-02T08:00:00Z"), + // now is 1970-01-02T03:46:40Z, so the next UTC midnight is the 3rd. + // This must differ from the CLOCK_LOCAL case above: under TZ=America/Los_Angeles + // a UTC-clock schedule fires on UTC wall-clock, not local wall-clock. + wantTime: mustParseTime(t, "1970-01-03T00:00:00Z"), }, { name: "backup schedule cron since last run", diff --git a/internal/protoutil/schedule.go b/internal/protoutil/schedule.go index da4ffaa5..451ccf53 100644 --- a/internal/protoutil/schedule.go +++ b/internal/protoutil/schedule.go @@ -6,7 +6,7 @@ import ( "time" v1 "github.com/garethgeorge/backrest/gen/go/v1" - "github.com/gitploy-io/cronexpr" + "github.com/hashicorp/cronexpr" ) var ErrScheduleDisabled = errors.New("never") @@ -51,14 +51,18 @@ func ResolveSchedule(sched *v1.Schedule, lastRan time.Time, curTime time.Time) ( case *v1.Schedule_MaxFrequencyHours: return t.Add(time.Duration(s.MaxFrequencyHours) * time.Hour), nil case *v1.Schedule_Cron: - cron, err := cronexpr.ParseInLocation(s.Cron, time.Now().Location().String()) + cron, err := cronexpr.Parse(s.Cron) if err != nil { return time.Time{}, fmt.Errorf("parse cron %q: %w", s.Cron, err) } - if cron.Next(t).IsZero() || cron.Next(t).Before(t) { + // Next evaluates the expression against t's own location, so the clock + // selection above (local / UTC / last run time) determines the zone the + // cron's wall-clock fields are interpreted in. + next := cron.Next(t) + if next.IsZero() || next.Before(t) { return time.Time{}, fmt.Errorf("cron %q may be malformed, next scheduled time is in the past %v", s.Cron, t) } - return cron.Next(t), nil + return next, nil default: return time.Time{}, fmt.Errorf("unknown schedule type: %T", s) } @@ -80,7 +84,7 @@ func NominalPeriod(sched *v1.Schedule, from time.Time) (time.Duration, error) { case *v1.Schedule_MaxFrequencyHours: return time.Duration(s.MaxFrequencyHours) * time.Hour, nil case *v1.Schedule_Cron: - cron, err := cronexpr.ParseInLocation(s.Cron, from.Location().String()) + cron, err := cronexpr.Parse(s.Cron) if err != nil { return 0, fmt.Errorf("parse cron %q: %w", s.Cron, err) } @@ -116,12 +120,15 @@ func ValidateSchedule(sched *v1.Schedule) error { if s.Cron == "" { return errors.New("empty cron expression") } - cron, err := cronexpr.ParseInLocation(s.Cron, time.Now().Location().String()) + cron, err := cronexpr.Parse(s.Cron) if err != nil { return fmt.Errorf("invalid cron %q: %w", s.Cron, err) } + // A syntactically valid expression can still describe a date that never + // occurs (e.g. "0 0 30 2 *"); Next reports that as the zero time rather + // than an error, and such a schedule would silently never run. if next := cron.Next(time.Now()); next.IsZero() || next.Year() < 2000 { - return fmt.Errorf("invalid cron %q: next scheduled time is invalid (check for DOW=7 usage)", s.Cron) + return fmt.Errorf("invalid cron %q: expression never matches a real date", s.Cron) } case nil: return nil diff --git a/internal/protoutil/schedule_test.go b/internal/protoutil/schedule_test.go index 48e90f00..b49d0e14 100644 --- a/internal/protoutil/schedule_test.go +++ b/internal/protoutil/schedule_test.go @@ -11,8 +11,8 @@ import ( ) func TestResolveSchedule(t *testing.T) { - // Use Local time to match ResolveSchedule's ParseInLocation(time.Now().Location()) - // ensuring cron expression and reference time are in same zone. + // Cron expressions are evaluated in the reference time's own zone, so use + // Local here to match the CLOCK_LOCAL cases below. now := time.Date(2023, 10, 1, 10, 0, 0, 0, time.Local) // Sunday, Oct 1st 2023 10:00 Local tests := []struct { @@ -62,6 +62,31 @@ func TestResolveSchedule(t *testing.T) { // Let's assume next slot is next week. expected: now.Add(7 * 24 * time.Hour), }, + { + name: "Cron - First Monday of the month (#)", + schedule: &v1.Schedule{ + Clock: v1.Schedule_CLOCK_LOCAL, + Schedule: &v1.Schedule_Cron{ + Cron: "0 0 * * 1#1", + }, + }, + lastRan: now, + curTime: now, + // Oct 1st 2023 is a Sunday, so the first Monday is Oct 2nd. + expected: time.Date(2023, 10, 2, 0, 0, 0, 0, time.Local), + }, + { + name: "Cron - Last day of the month (L)", + schedule: &v1.Schedule{ + Clock: v1.Schedule_CLOCK_LOCAL, + Schedule: &v1.Schedule_Cron{ + Cron: "0 0 L * *", + }, + }, + lastRan: now, + curTime: now, + expected: time.Date(2023, 10, 31, 0, 0, 0, 0, time.Local), + }, } for _, tt := range tests { @@ -92,12 +117,11 @@ func TestValidateSchedule(t *testing.T) { expectError: false, }, { - name: "Invalid Cron (7) - Validation Error", + name: "Valid Cron (7) - Sunday, same as 0", schedule: &v1.Schedule{ Schedule: &v1.Schedule_Cron{Cron: "0 10 * * 7"}, }, - expectError: true, - errorContains: "check for DOW=7 usage", + expectError: false, }, { name: "Valid Frequency", @@ -106,6 +130,68 @@ func TestValidateSchedule(t *testing.T) { }, expectError: false, }, + // Extended (Quartz-style) syntax. cronstrue renders these in the WebUI, so + // the backend must accept everything the schedule form is willing to show. + { + name: "Nth weekday of month (#)", + schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * 1#1"}}, + expectError: false, + }, + { + name: "Nth weekday of month by name", + schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * MON#1"}}, + expectError: false, + }, + { + name: "Last weekday of month (L)", + schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * 5L"}}, + expectError: false, + }, + { + name: "Last day of month (L)", + schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 L * *"}}, + expectError: false, + }, + { + name: "Nearest weekday (W)", + schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 15W * *"}}, + expectError: false, + }, + { + name: "Descriptor", + schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "@daily"}}, + expectError: false, + }, + // Day-of-week ranges ending in 7 parse but never fire under the upstream + // (archived) gorhill parser; the hashicorp fork resolves 7 to Sunday. + { + name: "Day of week range ending in 7", + schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * 1-7"}}, + expectError: false, + }, + { + name: "Day of week range MON-SUN", + schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * MON-SUN"}}, + expectError: false, + }, + { + name: "Nth weekday out of range", + schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * 1#6"}}, + expectError: true, + errorContains: "invalid cron", + }, + { + name: "Expression that never matches a real date", + schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 30 2 *"}}, + expectError: true, + errorContains: "never matches a real date", + }, + { + name: "Empty cron", + schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: ""}}, + expectError: true, + errorContains: "empty cron expression", + }, } for _, tt := range tests { @@ -123,6 +209,37 @@ func TestValidateSchedule(t *testing.T) { } } +// TestResolveScheduleClockZone pins down which zone a cron expression's +// wall-clock fields are interpreted in. The parser evaluates against the +// reference time's own location, and ResolveSchedule picks that location from +// the schedule's clock, so CLOCK_UTC fires on UTC wall-clock rather than the +// host's local wall-clock. +func TestResolveScheduleClockZone(t *testing.T) { + curTime := time.Date(2023, 10, 1, 10, 0, 0, 0, time.UTC) + + tests := []struct { + name string + clock v1.Schedule_Clock + zone *time.Location + }{ + {name: "UTC clock fires on UTC wall-clock", clock: v1.Schedule_CLOCK_UTC, zone: time.UTC}, + {name: "Local clock fires on local wall-clock", clock: v1.Schedule_CLOCK_LOCAL, zone: time.Local}, + {name: "Default clock fires on local wall-clock", clock: v1.Schedule_CLOCK_DEFAULT, zone: time.Local}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := protoutil.ResolveSchedule(&v1.Schedule{ + Clock: tt.clock, + Schedule: &v1.Schedule_Cron{Cron: "0 3 * * *"}, // 03:00 daily + }, curTime, curTime) + require.NoError(t, err) + assert.Equal(t, 3, got.In(tt.zone).Hour(), "expected 03:00 in %v, got %v", tt.zone, got) + assert.Equal(t, 0, got.In(tt.zone).Minute()) + }) + } +} + func transactionalTimeEqual(t1, t2 time.Time) bool { return t1.Equal(t2) } @@ -163,6 +280,12 @@ func TestNominalPeriod(t *testing.T) { schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 9 * * 1-5"}}, expected: 72 * time.Hour, }, + { + name: "Cron first Monday of month uses widest gap", + // First Mondays sit 28 or 35 days apart depending on the month. + schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * 1#1"}}, + expected: 35 * 24 * time.Hour, + }, { name: "Disabled", schedule: &v1.Schedule{Schedule: &v1.Schedule_Disabled{Disabled: true}}, @@ -192,3 +315,24 @@ func TestNominalPeriod(t *testing.T) { }) } } + +// FuzzValidateSchedule guards the cron parser against panics on arbitrary +// expressions. Schedules come from the config file, so a panic here would take +// down the server rather than surface as a validation error. +func FuzzValidateSchedule(f *testing.F) { + for _, seed := range []string{ + "* * * * *", "0 0 * * 1#1", "0 0 L * *", "0 0 15W * *", "@daily", + "0 0 * * 1-7", "not a cron", "", "* * * * * * *", "0 0 30 2 *", + } { + f.Add(seed) + } + f.Fuzz(func(t *testing.T, cron string) { + sched := &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: cron}} + if err := protoutil.ValidateSchedule(sched); err != nil { + return + } + // Anything that validates must also resolve and report a period without panicking. + _, _ = protoutil.ResolveSchedule(sched, time.Now(), time.Now()) + _, _ = protoutil.NominalPeriod(sched, time.Now()) + }) +}