From 2de595a0f06f245f288b6e5a673205b8dcef64fe Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Mon, 2 Nov 2020 10:00:15 -0500 Subject: [PATCH] Add reader/writer for oci-archive multi image support Add reader/writer with helpers to allow podman save/load multi oci-archive images. Allow read oci-archive using source_index to point to the index from oci-archive manifest. Also reimplement ociArchiveImage{Source,Destination} to support this. Signed-off-by: Qi Wang Signed-off-by: Urvashi Mohnani --- docs/containers-transports.5.md | 11 +- oci/archive/oci_dest.go | 93 ++++----- oci/archive/oci_src.go | 74 ++++--- oci/archive/oci_transport.go | 75 +++++-- oci/archive/oci_transport_test.go | 169 +++++++++++++-- oci/archive/reader.go | 107 ++++++++++ oci/archive/writer.go | 79 +++++++ oci/internal/oci_util.go | 52 ++++- oci/internal/oci_util_test.go | 31 ++- .../fixtures/two_names_manifest/index.json | 25 +++ oci/layout/oci_dest.go | 4 + oci/layout/oci_dest_test.go | 8 +- oci/layout/oci_transport.go | 68 +++++-- oci/layout/oci_transport_test.go | 192 +++++++++++++++--- 14 files changed, 826 insertions(+), 162 deletions(-) create mode 100644 oci/archive/reader.go create mode 100644 oci/archive/writer.go create mode 100644 oci/layout/fixtures/two_names_manifest/index.json diff --git a/docs/containers-transports.5.md b/docs/containers-transports.5.md index 6c94d0a1fa..428520f603 100644 --- a/docs/containers-transports.5.md +++ b/docs/containers-transports.5.md @@ -57,14 +57,19 @@ An image stored in the docker daemon's internal storage. The image must be specified as a _docker-reference_ or in an alternative _algo:digest_ format when being used as an image source. The _algo:digest_ refers to the image ID reported by docker-inspect(1). -### **oci:**_path[:reference]_ +### **oci:**_path[:{reference|@source-index}]_ An image compliant with the "Open Container Image Layout Specification" at _path_. Using a _reference_ is optional and allows for storing multiple images at the same _path_. +For reading images, @_source-index_ is a zero-based index in manifest (to access untagged images). +If neither reference nor @_source_index is specified when reading an image, the path must contain exactly one image. -### **oci-archive:**_path[:reference]_ +### **oci-archive:**_path[:{reference|@source-index}]_ -An image compliant with the "Open Container Image Layout Specification" stored as a tar(1) archive at _path_. +An image compliant with the "Open Container Image Layout Specification" stored as a tar(1) archive at _path_. +Using a _reference_ is optional and allows for storing multiple images at the same _path_. +For reading archives, @_source-index_ is a zero-based index in archive manifest (to access untagged images). +If neither reference nor @_source_index is specified when reading an archive, the archive must contain exactly one image. ### **ostree:**_docker-reference[@/absolute/repo/path]_ diff --git a/oci/archive/oci_dest.go b/oci/archive/oci_dest.go index 3d8738db53..085bfa420e 100644 --- a/oci/archive/oci_dest.go +++ b/oci/archive/oci_dest.go @@ -2,38 +2,57 @@ package archive import ( "context" + "fmt" "io" - "os" + "github.com/containers/image/v5/oci/layout" "github.com/containers/image/v5/types" - "github.com/containers/storage/pkg/archive" digest "github.com/opencontainers/go-digest" - "github.com/pkg/errors" - "github.com/sirupsen/logrus" ) type ociArchiveImageDestination struct { - ref ociArchiveReference - unpackedDest types.ImageDestination - tempDirRef tempDirOCIRef + ref ociArchiveReference + individualWriterOrNil *Writer + unpackedDest types.ImageDestination } // newImageDestination returns an ImageDestination for writing to an existing directory. func newImageDestination(ctx context.Context, sys *types.SystemContext, ref ociArchiveReference) (types.ImageDestination, error) { - tempDirRef, err := createOCIRef(sys, ref.image) + var ( + archive, individualWriterOrNil *Writer + err error + ) + + if ref.sourceIndex != -1 { + return nil, fmt.Errorf("destination reference must not contain a manifest index @%d: %w", ref.sourceIndex, invalidOciArchiveErr) + } + + if ref.archiveWriter != nil { + archive = ref.archiveWriter + individualWriterOrNil = nil + } else { + archive, err = NewWriter(ctx, sys, ref.file) + if err != nil { + return nil, err + } + individualWriterOrNil = archive + } + newref, err := layout.NewReference(archive.tempDir, ref.image) if err != nil { - return nil, errors.Wrapf(err, "creating oci reference") + archive.Close() + return nil, err } - unpackedDest, err := tempDirRef.ociRefExtracted.NewImageDestination(ctx, sys) + dst, err := newref.NewImageDestination(ctx, sys) if err != nil { - if err := tempDirRef.deleteTempDir(); err != nil { - return nil, errors.Wrapf(err, "deleting temp directory %q", tempDirRef.tempDirectory) - } + archive.Close() return nil, err } - return &ociArchiveImageDestination{ref: ref, - unpackedDest: unpackedDest, - tempDirRef: tempDirRef}, nil + + return &ociArchiveImageDestination{ + unpackedDest: dst, + individualWriterOrNil: individualWriterOrNil, + ref: ref, + }, nil } // Reference returns the reference used to set up this destination. @@ -42,13 +61,12 @@ 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 { - defer func() { - err := d.tempDirRef.deleteTempDir() - logrus.Debugf("Error deleting temporary directory: %v", err) - }() - return d.unpackedDest.Close() + defer d.unpackedDest.Close() + if d.ref.archiveWriter != nil || d.individualWriterOrNil == nil { + return nil + } + return d.individualWriterOrNil.Close() } func (d *ociArchiveImageDestination) SupportedManifestMIMETypes() []string { @@ -135,34 +153,7 @@ func (d *ociArchiveImageDestination) PutSignatures(ctx context.Context, signatur // after the directory is made, it is tarred up into a file and the directory is deleted func (d *ociArchiveImageDestination) Commit(ctx context.Context, unparsedToplevel types.UnparsedImage) error { if err := d.unpackedDest.Commit(ctx, unparsedToplevel); err != nil { - return errors.Wrapf(err, "storing image %q", d.ref.image) + return fmt.Errorf("storing image %q: %w", d.ref.image, err) } - - // path of directory to tar up - src := d.tempDirRef.tempDirectory - // path to save tarred up file - dst := d.ref.resolvedFile - return tarDirectory(src, dst) -} - -// tar converts the directory at src and saves it to dst -func tarDirectory(src, dst string) error { - // input is a stream of bytes from the archive of the directory at path - input, err := archive.Tar(src, archive.Uncompressed) - if err != nil { - return errors.Wrapf(err, "retrieving stream of bytes from %q", src) - } - - // creates the tar file - outFile, err := os.Create(dst) - if err != nil { - return errors.Wrapf(err, "creating tar file %q", dst) - } - defer outFile.Close() - - // copies the contents of the directory to the tar file - // TODO: This can take quite some time, and should ideally be cancellable using a context.Context. - _, err = io.Copy(outFile, input) - - return err + return nil } diff --git a/oci/archive/oci_src.go b/oci/archive/oci_src.go index 20b392dc0e..7b6413f85b 100644 --- a/oci/archive/oci_src.go +++ b/oci/archive/oci_src.go @@ -2,40 +2,67 @@ package archive import ( "context" + "fmt" "io" + "github.com/containers/image/v5/oci/layout" ocilayout "github.com/containers/image/v5/oci/layout" "github.com/containers/image/v5/types" digest "github.com/opencontainers/go-digest" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" - "github.com/pkg/errors" "github.com/sirupsen/logrus" ) type ociArchiveImageSource struct { - ref ociArchiveReference - unpackedSrc types.ImageSource - tempDirRef tempDirOCIRef + ref ociArchiveReference + unpackedSrc types.ImageSource + individualReaderOrNil *Reader } // 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, "creating temp directory") + var ( + archive, individualReaderOrNil *Reader + newref types.ImageReference + err error + ) + + if ref.archiveReader != nil { + archive = ref.archiveReader + individualReaderOrNil = nil + } else { + archive, err = NewReader(ctx, sys, ref) + if err != nil { + return nil, err + } + individualReaderOrNil = archive } - unpackedSrc, err := tempDirRef.ociRefExtracted.NewImageSource(ctx, sys) - if err != nil { - if err := tempDirRef.deleteTempDir(); err != nil { - return nil, errors.Wrapf(err, "deleting temp directory %q", tempDirRef.tempDirectory) + if ref.sourceIndex != -1 { + newref, err = layout.NewIndexReference(archive.tempDirectory, ref.sourceIndex) + if err != nil { + archive.Close() + return nil, err + } + } else { + newref, err = layout.NewReference(archive.tempDirectory, ref.image) + if err != nil { + archive.Close() + return nil, err } + } + + src, err := newref.NewImageSource(ctx, sys) + if err != nil { + archive.Close() return nil, err } - return &ociArchiveImageSource{ref: ref, - unpackedSrc: unpackedSrc, - tempDirRef: tempDirRef}, nil + + return &ociArchiveImageSource{ + unpackedSrc: src, + ref: ref, + individualReaderOrNil: individualReaderOrNil, + }, nil } // LoadManifestDescriptor loads the manifest @@ -48,11 +75,11 @@ func LoadManifestDescriptor(imgRef types.ImageReference) (imgspecv1.Descriptor, func LoadManifestDescriptorWithContext(sys *types.SystemContext, imgRef types.ImageReference) (imgspecv1.Descriptor, error) { ociArchRef, ok := imgRef.(ociArchiveReference) if !ok { - return imgspecv1.Descriptor{}, errors.Errorf("error typecasting, need type ociArchiveReference") + return imgspecv1.Descriptor{}, fmt.Errorf("error typecasting, need type ociArchiveReference") } tempDirRef, err := createUntarTempDir(sys, ociArchRef) if err != nil { - return imgspecv1.Descriptor{}, errors.Wrap(err, "creating temp directory") + return imgspecv1.Descriptor{}, fmt.Errorf("creating temp directory: %w", err) } defer func() { err := tempDirRef.deleteTempDir() @@ -61,7 +88,7 @@ func LoadManifestDescriptorWithContext(sys *types.SystemContext, imgRef types.Im descriptor, err := ocilayout.LoadManifestDescriptor(tempDirRef.ociRefExtracted) if err != nil { - return imgspecv1.Descriptor{}, errors.Wrap(err, "loading index") + return imgspecv1.Descriptor{}, fmt.Errorf("loading index: %w", err) } return descriptor, nil } @@ -72,13 +99,12 @@ 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 { - defer func() { - err := s.tempDirRef.deleteTempDir() - logrus.Debugf("error deleting tmp dir: %v", err) - }() - return s.unpackedSrc.Close() + defer s.unpackedSrc.Close() + if s.ref.archiveReader != nil || s.individualReaderOrNil == nil { + return nil + } + return s.individualReaderOrNil.Close() } // GetManifest returns the image's manifest along with its MIME type (which may be empty when it can't be determined but the manifest is available). diff --git a/oci/archive/oci_transport.go b/oci/archive/oci_transport.go index 54d325d34d..08d9fcdcb8 100644 --- a/oci/archive/oci_transport.go +++ b/oci/archive/oci_transport.go @@ -23,6 +23,8 @@ func init() { transports.Register(Transport) } +var invalidOciArchiveErr error = errors.New("Invalid oci-archive: reference") + // Transport is an ImageTransport for OCI archive // it creates an oci-archive tar file by calling into the OCI transport // tarring the directory created by oci and deleting the directory @@ -32,9 +34,12 @@ type ociArchiveTransport struct{} // ociArchiveReference is an ImageReference for OCI Archive paths type ociArchiveReference struct { - file string - resolvedFile string - image string + file string + resolvedFile string + image string + sourceIndex int + archiveReader *Reader + archiveWriter *Writer } func (t ociArchiveTransport) Name() string { @@ -54,12 +59,24 @@ 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) - return NewReference(file, image) + file, image, index, err := internal.ParseReferenceIntoElements(reference) + if err != nil { + return nil, err + } + return newReference(file, image, index, nil, nil) } -// NewReference returns an OCI reference for a file and a image. +// NewReference returns an OCI reference for a file and an image. func NewReference(file, image string) (types.ImageReference, error) { + return newReference(file, image, -1, nil, nil) +} + +// NewIndexReference returns an OCI reference for a file and a zero-based source manifest index. +func NewIndexReference(file string, sourceIndex int) (types.ImageReference, error) { + return newReference(file, "", sourceIndex, nil, nil) +} + +func newReference(file, image string, sourceIndex int, archiveReader *Reader, archiveWriter *Writer) (types.ImageReference, error) { resolved, err := explicitfilepath.ResolvePathToFullyExplicit(file) if err != nil { return nil, err @@ -73,7 +90,20 @@ func NewReference(file, image string) (types.ImageReference, error) { return nil, err } - return ociArchiveReference{file: file, resolvedFile: resolved, image: image}, nil + if sourceIndex != -1 && sourceIndex < 0 { + return nil, fmt.Errorf("index @%d must not be negative: %w", sourceIndex, invalidOciArchiveErr) + } + if sourceIndex != -1 && image != "" { + return nil, fmt.Errorf("cannot set image %s and index @%d at the same time: %w", image, sourceIndex, invalidOciArchiveErr) + } + return ociArchiveReference{ + file: file, + resolvedFile: resolved, + image: image, + sourceIndex: sourceIndex, + archiveReader: archiveReader, + archiveWriter: archiveWriter, + }, nil } func (ref ociArchiveReference) Transport() types.ImageTransport { @@ -83,7 +113,10 @@ func (ref ociArchiveReference) Transport() types.ImageTransport { // StringWithinTransport returns a string representation of the reference, which MUST be such that // reference.Transport().ParseReference(reference.StringWithinTransport()) returns an equivalent reference. func (ref ociArchiveReference) StringWithinTransport() string { - return fmt.Sprintf("%s:%s", ref.file, ref.image) + if ref.sourceIndex == -1 { + return fmt.Sprintf("%s:%s", ref.file, ref.image) + } + return fmt.Sprintf("%s:@%d", ref.file, ref.sourceIndex) } // DockerReference returns a Docker reference associated with this reference @@ -144,7 +177,7 @@ func (ref ociArchiveReference) NewImageDestination(ctx context.Context, sys *typ // DeleteImage deletes the named image from the registry, if supported. func (ref ociArchiveReference) DeleteImage(ctx context.Context, sys *types.SystemContext) error { - return errors.Errorf("Deleting images not implemented for oci: images") + return fmt.Errorf("Deleting images not implemented for oci: images") } // struct to store the ociReference and temporary directory returned by createOCIRef @@ -160,14 +193,20 @@ func (t *tempDirOCIRef) deleteTempDir() error { // createOCIRef creates the oci reference of the image // If SystemContext.BigFilesTemporaryDir not "", overrides the temporary directory to use for storing big files -func createOCIRef(sys *types.SystemContext, image string) (tempDirOCIRef, error) { +func createOCIRef(sys *types.SystemContext, image string, sourceIndex int) (tempDirOCIRef, error) { dir, err := ioutil.TempDir(tmpdir.TemporaryDirectoryForBigFiles(sys), "oci") if err != nil { - return tempDirOCIRef{}, errors.Wrapf(err, "creating temp directory") + return tempDirOCIRef{}, fmt.Errorf("creating temp directory: %w", err) } - ociRef, err := ocilayout.NewReference(dir, image) - 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} @@ -176,9 +215,9 @@ func createOCIRef(sys *types.SystemContext, image string) (tempDirOCIRef, error) // creates the temporary directory and copies the tarred content to it func createUntarTempDir(sys *types.SystemContext, ref ociArchiveReference) (tempDirOCIRef, error) { - tempDirRef, err := createOCIRef(sys, ref.image) + tempDirRef, err := createOCIRef(sys, ref.image, ref.sourceIndex) if err != nil { - return tempDirOCIRef{}, errors.Wrap(err, "creating oci reference") + return tempDirOCIRef{}, fmt.Errorf("creating oci reference: %w", err) } src := ref.resolvedFile dst := tempDirRef.tempDirectory @@ -190,9 +229,9 @@ func createUntarTempDir(sys *types.SystemContext, ref ociArchiveReference) (temp defer arch.Close() if err := archive.NewDefaultArchiver().Untar(arch, dst, &archive.TarOptions{NoLchown: true}); err != nil { if err := tempDirRef.deleteTempDir(); err != nil { - return tempDirOCIRef{}, errors.Wrapf(err, "deleting temp directory %q", tempDirRef.tempDirectory) + return tempDirOCIRef{}, fmt.Errorf("deleting temp directory %q: %w", tempDirRef.tempDirectory, err) } - return tempDirOCIRef{}, errors.Wrapf(err, "untarring file %q", tempDirRef.tempDirectory) + return tempDirOCIRef{}, fmt.Errorf("untarring file %q: %w", tempDirRef.tempDirectory, err) } return tempDirRef, nil } diff --git a/oci/archive/oci_transport_test.go b/oci/archive/oci_transport_test.go index 046e2c2b9c..cecec3e8d9 100644 --- a/oci/archive/oci_transport_test.go +++ b/oci/archive/oci_transport_test.go @@ -60,11 +60,18 @@ func testParseReference(t *testing.T, fn func(string) (types.ImageReference, err "relativepath", tmpDir + "/thisdoesnotexist", } { - for _, image := range []struct{ suffix, image string }{ - {":notlatest:image", "notlatest:image"}, - {":latestimage", "latestimage"}, - {":", ""}, - {"", ""}, + for _, image := range []struct { + suffix, image string + expectedSourceIndex int + }{ + {":notlatest:image", "notlatest:image", -1}, + {":latestimage", "latestimage", -1}, + {":busybox@0", "busybox@0", -1}, + {":", "", -1}, // No Image + {"", "", -1}, + {":@0", "", 0}, // Explicit sourceIndex of image + {":@10", "", 10}, + {":@999999", "", 999999}, } { input := path + image.suffix ref, err := fn(input) @@ -73,11 +80,23 @@ func testParseReference(t *testing.T, fn func(string) (types.ImageReference, err require.True(t, ok) assert.Equal(t, path, ociArchRef.file, input) assert.Equal(t, image.image, ociArchRef.image, input) + assert.Equal(t, ociArchRef.sourceIndex, image.expectedSourceIndex, input) } } - _, err = fn(tmpDir + ":invalid'image!value@") - assert.Error(t, err) + for _, imageSuffix := range []string{ + ":invalid'image!value@", + ":@", + ":@-1", + ":@-2", + ":@busybox", + ":@0:buxybox", + } { + input := tmpDir + imageSuffix + ref, err := fn(input) + assert.Equal(t, ref, nil) + assert.Error(t, err) + } } func TestNewReference(t *testing.T) { @@ -112,12 +131,54 @@ 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) +} + +func TestNewIndexReference(t *testing.T) { + const imageValue = "imageValue" + + tmpDir, err := ioutil.TempDir("", "oci-transport-test") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + ref, err := NewIndexReference(tmpDir, 10) + require.NoError(t, err) + ociArchRef, ok := ref.(ociArchiveReference) + require.True(t, ok) + assert.Equal(t, tmpDir, ociArchRef.file) + assert.Equal(t, "", ociArchRef.image) + assert.Equal(t, 10, ociArchRef.sourceIndex) + + ref, err = NewIndexReference(tmpDir, 9999) + require.NoError(t, err) + ociArchRef, ok = ref.(ociArchiveReference) + require.True(t, ok) + assert.Equal(t, tmpDir, ociArchRef.file) + assert.Equal(t, "", ociArchRef.image) + assert.Equal(t, 9999, ociArchRef.sourceIndex) + + _, err = NewIndexReference(tmpDir+"/thisparentdoesnotexist/something", 10) + assert.Error(t, err) + + // sourceIndex cannot be less than -1 + _, err = NewIndexReference(tmpDir, -3) + assert.Error(t, err) + + _, err = NewIndexReference(tmpDir+"/has:colon", 99) + assert.Error(t, err) + + // Test private newReference + _, err = newReference(tmpDir, imageValue, 1, nil, nil) + assert.Error(t, err) } // refToTempOCI creates a temporary directory and returns an reference to it. // The caller should // defer os.RemoveAll(tmpDir) -func refToTempOCI(t *testing.T) (ref types.ImageReference, tmpDir string) { +func refToTempOCI(t *testing.T, sourceIndex bool) (ref types.ImageReference, tmpDir string) { tmpDir, err := ioutil.TempDir("", "oci-transport-test") require.NoError(t, err) m := `{ @@ -138,10 +199,34 @@ func refToTempOCI(t *testing.T) (ref types.ImageReference, tmpDir string) { ] } ` + if sourceIndex { + m = `{ + "schemaVersion": 2, + "manifests": [ + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "size": 7143, + "digest": "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", + "platform": { + "architecture": "ppc64le", + "os": "linux" + }, + } + ] + } +` + } + err = ioutil.WriteFile(filepath.Join(tmpDir, "index.json"), []byte(m), 0644) require.NoError(t, err) - ref, err = NewReference(tmpDir, "imageValue") - require.NoError(t, err) + + if sourceIndex { + ref, err = NewIndexReference(tmpDir, 1) + require.NoError(t, err) + } else { + ref, err = NewReference(tmpDir, "imageValue") + require.NoError(t, err) + } return ref, tmpDir } @@ -181,7 +266,7 @@ func refToTempOCIArchive(t *testing.T) (ref types.ImageReference, tmpTarFile str } func TestReferenceTransport(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) assert.Equal(t, Transport, ref.Transport()) } @@ -193,7 +278,8 @@ func TestReferenceStringWithinTransport(t *testing.T) { for _, c := range []struct{ input, result string }{ {"/dir1:notlatest:notlatest", "/dir1:notlatest:notlatest"}, // Explicit image - {"/dir3:", "/dir3:"}, // No image + {"/dir3:", "/dir3:"}, // No image + {"/dir1:@1", "/dir1:@1"}, // Explicit sourceIndex of image } { ref, err := ParseReference(tmpDir + c.input) require.NoError(t, err, c.input) @@ -208,13 +294,13 @@ func TestReferenceStringWithinTransport(t *testing.T) { } func TestReferenceDockerReference(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) assert.Nil(t, ref.DockerReference()) } func TestReferencePolicyConfigurationIdentity(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) assert.Equal(t, tmpDir, ref.PolicyConfigurationIdentity()) @@ -228,10 +314,26 @@ func TestReferencePolicyConfigurationIdentity(t *testing.T) { ref, err = NewReference("/", "image3") require.NoError(t, err) assert.Equal(t, "/", ref.PolicyConfigurationIdentity()) + + // Test the sourceIndex case + ref, tmpDir = refToTempOCI(t, true) + defer os.RemoveAll(tmpDir) + + assert.Equal(t, tmpDir, ref.PolicyConfigurationIdentity()) + // A non-canonical path. Test just one, the various other cases are + // tested in explicitfilepath.ResolvePathToFullyExplicit. + ref, err = NewIndexReference(tmpDir+"/.", 1) + require.NoError(t, err) + assert.Equal(t, tmpDir, ref.PolicyConfigurationIdentity()) + + // "/" as a corner case. + ref, err = NewIndexReference("/", 2) + require.NoError(t, err) + assert.Equal(t, "/", ref.PolicyConfigurationIdentity()) } func TestReferencePolicyConfigurationNamespaces(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) // We don't really know enough to make a full equality test here. ns := ref.PolicyConfigurationNamespaces() @@ -260,10 +362,41 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) { ref, err := NewReference("/", "image3") require.NoError(t, err) assert.Equal(t, []string{}, ref.PolicyConfigurationNamespaces()) + + // Test the sourceIndex case + ref, tmpDir = refToTempOCI(t, true) + defer os.RemoveAll(tmpDir) + // We don't really know enough to make a full equality test here. + ns = ref.PolicyConfigurationNamespaces() + require.NotNil(t, ns) + assert.True(t, len(ns) >= 2) + assert.Equal(t, tmpDir, ns[0]) + assert.Equal(t, filepath.Dir(tmpDir), ns[1]) + + // Test with a known path which should exist. Test just one non-canonical + // path, the various other cases are tested in explicitfilepath.ResolvePathToFullyExplicit. + // + // It would be nice to test a deeper hierarchy, but it is not obvious what + // deeper path is always available in the various distros, AND is not likely + // to contains a symbolic link. + for _, path := range []string{"/usr/share", "/usr/share/./."} { + _, err := os.Lstat(path) + require.NoError(t, err) + ref, err := NewIndexReference(path, 1) + require.NoError(t, err) + ns := ref.PolicyConfigurationNamespaces() + require.NotNil(t, ns) + assert.Equal(t, []string{"/usr/share", "/usr"}, ns) + } + + // "/" as a corner case. + ref, err = NewIndexReference("/", 2) + require.NoError(t, err) + assert.Equal(t, []string{}, ref.PolicyConfigurationNamespaces()) } func TestReferenceNewImage(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) _, err := ref.NewImage(context.Background(), nil) assert.Error(t, err) @@ -277,7 +410,7 @@ func TestReferenceNewImageSource(t *testing.T) { } func TestReferenceNewImageDestination(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) dest, err := ref.NewImageDestination(context.Background(), nil) assert.NoError(t, err) @@ -285,7 +418,7 @@ func TestReferenceNewImageDestination(t *testing.T) { } func TestReferenceDeleteImage(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) err := ref.DeleteImage(context.Background(), nil) assert.Error(t, err) diff --git a/oci/archive/reader.go b/oci/archive/reader.go new file mode 100644 index 0000000000..fedc344802 --- /dev/null +++ b/oci/archive/reader.go @@ -0,0 +1,107 @@ +package archive + +import ( + "context" + "encoding/json" + "fmt" + "io/ioutil" + "os" + "path/filepath" + + "github.com/containers/image/v5/internal/tmpdir" + "github.com/containers/image/v5/oci/internal" + "github.com/containers/image/v5/transports" + "github.com/containers/image/v5/types" + "github.com/containers/storage/pkg/archive" + imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" +) + +// Reader manages the temp directory that the oci archive is untarred to and the +// manifest of the images. It allows listing its contents and accessing +// individual images with less overhead than creating image references individually +// (because the archive is, if necessary, copied or decompressed only once) +type Reader struct { + manifest *imgspecv1.Index + tempDirectory string + path string // The original, user-specified path +} + +// NewReader returns a Reader for src. The caller should call Close() on the returned object +func NewReader(ctx context.Context, sys *types.SystemContext, ref types.ImageReference) (*Reader, error) { + standalone, ok := ref.(ociArchiveReference) + if !ok { + return nil, fmt.Errorf("Internal error: NewReader called for a non-oci/archive ImageReference %s", transports.ImageName(ref)) + } + if standalone.archiveReader != nil { + return nil, fmt.Errorf("Internal error: NewReader called for a reader-bound reference %s", standalone.StringWithinTransport()) + } + + src := standalone.resolvedFile + arch, err := os.Open(src) + if err != nil { + return nil, err + } + defer arch.Close() + + dst, err := ioutil.TempDir(tmpdir.TemporaryDirectoryForBigFiles(sys), "oci") + if err != nil { + return nil, fmt.Errorf("error creating temp directory: %w", err) + } + + reader := Reader{ + tempDirectory: dst, + path: src, + } + + succeeded := false + defer func() { + if !succeeded { + reader.Close() + } + }() + if err := archive.NewDefaultArchiver().Untar(arch, dst, &archive.TarOptions{NoLchown: true}); err != nil { + return nil, fmt.Errorf("error untarring file %q: %w", dst, err) + } + + indexJSON, err := os.Open(filepath.Join(dst, "index.json")) + if err != nil { + return nil, err + } + defer indexJSON.Close() + reader.manifest = &imgspecv1.Index{} + if err := json.NewDecoder(indexJSON).Decode(reader.manifest); err != nil { + return nil, err + } + succeeded = true + return &reader, nil +} + +// ListResult wraps the image reference and the manifest for loading +type ListResult struct { + ImageRef types.ImageReference + ManifestDescriptor imgspecv1.Descriptor +} + +// List returns a slice of manifests included in the archive +func (r *Reader) List() ([]ListResult, error) { + var res []ListResult + + for _, md := range r.manifest.Manifests { + refName := internal.NameFromAnnotations(md.Annotations) + ref, err := newReference(r.path, refName, -1, r, nil) + if err != nil { + return nil, fmt.Errorf("error creating image reference: %w", err) + } + reference := ListResult{ + ImageRef: ref, + ManifestDescriptor: md, + } + res = append(res, reference) + } + return res, nil +} + +// Close deletes temporary files associated with the Reader, if any. +func (r *Reader) Close() error { + return os.RemoveAll(r.tempDirectory) +} diff --git a/oci/archive/writer.go b/oci/archive/writer.go new file mode 100644 index 0000000000..59e120096f --- /dev/null +++ b/oci/archive/writer.go @@ -0,0 +1,79 @@ +package archive + +import ( + "context" + "fmt" + "io" + "io/ioutil" + "os" + + "github.com/containers/image/v5/internal/tmpdir" + "github.com/containers/image/v5/types" + "github.com/containers/storage/pkg/archive" +) + +// Writer manages an in-progress OCI archive and allows adding images to it +type Writer struct { + // tempDir will be tarred to oci archive + tempDir string + // user-specified path + path string +} + +// NewWriter creates a temp directory will be tarred to oci-archive. +// The caller should call .Close() on the returned object. +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, fmt.Errorf("error creating temp directory: %w", err) + } + ociWriter := &Writer{ + tempDir: dir, + 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) { + ref, err := newReference(w.path, name, -1, nil, w) + if err != nil { + return nil, fmt.Errorf("error creating image reference: %w", err) + } + return ref, nil +} + +// Close writes all outstanding data about images to the archive, and +// releases state associated with the Writer, if any. +// It deletes any temporary files associated with the Writer. +// No more images can be added after this is called. +func (w *Writer) Close() error { + err := tarDirectory(w.tempDir, w.path) + if err2 := os.RemoveAll(w.tempDir); err2 != nil && err == nil { + err = err2 + } + return err +} + +// tar converts the directory at src and saves it to dst +func tarDirectory(src, dst string) error { + // input is a stream of bytes from the archive of the directory at path + input, err := archive.Tar(src, archive.Uncompressed) + if err != nil { + return fmt.Errorf("retrieving stream of bytes from %q: %w", src, err) + } + + // creates the tar file + outFile, err := os.Create(dst) + if err != nil { + return fmt.Errorf("creating tar file %q: %w", dst, err) + } + defer outFile.Close() + + // copies the contents of the directory to the tar file + // TODO: This can take quite some time, and should ideally be cancellable using a context.Context. + _, err = io.Copy(outFile, input) + + return err +} diff --git a/oci/internal/oci_util.go b/oci/internal/oci_util.go index c2012e50e0..929990b4be 100644 --- a/oci/internal/oci_util.go +++ b/oci/internal/oci_util.go @@ -1,11 +1,15 @@ package internal import ( - "github.com/pkg/errors" + "fmt" "path/filepath" "regexp" "runtime" + "strconv" "strings" + + imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/pkg/errors" ) // annotation spex from https://github.com/opencontainers/image-spec/blob/master/annotations.md#pre-defined-annotation-keys @@ -32,9 +36,9 @@ func ValidateImageName(image string) error { return err } -// SplitPathAndImage tries to split the provided OCI reference into the OCI path and image. +// splitPathAndImage tries to split the provided OCI reference into the OCI path and image. // Neither path nor image parts are validated at this stage. -func SplitPathAndImage(reference string) (string, string) { +func splitPathAndImage(reference string) (string, string) { if runtime.GOOS == "windows" { return splitPathAndImageWindows(reference) } @@ -124,3 +128,45 @@ func validateScopeNonWindows(scope string) error { return nil } + +// parseOCIReferenceName parses the image from the oci reference. +func parseOCIReferenceName(image string) (img string, index int, err error) { + index = -1 + if strings.HasPrefix(image, "@") { + idx, err := strconv.Atoi(image[1:]) + if err != nil { + return "", index, fmt.Errorf("Invalid source index @%s: not an integer: %w", image[1:], err) + } + if idx < 0 { + return "", index, fmt.Errorf("Invalid source index @%d: must not be negative", idx) + } + index = idx + } else { + img = image + } + return img, index, nil +} + +// ParseReferenceIntoElements splits the oci reference into location, image name and source index if exists +func ParseReferenceIntoElements(reference string) (string, string, int, error) { + dir, image := splitPathAndImage(reference) + image, index, err := parseOCIReferenceName(image) + if err != nil { + return "", "", -1, err + } + return dir, image, index, nil +} + +// NameFromAnnotations returns a reference string to be used as an image name, +// or an empty string. The annotations map may be nil. +func NameFromAnnotations(annotations map[string]string) string { + if annotations == nil { + return "" + } + // buildkit/containerd are using a custom annotation see + // containers/podman/issues/12560. + if annotations["io.containerd.image.name"] != "" { + return annotations["io.containerd.image.name"] + } + return annotations[imgspecv1.AnnotationRefName] +} diff --git a/oci/internal/oci_util_test.go b/oci/internal/oci_util_test.go index b10071d2d7..d0d273359a 100644 --- a/oci/internal/oci_util_test.go +++ b/oci/internal/oci_util_test.go @@ -2,8 +2,9 @@ package internal import ( "fmt" - "github.com/stretchr/testify/assert" "testing" + + "github.com/stretchr/testify/assert" ) type testDataSplitReference struct { @@ -17,6 +18,12 @@ type testDataScopeValidation struct { errMessage string } +type testOCIReference struct { + ref string + image string + index int +} + func TestSplitReferenceIntoDirAndImageWindows(t *testing.T) { tests := []testDataSplitReference{ {`C:\foo\bar:busybox:latest`, `C:\foo\bar`, "busybox:latest"}, @@ -60,3 +67,25 @@ func TestValidateScopeWindows(t *testing.T) { } } } + +func TestParseOCIReferenceName(t *testing.T) { + validTests := []testOCIReference{ + {"@0", "", 0}, + {"notlatest@1", "notlatest@1", -1}, + } + for _, test := range validTests { + img, idx, err := parseOCIReferenceName(test.ref) + assert.NoError(t, err) + assert.Equal(t, img, test.image) + assert.Equal(t, idx, test.index) + } + + invalidTests := []string{ + "@-5", + "@invalidIndex", + } + for _, test := range invalidTests { + _, _, err := parseOCIReferenceName(test) + assert.Error(t, err) + } +} diff --git a/oci/layout/fixtures/two_names_manifest/index.json b/oci/layout/fixtures/two_names_manifest/index.json new file mode 100644 index 0000000000..8947602ddb --- /dev/null +++ b/oci/layout/fixtures/two_names_manifest/index.json @@ -0,0 +1,25 @@ +{ + "schemaVersion": 2, + "manifests": [ + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "size": 7143, + "digest": "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", + "platform": { + "architecture": "ppc64le", + "os": "linux" + }, + "annotations": { + "org.opencontainers.image.ref.name": "imageValue0" + } + }, + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:62ae1939cdb49e93f32cdb29d61ad276552532c6ae7fd543de7bf22511e4965e", + "size": 348, + "annotations": { + "org.opencontainers.image.ref.name": "imageValue1" + } + } + ] +} diff --git a/oci/layout/oci_dest.go b/oci/layout/oci_dest.go index c8156cc3a9..78046ab99a 100644 --- a/oci/layout/oci_dest.go +++ b/oci/layout/oci_dest.go @@ -3,6 +3,7 @@ package layout import ( "context" "encoding/json" + "fmt" "io" "io/ioutil" "os" @@ -27,6 +28,9 @@ type ociImageDestination struct { // newImageDestination returns an ImageDestination for writing to an existing directory. func newImageDestination(sys *types.SystemContext, ref ociReference) (types.ImageDestination, error) { + if ref.sourceIndex != -1 { + return nil, fmt.Errorf("Destination reference must not contain a manifest index @%d", ref.sourceIndex) + } var index *imgspecv1.Index if indexExists(ref) { var err error diff --git a/oci/layout/oci_dest_test.go b/oci/layout/oci_dest_test.go index 20b318c558..17d74441c4 100644 --- a/oci/layout/oci_dest_test.go +++ b/oci/layout/oci_dest_test.go @@ -29,7 +29,7 @@ func TestPutBlobDigestFailure(t *testing.T) { const digestErrorString = "Simulated digest error" const blobDigest = "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f" - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) dirRef, ok := ref.(ociReference) require.True(t, ok) @@ -70,7 +70,7 @@ func TestPutBlobDigestFailure(t *testing.T) { // TestPutManifestAppendsToExistingManifest tests that new manifests are getting added to existing index. func TestPutManifestAppendsToExistingManifest(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) @@ -94,7 +94,7 @@ func TestPutManifestAppendsToExistingManifest(t *testing.T) { // TestPutManifestTwice tests that existing manifest gets updated and not appended. func TestPutManifestTwice(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) @@ -110,7 +110,7 @@ func TestPutManifestTwice(t *testing.T) { } func TestPutTwoDifferentTags(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) diff --git a/oci/layout/oci_transport.go b/oci/layout/oci_transport.go index a99b631584..7aed1cd19e 100644 --- a/oci/layout/oci_transport.go +++ b/oci/layout/oci_transport.go @@ -61,22 +61,31 @@ type ociReference struct { // (But in general, we make no attempt to be completely safe against concurrent hostile filesystem modifications.) dir string // As specified by the user. May be relative, contain symlinks, etc. resolvedDir string // Absolute path with no symlinks, at least at the time of its creation. Primarily used for policy namespaces. - // If image=="", it means the "only image" in the index.json is used in the case it is a source - // for destinations, the image name annotation "image.ref.name" is not added to the index.json + // If image=="" && sourceIndex==-1, it means the "only image" in the index.json is used in the case it is a source + // for destinations, the image name annotation "image.ref.name" is not added to the index.json. + // + // Must not be set if sourceIndex is set (the value is not -1). image string + // If not -1, a zero-based index of an image in the manifest index. Valid only for sources. + // Must not be set if image is set. + sourceIndex int } // ParseReference converts a string, which should not start with the ImageTransport.Name prefix, into an OCI ImageReference. func ParseReference(reference string) (types.ImageReference, error) { - dir, image := internal.SplitPathAndImage(reference) - return NewReference(dir, image) + dir, image, index, err := internal.ParseReferenceIntoElements(reference) + if err != nil { + return nil, err + } + return newReference(dir, image, index) } -// NewReference returns an OCI reference for a directory and a image. +// newReference returns an OCI reference for a directory, and an image name annotation or sourceIndex. // +// If sourceIndex==-1, the index will not be valid to point out the source image, only image will be used. // We do not expose an API supplying the resolvedDir; we could, but recomputing it // is generally cheap enough that we prefer being confident about the properties of resolvedDir. -func NewReference(dir, image string) (types.ImageReference, error) { +func newReference(dir, image string, sourceIndex int) (types.ImageReference, error) { resolved, err := explicitfilepath.ResolvePathToFullyExplicit(dir) if err != nil { return nil, err @@ -90,7 +99,26 @@ func NewReference(dir, image string) (types.ImageReference, error) { return nil, err } - return ociReference{dir: dir, resolvedDir: resolved, image: image}, nil + if sourceIndex != -1 && sourceIndex < 0 { + return nil, fmt.Errorf("Invalid oci: layout reference: index @%d must not be negative", sourceIndex) + } + if sourceIndex != -1 && image != "" { + return nil, fmt.Errorf("Invalid oci: layout reference: cannot use both an image %s and a source index @%d", image, sourceIndex) + } + return ociReference{dir: dir, resolvedDir: resolved, image: image, sourceIndex: sourceIndex}, nil +} + +// NewIndexReference returns an OCI reference for a path and a zero-based source manifest index. +func NewIndexReference(dir string, sourceIndex int) (types.ImageReference, error) { + return newReference(dir, "", sourceIndex) +} + +// NewReference returns an OCI reference for a directory and a image. +// +// We do not expose an API supplying the resolvedDir; we could, but recomputing it +// is generally cheap enough that we prefer being confident about the properties of resolvedDir. +func NewReference(dir, image string) (types.ImageReference, error) { + return newReference(dir, image, -1) } func (ref ociReference) Transport() types.ImageTransport { @@ -103,7 +131,11 @@ func (ref ociReference) Transport() types.ImageTransport { // e.g. default attribute values omitted by the user may be filled in in the return value, or vice versa. // WARNING: Do not use the return value in the UI to describe an image, it does not contain the Transport().Name() prefix. func (ref ociReference) StringWithinTransport() string { - return fmt.Sprintf("%s:%s", ref.dir, ref.image) + if ref.sourceIndex == -1 { + return fmt.Sprintf("%s:%s", ref.dir, ref.image) + } + return fmt.Sprintf("%s:@%d", ref.dir, ref.sourceIndex) + } // DockerReference returns a Docker reference associated with this reference @@ -182,8 +214,16 @@ func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, error) { if err != nil { return imgspecv1.Descriptor{}, err } - var d *imgspecv1.Descriptor + + if ref.sourceIndex != -1 { + if ref.sourceIndex < len(index.Manifests) { + d = &index.Manifests[ref.sourceIndex] + return *d, nil + } else { + return imgspecv1.Descriptor{}, fmt.Errorf("index %d is too large, only %d entries available", ref.sourceIndex, len(index.Manifests)) + } + } if ref.image == "" { // return manifest if only one image is in the oci directory if len(index.Manifests) == 1 { @@ -198,8 +238,8 @@ func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, error) { if md.MediaType != imgspecv1.MediaTypeImageManifest && md.MediaType != imgspecv1.MediaTypeImageIndex { continue } - refName, ok := md.Annotations[imgspecv1.AnnotationRefName] - if !ok { + refName := internal.NameFromAnnotations(md.Annotations) + if refName == "" { continue } if refName == ref.image { @@ -219,7 +259,7 @@ func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, error) { func LoadManifestDescriptor(imgRef types.ImageReference) (imgspecv1.Descriptor, error) { ociRef, ok := imgRef.(ociReference) if !ok { - return imgspecv1.Descriptor{}, errors.Errorf("error typecasting, need type ociRef") + return imgspecv1.Descriptor{}, fmt.Errorf("error typecasting, need type ociRef") } return ociRef.getManifestDescriptor() } @@ -238,7 +278,7 @@ func (ref ociReference) NewImageDestination(ctx context.Context, sys *types.Syst // DeleteImage deletes the named image from the registry, if supported. func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContext) error { - return errors.Errorf("Deleting images not implemented for oci: images") + return fmt.Errorf("Deleting images not implemented for oci: images") } // ociLayoutPath returns a path for the oci-layout within a directory using OCI conventions. @@ -254,7 +294,7 @@ func (ref ociReference) indexPath() string { // blobPath returns a path for a blob within a directory using OCI image-layout conventions. func (ref ociReference) blobPath(digest digest.Digest, sharedBlobDir string) (string, error) { if err := digest.Validate(); err != nil { - return "", errors.Wrapf(err, "unexpected digest reference %s", digest) + return "", fmt.Errorf("unexpected digest reference %s: %w", digest, err) } blobDir := filepath.Join(ref.dir, "blobs") if sharedBlobDir != "" { diff --git a/oci/layout/oci_transport_test.go b/oci/layout/oci_transport_test.go index a795e9dd3a..e4be506242 100644 --- a/oci/layout/oci_transport_test.go +++ b/oci/layout/oci_transport_test.go @@ -13,16 +13,28 @@ import ( "github.com/stretchr/testify/require" ) -// TestGetManifestDescriptor is testing a regression issue where a nil error was being wrapped, -// this causes the returned error to be nil as well and the user wasn't getting a proper error output. -// -// More info: https://github.com/containers/skopeo/issues/496 func TestGetManifestDescriptor(t *testing.T) { imageRef, err := NewReference("fixtures/two_images_manifest", "") require.NoError(t, err) + // test a regression issue where a nil error was being wrapped, + // this causes the returned error to be nil as well and the user wasn't getting a proper error output. + // + // More info: https://github.com/containers/skopeo/issues/496 _, err = imageRef.(ociReference).getManifestDescriptor() assert.EqualError(t, err, ErrMoreThanOneImage.Error()) + + imageRef, err = NewReference("fixtures/two_names_manifest", "imageValue0") + require.NoError(t, err) + manDescriptor, err := imageRef.(ociReference).getManifestDescriptor() + require.NoError(t, err) + assert.Equal(t, manDescriptor.Annotations["org.opencontainers.image.ref.name"], "imageValue0") + + imageRef, err = NewIndexReference("fixtures/two_names_manifest", 1) + require.NoError(t, err) + manDescriptor, err = imageRef.(ociReference).getManifestDescriptor() + require.NoError(t, err) + assert.Equal(t, manDescriptor.Annotations["org.opencontainers.image.ref.name"], "imageValue1") } func TestTransportName(t *testing.T) { @@ -72,11 +84,17 @@ func testParseReference(t *testing.T, fn func(string) (types.ImageReference, err "relativepath", tmpDir + "/thisdoesnotexist", } { - for _, image := range []struct{ suffix, image string }{ - {":notlatest:image", "notlatest:image"}, - {":latestimage", "latestimage"}, - {":", ""}, - {"", ""}, + for _, image := range []struct { + suffix, image string + sourceIndex int + }{ + {":notlatest:image", "notlatest:image", -1}, + {":latestimage", "latestimage", -1}, + {":", "", -1}, + {"", "", -1}, + {":@0", "", 0}, + {":@10", "", 10}, + {":@999999", "", 999999}, } { input := path + image.suffix ref, err := fn(input) @@ -85,11 +103,16 @@ func testParseReference(t *testing.T, fn func(string) (types.ImageReference, err require.True(t, ok) assert.Equal(t, path, ociRef.dir, input) assert.Equal(t, image.image, ociRef.image, input) + assert.Equal(t, image.sourceIndex, ociRef.sourceIndex, input) } } _, err = fn(tmpDir + ":invalid'image!value@") assert.Error(t, err) + + _, err = fn(tmpDir + ":@-3") + assert.Error(t, err) + } func TestNewReference(t *testing.T) { @@ -108,6 +131,7 @@ func TestNewReference(t *testing.T) { require.True(t, ok) assert.Equal(t, tmpDir, ociRef.dir) assert.Equal(t, imageValue, ociRef.image) + assert.Equal(t, -1, ociRef.sourceIndex) ref, err = NewReference(tmpDir, noImageValue) require.NoError(t, err) @@ -115,6 +139,7 @@ func TestNewReference(t *testing.T) { require.True(t, ok) assert.Equal(t, tmpDir, ociRef.dir) assert.Equal(t, noImageValue, ociRef.image) + assert.Equal(t, -1, ociRef.sourceIndex) _, err = NewReference(tmpDir+"/thisparentdoesnotexist/something", imageValue) assert.Error(t, err) @@ -124,14 +149,57 @@ func TestNewReference(t *testing.T) { _, err = NewReference(tmpDir+"/has:colon", imageValue) assert.Error(t, err) + + // Test private newReference + _, err = newReference(tmpDir, imageValue, 1) + assert.Error(t, err) +} + +func TestNewIndexReference(t *testing.T) { + const imageValue = "imageValue" + + tmpDir, err := ioutil.TempDir("", "oci-transport-test") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + ref, err := NewIndexReference(tmpDir, 10) + require.NoError(t, err) + ociRef, ok := ref.(ociReference) + require.True(t, ok) + assert.Equal(t, tmpDir, ociRef.dir) + assert.Equal(t, "", ociRef.image) + assert.Equal(t, 10, ociRef.sourceIndex) + + ref, err = NewIndexReference(tmpDir, 9999) + require.NoError(t, err) + ociRef, ok = ref.(ociReference) + require.True(t, ok) + assert.Equal(t, tmpDir, ociRef.dir) + assert.Equal(t, "", ociRef.image) + assert.Equal(t, 9999, ociRef.sourceIndex) + + _, err = NewIndexReference(tmpDir+"/thisparentdoesnotexist/something", 10) + assert.Error(t, err) + + // sourceIndex cannot be less than -1 + _, err = NewIndexReference(tmpDir, -3) + assert.Error(t, err) + + _, err = NewIndexReference(tmpDir+"/has:colon", 99) + assert.Error(t, err) + + // Test private newReference + _, err = newReference(tmpDir, imageValue, 1) + assert.Error(t, err) } // refToTempOCI creates a temporary directory and returns an reference to it. // The caller should // defer os.RemoveAll(tmpDir) -func refToTempOCI(t *testing.T) (ref types.ImageReference, tmpDir string) { +func refToTempOCI(t *testing.T, sourceIndex bool) (ref types.ImageReference, tmpDir string) { tmpDir, err := ioutil.TempDir("", "oci-transport-test") require.NoError(t, err) + m := `{ "schemaVersion": 2, "manifests": [ @@ -150,15 +218,39 @@ func refToTempOCI(t *testing.T) (ref types.ImageReference, tmpDir string) { ] } ` + if sourceIndex { + m = `{ + "schemaVersion": 2, + "manifests": [ + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "size": 7143, + "digest": "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", + "platform": { + "architecture": "ppc64le", + "os": "linux" + }, + } + ] + } + ` + } + err = ioutil.WriteFile(filepath.Join(tmpDir, "index.json"), []byte(m), 0644) require.NoError(t, err) - ref, err = NewReference(tmpDir, "imageValue") - require.NoError(t, err) + + if sourceIndex { + ref, err = NewIndexReference(tmpDir, 1) + require.NoError(t, err) + } else { + ref, err = NewReference(tmpDir, "imageValue") + require.NoError(t, err) + } return ref, tmpDir } func TestReferenceTransport(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) assert.Equal(t, Transport, ref.Transport()) } @@ -170,7 +262,8 @@ func TestReferenceStringWithinTransport(t *testing.T) { for _, c := range []struct{ input, result string }{ {"/dir1:notlatest:notlatest", "/dir1:notlatest:notlatest"}, // Explicit image - {"/dir3:", "/dir3:"}, // No image + {"/dir3:", "/dir3:"}, // No image + {"/dir1:@1", "/dir1:@1"}, // Explicit sourceIndex of image } { ref, err := ParseReference(tmpDir + c.input) require.NoError(t, err, c.input) @@ -185,13 +278,13 @@ func TestReferenceStringWithinTransport(t *testing.T) { } func TestReferenceDockerReference(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) assert.Nil(t, ref.DockerReference()) } func TestReferencePolicyConfigurationIdentity(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) assert.Equal(t, tmpDir, ref.PolicyConfigurationIdentity()) @@ -205,10 +298,26 @@ func TestReferencePolicyConfigurationIdentity(t *testing.T) { ref, err = NewReference("/", "image3") require.NoError(t, err) assert.Equal(t, "/", ref.PolicyConfigurationIdentity()) + + // Test the sourceIndex case + ref, tmpDir = refToTempOCI(t, true) + defer os.RemoveAll(tmpDir) + + assert.Equal(t, tmpDir, ref.PolicyConfigurationIdentity()) + // A non-canonical path. Test just one, the various other cases are + // tested in explicitfilepath.ResolvePathToFullyExplicit. + ref, err = NewIndexReference(tmpDir+"/.", 1) + require.NoError(t, err) + assert.Equal(t, tmpDir, ref.PolicyConfigurationIdentity()) + + // "/" as a corner case. + ref, err = NewIndexReference("/", 2) + require.NoError(t, err) + assert.Equal(t, "/", ref.PolicyConfigurationIdentity()) } func TestReferencePolicyConfigurationNamespaces(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) // We don't really know enough to make a full equality test here. ns := ref.PolicyConfigurationNamespaces() @@ -237,24 +346,55 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) { ref, err := NewReference("/", "image3") require.NoError(t, err) assert.Equal(t, []string{}, ref.PolicyConfigurationNamespaces()) + + // Test the sourceIndex case + ref, tmpDir = refToTempOCI(t, true) + defer os.RemoveAll(tmpDir) + // We don't really know enough to make a full equality test here. + ns = ref.PolicyConfigurationNamespaces() + require.NotNil(t, ns) + assert.True(t, len(ns) >= 2) + assert.Equal(t, tmpDir, ns[0]) + assert.Equal(t, filepath.Dir(tmpDir), ns[1]) + + // Test with a known path which should exist. Test just one non-canonical + // path, the various other cases are tested in explicitfilepath.ResolvePathToFullyExplicit. + // + // It would be nice to test a deeper hierarchy, but it is not obvious what + // deeper path is always available in the various distros, AND is not likely + // to contains a symbolic link. + for _, path := range []string{"/usr/share", "/usr/share/./."} { + _, err := os.Lstat(path) + require.NoError(t, err) + ref, err := NewIndexReference(path, 1) + require.NoError(t, err) + ns := ref.PolicyConfigurationNamespaces() + require.NotNil(t, ns) + assert.Equal(t, []string{"/usr/share", "/usr"}, ns) + } + + // "/" as a corner case. + ref, err = NewIndexReference("/", 2) + require.NoError(t, err) + assert.Equal(t, []string{}, ref.PolicyConfigurationNamespaces()) } func TestReferenceNewImage(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) _, err := ref.NewImage(context.Background(), nil) assert.Error(t, err) } func TestReferenceNewImageSource(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) _, err := ref.NewImageSource(context.Background(), nil) assert.NoError(t, err) } func TestReferenceNewImageDestination(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) dest, err := ref.NewImageDestination(context.Background(), nil) assert.NoError(t, err) @@ -262,14 +402,14 @@ func TestReferenceNewImageDestination(t *testing.T) { } func TestReferenceDeleteImage(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) err := ref.DeleteImage(context.Background(), nil) assert.Error(t, err) } func TestReferenceOCILayoutPath(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) require.True(t, ok) @@ -277,7 +417,7 @@ func TestReferenceOCILayoutPath(t *testing.T) { } func TestReferenceIndexPath(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) require.True(t, ok) @@ -287,7 +427,7 @@ func TestReferenceIndexPath(t *testing.T) { func TestReferenceBlobPath(t *testing.T) { const hex = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) require.True(t, ok) @@ -299,7 +439,7 @@ func TestReferenceBlobPath(t *testing.T) { func TestReferenceSharedBlobPathShared(t *testing.T) { const hex = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) require.True(t, ok) @@ -311,7 +451,7 @@ func TestReferenceSharedBlobPathShared(t *testing.T) { func TestReferenceBlobPathInvalid(t *testing.T) { const hex = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) defer os.RemoveAll(tmpDir) ociRef, ok := ref.(ociReference) require.True(t, ok)