Skip to content

fix: don't use adduser and addgroup for docker images #3344

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Aug 1, 2022
Merged

fix: don't use adduser and addgroup for docker images #3344

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
177809c
fix: don't use adduser and addgroup for docker images
deansheather Aug 1, 2022
84a9893
Revert "fix: Remove alternative image architectures until we virtuali…
deansheather Aug 1, 2022
020e8e6
fixup! Revert "fix: Remove alternative image architectures until we v…
deansheather Aug 1, 2022
ab4d58f
fixup! Revert "fix: Remove alternative image architectures until we v…
deansheather Aug 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/release.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ jobs:

# build and (maybe) push Docker images for each architecture
images=()
for arch in amd64; do
for arch in amd64 armv7 arm64; do
img="$(
./scripts/build_docker.sh \
${{ (!github.event.inputs.dry_run && !github.event.inputs.snapshot) && '--push' || '' }} \
Expand Down
16 changes: 10 additions & 6 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
FROM alpine
# This is the multi-arch Dockerfile used for Coder. Since it's multi-arch and
# cross-compiled, it cannot have ANY "RUN" commands. All binaries are built
# using the go toolchain on the host and then copied into the build context by
# scripts/build_docker.sh.
FROM alpine:latest

# LABEL doesn't add any real layers so it's fine (and easier) to do it here than
# in the build script.
Expand All @@ -11,12 +15,12 @@ LABEL \
org.opencontainers.image.version="$CODER_VERSION" \
org.opencontainers.image.licenses="AGPL-3.0"

# Create coder group and user. We cannot use `addgroup` and `adduser` because
# they won't work if we're building the image for a different architecture.
COPY --chown=root:root --chmod=644 group passwd /etc/

# The coder binary is injected by scripts/build_docker.sh.
ADD coder /opt/coder
COPY --chown=coder:coder --chmod=755 coder /opt/coder

# Create coder group and user.
RUN addgroup -g 1000 coder && \
adduser -D -g "" -h /home/coder -G coder -u 1000 -S -s /bin/sh coder
USER coder:coder

ENTRYPOINT [ "/opt/coder", "server" ]
27 changes: 20 additions & 7 deletions scripts/build_docker.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,27 @@ ln -P Dockerfile "$temp_dir/"

cd "$temp_dir"

build_args=(
--platform "$arch"
--build-arg "CODER_VERSION=$version"
--tag "$image_tag"
)

log "--- Building Docker image for $arch ($image_tag)"
docker buildx build "${build_args[@]}" . 1>&2

# Pull the base image, copy the /etc/group and /etc/passwd files out of it, and
# add the coder group and user. We have to do this in a separate step instead of
# using the RUN directive in the Dockerfile because you can't use RUN if you're
# building the image for a different architecture than the host.
docker pull --platform "$arch" alpine:latest

temp_container_id="$(docker create --platform "$arch" alpine:latest)"
docker cp "$temp_container_id":/etc/group ./group
docker cp "$temp_container_id":/etc/passwd ./passwd
docker rm "$temp_container_id"

echo "coder:x:1000:coder" >> ./group
Copy link
Member

Choose a reason for hiding this comment

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

Should we just run the proper commands in the container here instead then copy the files? Seems like it could be simpler.

Copy link
Member Author

Choose a reason for hiding this comment

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

You can't run commands in the container because it's a different architecture. All of the binaries like sh, echo, useradd, groupadd etc. are for whatever architecture the image is.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could just check in the /etc/passwd and /etc/group files to the repo, but then they won't update if alpine changes anything, so I opted for this instead.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I mean you can run them in this container instead of doing the echo

Copy link
Member

Choose a reason for hiding this comment

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

e.g. docker exec $temp_container_id adduser

Copy link
Member Author

Choose a reason for hiding this comment

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

That won't work since the container is for a different architecture. Docker lets you create containers for different architectures but they won't start because of exec format errors. I'm taking advantage of that here to copy the files out of the image for the correct arch.

Copy link
Member

Choose a reason for hiding this comment

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

I 🤦🤦🤦🤦🤦🤦🤦🤦 so hard when I realized how wrong I was.

echo "coder:x:1000:1000::/:/bin/sh" >> ./passwd

docker buildx build \
--platform "$arch" \
--build-arg "CODER_VERSION=$version" \
--tag "$image_tag" \
. 1>&2

cdroot
rm -rf "$temp_dir"
Expand Down