From 165b26460ab404bd4100da29a6b48076cc6255eb Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Tue, 8 Oct 2019 14:55:25 +0200 Subject: [PATCH] Unauth metrics: Fix missing parse form and error response (#7569) * Unauth metrics: Fix missing parse form and error response * Change metrics error response to text/plain content type always --- helper/metricsutil/metricsutil.go | 75 +++++++++++++++++++------------ http/sys_metrics.go | 24 +++++++--- http/sys_metrics_test.go | 6 ++- vault/logical_system.go | 2 +- 4 files changed, 69 insertions(+), 38 deletions(-) diff --git a/helper/metricsutil/metricsutil.go b/helper/metricsutil/metricsutil.go index 13db458029..f6ac99f892 100644 --- a/helper/metricsutil/metricsutil.go +++ b/helper/metricsutil/metricsutil.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "fmt" + "net/http" "strings" "github.com/armon/go-metrics" @@ -14,6 +15,9 @@ import ( const ( OpenMetricsMIMEType = "application/openmetrics-text" + + // ErrorContentType is the content type returned by an error response. + ErrorContentType = "text/plain" ) const ( @@ -40,30 +44,39 @@ func FormatFromRequest(req *logical.Request) string { return "" } -func (m *MetricsHelper) ResponseForFormat(format string) (*logical.Response, error) { +func (m *MetricsHelper) ResponseForFormat(format string) *logical.Response { switch format { case PrometheusMetricFormat: return m.PrometheusResponse() case "": return m.GenericResponse() default: - return nil, fmt.Errorf("metric response format \"%s\" unknown", format) + return &logical.Response{ + Data: map[string]interface{}{ + logical.HTTPContentType: ErrorContentType, + logical.HTTPRawBody: fmt.Sprintf("metric response format \"%s\" unknown", format), + logical.HTTPStatusCode: http.StatusBadRequest, + }, + } } } -func (m *MetricsHelper) PrometheusResponse() (*logical.Response, error) { +func (m *MetricsHelper) PrometheusResponse() *logical.Response { + resp := &logical.Response{ + Data: map[string]interface{}{ + logical.HTTPContentType: ErrorContentType, + logical.HTTPStatusCode: http.StatusBadRequest, + }, + } + if !m.PrometheusEnabled { - return &logical.Response{ - Data: map[string]interface{}{ - logical.HTTPContentType: "text/plain", - logical.HTTPRawBody: "prometheus is not enabled", - logical.HTTPStatusCode: 400, - }, - }, nil + resp.Data[logical.HTTPRawBody] = "prometheus is not enabled" + return resp } metricsFamilies, err := prometheus.DefaultGatherer.Gather() if err != nil && len(metricsFamilies) == 0 { - return nil, fmt.Errorf("no prometheus metrics could be decoded: %s", err) + resp.Data[logical.HTTPRawBody] = fmt.Sprintf("no prometheus metrics could be decoded: %s", err) + return resp } // Initialize a byte buffer. @@ -74,32 +87,36 @@ func (m *MetricsHelper) PrometheusResponse() (*logical.Response, error) { for _, mf := range metricsFamilies { err := e.Encode(mf) if err != nil { - return nil, fmt.Errorf("error during the encoding of metrics: %s", err) + resp.Data[logical.HTTPRawBody] = fmt.Sprintf("error during the encoding of metrics: %s", err) + return resp } } - return &logical.Response{ - Data: map[string]interface{}{ - logical.HTTPContentType: string(expfmt.FmtText), - logical.HTTPRawBody: buf.Bytes(), - logical.HTTPStatusCode: 200, - }, - }, nil + resp.Data[logical.HTTPContentType] = string(expfmt.FmtText) + resp.Data[logical.HTTPRawBody] = buf.Bytes() + resp.Data[logical.HTTPStatusCode] = http.StatusOK + return resp } -func (m *MetricsHelper) GenericResponse() (*logical.Response, error) { +func (m *MetricsHelper) GenericResponse() *logical.Response { + resp := &logical.Response{ + Data: map[string]interface{}{ + logical.HTTPContentType: ErrorContentType, + logical.HTTPStatusCode: http.StatusBadRequest, + }, + } + summary, err := m.inMemSink.DisplayMetrics(nil, nil) if err != nil { - return nil, fmt.Errorf("error while fetching the in-memory metrics: %s", err) + resp.Data[logical.HTTPRawBody] = fmt.Sprintf("error while fetching the in-memory metrics: %s", err) + return resp } content, err := json.Marshal(summary) if err != nil { - return nil, fmt.Errorf("error while marshalling the in-memory metrics: %s", err) + resp.Data[logical.HTTPRawBody] = fmt.Sprintf("error while marshalling the in-memory metrics: %s", err) + return resp } - return &logical.Response{ - Data: map[string]interface{}{ - logical.HTTPContentType: "application/json", - logical.HTTPRawBody: content, - logical.HTTPStatusCode: 200, - }, - }, nil + resp.Data[logical.HTTPContentType] = "application/json" + resp.Data[logical.HTTPRawBody] = content + resp.Data[logical.HTTPStatusCode] = http.StatusOK + return resp } diff --git a/http/sys_metrics.go b/http/sys_metrics.go index 2cfce5eda2..ee55383f98 100644 --- a/http/sys_metrics.go +++ b/http/sys_metrics.go @@ -1,6 +1,7 @@ package http import ( + "fmt" "net/http" "github.com/hashicorp/vault/helper/metricsutil" @@ -11,22 +12,31 @@ import ( func handleMetricsUnauthenticated(core *vault.Core) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { req := &logical.Request{Headers: r.Header} + + // Parse form + if err := r.ParseForm(); err != nil { + respondError(w, http.StatusBadRequest, err) + return + } + format := r.Form.Get("format") if format == "" { format = metricsutil.FormatFromRequest(req) } // Define response - resp, err := core.MetricsHelper().ResponseForFormat(format) - if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) - return - } + resp := core.MetricsHelper().ResponseForFormat(format) // Manually extract the logical response and send back the information w.WriteHeader(resp.Data[logical.HTTPStatusCode].(int)) w.Header().Set("Content-Type", resp.Data[logical.HTTPContentType].(string)) - w.Write(resp.Data[logical.HTTPRawBody].([]byte)) + switch v := resp.Data[logical.HTTPRawBody].(type) { + case string: + w.Write([]byte(v)) + case []byte: + w.Write(v) + default: + respondError(w, http.StatusInternalServerError, fmt.Errorf("wrong response returned")) + } }) } diff --git a/http/sys_metrics_test.go b/http/sys_metrics_test.go index fda7430aab..5f93c55240 100644 --- a/http/sys_metrics_test.go +++ b/http/sys_metrics_test.go @@ -14,7 +14,7 @@ func TestSysMetricsUnauthenticated(t *testing.T) { metrics.DefaultInmemSignal(inm) conf := &vault.CoreConfig{ BuiltinRegistry: vault.NewMockBuiltinRegistry(), - MetricsHelper: metricsutil.NewMetricsHelper(inm, false), + MetricsHelper: metricsutil.NewMetricsHelper(inm, true), } core, _, token := vault.TestCoreUnsealedWithConfig(t, conf) ln, addr := TestServer(t, core) @@ -47,4 +47,8 @@ func TestSysMetricsUnauthenticated(t *testing.T) { // Should also work with token resp = testHttpGet(t, token, addr+"/v1/sys/metrics") testResponseStatus(t, resp, 200) + + // Test if prometheus response is correct + resp = testHttpGet(t, "", addr+"/v1/sys/metrics?format=prometheus") + testResponseStatus(t, resp, 200) } diff --git a/vault/logical_system.go b/vault/logical_system.go index 8fb0787df8..ed674c5e39 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -2610,7 +2610,7 @@ func (b *SystemBackend) handleMetrics(ctx context.Context, req *logical.Request, if format == "" { format = metricsutil.FormatFromRequest(req) } - return b.Core.metricsHelper.ResponseForFormat(format) + return b.Core.metricsHelper.ResponseForFormat(format), nil } // handleHostInfo collects and returns host-related information, which includes