From 8df9f581f55f4b66aed3466aa32135a32b4ee4d6 Mon Sep 17 00:00:00 2001 From: James Rasell Date: Tue, 9 Mar 2021 14:02:16 +0100 Subject: [PATCH 1/2] agent: return req error if prometheus metrics are disabled. --- agent/http/agent_test.go | 2 +- agent/http/health_test.go | 2 +- agent/http/metrics.go | 6 ++++++ agent/http/metrics_test.go | 19 ++++++++++++++----- agent/http/server.go | 13 +++++++++---- agent/http/testing.go | 4 ++-- command/agent.go | 3 ++- 7 files changed, 35 insertions(+), 14 deletions(-) diff --git a/agent/http/agent_test.go b/agent/http/agent_test.go index a553a092..2ab29271 100644 --- a/agent/http/agent_test.go +++ b/agent/http/agent_test.go @@ -26,7 +26,7 @@ func TestServer_agentReload(t *testing.T) { }, } - srv, stopSrv := TestServer(t) + srv, stopSrv := TestServer(t, false) defer stopSrv() for _, tc := range testCases { diff --git a/agent/http/health_test.go b/agent/http/health_test.go index 0f80537e..8fc3cc89 100644 --- a/agent/http/health_test.go +++ b/agent/http/health_test.go @@ -41,7 +41,7 @@ func TestServer_getHealth(t *testing.T) { } // Create our HTTP server. - srv, stopSrv := TestServer(t) + srv, stopSrv := TestServer(t, false) defer stopSrv() for _, tc := range testCases { diff --git a/agent/http/metrics.go b/agent/http/metrics.go index a05d6252..cfc4221e 100644 --- a/agent/http/metrics.go +++ b/agent/http/metrics.go @@ -25,6 +25,12 @@ func (s *Server) getMetrics(w http.ResponseWriter, r *http.Request) (interface{} } if format := r.URL.Query().Get("format"); format == "prometheus" { + + // Only return Prometheus formatted metrics if the user has enabled + // this functionality. + if !s.promEnabled { + return nil, newCodedError(http.StatusUnsupportedMediaType, "Prometheus is not enabled") + } s.getPrometheusMetrics().ServeHTTP(w, r) return nil, nil } diff --git a/agent/http/metrics_test.go b/agent/http/metrics_test.go index e65587a8..3cce40bd 100644 --- a/agent/http/metrics_test.go +++ b/agent/http/metrics_test.go @@ -14,6 +14,7 @@ func TestServer_getMetrics(t *testing.T) { inputWriter *httptest.ResponseRecorder expectedRespCode int expectedRespContains string + enableProm bool name string }{ { @@ -21,6 +22,7 @@ func TestServer_getMetrics(t *testing.T) { inputWriter: httptest.NewRecorder(), expectedRespCode: 405, expectedRespContains: "Invalid method", + enableProm: false, name: "incorrect request method", }, { @@ -28,27 +30,34 @@ func TestServer_getMetrics(t *testing.T) { inputWriter: httptest.NewRecorder(), expectedRespCode: 200, expectedRespContains: "Counters\":[],\"Gauges\":[],\"Points\":[],\"Samples\":[]", + enableProm: false, name: "correct request for JSON metrics", }, - { inputReq: httptest.NewRequest("GET", "/v1/metrics?format=prometheus", nil), inputWriter: httptest.NewRecorder(), expectedRespCode: 200, expectedRespContains: "# TYPE go_goroutines gauge", + enableProm: true, name: "correct request for Prometheus formatted metrics", }, + { + inputReq: httptest.NewRequest("GET", "/v1/metrics?format=prometheus", nil), + inputWriter: httptest.NewRecorder(), + expectedRespCode: 415, + expectedRespContains: "", + enableProm: false, + name: "Prometheus format metrics disabled", + }, } - // Create our HTTP server. - srv, stopSrv := TestServer(t) - defer stopSrv() - for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + srv, stopSrv := TestServer(t, tc.enableProm) srv.mux.ServeHTTP(tc.inputWriter, tc.inputReq) assert.Equal(t, tc.expectedRespCode, tc.inputWriter.Code, tc.name) assert.Contains(t, tc.inputWriter.Body.String(), tc.expectedRespContains, tc.name) + stopSrv() }) } } diff --git a/agent/http/server.go b/agent/http/server.go index 1cd018cd..93bd13c6 100644 --- a/agent/http/server.go +++ b/agent/http/server.go @@ -51,6 +51,10 @@ type Server struct { mux *http.ServeMux srv *http.Server + // promEnabled tracks whether Prometheus formatted metrics should be + // enabled. + promEnabled bool + // aliveness is used to describe the health response and should be set // atomically using healthAlivenessReady and healthAlivenessUnavailable // const declarations. @@ -62,12 +66,13 @@ type Server struct { } // NewHTTPServer creates a new agent HTTP server. -func NewHTTPServer(debug bool, cfg *config.HTTP, log hclog.Logger, agent AgentHTTP) (*Server, error) { +func NewHTTPServer(debug, prom bool, cfg *config.HTTP, log hclog.Logger, agent AgentHTTP) (*Server, error) { srv := &Server{ - log: log.Named("http_server"), - mux: http.NewServeMux(), - agent: agent, + log: log.Named("http_server"), + mux: http.NewServeMux(), + agent: agent, + promEnabled: prom, } // Setup our handlers. diff --git a/agent/http/testing.go b/agent/http/testing.go index 7d3ac818..ccfdf99e 100644 --- a/agent/http/testing.go +++ b/agent/http/testing.go @@ -8,13 +8,13 @@ import ( "github.com/hashicorp/nomad-autoscaler/agent/config" ) -func TestServer(t *testing.T) (*Server, func()) { +func TestServer(t *testing.T, enableProm bool) (*Server, func()) { cfg := &config.HTTP{ BindAddress: "127.0.0.1", BindPort: 0, // Use next available port. } - s, err := NewHTTPServer(false, cfg, hclog.NewNullLogger(), &agent.MockAgentHTTP{}) + s, err := NewHTTPServer(false, enableProm, cfg, hclog.NewNullLogger(), &agent.MockAgentHTTP{}) if err != nil { t.Fatalf("failed to start test server: %v", err) } diff --git a/command/agent.go b/command/agent.go index 29dcbcb0..e5fe925b 100644 --- a/command/agent.go +++ b/command/agent.go @@ -267,7 +267,8 @@ func (c *AgentCommand) Run(args []string) int { // create and run agent and HTTP server c.agent = agent.NewAgent(parsedConfig, configPaths, logger) - httpServer, err := agentHTTP.NewHTTPServer(parsedConfig.EnableDebug, parsedConfig.HTTP, logger, c.agent) + httpServer, err := agentHTTP.NewHTTPServer( + parsedConfig.Telemetry.PrometheusMetrics, parsedConfig.EnableDebug, parsedConfig.HTTP, logger, c.agent) if err != nil { logger.Error("failed to setup HTTP getHealth server", "error", err) return 1 From 32f00d58b16136360744f6c422ccf96d2914e29e Mon Sep 17 00:00:00 2001 From: James Rasell Date: Wed, 10 Mar 2021 09:18:14 +0100 Subject: [PATCH 2/2] changelog: add entry for #416 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bb1a92e..5ba8560f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## UNRELEASED BUG FIXES: + * agent: Only allow querying Prometheus formatted metrics if Prometheus is enabled within the config [[GH-416](https://github.com/hashicorp/nomad-autoscaler/pull/416)] * policy: Ensure metric emitters use the correct context and are stopped when appropriate [[GH-408](https://github.com/hashicorp/nomad-autoscaler/pull/408)] ## 0.3.0 (February 25, 2021)