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 authored and kingsleyzissou committed Apr 23, 2024
1 parent 78f8c0d commit fd264fa
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 13 deletions.
47 changes: 34 additions & 13 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 already pulled 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 architecture: %w", util.OutputErr(err))
}
output := strings.TrimSpace(string(outputB))

// TODO: make images:arch.FromString() return an error
defer func() {
if panicErr := recover(); panicErr != 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 @@ -145,19 +164,13 @@ func makeManifest(c *ManifestConfig, cacheRoot string) (manifest.OSBuildManifest

// Resolve container - the normal case is that host and target
// architecture are the same. However it is possible to build
// cross-arch images by using qemu-user. Just run everything
// cross-arch images by using qemu-user. This will run everything
// (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()

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

containerSpecs := make(map[string][]container.Spec)
for plName, sourceSpecs := range manifest.GetContainerSourceSpecs() {
Expand Down Expand Up @@ -195,7 +208,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 +228,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 @@ -257,11 +270,19 @@ func manifestFromCobra(cmd *cobra.Command, args []string) ([]byte, *mTLSConfig,
// 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(), fmt.Sprintf("--tls-verify=%v", tlsVerify), imgref).CombinedOutput(); err != nil {
if output, err := exec.Command("podman", "pull", "--arch", cntArch.String(), fmt.Sprintf("--tls-verify=%v", tlsVerify), imgref).CombinedOutput(); err != nil {
return nil, nil, fmt.Errorf("failed to pull container image: %w\n%s", err, output)
}
}

// TODO: check arch compat before pulling
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)
}
cntSize, err := getContainerSize(imgref)
if err != nil {
return nil, nil, fmt.Errorf("cannot get container size: %w", err)
Expand Down Expand Up @@ -295,7 +316,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 fd264fa

Please sign in to comment.