Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
korydraughn opened this issue Apr 12, 2022 · 10 comments
Labels
bug documentation updates for readme and other potential documentation

Comments

@korydraughn
Copy link
Collaborator

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'"
@korydraughn korydraughn added bug documentation updates for readme and other potential documentation labels Apr 12, 2022
@trel
Copy link
Member

trel commented Apr 12, 2022

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 alanking modified the milestones: 4.3.1.1, 4.3.1.2 Dec 15, 2023
@alanking
Copy link
Contributor

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

First, this seems at least related to if not a duplicate of #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 irods/irods#7465.

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

Maybe nothing actionable here, just something I thought about.

@korydraughn
Copy link
Collaborator Author

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
Copy link
Member

trel commented Jan 22, 2024

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
Copy link
Collaborator Author

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
Copy link
Member

trel commented Jan 22, 2024

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

@korydraughn
Copy link
Collaborator Author

Discush!

@alanking
Copy link
Contributor

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
Copy link
Member

trel commented Jan 22, 2024

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

@korydraughn
Copy link
Collaborator Author

I agree - read permission should be enough.

@korydraughn korydraughn removed this from the 4.3.1.2 milestone Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug documentation updates for readme and other potential documentation
Development

No branches or pull requests

3 participants