Skip to content

Commit

Permalink
tasks: Bind the host's podman API socket
Browse files Browse the repository at this point in the history
This paves the way for spawning per-job tasks containers from the
container (via `job-runner`).

Getting the permissions right is unfortunately annoyingly complicated, as the
host's socket has 660 permissions, but the tasks container runs as uid 1111.
Ideally we could use something like

    -v "${XDG_RUNTIME_DIR:-/run}/podman/podman.sock:/podman.sock:idmap=gids=$(id -g)-1111-1"

but that fails with "invalid mappings", and is generally not well documented.
`--mount=type=bind,[...],idmap --uidmap [...]` does not work either.

So resort to file permissions for the host's `podman.sock`:
 - For local developers in `run-local.sh`, add an ACL for user 1111.
   This does not hurt too much for a human developer: The socket itself
   may be accessible to uid 1111 (which *might* be an untrusted local
   user), but its directory  (/run/user/uid) is not.

 - This works fine locally, but for some yet unexplained reason not in
   GitHub workflows. For now just hack the permissions to 666 there, we
   really don't care.

 - For production, `setfacl` (i.e. the `acl` package) is unfortunately
   not installed in Fedora IoT/CoreOS. Just set the group to `1111`
   there. That doesn't matter much, the secrets are all already chmod'ed
   to the container user, and these machines don't do anything else.
  • Loading branch information
martinpitt committed Mar 4, 2024
1 parent c118069 commit e6091cd
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 1 deletion.
15 changes: 15 additions & 0 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,21 @@ jobs:
git config user.email github-actions@github.com
git rebase origin/main
# HACK: Ubuntu 22.04 has podman 3.4, which isn't compatible with podman-remote 4 in our tasks container
# This PPA is a backport of podman 4.3 from Debian 12; drop this when moving `runs-on:` to ubuntu-24.04
- name: Update to newer podman
run: |
sudo add-apt-repository -y ppa:quarckster/containers
sudo apt install -y podman
systemctl --user daemon-reload
# HACK: run-local.sh tries to do this with setfacl, which works on a developer
# machine but isn't enough on Ubuntu 22.04 (GH workflow)
- name: Allow tasks container to access podman socket
run: |
systemctl --user start podman.socket
chmod o+rw $XDG_RUNTIME_DIR/podman/podman.sock
- name: Check which containers changed
id: containers_changed
run: |
Expand Down
6 changes: 6 additions & 0 deletions tasks/install-service
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ touch "$IMAGE_STORES"
cat <<EOF > /etc/systemd/system/cockpit-tasks@.service
[Unit]
Description=Cockpit Tasks %i
Requires=podman.socket
After=podman.socket
[Service]
Environment="TEST_JOBS=${TEST_JOBS:-8}"
Expand All @@ -58,13 +60,17 @@ ExecStartPre=-/usr/bin/podman network rm cockpit-tasks-%i
ExecStartPre=/usr/bin/chcon -R -l s0 \${TEST_CACHE}/images/
ExecStartPre=/usr/bin/flock /tmp/cockpit-image-pull podman pull quay.io/cockpit/tasks
ExecStartPre=/usr/bin/podman network create cockpit-tasks-%i
# idmapped mount would be better, but did not figure out how
ExecStartPre=/usr/bin/chgrp 1111 %t/podman/podman.sock
ExecStart=/usr/bin/podman run --name=cockpit-tasks-%i --hostname=${CONTAINER_HOSTNAME} \
--device=/dev/kvm --network=cockpit-tasks-%i \
--memory=24g --pids-limit=16384 --shm-size=1024m ${TMPVOL:-} \
--volume=\${TEST_CACHE}/images:/cache/images:rw \
--volume=\${TEST_SECRETS}/tasks:/secrets:ro \
--volume=\${TEST_SECRETS}/webhook:/run/secrets/webhook:ro \
--volume=${IMAGE_STORES}:/work/.config/cockpit-dev/image-stores:ro \
--volume=%t/podman/podman.sock:/podman.sock:rw \
--env=CONTAINER_HOST=unix:///podman.sock \
--env=NPM_REGISTRY=\${NPM_REGISTRY} \
--env=NODE_EXTRA_CA_CERTS=${NODE_EXTRA_CA_CERTS:-} \
--env=TEST_JOBS=\${TEST_JOBS} \
Expand Down
22 changes: 21 additions & 1 deletion tasks/run-local.sh
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,13 @@ EOF
ln -s ..data/s3-keys--localhost.localdomain tasks/s3-keys--localhost.localdomain
)

# need to make files world-readable, as containers run as different user
# start podman API
systemctl $([ $(id -u) -eq 0 ] || echo "--user") start podman.socket

# need to make files world-readable, as containers run as different user 1111
chmod -R go+rX "$SECRETS"
# for the same reason, make podman socket accessible to that container user
setfacl -m user:1111:rw ${XDG_RUNTIME_DIR:-/run}/podman/podman.sock
fi
}

Expand Down Expand Up @@ -170,6 +175,8 @@ EOF
podman run -d -it --name cockpituous-tasks --pod=cockpituous \
-v "$SECRETS"/tasks:/secrets:ro,z \
-v "$SECRETS"/webhook:/run/secrets/webhook:ro,z \
-v "${XDG_RUNTIME_DIR:-/run}/podman/podman.sock:/podman.sock" \
-e CONTAINER_HOST=unix:///podman.sock \
-e COCKPIT_CA_PEM=/run/secrets/webhook/ca.pem \
-e COCKPIT_BOTS_REPO=${COCKPIT_BOTS_REPO:-} \
-e COCKPIT_BOTS_BRANCH=${COCKPIT_BOTS_BRANCH:-} \
Expand All @@ -186,6 +193,9 @@ cleanup_containers() {
# clean up dummy token, so that image-prune does not try to use it
rm "$SECRETS"/webhook/.config--github-token

# remove podman socket ACL
setfacl -b ${XDG_RUNTIME_DIR:-run}/podman/podman.sock

if [ -n "$INTERACTIVE" ]; then
podman stop cockpituous-tasks
else
Expand Down Expand Up @@ -289,6 +299,15 @@ test_queue() {
echo "$OUT" | grep -q 'queue public is empty'
}

test_podman() {
# tasks can connect to host's podman service
# this will be covered implicitly by job-runner, but as a more basal plumbing test this is easier to debug
out="$(podman exec -i cockpituous-tasks podman-remote ps)"
assert_in 'cockpituous-tasks' "$out"
out="$(podman exec -i cockpituous-tasks podman-remote run -it --rm quay.io/cockpit/tasks:latest whoami)"
assert_in '^user' "$out"
}

#
# main
#
Expand All @@ -311,6 +330,7 @@ else
# tests which don't need GitHub interaction
test_image
test_queue
test_podman
# if we have a PR number, run a unit test inside local deployment, and update PR status
[ -z "$PR" ] || test_pr
fi
Expand Down

0 comments on commit e6091cd

Please sign in to comment.