feat(api): refactore router

- Simplify router
- Single routes onfiguration file
- Improve HTTP logger

pkg/api/router.go

@@ -3,58 +3,19 @@ package api
 import (
 	"net/http"
 
-	"github.com/ncarlier/webhookd/pkg/auth"
 	"github.com/ncarlier/webhookd/pkg/config"
-	"github.com/ncarlier/webhookd/pkg/logger"
-	"github.com/ncarlier/webhookd/pkg/middleware"
-	"github.com/ncarlier/webhookd/pkg/pubkey"
 )
 
-var commonMiddlewares = []middleware.Middleware{
-	middleware.Cors,
-	middleware.Logger,
-	middleware.Tracing(nextRequestID),
-}
-
 // NewRouter creates router with declared routes
 func NewRouter(conf *config.Config) *http.ServeMux {
 	router := http.NewServeMux()
 
-	var middlewares = commonMiddlewares
-	if conf.TLS {
-		middlewares = append(middlewares, middleware.HSTS)
-	}
-
-	// Load trust store...
-	trustStore, err := pubkey.NewTrustStore(conf.TrustStoreFile)
-	if err != nil {
-		logger.Warning.Printf("unable to load trust store (\"%s\"): %s\n", conf.TrustStoreFile, err)
-	}
-	if trustStore != nil {
-		middlewares = append(middlewares, middleware.HTTPSignature(trustStore))
-	}
-
-	// Load authenticator...
-	authenticator, err := auth.NewHtpasswdFromFile(conf.PasswdFile)
-	if err != nil {
-		logger.Debug.Printf("unable to load htpasswd file (\"%s\"): %s\n", conf.PasswdFile, err)
-	}
-	if authenticator != nil {
-		middlewares = append(middlewares, middleware.AuthN(authenticator))
-	}
-
 	// Register HTTP routes...
-	for _, route := range routes {
+	for _, route := range routes(conf) {
 		handler := route.HandlerFunc(conf)
 		for _, mw := range route.Middlewares {
 			handler = mw(handler)
 		}
-		for _, mw := range middlewares {
-			if route.Path == "/healthz" {
-				continue
-			}
-			handler = mw(handler)
-		}
 		router.Handle(route.Path, handler)
 	}
 

pkg/api/routes.go

@@ -1,36 +1,67 @@
 package api
 
 import (
-	"net/http"
-
+	"github.com/ncarlier/webhookd/pkg/auth"
 	"github.com/ncarlier/webhookd/pkg/config"
+	"github.com/ncarlier/webhookd/pkg/logger"
 	"github.com/ncarlier/webhookd/pkg/middleware"
+	"github.com/ncarlier/webhookd/pkg/pubkey"
 )
 
-// HandlerFunc custom function handler
-type HandlerFunc func(conf *config.Config) http.Handler
-
-// Route is the structure of an HTTP route definition
-type Route struct {
-	Path        string
-	HandlerFunc HandlerFunc
-	Middlewares []middleware.Middleware
+var commonMiddlewares = middleware.Middlewares{
+	middleware.Cors,
+	middleware.Logger,
+	middleware.Tracing(nextRequestID),
 }
 
-func route(path string, handler HandlerFunc, middlewares ...middleware.Middleware) Route {
-	return Route{
-		Path:        path,
-		HandlerFunc: handler,
-		Middlewares: middlewares,
+func buildMiddlewares(conf *config.Config) middleware.Middlewares {
+	var middlewares = commonMiddlewares
+	if conf.TLS {
+		middlewares = middlewares.UseAfter(middleware.HSTS)
+	}
+
+	// Load trust store...
+	trustStore, err := pubkey.NewTrustStore(conf.TrustStoreFile)
+	if err != nil {
+		logger.Warning.Printf("unable to load trust store (\"%s\"): %s\n", conf.TrustStoreFile, err)
+	}
+	if trustStore != nil {
+		middlewares = middlewares.UseAfter(middleware.HTTPSignature(trustStore))
 	}
-}
 
-// Routes is a list of Route
-type Routes []Route
+	// Load authenticator...
+	authenticator, err := auth.NewHtpasswdFromFile(conf.PasswdFile)
+	if err != nil {
+		logger.Debug.Printf("unable to load htpasswd file (\"%s\"): %s\n", conf.PasswdFile, err)
+	}
+	if authenticator != nil {
+		middlewares = middlewares.UseAfter(middleware.AuthN(authenticator))
+	}
+	return middlewares
+}
 
-var routes = Routes{
-	route("/", index, middleware.Methods("GET", "POST")),
-	route("/static/", static("/static/"), middleware.Methods("GET")),
-	route("/healthz", healthz, middleware.Methods("GET")),
-	route("/varz", varz, middleware.Methods("GET")),
+func routes(conf *config.Config) Routes {
+	middlewares := buildMiddlewares(conf)
+	return Routes{
+		route(
+			"/",
+			index,
+			middlewares.UseBefore(middleware.Methods("GET", "POST"))...,
+		),
+		route(
+			"/static/",
+			static("/static/"),
+			middlewares.UseBefore(middleware.Methods("GET"))...,
+		),
+		route(
+			"/healthz",
+			healthz,
+			commonMiddlewares.UseBefore(middleware.Methods("GET"))...,
+		),
+		route(
+			"/varz",
+			varz,
+			middlewares.UseBefore(middleware.Methods("GET"))...,
+		),
+	}
 }

pkg/api/types.go

@@ -0,0 +1,29 @@
+package api
+
+import (
+	"net/http"
+
+	"github.com/ncarlier/webhookd/pkg/config"
+	"github.com/ncarlier/webhookd/pkg/middleware"
+)
+
+// HandlerFunc custom function handler
+type HandlerFunc func(conf *config.Config) http.Handler
+
+// Route is the structure of an HTTP route definition
+type Route struct {
+	Path        string
+	HandlerFunc HandlerFunc
+	Middlewares middleware.Middlewares
+}
+
+// Routes is a list of Route
+type Routes []Route
+
+func route(path string, handler HandlerFunc, middlewares ...middleware.Middleware) Route {
+	return Route{
+		Path:        path,
+		HandlerFunc: handler,
+		Middlewares: middlewares,
+	}
+}

pkg/middleware/cors.go

@@ -14,6 +14,5 @@ func Cors(inner http.Handler) http.Handler {
 		if r.Method != "OPTIONS" {
 			inner.ServeHTTP(w, r)
 		}
-		return
 	})
 }

pkg/middleware/hsts.go

@@ -9,6 +9,5 @@ func HSTS(inner http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Strict-Transport-Security", "max-age=15768000 ; includeSubDomains")
 		inner.ServeHTTP(w, r)
-		return
 	})
 }

pkg/middleware/logger.go

@@ -1,7 +1,9 @@
 package middleware
 
 import (
+	"fmt"
 	"net/http"
+	"strings"
 	"time"
 
 	"github.com/ncarlier/webhookd/pkg/logger"
@@ -16,14 +18,61 @@ const (
 // Logger is a middleware to log HTTP request
 func Logger(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		o := &responseObserver{ResponseWriter: w}
 		start := time.Now()
 		defer func() {
 			requestID, ok := r.Context().Value(requestIDKey).(string)
 			if !ok {
-				requestID = "unknown"
+				requestID = "0"
 			}
-			logger.Info.Println(requestID, r.Method, r.URL.Path, r.RemoteAddr, r.UserAgent(), time.Since(start))
+			addr := r.RemoteAddr
+			if i := strings.LastIndex(addr, ":"); i != -1 {
+				addr = addr[:i]
+			}
+			logger.Info.Printf(
+				"%s - - [%s] %q %d %d %q %q %q",
+				addr,
+				start.Format("02/Jan/2006:15:04:05 -0700"),
+				fmt.Sprintf("%s %s %s", r.Method, r.URL, r.Proto),
+				o.status,
+				o.written,
+				r.Referer(),
+				r.UserAgent(),
+				fmt.Sprintf("REQID=%s", requestID),
+			)
 		}()
-		next.ServeHTTP(w, r)
+		next.ServeHTTP(o, r)
 	})
 }
+
+type responseObserver struct {
+	http.ResponseWriter
+	status      int
+	written     int64
+	wroteHeader bool
+}
+
+func (o *responseObserver) Write(p []byte) (n int, err error) {
+	if !o.wroteHeader {
+		o.WriteHeader(http.StatusOK)
+	}
+	n, err = o.ResponseWriter.Write(p)
+	o.written += int64(n)
+	return
+}
+
+func (o *responseObserver) WriteHeader(code int) {
+	o.ResponseWriter.WriteHeader(code)
+	if o.wroteHeader {
+		return
+	}
+	o.wroteHeader = true
+	o.status = code
+}
+
+func (o *responseObserver) Flush() {
+	flusher, ok := o.ResponseWriter.(http.Flusher)
+	if ok {
+		flusher.Flush()
+	}
+}

pkg/middleware/methods.go

@@ -19,7 +19,6 @@ func Methods(methods ...string) Middleware {
 			}
 			w.WriteHeader(405)
 			w.Write([]byte("405 Method Not Allowed\n"))
-			return
 		})
 	}
 }

pkg/middleware/types.go

@@ -4,3 +4,16 @@ import "net/http"
 
 // Middleware function definition
 type Middleware func(inner http.Handler) http.Handler
+
+// Middlewares list
+type Middlewares []Middleware
+
+// UseBefore insert a middleware at the begining of the middleware chain
+func (ms Middlewares) UseBefore(m Middleware) Middlewares {
+	return append([]Middleware{m}, ms...)
+}
+
+// UseAfter add a middleware at the end of the middleware chain
+func (ms Middlewares) UseAfter(m Middleware) Middlewares {
+	return append(ms, m)
+}