Skip to content

Commit

Permalink
Add image.FromReference, replace most users of image.FromSource
Browse files Browse the repository at this point in the history
This avoids some repetition, and (in all but one case) fixes
a missing .Close() on a failure path.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
  • Loading branch information
mtrmac committed Jun 10, 2022
1 parent 320a1ff commit 08a6499
Show file tree
Hide file tree
Showing 11 changed files with 33 additions and 58 deletions.
3 changes: 1 addition & 2 deletions directory/directory_transport.go
Expand Up @@ -140,8 +140,7 @@ func (ref dirReference) PolicyConfigurationNamespaces() []string {
// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage.
// WARNING: This may not do the right thing for a manifest list, see image.FromSource for details.
func (ref dirReference) NewImage(ctx context.Context, sys *types.SystemContext) (types.ImageCloser, error) {
src := newImageSource(ref)
return image.FromSource(ctx, sys, src)
return image.FromReference(ctx, sys, ref)
}

// NewImageSource returns a types.ImageSource for this reference.
Expand Down
6 changes: 1 addition & 5 deletions docker/archive/transport.go
Expand Up @@ -185,11 +185,7 @@ func (ref archiveReference) PolicyConfigurationNamespaces() []string {
// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage.
// WARNING: This may not do the right thing for a manifest list, see image.FromSource for details.
func (ref archiveReference) NewImage(ctx context.Context, sys *types.SystemContext) (types.ImageCloser, error) {
src, err := newImageSource(ctx, sys, ref)
if err != nil {
return nil, err
}
return ctrImage.FromSource(ctx, sys, src)
return ctrImage.FromReference(ctx, sys, ref)
}

// NewImageSource returns a types.ImageSource for this reference.
Expand Down
6 changes: 1 addition & 5 deletions docker/daemon/daemon_transport.go
Expand Up @@ -195,11 +195,7 @@ func (ref daemonReference) PolicyConfigurationNamespaces() []string {
// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage.
// WARNING: This may not do the right thing for a manifest list, see image.FromSource for details.
func (ref daemonReference) NewImage(ctx context.Context, sys *types.SystemContext) (types.ImageCloser, error) {
src, err := newImageSource(ctx, sys, ref)
if err != nil {
return nil, err
}
return image.FromSource(ctx, sys, src)
return image.FromReference(ctx, sys, ref)
}

// NewImageSource returns a types.ImageSource for this reference.
Expand Down
23 changes: 23 additions & 0 deletions internal/image/sourced.go
Expand Up @@ -9,6 +9,27 @@ import (
"github.com/containers/image/v5/types"
)

// FromReference returns a types.ImageCloser implementation for the default instance reading from reference.
// If reference poitns to a manifest list, .Manifest() still returns the manifest list,
// but other methods transparently return data from an appropriate image instance.
//
// The caller must call .Close() on the returned ImageCloser.
//
// NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource,
// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage instead of calling this function.
func FromReference(ctx context.Context, sys *types.SystemContext, ref types.ImageReference) (types.ImageCloser, error) {
src, err := ref.NewImageSource(ctx, sys)
if err != nil {
return nil, err
}
img, err := FromSource(ctx, sys, src)
if err != nil {
src.Close()
return nil, err
}
return img, nil
}

// imageCloser implements types.ImageCloser, perhaps allowing simple users
// to use a single object without having keep a reference to a types.ImageSource
// only to call types.ImageSource.Close().
Expand All @@ -31,6 +52,8 @@ type imageCloser struct {
// NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource,
// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage instead of calling this function.
//
// Most callers can use either FromUnparsedImage or FromReference instead.
//
// This is publicly visible as c/image/image.FromSource.
func FromSource(ctx context.Context, sys *types.SystemContext, src types.ImageSource) (types.ImageCloser, error) {
img, err := FromUnparsedImage(ctx, sys, UnparsedInstance(src, nil))
Expand Down
6 changes: 1 addition & 5 deletions oci/archive/oci_transport.go
Expand Up @@ -122,11 +122,7 @@ func (ref ociArchiveReference) PolicyConfigurationNamespaces() []string {
// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage.
// WARNING: This may not do the right thing for a manifest list, see image.FromSource for details.
func (ref ociArchiveReference) NewImage(ctx context.Context, sys *types.SystemContext) (types.ImageCloser, error) {
src, err := newImageSource(ctx, sys, ref)
if err != nil {
return nil, err
}
return image.FromSource(ctx, sys, src)
return image.FromReference(ctx, sys, ref)
}

// NewImageSource returns a types.ImageSource for this reference.
Expand Down
6 changes: 1 addition & 5 deletions oci/layout/oci_transport.go
Expand Up @@ -154,11 +154,7 @@ func (ref ociReference) PolicyConfigurationNamespaces() []string {
// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage.
// WARNING: This may not do the right thing for a manifest list, see image.FromSource for details.
func (ref ociReference) NewImage(ctx context.Context, sys *types.SystemContext) (types.ImageCloser, error) {
src, err := newImageSource(sys, ref)
if err != nil {
return nil, err
}
return image.FromSource(ctx, sys, src)
return image.FromReference(ctx, sys, ref)
}

// getIndex returns a pointer to the index references by this ociReference. If an error occurs opening an index nil is returned together
Expand Down
6 changes: 1 addition & 5 deletions openshift/openshift_transport.go
Expand Up @@ -132,11 +132,7 @@ func (ref openshiftReference) PolicyConfigurationNamespaces() []string {
// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage.
// WARNING: This may not do the right thing for a manifest list, see image.FromSource for details.
func (ref openshiftReference) NewImage(ctx context.Context, sys *types.SystemContext) (types.ImageCloser, error) {
src, err := newImageSource(sys, ref)
if err != nil {
return nil, err
}
return genericImage.FromSource(ctx, sys, src)
return genericImage.FromReference(ctx, sys, ref)
}

// NewImageSource returns a types.ImageSource for this reference.
Expand Down
12 changes: 1 addition & 11 deletions ostree/ostree_transport.go
Expand Up @@ -184,17 +184,7 @@ func (s *ostreeImageCloser) Size() (int64, error) {
// NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource,
// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage.
func (ref ostreeReference) NewImage(ctx context.Context, sys *types.SystemContext) (types.ImageCloser, error) {
var tmpDir string
if sys == nil || sys.OSTreeTmpDirPath == "" {
tmpDir = os.TempDir()
} else {
tmpDir = sys.OSTreeTmpDirPath
}
src, err := newImageSource(tmpDir, ref)
if err != nil {
return nil, err
}
return image.FromSource(ctx, sys, src)
return image.FromReference(ctx, sys, ref)
}

// NewImageSource returns a types.ImageSource for this reference.
Expand Down
6 changes: 1 addition & 5 deletions pkg/blobcache/blobcache.go
Expand Up @@ -158,11 +158,7 @@ func (b *BlobCache) ClearCache() error {
}

func (b *BlobCache) NewImage(ctx context.Context, sys *types.SystemContext) (types.ImageCloser, error) {
src, err := b.NewImageSource(ctx, sys)
if err != nil {
return nil, errors.Wrapf(err, "error creating new image %q", transports.ImageName(b.reference))
}
return image.FromSource(ctx, sys, src)
return image.FromReference(ctx, sys, b)
}

func (b *BlobCache) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) {
Expand Down
6 changes: 1 addition & 5 deletions sif/transport.go
Expand Up @@ -139,11 +139,7 @@ func (ref sifReference) PolicyConfigurationNamespaces() []string {
// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage.
// WARNING: This may not do the right thing for a manifest list, see image.FromSource for details.
func (ref sifReference) NewImage(ctx context.Context, sys *types.SystemContext) (types.ImageCloser, error) {
src, err := newImageSource(ctx, sys, ref)
if err != nil {
return nil, err
}
return image.FromSource(ctx, sys, src)
return image.FromReference(ctx, sys, ref)
}

// NewImageSource returns a types.ImageSource for this reference.
Expand Down
11 changes: 1 addition & 10 deletions tarball/tarball_reference.go
Expand Up @@ -67,16 +67,7 @@ func (r *tarballReference) PolicyConfigurationNamespaces() []string {
// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage.
// WARNING: This may not do the right thing for a manifest list, see image.FromSource for details.
func (r *tarballReference) NewImage(ctx context.Context, sys *types.SystemContext) (types.ImageCloser, error) {
src, err := r.NewImageSource(ctx, sys)
if err != nil {
return nil, err
}
img, err := image.FromSource(ctx, sys, src)
if err != nil {
src.Close()
return nil, err
}
return img, nil
return image.FromReference(ctx, sys, r)
}

func (r *tarballReference) DeleteImage(ctx context.Context, sys *types.SystemContext) error {
Expand Down

0 comments on commit 08a6499

Please sign in to comment.