From de3239b49d872a1e0d52b39b878abd33f0bed2a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 25 Jul 2023 01:03:23 +0200 Subject: [PATCH 1/2] Add UnparsedInstanceWithReference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is useful for combining image data with other reference values, e.g. to check signatures on a locally-pulled image based on a remote-registry policy. This was historically possible to do by simply providing an external UnparsedInstance implementation; now that we have sigstore signatures which can't be represent that way, and a private.UnparsedImage, external users are not allowed to directly access/implement private.UnparsedImage, so this needs to be an explicitly-provided operation. Signed-off-by: Miloslav Trmač --- image/unparsed.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/image/unparsed.go b/image/unparsed.go index 123f6ce6f..f2ebb929a 100644 --- a/image/unparsed.go +++ b/image/unparsed.go @@ -2,6 +2,8 @@ package image import ( "github.com/containers/image/v5/internal/image" + "github.com/containers/image/v5/internal/private" + "github.com/containers/image/v5/internal/unparsedimage" "github.com/containers/image/v5/types" "github.com/opencontainers/go-digest" ) @@ -17,3 +19,23 @@ type UnparsedImage = image.UnparsedImage func UnparsedInstance(src types.ImageSource, instanceDigest *digest.Digest) *UnparsedImage { return image.UnparsedInstance(src, instanceDigest) } + +// unparsedWithRef wraps a private.UnparsedImage, claiming another replacementRef +type unparsedWithRef struct { + private.UnparsedImage + ref types.ImageReference +} + +func (uwr *unparsedWithRef) Reference() types.ImageReference { + return uwr.ref +} + +// UnparsedInstanceWithReference returns a types.UnparsedImage for wrappedInstance which claims to be a replacementRef. +// This is useful for combining image data with other reference values, e.g. to check signatures on a locally-pulled image +// based on a remote-registry policy. +func UnparsedInstanceWithReference(wrappedInstance types.UnparsedImage, replacementRef types.ImageReference) types.UnparsedImage { + return &unparsedWithRef{ + UnparsedImage: unparsedimage.FromPublic(wrappedInstance), + ref: replacementRef, + } +} From 6cb88ae2e6800b67b1f70e6d5f353f7d45de83dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 13 Oct 2023 01:27:57 +0200 Subject: [PATCH 2/2] Add storage.ResolveReference; deprecate GetImage and GetStoreImage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See the added comments for details. This allows things like > ref := ParseReference(userInput) > ref2, img := ResolveReference(ref) > src := ref2.NewImageSource while ensuring that img and src are _guaranteed_ to refer to the same image. Signed-off-by: Miloslav Trmač --- storage/storage_reference.go | 27 ++++++++++++++ storage/storage_reference_test.go | 58 +++++++++++++++++++++++++++++-- storage/storage_transport.go | 29 ++++++++++++++++ 3 files changed, 111 insertions(+), 3 deletions(-) diff --git a/storage/storage_reference.go b/storage/storage_reference.go index 49f7d03c8..ba230d1fd 100644 --- a/storage/storage_reference.go +++ b/storage/storage_reference.go @@ -10,6 +10,7 @@ import ( "github.com/containers/image/v5/docker/reference" "github.com/containers/image/v5/manifest" + "github.com/containers/image/v5/transports" "github.com/containers/image/v5/types" "github.com/containers/storage" digest "github.com/opencontainers/go-digest" @@ -283,3 +284,29 @@ func (s storageReference) NewImageSource(ctx context.Context, sys *types.SystemC func (s storageReference) NewImageDestination(ctx context.Context, sys *types.SystemContext) (types.ImageDestination, error) { return newImageDestination(sys, s) } + +// ResolveReference finds the underlying storage image for a storage.Transport reference. +// It returns that image, and an updated reference which can be used to refer back to the _same_ +// image again. +// +// This matters if the input reference contains a tagged name; the destination of the tag can +// move in local storage. The updated reference returned by this function contains the resolved +// image ID, so later uses of that updated reference will either continue to refer to the same +// image, or fail. +// +// Note that it _is_ possible for the later uses to fail, either because the image was removed +// completely, or because the name used in the reference was untaged (even if the underlying image +// ID still exists in local storage). +func ResolveReference(ref types.ImageReference) (types.ImageReference, *storage.Image, error) { + sref, ok := ref.(*storageReference) + if !ok { + return nil, nil, fmt.Errorf("trying to resolve a non-%s: reference %q", Transport.Name(), + transports.ImageName(ref)) + } + clone := *sref // A shallow copy we can update + img, err := clone.resolveImage(nil) + if err != nil { + return nil, nil, err + } + return clone, img, nil +} diff --git a/storage/storage_reference_test.go b/storage/storage_reference_test.go index 6fa7d0e71..32590a06d 100644 --- a/storage/storage_reference_test.go +++ b/storage/storage_reference_test.go @@ -9,6 +9,9 @@ import ( "testing" "github.com/containers/image/v5/docker/reference" + "github.com/containers/image/v5/pkg/blobinfocache/memory" + "github.com/containers/storage" + "github.com/containers/storage/pkg/archive" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -97,14 +100,19 @@ func TestStorageReferenceDockerReference(t *testing.T) { } } -func TestStorageReferenceStringWithinTransport(t *testing.T) { - store := newStore(t) +// The […] part of references created for store +func storeSpecForStringWithinTransport(store storage.Store) string { optionsList := "" options := store.GraphOptions() if len(options) > 0 { optionsList = ":" + strings.Join(options, ",") } - storeSpec := fmt.Sprintf("[%s@%s+%s%s]", store.GraphDriverName(), store.GraphRoot(), store.RunRoot(), optionsList) + return fmt.Sprintf("[%s@%s+%s%s]", store.GraphDriverName(), store.GraphRoot(), store.RunRoot(), optionsList) +} + +func TestStorageReferenceStringWithinTransport(t *testing.T) { + store := newStore(t) + storeSpec := storeSpecForStringWithinTransport(store) for _, c := range validReferenceTestCases { ref, err := Transport.ParseReference(c.input) @@ -142,3 +150,47 @@ func TestStorageReferencePolicyConfigurationNamespaces(t *testing.T) { } // NewImage, NewImageSource, NewImageDestination, DeleteImage tested in storage_test.go + +func TestResolveReference(t *testing.T) { + // This is, so far, only a minimal smoke test + + ensureTestCanCreateImages(t) + + store := newStore(t) + storeSpec := storeSpecForStringWithinTransport(store) + cache := memory.New() + + id := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + // Create an image with a known name and ID + ref, err := Transport.ParseStoreReference(store, "test@"+id) + require.NoError(t, err) + createImage(t, ref, cache, []testBlob{makeLayer(t, archive.Gzip)}, nil) + + for _, c := range []struct { + input string + expected string // "" on error + }{ + { // No ID match + "@bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", "", + }, + {"@" + id, "@" + id}, // ID-only lookup + {"test", "docker.io/library/test:latest@" + id}, // Name is resolved to include ID + {"nottest", ""}, // No name match + {"test@" + id, "docker.io/library/test:latest@" + id}, // Name+ID works, and is unchanged + {"nottest@" + id, ""}, // Name mismatch is rejected even with an ID + } { + input, err := Transport.ParseStoreReference(store, c.input) + require.NoError(t, err, c.input) + inputClone := *input + resolved, img, err := ResolveReference(input) + if c.expected == "" { + assert.Error(t, err, c.input) + } else { + require.NoError(t, err, c.input) + require.Equal(t, &inputClone, input) // input was not modified in-place + assert.Equal(t, id, img.ID, c.input) + assert.Equal(t, storeSpec+c.expected, resolved.StringWithinTransport(), c.input) + } + } +} diff --git a/storage/storage_transport.go b/storage/storage_transport.go index 58ba3ee65..e9f42dc0a 100644 --- a/storage/storage_transport.go +++ b/storage/storage_transport.go @@ -48,9 +48,24 @@ type StoreTransport interface { GetStoreIfSet() storage.Store // GetImage retrieves the image from the transport's store that's named // by the reference. + // Deprecated: Surprisingly, with a StoreTransport reference which contains an ID, + // this ignores that ID; and repeated calls of GetStoreImage with the same named reference + // can return different images, with no way for the caller to "freeze" the storage.Image identity + // without discarding the name entirely. + // + // Use storage.ResolveReference instead. GetImage(types.ImageReference) (*storage.Image, error) // GetStoreImage retrieves the image from a specified store that's named // by the reference. + // + // Deprecated: Surprisingly, with a StoreTransport reference which contains an ID, + // this ignores that ID; and repeated calls of GetStoreImage with the same named reference + // can return different images, with no way for the caller to "freeze" the storage.Image identity + // without discarding the name entirely. + // + // Also, a StoreTransport reference already contains a store, so providing another one is redundant. + // + // Use storage.ResolveReference instead. GetStoreImage(storage.Store, types.ImageReference) (*storage.Image, error) // ParseStoreReference parses a reference, overriding any store // specification that it may contain. @@ -290,6 +305,14 @@ func (s *storageTransport) ParseReference(reference string) (types.ImageReferenc return s.ParseStoreReference(store, reference) } +// Deprecated: Surprisingly, with a StoreTransport reference which contains an ID, +// this ignores that ID; and repeated calls of GetStoreImage with the same named reference +// can return different images, with no way for the caller to "freeze" the storage.Image identity +// without discarding the name entirely. +// +// Also, a StoreTransport reference already contains a store, so providing another one is redundant. +// +// Use storage.ResolveReference instead. func (s storageTransport) GetStoreImage(store storage.Store, ref types.ImageReference) (*storage.Image, error) { dref := ref.DockerReference() if dref != nil { @@ -306,6 +329,12 @@ func (s storageTransport) GetStoreImage(store storage.Store, ref types.ImageRefe return nil, storage.ErrImageUnknown } +// Deprecated: Surprisingly, with a StoreTransport reference which contains an ID, +// this ignores that ID; and repeated calls of GetStoreImage with the same named reference +// can return different images, with no way for the caller to "freeze" the storage.Image identity +// without discarding the name entirely. +// +// Use storage.ResolveReference instead. func (s *storageTransport) GetImage(ref types.ImageReference) (*storage.Image, error) { store, err := s.GetStore() if err != nil {