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 18, 2024
1 parent 25e17fc commit 9d23cd9
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 9 deletions.
5 changes: 5 additions & 0 deletions bib/cmd/bootc-image-builder/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ type ManifestConfig struct {
func Manifest(c *ManifestConfig) (*manifest.Manifest, error) {
rng := createRand()

// this is currently mostly here so that tests need less setup
if c.Architecture == arch.ARCH_UNSET {
c.Architecture = arch.Current()
}

switch c.BuildType {
case BuildTypeDisk:
return manifestForDiskImage(c, rng)
Expand Down
24 changes: 17 additions & 7 deletions bib/cmd/bootc-image-builder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,25 +178,35 @@ 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() {
for _, c := range sourceSpecs {
resolver.Add(c)
}
containerSpecs[plName], err = resolver.Finish()

specs, err := resolver.Finish()
if err != nil {
return nil, err
}
for _, spec := range specs {
// XXX: check go arch names vs container arch names (amd64 vs x86_64)
if spec.Arch != wantedArch {
return nil, fmt.Errorf("image found is for unexpected architecture %q (expected %q), if that is intentional, please make sure --target-arch matches", spec.Arch, wantedArch)
}
}
containerSpecs[plName] = specs
}

mf, err := manifest.Serialize(depsolvedSets, containerSpecs, nil, nil)
Expand Down
3 changes: 2 additions & 1 deletion bib/cmd/bootc-image-builder/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

main "github.com/osbuild/bootc-image-builder/bib/cmd/bootc-image-builder"
"github.com/osbuild/images/pkg/blueprint"
"github.com/osbuild/images/pkg/container"
"github.com/osbuild/images/pkg/manifest"
"github.com/osbuild/images/pkg/rpmmd"

main "github.com/osbuild/bootc-image-builder/bib/cmd/bootc-image-builder"
)

func TestCanChownInPathHappy(t *testing.T) {
Expand Down
6 changes: 5 additions & 1 deletion test/containerbuild.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,16 @@


@contextmanager
def make_container(container_path):
def make_container(container_path, arch=None):
# BIB only supports container tags, not hashes
container_tag = "bib-test-" + "".join(random.choices(string.digits, k=12))
arch_cmd = []
if arch:
arch_cmd = ["--arch", arch]
subprocess.check_call([
"podman", "build",
"-t", container_tag,
*arch_cmd,
container_path], encoding="utf8")
yield container_tag
subprocess.check_call(["podman", "rmi", container_tag])
Expand Down
24 changes: 24 additions & 0 deletions test/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,27 @@ 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(f"""\n
FROM scratch
"""), encoding="utf8")

# XXX: check go arch names vs container arch names (amd64 vs x86_64)
with make_container(tmp_path, arch="x86_64") as container_tag:
with pytest.raises(Exception) 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, encoding="utf8")
# XXX: make more precise
assert "unexpected architecture" in str(e.value)

0 comments on commit 9d23cd9

Please sign in to comment.