From 38fb6e78f7b417e402f5865370cdbabd76c92e11 Mon Sep 17 00:00:00 2001 From: deads2k Date: Thu, 9 Mar 2017 14:39:56 -0500 Subject: [PATCH] move legacy insecure options out of the main flow Kubernetes-commit: cd297546807fc08546905a2b96879d13bcf3a30b --- pkg/server/config.go | 57 ++++++++++------------------- pkg/server/config_selfclient.go | 30 +++------------ pkg/server/genericapiserver.go | 15 ++------ pkg/server/genericapiserver_test.go | 19 ++++------ pkg/server/options/serving.go | 4 +- pkg/server/options/serving_test.go | 1 - pkg/server/serve.go | 21 ++--------- 7 files changed, 38 insertions(+), 109 deletions(-) diff --git a/pkg/server/config.go b/pkg/server/config.go index 8b96194ec..6efd6e8be 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -116,8 +116,8 @@ type Config struct { // Fields you probably don't care about changing //=========================================================================== - // BuildHandlerChainsFunc allows you to build custom handler chains by decorating the apiHandler. - BuildHandlerChainsFunc func(apiHandler http.Handler, c *Config) (secure, insecure http.Handler) + // BuildHandlerChainFunc allows you to build custom handler chains by decorating the apiHandler. + BuildHandlerChainFunc func(apiHandler http.Handler, c *Config) (secure http.Handler) // DiscoveryAddresses is used to build the IPs pass to discovery. If nil, the ExternalAddress is // always reported DiscoveryAddresses DiscoveryAddresses @@ -152,10 +152,6 @@ type Config struct { // Predicate which is true for paths of long-running http requests LongRunningFunc genericfilters.LongRunningRequestCheck - // InsecureServingInfo is required to serve http. HTTP does NOT include authentication or authorization. - // You shouldn't be using this. It makes sig-auth sad. - InsecureServingInfo *ServingInfo - //=========================================================================== // values below here are targets for removal //=========================================================================== @@ -169,16 +165,12 @@ type Config struct { PublicAddress net.IP } -type ServingInfo struct { +type SecureServingInfo struct { // BindAddress is the ip:port to serve on BindAddress string // BindNetwork is the type of network to bind to - defaults to "tcp", accepts "tcp", // "tcp4", and "tcp6". BindNetwork string -} - -type SecureServingInfo struct { - ServingInfo // Cert is the main server cert which is used if SNI does not match. Cert must be non-nil and is // allowed to be in SNICerts. @@ -201,7 +193,7 @@ func NewConfig(codecs serializer.CodecFactory) *Config { Serializer: codecs, ReadWritePort: 443, RequestContextMapper: apirequest.NewRequestContextMapper(), - BuildHandlerChainsFunc: DefaultBuildHandlerChain, + BuildHandlerChainFunc: DefaultBuildHandlerChain, LegacyAPIGroupPrefixes: sets.NewString(DefaultLegacyAPIPrefix), HealthzChecks: []healthz.HealthzChecker{healthz.PingHealthz}, EnableIndex: true, @@ -402,9 +394,8 @@ func (c completedConfig) constructServer() (*GenericAPIServer, error) { minRequestTimeout: time.Duration(c.MinRequestTimeout) * time.Second, - SecureServingInfo: c.SecureServingInfo, - InsecureServingInfo: c.InsecureServingInfo, - ExternalAddress: c.ExternalAddress, + SecureServingInfo: c.SecureServingInfo, + ExternalAddress: c.ExternalAddress, apiGroupsForDiscovery: map[string]metav1.APIGroup{}, @@ -477,33 +468,23 @@ func (c completedConfig) buildHandlers(s *GenericAPIServer, delegate http.Handle installAPI(s, c.Config, delegate) - s.Handler, s.InsecureHandler = c.BuildHandlerChainsFunc(s.HandlerContainer.ServeMux, c.Config) + s.Handler = c.BuildHandlerChainFunc(s.HandlerContainer.ServeMux, c.Config) return s, nil } -func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) (secure, insecure http.Handler) { - generic := func(handler http.Handler) http.Handler { - handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, nil, "true") - handler = genericfilters.WithPanicRecovery(handler, c.RequestContextMapper) - handler = genericfilters.WithTimeoutForNonLongRunningRequests(handler, c.RequestContextMapper, c.LongRunningFunc) - handler = genericfilters.WithMaxInFlightLimit(handler, c.MaxRequestsInFlight, c.MaxMutatingRequestsInFlight, c.RequestContextMapper, c.LongRunningFunc) - handler = genericapifilters.WithRequestInfo(handler, NewRequestInfoResolver(c), c.RequestContextMapper) - handler = apirequest.WithRequestContext(handler, c.RequestContextMapper) - return handler - } - audit := func(handler http.Handler) http.Handler { - return genericapifilters.WithAudit(handler, c.RequestContextMapper, c.AuditWriter) - } - protect := func(handler http.Handler) http.Handler { - handler = genericapifilters.WithAuthorization(handler, c.RequestContextMapper, c.Authorizer) - handler = genericapifilters.WithImpersonation(handler, c.RequestContextMapper, c.Authorizer) - handler = audit(handler) // before impersonation to read original user - handler = genericapifilters.WithAuthentication(handler, c.RequestContextMapper, c.Authenticator, genericapifilters.Unauthorized(c.SupportsBasicAuth)) - return handler - } - - return generic(protect(apiHandler)), generic(audit(apiHandler)) +func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) http.Handler { + handler := genericapifilters.WithAuthorization(apiHandler, c.RequestContextMapper, c.Authorizer) + handler = genericapifilters.WithImpersonation(handler, c.RequestContextMapper, c.Authorizer) + handler = genericapifilters.WithAudit(handler, c.RequestContextMapper, c.AuditWriter) + handler = genericapifilters.WithAuthentication(handler, c.RequestContextMapper, c.Authenticator, genericapifilters.Unauthorized(c.SupportsBasicAuth)) + handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, nil, "true") + handler = genericfilters.WithPanicRecovery(handler, c.RequestContextMapper) + handler = genericfilters.WithTimeoutForNonLongRunningRequests(handler, c.RequestContextMapper, c.LongRunningFunc) + handler = genericfilters.WithMaxInFlightLimit(handler, c.MaxRequestsInFlight, c.MaxMutatingRequestsInFlight, c.RequestContextMapper, c.LongRunningFunc) + handler = genericapifilters.WithRequestInfo(handler, NewRequestInfoResolver(c), c.RequestContextMapper) + handler = apirequest.WithRequestContext(handler, c.RequestContextMapper) + return handler } func installAPI(s *GenericAPIServer, c *Config, delegate http.Handler) { diff --git a/pkg/server/config_selfclient.go b/pkg/server/config_selfclient.go index ba7976017..a027b6076 100644 --- a/pkg/server/config_selfclient.go +++ b/pkg/server/config_selfclient.go @@ -33,7 +33,7 @@ func (s *SecureServingInfo) NewLoopbackClientConfig(token string, loopbackCert [ return nil, nil } - host, port, err := s.ServingInfo.loopbackHostPort() + host, port, err := LoopbackHostPort(s.BindAddress) if err != nil { return nil, err } @@ -90,13 +90,13 @@ func findCA(chain []*x509.Certificate) (*x509.Certificate, error) { return nil, fmt.Errorf("no certificate with CA:TRUE found in chain") } -// loopbackHostPort returns the host and port loopback REST clients should use +// LoopbackHostPort returns the host and port loopback REST clients should use // to contact the server. -func (s *ServingInfo) loopbackHostPort() (string, string, error) { - host, port, err := net.SplitHostPort(s.BindAddress) +func LoopbackHostPort(bindAddress string) (string, string, error) { + host, port, err := net.SplitHostPort(bindAddress) if err != nil { // should never happen - return "", "", fmt.Errorf("invalid server bind address: %q", s.BindAddress) + return "", "", fmt.Errorf("invalid server bind address: %q", bindAddress) } // Value is expected to be an IP or DNS name, not "0.0.0.0". @@ -107,26 +107,6 @@ func (s *ServingInfo) loopbackHostPort() (string, string, error) { return host, port, nil } -func (s *ServingInfo) NewLoopbackClientConfig(token string) (*restclient.Config, error) { - if s == nil { - return nil, nil - } - - host, port, err := s.loopbackHostPort() - if err != nil { - return nil, err - } - - return &restclient.Config{ - Host: "http://" + net.JoinHostPort(host, port), - // Increase QPS limits. The client is currently passed to all admission plugins, - // and those can be throttled in case of higher load on apiserver - see #22340 and #22422 - // for more details. Once #22422 is fixed, we may want to remove it. - QPS: 50, - Burst: 100, - }, nil -} - func certMatchesName(cert *x509.Certificate, name string) bool { for _, certName := range cert.DNSNames { if certName == name { diff --git a/pkg/server/genericapiserver.go b/pkg/server/genericapiserver.go index 21bb8af9d..4a62e65c3 100644 --- a/pkg/server/genericapiserver.go +++ b/pkg/server/genericapiserver.go @@ -105,11 +105,10 @@ type GenericAPIServer struct { // The registered APIs HandlerContainer *genericmux.APIContainer - SecureServingInfo *SecureServingInfo - InsecureServingInfo *ServingInfo + SecureServingInfo *SecureServingInfo // numerical ports, set after listening - effectiveSecurePort, effectiveInsecurePort int + effectiveSecurePort int // ExternalAddress is the address (hostname or IP and port) that should be used in // external (public internet) URLs for this GenericAPIServer. @@ -123,8 +122,7 @@ type GenericAPIServer struct { Serializer runtime.NegotiatedSerializer // "Outputs" - Handler http.Handler - InsecureHandler http.Handler + Handler http.Handler // FallThroughHandler is the final HTTP handler in the chain. // It comes after all filters and the API handling FallThroughHandler *mux.PathRecorderMux @@ -277,13 +275,6 @@ func (s preparedGenericAPIServer) NonBlockingRun(stopCh <-chan struct{}) error { } } - if s.InsecureServingInfo != nil && s.InsecureHandler != nil { - if err := s.serveInsecurely(internalStopCh); err != nil { - close(internalStopCh) - return err - } - } - // Now that both listeners have bound successfully, it is the // responsibility of the caller to close the provided channel to // ensure cleanup. diff --git a/pkg/server/genericapiserver_test.go b/pkg/server/genericapiserver_test.go index 58a094326..bc500cefe 100644 --- a/pkg/server/genericapiserver_test.go +++ b/pkg/server/genericapiserver_test.go @@ -202,7 +202,7 @@ func TestInstallAPIGroups(t *testing.T) { groupPaths = append(groupPaths, APIGroupPrefix+"/"+api.GroupMeta.GroupVersion.Group) // /apis/ } - server := httptest.NewServer(s.InsecureHandler) + server := httptest.NewServer(s.Handler) defer server.Close() for i := range apis { @@ -336,14 +336,11 @@ func TestCustomHandlerChain(t *testing.T) { var protected, called bool - config.BuildHandlerChainsFunc = func(apiHandler http.Handler, c *Config) (secure, insecure http.Handler) { + config.BuildHandlerChainFunc = func(apiHandler http.Handler, c *Config) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - protected = true - apiHandler.ServeHTTP(w, req) - }), http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - protected = false - apiHandler.ServeHTTP(w, req) - }) + protected = true + apiHandler.ServeHTTP(w, req) + }) } handler := http.HandlerFunc(func(r http.ResponseWriter, req *http.Request) { called = true @@ -365,8 +362,6 @@ func TestCustomHandlerChain(t *testing.T) { for i, test := range []Test{ {s.Handler, "/nonswagger", true}, {s.Handler, "/secret", true}, - {s.InsecureHandler, "/nonswagger", false}, - {s.InsecureHandler, "/secret", false}, } { protected, called = false, false @@ -485,7 +480,7 @@ func TestDiscoveryAtAPIS(t *testing.T) { master, etcdserver, _, assert := newMaster(t) defer etcdserver.Terminate(t) - server := httptest.NewServer(master.InsecureHandler) + server := httptest.NewServer(master.Handler) groupList, err := getGroupList(server) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -535,7 +530,7 @@ func TestDiscoveryOrdering(t *testing.T) { master, etcdserver, _, assert := newMaster(t) defer etcdserver.Terminate(t) - server := httptest.NewServer(master.InsecureHandler) + server := httptest.NewServer(master.Handler) groupList, err := getGroupList(server) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/pkg/server/options/serving.go b/pkg/server/options/serving.go index 103a99c47..cfa4d1dba 100644 --- a/pkg/server/options/serving.go +++ b/pkg/server/options/serving.go @@ -176,9 +176,7 @@ func (s *SecureServingOptions) applyServingInfoTo(c *server.Config) error { } secureServingInfo := &server.SecureServingInfo{ - ServingInfo: server.ServingInfo{ - BindAddress: net.JoinHostPort(s.BindAddress.String(), strconv.Itoa(s.BindPort)), - }, + BindAddress: net.JoinHostPort(s.BindAddress.String(), strconv.Itoa(s.BindPort)), } serverCertFile, serverKeyFile := s.ServerCert.CertKey.CertFile, s.ServerCert.CertKey.KeyFile diff --git a/pkg/server/options/serving_test.go b/pkg/server/options/serving_test.go index d746a5c73..4d974d22f 100644 --- a/pkg/server/options/serving_test.go +++ b/pkg/server/options/serving_test.go @@ -474,7 +474,6 @@ NextTest: t.Errorf("%q - failed applying the SecureServingOptions: %v", title, err) return } - config.InsecureServingInfo = nil s, err := config.Complete().New() if err != nil { diff --git a/pkg/server/serve.go b/pkg/server/serve.go index 9c9712f08..4a1da40d5 100644 --- a/pkg/server/serve.go +++ b/pkg/server/serve.go @@ -78,28 +78,13 @@ func (s *GenericAPIServer) serveSecurely(stopCh <-chan struct{}) error { glog.Infof("Serving securely on %s", s.SecureServingInfo.BindAddress) var err error - s.effectiveSecurePort, err = runServer(secureServer, s.SecureServingInfo.BindNetwork, stopCh) + s.effectiveSecurePort, err = RunServer(secureServer, s.SecureServingInfo.BindNetwork, stopCh) return err } -// serveInsecurely run the insecure http server. It fails only if the initial listen -// call fails. The actual server loop (stoppable by closing stopCh) runs in a go -// routine, i.e. serveInsecurely does not block. -func (s *GenericAPIServer) serveInsecurely(stopCh <-chan struct{}) error { - insecureServer := &http.Server{ - Addr: s.InsecureServingInfo.BindAddress, - Handler: s.InsecureHandler, - MaxHeaderBytes: 1 << 20, - } - glog.Infof("Serving insecurely on %s", s.InsecureServingInfo.BindAddress) - var err error - s.effectiveInsecurePort, err = runServer(insecureServer, s.InsecureServingInfo.BindNetwork, stopCh) - return err -} - -// runServer listens on the given port, then spawns a go-routine continuously serving +// RunServer listens on the given port, then spawns a go-routine continuously serving // until the stopCh is closed. The port is returned. This function does not block. -func runServer(server *http.Server, network string, stopCh <-chan struct{}) (int, error) { +func RunServer(server *http.Server, network string, stopCh <-chan struct{}) (int, error) { if len(server.Addr) == 0 { return 0, errors.New("address cannot be empty") }