From 125e4b6665ab08354cfa6a9503ff65222a51736b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Va=C5=A1ek?= Date: Tue, 28 May 2024 16:27:17 +0200 Subject: [PATCH] Use image index not just image (#2284) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Always try daemon push first Daemon push is now tried always first withouth using net.DefaultResolver.LookupHost() to guess if the registry is reachable from the daemon. If the daemon returns error contaning "no such host" then use manual push with custom transport. Signed-off-by: Matej Vašek * Rework image pusher tests Merged daemon push and non-daemon tests to one table driven test. Signed-off-by: Matej Vašek * Use image index not just image This results in "multi-arch" image with single architecture this may seems weird but it helps multi-arch clusters to run pods on approprieate nodes. Signed-off-by: Matej Vašek * fixup: style Signed-off-by: Matej Vašek --------- Signed-off-by: Matej Vašek --- pkg/docker/pusher.go | 92 ++++++++-- pkg/docker/pusher_test.go | 352 +++++++++++++++++++------------------- 2 files changed, 252 insertions(+), 192 deletions(-) diff --git a/pkg/docker/pusher.go b/pkg/docker/pusher.go index 67c0f20129..0fb26a3dcf 100644 --- a/pkg/docker/pusher.go +++ b/pkg/docker/pusher.go @@ -8,10 +8,10 @@ import ( "errors" "fmt" "io" - "net" "net/http" "os" "regexp" + "strings" fn "knative.dev/func/pkg/functions" @@ -23,7 +23,11 @@ import ( "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/daemon" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/partial" "github.com/google/go-containerregistry/pkg/v1/remote" + types2 "github.com/google/go-containerregistry/pkg/v1/types" "golang.org/x/term" ) @@ -107,8 +111,72 @@ func GetRegistry(img string) (string, error) { return registry, nil } -// Push the image of the function. -func (n *Pusher) Push(ctx context.Context, f fn.Function) (digest string, err error) { +// Push the image index of the function. +func (n *Pusher) Push(ctx context.Context, f fn.Function) (string, error) { + credentials, err := n.credentialsProvider(ctx, f.Build.Image) + if err != nil { + return "", fmt.Errorf("failed to get credentials: %w", err) + } + + imgDigest, err := n.pushImage(ctx, f, credentials) + if err != nil { + return "", fmt.Errorf("cannot push image: %w", err) + } + + auth := &authn.Basic{ + Username: credentials.Username, + Password: credentials.Password, + } + + remoteOpts := []remote.Option{ + remote.WithAuth(auth), + remote.WithTransport(n.transport), + } + + imgRef, err := name.ParseReference(f.ImageNameWithDigest(imgDigest)) + if err != nil { + return "", fmt.Errorf("cannot parse image ref: %w", err) + } + img, err := remote.Image(imgRef, remoteOpts...) + if err != nil { + return "", fmt.Errorf("cannot get the image: %w", err) + } + + cf, err := img.ConfigFile() + if err != nil { + return "", fmt.Errorf("cannot get config file for the image: %w", err) + } + + newDesc, err := partial.Descriptor(img) + if err != nil { + return "", fmt.Errorf("cannot get partial descriptor for the image: %w", err) + } + newDesc.Platform = cf.Platform() + + base := mutate.IndexMediaType(empty.Index, types2.DockerManifestList) + idx := mutate.AppendManifests(base, mutate.IndexAddendum{ + Add: img, + Descriptor: *newDesc, + }) + + idxRef, err := name.ParseReference(f.Build.Image) + if err != nil { + return "", fmt.Errorf("cannot parse image index ref: %w", err) + } + err = remote.WriteIndex(idxRef, idx, remoteOpts...) + if err != nil { + return "", fmt.Errorf("cannot write image index: %w", err) + } + + d, err := idx.Digest() + if err != nil { + return "", fmt.Errorf("cannot obtain image index digest: %w", err) + } + + return d.String(), nil +} + +func (n *Pusher) pushImage(ctx context.Context, f fn.Function, credentials Credentials) (digest string, err error) { var output io.Writer @@ -127,19 +195,17 @@ func (n *Pusher) Push(ctx context.Context, f fn.Function) (digest string, err er return "", err } - credentials, err := n.credentialsProvider(ctx, f.Build.Image) - if err != nil { - return "", fmt.Errorf("failed to get credentials: %w", err) - } fmt.Fprintf(os.Stderr, "Pushing function image to the registry %q using the %q user credentials\n", registry, credentials.Username) - // if the registry is not cluster private do push directly from daemon - if _, err = net.DefaultResolver.LookupHost(ctx, registry); err == nil { - return n.daemonPush(ctx, f, credentials, output) + digest, err = n.daemonPush(ctx, f, credentials, output) + if err == nil { + return digest, nil } - - // push with custom transport to be able to push into cluster private registries - return n.push(ctx, f, credentials, output) + if strings.Contains(err.Error(), "no such host") { + // push with custom transport to be able to push into cluster private registries + return n.push(ctx, f, credentials, output) + } + return "", err } func (n *Pusher) daemonPush(ctx context.Context, f fn.Function, credentials Credentials, output io.Writer) (digest string, err error) { diff --git a/pkg/docker/pusher_test.go b/pkg/docker/pusher_test.go index ac10af2578..48bd5b5e25 100644 --- a/pkg/docker/pusher_test.go +++ b/pkg/docker/pusher_test.go @@ -23,6 +23,8 @@ import ( "net/http" "os" "reflect" + "slices" + "strconv" "strings" "testing" "time" @@ -33,6 +35,9 @@ import ( "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/registry" "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/tarball" + regTypes "github.com/google/go-containerregistry/pkg/v1/types" + "gotest.tools/v3/assert" "knative.dev/func/pkg/docker" fn "knative.dev/func/pkg/functions" @@ -68,9 +73,11 @@ const ( testUser = "testuser" testPwd = "testpwd" registryHostname = "my.testing.registry" - functionImage = "/testuser/func:latest" - functionImageRemote = registryHostname + functionImage - functionImageLocal = "localhost" + functionImage + functionImageRemote = registryHostname + "/testuser/func:latest" + + imageTarball = "testdata/image.tar" + imageID = "sha256:1fb61f35700f47e1e868f8b26fdd777016f96bb1b4b0b0e623efac39eb30d12e" + imageRepoDigest = "sha256:00af51d125f3092e157a7f8a717029412dc9d266c017e89cecdfeccb4cc3d7a7" ) var testCredProvider = docker.CredentialsProvider(func(ctx context.Context, registry string) (docker.Credentials, error) { @@ -80,211 +87,198 @@ var testCredProvider = docker.CredentialsProvider(func(ctx context.Context, regi }, nil }) -func TestDaemonPush(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), time.Minute*5) - defer cancel() - - var optsPassedToMock types.ImagePushOptions - var imagePassedToMock string - var closeCalledOnMock bool - - dockerClient := newMockPusherDockerClient() - - dockerClient.imagePush = func(ctx context.Context, ref string, options types.ImagePushOptions) (io.ReadCloser, error) { - imagePassedToMock = ref - optsPassedToMock = options - return io.NopCloser(strings.NewReader(`{ - "status": "latest: digest: sha256:00af51d125f3092e157a7f8a717029412dc9d266c017e89cecdfeccb4cc3d7a7 size: 2613" -} -`)), nil - } - - dockerClient.close = func() error { - closeCalledOnMock = true - return nil - } - - dockerClientFactory := func() (docker.PusherDockerClient, error) { - return dockerClient, nil - } - pusher := docker.NewPusher( - docker.WithCredentialsProvider(testCredProvider), - docker.WithPusherDockerClientFactory(dockerClientFactory), - ) - - f := fn.Function{ - Build: fn.BuildSpec{ - Image: functionImageLocal, - }, - } - - digest, err := pusher.Push(ctx, f) - if err != nil { - t.Fatal(err) - } - - if digest != "sha256:00af51d125f3092e157a7f8a717029412dc9d266c017e89cecdfeccb4cc3d7a7" { - t.Errorf("got bad digest: %q", digest) - } - - authData, err := base64.StdEncoding.DecodeString(optsPassedToMock.RegistryAuth) - if err != nil { - t.Fatal(err) - } - - authStruct := struct { - Username, Password string - }{} +func TestPush(t *testing.T) { - dec := json.NewDecoder(bytes.NewReader(authData)) - - err = dec.Decode(&authStruct) - if err != nil { - t.Fatal(err) + tests := []struct { + Name string + DaemonPush bool + }{ + {Name: "daemon push", DaemonPush: true}, + {Name: "non daemon push"}, } - if imagePassedToMock != functionImageLocal { - t.Errorf("Bad image name passed to the Docker API Client: %q.", imagePassedToMock) - } + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Minute*5) + defer cancel() - if authStruct.Username != testUser || authStruct.Password != testPwd { - t.Errorf("Bad credentials passed to the Docker API Client: %q:%q", authStruct.Username, authStruct.Password) - } + // in memory network emulation + connections := conns(make(chan net.Conn)) - if !closeCalledOnMock { - t.Error("The Close() function has not been called on the Docker API Client.") - } + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.TLSClientConfig = &tls.Config{ + InsecureSkipVerify: true, + } + transport.DialContext = connections.DialContext + + serveRegistry(t, connections) + + dockerClient := newMockPusherDockerClient() + + pushReachable := func(ctx context.Context, ref string, options types.ImagePushOptions) (io.ReadCloser, error) { + if ref != functionImageRemote { + return nil, fmt.Errorf("unexpected ref") + } + + var err error + + authData, err := base64.StdEncoding.DecodeString(options.RegistryAuth) + if err != nil { + return nil, err + } + + authStruct := struct { + Username, Password string + }{} + + dec := json.NewDecoder(bytes.NewReader(authData)) + + err = dec.Decode(&authStruct) + if err != nil { + return nil, err + } + + remoteOpts := []remote.Option{ + remote.WithTransport(transport), + remote.WithAuth(&authn.Basic{ + Username: authStruct.Username, + Password: authStruct.Password, + }), + } + tag, err := name.NewTag(ref) + if err != nil { + return nil, err + } + img, err := tarball.ImageFromPath(imageTarball, &tag) + if err != nil { + return nil, err + } + + err = remote.Write(tag, img, remoteOpts...) + if err != nil { + return nil, err + } + is, err := img.Size() + if err != nil { + return nil, err + } + return io.NopCloser(strings.NewReader(`{ + "status": "latest: digest: ` + imageRepoDigest + ` size: ` + strconv.FormatInt(is, 10) + `" } +`)), nil + } -func TestNonDaemonPush(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), time.Minute*5) - defer cancel() - - // in memory network emulation - connections := conns(make(chan net.Conn)) - - serveRegistry(t, connections) - - transport := http.DefaultTransport.(*http.Transport).Clone() - transport.TLSClientConfig = &tls.Config{ - InsecureSkipVerify: true, - } - transport.DialContext = connections.DialContext - - dockerClient := newMockPusherDockerClient() - - var imagesPassedToMock []string - dockerClient.imageSave = func(ctx context.Context, images []string) (io.ReadCloser, error) { - imagesPassedToMock = images - f, err := os.Open("./testdata/image.tar") - if err != nil { - return nil, fmt.Errorf("failed to load image tar: %w", err) - } - return f, nil - } - - dockerClient.imageInspect = func(ctx context.Context, s string) (types.ImageInspect, []byte, error) { - return types.ImageInspect{ID: "sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"}, []byte{}, nil - } - - dockerClientFactory := func() (docker.PusherDockerClient, error) { - return dockerClient, nil - } + pushUnreachable := func(ctx context.Context, ref string, options types.ImagePushOptions) (io.ReadCloser, error) { + return io.NopCloser(strings.NewReader(`{"errorDetail": {"message": "...no such host..."}}`)), nil + } - pusher := docker.NewPusher( - docker.WithTransport(transport), - docker.WithCredentialsProvider(testCredProvider), - docker.WithPusherDockerClientFactory(dockerClientFactory), - ) + if tt.DaemonPush { + dockerClient.imagePush = pushReachable + } else { + dockerClient.imagePush = pushUnreachable + } - f := fn.Function{ - Build: fn.BuildSpec{ - Image: functionImageRemote, - }, - } + dockerClient.imageInspect = func(ctx context.Context, s string) (types.ImageInspect, []byte, error) { + return types.ImageInspect{ID: imageID}, []byte{}, nil + } - actualDigest, err := pusher.Push(ctx, f) - if err != nil { - t.Fatal(err) - } + dockerClient.imageSave = func(ctx context.Context, tags []string) (io.ReadCloser, error) { + if slices.Equal(tags, []string{functionImageRemote}) { + f, err := os.Open(imageTarball) + if err != nil { + return nil, err + } + return f, nil + } + return nil, fmt.Errorf("unexpected tags") + } - if !reflect.DeepEqual(imagesPassedToMock, []string{f.Build.Image}) { - t.Errorf("Bad image name passed to the Docker API Client: %q.", imagesPassedToMock) - } + var closeCalledOnMock bool + dockerClient.close = func() error { + closeCalledOnMock = true + return nil + } - r, err := name.NewRegistry(registryHostname) - if err != nil { - t.Fatal(err) - } + dockerClientFactory := func() (docker.PusherDockerClient, error) { + return dockerClient, nil + } - remoteOpts := []remote.Option{ - remote.WithTransport(transport), - remote.WithAuth(&authn.Basic{ - Username: testUser, - Password: testPwd, - }), - } + pusher := docker.NewPusher( + docker.WithTransport(transport), + docker.WithCredentialsProvider(testCredProvider), + docker.WithPusherDockerClientFactory(dockerClientFactory), + ) - c, err := remote.Catalog(ctx, r, remoteOpts...) - if err != nil { - t.Fatal(err) - } + f := fn.Function{ + Build: fn.BuildSpec{ + Image: functionImageRemote, + }, + } - if !reflect.DeepEqual(c, []string{"testuser/func"}) { - t.Error("unexpected catalog content") - } + remoteOpts := []remote.Option{ + remote.WithTransport(transport), + remote.WithAuth(&authn.Basic{ + Username: testUser, + Password: testPwd, + }), + } - imgRef := name.MustParseReference(functionImageRemote) + _, err := pusher.Push(ctx, f) + if err != nil { + t.Fatal(err) + } - img, err := remote.Image(imgRef, remoteOpts...) - if err != nil { - t.Fatal(err) - } + r, err := name.NewRegistry(registryHostname) + if err != nil { + t.Fatal(err) + } - expectedDigest, err := img.Digest() - if err != nil { - t.Fatal(err) - } + c, err := remote.Catalog(ctx, r, remoteOpts...) + if err != nil { + t.Fatal(err) + } - if actualDigest != expectedDigest.String() { - t.Error("digest does not match") - } + if !reflect.DeepEqual(c, []string{"testuser/func"}) { + t.Error("unexpected catalog content") + } - layers, err := img.Layers() - if err != nil { - t.Fatal(err) - } + ref := name.MustParseReference(functionImageRemote) - expectedDiffs := []string{ - "sha256:cba17de713254df68662c399558da08f1cd9abfa4e3b404544757982b4f11597", - "sha256:d5c07940dc570b965530593384a3cf47f47bf07d68eb741d875090392a6331c3", - "sha256:a4dd43077393aff80a0bec5c1bf2db4941d620dcaa545662068476e324efefaa", - "sha256:abe6122e067d0f8bee1a8b8f0c4bb9d2b23cc73617bc8ff4addd6c1329bca23e", - "sha256:515e67f7a08c1798cad8ee4d5f0ce7e606540f5efe5163a967d8dc58994f9641", - "sha256:a5fdabf59fa2a4a0e60d21c1ffc4d482e62debe196bae742a476f4d5b893f0ce", - "sha256:8d6bae166e585b4b503d36a7de0ba749b68ef689884e94dfa0655bbf8ce4d213", - "sha256:b136b7c51981af6493ecbb1136f6ff0f23734f7b9feacb20c8090ac9dec6333d", - "sha256:e9055109e5f7999da5add4b5fff11a34783cddc9aef492d9b790024d1bc1b7d0", - "sha256:5a1ff39e0e0291a43170cbcd70515bfccef4bed4c7e7b97f82d49d3d557fe04b", - } + desc, err := remote.Get(ref, remoteOpts...) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, desc.MediaType, regTypes.DockerManifestList) - actualDiffs := make([]string, 0, len(expectedDiffs)) + img, err := remote.Image(ref, remoteOpts...) + if err != nil { + t.Fatal(err) + } - for _, layer := range layers { - diffID, err := layer.DiffID() - if err != nil { - t.Fatal(err) - } - actualDiffs = append(actualDiffs, diffID.String()) - } + d, err := img.Digest() + if err != nil { + t.Fatal(err) + } + assert.Equal(t, d.String(), imageRepoDigest) - if !reflect.DeepEqual(expectedDiffs, actualDiffs) { - t.Error("layer diffs in tar and from registry differs") + if !closeCalledOnMock { + t.Error("The Close() function has not been called on the Docker API Client.") + } + }) } } func newMockPusherDockerClient() *mockPusherDockerClient { return &mockPusherDockerClient{ + imagePush: func(ctx context.Context, ref string, options types.ImagePushOptions) (io.ReadCloser, error) { + return nil, fmt.Errorf(" imagePush not implemented") + }, + imageSave: func(ctx context.Context, strings []string) (io.ReadCloser, error) { + return nil, fmt.Errorf("imageSave not implemented") + }, + imageInspect: func(ctx context.Context, s string) (types.ImageInspect, []byte, error) { + return types.ImageInspect{}, nil, fmt.Errorf("imageInspect not implemented") + }, negotiateAPIVersion: func(ctx context.Context) {}, close: func() error { return nil }, }