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

diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c
index 7852ac3878411d4b1e80be1ad4d40bcbdc3ec0bd..92c40ddf71ab9810c4c767b3f537e87fd1eebb8a 100644 (file)
--- a/src/sysupdate/sysupdate.c
+++ b/src/sysupdate/sysupdate.c
@@ -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)

diff --git a/test/units/TEST-72-SYSUPDATE.sh b/test/units/TEST-72-SYSUPDATE.sh
index 21a3ea00a59046b745a05038d2b21c0621f8cfe3..e501a7647f3a609042dd77c29c6b4fc0155b87ca 100755 (executable)
--- a/test/units/TEST-72-SYSUPDATE.sh
+++ b/test/units/TEST-72-SYSUPDATE.sh
@@ -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"