containerimage: force oci-mediatypes for annotations/attestations

The new annotations and attestations features both utilize annotations in the oci image format. Many registries allow setting these annotation fields on docker formats, however, notably, GCR will reject these objects and only allow them for OCI media types. To work around this, we enable oci-mediatypes when annotations that require OCI are enabled by the user, printing a warning to the user, similar to how we already do for stargz compression. Signed-off-by: Justin Chadwell <[email protected]>
moby · Aug 24, 2022 · c40e12e · c40e12e
1 parent f2c770e
commit c40e12e
Show file tree

Hide file tree

Showing 5 changed files with 163 additions and 12 deletions.
diff --git a/client/client_test.go b/client/client_test.go
@@ -170,6 +170,7 @@ func TestIntegration(t *testing.T) {
 		testCallInfo,
 		testPullWithLayerLimit,
 		testExportAnnotations,
+		testExportAnnotationsMediaTypes,
 		testExportAttestations,
 	)
 	tests = append(tests, diffOpTestCases()...)
@@ -6257,6 +6258,127 @@ func testExportAnnotations(t *testing.T, sb integration.Sandbox) {
 	}
 }
 
+func testExportAnnotationsMediaTypes(t *testing.T, sb integration.Sandbox) {
+	requiresLinux(t)
+	c, err := New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	p := platforms.DefaultSpec()
+	ps := []ocispecs.Platform{p}
+
+	frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
+		res := gateway.NewResult()
+		expPlatforms := &exptypes.Platforms{
+			Platforms: make([]exptypes.Platform, len(ps)),
+		}
+		for i, p := range ps {
+			st := llb.Scratch().File(
+				llb.Mkfile("platform", 0600, []byte(platforms.Format(p))),
+			)
+
+			def, err := st.Marshal(ctx)
+			if err != nil {
+				return nil, err
+			}
+
+			r, err := c.Solve(ctx, gateway.SolveRequest{
+				Definition: def.ToPB(),
+			})
+			if err != nil {
+				return nil, err
+			}
+
+			ref, err := r.SingleRef()
+			if err != nil {
+				return nil, err
+			}
+
+			_, err = ref.ToState()
+			if err != nil {
+				return nil, err
+			}
+
+			k := platforms.Format(p)
+			res.AddRef(k, ref)
+
+			expPlatforms.Platforms[i] = exptypes.Platform{
+				ID:       k,
+				Platform: p,
+			}
+		}
+		dt, err := json.Marshal(expPlatforms)
+		if err != nil {
+			return nil, err
+		}
+		res.AddMeta(exptypes.ExporterPlatformsKey, dt)
+
+		return res, nil
+	}
+
+	// testing for image exporter
+
+	target := "testannotationsmedia:1"
+	_, err = c.Build(sb.Context(), SolveOpt{
+		Exports: []ExportEntry{
+			{
+				Type: ExporterImage,
+				Attrs: map[string]string{
+					"name":                  target,
+					"annotation-manifest.x": "y",
+				},
+			},
+		},
+	}, "", frontend, nil)
+	require.NoError(t, err)
+
+	target2 := "testannotationsmedia:2"
+	_, err = c.Build(sb.Context(), SolveOpt{
+		Exports: []ExportEntry{
+			{
+				Type: ExporterImage,
+				Attrs: map[string]string{
+					"name":               target2,
+					"annotation-index.x": "y",
+				},
+			},
+		},
+	}, "", frontend, nil)
+	require.NoError(t, err)
+
+	ctx := namespaces.WithNamespace(sb.Context(), "buildkit")
+	cdAddress := sb.ContainerdAddress()
+	if cdAddress != "" {
+		client, err := newContainerd(cdAddress)
+		require.NoError(t, err)
+		defer client.Close()
+
+		// test annotation in manifest
+		img, err := client.GetImage(ctx, target)
+		require.NoError(t, err)
+
+		var index ocispecs.Index
+		indexBytes, err := content.ReadBlob(ctx, client.ContentStore(), img.Target())
+		require.NoError(t, err)
+		require.NoError(t, json.Unmarshal(indexBytes, &index))
+		require.Equal(t, images.MediaTypeDockerSchema2ManifestList, index.MediaType)
+
+		mfst, err := images.Manifest(ctx, client.ContentStore(), img.Target(), platforms.Only(p))
+		require.NoError(t, err)
+		require.Equal(t, "y", mfst.Annotations["x"])
+
+		// test annotation in index
+		img, err = client.GetImage(ctx, target2)
+		require.NoError(t, err)
+
+		indexBytes, err = content.ReadBlob(ctx, client.ContentStore(), img.Target())
+		require.NoError(t, err)
+		require.NoError(t, json.Unmarshal(indexBytes, &index))
+		require.Equal(t, ocispecs.MediaTypeImageIndex, index.MediaType)
+		require.Equal(t, "y", index.Annotations["x"])
+	}
+}
+
 func testExportAttestations(t *testing.T, sb integration.Sandbox) {
 	requiresLinux(t)
 	c, err := New(sb.Context(), sb.Address())
@@ -6392,6 +6514,7 @@ func testExportAttestations(t *testing.T, sb integration.Sandbox) {
 	atts := index.Filter("unknown/unknown")
 	require.Equal(t, len(ps), len(atts))
 	for i, att := range atts {
+		require.Equal(t, ocispecs.MediaTypeImageManifest, att.Desc.MediaType)
 		require.Equal(t, "unknown/unknown", platforms.Format(*att.Desc.Platform))
 		require.Equal(t, "unknown/unknown", att.Img.OS+"/"+att.Img.Architecture)
 		require.Equal(t, attestation.DockerAnnotationReferenceTypeDefault, att.Desc.Annotations[attestation.DockerAnnotationReferenceType])

diff --git a/exporter/containerimage/export.go b/exporter/containerimage/export.go
@@ -76,8 +76,7 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp
 			RefCfg: cacheconfig.RefConfig{
 				Compression: compression.New(compression.Default),
 			},
-			BuildInfo:   true,
-			Annotations: make(AnnotationsGroup),
+			BuildInfo: true,
 		},
 		store: true,
 	}
@@ -208,7 +207,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source,
 	if err != nil {
 		return nil, err
 	}
-	opts.Annotations = as.Merge(opts.Annotations)
+	opts.AddAnnotations(as)
 
 	ctx, done, err := leaseutil.WithLease(ctx, e.opt.LeaseManager, leaseutil.MakeTemporary)
 	if err != nil {

diff --git a/exporter/containerimage/opts.go b/exporter/containerimage/opts.go
@@ -42,7 +42,6 @@ func (c *ImageCommitOpts) Load(opt map[string]string) (map[string]string, error)
 	if err != nil {
 		return nil, err
 	}
-	c.Annotations = as
 	opt = toStringMap(optb)
 
 	for k, v := range opt {
@@ -96,9 +95,36 @@ func (c *ImageCommitOpts) Load(opt map[string]string) (map[string]string, error)
 		c.OCITypes = true
 	}
 
+	c.AddAnnotations(as)
+
 	return rest, nil
 }
 
+func (c *ImageCommitOpts) AddAnnotations(annotations AnnotationsGroup) {
+	if annotations == nil {
+		return
+	}
+	if c.Annotations == nil {
+		c.Annotations = AnnotationsGroup{}
+	}
+	c.Annotations = c.Annotations.Merge(annotations)
+	for _, a := range annotations {
+		if len(a.Index)+len(a.IndexDescriptor)+len(a.ManifestDescriptor) > 0 {
+			if !c.OCITypes {
+				logrus.Warn("forcibly turning on oci-mediatype mode for annotations")
+				c.OCITypes = true
+			}
+		}
+	}
+}
+
+func (c *ImageCommitOpts) EnableAttestations() {
+	if !c.OCITypes {
+		logrus.Warn("forcibly turning on oci-mediatype mode for attestations")
+		c.OCITypes = true
+	}
+}
+
 func parseBool(dest *bool, key string, value string) error {
 	b, err := strconv.ParseBool(value)
 	if err != nil {

diff --git a/exporter/containerimage/writer.go b/exporter/containerimage/writer.go
@@ -98,12 +98,16 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp exporter.Source, sessionI
 		return mfstDesc, nil
 	}
 
-	refCount := len(p.Platforms)
+	attestCount := 0
 	for _, attests := range inp.Attestations {
-		refCount += len(attests)
+		attestCount += len(attests)
 	}
-	if refCount != len(inp.Refs) {
-		return nil, errors.Errorf("number of required refs does not match references %d %d", refCount, len(inp.Refs))
+	if count := attestCount + len(p.Platforms); count != len(inp.Refs) {
+		return nil, errors.Errorf("number of required refs does not match references %d %d", count, len(inp.Refs))
+	}
+
+	if attestCount > 0 {
+		opts.EnableAttestations()
 	}
 
 	refs := make([]cache.ImmutableRef, 0, len(inp.Refs))

diff --git a/exporter/oci/export.go b/exporter/oci/export.go
@@ -57,9 +57,8 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp
 			RefCfg: cacheconfig.RefConfig{
 				Compression: compression.New(compression.Default),
 			},
-			BuildInfo:   true,
-			OCITypes:    e.opt.Variant == VariantOCI,
-			Annotations: make(containerimage.AnnotationsGroup),
+			BuildInfo: true,
+			OCITypes:  e.opt.Variant == VariantOCI,
 		},
 	}
 
@@ -110,7 +109,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source,
 	if err != nil {
 		return nil, err
 	}
-	opts.Annotations = as.Merge(opts.Annotations)
+	opts.AddAnnotations(as)
 
 	ctx, done, err := leaseutil.WithLease(ctx, e.opt.LeaseManager, leaseutil.MakeTemporary)
 	if err != nil {