Disable the storage.options.pull_options.enable_partial_images option by
default, so that it will have to be explicitly enabled in order to be
used.
Update the apply-diff-from-staging-directory integration test to call
the test helper binary directly, so that the configuration file the test
writes won't have its settings overridden by command line options that
the storage() test helper function adds.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Instead of sharing the badRequestErr logic, duplicate it.
That's a bit ugly, but we get better debug messages and a more
traditional control flow.
Should not change behavior, except for debug messages.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... to centralize the fallback allowed / required logic.
Should not change behavior, apart from maybe some error text.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
The current value obtained by summing the sizes of regular file contents
does not match the size of the uncompressed layer tarball.
We don't have a convenient source to compute the correct size
for estargz without pulling the full layer and defeating the point;
so we must allow for the size being unknown.
For recent zstd:chunked images, we have the full tar-split,
so we can compute the correct size; that will happen in
the following commits.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
define a new error type so that the caller can determine whether it is
safe to ignore the error and retrieve the resource fully.
Closes: https://github.com/containers/storage/issues/2115
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Avoid handling cases where the server doesn't support at least 64
ranges in a request, in order to prevent falling back to the
traditional pull mechanism.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Conceptually, these read from the network and _should_ be interruptible.
But we could, at best, interrupt the wait on the channels returned by
GetBlobAt; we would then still get a ReadCloser where we can call
Read() but not concurrently notice a cancellation.
The cancellation needs to happen at the HTTP client side, i.e. inside
the c/image private.ImageSourceInternalOnly.GetBlobAt call (not ImageSourceSeekable),
and, although it seems not to be quite documented, the http stack's http.NewRequestWithContext
does cause the HTTP body transport to be terminated on a context cancellation.
I.e. it is out of control of this codebase anyway; so don't pretend to handle it.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
if the compressed digest was validated, as it happens when
'pull_options = {convert_images = "true"}' is set, then store it as
well so that reusing the blob by its compressed digest works.
Previously, when an image converted to zstd:chunked was pulled a
second time, it would not be recognized by its compressed digest,
resulting in the need to re-pull the image again.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
honor the pull_options configuration set for the current store instead
of using the default configuration.
Needed by Podman to override the pull options from the command line
for a single command.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
I was just reading the code and I have a mental checklist item
for "invoking open without O_CLOEXEC" that triggered here.
(See also e.g.
https://github.com/containers/composefs/pull/185#discussion_r1322925050
)
It has security-relevant properties for us, xref
CVE-2024-21626 for example.
This isn't the only missing variant of this in this codebase,
just using this targeted PR to test the waters for more PRs.
Signed-off-by: Colin Walters <walters@verbum.org>
Increase the threshold for auto-merging parts from 128 to 1024. This change
aims to reduce the number of parts in an HTTP multi-range request, thus
increasing the likelihood that the server will accept the request.
The previous threshold of 128 often resulted in a large number of small
ranges, which could lead to HTTP multi-range requests being rejected by
servers due to the excessive number of parts.
It partially addresses the reported issue.
Reported-by: https://github.com/containers/storage/issues/1928
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Other TOC formats don't fill the data in.
For now, this only increases memory usage, but we will
need the data soon.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This code path is usually never triggered because
the annotations are present; and it was broken until recently.
Remove it to simplify the code and analysis.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Make it structually clear that the code is all using the same value,
making it less likely for the verifier and other uses to get out of sync.
Also avoids some redundant parsing and error paths.
The conversion path looks longer, but that's just moving the parsing
from the called function (which is redundant for other callers).
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Make it structually clear that the code is all using the same value,
making it less likely for the verifier and other uses to get out of sync.
Also avoids some redundant parsing and error paths.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
if the file is created using the object-store flat directory format,
there is no need to set its inodes attributes, as anyway they are
ignored when creating the composefs binary blob.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
improve the function that combines neighbor chunks. Instead of using
the number of parts, which also includes local files, use only the
number of chunks that must be retrieved from the network.
In addition, introduce a threshold limit to merge chunks so that we
further reduce the number of requested ranges.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
move the check for `enable_partial_images` to GetDiffer so that it
doesn't attempt any operation if the feature is disabled.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
by default enable pulling a partial image, it is still possible to
disable the feature through the configuration file.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
even if we validated the full layer, report the TOC Digest as well so
the upper layer can use both.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
store the UncompressedDigest when the original tarball was converted
to zstd:chunked, since its diffID was computed and validated.
In this way the layer can be reused as any other layer that was fully
retrieved and validated.
Before this change, a layer that was converted to zstd:chunked was
always retrieved since it has not a TOC Digest.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
it prevents clobbering the chunk .Size element later. This filed was
ignored previously, but composefs uses it to retrieve the file size.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Nalin Dahyabhai