From 15895397d04af156c62c9bfdac202776518c99eb Mon Sep 17 00:00:00 2001 From: Jimmi Dyson Date: Wed, 18 Sep 2024 11:50:44 +0100 Subject: [PATCH] feat: Optionally force convert to OCI media types on push (#786) This enables support for stricter registries, such as zot, as well as enabling users to move to a future without vendor specific Docker media types. --- README.md | 5 ++ cmd/mindthegap/push/bundle/bundle.go | 98 ++++++++++++++++++++++- images/manifest.go | 40 +++++---- test/e2e/imagebundle/helpers/helpers.go | 5 ++ test/e2e/imagebundle/push_bundle_test.go | 32 ++++++-- test/e2e/imagebundle/serve_bundle_test.go | 2 + 6 files changed, 159 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index a4729946..02e24f8b 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,11 @@ mindthegap push image-bundle --image-bundle \ All images in the image bundle tar file will be pushed to the target OCI registry. +Some registries (e.g. [zot](https://zotregistry.dev/) are strict about what media types they support. If you are pushing +to a registry that only accepts OCI media types, then specify the `--force-oci-media-types` flag. This will internally +convert any images that currently use Docker media types (`application/vnd.docker.*`) to OCI compatible media types +(`application/vnd.oci.*`). Using the images via any container runtime does not change. + #### Serving an image bundle **_This command is deprecated - see [Serving a bundle](#serving-a-bundle-supports-both-image-or-helm-chart)_** diff --git a/cmd/mindthegap/push/bundle/bundle.go b/cmd/mindthegap/push/bundle/bundle.go index 26ee7f9c..1fc518db 100644 --- a/cmd/mindthegap/push/bundle/bundle.go +++ b/cmd/mindthegap/push/bundle/bundle.go @@ -16,8 +16,12 @@ import ( "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/logs" "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/remote/transport" + mediatypes "github.com/google/go-containerregistry/pkg/v1/types" "github.com/spf13/cobra" "github.com/thediveo/enumflag/v2" "golang.org/x/sync/errgroup" @@ -59,6 +63,7 @@ func NewCommand(out output.Output, bundleCmdName string) *cobra.Command { ecrLifecyclePolicy string onExistingTag = Overwrite imagePushConcurrency int + forceOCIMediaTypes bool ) cmd := &cobra.Command{ @@ -219,6 +224,7 @@ func NewCommand(out output.Output, bundleCmdName string) *cobra.Command { onExistingTag, imagePushConcurrency, out, + forceOCIMediaTypes, prePushFuncs..., ) if err != nil { @@ -289,6 +295,8 @@ func NewCommand(out output.Output, bundleCmdName string) *cobra.Command { cmd.Flags(). IntVar(&imagePushConcurrency, "image-push-concurrency", 1, "Image push concurrency") + cmd.Flags().BoolVar(&forceOCIMediaTypes, "force-oci-media-types", false, "force OCI media types") + return cmd } @@ -301,6 +309,7 @@ func pushImages( onExistingTag onExistingTagMode, imagePushConcurrency int, out output.Output, + forceOCIMediaTypes bool, prePushFuncs ...prePushFunc, ) error { puller, err := remote.NewPuller(destRemoteOpts...) @@ -379,7 +388,9 @@ func pushImages( case Skip: // If tag exists already then do nothing. if _, exists := existingImageTags[imageTag]; exists { - pushFn = func(_ name.Reference, _ []remote.Option, _ name.Reference, _ []remote.Option) error { + pushFn = func( + _ name.Reference, _ []remote.Option, _ name.Reference, _ []remote.Option, _ bool, + ) error { return nil } } @@ -391,7 +402,7 @@ func pushImages( } } - if err := pushFn(srcImage, sourceRemoteOpts, destImage, destRemoteOpts); err != nil { + if err := pushFn(srcImage, sourceRemoteOpts, destImage, destRemoteOpts, forceOCIMediaTypes); err != nil { return err } @@ -418,12 +429,20 @@ func pushTag( sourceRemoteOpts []remote.Option, destImage name.Reference, destRemoteOpts []remote.Option, + forceOCIMediaTypes bool, ) error { idx, err := remote.Index(srcImage, sourceRemoteOpts...) if err != nil { return err } + if forceOCIMediaTypes { + idx, err = convertToOCIIndex(idx, srcImage, sourceRemoteOpts) + if err != nil { + return fmt.Errorf("failed to convert index to OCI format: %w", err) + } + } + return remote.WriteIndex(destImage, idx, destRemoteOpts...) } @@ -518,3 +537,78 @@ func getExistingImages( return existingTags, nil } + +func convertToOCIIndex( + originalIndex v1.ImageIndex, + srcImage name.Reference, + sourceRemoteOpts []remote.Option, +) (v1.ImageIndex, error) { + originalMediaType, err := originalIndex.MediaType() + if err != nil { + return nil, fmt.Errorf("failed to get media type of image index: %w", err) + } + + if originalMediaType == mediatypes.OCIImageIndex { + return originalIndex, nil + } + + var ociIdx v1.ImageIndex = empty.Index + ociIdx = mutate.IndexMediaType(ociIdx, mediatypes.OCIImageIndex) + + originalIdx, err := originalIndex.IndexManifest() + if err != nil { + return nil, fmt.Errorf("failed to read original image index manifest: %w", err) + } + + for i := range originalIdx.Manifests { + manifest := originalIdx.Manifests[i] + manifest.MediaType = mediatypes.OCIManifestSchema1 + + digestRef, err := name.NewDigest(fmt.Sprintf("%s@%s", srcImage.Context().Name(), manifest.Digest.String())) + if err != nil { + return nil, fmt.Errorf("failed to create digest reference: %w", err) + } + + imgDesc, err := remote.Get(digestRef, sourceRemoteOpts...) + if err != nil { + return nil, fmt.Errorf("failed to get image %q: %w", digestRef, err) + } + + img, err := imgDesc.Image() + if err != nil { + return nil, fmt.Errorf("failed to convert image descriptor for %q to image: %w", digestRef, err) + } + + ociImg := empty.Image + ociImg = mutate.MediaType(ociImg, mediatypes.OCIManifestSchema1) + ociImg = mutate.ConfigMediaType(ociImg, mediatypes.OCIConfigJSON) + layers, err := img.Layers() + if err != nil { + return nil, fmt.Errorf("failed to get layers for image %q: %w", digestRef, err) + } + + for _, layer := range layers { + ociImg, err = mutate.Append(ociImg, mutate.Addendum{ + Layer: layer, + MediaType: mediatypes.OCILayer, + }) + if err != nil { + return nil, fmt.Errorf("failed to append layer to image %q: %w", digestRef, err) + } + } + + ociImgDigest, err := ociImg.Digest() + if err != nil { + return nil, fmt.Errorf("failed to get digest for image %q: %w", digestRef, err) + } + + manifest.Digest = ociImgDigest + + ociIdx = mutate.AppendManifests(ociIdx, mutate.IndexAddendum{ + Add: ociImg, + Descriptor: manifest, + }) + } + + return ociIdx, nil +} diff --git a/images/manifest.go b/images/manifest.go index 1dd13352..de8b0614 100644 --- a/images/manifest.go +++ b/images/manifest.go @@ -5,6 +5,7 @@ package images import ( "fmt" + "strings" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -111,7 +112,7 @@ func platformsIgnoringVariantIfNotSpecified(platforms ...v1.Platform) match.Matc func indexForSinglePlatformImage( ref name.Reference, - img v1.Image, + image v1.Image, platforms ...string, ) (v1.ImageIndex, error) { if len(platforms) > 1 { @@ -123,29 +124,40 @@ func indexForSinglePlatformImage( ) } - imgConfig, err := img.ConfigFile() + imageConfig, err := image.ConfigFile() if err != nil { return nil, fmt.Errorf("failed to get image config for image %q: %w", ref, err) } - imgPlatform := v1.Platform{ - OS: imgConfig.OS, - OSVersion: imgConfig.OSVersion, - Architecture: imgConfig.Architecture, - Variant: imgConfig.Variant, + imagePlatform := v1.Platform{ + OS: imageConfig.OS, + OSVersion: imageConfig.OSVersion, + Architecture: imageConfig.Architecture, + Variant: imageConfig.Variant, } var index v1.ImageIndex = empty.Index index = mutate.AppendManifests( index, mutate.IndexAddendum{ - Add: img, + Add: image, Descriptor: v1.Descriptor{ - Platform: &imgPlatform, + Platform: &imagePlatform, }, }, ) - index = mutate.IndexMediaType(index, types.DockerManifestList) + + imageMediaType, err := image.MediaType() + if err != nil { + return nil, fmt.Errorf("failed to get image media type for image %q: %w", ref, err) + } + + indexMediaType := types.OCIImageIndex + if strings.Contains(string(imageMediaType), types.DockerVendorPrefix) { + indexMediaType = types.DockerManifestList + } + + index = mutate.IndexMediaType(index, indexMediaType) if len(platforms) == 0 { return index, nil @@ -156,17 +168,17 @@ func indexForSinglePlatformImage( return nil, fmt.Errorf("invalid platform %q: %w", platforms[0], err) } - imgPlatformForComparison := imgPlatform + imagePlatformForComparison := imagePlatform if v1Platform.Variant == "" { - imgPlatformForComparison.Variant = "" + imagePlatformForComparison.Variant = "" } - if !imgPlatformForComparison.Equals(*v1Platform) { + if !imagePlatformForComparison.Equals(*v1Platform) { return nil, fmt.Errorf( "requested image %q does not match requested platform %q (image is for %q)", ref, v1Platform, - imgPlatform, + imagePlatform, ) } diff --git a/test/e2e/imagebundle/helpers/helpers.go b/test/e2e/imagebundle/helpers/helpers.go index 046b7810..d817c52d 100644 --- a/test/e2e/imagebundle/helpers/helpers.go +++ b/test/e2e/imagebundle/helpers/helpers.go @@ -25,6 +25,7 @@ import ( "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/types" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" "github.com/onsi/gomega/gstruct" @@ -189,6 +190,7 @@ func ValidateImageIsAvailable( port int, registryPath, image, tag string, platforms []*v1.Platform, + forceOCIMediaTypes bool, opts ...remote.Option, ) { t.Helper() @@ -205,6 +207,9 @@ func ValidateImageIsAvailable( manifest, err := idx.IndexManifest() gomega.ExpectWithOffset(1, err).NotTo(gomega.HaveOccurred()) + if forceOCIMediaTypes { + gomega.ExpectWithOffset(1, manifest.MediaType).To(gomega.Equal(types.OCIImageIndex)) + } gomega.ExpectWithOffset(1, manifest.Manifests).To(gomega.HaveLen(len(platforms))) diff --git a/test/e2e/imagebundle/push_bundle_test.go b/test/e2e/imagebundle/push_bundle_test.go index 33578b35..2a5148fa 100644 --- a/test/e2e/imagebundle/push_bundle_test.go +++ b/test/e2e/imagebundle/push_bundle_test.go @@ -57,6 +57,7 @@ var _ = Describe("Push Bundle", func() { registryScheme string, registryPath string, registryInsecure bool, + forceOCIMediaTypes bool, ) { registryCACertFile := "" registryCertFile := "" @@ -113,6 +114,10 @@ var _ = Describe("Push Bundle", func() { args = append(args, "--to-registry-ca-cert-file", registryCACertFile) } + if forceOCIMediaTypes { + args = append(args, "--force-oci-media-types") + } + cmd.SetArgs(args) Expect(cmd.Execute()).To(Succeed()) @@ -136,6 +141,7 @@ var _ = Describe("Push Bundle", func() { OS: "linux", Architecture: runtime.GOARCH, }}, + forceOCIMediaTypes, remote.WithTransport(testRoundTripper), ) @@ -150,6 +156,7 @@ var _ = Describe("Push Bundle", func() { registryScheme string, registryPath string, registryInsecure bool, + forceOCIMediaTypes bool, ) { helpers.CreateBundle( GinkgoT(), @@ -158,14 +165,14 @@ var _ = Describe("Push Bundle", func() { "linux/"+runtime.GOARCH, ) - runTest(registryHost, registryScheme, registryPath, registryInsecure) + runTest(registryHost, registryScheme, registryPath, registryInsecure, forceOCIMediaTypes) }, - Entry("Without TLS", "127.0.0.1", "", "", true), + Entry("Without TLS", "127.0.0.1", "", "", true, false), - Entry("With TLS", helpers.GetFirstNonLoopbackIP(GinkgoT()).String(), "", "", false), + Entry("With TLS", helpers.GetFirstNonLoopbackIP(GinkgoT()).String(), "", "", false, false), - Entry("With Insecure TLS", helpers.GetFirstNonLoopbackIP(GinkgoT()).String(), "", "", true), + Entry("With Insecure TLS", helpers.GetFirstNonLoopbackIP(GinkgoT()).String(), "", "", true, false), Entry( "With http registry", @@ -173,6 +180,7 @@ var _ = Describe("Push Bundle", func() { "http", "", true, + false, ), Entry( @@ -181,9 +189,19 @@ var _ = Describe("Push Bundle", func() { "http", "", false, + false, + ), + + Entry( + "With Subpath", + helpers.GetFirstNonLoopbackIP(GinkgoT()).String(), + "", + "/nested/path/for/registry", + false, + false, ), - Entry("With Subpath", helpers.GetFirstNonLoopbackIP(GinkgoT()).String(), "", "/nested/path/for/registry", false), + Entry("With force OCI media types", helpers.GetFirstNonLoopbackIP(GinkgoT()).String(), "", "", false, true), ) It("Bundle does not exist", func() { @@ -221,7 +239,7 @@ var _ = Describe("Push Bundle", func() { }) It("Success", func() { - runTest(helpers.GetFirstNonLoopbackIP(GinkgoT()).String(), "", "", false) + runTest(helpers.GetFirstNonLoopbackIP(GinkgoT()).String(), "", "", false, false) }) Context("With headers from Docker config", func() { @@ -242,7 +260,7 @@ var _ = Describe("Push Bundle", func() { }) It("Success", func() { - runTest(helpers.GetFirstNonLoopbackIP(GinkgoT()).String(), "", "", false) + runTest(helpers.GetFirstNonLoopbackIP(GinkgoT()).String(), "", "", false, false) }) }) }) diff --git a/test/e2e/imagebundle/serve_bundle_test.go b/test/e2e/imagebundle/serve_bundle_test.go index 81847b26..c288103b 100644 --- a/test/e2e/imagebundle/serve_bundle_test.go +++ b/test/e2e/imagebundle/serve_bundle_test.go @@ -82,6 +82,7 @@ var _ = Describe("Serve Bundle", func() { OS: "linux", Architecture: runtime.GOARCH, }}, + false, ) close(stopCh) @@ -146,6 +147,7 @@ var _ = Describe("Serve Bundle", func() { OS: "linux", Architecture: runtime.GOARCH, }}, + false, remote.WithTransport(testRoundTripper), )