From 9713b58caa462f1fadbb162a78fa891c5dfc5b52 Mon Sep 17 00:00:00 2001 From: Eric Tune Date: Tue, 28 Oct 2014 13:02:19 -0700 Subject: [PATCH] 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. --- cmd/apiserver/apiserver.go | 2 -- cmd/integration/integration.go | 6 ++--- pkg/master/master.go | 43 +++++++++++++++++++++++++++++++-- test/integration/auth_test.go | 34 +++++++++++++++----------- test/integration/client_test.go | 8 ++---- 5 files changed, 65 insertions(+), 28 deletions(-) diff --git a/cmd/apiserver/apiserver.go b/cmd/apiserver/apiserver.go index ac06f668370c8..083e73ff756ad 100644 --- 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 index 6aec0b8e6de38..8182f699073cf 100644 --- a/cmd/integration/integration.go +++ b/cmd/integration/integration.go @@ -137,14 +137,12 @@ 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", @@ -152,7 +150,7 @@ func startComponents(manifestURL string) (apiServerURL string) { 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 index ea62613a87cf4..6aa6ccb0ca2a0 100644 --- 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 index a22c7581a0515..515a9204c1885 100644 --- 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 index 821564f850427..999930cd7dd0f 100644 --- 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",