diff --git a/cmd/machine-config-server/bootstrap.go b/cmd/machine-config-server/bootstrap.go index d28888c074..ac48bad917 100644 --- a/cmd/machine-config-server/bootstrap.go +++ b/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") } diff --git a/cmd/machine-config-server/start.go b/cmd/machine-config-server/start.go index 2d6e528be0..134f09af9e 100644 --- a/cmd/machine-config-server/start.go +++ b/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") } diff --git a/pkg/server/api.go b/pkg/server/api.go index 542ffaa321..0355e2e0b4 100644 --- a/pkg/server/api.go +++ b/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 +} diff --git a/pkg/server/api_test.go b/pkg/server/api_test.go index 8b632bf297..008abb89f9 100644 --- a/pkg/server/api_test.go +++ b/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,11 +82,11 @@ 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) }, @@ -94,8 +94,8 @@ func TestAPIHandler(t *testing.T) { { 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() diff --git a/pkg/server/bootstrap_server.go b/pkg/server/bootstrap_server.go index ee9227888d..fba5037cdb 100644 --- a/pkg/server/bootstrap_server.go +++ b/pkg/server/bootstrap_server.go @@ -3,6 +3,7 @@ package server import ( "fmt" "io/ioutil" + "net/http" "os" "path" @@ -14,32 +15,32 @@ 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. @@ -47,19 +48,19 @@ func NewBootstrapServer(dir, kubeconfig string) (Server, error) { // The method does the following: // // 1. Read the machine config pool by using the following path template: -// "/machine-pools/.yaml" +// "/machine-pools/.yaml" // // 2. Read the currentConfig field from the Status and read the config file // using the following path template: -// "/machine-configs/.yaml" +// "/machine-configs/.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) { diff --git a/pkg/server/cluster_server.go b/pkg/server/cluster_server.go index 09653cbbd7..de5da37acd 100644 --- a/pkg/server/cluster_server.go +++ b/pkg/server/cluster_server.go @@ -3,6 +3,7 @@ package server import ( "fmt" "io/ioutil" + "net/http" "path/filepath" ignv2_2types "github.com/coreos/ignition/config/v2_2/types" @@ -24,11 +25,7 @@ const ( bootstrapTokenDir = "/etc/mcs/bootstrap-token" ) -// ensure clusterServer implements the -// Server interface. -var _ = Server(&clusterServer{}) - -type clusterServer struct { +type clusterConfig struct { // machineClient is used to interact with the // machine config, pool objects. machineClient v1.MachineconfigurationV1Interface @@ -36,28 +33,31 @@ type clusterServer struct { kubeconfigFunc kubeconfigFunc } -// NewClusterServer is used to initialize the machine config -// server that will be used to fetch the requested machine pool -// objects from within the cluster. -// It accepts the kubeConfig which is not required when it's -// run from within the cluster(useful in testing). -// It accepts the apiserverURL which is the location of the KubeAPIServer. -func NewClusterServer(kubeConfig, apiserverURL string) (Server, error) { +// NewClusterServer initializes a new Server that loads its +// configuration from the cluster. It accepts the kubeConfig which is +// not required when it's run from within the cluster (useful in +// testing). It accepts the apiServerURL which is the location of the +// Kubernetes API server. +func NewClusterServer(kubeConfig, apiServerURL string) (*http.Server, error) { restConfig, err := getClientConfig(kubeConfig) if err != nil { return nil, fmt.Errorf("Failed to create Kubernetes rest client: %v", err) } mc := v1.NewForConfigOrDie(restConfig) - return &clusterServer{ + config := &clusterConfig{ machineClient: mc, - kubeconfigFunc: func() ([]byte, []byte, error) { return kubeconfigFromSecret(bootstrapTokenDir, apiserverURL) }, + kubeconfigFunc: func() ([]byte, []byte, error) { return kubeconfigFromSecret(bootstrapTokenDir, apiServerURL) }, + } + + return &http.Server{ + Handler: newHandler(config.getConfig), }, nil } -// GetConfig fetches the machine config(type - Ignition) from the cluster, +// getConfig fetches the machine config(type - Ignition) from the cluster, // based on the pool request. -func (cs *clusterServer) GetConfig(cr poolRequest) (*ignv2_2types.Config, error) { +func (cs *clusterConfig) getConfig(cr poolRequest) (*ignv2_2types.Config, error) { mp, err := cs.machineClient.MachineConfigPools().Get(cr.machinePool, metav1.GetOptions{}) if err != nil { return nil, fmt.Errorf("could not fetch pool. err: %v", err) diff --git a/pkg/server/server.go b/pkg/server/server.go index 03ecbd90e9..052d5275d1 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -28,12 +28,6 @@ type kubeconfigFunc func() (kubeconfigData []byte, rootCAData []byte, err error) // appenderFunc appends Config. type appenderFunc func(*ignv2_2types.Config) error -// Server defines the interface that is implemented by different -// machine config server implementations. -type Server interface { - GetConfig(poolRequest) (*ignv2_2types.Config, error) -} - func getAppenders(cr poolRequest, currMachineConfig string, f kubeconfigFunc) []appenderFunc { appenders := []appenderFunc{ // append machine annotations file. diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index dc6a5d1469..0c24d5b600 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -56,7 +56,7 @@ func TestStringEncode(t *testing.T) { // the node-annotations file, the kubeconfig file(which is read // from the testdata). This ignition config is then // labeled as expected Ignition config. -// 4. Call the Bootstrap GetConfig method by passing the reference to the +// 4. Call the Bootstrap getConfig method by passing the reference to the // machine pool present in the testdata folder. // 5. Compare the Ignition configs from Step 3 and Step 4. func TestBootstrapServer(t *testing.T) { @@ -90,14 +90,14 @@ func TestBootstrapServer(t *testing.T) { appendFileToIgnition(&mc.Spec.Config, daemon.InitialNodeAnnotationsFilePath, anno) // initialize bootstrap server and get config. - bs := &bootstrapServer{ - serverBaseDir: testDir, + bs := &bootstrapConfig{ + configBaseDir: testDir, kubeconfigFunc: func() ([]byte, []byte, error) { return getKubeConfigContent(t) }, } if err != nil { t.Fatal(err) } - res, err := bs.GetConfig(poolRequest{ + res, err := bs.getConfig(poolRequest{ machinePool: testPool, }) if err != nil { @@ -121,7 +121,7 @@ func TestBootstrapServer(t *testing.T) { // labeled as expected Ignition config (mc). // 4. Use the Kubernetes fake client to Create the machine pool and the config // objects from Step 1, 2 inside the cluster. -// 5. Call the Cluster GetConfig method. +// 5. Call the Cluster getConfig method. // 6. Compare the Ignition configs from Step 3 and Step 5. func TestClusterServer(t *testing.T) { mp, err := getTestMachinePool() @@ -150,7 +150,7 @@ func TestClusterServer(t *testing.T) { t.Logf("err: %v", err) } - csc := &clusterServer{ + csc := &clusterConfig{ machineClient: cs.MachineconfigurationV1(), kubeconfigFunc: func() ([]byte, []byte, error) { return getKubeConfigContent(t) }, } @@ -172,7 +172,7 @@ func TestClusterServer(t *testing.T) { } appendFileToIgnition(&mc.Spec.Config, daemon.InitialNodeAnnotationsFilePath, anno) - res, err := csc.GetConfig(poolRequest{ + res, err := csc.getConfig(poolRequest{ machinePool: testPool, }) if err != nil {