reimplmeemnt ociArchiveImage{Source,Destination}

Signed-off-by: Qi Wang <[email protected]>

docs/containers-transports.5.md

@@ -61,7 +61,7 @@ The _algo:digest_ refers to the image ID reported by docker-inspect(1).
 
 An image compliant with the "Open Container Image Layout Specification" at _path_.
 Using a _tag_ is optional and allows for storing multiple images at the same _path_.
-@_source-index_ is a zero-based index in manifest (to access untagged images).
+For reading images, @_source-index_ is a zero-based index in manifest (to access untagged images).
 If neither tag nor @_source_index is specified when reading an image, the path must contain exactly one image.
 
 ### **oci-archive:**_path[:{tag|@source-index}]_

oci/archive/oci_dest.go

@@ -13,19 +13,30 @@ import (
 )
 
 type ociArchiveImageDestination struct {
-	ref          ociArchiveReference
-	unpackedDest types.ImageDestination
-	tempDirRef   tempDirOCIRef
+	ref              ociArchiveReference
+	unpackedDest     types.ImageDestination
+	tempDirRef       tempDirOCIRef
+	archiveWriterRef *tempDirOCIRef
 }
 
 // newImageDestination returns an ImageDestination for writing to an existing directory.
-func newImageDestination(ctx context.Context, sys *types.SystemContext, ref ociArchiveReference) (types.ImageDestination, error) {
+func newImageDestination(ctx context.Context, sys *types.SystemContext, ref ociArchiveReference, archiveWriterRef *tempDirOCIRef) (types.ImageDestination, error) {
+	var (
+		tempDirRef tempDirOCIRef
+		err        error
+	)
+
 	if ref.sourceIndex != -1 {
-		return nil, errors.Errorf("invalid sourceIndex %d for creating image destination", ref.sourceIndex)
+		return nil, errors.Errorf("Destination reference must not contain a manifest index @%d", ref.sourceIndex)
 	}
-	tempDirRef, err := createOCIRef(sys, ref.image, -1)
-	if err != nil {
-		return nil, errors.Wrapf(err, "error creating oci reference")
+
+	if archiveWriterRef != nil {
+		tempDirRef = *archiveWriterRef
+	} else {
+		tempDirRef, err = createOCIRef(sys, ref.image, -1)
+		if err != nil {
+			return nil, errors.Wrapf(err, "error creating oci reference")
+		}
 	}
 	unpackedDest, err := tempDirRef.ociRefExtracted.NewImageDestination(ctx, sys)
 	if err != nil {
@@ -35,8 +46,9 @@ func newImageDestination(ctx context.Context, sys *types.SystemContext, ref ociA
 		return nil, err
 	}
 	return &ociArchiveImageDestination{ref: ref,
-		unpackedDest: unpackedDest,
-		tempDirRef:   tempDirRef}, nil
+		unpackedDest:     unpackedDest,
+		archiveWriterRef: archiveWriterRef,
+		tempDirRef:       tempDirRef}, nil
 }
 
 // Reference returns the reference used to set up this destination.
@@ -47,6 +59,9 @@ func (d *ociArchiveImageDestination) Reference() types.ImageReference {
 // Close removes resources associated with an initialized ImageDestination, if any
 // Close deletes the temp directory of the oci-archive image
 func (d *ociArchiveImageDestination) Close() error {
+	if d.archiveWriterRef != nil {
+		return nil
+	}
 	defer func() {
 		err := d.tempDirRef.deleteTempDir()
 		logrus.Debugf("Error deleting temporary directory: %v", err)
@@ -136,6 +151,10 @@ func (d *ociArchiveImageDestination) Commit(ctx context.Context, unparsedTopleve
 		return errors.Wrapf(err, "error storing image %q", d.ref.image)
 	}
 
+	if d.archiveWriterRef != nil {
+		return nil
+	}
+
 	// path of directory to tar up
 	src := d.tempDirRef.tempDirectory
 	// path to save tarred up file

oci/archive/oci_src.go

@@ -13,19 +13,27 @@ import (
 )
 
 type ociArchiveImageSource struct {
-	ref         ociArchiveReference
-	unpackedSrc types.ImageSource
-	tempDirRef  tempDirOCIRef
+	ref              ociArchiveReference
+	unpackedSrc      types.ImageSource
+	tempDirRef       tempDirOCIRef
+	archiveReaderRef *tempDirOCIRef
 }
 
 // newImageSource returns an ImageSource for reading from an existing directory.
 // newImageSource untars the file and saves it in a temp directory
-func newImageSource(ctx context.Context, sys *types.SystemContext, ref ociArchiveReference) (types.ImageSource, error) {
-	tempDirRef, err := createUntarTempDir(sys, ref)
-	if err != nil {
-		return nil, errors.Wrap(err, "error creating temp directory")
+func newImageSource(ctx context.Context, sys *types.SystemContext, ref ociArchiveReference, archiveReaderRef *tempDirOCIRef) (types.ImageSource, error) {
+	var (
+		tempDirRef tempDirOCIRef
+		err        error
+	)
+	if archiveReaderRef != nil {
+		tempDirRef = *archiveReaderRef
+	} else {
+		tempDirRef, err = createUntarTempDir(sys, ref)
+		if err != nil {
+			return nil, errors.Wrap(err, "error creating temp directory")
+		}
 	}
-
 	unpackedSrc, err := tempDirRef.ociRefExtracted.NewImageSource(ctx, sys)
 	if err != nil {
 		if err := tempDirRef.deleteTempDir(); err != nil {
@@ -34,8 +42,9 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref ociArchiv
 		return nil, err
 	}
 	return &ociArchiveImageSource{ref: ref,
-		unpackedSrc: unpackedSrc,
-		tempDirRef:  tempDirRef}, nil
+		unpackedSrc:      unpackedSrc,
+		archiveReaderRef: archiveReaderRef,
+		tempDirRef:       tempDirRef}, nil
 }
 
 // LoadManifestDescriptor loads the manifest
@@ -74,6 +83,9 @@ func (s *ociArchiveImageSource) Reference() types.ImageReference {
 // Close removes resources associated with an initialized ImageSource, if any.
 // Close deletes the temporary directory at dst
 func (s *ociArchiveImageSource) Close() error {
+	if s.archiveReaderRef != nil {
+		return nil
+	}
 	defer func() {
 		err := s.tempDirRef.deleteTempDir()
 		logrus.Debugf("error deleting tmp dir: %v", err)

oci/archive/oci_transport.go

@@ -32,10 +32,12 @@ type ociArchiveTransport struct{}
 
 // ociArchiveReference is an ImageReference for OCI Archive paths
 type ociArchiveReference struct {
-	file         string
-	resolvedFile string
-	image        string
-	sourceIndex  int
+	file             string
+	resolvedFile     string
+	image            string
+	sourceIndex      int
+	archiveReaderRef *tempDirOCIRef
+	archiveWriterRef *tempDirOCIRef
 }
 
 func (t ociArchiveTransport) Name() string {
@@ -55,20 +57,19 @@ func (t ociArchiveTransport) ValidatePolicyConfigurationScope(scope string) erro
 
 // ParseReference converts a string, which should not start with the ImageTransport.Name prefix, into an OCI ImageReference.
 func ParseReference(reference string) (types.ImageReference, error) {
-	file, image := internal.SplitPathAndImage(reference)
-	image, index, err := internal.ParseOCIReferenceName(image)
+	file, image, index, err := internal.ParseReferenceIntoElements(reference)
 	if err != nil {
 		return nil, err
 	}
-	return newReference(file, image, index)
+	return newReference(file, image, index, nil, nil)
 }
 
 // NewReference returns an OCI reference for a file and an image.
 func NewReference(file, image string) (types.ImageReference, error) {
-	return newReference(file, image, -1)
+	return newReference(file, image, -1, nil, nil)
 }
 
-func newReference(file, image string, sourceIndex int) (types.ImageReference, error) {
+func newReference(file, image string, sourceIndex int, archiveReaderRef, archiveWriterRef *tempDirOCIRef) (types.ImageReference, error) {
 	resolved, err := explicitfilepath.ResolvePathToFullyExplicit(file)
 	if err != nil {
 		return nil, err
@@ -85,7 +86,10 @@ func newReference(file, image string, sourceIndex int) (types.ImageReference, er
 	if sourceIndex != -1 && sourceIndex < 0 {
 		return nil, errors.Errorf("Invalid oci archive: reference: index @%d must not be negative", sourceIndex)
 	}
-	return ociArchiveReference{file: file, resolvedFile: resolved, image: image, sourceIndex: sourceIndex}, nil
+	if sourceIndex != -1 && image != "" {
+		return nil, errors.Errorf("Can not set image %s and index @%d at same time", image, sourceIndex)
+	}
+	return ociArchiveReference{file: file, resolvedFile: resolved, image: image, sourceIndex: sourceIndex, archiveReaderRef: archiveReaderRef, archiveWriterRef: archiveWriterRef}, nil
 }
 
 func (ref ociArchiveReference) Transport() types.ImageTransport {
@@ -138,7 +142,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)
+	src, err := newImageSource(ctx, sys, ref, ref.archiveReaderRef)
 	if err != nil {
 		return nil, err
 	}
@@ -148,13 +152,13 @@ func (ref ociArchiveReference) NewImage(ctx context.Context, sys *types.SystemCo
 // NewImageSource returns a types.ImageSource for this reference.
 // The caller must call .Close() on the returned ImageSource.
 func (ref ociArchiveReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) {
-	return newImageSource(ctx, sys, ref)
+	return newImageSource(ctx, sys, ref, ref.archiveReaderRef)
 }
 
 // NewImageDestination returns a types.ImageDestination for this reference.
 // The caller must call .Close() on the returned ImageDestination.
 func (ref ociArchiveReference) NewImageDestination(ctx context.Context, sys *types.SystemContext) (types.ImageDestination, error) {
-	return newImageDestination(ctx, sys, ref)
+	return newImageDestination(ctx, sys, ref, ref.archiveWriterRef)
 }
 
 // DeleteImage deletes the named image from the registry, if supported.
@@ -180,9 +184,15 @@ func createOCIRef(sys *types.SystemContext, image string, sourceIndex int) (temp
 	if err != nil {
 		return tempDirOCIRef{}, errors.Wrapf(err, "error creating temp directory")
 	}
-	ociRef, err := ocilayout.NewReferenceWithIndex(dir, image, sourceIndex)
-	if err != nil {
-		return tempDirOCIRef{}, err
+	var ociRef types.ImageReference
+	if sourceIndex > -1 {
+		if ociRef, err = ocilayout.NewIndexReference(dir, sourceIndex); err != nil {
+			return tempDirOCIRef{}, err
+		}
+	} else {
+		if ociRef, err = ocilayout.NewReference(dir, image); err != nil {
+			return tempDirOCIRef{}, err
+		}
 	}
 
 	tempDirRef := tempDirOCIRef{tempDirectory: dir, ociRefExtracted: ociRef}

oci/archive/oci_transport_test.go

@@ -66,12 +66,12 @@ func testParseReference(t *testing.T, fn func(string) (types.ImageReference, err
 		}{
 			{":notlatest:image", "notlatest:image", -1},
 			{":latestimage", "latestimage", -1},
-			{":", "", -1},
+			{":busybox@0", "busybox@0", -1},
+			{":", "", -1}, // No Image
 			{"", "", -1},
-			{":@0", "", 0},
+			{":@0", "", 0}, // Explicit sourceIndex of image
 			{":@10", "", 10},
 			{":@999999", "", 999999},
-			{":busybox@0", "busybox@0", -1},
 		} {
 			input := path + image.suffix
 			ref, err := fn(input)
@@ -91,6 +91,7 @@ func testParseReference(t *testing.T, fn func(string) (types.ImageReference, err
 		":@-2",
 		":@busybox",
 		":@0:buxybox",
+		":latestImage@1",
 	} {
 		input := tmpDir + imageSuffix
 		ref, err := fn(input)
@@ -131,6 +132,10 @@ func TestNewReference(t *testing.T) {
 
 	_, err = NewReference(tmpDir+"/has:colon", imageValue)
 	assert.Error(t, err)
+
+	// Test private newReference
+	_, err = newReference(tmpDir, "imageName", 1, nil, nil) // Both image and sourceIndex specified
+	assert.Error(t, err)
 }
 
 // refToTempOCI creates a temporary directory and returns an reference to it.

oci/archive/reader.go

@@ -19,11 +19,12 @@ import (
 type Reader struct {
 	manifest      *imgspecv1.Index
 	tempDirectory string
+	path          string // The original, user-specified path; not the maintained temporary file, if any
 }
 
 // NewReader creates the temp directory that keeps the untarred archive from src.
 // The caller should call .Close() on the returned object.
-func NewReader(ctx context.Context, src string, sys *types.SystemContext) (*Reader, error) {
+func NewReader(ctx context.Context, sys *types.SystemContext, src string) (*Reader, error) {
 	// TODO: This can take quite some time, and should ideally be cancellable using a context.Context.
 	arch, err := os.Open(src)
 	if err != nil {
@@ -38,12 +39,12 @@ func NewReader(ctx context.Context, src string, sys *types.SystemContext) (*Read
 
 	reader := Reader{
 		tempDirectory: dst,
+		path:          src,
 	}
 
+	succeeded := true
 	if err := archive.NewDefaultArchiver().Untar(arch, dst, &archive.TarOptions{NoLchown: true}); err != nil {
-		if err := reader.Close(); err != nil {
-			return nil, errors.Wrapf(err, "error deleting temp directory %q", dst)
-		}
+		succeeded = false
 		return nil, errors.Wrapf(err, "error untarring file %q", dst)
 	}
 
@@ -54,9 +55,14 @@ func NewReader(ctx context.Context, src string, sys *types.SystemContext) (*Read
 	defer indexJSON.Close()
 	reader.manifest = &imgspecv1.Index{}
 	if err := json.NewDecoder(indexJSON).Decode(reader.manifest); err != nil {
+		succeeded = false
 		return nil, err
 	}
-
+	defer func() {
+		if !succeeded {
+			reader.Close()
+		}
+	}()
 	return &reader, nil
 }
 
@@ -84,10 +90,18 @@ func (r *Reader) List() (map[string]types.ImageReference, error) {
 				return nil, err
 			}
 		}
+		archiveReaderRef := &tempDirOCIRef{
+			tempDirectory:   r.tempDirectory,
+			ociRefExtracted: ref,
+		}
+		archiveRef, err := newReference(r.path, "", -1, archiveReaderRef, nil)
+		if err != nil {
+			return nil, errors.Errorf("error creating image reference: %v", err)
+		}
 		if _, ok := res[refName]; ok {
 			return nil, errors.Errorf("image descriptor %s conflict", refName)
 		}
-		res[refName] = ref
+		res[refName] = archiveRef
 	}
 	return res, nil
 }

oci/archive/writer.go

@@ -1,10 +1,10 @@
 package archive
 
 import (
+	"context"
 	"io/ioutil"
 	"os"
 
-	"github.com/containers/image/v5/directory/explicitfilepath"
 	"github.com/containers/image/v5/internal/tmpdir"
 	"github.com/containers/image/v5/oci/layout"
 	"github.com/containers/image/v5/types"
@@ -21,26 +21,34 @@ type Writer struct {
 
 // NewWriter creates a temp directory will be tarred to oci-archive.
 // The caller should call .Close() on the returned object.
-func NewWriter(sys *types.SystemContext, file string) (*Writer, error) {
+func NewWriter(ctx context.Context, sys *types.SystemContext, file string) (*Writer, error) {
 	dir, err := ioutil.TempDir(tmpdir.TemporaryDirectoryForBigFiles(sys), "oci")
 	if err != nil {
 		return nil, errors.Wrapf(err, "error creating temp directory")
 	}
-	dst, err := explicitfilepath.ResolvePathToFullyExplicit(file)
-	if err != nil {
-		return nil, err
-	}
 	ociWriter := &Writer{
 		tempDir: dir,
-		path:    dst,
+		path:    file,
 	}
 	return ociWriter, nil
 }
 
 // NewReference returns an ImageReference that allows adding an image to Writer,
 // with an optional image name
 func (w *Writer) NewReference(name string) (types.ImageReference, error) {
-	return layout.NewReference(w.tempDir, name)
+	ref, err := layout.NewReference(w.tempDir, name)
+	if err != nil {
+		return nil, errors.Errorf("error creating image reference: %v", err)
+	}
+	archiveWriterRef := &tempDirOCIRef{
+		tempDirectory:   w.tempDir,
+		ociRefExtracted: ref,
+	}
+	archiveRef, err := newReference(w.path, "", -1, nil, archiveWriterRef)
+	if err != nil {
+		return nil, errors.Errorf("error creating image reference: %v", err)
+	}
+	return archiveRef, nil
 }
 
 // Close converts the data about images in the temp directory to the archive and