portablectl: fix regression when using --force without extension parameters

c18f4eb9e96836a made it possible to use --force with various verbs, by
going through the newer D-Bus methods. Except it didn't, as it regressed
during PR review refactorings, and nobody noticed because there were no
tests for it. Fix it, and add tests.

Follow-up for c18f4eb9e96836a6a8285ec42fd8a34c8909f6d9

diff --git a/src/portable/portablectl.c b/src/portable/portablectl.c
index e77ee50dab2f464978bd51979e1335f099c0a10f..3a25624b088f326aefa899f35c75cd3743f81b86 100644 (file)
--- a/src/portable/portablectl.c
+++ b/src/portable/portablectl.c
@@ -91,12 +91,14 @@ static int determine_image(const char *image, bool permit_non_existing, char **r
         return 0;
 }
 
-static int attach_extensions_to_message(sd_bus_message *m, char **extensions) {
+static int attach_extensions_to_message(sd_bus_message *m, const char *method, char **extensions) {
         int r;
 
         assert(m);
+        assert(method);
 
-        if (strv_isempty(extensions))
+        /* The new methods also have flags parameters that are independent of the extensions */
+        if (strv_isempty(extensions) && !endswith(method, "WithExtensions"))
                 return 0;
 
         r = sd_bus_message_open_container(m, 'a', "s");
@@ -263,7 +265,7 @@ static int get_image_metadata(sd_bus *bus, const char *image, char **matches, sd
         if (r < 0)
                 return bus_log_create_error(r);
 
-        r = attach_extensions_to_message(m, arg_extension_images);
+        r = attach_extensions_to_message(m, method, arg_extension_images);
         if (r < 0)
                 return r;
 
@@ -271,7 +273,7 @@ static int get_image_metadata(sd_bus *bus, const char *image, char **matches, sd
         if (r < 0)
                 return bus_log_create_error(r);
 
-        if (!strv_isempty(arg_extension_images)) {
+        if (streq(method, "GetImageMetadataWithExtensions")) {
                 r = sd_bus_message_append(m, "t", flags);
                 if (r < 0)
                         return bus_log_create_error(r);
@@ -774,8 +776,9 @@ static int maybe_stop_disable(sd_bus *bus, char *image, char *argv[]) {
         if (r < 0)
                 return bus_log_parse_error(r);
 
-        /* If we specified any extensions, we'll first an array of extension-release metadata. */
-        if (!strv_isempty(arg_extension_images)) {
+        /* If we specified any extensions or --force (which makes the request go through the new
+         * WithExtensions calls), we'll first get an array of extension-release metadata. */
+        if (!strv_isempty(arg_extension_images) || arg_force) {
                 r = sd_bus_message_skip(reply, "a{say}");
                 if (r < 0)
                         return bus_log_parse_error(r);
@@ -855,7 +858,7 @@ static int attach_reattach_image(int argc, char *argv[], const char *method) {
         if (r < 0)
                 return bus_log_create_error(r);
 
-        r = attach_extensions_to_message(m, arg_extension_images);
+        r = attach_extensions_to_message(m, method, arg_extension_images);
         if (r < 0)
                 return r;
 
@@ -933,11 +936,11 @@ static int detach_image(int argc, char *argv[], void *userdata) {
         if (r < 0)
                 return bus_log_create_error(r);
 
-        r = attach_extensions_to_message(m, arg_extension_images);
+        r = attach_extensions_to_message(m, method, arg_extension_images);
         if (r < 0)
                 return r;
 
-        if (strv_isempty(arg_extension_images))
+        if (streq(method, "DetachImage"))
                 r = sd_bus_message_append(m, "b", arg_runtime);
         else {
                 uint64_t flags = (arg_runtime ? PORTABLE_RUNTIME : 0) | (arg_force ? PORTABLE_FORCE_ATTACH | PORTABLE_FORCE_SYSEXT : 0);
@@ -1145,7 +1148,7 @@ static int is_image_attached(int argc, char *argv[], void *userdata) {
         if (r < 0)
                 return bus_log_create_error(r);
 
-        r = attach_extensions_to_message(m, arg_extension_images);
+        r = attach_extensions_to_message(m, method, arg_extension_images);
         if (r < 0)
                 return r;
 

diff --git a/test/units/testsuite-29.sh b/test/units/testsuite-29.sh
index 18ec41727d002904d441e483cea51fad565ac5c9..36e5cdc12637c2321380042542041f97333b771a 100755 (executable)
--- a/test/units/testsuite-29.sh
+++ b/test/units/testsuite-29.sh
@@ -43,12 +43,16 @@ EOF
 
 portablectl "${ARGS[@]}" attach --now --runtime /usr/share/minimal_0.raw minimal-app0
 
+portablectl is-attached minimal-app0
+portablectl inspect /usr/share/minimal_0.raw minimal-app0.service
 systemctl is-active minimal-app0.service
 systemctl is-active minimal-app0-foo.service
 systemctl is-active minimal-app0-bar.service && exit 1
 
 portablectl "${ARGS[@]}" reattach --now --runtime /usr/share/minimal_1.raw minimal-app0
 
+portablectl is-attached minimal-app0
+portablectl inspect /usr/share/minimal_0.raw minimal-app0.service
 systemctl is-active minimal-app0.service
 systemctl is-active minimal-app0-bar.service
 systemctl is-active minimal-app0-foo.service && exit 1
@@ -61,6 +65,32 @@ portablectl detach --now --runtime /usr/share/minimal_1.raw minimal-app0
 portablectl list | grep -q -F "No images."
 busctl tree org.freedesktop.portable1 --no-pager | grep -q -F '/org/freedesktop/portable1/image/minimal_5f1' && exit 1
 
+# Ensure we don't regress (again) when using --force
+
+portablectl "${ARGS[@]}" attach --force --now --runtime /usr/share/minimal_0.raw minimal-app0
+
+portablectl is-attached --force minimal-app0
+portablectl inspect --force /usr/share/minimal_0.raw minimal-app0.service
+systemctl is-active minimal-app0.service
+systemctl is-active minimal-app0-foo.service
+systemctl is-active minimal-app0-bar.service && exit 1
+
+portablectl "${ARGS[@]}" reattach --force --now --runtime /usr/share/minimal_1.raw minimal-app0
+
+portablectl is-attached --force minimal-app0
+portablectl inspect --force /usr/share/minimal_0.raw minimal-app0.service
+systemctl is-active minimal-app0.service
+systemctl is-active minimal-app0-bar.service
+systemctl is-active minimal-app0-foo.service && exit 1
+
+portablectl list | grep -q -F "minimal_1"
+busctl tree org.freedesktop.portable1 --no-pager | grep -q -F '/org/freedesktop/portable1/image/minimal_5f1'
+
+portablectl detach --force --now --runtime /usr/share/minimal_1.raw minimal-app0
+
+portablectl list | grep -q -F "No images."
+busctl tree org.freedesktop.portable1 --no-pager | grep -q -F '/org/freedesktop/portable1/image/minimal_5f1' && exit 1
+
 # portablectl also works with directory paths rather than images
 
 unsquashfs -dest /tmp/minimal_0 /usr/share/minimal_0.raw