-
Notifications
You must be signed in to change notification settings - Fork 53
Allow running under root on Linux when unshare is available #39
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
Conversation
d7704c3
to
6634cf7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for your effort. The issues related to Docker must be addressed before merging.
src/main/java/io/zonky/test/db/postgres/embedded/EmbeddedPostgres.java
Outdated
Show resolved
Hide resolved
src/main/java/io/zonky/test/db/postgres/embedded/EmbeddedPostgres.java
Outdated
Show resolved
Hide resolved
src/main/java/io/zonky/test/db/postgres/embedded/EmbeddedPostgres.java
Outdated
Show resolved
Hide resolved
src/main/java/io/zonky/test/db/postgres/embedded/EmbeddedPostgres.java
Outdated
Show resolved
Hide resolved
src/main/java/io/zonky/test/db/postgres/embedded/PreparedDbProvider.java
Outdated
Show resolved
Hide resolved
} catch (InterruptedException e) { | ||
throw new RuntimeException(e); | ||
} | ||
if (process.exitValue() == 0 && br.readLine() != "0") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, when I tried it on Docker, it didn't work as expected. The useUnshare
variable was always set to false
. I also tried to run unshare -U id -u
command manually in a docker container but I got unshare failed: Operation not permitted
error. When I investigated the problem further, I came to the conclusion that Docker blocks the unshare
command by default because it has caused a lot of local privilege escalations over time. So the only workaround I've found is to use --privileged
option, which works but it doesn't seem to be an ideal solution.
Could you please investigate it further and find a suitable solution? Because without working it on Docker is this change useless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I hadn't really targeted docker as the main use case for this change so much as throwaway testing VM's where root is the default user or for cases where one wants to test a java application that itself needs to run as root(for example some system utility than needs full privileges) on the host. This check is to ensure that unshare
is only used when it is confirmed to function correctly so that confusing error messages don't get generated.
Basically the unshare
command can be thought of as a lightweight docker alternative for changing the uid. I think --privileged
is needed because this is somewhat like a docker in docker.
6634cf7
to
12e7bd6
Compare
12e7bd6
to
1d968cf
Compare
1d968cf
to
0fc492c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for your contribution. I've polished the code a bit and updated the documentation accordingly.
I've reworked the
unshare
parts from #23 so that we test once on initialization thatunshare
is useable when running as root on Linux. This should avoid any cryptic errors whenunshare
is unavailable.I've also reworked the runner to execute the
postgres
binary directly as opposed to being backgrounded bypg_ctl
. This has the advantage of preventing orphanpostgres
in the event that cleanup fails as thepostgres
process should effectively be a child of the java module which would get killed at the same time as the java module.