From d36f5fba2a17d1437831912d068e7953410cf531 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Sat, 29 Nov 2025 16:52:34 +0100 Subject: [PATCH 01/11] Add internal/digests.Options and internal/digests.Options.Choose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The goal is to allow building the digest-choice-dependent machinery from the bottom up without committing to an API while we don't understand the problem space; for now, we don't expose any way for users to actually make a choice. This code is never called at this point, so this should not change behavior. Signed-off-by: Miloslav Trmač --- image/internal/digests/digests.go | 146 ++++++++++++++++++ image/internal/digests/digests_test.go | 201 +++++++++++++++++++++++++ 2 files changed, 347 insertions(+) create mode 100644 image/internal/digests/digests.go create mode 100644 image/internal/digests/digests_test.go diff --git a/image/internal/digests/digests.go b/image/internal/digests/digests.go new file mode 100644 index 0000000000..ca76ea3be7 --- /dev/null +++ b/image/internal/digests/digests.go @@ -0,0 +1,146 @@ +// Package digests provides an internal representation of users’ digest use preferences. +// +// Something like this _might_ be eventually made available as a public API: +// before doing so, carefully think whether the API should be modified before we commit to it. + +package digests + +import ( + "errors" + "fmt" + + "github.com/opencontainers/go-digest" +) + +// Options records users’ preferences for used digest algorithm usage. +// It is a value type and can be copied using ordinary assignment. +// +// It can only be created using one of the provided constructors. +type Options struct { + initialized bool // To prevent uses that don’t call a public constructor; this is necessary to enforce the .Available() promise. + + // If any of the fields below is set, it is guaranteed to be .Available(). + + mustUse digest.Algorithm // If not "", written digests must use this algorithm. + prefer digest.Algorithm // If not "", use this algorithm whenever possible. + defaultAlgo digest.Algorithm // If not "", use this algorithm if there is no reason to use anything else. +} + +// CanonicalDefault is Options which default to using digest.Canonical if there is no reason to use a different algorithm +// (e.g. when there is no pre-existing digest). +// +// The configuration can be customized using .WithPreferred() or .WithDefault(). +func CanonicalDefault() Options { + // This does not set .defaultAlgo so that .WithDefault() can be called (once). + return Options{ + initialized: true, + } +} + +// MustUse constructs Options which always use algo. +func MustUse(algo digest.Algorithm) (Options, error) { + // We don’t provide Options.WithMustUse because there is no other option that makes a difference + // once .mustUse is set. + if !algo.Available() { + return Options{}, fmt.Errorf("attempt to use an unavailable digest algorithm %q", algo.String()) + } + return Options{ + initialized: true, + mustUse: algo, + }, nil +} + +// WithPreferred returns a copy of o with a “preferred” algorithm set to algo. +// The preferred algorithm is used whenever possible (but if there is a strict requirement to use something else, it will be overridden). +func (o Options) WithPreferred(algo digest.Algorithm) (Options, error) { + if err := o.ensureInitialized(); err != nil { + return Options{}, err + } + if o.prefer != "" { + return Options{}, errors.New("digests.Options already have a 'prefer' algorithm configured") + } + + if !algo.Available() { + return Options{}, fmt.Errorf("attempt to use an unavailable digest algorithm %q", algo.String()) + } + o.prefer = algo + return o, nil +} + +// WithDefault returns a copy of o with a “default” algorithm set to algo. +// The default algorithm is used if there is no reason to use anything else (e.g. when there is no pre-existing digest). +func (o Options) WithDefault(algo digest.Algorithm) (Options, error) { + if err := o.ensureInitialized(); err != nil { + return Options{}, err + } + if o.defaultAlgo != "" { + return Options{}, errors.New("digests.Options already have a 'default' algorithm configured") + } + + if !algo.Available() { + return Options{}, fmt.Errorf("attempt to use an unavailable digest algorithm %q", algo.String()) + } + o.defaultAlgo = algo + return o, nil +} + +// ensureInitialized returns an error if o is not initialized. +func (o Options) ensureInitialized() error { + if !o.initialized { + return errors.New("internal error: use of uninitialized digests.Options") + } + return nil +} + +// Situation records the context in which a digest is being chosen. +type Situation struct { + Preexisting digest.Digest // If not "", a pre-existing digest value (frequently one which is cheaper to use than others) + CannotChangeAlgorithmReason string // The reason why we must use Preexisting, or "" if we can use other algorithms. +} + +// Choose chooses a digest algorithm based on the options and the situation. +func (o Options) Choose(s Situation) (digest.Algorithm, error) { + if err := o.ensureInitialized(); err != nil { + return "", err + } + + if s.CannotChangeAlgorithmReason != "" && s.Preexisting == "" { + return "", fmt.Errorf("internal error: digests.Situation.CannotChangeAlgorithmReason is set but Preexisting is empty") + } + + var choice digest.Algorithm // = what we want to use + switch { + case o.mustUse != "": + choice = o.mustUse + case s.CannotChangeAlgorithmReason != "": + choice = s.Preexisting.Algorithm() + if !choice.Available() { + return "", fmt.Errorf("existing digest uses unimplemented algorithm %s", choice) + } + case o.prefer != "": + choice = o.prefer + case s.Preexisting != "" && s.Preexisting.Algorithm().Available(): + choice = s.Preexisting.Algorithm() + case o.defaultAlgo != "": + choice = o.defaultAlgo + default: + choice = digest.Canonical // We assume digest.Canonical is always available. + } + + if s.CannotChangeAlgorithmReason != "" && choice != s.Preexisting.Algorithm() { + return "", fmt.Errorf("requested to always use digest algorithm %s but we cannot replace existing digest algorithm %s: %s", + choice, s.Preexisting.Algorithm(), s.CannotChangeAlgorithmReason) + } + + return choice, nil +} + +// MustUseSet returns an algorithm if o is set to always use a specific algorithm, "" if it is flexible. +func (o Options) MustUseSet() digest.Algorithm { + // We don’t do .ensureInitialized() because that would require an extra error value just for that. + // This should not be a part of any public API either way. + if o.mustUse != "" { + return o.mustUse + } + return "" +} diff --git a/image/internal/digests/digests_test.go b/image/internal/digests/digests_test.go new file mode 100644 index 0000000000..e5a98a851f --- /dev/null +++ b/image/internal/digests/digests_test.go @@ -0,0 +1,201 @@ +package digests + +import ( + "testing" + + "github.com/opencontainers/go-digest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCanonicalDefault(t *testing.T) { + o := CanonicalDefault() + assert.Equal(t, Options{initialized: true}, o) +} + +func TestMustUse(t *testing.T) { + o, err := MustUse(digest.SHA512) + require.NoError(t, err) + assert.Equal(t, Options{ + initialized: true, + mustUse: digest.SHA512, + }, o) + + _, err = MustUse(digest.Algorithm("this is not a known algorithm")) + require.Error(t, err) +} + +func TestOptionsWithPreferred(t *testing.T) { + preferSHA512, err := CanonicalDefault().WithPreferred(digest.SHA512) + require.NoError(t, err) + assert.Equal(t, Options{ + initialized: true, + prefer: digest.SHA512, + }, preferSHA512) + + for _, c := range []struct { + base Options + algo digest.Algorithm + }{ + { // Uninitialized Options + base: Options{}, + algo: digest.SHA256, + }, + { // Unavailable algorithm + base: CanonicalDefault(), + algo: digest.Algorithm("this is not a known algorithm"), + }, + { // WithPreferred already called + base: preferSHA512, + algo: digest.SHA512, + }, + } { + _, err := c.base.WithPreferred(c.algo) + assert.Error(t, err) + } +} + +func TestOptionsWithDefault(t *testing.T) { + defaultSHA512, err := CanonicalDefault().WithDefault(digest.SHA512) + require.NoError(t, err) + assert.Equal(t, Options{ + initialized: true, + defaultAlgo: digest.SHA512, + }, defaultSHA512) + + for _, c := range []struct { + base Options + algo digest.Algorithm + }{ + { // Uninitialized Options + base: Options{}, + algo: digest.SHA256, + }, + { // Unavailable algorithm + base: CanonicalDefault(), + algo: digest.Algorithm("this is not a known algorithm"), + }, + { // WithDefault already called + base: defaultSHA512, + algo: digest.SHA512, + }, + } { + _, err := c.base.WithDefault(c.algo) + assert.Error(t, err) + } +} + +func TestOptionsChoose(t *testing.T) { + sha512Digest := digest.Digest("sha512:cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e") + unknownDigest := digest.Digest("unknown:abcd1234") + + // The tests use sha512 = pre-existing if any; sha384 = primary configured; sha256 = supposedly irrelevant. + + mustSHA384, err := MustUse(digest.SHA384) + require.NoError(t, err) + mustSHA384, err = mustSHA384.WithPreferred(digest.SHA256) + require.NoError(t, err) + mustSHA384, err = mustSHA384.WithDefault(digest.SHA256) + require.NoError(t, err) + + preferSHA384, err := CanonicalDefault().WithPreferred(digest.SHA384) + require.NoError(t, err) + preferSHA384, err = preferSHA384.WithDefault(digest.SHA256) + require.NoError(t, err) + + defaultSHA384, err := CanonicalDefault().WithDefault(digest.SHA384) + require.NoError(t, err) + + cases := []struct { + opts Options + wantDefault digest.Algorithm + wantPreexisting digest.Algorithm // Pre-existing sha512 + wantCannotChange digest.Algorithm // Pre-existing sha512, CannotChange + wantUnavailable digest.Algorithm // Pre-existing unavailable + }{ + { + opts: Options{}, // uninitialized + wantDefault: "", + wantPreexisting: "", + wantCannotChange: "", + wantUnavailable: "", + }, + { + opts: mustSHA384, + wantDefault: digest.SHA384, + wantPreexisting: digest.SHA384, + wantCannotChange: "", + // Warning: We don’t generally _promise_ that unavailable digests are going to be silently ignored + // in these situations (e.g. we might still try to validate them when reading inputs). + wantUnavailable: digest.SHA384, + }, + { + opts: preferSHA384, + wantDefault: digest.SHA384, + wantPreexisting: digest.SHA384, + wantCannotChange: digest.SHA512, + wantUnavailable: digest.SHA384, + }, + { + opts: defaultSHA384, + wantDefault: digest.SHA384, + wantPreexisting: digest.SHA512, + wantCannotChange: digest.SHA512, + wantUnavailable: digest.SHA384, + }, + { + opts: CanonicalDefault(), + wantDefault: digest.SHA256, + wantPreexisting: digest.SHA512, + wantCannotChange: digest.SHA512, + wantUnavailable: digest.SHA256, + }, + } + for _, c := range cases { + run := func(s Situation, want digest.Algorithm) { + res, err := c.opts.Choose(s) + if want == "" { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, want, res) + } + } + + run(Situation{}, c.wantDefault) + run(Situation{Preexisting: sha512Digest}, c.wantPreexisting) + run(Situation{Preexisting: sha512Digest, CannotChangeAlgorithmReason: "test reason"}, c.wantCannotChange) + run(Situation{Preexisting: unknownDigest}, c.wantUnavailable) + + run(Situation{Preexisting: unknownDigest, CannotChangeAlgorithmReason: "test reason"}, "") + run(Situation{CannotChangeAlgorithmReason: "test reason"}, "") // CannotChangeAlgorithm with missing Preexisting + } +} + +func TestOptionsMustUseSet(t *testing.T) { + mustSHA512, err := MustUse(digest.SHA512) + require.NoError(t, err) + prefersSHA512, err := CanonicalDefault().WithPreferred(digest.SHA512) + require.NoError(t, err) + defaultSHA512, err := CanonicalDefault().WithDefault(digest.SHA512) + require.NoError(t, err) + for _, c := range []struct { + opts Options + want digest.Algorithm + }{ + { + opts: mustSHA512, + want: digest.SHA512, + }, + { + opts: prefersSHA512, + want: "", + }, + { + opts: defaultSHA512, + want: "", + }, + } { + assert.Equal(t, c.want, c.opts.MustUseSet()) + } +} From 953fb6569108f231136c8ddb3aabeacde3825075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 5 Dec 2025 23:12:57 +0100 Subject: [PATCH 02/11] Add image.copy.digestOptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit for now, purely to let us internally work on implementing digest choice / conversions; there is no way for a caller to set it. Should not change behavior, the field has no users. Signed-off-by: Miloslav Trmač --- image/copy/copy.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/image/copy/copy.go b/image/copy/copy.go index eed5f8d96d..cc165ff711 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -14,6 +14,7 @@ import ( "github.com/sirupsen/logrus" "go.podman.io/image/v5/docker/reference" internalblobinfocache "go.podman.io/image/v5/internal/blobinfocache" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/image" "go.podman.io/image/v5/internal/imagedestination" "go.podman.io/image/v5/internal/imagesource" @@ -155,6 +156,15 @@ type Options struct { // In oci-archive: destinations, this will set the create/mod/access timestamps in each tar entry // (but not a timestamp of the created archive file). DestinationTimestamp *time.Time + + // FIXME: + // - this reference to an internal type is unusable from the outside even if we made the field public + // - what is the actual semantics? Right now it is probably “choices to use when writing to the destination”, TBD + // - anyway do we want to expose _all_ of the digests.Options tunables, or fewer? + // - … do we want to expose _more_ granularity than that? + // - (“must have at least sha512 integrity when reading”, what does “at least” mean for random pairs of algorithms?) + // - should some of this be in config files, maybe ever per-registry? + digestOptions digests.Options } // OptionCompressionVariant allows to supply information about @@ -200,6 +210,12 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, if options == nil { options = &Options{} } + // FIXME: Currently, digestsOptions is not implemented at all, and exists in the codebase + // only to allow gradually building the feature set. + // After c/image/copy consistently implements it, provide a public digest options API of some kind. + optionsCopy := *options + optionsCopy.digestOptions = digests.CanonicalDefault() + options = &optionsCopy if err := validateImageListSelection(options.ImageListSelection); err != nil { return nil, err From 8211a21035eec5f83144272b2ca2d3599dc0c48b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 1 Dec 2025 15:29:53 +0100 Subject: [PATCH 03/11] oci/archive: Delete temporary directory on error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/oci/archive/oci_transport.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/image/oci/archive/oci_transport.go b/image/oci/archive/oci_transport.go index c0636b53d1..ad982baf55 100644 --- a/image/oci/archive/oci_transport.go +++ b/image/oci/archive/oci_transport.go @@ -161,13 +161,20 @@ func createOCIRef(sys *types.SystemContext, image string) (tempDirOCIRef, error) if err != nil { return tempDirOCIRef{}, fmt.Errorf("creating temp directory: %w", err) } + succeeded := false + defer func() { + if !succeeded { + _ = os.RemoveAll(dir) + } + }() + ociRef, err := ocilayout.NewReference(dir, image) if err != nil { return tempDirOCIRef{}, err } - tempDirRef := tempDirOCIRef{tempDirectory: dir, ociRefExtracted: ociRef} - return tempDirRef, nil + succeeded = true + return tempDirOCIRef{tempDirectory: dir, ociRefExtracted: ociRef}, nil } // creates the temporary directory and copies the tarred content to it From 038ed18c57421629c8bbc0c1021bad3a2aa39da5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 1 Dec 2025 15:30:56 +0100 Subject: [PATCH 04/11] Correctly handle nil SystemContext in BlobCache.NewImageSource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/pkg/blobcache/src.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/image/pkg/blobcache/src.go b/image/pkg/blobcache/src.go index 806d2de269..29a2f43a4c 100644 --- a/image/pkg/blobcache/src.go +++ b/image/pkg/blobcache/src.go @@ -41,7 +41,11 @@ func (b *BlobCache) NewImageSource(ctx context.Context, sys *types.SystemContext return nil, fmt.Errorf("error creating new image source %q: %w", transports.ImageName(b.reference), err) } logrus.Debugf("starting to read from image %q using blob cache in %q (compression=%v)", transports.ImageName(b.reference), b.directory, b.compress) - s := &blobCacheSource{reference: b, source: imagesource.FromPublic(src), sys: *sys} + var sys_ types.SystemContext + if sys != nil { + sys_ = *sys + } + s := &blobCacheSource{reference: b, source: imagesource.FromPublic(src), sys: sys_} s.Compat = impl.AddCompat(s) return s, nil } From 61f527116f33de2f58b17dc4d2f8134d4fa59273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Sat, 6 Dec 2025 01:43:00 +0100 Subject: [PATCH 05/11] Add NewImageSourceWithOptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently it is only called in oci/archive and openshift to forward to another transport; real users will be added later. Should not change behavior (yet). Signed-off-by: Miloslav Trmač --- image/directory/directory_transport.go | 10 +++++++- image/directory/directory_transport_test.go | 3 +++ image/docker/archive/src.go | 4 ++-- image/docker/archive/transport.go | 10 +++++++- image/docker/archive/transport_test.go | 3 +++ image/docker/daemon/daemon_src.go | 6 ++--- image/docker/daemon/daemon_transport.go | 10 +++++++- image/docker/daemon/daemon_transport_test.go | 3 +++ image/docker/docker_image.go | 7 +++++- image/docker/docker_image_src.go | 10 ++++---- image/docker/docker_transport.go | 10 +++++++- image/docker/docker_transport_test.go | 3 +++ image/internal/imagereference/impl/compat.go | 24 ++++++++++++++++++++ image/internal/private/private.go | 20 ++++++++++++++++ image/oci/archive/oci_src.go | 9 ++++---- image/oci/archive/oci_transport.go | 18 ++++++++++++--- image/oci/archive/oci_transport_test.go | 3 +++ image/oci/layout/oci_src.go | 12 +++++----- image/oci/layout/oci_transport.go | 12 ++++++++-- image/oci/layout/oci_transport_test.go | 3 +++ image/openshift/openshift_src.go | 17 +++++++++----- image/openshift/openshift_transport.go | 10 +++++++- image/openshift/openshift_transport_test.go | 3 +++ image/pkg/blobcache/blobcache_test.go | 1 + image/pkg/blobcache/src.go | 19 +++++++++++----- image/sif/src.go | 4 ++-- image/sif/transport.go | 10 +++++++- image/sif/transport_test.go | 3 +++ image/storage/storage_image.go | 7 +++++- image/storage/storage_reference.go | 10 +++++++- image/storage/storage_reference_test.go | 3 +++ image/storage/storage_src.go | 7 +++--- image/tarball/tarball_reference.go | 14 ++++++++++++ image/tarball/tarball_reference_test.go | 5 ++++ image/tarball/tarball_src.go | 3 ++- 35 files changed, 243 insertions(+), 53 deletions(-) create mode 100644 image/internal/imagereference/impl/compat.go create mode 100644 image/tarball/tarball_reference_test.go diff --git a/image/directory/directory_transport.go b/image/directory/directory_transport.go index 165159e588..f3e94c3889 100644 --- a/image/directory/directory_transport.go +++ b/image/directory/directory_transport.go @@ -11,6 +11,8 @@ import ( "go.podman.io/image/v5/directory/explicitfilepath" "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/internal/image" + "go.podman.io/image/v5/internal/imagereference/impl" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/transports" "go.podman.io/image/v5/types" ) @@ -143,10 +145,16 @@ func (ref dirReference) NewImage(ctx context.Context, sys *types.SystemContext) return image.FromReference(ctx, sys, ref) } +// NewImageSourceWithOptions returns a types.ImageSource for this reference. +// The caller must call .Close() on the returned ImageSource. +func (ref dirReference) NewImageSourceWithOptions(ctx context.Context, options private.NewImageSourceOptions) (private.ImageSource, error) { + return newImageSource(ref) +} + // NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (ref dirReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - return newImageSource(ref) + return impl.NewImageSource(ref, ctx, sys) } // NewImageDestination returns a types.ImageDestination for this reference. diff --git a/image/directory/directory_transport_test.go b/image/directory/directory_transport_test.go index b9e6ce581c..720afc560e 100644 --- a/image/directory/directory_transport_test.go +++ b/image/directory/directory_transport_test.go @@ -9,10 +9,13 @@ import ( digest "github.com/opencontainers/go-digest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.podman.io/image/v5/internal/private" _ "go.podman.io/image/v5/internal/testing/explicitfilepath-tmpdir" "go.podman.io/image/v5/types" ) +var _ private.ImageReference = (*dirReference)(nil) + func TestTransportName(t *testing.T) { assert.Equal(t, "dir", Transport.Name()) } diff --git a/image/docker/archive/src.go b/image/docker/archive/src.go index 8ed60b3ba6..37deb72d24 100644 --- a/image/docker/archive/src.go +++ b/image/docker/archive/src.go @@ -13,14 +13,14 @@ type archiveImageSource struct { // newImageSource returns a types.ImageSource for the specified image reference. // The caller must call .Close() on the returned ImageSource. -func newImageSource(sys *types.SystemContext, ref archiveReference) (private.ImageSource, error) { +func newImageSource(ref archiveReference, options private.NewImageSourceOptions) (private.ImageSource, error) { var archive *tarfile.Reader var closeArchive bool if ref.archiveReader != nil { archive = ref.archiveReader closeArchive = false } else { - a, err := tarfile.NewReaderFromFile(sys, ref.path) + a, err := tarfile.NewReaderFromFile(options.Sys, ref.path) if err != nil { return nil, err } diff --git a/image/docker/archive/transport.go b/image/docker/archive/transport.go index a1b77adbef..4dd774c730 100644 --- a/image/docker/archive/transport.go +++ b/image/docker/archive/transport.go @@ -10,6 +10,8 @@ import ( "go.podman.io/image/v5/docker/internal/tarfile" "go.podman.io/image/v5/docker/reference" ctrImage "go.podman.io/image/v5/internal/image" + "go.podman.io/image/v5/internal/imagereference/impl" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/transports" "go.podman.io/image/v5/types" ) @@ -191,10 +193,16 @@ func (ref archiveReference) NewImage(ctx context.Context, sys *types.SystemConte return ctrImage.FromReference(ctx, sys, ref) } +// NewImageSourceWithOptions returns a types.ImageSource for this reference. +// The caller must call .Close() on the returned ImageSource. +func (ref archiveReference) NewImageSourceWithOptions(ctx context.Context, options private.NewImageSourceOptions) (private.ImageSource, error) { + return newImageSource(ref, options) +} + // NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (ref archiveReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - return newImageSource(sys, ref) + return impl.NewImageSource(ref, ctx, sys) } // NewImageDestination returns a types.ImageDestination for this reference. diff --git a/image/docker/archive/transport_test.go b/image/docker/archive/transport_test.go index 0b3c1b84a5..32787d5d3f 100644 --- a/image/docker/archive/transport_test.go +++ b/image/docker/archive/transport_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.podman.io/image/v5/docker/reference" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/types" ) @@ -19,6 +20,8 @@ const ( tarFixture = "fixtures/almostempty.tar" ) +var _ private.ImageReference = (*archiveReference)(nil) + func TestTransportName(t *testing.T) { assert.Equal(t, "docker-archive", Transport.Name()) } diff --git a/image/docker/daemon/daemon_src.go b/image/docker/daemon/daemon_src.go index 53f7a8fb86..aac112b9d3 100644 --- a/image/docker/daemon/daemon_src.go +++ b/image/docker/daemon/daemon_src.go @@ -23,8 +23,8 @@ type daemonImageSource struct { // (We could, perhaps, expect an exact sequence, assume that the first plaintext file // is the config, and that the following len(RootFS) files are the layers, but that feels // way too brittle.) -func newImageSource(ctx context.Context, sys *types.SystemContext, ref daemonReference) (private.ImageSource, error) { - c, err := newDockerClient(sys) +func newImageSource(ctx context.Context, ref daemonReference, options private.NewImageSourceOptions) (private.ImageSource, error) { + c, err := newDockerClient(options.Sys) if err != nil { return nil, fmt.Errorf("initializing docker engine client: %w", err) } @@ -38,7 +38,7 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref daemonRef } defer inputStream.Close() - archive, err := tarfile.NewReaderFromStream(sys, inputStream) + archive, err := tarfile.NewReaderFromStream(options.Sys, inputStream) if err != nil { return nil, err } diff --git a/image/docker/daemon/daemon_transport.go b/image/docker/daemon/daemon_transport.go index 028ea0ee08..0e4fb8c879 100644 --- a/image/docker/daemon/daemon_transport.go +++ b/image/docker/daemon/daemon_transport.go @@ -9,6 +9,8 @@ import ( "go.podman.io/image/v5/docker/policyconfiguration" "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/internal/image" + "go.podman.io/image/v5/internal/imagereference/impl" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/transports" "go.podman.io/image/v5/types" ) @@ -201,10 +203,16 @@ func (ref daemonReference) NewImage(ctx context.Context, sys *types.SystemContex return image.FromReference(ctx, sys, ref) } +// NewImageSourceWithOptions returns a types.ImageSource for this reference. +// The caller must call .Close() on the returned ImageSource. +func (ref daemonReference) NewImageSourceWithOptions(ctx context.Context, options private.NewImageSourceOptions) (private.ImageSource, error) { + return newImageSource(ctx, ref, options) +} + // NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (ref daemonReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - return newImageSource(ctx, sys, ref) + return impl.NewImageSource(ref, ctx, sys) } // NewImageDestination returns a types.ImageDestination for this reference. diff --git a/image/docker/daemon/daemon_transport_test.go b/image/docker/daemon/daemon_transport_test.go index f14e3bfd33..cfe7276897 100644 --- a/image/docker/daemon/daemon_transport_test.go +++ b/image/docker/daemon/daemon_transport_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.podman.io/image/v5/docker/reference" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/types" ) @@ -16,6 +17,8 @@ const ( sha256digest = "sha256:" + sha256digestHex ) +var _ private.ImageReference = (*daemonReference)(nil) + func TestTransportName(t *testing.T) { assert.Equal(t, "docker-daemon", Transport.Name()) } diff --git a/image/docker/docker_image.go b/image/docker/docker_image.go index 1e5de65a74..014a08a9a8 100644 --- a/image/docker/docker_image.go +++ b/image/docker/docker_image.go @@ -12,7 +12,9 @@ import ( "github.com/opencontainers/go-digest" "github.com/sirupsen/logrus" "go.podman.io/image/v5/docker/reference" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/image" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/manifest" "go.podman.io/image/v5/types" ) @@ -28,7 +30,10 @@ type Image struct { // a client to the registry hosting the given image. // The caller must call .Close() on the returned Image. func newImage(ctx context.Context, sys *types.SystemContext, ref dockerReference) (types.ImageCloser, error) { - s, err := newImageSource(ctx, sys, ref) + s, err := newImageSource(ctx, ref, private.NewImageSourceOptions{ + Sys: sys, + Digests: digests.CanonicalDefault(), + }) if err != nil { return nil, err } diff --git a/image/docker/docker_image_src.go b/image/docker/docker_image_src.go index 4003af5d27..b5e2a652ac 100644 --- a/image/docker/docker_image_src.go +++ b/image/docker/docker_image_src.go @@ -53,16 +53,16 @@ type dockerImageSource struct { // newImageSource creates a new ImageSource for the specified image reference. // The caller must call .Close() on the returned ImageSource. // The caller must ensure !ref.isUnknownDigest. -func newImageSource(ctx context.Context, sys *types.SystemContext, ref dockerReference) (*dockerImageSource, error) { +func newImageSource(ctx context.Context, ref dockerReference, options private.NewImageSourceOptions) (*dockerImageSource, error) { if ref.isUnknownDigest { return nil, fmt.Errorf("reading images from docker: reference %q without a tag or digest is not supported", ref.StringWithinTransport()) } - registryConfig, err := loadRegistryConfiguration(sys) + registryConfig, err := loadRegistryConfiguration(options.Sys) if err != nil { return nil, err } - registry, err := sysregistriesv2.FindRegistry(sys, ref.ref.Name()) + registry, err := sysregistriesv2.FindRegistry(options.Sys, ref.ref.Name()) if err != nil { return nil, fmt.Errorf("loading registries configuration: %w", err) } @@ -92,12 +92,12 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref dockerRef } attempts := []attempt{} for _, pullSource := range pullSources { - if sys != nil && sys.DockerLogMirrorChoice { + if options.Sys != nil && options.Sys.DockerLogMirrorChoice { logrus.Infof("Trying to access %q", pullSource.Reference) } else { logrus.Debugf("Trying to access %q", pullSource.Reference) } - s, err := newImageSourceAttempt(ctx, sys, ref, pullSource, registryConfig) + s, err := newImageSourceAttempt(ctx, options.Sys, ref, pullSource, registryConfig) if err == nil { return s, nil } diff --git a/image/docker/docker_transport.go b/image/docker/docker_transport.go index 5831dc3cea..ca52ef62d6 100644 --- a/image/docker/docker_transport.go +++ b/image/docker/docker_transport.go @@ -8,6 +8,8 @@ import ( "go.podman.io/image/v5/docker/policyconfiguration" "go.podman.io/image/v5/docker/reference" + "go.podman.io/image/v5/internal/imagereference/impl" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/transports" "go.podman.io/image/v5/types" ) @@ -177,10 +179,16 @@ func (ref dockerReference) NewImage(ctx context.Context, sys *types.SystemContex return newImage(ctx, sys, ref) } +// NewImageSourceWithOptions returns a types.ImageSource for this reference. +// The caller must call .Close() on the returned ImageSource. +func (ref dockerReference) NewImageSourceWithOptions(ctx context.Context, options private.NewImageSourceOptions) (private.ImageSource, error) { + return newImageSource(ctx, ref, options) +} + // NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (ref dockerReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - return newImageSource(ctx, sys, ref) + return impl.NewImageSource(ref, ctx, sys) } // NewImageDestination returns a types.ImageDestination for this reference. diff --git a/image/docker/docker_transport_test.go b/image/docker/docker_transport_test.go index df77536031..6174d5b53e 100644 --- a/image/docker/docker_transport_test.go +++ b/image/docker/docker_transport_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.podman.io/image/v5/docker/reference" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/types" ) @@ -17,6 +18,8 @@ const ( unknownDigestSuffixTest = "@@unknown-digest@@" ) +var _ private.ImageReference = (*dockerReference)(nil) + func TestTransportName(t *testing.T) { assert.Equal(t, "docker", Transport.Name()) } diff --git a/image/internal/imagereference/impl/compat.go b/image/internal/imagereference/impl/compat.go new file mode 100644 index 0000000000..c0fb8ce3e8 --- /dev/null +++ b/image/internal/imagereference/impl/compat.go @@ -0,0 +1,24 @@ +package impl + +import ( + "context" + + "go.podman.io/image/v5/internal/digests" + "go.podman.io/image/v5/internal/private" + "go.podman.io/image/v5/types" +) + +// This does not define a Compat struct similar to imagesource/impl, because that +// would ~require the ImageReference implementations to use a pointer receiver, +// and structurally they are much closer to value types. +// +// (In particular, the c/storage transport copies reference struct by value in some code, +// and it’s easier to keep it that way.) + +// NewImageSource implements types.ImageReference.NewImageSource for private.ImageReference. +func NewImageSource(ref private.ImageReferenceInternalOnly, ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { + return ref.NewImageSourceWithOptions(ctx, private.NewImageSourceOptions{ + Sys: sys, + Digests: digests.CanonicalDefault(), + }) +} diff --git a/image/internal/private/private.go b/image/internal/private/private.go index a5d2057aef..824bd6a857 100644 --- a/image/internal/private/private.go +++ b/image/internal/private/private.go @@ -9,11 +9,31 @@ import ( imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/internal/blobinfocache" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/signature" compression "go.podman.io/image/v5/pkg/compression/types" "go.podman.io/image/v5/types" ) +// ImageReferenceInternalOnly is the part of private.ImageReference that is not +// a part of types.ImageReference +type ImageReferenceInternalOnly interface { + // NewImageSourceWithOptions returns a types.ImageSource for this reference. + // The caller must call .Close() on the returned ImageSource. + NewImageSourceWithOptions(ctx context.Context, options NewImageSourceOptions) (ImageSource, error) +} + +// ImageReference is an internal extension to the types.ImageReference interface. +type ImageReference interface { + types.ImageReference + ImageReferenceInternalOnly +} + +type NewImageSourceOptions struct { + Sys *types.SystemContext + Digests digests.Options // If creating a source involves computing new digests +} + // ImageSourceInternalOnly is the part of private.ImageSource that is not // a part of types.ImageSource. type ImageSourceInternalOnly interface { diff --git a/image/oci/archive/oci_src.go b/image/oci/archive/oci_src.go index cb15c06f03..dcc7ff043c 100644 --- a/image/oci/archive/oci_src.go +++ b/image/oci/archive/oci_src.go @@ -9,7 +9,6 @@ import ( digest "github.com/opencontainers/go-digest" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" - "go.podman.io/image/v5/internal/imagesource" "go.podman.io/image/v5/internal/imagesource/impl" "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/internal/signature" @@ -50,13 +49,13 @@ type ociArchiveImageSource struct { // 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) (private.ImageSource, error) { - tempDirRef, err := createUntarTempDir(sys, ref) +func newImageSource(ctx context.Context, ref ociArchiveReference, options private.NewImageSourceOptions) (private.ImageSource, error) { + tempDirRef, err := createUntarTempDir(options.Sys, ref) if err != nil { return nil, fmt.Errorf("creating temp directory: %w", err) } - unpackedSrc, err := tempDirRef.ociRefExtracted.NewImageSource(ctx, sys) + unpackedSrc, err := tempDirRef.ociRefExtracted.NewImageSourceWithOptions(ctx, options) if err != nil { var notFound ocilayout.ImageNotFoundError if errors.As(err, ¬Found) { @@ -69,7 +68,7 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref ociArchiv } s := &ociArchiveImageSource{ ref: ref, - unpackedSrc: imagesource.FromPublic(unpackedSrc), + unpackedSrc: unpackedSrc, tempDirRef: tempDirRef, } s.Compat = impl.AddCompat(s) diff --git a/image/oci/archive/oci_transport.go b/image/oci/archive/oci_transport.go index ad982baf55..41392a4c6b 100644 --- a/image/oci/archive/oci_transport.go +++ b/image/oci/archive/oci_transport.go @@ -11,6 +11,8 @@ import ( "go.podman.io/image/v5/directory/explicitfilepath" "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/internal/image" + "go.podman.io/image/v5/internal/imagereference/impl" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/internal/tmpdir" "go.podman.io/image/v5/oci/internal" ocilayout "go.podman.io/image/v5/oci/layout" @@ -126,10 +128,16 @@ func (ref ociArchiveReference) NewImage(ctx context.Context, sys *types.SystemCo return image.FromReference(ctx, sys, ref) } +// NewImageSourceWithOptions returns a types.ImageSource for this reference. +// The caller must call .Close() on the returned ImageSource. +func (ref ociArchiveReference) NewImageSourceWithOptions(ctx context.Context, options private.NewImageSourceOptions) (private.ImageSource, error) { + return newImageSource(ctx, ref, options) +} + // NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (ref ociArchiveReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - return newImageSource(ctx, sys, ref) + return impl.NewImageSource(ref, ctx, sys) } // NewImageDestination returns a types.ImageDestination for this reference. @@ -146,7 +154,7 @@ func (ref ociArchiveReference) DeleteImage(ctx context.Context, sys *types.Syste // struct to store the ociReference and temporary directory returned by createOCIRef type tempDirOCIRef struct { tempDirectory string - ociRefExtracted types.ImageReference + ociRefExtracted private.ImageReference } // deletes the temporary directory created @@ -168,10 +176,14 @@ func createOCIRef(sys *types.SystemContext, image string) (tempDirOCIRef, error) } }() - ociRef, err := ocilayout.NewReference(dir, image) + ociRef_, err := ocilayout.NewReference(dir, image) if err != nil { return tempDirOCIRef{}, err } + ociRef, ok := ociRef_.(private.ImageReference) + if !ok { + return tempDirOCIRef{}, errors.New("internal error: oci/layout does not implement private.ImageReference") + } succeeded = true return tempDirOCIRef{tempDirectory: dir, ociRefExtracted: ociRef}, nil diff --git a/image/oci/archive/oci_transport_test.go b/image/oci/archive/oci_transport_test.go index 41520795b1..3eb229b88d 100644 --- a/image/oci/archive/oci_transport_test.go +++ b/image/oci/archive/oci_transport_test.go @@ -11,10 +11,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.podman.io/image/v5/internal/private" _ "go.podman.io/image/v5/internal/testing/explicitfilepath-tmpdir" "go.podman.io/image/v5/types" ) +var _ private.ImageReference = (*ociArchiveReference)(nil) + func TestTransportName(t *testing.T) { assert.Equal(t, "oci-archive", Transport.Name()) } diff --git a/image/oci/layout/oci_src.go b/image/oci/layout/oci_src.go index f265a21d70..a5a18efa6e 100644 --- a/image/oci/layout/oci_src.go +++ b/image/oci/layout/oci_src.go @@ -49,7 +49,7 @@ type ociImageSource struct { } // newImageSource returns an ImageSource for reading from an existing directory. -func newImageSource(sys *types.SystemContext, ref ociReference) (private.ImageSource, error) { +func newImageSource(ref ociReference, options private.NewImageSourceOptions) (private.ImageSource, error) { tr := tlsclientconfig.NewTransport() tr.TLSClientConfig = &tls.Config{ // As of 2025-08, tlsconfig.ClientDefault() differs from Go 1.23 defaults only in CipherSuites; @@ -60,11 +60,11 @@ func newImageSource(sys *types.SystemContext, ref ociReference) (private.ImageSo CipherSuites: tlsconfig.ClientDefault().CipherSuites, } - if sys != nil && sys.OCICertPath != "" { - if err := tlsclientconfig.SetupCertificates(sys.OCICertPath, tr.TLSClientConfig); err != nil { + if options.Sys != nil && options.Sys.OCICertPath != "" { + if err := tlsclientconfig.SetupCertificates(options.Sys.OCICertPath, tr.TLSClientConfig); err != nil { return nil, err } - tr.TLSClientConfig.InsecureSkipVerify = sys.OCIInsecureSkipTLSVerify + tr.TLSClientConfig.InsecureSkipVerify = options.Sys.OCIInsecureSkipTLSVerify } client := &http.Client{} @@ -88,9 +88,9 @@ func newImageSource(sys *types.SystemContext, ref ociReference) (private.ImageSo descriptor: descriptor, client: client, } - if sys != nil { + if options.Sys != nil { // TODO(jonboulle): check dir existence? - s.sharedBlobDir = sys.OCISharedBlobDirPath + s.sharedBlobDir = options.Sys.OCISharedBlobDirPath } s.Compat = impl.AddCompat(s) return s, nil diff --git a/image/oci/layout/oci_transport.go b/image/oci/layout/oci_transport.go index 7b5086cd88..015b486bee 100644 --- a/image/oci/layout/oci_transport.go +++ b/image/oci/layout/oci_transport.go @@ -14,7 +14,9 @@ import ( "go.podman.io/image/v5/directory/explicitfilepath" "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/internal/image" + "go.podman.io/image/v5/internal/imagereference/impl" "go.podman.io/image/v5/internal/manifest" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/oci/internal" "go.podman.io/image/v5/transports" "go.podman.io/image/v5/types" @@ -86,7 +88,7 @@ func ParseReference(reference string) (types.ImageReference, error) { // 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, sourceIndex int) (types.ImageReference, error) { +func newReference(dir, image string, sourceIndex int) (private.ImageReference, error) { resolved, err := explicitfilepath.ResolvePathToFullyExplicit(dir) if err != nil { return nil, err @@ -267,10 +269,16 @@ func LoadManifestDescriptor(imgRef types.ImageReference) (imgspecv1.Descriptor, return md, err } +// NewImageSourceWithOptions returns a types.ImageSource for this reference. +// The caller must call .Close() on the returned ImageSource. +func (ref ociReference) NewImageSourceWithOptions(ctx context.Context, options private.NewImageSourceOptions) (private.ImageSource, error) { + return newImageSource(ref, options) +} + // NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (ref ociReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - return newImageSource(sys, ref) + return impl.NewImageSource(ref, ctx, sys) } // NewImageDestination returns a types.ImageDestination for this reference. diff --git a/image/oci/layout/oci_transport_test.go b/image/oci/layout/oci_transport_test.go index 1f24ca4bcc..1d7e0f897d 100644 --- a/image/oci/layout/oci_transport_test.go +++ b/image/oci/layout/oci_transport_test.go @@ -9,10 +9,13 @@ import ( imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.podman.io/image/v5/internal/private" _ "go.podman.io/image/v5/internal/testing/explicitfilepath-tmpdir" "go.podman.io/image/v5/types" ) +var _ private.ImageReference = (*ociReference)(nil) + func TestGetManifestDescriptor(t *testing.T) { emptyDir := t.TempDir() diff --git a/image/openshift/openshift_src.go b/image/openshift/openshift_src.go index 4842ba3018..aed1f826f0 100644 --- a/image/openshift/openshift_src.go +++ b/image/openshift/openshift_src.go @@ -28,7 +28,7 @@ type openshiftImageSource struct { client *openshiftClient // Values specific to this image - sys *types.SystemContext + newImageSourceOptions private.NewImageSourceOptions // State docker types.ImageSource // The docker/distribution API endpoint, or nil if not resolved yet imageStreamImageName string // Resolved image identifier, or "" if not known yet @@ -36,7 +36,7 @@ type openshiftImageSource struct { // newImageSource creates a new ImageSource for the specified reference. // The caller must call .Close() on the returned ImageSource. -func newImageSource(sys *types.SystemContext, ref openshiftReference) (private.ImageSource, error) { +func newImageSource(ref openshiftReference, options private.NewImageSourceOptions) (private.ImageSource, error) { client, err := newOpenshiftClient(ref) if err != nil { return nil, err @@ -45,8 +45,8 @@ func newImageSource(sys *types.SystemContext, ref openshiftReference) (private.I s := &openshiftImageSource{ NoGetBlobAtInitialize: stubs.NoGetBlobAt(ref), - client: client, - sys: sys, + client: client, + newImageSourceOptions: options, } s.Compat = impl.AddCompat(s) return s, nil @@ -163,11 +163,16 @@ func (s *openshiftImageSource) ensureImageIsResolved(ctx context.Context) error return err } logrus.Debugf("Resolved reference %#v", dockerRefString) - dockerRef, err := docker.ParseReference("//" + dockerRefString) + dockerRef_, err := docker.ParseReference("//" + dockerRefString) if err != nil { return err } - d, err := dockerRef.NewImageSource(ctx, s.sys) + dockerRef, ok := dockerRef_.(private.ImageReference) + if !ok { + return errors.New("internal error: docker does not implement private.ImageReference") + } + + d, err := dockerRef.NewImageSourceWithOptions(ctx, s.newImageSourceOptions) if err != nil { return err } diff --git a/image/openshift/openshift_transport.go b/image/openshift/openshift_transport.go index 8d959d61c0..53cb016837 100644 --- a/image/openshift/openshift_transport.go +++ b/image/openshift/openshift_transport.go @@ -9,6 +9,8 @@ import ( "go.podman.io/image/v5/docker/policyconfiguration" "go.podman.io/image/v5/docker/reference" genericImage "go.podman.io/image/v5/internal/image" + "go.podman.io/image/v5/internal/imagereference/impl" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/transports" "go.podman.io/image/v5/types" "go.podman.io/storage/pkg/regexp" @@ -135,10 +137,16 @@ func (ref openshiftReference) NewImage(ctx context.Context, sys *types.SystemCon return genericImage.FromReference(ctx, sys, ref) } +// NewImageSourceWithOptions returns a types.ImageSource for this reference. +// The caller must call .Close() on the returned ImageSource. +func (ref openshiftReference) NewImageSourceWithOptions(ctx context.Context, options private.NewImageSourceOptions) (private.ImageSource, error) { + return newImageSource(ref, options) +} + // NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (ref openshiftReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - return newImageSource(sys, ref) + return impl.NewImageSource(ref, ctx, sys) } // NewImageDestination returns a types.ImageDestination for this reference. diff --git a/image/openshift/openshift_transport_test.go b/image/openshift/openshift_transport_test.go index 0dc7435572..d11001576b 100644 --- a/image/openshift/openshift_transport_test.go +++ b/image/openshift/openshift_transport_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.podman.io/image/v5/docker/reference" + "go.podman.io/image/v5/internal/private" ) const ( @@ -14,6 +15,8 @@ const ( sha256digest = "@sha256:" + sha256digestHex ) +var _ private.ImageReference = (*openshiftReference)(nil) + func TestTransportName(t *testing.T) { assert.Equal(t, "atomic", Transport.Name()) } diff --git a/image/pkg/blobcache/blobcache_test.go b/image/pkg/blobcache/blobcache_test.go index 5af0429fdd..deab622979 100644 --- a/image/pkg/blobcache/blobcache_test.go +++ b/image/pkg/blobcache/blobcache_test.go @@ -30,6 +30,7 @@ import ( var ( _ types.ImageReference = &BlobCache{} + _ private.ImageReference = &BlobCache{} _ types.ImageSource = &blobCacheSource{} _ private.ImageSource = (*blobCacheSource)(nil) _ types.ImageDestination = &blobCacheDestination{} diff --git a/image/pkg/blobcache/src.go b/image/pkg/blobcache/src.go index 29a2f43a4c..8c30c2c8ce 100644 --- a/image/pkg/blobcache/src.go +++ b/image/pkg/blobcache/src.go @@ -12,6 +12,7 @@ import ( v1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" "go.podman.io/image/v5/internal/image" + refImpl "go.podman.io/image/v5/internal/imagereference/impl" "go.podman.io/image/v5/internal/imagesource" "go.podman.io/image/v5/internal/imagesource/impl" "go.podman.io/image/v5/internal/manifest" @@ -35,21 +36,27 @@ type blobCacheSource struct { cacheErrors int64 } -func (b *BlobCache) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - src, err := b.reference.NewImageSource(ctx, sys) +// NewImageSourceWithOptions returns a types.ImageSource for this reference. +// The caller must call .Close() on the returned ImageSource. +func (b *BlobCache) NewImageSourceWithOptions(ctx context.Context, options private.NewImageSourceOptions) (private.ImageSource, error) { + src, err := b.reference.NewImageSource(ctx, options.Sys) if err != nil { return nil, fmt.Errorf("error creating new image source %q: %w", transports.ImageName(b.reference), err) } logrus.Debugf("starting to read from image %q using blob cache in %q (compression=%v)", transports.ImageName(b.reference), b.directory, b.compress) - var sys_ types.SystemContext - if sys != nil { - sys_ = *sys + var sys types.SystemContext + if options.Sys != nil { + sys = *options.Sys } - s := &blobCacheSource{reference: b, source: imagesource.FromPublic(src), sys: sys_} + s := &blobCacheSource{reference: b, source: imagesource.FromPublic(src), sys: sys} s.Compat = impl.AddCompat(s) return s, nil } +func (b *BlobCache) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { + return refImpl.NewImageSource(b, ctx, sys) +} + func (s *blobCacheSource) Reference() types.ImageReference { return s.reference } diff --git a/image/sif/src.go b/image/sif/src.go index 5aaa74218d..60ae3f63b5 100644 --- a/image/sif/src.go +++ b/image/sif/src.go @@ -64,7 +64,7 @@ func getBlobInfo(path string) (digest.Digest, int64, error) { // newImageSource returns an ImageSource for reading from an existing directory. // newImageSource extracts SIF objects and saves them in a temp directory. -func newImageSource(ctx context.Context, sys *types.SystemContext, ref sifReference) (private.ImageSource, error) { +func newImageSource(ctx context.Context, ref sifReference, options private.NewImageSourceOptions) (private.ImageSource, error) { sifImg, err := sif.LoadContainerFromPath(ref.file, sif.OptLoadWithFlag(os.O_RDONLY)) if err != nil { return nil, fmt.Errorf("loading SIF file: %w", err) @@ -73,7 +73,7 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref sifRefere _ = sifImg.UnloadContainer() }() - workDir, err := tmpdir.MkDirBigFileTemp(sys, "sif") + workDir, err := tmpdir.MkDirBigFileTemp(options.Sys, "sif") if err != nil { return nil, fmt.Errorf("creating temp directory: %w", err) } diff --git a/image/sif/transport.go b/image/sif/transport.go index 6a42f13272..b7f7b1a3ef 100644 --- a/image/sif/transport.go +++ b/image/sif/transport.go @@ -10,6 +10,8 @@ import ( "go.podman.io/image/v5/directory/explicitfilepath" "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/internal/image" + "go.podman.io/image/v5/internal/imagereference/impl" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/transports" "go.podman.io/image/v5/types" ) @@ -142,10 +144,16 @@ func (ref sifReference) NewImage(ctx context.Context, sys *types.SystemContext) return image.FromReference(ctx, sys, ref) } +// NewImageSourceWithOptions returns a types.ImageSource for this reference. +// The caller must call .Close() on the returned ImageSource. +func (ref sifReference) NewImageSourceWithOptions(ctx context.Context, options private.NewImageSourceOptions) (private.ImageSource, error) { + return newImageSource(ctx, ref, options) +} + // NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (ref sifReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - return newImageSource(ctx, sys, ref) + return impl.NewImageSource(ref, ctx, sys) } // NewImageDestination returns a types.ImageDestination for this reference. diff --git a/image/sif/transport_test.go b/image/sif/transport_test.go index 49837c8a45..e1983f3d3a 100644 --- a/image/sif/transport_test.go +++ b/image/sif/transport_test.go @@ -8,10 +8,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.podman.io/image/v5/internal/private" _ "go.podman.io/image/v5/internal/testing/explicitfilepath-tmpdir" "go.podman.io/image/v5/types" ) +var _ private.ImageReference = (*sifReference)(nil) + func TestTransportName(t *testing.T) { assert.Equal(t, "sif", Transport.Name()) } diff --git a/image/storage/storage_image.go b/image/storage/storage_image.go index 7d14668357..ba3501672b 100644 --- a/image/storage/storage_image.go +++ b/image/storage/storage_image.go @@ -6,7 +6,9 @@ import ( "context" digest "github.com/opencontainers/go-digest" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/image" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/types" "go.podman.io/storage" ) @@ -52,7 +54,10 @@ func (s *storageImageCloser) Size() (int64, error) { // newImage creates an image that also knows its size func newImage(ctx context.Context, sys *types.SystemContext, s storageReference) (types.ImageCloser, error) { - src, err := newImageSource(sys, s) + src, err := newImageSource(s, private.NewImageSourceOptions{ + Sys: sys, + Digests: digests.CanonicalDefault(), + }) if err != nil { return nil, err } diff --git a/image/storage/storage_reference.go b/image/storage/storage_reference.go index dacffeef78..cd80b4019f 100644 --- a/image/storage/storage_reference.go +++ b/image/storage/storage_reference.go @@ -11,6 +11,8 @@ import ( digest "github.com/opencontainers/go-digest" "github.com/sirupsen/logrus" "go.podman.io/image/v5/docker/reference" + "go.podman.io/image/v5/internal/imagereference/impl" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/manifest" "go.podman.io/image/v5/transports" "go.podman.io/image/v5/types" @@ -286,8 +288,14 @@ func (s storageReference) DeleteImage(ctx context.Context, sys *types.SystemCont return err } +// NewImageSourceWithOptions returns a types.ImageSource for this reference. +// The caller must call .Close() on the returned ImageSource. +func (s storageReference) NewImageSourceWithOptions(ctx context.Context, options private.NewImageSourceOptions) (private.ImageSource, error) { + return newImageSource(s, options) +} + func (s storageReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - return newImageSource(sys, s) + return impl.NewImageSource(s, ctx, sys) } func (s storageReference) NewImageDestination(ctx context.Context, sys *types.SystemContext) (types.ImageDestination, error) { diff --git a/image/storage/storage_reference_test.go b/image/storage/storage_reference_test.go index 9ee5619ced..8ec7feb14d 100644 --- a/image/storage/storage_reference_test.go +++ b/image/storage/storage_reference_test.go @@ -10,11 +10,14 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.podman.io/image/v5/docker/reference" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/pkg/blobinfocache/memory" "go.podman.io/storage" "go.podman.io/storage/pkg/archive" ) +var _ private.ImageReference = (*storageReference)(nil) + func TestNewReference(t *testing.T) { newStore(t) st, ok := Transport.(*storageTransport) diff --git a/image/storage/storage_src.go b/image/storage/storage_src.go index e76b9ceb03..4aa35609da 100644 --- a/image/storage/storage_src.go +++ b/image/storage/storage_src.go @@ -21,6 +21,7 @@ import ( "go.podman.io/image/v5/internal/image" "go.podman.io/image/v5/internal/imagesource/impl" "go.podman.io/image/v5/internal/imagesource/stubs" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/internal/signature" "go.podman.io/image/v5/internal/tmpdir" "go.podman.io/image/v5/manifest" @@ -62,9 +63,9 @@ type getBlobMutexProtected struct { const expectedLayerDiffIDFlag = "expected-layer-diffid" // newImageSource sets up an image for reading. -func newImageSource(sys *types.SystemContext, imageRef storageReference) (*storageImageSource, error) { +func newImageSource(imageRef storageReference, options private.NewImageSourceOptions) (*storageImageSource, error) { // First, locate the image. - img, err := imageRef.resolveImage(sys) + img, err := imageRef.resolveImage(options.Sys) if err != nil { return nil, err } @@ -77,7 +78,7 @@ func newImageSource(sys *types.SystemContext, imageRef storageReference) (*stora NoGetBlobAtInitialize: stubs.NoGetBlobAt(imageRef), imageRef: imageRef, - systemContext: sys, + systemContext: options.Sys, image: img, metadata: storageImageMetadata{ SignatureSizes: []int{}, diff --git a/image/tarball/tarball_reference.go b/image/tarball/tarball_reference.go index 81f84fbee5..493f9e17de 100644 --- a/image/tarball/tarball_reference.go +++ b/image/tarball/tarball_reference.go @@ -10,6 +10,8 @@ import ( imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/internal/image" + "go.podman.io/image/v5/internal/imagereference/impl" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/types" ) @@ -68,6 +70,18 @@ func (r *tarballReference) NewImage(ctx context.Context, sys *types.SystemContex return image.FromReference(ctx, sys, r) } +// NewImageSourceWithOptions returns a types.ImageSource for this reference. +// The caller must call .Close() on the returned ImageSource. +func (r *tarballReference) NewImageSourceWithOptions(ctx context.Context, options private.NewImageSourceOptions) (private.ImageSource, error) { + return r.newImageSource() +} + +// NewImageSource returns a types.ImageSource for this reference. +// The caller must call .Close() on the returned ImageSource. +func (r *tarballReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { + return impl.NewImageSource(r, ctx, sys) +} + func (r *tarballReference) DeleteImage(ctx context.Context, sys *types.SystemContext) error { for _, filename := range r.filenames { if err := os.Remove(filename); err != nil && !os.IsNotExist(err) { diff --git a/image/tarball/tarball_reference_test.go b/image/tarball/tarball_reference_test.go new file mode 100644 index 0000000000..dfc5e76506 --- /dev/null +++ b/image/tarball/tarball_reference_test.go @@ -0,0 +1,5 @@ +package tarball + +import "go.podman.io/image/v5/internal/private" + +var _ private.ImageReference = (*tarballReference)(nil) diff --git a/image/tarball/tarball_src.go b/image/tarball/tarball_src.go index a8af4b3256..a3bd0f3b38 100644 --- a/image/tarball/tarball_src.go +++ b/image/tarball/tarball_src.go @@ -17,6 +17,7 @@ import ( imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "go.podman.io/image/v5/internal/imagesource/impl" "go.podman.io/image/v5/internal/imagesource/stubs" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/pkg/compression" compressionTypes "go.podman.io/image/v5/pkg/compression/types" "go.podman.io/image/v5/types" @@ -41,7 +42,7 @@ type tarballBlob struct { size int64 } -func (r *tarballReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { +func (r *tarballReference) newImageSource() (private.ImageSource, error) { // Pick up the layer comment from the configuration's history list, if one is set. comment := "imported from tarball" if len(r.config.History) > 0 && r.config.History[0].Comment != "" { From 511b8863ee79ceaeda876c7d0c3db9a560412e9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 1 Dec 2025 16:32:29 +0100 Subject: [PATCH 06/11] Add imagesource.NewImageSource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will make it a bit easier to use NewImageSourceOptions for transports that might only implement the public types.ImageReference. This commit does not (yet) add any callers, so this should not change behavior. Signed-off-by: Miloslav Trmač --- image/internal/imagesource/wrapper.go | 46 +++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/image/internal/imagesource/wrapper.go b/image/internal/imagesource/wrapper.go index 00bf8893fc..c44216df9d 100644 --- a/image/internal/imagesource/wrapper.go +++ b/image/internal/imagesource/wrapper.go @@ -4,12 +4,56 @@ import ( "context" "github.com/opencontainers/go-digest" + "github.com/sirupsen/logrus" "go.podman.io/image/v5/internal/imagesource/stubs" "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/internal/signature" + "go.podman.io/image/v5/transports" "go.podman.io/image/v5/types" ) +// NewImageSource is like ref.NewImageSourceWithOptions, but it works for +// references that only implement types.ImageReference, and returns an object that +// provides the private.ImageSource API. +// +// The caller must call .Close() on the returned ImageSource. +func NewImageSource(ctx context.Context, ref types.ImageReference, options private.NewImageSourceOptions) (private.ImageSource, error) { + // We don’t provide this as an imagereference.FromPublic wrapper for all of ImageReference, + // because that would depend on both imagesource.FromPublic and imagedestination.FromPublic; + // this way, some callers might only depend on one of them. + + if ref2, ok := ref.(private.ImageReference); ok { + return ref2.NewImageSourceWithOptions(ctx, options) + } + // We have no idea whether the implementation uses unwanted digests + // different from options.Digests.mustUse; if set, we can hope for the best + // or entirely refuse to use non-private implementations. + // + // If we refused entirely, that would break c/common/pkg/supplemented… while that one, + // hopefully, doesn’t actually need the digest options. + // + // Ultimately we might have to make NewImageSourceWithOptions a public API?? + // But that doesn't help all that much — if we exposed the whole options struct, + // how can we tell whether the external implementation actually handles any + // option we might add in the future?! + // + // Or (??!) would we want to parse data from GetManifest (and config from GetBlob??) + // to fail if it does not conform to options.Digest.MustUse? + // + // In practice, external wrappers tend to wrap docker:// and oci:…, where options.Digests + // don’t matter. + + if mustUse := options.Digests.MustUseSet(); mustUse != "" && mustUse != digest.Canonical { + logrus.Debugf("%q does not implement digest choices for image sources; request to use %s is ignored", + transports.ImageName(ref), mustUse) + } + src, err := ref.NewImageSource(ctx, options.Sys) + if err != nil { + return nil, err + } + return FromPublic(src), nil +} + // wrapped provides the private.ImageSource operations // for a source that only implements types.ImageSource type wrapped struct { @@ -20,6 +64,8 @@ type wrapped struct { // FromPublic(src) returns an object that provides the private.ImageSource API // +// Internal callers should use NewImageSource instead, where possible. +// // Eventually, we might want to expose this function, and methods of the returned object, // as a public API (or rather, a variant that does not include the already-superseded // methods of types.ImageSource, and has added more future-proofing), and more strongly From ffe7f2d490b360280f4044ee6e667f2d8acab323 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 1 Dec 2025 16:32:43 +0100 Subject: [PATCH 07/11] Forward NewImageSourceOptions in pkg/blobcache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/pkg/blobcache/src.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/image/pkg/blobcache/src.go b/image/pkg/blobcache/src.go index 8c30c2c8ce..f7900c7361 100644 --- a/image/pkg/blobcache/src.go +++ b/image/pkg/blobcache/src.go @@ -39,7 +39,7 @@ type blobCacheSource struct { // NewImageSourceWithOptions returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (b *BlobCache) NewImageSourceWithOptions(ctx context.Context, options private.NewImageSourceOptions) (private.ImageSource, error) { - src, err := b.reference.NewImageSource(ctx, options.Sys) + src, err := imagesource.NewImageSource(ctx, b.reference, options) if err != nil { return nil, fmt.Errorf("error creating new image source %q: %w", transports.ImageName(b.reference), err) } @@ -48,7 +48,7 @@ func (b *BlobCache) NewImageSourceWithOptions(ctx context.Context, options priva if options.Sys != nil { sys = *options.Sys } - s := &blobCacheSource{reference: b, source: imagesource.FromPublic(src), sys: sys} + s := &blobCacheSource{reference: b, source: src, sys: sys} s.Compat = impl.AddCompat(s) return s, nil } From cf17f953e257901ad05130761ee0cdef7562f3bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 1 Dec 2025 16:33:13 +0100 Subject: [PATCH 08/11] Use DigestOptions when creating ImageSource in c/image/copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/copy/copy.go | 6 ++++-- image/copy/single.go | 6 +++++- image/internal/image/sourced.go | 10 +++++++--- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/image/copy/copy.go b/image/copy/copy.go index cc165ff711..62f469d29e 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -248,11 +248,13 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, dest := imagedestination.FromPublic(publicDest) defer safeClose("dest", dest) - publicRawSource, err := srcRef.NewImageSource(ctx, options.SourceCtx) + rawSource, err := imagesource.NewImageSource(ctx, srcRef, private.NewImageSourceOptions{ + Sys: options.SourceCtx, + Digests: options.digestOptions, + }) if err != nil { return nil, fmt.Errorf("initializing source %s: %w", transports.ImageName(srcRef), err) } - rawSource := imagesource.FromPublic(publicRawSource) defer safeClose("src", rawSource) // If reportWriter is not a TTY (e.g., when piping to a file), do not diff --git a/image/copy/single.go b/image/copy/single.go index 5a5d5e3ddb..8a3f5230c1 100644 --- a/image/copy/single.go +++ b/image/copy/single.go @@ -19,6 +19,7 @@ import ( "github.com/vbauerster/mpb/v8" "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/internal/image" + "go.podman.io/image/v5/internal/imagesource" "go.podman.io/image/v5/internal/pkg/platform" "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/internal/set" @@ -379,7 +380,10 @@ func (ic *imageCopier) noPendingManifestUpdates() bool { // compareImageDestinationManifestEqual compares the source and destination image manifests (reading the manifest from the // (possibly remote) destination). If they are equal, it returns a full copySingleImageResult, nil otherwise. func (ic *imageCopier) compareImageDestinationManifestEqual(ctx context.Context, targetInstance *digest.Digest) (*copySingleImageResult, error) { - destImageSource, err := ic.c.dest.Reference().NewImageSource(ctx, ic.c.options.DestinationCtx) + destImageSource, err := imagesource.NewImageSource(ctx, ic.c.dest.Reference(), private.NewImageSourceOptions{ + Sys: ic.c.options.DestinationCtx, + Digests: ic.c.options.digestOptions, + }) if err != nil { logrus.Debugf("Unable to create destination image %s source: %v", ic.c.dest.Reference(), err) return nil, nil diff --git a/image/internal/image/sourced.go b/image/internal/image/sourced.go index ba2eaa0c9a..48ce7f7f44 100644 --- a/image/internal/image/sourced.go +++ b/image/internal/image/sourced.go @@ -15,8 +15,12 @@ import ( // // The caller must call .Close() on the returned ImageCloser. // -// NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource, -// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage instead of calling this function. +// NOTE: This exists only for conveniently implementing types.ImageReference.NewImage() +// in transports, and should not be called in any other new code: +// +// - If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource, +// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage instead of calling this function. +// - Also, this does not expose NewImageSourceOptions. (That could be changed, but most callers need a different structure anyway.) func FromReference(ctx context.Context, sys *types.SystemContext, ref types.ImageReference) (types.ImageCloser, error) { src, err := ref.NewImageSource(ctx, sys) if err != nil { @@ -52,7 +56,7 @@ type imageCloser struct { // NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource, // verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage instead of calling this function. // -// Most callers can use either FromUnparsedImage or FromReference instead. +// Internal callers should use FromUnparsedImage instead, except within an implementation of types.ImageReference.NewImage. // // This is publicly visible as c/image/image.FromSource. func FromSource(ctx context.Context, sys *types.SystemContext, src types.ImageSource) (types.ImageCloser, error) { From 8cb306e9d2d5d9fba6d6ee6a202d62a25cfc343b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 1 Dec 2025 16:39:23 +0100 Subject: [PATCH 09/11] Implement digest choice in tarball: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/tarball/tarball_reference.go | 2 +- image/tarball/tarball_src.go | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/image/tarball/tarball_reference.go b/image/tarball/tarball_reference.go index 493f9e17de..b6d7e2f636 100644 --- a/image/tarball/tarball_reference.go +++ b/image/tarball/tarball_reference.go @@ -73,7 +73,7 @@ func (r *tarballReference) NewImage(ctx context.Context, sys *types.SystemContex // NewImageSourceWithOptions returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (r *tarballReference) NewImageSourceWithOptions(ctx context.Context, options private.NewImageSourceOptions) (private.ImageSource, error) { - return r.newImageSource() + return r.newImageSource(options) } // NewImageSource returns a types.ImageSource for this reference. diff --git a/image/tarball/tarball_src.go b/image/tarball/tarball_src.go index a3bd0f3b38..fac0d08aec 100644 --- a/image/tarball/tarball_src.go +++ b/image/tarball/tarball_src.go @@ -15,6 +15,7 @@ import ( digest "github.com/opencontainers/go-digest" imgspecs "github.com/opencontainers/image-spec/specs-go" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/imagesource/impl" "go.podman.io/image/v5/internal/imagesource/stubs" "go.podman.io/image/v5/internal/private" @@ -42,7 +43,7 @@ type tarballBlob struct { size int64 } -func (r *tarballReference) newImageSource() (private.ImageSource, error) { +func (r *tarballReference) newImageSource(options private.NewImageSourceOptions) (private.ImageSource, error) { // Pick up the layer comment from the configuration's history list, if one is set. comment := "imported from tarball" if len(r.config.History) > 0 && r.config.History[0].Comment != "" { @@ -50,6 +51,11 @@ func (r *tarballReference) newImageSource() (private.ImageSource, error) { } // Gather up the digests, sizes, and history information for all of the files. + digestAlgorithm, err := options.Digests.Choose(digests.Situation{}) + if err != nil { + return nil, err + } + blobs := map[digest.Digest]tarballBlob{} diffIDs := []digest.Digest{} created := time.Time{} @@ -85,7 +91,7 @@ func (r *tarballReference) newImageSource() (private.ImageSource, error) { } // Set up to digest the file as it is. - blobIDdigester := digest.Canonical.Digester() + blobIDdigester := digestAlgorithm.Digester() reader = io.TeeReader(reader, blobIDdigester.Hash()) var layerType string @@ -103,7 +109,7 @@ func (r *tarballReference) newImageSource() (private.ImageSource, error) { } defer uncompressed.Close() // It is compressed, so the diffID is the digest of the uncompressed version - diffIDdigester = digest.Canonical.Digester() + diffIDdigester = digestAlgorithm.Digester() reader = io.TeeReader(uncompressed, diffIDdigester.Hash()) switch format.Name() { case compressionTypes.GzipAlgorithmName: @@ -172,7 +178,7 @@ func (r *tarballReference) newImageSource() (private.ImageSource, error) { if err != nil { return nil, fmt.Errorf("error generating configuration blob for %q: %w", strings.Join(r.filenames, separator), err) } - configID := digest.Canonical.FromBytes(configBytes) + configID := digestAlgorithm.FromBytes(configBytes) blobs[configID] = tarballBlob{ contents: configBytes, size: int64(len(configBytes)), From 25de275ddc7f2005659f61436a4fbaa14e39708a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 1 Dec 2025 16:45:28 +0100 Subject: [PATCH 10/11] Implement digest choice in sif: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/sif/src.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/image/sif/src.go b/image/sif/src.go index 60ae3f63b5..7ba466c598 100644 --- a/image/sif/src.go +++ b/image/sif/src.go @@ -14,6 +14,7 @@ import ( imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" "github.com/sylabs/sif/v2/pkg/sif" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/imagesource/impl" "go.podman.io/image/v5/internal/imagesource/stubs" "go.podman.io/image/v5/internal/private" @@ -39,7 +40,7 @@ type sifImageSource struct { } // getBlobInfo returns the digest, and size of the provided file. -func getBlobInfo(path string) (digest.Digest, int64, error) { +func getBlobInfo(path string, digestAlgorithm digest.Algorithm) (digest.Digest, int64, error) { f, err := os.Open(path) if err != nil { return "", -1, fmt.Errorf("opening %q for reading: %w", path, err) @@ -50,7 +51,7 @@ func getBlobInfo(path string) (digest.Digest, int64, error) { // it here again, stream the tar file to a pipe and // compute the digest while writing it to disk. logrus.Debugf("Computing a digest of the SIF conversion output...") - digester := digest.Canonical.Digester() + digester := digestAlgorithm.Digester() // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). size, err := io.Copy(digester.Hash(), f) if err != nil { @@ -65,6 +66,11 @@ func getBlobInfo(path string) (digest.Digest, int64, error) { // newImageSource returns an ImageSource for reading from an existing directory. // newImageSource extracts SIF objects and saves them in a temp directory. func newImageSource(ctx context.Context, ref sifReference, options private.NewImageSourceOptions) (private.ImageSource, error) { + digestAlgorithm, err := options.Digests.Choose(digests.Situation{}) + if err != nil { + return nil, err + } + sifImg, err := sif.LoadContainerFromPath(ref.file, sif.OptLoadWithFlag(os.O_RDONLY)) if err != nil { return nil, fmt.Errorf("loading SIF file: %w", err) @@ -89,7 +95,7 @@ func newImageSource(ctx context.Context, ref sifReference, options private.NewIm return nil, fmt.Errorf("converting rootfs from SquashFS to Tarball: %w", err) } - layerDigest, layerSize, err := getBlobInfo(layerPath) + layerDigest, layerSize, err := getBlobInfo(layerPath, digestAlgorithm) if err != nil { return nil, fmt.Errorf("gathering blob information: %w", err) } @@ -125,7 +131,7 @@ func newImageSource(ctx context.Context, ref sifReference, options private.NewIm if err != nil { return nil, fmt.Errorf("generating configuration blob for %q: %w", ref.resolvedFile, err) } - configDigest := digest.Canonical.FromBytes(configBytes) + configDigest := digestAlgorithm.FromBytes(configBytes) manifest := imgspecv1.Manifest{ Versioned: imgspecs.Versioned{SchemaVersion: 2}, From 05a8d27e9c2299406dc6dc83e3adcbd7539e3faf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Sat, 6 Dec 2025 01:43:10 +0100 Subject: [PATCH 11/11] Use options.Digest for the config digest in docker-archive: and docker-daemon: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note that we DO NOT use options.Digest for the layer IDs; that would require extra CPU cost to digest files more than once. Signed-off-by: Miloslav Trmač --- image/docker/archive/src.go | 2 +- image/docker/daemon/daemon_src.go | 2 +- image/docker/internal/tarfile/src.go | 41 ++++++++++++++++++----- image/docker/internal/tarfile/src_test.go | 6 +++- image/docker/tarfile/src.go | 12 +++++-- 5 files changed, 49 insertions(+), 14 deletions(-) diff --git a/image/docker/archive/src.go b/image/docker/archive/src.go index 37deb72d24..c49a66bcfb 100644 --- a/image/docker/archive/src.go +++ b/image/docker/archive/src.go @@ -27,7 +27,7 @@ func newImageSource(ref archiveReference, options private.NewImageSourceOptions) archive = a closeArchive = true } - src := tarfile.NewSource(archive, closeArchive, ref.Transport().Name(), ref.ref, ref.sourceIndex) + src := tarfile.NewSource(archive, closeArchive, ref.Transport().Name(), ref.ref, ref.sourceIndex, options) return &archiveImageSource{ Source: src, ref: ref, diff --git a/image/docker/daemon/daemon_src.go b/image/docker/daemon/daemon_src.go index aac112b9d3..c71ddab1bf 100644 --- a/image/docker/daemon/daemon_src.go +++ b/image/docker/daemon/daemon_src.go @@ -42,7 +42,7 @@ func newImageSource(ctx context.Context, ref daemonReference, options private.Ne if err != nil { return nil, err } - src := tarfile.NewSource(archive, true, ref.Transport().Name(), nil, -1) + src := tarfile.NewSource(archive, true, ref.Transport().Name(), nil, -1, options) return &daemonImageSource{ ref: ref, Source: src, diff --git a/image/docker/internal/tarfile/src.go b/image/docker/internal/tarfile/src.go index 56421afa13..8cd17d7e0b 100644 --- a/image/docker/internal/tarfile/src.go +++ b/image/docker/internal/tarfile/src.go @@ -14,9 +14,11 @@ import ( digest "github.com/opencontainers/go-digest" "go.podman.io/image/v5/docker/reference" + "go.podman.io/image/v5/internal/digests" "go.podman.io/image/v5/internal/imagesource/impl" "go.podman.io/image/v5/internal/imagesource/stubs" "go.podman.io/image/v5/internal/iolimits" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/manifest" "go.podman.io/image/v5/pkg/compression" "go.podman.io/image/v5/types" @@ -33,8 +35,9 @@ type Source struct { archive *Reader closeArchive bool // .Close() the archive when the source is closed. // If ref is nil and sourceIndex is -1, indicates the only image in the archive. - ref reference.NamedTagged // May be nil - sourceIndex int // May be -1 + ref reference.NamedTagged // May be nil + sourceIndex int // May be -1 + digestOptions digests.Options // The following data is only available after ensureCachedDataIsPresent() succeeds tarManifest *ManifestItem // nil if not available yet. configBytes []byte @@ -55,17 +58,18 @@ type layerInfo struct { // NewSource returns a tarfile.Source for an image in the specified archive matching ref // and sourceIndex (or the only image if they are (nil, -1)). // The archive will be closed if closeArchive -func NewSource(archive *Reader, closeArchive bool, transportName string, ref reference.NamedTagged, sourceIndex int) *Source { +func NewSource(archive *Reader, closeArchive bool, transportName string, ref reference.NamedTagged, sourceIndex int, options private.NewImageSourceOptions) *Source { s := &Source{ PropertyMethodsInitialize: impl.PropertyMethods(impl.Properties{ HasThreadSafeGetBlob: true, }), NoGetBlobAtInitialize: stubs.NoGetBlobAtRaw(transportName), - archive: archive, - closeArchive: closeArchive, - ref: ref, - sourceIndex: sourceIndex, + archive: archive, + closeArchive: closeArchive, + ref: ref, + sourceIndex: sourceIndex, + digestOptions: options.Digests, // We use options.Digests for configDigest, but not for layers; see a more detailed comment in prepareLayerData. } s.Compat = impl.AddCompat(s) return s @@ -105,11 +109,15 @@ func (s *Source) ensureCachedDataIsPresentPrivate() error { if err != nil { return err } + digestAlgorithm, err := s.digestOptions.Choose(digests.Situation{}) + if err != nil { + return err + } // Success; commit. s.tarManifest = tarManifest s.configBytes = configBytes - s.configDigest = digest.FromBytes(configBytes) + s.configDigest = digestAlgorithm.FromBytes(configBytes) s.orderedDiffIDList = parsedConfig.RootFS.DiffIDs s.knownLayers = knownLayers return nil @@ -130,6 +138,21 @@ func (s *Source) TarManifest() []ManifestItem { func (s *Source) prepareLayerData(tarManifest *ManifestItem, parsedConfig *manifest.Schema2Image) (map[digest.Digest]*layerInfo, error) { // Collect layer data available in manifest and config. + + // We use the DiffID values from parsedConfig as layer identifiers in the manifest we generate, + // regardless of s.digestOptions. + // + // We _could_ use s.digestOptions, by consuming only the paths in tarManifest.Layers, ignoring parsedConfig.RootFS.DiffIDs, + // but we would have to compute digests of all layers, an extra CPU cost (and possibly extra I/O cost because, later in this function, + // we don’t read the full layer contents for uncompressed layers). + // + // That would, potentially _introduce_ new risk of digest collisions (if the layers collided under our chosen digest algorithm); + // meanwhile, _assuming_ that the image producer identified layers by the DiffID values written into the produced parsedConfig, + // the process would _still_ be exposed to risk of digest collisions in the producer-chosen algorithm. + // + // Us spending the CPU time to digest all layers would only make a difference if the producer could produce an image + // where layers collide under the producers’ parsedConfig.RootFS.DiffIDs algorithm, _but_ they are otherwise correctly + // differentiated by the producer, and use different tarManifest.Layers paths. if len(tarManifest.Layers) != len(parsedConfig.RootFS.DiffIDs) { return nil, fmt.Errorf("Inconsistent layer count: %d in manifest, %d in config", len(tarManifest.Layers), len(parsedConfig.RootFS.DiffIDs)) } @@ -272,7 +295,7 @@ func (s *Source) GetBlob(ctx context.Context, info types.BlobInfo, cache types.B return nil, 0, err } - if info.Digest == s.configDigest { // FIXME? Implement a more general algorithm matching instead of assuming sha256. + if info.Digest == s.configDigest { return io.NopCloser(bytes.NewReader(s.configBytes)), int64(len(s.configBytes)), nil } diff --git a/image/docker/internal/tarfile/src_test.go b/image/docker/internal/tarfile/src_test.go index 4958037950..009bba17e5 100644 --- a/image/docker/internal/tarfile/src_test.go +++ b/image/docker/internal/tarfile/src_test.go @@ -9,6 +9,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.podman.io/image/v5/internal/digests" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/manifest" "go.podman.io/image/v5/pkg/blobinfocache/memory" "go.podman.io/image/v5/types" @@ -47,7 +49,9 @@ func TestSourcePrepareLayerData(t *testing.T) { reader, err := NewReaderFromStream(nil, &tarfileBuffer) require.NoError(t, err, c.config) - src := NewSource(reader, true, "transport name", nil, -1) + src := NewSource(reader, true, "transport name", nil, -1, private.NewImageSourceOptions{ + Digests: digests.CanonicalDefault(), + }) require.NoError(t, err, c.config) defer src.Close() configStream, _, err := src.GetBlob(ctx, types.BlobInfo{ diff --git a/image/docker/tarfile/src.go b/image/docker/tarfile/src.go index c9d75c07b5..f1a66b9e21 100644 --- a/image/docker/tarfile/src.go +++ b/image/docker/tarfile/src.go @@ -6,6 +6,8 @@ import ( digest "github.com/opencontainers/go-digest" internal "go.podman.io/image/v5/docker/internal/tarfile" + "go.podman.io/image/v5/internal/digests" + "go.podman.io/image/v5/internal/private" "go.podman.io/image/v5/types" ) @@ -28,7 +30,10 @@ func NewSourceFromFileWithContext(sys *types.SystemContext, path string) (*Sourc if err != nil { return nil, err } - src := internal.NewSource(archive, true, "[An external docker/tarfile caller]", nil, -1) + src := internal.NewSource(archive, true, "[An external docker/tarfile caller]", nil, -1, private.NewImageSourceOptions{ + Sys: sys, + Digests: digests.CanonicalDefault(), + }) return &Source{internal: src}, nil } @@ -49,7 +54,10 @@ func NewSourceFromStreamWithSystemContext(sys *types.SystemContext, inputStream if err != nil { return nil, err } - src := internal.NewSource(archive, true, "[An external docker/tarfile caller]", nil, -1) + src := internal.NewSource(archive, true, "[An external docker/tarfile caller]", nil, -1, private.NewImageSourceOptions{ + Sys: sys, + Digests: digests.CanonicalDefault(), + }) return &Source{internal: src}, nil }