From 1b9fe11c1f6efba93c0bac860e4e677c147e6896 Mon Sep 17 00:00:00 2001 From: Martin Treml Date: Thu, 20 Dec 2018 17:09:12 +0100 Subject: [PATCH 1/5] Add gitlab system hook parsing If you use gitlab system hooks you can now send push/tag_push/merge_request events --- gitlab/gitlab.go | 60 ++++++++++++++++++++++++++++++--------- gitlab/gitlab_test.go | 65 ++++++++++++++++++++++++++++++++++++++++--- gitlab/payload.go | 5 ++++ 3 files changed, 113 insertions(+), 17 deletions(-) diff --git a/gitlab/gitlab.go b/gitlab/gitlab.go index e603868..48879ed 100644 --- a/gitlab/gitlab.go +++ b/gitlab/gitlab.go @@ -17,6 +17,7 @@ var ( ErrGitLabTokenVerificationFailed = errors.New("X-Gitlab-Token validation failed") ErrEventNotFound = errors.New("event not defined to be parsed") ErrParsingPayload = errors.New("error parsing payload") + ErrParsingSystemPayload = errors.New("error parsing system payload") // ErrHMACVerificationFailed = errors.New("HMAC verification failed") ) @@ -31,6 +32,11 @@ const ( WikiPageEvents Event = "Wiki Page Hook" PipelineEvents Event = "Pipeline Hook" BuildEvents Event = "Build Hook" + SystemHooks Event = "System Hook" + + objectPush string = "push" + objectTag string = "tag_push" + objectMergeRequest string = "merge_request" ) // Option is a configuration option for the webhook @@ -55,7 +61,7 @@ type Webhook struct { secret string } -// Event defines a GitHub hook event type +// Event defines a GitLab hook event type by the X-Gitlab-Event Header type Event string // New creates and returns a WebHook instance denoted by the Provider type @@ -89,6 +95,31 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) } gitLabEvent := Event(event) + payload := []byte{} + + if gitLabEvent == SystemHooks { + + sysPayload, err := ioutil.ReadAll(r.Body) + if err != nil || len(sysPayload) == 0 { + return nil, ErrParsingSystemPayload + } + payload = sysPayload + + var sysPl SystemHookPayload + err = json.Unmarshal([]byte(payload), &sysPl) + if err != nil { + return nil, err + } + + switch sysPl.ObjectKind { + case objectPush: + gitLabEvent = PushEvents + case objectTag: + gitLabEvent = TagEvents + case objectMergeRequest: + gitLabEvent = MergeRequestEvents + } + } var found bool for _, evt := range events { @@ -102,9 +133,12 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) return nil, ErrEventNotFound } - payload, err := ioutil.ReadAll(r.Body) - if err != nil || len(payload) == 0 { - return nil, ErrParsingPayload + // check if payload is empty - that means it was no system hook call + if len(payload) == 0 { + payload, err := ioutil.ReadAll(r.Body) + if err != nil || len(payload) == 0 { + return nil, ErrParsingPayload + } } // If we have a Secret set, we should check the MAC @@ -118,47 +152,47 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) switch gitLabEvent { case PushEvents: var pl PushEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case TagEvents: var pl TagEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case ConfidentialIssuesEvents: var pl ConfidentialIssueEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case IssuesEvents: var pl IssueEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case CommentEvents: var pl CommentEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case MergeRequestEvents: var pl MergeRequestEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case WikiPageEvents: var pl WikiPageEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case PipelineEvents: var pl PipelineEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case BuildEvents: var pl BuildEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err default: return nil, fmt.Errorf("unknown event %s", gitLabEvent) diff --git a/gitlab/gitlab_test.go b/gitlab/gitlab_test.go index 2d9c256..da5cd11 100644 --- a/gitlab/gitlab_test.go +++ b/gitlab/gitlab_test.go @@ -2,15 +2,13 @@ package gitlab import ( "bytes" + "io" "log" "net/http" "net/http/httptest" "os" - "testing" - - "io" - "reflect" + "testing" "github.com/stretchr/testify/require" ) @@ -266,3 +264,62 @@ func TestWebhooks(t *testing.T) { }) } } + +func TestSystemHooks(t *testing.T) { + assert := require.New(t) + tests := []struct { + name string + event Event + typ interface{} + filename string + }{ + { + name: "PushEvent", + event: PushEvents, + typ: PushEventPayload{}, + filename: "../testdata/gitlab/push-event.json", + }, + { + name: "TagEvent", + event: TagEvents, + typ: TagEventPayload{}, + filename: "../testdata/gitlab/tag-event.json", + }, + { + name: "MergeRequestEvent", + event: MergeRequestEvents, + typ: MergeRequestEventPayload{}, + filename: "../testdata/gitlab/merge-request-event.json", + }, + } + for _, tt := range tests { + tc := tt + client := &http.Client{} + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + payload, err := os.Open(tc.filename) + assert.NoError(err) + defer func() { + _ = payload.Close() + }() + + var parseError error + var results interface{} + server := newServer(func(w http.ResponseWriter, r *http.Request) { + results, parseError = hook.Parse(r, tc.event) + }) + defer server.Close() + req, err := http.NewRequest(http.MethodPost, server.URL+path, payload) + assert.NoError(err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Gitlab-Token", "sampleToken!") + req.Header.Set("X-Gitlab-Event", "System Hook") + + resp, err := client.Do(req) + assert.NoError(err) + assert.Equal(http.StatusOK, resp.StatusCode) + assert.NoError(parseError) + assert.Equal(reflect.TypeOf(tc.typ), reflect.TypeOf(results)) + }) + } +} diff --git a/gitlab/payload.go b/gitlab/payload.go index c4b9238..f5a1baa 100644 --- a/gitlab/payload.go +++ b/gitlab/payload.go @@ -148,6 +148,11 @@ type BuildEventPayload struct { Repository Repository `json:"repository"` } +// SystemHookPayload contains the ObjectKind to match with real hook events +type SystemHookPayload struct { + ObjectKind string `json:"object_kind"` +} + // Issue contains all of the GitLab issue information type Issue struct { ID int64 `json:"id"` From 57ccf4fc307dc75e519fa16e4ecbdd3267af5418 Mon Sep 17 00:00:00 2001 From: Martin Treml Date: Thu, 20 Dec 2018 17:35:44 +0100 Subject: [PATCH 2/5] Declare err and payload to avoid scope issues --- gitlab/gitlab.go | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/gitlab/gitlab.go b/gitlab/gitlab.go index 48879ed..e1279da 100644 --- a/gitlab/gitlab.go +++ b/gitlab/gitlab.go @@ -82,6 +82,9 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) _ = r.Body.Close() }() + var err error + var payload []byte + if len(events) == 0 { return nil, ErrEventNotSpecifiedToParse } @@ -95,15 +98,13 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) } gitLabEvent := Event(event) - payload := []byte{} if gitLabEvent == SystemHooks { - sysPayload, err := ioutil.ReadAll(r.Body) - if err != nil || len(sysPayload) == 0 { + payload, err = ioutil.ReadAll(r.Body) + if err != nil || len(payload) == 0 { return nil, ErrParsingSystemPayload } - payload = sysPayload var sysPl SystemHookPayload err = json.Unmarshal([]byte(payload), &sysPl) @@ -135,7 +136,7 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) // check if payload is empty - that means it was no system hook call if len(payload) == 0 { - payload, err := ioutil.ReadAll(r.Body) + payload, err = ioutil.ReadAll(r.Body) if err != nil || len(payload) == 0 { return nil, ErrParsingPayload } @@ -152,47 +153,47 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) switch gitLabEvent { case PushEvents: var pl PushEventPayload - err := json.Unmarshal([]byte(payload), &pl) + err = json.Unmarshal([]byte(payload), &pl) return pl, err case TagEvents: var pl TagEventPayload - err := json.Unmarshal([]byte(payload), &pl) + err = json.Unmarshal([]byte(payload), &pl) return pl, err case ConfidentialIssuesEvents: var pl ConfidentialIssueEventPayload - err := json.Unmarshal([]byte(payload), &pl) + err = json.Unmarshal([]byte(payload), &pl) return pl, err case IssuesEvents: var pl IssueEventPayload - err := json.Unmarshal([]byte(payload), &pl) + err = json.Unmarshal([]byte(payload), &pl) return pl, err case CommentEvents: var pl CommentEventPayload - err := json.Unmarshal([]byte(payload), &pl) + err = json.Unmarshal([]byte(payload), &pl) return pl, err case MergeRequestEvents: var pl MergeRequestEventPayload - err := json.Unmarshal([]byte(payload), &pl) + err = json.Unmarshal([]byte(payload), &pl) return pl, err case WikiPageEvents: var pl WikiPageEventPayload - err := json.Unmarshal([]byte(payload), &pl) + err = json.Unmarshal([]byte(payload), &pl) return pl, err case PipelineEvents: var pl PipelineEventPayload - err := json.Unmarshal([]byte(payload), &pl) + err = json.Unmarshal([]byte(payload), &pl) return pl, err case BuildEvents: var pl BuildEventPayload - err := json.Unmarshal([]byte(payload), &pl) + err = json.Unmarshal([]byte(payload), &pl) return pl, err default: return nil, fmt.Errorf("unknown event %s", gitLabEvent) From 175959ef6086b75df475b86cda4c20dcfbcc9421 Mon Sep 17 00:00:00 2001 From: Martin Treml Date: Wed, 27 Feb 2019 17:58:42 +0100 Subject: [PATCH 3/5] Refactor the whole event switch into own function --- gitlab/gitlab.go | 97 +++++++++++++++++++++++------------------------- 1 file changed, 47 insertions(+), 50 deletions(-) diff --git a/gitlab/gitlab.go b/gitlab/gitlab.go index e1279da..9d478fc 100644 --- a/gitlab/gitlab.go +++ b/gitlab/gitlab.go @@ -32,7 +32,7 @@ const ( WikiPageEvents Event = "Wiki Page Hook" PipelineEvents Event = "Pipeline Hook" BuildEvents Event = "Build Hook" - SystemHooks Event = "System Hook" + SystemHookEvents Event = "System Hook" objectPush string = "push" objectTag string = "tag_push" @@ -82,9 +82,6 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) _ = r.Body.Close() }() - var err error - var payload []byte - if len(events) == 0 { return nil, ErrEventNotSpecifiedToParse } @@ -92,6 +89,17 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) return nil, ErrInvalidHTTPMethod } + // Add SystemHookEvents to event slice to allow parsing of it + events = append(events, SystemHookEvents) + + // If we have a Secret set, we should check the MAC + if len(hook.secret) > 0 { + signature := r.Header.Get("X-Gitlab-Token") + if signature != hook.secret { + return nil, ErrGitLabTokenVerificationFailed + } + } + event := r.Header.Get("X-Gitlab-Event") if len(event) == 0 { return nil, ErrMissingGitLabEventHeader @@ -99,29 +107,17 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) gitLabEvent := Event(event) - if gitLabEvent == SystemHooks { - - payload, err = ioutil.ReadAll(r.Body) - if err != nil || len(payload) == 0 { - return nil, ErrParsingSystemPayload - } - - var sysPl SystemHookPayload - err = json.Unmarshal([]byte(payload), &sysPl) - if err != nil { - return nil, err - } - - switch sysPl.ObjectKind { - case objectPush: - gitLabEvent = PushEvents - case objectTag: - gitLabEvent = TagEvents - case objectMergeRequest: - gitLabEvent = MergeRequestEvents - } + // check if payload is empty - that means it was no system hook call + payload, err := ioutil.ReadAll(r.Body) + if err != nil || len(payload) == 0 { + return nil, ErrParsingPayload } + return eventParsing(gitLabEvent, events, payload) +} + +func eventParsing(gitLabEvent Event, events []Event, payload []byte) (interface{}, error) { + var found bool for _, evt := range events { if evt == gitLabEvent { @@ -134,67 +130,68 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) return nil, ErrEventNotFound } - // check if payload is empty - that means it was no system hook call - if len(payload) == 0 { - payload, err = ioutil.ReadAll(r.Body) - if err != nil || len(payload) == 0 { - return nil, ErrParsingPayload - } - } - - // If we have a Secret set, we should check the MAC - if len(hook.secret) > 0 { - signature := r.Header.Get("X-Gitlab-Token") - if signature != hook.secret { - return nil, ErrGitLabTokenVerificationFailed - } - } - switch gitLabEvent { case PushEvents: var pl PushEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case TagEvents: var pl TagEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case ConfidentialIssuesEvents: var pl ConfidentialIssueEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case IssuesEvents: var pl IssueEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case CommentEvents: var pl CommentEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case MergeRequestEvents: var pl MergeRequestEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case WikiPageEvents: var pl WikiPageEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case PipelineEvents: var pl PipelineEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err case BuildEvents: var pl BuildEventPayload - err = json.Unmarshal([]byte(payload), &pl) + err := json.Unmarshal([]byte(payload), &pl) return pl, err + + case SystemHookEvents: + var pl SystemHookPayload + err := json.Unmarshal([]byte(payload), &pl) + if err != nil { + return nil, err + } + switch pl.ObjectKind { + case objectPush: + return eventParsing(PushEvents, events, payload) + case objectTag: + return eventParsing(TagEvents, events, payload) + case objectMergeRequest: + return eventParsing(MergeRequestEvents, events, payload) + default: + return nil, fmt.Errorf("unknown system hook event %s", gitLabEvent) + } default: return nil, fmt.Errorf("unknown event %s", gitLabEvent) } From 66059be93bdb4beac5208fd063100e5d8060576d Mon Sep 17 00:00:00 2001 From: Martin Treml Date: Tue, 5 Mar 2019 11:38:26 +0100 Subject: [PATCH 4/5] Remove unnecessary comment --- gitlab/gitlab.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gitlab/gitlab.go b/gitlab/gitlab.go index 9d478fc..dfefb1a 100644 --- a/gitlab/gitlab.go +++ b/gitlab/gitlab.go @@ -89,7 +89,7 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) return nil, ErrInvalidHTTPMethod } - // Add SystemHookEvents to event slice to allow parsing of it + // Add SystemHookEvents to event slice to allow parsing events = append(events, SystemHookEvents) // If we have a Secret set, we should check the MAC @@ -107,7 +107,6 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) gitLabEvent := Event(event) - // check if payload is empty - that means it was no system hook call payload, err := ioutil.ReadAll(r.Body) if err != nil || len(payload) == 0 { return nil, ErrParsingPayload From bad16bfcd05d239059da3d7196bbfbe64099fc45 Mon Sep 17 00:00:00 2001 From: Martin Treml Date: Mon, 11 Mar 2019 08:13:22 +0100 Subject: [PATCH 5/5] Give the user the choice to parse SystemHooks --- gitlab/gitlab.go | 3 --- gitlab/gitlab_test.go | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/gitlab/gitlab.go b/gitlab/gitlab.go index dfefb1a..dffe9b9 100644 --- a/gitlab/gitlab.go +++ b/gitlab/gitlab.go @@ -89,9 +89,6 @@ func (hook Webhook) Parse(r *http.Request, events ...Event) (interface{}, error) return nil, ErrInvalidHTTPMethod } - // Add SystemHookEvents to event slice to allow parsing - events = append(events, SystemHookEvents) - // If we have a Secret set, we should check the MAC if len(hook.secret) > 0 { signature := r.Header.Get("X-Gitlab-Token") diff --git a/gitlab/gitlab_test.go b/gitlab/gitlab_test.go index da5cd11..ed13f57 100644 --- a/gitlab/gitlab_test.go +++ b/gitlab/gitlab_test.go @@ -306,7 +306,7 @@ func TestSystemHooks(t *testing.T) { var parseError error var results interface{} server := newServer(func(w http.ResponseWriter, r *http.Request) { - results, parseError = hook.Parse(r, tc.event) + results, parseError = hook.Parse(r, SystemHookEvents, tc.event) }) defer server.Close() req, err := http.NewRequest(http.MethodPost, server.URL+path, payload)