deploy: Copy parent directories for modified config files

Previously, in the case where a parent directory of a modified config
file was removed, we would throw an exception.  This happens when
switching from a tree that has some software (e.g. firewalld), to one
that does not.

While it's nice to have this warning that your config file probably no
longer applies, there's no need to make it so...fatal.

It's particularly problematic that the only easy workaround is to
remove the config files from your current tree - which breaks
rollback.

The solution then is for for us to take ownership of the parent
directories too into the new /etc.  Admins can clean up these files
afterwards at any time.

https://bugzilla.gnome.org/show_bug.cgi?id=734293

2 files changed, 158 insertions, 61 deletions

diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index fbb3ea1d..905854f1 100644
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -156,6 +156,50 @@ copy_one_file_fsync_at (int              src_parent_dfd,
 }
 
 static gboolean
+dirfd_copy_attributes_and_xattrs (int            src_parent_dfd,
+                                  const char    *src_name,
+                                  int            src_dfd,
+                                  int            dest_dfd,
+                                  GCancellable  *cancellable,
+                                  GError       **error)
+{
+  gboolean ret = FALSE;
+  struct stat src_stbuf;
+  gs_unref_variant GVariant *xattrs = NULL;
+
+  /* Clone all xattrs first, so we get the SELinux security context
+   * right.  This will allow other users access if they have ACLs, but
+   * oh well.
+   */ 
+  if (!dfd_and_name_get_all_xattrs (src_parent_dfd, src_name,
+                                    &xattrs, cancellable, error))
+    goto out;
+  if (!gs_fd_set_all_xattrs (dest_dfd, xattrs,
+                             cancellable, error))
+    goto out;
+
+  if (fstat (src_dfd, &src_stbuf) != 0)
+    {
+      ot_util_set_error_from_errno (error, errno);
+      goto out;
+    }
+  if (fchown (dest_dfd, src_stbuf.st_uid, src_stbuf.st_gid) != 0)
+    {
+      ot_util_set_error_from_errno (error, errno);
+      goto out;
+    }
+  if (fchmod (dest_dfd, src_stbuf.st_mode) != 0)
+    {
+      ot_util_set_error_from_errno (error, errno);
+      goto out;
+    }
+
+  ret = TRUE;
+ out:
+  return ret;
+}
+
+static gboolean
 copy_dir_recurse_fsync (int              src_parent_dfd,
                         int              dest_parent_dfd,
                         const char      *name,
@@ -163,12 +207,10 @@ copy_dir_recurse_fsync (int              src_parent_dfd,
                         GError         **error)
 {
   gboolean ret = FALSE;
-  struct stat src_stbuf;
   int src_dfd = -1;
   int dest_dfd = -1;
   DIR *srcd = NULL;
   struct dirent *dent;
-  gs_unref_variant GVariant *xattrs = NULL;
 
   if (!ot_gopendirat (src_parent_dfd, name, TRUE, &src_dfd, error))
     goto out;
@@ -183,18 +225,10 @@ copy_dir_recurse_fsync (int              src_parent_dfd,
   if (!ot_gopendirat (dest_parent_dfd, name, TRUE, &dest_dfd, error))
     goto out;
 
-  /* Clone all xattrs first, so we get the SELinux security context
-   * right.  This will allow other users access if they have ACLs, but
-   * oh well.
-   */ 
-  if (!dfd_and_name_get_all_xattrs (src_parent_dfd, name,
-                                    &xattrs,
-                                    cancellable, error))
-    goto out;
-  if (!gs_fd_set_all_xattrs (dest_dfd, xattrs,
-                             cancellable, error))
+  if (!dirfd_copy_attributes_and_xattrs (src_parent_dfd, name, src_dfd, dest_dfd,
+                                         cancellable, error))
     goto out;
-
+ 
   srcd = fdopendir (src_dfd);
   if (!srcd)
     {
@@ -233,22 +267,6 @@ copy_dir_recurse_fsync (int              src_parent_dfd,
         }
     }
 
-  if (fstat (src_dfd, &src_stbuf) != 0)
-    {
-      ot_util_set_error_from_errno (error, errno);
-      goto out;
-    }
-  if (fchown (dest_dfd, src_stbuf.st_uid, src_stbuf.st_gid) != 0)
-    {
-      ot_util_set_error_from_errno (error, errno);
-      goto out;
-    }
-  if (fchmod (dest_dfd, src_stbuf.st_mode) != 0)
-    {
-      ot_util_set_error_from_errno (error, errno);
-      goto out;
-    }
-
   /* And finally, fsync the fd */
   if (fsync (dest_dfd) != 0)
     {
@@ -271,15 +289,88 @@ copy_dir_recurse_fsync (int              src_parent_dfd,
   return ret;
 }
 
+static gboolean
+ensure_directory_from_template (int                 orig_etc_fd,
+                                int                 modified_etc_fd,
+                                int                 new_etc_fd,
+                                const char         *path,
+                                int                *out_dfd,
+                                GCancellable       *cancellable,
+                                GError            **error)
+{
+  gboolean ret = FALSE;
+  int src_dfd = -1;
+  int target_dfd = -1;
+
+  g_assert (path != NULL);
+  g_assert (*path != '/' && *path != '\0');
+
+  if (!ot_gopendirat (modified_etc_fd, path, TRUE, &src_dfd, error))
+    goto out;
+
+  /* Create with mode 0700, we'll fchmod/fchown later */
+ again:
+  if (mkdirat (new_etc_fd, path, 0700) != 0)
+    {
+      if (errno == EEXIST)
+        {
+          /* Fall through */
+        }
+      else if (errno == ENOENT)
+        {
+          gs_free char *parent_path = g_path_get_dirname (path);
+
+          if (strcmp (parent_path, ".") != 0)
+            {
+              if (!ensure_directory_from_template (orig_etc_fd, modified_etc_fd, new_etc_fd,
+                                                   parent_path, NULL, cancellable, error))
+                goto out;
+
+              /* Loop */
+              goto again;
+            }
+          else
+            {
+              /* Fall through...shouldn't happen, but we'll propagate
+               * an error from open. */
+            }
+        }
+      else
+        {
+          ot_util_set_error_from_errno (error, errno);
+          g_prefix_error (error, "mkdirat: ");
+          goto out;
+        }
+    }
+
+  if (!ot_gopendirat (new_etc_fd, path, TRUE, &target_dfd, error))
+    goto out;
+
+  if (!dirfd_copy_attributes_and_xattrs (modified_etc_fd, path, src_dfd, target_dfd,
+                                         cancellable, error))
+    goto out;
+
+  ret = TRUE;
+  if (out_dfd)
+    {
+      g_assert (target_dfd != -1);
+      *out_dfd = target_dfd;
+      target_dfd = -1;
+    }
+ out:
+  if (src_dfd != -1)
+    (void) close (src_dfd);
+  if (target_dfd != -1)
+    (void) close (target_dfd);
+  return ret;
+}
+
 /**
  * copy_modified_config_file:
  *
  * Copy @file from @modified_etc to @new_etc, overwriting any existing
  * file there.  The @file may refer to a regular file, a symbolic
  * link, or a directory.  Directories will be copied recursively.
- *
- * Note this function does not (yet) handle the case where a directory
- * needed by a modified file is deleted in a newer tree.
  */
 static gboolean
 copy_modified_config_file (int                 orig_etc_fd,
@@ -292,8 +383,6 @@ copy_modified_config_file (int                 orig_etc_fd,
   gboolean ret = FALSE;
   struct stat modified_stbuf;
   struct stat new_stbuf;
-  const char *parent_slash;
-  gs_free char *parent_path = NULL;
   int dest_parent_dfd = -1;
 
   if (fstatat (modified_etc_fd, path, &modified_stbuf, AT_SYMLINK_NOFOLLOW) < 0)
@@ -303,30 +392,16 @@ copy_modified_config_file (int                 orig_etc_fd,
       goto out;
     }
 
-  parent_slash = strrchr (path, '/');
-  if (parent_slash != NULL)
+  if (strchr (path, '/') != NULL)
     {
-      parent_path = g_strndup (path, parent_slash - path);
-      dest_parent_dfd = ot_opendirat (new_etc_fd, parent_path, FALSE);
-      if (dest_parent_dfd == -1)
-        {
-          if (errno == ENOENT)
-            {
-              g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
-                           "New tree removes parent directory '%s', cannot merge",
-                           parent_path);
-            }
-          else
-            {
-              g_prefix_error (error, "openat: ");
-              ot_util_set_error_from_errno (error, errno);
-            }
-          goto out;
-        }
+      gs_free char *parent = g_path_get_dirname (path);
+
+      if (!ensure_directory_from_template (orig_etc_fd, modified_etc_fd, new_etc_fd,
+                                           parent, &dest_parent_dfd, cancellable, error))
+        goto out;
     }
   else
     {
-      parent_path = NULL;
       dest_parent_dfd = dup (new_etc_fd);
       if (dest_parent_dfd == -1)
         {
@@ -335,6 +410,8 @@ copy_modified_config_file (int                 orig_etc_fd,
         }
     }
 
+  g_assert (dest_parent_dfd != -1);
+
   if (fstatat (new_etc_fd, path, &new_stbuf, AT_SYMLINK_NOFOLLOW) < 0)
     {
       if (errno == ENOENT)
@@ -351,7 +428,7 @@ copy_modified_config_file (int                 orig_etc_fd,
         {
           g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
                        "Modified config file newly defaults to directory '%s', cannot merge",
-                       parent_path);
+                       path);
           goto out;
         }
       else
diff --git a/tests/test-admin-deploy-etcmerge-cornercases.sh b/tests/test-admin-deploy-etcmerge-cornercases.sh
index 17900941..1464fb65 100644
--- a/tests/test-admin-deploy-etcmerge-cornercases.sh
+++ b/tests/test-admin-deploy-etcmerge-cornercases.sh
@@ -37,8 +37,12 @@ echo "rev=${rev}"
 ostree admin --sysroot=sysroot deploy --karg=root=LABEL=MOO --karg=quiet --os=testos testos:testos/buildmaster/x86_64-runtime
 assert_has_dir sysroot/boot/ostree/testos-${bootcsum}
 
-# Ok, let's create a long directory chain with custom permissions
 etc=sysroot/ostree/deploy/testos/deploy/${rev}.0/etc
+
+# modified config file
+echo "a modified config file" > ${etc}/NetworkManager/nm.conf
+
+# Ok, let's create a long directory chain with custom permissions
 mkdir -p ${etc}/a/long/dir/chain
 mkdir -p ${etc}/a/long/dir/forking
 chmod 700 ${etc}/a
@@ -47,14 +51,24 @@ chmod 777 ${etc}/a/long/dir
 chmod 707 ${etc}/a/long/dir/chain
 chmod 700 ${etc}/a/long/dir/forking
 
+# Symlink to nonexistent path, to ensure we aren't walking symlinks
+ln -s no-such-file ${etc}/a/link-to-no-such-file
+
+# Remove a directory
+rm ${etc}/testdirectory -rf
+
 # Now deploy a new commit
 os_repository_new_commit
 ostree --repo=sysroot/ostree/repo remote add --set=gpg-verify=false testos file://$(pwd)/testos-repo testos/buildmaster/x86_64-runtime
 ostree admin --sysroot=sysroot upgrade --os=testos
 newrev=$(ostree --repo=sysroot/ostree/repo rev-parse testos/buildmaster/x86_64-runtime)
 echo "newrev=${newrev}"
+newroot=sysroot/ostree/deploy/testos/deploy/${newrev}.0
+newetc=${newroot}/etc
+
+assert_file_has_content ${newroot}/usr/etc/NetworkManager/nm.conf "a default daemon file"
+assert_file_has_content ${newetc}/NetworkManager/nm.conf "a modified config file"
 
-newetc=sysroot/ostree/deploy/testos/deploy/${newrev}.0/etc
 assert_file_has_mode() {
   stat -c '%a' $1 > mode.txt
   if ! grep -q -e "$2" mode.txt; then 
@@ -63,6 +77,7 @@ assert_file_has_mode() {
   fi
   rm -f mode.txt
 }
+
 assert_file_has_mode ${newetc}/a 700
 assert_file_has_mode ${newetc}/a/long 770
 assert_file_has_mode ${newetc}/a/long/dir 777
@@ -70,6 +85,11 @@ assert_file_has_mode ${newetc}/a/long/dir/chain 707
 assert_file_has_mode ${newetc}/a/long/dir/forking 700
 assert_file_has_mode ${newetc}/a/long/dir 777
 
+test -L ${newetc}/a/link-to-no-such-file || assert_not_reached "should have symlink"
+
+assert_has_dir ${newroot}/usr/etc/testdirectory
+assert_not_has_dir ${newetc}/testdirectory
+
 echo "ok"
 
 # Add /etc/initially-empty
@@ -116,10 +136,10 @@ ostree --repo=${test_tmpdir}/testos-repo commit -b testos/buildmaster/x86_64-run
 cd ${test_tmpdir}
 newconfpath=sysroot/ostree/deploy/testos/deploy/${rev}.0/etc/initially-empty/mynewfile
 touch ${newconfpath}
-if ostree admin --sysroot=sysroot upgrade --os=testos 2>err.txt; then
-    assert_not_reached "upgrade should have failed"
-fi
-assert_file_has_content err.txt "New tree removes parent directory"
+ostree admin --sysroot=sysroot upgrade --os=testos
+rev=$(ostree --repo=sysroot/ostree/repo rev-parse testos/buildmaster/x86_64-runtime)
+assert_not_has_file sysroot/ostree/deploy/testos/deploy/${rev}.0/usr/etc/initially-empty
+assert_has_file sysroot/ostree/deploy/testos/deploy/${rev}.0/etc/initially-empty/mynewfile
 rm ${newconfpath}
 
 echo "ok"