From 98f87075352a9616740d04bf1827760807ad868b Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Mon, 12 Feb 2024 14:11:59 -0500 Subject: [PATCH] build --all-platforms: skip some base "image" platforms When figuring out which platforms the base images allow us to build derived images for, screen out any images with non-empty artifactType values. Also screen out any which use empty values or the word "unknown" in the OS and Architecture platform fields, and any Architecture values that the compiler hasn't heard of. Signed-off-by: Nalin Dahyabhai --- imagebuildah/build.go | 32 +++++++++++++++++++++++++++++-- tests/bud/from-base/Containerfile | 2 ++ tests/lists.bats | 27 ++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 tests/bud/from-base/Containerfile diff --git a/imagebuildah/build.go b/imagebuildah/build.go index 112bd2b62a7..39e98370635 100644 --- a/imagebuildah/build.go +++ b/imagebuildah/build.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + gotypes "go/types" "io" "net/http" "os" @@ -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" ) @@ -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. @@ -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) @@ -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) diff --git a/tests/bud/from-base/Containerfile b/tests/bud/from-base/Containerfile new file mode 100644 index 00000000000..207b4dfd6ef --- /dev/null +++ b/tests/bud/from-base/Containerfile @@ -0,0 +1,2 @@ +FROM base +ADD . . diff --git a/tests/lists.bats b/tests/lists.bats index be51af6eea8..d4776073807 100644 --- a/tests/lists.bats +++ b/tests/lists.bats @@ -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 ]] +}