remotecache: split the import blob size limits (manifest 4 MiB, cache config 16 MiB) - #7126
Open
dsloane-easygo wants to merge 1 commit into
Open
dsloane-easygo wants to merge 1 commit into
dsloane-easygo wants to merge 1 commit into
Conversation
… config 16 MiB) readBlob refused any blob over 1 MiB, for the cache manifest and the cache config alike. The manifest comes from the registry's manifest endpoint, which registries cap at a few MiB; the cache config (application/vnd.buildkit.cacheconfig.v0) is served from the blob endpoint and has no such ceiling, and a mode=max export of a large multi-stage build on a busy shared daemon produces a config of 1-2 MiB, after which every import fails with "blob ... is too large" and the build silently loses its registry cache. Give readBlob a per-call limit: 4 MiB for the manifest read, 16 MiB for the cache config read, as discussed in moby#3719. Add a unit test that pins both ceilings and covers the previously refused 1 MiB + 1 byte config. Fixes moby#3719 Fixes moby#4916 Signed-off-by: Damian Sloane <d.sloane@easygo.io>
tonistiigi
reviewed
Sep 11, 2026
| require.True(t, strings.Contains(err.Error(), "is too large"), err.Error()) | ||
| }) | ||
|
|
||
| t.Run("manifest over its ceiling is refused before reading", func(t *testing.T) { |
Member
There was a problem hiding this comment.
These tests don't seem to do much as they just check the variable that is directly passed in. This would pass even if the actual manifest reader doesn't use these vars at all. Better to remove I think.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
readBlobincache/remotecache/import.gorefuses any blob over 1 MiB, and it is used for both reads the importer makes: the cache manifest (or index) and the cache config blob (application/vnd.buildkit.cacheconfig.v0). The manifest comes from the registry's manifest endpoint, which registries cap at a few MiB, so a ceiling there is reasonable. The cache config is served from the blob endpoint and has no such ceiling; amode=maxexport of a large multi-stage build on a busy shared daemon produces a config of 1 to 2 MiB, and from then on every import fails withand the build silently loses its registry cache (the export succeeds, the import never does). Measured on one shared buildkitd v0.30.0: 2,664 records, 127 layers, 960,778 bytes one export and 1,618,088 bytes the next, for the same Dockerfile.
This follows the direction in #3719 ("raising the limit to 4MB for the manifest and maybe ~12MB for the cache config"):
readBlobtakes a per-call limit, the manifest read passesmaxManifestBlobSize(4 MiB) and the cache-config read passesmaxCacheConfigBlobSize(16 MiB). The error text is unchanged.import_test.gopins both ceilings: a 1 MiB + 1 byte config (the previously refused case) imports, blobs above either ceiling are refused before being read, a manifest within its ceiling reads. Reverting either constant to1 << 20fails the test.Fixes #3719
Fixes #4916