[ARVADOS] updated: 1.3.0-2234-gf39904077
Git user
git at public.arvados.org
Mon Apr 13 15:28:02 UTC 2020
Summary of changes:
apps/workbench/Gemfile.lock | 8 +-
.../config/initializers/actionview_xss_fix.rb | 32 +++
.../test/unit/helpers/javascript_helper_test.rb | 17 ++
lib/config/config.default.yml | 17 +-
lib/config/generated_config.go | 17 +-
lib/controller/federation/conn.go | 163 ++++++------
lib/controller/federation/list.go | 7 +
lib/controller/federation/list_test.go | 8 +-
lib/controller/federation/user_test.go | 38 +++
lib/controller/router/request.go | 1 +
lib/controller/rpc/conn.go | 20 +-
lib/dispatchcloud/container/queue.go | 7 +-
lib/dispatchcloud/container/queue_test.go | 7 +
lib/dispatchcloud/worker/pool.go | 6 +-
sdk/cwl/tests/federation/README | 2 +-
.../tests/federation/arvbox-make-federation.cwl | 12 +-
.../{arvbox => arvboxcwl}/fed-config.cwl | 6 +-
.../federation/{arvbox => arvboxcwl}/mkdir.cwl | 6 +-
.../{arvbox => arvboxcwl}/setup-user.cwl | 8 +-
.../federation/{arvbox => arvboxcwl}/setup_user.py | 0
.../federation/{arvbox => arvboxcwl}/start.cwl | 6 +-
.../federation/{arvbox => arvboxcwl}/stop.cwl | 0
sdk/go/arvados/api.go | 16 +-
sdk/go/arvados/config.go | 39 +--
sdk/go/arvados/config_test.go | 26 ++
sdk/go/arvados/resource_list.go | 2 +-
sdk/go/arvados/resource_list_test.go | 37 +++
sdk/python/arvados/commands/arv_copy.py | 294 +--------------------
sdk/python/arvados/commands/federation_migrate.py | 67 +++--
sdk/python/arvados/util.py | 2 +-
sdk/python/tests/fed-migrate/README | 2 +-
.../tests/fed-migrate/arvbox-make-federation.cwl | 4 +-
sdk/python/tests/fed-migrate/check.py | 72 +++--
services/api/Gemfile.lock | 4 +-
.../api/app/controllers/application_controller.rb | 36 +++
.../controllers/arvados/v1/schema_controller.rb | 4 +-
.../app/controllers/arvados/v1/users_controller.rb | 14 +-
.../functional/arvados/v1/users_controller_test.rb | 33 +++
services/api/test/integration/users_test.rb | 17 ++
services/keepstore/command.go | 5 +-
40 files changed, 576 insertions(+), 486 deletions(-)
create mode 100644 apps/workbench/config/initializers/actionview_xss_fix.rb
create mode 100644 apps/workbench/test/unit/helpers/javascript_helper_test.rb
rename sdk/cwl/tests/federation/{arvbox => arvboxcwl}/fed-config.cwl (96%)
rename sdk/cwl/tests/federation/{arvbox => arvboxcwl}/mkdir.cwl (91%)
rename sdk/cwl/tests/federation/{arvbox => arvboxcwl}/setup-user.cwl (87%)
rename sdk/cwl/tests/federation/{arvbox => arvboxcwl}/setup_user.py (100%)
rename sdk/cwl/tests/federation/{arvbox => arvboxcwl}/start.cwl (95%)
rename sdk/cwl/tests/federation/{arvbox => arvboxcwl}/stop.cwl (100%)
via f39904077030634f31fb0d1288fc963fb4cd22fb (commit)
via 2014c3c987831c25866906a9f8450f5c02fdae2c (commit)
via 51a627a92a84783adcc895fdf652be95fce1e3f0 (commit)
via d383273ba261256afae6112d3b0d4e2d03cc8240 (commit)
via 733215b7022250633485f599ee80f6aa36425343 (commit)
via 60661ac34a5a3af23f0db792c58401d8c5051ad1 (commit)
via bbf58b8ed64c47900c7204e70fd342db90eb8348 (commit)
via b79ae856450ab7442954b0454061173f8d3f540c (commit)
via 9574055ff8e6a1a96cc76bdccb139652bc158f27 (commit)
via b9836aad61698e653fbf7c941c1f818d412b69d2 (commit)
via 31943856d8150a70c3262d70f818af700ed2f177 (commit)
via a9546ab1a73665896dc4d6c6ad0ea5da3c5eaa66 (commit)
via 7a33961d2067a625d240a600318dd008eebe7361 (commit)
via c65d4fe115b500c6b248a1a8faa50eca3034a91e (commit)
via ede1354bb84dc5687d6cc588f16121165636de88 (commit)
via bf76fe28bf456e93e29b77804f9ba59539323235 (commit)
via a19837776bf215aa62cd53187cb60064c6205679 (commit)
via 5862a51baa10297165a3c49098f312f3e95a828e (commit)
via 0ed3f6e33bdc9d8debfd94f2cba8f388539aad32 (commit)
via 5efd7f8a01d6efc4480e2d6f460489a890ae913c (commit)
via 2cc2d6440dd0be3557af76f1670192268d87f82b (commit)
via 38db937185cba295ecffe0f4d78a65fab6ea686c (commit)
via d97308ee0db8ad22f8eb03de75ce55cf7e228afd (commit)
via 01baf16387609f936e45940561c0c289046a39d0 (commit)
via 29486979fc8a48788f5c12fec399d8d41588cba0 (commit)
via 4fbb5b4751488d9751710f03ebf99406d4c55410 (commit)
via da84332fc7b4a549e08e0be87d4b52ffe2eeddd9 (commit)
via 845381a133ebecfaff0335e432fda2fc0ac9f280 (commit)
via 8a125907af9a75bbf905de4b715309e562092704 (commit)
via 3eeae9396640383abeecfb3ea5289b75a144d969 (commit)
via 7781042d8602072fe95b607ad9c64713a46f1637 (commit)
via 6278100f84d7cc021932ce230ae7e713ed129b01 (commit)
via 891f67ea72aa35895c57421531b2b5cbbd25e70d (commit)
via eec034bf27818af1c3bd998e51c85772e7cdc9dd (commit)
via 89705b6140489b7badc42b6a67367dafaeca78d0 (commit)
via b7e6ce193a9158b9e4700fe9334018702dedb3d8 (commit)
via fca7b8b5b8d11938f2f1c2684a9a34814968f86a (commit)
via eeb4aa1bcf4462466170a4d3053d6c37f3497d7c (commit)
via 445465e4e859de272bee53c2eed3cd8f54e33d34 (commit)
via 7d1defd3d60fe165522a736855346c9d34eae28d (commit)
via 883f51266e8ac9275b7842704c56ff195f853080 (commit)
via 2713cc305c5065959da84b61e3a1b8867c72f4e8 (commit)
via a9866cc99faa6de3df2884b8ad97e858b922036c (commit)
via c4f5611d0e67028b86b5c468ee1a7f200866cb32 (commit)
from 7ad5beea6c92dbb13af52a380a86f8ca1b7e0ff8 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit f39904077030634f31fb0d1288fc963fb4cd22fb
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Apr 13 10:25:11 2020 -0400
16263: Add test for BypassFederation
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/federation/user_test.go b/lib/controller/federation/user_test.go
index c55ec24d4..09aa5086d 100644
--- a/lib/controller/federation/user_test.go
+++ b/lib/controller/federation/user_test.go
@@ -5,6 +5,7 @@
package federation
import (
+ "context"
"encoding/json"
"errors"
"math"
@@ -15,6 +16,8 @@ import (
"git.arvados.org/arvados.git/lib/controller/rpc"
"git.arvados.org/arvados.git/sdk/go/arvados"
"git.arvados.org/arvados.git/sdk/go/arvadostest"
+ "git.arvados.org/arvados.git/sdk/go/auth"
+ "git.arvados.org/arvados.git/sdk/go/ctxlog"
check "gopkg.in/check.v1"
)
@@ -114,6 +117,36 @@ func (s *UserSuite) TestLoginClusterUserList(c *check.C) {
}
}
+func (s *UserSuite) TestLoginClusterUserListBypassFederation(c *check.C) {
+ s.cluster.ClusterID = "local"
+ s.cluster.Login.LoginCluster = "zzzzz"
+ s.fed = New(s.cluster)
+ s.addDirectRemote(c, "zzzzz", rpc.NewConn("zzzzz", &url.URL{Scheme: "https", Host: os.Getenv("ARVADOS_API_HOST")},
+ true, rpc.PassthroughTokenProvider))
+
+ spy := arvadostest.NewProxy(c, s.cluster.Services.RailsAPI)
+ s.fed.local = rpc.NewConn(s.cluster.ClusterID, spy.URL, true, rpc.PassthroughTokenProvider)
+
+ _, err := s.fed.UserList(s.ctx, arvados.ListOptions{Offset: 0, Limit: math.MaxInt64, Select: nil, BypassFederation: true})
+ // this will fail because it is not using a root token
+ c.Check(err.(*arvados.TransactionError).StatusCode, check.Equals, 403)
+
+ // Now use SystemRootToken
+ ctx := context.Background()
+ ctx = ctxlog.Context(ctx, ctxlog.TestLogger(c))
+ ctx = auth.NewContext(ctx, &auth.Credentials{Tokens: []string{arvadostest.SystemRootToken}})
+
+ // Assert that it did not try to batch update users.
+ _, err = s.fed.UserList(ctx, arvados.ListOptions{Offset: 0, Limit: math.MaxInt64, Select: nil, BypassFederation: true})
+ for _, d := range spy.RequestDumps {
+ d := string(d)
+ if strings.Contains(d, "PATCH /arvados/v1/users/batch") {
+ c.Fail()
+ }
+ }
+ c.Check(err, check.IsNil)
+}
+
// userAttrsCachedFromLoginCluster must have an entry for every field
// in the User struct.
func (s *UserSuite) TestUserAttrsUpdateWhitelist(c *check.C) {
commit 2014c3c987831c25866906a9f8450f5c02fdae2c
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Mon Apr 6 11:01:18 2020 -0300
16263: Pulls from official git repo when running federation tests.
This is helpful to ask for specific branches that aren't published on github.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/sdk/cwl/tests/federation/arvboxcwl/start.cwl b/sdk/cwl/tests/federation/arvboxcwl/start.cwl
index 5d9aaba2d..a7f46d6b2 100644
--- a/sdk/cwl/tests/federation/arvboxcwl/start.cwl
+++ b/sdk/cwl/tests/federation/arvboxcwl/start.cwl
@@ -74,7 +74,7 @@ arguments:
mkdir -p $ARVBOX_DATA
if ! test -d $ARVBOX_DATA/arvados ; then
cd $ARVBOX_DATA
- git clone https://github.com/arvados/arvados.git
+ git clone https://git.arvados.org/arvados.git
fi
cd $ARVBOX_DATA/arvados
gitver=`git rev-parse HEAD`
commit 51a627a92a84783adcc895fdf652be95fce1e3f0
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Thu Apr 2 10:52:44 2020 -0400
16263: User migration test also checks federated user behavior
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/sdk/python/tests/fed-migrate/check.py b/sdk/python/tests/fed-migrate/check.py
index 8165b3eba..a2c009616 100644
--- a/sdk/python/tests/fed-migrate/check.py
+++ b/sdk/python/tests/fed-migrate/check.py
@@ -5,47 +5,54 @@ import sys
j = json.load(open(sys.argv[1]))
apiA = arvados.api(host=j["arvados_api_hosts"][0], token=j["superuser_tokens"][0], insecure=True)
-apiB = arvados.api(host=j["arvados_api_hosts"][1], token=j["superuser_tokens"][1], insecure=True)
-apiC = arvados.api(host=j["arvados_api_hosts"][2], token=j["superuser_tokens"][2], insecure=True)
+tok = apiA.api_client_authorizations().current().execute()
+v2_token = "v2/%s/%s" % (tok["uuid"], tok["api_token"])
+
+apiB = arvados.api(host=j["arvados_api_hosts"][1], token=v2_token, insecure=True)
+apiC = arvados.api(host=j["arvados_api_hosts"][2], token=v2_token, insecure=True)
###
### Check users on API server "A" (the LoginCluster) ###
###
-
-users = apiA.users().list(bypass_federation=True).execute()
-
-assert len(users["items"]) == 11
-
by_username = {}
-
-for i in range(1, 10):
+def check_A(users):
+ assert len(users["items"]) == 11
+
+ for i in range(1, 10):
+ found = False
+ for u in users["items"]:
+ if u["username"] == ("case%d" % i) and u["email"] == ("case%d at test" % i):
+ found = True
+ by_username[u["username"]] = u["uuid"]
+ assert found
+
+ # Should be active
+ for i in (1, 2, 3, 4, 5, 6, 7, 8):
+ found = False
+ for u in users["items"]:
+ if u["username"] == ("case%d" % i) and u["email"] == ("case%d at test" % i) and u["is_active"] is True:
+ found = True
+ assert found, "Not found case%i" % i
+
+ # case9 should not be active
found = False
for u in users["items"]:
- if u["username"] == ("case%d" % i) and u["email"] == ("case%d at test" % i):
+ if (u["username"] == "case9" and u["email"] == "case9 at test" and
+ u["uuid"] == by_username[u["username"]] and u["is_active"] is False):
found = True
- by_username[u["username"]] = u["uuid"]
assert found
-# Should be active
-for i in (1, 2, 3, 4, 5, 6, 7, 8):
- found = False
- for u in users["items"]:
- if u["username"] == ("case%d" % i) and u["email"] == ("case%d at test" % i) and u["is_active"] is True:
- found = True
- assert found, "Not found case%i" % i
-
-# case9 should not be active
-found = False
-for u in users["items"]:
- if (u["username"] == "case9" and u["email"] == "case9 at test" and
- u["uuid"] == by_username[u["username"]] and u["is_active"] is False):
- found = True
-assert found
+users = apiA.users().list().execute()
+check_A(users)
+users = apiA.users().list(bypass_federation=True).execute()
+check_A(users)
###
### Check users on API server "B" (federation member) ###
###
+
+# check for expected migrations on B
users = apiB.users().list(bypass_federation=True).execute()
assert len(users["items"]) == 11
@@ -64,10 +71,15 @@ for u in users["items"]:
found = True
assert found
+# check that federated user listing works
+users = apiB.users().list().execute()
+check_A(users)
###
### Check users on API server "C" (federation member) ###
###
+
+# check for expected migrations on C
users = apiC.users().list(bypass_federation=True).execute()
assert len(users["items"]) == 8
@@ -89,4 +101,8 @@ for i in (3, 5, 9):
found = True
assert not found
+# check that federated user listing works
+users = apiC.users().list().execute()
+check_A(users)
+
print("Passed checks")
commit d383273ba261256afae6112d3b0d4e2d03cc8240
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Mar 31 21:03:42 2020 -0400
16263: Tweak federation tests, use CWL 1.1
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/sdk/cwl/tests/federation/README b/sdk/cwl/tests/federation/README
index e5eb04c60..f97ca9726 100644
--- a/sdk/cwl/tests/federation/README
+++ b/sdk/cwl/tests/federation/README
@@ -26,7 +26,7 @@ Create main-test.json:
Or create an arvbox test cluster:
-$ cwltool --enable-ext arvbox-make-federation.cwl --arvbox_base ~/.arvbox/ --in_acr /path/to/arvados-cwl-runner > main-test.json
+$ cwltool arvbox-make-federation.cwl --arvbox_base ~/.arvbox/ --in_acr /path/to/arvados-cwl-runner > main-test.json
Run tests:
diff --git a/sdk/cwl/tests/federation/arvbox-make-federation.cwl b/sdk/cwl/tests/federation/arvbox-make-federation.cwl
index 5872dbef5..593f2399f 100644
--- a/sdk/cwl/tests/federation/arvbox-make-federation.cwl
+++ b/sdk/cwl/tests/federation/arvbox-make-federation.cwl
@@ -2,7 +2,7 @@
#
# SPDX-License-Identifier: Apache-2.0
-cwlVersion: v1.0
+cwlVersion: v1.1
class: Workflow
$namespaces:
arv: "http://arvados.org/cwl#"
@@ -10,7 +10,7 @@ $namespaces:
requirements:
ScatterFeatureRequirement: {}
StepInputExpressionRequirement: {}
- cwltool:LoadListingRequirement:
+ LoadListingRequirement:
loadListing: no_listing
InlineJavascriptRequirement: {}
inputs:
@@ -64,7 +64,7 @@ steps:
containers: containers
arvbox_base: arvbox_base
out: [arvbox_data]
- run: arvbox/mkdir.cwl
+ run: arvboxcwl/mkdir.cwl
start:
in:
container_name: containers
@@ -74,7 +74,7 @@ steps:
out: [cluster_id, container_host, arvbox_data_out, superuser_token]
scatter: [container_name, arvbox_data]
scatterMethod: dotproduct
- run: arvbox/start.cwl
+ run: arvboxcwl/start.cwl
fed-config:
in:
container_name: containers
@@ -87,10 +87,10 @@ steps:
out: []
scatter: [container_name, this_cluster_id, arvbox_data]
scatterMethod: dotproduct
- run: arvbox/fed-config.cwl
+ run: arvboxcwl/fed-config.cwl
setup-user:
in:
container_host: {source: start/container_host, valueFrom: "$(self[0])"}
superuser_token: {source: start/superuser_token, valueFrom: "$(self[0])"}
out: [test_user_uuid, test_user_token]
- run: arvbox/setup-user.cwl
+ run: arvboxcwl/setup-user.cwl
diff --git a/sdk/cwl/tests/federation/arvbox/fed-config.cwl b/sdk/cwl/tests/federation/arvboxcwl/fed-config.cwl
similarity index 96%
rename from sdk/cwl/tests/federation/arvbox/fed-config.cwl
rename to sdk/cwl/tests/federation/arvboxcwl/fed-config.cwl
index 37936df63..e1cacdcaf 100644
--- a/sdk/cwl/tests/federation/arvbox/fed-config.cwl
+++ b/sdk/cwl/tests/federation/arvboxcwl/fed-config.cwl
@@ -2,7 +2,7 @@
#
# SPDX-License-Identifier: Apache-2.0
-cwlVersion: v1.0
+cwlVersion: v1.1
class: CommandLineTool
$namespaces:
arv: "http://arvados.org/cwl#"
@@ -56,11 +56,11 @@ requirements:
}
return JSON.stringify({"development": {"remote_hosts": remoteClusters}});
}
- cwltool:LoadListingRequirement:
+ LoadListingRequirement:
loadListing: no_listing
ShellCommandRequirement: {}
InlineJavascriptRequirement: {}
- cwltool:InplaceUpdateRequirement:
+ InplaceUpdateRequirement:
inplaceUpdate: true
arguments:
- shellQuote: false
diff --git a/sdk/cwl/tests/federation/arvbox/mkdir.cwl b/sdk/cwl/tests/federation/arvboxcwl/mkdir.cwl
similarity index 91%
rename from sdk/cwl/tests/federation/arvbox/mkdir.cwl
rename to sdk/cwl/tests/federation/arvboxcwl/mkdir.cwl
index 727d491a3..854a727c6 100644
--- a/sdk/cwl/tests/federation/arvbox/mkdir.cwl
+++ b/sdk/cwl/tests/federation/arvboxcwl/mkdir.cwl
@@ -2,7 +2,7 @@
#
# SPDX-License-Identifier: Apache-2.0
-cwlVersion: v1.0
+cwlVersion: v1.1
class: CommandLineTool
$namespaces:
arv: "http://arvados.org/cwl#"
@@ -37,10 +37,10 @@ requirements:
- entry: $(inputs.arvbox_base)
entryname: base
writable: true
- cwltool:LoadListingRequirement:
+ LoadListingRequirement:
loadListing: no_listing
InlineJavascriptRequirement: {}
- cwltool:InplaceUpdateRequirement:
+ InplaceUpdateRequirement:
inplaceUpdate: true
arguments:
- mkdir
diff --git a/sdk/cwl/tests/federation/arvbox/setup-user.cwl b/sdk/cwl/tests/federation/arvboxcwl/setup-user.cwl
similarity index 87%
rename from sdk/cwl/tests/federation/arvbox/setup-user.cwl
rename to sdk/cwl/tests/federation/arvboxcwl/setup-user.cwl
index a3ad6e575..d8f2d9e12 100644
--- a/sdk/cwl/tests/federation/arvbox/setup-user.cwl
+++ b/sdk/cwl/tests/federation/arvboxcwl/setup-user.cwl
@@ -2,7 +2,7 @@
#
# SPDX-License-Identifier: Apache-2.0
-cwlVersion: v1.0
+cwlVersion: v1.1
class: CommandLineTool
$namespaces:
arv: "http://arvados.org/cwl#"
@@ -13,13 +13,15 @@ requirements:
ARVADOS_API_HOST: $(inputs.container_host)
ARVADOS_API_TOKEN: $(inputs.superuser_token)
ARVADOS_API_HOST_INSECURE: "true"
- cwltool:LoadListingRequirement:
+ LoadListingRequirement:
loadListing: no_listing
InlineJavascriptRequirement: {}
- cwltool:InplaceUpdateRequirement:
+ InplaceUpdateRequirement:
inplaceUpdate: true
DockerRequirement:
dockerPull: arvados/jobs
+ NetworkAccess:
+ networkAccess: true
inputs:
container_host: string
superuser_token: string
diff --git a/sdk/cwl/tests/federation/arvbox/setup_user.py b/sdk/cwl/tests/federation/arvboxcwl/setup_user.py
similarity index 100%
rename from sdk/cwl/tests/federation/arvbox/setup_user.py
rename to sdk/cwl/tests/federation/arvboxcwl/setup_user.py
diff --git a/sdk/cwl/tests/federation/arvbox/start.cwl b/sdk/cwl/tests/federation/arvboxcwl/start.cwl
similarity index 97%
rename from sdk/cwl/tests/federation/arvbox/start.cwl
rename to sdk/cwl/tests/federation/arvboxcwl/start.cwl
index 57b348973..5d9aaba2d 100644
--- a/sdk/cwl/tests/federation/arvbox/start.cwl
+++ b/sdk/cwl/tests/federation/arvboxcwl/start.cwl
@@ -2,7 +2,7 @@
#
# SPDX-License-Identifier: Apache-2.0
-cwlVersion: v1.0
+cwlVersion: v1.1
class: CommandLineTool
$namespaces:
arv: "http://arvados.org/cwl#"
@@ -64,7 +64,7 @@ requirements:
- entry: $(inputs.arvbox_data)
entryname: $(inputs.container_name)
writable: true
- cwltool:InplaceUpdateRequirement:
+ InplaceUpdateRequirement:
inplaceUpdate: true
InlineJavascriptRequirement: {}
arguments:
diff --git a/sdk/cwl/tests/federation/arvbox/stop.cwl b/sdk/cwl/tests/federation/arvboxcwl/stop.cwl
similarity index 100%
rename from sdk/cwl/tests/federation/arvbox/stop.cwl
rename to sdk/cwl/tests/federation/arvboxcwl/stop.cwl
diff --git a/sdk/python/tests/fed-migrate/README b/sdk/python/tests/fed-migrate/README
index 83d659d4d..1591b7e17 100644
--- a/sdk/python/tests/fed-migrate/README
+++ b/sdk/python/tests/fed-migrate/README
@@ -6,7 +6,7 @@ arv-federation-migrate should be in the path or the full path supplied
in the 'fed_migrate' input parameter.
# Create arvbox containers fedbox(1,2,3) for the federation
-$ cwltool --enable-ext arvbox-make-federation.cwl --arvbox_base ~/.arvbox > fed.json
+$ cwltool arvbox-make-federation.cwl --arvbox_base ~/.arvbox > fed.json
# Configure containers and run tests
$ cwltool fed-migrate.cwl fed.json
diff --git a/sdk/python/tests/fed-migrate/arvbox-make-federation.cwl b/sdk/python/tests/fed-migrate/arvbox-make-federation.cwl
index 0aa6f177a..aa859cba4 100644
--- a/sdk/python/tests/fed-migrate/arvbox-make-federation.cwl
+++ b/sdk/python/tests/fed-migrate/arvbox-make-federation.cwl
@@ -1,4 +1,4 @@
-cwlVersion: v1.0
+cwlVersion: v1.1
class: Workflow
$namespaces:
arv: "http://arvados.org/cwl#"
@@ -32,7 +32,7 @@ requirements:
SubworkflowFeatureRequirement: {}
ScatterFeatureRequirement: {}
StepInputExpressionRequirement: {}
- cwltool:LoadListingRequirement:
+ LoadListingRequirement:
loadListing: no_listing
steps:
start:
commit 733215b7022250633485f599ee80f6aa36425343
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Mar 31 17:42:03 2020 -0400
16263: Add bypass_federation test
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/test/integration/users_test.rb b/services/api/test/integration/users_test.rb
index ee230d514..ccc82a67f 100644
--- a/services/api/test/integration/users_test.rb
+++ b/services/api/test/integration/users_test.rb
@@ -440,5 +440,22 @@ class UsersTest < ActionDispatch::IntegrationTest
assert_match(/Cannot activate without being invited/, json_response['errors'][0])
end
+ test "bypass_federation only accepted for admins" do
+ get "/arvados/v1/users",
+ params: {
+ bypass_federation: true
+ },
+ headers: auth(:admin)
+
+ assert_response :success
+
+ get "/arvados/v1/users",
+ params: {
+ bypass_federation: true
+ },
+ headers: auth(:active)
+
+ assert_response 403
+ end
end
commit 60661ac34a5a3af23f0db792c58401d8c5051ad1
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Mar 31 17:16:57 2020 -0400
16263: Fix no_federation -> bypass_federation in boolParms
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/router/request.go b/lib/controller/router/request.go
index 88763524a..977a15f3a 100644
--- a/lib/controller/router/request.go
+++ b/lib/controller/router/request.go
@@ -169,7 +169,7 @@ var boolParams = map[string]bool{
"include_old_versions": true,
"redirect_to_new_user": true,
"send_notification_email": true,
- "no_federation": true,
+ "bypass_federation": true,
}
func stringToBool(s string) bool {
commit bbf58b8ed64c47900c7204e70fd342db90eb8348
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Mar 31 17:12:31 2020 -0400
16263: Fix only_admin_can_bypass_federation
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index a3435d0b6..83a233cd5 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -141,7 +141,7 @@ class ApplicationController < ActionController::Base
end
def only_admin_can_bypass_federation
- if params[:bypass_federation] && current_user.nil? or !current_user.is_admin
+ unless !params[:bypass_federation] || current_user.andand.is_admin
send_error("The bypass_federation parameter is only permitted when current user is admin", status: 403)
end
end
commit b79ae856450ab7442954b0454061173f8d3f540c
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Mar 31 16:45:49 2020 -0400
16263: Missed rename
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/federation/list.go b/lib/controller/federation/list.go
index d1d52cfc6..0a596eb9c 100644
--- a/lib/controller/federation/list.go
+++ b/lib/controller/federation/list.go
@@ -107,7 +107,7 @@ func (conn *Conn) generated_CollectionList(ctx context.Context, options arvados.
// backend.
func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions, fn func(context.Context, string, arvados.API, arvados.ListOptions) ([]string, error)) error {
- if opts.NoFederation {
+ if opts.BypassFederation {
// Client requested no federation. Pass through.
_, err := fn(ctx, conn.cluster.ClusterID, conn.local, opts)
return err
commit 9574055ff8e6a1a96cc76bdccb139652bc158f27
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Mar 31 16:33:27 2020 -0400
16263: Rename no_federation -> bypass_federation
Enforce if bypass_federation is true that user is admin.
Update API revision and make federation migrate check for it.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index a7c5518d5..8f5c9e0a3 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -448,7 +448,7 @@ func (conn *Conn) batchUpdateUsers(ctx context.Context,
}
func (conn *Conn) UserList(ctx context.Context, options arvados.ListOptions) (arvados.UserList, error) {
- if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID && !options.NoFederation {
+ if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID && !options.BypassFederation {
resp, err := conn.chooseBackend(id).UserList(ctx, options)
if err != nil {
return resp, err
@@ -468,7 +468,7 @@ func (conn *Conn) UserCreate(ctx context.Context, options arvados.CreateOptions)
}
func (conn *Conn) UserUpdate(ctx context.Context, options arvados.UpdateOptions) (arvados.User, error) {
- if options.NoFederation {
+ if options.BypassFederation {
return conn.local.UserUpdate(ctx, options)
}
return conn.chooseBackend(options.UUID).UserUpdate(ctx, options)
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index f3fdd254f..e60108335 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -84,7 +84,7 @@ type ListOptions struct {
Count string `json:"count"`
IncludeTrash bool `json:"include_trash"`
IncludeOldVersions bool `json:"include_old_versions"`
- NoFederation bool `json:"no_federation"`
+ BypassFederation bool `json:"bypass_federation"`
}
type CreateOptions struct {
@@ -95,9 +95,9 @@ type CreateOptions struct {
}
type UpdateOptions struct {
- UUID string `json:"uuid"`
- Attrs map[string]interface{} `json:"attrs"`
- NoFederation bool `json:"no_federation"`
+ UUID string `json:"uuid"`
+ Attrs map[string]interface{} `json:"attrs"`
+ BypassFederation bool `json:"bypass_federation"`
}
type UpdateUUIDOptions struct {
diff --git a/sdk/python/arvados/commands/federation_migrate.py b/sdk/python/arvados/commands/federation_migrate.py
index 0eaf1c03e..445775cce 100755
--- a/sdk/python/arvados/commands/federation_migrate.py
+++ b/sdk/python/arvados/commands/federation_migrate.py
@@ -66,8 +66,8 @@ def connect_clusters(args):
errors.append("Inconsistent login cluster configuration, expected '%s' on %s but was '%s'" % (loginCluster, config["ClusterID"], config["Login"]["LoginCluster"]))
continue
- if arv._rootDesc["revision"] < "20190926":
- errors.append("Arvados API server revision on cluster '%s' is too old, must be updated to at least Arvados 1.5 before running migration." % config["ClusterID"])
+ if arv._rootDesc["revision"] < "20200331":
+ errors.append("Arvados API server revision on cluster '%s' is too old, must be updated to at least Arvados 2.0.2 before running migration." % config["ClusterID"])
continue
try:
@@ -98,7 +98,7 @@ def fetch_users(clusters, loginCluster):
users = []
for c, arv in clusters.items():
print("Getting user list from %s" % c)
- ul = arvados.util.list_all(arv.users().list, no_federation=True)
+ ul = arvados.util.list_all(arv.users().list, bypass_federation=True)
for l in ul:
if l["uuid"].startswith(c):
users.append(l)
@@ -171,14 +171,14 @@ def update_username(args, email, user_uuid, username, migratecluster, migratearv
print("(%s) Updating username of %s to '%s' on %s" % (email, user_uuid, username, migratecluster))
if not args.dry_run:
try:
- conflicts = migratearv.users().list(filters=[["username", "=", username]], no_federation=True).execute()
+ conflicts = migratearv.users().list(filters=[["username", "=", username]], bypass_federation=True).execute()
if conflicts["items"]:
# There's already a user with the username, move the old user out of the way
migratearv.users().update(uuid=conflicts["items"][0]["uuid"],
- no_federation=True,
+ bypass_federation=True,
body={"user": {"username": username+"migrate"}}).execute()
migratearv.users().update(uuid=user_uuid,
- no_federation=True,
+ bypass_federation=True,
body={"user": {"username": username}}).execute()
except arvados.errors.ApiError as e:
print("(%s) Error updating username of %s to '%s' on %s: %s" % (email, user_uuid, username, migratecluster, e))
@@ -210,10 +210,10 @@ def choose_new_user(args, by_email, email, userhome, username, old_user_uuid, cl
try:
olduser = oldhomearv.users().get(uuid=old_user_uuid).execute()
conflicts = homearv.users().list(filters=[["username", "=", username]],
- no_federation=True).execute()
+ bypass_federation=True).execute()
if conflicts["items"]:
homearv.users().update(uuid=conflicts["items"][0]["uuid"],
- no_federation=True,
+ bypass_federation=True,
body={"user": {"username": username+"migrate"}}).execute()
user = homearv.users().create(body={"user": {"email": email, "username": username,
"is_active": olduser["is_active"]}}).execute()
@@ -250,7 +250,7 @@ def activate_remote_user(args, email, homearv, migratearv, old_user_uuid, new_us
return None
try:
- findolduser = migratearv.users().list(filters=[["uuid", "=", old_user_uuid]], no_federation=True).execute()
+ findolduser = migratearv.users().list(filters=[["uuid", "=", old_user_uuid]], bypass_federation=True).execute()
if len(findolduser["items"]) == 0:
return False
if len(findolduser["items"]) == 1:
@@ -280,7 +280,7 @@ def activate_remote_user(args, email, homearv, migratearv, old_user_uuid, new_us
print("(%s) Activating user %s on %s" % (email, new_user_uuid, migratecluster))
try:
if not args.dry_run:
- migratearv.users().update(uuid=new_user_uuid, no_federation=True,
+ migratearv.users().update(uuid=new_user_uuid, bypass_federation=True,
body={"is_active": True}).execute()
except arvados.errors.ApiError as e:
print("(%s) Could not activate user %s on %s: %s" % (email, new_user_uuid, migratecluster, e))
diff --git a/sdk/python/tests/fed-migrate/check.py b/sdk/python/tests/fed-migrate/check.py
index 85d2d31f2..8165b3eba 100644
--- a/sdk/python/tests/fed-migrate/check.py
+++ b/sdk/python/tests/fed-migrate/check.py
@@ -12,7 +12,7 @@ apiC = arvados.api(host=j["arvados_api_hosts"][2], token=j["superuser_tokens"][2
### Check users on API server "A" (the LoginCluster) ###
###
-users = apiA.users().list().execute()
+users = apiA.users().list(bypass_federation=True).execute()
assert len(users["items"]) == 11
@@ -46,7 +46,7 @@ assert found
###
### Check users on API server "B" (federation member) ###
###
-users = apiB.users().list().execute()
+users = apiB.users().list(bypass_federation=True).execute()
assert len(users["items"]) == 11
for i in range(2, 9):
@@ -68,7 +68,7 @@ assert found
###
### Check users on API server "C" (federation member) ###
###
-users = apiC.users().list().execute()
+users = apiC.users().list(bypass_federation=True).execute()
assert len(users["items"]) == 8
for i in (2, 4, 6, 7, 8):
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 68fa7d880..a3435d0b6 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -53,6 +53,7 @@ class ApplicationController < ActionController::Base
before_action :reload_object_before_update, :only => :update
before_action(:render_404_if_no_object,
except: [:index, :create] + ERROR_ACTIONS)
+ before_action :only_admin_can_bypass_federation
attr_writer :resource_attrs
@@ -139,6 +140,12 @@ class ApplicationController < ActionController::Base
render_not_found "Object not found" if !@object
end
+ def only_admin_can_bypass_federation
+ if params[:bypass_federation] && current_user.nil? or !current_user.is_admin
+ send_error("The bypass_federation parameter is only permitted when current user is admin", status: 403)
+ end
+ end
+
def render_error(e)
logger.error e.inspect
if e.respond_to? :backtrace and e.backtrace
@@ -656,7 +663,7 @@ class ApplicationController < ActionController::Base
location: "query",
required: false,
},
- no_federation: {
+ bypass_federation: {
type: 'boolean',
required: false,
description: 'bypass federation behavior, list items from local instance database only'
diff --git a/services/api/app/controllers/arvados/v1/schema_controller.rb b/services/api/app/controllers/arvados/v1/schema_controller.rb
index aee5d1f95..b9aba2726 100644
--- a/services/api/app/controllers/arvados/v1/schema_controller.rb
+++ b/services/api/app/controllers/arvados/v1/schema_controller.rb
@@ -33,10 +33,10 @@ class Arvados::V1::SchemaController < ApplicationController
id: "arvados:v1",
name: "arvados",
version: "v1",
- # format is YYYYMMDD, must be fixed with (needs to be linearly
+ # format is YYYYMMDD, must be fixed width (needs to be lexically
# sortable), updated manually, may be used by clients to
# determine availability of API server features.
- revision: "20190926",
+ revision: "20200331",
source_version: AppVersion.hash,
sourceVersion: AppVersion.hash, # source_version should be deprecated in the future
packageVersion: AppVersion.package_version,
diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index adb65bd7d..6a5fbbc50 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -248,7 +248,7 @@ class Arvados::V1::UsersController < ApplicationController
def self._update_requires_parameters
super.merge({
- no_federation: {
+ bypass_federation: {
type: 'boolean', required: false,
},
})
commit b9836aad61698e653fbf7c941c1f818d412b69d2
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Mar 31 14:42:46 2020 -0400
16263: Add no_federation to user update
We might agree on a different API but try this and see if it helps
pass the test.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 89b335e0d..a7c5518d5 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -468,6 +468,9 @@ func (conn *Conn) UserCreate(ctx context.Context, options arvados.CreateOptions)
}
func (conn *Conn) UserUpdate(ctx context.Context, options arvados.UpdateOptions) (arvados.User, error) {
+ if options.NoFederation {
+ return conn.local.UserUpdate(ctx, options)
+ }
return conn.chooseBackend(options.UUID).UserUpdate(ctx, options)
}
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index a30da6242..f3fdd254f 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -95,8 +95,9 @@ type CreateOptions struct {
}
type UpdateOptions struct {
- UUID string `json:"uuid"`
- Attrs map[string]interface{} `json:"attrs"`
+ UUID string `json:"uuid"`
+ Attrs map[string]interface{} `json:"attrs"`
+ NoFederation bool `json:"no_federation"`
}
type UpdateUUIDOptions struct {
diff --git a/sdk/python/arvados/commands/federation_migrate.py b/sdk/python/arvados/commands/federation_migrate.py
index e1b8ee7d8..0eaf1c03e 100755
--- a/sdk/python/arvados/commands/federation_migrate.py
+++ b/sdk/python/arvados/commands/federation_migrate.py
@@ -173,8 +173,13 @@ def update_username(args, email, user_uuid, username, migratecluster, migratearv
try:
conflicts = migratearv.users().list(filters=[["username", "=", username]], no_federation=True).execute()
if conflicts["items"]:
- migratearv.users().update(uuid=conflicts["items"][0]["uuid"], body={"user": {"username": username+"migrate"}}).execute()
- migratearv.users().update(uuid=user_uuid, body={"user": {"username": username}}).execute()
+ # There's already a user with the username, move the old user out of the way
+ migratearv.users().update(uuid=conflicts["items"][0]["uuid"],
+ no_federation=True,
+ body={"user": {"username": username+"migrate"}}).execute()
+ migratearv.users().update(uuid=user_uuid,
+ no_federation=True,
+ body={"user": {"username": username}}).execute()
except arvados.errors.ApiError as e:
print("(%s) Error updating username of %s to '%s' on %s: %s" % (email, user_uuid, username, migratecluster, e))
@@ -204,10 +209,14 @@ def choose_new_user(args, by_email, email, userhome, username, old_user_uuid, cl
user = None
try:
olduser = oldhomearv.users().get(uuid=old_user_uuid).execute()
- conflicts = homearv.users().list(filters=[["username", "=", username]], no_federation=True).execute()
+ conflicts = homearv.users().list(filters=[["username", "=", username]],
+ no_federation=True).execute()
if conflicts["items"]:
- homearv.users().update(uuid=conflicts["items"][0]["uuid"], body={"user": {"username": username+"migrate"}}).execute()
- user = homearv.users().create(body={"user": {"email": email, "username": username, "is_active": olduser["is_active"]}}).execute()
+ homearv.users().update(uuid=conflicts["items"][0]["uuid"],
+ no_federation=True,
+ body={"user": {"username": username+"migrate"}}).execute()
+ user = homearv.users().create(body={"user": {"email": email, "username": username,
+ "is_active": olduser["is_active"]}}).execute()
except arvados.errors.ApiError as e:
print("(%s) Could not create user: %s" % (email, str(e)))
return None
@@ -259,7 +268,8 @@ def activate_remote_user(args, email, homearv, migratearv, old_user_uuid, new_us
try:
ru = urllib.parse.urlparse(migratearv._rootDesc["rootUrl"])
if not args.dry_run:
- newuser = arvados.api(host=ru.netloc, token=salted, insecure=os.environ.get("ARVADOS_API_HOST_INSECURE")).users().current().execute()
+ newuser = arvados.api(host=ru.netloc, token=salted,
+ insecure=os.environ.get("ARVADOS_API_HOST_INSECURE")).users().current().execute()
else:
newuser = {"is_active": True, "username": username}
except arvados.errors.ApiError as e:
@@ -270,7 +280,8 @@ def activate_remote_user(args, email, homearv, migratearv, old_user_uuid, new_us
print("(%s) Activating user %s on %s" % (email, new_user_uuid, migratecluster))
try:
if not args.dry_run:
- migratearv.users().update(uuid=new_user_uuid, body={"is_active": True}).execute()
+ migratearv.users().update(uuid=new_user_uuid, no_federation=True,
+ body={"is_active": True}).execute()
except arvados.errors.ApiError as e:
print("(%s) Could not activate user %s on %s: %s" % (email, new_user_uuid, migratecluster, e))
return None
diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index eb2402b80..adb65bd7d 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -246,6 +246,14 @@ class Arvados::V1::UsersController < ApplicationController
}
end
+ def self._update_requires_parameters
+ super.merge({
+ no_federation: {
+ type: 'boolean', required: false,
+ },
+ })
+ end
+
def self._update_uuid_requires_parameters
{
new_uuid: {
commit 31943856d8150a70c3262d70f818af700ed2f177
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Mar 30 22:30:10 2020 -0400
16263: local_user_list -> no_federation in boolParams
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/router/request.go b/lib/controller/router/request.go
index d769c5006..88763524a 100644
--- a/lib/controller/router/request.go
+++ b/lib/controller/router/request.go
@@ -169,7 +169,7 @@ var boolParams = map[string]bool{
"include_old_versions": true,
"redirect_to_new_user": true,
"send_notification_email": true,
- "local_user_list": true,
+ "no_federation": true,
}
func stringToBool(s string) bool {
commit a9546ab1a73665896dc4d6c6ad0ea5da3c5eaa66
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Mar 30 22:14:23 2020 -0400
16263: Generalize "local_user_list" flag to "no_federation"
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 21c156088..89b335e0d 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -386,64 +386,76 @@ var userAttrsCachedFromLoginCluster = map[string]bool{
"writable_by": false,
}
-func (conn *Conn) UserList(ctx context.Context, options arvados.ListOptions) (arvados.UserList, error) {
+func (conn *Conn) batchUpdateUsers(ctx context.Context,
+ options arvados.ListOptions,
+ items []arvados.User) (err error) {
+
+ id := conn.cluster.Login.LoginCluster
logger := ctxlog.FromContext(ctx)
- if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID && !options.LocalUserList {
- resp, err := conn.chooseBackend(id).UserList(ctx, options)
- if err != nil {
- return resp, err
+ batchOpts := arvados.UserBatchUpdateOptions{Updates: map[string]map[string]interface{}{}}
+ for _, user := range items {
+ if !strings.HasPrefix(user.UUID, id) {
+ continue
+ }
+ logger.Debugf("cache user info for uuid %q", user.UUID)
+
+ // If the remote cluster has null timestamps
+ // (e.g., test server with incomplete
+ // fixtures) use dummy timestamps (instead of
+ // the zero time, which causes a Rails API
+ // error "year too big to marshal: 1 UTC").
+ if user.ModifiedAt.IsZero() {
+ user.ModifiedAt = time.Now()
+ }
+ if user.CreatedAt.IsZero() {
+ user.CreatedAt = time.Now()
}
- batchOpts := arvados.UserBatchUpdateOptions{Updates: map[string]map[string]interface{}{}}
- for _, user := range resp.Items {
- if !strings.HasPrefix(user.UUID, id) {
- continue
- }
- logger.Debugf("cache user info for uuid %q", user.UUID)
-
- // If the remote cluster has null timestamps
- // (e.g., test server with incomplete
- // fixtures) use dummy timestamps (instead of
- // the zero time, which causes a Rails API
- // error "year too big to marshal: 1 UTC").
- if user.ModifiedAt.IsZero() {
- user.ModifiedAt = time.Now()
- }
- if user.CreatedAt.IsZero() {
- user.CreatedAt = time.Now()
- }
- var allFields map[string]interface{}
- buf, err := json.Marshal(user)
- if err != nil {
- return arvados.UserList{}, fmt.Errorf("error encoding user record from remote response: %s", err)
- }
- err = json.Unmarshal(buf, &allFields)
- if err != nil {
- return arvados.UserList{}, fmt.Errorf("error transcoding user record from remote response: %s", err)
- }
- updates := allFields
- if len(options.Select) > 0 {
- updates = map[string]interface{}{}
- for _, k := range options.Select {
- if v, ok := allFields[k]; ok && userAttrsCachedFromLoginCluster[k] {
- updates[k] = v
- }
+ var allFields map[string]interface{}
+ buf, err := json.Marshal(user)
+ if err != nil {
+ return fmt.Errorf("error encoding user record from remote response: %s", err)
+ }
+ err = json.Unmarshal(buf, &allFields)
+ if err != nil {
+ return fmt.Errorf("error transcoding user record from remote response: %s", err)
+ }
+ updates := allFields
+ if len(options.Select) > 0 {
+ updates = map[string]interface{}{}
+ for _, k := range options.Select {
+ if v, ok := allFields[k]; ok && userAttrsCachedFromLoginCluster[k] {
+ updates[k] = v
}
- } else {
- for k := range updates {
- if !userAttrsCachedFromLoginCluster[k] {
- delete(updates, k)
- }
+ }
+ } else {
+ for k := range updates {
+ if !userAttrsCachedFromLoginCluster[k] {
+ delete(updates, k)
}
}
- batchOpts.Updates[user.UUID] = updates
}
- if len(batchOpts.Updates) > 0 {
- ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
- _, err = conn.local.UserBatchUpdate(ctxRoot, batchOpts)
- if err != nil {
- return arvados.UserList{}, fmt.Errorf("error updating local user records: %s", err)
- }
+ batchOpts.Updates[user.UUID] = updates
+ }
+ if len(batchOpts.Updates) > 0 {
+ ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
+ _, err = conn.local.UserBatchUpdate(ctxRoot, batchOpts)
+ if err != nil {
+ return fmt.Errorf("error updating local user records: %s", err)
+ }
+ }
+ return nil
+}
+
+func (conn *Conn) UserList(ctx context.Context, options arvados.ListOptions) (arvados.UserList, error) {
+ if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID && !options.NoFederation {
+ resp, err := conn.chooseBackend(id).UserList(ctx, options)
+ if err != nil {
+ return resp, err
+ }
+ err = conn.batchUpdateUsers(ctx, options, resp.Items)
+ if err != nil {
+ return arvados.UserList{}, err
}
return resp, nil
} else {
@@ -460,7 +472,7 @@ func (conn *Conn) UserUpdate(ctx context.Context, options arvados.UpdateOptions)
}
func (conn *Conn) UserUpdateUUID(ctx context.Context, options arvados.UpdateUUIDOptions) (arvados.User, error) {
- return conn.chooseBackend(options.UUID).UserUpdateUUID(ctx, options)
+ return conn.local.UserUpdateUUID(ctx, options)
}
func (conn *Conn) UserMerge(ctx context.Context, options arvados.UserMergeOptions) (arvados.User, error) {
diff --git a/lib/controller/federation/list.go b/lib/controller/federation/list.go
index 6ee813317..d1d52cfc6 100644
--- a/lib/controller/federation/list.go
+++ b/lib/controller/federation/list.go
@@ -106,6 +106,13 @@ func (conn *Conn) generated_CollectionList(ctx context.Context, options arvados.
// corresponding options argument suitable for sending to that
// backend.
func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions, fn func(context.Context, string, arvados.API, arvados.ListOptions) ([]string, error)) error {
+
+ if opts.NoFederation {
+ // Client requested no federation. Pass through.
+ _, err := fn(ctx, conn.cluster.ClusterID, conn.local, opts)
+ return err
+ }
+
cannotSplit := false
var matchAllFilters map[string]bool
for _, f := range opts.Filters {
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index 3b20955a1..a30da6242 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -84,7 +84,7 @@ type ListOptions struct {
Count string `json:"count"`
IncludeTrash bool `json:"include_trash"`
IncludeOldVersions bool `json:"include_old_versions"`
- LocalUserList bool `json:"local_user_list"`
+ NoFederation bool `json:"no_federation"`
}
type CreateOptions struct {
diff --git a/sdk/python/arvados/commands/federation_migrate.py b/sdk/python/arvados/commands/federation_migrate.py
index 344390b48..e1b8ee7d8 100755
--- a/sdk/python/arvados/commands/federation_migrate.py
+++ b/sdk/python/arvados/commands/federation_migrate.py
@@ -98,7 +98,7 @@ def fetch_users(clusters, loginCluster):
users = []
for c, arv in clusters.items():
print("Getting user list from %s" % c)
- ul = arvados.util.list_all(arv.users().list, local_user_list=True)
+ ul = arvados.util.list_all(arv.users().list, no_federation=True)
for l in ul:
if l["uuid"].startswith(c):
users.append(l)
@@ -171,7 +171,7 @@ def update_username(args, email, user_uuid, username, migratecluster, migratearv
print("(%s) Updating username of %s to '%s' on %s" % (email, user_uuid, username, migratecluster))
if not args.dry_run:
try:
- conflicts = migratearv.users().list(filters=[["username", "=", username]], local_user_list=True).execute()
+ conflicts = migratearv.users().list(filters=[["username", "=", username]], no_federation=True).execute()
if conflicts["items"]:
migratearv.users().update(uuid=conflicts["items"][0]["uuid"], body={"user": {"username": username+"migrate"}}).execute()
migratearv.users().update(uuid=user_uuid, body={"user": {"username": username}}).execute()
@@ -204,7 +204,7 @@ def choose_new_user(args, by_email, email, userhome, username, old_user_uuid, cl
user = None
try:
olduser = oldhomearv.users().get(uuid=old_user_uuid).execute()
- conflicts = homearv.users().list(filters=[["username", "=", username]], local_user_list=True).execute()
+ conflicts = homearv.users().list(filters=[["username", "=", username]], no_federation=True).execute()
if conflicts["items"]:
homearv.users().update(uuid=conflicts["items"][0]["uuid"], body={"user": {"username": username+"migrate"}}).execute()
user = homearv.users().create(body={"user": {"email": email, "username": username, "is_active": olduser["is_active"]}}).execute()
@@ -241,10 +241,16 @@ def activate_remote_user(args, email, homearv, migratearv, old_user_uuid, new_us
return None
try:
- olduser = migratearv.users().get(uuid=old_user_uuid).execute()
+ findolduser = migratearv.users().list(filters=[["uuid", "=", old_user_uuid]], no_federation=True).execute()
+ if len(findolduser["items"]) == 0:
+ return False
+ if len(findolduser["items"]) == 1:
+ olduser = findolduser["items"][0]
+ else:
+ print("(%s) Unexpected result" % (email))
+ return None
except arvados.errors.ApiError as e:
- if e.resp.status != 404:
- print("(%s) Could not retrieve user %s from %s, user may have already been migrated: %s" % (email, old_user_uuid, migratecluster, e))
+ print("(%s) Could not retrieve user %s from %s, user may have already been migrated: %s" % (email, old_user_uuid, migratecluster, e))
return None
salted = 'v2/' + newtok["uuid"] + '/' + hmac.new(newtok["api_token"].encode(),
@@ -351,10 +357,10 @@ def main():
if new_user_uuid is None:
continue
- # cluster where the migration is happening
remote_users = {}
got_error = False
for migratecluster in clusters:
+ # cluster where the migration is happening
migratearv = clusters[migratecluster]
# the user's new home cluster
@@ -370,6 +376,8 @@ def main():
for migratecluster in clusters:
migratearv = clusters[migratecluster]
newuser = remote_users[migratecluster]
+ if newuser is False:
+ continue
print("(%s) Migrating %s to %s on %s" % (email, old_user_uuid, new_user_uuid, migratecluster))
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 7b82cdb61..68fa7d880 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -656,6 +656,11 @@ class ApplicationController < ActionController::Base
location: "query",
required: false,
},
+ no_federation: {
+ type: 'boolean',
+ required: false,
+ description: 'bypass federation behavior, list items from local instance database only'
+ }
}
end
diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index c94fa113e..eb2402b80 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -254,12 +254,6 @@ class Arvados::V1::UsersController < ApplicationController
}
end
- def self._index_requires_parameters
- super.merge(
- { local_user_list: {required: false, type: 'boolean',
- description: 'only list users from local database, no effect if LoginCluster is not set'} })
- end
-
def apply_filters(model_class=nil)
return super if @read_users.any?(&:is_admin)
if params[:uuid] != current_user.andand.uuid
commit 7a33961d2067a625d240a600318dd008eebe7361
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Mar 30 17:12:17 2020 -0400
16263: UserMerge shouldn't be federated
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index dc676c263..21c156088 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -464,7 +464,7 @@ func (conn *Conn) UserUpdateUUID(ctx context.Context, options arvados.UpdateUUID
}
func (conn *Conn) UserMerge(ctx context.Context, options arvados.UserMergeOptions) (arvados.User, error) {
- return conn.chooseBackend(options.OldUserUUID).UserMerge(ctx, options)
+ return conn.local.UserMerge(ctx, options)
}
func (conn *Conn) UserActivate(ctx context.Context, options arvados.UserActivateOptions) (arvados.User, error) {
commit c65d4fe115b500c6b248a1a8faa50eca3034a91e
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Mar 30 14:50:06 2020 -0400
16263: Fix omitempty placement
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index d5f5dcaf8..3b20955a1 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -60,11 +60,11 @@ var (
)
type GetOptions struct {
- UUID string `json:"uuid",omitempty`
+ UUID string `json:"uuid,omitempty"`
Select []string `json:"select"`
IncludeTrash bool `json:"include_trash"`
- ForwardedFor string `json:"forwarded_for",omitempty`
- Remote string `json:"remote",omitempty`
+ ForwardedFor string `json:"forwarded_for,omitempty"`
+ Remote string `json:"remote,omitempty"`
}
type UntrashOptions struct {
commit ede1354bb84dc5687d6cc588f16121165636de88
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Mar 30 14:43:36 2020 -0400
16263: Add omitempty to GetOptions
Don't try to migrate if activate_remote_user fails.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index 7bb3bfaa7..d5f5dcaf8 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -60,11 +60,11 @@ var (
)
type GetOptions struct {
- UUID string `json:"uuid"`
+ UUID string `json:"uuid",omitempty`
Select []string `json:"select"`
IncludeTrash bool `json:"include_trash"`
- ForwardedFor string `json:"forwarded_for"`
- Remote string `json:"remote"`
+ ForwardedFor string `json:"forwarded_for",omitempty`
+ Remote string `json:"remote",omitempty`
}
type UntrashOptions struct {
diff --git a/sdk/python/arvados/commands/federation_migrate.py b/sdk/python/arvados/commands/federation_migrate.py
index b66aa5d7c..344390b48 100755
--- a/sdk/python/arvados/commands/federation_migrate.py
+++ b/sdk/python/arvados/commands/federation_migrate.py
@@ -352,6 +352,8 @@ def main():
continue
# cluster where the migration is happening
+ remote_users = {}
+ got_error = False
for migratecluster in clusters:
migratearv = clusters[migratecluster]
@@ -361,14 +363,20 @@ def main():
newuser = activate_remote_user(args, email, homearv, migratearv, old_user_uuid, new_user_uuid)
if newuser is None:
- continue
+ got_error = True
+ remote_users[migratecluster] = newuser
+
+ if not got_error:
+ for migratecluster in clusters:
+ migratearv = clusters[migratecluster]
+ newuser = remote_users[migratecluster]
- print("(%s) Migrating %s to %s on %s" % (email, old_user_uuid, new_user_uuid, migratecluster))
+ print("(%s) Migrating %s to %s on %s" % (email, old_user_uuid, new_user_uuid, migratecluster))
- migrate_user(args, migratearv, email, new_user_uuid, old_user_uuid)
+ migrate_user(args, migratearv, email, new_user_uuid, old_user_uuid)
- if newuser['username'] != username:
- update_username(args, email, new_user_uuid, username, migratecluster, migratearv)
+ if newuser['username'] != username:
+ update_username(args, email, new_user_uuid, username, migratecluster, migratearv)
if __name__ == "__main__":
main()
commit bf76fe28bf456e93e29b77804f9ba59539323235
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri Mar 27 21:36:09 2020 -0400
16263: Add local_user_list to boolParams
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/router/request.go b/lib/controller/router/request.go
index 39b4c5100..d769c5006 100644
--- a/lib/controller/router/request.go
+++ b/lib/controller/router/request.go
@@ -169,6 +169,7 @@ var boolParams = map[string]bool{
"include_old_versions": true,
"redirect_to_new_user": true,
"send_notification_email": true,
+ "local_user_list": true,
}
func stringToBool(s string) bool {
commit a19837776bf215aa62cd53187cb60064c6205679
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri Mar 27 17:08:48 2020 -0400
16263: Add local_user_list flag to bypass LoginCluster behavior
Required by federation migrate script.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 226bfabf1..dc676c263 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -388,7 +388,7 @@ var userAttrsCachedFromLoginCluster = map[string]bool{
func (conn *Conn) UserList(ctx context.Context, options arvados.ListOptions) (arvados.UserList, error) {
logger := ctxlog.FromContext(ctx)
- if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID {
+ if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID && !options.LocalUserList {
resp, err := conn.chooseBackend(id).UserList(ctx, options)
if err != nil {
return resp, err
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index 4eb5b61b3..7bb3bfaa7 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -84,6 +84,7 @@ type ListOptions struct {
Count string `json:"count"`
IncludeTrash bool `json:"include_trash"`
IncludeOldVersions bool `json:"include_old_versions"`
+ LocalUserList bool `json:"local_user_list"`
}
type CreateOptions struct {
diff --git a/sdk/python/arvados/commands/federation_migrate.py b/sdk/python/arvados/commands/federation_migrate.py
index e74d6215c..b66aa5d7c 100755
--- a/sdk/python/arvados/commands/federation_migrate.py
+++ b/sdk/python/arvados/commands/federation_migrate.py
@@ -98,7 +98,7 @@ def fetch_users(clusters, loginCluster):
users = []
for c, arv in clusters.items():
print("Getting user list from %s" % c)
- ul = arvados.util.list_all(arv.users().list)
+ ul = arvados.util.list_all(arv.users().list, local_user_list=True)
for l in ul:
if l["uuid"].startswith(c):
users.append(l)
@@ -171,7 +171,7 @@ def update_username(args, email, user_uuid, username, migratecluster, migratearv
print("(%s) Updating username of %s to '%s' on %s" % (email, user_uuid, username, migratecluster))
if not args.dry_run:
try:
- conflicts = migratearv.users().list(filters=[["username", "=", username]]).execute()
+ conflicts = migratearv.users().list(filters=[["username", "=", username]], local_user_list=True).execute()
if conflicts["items"]:
migratearv.users().update(uuid=conflicts["items"][0]["uuid"], body={"user": {"username": username+"migrate"}}).execute()
migratearv.users().update(uuid=user_uuid, body={"user": {"username": username}}).execute()
@@ -204,7 +204,7 @@ def choose_new_user(args, by_email, email, userhome, username, old_user_uuid, cl
user = None
try:
olduser = oldhomearv.users().get(uuid=old_user_uuid).execute()
- conflicts = homearv.users().list(filters=[["username", "=", username]]).execute()
+ conflicts = homearv.users().list(filters=[["username", "=", username]], local_user_list=True).execute()
if conflicts["items"]:
homearv.users().update(uuid=conflicts["items"][0]["uuid"], body={"user": {"username": username+"migrate"}}).execute()
user = homearv.users().create(body={"user": {"email": email, "username": username, "is_active": olduser["is_active"]}}).execute()
diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index eb2402b80..c94fa113e 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -254,6 +254,12 @@ class Arvados::V1::UsersController < ApplicationController
}
end
+ def self._index_requires_parameters
+ super.merge(
+ { local_user_list: {required: false, type: 'boolean',
+ description: 'only list users from local database, no effect if LoginCluster is not set'} })
+ end
+
def apply_filters(model_class=nil)
return super if @read_users.any?(&:is_admin)
if params[:uuid] != current_user.andand.uuid
commit 5862a51baa10297165a3c49098f312f3e95a828e
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Thu Mar 26 11:46:34 2020 -0300
16263: Decodes JSON numbers as strings instead of float64.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index 4b143b770..b5c56dbc4 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -5,6 +5,7 @@
package rpc
import (
+ "bytes"
"context"
"crypto/tls"
"encoding/json"
@@ -14,6 +15,7 @@ import (
"net"
"net/http"
"net/url"
+ "strconv"
"strings"
"time"
@@ -100,19 +102,23 @@ func (conn *Conn) requestAndDecode(ctx context.Context, dst interface{}, ep arva
return fmt.Errorf("%T: requestAndDecode: Marshal opts: %s", conn, err)
}
var params map[string]interface{}
- err = json.Unmarshal(j, ¶ms)
+ dec := json.NewDecoder(bytes.NewBuffer(j))
+ dec.UseNumber()
+ err = dec.Decode(¶ms)
if err != nil {
- return fmt.Errorf("%T: requestAndDecode: Unmarshal opts: %s", conn, err)
+ return fmt.Errorf("%T: requestAndDecode: Decode opts: %s", conn, err)
}
if attrs, ok := params["attrs"]; ok && ep.AttrsKey != "" {
params[ep.AttrsKey] = attrs
delete(params, "attrs")
}
- if limit, ok := params["limit"].(float64); ok && limit < 0 {
- // Negative limit means "not specified" here, but some
- // servers/versions do not accept that, so we need to
- // remove it entirely.
- delete(params, "limit")
+ if limitStr, ok := params["limit"]; ok {
+ if limit, err := strconv.ParseInt(string(limitStr.(json.Number)), 10, 64); err == nil && limit < 0 {
+ // Negative limit means "not specified" here, but some
+ // servers/versions do not accept that, so we need to
+ // remove it entirely.
+ delete(params, "limit")
+ }
}
if len(tokens) > 1 {
params["reader_tokens"] = tokens[1:]
commit 0ed3f6e33bdc9d8debfd94f2cba8f388539aad32
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Wed Mar 25 16:35:18 2020 -0300
16263: Adds unit test case confirming 'limit' bug.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/lib/controller/federation/list_test.go b/lib/controller/federation/list_test.go
index ce84378a3..e6d2816f6 100644
--- a/lib/controller/federation/list_test.go
+++ b/lib/controller/federation/list_test.go
@@ -58,7 +58,7 @@ func (cl *collectionLister) CollectionList(ctx context.Context, options arvados.
if cl.MaxPageSize > 0 && len(resp.Items) >= cl.MaxPageSize {
break
}
- if options.Limit >= 0 && len(resp.Items) >= options.Limit {
+ if options.Limit >= 0 && int64(len(resp.Items)) >= options.Limit {
break
}
if cl.matchFilters(c, options.Filters) {
@@ -115,8 +115,8 @@ func (s *CollectionListSuite) SetUpTest(c *check.C) {
type listTrial struct {
count string
- limit int
- offset int
+ limit int64
+ offset int64
order []string
filters []arvados.Filter
selectfields []string
@@ -314,7 +314,7 @@ func (s *CollectionListSuite) TestCollectionListMultiSiteWithCount(c *check.C) {
}
func (s *CollectionListSuite) TestCollectionListMultiSiteWithLimit(c *check.C) {
- for _, limit := range []int{0, 1, 2} {
+ for _, limit := range []int64{0, 1, 2} {
s.test(c, listTrial{
count: "none",
limit: limit,
diff --git a/lib/controller/federation/user_test.go b/lib/controller/federation/user_test.go
index c087273af..c55ec24d4 100644
--- a/lib/controller/federation/user_test.go
+++ b/lib/controller/federation/user_test.go
@@ -7,6 +7,7 @@ package federation
import (
"encoding/json"
"errors"
+ "math"
"net/url"
"os"
"strings"
@@ -32,6 +33,7 @@ func (s *UserSuite) TestLoginClusterUserList(c *check.C) {
for _, updateFail := range []bool{false, true} {
for _, opts := range []arvados.ListOptions{
{Offset: 0, Limit: -1, Select: nil},
+ {Offset: 0, Limit: math.MaxInt64, Select: nil},
{Offset: 1, Limit: 1, Select: nil},
{Offset: 0, Limit: 2, Select: []string{"uuid"}},
{Offset: 0, Limit: 2, Select: []string{"uuid", "email"}},
@@ -45,6 +47,9 @@ func (s *UserSuite) TestLoginClusterUserList(c *check.C) {
s.fed.local = rpc.NewConn(s.cluster.ClusterID, spy.URL, true, rpc.PassthroughTokenProvider)
}
userlist, err := s.fed.UserList(s.ctx, opts)
+ if err != nil {
+ c.Logf("... UserList failed %q", err)
+ }
if updateFail && err == nil {
// All local updates fail, so the only
// cases expected to succeed are the
commit 5efd7f8a01d6efc4480e2d6f460489a890ae913c
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Wed Mar 25 11:26:55 2020 -0300
16263: Adds test exposing a bug when using 'limit' with the max int64 value.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index 0c5d32e8b..4eb5b61b3 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -77,8 +77,8 @@ type ListOptions struct {
Select []string `json:"select"`
Filters []Filter `json:"filters"`
Where map[string]interface{} `json:"where"`
- Limit int `json:"limit"`
- Offset int `json:"offset"`
+ Limit int64 `json:"limit"`
+ Offset int64 `json:"offset"`
Order []string `json:"order"`
Distinct bool `json:"distinct"`
Count string `json:"count"`
commit 2cc2d6440dd0be3557af76f1670192268d87f82b
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Tue Mar 24 17:39:01 2020 -0300
16263: Bool filter bug fix.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/sdk/go/arvados/resource_list.go b/sdk/go/arvados/resource_list.go
index d1a25c438..a5cc7d3b9 100644
--- a/sdk/go/arvados/resource_list.go
+++ b/sdk/go/arvados/resource_list.go
@@ -55,7 +55,7 @@ func (f *Filter) UnmarshalJSON(data []byte) error {
}
operand := elements[2]
switch operand.(type) {
- case string, float64, []interface{}, nil:
+ case string, float64, []interface{}, nil, bool:
default:
return fmt.Errorf("invalid filter operand %q", elements[2])
}
commit 38db937185cba295ecffe0f4d78a65fab6ea686c
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Tue Mar 24 18:03:34 2020 -0300
16263: Adds test exposing a bug on filter unmarshalling.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/sdk/go/arvados/resource_list_test.go b/sdk/go/arvados/resource_list_test.go
index 4e09c5375..b36e82c91 100644
--- a/sdk/go/arvados/resource_list_test.go
+++ b/sdk/go/arvados/resource_list_test.go
@@ -34,3 +34,40 @@ func TestMarshalFiltersWithNil(t *testing.T) {
t.Errorf("Encoded as %q, expected %q", buf, expect)
}
}
+
+func TestUnmarshalFiltersWithNil(t *testing.T) {
+ buf := []byte(`["modified_at","=",null]`)
+ f := &Filter{}
+ err := f.UnmarshalJSON(buf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ expect := Filter{Attr: "modified_at", Operator: "=", Operand: nil}
+ if f.Attr != expect.Attr || f.Operator != expect.Operator || f.Operand != expect.Operand {
+ t.Errorf("Decoded as %q, expected %q", f, expect)
+ }
+}
+
+func TestMarshalFiltersWithBoolean(t *testing.T) {
+ buf, err := json.Marshal([]Filter{
+ {Attr: "is_active", Operator: "=", Operand: true}})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if expect := []byte(`[["is_active","=",true]]`); 0 != bytes.Compare(buf, expect) {
+ t.Errorf("Encoded as %q, expected %q", buf, expect)
+ }
+}
+
+func TestUnmarshalFiltersWithBoolean(t *testing.T) {
+ buf := []byte(`["is_active","=",true]`)
+ f := &Filter{}
+ err := f.UnmarshalJSON(buf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ expect := Filter{Attr: "is_active", Operator: "=", Operand: true}
+ if f.Attr != expect.Attr || f.Operator != expect.Operator || f.Operand != expect.Operand {
+ t.Errorf("Decoded as %q, expected %q", f, expect)
+ }
+}
commit d97308ee0db8ad22f8eb03de75ce55cf7e228afd
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Mon Mar 23 16:20:41 2020 -0300
16263: Makes gofmt happy.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 42a1a5727..226bfabf1 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -365,15 +365,15 @@ func (conn *Conn) SpecimenDelete(ctx context.Context, options arvados.DeleteOpti
}
var userAttrsCachedFromLoginCluster = map[string]bool{
- "created_at": true,
- "email": true,
- "first_name": true,
- "is_active": true,
- "is_admin": true,
- "last_name": true,
- "modified_at": true,
- "prefs": true,
- "username": true,
+ "created_at": true,
+ "email": true,
+ "first_name": true,
+ "is_active": true,
+ "is_admin": true,
+ "last_name": true,
+ "modified_at": true,
+ "prefs": true,
+ "username": true,
"etag": false,
"full_name": false,
commit 01baf16387609f936e45940561c0c289046a39d0
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Mon Mar 23 15:32:59 2020 -0300
16263: Don't cache modified_by_*_uuid fields when using LoginCluster.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 56f117ee7..42a1a5727 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -372,18 +372,18 @@ var userAttrsCachedFromLoginCluster = map[string]bool{
"is_admin": true,
"last_name": true,
"modified_at": true,
- "modified_by_client_uuid": true,
- "modified_by_user_uuid": true,
"prefs": true,
"username": true,
- "etag": false,
- "full_name": false,
- "identity_url": false,
- "is_invited": false,
- "owner_uuid": false,
- "uuid": false,
- "writable_by": false,
+ "etag": false,
+ "full_name": false,
+ "identity_url": false,
+ "is_invited": false,
+ "modified_by_client_uuid": false,
+ "modified_by_user_uuid": false,
+ "owner_uuid": false,
+ "uuid": false,
+ "writable_by": false,
}
func (conn *Conn) UserList(ctx context.Context, options arvados.ListOptions) (arvados.UserList, error) {
commit 29486979fc8a48788f5c12fec399d8d41588cba0
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Mon Mar 23 12:27:18 2020 -0300
16263: Adds nullify behavior to users's batch_update endpoint.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index fbf177b01..7b82cdb61 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -486,12 +486,20 @@ class ApplicationController < ActionController::Base
# Go code may send empty values (ie: empty string instead of NULL) that
# should be translated to NULL on the database.
def set_nullable_attrs_to_null
- (resource_attrs.keys & nullable_attributes).each do |attr|
- val = resource_attrs[attr]
+ nullify_attrs(resource_attrs.to_hash).each do |k, v|
+ resource_attrs[k] = v
+ end
+ end
+
+ def nullify_attrs(a = {})
+ new_attrs = a.to_hash.symbolize_keys
+ (new_attrs.keys & nullable_attributes).each do |attr|
+ val = new_attrs[attr]
if (val.class == Integer && val == 0) || (val.class == String && val == "")
- resource_attrs[attr] = nil
+ new_attrs[attr] = nil
end
end
+ return new_attrs
end
def reload_object_before_update
diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index fecc0620c..eb2402b80 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -22,7 +22,7 @@ class Arvados::V1::UsersController < ApplicationController
rescue ActiveRecord::RecordNotUnique
retry
end
- u.update_attributes!(attrs)
+ u.update_attributes!(nullify_attrs(attrs))
@objects << u
end
@offset = 0
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index b38f0d52f..817a1c9ef 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -1057,6 +1057,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
newuuid => {
'first_name' => 'noot',
'email' => 'root at remot.example.com',
+ 'username' => '',
},
}})
assert_response(:success)
commit 4fbb5b4751488d9751710f03ebf99406d4c55410
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Thu Mar 19 18:15:52 2020 -0300
16263: Assigns nil to select attributes when receiving empty values.
Controller may translate NULL values to "" on certain object string fields.
The same with integers, NULLs are converted to 0.
When controller retrieves objects from railsAPI and uses the data to create
or update objects, some of those fields should get converted back to NULL.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 369043e78..fbf177b01 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -45,6 +45,7 @@ class ApplicationController < ActionController::Base
before_action :load_required_parameters
before_action(:find_object_by_uuid,
except: [:index, :create] + ERROR_ACTIONS)
+ before_action(:set_nullable_attrs_to_null, only: [:update, :create])
before_action :load_limit_offset_order_params, only: [:index, :contents]
before_action :load_where_param, only: [:index, :contents]
before_action :load_filters_param, only: [:index, :contents]
@@ -478,6 +479,21 @@ class ApplicationController < ActionController::Base
@object = @objects.first
end
+ def nullable_attributes
+ []
+ end
+
+ # Go code may send empty values (ie: empty string instead of NULL) that
+ # should be translated to NULL on the database.
+ def set_nullable_attrs_to_null
+ (resource_attrs.keys & nullable_attributes).each do |attr|
+ val = resource_attrs[attr]
+ if (val.class == Integer && val == 0) || (val.class == String && val == "")
+ resource_attrs[attr] = nil
+ end
+ end
+ end
+
def reload_object_before_update
# This is necessary to prevent an ActiveRecord::ReadOnlyRecord
# error when updating an object which was retrieved using a join.
diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 1cf3b9d78..fecc0620c 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -268,4 +268,8 @@ class Arvados::V1::UsersController < ApplicationController
end
super
end
+
+ def nullable_attributes
+ super + [:email, :first_name, :last_name, :username]
+ end
end
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index 753e707b6..b38f0d52f 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -88,6 +88,38 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
assert_nil created['identity_url'], 'expected no identity_url'
end
+ test "create new user with empty username" do
+ authorize_with :admin
+ post :create, params: {
+ user: {
+ first_name: "test_first_name",
+ last_name: "test_last_name",
+ username: ""
+ }
+ }
+ assert_response :success
+ created = JSON.parse(@response.body)
+ assert_equal 'test_first_name', created['first_name']
+ assert_not_nil created['uuid'], 'expected uuid for the newly created user'
+ assert_nil created['email'], 'expected no email'
+ assert_nil created['username'], 'expected no username'
+ end
+
+ test "update user with empty username" do
+ authorize_with :admin
+ user = users('spectator')
+ assert_not_nil user['username']
+ put :update, params: {
+ id: users('spectator')['uuid'],
+ user: {
+ username: ""
+ }
+ }
+ assert_response :success
+ updated = JSON.parse(@response.body)
+ assert_nil updated['username'], 'expected no username'
+ end
+
test "create user with user, vm and repo as input" do
authorize_with :admin
repo_name = 'usertestrepo'
commit da84332fc7b4a549e08e0be87d4b52ffe2eeddd9
Author: Ward Vandewege <ward at jhvc.com>
Date: Fri Apr 10 08:56:56 2020 -0400
Fix tests.
refs #16326
Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at jhvc.com>
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 6813bee40..7b7c3c20d 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -190,12 +190,21 @@ Clusters:
MaxItemsPerResponse: 1000
# Maximum number of concurrent requests to accept in a single
- # service process, or 0 for no limit. Currently supported only
- # by keepstore.
+ # service process, or 0 for no limit.
MaxConcurrentRequests: 0
- # Maximum number of 64MiB memory buffers per keepstore server
- # process, or 0 for no limit.
+ # Maximum number of 64MiB memory buffers per Keepstore server process, or
+ # 0 for no limit. When this limit is reached, up to
+ # (MaxConcurrentRequests - MaxKeepBlobBuffers) HTTP requests requiring
+ # buffers (like GET and PUT) will wait for buffer space to be released.
+ # Any HTTP requests beyond MaxConcurrentRequests will receive an
+ # immediate 503 response.
+ #
+ # MaxKeepBlobBuffers should be set such that (MaxKeepBlobBuffers * 64MiB
+ # * 1.1) fits comfortably in memory. On a host dedicated to running
+ # Keepstore, divide total memory by 88MiB to suggest a suitable value.
+ # For example, if grep MemTotal /proc/meminfo reports MemTotal: 7125440
+ # kB, compute 7125440 / (88 * 1024)=79 and configure MaxBuffers: 79
MaxKeepBlobBuffers: 128
# API methods to disable. Disabled methods are not listed in the
commit 845381a133ebecfaff0335e432fda2fc0ac9f280
Author: Ward Vandewege <ward at jhvc.com>
Date: Thu Apr 9 17:53:31 2020 -0400
documentation: update descriptions for MaxKeepBlobBuffers and MaxConcurrentRequests
keepstore: MaxConcurrentRequests set to zero should mean no limit
refs #16326
Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at jhvc.com>
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 3750adcab..a0def71f7 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -184,12 +184,21 @@ Clusters:
MaxItemsPerResponse: 1000
# Maximum number of concurrent requests to accept in a single
- # service process, or 0 for no limit. Currently supported only
- # by keepstore.
+ # service process, or 0 for no limit.
MaxConcurrentRequests: 0
- # Maximum number of 64MiB memory buffers per keepstore server
- # process, or 0 for no limit.
+ # Maximum number of 64MiB memory buffers per Keepstore server process, or
+ # 0 for no limit. When this limit is reached, up to
+ # (MaxConcurrentRequests - MaxKeepBlobBuffers) HTTP requests requiring
+ # buffers (like GET and PUT) will wait for buffer space to be released.
+ # Any HTTP requests beyond MaxConcurrentRequests will receive an
+ # immediate 503 response.
+ #
+ # MaxKeepBlobBuffers should be set such that (MaxKeepBlobBuffers * 64MiB
+ # * 1.1) fits comfortably in memory. On a host dedicated to running
+ # Keepstore, divide total memory by 88MiB to suggest a suitable value.
+ # For example, if grep MemTotal /proc/meminfo reports MemTotal: 7125440
+ # kB, compute 7125440 / (88 * 1024)=79 and configure MaxBuffers: 79
MaxKeepBlobBuffers: 128
# API methods to disable. Disabled methods are not listed in the
diff --git a/services/keepstore/command.go b/services/keepstore/command.go
index 0593460a2..c17ee35a3 100644
--- a/services/keepstore/command.go
+++ b/services/keepstore/command.go
@@ -153,10 +153,6 @@ func (h *handler) setup(ctx context.Context, cluster *arvados.Cluster, token str
}
bufs = newBufferPool(h.Logger, h.Cluster.API.MaxKeepBlobBuffers, BlockSize)
- if h.Cluster.API.MaxConcurrentRequests < 1 {
- h.Cluster.API.MaxConcurrentRequests = h.Cluster.API.MaxKeepBlobBuffers * 2
- h.Logger.Warnf("API.MaxConcurrentRequests <1 or not specified; defaulting to MaxKeepBlobBuffers * 2 == %d", h.Cluster.API.MaxConcurrentRequests)
- }
if h.Cluster.API.MaxConcurrentRequests > 0 && h.Cluster.API.MaxConcurrentRequests < h.Cluster.API.MaxKeepBlobBuffers {
h.Logger.Warnf("Possible configuration mistake: not useful to set API.MaxKeepBlobBuffers (%d) higher than API.MaxConcurrentRequests (%d)", h.Cluster.API.MaxKeepBlobBuffers, h.Cluster.API.MaxConcurrentRequests)
}
commit 8a125907af9a75bbf905de4b715309e562092704
Author: Tom Clegg <tom at tomclegg.ca>
Date: Thu Apr 9 16:58:36 2020 -0400
Warn if MaxKeepBlobBuffers > MaxConcurrentRequests.
No issue #
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/services/keepstore/command.go b/services/keepstore/command.go
index e0509393c..0593460a2 100644
--- a/services/keepstore/command.go
+++ b/services/keepstore/command.go
@@ -157,6 +157,9 @@ func (h *handler) setup(ctx context.Context, cluster *arvados.Cluster, token str
h.Cluster.API.MaxConcurrentRequests = h.Cluster.API.MaxKeepBlobBuffers * 2
h.Logger.Warnf("API.MaxConcurrentRequests <1 or not specified; defaulting to MaxKeepBlobBuffers * 2 == %d", h.Cluster.API.MaxConcurrentRequests)
}
+ if h.Cluster.API.MaxConcurrentRequests > 0 && h.Cluster.API.MaxConcurrentRequests < h.Cluster.API.MaxKeepBlobBuffers {
+ h.Logger.Warnf("Possible configuration mistake: not useful to set API.MaxKeepBlobBuffers (%d) higher than API.MaxConcurrentRequests (%d)", h.Cluster.API.MaxKeepBlobBuffers, h.Cluster.API.MaxConcurrentRequests)
+ }
if h.Cluster.Collections.BlobSigningKey != "" {
} else if h.Cluster.Collections.BlobSigning {
commit 3eeae9396640383abeecfb3ea5289b75a144d969
Author: Tom Clegg <tom at tomclegg.ca>
Date: Thu Mar 5 16:42:39 2020 -0500
16219: Test populated container fields.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/dispatchcloud/container/queue_test.go b/lib/dispatchcloud/container/queue_test.go
index 31f321488..0075ee324 100644
--- a/lib/dispatchcloud/container/queue_test.go
+++ b/lib/dispatchcloud/container/queue_test.go
@@ -106,6 +106,13 @@ func (suite *IntegrationSuite) TestGetLockUnlockCancel(c *check.C) {
func (suite *IntegrationSuite) TestCancelIfNoInstanceType(c *check.C) {
errorTypeChooser := func(ctr *arvados.Container) (arvados.InstanceType, error) {
+ // Make sure the relevant container fields are
+ // actually populated.
+ c.Check(ctr.ContainerImage, check.Equals, "test")
+ c.Check(ctr.RuntimeConstraints.VCPUs, check.Equals, 4)
+ c.Check(ctr.RuntimeConstraints.RAM, check.Equals, int64(12000000000))
+ c.Check(ctr.Mounts["/tmp"].Capacity, check.Equals, int64(24000000000))
+ c.Check(ctr.Mounts["/var/spool/cwl"].Capacity, check.Equals, int64(24000000000))
return arvados.InstanceType{}, errors.New("no suitable instance type")
}
commit 7781042d8602072fe95b607ad9c64713a46f1637
Author: Tom Clegg <tom at tomclegg.ca>
Date: Thu Mar 5 14:44:22 2020 -0500
16219: Load all fields needed to compute node size.
Without ContainerImage and Mounts, the scratch size requirement can't
be computed correctly.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/dispatchcloud/container/queue.go b/lib/dispatchcloud/container/queue.go
index a4a270dd1..d128c265f 100644
--- a/lib/dispatchcloud/container/queue.go
+++ b/lib/dispatchcloud/container/queue.go
@@ -26,8 +26,9 @@ type APIClient interface {
// A QueueEnt is an entry in the queue, consisting of a container
// record and the instance type that should be used to run it.
type QueueEnt struct {
- // The container to run. Only the UUID, State, Priority, and
- // RuntimeConstraints fields are populated.
+ // The container to run. Only the UUID, State, Priority,
+ // RuntimeConstraints, Mounts, and ContainerImage fields are
+ // populated.
Container arvados.Container `json:"container"`
InstanceType arvados.InstanceType `json:"instance_type"`
}
@@ -381,7 +382,7 @@ func (cq *Queue) poll() (map[string]*arvados.Container, error) {
*next[upd.UUID] = upd
}
}
- selectParam := []string{"uuid", "state", "priority", "runtime_constraints"}
+ selectParam := []string{"uuid", "state", "priority", "runtime_constraints", "container_image", "mounts"}
limitParam := 1000
mine, err := cq.fetchAll(arvados.ResourceListParams{
commit 6278100f84d7cc021932ce230ae7e713ed129b01
Author: Tom Clegg <tom at tomclegg.ca>
Date: Thu Mar 12 00:15:16 2020 -0400
16221: Fix test for config endpoint.
x['configs'] raises KeyError on an old discovery doc -- test
x.get('configs', False) instead.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/sdk/python/arvados/util.py b/sdk/python/arvados/util.py
index 9e0a31783..dcc0417c1 100644
--- a/sdk/python/arvados/util.py
+++ b/sdk/python/arvados/util.py
@@ -421,7 +421,7 @@ def new_request_id():
return rid
def get_config_once(svc):
- if not svc._rootDesc.get('resources')['configs']:
+ if not svc._rootDesc.get('resources').get('configs', False):
# Old API server version, no config export endpoint
return {}
if not hasattr(svc, '_cached_config'):
commit 891f67ea72aa35895c57421531b2b5cbbd25e70d
Author: Tom Clegg <tom at tomclegg.ca>
Date: Wed Mar 11 16:52:04 2020 -0400
Fix instance IDs in arvados-dispatch-cloud log messages.
Some logs had {"Instance": {}} instead of the provider's instance ID.
No issue #
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/dispatchcloud/worker/pool.go b/lib/dispatchcloud/worker/pool.go
index c52422c41..14b14ea5d 100644
--- a/lib/dispatchcloud/worker/pool.go
+++ b/lib/dispatchcloud/worker/pool.go
@@ -435,7 +435,7 @@ func (wp *Pool) Shutdown(it arvados.InstanceType) bool {
// time (Idle) or the earliest create time (Booting)
for _, wkr := range wp.workers {
if wkr.idleBehavior != IdleBehaviorHold && wkr.state == tryState && wkr.instType == it {
- logger.WithField("Instance", wkr.instance).Info("shutting down")
+ logger.WithField("Instance", wkr.instance.ID()).Info("shutting down")
wkr.shutdown()
return true
}
@@ -835,13 +835,13 @@ func (wp *Pool) sync(threshold time.Time, instances []cloud.Instance) {
itTag := inst.Tags()[wp.tagKeyPrefix+tagKeyInstanceType]
it, ok := wp.instanceTypes[itTag]
if !ok {
- wp.logger.WithField("Instance", inst).Errorf("unknown InstanceType tag %q --- ignoring", itTag)
+ wp.logger.WithField("Instance", inst.ID()).Errorf("unknown InstanceType tag %q --- ignoring", itTag)
continue
}
if wkr, isNew := wp.updateWorker(inst, it); isNew {
notify = true
} else if wkr.state == StateShutdown && time.Since(wkr.destroyed) > wp.timeoutShutdown {
- wp.logger.WithField("Instance", inst).Info("worker still listed after shutdown; retrying")
+ wp.logger.WithField("Instance", inst.ID()).Info("worker still listed after shutdown; retrying")
wkr.shutdown()
}
}
commit eec034bf27818af1c3bd998e51c85772e7cdc9dd
Author: Tom Clegg <tom at tomclegg.ca>
Date: Wed Apr 1 16:44:02 2020 -0400
16270: Fill in missing scratch fields on InstanceType entries.
Previously they were being filled in correctly when written as an
array in the config file, but not when written as a map.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index a70980cbd..79e47ba5d 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -421,6 +421,24 @@ var errDuplicateInstanceTypeName = errors.New("duplicate instance type name")
// UnmarshalJSON handles old config files that provide an array of
// instance types instead of a hash.
func (it *InstanceTypeMap) UnmarshalJSON(data []byte) error {
+ fixup := func(t InstanceType) (InstanceType, error) {
+ if t.ProviderType == "" {
+ t.ProviderType = t.Name
+ }
+ if t.Scratch == 0 {
+ t.Scratch = t.IncludedScratch + t.AddedScratch
+ } else if t.AddedScratch == 0 {
+ t.AddedScratch = t.Scratch - t.IncludedScratch
+ } else if t.IncludedScratch == 0 {
+ t.IncludedScratch = t.Scratch - t.AddedScratch
+ }
+
+ if t.Scratch != (t.IncludedScratch + t.AddedScratch) {
+ return t, fmt.Errorf("InstanceType %q: Scratch != (IncludedScratch + AddedScratch)", t.Name)
+ }
+ return t, nil
+ }
+
if len(data) > 0 && data[0] == '[' {
var arr []InstanceType
err := json.Unmarshal(data, &arr)
@@ -436,19 +454,9 @@ func (it *InstanceTypeMap) UnmarshalJSON(data []byte) error {
if _, ok := (*it)[t.Name]; ok {
return errDuplicateInstanceTypeName
}
- if t.ProviderType == "" {
- t.ProviderType = t.Name
- }
- if t.Scratch == 0 {
- t.Scratch = t.IncludedScratch + t.AddedScratch
- } else if t.AddedScratch == 0 {
- t.AddedScratch = t.Scratch - t.IncludedScratch
- } else if t.IncludedScratch == 0 {
- t.IncludedScratch = t.Scratch - t.AddedScratch
- }
-
- if t.Scratch != (t.IncludedScratch + t.AddedScratch) {
- return fmt.Errorf("%v: Scratch != (IncludedScratch + AddedScratch)", t.Name)
+ t, err := fixup(t)
+ if err != nil {
+ return err
}
(*it)[t.Name] = t
}
@@ -464,8 +472,9 @@ func (it *InstanceTypeMap) UnmarshalJSON(data []byte) error {
*it = InstanceTypeMap(hash)
for name, t := range *it {
t.Name = name
- if t.ProviderType == "" {
- t.ProviderType = name
+ t, err := fixup(t)
+ if err != nil {
+ return err
}
(*it)[name] = t
}
diff --git a/sdk/go/arvados/config_test.go b/sdk/go/arvados/config_test.go
index b984cb566..e4d26e03f 100644
--- a/sdk/go/arvados/config_test.go
+++ b/sdk/go/arvados/config_test.go
@@ -45,3 +45,29 @@ func (s *ConfigSuite) TestInstanceTypeSize(c *check.C) {
c.Check(int64(it.Scratch), check.Equals, int64(4000000000))
c.Check(int64(it.RAM), check.Equals, int64(4294967296))
}
+
+func (s *ConfigSuite) TestInstanceTypeFixup(c *check.C) {
+ for _, confdata := range []string{
+ // Current format: map of entries
+ `{foo4: {IncludedScratch: 4GB}, foo8: {ProviderType: foo_8, Scratch: 8GB}}`,
+ // Legacy format: array of entries with key in "Name" field
+ `[{Name: foo4, IncludedScratch: 4GB}, {Name: foo8, ProviderType: foo_8, Scratch: 8GB}]`,
+ } {
+ c.Log(confdata)
+ var itm InstanceTypeMap
+ err := yaml.Unmarshal([]byte(confdata), &itm)
+ c.Check(err, check.IsNil)
+
+ c.Check(itm["foo4"].Name, check.Equals, "foo4")
+ c.Check(itm["foo4"].ProviderType, check.Equals, "foo4")
+ c.Check(itm["foo4"].Scratch, check.Equals, ByteSize(4000000000))
+ c.Check(itm["foo4"].AddedScratch, check.Equals, ByteSize(0))
+ c.Check(itm["foo4"].IncludedScratch, check.Equals, ByteSize(4000000000))
+
+ c.Check(itm["foo8"].Name, check.Equals, "foo8")
+ c.Check(itm["foo8"].ProviderType, check.Equals, "foo_8")
+ c.Check(itm["foo8"].Scratch, check.Equals, ByteSize(8000000000))
+ c.Check(itm["foo8"].AddedScratch, check.Equals, ByteSize(8000000000))
+ c.Check(itm["foo8"].IncludedScratch, check.Equals, ByteSize(0))
+ }
+}
commit 89705b6140489b7badc42b6a67367dafaeca78d0
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date: Tue Mar 3 21:27:27 2020 +0000
Bump rake from 12.3.2 to 13.0.1 in /services/api
Bumps [rake](https://github.com/ruby/rake) from 12.3.2 to 13.0.1.
- [Release notes](https://github.com/ruby/rake/releases)
- [Changelog](https://github.com/ruby/rake/blob/master/History.rdoc)
- [Commits](https://github.com/ruby/rake/compare/v12.3.2...v13.0.1)
Signed-off-by: dependabot[bot] <support at github.com>
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index 53cdc906e..24d5ad5b6 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -213,7 +213,7 @@ GEM
method_source
rake (>= 0.8.7)
thor (>= 0.18.1, < 2.0)
- rake (12.3.2)
+ rake (13.0.1)
rb-fsevent (0.10.3)
rb-inotify (0.9.10)
ffi (>= 0.5.0, < 2)
commit b7e6ce193a9158b9e4700fe9334018702dedb3d8
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date: Tue Mar 3 21:27:30 2020 +0000
Bump rake from 12.3.2 to 13.0.1 in /apps/workbench
Bumps [rake](https://github.com/ruby/rake) from 12.3.2 to 13.0.1.
- [Release notes](https://github.com/ruby/rake/releases)
- [Changelog](https://github.com/ruby/rake/blob/master/History.rdoc)
- [Commits](https://github.com/ruby/rake/compare/v12.3.2...v13.0.1)
Signed-off-by: dependabot[bot] <support at github.com>
diff --git a/apps/workbench/Gemfile.lock b/apps/workbench/Gemfile.lock
index 6f36def0c..2af9c8b16 100644
--- a/apps/workbench/Gemfile.lock
+++ b/apps/workbench/Gemfile.lock
@@ -247,7 +247,7 @@ GEM
method_source
rake (>= 0.8.7)
thor (>= 0.18.1, < 2.0)
- rake (12.3.2)
+ rake (13.0.1)
raphael-rails (2.1.2)
rb-fsevent (0.10.3)
rb-inotify (0.10.0)
commit fca7b8b5b8d11938f2f1c2684a9a34814968f86a
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date: Tue Feb 25 08:04:58 2020 +0000
Bump nokogiri from 1.10.4 to 1.10.8 in /apps/workbench
Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.10.4 to 1.10.8.
- [Release notes](https://github.com/sparklemotion/nokogiri/releases)
- [Changelog](https://github.com/sparklemotion/nokogiri/blob/master/CHANGELOG.md)
- [Commits](https://github.com/sparklemotion/nokogiri/compare/v1.10.4...v1.10.8)
Signed-off-by: dependabot[bot] <support at github.com>
diff --git a/apps/workbench/Gemfile.lock b/apps/workbench/Gemfile.lock
index 829bc8584..6f36def0c 100644
--- a/apps/workbench/Gemfile.lock
+++ b/apps/workbench/Gemfile.lock
@@ -195,7 +195,7 @@ GEM
net-ssh-gateway (2.0.0)
net-ssh (>= 4.0.0)
nio4r (2.3.1)
- nokogiri (1.10.5)
+ nokogiri (1.10.8)
mini_portile2 (~> 2.4.0)
npm-rails (0.2.1)
rails (>= 3.2)
commit eeb4aa1bcf4462466170a4d3053d6c37f3497d7c
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date: Fri Nov 8 21:48:47 2019 +0000
Bump loofah from 2.2.3 to 2.3.1 in /apps/workbench
Bumps [loofah](https://github.com/flavorjones/loofah) from 2.2.3 to 2.3.1.
- [Release notes](https://github.com/flavorjones/loofah/releases)
- [Changelog](https://github.com/flavorjones/loofah/blob/master/CHANGELOG.md)
- [Commits](https://github.com/flavorjones/loofah/compare/v2.2.3...v2.3.1)
Signed-off-by: dependabot[bot] <support at github.com>
diff --git a/apps/workbench/Gemfile.lock b/apps/workbench/Gemfile.lock
index b02161c59..829bc8584 100644
--- a/apps/workbench/Gemfile.lock
+++ b/apps/workbench/Gemfile.lock
@@ -122,7 +122,7 @@ GEM
coffee-script-source (1.12.2)
commonjs (0.2.7)
concurrent-ruby (1.1.5)
- crass (1.0.4)
+ crass (1.0.5)
deep_merge (1.2.1)
docile (1.3.1)
erubis (2.7.0)
@@ -167,7 +167,7 @@ GEM
railties (>= 4)
request_store (~> 1.0)
logstash-event (1.2.02)
- loofah (2.2.3)
+ loofah (2.3.1)
crass (~> 1.0.2)
nokogiri (>= 1.5.9)
mail (2.7.1)
@@ -195,7 +195,7 @@ GEM
net-ssh-gateway (2.0.0)
net-ssh (>= 4.0.0)
nio4r (2.3.1)
- nokogiri (1.10.4)
+ nokogiri (1.10.5)
mini_portile2 (~> 2.4.0)
npm-rails (0.2.1)
rails (>= 3.2)
commit 445465e4e859de272bee53c2eed3cd8f54e33d34
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date: Tue Feb 25 08:04:57 2020 +0000
Bump nokogiri from 1.10.2 to 1.10.8 in /services/api
Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.10.2 to 1.10.8.
- [Release notes](https://github.com/sparklemotion/nokogiri/releases)
- [Changelog](https://github.com/sparklemotion/nokogiri/blob/master/CHANGELOG.md)
- [Commits](https://github.com/sparklemotion/nokogiri/compare/v1.10.2...v1.10.8)
Signed-off-by: dependabot[bot] <support at github.com>
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index 2c780d477..53cdc906e 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -157,7 +157,7 @@ GEM
net-ssh-gateway (2.0.0)
net-ssh (>= 4.0.0)
nio4r (2.3.1)
- nokogiri (1.10.2)
+ nokogiri (1.10.8)
mini_portile2 (~> 2.4.0)
oauth2 (1.4.1)
faraday (>= 0.8, < 0.16.0)
commit 7d1defd3d60fe165522a736855346c9d34eae28d
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Mon Mar 23 18:07:56 2020 -0300
16266: Applies monkeypatch to fix CVE-2020-5267 on workbench1.
As adviced on https://github.com/advisories/GHSA-65cv-r6x7-79hv
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/apps/workbench/config/initializers/actionview_xss_fix.rb b/apps/workbench/config/initializers/actionview_xss_fix.rb
new file mode 100644
index 000000000..3f5e239ef
--- /dev/null
+++ b/apps/workbench/config/initializers/actionview_xss_fix.rb
@@ -0,0 +1,32 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+# This is related to:
+# * https://github.com/advisories/GHSA-65cv-r6x7-79hv
+# * https://nvd.nist.gov/vuln/detail/CVE-2020-5267
+#
+# Until we upgrade to rails 5.2, this monkeypatch should be enough
+ActionView::Helpers::JavaScriptHelper::JS_ESCAPE_MAP.merge!(
+ {
+ "`" => "\\`",
+ "$" => "\\$"
+ }
+)
+
+module ActionView::Helpers::JavaScriptHelper
+ alias :old_ej :escape_javascript
+ alias :old_j :j
+
+ def escape_javascript(javascript)
+ javascript = javascript.to_s
+ if javascript.empty?
+ result = ""
+ else
+ result = javascript.gsub(/(\\|<\/|\r\n|\342\200\250|\342\200\251|[\n\r"']|[`]|[$])/u, JS_ESCAPE_MAP)
+ end
+ javascript.html_safe? ? result.html_safe : result
+ end
+
+ alias :j :escape_javascript
+end
\ No newline at end of file
commit 883f51266e8ac9275b7842704c56ff195f853080
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Thu Mar 26 13:29:40 2020 -0300
16266: Adds tests exposing potential XSS vulnerability on escape_javascript()
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/apps/workbench/test/unit/helpers/javascript_helper_test.rb b/apps/workbench/test/unit/helpers/javascript_helper_test.rb
new file mode 100644
index 000000000..9d5a55345
--- /dev/null
+++ b/apps/workbench/test/unit/helpers/javascript_helper_test.rb
@@ -0,0 +1,17 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require 'test_helper'
+
+# Tests XSS vulnerability monkeypatch
+# See: https://github.com/advisories/GHSA-65cv-r6x7-79hv
+class JavascriptHelperTest < ActionView::TestCase
+ def test_escape_backtick
+ assert_equal "\\`", escape_javascript("`")
+ end
+
+ def test_escape_dollar_sign
+ assert_equal "\\$", escape_javascript("$")
+ end
+end
commit 2713cc305c5065959da84b61e3a1b8867c72f4e8
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Mar 24 15:35:38 2020 -0400
16138: Remove additional/code references to copying pipelines
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/sdk/python/arvados/commands/arv_copy.py b/sdk/python/arvados/commands/arv_copy.py
index d39ed3387..5f12b62ee 100755
--- a/sdk/python/arvados/commands/arv_copy.py
+++ b/sdk/python/arvados/commands/arv_copy.py
@@ -98,18 +98,9 @@ def main():
copy_opts.add_argument(
'--no-recursive', dest='recursive', action='store_false',
help='Do not copy any dependencies. NOTE: if this option is given, the copied object will need to be updated manually in order to be functional.')
- copy_opts.add_argument(
- '--dst-git-repo', dest='dst_git_repo',
- help='The name of the destination git repository. Required when copying a pipeline recursively.')
copy_opts.add_argument(
'--project-uuid', dest='project_uuid',
- help='The UUID of the project at the destination to which the pipeline should be copied.')
- copy_opts.add_argument(
- '--allow-git-http-src', action="store_true",
- help='Allow cloning git repositories over insecure http')
- copy_opts.add_argument(
- '--allow-git-http-dst', action="store_true",
- help='Allow pushing git repositories over insecure http')
+ help='The UUID of the project at the destination to which the collection or workflow should be copied.')
copy_opts.add_argument(
'object_uuid',
@@ -118,7 +109,7 @@ def main():
copy_opts.set_defaults(recursive=True)
parser = argparse.ArgumentParser(
- description='Copy a pipeline instance, template, workflow, or collection from one Arvados instance to another.',
+ description='Copy a workflow or collection from one Arvados instance to another.',
parents=[copy_opts, arv_cmd.retry_opt])
args = parser.parse_args()
@@ -468,7 +459,7 @@ def copy_collection(obj_uuid, src, dst, args):
c = items[0]
if not c:
# See if there is a collection that's in the same project
- # as the root item (usually a pipeline) being copied.
+ # as the root item (usually a workflow) being copied.
for i in items:
if i.get("owner_uuid") == src_owner_uuid and i.get("name"):
c = i
@@ -618,55 +609,6 @@ def select_git_url(api, repo_name, retries, allow_insecure_http, allow_insecure_
return (git_url, git_config)
-# copy_git_repo(src_git_repo, src, dst, dst_git_repo, script_version, args)
-#
-# Copies commits from git repository 'src_git_repo' on Arvados
-# instance 'src' to 'dst_git_repo' on 'dst'. Both src_git_repo
-# and dst_git_repo are repository names, not UUIDs (i.e. "arvados"
-# or "jsmith")
-#
-# All commits will be copied to a destination branch named for the
-# source repository URL.
-#
-# The destination repository must already exist.
-#
-# The user running this command must be authenticated
-# to both repositories.
-#
-def copy_git_repo(src_git_repo, src, dst, dst_git_repo, script_version, args):
- # Identify the fetch and push URLs for the git repositories.
-
- (src_git_url, src_git_config) = select_git_url(src, src_git_repo, args.retries, args.allow_git_http_src, "--allow-git-http-src")
- (dst_git_url, dst_git_config) = select_git_url(dst, dst_git_repo, args.retries, args.allow_git_http_dst, "--allow-git-http-dst")
-
- logger.debug('src_git_url: {}'.format(src_git_url))
- logger.debug('dst_git_url: {}'.format(dst_git_url))
-
- dst_branch = re.sub(r'\W+', '_', "{}_{}".format(src_git_url, script_version))
-
- # Copy git commits from src repo to dst repo.
- if src_git_repo not in local_repo_dir:
- local_repo_dir[src_git_repo] = tempfile.mkdtemp()
- arvados.util.run_command(
- ["git"] + src_git_config + ["clone", "--bare", src_git_url,
- local_repo_dir[src_git_repo]],
- cwd=os.path.dirname(local_repo_dir[src_git_repo]),
- env={"HOME": os.environ["HOME"],
- "ARVADOS_API_TOKEN": src.api_token,
- "GIT_ASKPASS": "/bin/false"})
- arvados.util.run_command(
- ["git", "remote", "add", "dst", dst_git_url],
- cwd=local_repo_dir[src_git_repo])
- arvados.util.run_command(
- ["git", "branch", dst_branch, script_version],
- cwd=local_repo_dir[src_git_repo])
- arvados.util.run_command(["git"] + dst_git_config + ["push", "dst", dst_branch],
- cwd=local_repo_dir[src_git_repo],
- env={"HOME": os.environ["HOME"],
- "ARVADOS_API_TOKEN": dst.api_token,
- "GIT_ASKPASS": "/bin/false"})
-
-
def copy_docker_image(docker_image, docker_image_tag, src, dst, args):
"""Copy the docker image identified by docker_image and
docker_image_tag from src to dst. Create appropriate
@@ -707,7 +649,7 @@ def git_rev_parse(rev, repo):
# the second field of the uuid. This function consults the api's
# schema to identify the object class.
#
-# It returns a string such as 'Collection', 'PipelineInstance', etc.
+# It returns a string such as 'Collection', 'Workflow', etc.
#
# Special case: if handed a Keep locator hash, return 'Collection'.
#
commit a9866cc99faa6de3df2884b8ad97e858b922036c
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Mar 23 11:00:07 2020 -0400
16138: Remove additional pipelines related code
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/sdk/python/arvados/commands/arv_copy.py b/sdk/python/arvados/commands/arv_copy.py
index d94d19efc..d39ed3387 100755
--- a/sdk/python/arvados/commands/arv_copy.py
+++ b/sdk/python/arvados/commands/arv_copy.py
@@ -8,8 +8,8 @@
#
# By default, arv-copy recursively copies any dependent objects
# necessary to make the object functional in the new instance
-# (e.g. for a pipeline instance, arv-copy copies the pipeline
-# template, input collection, docker images, git repositories). If
+# (e.g. for a workflow, arv-copy copies the workflow,
+# input collections, and docker images). If
# --no-recursive is given, arv-copy copies only the single record
# identified by object-uuid.
#
@@ -86,9 +86,6 @@ def main():
copy_opts.add_argument(
'-f', '--force', dest='force', action='store_true',
help='Perform copy even if the object appears to exist at the remote destination.')
- copy_opts.add_argument(
- '--force-filters', action='store_true', default=False,
- help="Copy pipeline template filters verbatim, even if they act differently on the destination cluster.")
copy_opts.add_argument(
'--src', dest='source_arvados', required=True,
help='The name of the source Arvados instance (required) - points at an Arvados config file. May be either a pathname to a config file, or (for example) "foo" as shorthand for $HOME/.config/arvados/foo.conf.')
@@ -270,41 +267,6 @@ def exception_handler(handler, *exc_types):
except exc_types as error:
handler(error)
-def migrate_components_filters(template_components, dst_git_repo):
- """Update template component filters in-place for the destination.
-
- template_components is a dictionary of components in a pipeline template.
- This method walks over each component's filters, and updates them to have
- identical semantics on the destination cluster. It returns a list of
- error strings that describe what filters could not be updated safely.
-
- dst_git_repo is the name of the destination Git repository, which can
- be None if that is not known.
- """
- errors = []
- for cname, cspec in template_components.items():
- def add_error(errmsg):
- errors.append("{}: {}".format(cname, errmsg))
- if not isinstance(cspec, dict):
- add_error("value is not a component definition")
- continue
- src_repository = cspec.get('repository')
- filters = cspec.get('filters', [])
- if not isinstance(filters, list):
- add_error("filters are not a list")
- continue
- for cfilter in filters:
- if not (isinstance(cfilter, list) and (len(cfilter) == 3)):
- add_error("malformed filter {!r}".format(cfilter))
- continue
- if attr_filtered(cfilter, 'repository'):
- with exception_handler(add_error, ValueError):
- migrate_repository_filter(cfilter, src_repository, dst_git_repo)
- if attr_filtered(cfilter, 'script_version'):
- with exception_handler(add_error, ValueError):
- migrate_script_version_filter(cfilter)
- return errors
-
# copy_workflow(wf_uuid, src, dst, args)
#
@@ -407,53 +369,6 @@ def copy_collections(obj, src, dst, args):
return type(obj)(copy_collections(v, src, dst, args) for v in obj)
return obj
-def migrate_jobspec(jobspec, src, dst, dst_repo, args):
- """Copy a job's script to the destination repository, and update its record.
-
- Given a jobspec dictionary, this function finds the referenced script from
- src and copies it to dst and dst_repo. It also updates jobspec in place to
- refer to names on the destination.
- """
- repo = jobspec.get('repository')
- if repo is None:
- return
- # script_version is the "script_version" parameter from the source
- # component or job. If no script_version was supplied in the
- # component or job, it is a mistake in the pipeline, but for the
- # purposes of copying the repository, default to "master".
- script_version = jobspec.get('script_version') or 'master'
- script_key = (repo, script_version)
- if script_key not in scripts_copied:
- copy_git_repo(repo, src, dst, dst_repo, script_version, args)
- scripts_copied.add(script_key)
- jobspec['repository'] = dst_repo
- repo_dir = local_repo_dir[repo]
- for version_key in ['script_version', 'supplied_script_version']:
- if version_key in jobspec:
- jobspec[version_key] = git_rev_parse(jobspec[version_key], repo_dir)
-
-# copy_git_repos(p, src, dst, dst_repo, args)
-#
-# Copies all git repositories referenced by pipeline instance or
-# template 'p' from src to dst.
-#
-# For each component c in the pipeline:
-# * Copy git repositories named in c['repository'] and c['job']['repository'] if present
-# * Rename script versions:
-# * c['script_version']
-# * c['job']['script_version']
-# * c['job']['supplied_script_version']
-# to the commit hashes they resolve to, since any symbolic
-# names (tags, branches) are not preserved in the destination repo.
-#
-# The pipeline object is updated in place with the new repository
-# names. The return value is undefined.
-#
-def copy_git_repos(p, src, dst, dst_repo, args):
- for component in p['components'].values():
- migrate_jobspec(component, src, dst, dst_repo, args)
- if 'job' in component:
- migrate_jobspec(component['job'], src, dst, dst_repo, args)
def total_collection_size(manifest_text):
"""Return the total number of bytes in this collection (excluding
@@ -751,19 +666,6 @@ def copy_git_repo(src_git_repo, src, dst, dst_git_repo, script_version, args):
"ARVADOS_API_TOKEN": dst.api_token,
"GIT_ASKPASS": "/bin/false"})
-def copy_docker_images(pipeline, src, dst, args):
- """Copy any docker images named in the pipeline components'
- runtime_constraints field from src to dst."""
-
- logger.debug('copy_docker_images: {}'.format(pipeline['uuid']))
- for c_name, c_info in pipeline['components'].items():
- if ('runtime_constraints' in c_info and
- 'docker_image' in c_info['runtime_constraints']):
- copy_docker_image(
- c_info['runtime_constraints']['docker_image'],
- c_info['runtime_constraints'].get('docker_image_tag', 'latest'),
- src, dst, args)
-
def copy_docker_image(docker_image, docker_image_tag, src, dst, args):
"""Copy the docker image identified by docker_image and
commit c4f5611d0e67028b86b5c468ee1a7f200866cb32
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri Mar 20 15:47:00 2020 -0400
16138: whitelist fields for arv-copy
Also removed code for copying
PipelineTemplate/PipelineInstance (obsolete jobs API).
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/sdk/python/arvados/commands/arv_copy.py b/sdk/python/arvados/commands/arv_copy.py
index 0ba3f0a48..d94d19efc 100755
--- a/sdk/python/arvados/commands/arv_copy.py
+++ b/sdk/python/arvados/commands/arv_copy.py
@@ -144,15 +144,6 @@ def main():
result = copy_collection(args.object_uuid,
src_arv, dst_arv,
args)
- elif t == 'PipelineInstance':
- set_src_owner_uuid(src_arv.pipeline_instances(), args.object_uuid, args)
- result = copy_pipeline_instance(args.object_uuid,
- src_arv, dst_arv,
- args)
- elif t == 'PipelineTemplate':
- set_src_owner_uuid(src_arv.pipeline_templates(), args.object_uuid, args)
- result = copy_pipeline_template(args.object_uuid,
- src_arv, dst_arv, args)
elif t == 'Workflow':
set_src_owner_uuid(src_arv.workflows(), args.object_uuid, args)
result = copy_workflow(args.object_uuid, src_arv, dst_arv, args)
@@ -225,67 +216,6 @@ def check_git_availability():
except Exception:
abort('git command is not available. Please ensure git is installed.')
-# copy_pipeline_instance(pi_uuid, src, dst, args)
-#
-# Copies a pipeline instance identified by pi_uuid from src to dst.
-#
-# If the args.recursive option is set:
-# 1. Copies all input collections
-# * For each component in the pipeline, include all collections
-# listed as job dependencies for that component)
-# 2. Copy docker images
-# 3. Copy git repositories
-# 4. Copy the pipeline template
-#
-# The only changes made to the copied pipeline instance are:
-# 1. The original pipeline instance UUID is preserved in
-# the 'properties' hash as 'copied_from_pipeline_instance_uuid'.
-# 2. The pipeline_template_uuid is changed to the new template uuid.
-# 3. The owner_uuid of the instance is changed to the user who
-# copied it.
-#
-def copy_pipeline_instance(pi_uuid, src, dst, args):
- # Fetch the pipeline instance record.
- pi = src.pipeline_instances().get(uuid=pi_uuid).execute(num_retries=args.retries)
-
- if args.recursive:
- check_git_availability()
-
- if not args.dst_git_repo:
- abort('--dst-git-repo is required when copying a pipeline recursively.')
- # Copy the pipeline template and save the copied template.
- if pi.get('pipeline_template_uuid', None):
- pt = copy_pipeline_template(pi['pipeline_template_uuid'],
- src, dst, args)
-
- # Copy input collections, docker images and git repos.
- pi = copy_collections(pi, src, dst, args)
- copy_git_repos(pi, src, dst, args.dst_git_repo, args)
- copy_docker_images(pi, src, dst, args)
-
- # Update the fields of the pipeline instance with the copied
- # pipeline template.
- if pi.get('pipeline_template_uuid', None):
- pi['pipeline_template_uuid'] = pt['uuid']
-
- else:
- # not recursive
- logger.info("Copying only pipeline instance %s.", pi_uuid)
- logger.info("You are responsible for making sure all pipeline dependencies have been updated.")
-
- # Update the pipeline instance properties, and create the new
- # instance at dst.
- pi['properties']['copied_from_pipeline_instance_uuid'] = pi_uuid
- pi['description'] = "Pipeline copied from {}\n\n{}".format(
- pi_uuid,
- pi['description'] if pi.get('description', None) else '')
-
- pi['owner_uuid'] = args.project_uuid
-
- del pi['uuid']
-
- new_pi = dst.pipeline_instances().create(body=pi, ensure_unique_name=True).execute(num_retries=args.retries)
- return new_pi
def filter_iter(arg):
"""Iterate a filter string-or-list.
@@ -375,47 +305,6 @@ def migrate_components_filters(template_components, dst_git_repo):
migrate_script_version_filter(cfilter)
return errors
-# copy_pipeline_template(pt_uuid, src, dst, args)
-#
-# Copies a pipeline template identified by pt_uuid from src to dst.
-#
-# If args.recursive is True, also copy any collections, docker
-# images and git repositories that this template references.
-#
-# The owner_uuid of the new template is changed to that of the user
-# who copied the template.
-#
-# Returns the copied pipeline template object.
-#
-def copy_pipeline_template(pt_uuid, src, dst, args):
- # fetch the pipeline template from the source instance
- pt = src.pipeline_templates().get(uuid=pt_uuid).execute(num_retries=args.retries)
-
- if not args.force_filters:
- filter_errors = migrate_components_filters(pt['components'], args.dst_git_repo)
- if filter_errors:
- abort("Template filters cannot be copied safely. Use --force-filters to copy anyway.\n" +
- "\n".join(filter_errors))
-
- if args.recursive:
- check_git_availability()
-
- if not args.dst_git_repo:
- abort('--dst-git-repo is required when copying a pipeline recursively.')
- # Copy input collections, docker images and git repos.
- pt = copy_collections(pt, src, dst, args)
- copy_git_repos(pt, src, dst, args.dst_git_repo, args)
- copy_docker_images(pt, src, dst, args)
-
- pt['description'] = "Pipeline template copied from {}\n\n{}".format(
- pt_uuid,
- pt['description'] if pt.get('description', None) else '')
- pt['name'] = "{} copied from {}".format(pt.get('name', ''), pt_uuid)
- del pt['uuid']
-
- pt['owner_uuid'] = args.project_uuid
-
- return dst.pipeline_templates().create(body=pt, ensure_unique_name=True).execute(num_retries=args.retries)
# copy_workflow(wf_uuid, src, dst, args)
#
@@ -590,17 +479,16 @@ def create_collection_from(c, src, dst, args):
available."""
collection_uuid = c['uuid']
- del c['uuid']
-
- if not c["name"]:
- c['name'] = "copied from " + collection_uuid
+ body = {}
+ for d in ('description', 'manifest_text', 'name', 'portable_data_hash', 'properties'):
+ body[d] = c[d]
- if 'properties' in c:
- del c['properties']
+ if not body["name"]:
+ body['name'] = "copied from " + collection_uuid
- c['owner_uuid'] = args.project_uuid
+ body['owner_uuid'] = args.project_uuid
- dst_collection = dst.collections().create(body=c, ensure_unique_name=True).execute(num_retries=args.retries)
+ dst_collection = dst.collections().create(body=body, ensure_unique_name=True).execute(num_retries=args.retries)
# Create docker_image_repo+tag and docker_image_hash links
# at the destination.
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list