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)
2023-04-13 00:16:47 +08:00 · 2023-04-13 00:16:47 +08:00 · 2d91afaa92
commit 2d91afaa92
parent 53e324fd80
2 changed files with 362 additions and 25 deletions
--- 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())
--- a/modules/actions/workflows_test.go
+++ 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]))
+		})
+	}
+}