]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
sysupdate: Track incompletely-installed versions
authorAdrian Vovk <adrianvovk@gmail.com>
Tue, 2 Jul 2024 16:13:45 +0000 (12:13 -0400)
committerAdrian Vovk <adrianvovk@gmail.com>
Thu, 22 Aug 2024 20:00:45 +0000 (16:00 -0400)
When enumerating what versions exist for a given target, sysupdate would
completely throw out any version that's incomplete (where some of the
transfers in the target have that version installed or available, and
other transfers do not).

If we're trying to find what versions we can offer for download, this is
great behavior. If the server side is advertising a partial update to
download, we shouldn't present it to the user.

On the other hand, if we're enumerating what versions we have currently
installed, this is a bad behavior. It makes sysupdate fragile. For
example, if a sysext introduces a new .conf file into
/usr/lib/sysupdate.d, suddenly the currently-installed OS stops being a
version that we've enumerated. Since it's not enumerated, it's not
protected, and so sysupdate will wipe the booted OS.

So if we're looking for installed versions, we now loosen the
restrictions and enumerate incomplete installations.

Partial fix for https://github.com/systemd/systemd/issues/33339

man/org.freedesktop.sysupdate1.xml
src/sysupdate/sysupdate-update-set-flags.c
src/sysupdate/sysupdate-update-set-flags.h
src/sysupdate/sysupdate.c
src/sysupdate/updatectl.c

index 66c21fab3d3fed7026eb305e4acd313cff21a079..3acae3d701ccffe4683ec7daf214c1920ce79779 100644 (file)
@@ -222,6 +222,13 @@ node /org/freedesktop/sysupdate1/target/host {
           <function>Vacuum()</function> operation.</para></listitem>
         </varlistentry>
 
+        <varlistentry>
+          <term><literal>incomplete</literal></term>
+          <listitem><para>A boolean indicating whether this version is incomplete, which means that it is
+          missing some file. Note that only installed incomplete versions will be offered by the service;
+          versions that are incomplete on the server-side are completely ignored.</para></listitem>
+        </varlistentry>
+
         <varlistentry>
           <term><literal>changelog_urls</literal></term>
           <listitem><para>A list of strings that contain user-presentable URLs to ChangeLogs associated with
index d9232e4a17b6ad24a932d6eeeb7ff86b54e2f679..d37363e0731cd349b9f5231ae607968ade3c3c5d 100644 (file)
@@ -10,6 +10,9 @@ const char* update_set_flags_to_color(UpdateSetFlags flags) {
         if (flags == 0 || (flags & UPDATE_OBSOLETE))
                 return (flags & UPDATE_NEWEST) ? ansi_highlight_grey() : ansi_grey();
 
+        if (FLAGS_SET(flags, UPDATE_INSTALLED|UPDATE_INCOMPLETE))
+                return ansi_highlight_yellow();
+
         if (FLAGS_SET(flags, UPDATE_INSTALLED|UPDATE_NEWEST))
                 return ansi_highlight();
 
@@ -68,6 +71,27 @@ const char* update_set_flags_to_string(UpdateSetFlags flags) {
         case UPDATE_AVAILABLE|UPDATE_PROTECTED:
                 return "available";
 
+        case UPDATE_INSTALLED|UPDATE_INCOMPLETE|UPDATE_NEWEST:
+        case UPDATE_INSTALLED|UPDATE_INCOMPLETE|UPDATE_NEWEST|UPDATE_PROTECTED:
+        case UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_INCOMPLETE|UPDATE_NEWEST:
+        case UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_INCOMPLETE|UPDATE_NEWEST|UPDATE_PROTECTED:
+                return "current+incomplete";
+
+        case UPDATE_INSTALLED|UPDATE_INCOMPLETE:
+        case UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_INCOMPLETE:
+                return "installed+incomplete";
+
+        case UPDATE_INSTALLED|UPDATE_INCOMPLETE|UPDATE_PROTECTED:
+        case UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_INCOMPLETE|UPDATE_PROTECTED:
+                return "protected+incomplete";
+
+        case UPDATE_AVAILABLE|UPDATE_INCOMPLETE:
+        case UPDATE_AVAILABLE|UPDATE_INCOMPLETE|UPDATE_PROTECTED:
+        case UPDATE_AVAILABLE|UPDATE_INCOMPLETE|UPDATE_NEWEST:
+        case UPDATE_AVAILABLE|UPDATE_INCOMPLETE|UPDATE_NEWEST|UPDATE_PROTECTED:
+                /* We must never offer an update as available for download if it's incomplete */
+                assert_not_reached();
+
         case UPDATE_INSTALLED|UPDATE_OBSOLETE|UPDATE_NEWEST:
         case UPDATE_INSTALLED|UPDATE_OBSOLETE|UPDATE_NEWEST|UPDATE_PROTECTED:
         case UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_OBSOLETE|UPDATE_NEWEST:
@@ -88,6 +112,26 @@ const char* update_set_flags_to_string(UpdateSetFlags flags) {
         case UPDATE_AVAILABLE|UPDATE_OBSOLETE|UPDATE_NEWEST|UPDATE_PROTECTED:
                 return "available+obsolete";
 
+        case UPDATE_INSTALLED|UPDATE_OBSOLETE|UPDATE_INCOMPLETE|UPDATE_NEWEST:
+        case UPDATE_INSTALLED|UPDATE_OBSOLETE|UPDATE_INCOMPLETE|UPDATE_NEWEST|UPDATE_PROTECTED:
+        case UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_OBSOLETE|UPDATE_INCOMPLETE|UPDATE_NEWEST:
+        case UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_OBSOLETE|UPDATE_INCOMPLETE|UPDATE_NEWEST|UPDATE_PROTECTED:
+                return "current+obsolete+incomplete";
+
+        case UPDATE_INSTALLED|UPDATE_OBSOLETE|UPDATE_INCOMPLETE:
+        case UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_OBSOLETE|UPDATE_INCOMPLETE:
+                return "installed+obsolete+incomplete";
+
+        case UPDATE_INSTALLED|UPDATE_OBSOLETE|UPDATE_INCOMPLETE|UPDATE_PROTECTED:
+        case UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_OBSOLETE|UPDATE_INCOMPLETE|UPDATE_PROTECTED:
+                return "protected+obsolete+incomplete";
+
+        case UPDATE_AVAILABLE|UPDATE_OBSOLETE|UPDATE_INCOMPLETE:
+        case UPDATE_AVAILABLE|UPDATE_OBSOLETE|UPDATE_INCOMPLETE|UPDATE_PROTECTED:
+        case UPDATE_AVAILABLE|UPDATE_OBSOLETE|UPDATE_INCOMPLETE|UPDATE_NEWEST:
+        case UPDATE_AVAILABLE|UPDATE_OBSOLETE|UPDATE_INCOMPLETE|UPDATE_NEWEST|UPDATE_PROTECTED:
+                assert_not_reached();
+
         default:
                 assert_not_reached();
         }
index 49ac5e183582aedffc7e22c2efa903f4643f8a97..881dfdeffff78d7aee0fd8d96594875f644bd5c0 100644 (file)
@@ -2,11 +2,12 @@
 #pragma once
 
 typedef enum UpdateSetFlags {
-        UPDATE_NEWEST    = 1 << 0,
-        UPDATE_AVAILABLE = 1 << 1,
-        UPDATE_INSTALLED = 1 << 2,
-        UPDATE_OBSOLETE  = 1 << 3,
-        UPDATE_PROTECTED = 1 << 4,
+        UPDATE_NEWEST     = 1 << 0,
+        UPDATE_AVAILABLE  = 1 << 1,
+        UPDATE_INSTALLED  = 1 << 2,
+        UPDATE_OBSOLETE   = 1 << 3,
+        UPDATE_PROTECTED  = 1 << 4,
+        UPDATE_INCOMPLETE = 1 << 5,
 } UpdateSetFlags;
 
 const char* update_set_flags_to_color(UpdateSetFlags flags);
index 585453542764f3eb95e86df402a576be33e44273..9b56bf1a8f29c69791af4e0323f419b033fe9fcc 100644 (file)
@@ -215,7 +215,6 @@ static int context_load_available_instances(Context *c) {
 }
 
 static int context_discover_update_sets_by_flag(Context *c, UpdateSetFlags flags) {
-        _cleanup_free_ Instance **cursor_instances = NULL;
         _cleanup_free_ char *boundary = NULL;
         bool newest_found = false;
         int r;
@@ -224,64 +223,91 @@ static int context_discover_update_sets_by_flag(Context *c, UpdateSetFlags flags
         assert(IN_SET(flags, UPDATE_AVAILABLE, UPDATE_INSTALLED));
 
         for (;;) {
-                bool incomplete = false, exists = false;
+                _cleanup_free_ Instance **cursor_instances = NULL;
+                bool skip = false;
                 UpdateSetFlags extra_flags = 0;
                 _cleanup_free_ char *cursor = NULL;
                 UpdateSet *us = NULL;
 
-                for (size_t k = 0; k < c->n_transfers; k++) {
-                        Transfer *t = c->transfers[k];
-                        bool cursor_found = false;
+                /* First, let's find the newest version that's older than the boundary. */
+                FOREACH_ARRAY(tr, c->transfers, c->n_transfers) {
                         Resource *rr;
 
-                        assert(t);
+                        assert(*tr);
 
                         if (flags == UPDATE_AVAILABLE)
-                                rr = &t->source;
+                                rr = &(*tr)->source;
                         else {
                                 assert(flags == UPDATE_INSTALLED);
-                                rr = &t->target;
+                                rr = &(*tr)->target;
                         }
 
                         FOREACH_ARRAY(inst, rr->instances, rr->n_instances) {
-                                Instance *i = *inst;
+                                Instance *i = *inst; /* Sorted newest-to-oldest */
+
                                 assert(i);
 
-                                /* Is the instance we are looking at equal or newer than the boundary? If so, we
-                                 * already checked this version, and it wasn't complete, let's ignore it. */
                                 if (boundary && strverscmp_improved(i->metadata.version, boundary) >= 0)
-                                        continue;
+                                        continue; /* Not older than the boundary */
 
-                                if (cursor) {
-                                        if (strverscmp_improved(i->metadata.version, cursor) != 0)
-                                                continue;
-                                } else {
-                                        cursor = strdup(i->metadata.version);
-                                        if (!cursor)
-                                                return log_oom();
-                                }
+                                if (cursor && strverscmp(i->metadata.version, cursor) <= 0)
+                                        break; /* Not newer than the cursor. The same will be true for all
+                                                * subsequent instances (due to sorting) so let's skip to the
+                                                * next transfer. */
 
-                                cursor_found = true;
+                                if (free_and_strdup(&cursor, i->metadata.version) < 0)
+                                        return log_oom();
 
-                                if (!cursor_instances) {
-                                        cursor_instances = new(Instance*, c->n_transfers);
-                                        if (!cursor_instances)
-                                                return -ENOMEM;
-                                }
-                                cursor_instances[k] = i;
-                                break;
+                                break; /* All subsequent instances will be older than this one */
                         }
 
-                        if (!cursor) /* No suitable instance beyond the boundary found? Then we are done! */
-                                break;
+                        if (flags == UPDATE_AVAILABLE && !cursor)
+                                break; /* This transfer didn't have a version older than the boundary,
+                                        * so any older-than-boundary version that might exist in a different
+                                        * transfer must always be incomplete. For reasons described below,
+                                        * we don't include incomplete versions for AVAILABLE updates. So we
+                                        * are completely done looking. */
+                }
 
-                        if (!cursor_found) {
-                                /* Hmm, we didn't find the version indicated by 'cursor' among the instances
-                                 * of this transfer, let's skip it. */
-                                incomplete = true;
-                                break;
+                if (!cursor) /* We didn't find anything older than the boundary, so we're done. */
+                        break;
+
+                cursor_instances = new0(Instance*, c->n_transfers);
+                if (!cursor_instances)
+                        return log_oom();
+
+                /* Now let's find all the instances that match the version of the cursor, if we have them */
+                for (size_t k = 0; k < c->n_transfers; k++) {
+                        Transfer *t = c->transfers[k];
+                        Instance *match = NULL;
+
+                        assert(t);
+
+                        if (flags == UPDATE_AVAILABLE) {
+                                match = resource_find_instance(&t->source, cursor);
+                                if (!match) {
+                                        /* When we're looking for updates to download, we don't offer
+                                         * incomplete versions at all. The server wants to send us an update
+                                         * with parts of the OS missing. For robustness sake, let's not do
+                                         * that. */
+                                        skip = true;
+                                        break;
+                                }
+                        } else {
+                                assert(flags == UPDATE_INSTALLED);
+
+                                match = resource_find_instance(&t->target, cursor);
+                                if (!match) {
+                                        /* When we're looking for installed versions, let's be robust and treat
+                                         * an incomplete installation as an installation. Otherwise, there are
+                                         * situations that can lead to sysupdate wiping the currently booted OS.
+                                         * See https://github.com/systemd/systemd/issues/33339 */
+                                        extra_flags |= UPDATE_INCOMPLETE;
+                                }
                         }
 
+                        cursor_instances[k] = match;
+
                         if (t->min_version && strverscmp_improved(t->min_version, cursor) > 0)
                                 extra_flags |= UPDATE_OBSOLETE;
 
@@ -289,14 +315,11 @@ static int context_discover_update_sets_by_flag(Context *c, UpdateSetFlags flags
                                 extra_flags |= UPDATE_PROTECTED;
                 }
 
-                if (!cursor) /* EOL */
-                        break;
-
                 r = free_and_strdup_warn(&boundary, cursor);
                 if (r < 0)
                         return r;
 
-                if (incomplete) /* One transfer was missing this version, ignore the whole thing */
+                if (skip)
                         continue;
 
                 /* See if we already have this update set in our table */
@@ -307,12 +330,12 @@ static int context_discover_update_sets_by_flag(Context *c, UpdateSetFlags flags
 
                         /* We only store the instances we found first, but we remember we also found it again */
                         u->flags |= flags | extra_flags;
-                        exists = true;
+                        skip = true;
                         newest_found = true;
                         break;
                 }
 
-                if (exists)
+                if (skip)
                         continue;
 
                 /* Doesn't exist yet, let's add it */
@@ -504,6 +527,12 @@ static int context_show_version(Context *c, const char *version) {
 
         FOREACH_ARRAY(inst, us->instances, us->n_instances) {
                 Instance *i = *inst;
+
+                if (!i) {
+                        assert(FLAGS_SET(us->flags, UPDATE_INCOMPLETE));
+                        continue;
+                }
+
                 r = table_add_many(t,
                                    TABLE_STRING, resource_type_to_string(i->resource->type),
                                    TABLE_PATH, i->path);
@@ -641,13 +670,14 @@ static int context_show_version(Context *c, const char *version) {
         if (FLAGS_SET(arg_json_format_flags, SD_JSON_FORMAT_OFF)) {
                 printf("%s%s%s Version: %s\n"
                        "    State: %s%s%s\n"
-                       "Installed: %s%s\n"
+                       "Installed: %s%s%s%s%s\n"
                        "Available: %s%s\n"
                        "Protected: %s%s%s\n"
                        " Obsolete: %s%s%s\n",
                        strempty(update_set_flags_to_color(us->flags)), update_set_flags_to_glyph(us->flags), ansi_normal(), us->version,
                        strempty(update_set_flags_to_color(us->flags)), update_set_flags_to_string(us->flags), ansi_normal(),
                        yes_no(us->flags & UPDATE_INSTALLED), FLAGS_SET(us->flags, UPDATE_INSTALLED|UPDATE_NEWEST) ? " (newest)" : "",
+                       FLAGS_SET(us->flags, UPDATE_INCOMPLETE) ? ansi_highlight_yellow() : "", FLAGS_SET(us->flags, UPDATE_INCOMPLETE) ? " (incomplete)" : "", ansi_normal(),
                        yes_no(us->flags & UPDATE_AVAILABLE), (us->flags & (UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_NEWEST)) == (UPDATE_AVAILABLE|UPDATE_NEWEST) ? " (newest)" : "",
                        FLAGS_SET(us->flags, UPDATE_INSTALLED|UPDATE_PROTECTED) ? ansi_highlight() : "", yes_no(FLAGS_SET(us->flags, UPDATE_INSTALLED|UPDATE_PROTECTED)), ansi_normal(),
                        us->flags & UPDATE_OBSOLETE ? ansi_highlight_red() : "", yes_no(us->flags & UPDATE_OBSOLETE), ansi_normal());
@@ -675,6 +705,7 @@ static int context_show_version(Context *c, const char *version) {
                                           SD_JSON_BUILD_PAIR_BOOLEAN("installed", FLAGS_SET(us->flags, UPDATE_INSTALLED)),
                                           SD_JSON_BUILD_PAIR_BOOLEAN("obsolete", FLAGS_SET(us->flags, UPDATE_OBSOLETE)),
                                           SD_JSON_BUILD_PAIR_BOOLEAN("protected", FLAGS_SET(us->flags, UPDATE_PROTECTED)),
+                                          SD_JSON_BUILD_PAIR_BOOLEAN("incomplete", FLAGS_SET(us->flags, UPDATE_INCOMPLETE)),
                                           SD_JSON_BUILD_PAIR_STRV("changelog_urls", changelog_urls),
                                           SD_JSON_BUILD_PAIR_VARIANT("contents", t_json));
                 if (r < 0)
index 151fa9e1d7a7b8240fa0160a56ab54f3a712ef24..d8db49e57c90300830d3d6ed22ee71a34a0fd93a 100644 (file)
@@ -324,7 +324,8 @@ static int parse_describe(sd_bus_message *reply, Version *ret) {
         Version v = {};
         char *version_json = NULL;
         _cleanup_(sd_json_variant_unrefp) sd_json_variant *json = NULL, *contents_json = NULL;
-        bool newest = false, available = false, installed = false, obsolete = false, protected = false;
+        bool newest = false, available = false, installed = false, obsolete = false, protected = false,
+                incomplete = false;
         int r;
 
         assert(reply);
@@ -348,6 +349,7 @@ static int parse_describe(sd_bus_message *reply, Version *ret) {
                                      { "installed",      SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, PTR_TO_SIZE(&installed),     0 },
                                      { "obsolete",       SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, PTR_TO_SIZE(&obsolete),      0 },
                                      { "protected",      SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, PTR_TO_SIZE(&protected),     0 },
+                                     { "incomplete",     SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, PTR_TO_SIZE(&incomplete),    0 },
                                      { "changelog_urls", SD_JSON_VARIANT_ARRAY,   sd_json_dispatch_strv,    PTR_TO_SIZE(&v.changelog),   0 },
                                      { "contents",       SD_JSON_VARIANT_ARRAY,   sd_json_dispatch_variant, PTR_TO_SIZE(&contents_json), 0 },
                                      {},
@@ -362,6 +364,7 @@ static int parse_describe(sd_bus_message *reply, Version *ret) {
         SET_FLAG(v.flags, UPDATE_INSTALLED, installed);
         SET_FLAG(v.flags, UPDATE_OBSOLETE, obsolete);
         SET_FLAG(v.flags, UPDATE_PROTECTED, protected);
+        SET_FLAG(v.flags, UPDATE_INCOMPLETE, incomplete);
 
         r = sd_json_variant_format(contents_json, 0, &v.contents_json);
         if (r < 0)