From b48ac54e1c97a2e72a55aa2214efd9d6126bf0fe Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Mon, 26 Sep 2016 11:20:04 +0200 Subject: [PATCH 1/4] Make audit writer accessible from Config ... such that it can be used for a custom handler chain. --- pkg/genericapiserver/config.go | 33 +++++++++++------------- pkg/genericapiserver/genericapiserver.go | 4 --- 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/pkg/genericapiserver/config.go b/pkg/genericapiserver/config.go index 89aed26e6f2..d9645163f69 100644 --- a/pkg/genericapiserver/config.go +++ b/pkg/genericapiserver/config.go @@ -19,6 +19,7 @@ package genericapiserver import ( "crypto/tls" "fmt" + "io" "mime" "net" "net/http" @@ -53,10 +54,8 @@ import ( // Config is a structure used to configure a GenericAPIServer. type Config struct { - AuditLogPath string - AuditLogMaxAge int - AuditLogMaxBackups int - AuditLogMaxSize int + // Destination for audit logs + AuditWriter io.Writer // Allow downstream consumers to disable swagger. // This includes returning the generated swagger spec at /swaggerapi and swagger ui at /swagger-ui. EnableSwaggerSupport bool @@ -165,14 +164,21 @@ type Config struct { } func NewConfig(options *options.ServerRunOptions) *Config { + var auditWriter io.Writer + if len(options.AuditLogPath) != 0 { + auditWriter = &lumberjack.Logger{ + Filename: options.AuditLogPath, + MaxAge: options.AuditLogMaxAge, + MaxBackups: options.AuditLogMaxBackups, + MaxSize: options.AuditLogMaxSize, + } + } + return &Config{ APIGroupPrefix: options.APIGroupPrefix, APIPrefix: options.APIPrefix, CorsAllowedOriginList: options.CorsAllowedOriginList, - AuditLogPath: options.AuditLogPath, - AuditLogMaxAge: options.AuditLogMaxAge, - AuditLogMaxBackups: options.AuditLogMaxBackups, - AuditLogMaxSize: options.AuditLogMaxSize, + AuditWriter: auditWriter, EnableGarbageCollection: options.EnableGarbageCollection, EnableIndex: true, EnableProfiling: options.EnableProfiling, @@ -332,15 +338,6 @@ func (c Config) New() (*GenericAPIServer, error) { }) } - if len(c.AuditLogPath) != 0 { - s.auditWriter = &lumberjack.Logger{ - Filename: c.AuditLogPath, - MaxAge: c.AuditLogMaxAge, - MaxBackups: c.AuditLogMaxBackups, - MaxSize: c.AuditLogMaxSize, - } - } - // Send correct mime type for .svg files. // TODO: remove when https://github.com/golang/go/commit/21e47d831bafb59f22b1ea8098f709677ec8ce33 // makes it into all of our supported go versions (only in v1.7.1 now). @@ -371,7 +368,7 @@ func (s *GenericAPIServer) buildHandlerChains(c *Config, handler http.Handler) ( secure = handler secure = apiserverfilters.WithAuthorization(secure, attributeGetter, c.Authorizer) secure = apiserverfilters.WithImpersonation(secure, c.RequestContextMapper, c.Authorizer) - secure = apiserverfilters.WithAudit(secure, attributeGetter, s.auditWriter) // before impersonation to read original user + secure = apiserverfilters.WithAudit(secure, attributeGetter, c.AuditWriter) // before impersonation to read original user secure = authhandlers.WithAuthentication(secure, c.RequestContextMapper, c.Authenticator, authhandlers.Unauthorized(c.SupportsBasicAuth)) secure = genericfilters.WithPanicRecovery(secure, s.NewRequestInfoResolver()) secure = genericfilters.WithTimeoutForNonLongRunningRequests(secure, longRunningFunc) diff --git a/pkg/genericapiserver/genericapiserver.go b/pkg/genericapiserver/genericapiserver.go index f7432b41e9d..107b6abb0fc 100644 --- a/pkg/genericapiserver/genericapiserver.go +++ b/pkg/genericapiserver/genericapiserver.go @@ -19,7 +19,6 @@ package genericapiserver import ( "crypto/tls" "fmt" - "io" "net" "net/http" "path" @@ -166,9 +165,6 @@ type GenericAPIServer struct { postStartHooks map[string]PostStartHookFunc postStartHookLock sync.Mutex postStartHooksCalled bool - - // Writer to write the audit log to. - auditWriter io.Writer } // RequestContextMapper is exposed so that third party resource storage can be build in a different location. From e2c17d69f7f381b546ca23797c9409794ea4ace5 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Mon, 26 Sep 2016 11:21:18 +0200 Subject: [PATCH 2/4] Move one-time svg mime init code into Run() --- pkg/genericapiserver/config.go | 6 ------ pkg/genericapiserver/genericapiserver.go | 8 ++++++++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/genericapiserver/config.go b/pkg/genericapiserver/config.go index d9645163f69..793da58f4c2 100644 --- a/pkg/genericapiserver/config.go +++ b/pkg/genericapiserver/config.go @@ -20,7 +20,6 @@ import ( "crypto/tls" "fmt" "io" - "mime" "net" "net/http" "os" @@ -338,11 +337,6 @@ func (c Config) New() (*GenericAPIServer, error) { }) } - // Send correct mime type for .svg files. - // TODO: remove when https://github.com/golang/go/commit/21e47d831bafb59f22b1ea8098f709677ec8ce33 - // makes it into all of our supported go versions (only in v1.7.1 now). - mime.AddExtensionType(".svg", "image/svg+xml") - apiserver.InstallServiceErrorHandler(s.Serializer, s.HandlerContainer) s.installAPI(&c) diff --git a/pkg/genericapiserver/genericapiserver.go b/pkg/genericapiserver/genericapiserver.go index 107b6abb0fc..39e48ab6079 100644 --- a/pkg/genericapiserver/genericapiserver.go +++ b/pkg/genericapiserver/genericapiserver.go @@ -19,6 +19,7 @@ package genericapiserver import ( "crypto/tls" "fmt" + "mime" "net" "net/http" "path" @@ -167,6 +168,13 @@ type GenericAPIServer struct { postStartHooksCalled bool } +func init() { + // Send correct mime type for .svg files. + // TODO: remove when https://github.com/golang/go/commit/21e47d831bafb59f22b1ea8098f709677ec8ce33 + // makes it into all of our supported go versions (only in v1.7.1 now). + mime.AddExtensionType(".svg", "image/svg+xml") +} + // RequestContextMapper is exposed so that third party resource storage can be build in a different location. // TODO refactor third party resource storage func (s *GenericAPIServer) RequestContextMapper() api.RequestContextMapper { From b806116e40945b18b3aeb71b00965bf08255aa3b Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Tue, 27 Sep 2016 11:29:22 +0200 Subject: [PATCH 3/4] Move service error handler installer --- pkg/genericapiserver/config.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/genericapiserver/config.go b/pkg/genericapiserver/config.go index 793da58f4c2..b613d1824d5 100644 --- a/pkg/genericapiserver/config.go +++ b/pkg/genericapiserver/config.go @@ -329,6 +329,7 @@ func (c Config) New() (*GenericAPIServer, error) { // Use CurlyRouter to be able to use regular expressions in paths. Regular expressions are required in paths for example for proxy (where the path is proxy/{kind}/{name}/{*}) s.HandlerContainer.Router(restful.CurlyRouter{}) s.Mux = apiserver.NewPathRecorderMux(s.HandlerContainer.ServeMux) + apiserver.InstallServiceErrorHandler(s.Serializer, s.HandlerContainer) if c.ProxyDialer != nil || c.ProxyTLSClientConfig != nil { s.ProxyTransport = utilnet.SetTransportDefaults(&http.Transport{ @@ -337,8 +338,6 @@ func (c Config) New() (*GenericAPIServer, error) { }) } - apiserver.InstallServiceErrorHandler(s.Serializer, s.HandlerContainer) - s.installAPI(&c) s.Handler, s.InsecureHandler = s.buildHandlerChains(&c, http.Handler(s.Mux.BaseMux().(*http.ServeMux))) From 157dcda8cc45ceac33fea72aeb8ed0645b95ce93 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Mon, 26 Sep 2016 11:25:02 +0200 Subject: [PATCH 4/4] Move longrunning predicate into Config instead of RE ... in order to be available for custom handler chains. --- pkg/genericapiserver/config.go | 21 +++++++++++---------- pkg/genericapiserver/filters/timeout.go | 3 +++ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/pkg/genericapiserver/config.go b/pkg/genericapiserver/config.go index b613d1824d5..12cd5789d03 100644 --- a/pkg/genericapiserver/config.go +++ b/pkg/genericapiserver/config.go @@ -158,11 +158,15 @@ type Config struct { // MaxRequestsInFlight is the maximum number of parallel non-long-running requests. Every further // request has to wait. - MaxRequestsInFlight int - LongRunningRequestRE string + MaxRequestsInFlight int + + // Predicate which is true for paths of long-running http requests + LongRunningFunc genericfilters.LongRunningRequestCheck } func NewConfig(options *options.ServerRunOptions) *Config { + longRunningRE := regexp.MustCompile(options.LongRunningRequestRE) + var auditWriter io.Writer if len(options.AuditLogPath) != 0 { auditWriter = &lumberjack.Logger{ @@ -201,8 +205,8 @@ func NewConfig(options *options.ServerRunOptions) *Config { Version: "unversioned", }, }, - MaxRequestsInFlight: options.MaxRequestsInFlight, - LongRunningRequestRE: options.LongRunningRequestRE, + MaxRequestsInFlight: options.MaxRequestsInFlight, + LongRunningFunc: genericfilters.BasicLongRunningRequestCheck(longRunningRE, map[string]string{"watch": "true"}), } } @@ -345,16 +349,13 @@ func (c Config) New() (*GenericAPIServer, error) { } func (s *GenericAPIServer) buildHandlerChains(c *Config, handler http.Handler) (secure http.Handler, insecure http.Handler) { - longRunningRE := regexp.MustCompile(c.LongRunningRequestRE) - longRunningFunc := genericfilters.BasicLongRunningRequestCheck(longRunningRE, map[string]string{"watch": "true"}) - // filters which insecure and secure have in common handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, "true") // insecure filters insecure = handler insecure = genericfilters.WithPanicRecovery(insecure, s.NewRequestInfoResolver()) - insecure = genericfilters.WithTimeoutForNonLongRunningRequests(insecure, longRunningFunc) + insecure = genericfilters.WithTimeoutForNonLongRunningRequests(insecure, c.LongRunningFunc) // secure filters attributeGetter := apiserverfilters.NewRequestAttributeGetter(c.RequestContextMapper, s.NewRequestInfoResolver()) @@ -364,8 +365,8 @@ func (s *GenericAPIServer) buildHandlerChains(c *Config, handler http.Handler) ( secure = apiserverfilters.WithAudit(secure, attributeGetter, c.AuditWriter) // before impersonation to read original user secure = authhandlers.WithAuthentication(secure, c.RequestContextMapper, c.Authenticator, authhandlers.Unauthorized(c.SupportsBasicAuth)) secure = genericfilters.WithPanicRecovery(secure, s.NewRequestInfoResolver()) - secure = genericfilters.WithTimeoutForNonLongRunningRequests(secure, longRunningFunc) - secure = genericfilters.WithMaxInFlightLimit(secure, c.MaxRequestsInFlight, longRunningFunc) + secure = genericfilters.WithTimeoutForNonLongRunningRequests(secure, c.LongRunningFunc) + secure = genericfilters.WithMaxInFlightLimit(secure, c.MaxRequestsInFlight, c.LongRunningFunc) return } diff --git a/pkg/genericapiserver/filters/timeout.go b/pkg/genericapiserver/filters/timeout.go index 4315af95e1e..474635759c2 100644 --- a/pkg/genericapiserver/filters/timeout.go +++ b/pkg/genericapiserver/filters/timeout.go @@ -35,6 +35,9 @@ var errConnKilled = fmt.Errorf("kill connection/stream") // WithTimeoutForNonLongRunningRequests times out non-long-running requests after the time given by globalTimeout. func WithTimeoutForNonLongRunningRequests(handler http.Handler, longRunning LongRunningRequestCheck) http.Handler { + if longRunning == nil { + return handler + } timeoutFunc := func(req *http.Request) (<-chan time.Time, string) { // TODO unify this with apiserver.MaxInFlightLimit if longRunning(req) {