Skip to content

Conversation

flaki
Copy link
Contributor

@flaki flaki commented Aug 4, 2022

Fixes #175

Discord thread

As noted in that issue, on Linux, Docker is run as root and this is how the current runnable is mounted into the Docker filesystem. This results in root-owned build artifacts created in the user's directory that, among other things, cannot be deleted (without sudo).

This does not seem to happen on most other platforms due to specific ways how Docker mounts folders on those platforms.

This fix explores the solution of using docker run -u to run the invoked toolchain with the current user (os.Getuid).

Running as a limited user breaks at least the Rust builder which needed to be updated slightly, and may break other builders, I will do some testing around those and will include any fixes in here, will keep this as a draft PR until then.

Rust builder breakage

For posterity, the reason for this breakage is that while the official upstream Rust builder makes relevant directories (such as the one used by cargo for the registry index) world-writable, our builder image (which still runs as root at this point) initializes the index as root, and the files and folders created by it need to be chmod-ded to allow writing by anyone, otherwise the limited user builds break when the command tries to write into these directories.

@flaki flaki force-pushed the flaki/175-fix-root branch from 2c40513 to 003839f Compare August 4, 2022 21:28
@flaki flaki changed the title Flaki/175 fix root Fix build toolchain creating root-owned files on Linux Aug 4, 2022
@flaki flaki force-pushed the flaki/175-fix-root branch from 003839f to a51f0ae Compare September 13, 2022 13:15
@flaki flaki changed the base branch from vmain to main September 13, 2022 13:15
@flaki
Copy link
Contributor Author

flaki commented Sep 13, 2022

Updated to track main instead of vmain

@flaki flaki force-pushed the flaki/175-fix-root branch from a51f0ae to 06f1dff Compare September 22, 2022 06:27
@flaki flaki marked this pull request as ready for review September 22, 2022 06:28
@flaki
Copy link
Contributor Author

flaki commented Sep 22, 2022

The latest update:

  • Pulls in changes from Update smoketest commands to replace subo create runnable #355, this allows us to use subo/toolchaintest (an updated smoketest) to build & test the builder Docker toolchain images locally
  • Adds a --verbose flag to subo build which enables logging of the Docker toolchain commands for debugging
  • Explicitly sets HOME=/tmp when invoking the Docker toolchains

About that last part:

We are calling the Docker toolchain as a limited user, quite possibly a user that does not even exist inside the container (thus, HOME=/). This is not a problem as long as we only read world-readable files and only write to world-writable directories. Most toolchains (e.g. Swift, Go) will write temporary files (like .cache-s) to the HOME folder and will break when attempting to put files into /, setting HOME to /tmp resolves this issue, and since these are temporary files we don't need them anyway.

toolchaintest is passing for me on Linux but it would be good to have someone have a look at this on OSX and see how it fares.

Copy link
Contributor

@ospencer ospencer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is sweet! Someone with more Docker expertise than me should probably be the one to approve though :)

Comment on lines +161 to +164
uid := os.Getuid()
gid := os.Getgid()

toolchainCmd := fmt.Sprintf("docker run --rm --mount type=bind,source=%s,target=/usr/src/runnable -u %d:%d --env HOME=/tmp %s subo build %s --native --langs %s", b.Context.MountPath, uid, gid, img, b.Context.RelDockerPath, lang)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great and obvious in hindsight!


FROM node:16-buster-slim
WORKDIR /root/runnable
WORKDIR /usr/src/runnable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this path chosen? /usr/* paths are usually root owned as well and I don't see a chown anywhere for it. Does all of the writing happen in the /tmp dir set as $HOME in the Docker invocation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I...honestly don't remember where /usr/src came from, I definitely remember /root being an issue though.

The /tmp as HOME change came later for the Go/Swift images and I don't remember that being needed for other images, but I think this might vary depending on what's the UID/GID of the invoking user.

Comment on lines +30 to +36
docker build . -f builder/docker/assemblyscript/Dockerfile -t suborbital/builder-as:dev
docker build . -f builder/docker/grain/Dockerfile --platform linux/amd64 -t suborbital/builder-gr:dev
docker build . -f builder/docker/javascript/Dockerfile -t suborbital/builder-js:dev
docker build . -f builder/docker/rust/Dockerfile -t suborbital/builder-rs:dev
docker build . -f builder/docker/swift/Dockerfile -t suborbital/builder-swift:dev
docker build . -f builder/docker/tinygo/Dockerfile --platform linux/amd64 --build-arg TARGETARCH=amd64 -t suborbital/builder-tinygo:dev
docker build . -f builder/docker/wat/Dockerfile -t suborbital/builder-wat:dev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have an explicit dev target for local builds (which is weird) but these can be replaced with calls to make like this:

make ver=dev builder/docker/tinygo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I'll change it around!

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.

Docker toolchain build artifacts are created under the root user on Linux
4 participants