diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl index d65d1d3..2fd90da 100644 --- a/test/hooks.json.tmpl +++ b/test/hooks.json.tmpl @@ -180,5 +180,29 @@ "execute-command": "{{ .Hookecho }}", "include-command-output-in-response": true, "include-command-output-in-response-on-error": true + }, + { + "id": "static-params-ok", + "execute-command": "{{ .Hookecho }}", + "response-message": "success", + "include-command-output-in-response": true, + "pass-arguments-to-command": [ + { + "source": "string", + "name": "passed" + } + ], + }, + { + "id": "warn-on-space", + "execute-command": "{{ .Hookecho }} foo", + "response-message": "success", + "include-command-output-in-response": true, + "pass-arguments-to-command": [ + { + "source": "string", + "name": "passed" + } + ], } ] diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl index c0377e5..c2fffcd 100644 --- a/test/hooks.yaml.tmpl +++ b/test/hooks.yaml.tmpl @@ -1,4 +1,5 @@ -- trigger-rule: +- id: github + trigger-rule: and: - match: parameter: @@ -23,9 +24,10 @@ pass-environment-to-command: - source: payload name: head_commit.timestamp - id: github command-working-directory: / -- trigger-rule: + +- id: bitbucket + trigger-rule: and: - match: parameter: @@ -52,9 +54,10 @@ execute-command: '{{ .Hookecho }}' response-message: success include-command-output-in-response: false - id: bitbucket command-working-directory: / -- trigger-rule: + +- id: gitlab + trigger-rule: match: parameter: source: payload @@ -71,25 +74,28 @@ execute-command: '{{ .Hookecho }}' response-message: success include-command-output-in-response: true - id: gitlab command-working-directory: / + - id: capture-command-output-on-success-not-by-default pass-arguments-to-command: - source: string name: exit=0 execute-command: '{{ .Hookecho }}' + - id: capture-command-output-on-success-yes-with-flag pass-arguments-to-command: - source: string name: exit=0 execute-command: '{{ .Hookecho }}' include-command-output-in-response: true + - id: capture-command-output-on-error-not-by-default pass-arguments-to-command: - source: string name: exit=1 execute-command: '{{ .Hookecho }}' include-command-output-in-response: true + - id: capture-command-output-on-error-yes-with-extra-flag pass-arguments-to-command: - source: string @@ -97,3 +103,14 @@ execute-command: '{{ .Hookecho }}' include-command-output-in-response: true include-command-output-in-response-on-error: true + +- id: static-params-ok + execute-command: '{{ .Hookecho }}' + include-command-output-in-response: true + pass-arguments-to-command: + - source: string + name: passed + +- id: warn-on-space + execute-command: '{{ .Hookecho }} foo' + include-command-output-in-response: true \ No newline at end of file diff --git a/webhook_test.go b/webhook_test.go index 4373b16..6113b9b 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -13,6 +13,7 @@ import ( "regexp" "runtime" "strings" + "sync" "testing" "text/template" "time" @@ -21,15 +22,26 @@ import ( ) func TestStaticParams(t *testing.T) { + // FIXME(moorereason): incorporate this test into TestWebhook. + // Need to be able to execute a binary with a space in the filename. + if runtime.GOOS == "windows" { + t.Skip("Skipping on Windows") + } spHeaders := make(map[string]interface{}) spHeaders["User-Agent"] = "curl/7.54.0" spHeaders["Accept"] = "*/*" - // case 1: correct entry + // case 2: binary with spaces in its name + err := os.Symlink("/bin/echo", "/tmp/with space") + if err != nil { + t.Fatalf("%v", err) + } + defer os.Remove("/tmp/with space") + spHook := &hook.Hook{ - ID: "static-params-ok", - ExecuteCommand: "/bin/echo", + ID: "static-params-name-space", + ExecuteCommand: "/tmp/with space", CommandWorkingDirectory: "/tmp", ResponseMessage: "success", CaptureCommandOutput: true, @@ -41,159 +53,126 @@ func TestStaticParams(t *testing.T) { b := &bytes.Buffer{} log.SetOutput(b) - s, err := handleHook(spHook, "test", &spHeaders, &map[string]interface{}{}, &map[string]interface{}{}, &[]byte{}) + _, err = handleHook(spHook, "test", &spHeaders, &map[string]interface{}{}, &map[string]interface{}{}, &[]byte{}) if err != nil { t.Fatalf("Unexpected error: %v\n", err) } - matched, _ := regexp.MatchString("(?s).*command output: passed.*static-params-ok", b.String()) - if !matched { - t.Fatalf("Unexpected log output:\n%s", b) - } - - // case 2: binary with spaces in its name - err = os.Symlink("/bin/echo", "/tmp/with space") - if err != nil { - t.Fatalf("%v", err) - } - defer os.Remove("/tmp/with space") - - spHook = &hook.Hook{ - ID: "static-params-name-space", - ExecuteCommand: "/tmp/with space", - CommandWorkingDirectory: "/tmp", - ResponseMessage: "success", - CaptureCommandOutput: true, - PassArgumentsToCommand: []hook.Argument{ - hook.Argument{Source: "string", Name: "passed"}, - }, - } - - b = &bytes.Buffer{} - log.SetOutput(b) - - s, err = handleHook(spHook, "test", &spHeaders, &map[string]interface{}{}, &map[string]interface{}{}, &[]byte{}) - if err != nil { - t.Fatalf("Unexpected error: %v\n", err) - } - matched, _ = regexp.MatchString("(?s)command output: .*static-params-name-space", b.String()) + matched, _ := regexp.MatchString("(?s)command output: .*static-params-name-space", b.String()) if !matched { t.Fatalf("Unexpected log output:\n%sn", b) } - - // case 3: parameters specified in execute-command - spHook = &hook.Hook{ - ID: "static-params-bad", - ExecuteCommand: "/bin/echo success", - CommandWorkingDirectory: "/tmp", - ResponseMessage: "success", - CaptureCommandOutput: true, - PassArgumentsToCommand: []hook.Argument{ - hook.Argument{Source: "string", Name: "failed"}, - }, - } - - b = &bytes.Buffer{} - log.SetOutput(b) - - s, err = handleHook(spHook, "test", &spHeaders, &map[string]interface{}{}, &map[string]interface{}{}, &[]byte{}) - if err == nil { - t.Fatalf("Error expected, but none returned: %s\n", s) - } - matched, _ = regexp.MatchString("(?s)unable to locate command: ..bin.echo success.*use 'pass-arguments-to-command'", b.String()) - if !matched { - t.Fatalf("Unexpected log output:\n%s\n", b) - } } func TestWebhook(t *testing.T) { hookecho, cleanupHookecho := buildHookecho(t) defer cleanupHookecho() + webhook, cleanupWebhookFn := buildWebhook(t) + defer cleanupWebhookFn() + for _, hookTmpl := range []string{"test/hooks.json.tmpl", "test/hooks.yaml.tmpl"} { - config, cleanupConfig := genConfig(t, hookecho, hookTmpl) - defer cleanupConfig() - - webhook, cleanupWebhook := buildWebhook(t) - defer cleanupWebhook() - - ip, port := serverAddress(t) - args := []string{fmt.Sprintf("-hooks=%s", config), fmt.Sprintf("-ip=%s", ip), fmt.Sprintf("-port=%s", port), "-verbose"} - - cmd := exec.Command(webhook, args...) - //cmd.Stderr = os.Stderr // uncomment to see verbose output - cmd.Env = webhookEnv() - cmd.Args[0] = "webhook" - if err := cmd.Start(); err != nil { - t.Fatalf("failed to start webhook: %s", err) - } - defer killAndWait(cmd) - - waitForServerReady(t, ip, port) + configPath, cleanupConfigFn := genConfig(t, hookecho, hookTmpl) + defer cleanupConfigFn() for _, tt := range hookHandlerTests { - url := fmt.Sprintf("http://%s:%s/hooks/%s", ip, port, tt.id) + t.Run(tt.desc, func(t *testing.T) { - req, err := http.NewRequest("POST", url, ioutil.NopCloser(strings.NewReader(tt.body))) - if err != nil { - t.Errorf("New request failed: %s", err) - } + ip, port := serverAddress(t) + args := []string{fmt.Sprintf("-hooks=%s", configPath), fmt.Sprintf("-ip=%s", ip), fmt.Sprintf("-port=%s", port), "-verbose"} - for k, v := range tt.headers { - req.Header.Add(k, v) - } + // Setup a buffer for capturing webhook logs for later evaluation + b := &buffer{} - var res *http.Response + cmd := exec.Command(webhook, args...) + cmd.Stderr = b + cmd.Env = webhookEnv() + cmd.Args[0] = "webhook" + if err := cmd.Start(); err != nil { + t.Fatalf("failed to start webhook: %s", err) + } + defer killAndWait(cmd) - if tt.urlencoded == true { - req.Header.Add("Content-Type", "application/x-www-form-urlencoded") - } else { - req.Header.Add("Content-Type", "application/json") - } + waitForServerReady(t, ip, port) - client := &http.Client{} - res, err = client.Do(req) - if err != nil { - t.Errorf("client.Do failed: %s", err) - } + url := fmt.Sprintf("http://%s:%s/hooks/%s", ip, port, tt.id) - body, err := ioutil.ReadAll(res.Body) - res.Body.Close() - if err != nil { - t.Errorf("POST %q: failed to ready body: %s", tt.desc, err) - } + req, err := http.NewRequest("POST", url, ioutil.NopCloser(strings.NewReader(tt.body))) + if err != nil { + t.Errorf("New request failed: %s", err) + } - if res.StatusCode != tt.respStatus || string(body) != tt.respBody { - t.Errorf("failed %q (id: %s):\nexpected status: %#v, response: %s\ngot status: %#v, response: %s", tt.desc, tt.id, tt.respStatus, tt.respBody, res.StatusCode, body) - } + for k, v := range tt.headers { + req.Header.Add(k, v) + } + + var res *http.Response + + if tt.urlencoded == true { + req.Header.Add("Content-Type", "application/x-www-form-urlencoded") + } else { + req.Header.Add("Content-Type", "application/json") + } + + client := &http.Client{} + res, err = client.Do(req) + if err != nil { + t.Errorf("client.Do failed: %s", err) + } + + body, err := ioutil.ReadAll(res.Body) + res.Body.Close() + if err != nil { + t.Errorf("POST %q: failed to ready body: %s", tt.desc, err) + } + + if res.StatusCode != tt.respStatus || string(body) != tt.respBody { + t.Errorf("failed %q (id: %s):\nexpected status: %#v, response: %s\ngot status: %#v, response: %s", tt.desc, tt.id, tt.respStatus, tt.respBody, res.StatusCode, body) + } + + if tt.logMatch == "" { + return + } + + // There's the potential for a race condition below where we + // try to read the logs buffer b before the logs have been + // flushed by the webhook process. Kill the process to flush + // the logs. + killAndWait(cmd) + + matched, _ := regexp.MatchString(tt.logMatch, b.String()) + if !matched { + t.Errorf("failed log match for %q (id: %s):\nmatch pattern: %q\ngot:\n%s", tt.desc, tt.id, tt.logMatch, b) + } + }) } } } -func buildHookecho(t *testing.T) (bin string, cleanup func()) { +func buildHookecho(t *testing.T) (binPath string, cleanupFn func()) { tmp, err := ioutil.TempDir("", "hookecho-test-") if err != nil { t.Fatal(err) } defer func() { - if cleanup == nil { + if cleanupFn == nil { os.RemoveAll(tmp) } }() - bin = filepath.Join(tmp, "hookecho") + binPath = filepath.Join(tmp, "hookecho") if runtime.GOOS == "windows" { - bin += ".exe" + binPath += ".exe" } - cmd := exec.Command("go", "build", "-o", bin, "test/hookecho.go") + cmd := exec.Command("go", "build", "-o", binPath, "test/hookecho.go") if err := cmd.Run(); err != nil { t.Fatalf("Building hookecho: %v", err) } - return bin, func() { os.RemoveAll(tmp) } + return binPath, func() { os.RemoveAll(tmp) } } -func genConfig(t *testing.T, bin string, hookTemplate string) (config string, cleanup func()) { +func genConfig(t *testing.T, bin string, hookTemplate string) (configPath string, cleanupFn func()) { tmpl := template.Must(template.ParseFiles(hookTemplate)) tmp, err := ioutil.TempDir("", "webhook-config-") @@ -201,7 +180,7 @@ func genConfig(t *testing.T, bin string, hookTemplate string) (config string, cl t.Fatal(err) } defer func() { - if cleanup == nil { + if cleanupFn == nil { os.RemoveAll(tmp) } }() @@ -215,7 +194,11 @@ func genConfig(t *testing.T, bin string, hookTemplate string) (config string, cl } defer file.Close() - data := struct{ Hookecho string }{filepath.ToSlash(bin)} + data := struct{ Hookecho string }{filepath.FromSlash(bin)} + if runtime.GOOS == "windows" { + // Simulate escaped backslashes on Windows. + data.Hookecho = strings.Replace(data.Hookecho, `\`, `\\`, -1) + } if err := tmpl.Execute(file, data); err != nil { t.Fatalf("Executing template: %v", err) } @@ -223,28 +206,28 @@ func genConfig(t *testing.T, bin string, hookTemplate string) (config string, cl return path, func() { os.RemoveAll(tmp) } } -func buildWebhook(t *testing.T) (bin string, cleanup func()) { +func buildWebhook(t *testing.T) (binPath string, cleanupFn func()) { tmp, err := ioutil.TempDir("", "webhook-test-") if err != nil { t.Fatal(err) } defer func() { - if cleanup == nil { + if cleanupFn == nil { os.RemoveAll(tmp) } }() - bin = filepath.Join(tmp, "webhook") + binPath = filepath.Join(tmp, "webhook") if runtime.GOOS == "windows" { - bin += ".exe" + binPath += ".exe" } - cmd := exec.Command("go", "build", "-o", bin) + cmd := exec.Command("go", "build", "-o", binPath) if err := cmd.Run(); err != nil { t.Fatalf("Building webhook: %v", err) } - return bin, func() { os.RemoveAll(tmp) } + return binPath, func() { os.RemoveAll(tmp) } } func serverAddress(t *testing.T) (string, string) { @@ -288,6 +271,10 @@ func waitForServer(t *testing.T, url string, status int, timeout time.Duration) } func killAndWait(cmd *exec.Cmd) { + if cmd == nil || cmd.ProcessState != nil && cmd.ProcessState.Exited() { + return + } + cmd.Process.Kill() cmd.Wait() } @@ -313,6 +300,7 @@ var hookHandlerTests = []struct { respStatus int respBody string + logMatch string }{ { "github", @@ -467,6 +455,7 @@ var hookHandlerTests = []struct { `arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 `, + ``, }, { "bitbucket", // bitbucket sends their payload using uriencoded params. @@ -476,6 +465,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 true, http.StatusOK, `success`, + ``, }, { "gitlab", @@ -527,6 +517,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 http.StatusOK, `arg: b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327 John Smith john@example.com `, + ``, }, { @@ -567,6 +558,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 `arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 `, + ``, }, { @@ -605,20 +597,55 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 http.StatusOK, `arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz `, + ``, }, // test with custom return code - {"empty payload", "github", nil, `{}`, false, http.StatusBadRequest, `Hook rules were not satisfied.`}, + {"empty payload", "github", nil, `{}`, false, http.StatusBadRequest, `Hook rules were not satisfied.`, ``}, // test with custom invalid http code, should default to 200 OK - {"empty payload", "bitbucket", nil, `{}`, false, http.StatusOK, `Hook rules were not satisfied.`}, + {"empty payload", "bitbucket", nil, `{}`, false, http.StatusOK, `Hook rules were not satisfied.`, ``}, // test with no configured http return code, should default to 200 OK - {"empty payload", "gitlab", nil, `{}`, false, http.StatusOK, `Hook rules were not satisfied.`}, + {"empty payload", "gitlab", nil, `{}`, false, http.StatusOK, `Hook rules were not satisfied.`, ``}, // test capturing command output - {"don't capture output on success by default", "capture-command-output-on-success-not-by-default", nil, `{}`, false, http.StatusOK, ``}, + {"don't capture output on success by default", "capture-command-output-on-success-not-by-default", nil, `{}`, false, http.StatusOK, ``, ``}, {"capture output on success with flag set", "capture-command-output-on-success-yes-with-flag", nil, `{}`, false, http.StatusOK, `arg: exit=0 -`}, - {"don't capture output on error by default", "capture-command-output-on-error-not-by-default", nil, `{}`, false, http.StatusInternalServerError, `Error occurred while executing the hook's command. Please check your logs for more details.`}, +`, ``}, + {"don't capture output on error by default", "capture-command-output-on-error-not-by-default", nil, `{}`, false, http.StatusInternalServerError, `Error occurred while executing the hook's command. Please check your logs for more details.`, ``}, {"capture output on error with extra flag set", "capture-command-output-on-error-yes-with-extra-flag", nil, `{}`, false, http.StatusInternalServerError, `arg: exit=1 -`}, +`, ``}, + + // Check logs + {"static params should pass", "static-params-ok", nil, `{}`, false, http.StatusOK, "arg: passed\n", `(?s)command output: arg: passed`}, + {"command with space logs warning", "warn-on-space", nil, `{}`, false, http.StatusInternalServerError, "Error occurred while executing the hook's command. Please check your logs for more details.", `(?s)unable to locate command.*use 'pass[-]arguments[-]to[-]command' to specify args`}, +} + +// buffer provides a concurrency-safe bytes.Buffer to tests above. +type buffer struct { + b bytes.Buffer + m sync.Mutex +} + +func (b *buffer) Read(p []byte) (n int, err error) { + b.m.Lock() + defer b.m.Unlock() + return b.b.Read(p) +} + +func (b *buffer) Write(p []byte) (n int, err error) { + b.m.Lock() + defer b.m.Unlock() + return b.b.Write(p) +} + +func (b *buffer) String() string { + b.m.Lock() + defer b.m.Unlock() + return b.b.String() +} + +func (b *buffer) Reset() { + b.m.Lock() + defer b.m.Unlock() + b.b.Reset() }