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

Add support for mariner rootfs build scenarios #12900

Merged
merged 11 commits into from
Mar 23, 2023
Merged

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Mar 16, 2023

This makes a number of fixes to the rootfs build logic that enables running from a mariner host:

  • Require signing keys for ubuntu and alpine packages (this is an attempt to better follow secure supply chain practices).
    • The alpine keys are included with the script.
    • Ubuntu signing keys are expected to be available on the host container.
    • Add a new --skipsigcheck argument that can be used to work around the requirement for signature checking locally if needed.
  • Allow building x64 alpine rootfs.
  • Fall back on [Use] debootstrap since qemu-debootstrap isn't available in mariner (and is deprecated upstream).
    • This will also create a minimal sysroot when using debootstrap, and force package signature checking.
    • I didn't force signature checking for qemu-debootstrap to avoid breaking existing scenarios.
  • Verify checksum of apk-tools download.
  • Don't try to install libnuma-dev on xenial arm (the package wasn't available). This is similar to the logic which excludes numactl-dev on alpine 3.13 arm. We will need to decide how to address this dependency when we turn on mariner arm builds, but this change at least allows building a xenial arm sysroot.

Contributes to dotnet/runtime#83428

@sbomer
Copy link
Member Author

sbomer commented Mar 16, 2023

@am11 I'd appreciate your eyes on this too (I can't add you directly as a reviewer).

Copy link
Member

@am11 am11 left a comment

Choose a reason for hiding this comment

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

Overall LGTM, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

nit: if we could avoid adding new files and instead burn keys lines exactly like the official script does: https://github.com/alpinelinux/alpine-chroot-install/blob/6d08f12a8a70dd9b9dc7d997c88aa7789cc03c42/alpine-chroot-install#L85-L133, that would be cleaner. (we already have too many small fragmented files than necessary in this part of the infra).

if command -v qemu-debootstrap > /dev/null; then
__Debootstrap="qemu-debootstrap"
else
__Debootstrap="debootstrap"
Copy link
Member

Choose a reason for hiding this comment

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

The main consumer of this script is https://github.com/dotnet/dotnet-buildtools-prereqs-docker repo where it will always use the if branch. We should:

  • invert this condition because qemu-debootstrap gives deprecation warning and tells us to use debootstrap
  • move --keyring option in the conditions above which are missing it (riscv64 and armv6 already define --keyring and expect it to install it on the host)

Copy link
Member Author

Choose a reason for hiding this comment

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

I am planning to add new build images in dotnet-buildtools-prereqs-docker based on CBL-mariner, which will use the fall-back. I wanted to minimize changes to the existing build images (which are used for servicing for example), but I will give it a try.

Copy link
Member Author

@sbomer sbomer Mar 23, 2023

Choose a reason for hiding this comment

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

I have switched this to always use debootstrap with the minbase package set. I locally built all of the dotnet-buildtools-prereqs-docker images which build a rootfs and it seemed to work with the changes in dotnet/dotnet-buildtools-prereqs-docker#829.

For the mariner builds I will do something similar and place the ubuntu keyring at the location that debootstrap expects by default, so no changes to the other cases were needed.

Copy link
Member

Choose a reason for hiding this comment

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

I think we must not check in the full gpg key to git repo. As mentioned above, these keys are installed on the intended host system (docker builder layer primarily). The keys in the docker layer https://github.com/dotnet/dotnet-buildtools-prereqs-docker images are installed from Debian/Ubuntu servers, for which -- I believe -- the supply chain has a blanket trust.

Copy link
Member Author

Choose a reason for hiding this comment

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

The mariner docker images which I plan to add don't have the ubuntu keyring available. Any suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

If we are going to acquire https://src.fedoraproject.org/rpms/debootstrap in mariner's base image, we can also acquire https://src.fedoraproject.org/rpms/debian-keyring and use the keys this package provides. (Debian keys usually work for Ubuntu and vice-versa)

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed this to require the debootstrap keyring to be available on the host. It will make this script less convenient to use in a vanilla mariner environment which doesn't have the signing keys available (they are not available in a mariner package either, whereas debootstrap is). To make local development less painful I've added an argument to allow skipping signature checks (which was the default before this change).

@sbomer
Copy link
Member Author

sbomer commented Mar 23, 2023

@janvorli PTAL

Copy link
Member

@am11 am11 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

eng/common/cross/build-rootfs.sh Show resolved Hide resolved
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants