... to prevent panics if the value does not contain a :, or other unexpected
values (e.g. a path traversal).
Don't bother on paths where we computed the digest ourselves, or it is already trusted
for other reasons.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... and identify them using UncompressedDigest, not TOCDigest
On pushes, also use the trusted UncompressedDigest if available
instead of preferring the untrusted value when a TOC digest
is present.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... so that the fields of storageImageSource read in
approximately natural order.
Only moves unchanged code, should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This way we:
- have a single type, guaranteeing the source and destination don't
get out of sync
- separate the JSON-encoded data, without having to worry about
Marshal/Unmarshal affecting unrelated fields of the source/destination.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
when copying a partial image, store the expected diffID so that it can
be later used to validate the obtained layer stream.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
If you hit Ctr-C while pulling an image files and directories get
left in /var/tmp. By adding "containers_images" prefix, we can use
systemd tmpfiles handling to remove them on reboot safely.
Help to make https://github.com/containers/podman/pull/19201 safer.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
It's completely undefined whether the OCI blob annotations apply to
the object as a concept, regardless of representation, or to the specific
binary representation. So it's unclear whether we should preserve or drop them
when compressing/decompressing/substituting blobs.
In particular, we currently don't truly correctly handle the zstd:chunked
annotations on:
- decompression (should be dropped)
- recompression (should be dropped)
- substitution (should be replaced by data about the other blob, if any; we don't record that)
Right now, we drop all annotations on decompression and recompression (which
happens to work fine), and preserve annotations on substitution (which is technically
invalid).
Luckily, the zstd:chunked use is opportunistic, and if the annotations are invalid
or not applicable, the manifest checksum fails, and we fall through to an ordinary pull;
so, that is not quite a deal breaker.
So, for now, just add FIXMEs recording the pain points.
To fix this truly correctly, we would need:
- a new metadataCleaner field in pkg/compression/internal.Algorithm
- a new pkg/compression.CleanMetadata
- turning public manifest.Manifest into internal/manifest.Manifest where we can add methods
- adding internal/manifest.Manifest.LayerInfosWithCompression that turns MIME types into compression.Algorithm values
- (using that in copy.copyLayer instead of the current hard-coded switch)
- then either defining a new alternative to UpdatedImage that can handle these annotations naturally,
or all the marked users that need to clean the annotations themselves calling LayerInfosWithCompression
and CleanMetadata on the affected blobs.
- recording the zstd annotations in the blob info cache
- reading those annotations when substituting blobs based on the cache
We should do all that long-term, but that's quite a lot of work to fix a metadata inconsistency
which we can currently silently, with moderate cost, hide from the user.
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>
All we need in this case is an anonymous temporary file, so
unlink the file immediately after creating it. This avoids
leaking space if the caller forgets to call `Close()` (and the
process exits).
This would have mitigated https://github.com/containers/skopeo/pull/1837
which was a missing `Close()` in the skopeo proxy code.
(It'd be better to use Linux `O_TMPFILE`, but this path is
portable and avoids a new dependency)
Signed-off-by: Colin Walters <walters@verbum.org>
To avoid locking the store for a large time we currently make a copy
of the blob into TemporaryDirectoryForBigFiles (i.e. /var/tmp) when
getting a stream to the blob. During a simple "podman run" this
happens 6 time.
However, all those calls are for the config blob, which we don't stream
anyway, so the temporary copy is unnecessary. This commit moves the
check for a the config blob outside of `getBlobAndLayerID()` allowing
us to return early in the config blob case and avoid the temporary file.
Signed-off-by: Alexander Larsson <alexl@redhat.com>
The hard-coded 0 is defined in the API, so this works fine;
this is just a readability improvement.
Should not change behavior.
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 matches internal/imagedestination/impl.Properties; it's not
quite worth it for the single value, but the consistency is
attractive.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This sets up the precedent that all transports should primarily implement
the private interface; that will allow us to make future changes to the
private interface easier, because we can just change the public interface
wrappers in a single place instead of modifying transports - especially
as more stubs are added soonish.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
storage_image.go is becoming too large, and we'd like to
have easy access to both internal/imagesource/impl and
internal/imagedestination/impl, which is easier with separate files.
Only moves unchanged code, should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Miloslav Trmač