Merge pull request #1589 from mtrmac/private-dest-impls

Implement private.ImageDestination in all transports, improve transport helpers

directory/directory_dest.go

@@ -9,6 +9,9 @@ import (
 	"path/filepath"
 	"runtime"
 
+	"github.com/containers/image/v5/internal/imagedestination/impl"
+	"github.com/containers/image/v5/internal/imagedestination/stubs"
+	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/internal/putblobdigest"
 	"github.com/containers/image/v5/types"
 	"github.com/opencontainers/go-digest"
@@ -23,12 +26,16 @@ const version = "Directory Transport Version: 1.1\n"
 var ErrNotContainerImageDir = errors.New("not a containers image directory, don't want to overwrite important data")
 
 type dirImageDestination struct {
-	ref                     dirReference
-	desiredLayerCompression types.LayerCompression
+	impl.Compat
+	impl.PropertyMethodsInitialize
+	stubs.NoPutBlobPartialInitialize
+	stubs.AlwaysSupportsSignatures
+
+	ref dirReference
 }
 
 // newImageDestination returns an ImageDestination for writing to a directory.
-func newImageDestination(sys *types.SystemContext, ref dirReference) (types.ImageDestination, error) {
+func newImageDestination(sys *types.SystemContext, ref dirReference) (private.ImageDestination, error) {
 	desiredLayerCompression := types.PreserveOriginal
 	if sys != nil {
 		if sys.DirForceCompress {
@@ -42,28 +49,27 @@ func newImageDestination(sys *types.SystemContext, ref dirReference) (types.Imag
 			desiredLayerCompression = types.Decompress
 		}
 	}
-	d := &dirImageDestination{ref: ref, desiredLayerCompression: desiredLayerCompression}
 
 	// If directory exists check if it is empty
 	// if not empty, check whether the contents match that of a container image directory and overwrite the contents
 	// if the contents don't match throw an error
-	dirExists, err := pathExists(d.ref.resolvedPath)
+	dirExists, err := pathExists(ref.resolvedPath)
 	if err != nil {
-		return nil, perrors.Wrapf(err, "checking for path %q", d.ref.resolvedPath)
+		return nil, perrors.Wrapf(err, "checking for path %q", ref.resolvedPath)
 	}
 	if dirExists {
-		isEmpty, err := isDirEmpty(d.ref.resolvedPath)
+		isEmpty, err := isDirEmpty(ref.resolvedPath)
 		if err != nil {
 			return nil, err
 		}
 
 		if !isEmpty {
-			versionExists, err := pathExists(d.ref.versionPath())
+			versionExists, err := pathExists(ref.versionPath())
 			if err != nil {
-				return nil, perrors.Wrapf(err, "checking if path exists %q", d.ref.versionPath())
+				return nil, perrors.Wrapf(err, "checking if path exists %q", ref.versionPath())
 			}
 			if versionExists {
-				contents, err := os.ReadFile(d.ref.versionPath())
+				contents, err := os.ReadFile(ref.versionPath())
 				if err != nil {
 					return nil, err
 				}
@@ -75,22 +81,37 @@ func newImageDestination(sys *types.SystemContext, ref dirReference) (types.Imag
 				return nil, ErrNotContainerImageDir
 			}
 			// delete directory contents so that only one image is in the directory at a time
-			if err = removeDirContents(d.ref.resolvedPath); err != nil {
-				return nil, perrors.Wrapf(err, "erasing contents in %q", d.ref.resolvedPath)
+			if err = removeDirContents(ref.resolvedPath); err != nil {
+				return nil, perrors.Wrapf(err, "erasing contents in %q", ref.resolvedPath)
 			}
-			logrus.Debugf("overwriting existing container image directory %q", d.ref.resolvedPath)
+			logrus.Debugf("overwriting existing container image directory %q", ref.resolvedPath)
 		}
 	} else {
 		// create directory if it doesn't exist
-		if err := os.MkdirAll(d.ref.resolvedPath, 0755); err != nil {
-			return nil, perrors.Wrapf(err, "unable to create directory %q", d.ref.resolvedPath)
+		if err := os.MkdirAll(ref.resolvedPath, 0755); err != nil {
+			return nil, perrors.Wrapf(err, "unable to create directory %q", ref.resolvedPath)
 		}
 	}
 	// create version file
-	err = os.WriteFile(d.ref.versionPath(), []byte(version), 0644)
+	err = os.WriteFile(ref.versionPath(), []byte(version), 0644)
 	if err != nil {
-		return nil, perrors.Wrapf(err, "creating version file %q", d.ref.versionPath())
+		return nil, perrors.Wrapf(err, "creating version file %q", ref.versionPath())
 	}
+
+	d := &dirImageDestination{
+		PropertyMethodsInitialize: impl.PropertyMethods(impl.Properties{
+			SupportedManifestMIMETypes:     nil,
+			DesiredLayerCompression:        desiredLayerCompression,
+			AcceptsForeignLayerURLs:        false,
+			MustMatchRuntimeOS:             false,
+			IgnoresEmbeddedDockerReference: false, // N/A, DockerReference() returns nil.
+			HasThreadSafePutBlob:           false,
+		}),
+		NoPutBlobPartialInitialize: stubs.NoPutBlobPartial(ref),
+
+		ref: ref,
+	}
+	d.Compat = impl.AddCompat(d)
 	return d, nil
 }
 
@@ -105,51 +126,14 @@ func (d *dirImageDestination) Close() error {
 	return nil
 }
 
-func (d *dirImageDestination) SupportedManifestMIMETypes() []string {
-	return nil
-}
-
-// SupportsSignatures returns an error (to be displayed to the user) if the destination certainly can't store signatures.
-// Note: It is still possible for PutSignatures to fail if SupportsSignatures returns nil.
-func (d *dirImageDestination) SupportsSignatures(ctx context.Context) error {
-	return nil
-}
-
-func (d *dirImageDestination) DesiredLayerCompression() types.LayerCompression {
-	return d.desiredLayerCompression
-}
-
-// AcceptsForeignLayerURLs returns false iff foreign layers in manifest should be actually
-// uploaded to the image destination, true otherwise.
-func (d *dirImageDestination) AcceptsForeignLayerURLs() bool {
-	return false
-}
-
-// MustMatchRuntimeOS returns true iff the destination can store only images targeted for the current runtime architecture and OS. False otherwise.
-func (d *dirImageDestination) MustMatchRuntimeOS() bool {
-	return false
-}
-
-// IgnoresEmbeddedDockerReference returns true iff the destination does not care about Image.EmbeddedDockerReferenceConflicts(),
-// and would prefer to receive an unmodified manifest instead of one modified for the destination.
-// Does not make a difference if Reference().DockerReference() is nil.
-func (d *dirImageDestination) IgnoresEmbeddedDockerReference() bool {
-	return false // N/A, DockerReference() returns nil.
-}
-
-// HasThreadSafePutBlob indicates whether PutBlob can be executed concurrently.
-func (d *dirImageDestination) HasThreadSafePutBlob() bool {
-	return false
-}
-
-// 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 *dirImageDestination) PutBlob(ctx context.Context, stream io.Reader, inputInfo types.BlobInfo, cache types.BlobInfoCache, isConfig bool) (types.BlobInfo, error) {
+func (d *dirImageDestination) PutBlobWithOptions(ctx context.Context, stream io.Reader, inputInfo types.BlobInfo, options private.PutBlobOptions) (types.BlobInfo, error) {
 	blobFile, err := os.CreateTemp(d.ref.path, "dir-put-blob")
 	if err != nil {
 		return types.BlobInfo{}, err
@@ -200,16 +184,14 @@ func (d *dirImageDestination) PutBlob(ctx context.Context, stream io.Reader, inp
 	return types.BlobInfo{Digest: blobDigest, Size: size}, 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 *dirImageDestination) TryReusingBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache, canSubstitute bool) (bool, types.BlobInfo, error) {
+func (d *dirImageDestination) TryReusingBlobWithOptions(ctx context.Context, info types.BlobInfo, options private.TryReusingBlobOptions) (bool, types.BlobInfo, error) {
 	if info.Digest == "" {
 		return false, types.BlobInfo{}, fmt.Errorf("Can not check for a blob with unknown digest")
 	}

directory/directory_test.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/image/v5/pkg/blobinfocache/memory"
 	"github.com/containers/image/v5/types"
@@ -16,6 +17,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+var _ private.ImageDestination = (*dirImageDestination)(nil)
+
 func TestDestinationReference(t *testing.T) {
 	ref, tmpDir := refToTempDir(t)
 

docker/archive/dest.go

@@ -6,6 +6,7 @@ import (
 	"io"
 
 	"github.com/containers/image/v5/docker/internal/tarfile"
+	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/types"
 )
 
@@ -16,7 +17,7 @@ type archiveImageDestination struct {
 	writer               io.Closer       // May be nil if the archive is shared
 }
 
-func newImageDestination(sys *types.SystemContext, ref archiveReference) (types.ImageDestination, error) {
+func newImageDestination(sys *types.SystemContext, ref archiveReference) (private.ImageDestination, error) {
 	if ref.sourceIndex != -1 {
 		return nil, fmt.Errorf("Destination reference must not contain a manifest index @%d", ref.sourceIndex)
 	}
@@ -35,7 +36,7 @@ func newImageDestination(sys *types.SystemContext, ref archiveReference) (types.
 		archive = tarfile.NewWriter(fh)
 		writer = fh
 	}
-	tarDest := tarfile.NewDestination(sys, archive, ref.ref)
+	tarDest := tarfile.NewDestination(sys, archive, ref.Transport().Name(), ref.ref)
 	if sys != nil && sys.DockerArchiveAdditionalTags != nil {
 		tarDest.AddRepoTags(sys.DockerArchiveAdditionalTags)
 	}
@@ -47,11 +48,6 @@ func newImageDestination(sys *types.SystemContext, ref archiveReference) (types.
 	}, nil
 }
 
-// DesiredLayerCompression indicates if layers must be compressed, decompressed or preserved
-func (d *archiveImageDestination) DesiredLayerCompression() types.LayerCompression {
-	return types.Decompress
-}
-
 // Reference returns the reference used to set up this destination.  Note that this should directly correspond to user's intent,
 // e.g. it should use the public hostname instead of the result of resolving CNAMEs or following redirects.
 func (d *archiveImageDestination) Reference() types.ImageReference {

docker/archive/dest_test.go

@@ -0,0 +1,5 @@
+package archive
+
+import "github.com/containers/image/v5/internal/private"
+
+var _ private.ImageDestination = (*archiveImageDestination)(nil)

docker/daemon/daemon_dest.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/containers/image/v5/docker/internal/tarfile"
 	"github.com/containers/image/v5/docker/reference"
+	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/types"
 	"github.com/docker/docker/client"
 	perrors "github.com/pkg/errors"
@@ -28,7 +29,7 @@ type daemonImageDestination struct {
 }
 
 // newImageDestination returns a types.ImageDestination for the specified image reference.
-func newImageDestination(ctx context.Context, sys *types.SystemContext, ref daemonReference) (types.ImageDestination, error) {
+func newImageDestination(ctx context.Context, sys *types.SystemContext, ref daemonReference) (private.ImageDestination, error) {
 	if ref.ref == nil {
 		return nil, fmt.Errorf("Invalid destination docker-daemon:%s: a destination must be a name:tag", ref.StringWithinTransport())
 	}
@@ -58,7 +59,7 @@ func newImageDestination(ctx context.Context, sys *types.SystemContext, ref daem
 	return &daemonImageDestination{
 		ref:                ref,
 		mustMatchRuntimeOS: mustMatchRuntimeOS,
-		Destination:        tarfile.NewDestination(sys, archive, namedTaggedRef),
+		Destination:        tarfile.NewDestination(sys, archive, ref.Transport().Name(), namedTaggedRef),
 		archive:            archive,
 		goroutineCancel:    goroutineCancel,
 		statusChannel:      statusChannel,

docker/daemon/daemon_dest_test.go

@@ -0,0 +1,5 @@
+package daemon
+
+import "github.com/containers/image/v5/internal/private"
+
+var _ private.ImageDestination = (*daemonImageDestination)(nil)