Allocate mux in master.New()

Callsites no longer allocate a mux. Master now exposes method to install handlers which use the master's auth code. Not used but forks (openshift) are expected to use these methods. These methods will later be a point for additional plug-in functionality. Integration tests now use the master-provided handler which has auth, rather than using the mux, which didn't. Fix TestWhoAmI now that /_whoami sits behind auth.
kubernetes · Oct 29, 2014 · 9713b58 · 9713b58
1 parent ecdf65f
commit 9713b58
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 28 deletions.
diff --git a/cmd/apiserver/apiserver.go b/cmd/apiserver/apiserver.go
@@ -197,7 +197,6 @@ func main() {
 	}
 
 	n := net.IPNet(portalNet)
-	mux := http.NewServeMux()
 	config := &master.Config{
 		Client:             client,
 		Cloud:              cloud,
@@ -215,7 +214,6 @@ func main() {
 			},
 		},
 		PortalNet:             &n,
-		Mux:                   mux,
 		EnableLogsSupport:     *enableLogsSupport,
 		EnableUISupport:       true,
 		APIPrefix:             *apiPrefix,

diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go
@@ -137,22 +137,20 @@ func startComponents(manifestURL string) (apiServerURL string) {
 	if err != nil {
 		glog.Fatalf("Nonnumeric port? %v", err)
 	}
-	mux := http.NewServeMux()
 	// Create a master and install handlers into mux.
-	master.New(&master.Config{
+	m := master.New(&master.Config{
 		Client:            cl,
 		EtcdHelper:        helper,
 		Minions:           machineList,
 		KubeletClient:     fakeKubeletClient{},
-		Mux:               mux,
 		EnableLogsSupport: false,
 		APIPrefix:         "/api",
 
 		ReadWritePort: portNumber,
 		ReadOnlyPort:  portNumber,
 		PublicAddress: host,
 	})
-	handler.delegate = mux
+	handler.delegate = m.Handler
 
 	// Scheduler
 	schedulerConfigFactory := &factory.ConfigFactory{cl}

diff --git a/pkg/master/master.go b/pkg/master/master.go
@@ -180,7 +180,28 @@ func setDefaults(c *Config) {
 	}
 }
 
-// New returns a new instance of Master connected to the given etcd server.
+// New returns a new instance of Master from the given config.
+// Certain config fields will be set to a default value if unset,
+// including:
+//   PortalNet
+//   MasterCount
+//   ReadOnlyPort
+//   ReadWritePort
+//   PublicAddress
+// Certain config fields must be specified, including:
+//   KubeletClient
+// Public fields:
+//   Handler -- The returned master has a field TopHandler which is an
+//   http.Handler which handles all the endpoints provided by the master,
+//   including the API, the UI, and miscelaneous debugging endpoints.  All
+//   these are subject to authorization and authentication.
+// Public methods:
+//   HandleWithAuth -- Allows caller to add an http.Handler for an endpoint
+//   that uses the same authentication and authorization (if any is configured)
+//   as the master's built-in endpoints.
+//   If the caller wants to add additional endpoints not using the master's
+//   auth, then the caller should create a handler for those endpoints, which delegates the
+//   any unhandled paths to "Handler".
 func New(c *Config) *Master {
 	setDefaults(c)
 	minionRegistry := makeMinionRegistry(c)
@@ -198,7 +219,7 @@ func New(c *Config) *Master {
 		minionRegistry:        minionRegistry,
 		client:                c.Client,
 		portalNet:             c.PortalNet,
-		mux:                   c.Mux,
+		mux:                   http.NewServeMux(),
 		enableLogsSupport:     c.EnableLogsSupport,
 		enableUISupport:       c.EnableUISupport,
 		apiPrefix:             c.APIPrefix,
@@ -213,6 +234,24 @@ func New(c *Config) *Master {
 	return m
 }
 
+// HandleWithAuth adds an http.Handler for pattern to an http.ServeMux
+// Applies the same authentication and authorization (if any is configured)
+// to the request is used for the master's built-in endpoints.
+func (m *Master) HandleWithAuth(pattern string, handler http.Handler) {
+	// TODO: Add a way for plugged-in endpoints to translate their
+	// URLs into attributes that an Authorizer can understand, and have
+	// sensible policy defaults for plugged-in endpoints.  This will be different
+	// for generic endpoints versus REST object endpoints.
+	m.mux.Handle(pattern, handler)
+}
+
+// HandleFuncWithAuth adds an http.Handler for pattern to an http.ServeMux
+// Applies the same authentication and authorization (if any is configured)
+// to the request is used for the master's built-in endpoints.
+func (m *Master) HandleFuncWithAuth(pattern string, handler func(http.ResponseWriter, *http.Request)) {
+	m.mux.HandleFunc(pattern, handler)
+}
+
 func makeMinionRegistry(c *Config) minion.Registry {
 	var minionRegistry minion.Registry = etcd.NewRegistry(c.EtcdHelper, nil)
 	if c.HealthCheckMinions {

diff --git a/test/integration/auth_test.go b/test/integration/auth_test.go
@@ -63,18 +63,16 @@ xyz987,bob,2
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	mux := http.NewServeMux()
 
-	master.New(&master.Config{
+	m := master.New(&master.Config{
 		EtcdHelper:        helper,
-		Mux:               mux,
 		EnableLogsSupport: false,
 		EnableUISupport:   false,
 		APIPrefix:         "/api",
 		TokenAuthFile:     f.Name(),
 	})
 
-	s := httptest.NewServer(mux)
+	s := httptest.NewServer(m.Handler)
 	defer s.Close()
 
 	// TODO: also test TLS, using e.g NewUnsafeTLSTransport() and NewClientCertTLSTransport() (see pkg/client/helper.go)
@@ -84,10 +82,11 @@ xyz987,bob,2
 		name     string
 		token    string
 		expected string
+		succeeds bool
 	}{
-		{"Valid token", "abc123", "AUTHENTICATED AS alice"},
-		{"Unknown token", "456jkl", "NOT AUTHENTICATED"},
-		{"Empty token", "", "NOT AUTHENTICATED"},
+		{"Valid token", "abc123", "AUTHENTICATED AS alice", true},
+		{"Unknown token", "456jkl", "", false},
+		{"No token", "", "", false},
 	}
 	for _, tc := range testCases {
 		req, err := http.NewRequest("GET", s.URL+"/_whoami", nil)
@@ -101,14 +100,21 @@ xyz987,bob,2
 			t.Fatalf("unexpected error: %v", err)
 		}
 
-		body, err := ioutil.ReadAll(resp.Body)
-		if err != nil {
-			t.Fatalf("unexpected error: %v", err)
-		}
+		if tc.succeeds {
+			body, err := ioutil.ReadAll(resp.Body)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			actual := string(body)
+			if tc.expected != actual {
+				t.Errorf("case: %s expected: %v got: %v", tc.name, tc.expected, actual)
+			}
+		} else {
+			if resp.StatusCode != http.StatusUnauthorized {
+				t.Errorf("case: %s expected Unauthorized, got: %v", tc.name, resp.StatusCode)
+			}
 
-		actual := string(body)
-		if tc.expected != actual {
-			t.Errorf("case: %s expected: %v got: %v", tc.name, tc.expected, actual)
 		}
 	}
 }
diff --git a/test/integration/client_test.go b/test/integration/client_test.go
@@ -19,7 +19,6 @@ limitations under the License.
 package integration
 
 import (
-	"net/http"
 	"net/http/httptest"
 	"reflect"
 	"testing"
@@ -40,17 +39,14 @@ func TestClient(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	mux := http.NewServeMux()
-
-	master.New(&master.Config{
+	m := master.New(&master.Config{
 		EtcdHelper:        helper,
-		Mux:               mux,
 		EnableLogsSupport: false,
 		EnableUISupport:   false,
 		APIPrefix:         "/api",
 	})
 
-	s := httptest.NewServer(mux)
+	s := httptest.NewServer(m.Handler)
 
 	testCases := []string{
 		"v1beta1",