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

Use supermin in unprivileged environments #190

Merged
merged 7 commits into from
Nov 6, 2018
Merged
Show file tree
Hide file tree
Changes from 5 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
6 changes: 5 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ FROM registry.fedoraproject.org/fedora:28
WORKDIR /root/containerbuild

# Only need a few of our scripts for the first few steps
COPY ./build.sh ./deps.txt ./build-deps.txt /root/containerbuild/
COPY ./build.sh ./deps.txt ./vmdeps.txt ./build-deps.txt /root/containerbuild/
RUN ./build.sh configure_yum_repos
RUN ./build.sh install_rpms

Expand All @@ -15,6 +15,10 @@ RUN ./build.sh configure_user
WORKDIR /srv/
RUN rm -rf /root/containerbuild

# allow writing to /etc/passwd from arbitrary UID
# https://docs.openshift.com/container-platform/3.10/creating_images/guidelines.html
RUN chmod g=u /etc/passwd

# run as `builder` user
USER builder
ENTRYPOINT ["/usr/bin/dumb-init", "/usr/bin/coreos-assembler"]
17 changes: 16 additions & 1 deletion coreos-assembler
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,24 @@ set -euo pipefail
# Currently this just wraps the two binaries we have today
# under a global entrypoint with subcommands.

# docker/podman don't run through PAM, but we want this set
# docker/podman don't run through PAM, but we want this set for the privileged
# (non-virtualized) path
export USER=${USER:-$(id -nu)}

# When trying to connect to libvirt we get "Failed to find user record
# for uid" errors if there is no entry for our UID in /etc/passwd.
# This was taken from 'Support Arbitrary User IDs' section of:
# https://docs.openshift.com/container-platform/3.10/creating_images/guidelines.html
if ! whoami &> /dev/null; then
# We need to make sure we set $HOME in the /etc/passwd file because
# if we don't libvirt will try to use `/` and we will get permission
# issues
export HOME="/var/tmp/${USER_NAME:-default}" && mkdir -p $HOME
Copy link
Member Author

Choose a reason for hiding this comment

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

@dustymabe I slightly modified your patch here to put it in /var/tmp instead of /tmp since the former is less likely to be on tmpfs.

Copy link
Member

Choose a reason for hiding this comment

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

sounds good

if [ -w /etc/passwd ]; then
echo "${USER_NAME:-default}:x:$(id -u):0:${USER_NAME:-default} user:${HOME}:/sbin/nologin" >> /etc/passwd
fi
fi

# Ensure we've unshared our mount namespace so
# the later umount doesn't affect the host potentially
if [ -e /sys/fs/selinux/status ]; then
Expand Down
17 changes: 4 additions & 13 deletions src/cmd-build
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ fi

previous_commit=
if [ -n "${ref:-}" ]; then
previous_commit=$(ostree --repo=${workdir}/repo rev-parse ${ref} || true)
previous_commit=$(ostree --repo=${workdir}/repo rev-parse ${ref} 2>/dev/null || true)
fi
# If the ref was unset or missing, look at the previous build
if [ -z "${previous_commit}" ] && [ -n "${previous_build}" ]; then
Expand Down Expand Up @@ -145,13 +145,13 @@ else
fi

if [ -n "${previous_build}" ]; then
rpm-ostree --repo=${workdir}/repo-build db diff ${previous_commit} ${commit}
rpm-ostree --repo=${workdir}/repo db diff ${previous_commit} ${commit}
fi

image_input_checksum=$((echo ${commit} && echo ${kickstart_checksum}) | sha256sum_str)
echo "New image input checksum: ${image_input_checksum}"
version=$(ostree --repo=${workdir}/repo-build show --print-metadata-key=version ${commit} | sed -e "s,',,g")
if [ "${previous_commit}" = "${commit}" ]; then
version=$(ostree --repo=${workdir}/repo show --print-metadata-key=version ${commit} | sed -e "s,',,g")
if [ "${previous_commit}" = "${commit}" ] && [ -n "${previous_image_genver:-}" ]; then
image_genver=$((${previous_image_genver} + 1))
buildid=${version}-${image_genver}
else
Expand All @@ -160,15 +160,6 @@ else
fi
echo "New build ID: ${buildid}"

# https://github.com/ostreedev/ostree/issues/1562#issuecomment-385393872
# The passwd files (among others) don't have world readability. This won't
# actually corrupt the repository as the *canonical* permissions are stored
# as xattrs. Probably what we should do is have an ostree option to specify
# a permission mask for objects.
sudo chmod -R a+rX ${workdir}/repo-build/objects
ostree --repo=${workdir}/repo pull-local ${workdir}/repo-build "${ref:-${commit}}"
ostree --repo=${workdir}/repo summary -u

# Generate JSON
if [ -n "${previous_commit}" ]; then
previous_commit_json='"'"${previous_commit}"'"'
Expand Down
5 changes: 2 additions & 3 deletions src/cmd-clean
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ prepare_build

# But go back to the toplevel
cd ${workdir}
# Note we don't prune the cache/ dir or the objects
# in the repos. If you want that, just rm -rf them.
# Note we don't prune the cache.qcow2 or the objects
# in the repo. If you want that, just rm -rf them.
rm -rf repo/refs/heads/* builds/* tmp/*
ostree --repo=repo summary -u
sudo rm -rf repo-build/refs/heads/*
12 changes: 9 additions & 3 deletions src/cmd-init
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,16 @@ if [ "$FORCE" != "1" -a ! -z "$(ls -A ./)" ]; then
fatal "init: current directory is not empty, override with --force"
fi

set -x
source=$1; shift
subdir=${1:-}

preflight

sudo chown $USER: .
if has_privileges; then
sudo chown $USER: .
elif [ ! -w . ]; then
fatal "init: running unprivileged, and current directory not writable"
fi

INSTALLER=https://download.fedoraproject.org/pub/fedora/linux/releases/28/Everything/x86_64/iso/Fedora-Everything-netinst-x86_64-28-1.1.iso
INSTALLER_CHECKSUM=https://download.fedoraproject.org/pub/fedora/linux/releases/28/Everything/x86_64/iso/Fedora-Everything-28-1.1-x86_64-CHECKSUM
Expand Down Expand Up @@ -116,4 +119,7 @@ mkdir -p cache
mkdir -p builds
mkdir -p tmp
ostree --repo=repo init --mode=archive
ostree --repo=repo-build init --mode=bare-user
if ! has_privileges; then
qemu-img create -f qcow2 cache/cache.qcow2 10G
LIBGUESTFS_BACKEND=direct virt-format --filesystem=xfs -a cache/cache.qcow2
fi
139 changes: 117 additions & 22 deletions src/cmdlib.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,32 @@
# Shared shell script library

DIR=$(dirname $0)

info() {
echo "info: $@" 1>&2
}

fatal() {
echo "error: $@" 1>&2; exit 1
info "$@"; exit 1
}

_privileged=
jlebon marked this conversation as resolved.
Show resolved Hide resolved
has_privileges() {
if [ -z "${_privileged:-}" ]; then
if [ -n "${FORCE_UNPRIVILEGED:-}" ]; then
info "Detected FORCE_UNPRIVILEGED; using virt"
_privileged=0
elif ! capsh --print | grep -q 'Current.*cap_sys_admin'; then
info "Missing CAP_SYS_ADMIN; using virt"
_privileged=0
elif ! sudo true; then
info "Missing sudo privs; using virt"
_privileged=0
else
_privileged=1
fi
fi
[ ${_privileged} == 1 ]
}

preflight() {
Expand All @@ -28,27 +53,25 @@ preflight() {
fatal "Unable to find /dev/kvm"
fi

if ! capsh --print | grep -q 'Current.*cap_sys_admin'; then
fatal "This container must currently be run with --privileged"
fi

if ! sudo true; then
fatal "The user must currently have sudo privileges"
fi

# permissions on /dev/kvm vary by (host) distro. If it's
# not writable, recreate it.
if ! [ -w /dev/kvm ]; then
sudo rm -f /dev/kvm
sudo mknod /dev/kvm c 10 232
sudo setfacl -m u:$USER:rw /dev/kvm
if ! has_privileges; then
fatal "running unprivileged, and /dev/kvm not writable"
else
sudo rm -f /dev/kvm
sudo mknod /dev/kvm c 10 232
sudo setfacl -m u:$USER:rw /dev/kvm
fi
fi
}

prepare_build() {
preflight
if ! [ -d repo ]; then
fatal "No $(pwd)/repo found; did you run coreos-assembler init?"
elif ! has_privileges && [ ! -f cache/cache.qcow2 ]; then
fatal "No cache.qcow2 found; did you run coreos-assembler init?"
fi

export workdir=$(pwd)
Expand Down Expand Up @@ -95,10 +118,6 @@ prepare_build() {
}

runcompose() {
local treecompose_args=""
if ! grep -q '^# disable-unified-core' "${manifest}"; then
treecompose_args="${treecompose_args} --unified-core"
fi
# Implement support for automatic local overrides:
# https://github.com/coreos/coreos-assembler/issues/118
local overridesdir=${workdir}/overrides/
Expand Down Expand Up @@ -126,10 +145,86 @@ EOF
fi

rm -f ${changed_stamp}
set -x
sudo rpm-ostree compose tree --repo=${workdir}/repo-build --cachedir=${workdir}/cache \
--touch-if-changed "${changed_stamp}" \
${treecompose_args} \
${TREECOMPOSE_FLAGS:-} ${manifest} "$@"
set +x

set - rpm-ostree compose tree --repo=${workdir}/repo \
--cachedir=${workdir}/cache --touch-if-changed "${changed_stamp}" \
${manifest} "$@"

if ! grep -q '^# disable-unified-core' "${manifest}"; then
set - "$@" --unified-core
Copy link
Member

Choose a reason for hiding this comment

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

colin had told me that I could drop this check so I did in my fork. i.e. it is safe to assume anything using coreos-assembler will be using unified core in the future.

fi

echo "Running: $@"

# this is the heart of the privs vs no privs dual path
if has_privileges; then
sudo "$@"
else
runvm "$@"
fi
}

runvm() {
local vmpreparedir=${workdir}/tmp/supermin.prepare
local vmbuilddir=${workdir}/tmp/supermin.build

# use REBUILDVM=1 if e.g. hacking on rpm-ostree/ostree and wanting to get
Copy link
Member

Choose a reason for hiding this comment

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

the problem with not building the vm every time is that it can get out of date (i.e. new deps needed, etc). This is probably fine for your local dev case, but not ideal for having to go do something in your CI. I converted my master branch to just rebuild it every time. I found it didn't really add that much extra time to the compose.

Copy link
Member

Choose a reason for hiding this comment

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

then again, if we are doing it for fetch and build then it starts to get a bit more heavyweight

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that's the thing. We're calling this now both on fetch and build. I'm open to dropping that logic if it causes problems, though offhand it's nice to be able to skip this also for the local dev case (where you're actually testing the unpriv path).

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 could make fetch unprivileged without too much effort BTW.

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 could make fetch unprivileged without too much effort BTW.

right, but if we are in "unpriv mode" we still need to run it in the VM so it can access the cache I think.

# the new bits in the VM
if [ ! -f ${vmbuilddir}/.done ] || [ -n "${REBUILDVM:-}" ]; then
rm -rf ${vmpreparedir} ${vmbuilddir}
mkdir -p ${vmpreparedir} ${vmbuilddir}

local rpms=
# then add all the base deps
for dep in $(grep -v '^#' ${DIR}/vmdeps.txt); do
rpms+="$dep "
done

supermin --prepare --use-installed $rpms -o "${vmpreparedir}"

# the reason we do a heredoc here is so that the var substition takes
# place immediately instead of having to proxy them through to the VM
cat > "${vmpreparedir}/init" <<EOF
#!/bin/bash
set -xeuo pipefail
workdir=${workdir}
$(cat ${DIR}/supermin-init-prelude.sh)
rc=0
sh ${TMPDIR}/cmd.sh || rc=\$?
echo \$rc > ${workdir}/tmp/rc
/sbin/fstrim -v ${workdir}/cache
/sbin/reboot -f
EOF
chmod a+x ${vmpreparedir}/init
(cd ${vmpreparedir} && tar -czf init.tar.gz --remove-files init)
supermin --build "${vmpreparedir}" --size 5G -f ext2 -o "${vmbuilddir}"
touch "${vmbuilddir}/.done"
fi

echo "$@" > ${TMPDIR}/cmd.sh
Copy link
Member

Choose a reason for hiding this comment

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

in this case this works because ${TMPDIR} is under ${workdir} right?

I've actually seen issues running in openshift where having $TMPDIR be on the nfs mounted volume caused real issues for me so I changed it to not be exported: dustymabe@7ecba34


# support local dev cases where src/config is a symlink
srcvirtfs=
if [ -L "${workdir}/src/config" ]; then
# qemu follows symlinks
srcvirtfs="-virtfs local,id=source,path=${workdir}/src/config,security_model=none,mount_tag=source"
fi

qemu-kvm -nodefaults -nographic -m 2048 -no-reboot \
-kernel "${vmbuilddir}/kernel" \
-initrd "${vmbuilddir}/initrd" \
-netdev user,id=eth0,hostname=supermin \
-device virtio-net-pci,netdev=eth0 \
-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \
-drive if=none,id=drive-scsi0-0-0-0,snapshot=on,file="${vmbuilddir}/root" \
-device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 \
-drive if=none,id=drive-scsi0-0-0-1,discard=unmap,file="${workdir}/cache/cache.qcow2" \
-device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=1,drive=drive-scsi0-0-0-1,id=scsi0-0-0-1 \
-virtfs local,id=workdir,path="${workdir}",security_model=none,mount_tag=workdir \
${srcvirtfs} -serial stdio -append "root=/dev/sda console=ttyS0 selinux=1 enforcing=0 autorelabel=1"

if [ ! -f ${workdir}/tmp/rc ]; then
fatal "Couldn't find rc file, something went terribly wrong!"
fi
return $(cat ${workdir}/tmp/rc)
}
3 changes: 3 additions & 0 deletions src/deps.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# For privileged ops
supermin

# We default to builder user, but sudo where necessary
sudo

Expand Down
1 change: 0 additions & 1 deletion src/prune_builds
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,3 @@ def ostree_prune(repo_name, sudo=False):
subprocess.run(argv, check=True)

ostree_prune('repo')
ostree_prune('repo-build', sudo=True)
27 changes: 27 additions & 0 deletions src/supermin-init-prelude.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
mount -t proc /proc /proc
mount -t sysfs /sys /sys
mount -t devtmpfs devtmpfs /dev

# load selinux policy
LANG=C /sbin/load_policy -i

# load kernel module for 9pnet_virtio for 9pfs mount
/sbin/modprobe 9pnet_virtio

# need fuse module for rofiles-fuse/bwrap during post scripts run
/sbin/modprobe fuse

# set up networking
/usr/sbin/dhclient eth0
jlebon marked this conversation as resolved.
Show resolved Hide resolved

# set up workdir
mkdir -p ${workdir}
mount -t 9p -o rw,trans=virtio,version=9p2000.L workdir ${workdir}
if [ -L ${workdir}/src/config ]; then
mkdir -p $(readlink ${workdir}/src/config)
mount -t 9p -o rw,trans=virtio,version=9p2000.L source ${workdir}/src/config
fi
mkdir -p ${workdir}/cache
mount /dev/sdb1 ${workdir}/cache

cd ${workdir}
16 changes: 16 additions & 0 deletions src/vmdeps.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Base deps for a viable VM environment.

# bare essentials
bash vim-minimal coreutils util-linux procps-ng kmod kernel-modules

# for composes
rpm-ostree distribution-gpg-keys jq

# for clean reboot
systemd

# networking
dhcp-client bind-export-libs iproute

# SELinux
selinux-policy selinux-policy-targeted policycoreutils
1 change: 1 addition & 0 deletions vmdeps.txt