17830: Copies request's X-Request-Id header to response. Moves tests.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima@curii.com>

diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 44c99bf30f8c3a6ae9aa70b8306268b7c4c8fb6d..bd910aab433b43087d4791961cae0b812e47399f 100644 (file)
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -432,6 +432,42 @@ func (s *IntegrationSuite) TestCreateContainerRequestWithBadToken(c *check.C) {
        }
 }
 
+func (s *IntegrationSuite) TestRequestIDHeader(c *check.C) {
+       conn1 := s.testClusters["z1111"].Conn()
+       rootctx1, _, _ := s.testClusters["z1111"].RootClients()
+       _, ac1, _, _ := s.testClusters["z1111"].UserClients(rootctx1, c, conn1, "user@example.com", true)
+
+       tests := []struct {
+               path          string
+               reqIdProvided bool
+       }{
+               {"/arvados/v1/container_requests", false},
+               {"/arvados/v1/container_requests", true},
+               {"/arvados/v1/links", false},
+               {"/arvados/v1/links", true},
+       }
+
+       for _, tt := range tests {
+               c.Log(c.TestName() + " " + tt.path)
+               req, err := http.NewRequest("GET", "https://"+ac1.APIHost+tt.path, nil)
+               c.Assert(err, check.IsNil)
+               customReqId := "abcdeG"
+               if !tt.reqIdProvided {
+                       c.Assert(req.Header.Get("X-Request-Id"), check.Equals, "")
+               } else {
+                       req.Header.Set("X-Request-Id", customReqId)
+               }
+               resp, err := ac1.Do(req)
+               c.Assert(err, check.IsNil)
+               c.Check(resp.StatusCode, check.Equals, http.StatusOK)
+               if !tt.reqIdProvided {
+                       c.Check(resp.Header.Get("X-Request-Id"), check.Matches, "^req-[0-9a-zA-Z]{20}$")
+               } else {
+                       c.Check(resp.Header.Get("X-Request-Id"), check.Equals, customReqId)
+               }
+       }
+}
+
 // We test the direct access to the database
 // normally an integration test would not have a database access, but  in this case we need
 // to test tokens that are secret, so there is no API response that will give them back

diff --git a/lib/controller/router/response.go b/lib/controller/router/response.go
index 500fb307171c4f59442aca8cbf503092ee7232a6..35a4e5f1bfd24f466e1be858d8cd248602bc7771 100644 (file)
--- a/lib/controller/router/response.go
+++ b/lib/controller/router/response.go
@@ -57,7 +57,7 @@ func applySelectParam(selectParam []string, orig map[string]interface{}) map[str
        return selected
 }
 
-func (rtr *router) sendResponse(w http.ResponseWriter, req *http.Request, resp interface{}, opts responseOptions, reqId string) {
+func (rtr *router) sendResponse(w http.ResponseWriter, req *http.Request, resp interface{}, opts responseOptions) {
        var tmp map[string]interface{}
 
        if resp, ok := resp.(http.Handler); ok {
@@ -67,7 +67,7 @@ func (rtr *router) sendResponse(w http.ResponseWriter, req *http.Request, resp i
                return
        }
 
-       w.Header().Set("X-Request-Id", reqId)
+       w.Header().Set("X-Request-Id", req.Header.Get("X-Request-Id"))
        err := rtr.transcode(resp, &tmp)
        if err != nil {
                rtr.sendError(w, err)

diff --git a/lib/controller/router/router.go b/lib/controller/router/router.go
index 82e81d089b70625722ac99906ba5e4c74639fc56..5ceabbfb1d56fab171d8d4a8dfabca585f1362f6 100644 (file)
--- a/lib/controller/router/router.go
+++ b/lib/controller/router/router.go
@@ -505,12 +505,7 @@ func (rtr *router) addRoute(endpoint arvados.APIEndpoint, defaultOpts func() int
                        }
                }
                ctx := auth.NewContext(req.Context(), creds)
-               var reqId string
-               if reqId = req.Header.Get("X-Request-Id"); reqId == "" {
-                       reqIDGen := httpserver.IDGenerator{Prefix: "req-"}
-                       reqId = reqIDGen.Next()
-               }
-               ctx = arvados.ContextWithRequestID(ctx, reqId)
+               ctx = arvados.ContextWithRequestID(ctx, req.Header.Get("X-Request-Id"))
                logger.WithFields(logrus.Fields{
                        "apiEndpoint": endpoint,
                        "apiOptsType": fmt.Sprintf("%T", opts),
@@ -522,7 +517,7 @@ func (rtr *router) addRoute(endpoint arvados.APIEndpoint, defaultOpts func() int
                        rtr.sendError(w, err)
                        return
                }
-               rtr.sendResponse(w, req, resp, respOpts, reqId)
+               rtr.sendResponse(w, req, resp, respOpts)
        })
 }
 

diff --git a/lib/controller/router/router_test.go b/lib/controller/router/router_test.go
index 0e0edad3178197fc9bdceefa01c22a370767faf7..0330ec4252c9ad3ee8f461faf9ce7508c17bd3fc 100644 (file)
--- a/lib/controller/router/router_test.go
+++ b/lib/controller/router/router_test.go
@@ -370,35 +370,6 @@ func (s *RouterIntegrationSuite) TestHEAD(c *check.C) {
        c.Check(rr.Code, check.Equals, http.StatusOK)
 }
 
-func (s *RouterIntegrationSuite) TestRequestIDHeader(c *check.C) {
-       token := arvadostest.ActiveTokenV2
-       req := (&testReq{
-               method: "GET",
-               path:   "arvados/v1/collections/" + arvadostest.FooCollection,
-               token:  token,
-       }).Request()
-       rr := httptest.NewRecorder()
-       s.rtr.ServeHTTP(rr, req)
-       c.Check(rr.Code, check.Equals, http.StatusOK)
-       c.Check(rr.Result().Header.Get("X-Request-Id"), check.Matches, "^req-[0-9a-zA-Z]{20}$")
-}
-
-func (s *RouterIntegrationSuite) TestRequestIDHeaderProvidedByClient(c *check.C) {
-       token := arvadostest.ActiveTokenV2
-       req := (&testReq{
-               method: "GET",
-               path:   "arvados/v1/collections/" + arvadostest.FooCollection,
-               token:  token,
-               header: http.Header{
-                       "X-Request-Id": []string{"abcdeG"},
-               },
-       }).Request()
-       rr := httptest.NewRecorder()
-       s.rtr.ServeHTTP(rr, req)
-       c.Check(rr.Code, check.Equals, http.StatusOK)
-       c.Check(rr.Result().Header.Get("X-Request-Id"), check.Equals, "abcdeG")
-}
-
 func (s *RouterIntegrationSuite) TestRouteNotFound(c *check.C) {
        token := arvadostest.ActiveTokenV2
        req := (&testReq{