ome / ansible-role-omero-server

Installs and configures OMERO.server
https://galaxy.ansible.com/ome/omero_server/
BSD 2-Clause "Simplified" License
4 stars 14 forks source link

Add +t (sticky bit) to default omero_server_datadir_managedrepo_mode #32

Open manics opened 6 years ago

manics commented 6 years ago

After a discussion with @pwalczysko over https://github.com/openmicroscopy/ome-documentation/pull/1873 we realised the current outreach playbook has the +t permission on the managed repo: https://github.com/openmicroscopy/prod-playbooks/blob/51ddfa88c52248e2affa4300eb73df488be99efd/training-server.yml#L218

It sounds like this may be a good idea but could do with some input from @rleigh-codelibre @mtbc

Note this requires Ansible 2.4 so travis will fail with bad symbolic permission for mode: +t, though we can discuss this change as part of https://trello.com/c/SsE1sGqR/184-in-place-import-doc-is-unclear

sbesson commented 6 years ago

Re the Ansible 2.4 minimal requirement, this might be the moment to start planning what it would involve across the board. Step 1 might be to come back to https://github.com/openmicroscopy/ome-ansible-molecule-dependencies/pull/10 and review our Ansible ecosystem.

manics commented 6 years ago

The biggest barrier is rewriting the molecule tests for molecule 2.x, all the roles used by the IDR (which should cover most of our galaxy roles) work fine with Ansible 2.4.

rleigh-codelibre commented 6 years ago

It might be worth for checking if there are any complications when it comes to deleting files, particularly for e.g. in-place imports or any situation where the user performing the deletion is not the owner of the directory with the sticky bit, and/or the owner of the content to be deleted in this directory.

mtbc commented 6 years ago

It's a plausible change but probably also requires opinion from @joshmoore as I'm really not sure.

pwalczysko commented 6 years ago

any situation where the user performing the deletion is not the owner of the directory with the sticky bit, and/or the owner of the content to be deleted in this directory.

So this is a point of confusion for me. I would actually say that the deletion only matters in the sense whether or not omero can delete things. In wherever I look, both the directories and the files in them are always owned by omero. And yes, from OMERO I can delete those without problem. Maybe there is a question of whether or not the clean scripts would work as well. But how could they not, since again, it is omero who is executing them ?

I understand that we constantly worry about who can do this and that diractly manipulating the ManagedRepo dir, but I think I have proven that this is safe already.

Where am I making a mistake in my musing above ?

joshmoore commented 6 years ago

So here are my assumptions when I look at this:

i.e. this is safe but doesn't necessarily enable much. We'd need to see how to have the next few levels of directories created in the same fashion.

I haven't thought through cases where ManagedRepository doesn't belong to omero-server, but likely we don't want to support that.

pwalczysko commented 6 years ago

I have started on a testing matrix as encouraged by @manics . Please let me know if you find that testing sufficient.

@joshmoore Seeing the columns L and M of this testing matrix, does it not disprove your assumption about non-propagation of perms into lower level dirs ?

We'd need to see how to have the next few levels of directories created in the same fashion.

pwalczysko commented 6 years ago

Yes, but now I have something a bit worrying. See the cell P5 in https://docs.google.com/spreadsheets/d/1m27kar8CzCFJEyvQprkvsANUQrmC9b5TYMzacYD9b98/edit#gid=0 It looks like there is something not right. Is it possible that the flaw highlighted in cell C5 is also there when the other setup of in-place importer is facilitated, as mentioned in the doc ? (@joshmoore )

joshmoore commented 6 years ago

@pwalczysko : I agree with your spreadsheet that there is at least one other dimension to consider: fresh repo vs. re-used repo.

pwalczysko commented 6 years ago

Analysing the situation with @manics and @mtbc we concluded that the problem noticed in the cell P5 in https://docs.google.com/spreadsheets/d/1m27kar8CzCFJEyvQprkvsANUQrmC9b5TYMzacYD9b98/edit#gid=0 is an OMERO Bug which is not specific to this setup, but to any in-place import setup.