From 05a3965602d11a109f4043211aa27835a6882e37 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Fri, 11 Mar 2016 17:40:59 -0700 Subject: [PATCH] fix(manager): add API test for healthz. --- cmd/manager/deployments.go | 2 +- cmd/manager/deployments_test.go | 40 +++++++++++++++ cmd/manager/main.go | 5 +- cmd/manager/router/router.go | 46 ++++------------- glide.lock | 2 +- pkg/httputil/doc.go | 21 ++++++++ .../router => pkg/httputil}/encoder.go | 19 +++++-- .../router => pkg/httputil}/encoder_test.go | 14 +++--- pkg/httputil/httperrors.go | 48 ++++++++++++++++++ pkg/httputil/httperrors_test.go | 50 +++++++++++++++++++ 10 files changed, 195 insertions(+), 52 deletions(-) create mode 100644 cmd/manager/deployments_test.go create mode 100644 pkg/httputil/doc.go rename {cmd/manager/router => pkg/httputil}/encoder.go (82%) rename {cmd/manager/router => pkg/httputil}/encoder_test.go (90%) create mode 100644 pkg/httputil/httperrors.go create mode 100644 pkg/httputil/httperrors_test.go diff --git a/cmd/manager/deployments.go b/cmd/manager/deployments.go index e71a2418e..6068dae46 100644 --- a/cmd/manager/deployments.go +++ b/cmd/manager/deployments.go @@ -99,7 +99,7 @@ func registerRoutes(c *router.Context) router.Routes { } func healthz(w http.ResponseWriter, r *http.Request, c *router.Context) error { - c.Log("manager: healthz checkpoint") + log.Println("manager: healthz checkpoint") // TODO: This should check the availability of the repository, and fail if it // cannot connect. fmt.Fprintln(w, "OK") diff --git a/cmd/manager/deployments_test.go b/cmd/manager/deployments_test.go new file mode 100644 index 000000000..58a5ad3f6 --- /dev/null +++ b/cmd/manager/deployments_test.go @@ -0,0 +1,40 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/kubernetes/deployment-manager/cmd/manager/router" +) + +func TestHealthz(t *testing.T) { + c := mockContext() + s := httpHarness(c, "GET /", healthz) + defer s.Close() + + res, err := http.Get(s.URL) + if err != nil { + t.Fatalf("Failed to GET healthz: %s", err) + } else if res.StatusCode != 200 { + t.Fatalf("Unexpected status: %d", res.StatusCode) + } + + // TODO: Get the body and check on the content type and the body. +} + +// 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 *router.Context, route string, fn router.HandlerFunc) *httptest.Server { + r := router.NewRoutes() + r.Add(route, fn) + h := router.NewHandler(c, r) + return httptest.NewServer(h) +} + +func mockContext() *router.Context { + // TODO: We need mocks for credentials and manager. + return &router.Context{} +} diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 7aca7e80a..4c26d599f 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -22,6 +22,7 @@ import ( "flag" "fmt" + "log" "net/http" "os" ) @@ -55,9 +56,9 @@ func main() { routes := registerRoutes(c) // Now create a server. - c.Log("Starting Manager %s on %s", version.Version, c.Config.Address) + 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 { - c.Err("Server exited with error %s", err) + log.Printf("Server exited with error %s", err) os.Exit(1) } } diff --git a/cmd/manager/router/router.go b/cmd/manager/router/router.go index 6f15292cd..3a6bf5a42 100644 --- a/cmd/manager/router/router.go +++ b/cmd/manager/router/router.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -/* Package router is an HTTP router. +/*Package router is an HTTP router. This router provides appropriate dependency injection/encapsulation for the HTTP routing layer. This removes the requirement to set global variables for @@ -29,17 +29,15 @@ request and response. package router import ( - "fmt" + "log" "net/http" - "os" "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" ) -const LogAccess = "Access: %s %s" - // Config holds the global configuration parameters passed into the router. // // Config is used concurrently. Once a config is created, it should be treated @@ -80,36 +78,10 @@ type Context struct { Config *Config // Manager is a deployment-manager/manager/manager.Manager Manager manager.Manager - Encoder Encoder + Encoder helmhttp.Encoder CredentialProvider common.CredentialProvider } -func (c *Context) Log(msg string, v ...interface{}) { - // FIXME: This should be configurable via the context. - fmt.Fprintf(os.Stdout, msg+"\n", v...) -} - -func (c *Context) Err(msg string, v ...interface{}) { - // FIXME: This should be configurable via the context. - fmt.Fprintf(os.Stderr, msg+"\n", v...) -} - -// NotFound writes a 404 error to the client and logs an error. -func NotFound(w http.ResponseWriter, r *http.Request) { - // TODO: Log this. - w.WriteHeader(http.StatusNotFound) - fmt.Fprintln(w, "File Not Found") -} - -// Fatal writes a 500 response to the client and logs the message. -// -// Additional arguments are past into the the formatter as params to msg. -func Fatal(w http.ResponseWriter, r *http.Request, msg string, v ...interface{}) { - // TODO: Log this. - w.WriteHeader(http.StatusInternalServerError) - fmt.Fprintln(w, "Internal Server Error") -} - // HandlerFunc responds to an individual HTTP request. // // Returned errors will be captured, logged, and returned as HTTP 500 errors. @@ -124,7 +96,7 @@ type Handler struct { routes Routes } -// Create a new Handler. +// 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. @@ -145,20 +117,20 @@ func NewHandler(c *Context, r Routes) *Handler { // ServeHTTP serves an HTTP request. func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - h.c.Log(LogAccess, r.Method, r.URL) + log.Printf(helmhttp.LogAccess, r.Method, r.URL) route, err := h.resolver.Resolve(r) if err != nil { - NotFound(w, r) + helmhttp.NotFound(w, r) return } fn, ok := h.routes.Get(route) if !ok { - Fatal(w, r, "route %s missing", route) + helmhttp.Fatal(w, r, "route %s missing", route) } if err := fn(w, r, h.c); err != nil { - Fatal(w, r, err.Error()) + helmhttp.Fatal(w, r, err.Error()) } } diff --git a/glide.lock b/glide.lock index 67961e905..9c507ef66 100644 --- a/glide.lock +++ b/glide.lock @@ -1,5 +1,5 @@ hash: f34e830b237fcba5202f84c55b21764226b7e8c201f7622e9639c7a628e33b0b -updated: 2016-03-10T15:03:34.650809572-07:00 +updated: 2016-03-14T10:53:18.091793732-06:00 imports: - name: github.com/aokoli/goutils version: 9c37978a95bd5c709a15883b6242714ea6709e64 diff --git a/pkg/httputil/doc.go b/pkg/httputil/doc.go new file mode 100644 index 000000000..2c33cf5b2 --- /dev/null +++ b/pkg/httputil/doc.go @@ -0,0 +1,21 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/*Package httputil provides common HTTP tools. + +This package provides tools for working with HTTP requests and responses. +*/ +package httputil diff --git a/cmd/manager/router/encoder.go b/pkg/httputil/encoder.go similarity index 82% rename from cmd/manager/router/encoder.go rename to pkg/httputil/encoder.go index fd401cb59..beefd20c0 100644 --- a/cmd/manager/router/encoder.go +++ b/pkg/httputil/encoder.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package router +package httputil import ( "encoding/json" @@ -28,26 +28,35 @@ import ( "github.com/ghodss/yaml" ) +// Encoder takes input and translate it to an expected encoded output. +// +// Implementations of encoders may use details of the HTTP request and response +// to correctly encode an object for return to the client. +// +// Encoders are expected to produce output, even if that output is an error +// message. type Encoder interface { // Encode encoders a given response // // When an encoder fails, it logs any necessary data and then responds to // the client. - Encode(http.ResponseWriter, *http.Request, *Context, interface{}) + Encode(http.ResponseWriter, *http.Request, interface{}) } -// AcceptEncodder uses the accept headers on a request to determine the response type. +// AcceptEncoder uses the accept headers on a request to determine the response type. // // It supports the following encodings: // - application/json: passed to encoding/json.Marshal // - text/yaml: passed to gopkg.in/yaml.v2.Marshal // - text/plain: passed to fmt.Sprintf("%V") +// +// It treats `application/x-yaml` as `text/yaml`. type AcceptEncoder struct { DefaultEncoding string } // Encode encodeds the given interface to the first available type in the Accept header. -func (e *AcceptEncoder) Encode(w http.ResponseWriter, r *http.Request, c *Context, out interface{}) { +func (e *AcceptEncoder) Encode(w http.ResponseWriter, r *http.Request, out interface{}) { a := r.Header.Get("accept") fn := encoders[e.DefaultEncoding] mt := e.DefaultEncoding @@ -82,6 +91,7 @@ func (e *AcceptEncoder) parseAccept(h string) (string, Marshaler) { return e.DefaultEncoding, encoders[e.DefaultEncoding] } +// Marshaler marshals an interface{} into a []byte. type Marshaler func(interface{}) ([]byte, error) var encoders = map[string]Marshaler{ @@ -91,6 +101,7 @@ var encoders = map[string]Marshaler{ "text/plain": textMarshal, } +// ErrUnsupportedKind indicates that the marshal cannot marshal a particular Go Kind (e.g. struct or chan). var ErrUnsupportedKind = errors.New("unsupported kind") // textMarshal marshals v into a text representation ONLY IN NARROW CASES. diff --git a/cmd/manager/router/encoder_test.go b/pkg/httputil/encoder_test.go similarity index 90% rename from cmd/manager/router/encoder_test.go rename to pkg/httputil/encoder_test.go index 283581b3e..115c961d5 100644 --- a/cmd/manager/router/encoder_test.go +++ b/pkg/httputil/encoder_test.go @@ -14,13 +14,14 @@ See the License for the specific language governing permissions and limitations under the License. */ -package router +package httputil import ( "encoding/json" "errors" "io/ioutil" "net/http" + "net/http/httptest" "testing" ) @@ -72,14 +73,13 @@ func TestTextMarshal(t *testing.T) { } func TestAcceptEncoder(t *testing.T) { - c := &Context{ - Encoder: &AcceptEncoder{DefaultEncoding: "application/json"}, + enc := &AcceptEncoder{ + DefaultEncoding: "application/json", } - fn := func(w http.ResponseWriter, r *http.Request, c *Context) error { - c.Encoder.Encode(w, r, c, []string{"hello", "world"}) - return nil + fn := func(w http.ResponseWriter, r *http.Request) { + enc.Encode(w, r, []string{"hello", "world"}) } - s := httpHarness(c, "GET /", fn) + s := httptest.NewServer(http.HandlerFunc(fn)) defer s.Close() res, err := http.Get(s.URL) diff --git a/pkg/httputil/httperrors.go b/pkg/httputil/httperrors.go new file mode 100644 index 000000000..1d9f0d56e --- /dev/null +++ b/pkg/httputil/httperrors.go @@ -0,0 +1,48 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package httputil + +import ( + "fmt" + "log" + "net/http" +) + +const ( + // LogAccess is for logging access messages. Form: Access r.Method, r.URL + LogAccess = "Access: %s %s" + // LogNotFound is for logging 404 errors. Form: Not Found r.Method, r.URL + LogNotFound = "Not Found: %s %s" + // LogFatal is for logging 500 errors. Form: Internal Server Error r.Method r.URL message + LogFatal = "Internal Server Error: %s %s %s" +) + +// NotFound writes a 404 error to the client and logs an error. +func NotFound(w http.ResponseWriter, r *http.Request) { + log.Printf(LogNotFound, r.Method, r.URL) + w.WriteHeader(http.StatusNotFound) + fmt.Fprintln(w, "File Not Found") +} + +// Fatal writes a 500 response to the client and logs the message. +// +// Additional arguments are past into the the formatter as params to msg. +func Fatal(w http.ResponseWriter, r *http.Request, msg string, v ...interface{}) { + log.Printf(LogFatal, r.Method, r.URL, fmt.Sprintf(msg, v...)) + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprintln(w, "Internal Server Error") +} diff --git a/pkg/httputil/httperrors_test.go b/pkg/httputil/httperrors_test.go new file mode 100644 index 000000000..ea1fecad9 --- /dev/null +++ b/pkg/httputil/httperrors_test.go @@ -0,0 +1,50 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package httputil + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestNotFound(t *testing.T) { + fn := func(w http.ResponseWriter, r *http.Request) { + NotFound(w, r) + } + testStatusCode(http.HandlerFunc(fn), 404, t) +} + +func TestFatal(t *testing.T) { + fn := func(w http.ResponseWriter, r *http.Request) { + Fatal(w, r, "fatal %s", "foo") + } + testStatusCode(http.HandlerFunc(fn), 500, t) +} + +func testStatusCode(fn http.HandlerFunc, expect int, t *testing.T) { + s := httptest.NewServer(fn) + defer s.Close() + + res, err := http.Get(s.URL) + if err != nil { + t.Fatal(err) + } + if res.StatusCode != expect { + t.Errorf("Expected %d, got %d", expect, res.StatusCode) + } +}