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:
parent
2b79d3fd52
commit
c1a10be07e
2 changed files with 55 additions and 33 deletions
|
@ -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 {
|
||||||
|
|
|
@ -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,
|
||||||
|
|
Loading…
Reference in a new issue