Skip to content

Commit c1a10be

Browse files
authored
Fix activity type match in matchPullRequestEvent (#25746) (#25796)
Backport #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.
1 parent 2b79d3f commit c1a10be

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
@@ -321,44 +321,47 @@ func matchIssuesEvent(commit *git.Commit, issuePayload *api.IssuePayload, evt *j
321321
}
322322

323323
func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
324-
// with no special filter parameters
325-
if len(evt.Acts()) == 0 {
324+
acts := evt.Acts()
325+
activityTypeMatched := false
326+
matchTimes := 0
327+
328+
if vals, ok := acts["types"]; !ok {
326329
// defaultly, only pull request `opened`, `reopened` and `synchronized` will trigger workflow
327330
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
328-
return prPayload.Action == api.HookIssueSynchronized || prPayload.Action == api.HookIssueOpened || prPayload.Action == api.HookIssueReOpened
331+
activityTypeMatched = prPayload.Action == api.HookIssueSynchronized || prPayload.Action == api.HookIssueOpened || prPayload.Action == api.HookIssueReOpened
332+
} else {
333+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
334+
// Actions with the same name:
335+
// opened, edited, closed, reopened, assigned, unassigned
336+
// Actions need to be converted:
337+
// synchronized -> synchronize
338+
// label_updated -> labeled
339+
// label_cleared -> unlabeled
340+
// Unsupported activity types:
341+
// converted_to_draft, ready_for_review, locked, unlocked, review_requested, review_request_removed, auto_merge_enabled, auto_merge_disabled
342+
343+
action := prPayload.Action
344+
switch action {
345+
case api.HookIssueSynchronized:
346+
action = "synchronize"
347+
case api.HookIssueLabelUpdated:
348+
action = "labeled"
349+
case api.HookIssueLabelCleared:
350+
action = "unlabeled"
351+
}
352+
log.Trace("matching pull_request %s with %v", action, vals)
353+
for _, val := range vals {
354+
if glob.MustCompile(val, '/').Match(string(action)) {
355+
activityTypeMatched = true
356+
matchTimes++
357+
break
358+
}
359+
}
329360
}
330361

331-
matchTimes := 0
332362
// all acts conditions should be satisfied
333-
for cond, vals := range evt.Acts() {
363+
for cond, vals := range acts {
334364
switch cond {
335-
case "types":
336-
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
337-
// Actions with the same name:
338-
// opened, edited, closed, reopened, assigned, unassigned
339-
// Actions need to be converted:
340-
// synchronized -> synchronize
341-
// label_updated -> labeled
342-
// label_cleared -> unlabeled
343-
// Unsupported activity types:
344-
// converted_to_draft, ready_for_review, locked, unlocked, review_requested, review_request_removed, auto_merge_enabled, auto_merge_disabled
345-
346-
action := prPayload.Action
347-
switch action {
348-
case api.HookIssueSynchronized:
349-
action = "synchronize"
350-
case api.HookIssueLabelUpdated:
351-
action = "labeled"
352-
case api.HookIssueLabelCleared:
353-
action = "unlabeled"
354-
}
355-
log.Trace("matching pull_request %s with %v", action, vals)
356-
for _, val := range vals {
357-
if glob.MustCompile(val, '/').Match(string(action)) {
358-
matchTimes++
359-
break
360-
}
361-
}
362365
case "branches":
363366
refName := git.RefName(prPayload.PullRequest.Base.Ref)
364367
patterns, err := workflowpattern.CompilePatterns(vals...)
@@ -407,7 +410,7 @@ func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload
407410
log.Warn("pull request event unsupported condition %q", cond)
408411
}
409412
}
410-
return matchTimes == len(evt.Acts())
413+
return activityTypeMatched && matchTimes == len(evt.Acts())
411414
}
412415

413416
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)