From 21a73ae642b15982a911837775c9583deb47220c Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 1 Aug 2024 18:02:46 +0800 Subject: [PATCH] Use UTC as default timezone when schedule Actions cron tasks (#31742) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #31657. According to the [doc](https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#onschedule) of GitHub Actions, The timezone for cron should be UTC, not the local timezone. And Gitea Actions doesn't have any reasons to change this, so I think it's a bug. However, Gitea Actions has extended the syntax, as it supports descriptors like `@weekly` and `@every 5m`, and supports specifying the timezone like `TZ=UTC 0 10 * * *`. So we can make it use UTC only when the timezone is not specified, to be compatible with GitHub Actions, and also respect the user's specified. It does break the feature because the times to run tasks would be changed, and it may confuse users. So I don't think we should backport this. ## ⚠️ BREAKING ⚠️ If the server's local time zone is not UTC, a scheduled task would run at a different time after upgrading Gitea to this version. --- models/actions/schedule.go | 20 ++++---- models/actions/schedule_spec.go | 25 +++++++++- models/actions/schedule_spec_test.go | 71 ++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 12 deletions(-) create mode 100644 models/actions/schedule_spec_test.go diff --git a/models/actions/schedule.go b/models/actions/schedule.go index 3646a046a0..c751ef51ca 100644 --- a/models/actions/schedule.go +++ b/models/actions/schedule.go @@ -13,8 +13,6 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/timeutil" webhook_module "code.gitea.io/gitea/modules/webhook" - - "github.com/robfig/cron/v3" ) // ActionSchedule represents a schedule of a workflow file @@ -53,8 +51,6 @@ func GetReposMapByIDs(ctx context.Context, ids []int64) (map[int64]*repo_model.R return repos, db.GetEngine(ctx).In("id", ids).Find(&repos) } -var cronParser = cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor) - // CreateScheduleTask creates new schedule task. func CreateScheduleTask(ctx context.Context, rows []*ActionSchedule) error { // Return early if there are no rows to insert @@ -80,19 +76,21 @@ func CreateScheduleTask(ctx context.Context, rows []*ActionSchedule) error { now := time.Now() for _, spec := range row.Specs { + specRow := &ActionScheduleSpec{ + RepoID: row.RepoID, + ScheduleID: row.ID, + Spec: spec, + } // Parse the spec and check for errors - schedule, err := cronParser.Parse(spec) + schedule, err := specRow.Parse() if err != nil { continue // skip to the next spec if there's an error } + specRow.Next = timeutil.TimeStamp(schedule.Next(now).Unix()) + // Insert the new schedule spec row - if err = db.Insert(ctx, &ActionScheduleSpec{ - RepoID: row.RepoID, - ScheduleID: row.ID, - Spec: spec, - Next: timeutil.TimeStamp(schedule.Next(now).Unix()), - }); err != nil { + if err = db.Insert(ctx, specRow); err != nil { return err } } diff --git a/models/actions/schedule_spec.go b/models/actions/schedule_spec.go index 91240459a0..923e5f7807 100644 --- a/models/actions/schedule_spec.go +++ b/models/actions/schedule_spec.go @@ -5,6 +5,8 @@ package actions import ( "context" + "strings" + "time" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" @@ -32,8 +34,29 @@ type ActionScheduleSpec struct { Updated timeutil.TimeStamp `xorm:"updated"` } +// Parse parses the spec and returns a cron.Schedule +// Unlike the default cron parser, Parse uses UTC timezone as the default if none is specified. func (s *ActionScheduleSpec) Parse() (cron.Schedule, error) { - return cronParser.Parse(s.Spec) + parser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor) + schedule, err := parser.Parse(s.Spec) + if err != nil { + return nil, err + } + + // If the spec has specified a timezone, use it + if strings.HasPrefix(s.Spec, "TZ=") || strings.HasPrefix(s.Spec, "CRON_TZ=") { + return schedule, nil + } + + specSchedule, ok := schedule.(*cron.SpecSchedule) + // If it's not a spec schedule, like "@every 5m", timezone is not relevant + if !ok { + return schedule, nil + } + + // Set the timezone to UTC + specSchedule.Location = time.UTC + return specSchedule, nil } func init() { diff --git a/models/actions/schedule_spec_test.go b/models/actions/schedule_spec_test.go new file mode 100644 index 0000000000..0c26fce4b2 --- /dev/null +++ b/models/actions/schedule_spec_test.go @@ -0,0 +1,71 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package actions + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestActionScheduleSpec_Parse(t *testing.T) { + // Mock the local timezone is not UTC + local := time.Local + tz, err := time.LoadLocation("Asia/Shanghai") + require.NoError(t, err) + defer func() { + time.Local = local + }() + time.Local = tz + + now, err := time.Parse(time.RFC3339, "2024-07-31T15:47:55+08:00") + require.NoError(t, err) + + tests := []struct { + name string + spec string + want string + wantErr assert.ErrorAssertionFunc + }{ + { + name: "regular", + spec: "0 10 * * *", + want: "2024-07-31T10:00:00Z", + wantErr: assert.NoError, + }, + { + name: "invalid", + spec: "0 10 * *", + want: "", + wantErr: assert.Error, + }, + { + name: "with timezone", + spec: "TZ=America/New_York 0 10 * * *", + want: "2024-07-31T14:00:00Z", + wantErr: assert.NoError, + }, + { + name: "timezone irrelevant", + spec: "@every 5m", + want: "2024-07-31T07:52:55Z", + wantErr: assert.NoError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &ActionScheduleSpec{ + Spec: tt.spec, + } + got, err := s.Parse() + tt.wantErr(t, err) + + if err == nil { + assert.Equal(t, tt.want, got.Next(now).UTC().Format(time.RFC3339)) + } + }) + } +}