store: allow users in the rkt group to delete images #2961
Conversation
87d3efd to
2474f08
Compare
store/store.go
Outdated
|
|
||
| if !userInDirGroup && uid != 0 { | ||
| return fmt.Errorf("permission denied on %s, user not in the directory group id (%d). Are you root or in the %q group?", path, dirGid, common.RktGroup) | ||
| } |
There was a problem hiding this comment.
I think that, if removed, and permissions are correctly setup it should return an error anyway right?
The check is useful just for better error reporting.
Probably this check, if kept, should be used also in WriteACI, ReadStream etc... (or are there upper checks for this?)
There was a problem hiding this comment.
Right, it's just better UX.
I guess there're two possibilities:
- You are in the rkt group or root: Everything is fine
- You are not in the rkt group: You'll get errors before this function.
I guess we can remove it then. It was there before because it was depending on the user that fetched the image but now all users on the rkt group can do anything with images.
Don't check if we're the owner of the image before removing, all the users in the rkt group should be able to remove any image. If the user removing the image is in the rkt group and permissions are correctly set up (or if they're root), then it's fine, they can remove images. If the user is not in the rkt group or permissions are not correctly set up, they will get errors before this function because they're not allowed to use the CAS directory.
The members of the rkt group can now manipulate images freely.
2474f08 to
3e1ac4a
Compare
|
Updated removing the check altogether. |
|
LGTM here, but I'll give @sgotti another chance to re-check before merging. |
|
@iaguis LGTM. Some notes: Probably users in rkt group should be able to run image store gc (but not treestore gc). But for this I'll add an additional check to skip the treestore gc (and report that why it has been skipped). Should we do this as a follow up? In addition I'd like to let an user choose which kind of gc to run (two different commands or options to image gc to choose which kind of gc) for #2890 (comment) . |
|
I fully agree with #2961 (comment) And yeah, let's do it on a follow up |
This makes sense when we think of the cas as a cache. Users in the rkt
group should be able to manage images.
cc @sgotti
Fixes #2477