Feature #14874: [API] Protected collection property -- preserves original project owner through copy/move/modify operations - Arvados

Custom queries

All assigned issues
All issues assigned for grooming
My issues for grooming
My issues for grooming (no story pts)
Prioritized open issues

Actions

Copy link

Feature #14874

closed

[API] Protected collection property -- preserves original project owner through copy/move/modify operations

Added by Tom Clegg about 5 years ago. Updated almost 5 years ago.

Status:

Resolved

Priority:

Normal

Assigned To:

Lucas Di Pentima

Category:

Target version:

2019-07-03 Sprint

Story points:

2.0

Release:

Arvados 2.0 - Early 2020

Release relationship:

Auto

Description

This feature enables a site to use a collection properties entry for tracking ownership/responsibility for file data found in collections ("who was responsible for uploading these files to Arvados?") even after the collections are modified and copied/moved from one project to another.

The feature will be activated by configuring two new API behaviors.

  # In application.yml / cluster configuration file:
  collection_properties:
    responsible_person_uuid: {function: original_owner, protected: true}
    foo_bar: {value: baz, protected: false}

When creating a new collection, if the caller has not provided values for these keys in the properties hash, they are automatically populated with the UUID of the user who owns [the parent of …] the containing project.

When updating a collection, a protected entry in the properties hash cannot be changed by a non-admin user, even the collection's owner. Attempting to do so results in a 403 error.

The default is the empty set (no special behavior).

Supporting this, we should provide some example admin scripts:

List UUIDs/names of collections with no responsible_person_uuid property value
Update the responsible_person_uuid property from nil to X on all collections in the project hierarchy rooted at P, where P is a user UUID or a group UUID.
Update the responsible_person_uuid property from X to Y on all collections where it is X.

We should also confirm that Workbench/Workbench2 preserve the properties hash when copying a collection.

Subtasks 1 (0 open — 1 closed)

Task #15268: Review 14874-protected-collection-properties (arvados repo) & 14874-properties-editor-error-handling (wb2 repo)

Resolved

Peter Amstutz

06/18/2019

Actions

Related issues

Related to Arvados - Bug #15407: [WB2] Property keys on collections are getting translated from snake_case to camelCase

Resolved

Lucas Di Pentima

08/19/2019

Actions

Issue # Delay: days Cancel

History
Notes
Property changes
Associated revisions

Actions

Copy link

Updated by Tom Morris about 5 years ago

Target version set to To Be Groomed

Actions

Copy link

Updated by Tom Clegg about 5 years ago

Subject changed from [API] Preserve collection's original owner "data owner" in subsequent copy/move/modify operations to [API] Protected collection property -- preserves original project owner through copy/move/modify operations
Description updated (diff)

Actions

Copy link

Updated by Tom Clegg about 5 years ago

Description updated (diff)

Actions

Copy link

Updated by Tom Clegg about 5 years ago

Description updated (diff)

Actions

Copy link

Updated by Tom Clegg about 5 years ago

Description updated (diff)

Actions

Copy link

Updated by Tom Clegg about 5 years ago

Description updated (diff)

Actions

Copy link

Updated by Tom Clegg about 5 years ago

Story points set to 2.0

Actions

Copy link

Updated by Tom Morris about 5 years ago

Target version changed from To Be Groomed to Arvados Future Sprints

Actions

Copy link

Updated by Tom Morris almost 5 years ago

Target version changed from Arvados Future Sprints to 2019-06-05 Sprint

Actions

Copy link

#10

Updated by Tom Morris almost 5 years ago

Assigned To set to Lucas Di Pentima

Actions

Copy link

#11

Updated by Lucas Di Pentima almost 5 years ago

Status changed from New to In Progress

Actions

Copy link

#12

Updated by Lucas Di Pentima almost 5 years ago

Target version changed from 2019-06-05 Sprint to 2019-06-19 Sprint

Actions

Copy link

#13

Updated by Ward Vandewege almost 5 years ago

Release set to 22

Actions

Copy link

#14

Updated by Lucas Di Pentima almost 5 years ago

Updates at a981ba700 - branch 14874-protected-collection-properties
Test run: https://ci.curoverse.com/job/developer-run-tests/1321/

Adds Collections.DefaultProperties config section to arvados config
Adds behaviors described on this story (default value & original_owner)
Adds documentation to the Admin section
Fixes workbench's tag editor to handle protected tags errors

Updates at commit:f6129f70 - branch 14874-properties-editor-error-handling (workbench2's repo):

Fixes protected error handling on tag editor
Modifies the way the update data is sent to the API server by only sending the properties field

Pending: Admin example scripts (should those be added as documentation?)

Actions

Copy link

#15

Updated by Peter Amstutz almost 5 years ago

Lucas Di Pentima wrote:

Updates at a981ba700 - branch 14874-protected-collection-properties
Test run: https://ci.curoverse.com/job/developer-run-tests/1321/

Adds Collections.DefaultProperties config section to arvados config

Adds behaviors described on this story (default value & original_owner)

Adds documentation to the Admin section

Fixes workbench's tag editor to handle protected tags errors

Updates at commit:f6129f70 - branch 14874-properties-editor-error-handling (workbench2's repo):

Fixes protected error handling on tag editor

Modifies the way the update data is sent to the API server by only sending the properties field

Pending: Admin example scripts (should those be added as documentation?)

"If there is a need to ~~avoid~~ prevent a non-admin user ~~to modify~~ from modifying a specific property"

The documentation should describe updating config.yml (Collections.DefaultProperties) not application.yml (collection_default_properties).

Instead of "Collections.DefaultProperties" maybe it should be "Collections.ManagedProperties" ?

Still need to play with the feature a bit more.

Actions

Copy link

#16

Updated by Lucas Di Pentima almost 5 years ago

Target version changed from 2019-06-19 Sprint to 2019-07-03 Sprint

Actions

Copy link

#17

Updated by Peter Amstutz almost 5 years ago

Things that I manually tested that worked:

I created a collection in workbench2 and it had the expected initial properties set on it
In Workbench1 I can edit/delete/add non-protected properties, and trying to modify or delete the protected property displays an error.
I tested copying the collection in both workbench1 and workbench2 and the properties were correctly copied.

Things to fix:

I think you mentioned this in chat -- something is turning 'responsible_person_uuid' to 'responsiblePersonUuid'. We should fix that.
In Workbench2, when I try to delete a non-protected property on a collection with a protected property, it will report "Protected property cannot be updated" for the protected property. I get the same error when I try to add a tag.

Actions

Copy link

#18

Updated by Peter Amstutz almost 5 years ago

From chat: we tracked down the use of mapKeys on responses from the API server. The method applies recursively, and has the effect of corrupting the keys of any field containing a JSON object such as properties, mounts, etc containing punctuation such as "_" or "/".

That explains why wb2 reports 'Protected property cannot be updated: responsible_person_uuid' because it was mangled into 'responsiblePersonUuid' so when it attempts to write it back, the protected field is indeed no longer there.

This method should not apply recursively.

Actions

Copy link

#19

Updated by Lucas Di Pentima almost 5 years ago

Updates at e6eb33290 (arvados repo)
Test run: https://ci.curoverse.com/job/developer-run-tests/1335/

Renames Collections.DefaultProperties to Collections.ManagedProperties
Adds example admin scripts to documentation.

Pending: wb2 issue fix.

Actions

Copy link

#20

Updated by Lucas Di Pentima almost 5 years ago

Related to Bug #15407: [WB2] Property keys on collections are getting translated from snake_case to camelCase added

Actions

Copy link

#21

Updated by Peter Amstutz almost 5 years ago

Lucas Di Pentima wrote:

Updates at e6eb33290 (arvados repo)
Test run: https://ci.curoverse.com/job/developer-run-tests/1335/

Renames Collections.DefaultProperties to Collections.ManagedProperties

Adds example admin scripts to documentation.

These look fine. You could simplify them a bit using arvados.util.list_all().

I am generally reluctant to add code to the documentation where the user is expected to directly copy and paste, it is an awkward category that is more than a small example but less than a fully supported admin tool. I don't know if we will have met the customer need until they actually have a chance to try it.

LGTM.

Actions

Copy link

#22

Updated by Lucas Di Pentima almost 5 years ago

Updates at 067bb74eb

Simplifies example scripts
Adds more descriptive comments on documentation

Actions

Copy link

#23

Updated by Peter Amstutz almost 5 years ago

Lucas Di Pentima wrote:

Updates at 067bb74eb

Simplifies example scripts

Adds more descriptive comments on documentation

I know we're trying to wrap this story up but it just occurred to me that new API call(s) "apply default properties" and/or "reset default properties" might be a better approach than reimplementing the "root owner of project" logic in Python. What do you think?

Actions

Copy link

#24

Updated by Peter Amstutz almost 5 years ago

Peter Amstutz wrote:

Lucas Di Pentima wrote:

Updates at 067bb74eb

Simplifies example scripts

Adds more descriptive comments on documentation

I know we're trying to wrap this story up but it just occurred to me that new API call(s) "apply default properties" and/or "reset default properties" might be a better approach than reimplementing the "root owner of project" logic in Python. What do you think?

From discussion in chat, we don't want to add new API design to this ticket, so this LGTM.

Actions

Copy link

#25

Updated by Lucas Di Pentima almost 5 years ago

Status changed from In Progress to Resolved
% Done changed from 0 to 100

Applied in changeset arvados|e16542e3cb4fddf05c407cb013c9e1573eb9c289.

Actions

Copy link

#26

Updated by Lucas Di Pentima almost 5 years ago

Updates at f0475ef9a - branch 14874-post-merge-fixes
Test run: https://ci.curoverse.com/job/developer-run-tests/1345/

Adds Collections.ManagedProperties to Go SDK
Whitelists Collections.ManagedProperties.*.* so they can be accessed on the safe config export API.
Capitalizes ManagedProperties keys (Value, Protected & Function), with docs updates.

Actions

Copy link

Also available in: Atom PDF

Project

General

Profile

Arvados

Custom queries

Feature #14874

[API] Protected collection property -- preserves original project owner through copy/move/modify operations

Updated by Tom Morris about 5 years ago

Updated by Tom Clegg about 5 years ago

Updated by Tom Clegg about 5 years ago

Updated by Tom Clegg about 5 years ago

Updated by Tom Clegg about 5 years ago

Updated by Tom Clegg about 5 years ago

Updated by Tom Clegg about 5 years ago

Updated by Tom Morris about 5 years ago

Updated by Tom Morris almost 5 years ago

Updated by Tom Morris almost 5 years ago

Updated by Lucas Di Pentima almost 5 years ago

Updated by Lucas Di Pentima almost 5 years ago

Updated by Ward Vandewege almost 5 years ago

Updated by Lucas Di Pentima almost 5 years ago

Updated by Peter Amstutz almost 5 years ago

Updated by Lucas Di Pentima almost 5 years ago

Updated by Peter Amstutz almost 5 years ago

Updated by Peter Amstutz almost 5 years ago

Updated by Lucas Di Pentima almost 5 years ago

Updated by Lucas Di Pentima almost 5 years ago

Updated by Peter Amstutz almost 5 years ago

Updated by Lucas Di Pentima almost 5 years ago

Updated by Peter Amstutz almost 5 years ago

Updated by Peter Amstutz almost 5 years ago

Updated by Lucas Di Pentima almost 5 years ago

Updated by Lucas Di Pentima almost 5 years ago