Merge pull request #37697 from deads2k/auth-06-simplify-authz

Automatic merge from submit-queue (batch tested with PRs 35300, 36709, 37643, 37813, 37697)

simplify the authorization attribute getter

Construct the authorization attributes directly from the context.  This eliminates unnecessary redirection.  

@sttts

pkg/apiserver/api_installer.go

@@ -752,7 +752,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		default:
 			return nil, fmt.Errorf("unrecognized action verb: %s", action.Verb)
 		}
-		// Note: update GetAttribs() when adding a custom handler.
+		// Note: update GetAuthorizerAttributes() when adding a custom handler.
 	}
 	return &apiResource, nil
 }

pkg/apiserver/filters/audit.go

@@ -18,6 +18,7 @@ package filters
 
 import (
 	"bufio"
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -28,6 +29,7 @@ import (
 	"github.com/golang/glog"
 	"github.com/pborman/uuid"
 
+	"k8s.io/kubernetes/pkg/api"
 	authenticationapi "k8s.io/kubernetes/pkg/apis/authentication"
 	utilnet "k8s.io/kubernetes/pkg/util/net"
 )
@@ -86,12 +88,17 @@ var _ http.Hijacker = &fancyResponseWriterDelegator{}
 // 2. the response line containing:
 //    - the unique id from 1
 //    - response code
-func WithAudit(handler http.Handler, attributeGetter RequestAttributeGetter, out io.Writer) http.Handler {
+func WithAudit(handler http.Handler, requestContextMapper api.RequestContextMapper, out io.Writer) http.Handler {
 	if out == nil {
 		return handler
 	}
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		attribs, err := attributeGetter.GetAttribs(req)
+		ctx, ok := requestContextMapper.Get(req)
+		if !ok {
+			internalError(w, req, errors.New("no context found for request"))
+			return
+		}
+		attribs, err := GetAuthorizerAttributes(ctx)
 		if err != nil {
 			internalError(w, req, err)
 			return

pkg/apiserver/filters/audit_test.go

@@ -74,10 +74,9 @@ func (*fakeHTTPHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 func TestAudit(t *testing.T) {
 	var buf bytes.Buffer
 
-	attributeGetter := NewRequestAttributeGetter(&fakeRequestContextMapper{
+	handler := WithAudit(&fakeHTTPHandler{}, &fakeRequestContextMapper{
 		user: &user.DefaultInfo{Name: "admin"},
-	})
-	handler := WithAudit(&fakeHTTPHandler{}, attributeGetter, &buf)
+	}, &buf)
 
 	req, _ := http.NewRequest("GET", "/api/v1/namespaces/default/pods", nil)
 	req.RemoteAddr = "127.0.0.1"

pkg/apiserver/filters/authorization.go

@@ -28,13 +28,19 @@ import (
 )
 
 // WithAuthorizationCheck passes all authorized requests on to handler, and returns a forbidden error otherwise.
-func WithAuthorization(handler http.Handler, getAttribs RequestAttributeGetter, a authorizer.Authorizer) http.Handler {
+func WithAuthorization(handler http.Handler, requestContextMapper api.RequestContextMapper, a authorizer.Authorizer) http.Handler {
 	if a == nil {
 		glog.Warningf("Authorization is disabled")
 		return handler
 	}
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		attrs, err := getAttribs.GetAttribs(req)
+		ctx, ok := requestContextMapper.Get(req)
+		if !ok {
+			internalError(w, req, errors.New("no context found for request"))
+			return
+		}
+
+		attrs, err := GetAuthorizerAttributes(ctx)
 		if err != nil {
 			internalError(w, req, err)
 			return
@@ -54,28 +60,9 @@ func WithAuthorization(handler http.Handler, getAttribs RequestAttributeGetter,
 	})
 }
 
-// RequestAttributeGetter is a function that extracts authorizer.Attributes from an http.Request
-type RequestAttributeGetter interface {
-	GetAttribs(req *http.Request) (authorizer.Attributes, error)
-}
-
-type requestAttributeGetter struct {
-	requestContextMapper api.RequestContextMapper
-}
-
-// NewAttributeGetter returns an object which implements the RequestAttributeGetter interface.
-func NewRequestAttributeGetter(requestContextMapper api.RequestContextMapper) RequestAttributeGetter {
-	return &requestAttributeGetter{requestContextMapper}
-}
-
-func (r *requestAttributeGetter) GetAttribs(req *http.Request) (authorizer.Attributes, error) {
+func GetAuthorizerAttributes(ctx api.Context) (authorizer.Attributes, error) {
 	attribs := authorizer.AttributesRecord{}
 
-	ctx, ok := r.requestContextMapper.Get(req)
-	if !ok {
-		return nil, errors.New("no context found for request")
-	}
-
 	user, ok := api.UserFrom(ctx)
 	if ok {
 		attribs.User = user

pkg/apiserver/filters/authorization_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package filters
 
 import (
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"reflect"
@@ -27,9 +28,8 @@ import (
 	"k8s.io/kubernetes/pkg/auth/authorizer"
 )
 
-func TestGetAttribs(t *testing.T) {
+func TestGetAuthorizerAttributes(t *testing.T) {
 	mapper := api.NewRequestContextMapper()
-	attributeGetter := NewRequestAttributeGetter(mapper)
 
 	testcases := map[string]struct {
 		Verb               string
@@ -108,7 +108,12 @@ func TestGetAttribs(t *testing.T) {
 		var attribs authorizer.Attributes
 		var err error
 		var handler http.Handler = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-			attribs, err = attributeGetter.GetAttribs(req)
+			ctx, ok := mapper.Get(req)
+			if !ok {
+				internalError(w, req, errors.New("no context found for request"))
+				return
+			}
+			attribs, err = GetAuthorizerAttributes(ctx)
 		})
 		handler = WithRequestInfo(handler, newTestRequestInfoResolver(), mapper)
 		handler = api.WithRequestContext(handler, mapper)

pkg/genericapiserver/config.go

@@ -485,8 +485,6 @@ func (c *Config) MaybeGenerateServingCerts(alternateIPs ...net.IP) error {
 }
 
 func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) (secure, insecure http.Handler) {
-	attributeGetter := apiserverfilters.NewRequestAttributeGetter(c.RequestContextMapper)
-
 	generic := func(handler http.Handler) http.Handler {
 		handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, nil, "true")
 		handler = genericfilters.WithPanicRecovery(handler, c.RequestContextMapper)
@@ -497,10 +495,10 @@ func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) (secure, insec
 		return handler
 	}
 	audit := func(handler http.Handler) http.Handler {
-		return apiserverfilters.WithAudit(handler, attributeGetter, c.AuditWriter)
+		return apiserverfilters.WithAudit(handler, c.RequestContextMapper, c.AuditWriter)
 	}
 	protect := func(handler http.Handler) http.Handler {
-		handler = apiserverfilters.WithAuthorization(handler, attributeGetter, c.Authorizer)
+		handler = apiserverfilters.WithAuthorization(handler, c.RequestContextMapper, c.Authorizer)
 		handler = apiserverfilters.WithImpersonation(handler, c.RequestContextMapper, c.Authorizer)
 		handler = audit(handler) // before impersonation to read original user
 		handler = authhandlers.WithAuthentication(handler, c.RequestContextMapper, c.Authenticator, authhandlers.Unauthorized(c.SupportsBasicAuth))