Commit Graph

311 Commits

Author SHA1 Message Date
Giuseppe Scrivano 8c9ca13d37
store: remove deprecated ApplyDiffWithDiffer
The ApplyDiffWithDiffer function was marked as deprecated,
with PrepareStagedLayer being the recommended replacement.
Its implementation was just a wrapper around PrepareStagedLayer.

Remove the deprecated function from the Store and LayerStore
interfaces and its implementation.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2025-05-07 08:52:18 +02:00
Giuseppe Scrivano 729821c12e
chunked: Refactor temporary file creation
Replace the direct call to unix.Open with the O_TMPFILE flag
with the dedicated openTmpFile helper function.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2025-04-29 12:26:57 +02:00
Giuseppe Scrivano f5bdfdc07e
chunked: use temporary file for tar-split data
Replace the in-memory buffer with a O_TMPFILE file.  This reduces the
memory requirements for a partial pull since the tar-split data can be
written to disk.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2025-04-29 12:26:57 +02:00
Giuseppe Scrivano 3724a5a0bd
driver: add Close() method to Differ
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2025-04-28 22:08:03 +02:00
Giuseppe Scrivano 8ebe9600e3
chunked: extract blob validation into standalone function
extract the blob checksum validation logic from decodeAndValidateBlob
into a separate function.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2025-04-28 22:07:58 +02:00
Kir Kolyshkin 8395a8b205 Fix and annotate Stat_t fields conversion
For struct Stat_t in syscall pkg:
 - Ino is always uint64;
 - Dev/Rdev can be uint64, uint32, or int32;
 - Nlink might be uint64, uint32, or uint16.

Fix the code accordingly, adding or removing typecasts where needed,
and annotating those with //nolint:unconvert to calm down the unconvert
linter.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2025-04-01 16:18:55 -07:00
Kir Kolyshkin b7fb12e894 Remove unneeded conversion
Those are the cases where the value being converted is already of that
type (checked to be that way for all os/arch combinations).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2025-04-01 16:18:43 -07:00
openshift-merge-bot[bot] 6f4bc1fa7c
Merge pull request #2305 from kolyshkin/for-range
Use for range over integers
2025-04-01 21:57:19 +00:00
Kir Kolyshkin 73b194b8ed Use for range over integers
This form is available since Go 1.22 (see
https://tip.golang.org/ref/spec#For_range) and will probably be seen
more and more in the new code.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2025-03-31 15:39:12 -07:00
Kir Kolyshkin c3ff7f58df Use any instead of interface{}
It's available since Go 1.18 (see https://pkg.go.dev/builtin#any).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2025-03-31 15:37:25 -07:00
Kir Kolyshkin e1b7a98a21 Use min/max to simplify the code
Should not change functionality.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2025-03-28 20:08:32 -07:00
Kir Kolyshkin 0ee055d8d2 Do not capitalize error strings
Fix a bunch of linter warnings:

> ST1005: error strings should not be capitalized

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2025-03-26 14:29:57 -07:00
Miloslav Trmač 05de8c7758 Remove the remaining user of golang.org/x/exp/maps
Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2025-03-26 21:00:20 +01:00
Марк Булатов 93d250c406
Fix handle leak
Signed-off-by: Марк Булатов <markus7bulatenko@yandex.ru>
2025-03-06 13:04:28 +01:00
Miloslav Trmač e3e4ef8552 Bump maxTocSize to 150 MB
We have seen an image with:
- total size 1.43 GB
- uncompressed zstd:chunked manifest size of 91.7 MB
- uncompressed tar-split size (not constrained by maxTocSize) 310 MB

Without more infrastructure, we are just guessing about what
the system we are running on can support, so, for now, *shrug*, bump
the number.

Eventually we should stream the data from/to disk, making this
much less relevant; that makes building the infrastructure to
estimate available memory unattractive.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2025-01-30 21:04:50 +01:00
Miloslav Trmač 009737bdce Allow falling back from partial pulls if the metadata is too large
... but not if the fallback would be convert_images, again
creating too large metadata.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2025-01-30 18:17:50 +01:00
Miloslav Trmač 8e3996ba14 Move the canFallback logic from make...Differ into read...Manifest
That's a logically better place, it pairs the getBlobAt
calls with the ErrBadRequest types specific to those call sites.

We will, also, add more fallback reasons.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2025-01-30 18:17:50 +01:00
Miloslav Trmač 0dbf0feab9 Remove the canFallback return value
Instead, have the callees produce ErrFallbackToOrdinaryLayerDownload
directly.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2025-01-30 18:17:50 +01:00
Miloslav Trmač 9c42f91cd8 Reorganize chunkedDiffer
Try to split fields by purpose, and document some of the context.

For consistency, use the same order in the struct literals.

Only reorders existing fields, should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2025-01-30 18:17:50 +01:00
Giuseppe Scrivano 87f7d5268e
chunked: use fallback mechanism on non-linux platforms
inform the caller to fallback to an ordinary copy when not running on
Linux.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2025-01-27 13:39:33 +01:00
Miloslav Trmač 9119f36918 When applying a chunked layer with a tar-split, compute its uncompressed digest
This will allow c/image to validate the uncompressed digest against the config's
RootFS.DiffID value (ensuring that the layer's contents are the same when pulled
via TOC and traditionally); and the uncompressed digest will be used as a layer ID,
ensuring users see the traditional layer and image IDs they are used to.

This doesn't work for layers without a tar-split (all estargz, and old zstd:chunked
layers); for those, we fall back to traditional pulls.

Alternatively, for EXTREMELY restricted use cases, add an
"insecure_allow_unpredictable_image_contents" option to storage.conf. This option
allows partial pulls of estargz and old zstd:chunked layers, and skips the costly
uncompressed digest computation. It is then up to the user to worry about
images where the tar representation and the TOC representation don't match,
and about unpredictable image IDs.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2025-01-07 16:56:09 +01:00
Miloslav Trmač 3c62db3445 Push the "canFallback" logic from getProperDiffer into the format handlers
We will add more, format-specific, logic to the decision.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2025-01-07 16:50:19 +01:00
Miloslav Trmač 25b8040272 Inline parseBooleanPullOption into its only caller
Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2025-01-07 16:50:19 +01:00
Miloslav Trmač 6efb877bae Centralize parsing of store.PullOptions
... so that it happens
- before we start doing anything destructive
- only once (with no risk of defaults getting out of sync)
- in a single place

Ideally this should happen along with the initial parsing
of the config file; this is not that, but it is a minor step
in that direction.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2025-01-07 16:50:19 +01:00
Miloslav Trmač 116abed46c Introduce pkg/chunked/internal/path.RegularFilePathForValidatedDigest
... to centralize the composefs backing file layout design.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2025-01-07 16:50:19 +01:00
Miloslav Trmač 603328d069 Change how makeEntriesFlat tracks duplicate entries
- Use a map of struct{} to save on storage
- Track the items by path, not by the digest; that's one byte more
  per entry, but it will allow us to abstract the path logic.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2025-01-07 16:50:19 +01:00
Miloslav Trmač 6ae83ca1a9 Use realistic digests in tests of pkg/chunked/dump
We will start validating them.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2025-01-07 16:50:19 +01:00
Miloslav Trmač 5c67136767 Move pkg/chunked/internal to pkg/chunked/internal/minimal
We now have several internal subpackages of pkg/chunked, so delineate
more explicitly the parts that should be kept as small as possible
because the c/image compression package depends on them.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2024-12-13 01:27:58 +01:00
Giuseppe Scrivano 8fa5ca4cd6
chunked, dump: use CleanAbsPath
replacing the usage of a private function.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2024-12-11 13:37:12 +01:00
Giuseppe Scrivano ce0186617a
chunked: improve creation of files under root
ihen the file name is the root directory, avoid using an empty string
or the ".." name to open the file.  The latter does not cause any
security issues or unexpected behavior, it is logically incorrect and
should be avoided.

Closes: https://github.com/containers/storage/issues/2191

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2024-12-11 13:36:03 +01:00
Giuseppe Scrivano 478fc7510d
chunked: improve detection of root directory path
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2024-12-11 13:34:46 +01:00
Giuseppe Scrivano f5d0b673c6
chunked: define new function CleanAbsPath
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2024-12-11 13:33:31 +01:00
Giuseppe Scrivano 489db7e50f
chunked: change some assert to require
the returned file object is used later, so fail immediately if it
could not be opened.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2024-12-11 11:03:41 +01:00
Giuseppe Scrivano c33a45d30b
chunked: clarify recursion termination condition
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2024-12-11 11:03:41 +01:00
Giuseppe Scrivano 40a1ea7fb2
chunked: store the override xattr with the inode type
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2024-12-06 08:50:08 +01:00
Giuseppe Scrivano f098bdd5a5
chunked: honor ForceMask for created files
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2024-12-04 11:52:59 +01:00
Giuseppe Scrivano 1f54749ea9
chunked: refactor value into const
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2024-11-14 22:59:03 +01:00
Giuseppe Scrivano dcdc061f21
chunked: rework GetBlobAt usage
rewrite how the result from GetBlobAt is used, to make sure 1) that
the streams are always closed, and 2) that any error is processed.

Closes: https://issues.redhat.com/browse/OCPBUGS-43968

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2024-11-14 22:59:03 +01:00
Nalin Dahyabhai 435aa93e14 Disable partial pulls (zstd:chunked) by default
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>
2024-11-04 13:52:48 -05:00
Giuseppe Scrivano 8b5dadb045
chunked: close payload stream
The payload stream must be closed after being used.

Reported here: https://github.com/containers/storage/pull/2041#discussion_r1690061874

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2024-10-24 22:43:59 +02:00
Miloslav Trmač 82234e1a36 Fall back from partial pulls to ordinary pulls on VFS
... and other graph drivers which don't support partial pulls.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2024-10-18 21:54:35 +02:00
Miloslav Trmač bdbd5ed90d Refactor getProperDiffer further
Use switch to handle the various TOC presence cases

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2024-10-18 21:54:27 +02:00
Miloslav Trmač da9f66a4ac Refactor getProperDiffer a bit
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>
2024-10-18 21:54:06 +02:00
Miloslav Trmač 8e3938748f Split getProperDiffer from GetDiffer
... 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>
2024-10-18 21:53:56 +02:00
openshift-merge-bot[bot] 4db236377c
Merge pull request #2137 from kolyshkin/codeslepp
ci: add codespell
2024-10-17 15:52:35 +00:00
Kir Kolyshkin cfe8024bff ci: add codespell
1. Move codespell config out of Makefile, simplify (remove unused
   stuff).

2. Fix found issues (using codespell -w).

3. Add a codespell CI job.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2024-10-15 17:39:07 -07:00
Miloslav Trmač 1a8fde7e01 Don't discard the error reported by tarSizeFromTarSplit
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2024-10-16 00:36:13 +02:00
Miloslav Trmač f979bada64 Compute the layer size from tar-split for zstd:chunked layers
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2024-10-14 20:10:07 +02:00
Miloslav Trmač 7eb4a104ef Explicitly differentiate between empty and missing tar-split
Empty tar-split shouldn't ever happen, but being precise
here doesn't hurt.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2024-10-14 20:10:07 +02:00
Miloslav Trmač 71fc790550 Don't set UncompressedSize on chunked pull
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>
2024-10-14 20:10:07 +02:00