WIP RFC: internal/external interface separation/adapters

docker/docker_image_dest.go

@@ -16,6 +16,8 @@ import (
 
 	"github.com/containers/image/v5/docker/reference"
 	"github.com/containers/image/v5/internal/blobinfocache"
+	"github.com/containers/image/v5/internal/imagedestination/impl"
+	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/internal/putblobdigest"
 	"github.com/containers/image/v5/internal/streamdigest"
 	"github.com/containers/image/v5/internal/uploadreader"
@@ -30,6 +32,8 @@ import (
 )
 
 type dockerImageDestination struct {
+	impl.Compat
+
 	ref dockerReference
 	c   *dockerClient
 	// State
@@ -42,10 +46,12 @@ func newImageDestination(sys *types.SystemContext, ref dockerReference) (types.I
 	if err != nil {
 		return nil, err
 	}
-	return &dockerImageDestination{
+	dest := &dockerImageDestination{
 		ref: ref,
 		c:   c,
-	}, nil
+	}
+	dest.Compat = impl.AddCompat(dest)
+	return dest, nil
 }
 
 // Reference returns the reference used to set up this destination.  Note that this should directly correspond to user's intent,
@@ -123,14 +129,14 @@ func (d *dockerImageDestination) HasThreadSafePutBlob() bool {
 	return true
 }
 
-// PutBlob writes contents of stream and returns data representing the result (with all data filled in).
+// PutBlobWithOptions writes contents of stream and returns data representing the result.
 // inputInfo.Digest can be optionally provided if known; if provided, and stream is read to the end without error, the digest MUST match the stream contents.
 // inputInfo.Size is the expected length of stream, if known.
-// May update cache.
+// inputInfo.MediaType describes the blob format, if known.
 // WARNING: The contents of stream are being verified on the fly.  Until stream.Read() returns io.EOF, the contents of the data SHOULD NOT be available
 // to any other readers for download using the supplied digest.
 // If stream.Read() at any time, ESPECIALLY at end of input, returns an error, PutBlob MUST 1) fail, and 2) delete any data stored so far.
-func (d *dockerImageDestination) PutBlob(ctx context.Context, stream io.Reader, inputInfo types.BlobInfo, cache types.BlobInfoCache, isConfig bool) (types.BlobInfo, error) {
+func (d *dockerImageDestination) PutBlobWithOptions(ctx context.Context, stream io.Reader, inputInfo types.BlobInfo, options private.PutBlobOptions) (types.BlobInfo, error) {
 	// If requested, precompute the blob digest to prevent uploading layers that already exist on the registry.
 	// This functionality is particularly useful when BlobInfoCache has not been populated with compressed digests,
 	// the source blob is uncompressed, and the destination blob is being compressed "on the fly".
@@ -147,7 +153,7 @@ func (d *dockerImageDestination) PutBlob(ctx context.Context, stream io.Reader,
 	if inputInfo.Digest != "" {
 		// This should not really be necessary, at least the copy code calls TryReusingBlob automatically.
 		// Still, we need to check, if only because the "initiate upload" endpoint does not have a documented "blob already exists" return value.
-		haveBlob, reusedInfo, err := d.tryReusingExactBlob(ctx, inputInfo, cache)
+		haveBlob, reusedInfo, err := d.tryReusingExactBlob(ctx, inputInfo, options.Cache)
 		if err != nil {
 			return types.BlobInfo{}, err
 		}
@@ -218,10 +224,24 @@ func (d *dockerImageDestination) PutBlob(ctx context.Context, stream io.Reader,
 	}
 
 	logrus.Debugf("Upload of layer %s complete", blobDigest)
-	cache.RecordKnownLocation(d.ref.Transport(), bicTransportScope(d.ref), blobDigest, newBICLocationReference(d.ref))
+	options.Cache.RecordKnownLocation(d.ref.Transport(), bicTransportScope(d.ref), blobDigest, newBICLocationReference(d.ref))
 	return types.BlobInfo{Digest: blobDigest, Size: sizeCounter.size}, nil
 }
 
+// SupportsPutBlobPartial returns true if PutBlobPartial is supported.
+func (d *dockerImageDestination) SupportsPutBlobPartial() bool {
+	return false
+}
+
+// PutBlobPartial attempts to create a blob using the data that is already present
+// at the destination. chunkAccessor is accessed in a non-sequential way to retrieve the missing chunks.
+// It is available only if SupportsPutBlobPartial().
+// Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller
+// should fall back to PutBlobWithOptions.
+func (d *dockerImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache blobinfocache.BlobInfoCache2) (types.BlobInfo, error) {
+	return types.BlobInfo{}, fmt.Errorf("internal error: PutBlobPartial is not supported by the %q transport", d.Reference().Transport().Name())
+}
+
 // blobExists returns true iff repo contains a blob with digest, and if so, also its size.
 // If the destination does not contain the blob, or it is unknown, blobExists ordinarily returns (false, -1, nil);
 // it returns a non-nil error only on an unexpected failure.
@@ -296,7 +316,7 @@ func (d *dockerImageDestination) mountBlob(ctx context.Context, srcRepo referenc
 // tryReusingExactBlob is a subset of TryReusingBlob which _only_ looks for exactly the specified
 // blob in the current repository, with no cross-repo reuse or mounting; cache may be updated, it is not read.
 // The caller must ensure info.Digest is set.
-func (d *dockerImageDestination) tryReusingExactBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache) (bool, types.BlobInfo, error) {
+func (d *dockerImageDestination) tryReusingExactBlob(ctx context.Context, info types.BlobInfo, cache blobinfocache.BlobInfoCache2) (bool, types.BlobInfo, error) {
 	exists, size, err := d.blobExists(ctx, d.ref.ref, info.Digest, nil)
 	if err != nil {
 		return false, types.BlobInfo{}, err
@@ -308,22 +328,20 @@ func (d *dockerImageDestination) tryReusingExactBlob(ctx context.Context, info t
 	return false, types.BlobInfo{}, nil
 }
 
-// TryReusingBlob checks whether the transport already contains, or can efficiently reuse, a blob, and if so, applies it to the current destination
+// TryReusingBlobWithOptions checks whether the transport already contains, or can efficiently reuse, a blob, and if so, applies it to the current destination
 // (e.g. if the blob is a filesystem layer, this signifies that the changes it describes need to be applied again when composing a filesystem tree).
 // info.Digest must not be empty.
-// If canSubstitute, TryReusingBlob can use an equivalent equivalent of the desired blob; in that case the returned info may not match the input.
 // If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size, and may
 // include CompressionOperation and CompressionAlgorithm fields to indicate that a change to the compression type should be
 // reflected in the manifest that will be written.
 // If the transport can not reuse the requested blob, TryReusingBlob returns (false, {}, nil); it returns a non-nil error only on an unexpected failure.
-// May use and/or update cache.
-func (d *dockerImageDestination) TryReusingBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache, canSubstitute bool) (bool, types.BlobInfo, error) {
+func (d *dockerImageDestination) TryReusingBlobWithOptions(ctx context.Context, info types.BlobInfo, options private.TryReusingBlobOptions) (bool, types.BlobInfo, error) {
 	if info.Digest == "" {
 		return false, types.BlobInfo{}, errors.New("Can not check for a blob with unknown digest")
 	}
 
 	// First, check whether the blob happens to already exist at the destination.
-	haveBlob, reusedInfo, err := d.tryReusingExactBlob(ctx, info, cache)
+	haveBlob, reusedInfo, err := d.tryReusingExactBlob(ctx, info, options.Cache)
 	if err != nil {
 		return false, types.BlobInfo{}, err
 	}
@@ -332,8 +350,7 @@ func (d *dockerImageDestination) TryReusingBlob(ctx context.Context, info types.
 	}
 
 	// Then try reusing blobs from other locations.
-	bic := blobinfocache.FromBlobInfoCache(cache)
-	candidates := bic.CandidateLocations2(d.ref.Transport(), bicTransportScope(d.ref), info.Digest, canSubstitute)
+	candidates := options.Cache.CandidateLocations2(d.ref.Transport(), bicTransportScope(d.ref), info.Digest, options.CanSubstitute)
 	for _, candidate := range candidates {
 		candidateRepo, err := parseBICLocationReference(candidate.Location)
 		if err != nil {
@@ -387,7 +404,7 @@ func (d *dockerImageDestination) TryReusingBlob(ctx context.Context, info types.
 			}
 		}
 
-		bic.RecordKnownLocation(d.ref.Transport(), bicTransportScope(d.ref), candidate.Digest, newBICLocationReference(d.ref))
+		options.Cache.RecordKnownLocation(d.ref.Transport(), bicTransportScope(d.ref), candidate.Digest, newBICLocationReference(d.ref))
 
 		compressionOperation, compressionAlgorithm, err := blobinfocache.OperationAndAlgorithmForCompressor(candidate.CompressorName)
 		if err != nil {

docker/docker_image_dest_test.go

@@ -6,10 +6,13 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/containers/image/v5/internal/private"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+var _ private.ImageDestination = (*dockerImageDestination)(nil)
+
 func TestIsManifestInvalidError(t *testing.T) {
 	// Sadly only a smoke test; this really should record all known errors exactly as they happen.
 

internal/imagedestination/impl/compat.go

@@ -0,0 +1,63 @@
+package impl
+
+import (
+	"context"
+	"io"
+
+	"github.com/containers/image/v5/internal/blobinfocache"
+	"github.com/containers/image/v5/internal/private"
+	"github.com/containers/image/v5/types"
+)
+
+// Compat implements the obsolete parts of types.ImageDestination
+// for implementations of private.ImageDestination.
+// See AddCompat below.
+type Compat struct {
+	dest private.ImageDestinationInternalOnly
+}
+
+// AddCompat initializes Compat to implement the obsolete parts of types.ImageDestination
+// for implementations of private.ImageDestination.
+//
+// Use it like this:
+// type yourDestination struct {
+//     impl.Compat
+//     …
+// }
+// dest := &yourDestination{…}
+// dest.Compat = impl.AddCompat(dest)
+//
+func AddCompat(dest private.ImageDestinationInternalOnly) Compat {
+	return Compat{dest}
+}
+
+// PutBlob writes contents of stream and returns data representing the result.
+// inputInfo.Digest can be optionally provided if known; if provided, and stream is read to the end without error, the digest MUST match the stream contents.
+// inputInfo.Size is the expected length of stream, if known.
+// inputInfo.MediaType describes the blob format, if known.
+// May update cache.
+// WARNING: The contents of stream are being verified on the fly.  Until stream.Read() returns io.EOF, the contents of the data SHOULD NOT be available
+// to any other readers for download using the supplied digest.
+// If stream.Read() at any time, ESPECIALLY at end of input, returns an error, PutBlob MUST 1) fail, and 2) delete any data stored so far.
+func (c *Compat) PutBlob(ctx context.Context, stream io.Reader, inputInfo types.BlobInfo, cache types.BlobInfoCache, isConfig bool) (types.BlobInfo, error) {
+	return c.dest.PutBlobWithOptions(ctx, stream, inputInfo, private.PutBlobOptions{
+		Cache:    blobinfocache.FromBlobInfoCache(cache),
+		IsConfig: isConfig,
+	})
+}
+
+// TryReusingBlob checks whether the transport already contains, or can efficiently reuse, a blob, and if so, applies it to the current destination
+// (e.g. if the blob is a filesystem layer, this signifies that the changes it describes need to be applied again when composing a filesystem tree).
+// info.Digest must not be empty.
+// If canSubstitute, TryReusingBlob can use an equivalent equivalent of the desired blob; in that case the returned info may not match the input.
+// If the blob has been successfully reused, returns (true, info, nil); info must contain at least a digest and size, and may
+// include CompressionOperation and CompressionAlgorithm fields to indicate that a change to the compression type should be
+// reflected in the manifest that will be written.
+// If the transport can not reuse the requested blob, TryReusingBlob returns (false, {}, nil); it returns a non-nil error only on an unexpected failure.
+// May use and/or update cache.
+func (c *Compat) TryReusingBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache, canSubstitute bool) (bool, types.BlobInfo, error) {
+	return c.dest.TryReusingBlobWithOptions(ctx, info, private.TryReusingBlobOptions{
+		Cache:         blobinfocache.FromBlobInfoCache(cache),
+		CanSubstitute: canSubstitute,
+	})
+}

internal/imagedestination/wrapper.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 
+	"github.com/containers/image/v5/internal/blobinfocache"
 	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/types"
 )
@@ -53,7 +54,7 @@ func (w *wrapped) PutBlobWithOptions(ctx context.Context, stream io.Reader, inpu
 // It is available only if SupportsPutBlobPartial().
 // Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller
 // should fall back to PutBlobWithOptions.
-func (w *wrapped) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache types.BlobInfoCache) (types.BlobInfo, error) {
+func (w *wrapped) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache blobinfocache.BlobInfoCache2) (types.BlobInfo, error) {
 	return types.BlobInfo{}, fmt.Errorf("internal error: PutBlobPartial is not supported by the %q transport", w.Reference().Transport().Name())
 }
 

internal/private/private.go

@@ -5,6 +5,7 @@ import (
 	"io"
 
 	"github.com/containers/image/v5/docker/reference"
+	"github.com/containers/image/v5/internal/blobinfocache"
 	"github.com/containers/image/v5/types"
 )
 
@@ -18,11 +19,9 @@ type ImageSource interface {
 	BlobChunkAccessor
 }
 
-// ImageDestination is an internal extension to the types.ImageDestination
-// interface.
-type ImageDestination interface {
-	types.ImageDestination
-
+// ImageDestinationInternalOnly is the part of private.ImageDestination that is not
+// a part of types.ImageDestination.
+type ImageDestinationInternalOnly interface {
 	// SupportsPutBlobPartial returns true if PutBlobPartial is supported.
 	SupportsPutBlobPartial() bool
 
@@ -40,7 +39,7 @@ type ImageDestination interface {
 	// It is available only if SupportsPutBlobPartial().
 	// Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller
 	// should fall back to PutBlobWithOptions.
-	PutBlobPartial(ctx context.Context, chunkAccessor BlobChunkAccessor, srcInfo types.BlobInfo, cache types.BlobInfoCache) (types.BlobInfo, error)
+	PutBlobPartial(ctx context.Context, chunkAccessor BlobChunkAccessor, srcInfo types.BlobInfo, cache blobinfocache.BlobInfoCache2) (types.BlobInfo, error)
 
 	// TryReusingBlobWithOptions checks whether the transport already contains, or can efficiently reuse, a blob, and if so, applies it to the current destination
 	// (e.g. if the blob is a filesystem layer, this signifies that the changes it describes need to be applied again when composing a filesystem tree).
@@ -52,27 +51,40 @@ type ImageDestination interface {
 	TryReusingBlobWithOptions(ctx context.Context, info types.BlobInfo, options TryReusingBlobOptions) (bool, types.BlobInfo, error)
 }
 
+// ImageDestination is an internal extension to the types.ImageDestination
+// interface.
+type ImageDestination interface {
+	types.ImageDestination
+	ImageDestinationInternalOnly
+}
+
 // PutBlobOptions are used in PutBlobWithOptions.
 type PutBlobOptions struct {
-	Cache    types.BlobInfoCache // Cache to optionally update with the uploaded bloblook up blob infos.
-	IsConfig bool                // True if the blob is a config
+	Cache    blobinfocache.BlobInfoCache2 // Cache to optionally update with the uploaded bloblook up blob infos.
+	IsConfig bool                         // True if the blob is a config
 
 	// The following fields are new to internal/private.  Users of internal/private MUST fill them in,
 	// but they also must expect that they will be ignored by types.ImageDestination transports.
+	// Transports, OTOH, MUST support these fields being zero-valued for types.ImageDestination callers
+	// if they use internal/imagedestination/impl.Compat;
+	// in that case, they will all be consistently zero-valued.
 
 	EmptyLayer bool // True if the blob is an "empty"/"throwaway" layer, and may not necessarily be physically represented.
 	LayerIndex *int // If the blob is a layer, a zero-based index of the layer within the image; nil otherwise.
 }
 
 // TryReusingBlobOptions are used in TryReusingBlobWithOptions.
 type TryReusingBlobOptions struct {
-	Cache types.BlobInfoCache // Cache to use and/or update.
+	Cache blobinfocache.BlobInfoCache2 // Cache to use and/or update.
 	// If true, it is allowed to use an equivalent of the desired blob;
 	// in that case the returned info may not match the input.
 	CanSubstitute bool
 
 	// The following fields are new to internal/private.  Users of internal/private MUST fill them in,
 	// but they also must expect that they will be ignored by types.ImageDestination transports.
+	// Transports, OTOH, MUST support these fields being zero-valued for types.ImageDestination callers
+	// if they use internal/imagedestination/impl.Compat;
+	// in that case, they will all be consistently zero-valued.
 
 	EmptyLayer bool            // True if the blob is an "empty"/"throwaway" layer, and may not necessarily be physically represented.
 	LayerIndex *int            // If the blob is a layer, a zero-based index of the layer within the image; nil otherwise.