17014: controller handles container requests
authorNico Cesar <nico@nicocesar.com>
Thu, 7 Jan 2021 16:23:21 +0000 (11:23 -0500)
committerNico Cesar <nico@nicocesar.com>
Thu, 21 Jan 2021 01:38:24 +0000 (20:38 -0500)
This commit has:
  * Container Requests in the new codepath
  * runtime_constraints and scheduling parameters have now default
    values and cannot be nil. This affects Railsapi CR comparison
    in Final state
  * Adapted Login and Logout so they work for CR
  * Tests in  lib/controller/integration_test.go that take in account
    runtime_token with intermediate clusters
  * Some tests in  lib/controller/federation_test.go had to be addapted,
    but for the most part remained as is to gurantee compatibilty
  * Added the check that SystemRootToken has to be non-empty
  * Railsapi fixtures that include finalized CR that are used in the tests to
    make sure we return the right object
  * Minimal changes in the documentation to reflect all the user visible
    changes
  * Added expires_at parameter for login and related test

Arvados-DCO-1.1-Signed-off-by: Nico Cesar <nico@curii.com>

37 files changed:
apps/workbench/test/controllers/container_requests_controller_test.rb
doc/api/methods/container_requests.html.textile.liquid
lib/boot/supervisor.go
lib/controller/cmd.go
lib/controller/federation/conn.go
lib/controller/federation/generate.go
lib/controller/federation/generated.go
lib/controller/federation_test.go
lib/controller/handler.go
lib/controller/handler_test.go
lib/controller/integration_test.go
lib/controller/localdb/conn.go
lib/controller/localdb/login.go
lib/controller/localdb/login_ldap.go
lib/controller/localdb/login_ldap_test.go
lib/controller/localdb/login_oidc.go
lib/controller/localdb/login_pam.go
lib/controller/localdb/login_pam_test.go
lib/controller/localdb/login_testuser.go
lib/controller/localdb/login_testuser_test.go
lib/controller/router/response.go
lib/controller/router/router.go
lib/controller/rpc/conn.go
lib/crunchrun/crunchrun.go
lib/crunchrun/crunchrun_test.go
sdk/go/arvados/api.go
sdk/go/arvados/container.go
sdk/go/arvadostest/api.go
services/api/app/controllers/user_sessions_controller.rb
services/api/app/models/arvados_model.rb
services/api/app/models/container.rb
services/api/app/models/container_request.rb
services/api/test/fixtures/container_requests.yml
services/api/test/functional/arvados/v1/container_requests_controller_test.rb
services/api/test/functional/user_sessions_controller_test.rb
services/api/test/unit/container_request_test.rb
services/api/test/unit/container_test.rb

index 73d357f3a60f6a9da27db76a452a5ded6b0e3bd8..c8709df3c35154e172898e060a9cb526196ef064 100644 (file)
@@ -138,7 +138,6 @@ class ContainerRequestsControllerTest < ActionController::TestCase
     assert_includes @response.body, "href=\"\/collections/fa7aeb5140e2848d39b416daeef4ffc5+45/foobar\?" # locator on command
     assert_includes @response.body, "href=\"\/collections/fa7aeb5140e2848d39b416daeef4ffc5+45/foo" # mount input1
     assert_includes @response.body, "href=\"\/collections/fa7aeb5140e2848d39b416daeef4ffc5+45/bar" # mount input2
-    assert_includes @response.body, "href=\"\/collections/f9ddda46bb293b6847da984e3aa735db+290" # mount workflow
     assert_includes @response.body, "href=\"#Log\""
     assert_includes @response.body, "href=\"#Provenance\""
   end
index cd566f5ce42dcf29edab62c6d38a103f9737ba94..b24a24e0674b9a6f01e7463072e8ccd18ed15213 100644 (file)
@@ -49,7 +49,7 @@ table(table table-bordered table-condensed).
 |cwd|string|Initial working directory, given as an absolute path (in the container) or a path relative to the WORKDIR given in the image's Dockerfile.|Required.|
 |command|array of strings|Command to execute in the container.|Required. e.g., @["echo","hello"]@|
 |output_path|string|Path to a directory or file inside the container that should be preserved as container's output when it finishes. This path must be one of the mount targets. For best performance, point output_path to a writable collection mount.  See "Pre-populate output using Mount points":#pre-populate-output for details regarding optional output pre-population using mount points and "Symlinks in output":#symlinks-in-output for additional details.|Required.|
-|output_name|string|Desired name for the output collection. If null, a name will be assigned automatically.||
+|output_name|string|Desired name for the output collection. If null or empty, a name will be assigned automatically.||
 |output_ttl|integer|Desired lifetime for the output collection, in seconds. If zero, the output collection will not be deleted automatically.||
 |priority|integer|Range 0-1000.  Indicate scheduling order preference.|Clients are expected to submit container requests with zero priority in order to preview the container that will be used to satisfy it. Priority can be null if and only if state!="Committed".  See "below for more details":#priority .|
 |expires_at|datetime|After this time, priority is considered to be zero.|Not yet implemented.|
index f2e715a76669ed7ea1db21a7720b3fbd382cb8a3..752466c2a2030d9ff5db66bbb49cfaf1e1646153 100644 (file)
@@ -59,6 +59,8 @@ type Supervisor struct {
        environ    []string // for child processes
 }
 
+func (super *Supervisor) Cluster() *arvados.Cluster { return super.cluster }
+
 func (super *Supervisor) Start(ctx context.Context, cfg *arvados.Config, cfgPath string) {
        super.ctx, super.cancel = context.WithCancel(ctx)
        super.done = make(chan struct{})
index 0c46e857b3aa8604743575a5f0be37f022e6297c..7ab7f5305b4fe83113d1a47f499f7d3eb8298804 100644 (file)
@@ -13,6 +13,7 @@ import (
        "github.com/prometheus/client_golang/prometheus"
 )
 
+// Command starts a controller service. See cmd/arvados-server/cmd.go
 var Command cmd.Handler = service.Command(arvados.ServiceNameController, newHandler)
 
 func newHandler(_ context.Context, cluster *arvados.Cluster, _ string, _ *prometheus.Registry) service.Handler {
index 130368124cdd904a40ceb3938122181594c26804..00523c7826a74331ea8c1560013c40aebca86f3d 100644 (file)
@@ -336,6 +336,68 @@ func (conn *Conn) ContainerUnlock(ctx context.Context, options arvados.GetOption
        return conn.chooseBackend(options.UUID).ContainerUnlock(ctx, options)
 }
 
+func (conn *Conn) ContainerRequestList(ctx context.Context, options arvados.ListOptions) (arvados.ContainerRequestList, error) {
+       return conn.generated_ContainerRequestList(ctx, options)
+}
+
+func (conn *Conn) ContainerRequestCreate(ctx context.Context, options arvados.CreateOptions) (arvados.ContainerRequest, error) {
+       be := conn.chooseBackend(options.ClusterID)
+       if be == conn.local {
+               return be.ContainerRequestCreate(ctx, options)
+       }
+       if _, ok := options.Attrs["runtime_token"]; !ok {
+               // If runtime_token is not set, create a new token
+               aca, err := conn.local.APIClientAuthorizationCurrent(ctx, arvados.GetOptions{})
+               if err != nil {
+                       // This should probably be StatusUnauthorized
+                       // (need to update test in
+                       // lib/controller/federation_test.go):
+                       // When RoR is out of the picture this should be:
+                       // return arvados.ContainerRequest{}, httpErrorf(http.StatusUnauthorized, "%w", err)
+                       return arvados.ContainerRequest{}, httpErrorf(http.StatusForbidden, "%s", "invalid API token")
+               }
+               user, err := conn.local.UserGetCurrent(ctx, arvados.GetOptions{})
+               if err != nil {
+                       return arvados.ContainerRequest{}, err
+               }
+               if len(aca.Scopes) == 0 || aca.Scopes[0] != "all" {
+                       return arvados.ContainerRequest{}, httpErrorf(http.StatusForbidden, "token scope is not [all]")
+               }
+               if strings.HasPrefix(aca.UUID, conn.cluster.ClusterID) {
+                       // Local user, submitting to a remote cluster.
+                       // Create a new time-limited token.
+                       local, ok := conn.local.(*localdb.Conn)
+                       if !ok {
+                               return arvados.ContainerRequest{}, httpErrorf(http.StatusInternalServerError, "bug: local backend is a %T, not a *localdb.Conn", conn.local)
+                       }
+                       aca, err = local.CreateAPIClientAuthorization(ctx, conn.cluster.SystemRootToken, rpc.UserSessionAuthInfo{UserUUID: user.UUID,
+                               ExpiresAt: time.Now().UTC().Add(conn.cluster.Collections.BlobSigningTTL.Duration())})
+                       if err != nil {
+                               return arvados.ContainerRequest{}, err
+                       }
+                       options.Attrs["runtime_token"] = aca.TokenV2()
+               } else {
+                       // Remote user. Container request will use the
+                       // current token, minus the trailing portion
+                       // (optional container uuid).
+                       options.Attrs["runtime_token"] = aca.TokenV2()
+               }
+       }
+       return be.ContainerRequestCreate(ctx, options)
+}
+
+func (conn *Conn) ContainerRequestUpdate(ctx context.Context, options arvados.UpdateOptions) (arvados.ContainerRequest, error) {
+       return conn.chooseBackend(options.UUID).ContainerRequestUpdate(ctx, options)
+}
+
+func (conn *Conn) ContainerRequestGet(ctx context.Context, options arvados.GetOptions) (arvados.ContainerRequest, error) {
+       return conn.chooseBackend(options.UUID).ContainerRequestGet(ctx, options)
+}
+
+func (conn *Conn) ContainerRequestDelete(ctx context.Context, options arvados.DeleteOptions) (arvados.ContainerRequest, error) {
+       return conn.chooseBackend(options.UUID).ContainerRequestDelete(ctx, options)
+}
+
 func (conn *Conn) SpecimenList(ctx context.Context, options arvados.ListOptions) (arvados.SpecimenList, error) {
        return conn.generated_SpecimenList(ctx, options)
 }
index ab5d9966a4409479ec2bd1725e14a629c1770f12..9ce7fdcb21f6aa19b264b57982facaeee39305f9 100644 (file)
@@ -52,7 +52,7 @@ func main() {
                defer out.Close()
                out.Write(regexp.MustCompile(`(?ms)^.*package .*?import.*?\n\)\n`).Find(buf))
                io.WriteString(out, "//\n// -- this file is auto-generated -- do not edit -- edit list.go and run \"go generate\" instead --\n//\n\n")
-               for _, t := range []string{"Container", "Specimen", "User"} {
+               for _, t := range []string{"Container", "ContainerRequest", "Specimen", "User"} {
                        _, err := out.Write(bytes.ReplaceAll(orig, []byte("Collection"), []byte(t)))
                        if err != nil {
                                panic(err)
index 8745f3b9730b068faa2cc9e8a02d4c5638c7164a..ab9db93a4d105a17572beee6080cd5b13f2755c9 100755 (executable)
@@ -58,6 +58,47 @@ func (conn *Conn) generated_ContainerList(ctx context.Context, options arvados.L
        return merged, err
 }
 
+func (conn *Conn) generated_ContainerRequestList(ctx context.Context, options arvados.ListOptions) (arvados.ContainerRequestList, error) {
+       var mtx sync.Mutex
+       var merged arvados.ContainerRequestList
+       var needSort atomic.Value
+       needSort.Store(false)
+       err := conn.splitListRequest(ctx, options, func(ctx context.Context, _ string, backend arvados.API, options arvados.ListOptions) ([]string, error) {
+               options.ForwardedFor = conn.cluster.ClusterID + "-" + options.ForwardedFor
+               cl, err := backend.ContainerRequestList(ctx, options)
+               if err != nil {
+                       return nil, err
+               }
+               mtx.Lock()
+               defer mtx.Unlock()
+               if len(merged.Items) == 0 {
+                       merged = cl
+               } else if len(cl.Items) > 0 {
+                       merged.Items = append(merged.Items, cl.Items...)
+                       needSort.Store(true)
+               }
+               uuids := make([]string, 0, len(cl.Items))
+               for _, item := range cl.Items {
+                       uuids = append(uuids, item.UUID)
+               }
+               return uuids, nil
+       })
+       if needSort.Load().(bool) {
+               // Apply the default/implied order, "modified_at desc"
+               sort.Slice(merged.Items, func(i, j int) bool {
+                       mi, mj := merged.Items[i].ModifiedAt, merged.Items[j].ModifiedAt
+                       return mj.Before(mi)
+               })
+       }
+       if merged.Items == nil {
+               // Return empty results as [], not null
+               // (https://github.com/golang/go/issues/27589 might be
+               // a better solution in the future)
+               merged.Items = []arvados.ContainerRequest{}
+       }
+       return merged, err
+}
+
 func (conn *Conn) generated_SpecimenList(ctx context.Context, options arvados.ListOptions) (arvados.SpecimenList, error) {
        var mtx sync.Mutex
        var merged arvados.SpecimenList
index 031166b29151d3023ae386b34ffdb29afc521728..a92fc71053cea1f5ff3adfa4c5db0fca5c84576b 100644 (file)
@@ -352,7 +352,13 @@ func (s *FederationSuite) localServiceReturns404(c *check.C) *httpserver.Server
        return s.localServiceHandler(c, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
                if req.URL.Path == "/arvados/v1/api_client_authorizations/current" {
                        if req.Header.Get("Authorization") == "Bearer "+arvadostest.ActiveToken {
-                               json.NewEncoder(w).Encode(arvados.APIClientAuthorization{UUID: arvadostest.ActiveTokenUUID, APIToken: arvadostest.ActiveToken})
+                               json.NewEncoder(w).Encode(arvados.APIClientAuthorization{UUID: arvadostest.ActiveTokenUUID, APIToken: arvadostest.ActiveToken, Scopes: []string{"all"}})
+                       } else {
+                               w.WriteHeader(http.StatusUnauthorized)
+                       }
+               } else if req.URL.Path == "/arvados/v1/users/current" {
+                       if req.Header.Get("Authorization") == "Bearer "+arvadostest.ActiveToken {
+                               json.NewEncoder(w).Encode(arvados.User{UUID: arvadostest.ActiveUserUUID})
                        } else {
                                w.WriteHeader(http.StatusUnauthorized)
                        }
@@ -627,38 +633,78 @@ func (s *FederationSuite) TestCreateRemoteContainerRequest(c *check.C) {
        c.Check(strings.HasPrefix(cr.UUID, "zzzzz-"), check.Equals, true)
 }
 
+// getCRfromMockRequest returns a ContainerRequest with the content of the
+// request sent to the remote mock. This function takes into account the
+// Content-Type and acts accordingly.
+func (s *FederationSuite) getCRfromMockRequest(c *check.C) arvados.ContainerRequest {
+
+       // Body can be a json formated or something like:
+       //  cluster_id=zmock&container_request=%7B%22command%22%3A%5B%22abc%22%5D%2C%22container_image%22%3A%22123%22%2C%22...7D
+       // or:
+       //  "{\"container_request\":{\"command\":[\"abc\"],\"container_image\":\"12...Uncommitted\"}}"
+
+       var cr arvados.ContainerRequest
+       data, err := ioutil.ReadAll(s.remoteMockRequests[0].Body)
+       c.Check(err, check.IsNil)
+
+       if s.remoteMockRequests[0].Header.Get("Content-Type") == "application/json" {
+               // legacy code path sends a JSON request body
+               var answerCR struct {
+                       ContainerRequest arvados.ContainerRequest `json:"container_request"`
+               }
+               c.Check(json.Unmarshal(data, &answerCR), check.IsNil)
+               cr = answerCR.ContainerRequest
+       } else if s.remoteMockRequests[0].Header.Get("Content-Type") == "application/x-www-form-urlencoded" {
+               // new code path sends a form-encoded request body with a JSON-encoded parameter value
+               decodedValue, err := url.ParseQuery(string(data))
+               c.Check(err, check.IsNil)
+               decodedValueCR := decodedValue.Get("container_request")
+               c.Check(json.Unmarshal([]byte(decodedValueCR), &cr), check.IsNil)
+       } else {
+               // mock needs to have Content-Type that we can parse.
+               c.Fail()
+       }
+
+       return cr
+}
+
 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"]
-  }
-}
-`))
+         "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")
 
+       // We replace zhome with zzzzz values (RailsAPI, ClusterID, SystemRootToken)
+       // SystemRoot token is needed because we check the
+       // https://[RailsAPI]/arvados/v1/api_client_authorizations/current
+       // https://[RailsAPI]/arvados/v1/users/current and
+       // https://[RailsAPI]/auth/controller/callback
        arvadostest.SetServiceURL(&s.testHandler.Cluster.Services.RailsAPI, "https://"+os.Getenv("ARVADOS_TEST_API_HOST"))
        s.testHandler.Cluster.ClusterID = "zzzzz"
+       s.testHandler.Cluster.SystemRootToken = arvadostest.SystemRootToken
 
        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)
+
+       cr := s.getCRfromMockRequest(c)
+
+       // Runtime token must match zzzzz cluster
+       c.Check(cr.RuntimeToken, check.Matches, "v2/zzzzz-gj3su-.*")
+       // RuntimeToken must be different than the Original Token we originally did the request with.
+       c.Check(cr.RuntimeToken, check.Not(check.Equals), arvadostest.ActiveTokenV2)
 }
 
 func (s *FederationSuite) TestCreateRemoteContainerRequestCheckSetRuntimeToken(c *check.C) {
@@ -670,54 +716,25 @@ func (s *FederationSuite) TestCreateRemoteContainerRequestCheckSetRuntimeToken(c
        // 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"
-  }
-}
-`))
+         "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.
+       cr := s.getCRfromMockRequest(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=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)
+       // After mocking around now making sure the runtime_token we sent is still there.
+       c.Check(cr.RuntimeToken, check.Equals, "xyz")
 }
 
 func (s *FederationSuite) TestCreateRemoteContainerRequestError(c *check.C) {
index 6669e020fdb9046abdceca72709348526af663c5..b04757ac338fc1549f38daf960c07381803442dd 100644 (file)
@@ -100,6 +100,8 @@ func (h *Handler) setup() {
                mux.Handle("/arvados/v1/collections/", rtr)
                mux.Handle("/arvados/v1/users", rtr)
                mux.Handle("/arvados/v1/users/", rtr)
+               mux.Handle("/arvados/v1/container_requests", rtr)
+               mux.Handle("/arvados/v1/container_requests/", rtr)
                mux.Handle("/login", rtr)
                mux.Handle("/logout", rtr)
        }
index 7d8266a85cab13af302cc22dcae44800465dca08..d12e4fa33d32a72d8f9b5342c94aab84664fbe03 100644 (file)
@@ -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)
index 0388f21bee916ff4a0046971edfa9908553d00b1..a574f3978052538a3efcf642710cc208f2de9e4e 100644 (file)
@@ -7,6 +7,7 @@ package controller
 import (
        "bytes"
        "context"
+       "database/sql"
        "encoding/json"
        "fmt"
        "io"
@@ -20,6 +21,7 @@ import (
        "path/filepath"
        "strconv"
        "strings"
+       "time"
 
        "git.arvados.org/arvados.git/lib/boot"
        "git.arvados.org/arvados.git/lib/config"
@@ -209,6 +211,7 @@ func (s *IntegrationSuite) userClients(rootctx context.Context, c *check.C, conn
                        FirstName: "Example",
                        LastName:  "User",
                        Username:  "example",
+                       ExpiresAt: time.Now().Add(1 * time.Hour),
                },
        })
        c.Assert(err, check.IsNil)
@@ -460,6 +463,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")
@@ -486,10 +490,153 @@ 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.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) {
+       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},
+       }
+
+       body, _ := json.Marshal(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))
+               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) dbConn(c *check.C, clusterID string) (*sql.DB, *sql.Conn) {
+       ctx := context.Background()
+       db, err := sql.Open("postgres", s.testClusters[clusterID].super.Cluster().PostgreSQL.Connection.String())
+       c.Assert(err, check.IsNil)
+
+       conn, err := db.Conn(ctx)
+       c.Assert(err, check.IsNil)
+
+       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))
+       return db, conn
+}
+
+// TestRuntimeTokenInCR will test several different tokens in the runtime attribute
+// and check the expected results accessing directly to the database if needed.
+func (s *IntegrationSuite) TestRuntimeTokenInCR(c *check.C) {
+       db, dbconn := s.dbConn(c, "z1111")
+       defer db.Close()
+       defer dbconn.Close()
+       conn1 := s.conn("z1111")
+       rootctx1, _, _ := s.rootClients("z1111")
+       userctx1, 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(userctx1, arvados.CreateOptions{Attrs: rq})
+               if tt.expectAToGetAValidCR {
+                       c.Check(err, check.IsNil)
+                       c.Check(cr, check.NotNil)
+                       c.Check(cr.UUID, check.Not(check.Equals), "")
+               }
+
+               if tt.expectedToken == nil {
+                       continue
+               }
+
+               c.Logf("cr.UUID: %s", cr.UUID)
+               row := dbconn.QueryRowContext(rootctx1, `SELECT runtime_token from container_requests where uuid=$1`, cr.UUID)
+               c.Check(row, check.NotNil)
+               var token sql.NullString
+               row.Scan(&token)
+               if c.Check(token.Valid, check.Equals, true) {
+                       c.Check(token.String, check.Equals, *tt.expectedToken)
+               }
+       }
+}
+
+// TestIntermediateCluster will send a container request to
+// one cluster with another cluster as the destination
+// and check the tokens are being handled properly
+func (s *IntegrationSuite) TestIntermediateCluster(c *check.C) {
+       conn1 := s.conn("z1111")
+       rootctx1, _, _ := s.rootClients("z1111")
+       uctx1, ac1, _, _ := s.userClients(rootctx1, c, conn1, "z1111", true)
+
+       tests := []struct {
+               name                 string
+               token                string
+               expectedRuntimeToken string
+               expectedUUIDprefix   string
+       }{
+               {"Good token z1111 user sending a CR to z2222", ac1.AuthToken, "", "z2222-xvhdp-"},
+       }
+
+       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(uctx1, arvados.CreateOptions{ClusterID: "z2222", Attrs: rq})
+
                c.Check(err, check.IsNil)
-               c.Check(cr.UUID, check.Matches, "z2222-.*")
+               c.Check(strings.HasPrefix(cr.UUID, tt.expectedUUIDprefix), check.Equals, true)
+               c.Check(cr.RuntimeToken, check.Equals, tt.expectedRuntimeToken)
        }
 }
 
index 4f0035edf993ad525c4d82b8d5e880049432c6c2..d197675f8dc6e774d10427b11121a8f27e2c4823 100644 (file)
@@ -24,21 +24,24 @@ func NewConn(cluster *arvados.Cluster) *Conn {
        railsProxy := railsproxy.NewConn(cluster)
        var conn Conn
        conn = Conn{
-               cluster:         cluster,
-               railsProxy:      railsProxy,
-               loginController: chooseLoginController(cluster, railsProxy),
+               cluster:    cluster,
+               railsProxy: railsProxy,
        }
+       conn.loginController = chooseLoginController(cluster, &conn)
        return &conn
 }
 
+// Logout handles the logout of conn giving to the appropriate loginController
 func (conn *Conn) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
        return conn.loginController.Logout(ctx, opts)
 }
 
+// Login handles the login of conn giving to the appropriate loginController
 func (conn *Conn) Login(ctx context.Context, opts arvados.LoginOptions) (arvados.LoginResponse, error) {
        return conn.loginController.Login(ctx, opts)
 }
 
+// UserAuthenticate handles the User Authentication of conn giving to the appropriate loginController
 func (conn *Conn) UserAuthenticate(ctx context.Context, opts arvados.UserAuthenticateOptions) (arvados.APIClientAuthorization, error) {
        return conn.loginController.UserAuthenticate(ctx, opts)
 }
index f4632751e30dc24944d04157e939d676ee33c53a..4bf515fc3f1113b6a240e991ecc3e236c8b49ee3 100644 (file)
@@ -27,7 +27,7 @@ type loginController interface {
        UserAuthenticate(ctx context.Context, options arvados.UserAuthenticateOptions) (arvados.APIClientAuthorization, error)
 }
 
-func chooseLoginController(cluster *arvados.Cluster, railsProxy *railsProxy) loginController {
+func chooseLoginController(cluster *arvados.Cluster, parent *Conn) loginController {
        wantGoogle := cluster.Login.Google.Enable
        wantOpenIDConnect := cluster.Login.OpenIDConnect.Enable
        wantSSO := cluster.Login.SSO.Enable
@@ -43,7 +43,7 @@ func chooseLoginController(cluster *arvados.Cluster, railsProxy *railsProxy) log
        case wantGoogle:
                return &oidcLoginController{
                        Cluster:            cluster,
-                       RailsProxy:         railsProxy,
+                       Parent:             parent,
                        Issuer:             "https://accounts.google.com",
                        ClientID:           cluster.Login.Google.ClientID,
                        ClientSecret:       cluster.Login.Google.ClientSecret,
@@ -54,7 +54,7 @@ func chooseLoginController(cluster *arvados.Cluster, railsProxy *railsProxy) log
        case wantOpenIDConnect:
                return &oidcLoginController{
                        Cluster:            cluster,
-                       RailsProxy:         railsProxy,
+                       Parent:             parent,
                        Issuer:             cluster.Login.OpenIDConnect.Issuer,
                        ClientID:           cluster.Login.OpenIDConnect.ClientID,
                        ClientSecret:       cluster.Login.OpenIDConnect.ClientSecret,
@@ -63,13 +63,13 @@ func chooseLoginController(cluster *arvados.Cluster, railsProxy *railsProxy) log
                        UsernameClaim:      cluster.Login.OpenIDConnect.UsernameClaim,
                }
        case wantSSO:
-               return &ssoLoginController{railsProxy}
+               return &ssoLoginController{Parent: parent}
        case wantPAM:
-               return &pamLoginController{Cluster: cluster, RailsProxy: railsProxy}
+               return &pamLoginController{Cluster: cluster, Parent: parent}
        case wantLDAP:
-               return &ldapLoginController{Cluster: cluster, RailsProxy: railsProxy}
+               return &ldapLoginController{Cluster: cluster, Parent: parent}
        case wantTest:
-               return &testLoginController{Cluster: cluster, RailsProxy: railsProxy}
+               return &testLoginController{Cluster: cluster, Parent: parent}
        case wantLoginCluster:
                return &federatedLoginController{Cluster: cluster}
        default:
@@ -89,10 +89,16 @@ func countTrue(vals ...bool) int {
        return n
 }
 
-// Login and Logout are passed through to the wrapped railsProxy;
+// Login and Logout are passed through to the parent's railsProxy;
 // UserAuthenticate is rejected.
-type ssoLoginController struct{ *railsProxy }
+type ssoLoginController struct{ Parent *Conn }
 
+func (ctrl *ssoLoginController) Login(ctx context.Context, opts arvados.LoginOptions) (arvados.LoginResponse, error) {
+       return ctrl.Parent.railsProxy.Login(ctx, opts)
+}
+func (ctrl *ssoLoginController) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
+       return ctrl.Parent.railsProxy.Logout(ctx, opts)
+}
 func (ctrl *ssoLoginController) UserAuthenticate(ctx context.Context, opts arvados.UserAuthenticateOptions) (arvados.APIClientAuthorization, error) {
        return arvados.APIClientAuthorization{}, httpserver.ErrorWithStatus(errors.New("username/password authentication is not available"), http.StatusBadRequest)
 }
@@ -135,9 +141,12 @@ func noopLogout(cluster *arvados.Cluster, opts arvados.LogoutOptions) (arvados.L
        return arvados.LogoutResponse{RedirectLocation: target}, nil
 }
 
-func createAPIClientAuthorization(ctx context.Context, conn *rpc.Conn, rootToken string, authinfo rpc.UserSessionAuthInfo) (resp arvados.APIClientAuthorization, err error) {
+func (conn *Conn) CreateAPIClientAuthorization(ctx context.Context, rootToken string, authinfo rpc.UserSessionAuthInfo) (resp arvados.APIClientAuthorization, err error) {
+       if rootToken == "" {
+               return arvados.APIClientAuthorization{}, errors.New("configuration error: empty SystemRootToken")
+       }
        ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{rootToken}})
-       newsession, err := conn.UserSessionCreate(ctxRoot, rpc.UserSessionCreateOptions{
+       newsession, err := conn.railsProxy.UserSessionCreate(ctxRoot, rpc.UserSessionCreateOptions{
                // Send a fake ReturnTo value instead of the caller's
                // opts.ReturnTo. We won't follow the resulting
                // redirect target anyway.
index 6c430d69bbfee5505c056430540000059c0b4c4b..49f557ae5b9ce50a8f7c3ceb59cc0fb31b50e187 100644 (file)
@@ -21,8 +21,8 @@ import (
 )
 
 type ldapLoginController struct {
-       Cluster    *arvados.Cluster
-       RailsProxy *railsProxy
+       Cluster *arvados.Cluster
+       Parent  *Conn
 }
 
 func (ctrl *ldapLoginController) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
@@ -143,7 +143,7 @@ func (ctrl *ldapLoginController) UserAuthenticate(ctx context.Context, opts arva
                return arvados.APIClientAuthorization{}, errors.New("authentication succeeded but ldap returned no email address")
        }
 
-       return createAPIClientAuthorization(ctx, ctrl.RailsProxy, ctrl.Cluster.SystemRootToken, rpc.UserSessionAuthInfo{
+       return ctrl.Parent.CreateAPIClientAuthorization(ctx, ctrl.Cluster.SystemRootToken, rpc.UserSessionAuthInfo{
                Email:     email,
                FirstName: attrs["givenname"],
                LastName:  attrs["sn"],
index bce1ecfcf260696247bb83de3cdce2fa9d27cabe..b8ba6b4676712efcad438bf9066195294b8702c3 100644 (file)
@@ -90,8 +90,8 @@ func (s *LDAPSuite) SetUpSuite(c *check.C) {
        s.cluster.Login.LDAP.SearchBase = "dc=example,dc=com"
        c.Assert(err, check.IsNil)
        s.ctrl = &ldapLoginController{
-               Cluster:    s.cluster,
-               RailsProxy: railsproxy.NewConn(s.cluster),
+               Cluster: s.cluster,
+               Parent:  &Conn{railsProxy: railsproxy.NewConn(s.cluster)},
        }
        s.db = arvadostest.DB(c, s.cluster)
 }
index 5f96da56244325d86b3e9d4f252ec714f55f534c..b99a1c2aa5b9fc47f4d461615873cc6204c24b80 100644 (file)
@@ -22,7 +22,6 @@ import (
        "time"
 
        "git.arvados.org/arvados.git/lib/controller/api"
-       "git.arvados.org/arvados.git/lib/controller/railsproxy"
        "git.arvados.org/arvados.git/lib/controller/rpc"
        "git.arvados.org/arvados.git/lib/ctrlctx"
        "git.arvados.org/arvados.git/sdk/go/arvados"
@@ -46,7 +45,7 @@ const (
 
 type oidcLoginController struct {
        Cluster            *arvados.Cluster
-       RailsProxy         *railsProxy
+       Parent             *Conn
        Issuer             string // OIDC issuer URL, e.g., "https://accounts.google.com"
        ClientID           string
        ClientSecret       string
@@ -143,7 +142,7 @@ func (ctrl *oidcLoginController) Login(ctx context.Context, opts arvados.LoginOp
                return loginError(err)
        }
        ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{ctrl.Cluster.SystemRootToken}})
-       return ctrl.RailsProxy.UserSessionCreate(ctxRoot, rpc.UserSessionCreateOptions{
+       return ctrl.Parent.UserSessionCreate(ctxRoot, rpc.UserSessionCreateOptions{
                ReturnTo: state.Remote + "," + state.ReturnTo,
                AuthInfo: *authinfo,
        })
@@ -322,7 +321,7 @@ func OIDCAccessTokenAuthorizer(cluster *arvados.Cluster, getdb func(context.Cont
        // We want ctrl to be nil if the chosen controller is not a
        // *oidcLoginController, so we can ignore the 2nd return value
        // of this type cast.
-       ctrl, _ := chooseLoginController(cluster, railsproxy.NewConn(cluster)).(*oidcLoginController)
+       ctrl, _ := NewConn(cluster).loginController.(*oidcLoginController)
        cache, err := lru.New2Q(tokenCacheSize)
        if err != nil {
                panic(err)
@@ -474,7 +473,7 @@ func (ta *oidcTokenAuthorizer) registerToken(ctx context.Context, tok string) er
                }
                ctxlog.FromContext(ctx).WithField("HMAC", hmac).Debug("(*oidcTokenAuthorizer)registerToken: updated api_client_authorizations row")
        } else {
-               aca, err = createAPIClientAuthorization(ctx, ta.ctrl.RailsProxy, ta.ctrl.Cluster.SystemRootToken, *authinfo)
+               aca, err = ta.ctrl.Parent.CreateAPIClientAuthorization(ctx, ta.ctrl.Cluster.SystemRootToken, *authinfo)
                if err != nil {
                        return err
                }
index 2447713a2cf453ea05cfc29e2c643fa0713848a9..5d116a9e8f490be37225d0e3333ab6413c79e979 100644 (file)
@@ -20,8 +20,8 @@ import (
 )
 
 type pamLoginController struct {
-       Cluster    *arvados.Cluster
-       RailsProxy *railsProxy
+       Cluster *arvados.Cluster
+       Parent  *Conn
 }
 
 func (ctrl *pamLoginController) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
@@ -87,7 +87,7 @@ func (ctrl *pamLoginController) UserAuthenticate(ctx context.Context, opts arvad
                "user":  user,
                "email": email,
        }).Debug("pam authentication succeeded")
-       return createAPIClientAuthorization(ctx, ctrl.RailsProxy, ctrl.Cluster.SystemRootToken, rpc.UserSessionAuthInfo{
+       return ctrl.Parent.CreateAPIClientAuthorization(ctx, ctrl.Cluster.SystemRootToken, rpc.UserSessionAuthInfo{
                Username: user,
                Email:    email,
        })
index e6b967c9440b887cc6b2e68bd3ccb5e7a8fa78eb..c5876bbfad6280ad407aadece8cef5be606b187a 100644 (file)
@@ -36,8 +36,8 @@ func (s *PamSuite) SetUpSuite(c *check.C) {
        s.cluster.Login.PAM.DefaultEmailDomain = "example.com"
        s.railsSpy = arvadostest.NewProxy(c, s.cluster.Services.RailsAPI)
        s.ctrl = &pamLoginController{
-               Cluster:    s.cluster,
-               RailsProxy: rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider),
+               Cluster: s.cluster,
+               Parent:  &Conn{railsProxy: rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider)},
        }
 }
 
index 5852273529e6434b2f54ce7fcb551a85eb360880..c567a0668344b6eadc2d439f36a94147d3ae453f 100644 (file)
@@ -17,8 +17,8 @@ import (
 )
 
 type testLoginController struct {
-       Cluster    *arvados.Cluster
-       RailsProxy *railsProxy
+       Cluster *arvados.Cluster
+       Parent  *Conn
 }
 
 func (ctrl *testLoginController) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
@@ -45,7 +45,7 @@ func (ctrl *testLoginController) UserAuthenticate(ctx context.Context, opts arva
                                "username": username,
                                "email":    user.Email,
                        }).Debug("test authentication succeeded")
-                       return createAPIClientAuthorization(ctx, ctrl.RailsProxy, ctrl.Cluster.SystemRootToken, rpc.UserSessionAuthInfo{
+                       return ctrl.Parent.CreateAPIClientAuthorization(ctx, ctrl.Cluster.SystemRootToken, rpc.UserSessionAuthInfo{
                                Username: username,
                                Email:    user.Email,
                        })
index 7589088899744efca9187e2cc9d3094b8d39db03..7a520428b6d6836d9dafd6008ac9bf18433ca70f 100644 (file)
@@ -41,8 +41,8 @@ func (s *TestUserSuite) SetUpSuite(c *check.C) {
        }
        s.railsSpy = arvadostest.NewProxy(c, s.cluster.Services.RailsAPI)
        s.ctrl = &testLoginController{
-               Cluster:    s.cluster,
-               RailsProxy: rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider),
+               Cluster: s.cluster,
+               Parent:  &Conn{railsProxy: rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider)},
        }
        s.db = arvadostest.DB(c, s.cluster)
 }
index 543e25d0ce44f73c6cd87cc221aaace564f41a2c..d554ab930f30ce54b5405466c596a58843d237f2 100644 (file)
@@ -97,38 +97,18 @@ func (rtr *router) sendResponse(w http.ResponseWriter, req *http.Request, resp i
                        } else if defaultItemKind != "" {
                                item["kind"] = defaultItemKind
                        }
-                       items[i] = applySelectParam(opts.Select, item)
+                       item = applySelectParam(opts.Select, item)
+                       rtr.mungeItemFields(item)
+                       items[i] = item
                }
                if opts.Count == "none" {
                        delete(tmp, "items_available")
                }
        } else {
                tmp = applySelectParam(opts.Select, tmp)
+               rtr.mungeItemFields(tmp)
        }
 
-       // Format non-nil timestamps as rfc3339NanoFixed (by default
-       // they will have been encoded to time.RFC3339Nano, which
-       // omits trailing zeroes).
-       for k, v := range tmp {
-               if !strings.HasSuffix(k, "_at") {
-                       continue
-               }
-               switch tv := v.(type) {
-               case *time.Time:
-                       if tv == nil {
-                               break
-                       }
-                       tmp[k] = tv.Format(rfc3339NanoFixed)
-               case time.Time:
-                       tmp[k] = tv.Format(rfc3339NanoFixed)
-               case string:
-                       t, err := time.Parse(time.RFC3339Nano, tv)
-                       if err != nil {
-                               break
-                       }
-                       tmp[k] = t.Format(rfc3339NanoFixed)
-               }
-       }
        w.Header().Set("Content-Type", "application/json")
        enc := json.NewEncoder(w)
        enc.SetEscapeHTML(false)
@@ -160,3 +140,51 @@ func kind(resp interface{}) string {
                return "#" + strings.ToLower(s[1:])
        })
 }
+
+func (rtr *router) mungeItemFields(tmp map[string]interface{}) {
+       for k, v := range tmp {
+               if strings.HasSuffix(k, "_at") {
+                       // Format non-nil timestamps as
+                       // rfc3339NanoFixed (otherwise they would use
+                       // the default time encoding, which omits
+                       // trailing zeroes).
+                       switch tv := v.(type) {
+                       case *time.Time:
+                               if tv == nil || tv.IsZero() {
+                                       tmp[k] = nil
+                               } else {
+                                       tmp[k] = tv.Format(rfc3339NanoFixed)
+                               }
+                       case time.Time:
+                               if tv.IsZero() {
+                                       tmp[k] = nil
+                               } else {
+                                       tmp[k] = tv.Format(rfc3339NanoFixed)
+                               }
+                       case string:
+                               if tv == "" {
+                                       tmp[k] = nil
+                               } else if t, err := time.Parse(time.RFC3339Nano, tv); err != nil {
+                                       // pass through an invalid time value (?)
+                               } else if t.IsZero() {
+                                       tmp[k] = nil
+                               } else {
+                                       tmp[k] = t.Format(rfc3339NanoFixed)
+                               }
+                       }
+               }
+               // Arvados API spec says when these fields are empty
+               // they appear in responses as null, rather than a
+               // zero value.
+               switch k {
+               case "output_uuid", "output_name", "log_uuid", "description", "requesting_container_uuid", "container_uuid":
+                       if v == "" {
+                               tmp[k] = nil
+                       }
+               case "container_count_max":
+                       if v == float64(0) {
+                               tmp[k] = nil
+                       }
+               }
+       }
+}
index 2944524344e9028fa22cf0c9d18327cb39193733..9fb2a0d32b49b5af5c6337cc2187c94b7d86a991 100644 (file)
@@ -168,6 +168,41 @@ func (rtr *router) addRoutes() {
                                return rtr.backend.ContainerDelete(ctx, *opts.(*arvados.DeleteOptions))
                        },
                },
+               {
+                       arvados.EndpointContainerRequestCreate,
+                       func() interface{} { return &arvados.CreateOptions{} },
+                       func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               return rtr.backend.ContainerRequestCreate(ctx, *opts.(*arvados.CreateOptions))
+                       },
+               },
+               {
+                       arvados.EndpointContainerRequestUpdate,
+                       func() interface{} { return &arvados.UpdateOptions{} },
+                       func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               return rtr.backend.ContainerRequestUpdate(ctx, *opts.(*arvados.UpdateOptions))
+                       },
+               },
+               {
+                       arvados.EndpointContainerRequestGet,
+                       func() interface{} { return &arvados.GetOptions{} },
+                       func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               return rtr.backend.ContainerRequestGet(ctx, *opts.(*arvados.GetOptions))
+                       },
+               },
+               {
+                       arvados.EndpointContainerRequestList,
+                       func() interface{} { return &arvados.ListOptions{Limit: -1} },
+                       func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               return rtr.backend.ContainerRequestList(ctx, *opts.(*arvados.ListOptions))
+                       },
+               },
+               {
+                       arvados.EndpointContainerRequestDelete,
+                       func() interface{} { return &arvados.DeleteOptions{} },
+                       func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               return rtr.backend.ContainerRequestDelete(ctx, *opts.(*arvados.DeleteOptions))
+                       },
+               },
                {
                        arvados.EndpointContainerLock,
                        func() interface{} {
index cd98b64718a0b1f52702f4997f432d3d5210f353..455c20fa91ed48be4ba778afafabcc7b09d7e857 100644 (file)
@@ -23,6 +23,8 @@ import (
        "git.arvados.org/arvados.git/sdk/go/auth"
 )
 
+const rfc3339NanoFixed = "2006-01-02T15:04:05.000000000Z07:00"
+
 type TokenProvider func(context.Context) ([]string, error)
 
 func PassthroughTokenProvider(ctx context.Context) ([]string, error) {
@@ -120,6 +122,20 @@ func (conn *Conn) requestAndDecode(ctx context.Context, dst interface{}, ep arva
                        delete(params, "limit")
                }
        }
+
+       if authinfo, ok := params["auth_info"]; ok {
+               if tmp, ok2 := authinfo.(map[string]interface{}); ok2 {
+                       for k, v := range tmp {
+                               if strings.HasSuffix(k, "_at") {
+                                       // Change zero times values to nil
+                                       if v, ok3 := v.(string); ok3 && (strings.HasPrefix(v, "0001-01-01T00:00:00") || v == "") {
+                                               tmp[k] = nil
+                                       }
+                               }
+                       }
+               }
+       }
+
        if len(tokens) > 1 {
                params["reader_tokens"] = tokens[1:]
        }
@@ -286,6 +302,41 @@ func (conn *Conn) ContainerUnlock(ctx context.Context, options arvados.GetOption
        return resp, err
 }
 
+func (conn *Conn) ContainerRequestCreate(ctx context.Context, options arvados.CreateOptions) (arvados.ContainerRequest, error) {
+       ep := arvados.EndpointContainerRequestCreate
+       var resp arvados.ContainerRequest
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       return resp, err
+}
+
+func (conn *Conn) ContainerRequestUpdate(ctx context.Context, options arvados.UpdateOptions) (arvados.ContainerRequest, error) {
+       ep := arvados.EndpointContainerRequestUpdate
+       var resp arvados.ContainerRequest
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       return resp, err
+}
+
+func (conn *Conn) ContainerRequestGet(ctx context.Context, options arvados.GetOptions) (arvados.ContainerRequest, error) {
+       ep := arvados.EndpointContainerRequestGet
+       var resp arvados.ContainerRequest
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       return resp, err
+}
+
+func (conn *Conn) ContainerRequestList(ctx context.Context, options arvados.ListOptions) (arvados.ContainerRequestList, error) {
+       ep := arvados.EndpointContainerRequestList
+       var resp arvados.ContainerRequestList
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       return resp, err
+}
+
+func (conn *Conn) ContainerRequestDelete(ctx context.Context, options arvados.DeleteOptions) (arvados.ContainerRequest, error) {
+       ep := arvados.EndpointContainerRequestDelete
+       var resp arvados.ContainerRequest
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       return resp, err
+}
+
 func (conn *Conn) SpecimenCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Specimen, error) {
        ep := arvados.EndpointSpecimenCreate
        var resp arvados.Specimen
@@ -402,11 +453,13 @@ func (conn *Conn) APIClientAuthorizationCurrent(ctx context.Context, options arv
 }
 
 type UserSessionAuthInfo struct {
-       Email           string   `json:"email"`
-       AlternateEmails []string `json:"alternate_emails"`
-       FirstName       string   `json:"first_name"`
-       LastName        string   `json:"last_name"`
-       Username        string   `json:"username"`
+       UserUUID        string    `json:"user_uuid"`
+       Email           string    `json:"email"`
+       AlternateEmails []string  `json:"alternate_emails"`
+       FirstName       string    `json:"first_name"`
+       LastName        string    `json:"last_name"`
+       Username        string    `json:"username"`
+       ExpiresAt       time.Time `json:"expires_at"`
 }
 
 type UserSessionCreateOptions struct {
@@ -416,6 +469,7 @@ type UserSessionCreateOptions struct {
 
 func (conn *Conn) UserSessionCreate(ctx context.Context, options UserSessionCreateOptions) (arvados.LoginResponse, error) {
        ep := arvados.APIEndpoint{Method: "POST", Path: "auth/controller/callback"}
+       // if ExpiresAt is empty value then add 2 hour expiration
        var resp arvados.LoginResponse
        err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
        return resp, err
index 341938354cd963997730eddecd35946f1cb44af9..730185c1969f2af43b6cb76148f07541711ec451 100644 (file)
@@ -621,7 +621,7 @@ func (runner *ContainerRunner) SetupMounts() (err error) {
                return fmt.Errorf("output path does not correspond to a writable mount point")
        }
 
-       if wantAPI := runner.Container.RuntimeConstraints.API; needCertMount && wantAPI != nil && *wantAPI {
+       if needCertMount && runner.Container.RuntimeConstraints.API {
                for _, certfile := range arvadosclient.CertFiles {
                        _, err := os.Stat(certfile)
                        if err == nil {
@@ -1092,7 +1092,7 @@ func (runner *ContainerRunner) CreateContainer() error {
                },
        }
 
-       if wantAPI := runner.Container.RuntimeConstraints.API; wantAPI != nil && *wantAPI {
+       if runner.Container.RuntimeConstraints.API {
                tok, err := runner.ContainerToken()
                if err != nil {
                        return err
@@ -1269,7 +1269,7 @@ func (runner *ContainerRunner) updateLogs() {
 // CaptureOutput saves data from the container's output directory if
 // needed, and updates the container output accordingly.
 func (runner *ContainerRunner) CaptureOutput() error {
-       if wantAPI := runner.Container.RuntimeConstraints.API; wantAPI != nil && *wantAPI {
+       if runner.Container.RuntimeConstraints.API {
                // Output may have been set directly by the container, so
                // refresh the container record to check.
                err := runner.DispatcherArvClient.Get("containers", runner.Container.UUID,
index eb83bbd4106cf51f89dc04e1d9d3a4dc5e5798fc..dbdaa6293d28c964efc237c9e1b98e44b5ef921c 100644 (file)
@@ -1291,9 +1291,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
                cr.Container.Mounts = make(map[string]arvados.Mount)
                cr.Container.Mounts["/tmp"] = arvados.Mount{Kind: "tmp"}
                cr.Container.OutputPath = "/tmp"
-
-               apiflag := true
-               cr.Container.RuntimeConstraints.API = &apiflag
+               cr.Container.RuntimeConstraints.API = true
 
                err := cr.SetupMounts()
                c.Check(err, IsNil)
@@ -1305,7 +1303,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
                cr.CleanupDirs()
                checkEmpty()
 
-               apiflag = false
+               cr.Container.RuntimeConstraints.API = false
        }
 
        {
index 5a2cfb8800402496f3f8fe400cf38c786e57d6eb..a11872971a9dffc9a2563dedb7338a1438ca40b3 100644 (file)
@@ -41,6 +41,11 @@ var (
        EndpointContainerDelete               = APIEndpoint{"DELETE", "arvados/v1/containers/{uuid}", ""}
        EndpointContainerLock                 = APIEndpoint{"POST", "arvados/v1/containers/{uuid}/lock", ""}
        EndpointContainerUnlock               = APIEndpoint{"POST", "arvados/v1/containers/{uuid}/unlock", ""}
+       EndpointContainerRequestCreate        = APIEndpoint{"POST", "arvados/v1/container_requests", "container_request"}
+       EndpointContainerRequestUpdate        = APIEndpoint{"PATCH", "arvados/v1/container_requests/{uuid}", "container_request"}
+       EndpointContainerRequestGet           = APIEndpoint{"GET", "arvados/v1/container_requests/{uuid}", ""}
+       EndpointContainerRequestList          = APIEndpoint{"GET", "arvados/v1/container_requests", ""}
+       EndpointContainerRequestDelete        = APIEndpoint{"DELETE", "arvados/v1/container_requests/{uuid}", ""}
        EndpointUserActivate                  = APIEndpoint{"POST", "arvados/v1/users/{uuid}/activate", ""}
        EndpointUserCreate                    = APIEndpoint{"POST", "arvados/v1/users", "user"}
        EndpointUserCurrent                   = APIEndpoint{"GET", "arvados/v1/users/current", ""}
@@ -175,6 +180,11 @@ type API interface {
        ContainerDelete(ctx context.Context, options DeleteOptions) (Container, error)
        ContainerLock(ctx context.Context, options GetOptions) (Container, error)
        ContainerUnlock(ctx context.Context, options GetOptions) (Container, error)
+       ContainerRequestCreate(ctx context.Context, options CreateOptions) (ContainerRequest, error)
+       ContainerRequestUpdate(ctx context.Context, options UpdateOptions) (ContainerRequest, error)
+       ContainerRequestGet(ctx context.Context, options GetOptions) (ContainerRequest, error)
+       ContainerRequestList(ctx context.Context, options ListOptions) (ContainerRequestList, error)
+       ContainerRequestDelete(ctx context.Context, options DeleteOptions) (ContainerRequest, error)
        SpecimenCreate(ctx context.Context, options CreateOptions) (Specimen, error)
        SpecimenUpdate(ctx context.Context, options UpdateOptions) (Specimen, error)
        SpecimenGet(ctx context.Context, options GetOptions) (Specimen, error)
index 265944e81d52fdab08d55e767b9626a52f40c3c2..3ff7c52055caf0e6ce72d0f32afc0fa1809007a3 100644 (file)
@@ -65,6 +65,9 @@ type ContainerRequest struct {
        LogUUID                 string                 `json:"log_uuid"`
        OutputUUID              string                 `json:"output_uuid"`
        RuntimeToken            string                 `json:"runtime_token"`
+       ExpiresAt               time.Time              `json:"expires_at"`
+       Filters                 []Filter               `json:"filters"`
+       ContainerCount          int                    `json:"container_count"`
 }
 
 // Mount is special behavior to attach to a filesystem path or device.
@@ -86,7 +89,7 @@ type Mount struct {
 // RuntimeConstraints specify a container's compute resources (RAM,
 // CPU) and network connectivity.
 type RuntimeConstraints struct {
-       API          *bool
+       API          bool  `json:"api"`
        RAM          int64 `json:"ram"`
        VCPUs        int   `json:"vcpus"`
        KeepCacheRAM int64 `json:"keep_cache_ram"`
index 039d7ae116f7b6c09c7b768d601d6f305456f86b..df3e46febd5598f9d065fba24c964701b480c1d1 100644 (file)
@@ -105,6 +105,26 @@ func (as *APIStub) ContainerUnlock(ctx context.Context, options arvados.GetOptio
        as.appendCall(ctx, as.ContainerUnlock, options)
        return arvados.Container{}, as.Error
 }
+func (as *APIStub) ContainerRequestCreate(ctx context.Context, options arvados.CreateOptions) (arvados.ContainerRequest, error) {
+       as.appendCall(ctx, as.ContainerRequestCreate, options)
+       return arvados.ContainerRequest{}, as.Error
+}
+func (as *APIStub) ContainerRequestUpdate(ctx context.Context, options arvados.UpdateOptions) (arvados.ContainerRequest, error) {
+       as.appendCall(ctx, as.ContainerRequestUpdate, options)
+       return arvados.ContainerRequest{}, as.Error
+}
+func (as *APIStub) ContainerRequestGet(ctx context.Context, options arvados.GetOptions) (arvados.ContainerRequest, error) {
+       as.appendCall(ctx, as.ContainerRequestGet, options)
+       return arvados.ContainerRequest{}, as.Error
+}
+func (as *APIStub) ContainerRequestList(ctx context.Context, options arvados.ListOptions) (arvados.ContainerRequestList, error) {
+       as.appendCall(ctx, as.ContainerRequestList, options)
+       return arvados.ContainerRequestList{}, as.Error
+}
+func (as *APIStub) ContainerRequestDelete(ctx context.Context, options arvados.DeleteOptions) (arvados.ContainerRequest, error) {
+       as.appendCall(ctx, as.ContainerRequestDelete, options)
+       return arvados.ContainerRequest{}, as.Error
+}
 func (as *APIStub) SpecimenCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Specimen, error) {
        as.appendCall(ctx, as.SpecimenCreate, options)
        return arvados.Specimen{}, as.Error
index 8e3c3ac5e3d8b8656d587e86626f86f57c33b045..3d109c49ca02cf4c5c7e904f08b1bb63b8a1567f 100644 (file)
@@ -17,6 +17,7 @@ class UserSessionsController < ApplicationController
       raise "Local login disabled when LoginCluster is set"
     end
 
+    max_expires_at = nil
     if params[:provider] == 'controller'
       if request.headers['Authorization'] != 'Bearer ' + Rails.configuration.SystemRootToken
         return send_error('Invalid authorization header', status: 401)
@@ -24,18 +25,27 @@ class UserSessionsController < ApplicationController
       # arvados-controller verified the user and is passing auth_info
       # in request params.
       authinfo = SafeJSON.load(params[:auth_info])
+      max_expires_at = authinfo["expires_at"]
     else
       # omniauth middleware verified the user and is passing auth_info
       # in request.env.
       authinfo = request.env['omniauth.auth']['info'].with_indifferent_access
     end
 
-    begin
-      user = User.register(authinfo)
-    rescue => e
-      Rails.logger.warn "User.register error #{e}"
-      Rails.logger.warn "authinfo was #{authinfo.inspect}"
-      return redirect_to login_failure_url
+    if !authinfo['user_uuid'].blank?
+      user = User.find_by_uuid(authinfo['user_uuid'])
+      if !user
+        Rails.logger.warn "Nonexistent user_uuid in authinfo #{authinfo.inspect}"
+        return redirect_to login_failure_url
+      end
+    else
+      begin
+        user = User.register(authinfo)
+      rescue => e
+        Rails.logger.warn "User.register error #{e}"
+        Rails.logger.warn "authinfo was #{authinfo.inspect}"
+        return redirect_to login_failure_url
+      end
     end
 
     # For the benefit of functional and integration tests:
@@ -72,7 +82,7 @@ class UserSessionsController < ApplicationController
         return send_error 'Invalid remote cluster id', status: 400
       end
       remote = nil if remote == ''
-      return send_api_token_to(return_to_url, user, remote)
+      return send_api_token_to(return_to_url, user, remote, max_expires_at)
     end
     redirect_to @redirect_to
   end
@@ -106,7 +116,7 @@ class UserSessionsController < ApplicationController
       # FIXME: if current_user has never authorized this app before,
       # ask for confirmation here!
 
-      return send_api_token_to(params[:return_to], current_user, params[:remote])
+      return send_api_token_to(params[:return_to], current_user, params[:remote], params[:expires_at])
     end
     p = []
     p << "auth_provider=#{CGI.escape(params[:auth_provider])}" if params[:auth_provider]
@@ -136,7 +146,7 @@ class UserSessionsController < ApplicationController
     end
   end
 
-  def send_api_token_to(callback_url, user, remote=nil)
+  def send_api_token_to(callback_url, user, remote=nil, token_expiration=nil)
     # Give the API client a token for making API calls on behalf of
     # the authenticated user
 
@@ -146,11 +156,14 @@ class UserSessionsController < ApplicationController
       @api_client = ApiClient.
         find_or_create_by(url_prefix: api_client_url_prefix)
     end
-
-    token_expiration = nil
     if Rails.configuration.Login.TokenLifetime > 0
-      token_expiration = Time.now + Rails.configuration.Login.TokenLifetime
+      if token_expiration == nil
+        token_expiration = Time.now + Rails.configuration.Login.TokenLifetime
+      else
+        token_expiration = [token_expiration, Time.now + Rails.configuration.Login.TokenLifetime].min
+      end
     end
+
     @api_client_auth = ApiClientAuthorization.
       new(user: user,
           api_client: @api_client,
index 6a0a58f08d05e57f10d61b770a04bb6a3760c53d..7d5bea8faeb791482be7be26cc47834c42d88db5 100644 (file)
@@ -855,6 +855,40 @@ class ArvadosModel < ApplicationRecord
     nil
   end
 
+  # Fill in implied zero/false values in database records that were
+  # created before #17014 made them explicit, and reset the Rails
+  # "changed" state so the record doesn't appear to have been modified
+  # after loading.
+  #
+  # Invoked by Container and ContainerRequest models as an after_find
+  # hook.
+  def fill_container_defaults_after_find
+    fill_container_defaults
+    set_attribute_was('runtime_constraints', runtime_constraints)
+    set_attribute_was('scheduling_parameters', scheduling_parameters)
+    clear_changes_information
+  end
+
+  # Fill in implied zero/false values. Invoked by ContainerRequest as
+  # a before_validation hook in order to (a) ensure every key has a
+  # value in the updated database record and (b) ensure the attribute
+  # whitelist doesn't reject a change from an explicit zero/false
+  # value in the database to an implicit zero/false value in an update
+  # request.
+  def fill_container_defaults
+    self.runtime_constraints = {
+      'api' => false,
+      'keep_cache_ram' => 0,
+      'ram' => 0,
+      'vcpus' => 0,
+    }.merge(attributes['runtime_constraints'] || {})
+    self.scheduling_parameters = {
+      'max_run_time' => 0,
+      'partitions' => [],
+      'preemptible' => false,
+    }.merge(attributes['scheduling_parameters'] || {})
+  end
+
   # ArvadosModel.find_by_uuid needs extra magic to allow it to return
   # an object in any class.
   def self.find_by_uuid uuid
index 5833c2251f9b8db26a5ebf5834130d96fc4690d0..d01787cbc7c0389e50fc0ebe421bc9bd0672f5ed 100644 (file)
@@ -29,6 +29,7 @@ class Container < ArvadosModel
   serialize :command, Array
   serialize :scheduling_parameters, Hash
 
+  after_find :fill_container_defaults_after_find
   before_validation :fill_field_defaults, :if => :new_record?
   before_validation :set_timestamps
   before_validation :check_lock
@@ -207,17 +208,16 @@ class Container < ArvadosModel
   # containers are suitable).
   def self.resolve_runtime_constraints(runtime_constraints)
     rc = {}
-    defaults = {
-      'keep_cache_ram' =>
-      Rails.configuration.Containers.DefaultKeepCacheRAM,
-    }
-    defaults.merge(runtime_constraints).each do |k, v|
+    runtime_constraints.each do |k, v|
       if v.is_a? Array
         rc[k] = v[0]
       else
         rc[k] = v
       end
     end
+    if rc['keep_cache_ram'] == 0
+      rc['keep_cache_ram'] = Rails.configuration.Containers.DefaultKeepCacheRAM
+    end
     rc
   end
 
index 77536eee4f28f53a2acae66cc90d647967ff6b51..837f2cdc7010eaacc1182f868e9fb01084367ddb 100644 (file)
@@ -30,14 +30,16 @@ class ContainerRequest < ArvadosModel
   serialize :command, Array
   serialize :scheduling_parameters, Hash
 
+  after_find :fill_container_defaults_after_find
   before_validation :fill_field_defaults, :if => :new_record?
-  before_validation :validate_runtime_constraints
+  before_validation :fill_container_defaults
   before_validation :set_default_preemptible_scheduling_parameter
   before_validation :set_container
   validates :command, :container_image, :output_path, :cwd, :presence => true
   validates :output_ttl, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
   validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 1000 }
   validate :validate_datatypes
+  validate :validate_runtime_constraints
   validate :validate_scheduling_parameters
   validate :validate_state_change
   validate :check_update_whitelist
@@ -194,7 +196,7 @@ class ContainerRequest < ArvadosModel
       coll_name = "Container #{out_type} for request #{uuid}"
       trash_at = nil
       if out_type == 'output'
-        if self.output_name
+        if self.output_name and self.output_name != ""
           coll_name = self.output_name
         end
         if self.output_ttl > 0
@@ -312,28 +314,17 @@ class ContainerRequest < ArvadosModel
   end
 
   def set_default_preemptible_scheduling_parameter
-    c = get_requesting_container()
-    if self.state == Committed
-      # If preemptible instances (eg: AWS Spot Instances) are allowed,
-      # ask them on child containers by default.
-      if Rails.configuration.Containers.UsePreemptibleInstances and !c.nil? and
-        self.scheduling_parameters['preemptible'].nil?
-          self.scheduling_parameters['preemptible'] = true
-      end
+    if Rails.configuration.Containers.UsePreemptibleInstances && state == Committed && get_requesting_container()
+      self.scheduling_parameters['preemptible'] = true
     end
   end
 
   def validate_runtime_constraints
     case self.state
     when Committed
-      [['vcpus', true],
-       ['ram', true],
-       ['keep_cache_ram', false]].each do |k, required|
-        if !required && !runtime_constraints.include?(k)
-          next
-        end
+      ['vcpus', 'ram'].each do |k|
         v = runtime_constraints[k]
-        unless (v.is_a?(Integer) && v > 0)
+        if !v.is_a?(Integer) || v <= 0
           errors.add(:runtime_constraints,
                      "[#{k}]=#{v.inspect} must be a positive integer")
         end
index ab0400a67854c47b3967fb84aca44265e5f7f227..1c626050291d6247716b3111ee5fcab7dcc0b814 100644 (file)
@@ -20,6 +20,7 @@ queued:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 running:
   uuid: zzzzz-xvhdp-cr4runningcntnr
@@ -39,6 +40,7 @@ running:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 requester_for_running:
   uuid: zzzzz-xvhdp-req4runningcntr
@@ -59,6 +61,7 @@ requester_for_running:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 running_older:
   uuid: zzzzz-xvhdp-cr4runningcntn2
@@ -78,6 +81,7 @@ running_older:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 completed:
   uuid: zzzzz-xvhdp-cr4completedctr
@@ -99,6 +103,7 @@ completed:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 completed-older:
   uuid: zzzzz-xvhdp-cr4completedcr2
@@ -120,6 +125,7 @@ completed-older:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 completed_diagnostics:
   name: CWL diagnostics hasher
@@ -365,6 +371,7 @@ requester:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 cr_for_requester:
   uuid: zzzzz-xvhdp-cr4requestercnt
@@ -385,6 +392,7 @@ cr_for_requester:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 cr_for_requester2:
   uuid: zzzzz-xvhdp-cr4requestercn2
@@ -404,6 +412,7 @@ cr_for_requester2:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 running_anonymous_accessible:
   uuid: zzzzz-xvhdp-runninganonaccs
@@ -423,6 +432,7 @@ running_anonymous_accessible:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 cr_for_failed:
   uuid: zzzzz-xvhdp-cr4failedcontnr
@@ -442,6 +452,7 @@ cr_for_failed:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 canceled_with_queued_container:
   uuid: zzzzz-xvhdp-canceledqueuedc
@@ -461,6 +472,7 @@ canceled_with_queued_container:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 canceled_with_locked_container:
   uuid: zzzzz-xvhdp-canceledlocekdc
@@ -480,6 +492,7 @@ canceled_with_locked_container:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 canceled_with_running_container:
   uuid: zzzzz-xvhdp-canceledrunning
@@ -499,6 +512,7 @@ canceled_with_running_container:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 running_to_be_deleted:
   uuid: zzzzz-xvhdp-cr5runningcntnr
@@ -518,6 +532,7 @@ running_to_be_deleted:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 completed_with_input_mounts:
   uuid: zzzzz-xvhdp-crwithinputmnts
@@ -539,18 +554,23 @@ completed_with_input_mounts:
   container_uuid: zzzzz-dz642-compltcontainer
   log_uuid: zzzzz-4zz18-logcollection01
   output_uuid: zzzzz-4zz18-znfnqtbbv4spc3w
-  mounts:
-    /var/lib/cwl/cwl.input.json:
-      content:
-        input1:
-          basename: foo
-          class: File
-          location: "keep:fa7aeb5140e2848d39b416daeef4ffc5+45/foo"
-        input2:
-          basename: bar
-          class: File
-          location: "keep:fa7aeb5140e2848d39b416daeef4ffc5+45/bar"
-    /var/lib/cwl/workflow.json: "keep:f9ddda46bb293b6847da984e3aa735db+290"
+  mounts: {
+    "/var/lib/cwl/cwl.input.json": {
+      "kind": "json",
+      "content": {
+        "input1": {
+          "basename": "foo",
+          "class": "File",
+          "location": "keep:fa7aeb5140e2848d39b416daeef4ffc5+45/foo",
+        },
+        "input2": {
+          "basename": "bar",
+          "class": "File",
+          "location": "keep:fa7aeb5140e2848d39b416daeef4ffc5+45/bar",
+        }
+      }
+    }
+  }
 
 uncommitted:
   uuid: zzzzz-xvhdp-cr4uncommittedc
@@ -991,6 +1011,7 @@ cr_in_trashed_project:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 runtime_token:
   uuid: zzzzz-xvhdp-11eklkhy0n4dm86
@@ -1011,6 +1032,7 @@ runtime_token:
   runtime_constraints:
     vcpus: 1
     ram: 123
+  mounts: {}
 
 
 # Test Helper trims the rest of the file
@@ -1026,6 +1048,7 @@ cr_<%=i%>_of_60:
   name: cr-<%= i.to_s %>
   output_path: test
   command: ["echo", "hello"]
+  mounts: {}
 <% end %>
 
 # Do not add your fixtures below this line as the rest of this file will be trimmed by test_helper
index 95c477f4118adc269684dd3cbdbab3d81b51600a..e99af39c9c18625c40879bc6f96439d0fa3b6131 100644 (file)
@@ -24,7 +24,8 @@ class Arvados::V1::ContainerRequestsControllerTest < ActionController::TestCase
 
     cr = JSON.parse(@response.body)
     assert_not_nil cr, 'Expected container request'
-    assert_equal sp, cr['scheduling_parameters']
+    assert_equal sp['partitions'], cr['scheduling_parameters']['partitions']
+    assert_equal false, cr['scheduling_parameters']['preemptible']
   end
 
   test "secret_mounts not in #create responses" do
@@ -62,6 +63,28 @@ class Arvados::V1::ContainerRequestsControllerTest < ActionController::TestCase
     assert_equal 'bar', req.secret_mounts['/foo']['content']
   end
 
+  test "cancel with runtime_constraints and scheduling_params with default values" do
+    authorize_with :active
+    req = container_requests(:queued)
+
+    patch :update, params: {
+      id: req.uuid,
+      container_request: {
+        state: 'Final',
+        priority: 0,
+        runtime_constraints: {
+          'vcpus' => 1,
+          'ram' => 123,
+          'keep_cache_ram' => 0,
+        },
+        scheduling_parameters: {
+          "preemptible"=>false
+        }
+      },
+    }
+    assert_response :success
+  end
+
   test "update without deleting secret_mounts" do
     authorize_with :active
     req = container_requests(:uncommitted)
index d979208d381b1a28d5f6ead099e45e6e7d4f9302..129464cf1c5dd5c9e5e3730aa4f9abf12565c2e2 100644 (file)
@@ -47,6 +47,31 @@ class UserSessionsControllerTest < ActionController::TestCase
                     1.second)
   end
 
+  [[0, 1.hour, 1.hour],
+  [1.hour, 2.hour, 1.hour],
+  [2.hour, 1.hour, 1.hour],
+  [2.hour, nil, 2.hour],
+  ].each do |config_lifetime, request_lifetime, expect_lifetime|
+    test "login with TokenLifetime=#{config_lifetime} and request has expires_at=#{ request_lifetime.nil? ? "nil" : request_lifetime }" do
+      Rails.configuration.Login.TokenLifetime = config_lifetime
+      expected_expiration_time =  Time.now() + expect_lifetime
+      authorize_with :inactive
+      @request.headers['Authorization'] = 'Bearer '+Rails.configuration.SystemRootToken
+      if request_lifetime.nil?
+        get :create, params: {provider: 'controller', auth_info: {email: "foo@bar.com"}, return_to: ',https://app.example'}
+      else
+        get :create, params: {provider: 'controller', auth_info: {email: "foo@bar.com", expires_at: Time.now() + request_lifetime}, return_to: ',https://app.example'}
+      end
+      assert_response :redirect
+      api_client_auth = assigns(:api_client_auth)
+      assert_not_nil api_client_auth
+      assert_not_nil assigns(:api_client)
+      assert_in_delta(api_client_auth.expires_at,
+                      expected_expiration_time,
+                      1.second)
+    end
+  end
+
   test "login with remote param returns a salted token" do
     authorize_with :inactive
     api_client_page = 'http://client.example.com/home'
index 90de800b2fa472e05e711a73a56fe97bb5c67572..b2dde7995606d71680b062b5e1717a07114557bb 100644 (file)
@@ -153,7 +153,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
     cr.reload
 
-    assert_equal({"vcpus" => 2, "ram" => 30}, cr.runtime_constraints)
+    assert ({"vcpus" => 2, "ram" => 30}.to_a - cr.runtime_constraints.to_a).empty?
 
     assert_not_nil cr.container_uuid
     c = Container.find_by_uuid cr.container_uuid
@@ -164,7 +164,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
     assert_equal({}, c.environment)
     assert_equal({"/out" => {"kind"=>"tmp", "capacity"=>1000000}}, c.mounts)
     assert_equal "/out", c.output_path
-    assert_equal({"keep_cache_ram"=>268435456, "vcpus" => 2, "ram" => 30}, c.runtime_constraints)
+    assert ({"keep_cache_ram"=>268435456, "vcpus" => 2, "ram" => 30}.to_a - c.runtime_constraints.to_a).empty?
     assert_operator 0, :<, c.priority
 
     assert_raises(ActiveRecord::RecordInvalid) do
@@ -973,47 +973,16 @@ class ContainerRequestTest < ActiveSupport::TestCase
         end
       else
         cr.save!
-        assert_equal sp, cr.scheduling_parameters
+        assert (sp.to_a - cr.scheduling_parameters.to_a).empty?
       end
     end
   end
 
-  [
-    'zzzzz-dz642-runningcontainr',
-    nil,
-  ].each do |requesting_c|
-    test "having preemptible instances active on the API server, a committed #{requesting_c.nil? ? 'non-':''}child CR should not ask for preemptible instance if parameter already set to false" do
-      common_attrs = {cwd: "test",
-                      priority: 1,
-                      command: ["echo", "hello"],
-                      output_path: "test",
-                      scheduling_parameters: {"preemptible" => false},
-                      mounts: {"test" => {"kind" => "json"}}}
-
-      Rails.configuration.Containers.UsePreemptibleInstances = true
-      set_user_from_auth :active
-
-      if requesting_c
-        cr = with_container_auth(Container.find_by_uuid requesting_c) do
-          create_minimal_req!(common_attrs)
-        end
-        assert_not_nil cr.requesting_container_uuid
-      else
-        cr = create_minimal_req!(common_attrs)
-      end
-
-      cr.state = ContainerRequest::Committed
-      cr.save!
-
-      assert_equal false, cr.scheduling_parameters['preemptible']
-    end
-  end
-
   [
     [true, 'zzzzz-dz642-runningcontainr', true],
-    [true, nil, nil],
-    [false, 'zzzzz-dz642-runningcontainr', nil],
-    [false, nil, nil],
+    [true, nil, false],
+    [false, 'zzzzz-dz642-runningcontainr', false],
+    [false, nil, false],
   ].each do |preemptible_conf, requesting_c, schedule_preemptible|
     test "having Rails.configuration.Containers.UsePreemptibleInstances=#{preemptible_conf}, #{requesting_c.nil? ? 'non-':''}child CR should #{schedule_preemptible ? '':'not'} ask for preemptible instance by default" do
       common_attrs = {cwd: "test",
@@ -1068,11 +1037,11 @@ class ContainerRequestTest < ActiveSupport::TestCase
         end
       else
         cr = create_minimal_req!(common_attrs.merge({state: state}))
-        assert_equal sp, cr.scheduling_parameters
+        assert (sp.to_a - cr.scheduling_parameters.to_a).empty?
 
         if state == ContainerRequest::Committed
           c = Container.find_by_uuid(cr.container_uuid)
-          assert_equal sp, c.scheduling_parameters
+          assert (sp.to_a - c.scheduling_parameters.to_a).empty?
         end
       end
     end
index 98e60e057910f034194ad9f47289627a245f97e4..4d853852422bc5c7abeadd2b5e3d72601a2d7a37 100644 (file)
@@ -23,6 +23,8 @@ class ContainerTest < ActiveSupport::TestCase
     command: ["echo", "hello"],
     output_path: "test",
     runtime_constraints: {
+      "api" => false,
+      "keep_cache_ram" => 0,
       "ram" => 12000000000,
       "vcpus" => 4,
     },
@@ -227,11 +229,12 @@ class ContainerTest < ActiveSupport::TestCase
     set_user_from_auth :active
     env = {"C" => "3", "B" => "2", "A" => "1"}
     m = {"F" => {"kind" => "3"}, "E" => {"kind" => "2"}, "D" => {"kind" => "1"}}
-    rc = {"vcpus" => 1, "ram" => 1, "keep_cache_ram" => 1}
+    rc = {"vcpus" => 1, "ram" => 1, "keep_cache_ram" => 1, "api" => true}
     c, _ = minimal_new(environment: env, mounts: m, runtime_constraints: rc)
-    assert_equal c.environment.to_json, Container.deep_sort_hash(env).to_json
-    assert_equal c.mounts.to_json, Container.deep_sort_hash(m).to_json
-    assert_equal c.runtime_constraints.to_json, Container.deep_sort_hash(rc).to_json
+    c.reload
+    assert_equal Container.deep_sort_hash(env).to_json, c.environment.to_json
+    assert_equal Container.deep_sort_hash(m).to_json, c.mounts.to_json
+    assert_equal Container.deep_sort_hash(rc).to_json, c.runtime_constraints.to_json
   end
 
   test 'deep_sort_hash on array of hashes' do
@@ -561,6 +564,7 @@ class ContainerTest < ActiveSupport::TestCase
     assert_equal Container::Queued, c1.state
     reused = Container.find_reusable(common_attrs.merge(runtime_token_attr(:container_runtime_token)))
     # See #14584
+    assert_not_nil reused
     assert_equal c1.uuid, reused.uuid
   end
 
@@ -571,6 +575,7 @@ class ContainerTest < ActiveSupport::TestCase
     assert_equal Container::Queued, c1.state
     reused = Container.find_reusable(common_attrs.merge(runtime_token_attr(:container_runtime_token)))
     # See #14584
+    assert_not_nil reused
     assert_equal c1.uuid, reused.uuid
   end
 
@@ -581,6 +586,7 @@ class ContainerTest < ActiveSupport::TestCase
     assert_equal Container::Queued, c1.state
     reused = Container.find_reusable(common_attrs.merge(runtime_token_attr(:container_runtime_token)))
     # See #14584
+    assert_not_nil reused
     assert_equal c1.uuid, reused.uuid
   end