root
/
buildah
mirror of https://github.com/containers/buildah.git
1
0
Fork 0
Code Issues Actions 3 Packages Projects Releases Wiki Activity

stage_executor: check platform of cache candidates 

When building images for `manifest` list using `--platform` same image
is used for multiple platform if base is `scratch` , following PR adds a
check to always verify `platform` of `cache` with `target`.

Closes: https://github.com/containers/podman/issues/18723

Signed-off-by: flouthoc <flouthoc.git@gmail.com>
This commit is contained in:
flouthoc 2025-07-08 07:46:31 -07:00
parent 252cc24fd5
commit 3502889676
No known key found for this signature in database
GPG Key ID: F1993E9E45D69299
3 changed files with 115 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
imagebuildah/executor.go
View File

@ -179,6 +179,8 @@ type imageTypeAndHistoryAndDiffIDs struct {
history []v1.History
diffIDs []digest.Digest
err error
architecture string
os string
}
// newExecutor creates a new instance of the imagebuilder.Executor interface.
@ -476,30 +478,30 @@ func (b *Executor) waitForStage(ctx context.Context, name string, stages imagebu
}
}
// getImageTypeAndHistoryAndDiffIDs returns the manifest type, history, and diff IDs list of imageID.
func (b *Executor) getImageTypeAndHistoryAndDiffIDs(ctx context.Context, imageID string) (string, []v1.History, []digest.Digest, error) {
// getImageTypeAndHistoryAndDiffIDs returns the os, architecture, manifest type, history, and diff IDs list of imageID.
func (b *Executor) getImageTypeAndHistoryAndDiffIDs(ctx context.Context, imageID string) (string, string, string, []v1.History, []digest.Digest, error) {
b.imageInfoLock.Lock()
imageInfo, ok := b.imageInfoCache[imageID]
b.imageInfoLock.Unlock()
if ok {
return imageInfo.manifestType, imageInfo.history, imageInfo.diffIDs, imageInfo.err
return imageInfo.os, imageInfo.architecture, imageInfo.manifestType, imageInfo.history, imageInfo.diffIDs, imageInfo.err
}
imageRef, err := storageTransport.Transport.ParseStoreReference(b.store, "@"+imageID)
if err != nil {
return "", nil, nil, fmt.Errorf("getting image reference %q: %w", imageID, err)
return "", "", "", nil, nil, fmt.Errorf("getting image reference %q: %w", imageID, err)
}
ref, err := imageRef.NewImage(ctx, nil)
if err != nil {
return "", nil, nil, fmt.Errorf("creating new image from reference to image %q: %w", imageID, err)
return "", "", "", nil, nil, fmt.Errorf("creating new image from reference to image %q: %w", imageID, err)
}
defer ref.Close()
oci, err := ref.OCIConfig(ctx)
if err != nil {
return "", nil, nil, fmt.Errorf("getting possibly-converted OCI config of image %q: %w", imageID, err)
return "", "", "", nil, nil, fmt.Errorf("getting possibly-converted OCI config of image %q: %w", imageID, err)
}
manifestBytes, manifestFormat, err := ref.Manifest(ctx)
if err != nil {
return "", nil, nil, fmt.Errorf("getting manifest of image %q: %w", imageID, err)
return "", "", "", nil, nil, fmt.Errorf("getting manifest of image %q: %w", imageID, err)
}
if manifestFormat == "" && len(manifestBytes) > 0 {
manifestFormat = manifest.GuessMIMEType(manifestBytes)
@ -510,9 +512,11 @@ func (b *Executor) getImageTypeAndHistoryAndDiffIDs(ctx context.Context, imageID
history: oci.History,
diffIDs: oci.RootFS.DiffIDs,
err: nil,
architecture: oci.Architecture,
os: oci.OS,
}
b.imageInfoLock.Unlock()
return manifestFormat, oci.History, oci.RootFS.DiffIDs, nil
return oci.OS, oci.Architecture, manifestFormat, oci.History, oci.RootFS.DiffIDs, nil
}
func (b *Executor) buildStage(ctx context.Context, cleanupStages map[int]*StageExecutor, stages imagebuilder.Stages, stageIndex int) (imageID string, ref reference.Canonical, onlyBaseImage bool, err error) {

25
imagebuildah/stage_executor.go
View File

@ -2148,7 +2148,7 @@ func (s *StageExecutor) generateCacheKey(ctx context.Context, currNode *parser.N
var manifestType string
var err error
if s.builder.FromImageID != "" {
manifestType, baseHistory, diffIDs, err = s.executor.getImageTypeAndHistoryAndDiffIDs(ctx, s.builder.FromImageID)
_, _, manifestType, baseHistory, diffIDs, err = s.executor.getImageTypeAndHistoryAndDiffIDs(ctx, s.builder.FromImageID)
if err != nil {
return "", fmt.Errorf("getting history of base image %q: %w", s.builder.FromImageID, err)
}
@ -2284,7 +2284,7 @@ func (s *StageExecutor) intermediateImageExists(ctx context.Context, currNode *p
var baseHistory []v1.History
var baseDiffIDs []digest.Digest
if s.builder.FromImageID != "" {
_, baseHistory, baseDiffIDs, err = s.executor.getImageTypeAndHistoryAndDiffIDs(ctx, s.builder.FromImageID)
_, _, _, baseHistory, baseDiffIDs, err = s.executor.getImageTypeAndHistoryAndDiffIDs(ctx, s.builder.FromImageID)
if err != nil {
return "", fmt.Errorf("getting history of base image %q: %w", s.builder.FromImageID, err)
}
@ -2328,7 +2328,7 @@ func (s *StageExecutor) intermediateImageExists(ctx context.Context, currNode *p
// Next we double check that the history of this image is equivalent to the previous
// lines in the Dockerfile up till the point we are at in the build.
manifestType, history, diffIDs, err := s.executor.getImageTypeAndHistoryAndDiffIDs(ctx, image.ID)
imageOS, imageArchitecture, manifestType, history, diffIDs, err := s.executor.getImageTypeAndHistoryAndDiffIDs(ctx, image.ID)
if err != nil {
// It's possible that this image is for another architecture, which results
// in a custom-crafted error message that we'd have to use substring matching
@ -2341,6 +2341,25 @@ func (s *StageExecutor) intermediateImageExists(ctx context.Context, currNode *p
if manifestType != s.executor.outputFormat {
continue
}
// Compare the cached image's platform with the current build's target platform
currentArch := s.executor.architecture
currentOS := s.executor.os
if currentArch == "" && currentOS == "" {
currentOS, currentArch, _, err = parse.Platform(s.stage.Builder.Platform)
if err != nil {
logrus.Debugf("unable to parse default OS and Arch for the current build: %v", err)
}
}
if currentArch != "" && imageArchitecture != currentArch {
logrus.Debugf("cached image %q has architecture %q but current build targets %q, ignoring it", image.ID, imageArchitecture, currentArch)
continue
}
if currentOS != "" && imageOS != currentOS {
logrus.Debugf("cached image %q has OS %q but current build targets %q, ignoring it", image.ID, imageOS, currentOS)
continue
}
// children + currNode is the point of the Dockerfile we are currently at.
foundMatch, err := s.historyAndDiffIDsMatch(baseHistory, baseDiffIDs, currNode, history, diffIDs, addedContentDigest, buildAddsLayer, lastInstruction)
if err != nil {

81
tests/bud.bats
View File

@ -8398,3 +8398,84 @@ EOF
expect_output --substring "included.txt"
assert "$output" !~ "excluded.txt"
}
@test "bud multi-platform --layers should create separate images for each platform" {
_prefetch busybox
local contextdir=${TEST_SCRATCH_DIR}/bud/multiplatform-layers
mkdir -p $contextdir
# Create a simple Dockerfile that starts from scratch to test the specific caching bug
cat > $contextdir/hello.txt << _EOF
Hello from multi-platform build!
_EOF
cat > $contextdir/Dockerfile << _EOF
FROM scratch
COPY hello.txt /hello.txt
CMD ["/hello.txt"]
_EOF
# Build for multiple platforms with --layers and --manifest
# This should create separate images for each platform, not reuse cached images incorrectly
local manifestname="multiplatform-test-$(safename)"
run_buildah build $WITH_POLICY_JSON --layers --platform linux/amd64,linux/arm64 --manifest "$manifestname" -f $contextdir/Dockerfile $contextdir
# Verify that the build completed successfully
expect_output --substring "linux/amd64"
expect_output --substring "linux/arm64"
# Check that we have a manifest list
run_buildah manifest exists "$manifestname"
# Inspect the manifest list to verify it contains entries for both platforms
run_buildah manifest inspect "$manifestname"
local manifest_content="$output"
# Verify the manifest contains both amd64 and arm64 entries
assert "$manifest_content" =~ "amd64" "manifest should contain amd64 platform"
assert "$manifest_content" =~ "arm64" "manifest should contain arm64 platform"
# Count the number of manifests - should have at least 2 (one for each platform)
run jq -r '.manifests | length' <<< "$manifest_content"
local manifest_count="$output"
assert "$manifest_count" -ge 2 "manifest list should contain at least 2 platform-specific manifests"
# Verify that the platforms are correctly specified in the manifest
run jq -r '.manifests[].platform.architecture' <<< "$manifest_content"
local architectures="$output"
assert "$architectures" =~ "amd64" "manifest should list amd64 architecture"
assert "$architectures" =~ "arm64" "manifest should list arm64 architecture"
# Verify we have multiple different image IDs (not the same image reused)
# This is the key test - before the fix, the same image would be reused for both platforms
run jq -r '.manifests[].digest' <<< "$manifest_content"
local digests=($output)
# Each platform should have a unique digest - they should not be the same
if [ "${#digests[@]}" -ge 2 ]; then
local first_digest="${digests[0]}"
local found_different=false
for digest in "${digests[@]:1}"; do
if [ "$digest" != "$first_digest" ]; then
found_different=true
break
fi
done
assert "$found_different" = true "each platform should have a unique image digest, not reuse the same cached image"
fi
}
@test "bud multi-platform --layers should not reuse different platform if default platform is being used" {
run_buildah info --format '{{.host.arch}}'
myarch="$output"
otherarch="ppc64le"
# just make sure that other arch is not equivalent to host arch
if [[ "$otherarch" == "$myarch" ]]; then
otherarch="amd64"
fi
run_buildah build --layers --platform linux/$otherarch -f $BUDFILES/from-scratch/Containerfile2 $BUDFILES/from-scratch/
# Second build should not use cache at all
run_buildah build --layers -f $BUDFILES/from-scratch/Containerfile2 $BUDFILES/from-scratch/
assert "$output" !~ "Using cache"
}