Skip to content

Commit 2d91afa

Browse files
authored
Fix mismatch between hook events and github event types (#24048)
Some workflow trigger events can have multiple activity types, such as `issues` and `pull_request`, and user can specify which types can trigger the workflow. See GitHub documentation: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows Now some hook events cannot match the workflow trigger events correctly because we don't check the activity types. For example, `pull_request_label` is an individual hook event. But there isn't a `pull_request_label` workflow trigger event, we can only use `pull_request` event's `label` activity type. If we don't check the activity types, the workflows without the `label` activity type may be triggered by the `pull_request_label` event by mistake. We need to improve the match logic. - [x] [`issues` ](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issues) - [x] [`issue_comment`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issue_comment) - [x] [`pull_request`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request) - [x] [`pull_request_review`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_review) - [x] [`pull_request_review_comment`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_review_comment) - [x] [`release`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#release) - [x] [`registry_package`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#registry_package)
1 parent 53e324f commit 2d91afa

File tree

2 files changed

+362
-25
lines changed

2 files changed

+362
-25
lines changed

modules/actions/workflows.go

Lines changed: 257 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -115,40 +115,60 @@ func detectMatched(commit *git.Commit, triggedEvent webhook_module.HookEventType
115115
}
116116

117117
switch triggedEvent {
118-
case webhook_module.HookEventCreate,
118+
case // events with no activity types
119+
webhook_module.HookEventCreate,
119120
webhook_module.HookEventDelete,
120121
webhook_module.HookEventFork,
121-
webhook_module.HookEventIssueAssign,
122-
webhook_module.HookEventIssueLabel,
123-
webhook_module.HookEventIssueMilestone,
124-
webhook_module.HookEventPullRequestAssign,
125-
webhook_module.HookEventPullRequestLabel,
126-
webhook_module.HookEventPullRequestMilestone,
127-
webhook_module.HookEventPullRequestComment,
128-
webhook_module.HookEventPullRequestReviewApproved,
129-
webhook_module.HookEventPullRequestReviewRejected,
130-
webhook_module.HookEventPullRequestReviewComment,
131-
webhook_module.HookEventWiki,
132-
webhook_module.HookEventRepository,
133-
webhook_module.HookEventRelease,
134-
webhook_module.HookEventPackage:
122+
// FIXME: `wiki` event should match `gollum` event
123+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#gollum
124+
webhook_module.HookEventWiki:
135125
if len(evt.Acts()) != 0 {
136126
log.Warn("Ignore unsupported %s event arguments %v", triggedEvent, evt.Acts())
137127
}
138128
// no special filter parameters for these events, just return true if name matched
139129
return true
140130

141-
case webhook_module.HookEventPush:
131+
case // push
132+
webhook_module.HookEventPush:
142133
return matchPushEvent(commit, payload.(*api.PushPayload), evt)
143134

144-
case webhook_module.HookEventIssues:
135+
case // issues
136+
webhook_module.HookEventIssues,
137+
webhook_module.HookEventIssueAssign,
138+
webhook_module.HookEventIssueLabel,
139+
webhook_module.HookEventIssueMilestone:
145140
return matchIssuesEvent(commit, payload.(*api.IssuePayload), evt)
146141

147-
case webhook_module.HookEventPullRequest, webhook_module.HookEventPullRequestSync:
142+
case // issue_comment
143+
webhook_module.HookEventIssueComment,
144+
// `pull_request_comment` is same as `issue_comment`
145+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_comment-use-issue_comment
146+
webhook_module.HookEventPullRequestComment:
147+
return matchIssueCommentEvent(commit, payload.(*api.IssueCommentPayload), evt)
148+
149+
case // pull_request
150+
webhook_module.HookEventPullRequest,
151+
webhook_module.HookEventPullRequestSync,
152+
webhook_module.HookEventPullRequestAssign,
153+
webhook_module.HookEventPullRequestLabel:
148154
return matchPullRequestEvent(commit, payload.(*api.PullRequestPayload), evt)
149155

150-
case webhook_module.HookEventIssueComment:
151-
return matchIssueCommentEvent(commit, payload.(*api.IssueCommentPayload), evt)
156+
case // pull_request_review
157+
webhook_module.HookEventPullRequestReviewApproved,
158+
webhook_module.HookEventPullRequestReviewRejected:
159+
return matchPullRequestReviewEvent(commit, payload.(*api.PullRequestPayload), evt)
160+
161+
case // pull_request_review_comment
162+
webhook_module.HookEventPullRequestReviewComment:
163+
return matchPullRequestReviewCommentEvent(commit, payload.(*api.PullRequestPayload), evt)
164+
165+
case // release
166+
webhook_module.HookEventRelease:
167+
return matchReleaseEvent(commit, payload.(*api.ReleasePayload), evt)
168+
169+
case // registry_package
170+
webhook_module.HookEventPackage:
171+
return matchPackageEvent(commit, payload.(*api.PackagePayload), evt)
152172

153173
default:
154174
log.Warn("unsupported event %q", triggedEvent)
@@ -265,8 +285,24 @@ func matchIssuesEvent(commit *git.Commit, issuePayload *api.IssuePayload, evt *j
265285
for cond, vals := range evt.Acts() {
266286
switch cond {
267287
case "types":
288+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issues
289+
// Actions with the same name:
290+
// opened, edited, closed, reopened, assigned, unassigned, milestoned, demilestoned
291+
// Actions need to be converted:
292+
// label_updated -> labeled
293+
// label_cleared -> unlabeled
294+
// Unsupported activity types:
295+
// deleted, transferred, pinned, unpinned, locked, unlocked
296+
297+
action := issuePayload.Action
298+
switch action {
299+
case api.HookIssueLabelUpdated:
300+
action = "labeled"
301+
case api.HookIssueLabelCleared:
302+
action = "unlabeled"
303+
}
268304
for _, val := range vals {
269-
if glob.MustCompile(val, '/').Match(string(issuePayload.Action)) {
305+
if glob.MustCompile(val, '/').Match(string(action)) {
270306
matchTimes++
271307
break
272308
}
@@ -281,18 +317,34 @@ func matchIssuesEvent(commit *git.Commit, issuePayload *api.IssuePayload, evt *j
281317
func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
282318
// with no special filter parameters
283319
if len(evt.Acts()) == 0 {
284-
// defaultly, only pull request opened and synchronized will trigger workflow
285-
return prPayload.Action == api.HookIssueSynchronized || prPayload.Action == api.HookIssueOpened
320+
// defaultly, only pull request `opened`, `reopened` and `synchronized` will trigger workflow
321+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
322+
return prPayload.Action == api.HookIssueSynchronized || prPayload.Action == api.HookIssueOpened || prPayload.Action == api.HookIssueReOpened
286323
}
287324

288325
matchTimes := 0
289326
// all acts conditions should be satisfied
290327
for cond, vals := range evt.Acts() {
291328
switch cond {
292329
case "types":
330+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
331+
// Actions with the same name:
332+
// opened, edited, closed, reopened, assigned, unassigned
333+
// Actions need to be converted:
334+
// synchronized -> synchronize
335+
// label_updated -> labeled
336+
// label_cleared -> unlabeled
337+
// Unsupported activity types:
338+
// converted_to_draft, ready_for_review, locked, unlocked, review_requested, review_request_removed, auto_merge_enabled, auto_merge_disabled
339+
293340
action := prPayload.Action
294-
if prPayload.Action == api.HookIssueSynchronized {
341+
switch action {
342+
case api.HookIssueSynchronized:
295343
action = "synchronize"
344+
case api.HookIssueLabelUpdated:
345+
action = "labeled"
346+
case api.HookIssueLabelCleared:
347+
action = "unlabeled"
296348
}
297349
log.Trace("matching pull_request %s with %v", action, vals)
298350
for _, val := range vals {
@@ -363,14 +415,194 @@ func matchIssueCommentEvent(commit *git.Commit, issueCommentPayload *api.IssueCo
363415
for cond, vals := range evt.Acts() {
364416
switch cond {
365417
case "types":
418+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issue_comment
419+
// Actions with the same name:
420+
// created, edited, deleted
421+
// Actions need to be converted:
422+
// NONE
423+
// Unsupported activity types:
424+
// NONE
425+
366426
for _, val := range vals {
367427
if glob.MustCompile(val, '/').Match(string(issueCommentPayload.Action)) {
368428
matchTimes++
369429
break
370430
}
371431
}
372432
default:
373-
log.Warn("issue comment unsupported condition %q", cond)
433+
log.Warn("issue comment event unsupported condition %q", cond)
434+
}
435+
}
436+
return matchTimes == len(evt.Acts())
437+
}
438+
439+
func matchPullRequestReviewEvent(commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
440+
// with no special filter parameters
441+
if len(evt.Acts()) == 0 {
442+
return true
443+
}
444+
445+
matchTimes := 0
446+
// all acts conditions should be satisfied
447+
for cond, vals := range evt.Acts() {
448+
switch cond {
449+
case "types":
450+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_review
451+
// Activity types with the same name:
452+
// NONE
453+
// Activity types need to be converted:
454+
// reviewed -> submitted
455+
// reviewed -> edited
456+
// Unsupported activity types:
457+
// dismissed
458+
459+
actions := make([]string, 0)
460+
if prPayload.Action == api.HookIssueReviewed {
461+
// the `reviewed` HookIssueAction can match the two activity types: `submitted` and `edited`
462+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_review
463+
actions = append(actions, "submitted", "edited")
464+
}
465+
466+
matched := false
467+
for _, val := range vals {
468+
for _, action := range actions {
469+
if glob.MustCompile(val, '/').Match(action) {
470+
matched = true
471+
break
472+
}
473+
}
474+
if matched {
475+
break
476+
}
477+
}
478+
if matched {
479+
matchTimes++
480+
}
481+
default:
482+
log.Warn("pull request review event unsupported condition %q", cond)
483+
}
484+
}
485+
return matchTimes == len(evt.Acts())
486+
}
487+
488+
func matchPullRequestReviewCommentEvent(commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
489+
// with no special filter parameters
490+
if len(evt.Acts()) == 0 {
491+
return true
492+
}
493+
494+
matchTimes := 0
495+
// all acts conditions should be satisfied
496+
for cond, vals := range evt.Acts() {
497+
switch cond {
498+
case "types":
499+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_review_comment
500+
// Activity types with the same name:
501+
// NONE
502+
// Activity types need to be converted:
503+
// reviewed -> created
504+
// reviewed -> edited
505+
// Unsupported activity types:
506+
// deleted
507+
508+
actions := make([]string, 0)
509+
if prPayload.Action == api.HookIssueReviewed {
510+
// the `reviewed` HookIssueAction can match the two activity types: `created` and `edited`
511+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_review_comment
512+
actions = append(actions, "created", "edited")
513+
}
514+
515+
matched := false
516+
for _, val := range vals {
517+
for _, action := range actions {
518+
if glob.MustCompile(val, '/').Match(action) {
519+
matched = true
520+
break
521+
}
522+
}
523+
if matched {
524+
break
525+
}
526+
}
527+
if matched {
528+
matchTimes++
529+
}
530+
default:
531+
log.Warn("pull request review comment event unsupported condition %q", cond)
532+
}
533+
}
534+
return matchTimes == len(evt.Acts())
535+
}
536+
537+
func matchReleaseEvent(commit *git.Commit, payload *api.ReleasePayload, evt *jobparser.Event) bool {
538+
// with no special filter parameters
539+
if len(evt.Acts()) == 0 {
540+
return true
541+
}
542+
543+
matchTimes := 0
544+
// all acts conditions should be satisfied
545+
for cond, vals := range evt.Acts() {
546+
switch cond {
547+
case "types":
548+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#release
549+
// Activity types with the same name:
550+
// published
551+
// Activity types need to be converted:
552+
// updated -> edited
553+
// Unsupported activity types:
554+
// unpublished, created, deleted, prereleased, released
555+
556+
action := payload.Action
557+
switch action {
558+
case api.HookReleaseUpdated:
559+
action = "edited"
560+
}
561+
for _, val := range vals {
562+
if glob.MustCompile(val, '/').Match(string(action)) {
563+
matchTimes++
564+
break
565+
}
566+
}
567+
default:
568+
log.Warn("release event unsupported condition %q", cond)
569+
}
570+
}
571+
return matchTimes == len(evt.Acts())
572+
}
573+
574+
func matchPackageEvent(commit *git.Commit, payload *api.PackagePayload, evt *jobparser.Event) bool {
575+
// with no special filter parameters
576+
if len(evt.Acts()) == 0 {
577+
return true
578+
}
579+
580+
matchTimes := 0
581+
// all acts conditions should be satisfied
582+
for cond, vals := range evt.Acts() {
583+
switch cond {
584+
case "types":
585+
// See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#registry_package
586+
// Activity types with the same name:
587+
// NONE
588+
// Activity types need to be converted:
589+
// created -> published
590+
// Unsupported activity types:
591+
// updated
592+
593+
action := payload.Action
594+
switch action {
595+
case api.HookPackageCreated:
596+
action = "published"
597+
}
598+
for _, val := range vals {
599+
if glob.MustCompile(val, '/').Match(string(action)) {
600+
matchTimes++
601+
break
602+
}
603+
}
604+
default:
605+
log.Warn("package event unsupported condition %q", cond)
374606
}
375607
}
376608
return matchTimes == len(evt.Acts())

0 commit comments

Comments
 (0)