Skip to content

Commit 7586b58

Browse files
Zettat123GiteaBot
andauthored
Fix activity type match in matchPullRequestEvent (#25746)
Fix #25736 Caused by #24048 Right now we only check the activity type for `pull_request` event when `types` is specified or there are no `types` and filter. If a workflow only specifies filters but no `types` like this: ``` on: pull_request: branches: [main] ``` the workflow will be triggered even if the activity type is not one of `[opened, reopened, sync]`. We need to check the activity type in this case. Co-authored-by: Giteabot <[email protected]>
1 parent 128d77a commit 7586b58

File tree

2 files changed

+55
-33
lines changed

2 files changed

+55
-33
lines changed

modules/actions/workflows.go

Lines changed: 36 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -335,44 +335,47 @@ func matchIssuesEvent(commit *git.Commit, issuePayload *api.IssuePayload, evt *j
335335
}
336336

337337
func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
338-
// with no special filter parameters
339-
if len(evt.Acts()) == 0 {
338+
acts := evt.Acts()
339+
activityTypeMatched := false
340+
matchTimes := 0
341+
342+
if vals, ok := acts["types"]; !ok {
340343
// defaultly, only pull request `opened`, `reopened` and `synchronized` will trigger workflow
341344
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
342-
return prPayload.Action == api.HookIssueSynchronized || prPayload.Action == api.HookIssueOpened || prPayload.Action == api.HookIssueReOpened
345+
activityTypeMatched = prPayload.Action == api.HookIssueSynchronized || prPayload.Action == api.HookIssueOpened || prPayload.Action == api.HookIssueReOpened
346+
} else {
347+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
348+
// Actions with the same name:
349+
// opened, edited, closed, reopened, assigned, unassigned
350+
// Actions need to be converted:
351+
// synchronized -> synchronize
352+
// label_updated -> labeled
353+
// label_cleared -> unlabeled
354+
// Unsupported activity types:
355+
// converted_to_draft, ready_for_review, locked, unlocked, review_requested, review_request_removed, auto_merge_enabled, auto_merge_disabled
356+
357+
action := prPayload.Action
358+
switch action {
359+
case api.HookIssueSynchronized:
360+
action = "synchronize"
361+
case api.HookIssueLabelUpdated:
362+
action = "labeled"
363+
case api.HookIssueLabelCleared:
364+
action = "unlabeled"
365+
}
366+
log.Trace("matching pull_request %s with %v", action, vals)
367+
for _, val := range vals {
368+
if glob.MustCompile(val, '/').Match(string(action)) {
369+
activityTypeMatched = true
370+
matchTimes++
371+
break
372+
}
373+
}
343374
}
344375

345-
matchTimes := 0
346376
// all acts conditions should be satisfied
347-
for cond, vals := range evt.Acts() {
377+
for cond, vals := range acts {
348378
switch cond {
349-
case "types":
350-
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
351-
// Actions with the same name:
352-
// opened, edited, closed, reopened, assigned, unassigned
353-
// Actions need to be converted:
354-
// synchronized -> synchronize
355-
// label_updated -> labeled
356-
// label_cleared -> unlabeled
357-
// Unsupported activity types:
358-
// converted_to_draft, ready_for_review, locked, unlocked, review_requested, review_request_removed, auto_merge_enabled, auto_merge_disabled
359-
360-
action := prPayload.Action
361-
switch action {
362-
case api.HookIssueSynchronized:
363-
action = "synchronize"
364-
case api.HookIssueLabelUpdated:
365-
action = "labeled"
366-
case api.HookIssueLabelCleared:
367-
action = "unlabeled"
368-
}
369-
log.Trace("matching pull_request %s with %v", action, vals)
370-
for _, val := range vals {
371-
if glob.MustCompile(val, '/').Match(string(action)) {
372-
matchTimes++
373-
break
374-
}
375-
}
376379
case "branches":
377380
refName := git.RefName(prPayload.PullRequest.Base.Ref)
378381
patterns, err := workflowpattern.CompilePatterns(vals...)
@@ -421,7 +424,7 @@ func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload
421424
log.Warn("pull request event unsupported condition %q", cond)
422425
}
423426
}
424-
return matchTimes == len(evt.Acts())
427+
return activityTypeMatched && matchTimes == len(evt.Acts())
425428
}
426429

427430
func matchIssueCommentEvent(commit *git.Commit, issueCommentPayload *api.IssueCommentPayload, evt *jobparser.Event) bool {

modules/actions/workflows_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,25 @@ func TestDetectMatched(t *testing.T) {
5757
yamlOn: "on: pull_request",
5858
expected: false,
5959
},
60+
{
61+
desc: "HookEventPullRequest(pull_request) `closed` action doesn't match GithubEventPullRequest(pull_request) with no activity type",
62+
triggedEvent: webhook_module.HookEventPullRequest,
63+
payload: &api.PullRequestPayload{Action: api.HookIssueClosed},
64+
yamlOn: "on: pull_request",
65+
expected: false,
66+
},
67+
{
68+
desc: "HookEventPullRequest(pull_request) `closed` action doesn't match GithubEventPullRequest(pull_request) with branches",
69+
triggedEvent: webhook_module.HookEventPullRequest,
70+
payload: &api.PullRequestPayload{
71+
Action: api.HookIssueClosed,
72+
PullRequest: &api.PullRequest{
73+
Base: &api.PRBranchInfo{},
74+
},
75+
},
76+
yamlOn: "on:\n pull_request:\n branches: [main]",
77+
expected: false,
78+
},
6079
{
6180
desc: "HookEventPullRequest(pull_request) `label_updated` action matches GithubEventPullRequest(pull_request) with `label` activity type",
6281
triggedEvent: webhook_module.HookEventPullRequest,

0 commit comments

Comments
 (0)