17464: MakeRESTRouter returns erro instead of panicking
authorPeter Amstutz <peter.amstutz@curii.com>
Mon, 21 Jun 2021 15:25:29 +0000 (11:25 -0400)
committerPeter Amstutz <peter.amstutz@curii.com>
Mon, 21 Jun 2021 15:25:29 +0000 (11:25 -0400)
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>

services/keepproxy/keepproxy.go
services/keepproxy/keepproxy_test.go

index 04c95cfee6aa729ce090e19518867e160f47192d..21bbfe40cc9e41b980d11172275c1c1f222e9a0c 100644 (file)
@@ -163,7 +163,10 @@ func run(logger log.FieldLogger, cluster *arvados.Cluster) error {
        signal.Notify(term, syscall.SIGINT)
 
        // Start serving requests.
-       router = MakeRESTRouter(kc, time.Duration(keepclient.DefaultProxyRequestTimeout), cluster, logger)
+       router, err = MakeRESTRouter(kc, time.Duration(keepclient.DefaultProxyRequestTimeout), cluster, logger)
+       if err != nil {
+               return err
+       }
        return http.Serve(listener, httpserver.AddRequestIDs(httpserver.LogRequests(router)))
 }
 
@@ -311,7 +314,7 @@ type proxyHandler struct {
 
 // MakeRESTRouter returns an http.Handler that passes GET and PUT
 // requests to the appropriate handlers.
-func MakeRESTRouter(kc *keepclient.KeepClient, timeout time.Duration, cluster *arvados.Cluster, logger log.FieldLogger) http.Handler {
+func MakeRESTRouter(kc *keepclient.KeepClient, timeout time.Duration, cluster *arvados.Cluster, logger log.FieldLogger) (http.Handler, error) {
        rest := mux.NewRouter()
 
        transport := defaultTransport
@@ -325,7 +328,7 @@ func MakeRESTRouter(kc *keepclient.KeepClient, timeout time.Duration, cluster *a
 
        cacheQ, err := lru.New2Q(500)
        if err != nil {
-               panic("Could not create 2Q")
+               return nil, fmt.Errorf("Error from lru.New2Q: %v", err)
        }
 
        h := &proxyHandler{
@@ -362,7 +365,7 @@ func MakeRESTRouter(kc *keepclient.KeepClient, timeout time.Duration, cluster *a
        }).Methods("GET")
 
        rest.NotFoundHandler = InvalidPathHandler{}
-       return h
+       return h, nil
 }
 
 var errLoopDetected = errors.New("loop detected")
index 67018f93c014b70c5f9b9234a8c0db67f123bb1b..9dc07ddaa0e1be60ce3f4b522dbc3d90aab4ed02 100644 (file)
@@ -26,6 +26,7 @@ import (
        "git.arvados.org/arvados.git/sdk/go/keepclient"
        log "github.com/sirupsen/logrus"
 
+       "gopkg.in/check.v1"
        . "gopkg.in/check.v1"
 )
 
@@ -264,7 +265,8 @@ func (s *ServerRequiredSuite) TestPutWrongContentLength(c *C) {
        // fixes the invalid Content-Length header. In order to test
        // our server behavior, we have to call the handler directly
        // using an httptest.ResponseRecorder.
-       rtr := MakeRESTRouter(kc, 10*time.Second, &arvados.Cluster{}, log.New())
+       rtr, err := MakeRESTRouter(kc, 10*time.Second, &arvados.Cluster{}, log.New())
+       c.Assert(err, check.IsNil)
 
        type testcase struct {
                sendLength   string
@@ -781,7 +783,8 @@ func (s *ServerRequiredSuite) TestPing(c *C) {
        kc, _ := runProxy(c, false, false, nil)
        defer closeListener()
 
-       rtr := MakeRESTRouter(kc, 10*time.Second, &arvados.Cluster{ManagementToken: arvadostest.ManagementToken}, log.New())
+       rtr, err := MakeRESTRouter(kc, 10*time.Second, &arvados.Cluster{ManagementToken: arvadostest.ManagementToken}, log.New())
+       c.Assert(err, check.IsNil)
 
        req, err := http.NewRequest("GET",
                "http://"+listener.Addr().String()+"/_health/ping",