Consider switching to ghcr.io/distroless images

[mattmoor]

We have been starting to pursue (with apko) the next generation of "distroless" tooling, and that has made curating images with complex dependencies (e.g. git) much more tractable than the old model. I chased the idea of "distroless git" years ago when we were starting knative/build and the dependency tree made this a nightmare, but with the new tooling, it's actually very tight!

I put together github.com/distroless/git largely based on the Tekton Dockerfile's apk add line: https://github.com/distroless/git/blob/1473a6a03596395baa6e99221405467c94f69798/.apko.yaml#L8-L10

Since this is a "distroless" image, it is actually smaller than what y'all have today (though since the bulk of it is from git, the difference isn't huge):

ls -l *.tar
-rw-r--r--  1 mattmoor  staff  35812864 Apr 12 08:18 bar.tar  <-- apko
-rw-r--r--  1 mattmoor  staff  38889472 Apr 12 08:09 foo.tar  <-- upstream (Dockerfile)

Another place worth considering a switch is your use of gcr.io/distroless/base:debug for your "shell image" here:

pipeline/config/controller.yaml

Lines 78 to 81 in 5ac6ef7

	# The shell image must be root in order to create directories and copy files to PVCs.
	# gcr.io/distroless/base:debug as of February 17, 2022
	# image shall not contains tag, so it will be supported on a runtime like cri-o
	"-shell-image", "gcr.io/distroless/base@sha256:3cebc059e7e52a4f5a389aa6788ac2b582227d7953933194764ea434f4d70d64",

This uses base:debug vs. static:debug because the latter doesn't exist, but really all you are after is the fact that :debug contains busybox. We created github.com/distroless/busybox for this, which (without glibc) is considerably smaller:

ls -l *.tar
-rw-r--r--  1 mattmoor  staff   5142016 Apr 12 12:52 bar.tar  <-- apko
-rw-r--r--  1 mattmoor  staff  23222272 Apr 12 12:51 foo.tar  <-- upstream distroless

We have been using ghcr.io/distroless/git and ghcr.io/distroless/busybox for each of these in our downstream testing for several days now without issue, and so I wanted to start a discussion around replacing these upstream (and eliminating the need to maintain the git-init Dockerfile + builds)...

cc @vdemeester @imjasonh @dlorenc @bobcatfish

[mattmoor]

cc @afrittoli

[imjasonh]

+1 to smaller images, but especially +1 to simpler, more declarative images.

Just so I can concretely understand the proposal:

• replace -shell-image with ghcr.io/distroless/busybox -- seems like a simple drop-in replacement 👍
• replace git-init's base image with ghcr.io/distroless/git
  • delete this Dockerfile, which is the only thing left in images/
  • this lets us delete the dind-based multiarch build for that base image, which means we drop its privilege:

pipeline/tekton/build-push-ma-base-image.yaml

Lines 90 to 91 in 5ac6ef7

	securityContext:
	privileged: true

I think it's definitely worth drafting a PR for, to get an idea what else shakes out from this. But I'm definitely happy with what I'm seeing so far. 😍

[mattmoor]

@imjasonh I'd probably split the switchover from deleting the Dockerfile, in case we want a really simple rollback, but that should definitely be the goal!

[vdemeester]

I do not see any reason not to switch to it so 👍🏼

[mattmoor]

Cool, I will try to draft the first part today. Then I'll stage something to try to remove the rest, which we can merge once we are comfortable with the switchover (I'll need help making sure I caught everything, but I'll start with @imjasonh fantastic list! 🤩 )

[mattmoor]

Alright, I kicked it and it picked up 2.35.2 from edge, so I'll send this shortly:

# docker run -ti ghcr.io/distroless/git --version
Unable to find image 'ghcr.io/distroless/git:latest' locally
latest: Pulling from distroless/git
27865ba1f7a8: Pull complete
Digest: sha256:b8ac8e9fc0b2dcafb666e68f56fe808fa7103da9e8b3d6f23a6cde4d54be22a2
Status: Downloaded newer image for ghcr.io/distroless/git:latest
git version 2.35.2

[mattmoor]

So grepping the code base, I also found tekton/publish.yaml rewrites .ko.yaml for windows support, and also adds (back?) several baseImageOverrides. I think this should be:

  - name: create-ko-yaml
    image: golang:1.17.8
    script: |
      #!/bin/sh
      set -ex

      # Setup docker-auth
      DOCKER_CONFIG=~/.docker
      mkdir -p ${DOCKER_CONFIG}
      cp /workspace/docker-config.json ${DOCKER_CONFIG}/config.json

      # Change to directory with vendor/
      cd ${PROJECT_ROOT}

      # Combine Distroless with a Windows base image, used for the entrypoint image.
      COMBINED_BASE_IMAGE=$(go run ./vendor/github.com/tektoncd/plumbing/cmd/combine/main.go \
        ghcr.io/distroless/busybox \
        mcr.microsoft.com/windows/nanoserver:1809 \
        ${CONTAINER_REGISTRY}/$(params.package)/combined-base-image:latest)

      cat <<EOF > ${PROJECT_ROOT}/.ko.yaml
      # This matches the value configured in .ko.yaml
      defaultBaseImage: gcr.io/distroless/static:nonroot
      baseImageOverrides:
        # Use the combined base image for images that should include Windows support.
        $(params.package)/cmd/entrypoint: ${COMBINED_BASE_IMAGE}
        $(params.package)/cmd/nop: ${COMBINED_BASE_IMAGE}
        $(params.package)/cmd/workingdirinit: ${COMBINED_BASE_IMAGE}

        # This matches values configured in .ko.yaml
        $(params.package)/cmd/git-init: ghcr.io/distroless/git
      EOF

      cat ${PROJECT_ROOT}/.ko.yaml

... but want to sanity check. I'll include this in my PR, but please pay attention and check my work because I kinda doubt this is tested presubmit 😅

[vdemeester]

That sounds about right 🙃
cc @imjasonh @afrittoli

[mattmoor]

Landing the first round of these now, please let me know if you see anything weird.

Next order of business will be to stage cleaning up the Dockerfile and such.

[mattmoor]

For deleting build-push-ma-base-image.yaml I'm unsure where this is TaskRun from, so we should clean that up before my deletion lands.

[imjasonh]

For deleting build-push-ma-base-image.yaml I'm unsure where this is TaskRun from, so we should clean that up before my deletion lands.

pipeline/tekton/release-pipeline.yaml

Lines 107 to 127 in 1ddb31c

	- name: build-base-image
	runAfter: [build, unit-tests]
	taskRef:
	name: build-multiarch-base-image
	params:
	- name: package
	value: $(params.package)
	- name: imageRegistry
	value: $(params.imageRegistry)
	- name: imageRegistryPath
	value: $(params.imageRegistryPath)
	- name: platforms
	value: $(params.buildPlatforms)
	- name: serviceAccountPath
	value: $(params.serviceAccountPath)
	workspaces:
	- name: source
	workspace: workarea
	subpath: git
	- name: release-secret
	workspace: release-secret

Which blocks publish-images:

pipeline/tekton/release-pipeline.yaml

Line 129 in 1ddb31c

runAfter: [build-base-image]

...which should just have:

runAfter: [build, unit-tests]

[imjasonh]

Looking closer, there's two different pipeline params, buildPlatforms (what platforms to pass to buildx, to build these base images) and publishPlatforms (what platforms to pass to ko).

So in removing build-base-image, we'll also want to remove buildPlatforms and only pass around publishPlatforms.

Though it looks like the default values defined in the Pipeline:

pipeline/tekton/release-pipeline.yaml

Lines 30 to 35 in 8fdcc09

	- name: publishPlatforms
	description: |
	Platforms to publish images for (e.g. linux/amd64,linux/arm64,windows/amd64). This
	can differ from buildPlatforms due to the fact that a windows-compatible base image
	is constructed for the publishing phase.
	default: linux/amd64,linux/arm,linux/arm64,linux/s390x,linux/ppc64le,windows/amd64

...match the defaults defined in the Task:

pipeline/tekton/publish.yaml

Lines 28 to 30 in 8fdcc09

	- name: platforms
	description: Platforms to publish for the images (e.g. linux/amd64,linux/arm64)
	default: linux/amd64,linux/arm,linux/arm64,linux/s390x,linux/ppc64le,windows/amd64

So maybe we can just remove the Pipeline param entirely and just rely on the Task param's default? 🤔

[mattmoor]

I wanted to circle back to this, since this has now presumably had reasonable bake time.

Do folks have any final thoughts on this, or should we remove the hold on #4765?

[vdemeester]

cc @jerop @lbernick @abayer
I think we can remove the hold. One less "base" image to maintain is looking good to me 😝

[mattmoor]

Ok, I am going to remove the hold. I believe Prow will rerun everything, and this gives us tomorrow to fix things before the weekend if some crazy merge conflict snuck in. 🤞