move legacy insecure options out of the main flow

Kubernetes-commit: cd297546807fc08546905a2b96879d13bcf3a30b

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.
+	// BuildHandlerChainFunc allows you to build custom handler chains by decorating the apiHandler.
-	BuildHandlerChainsFunc func(apiHandler http.Handler, c *Config) (secure, insecure http.Handler)
+	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,
+		SecureServingInfo: c.SecureServingInfo,
-		InsecureServingInfo: c.InsecureServingInfo,
+		ExternalAddress:   c.ExternalAddress,
-		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) {
+func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) http.Handler {
-	generic := func(handler http.Handler) http.Handler {
+	handler := genericapifilters.WithAuthorization(apiHandler, c.RequestContextMapper, c.Authorizer)
-		handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, nil, "true")
+	handler = genericapifilters.WithImpersonation(handler, c.RequestContextMapper, c.Authorizer)
-		handler = genericfilters.WithPanicRecovery(handler, c.RequestContextMapper)
+	handler = genericapifilters.WithAudit(handler, c.RequestContextMapper, c.AuditWriter)
-		handler = genericfilters.WithTimeoutForNonLongRunningRequests(handler, c.RequestContextMapper, c.LongRunningFunc)
+	handler = genericapifilters.WithAuthentication(handler, c.RequestContextMapper, c.Authenticator, genericapifilters.Unauthorized(c.SupportsBasicAuth))
-		handler = genericfilters.WithMaxInFlightLimit(handler, c.MaxRequestsInFlight, c.MaxMutatingRequestsInFlight, c.RequestContextMapper, c.LongRunningFunc)
+	handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, nil, "true")
-		handler = genericapifilters.WithRequestInfo(handler, NewRequestInfoResolver(c), c.RequestContextMapper)
+	handler = genericfilters.WithPanicRecovery(handler, c.RequestContextMapper)
-		handler = apirequest.WithRequestContext(handler, c.RequestContextMapper)
+	handler = genericfilters.WithTimeoutForNonLongRunningRequests(handler, c.RequestContextMapper, c.LongRunningFunc)
-		return handler
+	handler = genericfilters.WithMaxInFlightLimit(handler, c.MaxRequestsInFlight, c.MaxMutatingRequestsInFlight, c.RequestContextMapper, c.LongRunningFunc)
-	}
+	handler = genericapifilters.WithRequestInfo(handler, NewRequestInfoResolver(c), c.RequestContextMapper)
-	audit := func(handler http.Handler) http.Handler {
+	handler = apirequest.WithRequestContext(handler, c.RequestContextMapper)
-		return genericapifilters.WithAudit(handler, c.RequestContextMapper, c.AuditWriter)
+	return handler
-	}
-	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 installAPI(s *GenericAPIServer, c *Config, delegate http.Handler) {

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) {
+func LoopbackHostPort(bindAddress string) (string, string, error) {
-	host, port, err := net.SplitHostPort(s.BindAddress)
+	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 {

pkg/server/genericapiserver.go

@@ -105,11 +105,10 @@ type GenericAPIServer struct {
 	// The registered APIs
 	HandlerContainer *genericmux.APIContainer
 
-	SecureServingInfo   *SecureServingInfo
+	SecureServingInfo *SecureServingInfo
-	InsecureServingInfo *ServingInfo
 
 	// 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
+	Handler http.Handler
-	InsecureHandler 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.

pkg/server/genericapiserver_test.go

@@ -202,7 +202,7 @@ func TestInstallAPIGroups(t *testing.T) {
 		groupPaths = append(groupPaths, APIGroupPrefix+"/"+api.GroupMeta.GroupVersion.Group) // /apis/<group>
 	}
 
-	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
+			protected = true
-				apiHandler.ServeHTTP(w, req)
+			apiHandler.ServeHTTP(w, req)
-			}), http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		})
-				protected = false
-				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)

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

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 {

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
+// RunServer listens on the given port, then spawns a go-routine continuously serving
-// 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
 // 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")
 	}