Fix permissions conflict in tmp folder and allow running from root on linux. #23
Conversation
| if (SystemUtils.IS_OS_LINUX) { | ||
| Long uid = new com.sun.security.auth.module.UnixSystem().getUid(); | ||
| if (uid == 0) { | ||
| command.addAll(Arrays.asList("unshare", "-Uu")); |
There was a problem hiding this comment.
Could you please refer me to documentation where the following parameters are mentioned and described what they do? Because I can't find them anywhere in the official documentation, see the attached links.
https://www.postgresql.org/docs/10/app-initdb.html
https://www.postgresql.org/docs/10/app-pg-ctl.html
https://www.postgresql.org/docs/10/app-postgres.html
There was a problem hiding this comment.
looking at this again unshare -U may be all that's needed, going to test
There was a problem hiding this comment.
yeah, that worked, updated PR with changes
There was a problem hiding this comment.
Sorry for the late response. Now I get it. This is not a postgres parameter but a linux command. In that case I am a bit afraid that some linux distributions may not include this command out of the box. Also, when I tested your branch the unshare command was on some docker containers failing with unshare: unshare failed: Operation not permitted error.
So my concerns have increased and I would like to address this problem in some other way. For example, changing the default permissions to make binaries accessible and executable by all users. So, could you please review and test the following pull request #28? Would this solution be sufficient for you?
There was a problem hiding this comment.
Since unshare is only used in cases where postgresql is guaranteed to otherwise error it won't cause regressions or make the situation any worse.
There was a problem hiding this comment.
And what about the unshare: unshare failed: Operation not permitted error? Isn't that worse? :)
There was a problem hiding this comment.
Worse than poestgresql just throwing an error? I mean they are both fatal errors so not really sure how one is worse than the other.
There was a problem hiding this comment.
This could be reworked a bit to add capability checks(to check that there are appropriate permissions and that unshare exists) before actually running unshare I suppose, the main reason I didn't bother was just because this was already a fully fatal error as postgres uses essentially the exact same uid check as I'm doing here. I should note that from my understanding unshare is somewhat similar in regards to how docker namespace isolation works and may not necessarily place nice with it in all cases.
jameshilliard
force-pushed
the
uid-fixes
branch
from
7f8907d
to
a54ca3e
Compare
|
Closed in favor of #28 |
This splits the tmp folders by UID to avoid getting permission errors when running tests from multiple users on the same system. It also adds a workaround so that tests can be run from a root user.