From cf3acdcf8691f6eec6dee1d14d4a632db9c1aa53 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Tue, 13 Feb 2024 11:06:04 -0500 Subject: [PATCH] libimage.ManifestList: add AddArtifact() * Add a libimage.ManifestList.AddArtifact() method. * Add IndexAnnotations and Subject to libimage.ManifestListAnnotateOptions, for setting annotations on the index itself, and the index's subject, respectively. * In libimage/manifests.list.AddArtifact(), if the subject has an artifactType in its manifest, add its value to the subject descriptor. Signed-off-by: Nalin Dahyabhai --- libimage/manifest_list.go | 155 +++++++++++++++++- libimage/manifest_list_test.go | 273 +++++++++++++++++++++++++++++++- libimage/manifests/manifests.go | 14 +- 3 files changed, 434 insertions(+), 8 deletions(-) diff --git a/libimage/manifest_list.go b/libimage/manifest_list.go index 7a4eacc47..d7ee5e6b6 100644 --- a/libimage/manifest_list.go +++ b/libimage/manifest_list.go @@ -19,6 +19,7 @@ import ( structcopier "github.com/jinzhu/copier" "github.com/opencontainers/go-digest" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" + "golang.org/x/exp/maps" "golang.org/x/exp/slices" ) @@ -294,7 +295,7 @@ func (m *ManifestList) Inspect() (*define.ManifestListData, error) { return &inspectList, nil } -// Options for adding a manifest list. +// Options for adding an image or artifact to a manifest list. type ManifestListAddOptions struct { // Add all images to the list if the to-be-added image itself is a // manifest list. @@ -371,6 +372,104 @@ func (m *ManifestList) Add(ctx context.Context, name string, options *ManifestLi return newDigest, nil } +// Options for creating an artifact manifest for one or more files and adding +// the artifact manifest to a manifest list. +type ManifestListAddArtifactOptions struct { + // The artifactType to set in the artifact manifest. + Type *string `json:"artifact_type"` + // The mediaType to set in the config.MediaType field in the artifact manifest. + ConfigType string `json:"artifact_config_type"` + // Content to point to from the config field in the artifact manifest. + Config string `json:"artifact_config"` + // The mediaType to set in the layer descriptors in the artifact manifest. + LayerType string `json:"artifact_layer_type"` + // Whether or not to suppress the org.opencontainers.image.title annotation in layer descriptors. + ExcludeTitles bool `json:"exclude_layer_titles"` + // Annotations to set in the artifact manifest. + Annotations map[string]string `json:"annotations"` + // Subject to set in the artifact manifest. + Subject string `json:"subject"` +} + +// Add adds one or more manifests to the manifest list and returns the digest +// of the added instance. +func (m *ManifestList) AddArtifact(ctx context.Context, options *ManifestListAddArtifactOptions, files ...string) (digest.Digest, error) { + if options == nil { + options = &ManifestListAddArtifactOptions{} + } + opts := manifests.AddArtifactOptions{ + ManifestArtifactType: options.Type, + Annotations: maps.Clone(options.Annotations), + ExcludeTitles: options.ExcludeTitles, + } + if options.ConfigType != "" { + opts.ConfigDescriptor = &imgspecv1.Descriptor{ + MediaType: options.ConfigType, + Digest: imgspecv1.DescriptorEmptyJSON.Digest, + Size: imgspecv1.DescriptorEmptyJSON.Size, + Data: slices.Clone(imgspecv1.DescriptorEmptyJSON.Data), + } + } + if options.Config != "" { + if opts.ConfigDescriptor == nil { + opts.ConfigDescriptor = &imgspecv1.Descriptor{ + MediaType: imgspecv1.MediaTypeImageConfig, + } + } + opts.ConfigDescriptor.Digest = digest.FromString(options.Config) + opts.ConfigDescriptor.Size = int64(len(options.Config)) + opts.ConfigDescriptor.Data = slices.Clone([]byte(options.Config)) + } + if opts.ConfigDescriptor == nil { + empty := imgspecv1.DescriptorEmptyJSON + opts.ConfigDescriptor = &empty + } + if options.LayerType != "" { + opts.LayerMediaType = &options.LayerType + } + if options.Subject != "" { + ref, err := alltransports.ParseImageName(options.Subject) + if err != nil { + withDocker := fmt.Sprintf("%s://%s", docker.Transport.Name(), options.Subject) + ref, err = alltransports.ParseImageName(withDocker) + if err != nil { + image, _, err := m.image.runtime.LookupImage(options.Subject, &LookupImageOptions{ManifestList: true}) + if err != nil { + return "", fmt.Errorf("locating subject for artifact manifest: %w", err) + } + ref = image.storageReference + } + } + opts.SubjectReference = ref + } + + // Lock the image record where this list lives. + locker, err := manifests.LockerForImage(m.image.runtime.store, m.ID()) + if err != nil { + return "", err + } + locker.Lock() + defer locker.Unlock() + + systemContext := m.image.runtime.systemContextCopy() + + // Make sure to reload the image from the containers storage to fetch + // the latest data (e.g., new or delete digests). + if err := m.reload(); err != nil { + return "", err + } + newDigest, err := m.list.AddArtifact(ctx, systemContext, opts, files...) + if err != nil { + return "", err + } + + // Write the changes to disk. + if err := m.saveAndReload(); err != nil { + return "", err + } + return newDigest, nil +} + // Options for annotating a manifest list. type ManifestListAnnotateOptions struct { // Add the specified annotations to the added image. @@ -387,10 +486,16 @@ type ManifestListAnnotateOptions struct { OSVersion string // Add the specified variant to the added image. Variant string + // Add the specified annotations to the index itself. + IndexAnnotations map[string]string + // Set the subject to which the index refers. + Subject string } // Annotate an image instance specified by `d` in the manifest list. func (m *ManifestList) AnnotateInstance(d digest.Digest, options *ManifestListAnnotateOptions) error { + ctx := context.Background() + if options == nil { return nil } @@ -430,6 +535,54 @@ func (m *ManifestList) AnnotateInstance(d digest.Digest, options *ManifestListAn return err } } + if len(options.IndexAnnotations) > 0 { + if err := m.list.SetAnnotations(nil, options.IndexAnnotations); err != nil { + return err + } + } + if options.Subject != "" { + ref, err := alltransports.ParseImageName(options.Subject) + if err != nil { + withDocker := fmt.Sprintf("%s://%s", docker.Transport.Name(), options.Subject) + ref, err = alltransports.ParseImageName(withDocker) + if err != nil { + image, _, err := m.image.runtime.LookupImage(options.Subject, &LookupImageOptions{ManifestList: true}) + if err != nil { + return fmt.Errorf("locating subject for image index: %w", err) + } + ref = image.storageReference + } + } + src, err := ref.NewImageSource(ctx, &m.image.runtime.systemContext) + if err != nil { + return err + } + defer src.Close() + subjectManifestBytes, subjectManifestType, err := src.GetManifest(ctx, nil) + if err != nil { + return err + } + subjectManifestDigest, err := manifest.Digest(subjectManifestBytes) + if err != nil { + return err + } + var subjectArtifactType string + if !manifest.MIMETypeIsMultiImage(subjectManifestType) { + var subjectManifest imgspecv1.Manifest + if json.Unmarshal(subjectManifestBytes, &subjectManifest) == nil { + subjectArtifactType = subjectManifest.ArtifactType + } + } + descriptor := &imgspecv1.Descriptor{ + MediaType: subjectManifestType, + ArtifactType: subjectArtifactType, + Digest: subjectManifestDigest, + Size: int64(len(subjectManifestBytes)), + } + if err := m.list.SetSubject(descriptor); err != nil { + return err + } + } // Write the changes to disk. return m.saveAndReload() diff --git a/libimage/manifest_list_test.go b/libimage/manifest_list_test.go index 0dbb1fc02..5fbd9ae23 100644 --- a/libimage/manifest_list_test.go +++ b/libimage/manifest_list_test.go @@ -3,13 +3,31 @@ package libimage import ( + "bytes" "context" + "crypto/rand" "errors" + "fmt" + "io" + mathrand "math/rand" + "mime" + "net/http" + "os" "path/filepath" + "strconv" + "strings" "testing" "github.com/containers/common/pkg/config" + cp "github.com/containers/image/v5/copy" + "github.com/containers/image/v5/manifest" + "github.com/containers/image/v5/pkg/compression" + "github.com/containers/image/v5/transports/alltransports" "github.com/containers/storage" + "github.com/containers/storage/pkg/ioutils" + "github.com/opencontainers/go-digest" + imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -67,6 +85,12 @@ func TestInspectManifestListWithAnnotations(t *testing.T) { annotateOptions := ManifestListAnnotateOptions{} annotations := map[string]string{"hello": "world"} annotateOptions.Annotations = annotations + indexAnnotations := map[string]string{"goodbye": "globe"} + annotateOptions.IndexAnnotations = indexAnnotations + + subjectPath, err := filepath.Abs(filepath.Join("..", "pkg", "manifests", "testdata", "artifacts", "blobs-only")) + require.NoError(t, err) + annotateOptions.Subject = "oci:" + subjectPath err = list.AnnotateInstance(digest, &annotateOptions) require.NoError(t, err) @@ -75,6 +99,8 @@ func TestInspectManifestListWithAnnotations(t *testing.T) { require.NoError(t, err) // verify annotation require.Equal(t, inspectReport.Manifests[0].Annotations, annotations) + require.Equal(t, inspectReport.Annotations, indexAnnotations) + require.Equal(t, inspectReport.Subject.MediaType, imgspecv1.MediaTypeImageManifest) } // Following test ensure that `Tag` tags the manifest list instead of resolved image. @@ -149,10 +175,10 @@ func TestCreateAndRemoveManifestList(t *testing.T) { require.Equal(t, []string{"localhost/manifestlist:latest"}, rmReports[0].Untagged) } -// TestAddArtifacts ensures that we don't fail to add artifact manifests to a -// manifest list, even (or especially) when their config blobs aren't valid OCI -// or Docker config blobs. -func TestAddArtifacts(t *testing.T) { +// TestAddSomeArtifacts ensures that we don't fail to add artifact manifests to +// a manifest list, even (or especially) when their config blobs aren't valid +// OCI or Docker config blobs. +func TestAddSomeArtifacts(t *testing.T) { listName := "manifestlist" runtime := testNewRuntime(t) ctx := context.Background() @@ -177,3 +203,242 @@ func TestAddArtifacts(t *testing.T) { _, err = list.Add(ctx, "oci:"+absPath, manifestListOpts) require.NoError(t, err) } + +// TestAddArtifacts ensures that we don't fail to add artifact manifests to +// a manifest list, even (or especially) when their config blobs aren't valid +// OCI or Docker config blobs. +func TestAddArtifacts(t *testing.T) { + listName := "manifestlist" + ctx := context.Background() + dir := t.TempDir() + annotations := map[string]string{ + "a": "b", + } + indexAnnotations := map[string]string{ + "c": "d", + } + files := []struct { + path string + size int + data []byte + noCompress bool + guessedMediaType string // what we expect, might be wrong + }{ + {path: "first.txt", size: mathrand.Intn(256), guessedMediaType: "text/plain"}, + {path: "second.qcow2", size: 512 + mathrand.Intn(256), guessedMediaType: "application/x-qemu-disk"}, + {path: "third", size: 1024 + mathrand.Intn(256), guessedMediaType: "application/x-gzip"}, + {path: "fourth", size: 2048 + mathrand.Intn(256), noCompress: true, guessedMediaType: "application/octet-stream"}, + } + artifacts := make([]string, 0, len(files)) + for n := range files { + file := filepath.Join(dir, files[n].path) + abs, err := filepath.Abs(file) + require.NoError(t, err) + files[n].path = abs + if files[n].data == nil { + buf := bytes.Buffer{} + wc := ioutils.NopWriteCloser(&buf) + if !files[n].noCompress { + wc, err = compression.CompressStream(&buf, compression.Gzip, nil) + require.NoError(t, err) + } + _, err = io.CopyN(wc, rand.Reader, int64(files[n].size)) + require.NoError(t, err) + wc.Close() + files[n].size = buf.Len() + files[n].data = buf.Bytes() + } + err = os.WriteFile(abs, files[n].data, 0o600) + require.NoError(t, err) + artifacts = append(artifacts, abs) + } + artifactSubjectPath, err := filepath.Abs(filepath.Join("..", "pkg", "manifests", "testdata", "artifacts", "blobs-only")) + require.NoError(t, err) + artifactSubject := "oci:" + artifactSubjectPath + indexSubjectPath, err := filepath.Abs(filepath.Join("..", "pkg", "manifests", "testdata", "artifacts", "config-only")) + require.NoError(t, err) + indexSubject := "oci:" + indexSubjectPath + runtime := testNewRuntime(t) + descriptorForSubject := func(t *testing.T, refName string) imgspecv1.Descriptor { + if refName == "" { + return imgspecv1.Descriptor{} + } + ref, err := alltransports.ParseImageName(refName) + require.NoError(t, err) + src, err := ref.NewImageSource(ctx, nil) + require.NoError(t, err) + defer src.Close() + manifestBytes, manifestType, err := src.GetManifest(ctx, nil) + require.NoError(t, err) + manifestDigest, err := manifest.Digest(manifestBytes) + require.NoError(t, err) + artifactType := "" + if !manifest.MIMETypeIsMultiImage(manifestType) { + var manifestContents imgspecv1.Manifest + require.NoError(t, json.Unmarshal(manifestBytes, &manifestContents)) + artifactType = manifestContents.ArtifactType + } + return imgspecv1.Descriptor{ + MediaType: manifestType, + ArtifactType: artifactType, + Digest: manifestDigest, + Size: int64(len(manifestBytes)), + } + } + listIndex := 0 + testWith := func(t *testing.T, testName string, artifactTypeSpec string, configType string, configData string, layerType string, excludeTitles bool, artifactSubject string, artifactSubjectDescriptor imgspecv1.Descriptor, indexSubject string, indexSubjectDescriptor imgspecv1.Descriptor) { + listIndex++ + listName := listName + strconv.Itoa(listIndex) + t.Run(testName, func(t *testing.T) { + var artifactType *string + if artifactTypeSpec != "" { + artifactType = &artifactTypeSpec + } + options := ManifestListAddArtifactOptions{ + Type: artifactType, + ConfigType: configType, + Config: configData, + LayerType: layerType, + ExcludeTitles: excludeTitles, + Annotations: annotations, + Subject: artifactSubject, + } + list, err := runtime.CreateManifestList(listName) + require.NoError(t, err) + require.NotNil(t, list) + + d, err := list.AddArtifact(ctx, &options, artifacts...) + require.NoError(t, err) + + aoptions := ManifestListAnnotateOptions{ + IndexAnnotations: indexAnnotations, + Subject: indexSubject, + } + err = list.AnnotateInstance(d, &aoptions) + require.NoError(t, err) + + destination, err := os.MkdirTemp(dir, "pushed") + require.NoError(t, err) + + _, err = list.Push(ctx, "oci:"+destination+":tag", &ManifestListPushOptions{ImageListSelection: cp.CopyAllImages}) + require.NoError(t, err) + + ref, err := alltransports.ParseImageName("oci:" + destination + ":tag") + require.NoError(t, err) + + src, err := ref.NewImageSource(ctx, list.image.runtime.systemContextCopy()) + require.NoError(t, err) + indexManifest, indexType, err := src.GetManifest(ctx, nil) + require.NoError(t, err) + require.True(t, manifest.MIMETypeIsMultiImage(indexType)) + var index imgspecv1.Index + require.NoError(t, json.Unmarshal(indexManifest, &index)) + // check some things in the image index + assert.Equal(t, index.Annotations, indexAnnotations) + if index.Subject != nil { + assert.Equal(t, indexSubjectDescriptor, *index.Subject, "subject in index was not preserved") + } + for _, descriptor := range index.Manifests { + artifactManifest, artifactManifestType, err := src.GetManifest(ctx, &descriptor.Digest) + require.NoError(t, err) + require.False(t, manifest.MIMETypeIsMultiImage(artifactManifestType)) + var artifact imgspecv1.Manifest + require.NoError(t, json.Unmarshal(artifactManifest, &artifact)) + // check some things in the artifact manifest + switch artifactTypeSpec { + case "": + assert.Equal(t, "application/vnd.unknown.artifact.v1", artifact.ArtifactType) + default: + assert.Equal(t, *artifactType, artifact.ArtifactType) + } + // FIXME: require.Equal(t, artifact.ArtifactType, descriptor.ArtifactType, "artifact type in index descriptor not preserved during push") + switch configType { + case "": + if len(configData) > 0 { + assert.Equal(t, imgspecv1.MediaTypeImageConfig, artifact.Config.MediaType) + } else { + assert.Equal(t, imgspecv1.DescriptorEmptyJSON.MediaType, artifact.Config.MediaType) + } + default: + assert.Equal(t, configType, artifact.Config.MediaType) + } + for i, layer := range artifact.Layers { + switch layerType { + case "": + var rawMediaType string + baseName := filepath.Base(files[i].path) + if dotIndex := strings.LastIndex(filepath.Base(files[i].path), "."); dotIndex != -1 { + rawMediaType = mime.TypeByExtension(baseName[dotIndex:]) + } else { + rawMediaType = http.DetectContentType(files[i].data) + } + parsedMediaType, _, err := mime.ParseMediaType(rawMediaType) + require.NoError(t, err) + assert.Equal(t, files[i].guessedMediaType, parsedMediaType) + default: + assert.Equal(t, layerType, layer.MediaType) + } + if excludeTitles { + assert.NotContains(t, layer.Annotations, imgspecv1.AnnotationTitle) + // FIXME: } else { + // FIXME: require.Contains(t, layer.Annotations, imgspecv1.AnnotationTitle, "layer annotations lost during push") + // FIXME: assert.Equal(t, filepath.Base(files[i].path), layer.Annotations[imgspecv1.AnnotationTitle], "layer annotations lost during push") + } + if layer.MediaType != imgspecv1.MediaTypeImageLayerGzip { // might have been (re)compressed + assert.Equal(t, digest.FromBytes(files[i].data), layer.Digest, "layer content digest changed during push") + assert.Equal(t, int64(len(files[i].data)), layer.Size, "layer content size changed during push") + } + if artifact.Subject != nil { + assert.Equal(t, artifactSubjectDescriptor, *artifact.Subject) + } + } + } + }) + } + for _, artifactTypeSpec := range []string{ + "", + "", + "application/vnd.unknown.artifact.v1", + "application/x-something-else", + } { + testName := "artifactType=" + artifactTypeSpec + for _, configType := range []string{ + "", + imgspecv1.MediaTypeImageConfig, + imgspecv1.DescriptorEmptyJSON.MediaType, + } { + testName := testName + ",configType=" + configType + for _, configData := range []string{ + "", + `{"a":"b"}`, + } { + testName := testName + ",configLength=" + strconv.Itoa(len(configData)) + for _, layerType := range []string{ + "", + "application/octet-stream", + imgspecv1.MediaTypeImageLayerGzip, + } { + testName := testName + ",layerType=" + layerType + for _, excludeTitles := range []bool{false, true} { + testName := testName + ",excludeTitles=" + fmt.Sprintf("%v", excludeTitles) + for _, artifactSubject := range []string{"", artifactSubject} { + testName := testName + ",artifactSubject=" + if artifactSubject != "" { + testName += filepath.Base(artifactSubjectPath) + } + artifactSubjectDescriptor := descriptorForSubject(t, artifactSubject) + for _, indexSubject := range []string{"", indexSubject} { + testName := testName + ",indexSubject=" + if indexSubject != "" { + testName += filepath.Base(indexSubjectPath) + } + indexSubjectDescriptor := descriptorForSubject(t, indexSubject) + testWith(t, testName, artifactTypeSpec, configType, configData, layerType, excludeTitles, artifactSubject, artifactSubjectDescriptor, indexSubject, indexSubjectDescriptor) + } + } + } + } + } + } + } +} diff --git a/libimage/manifests/manifests.go b/libimage/manifests/manifests.go index aab804538..594ce437b 100644 --- a/libimage/manifests/manifests.go +++ b/libimage/manifests/manifests.go @@ -711,10 +711,18 @@ func (l *list) AddArtifact(ctx context.Context, sys *types.SystemContext, option if err != nil { return "", fmt.Errorf("digesting manifest of subject %q: %w", transports.ImageName(options.SubjectReference), err) } + var subjectArtifactType string + if !manifest.MIMETypeIsMultiImage(subjectManifestType) { + var subjectManifest v1.Manifest + if json.Unmarshal(subjectManifestBytes, &subjectManifest) == nil { + subjectArtifactType = subjectManifest.ArtifactType + } + } subject = &v1.Descriptor{ - MediaType: subjectManifestType, - Digest: subjectManifestDigest, - Size: int64(len(subjectManifestBytes)), + MediaType: subjectManifestType, + ArtifactType: subjectArtifactType, + Digest: subjectManifestDigest, + Size: int64(len(subjectManifestBytes)), } }