Merge pull request #1590 from mtrmac/private-src-impls

Implement private.ImageSource in all transports, improve transport helpers

directory/directory_src.go

@@ -5,19 +5,33 @@ import (
 	"io"
 	"os"
 
+	"github.com/containers/image/v5/internal/imagesource/impl"
+	"github.com/containers/image/v5/internal/imagesource/stubs"
+	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/image/v5/types"
 	"github.com/opencontainers/go-digest"
 )
 
 type dirImageSource struct {
+	impl.PropertyMethodsInitialize
+	impl.DoesNotAffectLayerInfosForCopy
+	stubs.NoGetBlobAtInitialize
+
 	ref dirReference
 }
 
 // newImageSource returns an ImageSource reading from an existing directory.
 // The caller must call .Close() on the returned ImageSource.
-func newImageSource(ref dirReference) types.ImageSource {
-	return &dirImageSource{ref}
+func newImageSource(ref dirReference) private.ImageSource {
+	return &dirImageSource{
+		PropertyMethodsInitialize: impl.PropertyMethods(impl.Properties{
+			HasThreadSafeGetBlob: false,
+		}),
+		NoGetBlobAtInitialize: stubs.NoGetBlobAt(ref),
+
+		ref: ref,
+	}
 }
 
 // Reference returns the reference used to set up this source, _as specified by the user_
@@ -43,11 +57,6 @@ func (s *dirImageSource) GetManifest(ctx context.Context, instanceDigest *digest
 	return m, manifest.GuessMIMEType(m), err
 }
 
-// HasThreadSafeGetBlob indicates whether GetBlob can be executed concurrently.
-func (s *dirImageSource) HasThreadSafeGetBlob() bool {
-	return false
-}
-
 // GetBlob returns a stream for the specified blob, and the blob’s size (or -1 if unknown).
 // The Digest field in BlobInfo is guaranteed to be provided, Size may be -1 and MediaType may be optionally provided.
 // May update BlobInfoCache, preferably after it knows for certain that a blob truly exists at a specific location.
@@ -81,15 +90,3 @@ func (s *dirImageSource) GetSignatures(ctx context.Context, instanceDigest *dige
 	}
 	return signatures, nil
 }
-
-// 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
-}

directory/directory_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+var _ private.ImageSource = (*dirImageSource)(nil)
 var _ private.ImageDestination = (*dirImageDestination)(nil)
 
 func TestDestinationReference(t *testing.T) {

docker/archive/src.go

@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"github.com/containers/image/v5/docker/internal/tarfile"
+	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/types"
 )
 
@@ -14,7 +15,7 @@ type archiveImageSource struct {
 
 // newImageSource returns a types.ImageSource for the specified image reference.
 // The caller must call .Close() on the returned ImageSource.
-func newImageSource(ctx context.Context, sys *types.SystemContext, ref archiveReference) (types.ImageSource, error) {
+func newImageSource(ctx context.Context, sys *types.SystemContext, ref archiveReference) (private.ImageSource, error) {
 	var archive *tarfile.Reader
 	var closeArchive bool
 	if ref.archiveReader != nil {
@@ -28,7 +29,7 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref archiveRe
 		archive = a
 		closeArchive = true
 	}
-	src := tarfile.NewSource(archive, closeArchive, ref.ref, ref.sourceIndex)
+	src := tarfile.NewSource(archive, closeArchive, ref.Transport().Name(), ref.ref, ref.sourceIndex)
 	return &archiveImageSource{
 		Source: src,
 		ref:    ref,

docker/archive/src_test.go

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

docker/daemon/daemon_src.go

@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"github.com/containers/image/v5/docker/internal/tarfile"
+	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/types"
 	perrors "github.com/pkg/errors"
 )
@@ -22,7 +23,7 @@ type daemonImageSource struct {
 // (We could, perhaps, expect an exact sequence, assume that the first plaintext file
 // is the config, and that the following len(RootFS) files are the layers, but that feels
 // way too brittle.)
-func newImageSource(ctx context.Context, sys *types.SystemContext, ref daemonReference) (types.ImageSource, error) {
+func newImageSource(ctx context.Context, sys *types.SystemContext, ref daemonReference) (private.ImageSource, error) {
 	c, err := newDockerClient(sys)
 	if err != nil {
 		return nil, perrors.Wrap(err, "initializing docker engine client")
@@ -39,7 +40,7 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref daemonRef
 	if err != nil {
 		return nil, err
 	}
-	src := tarfile.NewSource(archive, true, nil, -1)
+	src := tarfile.NewSource(archive, true, ref.Transport().Name(), nil, -1)
 	return &daemonImageSource{
 		ref:    ref,
 		Source: src,

docker/daemon/daemon_src_test.go

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

docker/docker_image_src.go

@@ -16,6 +16,8 @@ import (
 	"sync"
 
 	"github.com/containers/image/v5/docker/reference"
+	"github.com/containers/image/v5/internal/imagesource/impl"
+	"github.com/containers/image/v5/internal/imagesource/stubs"
 	"github.com/containers/image/v5/internal/iolimits"
 	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/manifest"
@@ -27,6 +29,10 @@ import (
 )
 
 type dockerImageSource struct {
+	impl.PropertyMethodsInitialize
+	impl.DoesNotAffectLayerInfosForCopy
+	stubs.ImplementsGetBlobAt
+
 	logicalRef  dockerReference // The reference the user requested.
 	physicalRef dockerReference // The actual reference we are accessing (possibly a mirror)
 	c           *dockerClient
@@ -126,6 +132,10 @@ func newImageSourceAttempt(ctx context.Context, sys *types.SystemContext, logica
 	client.tlsClientConfig.InsecureSkipVerify = pullSource.Endpoint.Insecure
 
 	s := &dockerImageSource{
+		PropertyMethodsInitialize: impl.PropertyMethods(impl.Properties{
+			HasThreadSafeGetBlob: true,
+		}),
+
 		logicalRef:  logicalRef,
 		physicalRef: physicalRef,
 		c:           client,
@@ -148,23 +158,6 @@ func (s *dockerImageSource) Close() error {
 	return nil
 }
 
-// SupportsGetBlobAt() returns true if GetBlobAt (BlobChunkAccessor) is supported.
-func (s *dockerImageSource) SupportsGetBlobAt() bool {
-	return true
-}
-
-// 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 *dockerImageSource) LayerInfosForCopy(context.Context, *digest.Digest) ([]types.BlobInfo, error) {
-	return nil, nil
-}
-
 // simplifyContentType drops parameters from a HTTP media type (see https://tools.ietf.org/html/rfc7231#section-3.1.1.1)
 // Alternatively, an empty string is returned unchanged, and invalid values are "simplified" to an empty string.
 func simplifyContentType(contentType string) string {
@@ -289,11 +282,6 @@ func getBlobSize(resp *http.Response) int64 {
 	return size
 }
 
-// HasThreadSafeGetBlob indicates whether GetBlob can be executed concurrently.
-func (s *dockerImageSource) HasThreadSafeGetBlob() bool {
-	return true
-}
-
 // splitHTTP200ResponseToPartial splits a 200 response in multiple streams as specified by the chunks
 func splitHTTP200ResponseToPartial(streams chan io.ReadCloser, errs chan error, body io.ReadCloser, chunks []private.ImageSourceChunk) {
 	defer close(streams)

docker/internal/tarfile/src.go

@@ -13,6 +13,8 @@ import (
 	"sync"
 
 	"github.com/containers/image/v5/docker/reference"
+	"github.com/containers/image/v5/internal/imagesource/impl"
+	"github.com/containers/image/v5/internal/imagesource/stubs"
 	"github.com/containers/image/v5/internal/iolimits"
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/image/v5/pkg/compression"
@@ -23,6 +25,11 @@ import (
 
 // Source is a partial implementation of types.ImageSource for reading from tarPath.
 type Source struct {
+	impl.PropertyMethodsInitialize
+	impl.NoSignatures
+	impl.DoesNotAffectLayerInfosForCopy
+	stubs.NoGetBlobAtInitialize
+
 	archive      *Reader
 	closeArchive bool // .Close() the archive when the source is closed.
 	// If ref is nil and sourceIndex is -1, indicates the only image in the archive.
@@ -48,8 +55,13 @@ type layerInfo struct {
 // NewSource returns a tarfile.Source for an image in the specified archive matching ref
 // and sourceIndex (or the only image if they are (nil, -1)).
 // The archive will be closed if closeArchive
-func NewSource(archive *Reader, closeArchive bool, ref reference.NamedTagged, sourceIndex int) *Source {
+func NewSource(archive *Reader, closeArchive bool, transportName string, ref reference.NamedTagged, sourceIndex int) *Source {
 	return &Source{
+		PropertyMethodsInitialize: impl.PropertyMethods(impl.Properties{
+			HasThreadSafeGetBlob: true,
+		}),
+		NoGetBlobAtInitialize: stubs.NoGetBlobAtRaw(transportName),
+
 		archive:      archive,
 		closeArchive: closeArchive,
 		ref:          ref,
@@ -250,11 +262,6 @@ func (r uncompressedReadCloser) Close() error {
 	return res
 }
 
-// HasThreadSafeGetBlob indicates whether GetBlob can be executed concurrently.
-func (s *Source) HasThreadSafeGetBlob() bool {
-	return true
-}
-
 // GetBlob returns a stream for the specified blob, and the blob’s size (or -1 if unknown).
 // The Digest field in BlobInfo is guaranteed to be provided, Size may be -1 and MediaType may be optionally provided.
 // May update BlobInfoCache, preferably after it knows for certain that a blob truly exists at a specific location.
@@ -308,25 +315,3 @@ func (s *Source) GetBlob(ctx context.Context, info types.BlobInfo, cache types.B
 
 	return nil, 0, fmt.Errorf("Unknown blob %s", info.Digest)
 }
-
-// GetSignatures returns the image's signatures.  It may use a remote (= slow) service.
-// This source implementation does not support manifest lists, so the passed-in instanceDigest should always be nil,
-// as there can be no secondary manifests.
-func (s *Source) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) {
-	if instanceDigest != nil {
-		// How did we even get here? GetManifest(ctx, nil) has returned a manifest.DockerV2Schema2MediaType.
-		return nil, errors.New(`Manifest lists are not supported by "docker-daemon:"`)
-	}
-	return [][]byte{}, nil
-}
-
-// 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.
-// This source implementation does not support manifest lists, so the passed-in instanceDigest should always be nil,
-// as the primary manifest can not be a list, so there can be no secondary manifests.
-// 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 *Source) LayerInfosForCopy(context.Context, *digest.Digest) ([]types.BlobInfo, error) {
-	return nil, nil
-}

docker/internal/tarfile/src_test.go

@@ -47,7 +47,7 @@ func TestSourcePrepareLayerData(t *testing.T) {
 
 		reader, err := NewReaderFromStream(nil, &tarfileBuffer)
 		require.NoError(t, err, c.config)
-		src := NewSource(reader, true, nil, -1)
+		src := NewSource(reader, true, "transport name", nil, -1)
 		require.NoError(t, err, c.config)
 		defer src.Close()
 		configStream, _, err := src.GetBlob(ctx, types.BlobInfo{

docker/tarfile/src.go

@@ -28,7 +28,7 @@ func NewSourceFromFileWithContext(sys *types.SystemContext, path string) (*Sourc
 	if err != nil {
 		return nil, err
 	}
-	src := internal.NewSource(archive, true, nil, -1)
+	src := internal.NewSource(archive, true, "[An external docker/tarfile caller]", nil, -1)
 	return &Source{internal: src}, nil
 }
 
@@ -49,7 +49,7 @@ func NewSourceFromStreamWithSystemContext(sys *types.SystemContext, inputStream
 	if err != nil {
 		return nil, err
 	}
-	src := internal.NewSource(archive, true, nil, -1)
+	src := internal.NewSource(archive, true, "[An external docker/tarfile caller]", nil, -1)
 	return &Source{internal: src}, nil
 }
 

internal/imagesource/impl/layer_infos.go

@@ -0,0 +1,23 @@
+package impl
+
+import (
+	"context"
+
+	"github.com/containers/image/v5/types"
+	"github.com/opencontainers/go-digest"
+)
+
+// DoesNotAffectLayerInfosForCopy implements LayerInfosForCopy() that returns nothing.
+type DoesNotAffectLayerInfosForCopy struct{}
+
+// 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 (stub DoesNotAffectLayerInfosForCopy) LayerInfosForCopy(ctx context.Context, instanceDigest *digest.Digest) ([]types.BlobInfo, error) {
+	return nil, nil
+}

internal/imagesource/impl/properties.go

@@ -0,0 +1,27 @@
+package impl
+
+// Properties collects properties of an ImageSource that are constant throughout its lifetime
+// (but might differ across instances).
+type Properties struct {
+	// HasThreadSafeGetBlob indicates whether GetBlob can be executed concurrently.
+	HasThreadSafeGetBlob bool
+}
+
+// PropertyMethodsInitialize implements parts of private.ImageSource corresponding to Properties.
+type PropertyMethodsInitialize struct {
+	// We need two separate structs, PropertyMethodsInitialize and Properties, because Go prohibits fields and methods with the same name.
+
+	vals Properties
+}
+
+// PropertyMethods creates an PropertyMethodsInitialize for vals.
+func PropertyMethods(vals Properties) PropertyMethodsInitialize {
+	return PropertyMethodsInitialize{
+		vals: vals,
+	}
+}
+
+// HasThreadSafeGetBlob indicates whether GetBlob can be executed concurrently.
+func (o PropertyMethodsInitialize) HasThreadSafeGetBlob() bool {
+	return o.vals.HasThreadSafeGetBlob
+}

internal/imagesource/impl/signatures.go

@@ -0,0 +1,18 @@
+package impl
+
+import (
+	"context"
+
+	"github.com/opencontainers/go-digest"
+)
+
+// NoSignatures implements GetSignatures() that returns nothing.
+type NoSignatures struct{}
+
+// GetSignatures returns the image's signatures.  It may use a remote (= slow) service.
+// If instanceDigest is not nil, it contains a digest of the specific manifest instance to retrieve signatures 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).
+func (stub NoSignatures) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) {
+	return nil, nil
+}