Cleanup genericapiserver handler chain

hack/.linted_packages

@@ -105,6 +105,8 @@ pkg/controller/volume/reconciler
 pkg/controller/volume/statusupdater
 pkg/conversion/queryparams
 pkg/credentialprovider/aws
+pkg/genericapiserver/filters
+pkg/genericapiserver/routes
 pkg/hyperkube
 pkg/kubelet/api
 pkg/kubelet/container

pkg/api/requestcontext.go

@@ -20,6 +20,8 @@ import (
 	"errors"
 	"net/http"
 	"sync"
+
+	"github.com/golang/glog"
 )
 
 // RequestContextMapper keeps track of the context associated with a particular request
@@ -89,21 +91,21 @@ func (c *requestContextMap) remove(req *http.Request) {
 	delete(c.contexts, req)
 }
 
-// NewRequestContextFilter ensures there is a Context object associated with the request before calling the passed handler.
+// WithRequestContext ensures there is a Context object associated with the request before calling the passed handler.
 // After the passed handler runs, the context is cleaned up.
-func NewRequestContextFilter(mapper RequestContextMapper, handler http.Handler) (http.Handler, error) {
-	if mapper, ok := mapper.(*requestContextMap); ok {
-		return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-			if mapper.init(req, NewContext()) {
-				// If we were the ones to successfully initialize, pair with a remove
-				defer mapper.remove(req)
-			}
-			handler.ServeHTTP(w, req)
-		}), nil
-	} else {
-		return handler, errors.New("Unknown RequestContextMapper implementation.")
+func WithRequestContext(handler http.Handler, mapper RequestContextMapper) http.Handler {
+	rcMap, ok := mapper.(*requestContextMap)
+	if !ok {
+		glog.Fatal("Unknown RequestContextMapper implementation.")
 	}
 
+	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		if rcMap.init(req, NewContext()) {
+			// If we were the ones to successfully initialize, pair with a remove
+			defer rcMap.remove(req)
+		}
+		handler.ServeHTTP(w, req)
+	})
 }
 
 // IsEmpty returns true if there are no contexts registered, or an error if it could not be determined. Intended for use by tests.

pkg/apiserver/apiserver.go

@@ -293,8 +293,8 @@ func keepUnversioned(group string) bool {
 	return group == "" || group == "extensions"
 }
 
-// Adds a service to return the supported api versions at /apis.
-func AddApisWebService(s runtime.NegotiatedSerializer, container *restful.Container, apiPrefix string, f func(req *restful.Request) []unversioned.APIGroup) {
+// NewApisWebService returns a webservice serving the available api version under /apis.
+func NewApisWebService(s runtime.NegotiatedSerializer, apiPrefix string, f func(req *restful.Request) []unversioned.APIGroup) *restful.WebService {
 	// Because in release 1.1, /apis returns response with empty APIVersion, we
 	// use StripVersionNegotiatedSerializer to keep the response backwards
 	// compatible.
@@ -309,12 +309,12 @@ func AddApisWebService(s runtime.NegotiatedSerializer, container *restful.Contai
 		Produces(s.SupportedMediaTypes()...).
 		Consumes(s.SupportedMediaTypes()...).
 		Writes(unversioned.APIGroupList{}))
-	container.Add(ws)
+	return ws
 }
 
-// Adds a service to return the supported versions, preferred version, and name
-// of a group. E.g., a such web service will be registered at /apis/extensions.
-func AddGroupWebService(s runtime.NegotiatedSerializer, container *restful.Container, path string, group unversioned.APIGroup) {
+// NewGroupWebService returns a webservice serving the supported versions, preferred version, and name
+// of a group. E.g., such a web service will be registered at /apis/extensions.
+func NewGroupWebService(s runtime.NegotiatedSerializer, path string, group unversioned.APIGroup) *restful.WebService {
 	ss := s
 	if keepUnversioned(group.Name) {
 		// Because in release 1.1, /apis/extensions returns response with empty
@@ -332,7 +332,7 @@ func AddGroupWebService(s runtime.NegotiatedSerializer, container *restful.Conta
 		Produces(s.SupportedMediaTypes()...).
 		Consumes(s.SupportedMediaTypes()...).
 		Writes(unversioned.APIGroup{}))
-	container.Add(ws)
+	return ws
 }
 
 // Adds a service to return the supported resources, E.g., a such web service

pkg/apiserver/apiserver_test.go

@@ -43,7 +43,6 @@ import (
 	"k8s.io/kubernetes/pkg/fields"
 	"k8s.io/kubernetes/pkg/labels"
 	"k8s.io/kubernetes/pkg/runtime"
-	"k8s.io/kubernetes/pkg/util"
 	"k8s.io/kubernetes/pkg/util/diff"
 	"k8s.io/kubernetes/pkg/util/sets"
 	"k8s.io/kubernetes/pkg/watch"
@@ -2995,80 +2994,6 @@ func TestCreateTimeout(t *testing.T) {
 	}
 }
 
-func TestCORSAllowedOrigins(t *testing.T) {
-	table := []struct {
-		allowedOrigins []string
-		origin         string
-		allowed        bool
-	}{
-		{[]string{}, "example.com", false},
-		{[]string{"example.com"}, "example.com", true},
-		{[]string{"example.com"}, "not-allowed.com", false},
-		{[]string{"not-matching.com", "example.com"}, "example.com", true},
-		{[]string{".*"}, "example.com", true},
-	}
-
-	for _, item := range table {
-		allowedOriginRegexps, err := util.CompileRegexps(item.allowedOrigins)
-		if err != nil {
-			t.Errorf("unexpected error: %v", err)
-		}
-
-		handler := CORS(
-			handle(map[string]rest.Storage{}),
-			allowedOriginRegexps, nil, nil, "true",
-		)
-		server := httptest.NewServer(handler)
-		defer server.Close()
-		client := http.Client{}
-
-		request, err := http.NewRequest("GET", server.URL+"/version", nil)
-		if err != nil {
-			t.Errorf("unexpected error: %v", err)
-		}
-		request.Header.Set("Origin", item.origin)
-
-		response, err := client.Do(request)
-		if err != nil {
-			t.Errorf("unexpected error: %v", err)
-		}
-
-		if item.allowed {
-			if !reflect.DeepEqual(item.origin, response.Header.Get("Access-Control-Allow-Origin")) {
-				t.Errorf("Expected %#v, Got %#v", item.origin, response.Header.Get("Access-Control-Allow-Origin"))
-			}
-
-			if response.Header.Get("Access-Control-Allow-Credentials") == "" {
-				t.Errorf("Expected Access-Control-Allow-Credentials header to be set")
-			}
-
-			if response.Header.Get("Access-Control-Allow-Headers") == "" {
-				t.Errorf("Expected Access-Control-Allow-Headers header to be set")
-			}
-
-			if response.Header.Get("Access-Control-Allow-Methods") == "" {
-				t.Errorf("Expected Access-Control-Allow-Methods header to be set")
-			}
-		} else {
-			if response.Header.Get("Access-Control-Allow-Origin") != "" {
-				t.Errorf("Expected Access-Control-Allow-Origin header to not be set")
-			}
-
-			if response.Header.Get("Access-Control-Allow-Credentials") != "" {
-				t.Errorf("Expected Access-Control-Allow-Credentials header to not be set")
-			}
-
-			if response.Header.Get("Access-Control-Allow-Headers") != "" {
-				t.Errorf("Expected Access-Control-Allow-Headers header to not be set")
-			}
-
-			if response.Header.Get("Access-Control-Allow-Methods") != "" {
-				t.Errorf("Expected Access-Control-Allow-Methods header to not be set")
-			}
-		}
-	}
-}
-
 func TestCreateChecksAPIVersion(t *testing.T) {
 	handler := handle(map[string]rest.Storage{"simple": &SimpleRESTStorage{}})
 	server := httptest.NewServer(handler)

pkg/apiserver/audit/audit.go

@@ -72,7 +72,8 @@ var _ http.Flusher = &fancyResponseWriterDelegator{}
 var _ http.Hijacker = &fancyResponseWriterDelegator{}
 
 // WithAudit decorates a http.Handler with audit logging information for all the
-// requests coming to the server. Each audit log contains two entries:
+// requests coming to the server. If out is nil, no decoration takes place.
+// Each audit log contains two entries:
 // 1. the request line containing:
 //    - unique id allowing to match the response line (see 2)
 //    - source ip of the request
@@ -85,6 +86,9 @@ var _ http.Hijacker = &fancyResponseWriterDelegator{}
 //    - the unique id from 1
 //    - response code
 func WithAudit(handler http.Handler, attributeGetter apiserver.RequestAttributeGetter, out io.Writer) http.Handler {
+	if out == nil {
+		return handler
+	}
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		attribs := attributeGetter.GetAttribs(req)
 		asuser := req.Header.Get("Impersonate-User")

pkg/apiserver/handler_impersonation_test.go

@@ -302,7 +302,7 @@ func TestImpersonationFilter(t *testing.T) {
 			delegate.ServeHTTP(w, req)
 		})
 	}(WithImpersonation(doNothingHandler, requestContextMapper, impersonateAuthorizer{}))
-	handler, _ = api.NewRequestContextFilter(requestContextMapper, handler)
+	handler = api.WithRequestContext(handler, requestContextMapper)
 
 	server := httptest.NewServer(handler)
 	defer server.Close()