Skip to content
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

Add a uid/gid flag to spawn a container with non-zero uid/gid. #28

Merged
merged 7 commits into from
Apr 20, 2021

Conversation

maleadt
Copy link
Contributor

@maleadt maleadt commented Apr 19, 2021

Turns out tar behaves differently when id==0 -- habitat-sh/builder#365 (comment), which doesn't work in the single-user containers we spawn (breaking BinaryProvider) -- so add an option to create a user map that uses a different uid/gid.

Fixes #24

@maleadt
Copy link
Contributor Author

maleadt commented Apr 19, 2021

/home/runner/.julia/artifacts/b75badd28f02d9a4824aa829a7537d1c6f39fb13/bin/sandbox: unrecognized option '--uid'

Doesn't look like the build local sandbox CI is actually using the local sandbox. Presumable Pkg.test uses a different environment that ignores the LocalPreferences.toml?

@giordano
Copy link
Member

Presumable Pkg.test uses a different environment that ignores the LocalPreferences.toml?

Yes: JuliaLang/Pkg.jl#2500. You should have a separate test/Project.toml like FFTW.jl

@maleadt maleadt force-pushed the non_root branch 2 times, most recently from dab6d48 to 23a479a Compare April 19, 2021 10:12
test/Project.toml Outdated Show resolved Hide resolved
@maleadt
Copy link
Contributor Author

maleadt commented Apr 19, 2021

Doesn't seem to help here.

@giordano
Copy link
Member

Maybe this is relevant?

@maleadt
Copy link
Contributor Author

maleadt commented Apr 19, 2021

Ah, yes, so that's setting a global preferences. Seems pretty hacky, at this point we could just run julia --project test/runtests.jl and just not use Pkg.test...

@maleadt maleadt force-pushed the non_root branch 2 times, most recently from eacc0dd to c9f9a4c Compare April 19, 2021 10:30
@maleadt
Copy link
Contributor Author

maleadt commented Apr 19, 2021

OK, only legit failures now.

test/runtests.jl Outdated Show resolved Hide resolved
@maleadt
Copy link
Contributor Author

maleadt commented Apr 19, 2021

OK, all done. How do we want to proceed here? Merge with failures, rebuild UserNSSandbox_jll, and re-run CI to verify?

@maleadt
Copy link
Contributor Author

maleadt commented Apr 19, 2021

@staticfloat, re. #24 (comment), I take it you're opposed to this? I don't see an alternative though, su or similar doesn't work in the userns, and id=0 really does make software behave differently.

src/Docker.jl Show resolved Hide resolved
src/SandboxConfig.jl Show resolved Hide resolved
src/Docker.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
@staticfloat
Copy link
Collaborator

I take it you're opposed to this?

Actually, I didn't really think that the patch to userns_sandbox.c would be so small, so I'm totally open to this change. In fact, it may be quite useful for BB to build things as non-root!

@giordano
Copy link
Member

it may be quite useful for BB to build things as non-root!

I thought having a single user helped with mapping permissions 🤔

@staticfloat
Copy link
Collaborator

Indeed it does, but this nicely dodges the problem by keeping things as a single-user system, it just allows using a UID that is nonzero. I think we will want to have the BB sandbox default to using the same numeric ID as the originating user, which will even allow us to avoid the chown calls we have to make after the Docker runner finishes.

@staticfloat
Copy link
Collaborator

I'm quite happy with this, with the one exception that using Tar.jl was one part of the hope that I had that this package would be able to work on Windows without further binary dependencies. :(

I guess we can work towards that in the future, in the Tar.jl repo.

@staticfloat staticfloat merged commit 16550b5 into JuliaContainerization:main Apr 20, 2021
@maleadt maleadt deleted the non_root branch April 21, 2021 05:37
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.

Feature request: non-root user inside the Docker container?
3 participants