From d394d095ab6dc32f1ee45c75e51b56ca9a443548 Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@redmoonstudios.org>
Date: Thu, 11 Sep 2014 18:03:45 +0200
Subject: [PATCH] containers: Don't descend into mounts on destroy.

This tells the sad tale of @the-kenny who had bind-mounted his home
directory into a container. After doing `nixos-container destroy` he
discovered that his home directory went from "full of precious data" to
"no more data".

We want to avoid having similar sad tales in the future, so this now also
check this in the containers VM test.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
---
 .../modules/virtualisation/nixos-container.pl | 23 ++++++++++++--
 nixos/tests/containers.nix                    | 30 +++++++++++++++++++
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/nixos/modules/virtualisation/nixos-container.pl b/nixos/modules/virtualisation/nixos-container.pl
index 7403a42f0f1..9ae5331786c 100644
--- a/nixos/modules/virtualisation/nixos-container.pl
+++ b/nixos/modules/virtualisation/nixos-container.pl
@@ -201,15 +201,32 @@ sub runInContainer {
     die "cannot run ‘nsenter’: $!\n";
 }
 
+# Remove a directory while recursively unmounting all mounted filesystems within
+# that directory and unmounting/removing that directory afterwards as well.
+#
+# NOTE: If the specified path is a mountpoint, its contents will be removed,
+#       only mountpoints underneath that path will be unmounted properly.
+sub safeRemoveTree {
+    my ($path) = @_;
+    system("find", $path, "-mindepth", "1", "-xdev",
+           "(", "-type", "d", "-exec", "mountpoint", "-q", "{}", ";", ")",
+           "-exec", "umount", "-fR", "{}", "+");
+    system("rm", "--one-file-system", "-rf", $path);
+    if (-e $path) {
+        system("umount", "-fR", $path);
+        system("rm", "--one-file-system", "-rf", $path);
+    }
+}
+
 if ($action eq "destroy") {
     die "$0: cannot destroy declarative container (remove it from your configuration.nix instead)\n"
         unless POSIX::access($confFile, &POSIX::W_OK);
 
     stopContainer if isContainerRunning;
 
-    rmtree($profileDir) if -e $profileDir;
-    rmtree($gcRootsDir) if -e $gcRootsDir;
-    rmtree($root) if -e $root;
+    safeRemoveTree($profileDir) if -e $profileDir;
+    safeRemoveTree($gcRootsDir) if -e $gcRootsDir;
+    safeRemoveTree($root) if -e $root;
     unlink($confFile) or die;
 }
 
diff --git a/nixos/tests/containers.nix b/nixos/tests/containers.nix
index 13d98d74207..331324139a1 100644
--- a/nixos/tests/containers.nix
+++ b/nixos/tests/containers.nix
@@ -56,12 +56,35 @@ import ./make-test.nix {
 
       die if $id1 eq $id2;
 
+      # Put the root of $id2 into a bind mount.
+      $machine->succeed(
+        "mv /var/lib/containers/$id2 /id2-bindmount",
+        "mount --bind /id2-bindmount /var/lib/containers/$id1"
+      );
+
       my $ip1 = $machine->succeed("nixos-container show-ip $id1");
       chomp $ip1;
       my $ip2 = $machine->succeed("nixos-container show-ip $id2");
       chomp $ip2;
       die if $ip1 eq $ip2;
 
+      # Create a directory and a file we can later check if it still exists
+      # after destruction of the container.
+      $machine->succeed(
+        "mkdir /nested-bindmount",
+        "echo important data > /nested-bindmount/dummy",
+      );
+
+      # Create a directory with a dummy file and bind-mount it into both
+      # containers.
+      foreach ($id1, $id2) {
+        my $importantPath = "/var/lib/containers/$_/very/important/data";
+        $machine->succeed(
+          "mkdir -p $importantPath",
+          "mount --bind /nested-bindmount $importantPath"
+        );
+      }
+
       # Start one of them.
       $machine->succeed("nixos-container start $id1");
 
@@ -72,6 +95,13 @@ import ./make-test.nix {
       $machine->succeed("nixos-container destroy $id1");
       $machine->succeed("nixos-container destroy $id2");
 
+      $machine->succeed(
+        # Check whether destruction of any container has killed important data
+        "grep -qF 'important data' /nested-bindmount/dummy",
+        # Ensure that the container path is gone
+        "test ! -e /var/lib/containers/$id1"
+      );
+
       # Destroying a declarative container should fail.
       $machine->fail("nixos-container destroy webserver");
     '';