Project

General

Profile

Actions

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 over 4 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Story points:
2.0
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 open1 closed)

Task #15268: Review 14874-protected-collection-properties (arvados repo) & 14874-properties-editor-error-handling (wb2 repo)ResolvedPeter Amstutz06/18/2019Actions

Related issues

Related to Arvados - Bug #15407: [WB2] Property keys on collections are getting translated from snake_case to camelCaseResolvedLucas Di Pentima08/19/2019Actions
Actions #1

Updated by Tom Morris about 5 years ago

  • Target version set to To Be Groomed
Actions #2

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 #3

Updated by Tom Clegg about 5 years ago

  • Description updated (diff)
Actions #4

Updated by Tom Clegg about 5 years ago

  • Description updated (diff)
Actions #5

Updated by Tom Clegg about 5 years ago

  • Description updated (diff)
Actions #6

Updated by Tom Clegg about 5 years ago

  • Description updated (diff)
Actions #7

Updated by Tom Clegg about 5 years ago

  • Story points set to 2.0
Actions #8

Updated by Tom Morris about 5 years ago

  • Target version changed from To Be Groomed to Arvados Future Sprints
Actions #9

Updated by Tom Morris almost 5 years ago

  • Target version changed from Arvados Future Sprints to 2019-06-05 Sprint
Actions #10

Updated by Tom Morris almost 5 years ago

  • Assigned To set to Lucas Di Pentima
Actions #11

Updated by Lucas Di Pentima almost 5 years ago

  • Status changed from New to In Progress
Actions #12

Updated by Lucas Di Pentima almost 5 years ago

  • Target version changed from 2019-06-05 Sprint to 2019-06-19 Sprint
Actions #13

Updated by Ward Vandewege almost 5 years ago

  • Release set to 22
Actions #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 #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 #16

Updated by Lucas Di Pentima almost 5 years ago

  • Target version changed from 2019-06-19 Sprint to 2019-07-03 Sprint
Actions #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 #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 #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 #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 #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 #22

Updated by Lucas Di Pentima almost 5 years ago

Updates at 067bb74eb

  • Simplifies example scripts
  • Adds more descriptive comments on documentation
Actions #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 #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 #25

Updated by Lucas Di Pentima almost 5 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100
Actions #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

Also available in: Atom PDF