imagebuildah: default RUN --mount type to bind for cache checksums - #6957
Merged
TomSweeneyRedHat merged 1 commit intoJul 14, 2026
Conversation
getFromAndSourceKeysFromMountFlag() parses a RUN --mount=... flag to
decide whether getCreatedBy() should checksum the mounted content for
the build-cache fingerprint. When a Dockerfile omits type= (the
normal, idiomatic way to write a bind mount, since bind is the
implicit default), this parser left Type empty instead of defaulting
it to "bind", unlike buildah's other mount parser used to actually set
up mounts (internal/volumes/volumes.go), which does default to
define.TypeBind.
Because of that, getCreatedBy()'s "if mountInfo.Type != bind { continue }"
check skipped checksumming the mounted content for any implicit-bind
mount, whether sourced from the build context, another build stage, an
image, or an additional build context. Two builds with identical
Dockerfile text but different mounted file content ended up with an
identical cache key, so the second build silently reused the first
build's cached layer instead of picking up the new content.
type=cache and type=tmpfs mounts are still correctly excluded from
checksumming; that exclusion is intentional, since their purpose is to
persist regardless of build content.
Reported downstream at
podman-container-tools/podman#29126.
Signed-off-by: MayukhSobo <mayukh2012@hotmail.com>
Contributor
|
Nice! TYVM! |
Contributor
|
/lgtm |
TomSweeneyRedHat
merged commit Jul 14, 2026
ce4012e
into
podman-container-tools:main
44 of 46 checks passed
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
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.
/kind bug
What this PR does / why we need it:
getFromAndSourceKeysFromMountFlag()parses aRUN --mount=...flag to decide whethergetCreatedBy()should checksum the mounted content for the build-cache fingerprint. When a Dockerfile omitstype=(the normal, idiomatic way to write a bind mount, since bind is the implicit default per the Dockerfile/BuildKit mount syntax), this parser leftTypeempty instead of defaulting it to"bind"— unlike buildah's other mount parser, the one that actually sets mounts up (internal/volumes/volumes.go'sgetMounts(), which does defaultmountType := define.TypeBind).Because of that,
getCreatedBy()'sif mountInfo.Type != "bind" { continue }check skipped checksumming the mounted content for any implicit-bind mount — whether sourced from the build context, another build stage, an image, or an additional build context. Two builds with identical Dockerfile text but different mounted file content ended up with an identical cache key, so the second build silently reused the first build's cached layer instead of picking up the new content.The fix defaults the parsed mount type to
define.TypeBind, matching the default already used by the real mount-setup parser.type=cacheandtype=tmpfsmounts remain correctly excluded from checksumming — that's intentional, since persisting regardless of build content is their whole purpose.How to verify it
data.txtcontainingfoo.type=):buildah build --layers .— it printsfoo.data.txtto containbar, then rebuild the same Dockerfile with--layersagain.foo, even though the mounted file changed. After this fix: the cache is correctly invalidated and the second build printsbar.A unit test was also added,
TestGetFromAndSourceKeysFromMountFlaginimagebuildah/util_test.go, which fails without this fix (assertsTypedefaults to"bind"whentype=is omitted) and passes with it:Which issue(s) this PR fixes:
Fixes podman-container-tools/podman#29126
Special notes for your reviewer:
This bug was reported against podman (originally podman-container-tools/podman#29126, now podman-container-tools/podman#29126 after the org transfer), but root-caused to buildah —
podman buildvendors buildah'simagebuildahpackage. There is no separate buildah-side issue tracking this; per CONTRIBUTING.md, a PR alone is sufficient as long as its description includes what an issue report would (repro steps + root cause), which this PR provides above.Does this PR introduce a user-facing change?