From: Terry Wilson Date: Wed, 21 Jan 2015 20:19:06 +0000 (-0600) Subject: Add run_as_root option to utils.execute X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=804b3b61386b3262be112d88dfd70eef08d764e4;p=openstack-build%2Fneutron-build.git 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 8fd1945b6..94193cf1d 100644 --- 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 ec0db57c6..8fa464402 100644 --- 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):