Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build --all-platforms: skip some base "image" platforms #5335

Merged
merged 1 commit into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions imagebuildah/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"errors"
"fmt"
gotypes "go/types"
"io"
"net/http"
"os"
Expand Down Expand Up @@ -36,6 +37,7 @@ import (
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/openshift/imagebuilder"
"github.com/sirupsen/logrus"
"golang.org/x/exp/slices"
"golang.org/x/sync/semaphore"
)

Expand Down Expand Up @@ -493,6 +495,26 @@ func preprocessContainerfileContents(logger *logrus.Logger, containerfile string
return &stdoutBuffer, nil
}

// platformIsUnknown checks if the platform value indicates that the
// corresponding index entry is suitable for use as a base image
func platformIsAcceptable(platform *v1.Platform, logger *logrus.Logger) bool {
if platform == nil {
logger.Trace("rejecting potential base image with no platform information")
return false
}
if gotypes.SizesFor("gc", platform.Architecture) == nil {
// the compiler's never heard of this
logger.Tracef("rejecting potential base image architecture %q for which Go has no knowledge of how to do unsafe code", platform.Architecture)
return false
}
if slices.Contains([]string{"", "unknown"}, platform.OS) {
// we're hard-wired to reject images with these values
logger.Tracef("rejecting potential base image for which the OS value is always-rejected value %q", platform.OS)
return false
}
return true
}

// platformsForBaseImages resolves the names of base images from the
// dockerfiles, and if they are all valid references to manifest lists, returns
// the list of platforms that are supported by all of the base images.
Expand Down Expand Up @@ -570,7 +592,10 @@ func platformsForBaseImages(ctx context.Context, logger *logrus.Logger, dockerfi
if baseImageIndex == 0 {
// populate the list with the first image's normalized platforms
for _, instance := range index.Manifests {
if instance.Platform == nil {
if !platformIsAcceptable(instance.Platform, logger) {
continue
}
if instance.ArtifactType != "" {
continue
}
platform := internalUtil.NormalizePlatform(*instance.Platform)
Expand All @@ -581,7 +606,10 @@ func platformsForBaseImages(ctx context.Context, logger *logrus.Logger, dockerfi
// prune the list of any normalized platforms this base image doesn't support
imagePlatforms := make(map[string]struct{})
for _, instance := range index.Manifests {
if instance.Platform == nil {
if !platformIsAcceptable(instance.Platform, logger) {
continue
}
if instance.ArtifactType != "" {
continue
}
platform := internalUtil.NormalizePlatform(*instance.Platform)
Expand Down
2 changes: 2 additions & 0 deletions tests/bud/from-base/Containerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
FROM base
ADD . .
27 changes: 27 additions & 0 deletions tests/lists.bats
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,30 @@ IMAGE_LIST_S390X_INSTANCE_DIGEST=sha256:882a20ee0df7399a445285361d38b711c299ca09
# since manifest does not exist in local storage this should exit with `1`
run_buildah 1 manifest exists test2
}

@test "manifest-skip-some-base-images-with-all-platforms" {
start_registry
run_buildah manifest create localhost:"${REGISTRY_PORT}"/base
run_buildah manifest add --all localhost:"${REGISTRY_PORT}"/base ${IMAGE_LIST}
# get a count of how many "real" base images there are
run_buildah manifest inspect localhost:"${REGISTRY_PORT}"/base
nbaseplatforms=$(grep '"platform"' <<< "$output" | wc -l)
echo $nbaseplatforms base platforms
# add some trash that we expect to skip in a --all-platforms build
run_buildah build --manifest localhost:"${REGISTRY_PORT}"/base --platform unknown/unknown --no-cache -f $BUDFILES/from-scratch/Containerfile2 $BUDFILES/from-scratch
run_buildah build --manifest localhost:"${REGISTRY_PORT}"/base --platform linux/unknown --no-cache -f $BUDFILES/from-scratch/Containerfile2 $BUDFILES/from-scratch
run_buildah build --manifest localhost:"${REGISTRY_PORT}"/base --platform unknown/amd64p32 --no-cache -f $BUDFILES/from-scratch/Containerfile2 $BUDFILES/from-scratch
# add a known combination of OS/arch that we can be pretty sure wasn't already there
run_buildah build --manifest localhost:"${REGISTRY_PORT}"/base --platform linux/amd64p32 --no-cache -f $BUDFILES/from-scratch/Containerfile2 $BUDFILES/from-scratch
# push the list to the local registry and clean up our local copy
run_buildah manifest push --tls-verify=false --creds testuser:testpassword --all localhost:"${REGISTRY_PORT}"/base
run_buildah rmi localhost:"${REGISTRY_PORT}"/base
# build a new list based on the valid base images in the list we just pushed
run_buildah build --tls-verify=false --creds testuser:testpassword --manifest derived --all-platforms --from localhost:"${REGISTRY_PORT}"/base $BUDFILES/from-base
run_buildah manifest inspect derived
nderivedplatforms=$(grep '"platform"' <<< "$output" | wc -l)
echo $nderivedplatforms derived platforms
nexpectedderivedplatforms=$((nbaseplatforms+1))
echo expected $nexpectedderivedplatforms derived platforms
[[ $nderivedplatforms -eq $nexpectedderivedplatforms ]]
}
Loading