enhancement: Change base image from alpine to distroless

[kvanzuijlen]

Description

Motivation and Context

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.

[tuunit]

Ongoing testing:

• Keycloak environment
• Dex environment
• Gitlab environment
• GitHub environment
• GitHub with Feature: Add GitHub groups (orgs/teams) support  #2196
• Kubernetes deployment
• Nginx upstream
• SSL/HTTPs upstream
• Custom CA handling for providers Issue: 2236 - adds an option to append CA certificates #2237

[JoelSpeed]

With a distroless base image, you don't have any system roots of trust for certs. This cause issues for users before because it meant that they either had to mount them into the container, or build from our base onto their own image.

Have we tested that the project is working correctly without having to provide additional CA certs with this distroless base?

[tuunit]

With a distroless base image, you don't have any system roots of trust for certs. This cause issues for users before because it meant that they either had to mount them into the container, or build from our base onto their own image.

Have we tested that the project is working correctly without having to provide additional CA certs with this distroless base?

That was exactly my concern but maybe we can copy the alpine trust store from the build container into the distroless container 🤔

This change definitely cannot be done lightly and needs proper testing.

[kvanzuijlen]

@JoelSpeed not sure what you mean. The base image comes with ca-certificates, is that what you're looking for? I only have been able to test it with Google Cloud auth and that worked perfectly fine. Decided to open it as a draft so that @tuunit can do some testing as well and so that I can do some testing in the future as well.

@tuunit Imo you can leave the Github environment for me to test as I also have ready access to it as well (as well as with orgs/teams testing)

[JoelSpeed]

It's quite possible when we tried this before it was scratch rather than distroless, which would possibly explain the difference. But he's, the system ca certificates is what I want to make sure we continue to provide

[kvanzuijlen]

This change definitely cannot be done lightly and needs proper testing.

100%, also since some people maybe rely on some commands to be available for i.e. healthcheck probes.

It's quite possible when we tried this before it was scratch rather than distroless, which would possibly explain the difference. But he's, the system ca certificates is what I want to make sure we continue to provide

Those are part of distroless base: see https://github.com/GoogleContainerTools/distroless/tree/main/base

[tuunit]

This change definitely cannot be done lightly and needs proper testing.

100%, also since some people maybe rely on some commands to be available for i.e. healthcheck probes.

It's quite possible when we tried this before it was scratch rather than distroless, which would possibly explain the difference. But he's, the system ca certificates is what I want to make sure we continue to provide

Those are part of distroless base: see https://github.com/GoogleContainerTools/distroless/tree/main/base

There is also some documentation / examples on how to for example include the java trust stores CAs in the distroless images:

https://github.com/GoogleContainerTools/distroless/blob/main/cacerts/jksutil/BUILD

[tuunit]

The documentation mentions the follow image should be used for statically linked binaries like go:

Statically compiled applications (Go) that do not require libc can use the gcr.io/distroless/static image, which contains:

ca-certificates
A /etc/passwd entry for a root user
A /tmp directory
tzdata

https://github.com/GoogleContainerTools/distroless/blob/main/base/README.md

That is why it was working for you with your Goolge test because the static image you used has the most common ca-certificates included.

@kvanzuijlen

I missed that you already shared the same url in your last comment.

[kvanzuijlen]

The documentation mentions the follow image should be used for statically linked binaries like go:

I'm not sure if we need libssl though. If that's not necessary then probably distroless/static will work as well.

[tuunit]

@JoelSpeed not sure what you mean. The base image comes with ca-certificates, is that what you're looking for? I only have been able to test it with Google Cloud auth and that worked perfectly fine. Decided to open it as a draft so that @tuunit can do some testing as well and so that I can do some testing in the future as well.

@tuunit Imo you can leave the Github environment for me to test as I also have ready access to it as well (as well as with orgs/teams testing)

As we use the GitHub feature branch in one of my projects at work, I'll test it anyway.

[tuunit]

Ongoing testing:

• Keycloak environment
• Dex environment
• Gitlab environment
• GitHub environment
• GitHub with Feature: Add GitHub groups (orgs/teams) support  #2196
• Custom CA handling Issue: 2236 - adds an option to append CA certificates #2237
• Kubernetes deployment
• Nginx upstream

Everything seems to be working fine with the distroless image. I'll do some more testing in the upcoming does.

I would suggest when all tests are successful, that we switch to the distroless base image for the default tags and additionally offer alpine as a custom tag v7.5.1-alpine3 for debugging and testing purposes. This is how a lot of projects do it. eg. https://hub.docker.com/_/nginx

[JoelSpeed]

Where are we at with testing this one?

[tuunit]

I only need to test that the custom CAs work. Other than that everything looks good to go.

[tuunit]

Ongoing testing:

• Keycloak environment
• Dex environment
• Gitlab environment
• GitHub environment
• GitHub with Feature: Add GitHub groups (orgs/teams) support  #2196
• Kubernetes deployment
• Nginx upstream
• SSL/HTTPs upstream
• Custom CA handling for providers Issue: 2236 - adds an option to append CA certificates #2237

@kvanzuijlen @JoelSpeed

Full testing done. I even created a custom trust authority to be able to create rootCA file and test #2237. Fully working within alpine and distroless. No breaking changes as far as I can tell.

Last question: Are we going to provide an official alpine image as well for debugging purposes?

[tuunit]

I'm going to open another PR for a complete overhaul of the local environment setup files soon. As I had to update a lot of them anyway.

[JoelSpeed]

I'm open to providing a <tag>-alpine image as well, but lets just include that one and not the myriad of specific architecture tags, use the multi-arch image style

[tuunit]

LGTM

[MattiasAng]

@kvanzuijlen @tuunit Any update on getting this merged?

[tuunit]

@kvanzuijlen testing on my part is done.

ToDos:

• Resolve merge conflict of backmerge
• Nightly build should use the multi arch alpine image base
• Changelog entry

[tuunit]

Maybe we should mention both image versions in the readme and docs 🤔 To clarify that we use distroless as the default base image and that we offer the alpine tag for debugging purposes.

[kvanzuijlen]

Hi @tuunit,

Thx for the todo, makes it easier for me to track what has to happen!

• Resolve merge conflict of backmerge

I see no merge conflict, just an outdated branch. Will resolve that while writing the changelog entry and updating the README.

• Nightly build should use the multi arch alpine image base

Right now it has both images for the nightly build. Any reason why we shouldn't provide both images? I think it's easier to understand if we keep it the same as the regular images.

• Changelog entry

I'll write a changelog entry and add a section to the README

[tuunit]

@kvanzuijlen @JoelSpeed what do you think about this?

[tuunit]

I was thinking maybe instead of -alpine we should call it -debug. That's how fluenbit does it: https://docs.fluentbit.io/manual/installation/docker

Suggested change

	$(DOCKER_BUILDX_X_PLATFORM_ALPINE) -t $(REGISTRY)/$(REPOSITORY):latest-alpine -t $(REGISTRY)/$(REPOSITORY):${VERSION}-alpine .
	$(DOCKER_BUILDX_X_PLATFORM_ALPINE) -t $(REGISTRY)/$(REPOSITORY):latest-debug-t $(REGISTRY)/$(REPOSITORY):${VERSION}-debug .

[kvanzuijlen]

I"m personally more a fan of -alpine, because it could also just be a conscious decision to work exclusively with alpine. It's also not an image that includes any extra debug tooling.

Since (imo) this is still a perfectly fine image for use in (regular) production, I think the -alpine is a better fit.

[JoelSpeed]

I'm a plus one for keeping it as alpine, I agree that calling it debug implies something more, does a debug build mean we didn't strip the symbols when compiling the binary for example?

[JoelSpeed]

LGTM if we think we are ready to merge