From cab8736d7d056a15acbd5528747e40c2bf616618 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 4 Jan 2024 19:30:33 +0800 Subject: [PATCH 1/5] Fix schedule tasks bugs --- models/actions/run.go | 3 +- models/actions/run_list.go | 5 ++ models/repo/repo_unit.go | 26 --------- modules/actions/workflows.go | 21 ++++--- modules/webhook/type.go | 1 + routers/api/v1/repo/repo.go | 2 +- routers/web/repo/setting/default_branch.go | 26 +++------ routers/web/repo/setting/setting.go | 2 +- services/actions/notifier_helper.go | 44 ++++++++------ services/actions/schedule_tasks.go | 3 +- services/repository/branch.go | 67 ++++++++++++++++++++++ services/repository/setting.go | 48 ++++++++++++++++ services/wiki/wiki.go | 3 +- tests/integration/actions_trigger_test.go | 4 +- 14 files changed, 177 insertions(+), 78 deletions(-) create mode 100644 services/repository/setting.go diff --git a/models/actions/run.go b/models/actions/run.go index 4656aa22a2933..45b9ddf0b6798 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -168,12 +168,13 @@ func updateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) err } // CancelRunningJobs cancels all running and waiting jobs associated with a specific workflow. -func CancelRunningJobs(ctx context.Context, repoID int64, ref, workflowID string) error { +func CancelRunningJobs(ctx context.Context, repoID int64, ref, workflowID string, event webhook_module.HookEventType) error { // Find all runs in the specified repository, reference, and workflow with statuses 'Running' or 'Waiting'. runs, total, err := db.FindAndCount[ActionRun](ctx, FindRunOptions{ RepoID: repoID, Ref: ref, WorkflowID: workflowID, + Event: event, Status: []Status{StatusRunning, StatusWaiting}, }) if err != nil { diff --git a/models/actions/run_list.go b/models/actions/run_list.go index 375c46221b043..ca96696e5f592 100644 --- a/models/actions/run_list.go +++ b/models/actions/run_list.go @@ -10,6 +10,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/container" + webhook_module "code.gitea.io/gitea/modules/webhook" "xorm.io/builder" ) @@ -71,6 +72,7 @@ type FindRunOptions struct { WorkflowID string Ref string // the commit/tag/… that caused this workflow TriggerUserID int64 + Event webhook_module.HookEventType Approved bool // not util.OptionalBool, it works only when it's true Status []Status } @@ -98,6 +100,9 @@ func (opts FindRunOptions) ToConds() builder.Cond { if opts.Ref != "" { cond = cond.And(builder.Eq{"ref": opts.Ref}) } + if opts.Event != "" { + cond = cond.And(builder.Eq{"event": opts.Event}) + } return cond } diff --git a/models/repo/repo_unit.go b/models/repo/repo_unit.go index 89f28b8fcaee4..8a3ba1ee89092 100644 --- a/models/repo/repo_unit.go +++ b/models/repo/repo_unit.go @@ -283,29 +283,3 @@ func UpdateRepoUnit(ctx context.Context, unit *RepoUnit) error { _, err := db.GetEngine(ctx).ID(unit.ID).Update(unit) return err } - -// UpdateRepositoryUnits updates a repository's units -func UpdateRepositoryUnits(ctx context.Context, repo *Repository, units []RepoUnit, deleteUnitTypes []unit.Type) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - // Delete existing settings of units before adding again - for _, u := range units { - deleteUnitTypes = append(deleteUnitTypes, u.Type) - } - - if _, err = db.GetEngine(ctx).Where("repo_id = ?", repo.ID).In("type", deleteUnitTypes).Delete(new(RepoUnit)); err != nil { - return err - } - - if len(units) > 0 { - if err = db.Insert(ctx, units); err != nil { - return err - } - } - - return committer.Commit() -} diff --git a/modules/actions/workflows.go b/modules/actions/workflows.go index 408fdb8f8ef22..50d2db53c009b 100644 --- a/modules/actions/workflows.go +++ b/modules/actions/workflows.go @@ -22,7 +22,7 @@ import ( type DetectedWorkflow struct { EntryName string - TriggerEvent string + TriggerEvent *jobparser.Event Content []byte } @@ -100,6 +100,7 @@ func DetectWorkflows( commit *git.Commit, triggedEvent webhook_module.HookEventType, payload api.Payloader, + detectSchedule bool, ) ([]*DetectedWorkflow, []*DetectedWorkflow, error) { entries, err := ListWorkflows(commit) if err != nil { @@ -114,6 +115,7 @@ func DetectWorkflows( return nil, nil, err } + // one workflow may have multiple events events, err := GetEventsFromContent(content) if err != nil { log.Warn("ignore invalid workflow %q: %v", entry.Name(), err) @@ -122,17 +124,18 @@ func DetectWorkflows( for _, evt := range events { log.Trace("detect workflow %q for event %#v matching %q", entry.Name(), evt, triggedEvent) if evt.IsSchedule() { - dwf := &DetectedWorkflow{ - EntryName: entry.Name(), - TriggerEvent: evt.Name, - Content: content, + if detectSchedule { + dwf := &DetectedWorkflow{ + EntryName: entry.Name(), + TriggerEvent: evt, + Content: content, + } + schedules = append(schedules, dwf) } - schedules = append(schedules, dwf) - } - if detectMatched(gitRepo, commit, triggedEvent, payload, evt) { + } else if detectMatched(gitRepo, commit, triggedEvent, payload, evt) { dwf := &DetectedWorkflow{ EntryName: entry.Name(), - TriggerEvent: evt.Name, + TriggerEvent: evt, Content: content, } workflows = append(workflows, dwf) diff --git a/modules/webhook/type.go b/modules/webhook/type.go index 7042d391b7eb4..b18fe2d7a4f9f 100644 --- a/modules/webhook/type.go +++ b/modules/webhook/type.go @@ -31,6 +31,7 @@ const ( HookEventRepository HookEventType = "repository" HookEventRelease HookEventType = "release" HookEventPackage HookEventType = "package" + HookEventScheduleCreated HookEventType = "schedule" ) // Event returns the HookEventType as an event string diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 6eb2cc4227429..8ce03cf29caa5 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -983,7 +983,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { } if len(units)+len(deleteUnitTypes) > 0 { - if err := repo_model.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil { + if err := repo_service.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateRepositoryUnits", err) return err } diff --git a/routers/web/repo/setting/default_branch.go b/routers/web/repo/setting/default_branch.go index 9bf54e706a37e..c8a576e57663a 100644 --- a/routers/web/repo/setting/default_branch.go +++ b/routers/web/repo/setting/default_branch.go @@ -6,13 +6,12 @@ package setting import ( "net/http" - repo_model "code.gitea.io/gitea/models/repo" + git_model "code.gitea.io/gitea/models/git" "code.gitea.io/gitea/modules/context" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/routers/web/repo" - notify_service "code.gitea.io/gitea/services/notify" + repo_service "code.gitea.io/gitea/services/repository" ) // SetDefaultBranchPost set default branch @@ -35,23 +34,14 @@ func SetDefaultBranchPost(ctx *context.Context) { } branch := ctx.FormString("branch") - if !ctx.Repo.GitRepo.IsBranchExist(branch) { - ctx.Status(http.StatusNotFound) - return - } else if repo.DefaultBranch != branch { - repo.DefaultBranch = branch - if err := ctx.Repo.GitRepo.SetDefaultBranch(branch); err != nil { - if !git.IsErrUnsupportedVersion(err) { - ctx.ServerError("SetDefaultBranch", err) - return - } - } - if err := repo_model.UpdateDefaultBranch(ctx, repo); err != nil { + if err := repo_service.SetRepoDefaultBranch(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, branch); err != nil { + switch { + case git_model.IsErrBranchNotExist(err): + ctx.Status(http.StatusNotFound) + default: ctx.ServerError("SetDefaultBranch", err) - return } - - notify_service.ChangeDefaultBranch(ctx, repo) + return } log.Trace("Repository basic settings updated: %s/%s", ctx.Repo.Owner.Name, repo.Name) diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 289cf5c9aea57..e721b7c739972 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -594,7 +594,7 @@ func SettingsPost(ctx *context.Context) { return } - if err := repo_model.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil { + if err := repo_service.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil { ctx.ServerError("UpdateRepositoryUnits", err) return } diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 705661ae7a775..265ae6c50cbdf 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -117,6 +117,11 @@ func notify(ctx context.Context, input *notifyInput) error { return nil } if unit_model.TypeActions.UnitGlobalDisabled() { + // If actions disabled when there is schedule task, this will remove the outdated schedule tasks + // There is no other place we can do this because the app.ini will be changed manually + if err := actions_model.DeleteScheduleTaskByRepo(ctx, input.Repo.ID); err != nil { + log.Error("DeleteCronTaskByRepo: %v", err) + } return nil } if err := input.Repo.LoadUnits(ctx); err != nil { @@ -153,7 +158,11 @@ func notify(ctx context.Context, input *notifyInput) error { var detectedWorkflows []*actions_module.DetectedWorkflow actionsConfig := input.Repo.MustGetUnit(ctx, unit_model.TypeActions).ActionsConfig() - workflows, schedules, err := actions_module.DetectWorkflows(gitRepo, commit, input.Event, input.Payload) + workflows, schedules, err := actions_module.DetectWorkflows(gitRepo, commit, + input.Event, + input.Payload, + input.Event == webhook_module.HookEventPush && input.Ref == input.Repo.DefaultBranch, + ) if err != nil { return fmt.Errorf("DetectWorkflows: %w", err) } @@ -167,7 +176,7 @@ func notify(ctx context.Context, input *notifyInput) error { continue } - if wf.TriggerEvent != actions_module.GithubEventPullRequestTarget { + if wf.TriggerEvent.Name != actions_module.GithubEventPullRequestTarget { detectedWorkflows = append(detectedWorkflows, wf) } } @@ -180,7 +189,7 @@ func notify(ctx context.Context, input *notifyInput) error { if err != nil { return fmt.Errorf("gitRepo.GetCommit: %w", err) } - baseWorkflows, _, err := actions_module.DetectWorkflows(gitRepo, baseCommit, input.Event, input.Payload) + baseWorkflows, _, err := actions_module.DetectWorkflows(gitRepo, baseCommit, input.Event, input.Payload, false) if err != nil { return fmt.Errorf("DetectWorkflows: %w", err) } @@ -188,7 +197,7 @@ func notify(ctx context.Context, input *notifyInput) error { log.Trace("repo %s with commit %s couldn't find pull_request_target workflows", input.Repo.RepoPath(), baseCommit.ID) } else { for _, wf := range baseWorkflows { - if wf.TriggerEvent == actions_module.GithubEventPullRequestTarget { + if wf.TriggerEvent.Name == actions_module.GithubEventPullRequestTarget { detectedWorkflows = append(detectedWorkflows, wf) } } @@ -265,7 +274,7 @@ func handleWorkflows( IsForkPullRequest: isForkPullRequest, Event: input.Event, EventPayload: string(p), - TriggerEvent: dwf.TriggerEvent, + TriggerEvent: dwf.TriggerEvent.Name, Status: actions_model.StatusWaiting, } if need, err := ifNeedApproval(ctx, run, input.Repo, input.Doer); err != nil { @@ -289,6 +298,7 @@ func handleWorkflows( run.RepoID, run.Ref, run.WorkflowID, + run.Event, ); err != nil { log.Error("CancelRunningJobs: %v", err) } @@ -417,6 +427,17 @@ func handleSchedules( if err := actions_model.DeleteScheduleTaskByRepo(ctx, input.Repo.ID); err != nil { log.Error("DeleteCronTaskByRepo: %v", err) } + + // cancel running cron jobs of this repository and delete old schedules + if err := actions_model.CancelRunningJobs( + ctx, + input.Repo.ID, + input.Ref, + "", + webhook_module.HookEventScheduleCreated, + ); err != nil { + log.Error("CancelRunningJobs: %v", err) + } } if len(detectedWorkflows) == 0 { @@ -456,19 +477,6 @@ func handleSchedules( Specs: schedules, Content: dwf.Content, } - - // cancel running jobs if the event is push - if run.Event == webhook_module.HookEventPush { - // cancel running jobs of the same workflow - if err := actions_model.CancelRunningJobs( - ctx, - run.RepoID, - run.Ref, - run.WorkflowID, - ); err != nil { - log.Error("CancelRunningJobs: %v", err) - } - } crons = append(crons, run) } diff --git a/services/actions/schedule_tasks.go b/services/actions/schedule_tasks.go index 8eef2b67bdc7a..40a9e94043a16 100644 --- a/services/actions/schedule_tasks.go +++ b/services/actions/schedule_tasks.go @@ -59,6 +59,7 @@ func startTasks(ctx context.Context) error { row.RepoID, row.Schedule.Ref, row.Schedule.WorkflowID, + webhook_module.HookEventScheduleCreated, ); err != nil { log.Error("CancelRunningJobs: %v", err) } @@ -111,7 +112,7 @@ func CreateScheduleTask(ctx context.Context, cron *actions_model.ActionSchedule) TriggerUserID: cron.TriggerUserID, Ref: cron.Ref, CommitSHA: cron.CommitSHA, - Event: cron.Event, + Event: webhook_module.HookEventScheduleCreated, EventPayload: cron.EventPayload, ScheduleID: cron.ID, Status: actions_model.StatusWaiting, diff --git a/services/repository/branch.go b/services/repository/branch.go index 7254778763371..a098842fc27c9 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -10,6 +10,7 @@ import ( "strings" "code.gitea.io/gitea/models" + actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" @@ -22,6 +23,7 @@ import ( repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" + webhook_module "code.gitea.io/gitea/modules/webhook" notify_service "code.gitea.io/gitea/services/notify" files_service "code.gitea.io/gitea/services/repository/files" @@ -315,6 +317,22 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, doer *user_m } if isDefault { + // if default branch changed, we need to delete all schedules and cron jobs + if err := actions_model.DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil { + log.Error("DeleteCronTaskByRepo: %v", err) + } + + // cancel running cron jobs of this repository and delete old schedules + if err := actions_model.CancelRunningJobs( + ctx, + repo.ID, + from, + "", + webhook_module.HookEventScheduleCreated, + ); err != nil { + log.Error("CancelRunningJobs: %v", err) + } + err2 = gitRepo.SetDefaultBranch(to) if err2 != nil { return err2 @@ -450,3 +468,52 @@ func AddAllRepoBranchesToSyncQueue(ctx context.Context, doerID int64) error { } return nil } + +func SetRepoDefaultBranch(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, newBranchName string) error { + if repo.DefaultBranch == newBranchName { + return nil + } + + if !gitRepo.IsBranchExist(newBranchName) { + return git_model.ErrBranchNotExist{ + BranchName: newBranchName, + } + } + + oldDefaultBranchName := repo.DefaultBranch + repo.DefaultBranch = newBranchName + if err := db.WithTx(ctx, func(ctx context.Context) error { + if err := repo_model.UpdateDefaultBranch(ctx, repo); err != nil { + return err + } + + if err := actions_model.DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil { + log.Error("DeleteCronTaskByRepo: %v", err) + } + + // delete actions + // cancel running cron jobs of this repository and delete old schedules + if err := actions_model.CancelRunningJobs( + ctx, + repo.ID, + oldDefaultBranchName, + "", + webhook_module.HookEventScheduleCreated, + ); err != nil { + log.Error("CancelRunningJobs: %v", err) + } + + if err := gitRepo.SetDefaultBranch(newBranchName); err != nil { + if !git.IsErrUnsupportedVersion(err) { + return err + } + } + return nil + }); err != nil { + return err + } + + notify_service.ChangeDefaultBranch(ctx, repo) + + return nil +} diff --git a/services/repository/setting.go b/services/repository/setting.go new file mode 100644 index 0000000000000..aefb75573c1d6 --- /dev/null +++ b/services/repository/setting.go @@ -0,0 +1,48 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repository + +import ( + "context" + "slices" + + actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/modules/log" +) + +// UpdateRepositoryUnits updates a repository's units +func UpdateRepositoryUnits(ctx context.Context, repo *repo_model.Repository, units []repo_model.RepoUnit, deleteUnitTypes []unit.Type) (err error) { + ctx, committer, err := db.TxContext(ctx) + if err != nil { + return err + } + defer committer.Close() + + // Delete existing settings of units before adding again + for _, u := range units { + deleteUnitTypes = append(deleteUnitTypes, u.Type) + } + + if slices.Contains(deleteUnitTypes, unit.TypeActions) { + // if actions is disabled, delete all cron tasks + if err := actions_model.DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil { + log.Error("DeleteCronTaskByRepo: %v", err) + } + } + + if _, err = db.GetEngine(ctx).Where("repo_id = ?", repo.ID).In("type", deleteUnitTypes).Delete(new(repo_model.RepoUnit)); err != nil { + return err + } + + if len(units) > 0 { + if err = db.Insert(ctx, units); err != nil { + return err + } + } + + return committer.Commit() +} diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go index f98854c8dd5f6..ce54a00da73b4 100644 --- a/services/wiki/wiki.go +++ b/services/wiki/wiki.go @@ -19,6 +19,7 @@ import ( repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/sync" asymkey_service "code.gitea.io/gitea/services/asymkey" + repo_service "code.gitea.io/gitea/services/repository" ) // TODO: use clustered lock (unique queue? or *abuse* cache) @@ -350,7 +351,7 @@ func DeleteWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model // DeleteWiki removes the actual and local copy of repository wiki. func DeleteWiki(ctx context.Context, repo *repo_model.Repository) error { - if err := repo_model.UpdateRepositoryUnits(ctx, repo, nil, []unit.Type{unit.TypeWiki}); err != nil { + if err := repo_service.UpdateRepositoryUnits(ctx, repo, nil, []unit.Type{unit.TypeWiki}); err != nil { return err } diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index 684b93ed1dfca..7744f33e5751f 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -47,7 +47,7 @@ func TestPullRequestTargetEvent(t *testing.T) { assert.NotEmpty(t, baseRepo) // enable actions - err = repo_model.UpdateRepositoryUnits(db.DefaultContext, baseRepo, []repo_model.RepoUnit{{ + err = repo_service.UpdateRepositoryUnits(db.DefaultContext, baseRepo, []repo_model.RepoUnit{{ RepoID: baseRepo.ID, Type: unit_model.TypeActions, }}, nil) @@ -216,7 +216,7 @@ func TestSkipCI(t *testing.T) { assert.NotEmpty(t, repo) // enable actions - err = repo_model.UpdateRepositoryUnits(db.DefaultContext, repo, []repo_model.RepoUnit{{ + err = repo_service.UpdateRepositoryUnits(db.DefaultContext, repo, []repo_model.RepoUnit{{ RepoID: repo.ID, Type: unit_model.TypeActions, }}, nil) From 3fe7aa13bef17441fbc845b3c8e0ce01c92b297e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 4 Jan 2024 19:45:11 +0800 Subject: [PATCH 2/5] more schedule improvements --- models/actions/run.go | 10 +++++----- models/actions/run_list.go | 6 +++--- modules/actions/github.go | 4 ++++ modules/actions/workflows.go | 3 ++- modules/actions/workflows_test.go | 7 +++++++ modules/webhook/type.go | 2 +- services/actions/notifier_helper.go | 2 +- services/actions/schedule_tasks.go | 5 +++-- services/repository/branch.go | 4 ++-- 9 files changed, 28 insertions(+), 15 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index 45b9ddf0b6798..1a3701b0b0aec 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -171,11 +171,11 @@ func updateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) err func CancelRunningJobs(ctx context.Context, repoID int64, ref, workflowID string, event webhook_module.HookEventType) error { // Find all runs in the specified repository, reference, and workflow with statuses 'Running' or 'Waiting'. runs, total, err := db.FindAndCount[ActionRun](ctx, FindRunOptions{ - RepoID: repoID, - Ref: ref, - WorkflowID: workflowID, - Event: event, - Status: []Status{StatusRunning, StatusWaiting}, + RepoID: repoID, + Ref: ref, + WorkflowID: workflowID, + TriggerEvent: event, + Status: []Status{StatusRunning, StatusWaiting}, }) if err != nil { return err diff --git a/models/actions/run_list.go b/models/actions/run_list.go index ca96696e5f592..388bfc4f86f94 100644 --- a/models/actions/run_list.go +++ b/models/actions/run_list.go @@ -72,7 +72,7 @@ type FindRunOptions struct { WorkflowID string Ref string // the commit/tag/… that caused this workflow TriggerUserID int64 - Event webhook_module.HookEventType + TriggerEvent webhook_module.HookEventType Approved bool // not util.OptionalBool, it works only when it's true Status []Status } @@ -100,8 +100,8 @@ func (opts FindRunOptions) ToConds() builder.Cond { if opts.Ref != "" { cond = cond.And(builder.Eq{"ref": opts.Ref}) } - if opts.Event != "" { - cond = cond.And(builder.Eq{"event": opts.Event}) + if opts.TriggerEvent != "" { + cond = cond.And(builder.Eq{"trigger_event": opts.TriggerEvent}) } return cond } diff --git a/modules/actions/github.go b/modules/actions/github.go index 71f81a89034c5..fafea4e11a805 100644 --- a/modules/actions/github.go +++ b/modules/actions/github.go @@ -22,6 +22,7 @@ const ( GithubEventRelease = "release" GithubEventPullRequestComment = "pull_request_comment" GithubEventGollum = "gollum" + GithubEventSchedule = "schedule" ) // canGithubEventMatch check if the input Github event can match any Gitea event. @@ -69,6 +70,9 @@ func canGithubEventMatch(eventName string, triggedEvent webhook_module.HookEvent return false } + case GithubEventSchedule: + return triggedEvent == webhook_module.HookEventSchedule + default: return eventName == string(triggedEvent) } diff --git a/modules/actions/workflows.go b/modules/actions/workflows.go index 50d2db53c009b..cbc7e011d1d1b 100644 --- a/modules/actions/workflows.go +++ b/modules/actions/workflows.go @@ -156,7 +156,8 @@ func detectMatched(gitRepo *git.Repository, commit *git.Commit, triggedEvent web webhook_module.HookEventCreate, webhook_module.HookEventDelete, webhook_module.HookEventFork, - webhook_module.HookEventWiki: + webhook_module.HookEventWiki, + webhook_module.HookEventSchedule: if len(evt.Acts()) != 0 { log.Warn("Ignore unsupported %s event arguments %v", triggedEvent, evt.Acts()) } diff --git a/modules/actions/workflows_test.go b/modules/actions/workflows_test.go index 2d57f19488e4d..c8e1e553fe94b 100644 --- a/modules/actions/workflows_test.go +++ b/modules/actions/workflows_test.go @@ -118,6 +118,13 @@ func TestDetectMatched(t *testing.T) { yamlOn: "on: gollum", expected: true, }, + { + desc: "HookEventSchedue(schedule) matches GithubEventSchedule(schedule)", + triggedEvent: webhook_module.HookEventSchedule, + payload: nil, + yamlOn: "on: schedule", + expected: true, + }, } for _, tc := range testCases { diff --git a/modules/webhook/type.go b/modules/webhook/type.go index b18fe2d7a4f9f..0013691c02f47 100644 --- a/modules/webhook/type.go +++ b/modules/webhook/type.go @@ -31,7 +31,7 @@ const ( HookEventRepository HookEventType = "repository" HookEventRelease HookEventType = "release" HookEventPackage HookEventType = "package" - HookEventScheduleCreated HookEventType = "schedule" + HookEventSchedule HookEventType = "schedule" ) // Event returns the HookEventType as an event string diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 265ae6c50cbdf..53def90da2040 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -434,7 +434,7 @@ func handleSchedules( input.Repo.ID, input.Ref, "", - webhook_module.HookEventScheduleCreated, + webhook_module.HookEventSchedule, ); err != nil { log.Error("CancelRunningJobs: %v", err) } diff --git a/services/actions/schedule_tasks.go b/services/actions/schedule_tasks.go index 40a9e94043a16..e7aa4a39accfe 100644 --- a/services/actions/schedule_tasks.go +++ b/services/actions/schedule_tasks.go @@ -59,7 +59,7 @@ func startTasks(ctx context.Context) error { row.RepoID, row.Schedule.Ref, row.Schedule.WorkflowID, - webhook_module.HookEventScheduleCreated, + webhook_module.HookEventSchedule, ); err != nil { log.Error("CancelRunningJobs: %v", err) } @@ -112,8 +112,9 @@ func CreateScheduleTask(ctx context.Context, cron *actions_model.ActionSchedule) TriggerUserID: cron.TriggerUserID, Ref: cron.Ref, CommitSHA: cron.CommitSHA, - Event: webhook_module.HookEventScheduleCreated, + Event: cron.Event, EventPayload: cron.EventPayload, + TriggerEvent: string(webhook_module.HookEventSchedule), ScheduleID: cron.ID, Status: actions_model.StatusWaiting, } diff --git a/services/repository/branch.go b/services/repository/branch.go index a098842fc27c9..0b82f5ebe509e 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -328,7 +328,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, doer *user_m repo.ID, from, "", - webhook_module.HookEventScheduleCreated, + webhook_module.HookEventSchedule, ); err != nil { log.Error("CancelRunningJobs: %v", err) } @@ -498,7 +498,7 @@ func SetRepoDefaultBranch(ctx context.Context, repo *repo_model.Repository, gitR repo.ID, oldDefaultBranchName, "", - webhook_module.HookEventScheduleCreated, + webhook_module.HookEventSchedule, ); err != nil { log.Error("CancelRunningJobs: %v", err) } From 42a30c6ade21eb18092049f928aa93c00d50f767 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 7 Jan 2024 15:43:58 +0800 Subject: [PATCH 3/5] extract clean schedule as one function --- models/actions/schedule.go | 20 ++++++++++++++++++++ services/actions/notifier_helper.go | 7 ++----- services/repository/branch.go | 3 --- services/repository/setting.go | 11 +++++++++++ 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/models/actions/schedule.go b/models/actions/schedule.go index 34d23f1c0179c..d450e7aa07291 100644 --- a/models/actions/schedule.go +++ b/models/actions/schedule.go @@ -5,6 +5,7 @@ package actions import ( "context" + "fmt" "time" "code.gitea.io/gitea/models/db" @@ -118,3 +119,22 @@ func DeleteScheduleTaskByRepo(ctx context.Context, id int64) error { return committer.Commit() } + +func CleanRepoScheduleTasks(ctx context.Context, repo *repo_model.Repository) error { + // If actions disabled when there is schedule task, this will remove the outdated schedule tasks + // There is no other place we can do this because the app.ini will be changed manually + if err := DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil { + return fmt.Errorf("DeleteCronTaskByRepo: %v", err) + } + // cancel running cron jobs of this repository and delete old schedules + if err := CancelRunningJobs( + ctx, + repo.ID, + repo.DefaultBranch, + "", + webhook_module.HookEventSchedule, + ); err != nil { + return fmt.Errorf("CancelRunningJobs: %v", err) + } + return nil +} diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 53def90da2040..52b9ac55bdf6f 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -117,10 +117,8 @@ func notify(ctx context.Context, input *notifyInput) error { return nil } if unit_model.TypeActions.UnitGlobalDisabled() { - // If actions disabled when there is schedule task, this will remove the outdated schedule tasks - // There is no other place we can do this because the app.ini will be changed manually - if err := actions_model.DeleteScheduleTaskByRepo(ctx, input.Repo.ID); err != nil { - log.Error("DeleteCronTaskByRepo: %v", err) + if err := actions_model.CleanRepoScheduleTasks(ctx, input.Repo); err != nil { + log.Error("CleanRepoScheduleTasks: %v", err) } return nil } @@ -427,7 +425,6 @@ func handleSchedules( if err := actions_model.DeleteScheduleTaskByRepo(ctx, input.Repo.ID); err != nil { log.Error("DeleteCronTaskByRepo: %v", err) } - // cancel running cron jobs of this repository and delete old schedules if err := actions_model.CancelRunningJobs( ctx, diff --git a/services/repository/branch.go b/services/repository/branch.go index 0b82f5ebe509e..55366251b62bf 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -321,7 +321,6 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, doer *user_m if err := actions_model.DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil { log.Error("DeleteCronTaskByRepo: %v", err) } - // cancel running cron jobs of this repository and delete old schedules if err := actions_model.CancelRunningJobs( ctx, @@ -490,8 +489,6 @@ func SetRepoDefaultBranch(ctx context.Context, repo *repo_model.Repository, gitR if err := actions_model.DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil { log.Error("DeleteCronTaskByRepo: %v", err) } - - // delete actions // cancel running cron jobs of this repository and delete old schedules if err := actions_model.CancelRunningJobs( ctx, diff --git a/services/repository/setting.go b/services/repository/setting.go index aefb75573c1d6..f140fc51b8f03 100644 --- a/services/repository/setting.go +++ b/services/repository/setting.go @@ -12,6 +12,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/log" + webhook_module "code.gitea.io/gitea/modules/webhook" ) // UpdateRepositoryUnits updates a repository's units @@ -32,6 +33,16 @@ func UpdateRepositoryUnits(ctx context.Context, repo *repo_model.Repository, uni if err := actions_model.DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil { log.Error("DeleteCronTaskByRepo: %v", err) } + // cancel running cron jobs of this repository and delete old schedules + if err := actions_model.CancelRunningJobs( + ctx, + repo.ID, + repo.DefaultBranch, + "", + webhook_module.HookEventSchedule, + ); err != nil { + log.Error("CancelRunningJobs: %v", err) + } } if _, err = db.GetEngine(ctx).Where("repo_id = ?", repo.ID).In("type", deleteUnitTypes).Delete(new(repo_model.RepoUnit)); err != nil { From 24fb217e0587a51ef7e3bd57410079d230c0bd0a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 8 Jan 2024 13:22:38 +0800 Subject: [PATCH 4/5] Fix more bugs --- models/git/branch.go | 4 ++-- services/actions/notifier_helper.go | 1 + services/repository/branch.go | 2 +- services/repository/setting.go | 16 ++-------------- 4 files changed, 6 insertions(+), 17 deletions(-) diff --git a/models/git/branch.go b/models/git/branch.go index ffd1d7ed164a0..db02fc9b28bbd 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -283,7 +283,7 @@ func FindRenamedBranch(ctx context.Context, repoID int64, from string) (branch * } // RenameBranch rename a branch -func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to string, gitAction func(isDefault bool) error) (err error) { +func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to string, gitAction func(ctx context.Context, isDefault bool) error) (err error) { ctx, committer, err := db.TxContext(ctx) if err != nil { return err @@ -358,7 +358,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str } // 5. do git action - if err = gitAction(isDefault); err != nil { + if err = gitAction(ctx, isDefault); err != nil { return err } diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 52b9ac55bdf6f..b5ca5b320ae6e 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -422,6 +422,7 @@ func handleSchedules( log.Error("CountSchedules: %v", err) return err } else if count > 0 { + actions_model.CleanRepoScheduleTasks(ctx, input.Repo) if err := actions_model.DeleteScheduleTaskByRepo(ctx, input.Repo.ID); err != nil { log.Error("DeleteCronTaskByRepo: %v", err) } diff --git a/services/repository/branch.go b/services/repository/branch.go index 55366251b62bf..6ddc6badfa6e2 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -310,7 +310,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, doer *user_m return "from_not_exist", nil } - if err := git_model.RenameBranch(ctx, repo, from, to, func(isDefault bool) error { + if err := git_model.RenameBranch(ctx, repo, from, to, func(ctx context.Context, isDefault bool) error { err2 := gitRepo.RenameBranch(from, to) if err2 != nil { return err2 diff --git a/services/repository/setting.go b/services/repository/setting.go index f140fc51b8f03..6496ac4014c11 100644 --- a/services/repository/setting.go +++ b/services/repository/setting.go @@ -12,7 +12,6 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/log" - webhook_module "code.gitea.io/gitea/modules/webhook" ) // UpdateRepositoryUnits updates a repository's units @@ -29,19 +28,8 @@ func UpdateRepositoryUnits(ctx context.Context, repo *repo_model.Repository, uni } if slices.Contains(deleteUnitTypes, unit.TypeActions) { - // if actions is disabled, delete all cron tasks - if err := actions_model.DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil { - log.Error("DeleteCronTaskByRepo: %v", err) - } - // cancel running cron jobs of this repository and delete old schedules - if err := actions_model.CancelRunningJobs( - ctx, - repo.ID, - repo.DefaultBranch, - "", - webhook_module.HookEventSchedule, - ); err != nil { - log.Error("CancelRunningJobs: %v", err) + if err := actions_model.CleanRepoScheduleTasks(ctx, repo); err != nil { + log.Error("CleanRepoScheduleTasks: %v", err) } } From 084c116765d4524baff491dde0e5485e68ce2bb0 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 8 Jan 2024 13:40:15 +0800 Subject: [PATCH 5/5] Fix test --- models/git/branch_test.go | 3 ++- services/actions/notifier_helper.go | 15 ++------------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/models/git/branch_test.go b/models/git/branch_test.go index d480e2ec30c22..fd5d6519e919d 100644 --- a/models/git/branch_test.go +++ b/models/git/branch_test.go @@ -4,6 +4,7 @@ package git_test import ( + "context" "testing" "code.gitea.io/gitea/models/db" @@ -132,7 +133,7 @@ func TestRenameBranch(t *testing.T) { }, git_model.WhitelistOptions{})) assert.NoError(t, committer.Commit()) - assert.NoError(t, git_model.RenameBranch(db.DefaultContext, repo1, "master", "main", func(isDefault bool) error { + assert.NoError(t, git_model.RenameBranch(db.DefaultContext, repo1, "master", "main", func(ctx context.Context, isDefault bool) error { _isDefault = isDefault return nil })) diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index b5ca5b320ae6e..0618f15602ee5 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -422,19 +422,8 @@ func handleSchedules( log.Error("CountSchedules: %v", err) return err } else if count > 0 { - actions_model.CleanRepoScheduleTasks(ctx, input.Repo) - if err := actions_model.DeleteScheduleTaskByRepo(ctx, input.Repo.ID); err != nil { - log.Error("DeleteCronTaskByRepo: %v", err) - } - // cancel running cron jobs of this repository and delete old schedules - if err := actions_model.CancelRunningJobs( - ctx, - input.Repo.ID, - input.Ref, - "", - webhook_module.HookEventSchedule, - ); err != nil { - log.Error("CancelRunningJobs: %v", err) + if err := actions_model.CleanRepoScheduleTasks(ctx, input.Repo); err != nil { + log.Error("CleanRepoScheduleTasks: %v", err) } }