From 6b4f368dfeb6a9641394973f7811c9408661077b Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sat, 8 Apr 2017 01:13:55 -0700 Subject: [PATCH] notify: Webhook endpoints can fail, but we must start the server. (#4060) Ignore any network errors when registering a webhook notifier during Minio startup sequence. This way server can be started even if the webhook endpoint is not available and unreachable. --- cmd/event-notifier.go | 1 - cmd/notify-webhook.go | 123 ++++++++++++++++++++++++++++++++----- cmd/notify-webhook_test.go | 2 +- cmd/utils.go | 18 ------ 4 files changed, 110 insertions(+), 34 deletions(-) diff --git a/cmd/event-notifier.go b/cmd/event-notifier.go index b38c4c053..8bbbcaf00 100644 --- a/cmd/event-notifier.go +++ b/cmd/event-notifier.go @@ -645,7 +645,6 @@ func loadAllQueueTargets() (map[string]*logrus.Logger, error) { if !webhookN.Enable { continue } - if _, err := addQueueTarget(queueTargets, accountID, queueTypeWebhook, newWebhookNotify); err != nil { return nil, err } diff --git a/cmd/notify-webhook.go b/cmd/notify-webhook.go index 01b141878..3bc036d0d 100644 --- a/cmd/notify-webhook.go +++ b/cmd/notify-webhook.go @@ -17,12 +17,14 @@ package cmd import ( + "bytes" "crypto/tls" "fmt" "io/ioutil" "net" "net/http" "net/url" + "strings" "time" "github.com/Sirupsen/logrus" @@ -48,33 +50,126 @@ type httpConn struct { Endpoint string } -// Lookup endpoint address by successfully dialing. -func lookupEndpoint(u *url.URL) error { - dialer := &net.Dialer{ - Timeout: 300 * time.Millisecond, - KeepAlive: 300 * time.Millisecond, +// List of success status. +var successStatus = []int{ + http.StatusOK, + http.StatusAccepted, + http.StatusContinue, + http.StatusNoContent, + http.StatusPartialContent, +} + +// isNetErrorIgnored - is network error ignored. +func isNetErrorIgnored(err error) bool { + if err == nil { + return false + } + if strings.Contains(err.Error(), "Client.Timeout exceeded while awaiting headers") { + return true } - nconn, err := dialer.Dial("tcp", canonicalAddr(u)) + switch err.(type) { + case net.Error: + switch err.(type) { + case *net.DNSError, *net.OpError, net.UnknownNetworkError: + return true + case *url.Error: + // For a URL error, where it replies back "connection closed" + // retry again. + if strings.Contains(err.Error(), "Connection closed by foreign host") { + return true + } + default: + if strings.Contains(err.Error(), "net/http: TLS handshake timeout") { + // If error is - tlsHandshakeTimeoutError, retry. + return true + } else if strings.Contains(err.Error(), "i/o timeout") { + // If error is - tcp timeoutError, retry. + return true + } else if strings.Contains(err.Error(), "connection timed out") { + // If err is a net.Dial timeout, retry. + return true + } + } + } + return false +} + +// Lookup endpoint address by successfully POSTting +// a JSON which would send out minio release. +func lookupEndpoint(urlStr string) error { + minioRelease := bytes.NewReader([]byte(ReleaseTag)) + req, err := http.NewRequest("POST", urlStr, minioRelease) if err != nil { return err } - return nconn.Close() + + client := &http.Client{ + Timeout: 1 * time.Second, + Transport: &http.Transport{ + // need to close connection after usage. + DisableKeepAlives: true, + }, + } + + // Set content-type. + req.Header.Set("Content-Type", "application/json") + + // Set proper server user-agent. + req.Header.Set("User-Agent", globalServerUserAgent) + + // Retry if the request needs to be re-directed. + // This code is necessary since Go 1.7.x do not + // support retrying for http 307 for POST operation. + // https://github.com/golang/go/issues/7912 + // + // FIXME: Remove this when we move to Go 1.8. + for { + resp, derr := client.Do(req) + if derr != nil { + if isNetErrorIgnored(derr) { + return nil + } + return derr + } + if resp == nil { + return fmt.Errorf("No response from server to download URL %s", urlStr) + } + resp.Body.Close() + + // Redo the request with the new redirect url if http 307 + // is returned, quit the loop otherwise + if resp.StatusCode == http.StatusTemporaryRedirect { + newURL, uerr := url.Parse(resp.Header.Get("Location")) + if uerr != nil { + return uerr + } + req.URL = newURL + continue + } + + // For any known successful http status, return quickly. + for _, httpStatus := range successStatus { + if httpStatus == resp.StatusCode { + return nil + } + } + + err = fmt.Errorf("Unexpected response from webhook server %s: (%s)", urlStr, resp.Status) + break + } + + // Succes. + return err } // Initializes new webhook logrus notifier. func newWebhookNotify(accountID string) (*logrus.Logger, error) { rNotify := serverConfig.Notify.GetWebhookByID(accountID) - if rNotify.Endpoint == "" { return nil, errInvalidArgument } - u, err := url.Parse(rNotify.Endpoint) - if err != nil { - return nil, err - } - - if err = lookupEndpoint(u); err != nil { + if err := lookupEndpoint(rNotify.Endpoint); err != nil { return nil, err } diff --git a/cmd/notify-webhook_test.go b/cmd/notify-webhook_test.go index ff974f751..7fd4382d1 100644 --- a/cmd/notify-webhook_test.go +++ b/cmd/notify-webhook_test.go @@ -51,7 +51,7 @@ func TestNewWebHookNotify(t *testing.T) { t.Fatal("Unexpected should fail") } - serverConfig.Notify.SetWebhookByID("10", webhookNotify{Enable: true, Endpoint: "http://www."}) + serverConfig.Notify.SetWebhookByID("10", webhookNotify{Enable: true, Endpoint: "http://127.0.0.1:xxx"}) _, err = newWebhookNotify("10") if err == nil { t.Fatal("Unexpected should fail with lookupHost") diff --git a/cmd/utils.go b/cmd/utils.go index ab194d189..39b8b7c9f 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -110,24 +110,6 @@ const ( httpsScheme = "https" ) -var portMap = map[string]string{ - httpScheme: "80", - httpsScheme: "443", -} - -// Given a string of the form "host", "host:port", or "[ipv6::address]:port", -// return true if the string includes a port. -func hasPort(s string) bool { return strings.LastIndex(s, ":") > strings.LastIndex(s, "]") } - -// canonicalAddr returns url.Host but always with a ":port" suffix -func canonicalAddr(u *url.URL) string { - addr := u.Host - if !hasPort(addr) { - return addr + ":" + portMap[u.Scheme] - } - return addr -} - // checkDuplicates - function to validate if there are duplicates in a slice of endPoints. func checkDuplicateEndpoints(endpoints []*url.URL) error { var strs []string