[ARVADOS] updated: 2.1.0-252-g882bfd238
Git user
git at public.arvados.org
Thu Jan 21 01:39:54 UTC 2021
Summary of changes:
discards 6e7530185206e4c55696d20078aae4677e474fb7 (commit)
discards f37a98d9509da449abf0d8c7bca298d0e702f4c9 (commit)
discards 7c3bc0685d120eab7365fc18c06a174427c92312 (commit)
discards e25d440d6d37538438c504f45ad194f9dc7913de (commit)
discards f5df3852a472d3c75cdcff394f0fa45423c05388 (commit)
discards 7044c78d3c48cb3e43beb9a88e01aa770f12af2d (commit)
discards b8ca048149f4cc2df284b409ffc254f17b1865c3 (commit)
discards e00aec9b399e249b3b8c022364f2d7eb4b2d1640 (commit)
discards 31e186b3a6f5f89584d50ab9e874551bcc99b674 (commit)
discards 33a1353e611685b4e22a91ed652fe7d2a6d19feb (commit)
discards 3203d60cc00cdd004ab717e8cc1898e5c9af75b8 (commit)
discards 87237527d78621513fb73f619d2b3f49e84451c5 (commit)
via 882bfd2383da2f9d3cbd2b5f258e727e385a4fbb (commit)
via 3ec51dcf456b2afe02857089895a261653abddd4 (commit)
This update added new revisions after undoing existing revisions. That is
to say, the old revision is not a strict subset of the new revision. This
situation occurs when you --force push a change and generate a repository
containing something like this:
* -- * -- B -- O -- O -- O (6e7530185206e4c55696d20078aae4677e474fb7)
\
N -- N -- N (882bfd2383da2f9d3cbd2b5f258e727e385a4fbb)
When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.
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 882bfd2383da2f9d3cbd2b5f258e727e385a4fbb
Author: Nico Cesar <nico at nicocesar.com>
Date: Wed Jan 20 20:35:36 2021 -0500
addressing https://dev.arvados.org/issues/17014#note-41
Arvados-DCO-1.1-Signed-off-by: Nico Cesar <nico at curii.com>
diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index a574f3978..b240c216c 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -21,7 +21,6 @@ import (
"path/filepath"
"strconv"
"strings"
- "time"
"git.arvados.org/arvados.git/lib/boot"
"git.arvados.org/arvados.git/lib/config"
@@ -211,7 +210,6 @@ 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)
diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index 455c20fa9..d9d24260b 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -469,7 +469,6 @@ 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
diff --git a/services/api/app/controllers/user_sessions_controller.rb b/services/api/app/controllers/user_sessions_controller.rb
index 3d109c49c..da0523d2b 100644
--- a/services/api/app/controllers/user_sessions_controller.rb
+++ b/services/api/app/controllers/user_sessions_controller.rb
@@ -116,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], params[:expires_at])
+ return send_api_token_to(params[:return_to], current_user, params[:remote])
end
p = []
p << "auth_provider=#{CGI.escape(params[:auth_provider])}" if params[:auth_provider]
commit 3ec51dcf456b2afe02857089895a261653abddd4
Author: Nico Cesar <nico at nicocesar.com>
Date: Thu Jan 7 11:23:21 2021 -0500
17014: controller handles container requests
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 at curii.com>
diff --git a/apps/workbench/test/controllers/container_requests_controller_test.rb b/apps/workbench/test/controllers/container_requests_controller_test.rb
index 73d357f3a..c8709df3c 100644
--- a/apps/workbench/test/controllers/container_requests_controller_test.rb
+++ b/apps/workbench/test/controllers/container_requests_controller_test.rb
@@ -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
diff --git a/doc/api/methods/container_requests.html.textile.liquid b/doc/api/methods/container_requests.html.textile.liquid
index cd566f5ce..b24a24e06 100644
--- a/doc/api/methods/container_requests.html.textile.liquid
+++ b/doc/api/methods/container_requests.html.textile.liquid
@@ -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.|
diff --git a/lib/boot/supervisor.go b/lib/boot/supervisor.go
index f2e715a76..752466c2a 100644
--- a/lib/boot/supervisor.go
+++ b/lib/boot/supervisor.go
@@ -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{})
diff --git a/lib/controller/cmd.go b/lib/controller/cmd.go
index 0c46e857b..7ab7f5305 100644
--- a/lib/controller/cmd.go
+++ b/lib/controller/cmd.go
@@ -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 {
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 130368124..00523c782 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -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)
}
diff --git a/lib/controller/federation/generate.go b/lib/controller/federation/generate.go
index ab5d9966a..9ce7fdcb2 100644
--- a/lib/controller/federation/generate.go
+++ b/lib/controller/federation/generate.go
@@ -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)
diff --git a/lib/controller/federation/generated.go b/lib/controller/federation/generated.go
index 8745f3b97..ab9db93a4 100755
--- a/lib/controller/federation/generated.go
+++ b/lib/controller/federation/generated.go
@@ -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
diff --git a/lib/controller/federation_test.go b/lib/controller/federation_test.go
index 031166b29..a92fc7105 100644
--- a/lib/controller/federation_test.go
+++ b/lib/controller/federation_test.go
@@ -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) {
diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index 6669e020f..b04757ac3 100644
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -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)
}
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 0388f21be..a574f3978 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -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)
}
}
diff --git a/lib/controller/localdb/conn.go b/lib/controller/localdb/conn.go
index 4f0035edf..d197675f8 100644
--- a/lib/controller/localdb/conn.go
+++ b/lib/controller/localdb/conn.go
@@ -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)
}
diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go
index f4632751e..4bf515fc3 100644
--- a/lib/controller/localdb/login.go
+++ b/lib/controller/localdb/login.go
@@ -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.
diff --git a/lib/controller/localdb/login_ldap.go b/lib/controller/localdb/login_ldap.go
index 6c430d69b..49f557ae5 100644
--- a/lib/controller/localdb/login_ldap.go
+++ b/lib/controller/localdb/login_ldap.go
@@ -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"],
diff --git a/lib/controller/localdb/login_ldap_test.go b/lib/controller/localdb/login_ldap_test.go
index bce1ecfcf..b8ba6b467 100644
--- a/lib/controller/localdb/login_ldap_test.go
+++ b/lib/controller/localdb/login_ldap_test.go
@@ -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)
}
diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go
index 5f96da562..b99a1c2aa 100644
--- a/lib/controller/localdb/login_oidc.go
+++ b/lib/controller/localdb/login_oidc.go
@@ -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
}
diff --git a/lib/controller/localdb/login_pam.go b/lib/controller/localdb/login_pam.go
index 2447713a2..5d116a9e8 100644
--- a/lib/controller/localdb/login_pam.go
+++ b/lib/controller/localdb/login_pam.go
@@ -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,
})
diff --git a/lib/controller/localdb/login_pam_test.go b/lib/controller/localdb/login_pam_test.go
index e6b967c94..c5876bbfa 100644
--- a/lib/controller/localdb/login_pam_test.go
+++ b/lib/controller/localdb/login_pam_test.go
@@ -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)},
}
}
diff --git a/lib/controller/localdb/login_testuser.go b/lib/controller/localdb/login_testuser.go
index 585227352..c567a0668 100644
--- a/lib/controller/localdb/login_testuser.go
+++ b/lib/controller/localdb/login_testuser.go
@@ -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,
})
diff --git a/lib/controller/localdb/login_testuser_test.go b/lib/controller/localdb/login_testuser_test.go
index 758908889..7a520428b 100644
--- a/lib/controller/localdb/login_testuser_test.go
+++ b/lib/controller/localdb/login_testuser_test.go
@@ -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)
}
diff --git a/lib/controller/router/response.go b/lib/controller/router/response.go
index 543e25d0c..d554ab930 100644
--- a/lib/controller/router/response.go
+++ b/lib/controller/router/response.go
@@ -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
+ }
+ }
+ }
+}
diff --git a/lib/controller/router/router.go b/lib/controller/router/router.go
index 294452434..9fb2a0d32 100644
--- a/lib/controller/router/router.go
+++ b/lib/controller/router/router.go
@@ -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{} {
diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index cd98b6471..455c20fa9 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -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
diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index 341938354..730185c19 100644
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -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,
diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go
index eb83bbd41..dbdaa6293 100644
--- a/lib/crunchrun/crunchrun_test.go
+++ b/lib/crunchrun/crunchrun_test.go
@@ -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
}
{
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index 5a2cfb880..a11872971 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -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)
diff --git a/sdk/go/arvados/container.go b/sdk/go/arvados/container.go
index 265944e81..3ff7c5205 100644
--- a/sdk/go/arvados/container.go
+++ b/sdk/go/arvados/container.go
@@ -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"`
diff --git a/sdk/go/arvadostest/api.go b/sdk/go/arvadostest/api.go
index 039d7ae11..df3e46feb 100644
--- a/sdk/go/arvadostest/api.go
+++ b/sdk/go/arvadostest/api.go
@@ -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
diff --git a/services/api/app/controllers/user_sessions_controller.rb b/services/api/app/controllers/user_sessions_controller.rb
index 8e3c3ac5e..3d109c49c 100644
--- a/services/api/app/controllers/user_sessions_controller.rb
+++ b/services/api/app/controllers/user_sessions_controller.rb
@@ -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,
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 6a0a58f08..7d5bea8fa 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -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
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 5833c2251..d01787cbc 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -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
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 77536eee4..837f2cdc7 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -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
diff --git a/services/api/test/fixtures/container_requests.yml b/services/api/test/fixtures/container_requests.yml
index ab0400a67..1c6260502 100644
--- a/services/api/test/fixtures/container_requests.yml
+++ b/services/api/test/fixtures/container_requests.yml
@@ -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
diff --git a/services/api/test/functional/arvados/v1/container_requests_controller_test.rb b/services/api/test/functional/arvados/v1/container_requests_controller_test.rb
index 95c477f41..e99af39c9 100644
--- a/services/api/test/functional/arvados/v1/container_requests_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/container_requests_controller_test.rb
@@ -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)
diff --git a/services/api/test/functional/user_sessions_controller_test.rb b/services/api/test/functional/user_sessions_controller_test.rb
index d979208d3..129464cf1 100644
--- a/services/api/test/functional/user_sessions_controller_test.rb
+++ b/services/api/test/functional/user_sessions_controller_test.rb
@@ -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 at bar.com"}, return_to: ',https://app.example'}
+ else
+ get :create, params: {provider: 'controller', auth_info: {email: "foo at 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'
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 90de800b2..b2dde7995 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -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
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 98e60e057..4d8538524 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -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
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list