Add "transient" unlock

I was thinking a bit more recently about the "live" changes
stuff https://github.com/coreos/rpm-ostree/issues/639
(particularly since https://github.com/coreos/rpm-ostree/pull/2060 )
and I realized reading the last debates in that issue that
there's really a much simpler solution; do exactly the same
thing we do for `ostree admin unlock`, except mount it read-only
by default.

Then, anything that wants to modify it does the same thing
libostree does for `/sysroot` and `/boot` as of recently; create
a new mount namespace and do the modifications there.

The advantages of this are numerous.  First, we already have
all of the code, it's basically just plumbing through a new
entry in the state enumeration and passing `MS_RDONLY` into
the `mount()` system call.

"live" changes here also naturally don't persist, unlike what
we are currently doing in rpm-ostree.

diff --git a/src/libostree/ostree-deployment.c b/src/libostree/ostree-deployment.c
index 6532a973dcdeb66194602a0397c30d821ac3a202..70e1bc4928e29eda3c7b71b925430bd458047370 100644 (file)
--- a/src/libostree/ostree-deployment.c
+++ b/src/libostree/ostree-deployment.c
@@ -318,6 +318,8 @@ ostree_deployment_unlocked_state_to_string (OstreeDeploymentUnlockedState state)
       return "hotfix";
     case OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT:
       return "development";
+    case OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT:
+      return "transient";
     }
   g_assert_not_reached ();
 }

diff --git a/src/libostree/ostree-deployment.h b/src/libostree/ostree-deployment.h
index 756e39d2b4c84edf872c5d62bccc3174401bbb63..dcfa25ec0b473aa7fed5668a8dc0c60991862467 100644 (file)
--- a/src/libostree/ostree-deployment.h
+++ b/src/libostree/ostree-deployment.h
@@ -99,7 +99,8 @@ char *ostree_deployment_get_origin_relpath (OstreeDeployment *self);
 typedef enum {
   OSTREE_DEPLOYMENT_UNLOCKED_NONE,
   OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT,
-  OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX
+  OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX,
+  OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT,
 } OstreeDeploymentUnlockedState;
 
 _OSTREE_PUBLIC

diff --git a/src/libostree/ostree-sysroot-private.h b/src/libostree/ostree-sysroot-private.h
index 96670b137b87d3ec4aa5a28ec172f527f2348149..fa1e53361c6bf77fc9ad8efe565abb9a7c353931 100644 (file)
--- a/src/libostree/ostree-sysroot-private.h
+++ b/src/libostree/ostree-sysroot-private.h
@@ -85,6 +85,7 @@ struct OstreeSysroot {
 #define _OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED "/run/ostree/staged-deployment-locked"
 #define _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_DIR "/run/ostree/deployment-state/"
 #define _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_DEVELOPMENT "unlocked-development"
+#define _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_TRANSIENT "unlocked-transient"
 
 gboolean
 _ostree_sysroot_ensure_writable (OstreeSysroot      *self,

diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c
index 0dc7a3920be03e97108ed33e579ba9f1761a013c..b211fea7231bad8b6994817ab445764e8462befc 100644 (file)
--- a/src/libostree/ostree-sysroot.c
+++ b/src/libostree/ostree-sysroot.c
@@ -747,9 +747,13 @@ parse_deployment (OstreeSysroot       *self,
   ret_deployment->unlocked = OSTREE_DEPLOYMENT_UNLOCKED_NONE;
   g_autofree char *unlocked_development_path =
     _ostree_sysroot_get_runstate_path (ret_deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_DEVELOPMENT);
+  g_autofree char *unlocked_transient_path =
+    _ostree_sysroot_get_runstate_path (ret_deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_TRANSIENT);
   struct stat stbuf;
   if (lstat (unlocked_development_path, &stbuf) == 0)
     ret_deployment->unlocked = OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT;
+  else if (lstat (unlocked_transient_path, &stbuf) == 0)
+    ret_deployment->unlocked = OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT;
   else
     {
       GKeyFile *origin = ostree_deployment_get_origin (ret_deployment);
@@ -1932,6 +1936,8 @@ ostree_sysroot_deployment_unlock (OstreeSysroot     *self,
 
   const char *ovl_options = NULL;
   static const char hotfix_ovl_options[] = "lowerdir=usr,upperdir=.usr-ovl-upper,workdir=.usr-ovl-work";
+  g_autofree char *unlock_ovldir = NULL;
+
   switch (unlocked_state)
     {
     case OSTREE_DEPLOYMENT_UNLOCKED_NONE:
@@ -1951,11 +1957,12 @@ ostree_sysroot_deployment_unlock (OstreeSysroot     *self,
       }
       break;
     case OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT:
+    case OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT:
       {
+        unlock_ovldir = g_strdup ("/var/tmp/ostree-unlock-ovl.XXXXXX");
         /* We're just doing transient development/hacking?  Okay,
          * stick the overlayfs bits in /var/tmp.
          */
-        char *development_ovldir = strdupa ("/var/tmp/ostree-unlock-ovl.XXXXXX");
         const char *development_ovl_upper;
         const char *development_ovl_work;
 
@@ -1966,14 +1973,14 @@ ostree_sysroot_deployment_unlock (OstreeSysroot     *self,
                                                     "/usr", usr_mode, error))
             return FALSE;
 
-          if (g_mkdtemp_full (development_ovldir, 0755) == NULL)
+          if (g_mkdtemp_full (unlock_ovldir, 0755) == NULL)
             return glnx_throw_errno_prefix (error, "mkdtemp");
         }
 
-        development_ovl_upper = glnx_strjoina (development_ovldir, "/upper");
+        development_ovl_upper = glnx_strjoina (unlock_ovldir, "/upper");
         if (!mkdir_unmasked (AT_FDCWD, development_ovl_upper, usr_mode, cancellable, error))
           return FALSE;
-        development_ovl_work = glnx_strjoina (development_ovldir, "/work");
+        development_ovl_work = glnx_strjoina (unlock_ovldir, "/work");
         if (!mkdir_unmasked (AT_FDCWD, development_ovl_work, usr_mode, cancellable, error))
           return FALSE;
         ovl_options = glnx_strjoina ("lowerdir=usr,upperdir=", development_ovl_upper,
@@ -1996,6 +2003,9 @@ ostree_sysroot_deployment_unlock (OstreeSysroot     *self,
       return glnx_throw_errno_prefix (error, "fork");
     else if (mount_child == 0)
       {
+        int mountflags = 0;
+        if (unlocked_state == OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT)
+          mountflags |= MS_RDONLY;
         /* Child process. Do NOT use any GLib API here; it's not generally fork() safe.
          *
          * TODO: report errors across a pipe (or use the journal?) rather than
@@ -2003,7 +2013,7 @@ ostree_sysroot_deployment_unlock (OstreeSysroot     *self,
          */
         if (fchdir (deployment_dfd) < 0)
           err (1, "fchdir");
-        if (mount ("overlay", "/usr", "overlay", 0, ovl_options) < 0)
+        if (mount ("overlay", "/usr", "overlay", mountflags, ovl_options) < 0)
           err (1, "mount");
         exit (EXIT_SUCCESS);
       }
@@ -2036,15 +2046,19 @@ ostree_sysroot_deployment_unlock (OstreeSysroot     *self,
         return FALSE;
       break;
     case OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT:
+    case OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT:
       {
         g_autofree char *devpath =
-          _ostree_sysroot_get_runstate_path (deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_DEVELOPMENT);
+          unlocked_state == OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT ?
+            _ostree_sysroot_get_runstate_path (deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_DEVELOPMENT)
+          :
+            _ostree_sysroot_get_runstate_path (deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_TRANSIENT);
         g_autofree char *devpath_parent = dirname (g_strdup (devpath));
 
         if (!glnx_shutil_mkdir_p_at (AT_FDCWD, devpath_parent, 0755, cancellable, error))
           return FALSE;
 
-        if (!g_file_set_contents (devpath, "", 0, error))
+        if (!g_file_set_contents (devpath, unlock_ovldir, -1, error))
           return FALSE;
       }
     }

diff --git a/src/ostree/ot-admin-builtin-unlock.c b/src/ostree/ot-admin-builtin-unlock.c
index cd46618301b586fc40ce66c92494b89028841d62..6c265f54a58a5cd292be1f858b2ebb32d96416c8 100644 (file)
--- a/src/ostree/ot-admin-builtin-unlock.c
+++ b/src/ostree/ot-admin-builtin-unlock.c
@@ -31,9 +31,11 @@
 #include <err.h>
 
 static gboolean opt_hotfix;
+static gboolean opt_transient;
 
 static GOptionEntry options[] = {
   { "hotfix", 0, 0, G_OPTION_ARG_NONE, &opt_hotfix, "Retain changes across reboots", NULL },
+  { "transient", 0, 0, G_OPTION_ARG_NONE, &opt_transient, "Mount overlayfs read-only by default", NULL },
   { NULL }
 };
 
@@ -67,7 +69,17 @@ ot_admin_builtin_unlock (int argc, char **argv, OstreeCommandInvocation *invocat
       goto out;
     }
 
-  target_state = opt_hotfix ? OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX : OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT;
+  if (opt_hotfix && opt_transient)
+    {
+      glnx_throw (error, "Cannot specify both --hotfix and --transient");
+      goto out;
+    }
+  else if (opt_hotfix)
+    target_state = OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX;
+  else if (opt_transient)
+    target_state = OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT;
+  else
+    target_state = OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT;
 
   if (!ostree_sysroot_deployment_unlock (sysroot, booted_deployment,
                                          target_state, cancellable, error))
@@ -87,6 +99,10 @@ ot_admin_builtin_unlock (int argc, char **argv, OstreeCommandInvocation *invocat
       g_print ("Development mode enabled.  A writable overlayfs is now mounted on /usr.\n"
                "All changes there will be discarded on reboot.\n");
       break;
+    case OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT:
+      g_print ("A writable overlayfs is prepared for /usr, but is mounted read-only by default.\n"
+               "All changes there will be discarded on reboot.\n");
+      break;
     }
 
   ret = TRUE;

diff --git a/tests/kolainst/destructive/unlock-transient.sh b/tests/kolainst/destructive/unlock-transient.sh
new file mode 100755 (executable)
index 0000000..8dce222
--- /dev/null
+++ b/tests/kolainst/destructive/unlock-transient.sh
@@ -0,0 +1,34 @@
+#!/bin/bash
+# Test unlock --transient
+set -xeuo pipefail
+
+. ${KOLA_EXT_DATA}/libinsttest.sh
+
+testfile=/usr/share/writable-usr-test
+
+case "${AUTOPKGTEST_REBOOT_MARK:-}" in
+  "") 
+    require_writable_sysroot
+    assert_not_has_file "${testfile}"
+    ostree admin unlock --transient
+    # It's still read-only
+    if touch ${testfile}; then
+      fatal "modified /usr"
+    fi
+    # But, we can affect it in a new mount namespace
+    unshare -m -- /bin/sh -c 'mount -o remount,rw /usr && echo hello from transient unlock >'"${testfile}"
+    assert_file_has_content "${testfile}" "hello from transient unlock"
+    # Still can't write to it from the outer namespace
+    if touch ${testfile} || rm -v "${testfile}" 2>/dev/null; then
+      fatal "modified ${testfile}"
+    fi
+    /tmp/autopkgtest-reboot 2
+    ;;
+  "2")
+    if test -f "${testfile}"; then
+      fatal "${testfile} persisted across reboot?"
+    fi
+    echo "ok unlock transient"
+    ;;
+  *) fatal "Unexpected boot mark ${AUTOPKGTEST_REBOOT_MARK}"
+esac