Arvados

• If log entry L is about object X (L.object_uuid X.uuid), and
• If X is in project/group P (X.owner_uuid P.uuid) when L was created, and
• User U can read P, then
• U can read L.

This makes logs visible to the appropriate users in common cases.

However, it also comes with some subtle behavior that might not be desirable. Say we have another project P2 and another user U2:

• U cannot read X (as expected)
• U cannot read new logs about X (as expected)
• U2 can read X (as expected)
• U2 can read new logs about X (as expected)
• U can still read logs about X that were created before time T (surprise?)
• U2 cannot read logs about X that were created before time T (surprise?)

• Container request CR is in project P
• User U can read project P
• U can read logs about CR
• Dispatcher runs as user D (D != U)
• Dispatcher creates logs about container C
• C is owned by system user
• Logs about C are owned by D
• Only D and admins can see the logs created by dispatcher

The result is that live container logs are only visible to admins.

Proposed approach: log entries for container C are effectively (e.g., by eventbus) broadcast to all container requests referencing that container.

• When a websocket client requests log entries for a container request with uuid=CR, websocket should send events with object_uuid = CR and events with object_uuid in (select container_uuid from container_requests where uuid=CR)
• Ditto REST client

This would be less awkward if our event bus were UUID-channel-oriented and didn't support subscribing to "events matching filters XYZ regardless of object_uuid"

Proposed approach 2:

Implement Container.readable_by() such that container C is readable if some container CR is readable and has CR.container_uuid==C.uuid.

The normal permission paths (for non-admins) are irrelevant for Containers.

9799-nonadmin-logs @ 4d27755 → https://ci.curoverse.com/job/developer-test-job/165/

Now at dcf7f7d, https://ci.curoverse.com/job/developer-test-job/172/

Some questions:

• At apps/workbench/app/models/proxy_work_unit.rb, method live_log_lines(limit): Couldn’t the API server query be improved to avoid the “select” block in Workbench’s ruby code? delegating that filtering to the Postgresql or API server, saving some bandwidth?

• At services/api/app/models/arvados_model.rb, line 201:

    # Collect the UUIDs of all groups readable by any of the
    # authorized users. If one of these (or the UUID of one of the
    # authorized users themselves) is an object's owner_uuid, that
    # object is readable.
    owner_uuids = user_uuids + users_list.flat_map { |u| u.groups_i_can(:read) }

Is there a possibility of owner_uuid having duplicated uuids coming from groups_i_can(:read)?

• At services/api/app/models/arvados_model.rb, mehod readable_by(*users_list): Is there a style standard for method declarations? For example, readable_by() declaration in arvados_model uses parenthesis, but the same method on container and log models, don’t.

Lucas Di Pentima wrote:

• At apps/workbench/app/models/proxy_work_unit.rb, method live_log_lines(limit): Couldn’t the API server query be improved to avoid the “select” block in Workbench’s ruby code? delegating that filtering to the Postgresql or API server, saving some bandwidth?

• API doesn't yet have good support for filtering on serialized fields
• We won't want this filter for very long -- in #9679 we want Workbench to do/say something about other events, like "state changed from Queued to Locked", even if they don't come with their own properties["text"]
• In practice I don't think there will be a lot of discarded entries

Is there a possibility of owner_uuid having duplicated uuids coming from groups_i_can(:read)?

Yes -- added a "uniq" here to save a bit of extra noise in the queries (is that what you mean?)

• At services/api/app/models/arvados_model.rb, mehod readable_by(*users_list): Is there a style standard for method declarations? For example, readable_by() declaration in arvados_model uses parenthesis, but the same method on container and log models, don’t.

Added parens, and added link to https://github.com/bbatsov/ruby-style-guide ("Use def with parentheses when there are parameters") .

LGTM, please merge.