Move prepare-root karg helpers into otcore, add unit tests
authorColin Walters <walters@verbum.org>
Wed, 23 Aug 2023 20:06:23 +0000 (16:06 -0400)
committerColin Walters <walters@verbum.org>
Wed, 23 Aug 2023 21:11:10 +0000 (17:11 -0400)
Add long overdue unit testing coverage for this, which
at least slightly closes out the android boot CI gap.

Actually, this *copies* the karg parsing code into otcore because
it now uses glib, which we're not yet using in the static
prepare-root.  It's pretty tempting to drop support for the
static prepare root entirely.  But for now we'll live with some
code duplication.

Makefile-otcore.am
src/libostree/ostree-impl-system-generator.c
src/libotcore/otcore-prepare-root.c [new file with mode: 0644]
src/libotcore/otcore.h
src/switchroot/ostree-prepare-root.c
tests/test-otcore.c

index 8accf858448368aa98f6331c11f2f73f6efb48c5..8252ead00ee3a0dbfb7cfa985fbf56cbac9e2a96 100644 (file)
@@ -18,6 +18,7 @@ noinst_LTLIBRARIES += libotcore.la
 libotcore_la_SOURCES = \
        src/libotcore/otcore.h \
     src/libotcore/otcore-ed25519-verify.c \
+    src/libotcore/otcore-prepare-root.c \
        $(NULL)
 
 libotcore_la_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/libglnx -I$(srcdir)/src/libotutil -DLOCALEDIR=\"$(datadir)/locale\" $(OT_INTERNAL_GIO_UNIX_CFLAGS) $(OT_INTERNAL_GPGME_CFLAGS) $(OT_DEP_CRYPTO_LIBS) $(LIBSYSTEMD_CFLAGS)
index c5db4aa3396b75f2cf6b22be0fada528f5450149..ad785eb9ecab9cc1e95c1a428e98b8725e1a8d49 100644 (file)
@@ -34,7 +34,7 @@
 #include "ostree-core-private.h"
 #include "ostree-mount-util.h"
 #include "ostree-sysroot-private.h"
-#include "ostree.h"
+#include "otcore.h"
 
 #ifdef HAVE_LIBMOUNT
 typedef FILE OtLibMountFile;
@@ -260,7 +260,7 @@ _ostree_impl_system_generator (const char *normal_dir, const char *early_dir, co
    * exit so that we don't error, but at the same time work where switchroot
    * is PID 1 (and so hasn't created /run/ostree-booted).
    */
-  g_autofree char *ostree_cmdline = find_proc_cmdline_key (cmdline, "ostree");
+  g_autofree char *ostree_cmdline = otcore_find_proc_cmdline_key (cmdline, "ostree");
   if (!ostree_cmdline)
     return TRUE;
 
diff --git a/src/libotcore/otcore-prepare-root.c b/src/libotcore/otcore-prepare-root.c
new file mode 100644 (file)
index 0000000..f3b1a08
--- /dev/null
@@ -0,0 +1,108 @@
+/*
+ * SPDX-License-Identifier: LGPL-2.0+
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see <https://www.gnu.org/licenses/>.
+ */
+
+#include "config.h"
+
+#include "otcore.h"
+
+static bool
+proc_cmdline_has_key_starting_with (const char *cmdline, const char *key)
+{
+  for (const char *iter = cmdline; iter;)
+    {
+      if (g_str_has_prefix (iter, key))
+        return true;
+
+      iter = strchr (iter, ' ');
+      if (iter == NULL)
+        return false;
+
+      iter += strspn (iter, " ");
+    }
+
+  return false;
+}
+
+// Parse a kernel cmdline to find the provided key.
+// TODO: Deduplicate this with the kernel argument code from libostree.so
+char *
+otcore_find_proc_cmdline_key (const char *cmdline, const char *key)
+{
+  const size_t key_len = strlen (key);
+  for (const char *iter = cmdline; iter;)
+    {
+      const char *next = strchr (iter, ' ');
+      if (strncmp (iter, key, key_len) == 0 && iter[key_len] == '=')
+        {
+          const char *start = iter + key_len + 1;
+          if (next)
+            return strndup (start, next - start);
+
+          return strdup (start);
+        }
+
+      if (next)
+        next += strspn (next, " ");
+
+      iter = next;
+    }
+
+  return NULL;
+}
+
+// Find the target OSTree root filesystem from parsing the provided kernel commandline.
+// If none is found, @out_target will be set to NULL, and the function will return successfully.
+//
+// If invalid data is found, @error will be set.
+gboolean
+otcore_get_ostree_target (const char *cmdline, char **out_target, GError **error)
+{
+  g_assert (cmdline);
+  g_assert (out_target && *out_target == NULL);
+  static const char slot_a[] = "/ostree/root.a";
+  static const char slot_b[] = "/ostree/root.b";
+
+  // First, handle the Android boot case
+  g_autofree char *slot_suffix = otcore_find_proc_cmdline_key (cmdline, "androidboot.slot_suffix");
+  if (slot_suffix)
+    {
+      if (strcmp (slot_suffix, "_a") == 0)
+        {
+          *out_target = strdup (slot_a);
+          return TRUE;
+        }
+      else if (strcmp (slot_suffix, "_b") == 0)
+        {
+          *out_target = strdup (slot_b);
+          return TRUE;
+        }
+      return glnx_throw (error, "androidboot.slot_suffix invalid: %s", slot_suffix);
+    }
+
+  /* Non-A/B androidboot:
+   * https://source.android.com/docs/core/ota/nonab
+   */
+  if (proc_cmdline_has_key_starting_with (cmdline, "androidboot."))
+    {
+      *out_target = strdup (slot_a);
+      return TRUE;
+    }
+
+  // Otherwise, fall back to the default `ostree=` kernel cmdline
+  *out_target = otcore_find_proc_cmdline_key (cmdline, "ostree");
+  return TRUE;
+}
index a5bdb8f9f1c436c2563d2a5bd30e7ddb232e8893..d41758e1c6e2eeeadea513afa65f10e73cbc6700 100644 (file)
@@ -43,6 +43,9 @@ bool otcore_ed25519_init (void);
 gboolean otcore_validate_ed25519_signature (GBytes *data, GBytes *pubkey, GBytes *signature,
                                             bool *out_valid, GError **error);
 
+char *otcore_find_proc_cmdline_key (const char *cmdline, const char *key);
+gboolean otcore_get_ostree_target (const char *cmdline, char **out_target, GError **error);
+
 // Our directory with transient state (eventually /run/ostree-booted should be a link to
 // /run/ostree/booted)
 #define OTCORE_RUN_OSTREE "/run/ostree"
index 155189b9c5c37efaf6a23a5da4a6769bc4a17032..62ea56171b7a6e2cafccd72f5bd0e82207367552 100644 (file)
@@ -150,53 +150,6 @@ sysroot_is_configured_ro (const char *sysroot)
   return g_key_file_get_boolean (repo_config, SYSROOT_KEY, READONLY_KEY, NULL);
 }
 
-static inline char *
-get_aboot_root_slot (const char *slot_suffix)
-{
-  if (strcmp (slot_suffix, "_a") == 0)
-    return strdup ("/ostree/root.a");
-  else if (strcmp (slot_suffix, "_b") == 0)
-    return strdup ("/ostree/root.b");
-
-  errx (EXIT_FAILURE, "androidboot.slot_suffix invalid: %s", slot_suffix);
-
-  return NULL;
-}
-
-static bool
-proc_cmdline_has_key_starting_with (const char *cmdline, const char *key)
-{
-  for (const char *iter = cmdline; iter;)
-    {
-      if (g_str_has_prefix (iter, key))
-        return true;
-
-      iter = strchr (iter, ' ');
-      if (iter == NULL)
-        return false;
-
-      iter += strspn (iter, " ");
-    }
-
-  return false;
-}
-
-static inline char *
-get_ostree_target (const char *cmdline)
-{
-  autofree char *slot_suffix = find_proc_cmdline_key (cmdline, "androidboot.slot_suffix");
-  if (slot_suffix)
-    return get_aboot_root_slot (slot_suffix);
-
-  /* Non-A/B androidboot:
-   * https://source.android.com/docs/core/ota/nonab
-   */
-  if (proc_cmdline_has_key_starting_with (cmdline, "androidboot."))
-    return strdup ("/ostree/root.a");
-
-  return find_proc_cmdline_key (cmdline, "ostree");
-}
-
 static char *
 resolve_deploy_path (const char *root_mountpoint)
 {
@@ -207,9 +160,12 @@ resolve_deploy_path (const char *root_mountpoint)
   if (!kernel_cmdline)
     errx (EXIT_FAILURE, "Failed to read kernel cmdline");
 
-  g_autofree char *ostree_target = get_ostree_target (kernel_cmdline);
+  g_autoptr (GError) error = NULL;
+  g_autofree char *ostree_target = NULL;
+  if (!otcore_get_ostree_target (kernel_cmdline, &ostree_target, &error))
+    errx (EXIT_FAILURE, "Failed to determine ostree target: %s", error->message);
   if (!ostree_target)
-    errx (EXIT_FAILURE, "No ostree target");
+    errx (EXIT_FAILURE, "No ostree target found");
 
   if (snprintf (destpath, sizeof (destpath), "%s/%s", root_mountpoint, ostree_target) < 0)
     err (EXIT_FAILURE, "failed to assemble ostree target path");
index cac4209629800ac974accc8d73df4da5bfd70cd8..03cfb6c8cf9d1ee9443a96a7f1dfb3f9f747c6f7 100644 (file)
@@ -31,11 +31,62 @@ test_ed25519 (void)
   g_clear_error (&error);
 }
 
+static void
+test_prepare_root_cmdline (void)
+{
+  g_autoptr (GError) error = NULL;
+  g_autofree char *target = NULL;
+
+  static const char *notfound_cases[]
+      = { "", "foo", "foo=bar baz  sometest", "xostree foo", "xostree=blah bar", NULL };
+  for (const char **iter = notfound_cases; iter && *iter; iter++)
+    {
+      const char *tcase = *iter;
+      g_assert (otcore_get_ostree_target (tcase, &target, &error));
+      g_assert_no_error (error);
+      g_assert (target == NULL);
+    }
+
+  // Test the default ostree=
+  g_assert (otcore_get_ostree_target ("blah baz=blah ostree=/foo/bar somearg", &target, &error));
+  g_assert_no_error (error);
+  g_assert_cmpstr (target, ==, "/foo/bar");
+  free (g_steal_pointer (&target));
+
+  // Test android boot
+  g_assert (otcore_get_ostree_target ("blah baz=blah androidboot.slot_suffix=_b somearg", &target,
+                                      &error));
+  g_assert_no_error (error);
+  g_assert_cmpstr (target, ==, "/ostree/root.b");
+  free (g_steal_pointer (&target));
+
+  g_assert (otcore_get_ostree_target ("blah baz=blah androidboot.slot_suffix=_a somearg", &target,
+                                      &error));
+  g_assert_no_error (error);
+  g_assert_cmpstr (target, ==, "/ostree/root.a");
+  free (g_steal_pointer (&target));
+
+  // And an expected failure to parse a "c" suffix
+  g_assert (!otcore_get_ostree_target ("blah baz=blah androidboot.slot_suffix=_c somearg", &target,
+                                       &error));
+  g_assert (error);
+  g_assert (target == NULL);
+  g_clear_error (&error);
+
+  // And non-A/B androidboot
+  g_assert (otcore_get_ostree_target ("blah baz=blah androidboot.somethingelse somearg", &target,
+                                      &error));
+  g_assert_no_error (error);
+  g_assert_cmpstr (target, ==, "/ostree/root.a");
+  free (g_steal_pointer (&target));
+}
+
 int
 main (int argc, char **argv)
 {
   g_test_init (&argc, &argv, NULL);
   otcore_ed25519_init ();
   g_test_add_func ("/ed25519", test_ed25519);
+  g_test_add_func ("/prepare-root-cmdline", test_prepare_root_cmdline);
   return g_test_run ();
 }