Merge branch 'saturday' of sneak/gohttpserver into master

* move metrics endpoint protection middleware to correct file
* move /metrics route to Chi route group
* update readme

README.md

@@ -25,15 +25,18 @@ Alternately, even just feedback is great:
 * Stub Authentication middleware
 * Helper functions for encoding/decoding json
 * Healthcheck route
-* No global state
+* Prometheus metrics endpoint
+* No global state of our own
+    * some deps have some, such as the metrics collector and Sentry
 
 # Design Decisions
 
 * TLS is terminated somewhere else, like on a sidecar or reverse proxy.
 * logging: [rs/zerolog](https://github.com/rs/zerolog)
 * configuration: [spf13/viper](https://github.com/spf13/viper)
-* mux/router: [go-chi/chi](https://github.com/go-chi/chi)
-* prometheus-style metrics via [slok/go-http-metrics](https://github.com/slok/go-http-metrics)
+    * used as a wrapper around env vars
+* router is Chi: [go-chi/chi](https://github.com/go-chi/chi)
+* Prometheus-style metrics via [slok/go-http-metrics](https://github.com/slok/go-http-metrics)
 * database: TBD (thinking about [go-gorm/gorm](https://github.com/go-gorm/gorm))
 * templating: TBD (suggestions welcome)
 
@@ -44,6 +47,11 @@ Alternately, even just feedback is great:
 * sync.Once example for precompiling templates
 * Bundling Static Assets Into Binary
 
+# Known Bugs (more TODO)
+
+* Chi recovery middleware logs non-json when in non-tty stdout mode,
+  breaking validity of stdout as a json stream
+
 # Author
 
 * [sneak@sneak.berlin](mailto:sneak@sneak.berlin)

go.mod

@@ -3,6 +3,7 @@ module git.eeqj.de/sneak/gohttpserver
 go 1.15
 
 require (
+	github.com/99designs/basicauth-go v0.0.0-20160802081356-2a93ba0f464d
 	github.com/getsentry/sentry-go v0.7.0
 	github.com/go-chi/chi v4.1.2+incompatible
 	github.com/joho/godotenv v1.3.0

go.sum

@@ -12,6 +12,8 @@ cloud.google.com/go/pubsub v1.0.1/go.mod h1:R0Gpsv3s54REJCy4fxDixWD93lHJMoZTyQ2k
 cloud.google.com/go/storage v1.0.0/go.mod h1:IhtSnM/ZTZV8YYJWCY8RULGVqBDmpoyjwiyrjsg+URw=
 contrib.go.opencensus.io/exporter/prometheus v0.1.0/go.mod h1:cGFniUXGZlKRjzOyuZJ6mgB+PgBcCIa79kEKR8YCW+A=
 dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
+github.com/99designs/basicauth-go v0.0.0-20160802081356-2a93ba0f464d h1:j6oB/WPCigdOkxtuPl1VSIiLpy7Mdsu6phQffbF19Ng=
+github.com/99designs/basicauth-go v0.0.0-20160802081356-2a93ba0f464d/go.mod h1:3cARGAK9CfW3HoxCy1a0G4TKrdiKke8ftOMEOHyySYs=
 github.com/AndreasBriese/bbloom v0.0.0-20190306092124-e2d15f34fcf9/go.mod h1:bOvUY6CB00SOBii9/FifXqc0awNKxLFCL/+pkDPuyl8=
 github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=

httpserver/main.go

@@ -65,7 +65,6 @@ func Run(appname, version, buildarch string) int {
 	})
 
 	// this does nothing if SENTRY_DSN is unset in env.
-	s.enableSentry()
 
 	// TODO remove:
 	if s.sentryEnabled {
@@ -75,6 +74,9 @@ func Run(appname, version, buildarch string) int {
 	s.configure()
 	s.setupLogging()
 
+	// logging before sentry, because sentry logs
+	s.enableSentry()
+
 	s.databaseURL = viper.GetString("DBURL")
 	s.port = viper.GetInt("PORT")
 
@@ -82,19 +84,21 @@ func Run(appname, version, buildarch string) int {
 }
 
 func (s *server) enableSentry() {
-	sentryDSN := os.Getenv("SENTRY_DSN")
-	if sentryDSN == "" {
-		s.sentryEnabled = false
+	s.sentryEnabled = false
+
+	if viper.GetString("SENTRY_DSN") == "" {
 		return
 	}
 
 	err := sentry.Init(sentry.ClientOptions{
-		Dsn:     sentryDSN,
+		Dsn:     viper.GetString("SENTRY_DSN"),
 		Release: fmt.Sprintf("%s-%s", s.appname, s.version),
 	})
 	if err != nil {
 		log.Fatal().Err(err).Msg("sentry init failure")
+		return
 	}
+	s.log.Info().Msg("sentry error reporting activated")
 	s.sentryEnabled = true
 }
 
@@ -175,6 +179,9 @@ func (s *server) configure() {
 	viper.SetDefault("MAINTENANCE_MODE", "false")
 	viper.SetDefault("PORT", "8080")
 	viper.SetDefault("DBURL", "")
+	viper.SetDefault("SENTRY_DSN", "")
+	viper.SetDefault("METRICS_USERNAME", "")
+	viper.SetDefault("METRICS_PASSWORD", "")
 
 	if err := viper.ReadInConfig(); err != nil {
 		if _, ok := err.(viper.ConfigFileNotFoundError); ok {

httpserver/middlewares.go

@@ -5,10 +5,13 @@ import (
 	"net/http"
 	"time"
 
+	basicauth "github.com/99designs/basicauth-go"
 	"github.com/go-chi/chi/middleware"
 	metrics "github.com/slok/go-http-metrics/metrics/prometheus"
 	ghmm "github.com/slok/go-http-metrics/middleware"
+
 	"github.com/slok/go-http-metrics/middleware/std"
+	"github.com/spf13/viper"
 )
 
 // the following is from
@@ -90,3 +93,15 @@ func (s *server) MetricsMiddleware() func(http.Handler) http.Handler {
 		return std.Handler("", mdlw, next)
 	}
 }
+
+func (s *server) MetricsAuthMiddleware() func(http.Handler) http.Handler {
+	return basicauth.New(
+		"metrics",
+		map[string][]string{
+			viper.GetString("METRICS_USERNAME"): {
+				viper.GetString("METRICS_PASSWORD"),
+			},
+		},
+	)
+
+}

httpserver/routes.go

@@ -8,6 +8,7 @@ import (
 	"github.com/go-chi/chi"
 	"github.com/go-chi/chi/middleware"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
+	"github.com/spf13/viper"
 )
 
 func (s *server) routes() {
@@ -20,32 +21,31 @@ func (s *server) routes() {
 	// can .Use() more than one) will be applied to every request into
 	// the service.
 
+	s.router.Use(middleware.Recoverer)
 	s.router.Use(middleware.RequestID)
 	s.router.Use(s.LoggingMiddleware())
-	s.router.Use(s.MetricsMiddleware())
+
+	// add metrics middleware only if we can serve them behind auth
+	if viper.GetString("METRICS_USERNAME") != "" {
+		s.router.Use(s.MetricsMiddleware())
+	}
 
 	// CHANGEME to suit your needs, or pull from config.
-	// timeout for request context: your handlers must finish within
+	// timeout for request context; your handlers must finish within
 	// this window:
 	s.router.Use(middleware.Timeout(60 * time.Second))
 
 	// this adds a sentry reporting middleware if and only if sentry is
-	// enabled via setting of SENTRY_DSN in env.  this was at the
-	// bottom, but chi requires *all* middlewares applied before any
-	// routes are, so now it's up here.  unfortunately this cannot
-	// coexist with the normal chi Recoverer handler which prints a nice
-	// colorful stack trace to the console
+	// enabled via setting of SENTRY_DSN in env.
 	if s.sentryEnabled {
 		// Options docs at
 		// https://docs.sentry.io/platforms/go/guides/http/
-		sentryHandler := sentryhttp.New(sentryhttp.Options{})
+		// we set sentry to repanic so that all panics bubble up to the
+		// Recoverer chi middleware above.
+		sentryHandler := sentryhttp.New(sentryhttp.Options{
+			Repanic: true,
+		})
 		s.router.Use(sentryHandler.Handle)
-		// FYI: the sentry panic-catcher seems to set the response
-		// code to 200.
-	} else {
-		// FYI: the chi Recoverer middleware sets the response code
-		// on panics to 500.
-		s.router.Use(middleware.Recoverer)
 	}
 
 	////////////////////////////////////////////////////////////////////////
@@ -58,34 +58,30 @@ func (s *server) routes() {
 	// if you want to use a general purpose middleware (http.Handler
 	// wrapper) on a specific HandleFunc route, you need to take the
 	// .ServeHTTP of the http.Handler to get its HandleFunc, viz:
-
 	authMiddleware := s.AuthMiddleware()
 	s.router.Get(
 		"/login",
 		authMiddleware(s.handleLogin()).ServeHTTP,
 	)
 
+	// route that panics for testing
+	// CHANGEME remove this
+	s.router.Get(
+		"/panic",
+		s.handlePanic(),
+	)
+
 	s.router.Get(
 		"/.well-known/healthcheck.json",
 		s.handleHealthCheck(),
 	)
 
-	// route that panics for testing
-	// CHANGEME remove this
+	// set up authenticated /metrics route:
+	if viper.GetString("METRICS_USERNAME") != "" {
+		s.router.Group(func(r chi.Router) {
+			r.Use(s.MetricsAuthMiddleware())
+			r.Get("/metrics", http.HandlerFunc(promhttp.Handler().ServeHTTP))
+		})
+	}
 
-	s.router.Get(
-		"/panic",
-		s.handlePanic(),
-	)
-
-	// CHANGEME you probably want to wrap the following in some kind of
-	// auth like http basic auth which is easy to set up on your
-	// rometheus collector
-	// TODO(sneak): read http basic auth user/pass for /metrics
-	// out of environment vars
-
-	s.router.Get(
-		"/metrics",
-		http.HandlerFunc(promhttp.Handler().ServeHTTP),
-	)
 }