Skip to content

Commit

Permalink
Add manifest list support
Browse files Browse the repository at this point in the history
Add the manifest.List interface, and implementations for OCIv1 Index and
Docker Schema2List documents.

Add an instanceDigest parameter to PutManifest(), PutSignatures(), and
LayerInfosForCopy, for symmetry with GetManifest() and GetSignatures().
Return an error if the instanceDigest is supplied to destinations which
don't support them, and add stubs that do so even to the transports
which would support it, so that we don't break compilation here.

Add a MultipleImages flag to copy.Options, and if the source for a copy
operation contains multiple images, copy all of the images if we can.
If we can't copy them all, but we were told to, return an error.

Use the generic manifest list API to select a single image to copy from
a list, so that we aren't just limited to the Docker manifest list
format for those cases.

When guessing at the type of a manifest, if the manifest contains a list
of manifests, use its declared MIME type if it included one, else assume
it's an OCI index, because an OCI index doesn't include its MIME type.

When copying, switch from using an encode-then-compare of the original
and updated versions of the list to checking if the instance list was
changed (one of the things we might have changed) or if its type has
changed due to conversion (the other change we might have made).  If
neither has changed, then we don't need to change the encoded value of
the manifest.

When copying, when checking for a digest mismatch in a target image
reference, ignore a mismatch between the digest in the reference and the
digest of the main manifest if we're copying one element from a list,
and the digest in the reference matches the digest of the manifest list.

When copying, if conversion of manifests for single images is being
forced, convert manifest lists to the corresponding list types.

When copying, supply the unparsed top level to Commit() by attaching the
value to the context.Context.

Support manifest lists in the directory transport by using the instance
digest as a prefix of the filename used to store a manifest or a piece
of signature data.

Support manifest lists in the oci-layout transport by accepting indexes
as we do images, and stop guessing about Platform values to add to the
top-level index.

Support storing manifest lists to registries in the docker: transport by
using the manifest digest when we're writing one image as part of
pushing a list of them, and by using the instance digest when reading or
writing signature data, when one is specified, or the cached digest of
the non-instanced digest when one is not specified.

Add partial support for manifest lists to the storage transport: when
committing one image from a list into storage, also add a copy of the
manifest list by extracting it from the context.Context.  The logic is
already in place to enable locating an image using any of multiple
manifest digests.

When writing an image that has an instanceDigest value (meaning it's a
secondary image), don't try to generate a canonical reference to add to
the image's list of names if the reference for the primary image doesn't
contain a name.  That should only happen if we're writing using just an
image ID, which is unlikely, but we still need to handle it.

Avoid computing the digest of the manifest, or retrieving the
either-a-tag-or-a-digest value from the target reference, if we're given
an instanceDigest, which would override them anyway.

Move the check for non-nil instanceDigest values up into the main
PutSignatures() method instead of duplicating it in the per-strategy
helpers.

Add mention of the instanceDigest parameter and its use to various
PutManifest, PutSignatures, and LayerInfosForCopy implementations and
their declarations in interfaces.

Signed-off-by: Nalin Dahyabhai <[email protected]>
  • Loading branch information
nalind committed Oct 18, 2019
1 parent 9da78d1 commit d0337d2
Show file tree
Hide file tree
Showing 44 changed files with 1,870 additions and 468 deletions.
345 changes: 297 additions & 48 deletions copy/copy.go

Large diffs are not rendered by default.

33 changes: 33 additions & 0 deletions copy/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,36 @@ func isMultiImage(ctx context.Context, img types.UnparsedImage) (bool, error) {
}
return manifest.MIMETypeIsMultiImage(mt), nil
}

// determineListConversion takes the current MIME type of a list of manifests,
// the list of MIME types supported for a given destination, and a possible
// forced value, and returns the MIME type to which we should convert the list
// of manifests, whether we are converting to it or using it unmodified.
func (c *copier) determineListConversion(currentListMIMEType string, destSupportedMIMETypes []string, forcedListMIMEType string) (string, error) {
// If there's no list of supported types, then anything we support is expected to be supported.
if len(destSupportedMIMETypes) == 0 {
destSupportedMIMETypes = manifest.SupportedListMIMETypes
}
var selectedType string
for i := range destSupportedMIMETypes {
// The second priority is the first member of the list of acceptable types that is a list,
// but keep going in case current type occurs later in the list.
if selectedType == "" && manifest.MIMETypeIsMultiImage(destSupportedMIMETypes[i]) {
selectedType = destSupportedMIMETypes[i]
}
// The first priority is the current type, if it's in the list, since that lets us avoid a
// conversion that isn't strictly necessary.
if destSupportedMIMETypes[i] == currentListMIMEType {
selectedType = destSupportedMIMETypes[i]
}
}
if forcedListMIMEType != "" {
// If we're forcing it, we prefer the forced value over everything else.
selectedType = forcedListMIMEType
}
if selectedType == "" {
return "", errors.Errorf("destination does not support any supported manifest list types (%v)", manifest.SupportedListMIMETypes)
}
// Done.
return selectedType, nil
}
2 changes: 2 additions & 0 deletions copy/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ func TestIsMultiImage(t *testing.T) {
}{
{manifest.DockerV2ListMediaType, true},
{manifest.DockerV2Schema2MediaType, false},
{v1.MediaTypeImageManifest, false},
{v1.MediaTypeImageIndex, true},
} {
src := fakeImageSource(c.mt)
res, err := isMultiImage(context.Background(), src)
Expand Down
17 changes: 12 additions & 5 deletions directory/directory_dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,16 +199,23 @@ func (d *dirImageDestination) TryReusingBlob(ctx context.Context, info types.Blo
}

// PutManifest writes manifest to the destination.
// If instanceDigest is not nil, it contains a digest of the specific manifest instance to write the manifest for (when
// the primary manifest is a manifest list); this should always be nil if the primary manifest is not a manifest list.
// It is expected but not enforced that the instanceDigest, when specified, matches the digest of `manifest` as generated
// by `manifest.Digest()`.
// FIXME? This should also receive a MIME type if known, to differentiate between schema versions.
// If the destination is in principle available, refuses this manifest type (e.g. it does not recognize the schema),
// but may accept a different manifest type, the returned error must be an ManifestTypeRejectedError.
func (d *dirImageDestination) PutManifest(ctx context.Context, manifest []byte) error {
return ioutil.WriteFile(d.ref.manifestPath(), manifest, 0644)
func (d *dirImageDestination) PutManifest(ctx context.Context, manifest []byte, instanceDigest *digest.Digest) error {
return ioutil.WriteFile(d.ref.manifestPath(instanceDigest), manifest, 0644)
}

func (d *dirImageDestination) PutSignatures(ctx context.Context, signatures [][]byte) error {
// PutSignatures writes a set of signatures to the destination.
// If instanceDigest is not nil, it contains a digest of the specific manifest instance to write or overwrite the signatures for
// (when the primary manifest is a manifest list); this should always be nil if the primary manifest is not a manifest list.
func (d *dirImageDestination) PutSignatures(ctx context.Context, signatures [][]byte, instanceDigest *digest.Digest) error {
for i, sig := range signatures {
if err := ioutil.WriteFile(d.ref.signaturePath(i), sig, 0644); err != nil {
if err := ioutil.WriteFile(d.ref.signaturePath(i, instanceDigest), sig, 0644); err != nil {
return err
}
}
Expand All @@ -219,7 +226,7 @@ func (d *dirImageDestination) PutSignatures(ctx context.Context, signatures [][]
// WARNING: This does not have any transactional semantics:
// - Uploaded data MAY be visible to others before Commit() is called
// - Uploaded data MAY be removed or MAY remain around if Close() is called without Commit() (i.e. rollback is allowed but not guaranteed)
func (d *dirImageDestination) Commit(ctx context.Context) error {
func (d *dirImageDestination) Commit(context.Context, types.UnparsedImage) error {
return nil
}

Expand Down
22 changes: 11 additions & 11 deletions directory/directory_src.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/containers/image/v4/manifest"
"github.com/containers/image/v4/types"
"github.com/opencontainers/go-digest"
"github.com/pkg/errors"
)

type dirImageSource struct {
Expand Down Expand Up @@ -38,10 +37,7 @@ func (s *dirImageSource) Close() error {
// If instanceDigest is not nil, it contains a digest of the specific manifest instance to retrieve (when the primary manifest is a manifest list);
// this never happens if the primary manifest is not a manifest list (e.g. if the source never returns manifest lists).
func (s *dirImageSource) GetManifest(ctx context.Context, instanceDigest *digest.Digest) ([]byte, string, error) {
if instanceDigest != nil {
return nil, "", errors.Errorf(`Getting target manifest not supported by "dir:"`)
}
m, err := ioutil.ReadFile(s.ref.manifestPath())
m, err := ioutil.ReadFile(s.ref.manifestPath(instanceDigest))
if err != nil {
return nil, "", err
}
Expand Down Expand Up @@ -73,12 +69,9 @@ func (s *dirImageSource) GetBlob(ctx context.Context, info types.BlobInfo, cache
// (when the primary manifest is a manifest list); this never happens if the primary manifest is not a manifest list
// (e.g. if the source never returns manifest lists).
func (s *dirImageSource) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) {
if instanceDigest != nil {
return nil, errors.Errorf(`Manifests lists are not supported by "dir:"`)
}
signatures := [][]byte{}
for i := 0; ; i++ {
signature, err := ioutil.ReadFile(s.ref.signaturePath(i))
signature, err := ioutil.ReadFile(s.ref.signaturePath(i, instanceDigest))
if err != nil {
if os.IsNotExist(err) {
break
Expand All @@ -90,7 +83,14 @@ func (s *dirImageSource) GetSignatures(ctx context.Context, instanceDigest *dige
return signatures, nil
}

// LayerInfosForCopy() returns updated layer info that should be used when copying, in preference to values in the manifest, if specified.
func (s *dirImageSource) LayerInfosForCopy(ctx context.Context) ([]types.BlobInfo, error) {
// LayerInfosForCopy returns either nil (meaning the values in the manifest are fine), or updated values for the layer
// blobsums that are listed in the image's manifest. If values are returned, they should be used when using GetBlob()
// to read the image's layers.
// If instanceDigest is not nil, it contains a digest of the specific manifest instance to retrieve BlobInfos for
// (when the primary manifest is a manifest list); this never happens if the primary manifest is not a manifest list
// (e.g. if the source never returns manifest lists).
// The Digest field is guaranteed to be provided; Size may be -1.
// WARNING: The list may contain duplicates, and they are semantically relevant.
func (s *dirImageSource) LayerInfosForCopy(context.Context, *digest.Digest) ([]types.BlobInfo, error) {
return nil, nil
}
57 changes: 38 additions & 19 deletions directory/directory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,31 @@ func TestGetPutManifest(t *testing.T) {
defer os.RemoveAll(tmpDir)

man := []byte("test-manifest")
list := []byte("test-manifest-list")
md, err := manifest.Digest(man)
require.NoError(t, err)
dest, err := ref.NewImageDestination(context.Background(), nil)
require.NoError(t, err)
defer dest.Close()
err = dest.PutManifest(context.Background(), man)
err = dest.PutManifest(context.Background(), man, &md)
assert.NoError(t, err)
err = dest.Commit(context.Background())
err = dest.PutManifest(context.Background(), list, nil)
assert.NoError(t, err)
err = dest.Commit(context.Background(), nil)
assert.NoError(t, err)

src, err := ref.NewImageSource(context.Background(), nil)
require.NoError(t, err)
defer src.Close()
m, mt, err := src.GetManifest(context.Background(), nil)
assert.NoError(t, err)
assert.Equal(t, man, m)
assert.Equal(t, list, m)
assert.Equal(t, "", mt)

// Non-default instances are not supported
md, err := manifest.Digest(man)
require.NoError(t, err)
_, _, err = src.GetManifest(context.Background(), &md)
assert.Error(t, err)
m, mt, err = src.GetManifest(context.Background(), &md)
assert.NoError(t, err)
assert.Equal(t, man, m)
assert.Equal(t, "", mt)
}

func TestGetPutBlob(t *testing.T) {
Expand All @@ -67,7 +71,7 @@ func TestGetPutBlob(t *testing.T) {
assert.Equal(t, types.PreserveOriginal, dest.DesiredLayerCompression())
info, err := dest.PutBlob(context.Background(), bytes.NewReader(blob), types.BlobInfo{Digest: digest.Digest("sha256:digest-test"), Size: int64(9)}, cache, false)
assert.NoError(t, err)
err = dest.Commit(context.Background())
err = dest.Commit(context.Background(), nil)
assert.NoError(t, err)
assert.Equal(t, int64(9), info.Size)
assert.Equal(t, digest.FromBytes(blob), info.Digest)
Expand Down Expand Up @@ -126,7 +130,7 @@ func TestPutBlobDigestFailure(t *testing.T) {
_, err = dest.PutBlob(context.Background(), reader, types.BlobInfo{Digest: blobDigest, Size: -1}, cache, false)
assert.Error(t, err)
assert.Contains(t, digestErrorString, err.Error())
err = dest.Commit(context.Background())
err = dest.Commit(context.Background(), nil)
assert.NoError(t, err)

_, err = os.Lstat(blobPath)
Expand All @@ -148,26 +152,41 @@ func TestGetPutSignatures(t *testing.T) {
}
err = dest.SupportsSignatures(context.Background())
assert.NoError(t, err)
err = dest.PutManifest(context.Background(), man)
err = dest.PutManifest(context.Background(), man, nil)
require.NoError(t, err)

err = dest.PutSignatures(context.Background(), signatures, nil)
listSignatures := [][]byte{
[]byte("sig3"),
[]byte("sig4"),
}
md, err := manifest.Digest(man)
require.NoError(t, err)

err = dest.SupportsSignatures(context.Background())
assert.NoError(t, err)
err = dest.PutManifest(context.Background(), man, nil)
require.NoError(t, err)
err = dest.PutManifest(context.Background(), man, &md)
require.NoError(t, err)

err = dest.PutSignatures(context.Background(), signatures)
err = dest.PutSignatures(context.Background(), listSignatures, nil)
assert.NoError(t, err)
err = dest.PutSignatures(context.Background(), signatures, &md)
assert.NoError(t, err)
err = dest.Commit(context.Background())
err = dest.Commit(context.Background(), nil)
assert.NoError(t, err)

src, err := ref.NewImageSource(context.Background(), nil)
require.NoError(t, err)
defer src.Close()
sigs, err := src.GetSignatures(context.Background(), nil)
assert.NoError(t, err)
assert.Equal(t, signatures, sigs)
assert.Equal(t, listSignatures, sigs)

// Non-default instances are not supported
md, err := manifest.Digest(man)
require.NoError(t, err)
_, err = src.GetSignatures(context.Background(), &md)
assert.Error(t, err)
sigs, err = src.GetSignatures(context.Background(), &md)
assert.NoError(t, err)
assert.Equal(t, signatures, sigs)
}

func TestSourceReference(t *testing.T) {
Expand Down
12 changes: 9 additions & 3 deletions directory/directory_transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,18 +166,24 @@ func (ref dirReference) DeleteImage(ctx context.Context, sys *types.SystemContex
}

// manifestPath returns a path for the manifest within a directory using our conventions.
func (ref dirReference) manifestPath() string {
func (ref dirReference) manifestPath(instanceDigest *digest.Digest) string {
if instanceDigest != nil {
return filepath.Join(ref.path, instanceDigest.Encoded()+".manifest.json")
}
return filepath.Join(ref.path, "manifest.json")
}

// layerPath returns a path for a layer tarball within a directory using our conventions.
func (ref dirReference) layerPath(digest digest.Digest) string {
// FIXME: Should we keep the digest identification?
return filepath.Join(ref.path, digest.Hex())
return filepath.Join(ref.path, digest.Encoded())
}

// signaturePath returns a path for a signature within a directory using our conventions.
func (ref dirReference) signaturePath(index int) string {
func (ref dirReference) signaturePath(index int, instanceDigest *digest.Digest) string {
if instanceDigest != nil {
return filepath.Join(ref.path, fmt.Sprintf(instanceDigest.Encoded()+".signature-%d", index+1))
}
return filepath.Join(ref.path, fmt.Sprintf("signature-%d", index+1))
}

Expand Down
22 changes: 15 additions & 7 deletions directory/directory_transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

_ "github.com/containers/image/v4/internal/testing/explicitfilepath-tmpdir"
"github.com/containers/image/v4/types"
digest "github.com/opencontainers/go-digest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -157,9 +158,9 @@ func TestReferenceNewImage(t *testing.T) {
defer dest.Close()
mFixture, err := ioutil.ReadFile("../manifest/fixtures/v2s1.manifest.json")
require.NoError(t, err)
err = dest.PutManifest(context.Background(), mFixture)
err = dest.PutManifest(context.Background(), mFixture, nil)
assert.NoError(t, err)
err = dest.Commit(context.Background())
err = dest.Commit(context.Background(), nil)
assert.NoError(t, err)

img, err := ref.NewImage(context.Background(), nil)
Expand All @@ -174,9 +175,9 @@ func TestReferenceNewImageNoValidManifest(t *testing.T) {
dest, err := ref.NewImageDestination(context.Background(), nil)
require.NoError(t, err)
defer dest.Close()
err = dest.PutManifest(context.Background(), []byte(`{"schemaVersion":1}`))
err = dest.PutManifest(context.Background(), []byte(`{"schemaVersion":1}`), nil)
assert.NoError(t, err)
err = dest.Commit(context.Background())
err = dest.Commit(context.Background(), nil)
assert.NoError(t, err)

_, err = ref.NewImage(context.Background(), nil)
Expand Down Expand Up @@ -207,11 +208,14 @@ func TestReferenceDeleteImage(t *testing.T) {
}

func TestReferenceManifestPath(t *testing.T) {
dhex := digest.Digest("sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef")

ref, tmpDir := refToTempDir(t)
defer os.RemoveAll(tmpDir)
dirRef, ok := ref.(dirReference)
require.True(t, ok)
assert.Equal(t, tmpDir+"/manifest.json", dirRef.manifestPath())
assert.Equal(t, tmpDir+"/manifest.json", dirRef.manifestPath(nil))
assert.Equal(t, tmpDir+"/"+dhex.Encoded()+".manifest.json", dirRef.manifestPath(&dhex))
}

func TestReferenceLayerPath(t *testing.T) {
Expand All @@ -225,12 +229,16 @@ func TestReferenceLayerPath(t *testing.T) {
}

func TestReferenceSignaturePath(t *testing.T) {
dhex := digest.Digest("sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef")

ref, tmpDir := refToTempDir(t)
defer os.RemoveAll(tmpDir)
dirRef, ok := ref.(dirReference)
require.True(t, ok)
assert.Equal(t, tmpDir+"/signature-1", dirRef.signaturePath(0))
assert.Equal(t, tmpDir+"/signature-10", dirRef.signaturePath(9))
assert.Equal(t, tmpDir+"/signature-1", dirRef.signaturePath(0, nil))
assert.Equal(t, tmpDir+"/signature-10", dirRef.signaturePath(9, nil))
assert.Equal(t, tmpDir+"/"+dhex.Encoded()+".signature-1", dirRef.signaturePath(0, &dhex))
assert.Equal(t, tmpDir+"/"+dhex.Encoded()+".signature-10", dirRef.signaturePath(9, &dhex))
}

func TestReferenceVersionPath(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion docker/archive/dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,6 @@ func (d *archiveImageDestination) Close() error {
// WARNING: This does not have any transactional semantics:
// - Uploaded data MAY be visible to others before Commit() is called
// - Uploaded data MAY be removed or MAY remain around if Close() is called without Commit() (i.e. rollback is allowed but not guaranteed)
func (d *archiveImageDestination) Commit(ctx context.Context) error {
func (d *archiveImageDestination) Commit(ctx context.Context, unparsedToplevel types.UnparsedImage) error {
return d.Destination.Commit(ctx)
}
5 changes: 0 additions & 5 deletions docker/archive/src.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,3 @@ func newImageSource(ctx context.Context, ref archiveReference) (types.ImageSourc
func (s *archiveImageSource) Reference() types.ImageReference {
return s.ref
}

// LayerInfosForCopy() returns updated layer info that should be used when reading, in preference to values in the manifest, if specified.
func (s *archiveImageSource) LayerInfosForCopy(ctx context.Context) ([]types.BlobInfo, error) {
return nil, nil
}
2 changes: 1 addition & 1 deletion docker/daemon/daemon_dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (d *daemonImageDestination) Reference() types.ImageReference {
// WARNING: This does not have any transactional semantics:
// - Uploaded data MAY be visible to others before Commit() is called
// - Uploaded data MAY be removed or MAY remain around if Close() is called without Commit() (i.e. rollback is allowed but not guaranteed)
func (d *daemonImageDestination) Commit(ctx context.Context) error {
func (d *daemonImageDestination) Commit(ctx context.Context, unparsedToplevel types.UnparsedImage) error {
logrus.Debugf("docker-daemon: Closing tar stream")
if err := d.Destination.Commit(ctx); err != nil {
return err
Expand Down
5 changes: 0 additions & 5 deletions docker/daemon/daemon_src.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,3 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref daemonRef
func (s *daemonImageSource) Reference() types.ImageReference {
return s.ref
}

// LayerInfosForCopy() returns updated layer info that should be used when reading, in preference to values in the manifest, if specified.
func (s *daemonImageSource) LayerInfosForCopy(ctx context.Context) ([]types.BlobInfo, error) {
return nil, nil
}
Loading

0 comments on commit d0337d2

Please sign in to comment.