From 2ea13ab6f3d9fded5606114c6a4f0e6942e8a56c Mon Sep 17 00:00:00 2001 From: Yves Brissaud Date: Mon, 28 Oct 2019 15:21:25 +0100 Subject: [PATCH] allow to push local docker images by digest In the case `contentDigest` is defined (for example after a build by docker app) try to push local docker image from the digest. In that case, the `image` of the service is not defined in the bundle.json. If `contentDigest` is empty, do the same thing as before, so try to resolve and if it's not resolvable try to push. The service name is now displayed in error messages to better understand what's going on on failures. -- A `internal.ImageClient` interface has been added to only deal with methods from Docker's `ImageAPIClient` we need. Easier to mock for example. Signed-off-by: Yves Brissaud --- internal/image_client.go | 14 +++++++ remotes/fixup.go | 88 ++++++++++++++++++++++++++------------- remotes/fixup_test.go | 89 ++++++++++++++++++++++++++++++++++++++++ remotes/fixupoptions.go | 7 ++-- remotes/mocks_test.go | 30 ++++++++++++++ remotes/push.go | 5 ++- 6 files changed, 200 insertions(+), 33 deletions(-) create mode 100644 internal/image_client.go diff --git a/internal/image_client.go b/internal/image_client.go new file mode 100644 index 0000000..f830232 --- /dev/null +++ b/internal/image_client.go @@ -0,0 +1,14 @@ +package internal + +import ( + "context" + "io" + + "github.com/docker/docker/api/types" +) + +// ImageClient is a subset of Docker's ImageAPIClient interface with only what we are using for cnab-to-oci. +type ImageClient interface { + ImagePush(ctx context.Context, ref string, options types.ImagePushOptions) (io.ReadCloser, error) + ImageTag(ctx context.Context, image, ref string) error +} diff --git a/remotes/fixup.go b/remotes/fixup.go index f92db63..aaf2ebd 100644 --- a/remotes/fixup.go +++ b/remotes/fixup.go @@ -48,12 +48,12 @@ func FixupBundle(ctx context.Context, b *bundle.Bundle, ref reference.Named, res } relocationMap := relocation.ImageRelocationMap{} - if err := fixupImage(ctx, &b.InvocationImages[0].BaseImage, relocationMap, cfg, events, cfg.invocationImagePlatformFilter); err != nil { + if err := fixupImage(ctx, "InvocationImage", &b.InvocationImages[0].BaseImage, relocationMap, cfg, events, cfg.invocationImagePlatformFilter); err != nil { return nil, err } // Fixup images for name, original := range b.Images { - if err := fixupImage(ctx, &original.BaseImage, relocationMap, cfg, events, cfg.componentImagePlatformFilter); err != nil { + if err := fixupImage(ctx, name, &original.BaseImage, relocationMap, cfg, events, cfg.componentImagePlatformFilter); err != nil { return nil, err } b.Images[name] = original @@ -63,14 +63,22 @@ func FixupBundle(ctx context.Context, b *bundle.Bundle, ref reference.Named, res return relocationMap, nil } -func fixupImage(ctx context.Context, baseImage *bundle.BaseImage, relocationMap relocation.ImageRelocationMap, cfg fixupConfig, events chan<- FixupEvent, platformFilter platforms.Matcher) error { +func fixupImage( + ctx context.Context, + name string, + baseImage *bundle.BaseImage, + relocationMap relocation.ImageRelocationMap, + cfg fixupConfig, + events chan<- FixupEvent, + platformFilter platforms.Matcher) error { + log.G(ctx).Debugf("Updating entry in relocation map for %q", baseImage.Image) ctx = withMutedContext(ctx) notifyEvent, progress := makeEventNotifier(events, baseImage.Image, cfg.targetRef) notifyEvent(FixupEventTypeCopyImageStart, "", nil) // Fixup Base image - fixupInfo, err := fixupBaseImage(ctx, baseImage, cfg) + fixupInfo, pushed, err := fixupBaseImage(ctx, name, baseImage, cfg) if err != nil { return notifyError(notifyEvent, err) } @@ -99,6 +107,11 @@ func fixupImage(ctx context.Context, baseImage *bundle.BaseImage, relocationMap } } + if pushed { + notifyEvent(FixupEventTypeCopyImageEnd, "Image has been pushed for service "+name, nil) + return nil + } + if fixupInfo.sourceRef.Name() == fixupInfo.targetRepo.Name() { notifyEvent(FixupEventTypeCopyImageEnd, "Nothing to do: image reference is already present in repository"+fixupInfo.targetRepo.String(), nil) return nil @@ -181,38 +194,57 @@ func fixupPlatforms(ctx context.Context, return nil } -func fixupBaseImage(ctx context.Context, baseImage *bundle.BaseImage, cfg fixupConfig) (imageFixupInfo, error) { +func fixupBaseImage(ctx context.Context, name string, baseImage *bundle.BaseImage, cfg fixupConfig) (imageFixupInfo, bool, error) { // Check image references if err := checkBaseImage(baseImage); err != nil { - return imageFixupInfo{}, fmt.Errorf("invalid image %q: %s", baseImage.Image, err) + return imageFixupInfo{}, false, fmt.Errorf("invalid image %q for service %q: %s", baseImage.Image, name, err) } targetRepoOnly, err := reference.ParseNormalizedNamed(cfg.targetRef.Name()) if err != nil { - return imageFixupInfo{}, err - } - sourceImageRef, err := reference.ParseNormalizedNamed(baseImage.Image) - if err != nil { - return imageFixupInfo{}, fmt.Errorf("%q is not a valid image reference for %q: %s", baseImage.Image, cfg.targetRef, err) + return imageFixupInfo{}, false, err } - sourceImageRef = reference.TagNameOnly(sourceImageRef) - // Try to fetch the image descriptor - _, descriptor, err := cfg.resolver.Resolve(ctx, sourceImageRef.String()) - if err != nil { - if cfg.pushImages { - descriptor, err = pushImageToTarget(ctx, baseImage, cfg) - if err != nil { - return imageFixupInfo{}, err + var ( + descriptor ocischemav1.Descriptor + pushed bool + sourceImageRef reference.Named + ) + + if baseImage.Image == "" && cfg.pushImages { + // no image is defined for the service, try to push by digest from local docker image store + descriptor, err = pushImageToTarget(ctx, baseImage.Digest, cfg) + if err != nil { + return imageFixupInfo{}, false, err + } + pushed = true + } else { + // try to resolve + sourceImageRef, err = reference.ParseNormalizedNamed(baseImage.Image) + if err != nil { + return imageFixupInfo{}, false, fmt.Errorf("%q is not a valid image reference for %q: %s", baseImage.Image, cfg.targetRef, err) + } + sourceImageRef = reference.TagNameOnly(sourceImageRef) + + // Try to fetch the image descriptor + _, descriptor, err = cfg.resolver.Resolve(ctx, sourceImageRef.String()) + if err != nil { + if cfg.pushImages { + // try to push from local docker image store + descriptor, err = pushImageToTarget(ctx, baseImage.Image, cfg) + if err != nil { + return imageFixupInfo{}, false, err + } + pushed = true + } else { + return imageFixupInfo{}, false, fmt.Errorf("failed to resolve %q, push the image for service %q to the registry before pushing the bundle: %s", sourceImageRef, name, err) } - } else { - return imageFixupInfo{}, fmt.Errorf("failed to resolve %q, push the image to the registry before pushing the bundle: %s", sourceImageRef, err) } } return imageFixupInfo{ - resolvedDescriptor: descriptor, - sourceRef: sourceImageRef, targetRepo: targetRepoOnly, - }, nil + sourceRef: sourceImageRef, + resolvedDescriptor: descriptor, + }, pushed, nil } // pushImageToTarget pushes the image from the local docker daemon store to the target defined in the configuration. @@ -226,15 +258,15 @@ func fixupBaseImage(ctx context.Context, baseImage *bundle.BaseImage, cfg fixupC // - tag the image to push with targeted reference // - push the image using a docker `ImageAPIClient` // - resolve the pushed image to grab its digest -func pushImageToTarget(ctx context.Context, baseImage *bundle.BaseImage, cfg fixupConfig) (ocischemav1.Descriptor, error) { +func pushImageToTarget(ctx context.Context, src string, cfg fixupConfig) (ocischemav1.Descriptor, error) { taggedRef := reference.TagNameOnly(cfg.targetRef) - if err := cfg.imageClient.ImageTag(ctx, baseImage.Image, cfg.targetRef.String()); err != nil { - return ocischemav1.Descriptor{}, fmt.Errorf("failed to push image %q, make sure the image exists locally: %s", baseImage.Image, err) + if err := cfg.imageClient.ImageTag(ctx, src, cfg.targetRef.String()); err != nil { + return ocischemav1.Descriptor{}, fmt.Errorf("failed to push image %q, make sure the image exists locally: %s", src, err) } if err := pushTaggedImage(ctx, cfg.imageClient, cfg.targetRef, cfg.pushOut); err != nil { - return ocischemav1.Descriptor{}, fmt.Errorf("failed to push image %q: %s", baseImage.Image, err) + return ocischemav1.Descriptor{}, fmt.Errorf("failed to push image %q: %s", src, err) } _, descriptor, err := cfg.resolver.Resolve(ctx, taggedRef.String()) diff --git a/remotes/fixup_test.go b/remotes/fixup_test.go index d778237..2d99e96 100644 --- a/remotes/fixup_test.go +++ b/remotes/fixup_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "io" "io/ioutil" + "os" "testing" "github.com/containerd/containerd/images" @@ -104,6 +105,94 @@ func TestFixupBundleWithAutoUpdate(t *testing.T) { assert.DeepEqual(t, b, expectedBundle) } +func TestFixupBundlePushImages(t *testing.T) { + index := ocischemav1.Manifest{} + bufManifest, err := json.Marshal(index) + assert.NilError(t, err) + fetcher := &mockFetcher{indexBuffers: []*bytes.Buffer{ + // Manifest index + bytes.NewBuffer(bufManifest), + }} + pusher := &mockPusher{} + resolver := &mockResolver{ + pusher: pusher, + fetcher: fetcher, + resolvedDescriptors: []ocischemav1.Descriptor{ + // Invocation image will not be resolved first, so push will occurs + { + // just a code to raise an error in the mock + Size: -1, + }, + // Invocation image is resolved after push + { + MediaType: ocischemav1.MediaTypeImageManifest, + Size: 42, + Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0343", + }, + // Image will be resolved after push based on Digest + { + MediaType: ocischemav1.MediaTypeImageManifest, + Size: 43, + Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0344", + }, + }, + } + b := &bundle.Bundle{ + SchemaVersion: "v1.0.0", + InvocationImages: []bundle.InvocationImage{ + { + BaseImage: bundle.BaseImage{ + Image: "my.registry/namespace/my-app-invoc", + ImageType: "docker", + }, + }, + }, + Images: map[string]bundle.Image{ + "my-service": { + BaseImage: bundle.BaseImage{ + Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0344", + Image: "", + ImageType: "docker", + }, + }, + }, + Name: "my-app", + Version: "0.1.0", + } + ref, err := reference.ParseNamed("my.registry/namespace/my-app") + assert.NilError(t, err) + _, err = FixupBundle(context.TODO(), b, ref, resolver, WithAutoBundleUpdate(), WithPushImages(newMockImageClient(), os.Stdout)) + assert.NilError(t, err) + expectedBundle := &bundle.Bundle{ + SchemaVersion: "v1.0.0", + InvocationImages: []bundle.InvocationImage{ + { + BaseImage: bundle.BaseImage{ + Image: "my.registry/namespace/my-app-invoc", + ImageType: "docker", + MediaType: ocischemav1.MediaTypeImageManifest, + Size: 42, + Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0343", + }, + }, + }, + Images: map[string]bundle.Image{ + "my-service": { + BaseImage: bundle.BaseImage{ + Image: "", + ImageType: "docker", + MediaType: ocischemav1.MediaTypeImageManifest, + Size: 43, + Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0344", + }, + }, + }, + Name: "my-app", + Version: "0.1.0", + } + assert.DeepEqual(t, b, expectedBundle) +} + func TestFixupBundleFailsWithDifferentDigests(t *testing.T) { index := ocischemav1.Manifest{} bufManifest, err := json.Marshal(index) diff --git a/remotes/fixupoptions.go b/remotes/fixupoptions.go index 635d8cf..dae58b0 100644 --- a/remotes/fixupoptions.go +++ b/remotes/fixupoptions.go @@ -5,11 +5,12 @@ import ( "io" "io/ioutil" + "github.com/docker/cnab-to-oci/internal" + "github.com/containerd/containerd/platforms" "github.com/containerd/containerd/remotes" "github.com/deislabs/cnab-go/bundle" "github.com/docker/distribution/reference" - "github.com/docker/docker/client" ocischemav1 "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -32,7 +33,7 @@ type fixupConfig struct { componentImagePlatformFilter platforms.Matcher autoBundleUpdate bool pushImages bool - imageClient client.ImageAPIClient + imageClient internal.ImageClient pushOut io.Writer } @@ -129,7 +130,7 @@ func WithAutoBundleUpdate() FixupOption { // target tag. // But local only images (for example after a local build of components of the bundle) must be pushed. // This option will allow to push images that are only available in the docker daemon image store to the defined target. -func WithPushImages(imageClient client.ImageAPIClient, out io.Writer) FixupOption { +func WithPushImages(imageClient internal.ImageClient, out io.Writer) FixupOption { return func(cfg *fixupConfig) error { cfg.pushImages = true if imageClient == nil { diff --git a/remotes/mocks_test.go b/remotes/mocks_test.go index 1ad9553..37db6c5 100644 --- a/remotes/mocks_test.go +++ b/remotes/mocks_test.go @@ -3,11 +3,13 @@ package remotes import ( "bytes" "context" + "fmt" "io" "io/ioutil" "github.com/containerd/containerd/content" "github.com/containerd/containerd/remotes" + "github.com/docker/docker/api/types" "github.com/opencontainers/go-digest" ocischemav1 "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -23,6 +25,9 @@ type mockResolver struct { func (r *mockResolver) Resolve(_ context.Context, ref string) (string, ocischemav1.Descriptor, error) { descriptor := r.resolvedDescriptors[0] r.resolvedDescriptors = r.resolvedDescriptors[1:] + if descriptor.Size == -1 { + return "", descriptor, fmt.Errorf("empty descriptor") + } return ref, descriptor, nil } func (r *mockResolver) Fetcher(_ context.Context, ref string) (remotes.Fetcher, error) { @@ -90,3 +95,28 @@ func (f *mockFetcher) Fetch(ctx context.Context, desc ocischemav1.Descriptor) (i f.indexBuffers = f.indexBuffers[1:] return rc, nil } + +type mockReadCloser struct { +} + +func (rc mockReadCloser) Read(p []byte) (n int, err error) { + return 0, io.EOF +} + +func (rc mockReadCloser) Close() error { + return nil +} + +type mockImageClient struct { +} + +func newMockImageClient() *mockImageClient { + return &mockImageClient{} +} + +func (c *mockImageClient) ImagePush(ctx context.Context, ref string, options types.ImagePushOptions) (io.ReadCloser, error) { + return mockReadCloser{}, nil +} +func (c *mockImageClient) ImageTag(ctx context.Context, image, ref string) error { + return nil +} diff --git a/remotes/push.go b/remotes/push.go index 6406ac3..0318f7d 100644 --- a/remotes/push.go +++ b/remotes/push.go @@ -8,10 +8,11 @@ import ( "io" "os" + "github.com/docker/cnab-to-oci/internal" + "github.com/docker/cli/cli/config" configtypes "github.com/docker/cli/cli/config/types" "github.com/docker/docker/api/types" - "github.com/docker/docker/client" "github.com/docker/docker/pkg/jsonmessage" "github.com/docker/docker/registry" @@ -247,7 +248,7 @@ func pushBundleConfigDescriptor(ctx context.Context, name string, resolver remot return descriptor, nil } -func pushTaggedImage(ctx context.Context, imageClient client.ImageAPIClient, targetRef reference.Named, out io.Writer) error { +func pushTaggedImage(ctx context.Context, imageClient internal.ImageClient, targetRef reference.Named, out io.Writer) error { repoInfo, err := registry.ParseRepositoryInfo(targetRef) if err != nil { return err