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

Implement private.ImageSource in all transports, improve transport helpers #1590

Merged
merged 13 commits into from Jul 5, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 16 additions & 19 deletions directory/directory_src.go
Expand Up @@ -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_
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
1 change: 1 addition & 0 deletions directory/directory_test.go
Expand Up @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions docker/archive/src.go
Expand Up @@ -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"
)

Expand All @@ -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 {
Expand All @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions docker/archive/src_test.go
@@ -0,0 +1,5 @@
package archive

import "github.com/containers/image/v5/internal/private"

var _ private.ImageSource = (*archiveImageSource)(nil)
5 changes: 3 additions & 2 deletions docker/daemon/daemon_src.go
Expand Up @@ -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"
)
Expand All @@ -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")
Expand All @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions 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)
32 changes: 10 additions & 22 deletions docker/docker_image_src.go
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
41 changes: 13 additions & 28 deletions docker/internal/tarfile/src.go
Expand Up @@ -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"
Expand All @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion docker/internal/tarfile/src_test.go
Expand Up @@ -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{
Expand Down
4 changes: 2 additions & 2 deletions docker/tarfile/src.go
Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down
23 changes: 23 additions & 0 deletions 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
}
27 changes: 27 additions & 0 deletions 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
}
18 changes: 18 additions & 0 deletions 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
}