Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Remove inject_files rule invocations and move injection of binaries to ext4_image #3497

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 62 additions & 82 deletions ic-os/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ load("//ci/src/artifacts:upload.bzl", "upload_artifacts")
load("//ic-os/bootloader:defs.bzl", "build_grub_partition")
load("//ic-os/components:boundary-guestos.bzl", boundary_component_files = "component_files")
load("//ic-os/components/conformance_tests:defs.bzl", "component_file_references_test")
load("//toolchains/sysimage:toolchain.bzl", "build_container_base_image", "build_container_filesystem", "disk_image", "disk_image_no_tar", "ext4_image", "inject_files", "sha256sum", "tar_extract", "tree_hash", "upgrade_image")
load("//toolchains/sysimage:toolchain.bzl", "build_container_base_image", "build_container_filesystem", "disk_image", "disk_image_no_tar", "ext4_image", "sha256sum", "tar_extract", "tree_hash", "upgrade_image")

def icos_build(
name,
Expand Down Expand Up @@ -123,7 +123,7 @@ def icos_build(
build_args = image_deps["build_args"],
file_build_arg = image_deps["file_build_arg"],
target_compatible_with = ["@platforms//os:linux"],
tags = ["manual", "no-cache"],
tags = ["manual"],
)

# Extract SElinux file_contexts to use later when building ext4 filesystems
Expand All @@ -139,80 +139,86 @@ def icos_build(

# -------------------- Extract root partition --------------------

# Note that we defer injecting the files from images_deps["rootfs"]. These are mostly slower to build.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer true, right?


# NOTE: e2fsdroid does not support filenames with spaces, fortunately,
# there are only two in our build.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I guess we forgot to update this last time. Maybe better would be to say:

... fortunately, these only occur in firmware that we do not use

PARTITION_ROOT_STRIP_PATHS = [
"/run",
"/boot",
"/var",
"/usr/lib/firmware/brcm/brcmfmac43241b4-sdio.Intel Corp.-VALLEYVIEW C0 PLATFORM.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-TF103CE.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac43362-sdio.ASUSTeK COMPUTER INC.-ME176C.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac43430a0-sdio.ONDA-V80 PLUS.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac43455-sdio.MINIX-NEO Z83-4.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac43455-sdio.Raspberry Pi Foundation-Raspberry Pi 4 Model B.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac43455-sdio.Raspberry Pi Foundation-Raspberry Pi Compute Module 4.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac4356-pcie.Intel Corporation-CHERRYVIEW D1 PLATFORM.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac4356-pcie.Xiaomi Inc-Mipad2.txt.zst",
]
ext4_image(
name = "static-partition-root-unsigned.tzst",
name = "partition-root-unsigned.tzst",
src = ":rootfs-tree.tar",
file_contexts = ":file_contexts",
partition_size = image_deps["rootfs_size"],
# NOTE: e2fsdroid does not support filenames with spaces, fortunately,
# there are only two in our build.
strip_paths = [
"/run",
"/boot",
"/var",
"/usr/lib/firmware/brcm/brcmfmac43241b4-sdio.Intel Corp.-VALLEYVIEW C0 PLATFORM.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-TF103CE.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac43362-sdio.ASUSTeK COMPUTER INC.-ME176C.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac43430a0-sdio.ONDA-V80 PLUS.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac43455-sdio.MINIX-NEO Z83-4.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac43455-sdio.Raspberry Pi Foundation-Raspberry Pi 4 Model B.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac43455-sdio.Raspberry Pi Foundation-Raspberry Pi Compute Module 4.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac4356-pcie.Intel Corporation-CHERRYVIEW D1 PLATFORM.txt.zst",
"/usr/lib/firmware/brcm/brcmfmac4356-pcie.Xiaomi Inc-Mipad2.txt.zst",
],
strip_paths = PARTITION_ROOT_STRIP_PATHS,
extra_files = {
k: v
for k, v in (image_deps["rootfs"].items() + [(":version.txt", "/opt/ic/share/version.txt:0644")])
},
target_compatible_with = [
"@platforms//os:linux",
],
tags = ["manual"],
tags = ["manual", "no-cache"],
)

component_file_references_test(
name = name + "_component_file_references_test",
image = ":partition-root-unsigned.tzst",
component_files = image_deps["component_files"].keys(),
# Inherit tags for this test, to avoid triggering builds for local base images
tags = tags,
)

# -------------------- Extract boot partition --------------------

ext4_image(
name = "static-partition-boot.tzst",
name = "partition-boot.tzst",
src = ":rootfs-tree.tar",
file_contexts = ":file_contexts",
partition_size = image_deps["bootfs_size"],
subdir = "boot",
target_compatible_with = [
"@platforms//os:linux",
],
tags = ["manual"],
)

# Defer injection to this point to allow caching most of the built images
# -------------------- Inject extra files --------------------

inject_files(
name = "partition-root-unsigned.tzst",
testonly = malicious,
base = "static-partition-root-unsigned.tzst",
file_contexts = ":file_contexts",
extra_files = {
k: v
for k, v in (image_deps["rootfs"].items() + [(":version.txt", "/opt/ic/share/version.txt:0644")])
for k, v in (
image_deps["bootfs"].items() + [
(":version.txt", "/version.txt:0644"),
(":extra_boot_args", "/extra_boot_args:0644"),
]
)
},
tags = ["manual", "no-cache"],
)

# Inherit tags for this test, to avoid triggering builds for local base images
component_file_references_test(
name = name + "_component_file_references_test",
image = ":partition-root-unsigned.tzst",
component_files = image_deps["component_files"].keys(),
tags = tags,
)

if upgrades:
inject_files(
ext4_image(
name = "partition-root-test-unsigned.tzst",
testonly = malicious,
base = "static-partition-root-unsigned.tzst",
src = ":rootfs-tree.tar",
file_contexts = ":file_contexts",
partition_size = image_deps["rootfs_size"],
strip_paths = PARTITION_ROOT_STRIP_PATHS,
extra_files = {
k: v
for k, v in (image_deps["rootfs"].items() + [(":version-test.txt", "/opt/ic/share/version.txt:0644")])
},
target_compatible_with = [
"@platforms//os:linux",
],
tags = ["manual", "no-cache"],
)

Expand Down Expand Up @@ -271,29 +277,16 @@ def icos_build(
tags = ["manual"],
)

inject_files(
name = "partition-boot.tzst",
base = "static-partition-boot.tzst",
file_contexts = ":file_contexts",
prefix = "/boot",
extra_files = {
k: v
for k, v in (
image_deps["bootfs"].items() + [
(":version.txt", "/version.txt:0644"),
(":extra_boot_args", "/extra_boot_args:0644"),
]
)
},
tags = ["manual", "no-cache"],
)

if upgrades:
inject_files(
ext4_image(
name = "partition-boot-test.tzst",
base = "static-partition-boot.tzst",
src = ":rootfs-tree.tar",
file_contexts = ":file_contexts",
prefix = "/boot",
partition_size = image_deps["bootfs_size"],
subdir = "boot",
target_compatible_with = [
"@platforms//os:linux",
],
extra_files = {
k: v
for k, v in (
Expand Down Expand Up @@ -671,7 +664,7 @@ def boundary_node_icos_build(
build_args = image_deps["build_args"],
file_build_arg = image_deps["file_build_arg"],
target_compatible_with = ["@platforms//os:linux"],
tags = ["manual", "no-cache"],
tags = ["manual"],
)

# Helpful tool to print a hash of all input component files
Expand Down Expand Up @@ -717,20 +710,13 @@ EOF
)

ext4_image(
name = "static-partition-boot.tzst",
name = "partition-boot.tzst",
src = ":rootfs-tree.tar",
partition_size = "1G",
subdir = "boot/",
target_compatible_with = [
"@platforms//os:linux",
],
tags = ["manual"],
)

inject_files(
name = "partition-boot.tzst",
base = "static-partition-boot.tzst",
prefix = "/boot",
extra_files = {
k: v
for k, v in (
Expand All @@ -744,27 +730,21 @@ EOF
)

ext4_image(
name = "static-partition-root-unsigned.tzst",
name = "partition-root-unsigned.tzst",
src = ":rootfs-tree.tar",
partition_size = "3G",
strip_paths = [
"/run",
"/boot",
],
tags = ["manual"],
target_compatible_with = [
"@platforms//os:linux",
],
)

inject_files(
name = "partition-root-unsigned.tzst",
base = "static-partition-root-unsigned.tzst",
extra_files = {
k: v
for k, v in (image_deps["rootfs"].items() + [(":version.txt", "/opt/ic/share/version.txt:0644")])
},
tags = ["manual", "no-cache"],
target_compatible_with = [
"@platforms//os:linux",
],
)

native.genrule(
Expand Down
59 changes: 29 additions & 30 deletions toolchains/sysimage/build_ext4_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def limit_file_contexts(file_contexts, base_path):
continue
if base_path:
if line.startswith(base_path):
lines.append(line[len(base_path) :])
lines.append(line[len(base_path):])
else:
lines.append(line)
return "\n".join(lines) + "\n"
Expand Down Expand Up @@ -105,41 +105,37 @@ def strip_files(fs_basedir, fakeroot_statefile, strip_paths):
)


def prepare_tree_from_tar(in_file, fakeroot_statefile, fs_basedir, dir_to_extract):
def prepare_tree_from_tar(in_file, fakeroot_statefile, fs_basedir, dir_to_extract, extra_files):
# We batch all commands together and run them under bash. This is significantly faster than invoking fakeroot
# multiple times.
commands = "set -euo pipefail\n"
if in_file:
subprocess.run(
[
"fakeroot",
"-s",
fakeroot_statefile,
"tar",
"xf",
in_file,
"--numeric-owner",
"-C",
fs_basedir,
dir_to_extract,
],
check=True,
)
# Untar files to the base dir.
commands += f"""tar xf {in_file} --numeric-owner -C "{fs_basedir}" "{dir_to_extract}";\n"""

# Copy extra files to the base dir and set permissions.
for path_target in extra_files or []:
(path, target, mod) = path_target.split(":")
target_in_basedir = os.path.join(fs_basedir, dir_to_extract, target.lstrip("/"))
commands += f"""cp "{path}" "{target_in_basedir}";\n"""
commands += f"""chmod "{mod}" "{target_in_basedir}";\n"""
else:
subprocess.run(
[
"fakeroot",
"-s",
fakeroot_statefile,
"chown",
"root:root",
fs_basedir,
],
check=True,
)
commands += f"""chown root:root "{fs_basedir}";\n"""

subprocess.run(
["fakeroot",
"-s",
fakeroot_statefile,
"bash"],
input=commands.encode(), check=True)


def make_argparser():
parser = argparse.ArgumentParser()
parser.add_argument("-s", "--size", help="Size of image to build", type=str)
parser.add_argument("-o", "--output", help="Target (tzst) file to write partition image to", type=str)
parser.add_argument("--extra-files", help="Extra files to inject into the image. "
"Format: source_path:target_path_in_image:target_permissions", nargs='*')
parser.add_argument(
"-i", "--input", help="Source (tar) file to take files from", type=str, default="", required=False
)
Expand Down Expand Up @@ -179,6 +175,7 @@ def main():
out_file = args.output
image_size = args.size
limit_prefix = args.path
extra_files = args.extra_files
file_contexts_file = args.file_contexts
strip_paths = args.strip_paths
if limit_prefix and limit_prefix[0] == "/":
Expand Down Expand Up @@ -206,7 +203,7 @@ def main():
# Prepare a filesystem tree that represents what will go into
# the fs image. Wrap everything in fakeroot so permissions and
# ownership will be preserved while unpacking (see below).
prepare_tree_from_tar(in_file, fakeroot_statefile, fs_basedir, limit_prefix)
prepare_tree_from_tar(in_file, fakeroot_statefile, fs_basedir, limit_prefix, extra_files)
strip_files(fs_basedir, fakeroot_statefile, strip_paths)
subprocess.run(["sync"], check=True)

Expand All @@ -221,6 +218,8 @@ def main():
"hash_seed=c61251eb-100b-48fe-b089-57dea7368612",
"-U",
"clear",
"-d",
os.path.join(fs_basedir, limit_prefix),
Comment on lines +221 to +222
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as #3476 (comment), we should make sure to investigate just in case.

"-F",
image_file,
str(image_size),
Expand Down Expand Up @@ -258,7 +257,7 @@ def main():
e2fsdroid_args += ["-C", fs_config_path]
if file_contexts_file:
e2fsdroid_args += ["-S", file_contexts_file]
e2fsdroid_args += ["-f", os.path.join(fs_basedir, limit_prefix), image_file]
e2fsdroid_args += [image_file]
subprocess.run(e2fsdroid_args, check=True, env={"E2FSPROGS_FAKE_TIME": "0"})

subprocess.run(["sync"], check=True)
Expand Down
10 changes: 10 additions & 0 deletions toolchains/sysimage/toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,12 @@ def _ext4_image_impl(ctx):
if len(ctx.attr.strip_paths) > 0:
args += ["--strip-paths"] + ctx.attr.strip_paths

if ctx.attr.extra_files.items():
args.append("--extra-files")
for input_target, install_target in ctx.attr.extra_files.items():
args.append(input_target.files.to_list()[0].path + ":" + install_target)
inputs += input_target.files.to_list()

_run_with_icos_wrapper(
ctx,
executable = tool.path,
Expand Down Expand Up @@ -370,6 +376,10 @@ ext4_image = _icos_build_rule(
"subdir": attr.string(
default = "/",
),
"extra_files": attr.label_keyed_string_dict(
allow_files = True,
mandatory = False,
),
"_build_ext4_image": attr.label(
allow_files = True,
default = ":build_ext4_image.py",
Expand Down
Loading