Skip to content
Open
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
117 changes: 99 additions & 18 deletions tests/kola/systemd/sysexts
Original file line number Diff line number Diff line change
@@ -1,34 +1,117 @@
#!/bin/bash
## kola:
## # Marked as exclusive as we layer content over /usr with the sysexts
## # Marked as exclusive as we layer content over /usr with sysexts
## exclusive: true
## # No sysexts exists for RHCOS right now
## # Limited to FCOS for now as we don't have easy access to repos on RHCOS
## distros: fcos
## # Limited to x86_64 & aarch64 for now (see inline TODO)
## architectures: "x86_64 aarch64"
## # Should work the same on all platforms
## # Needs internet access to download the sysexts
## # Needs internet access to download RPMs to build sysexts
## tags: "platform-independent needs-internet"
## description: Verify setting up and enabling systemd system extensions
## description: Verify building, setting up and enabling systemd system extensions (sysexts)

set -xeuo pipefail

# shellcheck disable=SC1091
. "$KOLA_EXT_DATA/commonlib.sh"

# Uses sysexts from https://extensions.fcos.fr until we have official sysexts
# available in Fedora.
SYSEXTS_BASE_URL="https://extensions.fcos.fr/fedora"
# Install tools that we need to build the sysexts
rpm-ostree install --apply-live erofs-utils lz4

build_sysext(){
local -r rpm="${1}"

tmpdir="/tmp/sysext-${rpm}"
mkdir "${tmpdir}"
Comment on lines +25 to +26

Choose a reason for hiding this comment

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

medium

Using a fixed temporary directory name can lead to conflicts and is a security risk. It's better to use mktemp -d to create a unique and secure temporary directory.

Suggested change
tmpdir="/tmp/sysext-${rpm}"
mkdir "${tmpdir}"
tmpdir=$(mktemp -d "/tmp/sysext-${rpm}.XXXXXX")

pushd "${tmpdir}" > /dev/null

mkdir -p "rpms"
pushd "rpms" > /dev/null

# Download RPMs:
# - Resolve dependency relative to the current root
# - Only get packages for the current arch and arch independent ones
# - Disable the OpenH264 repo as it's a frequent source of flakes
dnf download \
--resolve \
--arch="noarch" \
--arch="$(arch)" \
-disablerepo=fedora-cisco-openh264 \
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
-disablerepo=fedora-cisco-openh264 \
--disablerepo=fedora-cisco-openh264 \

"${rpm}"

# Figure out version to use
pkg="$(ls ${rpm}-*.rpm | sort -h | head -1)"

Choose a reason for hiding this comment

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

high

The sort -h command is for sorting human-readable numbers (e.g., 2K, 1G), not version strings. For correct version sorting, you should use sort -V. To get the latest version, you should select the last item after sorting, not the first. Using ls -1 is also recommended in scripts to ensure one file per line.

Suggested change
pkg="$(ls ${rpm}-*.rpm | sort -h | head -1)"
pkg="$(ls -1 ${rpm}-*.rpm | sort -V | tail -n 1)"

epoch="$(rpm -qp --queryformat '%{EPOCH}' ${pkg})"
version="$(rpm -qp --queryformat '%{VERSION}-%{RELEASE}' ${pkg})"
if [[ "${epoch}" == "(none)" ]]; then
epoch=""
else
epoch="${epoch}-"
fi
version="${epoch}${version}"

popd > /dev/null

mkdir rootfs
pushd rootfs > /dev/null

# Post process architecture to match systemd architecture list
# TODO: Figure out the mapping for other architectures
if [[ "$(arch)" == "x86_64" ]]; then
arch="x86-64"
elif [[ "$(arch)" == "aarch64" ]]; then
arch="arm64"
else
echo "Unsupported architecture"
exit 1
fi

id="$(source /etc/os-release; echo "${ID}")"
version_id="$(source /etc/os-release; echo "${VERSION_ID}")"

# Write extension config file
install -d -m0755 usr/lib/extension-release.d
{
echo "ID=\"${id}\""
echo "VERSION_ID=\"${version_id}\""
echo "ARCHITECTURE=\"${arch}\""
} | tee "usr/lib/extension-release.d/extension-release.${rpm}"

# Extract the RPMs
for r in ../rpms/*.rpm; do
echo "Extracting: $(basename ${r})"
rpm2cpio "${r}" > ${r}.tar
cpio -idmv &> /dev/null < ${r}.tar
rm ${r}.tar
done

# Reset SELinux labels
filecontexts="/etc/selinux/targeted/contexts/files/file_contexts"
setfiles -r . ${filecontexts} . && chcon --user=system_u --recursive .

popd > /dev/null

# Create the EROFS image
name="${rpm}-${version}-${version_id}-${arch}.raw"
mkfs.erofs -zlz4 "${name}" rootfs
mv "${name}" /tmp
Comment on lines +95 to +98
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 you don't have to make it a .raw image (it can just be a directory/fs tree of files.

I think you know this and your intent here was to test the .raw because that's more like what users are going to be using (i.e. downloading a .raw and using that extension), but just in case wanted to point it out that this step may not be needed if you didn't have testing that use case specifically in mind.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Maybe I should test both. I'll do that in a followup.


popd > /dev/null

Choose a reason for hiding this comment

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

medium

The temporary directory created for the build is not removed, which will leave files in /tmp. It's important to clean up this directory after it's no longer needed.

Suggested change
popd > /dev/null
popd > /dev/null
rm -rf "${tmpdir}"

}

build_sysext "python3"

# Setup folders
install -d -m 0755 -o 0 -g 0 /var/lib/extensions /var/lib/extensions.d
restorecon -RFv /var/lib/extensions /var/lib/extensions.d
systemctl enable --now systemd-sysext.service

install_sysext() {
local -r SYSEXT="${1}"
install -d -m 0755 -o 0 -g 0 "/etc/sysupdate.${SYSEXT}.d"
restorecon -RFv "/etc/sysupdate.${SYSEXT}.d"
curl --silent --fail --location "${SYSEXTS_BASE_URL}/${SYSEXT}.conf" \
| tee "/etc/sysupdate.${SYSEXT}.d/${SYSEXT}.conf"
/usr/lib/systemd/systemd-sysupdate update --component "${SYSEXT}"
local -r name="${1}"
mv "/tmp/${name}"*".raw" "/var/lib/extensions.d"
ln -snf "/var/lib/extensions.d/${name}"*".raw" "/var/lib/extensions/${name}.raw"
Comment on lines +112 to +113

Choose a reason for hiding this comment

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

medium

Using globs (*) with mv and ln can be risky. If the glob expands to zero, or more than one file, these commands can fail or have unintended consequences. It's safer to verify that the glob matches exactly one file before proceeding.

For example:

local files=(/path/to/"${name}"*.raw)
if [ "${#files[@]}" -ne 1 ]; then
    fatal "Expected 1 file, found ${#files[@]}"
fi
# Now use "${files[0]}"
safely

Please apply this pattern to both the mv and ln commands.

restorecon -RFv "/var/lib/extensions.d" "/var/lib/extensions"
}

install_sysext python3
Expand All @@ -45,11 +128,9 @@ if [[ "$(/usr/bin/python3 -c 'print("python3-on-fcos-via-sysext")')" != "python3
fi

uninstall_sysext() {
local -r SYSEXT="${1}"
rm "/var/lib/extensions/${SYSEXT}.raw"
rm "/var/lib/extensions.d/${SYSEXT}-"*".raw"
rm "/etc/sysupdate.${SYSEXT}.d/${SYSEXT}.conf"
rmdir "/etc/sysupdate.${SYSEXT}.d/"
local -r name="${1}"
rm "/var/lib/extensions/${name}.raw"
rm "/var/lib/extensions.d/${name}-"*".raw"
}

uninstall_sysext python3
Expand Down