bootcfg/http: Cleanup handler chains and source organization

* Simplify common http request handling chains
* Separate github.com/dghubble/ctxh source so we can move
it to a separate CoreOS package or vendor upstream
* Remove unused requireGET

bootcfg/http/ctxh.go

@@ -1,11 +1,8 @@
 package http
 
 import (
-	"net"
 	"net/http"
-	"strings"
 
-	"github.com/Sirupsen/logrus"
 	"golang.org/x/net/context"
 )
 
@@ -48,37 +45,3 @@ func NewHandler(h ContextHandler) http.Handler {
 func (h *handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	h.handler.ServeHTTP(h.ctx, w, req)
 }
-
-// labelsFromRequest returns request query parameters.
-func labelsFromRequest(logger *logrus.Logger, req *http.Request) map[string]string {
-	values := req.URL.Query()
-	labels := map[string]string{}
-	for key := range values {
-		switch strings.ToLower(key) {
-		case "mac":
-			// set mac if and only if it parses
-			if hw, err := parseMAC(values.Get(key)); err == nil {
-				labels[key] = hw.String()
-			} else {
-				if logger != nil {
-					logger.WithFields(logrus.Fields{
-						"mac": values.Get(key),
-					}).Warningf("ignoring unparseable MAC address: %v", err)
-				}
-			}
-		default:
-			// matchers don't use multi-value keys, drop later values
-			labels[key] = values.Get(key)
-		}
-	}
-	return labels
-}
-
-// parseMAC wraps net.ParseMAC with logging.
-func parseMAC(s string) (net.HardwareAddr, error) {
-	macAddr, err := net.ParseMAC(s)
-	if err != nil {
-		return nil, err
-	}
-	return macAddr, err
-}

bootcfg/http/ctxh_test.go

@@ -0,0 +1,22 @@
+package http
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"golang.org/x/net/context"
+)
+
+func TestNewHandler(t *testing.T) {
+	fn := func(ctx context.Context, w http.ResponseWriter, req *http.Request) {
+		fmt.Fprintf(w, "ContextHandler called")
+	}
+	h := NewHandler(ContextHandlerFunc(fn))
+	w := httptest.NewRecorder()
+	req, _ := http.NewRequest("GET", "/", nil)
+	h.ServeHTTP(w, req)
+	assert.Equal(t, "ContextHandler called", w.Body.String())
+}

bootcfg/http/handlers.go

@@ -10,22 +10,9 @@ import (
 	pb "github.com/coreos/coreos-baremetal/bootcfg/server/serverpb"
 )
 
-// requireGET requires requests to be an HTTP GET. Otherwise, it responds with
-// a 405 status code.
-func requireGET(next http.Handler) http.Handler {
-	fn := func(w http.ResponseWriter, req *http.Request) {
-		if req.Method != "GET" {
-			http.Error(w, "only HTTP GET is supported", http.StatusMethodNotAllowed)
-			return
-		}
-		next.ServeHTTP(w, req)
-	}
-	return http.HandlerFunc(fn)
-}
-
-// versionHandler shows the server name and version for root requests.
-// Otherwise, a 404 is returned.
-func versionHandler() http.Handler {
+// homeHandler shows the server name for rooted requests. Otherwise, a 404 is
+// returned.
+func homeHandler() http.Handler {
 	fn := func(w http.ResponseWriter, req *http.Request) {
 		if req.URL.Path != "/" {
 			http.NotFound(w, req)

bootcfg/http/handlers_test.go

@@ -15,30 +15,6 @@ import (
 	fake "github.com/coreos/coreos-baremetal/bootcfg/storage/testfakes"
 )
 
-func TestRequireGET(t *testing.T) {
-	next := func(w http.ResponseWriter, req *http.Request) {
-		fmt.Fprintf(w, "next")
-	}
-	h := requireGET(http.HandlerFunc(next))
-	req, _ := http.NewRequest("GET", "/", nil)
-	w := httptest.NewRecorder()
-	h.ServeHTTP(w, req)
-	assert.Equal(t, http.StatusOK, w.Code)
-	assert.Equal(t, "next", w.Body.String())
-}
-
-func TestRequireGET_WrongMethod(t *testing.T) {
-	next := func(w http.ResponseWriter, req *http.Request) {
-		fmt.Fprintf(w, "next")
-	}
-	h := requireGET(http.HandlerFunc(next))
-	req, _ := http.NewRequest("POST", "/", nil)
-	w := httptest.NewRecorder()
-	h.ServeHTTP(w, req)
-	assert.Equal(t, http.StatusMethodNotAllowed, w.Code)
-	assert.Equal(t, "only HTTP GET is supported\n", w.Body.String())
-}
-
 func TestSelectGroup(t *testing.T) {
 	store := &fake.FixedStore{
 		Groups: map[string]*storagepb.Group{fake.Group.Id: fake.Group},

bootcfg/http/ipxe.go

@@ -22,11 +22,11 @@ boot
 
 // ipxeInspect returns a handler that responds with the iPXE script to gather
 // client machine data and chainload to the ipxeHandler.
-func ipxeInspect() http.Handler {
-	fn := func(w http.ResponseWriter, req *http.Request) {
+func ipxeInspect() ContextHandler {
+	fn := func(ctx context.Context, w http.ResponseWriter, req *http.Request) {
 		fmt.Fprintf(w, ipxeBootstrap)
 	}
-	return http.HandlerFunc(fn)
+	return ContextHandlerFunc(fn)
 }
 
 // ipxeBoot returns a handler which renders the iPXE boot script for the

bootcfg/http/ipxe_test.go

@@ -17,7 +17,7 @@ func TestIPXEInspect(t *testing.T) {
 	h := ipxeInspect()
 	w := httptest.NewRecorder()
 	req, _ := http.NewRequest("GET", "/", nil)
-	h.ServeHTTP(w, req)
+	h.ServeHTTP(context.Background(), w, req)
 	assert.Equal(t, http.StatusOK, w.Code)
 	assert.Equal(t, ipxeBootstrap, w.Body.String())
 }

bootcfg/http/parse.go

@@ -0,0 +1,43 @@
+package http
+
+import (
+	"net"
+	"net/http"
+	"strings"
+
+	"github.com/Sirupsen/logrus"
+)
+
+// labelsFromRequest returns request query parameters.
+func labelsFromRequest(logger *logrus.Logger, req *http.Request) map[string]string {
+	values := req.URL.Query()
+	labels := map[string]string{}
+	for key := range values {
+		switch strings.ToLower(key) {
+		case "mac":
+			// set mac if and only if it parses
+			if hw, err := parseMAC(values.Get(key)); err == nil {
+				labels[key] = hw.String()
+			} else {
+				if logger != nil {
+					logger.WithFields(logrus.Fields{
+						"mac": values.Get(key),
+					}).Warningf("ignoring unparseable MAC address: %v", err)
+				}
+			}
+		default:
+			// matchers don't use multi-value keys, drop later values
+			labels[key] = values.Get(key)
+		}
+	}
+	return labels
+}
+
+// parseMAC wraps net.ParseMAC with logging.
+func parseMAC(s string) (net.HardwareAddr, error) {
+	macAddr, err := net.ParseMAC(s)
+	if err != nil {
+		return nil, err
+	}
+	return macAddr, err
+}

bootcfg/http/parse_test.go

@@ -1,26 +1,13 @@
 package http
 
 import (
-	"fmt"
 	"net/http"
-	"net/http/httptest"
 	"testing"
 
 	logtest "github.com/Sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/assert"
-	"golang.org/x/net/context"
 )
 
-func TestNewHandler(t *testing.T) {
-	fn := func(ctx context.Context, w http.ResponseWriter, req *http.Request) {
-		fmt.Fprintf(w, "ContextHandler called")
-	}
-	h := NewHandler(ContextHandlerFunc(fn))
-	w := httptest.NewRecorder()
-	req, _ := http.NewRequest("GET", "/", nil)
-	h.ServeHTTP(w, req)
-	assert.Equal(t, "ContextHandler called", w.Body.String())
-}
 
 func TestLabelsFromRequest(t *testing.T) {
 	emptyMap := map[string]string{}
@@ -45,4 +32,4 @@ func TestLabelsFromRequest(t *testing.T) {
 		assert.Nil(t, err)
 		assert.Equal(t, c.labels, labelsFromRequest(logger, req))
 	}
-}
+}

bootcfg/http/server.go

@@ -48,12 +48,12 @@ func (s *Server) HTTPHandler() http.Handler {
 		return s.logRequest(NewHandler(next))
 	}
 	// bootcfg version
-	mux.Handle("/", s.logRequest(versionHandler()))
+	mux.Handle("/", s.logRequest(homeHandler()))
 	// Boot via GRUB
 	mux.Handle("/grub", chain(s.selectProfile(s.core, s.grubHandler())))
 	// Boot via iPXE
-	mux.Handle("/boot.ipxe", s.logRequest(ipxeInspect()))
-	mux.Handle("/boot.ipxe.0", s.logRequest(ipxeInspect()))
+	mux.Handle("/boot.ipxe", chain(ipxeInspect()))
+	mux.Handle("/boot.ipxe.0", chain(ipxeInspect()))
 	mux.Handle("/ipxe", chain(s.selectProfile(s.core, s.ipxeHandler())))
 	// Boot via Pixiecore
 	mux.Handle("/pixiecore/v1/boot/", chain(s.pixiecoreHandler(s.core)))
@@ -68,32 +68,32 @@ func (s *Server) HTTPHandler() http.Handler {
 
 	// Signatures
 	if s.signer != nil {
-		signerChain := func(next http.Handler) http.Handler {
-			return s.logRequest(sign.SignatureHandler(s.signer, next))
+		signerChain := func(next ContextHandler) http.Handler {
+			return s.logRequest(sign.SignatureHandler(s.signer, NewHandler(next)))
 		}
-		mux.Handle("/grub.sig", signerChain(NewHandler(s.selectProfile(s.core, s.grubHandler()))))
+		mux.Handle("/grub.sig", signerChain(s.selectProfile(s.core, s.grubHandler())))
 		mux.Handle("/boot.ipxe.sig", signerChain(ipxeInspect()))
 		mux.Handle("/boot.ipxe.0.sig", signerChain(ipxeInspect()))
-		mux.Handle("/ipxe.sig", signerChain(NewHandler(s.selectProfile(s.core, s.ipxeHandler()))))
-		mux.Handle("/pixiecore/v1/boot.sig/", signerChain(NewHandler(s.pixiecoreHandler(s.core))))
-		mux.Handle("/ignition.sig", signerChain(NewHandler(s.selectGroup(s.core, s.ignitionHandler(s.core)))))
-		mux.Handle("/cloud.sig", signerChain(NewHandler(s.selectGroup(s.core, s.cloudHandler(s.core)))))
-		mux.Handle("/generic.sig", signerChain(NewHandler(s.selectGroup(s.core, s.genericHandler(s.core)))))
-		mux.Handle("/metadata.sig", signerChain(NewHandler(s.selectGroup(s.core, s.metadataHandler()))))
+		mux.Handle("/ipxe.sig", signerChain(s.selectProfile(s.core, s.ipxeHandler())))
+		mux.Handle("/pixiecore/v1/boot.sig/", signerChain(s.pixiecoreHandler(s.core)))
+		mux.Handle("/ignition.sig", signerChain(s.selectGroup(s.core, s.ignitionHandler(s.core))))
+		mux.Handle("/cloud.sig", signerChain(s.selectGroup(s.core, s.cloudHandler(s.core))))
+		mux.Handle("/generic.sig", signerChain(s.selectGroup(s.core, s.genericHandler(s.core))))
+		mux.Handle("/metadata.sig", signerChain(s.selectGroup(s.core, s.metadataHandler())))
 	}
 	if s.armoredSigner != nil {
-		signerChain := func(next http.Handler) http.Handler {
-			return s.logRequest(sign.SignatureHandler(s.armoredSigner, next))
+		signerChain := func(next ContextHandler) http.Handler {
+			return s.logRequest(sign.SignatureHandler(s.armoredSigner, NewHandler(next)))
 		}
-		mux.Handle("/grub.asc", signerChain(NewHandler(s.selectProfile(s.core, s.grubHandler()))))
+		mux.Handle("/grub.asc", signerChain(s.selectProfile(s.core, s.grubHandler())))
 		mux.Handle("/boot.ipxe.asc", signerChain(ipxeInspect()))
 		mux.Handle("/boot.ipxe.0.asc", signerChain(ipxeInspect()))
-		mux.Handle("/ipxe.asc", signerChain(NewHandler(s.selectProfile(s.core, s.ipxeHandler()))))
-		mux.Handle("/pixiecore/v1/boot.asc/", signerChain(NewHandler(s.pixiecoreHandler(s.core))))
-		mux.Handle("/ignition.asc", signerChain(NewHandler(s.selectGroup(s.core, s.ignitionHandler(s.core)))))
-		mux.Handle("/cloud.asc", signerChain(NewHandler(s.selectGroup(s.core, s.cloudHandler(s.core)))))
-		mux.Handle("/generic.asc", signerChain(NewHandler(s.selectGroup(s.core, s.genericHandler(s.core)))))
-		mux.Handle("/metadata.asc", signerChain(NewHandler(s.selectGroup(s.core, s.metadataHandler()))))
+		mux.Handle("/ipxe.asc", signerChain(s.selectProfile(s.core, s.ipxeHandler())))
+		mux.Handle("/pixiecore/v1/boot.asc/", signerChain(s.pixiecoreHandler(s.core)))
+		mux.Handle("/ignition.asc", signerChain(s.selectGroup(s.core, s.ignitionHandler(s.core))))
+		mux.Handle("/cloud.asc", signerChain(s.selectGroup(s.core, s.cloudHandler(s.core))))
+		mux.Handle("/generic.asc", signerChain(s.selectGroup(s.core, s.genericHandler(s.core))))
+		mux.Handle("/metadata.asc", signerChain(s.selectGroup(s.core, s.metadataHandler())))
 	}
 
 	// kernel, initrd, and TLS assets