Skip to content

Commit

Permalink
bib: check that architecture is expected arch
Browse files Browse the repository at this point in the history
When trying to build an image with an incompatible target arch
bib will currently not error because the container resolver is
not very strict about the architecture request.

This commit fixes this by double checking that the resolved
container is actually of the expected architecture.

This requires osbuild/images#585
  • Loading branch information
mvo5 committed Apr 23, 2024
1 parent 2bc76a6 commit 5abe18e
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 10 deletions.
50 changes: 40 additions & 10 deletions bib/cmd/bootc-image-builder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,25 @@ func loadConfig(path string) (*BuildConfig, error) {
return &conf, nil
}

// getContainerArch returns the architecture of an container image
func getContainerArch(imgref string) (cntArch arch.Arch, err error) {
outputB, err := exec.Command("podman", "image", "inspect", imgref, "--format", "{{.Architecture}}").Output()
if err != nil {
return 0, fmt.Errorf("failed inspect image for size: %w", util.OutputErr(err))
}
output := strings.TrimSpace(string(outputB))

// TODO: make images:arch.FromString() return an error
defer func() {
if err := recover(); err != nil {
err = fmt.Errorf("cannot convert %q to an architecture", output)
}
}()
cntArch = arch.FromString(output)

return cntArch, nil
}

// getContainerSize returns the size of an already pulled container image in bytes
func getContainerSize(imgref string) (uint64, error) {
output, err := exec.Command("podman", "image", "inspect", imgref, "--format", "{{.Size}}").Output()
Expand Down Expand Up @@ -149,15 +168,17 @@ func makeManifest(c *ManifestConfig, cacheRoot string) (manifest.OSBuildManifest
// (including the build-root) with the target arch then, it
// is fast enough (given that it's mostly I/O and all I/O is
// run naively via syscall translation)
hostArch := arch.Current().String()
targetArch := c.Architecture.String()
hostArch := arch.Current()
targetArch := c.Architecture

var resolver *container.Resolver
if targetArch != "" {
resolver = container.NewResolver(targetArch)
var wantedArch arch.Arch
if targetArch != arch.ARCH_UNSET {
wantedArch = targetArch
} else {
resolver = container.NewResolver(hostArch)
wantedArch = hostArch
}
// XXX: should NewResolver() take "arch.Arch"?
resolver := container.NewResolver(wantedArch.String())

containerSpecs := make(map[string][]container.Spec)
for plName, sourceSpecs := range manifest.GetContainerSourceSpecs() {
Expand Down Expand Up @@ -195,7 +216,7 @@ func saveManifest(ms manifest.OSBuildManifest, fpath string) error {
}

func manifestFromCobra(cmd *cobra.Command, args []string) ([]byte, *mTLSConfig, error) {
buildArch := arch.Current()
cntArch := arch.Current()

imgref := args[0]
configFile, _ := cmd.Flags().GetString("config")
Expand All @@ -215,7 +236,7 @@ func manifestFromCobra(cmd *cobra.Command, args []string) ([]byte, *mTLSConfig,
if slices.Contains(imgTypes, "iso") {
return nil, nil, fmt.Errorf("cannot build iso for different target arches yet")
}
buildArch = arch.FromString(targetArch)
cntArch = arch.FromString(targetArch)
}
// TODO: add "target-variant", see https://github.com/osbuild/bootc-image-builder/pull/139/files#r1467591868

Expand Down Expand Up @@ -250,14 +271,23 @@ func manifestFromCobra(cmd *cobra.Command, args []string) ([]byte, *mTLSConfig,
config = &BuildConfig{}
}

// check arch compatibility before pulling the container
pulledCntArch, err := getContainerArch(imgref)
if err != nil {
return nil, nil, fmt.Errorf("cannot get container architecture: %w", err)
}
if cntArch != pulledCntArch {
return nil, nil, fmt.Errorf("image found is for unexpected architecture %q (expected %q), if that is intentional, please make sure --target-arch matches", pulledCntArch, cntArch)
}

// If --local wasn't given, always pull the container.
// If the user mount a container storage inside bib (without --local), the code will try to pull
// a newer version of the container even if an older one is already present. This doesn't match
// how `podman run`` behaves by default, but it matches the bib's behaviour before the switch
// to using containers storage in all code paths happened.
// We might want to change this behaviour in the future to match podman.
if !localStorage {
if output, err := exec.Command("podman", "pull", "--arch", buildArch.String(), imgref).CombinedOutput(); err != nil {
if output, err := exec.Command("podman", "pull", "--arch", cntArch.String(), imgref).CombinedOutput(); err != nil {
return nil, nil, fmt.Errorf("failed to pull container image: %w\n%s", err, output)
}
}
Expand Down Expand Up @@ -295,7 +325,7 @@ func manifestFromCobra(cmd *cobra.Command, args []string) ([]byte, *mTLSConfig,
}

manifestConfig := &ManifestConfig{
Architecture: buildArch,
Architecture: cntArch,
Config: config,
BuildType: buildType,
Imgref: imgref,
Expand Down
24 changes: 24 additions & 0 deletions test/test_manifest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import platform
import subprocess
import textwrap

Expand Down Expand Up @@ -109,3 +110,26 @@ def test_manifest_local_checks_containers_storage_works(tmp_path, build_containe
f'--entrypoint=["/usr/bin/bootc-image-builder", "manifest", "--local", "localhost/{container_tag}"]',
build_container,
], check=True, encoding="utf8")


@pytest.mark.skipif(platform.uname().machine != "x86_64", reason="cross build test only runs on x86")
def test_manifest_cross_arch_check(tmp_path, build_container):
cntf_path = tmp_path / "Containerfile"
cntf_path.write_text(textwrap.dedent("""\n
# build for x86_64 only
FROM scratch
"""), encoding="utf8")

with make_container(tmp_path, arch="x86_64") as container_tag:
with pytest.raises(subprocess.CalledProcessError) as exc:
subprocess.run([
"podman", "run", "--rm",
"--privileged",
"-v", "/var/lib/containers/storage:/var/lib/containers/storage",
"--security-opt", "label=type:unconfined_t",
f'--entrypoint=["/usr/bin/bootc-image-builder", "manifest",\
"--target-arch=aarch64", "--local", \
"localhost/{container_tag}"]',
build_container,
], check=True, capture_output=True, encoding="utf8")
assert 'image found is for unexpected architecture "x86_64"' in exc.value.stderr

0 comments on commit 5abe18e

Please sign in to comment.