Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve support for manifest lists and image indexes #400

Merged
merged 1 commit into from
Oct 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
354 changes: 306 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 we're forcing it, we prefer the forced value over everything else.
if forcedListMIMEType != "" {
return forcedListMIMEType, nil
}
// 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]
}
vrothberg marked this conversation as resolved.
Show resolved Hide resolved
}
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