Add run_as_root option to utils.execute

This change adds the run_as_root option that exists in the
processutils.execute method. This option makes it possible
to just look up the root_helper from the config when
necessary, while still making it possible to not use any
privilege escalation.

Currently, processutils.execute exposes some eventlet issues
in the neutron codebase, otherwise this patch would just move
us over to using it.

Change-Id: Ic9ebe77dada4f7f044f009ff04494906d4f50ae2
Partially-Implements: blueprint rootwrap-daemon-mode

diff --git a/neutron/agent/linux/utils.py b/neutron/agent/linux/utils.py
index 8fd1945b61434bf2b50b790be6b51c976eaee818..94193cf1d8ea82925762037577bb0d05a9505451 100644 (file)
--- a/neutron/agent/linux/utils.py
+++ b/neutron/agent/linux/utils.py
@@ -23,8 +23,10 @@ import tempfile
 
 from eventlet.green import subprocess
 from eventlet import greenthread
+from oslo_config import cfg
 from oslo_utils import excutils
 
+from neutron.agent.common import config
 from neutron.common import constants
 from neutron.common import utils
 from neutron.i18n import _LE
@@ -32,6 +34,7 @@ from neutron.openstack.common import log as logging
 
 
 LOG = logging.getLogger(__name__)
+config.register_root_helper(cfg.CONF)
 
 
 def create_process(cmd, root_helper=None, addl_env=None):
@@ -58,9 +61,16 @@ def create_process(cmd, root_helper=None, addl_env=None):
     return obj, cmd
 
 
+# TODO(twilson) Remove root_helper argument in favor of run_as_root
+# root_helper= is only intended to handle old-style code that passes the
+# root_helper option exclusively, the run_as_root option is for new-style code
+# that leaves root_helper handling to the execute() function. The two options
+# should not be used in conjunction.
 def execute(cmd, root_helper=None, process_input=None, addl_env=None,
             check_exit_code=True, return_stderr=False, log_fail_as_error=True,
-            extra_ok_codes=None):
+            extra_ok_codes=None, run_as_root=False):
+    if not root_helper and run_as_root:
+        root_helper = config.get_root_helper(cfg.CONF)
     try:
         obj, cmd = create_process(cmd, root_helper=root_helper,
                                   addl_env=addl_env)

diff --git a/neutron/tests/unit/agent/linux/test_utils.py b/neutron/tests/unit/agent/linux/test_utils.py
index ec0db57c6c0e20d2adc569124c979f31f0694dd7..8fa464402440b963d6659789bbe814b19a642f6a 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_utils.py
+++ b/neutron/tests/unit/agent/linux/test_utils.py
@@ -21,27 +21,15 @@ from neutron.tests import base
 _marker = object()
 
 
-class FakeCreateProcess(object):
-    class FakeStdin(object):
-        def close(self):
-            pass
-
-    def __init__(self, returncode):
-        self.returncode = returncode
-        self.stdin = self.FakeStdin()
-
-    def communicate(self, process_input=None):
-        return '', ''
-
-
 class AgentUtilsExecuteTest(base.BaseTestCase):
     def setUp(self):
         super(AgentUtilsExecuteTest, self).setUp()
         self.root_helper = "echo"
         self.test_file = self.get_temp_file_path('test_execute.tmp')
         open(self.test_file, 'w').close()
-        self.mock_popen_p = mock.patch("subprocess.Popen.communicate")
-        self.mock_popen = self.mock_popen_p.start()
+        self.process = mock.patch('eventlet.green.subprocess.Popen').start()
+        self.process.return_value.returncode = 0
+        self.mock_popen = self.process.return_value.communicate
 
     def test_without_helper(self):
         expected = "%s\n" % self.test_file
@@ -89,34 +77,33 @@ class AgentUtilsExecuteTest(base.BaseTestCase):
         self.assertEqual(result, expected)
 
     def test_return_code_log_error_raise_runtime(self):
-        with mock.patch.object(utils, 'create_process') as create_process:
-            create_process.return_value = FakeCreateProcess(1), 'ls'
-            with mock.patch.object(utils, 'LOG') as log:
-                self.assertRaises(RuntimeError, utils.execute,
-                                  ['ls'])
-                self.assertTrue(log.error.called)
+        self.mock_popen.return_value = ('', '')
+        self.process.return_value.returncode = 1
+        with mock.patch.object(utils, 'LOG') as log:
+            self.assertRaises(RuntimeError, utils.execute,
+                              ['ls'])
+            self.assertTrue(log.error.called)
 
     def test_return_code_log_error_no_raise_runtime(self):
-        with mock.patch.object(utils, 'create_process') as create_process:
-            create_process.return_value = FakeCreateProcess(1), 'ls'
-            with mock.patch.object(utils, 'LOG') as log:
-                utils.execute(['ls'], check_exit_code=False)
-                self.assertTrue(log.error.called)
+        self.mock_popen.return_value = ('', '')
+        self.process.return_value.returncode = 1
+        with mock.patch.object(utils, 'LOG') as log:
+            utils.execute(['ls'], check_exit_code=False)
+            self.assertTrue(log.error.called)
 
     def test_return_code_log_debug(self):
-        with mock.patch.object(utils, 'create_process') as create_process:
-            create_process.return_value = FakeCreateProcess(0), 'ls'
-            with mock.patch.object(utils, 'LOG') as log:
-                utils.execute(['ls'])
-                self.assertTrue(log.debug.called)
+        self.mock_popen.return_value = ('', '')
+        with mock.patch.object(utils, 'LOG') as log:
+            utils.execute(['ls'])
+            self.assertTrue(log.debug.called)
 
     def test_return_code_raise_runtime_do_not_log_fail_as_error(self):
-        with mock.patch.object(utils, 'create_process') as create_process:
-            create_process.return_value = FakeCreateProcess(1), 'ls'
-            with mock.patch.object(utils, 'LOG') as log:
-                self.assertRaises(RuntimeError, utils.execute,
-                                  ['ls'], log_fail_as_error=False)
-                self.assertTrue(log.debug.called)
+        self.mock_popen.return_value = ('', '')
+        self.process.return_value.returncode = 1
+        with mock.patch.object(utils, 'LOG') as log:
+            self.assertRaises(RuntimeError, utils.execute,
+                              ['ls'], log_fail_as_error=False)
+            self.assertFalse(log.error.called)
 
 
 class AgentUtilsGetInterfaceMAC(base.BaseTestCase):