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

build protoc-* containers as nonroot #473

Merged
merged 2 commits into from
Jan 14, 2025
Merged

Conversation

bobcallaway
Copy link
Member

Fixes: #251

This switches the base image for protoc-base from alpine to debian because alpine doesn't support the full range of UID/GID values that debian does.

this allows you to make clean && make all without sudo :)

cpanato
cpanato previously approved these changes Jan 14, 2025
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

nice

woodruffw
woodruffw previously approved these changes Jan 14, 2025
Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

Awesome, thanks @bobcallaway!

@kommendorkapten
Copy link
Member

Running into some issues on mac:

 => ERROR [protoc-builder 4/9] RUN addgroup --gid 20 myuser && adduser --  0.4s
------
 > [protoc-builder 4/9] RUN addgroup --gid 20 myuser && adduser --uid 501 --ingroup myuser --disabled-password myuser:
#11 0.415 addgroup: The GID `20' is already in use.
------
executor failed running [/bin/sh -c addgroup --gid ${GID} myuser && adduser --uid ${UID} --ingroup myuser --disabled-password myuser]: exit code: 1
make: *** [docker-image] Error 1

I'll debug and see what issue I'm facing.

@kommendorkapten
Copy link
Member

kommendorkapten commented Jan 14, 2025

When I worked on this in #251 I never bothered to set the group in the Dockerfile, but passed the group id via the run command. Reverting the group stuff locally works.

I can push the change if you want, but this is what I did:

diff --git a/Makefile b/Makefile
index 093e86a..e5e67f3 100644
--- a/Makefile
+++ b/Makefile
@@ -26,7 +26,7 @@ RUST_ACTION ?= run -p sigstore-protobuf-specs-codegen
 PLATFORM ?= linux/amd64
 UID ?= $(shell id -u)
 GID ?= $(shell id -g)
-DOCKER_BUILD = docker build --platform ${PLATFORM} --build-arg UID=${UID} --build-arg GID=${GID}
+DOCKER_BUILD = docker build --platform ${PLATFORM} --build-arg UID=${UID}
 DOCKER_RUN = docker run --platform ${PLATFORM} --user ${UID}:${GID}

 PROTOS = $(shell find protos/ -iname "*.proto" | sed 's|^|/defs/|')
diff --git a/protoc-builder/Dockerfile.protoc b/protoc-builder/Dockerfile.protoc
index 3595cc4..9ab3f87 100644
--- a/protoc-builder/Dockerfile.protoc
+++ b/protoc-builder/Dockerfile.protoc
@@ -11,12 +11,11 @@ RUN apt-get update && apt-get install -y unzip git

 # Set up user and group to match host we're building the container on
 ARG UID
-ARG GID

-RUN addgroup --gid ${GID} myuser && adduser --uid ${UID} --ingroup myuser --disabled-password myuser
+RUN adduser --uid ${UID} --disabled-password myuser

 # Set permissions on the output directories so the user can write to them
-RUN chown myuser:myuser /protobuf /googleapis
+RUN chown myuser /protobuf /googleapis

 # Switch to user to execute the remaining commands
 USER myuser

Makefile Outdated Show resolved Hide resolved
Co-authored-by: Fredrik Skogman <[email protected]>
Signed-off-by: Bob Callaway <[email protected]>
@bobcallaway bobcallaway dismissed stale reviews from woodruffw and cpanato via 574d51d January 14, 2025 15:34
@bobcallaway
Copy link
Member Author

When I worked on this in #251 I never bothered to set the group in the Dockerfile, but passed the group id via the run command. Reverting the group stuff locally works.

I can push the change if you want, but this is what I did:

diff --git a/Makefile b/Makefile
index 093e86a..e5e67f3 100644
--- a/Makefile
+++ b/Makefile
@@ -26,7 +26,7 @@ RUST_ACTION ?= run -p sigstore-protobuf-specs-codegen
 PLATFORM ?= linux/amd64
 UID ?= $(shell id -u)
 GID ?= $(shell id -g)
-DOCKER_BUILD = docker build --platform ${PLATFORM} --build-arg UID=${UID} --build-arg GID=${GID}
+DOCKER_BUILD = docker build --platform ${PLATFORM} --build-arg UID=${UID}
 DOCKER_RUN = docker run --platform ${PLATFORM} --user ${UID}:${GID}

 PROTOS = $(shell find protos/ -iname "*.proto" | sed 's|^|/defs/|')
diff --git a/protoc-builder/Dockerfile.protoc b/protoc-builder/Dockerfile.protoc
index 3595cc4..9ab3f87 100644
--- a/protoc-builder/Dockerfile.protoc
+++ b/protoc-builder/Dockerfile.protoc
@@ -11,12 +11,11 @@ RUN apt-get update && apt-get install -y unzip git

 # Set up user and group to match host we're building the container on
 ARG UID
-ARG GID

-RUN addgroup --gid ${GID} myuser && adduser --uid ${UID} --ingroup myuser --disabled-password myuser
+RUN adduser --uid ${UID} --disabled-password myuser

 # Set permissions on the output directories so the user can write to them
-RUN chown myuser:myuser /protobuf /googleapis
+RUN chown myuser /protobuf /googleapis

 # Switch to user to execute the remaining commands
 USER myuser

ack, applied changes and this runs clean on my linux box too

Copy link
Member

@kommendorkapten kommendorkapten left a comment

Choose a reason for hiding this comment

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

Tested on macOS, works as expected. Nice job 👏

@bobcallaway bobcallaway merged commit 9de5f28 into sigstore:main Jan 14, 2025
2 checks passed
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.

4 participants