Skip to content

Commit

Permalink
build --all-platforms: skip some base "image" platforms
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
nalind committed Feb 12, 2024
1 parent d62beb9 commit 98f8707
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 2 deletions.
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 ]]
}

0 comments on commit 98f8707

Please sign in to comment.