[ARVADOS] created: 0e0dc3c8ebf442a35c41816fed42fdddb53aed53

Git user git at public.curoverse.com
Thu Jul 20 17:25:47 EDT 2017


        at  0e0dc3c8ebf442a35c41816fed42fdddb53aed53 (commit)


commit 0e0dc3c8ebf442a35c41816fed42fdddb53aed53
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Jul 20 17:24:09 2017 -0400

    11906: Remove keepstore's health-check unit tests.
    
    Now that the health check handler has its own unit tests, keepstore
    only needs to check that it's wired up correctly.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curoverse.com>

diff --git a/services/keepstore/handler_test.go b/services/keepstore/handler_test.go
index 7429d7a..424910d 100644
--- a/services/keepstore/handler_test.go
+++ b/services/keepstore/handler_test.go
@@ -1155,59 +1155,19 @@ func TestUntrashHandlerWithNoWritableVolumes(t *testing.T) {
 }
 
 func TestHealthCheckPing(t *testing.T) {
-	defer teardown()
-
-	KeepVM = MakeTestVolumeManager(2)
-	defer KeepVM.Close()
-
-	// ping when disabled
-	theConfig.ManagementToken = ""
-	pingReq := &RequestTester{
-		method: "GET",
-		uri:    "/_health/ping",
-	}
-	response := IssueHealthCheckRequest(pingReq)
-	ExpectStatusCode(t,
-		"disabled",
-		http.StatusNotFound,
-		response)
-
-	// ping with no token
 	theConfig.ManagementToken = arvadostest.ManagementToken
-	pingReq = &RequestTester{
-		method: "GET",
-		uri:    "/_health/ping",
-	}
-	response = IssueHealthCheckRequest(pingReq)
-	ExpectStatusCode(t,
-		"authorization required",
-		http.StatusUnauthorized,
-		response)
-
-	// ping with wrong token
-	pingReq = &RequestTester{
-		method:   "GET",
-		uri:      "/_health/ping",
-		apiToken: "youarenotwelcomehere",
-	}
-	response = IssueHealthCheckRequest(pingReq)
-	ExpectStatusCode(t,
-		"authorization error",
-		http.StatusForbidden,
-		response)
-
-	// ping with management token
-	pingReq = &RequestTester{
+	pingReq := &RequestTester{
 		method:   "GET",
 		uri:      "/_health/ping",
 		apiToken: arvadostest.ManagementToken,
 	}
-	response = IssueHealthCheckRequest(pingReq)
+	response := IssueHealthCheckRequest(pingReq)
 	ExpectStatusCode(t,
 		"",
 		http.StatusOK,
 		response)
-	if !strings.Contains(response.Body.String(), `{"health":"OK"}`) {
-		t.Errorf("expected response to include %s: got %s", `{"health":"OK"}`, response.Body.String())
+	want := `{"health":"OK"}`
+	if !strings.Contains(response.Body.String(), want) {
+		t.Errorf("expected response to include %s: got %s", want, response.Body.String())
 	}
 }

commit a2fc189f7a50964340bc60704c5cd3bef9114c7d
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Jul 20 17:16:41 2017 -0400

    11906: Add tests for sdk/go/health.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curoverse.com>

diff --git a/build/run-tests.sh b/build/run-tests.sh
index 00987a8..3952b36 100755
--- a/build/run-tests.sh
+++ b/build/run-tests.sh
@@ -95,6 +95,7 @@ sdk/go/arvados
 sdk/go/arvadosclient
 sdk/go/dispatch
 sdk/go/keepclient
+sdk/go/health
 sdk/go/httpserver
 sdk/go/manifest
 sdk/go/blockdigest
@@ -774,6 +775,7 @@ gostuff=(
     sdk/go/arvadosclient
     sdk/go/blockdigest
     sdk/go/dispatch
+    sdk/go/health
     sdk/go/httpserver
     sdk/go/manifest
     sdk/go/streamer
diff --git a/sdk/go/health/handler_test.go b/sdk/go/health/handler_test.go
new file mode 100644
index 0000000..055e2c3
--- /dev/null
+++ b/sdk/go/health/handler_test.go
@@ -0,0 +1,121 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package health
+
+import (
+	"encoding/json"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	check "gopkg.in/check.v1"
+)
+
+// Gocheck boilerplate
+var _ = check.Suite(&Suite{})
+
+func Test(t *testing.T) {
+	check.TestingT(t)
+}
+
+type Suite struct{}
+
+const (
+	goodToken = "supersecret"
+	badToken  = "pwn"
+)
+
+func (s *Suite) TestPassFailRefuse(c *check.C) {
+	h := &Handler{
+		Token:  goodToken,
+		Prefix: "/_health/",
+		Routes: Routes{
+			"success": func() error { return nil },
+			"miracle": func() error { return errors.New("unimplemented") },
+		},
+	}
+
+	resp := httptest.NewRecorder()
+	h.ServeHTTP(resp, s.request("/_health/ping", goodToken))
+	s.checkHealthy(c, resp)
+
+	resp = httptest.NewRecorder()
+	h.ServeHTTP(resp, s.request("/_health/success", goodToken))
+	s.checkHealthy(c, resp)
+
+	resp = httptest.NewRecorder()
+	h.ServeHTTP(resp, s.request("/_health/miracle", goodToken))
+	s.checkUnhealthy(c, resp)
+
+	resp = httptest.NewRecorder()
+	h.ServeHTTP(resp, s.request("/_health/miracle", badToken))
+	c.Check(resp.Code, check.Equals, http.StatusForbidden)
+
+	resp = httptest.NewRecorder()
+	h.ServeHTTP(resp, s.request("/_health/miracle", ""))
+	c.Check(resp.Code, check.Equals, http.StatusUnauthorized)
+}
+
+func (s *Suite) TestPingOverride(c *check.C) {
+	var ok bool
+	h := &Handler{
+		Token: goodToken,
+		Routes: Routes{
+			"ping": func() error {
+				ok = !ok
+				if ok {
+					return nil
+				} else {
+					return errors.New("good error")
+				}
+			},
+		},
+	}
+	resp := httptest.NewRecorder()
+	h.ServeHTTP(resp, s.request("/ping", goodToken))
+	s.checkHealthy(c, resp)
+
+	resp = httptest.NewRecorder()
+	h.ServeHTTP(resp, s.request("/ping", goodToken))
+	s.checkUnhealthy(c, resp)
+}
+
+func (s *Suite) TestZeroValue(c *check.C) {
+	resp := httptest.NewRecorder()
+	(&Handler{}).ServeHTTP(resp, s.request("/ping", goodToken))
+	c.Check(resp.Code, check.Equals, http.StatusNotFound)
+}
+
+func (s *Suite) request(path, token string) *http.Request {
+	u, _ := url.Parse("http://foo.local" + path)
+	req := &http.Request{
+		Method:     "GET",
+		Host:       u.Host,
+		URL:        u,
+		RequestURI: u.RequestURI(),
+	}
+	if token != "" {
+		req.Header = http.Header{
+			"Authorization": {"Bearer " + token},
+		}
+	}
+	return req
+}
+
+func (s *Suite) checkHealthy(c *check.C, resp *httptest.ResponseRecorder) {
+	c.Check(resp.Code, check.Equals, http.StatusOK)
+	c.Check(resp.Body.String(), check.Equals, `{"health":"OK"}`+"\n")
+}
+
+func (s *Suite) checkUnhealthy(c *check.C, resp *httptest.ResponseRecorder) {
+	c.Check(resp.Code, check.Equals, http.StatusOK)
+	var result map[string]interface{}
+	err := json.Unmarshal(resp.Body.Bytes(), &result)
+	c.Assert(err, check.IsNil)
+	c.Check(result["health"], check.Equals, "ERROR")
+	c.Check(result["error"].(string), check.Not(check.Equals), "")
+}

commit 2835665e43a3b85c85fefcd348015b5dee829cba
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Jul 20 16:18:38 2017 -0400

    11906: Use sdk/go/health for keepstore health checks.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curoverse.com>

diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index 101f42c..0a540f5 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -29,6 +29,7 @@ import (
 
 	"github.com/gorilla/mux"
 
+	"git.curoverse.com/arvados.git/sdk/go/health"
 	"git.curoverse.com/arvados.git/sdk/go/httpserver"
 	log "github.com/Sirupsen/logrus"
 )
@@ -78,8 +79,10 @@ func MakeRESTRouter() *router {
 	// Untrash moves blocks from trash back into store
 	rest.HandleFunc(`/untrash/{hash:[0-9a-f]{32}}`, UntrashHandler).Methods("PUT")
 
-	// Health check ping
-	rest.HandleFunc(`/_health/ping`, HealthCheckPingHandler).Methods("GET")
+	rest.Handle("/_health/{check}", &health.Handler{
+		Token:  theConfig.ManagementToken,
+		Prefix: "/_health/",
+	}).Methods("GET")
 
 	// Any request which does not match any of these routes gets
 	// 400 Bad Request.
@@ -620,45 +623,6 @@ func UntrashHandler(resp http.ResponseWriter, req *http.Request) {
 	}
 }
 
-// HealthCheckPingHandler processes "GET /_health/ping" requests
-func HealthCheckPingHandler(resp http.ResponseWriter, req *http.Request) {
-	fn := func() interface{} {
-		return map[string]string{"health": "OK"}
-	}
-
-	healthCheckDo(resp, req, fn)
-}
-
-// Any health check handlers can pass this "func" which returns json to healthCheckDo
-type healthCheckFunc func() interface{}
-
-func healthCheckDo(resp http.ResponseWriter, req *http.Request, fn healthCheckFunc) {
-	msg, code := healthCheckAuth(resp, req)
-	if msg != "" {
-		http.Error(resp, msg, code)
-		return
-	}
-
-	ok, err := json.Marshal(fn())
-	if err != nil {
-		http.Error(resp, err.Error(), 500)
-		return
-	}
-
-	resp.Write(ok)
-}
-
-func healthCheckAuth(resp http.ResponseWriter, req *http.Request) (string, int) {
-	if theConfig.ManagementToken == "" {
-		return "disabled", http.StatusNotFound
-	} else if h := req.Header.Get("Authorization"); h == "" {
-		return "authorization required", http.StatusUnauthorized
-	} else if h != "Bearer "+theConfig.ManagementToken {
-		return "authorization error", http.StatusForbidden
-	}
-	return "", 0
-}
-
 // GetBlock and PutBlock implement lower-level code for handling
 // blocks by rooting through volumes connected to the local machine.
 // Once the handler has determined that system policy permits the

commit 34c4b3001b3aafc91213f02689d6e61977ad29b3
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Jul 20 13:45:12 2017 -0400

    11906: Refactor health-check handler into SDK.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curoverse.com>

diff --git a/sdk/go/health/handler.go b/sdk/go/health/handler.go
new file mode 100644
index 0000000..aef72c2
--- /dev/null
+++ b/sdk/go/health/handler.go
@@ -0,0 +1,109 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package health
+
+import (
+	"encoding/json"
+	"errors"
+	"net/http"
+	"strings"
+	"sync"
+)
+
+// Func is a health-check function: it returns nil when healthy, an
+// error when not.
+type Func func() error
+
+// Routes is a map of URI path to health-check function.
+type Routes map[string]Func
+
+// Handler is an http.Handler that responds to authenticated
+// health-check requests with JSON responses like {"health":"OK"} or
+// {"health":"ERROR","error":"error text"}.
+//
+// Fields of a Handler should not be changed after the Handler is
+// first used.
+type Handler struct {
+	setupOnce sync.Once
+	mux       *http.ServeMux
+
+	// Authentication token. If empty, all requests will return 404.
+	Token string
+
+	// Route prefix, typically "/_health/".
+	Prefix string
+
+	// Map of URI paths to health-check Func. The prefix is
+	// omitted: Routes["foo"] is the health check invoked by a
+	// request to "/_health/foo".
+	//
+	// If "ping" is not listed here, it will be added
+	// automatically and will always return a "healthy" response.
+	Routes Routes
+
+	// If non-nil, Log is called after handling each request. The
+	// error argument is nil if the request was succesfully
+	// authenticated and served, even if the health check itself
+	// failed.
+	Log func(*http.Request, error)
+}
+
+// ServeHTTP implements http.Handler.
+func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	h.setupOnce.Do(h.setup)
+	h.mux.ServeHTTP(w, r)
+}
+
+func (h *Handler) setup() {
+	h.mux = http.NewServeMux()
+	prefix := h.Prefix
+	if !strings.HasSuffix(prefix, "/") {
+		prefix = prefix + "/"
+	}
+	for name, fn := range h.Routes {
+		h.mux.Handle(prefix+name, h.healthJSON(fn))
+	}
+	if _, ok := h.Routes["ping"]; !ok {
+		h.mux.Handle(prefix+"ping", h.healthJSON(func() error { return nil }))
+	}
+}
+
+var (
+	healthyBody     = []byte(`{"health":"OK"}` + "\n")
+	errNotFound     = errors.New(http.StatusText(http.StatusNotFound))
+	errUnauthorized = errors.New(http.StatusText(http.StatusUnauthorized))
+	errForbidden    = errors.New(http.StatusText(http.StatusForbidden))
+)
+
+func (h *Handler) healthJSON(fn Func) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		var err error
+		defer func() {
+			if h.Log != nil {
+				h.Log(r, err)
+			}
+		}()
+		if h.Token == "" {
+			http.Error(w, "disabled", http.StatusNotFound)
+			err = errNotFound
+		} else if ah := r.Header.Get("Authorization"); ah == "" {
+			http.Error(w, "authorization required", http.StatusUnauthorized)
+			err = errUnauthorized
+		} else if ah != "Bearer "+h.Token {
+			http.Error(w, "authorization error", http.StatusForbidden)
+			err = errForbidden
+		} else if err = fn(); err == nil {
+			w.Header().Set("Content-Type", "application/json")
+			w.Write(healthyBody)
+		} else {
+			w.Header().Set("Content-Type", "application/json")
+			enc := json.NewEncoder(w)
+			err = enc.Encode(map[string]string{
+				"health": "ERROR",
+				"error":  err.Error(),
+			})
+		}
+	})
+}
diff --git a/services/ws/router.go b/services/ws/router.go
index 3f3a051..2b9bd66 100644
--- a/services/ws/router.go
+++ b/services/ws/router.go
@@ -14,6 +14,7 @@ import (
 	"time"
 
 	"git.curoverse.com/arvados.git/sdk/go/ctxlog"
+	"git.curoverse.com/arvados.git/sdk/go/health"
 	"github.com/Sirupsen/logrus"
 	"golang.org/x/net/websocket"
 )
@@ -60,10 +61,18 @@ func (rtr *router) setup() {
 	rtr.mux.Handle("/debug.json", rtr.jsonHandler(rtr.DebugStatus))
 	rtr.mux.Handle("/status.json", rtr.jsonHandler(rtr.Status))
 
-	health := http.NewServeMux()
-	rtr.mux.Handle("/_health/", rtr.mgmtAuth(health))
-	health.Handle("/_health/ping", rtr.jsonHandler(rtr.HealthFunc(func() error { return nil })))
-	health.Handle("/_health/db", rtr.jsonHandler(rtr.HealthFunc(rtr.eventSource.DBHealth)))
+	rtr.mux.Handle("/_health/", &health.Handler{
+		Token:  rtr.Config.ManagementToken,
+		Prefix: "/_health/",
+		Routes: health.Routes{
+			"db": rtr.eventSource.DBHealth,
+		},
+		Log: func(r *http.Request, err error) {
+			if err != nil {
+				logger(r.Context()).WithError(err).Error("error")
+			}
+		},
+	})
 }
 
 func (rtr *router) makeServer(newSession sessionFactory) *websocket.Server {
@@ -111,21 +120,6 @@ func (rtr *router) DebugStatus() interface{} {
 	return s
 }
 
-var pingResponseOK = map[string]string{"health": "OK"}
-
-func (rtr *router) HealthFunc(f func() error) func() interface{} {
-	return func() interface{} {
-		err := f()
-		if err == nil {
-			return pingResponseOK
-		}
-		return map[string]string{
-			"health": "ERROR",
-			"error":  err.Error(),
-		}
-	}
-}
-
 func (rtr *router) Status() interface{} {
 	return map[string]interface{}{
 		"Clients": atomic.LoadInt64(&rtr.status.ReqsActive),
@@ -149,20 +143,6 @@ func (rtr *router) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
 	rtr.mux.ServeHTTP(resp, req)
 }
 
-func (rtr *router) mgmtAuth(h http.Handler) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if rtr.Config.ManagementToken == "" {
-			http.Error(w, "disabled", http.StatusNotFound)
-		} else if ah := r.Header.Get("Authorization"); ah == "" {
-			http.Error(w, "authorization required", http.StatusUnauthorized)
-		} else if ah != "Bearer "+rtr.Config.ManagementToken {
-			http.Error(w, "authorization error", http.StatusForbidden)
-		} else {
-			h.ServeHTTP(w, r)
-		}
-	})
-}
-
 func (rtr *router) jsonHandler(fn func() interface{}) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		logger := logger(r.Context())

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list