From cc33bdb239c4535f6d32dbef923bbe039831ba9d Mon Sep 17 00:00:00 2001
From: Michael Still <mikal@stillhq.com>
Date: Thu, 2 Nov 2017 16:16:12 +1100
Subject: [PATCH] Convert ext filesystem resizes to privsep.

This patch introduces the first code in the privsep directory which
_does_not_ run with escalated premissions. This is a requirement because
the privsep code has a restricted python path when executing. This
pattern will be used for other methods which are only sometimes
escalated.

Change-Id: Ie09e40d6476dcabda2d599e96701d419e3e8bdf0
blueprint: hurrah-for-privsep
---
 nova/privsep/fs.py                    | 25 +++++++++++++++++++++++++
 nova/tests/unit/virt/disk/test_api.py | 27 +++++++++++++++++----------
 nova/virt/disk/api.py                 | 16 +++-------------
 3 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/nova/privsep/fs.py b/nova/privsep/fs.py
index e637a05e3d8a..34e56690b4da 100644
--- a/nova/privsep/fs.py
+++ b/nova/privsep/fs.py
@@ -18,10 +18,14 @@ Helpers for filesystem related routines.
 """
 
 from oslo_concurrency import processutils
+from oslo_log import log as logging
 
 import nova.privsep
 
 
+LOG = logging.getLogger(__name__)
+
+
 @nova.privsep.sys_admin_pctxt.entrypoint
 def mount(fstype, device, mountpoint, options):
     mount_cmd = ['mount']
@@ -123,3 +127,24 @@ def remove_device_maps(device):
 def get_filesystem_type(device):
     return processutils.execute('blkid', '-o', 'value', '-s', 'TYPE', device,
                                 check_exit_code=[0, 2])
+
+
+@nova.privsep.sys_admin_pctxt.entrypoint
+def resize2fs(image, check_exit_code):
+    unprivileged_resize2fs(image, check_exit_code)
+
+
+# NOTE(mikal): this method is deliberately not wrapped in a privsep entrypoint
+def unprivileged_resize2fs(image, check_exit_code):
+    try:
+        processutils.execute('e2fsck',
+                             '-fp',
+                             image,
+                             check_exit_code=[0, 1, 2])
+    except processutils.ProcessExecutionError as exc:
+        LOG.debug("Checking the file system with e2fsck has failed, "
+                  "the resize will be aborted. (%s)", exc)
+    else:
+        processutils.execute('resize2fs',
+                             image,
+                             check_exit_code=check_exit_code)
diff --git a/nova/tests/unit/virt/disk/test_api.py b/nova/tests/unit/virt/disk/test_api.py
index 0b8ac060fd1a..df2911c3f793 100644
--- a/nova/tests/unit/virt/disk/test_api.py
+++ b/nova/tests/unit/virt/disk/test_api.py
@@ -59,7 +59,7 @@ class APITestCase(test.NoDBTestCase):
         self.assertTrue(api.is_image_extendable(image))
         mock_exec.assert_called_once_with('e2label', imgfile)
 
-    @mock.patch.object(utils, 'execute', autospec=True)
+    @mock.patch('oslo_concurrency.processutils.execute', autospec=True)
     def test_resize2fs_success(self, mock_exec):
         imgfile = tempfile.NamedTemporaryFile()
         self.addCleanup(imgfile.close)
@@ -70,16 +70,24 @@ class APITestCase(test.NoDBTestCase):
             [mock.call('e2fsck',
                        '-fp',
                        imgfile,
-                       check_exit_code=[0, 1, 2],
-                       run_as_root=False),
+                       check_exit_code=[0, 1, 2]),
              mock.call('resize2fs',
                        imgfile,
-                       check_exit_code=False,
-                       run_as_root=False)])
+                       check_exit_code=False)])
 
-    @mock.patch.object(utils, 'execute', autospec=True,
-                       side_effect=processutils.ProcessExecutionError(
-                           "fs error"))
+    @mock.patch('oslo_concurrency.processutils.execute')
+    @mock.patch('nova.privsep.fs.resize2fs')
+    def test_resize2fs_success_as_root(self, mock_resize, mock_exec):
+        imgfile = tempfile.NamedTemporaryFile()
+        self.addCleanup(imgfile.close)
+
+        api.resize2fs(imgfile, run_as_root=True)
+
+        mock_exec.assert_not_called()
+        mock_resize.assert_called()
+
+    @mock.patch('oslo_concurrency.processutils.execute', autospec=True,
+                side_effect=processutils.ProcessExecutionError("fs error"))
     def test_resize2fs_e2fsck_fails(self, mock_exec):
         imgfile = tempfile.NamedTemporaryFile()
         self.addCleanup(imgfile.close)
@@ -88,8 +96,7 @@ class APITestCase(test.NoDBTestCase):
         mock_exec.assert_called_once_with('e2fsck',
                                           '-fp',
                                           imgfile,
-                                          check_exit_code=[0, 1, 2],
-                                          run_as_root=False)
+                                          check_exit_code=[0, 1, 2])
 
     @mock.patch.object(api, 'can_resize_image', autospec=True,
                        return_value=True)
diff --git a/nova/virt/disk/api.py b/nova/virt/disk/api.py
index a3ac3f28b71f..04f8b93a855f 100644
--- a/nova/virt/disk/api.py
+++ b/nova/virt/disk/api.py
@@ -121,20 +121,10 @@ def mkfs(os_type, fs_label, target, run_as_root=True, specified_fs=None):
 
 
 def resize2fs(image, check_exit_code=False, run_as_root=False):
-    try:
-        utils.execute('e2fsck',
-                      '-fp',
-                      image,
-                      check_exit_code=[0, 1, 2],
-                      run_as_root=run_as_root)
-    except processutils.ProcessExecutionError as exc:
-        LOG.debug("Checking the file system with e2fsck has failed, "
-                  "the resize will be aborted. (%s)", exc)
+    if run_as_root:
+        nova.privsep.fs.resize2fs(image, check_exit_code)
     else:
-        utils.execute('resize2fs',
-                      image,
-                      check_exit_code=check_exit_code,
-                      run_as_root=run_as_root)
+        nova.privsep.fs.unprivileged_resize2fs(image, check_exit_code)
 
 
 def get_disk_size(path):