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

RFC: Statically link cred helpers, include them in slim image #1872

Closed
imjasonh opened this issue Jan 5, 2022 · 5 comments · Fixed by #1891
Closed

RFC: Statically link cred helpers, include them in slim image #1872

imjasonh opened this issue Jan 5, 2022 · 5 comments · Fixed by #1891

Comments

@imjasonh
Copy link
Collaborator

imjasonh commented Jan 5, 2022

Opening this to gather feedback from the community:

Today, Kaniko's executor image includes cred helper executables for GCR, ECR and ACR, and configures a keychain that calls those executables, like Docker does, to get credentials to authorize requests.

The slim image is "slim" because it doesn't install these helpers. The total compressed size of the executor image is 28 MB, slim is 13 MB (~1/2 size):

$ img=gcr.io/kaniko-project/executor:ee2249b3d58267b7d34009b91f9c536b969bc4fd
$ crane manifest $img --platform=linux/amd64 | jq '.config.size + ([.layers[].size] | add)' | numfmt --to=iec
28M
$ crane manifest $img-slim --platform=linux/amd64 | jq '.config.size + ([.layers[].size] | add)' | numfmt --to=iec
13M

Instead of installing these cred helpers, because they're implemented in Go and open source, we can simply depend on that code and statically link it into binary. This would be made easier by google/go-containerregistry#1227

This has some benefits:

  • the image doesn't contain N compiled Go executables that are invoked as needed, it would include 1 Go executable containing all the equivalent logic
  • the version of that Go code would be tracked in go.mod, which can be automatically upgraded using Dependabot
  • the Go code can be compiled to any architecture we need -- we do this today using go install in our Dockerfile, instead of downloading pre-built released binaries, so this is effectively the same

By statically linking the three cred helpers, we actually shrink the size of the non-slim executor binary from 57 MB to 41 MB (both uncompressed), because today it depends on k8schain, which pulls in K8s client-go, etc., even though it only uses k8schain.NewNoClient, which doesn't call any K8s APIs -- it just imports K8s code to get their credential helpers.

Statically linking would also let us remove the three cred helper executables from the image, which amount to 30.4 MB (all uncompressed, GCP=7.8 MB, ECR=11.7, ACR=10.9).

At this point, there'd be no effective difference between the slim image and the regular executor image -- we should maintain the slim tag for backward compatibility, it could just point to the same executor image. This means the slim image will get bigger than it is today, but it's still very small, and not having to contend with cred helper executables seems like a useful win for users and maintainers.

If for some reason people really wanted a slim image without cred helper logic increasing the size, we could provide on that using Go build tags to exclude the ECR and ACR helpers, which brings the build-time-slimmed executor image down to 37 MB (uncompressed) -- this should compress to roughly the same size as the slim image today, and probably smaller, since K8s client-go would be removed as well.


Concretely, the proposal is to change pkg/creds/creds.go (for Linux and Darwin) to be:

package creds

import (
        "github.com/awslabs/amazon-ecr-credential-helper/ecr-login"
        "github.com/awslabs/amazon-ecr-credential-helper/ecr-login/api"
        "github.com/chrismellard/docker-credential-acr-env/pkg/credhelper"
        "github.com/google/go-containerregistry/pkg/authn"
        "github.com/google/go-containerregistry/pkg/v1/google"
)

// GetKeychain returns a keychain for accessing container registries.
func GetKeychain() authn.Keychain {
        return authn.NewMultiKeychain(
                authn.DefaultKeychain,
                google.Keychain,
                authn.NewKeychainFromHelper(ecr.ECRHelper{ClientFactory: api.DefaultClientFactory{}}),
                authn.NewKeychainFromHelper(credhelper.NewACRCredentialsHelper()),
        )
}

imjasonh/kaniko@bump-docker...imjasonh:cred-helpers

I'd propose holding off on this until we've had a successful v1.8.0 release (#1871), and ideally would also come with some manner of automated testing for GCR/ECR/ACR, but that might be difficult.

@imjasonh
Copy link
Collaborator Author

imjasonh commented Jan 5, 2022

cc @jonjohnsonjr

@mattmoor
Copy link
Collaborator

mattmoor commented Jan 6, 2022

The other advantage not called out is that this would immediately make slim more portable, since it would work with AWS and Azure where today you need the full executor.

I'm totally +1 on this.

@mattmoor
Copy link
Collaborator

mattmoor commented Jan 6, 2022

If folks are worried about compat, one course we might consider is to ship slim this way with messaging that we plan to switch everything in a subsequent release? That gives folks time to "try before they buy" and report issues.

@Vrtak-CZ
Copy link

Does this mean it will drop support for any additional credential helpers? We are using env one (https://github.com/isometry/docker-credential-env) for pushing our public images to Docker Hub. So we don't need to generate config.json "on-the-fly".

@imjasonh
Copy link
Collaborator Author

Does this mean it will drop support for any additional credential helpers? We are using env one (https://github.com/isometry/docker-credential-env) for pushing our public images to Docker Hub. So we don't need to generate config.json "on-the-fly".

Nope, any other cred helpers configured in config.json will still be consulted via the authn.DefaultKeychain.

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 a pull request may close this issue.

3 participants