From ce32351bec1fbde402c8a1315852ae9c2a6a5e00 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Mon, 14 Mar 2016 12:41:59 -0600 Subject: [PATCH] fix(manager): simplify adding routes This removes the Routes interface and routesMap struct in favor of just maintaining the routes inside of the handler. --- cmd/manager/deployments.go | 9 +-- cmd/manager/deployments_test.go | 5 +- cmd/manager/main.go | 5 +- cmd/manager/router/context.go | 51 +++++++++++++ cmd/manager/router/router.go | 114 +++++------------------------- cmd/manager/router/router_test.go | 22 +----- 6 files changed, 78 insertions(+), 128 deletions(-) create mode 100644 cmd/manager/router/context.go diff --git a/cmd/manager/deployments.go b/cmd/manager/deployments.go index 6068dae46..39764121c 100644 --- a/cmd/manager/deployments.go +++ b/cmd/manager/deployments.go @@ -79,23 +79,20 @@ type Route struct { Type string } -func registerRoutes(c *router.Context) router.Routes { +func registerRoutes(c *router.Context, h *router.Handler) { re := regexp.MustCompile("{[a-z]+}") - r := router.NewRoutes() - r.Add("GET /healthz", healthz) + h.Add("GET /healthz", healthz) // TODO: Replace these routes with updated ones. for _, d := range deployments { path := fmt.Sprintf("%s %s", d.Methods, re.ReplaceAllString(d.Path, "*")) fmt.Printf("\t%s\n", path) - r.Add(path, func(w http.ResponseWriter, r *http.Request, c *router.Context) error { + h.Add(path, func(w http.ResponseWriter, r *http.Request, c *router.Context) error { d.HandlerFunc(w, r) return nil }) } - - return r } func healthz(w http.ResponseWriter, r *http.Request, c *router.Context) error { diff --git a/cmd/manager/deployments_test.go b/cmd/manager/deployments_test.go index 58a5ad3f6..b4560f794 100644 --- a/cmd/manager/deployments_test.go +++ b/cmd/manager/deployments_test.go @@ -28,9 +28,8 @@ func TestHealthz(t *testing.T) { // // You must Close() the returned server. func httpHarness(c *router.Context, route string, fn router.HandlerFunc) *httptest.Server { - r := router.NewRoutes() - r.Add(route, fn) - h := router.NewHandler(c, r) + h := router.NewHandler(c) + h.Add(route, fn) return httptest.NewServer(h) } diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 4c26d599f..297f53706 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -53,11 +53,12 @@ func main() { } // Set up routes - routes := registerRoutes(c) + handler := router.NewHandler(c) + registerRoutes(c, handler) // Now create a server. log.Printf("Starting Manager %s on %s", version.Version, c.Config.Address) - if err := http.ListenAndServe(c.Config.Address, router.NewHandler(c, routes)); err != nil { + if err := http.ListenAndServe(c.Config.Address, handler); err != nil { log.Printf("Server exited with error %s", err) os.Exit(1) } diff --git a/cmd/manager/router/context.go b/cmd/manager/router/context.go new file mode 100644 index 000000000..bca74ddfc --- /dev/null +++ b/cmd/manager/router/context.go @@ -0,0 +1,51 @@ +package router + +import ( + "github.com/kubernetes/deployment-manager/cmd/manager/manager" + "github.com/kubernetes/deployment-manager/pkg/common" + helmhttp "github.com/kubernetes/deployment-manager/pkg/httputil" +) + +// Config holds the global configuration parameters passed into the router. +// +// Config is used concurrently. Once a config is created, it should be treated +// as immutable. +type Config struct { + // Address is the host and port (:8080) + Address string + // MaxTemplateLength is the maximum length of a template. + MaxTemplateLength int64 + // ExpanderName is the DNS name of the expansion service. + ExpanderName string + // ExpanderURL is the expander service's URL. + ExpanderURL string + // DeployerName is the deployer's DNS name + DeployerName string + // DeployerURL is the deployer's URL + DeployerURL string + // CredentialFile is the file to the credentials. + CredentialFile string + // CredentialSecrets tells the service to use a secrets file instead. + CredentialSecrets bool + // MongoName is the DNS name of the mongo server. + MongoName string + // MongoPort is the port for the MongoDB protocol on the mongo server. + // It is a string for historical reasons. + MongoPort string + // MongoAddress is the name and port. + MongoAddress string +} + +// Context contains dependencies that are passed to each handler function. +// +// Context carries typed information, often scoped to interfaces, so that the +// caller's contract with the service is known at compile time. +// +// Members of the context must be concurrency safe. +type Context struct { + Config *Config + // Manager is a deployment-manager/manager/manager.Manager + Manager manager.Manager + Encoder helmhttp.Encoder + CredentialProvider common.CredentialProvider +} diff --git a/cmd/manager/router/router.go b/cmd/manager/router/router.go index 3a6bf5a42..69ecc1e72 100644 --- a/cmd/manager/router/router.go +++ b/cmd/manager/router/router.go @@ -33,55 +33,9 @@ import ( "net/http" "github.com/Masterminds/httputil" - "github.com/kubernetes/deployment-manager/cmd/manager/manager" - "github.com/kubernetes/deployment-manager/pkg/common" helmhttp "github.com/kubernetes/deployment-manager/pkg/httputil" ) -// Config holds the global configuration parameters passed into the router. -// -// Config is used concurrently. Once a config is created, it should be treated -// as immutable. -type Config struct { - // Address is the host and port (:8080) - Address string - // MaxTemplateLength is the maximum length of a template. - MaxTemplateLength int64 - // ExpanderName is the DNS name of the expansion service. - ExpanderName string - // ExpanderURL is the expander service's URL. - ExpanderURL string - // DeployerName is the deployer's DNS name - DeployerName string - // DeployerURL is the deployer's URL - DeployerURL string - // CredentialFile is the file to the credentials. - CredentialFile string - // CredentialSecrets tells the service to use a secrets file instead. - CredentialSecrets bool - // MongoName is the DNS name of the mongo server. - MongoName string - // MongoPort is the port for the MongoDB protocol on the mongo server. - // It is a string for historical reasons. - MongoPort string - // MongoAddress is the name and port. - MongoAddress string -} - -// Context contains dependencies that are passed to each handler function. -// -// Context carries typed information, often scoped to interfaces, so that the -// caller's contract with the service is known at compile time. -// -// Members of the context must be concurrency safe. -type Context struct { - Config *Config - // Manager is a deployment-manager/manager/manager.Manager - Manager manager.Manager - Encoder helmhttp.Encoder - CredentialProvider common.CredentialProvider -} - // HandlerFunc responds to an individual HTTP request. // // Returned errors will be captured, logged, and returned as HTTP 500 errors. @@ -93,28 +47,32 @@ type HandlerFunc func(w http.ResponseWriter, r *http.Request, c *Context) error type Handler struct { c *Context resolver *httputil.Resolver - routes Routes + routes map[string]HandlerFunc + paths []string } // NewHandler creates a new Handler. // // Routes cannot be modified after construction. The order that the route // names are returned by Routes.Paths() determines the lookup order. -func NewHandler(c *Context, r Routes) *Handler { - paths := make([]string, r.Len()) - i := 0 - for _, k := range r.Paths() { - paths[i] = k - i++ - } - +func NewHandler(c *Context) *Handler { return &Handler{ c: c, - resolver: httputil.NewResolver(paths), - routes: r, + resolver: httputil.NewResolver([]string{}), + routes: map[string]HandlerFunc{}, + paths: []string{}, } } +// Add a route to a handler. +// +// The route name is "VERB /ENPOINT/PATH", e.g. "GET /foo". +func (h *Handler) Add(route string, fn HandlerFunc) { + h.routes[route] = fn + h.paths = append(h.paths, route) + h.resolver = httputil.NewResolver(h.paths) +} + // ServeHTTP serves an HTTP request. func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { log.Printf(helmhttp.LogAccess, r.Method, r.URL) @@ -124,7 +82,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - fn, ok := h.routes.Get(route) + fn, ok := h.routes[route] if !ok { helmhttp.Fatal(w, r, "route %s missing", route) } @@ -133,43 +91,3 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { helmhttp.Fatal(w, r, err.Error()) } } - -// Routes defines a container for route-to-function mapping. -type Routes interface { - Add(string, HandlerFunc) - Get(string) (HandlerFunc, bool) - Len() int - Paths() []string -} - -// NewRoutes creates a default implementation of a Routes. -// -// The ordering of routes is nonderministic. -func NewRoutes() Routes { - return routeMap{} -} - -type routeMap map[string]HandlerFunc - -func (r routeMap) Add(name string, fn HandlerFunc) { - r[name] = fn -} - -func (r routeMap) Get(name string) (HandlerFunc, bool) { - f, ok := r[name] - return f, ok -} - -func (r routeMap) Len() int { - return len(r) -} - -func (r routeMap) Paths() []string { - b := make([]string, len(r)) - i := 0 - for k := range r { - b[i] = k - i++ - } - return b -} diff --git a/cmd/manager/router/router_test.go b/cmd/manager/router/router_test.go index df673ad04..d34057919 100644 --- a/cmd/manager/router/router_test.go +++ b/cmd/manager/router/router_test.go @@ -24,24 +24,19 @@ import ( "testing" ) -// Canary -var v Routes = routeMap{} - func TestHandler(t *testing.T) { c := &Context{} - r := NewRoutes() - r.Add("GET /", func(w http.ResponseWriter, r *http.Request, c *Context) error { + h := NewHandler(c) + h.Add("GET /", func(w http.ResponseWriter, r *http.Request, c *Context) error { fmt.Fprintln(w, "hello") return nil }) - r.Add("POST /", func(w http.ResponseWriter, r *http.Request, c *Context) error { + h.Add("POST /", func(w http.ResponseWriter, r *http.Request, c *Context) error { fmt.Fprintln(w, "goodbye") return nil }) - h := NewHandler(c, r) - s := httptest.NewServer(h) defer s.Close() @@ -59,14 +54,3 @@ func TestHandler(t *testing.T) { t.Errorf("Expected 'hello', got %q", data) } } - -// httpHarness is a simple test server fixture. -// Simple fixture for standing up a test server with a single route. -// -// You must Close() the returned server. -func httpHarness(c *Context, route string, fn HandlerFunc) *httptest.Server { - r := NewRoutes() - r.Add(route, fn) - h := NewHandler(c, r) - return httptest.NewServer(h) -}