Parameterise runtime image #1478

omBratteng · 2021-12-17T12:29:02Z

Description

Adds a build-arg for setting which docker image to be used as runtime.

Motivation and Context

Enables future implementation of building distroless versions of the oauth2-proxy docker image.

How Has This Been Tested?

Checklist:

My change requires a change to the documentation or CHANGELOG.
I have updated the documentation/CHANGELOG accordingly.
I have created a feature (non-master) branch for my PR.

omBratteng · 2021-12-17T12:58:59Z

I am unfamiliar what Name Service Switch does, and if it works as intended in a distroless image.

Another thing I noticed when doing this PoC, is the line COPY . . is making the layer caching a tad bit ineffective.
Since any changes I do, forces Docker to start from the line, which is not necessary if I e.g. just change a line in the Dockerfile below that line.
Now, the building of the binary is quite fast, so not much time is lost.

JoelSpeed · 2021-12-23T17:11:18Z

Since any changes I do, forces Docker to start from the line, which is not necessary if I e.g. just change a line in the Dockerfile below that line.

There are a couple of things we could do here, either we can add more files to the .dockerignore or we can change the COPY to be more specific. I'm not sure whether we can tell the copy just to include go files recursively or not, that may make things a little better

JoelSpeed · 2021-12-23T17:13:21Z

Dockerfile

@@ -37,12 +37,11 @@ RUN case ${TARGETPLATFORM} in \
    GOARCH=${GOARCH} VERSION=${VERSION} make build && touch jwt_signing_key.pem

 # Copy binary to alpine
-FROM alpine:3.15
+FROM gcr.io/distroless/static-debian11:nonroot


I would expect to publish a separate image so that we maintain the old alpine image for those that rely on it. Is there a way we can parameterise this?

There is, I will take a look and update the PR

I've parameterised the runtime image, so it's possible to adjust which image to be used in the make file.

Which would you prefer to be default, distroless or alpine image?

Just a question, is it necessary to create a separate tag for different platforms? The $(DOCKER_BUILDX_X_PLATFORM) -f Dockerfile -t $(REGISTRY)/oauth2-proxy:${VERSION} . does cover all of them in one

At the moment, we can only push one version right? Perhaps we need a separate target that pushes a set of distroless tags?

For now I think the default should remain as alpine so that it's not a surprising change to users

Dockerfile

omBratteng · 2021-12-23T17:29:18Z

Since any changes I do, forces Docker to start from the line, which is not necessary if I e.g. just change a line in the Dockerfile below that line.

There are a couple of things we could do here, either we can add more files to the .dockerignore or we can change the COPY to be more specific. I'm not sure whether we can tell the copy just to include go files recursively or not, that may make things a little better

If I’m not mistaken, the .dockerignore would not help with layer caching. But I will do some more testing in that regards, could be another PR down the line.

Dockerfile

JoelSpeed

Looks good, lets add a changelog entry and in particular I would like to see an important note denoting the change in the UID:GID for the file

Dockerfile

JoelSpeed · 2021-12-29T19:36:46Z

Dockerfile

+# UID/GID 65532 is also known as nonroot user in distroless image
+USER 65532:65532


Do we expect users will be relying on the existing user number? We probably won't consider this a breaking change but we should definitely add an important note to the changelog for this

I really doubt it would break for anyone. Only if they've added some restrictions to what UIDs can start processes on the host machine.

But a note in the changelog about the change sounds good.

I've added an entry under Important Notes

#1478 Changes the UID and GID of the runtime user to 65532.
Which also is known as nonroot user in distroless images.

omBratteng · 2022-02-24T18:48:13Z

Not exactly sure how you want to release the docker images and the following changes required in the Makefile, so I have changed the PR title to Parameterise runtime image, and is ready for proper review.

Makefile

JoelSpeed · 2022-02-26T12:52:37Z

Makefile

 DOCKER_BUILDX_ARGS ?=
-DOCKER_BUILDX := docker buildx build ${DOCKER_BUILDX_ARGS} --build-arg VERSION=${VERSION}
+DOCKER_BUILDX := docker buildx build ${DOCKER_BUILDX_ARGS} --build-arg VERSION=${VERSION} --build-arg RUNTIME_IMAGE=${DOCKER_BUILD_RUNTIME_IMAGE}
 DOCKER_BUILDX_X_PLATFORM := $(DOCKER_BUILDX) --platform ${DOCKER_BUILD_PLATFORM}
 DOCKER_BUILDX_PUSH := docker buildx build --push ${DOCKER_BUILDX_ARGS} --build-arg VERSION=${VERSION}


The new build args will need to be added here as well else the push won't recognise the cached builds/won't have the correct RUNTIME_IMAGE

I moved it to DOCKER_BUILDX_ARGS, as it's used in both DOCKER_BUILDX and DOCKER_BUILDX_PUSH

Alpine does not have that user, and it cause issues when trying to start the container

JoelSpeed

Apologies for the delay in getting back to this, LGTM

omBratteng mentioned this pull request Dec 17, 2021

Feature Request - Provide scratch based image #1372

Closed

JoelSpeed reviewed Dec 23, 2021

View reviewed changes

omBratteng requested a review from JoelSpeed December 24, 2021 00:58

omBratteng commented Dec 24, 2021

View reviewed changes

Dockerfile Show resolved Hide resolved

JoelSpeed reviewed Dec 29, 2021

View reviewed changes

omBratteng requested a review from JoelSpeed February 24, 2022 18:36

omBratteng changed the title ~~Use distroless debian11 docker image~~ Parameterise runtime image Feb 24, 2022

omBratteng marked this pull request as ready for review February 24, 2022 18:48

omBratteng requested a review from a team as a code owner February 24, 2022 18:48

JoelSpeed reviewed Feb 24, 2022

View reviewed changes

Makefile Show resolved Hide resolved

JoelSpeed reviewed Feb 26, 2022

View reviewed changes

omBratteng requested a review from JoelSpeed March 22, 2022 08:58

omBratteng and others added 7 commits March 22, 2022 10:10

Use distroless debian11 docker image

9ac787f

Add Dockerfile to .dockerignore

2a86a1b

Replace nonroot with the matching UID/GID

423e91f

Alpine does not have that user, and it cause issues when trying to start the container

Use a build arg for setting the runtime image

5eb44e7

Explain why ARG RUNTIME_IMAGE is at the top

9e883b6

Add entry to CHANGELOG

0d50073

Move build-arg to DOCKER_BUILDX_ARGS

41e17e6

JoelSpeed approved these changes Apr 14, 2022

View reviewed changes

JoelSpeed merged commit 2e9c30a into oauth2-proxy:master Apr 14, 2022

omBratteng deleted the distroless-dockerimage branch April 14, 2022 14:10

polarctos mentioned this pull request Sep 5, 2023

Update alpine to 3.18 #2217

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parameterise runtime image #1478

Parameterise runtime image #1478

omBratteng commented Dec 17, 2021 •

edited

omBratteng commented Dec 17, 2021

JoelSpeed commented Dec 23, 2021

JoelSpeed Dec 23, 2021

omBratteng Dec 23, 2021

omBratteng Dec 24, 2021

omBratteng Feb 24, 2022

omBratteng Feb 25, 2022

JoelSpeed Feb 26, 2022

omBratteng commented Dec 23, 2021

JoelSpeed left a comment

JoelSpeed Dec 29, 2021

omBratteng Dec 29, 2021 •

edited

omBratteng Feb 24, 2022

omBratteng commented Feb 24, 2022

JoelSpeed Feb 26, 2022

omBratteng Mar 22, 2022

JoelSpeed left a comment

		# UID/GID 65532 is also known as nonroot user in distroless image
		USER 65532:65532

Parameterise runtime image #1478

Parameterise runtime image #1478

Conversation

omBratteng commented Dec 17, 2021 • edited

Description

Motivation and Context

How Has This Been Tested?

Checklist:

omBratteng commented Dec 17, 2021

JoelSpeed commented Dec 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omBratteng commented Dec 23, 2021

JoelSpeed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omBratteng Dec 29, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omBratteng commented Feb 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoelSpeed left a comment

Choose a reason for hiding this comment

omBratteng commented Dec 17, 2021 •

edited

omBratteng Dec 29, 2021 •

edited