Skip to content
This repository has been archived by the owner on Dec 7, 2020. It is now read-only.

Port over Gatekeeper's Dockerfile and kube YAMLs (#609) #638

Merged
merged 1 commit into from
Jun 15, 2020

Conversation

ASzc
Copy link
Contributor

@ASzc ASzc commented Jun 8, 2020

I ended up rewriting a good portion of the Dockerfile. It now uses a multi-stage build. It can accept source code to build, or unpack a premade binary.

Currently I've only tested it with local sources. Testing it with a tar of sources or a tar of binaries can be done when a release is available and attached to the repo releases tab here on GitHub.

I also updated the Makefile

Fixes #609
Fixes #541

Dockerfile Outdated
# Actual image
#

FROM registry.access.redhat.com/ubi8/ubi-minimal:8.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this base image (this project is not under RedHat anymore :-) )? Why static build and FROM scratch can't be used? It will remove dependency + minimize attack surface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding why UBI, it'd be something I'd trust over docker-conventional busybox or alpine, and offers the same minimality as them without involving unconventional libc's.

Regarding why any parent image at all, instead of scratch, I didn't consider that. I assumed the container, as it was written for Gatekeeper, does the user setup for a reason. If that's not the case, then maybe scratch is viable.

If we were going to switch to scratch, I'd need to double check for guidance on scratch vs UBI.

Choose a reason for hiding this comment

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

@jangaraj from your experience what is the advantage of using scratch instead of UBI?

Copy link
Contributor

@jangaraj jangaraj Jun 9, 2020

Choose a reason for hiding this comment

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

@abstractj The security. There is no chance that any security scanner will report vulnerable/outdated packages in the image (e.g. curl, openssl, apr, ...). An administrator with image based on the SCRATCH will just watch for Golang security fixes. But regular image builds and container deployments will be needed with UBI image to have safe container running - for example 4 months old UBI image is vulnerable - https://catalog.redhat.com/software/containers/ubi8/ubi-minimal/5c359a62bed8bd75a2c3fba8?architecture=amd64&tag=8.1-398

Some Docker tools may have a problem with the scratch base image, e.g. CloudFoundry needs:

The Docker image must contain an /etc/passwd file with an entry for the root user. In addition, the home directory and the shell for that root user must be present in the image file system.

I made custom /etc/passwd and "fake" root shell script, which is able to execute only louketo binary, so image is CloudFoundry compatible even with scratch base image.

@abstractj abstractj self-assigned this Jun 8, 2020
@abstractj abstractj added this to the 1.0.0 milestone Jun 9, 2020
@abstractj abstractj changed the title Issue-609 Port over Gatekeeper's Dockerfile and kube YAMLs Port over Gatekeeper's Dockerfile and kube YAMLs (#609) Jun 9, 2020
@abstractj
Copy link

@ASzc I will give your changes a try, but looking at the code I'd suggest to also revisit our Makefile:

REGISTRY=docker.io
.PHONY: test authors changelog build docker static release lint cover vet
docker-build:
        docker run --rm \
docker-test:
        @echo "--> Running the docker test"
        docker run --rm -ti -p 3000:3000 \
docker-release:
        @$(MAKE) docker
        @docker push ${REGISTRY}/${AUTHOR}/${NAME}:${VERSION}
docker:
        @echo "--> Building the docker image"
        docker build -t ${REGISTRY}/${AUTHOR}/${NAME}:${VERSION} .

We have several targets into the Makefile for Docker. I believe it's just the matter of decide if we're going to remove it or update.

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated
# Actual image
#

FROM registry.access.redhat.com/ubi8/ubi-minimal:8.1

Choose a reason for hiding this comment

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

@jangaraj from your experience what is the advantage of using scratch instead of UBI?

@abstractj

This comment has been minimized.

@ASzc
Copy link
Contributor Author

ASzc commented Jun 9, 2020

scratch would certainly make it small.

The UBI option can be made much smaller though. I'm going to push an update that moves to :8.2 and foregoes any updates to RHEL/UBI in our image build, and thereby brings the virtual size down to 162 MB (non-virtual size 162 - 146 = 16 MB). Caveat to this approach is that the image cache must be cleared (or otherwise the latest 8.2 image must be fetched) before an official release build is performed.

Copy link

@abstractj abstractj left a comment

Choose a reason for hiding this comment

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

@ASzc I got the opportunity to talk with @stianst about this today. For now let's stick with Alpine, see the comparison:

docker.io/library/alpine                          3.11          5.88 MB
registry.access.redhat.com/ubi8/ubi-minimal       8.2           146 MB

In the future when we start the discussions about the product we can revisit the ideas here.

Dockerfile Show resolved Hide resolved
@ASzc
Copy link
Contributor Author

ASzc commented Jun 10, 2020

@abstractj Alpine is very much not supportable. Looking at base layer size is the wrong metric. I'll implement it, but under objection.

@abstractj
Copy link

abstractj commented Jun 10, 2020

@abstractj Alpine is very much not supportable. Looking at base layer size is the wrong metric. I'll implement it, but under objection.

@ASzc I understand. If we can make UBI images smaller than images with Alpine or Scratch, I don't have any problems about using it. Otherwise, let's stick with one of them.

There's no reason why someone would pick a 7x bigger image. Eclipse Che had the same discussion a long time ago and they ended up with 2 Dockerfiles. See:

[1] - eclipse-che/che#14113 (comment)
[2] - eclipse-che/che-plugin-registry#205

@ASzc
Copy link
Contributor Author

ASzc commented Jun 10, 2020

That's the thing: it's not a x7 bigger image. By design, the parent image is reusable through multiple images that are based on it. All this alpine stuff is based on the general community's misunderstanding of how container images work, due to the bad design of the docker images command reporting virtual size instead of actual size.

I can see why it's a nice marketing thing to be able to say "our image is small!", but I hate pandering to that misinformation.

@ASzc
Copy link
Contributor Author

ASzc commented Jun 10, 2020

Pushed a new commit, with alpine FROM, and a COPY for the templates directory.

@abstractj
Copy link

abstractj commented Jun 10, 2020

That's the thing: it's not a x7 bigger image. By design, the parent image is reusable through multiple images that are based on it. All this alpine stuff is based on the general community's misunderstanding of how container images work, due to the bad design of the docker images command reporting virtual size instead of actual size.

I can see why it's a nice marketing thing to be able to say "our image is small!", but I hate pandering to that misinformation.

After reading @ASzc comments and discussing it with him, plus getting more information from Stian. I believe he has some valid points and I was wrong about that. My apologies Alex.

Alpine has some issues like:

  • Using musl libc instead of glibc, which means that binaries built with glibc are not compatible with musl
  • Older package libraries can disappear from the distribution library from time to time
  • The lack of availability of old packages, means that it makes it harder to rebuild containers from scratch

That being said, we're going to move forward with ubi-minimal images. That's the trade-off of quality over image size.

@ASzc when you revert it back to UBI, please take into consideration #541 and maybe the comments here: #576 (comment)

@jangaraj
Copy link
Contributor

Alpine has some issues like:

Using musl libc instead of glibc, which means that binaries built with glibc are not compatible with musl
Older package libraries can disappear from the distribution library from time to time
The lack of availability of old packages, means that it makes it harder to rebuild containers from scratch

These issues are not valid for current Dockerfile in the PR (alpine based). It builds static binary -> no glibc/musl is used at all + no additional Alpine packages are installed.

@abstractj
Copy link

These issues are not valid for current Dockerfile in the PR (alpine based). It builds static binary -> no glibc/musl is used at all + no additional Alpine packages are installed.

@jangaraj thanks for your input on this. Thinking about our long term goals and maintenance, we're going to move forward with UBI

I ended up rewriting a good portion of the Dockerfile. It now uses a
multi-stage build. It can accept source code to build, or unpack a premade
binary.

I also updated the Makefile
@ASzc
Copy link
Contributor Author

ASzc commented Jun 12, 2020

#576 doesn't need any changes here with the UBI parent (already has an appropriate nsswitch.conf).

#541 will be fixed by this PR, although I've implemented a slightly different approach to what was previously under the docker-build target. It'll build inside the container and then extract the binary, as was discussed recently. The old method of mounting the git workspace as a volume is much faster, but it can get really messy with Fedora selinux labels and file modes.

Pushing a new commit, with ubi as FROM, and changes to the Makefile, including podman/docker autodetect, trademark-free target aliases (old "docker" ones remain for backward compatibility), and the reworked container-build target.

@abstractj abstractj merged commit aa9ae98 into louketo:master Jun 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate Gatekeeper Docker image to Louketo make docker-build and make docker do not work on 9.0.2 branch
3 participants