From f7c1a59de1be8bac1f59da247f17e51d0bb0053c Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sun, 16 Aug 2020 10:25:00 -0700 Subject: [PATCH] add validation logs for configured Logger/Audit HTTP targets (#10274) extra logs in-case of misconfiguration of audit/logger targets --- cmd/config-current.go | 12 +++-- cmd/consolelogger.go | 5 +++ cmd/data-update-tracker_test.go | 4 ++ cmd/logger/audit.go | 9 ---- cmd/logger/target/console/console.go | 5 +++ cmd/logger/target/http/http.go | 65 +++++++++++++++++++++++++++- cmd/logger/targets.go | 21 ++++++++- 7 files changed, 105 insertions(+), 16 deletions(-) diff --git a/cmd/config-current.go b/cmd/config-current.go index 13c826391..76bd91ded 100644 --- a/cmd/config-current.go +++ b/cmd/config-current.go @@ -455,28 +455,32 @@ func lookupConfigs(s config.Config, setDriveCount int) { for _, l := range loggerCfg.HTTP { if l.Enabled { // Enable http logging - logger.AddTarget( + if err = logger.AddTarget( http.New(http.WithEndpoint(l.Endpoint), http.WithAuthToken(l.AuthToken), http.WithUserAgent(loggerUserAgent), http.WithLogKind(string(logger.All)), http.WithTransport(NewGatewayHTTPTransport()), ), - ) + ); err != nil { + logger.LogIf(ctx, fmt.Errorf("Unable to initialize console HTTP target: %w", err)) + } } } for _, l := range loggerCfg.Audit { if l.Enabled { // Enable http audit logging - logger.AddAuditTarget( + if err = logger.AddAuditTarget( http.New(http.WithEndpoint(l.Endpoint), http.WithAuthToken(l.AuthToken), http.WithUserAgent(loggerUserAgent), http.WithLogKind(string(logger.All)), http.WithTransport(NewGatewayHTTPTransport()), ), - ) + ); err != nil { + logger.LogIf(ctx, fmt.Errorf("Unable to initialize audit HTTP target: %w", err)) + } } } diff --git a/cmd/consolelogger.go b/cmd/consolelogger.go index 81f8438d6..7725633ef 100644 --- a/cmd/consolelogger.go +++ b/cmd/consolelogger.go @@ -117,6 +117,11 @@ func (sys *HTTPConsoleLoggerSys) Subscribe(subCh chan interface{}, doneCh <-chan sys.pubsub.Subscribe(subCh, doneCh, filter) } +// Validate if HTTPConsoleLoggerSys is valid, always returns nil right now +func (sys *HTTPConsoleLoggerSys) Validate() error { + return nil +} + // Send log message 'e' to console and publish to console // log pubsub system func (sys *HTTPConsoleLoggerSys) Send(e interface{}, logKind string) error { diff --git a/cmd/data-update-tracker_test.go b/cmd/data-update-tracker_test.go index 9af0c9d11..ee35ab6f2 100644 --- a/cmd/data-update-tracker_test.go +++ b/cmd/data-update-tracker_test.go @@ -42,6 +42,10 @@ type testingLogger struct { t testLoggerI } +func (t *testingLogger) Validate() error { + return nil +} + func (t *testingLogger) Send(entry interface{}, errKind string) error { t.mu.Lock() defer t.mu.Unlock() diff --git a/cmd/logger/audit.go b/cmd/logger/audit.go index 9883f9f05..84c0a3d54 100644 --- a/cmd/logger/audit.go +++ b/cmd/logger/audit.go @@ -125,15 +125,6 @@ func (lrw *ResponseWriter) Size() int { return lrw.bytesWritten } -// AuditTargets is the list of enabled audit loggers -var AuditTargets = []Target{} - -// AddAuditTarget adds a new audit logger target to the -// list of enabled loggers -func AddAuditTarget(t Target) { - AuditTargets = append(AuditTargets, t) -} - // AuditLog - logs audit logs to all audit targets. func AuditLog(w http.ResponseWriter, r *http.Request, api string, reqClaims map[string]interface{}, filterKeys ...string) { // Fast exit if there is not audit target configured diff --git a/cmd/logger/target/console/console.go b/cmd/logger/target/console/console.go index 74e0c7b2b..8e99ca0b8 100644 --- a/cmd/logger/target/console/console.go +++ b/cmd/logger/target/console/console.go @@ -32,6 +32,11 @@ import ( // in plain or json format to the standard output. type Target struct{} +// Validate - validate if the tty can be written to +func (c *Target) Validate() error { + return nil +} + // Send log message 'e' to console func (c *Target) Send(e interface{}, logKind string) error { entry, ok := e.(log.Entry) diff --git a/cmd/logger/target/http/http.go b/cmd/logger/target/http/http.go index b777a01f8..1c5a72689 100644 --- a/cmd/logger/target/http/http.go +++ b/cmd/logger/target/http/http.go @@ -18,12 +18,16 @@ package http import ( "bytes" + "context" "encoding/json" "errors" + "fmt" "net/http" "strings" + "time" xhttp "github.com/minio/minio/cmd/http" + "github.com/minio/minio/cmd/logger" ) // Target implements logger.Target and sends the json @@ -45,6 +49,47 @@ type Target struct { client http.Client } +// Validate validate the http target +func (h *Target) Validate() error { + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, h.endpoint, strings.NewReader(`{}`)) + if err != nil { + return err + } + + req.Header.Set(xhttp.ContentType, "application/json") + + // Set user-agent to indicate MinIO release + // version to the configured log endpoint + req.Header.Set("User-Agent", h.userAgent) + + if h.authToken != "" { + req.Header.Set("Authorization", h.authToken) + } + + resp, err := h.client.Do(req) + if err != nil { + return err + } + + // Drain any response. + xhttp.DrainBody(resp.Body) + + if resp.StatusCode != http.StatusOK { + switch resp.StatusCode { + case http.StatusForbidden: + return fmt.Errorf("%s returned '%s', please check if your auth token is correctly set", + h.endpoint, resp.Status) + } + return fmt.Errorf("%s returned '%s', please check your endpoint configuration", + h.endpoint, resp.Status) + } + + return nil +} + func (h *Target) startHTTPLogger() { // Create a routine which sends json logs received // from an internal channel. @@ -55,8 +100,11 @@ func (h *Target) startHTTPLogger() { continue } - req, err := http.NewRequest(http.MethodPost, h.endpoint, bytes.NewReader(logJSON)) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, + h.endpoint, bytes.NewReader(logJSON)) if err != nil { + cancel() continue } req.Header.Set(xhttp.ContentType, "application/json") @@ -70,13 +118,26 @@ func (h *Target) startHTTPLogger() { } resp, err := h.client.Do(req) + cancel() if err != nil { - h.client.CloseIdleConnections() + logger.LogIf(ctx, fmt.Errorf("%s returned '%w', please check your endpoint configuration\n", + h.endpoint, err)) continue } // Drain any response. xhttp.DrainBody(resp.Body) + + if resp.StatusCode != http.StatusOK { + switch resp.StatusCode { + case http.StatusForbidden: + logger.LogIf(ctx, fmt.Errorf("%s returned '%s', please check if your auth token is correctly set", + h.endpoint, resp.Status)) + default: + logger.LogIf(ctx, fmt.Errorf("%s returned '%s', please check your endpoint configuration", + h.endpoint, resp.Status)) + } + } } }() } diff --git a/cmd/logger/targets.go b/cmd/logger/targets.go index dbc443937..0a166deb1 100644 --- a/cmd/logger/targets.go +++ b/cmd/logger/targets.go @@ -20,14 +20,33 @@ package logger // a single log entry and Send it to the log target // e.g. Send the log to a http server type Target interface { + Validate() error Send(entry interface{}, errKind string) error } // Targets is the set of enabled loggers var Targets = []Target{} +// AuditTargets is the list of enabled audit loggers +var AuditTargets = []Target{} + +// AddAuditTarget adds a new audit logger target to the +// list of enabled loggers +func AddAuditTarget(t Target) error { + if err := t.Validate(); err != nil { + return err + } + + AuditTargets = append(AuditTargets, t) + return nil +} + // AddTarget adds a new logger target to the // list of enabled loggers -func AddTarget(t Target) { +func AddTarget(t Target) error { + if err := t.Validate(); err != nil { + return err + } Targets = append(Targets, t) + return nil }