sysupdate: List default component only if transfer definition exists
`sysupdate --json=short components` lists the components known to
sysupdate; these are the components which something like `updatectl`
will try to update.
The `default` component represents the host, and is meant to be listed
if transfer definitions exist in (for example) `/etc/sysupdate.d`
corresponding to the host OS. This then corresponds to `TARGET_HOST` in
`updatectl` and causes it to try updating that target.
The logic for working out whether the `default` component was present
essentially boiled down to “does `{/run,/etc,/usr/lib}/sysupdate.d`
exist”, and it didn’t check whether a `.transfer` or `.conf` file
actually existed in the config directory.
This is quite the corner case, but becomes more evident on systems where
sysupdate is being used to update a portable service but not the main
OS. At that point, if `/etc/sysupdate.d` exists empty (for some reason),
`updatectl` falls over because it starts trying to update the host OS
without any configuration to do so.
So, modify `sysupdate` to more fully load the available configuration
when listing components, and query it a bit more deeply to check whether
a default component exists.
If `sysupdate` is called with various command line arguments to affect
how its configuration is loaded, do *not* say that a default component
exists, as these arguments essentially anull the possibility of a
default being used in that process.
Add an integration test based on the reproducer provided by the issue
reporter. This test has been tested to fail if the changes to
`sysupdate.c` aren’t applied — if so, the second call to `sysupdate
components` would return
`{"default":true,"components":["some-component"]}`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: https://github.com/systemd/systemd/issues/41501
This commit is contained in:
@@ -1731,6 +1731,7 @@ static int component_name_valid(const char *c) {
|
||||
VERB_NOARG(verb_components, "components",
|
||||
"Show list of components");
|
||||
static int verb_components(int argc, char *argv[], uintptr_t _data, void *userdata) {
|
||||
_cleanup_(context_freep) Context* context = NULL;
|
||||
_cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL;
|
||||
_cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL;
|
||||
_cleanup_set_free_ Set *names = NULL;
|
||||
@@ -1743,6 +1744,10 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
r = context_make_offline(&context, loop_device ? loop_device->node : NULL, 0);
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
ConfFile **directories = NULL;
|
||||
size_t n_directories = 0;
|
||||
|
||||
@@ -1757,7 +1762,6 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda
|
||||
ConfFile *e = *i;
|
||||
|
||||
if (streq(e->filename, "sysupdate.d")) {
|
||||
has_default_component = true;
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -1785,6 +1789,14 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda
|
||||
TAKE_PTR(n);
|
||||
}
|
||||
|
||||
/* Does the system have at least one transfer file in /etc/sysupdate.d, which can be considered a
|
||||
* TARGET_HOST? See target_get_argument() in sysupdated.c */
|
||||
has_default_component = (!arg_definitions &&
|
||||
!arg_component &&
|
||||
!arg_root &&
|
||||
!arg_image &&
|
||||
context->n_transfers > 0);
|
||||
|
||||
/* We use simple free() rather than strv_free() here, since set_free() will free the strings for us */
|
||||
_cleanup_free_ char **z = set_get_strv(names);
|
||||
if (!z)
|
||||
|
||||
@@ -552,6 +552,35 @@ EOF
|
||||
done
|
||||
done
|
||||
|
||||
# Regression test for https://github.com/systemd/systemd/issues/41501 — check
|
||||
# that a ‘default’ component is only listed by sysupdate if it’s fully configured
|
||||
mv "$CONFIGDIR" "$CONFIGDIR.backup"
|
||||
mkdir -p /run/sysupdate.some-component.d
|
||||
tee /run/sysupdate.some-component.d/portable.transfer << EOF
|
||||
[Transfer]
|
||||
ChangeLog=https://example.com/changelog/@v
|
||||
Verify=no
|
||||
|
||||
[Source]
|
||||
Type=url-tar
|
||||
Path=https://example.com/does-not-matter/@v.tar.xz
|
||||
MatchPattern=some-component_@v-portable.tar.xz
|
||||
|
||||
[Target]
|
||||
Type=directory
|
||||
Path=/var/lib/portables
|
||||
MatchPattern=some-component_@v
|
||||
CurrentSymlink=some-component
|
||||
EOF
|
||||
"$SYSUPDATE" --json=short components | grep -F '{"default":false,"components":["some-component"]}' >/dev/null
|
||||
mkdir /run/sysupdate.d
|
||||
"$SYSUPDATE" --json=short components | grep -F '{"default":false,"components":["some-component"]}' >/dev/null
|
||||
|
||||
# Clean up regression test
|
||||
rmdir /run/sysupdate.d
|
||||
rm -rf /run/sysupdate.some-component.d
|
||||
mv "$CONFIGDIR.backup" "$CONFIGDIR"
|
||||
|
||||
# Make sure the processing of compressed streams still handles uncompressed streams shorter than
|
||||
# COMPRESSION_MAGIC_BYTES_MAX correctly.
|
||||
rm -rf "$CONFIGDIR" "$WORKDIR/blobs"
|
||||
|
||||
Reference in New Issue
Block a user