From b495713b69e10b5218296c64d8df62b8f31c7e6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 10 Sep 2024 20:22:47 +0200 Subject: [PATCH 1/3] Add @sourceIndex syntax to oci/layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Images in the index can now be referenced via the @sourceIndex syntax. Signed-off-by: Miloslav Trmač Signed-off-by: Valentin Rothberg --- docs/containers-transports.5.md | 4 +- oci/internal/oci_util.go | 29 +++++++ oci/internal/oci_util_test.go | 28 ++++++ oci/layout/oci_dest.go | 3 + oci/layout/oci_dest_test.go | 8 +- oci/layout/oci_transport.go | 75 ++++++++++++---- oci/layout/oci_transport_test.go | 145 ++++++++++++++++++++++++++----- 7 files changed, 250 insertions(+), 42 deletions(-) diff --git a/docs/containers-transports.5.md b/docs/containers-transports.5.md index 2920781e60..da88e0933a 100644 --- a/docs/containers-transports.5.md +++ b/docs/containers-transports.5.md @@ -71,13 +71,15 @@ 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 in a directory structure compliant with the "Open Container Image Layout Specification" at _path_. The _path_ value terminates at the first `:` character; any further `:` characters are not separators, but a part of _reference_. The _reference_ is used to set, or match, the `org.opencontainers.image.ref.name` annotation in the top-level index. If _reference_ is not specified when reading an image, the directory must contain exactly one image. +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_] diff --git a/oci/internal/oci_util.go b/oci/internal/oci_util.go index 72af74e251..c4eaed0eee 100644 --- a/oci/internal/oci_util.go +++ b/oci/internal/oci_util.go @@ -6,6 +6,7 @@ import ( "path/filepath" "regexp" "runtime" + "strconv" "strings" ) @@ -119,3 +120,31 @@ 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 +} diff --git a/oci/internal/oci_util_test.go b/oci/internal/oci_util_test.go index 2438acf1d3..d0d273359a 100644 --- a/oci/internal/oci_util_test.go +++ b/oci/internal/oci_util_test.go @@ -18,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"}, @@ -61,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/oci_dest.go b/oci/layout/oci_dest.go index cb2768d745..19c0cff570 100644 --- a/oci/layout/oci_dest.go +++ b/oci/layout/oci_dest.go @@ -38,6 +38,9 @@ type ociImageDestination struct { // newImageDestination returns an ImageDestination for writing to an existing directory. func newImageDestination(sys *types.SystemContext, ref ociReference) (private.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 33cf8516bc..57981c37a3 100644 --- a/oci/layout/oci_dest_test.go +++ b/oci/layout/oci_dest_test.go @@ -32,7 +32,7 @@ func TestPutBlobDigestFailure(t *testing.T) { const digestErrorString = "Simulated digest error" const blobDigest = "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f" - ref, _ := refToTempOCI(t) + ref, _ := refToTempOCI(t, false) dirRef, ok := ref.(ociReference) require.True(t, ok) blobPath, err := dirRef.blobPath(blobDigest, "") @@ -71,7 +71,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) ociRef, ok := ref.(ociReference) require.True(t, ok) @@ -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) ociRef, ok := ref.(ociReference) require.True(t, ok) @@ -109,7 +109,7 @@ func TestPutManifestTwice(t *testing.T) { } func TestPutTwoDifferentTags(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) ociRef, ok := ref.(ociReference) require.True(t, ok) diff --git a/oci/layout/oci_transport.go b/oci/layout/oci_transport.go index 816dfa7a1e..9cdbe292fd 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,10 @@ func (ref ociReference) Transport() types.ImageTransport { // e.g. default attribute values omitted by the user may be filled 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 @@ -187,14 +218,18 @@ func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, int, erro return imgspecv1.Descriptor{}, -1, err } - if ref.image == "" { - // return manifest if only one image is in the oci directory - if len(index.Manifests) != 1 { - // ask user to choose image when more than one image in the oci directory - return imgspecv1.Descriptor{}, -1, ErrMoreThanOneImage + switch { + case ref.image != "" && ref.sourceIndex != -1: + return imgspecv1.Descriptor{}, -1, fmt.Errorf("Internal error: Cannot have both ref %s and source index @%d", + ref.image, ref.sourceIndex) + + case ref.sourceIndex != -1: + if ref.sourceIndex >= len(index.Manifests) { + return imgspecv1.Descriptor{}, -1, fmt.Errorf("index %d is too large, only %d entries available", ref.sourceIndex, len(index.Manifests)) } - return index.Manifests[0], 0, nil - } else { + return index.Manifests[ref.sourceIndex], ref.sourceIndex, nil + + case ref.image != "": // if image specified, look through all manifests for a match var unsupportedMIMETypes []string for i, md := range index.Manifests { @@ -208,8 +243,16 @@ func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, int, erro if len(unsupportedMIMETypes) != 0 { return imgspecv1.Descriptor{}, -1, fmt.Errorf("reference %q matches unsupported manifest MIME types %q", ref.image, unsupportedMIMETypes) } + return imgspecv1.Descriptor{}, -1, ImageNotFoundError{ref} + + default: + // return manifest if only one image is in the oci directory + if len(index.Manifests) != 1 { + // ask user to choose image when more than one image in the oci directory + return imgspecv1.Descriptor{}, -1, ErrMoreThanOneImage + } + return index.Manifests[0], 0, nil } - return imgspecv1.Descriptor{}, -1, ImageNotFoundError{ref} } // LoadManifestDescriptor loads the manifest descriptor to be used to retrieve the image name diff --git a/oci/layout/oci_transport_test.go b/oci/layout/oci_transport_test.go index 8beb52dd19..da1ec3f7dd 100644 --- a/oci/layout/oci_transport_test.go +++ b/oci/layout/oci_transport_test.go @@ -148,11 +148,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) @@ -161,11 +167,15 @@ 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) { @@ -182,6 +192,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) @@ -189,6 +200,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) @@ -198,10 +210,50 @@ 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 := t.TempDir() + + 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. -func refToTempOCI(t *testing.T) (types.ImageReference, string) { +func refToTempOCI(t *testing.T, sourceIndex bool) (types.ImageReference, string) { tmpDir := t.TempDir() m := `{ "schemaVersion": 2, @@ -221,15 +273,39 @@ func refToTempOCI(t *testing.T) (types.ImageReference, 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 := os.WriteFile(filepath.Join(tmpDir, "index.json"), []byte(m), 0644) require.NoError(t, err) - ref, err := NewReference(tmpDir, "imageValue") - require.NoError(t, err) + var ref types.ImageReference + 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, _ := refToTempOCI(t) + ref, _ := refToTempOCI(t, false) assert.Equal(t, Transport, ref.Transport()) } @@ -238,7 +314,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 + {"/dir4:@1", "/dir4:@1"}, // Explicit sourceIndex of image } { ref, err := ParseReference(tmpDir + c.input) require.NoError(t, err, c.input) @@ -253,12 +330,12 @@ func TestReferenceStringWithinTransport(t *testing.T) { } func TestReferenceDockerReference(t *testing.T) { - ref, _ := refToTempOCI(t) + ref, _ := refToTempOCI(t, false) assert.Nil(t, ref.DockerReference()) } func TestReferencePolicyConfigurationIdentity(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) assert.Equal(t, tmpDir, ref.PolicyConfigurationIdentity()) // A non-canonical path. Test just one, the various other cases are @@ -267,14 +344,27 @@ func TestReferencePolicyConfigurationIdentity(t *testing.T) { require.NoError(t, err) assert.Equal(t, tmpDir, ref.PolicyConfigurationIdentity()) + // Test the sourceIndex case + ref, tmpDir = refToTempOCI(t, true) + 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 = NewReference("/", "image3") require.NoError(t, err) assert.Equal(t, "/", ref.PolicyConfigurationIdentity()) + + 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) // We don't really know enough to make a full equality test here. ns := ref.PolicyConfigurationNamespaces() require.NotNil(t, ns) @@ -282,6 +372,15 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) { assert.Equal(t, tmpDir, ns[0]) assert.Equal(t, filepath.Dir(tmpDir), ns[1]) + // Test the sourceIndex case + ref, tmpDir = refToTempOCI(t, true) + // 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. // @@ -302,37 +401,41 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) { ref, err := NewReference("/", "image3") require.NoError(t, err) assert.Equal(t, []string{}, ref.PolicyConfigurationNamespaces()) + + ref, err = NewIndexReference("/", 2) + require.NoError(t, err) + assert.Equal(t, []string{}, ref.PolicyConfigurationNamespaces()) } func TestReferenceNewImage(t *testing.T) { - ref, _ := refToTempOCI(t) + ref, _ := refToTempOCI(t, false) _, err := ref.NewImage(context.Background(), nil) assert.Error(t, err) } func TestReferenceNewImageSource(t *testing.T) { - ref, _ := refToTempOCI(t) + ref, _ := refToTempOCI(t, false) src, err := ref.NewImageSource(context.Background(), nil) assert.NoError(t, err) defer src.Close() } func TestReferenceNewImageDestination(t *testing.T) { - ref, _ := refToTempOCI(t) + ref, _ := refToTempOCI(t, false) dest, err := ref.NewImageDestination(context.Background(), nil) assert.NoError(t, err) defer dest.Close() } func TestReferenceOCILayoutPath(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) ociRef, ok := ref.(ociReference) require.True(t, ok) assert.Equal(t, tmpDir+"/oci-layout", ociRef.ociLayoutPath()) } func TestReferenceIndexPath(t *testing.T) { - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) ociRef, ok := ref.(ociReference) require.True(t, ok) assert.Equal(t, tmpDir+"/index.json", ociRef.indexPath()) @@ -341,7 +444,7 @@ func TestReferenceIndexPath(t *testing.T) { func TestReferenceBlobPath(t *testing.T) { const hex = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" - ref, tmpDir := refToTempOCI(t) + ref, tmpDir := refToTempOCI(t, false) ociRef, ok := ref.(ociReference) require.True(t, ok) bp, err := ociRef.blobPath("sha256:"+hex, "") @@ -352,7 +455,7 @@ func TestReferenceBlobPath(t *testing.T) { func TestReferenceSharedBlobPathShared(t *testing.T) { const hex = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" - ref, _ := refToTempOCI(t) + ref, _ := refToTempOCI(t, false) ociRef, ok := ref.(ociReference) require.True(t, ok) bp, err := ociRef.blobPath("sha256:"+hex, "/external/path") @@ -363,7 +466,7 @@ func TestReferenceSharedBlobPathShared(t *testing.T) { func TestReferenceBlobPathInvalid(t *testing.T) { const hex = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" - ref, _ := refToTempOCI(t) + ref, _ := refToTempOCI(t, false) ociRef, ok := ref.(ociReference) require.True(t, ok) _, err := ociRef.blobPath(hex, "") From 786c896587ccd788c23778a5aadb17bbdd6d6645 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 10 Sep 2024 20:31:53 +0200 Subject: [PATCH 2/3] Add oci/layout.List MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new API allows for listing all manifests in an OCI layout's index. Signed-off-by: Miloslav Trmač Signed-off-by: Valentin Rothberg --- oci/layout/reader.go | 52 ++++++++++++++++++++++++++++++ oci/layout/reader_test.go | 67 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 oci/layout/reader.go create mode 100644 oci/layout/reader_test.go diff --git a/oci/layout/reader.go b/oci/layout/reader.go new file mode 100644 index 0000000000..112db2d705 --- /dev/null +++ b/oci/layout/reader.go @@ -0,0 +1,52 @@ +package layout + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + + "github.com/containers/image/v5/types" + imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" +) + +// This file is named reader.go for consistency with other transports’ +// handling of “image containers”, but we don’t actually need a stateful reader object. + +// ListResult wraps the image reference and the manifest for loading +type ListResult struct { + Reference types.ImageReference + ManifestDescriptor imgspecv1.Descriptor +} + +// List returns a slice of manifests included in the archive +func List(dir string) ([]ListResult, error) { + var res []ListResult + + indexJSON, err := os.ReadFile(filepath.Join(dir, imgspecv1.ImageIndexFile)) + if err != nil { + return nil, err + } + var index imgspecv1.Index + if err := json.Unmarshal(indexJSON, &index); err != nil { + return nil, err + } + + for manifestIndex, md := range index.Manifests { + refName := md.Annotations[imgspecv1.AnnotationRefName] + index := -1 + if refName == "" { + index = manifestIndex + } + ref, err := newReference(dir, refName, index) + if err != nil { + return nil, fmt.Errorf("error creating image reference: %w", err) + } + reference := ListResult{ + Reference: ref, + ManifestDescriptor: md, + } + res = append(res, reference) + } + return res, nil +} diff --git a/oci/layout/reader_test.go b/oci/layout/reader_test.go new file mode 100644 index 0000000000..fdf0cc4c21 --- /dev/null +++ b/oci/layout/reader_test.go @@ -0,0 +1,67 @@ +package layout + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestList(t *testing.T) { + + for _, test := range []struct { + path string + num int + digests []string + names map[int]string + }{ + { + path: "fixtures/two_images_manifest", + num: 2, + digests: []string{ + "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", + "sha256:5b0bcabd1ed22e9fb1310cf6c2dec7cdef19f0ad69efa1f392e94a4333501270", + }, + names: map[int]string{0: "", 1: ""}, + }, + { + path: "fixtures/manifest", + num: 1, + digests: []string{ + "sha256:84afb6189c4d69f2d040c5f1dc4e0a16fed9b539ce9cfb4ac2526ae4e0576cc0", + }, + names: map[int]string{0: "v0.1.1"}, + }, + { + path: "fixtures/name_lookups", + num: 4, + digests: []string{ + "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", + "sha256:dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd", + }, + names: map[int]string{0: "a", 1: "b", 2: "invalid-mime", 3: "invalid-mime"}, + }, + } { + results, err := List(test.path) + require.NoError(t, err) + require.NotNil(t, results) + require.Len(t, results, test.num) + for i, res := range results { + ociRef, ok := res.Reference.(ociReference) + require.True(t, ok) + require.Equal(t, test.digests[i], res.ManifestDescriptor.Digest.String()) + require.Equal(t, test.names[i], ociRef.image) + if test.names[i] != "" { + require.True(t, strings.HasSuffix(res.Reference.StringWithinTransport(), ":"+test.names[i])) + } + _, err := ParseReference(fmt.Sprintf("%s:@%d", test.path, i)) + require.NoError(t, err) + } + } + + _, err := List("fixtures/i_do_not_exist") + require.Error(t, err) +} From 8565739d0ba8bb3dcc162e681f9478f5e7671c01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 24 Sep 2024 21:10:50 +0200 Subject: [PATCH 3/3] Add oci/layout.PutBlobFromLocalFile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Try to reflink the file and restort to copying it in case of failure. Also add an Options struct to be future proof. Signed-off-by: Miloslav Trmač Signed-off-by: Valentin Rothberg --- internal/reflink/reflink_linux.go | 30 +++++++++ internal/reflink/reflink_unsupported.go | 15 +++++ oci/layout/fixtures/files/a.txt | 1 + oci/layout/fixtures/files/b.txt | 1 + oci/layout/oci_dest.go | 86 ++++++++++++++++++++++--- oci/layout/oci_dest_test.go | 38 +++++++++++ 6 files changed, 162 insertions(+), 9 deletions(-) create mode 100644 internal/reflink/reflink_linux.go create mode 100644 internal/reflink/reflink_unsupported.go create mode 100644 oci/layout/fixtures/files/a.txt create mode 100644 oci/layout/fixtures/files/b.txt diff --git a/internal/reflink/reflink_linux.go b/internal/reflink/reflink_linux.go new file mode 100644 index 0000000000..bb29cfa929 --- /dev/null +++ b/internal/reflink/reflink_linux.go @@ -0,0 +1,30 @@ +//go:build cgo + +package reflink + +/* +#include + +#ifndef FICLONE +#define FICLONE _IOW(0x94, 9, int) +#endif +*/ +import "C" +import ( + "io" + "os" + + "golang.org/x/sys/unix" +) + +// Copy attempts to reflink the source to the destination fd. +// If reflinking fails or is unsupported, it falls back to io.Copy(). +func Copy(src, dst *os.File) error { + _, _, errno := unix.Syscall(unix.SYS_IOCTL, dst.Fd(), C.FICLONE, src.Fd()) + if errno == 0 { + return nil + } + + _, err := io.Copy(dst, src) + return err +} diff --git a/internal/reflink/reflink_unsupported.go b/internal/reflink/reflink_unsupported.go new file mode 100644 index 0000000000..3eb6bb59ab --- /dev/null +++ b/internal/reflink/reflink_unsupported.go @@ -0,0 +1,15 @@ +//go:build !linux || !cgo + +package reflink + +import ( + "io" + "os" +) + +// Copy attempts to reflink the source to the destination fd. +// If reflinking fails or is unsupported, it falls back to io.Copy(). +func Copy(src, dst *os.File) error { + _, err := io.Copy(dst, src) + return err +} diff --git a/oci/layout/fixtures/files/a.txt b/oci/layout/fixtures/files/a.txt new file mode 100644 index 0000000000..5582bcf401 --- /dev/null +++ b/oci/layout/fixtures/files/a.txt @@ -0,0 +1 @@ +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa diff --git a/oci/layout/fixtures/files/b.txt b/oci/layout/fixtures/files/b.txt new file mode 100644 index 0000000000..59f630b80c --- /dev/null +++ b/oci/layout/fixtures/files/b.txt @@ -0,0 +1 @@ +bbbbbbbbbbbbbbbbbbbbbbbb diff --git a/oci/layout/oci_dest.go b/oci/layout/oci_dest.go index 19c0cff570..694109f195 100644 --- a/oci/layout/oci_dest.go +++ b/oci/layout/oci_dest.go @@ -17,6 +17,7 @@ import ( "github.com/containers/image/v5/internal/manifest" "github.com/containers/image/v5/internal/private" "github.com/containers/image/v5/internal/putblobdigest" + "github.com/containers/image/v5/internal/reflink" "github.com/containers/image/v5/types" "github.com/containers/storage/pkg/fileutils" digest "github.com/opencontainers/go-digest" @@ -141,9 +142,18 @@ func (d *ociImageDestination) PutBlobWithOptions(ctx context.Context, stream io. if inputInfo.Size != -1 && size != inputInfo.Size { return private.UploadedBlob{}, fmt.Errorf("Size mismatch when copying %s, expected %d, got %d", blobDigest, inputInfo.Size, size) } - if err := blobFile.Sync(); err != nil { + + if err := d.blobFileSyncAndRename(blobFile, blobDigest, &explicitClosed); err != nil { return private.UploadedBlob{}, err } + succeeded = true + return private.UploadedBlob{Digest: blobDigest, Size: size}, nil +} + +func (d *ociImageDestination) blobFileSyncAndRename(blobFile *os.File, blobDigest digest.Digest, closed *bool) error { + if err := blobFile.Sync(); err != nil { + return err + } // On POSIX systems, blobFile was created with mode 0600, so we need to make it readable. // On Windows, the “permissions of newly created files” argument to syscall.Open is @@ -151,26 +161,27 @@ func (d *ociImageDestination) PutBlobWithOptions(ctx context.Context, stream io. // always fails on Windows. if runtime.GOOS != "windows" { if err := blobFile.Chmod(0644); err != nil { - return private.UploadedBlob{}, err + return err } } blobPath, err := d.ref.blobPath(blobDigest, d.sharedBlobDir) if err != nil { - return private.UploadedBlob{}, err + return err } if err := ensureParentDirectoryExists(blobPath); err != nil { - return private.UploadedBlob{}, err + return err } - // need to explicitly close the file, since a rename won't otherwise not work on Windows + // need to explicitly close the file, since a rename won't otherwise work on Windows blobFile.Close() - explicitClosed = true + + *closed = true if err := os.Rename(blobFile.Name(), blobPath); err != nil { - return private.UploadedBlob{}, err + return err } - succeeded = true - return private.UploadedBlob{Digest: blobDigest, Size: size}, nil + + return nil } // TryReusingBlobWithOptions checks whether the transport already contains, or can efficiently reuse, a blob, and if so, applies it to the current destination @@ -303,6 +314,63 @@ func (d *ociImageDestination) CommitWithOptions(ctx context.Context, options pri return os.WriteFile(d.ref.indexPath(), indexJSON, 0644) } +// PutBlobFromLocalFileOptions is unused but may receive functionality in the future. +type PutBlobFromLocalFileOptions struct{} + +// PutBlobFromLocalFile arranges the data from path to be used as blob with digest. +// It computes, and returns, the digest and size of the used file. +// +// This function can be used instead of dest.PutBlob() where the ImageDestination requires PutBlob() to be called. +func PutBlobFromLocalFile(ctx context.Context, dest types.ImageDestination, file string, options ...PutBlobFromLocalFileOptions) (digest.Digest, int64, error) { + d, ok := dest.(*ociImageDestination) + if !ok { + return "", -1, errors.New("internal error: PutBlobFromLocalFile called with a non-oci: destination") + } + + blobFileClosed := false + blobFile, err := os.CreateTemp(d.ref.dir, "oci-put-blob") + if err != nil { + return "", -1, err + } + defer func() { + if !blobFileClosed { + blobFile.Close() + } + }() + + srcFile, err := os.Open(file) + if err != nil { + return "", -1, err + } + defer srcFile.Close() + + // reflink.Copy will io.Copy() in case reflinking fails + err = reflink.Copy(srcFile, blobFile) + if err != nil { + return "", -1, err + } + + _, err = blobFile.Seek(0, 0) + if err != nil { + return "", -1, err + } + blobDigest, err := digest.FromReader(blobFile) + if err != nil { + return "", -1, err + } + + fileInfo, err := blobFile.Stat() + if err != nil { + return "", -1, err + } + + if err := d.blobFileSyncAndRename(blobFile, blobDigest, &blobFileClosed); err != nil { + return "", -1, err + } + + return blobDigest, fileInfo.Size(), nil +} + func ensureDirectoryExists(path string) error { if err := fileutils.Exists(path); err != nil && errors.Is(err, fs.ErrNotExist) { if err := os.MkdirAll(path, 0755); err != nil { diff --git a/oci/layout/oci_dest_test.go b/oci/layout/oci_dest_test.go index 57981c37a3..9a8bd03ce4 100644 --- a/oci/layout/oci_dest_test.go +++ b/oci/layout/oci_dest_test.go @@ -178,3 +178,41 @@ func putTestManifest(t *testing.T, ociRef ociReference, tmpDir string) { digest := digest.FromBytes(data).Encoded() assert.Contains(t, paths, filepath.Join(tmpDir, "blobs", "sha256", digest), "The OCI directory does not contain the new manifest data") } + +func TestPutblobFromLocalFile(t *testing.T) { + ref, _ := refToTempOCI(t, false) + dest, err := ref.NewImageDestination(context.Background(), nil) + require.NoError(t, err) + defer dest.Close() + ociDest, ok := dest.(*ociImageDestination) + require.True(t, ok) + + for _, test := range []struct { + path string + size int64 + digest string + }{ + {path: "fixtures/files/a.txt", size: 31, digest: "sha256:c8a3f498ce6aaa13c803fa3a6a0d5fd6b5d75be5781f98f56c0f960efcc53174"}, + {path: "fixtures/files/b.txt", size: 25, digest: "sha256:8c1e9b03116b95e6dfac68c588190d56bfc82b9cc550ada726e882e138a3b93b"}, + {path: "fixtures/files/b.txt", size: 25, digest: "sha256:8c1e9b03116b95e6dfac68c588190d56bfc82b9cc550ada726e882e138a3b93b"}, // Must not fail + } { + digest, size, err := PutBlobFromLocalFile(context.Background(), dest, test.path) + require.NoError(t, err) + require.Equal(t, test.size, size) + require.Equal(t, test.digest, digest.String()) + + blobPath, err := ociDest.ref.blobPath(digest, ociDest.sharedBlobDir) + require.NoError(t, err) + require.FileExists(t, blobPath) + + expectedContent, err := os.ReadFile(test.path) + require.NoError(t, err) + require.NotEmpty(t, expectedContent) + blobContent, err := os.ReadFile(blobPath) + require.NoError(t, err) + require.Equal(t, expectedContent, blobContent) + } + + err = ociDest.CommitWithOptions(context.Background(), private.CommitOptions{}) + require.NoError(t, err) +}