This repository has been archived by the owner on Feb 25, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 28
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR can be tested by removing the
|
Closed
By using the same user inside the container as on the host, we avoid issues with the host user lacking the privileges to e.g remove files under `build/`, which are bind-mounted from the host into the container. Before this change, those files would run as the super-user (`id -u` of `0`), which caused the issues mentioned above, specifically on Gitlab, where subsequent test runners would find themselves unable to clean up the `build/` artifacts from prior builds. The unfortunate edge case here is for users that (wisely!) require `sudo` to run `docker` commands, there is no way for the `Makefile` to deduce what their non-privileged user's id is. Those users need to either deal with the super-user (id `0`, commonly `root`) owning the files under `build/`, or to explicitly specify their non-privileged user's id when calling `make` commands: ``` sudo make BUILDER_UID=$(id -u) ```
9d751d0
to
736aa3d
Compare
utACK 736aa3d |
hkjn
added a commit
to hkjn/bitbox-base
that referenced
this pull request
Jun 26, 2019
This reverts commit 0e5efd5. We noticed that 0e5efd5 introduced a regression, which made it necessary for users who require `sudo` for their `docker` commands to also specify `BUILDER_UID=$(id -u)` explicitly on the commandline. (We were aware that this parameter was needed if users wanted to run `docker` commands with `sudo`, and they also wanted the `build/` directory to be owned by their non-privileged user, but it was not known or intended that the build wouldn't work at all with `sudo make`.)
hkjn
commented
Jun 26, 2019
@@ -32,6 +32,8 @@ RUN make -C "tools" | |||
|
|||
# Final | |||
FROM golang:1.12.5-stretch as final | |||
|
|||
ARG builder_uid | |||
RUN adduser --disabled-password --gecos "" --uid ${builder_uid} builder |
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.
This commit actually introduced a regression, where if users have sudo
required to run docker
commands, their build will fail on this line. I've sent #99 to revert this commit for now, as we figure out the best path forward.
hkjn
pushed a commit
that referenced
this pull request
Jun 26, 2019
This reverts commit 0e5efd5. We noticed that 0e5efd5 introduced a regression, which made it necessary for users who require `sudo` for their `docker` commands to also specify `BUILDER_UID=$(id -u)` explicitly on the commandline. (We were aware that this parameter was needed if users wanted to run `docker` commands with `sudo`, and they also wanted the `build/` directory to be owned by their non-privileged user, but it was not known or intended that the build wouldn't work at all with `sudo make`.)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By using the same user inside the container as on the host, we avoid
issues with the host user lacking the privileges to e.g remove files
under
build/
, which are bind-mounted from the host into the container.Before this change, those files would run as the super-user (
id -u
of0
), which caused the issues mentioned above, specifically on Gitlab,where subsequent test runners would find themselves unable to clean up
the
build/
artifacts from prior builds.The unfortunate edge case here is for users that (wisely!) require
sudo
to rundocker
commands, there is no way for theMakefile
to deduce what their non-privileged user's id is. Those users
need to either deal with the super-user (id
0
, commonlyroot
)owning the files under
build/
, or to explicitly specify theirnon-privileged user's id when calling
make
commands: