From 9785c44dd83a44c2231316f912b4adf768a09f20 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 28 Aug 2020 14:17:31 +0100 Subject: [PATCH 1/5] test-50-dissect: move minimal image setup to common setup function So that it can be re-used for other tests by simply setting TEST_INSTALL_VERITY_MINIMAL=1 in test.sh, without having to replicate the setup commands. --- test/TEST-50-DISSECT/test.sh | 21 ++------------- test/test-functions | 50 ++++++++++++++++++++++++++++++++++++ test/units/testsuite-50.sh | 4 +-- 3 files changed, 54 insertions(+), 21 deletions(-) diff --git a/test/TEST-50-DISSECT/test.sh b/test/TEST-50-DISSECT/test.sh index c9e4faf5cb..2ff81bf4fa 100755 --- a/test/TEST-50-DISSECT/test.sh +++ b/test/TEST-50-DISSECT/test.sh @@ -5,11 +5,10 @@ set -e TEST_DESCRIPTION="test systemd-dissect" IMAGE_NAME="dissect" TEST_NO_NSPAWN=1 +TEST_INSTALL_VERITY_MINIMAL=1 . $TEST_BASE_DIR/test-functions -command -v mksquashfs >/dev/null 2>&1 || exit 0 -command -v veritysetup >/dev/null 2>&1 || exit 0 command -v sfdisk >/dev/null 2>&1 || exit 0 # Need loop devices for systemd-dissect @@ -21,23 +20,7 @@ test_append_files() { install_dmevent generate_module_dependencies inst_binary losetup - - BASICTOOLS=( - bash - cat - mount - ) - oldinitdir=$initdir - export initdir=$TESTDIR/minimal - mkdir -p $initdir/usr/lib $initdir/etc - setup_basic_dirs - install_basic_tools - cp $os_release $initdir/usr/lib/os-release - ln -s ../usr/lib/os-release $initdir/etc/os-release - echo MARKER=1 >> $initdir/usr/lib/os-release - mksquashfs $initdir $oldinitdir/usr/share/minimal.raw - veritysetup format $oldinitdir/usr/share/minimal.raw $oldinitdir/usr/share/minimal.verity | grep '^Root hash:' | cut -f2 | tr -d '\n' > $oldinitdir/usr/share/minimal.roothash - export initdir=$oldinitdir + install_verity_minimal ) } diff --git a/test/test-functions b/test/test-functions index 44696aff3b..612a693e8a 100644 --- a/test/test-functions +++ b/test/test-functions @@ -462,6 +462,53 @@ run_nspawn() { return 0 } +# Build two very minimal root images, with two units, one is the same and one is different across them +install_verity_minimal() { + if [ -e $initdir/usr/share/minimal.raw ]; then + return + fi + if ! command -v mksquashfs >/dev/null 2>&1; then + dfatal "mksquashfs not found" + exit 1 + fi + if ! command -v veritysetup >/dev/null 2>&1; then + dfatal "veritysetup not found" + exit 1 + fi + ( + BASICTOOLS=( + bash + cat + mount + sleep + ) + oldinitdir=$initdir + rm -rfv $TESTDIR/minimal + export initdir=$TESTDIR/minimal + mkdir -p $initdir/usr/lib/systemd/system $initdir/etc + setup_basic_dirs + install_basic_tools + cp $os_release $initdir/usr/lib/os-release + ln -s ../usr/lib/os-release $initdir/etc/os-release + touch $initdir/etc/machine-id $initdir/etc/resolv.conf + echo MARKER=1 >> $initdir/usr/lib/os-release + echo -e "[Service]\nExecStartPre=cat /usr/lib/os-release\nExecStart=sleep 120" > $initdir/usr/lib/systemd/system/app0.service + cp $initdir/usr/lib/systemd/system/app0.service $initdir/usr/lib/systemd/system/app0-foo.service + + mksquashfs $initdir $oldinitdir/usr/share/minimal_0.raw + veritysetup format $oldinitdir/usr/share/minimal_0.raw $oldinitdir/usr/share/minimal_0.verity | \ + grep '^Root hash:' | cut -f2 | tr -d '\n' > $oldinitdir/usr/share/minimal_0.roothash + + sed -i "s/MARKER=1/MARKER=2/g" $initdir/usr/lib/os-release + rm $initdir/usr/lib/systemd/system/app0-foo.service + cp $initdir/usr/lib/systemd/system/app0.service $initdir/usr/lib/systemd/system/app0-bar.service + + mksquashfs $initdir $oldinitdir/usr/share/minimal_1.raw + veritysetup format $oldinitdir/usr/share/minimal_1.raw $oldinitdir/usr/share/minimal_1.verity | \ + grep '^Root hash:' | cut -f2 | tr -d '\n' > $oldinitdir/usr/share/minimal_1.roothash + ) +} + setup_basic_environment() { # create the basic filesystem layout setup_basic_dirs @@ -492,6 +539,9 @@ setup_basic_environment() { if [[ "$IS_BUILT_WITH_ASAN" = "yes" ]]; then create_asan_wrapper fi + if [ -n "$TEST_INSTALL_VERITY_MINIMAL" ]; then + install_verity_minimal + fi } setup_selinux() { diff --git a/test/units/testsuite-50.sh b/test/units/testsuite-50.sh index 783dfbf50e..1dd4b5dbd1 100755 --- a/test/units/testsuite-50.sh +++ b/test/units/testsuite-50.sh @@ -24,8 +24,8 @@ fi trap cleanup EXIT -cp /usr/share/minimal.* "${image_dir}/" -image="${image_dir}/minimal" +cp /usr/share/minimal* "${image_dir}/" +image="${image_dir}/minimal_0" roothash="$(cat ${image}.roothash)" os_release=$(test -e /etc/os-release && echo /etc/os-release || echo /usr/lib/os-release) From e763342091fc97c15d0f54119e27452433c59e88 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Tue, 2 Feb 2021 17:16:41 +0000 Subject: [PATCH 2/5] test: add empty /etc/resolv.conf in test image Portable services have a BindPath pointing to it, so it needs to be available in the image --- test/test-functions | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test-functions b/test/test-functions index 612a693e8a..e3831f28f1 100644 --- a/test/test-functions +++ b/test/test-functions @@ -1106,6 +1106,7 @@ install_config_files() { # we want an empty environment > $initdir/etc/environment > $initdir/etc/machine-id + > $initdir/etc/resolv.conf # set the hostname echo systemd-testsuite > $initdir/etc/hostname From 8b535f3aac0f7673e1637cc3a4410b64e5072ec1 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Tue, 2 Feb 2021 21:13:44 +0000 Subject: [PATCH 3/5] test: setup var/tmp in the test image as well --- test/test-functions | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-functions b/test/test-functions index e3831f28f1..e5f2495395 100644 --- a/test/test-functions +++ b/test/test-functions @@ -1322,7 +1322,7 @@ setup_basic_dirs() { mkdir -p $initdir/etc/systemd/system mkdir -p $initdir/var/log/journal - for d in usr/bin usr/sbin bin etc lib "$libdir" sbin tmp usr var var/log dev proc sys sysroot root run run/lock run/initramfs; do + for d in usr/bin usr/sbin bin etc lib "$libdir" sbin tmp usr var var/log var/tmp dev proc sys sysroot root run run/lock run/initramfs; do if [ -L "/$d" ]; then inst_symlink "/$d" else From 9e4079d4111894db0cbd6c51e9f8252c9fd22330 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 5 Feb 2021 21:24:54 +0000 Subject: [PATCH 4/5] portable: allow Detach to match images with different version suffixes --- src/portable/portable.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/portable/portable.c b/src/portable/portable.c index d18e03afd4..77e8d21287 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -1035,10 +1035,14 @@ static bool marker_matches_image(const char *marker, const char *name_or_path) { a = last_path_component(marker); if (image_name_is_valid(name_or_path)) { - const char *e; + const char *e, *underscore; /* We shall match against an image name. In that case let's compare the last component, and optionally - * allow either a suffix of ".raw" or a series of "/". */ + * allow either a suffix of ".raw" or a series of "/". + * But allow matching on a different version of the same image, when a "_" is used as a separator. */ + underscore = strchr(name_or_path, '_'); + if (underscore) + return strneq(a, name_or_path, underscore - name_or_path); e = startswith(a, name_or_path); if (!e) @@ -1048,7 +1052,7 @@ static bool marker_matches_image(const char *marker, const char *name_or_path) { e[strspn(e, "/")] == 0 || streq(e, ".raw"); } else { - const char *b; + const char *b, *underscore; size_t l; /* We shall match against a path. Let's ignore any prefix here though, as often there are many ways to @@ -1060,7 +1064,11 @@ static bool marker_matches_image(const char *marker, const char *name_or_path) { if (strcspn(b, "/") != l) return false; - return memcmp(a, b, l) == 0; + underscore = strchr(b, '_'); + if (underscore) + l = underscore - b; + + return strneq(a, b, l); } } From e26fe5f91158a0d9133e61347650dfb73aadd6bb Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 1 Feb 2021 14:29:40 +0000 Subject: [PATCH 5/5] portable: add 'reattach' verb and DBUS interface Add 'reattach' verb to portablectl, and corresponding DBUS interface to systemd-portabled. Takes the same parameters as 'attach', but it will do a 'detach' (and it will refuse to proceed if it cannot be done) first, matching on the unversioned prefix of the new image. Eg: portablectl reattach /tmp/foo_2.raw will cause foo_1.raw to be detached, and foo_2.raw to be attached. The key difference with a manual 'detach old' plus 'attach new' is that the running units are not disturbed until after the attach completed, and if --now is passed they are then restarted. A 'detach' is not allowed normally if the units are running. By using a restart-after-deploy method, 'reattach' allows for minimal interruption of service and also for features that only work on restart (eg: file descriptor store) to work as intended. The DBUS interface returns two lists: first the removals from the detach that were not immediately re-added in the attach, so that the caller can stop the relevant units, and then the list of additions that are either new or updates, so that the caller can restart/enable the relevant units. portablectl already implements this with the existing --now/--enable switches. --- man/portablectl.xml | 17 +- shell-completion/bash/portablectl | 2 +- src/portable/org.freedesktop.portable1.conf | 8 + src/portable/portable.c | 6 +- src/portable/portable.h | 1 + src/portable/portablectl.c | 128 ++++++++++++-- src/portable/portabled-bus.c | 58 ++++++- src/portable/portabled-bus.h | 1 + src/portable/portabled-image-bus.c | 181 ++++++++++++++++++++ src/portable/portabled-image-bus.h | 1 + test/TEST-58-PORTABLE/Makefile | 1 + test/TEST-58-PORTABLE/test.sh | 27 +++ test/units/testsuite-58.service | 7 + test/units/testsuite-58.sh | 68 ++++++++ 14 files changed, 480 insertions(+), 26 deletions(-) create mode 120000 test/TEST-58-PORTABLE/Makefile create mode 100755 test/TEST-58-PORTABLE/test.sh create mode 100644 test/units/testsuite-58.service create mode 100755 test/units/testsuite-58.sh diff --git a/man/portablectl.xml b/man/portablectl.xml index 3653207d72..bd0c9e0573 100644 --- a/man/portablectl.xml +++ b/man/portablectl.xml @@ -155,6 +155,20 @@ to be used in case the unit names do not match the image name as described in the attach. + + reattach IMAGE [PREFIX…] + + Detaches an existing portable service image from the host, and immediately attaches it again. + This is useful in case the image was replaced. Running units are not stopped during the process. Partial matching, + to allow for different versions in the image name, is allowed: only the part before the first _ + character has to match. If the new image doesn't exist, the existing one will not be detached. The parameters + follow the same syntax as the attach command. + + If and/or are passed, the portable service(s) are + immediately stopped if removed, started and/or enabled if added, or restarted if updated. Prefixes are also + accepted, in the same way as described in the attach case. + + inspect IMAGE [PREFIX…] @@ -328,7 +342,8 @@ - Immediately start/stop the portable service after attaching/before detaching. + Immediately start/stop/restart the portable service after attaching/before + detaching/after upgrading. diff --git a/shell-completion/bash/portablectl b/shell-completion/bash/portablectl index fe3d925d78..e18d04c7ea 100644 --- a/shell-completion/bash/portablectl +++ b/shell-completion/bash/portablectl @@ -40,7 +40,7 @@ _portablectl() { local -A VERBS=( [STANDALONE]='list' - [IMAGE]='attach detach inspect is-attached set-limit' + [IMAGE]='attach detach reattach inspect is-attached set-limit' [IMAGES]='remove' [IMAGE_WITH_BOOL]='read-only' ) diff --git a/src/portable/org.freedesktop.portable1.conf b/src/portable/org.freedesktop.portable1.conf index 1343e1d544..5a09f5c2d9 100644 --- a/src/portable/org.freedesktop.portable1.conf +++ b/src/portable/org.freedesktop.portable1.conf @@ -61,6 +61,10 @@ send_interface="org.freedesktop.portable1.Manager" send_member="DetachImage"/> + + @@ -99,6 +103,10 @@ send_interface="org.freedesktop.portable1.Image" send_member="Detach"/> + + diff --git a/src/portable/portable.c b/src/portable/portable.c index 77e8d21287..0a3b1e8fde 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -1003,13 +1003,13 @@ int portable_attach( r = unit_file_exists(UNIT_FILE_SYSTEM, &paths, item->name); if (r < 0) return sd_bus_error_set_errnof(error, r, "Failed to determine whether unit '%s' exists on the host: %m", item->name); - if (r > 0) + if (!FLAGS_SET(flags, PORTABLE_REATTACH) && r > 0) return sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, "Unit file '%s' exists on the host already, refusing.", item->name); r = unit_file_is_active(bus, item->name, error); if (r < 0) return r; - if (r > 0) + if (!FLAGS_SET(flags, PORTABLE_REATTACH) && r > 0) return sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, "Unit file '%s' is active already, refusing.", item->name); } @@ -1193,7 +1193,7 @@ int portable_detach( r = unit_file_is_active(bus, de->d_name, error); if (r < 0) return r; - if (r > 0) + if (!FLAGS_SET(flags, PORTABLE_REATTACH) && r > 0) return sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, "Unit file '%s' is active, can't detach.", de->d_name); r = set_put_strdup(&unit_files, de->d_name); diff --git a/src/portable/portable.h b/src/portable/portable.h index 00ff80c38a..724634feb8 100644 --- a/src/portable/portable.h +++ b/src/portable/portable.h @@ -21,6 +21,7 @@ typedef enum PortableFlags { PORTABLE_PREFER_COPY = 1 << 0, PORTABLE_PREFER_SYMLINK = 1 << 1, PORTABLE_RUNTIME = 1 << 2, + PORTABLE_REATTACH = 1 << 3, } PortableFlags; typedef enum PortableChangeType { diff --git a/src/portable/portablectl.c b/src/portable/portablectl.c index edfd74d540..278dae0e1b 100644 --- a/src/portable/portablectl.c +++ b/src/portable/portablectl.c @@ -45,6 +45,10 @@ static bool arg_enable = false; static bool arg_now = false; static bool arg_no_block = false; +static bool is_portable_managed(const char *unit) { + return ENDSWITH_SET(unit, ".service", ".target", ".socket", ".path", ".timer"); +} + static int determine_image(const char *image, bool permit_non_existing, char **ret) { int r; @@ -436,12 +440,14 @@ static int maybe_enable_disable(sd_bus *bus, const char *path, bool enable) { return 0; } -static int maybe_start_stop(sd_bus *bus, const char *path, bool start, BusWaitForJobs *wait) { +static int maybe_start_stop_restart(sd_bus *bus, const char *path, const char *method, BusWaitForJobs *wait) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; char *name = (char *)basename(path), *job = NULL; int r; + assert(STR_IN_SET(method, "StartUnit", "StopUnit", "RestartUnit")); + if (!arg_now) return 0; @@ -450,13 +456,13 @@ static int maybe_start_stop(sd_bus *bus, const char *path, bool start, BusWaitFo "org.freedesktop.systemd1", "/org/freedesktop/systemd1", "org.freedesktop.systemd1.Manager", - start ? "StartUnit" : "StopUnit", + method, &error, &reply, "ss", name, "replace"); if (r < 0) - return log_error_errno(r, "Failed to %s the portable service %s: %s", - start ? "start" : "stop", + return log_error_errno(r, "Failed to call %s on the portable service %s: %s", + method, path, bus_error_message(&error, r)); @@ -465,13 +471,13 @@ static int maybe_start_stop(sd_bus *bus, const char *path, bool start, BusWaitFo return bus_log_parse_error(r); if (!arg_quiet) - log_info("Queued %s to %s portable service %s.", job, start ? "start" : "stop", name); + log_info("Queued %s to call %s on portable service %s.", job, method, name); if (wait) { r = bus_wait_for_jobs_add(wait, job); if (r < 0) - return log_error_errno(r, "Failed to watch %s job for %s %s: %m", - job, start ? "starting" : "stopping", name); + return log_error_errno(r, "Failed to watch %s job to call %s on %s: %m", + job, method, name); } return 0; @@ -506,9 +512,83 @@ static int maybe_enable_start(sd_bus *bus, sd_bus_message *reply) { if (r == 0) break; - if (STR_IN_SET(type, "symlink", "copy") && ENDSWITH_SET(path, ".service", ".target", ".socket")) { + if (STR_IN_SET(type, "symlink", "copy") && is_portable_managed(path)) { (void) maybe_enable_disable(bus, path, true); - (void) maybe_start_stop(bus, path, true, wait); + (void) maybe_start_stop_restart(bus, path, "StartUnit", wait); + } + } + + r = sd_bus_message_exit_container(reply); + if (r < 0) + return r; + + if (!arg_no_block) { + r = bus_wait_for_jobs(wait, arg_quiet, NULL); + if (r < 0) + return r; + } + + return 0; +} + +static int maybe_stop_enable_restart(sd_bus *bus, sd_bus_message *reply) { + _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *wait = NULL; + int r; + + if (!arg_enable && !arg_now) + return 0; + + if (!arg_no_block) { + r = bus_wait_for_jobs_new(bus, &wait); + if (r < 0) + return log_error_errno(r, "Could not watch jobs: %m"); + } + + r = sd_bus_message_rewind(reply, true); + if (r < 0) + return r; + + /* First we get a list of units that were definitely removed, not just re-attached, + * so we can also stop them if the user asked us to. */ + r = sd_bus_message_enter_container(reply, 'a', "(sss)"); + if (r < 0) + return bus_log_parse_error(r); + + for (;;) { + char *type, *path, *source; + + r = sd_bus_message_read(reply, "(sss)", &type, &path, &source); + if (r < 0) + return bus_log_parse_error(r); + if (r == 0) + break; + + if (streq(type, "unlink") && is_portable_managed(path)) + (void) maybe_start_stop_restart(bus, path, "StopUnit", wait); + } + + r = sd_bus_message_exit_container(reply); + if (r < 0) + return r; + + /* Then we get a list of units that were either added or changed, so that we can + * enable them and/or restart them if the user asked us to. */ + r = sd_bus_message_enter_container(reply, 'a', "(sss)"); + if (r < 0) + return bus_log_parse_error(r); + + for (;;) { + char *type, *path, *source; + + r = sd_bus_message_read(reply, "(sss)", &type, &path, &source); + if (r < 0) + return bus_log_parse_error(r); + if (r == 0) + break; + + if (STR_IN_SET(type, "symlink", "copy") && is_portable_managed(path)) { + (void) maybe_enable_disable(bus, path, true); + (void) maybe_start_stop_restart(bus, path, "RestartUnit", wait); } } @@ -588,7 +668,7 @@ static int maybe_stop_disable(sd_bus *bus, char *image, char *argv[]) { if (r < 0) return bus_log_parse_error(r); - (void) maybe_start_stop(bus, name, false, wait); + (void) maybe_start_stop_restart(bus, name, "StopUnit", wait); (void) maybe_enable_disable(bus, name, false); } @@ -604,7 +684,7 @@ static int maybe_stop_disable(sd_bus *bus, char *image, char *argv[]) { return 0; } -static int attach_image(int argc, char *argv[], void *userdata) { +static int attach_reattach_image(int argc, char *argv[], const char *method) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL, *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; @@ -612,6 +692,9 @@ static int attach_image(int argc, char *argv[], void *userdata) { _cleanup_free_ char *image = NULL; int r; + assert(method); + assert(STR_IN_SET(method, "AttachImage", "ReattachImage")); + r = determine_image(argv[1], false, &image); if (r < 0) return r; @@ -626,7 +709,7 @@ static int attach_image(int argc, char *argv[], void *userdata) { (void) polkit_agent_open_if_enabled(arg_transport, arg_ask_password); - r = bus_message_new_method_call(bus, &m, bus_portable_mgr, "AttachImage"); + r = bus_message_new_method_call(bus, &m, bus_portable_mgr, method); if (r < 0) return bus_log_create_error(r); @@ -644,17 +727,31 @@ static int attach_image(int argc, char *argv[], void *userdata) { r = sd_bus_call(bus, m, 0, &error, &reply); if (r < 0) - return log_error_errno(r, "Failed to attach image: %s", bus_error_message(&error, r)); + return log_error_errno(r, "%s failed: %s", method, bus_error_message(&error, r)); (void) maybe_reload(&bus); print_changes(reply); - (void) maybe_enable_start(bus, reply); + if (streq(method, "AttachImage")) + (void) maybe_enable_start(bus, reply); + else { + /* ReattachImage returns 2 lists - removed units first, and changed/added second */ + print_changes(reply); + (void) maybe_stop_enable_restart(bus, reply); + } return 0; } +static int attach_image(int argc, char *argv[], void *userdata) { + return attach_reattach_image(argc, argv, "AttachImage"); +} + +static int reattach_image(int argc, char *argv[], void *userdata) { + return attach_reattach_image(argc, argv, "ReattachImage"); +} + static int detach_image(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -920,6 +1017,8 @@ static int help(int argc, char *argv[], void *userdata) { " Attach the specified portable service image\n" " detach NAME|PATH [PREFIX...]\n" " Detach the specified portable service image\n" + " reattach NAME|PATH [PREFIX...]\n" + " Reattach the specified portable service image\n" " inspect NAME|PATH [PREFIX...]\n" " Show details of specified portable service image\n" " is-attached NAME|PATH Query if portable service image is attached\n" @@ -1108,6 +1207,7 @@ static int run(int argc, char *argv[]) { { "read-only", 2, 3, 0, read_only_image }, { "remove", 2, VERB_ANY, 0, remove_image }, { "set-limit", 3, 3, 0, set_limit }, + { "reattach", 2, VERB_ANY, 0, reattach_image }, {} }; diff --git a/src/portable/portabled-bus.c b/src/portable/portabled-bus.c index 20a33dc671..c31ab092b4 100644 --- a/src/portable/portabled-bus.c +++ b/src/portable/portabled-bus.c @@ -299,6 +299,10 @@ finish: return r; } +static int method_reattach_image(sd_bus_message *message, void *userdata, sd_bus_error *error) { + return redirect_method_to_image(userdata, message, error, bus_image_common_reattach); +} + static int method_remove_image(sd_bus_message *message, void *userdata, sd_bus_error *error) { return redirect_method_to_image(userdata, message, error, bus_image_common_remove); } @@ -362,6 +366,7 @@ const sd_bus_vtable manager_vtable[] = { SD_BUS_METHOD("GetImageState", "s", "s", method_get_image_state, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("AttachImage", "sassbs", "a(sss)", method_attach_image, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("DetachImage", "sb", "a(sss)", method_detach_image, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("ReattachImage", "sassbs", "a(sss)a(sss)", method_reattach_image, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("RemoveImage", "s", NULL, method_remove_image, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("MarkImageReadOnly", "sb", NULL, method_mark_image_read_only, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("SetImageLimit", "st", NULL, method_set_image_limit, SD_BUS_VTABLE_UNPRIVILEGED), @@ -369,18 +374,13 @@ const sd_bus_vtable manager_vtable[] = { SD_BUS_VTABLE_END }; -int reply_portable_changes(sd_bus_message *m, const PortableChange *changes, size_t n_changes) { - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; +static int reply_portable_compose_message(sd_bus_message *reply, const PortableChange *changes, size_t n_changes) { size_t i; int r; - assert(m); + assert(reply); assert(changes || n_changes == 0); - r = sd_bus_message_new_method_return(m, &reply); - if (r < 0) - return r; - r = sd_bus_message_open_container(reply, 'a', "(sss)"); if (r < 0) return r; @@ -398,5 +398,49 @@ int reply_portable_changes(sd_bus_message *m, const PortableChange *changes, siz if (r < 0) return r; + return 0; +} + +int reply_portable_changes(sd_bus_message *m, const PortableChange *changes, size_t n_changes) { + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + int r; + + assert(m); + + r = sd_bus_message_new_method_return(m, &reply); + if (r < 0) + return r; + + r = reply_portable_compose_message(reply, changes, n_changes); + if (r < 0) + return r; + + return sd_bus_send(NULL, reply, NULL); +} + +int reply_portable_changes_pair( + sd_bus_message *m, + const PortableChange *changes_first, + size_t n_changes_first, + const PortableChange *changes_second, + size_t n_changes_second) { + + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + int r; + + assert(m); + + r = sd_bus_message_new_method_return(m, &reply); + if (r < 0) + return r; + + r = reply_portable_compose_message(reply, changes_first, n_changes_first); + if (r < 0) + return r; + + r = reply_portable_compose_message(reply, changes_second, n_changes_second); + if (r < 0) + return r; + return sd_bus_send(NULL, reply, NULL); } diff --git a/src/portable/portabled-bus.h b/src/portable/portabled-bus.h index e8e4c3a600..7da366c1c3 100644 --- a/src/portable/portabled-bus.h +++ b/src/portable/portabled-bus.h @@ -8,3 +8,4 @@ extern const sd_bus_vtable manager_vtable[]; int reply_portable_changes(sd_bus_message *m, const PortableChange *changes, size_t n_changes); +int reply_portable_changes_pair(sd_bus_message *m, const PortableChange *changes_first, size_t n_changes_first, const PortableChange *changes_second, size_t n_changes_second); diff --git a/src/portable/portabled-image-bus.c b/src/portable/portabled-image-bus.c index babdf4197f..630648ba3c 100644 --- a/src/portable/portabled-image-bus.c +++ b/src/portable/portabled-image-bus.c @@ -422,6 +422,186 @@ static int bus_image_method_remove(sd_bus_message *message, void *userdata, sd_b return bus_image_common_remove(NULL, message, NULL, userdata, error); } +/* Given two PortableChange arrays, return a new array that has all elements of the first that are + * not also present in the second, comparing the basename of the path values. */ +static int normalize_portable_changes( + const PortableChange *changes_attached, + size_t n_changes_attached, + const PortableChange *changes_detached, + size_t n_changes_detached, + PortableChange **ret_changes, + size_t *ret_n_changes) { + + PortableChange *changes = NULL; + size_t n_changes = 0; + int r = 0; + + assert(ret_n_changes); + assert(ret_changes); + + if (n_changes_detached == 0) + return 0; /* Nothing to do */ + + changes = new0(PortableChange, n_changes_attached + n_changes_detached); + if (!changes) + return -ENOMEM; + + /* Corner case: only detached, nothing attached */ + if (n_changes_attached == 0) { + memcpy(changes, changes_detached, sizeof(PortableChange) * n_changes_detached); + *ret_changes = TAKE_PTR(changes); + *ret_n_changes = n_changes_detached; + + return 0; + } + + for (size_t i = 0; i < n_changes_detached; ++i) { + bool found = false; + + for (size_t j = 0; j < n_changes_attached; ++j) + if (streq(basename(changes_detached[i].path), basename(changes_attached[j].path))) { + found = true; + break; + } + + if (!found) { + _cleanup_free_ char *path = NULL, *source = NULL; + + path = strdup(changes_detached[i].path); + if (!path) { + r = -ENOMEM; + goto fail; + } + + if (changes_detached[i].source) { + source = strdup(changes_detached[i].source); + if (!source) { + r = -ENOMEM; + goto fail; + } + } + + changes[n_changes++] = (PortableChange) { + .type = changes_detached[i].type, + .path = TAKE_PTR(path), + .source = TAKE_PTR(source), + }; + } + } + + *ret_n_changes = n_changes; + *ret_changes = TAKE_PTR(changes); + + return 0; + +fail: + portable_changes_free(changes, n_changes); + return r; +} + +int bus_image_common_reattach( + Manager *m, + sd_bus_message *message, + const char *name_or_path, + Image *image, + sd_bus_error *error) { + + PortableChange *changes_detached = NULL, *changes_attached = NULL, *changes_gone = NULL; + size_t n_changes_detached = 0, n_changes_attached = 0, n_changes_gone = 0; + _cleanup_strv_free_ char **matches = NULL; + PortableFlags flags = PORTABLE_REATTACH; + const char *profile, *copy_mode; + int runtime, r; + + assert(message); + assert(name_or_path || image); + + if (!m) { + assert(image); + m = image->userdata; + } + + r = sd_bus_message_read_strv(message, &matches); + if (r < 0) + return r; + + r = sd_bus_message_read(message, "sbs", &profile, &runtime, ©_mode); + if (r < 0) + return r; + + if (streq(copy_mode, "symlink")) + flags |= PORTABLE_PREFER_SYMLINK; + else if (streq(copy_mode, "copy")) + flags |= PORTABLE_PREFER_COPY; + else if (!isempty(copy_mode)) + return sd_bus_reply_method_errorf(message, SD_BUS_ERROR_INVALID_ARGS, "Unknown copy mode '%s'", copy_mode); + + if (runtime) + flags |= PORTABLE_RUNTIME; + + r = bus_image_acquire(m, + message, + name_or_path, + image, + BUS_IMAGE_AUTHENTICATE_ALL, + "org.freedesktop.portable1.attach-images", + &image, + error); + if (r < 0) + return r; + if (r == 0) /* Will call us back */ + return 1; + + r = portable_detach( + sd_bus_message_get_bus(message), + image->path, + flags, + &changes_detached, + &n_changes_detached, + error); + if (r < 0) + goto finish; + + r = portable_attach( + sd_bus_message_get_bus(message), + image->path, + matches, + profile, + flags, + &changes_attached, + &n_changes_attached, + error); + if (r < 0) + goto finish; + + /* We want to return the list of units really removed by the detach, + * and not added again by the attach */ + r = normalize_portable_changes(changes_attached, n_changes_attached, + changes_detached, n_changes_detached, + &changes_gone, &n_changes_gone); + if (r < 0) + goto finish; + + /* First, return the units that are gone (so that the caller can stop them) + * Then, return the units that are changed/added (so that the caller can + * start/restart/enable them) */ + r = reply_portable_changes_pair(message, + changes_gone, n_changes_gone, + changes_attached, n_changes_attached); + if (r < 0) + goto finish; + +finish: + portable_changes_free(changes_detached, n_changes_detached); + portable_changes_free(changes_attached, n_changes_attached); + portable_changes_free(changes_gone, n_changes_gone); + return r; +} + +static int bus_image_method_reattach(sd_bus_message *message, void *userdata, sd_bus_error *error) { + return bus_image_common_reattach(NULL, message, NULL, userdata, error); +} + int bus_image_common_mark_read_only( Manager *m, sd_bus_message *message, @@ -532,6 +712,7 @@ const sd_bus_vtable image_vtable[] = { SD_BUS_METHOD("GetState", NULL, "s", bus_image_method_get_state, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("Attach", "assbs", "a(sss)", bus_image_method_attach, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("Detach", "b", "a(sss)", bus_image_method_detach, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("Reattach", "assbs", "a(sss)a(sss)", bus_image_method_reattach, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("Remove", NULL, NULL, bus_image_method_remove, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("MarkReadOnly", "b", NULL, bus_image_method_mark_read_only, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("SetLimit", "t", NULL, bus_image_method_set_limit, SD_BUS_VTABLE_UNPRIVILEGED), diff --git a/src/portable/portabled-image-bus.h b/src/portable/portabled-image-bus.h index 50f1d935b1..e5faf43d90 100644 --- a/src/portable/portabled-image-bus.h +++ b/src/portable/portabled-image-bus.h @@ -10,6 +10,7 @@ int bus_image_common_get_os_release(Manager *m, sd_bus_message *message, const c int bus_image_common_get_metadata(Manager *m, sd_bus_message *message, const char *name_or_path, Image *image, sd_bus_error *error); int bus_image_common_attach(Manager *m, sd_bus_message *message, const char *name_or_path, Image *image, sd_bus_error *error); int bus_image_common_remove(Manager *m, sd_bus_message *message, const char *name_or_path, Image *image, sd_bus_error *error); +int bus_image_common_reattach(Manager *m, sd_bus_message *message, const char *name_or_path, Image *image, sd_bus_error *error); int bus_image_common_mark_read_only(Manager *m, sd_bus_message *message, const char *name_or_path, Image *image, sd_bus_error *error); int bus_image_common_set_limit(Manager *m, sd_bus_message *message, const char *name_or_path, Image *image, sd_bus_error *error); diff --git a/test/TEST-58-PORTABLE/Makefile b/test/TEST-58-PORTABLE/Makefile new file mode 120000 index 0000000000..e9f93b1104 --- /dev/null +++ b/test/TEST-58-PORTABLE/Makefile @@ -0,0 +1 @@ +../TEST-01-BASIC/Makefile \ No newline at end of file diff --git a/test/TEST-58-PORTABLE/test.sh b/test/TEST-58-PORTABLE/test.sh new file mode 100755 index 0000000000..98e697962e --- /dev/null +++ b/test/TEST-58-PORTABLE/test.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*- +# ex: ts=8 sw=4 sts=4 et filetype=sh +set -e +TEST_DESCRIPTION="test systemd-portabled" +IMAGE_NAME="portabled" +TEST_NO_NSPAWN=1 +TEST_INSTALL_VERITY_MINIMAL=1 + +. $TEST_BASE_DIR/test-functions + +# Need loop devices for mounting images +test_append_files() { + ( + instmods loop =block + instmods squashfs =squashfs + instmods dm_verity =md + install_dmevent + generate_module_dependencies + inst_binary losetup + inst_binary mksquashfs + inst_binary unsquashfs + install_verity_minimal + ) +} + +do_test "$@" 58 diff --git a/test/units/testsuite-58.service b/test/units/testsuite-58.service new file mode 100644 index 0000000000..47deba7d53 --- /dev/null +++ b/test/units/testsuite-58.service @@ -0,0 +1,7 @@ +[Unit] +Description=TEST-58-PORTABLE + +[Service] +ExecStartPre=rm -f /failed /testok +ExecStart=/usr/lib/systemd/tests/testdata/units/%N.sh +Type=oneshot diff --git a/test/units/testsuite-58.sh b/test/units/testsuite-58.sh new file mode 100755 index 0000000000..b5b05b42d9 --- /dev/null +++ b/test/units/testsuite-58.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env bash +# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*- +# ex: ts=8 sw=4 sts=4 et filetype=sh +set -ex +set -o pipefail + +export SYSTEMD_LOG_LEVEL=debug + +portablectl attach --now --runtime /usr/share/minimal_0.raw app0 + +systemctl is-active app0.service +systemctl is-active app0-foo.service +set +o pipefail +set +e +systemctl is-active app0-bar.service && exit 1 +set -e +set -o pipefail + +portablectl reattach --now --runtime /usr/share/minimal_1.raw app0 + +systemctl is-active app0.service +systemctl is-active app0-bar.service +set +o pipefail +set +e +systemctl is-active app0-foo.service && exit 1 +set -e +set -o pipefail + +portablectl list | grep -q -F "minimal_1" + +portablectl detach --now --runtime /usr/share/minimal_1.raw app0 + +portablectl list | grep -q -F "No images." + +# portablectl also works with directory paths rather than images + +unsquashfs -dest /tmp/minimal_0 /usr/share/minimal_0.raw +unsquashfs -dest /tmp/minimal_1 /usr/share/minimal_1.raw + +portablectl attach --copy=symlink --now --runtime /tmp/minimal_0 app0 + +systemctl is-active app0.service +systemctl is-active app0-foo.service +set +o pipefail +set +e +systemctl is-active app0-bar.service && exit 1 +set -e +set -o pipefail + +portablectl reattach --now --enable --runtime /tmp/minimal_1 app0 + +systemctl is-active app0.service +systemctl is-active app0-bar.service +set +o pipefail +set +e +systemctl is-active app0-foo.service && exit 1 +set -e +set -o pipefail + +portablectl list | grep -q -F "minimal_1" + +portablectl detach --now --enable --runtime /tmp/minimal_1 app0 + +portablectl list | grep -q -F "No images." + +echo OK > /testok + +exit 0