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.
This commit is contained in:
Zettat123 2023-07-11 14:42:07 +08:00 committed by GitHub
parent 2b79d3fd52
commit c1a10be07e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 55 additions and 33 deletions

View file

@ -321,44 +321,47 @@ func matchIssuesEvent(commit *git.Commit, issuePayload *api.IssuePayload, evt *j
} }
func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool { func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
// with no special filter parameters acts := evt.Acts()
if len(evt.Acts()) == 0 { activityTypeMatched := false
matchTimes := 0
if vals, ok := acts["types"]; !ok {
// defaultly, only pull request `opened`, `reopened` and `synchronized` will trigger workflow // defaultly, only pull request `opened`, `reopened` and `synchronized` will trigger workflow
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
return prPayload.Action == api.HookIssueSynchronized || prPayload.Action == api.HookIssueOpened || prPayload.Action == api.HookIssueReOpened activityTypeMatched = prPayload.Action == api.HookIssueSynchronized || prPayload.Action == api.HookIssueOpened || prPayload.Action == api.HookIssueReOpened
} else {
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
// Actions with the same name:
// opened, edited, closed, reopened, assigned, unassigned
// Actions need to be converted:
// synchronized -> synchronize
// label_updated -> labeled
// label_cleared -> unlabeled
// Unsupported activity types:
// converted_to_draft, ready_for_review, locked, unlocked, review_requested, review_request_removed, auto_merge_enabled, auto_merge_disabled
action := prPayload.Action
switch action {
case api.HookIssueSynchronized:
action = "synchronize"
case api.HookIssueLabelUpdated:
action = "labeled"
case api.HookIssueLabelCleared:
action = "unlabeled"
}
log.Trace("matching pull_request %s with %v", action, vals)
for _, val := range vals {
if glob.MustCompile(val, '/').Match(string(action)) {
activityTypeMatched = true
matchTimes++
break
}
}
} }
matchTimes := 0
// all acts conditions should be satisfied // all acts conditions should be satisfied
for cond, vals := range evt.Acts() { for cond, vals := range acts {
switch cond { switch cond {
case "types":
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
// Actions with the same name:
// opened, edited, closed, reopened, assigned, unassigned
// Actions need to be converted:
// synchronized -> synchronize
// label_updated -> labeled
// label_cleared -> unlabeled
// Unsupported activity types:
// converted_to_draft, ready_for_review, locked, unlocked, review_requested, review_request_removed, auto_merge_enabled, auto_merge_disabled
action := prPayload.Action
switch action {
case api.HookIssueSynchronized:
action = "synchronize"
case api.HookIssueLabelUpdated:
action = "labeled"
case api.HookIssueLabelCleared:
action = "unlabeled"
}
log.Trace("matching pull_request %s with %v", action, vals)
for _, val := range vals {
if glob.MustCompile(val, '/').Match(string(action)) {
matchTimes++
break
}
}
case "branches": case "branches":
refName := git.RefName(prPayload.PullRequest.Base.Ref) refName := git.RefName(prPayload.PullRequest.Base.Ref)
patterns, err := workflowpattern.CompilePatterns(vals...) patterns, err := workflowpattern.CompilePatterns(vals...)
@ -407,7 +410,7 @@ func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload
log.Warn("pull request event unsupported condition %q", cond) log.Warn("pull request event unsupported condition %q", cond)
} }
} }
return matchTimes == len(evt.Acts()) return activityTypeMatched && matchTimes == len(evt.Acts())
} }
func matchIssueCommentEvent(commit *git.Commit, issueCommentPayload *api.IssueCommentPayload, evt *jobparser.Event) bool { func matchIssueCommentEvent(commit *git.Commit, issueCommentPayload *api.IssueCommentPayload, evt *jobparser.Event) bool {

View file

@ -57,6 +57,25 @@ func TestDetectMatched(t *testing.T) {
yamlOn: "on: pull_request", yamlOn: "on: pull_request",
expected: false, expected: false,
}, },
{
desc: "HookEventPullRequest(pull_request) `closed` action doesn't match GithubEventPullRequest(pull_request) with no activity type",
triggedEvent: webhook_module.HookEventPullRequest,
payload: &api.PullRequestPayload{Action: api.HookIssueClosed},
yamlOn: "on: pull_request",
expected: false,
},
{
desc: "HookEventPullRequest(pull_request) `closed` action doesn't match GithubEventPullRequest(pull_request) with branches",
triggedEvent: webhook_module.HookEventPullRequest,
payload: &api.PullRequestPayload{
Action: api.HookIssueClosed,
PullRequest: &api.PullRequest{
Base: &api.PRBranchInfo{},
},
},
yamlOn: "on:\n pull_request:\n branches: [main]",
expected: false,
},
{ {
desc: "HookEventPullRequest(pull_request) `label_updated` action matches githubEventPullRequest(pull_request) with `label` activity type", desc: "HookEventPullRequest(pull_request) `label_updated` action matches githubEventPullRequest(pull_request) with `label` activity type",
triggedEvent: webhook_module.HookEventPullRequest, triggedEvent: webhook_module.HookEventPullRequest,