core: support optionally setting explicit mediatypes (dagger#5467)

* core: support optionally setting explicit mediatypes We previously made a change to default to using oci mediatypes, which helped compatibility with certain compression types that are only supported with oci types. However, older registries (specifically a user reported Artifactory versions from pre-2020) don't have OCI support. This change fixes that by leaving OCI as the default but enabling users to specify docker mediatypes to be used instead as a fallback. Signed-off-by: Erik Sipsma <erik@dagger.io> * Specify default value for mediatypes in gql schema. Signed-off-by: Erik Sipsma <erik@dagger.io> --------- Signed-off-by: Erik Sipsma <erik@dagger.io>
vito · Jul 15, 2023 · e9d557a · e9d557a
1 parent 13aea3d
commit e9d557a
Show file tree

Hide file tree

Showing 8 changed files with 281 additions and 57 deletions.
diff --git a/core/container.go b/core/container.go
@@ -1411,19 +1411,24 @@ func (container *Container) Publish(
 	ref string,
 	platformVariants []ContainerID,
 	forcedCompression ImageLayerCompression,
+	mediaTypes ImageMediaTypes,
 	bkClient *bkclient.Client,
 	solveOpts bkclient.SolveOpt,
 	solveCh chan<- *bkclient.SolveStatus,
 ) (string, error) {
+	if mediaTypes == "" {
+		// Modern registry implementations support oci types and docker daemons
+		// have been capable of pulling them since 2018:
+		// https://github.com/moby/moby/pull/37359
+		// So they are a safe default.
+		mediaTypes = OCIMediaTypes
+	}
 	exportOpts := bkclient.ExportEntry{
 		Type: bkclient.ExporterImage,
 		Attrs: map[string]string{
-			"name": ref,
-			"push": strconv.FormatBool(true),
-			// Every registry supports oci types and docker daemons have been capable of pulling them
-			// since 2018: https://github.com/moby/moby/pull/37359
-			// So it should be safe to always use them.
-			"oci-mediatypes": strconv.FormatBool(true),
+			"name":           ref,
+			"push":           strconv.FormatBool(true),
+			"oci-mediatypes": strconv.FormatBool(mediaTypes == OCIMediaTypes),
 		},
 	}
 	if forcedCompression != "" {
@@ -1473,10 +1478,14 @@ func (container *Container) Export(
 	dest string,
 	platformVariants []ContainerID,
 	forcedCompression ImageLayerCompression,
+	mediaTypes ImageMediaTypes,
 	bkClient *bkclient.Client,
 	solveOpts bkclient.SolveOpt,
 	solveCh chan<- *bkclient.SolveStatus,
 ) error {
+	if mediaTypes == "" {
+		mediaTypes = OCIMediaTypes
+	}
 	dest, err := host.NormalizeDest(dest)
 	if err != nil {
 		return err
@@ -1496,7 +1505,7 @@ func (container *Container) Export(
 	exportOpts := bkclient.ExportEntry{
 		Type: bkclient.ExporterOCI,
 		Attrs: map[string]string{
-			"oci-mediatypes": strconv.FormatBool(true),
+			"oci-mediatypes": strconv.FormatBool(mediaTypes == OCIMediaTypes),
 		},
 	}
 	if forcedCompression != "" {
@@ -1988,3 +1997,10 @@ const (
 	CompressionEStarGZ      ImageLayerCompression = "EStarGZ"
 	CompressionUncompressed ImageLayerCompression = "Uncompressed"
 )
+
+type ImageMediaTypes string
+
+const (
+	OCIMediaTypes    ImageMediaTypes = "OCIMediaTypes"
+	DockerMediaTypes ImageMediaTypes = "DockerMediaTypes"
+)
diff --git a/core/integration/container_test.go b/core/integration/container_test.go
@@ -3734,6 +3734,83 @@ func TestContainerForceCompression(t *testing.T) {
 	}
 }
 
+func TestContainerMediaTypes(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		mediaTypes           dagger.ImageMediaTypes
+		expectedOCIMediaType string
+	}{
+		{
+			"", // use default
+			"application/vnd.oci.image.layer.v1.tar+gzip",
+		},
+		{
+			dagger.Ocimediatypes,
+			"application/vnd.oci.image.layer.v1.tar+gzip",
+		},
+		{
+			dagger.Dockermediatypes,
+			"application/vnd.docker.image.rootfs.diff.tar.gzip",
+		},
+	} {
+		tc := tc
+		t.Run(string(tc.mediaTypes), func(t *testing.T) {
+			t.Parallel()
+
+			c, ctx := connect(t)
+			defer c.Close()
+
+			ref := registryRef("testcontainerpublishmediatypes" + strings.ToLower(string(tc.mediaTypes)))
+			_, err := c.Container().
+				From("alpine:3.16.2").
+				Publish(ctx, ref, dagger.ContainerPublishOpts{
+					MediaTypes: tc.mediaTypes,
+				})
+			require.NoError(t, err)
+
+			parsedRef, err := name.ParseReference(ref, name.Insecure)
+			require.NoError(t, err)
+
+			imgDesc, err := remote.Get(parsedRef, remote.WithTransport(http.DefaultTransport))
+			require.NoError(t, err)
+			img, err := imgDesc.Image()
+			require.NoError(t, err)
+			layers, err := img.Layers()
+			require.NoError(t, err)
+			for _, layer := range layers {
+				mediaType, err := layer.MediaType()
+				require.NoError(t, err)
+				require.EqualValues(t, tc.expectedOCIMediaType, mediaType)
+			}
+
+			tarPath := filepath.Join(t.TempDir(), "export.tar")
+			_, err = c.Container().
+				From("alpine:3.16.2").
+				Export(ctx, tarPath, dagger.ContainerExportOpts{
+					MediaTypes: tc.mediaTypes,
+				})
+			require.NoError(t, err)
+
+			// check that docker compatible manifest is present
+			dockerManifestBytes := readTarFile(t, tarPath, "manifest.json")
+			require.NotNil(t, dockerManifestBytes)
+
+			indexBytes := readTarFile(t, tarPath, "index.json")
+			var index ocispecs.Index
+			require.NoError(t, json.Unmarshal(indexBytes, &index))
+
+			manifestDigest := index.Manifests[0].Digest
+			manifestBytes := readTarFile(t, tarPath, "blobs/sha256/"+manifestDigest.Encoded())
+			var manifest ocispecs.Manifest
+			require.NoError(t, json.Unmarshal(manifestBytes, &manifest))
+			for _, layer := range manifest.Layers {
+				require.EqualValues(t, tc.expectedOCIMediaType, layer.MediaType)
+			}
+		})
+	}
+}
+
 func TestContainerBuildMergesWithParent(t *testing.T) {
 	t.Parallel()
 
@@ -3866,11 +3943,12 @@ func TestContainerFromMergesWithParent(t *testing.T) {
 func TestContainerImageLoadCompatibility(t *testing.T) {
 	t.Parallel()
 	c, ctx := connect(t)
-	defer c.Close()
+	t.Cleanup(func() { c.Close() })
 	ctx, cancel := context.WithCancel(ctx)
-	defer cancel()
+	t.Cleanup(cancel)
 
 	for i, dockerVersion := range []string{"20.10", "23.0", "24.0"} {
+		dockerVersion := dockerVersion
 		port := 2375 + i
 		dockerd := c.Container().From(fmt.Sprintf("docker:%s-dind", dockerVersion)).
 			WithMountedCache("/var/lib/docker", c.CacheVolume(t.Name()+"-"+dockerVersion+"-docker-lib"), dagger.ContainerWithMountedCacheOpts{
@@ -3891,50 +3969,58 @@ func TestContainerImageLoadCompatibility(t *testing.T) {
 		})
 		require.NoError(t, err)
 
-		for _, compression := range []dagger.ImageLayerCompression{dagger.Gzip, dagger.Zstd, dagger.Uncompressed} {
-			tmpdir := t.TempDir()
-			tmpfile := filepath.Join(tmpdir, fmt.Sprintf("test-%s-%s.tar", dockerVersion, compression))
-			_, err := c.Container().From("alpine:3.16.2").
-				// we need a unique image, otherwise docker load skips it after the first tar load
-				WithExec([]string{"sh", "-c", "echo " + string(compression) + " > /foo"}).
-				Export(ctx, tmpfile, dagger.ContainerExportOpts{
-					ForcedCompression: compression,
+		for _, mediaType := range []dagger.ImageMediaTypes{dagger.Ocimediatypes, dagger.Dockermediatypes} {
+			mediaType := mediaType
+			for _, compression := range []dagger.ImageLayerCompression{dagger.Gzip, dagger.Zstd, dagger.Uncompressed} {
+				compression := compression
+				t.Run(fmt.Sprintf("%s-%s-%s-%s", t.Name(), dockerVersion, mediaType, compression), func(t *testing.T) {
+					t.Parallel()
+					tmpdir := t.TempDir()
+					tmpfile := filepath.Join(tmpdir, fmt.Sprintf("test-%s-%s-%s.tar", dockerVersion, mediaType, compression))
+					_, err := c.Container().From("alpine:3.16.2").
+						// we need a unique image, otherwise docker load skips it after the first tar load
+						WithExec([]string{"sh", "-c", "echo '" + string(compression) + string(mediaType) + "' > /foo"}).
+						Export(ctx, tmpfile, dagger.ContainerExportOpts{
+							MediaTypes:        mediaType,
+							ForcedCompression: compression,
+						})
+					require.NoError(t, err)
+
+					randID := identity.NewID()
+					ctr := c.Container().From(fmt.Sprintf("docker:%s-cli", dockerVersion)).
+						WithEnvVariable("CACHEBUST", randID).
+						WithServiceBinding("docker", dockerd).
+						WithEnvVariable("DOCKER_HOST", dockerHost).
+						WithMountedCache("/tmp", c.CacheVolume(t.Name()+"-share-tmp")).
+						WithMountedFile(path.Join("/", path.Base(tmpfile)), c.Host().File(tmpfile)).
+						WithExec([]string{"cp", path.Join("/", path.Base(tmpfile)), "/tmp/"}, dagger.ContainerWithExecOpts{
+							SkipEntrypoint: true,
+						}).
+						WithExec([]string{"docker", "version"}).
+						WithExec([]string{"docker", "load", "-i", "/tmp/" + path.Base(tmpfile)})
+
+					output, err := ctr.Stdout(ctx)
+					if dockerVersion == "20.10" && compression == dagger.Zstd {
+						// zstd wasn't added until 23, so sanity check that it fails
+						require.Error(t, err)
+					} else {
+						require.NoError(t, err)
+						_, imageID, ok := strings.Cut(output, "sha256:")
+						require.True(t, ok)
+						imageID = strings.TrimSpace(imageID)
+
+						_, err = ctr.WithExec([]string{"docker", "run", "--rm", imageID, "echo", "hello"}).Sync(ctx)
+						require.NoError(t, err)
+					}
+
+					// also check that buildkit can load+run it too
+					_, err = c.Container().
+						Import(c.Host().File(tmpfile)).
+						WithExec([]string{"echo", "hello"}).
+						Sync(ctx)
+					require.NoError(t, err)
 				})
-			require.NoError(t, err)
-
-			randID := identity.NewID()
-			ctr := c.Container().From(fmt.Sprintf("docker:%s-cli", dockerVersion)).
-				WithEnvVariable("CACHEBUST", randID).
-				WithServiceBinding("docker", dockerd).
-				WithEnvVariable("DOCKER_HOST", dockerHost).
-				WithMountedCache("/tmp", c.CacheVolume(t.Name()+"-share-tmp")).
-				WithMountedFile(path.Join("/", path.Base(tmpfile)), c.Host().File(tmpfile)).
-				WithExec([]string{"cp", path.Join("/", path.Base(tmpfile)), "/tmp/"}, dagger.ContainerWithExecOpts{
-					SkipEntrypoint: true,
-				}).
-				WithExec([]string{"docker", "version"}).
-				WithExec([]string{"docker", "load", "-i", "/tmp/" + path.Base(tmpfile)})
-
-			output, err := ctr.Stdout(ctx)
-			if dockerVersion == "20.10" && compression == dagger.Zstd {
-				// zstd wasn't added until 23, so sanity check that it fails
-				require.Error(t, err)
-			} else {
-				require.NoError(t, err)
-				_, imageID, ok := strings.Cut(output, "sha256:")
-				require.True(t, ok)
-				imageID = strings.TrimSpace(imageID)
-
-				_, err = ctr.WithExec([]string{"docker", "run", "--rm", imageID, "echo", "hello"}).Sync(ctx)
-				require.NoError(t, err)
 			}
-
-			// also check that buildkit can load+run it too
-			_, err = c.Container().
-				Import(c.Host().File(tmpfile)).
-				WithExec([]string{"echo", "hello"}).
-				Sync(ctx)
-			require.NoError(t, err)
 		}
 	}
 }
diff --git a/core/schema/container.go b/core/schema/container.go
@@ -445,10 +445,11 @@ type containerPublishArgs struct {
 	Address           string
 	PlatformVariants  []core.ContainerID
 	ForcedCompression core.ImageLayerCompression
+	MediaTypes        core.ImageMediaTypes
 }
 
 func (s *containerSchema) publish(ctx *router.Context, parent *core.Container, args containerPublishArgs) (string, error) {
-	return parent.Publish(ctx, args.Address, args.PlatformVariants, args.ForcedCompression, s.bkClient, s.solveOpts, s.solveCh)
+	return parent.Publish(ctx, args.Address, args.PlatformVariants, args.ForcedCompression, args.MediaTypes, s.bkClient, s.solveOpts, s.solveCh)
 }
 
 type containerWithMountedFileArgs struct {
@@ -657,10 +658,11 @@ type containerExportArgs struct {
 	Path              string
 	PlatformVariants  []core.ContainerID
 	ForcedCompression core.ImageLayerCompression
+	MediaTypes        core.ImageMediaTypes
 }
 
 func (s *containerSchema) export(ctx *router.Context, parent *core.Container, args containerExportArgs) (bool, error) {
-	if err := parent.Export(ctx, s.host, args.Path, args.PlatformVariants, args.ForcedCompression, s.bkClient, s.solveOpts, s.solveCh); err != nil {
+	if err := parent.Export(ctx, s.host, args.Path, args.PlatformVariants, args.ForcedCompression, args.MediaTypes, s.bkClient, s.solveOpts, s.solveCh); err != nil {
 		return false, err
 	}
 

diff --git a/core/schema/container.graphqls b/core/schema/container.graphqls
@@ -650,6 +650,13 @@ type Container {
     engine's cache, then it will be compressed using Gzip.
     """
     forcedCompression: ImageLayerCompression
+
+    """
+    Use the specified media types for the published image's layers. Defaults to OCI, which
+    is largely compatible with most recent registries, but Docker may be needed for older
+    registries without OCI support.
+    """
+    mediaTypes: ImageMediaTypes = OCIMediaTypes
   ): String!
 
   """
@@ -679,6 +686,13 @@ type Container {
     engine's cache, then it will be compressed using Gzip.
     """
     forcedCompression: ImageLayerCompression
+
+    """
+    Use the specified media types for the exported image's layers. Defaults to OCI, which
+    is largely compatible with most recent container runtimes, but Docker may be needed 
+    for older runtimes without OCI support.
+    """
+    mediaTypes: ImageMediaTypes = OCIMediaTypes
   ): Boolean!
 
   """
@@ -867,10 +881,16 @@ enum NetworkProtocol {
   UDP
 }
 
-"Compression algorithm to use for image layers"
+"Compression algorithm to use for image layers."
 enum ImageLayerCompression {
   Gzip
   Zstd
   EStarGZ
   Uncompressed
 }
+
+"Mediatypes to use in published or exported image metadata."
+enum ImageMediaTypes {
+  OCIMediaTypes
+  DockerMediaTypes
+}
diff --git a/sdk/go/api.gen.go b/sdk/go/api.gen.go