From 6c273960d56e4bbb90ee23cd9c8cb70f3e6b82ea Mon Sep 17 00:00:00 2001 From: Faizan Barmawer Date: Mon, 16 Nov 2015 03:31:25 -0800 Subject: [PATCH] Replace rootwrap_config and rootwrap_helper_cmd with root_helper ironic-lib will use the command set in the new 'root_helper' conf parameter to execute commands as root user. If this configuration paramter is not specified, ironic-lib will execute commands with run_as_root=False. This configuration is not set by default. 'rootwrap_config' and 'rootwrap_helper_cmd' configs are deleted, since they are replaced by the new 'root_helper' config. This fix also delivers a sample ironic-lib.filters file, which should be used with rootwrap command. Change-Id: I61ef7c15237c995e9d4cc85095ac48a30a8f6c7d Closes-bug: #1515943 --- etc/rootwrap.d/ironic-lib.filters | 19 +++++++++++++++++++ ironic_lib/tests/test_utils.py | 30 ++++++++++++++++++++---------- ironic_lib/utils.py | 28 +++++++++++----------------- 3 files changed, 50 insertions(+), 27 deletions(-) create mode 100644 etc/rootwrap.d/ironic-lib.filters diff --git a/etc/rootwrap.d/ironic-lib.filters b/etc/rootwrap.d/ironic-lib.filters new file mode 100644 index 00000000..acce7a7b --- /dev/null +++ b/etc/rootwrap.d/ironic-lib.filters @@ -0,0 +1,19 @@ +# An ironic-lib.filters to be used with rootwrap command. +# The following commands should be used in filters for disk manipulation. +# This file should be owned by (and only-writeable by) the root user. + +[Filters] +# ironic_lib/disk_utils.py +blkid: CommandFilter, blkid, root +blockdev: CommandFilter, blockdev, root +hexdump: CommandFilter, hexdump, root +qemu-img: CommandFilter, qemu-img, root + +# ironic_lib/utils.py +mkswap: CommandFilter, mkswap, root +mkfs: CommandFilter, mkfs, root +dd: CommandFilter, dd, root + +# ironic_lib/disk_partitioner.py +fuser: CommandFilter, fuser, root +parted: CommandFilter, parted, root diff --git a/ironic_lib/tests/test_utils.py b/ironic_lib/tests/test_utils.py index c4c30144..fe9e2aaf 100644 --- a/ironic_lib/tests/test_utils.py +++ b/ironic_lib/tests/test_utils.py @@ -146,7 +146,8 @@ grep foo execute_mock): utils.execute('foo', use_standard_locale=True) execute_mock.assert_called_once_with('foo', - env_variables={'LC_ALL': 'C'}) + env_variables={'LC_ALL': 'C'}, + run_as_root=False) @mock.patch.object(processutils, 'execute') def test_execute_use_standard_locale_with_env_variables(self, @@ -155,27 +156,36 @@ grep foo env_variables={'foo': 'bar'}) execute_mock.assert_called_once_with('foo', env_variables={'LC_ALL': 'C', - 'foo': 'bar'}) + 'foo': 'bar'}, + run_as_root=False) @mock.patch.object(processutils, 'execute') def test_execute_not_use_standard_locale(self, execute_mock): utils.execute('foo', use_standard_locale=False, env_variables={'foo': 'bar'}) execute_mock.assert_called_once_with('foo', - env_variables={'foo': 'bar'}) - - def test_execute_get_root_helper(self): - with mock.patch.object(processutils, 'execute') as execute_mock: - helper = utils._get_root_helper() - utils.execute('foo', run_as_root=True) - execute_mock.assert_called_once_with('foo', run_as_root=True, - root_helper=helper) + env_variables={'foo': 'bar'}, + run_as_root=False) def test_execute_without_root_helper(self): + CONF.set_override('root_helper', None, group='ironic_lib') with mock.patch.object(processutils, 'execute') as execute_mock: utils.execute('foo', run_as_root=False) execute_mock.assert_called_once_with('foo', run_as_root=False) + def test_execute_without_root_helper_run_as_root(self): + CONF.set_override('root_helper', None, group='ironic_lib') + with mock.patch.object(processutils, 'execute') as execute_mock: + utils.execute('foo', run_as_root=True) + execute_mock.assert_called_once_with('foo', run_as_root=False) + + def test_execute_with_root_helper(self): + CONF.set_override('root_helper', 'sudo', group='ironic_lib') + with mock.patch.object(processutils, 'execute') as execute_mock: + utils.execute('foo', run_as_root=True) + execute_mock.assert_called_once_with('foo', run_as_root=True, + root_helper='sudo') + class MkfsTestCase(test_base.BaseTestCase): diff --git a/ironic_lib/utils.py b/ironic_lib/utils.py index 05587b42..a09a8173 100644 --- a/ironic_lib/utils.py +++ b/ironic_lib/utils.py @@ -31,15 +31,10 @@ from ironic_lib.common.i18n import _LW from ironic_lib import exception utils_opts = [ - cfg.StrOpt('rootwrap_config', - default="", - help='Path to the rootwrap configuration file to use for ' - 'running commands as root.', - deprecated_group='DEFAULT'), - cfg.StrOpt('rootwrap_helper_cmd', - default="", - help='Command that is used with the path to the rootwrap ' - 'configuration file, when running commands as root.'), + cfg.StrOpt('root_helper', + default=None, + help='Command that is prefixed to commands that are run as ' + 'root. If not specified, no commands are run as root.'), ] CONF = cfg.CONF @@ -48,12 +43,6 @@ CONF.register_opts(utils_opts, group='ironic_lib') LOG = logging.getLogger(__name__) -def _get_root_helper(): - root_helper = '%s %s' % (CONF.ironic_lib.rootwrap_helper_cmd, - CONF.ironic_lib.rootwrap_config) - return root_helper - - def execute(*cmd, **kwargs): """Convenience wrapper around oslo's execute() method. @@ -71,8 +60,13 @@ def execute(*cmd, **kwargs): env = kwargs.pop('env_variables', os.environ.copy()) env['LC_ALL'] = 'C' kwargs['env_variables'] = env - if kwargs.get('run_as_root') and 'root_helper' not in kwargs: - kwargs['root_helper'] = _get_root_helper() + + # If root_helper config is not specified, no commands are run as root. + if not CONF.ironic_lib.root_helper: + kwargs['run_as_root'] = False + else: + kwargs['root_helper'] = CONF.ironic_lib.root_helper + result = processutils.execute(*cmd, **kwargs) LOG.debug('Execution completed, command line is "%s"', ' '.join(map(str, cmd)))