Implement webhook branch filter (#7791)

* Fix validate() function to handle errors in embedded anon structs

* Implement webhook branch filter

See #2025, #3998.

3 files changed, 90 insertions, 3 deletions

diff --git a/models/fixtures/webhook.yml b/models/fixtures/webhook.yml
index 11d7439cf4..5563dcada7 100644
--- a/models/fixtures/webhook.yml
+++ b/models/fixtures/webhook.yml
@@ -22,3 +22,10 @@
   content_type: 1 # json
   events: '{"push_only":false,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":true}}'
   is_active: true
+-
+  id: 4
+  repo_id: 2
+  url: www.example.com/url4
+  content_type: 1 # json
+  events: '{"push_only":true,"branch_filter":"{master,feature*}"}'
+  is_active: true
diff --git a/models/webhook.go b/models/webhook.go
index 4eda08fdb5..67ae783759 100644
--- a/models/webhook.go
+++ b/models/webhook.go
@@ -19,12 +19,14 @@ import (
 	"strings"
 	"time"
 
+	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/sync"
 	"code.gitea.io/gitea/modules/timeutil"
 
+	"github.com/gobwas/glob"
 	gouuid "github.com/satori/go.uuid"
 	"github.com/unknwon/com"
 )
@@ -84,9 +86,10 @@ type HookEvents struct {
 
 // HookEvent represents events that will delivery hook.
 type HookEvent struct {
-	PushOnly       bool `json:"push_only"`
-	SendEverything bool `json:"send_everything"`
-	ChooseEvents   bool `json:"choose_events"`
+	PushOnly       bool   `json:"push_only"`
+	SendEverything bool   `json:"send_everything"`
+	ChooseEvents   bool   `json:"choose_events"`
+	BranchFilter   string `json:"branch_filter"`
 
 	HookEvents `json:"events"`
 }
@@ -256,6 +259,21 @@ func (w *Webhook) EventsArray() []string {
 	return events
 }
 
+func (w *Webhook) checkBranch(branch string) bool {
+	if w.BranchFilter == "" || w.BranchFilter == "*" {
+		return true
+	}
+
+	g, err := glob.Compile(w.BranchFilter)
+	if err != nil {
+		// should not really happen as BranchFilter is validated
+		log.Error("CheckBranch failed: %s", err)
+		return false
+	}
+
+	return g.Match(branch)
+}
+
 // CreateWebhook creates a new web hook.
 func CreateWebhook(w *Webhook) error {
 	return createWebhook(x, w)
@@ -651,6 +669,25 @@ func PrepareWebhook(w *Webhook, repo *Repository, event HookEventType, p api.Pay
 	return prepareWebhook(x, w, repo, event, p)
 }
 
+// getPayloadBranch returns branch for hook event, if applicable.
+func getPayloadBranch(p api.Payloader) string {
+	switch pp := p.(type) {
+	case *api.CreatePayload:
+		if pp.RefType == "branch" {
+			return pp.Ref
+		}
+	case *api.DeletePayload:
+		if pp.RefType == "branch" {
+			return pp.Ref
+		}
+	case *api.PushPayload:
+		if strings.HasPrefix(pp.Ref, git.BranchPrefix) {
+			return pp.Ref[len(git.BranchPrefix):]
+		}
+	}
+	return ""
+}
+
 func prepareWebhook(e Engine, w *Webhook, repo *Repository, event HookEventType, p api.Payloader) error {
 	for _, e := range w.eventCheckers() {
 		if event == e.typ {
@@ -660,6 +697,15 @@ func prepareWebhook(e Engine, w *Webhook, repo *Repository, event HookEventType,
 		}
 	}
 
+	// If payload has no associated branch (e.g. it's a new tag, issue, etc.),
+	// branch filter has no effect.
+	if branch := getPayloadBranch(p); branch != "" {
+		if !w.checkBranch(branch) {
+			log.Info("Branch %q doesn't match branch filter %q, skipping", branch, w.BranchFilter)
+			return nil
+		}
+	}
+
 	var payloader api.Payloader
 	var err error
 	// Use separate objects so modifications won't be made on payload on non-Gogs/Gitea type hooks.
diff --git a/models/webhook_test.go b/models/webhook_test.go
index d2fb3ea6ed..343000f5b5 100644
--- a/models/webhook_test.go
+++ b/models/webhook_test.go
@@ -270,6 +270,40 @@ func TestPrepareWebhooks(t *testing.T) {
 	}
 }
 
+func TestPrepareWebhooksBranchFilterMatch(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 2}).(*Repository)
+	hookTasks := []*HookTask{
+		{RepoID: repo.ID, HookID: 4, EventType: HookEventPush},
+	}
+	for _, hookTask := range hookTasks {
+		AssertNotExistsBean(t, hookTask)
+	}
+	// this test also ensures that * doesn't handle / in any special way (like shell would)
+	assert.NoError(t, PrepareWebhooks(repo, HookEventPush, &api.PushPayload{Ref: "refs/heads/feature/7791"}))
+	for _, hookTask := range hookTasks {
+		AssertExistsAndLoadBean(t, hookTask)
+	}
+}
+
+func TestPrepareWebhooksBranchFilterNoMatch(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 2}).(*Repository)
+	hookTasks := []*HookTask{
+		{RepoID: repo.ID, HookID: 4, EventType: HookEventPush},
+	}
+	for _, hookTask := range hookTasks {
+		AssertNotExistsBean(t, hookTask)
+	}
+	assert.NoError(t, PrepareWebhooks(repo, HookEventPush, &api.PushPayload{Ref: "refs/heads/fix_weird_bug"}))
+
+	for _, hookTask := range hookTasks {
+		AssertNotExistsBean(t, hookTask)
+	}
+}
+
 // TODO TestHookTask_deliver
 
 // TODO TestDeliverHooks