20187: Check that the discovery doc is really a discovery doc.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index b3d850ac04a224767ac22556760a5534ae7b7fdb..c213c897c35dcaa734b5869ca6567047bf07a369 100644 (file)
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -171,7 +171,7 @@ func (h *Handler) setup() {
                Name: "arvados-controller",
        }
        h.cache = map[string]*cacheEnt{
-               "/discovery/v1/apis/arvados/v1/rest": &cacheEnt{},
+               "/discovery/v1/apis/arvados/v1/rest": &cacheEnt{validateDD: true},
        }
 
        go h.trashSweepWorker()
@@ -222,6 +222,7 @@ func (h *Handler) limitLogCreateRequests(w http.ResponseWriter, req *http.Reques
 // cacheEnt implements a basic stale-while-revalidate cache, suitable
 // for the Arvados discovery document.
 type cacheEnt struct {
+       validateDD   bool
        mtx          sync.Mutex
        header       http.Header
        body         []byte
@@ -277,7 +278,14 @@ func (ent *cacheEnt) refresh(path string, do func(*http.Request) (*http.Response
                        header[k] = v
                }
        }
-       if mediatype, _, err := mime.ParseMediaType(header.Get("Content-Type")); err == nil && mediatype == "application/json" {
+       if ent.validateDD {
+               var dd arvados.DiscoveryDocument
+               if err := json.Unmarshal(body, &dd); err != nil {
+                       return nil, nil, fmt.Errorf("error decoding JSON response: %w", err)
+               } else if dd.BasePath == "" {
+                       return nil, nil, errors.New("error in discovery document: no value for basePath")
+               }
+       } else if mediatype, _, err := mime.ParseMediaType(header.Get("Content-Type")); err == nil && mediatype == "application/json" {
                if !json.Valid(body) {
                        return nil, nil, errors.New("invalid JSON encoding in response")
                }

diff --git a/lib/controller/handler_test.go b/lib/controller/handler_test.go
index 4a7e1ad78acaf655748190a7bcc7f4a38d9aef10..02b72dc4595ae4f1d2f460eb839c3b059f4bbc0e 100644 (file)
--- a/lib/controller/handler_test.go
+++ b/lib/controller/handler_test.go
@@ -131,8 +131,12 @@ func (s *HandlerSuite) TestDiscoveryDocCache(c *check.C) {
                return &wg
        }
        clearCache := func() {
-               for path := range s.handler.cache {
-                       s.handler.cache[path] = &cacheEnt{}
+               for _, ent := range s.handler.cache {
+                       ent.refreshLock.Lock()
+                       ent.mtx.Lock()
+                       ent.body, ent.header, ent.refreshAfter = nil, nil, time.Time{}
+                       ent.mtx.Unlock()
+                       ent.refreshLock.Unlock()
                }
        }
        expireCache := func() {
@@ -188,11 +192,15 @@ func (s *HandlerSuite) TestDiscoveryDocCache(c *check.C) {
        }
        c.Check(countRailsReqs(), check.Equals, reqsBefore+1)
 
-       // Configure railsSpy to return an error when wantError==true.
-       var wantError bool
+       // Configure railsSpy to return an error or bad content
+       // depending on flags.
+       var wantError, wantBadContent bool
        s.railsSpy.Director = func(req *http.Request) {
                if wantError {
                        req.Method = "MAKE-COFFEE"
+               } else if wantBadContent {
+                       req.URL.Path = "/_health/ping"
+                       req.Header.Set("Authorization", "Bearer "+arvadostest.ManagementToken)
                }
        }
 
@@ -208,9 +216,16 @@ func (s *HandlerSuite) TestDiscoveryDocCache(c *check.C) {
        wg.Wait()
        c.Check(countRailsReqs(), check.Equals, reqsBefore+5)
 
+       // Response status is OK but body is not a discovery document
+       wantError = false
+       wantBadContent = true
+       reqsBefore = countRailsReqs()
+       c.Check(getDD(), check.Equals, http.StatusBadGateway)
+       c.Check(countRailsReqs(), check.Equals, reqsBefore+1)
+
        // Error condition clears => caller gets OK, cache is warmed
        // up
-       wantError = false
+       wantBadContent = false
        reqsBefore = countRailsReqs()
        getDDConcurrently(5, http.StatusOK, check.Commentf("success after errors at startup")).Wait()
        c.Check(countRailsReqs(), check.Equals, reqsBefore+1)