api: Convert handlers to ContextHandlers to isolate responsibility

* Add SpecMatcherHandler to match machine requests to Specs

api/cloud.go

@@ -4,6 +4,8 @@ import (
 	"net/http"
 	"strings"
 	"time"
+
+	"golang.org/x/net/context"
 )
 
 // CloudConfig defines a cloud-init config.
@@ -13,15 +15,13 @@ type CloudConfig struct {
 
 // cloudHandler returns a handler that responds with the cloud config for the
 // requester.
-func cloudHandler(store Store) http.Handler {
-	fn := func(w http.ResponseWriter, req *http.Request) {
-		attrs := labelsFromRequest(req)
-		spec, err := getMatchingSpec(store, attrs)
+func cloudHandler(store Store) ContextHandler {
+	fn := func(ctx context.Context, w http.ResponseWriter, req *http.Request) {
+		spec, err := specFromContext(ctx)
 		if err != nil || spec.CloudConfig == "" {
 			http.NotFound(w, req)
 			return
 		}
-
 		config, err := store.CloudConfig(spec.CloudConfig)
 		if err != nil {
 			http.NotFound(w, req)
@@ -29,5 +29,5 @@ func cloudHandler(store Store) http.Handler {
 		}
 		http.ServeContent(w, req, "", time.Time{}, strings.NewReader(config.Content))
 	}
-	return http.HandlerFunc(fn)
+	return ContextHandlerFunc(fn)
 }

api/cloud_test.go

@@ -6,45 +6,38 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"golang.org/x/net/context"
 )
 
 func TestCloudHandler(t *testing.T) {
-	cloudcfg := &CloudConfig{
-		Content: "#cloud-config",
-	}
+	cloudcfg := &CloudConfig{Content: "#cloud-config"}
 	store := &fixedStore{
-		Groups:       []Group{testGroup},
-		Specs:        map[string]*Spec{testGroup.Spec: testSpec},
 		CloudConfigs: map[string]*CloudConfig{testSpec.CloudConfig: cloudcfg},
 	}
 	h := cloudHandler(store)
-	req, _ := http.NewRequest("GET", "?uuid=a1b2c3d4", nil)
+	ctx := withSpec(context.Background(), testSpec)
 	w := httptest.NewRecorder()
-	h.ServeHTTP(w, req)
+	req, _ := http.NewRequest("GET", "/", nil)
+	h.ServeHTTP(ctx, w, req)
 	// assert that:
-	// - match parameters to a Spec
-	// - render the Spec's cloud config
+	// - the Spec's cloud config is served
 	assert.Equal(t, http.StatusOK, w.Code)
 	assert.Equal(t, cloudcfg.Content, w.Body.String())
 }
 
-func TestCloudHandler_NoMatchingSpec(t *testing.T) {
-	store := &emptyStore{}
-	h := cloudHandler(store)
-	req, _ := http.NewRequest("GET", "?uuid=a1b2c3d4", nil)
+func TestCloudHandler_MissingCtxSpec(t *testing.T) {
+	h := cloudHandler(&emptyStore{})
 	w := httptest.NewRecorder()
-	h.ServeHTTP(w, req)
+	req, _ := http.NewRequest("GET", "/", nil)
+	h.ServeHTTP(context.Background(), w, req)
 	assert.Equal(t, http.StatusNotFound, w.Code)
 }
 
 func TestCloudHandler_MissingCloudConfig(t *testing.T) {
-	store := &fixedStore{
-		Groups: []Group{testGroup},
-		Specs:  map[string]*Spec{testGroup.Spec: testSpec},
-	}
-	h := cloudHandler(store)
-	req, _ := http.NewRequest("GET", "?uuid=a1b2c3d4", nil)
+	h := cloudHandler(&emptyStore{})
+	ctx := withSpec(context.Background(), testSpec)
 	w := httptest.NewRecorder()
-	h.ServeHTTP(w, req)
+	req, _ := http.NewRequest("GET", "/", nil)
+	h.ServeHTTP(ctx, w, req)
 	assert.Equal(t, http.StatusNotFound, w.Code)
 }

api/context.go

@@ -0,0 +1,32 @@
+package api
+
+import (
+	"errors"
+
+	"golang.org/x/net/context"
+)
+
+// unexported key prevents collisions
+type key int
+
+const (
+	specKey key = iota
+)
+
+var (
+	errNoSpecFromContext = errors.New("api: Context missing a Spec")
+)
+
+// withSpec returns a copy of ctx that stores the given Spec.
+func withSpec(ctx context.Context, spec *Spec) context.Context {
+	return context.WithValue(ctx, specKey, spec)
+}
+
+// specFromContext returns the Spec from the ctx.
+func specFromContext(ctx context.Context) (*Spec, error) {
+	spec, ok := ctx.Value(specKey).(*Spec)
+	if !ok {
+		return nil, errNoSpecFromContext
+	}
+	return spec, nil
+}

api/groups.go

@@ -2,7 +2,10 @@ package api
 
 import (
 	"fmt"
+	"net/http"
 	"sort"
+
+	"golang.org/x/net/context"
 )
 
 // Group associates matcher conditions with a Specification identifier. The
@@ -48,15 +51,36 @@ func newGroupsResource(store Store) *groupsResource {
 }
 
 // listGroups lists all Group resources.
-func (r *groupsResource) listGroups() ([]Group, error) {
-	return r.store.ListGroups()
+func (gr *groupsResource) listGroups() ([]Group, error) {
+	return gr.store.ListGroups()
+}
+
+// matchSpecHandler returns a ContextHandler that matches machine requests
+// to a Spec and adds the Spec to the ctx and calls the next handler. The
+// next handler should handle the case that no matching Spec is found.
+func (gr *groupsResource) matchSpecHandler(next ContextHandler) ContextHandler {
+	fn := func(ctx context.Context, w http.ResponseWriter, req *http.Request) {
+		attrs := labelsFromRequest(req)
+		// match machine request
+		group, err := gr.findMatch(attrs)
+		if err == nil {
+			// lookup Spec by id
+			spec, err := gr.store.Spec(group.Spec)
+			if err == nil {
+				// add the Spec to the ctx for next handler
+				ctx = withSpec(ctx, spec)
+			}
+		}
+		next.ServeHTTP(ctx, w, req)
+	}
+	return ContextHandlerFunc(fn)
 }
 
 // findMatch returns the first Group whose Matcher is satisfied by the given
 // labels. Groups are attempted in sorted order, preferring those with
 // more matcher conditions, alphabetically.
-func (r *groupsResource) findMatch(labels Labels) (*Group, error) {
-	groups, err := r.store.ListGroups()
+func (gr *groupsResource) findMatch(labels Labels) (*Group, error) {
+	groups, err := gr.store.ListGroups()
 	if err != nil {
 		return nil, err
 	}

api/ignition.go

@@ -2,19 +2,19 @@ package api
 
 import (
 	"net/http"
+
+	"golang.org/x/net/context"
 )
 
 // ignitionHandler returns a handler that responds with the ignition config
 // for the requester.
-func ignitionHandler(store Store) http.Handler {
-	fn := func(w http.ResponseWriter, req *http.Request) {
-		attrs := labelsFromRequest(req)
-		spec, err := getMatchingSpec(store, attrs)
+func ignitionHandler(store Store) ContextHandler {
+	fn := func(ctx context.Context, w http.ResponseWriter, req *http.Request) {
+		spec, err := specFromContext(ctx)
 		if err != nil || spec.IgnitionConfig == "" {
 			http.NotFound(w, req)
 			return
 		}
-
 		config, err := store.IgnitionConfig(spec.IgnitionConfig)
 		if err != nil {
 			http.NotFound(w, req)
@@ -22,5 +22,5 @@ func ignitionHandler(store Store) http.Handler {
 		}
 		renderJSON(w, config)
 	}
-	return http.HandlerFunc(fn)
+	return ContextHandlerFunc(fn)
 }

api/ignition_test.go

@@ -7,45 +7,40 @@ import (
 
 	ignition "github.com/coreos/ignition/src/config"
 	"github.com/stretchr/testify/assert"
+	"golang.org/x/net/context"
 )
 
 func TestIgnitionHandler(t *testing.T) {
 	ignitioncfg := &ignition.Config{}
 	store := &fixedStore{
-		Groups:          []Group{testGroup},
-		Specs:           map[string]*Spec{testGroup.Spec: testSpec},
 		IgnitionConfigs: map[string]*ignition.Config{testSpec.IgnitionConfig: ignitioncfg},
 	}
 	h := ignitionHandler(store)
-	req, _ := http.NewRequest("GET", "?uuid=a1b2c3d4", nil)
+	ctx := withSpec(context.Background(), testSpec)
 	w := httptest.NewRecorder()
-	h.ServeHTTP(w, req)
+	req, _ := http.NewRequest("GET", "/", nil)
+	h.ServeHTTP(ctx, w, req)
 	// assert that:
-	// - match parameters to a Spec
-	// - render the Spec's ignition config
+	// - the Spec's ignition config is rendered
 	expectedJSON := `{"ignitionVersion":0,"storage":{},"systemd":{},"networkd":{},"passwd":{}}`
 	assert.Equal(t, http.StatusOK, w.Code)
 	assert.Equal(t, jsonContentType, w.HeaderMap.Get(contentType))
 	assert.Equal(t, expectedJSON, w.Body.String())
 }
 
-func TestIgnitionHandler_NoMatchingSpec(t *testing.T) {
-	store := &emptyStore{}
-	h := ignitionHandler(store)
-	req, _ := http.NewRequest("GET", "?uuid=a1b2c3d4", nil)
+func TestIgnitionHandler_MissingCtxSpec(t *testing.T) {
+	h := ignitionHandler(&emptyStore{})
 	w := httptest.NewRecorder()
-	h.ServeHTTP(w, req)
+	req, _ := http.NewRequest("GET", "/", nil)
+	h.ServeHTTP(context.Background(), w, req)
 	assert.Equal(t, http.StatusNotFound, w.Code)
 }
 
 func TestIgnitionHandler_MissingIgnitionConfig(t *testing.T) {
-	store := &fixedStore{
-		Groups: []Group{testGroup},
-		Specs:  map[string]*Spec{testGroup.Spec: testSpec},
-	}
-	h := ignitionHandler(store)
-	req, _ := http.NewRequest("GET", "?uuid=a1b2c3d4", nil)
+	h := ignitionHandler(&emptyStore{})
+	ctx := withSpec(context.Background(), testSpec)
 	w := httptest.NewRecorder()
-	h.ServeHTTP(w, req)
+	req, _ := http.NewRequest("GET", "/", nil)
+	h.ServeHTTP(ctx, w, req)
 	assert.Equal(t, http.StatusNotFound, w.Code)
 }

api/ipxe.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"net/http"
 	"text/template"
+
+	"golang.org/x/net/context"
 )
 
 const ipxeBootstrap = `#!ipxe
@@ -28,15 +30,13 @@ func ipxeInspect() http.Handler {
 
 // ipxeBoot returns a handler which renders the iPXE boot script for the
 // requester.
-func ipxeHandler(store Store) http.Handler {
-	fn := func(w http.ResponseWriter, req *http.Request) {
-		attrs := labelsFromRequest(req)
-		spec, err := getMatchingSpec(store, attrs)
+func ipxeHandler() ContextHandler {
+	fn := func(ctx context.Context, w http.ResponseWriter, req *http.Request) {
+		spec, err := specFromContext(ctx)
 		if err != nil {
 			http.NotFound(w, req)
 			return
 		}
-
 		var buf bytes.Buffer
 		err = ipxeTemplate.Execute(&buf, spec.BootConfig)
 		if err != nil {
@@ -49,5 +49,5 @@ func ipxeHandler(store Store) http.Handler {
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 	}
-	return http.HandlerFunc(fn)
+	return ContextHandlerFunc(fn)
 }

api/ipxe_test.go

@@ -6,28 +6,26 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"golang.org/x/net/context"
 )
 
 func TestIPXEInspect(t *testing.T) {
 	h := ipxeInspect()
-	req, _ := http.NewRequest("GET", "/", nil)
 	w := httptest.NewRecorder()
+	req, _ := http.NewRequest("GET", "/", nil)
 	h.ServeHTTP(w, req)
 	assert.Equal(t, http.StatusOK, w.Code)
 	assert.Equal(t, ipxeBootstrap, w.Body.String())
 }
 
 func TestIPXEHandler(t *testing.T) {
-	store := &fixedStore{
-		Groups: []Group{testGroup},
-		Specs:  map[string]*Spec{testGroup.Spec: testSpec},
-	}
-	h := ipxeHandler(store)
-	req, _ := http.NewRequest("GET", "?uuid=a1b2c3d4", nil)
+	h := ipxeHandler()
+	ctx := withSpec(context.Background(), testSpec)
 	w := httptest.NewRecorder()
-	h.ServeHTTP(w, req)
+	req, _ := http.NewRequest("GET", "/", nil)
+	h.ServeHTTP(ctx, w, req)
 	// assert that:
-	// - boot config is rendered as an iPXE script
+	// - the Spec's boot config is rendered as an iPXE script
 	expectedScript := `#!ipxe
 kernel /image/kernel a=b c
 initrd /image/initrd_a /image/initrd_b 
@@ -37,37 +35,30 @@ boot
 	assert.Equal(t, expectedScript, w.Body.String())
 }
 
-func TestIPXEHandler_NoMatchingSpec(t *testing.T) {
-	store := &emptyStore{}
-	h := ipxeHandler(store)
-	req, _ := http.NewRequest("GET", "?uuid=a1b2c3d4", nil)
+func TestIPXEHandler_MissingCtxSpec(t *testing.T) {
+	h := ipxeHandler()
 	w := httptest.NewRecorder()
-	h.ServeHTTP(w, req)
+	req, _ := http.NewRequest("GET", "/", nil)
+	h.ServeHTTP(context.Background(), w, req)
 	assert.Equal(t, http.StatusNotFound, w.Code)
 }
 
 func TestIPXEHandler_RenderTemplateError(t *testing.T) {
-	// nil BootConfig forces a template.Execute error
-	store := &fixedStore{
-		Groups: []Group{testGroup},
-		Specs:  map[string]*Spec{testGroup.Spec: &Spec{BootConfig: nil}},
-	}
-	h := ipxeHandler(store)
-	req, _ := http.NewRequest("GET", "/?uuid=a1b2c3d4", nil)
+	h := ipxeHandler()
+	// a Spec with nil BootConfig forces a template.Execute error
+	ctx := withSpec(context.Background(), &Spec{BootConfig: nil})
 	w := httptest.NewRecorder()
-	h.ServeHTTP(w, req)
+	req, _ := http.NewRequest("GET", "/", nil)
+	h.ServeHTTP(ctx, w, req)
 	assert.Equal(t, http.StatusNotFound, w.Code)
 }
 
 func TestIPXEHandler_WriteError(t *testing.T) {
-	store := &fixedStore{
-		Groups: []Group{testGroup},
-		Specs:  map[string]*Spec{testGroup.Spec: testSpec},
-	}
-	h := ipxeHandler(store)
-	req, _ := http.NewRequest("GET", "/?uuid=a1b2c3d4", nil)
+	h := ipxeHandler()
+	ctx := withSpec(context.Background(), testSpec)
 	w := NewUnwriteableResponseWriter()
-	h.ServeHTTP(w, req)
+	req, _ := http.NewRequest("GET", "/", nil)
+	h.ServeHTTP(ctx, w, req)
 	assert.Equal(t, http.StatusInternalServerError, w.Code)
 	assert.Empty(t, w.Body.String())
 }

api/pixiecore.go

@@ -8,7 +8,7 @@ import (
 // pixiecoreHandler returns a handler that renders the boot config JSON for
 // the requester, to implement the Pixiecore API specification.
 // https://github.com/danderson/pixiecore/blob/master/README.api.md
-func pixiecoreHandler(store Store) http.Handler {
+func pixiecoreHandler(gr *groupsResource, store Store) http.Handler {
 	fn := func(w http.ResponseWriter, req *http.Request) {
 		macAddr, err := parseMAC(filepath.Base(req.URL.Path))
 		if err != nil {
@@ -17,7 +17,12 @@ func pixiecoreHandler(store Store) http.Handler {
 		}
 		// pixiecore only provides MAC addresses
 		attrs := LabelSet(map[string]string{"mac": macAddr.String()})
-		spec, err := getMatchingSpec(store, attrs)
+		group, err := gr.findMatch(attrs)
+		if err != nil {
+			http.NotFound(w, req)
+			return
+		}
+		spec, err := store.Spec(group.Spec)
 		if err != nil {
 			http.NotFound(w, req)
 			return

api/pixiecore_test.go

@@ -13,12 +13,13 @@ func TestPixiecoreHandler(t *testing.T) {
 		Groups: []Group{testGroupWithMAC},
 		Specs:  map[string]*Spec{testGroupWithMAC.Spec: testSpec},
 	}
-	h := pixiecoreHandler(store)
-	req, _ := http.NewRequest("GET", "/"+validMACStr, nil)
+	h := pixiecoreHandler(newGroupsResource(store), store)
 	w := httptest.NewRecorder()
+	req, _ := http.NewRequest("GET", "/"+validMACStr, nil)
 	h.ServeHTTP(w, req)
 	// assert that:
-	// - boot config is rendered as Pixiecore JSON
+	// - MAC address argument is used for Spec matching
+	// - the Spec's boot config is rendered as Pixiecore JSON
 	expectedJSON := `{"kernel":"/image/kernel","initrd":["/image/initrd_a","/image/initrd_b"],"cmdline":{"a":"b","c":""}}`
 	assert.Equal(t, http.StatusOK, w.Code)
 	assert.Equal(t, jsonContentType, w.HeaderMap.Get(contentType))
@@ -26,20 +27,21 @@ func TestPixiecoreHandler(t *testing.T) {
 }
 
 func TestPixiecoreHandler_InvalidMACAddress(t *testing.T) {
-	store := &emptyStore{}
-	h := pixiecoreHandler(store)
-	req, _ := http.NewRequest("GET", "/", nil)
+	h := pixiecoreHandler(&groupsResource{}, &emptyStore{})
 	w := httptest.NewRecorder()
+	req, _ := http.NewRequest("GET", "/", nil)
 	h.ServeHTTP(w, req)
 	assert.Equal(t, http.StatusBadRequest, w.Code)
 	assert.Equal(t, "invalid MAC address /\n", w.Body.String())
 }
 
 func TestPixiecoreHandler_NoMatchingSpec(t *testing.T) {
-	store := &emptyStore{}
-	h := pixiecoreHandler(store)
-	req, _ := http.NewRequest("GET", "/"+validMACStr, nil)
+	store := &fixedStore{
+		Groups: []Group{testGroupWithMAC},
+	}
+	h := pixiecoreHandler(newGroupsResource(store), &emptyStore{})
 	w := httptest.NewRecorder()
+	req, _ := http.NewRequest("GET", "/"+validMACStr, nil)
 	h.ServeHTTP(w, req)
 	assert.Equal(t, http.StatusNotFound, w.Code)
 }

api/server.go

@@ -38,20 +38,22 @@ func NewServer(config *Config) *Server {
 // HTTPHandler returns a HTTP handler for the server.
 func (s *Server) HTTPHandler() http.Handler {
 	mux := http.NewServeMux()
-	// iPXE
+	// API Resources
+	newSpecResource(mux, "/spec/", s.store)
+	gr := newGroupsResource(s.store)
+
+	// Endpoints
+	// Boot via iPXE
 	mux.Handle("/boot.ipxe", logRequests(ipxeInspect()))
 	mux.Handle("/boot.ipxe.0", logRequests(ipxeInspect()))
-	mux.Handle("/ipxe", logRequests(ipxeHandler(s.store)))
-	// Pixiecore
-	mux.Handle("/pixiecore/v1/boot/", logRequests(pixiecoreHandler(s.store)))
+	mux.Handle("/ipxe", logRequests(NewHandler(gr.matchSpecHandler(ipxeHandler()))))
+	// Boot via Pixiecore
+	mux.Handle("/pixiecore/v1/boot/", logRequests(pixiecoreHandler(gr, s.store)))
 	// cloud configs
-	mux.Handle("/cloud", logRequests(cloudHandler(s.store)))
+	mux.Handle("/cloud", logRequests(NewHandler(gr.matchSpecHandler(cloudHandler(s.store)))))
 	// ignition configs
-	mux.Handle("/ignition", logRequests(ignitionHandler(s.store)))
+	mux.Handle("/ignition", logRequests(NewHandler(gr.matchSpecHandler(ignitionHandler(s.store)))))
 
-	// API Resources
-	// specs
-	newSpecResource(mux, "/spec/", s.store)
 	// kernel, initrd, and TLS assets
 	mux.Handle("/assets/", http.StripPrefix("/assets/", http.FileServer(http.Dir(s.assetsPath))))
 	return mux

api/spec.go

@@ -38,13 +38,3 @@ func (r *specResource) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	}
 	renderJSON(w, spec)
 }
-
-// getMatchingSpec returns the Spec matching the given attributes.
-func getMatchingSpec(store Store, labels Labels) (*Spec, error) {
-	groups := newGroupsResource(store)
-	group, err := groups.findMatch(labels)
-	if err != nil {
-		return nil, err
-	}
-	return store.Spec(group.Spec)
-}