[ARVADOS] updated: 2.3.0-28-gbbc934a55
Git user
git at public.arvados.org
Wed Nov 17 19:50:22 UTC 2021
Summary of changes:
apps/workbench/Gemfile.lock | 8 +-
...llection-managed-properties.html.textile.liquid | 12 ++-
doc/admin/upgrading.html.textile.liquid | 4 +
.../collection-versioning.html.textile.liquid | 2 +-
lib/config/config.default.yml | 10 +++
lib/config/export.go | 3 +-
lib/config/generated_config.go | 10 +++
lib/controller/integration_test.go | 6 +-
lib/controller/router/response.go | 4 +
lib/controller/router/router_test.go | 21 +++++
lib/crunchrun/crunchrun.go | 7 +-
lib/crunchrun/crunchrun_test.go | 14 ++--
sdk/cwl/setup.py | 4 +-
sdk/go/arvados/config.go | 1 +
sdk/python/arvados/collection.py | 30 ++++++-
sdk/python/setup.py | 2 +-
sdk/python/tests/run_test_server.py | 1 +
sdk/python/tests/test_collections.py | 19 +++++
services/api/Gemfile.lock | 8 +-
.../controllers/arvados/v1/groups_controller.rb | 13 ++-
services/api/app/models/user.rb | 33 ++++++--
...7154300_delete_disabled_user_tokens_and_keys.rb | 15 ++++
services/api/db/structure.sql | 3 +-
.../arvados/v1/groups_controller_test.rb | 15 ++++
.../functional/arvados/v1/users_controller_test.rb | 1 +
services/api/test/integration/users_test.rb | 22 ++++-
services/api/test/unit/permission_test.rb | 2 +
services/api/test/unit/user_test.rb | 47 +++++++----
services/fuse/arvados_fuse/command.py | 4 +-
services/fuse/arvados_fuse/fusedir.py | 93 ++++++++++++++--------
services/fuse/arvados_fuse/fusefile.py | 7 +-
services/fuse/tests/mount_test_base.py | 7 +-
services/fuse/tests/test_mount.py | 24 +++++-
services/keepstore/unix_volume.go | 92 +++++++++++----------
services/login-sync/arvados-login-sync.gemspec | 4 +-
35 files changed, 400 insertions(+), 148 deletions(-)
create mode 100644 services/api/db/migrate/20211027154300_delete_disabled_user_tokens_and_keys.rb
via bbc934a55d42bcd46ad0a7d33456b37c0be18f61 (commit)
via c73a78ead6b493df1f4b44cd1e1a43d6c268f6fa (commit)
via 6a7233ad1f3afc8b128c647810d38ad9cd158f69 (commit)
via 59240220e48bcf508daebcf980c1e2db20ccc0e7 (commit)
via b9b43736e711f10fcf9c031bafba2464bb2ce386 (commit)
via ca0dd0691c1d5053794681bbfb063926e49c039a (commit)
via 595af530fb6a19152421af0f7134953bb366f668 (commit)
via 9e3e3bcd81a4fc80e1aaa33e7a1711a74099e0e4 (commit)
via bd8ee613953e8cbcbb572b648e87602397ba31bb (commit)
via 773413b6decf25e4ab669881e00c507aa8a1486f (commit)
via 28e35c535b8fd442dce3a286c4503517dc848848 (commit)
via 1dc17e4eee5367c7684888c8dcaa6445b576537c (commit)
via bc9d8d1e4caeef8c4b2da02f9a134fc7b57148d7 (commit)
via ac92153c3aa05af1755b1afe225d3355fcca160d (commit)
via 73d113eab7fef74d9519be5236e89b48aeb2eab2 (commit)
via dcd9dd1ec965190dcece4b8ef3f9379776a309e8 (commit)
via 7dd7f8d08b1bbf4692b2f1678d78047489b6fd37 (commit)
via a6f94a674bdbb99cc3fb19cff6a7ffbf4c3520ee (commit)
from 5c4316723fda70348f841a3ad1a7d8385f9e3c4a (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 bbc934a55d42bcd46ad0a7d33456b37c0be18f61
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed Nov 17 12:55:16 2021 -0500
Merge branch '18285-cwl-hint-warning' refs #18285
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/sdk/cwl/setup.py b/sdk/cwl/setup.py
index e39fdd8d9..f034ca5ab 100644
--- a/sdk/cwl/setup.py
+++ b/sdk/cwl/setup.py
@@ -39,8 +39,8 @@ setup(name='arvados-cwl-runner',
# file to determine what version of cwltool and schema-salad to
# build.
install_requires=[
- 'cwltool==3.1.20211020155521',
- 'schema-salad==8.2.20211020114435',
+ 'cwltool==3.1.20211107152837',
+ 'schema-salad==8.2.20211116214159',
'arvados-python-client{}'.format(pysdk_dep),
'setuptools',
'ciso8601 >= 2.0.0',
commit c73a78ead6b493df1f4b44cd1e1a43d6c268f6fa
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Wed Nov 17 15:28:37 2021 -0300
Merge branch '18363-managed-properties-doc-improvement' into main.
Closes #18363
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/doc/admin/collection-managed-properties.html.textile.liquid b/doc/admin/collection-managed-properties.html.textile.liquid
index 395200126..341030c41 100644
--- a/doc/admin/collection-managed-properties.html.textile.liquid
+++ b/doc/admin/collection-managed-properties.html.textile.liquid
@@ -41,13 +41,23 @@ h4. Protected properties
If there's a need to prevent a non-admin user from modifying a specific property, even by its owner, the @Protected@ attribute can be set to @true@, like so:
+<pre>
+Collections:
+ ManagedProperties:
+ sample_id: {Protected: true}
+</pre>
+
+This configuration won't assign a @sample_id@ property on collection creation, but if the user adds it to any collection, its value is protected from that point on.
+
+Another use case would be to protect properties that were automatically assigned by the system:
+
<pre>
Collections:
ManagedProperties:
responsible_person_uuid: {Function: original_owner, Protected: true}
</pre>
-This property can be applied to any of the defined managed properties. If missing, it's assumed as being @false@ by default.
+If missing, the @Protected@ attribute it’s assumed as being @false@ by default.
h3. Supporting example scripts
commit 6a7233ad1f3afc8b128c647810d38ad9cd158f69
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Wed Nov 17 12:03:42 2021 -0300
Merge branch '18340-delete-role-filter-groups' into main. Closes #18340.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/services/api/app/controllers/arvados/v1/groups_controller.rb b/services/api/app/controllers/arvados/v1/groups_controller.rb
index 8d15bb1c5..7fbb86c01 100644
--- a/services/api/app/controllers/arvados/v1/groups_controller.rb
+++ b/services/api/app/controllers/arvados/v1/groups_controller.rb
@@ -10,6 +10,8 @@ class Arvados::V1::GroupsController < ApplicationController
skip_before_action :find_object_by_uuid, only: :shared
skip_before_action :render_404_if_no_object, only: :shared
+ TRASHABLE_CLASSES = ['project']
+
def self._index_requires_parameters
(super rescue {}).
merge({
@@ -99,6 +101,15 @@ class Arvados::V1::GroupsController < ApplicationController
end
end
+ def destroy
+ if !TRASHABLE_CLASSES.include?(@object.group_class)
+ return @object.destroy
+ show
+ else
+ super # Calls destroy from TrashableController module
+ end
+ end
+
def render_404_if_no_object
if params[:action] == 'contents'
if !params[:uuid]
@@ -351,8 +362,6 @@ class Arvados::V1::GroupsController < ApplicationController
@offset = offset_all
end
- protected
-
def exclude_home objectlist, klass
# select records that are readable by current user AND
# the owner_uuid is a user (but not the current user) OR
diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb
index 02a4ce966..4dbccc5eb 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -538,6 +538,21 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
assert_includes(owners, groups(:asubproject).uuid)
end
+ [:afiltergroup, :private_role].each do |grp|
+ test "delete non-project group #{grp}" do
+ authorize_with :admin
+ assert_not_nil Group.find_by_uuid(groups(grp).uuid)
+ assert !Group.find_by_uuid(groups(grp).uuid).is_trashed
+ post :destroy, params: {
+ id: groups(grp).uuid,
+ format: :json,
+ }
+ assert_response :success
+ # Should not be trashed
+ assert_nil Group.find_by_uuid(groups(grp).uuid)
+ end
+ end
+
### trashed project tests ###
#
commit 59240220e48bcf508daebcf980c1e2db20ccc0e7
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Wed Nov 17 12:16:54 2021 -0300
Merge branch '18336-httplib2-pysdk-issues' into main. Closes #18336
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/sdk/python/setup.py b/sdk/python/setup.py
index 8d637303b..f82d44ab6 100644
--- a/sdk/python/setup.py
+++ b/sdk/python/setup.py
@@ -50,7 +50,7 @@ setup(name='arvados-python-client',
'future',
'google-api-python-client >=1.6.2, <2',
'google-auth<2',
- 'httplib2 >=0.9.2',
+ 'httplib2 >=0.9.2, <0.20.2',
'pycurl >=7.19.5.1',
'ruamel.yaml >=0.15.54, <0.17.11',
'setuptools',
commit b9b43736e711f10fcf9c031bafba2464bb2ce386
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri Nov 12 09:38:05 2021 -0500
Merge branch '18346-crunchrun-no-events' refs #18346
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index 3036d5555..10e5193a8 100644
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -605,10 +605,15 @@ func (runner *ContainerRunner) SetupMounts() (map[string]bindmount, error) {
}
if pdhOnly {
- arvMountCmd = append(arvMountCmd, "--mount-by-pdh", "by_id")
+ // If we are only mounting collections by pdh, make
+ // sure we don't subscribe to websocket events to
+ // avoid putting undesired load on the API server
+ arvMountCmd = append(arvMountCmd, "--mount-by-pdh", "by_id", "--disable-event-listening")
} else {
arvMountCmd = append(arvMountCmd, "--mount-by-id", "by_id")
}
+ // the by_uuid mount point is used by singularity when writing
+ // out docker images converted to SIF
arvMountCmd = append(arvMountCmd, "--mount-by-id", "by_uuid")
arvMountCmd = append(arvMountCmd, runner.ArvMountPoint)
diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go
index 4c5f517b1..c28cf73cb 100644
--- a/lib/crunchrun/crunchrun_test.go
+++ b/lib/crunchrun/crunchrun_test.go
@@ -1126,7 +1126,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
c.Check(err, IsNil)
c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground",
"--read-write", "--storage-classes", "default", "--crunchstat-interval=5",
- "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
+ "--mount-by-pdh", "by_id", "--disable-event-listening", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
c.Check(bindmounts, DeepEquals, map[string]bindmount{"/tmp": {realTemp + "/tmp2", false}})
os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
@@ -1146,7 +1146,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
c.Check(err, IsNil)
c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground",
"--read-write", "--storage-classes", "foo,bar", "--crunchstat-interval=5",
- "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
+ "--mount-by-pdh", "by_id", "--disable-event-listening", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
c.Check(bindmounts, DeepEquals, map[string]bindmount{"/out": {realTemp + "/tmp2", false}, "/tmp": {realTemp + "/tmp3", false}})
os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
@@ -1166,7 +1166,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
c.Check(err, IsNil)
c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground",
"--read-write", "--storage-classes", "default", "--crunchstat-interval=5",
- "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
+ "--mount-by-pdh", "by_id", "--disable-event-listening", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
c.Check(bindmounts, DeepEquals, map[string]bindmount{"/tmp": {realTemp + "/tmp2", false}, "/etc/arvados/ca-certificates.crt": {stubCertPath, true}})
os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
@@ -1189,7 +1189,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
c.Check(err, IsNil)
c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground",
"--read-write", "--storage-classes", "default", "--crunchstat-interval=5",
- "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
+ "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", "--disable-event-listening", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
c.Check(bindmounts, DeepEquals, map[string]bindmount{"/keeptmp": {realTemp + "/keep1/tmp0", false}})
os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
@@ -1212,7 +1212,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
c.Check(err, IsNil)
c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground",
"--read-write", "--storage-classes", "default", "--crunchstat-interval=5",
- "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
+ "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", "--disable-event-listening", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
c.Check(bindmounts, DeepEquals, map[string]bindmount{
"/keepinp": {realTemp + "/keep1/by_id/59389a8f9ee9d399be35462a0f92541c+53", true},
"/keepout": {realTemp + "/keep1/tmp0", false},
@@ -1239,7 +1239,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
c.Check(err, IsNil)
c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground",
"--read-write", "--storage-classes", "default", "--crunchstat-interval=5",
- "--file-cache", "512", "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
+ "--file-cache", "512", "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", "--disable-event-listening", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
c.Check(bindmounts, DeepEquals, map[string]bindmount{
"/keepinp": {realTemp + "/keep1/by_id/59389a8f9ee9d399be35462a0f92541c+53", true},
"/keepout": {realTemp + "/keep1/tmp0", false},
@@ -1322,7 +1322,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
c.Check(err, IsNil)
c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground",
"--read-write", "--storage-classes", "default", "--crunchstat-interval=5",
- "--file-cache", "512", "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
+ "--file-cache", "512", "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", "--disable-event-listening", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
c.Check(bindmounts, DeepEquals, map[string]bindmount{
"/tmp": {realTemp + "/tmp2", false},
"/tmp/foo": {realTemp + "/keep1/tmp0", true},
commit ca0dd0691c1d5053794681bbfb063926e49c039a
Author: Tom Clegg <tom at curii.com>
Date: Tue Nov 16 16:34:45 2021 -0500
Merge branch '18376-nfs-readdirent'
fixes #18376
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/services/keepstore/unix_volume.go b/services/keepstore/unix_volume.go
index f076ccf18..46f4db409 100644
--- a/services/keepstore/unix_volume.go
+++ b/services/keepstore/unix_volume.go
@@ -359,47 +359,53 @@ var blockFileRe = regexp.MustCompile(`^[0-9a-f]{32}$`)
// e4de7a2810f5554cd39b36d8ddb132ff+67108864 1388701136
//
func (v *UnixVolume) IndexTo(prefix string, w io.Writer) error {
- var lastErr error
rootdir, err := v.os.Open(v.Root)
if err != nil {
return err
}
- defer rootdir.Close()
v.os.stats.TickOps("readdir")
v.os.stats.Tick(&v.os.stats.ReaddirOps)
- for {
- names, err := rootdir.Readdirnames(1)
- if err == io.EOF {
- return lastErr
- } else if err != nil {
- return err
- }
- if !strings.HasPrefix(names[0], prefix) && !strings.HasPrefix(prefix, names[0]) {
+ subdirs, err := rootdir.Readdirnames(-1)
+ rootdir.Close()
+ if err != nil {
+ return err
+ }
+ for _, subdir := range subdirs {
+ if !strings.HasPrefix(subdir, prefix) && !strings.HasPrefix(prefix, subdir) {
// prefix excludes all blocks stored in this dir
continue
}
- if !blockDirRe.MatchString(names[0]) {
+ if !blockDirRe.MatchString(subdir) {
continue
}
- blockdirpath := filepath.Join(v.Root, names[0])
+ blockdirpath := filepath.Join(v.Root, subdir)
blockdir, err := v.os.Open(blockdirpath)
if err != nil {
v.logger.WithError(err).Errorf("error reading %q", blockdirpath)
- lastErr = fmt.Errorf("error reading %q: %s", blockdirpath, err)
- continue
+ return fmt.Errorf("error reading %q: %s", blockdirpath, err)
}
v.os.stats.TickOps("readdir")
v.os.stats.Tick(&v.os.stats.ReaddirOps)
- for {
- fileInfo, err := blockdir.Readdir(1)
- if err == io.EOF {
- break
+ // ReadDir() (compared to Readdir(), which returns
+ // FileInfo structs) helps complete the sequence of
+ // readdirent calls as quickly as possible, reducing
+ // the likelihood of NFS EBADCOOKIE (523) errors.
+ dirents, err := blockdir.ReadDir(-1)
+ blockdir.Close()
+ if err != nil {
+ v.logger.WithError(err).Errorf("error reading %q", blockdirpath)
+ return fmt.Errorf("error reading %q: %s", blockdirpath, err)
+ }
+ for _, dirent := range dirents {
+ fileInfo, err := dirent.Info()
+ if os.IsNotExist(err) {
+ // File disappeared between ReadDir() and now
+ continue
} else if err != nil {
- v.logger.WithError(err).Errorf("error reading %q", blockdirpath)
- lastErr = fmt.Errorf("error reading %q: %s", blockdirpath, err)
- break
+ v.logger.WithError(err).Errorf("error getting FileInfo for %q in %q", dirent.Name(), blockdirpath)
+ return err
}
- name := fileInfo[0].Name()
+ name := fileInfo.Name()
if !strings.HasPrefix(name, prefix) {
continue
}
@@ -408,16 +414,15 @@ func (v *UnixVolume) IndexTo(prefix string, w io.Writer) error {
}
_, err = fmt.Fprint(w,
name,
- "+", fileInfo[0].Size(),
- " ", fileInfo[0].ModTime().UnixNano(),
+ "+", fileInfo.Size(),
+ " ", fileInfo.ModTime().UnixNano(),
"\n")
if err != nil {
- blockdir.Close()
return fmt.Errorf("error writing: %s", err)
}
}
- blockdir.Close()
}
+ return nil
}
// Trash trashes the block data from the unix storage
commit 595af530fb6a19152421af0f7134953bb366f668
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Tue Nov 16 15:48:47 2021 -0300
Merge branch '17635-pysdk-collection-preserve-version' into main. Closes #17635
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/doc/user/topics/collection-versioning.html.textile.liquid b/doc/user/topics/collection-versioning.html.textile.liquid
index 9a32de0d0..d6a3bb4c1 100644
--- a/doc/user/topics/collection-versioning.html.textile.liquid
+++ b/doc/user/topics/collection-versioning.html.textile.liquid
@@ -18,7 +18,7 @@ A version will be saved when one of the following conditions is true:
One is by "configuring (system-wide) the collection's idle time":{{site.baseurl}}/admin/collection-versioning.html. This idle time is checked against the @modified_at@ attribute so that the version is saved when one or more of the previously enumerated attributes get updated and the @modified_at@ is at least at the configured idle time in the past. This way, a frequently updated collection won't create lots of version records that may not be useful.
-The other way to trigger a version save, is by setting @preserve_version@ to @true@ on the current version collection record: this ensures that the current state will be preserved as a version the next time it gets updated.
+The other way to trigger a version save, is by setting @preserve_version@ to @true@ on the current version collection record: this ensures that the current state will be preserved as a version the next time it gets updated. This includes either creating a new collection or updating a preexisting one. In the case of using @preserve_version = true@ on a collection's create call, the new record state will be preserved as a snapshot on the next update.
h3. Collection's past versions behavior & limitations
diff --git a/lib/config/export.go b/lib/config/export.go
index 1d2ea6c98..b413bcd75 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -96,7 +96,7 @@ var whitelist = map[string]bool{
"Collections.BlobTrashCheckInterval": false,
"Collections.BlobTrashConcurrency": false,
"Collections.BlobTrashLifetime": false,
- "Collections.CollectionVersioning": false,
+ "Collections.CollectionVersioning": true,
"Collections.DefaultReplication": true,
"Collections.DefaultTrashLifetime": true,
"Collections.ForwardSlashNameSubstitution": true,
diff --git a/sdk/python/arvados/collection.py b/sdk/python/arvados/collection.py
index d03265ca4..55be40fa0 100644
--- a/sdk/python/arvados/collection.py
+++ b/sdk/python/arvados/collection.py
@@ -1546,7 +1546,8 @@ class Collection(RichCollectionBase):
storage_classes=None,
trash_at=None,
merge=True,
- num_retries=None):
+ num_retries=None,
+ preserve_version=False):
"""Save collection to an existing collection record.
Commit pending buffer blocks to Keep, merge with remote record (if
@@ -1576,6 +1577,13 @@ class Collection(RichCollectionBase):
:num_retries:
Retry count on API calls (if None, use the collection default)
+ :preserve_version:
+ If True, indicate that the collection content being saved right now
+ should be preserved in a version snapshot if the collection record is
+ updated in the future. Requires that the API server has
+ Collections.CollectionVersioning enabled, if not, setting this will
+ raise an exception.
+
"""
if properties and type(properties) is not dict:
raise errors.ArgumentError("properties must be dictionary type.")
@@ -1588,6 +1596,9 @@ class Collection(RichCollectionBase):
if trash_at and type(trash_at) is not datetime.datetime:
raise errors.ArgumentError("trash_at must be datetime type.")
+ if preserve_version and not self._my_api().config()['Collections'].get('CollectionVersioning', False):
+ raise errors.ArgumentError("preserve_version is not supported when CollectionVersioning is not enabled.")
+
body={}
if properties:
body["properties"] = properties
@@ -1596,6 +1607,8 @@ class Collection(RichCollectionBase):
if trash_at:
t = trash_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ")
body["trash_at"] = t
+ if preserve_version:
+ body["preserve_version"] = preserve_version
if not self.committed():
if self._has_remote_blocks:
@@ -1641,7 +1654,8 @@ class Collection(RichCollectionBase):
storage_classes=None,
trash_at=None,
ensure_unique_name=False,
- num_retries=None):
+ num_retries=None,
+ preserve_version=False):
"""Save collection to a new collection record.
Commit pending buffer blocks to Keep and, when create_collection_record
@@ -1680,6 +1694,13 @@ class Collection(RichCollectionBase):
:num_retries:
Retry count on API calls (if None, use the collection default)
+ :preserve_version:
+ If True, indicate that the collection content being saved right now
+ should be preserved in a version snapshot if the collection record is
+ updated in the future. Requires that the API server has
+ Collections.CollectionVersioning enabled, if not, setting this will
+ raise an exception.
+
"""
if properties and type(properties) is not dict:
raise errors.ArgumentError("properties must be dictionary type.")
@@ -1690,6 +1711,9 @@ class Collection(RichCollectionBase):
if trash_at and type(trash_at) is not datetime.datetime:
raise errors.ArgumentError("trash_at must be datetime type.")
+ if preserve_version and not self._my_api().config()['Collections'].get('CollectionVersioning', False):
+ raise errors.ArgumentError("preserve_version is not supported when CollectionVersioning is not enabled.")
+
if self._has_remote_blocks:
# Copy any remote blocks to the local cluster.
self._copy_remote_blocks(remote_blocks={})
@@ -1718,6 +1742,8 @@ class Collection(RichCollectionBase):
if trash_at:
t = trash_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ")
body["trash_at"] = t
+ if preserve_version:
+ body["preserve_version"] = preserve_version
self._remember_api_response(self._my_api().collections().create(ensure_unique_name=ensure_unique_name, body=body).execute(num_retries=num_retries))
text = self._api_response["manifest_text"]
diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index 6d2643a96..f91783250 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -791,6 +791,7 @@ def setup_config():
"UserProfileNotificationAddress": "arvados at example.com",
},
"Collections": {
+ "CollectionVersioning": True,
"BlobSigningKey": "zfhgfenhffzltr9dixws36j1yhksjoll2grmku38mi7yxd66h5j4q9w4jzanezacp8s6q0ro3hxakfye02152hncy6zml2ed0uc",
"TrustAllContent": False,
"ForwardSlashNameSubstitution": "/",
diff --git a/sdk/python/tests/test_collections.py b/sdk/python/tests/test_collections.py
index f821ff952..a43e0d40d 100644
--- a/sdk/python/tests/test_collections.py
+++ b/sdk/python/tests/test_collections.py
@@ -1360,6 +1360,25 @@ class NewCollectionTestCaseWithServersAndTokens(run_test_server.TestCaseWithServ
class NewCollectionTestCaseWithServers(run_test_server.TestCaseWithServers):
+ def test_preserve_version_on_save(self):
+ c = Collection()
+ c.save_new(preserve_version=True)
+ coll_record = arvados.api().collections().get(uuid=c.manifest_locator()).execute()
+ self.assertEqual(coll_record['version'], 1)
+ self.assertEqual(coll_record['preserve_version'], True)
+ with c.open("foo.txt", "wb") as foo:
+ foo.write(b"foo")
+ c.save(preserve_version=True)
+ coll_record = arvados.api().collections().get(uuid=c.manifest_locator()).execute()
+ self.assertEqual(coll_record['version'], 2)
+ self.assertEqual(coll_record['preserve_version'], True)
+ with c.open("bar.txt", "wb") as foo:
+ foo.write(b"bar")
+ c.save(preserve_version=False)
+ coll_record = arvados.api().collections().get(uuid=c.manifest_locator()).execute()
+ self.assertEqual(coll_record['version'], 3)
+ self.assertEqual(coll_record['preserve_version'], False)
+
def test_get_manifest_text_only_committed(self):
c = Collection()
with c.open("count.txt", "wb") as f:
commit 9e3e3bcd81a4fc80e1aaa33e7a1711a74099e0e4
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Mon Nov 15 15:12:19 2021 -0300
Merge branch '18215-select-param-update-create' into main. Refs #18215
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/lib/controller/router/response.go b/lib/controller/router/response.go
index 03cdcf18d..01126bcb4 100644
--- a/lib/controller/router/response.go
+++ b/lib/controller/router/response.go
@@ -26,6 +26,10 @@ type responseOptions struct {
func (rtr *router) responseOptions(opts interface{}) (responseOptions, error) {
var rOpts responseOptions
switch opts := opts.(type) {
+ case *arvados.CreateOptions:
+ rOpts.Select = opts.Select
+ case *arvados.UpdateOptions:
+ rOpts.Select = opts.Select
case *arvados.GetOptions:
rOpts.Select = opts.Select
case *arvados.ListOptions:
diff --git a/lib/controller/router/router_test.go b/lib/controller/router/router_test.go
index 722895645..ce440dac5 100644
--- a/lib/controller/router/router_test.go
+++ b/lib/controller/router/router_test.go
@@ -379,6 +379,7 @@ func (s *RouterIntegrationSuite) TestFullTimestampsInResponse(c *check.C) {
func (s *RouterIntegrationSuite) TestSelectParam(c *check.C) {
uuid := arvadostest.QueuedContainerUUID
token := arvadostest.ActiveTokenV2
+ // GET
for _, sel := range [][]string{
{"uuid", "command"},
{"uuid", "command", "uuid"},
@@ -395,6 +396,26 @@ func (s *RouterIntegrationSuite) TestSelectParam(c *check.C) {
_, hasMounts := resp["mounts"]
c.Check(hasMounts, check.Equals, false)
}
+ // POST & PUT
+ uuid = arvadostest.FooCollection
+ j, err := json.Marshal([]string{"uuid", "description"})
+ c.Assert(err, check.IsNil)
+ for _, method := range []string{"PUT", "POST"} {
+ desc := "Today is " + time.Now().String()
+ reqBody := "{\"description\":\"" + desc + "\"}"
+ var resp map[string]interface{}
+ var rr *httptest.ResponseRecorder
+ if method == "PUT" {
+ _, rr, resp = doRequest(c, s.rtr, token, method, "/arvados/v1/collections/"+uuid+"?select="+string(j), nil, bytes.NewReader([]byte(reqBody)))
+ } else {
+ _, rr, resp = doRequest(c, s.rtr, token, method, "/arvados/v1/collections?select="+string(j), nil, bytes.NewReader([]byte(reqBody)))
+ }
+ c.Check(rr.Code, check.Equals, http.StatusOK)
+ c.Check(resp["kind"], check.Equals, "arvados#collection")
+ c.Check(resp["uuid"], check.HasLen, 27)
+ c.Check(resp["description"], check.Equals, desc)
+ c.Check(resp["manifest_text"], check.IsNil)
+ }
}
func (s *RouterIntegrationSuite) TestHEAD(c *check.C) {
commit bd8ee613953e8cbcbb572b648e87602397ba31bb
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Nov 15 12:50:54 2021 -0500
Merge branch '18316-fuse-read-only' refs #18316
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/fuse/arvados_fuse/command.py b/services/fuse/arvados_fuse/command.py
index 67a2aaa4d..5f0a1f80f 100644
--- a/services/fuse/arvados_fuse/command.py
+++ b/services/fuse/arvados_fuse/command.py
@@ -244,7 +244,7 @@ class Mount(object):
usr = self.api.users().current().execute(num_retries=self.args.retries)
now = time.time()
dir_class = None
- dir_args = [llfuse.ROOT_INODE, self.operations.inodes, self.api, self.args.retries]
+ dir_args = [llfuse.ROOT_INODE, self.operations.inodes, self.api, self.args.retries, self.args.enable_write]
mount_readme = False
storage_classes = None
@@ -310,7 +310,7 @@ class Mount(object):
return
e = self.operations.inodes.add_entry(Directory(
- llfuse.ROOT_INODE, self.operations.inodes, self.api.config))
+ llfuse.ROOT_INODE, self.operations.inodes, self.api.config, self.args.enable_write))
dir_args[0] = e.inode
for name in self.args.mount_by_id:
diff --git a/services/fuse/arvados_fuse/fusedir.py b/services/fuse/arvados_fuse/fusedir.py
index d5a018ae8..a2e33c7b3 100644
--- a/services/fuse/arvados_fuse/fusedir.py
+++ b/services/fuse/arvados_fuse/fusedir.py
@@ -36,7 +36,7 @@ class Directory(FreshBase):
and the value referencing a File or Directory object.
"""
- def __init__(self, parent_inode, inodes, apiconfig):
+ def __init__(self, parent_inode, inodes, apiconfig, enable_write):
"""parent_inode is the integer inode number"""
super(Directory, self).__init__()
@@ -49,6 +49,7 @@ class Directory(FreshBase):
self.apiconfig = apiconfig
self._entries = {}
self._mtime = time.time()
+ self._enable_write = enable_write
def forward_slash_subst(self):
if not hasattr(self, '_fsns'):
@@ -269,8 +270,8 @@ class CollectionDirectoryBase(Directory):
"""
- def __init__(self, parent_inode, inodes, apiconfig, collection):
- super(CollectionDirectoryBase, self).__init__(parent_inode, inodes, apiconfig)
+ def __init__(self, parent_inode, inodes, apiconfig, enable_write, collection):
+ super(CollectionDirectoryBase, self).__init__(parent_inode, inodes, apiconfig, enable_write)
self.apiconfig = apiconfig
self.collection = collection
@@ -284,10 +285,10 @@ class CollectionDirectoryBase(Directory):
item.fuse_entry.dead = False
self._entries[name] = item.fuse_entry
elif isinstance(item, arvados.collection.RichCollectionBase):
- self._entries[name] = self.inodes.add_entry(CollectionDirectoryBase(self.inode, self.inodes, self.apiconfig, item))
+ self._entries[name] = self.inodes.add_entry(CollectionDirectoryBase(self.inode, self.inodes, self.apiconfig, self._enable_write, item))
self._entries[name].populate(mtime)
else:
- self._entries[name] = self.inodes.add_entry(FuseArvadosFile(self.inode, item, mtime))
+ self._entries[name] = self.inodes.add_entry(FuseArvadosFile(self.inode, item, mtime, self._enable_write))
item.fuse_entry = self._entries[name]
def on_event(self, event, collection, name, item):
@@ -348,28 +349,36 @@ class CollectionDirectoryBase(Directory):
self.new_entry(entry, item, self.mtime())
def writable(self):
- return self.collection.writable()
+ return self._enable_write and self.collection.writable()
@use_counter
def flush(self):
+ if not self.writable():
+ return
with llfuse.lock_released:
self.collection.root_collection().save()
@use_counter
@check_update
def create(self, name):
+ if not self.writable():
+ raise llfuse.FUSEError(errno.EROFS)
with llfuse.lock_released:
self.collection.open(name, "w").close()
@use_counter
@check_update
def mkdir(self, name):
+ if not self.writable():
+ raise llfuse.FUSEError(errno.EROFS)
with llfuse.lock_released:
self.collection.mkdirs(name)
@use_counter
@check_update
def unlink(self, name):
+ if not self.writable():
+ raise llfuse.FUSEError(errno.EROFS)
with llfuse.lock_released:
self.collection.remove(name)
self.flush()
@@ -377,6 +386,8 @@ class CollectionDirectoryBase(Directory):
@use_counter
@check_update
def rmdir(self, name):
+ if not self.writable():
+ raise llfuse.FUSEError(errno.EROFS)
with llfuse.lock_released:
self.collection.remove(name)
self.flush()
@@ -384,6 +395,9 @@ class CollectionDirectoryBase(Directory):
@use_counter
@check_update
def rename(self, name_old, name_new, src):
+ if not self.writable():
+ raise llfuse.FUSEError(errno.EROFS)
+
if not isinstance(src, CollectionDirectoryBase):
raise llfuse.FUSEError(errno.EPERM)
@@ -413,8 +427,8 @@ class CollectionDirectoryBase(Directory):
class CollectionDirectory(CollectionDirectoryBase):
"""Represents the root of a directory tree representing a collection."""
- def __init__(self, parent_inode, inodes, api, num_retries, collection_record=None, explicit_collection=None):
- super(CollectionDirectory, self).__init__(parent_inode, inodes, api.config, None)
+ def __init__(self, parent_inode, inodes, api, num_retries, enable_write, collection_record=None, explicit_collection=None):
+ super(CollectionDirectory, self).__init__(parent_inode, inodes, api.config, enable_write, None)
self.api = api
self.num_retries = num_retries
self.collection_record_file = None
@@ -434,14 +448,14 @@ class CollectionDirectory(CollectionDirectoryBase):
self._mtime = 0
self._manifest_size = 0
if self.collection_locator:
- self._writable = (uuid_pattern.match(self.collection_locator) is not None)
+ self._writable = (uuid_pattern.match(self.collection_locator) is not None) and enable_write
self._updating_lock = threading.Lock()
def same(self, i):
return i['uuid'] == self.collection_locator or i['portable_data_hash'] == self.collection_locator
def writable(self):
- return self.collection.writable() if self.collection is not None else self._writable
+ return self._enable_write and (self.collection.writable() if self.collection is not None else self._writable)
def want_event_subscribe(self):
return (uuid_pattern.match(self.collection_locator) is not None)
@@ -603,14 +617,16 @@ class TmpCollectionDirectory(CollectionDirectoryBase):
def save_new(self):
pass
- def __init__(self, parent_inode, inodes, api_client, num_retries, storage_classes=None):
+ def __init__(self, parent_inode, inodes, api_client, num_retries, enable_write, storage_classes=None):
collection = self.UnsaveableCollection(
api_client=api_client,
keep_client=api_client.keep,
num_retries=num_retries,
storage_classes_desired=storage_classes)
+ # This is always enable_write=True because it never tries to
+ # save to the backend
super(TmpCollectionDirectory, self).__init__(
- parent_inode, inodes, api_client.config, collection)
+ parent_inode, inodes, api_client.config, True, collection)
self.collection_record_file = None
self.populate(self.mtime())
@@ -703,8 +719,8 @@ and the directory will appear if it exists.
""".lstrip()
- def __init__(self, parent_inode, inodes, api, num_retries, pdh_only=False, storage_classes=None):
- super(MagicDirectory, self).__init__(parent_inode, inodes, api.config)
+ def __init__(self, parent_inode, inodes, api, num_retries, enable_write, pdh_only=False, storage_classes=None):
+ super(MagicDirectory, self).__init__(parent_inode, inodes, api.config, enable_write)
self.api = api
self.num_retries = num_retries
self.pdh_only = pdh_only
@@ -720,7 +736,8 @@ and the directory will appear if it exists.
# If we're the root directory, add an identical by_id subdirectory.
if self.inode == llfuse.ROOT_INODE:
self._entries['by_id'] = self.inodes.add_entry(MagicDirectory(
- self.inode, self.inodes, self.api, self.num_retries, self.pdh_only))
+ self.inode, self.inodes, self.api, self.num_retries, self._enable_write,
+ self.pdh_only))
def __contains__(self, k):
if k in self._entries:
@@ -738,11 +755,11 @@ and the directory will appear if it exists.
if project[u'items_available'] == 0:
return False
e = self.inodes.add_entry(ProjectDirectory(
- self.inode, self.inodes, self.api, self.num_retries,
+ self.inode, self.inodes, self.api, self.num_retries, self._enable_write,
project[u'items'][0], storage_classes=self.storage_classes))
else:
e = self.inodes.add_entry(CollectionDirectory(
- self.inode, self.inodes, self.api, self.num_retries, k))
+ self.inode, self.inodes, self.api, self.num_retries, self._enable_write, k))
if e.update():
if k not in self._entries:
@@ -776,8 +793,8 @@ and the directory will appear if it exists.
class TagsDirectory(Directory):
"""A special directory that contains as subdirectories all tags visible to the user."""
- def __init__(self, parent_inode, inodes, api, num_retries, poll_time=60):
- super(TagsDirectory, self).__init__(parent_inode, inodes, api.config)
+ def __init__(self, parent_inode, inodes, api, num_retries, enable_write, poll_time=60):
+ super(TagsDirectory, self).__init__(parent_inode, inodes, api.config, enable_write)
self.api = api
self.num_retries = num_retries
self._poll = True
@@ -798,7 +815,8 @@ class TagsDirectory(Directory):
self.merge(tags['items']+[{"name": n} for n in self._extra],
lambda i: i['name'],
lambda a, i: a.tag == i['name'],
- lambda i: TagDirectory(self.inode, self.inodes, self.api, self.num_retries, i['name'], poll=self._poll, poll_time=self._poll_time))
+ lambda i: TagDirectory(self.inode, self.inodes, self.api, self.num_retries, self._enable_write,
+ i['name'], poll=self._poll, poll_time=self._poll_time))
@use_counter
@check_update
@@ -832,9 +850,9 @@ class TagDirectory(Directory):
to the user that are tagged with a particular tag.
"""
- def __init__(self, parent_inode, inodes, api, num_retries, tag,
+ def __init__(self, parent_inode, inodes, api, num_retries, enable_write, tag,
poll=False, poll_time=60):
- super(TagDirectory, self).__init__(parent_inode, inodes, api.config)
+ super(TagDirectory, self).__init__(parent_inode, inodes, api.config, enable_write)
self.api = api
self.num_retries = num_retries
self.tag = tag
@@ -856,15 +874,15 @@ class TagDirectory(Directory):
self.merge(taggedcollections['items'],
lambda i: i['head_uuid'],
lambda a, i: a.collection_locator == i['head_uuid'],
- lambda i: CollectionDirectory(self.inode, self.inodes, self.api, self.num_retries, i['head_uuid']))
+ lambda i: CollectionDirectory(self.inode, self.inodes, self.api, self.num_retries, self._enable_write, i['head_uuid']))
class ProjectDirectory(Directory):
"""A special directory that contains the contents of a project."""
- def __init__(self, parent_inode, inodes, api, num_retries, project_object,
+ def __init__(self, parent_inode, inodes, api, num_retries, enable_write, project_object,
poll=True, poll_time=3, storage_classes=None):
- super(ProjectDirectory, self).__init__(parent_inode, inodes, api.config)
+ super(ProjectDirectory, self).__init__(parent_inode, inodes, api.config, enable_write)
self.api = api
self.num_retries = num_retries
self.project_object = project_object
@@ -882,12 +900,13 @@ class ProjectDirectory(Directory):
def createDirectory(self, i):
if collection_uuid_pattern.match(i['uuid']):
- return CollectionDirectory(self.inode, self.inodes, self.api, self.num_retries, i)
+ return CollectionDirectory(self.inode, self.inodes, self.api, self.num_retries, self._enable_write, i)
elif group_uuid_pattern.match(i['uuid']):
- return ProjectDirectory(self.inode, self.inodes, self.api, self.num_retries, i, self._poll, self._poll_time, self.storage_classes)
+ return ProjectDirectory(self.inode, self.inodes, self.api, self.num_retries, self._enable_write,
+ i, self._poll, self._poll_time, self.storage_classes)
elif link_uuid_pattern.match(i['uuid']):
if i['head_kind'] == 'arvados#collection' or portable_data_hash_pattern.match(i['head_uuid']):
- return CollectionDirectory(self.inode, self.inodes, self.api, self.num_retries, i['head_uuid'])
+ return CollectionDirectory(self.inode, self.inodes, self.api, self.num_retries, self._enable_write, i['head_uuid'])
else:
return None
elif uuid_pattern.match(i['uuid']):
@@ -1022,6 +1041,8 @@ class ProjectDirectory(Directory):
@use_counter
@check_update
def writable(self):
+ if not self._enable_write:
+ return False
with llfuse.lock_released:
if not self._current_user:
self._current_user = self.api.users().current().execute(num_retries=self.num_retries)
@@ -1033,6 +1054,9 @@ class ProjectDirectory(Directory):
@use_counter
@check_update
def mkdir(self, name):
+ if not self.writable():
+ raise llfuse.FUSEError(errno.EROFS)
+
try:
with llfuse.lock_released:
c = {
@@ -1053,6 +1077,9 @@ class ProjectDirectory(Directory):
@use_counter
@check_update
def rmdir(self, name):
+ if not self.writable():
+ raise llfuse.FUSEError(errno.EROFS)
+
if name not in self:
raise llfuse.FUSEError(errno.ENOENT)
if not isinstance(self[name], CollectionDirectory):
@@ -1066,6 +1093,9 @@ class ProjectDirectory(Directory):
@use_counter
@check_update
def rename(self, name_old, name_new, src):
+ if not self.writable():
+ raise llfuse.FUSEError(errno.EROFS)
+
if not isinstance(src, ProjectDirectory):
raise llfuse.FUSEError(errno.EPERM)
@@ -1138,9 +1168,9 @@ class ProjectDirectory(Directory):
class SharedDirectory(Directory):
"""A special directory that represents users or groups who have shared projects with me."""
- def __init__(self, parent_inode, inodes, api, num_retries, exclude,
+ def __init__(self, parent_inode, inodes, api, num_retries, enable_write, exclude,
poll=False, poll_time=60, storage_classes=None):
- super(SharedDirectory, self).__init__(parent_inode, inodes, api.config)
+ super(SharedDirectory, self).__init__(parent_inode, inodes, api.config, enable_write)
self.api = api
self.num_retries = num_retries
self.current_user = api.users().current().execute(num_retries=num_retries)
@@ -1231,7 +1261,8 @@ class SharedDirectory(Directory):
self.merge(contents.items(),
lambda i: i[0],
lambda a, i: a.uuid() == i[1]['uuid'],
- lambda i: ProjectDirectory(self.inode, self.inodes, self.api, self.num_retries, i[1], poll=self._poll, poll_time=self._poll_time, storage_classes=self.storage_classes))
+ lambda i: ProjectDirectory(self.inode, self.inodes, self.api, self.num_retries, self._enable_write,
+ i[1], poll=self._poll, poll_time=self._poll_time, storage_classes=self.storage_classes))
except Exception:
_logger.exception("arv-mount shared dir error")
finally:
diff --git a/services/fuse/arvados_fuse/fusefile.py b/services/fuse/arvados_fuse/fusefile.py
index 116b5462b..45d3db16f 100644
--- a/services/fuse/arvados_fuse/fusefile.py
+++ b/services/fuse/arvados_fuse/fusefile.py
@@ -50,11 +50,12 @@ class File(FreshBase):
class FuseArvadosFile(File):
"""Wraps a ArvadosFile."""
- __slots__ = ('arvfile',)
+ __slots__ = ('arvfile', '_enable_write')
- def __init__(self, parent_inode, arvfile, _mtime):
+ def __init__(self, parent_inode, arvfile, _mtime, enable_write):
super(FuseArvadosFile, self).__init__(parent_inode, _mtime)
self.arvfile = arvfile
+ self._enable_write = enable_write
def size(self):
with llfuse.lock_released:
@@ -72,7 +73,7 @@ class FuseArvadosFile(File):
return False
def writable(self):
- return self.arvfile.writable()
+ return self._enable_write and self.arvfile.writable()
def flush(self):
with llfuse.lock_released:
diff --git a/services/fuse/tests/mount_test_base.py b/services/fuse/tests/mount_test_base.py
index fe2ff929d..7cf8aa373 100644
--- a/services/fuse/tests/mount_test_base.py
+++ b/services/fuse/tests/mount_test_base.py
@@ -57,12 +57,15 @@ class MountTestBase(unittest.TestCase):
llfuse.close()
def make_mount(self, root_class, **root_kwargs):
+ enable_write = True
+ if 'enable_write' in root_kwargs:
+ enable_write = root_kwargs.pop('enable_write')
self.operations = fuse.Operations(
os.getuid(), os.getgid(),
api_client=self.api,
- enable_write=True)
+ enable_write=enable_write)
self.operations.inodes.add_entry(root_class(
- llfuse.ROOT_INODE, self.operations.inodes, self.api, 0, **root_kwargs))
+ llfuse.ROOT_INODE, self.operations.inodes, self.api, 0, enable_write, **root_kwargs))
llfuse.init(self.operations, self.mounttmp, [])
self.llfuse_thread = threading.Thread(None, lambda: self._llfuse_main())
self.llfuse_thread.daemon = True
diff --git a/services/fuse/tests/test_mount.py b/services/fuse/tests/test_mount.py
index 157f55e4a..ece316193 100644
--- a/services/fuse/tests/test_mount.py
+++ b/services/fuse/tests/test_mount.py
@@ -1113,7 +1113,7 @@ class MagicDirApiError(FuseMagicTest):
class SanitizeFilenameTest(MountTestBase):
def test_sanitize_filename(self):
- pdir = fuse.ProjectDirectory(1, {}, self.api, 0, project_object=self.api.users().current().execute())
+ pdir = fuse.ProjectDirectory(1, {}, self.api, 0, False, project_object=self.api.users().current().execute())
acceptable = [
"foo.txt",
".foo",
@@ -1293,3 +1293,25 @@ class StorageClassesTest(IntegrationTest):
@staticmethod
def _test_collection_custom_storage_classes(self, coll):
self.assertEqual(storage_classes_desired(coll), ['foo'])
+
+def _readonlyCollectionTestHelper(mounttmp):
+ f = open(os.path.join(mounttmp, 'thing1.txt'), 'rt')
+ # Testing that close() doesn't raise an error.
+ f.close()
+
+class ReadonlyCollectionTest(MountTestBase):
+ def setUp(self):
+ super(ReadonlyCollectionTest, self).setUp()
+ cw = arvados.collection.Collection()
+ with cw.open('thing1.txt', 'wt') as f:
+ f.write("data 1")
+ cw.save_new(owner_uuid=run_test_server.fixture("groups")["aproject"]["uuid"])
+ self.testcollection = cw.api_response()
+
+ def runTest(self):
+ settings = arvados.config.settings().copy()
+ settings["ARVADOS_API_TOKEN"] = run_test_server.fixture("api_client_authorizations")["project_viewer"]["api_token"]
+ self.api = arvados.safeapi.ThreadSafeApiCache(settings)
+ self.make_mount(fuse.CollectionDirectory, collection_record=self.testcollection, enable_write=False)
+
+ self.pool.apply(_readonlyCollectionTestHelper, (self.mounttmp,))
commit 773413b6decf25e4ab669881e00c507aa8a1486f
Author: Tom Clegg <tom at curii.com>
Date: Thu Nov 11 15:47:05 2021 -0500
Merge branch '16817-users-visible-upon-activation'
closes #16817
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/doc/admin/upgrading.html.textile.liquid b/doc/admin/upgrading.html.textile.liquid
index be1103243..dfee7e0b5 100644
--- a/doc/admin/upgrading.html.textile.liquid
+++ b/doc/admin/upgrading.html.textile.liquid
@@ -39,6 +39,10 @@ h2(#main). development main (as of 2021-11-10)
"previous: Upgrading from 2.3.0":#v2_3_0
+h3. Users are visible to other users by default
+
+When a new user is set up (either via @AutoSetupNewUsers@ config or via Workbench admin interface) the user immediately becomes visible to other users. To revert to the previous behavior, where the administrator must add two users to the same group using the Workbench admin interface in order for the users to see each other, change the new @Users.ActivatedUsersAreVisibleToOthers@ config to @false at .
+
h3. Dedicated keepstore process for each container
When Arvados runs a container via @arvados-dispatch-cloud@, the @crunch-run@ supervisor process now brings up its own keepstore server to handle I/O for mounted collections, outputs, and logs. With the default configuration, the keepstore process allocates one 64 MiB block buffer per VCPU requested by the container. For most workloads this will increase throughput, reduce total network traffic, and make it possible to run more containers at once without provisioning additional keepstore nodes to handle the I/O load.
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 9971d3cae..bbdbe6ab9 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -265,6 +265,16 @@ Clusters:
# user agreements. Should only be enabled for development.
NewUsersAreActive: false
+ # Newly activated users (whether set up by an admin or via
+ # AutoSetupNewUsers) immediately become visible to other active
+ # users.
+ #
+ # On a multi-tenant cluster, where the intent is for users to be
+ # invisible to one another unless they have been added to the
+ # same group(s) via Workbench admin interface, change this to
+ # false.
+ ActivatedUsersAreVisibleToOthers: true
+
# The e-mail address of the user you would like to become marked as an admin
# user on their first login.
AutoAdminUserWithEmail: ""
diff --git a/lib/config/export.go b/lib/config/export.go
index f2c15b0ee..1d2ea6c98 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -214,6 +214,7 @@ var whitelist = map[string]bool{
"SystemRootToken": false,
"TLS": false,
"Users": true,
+ "Users.ActivatedUsersAreVisibleToOthers": false,
"Users.AdminNotifierEmailFrom": false,
"Users.AnonymousUserToken": true,
"Users.AutoAdminFirstUser": false,
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 4b4248db6..576eb0c00 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -271,6 +271,16 @@ Clusters:
# user agreements. Should only be enabled for development.
NewUsersAreActive: false
+ # Newly activated users (whether set up by an admin or via
+ # AutoSetupNewUsers) immediately become visible to other active
+ # users.
+ #
+ # On a multi-tenant cluster, where the intent is for users to be
+ # invisible to one another unless they have been added to the
+ # same group(s) via Workbench admin interface, change this to
+ # false.
+ ActivatedUsersAreVisibleToOthers: true
+
# The e-mail address of the user you would like to become marked as an admin
# user on their first login.
AutoAdminUserWithEmail: ""
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 1cd002082..dcb81285c 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -223,6 +223,7 @@ type Cluster struct {
Insecure bool
}
Users struct {
+ ActivatedUsersAreVisibleToOthers bool
AnonymousUserToken string
AdminNotifierEmailFrom string
AutoAdminFirstUser bool
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 366c03e30..febb8ea51 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -234,8 +234,9 @@ SELECT target_uuid, perm_level
name: 'can_read').empty?
# Add can_read link from this user to "all users" which makes this
- # user "invited"
- group_perm = create_user_group_link
+ # user "invited", and (depending on config) a link in the opposite
+ # direction which makes this user visible to other users.
+ group_perms = add_to_all_users_group
# Add git repo
repo_perm = if (!repo_name.nil? || Rails.configuration.Users.AutoSetupNewUsersWithRepository) and !username.nil?
@@ -267,7 +268,7 @@ SELECT target_uuid, perm_level
forget_cached_group_perms
- return [repo_perm, vm_login_perm, group_perm, self].compact
+ return [repo_perm, vm_login_perm, *group_perms, self].compact
end
# delete user signatures, login, repo, and vm perms, and mark as inactive
@@ -728,16 +729,26 @@ SELECT target_uuid, perm_level
login_perm
end
- # add the user to the 'All users' group
- def create_user_group_link
- return (Link.where(tail_uuid: self.uuid,
+ def add_to_all_users_group
+ resp = [Link.where(tail_uuid: self.uuid,
head_uuid: all_users_group_uuid,
link_class: 'permission',
- name: 'can_read').first or
+ name: 'can_read').first ||
Link.create(tail_uuid: self.uuid,
head_uuid: all_users_group_uuid,
link_class: 'permission',
- name: 'can_read'))
+ name: 'can_read')]
+ if Rails.configuration.Users.ActivatedUsersAreVisibleToOthers
+ resp += [Link.where(tail_uuid: all_users_group_uuid,
+ head_uuid: self.uuid,
+ link_class: 'permission',
+ name: 'can_read').first ||
+ Link.create(tail_uuid: all_users_group_uuid,
+ head_uuid: self.uuid,
+ link_class: 'permission',
+ name: 'can_read')]
+ end
+ return resp
end
# Give the special "System group" permission to manage this user and
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 c807a7d6c..ae7b21dec 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -13,6 +13,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
@initial_link_count = Link.count
@vm_uuid = virtual_machines(:testvm).uuid
ActionMailer::Base.deliveries = []
+ Rails.configuration.Users.ActivatedUsersAreVisibleToOthers = false
end
test "activate a user after signing UA" do
diff --git a/services/api/test/unit/permission_test.rb b/services/api/test/unit/permission_test.rb
index 123031b35..128d0ebaa 100644
--- a/services/api/test/unit/permission_test.rb
+++ b/services/api/test/unit/permission_test.rb
@@ -218,6 +218,7 @@ class PermissionTest < ActiveSupport::TestCase
end
test "manager user gets permission to minions' articles via can_manage link" do
+ Rails.configuration.Users.ActivatedUsersAreVisibleToOthers = false
manager = create :active_user, first_name: "Manage", last_name: "Er"
minion = create :active_user, first_name: "Min", last_name: "Ion"
minions_specimen = act_as_user minion do
@@ -314,6 +315,7 @@ class PermissionTest < ActiveSupport::TestCase
end
test "users with bidirectional read permission in group can see each other, but cannot see each other's private articles" do
+ Rails.configuration.Users.ActivatedUsersAreVisibleToOthers = false
a = create :active_user, first_name: "A"
b = create :active_user, first_name: "B"
other = create :active_user, first_name: "OTHER"
diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index c00164c0a..7368d8937 100644
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -447,30 +447,40 @@ class UserTest < ActiveSupport::TestCase
assert_not_allowed { User.new.save }
end
- test "setup new user" do
- set_user_from_auth :admin
+ [true, false].each do |visible|
+ test "setup new user with ActivatedUsersAreVisibleToOthers=#{visible}" do
+ Rails.configuration.Users.ActivatedUsersAreVisibleToOthers = visible
+ set_user_from_auth :admin
- email = 'foo at example.com'
+ email = 'foo at example.com'
- user = User.create ({uuid: 'zzzzz-tpzed-abcdefghijklmno', email: email})
+ user = User.create ({uuid: 'zzzzz-tpzed-abcdefghijklmno', email: email})
- vm = VirtualMachine.create
+ vm = VirtualMachine.create
- response = user.setup(repo_name: 'foo/testrepo',
- vm_uuid: vm.uuid)
+ response = user.setup(repo_name: 'foo/testrepo',
+ vm_uuid: vm.uuid)
- resp_user = find_obj_in_resp response, 'User'
- verify_user resp_user, email
+ resp_user = find_obj_in_resp response, 'User'
+ verify_user resp_user, email
- group_perm = find_obj_in_resp response, 'Link', 'arvados#group'
- verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil
+ group_perm = find_obj_in_resp response, 'Link', 'arvados#group'
+ verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil
- repo_perm = find_obj_in_resp response, 'Link', 'arvados#repository'
- verify_link repo_perm, 'permission', 'can_manage', resp_user[:uuid], nil
+ group_perm2 = find_obj_in_resp response, 'Link', 'arvados#user'
+ if visible
+ verify_link group_perm2, 'permission', 'can_read', groups(:all_users).uuid, nil
+ else
+ assert_nil group_perm2
+ end
- vm_perm = find_obj_in_resp response, 'Link', 'arvados#virtualMachine'
- verify_link vm_perm, 'permission', 'can_login', resp_user[:uuid], vm.uuid
- assert_equal("foo", vm_perm.properties["username"])
+ repo_perm = find_obj_in_resp response, 'Link', 'arvados#repository'
+ verify_link repo_perm, 'permission', 'can_manage', resp_user[:uuid], nil
+
+ vm_perm = find_obj_in_resp response, 'Link', 'arvados#virtualMachine'
+ verify_link vm_perm, 'permission', 'can_login', resp_user[:uuid], vm.uuid
+ assert_equal("foo", vm_perm.properties["username"])
+ end
end
test "setup new user with junk in database" do
@@ -514,6 +524,9 @@ class UserTest < ActiveSupport::TestCase
group_perm = find_obj_in_resp response, 'Link', 'arvados#group'
verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil
+ group_perm2 = find_obj_in_resp response, 'Link', 'arvados#user'
+ verify_link group_perm2, 'permission', 'can_read', groups(:all_users).uuid, nil
+
# invoke setup again with repo_name
response = user.setup(repo_name: 'foo/testrepo')
resp_user = find_obj_in_resp response, 'User', nil
@@ -560,7 +573,7 @@ class UserTest < ActiveSupport::TestCase
break
end
else # looking for a link
- if ArvadosModel::resource_class_for_uuid(x['head_uuid']).kind == head_kind
+ if ArvadosModel::resource_class_for_uuid(x['head_uuid']).andand.kind == head_kind
return_obj = x
break
end
commit 28e35c535b8fd442dce3a286c4503517dc848848
Author: Tom Clegg <tom at curii.com>
Date: Mon Nov 1 14:49:29 2021 -0400
12859: Fix unclosed file descriptors in local filesystem driver.
Temporary file was not being closed/removed in the case where client
disconnection is detected while waiting for the volume-level serialize
lock.
Also, GetDeviceID was leaking one file descriptor per volume at
startup time.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/services/keepstore/unix_volume.go b/services/keepstore/unix_volume.go
index a74616604..f076ccf18 100644
--- a/services/keepstore/unix_volume.go
+++ b/services/keepstore/unix_volume.go
@@ -135,6 +135,7 @@ func (v *UnixVolume) GetDeviceID() string {
if err != nil {
return giveup("opening %q: %s", udir, err)
}
+ defer d.Close()
uuids, err := d.Readdirnames(0)
if err != nil {
return giveup("reading %q: %s", udir, err)
@@ -274,29 +275,25 @@ func (v *UnixVolume) WriteBlock(ctx context.Context, loc string, rdr io.Reader)
return fmt.Errorf("error creating directory %s: %s", bdir, err)
}
- tmpfile, tmperr := v.os.TempFile(bdir, "tmp"+loc)
- if tmperr != nil {
- return fmt.Errorf("TempFile(%s, tmp%s) failed: %s", bdir, loc, tmperr)
- }
-
bpath := v.blockPath(loc)
+ tmpfile, err := v.os.TempFile(bdir, "tmp"+loc)
+ if err != nil {
+ return fmt.Errorf("TempFile(%s, tmp%s) failed: %s", bdir, loc, err)
+ }
+ defer v.os.Remove(tmpfile.Name())
+ defer tmpfile.Close()
- if err := v.lock(ctx); err != nil {
+ if err = v.lock(ctx); err != nil {
return err
}
defer v.unlock()
n, err := io.Copy(tmpfile, rdr)
v.os.stats.TickOutBytes(uint64(n))
if err != nil {
- err = fmt.Errorf("error writing %s: %s", bpath, err)
- tmpfile.Close()
- v.os.Remove(tmpfile.Name())
- return err
+ return fmt.Errorf("error writing %s: %s", bpath, err)
}
- if err := tmpfile.Close(); err != nil {
- err = fmt.Errorf("error closing %s: %s", tmpfile.Name(), err)
- v.os.Remove(tmpfile.Name())
- return err
+ if err = tmpfile.Close(); err != nil {
+ return fmt.Errorf("error closing %s: %s", tmpfile.Name(), err)
}
// ext4 uses a low-precision clock and effectively backdates
// files by up to 10 ms, sometimes across a 1-second boundary,
@@ -307,14 +304,10 @@ func (v *UnixVolume) WriteBlock(ctx context.Context, loc string, rdr io.Reader)
v.os.stats.TickOps("utimes")
v.os.stats.Tick(&v.os.stats.UtimesOps)
if err = os.Chtimes(tmpfile.Name(), ts, ts); err != nil {
- err = fmt.Errorf("error setting timestamps on %s: %s", tmpfile.Name(), err)
- v.os.Remove(tmpfile.Name())
- return err
+ return fmt.Errorf("error setting timestamps on %s: %s", tmpfile.Name(), err)
}
- if err := v.os.Rename(tmpfile.Name(), bpath); err != nil {
- err = fmt.Errorf("error renaming %s to %s: %s", tmpfile.Name(), bpath, err)
- v.os.Remove(tmpfile.Name())
- return err
+ if err = v.os.Rename(tmpfile.Name(), bpath); err != nil {
+ return fmt.Errorf("error renaming %s to %s: %s", tmpfile.Name(), bpath, err)
}
return nil
}
commit 1dc17e4eee5367c7684888c8dcaa6445b576537c
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Tue Nov 2 10:04:48 2021 -0300
18318: Updates the nokogiri dependency on API & WB1.
Addresses https://nvd.nist.gov/vuln/detail/CVE-2021-41098
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/apps/workbench/Gemfile.lock b/apps/workbench/Gemfile.lock
index ab9256a38..13c443096 100644
--- a/apps/workbench/Gemfile.lock
+++ b/apps/workbench/Gemfile.lock
@@ -178,7 +178,7 @@ GEM
mime-types-data (~> 3.2015)
mime-types-data (3.2019.0331)
mini_mime (1.1.0)
- mini_portile2 (2.5.3)
+ mini_portile2 (2.6.1)
minitest (5.10.3)
mocha (1.8.0)
metaclass (~> 0.0.1)
@@ -194,8 +194,8 @@ GEM
net-ssh-gateway (2.0.0)
net-ssh (>= 4.0.0)
nio4r (2.5.7)
- nokogiri (1.11.7)
- mini_portile2 (~> 2.5.0)
+ nokogiri (1.12.5)
+ mini_portile2 (~> 2.6.1)
racc (~> 1.4)
npm-rails (0.2.1)
rails (>= 3.2)
@@ -214,7 +214,7 @@ GEM
multi_json (~> 1.0)
websocket-driver (>= 0.2.0)
public_suffix (4.0.6)
- racc (1.5.2)
+ racc (1.6.0)
rack (2.2.3)
rack-mini-profiler (1.0.2)
rack (>= 1.2.0)
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index 6e149d45a..bdf791153 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -142,7 +142,7 @@ GEM
metaclass (0.0.4)
method_source (1.0.0)
mini_mime (1.1.0)
- mini_portile2 (2.5.3)
+ mini_portile2 (2.6.1)
minitest (5.10.3)
mocha (1.8.0)
metaclass (~> 0.0.1)
@@ -156,8 +156,8 @@ GEM
net-ssh-gateway (2.0.0)
net-ssh (>= 4.0.0)
nio4r (2.5.7)
- nokogiri (1.11.7)
- mini_portile2 (~> 2.5.0)
+ nokogiri (1.12.5)
+ mini_portile2 (~> 2.6.1)
racc (~> 1.4)
oj (3.9.2)
optimist (3.0.0)
@@ -168,7 +168,7 @@ GEM
pg (1.1.4)
power_assert (1.1.4)
public_suffix (4.0.6)
- racc (1.5.2)
+ racc (1.6.0)
rack (2.2.3)
rack-test (1.1.0)
rack (>= 1.0, < 3)
commit bc9d8d1e4caeef8c4b2da02f9a134fc7b57148d7
Author: Ward Vandewege <ward at curii.com>
Date: Fri Oct 29 11:29:34 2021 -0400
18309: remove faraday dependency in the arvados-login-sync gem, instead
depend on the correct version of the arvados-google-api-client
gem.
Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>
diff --git a/services/login-sync/arvados-login-sync.gemspec b/services/login-sync/arvados-login-sync.gemspec
index 826b4607e..f7fe4bc16 100644
--- a/services/login-sync/arvados-login-sync.gemspec
+++ b/services/login-sync/arvados-login-sync.gemspec
@@ -39,8 +39,8 @@ Gem::Specification.new do |s|
s.required_ruby_version = '>= 2.1.0'
s.add_runtime_dependency 'arvados', '>= 1.3.3.20190320201707'
s.add_runtime_dependency 'launchy', '< 2.5'
- # arvados-google-api-client 0.8.7.2 is incompatible with faraday 0.16.2
- s.add_dependency('faraday', '< 0.16')
+ # We need at least version 0.8.7.3, cf. https://dev.arvados.org/issues/15673
+ s.add_dependency('arvados-google-api-client', '>= 0.8.7.3', '< 0.8.9')
# arvados-google-api-client (and thus arvados) gems
# depend on signet, but signet 0.12 is incompatible with ruby 2.3.
s.add_dependency('signet', '< 0.12')
commit ac92153c3aa05af1755b1afe225d3355fcca160d
Author: Ward Vandewege <ward at curii.com>
Date: Thu Oct 28 19:47:05 2021 -0400
Controller test fix.
refs #18183
Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>
diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index f2f88eb25..4cf6a6832 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -831,11 +831,9 @@ func (s *IntegrationSuite) TestListUsers(c *check.C) {
}
c.Check(found, check.Equals, true)
- // Deactivated user can see is_active==false via "get current
- // user" API
+ // Deactivated user no longer has working token
user1, err = conn3.UserGetCurrent(userctx1, arvados.GetOptions{})
- c.Assert(err, check.IsNil)
- c.Check(user1.IsActive, check.Equals, false)
+ c.Assert(err, check.ErrorMatches, `.*401 Unauthorized.*`)
}
func (s *IntegrationSuite) TestSetupUserWithVM(c *check.C) {
commit 73d113eab7fef74d9519be5236e89b48aeb2eab2
Author: Ward Vandewege <ward at curii.com>
Date: Thu Oct 28 14:10:40 2021 -0400
Another rails test fix.
refs #18183
Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>
diff --git a/services/api/test/integration/users_test.rb b/services/api/test/integration/users_test.rb
index 81168e15b..f3e787e3d 100644
--- a/services/api/test/integration/users_test.rb
+++ b/services/api/test/integration/users_test.rb
@@ -434,20 +434,26 @@ class UsersTest < ActionDispatch::IntegrationTest
params: {},
headers: {"HTTP_AUTHORIZATION" => "Bearer #{token}"})
assert_response(:success)
- user = json_response
- assert_equal true, user['is_active']
+ userJSON = json_response
+ assert_equal true, userJSON['is_active']
post("/arvados/v1/users/#{user['uuid']}/unsetup",
params: {},
headers: auth(:admin))
assert_response :success
+ # Need to get a new token, the old one was invalidated by the unsetup call
+ act_as_system_user do
+ ap = ApiClientAuthorization.create!(user: user, api_client_id: 0)
+ token = ap.api_token
+ end
+
get("/arvados/v1/users/#{user['uuid']}",
params: {},
headers: {"HTTP_AUTHORIZATION" => "Bearer #{token}"})
assert_response(:success)
- user = json_response
- assert_equal false, user['is_active']
+ userJSON = json_response
+ assert_equal false, userJSON['is_active']
post("/arvados/v1/users/#{user['uuid']}/activate",
params: {},
commit dcd9dd1ec965190dcece4b8ef3f9379776a309e8
Author: Ward Vandewege <ward at curii.com>
Date: Thu Oct 28 12:48:02 2021 -0400
Update the test db structure.
refs #18183
Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 2f7748335..da9959593 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -3146,6 +3146,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('20210108033940'),
('20210126183521'),
('20210621204455'),
-('20210816191509');
+('20210816191509'),
+('20211027154300');
commit 7dd7f8d08b1bbf4692b2f1678d78047489b6fd37
Author: Ward Vandewege <ward at curii.com>
Date: Wed Oct 27 15:48:54 2021 -0400
18183: add a database migration that deletes tokens and ssh keys that
belong to inactive users.
Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>
diff --git a/services/api/db/migrate/20211027154300_delete_disabled_user_tokens_and_keys.rb b/services/api/db/migrate/20211027154300_delete_disabled_user_tokens_and_keys.rb
new file mode 100644
index 000000000..df3db6f5f
--- /dev/null
+++ b/services/api/db/migrate/20211027154300_delete_disabled_user_tokens_and_keys.rb
@@ -0,0 +1,15 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+class DeleteDisabledUserTokensAndKeys < ActiveRecord::Migration[5.2]
+ def up
+ execute "delete from api_client_authorizations where user_id in (select id from users where is_active ='false' and uuid not like '%-tpzed-anonymouspublic' and uuid not like '%-tpzed-000000000000000')"
+ execute "delete from authorized_keys where owner_uuid in (select uuid from users where is_active ='false' and uuid not like '%-tpzed-anonymouspublic' and uuid not like '%-tpzed-000000000000000')"
+ execute "delete from authorized_keys where authorized_user_uuid in (select uuid from users where is_active ='false' and uuid not like '%-tpzed-anonymouspublic' and uuid not like '%-tpzed-000000000000000')"
+ end
+
+ def down
+ # This migration is not reversible.
+ end
+end
commit a6f94a674bdbb99cc3fb19cff6a7ffbf4c3520ee
Author: Ward Vandewege <ward at curii.com>
Date: Wed Oct 27 15:05:00 2021 -0400
18183: When the user unsetup api endpoint is hit, any tokens owned by
the user should be deleted.
Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 2e862d3ae..366c03e30 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -300,6 +300,12 @@ SELECT target_uuid, perm_level
Link.where(link_class: 'signature',
tail_uuid: self.uuid).destroy_all
+ # delete tokens for this user
+ ApiClientAuthorization.where(user_id: self.id).destroy_all
+ # delete ssh keys for this user
+ AuthorizedKey.where(owner_uuid: self.uuid).destroy_all
+ AuthorizedKey.where(authorized_user_uuid: self.uuid).destroy_all
+
# delete user preferences (including profile)
self.prefs = {}
diff --git a/services/api/test/integration/users_test.rb b/services/api/test/integration/users_test.rb
index b24ddc5a5..81168e15b 100644
--- a/services/api/test/integration/users_test.rb
+++ b/services/api/test/integration/users_test.rb
@@ -198,6 +198,13 @@ class UsersTest < ActionDispatch::IntegrationTest
verify_link_existence created['uuid'], created['email'], true, true, true, true, false
+ # create a token
+ token = act_as_system_user do
+ ApiClientAuthorization.create!(user: User.find_by_uuid(created['uuid']), api_client: ApiClient.all.first).api_token
+ end
+
+ assert_equal 1, ApiClientAuthorization.where(user_id: User.find_by_uuid(created['uuid']).id).size, 'expected token not found'
+
post "/arvados/v1/users/#{created['uuid']}/unsetup", params: {}, headers: auth(:admin)
assert_response :success
@@ -205,6 +212,7 @@ class UsersTest < ActionDispatch::IntegrationTest
created2 = json_response
assert_not_nil created2['uuid'], 'expected uuid for the newly created user'
assert_equal created['uuid'], created2['uuid'], 'expected uuid not found'
+ assert_equal 0, ApiClientAuthorization.where(user_id: User.find_by_uuid(created['uuid']).id).size, 'token should have been deleted by user unsetup'
verify_link_existence created['uuid'], created['email'], false, false, false, false, false
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list