Use force flag and name validation in bluechictl set-default

Relates to: #944 The bluechictl set-default command used for the force option a separate CLI parameter with true/false values. This is a bit brittle and doesn't take into account other truthy values. Using the already defined --force option results in a much clearer usage of this command. In addition, when passing unit name other than a .target it failed with a rather cryptic error message from systemd. Therefore, lets do a pre-validation inside bluechictl before submitting any request and provide a proper error message. Signed-off-by: Michael Engel <[email protected]>
eclipse-bluechi · Oct 14, 2024 · b8dd4de · b8dd4de
1 parent 94dd30c
commit b8dd4de
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 24 deletions.
diff --git a/data/org.eclipse.bluechi.Node.xml b/data/org.eclipse.bluechi.Node.xml
@@ -272,7 +272,7 @@
 
     <!--
       GetDefaultTarget:
-      @defaulttarget the default target.
+      @defaulttarget The default target.
 
       Get the default value of the system to boot into.
     -->
@@ -282,16 +282,16 @@
 
     <!--
       SetDefaultTarget:
-      @defaulttarget the default target.
-      @force bollean value of force.
-      @out the result of the method.
+      @defaulttarget The default target.
+      @force Boolean value of force.
+      @out The result of the method.
 
       Set the default value of the system to boot into.
     -->
     <method name="SetDefaultTarget">
       <arg name="defaulttarget" type="s" direction="in" />
       <arg name="force" type="b" direction="in" />
-      <arg name="out" type="a(sss)" direction="out" />  
+      <arg name="out" type="a(sss)" direction="out" />
     </method>
 
     <!--

diff --git a/src/bindings/python/bluechi/api.py b/src/bindings/python/bluechi/api.py
@@ -902,7 +902,7 @@ def freeze_unit(self, name: str) -> None:
     def get_default_target(self) -> str:
         """
           GetDefaultTarget:
-        @defaulttarget the default target.
+        @defaulttarget The default target.
 
         Get the default value of the system to boot into.
         """
@@ -1056,9 +1056,9 @@ def set_default_target(
     ) -> List[Tuple[str, str, str]]:
         """
           SetDefaultTarget:
-        @defaulttarget the default target.
-        @force bollean value of force.
-        @out the result of the method.
+        @defaulttarget The default target.
+        @force Boolean value of force.
+        @out The result of the method.
 
         Set the default value of the system to boot into.
         """

diff --git a/src/client/main.c b/src/client/main.c
@@ -63,7 +63,7 @@ const Method methods[] = {
         { "status",          0, ARG_ANY, OPT_WATCH,                               method_status,             usage_method_status             },
         { "set-loglevel",    1, 2,       OPT_NONE,                                method_set_loglevel,       usage_method_set_loglevel       },
         { "get-default",     1, 1,       OPT_NONE,                                method_get_default_target, usage_method_get_default_target },
-        { "set-default",     2, 3,       OPT_NONE,                                method_set_default_target, usage_method_set_default_target },
+        { "set-default",     2, 2,       OPT_NONE,                                method_set_default_target, usage_method_set_default_target },
         { "version",         0, 0,       OPT_NONE,                                method_version,            usage_bluechi                   },
         { NULL,              0, 0,       0,                                       NULL,                      NULL                            }
 };

diff --git a/src/client/method-default-target.c b/src/client/method-default-target.c
@@ -46,15 +46,10 @@ static int method_get_default_target_on(Client *client, char *node_name) {
 }
 
 
-static int method_set_default_target_on(Client *client, char *node_name, char *target, char *force) {
+static int method_set_default_target_on(Client *client, char *node_name, char *target, int force) {
         _cleanup_sd_bus_error_ sd_bus_error error = SD_BUS_ERROR_NULL;
         _cleanup_sd_bus_message_ sd_bus_message *message = NULL;
         int r = 0;
-        bool force_b = 1;
-        if (streq(force, "false")) {
-                force_b = 0;
-        }
-
 
         r = assemble_object_path_string(NODE_OBJECT_PATH_PREFIX, node_name, &client->object_path);
         if (r < 0) {
@@ -71,7 +66,7 @@ static int method_set_default_target_on(Client *client, char *node_name, char *t
                         &message,
                         "sb",
                         target,
-                        force_b);
+                        force);
         if (r < 0) {
                 fprintf(stderr, "Failed to issue method call: %s\n", error.message);
                 return r;
@@ -105,23 +100,34 @@ static int method_set_default_target_on(Client *client, char *node_name, char *t
 
 void usage_method_get_default_target() {
         usage_print_header();
-        usage_print_usage("bluechictl get-default [nodename]");
+        usage_print_usage("bluechictl get-default [nodename] [options]");
+        printf("\n");
+        printf("Available options:\n");
+        printf("  --%s \t shows this help message\n", ARG_HELP);
 }
 
 void usage_method_set_default_target() {
         usage_print_header();
-        usage_print_usage("bluechictl set-default [nodename] [target]");
+        usage_print_usage("bluechictl set-default [nodename] [target] [options]");
+        printf("\n");
+        printf("Available options:\n");
+        printf("  --%s \t shows this help message\n", ARG_HELP);
 }
 
 
 int method_get_default_target(Command *command, void *userdata) {
         return method_get_default_target_on(userdata, command->opargv[0]);
 }
 
+static const char *glob_systemd_target = "*target";
+
 int method_set_default_target(Command *command, void *userdata) {
-        char *force = "true";
-        if (command->opargc == 3) {
-                force = command->opargv[2];
+        if (!match_glob(command->opargv[1], glob_systemd_target)) {
+                fprintf(stderr, "Error: Unit '%s' is not a target unit\n", command->opargv[1]);
+                return 1;
         }
-        return method_set_default_target_on(userdata, command->opargv[0], command->opargv[1], force);
+
+        // mimicking systemctl here and setting the force parameter to 1
+        // https://github.com/systemd/systemd/blob/7c52d5236a3bc85db1755de6a458934be095cd1c/src/systemctl/systemctl-set-default.c#L130
+        return method_set_default_target_on(userdata, command->opargv[0], command->opargv[1], 1);
 }
diff --git a/tests/bluechi_test/bluechictl.py b/tests/bluechi_test/bluechictl.py
@@ -189,7 +189,7 @@ def set_default_target(
         check_result: bool = True,
         expected_result: int = 0,
     ) -> Tuple[Optional[int], Union[Iterator[bytes], Any, Tuple[bytes, bytes]]]:
-        cmd = f"set-default {node_name} {target} true"
+        cmd = f"set-default {node_name} {target}"
         return self._run(
             f"SetDefaultTarget of node {node_name} ",
             cmd,