Skip to content

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

Open
manics wants to merge 1 commit into
ome:masterfrom
manics:omero_server_datadir_managedrepo_mode
Open

Add +t (sticky bit) to default omero_server_datadir_managedrepo_mode#32
manics wants to merge 1 commit into
ome:masterfrom
manics:omero_server_datadir_managedrepo_mode

Conversation

@manics

@manics manics commented May 4, 2018

Copy link
Copy Markdown
Member

After a discussion with @pwalczysko over ome/omero-documentation#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

sbesson commented May 4, 2018

Copy link
Copy Markdown
Member

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 ome/ome-ansible-molecule#10 and review our Ansible ecosystem.

@manics manics changed the title Add +t (sticky bit) to default +omero_server_datadir_managedrepo_mode Add +t (sticky bit) to default omero_server_datadir_managedrepo_mode May 4, 2018
@manics

manics commented May 4, 2018

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown

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

mtbc commented May 7, 2018

Copy link
Copy Markdown
Member

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

@pwalczysko

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

So here are my assumptions when I look at this:

  • In the case of "only user omero-server is a member of group omero and the directory ManagedRepository belongs to user omero-user", then this makes no difference.
  • If we add "plus one inplace importer in group omero", then I think this still doesn't have much effect.
  • i.e. this only becomes relevant in the "two or more inplaces importers in group omero" and the only effect is that each inplace importer cannot delete top-level directories created by other inplace importers. That, however, is an unlikely scenario since I assume that the top-level directories will typically be created by the user omero-server

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

pwalczysko commented May 7, 2018

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

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

@pwalczysko

Copy link
Copy Markdown
Member

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.

@manics manics closed this Jan 14, 2021
@manics manics reopened this Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants