Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use UTC as default timezone when schedule Actions cron tasks #31742

Merged
merged 6 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 9 additions & 11 deletions models/actions/schedule.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
}
}
Expand Down
25 changes: 24 additions & 1 deletion models/actions/schedule_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package actions

import (
"context"
"strings"
"time"

"code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo"
Expand Down Expand Up @@ -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=") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we handle leading whitespace?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not "we", it's how "robfig/cron" handle this.

https://github.com/robfig/cron/blob/bc59245fe10efaed9d51b56900192527ed733435/parser.go#L95

If we want to handle leading whitespace, it could be done before storing it into the database.

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() {
Expand Down
71 changes: 71 additions & 0 deletions models/actions/schedule_spec_test.go
Original file line number Diff line number Diff line change
@@ -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))
}
})
}
}