aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorZettat123 <zettat123@gmail.com>2023-04-13 00:16:47 +0800
committerGitHub <noreply@github.com>2023-04-12 12:16:47 -0400
commit2d91afaa92609d31b04aea9ad909cec8a574cacd (patch)
treeb5a0aba7d4fc181a84a63707dc8eb657ee039517
parent53e324fd80fe08dc78f58fa4e0d98e02974cd18f (diff)
downloadgitea-2d91afaa92609d31b04aea9ad909cec8a574cacd.tar.gz
gitea-2d91afaa92609d31b04aea9ad909cec8a574cacd.zip
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)
-rw-r--r--modules/actions/workflows.go282
-rw-r--r--modules/actions/workflows_test.go105
2 files changed, 362 insertions, 25 deletions
diff --git a/modules/actions/workflows.go b/modules/actions/workflows.go
index d30982a46b..d21dc1d809 100644
--- a/modules/actions/workflows.go
+++ b/modules/actions/workflows.go
@@ -115,40 +115,60 @@ func detectMatched(commit *git.Commit, triggedEvent webhook_module.HookEventType
}
switch triggedEvent {
- case webhook_module.HookEventCreate,
+ case // events with no activity types
+ webhook_module.HookEventCreate,
webhook_module.HookEventDelete,
webhook_module.HookEventFork,
- webhook_module.HookEventIssueAssign,
- webhook_module.HookEventIssueLabel,
- webhook_module.HookEventIssueMilestone,
- webhook_module.HookEventPullRequestAssign,
- webhook_module.HookEventPullRequestLabel,
- webhook_module.HookEventPullRequestMilestone,
- webhook_module.HookEventPullRequestComment,
- webhook_module.HookEventPullRequestReviewApproved,
- webhook_module.HookEventPullRequestReviewRejected,
- webhook_module.HookEventPullRequestReviewComment,
- webhook_module.HookEventWiki,
- webhook_module.HookEventRepository,
- webhook_module.HookEventRelease,
- webhook_module.HookEventPackage:
+ // FIXME: `wiki` event should match `gollum` event
+ // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#gollum
+ webhook_module.HookEventWiki:
if len(evt.Acts()) != 0 {
log.Warn("Ignore unsupported %s event arguments %v", triggedEvent, evt.Acts())
}
// no special filter parameters for these events, just return true if name matched
return true
- case webhook_module.HookEventPush:
+ case // push
+ webhook_module.HookEventPush:
return matchPushEvent(commit, payload.(*api.PushPayload), evt)
- case webhook_module.HookEventIssues:
+ case // issues
+ webhook_module.HookEventIssues,
+ webhook_module.HookEventIssueAssign,
+ webhook_module.HookEventIssueLabel,
+ webhook_module.HookEventIssueMilestone:
return matchIssuesEvent(commit, payload.(*api.IssuePayload), evt)
- case webhook_module.HookEventPullRequest, webhook_module.HookEventPullRequestSync:
+ case // issue_comment
+ webhook_module.HookEventIssueComment,
+ // `pull_request_comment` is same as `issue_comment`
+ // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_comment-use-issue_comment
+ webhook_module.HookEventPullRequestComment:
+ return matchIssueCommentEvent(commit, payload.(*api.IssueCommentPayload), evt)
+
+ case // pull_request
+ webhook_module.HookEventPullRequest,
+ webhook_module.HookEventPullRequestSync,
+ webhook_module.HookEventPullRequestAssign,
+ webhook_module.HookEventPullRequestLabel:
return matchPullRequestEvent(commit, payload.(*api.PullRequestPayload), evt)
- case webhook_module.HookEventIssueComment:
- return matchIssueCommentEvent(commit, payload.(*api.IssueCommentPayload), evt)
+ case // pull_request_review
+ webhook_module.HookEventPullRequestReviewApproved,
+ webhook_module.HookEventPullRequestReviewRejected:
+ return matchPullRequestReviewEvent(commit, payload.(*api.PullRequestPayload), evt)
+
+ case // pull_request_review_comment
+ webhook_module.HookEventPullRequestReviewComment:
+ return matchPullRequestReviewCommentEvent(commit, payload.(*api.PullRequestPayload), evt)
+
+ case // release
+ webhook_module.HookEventRelease:
+ return matchReleaseEvent(commit, payload.(*api.ReleasePayload), evt)
+
+ case // registry_package
+ webhook_module.HookEventPackage:
+ return matchPackageEvent(commit, payload.(*api.PackagePayload), evt)
default:
log.Warn("unsupported event %q", triggedEvent)
@@ -265,8 +285,24 @@ func matchIssuesEvent(commit *git.Commit, issuePayload *api.IssuePayload, evt *j
for cond, vals := range evt.Acts() {
switch cond {
case "types":
+ // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issues
+ // Actions with the same name:
+ // opened, edited, closed, reopened, assigned, unassigned, milestoned, demilestoned
+ // Actions need to be converted:
+ // label_updated -> labeled
+ // label_cleared -> unlabeled
+ // Unsupported activity types:
+ // deleted, transferred, pinned, unpinned, locked, unlocked
+
+ action := issuePayload.Action
+ switch action {
+ case api.HookIssueLabelUpdated:
+ action = "labeled"
+ case api.HookIssueLabelCleared:
+ action = "unlabeled"
+ }
for _, val := range vals {
- if glob.MustCompile(val, '/').Match(string(issuePayload.Action)) {
+ if glob.MustCompile(val, '/').Match(string(action)) {
matchTimes++
break
}
@@ -281,8 +317,9 @@ func matchIssuesEvent(commit *git.Commit, issuePayload *api.IssuePayload, evt *j
func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
// with no special filter parameters
if len(evt.Acts()) == 0 {
- // defaultly, only pull request opened and synchronized will trigger workflow
- return prPayload.Action == api.HookIssueSynchronized || prPayload.Action == api.HookIssueOpened
+ // 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
+ return prPayload.Action == api.HookIssueSynchronized || prPayload.Action == api.HookIssueOpened || prPayload.Action == api.HookIssueReOpened
}
matchTimes := 0
@@ -290,9 +327,24 @@ func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload
for cond, vals := range evt.Acts() {
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
- if prPayload.Action == api.HookIssueSynchronized {
+ 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 {
@@ -363,6 +415,14 @@ func matchIssueCommentEvent(commit *git.Commit, issueCommentPayload *api.IssueCo
for cond, vals := range evt.Acts() {
switch cond {
case "types":
+ // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issue_comment
+ // Actions with the same name:
+ // created, edited, deleted
+ // Actions need to be converted:
+ // NONE
+ // Unsupported activity types:
+ // NONE
+
for _, val := range vals {
if glob.MustCompile(val, '/').Match(string(issueCommentPayload.Action)) {
matchTimes++
@@ -370,7 +430,179 @@ func matchIssueCommentEvent(commit *git.Commit, issueCommentPayload *api.IssueCo
}
}
default:
- log.Warn("issue comment unsupported condition %q", cond)
+ log.Warn("issue comment event unsupported condition %q", cond)
+ }
+ }
+ return matchTimes == len(evt.Acts())
+}
+
+func matchPullRequestReviewEvent(commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
+ // with no special filter parameters
+ if len(evt.Acts()) == 0 {
+ return true
+ }
+
+ matchTimes := 0
+ // all acts conditions should be satisfied
+ for cond, vals := range evt.Acts() {
+ switch cond {
+ case "types":
+ // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_review
+ // Activity types with the same name:
+ // NONE
+ // Activity types need to be converted:
+ // reviewed -> submitted
+ // reviewed -> edited
+ // Unsupported activity types:
+ // dismissed
+
+ actions := make([]string, 0)
+ if prPayload.Action == api.HookIssueReviewed {
+ // the `reviewed` HookIssueAction can match the two activity types: `submitted` and `edited`
+ // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_review
+ actions = append(actions, "submitted", "edited")
+ }
+
+ matched := false
+ for _, val := range vals {
+ for _, action := range actions {
+ if glob.MustCompile(val, '/').Match(action) {
+ matched = true
+ break
+ }
+ }
+ if matched {
+ break
+ }
+ }
+ if matched {
+ matchTimes++
+ }
+ default:
+ log.Warn("pull request review event unsupported condition %q", cond)
+ }
+ }
+ return matchTimes == len(evt.Acts())
+}
+
+func matchPullRequestReviewCommentEvent(commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
+ // with no special filter parameters
+ if len(evt.Acts()) == 0 {
+ return true
+ }
+
+ matchTimes := 0
+ // all acts conditions should be satisfied
+ for cond, vals := range evt.Acts() {
+ switch cond {
+ case "types":
+ // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_review_comment
+ // Activity types with the same name:
+ // NONE
+ // Activity types need to be converted:
+ // reviewed -> created
+ // reviewed -> edited
+ // Unsupported activity types:
+ // deleted
+
+ actions := make([]string, 0)
+ if prPayload.Action == api.HookIssueReviewed {
+ // the `reviewed` HookIssueAction can match the two activity types: `created` and `edited`
+ // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_review_comment
+ actions = append(actions, "created", "edited")
+ }
+
+ matched := false
+ for _, val := range vals {
+ for _, action := range actions {
+ if glob.MustCompile(val, '/').Match(action) {
+ matched = true
+ break
+ }
+ }
+ if matched {
+ break
+ }
+ }
+ if matched {
+ matchTimes++
+ }
+ default:
+ log.Warn("pull request review comment event unsupported condition %q", cond)
+ }
+ }
+ return matchTimes == len(evt.Acts())
+}
+
+func matchReleaseEvent(commit *git.Commit, payload *api.ReleasePayload, evt *jobparser.Event) bool {
+ // with no special filter parameters
+ if len(evt.Acts()) == 0 {
+ return true
+ }
+
+ matchTimes := 0
+ // all acts conditions should be satisfied
+ for cond, vals := range evt.Acts() {
+ switch cond {
+ case "types":
+ // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#release
+ // Activity types with the same name:
+ // published
+ // Activity types need to be converted:
+ // updated -> edited
+ // Unsupported activity types:
+ // unpublished, created, deleted, prereleased, released
+
+ action := payload.Action
+ switch action {
+ case api.HookReleaseUpdated:
+ action = "edited"
+ }
+ for _, val := range vals {
+ if glob.MustCompile(val, '/').Match(string(action)) {
+ matchTimes++
+ break
+ }
+ }
+ default:
+ log.Warn("release event unsupported condition %q", cond)
+ }
+ }
+ return matchTimes == len(evt.Acts())
+}
+
+func matchPackageEvent(commit *git.Commit, payload *api.PackagePayload, evt *jobparser.Event) bool {
+ // with no special filter parameters
+ if len(evt.Acts()) == 0 {
+ return true
+ }
+
+ matchTimes := 0
+ // all acts conditions should be satisfied
+ for cond, vals := range evt.Acts() {
+ switch cond {
+ case "types":
+ // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#registry_package
+ // Activity types with the same name:
+ // NONE
+ // Activity types need to be converted:
+ // created -> published
+ // Unsupported activity types:
+ // updated
+
+ action := payload.Action
+ switch action {
+ case api.HookPackageCreated:
+ action = "published"
+ }
+ for _, val := range vals {
+ if glob.MustCompile(val, '/').Match(string(action)) {
+ matchTimes++
+ break
+ }
+ }
+ default:
+ log.Warn("package event unsupported condition %q", cond)
}
}
return matchTimes == len(evt.Acts())
diff --git a/modules/actions/workflows_test.go b/modules/actions/workflows_test.go
new file mode 100644
index 0000000000..6724abafd8
--- /dev/null
+++ b/modules/actions/workflows_test.go
@@ -0,0 +1,105 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package actions
+
+import (
+ "testing"
+
+ "code.gitea.io/gitea/modules/git"
+ api "code.gitea.io/gitea/modules/structs"
+ webhook_module "code.gitea.io/gitea/modules/webhook"
+
+ "github.com/stretchr/testify/assert"
+)
+
+func TestDetectMatched(t *testing.T) {
+ testCases := []struct {
+ desc string
+ commit *git.Commit
+ triggedEvent webhook_module.HookEventType
+ payload api.Payloader
+ yamlOn string
+ expected bool
+ }{
+ {
+ desc: "HookEventCreate(create) matches githubEventCreate(create)",
+ triggedEvent: webhook_module.HookEventCreate,
+ payload: nil,
+ yamlOn: "on: create",
+ expected: true,
+ },
+ {
+ desc: "HookEventIssues(issues) `opened` action matches githubEventIssues(issues)",
+ triggedEvent: webhook_module.HookEventIssues,
+ payload: &api.IssuePayload{Action: api.HookIssueOpened},
+ yamlOn: "on: issues",
+ expected: true,
+ },
+ {
+ desc: "HookEventIssues(issues) `milestoned` action matches githubEventIssues(issues)",
+ triggedEvent: webhook_module.HookEventIssues,
+ payload: &api.IssuePayload{Action: api.HookIssueMilestoned},
+ yamlOn: "on: issues",
+ expected: true,
+ },
+ {
+ desc: "HookEventPullRequestSync(pull_request_sync) matches githubEventPullRequest(pull_request)",
+ triggedEvent: webhook_module.HookEventPullRequestSync,
+ payload: &api.PullRequestPayload{Action: api.HookIssueSynchronized},
+ yamlOn: "on: pull_request",
+ expected: true,
+ },
+ {
+ desc: "HookEventPullRequest(pull_request) `label_updated` action doesn't match githubEventPullRequest(pull_request) with no activity type",
+ triggedEvent: webhook_module.HookEventPullRequest,
+ payload: &api.PullRequestPayload{Action: api.HookIssueLabelUpdated},
+ yamlOn: "on: pull_request",
+ expected: false,
+ },
+ {
+ desc: "HookEventPullRequest(pull_request) `label_updated` action matches githubEventPullRequest(pull_request) with `label` activity type",
+ triggedEvent: webhook_module.HookEventPullRequest,
+ payload: &api.PullRequestPayload{Action: api.HookIssueLabelUpdated},
+ yamlOn: "on:\n pull_request:\n types: [labeled]",
+ expected: true,
+ },
+ {
+ desc: "HookEventPullRequestReviewComment(pull_request_review_comment) matches githubEventPullRequestReviewComment(pull_request_review_comment)",
+ triggedEvent: webhook_module.HookEventPullRequestReviewComment,
+ payload: &api.PullRequestPayload{Action: api.HookIssueReviewed},
+ yamlOn: "on:\n pull_request_review_comment:\n types: [created]",
+ expected: true,
+ },
+ {
+ desc: "HookEventPullRequestReviewRejected(pull_request_review_rejected) doesn't match githubEventPullRequestReview(pull_request_review) with `dismissed` activity type (we don't support `dismissed` at present)",
+ triggedEvent: webhook_module.HookEventPullRequestReviewRejected,
+ payload: &api.PullRequestPayload{Action: api.HookIssueReviewed},
+ yamlOn: "on:\n pull_request_review:\n types: [dismissed]",
+ expected: false,
+ },
+ {
+ desc: "HookEventRelease(release) `published` action matches githubEventRelease(release) with `published` activity type",
+ triggedEvent: webhook_module.HookEventRelease,
+ payload: &api.ReleasePayload{Action: api.HookReleasePublished},
+ yamlOn: "on:\n release:\n types: [published]",
+ expected: true,
+ },
+ {
+ desc: "HookEventPackage(package) `created` action doesn't match githubEventRegistryPackage(registry_package) with `updated` activity type",
+ triggedEvent: webhook_module.HookEventPackage,
+ payload: &api.PackagePayload{Action: api.HookPackageCreated},
+ yamlOn: "on:\n registry_package:\n types: [updated]",
+ expected: false,
+ },
+ }
+
+ for _, tc := range testCases {
+ t.Run(tc.desc, func(t *testing.T) {
+ evts, err := GetEventsFromContent([]byte(tc.yamlOn))
+ assert.NoError(t, err)
+ assert.Len(t, evts, 1)
+ assert.Equal(t, tc.expected, detectMatched(tc.commit, tc.triggedEvent, tc.payload, evts[0]))
+ })
+ }
+}