Webhook endpoints can fail, we must start the server. (#4255)

This PR fixes a regression introduced in #4060
master
Harshavardhana 8 years ago committed by Dee Koder
parent a02575ebf9
commit df027a8f51
  1. 8
      cmd/notify-webhook.go
  2. 43
      cmd/notify-webhook_test.go

@ -69,10 +69,15 @@ func isNetErrorIgnored(err error) bool {
} }
switch err.(type) { switch err.(type) {
case net.Error: case net.Error:
switch err.(type) { switch e := err.(type) {
case *net.DNSError, *net.OpError, net.UnknownNetworkError: case *net.DNSError, *net.OpError, net.UnknownNetworkError:
return true return true
case *url.Error: case *url.Error:
// Fixes https://github.com/minio/minio/issues/4050
switch e.Err.(type) {
case *net.DNSError, *net.OpError, net.UnknownNetworkError:
return true
}
// For a URL error, where it replies back "connection closed" // For a URL error, where it replies back "connection closed"
// retry again. // retry again.
if strings.Contains(err.Error(), "Connection closed by foreign host") { if strings.Contains(err.Error(), "Connection closed by foreign host") {
@ -127,6 +132,7 @@ func lookupEndpoint(urlStr string) error {
resp, derr := client.Do(req) resp, derr := client.Do(req)
if derr != nil { if derr != nil {
if isNetErrorIgnored(derr) { if isNetErrorIgnored(derr) {
errorIf(derr, "Unable to lookup webhook endpoint %s", urlStr)
return nil return nil
} }
return derr return derr

@ -17,6 +17,7 @@
package cmd package cmd
import ( import (
"errors"
"fmt" "fmt"
"io" "io"
"net/http" "net/http"
@ -38,6 +39,12 @@ func (p postHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
io.Copy(w, r.Body) io.Copy(w, r.Body)
} }
type errorHandler struct{}
func (e errorHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
http.Error(w, fmt.Sprintf("Unexpected method %s", r.Method), http.StatusBadRequest)
}
// Tests web hook initialization. // Tests web hook initialization.
func TestNewWebHookNotify(t *testing.T) { func TestNewWebHookNotify(t *testing.T) {
root, err := newTestConfig(globalMinioDefaultRegion) root, err := newTestConfig(globalMinioDefaultRegion)
@ -53,8 +60,8 @@ func TestNewWebHookNotify(t *testing.T) {
serverConfig.Notify.SetWebhookByID("10", webhookNotify{Enable: true, Endpoint: "http://127.0.0.1:xxx"}) serverConfig.Notify.SetWebhookByID("10", webhookNotify{Enable: true, Endpoint: "http://127.0.0.1:xxx"})
_, err = newWebhookNotify("10") _, err = newWebhookNotify("10")
if err == nil { if err != nil {
t.Fatal("Unexpected should fail with lookupHost") t.Fatal("Unexpected should not fail with lookupEndpoint", err)
} }
serverConfig.Notify.SetWebhookByID("15", webhookNotify{Enable: true, Endpoint: "http://%"}) serverConfig.Notify.SetWebhookByID("15", webhookNotify{Enable: true, Endpoint: "http://%"})
@ -77,3 +84,35 @@ func TestNewWebHookNotify(t *testing.T) {
"EventType": "s3:ObjectCreated:Put", "EventType": "s3:ObjectCreated:Put",
}).Info() }).Info()
} }
// Add tests for lookup endpoint.
func TestLookupEndpoint(t *testing.T) {
server := httptest.NewServer(errorHandler{})
defer server.Close()
testCases := []struct {
endpoint string
err error
}{
// Ignore endpoints which don't exist.
{
endpoint: "http://unknown",
err: nil,
},
{
endpoint: "%%%",
err: errors.New("parse %%%: invalid URL escape \"%%%\""),
},
{
endpoint: server.URL,
err: fmt.Errorf("Unexpected response from webhook server %s: (400 Bad Request)", server.URL),
},
}
for _, test := range testCases {
if err := lookupEndpoint(test.endpoint); err != nil {
if err.Error() != test.err.Error() {
t.Errorf("Expected %s, got %s", test.err, err)
}
}
}
}

Loading…
Cancel
Save