... and acknowledge that various tests are strictly speaking
invalid, to reinforce that real callers must not pass nil.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Extend the blob info cache to also cache the name of the type of
compression used on a blob that we've seen, or specific values that
indicate that we know the blob was not compressed, or that we don't
know whether or not it was compressed.
New methods for adding known blob-compression pairs and reading
candidate locations including compression information are part of a new
internal BlobInfoCache2 interface which the library's BlobInfoCache
implementors also implement.
When we copy a blob, try to record the state of compression for the
source blob, and if we applied any changes, the blob we produced.
Make sure that when TryReusingBlob successfully uses a blob from the
blob info cache, that it provides compression information in the
BlobInfo that it returns, so that manifests can be updated to describe
layers using the correct MIME types.
When attempting to write a manifest, if a manifest can't be written
because layers were compressed using an algorithm which can't be
expressed using that manifest type, continue on to trying other manifest
formats.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Restrict the sizes of blobs which are copied into memory such as the
manifest, the config, signatures, etc. This will protect consumers of
c/image from rogue or hijacked registries that return too big blobs in
hope to cause an OOM DOS attack.
Note that error message should be improved in a future change to make
sure that it's clear in which code path we hit a limit.
Fixes: CVE-2020-1702
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1792796
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
This has arguably been implied (OTOH, also arguably, it's a breaking change),
make it explicit.
This does not yet implement the semantics.
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>
Add a HasThreadSafePutBlob() method to the ImageDestination interface
and all its implementations to indicate whether the corresponding
PutBlob() method can be executed concurrently. This is a first step to
enable parallel image copying. By default, all transports are not
thread-safe and must be carefully migrated in later changes.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Add a HasThreadSafeGetBlob() method to the ImageSource interface and all
its implementations to indicate whether the corresponding GetBlob()
method can be executed concurrently. This is a first step to enable
parallel image copying. By default, all transports are not thread-safe
and must be carefully migrated in later changes.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
This will allow TryReusingBlob to substitute the required blob with an equivalent
based on cache information if possible, or not doing so if it would break signatures.
In addition, we set canSubstitute to false when signing an image, to make 100% sure
the signed blob is safe (e.g. that we are not signing a third-party-compressed version
which has been maliciously designed to attack some decompressor implementations).
Nothing implements this, so does not change behavior yet.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
For now, none of the transports actually use it, so should not change behavior.
copy.Image uses its existing cache object; config parses in image/* usually
use NoCache because they wouldn't appreciably benefit anyway.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This will, primarily, make it easier for the transport to use
alternate locations without having to somehow carry state from
HasBlob to ReapplyBlob.
Also, all instances of ReapplyBlob have been either trivial or redundant,
and there is a single primary caller of HasBlob/ReapplyBlob (+ a few
in-transport users who are even a bit cleaner with the move to
TryReusingBlob).
Should not change behavior, apart from not doing the
storageImageDestination.HasBlob check redundantly in
storageImageDestination.ReapplyBlob.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
There have been multiple build failures found when running `go test`
through the git repo. Fix them to avoid test failures.
Signed-off-by: Dongsu Park <dongsu@kinvolk.io>
e.g. 504 (Gateway timeout) instead of just "504".
Note that for unknown status codes this will output "99999 ()"; this
seems not worth worrying about and building a helper for right now.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This allows a destination to opt out of updating the embedded name:tag in schema1
manifests without violating the ImageDestination.Reference / ImageReference API
expectations.
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>
Add an LayerInfosForCopy() method to source images which gives them a
chance to provide updated values for the blobsums contained in the
image's manifest, if they want to. Returning `nil` implies that they
have no changes to suggest compared to what's in the manifest.
When copying an image, if we can update the manifest with those new
values during copying, do so. If we have new values, but we can't
update the manifest, copying fails.
Update storageImageSource to return its manifest and reference in
unmodified form, and supply updated blob digests via LayerInfosForCopy()
so that copying images from storage works.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
This mirrors GetManifest, and allows / requires signatures to be per-instance.
Also add implementations in docker: and atomic:, the only transports which
support both manifest lists and signatures.
Does not change behavior yet, the only user always specifies nil
instanceDigest.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This will make the code paths more uniform for consumers of the
primary manifest and the manifest instances.
(Having an explicit support for manifest instances is necessary
for transports like docker-daemon: / oci-archive:, which
contain several images but setting up an ImageSource is very
expensive, or which don't even allow referencing images by digest.)
This is a direct replacement of GetTargetManifest, and should
not change behavior; notably the OCI implementation is still
blindly guessing the manifest type although it is probably
available in the index.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
The requestedManifestMIMETypes parameter was added because a destination
might not support all manifest MIME types that the the source supports,
but the original use case now passes all manifest types and lets
containers/image convert internally. In generally, internal conversion
may be more comprehensive, is more predictable, and avoids bypassing
internal checks.
Fixes: #331
Signed-off-by: Owen W. Taylor <otaylor@fishsoup.net>
Instead, just forward SupportedManifestMIMETypes to the underlying
docker: transport. Now that we can do autodetection, it should be safe
to declare schema2 (or, in the future, OCI) support.
One notable difference is that SupportedManifestMIMETypes affects what
we configure the source to accept: e.g. when streaming from docker: to
s1-only atomic:, previously the conversion was done by the docker:
registry server, but now the source registry gives us schema2 and we do
the conversion, changing the manifest digest. Right now that is a
difference, but soon(?) we will update the manifest to use the
destination’s name/tag, and then the manifest digest will change in any
case.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This will be used to trigger the fallback to other manifest schemas;
for now this only defines a type and updates comments.
Does not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
The current interface can return (haveBlob, _, err):
- (true, nil): blob exists
- (false, ErrBlobNotFound): blob does not exist or unknown
- (false, other non-nil): unexpected error
i.e. (haveBlob) is equivalent to (err == nil), which is both redundant
and makes the interface unnecessarily difficult to use for the typical
case where the caller wants to abort on an unexpected error, and then
decide based on haveBlob:
if err != nil && err != types.ErrBlobNotFound {
return err
}
if haveBlob { … }
Insted, define the interface to be
- (true, nil): blob exists
- (false, nil): blob does not exist or unknown
- (false, non-nil): unexpected error
which simplifies the “abort” conditional to a simple (err != nil).
A possible alternative would be to eliminate the haveBlob bool instead,
but that still requires two-clause checks for aborting, and semantically
a blob not being present is not an error, it is a succesfully obtained
return value.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This allows us to provide in the image interfaces a method of providing
an error at close time. This is only currently used in a few situations.
Signed-off-by: Erik Hollensbe <github@hollensbe.org>
This replaces the copy of github.com/docker/docker/reference in the same
place, which we have just gotten rid of, and allows using this package
even in consumers which insist on an incompatible version of
docker/distribution.
The copy has been edited to drop a reference to
github.com/docker/distribution/digestset .
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Instead, call NamedTagged.Tag in all users.
XNamedTagged is now equivalent to distreference.NamedTagged.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This is an intermediate step which will eventually go away.
The goal of this PR is to get rid of c/i/docker/daemon/reference and to
replace uses of it by direct calls to docker/distribution/reference.
We can't do that safely and easily, because the two have different
semantics for reference.Named.Name() and reference.Named.String(): we
return a minimized version, e.g. "busybox", upstream returns an expanded
version, e.g. "docker.io/library/busybox".
BEFORE this commit the difference is hidden by using
docker/distribution/reference.WithName, which allows using the minimized
version, and works with it correctly; but because we want to use the
upstream canonicalization code, which will change semantics, we can't
just mix and match.
To make the distinction explicit, this commmit adds an X to ALL public
names from c/i/docker/daemon/reference. E.g. a reference.XNamed type,
which has methods XName and XString.
This is pretty large, but does not change behavior at all. By
inspection it is clear to see that reference.XNamed and subtypes does
not expose any of the non-X, conflicting, method names.
Using e.g.
> git diff --word-diff-regex=.|grep -F '{+'|grep -v '^\([^{]\|{+X+}\)*{\?$'
it is possible to see that most lines in this diff only add a single X
letter, and manually inspect the few lines which don't match the regexp.
The only REALLY new code is an explicit definition of namedRef.XName()
and namedRef.XString(), and two newly added casts to namedRef in cases
where we need to use the underlying distreference.Reference within
a reference.XNamed value. Strictly speaking these changes change
behavior, in that third-party implementations of reference.XNamed are no
longer accepted; but we broke them by renaming at all.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Add HasBlob() and ReapplyBlob() methods to the ImageDestination
interface, for checking if a blob has already been Put() to a
destination, and allowing the destination implementation to offer the
caller a chance to avoid having to re-Get() the source blob.
This means that copyLayers() no longer keeps track of which blobs it's
copied and skips any -- it handes that down to copyLayer(). The
copyLayer() function is now the lone method of an object that caches the
diffIDs of blobs that it's copied, and skips copying a layer if the
destination blob is already present (HasBlob() says "yes") AND the
blob's diffID is already known.
The "directory" and "oci" destinations implement HasBlob() as a stat()
check for the file they'd create, and make ReapplyBlob() a no-op.
The "docker/daemon" destination now caches the digests of blobs that
it's already added to the output tarball, and checks that in HasBlob(),
making ReapplyBlob() also a no-op.
The "docker"/"openshift" destination HasBlob() checks if the destination
blob is present using an HTTP HEAD request, as PutBlob() does, making
its ReapplyBlob() also a no-op.
Lastly, the "storage" destination HasBlob() checks the image's blobs
list; ReapplyBlob() generates a diff of the most recent layer that
resulted from applying a blob with the specified digest and hands it off
to PutBlob().
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
https://github.com/containers/image/pull/139 has changed the semantics
to always expect a value from an ImageSource; that’s not really great
when the source has no idea, but we don’t have optionals in Golang to
distinguish between a nonsensical "" and unavailable, so this is at
least making the documentation consistent.
Hopefully ImageSources which call manifest.GuessMIMEType in GetManifest
will not proliferate…
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Instead of manipulating an ImageStreamMapping and having to do exactly
what OpenShift does, just use the Docker Registry API and let the server
side deal with that.
This fixes “unexpected end of JSON input” when uploading schema2
manifests (because we didn’t also include the config object), and should
resolve https://github.com/projectatomic/skopeo/issues/208 (by using the
existing code on the server side)
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Not pushing to a tag makes the image impossible to pull with a tag.
This is not really correct because we keep the Name and Tag in schema1
manifests unmodified, and older registry versions (in particular
OpenShift 1.1) do inspect those fields. But, it is better than nothing,
and for OpenShift manifest uploads via the Docker API
( https://github.com/containers/image/pull/95 )a tag is required.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This allows generic code to differentiate between transports where
bandwidth and storage costs are at premium (usually remote
repositories), and where memory/mmap and CPU time are more important
(usually local, especially temporary, storage) [or the dir: case
where we don’t want to modify the input at all if possible].
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This does not change behavior. Having a single struct for carrying
the (digest, size) pair around will make manipulating layers much
easier in the future.
... except that dockerImageDestination now logs the uploaded layer using
the realized digest, instead of the input digest = an empty string if the digest is
unknown.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Miloslav Trmač