dewrich closed pull request #2420: Fix TO Go logging unauthenticated endpoints
URL: https://github.com/apache/trafficcontrol/pull/2420
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/traffic_ops/traffic_ops_golang/api/api.go
b/traffic_ops/traffic_ops_golang/api/api.go
index 59b38da7f..b102dfa26 100644
--- a/traffic_ops/traffic_ops_golang/api/api.go
+++ b/traffic_ops/traffic_ops_golang/api/api.go
@@ -41,6 +41,7 @@ import (
const DBContextKey = "db"
const ConfigContextKey = "context"
+const ReqIDContextKey = "reqid"
// WriteResp takes any object, serializes it as JSON, and writes that to w.
Any errors are logged and written to w via tc.GetHandleErrorsFunc.
// This is a helper for the common case; not using this in unusual cases is
perfectly acceptable.
@@ -216,6 +217,7 @@ type APIInfo struct {
Params map[string]string
IntParams map[string]int
User *auth.CurrentUser
+ ReqID uint64
Tx *sqlx.Tx
CommitTx *bool
Config *config.Config
@@ -247,6 +249,11 @@ func NewInfo(r *http.Request, requiredParams []string,
intParamNames []string) (
if err != nil {
return nil, errors.New("getting config: " + err.Error()), nil,
http.StatusInternalServerError
}
+ reqID, err := getReqID(r.Context())
+ if err != nil {
+ return nil, errors.New("getting reqID: " + err.Error()), nil,
http.StatusInternalServerError
+ }
+
user, err := auth.GetCurrentUser(r.Context())
if err != nil {
return nil, errors.New("getting user: " + err.Error()), nil,
http.StatusInternalServerError
@@ -262,6 +269,7 @@ func NewInfo(r *http.Request, requiredParams []string,
intParamNames []string) (
falsePtr := false
return &APIInfo{
Config: cfg,
+ ReqID: reqID,
Params: params,
IntParams: intParams,
User: user,
@@ -302,3 +310,16 @@ func getConfig(ctx context.Context) (*config.Config,
error) {
}
return nil, errors.New("No config found in Context")
}
+
+func getReqID(ctx context.Context) (uint64, error) {
+ val := ctx.Value(ReqIDContextKey)
+ if val != nil {
+ switch v := val.(type) {
+ case uint64:
+ return v, nil
+ default:
+ return 0, fmt.Errorf("ReqID found with bad type: %T", v)
+ }
+ }
+ return 0, errors.New("No ReqID found in Context")
+}
diff --git a/traffic_ops/traffic_ops_golang/routing.go
b/traffic_ops/traffic_ops_golang/routing.go
index bd647e24e..a40f7029f 100644
--- a/traffic_ops/traffic_ops_golang/routing.go
+++ b/traffic_ops/traffic_ops_golang/routing.go
@@ -27,6 +27,8 @@ import (
"sort"
"strconv"
"strings"
+ "sync/atomic"
+ "time"
"github.com/apache/incubator-trafficcontrol/lib/go-log"
"github.com/apache/incubator-trafficcontrol/traffic_ops/traffic_ops_golang/api"
@@ -64,8 +66,8 @@ type RawRoute struct {
Middlewares []Middleware
}
-func getDefaultMiddleware() []Middleware {
- return []Middleware{wrapHeaders}
+func getDefaultMiddleware(secret string) []Middleware {
+ return []Middleware{getWrapAccessLog(secret), wrapHeaders}
}
// ServerData ...
@@ -129,7 +131,7 @@ func CreateRouteMap(rs []Route, rawRoutes []RawRoute,
authBase AuthBase) map[str
func getRouteMiddleware(middlewares []Middleware, authBase AuthBase,
authenticated bool, privLevel int) []Middleware {
if middlewares == nil {
- middlewares = getDefaultMiddleware()
+ middlewares = getDefaultMiddleware(authBase.secret)
}
if authenticated { // a privLevel of zero is an unauthenticated
endpoint.
authWrapper := authBase.GetWrapper(privLevel)
@@ -164,9 +166,17 @@ func CompileRoutes(routes map[string][]PathHandler)
map[string][]CompiledRoute {
}
// Handler - generic handler func used by the Handlers hooking into the routes
-func Handler(routes map[string][]CompiledRoute, catchall http.Handler, db
*sqlx.DB, cfg *config.Config, w http.ResponseWriter, r *http.Request) {
- requested := r.URL.Path[1:]
+func Handler(routes map[string][]CompiledRoute, catchall http.Handler, db
*sqlx.DB, cfg *config.Config, getReqID func() uint64, w http.ResponseWriter, r
*http.Request) {
+ reqID := getReqID()
+
+ reqIDStr := strconv.FormatUint(reqID, 10)
+ log.Infoln(r.Method + " " + r.URL.Path + " handling (reqid " + reqIDStr
+ ")")
+ start := time.Now()
+ defer func() {
+ log.Infoln(r.Method + " " + r.URL.Path + " handled (reqid " +
reqIDStr + ") in " + time.Since(start).String())
+ }()
+ requested := r.URL.Path[1:]
mRoutes, ok := routes[r.Method]
if !ok {
catchall.ServeHTTP(w, r)
@@ -187,6 +197,7 @@ func Handler(routes map[string][]CompiledRoute, catchall
http.Handler, db *sqlx.
ctx = context.WithValue(ctx, api.PathParamsKey, params)
ctx = context.WithValue(ctx, api.DBContextKey, db)
ctx = context.WithValue(ctx, api.ConfigContextKey, cfg)
+ ctx = context.WithValue(ctx, api.ReqIDContextKey, reqID)
r = r.WithContext(ctx)
compiledRoute.Handler(w, r)
return
@@ -209,8 +220,9 @@ func RegisterRoutes(d ServerData) error {
authBase := AuthBase{secret: d.Config.Secrets[0],
getCurrentUserInfoStmt: userInfoStmt, override: nil} //we know d.Config.Secrets
is a slice of at least one or start up would fail.
routes := CreateRouteMap(routeSlice, rawRoutes, authBase)
compiledRoutes := CompileRoutes(routes)
+ getReqID := nextReqIDGetter()
http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
- Handler(compiledRoutes, catchall, d.DB, &d.Config, w, r)
+ Handler(compiledRoutes, catchall, d.DB, &d.Config, getReqID, w,
r)
})
return nil
}
@@ -225,3 +237,11 @@ func use(h http.HandlerFunc, middlewares []Middleware)
http.HandlerFunc {
}
return h
}
+
+// nextReqIDGetter returns a function for getting incrementing identifiers.
The returned func is safe for calling with multiple goroutines. Note the
returned identifiers will not be unique after the max uint64 value.
+func nextReqIDGetter() func() uint64 {
+ id := uint64(0)
+ return func() uint64 {
+ return atomic.AddUint64(&id, 1)
+ }
+}
diff --git a/traffic_ops/traffic_ops_golang/wrappers.go
b/traffic_ops/traffic_ops_golang/wrappers.go
index 93b92c7ca..6267958b2 100644
--- a/traffic_ops/traffic_ops_golang/wrappers.go
+++ b/traffic_ops/traffic_ops_golang/wrappers.go
@@ -59,14 +59,8 @@ func (a AuthBase) GetWrapper(privLevelRequired int)
Middleware {
return func(handlerFunc http.HandlerFunc) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
// TODO remove, and make username available to
wrapLogTime
- start := time.Now()
iw := &Interceptor{w: w}
w = iw
- username := "-"
- defer func() {
- log.EventfRaw(`%s - %s [%s] "%v %v HTTP/1.1" %v
%v %v "%v"`, r.RemoteAddr, username, time.Now().Format(AccessLogTimeFormat),
r.Method, r.URL.Path, iw.code, iw.byteCount,
int(time.Now().Sub(start)/time.Millisecond), r.UserAgent())
- }()
-
handleErr := func(status int, err error) {
errBytes, jsonErr :=
json.Marshal(tc.CreateErrorAlerts(err))
if jsonErr != nil {
@@ -99,7 +93,7 @@ func (a AuthBase) GetWrapper(privLevelRequired int)
Middleware {
return
}
- username = oldCookie.AuthData
+ username := oldCookie.AuthData
currentUserInfo :=
auth.GetCurrentUserFromDB(a.getCurrentUserInfoStmt, username)
if currentUserInfo.PrivLevel < privLevelRequired {
handleErr(http.StatusForbidden,
errors.New("Forbidden."))
@@ -138,6 +132,12 @@ func wrapHeaders(h http.HandlerFunc) http.HandlerFunc {
// AccessLogTimeFormat ...
const AccessLogTimeFormat = "02/Jan/2006:15:04:05 -0700"
+func getWrapAccessLog(secret string) Middleware {
+ return func(h http.HandlerFunc) http.HandlerFunc {
+ return wrapAccessLog(secret, h)
+ }
+}
+
func wrapAccessLog(secret string, h http.Handler) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
iw := &Interceptor{w: w}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
