Updating to the lowest release that includes [protobuf@dfab275], which
removed use of the github.com/golang/protobuf/ptypes/timestamp.Timestamp
type alias (deprecated).
[protobuf@dfab275]: dfab275eca
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When support for zstd was introduced, these mediaTypes were not yet
available in released versions of their respective Go packages. That is
no longer the case.
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
The layerParent and diffParents cases locked each parent's mutex and
deferred the unlock to the end of the function, so the lock was held
longer than the release it protects -- and in the diffParents case the
lower parent's lock was still held while the upper parent was locked and
released.
Unlock each parent immediately after releasing it so the lock is held
only for that parent's release, matching the mergeParents case in the
same switch which already does this. This narrows the lock scope and
avoids holding one parent's lock while acquiring another's.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Update golangci-lint and adjust code for new gosec diagnostics. Use
root-scoped filesystem operations where appropriate, preserve explicit
user path behavior for SSH keys, and avoid background contexts in
request-scoped cleanup paths.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
readerAtCloser implements io.ReaderAt, whose documented contract
explicitly allows concurrent callers:
Clients of ReadAt can execute parallel ReadAt calls on the same
input source.
However the type has no synchronization protecting its mutable state
(rc, ra, offset, closed). When used as the body of an S3 upload via
manager.Uploader, the AWS SDK v2 upload manager spawns DefaultUpload-
Concurrency=5 worker goroutines, each given an io.NewSectionReader
slice that shares the same underlying body. Each worker's Read
eventually becomes a concurrent ReadAt on the same *readerAtCloser
at a different offset, racing on the close-then-reopen path.
Under the race detector this produces DATA RACE warnings on rc,
offset and ra. Without the race detector it intermittently crashes
buildkitd with a nil pointer dereference at the hrs.rc.Read(p) call
in the io.ReaderAt fallback branch, when one goroutine nils rc
between a peer's nil-check and Read dispatch. The crash kills the
buildkit daemon and cannot be recovered by the buildctl client.
The bug reliably reproduces whenever a cache layer exceeds the
default 5 MiB part size, i.e. essentially every real-world Docker
image build that exports cache to S3.
PR #5597 added an offset parameter to s3Client.getReader and reduced
the frequency of close-reopen on the common sequential-read path,
but did not address the underlying thread-safety violation. This
commit takes the minimum-risk correctness fix: serialize ReadAt and
Close with a sync.Mutex so the io.ReaderAt contract is honoured and
the existing close-reopen logic remains correct under concurrent
callers.
Under the mutex this becomes serialised per readerAtCloser: correct,
but potentially slower for a single large blob that is being
re-exported from S3 back to S3. This is not a global BuildKit lock:
different cache layers can still upload in parallel, but one blob
backed by this reader loses multipart read parallelism and still
pays the reopen churn. A proper follow-up would also eliminate the
shared-state optimisation that causes the thrashing, for example by
having ReaderAt open an independent reader per call (stateless), or
by keeping a small pool of per-offset readers. That optimisation
can be follow-up work after this correctness fix, or part of the
#3993 refactor (which would also need a mutex added to the
replacement contentutil.readerAt).
Closes#6674
Signed-off-by: Vladimir Kuznichenkov <vova@kuzaxak.dev>
Add configurable retry behavior for the S3 cache.
Users can now set `retry_mode` (standard/adaptive) and
`retry_max_attempts` to tune AWS SDK retry settings for their
environment. When not specified, the AWS SDK defaults apply
(standard mode, 3 max attempts).
Signed-off-by: Jiří Moravčík <jiri.moravcik@gmail.com>
Replace duplicate nopCloser, nopWriteCloser, and bufferCloser
type definitions across test and production code with the
shared iohelper.NopWriteCloser type.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Introduce util/pools.Pool[T] as a generic typed wrapper
around sync.Pool. Migrate existing pool usages in
contenthash, converter, and overlay packages to use the
new wrapper, removing unchecked type assertions at each
call site.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
This removes the MigrateV2 function, which was added in BuildKit v0.7.0
(Docker v20.10.0) in 31a9aeea88. That was
in 2019, which is now over 6 Years ago, so it's very unlikely for old
files to be still present.
Removing this code would impact users migrating from Docker 19.03 or
older, which are versions that reached EOL many years ago, so very
unlikely.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Registry cache blob pushing was sequential, copying one layer at a time. This
uses images.Dispatch to push all cache layer blobs in parallel, similar
to how image export already handles layer pushing.
The blobs are collected upfront, pushed concurrently, then
added to the cache manifest in order after all pushes complete.
Signed-off-by: Amr Mahdi <amrmahdi@meta.com>
Previously, each call to `ReadFile`/`ReadDir`/`StatFile` would trigger
an entire `Mount`+`Unmount` operation. This could result in terrible
performance for repeated filesystem accesses - potentially worse that
one `Mount`+`Unmount` per file, if the file is read in ranges.
To resolve this, we now directly store the `localMounter` in the
gateway, and rely on the fact that repeated calls to it return the same
value. These mounts will only be removed when the gateway itself is
released.
Signed-off-by: Justin Chadwell <me@jedevc.com>
This adds an additional `RequiredPaths` that is primarily intended for
use with `COPY --parents`. This parameter specifies expected directories
or files that should exist when performing the checksum. A not found
error will be produced if one of these paths is missing.
This fixes an issue with `COPY --parents` where a non existent directory
that was intended to be copied would be ignored.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This changes the way the checksum is calculated for content that has
wildcards or include/exclude paths. The previous method would result in
a different checksum with the same file contents but different patterns
due to a different method of hashing the data.
This normalizes the way the data is hashed to use similar methods. The
wildcard path now strips the prefix from the path instead of using the
base path. It no longer adds an additional zero byte between entries but
instead uses the zero byte from the directory path key.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Nested symlinks were not intended to be followed after the first level
of wildcard expansion. When include/exclude patterns were used or a
wildcard was present higher up in the tree, we would erroneously follow
ALL symlinks to compute the hash.
When a symlink was broken, this would result in an error. We weren't
supposed to be following the symlinks at all.
This fixes things by changing `includedPaths` to also return whether a
path should follow links. If a parent directory exists for the path,
then the symlink is not followed.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This is a complete refactor of the cache export Go interface.
Main aspect is to provide a strictly ordered walk of the
cache tree instead of previous one where modification could be
added to cache tree at any time by any component.
This should address subtle concurrency issues and remove
large parts of complicated (and likely buggy) normalization
and deduplication steps.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Removes the `tools/tools.go` file used as a hack to get `go.mod` to
track tools in favor of the new method introduced in go 1.24 of being
formally supported in the `go.mod` file. This will allow the tools to be
managed with the new `go get tool` and `go install tool` commands.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The walkBlob function was called with a function having a side effect: appending a descriptor to a list. However, the function could be called multiple times for the same descriptor (once in `walkBlob` and once in `walkBlobVariantsOnly` in that case) leading to duplicate entries in the final list.
Instead, we will check each time we want to add an element, that it's not already in the list using slice.ContainsFunc.
This should be fine from a performance perspective as the list is small (at most 2 compressions in the vast majority of cases).
Signed-off-by: Baptiste Girard-Carrabin <baptiste.girardcarrabin@datadoghq.com>
This allows opt-in to cache key debug database on
daemon startup.
If enabled, all cache keys generated by builds are
saved into this database together with the plaintexts
of the original data so a reverse lookup can be performed
later to compare two checksums and find out their original
difference. If checksum contains other checksums internally
then these are saved as well. For storage constraints, the
plaintext of file content is not saved but the metadata
portion can be still looked up.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Go 1.23 changed the behavior of `os.Stat` to set `ModeIrregular` when it
encountered a mount point reparse point on Windows. These were
previously labeled as symlinks.
We attempted to fix this by clearing `ModeIrregular` during checksum
generation, but we only cleared it in the case where `ModeIrregular` and
at least one other mode type was set.
After reviewing the original solution and issue, I believe that this was
too conservative. The original issue seems to indicate that only the
`ModeIrregular` bit was set so the condition would not have triggered
and cleared it properly. I thought mount points would have both
`ModeDir` and `ModeIrregular` set, but they only seem to have
`ModeIrregular` set.
This unconditionally clears the bit similar to socket files which should
properly fix the issue on Windows.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
