From e483fa505ca831c00db7c08b2e949b0a67f4e8ec Mon Sep 17 00:00:00 2001 From: Jan Berktold Date: Wed, 24 Apr 2019 23:53:01 +0200 Subject: [PATCH 1/3] Add DRONE_PROMETHEUS_ANONYMOUS_ACCESS configuration option --- CHANGELOG.md | 3 ++- cmd/drone-server/config/config.go | 27 ++++++++++++++++----------- cmd/drone-server/wire_gen.go | 2 +- metric/handler.go | 9 ++++++--- 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72de277b..33839130 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - endpoint to trigger new build for default branch, by [@bradrydzewski](https://github.com/bradrydzewski). [#2679](https://github.com/drone/drone/issues/2679). - endpoint to trigger new build for branch, by [@bradrydzewski](https://github.com/bradrydzewski). [#2679](https://github.com/drone/drone/issues/2679). - endpoint to trigger new build for branch and sha, by [@bradrydzewski](https://github.com/bradrydzewski). [#2679](https://github.com/drone/drone/issues/2679). - +- DRONE_PROMETHEUS_ANONYMOUS_ACCESS configuration option, by [@janberktold](https://github.com/janberktold) +- ## [1.1.0] - 2019-04-23 ### Added diff --git a/cmd/drone-server/config/config.go b/cmd/drone-server/config/config.go index e77a61df..3cdd4bfc 100644 --- a/cmd/drone-server/config/config.go +++ b/cmd/drone-server/config/config.go @@ -46,17 +46,17 @@ type ( Config struct { License string `envconfig:"DRONE_LICENSE"` - Authn Authentication - Agent Agent - Cron Cron - Cloning Cloning - Database Database - Datadog Datadog - Docker Docker - HTTP HTTP - Jsonnet Jsonnet - Logging Logging - // Prometheus Prometheus + Authn Authentication + Agent Agent + Cron Cron + Cloning Cloning + Database Database + Datadog Datadog + Docker Docker + HTTP HTTP + Jsonnet Jsonnet + Logging Logging + Prometheus Prometheus Proxy Proxy Registration Registration Registries Registries @@ -162,6 +162,11 @@ type ( Text bool `envconfig:"DRONE_LOGS_TEXT"` } + // Prometheus provides the prometheus configuration. + Prometheus struct { + EnableAnonymousAccess bool `envconfig:"DRONE_PROMETHEUS_ANONYMOUS_ACCESS" default:"false"` + } + // Repository provides the repository configuration. Repository struct { Filter []string `envconfig:"DRONE_REPOSITORY_FILTER"` diff --git a/cmd/drone-server/wire_gen.go b/cmd/drone-server/wire_gen.go index 415edb57..d8be4735 100644 --- a/cmd/drone-server/wire_gen.go +++ b/cmd/drone-server/wire_gen.go @@ -93,7 +93,7 @@ func InitializeApplication(config2 config.Config) (application, error) { options := provideServerOptions(config2) webServer := web.New(admissionService, buildStore, client, hookParser, coreLicense, licenseService, middleware, repositoryStore, session, syncer, triggerer, userStore, userService, webhookSender, options, system) handler := provideRPC(buildManager, config2) - metricServer := metric.NewServer(session) + metricServer := metric.NewServer(session, config2) mux := provideRouter(server, webServer, handler, metricServer) serverServer := provideServer(mux, config2) mainApplication := newApplication(cronScheduler, datadog, runner, serverServer, userStore) diff --git a/metric/handler.go b/metric/handler.go index 66c5ef7f..19749f3c 100644 --- a/metric/handler.go +++ b/metric/handler.go @@ -10,6 +10,7 @@ import ( "errors" "net/http" + "github.com/drone/drone/cmd/drone-server/config" "github.com/drone/drone/core" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -26,13 +27,15 @@ var errAccessDenied = errors.New("Access denied") type Server struct { metrics http.Handler session core.Session + config config.Config } // NewServer returns a new metrics server. -func NewServer(session core.Session) *Server { +func NewServer(session core.Session, config config.Config) *Server { return &Server{ metrics: promhttp.Handler(), session: session, + config: config, } } @@ -41,9 +44,9 @@ func NewServer(session core.Session) *Server { func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { user, _ := s.session.Get(r) switch { - case user == nil: + case !s.config.Prometheus.EnableAnonymousAccess && user == nil: http.Error(w, errInvalidToken.Error(), 401) - case !user.Admin && !user.Machine: + case !s.config.Prometheus.EnableAnonymousAccess && !user.Admin && !user.Machine: http.Error(w, errAccessDenied.Error(), 403) default: s.metrics.ServeHTTP(w, r) From 058187f1508f9e3356c5912a45c5564dd0c188d6 Mon Sep 17 00:00:00 2001 From: Jan Berktold Date: Thu, 25 Apr 2019 00:22:55 +0200 Subject: [PATCH 2/3] Code review feedback --- cmd/drone-server/inject_server.go | 9 ++++++++- cmd/drone-server/wire_gen.go | 3 +-- metric/handler.go | 19 +++++++++---------- metric/handler_test.go | 23 ++++++++++++++++++++--- 4 files changed, 38 insertions(+), 16 deletions(-) diff --git a/cmd/drone-server/inject_server.go b/cmd/drone-server/inject_server.go index da2fbee7..3acd3942 100644 --- a/cmd/drone-server/inject_server.go +++ b/cmd/drone-server/inject_server.go @@ -18,6 +18,7 @@ import ( "net/http" "github.com/drone/drone/cmd/drone-server/config" + "github.com/drone/drone/core" "github.com/drone/drone/handler/api" "github.com/drone/drone/handler/web" "github.com/drone/drone/metric" @@ -33,9 +34,9 @@ import ( // wire set for loading the server. var serverSet = wire.NewSet( manager.New, - metric.NewServer, api.New, web.New, + provideMetric, provideRouter, provideRPC, provideServer, @@ -53,6 +54,12 @@ func provideRouter(api api.Server, web web.Server, rpc http.Handler, metrics *me return r } +// provideMetric is a Wire provider function that returns the +// metrics server exposing metrics in prometheus format. +func provideMetric(session core.Session, config config.Config) *metric.Server { + return metric.NewServer(session, config.Prometheus.EnableAnonymousAccess) +} + // provideRPC is a Wire provider function that returns an rpc // handler that exposes the build manager to a remote agent. func provideRPC(m manager.BuildManager, config config.Config) http.Handler { diff --git a/cmd/drone-server/wire_gen.go b/cmd/drone-server/wire_gen.go index d8be4735..8608a5f9 100644 --- a/cmd/drone-server/wire_gen.go +++ b/cmd/drone-server/wire_gen.go @@ -10,7 +10,6 @@ import ( "github.com/drone/drone/handler/api" "github.com/drone/drone/handler/web" "github.com/drone/drone/livelog" - "github.com/drone/drone/metric" "github.com/drone/drone/operator/manager" "github.com/drone/drone/pubsub" "github.com/drone/drone/service/commit" @@ -93,7 +92,7 @@ func InitializeApplication(config2 config.Config) (application, error) { options := provideServerOptions(config2) webServer := web.New(admissionService, buildStore, client, hookParser, coreLicense, licenseService, middleware, repositoryStore, session, syncer, triggerer, userStore, userService, webhookSender, options, system) handler := provideRPC(buildManager, config2) - metricServer := metric.NewServer(session, config2) + metricServer := provideMetrics(session, config2) mux := provideRouter(server, webServer, handler, metricServer) serverServer := provideServer(mux, config2) mainApplication := newApplication(cronScheduler, datadog, runner, serverServer, userStore) diff --git a/metric/handler.go b/metric/handler.go index 19749f3c..724e3b42 100644 --- a/metric/handler.go +++ b/metric/handler.go @@ -10,7 +10,6 @@ import ( "errors" "net/http" - "github.com/drone/drone/cmd/drone-server/config" "github.com/drone/drone/core" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -25,17 +24,17 @@ var errAccessDenied = errors.New("Access denied") // Server is an http Metrics server. type Server struct { - metrics http.Handler - session core.Session - config config.Config + metrics http.Handler + session core.Session + anonymous bool } // NewServer returns a new metrics server. -func NewServer(session core.Session, config config.Config) *Server { +func NewServer(session core.Session, anonymous bool) *Server { return &Server{ - metrics: promhttp.Handler(), - session: session, - config: config, + metrics: promhttp.Handler(), + session: session, + anonymous: anonymous, } } @@ -44,9 +43,9 @@ func NewServer(session core.Session, config config.Config) *Server { func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { user, _ := s.session.Get(r) switch { - case !s.config.Prometheus.EnableAnonymousAccess && user == nil: + case !s.anonymous && user == nil: http.Error(w, errInvalidToken.Error(), 401) - case !s.config.Prometheus.EnableAnonymousAccess && !user.Admin && !user.Machine: + case !s.anonymous && !user.Admin && !user.Machine: http.Error(w, errAccessDenied.Error(), 403) default: s.metrics.ServeHTTP(w, r) diff --git a/metric/handler_test.go b/metric/handler_test.go index 27892aaf..2931e135 100644 --- a/metric/handler_test.go +++ b/metric/handler_test.go @@ -26,7 +26,7 @@ func TestHandleMetrics(t *testing.T) { session := mock.NewMockSession(controller) session.EXPECT().Get(r).Return(mockUser, nil) - NewServer(session).ServeHTTP(w, r) + NewServer(session, false).ServeHTTP(w, r) if got, want := w.Code, 200; got != want { t.Errorf("Want status code %d, got %d", want, got) } @@ -46,13 +46,30 @@ func TestHandleMetrics_NoSession(t *testing.T) { session := mock.NewMockSession(controller) session.EXPECT().Get(r).Return(nil, nil) - NewServer(session).ServeHTTP(w, r) + NewServer(session, false).ServeHTTP(w, r) if got, want := w.Code, 401; got != want { t.Errorf("Want status code %d, got %d", want, got) } } +func TestHandleMetrics_NoSessionButAnonymousAccessEnabled(t *testing.T) { + controller := gomock.NewController(t) + defer controller.Finish() + + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/", nil) + + session := mock.NewMockSession(controller) + session.EXPECT().Get(r).Return(nil, nil) + + NewServer(session, true).ServeHTTP(w, r) + + if got, want := w.Code, 200; got != want { + t.Errorf("Want status code %d, got %d", want, got) + } +} + func TestHandleMetrics_AccessDenied(t *testing.T) { controller := gomock.NewController(t) defer controller.Finish() @@ -64,7 +81,7 @@ func TestHandleMetrics_AccessDenied(t *testing.T) { session := mock.NewMockSession(controller) session.EXPECT().Get(r).Return(mockUser, nil) - NewServer(session).ServeHTTP(w, r) + NewServer(session, false).ServeHTTP(w, r) if got, want := w.Code, 403; got != want { t.Errorf("Want status code %d, got %d", want, got) } From 78547a6a010f144046d2002ed638ae385f61c980 Mon Sep 17 00:00:00 2001 From: Jan Berktold Date: Thu, 25 Apr 2019 00:33:00 +0200 Subject: [PATCH 3/3] Fixing wire_gen.go --- cmd/drone-server/wire_gen.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/drone-server/wire_gen.go b/cmd/drone-server/wire_gen.go index 8608a5f9..5b473eb7 100644 --- a/cmd/drone-server/wire_gen.go +++ b/cmd/drone-server/wire_gen.go @@ -92,7 +92,7 @@ func InitializeApplication(config2 config.Config) (application, error) { options := provideServerOptions(config2) webServer := web.New(admissionService, buildStore, client, hookParser, coreLicense, licenseService, middleware, repositoryStore, session, syncer, triggerer, userStore, userService, webhookSender, options, system) handler := provideRPC(buildManager, config2) - metricServer := provideMetrics(session, config2) + metricServer := provideMetric(session, config2) mux := provideRouter(server, webServer, handler, metricServer) serverServer := provideServer(mux, config2) mainApplication := newApplication(cronScheduler, datadog, runner, serverServer, userStore)