Re: [XEN PATCH v1 6/6] CI: Run the builds and tests that use the Debian 12 containers as a normal user

[Javi Merino <javi.merino@xxxxxxxxx>]

Hi Stefano,

On Thu, Oct 24, 2024 at 04:47:28PM -0700, Stefano Stabellini wrote:
> On Thu, 24 Oct 2024, Javi Merino wrote:
> > Use FF_DISABLE_UMASK_FOR_DOCKER_EXECUTOR so that GitLab CI clones xen
> > using the user in the image, instead of as root[0].
> > 
> > In qemu-smoke-dom0*.sh and qemu-alpine-x86_64.sh, use fakeroot to
> > create the rootfs images that untar a tarball that create character
> > devices.  cpio replicates the block and character devices, as well as
> > preserving the uid and gid it sees in the current directory.  fakeroot
> > lets tar think that it is creating block and character devices, and
> > all files are owned by root, but it is all smokes and mirrors for
> > cpio.
> > 
> > [0] https://gitlab.com/gitlab-org/gitlab-runner/-/issues/1736
> > 
> > Signed-off-by: Javi Merino <javi.merino@xxxxxxxxx>
> > ---
> > 
> > Regarding building the rootfs, I have chosen to use a fakeroot
> > subshell for the entire process.  automation/scripts/qubes-x86-64.sh
> > takes a different approach, it just uses fakeroot for the tar/cpio
> > commands.  I prefer to do it this way but I am happy to be overridden
> > if `fakeroot -s ../save tar` and `fakeroot -i ../save cpio` are
> > preferred.
> > 
> >  automation/build/debian/12-arm64v8.dockerfile   | 5 ++++-
> >  automation/build/debian/12-x86_64.dockerfile    | 5 ++++-
> >  automation/gitlab-ci/test.yaml                  | 4 ++++
> >  automation/scripts/qemu-alpine-x86_64.sh        | 4 +++-
> >  automation/scripts/qemu-smoke-dom0-arm64.sh     | 7 +++++--
> >  automation/scripts/qemu-smoke-dom0less-arm64.sh | 5 +++--
> >  6 files changed, 23 insertions(+), 7 deletions(-)
> > 
> > diff --git a/automation/build/debian/12-arm64v8.dockerfile 
> > b/automation/build/debian/12-arm64v8.dockerfile
> > index 4da1b074aedb..c2617956ed77 100644
> > --- a/automation/build/debian/12-arm64v8.dockerfile
> > +++ b/automation/build/debian/12-arm64v8.dockerfile
> > @@ -10,6 +10,8 @@ RUN <<EOF
> >  #!/bin/bash
> >      set -eu
> >  
> > +    useradd --create-home user
> > +
> >      apt-get update
> >      DEPS=(
> >          # Xen
> > @@ -53,6 +55,7 @@ RUN <<EOF
> >          curl
> >          device-tree-compiler
> >          expect
> > +        fakeroot
> >          u-boot-qemu
> >          # for imagebuilder
> >          file
> > @@ -64,5 +67,5 @@ RUN <<EOF
> >      rm -rf /var/lib/apt/lists*
> >  EOF
> >  
> > -USER root
> > +USER user
> >  WORKDIR /build
> > diff --git a/automation/build/debian/12-x86_64.dockerfile 
> > b/automation/build/debian/12-x86_64.dockerfile
> > index e0ca8b7e9c91..98b23ea3eaa4 100644
> > --- a/automation/build/debian/12-x86_64.dockerfile
> > +++ b/automation/build/debian/12-x86_64.dockerfile
> > @@ -10,6 +10,8 @@ RUN <<EOF
> >  #!/bin/bash
> >      set -eu
> >  
> > +    useradd --create-home user
> > +
> >      apt-get update
> >      DEPS=(
> >          # Xen
> > @@ -54,6 +56,7 @@ RUN <<EOF
> >          # for qemu-alpine-x86_64-gcc
> >          busybox-static
> >          cpio
> > +        fakeroot
> >  
> >          # For *-efi jobs
> >          ovmf
> > @@ -64,5 +67,5 @@ RUN <<EOF
> >      rm -rf /var/lib/apt/lists*
> >  EOF
> >  
> > -USER root
> > +USER user
> >  WORKDIR /build
> 
> This breaks the xilinx hardware jobs both arm and x86 as they
> require root inside the container at the moment
> 
> 
> > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> > index 42baa82fe36f..71f2beb68c4f 100644
> > --- a/automation/gitlab-ci/test.yaml
> > +++ b/automation/gitlab-ci/test.yaml
> > @@ -1,6 +1,10 @@
> >  .test-jobs-common:
> >    stage: test
> >    image: registry.gitlab.com/xen-project/xen/${CONTAINER}
> > +  variables:
> > +    # Clone xen as the user in the docker images, not root
> > +    # See https://gitlab.com/gitlab-org/gitlab-runner/-/issues/1736
> > +    FF_DISABLE_UMASK_FOR_DOCKER_EXECUTOR: true
> >  
> >  .arm64-test-needs: &arm64-test-needs
> >    - alpine-3.18-arm64-rootfs-export
> > diff --git a/automation/scripts/qemu-alpine-x86_64.sh 
> > b/automation/scripts/qemu-alpine-x86_64.sh
> > index 1ff689b577e3..2660403ab2b8 100755
> > --- a/automation/scripts/qemu-alpine-x86_64.sh
> > +++ b/automation/scripts/qemu-alpine-x86_64.sh
> > @@ -29,6 +29,7 @@ find . | cpio --create --format='newc' | gzip > 
> > ../initrd.cpio.gz
> >  cd ..
> >  
> >  # initrd.tar.gz is Dom0 rootfs
> > +fakeroot <<EOF
> >  mkdir -p rootfs
> >  cd rootfs
> >  tar xvzf ../initrd.tar.gz
> > @@ -63,7 +64,8 @@ chmod +x etc/local.d/xen.start
> >  echo "rc_verbose=yes" >> etc/rc.conf
> >  # rebuild Dom0 rootfs
> >  find . |cpio -H newc -o|gzip > ../xen-rootfs.cpio.gz
> > -cd ../..
> > +EOF
> > +cd ..
> 
> I admit I am not a fan of this as it makes the script harder to read.
> Given that almost everything on this script and similar scripts is
> better run as root because it is all about repackaging cpio archivies,
> instead I would do this:
> 
> diff --git a/automation/scripts/qemu-alpine-x86_64.sh 
> b/automation/scripts/qemu-alpine-x86_64.sh
> index 2660403ab2..7c0ec01e05 100755
> --- a/automation/scripts/qemu-alpine-x86_64.sh
> +++ b/automation/scripts/qemu-alpine-x86_64.sh
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!/usr/bin/fakeroot
>  
>  set -ex -o pipefail

Running the entire script as a fakeroot subshell is ugly and not
necessary.

A better fix is what I suggested under the commit message, which is
also what the qubes containers do:

--- a/automation/scripts/qemu-alpine-x86_64.sh
+++ b/automation/scripts/qemu-alpine-x86_64.sh
@@ -31,7 +31,7 @@ cd ..
 # initrd.tar.gz is Dom0 rootfs
 mkdir -p rootfs
 cd rootfs
-tar xvzf ../initrd.tar.gz
+fakeroot -s ../fakeroot-save tar xvzf ../initrd.tar.gz
 mkdir proc
 mkdir run
 mkdir srv
@@ -62,7 +62,7 @@ xl create -c /root/test.cfg
 chmod +x etc/local.d/xen.start
 echo "rc_verbose=yes" >> etc/rc.conf
 # rebuild Dom0 rootfs
-find . |cpio -H newc -o|gzip > ../xen-rootfs.cpio.gz
+find . | fakeroot -i ../fakeroot-save cpio -H newc -o|gzip > 
../xen-rootfs.cpio.gz
 cd ../..

 cat >> binaries/pxelinux.0 << EOF


Similar for the dom0 and dom0less scripts and for the xilinx scripts.

> Keeping in mind that anyone could push a branch without fakeroot to
> their personal tree triggering a gitlab-ci pipeline, the advantage of
> using fakeroot would be if we force the container execution envinronment
> (gitlab runner) to run containers as user. This is not currently the
> configuration we have. As of now, it doesn't bring an advantage.
> 
> Given that the gitlab runners are in flux at the moment, and that this
> patch cannot work with the xilinx runners, I would ask you to please
> hold on on this patch until the gitlab runners are settled (~1 month).

Ok, I will hold the patch.  I will also fix the xilinx scripts.

Cheers,
Javi