We are seeing
> reference "[overlay@...]quay.io/...@sha256:..." does not resolve to an image ID: identifier is not an image
which is misleading; hide the ErrNoSuchImage text.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Some code was not using it while it should (this allows a caller to
better inspect the error, if a need arises).
Note in a single case where we have two errors, we only make the
"primary" one unwrappable (by using %w), and explicitly convert the
"secondary" one to a string (which is a way to tell a code reviewer
and a linter that we don't want it to be unwrappable).
Found by errorlint linter.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
- Document the error returned if image is not found
- Also document it for the internal resolveImage, to record
the commitment
- Update the deprecation comments to warn about the error
return value change
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
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č <mitr@redhat.com>
Usually by removing them, sometimes by actually using an already-available value.
golangci-lint linter: unparam
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
`github.com/pkg/errors` is deprecated since quite some time so we now
use the native error wrapping for more idiomatic golang.
Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
This will allow imports of errors and pkg/errors to
coexist in a single package, avoid adding accidental
new uses via errors.New(), make any potential new
additions more visible in reviews.
Files that can import errors only (typically
because they only use errors.New) have been updated to
that import instead of renaming.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Go 1.17 introduces a much more reasonable build constraint format,
and gofmt now fails without using it.
Sadly we still need the old format _as well_, to support <1.17 builds.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Podman and other tools already add Error: to the front of returned error
message, and this ends up as a stutter.
podman pull fedora.io/fred
Trying to pull fedora.io/fred:latest...
Error: Error initializing image from source docker://fedora.io/fred:latest: invalid character '<' looking for beginning of value
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
- If the digest references a single-platform manifest,
all matching images will have the same manifest, same config,
and same behavior of the dummy index created by
imageMatchesSystemContext ; so don't build that dummy index
and rename the function to multiArchImageMatchesSystemContext .
- If the digest references a multi-platform list, just check
whether ChooseInstance would chose the current image;
don't check _both_ ChooseInstance based on the multi-platform
list and config values through the dummy index.
The original pull did both (ChooseInstance to choose
a per-platform image, and
copy.checkImageDestinationForCurrentRuntime(), so they
are expected to match (and we don't do a config check
vs. the current SystemContext to refuse handling
single-platform images in c/storage, in general).
- Even more verbosely document the situation,
explaining how we can end up with multiple images that
match the same digest, and what we want to achieve.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Add the manifest.List interface, and implementations for OCIv1 Index and
Docker Schema2List documents.
Add an instanceDigest parameter to PutManifest(), PutSignatures(), and
LayerInfosForCopy, for symmetry with GetManifest() and GetSignatures().
Return an error if the instanceDigest is supplied to destinations which
don't support them, and add stubs that do so even to the transports
which would support it, so that we don't break compilation here.
Add a MultipleImages flag to copy.Options, and if the source for a copy
operation contains multiple images, copy all of the images if we can.
If we can't copy them all, but we were told to, return an error.
Use the generic manifest list API to select a single image to copy from
a list, so that we aren't just limited to the Docker manifest list
format for those cases.
When guessing at the type of a manifest, if the manifest contains a list
of manifests, use its declared MIME type if it included one, else assume
it's an OCI index, because an OCI index doesn't include its MIME type.
When copying, switch from using an encode-then-compare of the original
and updated versions of the list to checking if the instance list was
changed (one of the things we might have changed) or if its type has
changed due to conversion (the other change we might have made). If
neither has changed, then we don't need to change the encoded value of
the manifest.
When copying, when checking for a digest mismatch in a target image
reference, ignore a mismatch between the digest in the reference and the
digest of the main manifest if we're copying one element from a list,
and the digest in the reference matches the digest of the manifest list.
When copying, if conversion of manifests for single images is being
forced, convert manifest lists to the corresponding list types.
When copying, supply the unparsed top level to Commit() by attaching the
value to the context.Context.
Support manifest lists in the directory transport by using the instance
digest as a prefix of the filename used to store a manifest or a piece
of signature data.
Support manifest lists in the oci-layout transport by accepting indexes
as we do images, and stop guessing about Platform values to add to the
top-level index.
Support storing manifest lists to registries in the docker: transport by
using the manifest digest when we're writing one image as part of
pushing a list of them, and by using the instance digest when reading or
writing signature data, when one is specified, or the cached digest of
the non-instanced digest when one is not specified.
Add partial support for manifest lists to the storage transport: when
committing one image from a list into storage, also add a copy of the
manifest list by extracting it from the context.Context. The logic is
already in place to enable locating an image using any of multiple
manifest digests.
When writing an image that has an instanceDigest value (meaning it's a
secondary image), don't try to generate a canonical reference to add to
the image's list of names if the reference for the primary image doesn't
contain a name. That should only happen if we're writing using just an
image ID, which is unlikely, but we still need to handle it.
Avoid computing the digest of the manifest, or retrieving the
either-a-tag-or-a-digest value from the target reference, if we're given
an instanceDigest, which would override them anyway.
Move the check for non-nil instanceDigest values up into the main
PutSignatures() method instead of duplicating it in the per-strategy
helpers.
Add mention of the instanceDigest parameter and its use to various
PutManifest, PutSignatures, and LayerInfosForCopy implementations and
their declarations in interfaces.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
... so that major-version-aware Go module import
(as opposed to vX.Y.Z+incompatible, which does not allow different
packages to use different versions) works right.
Also requires adding some more GO111MODULE=on options to Makefile.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
If the reference that we use to locate an image includes a digest, set
the returned Image's Digest field to match it.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
If we find an image and determine the ID, it seems pointless to load it again by ID
immediately afterwards.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Instead of three cases, each handling a subset of fields, construct
the return value in steps, making it clear how the fields fit together.
(The code could, in theory, return just a [storeSpec] with no name nor ID;
but newReference has just been edited to refuse to create such values.)
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
The code assumes that anyway, and this allows us to move an unreachable
check from ParseStoreReference into a testable one in newReference.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Now that the implementation transition is complete, the value being
"complete" is no longer a differentiating factor; instead, use a shorter,
easier to use name.
(We could also use just "ref", but there are several kinds of references
around, this suggests more about the type.)
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Now that all data for DockerReference comes from completeReference,
just return it directly, and eliminate the weird theoretical
failure paths where we can silently return incorrect data.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
They are mostly used only for boolean presence checking; only DockerReference
actually uses the value; in that case the transition to completeReference
may add a digest value, but a few lines below we do a, now redundant,
WithDigest computation which would have added the same digest anyway.
NOTE: This changes PolicyConfigurationIdentity and PolicyConfigurationNamespaces,
which now return values even if breakDockerReference. That seems to be the right
thing to do anyway.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
name:tag@digest is actually a valid input to reference.ParseNormalizedNamed;
the code used to set name:tag in s.name, but leave s.tag empty.
That happened to mostly work correctly, but for the wrong reasons. So, fix
setting s.tag for name:tag@digest inputs, and then update DockerReference
to handle such cases.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This starts a series of commits reworking storageReference to rely more on
reference.Named than on a set of independent string members.
First, introduce completeReference: this is a reference.Named, like
storageReference.name, but it includes both the tag and the digest, possibly
at the same time. For now, the field is only set, never used; users will be
ported one aspect at a time in the following commits; ultimately it will be
renamed to something shorter after we eliminate storageReference.name as well.
Also, to preserve the ImageDestination.DockerReference() ... interesting
behavior ..., add an explicit breakDockerReference boolean. This will allow
us to rely on completeDockerReference being always available, while keeping the
existing behavior unchanged until we can improve upon it.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Extract an imageMatchesRepo helper from resolveImage. This avoids
duplicating it, and a separate helper with "return" is easier to follow
than labeled breaks.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This should produce equivalent results, but Name() uses the fully-qualified
format (i.e. is safer against normalization changes), while FamiliarName is
primarily intended for UI output, and also more costly to compute.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... if PolicyConfigurationIdentity includes @id. At least the :tag form is clearly useful.
This does not handle the name:tag@digest case (which _can_ happen), where the tag is
currently recorded inside s.name but not s.tag; the possible code handling it would
be very non-obviously pointing out this difference. For now, only leave a FIXME in the test.
Maybe it should be handled by refusing such input instead; e.g. that's what
docker.Transport does.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
- Network IO paths should react to cancels now.
- File IO paths generally still won't.
- `SystemContext` objects have been renamed to `sys` to leave `ctx`
available for the stdlib context objects.
Signed-off-by: Mike Lundy <mike@fluffypenguin.org>
Log the message about a given reference not being resolvable to an image
ID at the debug level rather than as an error, and wrap the error that
we return in case the caller wants to relay the reference's value back
as part of the error.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When attempting to locate an image, if we have a name+digest (i.e.,
canonical reference), use the store's new ImagesByDigest() method to
search for images that match a given digest, and which have at least one
name that matches the specified name, treating the reference as an
implicit name so long as it has an explicit name that matches (give or
take a tag).
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Do not construct storageImageSource around storageImage around
storageUnnamedImageSource; instead, use the ~simple storageUnnamedImageSource
implementations, except using manifest.Manifest to edit layers in GetManifest.
Revert storageImage to only implementing .Size()
Still modifies the input reference, which breaks assumptions of signature validation.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Miloslav Trmač