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

@@ -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 {

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)
 }
 

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)
 	}

cmd/manager/router/context.go

+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
+}

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
-}

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)
-}