Skip to content

K
k3s
Commit 3149a2b7 authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub
Browse files
Options

Merge pull request #33286 from PhilibertDugas/fix/32747_stacktrace-log-from-apiserver

Automatic merge from submit-queue Apiserver don't log stacktrace when proxying <!-- Thanks for sending a pull request! Here are some tips for you: 1. If this is your first time, read our contributor guidelines https://github.com/kubernetes/kubernetes/blob/master/CONTRIBUTING.md and developer guide https://github.com/kubernetes/kubernetes/blob/master/docs/devel/development.md 2. If you want *faster* PR reviews, read how: https://github.com/kubernetes/kubernetes/blob/master/docs/devel/faster_reviews.md 3. Follow the instructions for writing a release note: https://github.com/kubernetes/kubernetes/blob/master/docs/devel/pull-requests.md#release-notes --> **What this PR does / why we need it**: When we are proxying unexpected status from a service or a pod, we print the stack traces (which is not the wanted behaviour). This is an attempt at fixing the issue #32747, With the `RequestInfoResolver` struct, it's possible to inspect the request and get the `Verb`. In this case, the `proxy` value is what I was looking for to avoid logging stack traces. I'm wrapping the `.Log()` call with an `if` statement to remove all stack traces logging when the call is a proxy from a service or a pod Another approach would have been to add another kind of `StacktracePred` in the `httplog` package. I found this path to be trickier to code as it's currently only accepting int values. **Which issue this PR fixes** : fixes #32747 **Special notes for your reviewer**: N/A **Release note**: <!-- Steps to write your release note: 1. Use the release-note-* labels to set the release note state (if you have access) 2. Enter your extended release note in the below block; leaving it blank means using the PR title as the release note. If no release note is required, just write `NONE`. --> ```release-note ```
parents 7ee656c4 b557acf9
Show whitespace changes
Inline Side-by-side
Showing with 11 additions and 6 deletions
pkg/apiserver/handlers.go
View file @ 3149a2b7
...@@ -134,13 +134,17 @@ func tooManyRequests(req *http.Request, w http.ResponseWriter) { ...@@ -134,13 +134,17 @@ func tooManyRequests(req *http.Request, w http.ResponseWriter) {
} }
// RecoverPanics wraps an http Handler to recover and log panics. // RecoverPanics wraps an http Handler to recover and log panics.
func RecoverPanics(handler http.Handler) http.Handler { func RecoverPanics(handler http.Handler, resolver *RequestInfoResolver) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
defer runtime.HandleCrash(func(err interface{}) { defer runtime.HandleCrash(func(err interface{}) {
http.Error(w, "This request caused apisever to panic. Look in log for details.", http.StatusInternalServerError) http.Error(w, "This request caused apisever to panic. Look in log for details.", http.StatusInternalServerError)
glog.Errorf("APIServer panic'd on %v %v: %v\n%s\n", req.Method, req.RequestURI, err, debug.Stack()) glog.Errorf("APIServer panic'd on %v %v: %v\n%s\n", req.Method, req.RequestURI, err, debug.Stack())
}) })
defer httplog.NewLogged(req, &w).StacktraceWhen(
logger := httplog.NewLogged(req, &w)
requestInfo, err := resolver.GetRequestInfo(req)
if err != nil || requestInfo.Verb != "proxy" {
logger.StacktraceWhen(
httplog.StatusIsNot( httplog.StatusIsNot(
http.StatusOK, http.StatusOK,
http.StatusCreated, http.StatusCreated,
...@@ -156,8 +160,9 @@ func RecoverPanics(handler http.Handler) http.Handler { ...@@ -156,8 +160,9 @@ func RecoverPanics(handler http.Handler) http.Handler {
errors.StatusUnprocessableEntity, errors.StatusUnprocessableEntity,
http.StatusSwitchingProtocols, http.StatusSwitchingProtocols,
), ),
).Log() )
}
defer logger.Log()
// Dispatch to the internal handler // Dispatch to the internal handler
handler.ServeHTTP(w, req) handler.ServeHTTP(w, req)
}) })
......
pkg/genericapiserver/genericapiserver.go
View file @ 3149a2b7
...@@ -288,7 +288,7 @@ func (s *GenericAPIServer) Run(options *options.ServerRunOptions) { ...@@ -288,7 +288,7 @@ func (s *GenericAPIServer) Run(options *options.ServerRunOptions) {
secureStartedCh := make(chan struct{}) secureStartedCh := make(chan struct{})
if secureLocation != "" { if secureLocation != "" {
handler := apiserver.TimeoutHandler(apiserver.RecoverPanics(s.Handler), longRunningTimeout) handler := apiserver.TimeoutHandler(apiserver.RecoverPanics(s.Handler, s.NewRequestInfoResolver()), longRunningTimeout)
secureServer := &http.Server{ secureServer := &http.Server{
Addr: secureLocation, Addr: secureLocation,
Handler: apiserver.MaxInFlightLimit(sem, longRunningRequestCheck, handler), Handler: apiserver.MaxInFlightLimit(sem, longRunningRequestCheck, handler),
...@@ -358,7 +358,7 @@ func (s *GenericAPIServer) Run(options *options.ServerRunOptions) { ...@@ -358,7 +358,7 @@ func (s *GenericAPIServer) Run(options *options.ServerRunOptions) {
close(secureStartedCh) close(secureStartedCh)
} }
handler := apiserver.TimeoutHandler(apiserver.RecoverPanics(s.InsecureHandler), longRunningTimeout) handler := apiserver.TimeoutHandler(apiserver.RecoverPanics(s.InsecureHandler, s.NewRequestInfoResolver()), longRunningTimeout)
http := &http.Server{ http := &http.Server{
Addr: insecureLocation, Addr: insecureLocation,
Handler: handler, Handler: handler,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment