irods / irods_capability_storage_tiering

BSD 3-Clause "New" or "Revised" License
5 stars 10 forks source link

Update violating query examples in README to include metadata permissions check #189

Open korydraughn opened 2 years ago

korydraughn commented 2 years ago

Observed in 4.2.7 plugin.

Data objects can have multiple users with different permissions on them. iRODS requires that users wanting to modify metadata on a data object to have at least WRITE permissions.

This requirement can lead to storage tiering issues. Users will need to tweak the violating query so that it only returns users who have the necessary permissions to modify metadata.

The README needs to be updated to include DATA_ACCESS_TYPE >= '1120' as an additional condition. For example:

imeta set -R fast_resc irods::storage_tiering::query "SELECT DATA_NAME, COLL_NAME, USER_NAME, DATA_REPL_NUM WHERE META_DATA_ATTR_NAME = 'irods::access_time' AND META_DATA_ATTR_VALUE < 'TIME_CHECK_STRING' AND DATA_RESC_ID IN ('10068', '10069') AND DATA_ACCESS_TYPE >= '1120'"
trel commented 2 years ago

Two changes:

  1. update the default constructed query in the code itself to include the permissions check
  2. update the README for admins who want to override the default, to also include the permissions check
alanking commented 7 months ago

Something that crossed my mind that I wanted to write down...

First, this seems at least related to if not a duplicate of https://github.com/irods/irods_capability_storage_tiering/issues/164.

Secondly, the proposed solution would imply that every data object under storage tiering management requires at least one user to have write permissions on it. This is certainly true now due to the issue stated above about modifying metadata requiring write permissions as well as https://github.com/irods/irods/issues/7465.

Would the "real" solution be to have the plugin manage the data migrations as an administrator (suggested here? https://github.com/irods/irods_capability_storage_tiering/issues/164#issuecomment-1496348949)?

Maybe nothing actionable here, just something I thought about.

korydraughn commented 7 months ago

Would the "real" solution be to have the plugin manage the data migrations as an administrator (suggested here? #164 (comment))?

If we're talking about only metadata, then sure. That would improve the implementation. It wouldn't help with data movement unless we decided to allow the permissions to be changed. We decided against changing permissions when this situation appeared in the logical quotas implementation.

trel commented 7 months ago

I think it could be argued that once a data object is in a tier group, then the server / policy should be able to do whatever it needs to do. Both for metadata and possibly(?) even data movement (repl and trim).

korydraughn commented 7 months ago

I agree, but we don't have a safe way of doing that AFAIK.

If all APIs supported the ADMIN_KW, then that would be doable.

trel commented 7 months ago

meta, repl, and trim.... all have it?

korydraughn commented 7 months ago

Discush!

alanking commented 7 months ago

Just to round out the original thought, this seems like a limitation which should not exist in an ideal world. That is, read-only replicas could be under storage tiering management and could be moved around by an admin account as they are accessed by the read-only users.

If we're not in agreement on that point, then the solution proposed by this issue should be the solution and would ensure that only users with write permission can cause storage tiering to take effect (not just act as a workaround for the limitations in the APIs). Even writing that out sounds wrong to me. :)

But, yes, can discuss later and report back here.

trel commented 7 months ago

Agreed - feels like read-only should be sufficient to trigger storage tiering behavior by 'policy/admin'.

korydraughn commented 7 months ago

I agree - read permission should be enough.