[ARVADOS] updated: 2.1.0-22-gba60b5b80

Git user git at public.arvados.org
Fri Oct 23 19:39:03 UTC 2020


Summary of changes:
 lib/controller/cmd.go              |   1 +
 lib/controller/fed_containers.go   |   1 -
 lib/controller/fed_generic.go      |   7 +-
 lib/controller/federation.go       |   6 ++
 lib/controller/federation_test.go  | 161 ++++---------------------------------
 lib/controller/handler_test.go     |   2 +
 lib/controller/integration_test.go | 143 ++++++++++++++++++++++++++++----
 lib/controller/localdb/conn.go     |  10 ++-
 lib/controller/router/response.go  |   2 +
 9 files changed, 161 insertions(+), 172 deletions(-)

       via  ba60b5b809bcc6bdb1b47c6b48b1977f62c8f4c6 (commit)
      from  c77a932d5ec11838ff447ed56570b87823b9dd04 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit ba60b5b809bcc6bdb1b47c6b48b1977f62c8f4c6
Author: Nico Cesar <nico at nicocesar.com>
Date:   Fri Oct 23 15:37:53 2020 -0400

    Moved some tests to integration_test.go
    
    This is an effort to have container_requests in the new codepath
    and the tests to be all integration tests.
    
    Arvados-DCO-1.1-Signed-off-by: <nico at curii.com>

diff --git a/lib/controller/cmd.go b/lib/controller/cmd.go
index 0c46e857b..c6b53eb8f 100644
--- a/lib/controller/cmd.go
+++ b/lib/controller/cmd.go
@@ -13,6 +13,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
+// Command intanciates a cmd.Handler with the service.Command used in package cmd
 var Command cmd.Handler = service.Command(arvados.ServiceNameController, newHandler)
 
 func newHandler(_ context.Context, cluster *arvados.Cluster, _ string, _ *prometheus.Registry) service.Handler {
diff --git a/lib/controller/fed_containers.go b/lib/controller/fed_containers.go
index c62cea116..68300786f 100644
--- a/lib/controller/fed_containers.go
+++ b/lib/controller/fed_containers.go
@@ -28,7 +28,6 @@ func remoteContainerRequestCreate(
 	if effectiveMethod != "POST" || uuid != "" || remainder != "" {
 		return false
 	}
-
 	// First make sure supplied token is valid.
 	creds := auth.NewCredentials()
 	creds.LoadTokensFromHTTPRequest(req)
diff --git a/lib/controller/fed_generic.go b/lib/controller/fed_generic.go
index 476fd97b0..59a401a0d 100644
--- a/lib/controller/fed_generic.go
+++ b/lib/controller/fed_generic.go
@@ -112,7 +112,6 @@ func (h *genericFederatedRequestHandler) remoteQueryUUIDs(w http.ResponseWriter,
 
 func (h *genericFederatedRequestHandler) handleMultiClusterQuery(w http.ResponseWriter,
 	req *http.Request, clusterId *string) bool {
-
 	var filters [][]interface{}
 	err := json.Unmarshal([]byte(req.Form.Get("filters")), &filters)
 	if err != nil {
@@ -143,7 +142,7 @@ func (h *genericFederatedRequestHandler) handleMultiClusterQuery(w http.Response
 					if u, ok := i.(string); ok && len(u) == 27 {
 						*clusterId = u[0:5]
 						queryClusters[u[0:5]] = append(queryClusters[u[0:5]], u)
-						expectCount += 1
+						expectCount++
 					}
 				}
 			}
@@ -151,7 +150,7 @@ func (h *genericFederatedRequestHandler) handleMultiClusterQuery(w http.Response
 			if u, ok := filter[2].(string); ok && len(u) == 27 {
 				*clusterId = u[0:5]
 				queryClusters[u[0:5]] = append(queryClusters[u[0:5]], u)
-				expectCount += 1
+				expectCount++
 			}
 		} else {
 			return false
@@ -295,6 +294,8 @@ func (h *genericFederatedRequestHandler) ServeHTTP(w http.ResponseWriter, req *h
 		uuid = m[1][1:]
 	}
 	for _, d := range h.delegates {
+		// Any delegate that was successful on dealing with the request will
+		// return true and probably sent the response in w
 		if d(h, effectiveMethod, &clusterId, uuid, m[3], w, req) {
 			return
 		}
diff --git a/lib/controller/federation.go b/lib/controller/federation.go
index aceaba808..1f91a39b3 100644
--- a/lib/controller/federation.go
+++ b/lib/controller/federation.go
@@ -164,6 +164,12 @@ func (h *Handler) validateAPItoken(req *http.Request, token string) (*CurrentUse
 		uuid = sp[1]
 		token = sp[2]
 	}
+	// REVIEW: this is covering the case of some "abcdefg" tokens, Tom lets talk about this approach when we do peer review
+	if len(token) < 41 {
+		ctxlog.FromContext(req.Context()).Debugf("validateAPItoken(%s): The lenght of the token is not 41", token)
+		return nil, false, nil
+	}
+
 	user.Authorization.APIToken = token
 	var scopes string
 	err = db.QueryRowContext(req.Context(), `SELECT api_client_authorizations.uuid, api_client_authorizations.scopes, users.uuid FROM api_client_authorizations JOIN users on api_client_authorizations.user_id=users.id WHERE api_token=$1 AND (expires_at IS NULL OR expires_at > current_timestamp AT TIME ZONE 'UTC') LIMIT 1`, token).Scan(&user.Authorization.UUID, &scopes, &user.UUID)
diff --git a/lib/controller/federation_test.go b/lib/controller/federation_test.go
index 6a9ad8c15..ee76237d1 100644
--- a/lib/controller/federation_test.go
+++ b/lib/controller/federation_test.go
@@ -206,7 +206,7 @@ func (s *FederationSuite) TestOptionsMethod(c *check.C) {
 }
 
 func (s *FederationSuite) TestRemoteWithTokenInQuery(c *check.C) {
-	req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+strings.Replace(arvadostest.WorkflowWithDefinitionYAMLUUID, "zzzzz-", "zmock-", 1)+"?api_token="+arvadostest.ActiveToken, nil)
+	req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+strings.Replace(arvadostest.WorkflowWithDefinitionYAMLUUID, "zzzzz-", "zmock-", 1)+"?api_token="+arvadostest.ActiveTokenV2, nil)
 	s.testRequest(req).Result()
 	c.Assert(s.remoteMockRequests, check.HasLen, 1)
 	pr := s.remoteMockRequests[0]
@@ -227,7 +227,7 @@ func (s *FederationSuite) TestLocalTokenSalted(c *check.C) {
 		c.Log("testing path ", path)
 		s.remoteMockRequests = nil
 		req := httptest.NewRequest("GET", path, nil)
-		req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+		req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
 		s.testRequest(req).Result()
 		c.Assert(s.remoteMockRequests, check.HasLen, 1)
 		pr := s.remoteMockRequests[0]
@@ -587,139 +587,6 @@ func (s *FederationSuite) TestUpdateRemoteContainerRequest(c *check.C) {
 	setPri(1) // Reset fixture so side effect doesn't break other tests.
 }
 
-func (s *FederationSuite) TestCreateContainerRequestBadToken(c *check.C) {
-	defer s.localServiceReturns404(c).Close()
-	// pass cluster_id via query parameter, this allows arvados-controller
-	// to avoid parsing the body
-	req := httptest.NewRequest("POST", "/arvados/v1/container_requests?cluster_id=zzzzz",
-		strings.NewReader(`{"container_request":{}}`))
-	req.Header.Set("Authorization", "Bearer abcdefg")
-	req.Header.Set("Content-type", "application/json")
-	resp := s.testRequest(req).Result()
-	c.Check(resp.StatusCode, check.Equals, http.StatusForbidden)
-	var e map[string][]string
-	c.Check(json.NewDecoder(resp.Body).Decode(&e), check.IsNil)
-	c.Check(e["errors"], check.DeepEquals, []string{"invalid API token"})
-}
-
-func (s *FederationSuite) TestCreateRemoteContainerRequest(c *check.C) {
-	defer s.localServiceReturns404(c).Close()
-	// pass cluster_id via query parameter, this allows arvados-controller
-	// to avoid parsing the body
-	req := httptest.NewRequest("POST", "/arvados/v1/container_requests?cluster_id=zzzzz",
-		strings.NewReader(`{
-  "container_request": {
-    "name": "hello world",
-    "state": "Uncommitted",
-    "output_path": "/",
-    "container_image": "123",
-    "command": ["abc"]
-  }
-}
-`))
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-	req.Header.Set("Content-type", "application/json")
-	resp := s.testRequest(req).Result()
-	c.Check(resp.StatusCode, check.Equals, http.StatusOK)
-	var cr arvados.ContainerRequest
-	c.Check(json.NewDecoder(resp.Body).Decode(&cr), check.IsNil)
-	c.Check(cr.Name, check.Equals, "hello world")
-	c.Check(strings.HasPrefix(cr.UUID, "zzzzz-"), check.Equals, true)
-}
-
-func (s *FederationSuite) TestCreateRemoteContainerRequestCheckRuntimeToken(c *check.C) {
-	// Send request to zmock and check that outgoing request has
-	// runtime_token set with a new random v2 token.
-
-	defer s.localServiceReturns404(c).Close()
-	// pass cluster_id via query parameter, this allows arvados-controller
-	// to avoid parsing the body
-	req := httptest.NewRequest("POST", "/arvados/v1/container_requests?cluster_id=zmock",
-		strings.NewReader(`{
-  "container_request": {
-    "name": "hello world",
-    "state": "Uncommitted",
-    "output_path": "/",
-    "container_image": "123",
-    "command": ["abc"]
-  }
-}
-`))
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
-	req.Header.Set("Content-type", "application/json")
-
-	arvadostest.SetServiceURL(&s.testHandler.Cluster.Services.RailsAPI, "https://"+os.Getenv("ARVADOS_TEST_API_HOST"))
-	s.testHandler.Cluster.ClusterID = "zzzzz"
-
-	resp := s.testRequest(req).Result()
-	c.Check(resp.StatusCode, check.Equals, http.StatusOK)
-	var cr struct {
-		arvados.ContainerRequest `json:"container_request"`
-	}
-	c.Check(json.NewDecoder(s.remoteMockRequests[0].Body).Decode(&cr), check.IsNil)
-	c.Check(strings.HasPrefix(cr.ContainerRequest.RuntimeToken, "v2/zzzzz-gj3su-"), check.Equals, true)
-	c.Check(cr.ContainerRequest.RuntimeToken, check.Not(check.Equals), arvadostest.ActiveTokenV2)
-}
-
-func (s *FederationSuite) TestCreateRemoteContainerRequestCheckSetRuntimeToken(c *check.C) {
-	// Send request to zmock and check that outgoing request has
-	// runtime_token set with the explicitly provided token.
-
-	defer s.localServiceReturns404(c).Close()
-	// pass cluster_id via query parameter, this allows arvados-controller
-	// to avoid parsing the body
-	req := httptest.NewRequest("POST", "/arvados/v1/container_requests?cluster_id=zmock",
-		strings.NewReader(`{
-  "container_request": {
-    "name": "hello world",
-    "state": "Uncommitted",
-    "output_path": "/",
-    "container_image": "123",
-    "command": ["abc"],
-    "runtime_token": "xyz"
-  }
-}
-`))
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-	req.Header.Set("Content-type", "application/json")
-	resp := s.testRequest(req).Result()
-	c.Check(resp.StatusCode, check.Equals, http.StatusOK)
-	var cr struct {
-		arvados.ContainerRequest `json:"container_request"`
-	}
-	c.Check(json.NewDecoder(s.remoteMockRequests[0].Body).Decode(&cr), check.IsNil)
-	c.Check(cr.ContainerRequest.RuntimeToken, check.Equals, "xyz")
-}
-
-func (s *FederationSuite) TestCreateRemoteContainerRequestRuntimeTokenFromAuth(c *check.C) {
-	// Send request to zmock and check that outgoing request has
-	// runtime_token set using the Auth token because the user is remote.
-
-	defer s.localServiceReturns404(c).Close()
-	// pass cluster_id via query parameter, this allows arvados-controller
-	// to avoid parsing the body
-	req := httptest.NewRequest("POST", "/arvados/v1/container_requests?cluster_id=zmock",
-		strings.NewReader(`{
-  "container_request": {
-    "name": "hello world",
-    "state": "Uncommitted",
-    "output_path": "/",
-    "container_image": "123",
-    "command": ["abc"]
-  }
-}
-`))
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2+"/zzzzz-dz642-parentcontainer")
-	req.Header.Set("Content-type", "application/json")
-	resp := s.testRequest(req).Result()
-	c.Check(resp.StatusCode, check.Equals, http.StatusOK)
-	var cr struct {
-		arvados.ContainerRequest `json:"container_request"`
-	}
-	c.Check(json.NewDecoder(s.remoteMockRequests[0].Body).Decode(&cr), check.IsNil)
-	c.Check(cr.ContainerRequest.RuntimeToken, check.Equals, arvadostest.ActiveTokenV2)
-}
-
 func (s *FederationSuite) TestCreateRemoteContainerRequestError(c *check.C) {
 	defer s.localServiceReturns404(c).Close()
 	// pass cluster_id via query parameter, this allows arvados-controller
@@ -735,7 +602,7 @@ func (s *FederationSuite) TestCreateRemoteContainerRequestError(c *check.C) {
   }
 }
 `))
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
 	req.Header.Set("Content-type", "application/json")
 	resp := s.testRequest(req).Result()
 	c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
@@ -744,7 +611,7 @@ func (s *FederationSuite) TestCreateRemoteContainerRequestError(c *check.C) {
 func (s *FederationSuite) TestGetRemoteContainer(c *check.C) {
 	defer s.localServiceReturns404(c).Close()
 	req := httptest.NewRequest("GET", "/arvados/v1/containers/"+arvadostest.QueuedContainerUUID, nil)
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
 	resp := s.testRequest(req)
 	c.Check(resp.Code, check.Equals, http.StatusOK)
 	var cn arvados.Container
@@ -756,7 +623,7 @@ func (s *FederationSuite) TestListRemoteContainer(c *check.C) {
 	defer s.localServiceReturns404(c).Close()
 	req := httptest.NewRequest("GET", "/arvados/v1/containers?count=none&filters="+
 		url.QueryEscape(fmt.Sprintf(`[["uuid", "in", ["%v"]]]`, arvadostest.QueuedContainerUUID)), nil)
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
 	resp := s.testRequest(req).Result()
 	c.Check(resp.StatusCode, check.Equals, http.StatusOK)
 	var cn arvados.ContainerList
@@ -777,7 +644,7 @@ func (s *FederationSuite) TestListMultiRemoteContainers(c *check.C) {
 			arvadostest.QueuedContainerUUID)),
 		url.QueryEscape(`["uuid", "command"]`)),
 		nil)
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
 	resp := s.testRequest(req).Result()
 	c.Check(resp.StatusCode, check.Equals, http.StatusOK)
 	var cn arvados.ContainerList
@@ -820,7 +687,7 @@ func (s *FederationSuite) TestListMultiRemoteContainersPaged(c *check.C) {
 			w.WriteHeader(200)
 			w.Write([]byte(`{"kind": "arvados#containerList", "items": [{"uuid": "zhome-xvhdp-cr6queuedcontnr", "command": ["efg"]}]}`))
 		}
-		callCount += 1
+		callCount++
 	})).Close()
 	req := httptest.NewRequest("GET", fmt.Sprintf("/arvados/v1/containers?count=none&filters=%s",
 		url.QueryEscape(fmt.Sprintf(`[["uuid", "in", ["%v", "zhome-xvhdp-cr5queuedcontnr", "zhome-xvhdp-cr6queuedcontnr"]]]`,
@@ -856,13 +723,13 @@ func (s *FederationSuite) TestListMultiRemoteContainersMissing(c *check.C) {
 			w.WriteHeader(200)
 			w.Write([]byte(`{"kind": "arvados#containerList", "items": []}`))
 		}
-		callCount += 1
+		callCount++
 	})).Close()
 	req := httptest.NewRequest("GET", fmt.Sprintf("/arvados/v1/containers?count=none&filters=%s",
 		url.QueryEscape(fmt.Sprintf(`[["uuid", "in", ["%v", "zhome-xvhdp-cr5queuedcontnr", "zhome-xvhdp-cr6queuedcontnr"]]]`,
 			arvadostest.QueuedContainerUUID))),
 		nil)
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
 	resp := s.testRequest(req).Result()
 	c.Check(resp.StatusCode, check.Equals, http.StatusOK)
 	c.Check(callCount, check.Equals, 2)
@@ -883,7 +750,7 @@ func (s *FederationSuite) TestListMultiRemoteContainerPageSizeError(c *check.C)
 		url.QueryEscape(fmt.Sprintf(`[["uuid", "in", ["%v", "zhome-xvhdp-cr5queuedcontnr"]]]`,
 			arvadostest.QueuedContainerUUID))),
 		nil)
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
 	resp := s.testRequest(req).Result()
 	c.Check(resp.StatusCode, check.Equals, http.StatusBadRequest)
 	s.checkJSONErrorMatches(c, resp, `Federated multi-object request for 2 objects which is more than max page size 1.`)
@@ -894,7 +761,7 @@ func (s *FederationSuite) TestListMultiRemoteContainerLimitError(c *check.C) {
 		url.QueryEscape(fmt.Sprintf(`[["uuid", "in", ["%v", "zhome-xvhdp-cr5queuedcontnr"]]]`,
 			arvadostest.QueuedContainerUUID))),
 		nil)
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
 	resp := s.testRequest(req).Result()
 	c.Check(resp.StatusCode, check.Equals, http.StatusBadRequest)
 	s.checkJSONErrorMatches(c, resp, `Federated multi-object may not provide 'limit', 'offset' or 'order'.`)
@@ -905,7 +772,7 @@ func (s *FederationSuite) TestListMultiRemoteContainerOffsetError(c *check.C) {
 		url.QueryEscape(fmt.Sprintf(`[["uuid", "in", ["%v", "zhome-xvhdp-cr5queuedcontnr"]]]`,
 			arvadostest.QueuedContainerUUID))),
 		nil)
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
 	resp := s.testRequest(req).Result()
 	c.Check(resp.StatusCode, check.Equals, http.StatusBadRequest)
 	s.checkJSONErrorMatches(c, resp, `Federated multi-object may not provide 'limit', 'offset' or 'order'.`)
@@ -916,7 +783,7 @@ func (s *FederationSuite) TestListMultiRemoteContainerOrderError(c *check.C) {
 		url.QueryEscape(fmt.Sprintf(`[["uuid", "in", ["%v", "zhome-xvhdp-cr5queuedcontnr"]]]`,
 			arvadostest.QueuedContainerUUID))),
 		nil)
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
 	resp := s.testRequest(req).Result()
 	c.Check(resp.StatusCode, check.Equals, http.StatusBadRequest)
 	s.checkJSONErrorMatches(c, resp, `Federated multi-object may not provide 'limit', 'offset' or 'order'.`)
@@ -928,7 +795,7 @@ func (s *FederationSuite) TestListMultiRemoteContainerSelectError(c *check.C) {
 			arvadostest.QueuedContainerUUID)),
 		url.QueryEscape(`["command"]`)),
 		nil)
-	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
 	resp := s.testRequest(req).Result()
 	c.Check(resp.StatusCode, check.Equals, http.StatusBadRequest)
 	s.checkJSONErrorMatches(c, resp, `Federated multi-object request must include 'uuid' in 'select'`)
diff --git a/lib/controller/handler_test.go b/lib/controller/handler_test.go
index 7d8266a85..d12e4fa33 100644
--- a/lib/controller/handler_test.go
+++ b/lib/controller/handler_test.go
@@ -294,6 +294,8 @@ func (s *HandlerSuite) CheckObjectType(c *check.C, url string, token string, ski
 	}
 	resp2, err := client.Get(s.cluster.Services.RailsAPI.ExternalURL.String() + url + "/?api_token=" + token)
 	c.Check(err, check.Equals, nil)
+	c.Assert(resp2.StatusCode, check.Equals, http.StatusOK,
+		check.Commentf("Wasn't able to get data from the RailsAPI at %q", url))
 	defer resp2.Body.Close()
 	db, err := ioutil.ReadAll(resp2.Body)
 	c.Check(err, check.Equals, nil)
diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index cc0b697d4..5ec9d64f2 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -134,19 +134,6 @@ func (s *IntegrationSuite) SetUpSuite(c *check.C) {
 	}
 }
 
-func (s *IntegrationSuite) TestDatabaseConnection(c *check.C) {
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	db, err := sql.Open("postgres", s.testClusters["z1111"].super.Cluster().PostgreSQL.Connection.String())
-	c.Assert(err, check.IsNil)
-	defer db.Close()
-	conn, err := db.Conn(ctx)
-	c.Assert(err, check.IsNil)
-	defer conn.Close()
-	_, err = conn.ExecContext(ctx, `SELECT 1`)
-	c.Assert(err, check.IsNil)
-}
-
 func (s *IntegrationSuite) TearDownSuite(c *check.C) {
 	for _, c := range s.testClusters {
 		c.super.Stop()
@@ -368,6 +355,7 @@ func (s *IntegrationSuite) TestCreateContainerRequestWithFedToken(c *check.C) {
 	c.Assert(err, check.IsNil)
 	req.Header.Set("Content-Type", "application/json")
 	err = ac2.DoAndDecode(&cr, req)
+	c.Assert(err, check.IsNil)
 	c.Logf("err == %#v", err)
 
 	c.Log("...get user with good token")
@@ -394,10 +382,131 @@ func (s *IntegrationSuite) TestCreateContainerRequestWithFedToken(c *check.C) {
 	req.Header.Set("Content-Type", "application/json")
 	req.Header.Set("Authorization", "OAuth2 "+ac2.AuthToken)
 	resp, err = arvados.InsecureHTTPClient.Do(req)
-	if c.Check(err, check.IsNil) {
-		err = json.NewDecoder(resp.Body).Decode(&cr)
-		c.Check(err, check.IsNil)
-		c.Check(cr.UUID, check.Matches, "z2222-.*")
+	c.Assert(err, check.IsNil)
+	err = json.NewDecoder(resp.Body).Decode(&cr)
+	c.Check(err, check.IsNil)
+	c.Check(cr.UUID, check.Matches, "z2222-.*")
+}
+
+func (s *IntegrationSuite) TestCreateContainerRequestWithBadToken(c *check.C) {
+	var (
+		body bytes.Buffer
+		resp *http.Response
+	)
+
+	conn1 := s.conn("z1111")
+	rootctx1, _, _ := s.rootClients("z1111")
+	_, ac1, _, au := s.userClients(rootctx1, c, conn1, "z1111", true)
+
+	tests := []struct {
+		name         string
+		token        string
+		expectedCode int
+	}{
+		{"Good token", ac1.AuthToken, http.StatusOK},
+		{"Bogus token", "abcdef", http.StatusUnauthorized},
+		{"v1-looking token", "badtoken00badtoken00badtoken00badtoken00b", http.StatusUnauthorized},
+		{"v2-looking token", "v2/" + au.UUID + "/badtoken00badtoken00badtoken00badtoken00b", http.StatusUnauthorized},
+	}
+
+	json.NewEncoder(&body).Encode(map[string]interface{}{
+		"container_request": map[string]interface{}{
+			"command":         []string{"echo"},
+			"container_image": "d41d8cd98f00b204e9800998ecf8427e+0",
+			"cwd":             "/",
+			"output_path":     "/",
+		},
+	})
+
+	for _, tt := range tests {
+		c.Log(c.TestName() + " " + tt.name)
+		ac1.AuthToken = tt.token
+		req, err := http.NewRequest("POST", "https://"+ac1.APIHost+"/arvados/v1/container_requests", bytes.NewReader(body.Bytes()))
+		c.Assert(err, check.IsNil)
+		req.Header.Set("Content-Type", "application/json")
+		resp, err = ac1.Do(req)
+		c.Assert(err, check.IsNil)
+		c.Assert(resp.StatusCode, check.Equals, tt.expectedCode)
+	}
+}
+
+// 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
+func (s *IntegrationSuite) TestDatabaseConnection(c *check.C) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	db, err := sql.Open("postgres", s.testClusters["z1111"].super.Cluster().PostgreSQL.Connection.String())
+	c.Assert(err, check.IsNil)
+	defer db.Close()
+	conn, err := db.Conn(ctx)
+	c.Assert(err, check.IsNil)
+	defer conn.Close()
+	rows, err := conn.ExecContext(ctx, `SELECT 1`)
+	c.Assert(err, check.IsNil)
+	n, err := rows.RowsAffected()
+	c.Assert(err, check.IsNil)
+	c.Assert(n, check.Equals, int64(1))
+}
+
+func (s *IntegrationSuite) TestRuntimeTokenInCR(c *check.C) {
+	conn1 := s.conn("z1111")
+	rootctx1, _, _ := s.rootClients("z1111")
+	_, ac1, _, au := s.userClients(rootctx1, c, conn1, "z1111", true)
+
+	tests := []struct {
+		name                 string
+		token                string
+		expectAToGetAValidCR bool
+		expectedToken        *string
+	}{
+		{"Good token z1111 user", ac1.AuthToken, true, &ac1.AuthToken},
+		{"Bogus token", "abcdef", false, nil},
+		{"v1-looking token", "badtoken00badtoken00badtoken00badtoken00b", false, nil},
+		{"v2-looking token", "v2/" + au.UUID + "/badtoken00badtoken00badtoken00badtoken00b", false, nil},
+	}
+
+	for _, tt := range tests {
+		c.Log(c.TestName() + " " + tt.name)
+
+		rq := map[string]interface{}{
+			"command":         []string{"echo"},
+			"container_image": "d41d8cd98f00b204e9800998ecf8427e+0",
+			"cwd":             "/",
+			"output_path":     "/",
+			"runtime_token":   tt.token,
+		}
+		cr, err := conn1.ContainerRequestCreate(rootctx1, arvados.CreateOptions{Attrs: rq})
+		if tt.expectAToGetAValidCR {
+			c.Assert(err, check.IsNil)
+			c.Assert(cr, check.NotNil)
+			c.Assert(cr.UUID, check.Not(check.Equals), "")
+		}
+
+		if tt.expectedToken == nil {
+			break
+		}
+
+		ctx2, cancel2 := context.WithCancel(context.Background())
+		defer cancel2()
+
+		db, err := sql.Open("postgres", s.testClusters["z1111"].super.Cluster().PostgreSQL.Connection.String())
+		c.Assert(err, check.IsNil)
+		defer db.Close()
+
+		conn, err := db.Conn(ctx2)
+		c.Assert(err, check.IsNil)
+		defer conn.Close()
+
+		c.Logf("cr.UUID: %s", cr.UUID)
+		row := conn.QueryRowContext(ctx2, `SELECT runtime_token from container_requests where uuid=$1`, cr.UUID)
+		c.Assert(row, check.NotNil)
+		// runtimeToken is *string and not a string because the database has a NULL column for this
+		var runtimeToken *string
+		err = row.Scan(&runtimeToken)
+		c.Assert(err, check.IsNil)
+		c.Assert(runtimeToken, check.NotNil)
+		c.Assert(*runtimeToken, check.DeepEquals, *tt.expectedToken)
 	}
 }
 
diff --git a/lib/controller/localdb/conn.go b/lib/controller/localdb/conn.go
index 98ec6d59d..73bed2184 100644
--- a/lib/controller/localdb/conn.go
+++ b/lib/controller/localdb/conn.go
@@ -31,15 +31,17 @@ func NewConn(cluster *arvados.Cluster) *Conn {
 	return &conn
 }
 
+// Logout handles the logout of conn giving to the appropiate loginController
 func (conn *Conn) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
-	return conn.railsProxy.Logout(ctx, opts) // REVIEW: will this handle return_to?
+	return conn.loginController.Logout(ctx, opts)
 }
 
+// Login handles the logout of conn giving to the appropiate loginController
 func (conn *Conn) Login(ctx context.Context, opts arvados.LoginOptions) (arvados.LoginResponse, error) {
-	return conn.railsProxy.Login(ctx, opts)
+	return conn.loginController.Login(ctx, opts)
 }
 
+// UserAuthenticate handles the User Authentication of conn giving to the appropiate loginController
 func (conn *Conn) UserAuthenticate(ctx context.Context, opts arvados.UserAuthenticateOptions) (arvados.APIClientAuthorization, error) {
-	// REVIEW:Should this be conn.railsProxy?
-	return conn.railsProxy.UserAuthenticate(ctx, opts)
+	return conn.loginController.UserAuthenticate(ctx, opts)
 }
diff --git a/lib/controller/router/response.go b/lib/controller/router/response.go
index 1217d11d0..055595c8e 100644
--- a/lib/controller/router/response.go
+++ b/lib/controller/router/response.go
@@ -133,6 +133,8 @@ func (rtr *router) sendResponse(w http.ResponseWriter, req *http.Request, resp i
 			}
 		}
 		switch k {
+		// in all this cases, RoR returns nil instead the Zero value for the type.
+		// Maytbe, this should all go away when RoR is out of the picture.
 		case "output_uuid", "output_name", "log_uuid", "modified_by_client_uuid", "description", "requesting_container_uuid", "expires_at":
 			if v == "" {
 				tmp[k] = nil

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list