Skip to content
/ gitea Public
  • Sponsor go-gitea/gitea

  • Notifications You must be signed in to change notification settings
  • Fork 5.8k

Add tests for webhook and fix some webhook bugs (#33396) #33442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 2, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions models/webhook/webhook.go
Original file line number Diff line number Diff line change
@@ -299,6 +299,11 @@ func (w *Webhook) HasPackageEvent() bool {
(w.ChooseEvents && w.HookEvents.Package)
}

func (w *Webhook) HasStatusEvent() bool {
return w.SendEverything ||
(w.ChooseEvents && w.HookEvents.Status)
}

// HasPullRequestReviewRequestEvent returns true if hook enabled pull request review request event.
func (w *Webhook) HasPullRequestReviewRequestEvent() bool {
return w.SendEverything ||
@@ -337,6 +342,7 @@ func (w *Webhook) EventCheckers() []struct {
{w.HasReleaseEvent, webhook_module.HookEventRelease},
{w.HasPackageEvent, webhook_module.HookEventPackage},
{w.HasPullRequestReviewRequestEvent, webhook_module.HookEventPullRequestReviewRequest},
{w.HasStatusEvent, webhook_module.HookEventStatus},
}
}

2 changes: 1 addition & 1 deletion models/webhook/webhook_test.go
Original file line number Diff line number Diff line change
@@ -74,7 +74,7 @@ func TestWebhook_EventsArray(t *testing.T) {
"pull_request", "pull_request_assign", "pull_request_label", "pull_request_milestone",
"pull_request_comment", "pull_request_review_approved", "pull_request_review_rejected",
"pull_request_review_comment", "pull_request_sync", "wiki", "repository", "release",
"package", "pull_request_review_request",
"package", "pull_request_review_request", "status",
},
(&Webhook{
HookEvent: &webhook_module.HookEvent{SendEverything: true},
60 changes: 2 additions & 58 deletions modules/structs/hook.go
Original file line number Diff line number Diff line change
@@ -116,14 +116,7 @@ var (
_ Payloader = &PackagePayload{}
)

// _________ __
// \_ ___ \_______ ____ _____ _/ |_ ____
// / \ \/\_ __ \_/ __ \\__ \\ __\/ __ \
// \ \____| | \/\ ___/ / __ \| | \ ___/
// \______ /|__| \___ >____ /__| \___ >
// \/ \/ \/ \/

// CreatePayload FIXME
// CreatePayload represents a payload information of create event.
type CreatePayload struct {
Sha string `json:"sha"`
Ref string `json:"ref"`
@@ -157,13 +150,6 @@ func ParseCreateHook(raw []byte) (*CreatePayload, error) {
return hook, nil
}

// ________ .__ __
// \______ \ ____ | | _____/ |_ ____
// | | \_/ __ \| | _/ __ \ __\/ __ \
// | ` \ ___/| |_\ ___/| | \ ___/
// /_______ /\___ >____/\___ >__| \___ >
// \/ \/ \/ \/

// PusherType define the type to push
type PusherType string

@@ -186,13 +172,6 @@ func (p *DeletePayload) JSONPayload() ([]byte, error) {
return json.MarshalIndent(p, "", " ")
}

// ___________ __
// \_ _____/__________| | __
// | __)/ _ \_ __ \ |/ /
// | \( <_> ) | \/ <
// \___ / \____/|__| |__|_ \
// \/ \/

// ForkPayload represents fork payload
type ForkPayload struct {
Forkee *Repository `json:"forkee"`
@@ -232,13 +211,6 @@ func (p *IssueCommentPayload) JSONPayload() ([]byte, error) {
return json.MarshalIndent(p, "", " ")
}

// __________ .__
// \______ \ ____ | | ____ _____ ______ ____
// | _// __ \| | _/ __ \\__ \ / ___// __ \
// | | \ ___/| |_\ ___/ / __ \_\___ \\ ___/
// |____|_ /\___ >____/\___ >____ /____ >\___ >
// \/ \/ \/ \/ \/ \/

// HookReleaseAction defines hook release action type
type HookReleaseAction string

@@ -302,13 +274,6 @@ func (p *PushPayload) Branch() string {
return strings.ReplaceAll(p.Ref, "refs/heads/", "")
}

// .___
// | | ______ ________ __ ____
// | |/ ___// ___/ | \_/ __ \
// | |\___ \ \___ \| | /\ ___/
// |___/____ >____ >____/ \___ >
// \/ \/ \/

// HookIssueAction FIXME
type HookIssueAction string

@@ -371,13 +336,6 @@ type ChangesPayload struct {
Ref *ChangesFromPayload `json:"ref,omitempty"`
}

// __________ .__ .__ __________ __
// \______ \__ __| | | | \______ \ ____ ________ __ ____ _______/ |_
// | ___/ | \ | | | | _// __ \/ ____/ | \_/ __ \ / ___/\ __\
// | | | | / |_| |__ | | \ ___< <_| | | /\ ___/ \___ \ | |
// |____| |____/|____/____/ |____|_ /\___ >__ |____/ \___ >____ > |__|
// \/ \/ |__| \/ \/

// PullRequestPayload represents a payload information of pull request event.
type PullRequestPayload struct {
Action HookIssueAction `json:"action"`
@@ -402,13 +360,6 @@ type ReviewPayload struct {
Content string `json:"content"`
}

// __ __.__ __ .__
// / \ / \__| | _|__|
// \ \/\/ / | |/ / |
// \ /| | <| |
// \__/\ / |__|__|_ \__|
// \/ \/

// HookWikiAction an action that happens to a wiki page
type HookWikiAction string

@@ -435,13 +386,6 @@ func (p *WikiPayload) JSONPayload() ([]byte, error) {
return json.MarshalIndent(p, "", " ")
}

//__________ .__ __
//\______ \ ____ ______ ____ _____|__|/ |_ ___________ ___.__.
// | _// __ \\____ \ / _ \/ ___/ \ __\/ _ \_ __ < | |
// | | \ ___/| |_> > <_> )___ \| || | ( <_> ) | \/\___ |
// |____|_ /\___ > __/ \____/____ >__||__| \____/|__| / ____|
// \/ \/|__| \/ \/

// HookRepoAction an action that happens to a repo
type HookRepoAction string

@@ -480,7 +424,7 @@ type PackagePayload struct {
Action HookPackageAction `json:"action"`
Repository *Repository `json:"repository"`
Package *Package `json:"package"`
Organization *User `json:"organization"`
Organization *Organization `json:"organization"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking change?

Copy link
Member Author

@lunny lunny Feb 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to consider it a bug of the previous implementation, not a feature change. So it's not a break change although it will make something break because the previous implementation is wrong. Compare the two structs,
User have many fields which should not belong to Organization, i.e.

    // Is the user an administrator
	IsAdmin bool `json:"is_admin"`
	// swagger:strfmt date-time
	LastLogin time.Time `json:"last_login,omitempty"`
    // Is user active
	IsActive bool `json:"active"`
	// Is user login prohibited
	ProhibitLogin bool `json:"prohibit_login"`
...

But I'm OK if you prefer to add a breaking label and description.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yes, most useful fields are there

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But to backport, we need to mention that the struct has changed, to avoid wasting users' time

Sender *User `json:"sender"`
}

1 change: 1 addition & 0 deletions modules/webhook/structs.go
Original file line number Diff line number Diff line change
@@ -26,6 +26,7 @@ type HookEvents struct {
Repository bool `json:"repository"`
Release bool `json:"release"`
Package bool `json:"package"`
Status bool `json:"status"`
}

// HookEvent represents events that will delivery hook.
17 changes: 2 additions & 15 deletions modules/webhook/type.go
Original file line number Diff line number Diff line change
@@ -38,14 +38,6 @@ const (
// Event returns the HookEventType as an event string
func (h HookEventType) Event() string {
switch h {
case HookEventCreate:
return "create"
case HookEventDelete:
return "delete"
case HookEventFork:
return "fork"
case HookEventPush:
return "push"
case HookEventIssues, HookEventIssueAssign, HookEventIssueLabel, HookEventIssueMilestone:
return "issues"
case HookEventPullRequest, HookEventPullRequestAssign, HookEventPullRequestLabel, HookEventPullRequestMilestone,
@@ -59,14 +51,9 @@ func (h HookEventType) Event() string {
return "pull_request_rejected"
case HookEventPullRequestReviewComment:
return "pull_request_comment"
case HookEventWiki:
return "wiki"
case HookEventRepository:
return "repository"
case HookEventRelease:
return "release"
default:
return string(h)
}
return ""
}

// HookType is the type of a webhook
2 changes: 2 additions & 0 deletions routers/api/v1/utils/hook.go
Original file line number Diff line number Diff line change
@@ -205,6 +205,8 @@ func addHook(ctx *context.APIContext, form *api.CreateHookOption, ownerID, repoI
Wiki: util.SliceContainsString(form.Events, string(webhook_module.HookEventWiki), true),
Repository: util.SliceContainsString(form.Events, string(webhook_module.HookEventRepository), true),
Release: util.SliceContainsString(form.Events, string(webhook_module.HookEventRelease), true),
Package: util.SliceContainsString(form.Events, string(webhook_module.HookEventPackage), true),
Status: util.SliceContainsString(form.Events, string(webhook_module.HookEventStatus), true),
},
BranchFilter: form.BranchFilter,
},
4 changes: 4 additions & 0 deletions services/webhook/dingtalk.go
Original file line number Diff line number Diff line change
@@ -190,3 +190,7 @@ func newDingtalkRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_
var pc payloadConvertor[DingtalkPayload] = dingtalkConvertor{}
return newJSONRequest(pc, w, t, true)
}

func init() {
RegisterWebhookRequester(webhook_module.DINGTALK, newDingtalkRequest)
}
4 changes: 4 additions & 0 deletions services/webhook/discord.go
Original file line number Diff line number Diff line change
@@ -277,6 +277,10 @@ func newDiscordRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_m
return newJSONRequest(pc, w, t, true)
}

func init() {
RegisterWebhookRequester(webhook_module.DISCORD, newDiscordRequest)
}

func parseHookPullRequestEventType(event webhook_module.HookEventType) (string, error) {
switch event {
case webhook_module.HookEventPullRequestReviewApproved:
4 changes: 4 additions & 0 deletions services/webhook/feishu.go
Original file line number Diff line number Diff line change
@@ -170,3 +170,7 @@ func newFeishuRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_mo
var pc payloadConvertor[FeishuPayload] = feishuConvertor{}
return newJSONRequest(pc, w, t, true)
}

func init() {
RegisterWebhookRequester(webhook_module.FEISHU, newFeishuRequest)
}
4 changes: 2 additions & 2 deletions services/webhook/general_test.go
Original file line number Diff line number Diff line change
@@ -319,8 +319,8 @@ func packageTestPayload() *api.PackagePayload {
AvatarURL: "http://localhost:3000/user1/avatar",
},
Repository: nil,
Organization: &api.User{
UserName: "org1",
Organization: &api.Organization{
Name: "org1",
AvatarURL: "http://localhost:3000/org1/avatar",
},
Package: &api.Package{
4 changes: 4 additions & 0 deletions services/webhook/matrix.go
Original file line number Diff line number Diff line change
@@ -24,6 +24,10 @@ import (
webhook_module "code.gitea.io/gitea/modules/webhook"
)

func init() {
RegisterWebhookRequester(webhook_module.MATRIX, newMatrixRequest)
}

func newMatrixRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) {
meta := &MatrixMeta{}
if err := json.Unmarshal([]byte(w.Meta), meta); err != nil {
4 changes: 4 additions & 0 deletions services/webhook/msteams.go
Original file line number Diff line number Diff line change
@@ -349,3 +349,7 @@ func newMSTeamsRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_m
var pc payloadConvertor[MSTeamsPayload] = msteamsConvertor{}
return newJSONRequest(pc, w, t, true)
}

func init() {
RegisterWebhookRequester(webhook_module.MSTEAMS, newMSTeamsRequest)
}
13 changes: 10 additions & 3 deletions services/webhook/notifier.go
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ import (

git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/organization"
packages_model "code.gitea.io/gitea/models/packages"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
@@ -924,10 +925,16 @@ func notifyPackage(ctx context.Context, sender *user_model.User, pd *packages_mo
return
}

var org *api.Organization
if pd.Owner.IsOrganization() {
org = convert.ToOrganization(ctx, organization.OrgFromUser(pd.Owner))
}

if err := PrepareWebhooks(ctx, source, webhook_module.HookEventPackage, &api.PackagePayload{
Action: action,
Package: apiPackage,
Sender: convert.ToUser(ctx, sender, nil),
Action: action,
Package: apiPackage,
Organization: org,
Sender: convert.ToUser(ctx, sender, nil),
}); err != nil {
log.Error("PrepareWebhooks: %v", err)
}
4 changes: 4 additions & 0 deletions services/webhook/packagist.go
Original file line number Diff line number Diff line change
@@ -120,3 +120,7 @@ func newPackagistRequest(_ context.Context, w *webhook_model.Webhook, t *webhook
}
return newJSONRequest(pc, w, t, true)
}

func init() {
RegisterWebhookRequester(webhook_module.PACKAGIST, newPackagistRequest)
}
4 changes: 4 additions & 0 deletions services/webhook/slack.go
Original file line number Diff line number Diff line change
@@ -295,6 +295,10 @@ func newSlackRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_mod
return newJSONRequest(pc, w, t, true)
}

func init() {
RegisterWebhookRequester(webhook_module.SLACK, newSlackRequest)
}

var slackChannel = regexp.MustCompile(`^#?[a-z0-9_-]{1,80}$`)

// IsValidSlackChannel validates a channel name conforms to what slack expects:
4 changes: 4 additions & 0 deletions services/webhook/telegram.go
Original file line number Diff line number Diff line change
@@ -187,3 +187,7 @@ func newTelegramRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_
var pc payloadConvertor[TelegramPayload] = telegramConvertor{}
return newJSONRequest(pc, w, t, true)
}

func init() {
RegisterWebhookRequester(webhook_module.TELEGRAM, newTelegramRequest)
}
16 changes: 6 additions & 10 deletions services/webhook/webhook.go
Original file line number Diff line number Diff line change
@@ -27,16 +27,12 @@ import (
"github.com/gobwas/glob"
)

var webhookRequesters = map[webhook_module.HookType]func(context.Context, *webhook_model.Webhook, *webhook_model.HookTask) (req *http.Request, body []byte, err error){
webhook_module.SLACK: newSlackRequest,
webhook_module.DISCORD: newDiscordRequest,
webhook_module.DINGTALK: newDingtalkRequest,
webhook_module.TELEGRAM: newTelegramRequest,
webhook_module.MSTEAMS: newMSTeamsRequest,
webhook_module.FEISHU: newFeishuRequest,
webhook_module.MATRIX: newMatrixRequest,
webhook_module.WECHATWORK: newWechatworkRequest,
webhook_module.PACKAGIST: newPackagistRequest,
type Requester func(context.Context, *webhook_model.Webhook, *webhook_model.HookTask) (req *http.Request, body []byte, err error)

var webhookRequesters = map[webhook_module.HookType]Requester{}

func RegisterWebhookRequester(hookType webhook_module.HookType, requester Requester) {
webhookRequesters[hookType] = requester
}

// IsValidHookTaskType returns true if a webhook registered
4 changes: 4 additions & 0 deletions services/webhook/wechatwork.go
Original file line number Diff line number Diff line change
@@ -179,3 +179,7 @@ func newWechatworkRequest(_ context.Context, w *webhook_model.Webhook, t *webhoo
var pc payloadConvertor[WechatworkPayload] = wechatworkConvertor{}
return newJSONRequest(pc, w, t, true)
}

func init() {
RegisterWebhookRequester(webhook_module.WECHATWORK, newWechatworkRequest)
}
15 changes: 10 additions & 5 deletions tests/integration/api_repo_test.go
Original file line number Diff line number Diff line change
@@ -471,6 +471,15 @@ func TestAPIMirrorSyncNonMirrorRepo(t *testing.T) {
assert.Equal(t, "Repository is not a mirror", errRespJSON["message"])
}

func testAPIOrgCreateRepo(t *testing.T, session *TestSession, orgName, repoName string, status int) {
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteOrganization, auth_model.AccessTokenScopeWriteRepository)

req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/org/%s/repos", orgName), &api.CreateRepoOption{
Name: repoName,
}).AddTokenAuth(token)
MakeRequest(t, req, status)
}

func TestAPIOrgRepoCreate(t *testing.T) {
testCases := []struct {
ctxUserID int64
@@ -488,11 +497,7 @@ func TestAPIOrgRepoCreate(t *testing.T) {
for _, testCase := range testCases {
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: testCase.ctxUserID})
session := loginUser(t, user.Name)
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteOrganization, auth_model.AccessTokenScopeWriteRepository)
req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/org/%s/repos", testCase.orgName), &api.CreateRepoOption{
Name: testCase.repoName,
}).AddTokenAuth(token)
MakeRequest(t, req, testCase.expectedStatus)
testAPIOrgCreateRepo(t, session, testCase.orgName, testCase.repoName, testCase.expectedStatus)
}
}

24 changes: 14 additions & 10 deletions tests/integration/api_wiki_test.go
Original file line number Diff line number Diff line change
@@ -172,6 +172,19 @@ func TestAPIListWikiPages(t *testing.T) {
assert.Equal(t, dummymeta, meta)
}

func testAPICreateWikiPage(t *testing.T, session *TestSession, userName, repoName, title string, status int) {
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)

urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/wiki/new", userName, repoName)

req := NewRequestWithJSON(t, "POST", urlStr, &api.CreateWikiPageOptions{
Title: title,
ContentBase64: base64.StdEncoding.EncodeToString([]byte("Wiki page content for API unit tests")),
Message: "",
}).AddTokenAuth(token)
MakeRequest(t, req, status)
}

func TestAPINewWikiPage(t *testing.T) {
for _, title := range []string{
"New page",
@@ -180,16 +193,7 @@ func TestAPINewWikiPage(t *testing.T) {
defer tests.PrepareTestEnv(t)()
username := "user2"
session := loginUser(t, username)
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)

urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/wiki/new", username, "repo1")

req := NewRequestWithJSON(t, "POST", urlStr, &api.CreateWikiPageOptions{
Title: title,
ContentBase64: base64.StdEncoding.EncodeToString([]byte("Wiki page content for API unit tests")),
Message: "",
}).AddTokenAuth(token)
MakeRequest(t, req, http.StatusCreated)
testAPICreateWikiPage(t, session, username, "repo1", title, http.StatusCreated)
}
}

2 changes: 1 addition & 1 deletion tests/integration/issue_test.go
Original file line number Diff line number Diff line change
@@ -174,7 +174,7 @@ func testIssueAddComment(t *testing.T, session *TestSession, issueURL, content,

htmlDoc = NewHTMLParser(t, resp.Body)

val := htmlDoc.doc.Find(".comment-list .comment .render-content p").Eq(commentCount).Text()
val := strings.TrimSpace(htmlDoc.doc.Find(".comment-list .comment .render-content").Eq(commentCount).Text())
assert.Equal(t, content, val)

idAttr, has := htmlDoc.doc.Find(".comment-list .comment").Eq(commentCount).Attr("id")
565 changes: 565 additions & 0 deletions tests/integration/repo_webhook_test.go

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions tests/mssql.ini.tmpl
Original file line number Diff line number Diff line change
@@ -26,6 +26,9 @@ TYPE = immediate
[queue.push_update]
TYPE = immediate

[queue.webhook_sender]
TYPE = immediate

[repository]
ROOT = {{REPO_TEST_DIR}}tests/{{TEST_TYPE}}/gitea-{{TEST_TYPE}}-mssql/gitea-repositories

@@ -112,3 +115,6 @@ ENABLED = true

[actions]
ENABLED = true

[webhook]
ALLOWED_HOST_LIST = 127.0.0.1
6 changes: 6 additions & 0 deletions tests/mysql.ini.tmpl
Original file line number Diff line number Diff line change
@@ -28,6 +28,9 @@ TYPE = immediate
[queue.push_update]
TYPE = immediate

[queue.webhook_sender]
TYPE = immediate

[repository]
ROOT = {{REPO_TEST_DIR}}tests/{{TEST_TYPE}}/gitea-{{TEST_TYPE}}-mysql/gitea-repositories

@@ -119,3 +122,6 @@ REPLY_TO_ADDRESS = incoming+%{token}@localhost

[actions]
ENABLED = true

[webhook]
ALLOWED_HOST_LIST = 127.0.0.1
6 changes: 6 additions & 0 deletions tests/pgsql.ini.tmpl
Original file line number Diff line number Diff line change
@@ -27,6 +27,9 @@ TYPE = immediate
[queue.push_update]
TYPE = immediate

[queue.webhook_sender]
TYPE = immediate

[repository]
ROOT = {{REPO_TEST_DIR}}tests/{{TEST_TYPE}}/gitea-{{TEST_TYPE}}-pgsql/gitea-repositories

@@ -128,3 +131,6 @@ ENABLED = true

[actions]
ENABLED = true

[webhook]
ALLOWED_HOST_LIST = 127.0.0.1
6 changes: 6 additions & 0 deletions tests/sqlite.ini.tmpl
Original file line number Diff line number Diff line change
@@ -22,6 +22,9 @@ TYPE = immediate
[queue.push_update]
TYPE = immediate

[queue.webhook_sender]
TYPE = immediate

[repository]
ROOT = {{REPO_TEST_DIR}}tests/{{TEST_TYPE}}/gitea-{{TEST_TYPE}}-sqlite/gitea-repositories

@@ -117,3 +120,6 @@ RENDER_CONTENT_MODE=sanitized

[actions]
ENABLED = true

[webhook]
ALLOWED_HOST_LIST = 127.0.0.1