jobs,backup,vendor: switch from gorhill/cronexpr to robfig/cron

gorhill/cronexpr is abandoned, and we recently found a known panic that was
crashing nodes that upstream considered known behavior when an invalid expression
was provided. While robfig/cron is a full scheduled job execution framework, we can
use its parser separately as well, which is what this change does.

Release note (bug fix): Certain malformed backup schedule expressions no longer cause the node to crash.
Release note (backward-incompatible change): Non-standard cron expressions that specify seconds or year fields are no longer supported.

DEPS.bzl

@@ -3403,16 +3403,6 @@ def go_deps():
             "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/gordonklaus/ineffassign/com_github_gordonklaus_ineffassign-v0.0.0-20200309095847-7953dde2c7bf.zip",
         ],
     )
-    go_repository(
-        name = "com_github_gorhill_cronexpr",
-        build_file_proto_mode = "disable_global",
-        importpath = "github.com/gorhill/cronexpr",
-        sha256 = "742d8957d3f9fe773150fb3164868a755b2af5b705b38c72c45ca5386715c617",
-        strip_prefix = "github.com/gorhill/[email protected]",
-        urls = [
-            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/gorhill/cronexpr/com_github_gorhill_cronexpr-v0.0.0-20180427100037-88b0669f7d75.zip",
-        ],
-    )
     go_repository(
         name = "com_github_gorilla_context",
         build_file_proto_mode = "disable_global",
@@ -6444,6 +6434,16 @@ def go_deps():
             "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/retailnext/hllpp/com_github_retailnext_hllpp-v1.0.1-0.20180308014038-101a6d2f8b52.zip",
         ],
     )
+    go_repository(
+        name = "com_github_robfig_cron_v3",
+        build_file_proto_mode = "disable_global",
+        importpath = "github.com/robfig/cron/v3",
+        sha256 = "ebe6454642220832a451b8cc50eae5f9150fd8d36b90b242a5de27676be86c70",
+        strip_prefix = "github.com/robfig/cron/[email protected]",
+        urls = [
+            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/robfig/cron/v3/com_github_robfig_cron_v3-v3.0.1.zip",
+        ],
+    )
     go_repository(
         name = "com_github_rogpeppe_fastuuid",
         build_file_proto_mode = "disable_global",

go.mod

@@ -78,7 +78,6 @@ require (
 	github.com/google/go-github v17.0.0+incompatible
 	github.com/google/pprof v0.0.0-20210827144239-02619b876842
 	github.com/google/skylark v0.0.0-20181101142754-a5f7082aabed
-	github.com/gorhill/cronexpr v0.0.0-20180427100037-88b0669f7d75
 	github.com/gorilla/mux v1.8.0
 	github.com/goware/modvendor v0.5.0
 	github.com/grpc-ecosystem/grpc-gateway v1.16.0
@@ -121,6 +120,7 @@ require (
 	github.com/prometheus/prometheus v1.8.2-0.20210914090109-37468d88dce8
 	github.com/pseudomuto/protoc-gen-doc v1.3.2
 	github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475
+	github.com/robfig/cron/v3 v3.0.1
 	github.com/sasha-s/go-deadlock v0.3.1
 	github.com/shirou/gopsutil/v3 v3.21.12
 	github.com/slack-go/slack v0.9.5

go.sum

@@ -1065,8 +1065,6 @@ github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORR
 github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e h1:JKmoR8x90Iww1ks85zJ1lfDGgIiMDuIptTOhJq+zKyg=
 github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY=
 github.com/gordonklaus/ineffassign v0.0.0-20200309095847-7953dde2c7bf/go.mod h1:cuNKsD1zp2v6XfE/orVX2QE1LC+i254ceGcVeDT3pTU=
-github.com/gorhill/cronexpr v0.0.0-20180427100037-88b0669f7d75 h1:f0n1xnMSmBLzVfsMMvriDyA75NB/oBgILX2GcHXIQzY=
-github.com/gorhill/cronexpr v0.0.0-20180427100037-88b0669f7d75/go.mod h1:g2644b03hfBX9Ov0ZBDgXXens4rxSxmqFBbhvKv2yVA=
 github.com/gorilla/context v1.1.1/go.mod h1:kBGZzfjB9CEq2AlWe17Uuf7NDRt0dE0s8S51q0aT7Yg=
 github.com/gorilla/handlers v0.0.0-20150720190736-60c7bfde3e33/go.mod h1:Qkdc/uu4tH4g6mTK6auzZ766c4CA0Ng8+o/OAirnOIQ=
 github.com/gorilla/handlers v1.4.2/go.mod h1:Qkdc/uu4tH4g6mTK6auzZ766c4CA0Ng8+o/OAirnOIQ=
@@ -1782,6 +1780,8 @@ github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 h1:N/ElC8H3+5X
 github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
 github.com/remyoudompheng/bigfft v0.0.0-20170806203942-52369c62f446/go.mod h1:uYEyJGbgTkfkS4+E/PavXkNJcbFIpEtjt2B0KDQ5+9M=
 github.com/retailnext/hllpp v1.0.1-0.20180308014038-101a6d2f8b52/go.mod h1:RDpi1RftBQPUCDRw6SmxeaREsAaRKnOclghuzp/WRzc=
+github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs=
+github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro=
 github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
 github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ=
 github.com/rogpeppe/go-internal v1.1.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=

pkg/ccl/backupccl/BUILD.bazel

@@ -126,9 +126,9 @@ go_library(
         "@com_github_cockroachdb_logtags//:logtags",
         "@com_github_gogo_protobuf//jsonpb",
         "@com_github_gogo_protobuf//types",
-        "@com_github_gorhill_cronexpr//:cronexpr",
         "@com_github_kr_pretty//:pretty",
         "@com_github_lib_pq//oid",
+        "@com_github_robfig_cron_v3//:cron",
         "@com_github_stretchr_testify//require",
     ],
 )
@@ -251,10 +251,10 @@ go_test(
         "@com_github_cockroachdb_pebble//vfs",
         "@com_github_gogo_protobuf//proto",
         "@com_github_gogo_protobuf//types",
-        "@com_github_gorhill_cronexpr//:cronexpr",
         "@com_github_jackc_pgx_v4//:pgx",
         "@com_github_kr_pretty//:pretty",
         "@com_github_lib_pq//:pq",
+        "@com_github_robfig_cron_v3//:cron",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
         "@org_golang_x_sync//errgroup",

pkg/ccl/backupccl/create_scheduled_backup.go

@@ -39,7 +39,7 @@ import (
 	"github.com/cockroachdb/errors"
 	"github.com/gogo/protobuf/jsonpb"
 	pbtypes "github.com/gogo/protobuf/types"
-	"github.com/gorhill/cronexpr"
+	"github.com/robfig/cron/v3"
 )
 
 const (
@@ -195,19 +195,19 @@ func computeScheduleRecurrence(
 	if evalFn == nil {
 		return neverRecurs, nil
 	}
-	cron, err := evalFn()
+	cronStr, err := evalFn()
 	if err != nil {
 		return nil, err
 	}
-	expr, err := cronexpr.Parse(cron)
+	expr, err := cron.ParseStandard(cronStr)
 	if err != nil {
 		return nil, errors.Newf(
 			`error parsing schedule expression: %q; it must be a valid cron expression`,
-			cron)
+			cronStr)
 	}
 	nextRun := expr.Next(now)
 	frequency := expr.Next(nextRun).Sub(nextRun)
-	return &scheduleRecurrence{cron, frequency}, nil
+	return &scheduleRecurrence{cronStr, frequency}, nil
 }
 
 var forceFullBackup *scheduleRecurrence

pkg/ccl/backupccl/create_scheduled_backup_test.go

@@ -40,7 +40,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
 	pbtypes "github.com/gogo/protobuf/types"
-	"github.com/gorhill/cronexpr"
+	"github.com/robfig/cron/v3"
 	"github.com/stretchr/testify/require"
 )
 
@@ -1078,8 +1078,10 @@ INSERT INTO t values (1), (10), (100);
 		// to the next scheduled recurrence.
 		for _, id := range []int64{fullID, incID} {
 			s := th.loadSchedule(t, id)
+			e, err := cron.ParseStandard(s.ScheduleExpr())
+			require.NoError(t, err)
 			require.EqualValues(t,
-				cronexpr.MustParse(s.ScheduleExpr()).Next(th.env.Now()).Round(time.Microsecond),
+				e.Next(th.env.Now()).Round(time.Microsecond),
 				s.NextRun())
 		}
 

pkg/jobs/BUILD.bazel

@@ -61,8 +61,8 @@ go_library(
         "@com_github_cockroachdb_logtags//:logtags",
         "@com_github_cockroachdb_redact//:redact",
         "@com_github_gogo_protobuf//types",
-        "@com_github_gorhill_cronexpr//:cronexpr",
         "@com_github_prometheus_client_model//go",
+        "@com_github_robfig_cron_v3//:cron",
         "@io_opentelemetry_go_otel//attribute",
     ],
 )
@@ -135,8 +135,8 @@ go_test(
         "@com_github_cockroachdb_redact//:redact",
         "@com_github_gogo_protobuf//types",
         "@com_github_google_go_cmp//cmp",
-        "@com_github_gorhill_cronexpr//:cronexpr",
         "@com_github_kr_pretty//:pretty",
+        "@com_github_robfig_cron_v3//:cron",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
     ],

pkg/jobs/executor_impl_test.go

@@ -19,7 +19,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
-	"github.com/gorhill/cronexpr"
 	"github.com/stretchr/testify/require"
 )
 
@@ -40,7 +39,7 @@ func TestInlineExecutorFailedJobsHandling(t *testing.T) {
 	}{
 		{
 			onError:         jobspb.ScheduleDetails_RETRY_SCHED,
-			expectedNextRun: cronexpr.MustParse("@daily").Next(h.env.Now()).Round(time.Microsecond),
+			expectedNextRun: cronMustParse(t, "@daily").Next(h.env.Now()).Round(time.Microsecond),
 		},
 		{
 			onError:         jobspb.ScheduleDetails_RETRY_SOON,

pkg/jobs/job_scheduler_test.go

@@ -38,10 +38,16 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
 	"github.com/cockroachdb/errors"
 	"github.com/gogo/protobuf/types"
-	"github.com/gorhill/cronexpr"
+	"github.com/robfig/cron/v3"
 	"github.com/stretchr/testify/require"
 )
 
+func cronMustParse(t *testing.T, s string) cron.Schedule {
+	e, err := cron.ParseStandard(s)
+	require.NoError(t, err)
+	return e
+}
+
 func TestJobSchedulerReschedulesRunning(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
@@ -75,7 +81,7 @@ func TestJobSchedulerReschedulesRunning(t *testing.T) {
 				}))
 
 			// Verify the job has expected nextRun time.
-			expectedRunTime := cronexpr.MustParse("@hourly").Next(h.env.Now())
+			expectedRunTime := cronMustParse(t, "@hourly").Next(h.env.Now())
 			loaded := h.loadSchedule(t, j.ScheduleID())
 			require.Equal(t, expectedRunTime, loaded.NextRun())
 
@@ -93,7 +99,7 @@ func TestJobSchedulerReschedulesRunning(t *testing.T) {
 			if wait == jobspb.ScheduleDetails_WAIT {
 				expectedRunTime = h.env.Now().Add(recheckRunningAfter)
 			} else {
-				expectedRunTime = cronexpr.MustParse("@hourly").Next(h.env.Now())
+				expectedRunTime = cronMustParse(t, "@hourly").Next(h.env.Now())
 			}
 			loaded = h.loadSchedule(t, j.ScheduleID())
 			require.Equal(t, expectedRunTime, loaded.NextRun())
@@ -133,7 +139,7 @@ func TestJobSchedulerExecutesAfterTerminal(t *testing.T) {
 				}))
 
 			// Verify the job has expected nextRun time.
-			expectedRunTime := cronexpr.MustParse("@hourly").Next(h.env.Now())
+			expectedRunTime := cronMustParse(t, "@hourly").Next(h.env.Now())
 			loaded := h.loadSchedule(t, j.ScheduleID())
 			require.Equal(t, expectedRunTime, loaded.NextRun())
 
@@ -147,7 +153,7 @@ func TestJobSchedulerExecutesAfterTerminal(t *testing.T) {
 					return s.executeSchedules(ctx, allSchedules, txn)
 				}))
 
-			expectedRunTime = cronexpr.MustParse("@hourly").Next(h.env.Now())
+			expectedRunTime = cronMustParse(t, "@hourly").Next(h.env.Now())
 			loaded = h.loadSchedule(t, j.ScheduleID())
 			require.Equal(t, expectedRunTime, loaded.NextRun())
 		})
@@ -173,7 +179,7 @@ func TestJobSchedulerExecutesAndSchedulesNextRun(t *testing.T) {
 		}))
 
 	// Verify the job has expected nextRun time.
-	expectedRunTime := cronexpr.MustParse("@hourly").Next(h.env.Now())
+	expectedRunTime := cronMustParse(t, "@hourly").Next(h.env.Now())
 	loaded := h.loadSchedule(t, j.ScheduleID())
 	require.Equal(t, expectedRunTime, loaded.NextRun())
 
@@ -187,7 +193,7 @@ func TestJobSchedulerExecutesAndSchedulesNextRun(t *testing.T) {
 			return s.executeSchedules(ctx, allSchedules, txn)
 		}))
 
-	expectedRunTime = cronexpr.MustParse("@hourly").Next(h.env.Now())
+	expectedRunTime = cronMustParse(t, "@hourly").Next(h.env.Now())
 	loaded = h.loadSchedule(t, j.ScheduleID())
 	require.Equal(t, expectedRunTime, loaded.NextRun())
 }
@@ -550,7 +556,7 @@ func TestJobSchedulerRetriesFailed(t *testing.T) {
 	startTime := h.env.Now()
 	execTime := startTime.Add(time.Hour).Add(time.Second)
 
-	cron := cronexpr.MustParse("@hourly")
+	cron := cronMustParse(t, "@hourly")
 
 	for _, tc := range []struct {
 		onError jobspb.ScheduleDetails_ErrorHandlingBehavior

pkg/jobs/scheduled_job.go

@@ -27,7 +27,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlutil"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
 	"github.com/cockroachdb/errors"
-	"github.com/gorhill/cronexpr"
+	"github.com/robfig/cron/v3"
 )
 
 // scheduledJobRecord is a reflective representation of a row in
@@ -194,12 +194,13 @@ func (j *ScheduledJob) Frequency() (time.Duration, error) {
 		return 0, errors.Newf(
 			"schedule %d is not periodic", j.rec.ScheduleID)
 	}
-	expr, err := cronexpr.Parse(j.rec.ScheduleExpr)
+	expr, err := cron.ParseStandard(j.rec.ScheduleExpr)
 	if err != nil {
 		return 0, errors.Wrapf(err,
 			"parsing schedule expression: %q; it must be a valid cron expression",
 			j.rec.ScheduleExpr)
 	}
+
 	next := expr.Next(j.env.Now())
 	nextNext := expr.Next(next)
 	return nextNext.Sub(next), nil
@@ -211,7 +212,7 @@ func (j *ScheduledJob) ScheduleNextRun() error {
 		return errors.Newf(
 			"cannot set next run for schedule %d (empty schedule)", j.rec.ScheduleID)
 	}
-	expr, err := cronexpr.Parse(j.rec.ScheduleExpr)
+	expr, err := cron.ParseStandard(j.rec.ScheduleExpr)
 	if err != nil {
 		return errors.Wrapf(err, "parsing schedule expression: %q", j.rec.ScheduleExpr)
 	}

pkg/sql/sqlstats/persistedsqlstats/BUILD.bazel

@@ -46,7 +46,7 @@ go_library(
         "//pkg/util/timeutil",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_gogo_protobuf//types",
-        "@com_github_gorhill_cronexpr//:cronexpr",
+        "@com_github_robfig_cron_v3//:cron",
     ],
 )
 

pkg/sql/sqlstats/persistedsqlstats/cluster_settings.go

@@ -15,7 +15,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/errors"
-	"github.com/gorhill/cronexpr"
+	"github.com/robfig/cron/v3"
 )
 
 // SQLStatsFlushInterval is the cluster setting that controls how often the SQL
@@ -73,7 +73,7 @@ var SQLStatsCleanupRecurrence = settings.RegisterValidatedStringSetting(
 	"cron-tab recurrence for SQL Stats cleanup job",
 	"@hourly", /* defaultValue */
 	func(_ *settings.Values, s string) error {
-		if _, err := cronexpr.Parse(s); err != nil {
+		if _, err := cron.ParseStandard(s); err != nil {
 			return errors.Wrap(err, "invalid cron expression")
 		}
 		return nil

pkg/sql/sqlstats/persistedsqlstats/scheduled_sql_stats_compaction_test.go

@@ -198,7 +198,9 @@ func TestSQLStatsScheduleOperations(t *testing.T) {
 
 	t.Run("warn_schedule_long_run_interval", func(t *testing.T) {
 		t.Run("via cluster setting", func(t *testing.T) {
-			helper.sqlDB.Exec(t, "SET CLUSTER SETTING sql.stats.cleanup.recurrence = '0 59 23 24 12 ? 2099'")
+			// Craft an expression that next repeats next month.
+			expr := fmt.Sprintf("59 23 24 %d ?", timeutil.Now().AddDate(0, 1, 0).Month())
+			helper.sqlDB.Exec(t, "SET CLUSTER SETTING sql.stats.cleanup.recurrence = $1", expr)
 
 			var err error
 			testutils.SucceedsSoon(t, func() error {
@@ -208,7 +210,7 @@ func TestSQLStatsScheduleOperations(t *testing.T) {
 				if err == nil {
 					return errors.Newf("retry: next_run=%s, schedule_expr=%s", sj.NextRun(), sj.ScheduleExpr())
 				}
-				require.Equal(t, "0 59 23 24 12 ? 2099", sj.ScheduleExpr())
+				require.Equal(t, expr, sj.ScheduleExpr())
 				return nil
 			})
 			require.True(t, errors.Is(