hookHandler: Bug with multiple hooks having the same ID
It was possible to have multiple hooks with the same ID, but the return in the for/if of the hookHandler method were stopping the loop, thus making it impossible to continue the executions of the other hooks. This isn't the behaviour expected because a service might be down, because you want to run more than one hook, or don't want to stop due to one that failed. This commit fixes it, and add tests associated. The hooks have been copied from the github one, slightly modified and spread in the template file. A "dummy" hook which is never called has also been added, just in case it could mess with some other, and a test for non existing hook has also been added to the tests. This PR fixes: https://github.com/adnanh/webhook/issues/76
This commit is contained in:
Hugo Rosnet
parent
18b0573bc4
commit
c3c7b5fc1a
@ -53,6 +53,60 @@
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "github-called-twice",
|
||||
"execute-command": "{{ .Hookecho }}",
|
||||
"command-working-directory": "/",
|
||||
"include-command-output-in-response": true,
|
||||
"pass-environment-to-command":
|
||||
[
|
||||
{
|
||||
"source": "payload",
|
||||
"name": "created"
|
||||
}
|
||||
],
|
||||
"pass-arguments-to-command":
|
||||
[
|
||||
{
|
||||
"source": "payload",
|
||||
"name": "head_commit.id"
|
||||
},
|
||||
{
|
||||
"source": "payload",
|
||||
"name": "repository.fork"
|
||||
}
|
||||
],
|
||||
"trigger-rule":
|
||||
{
|
||||
"and":
|
||||
[
|
||||
{
|
||||
"match":
|
||||
{
|
||||
"type": "payload-hash-sha1",
|
||||
"secret": "mysecret",
|
||||
"parameter":
|
||||
{
|
||||
"source": "header",
|
||||
"name": "X-Hub-Signature"
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
"match":
|
||||
{
|
||||
"type": "value",
|
||||
"value": "refs/heads/master",
|
||||
"parameter":
|
||||
{
|
||||
"source": "payload",
|
||||
"name": "ref"
|
||||
}
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "bitbucket",
|
||||
"execute-command": "{{ .Hookecho }}",
|
||||
@ -134,6 +188,39 @@
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "uncalled-hook",
|
||||
"execute-command": "{{ .Hookecho }}",
|
||||
"command-working-directory": "/",
|
||||
"include-command-output-in-response": true,
|
||||
"response-message": "success",
|
||||
"parse-parameters-as-json": [
|
||||
{
|
||||
"source": "payload",
|
||||
"name": "payload"
|
||||
}
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": "github-called-twice",
|
||||
"execute-command": "{{ .Hookecho }}",
|
||||
"command-working-directory": "/",
|
||||
"include-command-output-in-response": true,
|
||||
"pass-environment-to-command":
|
||||
[
|
||||
{
|
||||
"source": "payload",
|
||||
"name": "pusher.email"
|
||||
}
|
||||
],
|
||||
"pass-arguments-to-command":
|
||||
[
|
||||
{
|
||||
"source": "payload",
|
||||
"name": "repository.created_at"
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
15
webhook.go
15
webhook.go
@ -187,6 +187,8 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
|
||||
// handle hook
|
||||
executed := 0
|
||||
|
||||
for _, h := range matchedHooks {
|
||||
|
||||
err := h.ParseJSONParameters(&headers, &query, &payload)
|
||||
@ -195,7 +197,7 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
|
||||
log.Printf(msg)
|
||||
w.WriteHeader(http.StatusBadRequest)
|
||||
fmt.Fprintf(w, msg)
|
||||
return
|
||||
continue
|
||||
}
|
||||
|
||||
var ok bool
|
||||
@ -209,7 +211,7 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
|
||||
log.Printf(msg)
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
fmt.Fprintf(w, msg)
|
||||
return
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
@ -227,15 +229,18 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
|
||||
go handleHook(h, &headers, &query, &payload, &body)
|
||||
fmt.Fprintf(w, h.ResponseMessage)
|
||||
}
|
||||
|
||||
return
|
||||
executed++
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
// if none of the hooks got triggered
|
||||
if executed == 0 {
|
||||
log.Printf("%s got matched (%d time(s)), but didn't get triggered because the trigger rules were not satisfied\n", matchedHooks[0].ID, len(matchedHooks))
|
||||
|
||||
fmt.Fprintf(w, "Hook rules were not satisfied.")
|
||||
} else if executed != len(matchedHooks) {
|
||||
log.Printf("%s got matched (%d time(s)), but executed only %d times. Please review previous log messages\n", matchedHooks[0].ID, len(matchedHooks), executed)
|
||||
}
|
||||
} else {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
fmt.Fprintf(w, "Hook not found.")
|
||||
|
||||
154
webhook_test.go
154
webhook_test.go
@ -434,6 +434,159 @@ var hookHandlerTests = []struct {
|
||||
`{"message":"success","output":"arg: b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327 John Smith john@example.com\n"}`,
|
||||
},
|
||||
|
||||
{
|
||||
"github",
|
||||
"github-called-twice",
|
||||
map[string]string{"X-Hub-Signature": "f68df0375d7b03e3eb29b4cf9f9ec12e08f42ff8"},
|
||||
`{
|
||||
"after":"1481a2de7b2a7d02428ad93446ab166be7793fbb",
|
||||
"before":"17c497ccc7cca9c2f735aa07e9e3813060ce9a6a",
|
||||
"commits":[
|
||||
{
|
||||
"added":[
|
||||
|
||||
],
|
||||
"author":{
|
||||
"email":"lolwut@noway.biz",
|
||||
"name":"Garen Torikian",
|
||||
"username":"octokitty"
|
||||
},
|
||||
"committer":{
|
||||
"email":"lolwut@noway.biz",
|
||||
"name":"Garen Torikian",
|
||||
"username":"octokitty"
|
||||
},
|
||||
"distinct":true,
|
||||
"id":"c441029cf673f84c8b7db52d0a5944ee5c52ff89",
|
||||
"message":"Test",
|
||||
"modified":[
|
||||
"README.md"
|
||||
],
|
||||
"removed":[
|
||||
|
||||
],
|
||||
"timestamp":"2013-02-22T13:50:07-08:00",
|
||||
"url":"https://github.com/octokitty/testing/commit/c441029cf673f84c8b7db52d0a5944ee5c52ff89"
|
||||
},
|
||||
{
|
||||
"added":[
|
||||
|
||||
],
|
||||
"author":{
|
||||
"email":"lolwut@noway.biz",
|
||||
"name":"Garen Torikian",
|
||||
"username":"octokitty"
|
||||
},
|
||||
"committer":{
|
||||
"email":"lolwut@noway.biz",
|
||||
"name":"Garen Torikian",
|
||||
"username":"octokitty"
|
||||
},
|
||||
"distinct":true,
|
||||
"id":"36c5f2243ed24de58284a96f2a643bed8c028658",
|
||||
"message":"This is me testing the windows client.",
|
||||
"modified":[
|
||||
"README.md"
|
||||
],
|
||||
"removed":[
|
||||
|
||||
],
|
||||
"timestamp":"2013-02-22T14:07:13-08:00",
|
||||
"url":"https://github.com/octokitty/testing/commit/36c5f2243ed24de58284a96f2a643bed8c028658"
|
||||
},
|
||||
{
|
||||
"added":[
|
||||
"words/madame-bovary.txt"
|
||||
],
|
||||
"author":{
|
||||
"email":"lolwut@noway.biz",
|
||||
"name":"Garen Torikian",
|
||||
"username":"octokitty"
|
||||
},
|
||||
"committer":{
|
||||
"email":"lolwut@noway.biz",
|
||||
"name":"Garen Torikian",
|
||||
"username":"octokitty"
|
||||
},
|
||||
"distinct":true,
|
||||
"id":"1481a2de7b2a7d02428ad93446ab166be7793fbb",
|
||||
"message":"Rename madame-bovary.txt to words/madame-bovary.txt",
|
||||
"modified":[
|
||||
|
||||
],
|
||||
"removed":[
|
||||
"madame-bovary.txt"
|
||||
],
|
||||
"timestamp":"2013-03-12T08:14:29-07:00",
|
||||
"url":"https://github.com/octokitty/testing/commit/1481a2de7b2a7d02428ad93446ab166be7793fbb"
|
||||
}
|
||||
],
|
||||
"compare":"https://github.com/octokitty/testing/compare/17c497ccc7cc...1481a2de7b2a",
|
||||
"created":false,
|
||||
"deleted":false,
|
||||
"forced":false,
|
||||
"head_commit":{
|
||||
"added":[
|
||||
"words/madame-bovary.txt"
|
||||
],
|
||||
"author":{
|
||||
"email":"lolwut@noway.biz",
|
||||
"name":"Garen Torikian",
|
||||
"username":"octokitty"
|
||||
},
|
||||
"committer":{
|
||||
"email":"lolwut@noway.biz",
|
||||
"name":"Garen Torikian",
|
||||
"username":"octokitty"
|
||||
},
|
||||
"distinct":true,
|
||||
"id":"1481a2de7b2a7d02428ad93446ab166be7793fbb",
|
||||
"message":"Rename madame-bovary.txt to words/madame-bovary.txt",
|
||||
"modified":[
|
||||
|
||||
],
|
||||
"removed":[
|
||||
"madame-bovary.txt"
|
||||
],
|
||||
"timestamp":"2013-03-12T08:14:29-07:00",
|
||||
"url":"https://github.com/octokitty/testing/commit/1481a2de7b2a7d02428ad93446ab166be7793fbb"
|
||||
},
|
||||
"pusher":{
|
||||
"email":"lolwut@noway.biz",
|
||||
"name":"Garen Torikian"
|
||||
},
|
||||
"ref":"refs/heads/master",
|
||||
"repository":{
|
||||
"created_at":1332977768,
|
||||
"description":"",
|
||||
"fork":false,
|
||||
"forks":0,
|
||||
"has_downloads":true,
|
||||
"has_issues":true,
|
||||
"has_wiki":true,
|
||||
"homepage":"",
|
||||
"id":3860742,
|
||||
"language":"Ruby",
|
||||
"master_branch":"master",
|
||||
"name":"testing",
|
||||
"open_issues":2,
|
||||
"owner":{
|
||||
"email":"lolwut@noway.biz",
|
||||
"name":"octokitty"
|
||||
},
|
||||
"private":false,
|
||||
"pushed_at":1363295520,
|
||||
"size":2156,
|
||||
"stargazers":1,
|
||||
"url":"https://github.com/octokitty/testing",
|
||||
"watchers":1
|
||||
}
|
||||
}`,
|
||||
false,
|
||||
http.StatusOK,
|
||||
`{"output":"arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb false\nenv: HOOK_created=false\n"}{"output":"arg: 1332977768\nenv: HOOK_pusher.email=lolwut@noway.biz\n"}`,
|
||||
},
|
||||
|
||||
{
|
||||
"missing-cmd-arg", // missing head_commit.author.email
|
||||
"github",
|
||||
@ -510,4 +663,5 @@ var hookHandlerTests = []struct {
|
||||
},
|
||||
|
||||
{"empty payload", "github", nil, `{}`, false, http.StatusOK, `Hook rules were not satisfied.`},
|
||||
{"non matching hook", "this-does-not-match-anything", nil, `{}`, false, http.StatusNotFound, `Hook not found.`},
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user