pkg/server: Trim public API to return *http.Server

cmd/machine-config-server/bootstrap.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"flag"
+	"fmt"
 
 	"github.com/golang/glog"
 	"github.com/openshift/machine-config-operator/pkg/server"
@@ -37,18 +38,18 @@ func runBootstrapCmd(cmd *cobra.Command, args []string) {
 	glog.Infof("Version: %+v", version.Version)
 
 	bs, err := server.NewBootstrapServer(bootstrapOpts.serverBaseDir, bootstrapOpts.serverKubeConfig)
-
 	if err != nil {
 		glog.Exitf("Machine Config Server exited with error: %v", err)
 	}
 
-	apiHandler := server.NewServerAPIHandler(bs)
-	secureServer := server.NewAPIServer(apiHandler, rootOpts.sport, false, rootOpts.cert, rootOpts.key)
-	insecureServer := server.NewAPIServer(apiHandler, rootOpts.isport, true, "", "")
+	secureServer := *bs
+	secureServer.Addr = fmt.Sprintf(":%d", rootOpts.sport)
+	insecureServer := *bs
+	insecureServer.Addr = fmt.Sprintf(":%d", rootOpts.isport)
 
 	stopCh := make(chan struct{})
-	go secureServer.Serve()
-	go insecureServer.Serve()
+	go secureServer.ListenAndServeTLS(rootOpts.cert, rootOpts.key)
+	go insecureServer.ListenAndServe()
 	<-stopCh
 	panic("not possible")
 }

cmd/machine-config-server/start.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"flag"
+	"fmt"
 
 	"github.com/golang/glog"
 	"github.com/openshift/machine-config-operator/pkg/server"
@@ -45,13 +46,14 @@ func runStartCmd(cmd *cobra.Command, args []string) {
 		glog.Exitf("Machine Config Server exited with error: %v", err)
 	}
 
-	apiHandler := server.NewServerAPIHandler(cs)
-	secureServer := server.NewAPIServer(apiHandler, rootOpts.sport, false, rootOpts.cert, rootOpts.key)
-	insecureServer := server.NewAPIServer(apiHandler, rootOpts.isport, true, "", "")
+	secureServer := *cs
+	secureServer.Addr = fmt.Sprintf(":%d", rootOpts.sport)
+	insecureServer := *cs
+	insecureServer.Addr = fmt.Sprintf(":%d", rootOpts.isport)
 
 	stopCh := make(chan struct{})
-	go secureServer.Serve()
-	go insecureServer.Serve()
+	go secureServer.ListenAndServeTLS(rootOpts.cert, rootOpts.key)
+	go insecureServer.ListenAndServe()
 	<-stopCh
 	panic("not possible")
 }

pkg/server/api.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"path"
 
+	ignv2_2types "github.com/coreos/ignition/config/v2_2/types"
 	"github.com/golang/glog"
 )
 
@@ -17,69 +18,23 @@ type poolRequest struct {
 	machinePool string
 }
 
-// APIServer provides the HTTP(s) endpoint
-// for providing the machine configs.
-type APIServer struct {
-	handler  *APIHandler
-	port     int
-	insecure bool
-	cert     string
-	key      string
-}
-
-// NewAPIServer initializes a new API server
-// that runs the Machine Config Server as a
-// handler.
-func NewAPIServer(a *APIHandler, p int, is bool, c, k string) *APIServer {
-	return &APIServer{
-		handler:  a,
-		port:     p,
-		insecure: is,
-		cert:     c,
-		key:      k,
-	}
-}
+type getConfig func(request poolRequest) (*ignv2_2types.Config, error)
 
-// Serve launches the API Server.
-func (a *APIServer) Serve() {
+func newHandler(getConfig getConfig) http.Handler {
 	mux := http.NewServeMux()
-	mux.Handle(apiPathConfig, a.handler)
-
-	mcs := &http.Server{
-		Addr:    fmt.Sprintf(":%v", a.port),
-		Handler: mux,
-	}
-
-	glog.Info("launching server")
-	if a.insecure {
-		// Serve a non TLS server.
-		if err := mcs.ListenAndServe(); err != http.ErrServerClosed {
-			glog.Exitf("Machine Config Server exited with error: %v", err)
-		}
-	} else {
-		if err := mcs.ListenAndServeTLS(a.cert, a.key); err != http.ErrServerClosed {
-			glog.Exitf("Machine Config Server exited with error: %v", err)
-		}
-	}
-}
-
-// APIHandler is the HTTP Handler for the
-// Machine Config Server.
-type APIHandler struct {
-	server Server
+	mux.Handle(apiPathConfig, &configHandler{getConfig: getConfig})
+	mux.Handle("/", &defaultHandler{})
+	return mux
 }
 
-// NewServerAPIHandler initializes a new API handler
-// for the Machine Config Server.
-func NewServerAPIHandler(s Server) *APIHandler {
-	return &APIHandler{
-		server: s,
-	}
+// configHandler is the HTTP Handler for machine configuration.
+type configHandler struct {
+	getConfig getConfig
 }
 
 // ServeHTTP handles the requests for the machine config server
 // API handler.
-func (sh *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+func (h *configHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if r.Method != http.MethodGet && r.Method != http.MethodHead {
 		w.Header().Set("Content-Length", "0")
 		w.WriteHeader(http.StatusMethodNotAllowed)
@@ -96,7 +51,7 @@ func (sh *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		machinePool: path.Base(r.URL.Path),
 	}
 
-	conf, err := sh.server.GetConfig(cr)
+	conf, err := h.getConfig(cr)
 	if err != nil {
 		w.Header().Set("Content-Length", "0")
 		w.WriteHeader(http.StatusInternalServerError)
@@ -129,3 +84,13 @@ func (sh *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		glog.Errorf("failed to write %v response: %v", cr, err)
 	}
 }
+
+// defaultHandler is the HTTP Handler for backstopping invalid requests.
+type defaultHandler struct{}
+
+// ServeHTTP handles invalid requests.
+func (h *defaultHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	w.Header().Set("Content-Length", "0")
+	w.WriteHeader(http.StatusNotFound)
+	return
+}

pkg/server/api_test.go

@@ -23,16 +23,16 @@ type checkResponse func(t *testing.T, response *http.Response)
 type scenario struct {
 	name          string
 	request       *http.Request
-	serverFunc    func(poolRequest) (*ignv2_2types.Config, error)
+	getConfig     getConfig
 	checkResponse checkResponse
 }
 
-func TestAPIHandler(t *testing.T) {
+func TestHandler(t *testing.T) {
 	scenarios := []scenario{
 		{
 			name:    "get non-config path that does not exist",
 			request: httptest.NewRequest(http.MethodGet, "http://testrequest/does-not-exist", nil),
-			serverFunc: func(poolRequest) (*ignv2_2types.Config, error) {
+			getConfig: func(poolRequest) (*ignv2_2types.Config, error) {
 				return nil, nil
 			},
 			checkResponse: func(t *testing.T, response *http.Response) {
@@ -44,7 +44,7 @@ func TestAPIHandler(t *testing.T) {
 		{
 			name:    "get config path that does not exist",
 			request: httptest.NewRequest(http.MethodGet, "http://testrequest/config/does-not-exist", nil),
-			serverFunc: func(poolRequest) (*ignv2_2types.Config, error) {
+			getConfig: func(poolRequest) (*ignv2_2types.Config, error) {
 				return new(ignv2_2types.Config), fmt.Errorf("not acceptable")
 			},
 			checkResponse: func(t *testing.T, response *http.Response) {
@@ -56,7 +56,7 @@ func TestAPIHandler(t *testing.T) {
 		{
 			name:    "get config path that exists",
 			request: httptest.NewRequest(http.MethodGet, "http://testrequest/config/master", nil),
-			serverFunc: func(poolRequest) (*ignv2_2types.Config, error) {
+			getConfig: func(poolRequest) (*ignv2_2types.Config, error) {
 				return new(ignv2_2types.Config), nil
 			},
 			checkResponse: func(t *testing.T, response *http.Response) {
@@ -69,7 +69,7 @@ func TestAPIHandler(t *testing.T) {
 		{
 			name:    "head config path that exists",
 			request: httptest.NewRequest(http.MethodHead, "http://testrequest/config/master", nil),
-			serverFunc: func(poolRequest) (*ignv2_2types.Config, error) {
+			getConfig: func(poolRequest) (*ignv2_2types.Config, error) {
 				return new(ignv2_2types.Config), nil
 			},
 			checkResponse: func(t *testing.T, response *http.Response) {
@@ -82,20 +82,20 @@ func TestAPIHandler(t *testing.T) {
 		{
 			name:    "post non-config path that does not exist",
 			request: httptest.NewRequest(http.MethodPost, "http://testrequest/post", nil),
-			serverFunc: func(poolRequest) (*ignv2_2types.Config, error) {
+			getConfig: func(poolRequest) (*ignv2_2types.Config, error) {
 				return nil, nil
 			},
 			checkResponse: func(t *testing.T, response *http.Response) {
-				checkStatus(t, response, http.StatusMethodNotAllowed)
+				checkStatus(t, response, http.StatusNotFound)
 				checkContentLength(t, response, 0)
 				checkBodyLength(t, response, 0)
 			},
 		},
 		{
 			name:    "post config path that exists",
 			request: httptest.NewRequest(http.MethodPost, "http://testrequest/config/master", nil),
-			serverFunc: func(poolRequest) (*ignv2_2types.Config, error) {
-				return new(ignv2_2types.Config), nil
+			getConfig: func(poolRequest) (*ignv2_2types.Config, error) {
+				return nil, nil
 			},
 			checkResponse: func(t *testing.T, response *http.Response) {
 				checkStatus(t, response, http.StatusMethodNotAllowed)
@@ -108,10 +108,7 @@ func TestAPIHandler(t *testing.T) {
 	for _, scenario := range scenarios {
 		t.Run(scenario.name, func(t *testing.T) {
 			w := httptest.NewRecorder()
-			ms := &mockServer{
-				GetConfigFn: scenario.serverFunc,
-			}
-			handler := NewServerAPIHandler(ms)
+			handler := newHandler(scenario.getConfig)
 			handler.ServeHTTP(w, scenario.request)
 
 			resp := w.Result()

pkg/server/bootstrap_server.go

@@ -3,6 +3,7 @@ package server
 import (
 	"fmt"
 	"io/ioutil"
+	"net/http"
 	"os"
 	"path"
 
@@ -14,52 +15,52 @@ import (
 	"github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
 )
 
-// ensure bootstrapServer implements the
-// Server interface.
-var _ = Server(&bootstrapServer{})
+type bootstrapConfig struct {
 
-type bootstrapServer struct {
-
-	// serverBaseDir is the root, relative to which
+	// configBaseDir is the root, relative to which
 	// the machine pool, configs will be picked
-	serverBaseDir string
+	configBaseDir string
 
 	kubeconfigFunc kubeconfigFunc
 }
 
-// NewBootstrapServer initializes a new Bootstrap server that implements
-// the Server interface.
-func NewBootstrapServer(dir, kubeconfig string) (Server, error) {
+// NewBootstrapServer initializes a new Server that loads
+// its configuration from the local filesystem.
+func NewBootstrapServer(dir, kubeconfig string) (*http.Server, error) {
 	if _, err := os.Stat(kubeconfig); err != nil {
 		return nil, fmt.Errorf("kubeconfig not found at location: %s", kubeconfig)
 	}
-	return &bootstrapServer{
-		serverBaseDir:  dir,
+	config := &bootstrapConfig{
+		configBaseDir:  dir,
 		kubeconfigFunc: func() ([]byte, []byte, error) { return kubeconfigFromFile(kubeconfig) },
+	}
+
+	return &http.Server{
+		Handler: newHandler(config.getConfig),
 	}, nil
 }
 
-// GetConfig fetches the machine config(type - Ignition) from the bootstrap server,
+// getConfig fetches the machine config(type - Ignition) from the bootstrap server,
 // based on the pool request.
 // It returns nil for conf, error if the config isn't found. It returns a formatted
 // error if any other error is encountered during its operations.
 //
 // The method does the following:
 //
 // 1. Read the machine config pool by using the following path template:
-// 		"<serverBaseDir>/machine-pools/<machineConfigPoolName>.yaml"
+// 		"<configBaseDir>/machine-pools/<machineConfigPoolName>.yaml"
 //
 // 2. Read the currentConfig field from the Status and read the config file
 //    using the following path template:
-// 		"<serverBaseDir>/machine-configs/<currentConfig>.yaml"
+// 		"<configBaseDir>/machine-configs/<currentConfig>.yaml"
 //
 // 3. Load the machine config.
 // 4. Append the machine annotations file.
 // 5. Append the KubeConfig file.
-func (bsc *bootstrapServer) GetConfig(cr poolRequest) (*ignv2_2types.Config, error) {
+func (bsc *bootstrapConfig) getConfig(cr poolRequest) (*ignv2_2types.Config, error) {
 
 	// 1. Read the Machine Config Pool object.
-	fileName := path.Join(bsc.serverBaseDir, "machine-pools", cr.machinePool+".yaml")
+	fileName := path.Join(bsc.configBaseDir, "machine-pools", cr.machinePool+".yaml")
 	glog.Infof("reading file %q", fileName)
 	data, err := ioutil.ReadFile(fileName)
 	if os.IsNotExist(err) {
@@ -79,7 +80,7 @@ func (bsc *bootstrapServer) GetConfig(cr poolRequest) (*ignv2_2types.Config, err
 	currConf := mp.Status.CurrentMachineConfig
 
 	// 2. Read the Machine Config object.
-	fileName = path.Join(bsc.serverBaseDir, "machine-configs", currConf+".yaml")
+	fileName = path.Join(bsc.configBaseDir, "machine-configs", currConf+".yaml")
 	glog.Infof("reading file %q", fileName)
 	data, err = ioutil.ReadFile(fileName)
 	if os.IsNotExist(err) {